refactor: centralize session log layout (#572)

changelog.d/572.changed.md

@@ -0,0 +1 @@
+Centralize session run-directory, proxy-log, adapter-log, and DAP-trace naming so launch, diagnostics, and stale-log cleanup share one exact layout contract.

src/adapters/adapter-registry.ts

@@ -4,6 +4,7 @@
  * @since 2.0.0
  */
 import { EventEmitter } from 'events';
+import { adapterLogPathFor } from '../proxy/session-log-layout.js';
 import {
   IAdapterRegistry,
   IAdapterFactory,
@@ -441,7 +442,7 @@ export class AdapterRegistry extends EventEmitter implements IAdapterRegistry {
   private async createDependencies(config: AdapterConfig): Promise<AdapterDependencies> {
     const { createProductionDependencies } = await import('../container/dependencies.js');
     const logFile = config.logDir && config.sessionId
-      ? `${config.logDir}/${config.sessionId}.log`
+      ? adapterLogPathFor(config.logDir, config.sessionId)
       : undefined;
     const deps = createProductionDependencies({
       logLevel: 'debug',

src/proxy/dap-proxy-dependencies.ts

@@ -6,7 +6,7 @@ import { spawn } from 'child_process';
 import fs from 'fs-extra';
 import { MinimalDapClient } from './minimal-dap.js';
 import { DapMirrorServer } from './dap-mirror-server.js';
-import { proxyLogPathFor } from './proxy-log-path.js';
+import { proxyLogPathFor } from './session-log-layout.js';
 import { createLogger, redirectProxyLoggers } from '../utils/logger.js';
 import {
   DapProxyDependencies,

src/proxy/dap-proxy-worker.ts

@@ -26,7 +26,7 @@ import {
 import type { IDapMirrorServer, MirrorEndpoint } from './dap-mirror-server.js';
 import { CallbackRequestTracker } from './dap-proxy-request-tracker.js';
 import { GenericAdapterManager, AdapterStdioSource } from './dap-proxy-adapter-manager.js';
-import { proxyLogPathFor } from './proxy-log-path.js';
+import { dapTracePathFor, proxyLogPathFor } from './session-log-layout.js';
 import { DapConnectionManager } from './dap-proxy-connection-manager.js';
 import { 
   validateProxyInitPayload
@@ -178,7 +178,7 @@ export class DapProxyWorker {
       if (flag !== '1' && flag !== 'true') {
         return undefined;
       }
-      const tracePath = path.join(logDir, `dap-trace-${sessionId}.ndjson`);
+      const tracePath = dapTracePathFor(logDir, sessionId);
       process.env.DAP_TRACE_FILE = tracePath;
       return tracePath;
     });

src/proxy/session-log-layout.ts

@@ -0,0 +1,60 @@
+/**
+ * The dependency-free naming contract for one debug session's logs.
+ *
+ * This module is bundled into the proxy worker, so it intentionally depends
+ * only on `node:path`. Keeping every producer, reader, and cleanup predicate
+ * here prevents a diagnostic path from drifting away from the file that was
+ * actually written.
+ */
+import path from 'node:path';
+
+const RUN_DIRECTORY_PATTERN = /^run-\d+$/;
+
+/** Directory name for one launch attempt. */
+export function sessionRunDirectoryName(startedAt: number): string {
+  return `run-${startedAt}`;
+}
+
+/** Whether an entry is a managed launch-attempt directory. */
+export function isSessionRunDirectoryName(name: string): boolean {
+  return RUN_DIRECTORY_PATTERN.test(name);
+}
+
+/** Absolute directory for one launch attempt of a debug session. */
+export function sessionRunDirectoryFor(
+  sessionLogBase: string,
+  sessionId: string,
+  startedAt: number
+): string {
+  return path.join(sessionLogBase, sessionId, sessionRunDirectoryName(startedAt));
+}
+
+/** File name (no directory) of the proxy log for one debug session. */
+export function proxyLogFileName(sessionId: string): string {
+  return `proxy-${sessionId}.log`;
+}
+
+/** Absolute path to the proxy log for one launch attempt. */
+export function proxyLogPathFor(runDirectory: string, sessionId: string): string {
+  return path.join(runDirectory, proxyLogFileName(sessionId));
+}
+
+/** File name (no directory) of the debug adapter log for one session. */
+export function adapterLogFileName(sessionId: string): string {
+  return `${sessionId}.log`;
+}
+
+/** Absolute path to the debug adapter log for one launch attempt. */
+export function adapterLogPathFor(runDirectory: string, sessionId: string): string {
+  return path.join(runDirectory, adapterLogFileName(sessionId));
+}
+
+/** File name (no directory) of the opt-in DAP protocol trace. */
+export function dapTraceFileName(sessionId: string): string {
+  return `dap-trace-${sessionId}.ndjson`;
+}
+
+/** Absolute path to the opt-in DAP protocol trace for one launch attempt. */
+export function dapTracePathFor(runDirectory: string, sessionId: string): string {
+  return path.join(runDirectory, dapTraceFileName(sessionId));
+}

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

@@ -21,7 +21,7 @@ import type { ManagedSession } from '../session-store.js';
 import type { IFileSystem, ILogger } from '../../interfaces/external-dependencies.js';
 import type { ProxyInitProgress } from '../../utils/error-messages.js';
 import { getErrorMessage, SessionNotFoundError } from '../../errors/debug-errors.js';
-import { proxyLogPathFor } from '../../proxy/proxy-log-path.js';
+import { proxyLogPathFor } from '../../proxy/session-log-layout.js';
 
 /** How many trailing proxy-log lines are worth reading after a failure. */
 const PROXY_LOG_TAIL_LINES = 80;

src/session/launch/proxy-launcher.ts

@@ -24,6 +24,7 @@ import {
 import { AdapterLease } from '../../adapters/adapter-lease.js';
 import { DebugSessionCreationError, PythonNotFoundError } from '../../errors/debug-errors.js';
 import { ProxyConfig } from '../../proxy/proxy-config.js';
+import { sessionRunDirectoryFor } from '../../proxy/session-log-layout.js';
 import { didYouMean } from '../../utils/did-you-mean.js';
 import { ErrorMessages } from '../../utils/error-messages.js';
 import type { CustomLaunchRequestArguments } from '../session-manager-core.js';
@@ -142,7 +143,7 @@ export class ProxyLauncher {
     const { scriptPath, scriptArgs, dapLaunchArgs, adapterLaunchConfig } = request;
 
     // Create session log directory
-    const sessionLogDir = path.join(this.ctx.logDirBase, sessionId, `run-${Date.now()}`);
+    const sessionLogDir = sessionRunDirectoryFor(this.ctx.logDirBase, sessionId, Date.now());
     this.ctx.logger.info(`[SessionManager] Ensuring session log directory: ${sessionLogDir}`);
     try {
       // ensureDir (fs-extra's recursive mkdir) rejects when it cannot create

src/utils/startup-janitor.ts

@@ -21,6 +21,7 @@
 import * as fsp from 'node:fs/promises';
 import os from 'node:os';
 import path from 'node:path';
+import { isSessionRunDirectoryName } from '../proxy/session-log-layout.js';
 import { scanProcessArgs, type ProcessScanOptions, type ScannedProcess } from './process-scan.js';
 import { reapOrphanJvms, parseArgs, type ReapResult as JvmReapResult, type ReapOptions as JvmReapOptions } from './jvm-orphan-reaper.js';
 import { reapOrphanProxies, parseProxyArgs, parseJsDebugAdapterArgs, type ReapResult as ProxyReapResult, type ReapOptions as ProxyReapOptions } from './proxy-orphan-reaper.js';
@@ -165,7 +166,7 @@ export async function sweepStaleSessionRuns(opts: SweepOptions = {}): Promise<Sw
       continue; // not a directory, or vanished
     }
     for (const runName of runNames) {
-      if (!runName.startsWith('run-')) continue;
+      if (!isSessionRunDirectoryName(runName)) continue;
       const runPath = path.join(sessionPath, runName);
       try {
         const stat = await fsp.stat(runPath);

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

@@ -16,7 +16,7 @@ import path from 'path';
 import { SessionManager, SessionManagerConfig } from '../../../../src/session/session-manager.js';
 import { DebugLanguage, SessionState } from '@debugmcp/shared';
 import type { ProxyInitProgress } from '../../../../src/utils/error-messages.js';
-import { proxyLogFileName } from '../../../../src/proxy/proxy-log-path.js';
+import { proxyLogFileName } from '../../../../src/proxy/session-log-layout.js';
 import { createMockDependencies } from './session-manager-test-utils.js';
 
 const initProgress: ProxyInitProgress = { transportConnected: true, pendingCommand: 'initialize' };

tests/unit/proxy/proxy-log-path.test.ts

@@ -8,9 +8,30 @@
  */
 import { describe, it, expect } from 'vitest';
 import path from 'path';
-import { proxyLogFileName, proxyLogPathFor } from '../../../src/proxy/proxy-log-path.js';
+import {
+  adapterLogFileName,
+  adapterLogPathFor,
+  dapTraceFileName,
+  dapTracePathFor,
+  isSessionRunDirectoryName,
+  proxyLogFileName,
+  proxyLogPathFor,
+  sessionRunDirectoryFor,
+  sessionRunDirectoryName,
+} from '../../../src/proxy/session-log-layout.js';
 
 describe('proxy log path', () => {
+  it('owns the launch-attempt directory layout and recognizes only managed names', () => {
+    expect(sessionRunDirectoryName(1234)).toBe('run-1234');
+    expect(sessionRunDirectoryFor('/logs', 'abc-123', 1234)).toBe(
+      path.join('/logs', 'abc-123', 'run-1234')
+    );
+    expect(isSessionRunDirectoryName('run-1234')).toBe(true);
+    expect(isSessionRunDirectoryName('run-')).toBe(false);
+    expect(isSessionRunDirectoryName('run-backup')).toBe(false);
+    expect(isSessionRunDirectoryName('run-1234.tmp')).toBe(false);
+  });
+
   it('names the file after the session', () => {
     expect(proxyLogFileName('abc-123')).toBe('proxy-abc-123.log');
   });
@@ -34,4 +55,15 @@ describe('proxy log path', () => {
       path.join('/tmp', 'logs', 'proxy-s1.log')
     );
   });
+
+  it('owns the adapter log and DAP trace names and paths', () => {
+    const logDir = path.join('/tmp', 'logs', 'abc-123', 'run-1');
+
+    expect(adapterLogFileName('abc-123')).toBe('abc-123.log');
+    expect(adapterLogPathFor(logDir, 'abc-123')).toBe(path.join(logDir, 'abc-123.log'));
+    expect(dapTraceFileName('abc-123')).toBe('dap-trace-abc-123.ndjson');
+    expect(dapTracePathFor(logDir, 'abc-123')).toBe(
+      path.join(logDir, 'dap-trace-abc-123.ndjson')
+    );
+  });
 });

tests/unit/utils/startup-janitor.test.ts

@@ -184,6 +184,23 @@ describe('sweepStaleSessionRuns', () => {
     }
   });
 
+  it('leaves similarly-prefixed directories outside the managed layout alone', async () => {
+    const base = makeSessionsDir();
+    try {
+      const unmanagedRun = path.join(base, 'sess-3', 'run-backup');
+      fs.mkdirSync(unmanagedRun, { recursive: true });
+      fs.writeFileSync(path.join(unmanagedRun, 'notes.txt'), 'keep');
+      const old = new Date(Date.now() - 8 * 24 * 60 * 60 * 1000);
+      fs.utimesSync(unmanagedRun, old, old);
+
+      await sweepStaleSessionRuns({ baseDir: base });
+
+      expect(fs.existsSync(unmanagedRun)).toBe(true);
+    } finally {
+      fs.rmSync(base, { recursive: true, force: true });
+    }
+  });
+
   it('is silent and safe when the base dir does not exist', async () => {
     await expect(
       sweepStaleSessionRuns({ baseDir: path.join(os.tmpdir(), 'janitor-nonexistent-xyz') })