feat(flow): support concurrent flow recordings

[hubgan]

Problem

Only one flow could be recorded at a time, anywhere on the machine. Recording state was three module globals in flow-utils.ts — harmless in a per-client process, but the tool-server is a host-wide singleton per install bundle. Every MCP client, every subagent sharing a parent's connection, every argent CLI call and every project on the machine attaches to the same process, the same Registry, and those same three globals. HTTP requests are served concurrently with no serialization.

Three failures followed, all reproduced against the pre-change code:

1. A second recording clobbered the first. startRecordingSession just overwrote, so the first agent's next flow-add-step silently appended into the other agent's flow file — in a different project.
2. Any agent could finish any agent's recording. flow-finish-recording's schema was literally z.object({}).
3. Replay stomped recording. resolveFlowFilePath called setActiveProjectRoot, so a flow-execute in project B rebound the global root mid-recording of project A. Already worked around, locally only, in flow-add-step.

Approach

Recording state is now a Map keyed by the resolved flow file path, <project_root>/.argent/flows/<name>.yaml — the identity of the artifact being built, so "same key" means "same output file" and two sessions on one key are a genuine collision rather than an accident of scoping. flow-add-step, flow-add-echo and flow-finish-recording each take name + project_root, so every call is self-contained; required rather than optional-with-fallback, since a fallback would succeed while an agent is alone and fail only once a second recording exists.

resolveFlowFilePath is pure, which fixes defect 3 and retires FLOW_PROJECT_ROOT_REQUIRED (the state it guarded can no longer exist).

