fix(server): finalize runs when provider streams stop

[Adamulek123]

A provider stream can end without an accepted root terminal, leaving a run active and partial assistant text or plans still streaming. Startup failure can also race event ingestion. This change finalizes those paths once, closes artifacts belonging to the ending attempt and preserves successor artifacts and longer projected content.

Finalization retains main's transactional event/effect writes and current-attempt guard. Synthetic failures can bypass a failed ownership-read predicate while the transaction still checks attempt identity. Normal terminal races and Stop retain the same guard. Checkpoint context lookup failures and missing checkpoint scopes are surfaced instead of silently skipping workspace refresh.

The latest review reproduced a regression in this PR's clean-drain handling: normal managed server shutdown could persist a failed run before recovery, causing a prepared restart continuation to be rejected. Managed event subscriptions now expose an optional local shutdown hint. Run execution reads it only when a stream closes successfully without an accepted root terminal and leaves settlement to runtime recovery. Unexpected stream loss, startup failure, manual provider release and accepted terminals keep their handling. This adds no client wire field or per-event work.

The regression uses the actual session manager, Orchestrator, effect worker and in-memory SQLite. Before the correction it observed a failed source run. Afterward, recovery cancels the source and a fresh manager and worker over the same database deliver the prepared continuation and persist a running successor provider turn. Four controls cover unexpected drain, manual release, provider-announced Stop without a terminal and normal completion followed by shutdown. Tests await persisted events or Deferred milestones without sleeps.

Validation on main d81afa0a6f403bd6e5562ffcee2484089a1fdc3f and head 0a115626b43ffba815df144d3066254ea9595e35:

• Six complete focused suites: 215 tests passed, 26 platform cases skipped. These cover execution, finalization, SQLite persistence, steering integration, session management and external-launcher compatibility.
• Independent reviewer: 23 targeted tests passed. Parent independently ran eight execution/lifecycle cases, including shutdown with fresh restart and genuine stream loss.
• Type-aware lint and type checking pass across all 11 changed TypeScript files. Formatting and whitespace checks pass. Direct server and provider-core TypeScript compilers exit zero; existing nonblocking Effect suggestions remain.

Replay remains a validation limit. A filtered replay attempt recorded a Pi thread_rollback_after_stop fixture failure with the first run still starting, plus Windows EBUSY cleanup output, and was interrupted before a completed aggregate receipt. The parent isolated that Pi fixture in a worktree whose relevant runtime, Pi adapter and replay sources match pinned main; it failed with the same starting-run error. That worktree differs from main only in the unrelated OpenCode adapter and its tests. No replay success count is inferred, the full replay matrix was not run, and timeouts, skips and cleanup were not changed to hide failures.

Current main's startup and interrupted-attach cleanup, idle unload, package moves and transactional finalization are preserved. Former external-launcher fixture helper corrections are now absorbed by main and no longer part of the diff. This PR does not add the separate attempt-ownership redesign or request-retirement changes. Shared server state serves web, desktop and mobile; no live client/provider verification or performance benchmark was run. Maintainer issue triage remains outstanding. CI must validate this new head.

Implemented and independently reviewed with GPT-6.1 Sol through the Codex harness.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR substantially changes production orchestration behavior across provider-stream termination, startup failures, steering, shutdown recovery, artifact persistence, and checkpoint finalization. Its transactional guards and concurrency changes span multiple server components, so the runtime lifecycle impact is broader than a routine bug fix.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 28ae21fd-a4ac-4b4b-85a1-4c2459cf0ed2

📥 Commits

Reviewing files that changed from the base of the PR and between efecd3c and 7a4666e.

📒 Files selected for processing (9)

• apps/server/src/orchestration-v2/EventSink.ts
• apps/server/src/orchestration-v2/FoundationPersistence.test.ts
• apps/server/src/orchestration-v2/RunExecutionService.test.ts
• apps/server/src/orchestration-v2/RunExecutionService.ts
• apps/server/src/orchestration-v2/RunFinalizationService.test.ts
• apps/server/src/orchestration-v2/RunFinalizationService.ts
• apps/server/src/orchestration-v2/SteeringCompletion.integration.test.ts
• apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts
• docs/user/composer.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Run finalization now tracks and closes open artifacts, guards writes by root and run ownership, and refreshes only after successful finalization. The changes also add checkpoint-context errors and tests for restarted attempt identity and stopped-turn text.

Changes

Run lifecycle and artifact state

