[navidnadali]

Problem

When one SQLite reader worker fails to start (an open error, a database not in WAL mode, or the 10 s start timeout, which a busy main thread can trip), nodeSqliteClient marks the whole reader pool unavailable for the rest of the process. Every read then runs on the writer connection until the server restarts, queued behind writes. A reader that crashes before it is ready is instead respawned on the next read, with no backoff and no warning.

Change

• A failed reader start turns the pool off with one warning; reads use the writer meanwhile, as today.
• One background retry brings the pool back: 1 s, doubling to 60 s, silent while it keeps failing, and one log line when readers are back. The retry belongs to the client and stops on close.
• A database that is not in WAL mode keeps readers off for good, with one warning and no retries.
• A reader that crashes before it is ready takes the same warn-and-back-off path.
• A gauge, t3_sqlite_reader_pool_available (0 or 1), shows the state.

The backoff is fixed in code; there is no setting. One file: packages/shared/src/nodeSqliteClient.ts. Server only; no client or contract change.

Scope and approval

This is submitted under the contributing guide's exception for a very small, focused fix to an obvious bug: the defect is shown by tests that fail on main (below), the product change is +120/−48 lines in one file (the rest is tests), and apart from the availability gauge it adds no behaviour, default or setting.

A focused reliability fix for an obvious defect: today a transient start failure leaves readers off for the life of the process. It changes no product default, setting or contract. The only addition beyond the fix is the availability gauge, so the state can be seen. A separate follow-up PR edits the same file; it will be opened after this one. Open PRs #27979 and #30067 also edit nodeSqliteClient.ts, for different problems. #30864 also edits it (fewer reader round trips); the two conflict in a few small hunks, so whichever lands second needs a small rebase.

Background and measurements: #31002 (context, not an approval request).

Verification

Failing first (test clock and queues only; the tests assert events and counts, never elapsed time):

