diff --git a/README.md b/README.md index 06333bc..880c235 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ DebugMCP is an MCP server that gives AI coding agents full control over the VS C | **continue_execution** | Continue until next breakpoint | None | | **pause_execution** | Interrupt a freely-running program and stop at its current location (no breakpoint needed) | None | | **restart_debugging** | Restart the current debug session | None | -| **add_breakpoint** | Add a breakpoint at a specific line (optionally conditional) | `fileFullPath` (required)
`lineContent` (required)
`condition` (optional) | +| **add_breakpoint** | Add a breakpoint at a specific line (optionally conditional) | `fileFullPath` (required)
`line` (required, 1-based)
`condition` (optional) | | **remove_breakpoint** | Remove a breakpoint from a specific line | `fileFullPath` (required)
`line` (required) | | **clear_all_breakpoints** | Remove all breakpoints at once | None | | **list_breakpoints** | List all active breakpoints | None | @@ -371,7 +371,7 @@ Yes. DebugMCP supports `.cs` files and `.csproj` project files for C#/.NET debug - **Symptom**: Breakpoints are set but execution doesn't pause - **Solution**: - Ensure the correct file is being debugged - - Check that the breakpoint line content matches exactly + - Check that the breakpoint line number is correct - Verify the relevant language debugger extension is installed #### Configuration Not Auto-Detected diff --git a/skills/debug-live/SKILL.md b/skills/debug-live/SKILL.md index ac8c010..5431e45 100644 --- a/skills/debug-live/SKILL.md +++ b/skills/debug-live/SKILL.md @@ -46,9 +46,8 @@ If you can step through the code in a few tool calls, do that instead of specula ## Core workflow -1. **Set a starting breakpoint.** Use `add_breakpoint` with the file path and the exact - line content you want to pause on (line content matching is more robust than line - numbers — it survives small edits). Place it at the earliest point that's still +1. **Set a starting breakpoint.** Use `add_breakpoint` with the file path and the 1-based + line number you want to pause on. Place it at the earliest point that's still relevant to the suspected issue. 2. **Optionally add strategic breakpoints.** Decision points, error-handling branches, data boundaries (where input enters, where output is produced). @@ -168,8 +167,8 @@ Before ending the debug session, confirm you can answer: - **Start broad, then narrow.** Begin at the entry point of the suspect function. As you isolate the issue, add tighter breakpoints around the problematic region. -- **Match line content, not numbers.** `add_breakpoint` takes the exact line text so - breakpoints survive small edits and refactors. +- **Use line numbers.** `add_breakpoint` takes a 1-based `line`; re-check the line after + edits since numbers shift when code changes. - **Don't overuse breakpoints.** A handful of well-placed pauses beats dozens of noisy ones. After each session, `clear_all_breakpoints` to start fresh. - **For test debugging,** pass `testName` to `start_debugging`. The server routes through @@ -182,7 +181,7 @@ Before ending the debug session, confirm you can answer: ### Investigating a bug in `calculate.py` ```text -add_breakpoint fileFullPath=/repo/src/calculate.py lineContent="result = parse(raw)" +add_breakpoint fileFullPath=/repo/src/calculate.py line=42 start_debugging fileFullPath=/repo/src/calculate.py workingDirectory=/repo # session pauses on the breakpoint get_variables_values scope=local @@ -194,7 +193,7 @@ clear_all_breakpoints ### Debugging a single xUnit test in C# ```text -add_breakpoint fileFullPath=C:\Repo\Calculator.Tests\CalculatorTests.cs lineContent="Assert.Equal(5, _calc.Add(2, 3));" +add_breakpoint fileFullPath=C:\Repo\Calculator.Tests\CalculatorTests.cs line=18 start_debugging fileFullPath=C:\Repo\Calculator.Tests\CalculatorTests.cs workingDirectory=C:\Repo testName=Add_ReturnsSum # pauses inside the test step_into diff --git a/skills/debug-live/references/troubleshooting/csharp.md b/skills/debug-live/references/troubleshooting/csharp.md index 86b04b2..c4e1f78 100644 --- a/skills/debug-live/references/troubleshooting/csharp.md +++ b/skills/debug-live/references/troubleshooting/csharp.md @@ -33,7 +33,7 @@ DebugMCP provides enhanced support for debugging C# applications using the C# De 3. Set appropriate working directories ### Breakpoint Management -1. Use line content matching for reliable breakpoint placement +1. Use the 1-based line number for reliable breakpoint placement 2. Clear breakpoints when stopping debug sessions 3. Verify breakpoints are hit in the expected files diff --git a/src/debugMCPServer.ts b/src/debugMCPServer.ts index 39530ac..7f4156f 100644 --- a/src/debugMCPServer.ts +++ b/src/debugMCPServer.ts @@ -11,6 +11,7 @@ import { IDebuggingHandler } from '.'; import { logger } from './utils/logger'; +import { withTimeout } from './utils/withTimeout'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import { isInitializeRequest } from '@modelcontextprotocol/sdk/types.js'; @@ -182,23 +183,15 @@ export class DebugMCPServer { toolName: string, run: () => Promise ): Promise<{ content: { type: 'text'; text: string }[] }> { - let timer: ReturnType | undefined; - const backstop = new Promise((_, reject) => { - timer = setTimeout(() => { - reject(new Error( - `Tool "${toolName}" did not complete within ${Math.round(this.toolBackstopMs / 1000)}s and was aborted. ` + - 'The debug adapter or target VS Code window may be unresponsive. Try stop_debugging and retry, or reload the window.' - )); - }, this.toolBackstopMs); - }); - try { - const result = await Promise.race([run(), backstop]); - return { content: [{ type: 'text' as const, text: result }] }; - } finally { - if (timer) { - clearTimeout(timer); - } - } + const result = await withTimeout( + run(), + this.toolBackstopMs, + () => new Error( + `Tool "${toolName}" did not complete within ${Math.round(this.toolBackstopMs / 1000)}s and was aborted. ` + + 'The debug adapter or target VS Code window may be unresponsive. Try stop_debugging and retry, or reload the window.' + ) + ); + return { content: [{ type: 'text' as const, text: result }] }; } /** @@ -271,10 +264,10 @@ export class DebugMCPServer { description: 'Set a breakpoint to pause execution at a critical line of code. Essential for debugging: pause before potential errors, examine state at decision points, or verify code paths. Breakpoints let you inspect variables and control flow at exact moments. Provide an optional condition to create a conditional breakpoint that only pauses when the expression evaluates to true (e.g. "i == 5" or "user.id === null").', inputSchema: { fileFullPath: z.string().describe('Full path to the file'), - lineContent: z.string().describe('Line content'), + line: z.number().int().describe('Line number (1-based) where the breakpoint should be set'), condition: z.string().optional().describe('Optional condition expression. When provided, execution only pauses if this expression evaluates to true at the breakpoint location.'), }, - }, async (args: { fileFullPath: string; lineContent: string; condition?: string }) => + }, async (args: { fileFullPath: string; line: number; condition?: string }) => this.runTool('add_breakpoint', () => debuggingHandler.handleAddBreakpoint(args))); // Remove breakpoint tool diff --git a/src/debuggingExecutor.ts b/src/debuggingExecutor.ts index 39d0b68..023088d 100644 --- a/src/debuggingExecutor.ts +++ b/src/debuggingExecutor.ts @@ -3,6 +3,7 @@ import * as vscode from 'vscode'; import { DebugState, StackFrame } from './debugState'; import { logger } from './utils/logger'; +import { withTimeout } from './utils/withTimeout'; /** * Outcome of dispatching `testing.debugAtCursor`. @@ -62,22 +63,14 @@ export class DebuggingExecutor implements IDebuggingExecutor { command: string, args: unknown ): Promise { - let timer: ReturnType | undefined; - const timeout = new Promise((_, reject) => { - timer = setTimeout(() => { - reject(new Error( - `Debug adapter did not respond to '${command}' within ` + - `${Math.round(DebuggingExecutor.DAP_REQUEST_TIMEOUT_MS / 1000)}s (it may be unresponsive).` - )); - }, DebuggingExecutor.DAP_REQUEST_TIMEOUT_MS); - }); - try { - return await Promise.race([session.customRequest(command, args), timeout]); - } finally { - if (timer) { - clearTimeout(timer); - } - } + return withTimeout( + Promise.resolve(session.customRequest(command, args)), + DebuggingExecutor.DAP_REQUEST_TIMEOUT_MS, + () => new Error( + `Debug adapter did not respond to '${command}' within ` + + `${Math.round(DebuggingExecutor.DAP_REQUEST_TIMEOUT_MS / 1000)}s (it may be unresponsive).` + ) + ); } /** diff --git a/src/debuggingHandler.ts b/src/debuggingHandler.ts index cec55e5..d068386 100644 --- a/src/debuggingHandler.ts +++ b/src/debuggingHandler.ts @@ -18,7 +18,7 @@ export interface IDebuggingHandler { handleContinue(): Promise; handlePause(): Promise; handleRestart(): Promise; - handleAddBreakpoint(args: { fileFullPath: string; lineContent: string; condition?: string }): Promise; + handleAddBreakpoint(args: { fileFullPath: string; line: number; condition?: string }): Promise; handleRemoveBreakpoint(args: { fileFullPath: string; line: number }): Promise; handleClearAllBreakpoints(): Promise; handleListBreakpoints(): Promise; @@ -300,40 +300,26 @@ export class DebuggingHandler implements IDebuggingHandler { * Add a breakpoint at specified location. An optional condition makes it a * conditional breakpoint that only pauses when the expression is true. */ - public async handleAddBreakpoint(args: { fileFullPath: string; lineContent: string; condition?: string }): Promise { - const { fileFullPath, lineContent, condition } = args; - + public async handleAddBreakpoint(args: { fileFullPath: string; line: number; condition?: string }): Promise { + const { fileFullPath, line, condition } = args; + try { - // Find the line number containing the line content - const document = await vscode.workspace.openTextDocument(vscode.Uri.file(fileFullPath)); - const text = document.getText(); - const lines = text.split(/\r?\n/); - const matchingLineNumbers: number[] = []; - - for (let i = 0; i < lines.length; i++) { - if (lines[i].includes(lineContent)) { - matchingLineNumbers.push(i + 1); // Convert to 1-based line numbers - } + if (!Number.isInteger(line) || line < 1) { + throw new Error(`Invalid line number: ${line}. Provide a 1-based line number.`); } - - if (matchingLineNumbers.length === 0) { - throw new Error(`Could not find any lines containing: ${lineContent}`); + + // Validate the line exists so we fail clearly instead of setting an + // unbound breakpoint past the end of the file. + const document = await vscode.workspace.openTextDocument(vscode.Uri.file(fileFullPath)); + if (line > document.lineCount) { + throw new Error(`Line ${line} is out of range: ${fileFullPath} has ${document.lineCount} lines.`); } - + const uri = vscode.Uri.file(fileFullPath); - - // Add breakpoints to all matching lines - for (const lineNumber of matchingLineNumbers) { - await this.executor.addBreakpoint(uri, lineNumber, condition); - } - + await this.executor.addBreakpoint(uri, line, condition); + const conditionInfo = condition ? ` (condition: ${condition})` : ''; - if (matchingLineNumbers.length === 1) { - return `Breakpoint added at ${fileFullPath}:${matchingLineNumbers[0]}${conditionInfo}`; - } else { - const linesList = matchingLineNumbers.join(', '); - return `Breakpoints added at ${matchingLineNumbers.length} locations in ${fileFullPath}: lines ${linesList}${conditionInfo}`; - } + return `Breakpoint added at ${fileFullPath}:${line}${conditionInfo}`; } catch (error) { throw new Error(`Error adding breakpoint: ${error}`); } diff --git a/src/routingDebuggingHandler.ts b/src/routingDebuggingHandler.ts index ee1acc4..f6489e9 100644 --- a/src/routingDebuggingHandler.ts +++ b/src/routingDebuggingHandler.ts @@ -19,14 +19,16 @@ export class RoutingDebuggingHandler implements IDebuggingHandler { // Bound the router->worker round-trip so a hung worker can't hang the // forward forever. Kept above the worker's own timeout to avoid preempting - // slow-but-progressing operations. + // slow-but-progressing operations. `forwardTimeoutMs` can be passed + // explicitly (mainly for tests) to override the derived value. private readonly forwardTimeoutMs: number; constructor( private readonly registry: WorkspaceRegistry, - timeoutInSeconds: number = 180 + timeoutInSeconds: number = 180, + forwardTimeoutMs?: number ) { - this.forwardTimeoutMs = timeoutInSeconds * 1000 + 15_000; + this.forwardTimeoutMs = forwardTimeoutMs ?? timeoutInSeconds * 1000 + 15_000; } /** @@ -198,7 +200,7 @@ export class RoutingDebuggingHandler implements IDebuggingHandler { return this.forward('handleRestart', {}); } - public handleAddBreakpoint(args: { fileFullPath: string; lineContent: string; condition?: string }): Promise { + public handleAddBreakpoint(args: { fileFullPath: string; line: number; condition?: string }): Promise { return this.forward('handleAddBreakpoint', args, args.fileFullPath); } diff --git a/src/test/routing.test.ts b/src/test/routing.test.ts index a53ccf8..2fe6334 100644 --- a/src/test/routing.test.ts +++ b/src/test/routing.test.ts @@ -2,6 +2,7 @@ import * as assert from 'assert'; import * as fs from 'fs'; +import * as http from 'http'; import * as os from 'os'; import * as path from 'path'; import { ControlServer } from '../controlServer'; @@ -37,6 +38,7 @@ class RecordingHandler implements IDebuggingHandler { suite('Multi-window routing', () => { let dir: string; const servers: ControlServer[] = []; + const deadServers: http.Server[] = []; setup(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'debugmcp-routing-test-')); @@ -44,6 +46,7 @@ suite('Multi-window routing', () => { teardown(async () => { await Promise.all(servers.splice(0).map(s => s.stop())); + await Promise.all(deadServers.splice(0).map(s => new Promise(resolve => s.close(() => resolve())))); fs.rmSync(dir, { recursive: true, force: true }); }); @@ -115,7 +118,7 @@ suite('Multi-window routing', () => { const routing = new RoutingDebuggingHandler(new WorkspaceRegistry(process.pid, dir)); const result = await routing.handleAddBreakpoint({ fileFullPath: path.join(repoA, 'src', 'y.py'), - lineContent: 'return 1' + line: 1 }); assert.strictEqual(result, 'A:addBp'); @@ -143,6 +146,49 @@ suite('Multi-window routing', () => { await assert.rejects(() => routing.handleStepOver(), /no active debug target/i); }); + /** + * Register a window whose control server accepts connections but never + * responds, to simulate a hung/unresponsive VS Code window. + */ + async function startDeadWindow(fileName: string, workspaceFolders: string[]): Promise { + const server = http.createServer(() => { /* accept, never respond */ }); + deadServers.push(server); + const port: number = await new Promise(resolve => { + server.listen(0, '127.0.0.1', () => resolve((server.address() as { port: number }).port)); + }); + fs.writeFileSync( + path.join(dir, fileName), + JSON.stringify({ + pid: process.pid, + controlPort: port, + controlToken: 'secret', + workspaceFolders, + name: fileName, + updatedAt: Date.now() + }), + 'utf8' + ); + } + + test('rejects with an actionable error when the target window is unresponsive', async () => { + const repoA = path.join(dir, 'repoA'); + await startDeadWindow('a.json', [repoA]); + + // 200ms forward timeout so the hung round-trip fails fast in the test. + const routing = new RoutingDebuggingHandler(new WorkspaceRegistry(process.pid, dir), 180, 200); + const start = Date.now(); + await assert.rejects( + () => routing.handleStartDebugging({ + fileFullPath: path.join(repoA, 'm.py'), + workingDirectory: repoA + }), + /did not respond within[\s\S]*unresponsive/i + ); + const elapsed = Date.now() - start; + assert.ok(elapsed >= 150, `should wait for the ~200ms forward timeout, only took ${elapsed}ms`); + assert.ok(elapsed < 5000, `forward timeout should fire promptly, took ${elapsed}ms`); + }); + test('control server rejects requests with a wrong token', async () => { const repoA = path.join(dir, 'repoA'); await startWindow('a.json', [repoA], new RecordingHandler('A'), 'right-token'); @@ -160,3 +206,4 @@ suite('Multi-window routing', () => { ); }); }); + diff --git a/src/test/withTimeout.test.ts b/src/test/withTimeout.test.ts new file mode 100644 index 0000000..c01e512 --- /dev/null +++ b/src/test/withTimeout.test.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. + +import * as assert from 'assert'; +import { withTimeout } from '../utils/withTimeout'; + +/** + * Regression tests for the shared timeout primitive that bounds every DAP + * request (DebuggingExecutor.dapRequest) and every MCP tool call + * (DebugMCPServer.runTool). These guard the "server stuck when a tool hangs" + * fix: a hung operation must reject with the caller's error instead of + * pending forever, and a fast operation must pass through untouched. + */ +suite('withTimeout', () => { + test('resolves with the work value when it settles in time', async () => { + const value = await withTimeout( + Promise.resolve('ok'), + 1000, + () => new Error('should not fire') + ); + assert.strictEqual(value, 'ok'); + }); + + test('propagates a rejection from the work promise (not the timeout error)', async () => { + await assert.rejects( + withTimeout( + Promise.reject(new Error('work failed')), + 1000, + () => new Error('timeout error') + ), + /work failed/ + ); + }); + + test('rejects with the onTimeout error when work never settles', async () => { + const start = Date.now(); + await assert.rejects( + withTimeout( + new Promise(() => { /* never settles */ }), + 50, + () => new Error('adapter unresponsive') + ), + /adapter unresponsive/ + ); + const elapsed = Date.now() - start; + assert.ok(elapsed >= 40, `should wait for the ~50ms timeout, only took ${elapsed}ms`); + assert.ok(elapsed < 1000, `timeout should fire promptly, took ${elapsed}ms`); + }); + + test('clears the timer so a late-but-eventual work result still wins', async () => { + // Work resolves before the (long) timeout; the timer must be cleared so + // it neither delays the result nor rejects afterwards. + const work = new Promise(resolve => setTimeout(() => resolve('late-ok'), 20)); + const value = await withTimeout(work, 5000, () => new Error('should not fire')); + assert.strictEqual(value, 'late-ok'); + // Give any stray timer a chance to (wrongly) fire; nothing should throw. + await new Promise(resolve => setTimeout(resolve, 30)); + }); +}); diff --git a/src/utils/withTimeout.ts b/src/utils/withTimeout.ts new file mode 100644 index 0000000..c35c72c --- /dev/null +++ b/src/utils/withTimeout.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. + +/** + * Race a promise against a timeout so a hung operation can't block forever. + * + * Resolves/rejects with `work`'s outcome if it settles within `timeoutMs`; + * otherwise rejects with the Error from `onTimeout()`. The timer is always + * cleared so a slow-but-eventual `work` doesn't leak a pending timer or a + * late rejection. Note: `work` itself is not cancelled — this only bounds how + * long the caller waits. + */ +export async function withTimeout( + work: Promise, + timeoutMs: number, + onTimeout: () => Error +): Promise { + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(onTimeout()), timeoutMs); + }); + try { + return await Promise.race([work, timeout]); + } finally { + if (timer) { + clearTimeout(timer); + } + } +}