fix(scanner): never build a persistent scanner for a cold-start scan - #879
Open
RonenMars wants to merge 1 commit into
Open
fix(scanner): never build a persistent scanner for a cold-start scan#879RonenMars wants to merge 1 commit into
RonenMars wants to merge 1 commit into
Conversation
The three scan-construction sites passed `statCache ? { persistent: false } : undefined`, and the undefined branch falls through to the scanner's persistent default.
That branch is taken exactly when the cache holds no stat rows, which is the state `tb-streamer cache clear` and the integrity monitor's reset-and-rescan both create.
A cold start therefore read the scanner's own persistent index instead of parsing the tree, importing rows for files deleted long ago.
On one machine that turned an emptied cache into 172 rows against 51 transcripts on disk, 122 of them naming files that no longer exist, and raised a high-severity integrity alert.
The streamer never serves from the scanner's persistent index in any other state, so forcing non-persistent costs nothing.
Closes #876
RonenMars
force-pushed
the
fix/cold-start-non-persistent-scan
branch
from
September 12, 2026 06:30
0a5c1ac to
361ef2f
Compare
RonenMars
added a commit
that referenced
this pull request
Sep 12, 2026
`startServer` in `__tests__/codex-active-writer.test.ts` builds a per-server `cacheDir` but lets `runtime.db` fall through to the shared default. The adopt cases register a managed session, and `runtime.db` is the authoritative store that deliberately outlives `server.close()`. So a later server's boot reconcile picks that row up through `reconcilePreviousSessions` and probes the previous case's pid with `isPidAlive`, which is `process.kill(pid, 0)`. That sends no signal, but the two cases asserting "and kills nothing" spy on `process.kill` broadly enough to record it, and they fail. ## Why this is worth fixing rather than tolerating It passes on main today only because the reconcile is fire-and-forget and usually lands outside the spy's window. Running just the takeover case and one refusal together already fails on main: `npx vitest run __tests__/codex-active-writer.test.ts -t "take"`. Anything that shifts startup timing turns a latent ordering dependency into a red suite, and the failure points at the scanner rather than at test isolation, which costs the next reader real time. ## The change Hoist the temp dir into a local and pass `runtimeDbPath: join(dir, "runtime.db")` alongside the existing `cacheDir`, matching how `access-probe.test.ts` and `auto-resume-on-boot.test.ts` already isolate it. ## Verification The file passes 12/12 on main with this fix. It also passes 12/12 with the #876 scanner change applied on top, which is the state that surfaced the problem. Full suite on this branch: 283 files, 3062 passed, 0 failed, lint clean across 517 files. Unblocks #879.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #876.
What was wrong
Three sites construct a scanner as
statCache ? { persistent: false } : undefined, andnewScannerresolvesundefinedto the scanner's own default, which is persistent.The
undefinedbranch is taken exactly whenbuildStatCachefinds no stat rows, and that is the statetb-streamer cache clearand the integrity monitor's reset-and-rescan both create.So the one condition under which the streamer reads the scanner's persistent index is the condition the documented recovery step produces, and a cold start imports whatever that index still remembers instead of parsing the tree.
Measured on the Windows box after a
cache clearplus a restart: 172 rows inconversation_metaagainst 51 transcripts on disk, 122 of them naming files verified absent withexistsSync, and a high-severity integrity alert that forced an automatic cache backup.The source index at
~/.config/threadbase-scanner/index.dbheld 643 conversations and 77 scanned directories, many deleted weeks earlier.None of the 122 carried a tail, so they were metadata ghosts rather than recoverable history.
The change
All three sites now pass
{ persistent: false }unconditionally.The streamer never serves from the scanner's persistent index in any other state, so giving it up costs nothing, and this covers the manual clear, the monitor's reset-and-rescan, and a fresh install alike.
I deliberately did not centralise the default inside
newScanner.That would be one line, but it removes the only read of
persistenceDisabledand leaves that field anddisablePersistence()dead, pulling the cache-open-failure path inserver.tsinto an otherwise three-line diff.I also left the two no-argument
newScanner()callers inconversations.handlers.tsalone, since those are single-file and cold-read paths rather than the cold-start scan this issue describes.They are worth a separate look.
Verification
The new test fails without the fix and passes with it, which I checked by reverting the source and re-running rather than assuming.
Without the fix all three cases fail with
expected undefined to deeply equal { persistent: false }.Full suite on this branch: 284 files, 3062 passed, 0 failed.
Lint clean across 518 files and
tsc -p tsconfig.build.json --noEmitclean.The suite reports 30 skipped here against 25 in a built checkout.
That delta is environmental and accounted for exactly: three
describe.skipIf(!existsSync(SCRIPT))blocks gate ondist/*.cjs, which a fresh worktree has not built, and they contain 1 + 2 + 2 tests.