Serialize logger error metadata safely

src/cli/fleet.test.ts

@@ -6,7 +6,7 @@ import { dirname, join } from 'node:path'
 import type { CloseProbePrInput, Factory, FactoryPorts, createFactory } from '../index'
 import { FileStateStore } from '../state/file-state-store'
 import { FakeFleetClient, FakeMountClient } from '../testing'
-import { installFactoryStopSignalHandlers, parseFleetCommand, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet'
+import { formatLogArgs, installFactoryStopSignalHandlers, parseFleetCommand, parseGlobalOptions, resolveBrokerConnectionPath, runFleetCli } from './fleet'
 
 const issuePath = '/linear/issues/AR-77__uuid-77.json'
 
@@ -62,6 +62,35 @@ const githubIssueFile = (repo: string, number = 48) => ({
   },
 })
 
+describe('fleet CLI logging', () => {
+  it('keeps every argument when Errors and unserializable values reach the stream logger', () => {
+    const circular: Record<string, unknown> = { sibling: 'kept' }
+    circular.self = circular
+    const throwingToJson = {
+      sibling: 'also kept',
+      toJSON: () => {
+        throw new Error('must not run')
+      },
+    }
+
+    const output = formatLogArgs([
+      new Error('top-level failure'),
+      { nested: new Error('nested failure') },
+      circular,
+      throwingToJson,
+      42n,
+      'tail',
+    ])
+
+    expect(output).toContain('"message":"top-level failure"')
+    expect(output).toContain('"message":"nested failure"')
+    expect(output).toContain('"self":"[Circular]"')
+    expect(output).toContain('also kept')
+    expect(output).toContain('"42n"')
+    expect(output).toContain(' tail')
+  })
+})
+
 describe('fleet CLI parsing', () => {
   it('parses spawn flags into a FleetClient spawn input shape', () => {
     expect(parseFleetCommand([

src/cli/fleet.ts

@@ -2,6 +2,7 @@ import { existsSync } from 'node:fs'
 import { readFile } from 'node:fs/promises'
 import { dirname, join, resolve } from 'node:path'
 
+import { stringifyLogValue } from '../logging'
 import { ensureLocalMount } from '../mount/local-mount-preflight'
 import { resolveLocalFactoryConfig, type LocalClonePathOptions } from '../config/local-clone-paths'
 import {
@@ -818,13 +819,9 @@ function streamLogger(stream: Pick<NodeJS.WriteStream, 'write'>): Logger {
   }
 }
 
-function formatLogArgs(args: unknown[]): string {
+export function formatLogArgs(args: unknown[]): string {
   if (args.length === 0) return ''
-  try {
-    return ` ${args.map((arg) => (typeof arg === 'string' ? arg : JSON.stringify(arg))).join(' ')}`
-  } catch {
-    return ''
-  }
+  return ` ${args.map((arg) => (typeof arg === 'string' ? arg : stringifyLogValue(arg))).join(' ')}`
 }
 
 export function resolveBrokerConnectionPath(startCwd = process.cwd()): string | undefined {

src/fleet/internal-fleet-client.test.ts

@@ -1227,7 +1227,11 @@ describe('InternalFleetClient', () => {
     await vi.waitFor(() => expect(harness.sent).toHaveLength(2))
     await vi.waitFor(() => expect(logger.warn).toHaveBeenCalledWith(
       '[factory-sdk] readiness-triggered resend failed, but original event is still active',
-      resendError,
+      expect.objectContaining({
+        name: 'Error',
+        message: 'resend transport failed',
+        stack: expect.stringContaining('Error: resend transport failed'),
+      }),
     ))
     harness.emit({
       kind: 'delivery_injected',

src/fleet/internal-fleet-client.ts

@@ -8,6 +8,7 @@ import type { BrokerEvent, ListAgent, SendMessageInput, SpawnPtyInput } from '@a
 
 import type { AgentMessage, AgentPidResolution, Capability, FleetClient, RosterEntry, SendInput, SpawnInput, SpawnResult } from '../ports/fleet'
 import type { Logger } from '../ports/system'
+import { normalizeLogger } from '../logging'
 import { resolveRelayWorkspaceKey } from './relay-workspace-key'
 
 const requireForResolve = createRequire(import.meta.url)
@@ -132,7 +133,7 @@ export class InternalFleetClient implements FleetClient {
     this.#connectionPath = options.connectionPath
     this.#workspaceKey = options.workspaceKey
     this.#resumeCapability = options.resumeCapability ?? 'spawn:codex'
-    this.#logger = options.logger
+    this.#logger = options.logger ? normalizeLogger(options.logger) : undefined
     this.#resolveAgentRelayMcpCommand = options.resolveAgentRelayMcpCommand ?? resolveAgentRelayMcpCommand
     this.#client = options.client ?? HarnessDriverClient.connect({ cwd: options.cwd, connectionPath: options.connectionPath })
     this.#ownsBroker = options.ownsBroker ?? false

src/logging.test.ts

@@ -0,0 +1,243 @@
+import { describe, expect, it, vi } from 'vitest'
+
+import { normalizeLogger, normalizeLogValue, stringifyLogValue } from './logging'
+
+describe('log metadata normalization', () => {
+  it('serializes a plain Error with its non-enumerable diagnostic fields', () => {
+    const error = new Error('plain failure')
+
+    expect(normalizeLogValue(error)).toMatchObject({
+      name: 'Error',
+      message: 'plain failure',
+      stack: expect.stringContaining('Error: plain failure'),
+    })
+    expect(stringifyLogValue(error)).not.toBe('{}')
+  })
+
+  it('preserves safe structured error fields while redacting sensitive metadata and omitting causes', () => {
+    class ProtocolError extends Error {
+      name = 'HarnessDriverProtocolError'
+      code = 'http_500'
+      status = 500
+      retryable = true
+      data = { error: 'internal reply dropped', accessToken: 'do-not-log' }
+      apiKey = 'do-not-log'
+    }
+    const error = new ProtocolError('broker request failed', { cause: new Error('secret upstream response') })
+    Object.defineProperty(error, 'cause', { value: error.cause, enumerable: true })
+
+    expect(normalizeLogValue(error)).toMatchObject({
+      name: 'HarnessDriverProtocolError',
+      message: 'broker request failed',
+      stack: expect.stringContaining('ProtocolError: broker request failed'),
+      code: 'http_500',
+      status: 500,
+      retryable: true,
+      data: {
+        error: 'internal reply dropped',
+        accessToken: '[Redacted]',
+      },
+      apiKey: '[Redacted]',
+    })
+    expect(normalizeLogValue(error)).not.toHaveProperty('cause')
+  })
+
+  it('contains nested errors, cycles, BigInts, getters, and throwing toJSON hooks', () => {
+    const metadata: Record<string, unknown> = {
+      error: new Error('nested failure'),
+      count: 12n,
+      sibling: 'still visible',
+      toJSON: () => {
+        throw new Error('must not run')
+      },
+    }
+    Object.defineProperty(metadata, 'broken', {
+      enumerable: true,
+      get: () => {
+        throw new Error('must not run')
+      },
+    })
+    metadata.self = metadata
+
+    const normalized = normalizeLogValue(metadata)
+    expect(normalized).toMatchObject({
+      error: {
+        name: 'Error',
+        message: 'nested failure',
+        stack: expect.stringContaining('Error: nested failure'),
+      },
+      count: '12n',
+      sibling: 'still visible',
+      broken: '[Getter]',
+      self: '[Circular]',
+    })
+    expect(() => stringifyLogValue(metadata)).not.toThrow()
+    expect(stringifyLogValue(metadata)).toContain('still visible')
+  })
+
+  it('does not invoke accessor-backed Error fields or array elements', () => {
+    let errorGets = 0
+    const error = new Error()
+    for (const key of ['name', 'message', 'code']) {
+      Object.defineProperty(error, key, {
+        configurable: true,
+        enumerable: true,
+        get: () => {
+          errorGets += 1
+          return `getter-${key}`
+        },
+      })
+    }
+    const stackError = new Error('safe message')
+    Object.defineProperty(stackError, 'stack', {
+      configurable: true,
+      get: () => {
+        errorGets += 1
+        return 'getter-stack'
+      },
+    })
+    let arrayGets = 0
+    const array: unknown[] = []
+    Object.defineProperty(array, '0', {
+      configurable: true,
+      enumerable: true,
+      get: () => {
+        arrayGets += 1
+        return 'getter-item'
+      },
+    })
+
+    expect(normalizeLogValue(error)).toEqual({ name: 'Error', message: 'Error' })
+    expect(normalizeLogValue(stackError)).toEqual({
+      name: 'Error',
+      message: 'safe message',
+      stack: 'Error: safe message',
+    })
+    expect(normalizeLogValue(array)).toEqual(['[Getter]'])
+    expect(errorGets).toBe(0)
+    expect(arrayGets).toBe(0)
+  })
+
+  it('uses intrinsic special-object conversion without calling instance hooks', () => {
+    let dateCalls = 0
+    const date = new Date(0)
+    Object.defineProperty(date, 'toISOString', {
+      value: () => {
+        dateCalls += 1
+        return 'unsafe date'
+      },
+    })
+    let regexpCalls = 0
+    const regexp = /factory/giu
+    Object.defineProperty(regexp, 'toString', {
+      value: () => {
+        regexpCalls += 1
+        return 'unsafe regexp'
+      },
+    })
+    Object.defineProperty(regexp, 'source', {
+      configurable: true,
+      get: () => {
+        regexpCalls += 1
+        return 'unsafe-source'
+      },
+    })
+    Object.defineProperty(regexp, 'flags', {
+      configurable: true,
+      get: () => {
+        regexpCalls += 1
+        return 'i'
+      },
+    })
+    let functionNameGets = 0
+    const fn = () => undefined
+    Object.defineProperty(fn, 'name', {
+      configurable: true,
+      get: () => {
+        functionNameGets += 1
+        return 'unsafe function name'
+      },
+    })
+
+    expect(normalizeLogValue(date)).toBe('1970-01-01T00:00:00.000Z')
+    expect(normalizeLogValue(regexp)).toBe('/factory/giu')
+    expect(normalizeLogValue(fn)).toBe('[Function]')
+    expect({ dateCalls, regexpCalls, functionNameGets }).toEqual({
+      dateCalls: 0,
+      regexpCalls: 0,
+      functionNameGets: 0,
+    })
+  })
+
+  it('does not materialize a lazy stack through a custom Error.prepareStackTrace hook', () => {
+    const errorCreatedBeforeHook = new Error('before hook')
+    const original = Object.getOwnPropertyDescriptor(Error, 'prepareStackTrace')
+    let calls = 0
+    Object.defineProperty(Error, 'prepareStackTrace', {
+      configurable: true,
+      value: () => {
+        calls += 1
+        return 'unsafe stack hook output'
+      },
+    })
+    try {
+      const errorCreatedAfterHook = new Error('after hook')
+      expect(normalizeLogValue(errorCreatedBeforeHook)).toEqual({
+        name: 'Error',
+        message: 'before hook',
+        stack: 'Error: before hook',
+      })
+      expect(normalizeLogValue(errorCreatedAfterHook)).toEqual({
+        name: 'Error',
+        message: 'after hook',
+        stack: 'Error: after hook',
+      })
+      expect(calls).toBe(0)
+    } finally {
+      if (original) Object.defineProperty(Error, 'prepareStackTrace', original)
+      else delete (Error as ErrorConstructor & { prepareStackTrace?: unknown }).prepareStackTrace
+    }
+  })
+
+  it('applies the global field budget to branching nested arrays', () => {
+    const branch = (depth: number): unknown => depth === 0
+      ? 'leaf'
+      : Array.from({ length: 5 }, () => branch(depth - 1))
+    const normalized = normalizeLogValue(branch(6))
+    const countNodes = (value: unknown): number => Array.isArray(value)
+      ? 1 + value.reduce((count, child) => count + countNodes(child), 0)
+      : 1
+
+    expect(countNodes(normalized)).toBeLessThanOrEqual(210)
+    expect(JSON.stringify(normalized)).toContain('[FieldLimit]')
+  })
+
+  it('normalizes every argument passed through an injected logger', () => {
+    const warn = vi.fn()
+    const logger = normalizeLogger({ warn })
+
+    logger.warn?.('representative warning', new Error('raw failure'), {
+      nested: new Error('nested failure'),
+    })
+
+    expect(warn).toHaveBeenCalledWith(
+      'representative warning',
+      expect.objectContaining({ message: 'raw failure', stack: expect.any(String) }),
+      { nested: expect.objectContaining({ message: 'nested failure', stack: expect.any(String) }) },
+    )
+  })
+
+  it('keeps the default console logger callable through the adapter', () => {
+    const error = vi.spyOn(console, 'error').mockImplementation(() => undefined)
+    try {
+      normalizeLogger(console).error?.('default console error', new Error('console failure'))
+
+      expect(error).toHaveBeenCalledWith(
+        'default console error',
+        expect.objectContaining({ message: 'console failure', stack: expect.any(String) }),
+      )
+    } finally {
+      error.mockRestore()
+    }
+  })
+})