fix(daemon): keep an active replay session's daemon alive over the CLI path

[thymikee]

Summary

• A replay <script>.ad with no terminal close reports its session as still active per ADR 0016's consumption contract ("the named session stays active and the successful ReplayCommandResult returns its session id"), but over the real CLI/IPC path the daemon that ran it is torn down (and its owned ephemeral state dir deleted) regardless — the session vanishes within ~120ms of the successful response, with no close/teardown event logged.
• Root cause: cleanupDaemonAfterRequest (src/daemon/client/daemon-client-lifecycle.ts) unconditionally stops the daemon after every one-shot replay/test request whenever the client itself started it (daemon.startedByClient), with no check for whether the response reported the session as still active. This fires regardless of whether the state dir was randomly generated (the common no-flag case) or passed explicitly via --state-dir/AGENT_DEVICE_STATE_DIR — matching the issue's literal repro, which used an explicit state dir.
• The in-process provider-scenario test harness (active-session-script-publication.test.ts) never exercises this client/IPC layer at all, which is why it passed while the live CLI path broke.

This addresses the "session vanishes after a close-less replay" symptom of #1384 specifically. #1384 also recorded two other, unverified symptoms (a lingering DEVICE_IN_USE claim right after close, and a one-off published-artifact rewrite under interleaved stale-session closes) — those are split out to #1391 rather than riding along to auto-close here.

Fix

Mirrors the existing ADR-0012 repair-divergence keep-alive pattern (same category of problem, same daemon-client layer, same shape of fix):

• ReplayCommandResult (src/contracts/replay.ts) gains sessionActive: boolean, computed from whether the session still exists in the daemon's own store after the run — never by re-parsing the script for close — set in both success-response builders (session-replay-runtime.ts, session-replay-maestro-response.ts).
• daemon-client-lifecycle.ts adds isActiveReplaySessionResponse(req, response) and ORs it into cleanupDaemonAfterRequest's early-bailout, so a still-active replay session's daemon/state-dir is never stopped/removed. Restricted to replay by explicit command check — test always tears down as before, since its own per-file runner already closes each session before the suite summary is built.
• daemon-client.ts attaches a --state-dir <dir> --session <name> address hint to a kept-alive owned-ephemeral-daemon's response (structured hint field + appended to message, since message is the only field the default text renderer surfaces). The session name is the response's session value used verbatim — that's already the fully-qualified, cwd-scoped store key, and an explicit --session <value> is used as-is by resolveEffectiveSessionName (skipping cwd-scoping), so a bare --session default would only resolve by coincidence from the same cwd.
• A completed (non-diverging) --save-script repair also falls under this same guard, since its terminal source close is always skipped by design (ADR 0012 Fix 3) — the healed-script commit (gated on teardown, not on the response) is now deferred to an explicit close/idle-reap instead of firing immediately after the response. Documented as a consequence in ADR 0012, alongside the plain-replay consequence already documented in ADR 0016.
• Documented in ADR 0016 and replay's CLI help: an unattended close-less replay (e.g. in CI) now leaks a live daemon+session exactly like an interactively opened one — bounded by ordinary idle-reap or an explicit close, not by the one-shot command's process lifetime. This is the intended shape of "the caller owns close," not an accident.

Test plan

• Regression tests in src/utils/__tests__/daemon-client-lifecycle.test.ts at the exact layer the bug evaded (real client/IPC lifecycle, not the in-process harness): active session kept alive + hinted (owned ephemeral dir), closed session still torn down, the issue's literal explicit---state-dir repro no longer stops the daemon, test still tears down unconditionally even if its response hypothetically carried sessionActive, a completed --save-script repair keeps its daemon alive under the same guard, and a follow-up sendToDaemon using the hinted --state-dir/--session reaches the same kept-alive daemon without spawning a new one.
• Verified the key new tests fail without the fix (reverted source, re-ran) and pass with it restored.
• active-session-script-publication.test.ts, replay-repair-abort-close.test.ts, replay-repair-record-exclusion.test.ts, session-replay-repair-transaction.test.ts all still pass.
• tsc --noEmit, oxlint, and oxfmt --check all clean.

Addresses #1384

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.82 MB	1.82 MB	+951 B
JS gzip	582.3 kB	582.7 kB	+356 B
npm tarball	696.5 kB	697.0 kB	+511 B
npm unpacked	2.44 MB	2.44 MB	+1.3 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.3 ms	28.6 ms	+0.3 ms
CLI --help	58.4 ms	58.4 ms	-0.0 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/agent-device-client.js	+622 B	+200 B
dist/src/registry.js	+162 B	+86 B
dist/src/server.js	+110 B	+52 B
dist/src/session.js	+57 B	+16 B
dist/src/internal/daemon.js	0 B	+1 B

[thymikee]