Rejected alternatives — keying by device udid (flows are deliberately device-portable, and flow-add-echo/flow-finish-recording have no device at all), an opaque recordingId (an agent that loses the token cannot recover), and per-caller identity from the transport (ToolContext carries no caller identity, and subagents share their parent's MCP connection, so it would fail the exact scenario being fixed).

Locking, and why readers needed more than a lock

Every mutation of one flow file runs under a lock keyed by that path — appends, flow-start-recording's truncate-and-register, and flow-finish-recording's read-summarize-clear. Keyed by file rather than owned by the session, because a restart replaces the session: a session-owned lock cannot exclude the operation that supersedes it. Per file, not global, so two recordings never queue behind each other. An append whose session was finished, restarted or evicted meanwhile fails with FLOW_NO_ACTIVE_RECORDING rather than writing into a different take.

The lock serializes writers only, and no reader of a flow YAML can join it — flow-execute's own load, its run: fragment load, flow-read-prerequisite, flow-add-step's sibling-fragment check, and the argent CLI reading from another process entirely, where an in-process lock cannot reach. Both writers used a plain fs.writeFile, which opens O_TRUNC, so a reader could land between the truncate and the write. That case is silent: parseFlow("") returns { steps: [] } with no error and summarize derives ok from "no failures", so a flow-execute racing an append reports a top-level PASS having replayed zero steps. Both writes now go through a sibling temp file and a rename, so a reader sees either the whole old file or the whole new one — and unlike the lock, that also holds cross-process.

stop-all-simulator-servers gets a devices scope

Every agent is told to call this at session end, and unscoped it walked the whole registry — so agent A wrapping up tore down agent B's devtools mid-recording, degrading B's flow to brittle coordinate taps. It now takes an optional devices scope and reports unmatched ids, so a mistyped id doesn't read as a clean machine.

Which namespaces count as "owned by a device" turned out to be the load-bearing part, because nothing cascades to most of them and unmatched turns any omission into an actively wrong diagnosis — a correct device id reported as a typo. The set now covers AXService (the in-sim ax daemon, spawned --timeout 3600; an iOS session that only ran boot/launch/describe owns this and nothing else), ScreenRecordingSession (an ffmpeg child plus the touch-visualizer overlay it enabled on the device), NativeProfilerSession (an xctrace child, or an on-device perfetto process and its trace file) and JsRuntimeDebugger (a bound loopback server, the CDP socket to Metro, a log handle). The debugger URNs interpose the Metro port — <ns>:<port>:<deviceId> — so the matcher understands both shapes, consuming only the first colon so a wireless adb serial after the port still compares whole.

stop-simulator-server shared none of this and had drifted: it looked URNs up with an exact, case-sensitive services.get(), so a lower-cased UDID silently no-op'd there while the scoped stop-all reaped it. Both tools now share one matcher. Their namespace sets stay deliberately different, documented where they are defined — stop-simulator-server is also the documented recovery for a wedged transport, and widening it to devtools/AX would make a routine retry drop the native-devtools connection another agent's recording depends on, which is the hazard the devices scope exists to prevent.

Verification

End-to-end, three devices, one shared tool-server (iPhone 16 Pro, iPhone 17 Pro, Pixel 3a): three interleaved recordings, plus an unrelated flow-execute in a fourth project while all three were live. Both iOS devices held their own live native-devtools concurrently (two per-UDID sockets, connected: true). Each YAML contained exactly its own steps in order, taps captured as selectors rather than coordinates, and neither picked up another's or the third root's path. All three replayed green — also through the released 0.17.0 CLI against the modified server, which checks wire compatibility. A device-scoped teardown of sim 1 left sim 2 and the emulator running.

Re-run against the final build:

• Two interleaved recordings across two project roots on one device, with a separate process polling one of the flow files as fast as it could for the duration. Zero empty or truncated observations; each YAML held exactly its own steps; both replayed green (6/6 and 5/5). Against the pre-change in-place write the same reader observes zero-step reads interleaved among the real ones — the silent-PASS input.
• A scoped teardown of the iPhone reaped AXService and NativeDevtools for that UDID only, reported no unmatched, and left the second sim and the emulator running. Lower-casing the UDID matched; a genuinely bogus id was still reported.
• With a screen recording live on the Pixel 3a, a scoped teardown reaped ScreenRecordingSession and the ffmpeg child actually exited (1 → 0). Before the fix it survived to the 180 s cap while the tool called the correct serial a typo.

The pre-change repros were re-run afterwards to confirm observed behavior changed, not just the tests.

npx vitest run in packages/tool-server (295 files, 3154 tests) and packages/argent-cli (280), tsc --build, tsc --noEmit on both test projects, eslint and prettier — all green.

Review notes

Several rounds of review found real defects in this change; each was reproduced before fixing and re-verified after. Worth knowing when reading the diff, since the tests exist to pin them:

• flow-start-recording truncated the .yaml outside any lock, so a step from the take being discarded landed in the freshly reset file and was reported as success (26-29 of 200 runs).
• flow-finish-recording never took the lock; its await fs.readFile was a yield during which a concurrent append committed, leaving the reported summary disagreeing with disk (11-16 of 200).
• A failed finish destroyed the recording. Hand-editing the .yaml mid-recording is a documented workflow, so parseFlow can legitimately throw — and the error told the agent to call flow-start-recording, which truncates the very take it would recover.
• unmatched reported a device this session had just stopped, because disposeService returns a node to IDLE and keeps it.
• A bare IP claimed every wireless-adb device at that address, since an adb serial is itself ip:port.
• The eviction backstop stamped with Date.now(); ties at millisecond resolution let it drop the session that was just used.
• requireRecordingSession ended with "Call flow-start-recording first" — reached for a key that was finished, superseded or evicted as well as one never started, and on those branches the file on disk is fully populated while that call truncates it and reports no restarted.
• The atomic write regressed twice on its own terms: its scratch name was derived from the flow file's basename, which has no length cap, so a long flow name that appended fine before failed ENAMETOOLONG; and the cleanup guard began after the temp file was created, leaking a scratch file into a committed directory on any write that failed after opening.
• Two tests passed against implementations they claimed to reject: the stop-simulator-server narrowness guard used a udid that classifies as android, where the iOS-only services it guards can never appear, so widening the iOS branch kept it green; and the client-mode finish asserted only the shape of savedTo, never its content, though that directive is the only thing that lands the file in client mode.

Notes for reviewers

• Client/remote persist mode is the least-exercised path, and now has its own concurrency tests (overlapping appends, and a restart superseding an in-flight one) — it previously had none. The project_root key is byte-stable across the file boundary (kind: "probe" passes it through unchanged), which is what keeps a remote recording addressable from flow-add-step, which declares no file input.

• Two spellings of one flow file still mint two sessions — and two independent locks — over one file, which bypasses every guarantee here. Two ways in: the root spelled two ways (a symlink, or a case variant on APFS), and the likelier one, the name cased two ways on a case-insensitive volume. Neither is normalized away because the correct normalization is the filesystem's: case-folding the key would wrongly merge two distinct flows on ext4, and resolving symlinks is impossible in client mode, where the root does not exist on this host. Documented at getFlowPath.

• flow-finish-recording cannot detect that another agent took its key; it finishes whatever take is live and reports it as yours. flow-add-step fails loudly in the same situation. There is no caller identity to key ownership on (see the rejected alternatives above), so the asymmetry is documented in the tool description rather than mechanized.

• Skew hazard in one direction only: a new client against an older tool-server. Zod strips the undeclared name/project_root, and the old server falls back to its module globals — so concurrent recordings silently collapse onto one session. The other direction (released 0.17.0 CLI against this server) was verified end-to-end.

• packages/argent-private/docs/reference.md still describes flow-add-step as appending to "the current recording", and names two tool ids that do not exist (flow-insert-echo/flow-run are file names; the ids are flow-add-echo/flow-execute). It is a submodule, so the fix cannot ride on this branch.

• A restart reported discardedSteps from the superseded session's in-memory flow while it truncates the file. Hand-editing the .yaml mid-recording is documented, so the two diverge: four steps wiped, one reported. It now counts the file, and reports no number at all when that file cannot be read or parsed - 0 is the answer a genuinely empty take gives. Client mode still counts from memory (this host cannot see the client's file), and the description now says which mode it is promising.

