From 1487c75fa42d4afc8d6de789f1b080696a70b68a Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Sat, 29 Aug 2026 03:36:07 +0530 Subject: [PATCH] fix(tui): clean cache locks on confirmed exit (#1165) --- src/cache-refresh-lock.ts | 7 + src/dashboard.tsx | 19 +-- src/session-cache.ts | 11 +- tests/cache-refresh-lock-process.test.ts | 28 +++- tests/dashboard-process-exit.test.ts | 178 +++++++++++++++++++++++ tests/fixtures/cache-refresh-worker.ts | 8 +- 6 files changed, 237 insertions(+), 14 deletions(-) create mode 100644 tests/dashboard-process-exit.test.ts diff --git a/src/cache-refresh-lock.ts b/src/cache-refresh-lock.ts index 02f3d41d..8c9d4a3d 100644 --- a/src/cache-refresh-lock.ts +++ b/src/cache-refresh-lock.ts @@ -89,6 +89,13 @@ function removeOurLockSync(): void { } } +/** Release every warm-refresh lock owned by this process before a caller uses + * `process.exit()`. Signal handlers already cover SIGINT/SIGTERM; the TUI's q + * path exits directly after restoring the terminal and needs the same cleanup. */ +export function releaseOwnedRefreshLocksForExit(): void { + removeOurLockSync() +} + // Arm once, only while we hold the lock: on a catchable termination (Ctrl-C, or // the menubar/desktop watchdog's SIGTERM) clean our lock before dying so a // killed refresh leaves no leftover. SIGKILL can't be caught, so that path still diff --git a/src/dashboard.tsx b/src/dashboard.tsx index d602e599..f72ae5f1 100644 --- a/src/dashboard.tsx +++ b/src/dashboard.tsx @@ -11,6 +11,7 @@ import { findUnpricedModels, isExpectedFreeModel, loadPricing } from './models.j import { aggregateModelTotals } from './model-breakdown.js' import { buildDurableOverviewFromNormalizedIndex, buildDurablePeriod, hydrateDailyCacheFromNormalizedProjects } from './usage-aggregator.js' import { loadDailyCache, type DailyCache } from './daily-cache.js' +import { exitAfterCacheCleanup } from './session-cache.js' import { getAllProviders } from './providers/index.js' import { classHeaderLine, classTotals, findingBasis, findingClass, scanAndDetect, type FindingClass, type WasteFinding, type WasteAction, type OptimizeResult } from './optimize.js' import { appliedFixGlyph, formatAppliedFix, type AppliedFix } from './act/types.js' @@ -1562,7 +1563,7 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje autoFallbackFromEmptyToday?: boolean /// CLI-only hard stop after Ink has been asked to restore the terminal. /// Component tests and embedders omit it and receive ordinary Ink exit. - terminateProcess?: () => void + terminateProcess?: (exitCode: number) => void }) { const { exit } = useApp() const [period, setPeriod] = useState(initialPeriod) @@ -1958,14 +1959,14 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje }, [switchDay]) useInput((input, key) => { - const quitNow = (): void => { + const quitNow = (exitCode: number): void => { exit() - terminateProcess?.() + terminateProcess?.(exitCode) } // #1143: Ctrl+C always exits immediately, regardless of fill state. The // #1109 abrupt path is kill-safe (nothing marked seen without being // parsed, resume converges), so this is the unconditional escape hatch. - if (key.ctrl && input === 'c') { quitNow(); return } + if (key.ctrl && input === 'c') { quitNow(130); return } // First q during an active fill: arm confirmation, do not exit. The fill // keeps running so the next launch starts warm. A second q takes the // abrupt path; q with no fill active exits immediately (no flicker). @@ -1973,12 +1974,12 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje if (indexing) { // A ref makes two q bytes decisive even when the background scan keeps // React from committing the confirmation state between them. - if (quitArmedRef.current) { quitNow(); return } + if (quitArmedRef.current) { quitNow(0); return } quitArmedRef.current = true setQuitArmed(true) return } - quitNow() + quitNow(0) return } if (input === 'o' && view === 'dashboard' && optimizeAvailable) { void loadOptimizeResult(); return } @@ -2438,16 +2439,16 @@ export async function renderDashboard(period: Period = 'week', provider: string const hardQuitGuard = (chunk: string | Buffer): void => { const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) for (const byte of bytes) { - if (byte === 3) setImmediate(() => process.exit(0)) + if (byte === 3) setImmediate(() => exitAfterCacheCleanup(130)) if (byte !== 113) continue const now = Date.now() - if (now - lastQuitByteAt <= 2000) setImmediate(() => process.exit(0)) + if (now - lastQuitByteAt <= 2000) setImmediate(() => exitAfterCacheCleanup(0)) lastQuitByteAt = now } } process.stdin.on('data', hardQuitGuard) const app = renderDebouncedInteractive(process.stdout, ({ columns }) => ( - { setImmediate(() => process.exit(0)) }} /> + { setImmediate(() => exitAfterCacheCleanup(exitCode)) }} /> )) try { await app.waitUntilExit() diff --git a/src/session-cache.ts b/src/session-cache.ts index b6ab1296..045d9f82 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -4,7 +4,7 @@ import { createHash, randomBytes } from 'crypto' import { join } from 'path' import { getCodeburnCacheDir } from './cache-dir.js' -import { acquireCacheRefreshLock } from './cache-refresh-lock.js' +import { acquireCacheRefreshLock, releaseOwnedRefreshLocksForExit } from './cache-refresh-lock.js' import type { ToolCall } from './types.js' // ── Types ────────────────────────────────────────────────────────────── @@ -1697,6 +1697,15 @@ function removeOurLockSync(): void { } catch { /* best-effort; nothing to clean or already gone */ } } +/** Terminate an interactive CLI after synchronously releasing both cache-lock + * families it can own. The process cannot wait for background parsing to drain, + * but a direct exit must not make the next launch recover a stale live-pid lock. */ +export function exitAfterCacheCleanup(exitCode: number): never { + releaseOwnedRefreshLocksForExit() + removeOurLockSync() + process.exit(exitCode) +} + // Arm once, only while we hold the lock: on a catchable termination (Ctrl-C, or a // SIGTERM from a parent) clean our lock before dying so a killed cold parse leaves // no leftover. SIGKILL can't be caught, so that path still relies on the next cold diff --git a/tests/cache-refresh-lock-process.test.ts b/tests/cache-refresh-lock-process.test.ts index 8d6d25ac..15c4358d 100644 --- a/tests/cache-refresh-lock-process.test.ts +++ b/tests/cache-refresh-lock-process.test.ts @@ -36,8 +36,8 @@ function waitForExit(child: ChildProcess): Promise { }) } -function worker(cacheDir: string, barriers: string, id: string, source: string, bypass = false): ChildProcess { - return spawn(process.execPath, ['--import', 'tsx', join(process.cwd(), 'tests/fixtures/cache-refresh-worker.ts'), cacheDir, barriers, id, source, String(bypass)], { +function worker(cacheDir: string, barriers: string, id: string, source: string, bypass = false, exitViaCleanup = false): ChildProcess { + return spawn(process.execPath, ['--import', 'tsx', join(process.cwd(), 'tests/fixtures/cache-refresh-worker.ts'), cacheDir, barriers, id, source, String(bypass), String(exitViaCleanup)], { cwd: process.cwd(), stdio: ['ignore', 'ignore', 'pipe'], }) @@ -159,6 +159,30 @@ describe('warm refresh child-process regression', () => { expect(existsSync(lock)).toBe(false) }) + it('unlinks its own warm-refresh lock on the TUI direct-exit boundary', async () => { + const root = await mkdtemp(join(tmpdir(), 'cb-refresh-direct-exit-')) + roots.push(root) + const cacheDir = join(root, 'cache') + const barriers = join(root, 'barriers') + await mkdir(cacheDir, { recursive: true }) + await mkdir(barriers, { recursive: true }) + process.env['CODEBURN_CACHE_DIR'] = cacheDir + const initial = emptyCache() + initial.complete = true + await saveCache(initial) + const source = join(root, 'changed.json') + await writeFile(source, JSON.stringify({ output: 606 })) + + const holder = worker(cacheDir, barriers, 'a', source, false, true) + await waitFor(join(barriers, 'a.parsed')) + const lock = join(cacheDir, 'session-refresh.lock') + expect(existsSync(lock)).toBe(true) + + await writeFile(join(barriers, 'a.exit'), '') + await waitForExit(holder) + expect(existsSync(lock)).toBe(false) + }) + it('serializes disjoint parsed updates so the later publication cannot drop the first', async () => { const root = await mkdtemp(join(tmpdir(), 'cb-refresh-process-')) roots.push(root) diff --git a/tests/dashboard-process-exit.test.ts b/tests/dashboard-process-exit.test.ts new file mode 100644 index 00000000..c800ef1d --- /dev/null +++ b/tests/dashboard-process-exit.test.ts @@ -0,0 +1,178 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { mkdir, mkdtemp, rm, stat, utimes, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, describe, expect, it } from 'vitest' + +type RunningDashboard = { + child: ChildProcess + home: string + hydrationLock: string + readOutput: () => string +} + +const running: RunningDashboard[] = [] + +async function waitFor( + predicate: () => boolean | Promise, + timeoutMs: number, + message: string, +): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + if (await predicate()) return + await new Promise(resolve => setTimeout(resolve, 10)) + } + throw new Error(message) +} + +async function pathExists(path: string): Promise { + try { + await stat(path) + return true + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false + throw error + } +} + +async function waitForExit(child: ChildProcess, timeoutMs: number, readOutput: () => string): Promise<{ + code: number | null + signal: NodeJS.Signals | null + elapsedMs: number +}> { + const startedAt = performance.now() + return await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error( + `dashboard process ${child.pid ?? 'unknown'} did not exit within ${timeoutMs} ms; output:\n${readOutput().slice(-2_000)}`, + )) + }, timeoutMs) + child.once('exit', (code, signal) => { + clearTimeout(timeout) + resolve({ code, signal, elapsedMs: performance.now() - startedAt }) + }) + }) +} + +async function startHydratingDashboard(): Promise { + const home = await mkdtemp(join(tmpdir(), 'codeburn-dashboard-exit-')) + const sessionsDir = join(home, '.claude', 'projects', 'fixture') + const cacheDir = join(home, 'cache') + await mkdir(sessionsDir, { recursive: true }) + + // Keep the real background hydration in flight long enough to exercise the + // public keystroke-to-process-exit seam without mocking cache ownership. + const body = '{}\n'.repeat(10_000) + const old = new Date(Date.now() - 200 * 24 * 60 * 60 * 1_000) + for (let batch = 0; batch < 40; batch++) { + await Promise.all(Array.from({ length: 50 }, async (_, offset) => { + const sessionPath = join(sessionsDir, `old-${batch * 50 + offset}.jsonl`) + await writeFile(sessionPath, body) + await utimes(sessionPath, old, old) + })) + } + + const bootstrap = ` + const define = (target, key, value) => Object.defineProperty(target, key, { configurable: true, value }); + define(process.stdin, 'isTTY', true); + define(process.stdout, 'isTTY', true); + define(process.stdout, 'columns', 120); + define(process.stdout, 'rows', 50); + if (typeof process.stdin.setRawMode !== 'function') define(process.stdin, 'setRawMode', enabled => { + process.stderr.write('CODEBURN_RAW_MODE ' + String(enabled) + '\\n'); + return process.stdin; + }); + ` + const ttyPreload = `data:text/javascript,${encodeURIComponent(bootstrap)}` + const child = spawn(process.execPath, [ + '--import', 'tsx', + '--import', ttyPreload, + join(process.cwd(), 'src', 'cli.ts'), + 'report', '--period', 'today', '--refresh', '0', + ], { + cwd: process.cwd(), + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + HOME: home, + CLAUDE_CONFIG_DIR: join(home, '.claude'), + CODEBURN_CACHE_DIR: cacheDir, + CODEBURN_DESKTOP_SESSIONS_DIR: join(home, 'desktop-sessions'), + CODEBURN_PARSE_WORKERS: '0', + CODEBURN_PRICING_SNAPSHOT_ONLY: '1', + CODEBURN_FX_NO_FETCH: '1', + CODEBURN_VERBOSE: '1', + FORCE_COLOR: '0', + TZ: 'UTC', + }, + }) + + let output = '' + child.stdout?.setEncoding('utf8') + child.stderr?.setEncoding('utf8') + child.stdout?.on('data', chunk => { output += String(chunk) }) + child.stderr?.on('data', chunk => { output += String(chunk) }) + + const dashboard = { child, home, hydrationLock: join(cacheDir, 'hydrating.lock'), readOutput: () => output } + running.push(dashboard) + await waitFor( + () => output.includes('progressive startup on') && pathExists(dashboard.hydrationLock), + 15_000, + `dashboard never entered background hydration; output:\n${output.slice(-2_000)}`, + ) + return dashboard +} + +afterEach(async () => { + await Promise.all(running.splice(0).map(async ({ child, home }) => { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL') + await new Promise(resolve => { + if (child.exitCode !== null || child.signalCode !== null) resolve() + else child.once('exit', () => resolve()) + }) + await rm(home, { recursive: true, force: true }) + })) +}) + +describe('interactive dashboard process exit during cold hydration', () => { + function expectTerminalTeardown(output: string): void { + expect(output).toContain('CODEBURN_RAW_MODE true') + expect(output).toContain('CODEBURN_RAW_MODE false') + expect(output).toContain('\x1b[?1006l\x1b[?1000l') + expect(output).toContain('\x1b[?1049l') + expect(output).toContain('\x1b[?25h') + } + + it('removes the owned hydration lock when the confirmed q exit returns control', async () => { + const { child, hydrationLock, readOutput } = await startHydratingDashboard() + + child.stdin?.write('q') + await waitFor( + () => readOutput().includes('Finishing background index'), + 1_000, + `dashboard did not render quit confirmation; output:\n${readOutput().slice(-2_000)}`, + ) + expect(await pathExists(hydrationLock)).toBe(true) + child.stdin?.write('q') + const exited = await waitForExit(child, 1_000, readOutput) + + expect(exited.elapsedMs).toBeLessThan(1_000) + expect({ code: exited.code, signal: exited.signal }).toEqual({ code: 0, signal: null }) + expect(await pathExists(hydrationLock)).toBe(false) + expectTerminalTeardown(readOutput()) + }, 30_000) + + it('removes the owned hydration lock and restores the terminal on raw Ctrl-C', async () => { + const { child, hydrationLock, readOutput } = await startHydratingDashboard() + + child.stdin?.write('\x03') + const exited = await waitForExit(child, 1_000, readOutput) + + expect(exited.elapsedMs).toBeLessThan(1_000) + expect({ code: exited.code, signal: exited.signal }).toEqual({ code: 130, signal: null }) + expect(await pathExists(hydrationLock)).toBe(false) + expectTerminalTeardown(readOutput()) + }, 30_000) +}) diff --git a/tests/fixtures/cache-refresh-worker.ts b/tests/fixtures/cache-refresh-worker.ts index 1a24c0c4..25506fa2 100644 --- a/tests/fixtures/cache-refresh-worker.ts +++ b/tests/fixtures/cache-refresh-worker.ts @@ -3,9 +3,9 @@ import { mkdir, readFile, writeFile } from 'fs/promises' import { join } from 'path' import { acquireCacheRefreshLock } from '../../src/cache-refresh-lock.js' -import { loadCache, markCacheDirty, saveCache } from '../../src/session-cache.js' +import { exitAfterCacheCleanup, loadCache, markCacheDirty, saveCache } from '../../src/session-cache.js' -const [cacheDir, barrierDir, id, sourcePath, bypass = 'false'] = process.argv.slice(2) +const [cacheDir, barrierDir, id, sourcePath, bypass = 'false', exitViaCleanup = 'false'] = process.argv.slice(2) if (!cacheDir || !barrierDir || !id || !sourcePath) throw new Error('missing worker argument') async function waitFor(name: string): Promise { @@ -35,6 +35,10 @@ try { } markCacheDirty(cache, 'regression') await writeFile(join(barrierDir, `${id}.parsed`), '') + if (exitViaCleanup === 'true') { + await waitFor(`${id}.exit`) + exitAfterCacheCleanup(0) + } await waitFor(`${id}.save`) const published = await saveCache(cache, refresh?.handle.verifyStillOwner) await writeFile(join(barrierDir, `${id}.${published ? 'published' : 'fenced'}`), '')