Reviewed (I filed #1384 and proposed this exact direction there, so mostly checking the execution — which is faithful: keyed off the session surviving in the daemon's own store rather than script parsing, explicit test carve-out proven by a test rather than left to response shape, hint only for owned ephemeral dirs, regression tests at the precise client/IPC layer the provider harness can't see, and the CI-leak consequence written into ADR 0016 as a decision). Findings, one substantive:

1. Side effect worth an explicit decision: a completed repair-armed replay's heal commit is now deferred. A replay --save-script that completes cleanly in one shot skips its terminal source close (ADR 0012 C4), so the session survives → sessionActive: true → this guard now keeps the daemon alive — where previously the one-shot client teardown fired immediately and, per the commit state machine, committed the healed .ad right then. After this PR the <stem>.healed.ad lands only when the agent closes or idle-reap tears down (bounded: an uncommitted transaction doesn't block daemon idle-reap per C5a, and reap-at-COMPLETE commits — so worst case is the idle window, default ~5min). I think this is actually more aligned with R7 ("commit at teardown"; the session stays inspectable post-repair), but it changes observable timing for any scripted caller that runs replay --save-script and immediately checks for the healed sibling. Suggest: one sentence in the ADR 0012 repair section acknowledging the deferred commit, and ideally a client-lifecycle test pinning that a sessionActive: true response on a repair-armed replay keeps the daemon (it falls out of the same guard, but today only the divergence-path keep-alive is pinned).

2. "Fixes #1384" closes three symptoms; this PR fixes one. The issue also records (a) the lingering DEVICE_IN_USE claim right after close (probably shrinks with fewer teardowns, but the claim-release gap itself isn't touched) and (b) the one-off published-artifact rewrite under interleaved stale-session closes. If those aren't verified fixed by this change, worth splitting them to their own issue before the auto-close eats them.

3. Hint completeness (minor): the hint says pass --state-dir <dir> on your next command — the caller also needs the session addressing to land on it (--session default, and session names are cwd-scoped on the CLI, so a follow-up from a different cwd won't resolve the same public name). Including the full continuation (--state-dir <dir> --session <name>) in the hint text would make it copy-pasteable.

4. Test gap (minor): nothing proves the kept daemon is actually addressable — a follow-up sendToDaemon with the hinted --state-dir reaching the same fixture would close the loop on the promise the hint makes.

One corroborating live observation from the #1384 repro that supports the guard's placement (before any stop call, rather than inside the stop path): with an explicit state dir, the daemon process survived the cleanup while its sessions died — i.e. the graceful-stop path tears sessions down first and the process can outlive it when a new request lands mid-shutdown. Guarding before the stop, as done here, sidesteps that whole ordering; no action needed.

🤖 Addressed by Claude Code

[thymikee]

Reviewed exact head a7f2e92. Not ready: (1) ADR 0016 still says a close-less replay does not change successful response shape, but this PR adds required public ReplayCommandResult.sessionActive without amending that clause; (2) the MCP replay output schema omits required sessionActive; (3) all new lifecycle tests inject sessionActive through a fake HTTP response, so deleting both real daemon producer lines reintroduces #1384 while the tests still pass—add real native and Maestro replay-route assertions for true on close-less and false after terminal close; (4) the ADR-required live iOS and Android CLI/device evidence is absent; and (5) Lint & Format fails on daemon-client-lifecycle.test.ts. The cleanup guard itself is directionally correct, but these contract, regression, evidence, and CI blockers must be resolved. No ready label.

[thymikee]

Thanks for the thorough review — addressed in 07c1fdd (plus a7f2e92 fixed the CI format-check failure separately):

1. Deferred repair-commit timing. Confirmed: a completed --save-script repair always skips its terminal source close (Fix 3), so sessionActive is now always true for it and it falls under the same keep-alive guard as the plain case — the healed .ad commit (gated on teardown, not the response) is now deferred to an explicit close/idle-reap instead of firing immediately. Added a paragraph in ADR 0012 acknowledging this, and a dedicated test (ADR 0012 R7 x ADR 0016: a completed --save-script repair also keeps its owning daemon alive...) pinning it separately from the plain-replay case.

2. Scope of "Fixes #1384". Filed #1391 for the two unverified symptoms (lingering DEVICE_IN_USE, the published-artifact rewrite under interleaved closes) and reworded the PR to "Addresses #1384" so merging this doesn't auto-close it out from under those.

3. Hint completeness. Went further than --session default: the hint now includes --session <name> using the response's session field verbatim. Traced resolveEffectiveSessionName/hasExplicitSessionFlag (session-routing.ts) — an explicit --session <value> is used as-is, bypassing cwd-scoping entirely, so the response's already-qualified session name (e.g. cwd:<hash>:default) is what actually reaches the same session from any cwd. A literal --session default in the hint would only work by coincidence (an implicit follow-up run from the identical cwd), so I used the verbatim value instead — noted the reasoning in the code comment.

4. Addressability test. Added closes the loop: a follow-up sendToDaemon using the hinted --state-dir/--session reaches the SAME kept-alive daemon, without spawning a new one — parses the hint, reissues a request against it, and asserts no new daemon was spawned and the request carried the hinted session.

Also re-ran the full affected test set (daemon-client-lifecycle, session-replay-repair-transaction, and the three provider-scenario repair/publication tests) plus tsc/oxlint/oxfmt --check — all clean.

[thymikee]

Addressed in 07c1fdd (already pushed) + c1bf116 (just pushed), plus live device validation:

1. ADR 0016 contradicted the new response shape. Fixed: amended the "on consumption" paragraph to document sessionActive: boolean as part of the contract, and why the real CLI/IPC client needs it as an explicit signal (a fake-HTTP-only signal can't reach a genuinely separate daemon process).

2. MCP replay output schema omitted required sessionActive. Fixed in src/mcp/command-output-schemas.ts — added it to the replay schema's properties and required array.

3. Tests only injected sessionActive via a fake HTTP response. Confirmed the concern was valid — added tests against the REAL producer (runReplayScriptFile, not a client-layer fixture): a close-less .ad replay → true, a replay whose script's terminal close actually removes the session from the store → false, and a close-less Maestro YAML replay → true. Also strengthened the provider-scenario (real daemon route, real Android world) test with the same assertion. Verified each new test fails when I revert the corresponding producer line (session-replay-runtime.ts's sessionActive: completedSession !== undefined and session-replay-maestro-response.ts's equivalent) — confirmed, then restored.

4. Live iOS/Android evidence. Built the CLI and ran issue #1384's exact repro end-to-end against a real booted iOS 16 simulator and a running Android Pixel 9 Pro XL emulator:

replay /tmp/open-only.ad --json   # sessionActive: true, daemon/session survive
session list --json               # see below
snapshot --json --session "cwd:<hash>:default"   # succeeds — session is genuinely alive

Confirmed on both platforms: the daemon process and the session both survive the close-less replay, and the session is fully addressable via the hinted --state-dir/--session. This live run also surfaced a separate, pre-existing bug: plain session list (no explicit --session) shows {sessions: []} even though the session is alive — traced to session-open.ts's resolveImplicitSessionScope(req) seeing a different (cwd-less) req when open is dispatched internally by replay vs. a top-level CLI request, leaving session.sessionScope unset and failing session_list's scope-match filter. The session itself is never lost (an explicit --session <name> reaches it fine on both platforms) — this is a sessionScope-propagation gap unrelated to the client-side teardown this PR fixes. Filed as #1394 rather than folding it in here.

5. Lint & Format failure. Already fixed at head a7f2e921's next commit (07c1fdd8f, pushed before this comment landed) — confirmed green in the CI run since.

Re-ran the full affected suite (daemon-client-lifecycle, session-replay-runtime, session-replay-repair-transaction, mcp command-tools, the three provider-scenario repair/publication tests — 95 tests) plus tsc/oxlint/oxfmt --check/fallow audit, all clean.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head c1bf116f. Not ready.

P2 — explicit --state-dir replay still omits the usable session address in normal CLI output. withActiveSessionAddressHintIfOwned suppresses the hint for an explicit state dir, while default text renders only data.message, not data.session. Production replay session IDs are cwd-qualified; after changing cwd the caller cannot recover that ID, and #1394 means session list cannot discover it either. Emit at least --session <data.session> for active replays at explicit state dirs, and test a non-JSON follow-up with a qualified name.

P2 — quote the generated address command. attachActiveSessionAddressHint interpolates both stateDir and sessionName raw despite describing the hint as copy-pasteable. Explicit session names and state-dir paths may contain spaces or shell metacharacters, and the test parser’s \S+ pattern only covers safe tokens. Use the shared shell-quoting helper and add unsafe-value coverage.

The prior ADR/MCP/producer/lifecycle findings are otherwise resolved, live iOS/Android evidence is present, and all current checks are green.

[thymikee]

Both P2s addressed in ef32057:

P2-1: explicit --state-dir replay omitted the usable session address. Fixed — withActiveSessionAddressHintIfOwned is renamed to withActiveSessionAddressHint and no longer gates on settings.ownedStateDir. Every active-replay response now gets a --session <name> hint (in both the structured hint field and appended to message, so it shows in default text mode too); --state-dir is only added on top when the state dir is the client's own randomly-generated one the caller has no other way to learn. Added a test for the explicit-state-dir case pinning the exact hint text (--session only, no --state-dir).

P2-2: quote the generated address command. Fixed — attachActiveSessionAddressHint now runs both the state dir and session name through shellQuoteIfNeeded (the same helper session-recovery-hints.ts/request-lock-policy.ts already use), so the hint stays copy-pasteable even with spaces or shell metacharacters. Added two tests: one with an unsafe state dir + session name together, one for the --session-only (explicit state dir) case with an unsafe session name — both assert the exact quoted output and confirm quoting was actually exercised (not coincidentally identical to the raw value). Verified both fail against a raw-interpolation reversion, then restored the fix.

Re-ran the full affected suite (daemon-client-lifecycle now 22 tests, plus session-replay-runtime, session-replay-repair-transaction, mcp command-tools, the three provider-scenario tests — 97 total) plus tsc/oxlint/oxfmt --check/fallow audit, all clean.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head ef320575. Clean and ready for human review. Active replay responses now surface the cwd-qualified --session in normal output for explicit state dirs, owned dirs include both address flags, and both values use the shared shell-quoting helper with unsafe-value coverage. The hint is attached before text rendering, current checks are green, and prior live iOS/Android evidence remains applicable.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-24 18:10 UTC