• ChromiumJsRuntimeDebugger cascades from ChromiumCdp and was therefore already being torn down - silently, while NetworkInspector and ReactProfilerSession cascade the same way and were named. stopped is documented as the services that were live and got shut down, so it is listed now too.

• The client-mode case named for a superseded in-flight append restarted between calls, so the next append re-resolved the key and succeeded - the guard was never reached, and neutering it left the whole file green. It now parks an append in its live sub-tool call across the restart.

• stop-tools.test.ts' mock handed the tool the live service map where the real getSnapshot copies, so a cascade rewrote the state the sweep was still reading and the answer depended on map insertion order. The mock copies and recurses like _teardown now, and the cascade case runs under both orders.

• Prose the code contradicted, each verified against source before rewriting: Vega owns JsRuntimeDebugger/NetworkInspector once the debugger has run (and can match anything at all through an ERROR node); "nothing here cascades" was denied twice in its own file; an iOS boot/launch/describe session owns NativeDevtools too; :tcp is mintable but unreached in production; the tool-server is a singleton per install bundle, not per machine; argent flow list does enumerate .argent/flows; the stop-tools comments narrated a "before" that only existed on this branch.

Found while verifying, out of scope, worth their own issues: a tool: flow-execute self-reference recurses past both cycle guards (run: is guarded, a nested flow-execute reseeds the stack); react-profiler artifacts collide host-wide on a second-resolution filename; stop-metro is port-scoped and kills another agent's bundler; debugger-reload-metro's HTTP fallback broadcasts to every attached client. A tool's capability is enforced only by the HTTP layer, not by registry.invokeTool, so a call that reaches the registry another way - flow-add-step takes command as a bare string - can resolve a service the tool declares it does not support, leaving an ERROR node behind (e.g. SimulatorServer:<vega serial>).

[latekvo]

[Diplomat]: Ran a full end-to-end sweep across the concurrency, ripple/call-site, client-mode, device-scoping, and docs lenses — the recording logic held up throughout. Two low-impact notes on the eviction test scaffolding are left inline.

[j-piasecki]

Three findings from a full review pass (each independently verified against the working tree; already-reported items and PR-acknowledged limitations excluded).

[latekvo]

[Diplomat]: Ran a full end-to-end sweep of this branch across the concurrency, atomic-write, tool-semantics, call-site/ripple, device-scoping, state-machine, resource-lifecycle, input-domain, test-quality and docs lenses, driving a tool-server built from af79e86 over HTTP: concurrent recordings across projects and against one device, a hammering reader during appends, restart/supersede, client persist mode, the MAX_RECORDINGS backstop, scoped teardown, replay, and the released 0.17.0 CLI against this server. The recording machinery itself held up everywhere I pushed on it — the isolation, the per-file lock and the temp-file+rename swap all behaved as described, with no torn reads and no cross-contamination.

