feat(sdk): deliver the approval manifest to the live approval card - #5761
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| manifest = payload.get("manifest") | ||
| if manifest is not None: | ||
| yield { | ||
| "type": "data-approval-manifest", |
There was a problem hiding this comment.
The manifest travels in its own data part.
It cannot travel in tool-approval-request. That chunk is a strict object with an exact key set, and the conformance test pins that set.
It must also stay out of input. input holds the arguments of the model, and the runner builds the manifest. A manifest inside input shows the human a payload that the model never wrote.
| # operations are enabled, so membership is tested against the catalog itself rather than | ||
| # re-reading the flag: an op name the catalog does not define raises `UnknownPlatformOpError` | ||
| # for every build-kit resolution. | ||
| _READ_CONFIG_OPS: tuple[str, ...] = ( |
There was a problem hiding this comment.
This line tests the catalog for the op. It does not read the flag.
The catalog defines read_config only when ordered operations are on. An op name that the catalog does not define raises UnknownPlatformOpError for every build-kit resolution.
The catalog test therefore prevents a broken build kit in a deployment that has the flag off.
| # every write to `parameters.agent`, and it is the ONLY commit surface the | ||
| # agent is given: the path comes from this catalog, and the sandbox holds no | ||
| # credential, so an unscoped agent commit cannot be expressed. | ||
| path="/api/workflows/revisions/commit/agent", |
There was a problem hiding this comment.
This path decides the write scope of the agent. The route confines every write to parameters.agent.
The agent cannot select a different path. This catalog is on the server, and the sandbox holds no credential.
If you change this path back to /api/workflows/revisions/commit, the agent can write harness.permissions and sandbox.kind.
| "entries plus your change), or you wipe the rest, including your own build-kit tools. " | ||
| "are replaced wholesale, not merged entry-by-entry: send the complete list, meaning " | ||
| "your current entries plus your change, or you wipe the rest. Leave the playground's " | ||
| "own tools (commit_revision, test_run, read_config) OUT of that list: they are not " |
There was a problem hiding this comment.
The server refuses a commit that contains a tool entry of the platform kind.
The earlier text told the model to send the complete tools list together with its own build-kit tools. A model that obeyed that text received a refusal on its first commit.
This text now names the three tools to omit, and it states the refusal.
f5dc465 to
0899359
Compare
dd50fb6 to
a2883a1
Compare
0899359 to
730dee1
Compare
a2883a1 to
4cc3fb3
Compare
|
@coderabbitai review |
|
4cc3fb3 to
7f77f92
Compare
477c1ac to
a9af42e
Compare
7f77f92 to
e80845e
Compare
a9af42e to
c4d1a0f
Compare
e80845e to
8d275fd
Compare
|
@coderabbitai review |
|
53924d9 to
4681e4d
Compare
6920fe4 to
bf1c586
Compare
4681e4d to
0f051ce
Compare
82b5862 to
23ce91f
Compare
28316b4 to
395f4ea
Compare
23ce91f to
4ef3731
Compare
395f4ea to
2be2849
Compare
4ef3731 to
b6f7f66
Compare
2be2849 to
6a9d9d7
Compare
b6f7f66 to
609644c
Compare
6a9d9d7 to
aa02582
Compare
aa02582 to
4834ef8
Compare
b52fda2 to
4e469c9
Compare
4834ef8 to
a229172
Compare
…uild kit The egress emits a sibling data-approval-manifest part before the strict approval chunk (which keeps exactly type/approvalId/toolCallId), so the live card and the replayed card share one payload shape. The catalog description stops naming a nonexistent revision_id field. read_config joins DEFAULT_BUILD_KIT_OPS gated on catalog membership, not on a re-read of the flag, so the kit cannot drift from the catalog.
…ion excludes build-kit tools (final review E2, F10)
…coped route and refusals
…ance in descriptions and skill (E2E U3-U4 root, U7)
… op (live QA stumble)
…e_revision_id required when ordered (items B and tolerance)
…talog half it pins; below this lane it fails in isolation because the SDK alignment does not exist yet (CI red on every intermediate PR)
… path it expects (instance seven of tests-below-their-code: it landed on s2 while /commit/agent enters the catalog only here, reddening four intermediate PRs)
…ped-route assertion now exists only where the route does
Railway Preview Environment
|
1 participant
Context
When an agent commits content it imported from its workspace, the approval card must show what it is approving: which files were read, and a diff for a field replaced from one. The runner builds that manifest at the permission gate. It reached the frontend only on a cold replay, so on the live path the card had nothing to render and a human approved bytes they never saw.
This lane wires the SDK side of that delivery, plus the catalog changes the read-then-edit loop needs.
Targets
agent-config-editing-s3b-wire-runnerand is part of the agent-config-editing stack. Read the stack bottom up.Changes
The manifest gets its own stream part. The Vercel stream adapter emits
data-approval-manifestbeside the approval request:{"type": "data-approval-manifest", "id": "<toolCallId>", "data": {"toolCallId": "...", "approvalId": "...", "manifest": {...}}}It cannot ride
tool-approval-request, which is a strict object with an exact key set pinned by the conformance test. It must not ride the tool'sinputeither, becauseinputis the model's own arguments and the manifest is runner-derived. Putting it there would show the human a payload the model never wrote, which is exactly the confusion the card exists to prevent. The AI SDK passes adata-*part through untouched, so a separate part is the honest place for it.base_revision_idis now required with the flag on. This is the one change here with a compatibility edge, so read it before approving. With ordered operations enabled, the commit schema listsbase_revision_idamong its required properties:The server already refuses an ordered delta that omits it. Saying so only in prose meant a model sent the call anyway and spent a turn on the refusal, so the schema now states it.
workflow_variant_idis bound from run context and stripped from the list along with the property, so the model never sees it as required. Any caller that builds an ordered commit without a base revision was already being refused; it now fails schema validation instead, and earlier.read_configjoins the build kit. A playground agent could commit but never read what it was editing. The op is added to the default build kit when the catalog defines it, which is when ordered operations are on, so the kit carries both halves of the loop or neither.The commit op points at the scoped route.
commit_revisionnow targets/api/workflows/revisions/commit/agent, in both flag states. That route confines every write toparameters.agent. The path comes from this catalog and the sandbox holds no credential, so this line is where the agent's write scope is decided.The per-call
descriptionis accepted in a second position. A model that puts its note inside the payload object rather than at the top level no longer loses it. Both positions carryx-ag-ephemeral: true, which is what lets the runner lift the field and drop it before the commit without hard-coding a tool name. The marker is the contract: once a nesteddescriptionis advertised as tolerated, the field name alone can no longer tell the endpoint's own field from a misplaced note, so the decision has to be machine-readable.The truthy set is a deliberate mirror.
_ordered_operations_enabledparses the flag with its own copy of the API's truthy values, because the SDK cannot import from the API. A copy that drifts would turn the ordered arm on in the API while this catalog kept advertising the legacy surface.api/oss/tests/pytest/unit/workflows/test_ordered_operations_flag.pypins the two sets equal; it lives on the API side because that is the one side that can import both.The flag-off tool description stops contradicting the server. It told the model to send the complete
toolslist including its own build-kit tools, or lose them. The server now refuses a commit that carries a platform-kind tool entry, so following that instruction earned a rejection on the first commit. The text now says to leavecommit_revision,test_run, andread_configout of the list, and states that a commit carrying one is refused.Documentation follows the protocol. Three design documents described the older shapes: the protocol reference, the tools reference, and the in-service tool resolution interface. They now match what this catalog emits, so the next reader is not working from a contract the code stopped honoring.
Tests
test_vercel_stream_approval_manifest.py, 4 tests: the part is emitted when a manifest is present, it is absent when there is none, and the approval request itself is unchanged.test_build_kit_overlay.pypins both build-kit shapes, one per flag state, with each op list written out in full. The flag is read directly rather than through the catalog, so the test states the flag-to-list mapping instead of restating the code it checks.test_ordered_operations_flag.pyis the cross-side guard on the truthy mirror. It sits under the API tests rather than the SDK ones, since only that side can import both definitions.test_op_catalog.pypins the scoped commit path, andtest_op_catalog_description.pycovers both description positions and the required-field split per flag state.What to QA
.agenta-imports/into a skill and commit. The approval card shows the file list and the diff before you approve, without a page reload.