feat(workflows): add stage introspection and control actions

[flora131]

Summary

Extends the workflow tool with six new actions — stages, stage, transcript, send, pause, and reload — giving agents full visibility into running workflow stages and direct control over prompt answering, steering, pausing, and resource reloading.

Refs #1023

New workflow tool actions

Action	Description
stages	List stages for a run, filterable by status (pending, running, awaiting_input, paused, blocked, completed, failed, skipped, all)
stage	Inspect a single stage snapshot (id, name, status, error, pending prompt, session info)
transcript	Read a stage's conversation transcript with snapshot fallback; supports tail/limit and snapshot-only includeToolOutput; terminal result/error entries are preserved after tool events
send	Deliver a message to a live or paused stage; delivery mode selects auto, answer, prompt, steer, followUp, or resume; omitted payload is a no-op while an explicit empty string remains a valid answer
pause	Pause a run (supports all: true for bulk pause; rejects all:true combined with stageId)
reload	Reload workflow resources in-process when no workflow runs are in flight

Changes

Schema additions (workflow-schema.ts)

New optional parameters: statusFilter, format (text | json), limit, tail, includeToolOutput, text, response, delivery (auto | answer | prompt | steer | followUp | resume), promptId, reason

Runtime behavior

• Added pauseAllRuns export for bulk-pause across all in-flight runs
• In-process reload is serialized and guarded against in-flight workflow runs to avoid swapping runtime/persistence wiring mid-run
• Stage and run-control result snapshots use structuredClone instead of JSON.parse(JSON.stringify(...)) for safe deep copies
• Live transcript text block handling preserves explicit empty text consistently and omits text for non-text-only blocks
• pause, interrupt, and kill now reject all:true + stageId instead of silently ignoring stageId
• Transcript notice rendering is bounded, and structured transcript output shows (no body) for entries without text/output

Agent-facing rendering

• render-result.ts: TUI notice renderers for all six new result types
• renderWorkflowToolContent: structured plain-text renderers for stages, stage, transcript; compact one-liners for send, pause, reload; format: "json" falls back to raw JSON for all actions

Tests

• test/unit/workflow-schema.test.ts — schema validation for all new parameters and action literals
• test/unit/slash-dispatch.test.ts — stage inspection/control behavior, transcript fallback ordering, limit/tail semantics, send semantics (no-op vs. empty string), pause/reload edge cases, and in-process reload behavior
• test/integration/mock-extension-api.test.ts — render-result width coverage for transcript notices

Documentation

• README updated with new action descriptions and complete parameter table
• packages/workflows/CHANGELOG.md updated under [Unreleased] with ### Added and ### Fixed entries

Test plan

• AGENT=1 bun test test/unit/workflow-schema.test.ts test/unit/slash-dispatch.test.ts
• bun test test/unit/slash-dispatch.test.ts test/unit/workflow-schema.test.ts test/unit/run-detail-render.test.ts
• bun test test/integration/mock-extension-api.test.ts test/integration/mcp-entrypoint.test.ts
• bun run typecheck
• Interactive tmux QA with bun packages/coding-agent/src/cli.ts: created a temporary .atomic/workflows/qa-dummy.ts, verified /workflow list, /workflow inputs, /workflow qa-dummy, /workflow status --all, workflow tool stages/stage/transcript, workflow tool pause --all, and workflow tool reload; removed the temporary workflow afterward
• Pre-commit / pre-push hooks ran bun run lint and bun run test:unit cleanly

[claude]

Code review

Solid feature with good schema and test discipline. A few things worth tightening before merge.

Bugs / dead code

1. Unreachable ctx.reload fallback in the /workflow reload slash handler (packages/workflows/src/extension/index.ts:2532-2543). reloadWorkflowResources is declared in the surrounding factory closure (index.ts:1845), so typeof reloadWorkflowResources === "function" is always true. The ctx.reload branch is dead, and PiCommandContext.reload?: becomes a dead interface property. The matching "Reload unavailable in this runtime." branch in makeExecuteWorkflowTool (index.ts:1113-1115) is also unreachable in the registered tool — only exercised by unit tests that omit the 3rd arg. Either drop the dead branches and the reload field, or wire them to a real condition.

