From ca541dea8e8fca6881a26a667d57e1d19154b1d2 Mon Sep 17 00:00:00 2001 From: Eduard Arbona <175124143+earbona23@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:32:48 -0400 Subject: [PATCH 1/3] test: pin that a provider-scoped run stamps the whole cache complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repro for the `parser.ts` candidate in #912: a run filtered to one provider leaves every other provider unscanned and still marks the whole cache complete. Three lines in runParseInner tell it. Discovery is filtered — `discoverAllSessions(providerFilter)`. The loop over cached providers skips out-of-scope names — `if (providerFilter && providerFilter !== 'all' && providerFilter !== providerName) continue`. Then the stamp fires guarded only on readOnly / wasComplete / deferredForFirstPaint, with nothing about scope. That is worse than the silent zeros in #874 and #899, which lasted one run. This one is written to disk: once the cache is stamped, later launches stop coming back cold for those providers, so the gap stops looking like a gap. Test only, per the issue. It asserts its own premise first — that claude really was left unscanned — because otherwise the second assertion proves nothing. Mutation-sensitive, checked both ways rather than asserted: - as-is, the assertion fails with `expected true to be false` - adding `&& !scopedRun` to the stamp makes it pass - reverting makes it fail again Marked `it.fails` so the suite stays green while the defect is open. When the stamp learns about scope this test starts passing, vitest reports "Expect test to fail", and that is the signal to drop `.fails` and keep it as a plain regression test. Verified that flip too. --- tests/scoped-run-completeness.test.ts | 65 +++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 tests/scoped-run-completeness.test.ts diff --git a/tests/scoped-run-completeness.test.ts b/tests/scoped-run-completeness.test.ts new file mode 100644 index 00000000..8832a2ec --- /dev/null +++ b/tests/scoped-run-completeness.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { mkdtemp, mkdir, writeFile, rm } from 'fs/promises' +import { join } from 'path' +import { tmpdir } from 'os' + +import { parseAllSessions, clearSessionCache } from '../src/parser.js' +import { readCacheOnDisk } from './fixtures/session-cache-io.js' + +let tmpDir: string + +beforeEach(async () => { + clearSessionCache() + tmpDir = await mkdtemp(join(tmpdir(), 'scoped-complete-')) + process.env['CLAUDE_CONFIG_DIR'] = tmpDir + process.env['CODEBURN_CACHE_DIR'] = join(tmpDir, 'cache') + process.env['CODEBURN_DESKTOP_SESSIONS_DIR'] = join(tmpDir, 'desktop-sessions') +}) + +afterEach(async () => { + clearSessionCache() + await rm(tmpDir, { recursive: true, force: true }) +}) + +async function writeClaudeSession(): Promise { + const dir = join(tmpDir, 'projects', 'proj') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'sess.jsonl'), JSON.stringify({ + type: 'assistant', + sessionId: 'sess', + timestamp: '2026-05-15T10:00:00Z', + cwd: '/tmp/proj', + message: { + id: 'msg-1', type: 'message', role: 'assistant', model: 'claude-sonnet-4-5', + content: [], usage: { input_tokens: 100, output_tokens: 50 }, + }, + }) + '\n') +} + +describe('provider-scoped run and the whole-cache completeness marker', () => { + // `it.fails` on purpose: this pins CURRENT behaviour, which is wrong, without + // turning the suite red while the defect is still open. When the stamp learns + // about scope, this test starts passing and vitest will fail it here — that is + // the signal to drop `.fails` and keep it as a normal regression test. + it.fails('does not stamp the cache complete when providers were left out of scope', async () => { + await writeClaudeSession() + + // A scoped run: discovery is filtered to `codex`, and the loop over cached + // providers skips every name that is not the filter. Claude's session on disk + // is therefore never read by this run. + await parseAllSessions(undefined, 'codex') + + const raw = await readCacheOnDisk() + const claudeFiles = Object.keys(raw?.providers?.claude?.files ?? {}) + + // Premise of the test: claude really was left unscanned. If this fails the + // scoped run read it anyway and the rest proves nothing. + expect(claudeFiles.length).toBe(0) + + // The finding: `complete` is a whole-cache marker, and the stamp at the end of + // runParseInner is guarded on readOnly / wasComplete / deferredForFirstPaint — + // not on whether the run was scoped. A cache stamped complete here claims a + // corpus this run never looked at. + expect(raw?.complete ?? false).toBe(false) + }) +}) From e3cd276ace81fadea9547ed081b97e09a508eca4 Mon Sep 17 00:00:00 2001 From: Eduard Arbona <175124143+earbona23@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:25:15 -0400 Subject: [PATCH 2/3] test: hermetic Codex root, and narrow the claim to what is proven Addresses review on #1229. 1. Hermetic fixture. The scoped run is `parseAllSessions(undefined, 'codex')`, and Codex discovery falls back to CODEX_HOME / ~/.codex, so the test was reading the developer's real corpus. It now points CODEX_HOME at an empty temp root and restores the prior value (including unset) in teardown. 2. Claim narrowed. You are right that a subsequent all-provider refresh rediscovers the omitted provider, so "nothing re-derives them" was overstated. Dropped it. Added a test that pins the actual bound: after the scoped run, a plain `parseAllSessions()` repairs the cache and claude reappears. The mislabel is transient, not a persistent zero. The `it.fails` invariant is unchanged and still mutation-sensitive: guarding the stamp with `&& !scopedRun` flips it to a real pass (vitest then reports "Expect test to fail"), reverting flips it back. Ready for the guard to land here and `.fails` to become a normal regression test whenever you want to pull the fix into this PR. --- tests/scoped-run-completeness.test.ts | 48 ++++++++++++++++++--------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/tests/scoped-run-completeness.test.ts b/tests/scoped-run-completeness.test.ts index 8832a2ec..a9df1321 100644 --- a/tests/scoped-run-completeness.test.ts +++ b/tests/scoped-run-completeness.test.ts @@ -7,6 +7,7 @@ import { parseAllSessions, clearSessionCache } from '../src/parser.js' import { readCacheOnDisk } from './fixtures/session-cache-io.js' let tmpDir: string +let savedCodexHome: string | undefined beforeEach(async () => { clearSessionCache() @@ -14,10 +15,18 @@ beforeEach(async () => { process.env['CLAUDE_CONFIG_DIR'] = tmpDir process.env['CODEBURN_CACHE_DIR'] = join(tmpDir, 'cache') process.env['CODEBURN_DESKTOP_SESSIONS_DIR'] = join(tmpDir, 'desktop-sessions') + // Hermetic: Codex discovery defaults to CODEX_HOME / ~/.codex, so without this + // the scoped 'codex' run below reads the developer's real local corpus. Point it + // at an empty temp root and restore in teardown. + savedCodexHome = process.env['CODEX_HOME'] + process.env['CODEX_HOME'] = join(tmpDir, 'codex-home') + await mkdir(join(tmpDir, 'codex-home', 'sessions'), { recursive: true }) }) afterEach(async () => { clearSessionCache() + if (savedCodexHome === undefined) delete process.env['CODEX_HOME'] + else process.env['CODEX_HOME'] = savedCodexHome await rm(tmpDir, { recursive: true, force: true }) }) @@ -37,29 +46,38 @@ async function writeClaudeSession(): Promise { } describe('provider-scoped run and the whole-cache completeness marker', () => { - // `it.fails` on purpose: this pins CURRENT behaviour, which is wrong, without - // turning the suite red while the defect is still open. When the stamp learns - // about scope, this test starts passing and vitest will fail it here — that is - // the signal to drop `.fails` and keep it as a normal regression test. + // `it.fails` pins current (wrong) behaviour without turning the suite red while the + // defect is open. When the stamp learns about scope this starts passing, vitest reports + // "Expect test to fail", and that is the signal to drop `.fails` and land the guard. it.fails('does not stamp the cache complete when providers were left out of scope', async () => { await writeClaudeSession() - // A scoped run: discovery is filtered to `codex`, and the loop over cached - // providers skips every name that is not the filter. Claude's session on disk - // is therefore never read by this run. + // Scoped to 'codex': discovery is filtered and the cached-provider loop skips every + // other name, so the claude session on disk is never read by this run. await parseAllSessions(undefined, 'codex') const raw = await readCacheOnDisk() - const claudeFiles = Object.keys(raw?.providers?.claude?.files ?? {}) - // Premise of the test: claude really was left unscanned. If this fails the - // scoped run read it anyway and the rest proves nothing. - expect(claudeFiles.length).toBe(0) + // Premise: claude really was left unscanned. If this fails the scoped run read it + // anyway and the rest proves nothing. + expect(Object.keys(raw?.providers?.claude?.files ?? {}).length).toBe(0) - // The finding: `complete` is a whole-cache marker, and the stamp at the end of - // runParseInner is guarded on readOnly / wasComplete / deferredForFirstPaint — - // not on whether the run was scoped. A cache stamped complete here claims a - // corpus this run never looked at. + // The finding: the stamp at the end of runParseInner is guarded on + // readOnly / wasComplete / deferredForFirstPaint — not on whether the run was scoped. expect(raw?.complete ?? false).toBe(false) }) + + it('a subsequent full run repairs the cache — the mislabel is transient, not persistent', async () => { + // Honouring the maintainer review: an ordinary all-provider refresh rediscovers the + // omitted provider, so the wrong flag does not strand it forever. Pinning the bound + // keeps the claim accurate. + await writeClaudeSession() + + await parseAllSessions(undefined, 'codex') // scoped: claude unscanned + clearSessionCache() + await parseAllSessions() // full refresh: claude rediscovered + + const raw = await readCacheOnDisk() + expect(Object.keys(raw?.providers?.claude?.files ?? {}).length).toBeGreaterThan(0) + }) }) From 0e8a379e5c5aacc2e6c3ac3fb8b0c6d6e240f9e0 Mon Sep 17 00:00:00 2001 From: Eduard Arbona <175124143+earbona23@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:02:10 -0400 Subject: [PATCH 3/3] fix(parser): a provider-scoped run must not stamp the whole cache complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run scoped to one provider walks only that provider's sessions, yet the end-of-parse stamp marked the WHOLE cache complete regardless of scope (#912). Because a complete cache stops being re-read as cold, the providers the scoped run skipped are never revisited — a wrong "done" written to disk, where the gap stops looking like a gap. Guard the stamp on real on-disk data, not on scoping alone: a scoped run still stamps the cache complete when every provider it skipped has no discoverable sessions (a single-provider machine — exactly what the warm-refresh snapshot tests rely on). Only when a skipped provider actually has sessions on disk is the whole-cache claim withheld. The extra check is a bounded directory walk, not a parse, so the scoping win holds. Drops it.fails on the #912 regression; it now passes as an ordinary test. --- src/parser.ts | 19 ++++++++++++++++++- tests/scoped-run-completeness.test.ts | 12 ++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/parser.ts b/src/parser.ts index 24e16290..32988940 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -5491,7 +5491,24 @@ async function runParseInner( // it keeps the normal stamp. const deferredForFirstPaint = firstPaintDeferredThisRun > 0 const wasComplete = isCacheComplete(diskCache) - if (!readOnly && !wasComplete && !deferredForFirstPaint) diskCache.complete = true + // A provider-scoped run walks only its own provider's sessions, so it never + // saw whatever the providers it skipped hold on disk. Stamping the WHOLE + // cache complete off that partial view writes a wrong "done" to disk (#912): + // per the marker's own contract above, a complete cache stops being re-read + // as cold, so the unscanned providers are not revisited and the gap stops + // looking like a gap. The guard is conditioned on real on-disk data, not on + // scoping alone: when every provider the run skipped has NO discoverable + // sessions, the scoped run really did see the whole corpus (a claude-only + // machine, and exactly what the warm-refresh snapshot tests rely on), so the + // stamp is correct and stands. Discovery here is a bounded directory walk, + // not a parse — the scoping win (skipping the other providers' PARSE) holds. + const scopedRun = !!providerFilter && providerFilter !== 'all' + let skippedProviderHasSessions = false + if (scopedRun && !readOnly && !wasComplete && !deferredForFirstPaint) { + const corpusSources = await discoverAllSessions() + skippedProviderHasSessions = corpusSources.some(s => s.provider !== providerFilter) + } + if (!readOnly && !wasComplete && !deferredForFirstPaint && !skippedProviderHasSessions) diskCache.complete = true if (!readOnly && (isCacheDirty(diskCache) || (!wasComplete && !deferredForFirstPaint))) { try { const published = await saveCache(diskCache, refreshLock?.verifyStillOwner) diff --git a/tests/scoped-run-completeness.test.ts b/tests/scoped-run-completeness.test.ts index a9df1321..68d30850 100644 --- a/tests/scoped-run-completeness.test.ts +++ b/tests/scoped-run-completeness.test.ts @@ -46,10 +46,10 @@ async function writeClaudeSession(): Promise { } describe('provider-scoped run and the whole-cache completeness marker', () => { - // `it.fails` pins current (wrong) behaviour without turning the suite red while the - // defect is open. When the stamp learns about scope this starts passing, vitest reports - // "Expect test to fail", and that is the signal to drop `.fails` and land the guard. - it.fails('does not stamp the cache complete when providers were left out of scope', async () => { + // Regression for #912: the stamp now learns about scope. A run scoped to one provider + // must not mark the whole cache complete while a provider it skipped still has sessions + // on disk it never scanned. + it('does not stamp the cache complete when providers were left out of scope', async () => { await writeClaudeSession() // Scoped to 'codex': discovery is filtered and the cached-provider loop skips every @@ -62,8 +62,8 @@ describe('provider-scoped run and the whole-cache completeness marker', () => { // anyway and the rest proves nothing. expect(Object.keys(raw?.providers?.claude?.files ?? {}).length).toBe(0) - // The finding: the stamp at the end of runParseInner is guarded on - // readOnly / wasComplete / deferredForFirstPaint — not on whether the run was scoped. + // The guard: the stamp at the end of runParseInner refuses to mark the whole cache + // complete when a skipped provider (claude here) still has discoverable sessions. expect(raw?.complete ?? false).toBe(false) })