diff --git a/.changeset/bug-report-server-exit-reason.md b/.changeset/bug-report-server-exit-reason.md new file mode 100644 index 00000000..b03891ac --- /dev/null +++ b/.changeset/bug-report-server-exit-reason.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Report a bug now records why the server last exited. When the background server process goes away, the desktop app writes the exit code and the reason it left (a clean shutdown, a crash, or an out-of-memory / OS kill) to `state/last-server-exit.json` in the report bundle, next to `server.lock`. Until now a bundle could only show that the server's port was "unreachable", which looks identical whether the server crashed or was shut down cleanly, so a "my app crashed" report could not be told apart from a routine stop. The new record captures the death even when the server was killed and had no chance to log anything itself. It holds only the exit code, the reason, the pid, and a timestamp, so it carries no document content or paths. diff --git a/packages/cli/src/diagnose/bundle.test.ts b/packages/cli/src/diagnose/bundle.test.ts index 89d0247f..7f8b60c7 100644 --- a/packages/cli/src/diagnose/bundle.test.ts +++ b/packages/cli/src/diagnose/bundle.test.ts @@ -345,6 +345,28 @@ describe('collectBundle — state files', () => { collected.cleanup(); }); + test('last-server-exit.json is staged when the desktop host wrote one', async () => { + const contentDir = makeTmpDir(); + const body = `${JSON.stringify( + { at: '2026-07-16T21:02:24.000Z', pid: 51502, code: null, reason: 'killed' }, + null, + 2, + )}\n`; + writeAt(contentDir, '.ok/local/last-server-exit.json', body); + const collected = await collectBundle({ contentDir, deps: makeDeterministicDeps() }); + expect( + readFileSync(join(collected.stagingDir, 'state', 'last-server-exit.json'), 'utf-8'), + ).toBe(body); + collected.cleanup(); + }); + + test('last-server-exit.json is omitted when the host never recorded an exit', async () => { + const contentDir = makeTmpDir(); + const collected = await collectBundle({ contentDir, deps: makeDeterministicDeps() }); + expect(existsSync(join(collected.stagingDir, 'state', 'last-server-exit.json'))).toBe(false); + collected.cleanup(); + }); + test('runtime.json carries ok, host blocks; desktop is null by default', async () => { const contentDir = makeTmpDir(); const collected = await collectBundle({ contentDir, deps: makeDeterministicDeps() }); diff --git a/packages/cli/src/diagnose/bundle.ts b/packages/cli/src/diagnose/bundle.ts index 16e52298..3b52e88f 100644 --- a/packages/cli/src/diagnose/bundle.ts +++ b/packages/cli/src/diagnose/bundle.ts @@ -29,6 +29,7 @@ import { import { arch as osArch, platform as osPlatform, tmpdir } from 'node:os'; import { basename, dirname, join, relative, resolve, sep } from 'node:path'; import type { BundleRedaction as SecretScrubEntry } from '@inkeep/open-knowledge-core'; +import { SERVER_EXIT_LOG } from '@inkeep/open-knowledge-core'; import { DEFAULT_LOGS_MAX_BYTES, DEFAULT_SPANS_MAX_BYTES, @@ -634,6 +635,11 @@ export async function collectBundle(opts: CollectBundleOpts): Promise/.ok/local/` where the desktop host records why + * the server process last exited — the exit `code` plus Electron's + * process-gone `reason` (`clean-exit` / `abnormal-exit` / `killed` / `crashed` + * / `oom`). Written by the desktop main process (which observes the child's + * death even when the child could not report it) and collected into a + * bug-report bundle's `state/` dir beside `server.lock`. + * + * This closes a diagnostic gap: without it, a bundle can't tell a server that + * crashed or was OS-killed from one that shut down cleanly — the liveness + * probe only reports "unreachable" either way. + */ +export const SERVER_EXIT_LOG = 'last-server-exit.json'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3032b2a7..8298ca6d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1139,6 +1139,7 @@ export { export { DEFAULT_SIGTERM_GRACE_MS, DEFAULT_SIGTERM_POLL_MS, + SERVER_EXIT_LOG, SPAWN_ERROR_LOG, } from './constants/lifecycle.ts'; export { diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index f64e5142..7d2591cb 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -298,6 +298,7 @@ import { raiseMostRecentlyFocusedAfterRestore, } from './restore-focus.ts'; import { handleRevealExternal } from './reveal-external.ts'; +import { createServerExitRecorder, type ServerExitRecorder } from './server-exit-record.ts'; import { startFirstRunHandshake } from './share-handoff.ts'; import { handleShellOpenExternal } from './shell-allowlist.ts'; import { createShowGateRegistry, type ShowGateRegistry } from './show-gate.ts'; @@ -934,6 +935,23 @@ let mcpWiringHandle: RunMcpWiringHandle | null = null; */ let crashDetection: CrashDetection | null = null; +/** + * Records the server's last exit (code + Electron process-gone reason) to + * `/last-server-exit.json` for bug-report diagnosis. Lazy singleton so + * the window-manager fork path and the `child-process-gone` listener — which + * initialize on different boot paths — share one correlator. + */ +let serverExitRecorder: ServerExitRecorder | null = null; +function getServerExitRecorder(): ServerExitRecorder { + if (serverExitRecorder === null) { + serverExitRecorder = createServerExitRecorder({ + now: () => new Date(), + logger: getLogger('server-exit'), + }); + } + return serverExitRecorder; +} + /** * Full-resolution PNG bytes of the app screenshot captured when a window's * report-a-bug dialog opened, keyed by the sender `webContents.id`. Held in @@ -1389,6 +1407,7 @@ function ensureWindowManager() { onUtilityExit: (utility) => { ensureDebugIpc().cancelPendingForUtility(utility); }, + recordServerExit: (info) => getServerExitRecorder().recordExit(info), // Presence-invisible keepalive WS — registers the desktop as an active // `/collab*` upgrade for as long as a project window is open, so a brief // MCP disconnect does not trip the server's idle-shutdown timer. The @@ -5078,6 +5097,12 @@ function bootPrimaryInstance(): void { }); crashDetection.detectBootCrash(); app.on('child-process-gone', (_event, details) => { + // Feed the server-exit recorder every Utility death (not just the crash + // reasons the invitation pipeline acts on) so the bundle can distinguish a + // `killed` / `oom` / `crashed` exit from a `clean-exit`. + if (details.type === 'Utility') { + getServerExitRecorder().noteGoneReason(details.reason); + } crashDetection?.handleChildProcessGone(details); }); diff --git a/packages/desktop/src/main/server-exit-record.test.ts b/packages/desktop/src/main/server-exit-record.test.ts new file mode 100644 index 00000000..be9c894e --- /dev/null +++ b/packages/desktop/src/main/server-exit-record.test.ts @@ -0,0 +1,163 @@ +/** + * Server-exit recorder tests: tmpdir-backed lockDir, an injected advancing + * clock, and a capturing logger — the same injectable-deps posture as the + * sibling crash-detection tests. Covers the two arrival orders of the exit + * `code` and the process-gone `reason`, the correlation window that joins + * them, and the fail-soft write path. + */ + +import { afterEach, describe, expect, test } from 'bun:test'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SERVER_EXIT_LOG } from '@inkeep/open-knowledge-core'; +import { createServerExitRecorder, type ServerExitRecord } from './server-exit-record.ts'; + +const tmpDirs: string[] = []; + +afterEach(() => { + for (const dir of tmpDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +function makeLockDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'ok-exit-')); + tmpDirs.push(dir); + return dir; +} + +/** Clock the recorder reads through `now()`; advance with `tick(ms)`. */ +function makeClock(startMs = 1_000_000) { + let ms = startMs; + return { + now: () => new Date(ms), + tick: (delta: number) => { + ms += delta; + }, + }; +} + +function makeLogger() { + const warnings: Array<{ payload: Record; msg: string }> = []; + return { + warnings, + logger: { + warn: (payload: Record, msg: string) => warnings.push({ payload, msg }), + }, + }; +} + +function readRecord(lockDir: string): ServerExitRecord { + return JSON.parse(readFileSync(join(lockDir, SERVER_EXIT_LOG), 'utf8')) as ServerExitRecord; +} + +describe('createServerExitRecorder', () => { + test('recordExit writes code, pid, and ISO timestamp with a null reason when none seen', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.recordExit({ lockDir, pid: 51502, code: 0 }); + + const record = readRecord(lockDir); + expect(record.pid).toBe(51502); + expect(record.code).toBe(0); + expect(record.reason).toBeNull(); + expect(new Date(record.at).toISOString()).toBe(record.at); + }); + + test('a null code (signal kill) is preserved', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.recordExit({ lockDir, pid: 999, code: null }); + + expect(readRecord(lockDir).code).toBeNull(); + }); + + test('noteGoneReason before recordExit attaches the reason', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.noteGoneReason('oom'); + clock.tick(50); + recorder.recordExit({ lockDir, pid: 1, code: null }); + + expect(readRecord(lockDir).reason).toBe('oom'); + }); + + test('recordExit before noteGoneReason patches the file within the window', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.recordExit({ lockDir, pid: 1, code: 1 }); + expect(readRecord(lockDir).reason).toBeNull(); + + clock.tick(100); + recorder.noteGoneReason('killed'); + + const record = readRecord(lockDir); + expect(record.reason).toBe('killed'); + // The patch preserves the originally-observed exit fields. + expect(record.code).toBe(1); + expect(record.pid).toBe(1); + }); + + test('a reason older than the correlation window is not attached to a later exit', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.noteGoneReason('killed'); + clock.tick(3_001); + recorder.recordExit({ lockDir, pid: 1, code: null }); + + expect(readRecord(lockDir).reason).toBeNull(); + }); + + test('a reason arriving after the window does not patch an earlier exit', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.recordExit({ lockDir, pid: 1, code: 0 }); + clock.tick(3_001); + recorder.noteGoneReason('clean-exit'); + + expect(readRecord(lockDir).reason).toBeNull(); + }); + + test('each exit overwrites the previous record (latest death wins)', () => { + const lockDir = makeLockDir(); + const clock = makeClock(); + const recorder = createServerExitRecorder({ now: clock.now, logger: makeLogger().logger }); + + recorder.recordExit({ lockDir, pid: 1, code: 0 }); + clock.tick(60_000); + recorder.recordExit({ lockDir, pid: 2, code: 1 }); + + const record = readRecord(lockDir); + expect(record.pid).toBe(2); + expect(record.code).toBe(1); + }); + + test('an unwritable lockDir warns instead of throwing', () => { + const parent = makeLockDir(); + // Put a regular file where the recorder expects a directory, so the + // recursive mkdir fails with ENOTDIR. + const filePath = join(parent, 'not-a-dir'); + writeFileSync(filePath, 'x'); + const badLockDir = join(filePath, 'nested'); + const clock = makeClock(); + const { warnings, logger } = makeLogger(); + const recorder = createServerExitRecorder({ now: clock.now, logger }); + + expect(() => recorder.recordExit({ lockDir: badLockDir, pid: 1, code: 1 })).not.toThrow(); + expect(warnings).toHaveLength(1); + expect(warnings[0]?.payload.event).toBe('server-exit-record.write-failed'); + }); +}); diff --git a/packages/desktop/src/main/server-exit-record.ts b/packages/desktop/src/main/server-exit-record.ts new file mode 100644 index 00000000..b993e8f0 --- /dev/null +++ b/packages/desktop/src/main/server-exit-record.ts @@ -0,0 +1,126 @@ +/** + * Records why the OpenKnowledge server process last exited, so a bug-report + * bundle can tell an unexpected death (a crash, or an OS OOM-kill / SIGKILL) + * apart from a managed shutdown. The desktop main process observes the child's + * death even when the child itself could not report it (a SIGKILL leaves no + * last words), which is exactly the case the bundle otherwise can't diagnose: + * its liveness probe only ever reports the port "unreachable", identical for a + * crashed server and one that was cleanly stopped. + * + * The record lands at `/last-server-exit.json` — beside `server.lock` + * under `/.ok/local/`, where the bundle collector already harvests + * runtime state. + * + * Two Electron signals describe the same death and can arrive in either order: + * - the per-window `utilityProcess.on('exit')` handler carries the exit + * `code` and the pid, and knows which server it belongs to (its `lockDir`); + * - `app.on('child-process-gone')` carries Electron's classified `reason` + * (`clean-exit` / `abnormal-exit` / `killed` / `crashed` / `oom` / ...) but + * no pid and no lockDir. + * This recorder joins them: whichever fires first writes the record, and the + * later one patches in its field. Correlation is a short time window rather + * than an identity match — the main process is single-threaded so the two + * handlers never interleave, and a lone desktop rarely tears down two distinct + * servers within the same window. On a wider overlap the `reason` may attach to + * the wrong record, so it is advisory; the `code` and timing are authoritative. + */ + +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { SERVER_EXIT_LOG } from '@inkeep/open-knowledge-core'; + +/** How long a `code` and a `reason` for the same death may be apart. */ +const REASON_CORRELATION_WINDOW_MS = 3_000; + +export interface ServerExitRecord { + /** ISO timestamp of the exit the desktop host observed. */ + at: string; + /** The server's pid, or null when the exit event carried none. */ + pid: number | null; + /** `utilityProcess` exit code; null when the process was killed by a signal. */ + code: number | null; + /** + * Electron's `child-process-gone` reason, or null when no reason arrived in + * the correlation window. `killed` covers an OS OOM-kill / SIGKILL; `crashed` + * / `oom` a genuine in-process crash; `clean-exit` / `abnormal-exit` a + * managed shutdown or a nonzero self-exit. + */ + reason: string | null; +} + +interface ServerExitRecorderLogger { + warn(payload: Record, msg: string): void; +} + +export interface ServerExitRecorderDeps { + now(): Date; + logger: ServerExitRecorderLogger; +} + +export interface ServerExitRecorder { + /** + * Record an observed server exit. Called from the window manager's + * `utilityProcess.on('exit')` handler, which knows the `lockDir`, pid, and + * exit code. Attaches a `reason` if `noteGoneReason` fired for the same death + * moments earlier. + */ + recordExit(info: { lockDir: string; pid: number | null; code: number | null }): void; + /** + * Note Electron's classified process-gone reason. Called from + * `app.on('child-process-gone')`, which has the reason but no lockDir. Patches + * the just-written record when the exit event already fired for this death. + */ + noteGoneReason(reason: string): void; +} + +export function createServerExitRecorder(deps: ServerExitRecorderDeps): ServerExitRecorder { + let lastExit: { lockDir: string; record: ServerExitRecord; atMs: number } | null = null; + let lastReason: { reason: string; atMs: number } | null = null; + + function write(lockDir: string, record: ServerExitRecord): void { + try { + mkdirSync(lockDir, { recursive: true }); + writeFileSync(join(lockDir, SERVER_EXIT_LOG), `${JSON.stringify(record, null, 2)}\n`); + } catch (err) { + // Best-effort diagnostic — a server death must never be masked by an + // unwritable state dir. The bundle simply won't carry the record. + deps.logger.warn( + { + event: 'server-exit-record.write-failed', + cause: err instanceof Error ? err.message : String(err), + }, + 'could not record server exit', + ); + } + } + + return { + recordExit({ lockDir, pid, code }): void { + const now = deps.now(); + const nowMs = now.getTime(); + const reason = + lastReason !== null && nowMs - lastReason.atMs <= REASON_CORRELATION_WINDOW_MS + ? lastReason.reason + : null; + const record: ServerExitRecord = { at: now.toISOString(), pid, code, reason }; + write(lockDir, record); + lastExit = { lockDir, record, atMs: nowMs }; + }, + + noteGoneReason(reason): void { + const nowMs = deps.now().getTime(); + lastReason = { reason, atMs: nowMs }; + // Patch the exit record when the `exit` event already landed for this + // death without a reason yet (the two events can arrive in either order). + if ( + lastExit !== null && + lastExit.record.reason === null && + nowMs - lastExit.atMs <= REASON_CORRELATION_WINDOW_MS + ) { + const patched: ServerExitRecord = { ...lastExit.record, reason }; + write(lastExit.lockDir, patched); + lastExit = { ...lastExit, record: patched }; + } + }, + }; +} diff --git a/packages/desktop/src/main/window-manager.ts b/packages/desktop/src/main/window-manager.ts index ff517d3c..1be5fd34 100644 --- a/packages/desktop/src/main/window-manager.ts +++ b/packages/desktop/src/main/window-manager.ts @@ -679,6 +679,12 @@ export interface WindowManagerDeps { * to `onUtilityMessage`, so the consumer can identity-match. */ onUtilityExit?(utility: UtilityProcessLike): void; + /** + * Record why the server just exited to `/last-server-exit.json`, so + * a later bug-report bundle can tell an unexpected death from a managed + * shutdown. No-op when omitted (tests, web). See `server-exit-record.ts`. + */ + recordServerExit?(info: { lockDir: string; pid: number | null; code: number | null }): void; /** * Startup-instrumentation hooks (desktop launch waterfall). All optional and * no-op when omitted (tests, web). Wired by `index.ts` only for the FIRST @@ -1698,6 +1704,10 @@ export class WindowManager { // independently. utility.on('exit', (code) => { this.deps.log?.info({ pid: utility.pid, code }, 'utility exited'); + // Persist the exit for bug-report diagnosis before any teardown — the + // main process observes this death even when the server (SIGKILL'd / + // OOM-killed) could not report it itself. + this.deps.recordServerExit?.({ lockDir, pid: utility.pid ?? null, code }); this.windowsByPath.delete(canonicalKey); // Reject any in-flight debug-IPC requests bound to this utility so // pending entries don't linger for the full timeout window after a