feat(credentials): migrate to global resource + spec reconciliation#1570
Merged
Conversation
Spec reconciliation run against components/ambient-api-server. Fixes stale
spec entries, removes a phantom OpenAPI field, and drops an unused DB column.
Spec changes (specs/api/ambient-model.spec.md):
- Status: Active (was "Proposed — Pending Consensus")
- Agent ERD: added 12 undocumented fields (parent_agent_id, owner_user_id,
display_name, description, repo_url, workflow_id, llm_model,
llm_temperature, llm_max_tokens, bot_account_name, resource_overrides,
environment_variables); documented ignite_handler propagation to Session
- ScheduledSession ERD: added timeout, inactivity_timeout,
stop_on_run_finished, runner_type
- Session field types: int → int32 for llm_max_tokens, timeout,
sdk_restart_count
- Ignite Response example: triggered_by_user_id → created_by_user_id
- Credentials: document scoping gap (impl=project-scoped, spec=global);
added vertex to provider enum table; CLI table updated to ✅ project-scoped
- RBAC endpoints: added GET/{id} and PATCH/{id} for role_bindings as ✅;
added 4 scoped role_binding query endpoints as 🔲
- Coverage matrix: credentials, RBAC full CRUD, project update all ✅
- CLI reference: project update ✅; RBAC list/get/delete ✅
- CLI Known Gaps: removed now-implemented items; added credential bind gap
- Agent section prose: full field table with all 15 fields
OpenAPI changes (components/ambient-api-server/openapi/):
- openapi.credentials.yaml: added vertex to provider enum
- openapi.projects.yaml: removed display_name property
- openapi.sessions.yaml: removed triggered_by_user_id (phantom field —
never existed in model.go)
Code changes (components/ambient-api-server/plugins/projects/):
- model.go: removed DisplayName from Project and ProjectPatchRequest
- grpc_handler.go: removed DisplayName from create/update handlers
- grpc_presenter.go: removed DisplayName from projectToProto()
- migration.go: added dropDisplayNameMigration() (ID 202505090001)
- plugin.go: registered new drop-column migration
- factory_test.go: removed DisplayName from test factory
- grpc_integration_test.go: removed display_name test assertions
- plugins/projectSettings/factory_test.go: removed stale DisplayName
Workflow update (workflows/sessions/ambient-model.workflow.md):
- Added 2026-05-09 run log with 5 lessons learned
Note: proto/ambient/v1/projects.proto still declares display_name (buf not
available); proto3 semantics make this safe — field transmits as zero value.
Follow-up: remove when buf is available.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Strip accidental 100644→100755 mode changes on all modified files - Add TODO(proto-cleanup) comments to projects.proto on the three display_name fields (Project, CreateProjectRequest, UpdateProjectRequest) so the next engineer with buf available finds them immediately Note on credential:token-reader guard: neither POST nor PATCH /role_bindings currently guards against assigning this role via the user-facing API — both handlers accept any RoleId without validation. This is a security gap in both operations, not just PATCH. Tracking as a follow-up item; out of scope for this reconciliation PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The NetworkPolicy applied podSelector:{} (all pods) with a single
ingress rule allowing only runner pods. kindnet (kind v1.35 / kind
v0.27) enforces NetworkPolicy, so this blocked all same-namespace
pod-to-pod traffic — including frontend → backend-api requests that
Cypress exercises in E2E tests.
Add an explicit same-namespace allow rule so intra-namespace traffic
continues while still permitting runner pods from any namespace.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The NetworkPolicy with policyTypes:Ingress creates an implicit deny-all
for every pod it selects. NodePort traffic (kube-proxy → frontend pod)
does not originate from a pod, so it was blocked even after the
same-namespace podSelector rule was added.
Replace the same-namespace rule with a wildcard ingress rule (- {}) that
permits all traffic sources, including NodePort and external clients.
The runner-pod rule is preserved for forward compatibility with any
future default-deny policy.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Credentials are now a global resource at /api/ambient/v1/credentials
instead of /api/ambient/v1/projects/{id}/credentials, matching the
spec target state.
Changes:
- openapi.credentials.yaml: global paths, project_id removed from schema
- plugins/credentials/model.go: drop ProjectID field
- plugins/credentials/migration.go: add drop-column migration (202505120001)
that removes project_id; prior addProjectIDMigration kept for existing DBs
- plugins/credentials/handler.go: remove project path-var guards and
project-filter injection from List
- plugins/credentials/presenter.go: remove ProjectId from responses;
drop projectID param from ConvertCredential
- plugins/credentials/plugin.go: subrouter moved from /projects to
/credentials; register new drop-column migration
- plugins/credentials/factory_test.go: remove ProjectID from factory
- SDK regenerated; credential_extensions.go updated to use literal path
(generator does not emit basePath() for top-level resources)
- specs/api/ambient-model.spec.md: remove implementation-gap notes;
mark all credential routes as implemented
- workflows/sessions/ambient-model.workflow.md: document migration and
basePath() lesson for extension files
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…egeneration Spec SHA256 changed when vertex was added to the credentials provider enum; regeneration updated the spec hash and timestamp in all generated files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
WalkthroughCredentials are migrated from project-scoped API routes ( ChangesCredential Scoping Migration
Sequence DiagramsequenceDiagram
participant Client as Client App
participant API as API Server
participant DB as Database
Note over Client,DB: Before: Project-scoped Credentials
Client->>API: GET /projects/{id}/credentials
API->>API: Validate project exists
API->>DB: SELECT * FROM credentials WHERE project_id = ?
DB-->>API: credentials
API-->>Client: Credentials list
Note over Client,DB: After: Global Credentials
Client->>API: GET /credentials
API->>API: No project validation
API->>DB: SELECT * FROM credentials
DB-->>API: credentials
API-->>Client: Credentials list
Note over Client,DB: Token Retrieval
Client->>API: GET /credentials/{cred_id}/token
API->>DB: SELECT token FROM credentials WHERE id = ?
DB-->>API: decrypted token
API-->>Client: CredentialTokenResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Merge Queue Status
This pull request spent 56 seconds in the queue, including 9 seconds running CI. Required conditions to merge |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/ambient-sdk/python-sdk/ambient_platform/agent.py (1)
80-80:⚠️ Potential issue | 🟠 MajorFix the Python SDK generator to use
field(default_factory=list)for list-typed fields in*Listdataclasses.The generator template
components/ambient-sdk/generator/templates/python/types.py.tmplhasitems: list[{{.Resource.Name}}] = ()which creates a type mismatch: the field is annotated aslist[Type]but defaults to()(empty tuple). This violates mypy strict mode requirements specified in the coding guidelines.This affects all 11 generated
*Listclasses (AgentList, UserList, SessionList, etc.). Thefrom_dictmethod correctly assigns a list, but the field annotation and default are incompatible.Update line 66 in the template:
items: list[{{.Resource.Name}}] = field(default_factory=list)And add the import:
from dataclasses import dataclass, fieldNote: mypy is configured in
pyproject.tomlbut not enforced in CI workflows, so this issue isn't caught during generation.🤖 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 `@components/ambient-sdk/python-sdk/ambient_platform/agent.py` at line 80, The list-typed fields in the generated "*List" dataclasses (e.g., AgentList, UserList, SessionList) use a tuple default which mismatches the list annotation; update the generator template types.py.tmpl to set items: list[{{.Resource.Name}}] = field(default_factory=list) and add the dataclasses import for field (i.e., ensure "from dataclasses import dataclass, field" is present) so the generated classes have a correct list default; modify the template line that currently has items: list[{{.Resource.Name}}] = () accordingly to fix all generated *List classes.
🤖 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 `@components/ambient-sdk/python-sdk/ambient_platform/_credential_api.py`:
- Line 26: The credential API currently interpolates raw resource IDs into
request paths (see uses in methods around resp = self._client._request("GET",
f"/credentials/{resource_id}") and similar calls at the other sites), which
breaks for IDs containing reserved characters; import quote from urllib.parse
and replace raw resource_id with quote(resource_id, safe="") in every path
construction in this module (e.g., the GET, PUT/POST, DELETE methods that build
"/credentials/{resource_id}" and any other "/credentials/{id}" usages) so the ID
is URL-path-escaped before calling self._client._request.
---
Outside diff comments:
In `@components/ambient-sdk/python-sdk/ambient_platform/agent.py`:
- Line 80: The list-typed fields in the generated "*List" dataclasses (e.g.,
AgentList, UserList, SessionList) use a tuple default which mismatches the list
annotation; update the generator template types.py.tmpl to set items:
list[{{.Resource.Name}}] = field(default_factory=list) and add the dataclasses
import for field (i.e., ensure "from dataclasses import dataclass, field" is
present) so the generated classes have a correct list default; modify the
template line that currently has items: list[{{.Resource.Name}}] = ()
accordingly to fix all generated *List classes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 05d9aceb-df1f-432a-852d-e3029cca561b
📒 Files selected for processing (87)
components/ambient-api-server/openapi/openapi.credentials.yamlcomponents/ambient-api-server/plugins/credentials/factory_test.gocomponents/ambient-api-server/plugins/credentials/handler.gocomponents/ambient-api-server/plugins/credentials/migration.gocomponents/ambient-api-server/plugins/credentials/model.gocomponents/ambient-api-server/plugins/credentials/plugin.gocomponents/ambient-api-server/plugins/credentials/presenter.gocomponents/ambient-sdk/go-sdk/client/agent_api.gocomponents/ambient-sdk/go-sdk/client/client.gocomponents/ambient-sdk/go-sdk/client/credential_api.gocomponents/ambient-sdk/go-sdk/client/credential_extensions.gocomponents/ambient-sdk/go-sdk/client/inbox_message_api.gocomponents/ambient-sdk/go-sdk/client/iterator.gocomponents/ambient-sdk/go-sdk/client/project_api.gocomponents/ambient-sdk/go-sdk/client/project_settings_api.gocomponents/ambient-sdk/go-sdk/client/role_api.gocomponents/ambient-sdk/go-sdk/client/role_binding_api.gocomponents/ambient-sdk/go-sdk/client/scheduled_session_api.gocomponents/ambient-sdk/go-sdk/client/session_api.gocomponents/ambient-sdk/go-sdk/client/session_message_api.gocomponents/ambient-sdk/go-sdk/client/user_api.gocomponents/ambient-sdk/go-sdk/types/agent.gocomponents/ambient-sdk/go-sdk/types/base.gocomponents/ambient-sdk/go-sdk/types/credential.gocomponents/ambient-sdk/go-sdk/types/inbox_message.gocomponents/ambient-sdk/go-sdk/types/list_options.gocomponents/ambient-sdk/go-sdk/types/project.gocomponents/ambient-sdk/go-sdk/types/project_settings.gocomponents/ambient-sdk/go-sdk/types/role.gocomponents/ambient-sdk/go-sdk/types/role_binding.gocomponents/ambient-sdk/go-sdk/types/scheduled_session.gocomponents/ambient-sdk/go-sdk/types/session.gocomponents/ambient-sdk/go-sdk/types/session_message.gocomponents/ambient-sdk/go-sdk/types/user.gocomponents/ambient-sdk/python-sdk/ambient_platform/__init__.pycomponents/ambient-sdk/python-sdk/ambient_platform/_agent_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_base.pycomponents/ambient-sdk/python-sdk/ambient_platform/_credential_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_inbox_message_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_iterator.pycomponents/ambient-sdk/python-sdk/ambient_platform/_project_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_project_settings_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_role_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_role_binding_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_scheduled_session_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_session_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_session_message_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/_user_api.pycomponents/ambient-sdk/python-sdk/ambient_platform/agent.pycomponents/ambient-sdk/python-sdk/ambient_platform/client.pycomponents/ambient-sdk/python-sdk/ambient_platform/credential.pycomponents/ambient-sdk/python-sdk/ambient_platform/inbox_message.pycomponents/ambient-sdk/python-sdk/ambient_platform/project.pycomponents/ambient-sdk/python-sdk/ambient_platform/project_settings.pycomponents/ambient-sdk/python-sdk/ambient_platform/role.pycomponents/ambient-sdk/python-sdk/ambient_platform/role_binding.pycomponents/ambient-sdk/python-sdk/ambient_platform/scheduled_session.pycomponents/ambient-sdk/python-sdk/ambient_platform/session.pycomponents/ambient-sdk/python-sdk/ambient_platform/session_message.pycomponents/ambient-sdk/python-sdk/ambient_platform/user.pycomponents/ambient-sdk/ts-sdk/src/agent.tscomponents/ambient-sdk/ts-sdk/src/agent_api.tscomponents/ambient-sdk/ts-sdk/src/base.tscomponents/ambient-sdk/ts-sdk/src/client.tscomponents/ambient-sdk/ts-sdk/src/credential.tscomponents/ambient-sdk/ts-sdk/src/credential_api.tscomponents/ambient-sdk/ts-sdk/src/inbox_message.tscomponents/ambient-sdk/ts-sdk/src/inbox_message_api.tscomponents/ambient-sdk/ts-sdk/src/index.tscomponents/ambient-sdk/ts-sdk/src/project.tscomponents/ambient-sdk/ts-sdk/src/project_api.tscomponents/ambient-sdk/ts-sdk/src/project_settings.tscomponents/ambient-sdk/ts-sdk/src/project_settings_api.tscomponents/ambient-sdk/ts-sdk/src/role.tscomponents/ambient-sdk/ts-sdk/src/role_api.tscomponents/ambient-sdk/ts-sdk/src/role_binding.tscomponents/ambient-sdk/ts-sdk/src/role_binding_api.tscomponents/ambient-sdk/ts-sdk/src/scheduled_session.tscomponents/ambient-sdk/ts-sdk/src/scheduled_session_api.tscomponents/ambient-sdk/ts-sdk/src/session.tscomponents/ambient-sdk/ts-sdk/src/session_api.tscomponents/ambient-sdk/ts-sdk/src/session_message.tscomponents/ambient-sdk/ts-sdk/src/session_message_api.tscomponents/ambient-sdk/ts-sdk/src/user.tscomponents/ambient-sdk/ts-sdk/src/user_api.tsspecs/api/ambient-model.spec.mdworkflows/sessions/ambient-model.workflow.md
💤 Files with no reviewable changes (1)
- components/ambient-api-server/plugins/credentials/model.go
|
|
||
| def get(self, resource_id: str) -> Credential: | ||
| resp = self._client._request("GET", f"{self._base_path()}/{resource_id}") | ||
| resp = self._client._request("GET", f"/credentials/{resource_id}") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether path escaping is already performed centrally, and confirm current call sites.
fd -i '(_credential_api|client|_base)\.py$' components/ambient-sdk/python-sdk/ambient_platform
rg -n -C3 --iglob '*_credential_api.py' '"/credentials/\{resource_id\}"|quote\(' components/ambient-sdk/python-sdk/ambient_platform
rg -n -C6 --iglob '*client.py' 'def _request\(' components/ambient-sdk/python-sdk/ambient_platformRepository: ambient-code/platform
Length of output: 3792
🏁 Script executed:
# Get full _request method implementation
sed -n '119,200p' components/ambient-sdk/python-sdk/ambient_platform/client.pyRepository: ambient-code/platform
Length of output: 2621
🏁 Script executed:
# Check if quote is used anywhere in the codebase currently
rg 'from urllib.parse import|quote\(' components/ambient-sdk/python-sdk/Repository: ambient-code/platform
Length of output: 1271
🏁 Script executed:
# Check Go SDK credential API for comparison
fd -i 'credential.*\.go$' components/ambient-sdk/go-sdk | head -5Repository: ambient-code/platform
Length of output: 293
🏁 Script executed:
# Check Go SDK credential API for encoding pattern
sed -n '1,50p' components/ambient-sdk/go-sdk/client/credential_api.go | head -40Repository: ambient-code/platform
Length of output: 1253
🏁 Script executed:
# Check if credential_api.py has quote import
head -15 components/ambient-sdk/python-sdk/ambient_platform/_credential_api.pyRepository: ambient-code/platform
Length of output: 546
🏁 Script executed:
# Check all uses of resource_id in credential_api.py
grep -n 'resource_id' components/ambient-sdk/python-sdk/ambient_platform/_credential_api.pyRepository: ambient-code/platform
Length of output: 501
URL-encode resource_id before inserting it into credential API paths.
Lines 26, 35, and 39 interpolate raw IDs directly into the request path. The Go SDK equivalent (credential_api.go) uses url.PathEscape(id) for this exact reason. Other Python API files in the same codebase (_agent_api.py, _scheduled_session_api.py, etc.) already follow this pattern by importing and using quote(). IDs containing reserved characters like /, ?, #, or spaces will break request routing or target unintended endpoints.
Proposed fix
from __future__ import annotations
from typing import Any, Iterator, Optional, TYPE_CHECKING
+from urllib.parse import quote
@@
- resp = self._client._request("GET", f"/credentials/{resource_id}")
+ resp = self._client._request("GET", f"/credentials/{quote(resource_id, safe='')}")
@@
- resp = self._client._request("PATCH", f"/credentials/{resource_id}", json=data)
+ resp = self._client._request("PATCH", f"/credentials/{quote(resource_id, safe='')}", json=data)
@@
- self._client._request("DELETE", f"/credentials/{resource_id}", expect_json=False)
+ self._client._request("DELETE", f"/credentials/{quote(resource_id, safe='')}", expect_json=False)🤖 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 `@components/ambient-sdk/python-sdk/ambient_platform/_credential_api.py` at
line 26, The credential API currently interpolates raw resource IDs into request
paths (see uses in methods around resp = self._client._request("GET",
f"/credentials/{resource_id}") and similar calls at the other sites), which
breaks for IDs containing reserved characters; import quote from urllib.parse
and replace raw resource_id with quote(resource_id, safe="") in every path
construction in this module (e.g., the GET, PUT/POST, DELETE methods that build
"/credentials/{resource_id}" and any other "/credentials/{id}" usages) so the ID
is URL-path-escaped before calling self._client._request.
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
/api/ambient/v1/projects/{id}/credentialsto/api/ambient/v1/credentials;project_idFK removed from model and DB viaDROP COLUMN IF EXISTSmigration (ID202505120001)runner-networkpolicy.yamlnow allows all ingress (- {}wildcard) so NodePort traffic from kube-proxy reaches the frontend pod; fixes Cypress E2E timeout onPOST /api/projectsspecs/api/ambient-model.spec.mdupdated to match current implementation — Agent ERD fields, ScheduledSession fields, RBAC routes, coverage matrix, int32 types, status → Activecredential_api.go/credential.goupdated to top-level paths; extension file fixed to use literal/credentials/path (generator does not emitbasePath()for top-level resources)vertexadded to credentials provider enum;triggered_by_user_idremoved from Session schema;display_nameremoved from Project schemaTest plan
cd components/ambient-sdk/go-sdk && go test ./...passescd components/ambient-cli && go test ./...passesPOST /api/projectsno longer times out (NodePort ingress fix)GET /api/ambient/v1/credentialsreturns 200 (global route active)GET /api/ambient/v1/projects/{id}/credentialsreturns 404 (old route gone)202505120001dropsproject_idcolumn on upgrade🤖 Generated with Claude Code