perf(server): shutdown no longer rewrites every stopped session row

[t3dotgg]

Problem

ProviderService.stopAll runs as a finalizer on every quit, update, and restart. It read the whole provider_session_runtime table twice. Then it rewrote every row, one at a time. Each rewrite is a SELECT plus an UPSERT. This included rows that were already stopped. Stopped rows stay on purpose because they hold resume cursors, so long-lived installs have many of them.

A report from one user with about 5,000 threads showed 3,129 stopped sessions, which suggests about 6.3k SQL statements on each shutdown. Those counts came from projection_thread_sessions, not provider_session_runtime, so the exact numbers are not proven. The direction is right: every stopped runtime row was rewritten on every shutdown. The shutdown also bumped lastSeenAt on every row, and sessionCount in provider.sessions.stopped_all counted every row in the table.

Fix

In apps/server/src/provider/Layers/ProviderService.ts:

• stopAll reads the table once. The listThreadIds read was only used for the analytics count.
• New isSettledBinding: status is stopped and the payload has no activeTurnId. stopAll skips these rows. A stopped row that still has a stale activeTurnId is rewritten, so the turn is cleared as before.
• provider.sessions.stopped_all now sends stoppedSessionCount: the rows this shutdown rewrites. It replaces sessionCount, which counted every row. The new name keeps the old and new meanings in separate series, the same way surface was added next to the legacy clientType.

Recovery on next boot does not change. Live sessions are persisted and rewritten as before, so their continuation markers and resume cursors carry over. Skipped rows are not written at all.

Verification

New test: "ProviderServiceLive shutdown leaves settled session rows untouched". It seeds a settled stopped row, a running row with a turn, and a stopped row with a stale turn. Then it closes the service scope. The settled row does not change at all (including lastSeenAt and payload). The other two rows are stopped with activeTurnId null, and stoppedSessionCount is 2.

Mutation checks: the test fails if the filter is removed, and it fails if stopped rows with a stale turn are skipped.

• vp test run apps/server/src/provider/Layers/ProviderService.test.ts apps/server/src/provider/Layers/ProviderSessionDirectory.test.ts apps/server/src/serverRuntimeStartup.reconcile.test.ts
• vp lint and vp fmt on the changed files
• vp run --filter t3 typecheck

Made by Claude Opus 5.5 (1M context) in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved session handling during provider shutdown. Sessions that are already stopped with no active work remain unchanged, while sessions with active or stale turn state are marked stopped and their active state is cleared.
  • Shutdown reporting now counts only sessions that required stopping, rather than all sessions found, providing a more accurate total for completed shutdown actions. This keeps reported shutdown totals aligned with the sessions actually updated.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: Team

Run ID: aa742b4a-574e-44f3-9542-cc6e4a52ebc8

📥 Commits

Reviewing files that changed from the base of the PR and between b320109 and f53ac44.

📒 Files selected for processing (2)

• apps/server/src/provider/Layers/ProviderService.test.ts
• apps/server/src/provider/Layers/ProviderService.ts

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

---

📝 Walkthrough

Walkthrough

Provider shutdown now filters settled bindings before rewriting session state. The stopped-all analytics event reports the number of bindings selected for rewriting. A test covers settled and unsettled bindings.

Changes

Provider shutdown

Layer / File(s)	Summary
Select and rewrite shutdown bindings apps/server/src/provider/Layers/ProviderService.ts, apps/server/src/provider/Layers/ProviderService.test.ts	Shutdown excludes settled bindings from rewrites and reports the selected binding count. The test checks that settled bindings remain unchanged and that unsettled bindings are stopped with active-turn markers cleared.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f53ac

No actionable merge-blocking risk is identified; the shutdown change appears ready for normal merge checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: shutdown skips rewriting settled stopped-session rows.
Description check	✅ Passed	The description explains the problem, the fix, and verification. Its Problem and Fix sections cover the template’s What Changed and Why sections. UI changes do not apply, but the Checklist section is …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	13.5 KiB	13.5 KiB	+27 B (+0.2%)	15.1 KiB	✅
Codex	Thread snapshot wire	7.1 KiB	7.1 KiB	+2 B (+0.0%)	7.3 KiB	✅
Codex	Live turn WebSocket wire	6.4 KiB	6.5 KiB	+25 B (+0.4%)	7.8 KiB	✅
Codex	Live turn WebSocket decoded	56.2 KiB	56.3 KiB	+44 B (+0.1%)	66.4 KiB	✅
Codex	Live turn messages	9	10	+1 (+11.1%)	21	✅
Claude	Total thread wire	13.5 KiB	13.5 KiB	+19 B (+0.1%)	15.1 KiB	✅
Claude	Thread snapshot wire	7.1 KiB	7.1 KiB	0 B (0.0%)	7.3 KiB	✅
Claude	Live turn WebSocket wire	6.4 KiB	6.4 KiB	+19 B (+0.3%)	7.8 KiB	✅
Claude	Live turn WebSocket decoded	57.0 KiB	57.0 KiB	0 B (0.0%)	66.4 KiB	✅
Claude	Live turn messages	9	9	0 (0.0%)	21	✅

Baseline: 6530de0 · PR result: f53ac44 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 113.9 KiB
• Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[macroscopeapp]

Approvability

Verdict: Approved at f53ac44

Macroscope's review found this PR approvable — This is a narrowly scoped shutdown performance fix that avoids rewriting settled session rows while preserving updates for active or stale-turn sessions. A focused regression test verifies the persistence behavior and revised shutdown metric.

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

Dismissing prior approval to re-evaluate e8c351f

Dismissing prior approval to re-evaluate f53ac44