fix(server): report session exits when a provider instance is torn down

[rileyjshaw]

What Changed

Provider instance teardown (a providerInstances settings edit that removes or replaces an instance) now reports session.exited for every live session instead of silently killing them.

• CodexSessionRuntime.close ends its events queue (Queue.end) instead of Queue.shutdown. shutdown clears buffered messages, so the session/closed event emitted one line earlier was dropped whenever the adapter's forwarder had not taken it yet.
• CodexAdapter.stopSessionInternal waits for the event forwarder to drain (bounded by a 5s timeout) before closing the session scope, which is what interrupts that forwarder.
• Both CodexAdapter and ClaudeAdapter end their runtime event queue on release instead of shutting it down, so exits already emitted stay readable by ProviderService.
• ProviderInstanceRegistry.reconcile calls adapter.stopAll() before closing a removed/replaced instance's scope. Adapter finalizers stop sessions silently on purpose (server shutdown keeps threads resumable), but a settings edit is not a shutdown.

Tests: runtime-level (real CodexSessionRuntime against the mock peer: events collected after close include session/closed), adapter-level (stopSession forwards the runtime's graceful exit as session.exited), and registry-level (reconcile removal calls stopAll before the scope finalizer).

Why

Observed on 0.0.42: a server.updateSettings call replaced providerInstances while two Codex threads were mid-turn. reconcile closed the old instance's scope, the app-server died, and neither thread ever received a terminal event. Both stayed in running with the "Thinking" indicator indefinitely (the provider event log shows the last event at the moment of teardown and nothing after). Interrupting them later left one thread stuck in starting because the queued turn waited on an abort from a session that no longer existed.

The graceful exit path already exists end to end (session/closed → session.exited → session stopped, active turn cleared); it just never made it out of the runtime's queue. This change makes the existing path deliver instead of adding a new one.

UI Changes

None.

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes
• I included a video for animation/interaction changes

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Active sessions now stop cleanly when a provider is removed or replaced.
  • Session status is updated correctly instead of remaining stuck in a running state.
  • Provider resources are shut down in the correct order during configuration changes.

• Tests

  • Added coverage verifying session shutdown, cleanup ordering, and registry state after reconciliation.

• Style

  • Updated internal test and service formatting to match project conventions.

[macroscopeapp]

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $37.56, which exceeds your per-review limit of $15.00.

The top 3 files driving up this estimate:

File	Diff Size	Estimate
apps/server/src/provider/Layers/ClaudeAdapter.ts	371.95KB	$18.60
apps/server/src/provider/Layers/CodexSessionRuntime.ts	180.22KB	$9.01
apps/server/src/provider/Layers/CodexAdapter.ts	170.69KB	$8.53

Tip

To get this pull request reviewed, you can:

1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
2. Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
3. Raise your cost limit in your workspace billing settings.

^{Turn off this reminder going forward}

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a focused provider-session teardown fix that preserves exit events across registry, Codex, and Claude shutdown paths, with targeted tests. Human review is required because the PR also adds a static-analysis suppression directive in an integration test.

Not approved because:

• Per-review cost limit exceeded (workspace setting). Approvability relies on correctness review in order to determine eligibility

^{Review your spending limits in Billing settings, or comment @macroscope-app review this PR to bypass the limit and review now. 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: 88aa6fa6-6be0-49ad-86f5-0f0042ec7a7b

📥 Commits

Reviewing files that changed from the base of the PR and between cb3d95c and dadbbfc.

📒 Files selected for processing (8)

• apps/server/src/provider/Layers/ClaudeAdapter.ts
• apps/server/src/provider/Layers/CodexAdapter.test.ts
• apps/server/src/provider/Layers/CodexAdapter.ts
• apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts
• apps/server/src/provider/Layers/CodexSessionRuntime.ts
• apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
• apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
• apps/server/src/provider/Services/ProviderInstanceRegistryMutator.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The registry now calls adapter.stopAll() before closing scopes for removed or replaced instances. A teardown test verifies the call order and registry cleanup. Other changes reformat provider tests and implementation files from spaces to tabs.

Changes

Provider reconcile teardown

Layer / File(s)	Summary
Reconcile teardown behavior apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts, apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts, apps/server/src/provider/Services/ProviderInstanceRegistryMutator.ts	Reconciliation stops all sessions before closing removed or replaced instance scopes. The new test verifies ["stopAll", "scope-closed"] order and an empty registry.
Collaboration integration test formatting apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts	The schema, helpers, fixtures, assertions, and tests retain their behavior while changing indentation to tabs.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: reporting session exits when a provider instance is torn down.
Description check	✅ Passed	The description includes the required What Changed, Why, UI Changes, and Checklist sections. It explains the implementation, rationale, tests, and confirms that no UI changes are included.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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