fix(desktop): prevent shared database ownership - #6098
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| return; | ||
| } | ||
|
|
||
| if (!(yield* probeT3Server(owner.origin))) { |
There was a problem hiding this comment.
🟠 High backend/DesktopBackendDatabaseOwner.ts:97
When probeT3Server returns false (probe failure or 2-second timeout), ensureDesktopBackendDatabaseAvailable returns successfully and lets the desktop proceed — but the owning process may still be alive and holding the database. A starting or transiently slow T3 server can fail the probe while already owning the SQLite database, so this preflight allows a second backend to launch against the same database, reintroducing the lock/duplicate-processor conflict it is meant to prevent. Consider failing (or blocking until the probe succeeds) when the owner process is alive but the probe does not confirm a healthy server, rather than treating a failed probe as safe.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/backend/DesktopBackendDatabaseOwner.ts around line 97:
When `probeT3Server` returns `false` (probe failure or 2-second timeout), `ensureDesktopBackendDatabaseAvailable` returns successfully and lets the desktop proceed — but the owning process may still be alive and holding the database. A starting or transiently slow T3 server can fail the probe while already owning the SQLite database, so this preflight allows a second backend to launch against the same database, reintroducing the lock/duplicate-processor conflict it is meant to prevent. Consider failing (or blocking until the probe succeeds) when the owner process is alive but the probe does not confirm a healthy server, rather than treating a failed probe as safe.
| } | ||
|
|
||
| const owner = runtimeOwner.value; | ||
| if (!(input.isProcessAlive ?? defaultIsProcessAlive)(owner.pid)) { |
There was a problem hiding this comment.
🟠 High backend/DesktopBackendDatabaseOwner.ts:93
ensureDesktopBackendDatabaseAvailable can block desktop startup on a stale runtime file even when no live server owns the state directory. The PID check only verifies that some process with owner.pid is alive, and the HTTP probe only verifies that some T3 server is reachable at owner.origin — the two checks are independent and never confirm that the live process is the one serving at that origin. PID reuse by an unrelated process and port reuse by a different T3 server make both checks pass, triggering a false DesktopBackendDatabaseOwnedError. If this loose coupling is acceptable, consider documenting the rationale; otherwise, bind the checks together (for example, have the probed server report its own PID and compare it to owner.pid).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/backend/DesktopBackendDatabaseOwner.ts around line 93:
`ensureDesktopBackendDatabaseAvailable` can block desktop startup on a stale runtime file even when no live server owns the state directory. The PID check only verifies that *some* process with `owner.pid` is alive, and the HTTP probe only verifies that *some* T3 server is reachable at `owner.origin` — the two checks are independent and never confirm that the live process is the one serving at that origin. PID reuse by an unrelated process and port reuse by a different T3 server make both checks pass, triggering a false `DesktopBackendDatabaseOwnedError`. If this loose coupling is acceptable, consider documenting the rationale; otherwise, bind the checks together (for example, have the probed server report its own PID and compare it to `owner.pid`).
What
server-runtime.jsonowner record before scanning for another portWhy
When the background service already listens on port 3773, the desktop currently scans to 3774 but still launches its embedded backend with the same default T3 home. Both long-lived servers then open
userdata/state.sqlite, causingdatabase is lockedHTTP 500s and duplicate resource usage.This enforces a single live backend owner for a desktop state directory. A SQLite busy timeout can reduce transient contention, but it cannot make duplicate schedulers, command processors, and WebSocket streams safe.
Impact
Users who launch the desktop while a background service owns the same T3 home now get a clear startup error with safe recovery instructions. They can stop the service or use a separate
T3CODE_HOMEand connect to the running environment.Checks
pnpm exec vp test run apps/desktop/src/backend/DesktopBackendDatabaseOwner.test.ts apps/desktop/src/app/DesktopAppErrors.test.ts— 2 files, 6 tests passedpnpm exec vp run --filter @t3tools/desktop typecheck— passedvp fmt --check— passedgit diff --cached --check— passedFixes #6097
Model: GPT-5 Codex
Harness: Codex desktop app
Note
Prevent desktop backend startup when another T3 server owns the database
DesktopBackendDatabaseOwner.ensureDesktopBackendDatabaseAvailablein DesktopBackendDatabaseOwner.ts, a pre-start guard that checks whether a previously recorded server process is still alive and serving a live T3 backend.process.kill(pid, 0), and probes/.well-known/t3/environmenton the recorded origin with a 2s timeout.DesktopBackendDatabaseOwnedErrorcontaining the origin, PID, and state directory.📊 Macroscope summarized 971ce3f. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.