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>
71 lines
2.8 KiB
TypeScript
71 lines
2.8 KiB
TypeScript
/**
|
|
* Gate on the guarantee run-sequence advertises to agents: "One screenshot is
|
|
* captured automatically after the whole sequence (not per step)".
|
|
*
|
|
* Nothing in run-sequence enforces it. The capture is appended once, by the MCP
|
|
* layer, because `run-sequence` is in that layer's AUTO_SCREENSHOT_TOOLS
|
|
* (pinned in argent-mcp's auto-screenshot test). The steps stay silent only
|
|
* because no tool the allow-list admits returns an image or an artifact handle.
|
|
*
|
|
* A step that returned one would be inlined: the 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 rather than step order, leaving the agent
|
|
* unable to tell which is the screen the sequence ended on. Hence the gate sits
|
|
* on the allow-list: such a tool has to be caught before it can be a step.
|
|
*/
|
|
import { describe, it, expect } from "vitest";
|
|
import { ALLOWED_TOOLS } from "../src/tools/run-sequence";
|
|
import { createRegistry } from "../src/utils/setup-registry";
|
|
|
|
// Spelled out rather than counted: adding an entry means answering the question
|
|
// this file exists to ask — does that tool put a frame of its own in front of
|
|
// the agent?
|
|
const EXPECTED_ALLOWED_TOOLS = [
|
|
"await-ui-element",
|
|
"button",
|
|
"gesture-custom",
|
|
"gesture-drag",
|
|
"gesture-pinch",
|
|
"gesture-rotate",
|
|
"gesture-scroll",
|
|
"gesture-swipe",
|
|
"gesture-tap",
|
|
"keyboard",
|
|
"paste",
|
|
"rotate",
|
|
"shake",
|
|
"tv-remote",
|
|
];
|
|
|
|
describe("run-sequence — one observation per sequence", () => {
|
|
it("admits exactly the reviewed set of step tools", () => {
|
|
expect([...ALLOWED_TOOLS].sort()).toEqual(EXPECTED_ALLOWED_TOOLS);
|
|
});
|
|
|
|
it("names only tools that are actually registered", () => {
|
|
const registry = createRegistry();
|
|
for (const id of ALLOWED_TOOLS) {
|
|
// A name the registry no longer answers to still reads as allowed, and
|
|
// the description still advertises it, but every step naming it dies on
|
|
// ToolNotFoundError and halts the sequence. It also leaves the check
|
|
// below no definition to inspect.
|
|
expect(
|
|
registry.getTool(id),
|
|
`${id} is allowed in run-sequence but not registered`
|
|
).toBeDefined();
|
|
}
|
|
});
|
|
|
|
it("admits no tool that renders as a frame of its own", () => {
|
|
const registry = createRegistry();
|
|
for (const id of ALLOWED_TOOLS) {
|
|
// `outputHint: "image"` declares a result that IS a picture — screenshot
|
|
// alone today. Such a tool also hands back the artifact behind it, so
|
|
// admitting one puts a mid-sequence frame in front of the agent.
|
|
expect(registry.getTool(id)?.outputHint, `${id}.outputHint`).not.toBe("image");
|
|
}
|
|
});
|
|
});
|