Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-crash-exit-log-flush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Fix the diagnostic log missing the actual error when the CLI exits unexpectedly.
9 changes: 9 additions & 0 deletions apps/kimi-code/src/cli/run-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { join } from 'node:path';

import {
createKimiHarness,
flushDiagnosticLogsSync,
log,
type KimiHarness,
type TelemetryClient,
Expand Down Expand Up @@ -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);
Expand Down
97 changes: 97 additions & 0 deletions apps/kimi-code/test/cli/run-shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion apps/kimi-code/test/helpers/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/agent-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export * from './plugin';
export { buildReplay } from './agent/replay/build';
export {
flushDiagnosticLogs,
flushDiagnosticLogsSync,
getRootLogger,
log,
redact,
Expand Down
1 change: 1 addition & 0 deletions packages/agent-core/src/logging/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export { LOG_LEVEL_RANK, levelEnabled } from './types';
export {
__resetRootLoggerForTest,
flushDiagnosticLogs,
flushDiagnosticLogsSync,
getRootLogger,
log,
redact,
Expand Down
10 changes: 10 additions & 0 deletions packages/agent-core/src/logging/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,16 @@ export function flushDiagnosticLogs(): Promise<boolean> {
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) {}

Expand Down
25 changes: 25 additions & 0 deletions packages/agent-core/test/logging/logger.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { mkdtemp, readFile, rm } from 'node:fs/promises';
import { readFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'pathe';

import { afterEach, beforeEach, describe, expect, it } from 'vitest';

import {
__resetRootLoggerForTest,
flushDiagnosticLogsSync,
getRootLogger,
log,
redact,
Expand Down Expand Up @@ -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' } });
Expand Down
1 change: 1 addition & 0 deletions packages/node-sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export {
// RootLogger / getRootLogger / LoggingConfig stay inside agent-core.
export {
flushDiagnosticLogs,
flushDiagnosticLogsSync,
log,
redact,
resolveGlobalLogPath,
Expand Down
Loading