mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-08-30 02:43:34 +00:00
fix(tui): clean cache locks on confirmed exit (#1165)
This commit is contained in:
parent
9a929c87ab
commit
1487c75fa4
6 changed files with 237 additions and 14 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<Period>(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 }) => (
|
||||
<InteractiveDashboard initialProjects={filteredProjects} initialDailyHistoryProjects={scrollableDailyHistory ? scannedProjects : undefined} initialPeriod={opened} initialProvider={provider} initialPlanUsages={planUsages} initialDurable={initialDurable} refreshSeconds={refreshSeconds} projectFilter={projectFilter} excludeFilter={excludeFilter} customRange={customRange} customRangeLabel={customRangeLabel} initialDay={initialDay} windowColumns={columns} initialIndexPendingFiles={paint.deferredFiles} initialHistoryIndexing={progressive} initialCacheWasCold={cacheWasCold} autoFallbackFromEmptyToday={auto} terminateProcess={() => { setImmediate(() => process.exit(0)) }} />
|
||||
<InteractiveDashboard initialProjects={filteredProjects} initialDailyHistoryProjects={scrollableDailyHistory ? scannedProjects : undefined} initialPeriod={opened} initialProvider={provider} initialPlanUsages={planUsages} initialDurable={initialDurable} refreshSeconds={refreshSeconds} projectFilter={projectFilter} excludeFilter={excludeFilter} customRange={customRange} customRangeLabel={customRangeLabel} initialDay={initialDay} windowColumns={columns} initialIndexPendingFiles={paint.deferredFiles} initialHistoryIndexing={progressive} initialCacheWasCold={cacheWasCold} autoFallbackFromEmptyToday={auto} terminateProcess={exitCode => { setImmediate(() => exitAfterCacheCleanup(exitCode)) }} />
|
||||
))
|
||||
try {
|
||||
await app.waitUntilExit()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -36,8 +36,8 @@ function waitForExit(child: ChildProcess): Promise<void> {
|
|||
})
|
||||
}
|
||||
|
||||
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)
|
||||
|
|
|
|||
178
tests/dashboard-process-exit.test.ts
Normal file
178
tests/dashboard-process-exit.test.ts
Normal file
|
|
@ -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<boolean>,
|
||||
timeoutMs: number,
|
||||
message: string,
|
||||
): Promise<void> {
|
||||
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<boolean> {
|
||||
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<RunningDashboard> {
|
||||
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<void>(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)
|
||||
})
|
||||
8
tests/fixtures/cache-refresh-worker.ts
vendored
8
tests/fixtures/cache-refresh-worker.ts
vendored
|
|
@ -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<void> {
|
||||
|
|
@ -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'}`), '')
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue