diff --git a/api/oss/src/apis/fastapi/applications/overlay.py b/api/oss/src/apis/fastapi/applications/overlay.py index f0b13de6fa..9be05d64c4 100644 --- a/api/oss/src/apis/fastapi/applications/overlay.py +++ b/api/oss/src/apis/fastapi/applications/overlay.py @@ -15,6 +15,7 @@ "commit_revision", "annotate_trace", "query_spans", + "test_run", "discover_triggers", "create_schedule", "create_subscription", diff --git a/api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py b/api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py index 6225da7ab5..ecb1f8660b 100644 --- a/api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py +++ b/api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py @@ -31,6 +31,7 @@ "commit_revision", "annotate_trace", "query_spans", + "test_run", "discover_triggers", "create_schedule", "create_subscription", diff --git a/api/oss/tests/pytest/unit/workflows/test_static_catalog.py b/api/oss/tests/pytest/unit/workflows/test_static_catalog.py index 522efdf080..57aa549ed3 100644 --- a/api/oss/tests/pytest/unit/workflows/test_static_catalog.py +++ b/api/oss/tests/pytest/unit/workflows/test_static_catalog.py @@ -126,6 +126,7 @@ def test_default_static_skill_catalog_replaces_old_authoring_skills(): skill = playbook.data.parameters["skill"] assert skill["name"] == "build-an-agent" assert skill["body"].startswith("# Build an Agenta agent") + assert "test_run" in skill["body"] assert "query_spans" in skill["body"] diff --git a/docs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.md b/docs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.md index 597731bb1e..c4eef98427 100644 --- a/docs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.md +++ b/docs/design/agent-workflows/interfaces/cross-service/runner-to-tool-callback.md @@ -65,9 +65,12 @@ The same `POST /tools/call` serves three kinds of callback tool, routed by the ` (`tools.agenta.find_capabilities` via `_call_agenta_tool`) is deleted; discovery is the `discover_tools` [platform tool](../in-service/tool-models-and-resolution.md), whose SDK resolution emits a direct `call` to `POST /api/tools/discover` (the `call` descriptor below), - bypassing `/tools/call`. Handler-mode resolution is flag-gated off - (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`) until the runner dispatches reserved refs with - spec-level context injection. + bypassing `/tools/call`. Handler-mode resolution is default-on in the SDK, with + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` as a kill switch (`0`, `false`, `f`, `n`, `no`, + `off`, `disable`, or `disabled`). The runner dispatches reserved `callRef` specs through + `/tools/call`, applies spec-level `contextBindings` after the permission verdict, and + forwards `timeoutMs` end to end. Positive per-spec timeouts add a 10s grace window to the + host callback fetch and the child relay poll. The runner is unchanged for all three: it relays a `callback` spec with whatever `call_ref` the resolver put on it. Only the router's prefix dispatch is aware of the grammars. @@ -95,9 +98,11 @@ platform-op resolver emits `call` (and `call.context` for self-targeting ops suc `commit_revision`); the runner dispatches it host-direct with the SSRF guardrails. Gateway and reference tools still route through `/tools/call`. Handler-mode platform ops add a second wire shape: a reserved `tools.agenta.{op}` `call_ref` plus spec-level `contextBindings` and -`timeoutMs` (SDK side only so far: `tools/models.py` + `wire_models.py`); the resolver -only emits it behind `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` (default off) because the runner -(`protocol.ts` included) does not yet carry or dispatch the new spec fields. Full spec: +`timeoutMs`. The runner carries those fields, dispatches `callRef` specs through the host relay, +applies `contextBindings` only after the permission verdict, forwards `runContext.run.kind` as +`x-agenta-run-kind`, and honors `timeoutMs` on both the host callback fetch and the child relay +poll. The SDK emits handler-mode ops by default; set `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` to a +disabled value to turn them off. Full spec: `docs/design/agent-workflows/projects/direct-call-tools/` and `docs/design/agent-workflows/projects/build-kit-tools-cleanup/api-design.md`. @@ -126,10 +131,12 @@ only emits it behind `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` (default off) becau `$ctx.` namespace, not camelCase. The runner dispatches `call` and fills `call.context` (`tools/direct.ts` `assembleBody`); the platform-op catalog emits them for endpoint-mode ops. - **Handler-mode spec fields.** A handler-mode op emits a reserved `call_ref` plus spec-level - `contextBindings` and `timeoutMs` (today only on the SDK side, `tools/models.py` + - `wire_models.py`). The runner does not carry or read them yet; keep the flag - (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`) off until `protocol.ts` mirrors them and the relay - injects them, and move the mirrors and the goldens together when it does. + `contextBindings` and `timeoutMs`. Keep those fields mirrored across `protocol.ts`, the SDK + `CallbackToolSpec`, `wire_models.py`, and the golden fixtures. The relay applies + `contextBindings` after the permission verdict and redacts bound paths from the Pi pending + approval display. `timeoutMs` is honored by the host `/tools/call` fetch and the child file-relay + poll; positive per-spec timeouts get the same 10s grace in both places. The SDK flag is default + on with `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` as a kill switch. - **Tool result content.** `call.data.content` is a JSON string already; do not double-encode it on the way out. - **Argument normalization.** Keep accepting both string and object arguments. diff --git a/docs/design/agent-workflows/projects/build-kit-tools-cleanup/api-design.md b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/api-design.md index f296f65b57..c71b4435a3 100644 --- a/docs/design/agent-workflows/projects/build-kit-tools-cleanup/api-design.md +++ b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/api-design.md @@ -143,24 +143,25 @@ In the tools domain (`core/tools/` service code, dispatched from the tool-call p skill tells the agent to warn the user (same convention as `test_subscription`'s blocking behavior note in the tools-review). -## 5a shipped / 5b contract - -Slice 5a ships the server handler and SDK catalog shape, but the committed runner is not -yet the 5b runner. Until 5b lands, these limits are part of the contract: - -- The recursion guard is inert until 5b propagates `x-agenta-run-kind` from - `RunContext` onto every child `/tools/call`. 5a sets `meta.run_kind = "test"` - on the child invoke and refuses marked requests, but old runners do not forward - the mark. -- On timeout, the child invoke is orphaned, not cancelled. The timeout response - carries no `trace_id`; recovery depends on a time-window `query_spans` lookup - in the same project. -- Until 5b honors spec `timeoutMs`, the committed runner aborts relayed calls at - `TOOL_CALL_TIMEOUT_MS=30s` while the server-side child run may continue up to - 120s. -- Handler-mode resolver output is gated by - `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off. 5b flips the default on - when the runner honors `callRef`, `contextBindings`, and `timeoutMs`. +## 5a shipped / 5b landed + +**5b landed on 2026-07-05.** The contract items below are now live: + +- The recursion header is propagated. The handler sets `meta.run_kind = "test"` + on the child invoke, the agent service surfaces it as `RunContext.run.kind`, + and the runner forwards it to child `/tools/call` requests as + `x-agenta-run-kind`. The handler refuses marked requests, so recursive + `test_run` calls remain capped at one level. +- `timeoutMs` is honored end to end. The runner applies it to the host + `/tools/call` fetch and the child file-relay poll. A positive per-spec timeout + gets a 10s grace window on both sides so digest/span work produced at the child + ceiling can still return. +- Handler-mode resolver output is default-on. Set + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` to `0`, `false`, `f`, `n`, `no`, + `off`, `disable`, or `disabled` to disable it. +- The orphaned-timeout caveat is unchanged. On timeout, the child invoke is + orphaned, not cancelled. The timeout response carries no `trace_id`; recovery + depends on a time-window `query_spans` lookup in the same project. ## Shape decision (provisional default: sync + delta, flagged for PR review) {#shape-decision} diff --git a/docs/design/agent-workflows/projects/build-kit-tools-cleanup/plan-5b.md b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/plan-5b.md new file mode 100644 index 0000000000..a25c2b2307 --- /dev/null +++ b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/plan-5b.md @@ -0,0 +1,353 @@ +# Plan 5b: the runner half of `test_run` + +Status: planned, 2026-07-05. Slice 5a is merged (big-agents `1756d3d838`, PR #5068). +This plan lands the runner half on ONE GitButler lane as ONE PR, in commit-sized +slices. Contract source: [api-design.md](api-design.md) "5a shipped / 5b contract"; +the open-issues entry "Land the runner half of test_run (slice 5b)" +(`docs/design/agent-workflows/scratch/open-issues.md:216`). + +All citations below verified against the working tree on 2026-07-05 (post-#5066 +pi-builtin-gating, post-#5068 slice 5a, post claude-client-tools-recut). + +## Contention verdict: the runner surface is FREE + +- `git status --porcelain -- services/runner/` is clean. No unassigned changes touch + `services/runner/` (the unassigned set is docs/scratch plus one SDK test). +- `feat/claude-client-tools-recut` is an ancestor of `origin/big-agents` (merged; the + runner commits `e69310fdd2..618764edae` are on big-agents). The pi-builtin-gating WIP + that blocked 5a is merged as #5066 (`85120747b2..8d1b836ca1`). +- No applied lane owns `services/runner/` files. Two applied lanes own NEARBY files — + plan around them, do not touch their files: + - `feat/mcp-default-on-recut` (`2263b059ff`) owns + `services/oss/src/agent/tools/resolver.py` + + `services/oss/tests/pytest/unit/agent/tools/test_resolution.py`. Slice 3 edits + `app.py` (disjoint), and adds a NEW test file rather than editing shared ones. + - `feat/pi-openai-codex-capability` (`834636ae53`) owns + `services/oss/tests/pytest/unit/agent/test_invoke_handler.py`. Same rule: new test + file, not that one. +- Take the board BUT-LOCK before any `but` writes and post a row on + `docs/design/agent-workflows/scratch/agent-coordination.md` when starting. + +## Code truth (what exists NOW, corrected against the merged tree) + +**The SDK half is already emitted.** Slice 5a shipped more of the wire than the old +contract text implies: + +- The resolver emits handler-mode specs with `call_ref="tools.agenta.test_run"` + + `context_bindings` + `timeout_ms` + (`sdks/python/agenta/sdk/agents/platform/platform_tools.py:86-114`; catalog entry + `op_catalog.py:942-950`, `timeout_ms=120000`, + `context_bindings={"target.workflow_variant_id": "$ctx.workflow.variant.id"}`). + Flag-gated: a configured handler-mode op RAISES when + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` is off (`platform_tools.py:86-93`). +- The spec model already carries + serializes the fields with the camelCase aliases + (`sdks/python/agenta/sdk/agents/tools/models.py:368-376`, `to_wire` at `:308-316` + dumps `by_alias`), and the schema-source wire models already pin them + (`sdks/python/agenta/sdk/agents/wire_models.py:257-260`). +- So the Python wire side needs only the `run_kind` addition; `contextBindings` / + `timeoutMs` need golden + TS-side work, not SDK emission work. + +**The runner ignores all three today:** + +- `protocol.ts` `ResolvedToolSpec` (`services/runner/src/protocol.ts:104-140`) has + `callRef` and `call` but NO `contextBindings` / `timeoutMs`. `RunContext` + (`protocol.ts:174-185`) has `workflow` + `trace`, no run-kind group. +- A `callRef` spec dispatches through the gateway fallback: + `relay.ts` `executeAllowedRelayedTool` → `callAgentaTool` posts `function.name = + spec.callRef` to `/tools/call` (`services/runner/src/tools/relay.ts:296-303`, + `callback.ts:52-64`). Args go verbatim; nothing injects bindings; the 5a server + reserved-registry dispatch (`api/oss/src/apis/fastapi/tools/router.py:1099-1101`) + therefore receives calls missing `target.workflow_variant_id`. +- The permission verdict fires BEFORE execution, structurally: + `executeRelayedTool` decides (`relay.ts:239-262`; deny returns, ask pauses via + `onPendingApproval`) and only then calls `executeAllowedRelayedTool` + (`relay.ts:264`). For Claude, `permissions.enforce` is false (`relay.ts:118-121`) + because the harness raises its own gates first; execution still funnels through + `executeAllowedRelayedTool`. Injection placed inside the `callRef` branch of + `executeAllowedRelayedTool` is therefore after-verdict by construction on BOTH + harnesses. On resume-after-approval, the harness re-issues the call and the stored + decision allows it through the SAME path (`permission-plan.ts:138-151`), so the + approved execution also gets the injection. +- Timeouts today: host `/tools/call` fetch aborts at `TOOL_CALL_TIMEOUT_MS` = 30s + (`callback.ts:15-17,42`); the CHILD-side file-relay poll gives up at + `RELAY_TIMEOUT_MS` = 60s (`dispatch.ts:87`, `relay.ts:60-62`). Both are below the + 120s the server handler runs to (`platform_handlers.py:132`). +- `$ctx` machinery to reuse: `resolveCtxToken` / `deepSet` / `deepDelete` / + `isPlainObject` in `services/runner/src/tools/direct.ts:129-147,52-93` — the + fail-closed binding precedent is `assembleBody` step 3 (`direct.ts:226-237`). +- Both live delivery paths relay every custom-tool execution to the HOST relay loop: + Pi via the extension (`extensions/agenta.ts:241-279`, requires + `AGENTA_AGENT_TOOLS_RELAY_DIR`) and Claude via the internal MCP channel + (`tool-mcp-http.ts:206-213`, always passes `relayDir`). The host loop is + `startToolRelay` (`relay.ts:399-495`), wired with `request.runContext` at + `engines/sandbox_agent.ts:838-847`. The `dispatch.ts` non-relay direct-POST branch + (`dispatch.ts:260-267`) is unreachable in live paths — keep bindings OUT of it (the + child must never hold private spec fields; `public-spec.ts` strips them). +- Public specs the child sees: `AdvertisedToolSpec` + (`services/runner/src/tools/public-spec.ts:11-17`) — no `timeoutMs` today, and the + env is filled from `advertisedToolSpecs` at + `engines/sandbox_agent/pi-assets.ts:60-66`. + +**run_kind path:** the 5a handler stamps `meta["run_kind"] = "test"` on the child +invoke and refuses requests carrying `x-agenta-run-kind: test` +(`api/oss/src/core/tools/platform_handlers.py:71-72,101-104,118-121`). The agent +service's handler receives that as `request.meta` (`WorkflowServiceRequest` = +`WorkflowInvokeRequest` extends the `Metadata` mixin with `meta`, +`sdks/python/agenta/sdk/models/workflows.py:245,285-290`, +`models/shared.py:155-158`), but `_agent` +(`services/oss/src/agent/app.py:207-278`) never reads it, and nothing puts it on the +`/run` wire (`wire.py` `request_to_wire`, `RunContext.to_wire` at +`sdks/python/agenta/sdk/agents/dtos.py:469-496`). + +**Flip targets:** `DEFAULT_BUILD_KIT_OPS` +(`api/oss/src/apis/fastapi/applications/overlay.py:13-26`, 12 ops, no `test_run`); +flag default off (`platform_tools.py:36-43`). **Playbook seam:** the marked +paragraph is step 6 of `_BUILD_AN_AGENT_BODY` +(`sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py:154-160`), the wired-tools +list (`:192-195`), and the seam comment (`:207-208`). + +**Contract pinning:** goldens at +`sdks/python/oss/tests/pytest/unit/agents/golden/run_request.{pi_core,claude}.json` +(neither carries `contextBindings`/`timeoutMs`/run kind today); Python side +`test_wire_contract.py:236,370` builds the payload from DTOs and compares whole-dict; +TS side `services/runner/tests/unit/wire-contract.test.ts` asserts every golden +top-level key is in `KNOWN_REQUEST_KEYS` (compile-time guard at `:66`) plus targeted +spec-field assertions. Adding fields means touching golden + Python builder + TS +assertions + `protocol.ts` + `wire_models.py` in ONE commit. + +## Design decisions baked in + +- **Injection is callRef-branch-only, runner-filled-last, fail-closed.** For a spec + with `callRef` + `contextBindings`, the runner deletes whatever the model put at + each bound arg path, resolves the `$ctx` token, and deep-sets the value — model can + never override; an unresolvable token throws (mirrors `assembleBody`, + `direct.ts:226-237`). The `call` branch keeps its own `call.context` mechanism; + nothing is applied twice. The SDK validator already enforces `context_bindings` XOR + direct-call (`models.py:393-396`). +- **run kind rides `runContext.run.kind`** (snake_case inner namespace, like + `workflow`/`trace`), not a new top-level field. It is the run's own identity, the + contract's suggested extension point (`protocol.ts:174-185`), and it makes + `$ctx.run.kind` resolvable for free. The runner forwards it verbatim as + `x-agenta-run-kind` on every child `/tools/call` (and, defensively, on direct + calls), whenever set — not hardcoded to `"test"`. +- **Per-spec `timeoutMs` must reach BOTH timeout sites**: the host fetch + (`callback.ts`) and the child/relay poll deadline (`dispatch.ts` `relayToolCall`). + `timeoutMs` is not secret, so it joins `AdvertisedToolSpec`; `contextBindings` and + `callRef` stay executor-private. +- **The kill switch changes semantics at flip time.** Today flag-off RAISES on a + configured handler op (`platform_tools.py:86-93`) — correct while `test_run` is + opt-in, but once it is in `DEFAULT_BUILD_KIT_OPS`, "flag off = raise" bricks every + default build-kit agent. The flip commit makes explicit-off SKIP the op with a + warning (resolution proceeds without `test_run`), keeps the env var + (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`) as the kill switch, and defaults it ON. + Recommendation: keep the flag one release, then delete via defer-todo. +- **Approval semantics untouched.** No changes to `permission-plan.ts`, + `responder.ts`, or the decide/pause flow (#5041/#5066 machinery). We only add + post-verdict argument shaping inside the already-allowed execution. + +## Slices (one lane, one PR; each slice = one verifiable commit) + +### Slice 5b.1 — the wire: `contextBindings` / `timeoutMs` / `runContext.run` on both sides + +Files: +- `services/runner/src/protocol.ts` — `ResolvedToolSpec` += `contextBindings?: + Record` and `timeoutMs?: number` (doc: valid with `callRef` only; + bindings are executor-private, runner-filled-last, hidden from the model). + `RunContext` += `run?: { kind?: string }` (snake_case namespace note). +- `sdks/python/agenta/sdk/agents/dtos.py` — `RunContextRun` model; `RunContext.run` + field; `to_wire` emits `run: {kind}` only when set (omit-when-empty, `:469-496`). +- `sdks/python/agenta/sdk/agents/wire_models.py` — `WireRunContext` += `run` + (`WireRunContextRun`). Spec fields already exist (`:257-260`) — verify only. +- Goldens `run_request.pi_core.json` + `run_request.claude.json` — the existing + callback spec (`customTools[0]`) gains `"contextBindings": {"target.workflow_variant_id": + "$ctx.workflow.variant.id"}` + `"timeoutMs": 120000`; `runContext` gains + `"run": {"kind": "test"}`. +- `sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py` — the pi/claude + request builders set the fields via `CallbackToolSpec(context_bindings=..., + timeout_ms=...)` and `RunContext(run=...)` so the whole-dict compare passes. +- `services/runner/tests/unit/wire-contract.test.ts` — assert + `tool.contextBindings`, `tool.timeoutMs`, `req.runContext.run.kind` from the golden; + `KNOWN_REQUEST_KEYS` unchanged (no new top-level key). +- `test_wire_models.py` schema assertion if it enumerates RunContext fields. + +Acceptance: `py-run-tests` wire suites + `pnpm test` wire-contract green; a request +with none of the new fields serializes byte-identical to before (omit-when-empty). + +### Slice 5b.2 — runner execution: inject bindings, honor `timeoutMs`, forward run kind + +Files: +- `services/runner/src/tools/direct.ts` — export + `applyContextBindings(args, bindings, runContext): Record`: + copy args if plain object else `{}`; per `[argPath, token]`: `deepDelete` → + `resolveCtxToken` → throw + `missing run-context value for tool binding ''` on `undefined` → `deepSet`. + `callDirect` gains optional `{ runKind }` → `x-agenta-run-kind` header (defense in + depth; reserved handlers are only reachable via `/tools/call` today). +- `services/runner/src/tools/callback.ts` — `callAgentaTool` gains an options arg + `{ timeoutMs?, runKind? }`: timeout = `timeoutMs ?? TOOL_CALL_TIMEOUT_MS` + (`:42`); set `x-agenta-run-kind` when `runKind` is set. Update the two callers. +- `services/runner/src/tools/relay.ts` — in `executeAllowedRelayedTool` + (`:267-304`): in the `callRef` (gateway fallback) branch ONLY, apply + `applyContextBindings` when `spec.contextBindings` is set, then call + `callAgentaTool` with `{ timeoutMs: spec.timeoutMs, runKind: + runContext?.run?.kind }`. Pass `runKind` on the `spec.call` branch's `callDirect` + too. No change to `executeRelayedTool`'s verdict flow (`:239-262`) — injection is + after-verdict by construction. +- `services/runner/src/tools/dispatch.ts` — `relayToolCall` deadline honors a + per-call timeout: `deadline = now + (timeoutMs ? timeoutMs + 10_000 : + RELAY_TIMEOUT_MS)` (`:87`); `runResolvedTool` threads `spec.timeoutMs` into it and + into the (unreachable-live) direct `callAgentaTool` branch. NO binding injection + here — the child never holds private spec fields. +- `services/runner/src/tools/public-spec.ts` — `AdvertisedToolSpec` += `timeoutMs`; + `advertisedToolSpec` copies it. `contextBindings`/`callRef` stay stripped. +- `pnpm run build:extension` — the Pi extension bundle in `dist/` must be rebuilt + (registerTools executes `runResolvedTool` in-sandbox from the bundle; stale-bundle + is a known silent failure mode from QA F-findings). + +Tests (`services/runner/tests/unit/`): +- New `tool-callref-bindings.test.ts` through `startToolRelay` with a fake + `RelayHost` + fetch stub: (a) allowed callRef spec with bindings → posted + `function.arguments` carries the runner value even when the model supplied a + conflicting `target.workflow_variant_id`; (b) missing run-context value → tool error + response, no fetch; (c) deny spec → deny text, fetch never called; (d) ask spec → + pause, fetch never called (verdict-before-injection pinned); (e) + `x-agenta-run-kind` present iff `runContext.run.kind` set; (f) fetch abort at + `timeoutMs` when set, `TOOL_CALL_TIMEOUT_MS` when not; (g) a `call`-descriptor spec + is untouched by spec-level bindings (callRef-branch-only pinned). +- `tool-dispatch.test.ts` — `relayToolCall` deadline extends with `timeoutMs`. +- A public-spec assertion: advertised specs never carry `contextBindings`/`callRef`, + do carry `timeoutMs`. + +Acceptance: `pnpm test` + `pnpm run typecheck` green in `services/runner`. + +### Slice 5b.3 — the agent service surfaces `meta.run_kind` into `runContext.run` + +Files: +- `services/oss/src/agent/app.py` — in `_agent` (`:207`): read + `(request.meta or {}).get("run_kind")`; when a non-empty string, stamp it onto the + computed run context (`rc = run_context() or RunContext(); rc.run = + RunContextRun(kind=...)`) before `SessionConfig(run_context=...)` (`:246-261`). +- New test `services/oss/tests/pytest/unit/agent/test_run_kind.py` (a NEW file — + `test_invoke_handler.py` is owned by the applied `feat/pi-openai-codex-capability` + lane): meta.run_kind="test" → wire payload `runContext.run.kind == "test"`; absent + meta → `runContext` unchanged (no `run` key). + +Acceptance: `cd services && py-run-tests` green; the end-to-end recursion chain is now +closed on paper: handler stamps meta (`platform_handlers.py:118-121`) → service stamps +runContext.run → runner forwards header → handler refuses (`:101-104`). + +### Slice 5b.4 — the flip: overlay + flag default ON (kill-switch semantics change) + +Files: +- `api/oss/src/apis/fastapi/applications/overlay.py:13-26` — add `"test_run"` to + `DEFAULT_BUILD_KIT_OPS` (13 ops). +- `sdks/python/agenta/sdk/agents/platform/platform_tools.py:36-43,86-93` — + `_platform_handlers_enabled()` defaults TRUE (env value `"false"`/`"0"` disables); + flag-off now SKIPS the handler-mode op with a warning instead of raising (see + design decisions — raise would brick every default agent under the kill switch). +- Tests: `api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py` (ops + pin + builtin grants pin), SDK resolver tests (flag-unset resolves; explicit-off + skips and the remaining tools still resolve; no raise path left). +- keep-docs-in-sync touch points in the same commit: + `docs/design/agent-workflows/documentation/tools.md` (op table + flag default), + `documentation/agent-configuration.md` if it lists the overlay ops, and any env + example that documents `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`. + +Acceptance: fresh overlay agent's resolved tool set includes `test_run`; setting the +env to `false` yields the 12-op set with a logged warning, no resolution error. + +### Slice 5b.5 — playbook seam swap + +Files: +- `sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py` — replace step 6 + (`:154-160`) with the `test_run` procedure: call `test_run` with a blunt + instruction-framed `inputs.messages` + `expectations.terminal_tool`; warn the user + first that a test run really fires external writes; read `verdict` / + `verdict_reason` / `approvals`; on `incomplete`, rewrite instructions as a blunter + numbered procedure and re-test; keep `query_spans` for verifying SCHEDULED fires + (that is still its job per api-design). Update the wired-tools list (`:192-195`) to + include `test_run`, adjust the footguns ("empty output ... inspect spans" line + softens to the test_run digest), delete the seam comment (`:207-208`). +- Update whatever test pins the skill body/roster (grep `build-an-agent` under + `sdks/python/oss/tests/` and `api/oss/tests/`; the 5a lane touched + `test_static_catalog.py` / skill catalog tests). + +Acceptance: skill tests green; the body names only tools that exist in the default +overlay after 5b.4. + +### Slice 5b.6 — live verification (debug-local-deployment phase, not a commit) + +Deploy notes first — three restart gotchas, all previously burned: +- The sub-sidecar has NO hot-reload and `run.sh --build` does not rebuild it: + `docker restart agenta-claude-sub-sidecar` after ANY runner TS change. +- SDK (`sdks/python`) edits need the services + api containers restarted. +- The Pi extension bundle is prebuilt: verify the deployed image/compose command runs + `build:extension`, or custom tools silently keep the old behavior (QA F-finding). + +The matrix, per the agent-workflows-qa discipline (local + sidecar, cheap models): +1. **The full loop in playground chat** (pi_agenta, in-process/local): ask the builder + agent to build a small agent, commit, then `test_run` itself → the approval gate + surfaces in chat (`test_run` is non-read under `allow_reads`) → approve → digest + returns with `output`, ordered `tools`, `resolved`, `verdict`. This is the lab + capstone acceptance from the open-issues entry. +2. **Gated write in the child**: child config with a write tool → `approvals` lists + it, verdict `unconfirmed`. +3. **Recursion refusal live**: `curl POST /tools/call` with the reserved ref and + header `x-agenta-run-kind: test` → refused; and end-to-end, a child run's own + `test_run` call is refused (runner forwards the header from + `runContext.run.kind`). +4. **Timeout override**: a child run engineered past 30s completes (proves + `TOOL_CALL_TIMEOUT_MS` no longer bites) and past 60s completes (proves the child + poll deadline honors `timeoutMs`). +5. **Hand-authored-config path**: a non-overlay agent config that lists + `{"type": "platform", "op": "test_run"}` resolves and runs (5a's flag error is + gone). +6. **Claude cell** (if sidecar credit allows): the same `test_run` call over the + internal MCP channel — the harness gate fires instead of the relay gate; injection + still lands (the relay executes post-gate). +7. Known environment caveat, do not chase as a 5b bug: the local in-process → + sub-sidecar path returns `trace_id=None` (sidecar persist 401s), leaving the span + digest empty — pre-existing, tracked in status.md "Deferred / follow-ups". + +## The 3 riskiest spots + +1. **The child-side relay deadline and the stale extension bundle** — + `services/runner/src/tools/dispatch.ts:87` (`deadline = Date.now() + + RELAY_TIMEOUT_MS`) with `RELAY_TIMEOUT_MS=60s` (`relay.ts:60-62`). Honoring 120s on + the host fetch alone still kills the call at 60s in the child's poll loop, and for + Pi that loop lives in the PREBUILT extension bundle + (`extensions/agenta.ts:266-270` → `dist/`), which has a documented history of + shipping stale. The fix needs `timeoutMs` on `AdvertisedToolSpec`, the deadline + change, a rebuilt bundle, AND live check 4 to prove it. +2. **The flip's kill-switch semantics and deploy skew** — + `platform_tools.py:86-93` raises on flag-off; combined with + `overlay.py:13-26` gaining `test_run`, an unchanged raise makes the kill switch + (or any stale env) fail EVERY default build-kit agent's tool resolution. Separately, + an old runner + new SDK skew silently drops the bindings, so the child `/tools/call` + arrives without `target.workflow_variant_id` and the server rejects it — ship + runner image and SDK together (same compose deploy), and make flag-off skip, not + raise. +3. **Injection ordering across harnesses and the resume path** — + `relay.ts:239-262` enforces the verdict only when `permissions.enforce` (Pi); + Claude's gate is the harness's own, and the approved re-issue must re-enter + `executeAllowedRelayedTool` to get the injection. The structure guarantees it + (every execution funnels through `relay.ts:264` → `:267-304`), but a future branch + that calls `callAgentaTool` directly (e.g. `dispatch.ts:260-267` becoming + reachable) would bypass both verdict and bindings. Test (g) + the public-spec + assertion pin this; keep injection ONLY in the callRef branch and bindings out of + advertised specs. + +## Constraints (restated from the project + owner rules) + +- Approval machinery (#5041/#5066) untouched: no edits to `permission-plan.ts`, + `responder.ts`, `acp-interactions.ts` beyond what the tests require (none expected). +- Injection in the `callRef` branch only; `call.context` stays the direct-call + mechanism. +- One lane over big-agents, `but commit --only` + verify per commit; board row + + BUT-LOCK for `but` writes; PR base `big-agents`, do not merge without review. +- Docs move in the same PR (keep-docs-in-sync): tools.md, agent-configuration.md, + the interface inventory pages listing the `/run` spec fields + (`cross-service/runner-to-tool-callback.md`, `public-edge/agent-config-schema.md`), + and the api-design.md "5a shipped / 5b contract" section gets a "5b landed" note. +- After landing: close the open-issues 5b entry, add the status.md row, and file a + defer-todo for deleting the kill-switch env after one release. diff --git a/docs/design/agent-workflows/projects/build-kit-tools-cleanup/status.md b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/status.md index 29f648c79f..8afaa5e14f 100644 --- a/docs/design/agent-workflows/projects/build-kit-tools-cleanup/status.md +++ b/docs/design/agent-workflows/projects/build-kit-tools-cleanup/status.md @@ -62,6 +62,16 @@ contended; approval for new ops is expressed by the `read_only` hint under the m permission plan (write + no explicit permission → ask). The coordination constraint moved from `op_catalog.py` to the runner/wire surface; see plan.md. +2026-07-05: Slice 5b PLANNED at [plan-5b.md](plan-5b.md). Contention re-checked: the +runner surface is FREE (`claude-client-tools-recut` and pi-builtin-gating #5066 are +merged; no lane or WIP owns `services/runner/`). Key code-truth corrections vs the old +contract text: the SDK already emits `contextBindings`/`timeoutMs` (5a), so 5b's wire +work is protocol.ts + goldens + run-kind only; the CHILD relay poll deadline (60s, +`dispatch.ts:87`) must honor `timeoutMs` too, not just the host fetch; and the flip +must change the flag's off-semantics from raise to skip, or the kill switch bricks +every default build-kit agent once `test_run` joins `DEFAULT_BUILD_KIT_OPS`. Six +slices on one lane, one PR; live-debug matrix per debug-local-deployment. + ## Decided (do not relitigate) 1. Hard-migrate renames, no aliases: `find_capabilities` -> `discover_tools`, diff --git a/sdks/python/agenta/sdk/agents/__init__.py b/sdks/python/agenta/sdk/agents/__init__.py index 27598e958a..a3237d05af 100644 --- a/sdks/python/agenta/sdk/agents/__init__.py +++ b/sdks/python/agenta/sdk/agents/__init__.py @@ -70,6 +70,7 @@ PiAgentTemplate, RunContext, RunContextReference, + RunContextRun, RunContextTrace, RunContextWorkflow, SandboxPermission, @@ -173,6 +174,7 @@ "TraceContext", "RunContext", "RunContextReference", + "RunContextRun", "RunContextWorkflow", "RunContextTrace", "ToolCallback", diff --git a/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py b/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py index 42cdb841ec..3ab6af412b 100644 --- a/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py +++ b/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py @@ -151,13 +151,16 @@ 5. Configure yourself. Put the chosen `capability.tool` entries and needed alternatives in `tools`, write `instructions.agents_md`, and call `commit_revision`. This is an approval stop. If the commit is denied or fails, earlier connections or triggers are not undone. -6. Verify with `query_spans`. Ask the user to send the agreed test message in this chat. Then - call `query_spans` with `windowing.oldest` and `windowing.newest` bracketing the test, plus a - sensible `windowing.limit`; add `filtering.conditions` on `trace_id` if you know it. Find the - run's spans, read the tool-call spans in order, and pass only when the terminal tool span is - 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 + tools may perform their action if approved. Then call `test_run` with + `inputs.messages` as a blunt instruction-framed test message and + `expectations.terminal_tool` set to the final tool that proves success. Read `verdict`, + `verdict_reason`, `tools`, and `approvals`; a 200 response is not proof. Pass only + when `verdict` is `pass`. If `approvals` is non-empty, this is an approval stop: + report the waiting gate and wait for the user. If `verdict` is `incomplete`, rewrite + `instructions.agents_md` as a blunter numbered procedure, call `commit_revision`, and + run `test_run` again. Use `query_spans` after schedules or subscriptions fire to read + back the SCHEDULED run spans. 7. Add a trigger only if asked. For schedules, cron is UTC, five fields, with a one-minute floor; convert the user's timezone yourself, then stop for approval before `create_schedule`: say what you are about to create and wait for the gate. After approval, call `create_schedule`, then @@ -189,14 +192,15 @@ ## Prefer wired tools Prefer your wired tools (`discover_tools`, `request_connection`, `commit_revision`, -`query_spans`, `create_schedule`, `list_schedules`, `discover_triggers`, +`test_run`, `query_spans`, `create_schedule`, `list_schedules`, `discover_triggers`, `create_subscription`, `test_subscription`, `list_deliveries`, `remove_schedule`, `remove_subscription`) over harness builtins. Touch Terminal, RemoteTrigger, File tools, or raw HTTP only when your wired tools cannot do the job, and say so when you do. ## Footguns -- Empty output with a healthy tool sequence is not failure; inspect the spans before judging. +- Empty output is not enough to fail a run; read the `test_run` verdict, tools, approvals, + and verdict_reason before judging. - Never surface raw provider slugs such as `provider_action` to the user; speak in Agenta terms. - Re-run discovery after the user connects an integration so the committed tool gets the concrete connection id. @@ -204,8 +208,6 @@ - Trigger inputs must match what the instructions expect, or the run starts empty. """ -# Slice 5 seam: replace the query_spans verification paragraph above with the test_run paragraph -# when that platform op ships. BUILD_AN_AGENT_SKILL = SkillTemplate( name="build-an-agent", description=( diff --git a/sdks/python/agenta/sdk/agents/dtos.py b/sdks/python/agenta/sdk/agents/dtos.py index 9fd5777a22..94993895b7 100644 --- a/sdks/python/agenta/sdk/agents/dtos.py +++ b/sdks/python/agenta/sdk/agents/dtos.py @@ -446,13 +446,23 @@ class RunContextTrace(BaseModel): span_id: Optional[str] = None +class RunContextRun(BaseModel): + """The current run's own identity inside ``runContext``. + + ``kind`` is deliberately nested under ``run`` so ``$ctx.run.kind`` is protocol context, not a + new top-level command field. Reserved platform handlers use it to identify child test runs. + """ + + kind: Optional[str] = None + + class RunContext(BaseModel): """The run's own context, delivered on ``/run`` and refreshed per turn (direct-call tools, Phase 3a; see ``projects/direct-call-tools/run-context.md``). The service computes this from the invocation's own trace + workflow identity and sends it on - the ``/run`` request. It is consumed ONLY by a tool's ``call.context`` binding: the runner - fills bound request fields from this blob at dispatch, server-side and hidden from the model. + the ``/run`` request. It is consumed by tool context bindings: ``call.context`` on direct-call specs and + ``contextBindings`` on callRef specs. The runner fills bound request fields from this blob at dispatch, server-side and hidden from the model. The model never reads run context directly. The inner keys are deliberately snake_case (``workflow.variant.id``, ``trace.trace_id``): they @@ -463,11 +473,20 @@ class RunContext(BaseModel): emits only the sub-objects/fields that are set, so a run with no identity yields an empty blob (and the serializer omits the key entirely).""" + run: Optional[RunContextRun] = None workflow: Optional[RunContextWorkflow] = None trace: Optional[RunContextTrace] = None def to_wire(self) -> Dict[str, Any]: out: Dict[str, Any] = {} + if self.run is not None: + run = { + key: value + for key, value in self.run.model_dump().items() + if value is not None + } + if run: + out["run"] = run if self.workflow is not None: workflow: Dict[str, Any] = {} for entity in ("artifact", "variant", "revision"): diff --git a/sdks/python/agenta/sdk/agents/platform/platform_tools.py b/sdks/python/agenta/sdk/agents/platform/platform_tools.py index 89cd1f835b..01d10fc04e 100644 --- a/sdks/python/agenta/sdk/agents/platform/platform_tools.py +++ b/sdks/python/agenta/sdk/agents/platform/platform_tools.py @@ -34,14 +34,15 @@ log = get_module_logger(__name__) _ENABLE_PLATFORM_HANDLERS_ENV = "AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS" -_TRUTHY_ENV_VALUES = {"1", "true", "t", "y", "yes", "on", "enable", "enabled"} +_DISABLED_ENV_VALUES = {"0", "false", "f", "n", "no", "off", "disable", "disabled"} +# Empty string intentionally follows the default-on behavior. Unset now means enabled. def _platform_handlers_enabled() -> bool: - return ( - os.getenv(_ENABLE_PLATFORM_HANDLERS_ENV, "").strip().lower() - in _TRUTHY_ENV_VALUES - ) + value = os.getenv(_ENABLE_PLATFORM_HANDLERS_ENV) + if value is None: + return True + return value.strip().lower() not in _DISABLED_ENV_VALUES class AgentaPlatformToolResolver: @@ -71,6 +72,15 @@ async def resolve( tool_specs: list[CallbackToolSpec] = [] for tool_config in tools: op = get_platform_op(tool_config.op) + if op.handler is not None and not _platform_handlers_enabled(): + log.warning( + "agent: skipping platform handler-mode op %r because " + "%s is explicitly disabled", + op.op, + _ENABLE_PLATFORM_HANDLERS_ENV, + ) + continue + if op.op in seen: error = GatewayToolResolutionError( f"Duplicate platform tool: {op.op}", @@ -84,14 +94,6 @@ async def resolve( # gateway ``call_ref`` (with spec-level bindings the relay injects); endpoint-mode # ops carry a direct ``call`` descriptor (bindings ride inside ``call.context``). if op.handler is not None: - if not _platform_handlers_enabled(): - error = GatewayToolResolutionError( - f"Platform handler-mode op '{op.op}' requires " - f"{_ENABLE_PLATFORM_HANDLERS_ENV}=true before it can resolve.", - reference=op.reserved_id, - ) - log.warning("agent: %s", error) - raise error target: dict = { "call_ref": op.to_call_ref(), "context_bindings": dict(op.context_bindings) or None, diff --git a/sdks/python/agenta/sdk/agents/utils/wire.py b/sdks/python/agenta/sdk/agents/utils/wire.py index d432441972..5a745e9990 100644 --- a/sdks/python/agenta/sdk/agents/utils/wire.py +++ b/sdks/python/agenta/sdk/agents/utils/wire.py @@ -116,8 +116,8 @@ def request_to_wire( that drops each entry into the cwd with no harness knowledge. ``run_context`` is the run's own context (trace + variant identity), refreshed per turn. When - set it rides as ``runContext`` and is consumed only by a tool's ``call.context`` binding at - dispatch (direct-call tools, Phase 3a). Omitted when unset (and when its ``to_wire`` is empty), + set it rides as ``runContext`` and is consumed by tool context bindings at dispatch + (``call.context`` on direct-call specs and ``contextBindings`` on callRef specs) (direct-call tools, Phase 3a). Omitted when unset (and when its ``to_wire`` is empty), so a run that needs no binding stays byte-identical to before. """ payload: Dict[str, Any] = { diff --git a/sdks/python/agenta/sdk/agents/wire_models.py b/sdks/python/agenta/sdk/agents/wire_models.py index b00abcfab2..6eb490ddda 100644 --- a/sdks/python/agenta/sdk/agents/wire_models.py +++ b/sdks/python/agenta/sdk/agents/wire_models.py @@ -196,11 +196,18 @@ class WireRunContextTrace(_WireModel): span_id: Optional[str] = None +class WireRunContextRun(_WireModel): + """The run's own identity inside ``runContext`` (mirrors ``RunContextRun``).""" + + kind: Optional[str] = None + + class WireRunContext(_WireModel): """The run's own context, delivered on ``/run`` and refreshed per turn (direct-call tools, Phase 3a; mirrors ``RunContext.to_wire``). - Consumed only by a tool's ``call.context`` binding at dispatch, server-side and hidden from + Consumed by tool context bindings at dispatch: ``call.context`` on direct-call specs and + ``contextBindings`` on callRef specs, server-side and hidden from the model. Unlike the rest of the wire, the INNER keys are snake_case (``workflow.variant.id`` / ``trace.trace_id``): they are the binding NAMESPACE a catalog entry's ``$ctx.`` token addresses, so they must match those tokens exactly rather @@ -208,6 +215,7 @@ class WireRunContext(_WireModel): the top-level camelCase ``sessionId`` field. The top-level field is still the camelCase ``runContext`` on the request.""" + run: Optional[WireRunContextRun] = None workflow: Optional[WireRunContextWorkflow] = None trace: Optional[WireRunContextTrace] = None diff --git a/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.claude.json b/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.claude.json index 7b93b96072..48ac69c51a 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.claude.json +++ b/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.claude.json @@ -10,6 +10,9 @@ "secrets": {"ANTHROPIC_API_KEY": "sk-ant"}, "context": null, "telemetry": null, + "runContext": { + "run": {"kind": "test"} + }, "tools": [], "customTools": [ { @@ -18,6 +21,8 @@ "inputSchema": {"type": "object", "properties": {}}, "callRef": "tools__composio__github__GET_THE_AUTHENTICATED_USER__github-tvn", "kind": "callback", + "contextBindings": {"target.workflow_variant_id": "$ctx.workflow.variant.id"}, + "timeoutMs": 120000, "readOnly": true } ], diff --git a/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.json b/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.json index 796287f9a4..d169cbd59a 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.json +++ b/sdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.json @@ -30,6 +30,7 @@ } }, "runContext": { + "run": {"kind": "test"}, "workflow": { "artifact": {"id": "wf_abc"}, "variant": {"id": "var_abc", "slug": "weather-agent"}, @@ -49,6 +50,8 @@ "inputSchema": {"type": "object", "properties": {}}, "callRef": "tools__composio__github__GET_THE_AUTHENTICATED_USER__github-tvn", "kind": "callback", + "contextBindings": {"target.workflow_variant_id": "$ctx.workflow.variant.id"}, + "timeoutMs": 120000, "readOnly": true }, { diff --git a/sdks/python/oss/tests/pytest/unit/agents/platform/test_op_catalog.py b/sdks/python/oss/tests/pytest/unit/agents/platform/test_op_catalog.py index 677990bcd3..406bb148e6 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/platform/test_op_catalog.py +++ b/sdks/python/oss/tests/pytest/unit/agents/platform/test_op_catalog.py @@ -13,6 +13,8 @@ from __future__ import annotations +import logging + import pytest from pydantic import ValidationError @@ -228,18 +230,9 @@ async def test_discover_tools_wire_carries_call_not_call_ref(connection): assert wire["call"]["path"] == "/api/tools/discover" -async def test_test_run_handler_call_ref_requires_platform_handlers_flag(connection): - with pytest.raises( - GatewayToolResolutionError, - match="AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS", - ): - await _resolver(connection).resolve([PlatformToolConfig(op="test_run")]) - - -async def test_test_run_emits_handler_call_ref_with_bindings_and_timeout( - connection, monkeypatch +async def test_test_run_emits_handler_call_ref_with_bindings_and_timeout_by_default( + connection, ): - monkeypatch.setenv("AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS", "true") resolution = await _resolver(connection).resolve( [PlatformToolConfig(op="test_run")] ) @@ -273,6 +266,39 @@ async def test_test_run_emits_handler_call_ref_with_bindings_and_timeout( assert "call" not in wire +@pytest.mark.parametrize( + "disabled_value", + ["false", "0", "f", "n", "no", "off", "disable", "disabled", " OFF "], +) +async def test_platform_handlers_flag_off_skips_handler_ops_and_keeps_endpoint_ops( + connection, monkeypatch, caplog, disabled_value +): + monkeypatch.setenv("AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS", disabled_value) + caplog.set_level(logging.WARNING) + + resolution = await _resolver(connection).resolve( + [ + PlatformToolConfig(op="test_run"), + PlatformToolConfig(op="discover_tools"), + ] + ) + + assert [spec.name for spec in resolution.tool_specs] == ["discover_tools"] + assert resolution.tool_callback.endpoint == "https://api.x/api/tools/call" + assert "skipping platform handler-mode op 'test_run'" in caplog.text + assert "AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS" in caplog.text + + +async def test_platform_handlers_empty_flag_uses_default_on(connection, monkeypatch): + monkeypatch.setenv("AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS", "") + + resolution = await _resolver(connection).resolve( + [PlatformToolConfig(op="test_run")] + ) + + assert [spec.name for spec in resolution.tool_specs] == ["test_run"] + + async def test_query_spans_emits_project_scoped_read_call(connection): # Project scoping comes from the caller credential on the endpoint; there is no target field # for the model to supply and no run-context binding to inject. diff --git a/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py b/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py index 1981057f88..39a23e56e2 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py +++ b/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py @@ -29,6 +29,7 @@ ResolvedConnection, RunContext, RunContextReference, + RunContextRun, RunContextTrace, RunContextWorkflow, SandboxPermission, @@ -81,6 +82,8 @@ "inputSchema": {"type": "object", "properties": {}}, "callRef": "tools__composio__github__GET_THE_AUTHENTICATED_USER__github-tvn", "kind": "callback", + "contextBindings": {"target.workflow_variant_id": "$ctx.workflow.variant.id"}, + "timeoutMs": 120000, "readOnly": True, } # A DIRECT-CALL tool (direct-call tools, Phase 1): a callback spec that carries a `call` @@ -145,6 +148,7 @@ def _pi_payload(): # drops the unset reference fields. The conversation id rides the top-level `session_id`, # not run context. run_context=RunContext( + run=RunContextRun(kind="test"), workflow=RunContextWorkflow( artifact=RunContextReference(id="wf_abc"), variant=RunContextReference(id="var_abc", slug="weather-agent"), @@ -181,6 +185,7 @@ def _claude_payload(): messages=[Message(role="user", content="hi")], secrets={"ANTHROPIC_API_KEY": "sk-ant"}, trace=None, + run_context=RunContext(run=RunContextRun(kind="test")), session_id=None, ) @@ -236,6 +241,11 @@ def test_request_to_wire_omits_skills_when_none(): def test_request_to_wire_pi_matches_golden(golden): payload = _pi_payload() assert payload == golden("run_request.pi_core.json") + # The callRef runner-only fields ride the wire for the executor, with camelCase names. + assert payload["customTools"][0]["contextBindings"] == { + "target.workflow_variant_id": "$ctx.workflow.variant.id" + } + assert payload["customTools"][0]["timeoutMs"] == 120000 # The Composio read-only hint rides the wire as camelCase `readOnly`. assert payload["customTools"][0]["readOnly"] is True # The direct-call tool rides the wire carrying its `call` descriptor and NO `callRef` @@ -256,6 +266,7 @@ def test_request_to_wire_pi_matches_golden(golden): # grouped into artifact / variant / revision references, and the unset reference fields dropped # by `to_wire`. The conversation id is NOT here — it rides the top-level `sessionId`. assert payload["runContext"] == { + "run": {"kind": "test"}, "workflow": { "artifact": {"id": "wf_abc"}, "variant": {"id": "var_abc", "slug": "weather-agent"}, @@ -370,8 +381,8 @@ def test_request_to_wire_omits_project_id_when_none(): def test_request_to_wire_claude_matches_golden(golden): payload = _claude_payload() assert payload == golden("run_request.claude.json") - # The claude payload threads no run context, so `runContext` is absent (the golden has none). - assert "runContext" not in payload + # Claude carries the child run identity so reserved handlers can reject recursive test runs. + assert payload["runContext"] == {"run": {"kind": "test"}} # No trace context threaded on this config: both role-separated keys are null (matching the # prior single `trace: null`), and the legacy `trace` key is gone. assert payload["context"] is None diff --git a/services/oss/src/agent/app.py b/services/oss/src/agent/app.py index e5e4aec42d..0eb48698a9 100644 --- a/services/oss/src/agent/app.py +++ b/services/oss/src/agent/app.py @@ -22,6 +22,8 @@ MissingProviderError, ModelRef, ResolvedConnection, + RunContext, + RunContextRun, RuntimeAuthContext, SandboxAgentBackend, SessionConfig, @@ -243,16 +245,22 @@ async def _agent( resolved_connection = await _resolve_session_connection(model_ref, ctx) secrets = resolved_connection.env + rc = run_context() + run_kind = (request.meta or {}).get("run_kind") + if isinstance(run_kind, str) and run_kind: + rc = rc or RunContext() + rc.run = RunContextRun(kind=run_kind) + session_config = SessionConfig( agent=agent_template, secrets=secrets, # the env compat alias the wire still reads resolved_connection=resolved_connection, permission_default=agent_template.permission_default, trace=trace_context(), - # The run's own context (trace + workflow identity), refreshed each turn and consumed only - # by a tool's `call.context` binding at dispatch (direct-call tools, Phase 3a). The - # conversation id is threaded separately as `session_id` below, not duplicated in here. - run_context=run_context(), + # The run's own context (run kind, trace, and workflow identity), refreshed each turn + # and consumed by tool context bindings at dispatch. The conversation id is threaded + # separately as `session_id` below, not duplicated in here. + run_context=rc, session_id=session_id, builtin_names=resolved_tools.builtin_names, tool_specs=resolved_tools.tool_specs, diff --git a/services/oss/src/agent/tracing.py b/services/oss/src/agent/tracing.py index 648571facd..bd00666b5d 100644 --- a/services/oss/src/agent/tracing.py +++ b/services/oss/src/agent/tracing.py @@ -173,14 +173,15 @@ def _run_context_trace() -> Optional[RunContextTrace]: def run_context() -> Optional[RunContext]: - """Capture the run's own context for tool ``call.context`` binding (direct-call tools, Phase 3a). + """Capture the run's own context for tool bindings and run-kind propagation. Assembles the run's own trace + workflow identity into a :class:`RunContext` the service sends - on ``/run`` (refreshed per turn). It is consumed ONLY by a tool's ``call.context`` binding at - dispatch, server-side and hidden from the model (see - ``projects/direct-call-tools/run-context.md``). The conversation id is not part of this blob — - it rides the top-level ``sessionId`` field. Best-effort: any failure (or an entirely empty - context) returns ``None`` so the run proceeds and the ``runContext`` key is simply omitted. + on ``/run`` (refreshed per turn). The full ``runContext`` is consumed by direct-call + ``call.context`` bindings, callRef ``contextBindings``, and ``run.kind`` forwarding at dispatch, + server-side and hidden from the model (see ``projects/direct-call-tools/run-context.md``). The + conversation id is not part of this blob; it rides the top-level ``sessionId`` field. + Best-effort: any failure (or an entirely empty context) returns ``None`` so the run proceeds and + the ``runContext`` key is simply omitted. The workflow and the trace are captured as INDEPENDENT failure domains: a failure reading the workflow references must not drop an otherwise-valid ``trace`` (and vice versa), so a trace-only diff --git a/services/oss/tests/pytest/unit/agent/test_run_kind.py b/services/oss/tests/pytest/unit/agent/test_run_kind.py new file mode 100644 index 0000000000..c66ff44ab0 --- /dev/null +++ b/services/oss/tests/pytest/unit/agent/test_run_kind.py @@ -0,0 +1,75 @@ +"""Run-kind plumbing for reserved platform handler recursion protection.""" + +from __future__ import annotations + +from agenta.sdk.agents import ( + AgentResult, + AgentTemplate, + ResolvedConnection, + ResolvedToolSet, + RunContext, + RunContextTrace, +) +from agenta.sdk.models.workflows import WorkflowServiceRequest + +from oss.src.agent import app + + +def _patch_agent(monkeypatch, backend, *, base_run_context=None): + async def _tools(tools, **_kw): + return ResolvedToolSet() + + async def _no_mcp(mcp_servers, **_kw): + return [] + + async def _no_connection(*, model, context): + return ResolvedConnection( + provider="openai", + model="m", + credential_mode="runtime_provided", + env={}, + ) + + monkeypatch.setattr(app, "resolve_tools", _tools) + monkeypatch.setattr(app, "resolve_mcp_servers", _no_mcp) + monkeypatch.setattr(app, "resolve_connection", _no_connection) + monkeypatch.setattr(app, "trace_context", lambda: None) + monkeypatch.setattr(app, "run_context", lambda: base_run_context) + monkeypatch.setattr(app, "record_usage", lambda usage: None) + monkeypatch.setattr(app, "select_backend", lambda selection: backend) + monkeypatch.setattr( + app, + "_default_agent_template", + lambda: AgentTemplate(instructions="x", model="m"), + ) + + +async def _invoke(meta=None): + return await app._agent( + request=WorkflowServiceRequest(meta=meta), + messages=[{"role": "user", "content": "hi"}], + parameters={"agent": {"harness": {"kind": "pi_core"}}}, + ) + + +async def test_meta_run_kind_reaches_run_context(monkeypatch, fake_backend): + backend = fake_backend(result=AgentResult(output="ok")) + _patch_agent(monkeypatch, backend) + + await _invoke(meta={"run_kind": "test"}) + + ctx = backend.created_run_contexts[0] + assert ctx is not None + assert ctx.to_wire() == {"run": {"kind": "test"}} + + +async def test_absent_run_kind_leaves_run_context_unchanged(monkeypatch, fake_backend): + backend = fake_backend(result=AgentResult(output="ok")) + base = RunContext(trace=RunContextTrace(trace_id="trace-1")) + _patch_agent(monkeypatch, backend, base_run_context=base) + + await _invoke(meta={}) + + ctx = backend.created_run_contexts[0] + assert ctx is base + assert ctx.to_wire() == {"trace": {"trace_id": "trace-1"}} diff --git a/services/runner/src/protocol.ts b/services/runner/src/protocol.ts index 3d189ba756..4b8c44e056 100644 --- a/services/runner/src/protocol.ts +++ b/services/runner/src/protocol.ts @@ -107,6 +107,14 @@ export interface ResolvedToolSpec { inputSchema?: Record | null; /** Set for gateway `callback` tools (routes through /tools/call); absent for `code` / `client`, and absent when `call` is set. */ callRef?: string; + /** + * Executor-private argument bindings for `callRef` tools. The runner fills each dotted argument + * path from `runContext` after the permission verdict and before posting to `/tools/call`, so + * the model cannot override the bound fields. Not advertised to the model. + */ + contextBindings?: Record; + /** Optional per-tool execution budget for the `/tools/call` round-trip and child relay wait. */ + timeoutMs?: number; /** * Direct-call descriptor (direct-call tools, Phase 1). When set, the runner calls this Agenta * endpoint DIRECTLY (reusing the run's `toolCallback.authorization`) instead of routing through @@ -156,9 +164,10 @@ export interface RunContextReference { /** * The run's own context, delivered on `/run` and refreshed per turn (direct-call tools, Phase 3a; * see `projects/direct-call-tools/run-context.md`). The service computes it from the invocation's - * own trace + workflow identity. It is consumed ONLY by a tool's `call.context` binding: the runner - * fills bound request fields from this blob at dispatch, server-side and hidden from the model. The - * model never reads run context directly. + * own trace + workflow identity. It is consumed by tool context bindings: `call.context` on + * direct-call specs and `contextBindings` on callRef specs. The runner fills bound request fields + * from this blob at dispatch, server-side and hidden from the model. The model never reads run + * context directly. * * `workflow` mirrors the platform's three workflow entities — the `artifact` (the workflow), the * `variant`, and the `revision` — so the run's identity reads the same way the rest of the platform @@ -172,6 +181,9 @@ export interface RunContextReference { * best-effort — the service fills what it holds and omits the rest. */ export interface RunContext { + run?: { + kind?: string; + }; workflow?: { artifact?: RunContextReference; variant?: RunContextReference; diff --git a/services/runner/src/tools/callback.ts b/services/runner/src/tools/callback.ts index 1b467f9e63..022347cf57 100644 --- a/services/runner/src/tools/callback.ts +++ b/services/runner/src/tools/callback.ts @@ -23,6 +23,21 @@ export const EMPTY_OBJECT_SCHEMA = { additionalProperties: true, }; +export interface CallAgentaToolOptions { + signal?: AbortSignal; + timeoutMs?: number; + runKind?: string; +} + +function callbackFetchTimeoutMs(timeoutMs: number | undefined): number { + // A positive spec timeout caps the server-side child run. The host fetch gets + // a short grace window so digest/span work produced after the child ceiling + // is not lost to an abort at the same deadline. + return timeoutMs && timeoutMs > 0 + ? timeoutMs + 10_000 + : TOOL_CALL_TIMEOUT_MS; +} + /** * One /tools/call round-trip. Returns the result text; throws on failure. Callers turn a * throw into a tool-error result so the model loop continues rather than crashing the run. @@ -34,15 +49,20 @@ export async function callAgentaTool( callRef: string, toolCallId: string, args: unknown, - signal?: AbortSignal, + options: CallAgentaToolOptions = {}, ): Promise { const headers: Record = { "content-type": "application/json" }; if (authorization) headers["authorization"] = authorization; + if (options.runKind) headers["x-agenta-run-kind"] = options.runKind; - const timeoutSignal = AbortSignal.timeout(TOOL_CALL_TIMEOUT_MS); + const timeoutSignal = AbortSignal.timeout( + callbackFetchTimeoutMs(options.timeoutMs), + ); const anyOf = (AbortSignal as any).any; const combined = - signal && typeof anyOf === "function" ? anyOf([signal, timeoutSignal]) : timeoutSignal; + options.signal && typeof anyOf === "function" + ? anyOf([options.signal, timeoutSignal]) + : timeoutSignal; const dbg = process.env.AGENTA_RUNNER_DEBUG_TOOLS ? console.error : undefined; dbg?.(`[tool-call] -> ${callRef} POST ${endpoint} auth=${authorization ? "yes" : "no"}`); diff --git a/services/runner/src/tools/direct.ts b/services/runner/src/tools/direct.ts index 0d07bd50dc..4936145663 100644 --- a/services/runner/src/tools/direct.ts +++ b/services/runner/src/tools/direct.ts @@ -238,6 +238,25 @@ export function assembleBody( return body; } +export function applyContextBindings( + args: unknown, + bindings: Record, + runContext?: RunContext, +): Record { + const body: Record = isPlainObject(args) + ? (structuredClone(args) as Record) + : {}; + for (const [argPath, token] of Object.entries(bindings)) { + deepDelete(body, argPath); + const value = resolveCtxToken(runContext, token); + if (value === undefined) { + throw new Error(`missing run-context value for tool binding '${argPath}'`); + } + deepSet(body, argPath, value); + } + return body; +} + /** * Validate the descriptor and build the absolute URL to call. The `call` is untrusted input, so * this is the SSRF guard, and it makes NO assumption about where the Agenta API is mounted: @@ -339,18 +358,19 @@ export async function callDirect( url: string, authorization: string | undefined, body: Record, - signal?: AbortSignal, + options: { signal?: AbortSignal; runKind?: string } = {}, ): Promise { const headers: Record = { "content-type": "application/json", }; if (authorization) headers["authorization"] = authorization; + if (options.runKind) headers["x-agenta-run-kind"] = options.runKind; const timeoutSignal = AbortSignal.timeout(TOOL_CALL_TIMEOUT_MS); const anyOf = (AbortSignal as any).any; const combined = - signal && typeof anyOf === "function" - ? anyOf([signal, timeoutSignal]) + options.signal && typeof anyOf === "function" + ? anyOf([options.signal, timeoutSignal]) : timeoutSignal; let response: Response; diff --git a/services/runner/src/tools/dispatch.ts b/services/runner/src/tools/dispatch.ts index 9a046cbd26..ff41211ff4 100644 --- a/services/runner/src/tools/dispatch.ts +++ b/services/runner/src/tools/dispatch.ts @@ -68,6 +68,7 @@ export async function relayToolCall( toolName: string, toolCallId: string, params: unknown, + timeoutMs?: number, signal?: AbortSignal, ): Promise { const id = sanitizeRelayId(toolCallId); @@ -84,7 +85,9 @@ export async function relayToolCall( "utf-8", ); - const deadline = Date.now() + RELAY_TIMEOUT_MS; + const deadline = + Date.now() + + (timeoutMs && timeoutMs > 0 ? timeoutMs + 10_000 : RELAY_TIMEOUT_MS); while (Date.now() < deadline) { if (signal?.aborted) throw new Error("aborted"); if (existsSync(resPath)) { @@ -240,6 +243,7 @@ export async function runResolvedTool( spec.name, opts.toolCallId, params, + spec.timeoutMs, opts.signal, ); } @@ -254,6 +258,7 @@ export async function runResolvedTool( spec.name, opts.toolCallId, params, + spec.timeoutMs, opts.signal, ); } @@ -263,6 +268,6 @@ export async function runResolvedTool( spec.callRef ?? "", opts.toolCallId, params, - opts.signal, + { signal: opts.signal, timeoutMs: spec.timeoutMs }, ); } diff --git a/services/runner/src/tools/public-spec.ts b/services/runner/src/tools/public-spec.ts index 372edb9040..b6c7a39022 100644 --- a/services/runner/src/tools/public-spec.ts +++ b/services/runner/src/tools/public-spec.ts @@ -2,8 +2,9 @@ * Public tool metadata safe to expose to harness child processes. * * ResolvedToolSpec also carries executor-private fields (`callRef`, `code`, scoped `env`, - * runtime). Those must stay in runner memory. Child processes only need the advertisement - * shape so the model can choose a tool; every execution is relayed back to the runner. + * runtime, contextBindings). Those must stay in runner memory. Child processes only need the + * advertisement shape so the model can choose a tool; every execution is relayed back to the + * runner. `timeoutMs` is public because the child relay wait needs the same per-tool budget. */ import type { ResolvedToolSpec } from "../protocol.ts"; import { specInputSchema } from "./spec-schema.ts"; @@ -14,6 +15,7 @@ export interface AdvertisedToolSpec { inputSchema?: Record | null; kind?: ResolvedToolSpec["kind"]; render?: ResolvedToolSpec["render"]; + timeoutMs?: ResolvedToolSpec["timeoutMs"]; } /** `client` tools are browser-fulfilled and are not executable by a runner child process. */ @@ -29,6 +31,7 @@ export function advertisedToolSpec(spec: ResolvedToolSpec): AdvertisedToolSpec { }; if (spec.kind) out.kind = spec.kind; if (spec.render) out.render = spec.render; + if (spec.timeoutMs !== undefined) out.timeoutMs = spec.timeoutMs; return out; } diff --git a/services/runner/src/tools/relay.ts b/services/runner/src/tools/relay.ts index e9e87eb39f..ac0db98774 100644 --- a/services/runner/src/tools/relay.ts +++ b/services/runner/src/tools/relay.ts @@ -26,6 +26,7 @@ import { import { callAgentaTool } from "./callback.ts"; import { runCodeTool } from "./code.ts"; import { + applyContextBindings, assembleBody, callDirect, deepDelete, @@ -252,10 +253,12 @@ async function executeRelayedTool( return permissionPolicyDenyReason(spec.name); } if (verdict.kind === "pendingApproval") { + // Pi file-relay approvals are recorded here. Claude's approval card is + // harness-rendered before this point, so this relay cannot redact it. permissions.onPendingApproval({ toolCallId: req.toolCallId, toolName: spec.name, - args: req.args, + args: pendingApprovalArgs(spec, req.args), }); return PAUSED; } @@ -291,15 +294,21 @@ async function executeAllowedRelayedTool( for (const name of pathParamNames(spec.call.path)) { deepDelete(body, name); } - return callDirect(spec.call.method, url, callback.authorization, body); + return callDirect(spec.call.method, url, callback.authorization, body, { + runKind: runContext?.run?.kind, + }); } // 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) + : req.args; return callAgentaTool( callback.endpoint, callback.authorization, spec.callRef ?? "", req.toolCallId, - req.args, + args, + { timeoutMs: spec.timeoutMs, runKind: runContext?.run?.kind }, ); } @@ -339,6 +348,47 @@ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function cloneJsonish(value: unknown): unknown { + if (Array.isArray(value)) return value.map((item) => cloneJsonish(item)); + if (!isRecord(value)) return value; + const out: Record = {}; + for (const [key, item] of Object.entries(value)) { + out[key] = cloneJsonish(item); + } + return out; +} + +function pruneEmptyAncestors(target: Record, path: string): void { + const parts = path.split("."); + const ancestors: Array<{ owner: Record; key: string }> = []; + let cursor = target; + for (const part of parts.slice(0, -1)) { + const next = cursor[part]; + if (!isRecord(next)) return; + ancestors.push({ owner: cursor, key: part }); + cursor = next; + } + for (const { owner, key } of ancestors.reverse()) { + const value = owner[key]; + if (!isRecord(value) || Object.keys(value).length > 0) return; + delete owner[key]; + } +} + +function pendingApprovalArgs( + spec: ResolvedToolSpec, + args: unknown, +): unknown { + if (!spec.callRef || !spec.contextBindings || !isRecord(args)) return args; + const displayArgs = cloneJsonish(args); + if (!isRecord(displayArgs)) return displayArgs; + for (const path of Object.keys(spec.contextBindings)) { + deepDelete(displayArgs, path); + pruneEmptyAncestors(displayArgs, path); + } + return displayArgs; +} + async function handlePermissionRelayRequest( req: Partial & { kind: "permission"; diff --git a/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts b/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts index 5db104210e..3bd08ac260 100644 --- a/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts +++ b/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts @@ -64,6 +64,8 @@ describe("buildPiExtensionEnv", () => { properties: { x: { type: "string" } }, }, callRef: "server-secret-ref", + contextBindings: { "target.workflow_variant_id": "$ctx.workflow.variant.id" }, + timeoutMs: 120000, env: { SECRET: "do-not-expose" }, kind: "callback", }, @@ -105,6 +107,7 @@ describe("buildPiExtensionEnv", () => { description: "safe", inputSchema: { type: "object", properties: { x: { type: "string" } } }, kind: "callback", + timeoutMs: 120000, }, { name: "client_only", @@ -118,6 +121,7 @@ describe("buildPiExtensionEnv", () => { }, ]); assert.equal(JSON.stringify(specs).includes("server-secret-ref"), false); + assert.equal(JSON.stringify(specs).includes("contextBindings"), false); assert.equal(JSON.stringify(specs).includes("do-not-expose"), false); }); diff --git a/services/runner/tests/unit/tool-callref-bindings.test.ts b/services/runner/tests/unit/tool-callref-bindings.test.ts new file mode 100644 index 0000000000..d51b4ebaf8 --- /dev/null +++ b/services/runner/tests/unit/tool-callref-bindings.test.ts @@ -0,0 +1,309 @@ +/** + * Unit tests for callRef callback execution through the host relay. + * + * These pin the test_run runner contract: contextBindings are applied only after the permission + * verdict and only in the callRef branch, timeoutMs reaches /tools/call, and runContext.run.kind + * is forwarded as x-agenta-run-kind. + */ +import { afterEach, describe, it } from "vitest"; +import assert from "node:assert/strict"; +import { + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import type { ResolvedToolSpec, RunContext } from "../../src/protocol.ts"; +import { + localRelayHost, + startToolRelay, + type RelayPermissions, + type RelayResponse, +} from "../../src/tools/relay.ts"; + +const ENDPOINT = "https://agenta.example/api/tools/call"; +const RUN_CONTEXT: RunContext = { + run: { kind: "test" }, + workflow: { variant: { id: "own-variant" } }, +}; + +interface CapturedFetch { + url: string; + init: RequestInit; +} + +type PendingInfo = { toolCallId: string; toolName: string; args: unknown }; + +const realFetch = globalThis.fetch; +const realTimeout = AbortSignal.timeout; + +afterEach(() => { + globalThis.fetch = realFetch; + AbortSignal.timeout = realTimeout; +}); + +function stubFetch(body = "ok"): CapturedFetch[] { + const calls: CapturedFetch[] = []; + globalThis.fetch = (async (url: any, init: any) => { + calls.push({ url: String(url), init: init ?? {} }); + return new Response(body, { status: 200 }); + }) as typeof fetch; + return calls; +} + +function permissions(input: { + verdict?: "allow" | "deny" | "pendingApproval"; + seenArgs?: unknown[]; + pending?: PendingInfo[]; +} = {}): RelayPermissions { + return { + enforce: true, + decide: (gate) => { + input.seenArgs?.push(gate.args); + if (input.verdict === "deny") return { kind: "deny" }; + if (input.verdict === "pendingApproval") { + return { kind: "pendingApproval" }; + } + return { kind: "allow" }; + }, + onPendingApproval: (info) => { + input.pending?.push(info); + return { emitted: true }; + }, + }; +} + +async function relayOnce(input: { + spec: ResolvedToolSpec; + args: unknown; + runContext?: RunContext; + permissions?: RelayPermissions; + expectResponse?: boolean; + stopWhen?: () => boolean; +}): Promise { + const dir = mkdtempSync(join(tmpdir(), "agenta-callref-relay-")); + try { + const id = "call-1"; + writeFileSync( + join(dir, `${id}.req.json`), + JSON.stringify({ toolName: input.spec.name, toolCallId: id, args: input.args }), + ); + const relay = startToolRelay( + localRelayHost(), + dir, + [input.spec], + { endpoint: ENDPOINT, authorization: "ApiKey secret" }, + input.permissions ?? permissions(), + input.runContext, + ); + const resPath = join(dir, `${id}.res.json`); + const deadline = Date.now() + 5000; + while (Date.now() < deadline && !existsSync(resPath)) { + if (input.stopWhen?.()) break; + await new Promise((resolve) => setTimeout(resolve, 20)); + } + await relay.stop(); + const wroteResponse = existsSync(resPath); + if (input.expectResponse === false) { + assert.equal(wroteResponse, false, "the relay did not write a response file"); + return undefined; + } + assert.ok(wroteResponse, "the relay wrote a response file"); + return JSON.parse(readFileSync(resPath, "utf-8")) as RelayResponse; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +function callRefSpec(overrides: Partial = {}): ResolvedToolSpec { + return { + name: "test_run", + kind: "callback", + callRef: "tools.agenta.test_run", + contextBindings: { + "target.workflow_variant_id": "$ctx.workflow.variant.id", + }, + timeoutMs: 120000, + ...overrides, + }; +} + +describe("startToolRelay callRef context bindings", () => { + it("applies bindings after the allow verdict and lets bindings override model args", async () => { + const calls = stubFetch(); + const seenArgs: unknown[] = []; + + const res = await relayOnce({ + spec: callRefSpec(), + args: { + target: { workflow_variant_id: "model-variant" }, + inputs: { city: "Berlin" }, + }, + runContext: RUN_CONTEXT, + permissions: permissions({ seenArgs }), + }); + + assert.equal(res?.ok, true); + assert.equal(calls.length, 1); + assert.deepEqual(seenArgs, [ + { + target: { workflow_variant_id: "model-variant" }, + inputs: { city: "Berlin" }, + }, + ]); + const posted = JSON.parse(calls[0].init.body as string); + assert.deepEqual(posted.data.function, { + name: "tools.agenta.test_run", + arguments: { + target: { workflow_variant_id: "own-variant" }, + inputs: { city: "Berlin" }, + }, + }); + }); + + it("fails closed when a binding token cannot be resolved, without fetching", async () => { + const calls = stubFetch(); + + const res = await relayOnce({ + spec: callRefSpec(), + args: { target: { workflow_variant_id: "model-variant" } }, + runContext: { run: { kind: "test" } }, + }); + + assert.equal(res?.ok, false); + assert.match( + res?.error ?? "", + /missing run-context value for tool binding 'target\.workflow_variant_id'/, + ); + assert.equal(calls.length, 0); + }); + + it("does not bind or fetch when the permission verdict denies", async () => { + const calls = stubFetch(); + + const res = await relayOnce({ + spec: callRefSpec({ permission: "deny" }), + args: { target: { workflow_variant_id: "model-variant" } }, + runContext: { run: { kind: "test" } }, + permissions: permissions({ verdict: "deny" }), + }); + + assert.equal(res?.ok, true); + assert.equal(res?.text, "Tool 'test_run' is denied by policy."); + assert.equal(calls.length, 0); + }); + + it("redacts bound args from pending display but still binds them on execution", async () => { + const calls = stubFetch(); + const pending: PendingInfo[] = []; + const args = { + target: { workflow_variant_id: "model-variant" }, + inputs: { city: "Berlin" }, + }; + + await relayOnce({ + spec: callRefSpec(), + args, + runContext: { run: { kind: "test" } }, + permissions: permissions({ verdict: "pendingApproval", pending }), + expectResponse: false, + stopWhen: () => pending.length === 1, + }); + + assert.deepEqual(pending, [ + { + toolCallId: "call-1", + toolName: "test_run", + args: { inputs: { city: "Berlin" } }, + }, + ]); + assert.equal(calls.length, 0); + + const res = await relayOnce({ + spec: callRefSpec(), + args, + runContext: RUN_CONTEXT, + }); + + assert.equal(res?.ok, true); + assert.equal(calls.length, 1); + const posted = JSON.parse(calls[0].init.body as string); + assert.deepEqual(posted.data.function.arguments, { + target: { workflow_variant_id: "own-variant" }, + inputs: { city: "Berlin" }, + }); + }); + + it("forwards run kind only when runContext.run.kind is set", async () => { + const calls = stubFetch(); + await relayOnce({ + spec: callRefSpec(), + args: {}, + runContext: RUN_CONTEXT, + }); + await relayOnce({ + spec: callRefSpec({ contextBindings: undefined }), + args: {}, + }); + + assert.equal(calls.length, 2); + assert.equal( + (calls[0].init.headers as Record)["x-agenta-run-kind"], + "test", + ); + assert.equal( + (calls[1].init.headers as Record)["x-agenta-run-kind"], + undefined, + ); + }); + + it("uses per-spec timeoutMs plus grace for callRef fetches and falls back to the global default", async () => { + stubFetch(); + const timeoutCalls: number[] = []; + AbortSignal.timeout = ((ms: number) => { + timeoutCalls.push(ms); + return new AbortController().signal; + }) as typeof AbortSignal.timeout; + + await relayOnce({ spec: callRefSpec(), args: {}, runContext: RUN_CONTEXT }); + await relayOnce({ + spec: callRefSpec({ contextBindings: undefined, timeoutMs: undefined }), + args: {}, + }); + + assert.deepEqual(timeoutCalls, [130000, 30000]); + }); + + it("ignores spec-level contextBindings on the direct call descriptor branch", async () => { + const calls = stubFetch("direct-ok"); + const spec = callRefSpec({ + callRef: undefined, + call: { method: "POST", path: "/api/tools/direct" }, + contextBindings: { + "target.workflow_variant_id": "$ctx.workflow.variant.id", + }, + }); + + const res = await relayOnce({ + spec, + args: { target: { workflow_variant_id: "model-variant" } }, + runContext: RUN_CONTEXT, + }); + + assert.equal(res?.ok, true); + assert.equal(res?.text, "direct-ok"); + assert.equal(calls.length, 1); + assert.equal(calls[0].url, "https://agenta.example/api/tools/direct"); + assert.equal( + (calls[0].init.headers as Record)["x-agenta-run-kind"], + "test", + ); + assert.deepEqual(JSON.parse(calls[0].init.body as string), { + target: { workflow_variant_id: "model-variant" }, + }); + }); +}); diff --git a/services/runner/tests/unit/tool-dispatch.test.ts b/services/runner/tests/unit/tool-dispatch.test.ts index 04025b0fff..334a56c4b0 100644 --- a/services/runner/tests/unit/tool-dispatch.test.ts +++ b/services/runner/tests/unit/tool-dispatch.test.ts @@ -12,9 +12,9 @@ * * Run: pnpm test (or: pnpm exec vitest run tests/unit/tool-dispatch.test.ts) */ -import { describe, it } from "vitest"; +import { afterEach, describe, it, vi } from "vitest"; import assert from "node:assert/strict"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -39,6 +39,13 @@ const codeSpec: ResolvedToolSpec = { code: 'def main(**kw):\n return {"echo": kw}\n', }; +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; + vi.useRealTimers(); +}); + describe("runResolvedTool", () => { it("fails code specs with a clear unsupported error", async () => { await assert.rejects( @@ -85,6 +92,51 @@ describe("runResolvedTool", () => { /missing required argument\(s\): integration/, ); }); + + + it("keeps callback specs on the file relay when relayDir is set", async () => { + const dir = mkdtempSync(join(tmpdir(), "agenta-relay-test-")); + const toolCallId = "call-callback"; + const reqPath = join(dir, sanitizeRelayId(toolCallId) + ".req.json"); + const resPath = join(dir, sanitizeRelayId(toolCallId) + RELAY_RES_SUFFIX); + let fetchCalls = 0; + globalThis.fetch = (async () => { + fetchCalls += 1; + throw new Error("direct /tools/call branch should be unreachable with relayDir"); + }) as typeof fetch; + try { + const promise = runResolvedTool( + { + name: "test_run", + kind: "callback", + callRef: "tools.agenta.test_run", + contextBindings: { + "target.workflow_variant_id": "$ctx.workflow.variant.id", + }, + timeoutMs: 120000, + }, + { target: { workflow_variant_id: "model-variant" } }, + { toolCallId, relayDir: dir, endpoint: "https://api.example/tools/call" }, + ); + + const deadline = Date.now() + 5000; + while (Date.now() < deadline && !existsSync(reqPath)) { + await new Promise((resolve) => setTimeout(resolve, 20)); + } + assert.ok(existsSync(reqPath), "relay request was written"); + assert.deepEqual(JSON.parse(readFileSync(reqPath, "utf-8")), { + toolName: "test_run", + toolCallId, + args: { target: { workflow_variant_id: "model-variant" } }, + }); + writeFileSync(resPath, JSON.stringify({ ok: true, text: "relayed" })); + + assert.equal(await promise, "relayed"); + assert.equal(fetchCalls, 0); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); // Directly exercises the Daytona file-relay path (the code site of the fixed `callRef` bug): @@ -120,4 +172,23 @@ describe("relayToolCall (Daytona file relay)", () => { rmSync(dir, { recursive: true, force: true }); } }); + + + it("extends the child relay deadline with timeoutMs", async () => { + vi.useFakeTimers(); + const dir = mkdtempSync(join(tmpdir(), "agenta-relay-test-")); + try { + const toolCallId = "call-slow"; + const resPath = join(dir, sanitizeRelayId(toolCallId) + RELAY_RES_SUFFIX); + const promise = relayToolCall(dir, "slowTool", toolCallId, {}, 120000); + setTimeout(() => { + writeFileSync(resPath, JSON.stringify({ ok: true, text: "slow-ok" })); + }, 65_000); + + await vi.advanceTimersByTimeAsync(65_500); + assert.equal(await promise, "slow-ok"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/services/runner/tests/unit/wire-contract.test.ts b/services/runner/tests/unit/wire-contract.test.ts index a4893501fa..c9700b45e2 100644 --- a/services/runner/tests/unit/wire-contract.test.ts +++ b/services/runner/tests/unit/wire-contract.test.ts @@ -96,6 +96,10 @@ describe("wire contract: requests (vs Python golden)", () => { tool.callRef && tool.callRef.length > 0, "callback tool carries its callRef", ); + assert.deepEqual(tool.contextBindings, { + "target.workflow_variant_id": "$ctx.workflow.variant.id", + }); + assert.equal(tool.timeoutMs, 120000); // The Composio read-only hint reaches the runner as `readOnly`. assert.equal(tool.readOnly, true); // No explicit author permission is derived onto the tool spec; the plan decides it. @@ -117,6 +121,7 @@ describe("wire contract: requests (vs Python golden)", () => { // snake_case inner keys (the `$ctx.` binding namespace) and the workflow grouped into the // platform's artifact / variant / revision entities. The runner fills a tool's `call.context` // from this blob at dispatch (see tools/direct.ts `assembleBody`); the model never reads it. + assert.equal(req.runContext!.run!.kind, "test"); assert.equal(req.runContext!.workflow!.variant!.id, "var_abc"); assert.equal(req.runContext!.workflow!.variant!.slug, "weather-agent"); assert.equal(req.runContext!.workflow!.revision!.id, "rev_abc123"); @@ -185,7 +190,7 @@ describe("wire contract: requests (vs Python golden)", () => { }); assert.equal(req.systemPrompt, undefined); // Claude exposes no prompt overrides assert.equal(req.appendSystemPrompt, undefined); - assert.equal(req.runContext, undefined); // no run context threaded on this config + assert.equal(req.runContext!.run!.kind, "test"); assert.equal(req.sandboxPermission, undefined); // no boundary declared on this config // The Claude harness's permission knobs are translated to a rendered file in Python: the // wire carries a generic `harnessFiles` entry the runner writes blind into the cwd.