From dfbefe384a78bba954acc8e9664f40910f1000fd Mon Sep 17 00:00:00 2001 From: Claude Assistant Date: Mon, 20 Apr 2026 18:31:11 +0800 Subject: [PATCH 1/2] fix(debug): buffer pre-init events and flush to real logger on init When getDebugLogger() is called before initDebugLogger(), events were silently dropped with noop stubs that returned empty DebugEvent objects. This fix implements a buffered noop logger that: - Captures up to 256 events in memory before initialization - Warns to console with buffer size when events are logged before init - Flushes all buffered events to the real logger when initDebugLogger runs Also adds unit tests for the pre-init buffering behavior. Fixes #412 --- packages/desktop/src/main/debug/index.ts | 98 ++++++++++++++++--- .../desktop/test/debug/debug-logger.test.ts | 75 ++++++++++++++ 2 files changed, 161 insertions(+), 12 deletions(-) create mode 100644 packages/desktop/test/debug/debug-logger.test.ts diff --git a/packages/desktop/src/main/debug/index.ts b/packages/desktop/src/main/debug/index.ts index fba97b9ee..66e23d755 100644 --- a/packages/desktop/src/main/debug/index.ts +++ b/packages/desktop/src/main/debug/index.ts @@ -1,25 +1,95 @@ import { BrowserWindow } from 'electron'; -import type { DebugEvent } from '@clawwork/shared'; +import type { DebugEvent, LogEventInput } from '@clawwork/shared'; import type { DebugLogger } from './logger.js'; import { createDebugLogger } from './logger.js'; -const noop = (): DebugEvent => ({ ts: '', level: 'debug', domain: 'app', event: '' }) as DebugEvent; -let debugLogger: DebugLogger = { - debug: noop, - info: noop, - warn: noop, - error: noop, - log: noop, - getRecentEvents: () => [], - currentFilePath: () => '', -}; +const MAX_PRE_INIT_BUFFER = 256; + +let debugLogger: DebugLogger; +let isInitialized = false; + +function createNoopLogger(): DebugLogger { + const preInitBuffer: DebugEvent[] = []; + + const flushToLogger = (logger: DebugLogger): void => { + for (const event of preInitBuffer) { + logger.log({ ...event, level: event.level as LogEventInput['level'] }); + } + preInitBuffer.length = 0; + }; + + const makeBufferedLog = + (level: 'debug' | 'info' | 'warn' | 'error') => + (input: LogEventInput): DebugEvent => { + const event: DebugEvent = { + ts: new Date().toISOString(), + level, + domain: input.domain, + event: input.event, + traceId: input.traceId, + feature: input.feature, + message: input.message, + gatewayId: input.gatewayId, + sessionKey: input.sessionKey, + taskId: input.taskId, + runId: input.runId, + requestId: input.requestId, + wsFrameId: input.wsFrameId, + seq: input.seq, + attempt: input.attempt, + durationMs: input.durationMs, + ok: input.ok, + error: input.error, + data: input.data, + }; + + if (preInitBuffer.length < MAX_PRE_INIT_BUFFER) { + preInitBuffer.push(event); + } + + console.warn( + `[debug] Logger not initialized yet (pre-init event captured, buffer size: ${preInitBuffer.length}/${MAX_PRE_INIT_BUFFER}):`, + `[${level}] [${input.domain}] ${input.event}`, + input, + ); + return event; + }; + + const noopLogger: DebugLogger = { + debug: makeBufferedLog('debug'), + info: makeBufferedLog('info'), + warn: makeBufferedLog('warn'), + error: makeBufferedLog('error'), + log: (input) => makeBufferedLog(input.level ?? 'debug')(input), + getRecentEvents: () => [...preInitBuffer], + currentFilePath: () => '', + }; + + Object.defineProperty(noopLogger, '__flush', { + value: flushToLogger, + writable: false, + enumerable: false, + }); + + return noopLogger; +} + +debugLogger = createNoopLogger(); export function initDebugLogger(debugDir: string): DebugLogger { - debugLogger = createDebugLogger({ + const realLogger = createDebugLogger({ debugDir, console: true, onEvent: broadcastDebugEvent, }); + + const noop = debugLogger as DebugLogger & { __flush?: (logger: DebugLogger) => void }; + if (noop.__flush) { + noop.__flush(realLogger); + } + + debugLogger = realLogger; + isInitialized = true; return debugLogger; } @@ -27,6 +97,10 @@ export function getDebugLogger(): DebugLogger { return debugLogger; } +export function isDebugLoggerInitialized(): boolean { + return isInitialized; +} + function broadcastDebugEvent(event: DebugEvent): void { for (const win of BrowserWindow.getAllWindows()) { try { diff --git a/packages/desktop/test/debug/debug-logger.test.ts b/packages/desktop/test/debug/debug-logger.test.ts new file mode 100644 index 000000000..67253ee9b --- /dev/null +++ b/packages/desktop/test/debug/debug-logger.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('electron', () => ({ + BrowserWindow: { + getAllWindows: () => [], + }, +})); + +describe('debug logger pre-init behavior', () => { + beforeEach(async () => { + vi.resetModules(); + vi.restoreAllMocks(); + }); + + it('buffers events before initDebugLogger runs', async () => { + const { getDebugLogger, isDebugLoggerInitialized } = await import( + '../../src/main/debug/index.js' + ); + + const logger = getDebugLogger(); + const event1 = logger.info({ domain: 'app', event: 'test-event-1' }); + const event2 = logger.error({ domain: 'gateway', event: 'test-event-2' }); + + expect(event1.level).toBe('info'); + expect(event1.domain).toBe('app'); + expect(event1.event).toBe('test-event-1'); + expect(event2.level).toBe('error'); + expect(event2.domain).toBe('gateway'); + + expect(getDebugLogger().getRecentEvents()).toHaveLength(2); + expect(isDebugLoggerInitialized()).toBe(false); + }); + + it('flushes buffered events to real logger after init', async () => { + const { getDebugLogger, initDebugLogger, isDebugLoggerInitialized } = await import( + '../../src/main/debug/index.js' + ); + + const logger = getDebugLogger(); + + logger.info({ domain: 'app', event: 'pre-init-event' }); + logger.warn({ domain: 'gateway', event: 'pre-init-warn' }); + + expect(isDebugLoggerInitialized()).toBe(false); + + const tempDir = `/tmp/test-debug-${Date.now()}`; + const realLogger = initDebugLogger(tempDir); + expect(isDebugLoggerInitialized()).toBe(true); + + const recentEvents = realLogger.getRecentEvents(); + expect(recentEvents.some((e) => e.event === 'pre-init-event')).toBe(true); + expect(recentEvents.some((e) => e.event === 'pre-init-warn')).toBe(true); + }); + + it('warns via console when logging before init', async () => { + const { getDebugLogger, isDebugLoggerInitialized } = await import( + '../../src/main/debug/index.js' + ); + + if (isDebugLoggerInitialized()) { + return; + } + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const logger = getDebugLogger(); + logger.error({ domain: 'app', event: 'pre-init-warning-test' }); + + expect(warnSpy).toHaveBeenCalled(); + const warningCall = warnSpy.mock.calls[0][0] as string; + expect(warningCall).toContain('[debug] Logger not initialized yet'); + + warnSpy.mockRestore(); + }); +}); From efa43225b28f73e3564b028c640db5fbbe21422f Mon Sep 17 00:00:00 2001 From: Claude Assistant Date: Mon, 20 Apr 2026 18:44:54 +0800 Subject: [PATCH 2/2] fix(debug): address code review feedback - Import sanitizeForLog to redact sensitive data before buffering - Fix buffer eviction: now drops oldest events (shift) instead of ignoring new ones - Simplify flushToLogger to pass event directly - Add test for oldest-event eviction behavior Review feedback from #448 --- packages/desktop/src/main/debug/index.ts | 32 ++++++------------- .../desktop/test/debug/debug-logger.test.ts | 26 ++++++++++++++- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/packages/desktop/src/main/debug/index.ts b/packages/desktop/src/main/debug/index.ts index 66e23d755..bed63c175 100644 --- a/packages/desktop/src/main/debug/index.ts +++ b/packages/desktop/src/main/debug/index.ts @@ -1,4 +1,5 @@ import { BrowserWindow } from 'electron'; +import { sanitizeForLog } from '@clawwork/shared'; import type { DebugEvent, LogEventInput } from '@clawwork/shared'; import type { DebugLogger } from './logger.js'; import { createDebugLogger } from './logger.js'; @@ -13,7 +14,7 @@ function createNoopLogger(): DebugLogger { const flushToLogger = (logger: DebugLogger): void => { for (const event of preInitBuffer) { - logger.log({ ...event, level: event.level as LogEventInput['level'] }); + logger.log(event); } preInitBuffer.length = 0; }; @@ -21,30 +22,15 @@ function createNoopLogger(): DebugLogger { const makeBufferedLog = (level: 'debug' | 'info' | 'warn' | 'error') => (input: LogEventInput): DebugEvent => { - const event: DebugEvent = { + const event = sanitizeForLog({ + ...input, ts: new Date().toISOString(), level, - domain: input.domain, - event: input.event, - traceId: input.traceId, - feature: input.feature, - message: input.message, - gatewayId: input.gatewayId, - sessionKey: input.sessionKey, - taskId: input.taskId, - runId: input.runId, - requestId: input.requestId, - wsFrameId: input.wsFrameId, - seq: input.seq, - attempt: input.attempt, - durationMs: input.durationMs, - ok: input.ok, - error: input.error, - data: input.data, - }; - - if (preInitBuffer.length < MAX_PRE_INIT_BUFFER) { - preInitBuffer.push(event); + } as DebugEvent); + + preInitBuffer.push(event); + if (preInitBuffer.length > MAX_PRE_INIT_BUFFER) { + preInitBuffer.shift(); } console.warn( diff --git a/packages/desktop/test/debug/debug-logger.test.ts b/packages/desktop/test/debug/debug-logger.test.ts index 67253ee9b..60947f377 100644 --- a/packages/desktop/test/debug/debug-logger.test.ts +++ b/packages/desktop/test/debug/debug-logger.test.ts @@ -43,7 +43,7 @@ describe('debug logger pre-init behavior', () => { expect(isDebugLoggerInitialized()).toBe(false); - const tempDir = `/tmp/test-debug-${Date.now()}`; + const tempDir = '/tmp/test-debug'; const realLogger = initDebugLogger(tempDir); expect(isDebugLoggerInitialized()).toBe(true); @@ -52,6 +52,30 @@ describe('debug logger pre-init behavior', () => { expect(recentEvents.some((e) => e.event === 'pre-init-warn')).toBe(true); }); + it('drops oldest events when buffer is full (keeps most recent 256)', async () => { + const { getDebugLogger, initDebugLogger } = await import( + '../../src/main/debug/index.js' + ); + + const logger = getDebugLogger(); + + logger.info({ domain: 'app', event: 'oldest-event' }); + for (let i = 1; i <= 256; i++) { + logger.debug({ domain: 'app', event: `buffer-overflow-test-${i}` }); + } + + const events = getDebugLogger().getRecentEvents(); + expect(events).toHaveLength(256); + expect(events.some((e) => e.event === 'oldest-event')).toBe(false); + expect(events[0].event).toBe('buffer-overflow-test-1'); + expect(events[255].event).toBe('buffer-overflow-test-256'); + + const tempDir = '/tmp/test-debug'; + const realLogger = initDebugLogger(tempDir); + const flushedEvents = realLogger.getRecentEvents(); + expect(flushedEvents.some((e) => e.event === 'oldest-event')).toBe(false); + }); + it('warns via console when logging before init', async () => { const { getDebugLogger, isDebugLoggerInitialized } = await import( '../../src/main/debug/index.js'