2. ExtensionAPI.sendUserMessage added but never called (index.ts:284-287). Production code does not invoke it; only the regression test references it as a sentinel asserting it is NOT called. The test can keep working without declaring it on the interface (cast through as never/as unknown). Avoid expanding public surface for sentinel-only consumers.

3. hasOwnPayloadProperty treats text: undefined as a payload. Object.hasOwn(args, \"text\") returns true for text: undefined, and promptPayloadFromArgs then forwards undefined to store.resolveStagePendingPrompt. The PR explicitly preserves the empty-string-is-valid case, but it should not extend to explicit undefined — that probably should remain a no-op. Consider tightening to args.text !== undefined (and same for response/message).

Concurrency / correctness

4. Reload-during-running-workflows race. reloadWorkflowResourcesNow rebuilds runtimeRef, persistenceRef, configLoadRef, and replaces statusWriterRef mid-flight. In-flight workflows that already captured the previous runtime keep running against stale state while new dispatches see the new runtime — status writes may end up split across files, persistence ports can mismatch the active save, and the running coding-agent sessions still hold the old MCP/intercom wiring. The trailing .catch(() => {}) queue serializes reloads against themselves but does not address concurrent stage execution. Consider either gating reload on runs.length === 0 (or just in-flight runs) with a clear noop message, or documenting the truncation semantics explicitly.

5. pause --all is silently interruptAllRuns(). interruptRun is currently a literal alias for pauseRun, so this works today, but the call labels are inconsistent and the coupling is fragile — if pauseRun and interruptRun diverge later, pause --all quietly becomes "interrupt all". Either add a pauseAllRuns() helper or leave a comment.

Smaller things

6. boundedCount(0) produces truncated: true for any non-empty input. A caller explicitly passing tail: 0 ends up with entries: [] and truncated: true. Arguably defensible ("you asked for 0; we discarded everything") but confusing. Returning truncated: false when count is 0 is more honest. Also, tail and limit are coalesced (args.tail ?? args.limit) so both produce "last N" semantics — the schema descriptions are slightly out of sync with the implementation.

7. cloneStage uses JSON.parse(JSON.stringify(...)). Works because stage snapshots are already JSON-clean, but structuredClone(stage) is faster, safer, and consistent with modern Bun/Node. Same opportunity for the existing inline clone inside inspectRun.

8. Very long inline return literals in the new tool dispatch paths. E.g. the transcript run-target error branches (index.ts:1007-1009) are 200–300+ chars per line; splitting them across multiple lines would dramatically improve diff/review readability without changing behavior.

9. Reload error propagation. loadWorkflowConfig()/discoverWorkflows() rejections propagate through reloadWorkflowResources to the slash handler's await, with no catch in subcommand === \"reload\". The tool-execute path is similar. A rejected reload from a config error becomes an unhandled rejection up the registerCommand boundary. Worth a try/catch around the slash invocation, or returning a structured noop result.

Test coverage gaps

• pause action — no test for any of its branches (single run, ambiguous, missing, --all, stage-scoped).
• transcript source \"live\" — only the snapshot fallback path is exercised. The liveHandle.messages.length > 0 branch and transcriptEntryFromMessage are uncovered.
• send delivery modes prompt, steer, resume, and the live auto → steer / resume selection paths are not unit-tested. Only prompt-answering and auto → followUp are covered.
• Reload queue serialization (concurrent reload calls) and reload failure recovery (does the queue survive a thrown reload?) are not exercised.
• format=json is only directly tested for transcript; stages and stage JSON output paths aren't asserted.

Nice things

• Schema descriptions are concrete and verified by the new descriptions test.
• Snapshot ordering coverage is genuinely thorough (tied timestamps, empty result/error, terminal entries after tools).
• The trailing .catch(() => {}) on the reload queue is a tidy way to keep the chain alive after a failure.
• Good split between TUI renderResult (clipped, |-joined) and the agent-facing renderWorkflowToolContent (full text, line-per-entry).
• delivery: \"answer\" explicitly does not fall through to live followUp, guarded by a dedicated regression test.

Overall recommendation: address #1, #2, #3, and #4 before merging; the rest can be follow-ups.

[claude]

Review — feat(workflows): add stage introspection and control actions

Nice scope and very solid test coverage (schema validation, prompt-vs-followUp delivery semantics, transcript ordering, reload paths). The action design feels coherent with the existing interrupt/kill/resume shape, and the move from JSON.parse(JSON.stringify(...)) to structuredClone is a welcome perf/correctness win. A handful of items worth a second pass below.

Bugs / correctness

1. stageFailureMessage leaks "interrupt" wording into the pause error path. packages/workflows/src/extension/index.ts:878-889 returns "No active stages to interrupt for run: …" in the default branch, and that helper is reused from pause (line 1237). Users pausing a run that returns an unexpected reason will see a misleading "interrupt" message. Either parameterize the verb or branch the default text on the action.

2. messageText collapses empty content inconsistently. index.ts:761-772 returns the raw "" for string content but returns undefined for an array of empty text blocks (text || undefined). A legitimately empty assistant message disappears from live transcripts only when authored as blocks. Suggest either preserving "" in both branches or normalizing to undefined in both, consistently — current behavior is a footgun.

3. summarizeStage returns the store's pendingPrompt by reference. index.ts:727-738 clones nothing — callers receiving a stages result can mutate the underlying store. The stage action correctly uses structuredClone via cloneStage; summarizeStage should do the same for pendingPrompt (or at least shallow-clone it) to keep the store immutable from the agent's perspective. The PR replaced JSON-clone elsewhere precisely to formalize this boundary.

4. PR description references API surface that isn't in the diff. The body lists PiCommandContext.reload?: () => Promise<void> | void and ExtensionAPI.sendUserMessage? as additions, but neither symbol appears in index.ts (grep is empty). Either the description is stale (callback wiring took their place — fine, just update the body) or those fields were dropped during review and the PR body needs trimming.

5. Doc comment on ExtensionAPI.sendMessage was deleted. The old block at the sendMessage?: declaration described the workflows:input-form sticky-card contract — it's gone with no replacement. Looks accidental; the field is still used by the inline input form so the rationale is still relevant.

Minor

6. Reload TOCTOU is benign but the messaging diverges. The outer case "reload" handler checks inFlightRunCount() (line 1241) and then awaits a serialized reloadWorkflowResourcesNow that re-checks (line 2000). If a run starts in the gap, the inner check throws WorkflowReloadBlockedError and the agent sees "Reload failed: Reload skipped: N workflow run(s) still in flight…" (double prefix from reloadFailureMessage). reloadFailureMessage already unwraps WorkflowReloadBlockedError to avoid that — good — but worth a one-line comment explaining why the outer check is kept (UX-only) and the inner is authoritative.

7. stringifyWorkflowToolResult defensive fallback is unreachable. JSON.stringify(result, null, 2) ?? String(result) (line 528): result is a typed object, so JSON.stringify always returns a string here. The ?? String(result) is dead code; drop it for clarity.

8. includeToolOutput is silently snapshot-only. The live transcript path (line 1117-1131) uses transcriptEntryFromMessage, which doesn't surface tool output. Either thread it through (if live tool outputs are reachable from AgentSession.messages) or document on the schema field that the flag only applies to snapshot fallback. Right now an agent that passes includeToolOutput: true against a live stage will quietly get back entries without tool output and may assume the stage has none.

9. text cast on liveHandle.messages. m as MessageLike (line 1119) is a structural narrowing cast. Reasonable for now, but consider tightening StageControlHandle.messages or exporting a WorkflowMessageLike type so the cast can become a declaration. The compiler currently can't catch upstream shape drift.

10. Schema description in README compresses three fields into one row. The parameter table renders "text/response/message" as a single key — agents reading just the README might think it's a literal field. Splitting into three rows (or noting "either of …") would be clearer.

Test coverage

Coverage of the new schema literals, prompt-vs-followUp delivery semantics, transcript ordering ties, and reload state machine is strong. Gaps worth filling:

• pause with --all/all: true when no in-flight runs exist (the noop branch at line 1224-1227).
• stage action with --all runId (the "Stage inspection requires a single run" branch).
• Happy-path coverage for delivery: "prompt" and delivery: "steer" — currently only auto→followUp is exercised against a live handle.
• pause on a run whose target stage was already paused / not active (so stageFailureMessage is hit from pause).

Nits

• index.ts is now ~3.1k lines. The stage-tool helpers (lines ~691-909) form a natural module — extracting to e.g. extension/stage-tool-helpers.ts would improve discoverability and shrink the import surface of index.ts. Not blocking.

Overall: ship-worthy after the stageFailureMessage wording fix and the summarizeStage cloning leak; the rest are quality-of-life items. The behavioral changes (in-process reload, snapshot ordering, empty-string answer semantics) are well-justified and the CHANGELOG entries capture them clearly.

🤖 Generated with Claude Code

[claude]

Review — feat(workflows): add stage introspection and control actions

Solid extension to the workflow tool — the new actions slot cleanly into the existing dispatcher and the test coverage is impressively thorough (transcript ordering ties, empty-string answer payloads, prompt-id mismatch fall-through, TOCTOU-blocked reload). The patch-3 fixes (structuredClone, cloned pendingPrompt, messageText empty-block preservation, pauseAllRuns, action-specific failure wording) address real corner cases. Below are constructive notes — none are blockers.

Bugs / correctness

1. compactWorkflowToolMessage produces awkward send output (packages/workflows/src/extension/index.ts:540-553). When target.kind resolves to all/not_found/ambiguous for send, the result has runId: \"--all\"|target.target and stageId: \"\". The compact renderer interpolates as send: --all noop — Send requires a single run., which is fine, but for a not-found stage you can also get send: noop — ... (empty runId for { kind: \"not_found\", target: \"\" }). Consider eliding empty stageId/runId segments to avoid the trailing space.

2. Schema doesn't constrain mutually-exclusive args. { action: \"pause\", all: true, stageId: \"foo\" } validates and silently drops stageId (only pauseAllRuns() runs). Same for kill/interrupt. Either reject in the schema or at least surface a warning in the result. At minimum, the README/schema descriptions should call out that stageId is ignored when all: true.

3. renderResult transcript notice doesn't bound width (packages/workflows/src/extension/render-result.ts). r.entries.map(...).join(\" | \") produces an unbounded one-liner; for a 50-entry transcript this will be a multi-thousand-char line. Consider capping by entry count or character budget and appending … (+N more) — renderNotice already deals with chrome but the inner payload is a single line.

4. Tool entries with includeToolOutput: false render with no body in the structured text renderer (renderTranscriptToolContent). The metadata header ([3] role=tool tool=read timestamp=…) is the only output. That matches the intent, but the chat-notice renderResult then prints tool: read via the entry.text ?? entry.output ?? entry.toolName ?? \"\" chain, which is fine; the structured renderer just looks visually empty. A (no body) placeholder or skipping the empty line would tighten it.

Schema / docs drift

5. limit / tail descriptions imply they're general, but both are only consulted by the transcript branch. Worth noting in the description (or moving them under an action-conditional schema). Same with includeToolOutput.

6. README parameter block is incomplete (packages/workflows/README.md:208-217). Newly added params (statusFilter, format, limit, tail, includeToolOutput, delivery, promptId, reason) aren't in the table — agents browsing the README won't discover them without reading the schema.

7. CHANGELOG only lists three ### Fixed entries for what is largely a feature add. The new actions (stages, stage, transcript, send, pause, reload) belong under ### Added. CLAUDE.md is explicit about appending to existing subsections under [Unreleased].

Code quality

8. liveHandle.messages.map((m) => transcriptEntryFromMessage(m as MessageLike)) uses a structural-shape cast (as MessageLike) over what is presumably a typed SDK message. If the upstream StageControlHandle.messages has a real type, prefer importing and adapting it rather than the MessageLike shim — that drops a layer of any-flavored unsafety the CLAUDE.md style guide explicitly discourages.

9. boundedCount's default of 50 is a magic number. A named constant (DEFAULT_TRANSCRIPT_LIMIT) at the top of the helpers block would make the schema description and the implementation consistent.

10. reload TOCTOU: the inline comment correctly notes the fast-path check is advisory and reloadWorkflowResourcesNow re-checks inside the serialized queue. Good. But the error message surfaced from WorkflowReloadBlockedError will reach a user who already passed the fast check — fine, just confirming this is intentional.

11. reloadFailureMessage echoes raw error messages. Not a security issue for workflow config errors, but worth flagging if any of the downstream config loaders ever embed file paths or env values you don't want surfaced in tool output.

Test coverage gaps

• No direct test for limit (everything exercises tail or defaults). One test pinning the precedence (tail overrides limit) and one for limit alone would lock in the schema description's claim.
• No test for { action: \"pause\", all: true, stageId: \"x\" } — current behavior silently ignores stageId. Worth pinning whichever semantics you settle on.
• The transcript source can be live or snapshot; the live-path tests use the MessageLike shape directly. A test exercising a non-text content block (e.g., { type: \"tool_use\" }) would confirm messageText correctly returns undefined (so the entry's text is omitted) rather than coercing to \"\".

Nits

• packages/workflows/src/extension/render-result.ts:288-310 — the inline as StageListResult / as TranscriptResult casts inside the switch are unnecessary since the discriminator narrows result already.
• Subject line of the second commit is fix(workflows): — these aren't really fixes-of-bugs-on-main, more iterative review responses. refactor or rolling them into the feature commit on merge would be cleaner history.

Overall: well-scoped, well-tested, ships an obvious capability gap. The items above are mostly polish + one real schema validation gap (--all + stageId).

[claude]

Review — feat(workflows): add stage introspection and control actions

Overall this is a solid, well-tested addition. Test coverage is thorough (transcript ordering, empty payload semantics, prompt-mismatch fall-through, live vs. snapshot fallback, --all + stageId rejection, reload TOCTOU). Replacing JSON.parse(JSON.stringify(...)) with structuredClone and defensively cloning pendingPrompt before handing it to the agent are nice cleanups.

A few things worth a look — none blocking:

Suggestions

1. transcript mislabels source when a live handle exists but is empty — packages/workflows/src/extension/index.ts:1148

   if (liveHandle !== undefined && liveHandle.messages.length > 0) { ... live ... }
   // else
   const fallback = snapshotTranscriptEntries(...);
   return { source: \"snapshot\", ... };

   If a stage is registered (stageControlRegistry.get returns a handle) but hasn't streamed any messages yet, the result is labelled source: \"snapshot\". That's a little misleading for callers using the field to decide whether to re-poll. Either keep the live path and return an empty live transcript, or add a third source like \"live-empty\".

2. limit / tail schema accepts non-integer / negative / NaN / Infinity — packages/workflows/src/extension/workflow-schema.ts:120-126
boundedCount defensively coerces all of these to 0, but the schema is Type.Number(). Consider Type.Integer({ minimum: 0 }) so agents get a clear validation error instead of an empty-transcript success.

3. Same source-label issue for transcript error cases — packages/workflows/src/extension/index.ts:1100-1124
   For all / ambiguous / not_found, source: \"snapshot\" is hard-coded even though no snapshot was consulted. The entries array carries the actual error notice, so this is mostly a docs concern, but a dedicated \"error\" (or omitting source) would be cleaner.

4. Nit: compactWorkflowToolMessage branches are identical when target is empty — packages/workflows/src/extension/index.ts:549-551

   return target.length > 0
     ? `${action}: ${target} ${status} — ${message}`
     : `${action}: ${status} — ${message}`;

   Could be collapsed to ${action}:${target ? ` ${target}` : \"\"} ${status} — ${message}.

5. reason: \"\" is treated as if reason was omitted — packages/workflows/src/extension/index.ts:1304-1307
args.reason ? ... : ... — elsewhere in this PR (e.g. text: \"\" answering a pending prompt) explicit empty strings are deliberately treated as meaningful. Worth a args.reason !== undefined check here for consistency, or document the intentional asymmetry.

6. reloadWorkflowResourcesNow({ allowInFlight: true }) silently bypasses the in-flight guard — packages/workflows/src/extension/index.ts:2058-2064
   Only used at startup today, where no runs exist, so this is fine in practice. To prevent a future caller from accidentally papering over a real concurrency issue, consider asserting (or at least warning) when allowInFlight: true is passed and inFlightRunCount() > 0.

7. Plain-text transcript output isn't ambiguity-free — renderTranscriptToolContent writes entry.text and entry.output on their own lines after entries: / tool output: headers. If a transcript entry happens to contain a line like entries: or tool output:, the output isn't unambiguously parseable. Probably fine since this is the human-readable / agent-glance format and format: \"json\" is available for structured consumption — just worth flagging if agents are expected to scrape the text form.

Strengths worth keeping

• Serialized workflowReloadQueue with an authoritative re-check inside the queue is the right pattern.
• The pause/interrupt/kill --all + stageId conflict turning from silent-ignore into a noop with a clear message is a real correctness improvement (and the CHANGELOG calls it out).
• Test for chronological ordering of terminal entries after tools (including tied timestamps and empty result/error preservation) is exactly the kind of edge case that prevents future regressions.
• Tool-content rendering preserves full transcript text and avoids TUI chrome — the assertion assert.equal(textContent.includes(\"╭\"), false, ...) is a great regression guard.

🤖 Generated with Claude Code

[claude]

Code Review — PR #1041 (workflow stage introspection and control)

Nice piece of work — the schema, dispatch, tests, and rendering all line up coherently and the iterative fix-up commits show the feedback loop closed cleanly (TOCTOU re-check on reload, messageText empty-block fix, Integer schema tightening, --all+stageId rejection). The default-active-run resolution, structured error returns, and delivery: \"answer\" not falling through to live followUp are all nicely thought-through. Below are the items I'd consider before merging.

Correctness

• transcript source label vs. README contract — Patch 5 changes the live-handle path to return source: \"live\" whenever a handle is registered, even when liveHandle.messages.length === 0 (the prior check was dropped at packages/workflows/src/extension/index.ts:1144). The README still says "falls back to stored stage snapshots when no live session transcript is available" (README:225), which now reads as if an empty live transcript will fall back. It does not — snapshot fallback only triggers when no live handle exists at all. Either tighten the README or restore an empty-live → snapshot fallback so a stage with a live handle but pre-streaming returns the stored result line. The unit test labels empty live handles as live transcript source locks the current behavior in either way; just make sure it's the one you want.

• Empty reload reason — args.reason !== undefined ? \Reloaded workflow resources (${args.reason}).` : ...produces the literal stringReloaded workflow resources ().whenreason: "". The test preserves explicit empty reload reasoncodifies this, but UI-wise it's a wart. Consider trimming and checking for non-empty:args.reason?.trim() ? `(...)` : ...`.

• m as MessageLike cast (index.ts:1146) — MessageLike.content accepts string | readonly { type?; text? }[]. If AgentSession[\"messages\"] ships text under a different shape (e.g. parts, text directly on the message), messageText silently returns undefined for valid live messages. The cast hides this from the compiler. Worth either adding a focused fixture that mirrors the real Pi SDK message shape, or extracting a guarded converter that fails loudly when the shape is unrecognized.

• renderResult StageListItem.pendingPrompt at render-result.ts:107 duplicates the StageSnapshot[\"pendingPrompt\"] shape inline. The renderer-side and the index-side summary types can drift if StageSnapshot.pendingPrompt grows fields. Suggest reusing StageSnapshot[\"pendingPrompt\"] (or a shared WorkflowStagePendingPrompt type) in both places.

Design / API surface

• delivery: \"auto\" semantics — auto routes to answer when snapshot?.pendingPrompt !== undefined. If a stage is awaiting_input and the caller actually wants to steer the live handle instead, they have to know to pass delivery: \"steer\". That's discoverable but not obvious from the schema description; consider mentioning the precedence rule in the delivery field description.

• send with no run target returns stageId: \"\" — The compact rendering path (compactWorkflowToolMessage) filters empty parts, so the output looks fine, but the structured result still carries runId: \"\", stageId: \"\" when the caller asked for a target that doesn't exist. Agents parsing the JSON might misread that as "send happened to nothing." Consider keeping runId populated with the original target.target even on the error legs (you already do this for the not_found/ambiguous legs — just the kind === \"all\" leg passes \"--all\" while the empty-target leg passes \"\").

• README parameter block describes text/response/message payload variants well but doesn't document the auto-delivery precedence (answer > resume > steer > prompt > followUp). Worth a short bullet in the send section since it's the trickiest action behaviorally.

Performance / resource

• Repeated store.runs().find(...) — transcript, stage, send each call store.runs().find((r) => r.id === target.runId) after the target is resolved. Resolving already proves the run exists; a single getRunById(runId) accessor on the store would save the linear scan and make the read-path simpler.

• stages filtering then mapping — (run?.stages ?? []).filter(...).map(summarizeStage) is fine at current scale, but summarizeStage structuredClones pendingPrompt per stage. Cheap for now; just noting it if stages-per-run grows.

Security

• No new external input vectors, no shell-outs, no auth surface touched. reload is gated by in-flight count + a serialized queue + a WorkflowReloadBlockedError re-check inside reloadWorkflowResourcesNow, which is the right shape for a TOCTOU window. The allowInFlight: true bootstrap call's debug-only console.warn is appropriate.

Tests

• Coverage is genuinely thorough: chronological ordering for tied timestamps, empty text vs. omitted payload distinction, promptId mismatch not falling through to followUp, delivery: \"answer\" without a pending prompt being a noop, --all + stageId rejected for pause/interrupt/kill, transcript limit/tail interplay, schema rejection of -1/1.5 for integer fields.
• Missing-ish: a live-handle test where AgentSession.messages uses a shape that isn't the MessageLike content array — to lock down what happens when the SDK's message representation drifts.
• Minor: the registered-tool test relies on factoryModule.default(pi). If the extension factory gains side effects (timers, subscriptions) the clear() in afterEach may not be enough. Worth verifying stageControlRegistry.clear() plus the existing killAllRuns covers everything; not an issue today.

Style / nits

• The two WorkflowToolArgs declarations — the canonical one in extension/index.ts and the trimmed one in extension/render-call.ts — already drift (the render-call copy is missing the new fields). That's intentional and harmless for renderer pruning, but a comment on the render-call interface saying "intentional renderer-only subset of the canonical args" would help the next reader.
• compactWorkflowToolMessage reaches into result.action === \"send\" ? result.stageId : undefined directly. A tiny secondaryTarget(result) helper would isolate the per-action knowledge in one spot.
• // Sentinel-only property: production ExtensionAPI does not expose this. next to the test's sendUserMessage stub is helpful — consider also removing the unused stub keys to avoid implying support.

Overall: solid and well-tested. The README/source-label mismatch on transcript and the empty-reason rendering are the two I'd want addressed before merging; everything else is suggestion-level.

[claude]

Code review

Solid PR — the new stages/stage/transcript/send/pause/reload actions are well structured, the discriminated-union return types stay clean, and test coverage is unusually thorough (800+ new lines covering payload semantics, chronological ordering, terminal-entry tie-breaking, prompt mismatch fall-through, etc.). structuredClone replacing the JSON round-trip is a good upgrade; the schema additions in workflow-schema.ts and the matching WorkflowToolArgs are tight; and the exhaustive never check at packages/workflows/src/extension/index.ts:1466 will catch any future action drift at the type level. Below are the issues I'd address before merge.

Bugs / correctness

1. transcript + --all returns no explanation. packages/workflows/src/extension/index.ts:1091-1100 returns source: \"error\" with entries: []. The ambiguous and not_found branches (1101-1121) push a { role: \"notice\", text: ... } entry — the --all branch should do the same. As-is, renderResult's transcriptNoticeText falls to \"no transcript entries\" and the agent-facing renderTranscriptToolContent prints entries: none, so the user only sees source: error with no message about why. Suggest pushing a notice entry like \"Transcript requires a single run.\".

2. resume doesn't apply the --all + stageId conflict guard. packages/workflows/src/extension/index.ts:1416-1418 returns \"Resume does not support --all.\" regardless of whether stageId is also set. pause/interrupt/kill use allStageConflictMessage(...) for that combination (1246-1253, 1312-1319, 1365-1372). It would be more consistent to either reuse the same helper for resume or document that resume intentionally rejects --all outright.

3. renderResult --all casts for pause/interrupt/kill claim runId: \"--all\" is a string but downstream slicers may misbehave. Specifically renderResult's transcript case does r.stageId.slice(0, 12) and the kill/pause renderers print ${r.runId}, so when runId === \"--all\" you'll get titles like WORKFLOW PAUSE --all: .... That's fine, but the structured compactWorkflowToolMessage builds target from runId and (only for send) stageId — for pause --all you currently get pause: --all noop — ... which is readable but a bit ugly. Consider special-casing --all in compactWorkflowToolMessage (line 544) to drop the literal sentinel from the human-facing line.

Concurrency / TOCTOU

4. Reload still has a small TOCTOU window. The comment at 1281-1282 acknowledges that reloadWorkflowResourcesNow re-checks inFlightRunCount() inside the serialized queue (status.ts re-check at 2057-2066), but between that re-check and the actual runtimeRef.current = … swap at 2111-2119, async work (loadWorkflowConfig, dynamic import(\"node:os\"), discoverWorkflows) happens. A new run dispatched mid-reload could land on the old runtime while config/persistence get swapped. The serialized queue only blocks concurrent reloads, not concurrent dispatch. For a developer-facing reload this is probably acceptable, but worth a comment or an assert to document the trade-off.

5. pauseAllRuns reports success before the actual pause settles. packages/workflows/src/runs/background/status.ts:394-403 iterates and calls pauseRun, which fires void handle.pause() (line 367 / 386) without awaiting. By the time the tool returns \"Paused N run(s).\" the SDK sessions may still be mid-turn. The existing single-run pause has the same behavior, but bulk amplifies it. Either await the handle pauses (returning a Promise<PauseResult[]>) or note in the result message that pause is best-effort.

Code quality

6. store.runs() is invoked 3× per stage-scoped action. e.g. case \"stage\" calls it via resolveToolRunTarget → resolveRunIdPrefix (1620-1628), then via resolveToolStageTarget → resolveStageTarget (1671), then directly at 1078. Each call walks the store. Hoist to a single const runs = store.runs() at the top of each case and thread it through. Same pattern in transcript and send.

7. stageMatchesIdentifier's prefix matching has no minimum length. packages/workflows/src/extension/index.ts:1659-1661 does stage.id.startsWith(target) for any non-empty target. A user passing stageId: \"s\" will match every stage whose id starts with s. resolveStageTarget will report ambiguity (good), but consider requiring something like ≥4 chars before the prefix branch — matches existing run prefix conventions and fails earlier with a more useful message.

8. renderResult uses as casts inside narrowed switch arms (315-355). The comment explains the default arm prevents narrowing — but you could remove the runtime default and rely on the exhaustive type to forbid unknown actions, then drop every as XResult line. Cosmetic.

9. DEFAULT_TRANSCRIPT_LIMIT = 50 is undocumented in the schema. The limit/tail descriptions should mention the default so agents know when omitting both will silently cap entries.

10. Live message → MessageLike cast at line 1146 (m as MessageLike) elides type-checking against the actual SDK AgentSession[\"messages\"] element type. If the SDK changes (e.g. content block discriminant rename), the runtime mapper in messageText/transcriptEntryFromMessage would silently degrade to text: undefined everywhere. Consider importing the SDK message type directly or adding a small runtime smoke test that exercises a real AgentSession.messages shape.

Documentation / nits

11. README --all row under the workflow tool description mentions pause/interrupt/kill but the table at packages/workflows/README.md:192-196 doesn't list reload's no-runId semantic. The new reload row says "Reload discovered workflow resources in-process" but doesn't mention the in-flight guard — worth a one-line note that it noops when runs are active.

12. Schema description for runId (workflow-schema.ts:115) lists status/stages/stage/transcript/send/pause/resume/interrupt/kill — correctly omits reload. Good.

13. applyEntryLimit returning {entries: [], truncated: false} for tail: 0/limit: 0 is tested and intentional, but if a user mentally maps 0 → \"unlimited\" (common in CLI conventions), they'd be surprised. The schema description for limit could explicitly say "set to omit for default of 50; 0 returns no entries".

Test coverage

Coverage looks strong. The chronological ordering tests (1263-1525) cover the trickiest behavior thoroughly. Two gaps worth filling:

• No test for pauseAllRuns returning success while some runs have no live handles (the partial-success path through pauseRun returning no_active_stages).
• No integration test for the in-flight reload guard at the serialized-queue layer (1763-1779 covers the fast-path check; the inner WorkflowReloadBlockedError only fires when allowInFlight !== true and a run starts between the fast check and the inner check — hard to provoke deterministically, but a setImmediate-injected run between the two checks would verify the TOCTOU re-check actually catches it).

Nothing here is a blocker — the bug in (1) is the only behavior issue I'd want fixed before merge; (2)-(7) are quality improvements.