diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ada426..d2f60150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING (behavioral): launch sessions now default to `breakOnExceptions: "uncaught"`** — a crashing script pauses at the uncaught exception (`lastStop.reason: "exception"`, stack/locals inspectable, `exceptionInfo` where supported) instead of running to termination. Applies uniformly to Python, JavaScript, Java, Go (panics), .NET, Rust (panics), and the mock adapter; Ruby keeps the old run-to-termination behavior (rdbg has no uncaught-only filter). Pass `breakOnExceptions: "none"` to restore the old behavior per session. Attach sessions are unchanged — no default is ever applied on attach (fixes #244) ### Added +- **`expectedContent` breakpoint assertions + loud snapping** — `set_breakpoint` accepts an optional `expectedContent`: the exact text you expect on the target line (whitespace-trimmed). On a mismatch the breakpoint is NOT set and the error shows expected vs actual plus the surrounding lines — converting an off-by-one line number from confusing session behavior into an immediate, self-explanatory failure. When an adapter binds a breakpoint to a different line than requested, the response now reports it prominently (`requested line N, bound to line M` in `message`/`warning`, with `requestedLine` alongside the bound `line`) instead of silently mutating the line; asynchronous relocations (js-debug-style breakpoint events) surface in `list_breakpoints` as `line` ≠ `requestedLine`. Session-layer live-sync failures now also reach the `set_breakpoint` response as a `warning` (previously discarded). Content assertions require a server-readable source file (rejected for Java FQCNs and attach sessions with a clear error), and the source-line cache is now mtime-validated so mid-session edits are seen immediately. The new `DEBUG_MCP_BP_ADDRESSING` env flag (`line` | `assert` | `content`, default `content`) restricts addressing features — runtime behavior, tool schema, server instructions, and prompt text all gate together, enabling controlled A/B comparisons of agent debugging behavior (#271) + - **Stop-reason normalization + `stopReason` in pause results** — a new per-adapter `normalizeStopReason` policy hook maps misleading raw DAP stop reasons to canonical ones before they drive auto-continue, `lastStop`, and `exceptionInfo` enrichment: CodeLLDB reports an explicit pause (delivered via SIGSTOP) as `"exception"`, and js-debug reports it as `"step"` — both now surface as `"pause"`, with the adapter's original value preserved as `lastStop.rawReason`. Real exceptions (SIGSEGV, panics) are never reclassified, and normalized pauses no longer trigger a spurious `exceptionInfo` request. `pause_execution` now reports `data.stopReason`/`data.rawStopReason` for fresh stops (stale earlier stops are never echoed) - **Optimized-binary warning on `get_local_variables`** — when the adapter annotates the locals scope (Delve's `"Locals (warning: optimized function)"` for optimized frames), the response now reports the actual scope name and a `warning` field with remediation guidance (rebuild with `-gcflags="all=-N -l"`, or use Delve debug mode). Previously Go's exact-name scope match silently returned `[]` for optimized frames; the scope is now prefix-matched so available variables are returned. `examples/go/README.md` no longer suggests `buildFlags` with `mode: "exec"` (Delve ignores it there) diff --git a/docs/architecture/api-reference.md b/docs/architecture/api-reference.md index 70b29852..1d32fa76 100644 --- a/docs/architecture/api-reference.md +++ b/docs/architecture/api-reference.md @@ -322,10 +322,10 @@ Starts debugging for a session. **Returns**: Debug result with success status -#### `setBreakpoint(sessionId: string, file: string, line: number, condition?: string): Promise` -Sets a breakpoint in a file. Internally sends a DAP `setBreakpoints` request for all breakpoints in the same source file. +#### `setBreakpoint(sessionId: string, bp: { file: string; line: number; condition?: string; logMessage?: string; suspendPolicy?: 'all' | 'thread'; requestedLine?: number }): Promise<{ breakpoint: Breakpoint; warning?: string }>` +Sets a breakpoint in a file. Internally sends a DAP `setBreakpoints` request for all breakpoints in the same source file. `requestedLine` records the originally requested line for loud snapping (issue #271); `warning` carries live-sync failures. -**Returns**: Breakpoint information +**Returns**: Breakpoint information plus an optional sync warning #### `continue(sessionId: string, threadId?: number): Promise` Resumes execution from a breakpoint. diff --git a/docs/docker-support.md b/docs/docker-support.md index 400354d4..e6064a56 100644 --- a/docs/docker-support.md +++ b/docs/docker-support.md @@ -91,6 +91,7 @@ Here's the recommended configuration for your MCP settings file: - The `:rw` suffix allows read-write access (required for debugging) - The Docker entrypoint (`scripts/docker-entry.sh`) runs `dist/bundle.cjs` and passes through command-line arguments (e.g., `stdio`). It does not hardcode `--log-level` or `--log-file` - When using the debugger, provide paths relative to the project root (e.g., `examples/test.py` not `/workspace/examples/test.py`) +- Optional env flags pass through with `-e`, e.g. `-e DEBUG_MCP_BP_ADDRESSING=line` restricts breakpoint addressing features (default: all enabled; see the set_breakpoint section of the tool reference) ## Rust support in Docker diff --git a/docs/tool-reference.md b/docs/tool-reference.md index b638e4ee..68b698e9 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -136,6 +136,7 @@ Sets a breakpoint in a source file. - `sessionId` (string, required): The ID of the debug session. - `file` (string, required): Path to the source file (absolute or relative to project root). - `line` (number, required): Line number where to set breakpoint (1-indexed). +- `expectedContent` (string, optional): Assert the exact text of the target line (leading/trailing whitespace ignored) before setting. On a mismatch the breakpoint is **not** set and the error shows the actual content of that line and its neighbors — a fast, self-explanatory failure instead of a breakpoint that silently lands on the wrong line. See [Content assertions and loud snapping](#content-assertions-and-loud-snapping). - `condition` (string, optional): Conditional expression — only break (or log) when it evaluates truthy. - `logMessage` (string, optional): Create a **logpoint** instead of a pausing breakpoint — see [Logpoints](#logpoints) below. - `suspendPolicy` (string, optional): Suspend policy when the breakpoint is hit — `"all"` suspends all threads (default), `"thread"` suspends only the event thread. Only supported by the Java/JDI adapter. @@ -167,6 +168,28 @@ Sets a breakpoint in a source file. - The response includes the absolute path even if you provide a relative path - Setting breakpoints on non-executable lines (comments, blank lines, declarations) may cause unexpected behavior - Executable lines that work well: assignments, function calls, conditionals, returns +- The top-level `content` field echoes the bound line's text (same as `context.lineContent`) + +#### Content assertions and loud snapping + +`expectedContent` is a checksum on intent: agents that compute line numbers from a code listing routinely land one line off, and a breakpoint on a blank line or brace produces confusing session behavior much later. With `expectedContent`, the mismatch fails at set time: + +``` +Breakpoint not set: line 12 of /abs/app.py does not match expectedContent. +Expected: "total = sum(prices)" +Actual: "return total" +Context: + 10 | prices = load() + 11 | total = sum(prices) +> 12 | return total + 13 | + 14 | def main(): +The file may have changed since you last read it. Pick the correct line from the context above. +``` + +Relatedly, when a debug adapter *accepts* a breakpoint but binds it to a different line (adapters snap requests on non-executable lines to the nearest valid one), the response reports it prominently instead of silently mutating the line: `message` and `warning` carry `"requested line 12, bound to line 13: \`...\`"`, and the response includes `requestedLine` alongside the bound `line`. Adapters that relocate breakpoints asynchronously (after the response) surface the move in `list_breakpoints`, where `line` ≠ `requestedLine` marks a snapped breakpoint. + +`expectedContent` requires a source file the server can read: it is rejected for Java FQCN breakpoints and attach-mode sessions (remote filesystems). Both addressing aids can be restricted with the `DEBUG_MCP_BP_ADDRESSING` environment variable (`line` = pre-existing behavior, `assert` = + expectedContent/loud snapping, `content` = all features; default `content`) — useful for controlled comparisons of agent behavior. #### Logpoints diff --git a/packages/adapter-mock/src/mock-adapter-process.ts b/packages/adapter-mock/src/mock-adapter-process.ts index 72ae5a95..d00ee84a 100644 --- a/packages/adapter-mock/src/mock-adapter-process.ts +++ b/packages/adapter-mock/src/mock-adapter-process.ts @@ -511,16 +511,33 @@ class MockDebugAdapterProcess { } } + /** + * Breakpoint snap simulation (issue #271): when the launched program path + * contains "snap", odd requested lines bind one line down (odd -> next + * even is idempotent, so replace-all re-syncs of already-snapped lines + * don't drift). "snap-event" additionally defers: the response echoes the + * requested line unverified, then a DAP breakpoint(changed) event delivers + * the relocation + verification asynchronously. + */ + private snapLine(line: number | undefined): number | undefined { + if (line === undefined) return line; + return line % 2 === 1 ? line + 1 : line; + } + private handleSetBreakpoints(request: DebugProtocol.SetBreakpointsRequest): void { const args = request.arguments; const breakpoints: DebugProtocol.Breakpoint[] = []; - + const simulateSnapEvent = /snap-event/i.test(this.programPath); + const simulateSnap = !simulateSnapEvent && /snap/i.test(this.programPath); + if (args.breakpoints) { for (const bp of args.breakpoints) { + const id = Math.floor(Math.random() * 100000); + const boundLine = simulateSnap ? this.snapLine(bp.line) : bp.line; breakpoints.push({ - id: Math.floor(Math.random() * 100000), - verified: true, - line: bp.line, + id, + verified: !simulateSnapEvent, + line: boundLine, source: args.source, // Retained for the run simulation: logpoint lines log instead of stopping ...(bp.logMessage !== undefined ? { logMessage: bp.logMessage } : {}) @@ -529,7 +546,7 @@ class MockDebugAdapterProcess { } this.breakpoints.set(args.source?.path || 'unknown', breakpoints); - + this.sendResponse({ seq: 0, type: 'response', @@ -540,6 +557,22 @@ class MockDebugAdapterProcess { breakpoints } }); + + if (simulateSnapEvent) { + for (const bp of breakpoints) { + setTimeout(() => { + const relocated = { ...bp, verified: true, line: this.snapLine(bp.line) }; + bp.verified = true; + bp.line = relocated.line; + this.sendEvent({ + seq: 0, + type: 'event', + event: 'breakpoint', + body: { reason: 'changed', breakpoint: relocated } + } as DebugProtocol.BreakpointEvent); + }, 100); + } + } } private handleThreads(request: DebugProtocol.ThreadsRequest): void { diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 8e48dcbd..83314058 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -225,6 +225,13 @@ export interface Breakpoint { message?: string; /** Breakpoint id assigned by the debug adapter (from setBreakpoints responses / breakpoint events) */ adapterId?: number; + /** + * The line originally requested by the client, recorded before the adapter + * had a chance to bind elsewhere. Present only in assert/content addressing + * modes (issue #271); `line` !== `requestedLine` means the adapter snapped + * the breakpoint. Never sent to the adapter. + */ + requestedLine?: number; } /** diff --git a/skills/debugging/SKILL.md b/skills/debugging/SKILL.md index 69b6d360..23332cb2 100644 --- a/skills/debugging/SKILL.md +++ b/skills/debugging/SKILL.md @@ -21,7 +21,7 @@ Do NOT reach for it when a single glance at the code or one log line would answe ```text 1. create_debug_session {language: "python"} -> sessionId -2. set_breakpoint {sessionId, file: "", line: N} +2. set_breakpoint {sessionId, file: "", line: N, expectedContent: ""} 3. start_debugging {sessionId, scriptPath: ""} 4. get_stack_trace {sessionId} -> frames (use frame.id, never assume 0) 5. get_scopes {sessionId, frameId: } -> scope variablesReference @@ -46,6 +46,7 @@ Rules that prevent 90% of failed sessions: 1. State a hypothesis about where reality diverges from expectation *before* setting breakpoints. 2. Set at most two breakpoints: last-known-good and first-known-bad. Run, inspect, halve the interval. Bisection beats stepping line-by-line from the top. Move the window mid-session with `remove_breakpoint` / `clear_breakpoints`; `list_breakpoints` shows what is currently set (with verified state and adapter ids). + - Pass `expectedContent: ""` with every line-addressed breakpoint: if your line number is stale or off by one, the set fails immediately with the actual nearby lines instead of binding somewhere surprising. A response saying `requested line N, bound to line M` means the adapter moved the breakpoint — trust the bound line. 3. When pausing is too disruptive (hot loops, live or attached processes), use a **logpoint**: `set_breakpoint` with `logMessage: "x={x}"` streams interpolated values into `get_output` without stopping the program (Python/JS/Go/Rust; Java and .NET reject it with a clear error). 4. At each pause, record what you *learned* (variable values, actual control flow), not just where you are. 5. When the diverging line is found, inspect every input to that line before concluding — the bug is usually an operand, not the operator. diff --git a/src/server.ts b/src/server.ts index 841999d0..f215ab93 100644 --- a/src/server.ts +++ b/src/server.ts @@ -16,7 +16,7 @@ import { McpError, ServerResult, } from '@modelcontextprotocol/sdk/types.js'; -import { SERVER_INSTRUCTIONS, DEBUGGING_WORKFLOW_PROMPT } from './skill-content.js'; +import { buildServerInstructions, buildDebuggingWorkflowPrompt } from './skill-content.js'; import { SessionNotFoundError, SessionTerminatedError, @@ -43,6 +43,14 @@ import { SimpleFileChecker, createSimpleFileChecker, FileExistenceResult } from import { LineReader, createLineReader } from './utils/line-reader.js'; import { getDisabledLanguages, isLanguageDisabled } from './utils/language-config.js'; import { isContainerMode, getWorkspaceRoot } from './utils/container-path-utils.js'; +import { + BP_ADDRESSING_ENV_KEY, + getBpAddressingMode, + supportsExpectedContent, + supportsStatementAnchors, + supportsLoudSnapping +} from './utils/bp-addressing.js'; +import { assertLineContent } from './utils/breakpoint-resolver.js'; const DEFAULT_LANGUAGES = Object.freeze([DebugLanguage.PYTHON, DebugLanguage.MOCK] as const); @@ -88,6 +96,7 @@ interface ToolArguments { line?: number; condition?: string; logMessage?: string; + expectedContent?: string; breakpointId?: string; scriptPath?: string; args?: string[]; @@ -121,6 +130,21 @@ interface ToolArguments { limit?: number; } +/** + * Request shape for DebugMcpServer.setBreakpoint (issue #271). + */ +export interface SetBreakpointRequest { + sessionId: string; + file: string; + /** 1-based target line */ + line: number; + /** Assert the target line's trimmed content before setting (assert/content modes) */ + expectedContent?: string; + condition?: string; + logMessage?: string; + suspendPolicy?: 'all' | 'thread'; +} + /** * Schema-driven type coercion for MCP tool arguments. * @@ -462,12 +486,12 @@ export class DebugMcpServer { sessionId: string, file: string, options?: { requireExists?: boolean } - ): Promise { + ): Promise<{ path: string; contentAddressable: boolean }> { // Check if the adapter handles non-file source identifiers (e.g. Java FQCNs) const policy = this.sessionManager.getSessionPolicy(sessionId); if (policy.isNonFileSourceIdentifier?.(file)) { this.logger.info(`[DebugMcpServer.resolveBreakpointFile] Non-file source identifier detected: ${file}`); - return file; + return { path: file, contentAddressable: false }; } // Attach sessions may debug a target on a remote filesystem (container, @@ -475,7 +499,7 @@ export class DebugMcpServer { // path through as-is — the debugger knows its own filesystem best. if (this.sessionManager.getSession(sessionId)?.attachMode) { this.logger.info(`[DebugMcpServer.resolveBreakpointFile] Attach session: skipping host file check for ${file}`); - return file; + return { path: file, contentAddressable: false }; } const fileCheck = await this.fileChecker.checkExists(file); @@ -484,14 +508,47 @@ export class DebugMcpServer { } this.logger.info(`[DebugMcpServer.resolveBreakpointFile] Resolved ${file} -> ${fileCheck.effectivePath} (exists: ${fileCheck.exists})`); - return fileCheck.effectivePath; + return { path: fileCheck.effectivePath, contentAddressable: true }; } - public async setBreakpoint(sessionId: string, file: string, line: number, condition?: string, suspendPolicy?: 'all' | 'thread', logMessage?: string): Promise { - this.validateSession(sessionId); + public async setBreakpoint(req: SetBreakpointRequest): Promise<{ breakpoint: Breakpoint; warning?: string }> { + this.validateSession(req.sessionId); - const effectiveFile = await this.resolveBreakpointFile(sessionId, file, { requireExists: true }); - return this.sessionManager.setBreakpoint(sessionId, effectiveFile, line, condition, suspendPolicy, logMessage); + const resolved = await this.resolveBreakpointFile(req.sessionId, req.file, { requireExists: true }); + const mode = getBpAddressingMode(this.environment); + + if (req.expectedContent !== undefined) { + if (!resolved.contentAddressable) { + throw new McpError( + McpErrorCode.InvalidParams, + `expectedContent requires a source file readable by the mcp-debugger server; "${req.file}" is a class name or remote path. Use line addressing instead.` + ); + } + const lines = await this.lineReader.getFileLines(resolved.path); + if (!lines) { + throw new McpError( + McpErrorCode.InvalidParams, + `Breakpoint not set: could not read ${resolved.path} to verify content (binary, too large, or unreadable). Use plain line addressing to skip verification.` + ); + } + const check = assertLineContent(lines, req.line, req.expectedContent, resolved.path, { + statementHint: supportsStatementAnchors(mode) + }); + if (!check.ok) { + throw new McpError(McpErrorCode.InvalidParams, check.message); + } + } + + return this.sessionManager.setBreakpoint(req.sessionId, { + file: resolved.path, + line: req.line, + condition: req.condition, + suspendPolicy: req.suspendPolicy, + logMessage: req.logMessage, + // Loud snapping bookkeeping is absent in line mode so the control arm's + // breakpoint records stay byte-identical to pre-#271 behavior. + ...(supportsLoudSnapping(mode) ? { requestedLine: req.line } : {}) + }); } // The breakpoint management tools below deliberately skip validateSession's @@ -507,13 +564,13 @@ export class DebugMcpServer { } public async removeBreakpointsByLocation(sessionId: string, file: string, line: number): Promise<{ removed: Breakpoint[]; warning?: string }> { - const effectiveFile = await this.resolveBreakpointFile(sessionId, file); - return this.sessionManager.removeBreakpointsByLocation(sessionId, effectiveFile, line); + const resolved = await this.resolveBreakpointFile(sessionId, file); + return this.sessionManager.removeBreakpointsByLocation(sessionId, resolved.path, line); } public async clearBreakpoints(sessionId: string, file?: string): Promise<{ cleared: number; files: string[]; warning?: string }> { const effectiveFile = file !== undefined - ? await this.resolveBreakpointFile(sessionId, file) + ? (await this.resolveBreakpointFile(sessionId, file)).path : undefined; return this.sessionManager.clearBreakpoints(sessionId, effectiveFile); } @@ -629,7 +686,9 @@ export class DebugMcpServer { { name: 'debug-mcp-server', version: '0.1.0' }, { capabilities: { tools: {}, resources: { subscribe: true, listChanged: true }, prompts: {} }, - instructions: SERVER_INSTRUCTIONS + // Mode-gated (issue #271): the handshake must not teach restricted + // addressing features. Env is process-stable, so constructor-time is fine. + instructions: buildServerInstructions(getBpAddressingMode(this.environment)) } ); @@ -704,13 +763,24 @@ export class DebugMcpServer { // Generate dynamic descriptions for path parameters const fileDescription = this.getPathDescription('source file'); const scriptPathDescription = this.getPathDescription('script'); - + + // Addressing-mode-gated set_breakpoint params (issue #271): a server + // restricted to line mode must not advertise content-addressing at all. + const bpMode = getBpAddressingMode(this.environment); + const setBreakpointExtraProps: Record = {}; + if (supportsExpectedContent(bpMode)) { + setBreakpointExtraProps.expectedContent = { + type: 'string', + description: 'Optional assertion: the exact text you expect on the target line (leading/trailing whitespace ignored). If it does not match, the breakpoint is NOT set and the error shows the actual content of that line and its neighbors — use this to catch stale or off-by-one line numbers before they cause confusing behavior' + }; + } + return { tools: [ { name: 'create_debug_session', description: 'Create a new debugging session. Provide host and port to attach to a running process; omit them for launch mode', inputSchema: { type: 'object', properties: { language: { type: 'string', enum: supportedLanguages, description: 'Programming language for debugging' }, name: { type: 'string', description: 'Optional session name' }, executablePath: {type: 'string', description: 'Path to language executable (optional, will auto-detect if not provided)'}, host: { type: 'string', description: 'Host to attach to for remote debugging (optional, triggers attach mode)' }, port: { type: 'number', description: 'Debug port to attach to for remote debugging (optional, triggers attach mode)' }, timeout: { type: 'number', description: 'Connection timeout in milliseconds for attach mode (default: 30000)' }, verifyTimeout: { type: 'number', description: 'Attach mode only: how long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 5000, max: 600000)' } }, required: ['language'] } }, { name: 'list_supported_languages', description: 'List all supported debugging languages with metadata', inputSchema: { type: 'object', properties: {} } }, { name: 'list_debug_sessions', description: 'List all active debugging sessions. Paused sessions include lastStop with the reason for the most recent stop (e.g. "breakpoint" vs "exception")', inputSchema: { type: 'object', properties: {} } }, - { name: 'set_breakpoint', description: 'Set a breakpoint. Setting breakpoints on non-executable lines (structural, declarative) may lead to unexpected behavior', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Path to the source file or Java FQCN. For Java, passing a fully-qualified class name (e.g. "com.example.MyClass" or "com.example.Outer$Inner") is preferred — it works reliably with all classloaders including custom classloaders. Alternatively, use absolute file paths.' }, line: { type: 'number', description: 'Line number where to set breakpoint. Executable statements (assignments, function calls, conditionals, returns) work best. Structural lines (function/class definitions), declarative lines (imports), or non-executable lines (comments, blank lines) may cause unexpected stepping behavior' }, condition: { type: 'string', description: 'Optional expression: only break (or log) when it evaluates truthy' }, logMessage: { type: 'string', description: 'Create a logpoint: instead of pausing, log this message when the line is hit. Expressions in {curly braces} are interpolated (e.g. "order={orderId} total={total}"). Messages arrive in get_output while the program runs at full speed. Supported by Python, JavaScript, Go, and Rust adapters; not by Java or .NET' }, suspendPolicy: { type: 'string', enum: ['all', 'thread'], description: 'Suspend policy when breakpoint is hit: "all" suspends all threads (default), "thread" only suspends the event thread. Only supported by the Java/JDI adapter.' } }, required: ['sessionId', 'file', 'line'] } }, + { name: 'set_breakpoint', description: 'Set a breakpoint. Setting breakpoints on non-executable lines (structural, declarative) may lead to unexpected behavior', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Path to the source file or Java FQCN. For Java, passing a fully-qualified class name (e.g. "com.example.MyClass" or "com.example.Outer$Inner") is preferred — it works reliably with all classloaders including custom classloaders. Alternatively, use absolute file paths.' }, line: { type: 'number', description: 'Line number where to set breakpoint. Executable statements (assignments, function calls, conditionals, returns) work best. Structural lines (function/class definitions), declarative lines (imports), or non-executable lines (comments, blank lines) may cause unexpected stepping behavior' }, ...setBreakpointExtraProps, condition: { type: 'string', description: 'Optional expression: only break (or log) when it evaluates truthy' }, logMessage: { type: 'string', description: 'Create a logpoint: instead of pausing, log this message when the line is hit. Expressions in {curly braces} are interpolated (e.g. "order={orderId} total={total}"). Messages arrive in get_output while the program runs at full speed. Supported by Python, JavaScript, Go, and Rust adapters; not by Java or .NET' }, suspendPolicy: { type: 'string', enum: ['all', 'thread'], description: 'Suspend policy when breakpoint is hit: "all" suspends all threads (default), "thread" only suspends the event thread. Only supported by the Java/JDI adapter.' } }, required: ['sessionId', 'file', 'line'] } }, { name: 'list_breakpoints', description: 'List all breakpoints in a session with their verified state and adapter-assigned ids. Works before launch (queued, verified=false), while running or paused, and after the program exits', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Optional: only list breakpoints in this file' } }, required: ['sessionId'] } }, { name: 'remove_breakpoint', description: 'Remove a breakpoint by breakpointId (returned by set_breakpoint / list_breakpoints), or by file + line (removes all breakpoints at that location). Takes effect immediately while the program is running or paused; also works after the program exits, before a relaunch', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, breakpointId: { type: 'string', description: 'Breakpoint id from set_breakpoint or list_breakpoints. Takes precedence over file + line' }, file: { type: 'string', description: 'Alternative addressing: source file path (use together with line)' }, line: { type: 'number', description: 'Alternative addressing: line number (use together with file)' } }, required: ['sessionId'] } }, { name: 'clear_breakpoints', description: 'Remove all breakpoints in a session, or all breakpoints in one file. Clearing zero breakpoints is success. Takes effect immediately while the program is running or paused', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Optional: only clear breakpoints in this file' } }, required: ['sessionId'] } }, @@ -895,7 +965,20 @@ export class DebugMcpServer { if (!args.sessionId || !args.file || args.line === undefined) { throw new McpError(McpErrorCode.InvalidParams, 'Missing required parameters'); } - + + // Addressing-mode gating (issue #271): reject params outside the + // configured mode even though the schema omits them — a client + // replaying a cached schema must not slip features into a + // restricted server. Checked on the raw args so unknown params + // are caught too. + const bpMode = getBpAddressingMode(this.environment); + if (args.expectedContent !== undefined && !supportsExpectedContent(bpMode)) { + throw new McpError( + McpErrorCode.InvalidParams, + `expectedContent is disabled (${BP_ADDRESSING_ENV_KEY}=${bpMode}). Use plain line addressing.` + ); + } + try { // Logpoint gating (issue #235): hard error for known-unsupported // adapters; a warning when support is unknown pre-launch. @@ -903,7 +986,15 @@ export class DebugMcpServer { ? this.validateLogPointSupport(args.sessionId) : {}; - const breakpoint = await this.setBreakpoint(args.sessionId, args.file, args.line, args.condition, args.suspendPolicy, args.logMessage); + const { breakpoint, warning: syncWarning } = await this.setBreakpoint({ + sessionId: args.sessionId, + file: args.file, + line: args.line, + expectedContent: args.expectedContent, + condition: args.condition, + suspendPolicy: args.suspendPolicy, + logMessage: args.logMessage + }); // Log breakpoint event this.logger.info('debug:breakpoint', { @@ -941,16 +1032,31 @@ export class DebugMcpServer { }); } - const warnings = [breakpoint.message, logPointGate.warning].filter(Boolean); + // Loud snapping (issue #271): if the adapter bound the + // breakpoint to a different line than requested, say so + // prominently instead of silently reporting the moved line. + const snapped = + breakpoint.requestedLine !== undefined && + breakpoint.line !== breakpoint.requestedLine; + const snapWarning = snapped + ? `Breakpoint moved by the debugger: requested line ${breakpoint.requestedLine}, bound to line ${breakpoint.line}${ + context ? `: \`${context.lineContent.trim()}\`` : '' + }` + : undefined; + + const warnings = [breakpoint.message, logPointGate.warning, syncWarning, snapWarning].filter(Boolean); result = { content: [{ type: 'text', text: JSON.stringify({ success: true, breakpointId: breakpoint.id, file: breakpoint.file, line: breakpoint.line, + requestedLine: breakpoint.requestedLine, + content: context?.lineContent, verified: breakpoint.verified, logMessage: breakpoint.logMessage, - message: breakpoint.message || `${breakpoint.logMessage !== undefined ? 'Logpoint' : 'Breakpoint'} set at ${breakpoint.file}:${breakpoint.line}`, - // Warn on adapter validation messages and unknown logpoint support + message: snapWarning || breakpoint.message || `${breakpoint.logMessage !== undefined ? 'Logpoint' : 'Breakpoint'} set at ${breakpoint.file}:${breakpoint.line}`, + // Warn on adapter validation messages, sync failures, snaps, + // and unknown logpoint support warning: warnings.length > 0 ? warnings.join('; ') : undefined, // Include context if available context: context || undefined @@ -1626,7 +1732,7 @@ export class DebugMcpServer { messages: [ { role: 'user' as const, - content: { type: 'text' as const, text: DEBUGGING_WORKFLOW_PROMPT } + content: { type: 'text' as const, text: buildDebuggingWorkflowPrompt(getBpAddressingMode(this.environment)) } } ] }; diff --git a/src/session/session-manager-operations.ts b/src/session/session-manager-operations.ts index 68845755..389cd331 100644 --- a/src/session/session-manager-operations.ts +++ b/src/session/session-manager-operations.ts @@ -943,12 +943,18 @@ export abstract class SessionManagerOperations extends SessionManagerData { async setBreakpoint( sessionId: string, - file: string, - line: number, - condition?: string, - suspendPolicy?: 'all' | 'thread', - logMessage?: string - ): Promise { + bp: { + /** Validated/translated by server.ts before reaching here */ + file: string; + /** Resolved line (anchors are resolved to a line in the server layer) */ + line: number; + condition?: string; + suspendPolicy?: 'all' | 'thread'; + logMessage?: string; + /** Set only in assert/content addressing modes (loud snapping, #271) */ + requestedLine?: number; + } + ): Promise<{ breakpoint: Breakpoint; warning?: string }> { const session = this._getSessionById(sessionId); // Check if session is terminated @@ -958,21 +964,31 @@ export abstract class SessionManagerOperations extends SessionManagerData { const bpId = uuidv4(); - // The file path has been validated and translated by server.ts before reaching here this.logger.info( - `[SessionManager setBreakpoint] Using validated file path "${file}" for session ${sessionId}` + `[SessionManager setBreakpoint] Using validated file path "${bp.file}" for session ${sessionId}` ); - const newBreakpoint: Breakpoint = { id: bpId, file, line, condition, suspendPolicy, logMessage, verified: false }; + const newBreakpoint: Breakpoint = { + id: bpId, + file: bp.file, + line: bp.line, + condition: bp.condition, + suspendPolicy: bp.suspendPolicy, + logMessage: bp.logMessage, + verified: false + }; + if (bp.requestedLine !== undefined) { + newBreakpoint.requestedLine = bp.requestedLine; + } if (!session.breakpoints) session.breakpoints = new Map(); session.breakpoints.set(bpId, newBreakpoint); this.logger.info( - `[SessionManager] Breakpoint ${bpId} queued for ${file}:${line} in session ${sessionId}.` + `[SessionManager] Breakpoint ${bpId} queued for ${bp.file}:${bp.line} in session ${sessionId}.` ); - await this.syncBreakpointsForFile(session, file); - return newBreakpoint; + const sync = await this.syncBreakpointsForFile(session, bp.file); + return { breakpoint: newBreakpoint, warning: sync.warning }; } /** diff --git a/src/skill-content.ts b/src/skill-content.ts index 2e7b0e84..9e3f07d1 100644 --- a/src/skill-content.ts +++ b/src/skill-content.ts @@ -6,9 +6,25 @@ * `debugging-workflow` prompt. Both are condensed views of the full agent * skill in skills/debugging/ — when editing one, check the other and the * skill for drift. + * + * Both texts are built per addressing mode (issue #271): the + * DEBUG_MCP_BP_ADDRESSING flag must hide content-addressing features from + * every surface a client can learn from, instructions included. */ +import { + BpAddressingMode, + DEFAULT_BP_ADDRESSING, + supportsExpectedContent +} from './utils/bp-addressing.js'; + +export function buildServerInstructions( + mode: BpAddressingMode = DEFAULT_BP_ADDRESSING +): string { + const expectedContentRule = supportsExpectedContent(mode) + ? `\n- set_breakpoint accepts expectedContent — pass the exact text of the target line (whitespace-trimmed); a mismatch fails fast and shows the actual nearby lines, catching off-by-one line numbers before they cause a confusing session. A response reporting "requested line N, bound to line M" means the adapter moved your breakpoint — trust the bound line.` + : ''; -export const SERVER_INSTRUCTIONS = `mcp-debugger drives real step-through debuggers (Python, JavaScript/TypeScript, Ruby, Rust, Go, Java, .NET) as MCP tools. + return `mcp-debugger drives real step-through debuggers (Python, JavaScript/TypeScript, Ruby, Rust, Go, Java, .NET) as MCP tools. Golden path: create_debug_session -> set_breakpoint (ABSOLUTE file path) -> start_debugging (ABSOLUTE scriptPath) -> get_stack_trace -> get_scopes(frameId from the stack frame's "id" field) -> get_variables / get_local_variables / evaluate_expression -> step_* or continue_execution -> get_output -> close_debug_session (always, even on failure). @@ -16,7 +32,7 @@ Key rules: - Stepping/evaluation/variable reads require the session to be PAUSED; the stop reason on each pause tells you why it stopped. - If a variable entry has a variablesReference, call get_variables with it to expand children. - Breakpoints may report unverified until the module/class loads — that is normal. -- list_breakpoints shows every breakpoint with its verified state; remove_breakpoint (by id or file+line) and clear_breakpoints take effect immediately, even mid-run — use them to move a bisection window without restarting. +- list_breakpoints shows every breakpoint with its verified state; remove_breakpoint (by id or file+line) and clear_breakpoints take effect immediately, even mid-run — use them to move a bisection window without restarting.${expectedContentRule} - Logpoints: set_breakpoint with logMessage ("order={orderId}") logs the interpolated message to get_output WITHOUT pausing — the prod-safe way to watch values on a hot path (Python/JS/Go/Rust; not Java/.NET). - restart_debugging {sessionId} relaunches with the same config in one call — breakpoints re-apply automatically, output buffer resets (read get_output from since=0). Works after the program exits; not for attach sessions. - get_output returns buffered debuggee stdout/stderr with a cursor; pass nextCursor back to read only new output. @@ -24,14 +40,24 @@ Key rules: - Launch sessions pause at uncaught exceptions by default (breakOnExceptions "uncaught"; Ruby excepted — rdbg has no uncaught filter). Pass "none" to let crashing scripts run to termination; attach applies no default. For the full debugging workflow (root-cause discipline, per-language quirks), request the "debugging-workflow" prompt or install the agent skill from skills/debugging/ in the repo.`; +} + +export const SERVER_INSTRUCTIONS = buildServerInstructions(); -export const DEBUGGING_WORKFLOW_PROMPT = `# Debugging workflow (mcp-debugger) +export function buildDebuggingWorkflowPrompt( + mode: BpAddressingMode = DEFAULT_BP_ADDRESSING +): string { + const expectedContentStep = supportsExpectedContent(mode) + ? ' — add expectedContent: "" so a stale or off-by-one line number fails fast instead of binding somewhere surprising' + : ''; + + return `# Debugging workflow (mcp-debugger) Prefer the debugger over print-debugging whenever you would need more than one edit-run cycle to see program state. ## Golden path (launch) 1. create_debug_session {language} -> sessionId -2. set_breakpoint {sessionId, file: ABSOLUTE path, line} +2. set_breakpoint {sessionId, file: ABSOLUTE path, line}${expectedContentStep} 3. start_debugging {sessionId, scriptPath: ABSOLUTE path} 4. get_stack_trace {sessionId} — use each frame's "id" field; it is adapter-assigned, never assume 0 5. get_scopes {sessionId, frameId} -> variablesReference per scope @@ -76,3 +102,6 @@ detach_from_process leaves the target running. - lastStop.description/text carry the exception class and message; where supported (Python/JS/Java/.NET), lastStop.exceptionInfo adds exceptionId/breakMode/details a moment after the pause. exitCode in list_debug_sessions distinguishes a crash (non-zero) from a clean exit. The full skill (with per-language reference files) lives in skills/debugging/ of the mcp-debugger repo.`; +} + +export const DEBUGGING_WORKFLOW_PROMPT = buildDebuggingWorkflowPrompt(); diff --git a/src/utils/bp-addressing.ts b/src/utils/bp-addressing.ts new file mode 100644 index 00000000..f92e3ac8 --- /dev/null +++ b/src/utils/bp-addressing.ts @@ -0,0 +1,44 @@ +/** + * Breakpoint addressing mode configuration (issue #271). + * + * DEBUG_MCP_BP_ADDRESSING restricts which breakpoint addressing features the + * server exposes — in runtime behavior AND in the set_breakpoint tool schema: + * - 'line': line numbers only (pre-#271 behavior) + * - 'assert': line + expectedContent assertion + loud snapping + * - 'content': assert + statement anchors (the default when unset) + * Modes are cumulative: line ⊂ assert ⊂ content. + */ + +export type BpAddressingMode = 'line' | 'assert' | 'content'; + +export const BP_ADDRESSING_ENV_KEY = 'DEBUG_MCP_BP_ADDRESSING'; + +export const DEFAULT_BP_ADDRESSING: BpAddressingMode = 'content'; + +const VALID_MODES: ReadonlySet = new Set(['line', 'assert', 'content']); + +/** + * Read the addressing mode from the environment. Unset, empty, or invalid + * values fall back to the shipped default ('content'). + */ +export function getBpAddressingMode(environment: { + get(key: string): string | undefined; +}): BpAddressingMode { + const raw = environment.get(BP_ADDRESSING_ENV_KEY)?.trim().toLowerCase(); + if (raw && VALID_MODES.has(raw)) { + return raw as BpAddressingMode; + } + return DEFAULT_BP_ADDRESSING; +} + +export function supportsExpectedContent(mode: BpAddressingMode): boolean { + return mode !== 'line'; +} + +export function supportsStatementAnchors(mode: BpAddressingMode): boolean { + return mode === 'content'; +} + +export function supportsLoudSnapping(mode: BpAddressingMode): boolean { + return mode !== 'line'; +} diff --git a/src/utils/breakpoint-resolver.ts b/src/utils/breakpoint-resolver.ts new file mode 100644 index 00000000..36d1e7d6 --- /dev/null +++ b/src/utils/breakpoint-resolver.ts @@ -0,0 +1,74 @@ +/** + * Pure content-addressing helpers for set_breakpoint (issue #271). + * + * All functions operate on a file's lines (1-based line numbers) and return + * result unions with fully formatted, agent-facing error messages. File I/O + * stays in the callers (server layer / session layer). + */ + +const CONTEXT_LINES = 2; + +export type LineContentAssertion = + | { ok: true } + | { ok: false; actual: string | null; message: string }; + +export interface AssertLineContentOptions { + /** Append the statement-addressing hint (content mode only). */ + statementHint?: boolean; +} + +/** + * Render a small window of numbered context lines around `line`, marking the + * target line with '>'. + */ +function formatContext(lines: string[], line: number): string { + const start = Math.max(1, line - CONTEXT_LINES); + const end = Math.min(lines.length, line + CONTEXT_LINES); + const width = String(end).length; + const rendered: string[] = []; + for (let n = start; n <= end; n++) { + const marker = n === line ? '>' : ' '; + rendered.push(`${marker} ${String(n).padStart(width)} | ${lines[n - 1]}`); + } + return rendered.join('\n'); +} + +/** + * Check that the trimmed content of `line` equals the trimmed expectation. + * On mismatch the message shows expected vs actual plus surrounding context so + * the agent can pick the correct line without another read. + */ +export function assertLineContent( + lines: string[], + line: number, + expectedContent: string, + filePath: string, + options?: AssertLineContentOptions +): LineContentAssertion { + if (line < 1 || line > lines.length) { + return { + ok: false, + actual: null, + message: + `Breakpoint not set: line ${line} of ${filePath} does not exist (file has ${lines.length} lines). ` + + `Re-read the file and pick a valid line.`, + }; + } + + const expected = expectedContent.trim(); + const actual = lines[line - 1].trim(); + if (actual === expected) { + return { ok: true }; + } + + let message = + `Breakpoint not set: line ${line} of ${filePath} does not match expectedContent.\n` + + `Expected: "${expected}"\n` + + `Actual: "${actual}"\n` + + `Context:\n${formatContext(lines, line)}\n` + + `The file may have changed since you last read it. Pick the correct line from the context above.`; + if (options?.statementHint) { + message += ` Or address by content: set_breakpoint {statement: "${expected}"}.`; + } + return { ok: false, actual, message }; +} diff --git a/src/utils/line-reader.ts b/src/utils/line-reader.ts index 24e858c4..cb9a7548 100644 --- a/src/utils/line-reader.ts +++ b/src/utils/line-reader.ts @@ -28,9 +28,18 @@ export interface LineReaderOptions { /** * Line reader with caching support */ +interface CachedFile { + lines: string[]; + mtimeMs: number; + size: number; +} + export class LineReader { - // LRU cache for recently read files (max 20 files, max age 5 minutes) - private fileCache = new LRUCache({ + // LRU cache for recently read files (max 20 files, max age 5 minutes). + // Entries are validated against the file's current mtime/size on every hit + // so agent edits invalidate immediately (breakpoint content assertions must + // never compare against a stale copy). + private fileCache = new LRUCache({ max: 20, ttl: 1000 * 60 * 5, // 5 minutes }); @@ -69,17 +78,22 @@ export class LineReader { * Read all lines from a file with caching */ private async readFileLines(filePath: string, options: LineReaderOptions): Promise { - // Check cache first const cacheKey = `${filePath}:${options.encoding || 'utf8'}`; - const cached = this.fileCache.get(cacheKey); - if (cached) { - this.logger?.debug(`[LineReader] Cache hit for: ${filePath}`); - return cached; - } try { - // Check file size const stats = await this.fileSystem.stat(filePath); + + // Serve from cache only while mtime and size still match + const cached = this.fileCache.get(cacheKey); + if (cached && cached.mtimeMs === stats.mtimeMs && cached.size === stats.size) { + this.logger?.debug(`[LineReader] Cache hit for: ${filePath}`); + return cached.lines; + } + if (cached) { + this.fileCache.delete(cacheKey); + this.logger?.debug(`[LineReader] Cache invalidated (file changed): ${filePath}`); + } + const maxSize = options.maxFileSize || 10 * 1024 * 1024; // 10MB default if (stats.size > maxSize) { this.logger?.debug(`[LineReader] File too large: ${filePath} (${stats.size} bytes)`); @@ -88,26 +102,26 @@ export class LineReader { // Read file content const content = await this.fileSystem.readFile(filePath, options.encoding || 'utf8'); - + // Check if binary if (this.isBinaryContent(content)) { this.logger?.debug(`[LineReader] Binary file detected: ${filePath}`); return null; } - + // Split into lines, preserving empty lines const lines = content.split(/\r?\n/); - + // Handle empty file case if (lines.length === 1 && lines[0] === '') { this.logger?.debug(`[LineReader] Empty file: ${filePath}`); return null; } - + // Cache the result - this.fileCache.set(cacheKey, lines); + this.fileCache.set(cacheKey, { lines, mtimeMs: stats.mtimeMs, size: stats.size }); this.logger?.debug(`[LineReader] Cached file: ${filePath} (${lines.length} lines)`); - + return lines; } catch (error) { this.logger?.debug(`[LineReader] Error reading file: ${filePath}`, { error }); @@ -115,6 +129,14 @@ export class LineReader { } } + /** + * Get all lines of a file (cached, mtime-validated). Returns null for + * binary, oversized, empty, or unreadable files. + */ + async getFileLines(filePath: string, options: LineReaderOptions = {}): Promise { + return this.readFileLines(filePath, options); + } + /** * Get line context for a specific line number */ diff --git a/tests/core/unit/server/server-bp-addressing-gating.test.ts b/tests/core/unit/server/server-bp-addressing-gating.test.ts new file mode 100644 index 00000000..056c0817 --- /dev/null +++ b/tests/core/unit/server/server-bp-addressing-gating.test.ts @@ -0,0 +1,139 @@ +/** + * DEBUG_MCP_BP_ADDRESSING gating tests (issue #271). + * + * The flag restricts breakpoint addressing features for A/B experiments; the + * restriction must hold in BOTH the tools/list schema and the call handler + * (schema omission alone doesn't stop a client that replays cached schemas), + * and in the server instructions text served in the initialize handshake. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies } from '../../../../src/container/dependencies.js'; +import { + createMockDependencies, + createMockServer, + createMockSessionManager, + createMockStdioTransport, + getToolHandlers +} from './server-test-helpers.js'; + +vi.mock('@modelcontextprotocol/sdk/server/index.js'); +vi.mock('@modelcontextprotocol/sdk/server/stdio.js'); +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); + +describe('DEBUG_MCP_BP_ADDRESSING gating (#271)', () => { + let mockServer: any; + let mockSessionManager: any; + let mockDependencies: any; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + + mockServer = createMockServer(); + vi.mocked(Server).mockImplementation(function() { return mockServer as any; }); + const mockStdioTransport = createMockStdioTransport(); + vi.mocked(StdioServerTransport).mockImplementation(function() { return mockStdioTransport as any; }); + + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + }); + + function buildServer() { + new DebugMcpServer(); + return getToolHandlers(mockServer); + } + + async function getSetBreakpointSchema(listToolsHandler: any) { + const { tools } = await listToolsHandler({ method: 'tools/list', params: {} }); + const tool = tools.find((t: { name: string }) => t.name === 'set_breakpoint'); + expect(tool).toBeDefined(); + return tool.inputSchema as { + properties: Record; + required: string[]; + }; + } + + it('omits expectedContent from the schema in line mode', async () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'line'); + const { listToolsHandler } = buildServer(); + + const schema = await getSetBreakpointSchema(listToolsHandler); + + expect(schema.properties.expectedContent).toBeUndefined(); + expect(schema.properties.statement).toBeUndefined(); + expect(schema.properties.nearLine).toBeUndefined(); + expect(schema.required).toEqual(['sessionId', 'file', 'line']); + }); + + it('exposes expectedContent in assert mode', async () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'assert'); + const { listToolsHandler } = buildServer(); + + const schema = await getSetBreakpointSchema(listToolsHandler); + + expect(schema.properties.expectedContent).toBeDefined(); + expect(schema.required).toContain('line'); + }); + + it('exposes expectedContent by default (env unset -> content mode)', async () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', undefined as unknown as string); + const { listToolsHandler } = buildServer(); + + const schema = await getSetBreakpointSchema(listToolsHandler); + + expect(schema.properties.expectedContent).toBeDefined(); + }); + + it('hard-errors an expectedContent argument in line mode, naming the env value', async () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'line'); + const { callToolHandler } = buildServer(); + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active' + }); + mockSessionManager.getSessionPolicy.mockReturnValue({}); + + await expect( + callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { + sessionId: 'test-session', + file: '/path/to/test.py', + line: 3, + expectedContent: 'anything' + } + } + }) + ).rejects.toThrow(/DEBUG_MCP_BP_ADDRESSING=line/); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('mentions expectedContent in server instructions only outside line mode', () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'line'); + new DebugMcpServer(); + const lineInstructions = (vi.mocked(Server).mock.calls.at(-1)![1] as { + instructions?: string; + }).instructions; + expect(lineInstructions).toBeDefined(); + expect(lineInstructions).not.toContain('expectedContent'); + + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'content'); + new DebugMcpServer(); + const contentInstructions = (vi.mocked(Server).mock.calls.at(-1)![1] as { + instructions?: string; + }).instructions; + expect(contentInstructions).toContain('expectedContent'); + }); +}); diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index d3c536f0..24a93cf8 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -64,7 +64,7 @@ describe('Server Control Tools Tests', () => { id: 'test-session', sessionLifecycle: 'ACTIVE' // Not terminated }); - mockSessionManager.setBreakpoint.mockResolvedValue(mockBreakpoint); + mockSessionManager.setBreakpoint.mockResolvedValue({ breakpoint: mockBreakpoint }); const result = await callToolHandler({ method: 'tools/call', @@ -80,11 +80,10 @@ describe('Server Control Tools Tests', () => { expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( 'test-session', - expect.stringContaining('/path/to/test.py'), - 10, - undefined, - undefined, - undefined + expect.objectContaining({ + file: expect.stringContaining('/path/to/test.py'), + line: 10 + }) ); const content = JSON.parse(result.content[0].text); @@ -107,7 +106,7 @@ describe('Server Control Tools Tests', () => { id: 'test-session', sessionLifecycle: 'ACTIVE' // Not terminated }); - mockSessionManager.setBreakpoint.mockResolvedValue(mockBreakpoint); + mockSessionManager.setBreakpoint.mockResolvedValue({ breakpoint: mockBreakpoint }); const result = await callToolHandler({ method: 'tools/call', @@ -124,11 +123,11 @@ describe('Server Control Tools Tests', () => { expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( 'test-session', - expect.stringContaining('/path/to/test.py'), - 20, - 'x > 10', - undefined, - undefined + expect.objectContaining({ + file: expect.stringContaining('/path/to/test.py'), + line: 20, + condition: 'x > 10' + }) ); }); @@ -145,7 +144,7 @@ describe('Server Control Tools Tests', () => { id: 'test-session', sessionLifecycle: 'ACTIVE' }); - mockSessionManager.setBreakpoint.mockResolvedValue(mockBreakpoint); + mockSessionManager.setBreakpoint.mockResolvedValue({ breakpoint: mockBreakpoint }); await callToolHandler({ method: 'tools/call', @@ -162,11 +161,11 @@ describe('Server Control Tools Tests', () => { expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( 'test-session', - expect.stringContaining('/path/to/test.py'), - 30, - undefined, - 'thread', - undefined + expect.objectContaining({ + file: expect.stringContaining('/path/to/test.py'), + line: 30, + suspendPolicy: 'thread' + }) ); }); diff --git a/tests/core/unit/server/server-expected-content.test.ts b/tests/core/unit/server/server-expected-content.test.ts new file mode 100644 index 00000000..72af4868 --- /dev/null +++ b/tests/core/unit/server/server-expected-content.test.ts @@ -0,0 +1,180 @@ +/** + * set_breakpoint expectedContent assertion tests (issue #271, phase 1). + * + * expectedContent is a set-time checksum on intent: the server hard-errors + * BEFORE storing anything when the target line's trimmed content does not + * match, and the error carries the actual nearby lines so the agent can + * correct in one step. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies } from '../../../../src/container/dependencies.js'; +import { + createMockDependencies, + createMockServer, + createMockSessionManager, + createMockStdioTransport, + getToolHandlers +} from './server-test-helpers.js'; + +vi.mock('@modelcontextprotocol/sdk/server/index.js'); +vi.mock('@modelcontextprotocol/sdk/server/stdio.js'); +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); + +const PY_FILE = [ + 'def total_cart():', + ' prices = load()', + ' total = sum(prices)', + ' return total', + '', + 'def main():' +].join('\n'); + +describe('set_breakpoint expectedContent (#271)', () => { + let mockServer: any; + let mockSessionManager: any; + let mockDependencies: any; + let callToolHandler: any; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + mockDependencies.fileSystem.readFile.mockResolvedValue(PY_FILE); + mockDependencies.fileSystem.stat.mockResolvedValue({ + isFile: () => true, + size: PY_FILE.length, + mtimeMs: 1000 + }); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + + mockServer = createMockServer(); + vi.mocked(Server).mockImplementation(function() { return mockServer as any; }); + const mockStdioTransport = createMockStdioTransport(); + vi.mocked(StdioServerTransport).mockImplementation(function() { return mockStdioTransport as any; }); + + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); + + new DebugMcpServer(); + callToolHandler = getToolHandlers(mockServer).callToolHandler; + + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active' + }); + mockSessionManager.getSessionPolicy.mockReturnValue({}); + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { + id: 'bp-1', + file: '/path/to/test.py', + line: 3, + verified: false + } + }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + }); + + function callSetBreakpoint(args: Record) { + return callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { + sessionId: 'test-session', + file: '/path/to/test.py', + ...args + } + } + }); + } + + it('sets the breakpoint when the line content matches', async () => { + const result = await callSetBreakpoint({ + line: 3, + expectedContent: 'total = sum(prices)' + }); + + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( + 'test-session', + expect.objectContaining({ line: 3 }) + ); + }); + + it('rejects with expected/actual and context when the content does not match', async () => { + let thrown: Error | undefined; + try { + await callSetBreakpoint({ line: 4, expectedContent: 'total = sum(prices)' }); + } catch (error) { + thrown = error as Error; + } + + expect(thrown).toBeDefined(); + expect(thrown!.message).toContain('does not match expectedContent'); + expect(thrown!.message).toContain('Expected: "total = sum(prices)"'); + expect(thrown!.message).toMatch(/Actual:\s+"return total"/); + expect(thrown!.message).toMatch(/>\s+4 \|/); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('rejects expectedContent for attach sessions (no host-readable file)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active', + attachMode: true + }); + + await expect( + callSetBreakpoint({ line: 3, expectedContent: 'total = sum(prices)' }) + ).rejects.toThrow(/line addressing/i); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('rejects expectedContent for non-file source identifiers (Java FQCN)', async () => { + mockSessionManager.getSessionPolicy.mockReturnValue({ + isNonFileSourceIdentifier: (f: string) => !f.includes('/') + }); + + await expect( + callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { + sessionId: 'test-session', + file: 'com.example.MyClass', + line: 3, + expectedContent: 'total = sum(prices)' + } + } + }) + ).rejects.toThrow(/line addressing/i); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('rejects clearly when the file cannot be read for verification', async () => { + mockDependencies.fileSystem.readFile.mockRejectedValue(new Error('EACCES')); + mockDependencies.fileSystem.stat.mockRejectedValue(new Error('EACCES')); + + await expect( + callSetBreakpoint({ line: 3, expectedContent: 'total = sum(prices)' }) + ).rejects.toThrow(/could not read/i); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + + it('applies no content check when expectedContent is absent', async () => { + const result = await callSetBreakpoint({ line: 4 }); + + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + expect(mockSessionManager.setBreakpoint).toHaveBeenCalled(); + }); +}); diff --git a/tests/core/unit/server/server-logpoint-gating.test.ts b/tests/core/unit/server/server-logpoint-gating.test.ts index 62c8a2ed..710ec797 100644 --- a/tests/core/unit/server/server-logpoint-gating.test.ts +++ b/tests/core/unit/server/server-logpoint-gating.test.ts @@ -53,11 +53,13 @@ describe('set_breakpoint logMessage gating', () => { sessionLifecycle: 'active' }); mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: '/path/to/test.py', - line: 10, - logMessage: 'x is {x}', - verified: false + breakpoint: { + id: 'bp-1', + file: '/path/to/test.py', + line: 10, + logMessage: 'x is {x}', + verified: false + } }); }); @@ -106,11 +108,11 @@ describe('set_breakpoint logMessage gating', () => { expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( 'test-session', - expect.stringContaining('/path/to/test.py'), - 10, - undefined, - undefined, - 'x is {x}' + expect.objectContaining({ + file: expect.stringContaining('/path/to/test.py'), + line: 10, + logMessage: 'x is {x}' + }) ); const content = JSON.parse(result.content[0].text); expect(content.success).toBe(true); @@ -131,7 +133,7 @@ describe('set_breakpoint logMessage gating', () => { it('applies no gating when logMessage is absent', async () => { mockSessionManager.getSessionPolicy.mockReturnValue({ name: 'java', supportsLogPoints: false }); mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-2', file: '/path/to/test.py', line: 10, verified: false + breakpoint: { id: 'bp-2', file: '/path/to/test.py', line: 10, verified: false } }); const result = await callToolHandler({ diff --git a/tests/core/unit/server/server-loud-snapping.test.ts b/tests/core/unit/server/server-loud-snapping.test.ts new file mode 100644 index 00000000..b9712957 --- /dev/null +++ b/tests/core/unit/server/server-loud-snapping.test.ts @@ -0,0 +1,156 @@ +/** + * Loud snapping tests (issue #271, phase 1). + * + * When the adapter binds a breakpoint to a different line than requested, the + * response must say so prominently instead of silently mutating the line. + * Line mode (the A/B control arm) must stay byte-identical to pre-#271 + * behavior: requestedLine is never sent to the session layer. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies } from '../../../../src/container/dependencies.js'; +import { + createMockDependencies, + createMockServer, + createMockSessionManager, + createMockStdioTransport, + getToolHandlers +} from './server-test-helpers.js'; + +vi.mock('@modelcontextprotocol/sdk/server/index.js'); +vi.mock('@modelcontextprotocol/sdk/server/stdio.js'); +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); + +const PY_FILE = [ + 'def total_cart():', // 1 + ' prices = load()', // 2 + '', // 3 (requested) + ' total = sum(prices)' // 4 (adapter binds here) +].join('\n'); + +describe('set_breakpoint loud snapping (#271)', () => { + let mockServer: any; + let mockSessionManager: any; + let mockDependencies: any; + let callToolHandler: any; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + mockDependencies.fileSystem.readFile.mockResolvedValue(PY_FILE); + mockDependencies.fileSystem.stat.mockResolvedValue({ + isFile: () => true, + size: PY_FILE.length, + mtimeMs: 1000 + }); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + + mockServer = createMockServer(); + vi.mocked(Server).mockImplementation(function() { return mockServer as any; }); + const mockStdioTransport = createMockStdioTransport(); + vi.mocked(StdioServerTransport).mockImplementation(function() { return mockStdioTransport as any; }); + + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); + + new DebugMcpServer(); + callToolHandler = getToolHandlers(mockServer).callToolHandler; + + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: 'active' + }); + mockSessionManager.getSessionPolicy.mockReturnValue({}); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + }); + + function callSetBreakpoint(args: Record = {}) { + return callToolHandler({ + method: 'tools/call', + params: { + name: 'set_breakpoint', + arguments: { + sessionId: 'test-session', + file: '/path/to/test.py', + line: 3, + ...args + } + } + }); + } + + it('passes requestedLine to the session layer by default (content mode)', async () => { + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 3, requestedLine: 3, verified: true } + }); + + await callSetBreakpoint(); + + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( + 'test-session', + expect.objectContaining({ line: 3, requestedLine: 3 }) + ); + }); + + it('omits requestedLine in line mode (control arm purity)', async () => { + vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'line'); + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 3, verified: true } + }); + + await callSetBreakpoint(); + + const options = mockSessionManager.setBreakpoint.mock.calls[0][1]; + expect('requestedLine' in options).toBe(false); + }); + + it('reports a snap prominently in message, warning, and requestedLine', async () => { + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 4, requestedLine: 3, verified: true } + }); + + const result = await callSetBreakpoint(); + const content = JSON.parse(result.content[0].text); + + expect(content.success).toBe(true); + expect(content.line).toBe(4); + expect(content.requestedLine).toBe(3); + expect(content.message).toContain('requested line 3, bound to line 4'); + expect(content.warning).toContain('requested line 3, bound to line 4'); + // bound-line content is echoed for orientation + expect(content.message).toContain('total = sum(prices)'); + }); + + it('emits no snap warning when the adapter honors the requested line', async () => { + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 3, requestedLine: 3, verified: true } + }); + + const result = await callSetBreakpoint(); + const content = JSON.parse(result.content[0].text); + + expect(content.success).toBe(true); + expect(content.warning).toBeUndefined(); + expect(content.message).not.toContain('bound to'); + }); + + it('surfaces the session-layer sync warning in the response', async () => { + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 3, requestedLine: 3, verified: false }, + warning: 'Breakpoint state updated, but live sync failed: adapter exploded' + }); + + const result = await callSetBreakpoint(); + const content = JSON.parse(result.content[0].text); + + expect(content.success).toBe(true); + expect(content.warning).toContain('adapter exploded'); + }); +}); diff --git a/tests/core/unit/server/server-redefine-and-attach.test.ts b/tests/core/unit/server/server-redefine-and-attach.test.ts index bc0fecd3..8e656865 100644 --- a/tests/core/unit/server/server-redefine-and-attach.test.ts +++ b/tests/core/unit/server/server-redefine-and-attach.test.ts @@ -62,10 +62,12 @@ describe('redefine_classes and attach stopOnEntry tests', () => { }); mockSessionManager.getSessionPolicy.mockReturnValue({}); mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: '/app/app.rb', - line: 18, - verified: true + breakpoint: { + id: 'bp-1', + file: '/app/app.rb', + line: 18, + verified: true + } }); const result = await callToolHandler({ @@ -79,7 +81,7 @@ describe('redefine_classes and attach stopOnEntry tests', () => { const response = JSON.parse(result.content[0].text); expect(response.success).toBe(true); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'attach-session', '/app/app.rb', 18, undefined, undefined, undefined + 'attach-session', expect.objectContaining({ file: '/app/app.rb', line: 18 }) ); }); }); diff --git a/tests/core/unit/session/session-manager-bp-addressing.test.ts b/tests/core/unit/session/session-manager-bp-addressing.test.ts new file mode 100644 index 00000000..cc7a5e10 --- /dev/null +++ b/tests/core/unit/session/session-manager-bp-addressing.test.ts @@ -0,0 +1,117 @@ +/** + * Issue #271 — SessionManager layer for content-addressed breakpoints: + * options-object setBreakpoint contract, requestedLine bookkeeping (loud + * snapping), and sync-warning propagation. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { SessionManager, SessionManagerConfig } from '../../../../src/session/session-manager.js'; +import { DebugLanguage } from '@debugmcp/shared'; +import { createMockDependencies } from './session-manager-test-utils.js'; + +describe('SessionManager - breakpoint addressing (#271)', () => { + let sessionManager: SessionManager; + let dependencies: ReturnType; + let config: SessionManagerConfig; + + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + dependencies = createMockDependencies(); + config = { + logDirBase: '/tmp/test-sessions', + defaultDapLaunchArgs: { stopOnEntry: true, justMyCode: true } + }; + sessionManager = new SessionManager(config, dependencies); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); + dependencies.mockProxyManager.reset(); + }); + + async function createLaunchedSession(script = 'test.py') { + const session = await sessionManager.createSession({ + language: DebugLanguage.MOCK, + executablePath: 'python' + }); + await sessionManager.startDebugging(session.id, script); + await vi.runAllTimersAsync(); + return session; + } + + it('accepts an options object and returns { breakpoint }', async () => { + const session = await createLaunchedSession(); + + const result = await sessionManager.setBreakpoint(session.id, { + file: 'test.py', + line: 10, + condition: 'x > 1' + }); + + expect(result.breakpoint.file).toBe('test.py'); + expect(result.breakpoint.line).toBe(10); + expect(result.breakpoint.condition).toBe('x > 1'); + expect(result.breakpoint.verified).toBe(true); + expect(result.warning).toBeUndefined(); + }); + + it('stores requestedLine and reflects the adapter-bound line after a snap', async () => { + const session = await createLaunchedSession(); + dependencies.mockProxyManager.setDapRequestHandler(async (command, args) => { + if (command === 'setBreakpoints') { + return { + success: true, + body: { + breakpoints: (args?.breakpoints ?? []).map((bp: { line: number }) => ({ + verified: true, + line: bp.line + 1 + })) + } + }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.setBreakpoint(session.id, { + file: 'test.py', + line: 12, + requestedLine: 12 + }); + + expect(result.breakpoint.line).toBe(13); + expect(result.breakpoint.requestedLine).toBe(12); + const [stored] = sessionManager.listBreakpoints(session.id); + expect(stored.line).toBe(13); + expect(stored.requestedLine).toBe(12); + }); + + it('does not record requestedLine when the caller omits it (line mode purity)', async () => { + const session = await createLaunchedSession(); + + const result = await sessionManager.setBreakpoint(session.id, { + file: 'test.py', + line: 10 + }); + + expect(result.breakpoint.requestedLine).toBeUndefined(); + expect('requestedLine' in result.breakpoint).toBe(false); + }); + + it('propagates the sync warning when the adapter rejects setBreakpoints', async () => { + const session = await createLaunchedSession(); + dependencies.mockProxyManager.setDapRequestHandler(async (command) => { + if (command === 'setBreakpoints') { + throw new Error('adapter exploded'); + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.setBreakpoint(session.id, { + file: 'test.py', + line: 10 + }); + + expect(result.breakpoint).toBeDefined(); + expect(result.warning).toContain('adapter exploded'); + }); +}); diff --git a/tests/core/unit/session/session-manager-dap.test.ts b/tests/core/unit/session/session-manager-dap.test.ts index 59e05b78..333c8bb3 100644 --- a/tests/core/unit/session/session-manager-dap.test.ts +++ b/tests/core/unit/session/session-manager-dap.test.ts @@ -58,8 +58,8 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - const bp1 = await sessionManager.setBreakpoint(session.id, 'test.py', 10); - const bp2 = await sessionManager.setBreakpoint(session.id, 'test.py', 20); + const { breakpoint: bp1 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); + const { breakpoint: bp2 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); expect(bp1.verified).toBe(false); expect(bp2.verified).toBe(false); @@ -82,7 +82,7 @@ describe('SessionManager - DAP Operations', () => { dependencies.mockProxyManager.dapRequestCalls = []; // Set breakpoint on active session - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 15); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 15 }); // Should be verified immediately expect(bp.verified).toBe(true); @@ -106,11 +106,9 @@ describe('SessionManager - DAP Operations', () => { dependencies.mockProxyManager.dapRequestCalls = []; - const bp = await sessionManager.setBreakpoint( - session.id, - 'test.py', - 25, - 'x > 10' + const { breakpoint: bp } = await sessionManager.setBreakpoint( + session.id, + { file: 'test.py', line: 25, condition: 'x > 10' } ); expect(bp.condition).toBe('x > 10'); @@ -145,7 +143,7 @@ describe('SessionManager - DAP Operations', () => { return { success: true }; }); - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 15); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 15 }); expect(bp.verified).toBe(true); expect(bp.adapterId).toBe(55); @@ -162,8 +160,8 @@ describe('SessionManager - DAP Operations', () => { await vi.runAllTimersAsync(); dependencies.mockProxyManager.dapRequestCalls = []; - const bp = await sessionManager.setBreakpoint( - session.id, 'test.py', 20, undefined, undefined, 'value is {x}' + const { breakpoint: bp } = await sessionManager.setBreakpoint( + session.id, { file: 'test.py', line: 20, logMessage: 'value is {x}' } ); expect(bp.logMessage).toBe('value is {x}'); @@ -182,7 +180,8 @@ describe('SessionManager - DAP Operations', () => { }); await sessionManager.setBreakpoint( - session.id, 'test.py', 20, 'x > 1', 'thread', 'value is {x}' + session.id, + { file: 'test.py', line: 20, condition: 'x > 1', suspendPolicy: 'thread', logMessage: 'value is {x}' } ); await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); @@ -209,7 +208,7 @@ describe('SessionManager - DAP Operations', () => { dependencies.mockProxyManager.dapRequestCalls = []; await sessionManager.setBreakpoint( - session.id, 'test.py', 21, 'x > 5', undefined, 'big x: {x}' + session.id, { file: 'test.py', line: 21, condition: 'x > 5', logMessage: 'big x: {x}' } ); expect(dependencies.mockProxyManager.dapRequestCalls[0].args.breakpoints[0]).toMatchObject({ @@ -231,7 +230,7 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); expect(bp.verified).toBe(false); await sessionManager.startDebugging(session.id, 'test.py'); @@ -249,9 +248,9 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - await sessionManager.setBreakpoint(session.id, 'b.py', 20); - await sessionManager.setBreakpoint(session.id, 'a.py', 30); - await sessionManager.setBreakpoint(session.id, 'a.py', 10); + await sessionManager.setBreakpoint(session.id, { file: 'b.py', line: 20 }); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 30 }); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 10 }); const breakpoints = sessionManager.listBreakpoints(session.id); @@ -270,8 +269,8 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - await sessionManager.setBreakpoint(session.id, 'a.py', 10); - await sessionManager.setBreakpoint(session.id, 'b.py', 20); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'b.py', line: 20 }); const breakpoints = sessionManager.listBreakpoints(session.id, 'b.py'); @@ -287,8 +286,8 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - const bp1 = await sessionManager.setBreakpoint(session.id, 'test.py', 10); - await sessionManager.setBreakpoint(session.id, 'test.py', 20); + const { breakpoint: bp1 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); const result = await sessionManager.removeBreakpoint(session.id, bp1.id); @@ -305,8 +304,8 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - const bp1 = await sessionManager.setBreakpoint(session.id, 'test.py', 10); - await sessionManager.setBreakpoint(session.id, 'test.py', 20); + const { breakpoint: bp1 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); dependencies.mockProxyManager.dapRequestCalls = []; const result = await sessionManager.removeBreakpoint(session.id, bp1.id); @@ -329,7 +328,7 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); dependencies.mockProxyManager.dapRequestCalls = []; await sessionManager.removeBreakpoint(session.id, bp.id); @@ -360,9 +359,9 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - await sessionManager.setBreakpoint(session.id, 'test.py', 10, 'x > 1'); - await sessionManager.setBreakpoint(session.id, 'test.py', 10, 'x > 2'); - await sessionManager.setBreakpoint(session.id, 'test.py', 20); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10, condition: 'x > 1' }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10, condition: 'x > 2' }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); const result = await sessionManager.removeBreakpointsByLocation(session.id, 'test.py', 10); @@ -376,7 +375,7 @@ describe('SessionManager - DAP Operations', () => { executablePath: 'python' }); - await sessionManager.setBreakpoint(session.id, 'test.py', 10); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); const result = await sessionManager.removeBreakpointsByLocation(session.id, 'test.py', 99); @@ -394,9 +393,9 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - await sessionManager.setBreakpoint(session.id, 'a.py', 10); - await sessionManager.setBreakpoint(session.id, 'a.py', 20); - await sessionManager.setBreakpoint(session.id, 'b.py', 30); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 20 }); + await sessionManager.setBreakpoint(session.id, { file: 'b.py', line: 30 }); dependencies.mockProxyManager.dapRequestCalls = []; const result = await sessionManager.clearBreakpoints(session.id, 'a.py'); @@ -422,8 +421,8 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - await sessionManager.setBreakpoint(session.id, 'a.py', 10); - await sessionManager.setBreakpoint(session.id, 'b.py', 20); + await sessionManager.setBreakpoint(session.id, { file: 'a.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'b.py', line: 20 }); dependencies.mockProxyManager.dapRequestCalls = []; const result = await sessionManager.clearBreakpoints(session.id); @@ -465,8 +464,8 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - const bp1 = await sessionManager.setBreakpoint(session.id, 'test.py', 10); - await sessionManager.setBreakpoint(session.id, 'test.py', 20); + const { breakpoint: bp1 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); dependencies.mockProxyManager.simulateEvent('terminated'); await vi.runAllTimersAsync(); @@ -492,8 +491,8 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'test.py'); await vi.runAllTimersAsync(); - const bp1 = await sessionManager.setBreakpoint(session.id, 'test.py', 10); - await sessionManager.setBreakpoint(session.id, 'test.py', 20); + const { breakpoint: bp1 } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 20 }); dependencies.mockProxyManager.shouldFailDapRequests = true; const result = await sessionManager.removeBreakpoint(session.id, bp1.id); @@ -529,7 +528,7 @@ describe('SessionManager - DAP Operations', () => { return { success: true }; }); - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); expect(bp.verified).toBe(false); return { session, bp }; } @@ -582,7 +581,7 @@ describe('SessionManager - DAP Operations', () => { } return { success: true }; }); - const bp = await sessionManager.setBreakpoint(session.id, 'C:\\proj\\app.js', 9); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'C:\\proj\\app.js', line: 9 }); expect(bp.verified).toBe(false); dependencies.mockProxyManager.simulateEvent('breakpoint', { @@ -644,7 +643,7 @@ describe('SessionManager - DAP Operations', () => { await sessionManager.startDebugging(session.id, 'app.js'); await vi.runAllTimersAsync(); - const bp = await sessionManager.setBreakpoint(session.id, 'app.js', 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'app.js', line: 10 }); expect(bp.verified).toBe(false); // Parent response ids must not be adopted for mirroring policies expect(bp.adapterId).toBeUndefined(); @@ -666,7 +665,7 @@ describe('SessionManager - DAP Operations', () => { // Adding a second breakpoint re-syncs the whole file; the parent // response reports verified:false for both positions - await sessionManager.setBreakpoint(session.id, 'app.js', 20); + await sessionManager.setBreakpoint(session.id, { file: 'app.js', line: 20 }); const stored = sessionManager.listBreakpoints(session.id); const bp1 = stored.find(bp => bp.line === 10)!; @@ -1321,7 +1320,7 @@ describe('SessionManager - DAP Operations', () => { it('keeps a stop matching a user breakpoint adapter id as breakpoint', async () => { const session = await createPausedRustSession(); respondToSetBreakpointsWithIds(1); - await sessionManager.setBreakpoint(session.id, 'main.rs', 17); + await sessionManager.setBreakpoint(session.id, { file: 'main.rs', line: 17 }); dependencies.mockProxyManager.simulateStopped(1, 'breakpoint', { reason: 'breakpoint', @@ -1338,7 +1337,7 @@ describe('SessionManager - DAP Operations', () => { it('normalizes a panic stop whose id is disjoint from user breakpoints', async () => { const session = await createPausedRustSession(); respondToSetBreakpointsWithIds(1); - await sessionManager.setBreakpoint(session.id, 'main.rs', 17); + await sessionManager.setBreakpoint(session.id, { file: 'main.rs', line: 17 }); dependencies.mockProxyManager.simulateStopped(1, 'breakpoint', { reason: 'breakpoint', @@ -1380,7 +1379,7 @@ describe('SessionManager - DAP Operations', () => { const session = await createPausedRustSession(); // Default mock setBreakpoints response carries no ids -> bookkeeping // incomplete -> the disjoint inference must be disabled. - await sessionManager.setBreakpoint(session.id, 'main.rs', 17); + await sessionManager.setBreakpoint(session.id, { file: 'main.rs', line: 17 }); dependencies.mockProxyManager.simulateStopped(1, 'breakpoint', { reason: 'breakpoint', @@ -1419,8 +1418,8 @@ describe('SessionManager - DAP Operations', () => { it('annotates stored logpoints when live capabilities do not advertise logpoint support (issue #235)', async () => { const session = await createPausedSession(); - const bp = await sessionManager.setBreakpoint( - session.id, 'test.py', 20, undefined, undefined, 'x is {x}' + const { breakpoint: bp } = await sessionManager.setBreakpoint( + session.id, { file: 'test.py', line: 20, logMessage: 'x is {x}' } ); dependencies.mockProxyManager.simulateEvent('adapter-capabilities', { supportsLogPoints: false }); @@ -1433,7 +1432,7 @@ describe('SessionManager - DAP Operations', () => { it('leaves logpoints unannotated when live capabilities advertise support', async () => { const session = await createPausedSession(); await sessionManager.setBreakpoint( - session.id, 'test.py', 20, undefined, undefined, 'x is {x}' + session.id, { file: 'test.py', line: 20, logMessage: 'x is {x}' } ); dependencies.mockProxyManager.simulateEvent('adapter-capabilities', { supportsLogPoints: true }); diff --git a/tests/core/unit/session/session-manager-paths.test.ts b/tests/core/unit/session/session-manager-paths.test.ts index 9c93bc52..42bd75d2 100644 --- a/tests/core/unit/session/session-manager-paths.test.ts +++ b/tests/core/unit/session/session-manager-paths.test.ts @@ -45,7 +45,7 @@ describe('SessionManager - Path Resolution', () => { ]; for (const { path: testPath, expectedFile } of windowsPaths) { - const bp = await sessionManager.setBreakpoint(session.id, testPath, 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: testPath, line: 10 }); // SessionManager passes through paths without modification // So the breakpoint file should match the input path @@ -60,10 +60,9 @@ describe('SessionManager - Path Resolution', () => { pythonPath: 'python' }); - const bp = await sessionManager.setBreakpoint( + const { breakpoint: bp } = await sessionManager.setBreakpoint( session.id, - 'src\\debug\\file.py', - 20 + { file: 'src\\debug\\file.py', line: 20 } ); // Check that path contains expected components @@ -79,10 +78,9 @@ describe('SessionManager - Path Resolution', () => { }); const testPath = 'test/file.py'; - const bp = await sessionManager.setBreakpoint( + const { breakpoint: bp } = await sessionManager.setBreakpoint( session.id, - testPath, - 30 + { file: testPath, line: 30 } ); // SessionManager should pass through the path as-is @@ -99,7 +97,7 @@ describe('SessionManager - Path Resolution', () => { }); const relativePath = 'src/test.py'; - const bp = await sessionManager.setBreakpoint(session.id, relativePath, 42); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: relativePath, line: 42 }); // SessionManager no longer converts paths - just passes through expect(bp.file).toBe(relativePath); @@ -112,7 +110,7 @@ describe('SessionManager - Path Resolution', () => { }); const absolutePath = '/home/user/project/test.py'; - const bp = await sessionManager.setBreakpoint(session.id, absolutePath, 50); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: absolutePath, line: 50 }); // SessionManager passes through paths without normalization expect(bp.file).toBe(absolutePath); @@ -126,7 +124,7 @@ describe('SessionManager - Path Resolution', () => { // Mix of path separators const mixedPath = 'src\\components/test.py'; - const bp = await sessionManager.setBreakpoint(session.id, mixedPath, 60); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: mixedPath, line: 60 }); // Should contain expected path components expect(bp.file.toLowerCase()).toContain('src'); diff --git a/tests/core/unit/session/session-manager-restart.test.ts b/tests/core/unit/session/session-manager-restart.test.ts index 5f23b792..294a1970 100644 --- a/tests/core/unit/session/session-manager-restart.test.ts +++ b/tests/core/unit/session/session-manager-restart.test.ts @@ -63,7 +63,7 @@ describe('SessionManager - restart and relaunch', () => { describe('per-launch breakpoint state reset', () => { it('clears stale verified state when a new launch begins', async () => { const session = await createLaunchedSession(); - const bp = await sessionManager.setBreakpoint(session.id, 'test.py', 10); + const { breakpoint: bp } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); expect(bp.verified).toBe(true); // mock default verifies live sets dependencies.mockProxyManager.simulateEvent('terminated'); @@ -88,7 +88,7 @@ describe('SessionManager - restart and relaunch', () => { language: DebugLanguage.MOCK, executablePath: 'python' }); - await sessionManager.setBreakpoint(session.id, 'test.py', 10); + await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 10 }); await sessionManager.startDebugging( session.id, 'test.py', ['--flag'], { stopOnEntry: true }, undefined, { custom: 'cfg' } ); diff --git a/tests/core/unit/session/session-manager-workflow.test.ts b/tests/core/unit/session/session-manager-workflow.test.ts index 16a1a2ee..91d4bac1 100644 --- a/tests/core/unit/session/session-manager-workflow.test.ts +++ b/tests/core/unit/session/session-manager-workflow.test.ts @@ -62,7 +62,7 @@ describe('SessionManager - Debug Session Workflow', () => { expect(startResult.state).toBe(SessionState.PAUSED); // Set a breakpoint - const breakpoint = await sessionManager.setBreakpoint(session.id, 'test.py', 15); + const { breakpoint } = await sessionManager.setBreakpoint(session.id, { file: 'test.py', line: 15 }); expect(breakpoint.verified).toBe(true); expect(dependencies.mockProxyManager.dapRequestCalls).toContainEqual({ command: 'setBreakpoints', diff --git a/tests/e2e/mcp-server-bp-addressing.test.ts b/tests/e2e/mcp-server-bp-addressing.test.ts new file mode 100644 index 00000000..1f8c36bc --- /dev/null +++ b/tests/e2e/mcp-server-bp-addressing.test.ts @@ -0,0 +1,161 @@ +/** + * E2E: agent-native breakpoint addressing (issue #271, phase 1) against the + * mock adapter. + * + * - expectedContent: a mismatched assertion fails fast with expected/actual + * and context; a matching one sets the breakpoint and echoes content. + * - Loud snapping: the mock adapter's `snap` fixture binds odd lines one line + * down; the response must report requested vs bound loudly. The + * `snap-event` fixture verifies asynchronously via a DAP breakpoint event; + * the relocation must surface in list_breakpoints with requestedLine + * preserved. + */ +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import path from 'path'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; +import { parseSdkToolResult, callToolSafely } from './smoke-test-utils.js'; +import { ROOT } from './language-matrix-utils.js'; + +const SNAP_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'mock-snap-fixture.py'); +const SNAP_EVENT_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'mock-snap-event-fixture.py'); + +describe('Breakpoint addressing e2e (#271, mock adapter)', () => { + let mcpClient: Client | null = null; + let transport: StdioClientTransport | null = null; + let currentSessionId: string | null = null; + + beforeAll(async () => { + transport = new StdioClientTransport({ + command: process.execPath, + args: [path.join(ROOT, 'dist', 'index.js'), '--log-level', 'info'], + env: { ...process.env, NODE_ENV: 'test' }, + }); + mcpClient = new Client( + { name: 'bp-addressing-test-client', version: '1.0.0' }, + { capabilities: {} }, + ); + await mcpClient.connect(transport); + }, 120_000); + + afterAll(async () => { + if (mcpClient) { + await mcpClient.close().catch(() => {}); + } + transport = null; + }); + + afterEach(async () => { + if (mcpClient && currentSessionId) { + await callToolSafely(mcpClient, 'close_debug_session', { sessionId: currentSessionId }); + currentSessionId = null; + } + }); + + async function createMockSession(name: string): Promise { + const createRes = await mcpClient!.callTool({ + name: 'create_debug_session', + arguments: { language: 'mock', name }, + }); + const sessionId = parseSdkToolResult(createRes).sessionId as string; + expect(sessionId).toBeTruthy(); + currentSessionId = sessionId; + return sessionId; + } + + it('rejects a mismatched expectedContent with expected/actual and context', async () => { + const sessionId = await createMockSession('expected-content-mismatch'); + + const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { + sessionId, + file: SNAP_SCRIPT, + line: 5, + expectedContent: 'return total', + }); + + expect(bpRes.success).toBe(false); + const text = JSON.stringify(bpRes); + expect(text).toContain('does not match expectedContent'); + expect(text).toContain('total = c * 2'); + + const listRes = await callToolSafely(mcpClient!, 'list_breakpoints', { sessionId }); + expect((listRes as { breakpoints?: unknown[] }).breakpoints).toHaveLength(0); + }); + + it('sets the breakpoint and echoes content when expectedContent matches', async () => { + const sessionId = await createMockSession('expected-content-match'); + + const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { + sessionId, + file: SNAP_SCRIPT, + line: 5, + expectedContent: 'total = c * 2', + }); + + expect(bpRes.success).toBe(true); + expect((bpRes as { line?: number }).line).toBe(5); + expect(String((bpRes as { content?: string }).content)).toContain('total = c * 2'); + }, 30_000); + + it('reports a snap loudly when the adapter binds a different line', async () => { + const sessionId = await createMockSession('loud-snap'); + + const startRes = await callToolSafely(mcpClient!, 'start_debugging', { + sessionId, + scriptPath: SNAP_SCRIPT, + dapLaunchArgs: { stopOnEntry: true }, + }); + expect(startRes.success).toBe(true); + + const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { + sessionId, + file: SNAP_SCRIPT, + line: 5, + }); + + expect(bpRes.success).toBe(true); + expect((bpRes as { line?: number }).line).toBe(6); + expect((bpRes as { requestedLine?: number }).requestedLine).toBe(5); + expect(String((bpRes as { warning?: string }).warning)).toMatch( + /requested line 5, bound to line 6/ + ); + expect(String((bpRes as { message?: string }).message)).toMatch( + /requested line 5, bound to line 6/ + ); + }, 30_000); + + it('surfaces an async breakpoint-event relocation in list_breakpoints', async () => { + const sessionId = await createMockSession('snap-event'); + + const startRes = await callToolSafely(mcpClient!, 'start_debugging', { + sessionId, + scriptPath: SNAP_EVENT_SCRIPT, + dapLaunchArgs: { stopOnEntry: true }, + }); + expect(startRes.success).toBe(true); + + const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { + sessionId, + file: SNAP_EVENT_SCRIPT, + line: 5, + }); + expect(bpRes.success).toBe(true); + // The synchronous response still shows the requested line, unverified — + // relocation arrives via the breakpoint event. + expect((bpRes as { verified?: boolean }).verified).toBe(false); + + // Poll list_breakpoints until the event lands. + const deadline = Date.now() + 10_000; + let bp: { line?: number; requestedLine?: number; verified?: boolean } | undefined; + while (Date.now() < deadline) { + const listRes = await callToolSafely(mcpClient!, 'list_breakpoints', { sessionId }); + bp = ((listRes as { breakpoints?: Array }).breakpoints ?? [])[0]; + if (bp?.verified && bp.line === 6) break; + await new Promise(resolve => setTimeout(resolve, 200)); + } + + expect(bp?.verified).toBe(true); + expect(bp?.line).toBe(6); + expect(bp?.requestedLine).toBe(5); + }, 30_000); +}); diff --git a/tests/fixtures/debug-scripts/mock-snap-event-fixture.py b/tests/fixtures/debug-scripts/mock-snap-event-fixture.py new file mode 100644 index 00000000..de628a4c --- /dev/null +++ b/tests/fixtures/debug-scripts/mock-snap-event-fixture.py @@ -0,0 +1,8 @@ +def compute(): + a = 1 + b = 2 + c = a + b + total = c * 2 + return total + +print(compute()) diff --git a/tests/fixtures/debug-scripts/mock-snap-fixture.py b/tests/fixtures/debug-scripts/mock-snap-fixture.py new file mode 100644 index 00000000..de628a4c --- /dev/null +++ b/tests/fixtures/debug-scripts/mock-snap-fixture.py @@ -0,0 +1,8 @@ +def compute(): + a = 1 + b = 2 + c = a + b + total = c * 2 + return total + +print(compute()) diff --git a/tests/integration/rust/rust-integration.test.ts b/tests/integration/rust/rust-integration.test.ts index 801e852f..6e65407a 100644 --- a/tests/integration/rust/rust-integration.test.ts +++ b/tests/integration/rust/rust-integration.test.ts @@ -61,11 +61,10 @@ describe('Rust Adapter Integration', () => { // Skip if test file doesn't exist try { - const breakpoint = await sessionManager.setBreakpoint( - sessionId, - testFile, - 5 // Line number in main function - ); + const { breakpoint } = await sessionManager.setBreakpoint(sessionId, { + file: testFile, + line: 5 // Line number in main function + }); expect(breakpoint).toBeDefined(); // Breakpoint may not be verified without a running debug session with a compiled binary diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index 6df14db2..b76fe8e9 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -72,7 +72,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { it('should handle session not found error', async () => { mockSessionManager.getSession.mockReturnValue(null); - await expect(server.setBreakpoint('invalid-session', 'test.py', 10)) + await expect(server.setBreakpoint({ sessionId: 'invalid-session', file: 'test.py', line: 10 })) .rejects.toThrow(McpError); }); @@ -308,7 +308,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }) }; - await expect(server.setBreakpoint('test-session', '/nonexistent/file.py', 10)) + await expect(server.setBreakpoint({ sessionId: 'test-session', file: '/nonexistent/file.py', line: 10 })) .rejects.toThrow('Breakpoint file not found'); }); @@ -327,7 +327,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { mockSessionManager.setBreakpoint.mockRejectedValue(new Error('Invalid line number')); - await expect(server.setBreakpoint('test-session', '/path/to/file.py', -1)) + await expect(server.setBreakpoint({ sessionId: 'test-session', file: '/path/to/file.py', line: -1 })) .rejects.toThrow('Invalid line number'); }); @@ -349,17 +349,25 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { (server as any).fileChecker = mockFileChecker; mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: 'com.example.MyClass', - line: 42, - verified: true + breakpoint: { + id: 'bp-1', + file: 'com.example.MyClass', + line: 42, + verified: true + } }); - const result = await server.setBreakpoint('test-session', 'com.example.MyClass', 42); + const { breakpoint: result } = await server.setBreakpoint({ sessionId: 'test-session', file: 'com.example.MyClass', line: 42 }); expect(result.verified).toBe(true); expect(mockFileChecker.checkExists).not.toHaveBeenCalled(); - expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith('test-session', 'com.example.MyClass', 42, undefined, undefined, undefined); + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith('test-session', expect.objectContaining({ + file: 'com.example.MyClass', + line: 42, + condition: undefined, + suspendPolicy: undefined, + logMessage: undefined + })); }); it('should skip file existence check for inner class notation via policy', async () => { @@ -379,13 +387,15 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { (server as any).fileChecker = mockFileChecker; mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: 'com.example.Outer$Inner', - line: 10, - verified: true + breakpoint: { + id: 'bp-1', + file: 'com.example.Outer$Inner', + line: 10, + verified: true + } }); - const result = await server.setBreakpoint('test-session', 'com.example.Outer$Inner', 10); + const { breakpoint: result } = await server.setBreakpoint({ sessionId: 'test-session', file: 'com.example.Outer$Inner', line: 10 }); expect(result.verified).toBe(true); expect(mockFileChecker.checkExists).not.toHaveBeenCalled(); @@ -408,13 +418,15 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { (server as any).fileChecker = mockFileChecker; mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: 'MyClass', - line: 5, - verified: true + breakpoint: { + id: 'bp-1', + file: 'MyClass', + line: 5, + verified: true + } }); - const result = await server.setBreakpoint('test-session', 'MyClass', 5); + const { breakpoint: result } = await server.setBreakpoint({ sessionId: 'test-session', file: 'MyClass', line: 5 }); expect(result.verified).toBe(true); expect(mockFileChecker.checkExists).not.toHaveBeenCalled(); @@ -439,13 +451,15 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }; mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: '/path/to/MyClass.java', - line: 10, - verified: true + breakpoint: { + id: 'bp-1', + file: '/path/to/MyClass.java', + line: 10, + verified: true + } }); - await server.setBreakpoint('test-session', '/path/to/MyClass.java', 10); + await server.setBreakpoint({ sessionId: 'test-session', file: '/path/to/MyClass.java', line: 10 }); expect((server as any).fileChecker.checkExists).toHaveBeenCalledWith('/path/to/MyClass.java'); }); @@ -467,13 +481,15 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }; mockSessionManager.setBreakpoint.mockResolvedValue({ - id: 'bp-1', - file: '/path/to/script.py', - line: 10, - verified: true + breakpoint: { + id: 'bp-1', + file: '/path/to/script.py', + line: 10, + verified: true + } }); - await server.setBreakpoint('test-session', '/path/to/script.py', 10); + await server.setBreakpoint({ sessionId: 'test-session', file: '/path/to/script.py', line: 10 }); expect((server as any).fileChecker.checkExists).toHaveBeenCalledWith('/path/to/script.py'); }); @@ -495,7 +511,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }) }; - await expect(server.setBreakpoint('test-session', 'MyClass', 5)) + await expect(server.setBreakpoint({ sessionId: 'test-session', file: 'MyClass', line: 5 })) .rejects.toThrow('Breakpoint file not found'); expect((server as any).fileChecker.checkExists).toHaveBeenCalledWith('MyClass'); @@ -688,31 +704,41 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // First breakpoint mockSessionManager.setBreakpoint.mockResolvedValueOnce({ - id: 'bp-1', - file: 'com.example.Foo', - line: 10, - verified: true, + breakpoint: { + id: 'bp-1', + file: 'com.example.Foo', + line: 10, + verified: true, + } }); - await server.setBreakpoint('test-session', 'com.example.Foo', 10); + await server.setBreakpoint({ sessionId: 'test-session', file: 'com.example.Foo', line: 10 }); // Second breakpoint on same file mockSessionManager.setBreakpoint.mockResolvedValueOnce({ - id: 'bp-2', - file: 'com.example.Foo', - line: 20, - verified: true, + breakpoint: { + id: 'bp-2', + file: 'com.example.Foo', + line: 20, + verified: true, + } }); - await server.setBreakpoint('test-session', 'com.example.Foo', 20); + await server.setBreakpoint({ sessionId: 'test-session', file: 'com.example.Foo', line: 20 }); // Both calls should have been made expect(mockSessionManager.setBreakpoint).toHaveBeenCalledTimes(2); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.example.Foo', 10, undefined, undefined, undefined + 'test-session', expect.objectContaining({ + file: 'com.example.Foo', line: 10, + condition: undefined, suspendPolicy: undefined, logMessage: undefined + }) ); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.example.Foo', 20, undefined, undefined, undefined + 'test-session', expect.objectContaining({ + file: 'com.example.Foo', line: 20, + condition: undefined, suspendPolicy: undefined, logMessage: undefined + }) ); }); @@ -722,14 +748,16 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // Simulate setBreakpoint returning updated breakpoint info mockSessionManager.setBreakpoint.mockResolvedValueOnce({ - id: 'bp-1', - file: 'com.example.Foo', - line: 10, - verified: true, - message: 'Breakpoint set', + breakpoint: { + id: 'bp-1', + file: 'com.example.Foo', + line: 10, + verified: true, + message: 'Breakpoint set', + } }); - const result = await server.setBreakpoint('test-session', 'com.example.Foo', 10); + const { breakpoint: result } = await server.setBreakpoint({ sessionId: 'test-session', file: 'com.example.Foo', line: 10 }); expect(result.id).toBe('bp-1'); expect(result.verified).toBe(true); expect(result.line).toBe(10); @@ -742,29 +770,39 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // BP on com.a.Foo mockSessionManager.setBreakpoint.mockResolvedValueOnce({ - id: 'bp-1', - file: 'com.a.Foo', - line: 10, - verified: true, + breakpoint: { + id: 'bp-1', + file: 'com.a.Foo', + line: 10, + verified: true, + } }); - await server.setBreakpoint('test-session', 'com.a.Foo', 10); + await server.setBreakpoint({ sessionId: 'test-session', file: 'com.a.Foo', line: 10 }); // BP on com.b.Foo (different package, same simple name) mockSessionManager.setBreakpoint.mockResolvedValueOnce({ - id: 'bp-2', - file: 'com.b.Foo', - line: 15, - verified: true, + breakpoint: { + id: 'bp-2', + file: 'com.b.Foo', + line: 15, + verified: true, + } }); - await server.setBreakpoint('test-session', 'com.b.Foo', 15); + await server.setBreakpoint({ sessionId: 'test-session', file: 'com.b.Foo', line: 15 }); // Both should be set independently expect(mockSessionManager.setBreakpoint).toHaveBeenCalledTimes(2); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.a.Foo', 10, undefined, undefined, undefined + 'test-session', expect.objectContaining({ + file: 'com.a.Foo', line: 10, + condition: undefined, suspendPolicy: undefined, logMessage: undefined + }) ); expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith( - 'test-session', 'com.b.Foo', 15, undefined, undefined, undefined + 'test-session', expect.objectContaining({ + file: 'com.b.Foo', line: 15, + condition: undefined, suspendPolicy: undefined, logMessage: undefined + }) ); }); }); diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 05d28587..92d5607f 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -410,7 +410,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () it('should handle setBreakpoint with no proxy', async () => { mockSession.proxyManager = null; - const result = await operations.setBreakpoint('test-session', 'test.py', 10); + const { breakpoint: result } = await operations.setBreakpoint('test-session', { file: 'test.py', line: 10 }); // Without proxy, breakpoint is queued but not verified expect(result.verified).toBe(false); @@ -430,7 +430,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () } }); - const result = await operations.setBreakpoint('test-session', 'test.py', 10); + const { breakpoint: result } = await operations.setBreakpoint('test-session', { file: 'test.py', line: 10 }); expect(result.verified).toBe(false); expect(result.message).toContain('Invalid line number'); @@ -443,7 +443,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () } }); - const result = await operations.setBreakpoint('test-session', 'test.py', 10); + const { breakpoint: result } = await operations.setBreakpoint('test-session', { file: 'test.py', line: 10 }); expect(result.verified).toBe(false); }); @@ -453,7 +453,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockRejectedValue(new Error('Connection lost')); // Error is caught and logged, breakpoint is still created but unverified - const result = await operations.setBreakpoint('test-session', 'test.py', 10); + const { breakpoint: result } = await operations.setBreakpoint('test-session', { file: 'test.py', line: 10 }); expect(result.verified).toBe(false); expect(mockLogger.error).toHaveBeenCalled(); @@ -1403,7 +1403,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () await expect(() => operations.continue('test-session')) .rejects.toThrow(SessionTerminatedError); - await expect(() => operations.setBreakpoint('test-session', 'test.py', 10)) + await expect(() => operations.setBreakpoint('test-session', { file: 'test.py', line: 10 })) .rejects.toThrow(SessionTerminatedError); }); }); @@ -2096,7 +2096,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () })) } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', i * 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: i * 10 }); } // The last DAP call should have all 3 BPs @@ -2117,7 +2117,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () })) } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', i * 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: i * 10 }); } // Remove first BP (line 10) via the real API — removal itself re-syncs @@ -2151,7 +2151,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () })) } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', i * 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: i * 10 }); } // Remove middle BP (line 20) via the real API @@ -2185,7 +2185,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () })) } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', i * 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: i * 10 }); } // Remove last BP (line 30) via the real API @@ -2213,13 +2213,13 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.a.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.a.Foo', line: 10 }); // Set BP on file B (different package, same simple name) mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 20 }] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 20 }); // Last DAP request should only contain the BP for com.b.Foo const lastBps = getLastDapBreakpoints(); @@ -2233,7 +2233,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10 }); // Second BP: both returned, second unverified mockProxyManager.sendDapRequest.mockResolvedValue({ @@ -2244,7 +2244,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 20 }); const bps = Array.from(mockSession.breakpoints.values()); const bp10 = bps.find((bp: any) => bp.line === 10); @@ -2262,7 +2262,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () breakpoints: [{ verified: true, line: 12 }] // adjusted from 10 to 12 } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10 }); const bps = Array.from(mockSession.breakpoints.values()); expect(bps).toHaveLength(1); @@ -2279,7 +2279,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () }] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10 }); const bp = Array.from(mockSession.breakpoints.values())[0] as any; expect(bp.message).toBe('Breakpoint bound to com.example.Foo:10'); @@ -2289,7 +2289,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10, 'x > 5'); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10, condition: 'x > 5' }); const lastBps = getLastDapBreakpoints(); expect(lastBps).toHaveLength(1); @@ -2301,7 +2301,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10, 'x > 5'); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10, condition: 'x > 5' }); // Second BP without condition — DAP request should contain both mockProxyManager.sendDapRequest.mockResolvedValue({ @@ -2312,7 +2312,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 20 }); const lastBps = getLastDapBreakpoints(); expect(lastBps).toHaveLength(2); @@ -2327,7 +2327,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 10 }); // DAP only returns 1 BP in response (e.g. adapter bug or limit) mockProxyManager.sendDapRequest.mockResolvedValue({ @@ -2336,7 +2336,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () // missing second BP } }); - await operations.setBreakpoint('test-session', 'com.example.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.example.Foo', line: 20 }); // First BP updated, second remains unverified (default) const bps = Array.from(mockSession.breakpoints.values()); @@ -2351,13 +2351,13 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.a.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.a.Foo', line: 10 }); // Set BP on com.b.Foo mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 20 }] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 20 }); // Both BPs exist expect(mockSession.breakpoints.size).toBe(2); @@ -2377,7 +2377,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 30); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 30 }); // DAP request should only contain com.b.Foo BPs (20, 30), not com.a.Foo const lastBps = getLastDapBreakpoints(); @@ -2397,7 +2397,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 10 }); mockProxyManager.sendDapRequest.mockResolvedValue({ body: { @@ -2407,13 +2407,13 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 20 }); // Set BP on com.a.Foo mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 50 }] } }); - await operations.setBreakpoint('test-session', 'com.a.Foo', 50); + await operations.setBreakpoint('test-session', { file: 'com.a.Foo', line: 50 }); expect(mockSession.breakpoints.size).toBe(3); @@ -2432,7 +2432,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.b.Foo', 30); + await operations.setBreakpoint('test-session', { file: 'com.b.Foo', line: 30 }); // DAP request for com.b.Foo should contain remaining + new (20, 30) const lastBps = getLastDapBreakpoints(); @@ -2453,13 +2453,13 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.A.Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.A.Foo', line: 10 }); // com.A$Foo = inner class Foo of class A in default package mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 20 }] } }); - await operations.setBreakpoint('test-session', 'com.A$Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.A$Foo', line: 20 }); expect(mockSession.breakpoints.size).toBe(2); @@ -2478,7 +2478,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.A$Foo', 30); + await operations.setBreakpoint('test-session', { file: 'com.A$Foo', line: 30 }); // DAP request should only contain com.A$Foo BPs const lastBps = getLastDapBreakpoints(); @@ -2498,13 +2498,13 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 10 }] } }); - await operations.setBreakpoint('test-session', 'com.A$Foo', 10); + await operations.setBreakpoint('test-session', { file: 'com.A$Foo', line: 10 }); // com.A.Foo (regular class) mockProxyManager.sendDapRequest.mockResolvedValue({ body: { breakpoints: [{ verified: true, line: 20 }] } }); - await operations.setBreakpoint('test-session', 'com.A.Foo', 20); + await operations.setBreakpoint('test-session', { file: 'com.A.Foo', line: 20 }); // Add second BP to com.A.Foo mockProxyManager.sendDapRequest.mockResolvedValue({ @@ -2515,7 +2515,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () ] } }); - await operations.setBreakpoint('test-session', 'com.A.Foo', 30); + await operations.setBreakpoint('test-session', { file: 'com.A.Foo', line: 30 }); // DAP request should only contain com.A.Foo BPs (20, 30) const lastBps = getLastDapBreakpoints(); @@ -3009,7 +3009,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () } }); - await operations.setBreakpoint('test-session', 'test.py', 10, undefined, 'thread'); + await operations.setBreakpoint('test-session', { file: 'test.py', line: 10, suspendPolicy: 'thread' }); expect(mockProxyManager.sendDapRequest).toHaveBeenCalledWith( 'setBreakpoints', @@ -3029,7 +3029,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () } }); - await operations.setBreakpoint('test-session', 'test.py', 10); + await operations.setBreakpoint('test-session', { file: 'test.py', line: 10 }); const call = mockProxyManager.sendDapRequest.mock.calls.find( (c: any[]) => c[0] === 'setBreakpoints' diff --git a/tests/unit/utils/bp-addressing.test.ts b/tests/unit/utils/bp-addressing.test.ts new file mode 100644 index 00000000..31f6f544 --- /dev/null +++ b/tests/unit/utils/bp-addressing.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect } from 'vitest'; +import { + BP_ADDRESSING_ENV_KEY, + DEFAULT_BP_ADDRESSING, + getBpAddressingMode, + supportsExpectedContent, + supportsStatementAnchors, + supportsLoudSnapping, +} from '../../../src/utils/bp-addressing.js'; + +function envWith(value: string | undefined) { + return { + get: (key: string) => (key === BP_ADDRESSING_ENV_KEY ? value : undefined), + }; +} + +describe('breakpoint addressing mode helpers', () => { + it('defaults to content when the env variable is unset', () => { + expect(getBpAddressingMode(envWith(undefined))).toBe('content'); + expect(DEFAULT_BP_ADDRESSING).toBe('content'); + }); + + it('defaults to content for empty or whitespace-only values', () => { + expect(getBpAddressingMode(envWith(''))).toBe('content'); + expect(getBpAddressingMode(envWith(' '))).toBe('content'); + }); + + it('parses each valid mode', () => { + expect(getBpAddressingMode(envWith('line'))).toBe('line'); + expect(getBpAddressingMode(envWith('assert'))).toBe('assert'); + expect(getBpAddressingMode(envWith('content'))).toBe('content'); + }); + + it('normalizes case and surrounding whitespace', () => { + expect(getBpAddressingMode(envWith(' LINE '))).toBe('line'); + expect(getBpAddressingMode(envWith('Assert'))).toBe('assert'); + }); + + it('falls back to content for invalid values', () => { + expect(getBpAddressingMode(envWith('full'))).toBe('content'); + expect(getBpAddressingMode(envWith('1'))).toBe('content'); + expect(getBpAddressingMode(envWith('statement'))).toBe('content'); + }); + + it('gates expectedContent to assert and content modes', () => { + expect(supportsExpectedContent('line')).toBe(false); + expect(supportsExpectedContent('assert')).toBe(true); + expect(supportsExpectedContent('content')).toBe(true); + }); + + it('gates statement anchors to content mode only', () => { + expect(supportsStatementAnchors('line')).toBe(false); + expect(supportsStatementAnchors('assert')).toBe(false); + expect(supportsStatementAnchors('content')).toBe(true); + }); + + it('gates loud snapping to assert and content modes', () => { + expect(supportsLoudSnapping('line')).toBe(false); + expect(supportsLoudSnapping('assert')).toBe(true); + expect(supportsLoudSnapping('content')).toBe(true); + }); +}); diff --git a/tests/unit/utils/breakpoint-resolver.test.ts b/tests/unit/utils/breakpoint-resolver.test.ts new file mode 100644 index 00000000..398cdc42 --- /dev/null +++ b/tests/unit/utils/breakpoint-resolver.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest'; +import { assertLineContent } from '../../../src/utils/breakpoint-resolver.js'; + +const FILE = '/abs/app.py'; +const LINES = [ + 'def total_cart():', // 1 + ' prices = load()', // 2 + ' total = sum(prices)', // 3 + ' return total', // 4 + '', // 5 + 'def main():', // 6 +]; + +describe('assertLineContent', () => { + it('passes when trimmed content matches', () => { + const result = assertLineContent(LINES, 3, 'total = sum(prices)', FILE); + expect(result.ok).toBe(true); + }); + + it('passes when the expectation itself has stray whitespace', () => { + const result = assertLineContent(LINES, 3, ' total = sum(prices) ', FILE); + expect(result.ok).toBe(true); + }); + + it('fails with expected/actual and marked context on mismatch', () => { + const result = assertLineContent(LINES, 4, 'total = sum(prices)', FILE); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.actual).toBe('return total'); + expect(result.message).toContain( + `line 4 of ${FILE} does not match expectedContent` + ); + expect(result.message).toContain('Expected: "total = sum(prices)"'); + expect(result.message).toMatch(/Actual:\s+"return total"/); + // context window with a > marker on the target line + expect(result.message).toMatch(/>\s+4 \| return total/); + expect(result.message).toMatch(/\s+3 \| total = sum\(prices\)/); + expect(result.message).toContain('may have changed since you last read it'); + // assert mode must not teach the statement param + expect(result.message).not.toContain('statement'); + }); + + it('appends the statement-mode hint only when requested', () => { + const result = assertLineContent(LINES, 4, 'total = sum(prices)', FILE, { + statementHint: true, + }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.message).toContain('statement: "total = sum(prices)"'); + }); + + it('fails clearly when the line is beyond end of file', () => { + const result = assertLineContent(LINES, 200, 'anything', FILE); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.actual).toBeNull(); + expect(result.message).toContain(`line 200 of ${FILE} does not exist`); + expect(result.message).toContain('6 lines'); + }); +}); diff --git a/tests/unit/utils/line-reader.test.ts b/tests/unit/utils/line-reader.test.ts new file mode 100644 index 00000000..facb7cb3 --- /dev/null +++ b/tests/unit/utils/line-reader.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect, vi } from 'vitest'; +import { LineReader } from '../../../src/utils/line-reader.js'; +import type { IFileSystem } from '@debugmcp/shared'; + +interface FakeFile { + content: string; + mtimeMs: number; +} + +function makeFs(files: Record) { + const readFile = vi.fn(async (path: string) => files[path].content); + const stat = vi.fn(async (path: string) => ({ + size: files[path].content.length, + mtimeMs: files[path].mtimeMs, + })); + const fs = { readFile, stat } as unknown as IFileSystem; + return { fs, readFile, stat }; +} + +describe('LineReader caching', () => { + it('serves repeated reads from cache while the file is unchanged', async () => { + const { fs, readFile } = makeFs({ + '/a.py': { content: 'one\ntwo\nthree', mtimeMs: 1000 }, + }); + const reader = new LineReader(fs); + + await reader.getLineContext('/a.py', 2); + await reader.getLineContext('/a.py', 2); + + expect(readFile).toHaveBeenCalledTimes(1); + }); + + it('re-reads the file when its mtime changes', async () => { + const files: Record = { + '/a.py': { content: 'one\ntwo\nthree', mtimeMs: 1000 }, + }; + const { fs, readFile } = makeFs(files); + const reader = new LineReader(fs); + + const before = await reader.getLineContext('/a.py', 2); + expect(before?.lineContent).toBe('two'); + + files['/a.py'] = { content: 'one\nTWO EDITED\nthree', mtimeMs: 2000 }; + const after = await reader.getLineContext('/a.py', 2); + + expect(after?.lineContent).toBe('TWO EDITED'); + expect(readFile).toHaveBeenCalledTimes(2); + }); +}); + +describe('LineReader.getFileLines', () => { + it('returns all lines of a text file', async () => { + const { fs } = makeFs({ + '/a.py': { content: 'one\ntwo\nthree', mtimeMs: 1000 }, + }); + const reader = new LineReader(fs); + + expect(await reader.getFileLines('/a.py')).toEqual(['one', 'two', 'three']); + }); + + it('returns null for binary content', async () => { + const { fs } = makeFs({ + '/bin': { content: 'abc\0def', mtimeMs: 1000 }, + }); + const reader = new LineReader(fs); + + expect(await reader.getFileLines('/bin')).toBeNull(); + }); + + it('returns null when the file cannot be read', async () => { + const readFile = vi.fn(async () => { + throw new Error('ENOENT'); + }); + const stat = vi.fn(async () => { + throw new Error('ENOENT'); + }); + const reader = new LineReader({ readFile, stat } as unknown as IFileSystem); + + expect(await reader.getFileLines('/missing')).toBeNull(); + }); +});