From 36004080a68e955b3f49b9b2d5a784d76014f50d Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 04:28:54 +0300 Subject: [PATCH 1/4] fix(cache): declare the provider env overrides that must invalidate the cache Nine providers honor an env var that relocates where discovery looks, but the var was never declared in PROVIDER_ENV_VARS, so computeEnvFingerprint() did not hash it and the provider's cache section survived the change: sessions parsed from the old root kept being reported and the new root was never read, with no diagnostic anywhere (#920, same silent-wrong-numbers family as #874). Declare every env var that changes what a provider discovers or how its sessions parse, including the platform path vars that resolve a discovery root on Windows and Linux, and the CodeBurn-side directory overrides. Ambient platform vars (APPDATA, LOCALAPPDATA, XDG_CONFIG_HOME, XDG_DATA_HOME) are set by the OS or the desktop session for everyone, so doctor must not name them as a deliberate override: without the guard every Windows user would be told Claude and Copilot discovery runs under an override. They stay in the fingerprint - a change to them does move the discovery root - but doctor skips them when collecting overrides, and the probed paths it already prints show where CodeBurn looked. --- src/doctor.ts | 8 ++++++++ src/session-cache.ts | 30 +++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/doctor.ts b/src/doctor.ts index 818da52a..e16428af 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -104,12 +104,20 @@ const PARSE_SPAWNS = new Set(['antigravity']) // in a NOTHING FOUND hint. const NON_DISCOVERY_ENV_VARS = new Set(['CODEBURN_CACHE_DIR']) +// Ambient platform paths (set by the OS or desktop session for everyone), not +// deliberate user overrides: they are fingerprinted (a change to them does +// move the discovery root, so the cache must invalidate) but doctor must not +// name them as an override, because the probed paths it already prints show +// exactly where CodeBurn looked. +const AMBIENT_ENV_VARS = new Set(['APPDATA', 'LOCALAPPDATA', 'XDG_CONFIG_HOME', 'XDG_DATA_HOME']) + // ── Collect (pure, testable) ───────────────────────────────────────────── function collectEnvOverrides(providerName: string): DoctorEnvOverride[] { const vars = PROVIDER_ENV_VARS[providerName] ?? [] const out: DoctorEnvOverride[] = [] for (const name of vars) { + if (AMBIENT_ENV_VARS.has(name)) continue const value = process.env[name] if (value !== undefined && value !== '') out.push({ name, value }) } diff --git a/src/session-cache.ts b/src/session-cache.ts index 4d0e31b7..29dcfed1 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -171,25 +171,45 @@ const CACHE_FILE = `session-cache.v${CACHE_VERSION}.json` const LEGACY_CACHE_FILE = 'session-cache.json' const TEMP_FILE_MAX_AGE_MS = 5 * 60 * 1000 +// Env vars that change what a provider discovers or how its sessions parse. +// computeEnvFingerprint hashes exactly these to decide when a provider's cache +// section is stale; a var read by the provider but missing here means changing +// it serves the old section silently, reporting nothing from the new root. +// Two reads in src/providers/ are deliberately absent: CODEBURN_VERBOSE +// (sqlite-session-parser.ts) only changes logging verbosity, never parsed +// output, and AI_GATEWAY_API_KEY / VERCEL_OIDC_TOKEN (vercel-gateway.ts) are +// network credentials — vercel-gateway is network:true, and parser.ts:2888 +// short-circuits it past the fingerprint compare, re-fetching its synthetic +// source every run, so no cached section of it can go stale. export const PROVIDER_ENV_VARS: Record = { - claude: ['CLAUDE_CONFIG_DIRS', 'CLAUDE_CONFIG_DIR'], + claude: ['CLAUDE_CONFIG_DIRS', 'CLAUDE_CONFIG_DIR', 'CODEBURN_DESKTOP_SESSIONS_DIR', 'APPDATA', 'LOCALAPPDATA'], 'cline-cli': ['CLINE_SESSION_DATA_DIR', 'CLINE_DATA_DIR', 'CLINE_DIR'], + codebuff: ['CODEBUFF_DATA_DIR'], codewhale: ['CODEWHALE_HOME'], codex: ['CODEX_HOME'], + copilot: ['CODEBURN_COPILOT_SESSION_STATE_DIR', 'CODEBURN_COPILOT_OTEL_DB', 'CODEBURN_COPILOT_JETBRAINS_DIR', 'CODEBURN_COPILOT_WS_STORAGE_DIR', 'CODEBURN_COPILOT_GLOBAL_STORAGE_DIR', 'CODEBURN_COPILOT_DISABLE_OTEL', 'APPDATA', 'LOCALAPPDATA', 'XDG_CONFIG_HOME'], hermes: ['HERMES_HOME'], 'lingtai-tui': ['LINGTAI_HOME', 'LINGTAI_TUI_HOME', 'LINGTAI_TUI_GLOBAL_DIR'], droid: ['FACTORY_DIR'], - cursor: ['XDG_DATA_HOME'], + cursor: ['XDG_DATA_HOME', 'CODEBURN_CURSOR_MAX_BUBBLES'], 'cursor-agent': ['XDG_DATA_HOME'], + 'open-design': ['CODEBURN_OPEN_DESIGN_DIR', 'APPDATA'], opencode: ['XDG_DATA_HOME', 'OPENCODE_DATA_DIR', 'OPENCODE_DB_PREFIX'], - goose: ['XDG_DATA_HOME'], - crush: ['XDG_DATA_HOME'], + goose: ['XDG_DATA_HOME', 'GOOSE_PATH_ROOT'], + grok: ['GROK_HOME'], + crush: ['XDG_DATA_HOME', 'CRUSH_GLOBAL_DATA', 'LOCALAPPDATA'], warp: ['WARP_DB_PATH'], antigravity: ['CODEBURN_CACHE_DIR'], + 'kilo-code': ['XDG_DATA_HOME'], + kimi: ['KIMI_SHARE_DIR', 'KIMI_MODEL_NAME'], + kiro: ['KIRO_HOME'], + 'mistral-vibe': ['VIBE_HOME'], + mux: ['MUX_ROOT', 'CODEBURN_MUX_DIR'], qwen: ['QWEN_DATA_DIR'], - 'ibm-bob': ['XDG_CONFIG_HOME'], + 'ibm-bob': ['XDG_CONFIG_HOME', 'APPDATA'], quickdesk: ['QUICKWORK_HOME'], kimicode: ['KIMI_CODE_HOME'], + zerostack: ['ZS_DATA_DIR', 'XDG_DATA_HOME'], } // Names of providers whose cache entries are never evicted when source files From eab0cecb6c8ab4689f0d4f3f05160226429f2fa0 Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 04:47:40 +0300 Subject: [PATCH 2/4] test(cache): guard that every provider env read is declared The nine undeclared overrides in #920 all slipped through the same way: the declaration lives in one file and the read in another, and nothing tied them together. Add the static guard the issue asked for - every process.env read in src/providers is either declared in PROVIDER_ENV_VARS for the provider(s) that file serves, or allowlisted with a reason. It resolves bracket literals, dot access and `process.env[CONST]` indirection (open-design's ENV_DIR), and fails loudly on any read it cannot resolve to a name rather than skipping it, since a silently skipped read is how this class of defect survives. A read-bearing provider file missing from the file-to-provider map fails too, so a new provider cannot join without being mapped. A second assertion catches a PROVIDER_ENV_VARS key that is not a registered provider name, which declares nothing and fails just as silently. Plus the direct regression: each of the nine reported (provider, var) pairs must move the fingerprint, with codex/CODEX_HOME as the control the issue used, and the round trip asserted so the hash stays a pure function of the environment. --- CHANGELOG.md | 1 + tests/provider-env-declarations.test.ts | 197 ++++++++++++++++++++++++ tests/session-cache.test.ts | 59 +++++++ 3 files changed, 257 insertions(+) create mode 100644 tests/provider-env-declarations.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 97d51957..ebb394bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - **`--provider ` no longer leaks Claude spend into the detail panels.** A provider-filtered run still ran the Claude scan, whose orphan pass re-injected every cached Claude session, so By Project / By Model / By Activity showed Claude usage under, e.g., `--provider cursor` while the headline was correct. (#872, thanks @ozymandiashh) - **A degraded session parse no longer freezes daily history.** A read-only parse that served a stale or missing session file was treated as complete and finalized days it never covered, freezing warm-cache ingestion; a corrupt refresh lock is now recovered rather than ending ingestion, and a legitimately idle tail is no longer re-derived on every launch. (#856, thanks @avs-io) - **Pi / Oh My Pi transcripts with a leading title record are discovered.** OMP writes a `type: "title"` line before the session header; discovery now scans a bounded number of leading lines for the first session record instead of requiring it on the first physical line. (#846, #859, thanks @jbspeakr, @avs-io) +- **Nine providers served silently stale numbers after you pointed their env override at a different profile or root.** Kiro, Grok, Kimi, Mux, Mistral Vibe, Zerostack, Codebuff, Goose and Crush each honor an env var that relocates where discovery looks, but the var was never declared in the provider env fingerprint, so the cache section survived the change and kept reporting sessions parsed from the old root — with no diagnostic anywhere. The fix declares those vars, and the adjacent OS-set path variables that resolve a discovery root for Claude, Copilot, IBM Bob, Open Design and Kilo Code on Windows and Linux, plus Cursor's parse-budget override, so the section invalidates when any of them changes. Your next run re-parses all fifteen providers whose declarations changed — the nine above plus Claude, Copilot, Cursor, Open Design, IBM Bob and Kilo Code — once, and only once; `codeburn doctor` still names only deliberate overrides, never OS-set path variables such as APPDATA or XDG_DATA_HOME. (#920) ### Fixed - Claude Desktop and Cowork sessions are discovered for Windows Microsoft Store (MSIX) installs. (#611) diff --git a/tests/provider-env-declarations.test.ts b/tests/provider-env-declarations.test.ts new file mode 100644 index 00000000..2274c90a --- /dev/null +++ b/tests/provider-env-declarations.test.ts @@ -0,0 +1,197 @@ +// Static guard for issue #920: every `process.env` read inside +// src/providers/*.ts must be declared in PROVIDER_ENV_VARS for every provider +// whose cache section that file's reads affect — or be allowlisted below with +// a reason. An env var that changes what a provider discovers or how its +// sessions parse but is not fingerprinted means the cache section survives +// the change and serves silently stale numbers, exactly the defect class #920 +// reported (nine providers slipped through it). +import { describe, expect, it } from 'vitest' +import { readdirSync, readFileSync } from 'fs' +import { dirname, join } from 'path' +import { fileURLToPath } from 'url' + +import { PROVIDER_ENV_VARS } from '../src/session-cache.js' +import { getAllProviders } from '../src/providers/index.js' + +// ── src/providers/ → provider registry name(s) ──────────────────── +// The provider(s) whose cache section the file's env reads affect. Derived +// from the real code at the freeze sha (3600408); registry names come from +// src/providers/index.ts. Do NOT infer this from the filename at runtime — +// the two diverge (e.g. the shared sqlite-session-parser.ts serves two +// providers). A file that contains env reads and is missing here fails the +// guard: add it, with the provider(s) the reads serve. +const FILE_PROVIDERS: Record = { + 'claude.ts': ['claude'], + 'cline-cli.ts': ['cline-cli'], + 'codebuff.ts': ['codebuff'], + 'codewhale.ts': ['codewhale'], + 'codex.ts': ['codex'], + 'copilot.ts': ['copilot'], + 'droid.ts': ['droid'], + 'hermes.ts': ['hermes'], + 'lingtai-tui.ts': ['lingtai-tui'], + // Its only literal read is CODEBURN_CURSOR_MAX_BUBBLES (cursor.ts:692); + // XDG_DATA_HOME is declared for cursor but not read literally in this file. + 'cursor.ts': ['cursor'], + // The ENV_DIR const (open-design.ts:10) resolves to CODEBURN_OPEN_DESIGN_DIR. + 'open-design.ts': ['open-design'], + 'opencode.ts': ['opencode'], + 'goose.ts': ['goose'], + 'grok.ts': ['grok'], + 'crush.ts': ['crush'], + 'warp.ts': ['warp'], + 'antigravity.ts': ['antigravity'], + 'kilo-code.ts': ['kilo-code'], + 'kimi.ts': ['kimi'], + 'kiro.ts': ['kiro'], + 'mistral-vibe.ts': ['mistral-vibe'], + 'mux.ts': ['mux'], + 'qwen.ts': ['qwen'], + 'ibm-bob.ts': ['ibm-bob'], + 'quickdesk.ts': ['quickdesk'], + 'kimicode.ts': ['kimicode'], + 'zerostack.ts': ['zerostack'], + // Shared sqlite parser; its only importers in src/ are kilo-code.ts and + // opencode.ts. Its single read (CODEBURN_VERBOSE) is allowlisted, so this + // entry is informational — but required, because the file has reads. + 'sqlite-session-parser.ts': ['kilo-code', 'opencode'], + // Registered (lazy) network provider; its credential reads are allowlisted + // (see below) because network sources are re-fetched on every run. + 'vercel-gateway.ts': ['vercel-gateway'], +} + +// ── Allowlisted reads ──────────────────────────────────────────────────── +// Reads that must NOT invalidate a cache section, one-line reason each. +// If you add an entry here, the guard goes silent for that var — so the +// reason must say exactly why a change to it cannot make a cached section +// stale. +const ALLOWLIST: Record = { + CODEBURN_VERBOSE: 'sqlite-session-parser.ts:276 — logging verbosity only; changes no discovered path and no parsed value', + // vercel-gateway is a registered (lazy) provider — not "not a provider" — + // but it is network:true (vercel-gateway.ts:123): its single synthetic + // source is re-fetched on every run and never served from the cached + // section, because parser.ts:2888 short-circuits network providers past the + // fingerprint compare. No fingerprint of it can therefore go stale. + AI_GATEWAY_API_KEY: 'vercel-gateway.ts:20 — network credential; parser.ts:2888 re-fetches every run', + VERCEL_OIDC_TOKEN: 'vercel-gateway.ts:20 — network credential; parser.ts:2888 re-fetches every run', +} + +// ── Static extraction ─────────────────────────────────────────────────── + +// Resolved relative to this test file, never the process cwd. +const PROVIDERS_DIR = join(dirname(fileURLToPath(import.meta.url)), '..', 'src', 'providers') + +type EnvRead = { varName: string; line: number } + +// `const IDENT = 'NAME'` string declarations, used to resolve +// `process.env[IDENT]` reads (open-design.ts does this with ENV_DIR). +const STRING_CONST = /const\s+([A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*(['"])([^'"]*)\2/g + +function extractEnvReads(source: string): { reads: EnvRead[]; unresolvable: Array<{ line: number; expr: string }> } { + const consts = new Map() + for (const m of source.matchAll(STRING_CONST)) consts.set(m[1]!, m[3]!) + + const reads: EnvRead[] = [] + const unresolvable: Array<{ line: number; expr: string }> = [] + const anyRead = /process\.env/g + for (const m of source.matchAll(anyRead)) { + const line = source.slice(0, m.index).split('\n').length + const rest = source.slice(m.index + 'process.env'.length) + // The expression as written, for failure messages. + const expr = rest.trim().split(/[;\n]/)[0]! + + if (rest.trimStart().startsWith('[')) { + const bracket = rest.slice(rest.indexOf('[')) + const literal = /^\[\s*(['"])([A-Z0-9_]+)\1\s*\]/.exec(bracket) + if (literal) { + reads.push({ varName: literal[2]!, line }) + continue + } + const ident = /^\[\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*\]/.exec(bracket) + if (ident) { + const resolved = consts.get(ident[1]!) + if (resolved) { + reads.push({ varName: resolved, line }) + continue + } + unresolvable.push({ line, expr: `process.env[${ident[1]}]` }) + continue + } + unresolvable.push({ line, expr: `process.env${expr}` }) + continue + } + + if (rest.trimStart().startsWith('.')) { + const dot = /^\.\s*([A-Za-z_$][A-Za-z0-9_$]*)/.exec(rest) + if (dot) { + reads.push({ varName: dot[1]!, line }) + continue + } + } + + // Bare `process.env` or any other form: cannot name a var — fail loudly, + // an unresolvable read must never be silently skipped. + unresolvable.push({ line, expr: `process.env${expr}` }) + } + return { reads, unresolvable } +} + +function failWith(problems: string[]): void { + if (problems.length > 0) throw new Error(`\n${problems.join('\n\n')}`) +} + +describe('provider env declarations (#920)', () => { + it('every process.env read in src/providers is declared for the provider(s) it serves', () => { + const problems: string[] = [] + + for (const entry of readdirSync(PROVIDERS_DIR, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith('.ts')) continue + + const source = readFileSync(join(PROVIDERS_DIR, entry.name), 'utf8') + const { reads, unresolvable } = extractEnvReads(source) + if (reads.length === 0 && unresolvable.length === 0) continue + + const served = FILE_PROVIDERS[entry.name] + if (!served) { + problems.push( + `src/providers/${entry.name} reads env vars (${reads.map(r => r.varName).join(', ')}) but is missing from FILE_PROVIDERS — add it with the provider(s) whose cache section these reads affect.`, + ) + continue + } + + for (const { line, expr } of unresolvable) { + problems.push( + `src/providers/${entry.name}:${line}: unresolvable env read \`${expr}\` — resolve it to a literal name (e.g. \`const IDENT = 'NAME'\` in the same file) so the guard can verify it is declared; an unresolvable read must never be silently skipped.`, + ) + } + + for (const { varName, line } of reads) { + if (ALLOWLIST[varName]) continue + for (const provider of served) { + if (!(PROVIDER_ENV_VARS[provider] ?? []).includes(varName)) { + problems.push( + `provider '${provider}' reads process.env['${varName}'] at src/providers/${entry.name}:${line} but it is not declared in PROVIDER_ENV_VARS['${provider}'] — declare it there (it changes what the provider discovers or how its sessions parse) or add it to ALLOWLIST with a reason.`, + ) + } + } + } + } + + failWith(problems) + }) + + it('every PROVIDER_ENV_VARS key is a real provider name from the registry', async () => { + const names = new Set((await getAllProviders()).map(p => p.name)) + const problems: string[] = [] + for (const key of Object.keys(PROVIDER_ENV_VARS)) { + if (!names.has(key)) { + // A typo'd key declares nothing and fails silently — the same defect + // class #920 fixed. Do NOT delete the key or weaken the assertion; + // surface it so the registry or the key gets corrected. + problems.push(`PROVIDER_ENV_VARS key '${key}' is not a registered provider name — a typo'd key declares nothing and fails silently.`) + } + } + failWith(problems) + expect(problems).toEqual([]) + }) +}) diff --git a/tests/session-cache.test.ts b/tests/session-cache.test.ts index 3591e43c..1f13e696 100644 --- a/tests/session-cache.test.ts +++ b/tests/session-cache.test.ts @@ -281,6 +281,65 @@ describe('computeEnvFingerprint', () => { }) }) +// ── provider env overrides invalidate the fingerprint (#920) ───────────── + +describe('provider env overrides invalidate the fingerprint (#920)', () => { + // Nine providers honored an env var that relocates where discovery looks + // without the var being declared in PROVIDER_ENV_VARS, so + // computeEnvFingerprint did not hash it and the cache section survived the + // change: sessions parsed from the old root kept being reported and the new + // root was never read. Each pair below must change the fingerprint when the + // var is set. codex/CODEX_HOME is the control — it already worked and must + // keep working. + const CASES: Array<[provider: string, varName: string]> = [ + ['kiro', 'KIRO_HOME'], + ['grok', 'GROK_HOME'], + ['kimi', 'KIMI_SHARE_DIR'], + ['mux', 'MUX_ROOT'], + ['mistral-vibe', 'VIBE_HOME'], + ['zerostack', 'ZS_DATA_DIR'], + ['codebuff', 'CODEBUFF_DATA_DIR'], + ['goose', 'GOOSE_PATH_ROOT'], + ['crush', 'CRUSH_GLOBAL_DATA'], + ['codex', 'CODEX_HOME'], + ] + const VARS = CASES.map(([, varName]) => varName) + + // Save and restore every var we touch (beforeEach/afterEach), so a leaked + // env var never breaks unrelated tests in the same worker — and an ambient + // value never makes the "unset" case a lie. + const saved = new Map() + + beforeEach(() => { + for (const varName of VARS) { + saved.set(varName, process.env[varName]) + delete process.env[varName] + } + }) + + afterEach(() => { + for (const varName of VARS) { + const original = saved.get(varName) + if (original === undefined) delete process.env[varName] + else process.env[varName] = original + } + }) + + for (const [provider, varName] of CASES) { + it(`changes the ${provider} fingerprint when ${varName} is set`, () => { + const unset = computeEnvFingerprint(provider) + process.env[varName] = '/tmp/codeburn-920-override' + const set = computeEnvFingerprint(provider) + expect(set).not.toBe(unset) + // Round trip: restoring the variable to its original state restores the + // original fingerprint, so the hash is a pure function of the + // environment. + delete process.env[varName] + expect(computeEnvFingerprint(provider)).toBe(unset) + }) + } +}) + // ── fingerprintFile ──────────────────────────────────────────────────── describe('fingerprintFile', () => { From 9c9a37d4bfe21e3b51ed9fb250336c038ab05aaa Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 05:22:50 +0300 Subject: [PATCH 3/4] fix(cache): act on the independent review of the env-fingerprint fix Five findings from a cross-model review of the previous two commits, each verified on the code before acting: Copilot is no longer declared. Declaring anything for it changes its fingerprint, and getOrCreateProviderSection keeps only cached entries whose source path is gone - but OTel discovery returns one source per DB file (copilot.ts:1935) and that DB keeps existing, so the entry would be dropped and re-parsed, destroying conversations Copilot has since pruned from the DB that only the cache still holds. Trading a staleness bug for a data-loss bug is a bad trade; copilot waits for the durable carry-forward to merge instead of drop, and its reads are allowlisted with that reason. The Vercel gateway credentials ARE declared, reversing the previous commit's reasoning, which was wrong: servedSources is seeded with every discovered source (parser.ts:2875) before the network branch, and the network re-fetch (parser.ts:2888) only runs when !readOnly, so a read-only refresh serves the cached report and an undeclared credential keeps reporting the previous account's usage after a swap. Doctor redacts credential values so a key can never reach terminal output or the JSON report. AMBIENT_ENV_VARS narrows to APPDATA and LOCALAPPDATA. Windows sets those for every process so they carry no intent, but the XDG vars are opt-in and do: suppressing them made doctor answer a deliberately relocated XDG_DATA_HOME with "tool likely not installed", which is worse than the noise it avoided. The guard's allowlist is keyed by file and var, not var alone - a var allowlisted for one file silenced every other file's undeclared read of it. Cursor drops its stale XDG_DATA_HOME declaration, which it never reads; its fingerprint already changes here, so this costs no extra migration. cursor-agent keeps its equally stale one, since removing it would force a re-parse to fix nothing. --- CHANGELOG.md | 2 +- src/doctor.ts | 26 +++++-- src/session-cache.ts | 32 ++++++--- tests/doctor.test.ts | 66 ++++++++++++++++++ tests/provider-env-declarations.test.ts | 90 ++++++++++++++++++++----- tests/session-cache.test.ts | 38 +++++++++++ 6 files changed, 222 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebb394bc..b5f4faa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - **`--provider ` no longer leaks Claude spend into the detail panels.** A provider-filtered run still ran the Claude scan, whose orphan pass re-injected every cached Claude session, so By Project / By Model / By Activity showed Claude usage under, e.g., `--provider cursor` while the headline was correct. (#872, thanks @ozymandiashh) - **A degraded session parse no longer freezes daily history.** A read-only parse that served a stale or missing session file was treated as complete and finalized days it never covered, freezing warm-cache ingestion; a corrupt refresh lock is now recovered rather than ending ingestion, and a legitimately idle tail is no longer re-derived on every launch. (#856, thanks @avs-io) - **Pi / Oh My Pi transcripts with a leading title record are discovered.** OMP writes a `type: "title"` line before the session header; discovery now scans a bounded number of leading lines for the first session record instead of requiring it on the first physical line. (#846, #859, thanks @jbspeakr, @avs-io) -- **Nine providers served silently stale numbers after you pointed their env override at a different profile or root.** Kiro, Grok, Kimi, Mux, Mistral Vibe, Zerostack, Codebuff, Goose and Crush each honor an env var that relocates where discovery looks, but the var was never declared in the provider env fingerprint, so the cache section survived the change and kept reporting sessions parsed from the old root — with no diagnostic anywhere. The fix declares those vars, and the adjacent OS-set path variables that resolve a discovery root for Claude, Copilot, IBM Bob, Open Design and Kilo Code on Windows and Linux, plus Cursor's parse-budget override, so the section invalidates when any of them changes. Your next run re-parses all fifteen providers whose declarations changed — the nine above plus Claude, Copilot, Cursor, Open Design, IBM Bob and Kilo Code — once, and only once; `codeburn doctor` still names only deliberate overrides, never OS-set path variables such as APPDATA or XDG_DATA_HOME. (#920) +- **Nine providers served silently stale numbers after you pointed their env override at a different profile or root.** Kiro, Grok, Kimi, Mux, Mistral Vibe, Zerostack, Codebuff, Goose and Crush each honor an env var that relocates where discovery looks, but the var was never declared in the provider env fingerprint, so the cache section survived the change and kept reporting sessions parsed from the old root — with no diagnostic anywhere. The fix declares those vars, the adjacent OS-set path variables that resolve a discovery root for Claude, IBM Bob, Open Design and Kilo Code on Windows and Linux, Cursor's parse-budget override, and the Vercel AI Gateway credential — which must invalidate the fingerprint because a read-only refresh serves the cached report and would otherwise keep reporting the previous account's usage after a swap. Your next run re-parses all fifteen providers whose declarations changed — the nine above plus Claude, Cursor, Open Design, IBM Bob, Kilo Code and Vercel AI Gateway — once, and only once; Copilot is deliberately NOT included, because declaring its overrides would force a re-parse that can drop OTel history only the cache still holds; `codeburn doctor` names deliberate overrides including the XDG_* vars, never the Windows ambient APPDATA / LOCALAPPDATA, and redacts credential values. (#920) ### Fixed - Claude Desktop and Cowork sessions are discovered for Windows Microsoft Store (MSIX) installs. (#611) diff --git a/src/doctor.ts b/src/doctor.ts index e16428af..2a1dfbc2 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -105,11 +105,23 @@ const PARSE_SPAWNS = new Set(['antigravity']) const NON_DISCOVERY_ENV_VARS = new Set(['CODEBURN_CACHE_DIR']) // Ambient platform paths (set by the OS or desktop session for everyone), not -// deliberate user overrides: they are fingerprinted (a change to them does -// move the discovery root, so the cache must invalidate) but doctor must not -// name them as an override, because the probed paths it already prints show -// exactly where CodeBurn looked. -const AMBIENT_ENV_VARS = new Set(['APPDATA', 'LOCALAPPDATA', 'XDG_CONFIG_HOME', 'XDG_DATA_HOME']) +// deliberate user overrides: Windows sets APPDATA and LOCALAPPDATA for every +// process, so they carry no user intent and doctor must not name them as an +// override. The XDG_* vars are the opposite — they are opt-in on Linux, so a +// set value IS a deliberate user override and stays visible: with XDG_DATA_HOME +// pointed at a missing dir, blaming the install instead of the override +// (the pre-#920 behavior) told the user the tool was missing when they had +// deliberately relocated it. All of them are still fingerprinted — a change +// to any of them does move the discovery root, so the cache must invalidate — +// and the probed paths doctor already prints show exactly where CodeBurn +// looked. +const AMBIENT_ENV_VARS = new Set(['APPDATA', 'LOCALAPPDATA']) + +// Credential names whose VALUE must never be printed: knowing whether the +// credential is set is a useful diagnostic, but the value is a live secret. +// Redact at collect time so BOTH the text render and the JSON report are +// covered, and doctor can never leak a key into a bug report or a paste. +const SECRET_ENV_VARS = new Set(['AI_GATEWAY_API_KEY', 'VERCEL_OIDC_TOKEN']) // ── Collect (pure, testable) ───────────────────────────────────────────── @@ -119,7 +131,9 @@ function collectEnvOverrides(providerName: string): DoctorEnvOverride[] { for (const name of vars) { if (AMBIENT_ENV_VARS.has(name)) continue const value = process.env[name] - if (value !== undefined && value !== '') out.push({ name, value }) + if (value !== undefined && value !== '') { + out.push(SECRET_ENV_VARS.has(name) ? { name, value: '' } : { name, value }) + } } return out } diff --git a/src/session-cache.ts b/src/session-cache.ts index 29dcfed1..73b49bfb 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -175,23 +175,32 @@ const TEMP_FILE_MAX_AGE_MS = 5 * 60 * 1000 // computeEnvFingerprint hashes exactly these to decide when a provider's cache // section is stale; a var read by the provider but missing here means changing // it serves the old section silently, reporting nothing from the new root. -// Two reads in src/providers/ are deliberately absent: CODEBURN_VERBOSE -// (sqlite-session-parser.ts) only changes logging verbosity, never parsed -// output, and AI_GATEWAY_API_KEY / VERCEL_OIDC_TOKEN (vercel-gateway.ts) are -// network credentials — vercel-gateway is network:true, and parser.ts:2888 -// short-circuits it past the fingerprint compare, re-fetching its synthetic -// source every run, so no cached section of it can go stale. +// One read in src/providers/ is deliberately absent: CODEBURN_VERBOSE +// (sqlite-session-parser.ts:276) only changes logging verbosity, never parsed +// output. +// +// Copilot is deliberately NOT declared here. Declaring any CODEBURN_COPILOT_* +// var would change its fingerprint, and on a fingerprint change +// getOrCreateProviderSection (src/parser.ts:2650) keeps only the cached +// entries whose source path no longer exists — but copilot's OTel discovery +// returns one source per DB file ({ path: dbPath }, src/providers/copilot.ts:1935) +// and that DB keeps existing, so its cached entry would be dropped and +// re-parsed, destroying conversations Copilot has since pruned from the DB +// that only the cache still holds (see DURABLE_PROVIDER_NAMES below). Do not +// "complete" the map for copilot until the durable carry-forward learns to +// merge instead of drop. export const PROVIDER_ENV_VARS: Record = { claude: ['CLAUDE_CONFIG_DIRS', 'CLAUDE_CONFIG_DIR', 'CODEBURN_DESKTOP_SESSIONS_DIR', 'APPDATA', 'LOCALAPPDATA'], 'cline-cli': ['CLINE_SESSION_DATA_DIR', 'CLINE_DATA_DIR', 'CLINE_DIR'], codebuff: ['CODEBUFF_DATA_DIR'], codewhale: ['CODEWHALE_HOME'], codex: ['CODEX_HOME'], - copilot: ['CODEBURN_COPILOT_SESSION_STATE_DIR', 'CODEBURN_COPILOT_OTEL_DB', 'CODEBURN_COPILOT_JETBRAINS_DIR', 'CODEBURN_COPILOT_WS_STORAGE_DIR', 'CODEBURN_COPILOT_GLOBAL_STORAGE_DIR', 'CODEBURN_COPILOT_DISABLE_OTEL', 'APPDATA', 'LOCALAPPDATA', 'XDG_CONFIG_HOME'], hermes: ['HERMES_HOME'], 'lingtai-tui': ['LINGTAI_HOME', 'LINGTAI_TUI_HOME', 'LINGTAI_TUI_GLOBAL_DIR'], droid: ['FACTORY_DIR'], - cursor: ['XDG_DATA_HOME', 'CODEBURN_CURSOR_MAX_BUBBLES'], + cursor: ['CODEBURN_CURSOR_MAX_BUBBLES'], + // XDG_DATA_HOME is stale here (cursor-agent never reads it) but deliberately + // kept: removing it would force a re-parse to fix nothing. 'cursor-agent': ['XDG_DATA_HOME'], 'open-design': ['CODEBURN_OPEN_DESIGN_DIR', 'APPDATA'], opencode: ['XDG_DATA_HOME', 'OPENCODE_DATA_DIR', 'OPENCODE_DB_PREFIX'], @@ -210,6 +219,13 @@ export const PROVIDER_ENV_VARS: Record = { quickdesk: ['QUICKWORK_HOME'], kimicode: ['KIMI_CODE_HOME'], zerostack: ['ZS_DATA_DIR', 'XDG_DATA_HOME'], + // The gateway credential is a deliberate user override and MUST move the + // fingerprint: a read-only refresh (the refresh-lock fallback) serves the + // cached report straight from the section (parser.ts:2875 seeds servedSources + // before the network re-fetch at parser.ts:2888, which only runs when + // !readOnly), so an undeclared credential would keep serving the previous + // account's usage after a swap — the exact #920 defect. + 'vercel-gateway': ['AI_GATEWAY_API_KEY', 'VERCEL_OIDC_TOKEN'], } // Names of providers whose cache entries are never evicted when source files diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index d8ca4113..131a1628 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from 'os' import { collectDoctorReport, renderDoctorTable, renderDoctorJson } from '../src/doctor.js' import { createCodexProvider } from '../src/providers/codex.js' +import { createOpenCodeProvider } from '../src/providers/opencode.js' import { emptyCache, type SessionCache } from '../src/session-cache.js' import type { Provider, ProbeRoot, SessionSource } from '../src/providers/types.js' @@ -141,6 +142,71 @@ describe('collectDoctorReport - env override', () => { else process.env['CODEX_HOME'] = prev } }) + + it('names a deliberate XDG_DATA_HOME override pointing at a missing dir, blaming the override not the install (opencode)', async () => { + const prev = process.env['XDG_DATA_HOME'] + const bogus = join(tmpDir, 'xdg-missing') + process.env['XDG_DATA_HOME'] = bogus + try { + // Construct after setting env so the provider resolves XDG_DATA_HOME + // (src/providers/opencode.ts:38 reads it to resolve the data dir). + const provider = createOpenCodeProvider() + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, 'opencode') + + expect(r.envOverrides).toContainEqual({ name: 'XDG_DATA_HOME', value: bogus }) + expect(r.status).toBe('empty') + // Regression (Ruling 3 of lane 04): with XDG_DATA_HOME treated as an + // ambient OS var, doctor skipped it and the verdict blamed the install + // ("tool likely not installed") instead of the override the user set. + expect(r.verdict).toContain('override XDG_DATA_HOME set') + expect(r.verdict).toContain('does not exist') + } finally { + if (prev === undefined) delete process.env['XDG_DATA_HOME'] + else process.env['XDG_DATA_HOME'] = prev + } + }) + + it('does not name APPDATA as an override for a provider that declares it', async () => { + const prev = process.env['APPDATA'] + process.env['APPDATA'] = join(tmpDir, 'appdata') + try { + const provider = fakeProvider({ name: 'claude', displayName: 'Claude' }) + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, 'claude') + + // Windows sets APPDATA for every process, so it carries no user intent: + // it is fingerprinted (a change moves the discovery root) but must never + // be named as a deliberate override (Ruling 3 of lane 04). + expect(r.envOverrides.some(o => o.name === 'APPDATA')).toBe(false) + } finally { + if (prev === undefined) delete process.env['APPDATA'] + else process.env['APPDATA'] = prev + } + }) + + it('redacts credential values (AI_GATEWAY_API_KEY) from overrides, the table render, and the JSON report', async () => { + const secret = 'sk-live-very-secret-value-12345' + const prev = process.env['AI_GATEWAY_API_KEY'] + process.env['AI_GATEWAY_API_KEY'] = secret + try { + const provider = fakeProvider({ name: 'vercel-gateway', displayName: 'Vercel AI Gateway', network: true }) + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, 'vercel-gateway') + + // The "is this credential set?" diagnostic is useful; the value is a + // live secret and must never leave doctor (Ruling 2 of lane 04). + expect(r.envOverrides).toContainEqual({ name: 'AI_GATEWAY_API_KEY', value: '' }) + expect(r.envOverrides.some(o => o.value.includes(secret))).toBe(false) + const table = renderDoctorTable(report, { color: false }) + expect(table).toContain('AI_GATEWAY_API_KEY=') + expect(table).not.toContain(secret) + expect(renderDoctorJson(report)).not.toContain(secret) + } finally { + if (prev === undefined) delete process.env['AI_GATEWAY_API_KEY'] + else process.env['AI_GATEWAY_API_KEY'] = prev + } + }) }) // ── Synthetic edge cases ─────────────────────────────────────────────────── diff --git a/tests/provider-env-declarations.test.ts b/tests/provider-env-declarations.test.ts index 2274c90a..672a8e34 100644 --- a/tests/provider-env-declarations.test.ts +++ b/tests/provider-env-declarations.test.ts @@ -5,6 +5,12 @@ // sessions parse but is not fingerprinted means the cache section survives // the change and serves silently stale numbers, exactly the defect class #920 // reported (nine providers slipped through it). +// +// Scoping rule for the allowlist: an entry is keyed '.ts:' and +// silences exactly one var in exactly one file. The same var read in any +// other file is checked against the declarations like every other read, so an +// entry can never mask a second file's undeclared read — the failure mode the +// original global-keyed allowlist had (Ruling 4 of lane 04). import { describe, expect, it } from 'vitest' import { readdirSync, readFileSync } from 'fs' import { dirname, join } from 'path' @@ -30,8 +36,7 @@ const FILE_PROVIDERS: Record = { 'droid.ts': ['droid'], 'hermes.ts': ['hermes'], 'lingtai-tui.ts': ['lingtai-tui'], - // Its only literal read is CODEBURN_CURSOR_MAX_BUBBLES (cursor.ts:692); - // XDG_DATA_HOME is declared for cursor but not read literally in this file. + // Its only literal read is CODEBURN_CURSOR_MAX_BUBBLES (cursor.ts:692). 'cursor.ts': ['cursor'], // The ENV_DIR const (open-design.ts:10) resolves to CODEBURN_OPEN_DESIGN_DIR. 'open-design.ts': ['open-design'], @@ -55,25 +60,41 @@ const FILE_PROVIDERS: Record = { // opencode.ts. Its single read (CODEBURN_VERBOSE) is allowlisted, so this // entry is informational — but required, because the file has reads. 'sqlite-session-parser.ts': ['kilo-code', 'opencode'], - // Registered (lazy) network provider; its credential reads are allowlisted - // (see below) because network sources are re-fetched on every run. + // Registered (lazy) network provider; its credential reads are declared in + // PROVIDER_ENV_VARS (session-cache.ts) so a read-only refresh that serves + // the cached report (parser.ts:2875/2888) cannot keep serving the previous + // account's usage after a swap. 'vercel-gateway.ts': ['vercel-gateway'], } // ── Allowlisted reads ──────────────────────────────────────────────────── // Reads that must NOT invalidate a cache section, one-line reason each. -// If you add an entry here, the guard goes silent for that var — so the -// reason must say exactly why a change to it cannot make a cached section -// stale. +// Scoping rule: a key is '.ts:' — it silences exactly one var in +// exactly one file, and a read of the same var anywhere else is still checked +// against the declarations (see the header comment). If you add an entry here, +// the guard goes silent for that var in that file — the reason must say +// exactly why a change to it cannot make a cached section stale. +// Reason shared by every copilot.ts entry (Ruling 1 of lane 04): copilot is +// deliberately undeclared in PROVIDER_ENV_VARS. Declaring any of its reads +// would change the copilot fingerprint, and on a fingerprint change +// getOrCreateProviderSection (src/parser.ts:2650) keeps only cached entries +// whose source path no longer exists — but OTel discovery returns one source +// per DB file ({ path: dbPath }, copilot.ts:1935) and that DB keeps existing, +// so the cached entry is dropped and re-parsed, destroying conversations +// Copilot has since pruned from the DB that only the cache still holds. +// Deferred until the durable carry-forward learns to merge instead of drop. +const COPILOT_DEFERRED = 'deferred (Ruling 1): declaring it would force the durable re-parse that loses pruned OTel history' const ALLOWLIST: Record = { - CODEBURN_VERBOSE: 'sqlite-session-parser.ts:276 — logging verbosity only; changes no discovered path and no parsed value', - // vercel-gateway is a registered (lazy) provider — not "not a provider" — - // but it is network:true (vercel-gateway.ts:123): its single synthetic - // source is re-fetched on every run and never served from the cached - // section, because parser.ts:2888 short-circuits network providers past the - // fingerprint compare. No fingerprint of it can therefore go stale. - AI_GATEWAY_API_KEY: 'vercel-gateway.ts:20 — network credential; parser.ts:2888 re-fetches every run', - VERCEL_OIDC_TOKEN: 'vercel-gateway.ts:20 — network credential; parser.ts:2888 re-fetches every run', + 'sqlite-session-parser.ts:CODEBURN_VERBOSE': 'sqlite-session-parser.ts:276 — logging verbosity only; changes no discovered path and no parsed value', + 'copilot.ts:CODEBURN_COPILOT_SESSION_STATE_DIR': COPILOT_DEFERRED, + 'copilot.ts:CODEBURN_COPILOT_OTEL_DB': COPILOT_DEFERRED, + 'copilot.ts:CODEBURN_COPILOT_JETBRAINS_DIR': COPILOT_DEFERRED, + 'copilot.ts:CODEBURN_COPILOT_WS_STORAGE_DIR': COPILOT_DEFERRED, + 'copilot.ts:CODEBURN_COPILOT_GLOBAL_STORAGE_DIR': COPILOT_DEFERRED, + 'copilot.ts:CODEBURN_COPILOT_DISABLE_OTEL': COPILOT_DEFERRED, + 'copilot.ts:APPDATA': COPILOT_DEFERRED, + 'copilot.ts:XDG_CONFIG_HOME': COPILOT_DEFERRED, + 'copilot.ts:LOCALAPPDATA': COPILOT_DEFERRED, } // ── Static extraction ─────────────────────────────────────────────────── @@ -166,11 +187,14 @@ describe('provider env declarations (#920)', () => { } for (const { varName, line } of reads) { - if (ALLOWLIST[varName]) continue + // File-scoped: an allowlist entry silences this var in this file only + // (see the header comment); a read of the same var in another file + // must be declared or allowlisted there. + if (ALLOWLIST[`${entry.name}:${varName}`]) continue for (const provider of served) { if (!(PROVIDER_ENV_VARS[provider] ?? []).includes(varName)) { problems.push( - `provider '${provider}' reads process.env['${varName}'] at src/providers/${entry.name}:${line} but it is not declared in PROVIDER_ENV_VARS['${provider}'] — declare it there (it changes what the provider discovers or how its sessions parse) or add it to ALLOWLIST with a reason.`, + `provider '${provider}' reads process.env['${varName}'] at src/providers/${entry.name}:${line} but it is not declared in PROVIDER_ENV_VARS['${provider}'] — declare it there (it changes what the provider discovers or how its sessions parse) or add '${entry.name}:${varName}' to ALLOWLIST with a reason.`, ) } } @@ -194,4 +218,36 @@ describe('provider env declarations (#920)', () => { failWith(problems) expect(problems).toEqual([]) }) + + it('allowlist entries are file-scoped: every key is .ts: shaped, names a real file, and names a var that file actually reads', () => { + const problems: string[] = [] + const providerFiles = new Set( + readdirSync(PROVIDERS_DIR, { withFileTypes: true }) + .filter(e => e.isFile() && e.name.endsWith('.ts')) + .map(e => e.name), + ) + + for (const key of Object.keys(ALLOWLIST)) { + const match = /^([A-Za-z0-9._-]+\.ts):([A-Z0-9_]+)$/.exec(key) + if (!match) { + // A global-keyed entry would mask an undeclared read of the same var + // in any other file (the pre-lane-04 failure mode). Reject it here so + // the scoping rule is enforced, not just documented. + problems.push(`ALLOWLIST key '${key}' is not '.ts:' shaped — an allowlist entry must silence exactly one var in exactly one file.`) + continue + } + const [, fileName, varName] = match + if (!providerFiles.has(fileName!)) { + problems.push(`ALLOWLIST key '${key}' names '${fileName}', which is not a file in src/providers — the entry silences nothing and must be removed.`) + continue + } + const { reads } = extractEnvReads(readFileSync(join(PROVIDERS_DIR, fileName!), 'utf8')) + if (!reads.some(r => r.varName === varName)) { + problems.push(`ALLOWLIST key '${key}' names var '${varName}' but src/providers/${fileName} never reads it — dead entry; remove it.`) + } + } + + failWith(problems) + expect(problems).toEqual([]) + }) }) diff --git a/tests/session-cache.test.ts b/tests/session-cache.test.ts index 1f13e696..dfb3bfd5 100644 --- a/tests/session-cache.test.ts +++ b/tests/session-cache.test.ts @@ -338,6 +338,44 @@ describe('provider env overrides invalidate the fingerprint (#920)', () => { expect(computeEnvFingerprint(provider)).toBe(unset) }) } + + it('changes the vercel-gateway fingerprint when AI_GATEWAY_API_KEY is set', () => { + const prev = process.env['AI_GATEWAY_API_KEY'] + try { + const unset = computeEnvFingerprint('vercel-gateway') + process.env['AI_GATEWAY_API_KEY'] = 'sk-live-secret-abc' + const set = computeEnvFingerprint('vercel-gateway') + expect(set).not.toBe(unset) + delete process.env['AI_GATEWAY_API_KEY'] + expect(computeEnvFingerprint('vercel-gateway')).toBe(unset) + } finally { + if (prev === undefined) delete process.env['AI_GATEWAY_API_KEY'] + else process.env['AI_GATEWAY_API_KEY'] = prev + } + }) + + // Copilot is deliberately NOT declared in PROVIDER_ENV_VARS (Ruling 1 of + // lane 04): its OTel discovery returns one source per DB file + // ({ path: dbPath }, src/providers/copilot.ts:1935), and the durable + // carry-forward in getOrCreateProviderSection (src/parser.ts:2650) drops + // every cached entry whose source still exists on a fingerprint change — so + // declaring any CODEBURN_COPILOT_* var would force a re-parse that destroys + // conversations Copilot has since pruned from the DB, which only the cache + // still holds. The fingerprint must therefore NOT move when one is set. + // This reads as intent, not as an oversight. + it('does not move the copilot fingerprint when CODEBURN_COPILOT_OTEL_DB is set (deliberately undeclared)', () => { + const prev = process.env['CODEBURN_COPILOT_OTEL_DB'] + try { + const before = computeEnvFingerprint('copilot') + process.env['CODEBURN_COPILOT_OTEL_DB'] = '/tmp/codeburn-copilot-otel' + expect(computeEnvFingerprint('copilot')).toBe(before) + delete process.env['CODEBURN_COPILOT_OTEL_DB'] + expect(computeEnvFingerprint('copilot')).toBe(before) + } finally { + if (prev === undefined) delete process.env['CODEBURN_COPILOT_OTEL_DB'] + else process.env['CODEBURN_COPILOT_OTEL_DB'] = prev + } + }) }) // ── fingerprintFile ──────────────────────────────────────────────────── From a67bd279a606ebfd400dad48a5ca2d9a1d14bbb4 Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 05:58:17 +0300 Subject: [PATCH 4/4] test(cache): make the round-2 review findings fail when broken Round 2 of the independent review proved five things by mutation: it broke the behavior and the tests stayed green. Every one is now pinned. The most important invariant in this change was the least guarded. Copilot must have NO entry in PROVIDER_ENV_VARS - declaring any of its nine reads moves its fingerprint and re-opens the durable history-loss path - but only one of the nine was covered, so declaring any of the other eight passed the whole suite. Now the absence of the entry is asserted directly, and all nine vars are table-tested for fingerprint stability. Doctor stops blaming parse-only overrides for a failed discovery. CODEBURN_CURSOR_MAX_BUBBLES caps how many bubbles Cursor parses and KIMI_MODEL_NAME renames an attributed model; neither relocates anything, so "NOTHING FOUND (override CODEBURN_CURSOR_MAX_BUBBLES set...)" pointed the user at the wrong thing. Both join NON_DISCOVERY_ENV_VARS, which exists for exactly this, and both still appear in Details - only the verdict's blame line changes. The secret-redaction and ambient-suppression tests are table-driven over both names each covers, since removing either second name (VERCEL_OIDC_TOKEN, LOCALAPPDATA) previously leaked or surfaced it with every test still passing. The changelog no longer claims a one-time re-parse for the Vercel gateway: it is a network provider re-fetched on every writable run, so its declaration is a read-only-path correction, not a migration. Fourteen file-backed providers migrate once. --- CHANGELOG.md | 2 +- src/doctor.ts | 16 +++-- tests/doctor.test.ts | 123 +++++++++++++++++++++++++----------- tests/session-cache.test.ts | 51 +++++++++++---- 4 files changed, 137 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5f4faa8..0ff0cc23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - **`--provider ` no longer leaks Claude spend into the detail panels.** A provider-filtered run still ran the Claude scan, whose orphan pass re-injected every cached Claude session, so By Project / By Model / By Activity showed Claude usage under, e.g., `--provider cursor` while the headline was correct. (#872, thanks @ozymandiashh) - **A degraded session parse no longer freezes daily history.** A read-only parse that served a stale or missing session file was treated as complete and finalized days it never covered, freezing warm-cache ingestion; a corrupt refresh lock is now recovered rather than ending ingestion, and a legitimately idle tail is no longer re-derived on every launch. (#856, thanks @avs-io) - **Pi / Oh My Pi transcripts with a leading title record are discovered.** OMP writes a `type: "title"` line before the session header; discovery now scans a bounded number of leading lines for the first session record instead of requiring it on the first physical line. (#846, #859, thanks @jbspeakr, @avs-io) -- **Nine providers served silently stale numbers after you pointed their env override at a different profile or root.** Kiro, Grok, Kimi, Mux, Mistral Vibe, Zerostack, Codebuff, Goose and Crush each honor an env var that relocates where discovery looks, but the var was never declared in the provider env fingerprint, so the cache section survived the change and kept reporting sessions parsed from the old root — with no diagnostic anywhere. The fix declares those vars, the adjacent OS-set path variables that resolve a discovery root for Claude, IBM Bob, Open Design and Kilo Code on Windows and Linux, Cursor's parse-budget override, and the Vercel AI Gateway credential — which must invalidate the fingerprint because a read-only refresh serves the cached report and would otherwise keep reporting the previous account's usage after a swap. Your next run re-parses all fifteen providers whose declarations changed — the nine above plus Claude, Cursor, Open Design, IBM Bob, Kilo Code and Vercel AI Gateway — once, and only once; Copilot is deliberately NOT included, because declaring its overrides would force a re-parse that can drop OTel history only the cache still holds; `codeburn doctor` names deliberate overrides including the XDG_* vars, never the Windows ambient APPDATA / LOCALAPPDATA, and redacts credential values. (#920) +- **Nine providers served silently stale numbers after you pointed their env override at a different profile or root.** Kiro, Grok, Kimi, Mux, Mistral Vibe, Zerostack, Codebuff, Goose and Crush each honor an env var that relocates where discovery looks, but the var was never declared in the provider env fingerprint, so the cache section survived the change and kept reporting sessions parsed from the old root — with no diagnostic anywhere. The fix declares those vars, the adjacent OS-set path variables that resolve a discovery root for Claude, IBM Bob, Open Design and Kilo Code on Windows and Linux, Cursor's parse-budget override, and the Vercel AI Gateway credential — which must invalidate the fingerprint because a read-only refresh serves the cached report and would otherwise keep reporting the previous account's usage after a swap. Your next run re-parses the fourteen file-backed providers whose declarations changed — the nine above plus Claude, Cursor, Open Design, IBM Bob and Kilo Code — once, and only once; the Vercel AI Gateway declaration is a read-only-path correction, not a migration (its report is re-fetched on every writable run anyway); Copilot is deliberately NOT included, because declaring its overrides would force a re-parse that can drop OTel history only the cache still holds; `codeburn doctor` names deliberate overrides including the XDG_* vars, never the Windows ambient APPDATA / LOCALAPPDATA, and redacts credential values. (#920) ### Fixed - Claude Desktop and Cowork sessions are discovered for Windows Microsoft Store (MSIX) installs. (#611) diff --git a/src/doctor.ts b/src/doctor.ts index 2a1dfbc2..95a3d44d 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -99,10 +99,18 @@ const PARSE_CALL_CAP = 500 // (readdir/stat only) still runs, so session counts stay meaningful. const PARSE_SPAWNS = new Set(['antigravity']) -// CodeBurn's own cache location: listed in PROVIDER_ENV_VARS for cache -// fingerprinting, but it is not a discovery path, so it must never be blamed -// in a NOTHING FOUND hint. -const NON_DISCOVERY_ENV_VARS = new Set(['CODEBURN_CACHE_DIR']) +// Vars listed in PROVIDER_ENV_VARS for cache fingerprinting that are NOT +// discovery paths: a change to them can never explain "nothing was +// discovered", so they must never be blamed in a NOTHING FOUND hint. +// - CODEBURN_CACHE_DIR: CodeBurn's own cache location — where the cache +// file lives, not where sessions are discovered. +// - CODEBURN_CURSOR_MAX_BUBBLES: caps how many bubbles Cursor parses +// (src/providers/cursor.ts:692) — a parse budget, not a discovery root. +// - KIMI_MODEL_NAME: renames the model attributed to Kimi sessions +// (src/providers/kimi.ts:155) — attribution, not discovery. +// All three still appear in the Details block; only the verdict's blame line +// is cleared of them. +const NON_DISCOVERY_ENV_VARS = new Set(['CODEBURN_CACHE_DIR', 'CODEBURN_CURSOR_MAX_BUBBLES', 'KIMI_MODEL_NAME']) // Ambient platform paths (set by the OS or desktop session for everyone), not // deliberate user overrides: Windows sets APPDATA and LOCALAPPDATA for every diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 131a1628..0ccbc874 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -167,46 +167,93 @@ describe('collectDoctorReport - env override', () => { } }) - it('does not name APPDATA as an override for a provider that declares it', async () => { - const prev = process.env['APPDATA'] - process.env['APPDATA'] = join(tmpDir, 'appdata') - try { - const provider = fakeProvider({ name: 'claude', displayName: 'Claude' }) - const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) - const r = only(report, 'claude') - - // Windows sets APPDATA for every process, so it carries no user intent: - // it is fingerprinted (a change moves the discovery root) but must never - // be named as a deliberate override (Ruling 3 of lane 04). - expect(r.envOverrides.some(o => o.name === 'APPDATA')).toBe(false) - } finally { - if (prev === undefined) delete process.env['APPDATA'] - else process.env['APPDATA'] = prev - } - }) + // Windows sets APPDATA and LOCALAPPDATA for every process, so neither + // carries user intent: both are fingerprinted (a change moves the discovery + // root) but must never be named as a deliberate override (Ruling 3 of lane + // 04). Table-driven over both so removing either from AMBIENT_ENV_VARS + // fails a test instead of leaking it into the overrides list. + for (const varName of ['APPDATA', 'LOCALAPPDATA']) { + it(`does not name ${varName} as an override for a provider that declares it`, async () => { + const prev = process.env[varName] + process.env[varName] = join(tmpDir, varName.toLowerCase()) + try { + const provider = fakeProvider({ name: 'claude', displayName: 'Claude' }) + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, 'claude') + + expect(r.envOverrides.some(o => o.name === varName)).toBe(false) + } finally { + if (prev === undefined) delete process.env[varName] + else process.env[varName] = prev + } + }) + } - it('redacts credential values (AI_GATEWAY_API_KEY) from overrides, the table render, and the JSON report', async () => { - const secret = 'sk-live-very-secret-value-12345' - const prev = process.env['AI_GATEWAY_API_KEY'] - process.env['AI_GATEWAY_API_KEY'] = secret - try { - const provider = fakeProvider({ name: 'vercel-gateway', displayName: 'Vercel AI Gateway', network: true }) - const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) - const r = only(report, 'vercel-gateway') + // Every credential in SECRET_ENV_VARS must be redacted at collect time so + // neither the text render nor the JSON report can leak it (Ruling 2 of lane + // 04). Table-driven over both, so a credential added to the set without a + // redaction test fails here instead of leaking into a bug report. + for (const varName of ['AI_GATEWAY_API_KEY', 'VERCEL_OIDC_TOKEN']) { + it(`redacts credential values (${varName}) from overrides, the table render, and the JSON report`, async () => { + const secret = `sk-live-${varName}-value-12345` + const prev = process.env[varName] + const sibling = varName === 'AI_GATEWAY_API_KEY' ? 'VERCEL_OIDC_TOKEN' : 'AI_GATEWAY_API_KEY' + const prevSibling = process.env[sibling] + process.env[varName] = secret + // Isolate the case under test: a stray ambient sibling must not change + // what this case observes. + delete process.env[sibling] + try { + const provider = fakeProvider({ name: 'vercel-gateway', displayName: 'Vercel AI Gateway', network: true }) + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, 'vercel-gateway') + + // The "is this credential set?" diagnostic is useful; the value is a + // live secret and must never leave doctor (Ruling 2 of lane 04). + expect(r.envOverrides).toContainEqual({ name: varName, value: '' }) + expect(r.envOverrides.some(o => o.value.includes(secret))).toBe(false) + const table = renderDoctorTable(report, { color: false }) + expect(table).toContain(`${varName}=`) + expect(table).not.toContain(secret) + expect(renderDoctorJson(report)).not.toContain(secret) + } finally { + if (prev === undefined) delete process.env[varName] + else process.env[varName] = prev + if (prevSibling === undefined) delete process.env[sibling] + else process.env[sibling] = prevSibling + } + }) + } - // The "is this credential set?" diagnostic is useful; the value is a - // live secret and must never leave doctor (Ruling 2 of lane 04). - expect(r.envOverrides).toContainEqual({ name: 'AI_GATEWAY_API_KEY', value: '' }) - expect(r.envOverrides.some(o => o.value.includes(secret))).toBe(false) - const table = renderDoctorTable(report, { color: false }) - expect(table).toContain('AI_GATEWAY_API_KEY=') - expect(table).not.toContain(secret) - expect(renderDoctorJson(report)).not.toContain(secret) - } finally { - if (prev === undefined) delete process.env['AI_GATEWAY_API_KEY'] - else process.env['AI_GATEWAY_API_KEY'] = prev - } - }) + // CODEBURN_CURSOR_MAX_BUBBLES caps how many bubbles Cursor parses + // (src/providers/cursor.ts:692) and KIMI_MODEL_NAME renames the model + // attributed to Kimi sessions (src/providers/kimi.ts:155): both are + // fingerprinted but cannot explain why nothing was discovered, so the + // verdict must not name them — while Details still lists them, because they + // ARE overrides in force. Each is asserted through the provider that + // declares it. + for (const [varName, providerName, displayName, value] of [ + ['CODEBURN_CURSOR_MAX_BUBBLES', 'cursor', 'Cursor', '5000'], + ['KIMI_MODEL_NAME', 'kimi', 'Kimi', 'kimi-latest-920'], + ] as const) { + it(`does not blame ${varName} for an empty ${displayName} (not a discovery path)`, async () => { + const prev = process.env[varName] + process.env[varName] = value + try { + const provider = fakeProvider({ name: providerName, displayName }) + const report = await collectDoctorReport('all', { providers: [provider], cache: emptyCache() }) + const r = only(report, providerName) + + expect(r.envOverrides).toContainEqual({ name: varName, value }) + expect(r.verdict).not.toContain(varName) + const table = renderDoctorTable(report, { color: false }) + expect(table).toContain(`${varName}=${value}`) + } finally { + if (prev === undefined) delete process.env[varName] + else process.env[varName] = prev + } + }) + } }) // ── Synthetic edge cases ─────────────────────────────────────────────────── diff --git a/tests/session-cache.test.ts b/tests/session-cache.test.ts index dfb3bfd5..4320e4ed 100644 --- a/tests/session-cache.test.ts +++ b/tests/session-cache.test.ts @@ -6,6 +6,7 @@ import { basename, join } from 'path' import { CACHE_VERSION, + PROVIDER_ENV_VARS, type CachedCall, type CachedFile, type CachedTurn, @@ -362,18 +363,44 @@ describe('provider env overrides invalidate the fingerprint (#920)', () => { // declaring any CODEBURN_COPILOT_* var would force a re-parse that destroys // conversations Copilot has since pruned from the DB, which only the cache // still holds. The fingerprint must therefore NOT move when one is set. - // This reads as intent, not as an oversight. - it('does not move the copilot fingerprint when CODEBURN_COPILOT_OTEL_DB is set (deliberately undeclared)', () => { - const prev = process.env['CODEBURN_COPILOT_OTEL_DB'] - try { - const before = computeEnvFingerprint('copilot') - process.env['CODEBURN_COPILOT_OTEL_DB'] = '/tmp/codeburn-copilot-otel' - expect(computeEnvFingerprint('copilot')).toBe(before) - delete process.env['CODEBURN_COPILOT_OTEL_DB'] - expect(computeEnvFingerprint('copilot')).toBe(before) - } finally { - if (prev === undefined) delete process.env['CODEBURN_COPILOT_OTEL_DB'] - else process.env['CODEBURN_COPILOT_OTEL_DB'] = prev + // This reads as intent, not as an oversight — and the assertions below pin + // the WHOLE invariant (no entry at all, plus every one of the nine deferred + // reads), so a future "completing" edit fails a test instead of silently + // re-opening the durable history-loss path. + describe('copilot is deliberately undeclared in PROVIDER_ENV_VARS', () => { + it('has no PROVIDER_ENV_VARS entry at all', () => { + expect(PROVIDER_ENV_VARS['copilot']).toBeUndefined() + }) + + // The nine reads copilot.ts performs whose declaration is deferred (each + // is allowlisted in tests/provider-env-declarations.test.ts): setting any + // of them must leave the copilot fingerprint untouched. + const DEFERRED_COPILOT_VARS = [ + 'CODEBURN_COPILOT_SESSION_STATE_DIR', + 'CODEBURN_COPILOT_OTEL_DB', + 'CODEBURN_COPILOT_JETBRAINS_DIR', + 'CODEBURN_COPILOT_WS_STORAGE_DIR', + 'CODEBURN_COPILOT_GLOBAL_STORAGE_DIR', + 'CODEBURN_COPILOT_DISABLE_OTEL', + 'APPDATA', + 'LOCALAPPDATA', + 'XDG_CONFIG_HOME', + ] + + for (const varName of DEFERRED_COPILOT_VARS) { + it(`does not move the copilot fingerprint when ${varName} is set (deliberately undeclared)`, () => { + const prev = process.env[varName] + try { + const before = computeEnvFingerprint('copilot') + process.env[varName] = `/tmp/codeburn-copilot-920/${varName}` + expect(computeEnvFingerprint('copilot')).toBe(before) + delete process.env[varName] + expect(computeEnvFingerprint('copilot')).toBe(before) + } finally { + if (prev === undefined) delete process.env[varName] + else process.env[varName] = prev + } + }) } }) })