From 0db3f6ddc546daa2a185c189d085f4c26b0a8edb Mon Sep 17 00:00:00 2001 From: ashrafchowdury Date: Mon, 20 Jul 2026 17:42:37 +0600 Subject: [PATCH 1/5] Add planning documents for progressive tool disclosure project --- .../progressive-tool-disclosure/README.md | 42 +++++++ .../progressive-tool-disclosure/context.md | 75 +++++++++++++ .../progressive-tool-disclosure/design.md | 101 +++++++++++++++++ .../progressive-tool-disclosure/plan.md | 77 +++++++++++++ .../progressive-tool-disclosure/research.md | 104 ++++++++++++++++++ .../progressive-tool-disclosure/status.md | 44 ++++++++ 6 files changed, 443 insertions(+) create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md new file mode 100644 index 0000000000..0bdcf042f6 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md @@ -0,0 +1,42 @@ +# Progressive tool disclosure (playground build kit) + +Status: PLANNING — design workspace only, no implementation. +Date: 2026-07-20 + +The playground advertises ~13 platform-op tool schemas to the model on every turn, before the +model has done anything — ~15K prompt tokens on a bare "hi", plus every extra always-on tool is +a "wander target" that derails runs. Skills already avoid this (on disk, only name+description +in the prompt, body loaded on demand). This project applies the same idea to the platform ops: +advertise a small **discovery meta-toolset** and load a full op schema only when the model asks. +The lazy layer lives at the runner's `advertisedToolSpecs()` seam, so it is harness-agnostic +(Pi + Claude) and touches no committed agent config. + +## Decisions (locked) + +- **Seam = runner advertisement layer.** Intercept the advertised projection at + `services/runner/src/tools/public-spec.ts` (`advertisedToolSpecs`), consumed at exactly two + sites (`pi-assets.ts`, `environment.ts`). Execution + permission stay below it, unchanged. +- **Skills are out of scope.** Already progressive. +- **Op-catalog contents / overlay-cleanup is out of scope.** This work makes *any* op set cheap + to carry; it does not decide which ops belong. +- **Playground overlay only.** No change to any saved/committed agent. +- **Schema diet is complementary, not exclusive.** Shrinking the embedded agent-template schema + is worth doing regardless and lands as its own slice. +- **No commits during planning.** Implementation happens later on its own branch. + +## Deliverables + +- [context.md](context.md) — problem, scope, non-goals, product language, success criteria. +- [research.md](research.md) — how the current advertise/execute path works, with `file:line`, + and the numbered seams the plan pins. +- [design.md](design.md) — the meta-toolset, the execution + permission path, alternatives. +- [plan.md](plan.md) — the sliced implementation plan, each slice with an exit check. +- [status.md](status.md) — living source of truth: locked decisions, open questions, next action. + +## Intended outcome + +A playground author opens an agent and types a message. The model sees two small platform tools +(`agenta_ops` to list what it can do, `agenta_op` to fetch a schema and act) instead of a wall +of op schemas. A no-op turn costs a low-hundreds token constant instead of ~15K, and stays flat +as the catalog grows. Every op still runs with its exact self-targeting binding and approval +gate. Builds are cheaper and, per the internal-tools review, more reliable. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md new file mode 100644 index 0000000000..a6fc3ef459 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md @@ -0,0 +1,75 @@ +# Context + +## Problem + +Open a playground agent that shows "Tools: None" and type "hi". The turn costs ~15K prompt +tokens. Nothing the author did explains it: the cost is the playground **build kit**, which +injects ~13 platform-op tool schemas into the agent template and advertises every one of them to +the model on every turn. + +Two costs, not one: + +- **Tokens.** Measured (tiktoken `o200k_base`, 2026-07-17): the ops dominate. `test_run` (~6.5K) + and `commit_revision` (~5.8K) each embed the full ~5.5K-token agent-template delta schema; + `query_spans` ~1.3K; all ops together ~15.4K. Skills are NOT the cost (a ~68-token + announcement only). This is a ~5x tax on every turn, paid before the model is useful. +- **Reliability.** The internal-tools review + (`../builder-agent-reliability/tools-review/part-2-internal-tools.md`) found the same tools are + a *double* cost: "each unused tool is context cost plus a wander target (the capstone showed + extra visible tools derail runs)." Fewer advertised tools is a correctness win, not just a bill. + +It scales the wrong way: adding a catalog op is a one-line data change that ships to every +playground agent unconditionally, so every op we add makes every turn heavier. + +Skills already solved this exact shape — on disk, only name+description in the prompt, body +loaded on demand. The platform ops never got the same treatment. + +## Scope (this delivery) + +- The **runner advertisement layer** for playground platform ops: `advertisedToolSpecs()` in + `services/runner/src/tools/public-spec.ts`, consumed by the Pi and Claude delivery paths. +- A **discovery meta-toolset** (`agenta_ops` + `agenta_op`) that replaces the always-advertised + op schemas: list ops cheaply, fetch one schema on demand, execute against the private spec. +- A **schema diet** for the two heaviest op schemas (`commit_revision`, `test_run`) — + independently valuable, folded in as its own slice. +- A **token + reliability baseline** so before/after is measured, not asserted. +- A **one-line nudge** in the always-loaded `build-an-agent` skill so the model uses the + meta-toolset. + +## Out of scope for the first delivery + +- **Skills.** Already progressive; untouched. +- **Which ops belong in the overlay** (the build-kit-tools-cleanup debate). Orthogonal — this + work makes any op set cheap, which lowers the pressure to prune. +- **External tool discovery (`discover_tools`).** That discovers *Composio* tools to wire into an + agent; it stays as-is and is itself one of the ops we disclose. +- **User / gateway / code / client tools.** The POC targets platform ops (the measured cost). The + mechanism can generalize later. +- **Committed non-playground agents.** They advertise only what their author declared; no problem + today. +- **Dynamic real-name re-advertisement (M2).** Advertising a loaded op under its real name with a + schema-validated signature is a productionization option, evaluated after the POC — not built + here. + +## Product language + +- **Platform op** — an existing Agenta endpoint exposed to the agent as a tool, defined in the + code catalog `op_catalog.py` (e.g. `commit_revision`, `query_spans`). +- **Advertised spec** — the `{name, description, inputSchema, …}` projection the model sees; + distinct from the **private resolved spec** the runner executes from. +- **Discovery meta-toolset** — the small fixed set (`agenta_ops`, `agenta_op`) that stands in for + the op schemas: list, describe-on-demand, invoke. +- **Disclosure** — moving an op's full schema out of the prompt (paid every turn) into a tool + result (paid once, only when fetched). + +## Success criteria + +1. A no-op turn's platform-op prompt cost drops from ~15K to a low-hundreds constant, and stays + flat as ops are added to the catalog. +2. Capability parity: the build-an-agent lab loop (discover → wire → commit → test → schedule) + passes with the disclosed toolset. +3. No safety regression: self-targeting `$ctx` bindings and per-op approval/permission behave + exactly as today, verified per mutating op. +4. Reliability does not regress (target: fewer "wander" failures) on the lab matrix. +5. Cost of laziness is bounded: ≤1 extra round-trip per distinct op used; a schema fetched at + most once per op per conversation. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md new file mode 100644 index 0000000000..be3ca67de2 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md @@ -0,0 +1,101 @@ +# Design — the discovery meta-toolset + +## The core move + +Stop advertising N full op schemas every turn. Advertise a small fixed meta-toolset; keep the op +specs resolved but **private** in runner memory; move each op's full schema from the *prompt* +(paid every turn) to a *tool result* (paid once, only for ops actually used). + +Two model-facing tools (names illustrative): + +- **`agenta_ops(query?)`** — returns the op catalog as a compact list: `{op, one_line, + read_only}` per op, no input schemas. Optional `query` filters. A few hundred tokens, flat, + regardless of catalog size. This is the "what can I do on the platform" index. +- **`agenta_op(op, args?)`** — the generic invoker. With no `args` (or `mode:"describe"`) it + returns **one** op's full `inputSchema` as a tool result; with `args` it executes that op. + +The 13 op specs stay in `plan.toolSpecs` and in `toolSpecsByName`. Only the *advertisement* is +replaced. This is the pattern Claude Code uses on its own ~200 tools: names are listed, and a +`ToolSearch` step loads a schema only when needed. + +## Why this is safe by construction + +Execution and permission read the **private** spec (research seams 2–4). The invoker feeds the +target op's private `call` descriptor into the *unchanged* `direct.ts` path, so: + +- **Self-targeting is preserved.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so + `commit_revision` still binds `$ctx.workflow.variant.id` and the model cannot retarget another + variant. The invoker never sees or forwards those fields — they were stripped at resolve time + and are re-applied below the invoker. +- **SSRF guard is preserved.** `directCallUrl` host-locks to the run's Agenta origin. +- **Approval is preserved — if the invoker gates per-op.** This is the one piece that is designed + in, not free (see below). + +## Execution path for `agenta_op(op, args)` + +The runner special-cases the invoker in its dispatch: + +1. **Resolve the target.** Look up `op` in `toolSpecsByName`. Unknown → tool error listing valid + ops (recoverable). +2. **Describe mode** (no `args` / `mode:"describe"`) → return the target spec's + `resolved input schema` as the tool result. No side effects, no approval. +3. **Execute mode** → build the permission **gate from the target op's spec** and run + `decide(gate, plan, stored)` (`permission-plan.ts:138`). `allow` runs; `deny` refuses; + `ask`/undecided pauses the turn and emits the normal `interaction_request(user_approval)` — + identical to calling the op directly today. Then run the target's `call` through + `assembleBody` → `directCallUrl` → `callDirect`, exactly as `executeRelayedTool` does now. +4. **Return** the endpoint response verbatim. + +The invoker is one ordinary advertised tool on both delivery paths (Pi native, Claude MCP), so no +harness-specific advertisement logic is needed. + +## Identifying the disclosure-eligible set + +A platform op is a `callback`-kind spec with a direct `call`; so is a `reference` (workflow) +tool. There is no explicit marker today (research seam 5). Two options: + +- **Heuristic (zero wire change).** Collapse every direct-`call` callback spec into the + meta-toolset; leave builtins, `client`, `code`, `gateway` (callRef), and MCP advertised as-is. + The playground overlay only injects platform ops (+ the two client tools), so this covers the + measured cost. Risk: an author who added a `reference` tool would see it disclosed too. Fine for + a flagged POC. +- **Marker (small wire add).** The platform resolver stamps a `source:"platform"` (or a + `disclosable` group tag) on the resolved spec; the runner collapses exactly that group. Precise, + and lets us disclose gateway/reference later on purpose. Costs a `protocol.ts` + `wire.py` + + golden change. + +Recommendation: heuristic for the POC to prove the numbers, marker before default-on. + +## The catalog summary (`agenta_ops`) + +Build `{op, one_line, read_only}` runner-side from the resolved specs already in memory — the +`description` and `read_only`/`permission` fields ride along; only `inputSchema` is dropped. Zero +new wire fields. (Alternative: thread a summary list from `op_catalog.py`; rejected for the POC as +extra plumbing.) + +## Discoverability + +The always-loaded `build-an-agent` skill (~68 tokens) gains one line: "platform actions are listed +by `agenta_ops`; fetch a schema with `agenta_op(op)` before calling it with args." Mirrors how the +skill already routes the builder; negligible always-on weight. + +## Alternatives considered + +- **M2 — dynamic real-name advertisement.** Advertise names only; a `load_op` call registers the + real op spec into the harness registry mid-session (Pi extension re-register; Claude MCP + `tools/list_changed`) so the model calls the op by its real name with a schema-validated + signature and native per-op permission. Highest fidelity, but needs mid-session re-registration + on both harnesses and, under the cold-replay runtime, reconstruction of the "loaded" set each + turn. Deferred to productionization. +- **Schema diet only.** Keep all ops advertised; replace the embedded ~5.5K agent-template delta + schema in `commit_revision`/`test_run` with an open object + a pointer to the skill's + `references/config-schema.md`. ~11K of ~15K is those two schemas, so this alone is a large, + near-zero-risk win. Complementary — it also shrinks each describe-mode fetch under the invoker. +- **Mode-gating.** Drop the build kit in Chat mode, keep it in Build mode. Removes capability + rather than deferring it; can layer on top, not a substitute. + +## Cost of laziness + +One extra round-trip per *distinct* op used (describe → then execute), paid only for ops the run +actually touches, and cacheable to once per op per conversation. A no-op turn pays nothing beyond +the two meta-tools. Acceptable for a builder flow; measured in Slice 3. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md new file mode 100644 index 0000000000..a4888c20a0 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md @@ -0,0 +1,77 @@ +# Plan — sliced implementation + +## Scope now + +- Behind a flag/env (default OFF) so the disclosed path A/B's against today's always-advertise + path before it becomes default. +- Playground platform ops only; no saved-agent change; no committed-agent behavior change. +- The schema diet (Slice 1) is independently shippable and lands first. +- Each slice leaves the tree working and testable. Order: baseline → diet → mechanism → measure. + +## Slice 0 — Baseline (pin the real cost) + +1. Write a measurement script (tiktoken `o200k_base`) that resolves the default build-kit overlay + and reports per-op advertised token cost + total, using the same `advertisedToolSpecs` + projection the runner ships (or a faithful mirror). +2. Confirm which ops actually advertise live today — resolve with `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` + off, and record whether `test_run` is in the advertised set. +3. Add a runner unit test asserting the current behavior: every resolved platform op appears in + `advertisedToolSpecs(plan.toolSpecs)`. This is the invariant later slices intentionally flip. +4. Check a `baseline.md` table into this folder (before-numbers, per op). + +**Exit:** `baseline.md` exists with per-op numbers; the "all platform ops advertised today" test +passes on `main`. + +## Slice 1 — Schema diet (large, low-risk win, independent) + +1. In `op_catalog.py`, replace the embedded agent-template delta schema in `commit_revision` + (and `test_run`) with an open object (`{"type":"object"}`) plus a description pointing at the + skill's `references/config-schema.md`. +2. Update the platform-op / wire contract tests that pin those schemas. +3. Re-run the Slice 0 script; record the drop (expect ~11K). +4. Lab check: an agent can still `commit_revision` with a valid config (the model has the shape + from the skill reference, and the server still validates). + +**Exit:** measured total drops by ~11K; contract tests green; a lab run commits a revision +successfully. + +## Slice 2 — Disclosure mechanism (the POC), flagged + +1. Add the flag/env (default off) that turns disclosure on for a run. +2. Introduce a disclosure transform applied to the advertised set at the two call sites + (`pi-assets.ts:353`, `environment.ts:721`): when on, replace the disclosure-eligible specs + (see design "Identifying the disclosure-eligible set" — heuristic for the POC) with the two + meta-tools `agenta_ops` + `agenta_op`; keep client tools and everything else advertised. + `plan.toolSpecs` / `toolSpecsByName` stay complete. +3. Implement the invoker dispatch: `agenta_ops` returns `{op, one_line, read_only}` built + runner-side from the resolved specs; `agenta_op` describe-mode returns one op's input schema; + `agenta_op` execute-mode builds the gate from the TARGET op's spec, runs `decide()`, then + `assembleBody`/`directCallUrl`/`callDirect` (reuse the `executeRelayedTool` core). +4. Unit tests: (a) with the flag on, only the meta-tools + client tools are advertised; (b) the + full private specs remain in `toolSpecsByName`; (c) per mutating op (`commit_revision`, + `create_schedule`, `remove_*`), `agenta_op` execute-mode produces the SAME approval verdict as + a direct call — no approval regression; (d) `$ctx` binding still fills server-side (the model + cannot retarget); (e) describe-mode has no side effect and no approval. +5. Add the one-line nudge to the `build-an-agent` skill. + +**Exit:** flag on → a lab run completes discover → wire → commit → schedule using only the +meta-tools; every mutating-op approval test passes; `tsc` + `pnpm test` green in `services/runner`. + +## Slice 3 — Measure, decide default, scope M2 + +1. Re-run the Slice 0 script with the flag on; record the no-op turn cost (target: low hundreds). +2. Run the build-an-agent lab / release gate with the flag on vs off; compare pass rate and + "wander" failures. +3. Write a `results.md` comparison (tokens + reliability, before/after). +4. Decide: flip default on? And is M2 (dynamic real-name advertisement) worth it for the ops that + most benefit from schema-validated calls? + +**Exit:** `results.md` checked in with the before/after; a go/no-go recommendation on +default-on and on M2. + +## Not in this plan + +- Marker-based eligibility (Slice 2 uses the heuristic); adding a `source:"platform"` wire marker + is a follow-up if we default-on for gateway/reference too. +- Disclosing gateway/code/client/MCP tools. +- M2 dynamic advertisement implementation. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md new file mode 100644 index 0000000000..c764f51fed --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md @@ -0,0 +1,104 @@ +# Research — how tools reach the model today + +Verified 2026-07-20 against the code cited. Paths are current (`services/runner/`; some docs +still say `services/agent/`). + +## The path a platform op takes, end to end + +1. **Overlay injects the op set (Python).** The playground build kit is a per-run overlay, not a + committed field. `build_agent_template_overlay()` (`api/oss/src/core/workflows/build_kit.py`) + emits `DEFAULT_BUILD_KIT_OPS` — ~13 platform ops (`discover_tools`, `commit_revision`, + `annotate_trace`, `query_spans`, `test_run`, `discover_triggers`, `create_schedule`, + `create_subscription`, `list_schedules`, `list_deliveries`, `test_subscription`, + `remove_schedule`, `remove_subscription`) plus the `request_connection` / `request_input` + client tools and the `build-an-agent` skill. It is served as the static workflow + `__ag__build_kit` and also delivered as read-only `additional_context` at + `api/oss/src/apis/fastapi/applications/router.py` (~L1916). The frontend merges it per run + (atom `workflowBuildKitOverlayReadyAtomFamily`, on by default via + `web/oss/src/lib/helpers/dynamicEnv.ts`). + +2. **Resolution (Python SDK).** `resolve_tools` + (`sdks/python/agenta/sdk/agents/platform/resolve.py` → `tools/resolver.py`) turns each + declared op into a `CallbackToolSpec` carrying a direct `call{method,path,context,args_into}`. + The op's `context_bindings` (self-targeting fields) are stripped from the model-visible schema + by `PlatformOp.resolved_input_schema()` (`.../platform/op_catalog.py`) and re-emitted as + `call.context`. No HTTP round-trip; the catalog fully describes the op. The resolved set rides + the `/run` wire as `customTools`. + +3. **Runner holds every spec privately.** `buildRunPlan` sets `plan.toolSpecs = request.customTools` + (`services/runner/src/engines/sandbox_agent/run-plan.ts:352`). The runner indexes them by name + with `toolSpecsByName(specs)` (`services/runner/src/tools/public-spec.ts:34`) — the ONE index + the relay execute loop, the internal tool-MCP server, and the ACP approval gate all key on. + +4. **Advertisement is a SEPARATE projection.** `advertisedToolSpecs(specs)` + (`public-spec.ts:57`) maps each spec to `{name, description, inputSchema, kind, render, + timeoutMs}` (`AdvertisedToolSpec`, `public-spec.ts:12`). `inputSchema` is the token weight. + It is called at exactly two sites: + - `services/runner/src/engines/sandbox_agent/pi-assets.ts:353` — Pi path; the specs become + `AGENTA_TOOL_PUBLIC_SPECS`, which the bundled extension (`extensions/agenta.ts`) reads and + registers as native Pi tools. + - `services/runner/src/engines/sandbox_agent/environment.ts:721` — Claude/ACP path; the specs + back the synthetic `agenta-tools` MCP server (`tools/mcp-bridge.ts`, `tool-mcp-http.ts`). + +5. **Execution reads the PRIVATE spec, never the advertisement.** For a platform op (direct + `call`), the relay path runs `executeRelayedTool` (`services/runner/src/tools/relay.ts:318`): + `assembleBody(spec.call, args, runContext)` (`relay.ts:384`) merges model args → static body → + `$ctx` context bindings (last, so a bound field always wins); `directCallUrl(...)` + (`relay.ts:385`, guard in `tools/direct.ts:286`) host-locks to the run's own Agenta origin and + confines to the `/api` mount; `callDirect(...)` (`relay.ts:392`) sends with the caller + credential. All keyed by the private `spec` — advertisement is irrelevant to execution. + +6. **Permission keys on the per-spec gate.** `decide(gate, plan, stored)` + (`services/runner/src/permission-plan.ts:138`) → `effectivePermission` (`:125`) resolves the + spec's own `permission` first, then rule match, then policy default (`allow_reads` → + read-only op runs, write asks). The gate is built from the resolved op's spec, so approval + fidelity lives with the private spec, not the advertised name. + +## Advertisement consumers + +| Harness | Delivery | Consumes `advertisedToolSpecs` at | Notes | +| --- | --- | --- | --- | +| Pi (`pi_core`/`pi_agenta`) | native, via bundled extension | `pi-assets.ts:353` → `AGENTA_TOOL_PUBLIC_SPECS` | No MCP server attached to Pi. | +| Claude (`claude`) | synthetic `agenta-tools` MCP server | `environment.ts:721` | Public metadata only; execution relays back. | + +Both paths consume the same projection function. Intercepting it once (or wrapping it at both +call sites behind a flag) covers both harnesses with no harness-specific logic. + +## Measured token cost (needs Slice 0 re-baseline) + +From the 2026-07-17 investigation (tiktoken `o200k_base`); flagged for re-measure because +`test_run` is handler-gated (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off) and may not +advertise live. + +| Op | Tokens | Note | +| --- | --- | --- | +| `test_run` | ~6,521 | embeds the ~5,462-token agent-template delta schema; handler-gated | +| `commit_revision` | ~5,844 | embeds the same ~5,462-token delta schema | +| `query_spans` | ~1,283 | filtering DSL `$defs` | +| others (10) | remainder | — | +| **all ~13 ops** | **~15,454** | the "hi" cost | + +Two schemas (`commit_revision` + `test_run`) account for ~11K of the ~15K — hence the schema-diet +slice is a large, low-risk win on its own. + +## Seams the plan must pin + +1. **Advertisement projection** (`advertisedToolSpecs`, two call sites). Where disclosure hooks. + Must not alter what execution/permission read. +2. **Private spec index** (`toolSpecsByName`). Must stay COMPLETE — the invoker looks the target + op up here; execution and the approval gate depend on it. +3. **Direct-call execution** (`relay.ts:318` → `direct.ts`). The invoker must reach this + unchanged, feeding the target op's private `call`. +4. **Permission decision** (`decide`/`effectivePermission`). The invoker must build the gate from + the TARGET op's spec, not from itself — otherwise `agenta_op` gates as one tool and writes + lose their approval prompt. +5. **Identifying disclosure-eligible specs.** A platform op is a `callback`-kind spec with a + direct `call`. So is a `reference` (workflow) tool (`direct.ts` header). There is no explicit + "this is a platform op" marker on `ResolvedToolSpec` today, so the runner cannot cleanly tell a + build-kit op from an author's reference tool without one. Two ways out (Open Question 4): + a heuristic (collapse all direct-`call` callback specs), or a small marker added by the platform + resolver (a wire add). +6. **Wire contract mirroring.** If Seam 5 uses a marker, `protocol.ts` + `wire.py` + goldens + change together (`services/runner/CLAUDE.md`, "The wire contract is mirrored"). +7. **Client tools stay advertised.** `request_connection` / `request_input` must remain + model-visible (the browser fulfils them) and are cheap; disclosure skips them. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md new file mode 100644 index 0000000000..056f08337e --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md @@ -0,0 +1,44 @@ +# Status + +**Last updated:** 2026-07-20 + +## Current stage + +Planning workspace drafted (context, research, design, plan). No code written, nothing +committed. Ready for team review of the open implementation questions below. Research is +grounded in the current runner code (`file:line` in research.md); the token numbers are from the +2026-07-17 investigation and are explicitly flagged for a Slice 0 re-baseline. + +## Locked decisions + +- Seam = the runner advertisement layer (`advertisedToolSpecs`, `services/runner/src/tools/public-spec.ts`), + consumed at `pi-assets.ts:353` and `environment.ts:721`. Execution + permission stay below it. +- Approach = a discovery meta-toolset (`agenta_ops` + `agenta_op`) that keeps op specs private in + runner memory and moves schemas from the prompt into on-demand tool results. +- Skills, op-catalog contents, external `discover_tools`, and non-platform tool types are out of + scope. +- Playground overlay only — no saved/committed-agent change. +- Schema diet is complementary and lands as its own slice (Slice 1). +- Ship behind a flag, default off, until measured. +- No commits during planning; implementation is a later, separate branch. + +## Open implementation questions + +1. **Invoker shape** — one `agenta_op` with describe-vs-execute mode, or two tools + (`describe_op` + `call_op`)? *Recommendation: one tool* (smaller advertised surface; the mode + is clear from presence of `args` or an explicit `mode`). +2. **Permission fidelity** — confirm `agenta_op` execute-mode builds the gate from the TARGET + op's private spec and runs the existing `decide()`, rather than gating the invoker as a single + tool. *Recommendation: yes, per-op* — otherwise `commit_revision` and other writes lose their + approval prompt. Non-negotiable in the reviewer's eyes IMO, but calling it out explicitly. +3. **Catalog summary source** — build `{op, one_line, read_only}` runner-side from the resolved + specs (zero new wire fields) vs. thread it from `op_catalog.py`. *Recommendation: runner-side.* +4. **Disclosure-eligible identification** — heuristic (collapse all direct-`call` callback specs) + vs. a `source:"platform"` marker on `ResolvedToolSpec` (a `protocol.ts` + `wire.py` + golden + add). *Recommendation: heuristic for the flagged POC; marker before default-on.* +5. **Sequencing** — do Slice 1 (schema diet) and ship it independently first, or hold it and land + the whole thing together? *Recommendation: ship Slice 1 first* — it is ~11K of the win at + near-zero risk and is useful even if the POC needs iteration. +6. **Default-on criteria** — what pass-rate / token target gates flipping the flag (Slice 3)? + Needs a number from the team (e.g. "no regression on the lab matrix and ≥90% token cut on a + no-op turn"). \ No newline at end of file From 4373db7434cc014ac8a927500f56f52542633056 Mon Sep 17 00:00:00 2001 From: ashrafchowdury Date: Sun, 26 Jul 2026 23:11:14 +0600 Subject: [PATCH 2/5] Refactor progressive tool disclosure plan and documentation - Revised the implementation plan for the progressive tool disclosure project, emphasizing the schema diet as the primary focus and deferring the meta-toolset. - Updated the measured token costs in baseline.md, reflecting a re-baseline on 2026-07-26, with detailed per-op costs and implications for slicing. - Added security.md to outline the risks associated with collapsing multiple ops into a single tool name, detailing permission decision impacts and required test coverage. - Enhanced research.md with insights on token costs and the necessity for a re-measurement of the live advertised set. - Created baseline.md to document the measured advertised token costs and the rationale behind the schema diet's low-risk profile. - Updated status.md to reflect the latest changes and decisions made on 2026-07-26, including corrections to earlier drafts and open questions for future consideration. --- .../progressive-tool-disclosure/README.md | 67 +++--- .../progressive-tool-disclosure/baseline.md | 104 +++++++++ .../progressive-tool-disclosure/context.md | 110 +++++----- .../progressive-tool-disclosure/design.md | 197 +++++++++++------- .../progressive-tool-disclosure/plan.md | 161 ++++++++------ .../progressive-tool-disclosure/research.md | 101 ++++++--- .../progressive-tool-disclosure/security.md | 151 ++++++++++++++ .../progressive-tool-disclosure/status.md | 104 ++++++--- 8 files changed, 722 insertions(+), 273 deletions(-) create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md index 0bdcf042f6..a988dc38ef 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md @@ -1,42 +1,59 @@ -# Progressive tool disclosure (playground build kit) +# Cheaper platform tools in the playground build kit Status: PLANNING — design workspace only, no implementation. -Date: 2026-07-20 +Date: 2026-07-20 · **Revised 2026-07-26** after a re-baseline and a code-grounded security review. -The playground advertises ~13 platform-op tool schemas to the model on every turn, before the -model has done anything — ~15K prompt tokens on a bare "hi", plus every extra always-on tool is -a "wander target" that derails runs. Skills already avoid this (on disk, only name+description -in the prompt, body loaded on demand). This project applies the same idea to the platform ops: -advertise a small **discovery meta-toolset** and load a full op schema only when the model asks. -The lazy layer lives at the runner's `advertisedToolSpecs()` seam, so it is harness-agnostic -(Pi + Claude) and touches no committed agent config. +The playground advertises 13 platform-op schemas to the model on every turn — **18,353 tokens** +measured — before the model has done anything. Every always-on tool is also a "wander target" that +derails runs. + +**What the re-baseline changed.** Three ops are 88% of that bill, and a *single* schema object +(`_build_agent_template_delta_schema`, 6,441 tokens, embedded in **two** ops) is 70% of it. The +long tail of ten ops is 2,120 tokens combined. So the win is not spread across the catalog — it is +concentrated in one duplicated object. See [baseline.md](baseline.md). + +That reordered this project. The **schema diet** — originally a side-slice — is now the headline: +~84% of the win, no runner change, no wire change, no permission change. The **discovery +meta-toolset** — originally the headline — is a follow-up worth ~14% more, and it carries by far +the highest risk in the project ([security.md](security.md)). ## Decisions (locked) -- **Seam = runner advertisement layer.** Intercept the advertised projection at - `services/runner/src/tools/public-spec.ts` (`advertisedToolSpecs`), consumed at exactly two - sites (`pi-assets.ts`, `environment.ts`). Execution + permission stay below it, unchanged. -- **Skills are out of scope.** Already progressive. -- **Op-catalog contents / overlay-cleanup is out of scope.** This work makes *any* op set cheap - to carry; it does not decide which ops belong. +- **Schema diet ships first, alone, and is independently valuable.** It is the bulk of the win at + near-zero risk. The replacement guidance (`references/config-schema.md`) already ships with the + skill and the skill already mandates reading it before `commit_revision`. +- **The meta-toolset is deferred behind evidence**, not cancelled. Gate it on: a measured + post-diet baseline, an answer on prompt caching, and a demonstrated reliability problem that + fewer visible tools actually fixes. +- **Seam for the meta-toolset = runner advertisement layer.** `advertisedToolSpecs` in + `services/runner/src/tools/public-spec.ts:57`, consumed at exactly two sites + (`pi-assets.ts:353`, `environment.ts:721`). Verified 2026-07-26. +- **Permission fidelity is a four-site problem, not a one-line one.** See + [security.md](security.md). This is the project's dominant risk. +- **Op-set curation is dropped, not deferred.** Hiding the 5-op event pack saves ~1,052 tokens + (~5%) and risks the agent being unable to schedule when a user pivots mid-conversation. Not + worth a capability regression. Revisit only with hard wander data. +- **Skills are out of scope.** Already progressive (64-token announcement). - **Playground overlay only.** No change to any saved/committed agent. -- **Schema diet is complementary, not exclusive.** Shrinking the embedded agent-template schema - is worth doing regardless and lands as its own slice. - **No commits during planning.** Implementation happens later on its own branch. ## Deliverables +- [baseline.md](baseline.md) — **measured** per-op cost, the concentration finding, the caching gap. +- [security.md](security.md) — what collapsing 13 ops behind one name breaks; the four gate sites. - [context.md](context.md) — problem, scope, non-goals, product language, success criteria. -- [research.md](research.md) — how the current advertise/execute path works, with `file:line`, - and the numbered seams the plan pins. -- [design.md](design.md) — the meta-toolset, the execution + permission path, alternatives. +- [research.md](research.md) — the current advertise/execute path with `file:line`, and the seams. +- [design.md](design.md) — the diet, then the meta-toolset; execution + permission path. - [plan.md](plan.md) — the sliced implementation plan, each slice with an exit check. - [status.md](status.md) — living source of truth: locked decisions, open questions, next action. ## Intended outcome -A playground author opens an agent and types a message. The model sees two small platform tools -(`agenta_ops` to list what it can do, `agenta_op` to fetch a schema and act) instead of a wall -of op schemas. A no-op turn costs a low-hundreds token constant instead of ~15K, and stays flat -as the catalog grows. Every op still runs with its exact self-targeting binding and approval -gate. Builds are cheaper and, per the internal-tools review, more reliable. +**After the diet (Slices 1–2):** a playground turn carries ~2,970 tokens of platform-op schema +instead of 18,353, with every tool still visible, every schema still authoritative enough to call +against, and not one line of permission code touched. + +**If the meta-toolset later clears its evidence bar (Slice 3):** the model sees two small platform +tools instead of thirteen, a turn costs a low-hundreds constant that stays flat as the catalog +grows, and every op still runs with its exact self-targeting binding and approval gate — with the +four-site permission work done properly and tested per mutating op. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md new file mode 100644 index 0000000000..b972bb6fd5 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md @@ -0,0 +1,104 @@ +# Baseline — measured advertised token cost + +**Measured 2026-07-26** (tiktoken `o200k_base`) against the live catalog on `main`. This +supersedes the 2026-07-17 numbers that the earlier drafts carried; those were ~19% low because +the catalog grew since. + +## Per-op advertised cost + +Counts the advertised projection the model actually sees — `{name, description, inputSchema}`, +with `inputSchema` = `PlatformOp.resolved_input_schema()` (type-refs expanded, context bindings +stripped), matching `advertisedToolSpec()` in `services/runner/src/tools/public-spec.ts:40`. + +| op | total | schema | descr | note | +| --- | ---: | ---: | ---: | --- | +| `test_run` | 7,777 | 7,593 | 163 | handler-gated; write | +| `commit_revision` | 6,878 | 6,713 | 147 | write | +| `query_spans` | 1,578 | 1,463 | 96 | read | +| `create_schedule` | 425 | 351 | 56 | write | +| `create_subscription` | 393 | 322 | 53 | write | +| `discover_triggers` | 356 | 171 | 165 | read | +| `annotate_trace` | 333 | 208 | 100 | write | +| `discover_tools` | 204 | 124 | 61 | read | +| `test_subscription` | 188 | 134 | 36 | write | +| `remove_schedule` | 69 | 41 | 10 | write | +| `remove_subscription` | 69 | 41 | 10 | write | +| `list_deliveries` | 46 | 11 | 14 | read | +| `list_schedules` | 37 | 11 | 6 | read | +| **total (13 ops)** | **18,353** | | | | + +## The three facts that drive the plan + +1. **Three ops are 88% of the bill.** `test_run` + `commit_revision` + `query_spans` = 16,233 of + 18,353. The other **ten ops combined are 2,120 tokens** — seven of them under 400 each. +2. **One schema object is 70% of the bill.** `_build_agent_template_delta_schema()` + (`sdks/python/agenta/sdk/agents/platform/op_catalog.py:317`) is **6,441 tokens** and is embedded + **twice** — in `commit_revision` and in `test_run`. That is 12,882 of 18,353. +3. **`test_run` may not be live.** It is handler-based (`handler="tools.agenta.test_run"`, no + `method`/`path`) and gated by `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off + (`sdks/python/agenta/sdk/agents/platform/platform_tools.py:36`). With handlers off the live + advertised cost is **10,576**, of which `commit_revision` alone is 65%. + +## Consequence for slicing + +| lever | est. after | cut from 18,353 | risk | +| --- | ---: | ---: | --- | +| Schema diet on `commit_revision` + `test_run` | ~5,500 | ~70% | near-zero (see below) | +| \+ trim `query_spans` filter DSL | ~2,970 | ~84% | low | +| \+ discovery meta-toolset | ~400 | ~98% | **high** — see [security.md](security.md) | + +The meta-toolset's *marginal* token win over a completed diet is ~2,600 tokens (~14% of the +original bill). That is the number its risk must be weighed against — not the full 18,353. + +## Why the diet is near-zero risk + +The embedded schema is **advisory duplication of a doc that already ships and is already +mandatory reading**: + +- `references/config-schema.md` is a 3,621-token prose + example reference, shipped as a + `SkillFile` on `BUILD_AN_AGENT_SKILL` + (`sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py:740`). +- The skill body already instructs: *"Read `references/config-schema.md` before your first + `commit_revision`"* (`agenta_builtins.py:579`), and again on failure (`:711`, `:716`). +- That reference states plainly that **the commit endpoint does not validate this shape**. So the + embedded JSON Schema is not enforcing a server contract — it is model guidance that a + better-written, on-demand doc already provides. + +What is genuinely lost by dieting: the harness's own pre-call JSON-Schema validation of the +argument shape (Pi passes `inputSchema` to `registerTool({parameters})`, +`services/runner/src/extensions/agenta.ts:305`). Malformed configs would surface as a server +error instead of a pre-call harness error. Given the server does not validate the config shape +anyway, this is a small change in *where* the error appears, not whether it is caught. + +## Reproducing + +```python +import sys, json, tiktoken +sys.path.insert(0, "sdks/python") +from agenta.sdk.agents.platform.op_catalog import get_platform_op + +OPS = ("discover_tools","commit_revision","annotate_trace","query_spans","test_run", + "discover_triggers","create_schedule","create_subscription","list_schedules", + "list_deliveries","test_subscription","remove_schedule","remove_subscription") +enc = tiktoken.get_encoding("o200k_base") +total = 0 +for name in OPS: + op = get_platform_op(name) + adv = {"name": f"tools.agenta.{name}", "description": op.description, + "inputSchema": op.resolved_input_schema()} + n = len(enc.encode(json.dumps(adv))) + total += n + print(f"{name:22} {n:>7}") +print(f"{'TOTAL':22} {total:>7}") +``` + +## Open measurement gap — prompt caching + +**Nothing in this project has measured whether these tokens are actually billed per turn.** Tool +definitions are a stable prompt prefix, which is exactly what provider prompt caching targets, and +both harnesses (Claude Code, Pi) run their own caching. `grep -ri "cache_control|prompt.cach"` over +`services/runner/src` and `sdks/python/agenta/sdk/agents` returns nothing, so the runner neither +sets nor inspects cache behavior — it is entirely the harness's. + +If the schemas are cached, the real cost is "18K once, a fraction thereafter", and the ROI of +everything past the diet drops sharply. **Measure this before committing to the meta-toolset.** diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md index a6fc3ef459..342c9bab3f 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md @@ -2,74 +2,84 @@ ## Problem -Open a playground agent that shows "Tools: None" and type "hi". The turn costs ~15K prompt -tokens. Nothing the author did explains it: the cost is the playground **build kit**, which -injects ~13 platform-op tool schemas into the agent template and advertises every one of them to -the model on every turn. +Open a playground agent that shows "Tools: None" and type "hi". The turn carries **18,353 prompt +tokens** of platform-op schema (measured 2026-07-26, tiktoken `o200k_base` — +[baseline.md](baseline.md)). Nothing the author did explains it: the cost is the playground +**build kit**, which injects 13 platform ops into the agent template and advertises every one of +them to the model on every turn. Two costs, not one: -- **Tokens.** Measured (tiktoken `o200k_base`, 2026-07-17): the ops dominate. `test_run` (~6.5K) - and `commit_revision` (~5.8K) each embed the full ~5.5K-token agent-template delta schema; - `query_spans` ~1.3K; all ops together ~15.4K. Skills are NOT the cost (a ~68-token - announcement only). This is a ~5x tax on every turn, paid before the model is useful. +- **Tokens.** And they are *concentrated*, not spread. `test_run` (7,777) + `commit_revision` + (6,878) + `query_spans` (1,578) are **88%** of the bill; the other ten ops are 2,120 tokens + combined. One schema object — `_build_agent_template_delta_schema()`, 6,441 tokens, embedded in + **both** `test_run` and `commit_revision` — is **70%** on its own. - **Reliability.** The internal-tools review - (`../builder-agent-reliability/tools-review/part-2-internal-tools.md`) found the same tools are - a *double* cost: "each unused tool is context cost plus a wander target (the capstone showed - extra visible tools derail runs)." Fewer advertised tools is a correctness win, not just a bill. + (`../builder-agent-reliability/tools-review/part-2-internal-tools.md`) found the same tools are a + *double* cost: "each unused tool is context cost plus a wander target (the capstone showed extra + visible tools derail runs)." -It scales the wrong way: adding a catalog op is a one-line data change that ships to every -playground agent unconditionally, so every op we add makes every turn heavier. +**These two costs need different fixes, and that is the central finding of this workspace.** +Shrinking schemas fixes the token cost completely and the wander cost not at all — the tool *count* +is unchanged. Reducing the advertised tool count fixes wander but, after the diet, buys only ~14% +more tokens. Conflating them is what made the first draft of this plan lead with the expensive, +risky lever instead of the cheap one. -Skills already solved this exact shape — on disk, only name+description in the prompt, body -loaded on demand. The platform ops never got the same treatment. +It also scales the wrong way: adding a catalog op is a one-line data change that ships to every +playground agent unconditionally. But note the tail is *cheap* — ten ops for 2,120 tokens — so +catalog growth is not currently the pain. Schema *depth* is. ## Scope (this delivery) -- The **runner advertisement layer** for playground platform ops: `advertisedToolSpecs()` in - `services/runner/src/tools/public-spec.ts`, consumed by the Pi and Claude delivery paths. -- A **discovery meta-toolset** (`agenta_ops` + `agenta_op`) that replaces the always-advertised - op schemas: list ops cheaply, fetch one schema on demand, execute against the private spec. -- A **schema diet** for the two heaviest op schemas (`commit_revision`, `test_run`) — - independently valuable, folded in as its own slice. -- A **token + reliability baseline** so before/after is measured, not asserted. -- A **one-line nudge** in the always-loaded `build-an-agent` skill so the model uses the - meta-toolset. +- A **schema diet** for the ops carrying deep expanded schemas — `commit_revision`, `test_run`, + then `query_spans`. This is the primary deliverable: ~84% of the token win, no runner change. +- A **token + reliability baseline** so before/after is measured, not asserted — done for tokens + ([baseline.md](baseline.md)), still open for reliability and for prompt caching. +- A **designed, risk-registered meta-toolset** (`agenta_ops` + `agenta_op`) held behind an + evidence gate, with its permission work scoped honestly ([security.md](security.md)). -## Out of scope for the first delivery +## Out of scope -- **Skills.** Already progressive; untouched. -- **Which ops belong in the overlay** (the build-kit-tools-cleanup debate). Orthogonal — this - work makes any op set cheap, which lowers the pressure to prune. -- **External tool discovery (`discover_tools`).** That discovers *Composio* tools to wire into an - agent; it stays as-is and is itself one of the ops we disclose. -- **User / gateway / code / client tools.** The POC targets platform ops (the measured cost). The - mechanism can generalize later. -- **Committed non-playground agents.** They advertise only what their author declared; no problem - today. -- **Dynamic real-name re-advertisement (M2).** Advertising a loaded op under its real name with a - schema-validated signature is a productionization option, evaluated after the POC — not built - here. +- **Skills.** Already progressive; a 64-token announcement. Untouched. +- **Op-set curation** (dropping the event pack from the default overlay). *Dropped, not deferred* — + it saves ~1,052 tokens and risks the agent being unable to schedule when the user pivots + mid-conversation. Revisit only with hard wander data. +- **External tool discovery (`discover_tools`).** It discovers *Composio* tools to wire into an + agent; it stays as-is and is itself one of the ops we would disclose. +- **User / gateway / code / client tools.** The measured cost is platform ops. +- **Committed non-playground agents.** They advertise only what their author declared. +- **Dynamic real-name re-advertisement (M2).** A productionization option, evaluated only if the + meta-toolset ships. ## Product language -- **Platform op** — an existing Agenta endpoint exposed to the agent as a tool, defined in the - code catalog `op_catalog.py` (e.g. `commit_revision`, `query_spans`). +- **Platform op** — an existing Agenta endpoint exposed to the agent as a tool, defined in the code + catalog `op_catalog.py` (e.g. `commit_revision`, `query_spans`). - **Advertised spec** — the `{name, description, inputSchema, …}` projection the model sees; distinct from the **private resolved spec** the runner executes from. -- **Discovery meta-toolset** — the small fixed set (`agenta_ops`, `agenta_op`) that stands in for - the op schemas: list, describe-on-demand, invoke. +- **Schema diet** — replacing a deep, type-expanded `inputSchema` with a shallow one plus a pointer + to on-demand prose guidance. Tool stays visible; only its manual shrinks. +- **Discovery meta-toolset** — the small fixed set (`agenta_ops`, `agenta_op`) that would stand in + for the op schemas: list, describe-on-demand, invoke. - **Disclosure** — moving an op's full schema out of the prompt (paid every turn) into a tool result (paid once, only when fetched). ## Success criteria -1. A no-op turn's platform-op prompt cost drops from ~15K to a low-hundreds constant, and stays - flat as ops are added to the catalog. -2. Capability parity: the build-an-agent lab loop (discover → wire → commit → test → schedule) - passes with the disclosed toolset. -3. No safety regression: self-targeting `$ctx` bindings and per-op approval/permission behave - exactly as today, verified per mutating op. -4. Reliability does not regress (target: fewer "wander" failures) on the lab matrix. -5. Cost of laziness is bounded: ≤1 extra round-trip per distinct op used; a schema fetched at - most once per op per conversation. +Rewritten 2026-07-26. The old criterion measured a no-op "hi" turn, which optimizes the cheapest +case nobody pays for; and it credited the meta-toolset with a win the diet delivers. + +1. **Diet:** platform-op prompt cost drops from 18,353 to under ~3,000 tokens, with no capability + loss — a lab agent still commits a valid config and runs a test. +2. **Session-level, not turn-level:** total platform-op tokens across a full build session + (discover → wire → commit → test → schedule) drop materially. A no-op turn is *reported*, never + the target. +3. **Caching answered:** we know whether these tokens are billed per turn or cached after the + first, and the ROI of anything past the diet is restated against that answer. +4. **No safety regression:** self-targeting `$ctx` bindings and per-op approval behave exactly as + today, verified per mutating op on **both** harness paths. +5. **Reliability measured, not assumed:** if the meta-toolset proceeds, it is justified by observed + wander reduction on the lab matrix — the token case alone (~14%) does not carry it. +6. **Cost of laziness bounded** (meta-toolset only): ≤1 extra round-trip per distinct op used, a + schema fetched at most once per op per conversation, and no net regression in session-level + tokens once history-resident schemas are counted. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md index be3ca67de2..b260ca073b 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md @@ -1,101 +1,158 @@ -# Design — the discovery meta-toolset +# Design + +Two independent levers, in the order their evidence supports. Revised 2026-07-26: the re-baseline +([baseline.md](baseline.md)) showed the token cost is concentrated in schema *depth*, not tool +*count*, which promotes the diet and demotes the meta-toolset. + +| | Lever A — schema diet | Lever B — meta-toolset | +| --- | --- | --- | +| Fixes | token cost | wander / tool count | +| Win | ~84% (18,353 → ~2,970) | ~14% more (→ ~400) | +| Touches | `op_catalog.py` schemas | advertisement + 4 permission sites | +| Runner change | none | substantial | +| Wire change | none | possible (marker) | +| Risk | near-zero | **high** ([security.md](security.md)) | + +--- + +# Lever A — the schema diet (primary) ## The core move -Stop advertising N full op schemas every turn. Advertise a small fixed meta-toolset; keep the op -specs resolved but **private** in runner memory; move each op's full schema from the *prompt* -(paid every turn) to a *tool result* (paid once, only for ops actually used). +Two ops embed the *same* 6,441-token object, `_build_agent_template_delta_schema()` +(`op_catalog.py:317`), expanded inline into their `input_schema`. That single object counted twice +is 70% of the entire advertised bill. Replace the deep expansion with a shallow schema plus a +pointer to prose guidance that already ships. -Two model-facing tools (names illustrative): +## Why this costs almost no capability -- **`agenta_ops(query?)`** — returns the op catalog as a compact list: `{op, one_line, - read_only}` per op, no input schemas. Optional `query` filters. A few hundred tokens, flat, - regardless of catalog size. This is the "what can I do on the platform" index. -- **`agenta_op(op, args?)`** — the generic invoker. With no `args` (or `mode:"describe"`) it - returns **one** op's full `inputSchema` as a tool result; with `args` it executes that op. +The replacement is not hypothetical — it is already in the product and already mandatory: -The 13 op specs stay in `plan.toolSpecs` and in `toolSpecsByName`. Only the *advertisement* is -replaced. This is the pattern Claude Code uses on its own ~200 tools: names are listed, and a -`ToolSearch` step loads a schema only when needed. +- `references/config-schema.md` (3,621 tokens of prose + worked examples) ships as a `SkillFile` on + `BUILD_AN_AGENT_SKILL` (`agenta_builtins.py:740`). +- The skill body already says *"Read `references/config-schema.md` before your first + `commit_revision`"* (`:579`), and points back to it on failure (`:711`, `:716`). +- That reference states the commit endpoint **does not validate this shape**. So the embedded + schema enforces no server contract; it is model guidance only. -## Why this is safe by construction +The model therefore already has a better-written, on-demand source for the same information. The +embedded schema is duplication. -Execution and permission read the **private** spec (research seams 2–4). The invoker feeds the -target op's private `call` descriptor into the *unchanged* `direct.ts` path, so: +## Shallow, not empty -- **Self-targeting is preserved.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so - `commit_revision` still binds `$ctx.workflow.variant.id` and the model cannot retarget another - variant. The invoker never sees or forwards those fields — they were stripped at resolve time - and are re-applied below the invoker. -- **SSRF guard is preserved.** `directCallUrl` host-locks to the run's Agenta origin. -- **Approval is preserved — if the invoker gates per-op.** This is the one piece that is designed - in, not free (see below). +The first draft proposed `{"type": "object"}` plus a description. Prefer a **shallow** schema over +an open one: keep the top-level keys (`instructions`, `llm`, `tools`, `mcps`, `skills`, `harness`, +`runner`, `sandbox`) with one-line descriptions, and stop before expanding nested `$defs`. That +costs a few hundred tokens instead of ~6,400 and keeps the model oriented about *which* keys exist, +while the reference doc supplies the shapes underneath. -## Execution path for `agenta_op(op, args)` +The expansion is what explodes — `expand_type_refs` / `_deep_partial_schema` (`op_catalog.py:249`). +A depth-limited expansion is the surgical form of this change and is reusable for any future op. + +## What is genuinely lost + +Pi validates arguments against the advertised JSON Schema before dispatch +(`registerTool({parameters})`, `extensions/agenta.ts:305`). A shallower schema catches less +pre-call, so a malformed config surfaces as a server error rather than a harness error. Bounded: +the server does not validate the config shape today either, so this changes *where* the failure +appears, not *whether* it is caught. `test_run` after commit remains the real verification, exactly +as the skill already instructs. + +--- + +# Lever B — the discovery meta-toolset (deferred behind evidence) + +## The core move -The runner special-cases the invoker in its dispatch: +Stop advertising N op schemas. Advertise a small fixed meta-toolset; keep the op specs resolved but +**private** in runner memory; move each op's schema from the *prompt* into a *tool result*. + +- **`agenta_ops(query?)`** — the op catalog as a compact list: `{op, one_line, read_only}`, no input + schemas. A few hundred tokens, flat regardless of catalog size. +- **`agenta_op(op, args?)`** — the generic invoker. No `args` (or `mode:"describe"`) returns one + op's `inputSchema` as a tool result; with `args` it executes that op. + +The op specs stay in `plan.toolSpecs` and `toolSpecsByName`. Only the *advertisement* changes. This +is the pattern Claude Code uses on its own tools: names listed, schema loaded on demand. + +## Why it is deferred + +1. **Its token case is weak after Lever A.** ~2,600 marginal tokens (~14% of the original bill). +2. **Its real case is reliability** — fewer wander targets — and that is a hypothesis from the + internal-tools review, not a measurement. The diet does not reduce tool count, so this argument + survives Lever A intact; it just has to be *proven* rather than assumed. +3. **Its cost is the permission plane.** Four independent gate sites, each of which must learn to + resolve the target from `args.op`, in code deliberately written to fail closed. Full analysis: + **[security.md](security.md)**. +4. **It can regress long sessions.** A fetched schema is history-resident for the rest of the + conversation, in a far less cache-friendly position than a stable tool-definition prefix. + Without Lever A first, disclosure can cost *more* across a real build session. + +## Execution path for `agenta_op(op, args)` 1. **Resolve the target.** Look up `op` in `toolSpecsByName`. Unknown → tool error listing valid - ops (recoverable). -2. **Describe mode** (no `args` / `mode:"describe"`) → return the target spec's - `resolved input schema` as the tool result. No side effects, no approval. -3. **Execute mode** → build the permission **gate from the target op's spec** and run - `decide(gate, plan, stored)` (`permission-plan.ts:138`). `allow` runs; `deny` refuses; - `ask`/undecided pauses the turn and emits the normal `interaction_request(user_approval)` — - identical to calling the op directly today. Then run the target's `call` through + ops (recoverable), and fail closed at every gate. +2. **Describe mode** → return the target's resolved input schema. No side effects, no approval. +3. **Execute mode** → resolve the permission gate from the **target** op's spec at all four sites + (see security.md), run `decide()`, then `assertRequiredArguments(target, innerArgs)` followed by `assembleBody` → `directCallUrl` → `callDirect`, exactly as `executeRelayedTool` does now. 4. **Return** the endpoint response verbatim. -The invoker is one ordinary advertised tool on both delivery paths (Pi native, Claude MCP), so no -harness-specific advertisement logic is needed. +## Why execution itself is safe by construction + +Execution reads the **private** spec (research seams 2–3), so feeding the target's private `call` +into the unchanged `direct.ts` path preserves: + +- **Self-targeting.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so + `commit_revision` still binds `$ctx.workflow.variant.id`; the model cannot retarget. Those fields + were stripped at resolve time and are re-applied below the invoker. +- **SSRF guard.** `directCallUrl` host-locks to the run's Agenta origin. + +Permission is the part that is **not** free. It is designed in, not inherited — see security.md. ## Identifying the disclosure-eligible set -A platform op is a `callback`-kind spec with a direct `call`; so is a `reference` (workflow) -tool. There is no explicit marker today (research seam 5). Two options: +A platform op is a `callback`-kind spec with a direct `call`; so is a `reference` (workflow) tool. +No explicit marker exists today (research seam 5). -- **Heuristic (zero wire change).** Collapse every direct-`call` callback spec into the - meta-toolset; leave builtins, `client`, `code`, `gateway` (callRef), and MCP advertised as-is. - The playground overlay only injects platform ops (+ the two client tools), so this covers the - measured cost. Risk: an author who added a `reference` tool would see it disclosed too. Fine for - a flagged POC. -- **Marker (small wire add).** The platform resolver stamps a `source:"platform"` (or a - `disclosable` group tag) on the resolved spec; the runner collapses exactly that group. Precise, - and lets us disclose gateway/reference later on purpose. Costs a `protocol.ts` + `wire.py` + - golden change. +- **Heuristic (zero wire change).** Collapse every direct-`call` callback spec. Risk: an author's + `reference` tool gets disclosed too. Acceptable for a flagged POC. +- **Marker (small wire add).** The platform resolver stamps `source:"platform"`; the runner + collapses exactly that group. Costs `protocol.ts` + `wire.py` + goldens. -Recommendation: heuristic for the POC to prove the numbers, marker before default-on. +Recommendation: heuristic for the POC, marker before default-on. ## The catalog summary (`agenta_ops`) -Build `{op, one_line, read_only}` runner-side from the resolved specs already in memory — the -`description` and `read_only`/`permission` fields ride along; only `inputSchema` is dropped. Zero -new wire fields. (Alternative: thread a summary list from `op_catalog.py`; rejected for the POC as -extra plumbing.) +Build `{op, one_line, read_only}` runner-side from the resolved specs already in memory — +`description` and `readOnly` ride along; only `inputSchema` is dropped. Zero new wire fields. ## Discoverability -The always-loaded `build-an-agent` skill (~68 tokens) gains one line: "platform actions are listed -by `agenta_ops`; fetch a schema with `agenta_op(op)` before calling it with args." Mirrors how the -skill already routes the builder; negligible always-on weight. +The always-loaded `build-an-agent` skill (64-token description, 2,619-token body) gains one line: +"platform actions are listed by `agenta_ops`; fetch a schema with `agenta_op(op)` before calling it +with args." + +--- ## Alternatives considered +- **Op-set curation** — drop the 5-op event pack from the default overlay unless the ask is + event-driven. **Rejected.** Saves ~1,052 tokens (~5%) and cannot be decided correctly at run + start: a user who pivots to "schedule this daily" mid-conversation would find the capability + missing. A capability regression is not worth 5%. - **M2 — dynamic real-name advertisement.** Advertise names only; a `load_op` call registers the - real op spec into the harness registry mid-session (Pi extension re-register; Claude MCP - `tools/list_changed`) so the model calls the op by its real name with a schema-validated - signature and native per-op permission. Highest fidelity, but needs mid-session re-registration - on both harnesses and, under the cold-replay runtime, reconstruction of the "loaded" set each - turn. Deferred to productionization. -- **Schema diet only.** Keep all ops advertised; replace the embedded ~5.5K agent-template delta - schema in `commit_revision`/`test_run` with an open object + a pointer to the skill's - `references/config-schema.md`. ~11K of ~15K is those two schemas, so this alone is a large, - near-zero-risk win. Complementary — it also shrinks each describe-mode fetch under the invoker. -- **Mode-gating.** Drop the build kit in Chat mode, keep it in Build mode. Removes capability - rather than deferring it; can layer on top, not a substitute. - -## Cost of laziness - -One extra round-trip per *distinct* op used (describe → then execute), paid only for ops the run -actually touches, and cacheable to once per op per conversation. A no-op turn pays nothing beyond -the two meta-tools. Acceptable for a builder flow; measured in Slice 3. + real spec into the harness registry mid-session (Pi re-register; Claude MCP `tools/list_changed`) + so the model calls the op by its real name with native per-op permission. **Highest fidelity, and + it dissolves most of security.md** — the gates keep seeing real names. But it needs mid-session + re-registration on both harnesses and, under the cold-replay runtime, reconstruction of the + "loaded" set each turn. If Lever B is ever justified, compare M2 against it directly rather than + treating M2 as a later upgrade — M2's extra complexity may be cheaper than four-site gate surgery. +- **Mode-gating.** Drop the build kit in Chat mode, keep it in Build mode. Removes capability rather + than deferring it; can layer on top, not a substitute. + +## Cost of laziness (Lever B) + +One extra round-trip per *distinct* op used, paid only for ops the run actually touches, cacheable +to once per op per conversation. A no-op turn pays nothing beyond the two meta-tools. Must be +measured at **session** level, counting history-resident schemas — not on a "hi" turn. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md index a4888c20a0..ef42436bf7 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md @@ -1,77 +1,112 @@ # Plan — sliced implementation +Re-sliced 2026-07-26 so risk tracks reward. The old plan ran baseline → diet → mechanism → measure +with the mechanism as the centrepiece. The re-baseline showed the diet *is* the project; the +mechanism is a separately-justified follow-up. + ## Scope now -- Behind a flag/env (default OFF) so the disclosed path A/B's against today's always-advertise - path before it becomes default. +- **Slices 1–2 (the diet) are the committed delivery.** They are independently shippable, touch + only `op_catalog.py` schemas, and change no runner, wire, or permission code. +- **Slice 3 (the meta-toolset) is gated on evidence**, behind a flag defaulting OFF, and does not + start until Slices 0–2 report. - Playground platform ops only; no saved-agent change; no committed-agent behavior change. -- The schema diet (Slice 1) is independently shippable and lands first. -- Each slice leaves the tree working and testable. Order: baseline → diet → mechanism → measure. - -## Slice 0 — Baseline (pin the real cost) - -1. Write a measurement script (tiktoken `o200k_base`) that resolves the default build-kit overlay - and reports per-op advertised token cost + total, using the same `advertisedToolSpecs` - projection the runner ships (or a faithful mirror). -2. Confirm which ops actually advertise live today — resolve with `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` - off, and record whether `test_run` is in the advertised set. -3. Add a runner unit test asserting the current behavior: every resolved platform op appears in - `advertisedToolSpecs(plan.toolSpecs)`. This is the invariant later slices intentionally flip. -4. Check a `baseline.md` table into this folder (before-numbers, per op). - -**Exit:** `baseline.md` exists with per-op numbers; the "all platform ops advertised today" test -passes on `main`. - -## Slice 1 — Schema diet (large, low-risk win, independent) - -1. In `op_catalog.py`, replace the embedded agent-template delta schema in `commit_revision` - (and `test_run`) with an open object (`{"type":"object"}`) plus a description pointing at the - skill's `references/config-schema.md`. -2. Update the platform-op / wire contract tests that pin those schemas. -3. Re-run the Slice 0 script; record the drop (expect ~11K). -4. Lab check: an agent can still `commit_revision` with a valid config (the model has the shape - from the skill reference, and the server still validates). - -**Exit:** measured total drops by ~11K; contract tests green; a lab run commits a revision -successfully. - -## Slice 2 — Disclosure mechanism (the POC), flagged - -1. Add the flag/env (default off) that turns disclosure on for a run. -2. Introduce a disclosure transform applied to the advertised set at the two call sites - (`pi-assets.ts:353`, `environment.ts:721`): when on, replace the disclosure-eligible specs - (see design "Identifying the disclosure-eligible set" — heuristic for the POC) with the two - meta-tools `agenta_ops` + `agenta_op`; keep client tools and everything else advertised. - `plan.toolSpecs` / `toolSpecsByName` stay complete. -3. Implement the invoker dispatch: `agenta_ops` returns `{op, one_line, read_only}` built - runner-side from the resolved specs; `agenta_op` describe-mode returns one op's input schema; - `agenta_op` execute-mode builds the gate from the TARGET op's spec, runs `decide()`, then - `assembleBody`/`directCallUrl`/`callDirect` (reuse the `executeRelayedTool` core). -4. Unit tests: (a) with the flag on, only the meta-tools + client tools are advertised; (b) the - full private specs remain in `toolSpecsByName`; (c) per mutating op (`commit_revision`, - `create_schedule`, `remove_*`), `agenta_op` execute-mode produces the SAME approval verdict as - a direct call — no approval regression; (d) `$ctx` binding still fills server-side (the model - cannot retarget); (e) describe-mode has no side effect and no approval. -5. Add the one-line nudge to the `build-an-agent` skill. +- Each slice leaves the tree working and testable. + +## Slice 0 — Baseline ✅ tokens done, 2 gaps open + +1. ~~Measurement script + per-op table.~~ **Done** — [baseline.md](baseline.md), measured + 2026-07-26: 18,353 total, top 3 ops = 88%, one duplicated schema object = 70%. +2. **OPEN — confirm the live advertised set.** `test_run` is handler-gated + (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off). Resolve the overlay in a real run and + record whether it advertises. If not, the live figure is 10,576 and `commit_revision` alone is + 65% of it. +3. **OPEN — answer prompt caching.** Nothing in the runner sets or inspects cache behavior; the + harnesses own it. Determine whether these tokens are billed per turn or only on the first. This + directly sets the ROI of Slice 3 and should be answered before it is scheduled. +4. Add a runner unit test asserting today's behavior: every resolved platform op appears in + `advertisedToolSpecs(plan.toolSpecs)`. The invariant Slice 3 would intentionally flip. + +**Exit:** live-set and caching questions answered in baseline.md; the "all platform ops advertised +today" test passes on `main`. + +## Slice 1 — Diet the duplicated agent-template schema (the main win) + +Targets 70% of the bill in one file. + +1. In `op_catalog.py`, stop expanding `_build_agent_template_delta_schema()` inline into + `commit_revision` and `test_run`. Emit a **shallow** schema — top-level keys with one-line + descriptions, no nested `$defs` expansion — plus a description pointing at + `references/config-schema.md` (already shipped, already mandatory reading; see design.md). + Prefer implementing this as a depth limit on the expansion so it is reusable. +2. Update the platform-op / wire contract tests and goldens that pin those schemas. +3. Re-run the baseline script; record the drop (expect ~12,900). +4. Lab check: an agent still commits a valid config and `test_run` verifies it. Confirm the model + reaches for `references/config-schema.md` as the skill instructs. + +**Exit:** measured total drops to ~5,500; contract tests green; a lab run commits a revision and +passes a test run without the embedded schema. + +## Slice 2 — Trim `query_spans` + +1. Shallow the filtering-DSL `$defs` in `_QUERY_SPANS_INPUT_SCHEMA` the same way (1,463 → target + ~300), pointing at prose guidance for the DSL. +2. Re-measure. + +**Exit:** platform-op total under ~3,000 (~84% cut from 18,353). Success criteria 1 and 2 in +context.md are met **without any runner change**. + +## Decision point + +Re-read [baseline.md](baseline.md) and [security.md](security.md) with the post-diet numbers and +the caching answer in hand, then decide whether Slice 3 proceeds. It should proceed **only** if: + +- there is measured evidence of wander failures attributable to tool count, **and** +- the residual ~2,600 tokens (or the caching answer) still justifies four-site permission surgery, + **and** +- M2 (dynamic real-name advertisement) has been compared head-to-head — it may be cheaper than the + gate rework, since it keeps real tool names in front of every gate. + +If those do not hold, close the project after Slice 2 and record why. + +## Slice 3 — Discovery meta-toolset (only if the gate above opens) + +Flagged, default off. + +1. Add the flag/env that turns disclosure on for a run. +2. Disclosure transform at the two advertisement call sites (`pi-assets.ts:353`, + `environment.ts:721`): replace disclosure-eligible specs with `agenta_ops` + `agenta_op`; keep + client tools and everything else advertised. `plan.toolSpecs` / `toolSpecsByName` stay complete. + Register the invoker into `piToolSpecsByName` too, or the Pi gate fails closed on it. +3. **Permission work — the bulk of this slice.** Teach all four gate sites to resolve the target + from `args.op` and validate it against the known spec map: + `relay-guard.ts:53`, `acp-interactions.ts:516` (Claude), `acp-interactions.ts:456` (Pi), + `extensions/agenta.ts:318` (in-sandbox). Keep `grant()`/`consume()` keying consistent. Read + [security.md](security.md) before writing any of it. +4. Invoker dispatch: `agenta_ops` returns `{op, one_line, read_only}` built runner-side; + `agenta_op` describe-mode returns one op's schema; execute-mode re-runs + `assertRequiredArguments` against the **target** spec, then reuses the `executeRelayedTool` core. +5. Tests — the full list in security.md ("Required test coverage"), per mutating op, on **both** + harness paths. Minimum: same verdict as a direct call; `allow_reads` still distinguishes read + from write; name-based policy rules still match; approvals do not cross ops; `$ctx` still binds + server-side; describe-mode is inert; unknown `op` fails closed everywhere. +6. Add the one-line nudge to the `build-an-agent` skill. **Exit:** flag on → a lab run completes discover → wire → commit → schedule using only the -meta-tools; every mutating-op approval test passes; `tsc` + `pnpm test` green in `services/runner`. +meta-tools; every test in security.md passes; `tsc` + `pnpm test` green in `services/runner`. -## Slice 3 — Measure, decide default, scope M2 +## Slice 4 — Measure and decide default-on -1. Re-run the Slice 0 script with the flag on; record the no-op turn cost (target: low hundreds). -2. Run the build-an-agent lab / release gate with the flag on vs off; compare pass rate and - "wander" failures. -3. Write a `results.md` comparison (tokens + reliability, before/after). -4. Decide: flip default on? And is M2 (dynamic real-name advertisement) worth it for the ops that - most benefit from schema-validated calls? +1. Re-measure at **session** level (not a no-op turn), counting history-resident fetched schemas. +2. Run the build-an-agent lab / release gate flag-on vs flag-off; compare pass rate and wander + failures. +3. Write `results.md`; decide default-on and whether M2 is worth it. -**Exit:** `results.md` checked in with the before/after; a go/no-go recommendation on -default-on and on M2. +**Exit:** `results.md` checked in; a go/no-go on default-on. ## Not in this plan -- Marker-based eligibility (Slice 2 uses the heuristic); adding a `source:"platform"` wire marker - is a follow-up if we default-on for gateway/reference too. +- **Op-set curation** — dropped (capability regression for ~5%; see design.md alternatives). +- Marker-based eligibility (Slice 3 uses the heuristic). - Disclosing gateway/code/client/MCP tools. -- M2 dynamic advertisement implementation. +- M2 implementation — but it must be *compared* at the decision point. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md index c764f51fed..7115c03192 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md @@ -1,13 +1,15 @@ # Research — how tools reach the model today -Verified 2026-07-20 against the code cited. Paths are current (`services/runner/`; some docs -still say `services/agent/`). +Verified 2026-07-20, **re-verified 2026-07-26** against the code cited — every `file:line` below +was re-checked on `main` and holds. Paths are current (`services/runner/`; some docs still say +`services/agent/`). ## The path a platform op takes, end to end 1. **Overlay injects the op set (Python).** The playground build kit is a per-run overlay, not a committed field. `build_agent_template_overlay()` (`api/oss/src/core/workflows/build_kit.py`) - emits `DEFAULT_BUILD_KIT_OPS` — ~13 platform ops (`discover_tools`, `commit_revision`, + emits `DEFAULT_BUILD_KIT_OPS` — exactly 13 platform ops (`build_kit.py:27`; `discover_tools`, + `commit_revision`, `annotate_trace`, `query_spans`, `test_run`, `discover_triggers`, `create_schedule`, `create_subscription`, `list_schedules`, `list_deliveries`, `test_subscription`, `remove_schedule`, `remove_subscription`) plus the `request_connection` / `request_input` @@ -64,41 +66,76 @@ still say `services/agent/`). Both paths consume the same projection function. Intercepting it once (or wrapping it at both call sites behind a flag) covers both harnesses with no harness-specific logic. -## Measured token cost (needs Slice 0 re-baseline) +## Measured token cost -From the 2026-07-17 investigation (tiktoken `o200k_base`); flagged for re-measure because -`test_run` is handler-gated (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off) and may not -advertise live. +**Re-baselined 2026-07-26** (tiktoken `o200k_base`) — full table, method, and reproduction script +in [baseline.md](baseline.md). The 2026-07-17 figures that earlier drafts carried (~15,454 total) +were ~19% low; the catalog grew. | Op | Tokens | Note | -| --- | --- | --- | -| `test_run` | ~6,521 | embeds the ~5,462-token agent-template delta schema; handler-gated | -| `commit_revision` | ~5,844 | embeds the same ~5,462-token delta schema | -| `query_spans` | ~1,283 | filtering DSL `$defs` | -| others (10) | remainder | — | -| **all ~13 ops** | **~15,454** | the "hi" cost | - -Two schemas (`commit_revision` + `test_run`) account for ~11K of the ~15K — hence the schema-diet -slice is a large, low-risk win on its own. +| --- | ---: | --- | +| `test_run` | 7,777 | embeds the 6,441-token agent-template delta schema; handler-gated | +| `commit_revision` | 6,878 | embeds the **same** 6,441-token delta schema | +| `query_spans` | 1,578 | filtering DSL `$defs` | +| others (10 ops) | 2,120 | combined; 7 of them under 400 each | +| **all 13 ops** | **18,353** | the "hi" cost | + +Two findings reorder the whole project: + +- **Concentration.** The top three ops are 88% of the bill. The catalog's *length* is not the + problem; two ops' schema *depth* is. +- **Duplication.** `_build_agent_template_delta_schema()` (`op_catalog.py:317`) is 6,441 tokens and + is embedded twice — 12,882 of 18,353, i.e. **70% of the bill is one object counted twice.** + +`test_run` is handler-based (`handler="tools.agenta.test_run"`) and gated by +`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` (default off, `platform_tools.py:36`), so the live +advertised cost may be 10,576 — in which case `commit_revision` alone is 65% of it. Confirm which +set advertises live before quoting a headline number. + +### The diet's replacement already exists + +`references/config-schema.md` ships as a `SkillFile` on `BUILD_AN_AGENT_SKILL` +(`agenta_builtins.py:740`) — 3,621 tokens of prose and worked examples covering the same config +shape. The skill body **already** instructs the model to read it before its first `commit_revision` +(`agenta_builtins.py:579`) and again on failure (`:711`, `:716`). That reference also states the +commit endpoint does **not** validate the config shape — so the embedded JSON Schema enforces no +server contract. It is advisory duplication of better, on-demand guidance. ## Seams the plan must pin +Seams 1–3 and 7 apply to the meta-toolset only. **The schema diet touches none of them** — it +changes `input_schema` values in `op_catalog.py` and nothing else. + 1. **Advertisement projection** (`advertisedToolSpecs`, two call sites). Where disclosure hooks. - Must not alter what execution/permission read. -2. **Private spec index** (`toolSpecsByName`). Must stay COMPLETE — the invoker looks the target - op up here; execution and the approval gate depend on it. -3. **Direct-call execution** (`relay.ts:318` → `direct.ts`). The invoker must reach this - unchanged, feeding the target op's private `call`. -4. **Permission decision** (`decide`/`effectivePermission`). The invoker must build the gate from - the TARGET op's spec, not from itself — otherwise `agenta_op` gates as one tool and writes - lose their approval prompt. -5. **Identifying disclosure-eligible specs.** A platform op is a `callback`-kind spec with a - direct `call`. So is a `reference` (workflow) tool (`direct.ts` header). There is no explicit - "this is a platform op" marker on `ResolvedToolSpec` today, so the runner cannot cleanly tell a - build-kit op from an author's reference tool without one. Two ways out (Open Question 4): - a heuristic (collapse all direct-`call` callback specs), or a small marker added by the platform - resolver (a wire add). -6. **Wire contract mirroring.** If Seam 5 uses a marker, `protocol.ts` + `wire.py` + goldens - change together (`services/runner/CLAUDE.md`, "The wire contract is mirrored"). + Must not alter what execution/permission read. Note it projects via `specInputSchema(spec)` + (`tools/spec-schema.ts:39`), the single camel/snake accessor — not `spec.inputSchema` directly. +2. **Private spec index** (`toolSpecsByName`, `public-spec.ts:34`). Must stay COMPLETE — the + invoker looks the target op up here; execution and the approval gate depend on it. +3. **Direct-call execution** (`relay.ts:318` → `direct.ts`). The invoker must reach this unchanged, + feeding the target op's private `call`. Note `executeAllowedRelayedTool` re-runs + `assertRequiredArguments(spec, req.args)` (`relay.ts:369`) — the invoker must pass the **target** + spec and the **unwrapped** args, or required-field validation silently degrades to the invoker's + loose schema. +4. **Permission decision — four sites, not one.** This is the project's dominant risk and has its + own document: **[security.md](security.md)**. In brief: the gates run *upstream of and + independently from* the relay execution path, each resolving a spec from the advertised name, in + `relay-guard.ts:53`, `acp-interactions.ts:516` (Claude), `acp-interactions.ts:456` (Pi, + fail-closed at `:473`), and `extensions/agenta.ts:318` (in-sandbox). Four inputs to + `effectivePermission` degrade at once — `readOnlyHint`, `specPermission`, name-matched policy + rules (`ruleMatches`, `permission-plan.ts:214`), and the `allow_reads` default + (`permission-plan.ts:248`). +5. **Identifying disclosure-eligible specs.** A platform op is a `callback`-kind spec with a direct + `call`. So is a `reference` (workflow) tool (`direct.ts` header). There is no explicit "this is a + platform op" marker on `ResolvedToolSpec` today, so the runner cannot cleanly tell a build-kit op + from an author's reference tool without one. Two ways out (Open Question 4): a heuristic + (collapse all direct-`call` callback specs), or a small marker added by the platform resolver + (a wire add). +6. **Wire contract mirroring.** If Seam 5 uses a marker, `protocol.ts` + `wire.py` + goldens change + together (`services/runner/CLAUDE.md`, "The wire contract is mirrored"). 7. **Client tools stay advertised.** `request_connection` / `request_input` must remain model-visible (the browser fulfils them) and are cheap; disclosure skips them. +8. **Harness-side schema validation.** Pi registers each tool with its real JSON Schema + (`registerTool({parameters: specInputSchema(spec)})`, `extensions/agenta.ts:305`). Both the diet + and the meta-toolset weaken this — the diet by shallowing the schema, the meta-toolset by making + `args` opaque. Bounded for the diet (the server does not validate the config shape either); + for the meta-toolset it must be recovered by re-validating against the target spec. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md new file mode 100644 index 0000000000..1433b2297e --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md @@ -0,0 +1,151 @@ +# Security — what collapsing 13 ops behind one tool name breaks + +**Verified 2026-07-26** against the runner on `main`. This is the risk register for the +**discovery meta-toolset** (Slice 3). It does **not** apply to the schema diet (Slices 1–2), which +changes no name, no spec, and no gate. + +Earlier drafts covered this in one bullet ("the invoker builds the gate from the target op's +spec"). That understates it by an order of magnitude, and it also puts the work in the wrong +place. The gates do **not** run inside the invoker's dispatch — they run *upstream of and +independently from* the relay execution path, on both harnesses. + +## Root cause: `toolName` is the discriminator for every permission decision + +Today each op arrives under its own name. That name resolves a private spec, and four separate +inputs to the decision ladder are read off it (`effectivePermission`, +`services/runner/src/permission-plan.ts:125`): + +``` +gate.specPermission (spec.permission) -> highest-priority override +gate.serverPermission (MCP server) -> n/a for platform ops +matchingRulePermission(gate, rules) -> matches on gate.toolName +defaultPermission(mode, gate) -> allow_reads: gate.readOnlyHint ? allow : ask +``` + +Collapse 13 ops behind one advertised name and `gate.toolName` stops discriminating. Every input +above degrades at once. + +### 1. `readOnlyHint` — the `allow_reads` default breaks (the core problem) + +`defaultPermission` (`permission-plan.ts:248`) is the whole policy for the default `allow_reads` +mode: + +```ts +if (mode === "allow_reads") return gate.readOnlyHint === true ? "allow" : "ask"; +``` + +`readOnlyHint` comes from `spec.readOnly`, which the catalog sets per op +(`op_catalog.py`: `query_spans` read-only, `commit_revision`/`remove_schedule` not). The invoker +is **one spec with one `readOnly` value covering thirteen ops**, so: + +- `readOnly: true` → **every write executes silently.** `commit_revision`, `remove_schedule`, + `remove_subscription` lose their approval prompt. This is the security regression. +- `readOnly: false` → **every read prompts.** `list_schedules` and `query_spans` now interrupt the + user. Habituated click-through is a worse outcome than not prompting at all. + +There is no correct static value. The gate must resolve `readOnly` from the *target* op per call. + +### 2. `specPermission` — same collapse, higher priority + +`spec.permission` short-circuits the whole ladder (`permission-plan.ts:129`). One invoker spec +means one permission for thirteen ops, and it wins over any rule. + +### 3. Policy rules stop matching — silent fall-through (not previously documented) + +`ruleMatches` (`permission-plan.ts:214`) compares the pattern against `gate.toolName` **exactly**, +or parses a `tool(prefix:*)` form that still requires `prefixPattern.toolName === gate.toolName`. + +Any operator- or config-authored rule written against an op name — `commit_revision: ask`, +`remove_schedule: deny` — **stops matching entirely** once the call arrives as `agenta_op`. It +does not error; it falls through to the default. A `deny` rule silently becomes whatever +`allow_reads` decides. Any rule authored before this change quietly stops being enforced. + +### 4. Grant / stored-decision keys — safe, but must stay consistent + +Both the turn's grant ledger (`ApprovedExecutionGrants`, `responder.ts:85`) and the replayed +approval store (`ConversationDecisions.take`, `responder.ts:255`) key on +`approvedCallKey(toolName, args)` — **name plus a canonical hash of the arguments**, not name +alone. + +Because the invoker's args contain the target op (`{op: "remove_schedule", args: {…}}`), distinct +ops produce distinct keys. **An approval for one op does not unlock another.** Granularity is +preserved for free. + +> Correction to an earlier review note: there is no cross-op approval-leak here. The args are part +> of the key. + +The residual requirement is **consistency**: `grant()` and `consume()` must key on the same name +(both the target's, or both the invoker's). Mismatch fails *closed* — every Pi write is refused +with "was not approved via the permission dialog" — which is a functional break, not a security +one, but it will look like a mysterious bug. + +## The four sites that must each resolve the target + +Each independently resolves a spec from a name and builds its own `GateDescriptor`. All four need +the same "if this is the invoker, re-resolve from `args.op`" logic. **Missing one does not fail +closed — it uses the invoker's permission, which is the fail-open direction if the invoker is +marked read-only.** + +| # | Site | Path | How it resolves today | +| --- | --- | --- | --- | +| 1 | `buildRelayExecutionGuard` | `engines/sandbox_agent/relay-guard.ts:53` | reads `spec.name` / `spec.permission` / `spec.readOnly` from the spec the relay looked up by `req.toolName` | +| 2 | `buildGateDescriptor` | `engines/sandbox_agent/acp-interactions.ts:516` | Claude/ACP dialog; `toolSpecsByName.get(bareToolName(displayName))` | +| 3 | `buildPiGateDescriptor` | `engines/sandbox_agent/acp-interactions.ts:456` | Pi dialog; **fails closed on an unknown name** (`:473`) — the invoker must be present in `piToolSpecsByName` | +| 4 | `piDialogAllows` call | `extensions/agenta.ts:318` | in-sandbox; passes `spec.name` and raw `params` up the gate envelope | + +Site 4 has an extra wrinkle: the in-sandbox extension only ever sees the **advertised** set +(`AGENTA_TOOL_PUBLIC_SPECS`). Under disclosure it no longer holds the target's `readOnly` or +`permission`, so it cannot decide `gateViaDialog` locally and must forward raw args for the runner +to classify. That is arguably *better* (the trust decision moves fully runner-side), but it is a +real behavior change on a path whose current design assumes the sandbox knows the tool it called. + +## What is *not* a new risk + +> Correction to an earlier review note: routing the decision through a sandbox-supplied `args.op` +> field is **not** a new trust exposure. + +The tool **name** is already sandbox-supplied today — the Pi gate envelope is written from inside +the sandbox, and `acp-interactions.ts:307` says so explicitly ("The envelope is sandbox-origin and +untrusted"). It is safe because the name is validated against a fixed map of known specs and fails +closed otherwise (`:473`). Reading `args.op` and validating it against the same map is the +identical pattern at the identical trust level. + +The real cost is **surface area**, not trust level: the validated-lookup pattern now has to be +duplicated correctly in four places instead of living in one. + +## Non-permission risks + +- **Loss of pre-call argument validation.** Pi registers each tool with its real JSON Schema + (`registerTool({parameters: specInputSchema(spec)})`, `extensions/agenta.ts:305`) and + `assertRequiredArguments` checks required fields (`:318`, and again in the relay at + `tools/relay.ts:369`). A generic invoker makes `args` an opaque blob, so both checks degrade to + the invoker's loose schema. Malformed op calls stop being caught before dispatch and surface as + server 4xx instead. Mitigated by having the invoker re-run `assertRequiredArguments` against the + **target** spec before execution — cheap, and it should be an explicit requirement. +- **Fetched schemas become history-resident.** A schema moved out of the prompt into a tool result + does not disappear — it sits in the conversation transcript for the rest of the session, in a + position far less cache-friendly than a stable tool-definition prefix. For a real build session + (as opposed to a "hi" turn), disclosure **without** the diet can cost more than today. This is a + strong argument for the diet landing first regardless. + +## Required test coverage before this ships + +Non-negotiable, per mutating op (`commit_revision`, `create_schedule`, `create_subscription`, +`remove_schedule`, `remove_subscription`, `annotate_trace`, `test_run`): + +1. `agenta_op` execute-mode produces the **same verdict** as calling the op directly, on **both** + harness paths — not just the relay guard. +2. Under `allow_reads`, read-only ops execute without a prompt and writes still prompt. +3. A policy rule written against an **op name** still matches through the invoker. +4. An approval granted for one op does **not** satisfy a later call to a different op. +5. `$ctx` bindings still fill server-side; the model cannot retarget another variant. +6. Describe-mode has no side effect, triggers no approval, and cannot execute. +7. An unknown or malformed `op` value fails **closed** on all four sites. + +## Recommendation + +The permission rework is the dominant cost and risk of this project, and it buys ~14% of the token +bill (see [baseline.md](baseline.md)). **Land the schema diet first and independently** — it +touches none of this. Only open the meta-toolset once the diet is measured, prompt caching is +understood, and there is a demonstrated reliability problem that fewer visible tools actually +fixes. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md index 056f08337e..871b5b026c 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md @@ -1,44 +1,82 @@ # Status -**Last updated:** 2026-07-20 +**Last updated:** 2026-07-26 ## Current stage -Planning workspace drafted (context, research, design, plan). No code written, nothing -committed. Ready for team review of the open implementation questions below. Research is -grounded in the current runner code (`file:line` in research.md); the token numbers are from the -2026-07-17 investigation and are explicitly flagged for a Slice 0 re-baseline. +Planning workspace revised after a re-baseline and a code-grounded security review. No code +written, nothing committed. + +**What changed on 2026-07-26.** The token cost was re-measured against the live catalog +([baseline.md](baseline.md)) and the permission plane was traced end to end +([security.md](security.md)). Both moved the plan: + +1. The bill is **18,353** tokens, not ~15,454 (the 2026-07-17 figures were ~19% low). +2. The cost is **concentrated, not spread**: three ops are 88%, and one schema object embedded + twice is 70%. The ten-op tail is 2,120 tokens combined. +3. Therefore the **schema diet is the project** (~84% of the win, no runner change), and the + meta-toolset is a follow-up worth ~14% more. +4. The meta-toolset's permission work is a **four-site problem in fail-closed security code**, not + the one-line "gate on the target spec" the earlier draft described. ## Locked decisions -- Seam = the runner advertisement layer (`advertisedToolSpecs`, `services/runner/src/tools/public-spec.ts`), - consumed at `pi-assets.ts:353` and `environment.ts:721`. Execution + permission stay below it. -- Approach = a discovery meta-toolset (`agenta_ops` + `agenta_op`) that keeps op specs private in - runner memory and moves schemas from the prompt into on-demand tool results. -- Skills, op-catalog contents, external `discover_tools`, and non-platform tool types are out of - scope. +- **Diet first, alone, and it is the committed delivery** (plan Slices 1–2). Near-zero risk: the + replacement guidance (`references/config-schema.md`) already ships with the skill, the skill + already mandates reading it before `commit_revision`, and the commit endpoint does not validate + the config shape anyway. +- **Shallow schema, not open object.** Keep top-level keys with one-line descriptions; stop before + expanding nested `$defs`. Prefer a depth limit on the expansion so it is reusable. +- **Meta-toolset deferred behind an evidence gate**, not cancelled. Gate: post-diet numbers + + a caching answer + demonstrated wander evidence. Compare M2 head-to-head at that point. +- **Op-set curation dropped** (not deferred). Hiding the 5-op event pack saves ~1,052 tokens (~5%) + and cannot be decided correctly at run start — a user pivoting to "schedule this daily" + mid-conversation would find the capability gone. Not worth a capability regression. +- **Success metric is session-level, not a no-op turn.** The "hi" turn is reported, never targeted. +- Seam for the meta-toolset = the runner advertisement layer (`advertisedToolSpecs`, + `public-spec.ts:57`), consumed at `pi-assets.ts:353` and `environment.ts:721`. Verified + 2026-07-26. +- Skills, external `discover_tools`, and non-platform tool types stay out of scope. - Playground overlay only — no saved/committed-agent change. -- Schema diet is complementary and lands as its own slice (Slice 1). -- Ship behind a flag, default off, until measured. - No commits during planning; implementation is a later, separate branch. -## Open implementation questions - -1. **Invoker shape** — one `agenta_op` with describe-vs-execute mode, or two tools - (`describe_op` + `call_op`)? *Recommendation: one tool* (smaller advertised surface; the mode - is clear from presence of `args` or an explicit `mode`). -2. **Permission fidelity** — confirm `agenta_op` execute-mode builds the gate from the TARGET - op's private spec and runs the existing `decide()`, rather than gating the invoker as a single - tool. *Recommendation: yes, per-op* — otherwise `commit_revision` and other writes lose their - approval prompt. Non-negotiable in the reviewer's eyes IMO, but calling it out explicitly. -3. **Catalog summary source** — build `{op, one_line, read_only}` runner-side from the resolved - specs (zero new wire fields) vs. thread it from `op_catalog.py`. *Recommendation: runner-side.* -4. **Disclosure-eligible identification** — heuristic (collapse all direct-`call` callback specs) - vs. a `source:"platform"` marker on `ResolvedToolSpec` (a `protocol.ts` + `wire.py` + golden - add). *Recommendation: heuristic for the flagged POC; marker before default-on.* -5. **Sequencing** — do Slice 1 (schema diet) and ship it independently first, or hold it and land - the whole thing together? *Recommendation: ship Slice 1 first* — it is ~11K of the win at - near-zero risk and is useful even if the POC needs iteration. -6. **Default-on criteria** — what pass-rate / token target gates flipping the flag (Slice 3)? - Needs a number from the team (e.g. "no regression on the lab matrix and ≥90% token cut on a - no-op turn"). \ No newline at end of file +## Corrections to earlier drafts + +Recorded so the reasoning is not re-litigated: + +- **Approval leak across ops — not real.** Both the grant ledger and the replayed decision store key + on `approvedCallKey(toolName, args)`, i.e. name **plus** a canonical args hash. Since the invoker's + args carry the op name, distinct ops produce distinct keys. Granularity survives for free. +- **Sandbox-supplied `args.op` — not a new trust exposure.** The tool *name* is already + sandbox-supplied today (`acp-interactions.ts:307`, "the envelope is sandbox-origin and + untrusted") and is safe because it is validated against a fixed spec map. Reading `args.op` and + validating it the same way is the same pattern at the same trust level. The real cost is + duplicating that validated lookup correctly in four places. +- **New finding not in the first draft:** name-matched **policy rules** silently stop matching. + `ruleMatches` (`permission-plan.ts:214`) compares `gate.toolName` exactly, so any rule written + against an op name (`commit_revision: ask`, `remove_schedule: deny`) falls through to the default + once calls arrive as `agenta_op`. It does not error. + +## Open questions + +**Blocking Slice 3 only — none block the diet.** + +1. **Prompt caching (blocking the Slice 3 decision).** Are these tokens billed every turn, or cached + after the first? Nothing in the runner sets or inspects cache behavior. This sets the ROI of + everything past the diet. *Owner: needs a measurement on a real run.* +2. **Live advertised set.** Does `test_run` advertise with `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` + off (default)? If not, the live bill is 10,576 and Slice 1's win is ~6,900, not ~12,900. +3. **Wander evidence.** Is there measured evidence that tool *count* causes run failures? The + internal-tools review asserts it; the meta-toolset's whole remaining case rests on it. +4. **M2 vs Lever B.** M2 (dynamic real-name advertisement) keeps real names in front of every gate + and so dissolves most of security.md. Is its mid-session re-registration cheaper than four-site + gate surgery? Compare at the decision point, not after building Lever B. +5. **Disclosure-eligible identification** (Slice 3 only) — heuristic vs a `source:"platform"` marker. + *Recommendation: heuristic for the flagged POC; marker before default-on.* +6. **Invoker shape** (Slice 3 only) — one `agenta_op` with describe/execute modes, or two tools? + *Recommendation: one.* + +## Next action + +Answer open questions 1 and 2 (both are measurements on a real run, both cheap), then start +**Slice 1** — it is not blocked by either. From 7c56fe48e7c65a5a30533d2f2d757876d49a6e81 Mon Sep 17 00:00:00 2001 From: ashrafchowdury Date: Sun, 26 Jul 2026 23:40:41 +0600 Subject: [PATCH 3/5] Update documentation for progressive tool disclosure project: clarify token costs, correct handler mode defaults, and refine slice implementation details. --- .../progressive-tool-disclosure/baseline.md | 23 ++++-- .../progressive-tool-disclosure/context.md | 5 +- .../progressive-tool-disclosure/design.md | 74 ++++++++++++----- .../progressive-tool-disclosure/plan.md | 74 ++++++++++------- .../progressive-tool-disclosure/research.md | 50 ++++++++---- .../progressive-tool-disclosure/security.md | 80 +++++++++++++++---- .../progressive-tool-disclosure/status.md | 54 +++++++++---- 7 files changed, 260 insertions(+), 100 deletions(-) diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md index b972bb6fd5..9a794861fd 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md @@ -12,7 +12,7 @@ stripped), matching `advertisedToolSpec()` in `services/runner/src/tools/public- | op | total | schema | descr | note | | --- | ---: | ---: | ---: | --- | -| `test_run` | 7,777 | 7,593 | 163 | handler-gated; write | +| `test_run` | 7,777 | 7,593 | 163 | handler-mode (`callRef`), advertised by default; write | | `commit_revision` | 6,878 | 6,713 | 147 | write | | `query_spans` | 1,578 | 1,463 | 96 | read | | `create_schedule` | 425 | 351 | 56 | write | @@ -34,10 +34,23 @@ stripped), matching `advertisedToolSpec()` in `services/runner/src/tools/public- 2. **One schema object is 70% of the bill.** `_build_agent_template_delta_schema()` (`sdks/python/agenta/sdk/agents/platform/op_catalog.py:317`) is **6,441 tokens** and is embedded **twice** — in `commit_revision` and in `test_run`. That is 12,882 of 18,353. -3. **`test_run` may not be live.** It is handler-based (`handler="tools.agenta.test_run"`, no - `method`/`path`) and gated by `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off - (`sdks/python/agenta/sdk/agents/platform/platform_tools.py:36`). With handlers off the live - advertised cost is **10,576**, of which `commit_revision` alone is 65%. +3. **All 13 ops are live by default — 18,353 is the real number.** `test_run` is handler-based + (`handler="tools.agenta.test_run"`, no `method`/`path`) and gated by + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, but that flag **defaults ON** + (`sdks/python/agenta/sdk/agents/platform/platform_tools.py:41`): + + ```python + if value is None: return True # unset -> ENABLED + return value.strip().lower() not in _DISABLED_ENV_VALUES + ``` + + Unset *and* empty both mean enabled; the resolver skips the op only for an explicit `off` / + `false` / `0` (`_DISABLED_ENV_VALUES`). The resolver's own log line says "explicitly disabled". + + > **Correction.** Drafts before 2026-07-26 (and the 2026-07-17 investigation they came from) + > claimed "default off" and carried a 10,576-token alternate figure. That was wrong. There is no + > alternate figure: `test_run` advertises unless someone opts out, so its 7,777 tokens — 42% of + > the bill — are real, and Slice 1's win is the full ~12,900. ## Consequence for slicing diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md index 342c9bab3f..3408e1d67f 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md @@ -12,8 +12,9 @@ Two costs, not one: - **Tokens.** And they are *concentrated*, not spread. `test_run` (7,777) + `commit_revision` (6,878) + `query_spans` (1,578) are **88%** of the bill; the other ten ops are 2,120 tokens - combined. One schema object — `_build_agent_template_delta_schema()`, 6,441 tokens, embedded in - **both** `test_run` and `commit_revision` — is **70%** on its own. + combined. One schema object — `_build_agent_template_delta_schema()`, 6,441 tokens — is embedded + in **both** `test_run` and `commit_revision`, so its two copies are 12,882 tokens: **70% of the + bill is one object counted twice.** - **Reliability.** The internal-tools review (`../builder-agent-reliability/tools-review/part-2-internal-tools.md`) found the same tools are a *double* cost: "each unused tool is context cost plus a wander target (the capstone showed extra diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md index b260ca073b..af76267eba 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md @@ -68,7 +68,10 @@ Stop advertising N op schemas. Advertise a small fixed meta-toolset; keep the op **private** in runner memory; move each op's schema from the *prompt* into a *tool result*. - **`agenta_ops(query?)`** — the op catalog as a compact list: `{op, one_line, read_only}`, no input - schemas. A few hundred tokens, flat regardless of catalog size. + schemas. The **advertised prompt cost is flat** regardless of catalog size — that is the win. The + *result* is O(n) in the catalog (~200 tokens at today's 13 ops, ~15/op), so it must be bounded: + `query` filters, results are capped at a hard limit, and the response states when it truncated. + Without a cap, a large catalog reintroduces the same cost one layer down. - **`agenta_op(op, args?)`** — the generic invoker. No `args` (or `mode:"describe"`) returns one op's `inputSchema` as a tool result; with `args` it executes that op. @@ -88,39 +91,74 @@ is the pattern Claude Code uses on its own tools: names listed, schema loaded on conversation, in a far less cache-friendly position than a stable tool-definition prefix. Without Lever A first, disclosure can cost *more* across a real build session. +## One shared target resolver (required) + +Four separate sites need to answer "which op is this really?", and a site that gets it wrong fails +**open** (see [security.md](security.md)). Four independent implementations of a security-sensitive +lookup is the wrong shape. + +**Requirement:** define exactly one runner-side primitive — +`resolveDisclosedTarget(toolName, args, specsByName)` — that + +1. returns `undefined` unless `toolName` is the invoker (so non-disclosed calls are untouched), +2. validates `args.op` against the same fixed spec map every gate already uses, returning + `undefined` (→ fail closed) for anything unknown or malformed, +3. returns the canonical `{spec, gateName, args}` triple the caller builds its `GateDescriptor` + from. + +All four gate sites and the invoker's dispatch consume that one result. No site re-implements the +lookup. This is what makes "resolve the target" a single reviewable, single-testable change instead +of four chances to fail open. + ## Execution path for `agenta_op(op, args)` -1. **Resolve the target.** Look up `op` in `toolSpecsByName`. Unknown → tool error listing valid - ops (recoverable), and fail closed at every gate. +1. **Resolve the target** via the shared primitive above. Unknown → tool error listing valid ops + (recoverable), and fail closed at every gate. 2. **Describe mode** → return the target's resolved input schema. No side effects, no approval. -3. **Execute mode** → resolve the permission gate from the **target** op's spec at all four sites - (see security.md), run `decide()`, then `assertRequiredArguments(target, innerArgs)` followed by - `assembleBody` → `directCallUrl` → `callDirect`, exactly as `executeRelayedTool` does now. -4. **Return** the endpoint response verbatim. +3. **Execute mode** → build the permission gate from the **target** spec at all four sites, run + `decide()`, then `assertRequiredArguments(target, innerArgs)`. +4. **Dispatch on the target's shape — there are two.** Reuse the `executeRelayedTool` core rather + than reimplementing either branch: + - **Endpoint-mode** (12 ops, direct `call`) → `assembleBody` → `directCallUrl` → `callDirect`. + - **Handler-mode** (`test_run`, `callRef`) → `applyContextBindings` → `callAgentaTool`. + + An invoker that forwards only `spec.call` cannot run `test_run` at all. +5. **Return** the response verbatim. ## Why execution itself is safe by construction -Execution reads the **private** spec (research seams 2–3), so feeding the target's private `call` -into the unchanged `direct.ts` path preserves: +Execution reads the **private** spec (research seams 2–3), so dispatching the target through its +own unchanged execution branch preserves: -- **Self-targeting.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so +- **Self-targeting, endpoint-mode.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so `commit_revision` still binds `$ctx.workflow.variant.id`; the model cannot retarget. Those fields were stripped at resolve time and are re-applied below the invoker. -- **SSRF guard.** `directCallUrl` host-locks to the run's Agenta origin. +- **Self-targeting, handler-mode.** `test_run` binds `target.workflow_variant_id` through + `applyContextBindings` on the `callAgentaTool` branch — a different mechanism, equally below the + invoker. Both branches must be covered by the `$ctx` test in security.md, not just the first. +- **SSRF guard.** `directCallUrl` host-locks to the run's Agenta origin (endpoint-mode); + `callAgentaTool` posts back through Agenta's own `/tools/call` so the secret stays server-side + (handler-mode). Permission is the part that is **not** free. It is designed in, not inherited — see security.md. ## Identifying the disclosure-eligible set -A platform op is a `callback`-kind spec with a direct `call`; so is a `reference` (workflow) tool. -No explicit marker exists today (research seam 5). +**Revised 2026-07-26.** The earlier draft recommended a "collapse every direct-`call` callback spec" +heuristic for the POC. That rule is wrong: platform ops do not share one shape (research seam 5). + +- `test_run` is **handler-mode** — it resolves to a `callRef`, not a `call` (`op_catalog.py:175`). + A direct-`call` heuristic **skips it**, leaving 7,777 tokens (42% of the bill, the largest single + target) advertised. The POC would miss most of its own goal. +- Conversely, an author's `reference` tool carries a direct `call` and a gateway tool carries a + `callRef`, so both fields over-collapse in the other direction. -- **Heuristic (zero wire change).** Collapse every direct-`call` callback spec. Risk: an author's - `reference` tool gets disclosed too. Acceptable for a flagged POC. -- **Marker (small wire add).** The platform resolver stamps `source:"platform"`; the runner - collapses exactly that group. Costs `protocol.ts` + `wire.py` + goldens. +**Decision: use the marker.** The platform resolver stamps `source:"platform"` (or a `disclosable` +group tag) on the resolved spec; the runner collapses exactly that group. Costs a `protocol.ts` + +`wire.py` + golden change — small, and it is the only rule that is actually correct. It also lets us +disclose gateway/reference groups later on purpose rather than by accident. -Recommendation: heuristic for the POC, marker before default-on. +The heuristic is retained only as a documented non-option, so it is not re-proposed. ## The catalog summary (`agenta_ops`) diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md index ef42436bf7..68f0f7cb75 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md @@ -13,22 +13,26 @@ mechanism is a separately-justified follow-up. - Playground platform ops only; no saved-agent change; no committed-agent behavior change. - Each slice leaves the tree working and testable. -## Slice 0 — Baseline ✅ tokens done, 2 gaps open +## Slice 0 — Baseline ✅ done (does not block Slice 1) 1. ~~Measurement script + per-op table.~~ **Done** — [baseline.md](baseline.md), measured 2026-07-26: 18,353 total, top 3 ops = 88%, one duplicated schema object = 70%. -2. **OPEN — confirm the live advertised set.** `test_run` is handler-gated - (`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, default off). Resolve the overlay in a real run and - record whether it advertises. If not, the live figure is 10,576 and `commit_revision` alone is - 65% of it. -3. **OPEN — answer prompt caching.** Nothing in the runner sets or inspects cache behavior; the - harnesses own it. Determine whether these tokens are billed per turn or only on the first. This - directly sets the ROI of Slice 3 and should be answered before it is scheduled. -4. Add a runner unit test asserting today's behavior: every resolved platform op appears in - `advertisedToolSpecs(plan.toolSpecs)`. The invariant Slice 3 would intentionally flip. - -**Exit:** live-set and caching questions answered in baseline.md; the "all platform ops advertised -today" test passes on `main`. +2. ~~Confirm the live advertised set.~~ **Done — all 13 ops advertise by default.** + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` **defaults ON** (unset and empty both enable; + `platform_tools.py:41`), so `test_run` is live and 18,353 is the real figure. Earlier drafts said + "default off" with a 10,576 alternate — deleted as wrong. +3. **OPEN — answer prompt caching (blocks the Slice 3 decision only, not Slice 1).** Nothing in the + runner sets or inspects cache behavior; the harnesses own it. Determine whether these tokens are + billed per turn or only on the first. +4. Add a **resolution-path** test, not a projection test. `advertisedToolSpecs` is a pure + `specs.map(...)`, so "every input appears in the output" is a tautology that can never detect a + missing op. Instead exercise platform resolution/materialization with + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` explicitly **on** and explicitly **off**, and assert the + resulting live advertised set (13 ops with it on/unset, 12 with it disabled). That is the + invariant Slice 3 would intentionally flip. + +**Exit:** items 1–2 recorded in baseline.md ✅; the resolution-path test passes on `main`. Item 3 is +tracked to the Slice 3 decision point and is **not** a gate on Slices 1–2. ## Slice 1 — Diet the duplicated agent-template schema (the main win) @@ -71,26 +75,39 @@ If those do not hold, close the project after Slice 2 and record why. ## Slice 3 — Discovery meta-toolset (only if the gate above opens) -Flagged, default off. +Behind a new disclosure feature flag, default off. (Unrelated to +`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, which defaults *on* — see Slice 0.) 1. Add the flag/env that turns disclosure on for a run. -2. Disclosure transform at the two advertisement call sites (`pi-assets.ts:353`, - `environment.ts:721`): replace disclosure-eligible specs with `agenta_ops` + `agenta_op`; keep - client tools and everything else advertised. `plan.toolSpecs` / `toolSpecsByName` stay complete. - Register the invoker into `piToolSpecsByName` too, or the Pi gate fails closed on it. -3. **Permission work — the bulk of this slice.** Teach all four gate sites to resolve the target - from `args.op` and validate it against the known spec map: +2. **Add the `source:"platform"` marker** — the eligibility rule, and a prerequisite, not a + follow-up. The platform resolver stamps it; `protocol.ts` + `wire.py` + goldens change together + (research seam 6). The previously-planned "collapse all direct-`call` callback specs" heuristic + is **rejected**: `test_run` is handler-mode (`callRef`), so the heuristic would skip the single + largest target while over-collapsing an author's reference tools (design.md, "Identifying the + disclosure-eligible set"). +3. Build the **shared resolver** `resolveDisclosedTarget(toolName, args, specsByName)` first, with + its own unit tests (security.md, "Required: one shared resolver"). Everything below consumes it; + no site re-implements the lookup. +4. Disclosure transform at the two advertisement call sites (`pi-assets.ts:353`, + `environment.ts:721`): replace marked specs with `agenta_ops` + `agenta_op`; keep client tools + and everything else advertised. `plan.toolSpecs` / `toolSpecsByName` stay complete. Register the + invoker into `piToolSpecsByName` too, or the Pi gate fails closed on it. +5. **Permission work — the bulk of this slice.** Wire the shared resolver into all four gate sites: `relay-guard.ts:53`, `acp-interactions.ts:516` (Claude), `acp-interactions.ts:456` (Pi), - `extensions/agenta.ts:318` (in-sandbox). Keep `grant()`/`consume()` keying consistent. Read + `extensions/agenta.ts:318` (in-sandbox). Each must carry the **target's** name into the gate so + op-name policy rules keep matching, and `grant()`/`consume()` must key consistently. Read [security.md](security.md) before writing any of it. -4. Invoker dispatch: `agenta_ops` returns `{op, one_line, read_only}` built runner-side; - `agenta_op` describe-mode returns one op's schema; execute-mode re-runs - `assertRequiredArguments` against the **target** spec, then reuses the `executeRelayedTool` core. -5. Tests — the full list in security.md ("Required test coverage"), per mutating op, on **both** +6. Invoker dispatch: `agenta_ops` returns a **bounded, filterable** `{op, one_line, read_only}` list + built runner-side (capped, and it says so when truncated); `agenta_op` describe-mode returns one + op's schema; execute-mode re-runs `assertRequiredArguments` against the **target** spec, then + dispatches on the target's shape — `call` → the direct branch, `callRef` → `callAgentaTool` — + reusing the `executeRelayedTool` core for both. +7. Tests — the full list in security.md ("Required test coverage"), per mutating op, on **both** harness paths. Minimum: same verdict as a direct call; `allow_reads` still distinguishes read from write; name-based policy rules still match; approvals do not cross ops; `$ctx` still binds - server-side; describe-mode is inert; unknown `op` fails closed everywhere. -6. Add the one-line nudge to the `build-an-agent` skill. + server-side on **both** execution branches; `test_run` is disclosed *and* executable; + describe-mode is inert; unknown `op` fails closed everywhere. +8. Add the one-line nudge to the `build-an-agent` skill. **Exit:** flag on → a lab run completes discover → wire → commit → schedule using only the meta-tools; every test in security.md passes; `tsc` + `pnpm test` green in `services/runner`. @@ -107,6 +124,7 @@ meta-tools; every test in security.md passes; `tsc` + `pnpm test` green in `serv ## Not in this plan - **Op-set curation** — dropped (capability regression for ~5%; see design.md alternatives). -- Marker-based eligibility (Slice 3 uses the heuristic). +- The direct-`call` eligibility heuristic — rejected as incorrect (misses handler-mode `test_run`); + the marker moved *into* Slice 3 as a prerequisite. - Disclosing gateway/code/client/MCP tools. - M2 implementation — but it must be *compared* at the decision point. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md index 7115c03192..a21dcb633e 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md @@ -74,7 +74,7 @@ were ~19% low; the catalog grew. | Op | Tokens | Note | | --- | ---: | --- | -| `test_run` | 7,777 | embeds the 6,441-token agent-template delta schema; handler-gated | +| `test_run` | 7,777 | embeds the 6,441-token agent-template delta schema; handler-mode (`callRef`) | | `commit_revision` | 6,878 | embeds the **same** 6,441-token delta schema | | `query_spans` | 1,578 | filtering DSL `$defs` | | others (10 ops) | 2,120 | combined; 7 of them under 400 each | @@ -87,10 +87,17 @@ Two findings reorder the whole project: - **Duplication.** `_build_agent_template_delta_schema()` (`op_catalog.py:317`) is 6,441 tokens and is embedded twice — 12,882 of 18,353, i.e. **70% of the bill is one object counted twice.** -`test_run` is handler-based (`handler="tools.agenta.test_run"`) and gated by -`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` (default off, `platform_tools.py:36`), so the live -advertised cost may be 10,576 — in which case `commit_revision` alone is 65% of it. Confirm which -set advertises live before quoting a headline number. +All 13 ops advertise by default, so 18,353 is the live figure. `test_run` is handler-based and +gated by `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, but that flag **defaults ON** — unset and empty +both resolve to enabled, and the resolver skips the op only for an explicit `off`/`false`/`0` +(`platform_tools.py:41`). Earlier drafts said "default off" and carried an alternate 10,576 figure; +that was wrong (see [baseline.md](baseline.md)). + +**Handler-mode ops take a different execution shape.** `test_run` is the only handler-mode op +(`_HANDLER_CALL_REFS`, `op_catalog.py:58`). It resolves via `to_call_ref()` to a **`callRef`**, and +`to_call()` explicitly raises for it (`op_catalog.py:175`). So a platform op is *not* always a +direct-`call` spec — which breaks the naive disclosure heuristic (seam 5) and the invoker's +execution path (seam 3). ### The diet's replacement already exists @@ -111,11 +118,14 @@ changes `input_schema` values in `op_catalog.py` and nothing else. (`tools/spec-schema.ts:39`), the single camel/snake accessor — not `spec.inputSchema` directly. 2. **Private spec index** (`toolSpecsByName`, `public-spec.ts:34`). Must stay COMPLETE — the invoker looks the target op up here; execution and the approval gate depend on it. -3. **Direct-call execution** (`relay.ts:318` → `direct.ts`). The invoker must reach this unchanged, - feeding the target op's private `call`. Note `executeAllowedRelayedTool` re-runs - `assertRequiredArguments(spec, req.args)` (`relay.ts:369`) — the invoker must pass the **target** - spec and the **unwrapped** args, or required-field validation silently degrades to the invoker's - loose schema. +3. **Execution — TWO shapes, not one** (`relay.ts:318` → `executeAllowedRelayedTool`, + `relay.ts:361`). Endpoint-mode ops carry a direct `call` and go through + `assembleBody` → `directCallUrl` → `callDirect`. Handler-mode ops (`test_run`) carry a + **`callRef`** instead and fall through to `callAgentaTool` with `applyContextBindings`. An + invoker that only forwards `spec.call` cannot execute `test_run` at all. Note also that + `executeAllowedRelayedTool` re-runs `assertRequiredArguments(spec, req.args)` (`relay.ts:369`) — + the invoker must pass the **target** spec and the **unwrapped** args, or required-field + validation silently degrades to the invoker's loose schema. 4. **Permission decision — four sites, not one.** This is the project's dominant risk and has its own document: **[security.md](security.md)**. In brief: the gates run *upstream of and independently from* the relay execution path, each resolving a spec from the advertised name, in @@ -124,12 +134,20 @@ changes `input_schema` values in `op_catalog.py` and nothing else. `effectivePermission` degrade at once — `readOnlyHint`, `specPermission`, name-matched policy rules (`ruleMatches`, `permission-plan.ts:214`), and the `allow_reads` default (`permission-plan.ts:248`). -5. **Identifying disclosure-eligible specs.** A platform op is a `callback`-kind spec with a direct - `call`. So is a `reference` (workflow) tool (`direct.ts` header). There is no explicit "this is a - platform op" marker on `ResolvedToolSpec` today, so the runner cannot cleanly tell a build-kit op - from an author's reference tool without one. Two ways out (Open Question 4): a heuristic - (collapse all direct-`call` callback specs), or a small marker added by the platform resolver - (a wire add). +5. **Identifying disclosure-eligible specs — harder than it looks.** Platform ops do **not** share + one shape: endpoint-mode ops carry a direct `call`, handler-mode `test_run` carries a `callRef`. + Meanwhile a `reference` (workflow) tool *also* carries a direct `call`, and a gateway (Composio) + tool *also* carries a `callRef`. So neither field identifies a platform op: + + | | direct `call` | `callRef` | + | --- | --- | --- | + | **platform op** | 12 endpoint-mode ops | `test_run` | + | **not a platform op** | author's `reference` tool | gateway (Composio) tool | + + A "collapse all direct-`call` callback specs" heuristic therefore **misses `test_run`** — 7,777 + tokens, 42% of the bill, the single largest target — while over-collapsing an author's reference + tools. There is no explicit "this is a platform op" marker on `ResolvedToolSpec` today. See + Open Question 4; the marker is now the recommended route, not the fallback. 6. **Wire contract mirroring.** If Seam 5 uses a marker, `protocol.ts` + `wire.py` + goldens change together (`services/runner/CLAUDE.md`, "The wire contract is mirrored"). 7. **Client tools stay advertised.** `request_connection` / `request_input` must remain diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md index 1433b2297e..e14ae26ebe 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md @@ -11,21 +11,27 @@ independently from* the relay execution path, on both harnesses. ## Root cause: `toolName` is the discriminator for every permission decision -Today each op arrives under its own name. That name resolves a private spec, and four separate -inputs to the decision ladder are read off it (`effectivePermission`, -`services/runner/src/permission-plan.ts:125`): - -``` -gate.specPermission (spec.permission) -> highest-priority override -gate.serverPermission (MCP server) -> n/a for platform ops -matchingRulePermission(gate, rules) -> matches on gate.toolName -defaultPermission(mode, gate) -> allow_reads: gate.readOnlyHint ? allow : ask +Today each op arrives under its own name, and that name does two jobs: it **resolves the private +spec** the gate reads its fields from, and it is **itself matched** by policy rules. The decision +ladder (`effectivePermission`, `services/runner/src/permission-plan.ts:125`) consumes both: + +```text +gate.specPermission (from resolved spec) -> short-circuits the rest when set +gate.serverPermission (MCP server) -> n/a for platform ops +matchingRulePermission(gate, rules) -> matches on gate.toolName itself +defaultPermission(mode, gate) -> allow_reads: gate.readOnlyHint ? allow : ask ``` -Collapse 13 ops behind one advertised name and `gate.toolName` stops discriminating. Every input -above degrades at once. +**The regression is not disclosure itself — it is building the gate from the invoker's spec instead +of re-resolving the target.** Get that right at every site and each input above keeps working; +`gate.toolName` restored to the target name even keeps existing op-name rules matching. Get it wrong +at any one site and that site silently evaluates thirteen ops against one spec. + +The sections below are therefore **failure modes to design against**, not inevitable outcomes. They +are listed because each has a distinct fix and a distinct test, and because the fix has to land in +four places (see "The four sites"). -### 1. `readOnlyHint` — the `allow_reads` default breaks (the core problem) +### 1. `readOnlyHint` — the `allow_reads` default (the core failure mode) `defaultPermission` (`permission-plan.ts:248`) is the whole policy for the default `allow_reads` mode: @@ -55,10 +61,16 @@ means one permission for thirteen ops, and it wins over any rule. `ruleMatches` (`permission-plan.ts:214`) compares the pattern against `gate.toolName` **exactly**, or parses a `tool(prefix:*)` form that still requires `prefixPattern.toolName === gate.toolName`. -Any operator- or config-authored rule written against an op name — `commit_revision: ask`, -`remove_schedule: deny` — **stops matching entirely** once the call arrives as `agenta_op`. It -does not error; it falls through to the default. A `deny` rule silently becomes whatever -`allow_reads` decides. Any rule authored before this change quietly stops being enforced. +This one is unique in the list: it keys on the **name itself**, not on the resolved spec. Any +operator- or config-authored rule written against an op name — `commit_revision: ask`, +`remove_schedule: deny` — **stops matching** if the call reaches the ladder still named `agenta_op`. +It does not error; it falls through to the default, so a `deny` rule silently becomes whatever +`allow_reads` decides. + +**Fix:** set `gate.toolName` to the *target* op's name (not the invoker's) before `decide()` runs. +Then existing rules keep matching unchanged and no operator has to rewrite a policy. That +substitution has to happen at all four sites, which is exactly why the shared resolver below +returns a `gateName` rather than leaving each site to name the gate itself. ### 4. Grant / stored-decision keys — safe, but must stay consistent @@ -99,6 +111,33 @@ Site 4 has an extra wrinkle: the in-sandbox extension only ever sees the **adver to classify. That is arguably *better* (the trust decision moves fully runner-side), but it is a real behavior change on a path whose current design assumes the sandbox knows the tool it called. +## Required: one shared resolver, not four implementations + +Four copies of a security-sensitive lookup, where a missed copy fails **open**, is the wrong shape. +The mitigation is structural, and it is a hard requirement of the design rather than an +implementation detail. + +Define exactly one runner-side primitive and have all four sites consume it: + +```ts +resolveDisclosedTarget(toolName, args, specsByName) + -> { spec, gateName, args } | undefined +``` + +- Returns `undefined` when `toolName` is not the invoker, so every non-disclosed call keeps its + current path byte-for-byte. +- Validates `args.op` against the **same fixed spec map** the gates already trust, returning + `undefined` for unknown or malformed values — so an unresolvable target fails closed everywhere, + by construction rather than by four separate `if` statements. +- Returns `gateName` = the **target op's** name, so the caller's `GateDescriptor` carries it into + `ruleMatches` and existing op-name policy rules keep matching (failure mode 3). +- Returns the **unwrapped** inner args, so `assertRequiredArguments` and the grant/decision keys all + operate on the same canonical value at every site (failure mode 4's consistency requirement). + +With this in place, "resolve the target" becomes one reviewable function with one test suite, and +the four sites reduce to a two-line call each. Without it, the review surface is four hand-written +lookups in code paths that are individually hard to exercise. + ## What is *not* a new risk > Correction to an earlier review note: routing the decision through a sandbox-supplied `args.op` @@ -138,9 +177,16 @@ Non-negotiable, per mutating op (`commit_revision`, `create_schedule`, `create_s 2. Under `allow_reads`, read-only ops execute without a prompt and writes still prompt. 3. A policy rule written against an **op name** still matches through the invoker. 4. An approval granted for one op does **not** satisfy a later call to a different op. -5. `$ctx` bindings still fill server-side; the model cannot retarget another variant. +5. `$ctx` bindings still fill server-side and the model cannot retarget another variant — on + **both** execution branches: `assembleBody`/`call.context` for endpoint-mode ops, and + `applyContextBindings` for handler-mode `test_run`. A test that only covers `commit_revision` + misses half the mechanism. 6. Describe-mode has no side effect, triggers no approval, and cannot execute. 7. An unknown or malformed `op` value fails **closed** on all four sites. +8. `test_run` (the only handler-mode op) is disclosed *and* executable through the invoker — + guarding against the direct-`call`-only eligibility and dispatch bugs that the first draft had. +9. `resolveDisclosedTarget` is unit-tested directly against the failure modes above, so the four + call sites only need a thin integration assertion each. ## Recommendation diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md index 871b5b026c..39305e9661 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md @@ -55,28 +55,54 @@ Recorded so the reasoning is not re-litigated: - **New finding not in the first draft:** name-matched **policy rules** silently stop matching. `ruleMatches` (`permission-plan.ts:214`) compares `gate.toolName` exactly, so any rule written against an op name (`commit_revision: ask`, `remove_schedule: deny`) falls through to the default - once calls arrive as `agenta_op`. It does not error. + once calls arrive as `agenta_op`. It does not error. *Fix: carry the target's name into the gate, + so existing rules keep matching and no operator rewrites a policy.* +- **"Disclosure inevitably breaks permission" — overstated.** Every failure mode in security.md is + fixable by re-resolving the target; the risk is that the fix must land correctly in four + fail-closed sites, not that the approach is unsound. security.md now frames them as failure modes + to design against, and requires a single shared resolver so there is one implementation instead of + four. *(Raised by CodeRabbit, 2026-07-26.)* +- **Handler flag "default off" — wrong, corrected.** It defaults **on**; see "Closed since the last + revision" below. *(Raised by CodeRabbit, 2026-07-26.)* +- **Direct-`call` eligibility heuristic — wrong, rejected.** It misses handler-mode `test_run`, the + largest single target; the marker is now the rule. See below. *(Raised by CodeRabbit, 2026-07-26.)* +- **`agenta_ops` "flat regardless of catalog size" — imprecise.** The *advertised prompt cost* is + flat; the *result payload* is O(n) and must be bounded/filterable. design.md corrected. ## Open questions -**Blocking Slice 3 only — none block the diet.** +**All of these gate the Slice 3 decision. None gate Slices 1–2 (the diet) — start those now.** -1. **Prompt caching (blocking the Slice 3 decision).** Are these tokens billed every turn, or cached - after the first? Nothing in the runner sets or inspects cache behavior. This sets the ROI of - everything past the diet. *Owner: needs a measurement on a real run.* -2. **Live advertised set.** Does `test_run` advertise with `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` - off (default)? If not, the live bill is 10,576 and Slice 1's win is ~6,900, not ~12,900. -3. **Wander evidence.** Is there measured evidence that tool *count* causes run failures? The +1. **Prompt caching.** Are these tokens billed every turn, or cached after the first? Nothing in the + runner sets or inspects cache behavior. This sets the ROI of everything past the diet. + *Owner: needs a measurement on a real run.* +2. **Wander evidence.** Is there measured evidence that tool *count* causes run failures? The internal-tools review asserts it; the meta-toolset's whole remaining case rests on it. -4. **M2 vs Lever B.** M2 (dynamic real-name advertisement) keeps real names in front of every gate +3. **M2 vs Lever B.** M2 (dynamic real-name advertisement) keeps real names in front of every gate and so dissolves most of security.md. Is its mid-session re-registration cheaper than four-site gate surgery? Compare at the decision point, not after building Lever B. -5. **Disclosure-eligible identification** (Slice 3 only) — heuristic vs a `source:"platform"` marker. - *Recommendation: heuristic for the flagged POC; marker before default-on.* -6. **Invoker shape** (Slice 3 only) — one `agenta_op` with describe/execute modes, or two tools? +4. **Invoker shape** — one `agenta_op` with describe/execute modes, or two tools? *Recommendation: one.* +## Closed since the last revision + +- **Live advertised set — CLOSED.** All 13 ops advertise by default. + `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` **defaults ON** (`platform_tools.py:41`: unset → `True`, + empty → enabled; only an explicit `off`/`false`/`0` disables). The "default off" claim and its + 10,576 alternate figure came from the 2026-07-17 investigation and were **wrong**. 18,353 is the + live number and Slice 1's win is the full ~12,900. +- **Disclosure-eligible identification — CLOSED, and the earlier recommendation was wrong.** The + proposed "collapse all direct-`call` callback specs" heuristic **misses `test_run`**, which is + handler-mode and resolves to a `callRef` (`op_catalog.py:175`) — 7,777 tokens, 42% of the bill, + the largest single target. A direct-`call` rule also over-collapses an author's `reference` tools. + **Decision: the `source:"platform"` marker is the eligibility rule**, and it moves into Slice 3 as + a prerequisite rather than a post-POC follow-up. + ## Next action -Answer open questions 1 and 2 (both are measurements on a real run, both cheap), then start -**Slice 1** — it is not blocked by either. +**Start Slice 1 (the schema diet) now.** No open question gates it: the live-set question is closed +(all 13 ops advertise, so the full ~12,900 win is real), and the caching question only affects +whether Slice 3 is worth doing at all. + +Run the caching measurement (open question 1) in parallel, on its own timeline — it feeds the +Slice 3 decision point, not the diet. From c35d470bf8faa259e07c94dbe810b814103e65eb Mon Sep 17 00:00:00 2001 From: ashrafchowdury Date: Mon, 27 Jul 2026 13:12:22 +0600 Subject: [PATCH 4/5] Refactor progressive tool disclosure plan and documentation - Revised the implementation plan for the progressive tool disclosure project, focusing on a diet-first strategy to reduce token costs significantly. - Updated the research documentation to clarify argument validation processes and the implications of schema changes on permission handling. - Removed the security.md and status.md files, consolidating security considerations into the main plan and ensuring clarity on the risks associated with the meta-toolset. - Enhanced the description of seams and their interactions with the schema diet and lazy schema implementations. - Documented the decision-making process for proceeding with the meta-toolset based on evidence from the diet phase. --- .../progressive-tool-disclosure/README.md | 150 ++++++----- .../alternatives.md | 194 +++++++++++++++ .../progressive-tool-disclosure/baseline.md | 38 ++- .../progressive-tool-disclosure/context.md | 93 +++---- .../progressive-tool-disclosure/design.md | 233 +++++------------- .../progressive-tool-disclosure/plan.md | 211 +++++++--------- .../progressive-tool-disclosure/research.md | 120 +++++---- .../progressive-tool-disclosure/security.md | 197 --------------- .../progressive-tool-disclosure/status.md | 108 -------- 9 files changed, 562 insertions(+), 782 deletions(-) create mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md delete mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md delete mode 100644 docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md index a988dc38ef..367dcba9c6 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md @@ -1,59 +1,95 @@ # Cheaper platform tools in the playground build kit -Status: PLANNING — design workspace only, no implementation. -Date: 2026-07-20 · **Revised 2026-07-26** after a re-baseline and a code-grounded security review. - -The playground advertises 13 platform-op schemas to the model on every turn — **18,353 tokens** -measured — before the model has done anything. Every always-on tool is also a "wander target" that -derails runs. - -**What the re-baseline changed.** Three ops are 88% of that bill, and a *single* schema object -(`_build_agent_template_delta_schema`, 6,441 tokens, embedded in **two** ops) is 70% of it. The -long tail of ten ops is 2,120 tokens combined. So the win is not spread across the catalog — it is -concentrated in one duplicated object. See [baseline.md](baseline.md). - -That reordered this project. The **schema diet** — originally a side-slice — is now the headline: -~84% of the win, no runner change, no wire change, no permission change. The **discovery -meta-toolset** — originally the headline — is a follow-up worth ~14% more, and it carries by far -the highest risk in the project ([security.md](security.md)). - -## Decisions (locked) - -- **Schema diet ships first, alone, and is independently valuable.** It is the bulk of the win at - near-zero risk. The replacement guidance (`references/config-schema.md`) already ships with the - skill and the skill already mandates reading it before `commit_revision`. -- **The meta-toolset is deferred behind evidence**, not cancelled. Gate it on: a measured - post-diet baseline, an answer on prompt caching, and a demonstrated reliability problem that - fewer visible tools actually fixes. -- **Seam for the meta-toolset = runner advertisement layer.** `advertisedToolSpecs` in - `services/runner/src/tools/public-spec.ts:57`, consumed at exactly two sites - (`pi-assets.ts:353`, `environment.ts:721`). Verified 2026-07-26. -- **Permission fidelity is a four-site problem, not a one-line one.** See - [security.md](security.md). This is the project's dominant risk. -- **Op-set curation is dropped, not deferred.** Hiding the 5-op event pack saves ~1,052 tokens - (~5%) and risks the agent being unable to schedule when a user pivots mid-conversation. Not - worth a capability regression. Revisit only with hard wander data. -- **Skills are out of scope.** Already progressive (64-token announcement). -- **Playground overlay only.** No change to any saved/committed agent. -- **No commits during planning.** Implementation happens later on its own branch. - -## Deliverables - -- [baseline.md](baseline.md) — **measured** per-op cost, the concentration finding, the caching gap. -- [security.md](security.md) — what collapsing 13 ops behind one name breaks; the four gate sites. -- [context.md](context.md) — problem, scope, non-goals, product language, success criteria. -- [research.md](research.md) — the current advertise/execute path with `file:line`, and the seams. -- [design.md](design.md) — the diet, then the meta-toolset; execution + permission path. -- [plan.md](plan.md) — the sliced implementation plan, each slice with an exit check. -- [status.md](status.md) — living source of truth: locked decisions, open questions, next action. - -## Intended outcome - -**After the diet (Slices 1–2):** a playground turn carries ~2,970 tokens of platform-op schema -instead of 18,353, with every tool still visible, every schema still authoritative enough to call -against, and not one line of permission code touched. - -**If the meta-toolset later clears its evidence bar (Slice 3):** the model sees two small platform -tools instead of thirteen, a turn costs a low-hundreds constant that stays flat as the catalog -grows, and every op still runs with its exact self-targeting binding and approval gate — with the -four-site permission work done properly and tested per mutating op. +Status: PLANNING — no implementation. +Date: 2026-07-20 · Revised 2026-07-27. + +The playground advertises 13 platform-op schemas on every turn — **18,353 tokens** measured — before +the model does anything. Every always-on tool is also a wander target. + +## Strategy + +**Diet the schemas, then make them lazy.** Two levers, that's it. + +| Phase | Lever | From → to | Cut | Effort | +| --- | --- | ---: | ---: | --- | +| 1–2 | schema diet | 18,353 → ~3,000 | 84% | 1 Python file | +| 3 | lazy schema | ~3,000 → ~500 | 97% | 1 runner function | + +Neither needs a permission change or transport work, and both land on all three harnesses at once. +Details: [design.md](design.md) · [plan.md](plan.md). + +## The two levers are different things + +| | the schema | the tool entry | +| --- | --- | --- | +| Diet | permanently shorter | present | +| Lazy schema | fetched on request | present | + +Both are token levers; the tool *count* is unchanged, so neither addresses wander. + +## Lazy activation is out of scope + +Deferring the tool entry itself is the only lever that would reduce tool count — and it is **not in +this plan** until explicitly asked for. It buys ~100 tokens over lazy schema, so its case rests +entirely on wander, which is asserted rather than measured. It is also the only lever needing +per-harness transport work: Pi has the API today, but Claude and Codex go over MCP and need +`notifications/tools/list_changed` — which the local HTTP server cannot send at all, which the stdio +shim would need re-plumbing to send, and which the pinned third-party ACP clients may ignore +mid-turn. Full record: +[alternatives.md](alternatives.md#3--lazy-activation-out-of-scope). + +## What the measurement changed + +Three ops are 88% of the bill, and one schema object embedded twice is 70% of it +([baseline.md](baseline.md)). The cost is schema *depth*, not catalog *length* — which is why the +diet, not the mechanism, is the delivery. + +## What the design review changed + +The original mechanism was a **card/menu invoker**: one `agenta_op(op, args)` tool proxying all 13 +ops. **Rejected 2026-07-27** — routing every op through one name breaks the permission ladder in +four fail-closed sites where a missed site fails open. + +Eight strategies were weighed in total. All of them, with the win each would deliver and why the +seven non-adopted ones are closed, are in one file: **[alternatives.md](alternatives.md)**. + +## Locked decisions + +- Diet ships first, alone. +- Lazy schema is the mechanism. One projection site, all harnesses, real names. +- Lazy activation is **out of scope** — not a later phase. Revisit only if asked for. +- Discovery is **not** lazy — names and one-liners stay in the prompt. +- Card/menu rejected, recorded, not to be re-proposed. +- Op-set curation dropped: ~5% of tokens for a real capability regression. +- Playground overlay only. No saved-agent change. No commits during planning. + +## Open questions + +Neither blocks the plan; both would only matter if lazy activation is ever asked for. + +1. **Wander evidence.** Is there measured evidence that tool *count* causes failures? +2. **Does a client honor `list_changed` mid-turn?** Only matters if 1 comes back positive. + +## Corrections and closed items + +- **Handler flag "default off" — wrong.** It defaults **on** (`platform_tools.py:41`, unset and + empty both enable). All 13 ops advertise; 18,353 is live. *(CodeRabbit, 2026-07-26.)* +- **"Malformed args become a server error" — overstated.** `assertRequiredArguments` runs + runner-side against the private spec (`relay.ts:327`, `:369`) before execution. +- **Direct-`call` eligibility heuristic — wrong.** It missed handler-mode `test_run`, 42% of the + bill. Moot now: real names need no eligibility rule. *(CodeRabbit, 2026-07-26.)* +- **Approval leak across ops — not real.** Grant and decision stores key on + `approvedCallKey(toolName, args)` — name **plus** args hash. +- **In-sandbox stdio MCP — already ships.** `tool-mcp-stdio.ts`, bundled and uploaded for Daytona. + Earlier notes treated it as future work. +- **"Disclosure inevitably breaks permission" — overstated.** Only the invoker did. + +## Docs + +- [baseline.md](baseline.md) — measured per-op cost and the concentration finding. +- [context.md](context.md) — scope, glossary, success criteria. +- [research.md](research.md) — how tools reach the model, with `file:line`. +- [design.md](design.md) — the two levers, in full. +- [plan.md](plan.md) — phases, each with its win and exit check. +- [alternatives.md](alternatives.md) — **all 8 strategies, win comparison, and why the rejected ones + were rejected.** diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md new file mode 100644 index 0000000000..e4d9cc53e1 --- /dev/null +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md @@ -0,0 +1,194 @@ +# Alternatives — every strategy considered, and the win comparison + +The single record of what was weighed and why. Kept so nothing here is re-proposed. +Verified against the runner on `main`, 2026-07-26. Baseline is 18,353 tokens +([baseline.md](baseline.md)). + +## All strategies + +| # | Strategy | What it does | Win | Status | Why not | +| --- | --- | --- | ---: | --- | --- | +| 1 | **Schema diet** | Shrink the two fat schemas to shallow + pointer to prose that already ships | 18,353 → ~3,000 | **adopted** | — | +| 2 | **Lazy schema** | Real names + stub schemas; `load_op` returns the full schema on request | → ~500 | **adopted** | — | +| 3 | **Lazy activation** | Tool entry absent until activated | → ~400 | **out of scope** | ~100 tokens over #2; its case is wander, which is unmeasured. Only lever needing per-harness transport work — free on Pi, hard on Claude/Codex. Detail below | +| 4 | **Card/menu invoker** | One `agenta_op(op, args)` proxies all 13 ops | → ~400 | **rejected** | Breaks the permission ladder in 4 fail-closed sites; a missed site fails **open**. Full analysis below | +| 5 | **Op-set curation** | Drop the 5-op event pack from the default overlay | ~1,052 (5%) | **rejected** | Cannot be decided at run start. A user who pivots to "schedule this daily" mid-conversation finds the capability gone | +| 6 | **Turn-boundary activation** | Activate only between turns, so no mid-turn push is needed | — | **rejected** | Either stops the agent mid-task waiting for the user, or costs a full transcript replay per activation | +| 7 | **SSE + session ids** | Give the local HTTP MCP server a push channel | — | **rejected** | Large rewrite of a deliberately minimal server, to enable #3's ~100 tokens | +| 8 | **Mode-gating** | No build kit in Chat mode | — | **not a substitute** | Removes capability rather than deferring it. Can layer on top | + +**#1 and #2 are the delivery.** Both keep real tool names, touch no permission code, and land on all +three harnesses through one projection site. Detail: [design.md](design.md). + +## #3 — lazy activation, out of scope + +Ops would start **inactive** — not in the model's tool list at all — with a loader adding one on +demand. This is the only lever that reduces tool *count*, which is what the internal-tools review +tied to wander. **Not being built until explicitly asked for.** + +### Why it is out of scope + +**It buys ~100 tokens over #2.** Both leave names and one-liners in the prompt and both fetch +schemas on demand; the difference is 13 stub entries. Its case is wander, not tokens — and wander is +currently asserted, not measured. A lever whose entire justification is unmeasured should not gate +the levers whose justification is measured. + +**It is the only lever needing per-harness transport work**, and two of three harnesses make it +hard: + +| harness | delivery | activation | state | +| --- | --- | --- | --- | +| Pi | native, bundled extension | `pi.setActiveTools()` | **works today** | +| Claude — Daytona | in-sandbox stdio shim | `notifications/tools/list_changed` | possible, unverified | +| Claude — local | HTTP loopback | none | blocked | +| Codex | MCP, same as Claude | same as Claude | after PR #5509 | + +### The hard part, precisely + +**Pi is free.** `getAllTools()` / `getActiveTools()` / `setActiveTools()` exist and are already used +(`extensions/agenta.ts:215`). Mid-turn, in-process, no transport involved. + +**Claude and Codex are MCP-forwarding** (`capabilities.ts:99`), so adding a tool mid-turn requires +the server to *push* `notifications/tools/list_changed`. Three problems stack: + +1. **Local HTTP cannot push at all.** The loopback server is *"stateless JSON mode… no SSE… 405 for + `GET`"* (`tool-mcp-http.ts:22-27`). There is no server→client channel. Fixing it means either + running the stdio shim on the runner host — a security-posture change, since `tool-mcp-stdio.ts` + is written to run inside the sandbox and its docstring flags runner-host stdio as a deliberately + closed hole (#4831) — or building SSE + session ids (#7), which undoes the "simplest conformant + server" design. +2. **The shim's tool list is a startup snapshot.** `tool-mcp-stdio.ts` reads specs once from + `AGENTA_AGENT_TOOLS_PUBLIC_SPECS_FILE`. It would need a runner signal over the relay dir to + re-read and notify, plus `capabilities: {tools: {listChanged: true}}` at `initialize` — today it + sends `{tools: {}}` (`tool-mcp-http.ts:124`). +3. **The clients may ignore the notification mid-turn.** `@agentclientprotocol/claude-agent-acp` and + `codex-acp` are third-party and version-pinned. Whether either re-lists **mid-turn** is + unverified. If they only re-list on reconnect, activation lands at a turn boundary — the agent + must stop and wait for the user mid-task, or pay a full transcript replay per activation. Both + are unacceptable in a builder flow (that is #6). + +Problem 3 is unfixable by us if it fails. Three units of transport work, one outside our control, +for ~100 tokens and an unmeasured reliability claim. + +**If it is ever asked for:** measure wander first. Then Pi only (free, no transport work), and +Claude/Codex only after a spike confirms mid-turn re-listing. + +## #4 — card/menu invoker, in full + +**Status: rejected 2026-07-27.** This was the original proposal, so it gets the full write-up. +**None of it applies to #1 or #2**, because neither changes a tool's name. + +| | Card/menu (#4) | Lazy schema (#2) | +| --- | --- | --- | +| Model calls | proxy: `agenta_op(op, args)` | real op: `commit_revision(args)` | +| Gate sees | `agenta_op` for all 13 ops | the real op | +| `readOnly` / `allow_reads` | one value for 13 ops — breaks | the op's own — works | +| Policy rules (`remove_schedule: deny`) | stop matching, silently | keep matching | +| Arg validation | lost (opaque `args`) | kept (runner-side, private spec) | +| Round trips per new op | 2 | 2 | +| Work | 4-site permission surgery + wire marker + dispatch | 1 projection function | +| Fails toward | **too permissive, silently** | schema not loaded yet, loudly | + +Card/menu's one advantage was that it is transport-agnostic — a plain tool, identical on every +harness. That is why it was proposed first. It stopped being an advantage once the levers were +split: **#2 is transport-agnostic too**, at one function, with no permission change at all. + +### The root cause + +A tool's name does two jobs: it **resolves the private spec** the gate reads its fields from, and it +is **itself matched** by policy rules. The decision ladder (`effectivePermission`, +`permission-plan.ts:125`) uses both: + +```text +gate.specPermission (from resolved spec) -> short-circuits the rest when set +matchingRulePermission(gate, rules) -> matches on gate.toolName itself +defaultPermission(mode, gate) -> allow_reads: gate.readOnlyHint ? allow : ask +``` + +The card/menu invoker routes all 13 ops through one name, `agenta_op`. Every input above then reads +off one spec. + +The regression is not the deferral — it is **building the gate from the invoker's spec instead of +re-resolving the target**. It is fixable, but the fix has to land in four fail-closed sites, and a +missed site fails **open**. + +### What breaks + +**1. `readOnlyHint` — the `allow_reads` default.** `defaultPermission` (`permission-plan.ts:248`): + +```ts +if (mode === "allow_reads") return gate.readOnlyHint === true ? "allow" : "ask"; +``` + +`readOnlyHint` comes from `spec.readOnly`, set per op. One invoker spec covers thirteen: + +- `readOnly: true` → every write executes silently. `commit_revision`, `remove_schedule`, + `remove_subscription` lose their prompt. +- `readOnly: false` → every read prompts. Habituated click-through is worse than not prompting. + +No static value is correct. + +**2. `specPermission`.** Short-circuits the whole ladder (`:129`). One value for thirteen ops, and +it wins over any rule. + +**3. Policy rules stop matching.** `ruleMatches` (`:214`) compares `gate.toolName` exactly. A rule +written as `commit_revision: ask` or `remove_schedule: deny` stops matching once calls arrive as +`agenta_op`. It does not error — it falls through to the default, so `deny` becomes whatever +`allow_reads` decides. Rules authored before the change quietly stop being enforced. + +**4. Grant / stored-decision keys — safe.** Both `ApprovedExecutionGrants` (`responder.ts:85`) and +`ConversationDecisions.take` (`:255`) key on `approvedCallKey(toolName, args)` — name **plus** a +canonical args hash. The invoker's args carry the op name, so distinct ops produce distinct keys. +No cross-op approval leak. The only requirement is that `grant()` and `consume()` key consistently; +a mismatch fails closed (every Pi write refused), which is a bug, not a hole. + +### The four sites + +Each independently resolves a spec from a name and builds its own `GateDescriptor`. + +| # | Site | Path | +| --- | --- | --- | +| 1 | `buildRelayExecutionGuard` | `engines/sandbox_agent/relay-guard.ts:53` | +| 2 | `buildGateDescriptor` (Claude/ACP) | `engines/sandbox_agent/acp-interactions.ts:516` | +| 3 | `buildPiGateDescriptor` (Pi) | `engines/sandbox_agent/acp-interactions.ts:456` | +| 4 | `piDialogAllows` call (in-sandbox) | `extensions/agenta.ts:318` | + +PR #5509 adds an `ExecutableToolGate` seam, so this becomes five once Codex merges. + +Site 4 has an extra problem: the in-sandbox extension only sees the **advertised** set, so under +disclosure it no longer holds the target's `readOnly` and cannot decide `gateViaDialog` locally. + +### What was *not* a risk + +Routing the decision through a sandbox-supplied `args.op` is **not** new trust exposure. The tool +name is already sandbox-supplied (`acp-interactions.ts:307`: *"the envelope is sandbox-origin and +untrusted"*) and is safe because it is validated against a fixed spec map, failing closed otherwise +(`:473`). Reading `args.op` and validating it the same way is the same pattern at the same trust +level. The cost is surface area — four hand-written lookups instead of one. + +### Other costs + +- **Pre-call validation lost.** Pi registers each tool with its real schema + (`extensions/agenta.ts:305`); `assertRequiredArguments` checks required fields (`:318`, + `relay.ts:369`). A generic invoker makes `args` opaque, degrading both to the invoker's loose + schema. +- **Two execution shapes.** Endpoint-mode ops carry a direct `call`; handler-mode `test_run` carries + a `callRef` and runs through `callAgentaTool`. An invoker forwarding only `spec.call` cannot + execute `test_run` at all. +- **Eligibility has no clean rule.** Platform ops do not share one shape, and neither field + identifies them: + + | | direct `call` | `callRef` | + | --- | --- | --- | + | platform op | 12 endpoint ops | `test_run` | + | not a platform op | author's `reference` tool | gateway tool | + + A direct-`call` heuristic misses `test_run` — 7,777 tokens, 42% of the bill — and over-collapses + author tools. A `source:"platform"` wire marker would have been required. + +### Verdict + +The invoker's cost is security-critical work that fails toward "too permissive," silently. The +chosen levers cost one Python file and one projection function, touch no permission code, and fail +toward "the schema isn't loaded yet," loudly. For a system whose job is gating an agent's writes to +a user's own agents, that asymmetry decides it. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md index 9a794861fd..2ef5a77e23 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md @@ -50,18 +50,24 @@ stripped), matching `advertisedToolSpec()` in `services/runner/src/tools/public- > **Correction.** Drafts before 2026-07-26 (and the 2026-07-17 investigation they came from) > claimed "default off" and carried a 10,576-token alternate figure. That was wrong. There is no > alternate figure: `test_run` advertises unless someone opts out, so its 7,777 tokens — 42% of - > the bill — are real, and Slice 1's win is the full ~12,900. + > the bill — are real, and Phase 1's win is the full ~12,900. -## Consequence for slicing +## Consequence for phasing | lever | est. after | cut from 18,353 | risk | | --- | ---: | ---: | --- | | Schema diet on `commit_revision` + `test_run` | ~5,500 | ~70% | near-zero (see below) | | \+ trim `query_spans` filter DSL | ~2,970 | ~84% | low | -| \+ discovery meta-toolset | ~400 | ~98% | **high** — see [security.md](security.md) | +| \+ lazy schema | ~500 | ~97% | one projection function; no permission change | +| \+ lazy activation | ~400 | ~98% | **out of scope** — per-harness transport work | -The meta-toolset's *marginal* token win over a completed diet is ~2,600 tokens (~14% of the -original bill). That is the number its risk must be weighed against — not the full 18,353. +Two numbers decide the plan's shape: + +- **Lazy schema's marginal win over a completed diet is ~2,500 tokens (~13%)** at the cost of one + function in `public-spec.ts` — and it is structural, so catalog growth stops inflating the prompt. +- **Lazy activation's marginal win over lazy schema is ~100 tokens**, which is why it is out of + scope ([alternatives.md](alternatives.md#3--lazy-activation-out-of-scope)). Its case is tool *count* + (wander), not tokens. Weigh its per-harness transport cost against wander evidence only. ## Why the diet is near-zero risk @@ -77,11 +83,12 @@ mandatory reading**: embedded JSON Schema is not enforcing a server contract — it is model guidance that a better-written, on-demand doc already provides. -What is genuinely lost by dieting: the harness's own pre-call JSON-Schema validation of the -argument shape (Pi passes `inputSchema` to `registerTool({parameters})`, -`services/runner/src/extensions/agenta.ts:305`). Malformed configs would surface as a server -error instead of a pre-call harness error. Given the server does not validate the config shape -anyway, this is a small change in *where* the error appears, not whether it is caught. +**Required-argument checking is not lost.** The runner re-validates against the **private** spec +before executing — `assertRequiredArguments(spec, req.args)` at `services/runner/src/tools/relay.ts:327` +(client shape) and `:369` (endpoint and handler shapes). What moves is *where* a malformed call is +caught: at the relay rather than pre-call in the harness (Pi passes `inputSchema` to +`registerTool({parameters})`, `services/runner/src/extensions/agenta.ts:305`). Either way the model +gets an error before anything executes. ## Reproducing @@ -104,14 +111,3 @@ for name in OPS: print(f"{name:22} {n:>7}") print(f"{'TOTAL':22} {total:>7}") ``` - -## Open measurement gap — prompt caching - -**Nothing in this project has measured whether these tokens are actually billed per turn.** Tool -definitions are a stable prompt prefix, which is exactly what provider prompt caching targets, and -both harnesses (Claude Code, Pi) run their own caching. `grep -ri "cache_control|prompt.cach"` over -`services/runner/src` and `sdks/python/agenta/sdk/agents` returns nothing, so the runner neither -sets nor inspects cache behavior — it is entirely the harness's. - -If the schemas are cached, the real cost is "18K once, a fraction thereafter", and the ROI of -everything past the diet drops sharply. **Measure this before committing to the meta-toolset.** diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md index 3408e1d67f..5da987453e 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md @@ -3,84 +3,73 @@ ## Problem Open a playground agent that shows "Tools: None" and type "hi". The turn carries **18,353 prompt -tokens** of platform-op schema (measured 2026-07-26, tiktoken `o200k_base` — -[baseline.md](baseline.md)). Nothing the author did explains it: the cost is the playground -**build kit**, which injects 13 platform ops into the agent template and advertises every one of -them to the model on every turn. +tokens** of platform-op schema (measured 2026-07-26 — [baseline.md](baseline.md)). Nothing the +author did explains it: the cost is the playground **build kit**, which injects 13 platform ops +into the agent template and advertises every one of them on every turn. Two costs, not one: -- **Tokens.** And they are *concentrated*, not spread. `test_run` (7,777) + `commit_revision` - (6,878) + `query_spans` (1,578) are **88%** of the bill; the other ten ops are 2,120 tokens - combined. One schema object — `_build_agent_template_delta_schema()`, 6,441 tokens — is embedded - in **both** `test_run` and `commit_revision`, so its two copies are 12,882 tokens: **70% of the - bill is one object counted twice.** +- **Tokens**, and they are *concentrated*. `test_run` (7,777) + `commit_revision` (6,878) + + `query_spans` (1,578) are **88%** of the bill; the other ten ops are 2,120 combined. One schema + object — `_build_agent_template_delta_schema()`, 6,441 tokens — is embedded in **both** + `test_run` and `commit_revision`, so **70% of the bill is one object counted twice.** - **Reliability.** The internal-tools review (`../builder-agent-reliability/tools-review/part-2-internal-tools.md`) found the same tools are a *double* cost: "each unused tool is context cost plus a wander target (the capstone showed extra visible tools derail runs)." **These two costs need different fixes, and that is the central finding of this workspace.** -Shrinking schemas fixes the token cost completely and the wander cost not at all — the tool *count* -is unchanged. Reducing the advertised tool count fixes wander but, after the diet, buys only ~14% -more tokens. Conflating them is what made the first draft of this plan lead with the expensive, -risky lever instead of the cheap one. +Shrinking schemas fixes the token cost and the wander cost not at all — the tool *count* is +unchanged. Reducing the advertised tool count fixes wander but, after the schema work, buys ~100 +tokens. Conflating them is what made the first draft lead with the expensive, risky lever instead +of the cheap one. -It also scales the wrong way: adding a catalog op is a one-line data change that ships to every -playground agent unconditionally. But note the tail is *cheap* — ten ops for 2,120 tokens — so -catalog growth is not currently the pain. Schema *depth* is. +The tail is *cheap* — ten ops for 2,120 tokens — so catalog growth is not the current pain. Schema +*depth* is. ## Scope (this delivery) - A **schema diet** for the ops carrying deep expanded schemas — `commit_revision`, `test_run`, - then `query_spans`. This is the primary deliverable: ~84% of the token win, no runner change. -- A **token + reliability baseline** so before/after is measured, not asserted — done for tokens - ([baseline.md](baseline.md)), still open for reliability and for prompt caching. -- A **designed, risk-registered meta-toolset** (`agenta_ops` + `agenta_op`) held behind an - evidence gate, with its permission work scoped honestly ([security.md](security.md)). + then `query_spans`. 84% of the token win, no runner change. +- **Lazy schema** — ops stay advertised under real names with stub schemas; `load_op` returns the + full schema on request. One projection site, all harnesses ([design.md](design.md)). +- A **token baseline** so before/after is measured, not asserted — done ([baseline.md](baseline.md)); + reliability measurement still open. ## Out of scope -- **Skills.** Already progressive; a 64-token announcement. Untouched. -- **Op-set curation** (dropping the event pack from the default overlay). *Dropped, not deferred* — - it saves ~1,052 tokens and risks the agent being unable to schedule when the user pivots - mid-conversation. Revisit only with hard wander data. -- **External tool discovery (`discover_tools`).** It discovers *Composio* tools to wire into an - agent; it stays as-is and is itself one of the ops we would disclose. -- **User / gateway / code / client tools.** The measured cost is platform ops. -- **Committed non-playground agents.** They advertise only what their author declared. -- **Dynamic real-name re-advertisement (M2).** A productionization option, evaluated only if the - meta-toolset ships. +- **Lazy activation** (deferring the tool entry itself). Out of scope until explicitly asked for — + ~100 tokens over lazy schema, an unmeasured wander case, and the only lever needing per-harness + transport work. Record: [alternatives.md](alternatives.md#3--lazy-activation-out-of-scope). ## Product language - **Platform op** — an existing Agenta endpoint exposed to the agent as a tool, defined in the code catalog `op_catalog.py` (e.g. `commit_revision`, `query_spans`). - **Advertised spec** — the `{name, description, inputSchema, …}` projection the model sees; - distinct from the **private resolved spec** the runner executes from. -- **Schema diet** — replacing a deep, type-expanded `inputSchema` with a shallow one plus a pointer - to on-demand prose guidance. Tool stays visible; only its manual shrinks. -- **Discovery meta-toolset** — the small fixed set (`agenta_ops`, `agenta_op`) that would stand in - for the op schemas: list, describe-on-demand, invoke. -- **Disclosure** — moving an op's full schema out of the prompt (paid every turn) into a tool - result (paid once, only when fetched). + distinct from the **private resolved spec** the runner executes and validates from. +- **Schema diet** — permanently shrinking a deep, type-expanded `inputSchema` to a shallow one plus + a pointer to prose. The detail is gone; the tool stays visible. +- **Lazy schema** — the full schema still exists but is not in the prompt. `load_op(name)` returns + it on request. The tool stays visible. +- **Lazy activation** — the tool entry itself is absent until activated. The only lever that reduces + tool count, and out of scope here. +- **Card/menu invoker** — the rejected alternative: one `agenta_op(op, args)` tool that both lists + and **proxies** every op. See [alternatives.md](alternatives.md). ## Success criteria -Rewritten 2026-07-26. The old criterion measured a no-op "hi" turn, which optimizes the cheapest -case nobody pays for; and it credited the meta-toolset with a win the diet delivers. - -1. **Diet:** platform-op prompt cost drops from 18,353 to under ~3,000 tokens, with no capability +1. **Diet:** platform-op prompt cost drops from 18,353 to under ~3,000 tokens with no capability loss — a lab agent still commits a valid config and runs a test. -2. **Session-level, not turn-level:** total platform-op tokens across a full build session +2. **Lazy schema:** under ~500 tokens, and adding a catalog op no longer grows the prompt. +3. **Session-level, not turn-level:** total platform-op tokens across a full build session (discover → wire → commit → test → schedule) drop materially. A no-op turn is *reported*, never the target. -3. **Caching answered:** we know whether these tokens are billed per turn or cached after the - first, and the ROI of anything past the diet is restated against that answer. -4. **No safety regression:** self-targeting `$ctx` bindings and per-op approval behave exactly as - today, verified per mutating op on **both** harness paths. -5. **Reliability measured, not assumed:** if the meta-toolset proceeds, it is justified by observed - wander reduction on the lab matrix — the token case alone (~14%) does not carry it. -6. **Cost of laziness bounded** (meta-toolset only): ≤1 extra round-trip per distinct op used, a - schema fetched at most once per op per conversation, and no net regression in session-level - tokens once history-resident schemas are counted. +4. **No safety regression:** `$ctx` bindings and per-op approval behave exactly as today — + preserved by construction (real names reach every gate) and verified per mutating op. +5. **Discovery never regresses:** the model can always name what it is able to do. An op it cannot + see is a capability it will deny having — worse than the token bill. +6. **Cost of laziness bounded:** ≤1 extra round-trip per distinct op used, and no net regression in + session-level tokens. +7. **Wander measured, not assumed** — the input that would decide whether lazy activation is ever + asked for. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md index af76267eba..e719527158 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md @@ -1,196 +1,93 @@ # Design -Two independent levers, in the order their evidence supports. Revised 2026-07-26: the re-baseline -([baseline.md](baseline.md)) showed the token cost is concentrated in schema *depth*, not tool -*count*, which promotes the diet and demotes the meta-toolset. +Two levers, in order. Each ships alone and each is useful alone. -| | Lever A — schema diet | Lever B — meta-toolset | -| --- | --- | --- | -| Fixes | token cost | wander / tool count | -| Win | ~84% (18,353 → ~2,970) | ~14% more (→ ~400) | -| Touches | `op_catalog.py` schemas | advertisement + 4 permission sites | -| Runner change | none | substantial | -| Wire change | none | possible (marker) | -| Risk | near-zero | **high** ([security.md](security.md)) | - ---- - -# Lever A — the schema diet (primary) - -## The core move - -Two ops embed the *same* 6,441-token object, `_build_agent_template_delta_schema()` -(`op_catalog.py:317`), expanded inline into their `input_schema`. That single object counted twice -is 70% of the entire advertised bill. Replace the deep expansion with a shallow schema plus a -pointer to prose guidance that already ships. - -## Why this costs almost no capability +| | Win | Leaves | Effort | Touches permissions | +| --- | ---: | ---: | --- | --- | +| **1 — schema diet** | 84% | ~3,000 | 1 Python file | no | +| **2 — lazy schema** | +13% | ~500 | 1 runner function | no | -The replacement is not hypothetical — it is already in the product and already mandatory: +Baseline is 18,353 tokens ([baseline.md](baseline.md)). -- `references/config-schema.md` (3,621 tokens of prose + worked examples) ships as a `SkillFile` on - `BUILD_AN_AGENT_SKILL` (`agenta_builtins.py:740`). -- The skill body already says *"Read `references/config-schema.md` before your first - `commit_revision`"* (`:579`), and points back to it on failure (`:711`, `:716`). -- That reference states the commit endpoint **does not validate this shape**. So the embedded - schema enforces no server contract; it is model guidance only. +**They are different things and get confused.** What each defers: -The model therefore already has a better-written, on-demand source for the same information. The -embedded schema is duplication. - -## Shallow, not empty - -The first draft proposed `{"type": "object"}` plus a description. Prefer a **shallow** schema over -an open one: keep the top-level keys (`instructions`, `llm`, `tools`, `mcps`, `skills`, `harness`, -`runner`, `sandbox`) with one-line descriptions, and stop before expanding nested `$defs`. That -costs a few hundred tokens instead of ~6,400 and keeps the model oriented about *which* keys exist, -while the reference doc supplies the shapes underneath. - -The expansion is what explodes — `expand_type_refs` / `_deep_partial_schema` (`op_catalog.py:249`). -A depth-limited expansion is the surgical form of this change and is reusable for any future op. - -## What is genuinely lost +| | the schema | the tool entry | +| --- | --- | --- | +| Diet | permanently shorter | present | +| Lazy schema | fetched on request | present | -Pi validates arguments against the advertised JSON Schema before dispatch -(`registerTool({parameters})`, `extensions/agenta.ts:305`). A shallower schema catches less -pre-call, so a malformed config surfaces as a server error rather than a harness error. Bounded: -the server does not validate the config shape today either, so this changes *where* the failure -appears, not *whether* it is caught. `test_run` after commit remains the real verification, exactly -as the skill already instructs. +Both are **token** levers — the tool *count* is unchanged, so neither addresses wander. The lever +that would (**lazy activation**, deferring the tool entry itself) is **out of scope** and is +recorded in [alternatives.md](alternatives.md#3--lazy-activation-out-of-scope). --- -# Lever B — the discovery meta-toolset (deferred behind evidence) - -## The core move +# Lever 1 — schema diet (ship first) -Stop advertising N op schemas. Advertise a small fixed meta-toolset; keep the op specs resolved but -**private** in runner memory; move each op's schema from the *prompt* into a *tool result*. +Two ops embed the same 6,441-token object, `_build_agent_template_delta_schema()` +(`op_catalog.py:317`). Two copies = 12,882 tokens = 70% of the bill. Replace the deep expansion +with a shallow schema plus a pointer to prose that already ships. -- **`agenta_ops(query?)`** — the op catalog as a compact list: `{op, one_line, read_only}`, no input - schemas. The **advertised prompt cost is flat** regardless of catalog size — that is the win. The - *result* is O(n) in the catalog (~200 tokens at today's 13 ops, ~15/op), so it must be bounded: - `query` filters, results are capped at a hard limit, and the response states when it truncated. - Without a cap, a large catalog reintroduces the same cost one layer down. -- **`agenta_op(op, args?)`** — the generic invoker. No `args` (or `mode:"describe"`) returns one - op's `inputSchema` as a tool result; with `args` it executes that op. +**Why it costs almost nothing:** `references/config-schema.md` (3,621 tokens, prose + examples) +ships as a `SkillFile` on `BUILD_AN_AGENT_SKILL` (`agenta_builtins.py:740`), and the skill already +says *"Read `references/config-schema.md` before your first `commit_revision`"* (`:579`). That +reference also states the commit endpoint does not validate the config shape. So the embedded JSON +Schema enforces nothing — it duplicates better guidance the model already has on demand. -The op specs stay in `plan.toolSpecs` and `toolSpecsByName`. Only the *advertisement* changes. This -is the pattern Claude Code uses on its own tools: names listed, schema loaded on demand. +**Shallow, not empty.** Keep top-level keys (`instructions`, `llm`, `tools`, `mcps`, `skills`, +`harness`, `runner`, `sandbox`) with one-line descriptions; stop before expanding nested `$defs`. +A few hundred tokens instead of 6,441. Implement as a depth limit on `expand_type_refs` so it is +reusable by Lever 2. -## Why it is deferred +**Cost: none that reaches execution.** Required-argument checking is not lost — the runner +re-validates against the private spec at `relay.ts:327` and `relay.ts:369`, covering both execution +shapes, before anything runs. What changes is *where* a malformed call is caught: at the relay +instead of pre-call in the harness. Either way the model gets an error, not a bad write. -1. **Its token case is weak after Lever A.** ~2,600 marginal tokens (~14% of the original bill). -2. **Its real case is reliability** — fewer wander targets — and that is a hypothesis from the - internal-tools review, not a measurement. The diet does not reduce tool count, so this argument - survives Lever A intact; it just has to be *proven* rather than assumed. -3. **Its cost is the permission plane.** Four independent gate sites, each of which must learn to - resolve the target from `args.op`, in code deliberately written to fail closed. Full analysis: - **[security.md](security.md)**. -4. **It can regress long sessions.** A fetched schema is history-resident for the rest of the - conversation, in a far less cache-friendly position than a stable tool-definition prefix. - Without Lever A first, disclosure can cost *more* across a real build session. - -## One shared target resolver (required) - -Four separate sites need to answer "which op is this really?", and a site that gets it wrong fails -**open** (see [security.md](security.md)). Four independent implementations of a security-sensitive -lookup is the wrong shape. - -**Requirement:** define exactly one runner-side primitive — -`resolveDisclosedTarget(toolName, args, specsByName)` — that - -1. returns `undefined` unless `toolName` is the invoker (so non-disclosed calls are untouched), -2. validates `args.op` against the same fixed spec map every gate already uses, returning - `undefined` (→ fail closed) for anything unknown or malformed, -3. returns the canonical `{spec, gateName, args}` triple the caller builds its `GateDescriptor` - from. - -All four gate sites and the invoker's dispatch consume that one result. No site re-implements the -lookup. This is what makes "resolve the target" a single reviewable, single-testable change instead -of four chances to fail open. - -## Execution path for `agenta_op(op, args)` - -1. **Resolve the target** via the shared primitive above. Unknown → tool error listing valid ops - (recoverable), and fail closed at every gate. -2. **Describe mode** → return the target's resolved input schema. No side effects, no approval. -3. **Execute mode** → build the permission gate from the **target** spec at all four sites, run - `decide()`, then `assertRequiredArguments(target, innerArgs)`. -4. **Dispatch on the target's shape — there are two.** Reuse the `executeRelayedTool` core rather - than reimplementing either branch: - - **Endpoint-mode** (12 ops, direct `call`) → `assembleBody` → `directCallUrl` → `callDirect`. - - **Handler-mode** (`test_run`, `callRef`) → `applyContextBindings` → `callAgentaTool`. - - An invoker that forwards only `spec.call` cannot run `test_run` at all. -5. **Return** the response verbatim. - -## Why execution itself is safe by construction - -Execution reads the **private** spec (research seams 2–3), so dispatching the target through its -own unchanged execution branch preserves: - -- **Self-targeting, endpoint-mode.** `assembleBody` still fills `call.context` (`$ctx.*`) last, so - `commit_revision` still binds `$ctx.workflow.variant.id`; the model cannot retarget. Those fields - were stripped at resolve time and are re-applied below the invoker. -- **Self-targeting, handler-mode.** `test_run` binds `target.workflow_variant_id` through - `applyContextBindings` on the `callAgentaTool` branch — a different mechanism, equally below the - invoker. Both branches must be covered by the `$ctx` test in security.md, not just the first. -- **SSRF guard.** `directCallUrl` host-locks to the run's Agenta origin (endpoint-mode); - `callAgentaTool` posts back through Agenta's own `/tools/call` so the secret stays server-side - (handler-mode). +--- -Permission is the part that is **not** free. It is designed in, not inherited — see security.md. +# Lever 2 — lazy schema -## Identifying the disclosure-eligible set +Every op stays advertised under its real name with a one-line description and a **stub** schema. +A `load_op` tool returns the full schema on request. -**Revised 2026-07-26.** The earlier draft recommended a "collapse every direct-`call` callback spec" -heuristic for the POC. That rule is wrong: platform ops do not share one shape (research seam 5). +```text +model → load_op("commit_revision") returns the schema as a tool result +model → commit_revision({...}) real name, real gate, real execution +``` -- `test_run` is **handler-mode** — it resolves to a `callRef`, not a `call` (`op_catalog.py:175`). - A direct-`call` heuristic **skips it**, leaving 7,777 tokens (42% of the bill, the largest single - target) advertised. The POC would miss most of its own goal. -- Conversely, an author's `reference` tool carries a direct `call` and a gateway tool carries a - `callRef`, so both fields over-collapse in the other direction. +**Why it is cheap:** one site. `advertisedToolSpecs` (`public-spec.ts:57`) is the single projection +every harness shares — Pi reads it at `pi-assets.ts:353`, the MCP path at `environment.ts:721`. +Stub there and all three harnesses get it. No notification, no transport work, no per-harness code. -**Decision: use the marker.** The platform resolver stamps `source:"platform"` (or a `disclosable` -group tag) on the resolved spec; the runner collapses exactly that group. Costs a `protocol.ts` + -`wire.py` + golden change — small, and it is the only rule that is actually correct. It also lets us -disclose gateway/reference groups later on purpose rather than by accident. +**Why it is safe:** the tool name never changes, so every permission gate behaves exactly as today. +And as with Lever 1, `assertRequiredArguments` still runs runner-side against the private spec +(`relay.ts:327`, `:369`) — the stub is a prompt-side projection, not a weakening of enforcement. -The heuristic is retained only as a documented non-option, so it is not re-proposed. +There is precedent for exactly this rule. Codex wraps MCP calls as `{server, tool, arguments}`, which +forced `unwrapCodexMcpArgs` into `storedDecisionKeyShape` (`permission-plan.ts`, PR #5509) so +permission keys match — it normalized the **arguments** and left the **tool name real**, which is +why it needed no gate surgery. Both levers here stay on the same side of that line. -## The catalog summary (`agenta_ops`) +**Cost:** one extra round-trip per distinct op used. Tool count is unchanged, so this does nothing +for wander. -Build `{op, one_line, read_only}` runner-side from the resolved specs already in memory — -`description` and `readOnly` ride along; only `inputSchema` is dropped. Zero new wire fields. +**Discovery stays eager.** Names and one-liners remain in the prompt. -## Discoverability +| in the prompt | cost | +| --- | --- | +| 13 names + tight one-liners | ~150 tokens | +| 13 names + today's descriptions | 917 tokens | -The always-loaded `build-an-agent` skill (64-token description, 2,619-token body) gains one line: -"platform actions are listed by `agenta_ops`; fetch a schema with `agenta_op(op)` before calling it -with args." +If the model cannot see that `create_schedule` exists, it answers "I can't schedule things" while +holding the capability — a silent failure worse than the token bill. Carry the index in the +`load_op` description and the always-read `build-an-agent` skill. Do **not** make discovery a tool +call. --- -## Alternatives considered - -- **Op-set curation** — drop the 5-op event pack from the default overlay unless the ask is - event-driven. **Rejected.** Saves ~1,052 tokens (~5%) and cannot be decided correctly at run - start: a user who pivots to "schedule this daily" mid-conversation would find the capability - missing. A capability regression is not worth 5%. -- **M2 — dynamic real-name advertisement.** Advertise names only; a `load_op` call registers the - real spec into the harness registry mid-session (Pi re-register; Claude MCP `tools/list_changed`) - so the model calls the op by its real name with native per-op permission. **Highest fidelity, and - it dissolves most of security.md** — the gates keep seeing real names. But it needs mid-session - re-registration on both harnesses and, under the cold-replay runtime, reconstruction of the - "loaded" set each turn. If Lever B is ever justified, compare M2 against it directly rather than - treating M2 as a later upgrade — M2's extra complexity may be cheaper than four-site gate surgery. -- **Mode-gating.** Drop the build kit in Chat mode, keep it in Build mode. Removes capability rather - than deferring it; can layer on top, not a substitute. - -## Cost of laziness (Lever B) - -One extra round-trip per *distinct* op used, paid only for ops the run actually touches, cacheable -to once per op per conversation. A no-op turn pays nothing beyond the two meta-tools. Must be -measured at **session** level, counting history-resident schemas — not on a "hi" turn. +## Everything else considered + +Eight strategies were weighed, including the **card/menu invoker** that was the original proposal +and was rejected on 2026-07-27. The comparison table, the win each would deliver, and the full +permission analysis live in one place: **[alternatives.md](alternatives.md)**. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md index 68f0f7cb75..db75c8b9a7 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md @@ -1,130 +1,91 @@ -# Plan — sliced implementation - -Re-sliced 2026-07-26 so risk tracks reward. The old plan ran baseline → diet → mechanism → measure -with the mechanism as the centrepiece. The re-baseline showed the diet *is* the project; the -mechanism is a separately-justified follow-up. - -## Scope now - -- **Slices 1–2 (the diet) are the committed delivery.** They are independently shippable, touch - only `op_catalog.py` schemas, and change no runner, wire, or permission code. -- **Slice 3 (the meta-toolset) is gated on evidence**, behind a flag defaulting OFF, and does not - start until Slices 0–2 report. -- Playground platform ops only; no saved-agent change; no committed-agent behavior change. -- Each slice leaves the tree working and testable. - -## Slice 0 — Baseline ✅ done (does not block Slice 1) - -1. ~~Measurement script + per-op table.~~ **Done** — [baseline.md](baseline.md), measured - 2026-07-26: 18,353 total, top 3 ops = 88%, one duplicated schema object = 70%. -2. ~~Confirm the live advertised set.~~ **Done — all 13 ops advertise by default.** - `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` **defaults ON** (unset and empty both enable; - `platform_tools.py:41`), so `test_run` is live and 18,353 is the real figure. Earlier drafts said - "default off" with a 10,576 alternate — deleted as wrong. -3. **OPEN — answer prompt caching (blocks the Slice 3 decision only, not Slice 1).** Nothing in the - runner sets or inspects cache behavior; the harnesses own it. Determine whether these tokens are - billed per turn or only on the first. -4. Add a **resolution-path** test, not a projection test. `advertisedToolSpecs` is a pure - `specs.map(...)`, so "every input appears in the output" is a tautology that can never detect a - missing op. Instead exercise platform resolution/materialization with - `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` explicitly **on** and explicitly **off**, and assert the - resulting live advertised set (13 ops with it on/unset, 12 with it disabled). That is the - invariant Slice 3 would intentionally flip. - -**Exit:** items 1–2 recorded in baseline.md ✅; the resolution-path test passes on `main`. Item 3 is -tracked to the Slice 3 decision point and is **not** a gate on Slices 1–2. - -## Slice 1 — Diet the duplicated agent-template schema (the main win) - -Targets 70% of the bill in one file. +# Plan + +**Strategy: diet, then lazy schema.** That is the whole plan — 97% of the token bill, no permission +change, no transport work, all three harnesses. + +| Phase | Lever | From → to | Cut | Effort | +| --- | --- | ---: | ---: | --- | +| 0 | baseline | — | — | done | +| 1 | diet the two fat ops | 18,353 → ~5,500 | 70% | 1 Python file | +| 2 | diet `query_spans` | ~5,500 → ~3,000 | 84% | same file | +| 3 | lazy schema | ~3,000 → ~500 | 97% | 1 runner function | +| 4 | measure | — | — | — | + +**Lazy activation is out of scope.** Not a later phase — it is not in this plan at all until +explicitly asked for. Record: [alternatives.md](alternatives.md#3--lazy-activation-out-of-scope). + +--- + +## Phase 0 — Baseline ✅ + +1. ~~Measure per-op cost.~~ [baseline.md](baseline.md): 18,353 total, top 3 ops = 88%, one + duplicated schema object = 70%. +2. ~~Confirm the live set.~~ All 13 ops advertise; `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` defaults + **on** (`platform_tools.py:41`). +3. Add a **resolution-path** test. `advertisedToolSpecs` is a pure `specs.map(...)`, so asserting + every input appears in its output is a tautology. Instead exercise platform resolution with the + handler flag explicitly on and off, and assert the live advertised set (13 ops / 12 ops). + +**Exit:** items 1–2 recorded; resolution-path test green on `main`. + +## Phase 1 — Diet the duplicated schema + +**Win: 18,353 → ~5,500 (70%).** The single biggest step, and the cheapest. 1. In `op_catalog.py`, stop expanding `_build_agent_template_delta_schema()` inline into - `commit_revision` and `test_run`. Emit a **shallow** schema — top-level keys with one-line - descriptions, no nested `$defs` expansion — plus a description pointing at - `references/config-schema.md` (already shipped, already mandatory reading; see design.md). - Prefer implementing this as a depth limit on the expansion so it is reusable. -2. Update the platform-op / wire contract tests and goldens that pin those schemas. -3. Re-run the baseline script; record the drop (expect ~12,900). -4. Lab check: an agent still commits a valid config and `test_run` verifies it. Confirm the model - reaches for `references/config-schema.md` as the skill instructs. - -**Exit:** measured total drops to ~5,500; contract tests green; a lab run commits a revision and -passes a test run without the embedded schema. - -## Slice 2 — Trim `query_spans` - -1. Shallow the filtering-DSL `$defs` in `_QUERY_SPANS_INPUT_SCHEMA` the same way (1,463 → target - ~300), pointing at prose guidance for the DSL. -2. Re-measure. - -**Exit:** platform-op total under ~3,000 (~84% cut from 18,353). Success criteria 1 and 2 in -context.md are met **without any runner change**. - -## Decision point - -Re-read [baseline.md](baseline.md) and [security.md](security.md) with the post-diet numbers and -the caching answer in hand, then decide whether Slice 3 proceeds. It should proceed **only** if: - -- there is measured evidence of wander failures attributable to tool count, **and** -- the residual ~2,600 tokens (or the caching answer) still justifies four-site permission surgery, - **and** -- M2 (dynamic real-name advertisement) has been compared head-to-head — it may be cheaper than the - gate rework, since it keeps real tool names in front of every gate. - -If those do not hold, close the project after Slice 2 and record why. - -## Slice 3 — Discovery meta-toolset (only if the gate above opens) - -Behind a new disclosure feature flag, default off. (Unrelated to -`AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS`, which defaults *on* — see Slice 0.) - -1. Add the flag/env that turns disclosure on for a run. -2. **Add the `source:"platform"` marker** — the eligibility rule, and a prerequisite, not a - follow-up. The platform resolver stamps it; `protocol.ts` + `wire.py` + goldens change together - (research seam 6). The previously-planned "collapse all direct-`call` callback specs" heuristic - is **rejected**: `test_run` is handler-mode (`callRef`), so the heuristic would skip the single - largest target while over-collapsing an author's reference tools (design.md, "Identifying the - disclosure-eligible set"). -3. Build the **shared resolver** `resolveDisclosedTarget(toolName, args, specsByName)` first, with - its own unit tests (security.md, "Required: one shared resolver"). Everything below consumes it; - no site re-implements the lookup. -4. Disclosure transform at the two advertisement call sites (`pi-assets.ts:353`, - `environment.ts:721`): replace marked specs with `agenta_ops` + `agenta_op`; keep client tools - and everything else advertised. `plan.toolSpecs` / `toolSpecsByName` stay complete. Register the - invoker into `piToolSpecsByName` too, or the Pi gate fails closed on it. -5. **Permission work — the bulk of this slice.** Wire the shared resolver into all four gate sites: - `relay-guard.ts:53`, `acp-interactions.ts:516` (Claude), `acp-interactions.ts:456` (Pi), - `extensions/agenta.ts:318` (in-sandbox). Each must carry the **target's** name into the gate so - op-name policy rules keep matching, and `grant()`/`consume()` must key consistently. Read - [security.md](security.md) before writing any of it. -6. Invoker dispatch: `agenta_ops` returns a **bounded, filterable** `{op, one_line, read_only}` list - built runner-side (capped, and it says so when truncated); `agenta_op` describe-mode returns one - op's schema; execute-mode re-runs `assertRequiredArguments` against the **target** spec, then - dispatches on the target's shape — `call` → the direct branch, `callRef` → `callAgentaTool` — - reusing the `executeRelayedTool` core for both. -7. Tests — the full list in security.md ("Required test coverage"), per mutating op, on **both** - harness paths. Minimum: same verdict as a direct call; `allow_reads` still distinguishes read - from write; name-based policy rules still match; approvals do not cross ops; `$ctx` still binds - server-side on **both** execution branches; `test_run` is disclosed *and* executable; - describe-mode is inert; unknown `op` fails closed everywhere. -8. Add the one-line nudge to the `build-an-agent` skill. - -**Exit:** flag on → a lab run completes discover → wire → commit → schedule using only the -meta-tools; every test in security.md passes; `tsc` + `pnpm test` green in `services/runner`. - -## Slice 4 — Measure and decide default-on - -1. Re-measure at **session** level (not a no-op turn), counting history-resident fetched schemas. -2. Run the build-an-agent lab / release gate flag-on vs flag-off; compare pass rate and wander - failures. -3. Write `results.md`; decide default-on and whether M2 is worth it. - -**Exit:** `results.md` checked in; a go/no-go on default-on. + `commit_revision` and `test_run`. Emit a shallow schema (top-level keys + one-liners, no nested + `$defs`) plus a pointer to `references/config-schema.md`. Implement as a depth limit so Phase 3 + reuses it. +2. Update the contract tests and goldens pinning those schemas. +3. Re-measure (expect ~12,900 drop). +4. Lab check: an agent still commits a valid config and `test_run` verifies it. + +**No regression:** required-argument checking is unaffected — the runner re-validates against the +private spec at `relay.ts:327` / `:369` before execution. + +**Exit:** total ~5,500; tests green; a lab run commits and tests successfully. + +## Phase 2 — Trim `query_spans` + +**Win: ~5,500 → ~3,000 (84% cumulative).** Ends the token problem without touching the runner. + +Shallow the filtering-DSL `$defs` in `_QUERY_SPANS_INPUT_SCHEMA` (1,463 → ~300), pointing at prose +for the DSL. Re-measure. + +**Exit:** under ~3,000 total. + +## Phase 3 — Lazy schema + +**Win: ~3,000 → ~500 (97% cumulative).** Also the structural fix: after this, catalog growth stops +growing the prompt. + +1. In `advertisedToolSpecs` (`public-spec.ts:57`), project a **stub** schema for platform ops + instead of the full one. One site — all three harnesses inherit it. +2. Add a `load_op(op)` tool that returns the full schema as its result. Its description carries the + op index (names + one-liners), so discovery is never a round trip. +3. Keep client tools (`request_connection`, `request_input`) fully advertised — the browser fulfils + them and they are cheap. +4. One line in the `build-an-agent` skill about `load_op`. +5. Tests: an op executes with the **same** approval verdict as today; `$ctx` still binds + server-side; a call with missing required args still errors from the relay, not from the server; + an unknown `op` errors cleanly. + +**No regression:** tool names are unchanged, so every permission gate is untouched. Enforcement +moves from harness-side to runner-side, both pre-execution. + +**Cost:** one extra round-trip per distinct op used. + +**Exit:** ~500 tokens; a lab run completes discover → wire → commit → schedule, loading schemas on +demand, with no permission code changed. + +## Phase 4 — Measure + +1. Session-level tokens (not a no-op turn), flag on vs off. +2. Lab / release-gate pass rate and wander failures. +3. `results.md`; decide default-on. ## Not in this plan -- **Op-set curation** — dropped (capability regression for ~5%; see design.md alternatives). -- The direct-`call` eligibility heuristic — rejected as incorrect (misses handler-mode `test_run`); - the marker moved *into* Slice 3 as a prerequisite. -- Disclosing gateway/code/client/MCP tools. -- M2 implementation — but it must be *compared* at the decision point. +Lazy activation, card/menu invoker, op-set curation, turn-boundary activation, SSE + session ids, +mode-gating — all weighed and closed. Reasons and win comparison: +[alternatives.md](alternatives.md). diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md index a21dcb633e..89baf2a577 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md @@ -50,6 +50,14 @@ was re-checked on `main` and holds. Paths are current (`services/runner/`; some confines to the `/api` mount; `callDirect(...)` (`relay.ts:392`) sends with the caller credential. All keyed by the private `spec` — advertisement is irrelevant to execution. + **Argument validation is runner-side, against the private spec.** + `assertRequiredArguments(spec, req.args)` runs in the relay loop at `relay.ts:327` (client shape) + and `relay.ts:369` (endpoint and handler shapes), before execution. The harness also validates + pre-call against the *advertised* schema (Pi at `extensions/agenta.ts:318`, MCP at + `tool-mcp-http.ts:172`), but that is a second, earlier check — not the only one. **Consequence: + shrinking or stubbing the advertised schema does not weaken enforcement**; it moves where a + malformed call is caught, and the model still gets an error rather than a bad write. + 6. **Permission keys on the per-spec gate.** `decide(gate, plan, stored)` (`services/runner/src/permission-plan.ts:138`) → `effectivePermission` (`:125`) resolves the spec's own `permission` first, then rule match, then policy default (`allow_reads` → @@ -61,10 +69,14 @@ was re-checked on `main` and holds. Paths are current (`services/runner/`; some | Harness | Delivery | Consumes `advertisedToolSpecs` at | Notes | | --- | --- | --- | --- | | Pi (`pi_core`/`pi_agenta`) | native, via bundled extension | `pi-assets.ts:353` → `AGENTA_TOOL_PUBLIC_SPECS` | No MCP server attached to Pi. | -| Claude (`claude`) | synthetic `agenta-tools` MCP server | `environment.ts:721` | Public metadata only; execution relays back. | +| Claude (`claude`) | `agenta-tools` MCP server | `environment.ts:721` | Local: HTTP loopback (`tool-mcp-http.ts`). Daytona: in-sandbox stdio shim (`tool-mcp-stdio.ts`). | +| Codex | MCP, same channel as Claude | `environment.ts:721` | MCP-forwarding per `capabilities.ts:99`. PR #5509 open. | -Both paths consume the same projection function. Intercepting it once (or wrapping it at both -call sites behind a flag) covers both harnesses with no harness-specific logic. +All paths share one projection function, so a change there covers every harness — which is why the +**schema diet** and **lazy schema** are single-site changes. **Activation** is the exception: it is +per-harness (seam 5). Pi mutates its active-tool list in-process; the MCP harnesses need a +`tools/list_changed` notification over a transport that can push. That asymmetry is the whole reason +lazy activation is out of scope while the schema levers ship. ## Measured token cost @@ -96,8 +108,9 @@ that was wrong (see [baseline.md](baseline.md)). **Handler-mode ops take a different execution shape.** `test_run` is the only handler-mode op (`_HANDLER_CALL_REFS`, `op_catalog.py:58`). It resolves via `to_call_ref()` to a **`callRef`**, and `to_call()` explicitly raises for it (`op_catalog.py:175`). So a platform op is *not* always a -direct-`call` spec — which breaks the naive disclosure heuristic (seam 5) and the invoker's -execution path (seam 3). +direct-`call` spec. This sank the rejected invoker (it could neither identify nor execute +`test_run` — [alternatives.md](alternatives.md)); under the chosen levers it is a non-issue, since each op +keeps its own name and its own execution branch. ### The diet's replacement already exists @@ -108,52 +121,51 @@ shape. The skill body **already** instructs the model to read it before its firs commit endpoint does **not** validate the config shape — so the embedded JSON Schema enforces no server contract. It is advisory duplication of better, on-demand guidance. -## Seams the plan must pin - -Seams 1–3 and 7 apply to the meta-toolset only. **The schema diet touches none of them** — it -changes `input_schema` values in `op_catalog.py` and nothing else. - -1. **Advertisement projection** (`advertisedToolSpecs`, two call sites). Where disclosure hooks. - Must not alter what execution/permission read. Note it projects via `specInputSchema(spec)` - (`tools/spec-schema.ts:39`), the single camel/snake accessor — not `spec.inputSchema` directly. -2. **Private spec index** (`toolSpecsByName`, `public-spec.ts:34`). Must stay COMPLETE — the - invoker looks the target op up here; execution and the approval gate depend on it. -3. **Execution — TWO shapes, not one** (`relay.ts:318` → `executeAllowedRelayedTool`, - `relay.ts:361`). Endpoint-mode ops carry a direct `call` and go through - `assembleBody` → `directCallUrl` → `callDirect`. Handler-mode ops (`test_run`) carry a - **`callRef`** instead and fall through to `callAgentaTool` with `applyContextBindings`. An - invoker that only forwards `spec.call` cannot execute `test_run` at all. Note also that - `executeAllowedRelayedTool` re-runs `assertRequiredArguments(spec, req.args)` (`relay.ts:369`) — - the invoker must pass the **target** spec and the **unwrapped** args, or required-field - validation silently degrades to the invoker's loose schema. -4. **Permission decision — four sites, not one.** This is the project's dominant risk and has its - own document: **[security.md](security.md)**. In brief: the gates run *upstream of and - independently from* the relay execution path, each resolving a spec from the advertised name, in - `relay-guard.ts:53`, `acp-interactions.ts:516` (Claude), `acp-interactions.ts:456` (Pi, - fail-closed at `:473`), and `extensions/agenta.ts:318` (in-sandbox). Four inputs to - `effectivePermission` degrade at once — `readOnlyHint`, `specPermission`, name-matched policy - rules (`ruleMatches`, `permission-plan.ts:214`), and the `allow_reads` default - (`permission-plan.ts:248`). -5. **Identifying disclosure-eligible specs — harder than it looks.** Platform ops do **not** share - one shape: endpoint-mode ops carry a direct `call`, handler-mode `test_run` carries a `callRef`. - Meanwhile a `reference` (workflow) tool *also* carries a direct `call`, and a gateway (Composio) - tool *also* carries a `callRef`. So neither field identifies a platform op: - - | | direct `call` | `callRef` | - | --- | --- | --- | - | **platform op** | 12 endpoint-mode ops | `test_run` | - | **not a platform op** | author's `reference` tool | gateway (Composio) tool | - - A "collapse all direct-`call` callback specs" heuristic therefore **misses `test_run`** — 7,777 - tokens, 42% of the bill, the single largest target — while over-collapsing an author's reference - tools. There is no explicit "this is a platform op" marker on `ResolvedToolSpec` today. See - Open Question 4; the marker is now the recommended route, not the fallback. -6. **Wire contract mirroring.** If Seam 5 uses a marker, `protocol.ts` + `wire.py` + goldens change - together (`services/runner/CLAUDE.md`, "The wire contract is mirrored"). -7. **Client tools stay advertised.** `request_connection` / `request_input` must remain - model-visible (the browser fulfils them) and are cheap; disclosure skips them. -8. **Harness-side schema validation.** Pi registers each tool with its real JSON Schema - (`registerTool({parameters: specInputSchema(spec)})`, `extensions/agenta.ts:305`). Both the diet - and the meta-toolset weaken this — the diet by shallowing the schema, the meta-toolset by making - `args` opaque. Bounded for the diet (the server does not validate the config shape either); - for the meta-toolset it must be recovered by re-validating against the target spec. +## Seams + +Which lever touches which: + +| seam | diet | lazy schema | lazy activation *(out of scope)* | +| --- | --- | --- | --- | +| 1 advertisement projection | — | **yes** | — | +| 2 private spec index | — | — | active subset only | +| 3 execution | — | — | — | +| 4 permission | — | — | — | +| 5 activation | — | — | **yes, per harness** | +| 7 harness-side validation | shallower | stubbed | — | + +The diet changes `input_schema` values in `op_catalog.py` and nothing on this list. + +1. **Advertisement projection** (`advertisedToolSpecs`, `public-spec.ts:57`), consumed at + `pi-assets.ts:353` and `environment.ts:721`. It projects via `specInputSchema(spec)` + (`tools/spec-schema.ts:39`), not `spec.inputSchema` directly. **This one site is the whole of + lazy schema** — every harness reads through it. +2. **Private spec index** (`toolSpecsByName`, `public-spec.ts:34`). Stays complete under every + lever in scope; only the *active* set would change under lazy activation. +3. **Execution — two shapes.** Endpoint-mode ops carry a direct `call` + (`assembleBody` → `directCallUrl` → `callDirect`); handler-mode `test_run` carries a `callRef` + and runs through `callAgentaTool` with `applyContextBindings` (`relay.ts:361`). Both run + unchanged under every lever, because each op is still called by its own name against its own + spec. +4. **Permission.** Untouched — real names reach every gate, so `readOnlyHint`, `specPermission`, + name-matched rules (`ruleMatches`, `permission-plan.ts:214`) and the `allow_reads` default + (`:248`) all behave as today. (The rejected card/menu invoker broke all four; see + [alternatives.md](alternatives.md).) +5. **Activation, per harness.** Pi: `getAllTools`/`getActiveTools`/`setActiveTools` + (`extensions/agenta.ts:215`) — in-process, mid-turn, no transport. MCP harnesses: + `notifications/tools/list_changed`, which requires a server→client push channel — plumbable on + the stdio shim, absent on the local HTTP server (*"stateless JSON mode… no SSE… 405 for `GET`"*, + `tool-mcp-http.ts:22`). `initialize` currently advertises `capabilities: {tools: {}}` (`:124`), + so `listChanged` is not declared. Whether the pinned ACP clients honor it mid-turn is unverified. + **This seam is why lazy activation is out of scope.** +6. **Client tools stay advertised.** `request_connection` / `request_input` must remain + model-visible (the browser fulfils them) and are cheap. +7. **Harness-side schema validation.** Pi registers each tool with its real JSON Schema + (`extensions/agenta.ts:305`) and checks it at `:318`; the MCP server checks at + `tool-mcp-http.ts:172`. Both are *earlier* copies of a check the relay repeats against the + private spec (`relay.ts:327`, `:369`), so the diet shallowing this — or lazy schema stubbing it — + costs an early error message, not enforcement. +8. **Codex.** MCP-forwarding (`capabilities.ts:99`), so it inherits the MCP activation path. It + wraps calls as `{server, tool, arguments}`, already handled by `unwrapCodexMcpArgs` in + `storedDecisionKeyShape` (PR #5509). That fix normalized the **arguments** and left the **tool + name real** — the same rule every lever here follows. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md deleted file mode 100644 index e14ae26ebe..0000000000 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/security.md +++ /dev/null @@ -1,197 +0,0 @@ -# Security — what collapsing 13 ops behind one tool name breaks - -**Verified 2026-07-26** against the runner on `main`. This is the risk register for the -**discovery meta-toolset** (Slice 3). It does **not** apply to the schema diet (Slices 1–2), which -changes no name, no spec, and no gate. - -Earlier drafts covered this in one bullet ("the invoker builds the gate from the target op's -spec"). That understates it by an order of magnitude, and it also puts the work in the wrong -place. The gates do **not** run inside the invoker's dispatch — they run *upstream of and -independently from* the relay execution path, on both harnesses. - -## Root cause: `toolName` is the discriminator for every permission decision - -Today each op arrives under its own name, and that name does two jobs: it **resolves the private -spec** the gate reads its fields from, and it is **itself matched** by policy rules. The decision -ladder (`effectivePermission`, `services/runner/src/permission-plan.ts:125`) consumes both: - -```text -gate.specPermission (from resolved spec) -> short-circuits the rest when set -gate.serverPermission (MCP server) -> n/a for platform ops -matchingRulePermission(gate, rules) -> matches on gate.toolName itself -defaultPermission(mode, gate) -> allow_reads: gate.readOnlyHint ? allow : ask -``` - -**The regression is not disclosure itself — it is building the gate from the invoker's spec instead -of re-resolving the target.** Get that right at every site and each input above keeps working; -`gate.toolName` restored to the target name even keeps existing op-name rules matching. Get it wrong -at any one site and that site silently evaluates thirteen ops against one spec. - -The sections below are therefore **failure modes to design against**, not inevitable outcomes. They -are listed because each has a distinct fix and a distinct test, and because the fix has to land in -four places (see "The four sites"). - -### 1. `readOnlyHint` — the `allow_reads` default (the core failure mode) - -`defaultPermission` (`permission-plan.ts:248`) is the whole policy for the default `allow_reads` -mode: - -```ts -if (mode === "allow_reads") return gate.readOnlyHint === true ? "allow" : "ask"; -``` - -`readOnlyHint` comes from `spec.readOnly`, which the catalog sets per op -(`op_catalog.py`: `query_spans` read-only, `commit_revision`/`remove_schedule` not). The invoker -is **one spec with one `readOnly` value covering thirteen ops**, so: - -- `readOnly: true` → **every write executes silently.** `commit_revision`, `remove_schedule`, - `remove_subscription` lose their approval prompt. This is the security regression. -- `readOnly: false` → **every read prompts.** `list_schedules` and `query_spans` now interrupt the - user. Habituated click-through is a worse outcome than not prompting at all. - -There is no correct static value. The gate must resolve `readOnly` from the *target* op per call. - -### 2. `specPermission` — same collapse, higher priority - -`spec.permission` short-circuits the whole ladder (`permission-plan.ts:129`). One invoker spec -means one permission for thirteen ops, and it wins over any rule. - -### 3. Policy rules stop matching — silent fall-through (not previously documented) - -`ruleMatches` (`permission-plan.ts:214`) compares the pattern against `gate.toolName` **exactly**, -or parses a `tool(prefix:*)` form that still requires `prefixPattern.toolName === gate.toolName`. - -This one is unique in the list: it keys on the **name itself**, not on the resolved spec. Any -operator- or config-authored rule written against an op name — `commit_revision: ask`, -`remove_schedule: deny` — **stops matching** if the call reaches the ladder still named `agenta_op`. -It does not error; it falls through to the default, so a `deny` rule silently becomes whatever -`allow_reads` decides. - -**Fix:** set `gate.toolName` to the *target* op's name (not the invoker's) before `decide()` runs. -Then existing rules keep matching unchanged and no operator has to rewrite a policy. That -substitution has to happen at all four sites, which is exactly why the shared resolver below -returns a `gateName` rather than leaving each site to name the gate itself. - -### 4. Grant / stored-decision keys — safe, but must stay consistent - -Both the turn's grant ledger (`ApprovedExecutionGrants`, `responder.ts:85`) and the replayed -approval store (`ConversationDecisions.take`, `responder.ts:255`) key on -`approvedCallKey(toolName, args)` — **name plus a canonical hash of the arguments**, not name -alone. - -Because the invoker's args contain the target op (`{op: "remove_schedule", args: {…}}`), distinct -ops produce distinct keys. **An approval for one op does not unlock another.** Granularity is -preserved for free. - -> Correction to an earlier review note: there is no cross-op approval-leak here. The args are part -> of the key. - -The residual requirement is **consistency**: `grant()` and `consume()` must key on the same name -(both the target's, or both the invoker's). Mismatch fails *closed* — every Pi write is refused -with "was not approved via the permission dialog" — which is a functional break, not a security -one, but it will look like a mysterious bug. - -## The four sites that must each resolve the target - -Each independently resolves a spec from a name and builds its own `GateDescriptor`. All four need -the same "if this is the invoker, re-resolve from `args.op`" logic. **Missing one does not fail -closed — it uses the invoker's permission, which is the fail-open direction if the invoker is -marked read-only.** - -| # | Site | Path | How it resolves today | -| --- | --- | --- | --- | -| 1 | `buildRelayExecutionGuard` | `engines/sandbox_agent/relay-guard.ts:53` | reads `spec.name` / `spec.permission` / `spec.readOnly` from the spec the relay looked up by `req.toolName` | -| 2 | `buildGateDescriptor` | `engines/sandbox_agent/acp-interactions.ts:516` | Claude/ACP dialog; `toolSpecsByName.get(bareToolName(displayName))` | -| 3 | `buildPiGateDescriptor` | `engines/sandbox_agent/acp-interactions.ts:456` | Pi dialog; **fails closed on an unknown name** (`:473`) — the invoker must be present in `piToolSpecsByName` | -| 4 | `piDialogAllows` call | `extensions/agenta.ts:318` | in-sandbox; passes `spec.name` and raw `params` up the gate envelope | - -Site 4 has an extra wrinkle: the in-sandbox extension only ever sees the **advertised** set -(`AGENTA_TOOL_PUBLIC_SPECS`). Under disclosure it no longer holds the target's `readOnly` or -`permission`, so it cannot decide `gateViaDialog` locally and must forward raw args for the runner -to classify. That is arguably *better* (the trust decision moves fully runner-side), but it is a -real behavior change on a path whose current design assumes the sandbox knows the tool it called. - -## Required: one shared resolver, not four implementations - -Four copies of a security-sensitive lookup, where a missed copy fails **open**, is the wrong shape. -The mitigation is structural, and it is a hard requirement of the design rather than an -implementation detail. - -Define exactly one runner-side primitive and have all four sites consume it: - -```ts -resolveDisclosedTarget(toolName, args, specsByName) - -> { spec, gateName, args } | undefined -``` - -- Returns `undefined` when `toolName` is not the invoker, so every non-disclosed call keeps its - current path byte-for-byte. -- Validates `args.op` against the **same fixed spec map** the gates already trust, returning - `undefined` for unknown or malformed values — so an unresolvable target fails closed everywhere, - by construction rather than by four separate `if` statements. -- Returns `gateName` = the **target op's** name, so the caller's `GateDescriptor` carries it into - `ruleMatches` and existing op-name policy rules keep matching (failure mode 3). -- Returns the **unwrapped** inner args, so `assertRequiredArguments` and the grant/decision keys all - operate on the same canonical value at every site (failure mode 4's consistency requirement). - -With this in place, "resolve the target" becomes one reviewable function with one test suite, and -the four sites reduce to a two-line call each. Without it, the review surface is four hand-written -lookups in code paths that are individually hard to exercise. - -## What is *not* a new risk - -> Correction to an earlier review note: routing the decision through a sandbox-supplied `args.op` -> field is **not** a new trust exposure. - -The tool **name** is already sandbox-supplied today — the Pi gate envelope is written from inside -the sandbox, and `acp-interactions.ts:307` says so explicitly ("The envelope is sandbox-origin and -untrusted"). It is safe because the name is validated against a fixed map of known specs and fails -closed otherwise (`:473`). Reading `args.op` and validating it against the same map is the -identical pattern at the identical trust level. - -The real cost is **surface area**, not trust level: the validated-lookup pattern now has to be -duplicated correctly in four places instead of living in one. - -## Non-permission risks - -- **Loss of pre-call argument validation.** Pi registers each tool with its real JSON Schema - (`registerTool({parameters: specInputSchema(spec)})`, `extensions/agenta.ts:305`) and - `assertRequiredArguments` checks required fields (`:318`, and again in the relay at - `tools/relay.ts:369`). A generic invoker makes `args` an opaque blob, so both checks degrade to - the invoker's loose schema. Malformed op calls stop being caught before dispatch and surface as - server 4xx instead. Mitigated by having the invoker re-run `assertRequiredArguments` against the - **target** spec before execution — cheap, and it should be an explicit requirement. -- **Fetched schemas become history-resident.** A schema moved out of the prompt into a tool result - does not disappear — it sits in the conversation transcript for the rest of the session, in a - position far less cache-friendly than a stable tool-definition prefix. For a real build session - (as opposed to a "hi" turn), disclosure **without** the diet can cost more than today. This is a - strong argument for the diet landing first regardless. - -## Required test coverage before this ships - -Non-negotiable, per mutating op (`commit_revision`, `create_schedule`, `create_subscription`, -`remove_schedule`, `remove_subscription`, `annotate_trace`, `test_run`): - -1. `agenta_op` execute-mode produces the **same verdict** as calling the op directly, on **both** - harness paths — not just the relay guard. -2. Under `allow_reads`, read-only ops execute without a prompt and writes still prompt. -3. A policy rule written against an **op name** still matches through the invoker. -4. An approval granted for one op does **not** satisfy a later call to a different op. -5. `$ctx` bindings still fill server-side and the model cannot retarget another variant — on - **both** execution branches: `assembleBody`/`call.context` for endpoint-mode ops, and - `applyContextBindings` for handler-mode `test_run`. A test that only covers `commit_revision` - misses half the mechanism. -6. Describe-mode has no side effect, triggers no approval, and cannot execute. -7. An unknown or malformed `op` value fails **closed** on all four sites. -8. `test_run` (the only handler-mode op) is disclosed *and* executable through the invoker — - guarding against the direct-`call`-only eligibility and dispatch bugs that the first draft had. -9. `resolveDisclosedTarget` is unit-tested directly against the failure modes above, so the four - call sites only need a thin integration assertion each. - -## Recommendation - -The permission rework is the dominant cost and risk of this project, and it buys ~14% of the token -bill (see [baseline.md](baseline.md)). **Land the schema diet first and independently** — it -touches none of this. Only open the meta-toolset once the diet is measured, prompt caching is -understood, and there is a demonstrated reliability problem that fewer visible tools actually -fixes. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md deleted file mode 100644 index 39305e9661..0000000000 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/status.md +++ /dev/null @@ -1,108 +0,0 @@ -# Status - -**Last updated:** 2026-07-26 - -## Current stage - -Planning workspace revised after a re-baseline and a code-grounded security review. No code -written, nothing committed. - -**What changed on 2026-07-26.** The token cost was re-measured against the live catalog -([baseline.md](baseline.md)) and the permission plane was traced end to end -([security.md](security.md)). Both moved the plan: - -1. The bill is **18,353** tokens, not ~15,454 (the 2026-07-17 figures were ~19% low). -2. The cost is **concentrated, not spread**: three ops are 88%, and one schema object embedded - twice is 70%. The ten-op tail is 2,120 tokens combined. -3. Therefore the **schema diet is the project** (~84% of the win, no runner change), and the - meta-toolset is a follow-up worth ~14% more. -4. The meta-toolset's permission work is a **four-site problem in fail-closed security code**, not - the one-line "gate on the target spec" the earlier draft described. - -## Locked decisions - -- **Diet first, alone, and it is the committed delivery** (plan Slices 1–2). Near-zero risk: the - replacement guidance (`references/config-schema.md`) already ships with the skill, the skill - already mandates reading it before `commit_revision`, and the commit endpoint does not validate - the config shape anyway. -- **Shallow schema, not open object.** Keep top-level keys with one-line descriptions; stop before - expanding nested `$defs`. Prefer a depth limit on the expansion so it is reusable. -- **Meta-toolset deferred behind an evidence gate**, not cancelled. Gate: post-diet numbers + - a caching answer + demonstrated wander evidence. Compare M2 head-to-head at that point. -- **Op-set curation dropped** (not deferred). Hiding the 5-op event pack saves ~1,052 tokens (~5%) - and cannot be decided correctly at run start — a user pivoting to "schedule this daily" - mid-conversation would find the capability gone. Not worth a capability regression. -- **Success metric is session-level, not a no-op turn.** The "hi" turn is reported, never targeted. -- Seam for the meta-toolset = the runner advertisement layer (`advertisedToolSpecs`, - `public-spec.ts:57`), consumed at `pi-assets.ts:353` and `environment.ts:721`. Verified - 2026-07-26. -- Skills, external `discover_tools`, and non-platform tool types stay out of scope. -- Playground overlay only — no saved/committed-agent change. -- No commits during planning; implementation is a later, separate branch. - -## Corrections to earlier drafts - -Recorded so the reasoning is not re-litigated: - -- **Approval leak across ops — not real.** Both the grant ledger and the replayed decision store key - on `approvedCallKey(toolName, args)`, i.e. name **plus** a canonical args hash. Since the invoker's - args carry the op name, distinct ops produce distinct keys. Granularity survives for free. -- **Sandbox-supplied `args.op` — not a new trust exposure.** The tool *name* is already - sandbox-supplied today (`acp-interactions.ts:307`, "the envelope is sandbox-origin and - untrusted") and is safe because it is validated against a fixed spec map. Reading `args.op` and - validating it the same way is the same pattern at the same trust level. The real cost is - duplicating that validated lookup correctly in four places. -- **New finding not in the first draft:** name-matched **policy rules** silently stop matching. - `ruleMatches` (`permission-plan.ts:214`) compares `gate.toolName` exactly, so any rule written - against an op name (`commit_revision: ask`, `remove_schedule: deny`) falls through to the default - once calls arrive as `agenta_op`. It does not error. *Fix: carry the target's name into the gate, - so existing rules keep matching and no operator rewrites a policy.* -- **"Disclosure inevitably breaks permission" — overstated.** Every failure mode in security.md is - fixable by re-resolving the target; the risk is that the fix must land correctly in four - fail-closed sites, not that the approach is unsound. security.md now frames them as failure modes - to design against, and requires a single shared resolver so there is one implementation instead of - four. *(Raised by CodeRabbit, 2026-07-26.)* -- **Handler flag "default off" — wrong, corrected.** It defaults **on**; see "Closed since the last - revision" below. *(Raised by CodeRabbit, 2026-07-26.)* -- **Direct-`call` eligibility heuristic — wrong, rejected.** It misses handler-mode `test_run`, the - largest single target; the marker is now the rule. See below. *(Raised by CodeRabbit, 2026-07-26.)* -- **`agenta_ops` "flat regardless of catalog size" — imprecise.** The *advertised prompt cost* is - flat; the *result payload* is O(n) and must be bounded/filterable. design.md corrected. - -## Open questions - -**All of these gate the Slice 3 decision. None gate Slices 1–2 (the diet) — start those now.** - -1. **Prompt caching.** Are these tokens billed every turn, or cached after the first? Nothing in the - runner sets or inspects cache behavior. This sets the ROI of everything past the diet. - *Owner: needs a measurement on a real run.* -2. **Wander evidence.** Is there measured evidence that tool *count* causes run failures? The - internal-tools review asserts it; the meta-toolset's whole remaining case rests on it. -3. **M2 vs Lever B.** M2 (dynamic real-name advertisement) keeps real names in front of every gate - and so dissolves most of security.md. Is its mid-session re-registration cheaper than four-site - gate surgery? Compare at the decision point, not after building Lever B. -4. **Invoker shape** — one `agenta_op` with describe/execute modes, or two tools? - *Recommendation: one.* - -## Closed since the last revision - -- **Live advertised set — CLOSED.** All 13 ops advertise by default. - `AGENTA_AGENT_ENABLE_PLATFORM_HANDLERS` **defaults ON** (`platform_tools.py:41`: unset → `True`, - empty → enabled; only an explicit `off`/`false`/`0` disables). The "default off" claim and its - 10,576 alternate figure came from the 2026-07-17 investigation and were **wrong**. 18,353 is the - live number and Slice 1's win is the full ~12,900. -- **Disclosure-eligible identification — CLOSED, and the earlier recommendation was wrong.** The - proposed "collapse all direct-`call` callback specs" heuristic **misses `test_run`**, which is - handler-mode and resolves to a `callRef` (`op_catalog.py:175`) — 7,777 tokens, 42% of the bill, - the largest single target. A direct-`call` rule also over-collapses an author's `reference` tools. - **Decision: the `source:"platform"` marker is the eligibility rule**, and it moves into Slice 3 as - a prerequisite rather than a post-POC follow-up. - -## Next action - -**Start Slice 1 (the schema diet) now.** No open question gates it: the live-set question is closed -(all 13 ops advertise, so the full ~12,900 win is real), and the caching question only affects -whether Slice 3 is worth doing at all. - -Run the caching measurement (open question 1) in parallel, on its own timeline — it feeds the -Slice 3 decision point, not the diet. From 0aadf3963c6e0751d676d8389841d434da18ef55 Mon Sep 17 00:00:00 2001 From: ashrafchowdury Date: Mon, 27 Jul 2026 13:26:36 +0600 Subject: [PATCH 5/5] Refine documentation for progressive tool disclosure project: clarify argument handling, schema constraints, and token cost implications. --- .../progressive-tool-disclosure/README.md | 6 ++-- .../alternatives.md | 5 +-- .../progressive-tool-disclosure/baseline.md | 21 +++++++---- .../progressive-tool-disclosure/context.md | 4 ++- .../progressive-tool-disclosure/design.md | 35 +++++++++++++++---- .../progressive-tool-disclosure/plan.md | 26 +++++++++----- .../progressive-tool-disclosure/research.md | 6 ++-- 7 files changed, 74 insertions(+), 29 deletions(-) diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md index 367dcba9c6..47a76235e8 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/README.md @@ -74,8 +74,10 @@ Neither blocks the plan; both would only matter if lazy activation is ever asked - **Handler flag "default off" — wrong.** It defaults **on** (`platform_tools.py:41`, unset and empty both enable). All 13 ops advertise; 18,353 is live. *(CodeRabbit, 2026-07-26.)* -- **"Malformed args become a server error" — overstated.** `assertRequiredArguments` runs - runner-side against the private spec (`relay.ts:327`, `:369`) before execution. +- **"Malformed args become a server error" — overstated, then re-sharpened.** + `assertRequiredArguments` runs runner-side against the private spec (`relay.ts:327`, `:369`) + before execution, so lazy schema loses nothing. The diet does lose nested `required` checks, + because it shrinks the private spec too. *(CodeRabbit, 2026-07-27.)* - **Direct-`call` eligibility heuristic — wrong.** It missed handler-mode `test_run`, 42% of the bill. Moot now: real names need no eligibility rule. *(CodeRabbit, 2026-07-26.)* - **Approval leak across ops — not real.** Grant and decision stores key on diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md index e4d9cc53e1..03936036bd 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/alternatives.md @@ -59,8 +59,9 @@ the server to *push* `notifications/tools/list_changed`. Three problems stack: server" design. 2. **The shim's tool list is a startup snapshot.** `tool-mcp-stdio.ts` reads specs once from `AGENTA_AGENT_TOOLS_PUBLIC_SPECS_FILE`. It would need a runner signal over the relay dir to - re-read and notify, plus `capabilities: {tools: {listChanged: true}}` at `initialize` — today it - sends `{tools: {}}` (`tool-mcp-http.ts:124`). + re-read and notify, plus `capabilities: {tools: {listChanged: true}}` at `initialize` — today the + shim sends `{tools: {}}` (`tool-mcp-stdio.ts:183`), as does the HTTP server + (`tool-mcp-http.ts:124`). 3. **The clients may ignore the notification mid-turn.** `@agentclientprotocol/claude-agent-acp` and `codex-acp` are third-party and version-pinned. Whether either re-lists **mid-turn** is unverified. If they only re-list on reconnect, activation lands at a turn boundary — the agent diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md index 2ef5a77e23..a26d16bcf4 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/baseline.md @@ -64,7 +64,8 @@ stripped), matching `advertisedToolSpec()` in `services/runner/src/tools/public- Two numbers decide the plan's shape: - **Lazy schema's marginal win over a completed diet is ~2,500 tokens (~13%)** at the cost of one - function in `public-spec.ts` — and it is structural, so catalog growth stops inflating the prompt. + function in `public-spec.ts` — and it is structural: schema growth stops inflating the prompt. + (The eager discovery index still grows by ~12 tokens per new op.) - **Lazy activation's marginal win over lazy schema is ~100 tokens**, which is why it is out of scope ([alternatives.md](alternatives.md#3--lazy-activation-out-of-scope)). Its case is tool *count* (wander), not tokens. Weigh its per-harness transport cost against wander evidence only. @@ -83,12 +84,18 @@ mandatory reading**: embedded JSON Schema is not enforcing a server contract — it is model guidance that a better-written, on-demand doc already provides. -**Required-argument checking is not lost.** The runner re-validates against the **private** spec -before executing — `assertRequiredArguments(spec, req.args)` at `services/runner/src/tools/relay.ts:327` -(client shape) and `:369` (endpoint and handler shapes). What moves is *where* a malformed call is -caught: at the relay rather than pre-call in the harness (Pi passes `inputSchema` to -`registerTool({parameters})`, `services/runner/src/extensions/agenta.ts:305`). Either way the model -gets an error before anything executes. +**Top-level required-argument checking is not lost.** The runner re-validates against the +**private** spec before executing — `assertRequiredArguments(spec, req.args)` at +`services/runner/src/tools/relay.ts:327` (client shape) and `:369` (endpoint and handler shapes). +What moves is *where* a malformed call is caught: at the relay rather than pre-call in the harness +(Pi passes `inputSchema` to `registerTool({parameters})`, +`services/runner/src/extensions/agenta.ts:305`). + +**What the diet does cost:** `missingRequiredFields` recurses through `properties`, so today a deep +schema enforces nested `required` too. The diet edits `op_catalog.py`, which shrinks the *private* +spec as well, so those nested checks go away at every layer. Bounded by the same fact that makes the +diet safe: the commit endpoint does not validate the config shape either. Lazy schema does **not** +have this cost — it stubs only the advertisement and leaves the private spec whole. ## Reproducing diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md index 5da987453e..12ea0d329b 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/context.md @@ -61,7 +61,9 @@ The tail is *cheap* — ten ops for 2,120 tokens — so catalog growth is not th 1. **Diet:** platform-op prompt cost drops from 18,353 to under ~3,000 tokens with no capability loss — a lab agent still commits a valid config and runs a test. -2. **Lazy schema:** under ~500 tokens, and adding a catalog op no longer grows the prompt. +2. **Lazy schema:** under ~500 tokens, and adding a catalog op grows the prompt by its index + entry only (name + one-liner, ~12 tokens), never by its schema. Discovery stays eager, so the + index itself still scales linearly; the target is that *schema* growth stops. 3. **Session-level, not turn-level:** total platform-op tokens across a full build session (discover → wire → commit → test → schedule) drop materially. A no-op turn is *reported*, never the target. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md index e719527158..25c6b9e65a 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/design.md @@ -39,10 +39,26 @@ Schema enforces nothing — it duplicates better guidance the model already has A few hundred tokens instead of 6,441. Implement as a depth limit on `expand_type_refs` so it is reusable by Lever 2. -**Cost: none that reaches execution.** Required-argument checking is not lost — the runner -re-validates against the private spec at `relay.ts:327` and `relay.ts:369`, covering both execution -shapes, before anything runs. What changes is *where* a malformed call is caught: at the relay -instead of pre-call in the harness. Either way the model gets an error, not a bad write. +**Cost: nested required-field checks inside the collapsed subtree.** Be precise about this, because +the two levers differ. `missingRequiredFields` (`spec-schema.ts`) walks `properties` recursively, so +a deep schema today enforces nested `required` as well as top-level. The diet edits `op_catalog.py`, +which changes the **private** spec too, so those nested checks disappear from every layer including +the relay. Top-level required fields are unaffected. + +That is acceptable here and only here: the commit endpoint does not validate the config shape +either, so a malformed nested config already fails at the server rather than being written. It is +not a general licence — see Lever 2, where the private schema is untouched and nothing is lost. + +**The shallow schema must be permissive.** Harnesses validate against the *advertised* schema before +the relay sees the call (Pi at `extensions/agenta.ts:318`, MCP at `tool-mcp-http.ts:172`), and Pi's +own framework may apply its JSON Schema more strictly than our required-only check. So the depth +limit must not tighten anything: + +- no `additionalProperties: false` on a collapsed node, +- no `required` beyond what the deep schema already required, +- collapsed subtrees typed as bare `object`, never a narrower type or `enum`. + +A depth limit that only *removes* constraints cannot reject a payload the deep schema accepted. --- @@ -61,8 +77,15 @@ every harness shares — Pi reads it at `pi-assets.ts:353`, the MCP path at `env Stub there and all three harnesses get it. No notification, no transport work, no per-harness code. **Why it is safe:** the tool name never changes, so every permission gate behaves exactly as today. -And as with Lever 1, `assertRequiredArguments` still runs runner-side against the private spec -(`relay.ts:327`, `:369`) — the stub is a prompt-side projection, not a weakening of enforcement. +And unlike Lever 1, the **private spec is untouched** — the stub lives only in the advertisement +projection, so `assertRequiredArguments` at `relay.ts:327` / `:369` still enforces the full schema, +nested `required` included. Nothing is lost; the check simply moves from the harness to the relay. + +**The stub must be permissive too**, for the same reason as Lever 1 and more so, since it is the +only schema the harness sees: `{type: "object"}` with no `required` and no +`additionalProperties: false`. Required fields are then reported by the relay with the exact +field names, which is a usable error for the model. Do **not** stub client tools +(`request_connection`, `request_input`) — the browser fulfils them and they are cheap. There is precedent for exactly this rule. Codex wraps MCP calls as `{server, tool, arguments}`, which forced `unwrapCodexMcpArgs` into `storedDecisionKeyShape` (`permission-plan.ts`, PR #5509) so diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md index db75c8b9a7..079bf1da0b 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/plan.md @@ -39,9 +39,13 @@ explicitly asked for. Record: [alternatives.md](alternatives.md#3--lazy-activati 2. Update the contract tests and goldens pinning those schemas. 3. Re-measure (expect ~12,900 drop). 4. Lab check: an agent still commits a valid config and `test_run` verifies it. +5. Assert the depth limit only *removes* constraints: no `additionalProperties: false` and no new + `required` on a collapsed node. Test a deeply nested valid config through **both** the Pi and MCP + paths and confirm it is not rejected pre-relay. -**No regression:** required-argument checking is unaffected — the runner re-validates against the -private spec at `relay.ts:327` / `:369` before execution. +**Known cost:** nested `required` inside the collapsed subtree stops being checked, because the diet +changes the private spec too. Top-level required fields still are. Accepted because the commit +endpoint does not validate the config shape either. **Exit:** total ~5,500; tests green; a lab run commits and tests successfully. @@ -56,22 +60,26 @@ for the DSL. Re-measure. ## Phase 3 — Lazy schema -**Win: ~3,000 → ~500 (97% cumulative).** Also the structural fix: after this, catalog growth stops -growing the prompt. +**Win: ~3,000 → ~500 (97% cumulative).** Also the structural fix: after this a new op costs its +index entry (~12 tokens), not its schema. Discovery stays eager, so the index still grows linearly; +what stops is *schema* growth. 1. In `advertisedToolSpecs` (`public-spec.ts:57`), project a **stub** schema for platform ops instead of the full one. One site — all three harnesses inherit it. 2. Add a `load_op(op)` tool that returns the full schema as its result. Its description carries the op index (names + one-liners), so discovery is never a round trip. -3. Keep client tools (`request_connection`, `request_input`) fully advertised — the browser fulfils +3. Keep the stub permissive: `{type: "object"}`, no `required`, no `additionalProperties: false`. + Client tools (`request_connection`, `request_input`) are **not** stubbed — the browser fulfils them and they are cheap. 4. One line in the `build-an-agent` skill about `load_op`. 5. Tests: an op executes with the **same** approval verdict as today; `$ctx` still binds - server-side; a call with missing required args still errors from the relay, not from the server; - an unknown `op` errors cleanly. + server-side; a valid deeply-nested payload passes on both the Pi and MCP paths; a call missing a + required arg errors from the relay naming the field, not from the server; an unknown `op` errors + cleanly. -**No regression:** tool names are unchanged, so every permission gate is untouched. Enforcement -moves from harness-side to runner-side, both pre-execution. +**No regression:** tool names are unchanged, so every permission gate is untouched. The private +spec is untouched too, so the relay still enforces the full schema including nested `required`. +Enforcement moves from harness-side to runner-side, both pre-execution. **Cost:** one extra round-trip per distinct op used. diff --git a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md index 89baf2a577..6e2d9aa7b9 100644 --- a/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md +++ b/docs/design/agent-workflows/projects/progressive-tool-disclosure/research.md @@ -55,8 +55,10 @@ was re-checked on `main` and holds. Paths are current (`services/runner/`; some and `relay.ts:369` (endpoint and handler shapes), before execution. The harness also validates pre-call against the *advertised* schema (Pi at `extensions/agenta.ts:318`, MCP at `tool-mcp-http.ts:172`), but that is a second, earlier check — not the only one. **Consequence: - shrinking or stubbing the advertised schema does not weaken enforcement**; it moves where a - malformed call is caught, and the model still gets an error rather than a bad write. + stubbing the *advertisement* alone does not weaken enforcement** (lazy schema); it moves where a + malformed call is caught. Shrinking the *private* schema does (the diet), because + `missingRequiredFields` walks `properties` recursively, so nested `required` is only enforced + while the private schema still describes it. 6. **Permission keys on the per-spec gate.** `decide(gate, plan, stored)` (`services/runner/src/permission-plan.ts:138`) → `effectivePermission` (`:125`) resolves the