fix: make terminal run publication atomic

Author: dennisonbertram

docs/logs/engineering-log.md

@@ -1,5 +1,47 @@
 # Engineering Log
 
+## 2026-07-31 (Terminal Status/Event Atomicity — Issue #1067)
+
+- Symptom: aggregate race load exposed `RunStatusFailed` from `GetRun` while
+  the same run's immediate replay ended at `llm.turn.requested` without
+  `run.failed`; code inspection found the same status-first window on completed
+  and cancelled paths.
+- Cause: every terminal helper called `setStatus` before `emit`, splitting the
+  public run record from the event journal's ledger, bounded store append,
+  subscriber fanout, and recorder drain.
+- Deterministic red: a no-sleep transition barrier reproduced all three states.
+  Completed replay lacked `run.completed`, failed replay contained the required
+  `error.context` but lacked `run.failed`, and cancelled replay lacked
+  `run.cancelled` while `GetRun` already returned each terminal status.
+- Fix: one `transitionTerminal` seam now lets the winning terminal emit seal and
+  append the matching event, completes the bounded store append and ordered
+  recorder dispatch/drain, commits and persists the matching status, then fans
+  out to subscribers. Per-run transition serialization prevents competing
+  terminal helpers from performing mismatched side effects or overwriting the
+  winning status.
+- Preserved reliability: terminal store I/O remains outside `Runner.mu`, and
+  status-store I/O remains outside the per-conversation journal lock; unrelated
+  `GetRun` and unrelated event journals stay responsive; durable-before-fanout, terminal
+  redaction sealing, event IDs, causal/error snapshot order, recorder order,
+  status persistence, backup, and pruning contracts remain intact.
+- Explicit exception: existing terminal `StorageModeNone` configurations still
+  suppress the matching replay event while sealing and publishing status, as
+  pinned by terminal-redaction tests. The stronger replay implication applies
+  to terminal events retained by policy.
+- Regression coverage: completed/failed/cancelled barrier checks; required
+  causal/error ordering; blocked terminal-store target status plus unrelated
+  query availability; 100-iteration competing terminal races; same-conversation
+  terminal-before-later-event subscriber ordering; and HTTP terminal poll
+  followed immediately by Last-Event-ID SSE replay.
+- Verification: focused normal/race stress passed at `-count=100`; complete
+  `internal/harness` + `internal/server` normal/race and affected `go vet`
+  passed; unchanged foreground non-TTY `./scripts/test-regression.sh` passed
+  normal, race, and coverage at 85.6% with zero uncovered functions.
+- Environmental retry evidence: the first coverage attempt hit two real
+  Keychain 15-second kills plus an OpenRouter connection reset. A direct
+  affected coverage run passed, and an unchanged full-gate retry passed without
+  code, test, or command changes.
+
 ## 2026-07-31 (Provider-Key Matrix Health Wait — Issue #1062)
 
 - Symptom: hosted race run `30583930460` failed

docs/logs/long-term-thinking-log.md

@@ -1,5 +1,28 @@
 # Long-Term Thinking Log
 
+## 2026-07-31 (Terminal Status/Event Atomicity — Issue #1067)
+
+- Command intent: repair the aggregate-load Runner race where `GetRun` exposes
+  completed, failed, or cancelled before the matching terminal event and
+  required causal evidence are replayable.
+- User intent: external monitoring must never report a terminal result from an
+  incomplete transcript, and this engine repair must remain isolated from PR
+  #1060/#1055.
+- Success definition: deterministic no-sleep red evidence covers all terminal
+  statuses; one shared lifecycle seam makes terminal ledger/store publication
+  precede status visibility while preserving recorder order; immediate
+  Subscribe and HTTP SSE replay agree; focused stress, affected race/vet, full
+  regression, and hosted checks pass on one unmerged closing PR.
+- Non-goals: conversation cursor redesign, cron/callback behavior, client UI,
+  provider routing, schemas, or workflow timing changes.
+- Guardrails: preserve out-of-lock bounded store writes, durable-before-fanout,
+  terminal sealing, recorder drain order, cleanup order, causal/error snapshots,
+  status persistence, SSE IDs, and unrelated query responsiveness.
+- Outcome: one winner-only transition now commits terminal ledger/store history
+  before matching status and subscriber fanout. Deterministic all-status red,
+  focused normal/race stress, affected normal/race/vet, HTTP reconnect, and the
+  unchanged full regression are green locally; PR and hosted review remain.
+
 ## 2026-07-31 (Provider-Key Matrix Health Wait — Issue #1062)
 
 - Command intent: isolate and clear the hosted race blocker first observed on