Layer / File(s)	Summary
Guarded artifact persistence apps/server/src/orchestration-v2/EventSink.ts, apps/server/src/orchestration-v2/FoundationPersistence.test.ts	Root-guarded writes filter node, message, and turn-item updates against current projections and provider-turn provenance. Run-current writes enqueue optional effects and notify outbox waiters only after a successful commit. Persistence tests cover ownership checks and artifact cleanup cases.
Root-run artifact finalization apps/server/src/orchestration-v2/RunExecutionService.ts, apps/server/src/orchestration-v2/RunExecutionService.test.ts	Finalization tracks open root artifacts and writes terminal updates on completion and failure paths. It serializes finalization and refreshes only when finalization succeeds. Tests cover stream closure, interruption, ownership conflicts, and provider-start races.
Checkpoint-context finalization errors apps/server/src/orchestration-v2/RunFinalizationService.ts, apps/server/src/orchestration-v2/RunFinalizationService.test.ts	Finalization reports checkpoint-context read failures and missing checkpoint scope as distinct errors. Tests verify their causes and that neither path refreshes the workspace.
Replacement-attempt identity and interruption guidance apps/server/src/orchestration-v2/SteeringCompletion.integration.test.ts, apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts, docs/user/composer.md	Integration and replay tests check root identity snapshots and stale predecessor events during replacement attempts. Composer documentation describes partial assistant text and streaming indicators after a turn is stopped or steered.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~55 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ProviderStream
  participant RunExecutionServiceV2
  participant EventSinkV2
  participant WorkspaceRefreshObserver
  ProviderStream->>RunExecutionServiceV2: Send terminal events or close
  RunExecutionServiceV2->>EventSinkV2: Write guarded run and artifact events
  EventSinkV2-->>RunExecutionServiceV2: Return commit result
  RunExecutionServiceV2->>WorkspaceRefreshObserver: Refresh after committed finalization

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 7a466

This change ensures runs end when a provider stream closes without a final result. It closes only the leftover streaming artifacts that belong to the ending attempt and reports checkpoint lookup problems explicitly. No concrete defect was identified, and the change appears ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 7a466

The change strengthens cleanup and recovery, but newly added stream-close handling can finalize an old attempt after a restart takes ownership. This can overwrite the replacement run’s state. The supported concern is execution ownership and failure containment; no new permission gain or cross-tenant attack path was established.

Retained concerns

• Medium · reliability · inferred: The new clean-stream-drain finalization path checks ownership outside the commit transaction and then defaults to writeWithEffects without a current-attempt guard. If steering admits a replacement after that read, the predecessor can persist its failed run snapshot over the replacement’s activeAttemptId, root, and status. Newly collected artifact snapshots also enter this unguarded finalization batch. The semaphore coordinates only finalizations within one attempt. This extends a pre-existing normal-terminal ownership race to a newly writing recovery path, weakening successor isolation and failure containment.

Security review details

Security Blast Radius

• inferred — The supported race affects a conversation’s run, attempt, root, artifact, and provider-thread projection state. It requires overlapping provider finalization and an authorized restart. The inspected path does not establish arbitrary tenant selection, credential access, or additional tool authority.

Security Findings and Attack Paths

• inferred — Provider-controlled stream timing can reach the new synthetic-failure path. A successful ownership read followed by replacement admission permits stale persistence because this path does not repeat the identity check within the write transaction. This is an ownership-integrity concern, not a verified authentication bypass or privilege-escalation finding.

Trust Boundaries and Controls

• observed — Current-run writes enforce thread, run, expected status, and active-attempt identity transactionally. Root-guarded fallback cleanup additionally protects successor-owned and already-settled artifacts. Persistence tests exercise successor provenance, retargeted records, and preservation of longer projected text, providing counterevidence for those guarded paths.

Resilience and Maintainability Implications

• observed — Checkpoint finalization now fails explicitly on context-read or missing-scope errors before workspace refresh. Existing checkpoint capture avoids recapturing an already checkpointed terminal run, while the worker retains durable retries with backoff and a terminal failure path. These mechanisms support recovery after capture succeeds but a later finalization stage fails.

Hardening Proposals

• proposed — Apply in-transaction current-attempt validation to every terminal and stream-drain finalization commit. If ownership is lost, retain only provenance-reconciled cleanup without run mutations or effects. Validate the transition where replacement admission occurs after a successful ownership read but before persistence.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 8 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check		The description thoroughly explains the problem, implementation, verification, limitations, and affected behavior. However, required scope approval or a linked triaged issue is still outstanding.	Add the triaged bug issue or maintainer approval with the approval comment. If no prior issue is required, explain why this focused fix qualifies for the exemption.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly identifies the primary change: finalizing runs when provider streams stop.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 8 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}