Files
software-mansion__argent/packages/tool-server/test/interaction-messages.test.ts
Hubert Gancarczyk 0f90d182b2 feat(flows): record a script step by running it (#940)
#864 (the executor) and #865 (the `script:` step) are merged. This
branch now
sits directly on `main` as a single commit, so the diff and the commit
list are
this change alone.

A `script:` step could only enter a flow by being typed into the YAML
after the
recording ended. The authoring skill said so in as many words, and
listed it
among the legal unrecorded insertions. That made it the one directive
exempt
from every rule the recorder exists to enforce, and it cost two things:

- **The script never ran while the flow was authored.** It first
executed during
the closing replay, where a failure reads as a flow that was never right
rather than a script that needs one more attempt. Worse, the device
steps
recorded after it were walked against backend state the author had
established
by hand — so the recording proved nothing about whether the script
establishes
  that same state.
- **The author never saw what the script returned.** A later release
reads
`{{output:user.id}}`; the only way to know that path exists is to have
seen the
  document.

This adds one MCP tool, `flow-add-script`, that runs a local `.mjs` and
records
the step it ran. **No new flow syntax** — the `script:` step is
unchanged, and
this is a second way for one to enter a flow file.

```text
flow-add-script { name: "checkout", project_root: "/Users/dev/shop",
                  path: "../../scripts/seed-order.mjs", timeout: 60000 }
```

## What it adds

- **One shared execution path.** Path resolution, the on-disk casing
guard, the
executor call and the verdict were private to `flow-run.ts`. They are
now
`runFlowScriptStep` in `flow-script-step.ts`, with the file-ref helpers
in
`flow-file-refs.ts`, and `runScriptStep` is that call plus its
`ExecState`
plumbing. This is the load-bearing change: "the recording runs the
script
exactly as replay will" is one function, not two code paths that agree
today.
- **A failure records nothing.** The opposite of `flow-add-step`,
deliberately.
A failed script did not establish the state the rest of the walkthrough
would
then be recorded against, so an appended red step costs every step after
it,
not just itself. The `message` also says whether a child actually *ran*
— a
path that resolved to no file forked nothing, while a script that threw
halfway did not roll back — because the author's next move is a retry or
a
  cleanup and they cannot pick without knowing.
- **The returned document, as JSON text.** Text rather than an object
because
this is the first tool result carrying bytes the server did not author:
every
  result is deep-walked for `__argentClientFile` and `__argentArtifact`
directives, which match on shape alone and write or fetch files on the
agent's
  machine. As a string it inherits none of that. Bounded at 64 KiB with
  `outputTruncated`, for the reason `log` is bounded.
- **`{{output:` refused by `parseFlow`.** Surfacing the document invites
an
author to write a reference that has no meaning yet — it would be
printed
literally by a step that passes, green and wrong. The scan runs over
exactly
the field list the output release declares supported, so it is a seam
that
release converts from reject to resolve rather than scaffolding it
deletes.
Patterns are excluded: a `{{` in a regular expression is a quantifier
the
  author meant.
- **Two boundaries.** A `client`-mode recording is refused — its `.mjs`
never
left the agent's machine, the same anchor reason `run:`, `snapshot` and
  uploaded flows are refused. And the tool is `longRunning`, so the MCP
adapter's 30s fetch abort cannot retry a call whose whole purpose is a
side
effect; the comment records what that flag does *not* close, and where
the
  rest belongs.
- **`flow-add-step` says when the call already ran.** The `{{output:`
refusal is
the one refusal this release adds on a path where the tool call has
already
fired, and its message asks for an edit-and-retry — which repeats the
device
  action. Verified: a `keyboard` step recorded twice left
`{{output:code}}{{output:code}}` in the field. That one refusal now
carries
  what already happened.

## Documentation

Rule 5 of `argent-create-flow` no longer lists `script:` as a legal
unrecorded
insertion — leaving it there would tell agents to keep doing the thing
this PR
removes. `live-authoring.md` gains the recorder contract, the return
fields that
are not self-evident, and the ordering rule: a setup script is recorded
*before*
the `restart-app` it prepares state for, because that is where it runs
at
replay. A leading script does not make the flow a fragment. `qa-flows`
and
`reliability-and-recovery` follow the count and the wording.

## Verification

`npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check`
over
tracked files, and `npm run typecheck:tests -w @argent/tool-server` are
green.
`npm test -w @argent/tool-server`: 358 files, 4657 passed, 1 skipped.

Exercised end to end against a live tool server built from this branch,
not
only in tests — recorded a flow, then replayed the saved YAML through
`argent flow run` with no hand editing:

```
flow-add-script → status "pass", durationMs 124
  log: "seeding order for the flow\nwarn: using the fake backend\n"
  outputJson: {"order":{"id":"ord_4417","total":39.5},
               "user":{"id":"usr_9","email":"buyer@example.com"}}
  recorded: 1. script: ../../scripts/seed-order.mjs (timeout 30000ms)
```

```yaml
steps:
  - script:
      path: ../../scripts/seed-order.mjs
      timeout: 30000
  - echo: order seeded; expect Home
```

```
Flow "checkout"
  ✓  1 script ../../scripts/seed-order.mjs
       │ seeding order for the flow
       │ warn: using the fake backend
  › order seeded; expect Home

PASS — 1 passed, 0 failed, 0 errored, 0 skipped
```

The three refusals, live on the same recording:

- A script that threw after creating records: `status "fail"`,
`stepCount`
unchanged at 1, and the message naming the side effects that were not
rolled
  back.
- A path resolving to no file: refused before any fork, no `durationMs`,
and
  "Nothing ran, so there is nothing to clean up".
- `flow-add-echo` holding `{{output:user.id}}`: refused with the
author-facing
  message naming the step, the field and the later release.

No device is involved at any point — a recording may open with a seeding
script
before it boots anything.



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added the `flow-add-script` tool to run local `.mjs` scripts and
record successful executions in flows.
* Added script status, output, duration, logs, and failure details to
recording results.
* Added clearer handling for script cancellation, timeouts, missing
files, and path casing.

* **Documentation**
* Updated flow guidance to clarify end-to-end classification and script
setup or cleanup usage.
* Documented script path requirements, failure conditions, and recording
workflows.

* **Bug Fixes**
* Flows now reject unsupported output-reference templates with more
precise diagnostics.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-07 12:59:29 +02:00

325 lines
14 KiB
TypeScript

import { describe, expect, it } from "vitest";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { FAILURE_CODES, Registry, type FailureSignal } from "@argent/registry";
import { createRegistry } from "../src/utils/setup-registry";
import { definitionsById, EXPECTED_TOOL_COUNT } from "./helpers/catalog";
import { flowStartRecordingTool } from "../src/tools/flows/flow-start-recording";
import { createFlowAddStepTool } from "../src/tools/flows/flow-add-step";
const failureSignal: FailureSignal = {
error_code: FAILURE_CODES.ARGENT_UNCLASSIFIED_FAILURE,
failure_stage: "interaction_message_test",
failure_area: "tool_server",
error_kind: "unknown",
};
describe("tool interaction messages", () => {
it("defines all three formatters for every tool", () => {
const definitions = definitionsById(createRegistry());
expect(definitions.size).toBe(EXPECTED_TOOL_COUNT);
for (const [id, definition] of definitions) {
expect(definition.interaction?.startedMsg, `${id}.startedMsg`).toBeTypeOf("function");
expect(definition.interaction?.completedMsg, `${id}.completedMsg`).toBeTypeOf("function");
expect(definition.interaction?.failedMsg, `${id}.failedMsg`).toBeTypeOf("function");
}
});
it("formats messages from parameters, results, and failure signals", () => {
const definitions = definitionsById(createRegistry());
expect(
definitions.get("gesture-tap")!.interaction!.completedMsg!({
params: { udid: "device-1", x: 0.5, y: 0.25, clickCount: 2 },
result: {},
})
).toBe("Double-tapped at (50%, 25%)");
// `keyboard` picks its wording from which of `text` / `key` was given, and
// the two formatters see DIFFERENT sets of shapes. `startedMsg` renders
// before `execute` runs, so it still sees a combined call — the one
// `execute` is about to reject — and has to word four: text alone, key
// alone, both, and neither. `completedMsg` runs only after a call that
// succeeded, so the combined shape never reaches it and it words three. In
// both, "neither" is a documented no-op returning { typed:"", keys:0 }
// rather than an error (see keyboard-android.test.ts).
const keyboard = definitions.get("keyboard")!.interaction!;
expect(keyboard.startedMsg!({ params: { udid: "device-1", text: "hi" } })).toBe(
"Entering text"
);
expect(keyboard.startedMsg!({ params: { udid: "device-1", key: "enter" } })).toBe(
"Pressing a key"
);
// The combined shape, which only THIS formatter reaches: `startedMsg` runs
// before the rejection. Without it, a `startedMsg` reduced to the two-way
// text/key split would still satisfy every other assertion here while
// announcing a rejected text+enter call as plain "Entering text".
expect(keyboard.startedMsg!({ params: { udid: "device-1", text: "hi", key: "enter" } })).toBe(
"Entering text and pressing a key"
);
// The empty request, on this formatter too. `completedMsg` appears in no
// other test file, `startedMsg` only in this file's secret-leak check (which
// every branch string satisfies), so neither empty-request branch is pinned
// anywhere else. Narrowing BOTH to `params.text === undefined &&
// params.key !== undefined` — so `keyboard {}` falls through to "Entering
// text" / "Entered text" — stays green across the whole suite once this
// assertion and its `completedMsg` twin below are removed. Both are
// load-bearing.
expect(keyboard.startedMsg!({ params: { udid: "device-1" } })).toBe("Pressing a key");
expect(keyboard.completedMsg!({ params: { udid: "device-1", text: "hi" }, result: {} })).toBe(
"Entered text"
);
expect(keyboard.completedMsg!({ params: { udid: "device-1", key: "enter" }, result: {} })).toBe(
"Pressed a key"
);
// The third shape. "Pressed a key" for a call that pressed nothing is
// inherited, not introduced here — pinned so the wording and the no-op
// contract can only diverge deliberately.
expect(keyboard.completedMsg!({ params: { udid: "device-1" }, result: {} })).toBe(
"Pressed a key"
);
expect(
definitions.get("screenshot")!.interaction!.completedMsg!({
params: { udid: "device-1" },
result: { image: { filename: "screenshot.png" } },
})
).toBe("Captured screenshot screenshot.png");
expect(
definitions.get("gesture-tap")!.interaction!.failedMsg!({
params: { udid: "device-1", x: 0.5, y: 0.25 },
error: new Error("raw error"),
failureSignal,
})
).toBe(`Failed to tap at (50%, 25%): ${failureSignal.error_code}`);
});
it("names the flow from either source in flow-execute messages", () => {
const definitions = definitionsById(createRegistry());
const interaction = definitions.get("flow-execute")!.interaction!;
const byName = { name: "settings-explore", project_root: "/repo" };
expect(interaction.startedMsg!({ params: byName })).toBe("Running flow settings-explore");
expect(interaction.completedMsg!({ params: byName, result: {} })).toBe(
"Ran flow settings-explore"
);
expect(interaction.failedMsg!({ params: byName, error: new Error("raw"), failureSignal })).toBe(
`Failed to run flow settings-explore: ${failureSignal.error_code}`
);
// A flow_path call has no name param — the messages derive the name from
// the YAML basename instead of rendering "undefined".
const byPath = { flow_path: "/repo/flows/login.yaml", project_root: "/repo" };
expect(interaction.startedMsg!({ params: byPath })).toBe("Running flow login");
expect(interaction.completedMsg!({ params: byPath, result: {} })).toBe("Ran flow login");
expect(interaction.failedMsg!({ params: byPath, error: new Error("raw"), failureSignal })).toBe(
`Failed to run flow login: ${failureSignal.error_code}`
);
// Degenerate sources (these calls fail validation, but the started message
// renders first) still say something honest — never "" or "undefined".
expect(
interaction.startedMsg!({ params: { flow_path: "/repo/flows/.yaml", project_root: "/repo" } })
).toBe("Running flow .yaml");
// Unlike the bare ".yaml" above, this path has an empty basename stem, so
// the raw path — not the stem — is what names the flow here.
expect(interaction.startedMsg!({ params: { flow_path: "/", project_root: "/repo" } })).toBe(
"Running flow /"
);
expect(interaction.startedMsg!({ params: { project_root: "/repo" } })).toBe(
"Running flow (unspecified)"
);
});
it("distinguishes a fresh recording start from a destructive restart", () => {
// A restart truncates and replaces a live take; if its message ever
// collapsed to the same wording as a fresh start (or reported a step
// count that was never actually obtained), an agent re-recording a flow
// would have no way to notice it just destroyed prior work.
const definitions = definitionsById(createRegistry());
const completedMsg = definitions.get("flow-start-recording")!.interaction!.completedMsg!;
const params = { name: "checkout", project_root: "/tmp/proj" };
expect(
completedMsg({
params,
result: { message: "", flowFile: "", savedTo: "project" },
})
).toBe("Started recording flow checkout");
expect(
completedMsg({
params,
result: { message: "", flowFile: "", savedTo: "project", restarted: true },
})
).toBe("Restarted recording flow checkout, discarding the previous take");
expect(
completedMsg({
params,
result: {
message: "",
flowFile: "",
savedTo: "project",
restarted: true,
discardedSteps: 1,
},
})
).toBe("Restarted recording flow checkout, discarding 1 step");
expect(
completedMsg({
params,
result: {
message: "",
flowFile: "",
savedTo: "project",
restarted: true,
discardedSteps: 4,
},
})
).toBe("Restarted recording flow checkout, discarding 4 steps");
});
it("names the flow in every recording-tool interaction line", () => {
// Recordings are concurrent, so several of these lines interleave in one log
// and an unqualified "flow recording" would not say which one died or
// finished. Hold every formatter on every recording tool to naming the flow
// — the property the concurrency support introduced — including the failure
// lines, which are the diagnostic when several recordings are live.
const definitions = definitionsById(createRegistry());
const name = "checkout";
const params = { name, project_root: "/tmp/proj", command: "gesture-tap", message: "note" };
// Each tool's OWN result shape. One shared `{ message, flowFile, savedTo }`
// used to stand in for all of them, which stopped describing any once
// the recorder dropped the per-step YAML: `flowFile` survives on start and
// finish only, and add-step/add-echo report `stepCount` (plus `recorded`
// on add-step) instead. A fixture that misdescribes the contract is the one
// that gets copied into a test that does.
const results: Record<string, Record<string, unknown>> = {
"flow-start-recording": { message: "", flowFile: "", savedTo: "project" },
"flow-add-step": {
message: "",
toolResult: {},
stepCount: 1,
recorded: "1. tap: (0.5, 0.3)",
savedTo: "project",
},
"flow-add-echo": { message: "", stepCount: 1, savedTo: "project" },
"flow-add-script": {
message: "",
status: "pass",
stepCount: 1,
recorded: "1. script: ../../scripts/seed.mjs",
savedTo: "project",
},
"flow-finish-recording": {
message: "",
path: "/tmp/proj/.argent/flows/checkout.yaml",
executionPrerequisite: "",
steps: 1,
summary: ["1. tap: (0.5, 0.3)"],
flowFile: "",
savedTo: "project",
},
};
for (const id of [
"flow-start-recording",
"flow-add-step",
"flow-add-echo",
"flow-add-script",
"flow-finish-recording",
]) {
const i = definitions.get(id)!.interaction!;
expect(i.startedMsg!({ params }), `${id}.startedMsg`).toContain(name);
expect(i.completedMsg!({ params, result: results[id] }), `${id}.completedMsg`).toContain(
name
);
expect(
i.failedMsg!({ params, error: new Error("raw error"), failureSignal }),
`${id}.failedMsg`
).toContain(name);
}
});
it("does not announce a recorded step on the paths that record nothing", () => {
const completedMsg =
definitionsById(createRegistry()).get("flow-add-step")!.interaction!.completedMsg!;
const params = { name: "checkout", project_root: "/tmp/proj", command: "echo" };
expect(
completedMsg({ params, result: { message: "", toolResult: undefined, stepCount: 0 } })
).toBe("Recorded no echo step in flow checkout");
expect(
completedMsg({
params,
result: { message: "", toolResult: undefined, stepCount: 1, recorded: "1. echo: hi" },
})
).toBe("Added echo step to flow checkout");
});
it("joins the record-nothing RESULT to the line the registry actually logs", async () => {
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "interaction-msg-"));
try {
await flowStartRecordingTool.execute(
{},
{ name: "checkout", project_root: tmpDir, executionPrerequisite: "on the form" }
);
const registry = new Registry();
registry.registerTool(createFlowAddStepTool(registry) as never);
const completions: string[] = [];
registry.events.on("toolCompleted", (_id, _callId, _ms, msg) => completions.push(msg));
const result = await registry.invokeTool<{ recorded?: string }>("flow-add-step", {
name: "checkout",
project_root: tmpDir,
command: "echo",
});
expect(result.recorded).toBeUndefined();
expect(completions).toEqual(["Recorded no echo step in flow checkout"]);
} finally {
await fs.rm(tmpDir, { recursive: true, force: true });
}
});
it("does not expose sensitive inputs", () => {
const definitions = definitionsById(createRegistry());
const secret = "INTERACTION_MESSAGE_SECRET";
const messages = [
definitions.get("keyboard")!.interaction!.startedMsg!({
params: { udid: "device-1", text: secret, key: secret },
}),
definitions.get("open-url")!.interaction!.startedMsg!({
params: {
udid: "device-1",
url: `https://user:${secret}@example.com/path?token=${secret}#fragment`,
},
}),
definitions.get("debugger-evaluate")!.interaction!.startedMsg!({
params: { device_id: "device-1", port: 8081, expression: secret },
}),
definitions.get("chromium-cookies")!.interaction!.completedMsg!({
params: { udid: "chromium-1", action: "set", name: "session", value: secret },
result: { set: true },
}),
// Recordings are keyed by `name` + `project_root`, so both are required
// and the message names the flow. The echoed `message` is the sensitive
// part — it is caller-authored free text — and stays out.
definitions.get("flow-add-echo")!.interaction!.completedMsg!({
params: { name: "checkout", project_root: "/tmp/proj", message: secret },
result: { message: secret, stepCount: 1, savedTo: "project" },
}),
];
expect(messages.join("\n")).not.toContain(secret);
expect(messages).toContain("Opening example.com");
expect(messages).toContain("Added note to flow checkout");
});
});