docs/logs/observational-log.md

@@ -2,6 +2,24 @@
 
 Use this file for observations about system behavior without immediately prescribing code changes.
 
+## 2026-07-31 (Terminal Run Publication Window)
+
+- Concurrency observation: a terminal event can be prepared under the Runner
+  lock and persisted outside it without blocking unrelated run queries, but the
+  matching status needs one explicit commit point between persistence and
+  subscriber fanout. Status before preparation yields incomplete replay;
+  status after fanout lets terminal-event consumers briefly read `running`.
+- Testing observation: a phase channel at the terminal transition boundary
+  deterministically exposes the forbidden state for completed, failed, and
+  cancelled paths without relying on aggregate load or fixed sleeps.
+- Replay observation: after an HTTP status poll returns terminal, reconnecting
+  run SSE from the first event ID must replay exactly one matching terminal
+  event and no terminal event of another status.
+- Conversation-stream observation: terminal persistence and terminal fanout
+  must share the same per-conversation critical section; releasing it between
+  those phases lets a later run's event overtake the terminal event for an
+  already-connected conversation subscriber.
+
 ## 2026-07-31 (Source-Workflow Dual-Error Arbitration)
 
 - Process observation: a child can exit non-zero while closing its stdin also

docs/logs/system-log.md

@@ -1,5 +1,30 @@
 # System Log
 
+## 2026-07-31 (Terminal Run Transition Publication — Issue #1067)
+
+- System/component: `Runner.transitionTerminal`, `Runner.emit`, and
+  `eventJournal` terminal persistence/fanout in `internal/harness`.
+- Responsibilities: the event journal remains the only event-ledger writer and
+  terminal-seal owner; the transition seam binds the winning terminal event to
+  its completed, failed, or cancelled `Run` record.
+- Order: prior causal/error events -> terminal ledger append/seal -> bounded
+  store append -> ordered terminal recorder dispatch/drain -> matching
+  in-memory and persisted status -> subscriber fanout -> backup/pruning
+  lifecycle.
+- Concurrency boundary: store and recorder I/O remain outside `Runner.mu`.
+  `conversationEventMu` preserves replay-to-live ordering through event-store
+  append, recorder drain, in-memory status commit, and terminal fanout; it is
+  released before status-store I/O. The status commit briefly reacquires only
+  the Runner state lock.
+- Consumers: `GetRun`, run summary, run SSE replay/live delivery,
+  conversation replay, CLI/TUI exit handling, and macOS transcript state keep
+  existing schemas and event IDs.
+- Failure boundary: bounded store errors remain non-fatal and live in-memory
+  replay remains available; terminal redaction drops still seal and publish
+  status by the existing explicit policy; a competing terminal helper is
+  serialized before terminal side effects, loses the seal, and cannot write a
+  mismatched audit/profile outcome or overwrite status.
+
 ## 2026-07-31 (Source-Workflow Terminal Error Arbitration)
 
 - System/component: `internal/workflow.SourceManager.runSourceWorkflow` and its

docs/plans/2026-07-31-issue-1067-terminal-status-event-atomicity-impact-map.md

