feat(spec,aip-go): inline K8s schemas, own go.mod, named client, transport ergonomics#274
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 53 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
📝 WalkthroughWalkthroughThis PR extracts the AIP Go client types into a standalone Changesaip-go module extraction, schema expansion, and client publishing
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GatewayClient
participant dynamicTransport
participant TokenSource
participant Gateway
Caller->>GatewayClient: RequestToken(ctx, registrationName, service, ttl)
GatewayClient->>dynamicTransport: RoundTrip(POST /session-token, NoWait=true)
dynamicTransport->>TokenSource: Token(ctx)
TokenSource-->>dynamicTransport: bearer token string
dynamicTransport->>Gateway: POST /session-token Authorization: Bearer <token>
alt HTTP 200
Gateway-->>GatewayClient: JSON body with token
GatewayClient-->>Caller: token string, nil
else HTTP 202
Gateway-->>GatewayClient: JSON body with agentRequestName
GatewayClient-->>Caller: "", PendingApprovalError{AgentRequestName}
else other status
Gateway-->>GatewayClient: error body text
GatewayClient-->>Caller: "", error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
pkg/aip/client.go (2)
11-15: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueUnused
gatewayfield inGatewayClient.The
gatewayfield is stored but never referenced after construction. Consider removing it or documenting its intended future use.♻️ Proposed fix
type GatewayClient struct { - inner *ClientWithResponses - gateway string + inner *ClientWithResponses }And update constructors accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/aip/client.go` around lines 11 - 15, The GatewayClient struct has a gateway field that is never referenced anywhere in the codebase after initialization. Remove the gateway field from the GatewayClient struct definition, and update the constructor function (NewGateway or similar) that populates this field to no longer assign the gateway parameter to the struct. If the field is intended for future use, add a comment explaining the planned usage instead of removing it.
161-167: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd comment explaining why
json.Unmarshalerror is ignored.Per coding guidelines, errors should not be silently ignored. If the intent is to proceed with an empty
AgentRequestNamewhen parsing fails, document this explicitly.♻️ Proposed fix
if resp.HTTPResponse.StatusCode == 202 { var ar struct { AgentRequestName string `json:"agentRequestName"` } - _ = json.Unmarshal(resp.Body, &ar) + // Best-effort parse; proceed with empty name if body is malformed + _ = json.Unmarshal(resp.Body, &ar) return "", &PendingApprovalError{AgentRequestName: ar.AgentRequestName} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/aip/client.go` around lines 161 - 167, In the 202 status code handling block where json.Unmarshal is called on resp.Body to parse the agentRequestName, add an explanatory comment above the json.Unmarshal line documenting why the error is intentionally ignored. The comment should clarify that if JSON parsing fails, the code intentionally proceeds with an empty AgentRequestName value in the PendingApprovalError rather than failing completely, making the error suppression deliberate and transparent to future maintainers.Source: Coding guidelines
api/openapi/openapi.yaml (1)
73-74: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding
maxItemsconstraints to unbounded arrays.The
requestedServicesandallowedSubjectsarrays lackmaxItemsconstraints. While not critical for internal APIs, unbounded arrays can be a DoS vector if clients send excessively large payloads.- requestedServices: { type: array, items: { type: string } } + requestedServices: { type: array, items: { type: string }, maxItems: 100 }Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/openapi/openapi.yaml` around lines 73 - 74, The `requestedServices` and `allowedSubjects` arrays in the OpenAPI specification lack `maxItems` constraints, which can allow clients to send excessively large payloads and create a potential DoS vector. Add a reasonable `maxItems` constraint to both the `requestedServices` array definition (around line 73) and the `allowedSubjects` array definition (around line 87) to limit the maximum number of items each array can contain and prevent unbounded payload sizes.Source: Linters/SAST tools
.github/workflows/publish-clients.yml (2)
10-10: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout actions.Prevents Git credentials from persisting in subsequent steps, reducing exposure if later steps are compromised.
🛡️ Proposed fix
- uses: actions/checkout@v4 + with: + persist-credentials: falseApply to both checkout steps (lines 10 and 29).
Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-clients.yml at line 10, Add the security configuration persist-credentials: false to both checkout action steps in the workflow file. Locate the first actions/checkout@v4 step (around line 10) and the second actions/checkout@v4 step (around line 29), and add persist-credentials: false as a with parameter to each checkout action to prevent Git credentials from being persisted across subsequent workflow steps.Source: Linters/SAST tools
10-10: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConsider pinning actions to commit SHAs for supply chain security.
Using version tags like
@v4can be updated by the action maintainer. Pinning to a specific commit SHA provides stronger guarantees.Also applies to: 13-13, 29-29, 32-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-clients.yml at line 10, Replace all action version tags with specific commit SHAs in the publish-clients.yml workflow file. Find each instance where actions are referenced with version tags (such as actions/checkout@v4 on line 10, and the additional occurrences on lines 13, 29, and 32) and replace the version tag with its corresponding full commit SHA. For example, change `actions/checkout@v4` to `actions/checkout@<full-commit-sha>` where the commit SHA should be the exact commit hash of that action version. This applies to all uses statements that specify actions in the workflow.Source: Linters/SAST tools
Makefile (1)
509-531: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffHardcoded
packageVersion=0.1.0won't match git tags.The workflow publishes on
v*tags but client versions are hardcoded to0.1.0. Consider parameterizing the version or extracting it from the git tag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 509 - 531, The packageVersion and npmVersion are hardcoded to 0.1.0 in the clients-python and clients-typescript targets, but this doesn't align with git tag versioning used in the workflow. Create a VERSION variable at the top of the Makefile (similar to how OPENAPI_GENERATOR_VERSION is defined), and replace the hardcoded 0.1.0 values in both the packageVersion and npmVersion properties with this new VERSION variable. This ensures the generated client versions will match your git tag releases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-clients.yml:
- Around line 1-5: The Publish Clients workflow is missing an explicit
permissions block, which defaults to write-all access and poses a security risk.
Add a permissions block after the on section (after the tags trigger) that
restricts the workflow to only the minimum required permissions needed for
publishing. For a workflow that runs scripts and uploads packages, this
typically means restricting to read-only access for contents or adding only the
specific write permissions needed for release operations, rather than allowing
unrestricted write access to all resources.
In `@go.mod`:
- Line 3: The go.mod file specifies an invalid Go version 1.26.1 which does not
exist as a released version. Update the go directive in go.mod to use a valid,
released Go version (such as 1.21, 1.22, 1.23, or another currently available
stable release) instead of 1.26.1.
---
Nitpick comments:
In @.github/workflows/publish-clients.yml:
- Line 10: Add the security configuration persist-credentials: false to both
checkout action steps in the workflow file. Locate the first actions/checkout@v4
step (around line 10) and the second actions/checkout@v4 step (around line 29),
and add persist-credentials: false as a with parameter to each checkout action
to prevent Git credentials from being persisted across subsequent workflow
steps.
- Line 10: Replace all action version tags with specific commit SHAs in the
publish-clients.yml workflow file. Find each instance where actions are
referenced with version tags (such as actions/checkout@v4 on line 10, and the
additional occurrences on lines 13, 29, and 32) and replace the version tag with
its corresponding full commit SHA. For example, change `actions/checkout@v4` to
`actions/checkout@<full-commit-sha>` where the commit SHA should be the exact
commit hash of that action version. This applies to all uses statements that
specify actions in the workflow.
In `@api/openapi/openapi.yaml`:
- Around line 73-74: The `requestedServices` and `allowedSubjects` arrays in the
OpenAPI specification lack `maxItems` constraints, which can allow clients to
send excessively large payloads and create a potential DoS vector. Add a
reasonable `maxItems` constraint to both the `requestedServices` array
definition (around line 73) and the `allowedSubjects` array definition (around
line 87) to limit the maximum number of items each array can contain and prevent
unbounded payload sizes.
In `@Makefile`:
- Around line 509-531: The packageVersion and npmVersion are hardcoded to 0.1.0
in the clients-python and clients-typescript targets, but this doesn't align
with git tag versioning used in the workflow. Create a VERSION variable at the
top of the Makefile (similar to how OPENAPI_GENERATOR_VERSION is defined), and
replace the hardcoded 0.1.0 values in both the packageVersion and npmVersion
properties with this new VERSION variable. This ensures the generated client
versions will match your git tag releases.
In `@pkg/aip/client.go`:
- Around line 11-15: The GatewayClient struct has a gateway field that is never
referenced anywhere in the codebase after initialization. Remove the gateway
field from the GatewayClient struct definition, and update the constructor
function (NewGateway or similar) that populates this field to no longer assign
the gateway parameter to the struct. If the field is intended for future use,
add a comment explaining the planned usage instead of removing it.
- Around line 161-167: In the 202 status code handling block where
json.Unmarshal is called on resp.Body to parse the agentRequestName, add an
explanatory comment above the json.Unmarshal line documenting why the error is
intentionally ignored. The comment should clarify that if JSON parsing fails,
the code intentionally proceeds with an empty AgentRequestName value in the
PendingApprovalError rather than failing completely, making the error
suppression deliberate and transparent to future maintainers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf55329e-60a5-489a-83fc-90eb29f77527
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumpkg/aip/go.sumis excluded by!**/*.sum
📒 Files selected for processing (46)
.github/workflows/publish-clients.ymlMakefileapi/openapi/openapi.yamlcmd/aipctl/client.gocmd/aipctl/register.gocmd/aipctl/registrations.gocmd/aipctl/token.gocmd/aipctl/token_flow.gocmd/gateway/accuracy_handlers.gocmd/gateway/admin_handlers_test.gocmd/gateway/agent_graduation_policy_handlers.gocmd/gateway/agent_graduation_policy_handlers_test.gocmd/gateway/agent_registration_handlers.gocmd/gateway/agent_registration_lifecycle_handlers.gocmd/gateway/agent_request_lifecycle_handlers.gocmd/gateway/agent_request_review_handlers.gocmd/gateway/agent_request_token_handlers.gocmd/gateway/audit_handlers.gocmd/gateway/cel_validation_test.gocmd/gateway/governed_resource_handlers.gocmd/gateway/handlers_test.gocmd/gateway/integration_agent_graduation_policy_test.gocmd/gateway/integration_agent_registration_test.gocmd/gateway/integration_auth_test.gocmd/gateway/integration_basic_test.gocmd/gateway/integration_registration_approval_test.gocmd/gateway/integration_registration_test.gocmd/gateway/integration_result_test.gocmd/gateway/integration_self_registration_test.gocmd/gateway/integration_session_token_test.gocmd/gateway/integration_sse_test.gocmd/gateway/integration_trust_test.gocmd/gateway/main.gocmd/gateway/optimization_test.gocmd/gateway/registration_sse.gocmd/gateway/safety_policy_handlers.gocmd/gateway/session_handlers.gocmd/gateway/sse.gocmd/gateway/sse_test.gocmd/gateway/trust_profile_handlers.gocmd/gateway/trust_profile_handlers_test.gogo.modpkg/aip/api.gen.gopkg/aip/client.gopkg/aip/go.modpkg/aip/transport.go
ravisantoshgudimetla
force-pushed
the
phase12-aip-go
branch
from
631eb8c to
5710295
Compare
ravisantoshgudimetla
added a commit
that referenced
this pull request
Jun 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-clients.yml:
- Line 13: The GitHub Actions workflow uses floating version tags (like `@v4` and
`@v5`) which are vulnerable to upstream drift and supply chain attacks. Replace
each floating tag reference in the uses clauses at lines 13, 23, 39, and 50 with
their corresponding immutable full commit SHA values. For example, convert
actions/checkout@v4 to actions/checkout@<full-commit-sha> format for each action
reference throughout the workflow file.
In `@api/openapi/openapi.yaml`:
- Around line 85-86: The `approvedServices` array field in the OpenAPI schema is
currently unbounded and lacks a `maxItems` constraint. Add a `maxItems` property
to the `approvedServices` field definition to limit the array size and prevent
oversized payloads. Use the same maximum value that is applied to other similar
array fields in the schema like `requestedServices` to maintain consistency
across the API specification.
In `@Makefile`:
- Line 517: The fallback logic for VERSION derivation is ineffective because sed
exits successfully even when git describe fails and produces no output. The
issue is that the pipeline operator runs sed on empty input, which succeeds
instead of failing to trigger the fallback. Wrap the entire git describe and sed
pipeline in parentheses before the || operator so that the fallback echo "0.1.0"
is triggered when git describe actually fails. This ensures that if git describe
produces no output, the grouped command fails and the fallback value is used
instead of leaving VERSION empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2017c121-96d2-428c-9950-a2a0f8f4172f
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumpkg/aip/go.sumis excluded by!**/*.sum
📒 Files selected for processing (58)
.github/workflows/publish-clients.ymlDockerfileDockerfile.dashboardDockerfile.gatewayMakefileapi/openapi/openapi.yamlapi/v1alpha1/mcpserver_types_test.gocmd/aipctl/client.gocmd/aipctl/register.gocmd/aipctl/registrations.gocmd/aipctl/token.gocmd/aipctl/token_flow.gocmd/gateway/accuracy_handlers.gocmd/gateway/admin_handlers_test.gocmd/gateway/agent_graduation_policy_handlers.gocmd/gateway/agent_graduation_policy_handlers_test.gocmd/gateway/agent_registration_handlers.gocmd/gateway/agent_registration_lifecycle_handlers.gocmd/gateway/agent_request_lifecycle_handlers.gocmd/gateway/agent_request_review_handlers.gocmd/gateway/agent_request_token_handlers.gocmd/gateway/audit_handlers.gocmd/gateway/cel_validation_test.gocmd/gateway/gateway_e2e_test.gocmd/gateway/gateway_oidc_test.gocmd/gateway/gateway_suite_test.gocmd/gateway/governed_resource_handlers.gocmd/gateway/handlers_test.gocmd/gateway/integration_agent_graduation_policy_test.gocmd/gateway/integration_agent_registration_test.gocmd/gateway/integration_auth_test.gocmd/gateway/integration_basic_test.gocmd/gateway/integration_registration_approval_test.gocmd/gateway/integration_registration_test.gocmd/gateway/integration_result_test.gocmd/gateway/integration_self_registration_test.gocmd/gateway/integration_session_token_test.gocmd/gateway/integration_sse_test.gocmd/gateway/integration_trust_test.gocmd/gateway/main.gocmd/gateway/optimization_test.gocmd/gateway/registration_sse.gocmd/gateway/safety_policy_handlers.gocmd/gateway/session_handlers.gocmd/gateway/sse.gocmd/gateway/sse_test.gocmd/gateway/trust_profile_handlers.gocmd/gateway/trust_profile_handlers_test.gogo.modinternal/controller/agentrequest_controller.gointernal/controller/agentrequest_controller_test.gointernal/controller/agenttrustprofile_controller_test.gointernal/evaluation/evaluator_test.gointernal/evaluation/fetchers/k8s_deployment_fetcher_test.gopkg/aip/api.gen.gopkg/aip/client.gopkg/aip/go.modpkg/aip/transport.go
✅ Files skipped from review due to trivial changes (15)
- cmd/gateway/integration_registration_approval_test.go
- pkg/aip/go.mod
- cmd/gateway/sse.go
- cmd/gateway/integration_session_token_test.go
- internal/evaluation/evaluator_test.go
- cmd/gateway/cel_validation_test.go
- cmd/gateway/integration_registration_test.go
- internal/controller/agentrequest_controller_test.go
- internal/controller/agenttrustprofile_controller_test.go
- internal/evaluation/fetchers/k8s_deployment_fetcher_test.go
- cmd/gateway/integration_basic_test.go
- cmd/gateway/integration_agent_registration_test.go
- api/v1alpha1/mcpserver_types_test.go
- internal/controller/agentrequest_controller.go
- cmd/gateway/agent_graduation_policy_handlers.go
🚧 Files skipped from review as they are similar to previous changes (23)
- cmd/gateway/integration_result_test.go
- cmd/gateway/handlers_test.go
- cmd/gateway/agent_request_token_handlers.go
- cmd/aipctl/register.go
- cmd/gateway/main.go
- cmd/gateway/agent_request_lifecycle_handlers.go
- cmd/gateway/accuracy_handlers.go
- cmd/aipctl/token.go
- cmd/gateway/agent_request_review_handlers.go
- cmd/gateway/integration_self_registration_test.go
- cmd/gateway/trust_profile_handlers.go
- cmd/gateway/admin_handlers_test.go
- cmd/gateway/audit_handlers.go
- cmd/gateway/trust_profile_handlers_test.go
- cmd/gateway/governed_resource_handlers.go
- go.mod
- cmd/gateway/agent_registration_handlers.go
- cmd/gateway/agent_graduation_policy_handlers_test.go
- cmd/gateway/session_handlers.go
- cmd/aipctl/registrations.go
- pkg/aip/transport.go
- pkg/aip/client.go
- cmd/gateway/registration_sse.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-clients.yml:
- Around line 50-53: The
actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 step in the publish
job is missing an explicit cache configuration, which creates a security risk in
a secret-bearing workflow. Add package-manager-cache: false to the with section
of the setup-node action to explicitly disable the implicit npm cache and
prevent potential cache-poisoning attacks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0a16e8b-e461-4e4b-88b1-0b9865877325
📒 Files selected for processing (4)
.github/workflows/publish-clients.ymlMakefileapi/openapi/openapi.yamltest/e2e/Dockerfile.fake-mcp
✅ Files skipped from review due to trivial changes (1)
- test/e2e/Dockerfile.fake-mcp
🚧 Files skipped from review as they are similar to previous changes (2)
- Makefile
- api/openapi/openapi.yaml
…sport ergonomics Extract pkg/aip as standalone module github.com/agent-control-plane/aip-go with its own go.mod and replace directive. Inline ObjectMeta/Spec/Status K8s schemas into openapi.yaml so generated types are self-contained (no api/v1alpha1 aliases). aip-go additions: - GatewayClient: named wrapper over ClientWithResponses with SelfRegister(), RequestToken(), and defaultTokenTTL constant - NewTransport(TokenSource): http.RoundTripper for in-cluster operators that cannot use exec credential plugins - PendingApprovalError: typed error on HTTP 202 so reconcile loops requeue cleanly - TokenSource interface: for rotating credentials (projected SA token, keychain) aipctl updates: - SelfRegister uses namespace parameter threaded to nsPtr() - register.go uses server-returned name when present in 201 response - registrations.go aligns with updated client signatures
…o Docker context go.mod requires go >= 1.26.1; all four builder images were still on golang:1.25. Also add pkg/aip/go.mod and pkg/aip/go.sum COPY lines before RUN go mod download in the three main Dockerfiles — the root go.mod replace directive requires the pkg/aip module files to be present for go mod download to succeed.
ravisantoshgudimetla
force-pushed
the
phase12-aip-go
branch
from
80ad834 to
91aa12e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/publish-clients.yml (1)
34-34: 🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy liftPrefer OIDC trusted publishing over long-lived registry tokens
Line 34 (
twine upload) and Line 61 (npm publish) currently depend on stored secrets. Consider moving to trusted publishing (OIDC) for PyPI and npm to remove long-lived credentials from repo settings.Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-clients.yml at line 34, The twine upload command on line 34 and npm publish command on line 61 are currently using long-lived registry tokens stored as secrets, which presents a security risk. Replace both the PyPI and npm publishing steps to use OIDC trusted publishing instead. For the twine upload step, configure it to use OIDC authentication with PyPI by adding the appropriate OIDC token exchange step and removing the reliance on stored token secrets. For the npm publish step, similarly configure it to use OIDC trusted publishing with npm registry, removing the npm registry token secret dependency. This will eliminate the need for long-lived credentials stored in repository settings.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-clients.yml:
- Around line 18-25: The "Validate generated client" step runs Python commands
(py_compile) before the
`actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065` step is
executed, which relies on pre-installed Python versions from the runner image
and can break when hosted images change. Move the `actions/setup-python` step
with python-version 3.12 to execute BEFORE the "Validate generated client" step
to ensure the correct Python runtime is configured before any Python commands
run. Apply the same reordering pattern to ensure `actions/setup-node` executes
before any Node commands (as noted in the "Also applies to" section).
---
Nitpick comments:
In @.github/workflows/publish-clients.yml:
- Line 34: The twine upload command on line 34 and npm publish command on line
61 are currently using long-lived registry tokens stored as secrets, which
presents a security risk. Replace both the PyPI and npm publishing steps to use
OIDC trusted publishing instead. For the twine upload step, configure it to use
OIDC authentication with PyPI by adding the appropriate OIDC token exchange step
and removing the reliance on stored token secrets. For the npm publish step,
similarly configure it to use OIDC trusted publishing with npm registry,
removing the npm registry token secret dependency. This will eliminate the need
for long-lived credentials stored in repository settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49f20792-1f9a-47ac-954b-c60453861580
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumpkg/aip/go.sumis excluded by!**/*.sum
📒 Files selected for processing (59)
.github/workflows/publish-clients.ymlDockerfileDockerfile.dashboardDockerfile.gatewayMakefileapi/openapi/openapi.yamlapi/v1alpha1/mcpserver_types_test.gocmd/aipctl/client.gocmd/aipctl/register.gocmd/aipctl/registrations.gocmd/aipctl/token.gocmd/aipctl/token_flow.gocmd/gateway/accuracy_handlers.gocmd/gateway/admin_handlers_test.gocmd/gateway/agent_graduation_policy_handlers.gocmd/gateway/agent_graduation_policy_handlers_test.gocmd/gateway/agent_registration_handlers.gocmd/gateway/agent_registration_lifecycle_handlers.gocmd/gateway/agent_request_lifecycle_handlers.gocmd/gateway/agent_request_review_handlers.gocmd/gateway/agent_request_token_handlers.gocmd/gateway/audit_handlers.gocmd/gateway/cel_validation_test.gocmd/gateway/gateway_e2e_test.gocmd/gateway/gateway_oidc_test.gocmd/gateway/gateway_suite_test.gocmd/gateway/governed_resource_handlers.gocmd/gateway/handlers_test.gocmd/gateway/integration_agent_graduation_policy_test.gocmd/gateway/integration_agent_registration_test.gocmd/gateway/integration_auth_test.gocmd/gateway/integration_basic_test.gocmd/gateway/integration_registration_approval_test.gocmd/gateway/integration_registration_test.gocmd/gateway/integration_result_test.gocmd/gateway/integration_self_registration_test.gocmd/gateway/integration_session_token_test.gocmd/gateway/integration_sse_test.gocmd/gateway/integration_trust_test.gocmd/gateway/main.gocmd/gateway/optimization_test.gocmd/gateway/registration_sse.gocmd/gateway/safety_policy_handlers.gocmd/gateway/session_handlers.gocmd/gateway/sse.gocmd/gateway/sse_test.gocmd/gateway/trust_profile_handlers.gocmd/gateway/trust_profile_handlers_test.gogo.modinternal/controller/agentrequest_controller.gointernal/controller/agentrequest_controller_test.gointernal/controller/agenttrustprofile_controller_test.gointernal/evaluation/evaluator_test.gointernal/evaluation/fetchers/k8s_deployment_fetcher_test.gopkg/aip/api.gen.gopkg/aip/client.gopkg/aip/go.modpkg/aip/transport.gotest/e2e/Dockerfile.fake-mcp
✅ Files skipped from review due to trivial changes (17)
- test/e2e/Dockerfile.fake-mcp
- internal/controller/agenttrustprofile_controller_test.go
- pkg/aip/go.mod
- internal/evaluation/fetchers/k8s_deployment_fetcher_test.go
- cmd/gateway/integration_registration_test.go
- cmd/gateway/agent_request_lifecycle_handlers.go
- internal/controller/agentrequest_controller_test.go
- cmd/gateway/integration_agent_registration_test.go
- cmd/gateway/agent_request_token_handlers.go
- cmd/gateway/integration_result_test.go
- api/v1alpha1/mcpserver_types_test.go
- cmd/gateway/safety_policy_handlers.go
- cmd/gateway/gateway_e2e_test.go
- cmd/gateway/integration_registration_approval_test.go
- cmd/gateway/agent_graduation_policy_handlers.go
- internal/controller/agentrequest_controller.go
- internal/evaluation/evaluator_test.go
🚧 Files skipped from review as they are similar to previous changes (39)
- cmd/gateway/integration_sse_test.go
- Dockerfile
- cmd/gateway/session_handlers.go
- cmd/gateway/integration_session_token_test.go
- cmd/aipctl/token.go
- cmd/gateway/audit_handlers.go
- cmd/gateway/cel_validation_test.go
- cmd/gateway/sse_test.go
- Dockerfile.dashboard
- cmd/gateway/handlers_test.go
- cmd/gateway/agent_registration_handlers.go
- cmd/gateway/integration_agent_graduation_policy_test.go
- cmd/gateway/agent_request_review_handlers.go
- cmd/gateway/integration_auth_test.go
- cmd/gateway/agent_registration_lifecycle_handlers.go
- cmd/aipctl/client.go
- cmd/gateway/governed_resource_handlers.go
- Dockerfile.gateway
- cmd/gateway/trust_profile_handlers.go
- cmd/gateway/admin_handlers_test.go
- cmd/gateway/registration_sse.go
- cmd/gateway/gateway_suite_test.go
- cmd/aipctl/registrations.go
- cmd/gateway/integration_trust_test.go
- cmd/aipctl/register.go
- cmd/gateway/trust_profile_handlers_test.go
- cmd/gateway/optimization_test.go
- go.mod
- cmd/gateway/integration_self_registration_test.go
- cmd/gateway/integration_basic_test.go
- api/openapi/openapi.yaml
- cmd/aipctl/token_flow.go
- cmd/gateway/accuracy_handlers.go
- cmd/gateway/agent_graduation_policy_handlers_test.go
- cmd/gateway/main.go
- pkg/aip/client.go
- pkg/aip/transport.go
- cmd/gateway/gateway_oidc_test.go
- pkg/aip/api.gen.go
…rray bounds Lint (Go 1.26 modernize linter): - Add //nolint:modernize to ptr.To() calls that carry runtime values where new(x) would be wrong per staticcheck SA4006 - Remove now-unused strPtr/boolPtr helper functions made unused by lint-fix - Fix comma-before-comment placement in composite literals (gofmt) CI/workflow: - Pin all GitHub Actions uses: to immutable commit SHAs (supply chain hardening) - Add package-manager-cache: false to setup-node (prevent cache poisoning in secret-bearing publish workflow) - Add permissions: contents: read and persist-credentials: false - Fix py_compile glob path (was double-prefixing working-directory) - Fix VERSION Makefile fallback: sh -c with empty-tag check so 0.1.0 fires correctly when git describe produces no output OpenAPI spec: - Add maxItems: 100 to approvedServices, requestedServices, allowedSubjects
ravisantoshgudimetla
force-pushed
the
phase12-aip-go
branch
from
91aa12e to
1653f89
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Bug Fixes
Chores