From 5cfb2e0975108fd7526808794df1e008da589882 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Sat, 10 Oct 2026 14:29:56 +0200 Subject: [PATCH] Limit MCP result sizes and add an end-to-end test --- README.md | 4 +- cmd/dbgp/mcp.go | 42 ++++++++++++ cmd/dbgp/mcp_e2e_test.go | 128 +++++++++++++++++++++++++++++++++++ cmd/dbgp/mcp_inspect.go | 48 ++++++++++--- cmd/dbgp/mcp_inspect_test.go | 15 ++-- cmd/dbgp/mcp_limits_test.go | 116 +++++++++++++++++++++++++++++++ cmd/dbgp/mcp_output.go | 52 ++++++++++---- cmd/dbgp/mcp_output_test.go | 2 +- cmd/dbgp/mcp_test.go | 12 +++- server.go | 30 +++++++- server_test.go | 27 ++++++++ session.go | 14 ++++ testdata/php/large.php | 23 +++++++ 13 files changed, 479 insertions(+), 34 deletions(-) create mode 100644 cmd/dbgp/mcp_e2e_test.go create mode 100644 cmd/dbgp/mcp_limits_test.go create mode 100644 testdata/php/large.php diff --git a/README.md b/README.md index d2ac2f6..97f6bfb 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,9 @@ claude mcp add dbgp -- dbgp mcp Unlike the interactive mode, it does not listen for PHP connections until the agent calls the `listen` tool, so PHP runs at full speed until there is something to debug; `-listen` starts listening right away. `-addr`, `-idekey` and `-map LOCAL=REMOTE` work as in the interactive mode. -Tools so far: `status` (listening state, sessions, breakpoints and path mappings), `listen` and `unlisten`; `add_breakpoint`, `remove_breakpoint` and `breakpoints`; `add_path_mapping` and `path_mappings`; `sessions`, `wait_for_session`, `continue` (run or step), `wait_for_stop`, `stop` and `detach`; `stack`, `variables`, `variable` (one variable, a page of its contents at a time) and `variable_value`; `source`, `output` and `warnings` (what the script printed and the PHP warnings it raised, each read incrementally); `eval` and `set_variable`, which run PHP code in the debugged script and can be left out with `-no-eval`. +Tools: `status` (listening state, sessions, breakpoints and path mappings), `listen` and `unlisten`; `add_breakpoint`, `remove_breakpoint` and `breakpoints`; `add_path_mapping` and `path_mappings`; `sessions`, `wait_for_session`, `continue` (run or step), `wait_for_stop`, `stop` and `detach`; `stack`, `variables`, `variable` (one variable, a page of its contents at a time) and `variable_value`; `source`, `output` and `warnings` (what the script printed and the PHP warnings it raised, each read incrementally); `eval` and `set_variable`, which run PHP code in the debugged script and can be left out with `-no-eval`. + +Results are kept small enough for an agent's context: stack frames and variables come in pages of 50, output and long values in parts of 32 KB, warnings 50 at a time with repeats counted once, and long source lines are cut at 500 bytes. The 50 most recent ended sessions are kept, with their output. ## Testing diff --git a/cmd/dbgp/mcp.go b/cmd/dbgp/mcp.go index b5b1705..7525e61 100644 --- a/cmd/dbgp/mcp.go +++ b/cmd/dbgp/mcp.go @@ -11,6 +11,7 @@ import ( "strings" "sync" "syscall" + "unicode/utf8" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -28,6 +29,18 @@ going on, listen before starting the PHP code to debug, and unlisten when done. Remove each breakpoint with remove_breakpoint as soon as you no longer need it: breakpoints apply to every later session and slow it down. +A typical run: add_breakpoint, listen, start the PHP code, wait_for_session, +then continue to run to the breakpoint. Start PHP in the background, or send +the web request without waiting for its answer: PHP waits for dbgp at every +stop, so it does not finish while you are debugging it. Once stopped, look +around with stack, variables, variable and source, and step or run on with +continue. output and warnings show what the script printed and the PHP +warnings it raised. End every session with stop or detach when you are done +with it: until then its PHP process or web server worker is held. + +Long results come in parts: lists in pages, see page and pages, and long text +when more or next is set. Ask only for the parts you need. + Before starting PHP, check which debugger it has with php -v: it prints "with PHP Debugger" or "with Xdebug". PHP Debugger is always in debug mode and starts debugging every request by itself, so do not set xdebug.mode or @@ -191,6 +204,35 @@ func outputSchema[T any](lists ...string) *jsonschema.Schema { return schema } +// Tool results are kept small enough for an agent's context: longer text +// and lists are returned in parts, which the agent asks for one at a time. +const ( + maxTextBytes = 32 << 10 // output, and values from variable_value + maxLineBytes = 500 // each line from source + maxListItems = 50 // stack frames, variables and warnings +) + +// cutText returns at most limit bytes of s, cut at a character boundary. +func cutText(s string, limit int) string { + if len(s) <= limit { + return s + } + for limit > 0 && !utf8.RuneStart(s[limit]) { + limit-- + } + return s[:limit] +} + +// pageOf returns page n of items, maxListItems per page, and how many pages +// there are; there is always at least one, maybe empty. +func pageOf[T any](items []T, n int) ([]T, int, error) { + pages := max(1, (len(items)+maxListItems-1)/maxListItems) + if n < 0 || n >= pages { + return nil, pages, fmt.Errorf("page must be from 0 to %d", pages-1) + } + return items[n*maxListItems : min(len(items), (n+1)*maxListItems)], pages, nil +} + // tools implements the MCP tools on a dbgp server. type tools struct { srv *dbgp.Server diff --git a/cmd/dbgp/mcp_e2e_test.go b/cmd/dbgp/mcp_e2e_test.go new file mode 100644 index 0000000..52e3ccb --- /dev/null +++ b/cmd/dbgp/mcp_e2e_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "context" + "net" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// A whole debugging run through the dbgp binary over stdio, as an agent +// does it: from listening to a finished script and nothing left behind. +func TestMCPEndToEnd(t *testing.T) { + requireEngine(t) + bin := buildBinary(t) + script, err := filepath.Abs("../../testdata/php/basic.php") + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + transport := &mcp.CommandTransport{Command: exec.CommandContext(ctx, bin, "mcp", "-addr", "127.0.0.1:0")} + cs, err := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "0"}, nil).Connect(ctx, transport, nil) + if err != nil { + t.Fatal(err) + } + defer func() { _ = cs.Close() }() + + var bp addBreakpointOutput + callTool(t, cs, "add_breakpoint", map[string]any{"file": script, "line": 24}, &bp) + var listening listenOutput + callTool(t, cs, "listen", nil, &listening) + _, port, err := net.SplitHostPort(listening.Address) + if err != nil { + t.Fatal(err) + } + + // The script runs in the background while it is debugged. + php := exec.Command("php", "-dxdebug.mode=debug", "-dxdebug.start_with_request=yes", + "-dxdebug.client_host=127.0.0.1", "-dxdebug.client_port="+port, "-ddisplay_errors=0", script) + if err := php.Start(); err != nil { + t.Fatal(err) + } + exited := make(chan error, 1) + go func() { exited <- php.Wait() }() + defer func() { _ = php.Process.Kill() }() + + var waited waitForSessionOutput + callTool(t, cs, "wait_for_session", map[string]any{"wait": 10}, &waited) + if !waited.Connected || waited.Session.Script != script { + t.Fatalf("wait_for_session = %+v", waited) + } + var st stateOutput + callTool(t, cs, "continue", nil, &st) + if st.Status != "break" || st.File != script || st.Line != 24 { + t.Fatalf("continue = %+v, want break at line 24", st) + } + + var stack stackOutput + callTool(t, cs, "stack", nil, &stack) + if len(stack.Frames) != 2 || stack.Frames[0].Function != "add" || stack.Frames[1].Line != 36 { + t.Errorf("stack = %+v", stack.Frames) + } + var vars variablesOutput + callTool(t, cs, "variables", nil, &vars) + if a := byName(vars.Variables)["$a"]; a.Value != "2" { + t.Errorf("$a = %+v", a) + } + var numbers variableOutput + callTool(t, cs, "variable", map[string]any{"name": "$numbers", "depth": 1}, &numbers) + if numbers.Variable.Size == nil || *numbers.Variable.Size != 40 { + t.Errorf("$numbers = %+v", numbers.Variable) + } + var result evalOutput + callTool(t, cs, "eval", map[string]any{"code": "$a + $b"}, &result) + if result.Result == nil || result.Result.Value != "5" { + t.Errorf("eval $a + $b = %+v", result.Result) + } + callTool(t, cs, "set_variable", map[string]any{"name": "$b", "value": "30"}, &setVariableOutput{}) + var source sourceOutput + callTool(t, cs, "source", nil, &source) + if len(source.Lines) == 0 { + t.Error("source returned no lines") + } + + callTool(t, cs, "continue", nil, &st) + if st.Status != "stopping" { + t.Fatalf("continue to the end = %+v, want stopping", st) + } + var printed outputOutput + callTool(t, cs, "output", nil, &printed) + if printed.Output != "x=32\ndone\n" { + t.Errorf("output = %q, want x=32 from the changed $b", printed.Output) + } + var raised warningsOutput + callTool(t, cs, "warnings", nil, &raised) + if len(raised.Warnings) != 1 || raised.Warnings[0].Line != 42 { + t.Errorf("warnings = %+v", raised.Warnings) + } + + // Cleaning up leaves nothing behind: PHP exits, and dbgp is idle. + callTool(t, cs, "stop", nil, &st) + if st.Status != "ended" { + t.Errorf("stop = %+v", st) + } + select { + case err := <-exited: + if err != nil { + t.Errorf("php: %v", err) + } + case <-time.After(10 * time.Second): + t.Error("php still running after stop") + } + callTool(t, cs, "remove_breakpoint", map[string]any{"id": bp.Breakpoint.ID}, &breakpointsOutput{}) + callTool(t, cs, "unlisten", nil, &unlistenOutput{}) + var status statusOutput + callTool(t, cs, "status", nil, &status) + if status.Listening || len(status.Breakpoints) != 0 || len(status.Sessions) != 1 || status.Sessions[0].Status != "ended" { + t.Errorf("status after cleaning up = %+v", status) + } + if conn, err := net.DialTimeout("tcp", "127.0.0.1:"+port, time.Second); err == nil { + _ = conn.Close() + t.Errorf("port %s still open after unlisten", port) + } +} diff --git a/cmd/dbgp/mcp_inspect.go b/cmd/dbgp/mcp_inspect.go index bfe2b67..b9a1928 100644 --- a/cmd/dbgp/mcp_inspect.go +++ b/cmd/dbgp/mcp_inspect.go @@ -14,13 +14,13 @@ import ( func (t *tools) addInspectTools(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "stack", - Description: "Show the call stack of a stopped session, innermost first.", + Description: "Show the call stack of a stopped session, innermost first, 50 frames per page.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, OutputSchema: outputSchema[stackOutput]("frames"), }, t.stack) mcp.AddTool(server, &mcp.Tool{ Name: "variables", - Description: "List the variables of a stack frame of a stopped session. Arrays and objects are " + + Description: "List the variables of a stack frame of a stopped session, 50 per page. Arrays and objects are " + "listed without their contents, with their size or class: use variable to look inside one.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, OutputSchema: outputSchema[variablesOutput]("variables"), @@ -35,7 +35,7 @@ func (t *tools) addInspectTools(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "variable_value", Description: "Show the whole value of a variable whose value variables or variable returned truncated, " + - "such as a long string.", + "such as a long string. A value longer than 32 KB is returned in parts: call again with offset set to next.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, }, t.variableValue) } @@ -47,12 +47,19 @@ type frameInfo struct { Line int `json:"line" jsonschema:"the line being executed in this frame"` } +type stackInput struct { + Session int `json:"session,omitempty" jsonschema:"the session id; defaults to the newest session that has not ended"` + Page int `json:"page,omitempty" jsonschema:"the page of frames, from 0 (the default); see pages"` +} + type stackOutput struct { Session int `json:"session" jsonschema:"the session id"` - Frames []frameInfo `json:"frames" jsonschema:"innermost first: depth 0 is where execution stopped"` + Frames []frameInfo `json:"frames" jsonschema:"one page of frames, innermost first: depth 0 is where execution stopped"` + Page int `json:"page" jsonschema:"this page, from 0"` + Pages int `json:"pages" jsonschema:"how many pages of frames there are; call stack with page for the others"` } -func (t *tools) stack(_ context.Context, _ *mcp.CallToolRequest, in sessionInput) (*mcp.CallToolResult, stackOutput, error) { +func (t *tools) stack(_ context.Context, _ *mcp.CallToolRequest, in stackInput) (*mcp.CallToolResult, stackOutput, error) { s, err := t.session(in.Session) if err != nil { return nil, stackOutput{}, err @@ -61,7 +68,11 @@ func (t *tools) stack(_ context.Context, _ *mcp.CallToolRequest, in sessionInput if err != nil { return nil, stackOutput{}, sessionError(s, err) } - out := stackOutput{Session: s.ID(), Frames: []frameInfo{}} + frames, pages, err := pageOf(frames, in.Page) + if err != nil { + return nil, stackOutput{}, err + } + out := stackOutput{Session: s.ID(), Frames: []frameInfo{}, Page: in.Page, Pages: pages} for _, f := range frames { out.Frames = append(out.Frames, frameInfo{Depth: f.Level, Function: f.Where, File: f.Filename, Line: f.Lineno}) } @@ -73,6 +84,7 @@ type frameInput struct { Session int `json:"session,omitempty" jsonschema:"the session id; defaults to the newest session that has not ended"` Depth int `json:"depth,omitempty" jsonschema:"the stack frame: 0 (the default) is where execution stopped, 1 its caller, and so on; see stack"` Context int `json:"context,omitempty" jsonschema:"0 (the default) local variables, 1 superglobals such as $_SERVER and $_GET, 2 user-defined constants"` + Page int `json:"page,omitempty" jsonschema:"the page of variables, from 0 (the default); see pages"` } type variableInfo struct { @@ -126,7 +138,9 @@ type variablesOutput struct { Session int `json:"session" jsonschema:"the session id"` Depth int `json:"depth" jsonschema:"the stack frame"` Context int `json:"context" jsonschema:"0 local variables, 1 superglobals, 2 user-defined constants"` - Variables []variableInfo `json:"variables" jsonschema:"the variables, without the contents of arrays and objects"` + Variables []variableInfo `json:"variables" jsonschema:"one page of the variables, without the contents of arrays and objects"` + Page int `json:"page" jsonschema:"this page, from 0"` + Pages int `json:"pages" jsonschema:"how many pages of variables there are; call variables with page for the others"` } func (t *tools) variables(_ context.Context, _ *mcp.CallToolRequest, in frameInput) (*mcp.CallToolResult, variablesOutput, error) { @@ -138,7 +152,11 @@ func (t *tools) variables(_ context.Context, _ *mcp.CallToolRequest, in frameInp if err != nil { return nil, variablesOutput{}, sessionError(s, err) } - out := variablesOutput{Session: s.ID(), Depth: in.Depth, Context: in.Context, Variables: []variableInfo{}} + props, pages, err := pageOf(props, in.Page) + if err != nil { + return nil, variablesOutput{}, err + } + out := variablesOutput{Session: s.ID(), Depth: in.Depth, Context: in.Context, Variables: []variableInfo{}, Page: in.Page, Pages: pages} for _, p := range props { out.Variables = append(out.Variables, toVariable(p)) } @@ -190,11 +208,14 @@ type variableValueInput struct { Name string `json:"name" jsonschema:"the variable, or a path into one, e.g. $html or $page[\"body\"]"` Depth int `json:"depth,omitempty" jsonschema:"the stack frame: 0 (the default) is where execution stopped, 1 its caller, and so on; see stack"` Context int `json:"context,omitempty" jsonschema:"0 (the default) local variables, 1 superglobals such as $_SERVER and $_GET, 2 user-defined constants"` + Offset int `json:"offset,omitempty" jsonschema:"the byte of the value to start from, 0 by default; see next"` } type variableValueOutput struct { Name string `json:"name" jsonschema:"the variable"` - Value string `json:"value" jsonschema:"its whole value"` + Value string `json:"value" jsonschema:"its value from offset, at most 32 KB of it"` + Size int `json:"size" jsonschema:"the length of the whole value, in bytes"` + Next int `json:"next,omitempty" jsonschema:"where the rest of the value starts: call variable_value again with this offset; absent when the value is complete"` } func (t *tools) variableValue(_ context.Context, _ *mcp.CallToolRequest, in variableValueInput) (*mcp.CallToolResult, variableValueOutput, error) { @@ -209,5 +230,12 @@ func (t *tools) variableValue(_ context.Context, _ *mcp.CallToolRequest, in vari if err != nil { return nil, variableValueOutput{}, inspectError(s, in.Name, in.Depth, err) } - return nil, variableValueOutput{Name: in.Name, Value: value}, nil + if in.Offset < 0 || in.Offset > len(value) { + return nil, variableValueOutput{}, fmt.Errorf("offset must be from 0 to %d, the size of %s", len(value), in.Name) + } + out := variableValueOutput{Name: in.Name, Value: cutText(value[in.Offset:], maxTextBytes), Size: len(value)} + if end := in.Offset + len(out.Value); end < len(value) { + out.Next = end + } + return nil, out, nil } diff --git a/cmd/dbgp/mcp_inspect_test.go b/cmd/dbgp/mcp_inspect_test.go index d1ffc0b..ccfa499 100644 --- a/cmd/dbgp/mcp_inspect_test.go +++ b/cmd/dbgp/mcp_inspect_test.go @@ -12,14 +12,21 @@ import ( // stoppedInAdd starts the debuggee through MCP and runs it to the // breakpoint in add(), at line 24. func stoppedInAdd(t *testing.T) (*mcp.ClientSession, *dbgp.Server, string) { + t.Helper() + return stoppedAt(t, "basic.php", 24) +} + +// stoppedAt starts a script from testdata/php through MCP and runs it to a +// breakpoint on line. +func stoppedAt(t *testing.T, name string, line int) (*mcp.ClientSession, *dbgp.Server, string) { t.Helper() requireEngine(t) cs, srv := newTestMCP(t) - script, err := filepath.Abs("../../testdata/php/basic.php") + script, err := filepath.Abs("../../testdata/php/" + name) if err != nil { t.Fatal(err) } - callTool(t, cs, "add_breakpoint", map[string]any{"file": script, "line": 24}, &addBreakpointOutput{}) + callTool(t, cs, "add_breakpoint", map[string]any{"file": script, "line": line}, &addBreakpointOutput{}) callTool(t, cs, "listen", nil, &listenOutput{}) startPHP(t, srv.Port(), script) var waited waitForSessionOutput @@ -29,8 +36,8 @@ func stoppedInAdd(t *testing.T) (*mcp.ClientSession, *dbgp.Server, string) { } var st stateOutput callTool(t, cs, "continue", nil, &st) - if st.Status != "break" || st.Line != 24 { - t.Fatalf("continue = %+v, want break at line 24", st) + if st.Status != "break" || st.Line != line { + t.Fatalf("continue = %+v, want break at line %d", st, line) } return cs, srv, script } diff --git a/cmd/dbgp/mcp_limits_test.go b/cmd/dbgp/mcp_limits_test.go new file mode 100644 index 0000000..c184a42 --- /dev/null +++ b/cmd/dbgp/mcp_limits_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "strings" + "testing" + "unicode/utf8" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// stoppedInLarge runs testdata/php/large.php to its deepest call, after it +// has printed its text and raised its warnings. +func stoppedInLarge(t *testing.T) *mcp.ClientSession { + t.Helper() + cs, _, _ := stoppedAt(t, "large.php", 8) + return cs +} + +func TestMCPLimitText(t *testing.T) { + cs := stoppedInLarge(t) + text := strings.Repeat("é", 50000) + + // Each part ends on a character boundary and the parts add up. + var value strings.Builder + var part variableValueOutput + for offset := 0; ; offset = part.Next { + callTool(t, cs, "variable_value", map[string]any{"name": "$text", "depth": 61, "offset": offset}, &part) + if len(part.Value) > maxTextBytes || !utf8.ValidString(part.Value) || part.Size != len(text) { + t.Fatalf("part at %d: %d bytes, size %d", offset, len(part.Value), part.Size) + } + value.WriteString(part.Value) + if part.Next == 0 { + break + } + } + if value.String() != text { + t.Errorf("variable_value parts make %d bytes, want %d", value.Len(), len(text)) + } + if msg := callToolError(t, cs, "variable_value", map[string]any{"name": "$text", "depth": 61, "offset": len(text) + 1}); !strings.Contains(msg, "offset must be from 0 to 100000") { + t.Errorf("offset past the end: %q", msg) + } + + var output strings.Builder + var printed outputOutput + calls := 0 + for { + callTool(t, cs, "output", nil, &printed) + calls++ + if len(printed.Output) > maxTextBytes || !utf8.ValidString(printed.Output) { + t.Fatalf("output part %d: %d bytes", calls, len(printed.Output)) + } + output.WriteString(printed.Output) + if !printed.More { + break + } + } + if output.String() != text || calls != 4 { + t.Errorf("output parts make %d bytes in %d calls, want %d in 4", output.Len(), calls, len(text)) + } +} + +func TestMCPLimitLists(t *testing.T) { + cs := stoppedInLarge(t) + + // 61 calls of down() and the script itself. + var stack stackOutput + callTool(t, cs, "stack", nil, &stack) + if len(stack.Frames) != maxListItems || stack.Pages != 2 || stack.Frames[0].Depth != 0 { + t.Errorf("stack page 0: %d frames of %d pages", len(stack.Frames), stack.Pages) + } + callTool(t, cs, "stack", map[string]any{"page": 1}, &stack) + if len(stack.Frames) != 12 || stack.Frames[0].Depth != 50 || stack.Frames[11].Function != "{main}" { + t.Errorf("stack page 1: %+v", stack.Frames) + } + if msg := callToolError(t, cs, "stack", map[string]any{"page": 2}); !strings.Contains(msg, "page must be from 0 to 1") { + t.Errorf("stack page 2: %q", msg) + } + + // $v1 to $v60, $text, $i and $line. + var vars variablesOutput + callTool(t, cs, "variables", map[string]any{"depth": 61}, &vars) + all := vars.Variables + if len(all) != maxListItems || vars.Pages != 2 { + t.Errorf("variables page 0: %d of %d pages", len(all), vars.Pages) + } + callTool(t, cs, "variables", map[string]any{"depth": 61, "page": 1}, &vars) + all = append(all, vars.Variables...) + if len(all) != 63 || vars.Page != 1 { + t.Errorf("variables: %d in all, want 63", len(all)) + } + + // 60 different warnings, then one raised 10 times. + var raised warningsOutput + callTool(t, cs, "warnings", nil, &raised) + if len(raised.Warnings) != maxListItems || !raised.More || raised.Warnings[49].Message != "warning 50" { + t.Fatalf("warnings: %d, more %v", len(raised.Warnings), raised.More) + } + callTool(t, cs, "warnings", nil, &raised) + last := raised.Warnings[len(raised.Warnings)-1] + if len(raised.Warnings) != 11 || raised.More || raised.Warnings[0].Message != "warning 51" || + last.Message != "again" || last.Count != 10 || last.Line != 20 { + t.Errorf("warnings after the first 50: %+v", raised.Warnings) + } +} + +func TestMCPLimitSourceLine(t *testing.T) { + cs := stoppedInLarge(t) + var out sourceOutput + callTool(t, cs, "source", map[string]any{"line": 22, "around": 1}, &out) + if long := out.Lines[1]; long.Line != 22 || len(long.Text) != maxLineBytes || !long.Truncated { + t.Errorf("line 22: %d bytes, truncated %v", len(long.Text), long.Truncated) + } + if short := out.Lines[0]; short.Truncated { + t.Errorf("line 21 truncated: %q", short.Text) + } +} diff --git a/cmd/dbgp/mcp_output.go b/cmd/dbgp/mcp_output.go index 8a7c1b8..94c77c9 100644 --- a/cmd/dbgp/mcp_output.go +++ b/cmd/dbgp/mcp_output.go @@ -30,13 +30,15 @@ func (t *tools) addOutputTools(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "output", Description: "Show what a session's script has printed since the last call, so nothing is shown twice; " + - "all returns everything kept so far. Works after the session has ended.", + "all returns everything kept so far. At most 32 KB is returned at a time: when more is set, call again for the rest. " + + "Works after the session has ended.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, }, t.output) mcp.AddTool(server, &mcp.Tool{ Name: "warnings", Description: "Show the PHP warnings, notices and other errors a session's script has raised since the last call; " + - "all returns every one kept so far. Works after the session has ended.", + "all returns every one kept so far. The same warning raised again from the same line is listed once, with a count. " + + "At most 50 are returned at a time: when more is set, call again for the rest. Works after the session has ended.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, OutputSchema: outputSchema[warningsOutput]("warnings"), }, t.warnings) @@ -63,9 +65,10 @@ type sourceInput struct { } type sourceLine struct { - Line int `json:"line" jsonschema:"the line number"` - Text string `json:"text" jsonschema:"the line's code"` - Current bool `json:"current,omitempty" jsonschema:"set on the line where execution is stopped"` + Line int `json:"line" jsonschema:"the line number"` + Text string `json:"text" jsonschema:"the line's code"` + Current bool `json:"current,omitempty" jsonschema:"set on the line where execution is stopped"` + Truncated bool `json:"truncated,omitempty" jsonschema:"the line is longer than 500 bytes and only its start is shown"` } type sourceOutput struct { @@ -118,8 +121,9 @@ func (t *tools) source(_ context.Context, _ *mcp.CallToolRequest, in sourceInput } for i, code := range strings.Split(strings.TrimSuffix(text, "\n"), "\n") { n := begin + i + short := cutText(code, maxLineBytes) out.Lines = append(out.Lines, sourceLine{ - Line: n, Text: code, + Line: n, Text: short, Truncated: len(short) < len(code), Current: st.Status == dbgp.StatusBreak && st.File == file && st.Line == n, }) } @@ -150,6 +154,7 @@ type outputOutput struct { Session int `json:"session" jsonschema:"the session id"` Output string `json:"output" jsonschema:"what the script printed; empty if nothing new"` Truncated bool `json:"truncated,omitempty" jsonschema:"earlier output was dropped: only the most recent 1 MB is kept"` + More bool `json:"more,omitempty" jsonschema:"there is more output after this: call output again for it"` } func (t *tools) output(_ context.Context, _ *mcp.CallToolRequest, in readInput) (*mcp.CallToolResult, outputOutput, error) { @@ -160,8 +165,9 @@ func (t *tools) output(_ context.Context, _ *mcp.CallToolRequest, in readInput) out := outputOutput{Session: s.ID()} t.readFrom(&t.outputFrom, s.ID(), in.All, func(from int) int { text, next, truncated := s.Output(from) - out.Output, out.Truncated = text, truncated - return next + out.Output, out.Truncated = cutText(text, maxTextBytes), truncated + out.More = len(out.Output) < len(text) + return next - (len(text) - len(out.Output)) }) return nil, out, nil } @@ -171,11 +177,13 @@ type warningInfo struct { Message string `json:"message" jsonschema:"PHP's message"` File string `json:"file" jsonschema:"local path of the file where it was raised"` Line int `json:"line" jsonschema:"the line where it was raised"` + Count int `json:"count" jsonschema:"how many times it was raised"` } type warningsOutput struct { Session int `json:"session" jsonschema:"the session id"` - Warnings []warningInfo `json:"warnings" jsonschema:"the warnings, notices and errors, oldest first; empty if nothing new"` + Warnings []warningInfo `json:"warnings" jsonschema:"the warnings, notices and errors, in the order first raised; empty if nothing new"` + More bool `json:"more,omitempty" jsonschema:"there are more after these: call warnings again for them"` } func (t *tools) warnings(_ context.Context, _ *mcp.CallToolRequest, in readInput) (*mcp.CallToolResult, warningsOutput, error) { @@ -186,14 +194,28 @@ func (t *tools) warnings(_ context.Context, _ *mcp.CallToolRequest, in readInput out := warningsOutput{Session: s.ID(), Warnings: []warningInfo{}} t.readFrom(&t.warningsFrom, s.ID(), in.All, func(from int) int { notes, next := s.Notifications(from) - for _, n := range notes { + seen := map[warningInfo]int{} // index in out.Warnings, by warning with no count + for i, n := range notes { // Other notifications, such as breakpoint_resolved, are routine. - if n.Name == "error" && n.Message != nil { - out.Warnings = append(out.Warnings, warningInfo{ - Type: n.Message.Type, Message: strings.TrimSpace(n.Message.Text), - File: n.Message.Filename, Line: n.Message.Lineno, - }) + if n.Name != "error" || n.Message == nil { + continue + } + w := warningInfo{ + Type: n.Message.Type, Message: strings.TrimSpace(n.Message.Text), + File: n.Message.Filename, Line: n.Message.Lineno, + } + if j, ok := seen[w]; ok { + out.Warnings[j].Count++ + continue + } + if len(out.Warnings) == maxListItems { + // The next call starts at this one. + out.More = true + return next - (len(notes) - i) } + seen[w] = len(out.Warnings) + w.Count = 1 + out.Warnings = append(out.Warnings, w) } return next }) diff --git a/cmd/dbgp/mcp_output_test.go b/cmd/dbgp/mcp_output_test.go index 09b19b4..d91ca37 100644 --- a/cmd/dbgp/mcp_output_test.go +++ b/cmd/dbgp/mcp_output_test.go @@ -80,7 +80,7 @@ func TestMCPOutputAndWarnings(t *testing.T) { var raised warningsOutput callTool(t, cs, "warnings", nil, &raised) - want := warningInfo{Type: "Warning", Message: "Undefined variable $undefinedVariable", File: script, Line: 42} + want := warningInfo{Type: "Warning", Message: "Undefined variable $undefinedVariable", File: script, Line: 42, Count: 1} if len(raised.Warnings) != 1 || raised.Warnings[0] != want { t.Errorf("warnings = %+v, want %+v", raised.Warnings, want) } diff --git a/cmd/dbgp/mcp_test.go b/cmd/dbgp/mcp_test.go index 040126f..bb523ad 100644 --- a/cmd/dbgp/mcp_test.go +++ b/cmd/dbgp/mcp_test.go @@ -118,7 +118,8 @@ func TestMCPTools(t *testing.T) { if got["status"].OutputSchema == nil { t.Error("status has no output schema") } - for _, want := range []string{"listen", "php -v", "with PHP Debugger", "do not set xdebug.mode", "remove_breakpoint"} { + for _, want := range []string{"listen", "php -v", "with PHP Debugger", "do not set xdebug.mode", "remove_breakpoint", + "in the background", "stop or detach", "in parts"} { if !strings.Contains(cs.InitializeResult().Instructions, want) { t.Errorf("instructions lack %q: %s", want, cs.InitializeResult().Instructions) } @@ -237,7 +238,9 @@ func TestMCPStatusShowsSession(t *testing.T) { } // The built binary in MCP mode, driven over stdin and stdout. -func TestMCPBinary(t *testing.T) { +// buildBinary builds dbgp into a temporary directory and returns its path. +func buildBinary(t *testing.T) string { + t.Helper() if testing.Short() { t.Skip("builds the binary") } @@ -245,6 +248,11 @@ func TestMCPBinary(t *testing.T) { if out, err := exec.Command("go", "build", "-o", bin, ".").CombinedOutput(); err != nil { t.Fatalf("go build: %v\n%s", err, out) } + return bin +} + +func TestMCPBinary(t *testing.T) { + bin := buildBinary(t) if out, _ := exec.Command(bin, "mcp", "-h").CombinedOutput(); !strings.Contains(string(out), "Usage: dbgp mcp [options]") { t.Errorf("mcp -h output:\n%s", out) diff --git a/server.go b/server.go index 0629caa..c7ad60b 100644 --- a/server.go +++ b/server.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net" + "slices" "sort" "strconv" "strings" @@ -339,6 +340,7 @@ func (s *Server) handshake(conn net.Conn) { } s.sessions = append(s.sessions, sess) s.queue = append(s.queue, sess) + s.pruneSessions() close(s.arrived) s.arrived = make(chan struct{}) } @@ -416,7 +418,33 @@ func (s *Server) AddPathMapping(m PathMapping) error { return s.syncStoppedSessions() } -// Sessions returns all sessions in connection order, including ended ones. +// pruneSessions forgets the oldest ended sessions beyond maxEndedSessions, +// with their output, so a server debugging many requests does not keep +// them all. Sessions WaitForSession has not returned yet are kept. +func (s *Server) pruneSessions() { + ended := 0 + for _, sess := range s.sessions { + if sess.ended() { + ended++ + } + } + if ended <= maxEndedSessions { + return + } + kept := s.sessions[:0] + for _, sess := range s.sessions { + if ended > maxEndedSessions && sess.ended() && !slices.Contains(s.queue, sess) { + ended-- + continue + } + kept = append(kept, sess) + } + clear(s.sessions[len(kept):]) + s.sessions = kept +} + +// Sessions returns the sessions in connection order, including the most +// recent ended ones. func (s *Server) Sessions() []*Session { s.mu.Lock() defer s.mu.Unlock() diff --git a/server_test.go b/server_test.go index 3499b96..4839d64 100644 --- a/server_test.go +++ b/server_test.go @@ -217,6 +217,33 @@ func TestServerTracksSessions(t *testing.T) { } } +func TestServerForgetsOldEndedSessions(t *testing.T) { + srv := newTestServer(t, Config{}) + dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) + open := waitSession(t, srv) + for range maxEndedSessions + 1 { + e := dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) + sess := waitSession(t, srv) + e.Close() + <-sess.Done() + } + // Ended sessions are pruned when the next one connects. + dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) + last := waitSession(t, srv) + + sessions := srv.Sessions() + if len(sessions) != maxEndedSessions+2 { + t.Fatalf("%d sessions kept, want %d", len(sessions), maxEndedSessions+2) + } + // The oldest ended session, id 2, is forgotten; the open one is kept. + if sessions[0] != open || sessions[1].ID() != 3 || sessions[len(sessions)-1] != last { + t.Errorf("kept sessions %d, %d … %d", sessions[0].ID(), sessions[1].ID(), sessions[len(sessions)-1].ID()) + } + if srv.Session(2) != nil { + t.Error("session 2 still found") + } +} + func TestServerCloseEndsSessions(t *testing.T) { srv := newTestServer(t, Config{}) e := dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) diff --git a/session.go b/session.go index 6a47a96..1580973 100644 --- a/session.go +++ b/session.go @@ -36,6 +36,10 @@ const ( maxNotifications = 1000 ) +// maxEndedSessions is how many ended sessions a Server keeps; older ones +// are forgotten. +const maxEndedSessions = 50 + var ( // ErrRunning is returned for commands sent while the script runs: the // engine only reads commands while it is stopped. @@ -211,6 +215,16 @@ func (s *Session) State() State { // Done is closed when the connection has ended. func (s *Session) Done() <-chan struct{} { return s.closed } +// ended reports whether the connection has ended. +func (s *Session) ended() bool { + select { + case <-s.closed: + return true + default: + return false + } +} + // SetupError reports features from Config.Features the engine rejected. func (s *Session) SetupError() error { s.mu.Lock() diff --git a/testdata/php/large.php b/testdata/php/large.php new file mode 100644 index 0000000..778761e --- /dev/null +++ b/testdata/php/large.php @@ -0,0 +1,23 @@ + "v$i", range(1, 60)), 1)); +$text = str_repeat("é", 50000); +echo $text; +for ($i = 1; $i <= 60; $i++) { + trigger_error("warning $i", E_USER_WARNING); +} +for ($i = 1; $i <= 10; $i++) { + trigger_error("again", E_USER_WARNING); +} +$line = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'; +down(60);