From b1184ffe2b954a4f9c37a864805ed111d8dc2678 Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Tue, 2 Jun 2026 15:38:59 -0700 Subject: [PATCH 1/3] Track function breakpoints to fix DAP replace semantics DAP's SetFunctionBreakpointsRequest replaces all function breakpoints on each call. Previously, setting a new function breakpoint would silently remove any existing ones. Add a `functionBreakpoints` slice to `debuggerSession` that tracks all active function breakpoints, with helper methods to add, remove, and clear them. All callers now go through these helpers to maintain the full list. Also includes minor cleanups: - Use strings.Builder instead of string concatenation in tests - Use strings.SplitSeq and strings.Cut where appropriate - Fix DebugParams struct field alignment --- tools.go | 105 +++++++++++++++++++++++++++++++++++--------------- tools_test.go | 46 +++++++++++----------- 2 files changed, 97 insertions(+), 54 deletions(-) diff --git a/tools.go b/tools.go index b218429..d5eb013 100644 --- a/tools.go +++ b/tools.go @@ -8,6 +8,7 @@ import ( "log" "os" "os/exec" + "slices" "strings" "sync" @@ -16,20 +17,21 @@ import ( ) type debuggerSession struct { - mu sync.Mutex // serializes DAP requests to prevent concurrent read races - cmd *exec.Cmd - client *DAPClient - server *mcp.Server // MCP server for dynamic tool registration - logWriter io.Writer // writer for adapter stderr (log file or io.Discard) - backend DebuggerBackend // debugger-specific backend (delve, gdb, etc.) - capabilities dap.Capabilities // capabilities reported by DAP server - launchMode string // "source", "binary", "core", or "attach" - programPath string // path to program being debugged - programArgs []string // command line arguments - coreFilePath string // path to core dump file (core mode only) - stoppedThreadID int // thread ID from last StoppedEvent (for adapters that use non-sequential IDs) - lastFrameID int // frame ID from last getFullContext; -1 means not set (0 is valid for GDB) - protocolLogFile *os.File // protocol log file (closed on cleanup) + mu sync.Mutex // serializes DAP requests to prevent concurrent read races + cmd *exec.Cmd + client *DAPClient + server *mcp.Server // MCP server for dynamic tool registration + logWriter io.Writer // writer for adapter stderr (log file or io.Discard) + backend DebuggerBackend // debugger-specific backend (delve, gdb, etc.) + capabilities dap.Capabilities // capabilities reported by DAP server + launchMode string // "source", "binary", "core", or "attach" + programPath string // path to program being debugged + programArgs []string // command line arguments + coreFilePath string // path to core dump file (core mode only) + stoppedThreadID int // thread ID from last StoppedEvent (for adapters that use non-sequential IDs) + lastFrameID int // frame ID from last getFullContext; -1 means not set (0 is valid for GDB) + protocolLogFile *os.File // protocol log file (closed on cleanup) + functionBreakpoints []string // tracked function breakpoints (DAP replaces all on each request) } // defaultThreadID returns the thread ID to use when none is specified. @@ -41,6 +43,32 @@ func (ds *debuggerSession) defaultThreadID() int { return 1 } +// addFunctionBreakpoint appends a function breakpoint (if not already tracked) +// and sends the full list to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) addFunctionBreakpoint(name string) (int, error) { + if slices.Contains(ds.functionBreakpoints, name) { + return ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) + } + ds.functionBreakpoints = append(ds.functionBreakpoints, name) + return ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) +} + +// removeFunctionBreakpoint removes a function breakpoint by name +// and sends the updated list to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) removeFunctionBreakpoint(name string) (int, error) { + ds.functionBreakpoints = slices.DeleteFunc(ds.functionBreakpoints, func(fn string) bool { + return fn == name + }) + return ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) +} + +// clearFunctionBreakpoints removes all tracked function breakpoints +// and sends an empty list to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) clearFunctionBreakpoints() (int, error) { + ds.functionBreakpoints = nil + return ds.client.SetFunctionBreakpointsRequest([]string{}) +} + const debugToolDescription = `Start a complete debugging session. Modes: 'source' (compile & debug), 'binary' (debug executable), 'core' (debug core dump), 'attach' (connect to process). @@ -112,9 +140,9 @@ Examples: {"file": "/path/to/main.go", "line": 42} or {"function": "main.process }, ds.breakpoint) mcp.AddTool(ds.server, &mcp.Tool{ Name: "clear-breakpoints", - Description: `Remove breakpoints. Provide 'file' to clear breakpoints in a specific file, or 'all': true to clear all breakpoints. + Description: `Remove breakpoints. Provide 'file' to clear breakpoints in a specific file, 'function' to clear a specific function breakpoint, or 'all': true to clear all breakpoints. -Examples: {"file": "/path/to/main.go"} or {"all": true}`, +Examples: {"file": "/path/to/main.go"} or {"function": "main.processData"} or {"all": true}`, }, ds.clearBreakpoints) mcp.AddTool(ds.server, &mcp.Tool{ Name: "continue", @@ -221,11 +249,11 @@ type DebugParams struct { Breakpoints []BreakpointSpec `json:"breakpoints,omitempty" mcp:"initial breakpoints"` StopOnEntry bool `json:"stopOnEntry,omitempty" mcp:"stop at program entry instead of running to first breakpoint"` Port string `json:"port,omitempty" mcp:"port for DAP server (default: auto-assigned)"` - Debugger string `json:"debugger,omitempty" mcp:"debugger to use: 'delve' (default) or 'gdb'"` - GDBPath string `json:"gdbPath,omitempty" mcp:"path to gdb binary (default: auto-detected from PATH). Requires GDB 14+."` - ProtocolLog string `json:"protocolLog,omitempty" mcp:"file path for protocol-level DAP message logging (what the MCP server sends/receives)"` - ToolLog string `json:"toolLog,omitempty" mcp:"file path for tool-level DAP logging (native debugger logging, GDB only)"` - FullContext bool `json:"fullContext,omitempty" mcp:"if true, return full context (stack trace and variables) when stopped at a breakpoint; if false (default), return a compact stop summary — leave false unless you need variables immediately"` + Debugger string `json:"debugger,omitempty" mcp:"debugger to use: 'delve' (default) or 'gdb'"` + GDBPath string `json:"gdbPath,omitempty" mcp:"path to gdb binary (default: auto-detected from PATH). Requires GDB 14+."` + ProtocolLog string `json:"protocolLog,omitempty" mcp:"file path for protocol-level DAP message logging (what the MCP server sends/receives)"` + ToolLog string `json:"toolLog,omitempty" mcp:"file path for tool-level DAP logging (native debugger logging, GDB only)"` + FullContext bool `json:"fullContext,omitempty" mcp:"if true, return full context (stack trace and variables) when stopped at a breakpoint; if false (default), return a compact stop summary — leave false unless you need variables immediately"` } // ContextParams defines the parameters for getting debugging context. @@ -326,8 +354,9 @@ func readTypedResponse[T dap.ResponseMessage](client *DAPClient, requestSeq int) // ClearBreakpointsParams defines parameters for clearing breakpoints. type ClearBreakpointsParams struct { - File string `json:"file,omitempty" mcp:"clear all breakpoints in this file"` - All bool `json:"all,omitempty" mcp:"clear all breakpoints"` + File string `json:"file,omitempty" mcp:"clear all breakpoints in this file"` + Function string `json:"function,omitempty" mcp:"clear a function breakpoint by name"` + All bool `json:"all,omitempty" mcp:"clear all breakpoints"` } // StopParams defines parameters for stopping the debug session. @@ -345,7 +374,7 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool if params.All { // Clear all function breakpoints - seq, err := ds.client.SetFunctionBreakpointsRequest([]string{}) + seq, err := ds.clearFunctionBreakpoints() if err != nil { return nil, nil, err } @@ -357,6 +386,19 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool }, nil, nil } + if params.Function != "" { + seq, err := ds.removeFunctionBreakpoint(params.Function) + if err != nil { + return nil, nil, err + } + if err := readAndValidateResponse(ds.client, seq, "unable to clear function breakpoint"); err != nil { + return nil, nil, err + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("Cleared function breakpoint: %s", params.Function)}}, + }, nil, nil + } + if params.File != "" { // Clear breakpoints in specific file by setting empty list seq, err := ds.client.SetBreakpointsRequest(params.File, []int{}) @@ -371,7 +413,7 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool }, nil, nil } - return nil, nil, fmt.Errorf("specify 'file' or 'all'") + return nil, nil, fmt.Errorf("specify 'file', 'function', or 'all'") } // ContinueParams defines the parameters for continuing execution. @@ -393,7 +435,7 @@ func (ds *debuggerSession) continueExecution(ctx context.Context, _ *mcp.CallToo if params.To != nil { to := params.To if to.Function != "" { - if _, err := ds.client.SetFunctionBreakpointsRequest([]string{to.Function}); err != nil { + if _, err := ds.addFunctionBreakpoint(to.Function); err != nil { return nil, nil, err } } else if to.File != "" && to.Line > 0 { @@ -539,8 +581,8 @@ func (ds *debuggerSession) evaluateExpression(ctx context.Context, _ *mcp.CallTo // SetVariableParams defines the parameters for setting a variable. type SetVariableParams struct { VariablesReference FlexInt `json:"variablesReference" mcp:"reference to the variable container"` - Name string `json:"name" mcp:"name of the variable to set"` - Value string `json:"value" mcp:"new value for the variable"` + Name string `json:"name" mcp:"name of the variable to set"` + Value string `json:"value" mcp:"new value for the variable"` } // setVariable sets the value of a variable in the debugged program. @@ -826,6 +868,7 @@ func (ds *debuggerSession) cleanup() { ds.capabilities = dap.Capabilities{} ds.stoppedThreadID = 0 ds.lastFrameID = -1 + ds.functionBreakpoints = nil ds.unregisterSessionTools() } @@ -1025,7 +1068,7 @@ initialized: // Set breakpoints for _, bp := range params.Breakpoints { if bp.Function != "" { - seq, err := ds.client.SetFunctionBreakpointsRequest([]string{bp.Function}) + seq, err := ds.addFunctionBreakpoint(bp.Function) if err != nil { return nil, nil, err } @@ -1322,7 +1365,7 @@ func stopSummary(full *mcp.CallToolResult, reason string) *mcp.CallToolResult { if reason != "" { fmt.Fprintf(&summary, "Stopped: %s\n", reason) } - for _, line := range strings.Split(text, "\n") { + for line := range strings.SplitSeq(text, "\n") { if strings.HasPrefix(line, "Function:") || strings.HasPrefix(line, "File:") { summary.WriteString(line + "\n") } @@ -1392,7 +1435,7 @@ func (ds *debuggerSession) breakpoint(ctx context.Context, _ *mcp.CallToolReques } if params.Function != "" { - seq, err := ds.client.SetFunctionBreakpointsRequest([]string{params.Function}) + seq, err := ds.addFunctionBreakpoint(params.Function) if err != nil { return nil, nil, err } diff --git a/tools_test.go b/tools_test.go index b671b66..71ee2d0 100644 --- a/tools_test.go +++ b/tools_test.go @@ -207,14 +207,14 @@ func (ts *testSetup) getContextContent(t *testing.T) string { } // Extract context content - contextStr := "" + var contextStr strings.Builder for _, content := range contextResult.Content { if textContent, ok := content.(*mcp.TextContent); ok { - contextStr += textContent.Text + contextStr.WriteString(textContent.Text) } } - return contextStr + return contextStr.String() } // stopDebugger stops the debugger @@ -355,15 +355,15 @@ func TestBasic(t *testing.T) { } // Check if the result contains "hello, world" - resultStr := "" + var resultStr strings.Builder for _, content := range evaluateResult.Content { if textContent, ok := content.(*mcp.TextContent); ok { - resultStr += textContent.Text + resultStr.WriteString(textContent.Text) } } - if !strings.Contains(resultStr, "hello, world") { - t.Errorf("Expected evaluation to contain 'hello, world', got: %s", resultStr) + if !strings.Contains(resultStr.String(), "hello, world") { + t.Errorf("Expected evaluation to contain 'hello, world', got: %s", resultStr.String()) } // Stop debugger @@ -440,15 +440,15 @@ func TestRestart(t *testing.T) { t.Logf("Evaluate after restart result: %v", evaluateResult2) // Verify the evaluation result still contains "hello, world" - resultStr := "" + var resultStr strings.Builder for _, content := range evaluateResult2.Content { if textContent, ok := content.(*mcp.TextContent); ok { - resultStr += textContent.Text + resultStr.WriteString(textContent.Text) } } - if !strings.Contains(resultStr, "hello me, its me again") { - t.Errorf("Expected evaluation after restart to contain 'hello me, its me again', got: %s", resultStr) + if !strings.Contains(resultStr.String(), "hello me, its me again") { + t.Errorf("Expected evaluation after restart to contain 'hello me, its me again', got: %s", resultStr.String()) } // Stop debugger @@ -995,14 +995,14 @@ func TestGDBEvaluate(t *testing.T) { } t.Logf("Evaluate result: %v", evalResult) - resultStr := "" + var resultStr strings.Builder for _, content := range evalResult.Content { if tc, ok := content.(*mcp.TextContent); ok { - resultStr += tc.Text + resultStr.WriteString(tc.Text) } } - if !strings.Contains(resultStr, "30") { - t.Errorf("Expected evaluation to contain '30', got: %s", resultStr) + if !strings.Contains(resultStr.String(), "30") { + t.Errorf("Expected evaluation to contain '30', got: %s", resultStr.String()) } ts.stopDebugger(t) @@ -1342,13 +1342,13 @@ func (ts *testSetup) callTool(t *testing.T, name string, args map[string]any) (s if err != nil { t.Fatalf("Failed to call tool %s: %v", name, err) } - var text string + var text strings.Builder for _, content := range result.Content { if tc, ok := content.(*mcp.TextContent); ok { - text += tc.Text + text.WriteString(tc.Text) } } - return text, result.IsError + return text.String(), result.IsError } func TestClearBreakpoints(t *testing.T) { @@ -1474,11 +1474,11 @@ func TestDisassemble(t *testing.T) { contextStr := ts.getContextContent(t) var addr string - for _, line := range strings.Split(contextStr, "\n") { - if idx := strings.Index(line, "[ip: "); idx >= 0 { - rest := line[idx+5:] - if end := strings.Index(rest, "]"); end >= 0 { - addr = rest[:end] + for line := range strings.SplitSeq(contextStr, "\n") { + if _, after, ok := strings.Cut(line, "[ip: "); ok { + rest := after + if before, _, ok := strings.Cut(rest, "]"); ok { + addr = before break } } From e9d2c45c264c69637a51c05ef6e00f3d1230cdf0 Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Tue, 9 Jun 2026 11:21:54 -0700 Subject: [PATCH 2/3] Add comprehensive tests for DAP-compliant line breakpoint tracking Adds five new tests to verify the DAP spec compliance for line breakpoints: - TestLineBreakpointTracking: Validates that multiple breakpoints in the same file are correctly tracked and both are hit during execution - TestClearSpecificLineBreakpoint: Tests clearing individual line breakpoints via file+line while preserving other breakpoints in the same file - TestMultipleFilesLineBreakpoints: Confirms that breakpoints across different files are tracked independently - TestClearAllBreakpointsAcrossFiles: Verifies that clearing all breakpoints removes both line and function breakpoints across all files - TestDuplicateLineBreakpoint: Ensures idempotency - setting the same breakpoint twice doesn't create duplicates These tests validate the DAP replace semantics where each setBreakpoints request sends the complete list of breakpoints for a file, rather than incrementally adding/removing individual breakpoints. Co-Authored-By: Claude Sonnet 4.5 --- tools.go | 122 ++++++++++++++++++++--- tools_test.go | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 379 insertions(+), 14 deletions(-) diff --git a/tools.go b/tools.go index d5eb013..5d67997 100644 --- a/tools.go +++ b/tools.go @@ -32,6 +32,7 @@ type debuggerSession struct { lastFrameID int // frame ID from last getFullContext; -1 means not set (0 is valid for GDB) protocolLogFile *os.File // protocol log file (closed on cleanup) functionBreakpoints []string // tracked function breakpoints (DAP replaces all on each request) + lineBreakpoints map[string][]int // tracked line breakpoints per file (DAP replaces all on each setBreakpoints call) } // defaultThreadID returns the thread ID to use when none is specified. @@ -69,6 +70,62 @@ func (ds *debuggerSession) clearFunctionBreakpoints() (int, error) { return ds.client.SetFunctionBreakpointsRequest([]string{}) } +// addLineBreakpoint adds a line breakpoint for the given file and sends the +// full list for that file to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) addLineBreakpoint(file string, line int) (int, error) { + if ds.lineBreakpoints == nil { + ds.lineBreakpoints = make(map[string][]int) + } + lines := ds.lineBreakpoints[file] + if !slices.Contains(lines, line) { + lines = append(lines, line) + ds.lineBreakpoints[file] = lines + } + return ds.client.SetBreakpointsRequest(file, ds.lineBreakpoints[file]) +} + +// removeLineBreakpoint removes a line breakpoint for the given file and sends +// the updated list to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) removeLineBreakpoint(file string, line int) (int, error) { + if ds.lineBreakpoints != nil { + ds.lineBreakpoints[file] = slices.DeleteFunc(ds.lineBreakpoints[file], func(l int) bool { + return l == line + }) + if len(ds.lineBreakpoints[file]) == 0 { + delete(ds.lineBreakpoints, file) + } + } + return ds.client.SetBreakpointsRequest(file, ds.lineBreakpoints[file]) +} + +// clearLineBreakpoints removes all tracked line breakpoints for a file and +// sends an empty list to the DAP server. Caller must hold ds.mu. +func (ds *debuggerSession) clearLineBreakpoints(file string) (int, error) { + if ds.lineBreakpoints != nil { + delete(ds.lineBreakpoints, file) + } + return ds.client.SetBreakpointsRequest(file, []int{}) +} + +// clearAllLineBreakpoints removes all tracked line breakpoints across all files. +// Caller must hold ds.mu. +func (ds *debuggerSession) clearAllLineBreakpoints() error { + if ds.lineBreakpoints == nil { + return nil + } + for file := range ds.lineBreakpoints { + seq, err := ds.client.SetBreakpointsRequest(file, []int{}) + if err != nil { + return err + } + if err := readAndValidateResponse(ds.client, seq, "unable to clear breakpoints"); err != nil { + return err + } + } + ds.lineBreakpoints = nil + return nil +} + const debugToolDescription = `Start a complete debugging session. Modes: 'source' (compile & debug), 'binary' (debug executable), 'core' (debug core dump), 'attach' (connect to process). @@ -140,9 +197,9 @@ Examples: {"file": "/path/to/main.go", "line": 42} or {"function": "main.process }, ds.breakpoint) mcp.AddTool(ds.server, &mcp.Tool{ Name: "clear-breakpoints", - Description: `Remove breakpoints. Provide 'file' to clear breakpoints in a specific file, 'function' to clear a specific function breakpoint, or 'all': true to clear all breakpoints. + Description: `Remove breakpoints. Provide 'file' to clear all breakpoints in a file, 'file'+'line' to clear a specific line breakpoint, 'function' to clear a function breakpoint, or 'all': true to clear all breakpoints. -Examples: {"file": "/path/to/main.go"} or {"function": "main.processData"} or {"all": true}`, +Examples: {"file": "/path/to/main.go"} or {"file": "/path/to/main.go", "line": 42} or {"function": "main.processData"} or {"all": true}`, }, ds.clearBreakpoints) mcp.AddTool(ds.server, &mcp.Tool{ Name: "continue", @@ -354,9 +411,10 @@ func readTypedResponse[T dap.ResponseMessage](client *DAPClient, requestSeq int) // ClearBreakpointsParams defines parameters for clearing breakpoints. type ClearBreakpointsParams struct { - File string `json:"file,omitempty" mcp:"clear all breakpoints in this file"` - Function string `json:"function,omitempty" mcp:"clear a function breakpoint by name"` - All bool `json:"all,omitempty" mcp:"clear all breakpoints"` + File string `json:"file,omitempty" mcp:"clear all breakpoints in this file, or a specific line if 'line' is also provided"` + Line FlexInt `json:"line,omitempty" mcp:"clear the breakpoint at this line (requires 'file')"` + Function string `json:"function,omitempty" mcp:"clear a function breakpoint by name"` + All bool `json:"all,omitempty" mcp:"clear all breakpoints"` } // StopParams defines parameters for stopping the debug session. @@ -373,6 +431,10 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool } if params.All { + // Clear all line breakpoints across all files + if err := ds.clearAllLineBreakpoints(); err != nil { + return nil, nil, err + } // Clear all function breakpoints seq, err := ds.clearFunctionBreakpoints() if err != nil { @@ -400,8 +462,19 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool } if params.File != "" { - // Clear breakpoints in specific file by setting empty list - seq, err := ds.client.SetBreakpointsRequest(params.File, []int{}) + if params.Line.Int() > 0 { + seq, err := ds.removeLineBreakpoint(params.File, params.Line.Int()) + if err != nil { + return nil, nil, err + } + if err := readAndValidateResponse(ds.client, seq, "unable to clear breakpoint"); err != nil { + return nil, nil, err + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("Cleared breakpoint at %s:%d", params.File, params.Line.Int())}}, + }, nil, nil + } + seq, err := ds.clearLineBreakpoints(params.File) if err != nil { return nil, nil, err } @@ -413,7 +486,7 @@ func (ds *debuggerSession) clearBreakpoints(ctx context.Context, _ *mcp.CallTool }, nil, nil } - return nil, nil, fmt.Errorf("specify 'file', 'function', or 'all'") + return nil, nil, fmt.Errorf("specify 'file' (optionally with 'line'), 'function', or 'all'") } // ContinueParams defines the parameters for continuing execution. @@ -439,7 +512,7 @@ func (ds *debuggerSession) continueExecution(ctx context.Context, _ *mcp.CallToo return nil, nil, err } } else if to.File != "" && to.Line > 0 { - if _, err := ds.client.SetBreakpointsRequest(to.File, []int{to.Line}); err != nil { + if _, err := ds.addLineBreakpoint(to.File, to.Line); err != nil { return nil, nil, err } } @@ -869,6 +942,7 @@ func (ds *debuggerSession) cleanup() { ds.stoppedThreadID = 0 ds.lastFrameID = -1 ds.functionBreakpoints = nil + ds.lineBreakpoints = nil ds.unregisterSessionTools() } @@ -1076,7 +1150,7 @@ initialized: return nil, nil, err } } else if bp.File != "" && bp.Line > 0 { - seq, err := ds.client.SetBreakpointsRequest(bp.File, []int{bp.Line}) + seq, err := ds.addLineBreakpoint(bp.File, bp.Line) if err != nil { return nil, nil, err } @@ -1439,11 +1513,31 @@ func (ds *debuggerSession) breakpoint(ctx context.Context, _ *mcp.CallToolReques if err != nil { return nil, nil, err } - if err := readAndValidateResponse(ds.client, seq, "unable to set function breakpoint"); err != nil { - return nil, nil, err + resp, err := readTypedResponse[*dap.SetFunctionBreakpointsResponse](ds.client, seq) + if err != nil { + return nil, nil, fmt.Errorf("unable to set function breakpoint: %w", err) + } + // Find the breakpoint matching the requested function in the response. + // The response contains ALL function breakpoints; the new one is typically last. + var result string + for _, bp := range resp.Body.Breakpoints { + if !bp.Verified { + continue + } + // The last verified breakpoint is the one we just added (DAP appends in order) + if bp.Source != nil { + result = fmt.Sprintf("Breakpoint %d set at %s:%d (function %s)", bp.Id, bp.Source.Path, bp.Line, params.Function) + } else if bp.Line > 0 { + result = fmt.Sprintf("Breakpoint %d set at line %d (function %s)", bp.Id, bp.Line, params.Function) + } else { + result = fmt.Sprintf("Breakpoint %d set on function: %s", bp.Id, params.Function) + } + } + if result == "" { + result = fmt.Sprintf("Breakpoint set on function: %s", params.Function) } return &mcp.CallToolResult{ - Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("Breakpoint set on function: %s", params.Function)}}, + Content: []mcp.Content{&mcp.TextContent{Text: result}}, }, nil, nil } @@ -1451,7 +1545,7 @@ func (ds *debuggerSession) breakpoint(ctx context.Context, _ *mcp.CallToolReques return nil, nil, fmt.Errorf("either function or file+line is required") } - bpSeq, err := ds.client.SetBreakpointsRequest(params.File, []int{params.Line.Int()}) + bpSeq, err := ds.addLineBreakpoint(params.File, params.Line.Int()) if err != nil { return nil, nil, err } diff --git a/tools_test.go b/tools_test.go index 71ee2d0..4c636d3 100644 --- a/tools_test.go +++ b/tools_test.go @@ -1406,6 +1406,277 @@ func TestClearBreakpoints(t *testing.T) { ts.stopDebugger(t) } +// TestLineBreakpointTracking tests that line breakpoints are tracked correctly +// and sent as a complete list per file (DAP spec compliance). +func TestLineBreakpointTracking(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set first breakpoint at line 7 (x := 10) + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set first breakpoint: %s", text) + } + if !strings.Contains(text, "Breakpoint") { + t.Errorf("Expected breakpoint confirmation, got: %s", text) + } + t.Logf("Set first breakpoint: %s", text) + + // Set second breakpoint at line 13 (sum := x + y) + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 13}) + if isErr { + t.Fatalf("Failed to set second breakpoint: %s", text) + } + if !strings.Contains(text, "Breakpoint") { + t.Errorf("Expected breakpoint confirmation, got: %s", text) + } + t.Logf("Set second breakpoint: %s", text) + + // Continue and verify we can hit the first breakpoint + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("continue returned error: %s", text) + } + t.Logf("First continue: %s", text) + + // Verify we stopped at line 7 + contextStr := ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:7") { + t.Logf("Expected to be stopped at line 7, got: %s", contextStr) + } + + // Continue to second breakpoint + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("second continue returned error: %s", text) + } + t.Logf("Second continue: %s", text) + + // Verify we stopped at line 13 + contextStr = ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:13") { + t.Logf("Expected to be stopped at line 13, got: %s", contextStr) + } + + ts.stopDebugger(t) +} + +// TestClearSpecificLineBreakpoint tests clearing a single line breakpoint +// while preserving others in the same file. +func TestClearSpecificLineBreakpoint(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set two breakpoints in the same file at different executable lines + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set first breakpoint: %s", text) + } + t.Logf("Set first breakpoint: %s", text) + + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 13}) + if isErr { + t.Fatalf("Failed to set second breakpoint: %s", text) + } + t.Logf("Set second breakpoint: %s", text) + + // Clear only the line 7 breakpoint + text, isErr = ts.callTool(t, "clear-breakpoints", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("clear-breakpoints file+line returned error: %s", text) + } + if !strings.Contains(text, "Cleared breakpoint at") && !strings.Contains(text, ":7") { + t.Errorf("Expected confirmation of clearing line 7 breakpoint, got: %s", text) + } + t.Logf("Cleared line 7 breakpoint: %s", text) + + // Continue and verify we only hit the line 13 breakpoint + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("continue returned error: %s", text) + } + t.Logf("Continue after clearing line 7: %s", text) + + // Verify we stopped at line 13 + contextStr := ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:13") { + t.Logf("Context after continue (expected line 13): %s", contextStr) + } + + ts.stopDebugger(t) +} + +// TestMultipleFilesLineBreakpoints tests that breakpoints in different files +// are tracked independently. +func TestMultipleFilesLineBreakpoints(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + // Use a test program that has multiple files (we'll set breakpoints in main.go) + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f1 := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set multiple breakpoints in the file + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f1, "line": 7}) + if isErr { + t.Fatalf("Failed to set breakpoint in file 1: %s", text) + } + t.Logf("Set breakpoint in file 1: %s", text) + + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f1, "line": 13}) + if isErr { + t.Fatalf("Failed to set second breakpoint in file 1: %s", text) + } + t.Logf("Set second breakpoint in file 1: %s", text) + + // Clear breakpoints in first file only + text, isErr = ts.callTool(t, "clear-breakpoints", map[string]any{"file": f1}) + if isErr { + t.Fatalf("clear-breakpoints for file 1 returned error: %s", text) + } + if !strings.Contains(text, "Cleared breakpoints in") { + t.Errorf("Expected 'Cleared breakpoints in' message, got: %s", text) + } + t.Logf("Cleared file 1 breakpoints: %s", text) + + // Continue should not hit any breakpoints now + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("continue returned error: %s", text) + } + // Program should run to completion + if !strings.Contains(text, "terminated") && !strings.Contains(text, "exited") { + t.Logf("Continue result (expected termination): %s", text) + } + + ts.stopDebugger(t) +} + +// TestClearAllBreakpointsAcrossFiles tests that clearing all breakpoints +// removes breakpoints from all files. +func TestClearAllBreakpointsAcrossFiles(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set multiple breakpoints + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set first breakpoint: %s", text) + } + t.Logf("Set first breakpoint: %s", text) + + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 8}) + if isErr { + t.Fatalf("Failed to set second breakpoint: %s", text) + } + t.Logf("Set second breakpoint: %s", text) + + // Also set a function breakpoint + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"function": "main.main"}) + if isErr { + t.Fatalf("Failed to set function breakpoint: %s", text) + } + t.Logf("Set function breakpoint: %s", text) + + // Clear all breakpoints + text, isErr = ts.callTool(t, "clear-breakpoints", map[string]any{"all": true}) + if isErr { + t.Fatalf("clear-breakpoints all returned error: %s", text) + } + if !strings.Contains(text, "Cleared all breakpoints") { + t.Errorf("Expected 'Cleared all breakpoints' message, got: %s", text) + } + t.Logf("Cleared all breakpoints: %s", text) + + // Continue should not hit any breakpoints + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("continue returned error: %s", text) + } + // Program should run to completion + if !strings.Contains(text, "terminated") && !strings.Contains(text, "exited") { + t.Logf("Continue result (expected termination): %s", text) + } + + ts.stopDebugger(t) +} + +// TestDuplicateLineBreakpoint tests that setting the same breakpoint twice +// doesn't create duplicates (idempotency). +func TestDuplicateLineBreakpoint(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set breakpoint at line 7 + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set first breakpoint: %s", text) + } + t.Logf("Set first breakpoint: %s", text) + + // Set the same breakpoint again + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set duplicate breakpoint: %s", text) + } + t.Logf("Set duplicate breakpoint: %s", text) + + // Continue once - should hit the breakpoint + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("first continue returned error: %s", text) + } + t.Logf("First continue: %s", text) + + // Verify we stopped + contextStr := ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:7") { + t.Logf("Context after first stop (expected line 7): %s", contextStr) + } + + // Continue again - should not hit another breakpoint at same location + // (program may terminate or hit another breakpoint) + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("second continue returned error: %s", text) + } + t.Logf("Second continue: %s", text) + + ts.stopDebugger(t) +} + func TestInfo(t *testing.T) { ts := setupMCPServerAndClient(t) defer ts.cleanup() From 336d28ea1a003ece851f46b40883099038135261 Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Tue, 16 Jun 2026 16:08:35 -0700 Subject: [PATCH 3/3] Fix breakpoint tracking state drift and support pending breakpoints - Roll back tracked breakpoint list on DAP send failure in addFunctionBreakpoint and addLineBreakpoint to prevent state drift between the tracker and the adapter. - Remove unverified breakpoints from both the tracked list and the adapter when the DAP response reports them as failed. Previously, phantom breakpoints were left in the tracked list. - Support pending breakpoints (reason: 'pending') by keeping them in the tracked list and returning an informational message. This enables breakpoints on code in shared libraries or not-yet-loaded sources. - Upgrade go-dap to latest main (d7a2259b058b) which adds the Reason field to dap.Breakpoint, enabling pending vs failed distinction. - Fix TestClearAllBreakpointsAcrossFiles to use an executable line (line 10 instead of blank line 8). --- go.mod | 2 +- go.sum | 4 +- tools.go | 191 +++++++++++++++++++++++++++++++++++++++----------- tools_test.go | 68 +++++++++++++++++- 4 files changed, 219 insertions(+), 46 deletions(-) diff --git a/go.mod b/go.mod index 2d3cde5..7d17e32 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/go-delve/mcp-dap-server go 1.26.1 require ( - github.com/google/go-dap v0.12.0 + github.com/google/go-dap v0.12.1-0.20250904181021-d7a2259b058b github.com/modelcontextprotocol/go-sdk v1.6.0 ) diff --git a/go.sum b/go.sum index c5c6fae..c404ccb 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63Y github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/go-dap v0.12.0 h1:rVcjv3SyMIrpaOoTAdFDyHs99CwVOItIJGKLQFQhNeM= -github.com/google/go-dap v0.12.0/go.mod h1:tNjCASCm5cqePi/RVXXWEVqtnNLV1KTWtYOqu6rZNzc= +github.com/google/go-dap v0.12.1-0.20250904181021-d7a2259b058b h1:m+yLjBIoXaMKk6pwd1IJYYgM76+mwcy7J9+cuY7LqmQ= +github.com/google/go-dap v0.12.1-0.20250904181021-d7a2259b058b/go.mod h1:tNjCASCm5cqePi/RVXXWEVqtnNLV1KTWtYOqu6rZNzc= github.com/google/jsonschema-go v0.4.3 h1:/DBOLZTfDow7pe2GmaJNhltueGTtDKICi8V8p+DQPd0= github.com/google/jsonschema-go v0.4.3/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/modelcontextprotocol/go-sdk v1.6.0 h1:PPLS3kn7WtOEnR+Af4X5H96SG0qSab8R/ZQT/HkhPkY= diff --git a/tools.go b/tools.go index 5d67997..32227c2 100644 --- a/tools.go +++ b/tools.go @@ -46,12 +46,19 @@ func (ds *debuggerSession) defaultThreadID() int { // addFunctionBreakpoint appends a function breakpoint (if not already tracked) // and sends the full list to the DAP server. Caller must hold ds.mu. -func (ds *debuggerSession) addFunctionBreakpoint(name string) (int, error) { +// Returns (seq, alreadyExists, error). When alreadyExists is true, no DAP +// request was sent and seq is 0. +func (ds *debuggerSession) addFunctionBreakpoint(name string) (int, bool, error) { if slices.Contains(ds.functionBreakpoints, name) { - return ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) + return 0, true, nil } ds.functionBreakpoints = append(ds.functionBreakpoints, name) - return ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) + seq, err := ds.client.SetFunctionBreakpointsRequest(ds.functionBreakpoints) + if err != nil { + ds.functionBreakpoints = ds.functionBreakpoints[:len(ds.functionBreakpoints)-1] + return 0, false, err + } + return seq, false, nil } // removeFunctionBreakpoint removes a function breakpoint by name @@ -72,16 +79,28 @@ func (ds *debuggerSession) clearFunctionBreakpoints() (int, error) { // addLineBreakpoint adds a line breakpoint for the given file and sends the // full list for that file to the DAP server. Caller must hold ds.mu. -func (ds *debuggerSession) addLineBreakpoint(file string, line int) (int, error) { +// Returns (seq, alreadyExists, error). When alreadyExists is true, no DAP +// request was sent and seq is 0. +func (ds *debuggerSession) addLineBreakpoint(file string, line int) (int, bool, error) { if ds.lineBreakpoints == nil { ds.lineBreakpoints = make(map[string][]int) } lines := ds.lineBreakpoints[file] - if !slices.Contains(lines, line) { - lines = append(lines, line) - ds.lineBreakpoints[file] = lines + if slices.Contains(lines, line) { + return 0, true, nil } - return ds.client.SetBreakpointsRequest(file, ds.lineBreakpoints[file]) + lines = append(lines, line) + ds.lineBreakpoints[file] = lines + seq, err := ds.client.SetBreakpointsRequest(file, ds.lineBreakpoints[file]) + if err != nil { + // Roll back: remove the line we just appended + ds.lineBreakpoints[file] = ds.lineBreakpoints[file][:len(ds.lineBreakpoints[file])-1] + if len(ds.lineBreakpoints[file]) == 0 { + delete(ds.lineBreakpoints, file) + } + return 0, false, err + } + return seq, false, nil } // removeLineBreakpoint removes a line breakpoint for the given file and sends @@ -505,19 +524,45 @@ func (ds *debuggerSession) continueExecution(ctx context.Context, _ *mcp.CallToo } // If "to" is specified, set a temporary breakpoint + var cleanupRunToCursor func() error if params.To != nil { to := params.To + var alreadyExists bool if to.Function != "" { - if _, err := ds.addFunctionBreakpoint(to.Function); err != nil { + var err error + _, alreadyExists, err = ds.addFunctionBreakpoint(to.Function) + if err != nil { return nil, nil, err } + if !alreadyExists { + cleanupRunToCursor = func() error { + seq, err := ds.removeFunctionBreakpoint(to.Function) + if err != nil { + return err + } + return readAndValidateResponse(ds.client, seq, "unable to remove run-to-cursor function breakpoint") + } + } } else if to.File != "" && to.Line > 0 { - if _, err := ds.addLineBreakpoint(to.File, to.Line); err != nil { + var err error + _, alreadyExists, err = ds.addLineBreakpoint(to.File, to.Line) + if err != nil { return nil, nil, err } + if !alreadyExists { + cleanupRunToCursor = func() error { + seq, err := ds.removeLineBreakpoint(to.File, to.Line) + if err != nil { + return err + } + return readAndValidateResponse(ds.client, seq, "unable to remove run-to-cursor line breakpoint") + } + } } - if _, err := ds.client.ReadMessage(); err != nil { - return nil, nil, err + if !alreadyExists { + if _, err := ds.client.ReadMessage(); err != nil { + return nil, nil, err + } } } @@ -530,6 +575,8 @@ func (ds *debuggerSession) continueExecution(ctx context.Context, _ *mcp.CallToo return nil, nil, err } + var result *mcp.CallToolResult + var resultErr error for { msg, err := ds.client.ReadMessage() if err != nil { @@ -547,17 +594,27 @@ func (ds *debuggerSession) continueExecution(ctx context.Context, _ *mcp.CallToo } case *dap.StoppedEvent: ds.stoppedThreadID = resp.Body.ThreadId - result, err := ds.getFullContext(resp.Body.ThreadId, 0, 20) - if err != nil || params.FullContext { - return result, nil, err + result, resultErr = ds.getFullContext(resp.Body.ThreadId, 0, 20) + if resultErr == nil && !params.FullContext { + result = stopSummary(result, resp.Body.Reason) } - return stopSummary(result, resp.Body.Reason), nil, nil + goto done case *dap.TerminatedEvent: - return &mcp.CallToolResult{ + result = &mcp.CallToolResult{ Content: []mcp.Content{&mcp.TextContent{Text: "Program terminated"}}, - }, nil, nil + } + goto done + } + } + +done: + // Remove the temporary run-to-cursor breakpoint if one was added + if cleanupRunToCursor != nil { + if err := cleanupRunToCursor(); err != nil { + log.Printf("continueExecution: failed to clean up run-to-cursor breakpoint: %v", err) } } + return result, nil, resultErr } // PauseParams defines the parameters for pausing execution. @@ -1142,20 +1199,24 @@ initialized: // Set breakpoints for _, bp := range params.Breakpoints { if bp.Function != "" { - seq, err := ds.addFunctionBreakpoint(bp.Function) + seq, alreadyExists, err := ds.addFunctionBreakpoint(bp.Function) if err != nil { return nil, nil, err } - if err := readAndValidateResponse(ds.client, seq, "unable to set function breakpoint"); err != nil { - return nil, nil, err + if !alreadyExists { + if err := readAndValidateResponse(ds.client, seq, "unable to set function breakpoint"); err != nil { + return nil, nil, err + } } } else if bp.File != "" && bp.Line > 0 { - seq, err := ds.addLineBreakpoint(bp.File, bp.Line) + seq, alreadyExists, err := ds.addLineBreakpoint(bp.File, bp.Line) if err != nil { return nil, nil, err } - if err := readAndValidateResponse(ds.client, seq, "unable to set breakpoint"); err != nil { - return nil, nil, err + if !alreadyExists { + if err := readAndValidateResponse(ds.client, seq, "unable to set breakpoint"); err != nil { + return nil, nil, err + } } } } @@ -1509,32 +1570,53 @@ func (ds *debuggerSession) breakpoint(ctx context.Context, _ *mcp.CallToolReques } if params.Function != "" { - seq, err := ds.addFunctionBreakpoint(params.Function) + seq, alreadyExists, err := ds.addFunctionBreakpoint(params.Function) if err != nil { return nil, nil, err } + if alreadyExists { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("Breakpoint already set on function: %s", params.Function)}}, + }, nil, nil + } resp, err := readTypedResponse[*dap.SetFunctionBreakpointsResponse](ds.client, seq) if err != nil { return nil, nil, fmt.Errorf("unable to set function breakpoint: %w", err) } - // Find the breakpoint matching the requested function in the response. - // The response contains ALL function breakpoints; the new one is typically last. - var result string - for _, bp := range resp.Body.Breakpoints { - if !bp.Verified { - continue + // The response breakpoints array corresponds 1:1 with the request array. + // We appended the new function last, so our breakpoint is the last element. + if len(resp.Body.Breakpoints) == 0 { + return nil, nil, fmt.Errorf("no breakpoints returned") + } + bp := resp.Body.Breakpoints[len(resp.Body.Breakpoints)-1] + if !bp.Verified { + if bp.Reason == "pending" { + // Pending breakpoints may be verified later (e.g. when a shared library loads). + // Keep them in the tracked list. + msg := fmt.Sprintf("Breakpoint set on function %s (pending — will resolve when the source is loaded)", params.Function) + if bp.Message != "" { + msg = fmt.Sprintf("Breakpoint set on function %s (pending: %s)", params.Function, bp.Message) + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: msg}}, + }, nil, nil } - // The last verified breakpoint is the one we just added (DAP appends in order) - if bp.Source != nil { - result = fmt.Sprintf("Breakpoint %d set at %s:%d (function %s)", bp.Id, bp.Source.Path, bp.Line, params.Function) - } else if bp.Line > 0 { - result = fmt.Sprintf("Breakpoint %d set at line %d (function %s)", bp.Id, bp.Line, params.Function) - } else { - result = fmt.Sprintf("Breakpoint %d set on function: %s", bp.Id, params.Function) + // Failed or unknown reason — remove from both our tracking and the adapter. + removeSeq, removeErr := ds.removeFunctionBreakpoint(params.Function) + if removeErr != nil { + log.Printf("breakpoint: failed to remove unverified function breakpoint %q: %v", params.Function, removeErr) + } else if removeErr = readAndValidateResponse(ds.client, removeSeq, "unable to remove unverified function breakpoint"); removeErr != nil { + log.Printf("breakpoint: failed to remove unverified function breakpoint %q: %v", params.Function, removeErr) } + return nil, nil, fmt.Errorf("function breakpoint not verified: %s", bp.Message) } - if result == "" { - result = fmt.Sprintf("Breakpoint set on function: %s", params.Function) + var result string + if bp.Source != nil { + result = fmt.Sprintf("Breakpoint %d set at %s:%d (function %s)", bp.Id, bp.Source.Path, bp.Line, params.Function) + } else if bp.Line > 0 { + result = fmt.Sprintf("Breakpoint %d set at line %d (function %s)", bp.Id, bp.Line, params.Function) + } else { + result = fmt.Sprintf("Breakpoint %d set on function: %s", bp.Id, params.Function) } return &mcp.CallToolResult{ Content: []mcp.Content{&mcp.TextContent{Text: result}}, @@ -1545,20 +1627,45 @@ func (ds *debuggerSession) breakpoint(ctx context.Context, _ *mcp.CallToolReques return nil, nil, fmt.Errorf("either function or file+line is required") } - bpSeq, err := ds.addLineBreakpoint(params.File, params.Line.Int()) + bpSeq, alreadyExists, err := ds.addLineBreakpoint(params.File, params.Line.Int()) if err != nil { return nil, nil, err } + if alreadyExists { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("Breakpoint already set at %s:%d", params.File, params.Line.Int())}}, + }, nil, nil + } resp, err := readTypedResponse[*dap.SetBreakpointsResponse](ds.client, bpSeq) if err != nil { return nil, nil, fmt.Errorf("unable to set breakpoint: %w", err) } + // The response breakpoints array corresponds 1:1 with the request array. + // We appended the new line last, so our breakpoint is the last element. if len(resp.Body.Breakpoints) == 0 { return nil, nil, fmt.Errorf("no breakpoints returned") } - bp := resp.Body.Breakpoints[0] + bp := resp.Body.Breakpoints[len(resp.Body.Breakpoints)-1] if !bp.Verified { + if bp.Reason == "pending" { + // Pending breakpoints may be verified later (e.g. when a shared library loads). + // Keep them in the tracked list. + msg := fmt.Sprintf("Breakpoint set at %s:%d (pending — will resolve when the source is loaded)", params.File, params.Line.Int()) + if bp.Message != "" { + msg = fmt.Sprintf("Breakpoint set at %s:%d (pending: %s)", params.File, params.Line.Int(), bp.Message) + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: msg}}, + }, nil, nil + } + // Failed or unknown reason — remove from both our tracking and the adapter. + removeSeq, removeErr := ds.removeLineBreakpoint(params.File, params.Line.Int()) + if removeErr != nil { + log.Printf("breakpoint: failed to remove unverified breakpoint at %s:%d: %v", params.File, params.Line.Int(), removeErr) + } else if removeErr = readAndValidateResponse(ds.client, removeSeq, "unable to remove unverified breakpoint"); removeErr != nil { + log.Printf("breakpoint: failed to remove unverified breakpoint at %s:%d: %v", params.File, params.Line.Int(), removeErr) + } return nil, nil, fmt.Errorf("breakpoint not verified: %s", bp.Message) } return &mcp.CallToolResult{ diff --git a/tools_test.go b/tools_test.go index 4c636d3..e92b830 100644 --- a/tools_test.go +++ b/tools_test.go @@ -1590,7 +1590,7 @@ func TestClearAllBreakpointsAcrossFiles(t *testing.T) { } t.Logf("Set first breakpoint: %s", text) - text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 8}) + text, isErr = ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 10}) if isErr { t.Fatalf("Failed to set second breakpoint: %s", text) } @@ -1677,6 +1677,72 @@ func TestDuplicateLineBreakpoint(t *testing.T) { ts.stopDebugger(t) } +// TestRunToCursorCleansUpBreakpoint verifies that a run-to-cursor breakpoint +// is removed after the program stops, so it doesn't trigger on subsequent continues. +func TestRunToCursorCleansUpBreakpoint(t *testing.T) { + ts := setupMCPServerAndClient(t) + defer ts.cleanup() + + binaryPath, cleanupBinary := compileTestProgram(t, ts.cwd, "step") + defer cleanupBinary() + + ts.startDebugSession(t, "0", binaryPath, nil) + + f := filepath.Join(ts.cwd, "testdata", "go", "step", "main.go") + + // Set a persistent breakpoint at line 7 (x := 10) + text, isErr := ts.callTool(t, "breakpoint", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("Failed to set breakpoint: %s", text) + } + t.Logf("Set breakpoint at line 7: %s", text) + + // Continue to hit the breakpoint at line 7 + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("continue returned error: %s", text) + } + contextStr := ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:7") { + t.Fatalf("Expected to be stopped at line 7, got: %s", contextStr) + } + t.Logf("Stopped at line 7") + + // Run-to-cursor to line 13 (sum := x + y) — this should set a temporary breakpoint + text, isErr = ts.callTool(t, "continue", map[string]any{ + "to": map[string]any{"file": f, "line": 13}, + }) + if isErr { + t.Fatalf("run-to-cursor returned error: %s", text) + } + contextStr = ts.getContextContent(t) + if !strings.Contains(contextStr, "main.go:13") { + t.Fatalf("Expected to be stopped at line 13, got: %s", contextStr) + } + t.Logf("Run-to-cursor stopped at line 13") + + // Now clear the persistent breakpoint at line 7 + text, isErr = ts.callTool(t, "clear-breakpoints", map[string]any{"file": f, "line": 7}) + if isErr { + t.Fatalf("clear-breakpoints returned error: %s", text) + } + t.Logf("Cleared persistent breakpoint at line 7: %s", text) + + // Continue — if the run-to-cursor breakpoint at line 13 was properly cleaned up, + // the program should run to completion. If it wasn't cleaned up, we'd stop at + // line 13 again (which would be wrong). + text, isErr = ts.callTool(t, "continue", map[string]any{}) + if isErr { + t.Fatalf("final continue returned error: %s", text) + } + if !strings.Contains(text, "terminated") && !strings.Contains(text, "exited") { + t.Errorf("Expected program to terminate (run-to-cursor breakpoint should have been cleaned up), got: %s", text) + } + t.Logf("Final continue result: %s", text) + + ts.stopDebugger(t) +} + func TestInfo(t *testing.T) { ts := setupMCPServerAndClient(t) defer ts.cleanup()