Repository navigation
fix(server): a start killed while copying V1 no longer leaves its copy on disk - #21964
immakermatty wants to merge 1 commit into
Conversation
…y on disk The first V2 start copies state.sqlite into a .v2-import-* directory before it publishes the copy. A start killed during the copy never removed that directory, and nothing else did, so every killed attempt left a database-sized directory in userdata. The directory name now carries the copying process ID. Every start removes .v2-import-* directories whose process no longer runs, and the ones from older builds, which carry no ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Written by thanks @immakermatty for the pid-aware cleanup, live-process guard, legacy-copy handling, and regression coverage. those address the leak directly. i tried updating your branch with main merged in, but github rejected the push to i compared the original diff against the replacement. the startup cleanup, pid-prefixed names, live-process guard, legacy cleanup, and nonblocking error handling remain. the pid probe moved into the persistence module to avoid a circular import through startup/authentication/sqlite. the filename match is narrower, the explanatory comment is shorter, and the existing regression also checks unrelated entries and existing v2 data. no other behavior was dropped. |
Problem
The first 0.1.x start copies
state.sqliteinto a.v2-import-*directory inuserdatabefore it publishes the copy. A normal failure removes the directory, but a start killed during the copy (SIGKILL, a crash, power loss) leaves it behind, and nothing removes it later. It is as big as the V1 database, and every killed attempt adds another one.Change
.v2-import-<pid>-XXXXXX..v2-import-*directories whose process no longer runs, usingisProcessAlivefromserverRuntimeState.ts, and the ones from older builds, which carry no ID.Limits:
userdataat the same moment would lose its directory. That start fails with the existing import error and copies again next time.Scope and approval
A small, focused fix for an obvious leak: a temporary directory that nothing removes. It sits on the same import path as #15017, but it's a separate defect, so it's not in #21965.
Verification
initializeV2Database.test.ts: a directory from a process that has exited and one from an older build are removed, and one owned by the running process stays. It fails without the cleanup, when the cleanup ignores whether the process runs, and when it spares directories from older builds.main: the 50 MB.v2-import-*directory is still there after the next start.main's code, which has no process ID, is removed on the following start.vp test run apps/server/src/persistence/initializeV2Database.test.ts(5 passed),vp run --filter t3 typecheckandvp linton the changed files pass.Not checked: Windows.
isProcessAlivealready relies onprocess.kill(pid, 0)there.Model: Claude Opus 5.5. Harness: Claude Code inside T3 Code.
🤖 Generated with Claude Code