mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
1409195138
## What `run-sequence` tells agents: > One screenshot is captured automatically after the whole sequence (not per step) The MCP-side half of that promise is already pinned — argent-mcp's auto-screenshot tests name `run-sequence` explicitly, twice. The tool-server half is pinned by nothing: this adds the gate that pins it, plus the renderer-side pin in argent-mcp. **No behavior change.** ## Why it is unpinned The steps stay silent only because no tool `ALLOWED_TOOLS` admits returns an image or an artifact handle. Nothing checks: - which tools the allow-list admits, - that every admitted name still resolves to a registered tool (a stale name still reads as allowed and stays advertised in the description, but every step naming it dies on `ToolNotFoundError` and halts the sequence), - that none of them declares `outputHint: "image"`. Admitting a frame-producing tool would go unnoticed: a sequence result reaches `toMcpContent` whole, and its `materializeArtifacts` walk collects handles from anywhere inside it, `steps[].result` included, appending one image block per handle. That walk recurses into arrays with `Promise.all`, so those frames arrive in **completion order, not step order** — the agent could not even tell which one is the screen the sequence ended on. Both halves are verified against the code, not assumed: - Putting an artifact handle in one step's result makes `toMcpContent` emit an image block for it, confirming the walk reaches `steps[].result`. - Materializing three step handles whose fetches complete fastest-last yields `[s2, s1, s0]` — exactly reversed from step order. ## What is gated **tool-server** — `test/run-sequence-observation-gate.test.ts`, against the real registry (`createRegistry()`, no device). `ALLOWED_TOOLS` is exported for it: - the allow-list admits exactly the reviewed set of 14 step tools - every name still resolves to a registered tool - none of them declares `outputHint: "image"` **argent-mcp** — `test/content.test.ts`: - a multi-step sequence result renders with no frame of its own, against a `fetchImpl` that resolves and is asserted never to be called (without a resolving fetch the assertion would hold equally for a sequence whose step frames merely failed to download, since `materializeArtifacts` swallows a failed download and renders the handle as nothing) - the positive control that pin rests on: a handle on `steps[].result` does surface as an image block, so the walk ceasing to reach steps cannot turn the pin vacuous The membership of `run-sequence` in `AUTO_SCREENSHOT_TOOLS` was already pinned by existing assertions and needed no new test. ## Verification Every test was mutation-checked — each fails on the broken code, not just passes on the good: | Mutation | Fails | | --- | --- | | add `screenshot` to `ALLOWED_TOOLS` | reviewed-set + `outputHint` checks | | rename an allow-list entry the registry lost | reviewed-set + registration checks | | give a step result an image artifact handle | the frameless-sequence check, on both its fetch-call and its image-block assertion | | stop `materializeArtifacts` recursing into arrays | the step-carried-handle check, and nothing else in the file | Locally green: `prettier --check`, `tsc --build`, `typecheck:tests` for both touched packages, the full argent-mcp suite (82 tests), and the gate test (3 tests). ## Scope note The allow-list is spelled out rather than counted, so adding a step tool means answering the question the file exists to ask — does it put a frame of its own in front of the agent? The gate deliberately sits on the allow-list rather than on the renderer: a tool that would break the guarantee is caught before it can be dispatched as a step, rather than having its frames filtered afterwards. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3 lines
30 B
YAML
3 lines
30 B
YAML
reviews:
|
|
profile: assertive
|