@@ -0,0 +1,114 @@
+# Cross-Surface Impact Map: Issue #1067 Terminal Publication Atomicity
+
+## Task
+
+- Task / issue: #1067, terminal status visible before matching replay event.
+- Plan: `2026-07-31-issue-1067-terminal-status-event-atomicity-plan.md`.
+- Owner: Codex.
+- Status: implemented and fully verified locally; hosted checks pending.
+
+## Current Ownership, Callers, and Data Flow
+
+- Entry points: `completeRun`, `failRun`, `failRunMaxSteps`,
+  `failRunMaxTurns`, and `cancelledRun`.
+- Source of truth: `Runner` owns `runState.run.Status`, `runState.events`,
+  `runState.terminated`, recorder channels, and run subscribers;
+  `eventJournal` owns append/store/fanout ordering.
+- Callers/consumers: step-engine completion/provider/tool/budget/cancellation
+  paths; `GetRun`; `Subscribe`; run HTTP/SSE routes; CLI, TUI, and macOS
+  transcript/lifecycle consumers.
+- Similar abstractions searched: `rg -n
+  "setStatus|completeRun|failRun|cancelledRun|publishTerminal|GetRun|Subscribe"
+  internal/harness internal/server`. No second terminal lifecycle owner exists.
+- Duplication conclusion: repair the shared Runner transition; do not add
+  provider-, server-, or client-specific compensation.
+
+## Config, API, CLI, and Tools
+
+- Config/env/defaults: none.
+- Endpoints/request/response/wire formats: unchanged `Run`, `Event`, event IDs,
+  SSE names, payload schema, and HTTP routes.
+- CLI/tools/integrations: no command changes; terminal polling and stream
+  consumers gain a stronger ordering guarantee.
+- Error states: unchanged completed/failed/cancelled values and payloads.
+
+## Persistence and Compatibility
+
+- Schemas/migrations/caches/generated data: none.
+- Store order: matching terminal `AppendEvent` remains bounded and precedes
+  recorder dispatch, terminal status `UpdateRun`, and subscriber fanout;
+  status persistence occurs after releasing the per-conversation journal lock.
+- Recorder: terminal JSONL remains queued after all prior events, closed once,
+  and drained before terminal transition returns.
+- Compatibility: additive ordering guarantee only; event/status values and
+  replay IDs remain stable.
+- Mixed-version behavior: process-local; older daemons retain the race until
+  upgraded, with no data migration.
+
+## Lifecycle, Security, and Reliability
+
+- Concurrency: the winning terminal event seals the ledger before status is
+  updated; competing terminal helpers cannot overwrite it with a mismatched
+  status.
+- Cancellation/retries/cleanup: cooperative cancellation and idempotency stay
+  unchanged; workspace/tool/MCP cleanup remains before terminal publication.
+- Locks/resources: terminal store/recorder waits remain outside `Runner.mu`, so
+  unrelated queries are not blocked; status-store I/O also remains outside the
+  per-conversation journal lock, so unrelated event journals are not blocked.
+- Auth/permissions/privacy/secrets: no boundary change after searches through
+  run routes and redaction/audit paths; terminal payload redaction remains
+  owned by the event journal. Explicit terminal `StorageModeNone` remains the
+  documented exception: it seals and publishes status without replaying the
+  intentionally suppressed event.
+- Failure/recovery: bounded store failures remain non-fatal and in-memory replay
+  remains authoritative for the live Runner; persisted-before-prune guards stay
+  intact.
+
+## Product and Integration Surfaces
+
+- Server/runtime: `GetRun` terminal now implies immediate `Subscribe` replay
+  contains the matching event.
+- TUI/web/macOS/other clients: terminal badges, failure text, exit codes, and
+  transcript state no longer disagree during the publication window; no client
+  code changes.
+- Provider/model/tool catalogs/routing: none; provider failure is only a caller.
+- External systems/automation: cron/callback/workflow semantics unchanged.
+- UX/accessibility/focus/motion: no visual or interaction change.
+
+## Deployment and Operations
+
+- Deployment/migrations/flags: ordinary daemon rollout; no migration or flag.
+- Observability: deterministic regression records transition phases without
+  logging prompts, event payload secrets, or credentials.
+- Rollback: revert if terminal fanout deadlocks, unrelated `GetRun` blocks,
+  cleanup order changes, recorder output truncates, or cancellation regresses.
+- Runbooks/operator docs: no public/operator command changes.
+
+## Regression Tests
+
+- First red: phase barrier proves old completed/failed/cancelled status can win
+  before terminal history.
+- Acceptance: terminal status implies one matching replay event; failed causal
+  snapshot precedes `run.failed`; competing terminal transitions match the
+  winning sealed event; later same-conversation events cannot overtake terminal
+  fanout to an existing conversation subscriber.
+- Store/recorder: durability and existing drain barriers preserve non-terminal
+  status without blocking unrelated queries; a blocked status-store write does
+  not block unrelated event journals.
+- Integration: HTTP poll immediately followed by run SSE replay for all three
+  statuses.
+- Exact gates: focused normal/race stress `-count=100`; harness/server
+  normal/race/vet; unchanged foreground non-TTY regression; hosted checks.
+
+## Documentation and Handoff
+
+- Plans/specs: issue-specific plan and this map.
+- Logs/indexes: engineering, observational, system, long-term, plans index, and
+  active plan.
+- Public/training/release docs: none because no new route, schema, or command is
+  introduced.
+
+## Warning Check
+
+- Every cross-surface heading is resolved. Unaffected surfaces are explicitly
+  named with search and data-flow rationale above.

