diff --git a/.changeset/error-log-raw-err-convention.md b/.changeset/error-log-raw-err-convention.md new file mode 100644 index 000000000..8cdf2b40d --- /dev/null +++ b/.changeset/error-log-raw-err-convention.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Error and warn log lines across the server, CLI, and desktop main process now attach the raw error under the `err` key, so on-disk JSONL logs (what bug-report bundles collect) carry the full name/message/stack instead of a pre-stringified message with no stack. API error log lines additionally carry the request's `x-request-id` for correlation with the access log and client reports, the MCP stdio logger serializes Error values instead of flattening them to `{}`, and the desktop root logger gained explicit `err` serializers. No wire-shape changes. diff --git a/packages/cli/src/commands/auth/git-credential.ts b/packages/cli/src/commands/auth/git-credential.ts index 81f66fd6e..ab99bfa9b 100644 --- a/packages/cli/src/commands/auth/git-credential.ts +++ b/packages/cli/src/commands/auth/git-credential.ts @@ -163,10 +163,7 @@ export function gitCredentialCommand( // A throw from getTokenStore / a callback / handleCredentialGet must // not skip the flush — that's the exact failure (a vanished credential) // we need persisted. Log and flush before exiting non-zero. - log?.error( - { error: err instanceof Error ? err.message : String(err) }, - '[auth] git-credential get: unexpected error', - ); + log?.error({ err }, '[auth] git-credential get: unexpected error'); await flushFileLogger(log); process.exit(1); } diff --git a/packages/cli/src/commands/bug-report-bundle.ts b/packages/cli/src/commands/bug-report-bundle.ts index 721a2858a..d8a1de065 100644 --- a/packages/cli/src/commands/bug-report-bundle.ts +++ b/packages/cli/src/commands/bug-report-bundle.ts @@ -109,7 +109,7 @@ export function resolveProjectSlug(cwd: string, logger?: BundleLogger): string | // (or null), but log it so a missing/wrong project slug in the bundle is // diagnosable rather than silent — same rationale as resolveContentDir. logger?.warn( - { configPath, err: err instanceof Error ? err.message : String(err) }, + { configPath, err }, 'bug-report: failed to read .ok/config.yml for project slug; using path-hash fallback', ); } @@ -248,10 +248,7 @@ function addContentFiles(args: { // A file we listed but can't read is dropped rather than aborting the // whole report; log it so the omission is diagnosable — the bundled // MANIFEST lists only what was written, never what was skipped. - args.logger?.warn( - { file, prefix: args.prefix, err: err instanceof Error ? err.message : String(err) }, - 'bug-report: skipped unreadable file', - ); + args.logger?.warn({ file, prefix: args.prefix, err }, 'bug-report: skipped unreadable file'); } } } diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 9ff5063d9..3e02f63dd 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -594,24 +594,15 @@ export function buildIdleShutdownHandler( 'idle-shutdown: SIGTERM grace expired — escalated to SIGKILL', ); } catch (err) { - input.log?.error( - { pid: lock.pid, err: err instanceof Error ? err.message : String(err) }, - 'idle-shutdown: SIGKILL failed', - ); + input.log?.error({ pid: lock.pid, err }, 'idle-shutdown: SIGKILL failed'); } } } catch (err) { - input.log?.warn( - { pid: lock.pid, err: err instanceof Error ? err.message : String(err) }, - 'idle-shutdown: failed to SIGTERM UI sibling', - ); + input.log?.warn({ pid: lock.pid, err }, 'idle-shutdown: failed to SIGTERM UI sibling'); } } } catch (err) { - input.log?.warn( - { err: err instanceof Error ? err.message : String(err) }, - 'idle-shutdown: UI lookup failed; proceeding with destroy', - ); + input.log?.warn({ err }, 'idle-shutdown: UI lookup failed; proceeding with destroy'); } await input.destroy(); }; diff --git a/packages/cli/src/report-bundle.ts b/packages/cli/src/report-bundle.ts index 51d0c5474..2eb97c2b2 100644 --- a/packages/cli/src/report-bundle.ts +++ b/packages/cli/src/report-bundle.ts @@ -94,7 +94,7 @@ function resolveContentDir(projectDir: string, logger?: BundleLogger): string { // root (the bundle should still succeed), but log it so a wrong // content-dir in the resulting bundle is diagnosable, not silent. logger?.warn( - { configPath, err: err instanceof Error ? err.message : String(err) }, + { configPath, err }, 'bug-report: failed to read .ok/config.yml; falling back to project root as content dir', ); } diff --git a/packages/desktop/src/main/auto-updater.ts b/packages/desktop/src/main/auto-updater.ts index 0fa9044af..fbbb2519a 100644 --- a/packages/desktop/src/main/auto-updater.ts +++ b/packages/desktop/src/main/auto-updater.ts @@ -670,7 +670,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa // consistent (fallback-attempted, no re-check) state instead. logger.error('proxy-feed fallback setFeedURL threw', { cause, - message: err instanceof Error ? err.message : String(err), + err, }); return; } @@ -680,7 +680,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa // right after the proxy one is operationally relevant, not debug noise. const ctx = { code: err?.code, - message: err instanceof Error ? err.message : String(err), + err, }; if (isClassifiedUpdaterError(err)) { logger.warn('post-fallback checkForUpdates rejected', ctx); @@ -754,7 +754,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa } catch (err) { logger.error('writeState failed — state gate not armed', { ctx, - message: err instanceof Error ? err.message : String(err), + err, }); return false; } @@ -937,7 +937,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa const logFn = isClassifiedUpdaterError(err) ? logger.warn : logger.debug; logFn('check-now checkForUpdates rejected', { code, - message: err instanceof Error ? err.message : String(err), + err, timestamp: now().toISOString(), }); // The synchronous-reject path is rare (electron-updater normally emits @@ -1013,8 +1013,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa const logFn = isClassifiedUpdaterError(err) ? logger.warn : logger.debug; logFn('downloadUpdate rejected', { code, - message: err instanceof Error ? err.message : String(err), - stack: err instanceof Error ? err.stack : undefined, + err, timestamp: now().toISOString(), }); }); @@ -1130,14 +1129,13 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa if (isClassifiedUpdaterError(err)) { logger.warn('error (classified)', { code: err.code, - message: err.message, + err, timestamp: now().toISOString(), }); onDispatch?.('error-classified'); } else { logger.error('error (unclassified)', { - message: err.message, - stack: err.stack, + err, timestamp: now().toISOString(), }); onDispatch?.('error-unclassified'); @@ -1268,7 +1266,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa await opts.prepareForRelaunch(); } catch (err) { logger.warn('prepareForRelaunch threw — proceeding to quitAndInstall anyway', { - message: err instanceof Error ? err.message : String(err), + err, }); } } @@ -1581,7 +1579,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa // also emits `error` for these, so the catch here is just a defensive // log. Event handlers run either way. logger.debug('checkForUpdates rejected', { - message: err instanceof Error ? err.message : String(err), + err, }); }); scheduleNextCheck(); @@ -1604,7 +1602,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa }) .catch((err: unknown) => { logger.debug('first-launch checkForUpdates rejected', { - message: err instanceof Error ? err.message : String(err), + err, }); // If the proxy feed caused it, revert to GitHub and re-check once. revertToGithubFeed('first-check-rejected'); @@ -1661,7 +1659,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa } catch (err) { logger.warn('updater.off failed during destroy', { event, - message: err instanceof Error ? err.message : String(err), + err, }); } }; @@ -1678,7 +1676,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa } catch (err) { logger.warn('ipcMain.removeHandler failed during destroy', { channel, - message: err instanceof Error ? err.message : String(err), + err, }); } }; @@ -1747,8 +1745,7 @@ export async function bootAutoUpdater( return startAutoUpdater({ updater: autoUpdater, ...opts }); } catch (err) { logger.error('auto-updater boot failed — app will run without updates this session', { - message: err instanceof Error ? err.message : String(err), - stack: err instanceof Error ? err.stack : undefined, + err, }); return null; } diff --git a/packages/desktop/src/main/branch-info-proxy.ts b/packages/desktop/src/main/branch-info-proxy.ts index 5bfa943e5..3809d0e44 100644 --- a/packages/desktop/src/main/branch-info-proxy.ts +++ b/packages/desktop/src/main/branch-info-proxy.ts @@ -163,7 +163,7 @@ export async function proxyFetchBranchInfo( raw = await res.json(); } catch (err) { deps.log?.warn('[branch-info-proxy] branch-info fetch failed', { - err: err instanceof Error ? err.message : String(err), + err, }); return null; } @@ -247,7 +247,7 @@ export async function proxyAwaitBranchSwitched( } } catch (err) { deps.log?.warn('[branch-info-proxy] server-info poll failed (will retry)', { - err: err instanceof Error ? err.message : String(err), + err, }); } if (raw !== undefined) { @@ -302,7 +302,7 @@ export async function proxyRunCheckout( raw = await res.json(); } catch (err) { deps.log?.warn('[branch-info-proxy] checkout fetch failed', { - err: err instanceof Error ? err.message : String(err), + err, }); return null; } @@ -364,7 +364,7 @@ export async function proxyShareTargetStatus( raw = await res.json(); } catch (err) { deps.log?.warn('[branch-info-proxy] target-status fetch failed', { - err: err instanceof Error ? err.message : String(err), + err, }); return null; } diff --git a/packages/desktop/src/main/bundle-replace-detector.ts b/packages/desktop/src/main/bundle-replace-detector.ts index 715f28325..793857762 100644 --- a/packages/desktop/src/main/bundle-replace-detector.ts +++ b/packages/desktop/src/main/bundle-replace-detector.ts @@ -193,7 +193,7 @@ export function startBundleReplaceWatcher( }); } catch (err) { logger.warn('detector threw', { - err: err instanceof Error ? err.message : String(err), + err, }); return; } @@ -242,7 +242,7 @@ export function startBundleReplaceWatcher( // log accumulates one entry per interval — bounded by intervalMs. if (!stopped) armed = true; logger.warn('dialog failed, re-armed for next tick', { - err: err instanceof Error ? err.message : String(err), + err, }); }); }; diff --git a/packages/desktop/src/main/consent-dialog.ts b/packages/desktop/src/main/consent-dialog.ts index e020066e0..a44cce3f1 100644 --- a/packages/desktop/src/main/consent-dialog.ts +++ b/packages/desktop/src/main/consent-dialog.ts @@ -164,28 +164,28 @@ export function requestUserConsent( ipcMain.removeHandler('ok:onboarding:confirm'); } catch (err) { logger.warn('removeHandler(confirm) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } try { ipcMain.removeHandler('ok:onboarding:cancel'); } catch (err) { logger.warn('removeHandler(cancel) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } try { ipcMain.removeHandler('ok:onboarding:probe-content'); } catch (err) { logger.warn('removeHandler(probe-content) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } try { ipcMain.removeHandler('ok:onboarding:renderer-ready'); } catch (err) { logger.warn('removeHandler(renderer-ready) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } } @@ -317,7 +317,7 @@ export function requestUserConsent( sendToRenderer(event.sender, 'ok:onboarding:show', payload); } catch (err) { logger.error('show dispatch failed — handler stays armed for retry', { - message: err instanceof Error ? err.message : String(err), + err, }); return undefined; } @@ -351,7 +351,7 @@ export function requestUserConsent( capturedSenderId = navigator.id; } catch (err) { logger.error('proactive show dispatch failed — falling back to renderer-ready', { - message: err instanceof Error ? err.message : String(err), + err, }); } } diff --git a/packages/desktop/src/main/crash-detection.ts b/packages/desktop/src/main/crash-detection.ts index 01b0c687d..8585b7cbb 100644 --- a/packages/desktop/src/main/crash-detection.ts +++ b/packages/desktop/src/main/crash-detection.ts @@ -277,7 +277,7 @@ export function createCrashDetection(deps: CrashDetectionDeps): CrashDetection { { event: 'crash-detection.sentinel-write-failed', context, - cause: err instanceof Error ? err.message : String(err), + err, }, context === 'arm' ? 'could not arm the dirty-shutdown sentinel' @@ -315,7 +315,7 @@ export function createCrashDetection(deps: CrashDetectionDeps): CrashDetection { deps.logger.warn( { event: 'crash-detection.store-write-failed', - cause: err instanceof Error ? err.message : String(err), + err, }, 'could not persist crash acknowledgment state', ); @@ -546,7 +546,7 @@ export function createCrashDetection(deps: CrashDetectionDeps): CrashDetection { deps.logger.warn( { event: 'crash-detection.sentinel-clear-failed', - cause: err instanceof Error ? err.message : String(err), + err, }, 'could not clear the dirty-shutdown sentinel — next boot may prompt spuriously', ); diff --git a/packages/desktop/src/main/desktop-logger.ts b/packages/desktop/src/main/desktop-logger.ts index d36d74e85..89e56230e 100644 --- a/packages/desktop/src/main/desktop-logger.ts +++ b/packages/desktop/src/main/desktop-logger.ts @@ -104,6 +104,10 @@ function getRootLogger(): pino.Logger { level: resolveLogLevel(), name: loggerName, redact: { paths: REDACT_PATHS, censor: '[REDACTED]' }, + // Raw Errors land under `err` (convention) — serialize name/message/stack + // explicitly on BOTH keys so a stray `error:` field never flattens an + // Error to `{}` in the JSONL file. Mirrors the server logger's setup. + serializers: { err: pino.stdSerializers.err, error: pino.stdSerializers.err }, base: { pid: process.pid, hostname: undefined, runtime: 'desktop' }, timestamp: pino.stdTimeFunctions.isoTime, }, diff --git a/packages/desktop/src/main/git-preflight-handler.ts b/packages/desktop/src/main/git-preflight-handler.ts index fae943799..d2a97f43d 100644 --- a/packages/desktop/src/main/git-preflight-handler.ts +++ b/packages/desktop/src/main/git-preflight-handler.ts @@ -115,7 +115,7 @@ async function showUnknownErrorDialog(deps: EnsureGitDeps, err: Error): Promise< }); } catch (dialogErr) { deps.log?.warn('ensureGitAvailable: unknown-error dialog failed', { - err: dialogErr instanceof Error ? dialogErr.message : String(dialogErr), + err: dialogErr, }); } } @@ -201,7 +201,7 @@ export async function ensureGitAvailable(deps: EnsureGitDeps): Promise { void openExternalSafely(url).catch((err: unknown) => { - getLogger('spellcheck-menu').warn( - { err: err instanceof Error ? err.message : String(err), url }, - 'context-menu search openExternal failed', - ); + getLogger('spellcheck-menu').warn({ err, url }, 'context-menu search openExternal failed'); }); }, popMenu: (input) => { @@ -2197,7 +2194,7 @@ async function openEphemeralFile(filePath: string): Promise { refreshApplicationMenu(); } catch (err) { getLogger('project').error( - { file: plan.canonicalFilePath, err: err instanceof Error ? err.message : String(err) }, + { file: plan.canonicalFilePath, err }, 'ephemeral single-file open failed', ); dialog.showErrorBox( @@ -2346,10 +2343,7 @@ async function runApplicationMenuRefresh(): Promise { onUninstall: desktopSelfUninstallAvailable() ? () => void startDesktopSelfUninstallFlow().catch((err) => { - getLogger('lifecycle').error( - { err: err instanceof Error ? err.message : String(err) }, - 'desktop self-uninstall flow failed', - ); + getLogger('lifecycle').error({ err }, 'desktop self-uninstall flow failed'); }) : undefined, // File menu state-aware items. activeTarget drives enable/disable; @@ -2467,10 +2461,7 @@ async function showDesktopUninstallNotice( `data:text/html;charset=utf-8,${encodeURIComponent(buildDesktopUninstallNoticeHtml(spec))}`, ) .catch((err) => { - getLogger('lifecycle').warn( - { err: err instanceof Error ? err.message : String(err) }, - 'desktop uninstall notice failed to load', - ); + getLogger('lifecycle').warn({ err }, 'desktop uninstall notice failed to load'); finish(closeMeansConfirm); }); }); @@ -2567,10 +2558,7 @@ async function showDesktopUninstallProjectPicker( )}`, ) .catch((err) => { - getLogger('lifecycle').warn( - { err: err instanceof Error ? err.message : String(err) }, - 'desktop uninstall project picker failed to load', - ); + getLogger('lifecycle').warn({ err }, 'desktop uninstall project picker failed to load'); finish({ action: 'cancel' }); }); }); @@ -2603,10 +2591,7 @@ async function withDesktopUninstallProgress(work: () => Promise): Promise< `data:text/html;charset=utf-8,${encodeURIComponent(buildDesktopUninstallProgressHtml())}`, ); } catch (err) { - getLogger('lifecycle').warn( - { err: err instanceof Error ? err.message : String(err) }, - 'desktop uninstall progress window failed to load', - ); + getLogger('lifecycle').warn({ err }, 'desktop uninstall progress window failed to load'); } return await work(); } finally { @@ -2634,7 +2619,7 @@ async function startDesktopSelfUninstallFlow(): Promise { lockDirs = await discoverLockDirs(); } catch (err) { getLogger('lifecycle').warn( - { err: err instanceof Error ? err.message : String(err) }, + { err }, 'desktop self-uninstall could not discover running project locks', ); } @@ -4423,7 +4408,7 @@ function registerIpcHandlers() { ); } catch (err) { getLogger('project').warn( - { gitRoot, err: err instanceof Error ? err.message : String(err) }, + { gitRoot, err }, 'remove-git-folder: worktree server stop failed', ); } @@ -5073,7 +5058,7 @@ const safetyNetLogger = getLogger('process-safety-net'); installStdioBrokenPipeGuard(process, { onNonBenignError: (stream, err) => { safetyNetLogger.error( - { stream, code: (err as NodeJS.ErrnoException).code, message: err.message }, + { stream, code: (err as NodeJS.ErrnoException).code, err }, 'unexpected stdio stream error', ); }, diff --git a/packages/desktop/src/main/integrations-settings.ts b/packages/desktop/src/main/integrations-settings.ts index 0779e73dc..20e0cfd65 100644 --- a/packages/desktop/src/main/integrations-settings.ts +++ b/packages/desktop/src/main/integrations-settings.ts @@ -169,7 +169,7 @@ export function registerIntegrationsSettings( // not take the whole section down — surface the row as unmanageable. logger.warn('editor classify failed', { id, - error: err instanceof Error ? err.message : String(err), + err, }); state = 'unmanageable'; } @@ -190,7 +190,7 @@ export function registerIntegrationsSettings( pathStatus = path.computeStatus(); } catch (err) { logger.warn('path status failed', { - error: err instanceof Error ? err.message : String(err), + err, }); pathStatus = { shellDetected: false, rcFilesToTouch: [], installed: false }; } @@ -199,7 +199,7 @@ export function registerIntegrationsSettings( skillStatuses = skills.computeStatuses(); } catch (err) { logger.warn('skill statuses failed', { - error: err instanceof Error ? err.message : String(err), + err, }); skillStatuses = []; } @@ -230,7 +230,7 @@ export function registerIntegrationsSettings( // Bookkeeping only — the entry write itself already succeeded, and the // startup repair scans configs directly rather than trusting the list. logger.warn('marker refresh failed', { - error: err instanceof Error ? err.message : String(err), + err, }); } } @@ -360,7 +360,7 @@ export function registerIntegrationsSettings( ipcMain.removeHandler('ok:integrations:dispatch'); } catch (err) { logger.warn('removeHandler(ok:integrations:dispatch) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } }, diff --git a/packages/desktop/src/main/ipc/bug-report.test.ts b/packages/desktop/src/main/ipc/bug-report.test.ts index 2495c319b..be188c0b5 100644 --- a/packages/desktop/src/main/ipc/bug-report.test.ts +++ b/packages/desktop/src/main/ipc/bug-report.test.ts @@ -12,7 +12,6 @@ * becomes a load-order-dependent failure in another suite. */ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import { execFileSync, execSync } from 'node:child_process'; import { existsSync, @@ -29,6 +28,7 @@ import { import { createServer, type Server } from 'node:http'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; import { createCrashDetection } from '../crash-detection.ts'; import { handleShellOpenExternal } from '../shell-allowlist.ts'; import { @@ -1222,7 +1222,7 @@ describe('handleBugReportCaptureScreenshot', () => { expect(await handleBugReportCaptureScreenshot(deps)).toBeNull(); expect(store.has(7)).toBe(false); expect(warnings).toHaveLength(1); - expect(warnings[0]?.payload.err).toContain('offscreen surface'); + expect((warnings[0]?.payload.err as Error).message).toContain('offscreen surface'); }); }); diff --git a/packages/desktop/src/main/ipc/bug-report.ts b/packages/desktop/src/main/ipc/bug-report.ts index 9a2f751ba..b49d4c51d 100644 --- a/packages/desktop/src/main/ipc/bug-report.ts +++ b/packages/desktop/src/main/ipc/bug-report.ts @@ -236,7 +236,7 @@ export async function handleBugReportCreate( // change the create outcome. await unlink(screenshotTmpPath).catch((err: unknown) => { deps.logger?.warn( - { screenshotTmpPath, err: err instanceof Error ? err.message : String(err) }, + { screenshotTmpPath, err }, 'bug-report: failed to remove temp screenshot file', ); }); @@ -323,7 +323,7 @@ export async function handleBugReportCaptureScreenshot( } catch (err) { dropExisting(); deps.logger?.warn( - { err: err instanceof Error ? err.message : String(err) }, + { err }, 'bug-report: screenshot capture failed; dialog will omit the screenshot option', ); return null; diff --git a/packages/desktop/src/main/mcp-wiring.ts b/packages/desktop/src/main/mcp-wiring.ts index 2d45ab03f..4b2331719 100644 --- a/packages/desktop/src/main/mcp-wiring.ts +++ b/packages/desktop/src/main/mcp-wiring.ts @@ -746,7 +746,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru }); } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error('detection failed — wiring inert for this boot', { message }); + logger.error('detection failed — wiring inert for this boot', { err }); logger.event({ event: 'mcp-wiring-detect-failed', error: message }); return inertHandle; } @@ -761,7 +761,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru pathDescriptor = pathInstall.computeDescriptor(); } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error('path-install descriptor failed — PATH row hidden for this boot', { message }); + logger.error('path-install descriptor failed — PATH row hidden for this boot', { err }); logger.event({ event: 'mcp-wiring-path-descriptor-failed', error: message }); pathDescriptor = { shellDetected: false, rcFilesToTouch: [], alreadyInstalled: false }; } @@ -774,7 +774,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru skillDescriptors = skills.computeDescriptors(); } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error('skill descriptors failed — skill rows hidden for this boot', { message }); + logger.error('skill descriptors failed — skill rows hidden for this boot', { err }); logger.event({ event: 'mcp-wiring-skill-descriptors-failed', error: message }); skillDescriptors = []; } @@ -840,7 +840,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru }); } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error('writeUserMcpConfigs threw — marker not written', { message }); + logger.error('writeUserMcpConfigs threw — marker not written', { err }); logIpcError({ event: 'ipc.error', channel: 'ok:mcp-wiring:confirm', @@ -1018,7 +1018,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru ); } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error('marker write failed', { message }); + logger.error('marker write failed', { err }); logIpcError({ event: 'ipc.error', channel: 'ok:mcp-wiring:confirm', @@ -1071,7 +1071,7 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru // dialog re-fires next boot with no explanation. Reset `handled` so // the user can retry Skip from the still-mounted dialog. const message = err instanceof Error ? err.message : String(err); - logger.error('skip-marker write failed', { message }); + logger.error('skip-marker write failed', { err }); logIpcError({ event: 'ipc.error', channel: 'ok:mcp-wiring:skip', @@ -1134,9 +1134,8 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru senderId: target.id, }); } catch (err) { - const message = err instanceof Error ? err.message : String(err); logger.error('show dispatch failed — handler remains armed for next renderer', { - message, + err, }); return false; } @@ -1195,21 +1194,21 @@ export function runMcpWiringOnFirstLaunch(opts: RunMcpWiringFirstLaunchOpts): Ru ipcMain.removeHandler('ok:mcp-wiring:confirm'); } catch (err) { logger.warn('removeHandler(confirm) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } try { ipcMain.removeHandler('ok:mcp-wiring:skip'); } catch (err) { logger.warn('removeHandler(skip) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } try { ipcMain.removeHandler('ok:mcp-wiring:renderer-ready'); } catch (err) { logger.warn('removeHandler(renderer-ready) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } }, diff --git a/packages/desktop/src/main/project-integrations-settings.ts b/packages/desktop/src/main/project-integrations-settings.ts index 34ed73cb4..de5022337 100644 --- a/packages/desktop/src/main/project-integrations-settings.ts +++ b/packages/desktop/src/main/project-integrations-settings.ts @@ -177,7 +177,7 @@ export function registerProjectIntegrationsSettings( logger.warn('project editor classify failed', { projectDir, id, - error: err instanceof Error ? err.message : String(err), + err, }); state = 'unmanageable'; } @@ -203,7 +203,7 @@ export function registerProjectIntegrationsSettings( } catch (err) { logger.warn('project editor statuses failed', { projectDir, - error: err instanceof Error ? err.message : String(err), + err, }); editors = []; } @@ -219,7 +219,7 @@ export function registerProjectIntegrationsSettings( } catch (err) { logger.warn('project skill status failed', { projectDir, - error: err instanceof Error ? err.message : String(err), + err, }); } skill = { installed, paths: skillPaths }; @@ -389,7 +389,7 @@ export function registerProjectIntegrationsSettings( projectDir = resolveProjectDir(event); } catch (err) { logger.warn('resolveProjectDir threw', { - error: err instanceof Error ? err.message : String(err), + err, }); projectDir = null; } @@ -406,7 +406,7 @@ export function registerProjectIntegrationsSettings( ipcMain.removeHandler('ok:project-integrations:dispatch'); } catch (err) { logger.warn('removeHandler(ok:project-integrations:dispatch) threw', { - message: err instanceof Error ? err.message : String(err), + err, }); } }, diff --git a/packages/desktop/src/main/server-exit-record.ts b/packages/desktop/src/main/server-exit-record.ts index b993e8f07..babeeaf3a 100644 --- a/packages/desktop/src/main/server-exit-record.ts +++ b/packages/desktop/src/main/server-exit-record.ts @@ -87,7 +87,7 @@ export function createServerExitRecorder(deps: ServerExitRecorderDeps): ServerEx deps.logger.warn( { event: 'server-exit-record.write-failed', - cause: err instanceof Error ? err.message : String(err), + err, }, 'could not record server exit', ); diff --git a/packages/desktop/src/main/share-handoff.ts b/packages/desktop/src/main/share-handoff.ts index b328d76e1..8cda03c6b 100644 --- a/packages/desktop/src/main/share-handoff.ts +++ b/packages/desktop/src/main/share-handoff.ts @@ -261,10 +261,7 @@ export function startFirstRunHandshake(deps: FirstRunHandshakeDeps): void { try { deps.routeShareUrl(decision.shareUrl); } catch (err) { - deps.log?.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[receive] source=deferred routeShareUrl threw', - ); + deps.log?.warn({ err }, '[receive] source=deferred routeShareUrl threw'); } } else { res.statusCode = 400; diff --git a/packages/desktop/src/main/show-gate.ts b/packages/desktop/src/main/show-gate.ts index d844f33f3..110403d04 100644 --- a/packages/desktop/src/main/show-gate.ts +++ b/packages/desktop/src/main/show-gate.ts @@ -129,7 +129,7 @@ export function createShowGateRegistry(deps: ShowGateRegistryDeps): ShowGateRegi { event: 'show-gate-on-shown-failed', windowKind: state.kind, - error: cbErr instanceof Error ? cbErr.message : String(cbErr), + err: cbErr, }, 'show-gate onShown callback threw', ); @@ -139,7 +139,7 @@ export function createShowGateRegistry(deps: ShowGateRegistryDeps): ShowGateRegi { event: 'show-gate-show-failed', windowKind: state.kind, - error: err instanceof Error ? err.message : String(err), + err, }, 'window.show() threw past the destroyed-window guard', ); diff --git a/packages/desktop/src/main/startup-trace.ts b/packages/desktop/src/main/startup-trace.ts index 2df54cf1e..9930e6e1c 100644 --- a/packages/desktop/src/main/startup-trace.ts +++ b/packages/desktop/src/main/startup-trace.ts @@ -55,7 +55,7 @@ export function beginRoot(): boolean { return true; } catch (err) { getLogger('startup-trace').warn( - { err: err instanceof Error ? err.message : String(err) }, + { err }, 'OTel root init failed in main — degrading to waterfall-log-only (Plan B)', ); rootSpan = undefined; diff --git a/packages/desktop/src/main/state-store.ts b/packages/desktop/src/main/state-store.ts index 551338ace..de7350469 100644 --- a/packages/desktop/src/main/state-store.ts +++ b/packages/desktop/src/main/state-store.ts @@ -526,7 +526,7 @@ export function saveAppStateToDir( return true; } catch (err) { logger.error('[main] saveAppState failed', { - err: (err as Error).message, + err, statePath, }); try { @@ -538,7 +538,7 @@ export function saveAppStateToDir( } } catch (err) { logger.error('[main] saveAppState userData setup failed', { - err: (err as Error).message, + err, userDataDir, }); return false; diff --git a/packages/desktop/src/main/url-scheme.ts b/packages/desktop/src/main/url-scheme.ts index a9134f852..3666f17b3 100644 --- a/packages/desktop/src/main/url-scheme.ts +++ b/packages/desktop/src/main/url-scheme.ts @@ -885,17 +885,14 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand deps.app.removeAsDefaultProtocolClient('openknowledge'); } catch (err) { deps.log?.warn( - { err: (err as Error).message }, + { err }, '[url-scheme] removeAsDefaultProtocolClient failed on before-quit', ); } }); } } catch (err) { - deps.log?.warn( - { err: (err as Error).message }, - '[url-scheme] setAsDefaultProtocolClient failed', - ); + deps.log?.warn({ err }, '[url-scheme] setAsDefaultProtocolClient failed'); } } @@ -1189,10 +1186,7 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand } }, (err) => { - deps.log?.warn( - { err: err instanceof Error ? err.message : String(err), url }, - '[receive] foreign-host gate rejected — share dropped', - ); + deps.log?.warn({ err, url }, '[receive] foreign-host gate rejected — share dropped'); }, ); return; @@ -1206,7 +1200,7 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand // gets a forward path (clone / locate manually) rather than a silent // drop, uniform with how resolution itself handles failure. deps.log?.warn( - { err: err instanceof Error ? err.message : String(err), url }, + { err, url }, '[receive] resolveShareTarget rejected — degrading to Navigator (miss)', ); dispatchResolvedShare(url, result.payload, { kind: 'miss' }); @@ -1260,10 +1254,7 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand return; } void open(fileOpen.file).catch((err) => { - deps.log?.warn( - { err: (err as Error).message, file: fileOpen.file }, - '[url-scheme] openEphemeralFile failed', - ); + deps.log?.warn({ err, file: fileOpen.file }, '[url-scheme] openEphemeralFile failed'); }); return; } @@ -1299,10 +1290,7 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand pendingDeepLinkTarget: { kind: parsed.kind, path: parsed.doc }, }) .catch((err) => { - deps.log?.warn( - { err: (err as Error).message, project: parsed.project }, - '[url-scheme] openProject failed', - ); + deps.log?.warn({ err, project: parsed.project }, '[url-scheme] openProject failed'); }); }; diff --git a/packages/desktop/src/main/window-manager.ts b/packages/desktop/src/main/window-manager.ts index ced07a666..031db76b6 100644 --- a/packages/desktop/src/main/window-manager.ts +++ b/packages/desktop/src/main/window-manager.ts @@ -775,7 +775,7 @@ export function signalDetachedServerStop( log?.warn( { event: 'update-install-server-stop-failed', - err: (err as Error).message, + err, code, pid, projectPath, @@ -805,7 +805,7 @@ export function signalStopOwnedUtilityForks( ctx.utility.kill('SIGKILL'); } catch (err) { log?.warn( - { err: (err as Error).message, projectPath: ctx.projectPath }, + { err, projectPath: ctx.projectPath }, 'utility SIGKILL failed during owned-server teardown', ); } @@ -1031,10 +1031,7 @@ export class WindowManager { if ((err as NodeJS.ErrnoException).code === 'ESRCH') { return; } - this.deps.log?.warn( - { err: (err as Error).message, pid, projectPath }, - 'SIGTERM failed during stopAllOwnedServers', - ); + this.deps.log?.warn({ err, pid, projectPath }, 'SIGTERM failed during stopAllOwnedServers'); } // Poll for PROCESS death, not lock release. The lock disappears while // the process is still flushing telemetry/logs (and historically, @@ -1072,7 +1069,7 @@ export class WindowManager { this.deps.log?.warn( { event: 'auto-update-server-stop-sigkill-failed', - err: (err as Error).message, + err, code, pid, projectPath, @@ -1554,10 +1551,7 @@ export class WindowManager { try { await this.deps.runClean({ lockDir }); } catch (err) { - this.deps.log?.warn( - { err: (err as Error).message, lockDir }, - 'runClean failed; proceeding to spawn server', - ); + this.deps.log?.warn({ err, lockDir }, 'runClean failed; proceeding to spawn server'); } } @@ -1595,7 +1589,7 @@ export class WindowManager { this.deps.log?.warn( { event: 'desktop-spawn-orphan-sigterm-failed', - err: (signalErr as Error).message, + err: signalErr, code, pid: handle.pid, projectPath, @@ -1901,7 +1895,7 @@ export class WindowManager { utility.postMessage({ type: 'shutdown' }); } catch (err) { this.deps.log?.warn( - { err: (err as Error).message, projectPath }, + { err, projectPath }, 'utility shutdown IPC failed on window close (likely already exited)', ); } @@ -2053,7 +2047,7 @@ export class WindowManager { this.deps.log?.warn( { event: 'desktop-ephemeral-spawn-orphan-sigterm-failed', - err: (signalErr as Error).message, + err: signalErr, code, pid: handle.pid, }, @@ -2215,7 +2209,7 @@ export class WindowManager { this.deps.log?.warn( { event: 'desktop-ephemeral-teardown', - err: err instanceof Error ? err.message : String(err), + err, projectDir: session.projectDir, }, '[window-manager] failed to remove ephemeral temp dir', @@ -2239,7 +2233,7 @@ export class WindowManager { ctx.utility.postMessage({ type: 'shutdown' }); } catch (err) { this.deps.log?.warn( - { err: (err as Error).message, projectPath }, + { err, projectPath }, 'utility shutdown IPC failed in closeProjectWindow (likely already exited)', ); } diff --git a/packages/desktop/src/main/worktree-setup-inherit.ts b/packages/desktop/src/main/worktree-setup-inherit.ts index d38c123a9..22cf2a330 100644 --- a/packages/desktop/src/main/worktree-setup-inherit.ts +++ b/packages/desktop/src/main/worktree-setup-inherit.ts @@ -158,10 +158,7 @@ export function seedWorktreeProjectSetup(worktreePath: string, mainRoot: string) try { initContent(worktreePath, { contentDir: readRootContentDir(mainRoot) }); } catch (err) { - logger.warn( - { worktreePath, err: err instanceof Error ? err.message : String(err) }, - 'failed to seed inherited .ok/ scaffold', - ); + logger.warn({ worktreePath, err }, 'failed to seed inherited .ok/ scaffold'); } // 2. Editor/MCP wiring, mirroring exactly the editors the root has wired. @@ -183,9 +180,6 @@ export function seedWorktreeProjectSetup(worktreePath: string, mainRoot: string) } catch (err) { // Defensive: the orchestrator is contract-bound not to throw, but a future // change must never let a wiring error abort the worktree open. - logger.warn( - { worktreePath, err: err instanceof Error ? err.message : String(err) }, - 'failed to seed inherited editor integrations', - ); + logger.warn({ worktreePath, err }, 'failed to seed inherited editor integrations'); } } diff --git a/packages/desktop/tests/integration/auto-updater.test.ts b/packages/desktop/tests/integration/auto-updater.test.ts index d87547f4c..10478bce3 100644 --- a/packages/desktop/tests/integration/auto-updater.test.ts +++ b/packages/desktop/tests/integration/auto-updater.test.ts @@ -17,9 +17,9 @@ * - Dev-mode guard skips first-launch check but keeps handlers wired */ -import { describe, expect, mock, test } from 'bun:test'; import { EventEmitter } from 'node:events'; import type { OutgoingHttpHeaders } from 'node:http'; +import { describe, expect, test, vi } from 'vitest'; import { bootAutoUpdater, buildCheckNowResultFromError, @@ -63,7 +63,7 @@ class FakeUpdater extends EventEmitter implements UpdaterLike { allowDowngrade = true; forceDevUpdateConfig = false; requestHeaders: OutgoingHttpHeaders | null = null; - setFeedURL = mock( + setFeedURL = vi.fn( ( _urlOrOptions: | string @@ -71,9 +71,9 @@ class FakeUpdater extends EventEmitter implements UpdaterLike { | { provider: 'github'; owner: string; repo: string }, ) => {}, ); - checkForUpdates = mock(() => Promise.resolve(undefined)); - downloadUpdate = mock(() => Promise.resolve([] as unknown[])); - quitAndInstall = mock(() => {}); + checkForUpdates = vi.fn(() => Promise.resolve(undefined)); + downloadUpdate = vi.fn(() => Promise.resolve([] as unknown[])); + quitAndInstall = vi.fn(() => {}); override on(event: string, listener: (...args: unknown[]) => void): this { return super.on(event, listener as (...args: unknown[]) => void); } @@ -121,8 +121,8 @@ function makeFakeWindow(captured: CapturedSend[]): SendTarget { } interface FakeClock { - setTimeout: ReturnType; - clearTimeout: ReturnType; + setTimeout: ReturnType; + clearTimeout: ReturnType; /** Most recently registered timer callback — fire it to simulate a tick. */ lastCallback: (() => void) | null; /** Most recently returned timer handle. */ @@ -133,20 +133,20 @@ interface FakeClock { function makeFakeClock(): FakeClock { const clock: FakeClock = { - setTimeout: mock(() => Symbol('timer-handle')), - clearTimeout: mock(() => {}), + setTimeout: vi.fn(() => Symbol('timer-handle')), + clearTimeout: vi.fn(() => {}), lastCallback: null, lastHandle: null, lastMs: null, }; - clock.setTimeout = mock((cb: () => void, ms: number) => { + clock.setTimeout = vi.fn((cb: () => void, ms: number) => { clock.lastCallback = cb; clock.lastMs = ms; const handle = Symbol('timer-handle'); clock.lastHandle = handle; return handle as unknown as ReturnType; }); - clock.clearTimeout = mock((h: unknown) => { + clock.clearTimeout = vi.fn((h: unknown) => { if (h === clock.lastHandle) { clock.lastCallback = null; clock.lastHandle = null; @@ -171,10 +171,10 @@ interface TestRig { dispatches: DispatchKind[]; now: Date; logger: { - info: ReturnType; - warn: ReturnType; - error: ReturnType; - debug: ReturnType; + info: ReturnType; + warn: ReturnType; + error: ReturnType; + debug: ReturnType; }; } @@ -248,10 +248,10 @@ function makeRig( dispatches: [], now: new Date('2026-04-21T12:00:00.000Z'), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }; const primaryWindow = makeFakeWindow(primaryCaptured); @@ -460,7 +460,7 @@ describe('startAutoUpdater — initial configuration (parent §8.10 LOCKED)', () proxyFeed: { base: PROXY_BASE, channels: new Set(['beta']) }, updaterSetup: (u) => { let firstCall = true; - u.checkForUpdates = mock(() => { + u.checkForUpdates = vi.fn(() => { if (firstCall) { firstCall = false; return Promise.reject(new Error('proxy 503')); @@ -542,7 +542,7 @@ describe('startAutoUpdater — initial configuration (parent §8.10 LOCKED)', () proxyFeed: { base: PROXY_BASE, channels: new Set(['beta']) }, updaterSetup: (u) => { const original = u.setFeedURL; - u.setFeedURL = mock((arg) => { + u.setFeedURL = vi.fn((arg) => { if (typeof arg === 'object' && arg?.provider === 'github') { throw new Error('setFeedURL boom'); } @@ -653,7 +653,7 @@ describe('cross-channel veto on update-available', () => { }); test('menu-driven check: cross-channel offer remaps to not-available + does not download', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ appVersion: '0.5.0-beta.5', showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); rig.updater.emit('update-available', { version: '0.5.0' }); @@ -733,10 +733,10 @@ describe('persist-before-emit ordering (Finding #2)', () => { const state: AppState = emptyState(); const dispatches: DispatchKind[] = []; const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; startAutoUpdater({ updater, @@ -789,10 +789,10 @@ describe('persist-before-emit ordering (Finding #2)', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); @@ -1132,10 +1132,10 @@ describe('boot-time stale versionPendingInstall reconciliation', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(state.versionPendingInstall).toBe('0.4.0'); @@ -1201,10 +1201,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); return { captured, dispatches }; @@ -1352,10 +1352,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); return { captured, dispatches }; @@ -1435,10 +1435,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(dispatches).not.toContain('attempted-install-cross-channel' as DispatchKind); @@ -1468,10 +1468,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(dispatches).not.toContain('install-failed-giveup' as DispatchKind); @@ -1505,10 +1505,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(0); @@ -1541,10 +1541,10 @@ describe('boot-time failed-install detection', () => { now: () => new Date(), onDispatch: (k) => dispatches.push(k), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(state.attemptedInstall).toBe('0.16.0-beta.3'); @@ -1853,15 +1853,15 @@ describe('periodic check singleton + jitter (AC10, D10)', () => { const clock = makeFakeClock(); const captured: CapturedSend[] = []; let state: AppState = emptyState(); - updater.checkForUpdates = mock(() => + updater.checkForUpdates = vi.fn(() => Promise.reject(new Error('net::ERR_INTERNET_DISCONNECTED')), ); const primaryWindow = makeFakeWindow(captured); const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; startAutoUpdater({ updater, @@ -1938,7 +1938,7 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { // re-broadcast the downloaded banner (same notice id replaces the stuck // card in place), then rethrow for the clicked window's error notice. const { rig } = makeRig({ versionPendingInstall: '0.3.2', extraWindowCount: 2 }); - rig.updater.quitAndInstall = mock(() => { + rig.updater.quitAndInstall = vi.fn(() => { throw new Error('SQRLInstallerErrorDomain Code=-9'); }); await expect(Promise.resolve(rig.ipc.invoke('ok:update:relaunch-now'))).rejects.toThrow( @@ -1959,7 +1959,7 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { } expect(rig.dispatches).toContain('relaunch-failed-rearm' as DispatchKind); // The restored gate makes a retry click work end-to-end. - rig.updater.quitAndInstall = mock(() => {}); + rig.updater.quitAndInstall = vi.fn(() => {}); await rig.ipc.invoke('ok:update:relaunch-now'); expect(rig.updater.quitAndInstall).toHaveBeenCalledTimes(1); }); @@ -2002,10 +2002,10 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { clock: makeFakeClock(), now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); await ipc.invoke('ok:update:relaunch-now'); @@ -2021,7 +2021,7 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { test('prepareForRelaunch fires BEFORE quitAndInstall — utility kill ordering', async () => { const calls: string[] = []; const updater = new FakeUpdater(); - updater.quitAndInstall = mock(() => { + updater.quitAndInstall = vi.fn(() => { calls.push('quitAndInstall'); }); const ipc = makeFakeIpc(); @@ -2047,10 +2047,10 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { clock: makeFakeClock(), now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); await ipc.invoke('ok:update:relaunch-now'); @@ -2058,7 +2058,7 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { }); test('prepareForRelaunch does NOT fire when versionPendingInstall is null', () => { - const prepareForRelaunch = mock(() => {}); + const prepareForRelaunch = vi.fn(() => {}); const { rig } = makeRig({ versionPendingInstall: null, prepareForRelaunch }); rig.ipc.invoke('ok:update:relaunch-now'); expect(prepareForRelaunch).not.toHaveBeenCalled(); @@ -2066,7 +2066,7 @@ describe('ok:update:relaunch-now IPC handler (AC18)', () => { }); test('prepareForRelaunch throw does NOT block quitAndInstall', () => { - const prepareForRelaunch = mock(() => { + const prepareForRelaunch = vi.fn(() => { throw new Error('teardown bug'); }); const { rig } = makeRig({ versionPendingInstall: '0.3.2', prepareForRelaunch }); @@ -2232,10 +2232,10 @@ describe('async relaunch failure — error event + no-quit watchdog', () => { clock, now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); await Promise.resolve(); @@ -2312,7 +2312,7 @@ describe('ok:update:check-now IPC handler', () => { test('rejection from updater.checkForUpdates is swallowed in IPC path', () => { const { rig } = makeRig(); - rig.updater.checkForUpdates = mock(() => Promise.reject(new Error('network down'))); + rig.updater.checkForUpdates = vi.fn(() => Promise.reject(new Error('network down'))); expect(() => rig.ipc.invoke('ok:update:check-now')).not.toThrow(); }); @@ -2325,7 +2325,7 @@ describe('ok:update:check-now IPC handler', () => { describe('check-now → showCheckNowResult feedback dispatch', () => { test('update-not-available after menu-check fires not-available result', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ appVersion: '0.4.0-beta.13', showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); rig.updater.emit('update-not-available', { version: '0.4.0-beta.13' }); @@ -2337,7 +2337,7 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { }); test('update-available after menu-check fires available result with versions', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ appVersion: '0.4.0-beta.13', showCheckNowResult, @@ -2353,7 +2353,7 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { }); test('error after menu-check fires error result with the message', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); rig.updater.emit('error', new Error('network timeout')); @@ -2376,7 +2376,7 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { // so the final 404 names `latest-mac.yml` even on the beta channel. // Functionally: there is no installable update right now → surface the // friendly "up to date" dialog instead of a scary HTTP-404 dump. - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ appVersion: '0.5.0-beta.21', showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); const err = Object.assign( @@ -2398,7 +2398,7 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { // remapped. Other classified codes (HTTP_ERROR_500, ZIP_FILE_NOT_FOUND, // CHECKSUM_MISMATCH, …) still surface as error dialogs — they describe // real failures, not a transient empty-release state. - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); const err = Object.assign(new Error('zip missing'), { @@ -2412,14 +2412,14 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { }); test('periodic check (NO menu-check) does NOT fire showCheckNowResult', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ showCheckNowResult }); rig.updater.emit('update-not-available', { version: '0.4.0-beta.13' }); expect(showCheckNowResult).not.toHaveBeenCalled(); }); test('subsequent events after dispatch do NOT re-fire (single-shot per check-now)', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ showCheckNowResult }); rig.ipc.invoke('ok:update:check-now'); rig.updater.emit('update-not-available', { version: '0.4.0-beta.13' }); @@ -2429,9 +2429,9 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { }); test('checkForUpdates synchronous reject fires error result', async () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ showCheckNowResult }); - rig.updater.checkForUpdates = mock(() => Promise.reject(new Error('feed not reachable'))); + rig.updater.checkForUpdates = vi.fn(() => Promise.reject(new Error('feed not reachable'))); rig.ipc.invoke('ok:update:check-now'); // Wait for the .catch handler in runMenuDrivenCheck to settle. await new Promise((r) => setTimeout(r, 0)); @@ -2448,12 +2448,12 @@ describe('check-now → showCheckNowResult feedback dispatch', () => { // both paths aligned. Today this path is rare (the error normally lands // on the event bus), but the unit test pins the contract so a refactor // that drops the helper's special-case fails loud. - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig } = makeRig({ appVersion: '0.5.0-beta.21', showCheckNowResult }); const err = Object.assign(new Error('Cannot find latest-mac.yml ...: HttpError: 404'), { code: 'ERR_UPDATER_CHANNEL_FILE_NOT_FOUND', }); - rig.updater.checkForUpdates = mock(() => Promise.reject(err)); + rig.updater.checkForUpdates = vi.fn(() => Promise.reject(err)); rig.ipc.invoke('ok:update:check-now'); await new Promise((r) => setTimeout(r, 0)); expect(showCheckNowResult).toHaveBeenCalledWith({ @@ -2519,7 +2519,7 @@ describe('buildCheckNowResultFromError', () => { // invariant unique to the menu seam is "it routes through that same path". describe('handle.checkForUpdatesNow() routes the menu through runMenuDrivenCheck', () => { test('a menu click arms menuCheckPending so the result reaches showCheckNowResult', () => { - const showCheckNowResult = mock(() => {}); + const showCheckNowResult = vi.fn(() => {}); const { rig, handle } = makeRig({ appVersion: '0.4.0-beta.27', showCheckNowResult }); void handle.checkForUpdatesNow(); rig.updater.emit('update-not-available', { version: '0.4.0-beta.27' }); @@ -2564,10 +2564,10 @@ describe('dev-mode guard (isPackaged=false)', () => { clock, now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); await Promise.resolve(); @@ -2769,10 +2769,10 @@ describe('markCheckSucceeded routes through persistSafely (Critical #1)', () => const primaryWindow = makeFakeWindow(captured); const state: AppState = emptyState(); const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; startAutoUpdater({ updater, @@ -2818,10 +2818,10 @@ describe('markCheckSucceeded routes through persistSafely (Critical #1)', () => clock, now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); expect(() => updater.emit('update-not-available', { version: '0.3.1' })).not.toThrow(); @@ -2854,10 +2854,10 @@ describe('Toast B persist-before-emit + whenRendererReady (Major #1)', () => { clock, now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); // Persist failed → no Toast B. @@ -2963,10 +2963,10 @@ describe('relaunch-now idempotency (Major #2)', () => { clock, now: () => new Date(), logger: { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }, }); ipc.invoke('ok:update:relaunch-now'); @@ -2983,10 +2983,10 @@ describe('relaunch-now idempotency (Major #2)', () => { describe('bootAutoUpdater catch-path (Major #5)', () => { test('dynamic-import failure → returns null + logs error, no throw', async () => { const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; const captured: CapturedSend[] = []; const primaryWindow = makeFakeWindow(captured); @@ -3010,9 +3010,7 @@ describe('bootAutoUpdater catch-path (Major #5)', () => { expect(logger.error).toHaveBeenCalled(); // Error log includes the failure message for triage. const errorCall = logger.error.mock.calls[0]; - expect(errorCall?.[1]).toMatchObject({ - message: expect.stringContaining('Cannot find module'), - }); + expect((errorCall?.[1] as { err?: Error })?.err?.message).toContain('Cannot find module'); }); test('successful import → returns a real handle with destroy', async () => { @@ -3043,10 +3041,10 @@ describe('bootAutoUpdater catch-path (Major #5)', () => { test('startAutoUpdater synchronous throw during wire-up is caught', async () => { const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; // A fake updater whose `.on(...)` throws simulates an API-shape drift // inside startAutoUpdater's wire-up (future electron-updater major @@ -3134,10 +3132,10 @@ describe('bootAutoUpdater catch-path (Major #5)', () => { test('module exposes neither top-level nor .default.autoUpdater → logs + returns null', async () => { const logger = { - info: mock(() => {}), - warn: mock(() => {}), - error: mock(() => {}), - debug: mock(() => {}), + info: vi.fn(() => {}), + warn: vi.fn(() => {}), + error: vi.fn(() => {}), + debug: vi.fn(() => {}), }; const handle = await bootAutoUpdater( // A degenerate module that has neither shape — simulates a future @@ -3158,8 +3156,8 @@ describe('bootAutoUpdater catch-path (Major #5)', () => { expect(handle).toBeNull(); expect(logger.error).toHaveBeenCalled(); const errorCall = logger.error.mock.calls[0]; - expect(errorCall?.[1]).toMatchObject({ - message: expect.stringContaining('electron-updater did not expose'), - }); + expect((errorCall?.[1] as { err?: Error })?.err?.message).toContain( + 'electron-updater did not expose', + ); }); }); diff --git a/packages/desktop/tests/main/show-gate.test.ts b/packages/desktop/tests/main/show-gate.test.ts index eb065a302..a92efb876 100644 --- a/packages/desktop/tests/main/show-gate.test.ts +++ b/packages/desktop/tests/main/show-gate.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, mock, test } from 'bun:test'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import { type BrowserWindowLike, createShowGateRegistry, @@ -25,7 +25,7 @@ interface CapturedTimer { } interface MockWindow extends BrowserWindowLike { - show: ReturnType; + show: ReturnType; fireReadyToShow: () => void; markDestroyed: () => void; markVisible: () => void; @@ -35,28 +35,28 @@ function makeWindow(): MockWindow { let readyToShowCb: (() => void) | null = null; let destroyed = false; let visible = false; - const show = mock(() => { + const show = vi.fn(() => { visible = true; }); return { show, - isDestroyed: mock(() => destroyed), - isVisible: mock(() => visible), - on: mock(() => {}) as BrowserWindowLike['on'], - once: mock((event: 'ready-to-show', cb: () => void) => { + isDestroyed: vi.fn(() => destroyed), + isVisible: vi.fn(() => visible), + on: vi.fn(() => {}) as BrowserWindowLike['on'], + once: vi.fn((event: 'ready-to-show', cb: () => void) => { if (event === 'ready-to-show') readyToShowCb = cb; }) as BrowserWindowLike['once'], - focus: mock(() => {}), - isMinimized: mock(() => false), - restore: mock(() => {}), + focus: vi.fn(() => {}), + isMinimized: vi.fn(() => false), + restore: vi.fn(() => {}), webContents: { - send: mock(() => {}), - once: mock(() => {}), - setWindowOpenHandler: mock(() => {}), - on: mock(() => {}) as BrowserWindowLike['webContents']['on'], + send: vi.fn(() => {}), + once: vi.fn(() => {}), + setWindowOpenHandler: vi.fn(() => {}), + on: vi.fn(() => {}) as BrowserWindowLike['webContents']['on'], }, - loadFile: mock(() => Promise.resolve()), - loadURL: mock(() => Promise.resolve()), + loadFile: vi.fn(() => Promise.resolve()), + loadURL: vi.fn(() => Promise.resolve()), fireReadyToShow: () => readyToShowCb?.(), markDestroyed: () => { destroyed = true; @@ -116,7 +116,7 @@ describe('createShowGateRegistry — dual-signal show contract', () => { }); test('onShown fires once with the window kind after a successful show', () => { - const onShown = mock((_kind: 'editor' | 'navigator') => {}); + const onShown = vi.fn((_kind: 'editor' | 'navigator') => {}); const registry = createShowGateRegistry({ log: { warn: () => {} }, setTimeout: (cb, ms) => ({ cb, ms }), @@ -424,7 +424,7 @@ describe('createShowGateRegistry — show() throws past the destroyed-window gua function makeThrowingWindow(): MockWindow { const win = makeWindow(); - win.show = mock(() => { + win.show = vi.fn(() => { throw new Error('Object has been destroyed'); }); return win; @@ -442,8 +442,8 @@ describe('createShowGateRegistry — show() throws past the destroyed-window gua expect(failure?.obj).toMatchObject({ event: 'show-gate-show-failed', windowKind: 'editor', - error: 'Object has been destroyed', }); + expect((failure?.obj as { err?: Error }).err?.message).toBe('Object has been destroyed'); }); test('happy-path show throws → states Map entry is released (no leak)', () => { @@ -457,7 +457,7 @@ describe('createShowGateRegistry — show() throws past the destroyed-window gua win.fireReadyToShow(); env.registry.fireThemeApplied(win); // Re-firing must be a no-op — entry is gone, show is not invoked again. - win.show = mock(() => {}); + win.show = vi.fn(() => {}); env.registry.fireThemeApplied(win); expect(win.show).not.toHaveBeenCalled(); }); @@ -476,8 +476,8 @@ describe('createShowGateRegistry — show() throws past the destroyed-window gua expect(failure?.obj).toMatchObject({ event: 'show-gate-show-failed', windowKind: 'navigator', - error: 'Object has been destroyed', }); + expect((failure?.obj as { err?: Error }).err?.message).toBe('Object has been destroyed'); // The timeout warn (`show-gate-timeout`) still fires — the failure warn // is additive, not a replacement. const timeout = env.warns.find( diff --git a/packages/server/src/api-extension.ts b/packages/server/src/api-extension.ts index 4e9463ed8..59a3aa5de 100644 --- a/packages/server/src/api-extension.ts +++ b/packages/server/src/api-extension.ts @@ -532,7 +532,12 @@ import { } from './http/error-response.ts'; import { errnoCode, parseQuery } from './http/handler-utils.ts'; import { methodRouter } from './http/method-router.ts'; -import { REQUEST_ID_HEADER, rememberRequestId, resolveRequestId } from './http/request-id.ts'; +import { + getRequestId, + REQUEST_ID_HEADER, + rememberRequestId, + resolveRequestId, +} from './http/request-id.ts'; import { validateBody, withValidation } from './http/request-validation.ts'; import { successResponse } from './http/success-response.ts'; import { initContent } from './init-project.ts'; @@ -5302,7 +5307,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { ); return; } - log.error({ err: e }, '[agent-write] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[agent-write] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-write', cause: e, @@ -5593,7 +5598,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { ); return; } - log.error({ err: e }, '[agent-write-md] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[agent-write-md] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-write-md', cause: e, @@ -5726,7 +5731,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { 'Too many agent sessions.', ); } - log.error({ err: e, docName }, '[agent-write-batch] entry failed'); + log.error( + { err: e, docName, requestId: getRequestId(_req) }, + '[agent-write-batch] entry failed', + ); return entryError( docName, 'urn:ok:error:internal-server-error', @@ -5958,7 +5966,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { handler: 'agent-write-batch' }, ); } catch (e) { - log.error({ err: e }, '[agent-write-batch] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[agent-write-batch] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-write-batch', cause: e, @@ -6271,7 +6279,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { ); return; } - log.error({ err: e }, '[frontmatter-patch] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[frontmatter-patch] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'frontmatter-patch', cause: e, @@ -7760,7 +7768,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { ); return; } - log.error({ err: e }, '[agent-patch] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[agent-patch] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-patch', cause: e, @@ -7924,7 +7932,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { respondDocInConflict(res, e, 'agent-undo'); return; } - log.error({ err: e }, '[agent-undo] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[agent-undo] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-undo', cause: e, @@ -7963,7 +7971,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { handler: 'agent-activity', }); } catch (e) { - log.error({ err: e }, '[agent-activity] handler failed'); + log.error({ err: e, requestId: getRequestId(req) }, '[agent-activity] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-activity', cause: e, @@ -8089,7 +8097,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { handler: 'agent-burst-diff' }, ); } catch (e) { - log.error({ err: e }, '[agent-burst-diff] handler failed'); + log.error({ err: e, requestId: getRequestId(req) }, '[agent-burst-diff] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'agent-burst-diff', cause: e, @@ -8120,7 +8128,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { await flushGitCommit?.(); successResponse(res, 200, TestFlushGitSuccessSchema, {}, { handler: 'test-flush-git' }); } catch (e) { - log.error({ err: e }, '[test-flush-git] flush failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[test-flush-git] flush failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'test-flush-git', cause: e, @@ -8489,7 +8497,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { handler: 'save-version' }, ); } catch (e) { - log.error({ err: e }, '[save-version] handler failed'); + log.error({ err: e, requestId: getRequestId(_req) }, '[save-version] handler failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'save-version', cause: e, @@ -9068,7 +9076,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { handler: 'metrics-reconciliation', }); } catch (e) { - log.error({ err: e }, '[metrics-reconciliation] handler failed'); + log.error( + { err: e, requestId: getRequestId(_req) }, + '[metrics-reconciliation] handler failed', + ); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'metrics-reconciliation', cause: e, @@ -9086,7 +9097,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { handler: 'metrics-parse-health', }); } catch (e) { - log.error({ err: e }, '[metrics-parse-health] handler failed'); + log.error( + { err: e, requestId: getRequestId(_req) }, + '[metrics-parse-health] handler failed', + ); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'metrics-parse-health', cause: e, @@ -9395,7 +9409,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { handler: 'metrics-agent-presence' }, ); } catch (e) { - log.error({ err: e }, '[metrics-agent-presence] handler failed'); + log.error( + { err: e, requestId: getRequestId(req) }, + '[metrics-agent-presence] handler failed', + ); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'metrics-agent-presence', cause: e, @@ -9539,7 +9556,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { handler: 'metrics-watcher-recent' }, ); } catch (e) { - log.error({ err: e }, '[metrics-watcher-recent] handler failed'); + log.error( + { err: e, requestId: getRequestId(req) }, + '[metrics-watcher-recent] handler failed', + ); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'metrics-watcher-recent', cause: e, @@ -11895,6 +11915,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { { event: 'upload', endpoint: req.url ?? '/api/upload', + requestId: getRequestId(req), agentId, agentName, filename: finalFilename, @@ -12242,10 +12263,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { child.on('error', (err) => { spawnErrorMessage = err.message; earlyExitCode = -1; - log.error( - { cwd: absDir, cliCmd, err: err.message }, - '[local-op/clone] failed to spawn child', - ); + log.error({ cwd: absDir, cliCmd, err }, '[local-op/clone] failed to spawn child'); }); // `unref` so the child survives past the parent. Do it after attaching @@ -13932,7 +13950,10 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { // throwing before its internal try/catch). Guard `headersSent` so we // don't double-emit if the inner handler already wrote a response. if (!res.headersSent) { - log.error({ err: e }, '[installed-agents] route wrapper failed'); + log.error( + { err: e, requestId: getRequestId(req) }, + '[installed-agents] route wrapper failed', + ); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'installed-agents', cause: e, @@ -17825,7 +17846,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { }); } catch (e) { if (!res.headersSent) { - log.error({ err: e }, '[handoff] route wrapper failed'); + log.error({ err: e, requestId: getRequestId(req) }, '[handoff] route wrapper failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'handoff', cause: e, @@ -17855,7 +17876,7 @@ export function createApiExtension(options: ApiExtensionOptions): Extension { // synchronously) so the client still receives a typed contract // response instead of a hung connection. Mirrors `handleInstalledAgentsRoute`. if (!res.headersSent) { - log.error({ err: e }, '[spawn-cursor] route wrapper failed'); + log.error({ err: e, requestId: getRequestId(req) }, '[spawn-cursor] route wrapper failed'); errorResponse(res, 500, 'urn:ok:error:internal-server-error', 'Internal server error.', { handler: 'spawn-cursor', cause: e, diff --git a/packages/server/src/boot.ts b/packages/server/src/boot.ts index eca646134..7c40f8fe1 100644 --- a/packages/server/src/boot.ts +++ b/packages/server/src/boot.ts @@ -501,10 +501,7 @@ export async function bootServer(opts: BootServerOptions): Promise // no-env-var path does. Warn so that disconnect is diagnosable: the // server's `ok.boot` would otherwise become a detached root in Tempo with // nothing in the logs to correlate against the missing join. - getLogger('boot').warn( - { err: err instanceof Error ? err.message : String(err) }, - 'ok.boot trace-join failed — starting unparented boot', - ); + getLogger('boot').warn({ err }, 'ok.boot trace-join failed — starting unparented boot'); } } return bootSpan(); @@ -1123,7 +1120,7 @@ async function bootServerInner(opts: BootServerOptions): Promise { } } catch (err) { log.warn?.( - { event: 'installed-skills-reconcile-failed', error: String(err) }, + { event: 'installed-skills-reconcile-failed', err }, 'Installed-skills reconcile failed (non-fatal).', ); } diff --git a/packages/server/src/config-file-watcher.ts b/packages/server/src/config-file-watcher.ts index 74d9d7413..1477b93fd 100644 --- a/packages/server/src/config-file-watcher.ts +++ b/packages/server/src/config-file-watcher.ts @@ -29,6 +29,7 @@ import { readFileSync } from 'node:fs'; import { dirname } from 'node:path'; import { tracedMkdirSync } from './fs-traced.ts'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; /** Cleanup function returned by `startConfigFileWatcher`. Idempotent. */ @@ -73,7 +74,7 @@ export async function startConfigFileWatcher( try { tracedMkdirSync(watchDir, { recursive: true }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'EEXIST') { log.warn({ err, watchDir }, 'failed to create watch directory; watcher may be inert'); } @@ -122,7 +123,7 @@ export async function startConfigFileWatcher( try { content = readFileSync(path, 'utf-8'); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code === 'ENOENT') { if (logMissing) log.debug({ path }, 'config file disappeared between event and read; dropping'); @@ -211,7 +212,7 @@ export async function startMultiPathConfigFileWatcher( try { tracedMkdirSync(dir, { recursive: true }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'EEXIST') { log.warn({ err, dir }, 'failed to create watch directory; watcher may be inert'); } @@ -246,7 +247,7 @@ export async function startMultiPathConfigFileWatcher( try { content = readFileSync(path, 'utf-8'); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code === 'ENOENT') { if (logMissing) log.debug({ path }, 'config file disappeared between event and read; dropping'); diff --git a/packages/server/src/content/templates-resolver.ts b/packages/server/src/content/templates-resolver.ts index a272627b0..de2d48d77 100644 --- a/packages/server/src/content/templates-resolver.ts +++ b/packages/server/src/content/templates-resolver.ts @@ -23,6 +23,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; import { join, posix } from 'node:path'; import { parseTemplateFile } from '@inkeep/open-knowledge-core'; +import { errnoCode } from '../http/handler-utils.ts'; import { getLogger } from '../logger.ts'; type TemplateScope = 'local' | 'inherited'; @@ -159,7 +160,7 @@ export function resolveProjectTemplates(projectDir: string): ProjectTemplatesRes // a folder existed when we queued it but was removed before we // walked into it (file watcher race). Mirrors the `readTemplateMeta` // pattern below, sharing its `templateMetaWarnedPaths` dedupe set. - const code = (err as NodeJS.ErrnoException | undefined)?.code; + const code = errnoCode(err); if (code !== 'ENOENT' && !templateMetaWarnedPaths.has(absDir)) { templateMetaWarnedPaths.add(absDir); const reason = err instanceof Error ? err.message : String(err); @@ -281,7 +282,7 @@ function readTemplateMeta(absPath: string): TemplateMeta { try { content = readFileSync(absPath, 'utf-8'); } catch (err) { - const code = (err as NodeJS.ErrnoException | undefined)?.code; + const code = errnoCode(err); if (code !== 'ENOENT' && !templateMetaWarnedPaths.has(absPath)) { templateMetaWarnedPaths.add(absPath); const reason = err instanceof Error ? err.message : String(err); diff --git a/packages/server/src/embeddings/semantic-search-service.ts b/packages/server/src/embeddings/semantic-search-service.ts index f9a300052..6ef3f3bb2 100644 --- a/packages/server/src/embeddings/semantic-search-service.ts +++ b/packages/server/src/embeddings/semantic-search-service.ts @@ -182,10 +182,7 @@ export class SemanticSearchService { } catch (err) { this.capable = false; this.ready = true; - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] warm failed', - ); + log.warn({ err }, '[embeddings] warm failed'); } } @@ -204,10 +201,7 @@ export class SemanticSearchService { try { await this.runEmbedPass(next); } catch (err) { - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] embed pass failed', - ); + log.warn({ err }, '[embeddings] embed pass failed'); } }); return this.embedChain; @@ -338,10 +332,7 @@ export class SemanticSearchService { [queryVec] = await this.embedder.embed([trimmed], { role: 'query' }); } catch (err) { // Provider error / timeout on the query path is non-fatal: degrade to BM25. - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] query embed failed — degrading to lexical', - ); + log.warn({ err }, '[embeddings] query embed failed — degrading to lexical'); return null; } if (!queryVec) return null; diff --git a/packages/server/src/embeddings/vector-cache.ts b/packages/server/src/embeddings/vector-cache.ts index 5343dea2c..59150a611 100644 --- a/packages/server/src/embeddings/vector-cache.ts +++ b/packages/server/src/embeddings/vector-cache.ts @@ -150,10 +150,7 @@ export class VectorCache { manifest = JSON.parse(await readFile(this.manifestPath, 'utf-8')) as ManifestFile; } } catch (err) { - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] unreadable cache manifest — rebuilding', - ); + log.warn({ err }, '[embeddings] unreadable cache manifest — rebuilding'); manifest = null; } @@ -191,7 +188,7 @@ export class VectorCache { } catch (err) { // Corrupt blob → drop the in-memory vectors so the doc re-embeds. log.warn( - { hash: entry.contentHash, err: err instanceof Error ? err.message : String(err) }, + { hash: entry.contentHash, err }, '[embeddings] corrupt vector blob — will re-embed', ); } @@ -312,10 +309,7 @@ export class VectorCache { } catch (err) { // Persistence is best-effort: an unwritable cache degrades to recompute // next boot, it must never fail a search or an embed pass. - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] failed to persist vector cache', - ); + log.warn({ err }, '[embeddings] failed to persist vector cache'); } } @@ -324,10 +318,7 @@ export class VectorCache { try { tracedRmSync(this.cacheDir, { recursive: true, force: true }); } catch (err) { - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[embeddings] failed to wipe stale cache', - ); + log.warn({ err }, '[embeddings] failed to wipe stale cache'); } } } diff --git a/packages/server/src/error-log-discipline.test.ts b/packages/server/src/error-log-discipline.test.ts new file mode 100644 index 000000000..9a73b44d8 --- /dev/null +++ b/packages/server/src/error-log-discipline.test.ts @@ -0,0 +1,300 @@ +/** + * Source-scan STOP rule for error-log payload discipline: error/warn logger + * calls pass the RAW error value under the `err` key — never a + * string-coerced copy (`err.message`, `String(err)`, `(err as + * Error).message`, `x instanceof Error ? x.message : String(x)`). The pino + * serializers on both the server logger (`logger.ts`) and the desktop root + * logger (`desktop-logger.ts`) capture name/message/stack from a raw Error; + * a pre-coerced string discards the stack, which is the difference between + * a correlatable JSONL bundle line and a dead-end one. + * + * Scope mirrors the Node-side logging surface: `packages/server/src`, + * `packages/cli/src`, and `packages/desktop/src/main` (renderer is not + * pino-backed). `console.*` receivers are exempt — the two sanctioned + * console.warn styles (bracket-prefix; structured JSON) are their own + * convention and stay out of this rule's reach. + * + * Escape hatch: suffix the offending line (or the call line) with + * `// error-log-shape-ok: ` for a site where a string copy is the + * point (e.g. capturing a message snapshot alongside the raw err), or add + * a FILE_ALLOWLIST entry with a structural reason for a whole surface. + * + * The predicate is line-window based, so it has planted-positive + + * adjacent-negative self-tests below (an absence-checker without a planted + * positive is a vacuous no-op — same discipline as + * console-discipline.test.ts). + */ + +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import { basename, dirname, join, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, test } from 'vitest'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +/** Scan roots, relative paths reported against the OK workspace root. */ +const WORKSPACE_ROOT = resolve(__dirname, '../../..'); +const SCAN_ROOTS = [ + resolve(__dirname), // packages/server/src + resolve(__dirname, '../../cli/src'), + resolve(__dirname, '../../desktop/src/main'), +]; + +/** This file embeds the banned patterns as predicate fixtures. */ +const SELF_BASENAME = basename(fileURLToPath(import.meta.url)); + +/** + * Whole-file exemptions, keyed by path relative to the OK workspace root. + * Every entry needs a reason explaining why the raw-`err` shape cannot + * serve the site. + */ +const FILE_ALLOWLIST: ReadonlyMap = new Map([]); + +const MARKER = 'error-log-shape-ok:'; + +interface FileLines { + /** Path relative to the OK workspace root for failure messages. */ + path: string; + lines: string[]; +} + +function listScannedSourceFiles(): FileLines[] { + const out: FileLines[] = []; + function walk(dir: string) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const abs = join(dir, entry.name); + if (entry.isDirectory()) { + walk(abs); + continue; + } + if (!entry.isFile() || !entry.name.endsWith('.ts')) continue; + if (entry.name.endsWith('.test.ts') || entry.name.endsWith('.test-helper.ts')) continue; + if (entry.name === SELF_BASENAME) continue; + out.push({ + path: relative(WORKSPACE_ROOT, abs), + lines: readFileSync(abs, 'utf-8').split('\n'), + }); + } + } + for (const root of SCAN_ROOTS) walk(root); + return out; +} + +function isCommentOnlyLine(line: string): boolean { + const trimmed = line.trim(); + return trimmed.startsWith('//') || trimmed.startsWith('*') || trimmed.startsWith('/*'); +} + +/** + * An `.error(` / `.warn(` call on anything that is not `console`. The + * receiver is intentionally loose (named loggers, injected logger deps, + * `getLogger(...)` chains) — the banned-shape check below is what keeps + * false positives out. + */ +const LOG_CALL = /(?` and + * non-error-ish reads (`message: i.message` over Zod issues) stay legal. + */ +const BANNED_FIELD = [ + /\b(?:err|error|cause)\s*:\s*String\(/, + /\b(?:err|error|cause)\s*:\s*\(?\s*\w+(?:\s+as\s+\w+)?\s*\)?\s*\.message\b/, + /\b(?:err|error|cause|message)\s*:\s*\(?\s*\w+\s+instanceof\s+Error\s*\?\s*\w+\.message\s*:/, + /\bmessage\s*:\s*\(?\s*(?:e|err|error|\w*[eE]rr(?:or)?)(?:\s+as\s+\w+)?\s*\)?\s*\.message\b/, +]; + +/** Lines the window may span — data objects in this codebase stay short. */ +const WINDOW_LINES = 8; + +export interface ErrorLogViolation { + line: number; + text: string; +} + +/** + * Find error/warn logger calls whose argument window contains a + * string-coerced error field. The window spans from the call opener until + * its paren balance closes (so trailing statements after the call are never + * misattributed), capped at WINDOW_LINES — a banned shape further down a + * very long argument list is missed, so keep log data objects compact. + * Parens inside string literals count toward the balance; that can only + * END a window early (fail-open), never extend it. + */ +export function findStringifiedErrorFields(lines: string[]): ErrorLogViolation[] { + const violations: ErrorLogViolation[] = []; + const flagged = new Set(); + for (let i = 0; i < lines.length; i++) { + const line = lines[i] ?? ''; + if (isCommentOnlyLine(line)) continue; + const m = LOG_CALL.exec(line); + if (!m) continue; + if (line.includes(MARKER)) continue; + let depth = 0; + for (let w = 0; w < WINDOW_LINES && i + w < lines.length; w++) { + const wLine = lines[i + w] ?? ''; + // Only inspect the segment from the call opener onward on the match + // line so content BEFORE the call is never misattributed. + const segment = w === 0 ? wLine.slice(wLine.indexOf(m[0]) + m[0].length - 1) : wLine; + if (!isCommentOnlyLine(wLine) && !wLine.includes(MARKER)) { + const testable = w === 0 ? segment.slice(1) : segment; + if (BANNED_FIELD.some((re) => re.test(testable))) { + if (!flagged.has(i + w)) { + flagged.add(i + w); + violations.push({ line: i + 1 + w, text: wLine.trim() }); + } + break; + } + } + for (const ch of segment) { + if (ch === '(') depth++; + else if (ch === ')') depth--; + } + if (depth <= 0) break; + } + } + return violations; +} + +describe('error-log payload discipline (server + cli + desktop main)', () => { + test('every scan root exists (layout sanity)', () => { + for (const root of SCAN_ROOTS) { + expect(existsSync(root)).toBe(true); + } + }); + + const files = listScannedSourceFiles(); + + test('there are source files to scan (sanity)', () => { + expect(files.length).toBeGreaterThan(0); + expect(files.some((f) => f.path === join('packages', 'server', 'src', 'file-watcher.ts'))).toBe( + true, + ); + expect( + files.some((f) => f.path === join('packages', 'desktop', 'src', 'main', 'auto-updater.ts')), + ).toBe(true); + }); + + test('every FILE_ALLOWLIST entry still exists on disk', () => { + const paths = new Set(files.map((f) => f.path)); + for (const allowed of FILE_ALLOWLIST.keys()) { + expect(paths.has(allowed)).toBe(true); + } + }); + + test('error/warn logger calls pass the raw error under err, not a string copy', () => { + const violations: string[] = []; + for (const file of files) { + if (FILE_ALLOWLIST.has(file.path)) continue; + for (const v of findStringifiedErrorFields(file.lines)) { + violations.push(` ${file.path}:${v.line} ${v.text}`); + } + } + if (violations.length > 0) { + throw new Error( + `String-coerced error field found in an error/warn log call. Pass the RAW error under ` + + `the \`err\` key (\`log.warn({ err }, '...')\`) — the pino serializers capture ` + + `name/message/stack; \`err.message\` / \`String(err)\` discard the stack the JSONL ` + + `bundle needs. For a site where a string copy is genuinely intended, suffix the line ` + + `with \`// ${MARKER} \` or add a FILE_ALLOWLIST entry in ` + + `error-log-discipline.test.ts:\n${violations.join('\n')}`, + ); + } + }); + + test('predicate fires on planted violations and not on adjacent negatives', () => { + // Planted positives: the raw shapes this rule bans. + expect(findStringifiedErrorFields([" log.warn({ err: String(err) }, 'x');"]).length).toBe(1); + expect( + findStringifiedErrorFields([' logger.warn(', " { event: 'x', error: String(err) },"]) + .length, + ).toBe(1); + expect(findStringifiedErrorFields([" log.error({ err: err.message }, 'x');"]).length).toBe(1); + expect( + findStringifiedErrorFields([" log.warn({ err: (e as Error).message }, 'x');"]).length, + ).toBe(1); + expect( + findStringifiedErrorFields([ + ' logger.error({', + ' err: err instanceof Error ? err.message : String(err),', + ' });', + ]).length, + ).toBe(1); + expect( + findStringifiedErrorFields([ + ' deps.logger.warn(', + ' {', + " event: 'x',", + ' cause: err instanceof Error ? err.message : String(err),', + ' },', + ]).length, + ).toBe(1); + expect( + findStringifiedErrorFields([" logger.error('failed', { message: err.message });"]).length, + ).toBe(1); + + // Adjacent negatives: the sanctioned raw-err shapes. + expect(findStringifiedErrorFields([" log.warn({ err }, 'x');"]).length).toBe(0); + expect(findStringifiedErrorFields([" log.error({ err: e, docName }, 'x');"]).length).toBe(0); + expect( + findStringifiedErrorFields([ + " log.warn({ err: err instanceof Error ? err : new Error(String(err)) }, 'x');", + ]).length, + ).toBe(0); + // console receivers are the sanctioned console-style carve-out. + expect( + findStringifiedErrorFields([" console.warn('[main] x', { err: (err as Error).message });"]) + .length, + ).toBe(0); + // Non-error-ish `.message` reads (Zod issues, typed results) stay legal. + expect( + findStringifiedErrorFields([ + ' logger.warn({ issues: x.map((i) => ({ path: i.path, message: i.message })) });', + ]).length, + ).toBe(0); + expect( + findStringifiedErrorFields([" log.warn({ message: result.message }, 'x');"]).length, + ).toBe(0); + // Comment-only lines are exempt. + expect( + findStringifiedErrorFields([' // like log.warn({ err: String(err) }) used to']).length, + ).toBe(0); + // The inline escape hatch suppresses the flagged line. + expect( + findStringifiedErrorFields([ + " log.warn({ err: String(err) }, 'x'); // error-log-shape-ok: message snapshot on purpose", + ]).length, + ).toBe(0); + + // A banned shape AFTER the call's parens close belongs to the next + // statement, not the log call — never misattributed. + expect( + findStringifiedErrorFields([ + " log.warn({ err }, 'x');", + ' phaseErrors.push({ phase: "y", error: String(err) });', + ]).length, + ).toBe(0); + + // Known limitation, pinned: a banned shape further than WINDOW_LINES - 1 + // lines below the call opener is not seen — keep log data objects compact. + expect( + findStringifiedErrorFields([ + ' log.warn(', + ' {', + ' a: 1,', + ' b: 2,', + ' c: 3,', + ' d: 4,', + ' e: 5,', + ' f: 6,', + ' err: String(err),', + ' },', + ]).length, + ).toBe(0); + }); +}); diff --git a/packages/server/src/file-watcher.ts b/packages/server/src/file-watcher.ts index 640d9dcd1..55c6f9201 100644 --- a/packages/server/src/file-watcher.ts +++ b/packages/server/src/file-watcher.ts @@ -28,6 +28,7 @@ import { stripDocExtension, } from './doc-extensions.ts'; import { classifyFsPath, normalizeFsPath } from './fs-traced.ts'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; import { extractPageIcon, extractPageTitle } from './page-identity.ts'; import { toPosix } from './path-utils.ts'; @@ -463,7 +464,7 @@ function eventEscapesContentDir(rawPath: string, contentDir: string): boolean { try { lst = lstatSync(rawPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code === 'ENOENT') return false; // deleted between event and check log.warn( { path: rawPath, code }, @@ -476,7 +477,7 @@ function eventEscapesContentDir(rawPath: string, contentDir: string): boolean { try { canonical = realpathSync(rawPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT' && code !== 'ELOOP') { log.warn( { path: rawPath, code }, @@ -635,7 +636,7 @@ export async function classifyEvents( // read (delete race) — the event is silently dropped downstream, so the // ring is the only record it ever existed. recordWatcherDecision('drop-read-failed', event.type, event.path); - if ((e as NodeJS.ErrnoException).code !== 'ENOENT') { + if (errnoCode(e) !== 'ENOENT') { log.warn({ path: event.path, err: e }, `Failed to read ${event.path}`); } } @@ -645,7 +646,7 @@ export async function classifyEvents( updateContents.set(event.path, await readFile(event.path, 'utf-8')); } catch (e) { recordWatcherDecision('drop-read-failed', event.type, event.path); - if ((e as NodeJS.ErrnoException).code !== 'ENOENT') { + if (errnoCode(e) !== 'ENOENT') { log.warn({ path: event.path, err: e }, `Failed to read ${event.path}`); } } @@ -661,7 +662,7 @@ export async function classifyEvents( try { lst = lstatSync(rawPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT') { log.warn({ path: rawPath, err: e }, `resolveDocName lstat failed for ${rawPath}`); } @@ -683,7 +684,7 @@ export async function classifyEvents( try { canonical = realpathSync(rawPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT' && code !== 'ELOOP') { log.warn({ path: rawPath, err: e }, `resolveDocName realpath failed for ${rawPath}`); } @@ -845,7 +846,7 @@ async function seedLastKnownHashes( try { lst = await lstat(fullPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT') { log.warn({ path: fullPath, err: e }, `Failed to lstat ${fullPath}, skipping`); } @@ -857,7 +858,7 @@ async function seedLastKnownHashes( try { canonical = await realpath(fullPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code === 'ENOENT' || code === 'ELOOP') { log.warn({ path: fullPath, code }, `Broken/cyclic symlink at ${fullPath}, skipping`); } else { @@ -959,7 +960,7 @@ async function seedLastKnownHashes( ...derivePageMeta(content, canonicalDocName), }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn({ path: canonical, err }, `Failed to seed hash for ${canonical}`); } @@ -1045,7 +1046,7 @@ async function seedLastKnownHashes( ...derivePageMeta(content, docName), }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code === 'EACCES') { log.warn( { path: fullPath, code }, @@ -1081,7 +1082,7 @@ async function seedLastKnownHashes( } } } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn({ dir, err }, `Failed to read directory ${dir}`); } @@ -1225,7 +1226,7 @@ function updateFolderIndexFromRawEvents( try { lst = lstatSync(raw.path); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn({ path: raw.path, code }, `folder lstat failed for ${raw.path} (${code})`); } @@ -1243,7 +1244,7 @@ function updateFolderIndexFromRawEvents( const stat = statSync(canonicalPath); if (stat.isDirectory()) folderStat = stat; } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn( { path: raw.path, code }, @@ -1303,7 +1304,7 @@ function scanForUntrackedSubfolders( try { entries = readdirSync(dir, { withFileTypes: true }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn({ dir, code }, `folder rescan readdir failed for ${dir} (${code})`); } @@ -1334,7 +1335,7 @@ function scanForUntrackedSubfolders( try { stat = lstatSync(fullPath); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn( { path: fullPath, code }, @@ -1479,7 +1480,7 @@ export async function handleRawEvents( try { checkPath = realpathSync(event.path); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT') { log.warn( { path: event.path, code }, @@ -1494,7 +1495,7 @@ export async function handleRawEvents( try { checkPath = realpathSync(event.newPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code !== 'ENOENT') { log.warn( { path: event.newPath, code }, @@ -1659,7 +1660,7 @@ export async function handleRawEvents( st = lstatSync(raw.path); if (st.isSymbolicLink()) st = statSync(raw.path); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); recordWatcherDecision('drop-stat-failed', raw.type, raw.path); if (code !== 'ENOENT') { log.warn({ path: raw.path, code }, `file-event lstat failed for ${raw.path} (${code})`); diff --git a/packages/server/src/http/error-response.ts b/packages/server/src/http/error-response.ts index b3e11020e..9ceb6a9cd 100644 --- a/packages/server/src/http/error-response.ts +++ b/packages/server/src/http/error-response.ts @@ -48,6 +48,7 @@ import { import type { Counter } from '@opentelemetry/api'; import { getLogger } from '../logger.ts'; import { getMeter, setActiveSpanAttributes } from '../telemetry.ts'; +import { getRequestId } from './request-id.ts'; // Lazy logger accessor — `loggerFactory.configure()` (used by capture-logger // tests in `server-factory.test.ts` and `logger.test.ts`) clears the @@ -184,6 +185,7 @@ export function errorResponse( { event: 'api.error.double-write', instance, + requestId: getRequestId(res.req), type, status, handler: options.handler, @@ -228,6 +230,7 @@ export function errorResponse( log().error( { event: 'api.error.malformed-envelope', + requestId: getRequestId(res.req), issues: validated.error.issues, body, handler: options.handler, @@ -301,6 +304,7 @@ export function errorResponse( { event: 'api.error', instance, + requestId: getRequestId(res.req), type, status, handler: options.handler, @@ -330,6 +334,7 @@ export function errorResponse( log().error( { event: 'api.error.unserializable-body', + requestId: getRequestId(res.req), bodyKeys: Object.keys(wireBody), handler: options.handler, originalStatus: status, @@ -520,6 +525,7 @@ export function createStreamingErrorWriter( log().error( { event: 'api.streaming.error.suppressed', + requestId: getRequestId(res.req), type, status, handler, @@ -545,6 +551,7 @@ export function createStreamingErrorWriter( log().error( { event: 'api.streaming.error.write-failed', + requestId: getRequestId(res.req), type, status, handler, diff --git a/packages/server/src/link-preview/preview-cache.ts b/packages/server/src/link-preview/preview-cache.ts index 011445be0..f48353dc1 100644 --- a/packages/server/src/link-preview/preview-cache.ts +++ b/packages/server/src/link-preview/preview-cache.ts @@ -160,10 +160,7 @@ export class LinkPreviewCache { manifest = JSON.parse(await readFile(this.manifestPath, 'utf-8')) as ManifestFile; } } catch (err) { - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[link-preview] unreadable cache manifest — starting empty', - ); + log.warn({ err }, '[link-preview] unreadable cache manifest — starting empty'); return; } if (!manifest || manifest.schemaVersion !== MANIFEST_SCHEMA_VERSION || !manifest.entries) { @@ -338,10 +335,7 @@ export class LinkPreviewCache { } catch (err) { // Persistence is best-effort: an unwritable cache degrades to a recompute // next boot, it must never fail a preview request. - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[link-preview] failed to persist cache', - ); + log.warn({ err }, '[link-preview] failed to persist cache'); } } @@ -361,10 +355,7 @@ export class LinkPreviewCache { try { tracedRmSync(this.cacheDir, { recursive: true, force: true }); } catch (err) { - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[link-preview] failed to wipe cache', - ); + log.warn({ err }, '[link-preview] failed to wipe cache'); } } @@ -390,10 +381,7 @@ export class LinkPreviewCache { } catch (err) { // Mirror the manifest-level warn: a corrupt/unreadable blob is a miss, not // a throw. Reason only — never the URL/host/content (topology-leak hygiene). - log.warn( - { err: err instanceof Error ? err.message : String(err) }, - '[link-preview] unreadable cache blob — treating as miss', - ); + log.warn({ err }, '[link-preview] unreadable cache blob — treating as miss'); return null; } } diff --git a/packages/server/src/local-op-security.ts b/packages/server/src/local-op-security.ts index ad9521586..13c6c2490 100644 --- a/packages/server/src/local-op-security.ts +++ b/packages/server/src/local-op-security.ts @@ -16,6 +16,7 @@ import type { IncomingMessage, ServerResponse } from 'node:http'; import { homedir } from 'node:os'; import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { errorResponse } from './http/error-response.ts'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; const log = getLogger('local-op-security'); @@ -75,7 +76,7 @@ function ancestorChainHasSymlink(start: string, root: string): boolean { try { stats = lstatSync(cursor); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); log.warn( { path: cursor, code: code ?? 'unknown' }, `ancestorChainHasSymlink: lstat failed on ${cursor} (${code ?? 'unknown'}); treating as symlink (fail-closed)`, @@ -103,7 +104,7 @@ export function isPathWithinHome(dirPath: string, home: string): boolean { try { realHome = realpathSync(home); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); log.warn( { path: home, code: code ?? 'unknown' }, `realpath failed on home dir ${home} (${code ?? 'unknown'}); rejecting all paths`, @@ -120,7 +121,7 @@ export function isPathWithinHome(dirPath: string, home: string): boolean { try { stats = lstatSync(current); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'ENOENT') { log.warn( { path: current, code: code ?? 'unknown' }, @@ -145,7 +146,7 @@ export function isPathWithinHome(dirPath: string, home: string): boolean { try { resolvedCurrent = realpathSync(current); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (stats.isSymbolicLink()) { log.warn( { path: current, code: code ?? 'unknown' }, diff --git a/packages/server/src/managed-artifact-persistence.ts b/packages/server/src/managed-artifact-persistence.ts index af45ffee8..4c8aa9f88 100644 --- a/packages/server/src/managed-artifact-persistence.ts +++ b/packages/server/src/managed-artifact-persistence.ts @@ -308,7 +308,7 @@ export function loadManagedArtifactDoc( try { raw = readFileSync(filePath, 'utf-8'); } catch (e) { - log.warn({ documentName, err: (e as Error).message }, 'load: could not read; seeding empty'); + log.warn({ documentName, err: e }, 'load: could not read; seeding empty'); return; } @@ -380,7 +380,7 @@ export async function storeManagedArtifactDoc( // returns 'write-failed' with no hint a READ preceded it. Log it. if ((readErr as NodeJS.ErrnoException).code !== 'ENOENT') { log.warn( - { documentName, err: (readErr as Error).message }, + { documentName, err: readErr }, 'store: pre-write disk read failed (non-ENOENT); proceeding to write', ); } @@ -405,7 +405,7 @@ export async function storeManagedArtifactDoc( log.warn({ documentName }, 'store: file lock timeout; skipping write'); return 'write-failed'; } - log.warn({ documentName, err: (e as Error).message }, 'store: write failed'); + log.warn({ documentName, err: e }, 'store: write failed'); return 'write-failed'; } } diff --git a/packages/server/src/managed-artifact-watcher.ts b/packages/server/src/managed-artifact-watcher.ts index fec3dbdc1..af132454f 100644 --- a/packages/server/src/managed-artifact-watcher.ts +++ b/packages/server/src/managed-artifact-watcher.ts @@ -27,6 +27,7 @@ import { readFileSync } from 'node:fs'; import { basename } from 'node:path'; import { tracedMkdirSync } from './fs-traced.ts'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; /** Cleanup function returned by `startManagedArtifactWatcher`. Idempotent. */ @@ -84,7 +85,7 @@ export async function startManagedArtifactWatcher( try { tracedMkdirSync(dir, { recursive: true }); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code !== 'EEXIST') { log.warn({ err, dir }, 'failed to create watch root; watcher may be inert'); } @@ -113,7 +114,7 @@ export async function startManagedArtifactWatcher( try { content = readFileSync(path, 'utf-8'); } catch (err) { - const code = (err as NodeJS.ErrnoException).code; + const code = errnoCode(err); if (code === 'ENOENT') { log.debug({ path }, 'managed-artifact leaf disappeared between event and read; dropping'); return; diff --git a/packages/server/src/mcp/logger.ts b/packages/server/src/mcp/logger.ts index b67956515..669f65723 100644 --- a/packages/server/src/mcp/logger.ts +++ b/packages/server/src/mcp/logger.ts @@ -28,6 +28,18 @@ interface McpLogEntry { const loggerContext = new AsyncLocalStorage(); +/** + * Raw Errors stringify to `{}` under JSON.stringify — flatten top-level Error + * values to a plain `{name, message, stack}` record so the JSONL line keeps + * the stack. Nested structures stay caller-shaped. + */ +function toSerializableError(value: unknown): unknown { + if (value instanceof Error) { + return { name: value.name, message: value.message, stack: value.stack }; + } + return value; +} + export class McpLogger { readonly sessionId: string; private corrId: string; @@ -53,7 +65,7 @@ export class McpLogger { } error(msg: string, err?: unknown, ctx: Record = {}): void { - const errCtx = err ? { error: err instanceof Error ? err.message : String(err), ...ctx } : ctx; + const errCtx = err !== undefined ? { error: err, ...ctx } : ctx; this.emit('error', msg, errCtx); } @@ -84,6 +96,10 @@ export class McpLogger { msg: string, ctx: Record, ): void { + const serializedCtx: Record = {}; + for (const [key, value] of Object.entries(ctx)) { + serializedCtx[key] = toSerializableError(value); + } const entry: McpLogEntry = { ts: new Date().toISOString(), level, @@ -91,7 +107,7 @@ export class McpLogger { corrId: this.corrId, component: this.component, msg, - ...ctx, + ...serializedCtx, }; const line = `${JSON.stringify(entry)}\n`; process.stderr.write(line); diff --git a/packages/server/src/mcp/tools/index.ts b/packages/server/src/mcp/tools/index.ts index a057a8cb2..9640bbd4e 100644 --- a/packages/server/src/mcp/tools/index.ts +++ b/packages/server/src/mcp/tools/index.ts @@ -111,7 +111,7 @@ export function registerAllTools(server: ServerInstance, opts: RegisterAllToolsO const activeLog = getCurrentMcpLogger() ?? log; activeLog?.warn('tool call failed', { tool, - error: err instanceof Error ? err.message : String(err), + err, ...(explicit ? { explicit } : {}), }); throw err; diff --git a/packages/server/src/persistence.ts b/packages/server/src/persistence.ts index 6e6775d93..e2eca027b 100644 --- a/packages/server/src/persistence.ts +++ b/packages/server/src/persistence.ts @@ -67,6 +67,7 @@ import { docNameToRelativePath } from './doc-extensions.ts'; import { applyDiskContentToDoc, FILE_WATCHER_ORIGIN } from './external-change.ts'; import { contentHash, registerWrite } from './file-watcher.ts'; import { tracedMkdir, tracedRename, tracedUnlinkSync, tracedWriteFile } from './fs-traced.ts'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; import { loadManagedArtifactDoc, @@ -901,7 +902,7 @@ export function createPersistenceExtension(options?: PersistenceOptions): Persis ); if (consecutiveGitFailures >= 3) { log.error( - { attempt: consecutiveGitFailures }, + { err: e, attempt: consecutiveGitFailures }, '[persistence] CRITICAL: Git auto-save has failed 3+ times. Version history is NOT being recorded.', ); } @@ -1085,7 +1086,7 @@ export function createPersistenceExtension(options?: PersistenceOptions): Persis ); if (consecutiveGitFailures >= 3) { log.error( - { attempt: consecutiveGitFailures }, + { err: e, attempt: consecutiveGitFailures }, '[persistence] CRITICAL: Git auto-save has failed 3+ times. Version history is NOT being recorded.', ); } @@ -1737,13 +1738,13 @@ export function createPersistenceExtension(options?: PersistenceOptions): Persis try { canonicalPath = await realpath(requestedPath); } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code === 'ENOENT') { let isBrokenSymlink = false; try { isBrokenSymlink = lstatSync(requestedPath).isSymbolicLink(); } catch (lstatErr) { - if ((lstatErr as NodeJS.ErrnoException).code !== 'ENOENT') { + if (errnoCode(lstatErr) !== 'ENOENT') { log.warn( { err: lstatErr, path: requestedPath }, '[persistence] lstat failed during broken-symlink check', @@ -1758,7 +1759,10 @@ export function createPersistenceExtension(options?: PersistenceOptions): Persis } canonicalPath = requestedPath; } else if (code === 'ELOOP') { - log.error({ path: requestedPath }, `[persistence] Symlink cycle at ${requestedPath}`); + log.error( + { path: requestedPath, err: e }, + `[persistence] Symlink cycle at ${requestedPath}`, + ); throw new Error(`Symlink cycle detected at ${requestedPath}`); } else { throw e; @@ -2149,7 +2153,7 @@ export function createPersistenceExtension(options?: PersistenceOptions): Persis } canonical = resolvedCanonical; } catch (e) { - const code = (e as NodeJS.ErrnoException).code; + const code = errnoCode(e); if (code === 'ELOOP') { log.warn( { path: filePath }, diff --git a/packages/server/src/process-lock.ts b/packages/server/src/process-lock.ts index 24444aeb6..9f0f22a70 100644 --- a/packages/server/src/process-lock.ts +++ b/packages/server/src/process-lock.ts @@ -21,6 +21,7 @@ import { } from 'node:fs'; import { hostname } from 'node:os'; import { resolve } from 'node:path'; +import { errnoCode } from './http/handler-utils.ts'; import { getLogger } from './logger.ts'; import { getMachineId } from './machine-id.ts'; import { isProcessAlive, isValidLockPid } from './process-alive.ts'; @@ -325,7 +326,7 @@ export function acquireProcessLock(opts: { registerExitUnlink(lockPath); return buildHandle({ lockName, lockDir, lockPath }); } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; + if (errnoCode(err) !== 'EEXIST') throw err; // EEXIST — another acquire raced us; fall through to re-inspect. } } @@ -489,7 +490,7 @@ export function markProcessLockDraining(opts: { lockName: LockName; lockDir: str // Anything else (unreadable, corrupt JSON) has the same consequence as a // failed WRITE (server keeps looking live during teardown), so it must be // as attributable as the write-failure warn below. - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + if (errnoCode(err) !== 'ENOENT') { log.warn( { lockPath, err }, `${logPrefix} Unreadable lock at ${lockPath} during draining mark — skipping: ${err instanceof Error ? err.message : String(err)}`, diff --git a/packages/server/src/project-git.ts b/packages/server/src/project-git.ts index fa46586a5..d93ba6d6c 100644 --- a/packages/server/src/project-git.ts +++ b/packages/server/src/project-git.ts @@ -132,7 +132,7 @@ export async function ensureProjectGit(projectRoot: string): Promise