feat(workflows): add lifecycle steer notifications

[flora131]

Summary

Delivers workflow lifecycle notices as steer prompts into the main chat/model context when a run completes, fails, or pauses awaiting input. Replaces the earlier passive append-only approach: notices now call pi.sendMessage(..., { triggerTurn: true, deliverAs: "steer" }), waking an idle model or delivering into a streaming one.

Key Changes

New: lifecycle-notifications.ts

• Store observer — subscribes to workflow store snapshots and emits one steer notice per lifecycle transition for completed, failed, and awaiting_input states (run-level and stage-level)
• Steer delivery — all emitted notices use { triggerTurn: true, deliverAs: "steer" } so the model context is updated on every lifecycle transition
• Deduplication — per-run and per-prompt delivered sets prevent repeat notices; keyed by active pause timestamp rather than stale prompt metadata to correctly handle promptless pauses after resolved structured prompts
• Suppression context — withWorkflowLifecycleNotificationsSuppressed wraps restore/replay paths so historical workflow states seed dedupe state without emitting notices into the current chat
• Renderer registration — registerLifecycleNoticeRenderer registers a CardComponent renderer for the workflows:lifecycle-notice custom message type (idempotent per-host)

Config (config-loader.ts)

• New WorkflowNotificationsConfig interface and workflowNotifications field on WorkflowExtensionConfig / WorkflowEffectiveConfig
• Supports enabled (default: true) and notifyOn (default: ["completed", "failed", "awaiting_input"])
• Full validation, merge, and default-application logic

Extension wiring (index.ts)

• Exports new PiMessageRenderComponent and PiMessageRendererResult types; broadens registerMessageRenderer signature from string to PiMessageRendererResult to match pi's runtime capabilities
• Creates and retains WorkflowLifecycleNotificationState for the session lifetime
• Reinstalls the notification subscriber on config load/reload
• Session-boundary reset — calls resetWorkflowLifecycleNotificationState on session_start so reused run IDs in a later chat session still emit lifecycle notices
• Restore suppression — wraps restoreOnSessionStart in withWorkflowLifecycleNotificationsSuppressed + explicit seedWorkflowLifecycleNotificationState to prevent historical run states from replaying notices into a fresh session
• Cleans up the subscriber on session stop

Type cleanup (chat-surface-message.ts, inline-form-overlay.ts)

• Removes unknown casts previously needed to work around the narrower registerMessageRenderer string-only signature; both renderers now typecheck cleanly against the broadened PiMessageRendererResult type

Docs / Changelog

• README.md — documents workflowNotifications config with example
• CHANGELOG.md — entries under [Unreleased] for the new feature and the awaiting-input dedupe fix

Tests

New test/unit/workflow-lifecycle-notifications.test.ts (591 lines) covering:

• All emit paths (run completed, run failed, stage completed, stage failed, awaiting input)
• Deduplication and suppression/seeding
• Session-boundary state reset
• Renderer registration (idempotency)
• notifyOn filtering
• Steer delivery options

Expanded config-loader.test.ts and config-loader-helpers.test.ts with workflowNotifications validation and merge cases.

bun run typecheck — passes
bun run test:unit — 1659 pass / 0 fail

Closes #1085

[flora131]

Implementation Notes

Files changed

• test/unit/workflow-lifecycle-notifications.test.ts
  • Added regression coverage for structured prompt -> resolve -> promptless awaiting input.
  • Added coverage that repeated promptless pauses dedupe by awaitingInputSince and emit again for a new timestamp.
  • Added coverage that a second structured prompt emits with the new prompt ID.
• packages/workflows/src/extension/lifecycle-notifications.ts
  • Changed stage awaiting-input notice details to use only stage.pendingPrompt for active structured prompt fields.
  • Changed stage awaiting-input dedupe key to use stage.pendingPrompt.id when active, otherwise stage.awaitingInputSince ?? "active".
  • Left stage.promptFootprint untouched for store/UI historical metadata.
• packages/workflows/CHANGELOG.md
  • Added an Unreleased/Fixed entry for promptless awaiting-input notices after resolved prompts.

Tests run

• bun test test/unit/workflow-lifecycle-notifications.test.ts
  • First run after adding tests failed as expected on the stale promptFootprint regression: promptless pauses after a resolved prompt were suppressed.
  • Final run passed: 16 pass, 0 fail.
• bun test test/unit/config-loader.test.ts test/unit/config-loader-helpers.test.ts
  • Passed: 67 pass, 0 fail.
• cd packages/coding-agent && bun test test/messages.test.ts test/session-manager/build-context.test.ts test/compaction.test.ts test/suite/agent-session-queue.test.ts
  • Passed: 59 pass, 2 skip, 0 fail. The skipped tests are existing LLM summarization skips in test/compaction.test.ts.
• bun run typecheck
  • Passed (tsc --noEmit).

Decisions / tradeoffs

• Preflight skipped bun install because the checkout already had bun.lock, populated node_modules/, packages/coding-agent/dist/, matching Bun 1.3.14, and no submodules.
• Treated pendingPrompt as the sole source of active structured prompt data in lifecycle notifications.
• Kept promptFootprint preserved in the store and ignored it only for active lifecycle notice key/content generation.
• Used the existing awaitingInputSince timestamp as the promptless pause generation key rather than adding new store metadata.

Blockers

• None.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
bastani	🟢 Ready	View Preview	May 28, 2026, 10:34 AM

[claude]

Review — feat(workflows): add lifecycle chat notifications

Thanks for the well-scoped PR and the thorough test suite. The new deliverAs: "append" delivery, the excludeFromContext plumbing, and the lifecycle-notification state machine hang together cleanly. The dedupe model — deliveredTerminalRuns keyed by ${kind}:${runId} and deliveredInputPrompts keyed by promptId-or-awaitingInputSince — is a nice fix for the stale-prompt-footprint bug, and the withWorkflowLifecycleNotificationsSuppressed + reset-on-session-start pair gives a clean way to seed restored state without re-emitting.

Most observations below are nits/polish — nothing I think blocks merging.

Potential bugs / correctness

1. Renderer return type cast may break the rendered output (packages/workflows/src/extension/lifecycle-notifications.ts:214-223). The renderer returns a CardComponent ({ render(): string[]; invalidate?(): void }), but it is registered through the host's pi.registerMessageRenderer, which in the wrapper at packages/workflows/src/extension/index.ts is typed (payload: unknown) => string:

   pi.registerMessageRenderer?.(
     event,
     renderer as (payload: unknown) => string,
   );

   The cast hides a real shape mismatch — if the host actually expects a string (or calls String(component)), users may see [object Object] in the TUI for the persisted notice. Worth verifying on the host side what registerMessageRenderer consumes, and either always returning a string here or widening the ExtensionAPI signature to allow component returns.

