diff --git a/changelog.d/572.changed.md b/changelog.d/572.changed.md new file mode 100644 index 00000000..e7f7ae3a --- /dev/null +++ b/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. diff --git a/src/adapters/adapter-registry.ts b/src/adapters/adapter-registry.ts index 4720aef1..847173fa 100644 --- a/src/adapters/adapter-registry.ts +++ b/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 { 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', diff --git a/src/proxy/dap-proxy-dependencies.ts b/src/proxy/dap-proxy-dependencies.ts index 26e65a15..57c20714 100644 --- a/src/proxy/dap-proxy-dependencies.ts +++ b/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, diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 46d8ce24..f604d476 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/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; }); diff --git a/src/proxy/proxy-log-path.ts b/src/proxy/proxy-log-path.ts deleted file mode 100644 index 7010903c..00000000 --- a/src/proxy/proxy-log-path.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * The single home for the per-session proxy log file name. - * - * `proxy-.log` was built by hand in three places — the production - * logger factory, the worker's own `redirectProxyLoggers` target, and the - * session layer's failure diagnostics — so the path the diagnostics point a - * user at was only *coincidentally* the path the proxy writes. Renaming the - * file in one place and not the others would have produced a `proxyLogPath` - * that never exists, with no test able to notice. - * - * Kept dependency-free: the worker half of this module is bundled into - * `proxy-bundle.cjs`, so it must not pull anything but `path` in behind it. - */ -import path from 'path'; - -/** 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 debug session's run directory. */ -export function proxyLogPathFor(logDir: string, sessionId: string): string { - return path.join(logDir, proxyLogFileName(sessionId)); -} diff --git a/src/proxy/session-log-layout.ts b/src/proxy/session-log-layout.ts new file mode 100644 index 00000000..2ac08b03 --- /dev/null +++ b/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)); +} diff --git a/src/session/launch/proxy-failure-diagnostics.ts b/src/session/launch/proxy-failure-diagnostics.ts index 5a02e944..60a1a05f 100644 --- a/src/session/launch/proxy-failure-diagnostics.ts +++ b/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; diff --git a/src/session/launch/proxy-launcher.ts b/src/session/launch/proxy-launcher.ts index a38076d2..32b26f98 100644 --- a/src/session/launch/proxy-launcher.ts +++ b/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 diff --git a/src/utils/startup-janitor.ts b/src/utils/startup-janitor.ts index 7e0d9a31..1c0d816e 100644 --- a/src/utils/startup-janitor.ts +++ b/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 { + 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') + ); + }); }); diff --git a/tests/unit/utils/startup-janitor.test.ts b/tests/unit/utils/startup-janitor.test.ts index 2c6dd1ab..a08544c7 100644 --- a/tests/unit/utils/startup-janitor.test.ts +++ b/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') })