feat(runner,api,sdk): Complete and enable the test_run self-test tool - #5074
Conversation
- protocol.ts: contextBindings + timeoutMs on resolved specs; runContext.run.kind - relay: post-verdict $ctx injection in the callRef branch only; bound paths redacted from the pending-approval record; per-spec timeout honored at host fetch (+10s grace) and child poll deadline; x-agenta-run-kind forwarded - service reads meta.run_kind into the run context; SDK wire + goldens + both contract tests Claude-Session: https://claude.ai/code/session_014iPB7HL5PjgT9npyPHaFMT
…ify step) - DEFAULT_BUILD_KIT_OPS += test_run (13 ops, builtin grants first) - AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS defaults ON; off = skip-with-warning kill switch (widened disabled set) - playbook verify step teaches test_run; query_spans stays for scheduled fires Claude-Session: https://claude.ai/code/session_014iPB7HL5PjgT9npyPHaFMT
…nc + sidecar recipe note Claude-Session: https://claude.ai/code/session_014iPB7HL5PjgT9npyPHaFMT
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds Changestest_run Enablement and Run-Kind/Context-Binding Propagation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Runner
participant ToolRelay
participant PermissionCheck
participant AgentaCallback
Runner->>ToolRelay: startToolRelay(spec, args, runContext)
ToolRelay->>PermissionCheck: request verdict
PermissionCheck-->>ToolRelay: allow / deny / pendingApproval
ToolRelay->>ToolRelay: pendingApprovalArgs (redact bound paths)
ToolRelay->>ToolRelay: applyContextBindings(args, contextBindings, runContext)
ToolRelay->>AgentaCallback: callAgentaTool(args, {timeoutMs, runKind})
AgentaCallback-->>ToolRelay: response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
mmabrouk
left a comment
There was a problem hiding this comment.
Review guide: four inline pointers to the load-bearing seams.
| } | ||
| // Gateway (Composio): POST back through Agenta's /tools/call so the secret stays server-side. | ||
| const args = spec.contextBindings | ||
| ? applyContextBindings(req.args, spec.contextBindings, runContext) |
There was a problem hiding this comment.
Review guide: the injection seam: post-verdict, callRef branch only, bound paths redacted from the approval record.
| runKind?: string; | ||
| } | ||
|
|
||
| function callbackFetchTimeoutMs(timeoutMs: number | undefined): number { |
There was a problem hiding this comment.
Review guide: per-spec budget honored at both deadlines with +10s grace; absent timeout keeps old behavior.
| "commit_revision", | ||
| "annotate_trace", | ||
| "query_spans", | ||
| "test_run", |
There was a problem hiding this comment.
Review guide: the arming flip: 13 ops, grants first; kill switch = skip, not raise.
| present, ordered correctly, and has no error status; a 200 response or empty assistant output is | ||
| not proof. If the terminal action is missing, rewrite the instructions as a blunter numbered | ||
| procedure and commit again. | ||
| 6. Verify with `test_run`. First warn the user that this is a real run: external write |
There was a problem hiding this comment.
Review guide: playbook teaches test_run as primary self-test; query_spans stays for scheduled fires.
|
@coderabbitai review |
|
Feedback needed on four things in particular:
|
✅ Action performedReview finished.
|
mmabrouk
left a comment
There was a problem hiding this comment.
lgtm if tested locally e2e
|
Review round 1 addressed. The only actionable item was the conditional lgtm ("lgtm if tested locally e2e"). CodeRabbit finished its review with no findings, and the four inline comments are our own review guides. I reran the e2e today (2026-07-06) on the local dev stack, at the exact code state of this PR's head (
The kill-switch off path ( One useful side finding from the rerun: calling No code changed this round, so the PR head is unchanged. |
Context
The playground has a "build kit": a chat where a builder agent assembles another agent by picking its tools, writing its instructions, and committing the result. The build kit hands that builder a set of platform ops. A platform op is a declarative tool that wraps one Agenta capability, such as committing a revision or querying traces.
test_runis the newest op: it fires one real run of the app the builder just built and returns a pass/fail verdict, so the builder can verify its own work.Part one (#5068) shipped the server half of
test_run: the op exists and the API can execute it, but it was flag-gated off, and the runner (the process that executes an agent and relays its tool calls back to the platform) could not dispatch the new tool shape. So a builder agent still could not test the app it built. This PR completes the loop and turns the feature on.Design doc:
docs/design/agent-workflows/projects/build-kit-tools-cleanup/plan-5b.md.What changed
Three commits.
1. Runner dispatch plus three safety properties (
5efd9fa0af: runner, SDK, agent service)New wire fields. A resolved tool spec (
protocol.ts) gainscontextBindingsandtimeoutMs, and the run context gainsrun.kind.callRefis the dispatch shape for server-handled ops: instead of a concrete URL, the spec carries an opaque reference that the runner posts back to Agenta's/tools/callendpoint. The SDK emits the new fields on the wire (wire.py,wire_models.py,dtos.py), and the golden request files and contract tests on both sides pin the shape.The target is injected server-side, after the permission check. A self-test must only ever test the calling agent itself, so the model never supplies the target. The spec's
contextBindingsmap argument paths to run-context values (for exampletarget.workflow_variant_idcomes from$ctx.workflow.variant.id). The relay (the runner component that executes a tool call once the permission layer allows it) applies these bindings after the permission verdict, on thecallRefbranch only. It deletes whatever the model put at a bound path, then writes the server value; if a bound value is missing from the run context, the call fails loudly instead of running unbound. The pending-approval record now strips bound paths, so before this change an approval prompt could display a forged target from the model's arguments, and after it the prompt shows only the arguments the model legitimately controls.The 2-minute budget is honored.
test_runcarriestimeoutMs: 120000, because a real child run takes minutes, not seconds. Before, two flat runner limits would have killed it: the 30-second host fetch (TOOL_CALL_TIMEOUT_MS) and the 60-second extension-to-host relay wait (RELAY_TIMEOUT_MS). Both layers now use the per-spec budget plus a 10-second grace window, so digest work produced right after the child's ceiling is not lost to an abort at the same deadline.A self-test cannot recurse. The server marks the child invoke with
meta.run_kind = "test". The agent service reads that into the run context, the runner forwards it on every tool callback as thex-agenta-run-kindheader, and the server rejects atest_runthat arrives with that header with a 400. A test run therefore cannot trigger another test run, directly or through a chain.2. Arm the op (
1220754293: API, SDK)DEFAULT_BUILD_KIT_OPS(the op list inoverlay.py; the overlay is the set of extra tools and skills the playground injects into a draft agent) gainstest_run, growing the list to 13 ops. The kill-switch env varAGENTA_AGENT_ENABLE_PLATFORM_HANDLERSflips to on by default. Before, unset meant disabled and resolving a handler-mode op raised an error. After, unset means enabled, and an explicit off value (the disabled-value set was widened) skips handler-mode ops with a warning instead of raising. The build-an-agent playbook skill replaces its "verify withquery_spans" step with atest_runverify step;query_spansstays in the playbook for reading back scheduled runs after triggers fire.3. Docs (
a90fbd9323)The plan doc, the landed status, and the runner-to-tool-callback interface page are synced to the shipped behavior.
How to review this PR
Read the runner first, then the SDK, then the service and API. Suggested stops:
services/runner/src/protocol.ts: the two new spec fields andrun.kind. Check they are optional, so payloads from an older SDK still parse.services/runner/src/tools/direct.ts(applyContextBindings): check that it deletes the model's value at each bound path before writing the server value, and that it throws when the run context lacks a bound value. The property is "never run a bound call with a model-controlled or missing target".services/runner/src/tools/relay.ts: the injection sits inexecuteAllowedRelayedTool, which only runs after an allow verdict, and only on thecallRefbranch.pendingApprovalArgsredacts bound paths (and prunes emptied parent objects) from the approval record. Note the comment about Claude: its approval card is harness-rendered before this relay, so only the Pi file-relay path gets the redaction.services/runner/src/tools/callback.tsanddispatch.ts: the per-spec timeout at the host fetch and the relay wait, both with the +10s grace, and thex-agenta-run-kindheader on both callback and direct calls.services/runner/src/tools/public-spec.ts:timeoutMsis advertised to the sandbox extension because the child relay wait needs the same budget. Check thatcallRef,contextBindings, andenvstill stay out of the advertised spec.platform_tools.py(default-on flag and skip-with-warning),dtos.py/wire_models.py/wire.py(run.kindserialization),agenta_builtins.py(the playbook's new verify step).services/oss/src/agent/app.pyandtracing.pyreadmeta.run_kindinto the run context each turn.overlay.pyis a one-line op-list addition, plus its test.tool-callref-bindings.test.tscovers the injection and redaction behavior end to end;test_run_kind.pycovers the service side; the wire-contract tests and goldens pin the shapes on both sides.How to QA
Prerequisites: EE dev stack with the agent sidecar (see the subscription-sidecar recipe). No flag needed; platform handlers now default on.
Steps:
test_runand an approval prompt appears. The prompt's arguments contain notarget.*fields (bound paths are redacted).test_runagain. Expected: the server refuses with a 400.AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS=offand re-resolve. Expected:test_runis skipped with a warning, not an error.Live evidence already collected on the dev stack:
test_runwith a hidden binding paused for approval, resumed through the real playground resume mechanism (approval-responded), the child digest returned "pong", and the model reported the pass.test_rungets a 400 when called directly and via the child header chain.trace_id, and full tool arguments.Automated tests:
Scope and risk
Ship the runner and SDK halves together. An old runner receiving the new SDK's specs drops the bindings, so child calls fail loudly instead of silently running unbound. That failure mode is deliberate, but it means you should not deploy the SDK half without the runner half.
Kill-switch semantics changed.
AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS=offnow skips platform handler ops with a warning; it no longer raises. Anyone relying on the old fail-closed error should know.Approval surface. The pending-approval record redacts bound
$ctxpaths, so values bound into child calls (including credentials) never reach the approval UI payload.Deploy note.
dist/is gitignored, so deploys must runbuild:extensionto pick up the runner changes. The subscription-sidecar recipe was updated for this; the recipe README hunk lands separately because it is entangled with the #5073 lane.Known environment items, neither caused by this PR: the sidecar CMD does not build the Pi extension (the recipe covers it, see above), and a dev-DB session-table schema drift is tracked separately.
https://claude.ai/code/session_014iPB7HL5PjgT9npyPHaFMT