Five notes inline. The one on stop-all-simulator-servers' schema is the one I would look at first — it is the only one with a behavioural consequence rather than a wording one.

[j-piasecki]

LGTM! I'll leave it for @latekvo

[latekvo]

Seven findings from a pass over the current head, each verified against a build of af79e86 and deduplicated against every existing thread. Six are prose that no longer describes the codebase; one is an unpinned behaviour I confirmed by mutation.

[latekvo]

[Diplomat]: Ran a full end-to-end sweep of this branch at a0ee67e6, then repeated independent passes over it until two consecutive sweeps came back clean.

A tool-server built from the branch drove two interleaved concurrent recordings across two project roots - each YAML kept exactly its own steps in order and both replayed green - while a separate reader observed 7,193,301 reads across 40 concurrent appends with zero empty or partial files. Also exercised: client/remote persist mode (nothing written to the host), the scoped and unscoped teardown paths including a case-varied UDID, a re-stop of an already-idle device, the devices: [] and misspelled-key cases, and the Chromium debugger cascade. Suites, typechecks, eslint and prettier are clean; the 6 failures in test/boot-device-hotboot.test.ts are pre-existing and identical at the merge base.

I also mutated the core invariants directly - the atomic write, the append lock, .strict(), touch()/LRU, the lock-map self-cleanup, the devices: [] semantics, the ERROR-node omission from stopped, and ownership-counted-regardless-of-state. Every one was caught by the suite. The recording, locking and atomic-write machinery held up throughout, and nothing inline below is a logic defect.

What did survive verification is prose that the code contradicts, plus one reported value. Findings are inline.

[latekvo]

Left inlines, please fix.

Other than these linguistic notes this is a great PR and i think its ready to merge ❤️

[latekvo]

Suggested change

