diff --git a/CHANGELOG.md b/CHANGELOG.md index 93878851..4191b244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +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 timed-out read is killed with 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; 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. - **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) diff --git a/app/electron/cli.test.ts b/app/electron/cli.test.ts index 0fb11504..a7348f5d 100644 --- a/app/electron/cli.test.ts +++ b/app/electron/cli.test.ts @@ -1,10 +1,11 @@ // @vitest-environment node import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { spawn } from 'node:child_process' import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, chmodSync } from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join, isAbsolute, relative, win32, posix } from 'node:path' -import { spawnCli, spawnCliAction, spawnEnvFor, spawnSpecFor, startServe, killAll, shutdownAll, CliError, nodeManagerDirs, notFoundStage, resolveCodeburnPath, resolveTarget } from './cli' +import { spawnCli, spawnCliAction, spawnEnvFor, spawnSpecFor, startServe, killAll, shutdownAll, CliError, nodeManagerDirs, notFoundStage, reapOrphanServe, resolveCodeburnPath, resolveTarget } from './cli' let dir: string const originalBin = process.env.CODEBURN_BIN @@ -364,7 +365,7 @@ describe('spawnCli', () => { it('rejects with kind "timeout" when the binary hangs', async () => { fakeBin('hang.js', 'setInterval(() => {}, 1000)') - await expect(spawnCli(['status'], { timeoutMs: 150 })).rejects.toMatchObject({ kind: 'timeout' }) + await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' }) }) it('rejects with kind "not-found" when no binary resolves', async () => { @@ -385,6 +386,152 @@ describe('spawnCli', () => { }) }) +describe('no-output watchdog (timeoutMs bounds SILENCE, not total runtime)', () => { + /** Emits one progress byte every `everyMs` for `ticks` ticks, then the payload. */ + function chattyBin(everyMs: number, ticks: number): void { + fakeBin( + 'chatty.js', + `let n = 0; + const t = setInterval(() => { + process.stderr.write('CODEBURN_PROGRESS {"kind":"tick","provider":"claude","done":' + n + ',"total":${ticks}}\\n'); + if (++n >= ${ticks}) { clearInterval(t); process.stdout.write(JSON.stringify({ ok: 1, ticks: n })); } + }, ${everyMs});`, + ) + } + + // The 0.9.20 failure class: a warm `optimize` measured 52.5s against a fixed + // 45s cap and was SIGKILLed even though the parse was making steady progress. + // Scaled down here — total runtime is 5x the window, so the OLD fixed-timeout + // code fails this test and only a resetting watchdog passes. + it('never kills a child that keeps producing output past the window', async () => { + chattyBin(100, 15) // ~1.5s of work under a 600ms window + await expect(spawnCli(['optimize'], { timeoutMs: 600 })).resolves.toEqual({ ok: 1, ticks: 15 }) + }) + + it('kills a child that goes silent after producing output', async () => { + fakeBin( + 'talks-then-hangs.js', + `process.stderr.write('CODEBURN_PROGRESS {"kind":"provider","provider":"claude","state":"start"}\\n'); + 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) + }) + + it('keeps progress heartbeats out of the surfaced error message', async () => { + fakeBin( + 'progress-then-fails.js', + `process.stderr.write('CODEBURN_PROGRESS {"kind":"tick","provider":"claude","done":1,"total":2}\\n'); + process.stderr.write('permission denied\\n'); + process.exit(2);`, + ) + await expect(spawnCli(['status'])).rejects.toMatchObject({ kind: 'nonzero', message: 'permission denied' }) + }) + + it('enables CODEBURN_PROGRESS on every read spawn so long parses heartbeat', async () => { + fakeBin('env-echo.js', 'process.stdout.write(JSON.stringify({ progress: process.env.CODEBURN_PROGRESS }))') + await expect(spawnCli(['act', 'report', '--json'])).resolves.toEqual({ progress: '1' }) + }) +}) + +describe('graceful kill (SIGTERM, then SIGKILL after the grace)', () => { + it('sends SIGTERM first and SIGKILLs a child that ignores it', async () => { + const signalFile = join(dir, 'signals') + const pidFile = join(dir, 'stubborn-pid') + fakeBin( + 'ignores-sigterm.js', + `const fs = require('node:fs'); + fs.writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)); + process.on('SIGTERM', () => fs.appendFileSync(${JSON.stringify(signalFile)}, 'TERM')); + setInterval(() => {}, 1000);`, + ) + + await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' }) + // SIGTERM arrives with the rejection; the child survives it and is SIGKILLed + // only after KILL_GRACE_MS (5s). + await waitFor(() => readMaybe(signalFile) === 'TERM') + const pid = Number(readMaybe(pidFile)) + expect(Number.isInteger(pid)).toBe(true) + expect(() => process.kill(pid, 0)).not.toThrow() // still alive inside the grace + await waitFor(() => { + try { process.kill(pid, 0); return false } catch { return true } + }, 12_000) + }, 20_000) + + it('lets a SIGTERM-handling child exit on its own without waiting for SIGKILL', async () => { + const cleanupFile = join(dir, 'cleanup') + fakeBin( + 'handles-sigterm.js', + `const fs = require('node:fs'); + process.on('SIGTERM', () => { fs.writeFileSync(${JSON.stringify(cleanupFile)}, 'released'); process.exit(0); }); + setInterval(() => {}, 1000);`, + ) + const began = Date.now() + await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' }) + await waitFor(() => readMaybe(cleanupFile) === 'released') + expect(Date.now() - began).toBeLessThan(5_000) // never waited out the grace + }) +}) + +describe('orphan serve reaping', () => { + /** A long-lived stand-in for an orphaned `codeburn serve --stdio` child. */ + function orphanServe(): { pid: number; pidFile: string } { + const bin = join(dir, 'codeburn') + writeFileSync(bin, '#!/usr/bin/env node\nsetInterval(() => {}, 1000);\n', { mode: 0o755 }) + chmodSync(bin, 0o755) + const child = spawn(process.execPath, [bin, 'serve', '--stdio'], { stdio: 'ignore' }) + child.unref() + const pidFile = join(dir, 'serve.pid') + writeFileSync(pidFile, String(child.pid)) + return { pid: child.pid!, pidFile } + } + + it('startServe records the resident pid so a later launch can find it', async () => { + fakeResidentBin() + const pidFile = join(dir, 'recorded.pid') + startServe(pidFile) + await waitFor(() => readMaybe(pidFile).length > 0) + expect(Number(readMaybe(pidFile))).toBeGreaterThan(1) + }) + + it('kills a serve child orphaned by a previous run and clears the pidfile', async () => { + const { pid, pidFile } = orphanServe() + await waitFor(() => { try { process.kill(pid, 0); return true } catch { return false } }) + + reapOrphanServe(pidFile) + + await waitFor(() => { + try { process.kill(pid, 0); return false } catch { return true } + }) + expect(readMaybe(pidFile)).toBe('') + }) + + it('never signals a recycled pid that is not a codeburn serve', async () => { + const bin = join(dir, 'unrelated-tool') + 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)) + try { + reapOrphanServe(pidFile) + await new Promise(resolve => setTimeout(resolve, 200)) + expect(() => process.kill(bystander.pid!, 0)).not.toThrow() + } finally { + bystander.kill('SIGKILL') + } + }) + + it('ignores a missing or unparseable 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() + }) +}) + describe('spawn PATH augmentation (GUI-launched apps have a minimal PATH)', () => { it("prepends the resolved binary's own directory so its env-shebang finds node", async () => { const bin = fakeBin('path-echo.js', 'process.stdout.write(JSON.stringify({ path: process.env.PATH }))') @@ -617,6 +764,43 @@ describe('resident serve single-flight', () => { expect(chunks.join('')).toBe('CODEBURN_PROGRESS {"kind":"provider","provider":"claude","state":"start","generation":1}\n') }) + it('resets the resident watchdog on every progress frame of a long request', async () => { + // Same contract as a one-shot spawn: a warm serve request that keeps + // streaming progress is never killed, however long the parse takes. The + // first status request warms serve, so the second runs under the plain + // (non-cold-floor) window and can only survive by resetting it. + fakeBin( + 'streaming-resident.js', + `const readline = require('node:readline'); + if (process.argv[2] === 'serve') { + const rl = readline.createInterface({ input: process.stdin }); + rl.on('line', line => { + const request = JSON.parse(line); + if (!request.args.includes('--stream')) { + process.stdout.write(JSON.stringify({ id: request.id, ok: true, output: JSON.stringify({ via: 'serve' }) }) + '\\n'); + return; + } + let n = 0; + const t = setInterval(() => { + process.stdout.write(JSON.stringify({ id: request.id, progress: 'CODEBURN_PROGRESS {"kind":"tick","provider":"claude","done":' + n + ',"total":15}\\n' }) + '\\n'); + if (++n >= 15) { + clearInterval(t); + process.stdout.write(JSON.stringify({ id: request.id, ok: true, output: JSON.stringify({ via: 'serve', ticks: n }) }) + '\\n'); + } + }, 100); + }); + } else { + process.stdout.write(JSON.stringify({ via: 'spawn' })); + }`, + ) + startServe() + + await expect(spawnCli(['status', '--warm'], { timeoutMs: 5_000 })).resolves.toEqual({ via: 'serve' }) + // ~1.5s of streamed work under a 600ms silence window. + await expect(spawnCli(['optimize', '--stream'], { timeoutMs: 600 })) + .resolves.toEqual({ via: 'serve', ticks: 15 }) + }) + it('rejects and terminates a resident that emits an oversized valid JSON frame', async () => { const startsFile = join(dir, 'oversized-frame-starts') const oneShotsFile = join(dir, 'oversized-frame-one-shots') diff --git a/app/electron/cli.ts b/app/electron/cli.ts index 2d449a43..3aa7c91c 100644 --- a/app/electron/cli.ts +++ b/app/electron/cli.ts @@ -1,5 +1,5 @@ -import { spawn, type ChildProcess } from 'node:child_process' -import { accessSync, constants, existsSync, readdirSync, readFileSync, statSync } from 'node:fs' +import { execFileSync, spawn, type ChildProcess } from 'node:child_process' +import { accessSync, constants, existsSync, readdirSync, readFileSync, statSync, unlinkSync, writeFileSync } from 'node:fs' import { homedir, platform } from 'node:os' import { delimiter, dirname, isAbsolute, join } from 'node:path' @@ -53,11 +53,24 @@ export class CliError extends Error { } } +// Read timeouts are a NO-OUTPUT watchdog, not a total-runtime cap: the window +// restarts every time the child emits a byte on stdout/stderr (serve: every +// frame for that request). A long-but-progressing parse on a slow machine is +// therefore never killed — only a genuinely silent child is. Read spawns all +// set CODEBURN_PROGRESS=1 so a multi-minute parse heartbeats through it. const DEFAULT_TIMEOUT_MS = 45_000 // The first status query may hydrate a power-user cache from scratch. Every // resident request admitted before that succeeds shares this floor so a later // short request cannot kill the child while it waits behind the cold scan. 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. +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 ' // A runaway CLI (or a compromised binary) must not exhaust main-process memory. const MAX_OUTPUT_BYTES = 16 * 1024 * 1024 // A cold-cache CLI spawn costs seconds at ~120% CPU; letting every poll + @@ -332,6 +345,20 @@ export function notFoundStage(): NotFoundStage { return 'no-path-match' } +/** Ask a child to exit, then insist. See {@link KILL_GRACE_MS}. */ +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) + grace.unref?.() + child.once('exit', () => clearTimeout(grace)) +} + +/** Progress heartbeats share the stderr stream with real diagnostics, and every + * read spawn now enables them — so they must never become the error message. */ +function withoutProgressLines(stderr: string): string { + return stderr.split('\n').filter(line => !line.startsWith(PROGRESS_LINE_PREFIX)).join('\n').trim() +} + function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?: (chunk: string) => void): Promise { return new Promise((resolve, reject) => { const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env }) @@ -344,19 +371,30 @@ function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?: const finish = (fn: () => void) => { if (settled) return settled = true - clearTimeout(timer) + clearTimeout(idleTimer) + clearTimeout(ceiling) activeChildren.delete(child) fn() } - const timer = setTimeout(() => { + const expire = (message: string) => { finish(() => { - child.kill('SIGKILL') - reject(new CliError('timeout', `codeburn ${cmdLabel} timed out after ${timeoutMs}ms`)) + killGracefully(child) + reject(new CliError('timeout', message)) }) - }, timeoutMs) + } + + // Restarted on every byte the child produces: `timeoutMs` bounds SILENCE. + let idleTimer: NodeJS.Timeout + const armIdle = () => { + clearTimeout(idleTimer) + idleTimer = setTimeout(() => expire(`codeburn ${cmdLabel} produced no output for ${timeoutMs}ms`), timeoutMs) + } + armIdle() + const ceiling = setTimeout(() => expire(`codeburn ${cmdLabel} exceeded ${MAX_RUNTIME_MS}ms`), MAX_RUNTIME_MS) const bump = (n: number) => { + armIdle() total += n if (total > MAX_OUTPUT_BYTES) { finish(() => { @@ -382,7 +420,7 @@ function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?: child.on('close', code => { finish(() => { if (code !== 0) { - reject(new CliError('nonzero', stderr.trim() || `codeburn exited with code ${code}`)) + reject(new CliError('nonzero', withoutProgressLines(stderr) || `codeburn exited with code ${code}`)) return } try { @@ -443,16 +481,21 @@ async function runScheduledCli( const SERVE_ROUTED = new Set(['status', 'models', 'sessions', 'compare', 'yield', 'spend', 'optimize', 'audit']) const SERVE_MAX_RESTARTS = 3 +type PendingServeRequest = { + resolve: (v: unknown) => void + reject: (e: Error) => void + /** Restart the no-output watchdog — called for every frame carrying this id. */ + arm: () => void + /** Cancel both the watchdog and the absolute ceiling. */ + clear: () => void + warmsServe: boolean + decodedBytes: number + onStderr?: (chunk: string) => void +} + class ServeClient { private child: ReturnType | null = null - private pending = new Map void - reject: (e: Error) => void - timer: NodeJS.Timeout - warmsServe: boolean - decodedBytes: number - onStderr?: (chunk: string) => void - }>() + private pending = new Map() private nextId = 1 private deaths = 0 private buffer = '' @@ -461,7 +504,7 @@ class ServeClient { private destroyed = false private requestTail: Promise = Promise.resolve() - constructor(private readonly spec: SpawnSpec) {} + constructor(private readonly spec: SpawnSpec, private readonly pidFile?: string) {} isRunning(): boolean { return this.child !== null } disabled(): boolean { return this.deaths >= SERVE_MAX_RESTARTS } @@ -471,6 +514,9 @@ class ServeClient { if (this.child || this.disabled() || this.destroyed) return 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 */ } + } child.stdout!.setEncoding('utf8') child.stdout!.on('data', (chunk: string) => { // A replaced child's stream can drain after its exit callback. Never let @@ -503,6 +549,9 @@ class ServeClient { if (typeof msg.id !== 'number') continue const waiter = this.pending.get(msg.id) if (!waiter) continue + // Any frame for this request is proof of life: restart its watchdog so a + // long cold parse that heartbeats progress is never killed mid-flight. + waiter.arm() if (typeof msg.progress === 'string') { if (!this.consumeDecodedOutput(child, waiter, msg.progress)) return if (waiter.onStderr) { @@ -513,7 +562,7 @@ class ServeClient { const terminalOutput = typeof msg.output === 'string' ? msg.output : typeof msg.error === 'string' ? msg.error : '' if (!this.consumeDecodedOutput(child, waiter, terminalOutput)) return this.pending.delete(msg.id) - clearTimeout(waiter.timer) + waiter.clear() if (msg.ok && typeof msg.output === 'string') { if (waiter.warmsServe) this.warmed = true try { waiter.resolve(JSON.parse(msg.output)) } @@ -551,7 +600,7 @@ class ServeClient { // budget, or three oversized payloads would disable serve for the app run. activeChildren.delete(child as never) for (const [, waiter] of this.pending) { - clearTimeout(waiter.timer) + waiter.clear() waiter.reject(error) } this.pending.clear() @@ -570,7 +619,7 @@ class ServeClient { if (countsTowardBudget) this.deaths += 1 activeChildren.delete(child as never) for (const [, waiter] of this.pending) { - clearTimeout(waiter.timer) + waiter.clear() waiter.reject(new CliError('nonzero', 'codeburn serve exited')) } this.pending.clear() @@ -601,27 +650,36 @@ class ServeClient { const child = this.child if (!child?.stdin) return Promise.reject(new CliError('nonzero', 'serve not running')) const id = this.nextId++ - const effectiveTimeoutMs = this.warmed ? timeoutMs : Math.max(timeoutMs, DESKTOP_COLD_TIMEOUT_MS) + const idleMs = this.warmed ? timeoutMs : Math.max(timeoutMs, DESKTOP_COLD_TIMEOUT_MS) return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - // A hung request would block the serialized queue behind it; kill the - // child so everything falls back to spawns and a fresh serve restarts. + let idleTimer: NodeJS.Timeout + // A hung request would block the serialized queue behind it; kill the + // child so everything falls back to spawns and a fresh serve restarts. + const expire = (message: string) => { + entry.clear() this.pending.delete(id) - reject(new CliError('timeout', 'codeburn serve timed out')) - child.kill('SIGKILL') - }, effectiveTimeoutMs) - this.pending.set(id, { + reject(new CliError('timeout', message)) + killGracefully(child) + } + const ceiling = setTimeout(() => expire(`codeburn serve exceeded ${MAX_RUNTIME_MS}ms`), MAX_RUNTIME_MS) + const entry: PendingServeRequest = { resolve, reject, - timer, + arm: () => { + clearTimeout(idleTimer) + idleTimer = setTimeout(() => expire(`codeburn serve produced no output for ${idleMs}ms`), idleMs) + }, + clear: () => { clearTimeout(idleTimer); clearTimeout(ceiling) }, warmsServe: args[0] === 'status', decodedBytes: 0, ...(onStderr ? { onStderr } : {}), - }) + } + entry.arm() + this.pending.set(id, entry) child.stdin!.write(JSON.stringify({ id, args }) + '\n', (err) => { if (err) { this.pending.delete(id) - clearTimeout(timer) + entry.clear() reject(new CliError('nonzero', 'serve write failed')) } }) @@ -642,8 +700,10 @@ let serveClient: ServeClient | null = null /** Start the resident serve child without issuing a query. The first real panel * request is accepted immediately (even before the ready frame) and performs - * the one cold-cache hydration while streaming progress back to the splash. */ -export function startServe(): void { + * the one cold-cache hydration while streaming progress back to the splash. + * `pidFile` records the child so {@link reapOrphanServe} can clean it up if the + * app dies without ever closing the child's stdin. */ +export function startServe(pidFile?: string): void { if (shuttingDown) return const target = resolveTarget() if (!target) return @@ -651,11 +711,35 @@ export function startServe(): void { if (!serveClient) { const spec = spawnSpecFor(target, ['serve', '--stdio']) spec.env = { ...spec.env, CODEBURN_PROGRESS: '1' } - serveClient = new ServeClient(spec) + serveClient = new ServeClient(spec, pidFile) } serveClient.start() } +/** + * Best-effort reap of a serve child orphaned by a previous run (an app crash + * leaves no one to close its stdin). Reads the pid recorded by + * {@link startServe}, and — because pids are recycled — signals it only after + * `ps` confirms the process is still a codeburn serve. SIGTERM, never SIGKILL: + * 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 } + try { unlinkSync(pidFile) } catch { /* stale file is harmless */ } + if (!Number.isInteger(pid) || pid <= 1 || pid === process.pid) 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 + } catch { + return + } + try { process.kill(pid, 'SIGTERM') } catch { /* already gone */ } +} + function restartServeAfterMutation(): void { // CLI-only consumers never started serve, so do not create a surprise daemon // for them. In Electron, replace the resident child immediately so its parser @@ -687,6 +771,10 @@ export function spawnCli( const target = resolveTarget() if (!target) return Promise.reject(new CliError('not-found', 'codeburn CLI not found', notFoundStage())) const spec = spawnSpecFor(target, args) + // Heartbeats for the no-output watchdog: a multi-minute parse writes progress + // lines to stderr instead of going silent. Only reads get this — mutations + // (spawnCliAction) keep their plain total-runtime cap. + spec.env = { ...spec.env, CODEBURN_PROGRESS: '1' } if (opts.extraEnv) spec.env = { ...spec.env, ...opts.extraEnv } const generation = readGeneration diff --git a/app/electron/main.test.ts b/app/electron/main.test.ts index 1bd27d49..c41f6332 100644 --- a/app/electron/main.test.ts +++ b/app/electron/main.test.ts @@ -467,6 +467,46 @@ describe('createBridgeHandlers (cold-start warmup)', () => { expect(opts[1]?.timeoutMs).toBe(10 * 60_000) }) + it('gives every section read the cold floor while hydration is still running', async () => { + // The repro: the moment `ready` flipped, act report / plan spawned with the + // plain 45s cap and were killed waiting behind the cold parse's lock. + const opts: Array<{ timeoutMs?: number } | undefined> = [] + const spawnCli = vi.fn(async (_args: string[], o?: { timeoutMs?: number }) => { + opts.push(o) + return { current: { cost: 1 } } + }) + const handlers = createBridgeHandlers(base({ spawnCli, emitProgress: vi.fn() })) + + await handlers['codeburn:getActReport']!() + await handlers['codeburn:getPlans']!('30days') + await handlers['codeburn:getOptimizeReport']!('30days', 'all') + expect(opts.map(o => o?.timeoutMs)).toEqual([10 * 60_000, 10 * 60_000, 10 * 60_000]) + + // Once the overview lands, the cold cache is hot and reads revert to the + // plain default so a genuinely stuck child is still caught quickly. + await handlers['codeburn:getOverview']!('30days', 'all') + await handlers['codeburn:getActReport']!() + expect(opts[4]?.timeoutMs).toBeUndefined() + }) + + it('flags a cold-hydration timeout so the renderer keeps the splash', async () => { + 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:getActReport']!()) + .toMatchObject({ ok: false, error: { kind: 'timeout', cold: true } }) + expect(await handlers['codeburn:getOverview']!('30days', 'all')) + .toMatchObject({ ok: false, error: { kind: 'timeout', cold: true } }) + }) + + 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() })) + + const res = await handlers['codeburn:getActReport']!() as { error: { cold?: true } } + expect(res.error.cold).toBeUndefined() + }) + it('parses CLI scan-progress stderr lines and forwards them to emitProgress', async () => { const spawnCli = vi.fn(async (_args: string[], o?: { onStderr?: (chunk: string) => void }) => { // A split line proves the reader buffers across chunks. diff --git a/app/electron/main.ts b/app/electron/main.ts index d3676474..d6e323f7 100644 --- a/app/electron/main.ts +++ b/app/electron/main.ts @@ -1,7 +1,7 @@ import { app, BrowserWindow, dialog, ipcMain, Menu, nativeTheme, shell, type MenuItemConstructorOptions } from 'electron' import path from 'node:path' -import { CliError, DESKTOP_COLD_TIMEOUT_MS, resolveCodeburnPath, shutdownAll, spawnCli, spawnCliAction, startServe, type ActionResult, type SpawnPriority } from './cli' +import { CliError, DESKTOP_COLD_TIMEOUT_MS, PROGRESS_LINE_PREFIX, reapOrphanServe, resolveCodeburnPath, shutdownAll, spawnCli, spawnCliAction, startServe, type ActionResult, type SpawnPriority } from './cli' import { getQuota, sanitizeError } from './quota' import { Telemetry } from './telemetry' import { createUpdateChecker, type UpdateChecker, type UpdateStatus } from './updates' @@ -69,7 +69,7 @@ export function createBeforeQuitHandler(deps: BeforeQuitDeps): (event: BeforeQui // Result envelope: handlers never throw across IPC so the structured error // `kind` survives contextBridge serialization. preload.ts unwraps it. -export type Envelope = { ok: true; value: T } | { ok: false; error: { kind: string; message: string } } +export type Envelope = { ok: true; value: T } | { ok: false; error: { kind: string; message: string; cold?: true } } // The first overview fetch after boot hydrates a cold cache from scratch (a full // history parse). That can far exceed the 45s read timeout, and killing it means @@ -78,8 +78,6 @@ export type Envelope = { ok: true; value: T } | { ok: false; error: // once it succeeds. Sections gate their own first poll on this one resolving so // the cold hydration runs ONCE, not once per section in parallel. const WARMUP_TIMEOUT_MS = DESKTOP_COLD_TIMEOUT_MS -// Wire marker for CLI scan-progress lines (src/parser.ts: PROGRESS_LINE_PREFIX). -const PROGRESS_LINE_PREFIX = 'CODEBURN_PROGRESS ' // IPC channel carrying cold-start scan-progress events to the splash. export const PROGRESS_CHANNEL = 'codeburn:progress' // IPC channel pushing update-availability status to open windows (launch + 24h). @@ -258,14 +256,28 @@ export function createBridgeHandlers(deps: Deps = { spawnCli, spawnCliAction, re telemetry?.track('cold_start', { ms: Date.now() - (coldStartBegan ?? Date.now()), timedOut }) } + // Until the cold hydration finishes, EVERY read shares the overview's floor. + // Sections start polling the moment `ready` flips (which an overview error + // also does), and a 45s section spawn queued behind a still-running cold parse + // was killed on arrival — the `act report`/`plan` red panels in the repro. + const readOpts = (): { timeoutMs: number } | undefined => + overviewWarmed ? undefined : { timeoutMs: WARMUP_TIMEOUT_MS } + // 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. + const coldError = (err: unknown): { kind: string; message: string; cold?: true } => { + const error = toEnvelopeError(err) + return overviewWarmed || error.kind !== 'timeout' ? error : { ...error, cold: true } + } + const run = (build: (...args: any[]) => string[]): Handler => async (...args: any[]) => { let cmd: string | undefined try { const argv = build(...args) cmd = argv[0] - return { ok: true, value: await deps.spawnCli(argv) } + return { ok: true, value: await deps.spawnCli(argv, readOpts()) } } catch (err) { - const error = toEnvelopeError(err) + const error = coldError(err) telemetry?.track('cli_error', cliErrorProps(err, cmd)) return { ok: false, error } } @@ -308,7 +320,7 @@ export function createBridgeHandlers(deps: Deps = { spawnCli, spawnCliAction, re emitColdStart(false) return { ok: true, value } } catch (err) { - const error = toEnvelopeError(err) + const error = coldError(err) if (!overviewWarmed) emitColdStart(error.kind === 'timeout') telemetry?.track('cli_error', cliErrorProps(err, 'status')) return { ok: false, error } @@ -572,7 +584,11 @@ function bootstrap(): void { // Start the resident child early, but issue no artificial warm-up query: // the first real overview request is the single cache hydration and streams // its progress through serve. Every later panel reuses that parsed cache. - startServe() + // A crash leaves no one to close the previous child's stdin, so reap it + // first — orphans hold FSEvents handles and a stale cache refresh lock. + const servePidFile = path.join(app.getPath('userData'), 'serve.pid') + reapOrphanServe(servePidFile) + startServe(servePidFile) // Consent-gated anonymous telemetry (desktop only). Nothing transmits until // the onboarding consent screen is completed and the toggle is on; EU/EEA/ // UK/CH installs default the toggle off. Dev builds never send. diff --git a/app/renderer/App.test.tsx b/app/renderer/App.test.tsx index d29b4c43..1e9f64b3 100644 --- a/app/renderer/App.test.tsx +++ b/app/renderer/App.test.tsx @@ -205,6 +205,23 @@ describe('App shortcuts', () => { clearPlatform() }) + it('keeps sections gated (and the splash up) while a cold hydration times out', async () => { + // The repro: the overview timed out cold, `ready` latched anyway, and every + // section then spawned its own 45s read behind the still-running parse. + mocks.getOverview.mockRejectedValue({ kind: 'timeout', message: 'no output for 45000ms', cold: true }) + render() + await waitFor(() => expect(mocks.getOverview).toHaveBeenCalled()) + await act(async () => { await Promise.resolve() }) + expect(mocks.getActReport).not.toHaveBeenCalled() + expect(screen.queryByText("Couldn't read data")).not.toBeInTheDocument() + }) + + it('releases the sections when the overview fails for a real reason', async () => { + mocks.getOverview.mockRejectedValue({ kind: 'nonzero', message: 'permission denied' }) + render() + await waitFor(() => expect(mocks.getActReport).toHaveBeenCalled()) + }) + it('applies the persisted theme on app boot before Settings mounts', async () => { localStorage.setItem('codeburn.theme', 'dark') render() diff --git a/app/renderer/App.tsx b/app/renderer/App.tsx index 48295d6b..6262abbc 100644 --- a/app/renderer/App.tsx +++ b/app/renderer/App.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { isColdHydrating } from './components/CliErrorPanel' import { EmptyNote } from './components/EmptyState' import { ErrorBoundary } from './components/ErrorBoundary' import { Hint } from './components/Hint' @@ -258,10 +259,15 @@ function AppMain() { // full-history parse per section. Flips true the moment overview first has data // OR a (resolved) error; LATCHED, so a later uncached switch (which clears // overview.data to paint a skeleton) can never re-gate the sections. + // A cold-hydration failure is NOT a resolution: flipping ready on it released + // every section to spawn its own read behind the still-running parse, and each + // one then died on its own timeout. Stay gated (and keep the splash) until the + // hydration actually settles. + const overviewCold = isColdHydrating(overview.error) const [ready, setReady] = useState(false) useEffect(() => { - if (overview.data != null || overview.error != null) setReady(true) - }, [overview.data, overview.error]) + if (overview.data != null || (overview.error != null && !overviewCold)) setReady(true) + }, [overview.data, overview.error, overviewCold]) // First-launch onboarding: shown until the telemetry consent screen has been // completed once. All telemetry bridge calls are typeof-guarded so an older @@ -525,7 +531,7 @@ function AppMain() { } /> - + {onboardingStatus && }