fix(daemon): reject unarmed close --save-script before teardown

Merged
TH
2mo ago
+315-14

Summary

Live evidence on 2026-08-02 showed that a plain open followed by close --save-script silently succeeded and published a script — but with a critical gap: open-time arming (recordSession + scriptPublication{authoring,armed}) is the only point ADR 0016 captures recording-time target-v1 identity evidence from action zero. An unarmed close-time save folded the request into the authoring lifecycle at close time (applyRecordedSaveScriptFlags's none -> authoring branch) and published anyway — a script with selector fallback chains but no target-v1 evidence, and no signal to the caller that evidence capture never ran. Degraded replay verification with no signal is worse than a loud refusal, so the fix rejects instead of warning.

Seam and behavior

assertTerminalRecordingCloseAllowed in src/daemon/handlers/session-close.ts is the existing gate that already rejects close --save-script on an ABORTED/PUBLISHED terminal recording, before any teardown or filesystem work. This PR extends the same gate/seam to the never-armed (scriptPublication.kind === 'none') case:

  • close --save-script on a never-armed session now throws INVALID_ARGS with a hint to start over with open --save-script, before platform close, cleanup, or any .ad write.
  • The rejection does not tear the session down — least-surprising choice: the request is refused, not the close. A plain close retry immediately afterward still completes cleanly (session deleted, no script written). This mirrors how ABORTED/PUBLISHED already behave and lets the caller recover/rearm instead of losing the live session on a mistake.
  • Repair transactions (ADR 0012, a disjoint scriptPublication.kind === 'repair' lifecycle armed by replay --save-script) are explicitly unaffected — the guard returns early for them, exactly as before.
  • session save-script (mid-session active publication, src/daemon/handlers/session-script-publication.ts) is untouched; it already has its own "not armed before this journey began" rejection and remains the sanctioned way to publish without closing.

Aggregate consistency (#1478 P4a)

The P4a arc deliberately folded "never-armed close-time publication" into the authoring lifecycle's shared --save-script ingress (applyRecordedSaveScriptFlags's none -> authoring armed branch in src/daemon/session-script-publication-capability.ts). That branch is now unreachable for close — the new daemon-seam guard rejects before any action recording (and therefore before that ingress) runs for an unarmed close. I updated its docstring to say so rather than leave it describing behavior that no longer happens for close; the branch is kept as a generic safety net for any future non-close caller of the shared ingress, not as documented close-time behavior. ADR 0016 (docs/adr/0016-active-session-script-publication.md) gets two small amendments recording the same decision (recording-lifecycle prose + a validation bullet), same style as its existing #1349 amendment.

Relationship to #1533

#1533 is a different, already-armed case: a session ARMED then ABORTED by a second open, whose flag ingress re-enables recordSession if that second open (or a later action) itself carries --save-script, letting a bare close (no --save-script flag on the close itself) publish anyway. This PR's guard only fires when close itself is passed --save-script — it leaves that bare-close publication path untouched. Related: #1533, deliberately NOT fixed here; noted in the ADR amendment so the two decisions don't collide later.

CLI help

The shared --save-script flag's usage description (src/commands/cli-grammar/flag-definitions-action.ts) implied a standalone close-time save just works; updated to state the arm-at-open requirement and the close --save-script-alone rejection, plus the session save-script mid-session alternative. No test pins the old string.

Tests

  • Unit (src/daemon/handlers/__tests__/session-close-shutdown.test.ts, appended):
    • close --save-script on a never-armed session is rejected before teardown, with no script written — asserts INVALID_ARGS + hint, session survives the rejection, no file written, then a follow-up plain close tears down cleanly.
    • close --save-script on a session with an active .ad repair transaction is unaffected by the unarmed-authoring guard — proves the repair lifecycle's own close path is untouched.
  • Provider-scenario integration (test/integration/provider-scenarios/active-session-script-publication.test.ts, appended):
    • an unarmed session refuses close --save-script and closes cleanly on plain close — full daemon round trip via the real RPC surface.
    • an armed session still publishes target-v1 evidence through close --save-script — proves the legitimate armed close-time path (not just session save-script) still works and still carries target-v1 evidence end to end.
  • Existing suite fixed: test/integration/provider-scenarios/android-lifecycle.test.ts's long composed scenario used a close --save-script on a session that had never been armed (opened many times across the flow, so it couldn't retroactively arm at its original open). Updated only its setup: end that session with a plain close, then open a fresh armed session before exercising close --save-script + --shutdown; all prior assertions preserved. No other provider-scenario or unit suite assumed the old unarmed-close-publishes behavior (repair-armed close --save-script flows all arm via replay --save-script first, unaffected).

Counterfactual (per docs/agents/testing.md)

Reverted just the new unarmed branch locally and reran both new regression tests — both went red exactly as expected, proving they pin real behavior:

Unit:

× close --save-script on a never-armed session is rejected before teardown, with no script written
  → promise resolved "{ ok: true, data: { …(2) } }" instead of rejecting

Provider-scenario:

× an unarmed session refuses close --save-script and closes cleanly on plain close
  → {"jsonrpc":"2.0","id":"...","result":{"data":{"session":"default","message":"Closed: default"}}}
  actual - expected
  + undefined
  - 'INVALID_ARGS'

Restored the fix afterward; both tests pass again.

Gates run

  • pnpm typecheck ✅
  • pnpm lint ✅
  • pnpm format:check ✅
  • pnpm check:layering ✅
  • npx vitest run src/daemon test/integration — 253 files / 1916 tests ✅
  • pnpm check:replay-compat ✅
  • npx vitest run (full) — 13 failures on first pass, all reproduced as CPU-contention timeouts (12) plus one flaky process-liveness assertion (owner-identity.test.ts, unrelated file); isolate-rerun of all 10 affected files together: 151/151 pass. Matches the documented unit-suite-flaky-under-contention pattern; none touch this change's files beyond android-lifecycle.test.ts, which passes standalone.

Test plan

  • pnpm typecheck && pnpm lint && pnpm format:check && pnpm check:layering
  • npx vitest run src/daemon test/integration
  • pnpm check:replay-compat
  • npx vitest run (full, contention flakes isolate-rerun clean)
  • Counterfactual: reverted guard, watched both new tests fail, restored, watched them pass

Generated by Claude Code