diff --git a/.changeset/fix-crash-exit-log-flush.md b/.changeset/fix-crash-exit-log-flush.md new file mode 100644 index 0000000000..e051320480 --- /dev/null +++ b/.changeset/fix-crash-exit-log-flush.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix the diagnostic log missing the actual error when the CLI exits unexpectedly. diff --git a/apps/kimi-code/src/cli/run-shell.ts b/apps/kimi-code/src/cli/run-shell.ts index caeae907ca..47d97a828b 100644 --- a/apps/kimi-code/src/cli/run-shell.ts +++ b/apps/kimi-code/src/cli/run-shell.ts @@ -4,6 +4,7 @@ import { join } from 'node:path'; import { createKimiHarness, + flushDiagnosticLogsSync, log, type KimiHarness, type TelemetryClient, @@ -159,6 +160,14 @@ export async function runShell( // raw mode with a hidden cursor and XON/XOFF flow control disabled. Restore // both before exiting so the user's shell is usable afterwards. const emergencyExit = (exitCode: number): void => { + // The crash log above is only enqueued into the async sink; flush it + // synchronously or the `process.exit()` below would drop the one line that + // explains why we crashed. Best-effort: an exit path must never throw. + try { + flushDiagnosticLogsSync(); + } catch { + /* ignore */ + } restoreTerminalModes(); restoreStty(); process.exit(exitCode); diff --git a/apps/kimi-code/test/cli/run-shell.test.ts b/apps/kimi-code/test/cli/run-shell.test.ts index eafc4a9e0e..cb9478d960 100644 --- a/apps/kimi-code/test/cli/run-shell.test.ts +++ b/apps/kimi-code/test/cli/run-shell.test.ts @@ -58,6 +58,7 @@ const mocks = vi.hoisted(() => { track: lifecycleTrack, })), resolveKimiHome: vi.fn((homeDir?: string) => homeDir ?? '/tmp/kimi-code-test-home'), + flushDiagnosticLogsSync: vi.fn(), harnessCreatesDeviceIdOnConstruction: false, execSync: vi.fn(), TuiConfigParseError, @@ -69,6 +70,7 @@ vi.mock('@moonshot-ai/kimi-code-sdk', async (importOriginal) => { return { ...actual, resolveKimiHome: mocks.resolveKimiHome, + flushDiagnosticLogsSync: mocks.flushDiagnosticLogsSync, createKimiHarness: (...args: unknown[]) => { const options = args[0] as { readonly homeDir?: string } | undefined; const homeDir = options?.homeDir ?? '/tmp/kimi-code-test-home'; @@ -508,6 +510,101 @@ describe('runShell', () => { }); }); + it('flushes diagnostic logs synchronously before exiting on a runtime crash', async () => { + mocks.loadTuiConfig.mockResolvedValue({ + theme: 'dark', + editorCommand: null, + notifications: { enabled: true, condition: 'unfocused' }, + }); + mocks.tuiStart.mockResolvedValue(undefined); + + const processOnSpy = vi.spyOn(process, 'on'); + const stdout = captureProcessWrite('stdout'); + const exitSpy = mockProcessExit(); + + try { + await runShell( + { + session: undefined, + continue: false, + yolo: false, + auto: false, + plan: false, + model: undefined, + outputFormat: undefined, + prompt: undefined, + skillsDirs: [], + }, + '1.2.3-test', + ); + + const handler = processOnSpy.mock.calls.find( + ([event]) => event === 'uncaughtException', + )?.[1] as ((error: unknown) => void) | undefined; + expect(handler).toBeDefined(); + + // The async log sink cannot flush before process.exit() runs, so the + // crash handler must force a synchronous flush or the crash reason is + // lost (regression: uncaughtException logs never reached disk). + expect(() => handler?.(new Error('boom'))).toThrow(ExitCalled); + expect(mocks.flushDiagnosticLogsSync).toHaveBeenCalledOnce(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mocks.flushDiagnosticLogsSync.mock.invocationCallOrder[0]!).toBeLessThan( + exitSpy.mock.invocationCallOrder[0]!, + ); + } finally { + processOnSpy.mockRestore(); + exitSpy.mockRestore(); + stdout.restore(); + } + }); + + it('flushes diagnostic logs synchronously before exiting on an unhandled rejection', async () => { + mocks.loadTuiConfig.mockResolvedValue({ + theme: 'dark', + editorCommand: null, + notifications: { enabled: true, condition: 'unfocused' }, + }); + mocks.tuiStart.mockResolvedValue(undefined); + + const processOnSpy = vi.spyOn(process, 'on'); + const stdout = captureProcessWrite('stdout'); + const exitSpy = mockProcessExit(); + + try { + await runShell( + { + session: undefined, + continue: false, + yolo: false, + auto: false, + plan: false, + model: undefined, + outputFormat: undefined, + prompt: undefined, + skillsDirs: [], + }, + '1.2.3-test', + ); + + const handler = processOnSpy.mock.calls.find( + ([event]) => event === 'unhandledRejection', + )?.[1] as ((reason: unknown) => void) | undefined; + expect(handler).toBeDefined(); + + expect(() => handler?.(new Error('boom'))).toThrow(ExitCalled); + expect(mocks.flushDiagnosticLogsSync).toHaveBeenCalledOnce(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mocks.flushDiagnosticLogsSync.mock.invocationCallOrder[0]!).toBeLessThan( + exitSpy.mock.invocationCallOrder[0]!, + ); + } finally { + processOnSpy.mockRestore(); + exitSpy.mockRestore(); + stdout.restore(); + } + }); + it('closes the harness when TUI startup fails', async () => { mocks.loadTuiConfig.mockResolvedValue({ theme: 'dark', diff --git a/apps/kimi-code/test/helpers/process.ts b/apps/kimi-code/test/helpers/process.ts index 5a861724b0..1da718facb 100644 --- a/apps/kimi-code/test/helpers/process.ts +++ b/apps/kimi-code/test/helpers/process.ts @@ -6,7 +6,7 @@ export class ExitCalled extends Error { } } -export function mockProcessExit(): { mockRestore(): void } { +export function mockProcessExit() { return vi.spyOn(process, 'exit').mockImplementation(((code?: string | number | null) => { throw new ExitCalled(Number(code ?? 0)); }) as never); diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index 97ccf3f068..25236a7d2b 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -10,6 +10,7 @@ export * from './plugin'; export { buildReplay } from './agent/replay/build'; export { flushDiagnosticLogs, + flushDiagnosticLogsSync, getRootLogger, log, redact, diff --git a/packages/agent-core/src/logging/index.ts b/packages/agent-core/src/logging/index.ts index 51be3b8a66..ab8ec689d5 100644 --- a/packages/agent-core/src/logging/index.ts +++ b/packages/agent-core/src/logging/index.ts @@ -14,6 +14,7 @@ export { LOG_LEVEL_RANK, levelEnabled } from './types'; export { __resetRootLoggerForTest, flushDiagnosticLogs, + flushDiagnosticLogsSync, getRootLogger, log, redact, diff --git a/packages/agent-core/src/logging/logger.ts b/packages/agent-core/src/logging/logger.ts index c8b507bd17..6a8b64a269 100644 --- a/packages/agent-core/src/logging/logger.ts +++ b/packages/agent-core/src/logging/logger.ts @@ -276,6 +276,16 @@ export function flushDiagnosticLogs(): Promise { return getRootInternal().flush(); } +/** + * Synchronous variant for crash / emergency-exit paths that call + * `process.exit()` on the same tick: pending entries are appended with + * `appendFileSync`, so they survive the immediate exit that would otherwise + * drop everything still sitting in the async queue. + */ +export function flushDiagnosticLogsSync(): void { + getRootInternal().flushSync(); +} + class LoggerImpl implements Logger { constructor(private readonly boundCtx: LogContext) {} diff --git a/packages/agent-core/test/logging/logger.test.ts b/packages/agent-core/test/logging/logger.test.ts index 2b70d20113..e46a79493b 100644 --- a/packages/agent-core/test/logging/logger.test.ts +++ b/packages/agent-core/test/logging/logger.test.ts @@ -1,4 +1,5 @@ import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'pathe'; @@ -6,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { __resetRootLoggerForTest, + flushDiagnosticLogsSync, getRootLogger, log, redact, @@ -429,6 +431,29 @@ describe('session routing', () => { }); }); +describe('flushDiagnosticLogsSync', () => { + it('persists enqueued entries synchronously, before the async drain could run', async () => { + await getRootLogger().configure(defaultConfig()); + log.error('crash marker', { error: new Error('boom') }); + + // No `await` between enqueue and flush: the async drain (microtask + async + // fs) cannot have written yet, so only the synchronous append can produce + // the file content below. This mirrors crash paths that call + // process.exit() on the same tick. + flushDiagnosticLogsSync(); + + const content = readFileSync(resolveGlobalLogPath(homeDir), 'utf-8'); + expect(content).toContain('crash marker'); + expect(content).toContain('boom'); + }); + + it('is a silent no-op before configure', () => { + expect(() => { + flushDiagnosticLogsSync(); + }).not.toThrow(); + }); +}); + describe('redact helper', () => { it('returns same shape with sensitive fields replaced', () => { const out = redact({ user: 'x', token: 'abc', nested: { apiKey: '1' } }); diff --git a/packages/node-sdk/src/index.ts b/packages/node-sdk/src/index.ts index f58e060751..6b731391ee 100644 --- a/packages/node-sdk/src/index.ts +++ b/packages/node-sdk/src/index.ts @@ -52,6 +52,7 @@ export { // RootLogger / getRootLogger / LoggingConfig stay inside agent-core. export { flushDiagnosticLogs, + flushDiagnosticLogsSync, log, redact, resolveGlobalLogPath,