2. Quote injection into the suggested workflow({...}) command (lifecycle-notifications.ts:241). The stage-scope awaiting-input message inlines IDs into a JS-looking snippet:

   `... workflow({ action: "send", runId: "${details.runId}", stageId: "${details.stageId ?? ""}", promptId: "${details.promptId ?? ""}", response: ... })`

   If a runId/stageId/promptId ever contains a " or \, the suggestion becomes unparseable. Not a security issue (it's a hint, not eval'd), but copy/paste would break. Either JSON.stringify each interpolated id, or assert id format upstream.

3. Renderer-dedupe WeakSet key is brittle (lifecycle-notifications.ts:211). The cache key is the register function reference. In index.ts the registerMessageRenderer passed in is a freshly-allocated arrow each time the factory runs:

   registerMessageRenderer: pi.registerMessageRenderer
     ? (event, renderer) => pi.registerMessageRenderer?.(...)
     : undefined,

   Today only one such wrapper exists, so the dedupe works incidentally. If anyone ever calls registerLifecycleNoticeRenderer more than once with a freshly-built wrapper, the WeakSet check would silently miss and you'd re-register. Safer to key off LIFECYCLE_NOTICE_CUSTOM_TYPE (e.g. a module-scoped let registered = false) since the event name is unique.

4. Fire-and-forget void send(...) (lifecycle-notifications.ts:135). sendMessage may return Promise<void> (the ExtensionAPI.sendMessage signature in the diff is => void | Promise<void>), and a rejection here becomes an unhandled rejection. A .catch(() => {}) (or routing through a logger) would prevent that without changing behavior.

5. Append delivery during streaming pushes directly to state.messages (packages/coding-agent/src/core/agent-session.ts:1395). The new branch unconditionally calls appendCustomMessage() when isAppendDelivery is true — even when this.isStreaming. The existing queue exists precisely to avoid concurrent mutation while the loop is iterating. Your test exercises this path and passes, so it likely works in practice, but it would be worth a one-line comment explaining why bypassing the queue is safe for append-only entries (or, if you want to be defensive, route appends through the queue and flush them without steering).

Style / consistency (minor)

6. isExcludedFromContext is duplicated between compaction.ts and messages.ts with identical bodies. Moving it next to CustomMessage in messages.ts and re-exporting would avoid the second copy and prevent the two from drifting.

7. Type-name mismatch: WorkflowNotificationsConfig (config-loader.ts) vs WorkflowLifecycleNotificationConfig (lifecycle-notifications.ts), and the public-config field workflowNotifications (no "lifecycle") vs the internal WorkflowLifecycleNoticeKind. Picking one shape across the codebase makes the docs/config easier to learn.

8. Fall-through cleanup in findValidCutPoints (compaction.ts:325-340). The new explicit case "bashExecution" / "custom" block with break is correct, but case "custom_message" in the entry-type switch is now a no-op because the real handling lives in the post-switch if (entry.type === "branch_summary" || isContextParticipatingCustomEntry(entry)). Consider folding both checks together to avoid the implicit "the switch is partial; the if finishes the job" pattern — easy to miss in a future refactor.

9. Set typed too widely: WORKFLOW_LIFECYCLE_NOTICE_KIND_SET: Set<string> in config-loader.ts. Typing it as Set<WorkflowLifecycleNoticeKind> and using a (value): value is WorkflowLifecycleNoticeKind predicate keeps the narrow type all the way through.

Performance

• store.subscribe(inspect) iterates all runs and stages on every change. For long-lived sessions with many workflow runs and stages, this is O(runs × stages) per change. The hot path is small here (Sets are O(1)), but if you ever expect hundreds of runs in a session, an "ended/awaiting changed since last tick" diff against the previous snapshot would scale better. Fine as-is for current usage.

Security

• No new I/O paths or untrusted-deserialization paths introduced. The notice payload contains user-controlled strings (workflow name, prompt message, error) but they are only rendered as plain text in a custom message — not interpolated into shell or HTML.
• Point 2 (quoting in suggested command) is a UX issue, not a security issue, since the suggestion is never executed.

Test coverage

Genuinely thorough — appreciated. In particular:

• Suppression-around-restore + re-seed (restore suppression after reset seeds restored history without emitting)
• Promptless-after-resolved-prompt regression (covers the dedupe-key bug fix)
• Reused-runId-after-session-boundary (covers reset)
• Filtering by notifyOn for both run-level and stage-level prompts
• deliverAs: "append" during streaming + on next prompt verifying the message does not enter context

A couple of small gaps you might consider adding:

• A test that an emitted notice with triggerTurn: true (and so deliverAs: "followUp") does enter context — a positive case mirroring the excludeFromContext: true negative case — to lock in that opt-in behavior.
• A test that compaction does not pick an excludeFromContext: true CustomMessage (the role-level message variant, not just custom_message entries) as a cut point or turn start. The new isExcludedFromContext branch in findValidCutPoints/findTurnStartIndex runs against entry.message, and that path is only indirectly exercised right now.

CLAUDE.md compliance

• Bun used everywhere; no node/npm in changes.
• bun:test + node:assert/strict for new repo-level tests; vitest for the new packages/coding-agent test matches the upstream-pi convention used by sibling files.
• .js import extensions throughout new files.
• CHANGELOG.md entries appended under [Unreleased] for both coding-agent and workflows; no released sections modified.
• README updated alongside the config schema.

Nice work overall — the regression-test selection is on point, and the suppression abstraction is much cleaner than ad-hoc flags would have been.

[claude]

Review — PR #1092

Nice scoped addition with a solid 407-line regression suite. The store-level subscription + dedupe-set design is clean, and the restore/seed/suppress dance reads correctly. Comments below.

High-priority

1. PR description does not match the implementation. The body advertises a substantial SDK change in packages/coding-agent — a new deliverAs: \"append\" mode, an excludeFromContext field on CustomMessage, edits to compaction.ts / branch-summarization.ts / buildSessionContext, plus a configurable triggerTurn flag. None of that is in the diff. Only packages/workflows/** and test/unit/** are touched, the call site emits deliverAs: \"steer\" (the only modes the ExtensionAPI.sendMessage signature accepts are \"steer\" | \"followUp\" | \"nextTurn\"), and triggerTurn is hard-coded true in lifecycle-notifications.ts:457. The CHANGELOG entries already acknowledge this ("Fixed … instead of passive append-only delivery"). Please rewrite the PR description so reviewers and the eventual release-notes audience see what actually shipped — at minimum drop the SDK section, the triggerTurn paragraph, and the "Notes" claim about `triggerTurn: false (default) produces pure append-only notices".

