fix: bound proxy diagnostic log reads (#569)

changelog.d/569.changed.md

@@ -0,0 +1 @@
+Bound proxy-failure log reads to the final 64 KiB before redacting and trimming them to 80 lines, and remove redundant raw-error serialization from diagnostic log metadata.

packages/adapter-rust/tests/rust-adapter.test.ts

@@ -43,6 +43,7 @@ import { deriveSourceMapFromBinary } from '@debugmcp/codelldb-common';
 const mockDependencies: AdapterDependencies = {
   fileSystem: {
     readFile: vi.fn(),
+    readTail: vi.fn(),
     writeFile: vi.fn(),
     outputFile: vi.fn(),
     exists: vi.fn(),

packages/adapter-rust/tests/rust-debug-adapter.toolchain.test.ts

@@ -50,6 +50,7 @@ import { resolveCodeLLDBExecutable } from '../src/utils/codelldb-resolver.js';
 const createDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: vi.fn(),
+    readTail: vi.fn(),
     writeFile: vi.fn(),
     outputFile: vi.fn(),
     exists: vi.fn(),

packages/shared/src/interfaces/external-dependencies.ts

@@ -21,6 +21,8 @@ export interface IProxyManager {
 export interface IFileSystem {
   // Basic fs operations
   readFile(path: string, encoding?: BufferEncoding): Promise<string>;
+  /** Read at most `maxBytes` from the end of a file as UTF-8 text. */
+  readTail(path: string, maxBytes: number): Promise<string>;
   writeFile(path: string, data: string | Buffer): Promise<void>;
   exists(path: string): Promise<boolean>;
   mkdir(path: string, options?: { recursive?: boolean }): Promise<void>;

src/implementations/file-system-impl.ts

@@ -3,6 +3,7 @@
  */
 import fs from 'fs-extra';
 import { Stats } from 'fs';
+import { open } from 'node:fs/promises';
 import { IFileSystem } from '@debugmcp/shared';
 
 export class FileSystemImpl implements IFileSystem {
@@ -11,6 +12,26 @@ export class FileSystemImpl implements IFileSystem {
     return fs.readFile(path, encoding || 'utf-8');
   }
 
+  async readTail(path: string, maxBytes: number): Promise<string> {
+    if (!Number.isSafeInteger(maxBytes) || maxBytes < 0) {
+      throw new RangeError('maxBytes must be a non-negative safe integer');
+    }
+    if (maxBytes === 0) {
+      return '';
+    }
+
+    const file = await open(path, 'r');
+    try {
+      const { size } = await file.stat();
+      const length = Math.min(size, maxBytes);
+      const buffer = Buffer.alloc(length);
+      const { bytesRead } = await file.read(buffer, 0, length, size - length);
+      return buffer.subarray(0, bytesRead).toString('utf8');
+    } finally {
+      await file.close();
+    }
+  }
+
   async writeFile(path: string, data: string | Buffer): Promise<void> {
     return fs.writeFile(path, data);
   }

src/interfaces/external-dependencies.ts

@@ -17,6 +17,8 @@ import type { IDebugAdapter } from '@debugmcp/shared';
 export interface IFileSystem {
   // Basic fs operations
   readFile(path: string, encoding?: BufferEncoding): Promise<string>;
+  /** Read at most `maxBytes` from the end of a file as UTF-8 text. */
+  readTail(path: string, maxBytes: number): Promise<string>;
   writeFile(path: string, data: string | Buffer): Promise<void>;
   exists(path: string): Promise<boolean>;
   mkdir(path: string, options?: { recursive?: boolean }): Promise<void>;

src/session/launch/proxy-failure-diagnostics.ts

@@ -23,15 +23,11 @@ import type { ProxyInitProgress } from '../../utils/error-messages.js';
 import { getErrorMessage, SessionNotFoundError } from '../../errors/debug-errors.js';
 import { proxyLogPathFor } from '../../proxy/session-log-layout.js';
 
-/** How many trailing proxy-log lines are worth reading after a failure. */
+/** How many trailing proxy-log lines are worth reporting after a failure. */
 const PROXY_LOG_TAIL_LINES = 80;
 
-/**
- * Character cap on the tail. Generous on purpose: it exists so a proxy log with
- * one pathological multi-megabyte line cannot blow up the record, not to trim a
- * normal 80-line tail, which is a few kilobytes.
- */
-const PROXY_LOG_TAIL_CHARS = 64 * 1024;
+/** Hard I/O and allocation cap applied before the log is sanitized. */
+export const PROXY_LOG_TAIL_MAX_BYTES = 64 * 1024;
 
   /** The pointers a failed launch/attach returns to the caller (issue #493 / #551). */
   export interface ProxyFailureDiagnostics {
@@ -45,7 +41,7 @@ const PROXY_LOG_TAIL_CHARS = 64 * 1024;
  */
 export interface ProxyFailureLogDeps {
   logger: ILogger;
-  fileSystem: Pick<IFileSystem, 'readFile'>;
+  fileSystem: Pick<IFileSystem, 'readTail'>;
 }
 
 /** The two operations that can fail this way, named as they appear in the log. */
@@ -94,7 +90,7 @@ export function collectProxyFailureDiagnostics(
 /**
  * Read the last `tailLineCount` lines of the proxy log, if there is one.
  *
- * Reads straight through rather than asking `pathExists` first: the proxy is
+ * Reads the bounded tail rather than asking `pathExists` first: the proxy is
  * still writing (and may rotate) this file, so an exists-then-read pair can
  * report "no log" for a file that appeared a millisecond later, and spends a
  * second syscall to do it. `ENOENT` — the answer that check was buying — is
@@ -111,18 +107,18 @@ export function collectProxyFailureDiagnostics(
  * error that sent us here still reaches the log intact.
  */
 export async function readProxyLogTail(
-  fileSystem: Pick<IFileSystem, 'readFile'>,
+  fileSystem: Pick<IFileSystem, 'readTail'>,
   proxyLogPath: string | undefined,
   tailLineCount: number = PROXY_LOG_TAIL_LINES
 ): Promise<string | undefined> {
   if (!proxyLogPath) {
     return undefined;
   }
   try {
-    const logContent = await fileSystem.readFile(proxyLogPath, 'utf-8');
+    const logContent = await fileSystem.readTail(proxyLogPath, PROXY_LOG_TAIL_MAX_BYTES);
     return sanitizeStderrTail(logContent, {
       maxLines: tailLineCount,
-      maxChars: PROXY_LOG_TAIL_CHARS
+      maxChars: PROXY_LOG_TAIL_MAX_BYTES
     });
   } catch (logReadError) {
     if ((logReadError as NodeJS.ErrnoException)?.code === 'ENOENT') {
@@ -155,13 +151,6 @@ export function buildProxyFailureErrorDetails(
     proxyLogTail
   };
 
-  // Try to capture raw error object
-  try {
-    errorDetails.raw = JSON.stringify(error);
-  } catch {
-    errorDetails.raw = 'Error not JSON serializable';
-  }
-
   return errorDetails;
 }
 

tests/adapters/go/integration/go-session-smoke.test.ts

@@ -8,6 +8,7 @@ import { GoAdapterFactory } from '@debugmcp/adapter-go';
 const createDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/go/unit/go-adapter-factory.test.ts

@@ -19,6 +19,7 @@ const mockSpawn = vi.mocked(spawn);
 const createMockDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/go/unit/go-debug-adapter.test.ts

@@ -19,6 +19,7 @@ const mockSpawn = vi.mocked(spawn);
 const createMockDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/java/unit/java-adapter-factory.test.ts

@@ -18,6 +18,7 @@ const mockSpawn = vi.mocked(spawn);
 const createMockDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/java/unit/java-debug-adapter.test.ts

@@ -19,6 +19,7 @@ const mockSpawn = vi.mocked(spawn);
 const createMockDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/ruby/integration/ruby-session-smoke.test.ts

@@ -9,6 +9,7 @@ import { RubyAdapterFactory } from '@debugmcp/adapter-ruby';
 const createDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/adapters/rust/integration/rust-session-smoke.test.ts

@@ -8,6 +8,7 @@ import { RustAdapterFactory } from '../../../../packages/adapter-rust/src/index.
 const createDependencies = (): AdapterDependencies => ({
   fileSystem: {
     readFile: async () => '',
+    readTail: async () => '',
     writeFile: async () => {},
     exists: async () => false,
     mkdir: async () => {},

tests/core/unit/session/launch/proxy-failure-diagnostics.test.ts

@@ -12,6 +12,7 @@ import {
   buildProxyFailureErrorDetails,
   collectProxyFailureDiagnostics,
   logProxyFailure,
+  PROXY_LOG_TAIL_MAX_BYTES,
   readProxyLogTail
 } from '../../../../../src/session/launch/proxy-failure-diagnostics.js';
 import type { ProxyInitProgress } from '../../../../../src/utils/error-messages.js';
@@ -78,7 +79,7 @@ describe('readProxyLogTail', () => {
   it('returns only the last N content lines, so a long log cannot swamp the record', async () => {
     const fileSystem = createMockFileSystem();
     const allLines = Array.from({ length: 200 }, (_, i) => `line ${i + 1}`);
-    fileSystem.readFile.mockResolvedValue(logFile(allLines));
+    fileSystem.readTail.mockResolvedValue(logFile(allLines));
 
     const tail = await readProxyLogTail(fileSystem, proxyLogPath, 80);
 
@@ -89,11 +90,12 @@ describe('readProxyLogTail', () => {
     expect(tailLines[79]).toContain('line 200');
     // The shared tailer labels what it dropped.
     expect(tail).toContain('(last 80 of 200 lines)');
+    expect(fileSystem.readTail).toHaveBeenCalledWith(proxyLogPath, PROXY_LOG_TAIL_MAX_BYTES);
   });
 
   it('splits CRLF logs, so a Windows proxy log is not one giant line', async () => {
     const fileSystem = createMockFileSystem();
-    fileSystem.readFile.mockResolvedValue('first\r\nsecond\r\nthird\r\n');
+    fileSystem.readTail.mockResolvedValue('first\r\nsecond\r\nthird\r\n');
 
     expect(await readProxyLogTail(fileSystem, proxyLogPath, 2)).toBe(
       'second\nthird (last 2 of 3 lines)'
@@ -104,7 +106,7 @@ describe('readProxyLogTail', () => {
     const fileSystem = createMockFileSystem();
     // The proxy log carries raw adapter argv and DAP output bodies, so the lines
     // a failure makes interesting are exactly the ones that can hold a token.
-    fileSystem.readFile.mockResolvedValue(
+    fileSystem.readTail.mockResolvedValue(
       logFile(['[Worker] spawning adapter', '[Worker] argv: --token=super-secret-value'])
     );
 
@@ -119,21 +121,21 @@ describe('readProxyLogTail', () => {
     const fileSystem = createMockFileSystem();
 
     expect(await readProxyLogTail(fileSystem, undefined)).toBeUndefined();
-    expect(fileSystem.readFile).not.toHaveBeenCalled();
+    expect(fileSystem.readTail).not.toHaveBeenCalled();
   });
 
   it('reads nothing when the proxy never got as far as writing its log', async () => {
     const fileSystem = createMockFileSystem();
     // ENOENT is the answer an exists-check would have bought, one syscall later
     // and with a rotation race in between.
-    fileSystem.readFile.mockRejectedValue(enoent());
+    fileSystem.readTail.mockRejectedValue(enoent());
 
     expect(await readProxyLogTail(fileSystem, proxyLogPath)).toBeUndefined();
   });
 
   it('reports any other read failure as the tail rather than throwing over the real error', async () => {
     const fileSystem = createMockFileSystem();
-    fileSystem.readFile.mockRejectedValue(new Error('permission denied'));
+    fileSystem.readTail.mockRejectedValue(new Error('permission denied'));
 
     expect(await readProxyLogTail(fileSystem, proxyLogPath)).toBe(
       '<<Failed to read proxy log: permission denied>>'
@@ -166,13 +168,11 @@ describe('buildProxyFailureErrorDetails', () => {
     expect(details.stack).toContain('spawn ENOENT');
   });
 
-  it('says so rather than throwing when the error will not serialize', () => {
+  it('does not redundantly serialize the raw error object', () => {
     const circular: Record<string, unknown> = { message: 'cycle' };
     circular.self = circular;
 
-    expect(buildProxyFailureErrorDetails(circular, {}, undefined).raw).toBe(
-      'Error not JSON serializable'
-    );
+    expect(buildProxyFailureErrorDetails(circular, {}, undefined)).not.toHaveProperty('raw');
   });
 
   it('describes a thrown non-error without pretending it has a stack', () => {
@@ -301,7 +301,7 @@ describe('logProxyFailure', () => {
   it('logs the proxy log tail but returns only the pointers', async () => {
     const logger = createMockLogger();
     const fileSystem = createMockFileSystem();
-    fileSystem.readFile.mockResolvedValue(logFile(['adapter said: could not open port']));
+    fileSystem.readTail.mockResolvedValue(logFile(['adapter said: could not open port']));
     const error = Object.assign(new Error('proxy init timed out'), { initProgress });
 
     const diagnostics = await logProxyFailure(

tests/core/unit/session/session-manager-attach-diagnostics.test.ts

@@ -39,7 +39,7 @@ describe('SessionManager - attach failure diagnostics (issue #561)', () => {
   /** Attach a session whose proxy dies during initialization. */
   async function attachAgainstADyingProxy(proxyLogContent: string) {
     const session = await sessionManager.createSession({ language: DebugLanguage.MOCK });
-    vi.mocked(dependencies.mockFileSystem.readFile).mockResolvedValue(proxyLogContent);
+    vi.mocked(dependencies.mockFileSystem.readTail).mockResolvedValue(proxyLogContent);
     vi.spyOn(dependencies.mockProxyManager, 'start').mockRejectedValue(
       Object.assign(new Error('Debug proxy initialization did not complete within 30s'), {
         initProgress
@@ -87,12 +87,12 @@ describe('SessionManager - attach failure diagnostics (issue #561)', () => {
     // Named by the same helper the proxy's own logger uses, so a rename cannot
     // leave the diagnostics pointing at a file nothing writes.
     expect(path.basename(proxyLogPath)).toBe(proxyLogFileName(session.id));
-    expect(dependencies.mockFileSystem.readFile).toHaveBeenCalledWith(proxyLogPath, 'utf-8');
+    expect(dependencies.mockFileSystem.readTail).toHaveBeenCalledWith(proxyLogPath, 64 * 1024);
   });
 
   it('still reports the failure when the proxy log cannot be read', async () => {
     const session = await sessionManager.createSession({ language: DebugLanguage.MOCK });
-    vi.mocked(dependencies.mockFileSystem.readFile).mockRejectedValue(new Error('permission denied'));
+    vi.mocked(dependencies.mockFileSystem.readTail).mockRejectedValue(new Error('permission denied'));
     vi.spyOn(dependencies.mockProxyManager, 'start').mockRejectedValue(new Error('adapter exited'));
 
     const result = await sessionManager.attachToProcess(session.id, { port: 5678 });

tests/test-utils/helpers/test-dependencies.ts

@@ -79,6 +79,7 @@ export function createMockLogger(): ILogger {
 export function createMockFileSystem(): IFileSystem {
   return {
     readFile: vi.fn(),
+    readTail: vi.fn(),
     writeFile: vi.fn(),
     exists: vi.fn(),
     existsSync: vi.fn(),

tests/test-utils/helpers/test-utils.ts

@@ -36,6 +36,7 @@ export function createMockFileSystem(): IFileSystem {
     pathExists: vi.fn().mockResolvedValue(true),
     ensureDir: vi.fn().mockResolvedValue(undefined),
     readFile: vi.fn().mockResolvedValue(''),
+    readTail: vi.fn().mockResolvedValue(''),
     writeFile: vi.fn().mockResolvedValue(undefined),
     remove: vi.fn().mockResolvedValue(undefined),
     copy: vi.fn().mockResolvedValue(undefined),

tests/unit/implementations/file-system-impl.test.ts

@@ -3,6 +3,9 @@
  */
 import { describe, it, expect, beforeEach, vi } from 'vitest';
 import fsExtra from 'fs-extra';
+import { mkdtemp, rm, writeFile as writeNodeFile } from 'node:fs/promises';
+import os from 'node:os';
+import path from 'node:path';
 
 // Mock fs-extra before importing FileSystemImpl
 vi.mock('fs-extra', () => {
@@ -100,6 +103,26 @@ describe('FileSystemImpl', () => {
     });
   });
 
+  describe('readTail', () => {
+    it('reads no more than the requested bytes from the end of a file', async () => {
+      const directory = await mkdtemp(path.join(os.tmpdir(), 'mcp-read-tail-'));
+      const file = path.join(directory, 'proxy.log');
+      try {
+        await writeNodeFile(file, '0123456789', 'utf8');
+
+        await expect(fileSystem.readTail(file, 4)).resolves.toBe('6789');
+        await expect(fileSystem.readTail(file, 64)).resolves.toBe('0123456789');
+        await expect(fileSystem.readTail(file, 0)).resolves.toBe('');
+      } finally {
+        await rm(directory, { recursive: true, force: true });
+      }
+    });
+
+    it('rejects an invalid byte cap before opening the file', async () => {
+      await expect(fileSystem.readTail('/unused', -1)).rejects.toThrow(RangeError);
+    });
+  });
+
   describe('writeFile', () => {
     it('should write string content to file', async () => {
       const content = 'new content';

tests/unit/proxy/proxy-manager.handshake.test.ts

@@ -15,6 +15,7 @@ describe('ProxyManager sendInitWithRetry', () => {
     pathExists: vi.fn(),
     exists: vi.fn(),
     readFile: vi.fn(),
+    readTail: vi.fn(),
     writeFile: vi.fn(),
     readdir: vi.fn(),
     stat: vi.fn(),