mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-08-22 23:14:33 +00:00
fix: address adversarial review of the timeout watchdog (F1-F7)
F1 - `codeburn serve` lost an in-flight request's response when stdin closed mid-flight: runStdioServe returned before its queue drained, and the explicit process.exit(0) then hit runCaptured's monkeypatched exit, throwing ExitSignal and exiting 1 with the frame never written. The finally now awaits the queue. F2 - the SIGTERM rationale was false. armSignalCleanup unlinks the refresh lock and re-raises; it publishes no partial cache, and a lock left by SIGKILL already self-heals through the stale-pid takeover. SIGTERM-first is kept for the real (smaller) benefit - a clean lock release instead of a takeover - and every comment plus the CHANGELOG now says only that. F3 - the cold gate had no exit. overviewWarmed only flips on success, so an install that can never hydrate sat behind an indexing splash forever with no error and no route to the CLI recovery. The cold claim now expires with the cold window itself. F4 - the real CLI does not heartbeat the way the demo did: a cold parse's inter-provider cache save measured 31.6s of total silence, which a 45s window survives only until the machine is 1.5x slower. Under CODEBURN_PROGRESS a running parse now emits a keepalive every 10s regardless of phase, so silence genuinely means stopped. Consumers that do not know the event ignore it. F5 - the orphan-reap identity check matched any `cli.js` running `serve`. The pidfile now records the exact argv and `ps -ww` must match it exactly. F6 - bump() re-armed the watchdog after settle, leaving a timer finish() never clears when a killed child's buffered output landed. F7 - kill paths dropped the child from activeChildren before SIGTERM and the SIGKILL backstop was unref'd, so a quit inside the 5s grace orphaned a child that ignores SIGTERM. It now stays registered until it actually dies. N8 - the silence test wrote its only byte at t~0, so it passed without the re-arm. The byte now lands mid-window and the kill is asserted from it. N9 - documented why mutations keep a plain total cap.
This commit is contained in:
iamtoruk
parent
d8bd428054
commit
3317189dd6
11 changed files with 298 additions and 34 deletions
|
|
@ -33,7 +33,7 @@
|
|||
- **The snap asks for the log directories it reads, not each tool's whole home.** The first Snap Store submission declared a `personal-files` read of every AI tool's root — `$HOME/.claude`, `$HOME/.codex`, `$HOME/.cursor` and the rest — and that interface is recursive, so it granted read of every credential file those roots hold. Each entry now names the subdirectory the provider actually opens (`.claude/projects`, `.codex/sessions`, `.cline/data`, `.vibe/logs/session`, `.dsh/sessions`, `.kiro/sessions`, `.quickwork/{profiles.json,sessions,metrics}`, `.config/Claude/local-agent-mode-sessions`, `.config/Open Design/{runs,data/runs,namespaces}`), two are single files (`.forge/.forge.db`, `.zcode/cli/db/db.sqlite`), and the editor entries name only the extension folders holding transcripts instead of the editor's whole configuration. Five providers that were missing entirely and would have shown no data are declared — opencode, crush, goose, kilo, kimi-code — and four roots stay roots only because the file the provider opens sits directly in them (`.config/github-copilot`, `.local/share/{opencode,crush,kilo}`). One credential file is now requested openly rather than implicitly: `.claude/.credentials.json`, read-only, for the live plan gauge. Codex's equivalent would need write access to the Codex CLI's own `auth.json` to rotate the token, so neither it nor a Codex root is declared and the Codex live gauge is disabled under `$SNAP`; Codex usage and cost are unaffected, they come from the session rollouts. Two consequences inside the snap: `.lingtai` is dropped, because its per-agent log directory needs a wildcard the interface has no form for, and `optimize`, `context-budget` and `act` no longer see the user-scope `~/.claude/settings.json`, `agents/`, `skills/` and `commands/` — project-scope copies still work through the `home` plug. Nothing outside the snap changes.
|
||||
|
||||
### Fixed (Desktop & Menubar)
|
||||
- **A long panel query is no longer killed for being slow.** The desktop app capped every read at 45 seconds of TOTAL runtime, so on a slow machine `optimize`, `yield`, `models`, `sessions`, `spend`, `audit`, `act report` and `plan` were SIGKILLed mid-parse and the panel painted a red "timed out" that a 60-second poll then reproduced forever. That cap is now a no-output watchdog: the window restarts on every byte the child writes, so only a genuinely silent child times out, and every read spawn sets `CODEBURN_PROGRESS=1` so a multi-minute parse heartbeats through it (a 15-minute absolute ceiling still catches a livelocked child). The resident `codeburn serve` requests follow the same rule, resetting on each frame of their own response. Alongside it: the cold-cache floor now covers EVERY read while the first hydration is still running, not just the overview, so a section that starts polling the moment the app is ready is not killed waiting behind that parse; a read killed for timing out, and a resident child replaced by a settings mutation, are both sent SIGTERM first and SIGKILL only after a 5-second grace, letting a mid-write parse publish its partial cache and release the refresh lock instead of leaving both stale (quit stays a hard kill, since its flush budget is shorter than the grace); and a read that times out while the hydration is still going keeps the indexing splash instead of painting an error panel. The app also records the resident child's pid and reaps a serve orphaned by a previous crash on the next launch, after confirming the pid still belongs to a codeburn serve.
|
||||
- **A long panel query is no longer killed for being slow.** The desktop app capped every read at 45 seconds of TOTAL runtime, so on a slow machine `optimize`, `yield`, `models`, `sessions`, `spend`, `audit`, `act report` and `plan` were SIGKILLed mid-parse and the panel painted a red "timed out" that a 60-second poll then reproduced forever. That cap is now a no-output watchdog: the window restarts on every byte the child writes, so only a genuinely silent child times out, with a 15-minute absolute ceiling still catching a livelocked one. Silence now means stopped rather than slow, because the parse itself heartbeats: every read spawn sets `CODEBURN_PROGRESS=1`, and under it a running parse emits a keepalive line every 10 seconds regardless of phase — a cold parse's inter-provider cache save measured 31.6 seconds of total silence on a large corpus, which the old scan-progress stream did not cover. Resident `codeburn serve` requests follow the same rule, resetting on each frame of their own response. Alongside it: the cold-cache floor now covers EVERY read while the first hydration is still running, not just the overview, so a section that starts polling the moment the app is ready is not killed waiting behind that parse; a read that times out while the hydration is still going keeps the indexing splash instead of painting an error panel, bounded so that an install which can never hydrate still reaches the real error (and its "locate the CLI" recovery) once the cold window has elapsed; and a read killed for timing out, or a resident child replaced by a settings mutation, is sent SIGTERM first and SIGKILL only after a 5-second grace, so the child can unlink its own cache refresh lock rather than leave it for the next parse's stale-pid takeover (quit stays a hard kill, its flush budget being shorter than the grace). The app also records the resident child's pid and full command line, and reaps a serve orphaned by a previous crash on the next launch, only when the pid still matches that exact command. Separately, `codeburn serve` now drains an in-flight request before exiting on stdin close, so a request that arrives as the app disappears is still answered in full.
|
||||
- **The menubar's copies of your Claude and Codex credentials move out of Application Support and into the login Keychain.** Connecting a provider used to leave the copied OAuth material in `~/Library/Application Support/CodeBurn/*-credentials.v1.json`, written world-readable (0644) because macOS ignores `.completeFileProtection` outside iOS. The copy now lives in a CodeBurn-owned login-Keychain item, and the first read after upgrading migrates the old file: it is reopened with `O_NOFOLLOW`, refused if it is a symlink or not owned by you, repaired to 0600 before a single secret byte is read, written to the Keychain, read back and compared, and only then unlinked — a failed or unverified write leaves the (now 0600) file in place so a retry can still find it, and the next read retries the cleanup. Where both a Keychain item and an old file exist, the one that expires later wins before anything is removed, so an item left behind by a much older build cannot displace a fresher token. Claude's entry no longer stores a refresh token at all — the CLI owns that grant and the menubar never spends it — and any refresh token in a historical blob is dropped on read. Disconnect only reports success once the material is actually gone; if the delete fails it says so and leaves the provider connected so you can retry. Keychain reads are non-interactive and are skipped outright while the login Keychain is locked, so a background quota refresh can never raise an unlock panel. (#1037)
|
||||
- **First launch no longer asks to control System Events.** The macOS menubar registered its login item by driving System Events over AppleScript, which made macOS put up an Automation consent dialog the first time the app ran. It now registers itself through `SMAppService.mainApp`, an in-process call that needs no Automation grant; there is no AppleScript fallback, so a failure logs and leaves the login item unset rather than bringing the prompt back. The same `codeburn.loginItemRegistered` guard still limits this to the first launch, so a login item you removed by hand stays removed. (#1026)
|
||||
- **The resident `codeburn serve` child.** The first real panel request is also the cache warm-up, so startup never runs an artificial warm-up query beside a duplicate one-shot child; each served command carries its own read-only option allowlist, and anything outside it falls back to a normal spawn; the child exits when its stdin closes, so it can never outlive the app. Requests whose response exceeds the 16 MiB frame limit still replace the child, but that deliberate kill no longer spends the resident's unexpected-death budget. (#972)
|
||||
|
|
|
|||
|
|
@ -408,16 +408,18 @@ describe('no-output watchdog (timeoutMs bounds SILENCE, not total runtime)', ()
|
|||
await expect(spawnCli(['optimize'], { timeoutMs: 600 })).resolves.toEqual({ ok: 1, ticks: 15 })
|
||||
})
|
||||
|
||||
it('kills a child that goes silent after producing output', async () => {
|
||||
it('kills a child that goes silent, measured from its LAST byte', async () => {
|
||||
// One byte lands ~300ms in (plus node boot), well inside the 1s window, then
|
||||
// silence. A fixed cap kills at 1s; only a window measured from the LAST byte
|
||||
// waits past 1.3s. The gap is wide enough that node's boot cost cannot blur it.
|
||||
fakeBin(
|
||||
'talks-then-hangs.js',
|
||||
`process.stderr.write('CODEBURN_PROGRESS {"kind":"provider","provider":"claude","state":"start"}\\n');
|
||||
`setTimeout(() => process.stderr.write('CODEBURN_PROGRESS {"kind":"keepalive"}\\n'), 300);
|
||||
setInterval(() => {}, 1000);`,
|
||||
)
|
||||
const began = Date.now()
|
||||
await expect(spawnCli(['optimize'], { timeoutMs: 400 })).rejects.toMatchObject({ kind: 'timeout' })
|
||||
// The window restarts at the last byte, so this cannot settle before it.
|
||||
expect(Date.now() - began).toBeGreaterThanOrEqual(400)
|
||||
await expect(spawnCli(['optimize'], { timeoutMs: 1_000 })).rejects.toMatchObject({ kind: 'timeout' })
|
||||
expect(Date.now() - began).toBeGreaterThanOrEqual(1_250)
|
||||
})
|
||||
|
||||
it('keeps progress heartbeats out of the surfaced error message', async () => {
|
||||
|
|
@ -460,6 +462,28 @@ describe('graceful kill (SIGTERM, then SIGKILL after the grace)', () => {
|
|||
}, 12_000)
|
||||
}, 20_000)
|
||||
|
||||
it('keeps a child inside the SIGTERM grace reapable, so quit cannot orphan it', async () => {
|
||||
// The grace timer dies with the app. A child that ignores SIGTERM must still
|
||||
// be in the reap set when quit sweeps, or it survives the app that spawned it.
|
||||
const pidFile = join(dir, 'grace-pid')
|
||||
fakeBin(
|
||||
'ignores-sigterm-quit.js',
|
||||
`require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));
|
||||
process.on('SIGTERM', () => {});
|
||||
setInterval(() => {}, 1000);`,
|
||||
)
|
||||
|
||||
await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' })
|
||||
await waitFor(() => readMaybe(pidFile).length > 0)
|
||||
const pid = Number(readMaybe(pidFile))
|
||||
expect(() => process.kill(pid, 0)).not.toThrow() // alive, mid-grace
|
||||
|
||||
shutdownAll() // the quit sweep, landing inside the 5s grace
|
||||
await waitFor(() => {
|
||||
try { process.kill(pid, 0); return false } catch { return true }
|
||||
}, 3_000)
|
||||
})
|
||||
|
||||
it('lets a SIGTERM-handling child exit on its own without waiting for SIGKILL', async () => {
|
||||
const cleanupFile = join(dir, 'cleanup')
|
||||
fakeBin(
|
||||
|
|
@ -484,7 +508,7 @@ describe('orphan serve reaping', () => {
|
|||
const child = spawn(process.execPath, [bin, 'serve', '--stdio'], { stdio: 'ignore' })
|
||||
child.unref()
|
||||
const pidFile = join(dir, 'serve.pid')
|
||||
writeFileSync(pidFile, String(child.pid))
|
||||
writeFileSync(pidFile, JSON.stringify({ pid: child.pid, cmd: [process.execPath, bin, 'serve', '--stdio'].join(' ') }))
|
||||
return { pid: child.pid!, pidFile }
|
||||
}
|
||||
|
||||
|
|
@ -493,7 +517,9 @@ describe('orphan serve reaping', () => {
|
|||
const pidFile = join(dir, 'recorded.pid')
|
||||
startServe(pidFile)
|
||||
await waitFor(() => readMaybe(pidFile).length > 0)
|
||||
expect(Number(readMaybe(pidFile))).toBeGreaterThan(1)
|
||||
const record = JSON.parse(readMaybe(pidFile)) as { pid: number; cmd: string }
|
||||
expect(record.pid).toBeGreaterThan(1)
|
||||
expect(record.cmd).toContain('serve --stdio')
|
||||
})
|
||||
|
||||
it('kills a serve child orphaned by a previous run and clears the pidfile', async () => {
|
||||
|
|
@ -508,13 +534,21 @@ describe('orphan serve reaping', () => {
|
|||
expect(readMaybe(pidFile)).toBe('')
|
||||
})
|
||||
|
||||
it('never signals a recycled pid that is not a codeburn serve', async () => {
|
||||
const bin = join(dir, 'unrelated-tool')
|
||||
// A keyword sniff ("looks like a cli.js running serve") matches plenty of
|
||||
// unrelated tools. Identity is the exact argv we recorded, nothing looser.
|
||||
it.each([
|
||||
['an unrelated tool', 'unrelated-tool', ['work']],
|
||||
['a lookalike cli.js running serve', 'cli.js', ['serve', '--stdio']],
|
||||
['a lookalike named codeburn', 'codeburn', ['serve', '--stdio']],
|
||||
])('never signals a recycled pid belonging to %s', async (_label, name, args) => {
|
||||
const bin = join(dir, name)
|
||||
writeFileSync(bin, '#!/usr/bin/env node\nsetInterval(() => {}, 1000);\n', { mode: 0o755 })
|
||||
chmodSync(bin, 0o755)
|
||||
const bystander = spawn(process.execPath, [bin, 'work'], { stdio: 'ignore' })
|
||||
const pidFile = join(dir, 'recycled.pid')
|
||||
writeFileSync(pidFile, String(bystander.pid))
|
||||
const bystander = spawn(process.execPath, [bin, ...args], { stdio: 'ignore' })
|
||||
const pidFile = join(dir, `recycled-${name}.pid`)
|
||||
// The pid is right; the recorded command belongs to the serve child that
|
||||
// used to own it. Only an exact match may fire.
|
||||
writeFileSync(pidFile, JSON.stringify({ pid: bystander.pid, cmd: '/opt/codeburn/dist/cli.js serve --stdio' }))
|
||||
try {
|
||||
reapOrphanServe(pidFile)
|
||||
await new Promise(resolve => setTimeout(resolve, 200))
|
||||
|
|
@ -524,11 +558,14 @@ describe('orphan serve reaping', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('ignores a missing or unparseable pidfile', () => {
|
||||
it('ignores a missing, unparseable, or incomplete pidfile', () => {
|
||||
expect(() => reapOrphanServe(join(dir, 'absent.pid'))).not.toThrow()
|
||||
const junk = join(dir, 'junk.pid')
|
||||
writeFileSync(junk, 'not-a-pid')
|
||||
expect(() => reapOrphanServe(junk)).not.toThrow()
|
||||
const noCmd = join(dir, 'no-cmd.pid')
|
||||
writeFileSync(noCmd, JSON.stringify({ pid: 999999 }))
|
||||
expect(() => reapOrphanServe(noCmd)).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -941,9 +978,10 @@ describe('resident serve single-flight', () => {
|
|||
})
|
||||
|
||||
it('SIGTERMs the outgoing resident on a mutation restart instead of hard-killing it', async () => {
|
||||
// A settings mutation replaces a child that may be mid-write. Same lock
|
||||
// hazard as a timeout, so it gets the same grace: SIGKILL here strands the
|
||||
// cache refresh lock the outgoing parse is holding.
|
||||
// A settings mutation replaces a child that may hold the cache refresh lock.
|
||||
// Same hazard as a timeout, so it gets the same grace: only a catchable
|
||||
// signal lets the outgoing child unlink its own lock instead of leaving it
|
||||
// for the next parse's stale-pid takeover.
|
||||
const signalFile = join(dir, 'restart-signals')
|
||||
fakeBin(
|
||||
'sigterm-aware-resident.js',
|
||||
|
|
|
|||
|
|
@ -66,8 +66,11 @@ export const DESKTOP_COLD_TIMEOUT_MS = 10 * 60_000
|
|||
// Backstop for the watchdog: a livelocked child that chatters forever without
|
||||
// ever finishing still gets reaped.
|
||||
const MAX_RUNTIME_MS = 15 * 60_000
|
||||
// SIGTERM lets the CLI's signal cleanup (src/session-cache.ts) publish a partial
|
||||
// parse and release the cross-process refresh lock; SIGKILL only if it ignores it.
|
||||
// SIGTERM is catchable, so the CLI's armSignalCleanup (src/session-cache.ts)
|
||||
// unlinks its own cache refresh lock before dying. Under SIGKILL the lock file
|
||||
// is simply left behind and the next cold parse takes it over once it sees the
|
||||
// dead pid — self-healing, but via the stale-takeover path rather than a clean
|
||||
// release. Neither signal publishes a partial parse; nothing does.
|
||||
const KILL_GRACE_MS = 5_000
|
||||
/** Wire marker for CLI scan-progress lines (src/parser.ts: PROGRESS_LINE_PREFIX). */
|
||||
export const PROGRESS_LINE_PREFIX = 'CODEBURN_PROGRESS '
|
||||
|
|
@ -125,8 +128,8 @@ function reapAll(): void {
|
|||
serveClient = null
|
||||
// Deliberately harder than every other kill path: quit has a 1.5s flush budget,
|
||||
// shorter than the SIGTERM grace, so waiting one out would just wedge the quit.
|
||||
// One-shot reads hold no lock worth releasing; the resident child (destroyed
|
||||
// above) does, and gets the grace.
|
||||
// A lock left behind here still self-heals via the next parse's stale-pid
|
||||
// takeover; a quit that hangs does not.
|
||||
for (const child of activeChildren) child.kill('SIGKILL')
|
||||
activeChildren.clear()
|
||||
// A queued waiter has no child to reap, so releaseSlot never fires for it;
|
||||
|
|
@ -349,12 +352,17 @@ export function notFoundStage(): NotFoundStage {
|
|||
return 'no-path-match'
|
||||
}
|
||||
|
||||
/** Ask a child to exit, then insist. See {@link KILL_GRACE_MS}. */
|
||||
/** Ask a child to exit, then insist. See {@link KILL_GRACE_MS}. The child is
|
||||
* (re-)registered as active for the whole grace: a quit landing inside that
|
||||
* window must still find it and SIGKILL it rather than orphan it. */
|
||||
function killGracefully(child: ChildProcess): void {
|
||||
try { child.kill('SIGTERM') } catch { /* already gone */ }
|
||||
const grace = setTimeout(() => { try { child.kill('SIGKILL') } catch { /* already gone */ } }, KILL_GRACE_MS)
|
||||
activeChildren.add(child)
|
||||
let grace: NodeJS.Timeout | undefined
|
||||
const settle = () => { activeChildren.delete(child); if (grace) clearTimeout(grace) }
|
||||
child.once('exit', settle)
|
||||
try { child.kill('SIGTERM') } catch { settle(); return }
|
||||
grace = setTimeout(() => { try { child.kill('SIGKILL') } catch { /* already gone */ } settle() }, KILL_GRACE_MS)
|
||||
grace.unref?.()
|
||||
child.once('exit', () => clearTimeout(grace))
|
||||
}
|
||||
|
||||
/** Progress heartbeats share the stderr stream with real diagnostics, and every
|
||||
|
|
@ -398,6 +406,9 @@ function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?:
|
|||
const ceiling = setTimeout(() => expire(`codeburn ${cmdLabel} exceeded ${MAX_RUNTIME_MS}ms`), MAX_RUNTIME_MS)
|
||||
|
||||
const bump = (n: number) => {
|
||||
// Buffered bytes can still land after the kill; re-arming then would leave
|
||||
// a timer nobody clears (finish() already ran).
|
||||
if (settled) return
|
||||
armIdle()
|
||||
total += n
|
||||
if (total > MAX_OUTPUT_BYTES) {
|
||||
|
|
@ -519,7 +530,8 @@ class ServeClient {
|
|||
const child = spawn(this.spec.bin, [...this.spec.args], { shell: false, stdio: ['pipe', 'pipe', 'ignore'], env: this.spec.env })
|
||||
this.child = child
|
||||
if (this.pidFile && child.pid) {
|
||||
try { writeFileSync(this.pidFile, String(child.pid)) } catch { /* reaping is best-effort */ }
|
||||
const record = JSON.stringify({ pid: child.pid, cmd: [this.spec.bin, ...this.spec.args].join(' ') })
|
||||
try { writeFileSync(this.pidFile, record) } catch { /* reaping is best-effort */ }
|
||||
}
|
||||
child.stdout!.setEncoding('utf8')
|
||||
child.stdout!.on('data', (chunk: string) => {
|
||||
|
|
@ -634,8 +646,8 @@ class ServeClient {
|
|||
if (child) {
|
||||
// This is an intentional replacement, not a crash. Detach first so the
|
||||
// later exit event cannot consume the unexpected-death budget. The
|
||||
// outgoing child may be mid-write, so it gets the same SIGTERM grace a
|
||||
// timed-out one does — a hard kill here strands the refresh lock.
|
||||
// outgoing child may hold the refresh lock, so it gets the same SIGTERM
|
||||
// grace a timed-out one does and can unlink that lock on its way out.
|
||||
this.onDeath(child, false)
|
||||
killGracefully(child)
|
||||
}
|
||||
|
|
@ -730,16 +742,21 @@ export function startServe(pidFile?: string): void {
|
|||
* the orphan may be holding the cache refresh lock.
|
||||
*/
|
||||
export function reapOrphanServe(pidFile: string): void {
|
||||
let pid: number
|
||||
try { pid = Number.parseInt(readFileSync(pidFile, 'utf-8').trim(), 10) } catch { return }
|
||||
let record: { pid?: unknown; cmd?: unknown }
|
||||
try { record = JSON.parse(readFileSync(pidFile, 'utf-8')) } catch { return }
|
||||
try { unlinkSync(pidFile) } catch { /* stale file is harmless */ }
|
||||
if (!Number.isInteger(pid) || pid <= 1 || pid === process.pid) return
|
||||
const pid = record.pid
|
||||
const cmd = record.cmd
|
||||
if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 1 || pid === process.pid) return
|
||||
if (typeof cmd !== 'string' || !cmd) return
|
||||
// No `ps` on Windows, so identity cannot be confirmed there; skipping is
|
||||
// strictly better than signalling a recycled pid.
|
||||
if (platform() === 'win32') return
|
||||
try {
|
||||
const command = execFileSync('ps', ['-o', 'command=', '-p', String(pid)], { encoding: 'utf-8', timeout: 2_000 })
|
||||
if (!/codeburn|cli\.js|launch\.js/.test(command) || !/\bserve\b/.test(command)) return
|
||||
// Exact argv match, not a keyword sniff: any looser test signals whatever
|
||||
// unrelated process inherited this pid. -ww defeats ps's width truncation.
|
||||
const command = execFileSync('ps', ['-ww', '-o', 'command=', '-p', String(pid)], { encoding: 'utf-8', timeout: 2_000 })
|
||||
if (command.trim() !== cmd) return
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
|
|
@ -860,6 +877,9 @@ export function spawnCliAction(args: string[], opts: { timeoutMs?: number } = {}
|
|||
})()
|
||||
}
|
||||
|
||||
// Mutations keep a plain total-runtime cap and no progress env: they are short
|
||||
// by design (a config write, an export), never a full-history parse, so there is
|
||||
// no long silent stretch for a watchdog to misread.
|
||||
function runAction(spec: SpawnSpec, args: string[], timeoutMs: number): Promise<ActionResult> {
|
||||
return new Promise<ActionResult>(resolve => {
|
||||
const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env })
|
||||
|
|
|
|||
|
|
@ -499,6 +499,31 @@ describe('createBridgeHandlers (cold-start warmup)', () => {
|
|||
.toMatchObject({ ok: false, error: { kind: 'timeout', cold: true } })
|
||||
})
|
||||
|
||||
it('gives up the cold claim once the cold window itself has elapsed', async () => {
|
||||
// Without a bound this is a forever-splash: overviewWarmed only flips on
|
||||
// success, so an install that can never hydrate would keep every timeout
|
||||
// tagged cold and never reach the real error panel (or its CLI recovery).
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
vi.setSystemTime(new Date('2026-08-16T00:00:00Z'))
|
||||
const spawnCli = vi.fn(async () => { throw new CliError('timeout', 'no output for 45000ms') })
|
||||
const handlers = createBridgeHandlers(base({ spawnCli, emitProgress: vi.fn() }))
|
||||
|
||||
expect(await handlers['codeburn:getOverview']!('30days', 'all'))
|
||||
.toMatchObject({ ok: false, error: { cold: true } })
|
||||
|
||||
// Past the 10-minute cold floor, still failing: this is an error, not news
|
||||
// that indexing is in progress.
|
||||
vi.setSystemTime(new Date('2026-08-16T00:10:01Z'))
|
||||
const late = await handlers['codeburn:getOverview']!('30days', 'all') as { error: { kind: string; cold?: true } }
|
||||
expect(late.error.kind).toBe('timeout')
|
||||
expect(late.error.cold).toBeUndefined()
|
||||
expect((await handlers['codeburn:getActReport']!() as { error: { cold?: true } }).error.cold).toBeUndefined()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('never flags a non-timeout failure as cold (a real error is real news)', async () => {
|
||||
const spawnCli = vi.fn(async () => { throw new CliError('nonzero', 'permission denied') })
|
||||
const handlers = createBridgeHandlers(base({ spawnCli, emitProgress: vi.fn() }))
|
||||
|
|
|
|||
|
|
@ -265,9 +265,16 @@ export function createBridgeHandlers(deps: Deps = { spawnCli, spawnCliAction, re
|
|||
// Marks a TIMEOUT that happened while the cold hydration was still running, so
|
||||
// the renderer keeps the splash instead of painting a red error panel. Only
|
||||
// timeouts: a permission or nonzero failure is real news even while cold.
|
||||
//
|
||||
// BOUNDED, deliberately. `overviewWarmed` only flips on success, so an install
|
||||
// that can never hydrate would otherwise sit behind an indexing splash forever
|
||||
// with no error and no way to reach the "Locate the CLI" recovery. Past the
|
||||
// cold window itself, a timeout stops being "still indexing" and surfaces.
|
||||
const bootedAt = Date.now()
|
||||
const stillCold = (): boolean => !overviewWarmed && Date.now() - bootedAt < WARMUP_TIMEOUT_MS
|
||||
const coldError = (err: unknown): { kind: string; message: string; cold?: true } => {
|
||||
const error = toEnvelopeError(err)
|
||||
return overviewWarmed || error.kind !== 'timeout' ? error : { ...error, cold: true }
|
||||
return stillCold() && error.kind === 'timeout' ? { ...error, cold: true } : error
|
||||
}
|
||||
|
||||
const run = (build: (...args: any[]) => string[]): Handler => async (...args: any[]) => {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,10 @@ function reduceProgress(state: Progress, event: ScanProgressEvent): Progress {
|
|||
for (const p of state.order) status[p] = 'done'
|
||||
return { ...state, status }
|
||||
}
|
||||
// 'keepalive' and anything a newer CLI adds: proof of life, no state change.
|
||||
// A pinned/dev CLI can be newer than this app, so never fall off the switch.
|
||||
default:
|
||||
return state
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -646,6 +646,8 @@ export type ScanProgressEvent =
|
|||
| { kind: 'providers'; providers: string[]; cold?: boolean }
|
||||
| { kind: 'provider'; provider: string; state: 'start' | 'done' | 'skipped'; files?: number }
|
||||
| { kind: 'tick'; provider: string; done: number; total: number }
|
||||
/** Proof of life during a silent parse phase; carries nothing else. */
|
||||
| { kind: 'keepalive' }
|
||||
| { kind: 'done' }
|
||||
|
||||
/** Update-availability status from the main process (app/electron/updates.ts). */
|
||||
|
|
|
|||
|
|
@ -2932,12 +2932,37 @@ export type ScanProgressEvent =
|
|||
| { kind: 'providers'; providers: string[]; cold?: boolean }
|
||||
| { kind: 'provider'; provider: string; state: 'start' | 'done' | 'skipped'; files?: number }
|
||||
| { kind: 'tick'; provider: string; done: number; total: number }
|
||||
// Carries no information beyond "this parse is still alive". Consumers that
|
||||
// do not know it must ignore it rather than fail (app/renderer Splash does).
|
||||
| { kind: 'keepalive' }
|
||||
|
||||
export function emitScanProgress(event: ScanProgressEvent): void {
|
||||
if (process.env['CODEBURN_PROGRESS'] !== '1') return
|
||||
try { process.stderr.write(`${PROGRESS_LINE_PREFIX}${JSON.stringify(event)}\n`) } catch { /* stderr closed */ }
|
||||
}
|
||||
|
||||
// A cold parse has genuinely silent stretches: the inter-provider cache save at
|
||||
// the end of each provider measured 31.6s of total silence on a large corpus,
|
||||
// and the desktop app's no-output watchdog reads silence as a dead child. Beat
|
||||
// unconditionally while a parse is running, so silence means stopped, not slow.
|
||||
const PROGRESS_KEEPALIVE_MS = 10_000
|
||||
let keepaliveTimer: ReturnType<typeof setInterval> | null = null
|
||||
let keepaliveDepth = 0
|
||||
|
||||
export function startProgressKeepalive(): void {
|
||||
keepaliveDepth += 1
|
||||
if (keepaliveTimer || process.env['CODEBURN_PROGRESS'] !== '1') return
|
||||
keepaliveTimer = setInterval(() => emitScanProgress({ kind: 'keepalive' }), PROGRESS_KEEPALIVE_MS)
|
||||
keepaliveTimer.unref?.()
|
||||
}
|
||||
|
||||
export function stopProgressKeepalive(): void {
|
||||
keepaliveDepth = Math.max(0, keepaliveDepth - 1)
|
||||
if (keepaliveDepth > 0 || !keepaliveTimer) return
|
||||
clearInterval(keepaliveTimer)
|
||||
keepaliveTimer = null
|
||||
}
|
||||
|
||||
// Files parsed between partial-progress saves during a cold parse. Low enough
|
||||
// that an interrupted long run loses little work, high enough that repeated
|
||||
// cache writes never dominate the parse.
|
||||
|
|
@ -4071,12 +4096,29 @@ type RunParseOptions = {
|
|||
parseStartedAt: number
|
||||
}
|
||||
|
||||
/** Thin wrapper so every runParse call site heartbeats for its whole duration,
|
||||
* including the paths that throw. See {@link startProgressKeepalive}. */
|
||||
async function runParse(
|
||||
key: string,
|
||||
diskCache: SessionCache,
|
||||
dateRange: DateRange | undefined,
|
||||
providerFilter: string | undefined,
|
||||
options: RunParseOptions,
|
||||
): Promise<ProjectSummary[]> {
|
||||
startProgressKeepalive()
|
||||
try {
|
||||
return await runParseInner(key, diskCache, dateRange, providerFilter, options)
|
||||
} finally {
|
||||
stopProgressKeepalive()
|
||||
}
|
||||
}
|
||||
|
||||
async function runParseInner(
|
||||
key: string,
|
||||
diskCache: SessionCache,
|
||||
dateRange: DateRange | undefined,
|
||||
providerFilter: string | undefined,
|
||||
options: RunParseOptions,
|
||||
): Promise<ProjectSummary[]> {
|
||||
const { isCold = false, readOnly = false, refreshLock } = options
|
||||
readOnlyServedStale = false
|
||||
|
|
|
|||
|
|
@ -456,6 +456,11 @@ export async function runStdioServe(buildProgram: () => Command): Promise<void>
|
|||
await transportClosed
|
||||
} finally {
|
||||
rl.close()
|
||||
// Drain the in-flight request BEFORE returning. The caller exits the process
|
||||
// once this resolves, and runCaptured() has process.exit monkeypatched into
|
||||
// a thrown ExitSignal - returning mid-request would lose that request's
|
||||
// response frame and turn a clean exit into a failure.
|
||||
await queue
|
||||
await watcherSetup
|
||||
rootReuseValidation = null
|
||||
try {
|
||||
|
|
|
|||
76
tests/parser-progress-keepalive.test.ts
Normal file
76
tests/parser-progress-keepalive.test.ts
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
import { describe, it, expect, afterEach, vi } from 'vitest'
|
||||
|
||||
import { PROGRESS_LINE_PREFIX, startProgressKeepalive, stopProgressKeepalive } from '../src/parser.js'
|
||||
|
||||
// A cold parse goes genuinely silent between providers (a measured 31.6s on a
|
||||
// large corpus, in the inter-provider cache save), and the desktop app reads
|
||||
// silence as a dead child. These pin the heartbeat that makes silence mean
|
||||
// stopped rather than slow.
|
||||
describe('scan-progress keepalive', () => {
|
||||
const original = process.env['CODEBURN_PROGRESS']
|
||||
|
||||
/** Collects the progress lines written to stderr while `fn` drives the clock. */
|
||||
function captureKeepalives(fn: () => void): string[] {
|
||||
const written: string[] = []
|
||||
const spy = vi.spyOn(process.stderr, 'write').mockImplementation(((chunk: unknown) => {
|
||||
written.push(String(chunk))
|
||||
return true
|
||||
}) as typeof process.stderr.write)
|
||||
try { fn() } finally { spy.mockRestore() }
|
||||
return written.filter(line => line.startsWith(PROGRESS_LINE_PREFIX) && line.includes('"keepalive"'))
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
stopProgressKeepalive()
|
||||
stopProgressKeepalive()
|
||||
vi.useRealTimers()
|
||||
if (original === undefined) delete process.env['CODEBURN_PROGRESS']
|
||||
else process.env['CODEBURN_PROGRESS'] = original
|
||||
})
|
||||
|
||||
it('beats through a silent stretch far longer than the app watchdog window', () => {
|
||||
process.env['CODEBURN_PROGRESS'] = '1'
|
||||
vi.useFakeTimers()
|
||||
// 90s of a parse doing nothing observable — three times the measured save
|
||||
// stall, and twice the app's 45s silence window.
|
||||
const beats = captureKeepalives(() => {
|
||||
startProgressKeepalive()
|
||||
vi.advanceTimersByTime(90_000)
|
||||
})
|
||||
expect(beats.length).toBeGreaterThanOrEqual(9)
|
||||
// No silent gap anywhere near the window the app kills on.
|
||||
expect(90_000 / beats.length).toBeLessThan(45_000)
|
||||
})
|
||||
|
||||
it('stops when the parse ends, so an idle process never chatters', () => {
|
||||
process.env['CODEBURN_PROGRESS'] = '1'
|
||||
vi.useFakeTimers()
|
||||
const afterStop = captureKeepalives(() => {
|
||||
startProgressKeepalive()
|
||||
vi.advanceTimersByTime(25_000)
|
||||
stopProgressKeepalive()
|
||||
})
|
||||
expect(afterStop.length).toBeGreaterThan(0)
|
||||
expect(captureKeepalives(() => vi.advanceTimersByTime(60_000))).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps beating until the outermost parse finishes', () => {
|
||||
process.env['CODEBURN_PROGRESS'] = '1'
|
||||
vi.useFakeTimers()
|
||||
startProgressKeepalive()
|
||||
startProgressKeepalive()
|
||||
stopProgressKeepalive() // an inner parse returned; the outer one is still running
|
||||
expect(captureKeepalives(() => vi.advanceTimersByTime(30_000)).length).toBeGreaterThan(0)
|
||||
stopProgressKeepalive()
|
||||
expect(captureKeepalives(() => vi.advanceTimersByTime(30_000))).toEqual([])
|
||||
})
|
||||
|
||||
it('emits nothing for a plain CLI run (no CODEBURN_PROGRESS)', () => {
|
||||
delete process.env['CODEBURN_PROGRESS']
|
||||
vi.useFakeTimers()
|
||||
expect(captureKeepalives(() => {
|
||||
startProgressKeepalive()
|
||||
vi.advanceTimersByTime(60_000)
|
||||
})).toEqual([])
|
||||
})
|
||||
})
|
||||
|
|
@ -347,4 +347,49 @@ describe('codeburn serve --stdio', () => {
|
|||
|
||||
expect(naturalExit).toBe(true)
|
||||
}, 10_000)
|
||||
|
||||
it('answers an in-flight request in full when stdin closes on the same tick', async () => {
|
||||
// The transport closing does not cancel work already accepted. Returning
|
||||
// before the queue drains loses the response frame outright, and because
|
||||
// runCaptured() monkeypatches process.exit into a thrown ExitSignal it also
|
||||
// turns the clean exit into a failure.
|
||||
const raceChild = spawn(process.execPath, ['--import', 'tsx', join(__dirname, '..', 'src', 'cli.ts'), 'serve', '--stdio'], {
|
||||
stdio: ['pipe', 'pipe', 'ignore'],
|
||||
env: { ...process.env },
|
||||
})
|
||||
let stdout = ''
|
||||
const lines = (): Array<Record<string, unknown>> => stdout.split('\n')
|
||||
.map(line => { try { return JSON.parse(line) as Record<string, unknown> } catch { return null } })
|
||||
.filter((v): v is Record<string, unknown> => v !== null)
|
||||
|
||||
const becameReady = new Promise<void>((resolve, reject) => {
|
||||
raceChild.once('error', reject)
|
||||
raceChild.stdout!.setEncoding('utf8')
|
||||
raceChild.stdout!.on('data', (chunk: string) => {
|
||||
stdout += chunk
|
||||
if (lines().some(msg => msg['ready'] === true)) resolve()
|
||||
})
|
||||
raceChild.once('exit', (code, signal) => reject(new Error(`serve exited before ready: ${code ?? signal}`)))
|
||||
})
|
||||
const exited = new Promise<number | null>(resolve => raceChild.once('exit', code => resolve(code)))
|
||||
|
||||
await becameReady
|
||||
// Request and EOF in the same tick: the request is accepted, then the
|
||||
// transport is gone before it can possibly have finished.
|
||||
raceChild.stdin!.write(JSON.stringify({ id: 77, args: ['status', '--format', 'json'] }) + '\n')
|
||||
raceChild.stdin!.end()
|
||||
|
||||
const code = await Promise.race([
|
||||
exited,
|
||||
new Promise<'hung'>(resolve => setTimeout(() => resolve('hung'), 15_000)),
|
||||
])
|
||||
if (code === 'hung') { raceChild.kill('SIGKILL'); await exited }
|
||||
|
||||
expect(code).toBe(0)
|
||||
const answer = lines().find(msg => msg['id'] === 77)
|
||||
expect(answer).toBeDefined()
|
||||
expect(answer!['ok']).toBe(true)
|
||||
expect(typeof answer!['output']).toBe('string')
|
||||
expect(() => JSON.parse(answer!['output'] as string)).not.toThrow()
|
||||
}, 25_000)
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue