From a6583de7b4ffbdd2fb53bdc0fc93c211aa1dceaf Mon Sep 17 00:00:00 2001 From: ozzafar <48795672+ozzafar@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:08:39 +0300 Subject: [PATCH 1/2] Fix #18: add_breakpoint takes a line number instead of lineContent Replace the substring-based `lineContent` parameter with a 1-based `line` number, making add_breakpoint consistent with remove_breakpoint, list_breakpoints, and the documented tool schema. The handler now validates the line (integer >= 1 and within the file's line count) and sets a single breakpoint at that line, failing clearly instead of substring-matching (which could set multiple/unexpected breakpoints). Updated the MCP tool schema, routing handler/test, README, SKILL.md, and the C# troubleshooting doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6ea24b92-40ad-4539-aa13-61aa6ff8751f --- README.md | 4 +- skills/debug-live/SKILL.md | 13 +++--- .../references/troubleshooting/csharp.md | 2 +- src/debugMCPServer.ts | 4 +- src/debuggingHandler.ts | 46 +++++++------------ src/routingDebuggingHandler.ts | 2 +- src/test/routing.test.ts | 2 +- 7 files changed, 29 insertions(+), 44 deletions(-) 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..ef23a3b 100644 --- a/src/debugMCPServer.ts +++ b/src/debugMCPServer.ts @@ -271,10 +271,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/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..6949780 100644 --- a/src/routingDebuggingHandler.ts +++ b/src/routingDebuggingHandler.ts @@ -198,7 +198,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..36f1a94 100644 --- a/src/test/routing.test.ts +++ b/src/test/routing.test.ts @@ -115,7 +115,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'); From 19bfde517ce3b34e3da4becd2eaf975e05eeba94 Mon Sep 17 00:00:00 2001 From: ozzafar <48795672+ozzafar@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:34:08 +0300 Subject: [PATCH 2/2] Add regression tests for tool-call timeout handling Extract the shared "race work against a timeout" logic used by DebuggingExecutor.dapRequest and DebugMCPServer.runTool into a tested withTimeout() utility (behavior preserved), and add regression coverage for the "server stuck when a tool call hangs" fix: - withTimeout.test.ts: resolves fast work, propagates work rejections, rejects with the caller's error when work hangs, and clears the timer so a late-but-eventual result still wins. - routing.test.ts: a control server that accepts but never responds must cause handleStartDebugging to reject with the actionable "did not respond within ... unresponsive" message. RoutingDebuggingHandler now accepts an optional forwardTimeoutMs override so the test runs fast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6ea24b92-40ad-4539-aa13-61aa6ff8751f --- src/debugMCPServer.ts | 27 ++++++---------- src/debuggingExecutor.ts | 25 ++++++--------- src/routingDebuggingHandler.ts | 8 +++-- src/test/routing.test.ts | 47 +++++++++++++++++++++++++++ src/test/withTimeout.test.ts | 58 ++++++++++++++++++++++++++++++++++ src/utils/withTimeout.ts | 28 ++++++++++++++++ 6 files changed, 157 insertions(+), 36 deletions(-) create mode 100644 src/test/withTimeout.test.ts create mode 100644 src/utils/withTimeout.ts diff --git a/src/debugMCPServer.ts b/src/debugMCPServer.ts index ef23a3b..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 }] }; } /** 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/routingDebuggingHandler.ts b/src/routingDebuggingHandler.ts index 6949780..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; } /** diff --git a/src/test/routing.test.ts b/src/test/routing.test.ts index 36f1a94..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 }); }); @@ -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); + } + } +}