docs/plans/2026-07-31-issue-1067-terminal-status-event-atomicity-plan.md

@@ -0,0 +1,119 @@
+# Plan: Make Terminal Status and Event Publication Atomic
+
+## Context
+
+- Governing GitHub issue: #1067.
+- Problem: terminal helpers publish `Run.Status` before the event journal has
+  prepared, persisted, and exposed the matching terminal event. A concurrent
+  `GetRun` can therefore return completed, failed, or cancelled while an
+  immediate `Subscribe` replay still ends at a non-terminal event.
+- User impact: API polling, SSE reconnect, CLI/TUI, and macOS clients can
+  briefly render terminal state without the authoritative terminal transcript
+  or its preceding causal/error evidence.
+- Constraints: preserve durable-before-fanout, terminal sealing, recorder
+  drain ordering, bounded store writes, run-independent query availability,
+  current event/status schemas, cleanup ordering, and the explicit
+  `StorageModeNone` terminal-redaction policy.
+
+## Scope
+
+- In scope: one shared Runner terminal-transition seam for completed, failed,
+  cancelled, max-step failed, and max-turn failed paths; deterministic
+  concurrency and replay regressions; real HTTP poll-then-replay proof.
+- Out of scope: PR #1060/#1055 changes, cron/callback behavior, conversation
+  cursor redesign, GUI visual changes, provider routing, schemas, and workflow
+  timing issue #1049.
+
+## Documentation Contract
+
+- Feature status: implemented and fully verified locally; hosted checks pending.
+- Public docs affected: none; existing terminal event/status wire formats stay
+  unchanged.
+- Spec docs before code: this plan and its linked impact map.
+- Implementation notes after code: engineering, observational, system, and
+  long-term logs plus the plans index and active plan.
+
+## Test Plan (TDD)
+
+- First red: a deterministic phase barrier pauses each completed, failed, and
+  cancelled helper after the old status write but before terminal event
+  publication. Concurrent `GetRun` plus `Subscribe` must never observe that
+  forbidden state.
+- Causal control: on an error-chain-enabled failure, the required
+  `error.context` snapshot must precede `run.failed` before failed status is
+  observable.
+- Store/recorder controls: block terminal store append and preserve the existing
+  recorder drain regressions; assert target status remains non-terminal while
+  unrelated run queries remain responsive, then becomes terminal only after
+  replay, durability, and recorder delivery are ready. Block status-store
+  persistence separately and prove unrelated event journals remain responsive.
+- Concurrency control: race competing terminal transitions and require the
+  winning status to match the single sealed terminal event; hold a terminal at
+  the pre-fanout boundary and prove a later same-conversation event cannot
+  overtake it for an existing conversation subscriber.
+- Real path: HTTP `GET /v1/runs/{id}` followed immediately by run-event SSE
+  replay contains the matching terminal event for all three statuses.
+- Focused stress: normal and race at `-count=100`.
+- Affected gates: `internal/harness` and `internal/server` normal/race and vet.
+- Repository gate: unchanged foreground non-TTY
+  `./scripts/test-regression.sh`.
+- Hosted gates: required PR checks, including `test-fast` and `test-race`.
+
+## Cross-Surface Impact Map
+
+- See `2026-07-31-issue-1067-terminal-status-event-atomicity-impact-map.md`.
+
+## Implementation Checklist
+
+- [x] Link contract-complete bug #1067 before implementation.
+- [x] Record current ownership, callers, sources of truth, and search evidence.
+- [x] Write this plan and impact map before code.
+- [x] Add and confirm the deterministic failing regressions.
+- [x] Implement the smallest shared terminal-transition repair.
+- [x] Confirm focused stress, affected normal/race/vet, and repository gates.
+- [x] Prove the HTTP poll-then-replay path.
+- [x] Update all required logs and documentation status.
+- [ ] Open one closing PR, push its exact head, and request `@codex` review.
+- [ ] Confirm hosted checks are green; do not merge.
+
+## Risks and Mitigations
+
+- Risk: holding `Runner.mu` across persistence would block unrelated queries.
+- Mitigation: retain the existing out-of-lock bounded terminal append/fanout
+  path and test unrelated `GetRun` responsiveness while it is blocked.
+- Risk: concurrent terminal helpers could publish one event and a different
+  status.
+- Mitigation: serialize each run's complete terminal helper lifecycle, make the
+  shared transition return whether it won terminal sealing, and update status
+  only for that winner.
+- Risk: moving status later could reorder cleanup, causal snapshots, audit,
+  profile persistence, backup, or pruning.
+- Mitigation: pin causal/event order and keep cleanup before terminal
+  transition, operational side effects after the matching status/event pair.
+- Risk: recorder or store failure could weaken the existing lifecycle.
+- Mitigation: preserve bounded non-fatal persistence semantics, recorder drain,
+  terminal retention, and terminal-event-persisted pruning guards.
+- Risk: an explicit terminal `StorageModeNone` policy intentionally removes the
+  matching event from replay.
+- Mitigation: preserve and test the existing redaction exception and scope the
+  stronger replay implication to terminal events retained by policy.
+
+## Verification Evidence
+
+- Semantic red:
+  `go test ./internal/harness -run
+  '^TestTerminalStatusNeverPrecedesTerminalReplayEvent$' -count=1` failed all
+  three cases: completed before `run.completed`, failed after `error.context`
+  but before `run.failed`, and cancelled before `run.cancelled`.
+- Focused final green: terminal atomicity, competing transitions, blocked store,
+  and HTTP replay passed normal and race at `-count=100` across
+  `internal/harness` and `internal/server`.
+- Affected packages: complete harness/server normal and race passed; `go vet`
+  passed.
+- Real path: HTTP terminal polling followed immediately by Last-Event-ID run
+  SSE replay passed for completed, failed, and cancelled.
+- Repository: unchanged foreground non-TTY `./scripts/test-regression.sh`
+  passed normal, race, and coverage
+  (`total=85.6%`, `zero-functions=0`). An earlier coverage attempt hit transient
+  real-Keychain timeouts plus an OpenRouter connection reset; the unchanged
+  retry passed completely without code or command changes.