description: `Execute a tool call and record it as a step in the flow named by \`name\` + \`project_root\` (the recording must already be open — see flow-start-recording). Use when recording a flow and you want to run and capture each action. A coordinate \`gesture-tap\` is recorded as a portable \`tap: { selector }\` step when the tapped element has stable text/identifier (otherwise coordinates are kept with a warning); a \`restart-app\` is recorded as a \`launch\` step (record one FIRST to make the flow a self-contained e2e flow). Returns { message, toolResult, flowFile, savedTo } on success - \`savedTo\` is where the YAML landed: a host path, or, against a remote client, the directive that has the client write it (the only field naming the destination in that mode). If it fails an error is returned and nothing is recorded.

description: `Execute a tool call and record it as a step in the flow named by \`name\` + \`project_root\` (the recording must already be open — see flow-start-recording). Use when recording a flow and you want to run and capture each action. A coordinate \`gesture-tap\` is recorded as a portable \`tap: { selector }\` step when the tapped element has stable text/identifier (otherwise coordinates are kept with a warning); a \`restart-app\` is recorded as a \`launch\` step (record one FIRST to make the flow a self-contained e2e flow). Returns { message, toolResult, flowFile, savedTo } on success. If it fails an error is returned and nothing is recorded.

Redundant. The model knows what a path is, there is no need to explain what a path is or where it could be leading to.

[latekvo]

This description grew 4x but 70% of it just says "be careful to not stop other agent recordings"

It's nonsensical, this description contains more rambling about a single edge-case than what the tool does, it's an insane level of overfixation on an irrelevant thing.

Please fix.

[latekvo]

Once again, savedTo is self evident. If you believe it does require any more explanation than message or flowFile, both of which are not as self-evident, then please at least don't make it occupy so much space.

[latekvo]

Another case of edge-case overfixation inflating a description to three times the size.

[latekvo]

[Diplomat]: Ran a full end-to-end sweep of this branch at 6c682c3, then a second independent pass over it.

A tool-server built from the branch drove concurrent recordings across two project roots and a burst of 8 concurrent appends on one key (all ordered, no cross-contamination, both replays green); a separate process observed 1,593,459 reads of a flow file during 120 concurrent appends with zero empty or partial — against a real in-place O_TRUNC writer the same detector sees 695,532 empty reads out of 909,926, so it is not blind. Also exercised: client/remote persist mode (nothing written to the host), the run: composition capture and its cross-project warning, the MAX_RECORDINGS backstop, hostile name / project_root inputs, the scoped and unscoped teardown paths including a case-varied UDID and a bogus id, and the real argent run path for start/add-step/add-echo/finish plus --help. Every documented behaviour I could execute matched, including the No active recording … message verbatim, (plus N in other projects), discardedSteps counted from disk (4) and omitted on an unparseable file, and the honest caveat that starting over a committed flow wipes it with no restarted.

Suites, typecheck and prettier are clean: tool-server 3164 passed, argent-cli 278 passed. The 6 boot-device-hotboot.test.ts failures are pre-existing — identical at the merge base 7189333 on this Linux host.

The recording, locking and atomic-write machinery held up everywhere I pushed on it, and nothing below is a runtime defect. What survived verification is prose the code contradicts, plus four invariants the diff documents as load-bearing that mutation shows nothing pins. Each finding was reproduced, then adversarially re-checked; several candidates were dropped as unreproducible, out of scope, or already covered by an open thread.

[latekvo]

[Diplomat]: The description's closing sentence is Never throws., while the schema declared 34 lines above is .strict() — whose own comment says the udids slip now "fails loudly" and is "a validation error instead". Two statements in one file give opposite answers to whether this call can fail, and only the false one is delivered to an agent.

Driven against a tool-server built from 6c682c3, every dispatch surface errors on that exact typo:

POST /tools/stop-all-simulator-servers {"devices":["x"]} -> 200 {"data":{"stopped":[],"unmatched":["x"]}}
POST /tools/stop-all-simulator-servers {"udids":["x"]}   -> 400 {"error":"[{\"code\":\"unrecognized_keys\",\"keys\":[\"udids\"],...}]"}
registry.invokeTool("stop-all-simulator-servers", {udids:["ABC"]})
                                                        -> ToolExecutionError: Invalid params for tool "stop-all-simulator-servers"
argent run stop-all-simulator-servers --udids x         -> unrecognized_keys
MCP CallTool                                            -> { isError: true }

execute() itself genuinely never throws — _teardown swallows dispose() errors, and disposeService cannot raise ServiceNotFoundError for a URN read out of the snapshot it just took. The sentence is false only about the schema gate every caller must pass, which is the only failure surface an agent can observe.

Provenance puts both halves inside this PR rather than inherited: git log -S places Never throws. in 0dd0609 and .strict() in 2f5d342 — the later commit, titled "make the scope key strict, and correct what the docs promise", which audited and corrected seven other drifted claims and left this one standing. At the merge base the sentence read "Fails silently if no servers are running."

Across the 20 files carrying a description: template literal under packages/tool-server/src/tools/, Never throws appears only here; every other repo hit is JSDoc on an internal helper that sits behind no runtime schema.

[latekvo]

[Diplomat]: NetworkInspector and ReactProfilerSession reach DEVICE_OWNED_NAMESPACES through the ...PORT_KEYED_NAMESPACES spread, and the block above documents their membership as load-bearing for what stopped names ("listed for what stopped reports: a session that had a network inspector or a React profiler open is told those went away by name"). Neither their membership nor their port-keyed URN grammar is pinned.

Mutation-tested in a private worktree at 6c682c3, test/stop-tools.test.ts:

baseline                                       Tests  49 passed (49)
drop both from PORT_KEYED_NAMESPACES           Tests  49 passed (49)      <- green
control: drop JS_RUNTIME_DEBUGGER_NAMESPACE    Tests   2 failed | 47 passed (49)
   x scopes the port-keyed debugger URNs to the right device
   x does not let a port-keyed URN's port be mistaken for a wireless-adb device id

The control is what makes this concrete rather than a coverage complaint: the suite can catch this class, it simply has no case for these two. grep -rn "NetworkInspector\|ReactProfilerSession" packages/tool-server/test/stop-tools.test.ts returns nothing, and DEVICE_OWNED_NAMESPACES is consumed only by the two stop tools, so nothing elsewhere pins it either. The same mutation against the full suite is byte-identical to baseline.

The reachable consequence is the reporting the block claims, not a leak or a false unmatched: Registry._resolve always inserts the JsRuntimeDebugger dependency node with the identical device-id payload, so an inspector or profiler node cannot appear without it and both still die via the cascade — they just stop being named in stopped, which is the one thing their membership is documented to buy.

[latekvo]

[Diplomat]: This sentence points at a paragraph that no longer exists. Printing the advertised description from a build of 6c682c3:

node -e "const d=require('.../dist/tools/flows/flow-start-recording.js').flowStartRecordingTool.description;
         for (const w of ['client','remote','host','memory','boundary']) console.log(w, new RegExp(w,'i').test(d))"
client false / remote false / host false / memory false / boundary false

The description says nothing about the host/client split. git log -S places this JSDoc sentence in 7f918db, and shows 9c6c116 ("cut the recording tool descriptions back to what the tool does") — a later commit on this branch — deleting exactly the paragraph it refers to: "That holds when the project root is on the tool-server host; against a REMOTE client the host never sees the file, so there the count is of the steps recorded through this server and a hand-edit on your machine is not in it."

The caveat itself survives, but in a different artifact — packages/skills/skills/argent-create-flow/SKILL.md:134 carries it verbatim. So the defect is that the JSDoc names the wrong surface: a maintainer checking whether the client-mode meaning of discardedSteps is stated on the tool itself finds that it is not, and the cross-reference cannot be resolved by anyone who did not watch the two commits land.

[latekvo]

[Diplomat]: The rationale names the layer that does not do the validating. All three callers POST the args verbatim and the tool-server validates with the zod schema — def.zodSchema.safeParse (http.ts:705) and definition.zodSchema.safeParse (registry.ts:126). Two of the three named callers are provably schema-blind:

• argent run is handed the advertised schema including additionalProperties: false and accepts the unknown flag anyway. parseFlags(["--udids","SIM-1"], <that schema>) returns {"udids":"SIM-1"} with no error, and flag-parser.ts:254-256 says so outright: "We still accept unknown flags so tools can evolve their schemas without breaking the CLI; tool-server will return a 400 if invalid".
• argent-mcp forwards params.arguments verbatim; CallToolRequestSchema.safeParse on {udids:['X'],totallyBogus:1} succeeds with the arguments preserved.

grep -rn 'ajv|json-schema-validat|@cfworker/json-schema' packages/*/package.json returns nothing — no JSON-schema validator exists anywhere in the request path.

The assertion the comment justifies also cannot fail independently of the safeParse line above it: the advertised additionalProperties: false is derived from the zod .strict() by z.toJSONSchema (packages/registry/src/zod-to-json-schema.ts), so the two cannot diverge.

[latekvo]

[Diplomat]: The required array is inert on the path these tests exercise. In a private worktree at 6c682c3:

baseline (this file + run-help.test.ts)          Tests  6 passed (6)
required: []  ->  run-flow-add-step-payload      Tests  3 passed (3)

The regression this file guards also stays fully reachable and fully caught with required: [] still in place: re-introducing #452 by changing flag-parser.ts:157 from if (flag === "args" && properties.args === undefined) to if (flag === "args") gives Tests 2 failed | 1 passed (3). So "reachable at all" does not hold in the strong sense either.

grep -rn required packages/argent-cli/src/*.ts shows schema.required is read in exactly one place — formatSchemaUsage (flag-parser.ts:273,288), the help renderer, which this file never invokes — and parseFlags' own JSDoc states the caller is responsible for "validating required fields server-side". The fixture's requiredness is therefore free to drift from the real flow-add-step schema with no signal here, which is the opposite of what this note tells a maintainer.

[latekvo]

[Diplomat]: This file hard-fails on exactly the drift the comment says passes silently. In a private worktree at 6c682c3:

baseline                  Tests  3 passed (3)
required: ["command"]     Tests  1 failed | 2 passed (3)
  x test/run-help.test.ts:127
    expect(help).toMatch(/--name <value>\s+string \(required\)/)

The negative assertions (/--args <value>\s+string(?! \(required\))/) pin the array against added entries too, so it is guarded in both directions.

The timeline makes it wrong-when-written rather than stale: git log -S 'renders each required flag with the (required) marker' puts that test in f12388b, while git log -L 28,34 puts this wording in 2f5d342 — the later commit. The drift direction that genuinely goes unnoticed is the opposite one, where the real flow-add-step schema relaxes and this fixture does not; nothing here covers that.

[latekvo]

[Diplomat]: Both interaction formatters this diff rewrites are unpinned, including the case the comment three lines below names as the one that must not happen.

Flattened both to their unconditional forms (startedMsg at line 37 always returning "Stopping all simulator servers"; this one dropping the unmatched clause) in a private worktree at 6c682c3:

baseline  test/interaction-messages.test.ts test/stop-tools.test.ts   Tests  52 passed (52)
mutated                                                               Tests  52 passed (52)
baseline  full tool-server suite   Tests  6 failed | 3164 passed | 1 skipped (3171)
mutated                            Tests  6 failed | 3164 passed | 1 skipped (3171)

(the 6 are the pre-existing boot-device-hotboot.test.ts GPU-mode failures, identical at the merge base)

npm run build passes under the mutation as well, so typecheck is not a backstop — the now-unused params destructure is simply dropped. grep -rn for "Stopping simulator servers for", "matched no service" and "Stopping all simulator servers" each return exactly one hit, the source definition itself. test/interaction-messages.test.ts does carry this tool in its catalog, but asserts only expect(definition.interaction?.startedMsg).toBeTypeOf("function"), which a constant-returning arrow satisfies — so that file gives the appearance of coverage without pinning either string.

The consequence is the one stated at line 46: a regression dropping the unmatched clause renders a scoped teardown in which every id was mistyped as plain "Stopped 0 simulator servers" — byte-identical to a clean machine.

[latekvo]

[Diplomat]: The restart branch is unpinned, even though the diff pins the equivalent claim on the result payload (restarted / discardedSteps). Two mutations in a private worktree at 6c682c3:

baseline   test/interaction-messages.test.ts test/flows/   Tests  608 passed (608)
(1) whole body -> `Started recording flow ${params.name}`  Tests  608 passed (608)
(2) keep the branch, drop the `discarded === undefined`
    sub-branch (always format a number)                    Tests  608 passed (608)
full tool-server suite, both mutations
    Tests  6 failed | 3164 passed | 1 skipped (3171)       (identical to baseline)

tsc --noEmit -p tsconfig.test.json exits 0 under mutation (1), so the mutated tree is a legitimately green build rather than a broken one.

Mutation (1) makes a destructive re-record announce itself in the client log exactly like a fresh start — the outcome this comment calls "the one outcome here worth reading twice". Mutation (2) is the case the sub-branch was added for: when the superseded file cannot be read or parsed, the line reports a step count that was never obtained.

[latekvo]

[Diplomat]: Neither state this helper exists for can occur. captureRunTarget runs only after await invokeSubTool(registry, ctx, params.command, args) has already resolved for command === "flow-execute", and two upstream guards reject every triggering input before that returns:

• an absent or non-string args.project_root is rejected by Registry.invokeTool's definition.zodSchema.safeParse (registry.ts:125-129) before flow-execute's execute is entered, since it declares project_root: z.string();
• a string root that getFlowsDir would reject is rejected by flow-execute calling that same function first — execute opens with resolveFlowFilePath(params, …) -> getFlowPath(params.project_root, …) -> getFlowsDir -> assertValidProjectRoot, unconditional and ahead of every early return.

Driven through the real Registry with the real createRunFlowTool + createFlowAddStepTool, a live host recording open and a sibling fragment present:

project_root absent     -> Invalid params for tool "flow-execute": expected string, received undefined
project_root number     -> Invalid params for tool "flow-execute": expected string, received number
project_root relative   -> project_root must be an absolute path (got "rel/dir")
project_root with ".."  -> project_root must not contain ".." segments
flow_file only          -> expected string, received undefined  (path ["project_root"])
control: valid root     -> Step added to "wrapper" flow    <- reaches captureRunTarget, records `run: helper`

Instrumenting the catch and adding a typeof ranIn !== "string" arm that appends a marker: test/flows/ runs 611 passed and the marker file is never created. flow_file cannot reach it either — resolveFlowFilePath computes getFlowPath(params.project_root, …) ahead of both the !params.flow_file and viaUpload escapes, and invokeSubTool forwards only { signal, toolInvocationId, recordChildInvocation }, so a nested flow-execute never receives ctx.fileInputs.

The JSDoc states a hazard — "a malformed root must not throw here — that would divert a resolvable target into the keep-the-raw-step branch" — that the code cannot experience, so a later change can be justified or blocked on it.