2. CHANGELOG mixes a feature and its in-development flip-flop. packages/workflows/CHANGELOG.md lists ### Added — main-chat lifecycle steer notices … and ### Fixed — Fixed workflow lifecycle notices to steer the model context … instead of passive append-only delivery. The second bullet describes an intermediate state of this same PR that never reached users. For a first-time feature, the "Fixed" entry is confusing — recommend collapsing to a single "Added" bullet and keeping only the genuinely separable fixes (stale-dedupe and session-boundary reset).

Medium

3. Renderer dedupe key won't survive a factory reload. registerLifecycleNoticeRenderer (lifecycle-notifications.ts:208) keys rendererRegisteredHosts on the wrapped arrow register, not on pi itself. packages/workflows/src/extension/index.ts:1996 rebuilds that wrapper inside factory(pi), so each /new / /resume / /fork / /reload invocation produces a fresh key and the WeakSet never short-circuits. Compare registerInlineFormRenderer (src/tui/inline-form-overlay.ts:94-95) which dedupes on pi directly — the doc-comment above it explicitly justifies this exact choice. Either pass pi through and key on it, or accept that the dedupe is effectively dead code and drop it. (If pi tolerates re-registration today, this is latent rather than urgent.)

4. triggerTurn is not configurable. Always { triggerTurn: true, deliverAs: \"steer\" } (lifecycle-notifications.ts:457). For users who want notices in the transcript without waking an idle model on every workflow completion, there's no escape hatch short of enabled: false. If you intended this knob (the PR description implied it), expose it on WorkflowNotificationsConfig; if you didn't, just delete the corresponding paragraph from the description.

5. Stage-awaiting-input notice prints empty-string fields. formatWorkflowLifecycleNoticeText for the stage scope (lifecycle-notifications.ts:238) emits:

workflow({ action: \"send\", runId: \"run-3\", stageId: \"\", promptId: \"\", response: ... })

…whenever stageId or promptId is missing. That's a worse hint than just suggesting /workflow connect <id>. Suggest building the workflow({...}) snippet only when both IDs are known, otherwise fall back to the slash-command form.

Lower-priority

6. Suppression "swallows forever". withWorkflowLifecycleNotificationsSuppressed mutates the dedupe sets before the suppression check (lifecycle-notifications.ts:472-474, 486-488, 497-499), so anything that happens inside the wrapper is permanently marked delivered. Correct for the current restoreOnSessionStart call site, but easy to misuse if reused for a transient mute. A short JSDoc on withWorkflowLifecycleNotificationsSuppressed calling out this "observe + dedupe, do not emit" semantic would save the next reader.

7. Redundant pre-marking + explicit seed. In index.ts:3401-3414, restoreOnSessionStart already pre-dedupes via the suppressed subscriber path, and then seedWorkflowLifecycleNotificationState(state, store.snapshot()) runs on the same data. Idempotent and arguably defensive, but the explicit seed call is dead under current semantics. Either drop it or add a one-line comment explaining why both are needed.

8. inspect runs a full O(runs × stages) scan on every store change. Fine for typical workloads, but notify() fires on every recordNotice / tool event. If you expect runs with many stages, a lastSeenVersion short-circuit (the store already increments a version counter) would cheaply skip the scan when nothing relevant changed. Optional.

9. CLAUDE.md style nit on unknown casts. (register as unknown as (event: string, renderer: RawRenderer) => void) (lifecycle-notifications.ts:217) and register as object (line 208) work around the documented ExtensionAPI.registerMessageRenderer return-type being string, even though the host accepts CardComponent | string | undefined (same pattern is used by registerInlineFormRenderer). Worth either updating ExtensionAPI.registerMessageRenderer's type to reflect what hosts actually accept (single fix, helps every call site) or wrapping the cast in a small typed helper. Project-wide concern, not a blocker for this PR.

Tests

Coverage is strong — happy paths, disabled/filtered modes, stage vs run scope, the new promptless-after-resolved-prompt dedupe path, reused-run-ID across session reset, restore-suppression. Two small thoughts:

• The renderer registration path (registerLifecycleNoticeRenderer + makeNoticeComponent) is never exercised. A small unit asserting host.has(...) dedupe and that the registered renderer returns a CardComponent for a details-bearing payload would lock in update readme and mcp servers #3 / Lavaman131/feature/24 7 agent #9.
• Tests live at test/unit/workflow-lifecycle-notifications.test.ts but the module under test lives in packages/workflows/src/extension/lifecycle-notifications.ts. The other workflows tests follow the same root-test/unit/ layout, so this is consistent — just flagging in case there's an ongoing migration toward per-package packages/workflows/test/.

Style / conventions

• Bun / TypeScript ESM .js import extensions used correctly.
• No any introduced; unknown is scoped to dynamic-payload boundaries (acceptable).
• WORKFLOW_LIFECYCLE_NOTICE_KINDS as const satisfies readonly WorkflowLifecycleNoticeKind[] is a nice idiom — keeps the array as the source of truth and validates against the union.

Overall: ship-worthy after the PR-description rewrite and the CHANGELOG cleanup. The renderer-dedupe key and the stage prompt format string are worth a follow-up but don't block.

[flora131]

Addressed feedback from #1092 (comment) in 879bff3.

• Rewrote/kept the PR description aligned with the steer-notification implementation and no coding-agent append API scope.
• Collapsed the in-development passive-append flip-flop out of the workflows changelog.
• Fixed lifecycle renderer dedupe to key on the extension host instead of the per-factory wrapper.
• Changed promptless stage awaiting-input notices to fall back to /workflow connect <run-id> instead of empty stageId/promptId fields.
• Added suppression semantics documentation and an explanatory restore-seed comment.
• Added regression coverage for renderer host dedupe/card rendering and promptless stage hints.

Validation: bun run typecheck, bun run test:unit.

[flora131]

Follow-up amendment pushed in 62b63ae after final type/API review.

Additional adjustment:

• Kept registerLifecycleNoticeRenderer compatible with the existing ExtensionAPI.registerMessageRenderer type while still deduping by rendererHost.

Validation: bun run typecheck, bun test test/unit/workflow-lifecycle-notifications.test.ts, git diff --check, and commit hooks (bun run lint, bun run test:unit).

[claude]

PR review (automated, claude-opus-4-7)

Thanks for tightening this up — the steer/triggerTurn delivery, the promptless-pause dedupe key, and the suppression-during-replay pattern are well-designed, and the test suite covers the interesting state-machine paths thoroughly. A few smaller observations below; nothing that should block merge.

Code quality

• emit is fire-and-forget without a .catch (lifecycle-notifications.ts:142-150). pi.sendMessage is typed to return void | Promise<void>; if the host implementation rejects (e.g. transient I/O), the void operator silently swallows it and the notice is dropped with no observable signal. Consider wrapping with Promise.resolve(send(...)).catch((err) => { /* log via host or ignore */ }) so failures don't disappear into an unhandled rejection.

• Module-level rendererRegisteredHosts: WeakSet<object> (lifecycle-notifications.ts:69) — works correctly for the production single-extension-instance case, but it's process-wide state with no removal path. If the extension were reloaded against the same pi host object (or in a test that reuses a host), re-registration is permanently skipped. The renderer-registration test on l.422–456 dodges this by using a fresh {} host, but a short comment on rendererRegisteredHosts noting the lifetime contract would help future readers.

• Redundant optional chain in index.ts:2014-2016:
  ```ts
  sendMessage: pi.sendMessage
  ? (message, options) => pi.sendMessage?.(message, options)
  : undefined,
  ```
  We've already narrowed `pi.sendMessage` to defined; the `?.` inside the closure is defensive against late mutation of `pi.sendMessage` but obscures intent. Either drop the `?.` or add a short comment ("rebind through `pi` so a later host swap is honored").

Correctness / edge cases

• `inspect` is O(runs × stages) per snapshot tick (l.194–205). Each `store.subscribe` callback iterates every run and every stage; for short sessions this is fine, but for sessions that accumulate many runs the cost compounds with snapshot frequency. Tracking only "runs that changed" would be a future optimization, not a blocker.

• `deliveredInputPrompts` and `deliveredTerminalRuns` grow for the session lifetime. They're bounded by `session_start` reset, but a long-running pi process that issues many small workflows in one chat session will keep accumulating keys. Probably fine in practice — flagging for awareness.

• `withWorkflowDefaults` honors explicit `notifyOn: []` (the `??` chain doesn't short-circuit on empty array), which is the right call for users who want to keep `enabled: true` but mute all kinds. Good — just worth a one-line README mention so users discover this escape hatch.

• Default `notifyOn` = all three kinds + `deliverAs: "steer"` means every workflow transition wakes the model. Intentional per the design, but for users who run many parallel/batch workflows this could become chatty. Worth a doc nudge that disabling `awaiting_input` (or `completed`) is the way out.

Tests

• Coverage is genuinely thorough: terminal/awaiting-input emit paths, promptless-pause dedupe keyed by `awaitingInputSince`, second-prompt distinct key, run-level vs stage-level scope, `notifyOn` filtering, shared-state dedupe across reinstall, omitted-`seedExisting` history seeding, reset for reused run IDs across sessions, and renderer-registration idempotency. Nice work.
• Worth adding: a test that asserts `emit` failures (rejected `sendMessage` Promise) don't crash the subscriber — pairs with the `.catch` suggestion above.

Security / performance

No new attack surface; notice content is built from store snapshots already trusted by the host. `truncateSnippet` correctly bounds error/prompt length at `LIFECYCLE_NOTICE_SNIPPET_LIMIT` (240) including the ellipsis. No regex backtracking concerns (`\s+` on bounded input).

Style

Matches repo conventions — `.js` import extensions, no `any`, Bun-only commands in scripts/tests, `bun:test` + `node:assert/strict`, `[Unreleased]` CHANGELOG section, README example included.

— Generated with Claude Code (opus-4-7)

[claude]

Code Review — workflow lifecycle steer notifications

Overall this is a clean, well-tested addition. The dedupe-state design, restore-suppression pattern, and the renderer-host WeakSet guard are all sensible. Most of my notes are minor / nice-to-have.

Strengths

• Good separation between the lifecycle module and the extension wiring. The module is testable in isolation and the test file (~457 lines) covers the tricky cases: stale-prompt footprints, promptless pauses, reset-across-sessions, restore replay seeding, and renderer-host dedupe.
• The withWorkflowLifecycleNotificationsSuppressed plus explicit seedWorkflowLifecycleNotificationState backstop in session_start handles the "subscriber installed but we don't want to replay history" path cleanly.
• The promptless-input fix (key by awaitingInputSince when there is no pendingPrompt.id) and the hint fallback that drops the malformed workflow(...) snippet when there is no promptId are both nice catches.
• Config validation properly enumerates the allowed notifyOn values and rejects unknown kinds with a useful message.

Issues / suggestions

1. Renderer signature cast is unsafe — packages/workflows/src/extension/index.ts:1995-2004

The cast renderer as (payload: unknown) => string hides two mismatches:

• pi.registerMessageRenderer expects MessageRenderer<T> = (message, options, theme) => Component | undefined, but RawRenderer in lifecycle-notifications.ts:67 only takes one argument. JS silently drops options/theme, so this works in practice but the type system can no longer warn.
• The return type is widened to plain string, even though makeNoticeComponent returns a CardComponent object. The TUI host's Component shape (render(width): string[]; dispose?()) is only compatible with this PR's CardComponent (render(width): string[]; invalidate?()) by duck typing.

Worth either using the real MessageRenderer<T> type in the lifecycle module (and ignoring the extra args) or adding a short comment at the cast site explaining the intended duck typing. As written, a future refactor to MessageRenderer would silently break.

2. CardComponent.render ignores width — lifecycle-notifications.ts:328-337

makeNoticeComponent returns a single-line string array regardless of the requested width. The stage-awaiting notice can be long (truncated prompt up to 240 chars plus the embedded workflow(...) tool snippet) and will wrap awkwardly in narrow panes. A simple wrap on width would render better.

3. Embedded JS-snippet in notice text is fragile — lifecycle-notifications.ts:244-247

If a runId/stageId/promptId ever contains a " or \, the rendered snippet is broken JS. They are internal IDs today, but this is exactly the kind of latent assumption that bites later. JSON.stringify(details.runId) (and similar) would escape correctly.

4. Notice content length and steer prompt budget — lifecycle-notifications.ts:140-151

The content sent via pi.sendMessage for a stage-awaiting notice can be ~300+ chars. With the default notifyOn: ["completed", "failed", "awaiting_input"] in a chatty workflow, this adds up in steer context. Worth at least documenting in the README that lifecycle notices steer real tokens into the model.

5. Loop micro-redundancy — lifecycle-notifications.ts:194-205

notifyOn.has("awaiting_input") is recomputed each iteration over snapshot.runs; hoist it into const notifyAwaiting = notifyOn.has("awaiting_input") outside the loop. Same for the notifyOn.has(kind) checks inside emitTerminalNoticeOnce. Cosmetic but cheap.

6. Race window in reinstallLifecycleNotifications — index.ts:2005-2018

The unsubscribe-then-install ordering means any store update between them is missed. In single-threaded JS with synchronous store.subscribe (verified at store.ts:835), the only way a notification fires between those two lines is if the seed itself causes a store mutation — which it does not. So this is currently safe, but a one-line comment ("seed reads only, so no events are dropped here") would harden the invariant against future edits.

7. Renderer is not unregistered on unsubscribe — lifecycle-notifications.ts:125-208

installWorkflowLifecycleNotifications always returns the store unsubscribe but does not unregister the renderer. That is the intended design (renderers are host-scoped and survive enable/disable toggles), but a one-line comment on registerLifecycleNoticeRenderer would prevent a future refactor from "fixing" the asymmetry.

Test coverage gaps (optional)

• No test exercises reloadWorkflowResources (mid-session config change). The closest is "shared state dedupes terminal notices across reinstall", but it does not go through withWorkflowDefaults / the runtime ref-swap path. A test that flips notifyOn from ["failed"] to ["completed", "failed"] mid-session and confirms previously-completed runs are NOT re-emitted (because seedExisting marks them delivered) would lock in the intended behavior.
• No test for enabled: false then true mid-session.
• No test that confirms notifyOn: [] is accepted by validation and produces zero notices end-to-end.

Style / nit

• config-loader.ts:175 — const value = c["workflowNotifications"] shadows the outer validateConfig parameter value. Renaming the inner one (e.g. notificationsValue) would avoid the read-flow confusion.
• The notifyOn.has(kind) early-returns mean that toggling notifyOn to include "completed" later only emits for future completions — the README should make this "going-forward" semantics explicit, since users might expect a config flip to surface historical state.

Nothing here is blocking. The implementation is solid and the test discipline is good. Suggestions above are mostly hardening for a feature that already does what it advertises.

[flora131]

Addressed the failing Windows CI in 5918486.

Root cause: test/unit/status-writer.test.ts used a fixed sleep(50) before reading the status file, which raced the async status writer on Windows. The test now uses await writer.flush() before writer.unsubscribe().

Validation:

• bun test test/unit/status-writer.test.ts
• bun run typecheck
• Commit/push hooks: bun run lint, bun run test:unit

[claude]

Review — feat(workflows): add lifecycle steer notifications

Solid feature. The deduplication design (per-run terminal keys + per-prompt input keys + active-pause timestamp fallback), the suppression-during-restore pattern, and the session-boundary reset all hang together cleanly. Tests are thorough (457 lines covering the tricky paths — promptless-after-resolved, ID reuse across sessions, restore replay, renderer idempotency). Below are observations from a careful read; most are nits, with two worth attention.

Worth attention

1. Subscriber error escape — a thrown send() could break sibling subscribers.
packages/workflows/src/extension/lifecycle-notifications.ts:140-151 does void send(...). The void operator only discards the return value; it does not catch synchronous throws or unhandled rejections. The store notifies listeners in a tight loop with no per-listener guard (packages/workflows/src/shared/store.ts:235-240):

function notify(): void {
  const snap = snapshot();
  for (const fn of _listeners) { fn(snap); }
}

If pi.sendMessage throws synchronously (e.g., bad customType in a future pi version, message-store overflow), the for…of aborts mid-iteration and downstream subscribers (status writer, store widget) silently stop receiving that snapshot. Suggest wrapping the call:

try {
  void Promise.resolve(send(...)).catch(() => {});
} catch { /* swallow — never break sibling subscribers */ }

The existing emitChatSurface shares this pattern, so it's not a regression — but this PR is adding the first subscriber that calls sendMessage from inside a store callback, which makes the blast radius newly relevant.

2. Dedupe sets grow unbounded for the lifetime of the chat session.
deliveredTerminalRuns and deliveredInputPrompts only shrink on resetWorkflowLifecycleNotificationState (session boundary). For sessions that spawn many short workflow runs (e.g., a long-lived agent loop firing a workflow per task), the sets accumulate forever. Each entry is small, but unbounded retention of run IDs over hours of usage is a smell. Two cheap mitigations:

• Prune entries when store evicts the corresponding run (if the store has such a hook).
• Cap each set at, say, 10k entries with FIFO eviction.

Not blocking, but worth a follow-up issue.

Smaller observations

3. terminalRunKey collapses completed and failed for the same runId into independent slots.
terminalRunKey('completed', id) and terminalRunKey('failed', id) differ, so a run that somehow oscillates status would emit both. Status is mutually exclusive in practice, but if a restored-from-disk run is re-ended with a different terminal status during recovery, you'd get a duplicate. Minor — flag for awareness.

4. awaitingInputKey fallback uses stage.awaitingInputSince ?? "active".
Tests exercise the numeric case (e.g., 123, 456) and the resolved-prompt path. The literal-"active" branch isn't tested. If awaitingInputSince is ever omitted by the store for a promptless pause, every promptless cycle for that stage collapses into one notice for the session — exactly the bug #1085 was fixing for the structured-prompt case, just on a different axis. Worth either asserting the invariant in the store or adding a test that covers awaitingInputSince === undefined.

5. Race between seed and subscribe.
installWorkflowLifecycleNotifications calls seedWorkflowLifecycleNotificationState(state, store.snapshot()) then store.subscribe(inspect). In single-threaded JS these are adjacent synchronous calls — no race in practice. Just noting the assumption: if snapshot() ever became async, you'd miss any transition occurring between the two awaits.

6. runId/stageId not sanitized in slash-command suggestions or dedupe keys.
formatWorkflowLifecycleNoticeText builds /workflow status ${details.runId} and inlines IDs into a JSON-like response hint:

workflow({ action: "send", runId: "${details.runId}", stageId: "${details.stageId}", promptId: "${details.promptId}", response: ... })

If any of those IDs ever contains a " or \, the rendered text is malformed (and the steer notice asks the model to literally run that code). Today IDs are server-generated and safe, but the assumption isn't documented at this boundary. Either escape or comment that callers guarantee shell-safe IDs.

Dedupe keys (${kind}:${runId}, etc.) are opaque tokens — collisions from : in IDs are theoretically possible but never observed/parsed, so harmless.

7. Renderer registration cast.
packages/workflows/src/extension/lifecycle-notifications.ts:225 does (register as unknown as (event, renderer: RawRenderer) => void)(...). This matches the documented pi pattern (see chat-surface-message.ts:138-147), but chat-surface-message.ts additionally uses .call(pi, …) to preserve this for pi's class-backed ExtensionAPI. Here this-binding is OK because register is the wrapper closure built in index.ts:1997-2003 (it captures pi.registerMessageRenderer via property access). Just worth a comment so the next person doesn't "fix" it to match the chat-surface version.

8. Module-level rendererRegisteredHosts WeakSet.
Process-global. Two extension instances loaded in the same process would each correctly register against their own host (different keys). Fine. The state is module-scoped — under jiti's caching this persists across hot reloads, which is the desired behavior (matches chat-surface-message.ts).

9. emit ignores the resolved-and-undefined case.
send is typed void | Promise<void> and there's no surface for failure. If the host queue is full / rejected, the notice is silently dropped — the dedupe set still records it as delivered, so a retry never happens. Probably acceptable (this is fire-and-forget steer messaging), but consider logging on rejection at least at debug level.

Config & docs

• Validation in config-loader.ts:174-196 is precise and the error messages name the exact field/value — nice.
• WORKFLOW_LIFECYCLE_NOTICE_KIND_SET is built once at module load; good O(1) validation.
• withWorkflowDefaults shallow-merges workflowNotifications field-by-field (enabled, notifyOn), so a user supplying only enabled: false retains the default notifyOn. Good.
• CHANGELOG entries follow the repo's [#1085] link convention. README example is concise and shows defaults.

Tests

Coverage is genuinely thorough. The two I'd add:

• awaitingInputSince === undefined fallback path (point Flora131/feat/add skills #4 above).
• Subscriber resilience: sendMessage throws → other store subscribers still fire (verifies point add agent instructions #1 fix when applied).

The existing 1659 pass / 0 fail count from the PR description and the diff-side test additions look healthy. The status-writer.test.ts change from sleep(50) to await writer.flush() is a nice incidental flake reduction — unrelated to lifecycle notifications but welcome.

Verdict

The core design is correct and the dedupe model handles the trickiest paths (promptless-after-resolved, ID reuse across sessions). The two items worth attention before merge are #1 (subscriber error isolation) and a follow-up tracking ticket for #2 (unbounded set growth). Everything else is polish.

[flora131]

Addressed the subscriber error escape feedback in 2247e60.

• Wrapped lifecycle sendMessage delivery in try + Promise.resolve(...).catch(...) so synchronous throws and rejected promises are swallowed.
• Added regression coverage proving a failed lifecycle send does not abort sibling store subscribers, and rejected sends do not surface as unhandled rejections.

Validation:

• bun test test/unit/workflow-lifecycle-notifications.test.ts
• bun run typecheck
• Commit/push hooks: bun run lint, bun run test:unit

[claude]

Code Review — PR #1092 (lifecycle steer notifications)

Solid PR overall. The architecture is clean (state / install / render / format are well separated), the suppression-on-restore pattern is correct, and the test suite genuinely earns its 505 lines — the promptless-pause-after-resolved-prompt test in particular pins down the bug the PR fixes. Findings ordered by impact.

Behavior worth confirming intent on

1. Every notice wakes an idle model with triggerTurn: true. In agent-session.ts:1389, when the session isn't streaming, triggerTurn: true calls agent.prompt(appMessage) — a real LLM turn. For a user who walked away from an idle session, a workflow completion 30 minutes later will burn tokens responding to "✅ Workflow X completed." Consistent with the PR description ("wakes an idle model"), but worth a config knob (e.g., wakeIdleModel: boolean) if users may want passive notices while idle and steers while streaming. Today the only escape hatch is enabled: false globally, which kills the renderer-side notices too.

2. No coalescing across concurrent runs. If a fan-out workflow finishes 8 stages within the same tick, or two parallel runs complete simultaneously, you'll emit multiple separate steers/turns. Dedupe is per-event (per-run, per-prompt), not per-tick. Consider a tiny debounce/coalesce window for steer delivery — or at least note in the README that bursts are possible.

3. Unbounded growth of dedupe sets. deliveredTerminalRuns and deliveredInputPrompts only ever grow until resetWorkflowLifecycleNotificationState on the next session_start. For a long-lived session orchestrating thousands of stage prompts, this is a small but real leak. Either cap with an LRU, or evict keys whose runs no longer appear in the latest snapshot — the snapshot already gives you the live-run set.

Bugs / correctness

4. truncateSnippet can split a surrogate pair. value.slice(0, LIFECYCLE_NOTICE_SNIPPET_LIMIT - 1) operates on UTF-16 code units. If the cutoff lands inside an emoji or astral character (plausible in user-facing prompt messages), you emit an unpaired surrogate that downstream renderers may show as �. Cheap fix: walk back one code unit if charCodeAt(slice.length - 1) is in the high-surrogate range, or use Array.from(normalized).slice(...).join("") since snippets are short.

5. notifyOn filtering races with config reload. In emitTerminalNoticeOnce the !notifyOn.has(kind) check returns before state.deliveredTerminalRuns.add(key). If a user has notifyOn: ["failed"], completes a run, then reloads config to add "completed", the seed-on-reinstall path saves you (it adds all terminal runs to deliveredTerminalRuns regardless of notifyOn), so behavior is correct today. But it's load-bearing implicit coupling — a future refactor that drops seedExisting: true on reload would resurrect old completion notices. Either add a code comment in emitTerminalNoticeOnce flagging this, or move the notifyOn check after add(key) so the invariant is local.

Cleanup

6. Redundant defensive seed in session_start. Inside the suppression block in index.ts:3402-3421, the explicit seedWorkflowLifecycleNotificationState(store.snapshot()) after restoreOnSessionStart is a no-op in practice — the subscriber installed by reinstallLifecycleNotifications() two lines above already observes every recordRunStart/recordRunEnd during restore under suppression and adds the keys. The comment says "for runtimes without a lifecycle-notification subscriber installed," but in this code path one is always installed. Either delete the call (simpler) or sharpen the comment.

7. Redundant ?. chain in the pi.sendMessage wrapper (index.ts:2014-2016):

   sendMessage: pi.sendMessage
     ? (message, options) => pi.sendMessage?.(message, options)
     : undefined,

   The outer truthy check guards entry; the ?. inside the closure only defends against pi.sendMessage being unset later. If that's intentional, worth a one-line comment. Otherwise drop the ?..

8. Double-cast on the renderer signature (lifecycle-notifications.ts:233):

   (register as unknown as (event: string, renderer: RawRenderer) => void)(...)

   RawRenderer returns CardComponent | string | undefined while pi's MessageRenderer<T> returns Component | undefined. Since pi's Component is structurally { render(width): string[]; ... }, your CardComponent already conforms — drop RawRenderer and type the renderer as MessageRenderer<{ details?: WorkflowLifecycleNoticeDetails }>, removing the cast and tightening the contract.

Tests

9. Coverage is strong. One small gap: no test for the config-reload path that toggles notifyOn (start ["failed"], complete a run, reload to ["completed", "failed"], verify no resurrection). Given point (5), this would lock in the implicit invariant.

10. status-writer.test.ts change (sleep(50) → await writer.flush()) is a good drive-by deflakier — but it's unrelated to the lifecycle-notifications change. Worth a brief mention in the PR description so future archaeologists know why it lives here.

CLAUDE.md compliance

• bun:test + node:assert/strict ✓
• .js import specifiers on TS source ✓
• No build artifacts introduced ✓
• CHANGELOG entries follow the documented format under [Unreleased] ✓
• No any/unknown in new code paths (modulo the as unknown cast in point 8) ✓

Nothing here is a blocker — items 1-3 are design tradeoffs worth a sentence in the PR description, items 4-5 are real bugs but low-likelihood, the rest is cleanup. Nice fix.

[flora131]

Pushed 0984b43 to address the Windows CI timeout that appeared after the feedback fix.

Failure was in test/unit/subagents-foreground-guard-propagation.test.ts, timing out while non-clarifying chain execution still performed eager skill discovery. The patch makes discoverAvailableSkills(...) lazy and only runs it when chain clarification UI is shown.

Validation:

• bun test test/unit/subagents-foreground-guard-propagation.test.ts
• bun test test/unit/workflow-lifecycle-notifications.test.ts
• bun run typecheck
• Commit/push hooks: bun run lint, bun run test:unit

[claude]

Review — feat(workflows): add lifecycle steer notifications

Solid feature, well-architected, and the test coverage is good (24 tests in the dedicated suite plus expanded config-loader coverage). The dedupe model — keying terminal notices by kind:runId and awaiting-input notices by promptId or by awaitingInputSince for promptless pauses — correctly addresses the regressions called out in the PR body. The suppression-context wrapper around restoreOnSessionStart and the resetWorkflowLifecycleNotificationState on session_start are the right primitives for the "reused run IDs in a later session" + "don't replay history into a fresh chat" pair of constraints.

A few notes, mostly minor.

Code quality / correctness

• packages/workflows/src/extension/lifecycle-notifications.ts:255-258 — the JSON-shaped response hint interpolates runId, stageId, and promptId raw into a double-quoted JSON-like fragment that the model is meant to copy/paste. If any of those ids ever contain " or \, the suggestion the model copies is malformed. UUIDs are safe today, but workflowName (interpolated unescaped between double quotes on lines 242 / 248 / 252) is user-supplied and can carry quote chars. Cheap fix: JSON.stringify(details.runId) etc. for the structured part, and at least escape inner quotes in workflowName.

• packages/workflows/src/extension/lifecycle-notifications.ts:233-236 — the register as unknown as (event, RawRenderer) => void cast bypasses the ExtensionAPI.registerMessageRenderer type, which is declared as (payload: unknown) => string but is being handed a renderer that returns a CardComponent. Since pi clearly supports the card return path (other renderers in this package use it), it would be cleaner to widen the type in index.ts:283-286 to string | CardComponent | undefined instead of forcing each call site to cast. Not a blocker, but every as unknown as is a place a future refactor can drift unnoticed.

• packages/workflows/src/extension/lifecycle-notifications.ts:140-159 — emit swallows both sync throws and rejected promises with no logging. The tests assert that sibling subscribers aren't disrupted (good), but for an end-user-facing notification path that can silently stop working, even a console.warn gated on ATOMIC_WORKFLOW_DEBUG === "1" would help diagnose "why didn't my workflow nudge the model?" reports. Same env var is already in use in index.ts:2130.

• packages/workflows/src/extension/lifecycle-notifications.ts:293 — createdAt: prompt?.createdAt ?? stage.awaitingInputSince ?? Date.now() — the Date.now() fallback can't be reached in current store behavior (recordStageAwaitingInput always stamps awaitingInputSince, see store.ts:689), so it's only defending against a future store change. Either drop it or add a brief comment noting it's defensive — right now it reads as if the timestamp might be wall-clock-on-render, which would be a dedupe footgun if the same createdAt value ever fed into a key.

• packages/workflows/src/extension/lifecycle-notifications.ts:69 — module-level rendererRegisteredHosts = new WeakSet<object>() is fine for this package's single-host case, but means there is no path to unregister or reset the renderer in tests that exercise multiple "hosts" with the same identity. The tests sidestep this by using fresh host = {} objects, so it works, but worth a short comment that the dedupe is process-lifetime.

• packages/workflows/src/extension/config-loader.ts:177-197 — no validation that notifyOn is non-empty. notifyOn: [] is silently equivalent to enabled: false and probably indicates user confusion. Either reject empty arrays or document the equivalence — leaning toward the former since enabled: false already expresses that intent.

Wiring / lifecycle

• packages/workflows/src/extension/index.ts:1989-2018, 3377-3389, 3444-3446 — the lifecycleNotificationsActive gate is only flipped on inside the session_start handler. Any lifecycle event between extension load and the first session_start is dropped. That is almost certainly intentional (no chat exists to steer yet), but worth one comment near the flag declaration or in reinstallLifecycleNotifications so a future contributor does not "fix" it by initializing to true.

• packages/workflows/src/extension/index.ts:3416-3419 — the comment correctly notes this seedWorkflowLifecycleNotificationState call is a defensive backstop because the suppressed subscriber already marks restored state as delivered. Good — though if a runtime really had no subscriber installed (pi.sendMessage undefined → install returns early at line 132 of lifecycle-notifications.ts), lifecycleNotificationsUnsubscribe stays null, the state is never primed via the subscriber, and the explicit seed is genuinely load-bearing. Worth mentioning that case in the comment so it is obvious the seed is not dead code.

Performance

• inspect(snapshot) runs on every store update and iterates every run × every stage. All inner checks are Set lookups, so it is bounded by total stages — fine in practice, no concern.
• installWorkflowLifecycleNotifications is re-invoked on each config reload, which re-seeds the snapshot. Since state is reused and seeding is idempotent, this is correct, but two installs racing could in principle install two inspect subscribers if reinstall is called concurrently. reloadWorkflowResources already serializes via workflowReloadQueue, so this is moot — but worth a one-liner pointing at the queue from reinstallLifecycleNotifications for clarity.

Security

Notice content (workflowName, error, promptMessage) is normalized + truncated via truncateSnippet, but never escaped before being embedded in a string steered into the model. Since steer prompts are trusted-but-bounded context, this is acceptable, and the truncation prevents arbitrary content blowing up the context — but it does mean a workflow author who puts adversarial-looking text in a prompt message could land it in the model context exactly. Not changed by this PR (the same content is already model-visible via other paths), but a steer-delivery path is a more attention-grabbing place for it. Worth a doc note in README.md saying lifecycle notices forward workflowName / promptMessage verbatim into the model.

Tests

• Coverage is strong: emit paths for run completed/failed/awaiting (run + stage scopes), dedupe across reinstall, the promptless-after-resolved-prompt case (good — this is the actual bug the dedupe-key fix addresses), suppression seeding via real restoreOnSessionStart, renderer idempotency, notifyOn filtering, steer delivery, and both sync-throw and rejected-promise isolation.
• Missing: no test for the session_shutdown path that flips lifecycleNotificationsActive = false and tears down the subscriber. With the wiring done in index.ts, the subscriber survives across session_start → session_shutdown → session_start cycles, but a regression that forgets to set lifecycleNotificationsActive = false would not be caught here.
• Minor: test/unit/status-writer.test.ts:331 switching await sleep(50) to await writer.flush() is a legitimately good flake fix unrelated to the main feature — worth calling out separately in the changelog if you want the test improvement on record.

Subagents change (packages/subagents/src/runs/foreground/chain-execution.ts:411-412)

Moving discoverAvailableSkills(cwd ?? ctx.cwd) inside the if (shouldClarify) block is correct (the result is only consumed at line 454, inside the same block) and avoids paying for filesystem skill discovery on every chain step when clarification is not shown. Bundled into this PR but unrelated to lifecycle notifications — fine, but it could have gone as a separate PR for cleaner history. Not a blocker.

Summary

The JSON/quote escaping nit (lines 242–258 of lifecycle-notifications.ts) is the only thing I would actually want addressed before merge; the rest are polish suggestions. Nice work on the dedupe model and the suppression/seed split — that is the part of this feature that is easiest to get subtly wrong, and the tests pin it down well.

[flora131]

Addressed the latest code quality feedback in d824f0d.

• Escaped workflow names in lifecycle notice text and JSON-stringified structured response hint IDs.
• Widened ExtensionAPI.registerMessageRenderer to allow component/card results and removed the lifecycle renderer cast (plus updated existing component renderer call sites/tests).
• Added ATOMIC_WORKFLOW_DEBUG=1 warnings for swallowed lifecycle sendMessage sync/rejection failures while preserving subscriber isolation.
• Documented defensive timestamp fallback and process-lifetime renderer dedupe semantics.
• Rejected empty workflowNotifications.notifyOn arrays and documented notifyOn as non-empty.

Validation:

• bun run typecheck
• bun test test/unit/workflow-lifecycle-notifications.test.ts test/unit/config-loader.test.ts test/integration/mock-extension-api.test.ts
• bun run test:unit
• Commit/push hooks: bun run lint, bun run test:unit

[claude]

Review: feat(workflows): add lifecycle steer notifications

Reviewed lifecycle-notifications.ts, the extension/index.ts wiring, config-loader changes, and the test suite. Overall this is a thoughtful, well-tested feature with a sensible architecture (store-observer + dedupe state + suppression context). A few things worth considering before merge.

What looks good

• Failure isolation in emit (packages/workflows/src/extension/lifecycle-notifications.ts:141-161) is exactly right — wrapping send(...) in void Promise.resolve(...).catch(...) plus an outer try/catch handles both synchronous throws and rejected promises, and prevents one notice from aborting sibling store subscribers. The dedicated tests at L508-554 cover both paths.
• withWorkflowLifecycleNotificationsSuppressed is a clean pattern: still observes snapshots (so dedupe state seeds correctly) but skips emission. Combining it with the explicit seedWorkflowLifecycleNotificationState call in session_start as a "defensive backstop" (index.ts:3416-3422) is belt-and-braces in a good way.
• Dedupe-by-awaitingInputSince for promptless pauses after a resolved structured prompt (awaitingInputKey, L339-343) — the regression test at workflow-lifecycle-notifications.test.ts:198-218 clearly demonstrates the fix.
• Renderer registration idempotency via the module-level WeakSet<object> (L70) prevents duplicate registrations across reinstalls.
• Config validation rejecting empty notifyOn arrays (config-loader.ts:191-193) is a nice touch; explicitly listing valid enum values in error messages aids debugging.

Potential issues / questions

1. Prompt injection surface. formatWorkflowLifecycleNoticeText (L239-259) interpolates workflowName, promptMessage, error, stageName, etc. directly into a steer prompt delivered into the model context with triggerTurn: true. escapeQuotedText handles " and \ for the workflow name but not newlines, and the prompt/error snippets are unescaped. Since workflows are typically owner-authored this is probably acceptable, but it is a real prompt-injection vector if any of those strings can be attacker-influenced (e.g. a stage tool that echoes external content into an error or pending-prompt message). Worth a note in the docs at minimum.

2. workflowName is not length-bounded. error and promptMessage go through truncateSnippet (240 chars), but workflowName is interpolated as-is. A pathological name would bloat the steer payload. Trivial fix — run it through truncateSnippet or a dedicated limit.

3. Dedupe sets grow unbounded within a session. deliveredTerminalRuns and deliveredInputPrompts only clear on resetWorkflowLifecycleNotificationState, which is only called on session_start. For long-running sessions running many workflows this is a small memory leak (string keys, but still). Probably fine in practice; flagging in case high-volume sessions are expected.

4. Module-global rendererRegisteredHosts WeakSet. Process-wide registration tracking is correct for production but couples test ordering: any test that supplies the same rendererHost object (or relies on default register-as-host) silently skips re-registration. The test at L556-590 uses a fresh host = {} per test which is fine, but a brief comment noting the per-process semantics would help future contributors.

5. seedExisting ignores notifyOn. Seeding adds every terminal run / awaiting_input prompt to the delivered sets regardless of whether the kind is in notifyOn. If a user changes notifyOn between sessions (e.g. enabling awaiting_input later), prior already-resolved prompts won't re-fire — correct for dedup but might surprise. Probably intentional; worth a sentence in the README to clarify.

6. Unrelated change in packages/subagents/src/runs/foreground/chain-execution.ts:412. Moving discoverAvailableSkills inside if (shouldClarify) is a sensible cost-avoidance refactor, but has no logical connection to the workflow lifecycle notice feature and is not mentioned in the PR description. Cleaner as a separate PR for atomic history/revert.

7. process.env.ATOMIC_WORKFLOW_DEBUG read on every send failure (warnLifecycleSendFailure, L311-315). Hot-path env reads are normally cheap on Node, but caching at module load would be slightly tidier and matches how other debug flags are handled in this repo.

8. makeStageAwaitingInputNotice createdAt precedence. prompt?.createdAt ?? stage.awaitingInputSince ?? Date.now() — when a structured prompt is later resolved and replaced by promptless awaiting-input, prompt is undefined so awaitingInputSince wins. The test at L185-196 confirms this. Worth a one-line code comment because the precedence reads ambiguously at first glance.

Test coverage

Solid. The new test/unit/workflow-lifecycle-notifications.test.ts covers the lifecycle paths, dedupe, suppression, reset, session-boundary reuse, renderer registration, notifyOn filtering, and both sync- and async-error paths. Config validation has its own coverage in config-loader.test.ts. One small gap: no test exercises a workflow that hits all three lifecycle states (awaiting_input → resume → completed) in sequence to confirm a single coherent dedupe trail end-to-end — would be a nice sanity test if it doesn't duplicate existing assertions.

Style / conventions

• Project follows Bun and bun:test conventions throughout.
• All imports use .js extensions (TS ESM convention).
• No dist/ or build-step pollution.
• Minor: lifecycle-notifications.ts has no top-of-file doc comment summarizing the module's contract; given the public API surface (state, suppression, renderer, format) it would help future readers.

Overall: ship-it once items 1 and 6 above get a decision. Nice work on the failure-isolation tests in particular — those are the kind of tests that look paranoid until they save you in production.