• vp test run packages/shared/src/nodeSqliteClient.test.ts -t "until a retry starts the reader again": two cases, a reader that crashes at start and one that times out at start. Reads go to the writer, a retry starts the reader again, reads route back to the readers and the gauge goes 0 → 1. Both fail on main by assertion.
• -t "backs off a reader that never starts without logging each retry": 12 retries in 10 virtual minutes (1 s doubling to a 60 s cap) and exactly one log line. Fails on main.
• -t "keeps readers off without further retries when": not in WAL mode, found at start or by a retry; one warning and no further retries (2 worker starts in a virtual hour). main also leaves readers off in both cases; there they fail only on what this change adds (the gauge, and the retry path's log lines).
• -t "stops retrying the reader when the client closes": closing the client while the retry waits, or while a retried reader is starting, leaves no reader starting in the 10 virtual minutes after. Both cases fail when the retry is made to outlive the client; on main, which never retries, only the second fails.

On main @ 8b44f42b1, six of the seven cases above fail and the file's other 19 tests pass; with this change, 26/26 pass. Removing the gauge update, the non-WAL stop, the backoff growth, the one-line logging, the retry's context isolation or the retry's tie to the client's scope each fails a test (mutation checks).

• vp run check:changed: passes (format, lint, knip, typecheck of 35 packages, and 31 tests in the 2 affected test files), on macOS with Node 26.12.1. The server integration suites (vp test run apps/server/integration) also pass, 352 tests in 26 files, and so do the provider replay fixture suites (vp test run apps/server/src/orchestration-v2/testkit), 248 tests in 17 files.

No benchmark: the problem appears only when a reader start fails, which a load benchmark does not exercise.

Not checked

• A real start failure under load on a running server; the evidence is tests on a virtual clock.
• Windows.
• Unchanged model: the whole pool still turns off when any one reader fails to start, and reads use the writer until the retry succeeds.

Made with Claude Opus 5.5 via Claude Code. Reviewed by GPT-6.1 Sol via Codex and, independently, by Claude Opus 5.5 via Claude Code.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at e667d99

Macroscope's review found this PR approvable — SQLite reader workers now recover automatically after transient startup failures, with capped backoff and availability metrics. Existing read-only safeguards and main-connection fallback remain in place, limiting the change to reader availability.

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

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified as-is at e667d990c0bf4177ea30ed18fc6bf8196fe01f09 on linux. ran vp i and background dev servers, one at a time. used repository playwright with /usr/bin/google-chrome, created a real git project and threads through the web app, and sent real codex gpt-6.1-sol turns through the local proxy. no mocks or seeded data.

reproduced on github/main (9e85538535) in a separate worktree, then repeated on this exact head. the fault retired a real reader through the node debugger and briefly denied access to the task's userdata directory, so its replacement failed its actual sqlite open. restored permissions without restarting the server. this also temporarily denied trace/provider-log writes; the gauge collector stayed outside the affected directory.

main: open failure logged at 14:20:26 UTC
      after access restored: 165 writer-routed operations, 0 reader-routed
      over 86 seconds; another codex turn and history reload succeeded

pr:   14:31:27.068 reader unavailable: unable to open database file
      14:31:31.038 filesystem access restored
      14:31:33.393 successful SELECT on writer fallback
      14:31:34.161 reader available again, without server restart
      actual OTLP availability gauge: 1 -> 0 -> 1
      following window: 155 reader-routed operations, 14 writer-routed

follow-up send, a fresh thread with another real codex response, and history reload passed after recovery. the counts above include only spans with an explicit connection annotation. these are recovery observations, not a performance benchmark.

blacksmith passed the 24 sqlite tests and vp run check:changed 8b44f42b14c8e273156474a924ccf29680dd7857: format, lint, knip, 34 affected package typechecks, and 25 tests. the explicit base supplies the pr ancestry missing from the remote snapshot's initial history. existing github checks: 29 successful, none failing or pending.

command transcript and gauge observations

Not verified

native ios/android, the electron shell, windows/macos, and remote relay/tunnel connections were not exercised on this linux vm. the running-app fault covered a transient open failure; startup timeout, the 60-second retry cap, permanent non-wal disablement, and closing during retry were not exercised in the app. the existing focused sqlite tests passed. no code changes were needed.

[navidnadali]

Note

Written by Claude Opus 5.5 (Claude Code) on behalf of Navid

Thanks for checking it in a running app, and for the gauge trace. On the paths you didn't exercise: the start timeout, the 60 s cap and the permanent non-WAL stop each have a focused test (the timeout case of "until a retry starts the reader again", "backs off a reader that never starts without logging each retry", and "keeps readers off without further retries when …"). Closing during a retry had no dedicated test, so I've added one in db37135e0: the client closes while a retry is waiting, or while a retried reader is starting, and no reader starts in the 10 virtual minutes after. It fails if the retry outlives the client.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The SQLite client classifies reader-worker startup failures, retries recoverable failures with backoff, and stops retries for permanent failures. It tracks reader-pool availability and falls back to the writer while readers are unavailable. Tests cover failure, recovery, retry timing, client closure, and non-WAL cases.

Changes: SQLite reader recovery

Layer / File(s)	Summary
Classify reader startup outcomes packages/shared/src/nodeSqliteClient.ts	Startup failures include retryability information. Non-WAL failures are permanent, and workers track readiness during startup and shutdown.
Open and retire readers packages/shared/src/nodeSqliteClient.ts	Reader startup returns a reader or a structured failure. Retired readers are terminated, and the active slot is cleared only when the retiring reader owns it. Read acquisition returns a reader only when the pool is available and the reader remains current.
Retry failures and report availability packages/shared/src/nodeSqliteClient.ts, packages/shared/src/nodeSqliteClient.test.ts	Retryable failures use exponential backoff, capped at one minute. The availability gauge reflects pool state. Tests cover recovery, retries, closure, and non-WAL behavior.

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SQLiteClient
  participant ReaderWorker
  participant WriterConnection
  participant AvailabilityGauge
  SQLiteClient->>ReaderWorker: Start reader
  ReaderWorker-->>SQLiteClient: Report startup failure
  SQLiteClient->>WriterConnection: Serve reads while readers are unavailable
  SQLiteClient->>ReaderWorker: Retry after backoff
  ReaderWorker-->>SQLiteClient: Report successful startup
  SQLiteClient->>AvailabilityGauge: Set availability to one

Loading

Possibly related PRs

• pingdotgg/t3code#30067: Changes reader-worker startup and fallback behavior in the same SQLite client.
• pingdotgg/t3code#24762: Introduces the reader pool and its fallback behavior, which this change further updates.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to 92c84

The change lets the SQLite reader pool recover after a transient start failure. Reads fall back to the writer in the meantime. No actionable merge-blocking risk was identified. Windows and real startup failure under load were not exercised.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title describes the main change: a SQLite reader pool that previously stayed disabled after a failed start now recovers through a background retry. It follows the conventional commit format and na…
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.
Description check		The description follows the repository template and covers the problem, change, scope, and verification. It explains the root cause (a failed reader start disables the whole pool for the process lifet…

Full details: Title check

Explanation

The title describes the main change: a SQLite reader pool that previously stayed disabled after a failed start now recovers through a background retry. It follows the conventional commit format and names the affected package (shared). It matches the changeset and the PR objectives, so it is a clear match rather than off-topic.

Full details: Description check

Explanation

The description follows the repository template and covers the problem, change, scope, and verification. It explains the root cause (a failed reader start disables the whole pool for the process lifetime) and lists the behavior changes: one warning, a background retry with 1 s to 60 s backoff, permanent stop for non-WAL databases, and a new availability gauge. Scope cites the contributing exception for small obvious bug fixes, links background discussion, and notes conflicting open PRs. Verification gives specific failing-first test names, the main-branch baseline (6 of 7 cases fail), the passing count (26/26), mutation checks, and a list of unchecked items (real start failure under load, Windows, and the unchanged whole-pool shutdown). The template's closing request to name the agent and harness is satisfied by the 'Made with Claude Opus 5.5 via Claude Code' line. Minor gaps: the Scope section asserts an exception to the contributing guide without linking the issue that shows the bug, and the template asks for the issue link or a reason why none is needed. The author gives the reason, so this is not a blocker.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: b2e86d7
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 08ae8e93-ce7d-4f88-9d7f-ce42e3a853eb

Commits

Reviewing files that changed from the base of the PR and between 6555086 and 92c8415.

Files selected for processing: 2

• packages/shared/src/nodeSqliteClient.test.ts
• packages/shared/src/nodeSqliteClient.ts

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