diff --git a/README.md b/README.md index 0ba1b4e..d2ac2f6 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ 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). Tools that run code in the script follow. +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`. ## Testing diff --git a/cmd/dbgp/mcp.go b/cmd/dbgp/mcp.go index 414d65f..b5b1705 100644 --- a/cmd/dbgp/mcp.go +++ b/cmd/dbgp/mcp.go @@ -40,6 +40,16 @@ and the listen tool's address is localhost:9003, dbgp's default, those defaults already reach dbgp and neither needs to be set. Otherwise set them to the port from that address and a host PHP can reach this machine on.` +// noCodeInstructions are added to mcpInstructions when dbgp mcp runs with +// -no-eval, so agents know the code tools were left out on purpose. +const noCodeInstructions = ` + +The user started dbgp with -no-eval, which turns off the eval and set_variable +tools, so you cannot evaluate PHP or change variables in the debugged script. +This is the user's choice, not a missing feature or an outdated build: do not +suggest rebuilding or restarting dbgp to get them. Read values with variables, +variable and variable_value instead, and work out results from them.` + // runMCP runs dbgp as an MCP server on stdin and stdout until the client // disconnects. stdout carries the protocol, so diagnostics go to stderr. func runMCP(args []string) int { @@ -47,6 +57,7 @@ func runMCP(args []string) int { addr := fs.String("addr", dbgp.DefaultAddr, "address to listen on for PHP connections") idekey := fs.String("idekey", "", "accept only sessions with this IDE key") listen := fs.Bool("listen", false, "listen for PHP connections from the start, instead of waiting for the listen tool") + noEval := fs.Bool("no-eval", false, "leave out the eval and set_variable tools, which run PHP code in the debugged script") var maps listFlag fs.Var(&maps, "map", "path mapping `LOCAL=REMOTE` for PHP in a container or on a server (repeatable)") fs.Usage = func() { @@ -82,17 +93,22 @@ func runMCP(args []string) int { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - if err := newMCPServer(srv).Run(ctx, &mcp.StdioTransport{}); err != nil && ctx.Err() == nil { + if err := newMCPServer(srv, !*noEval).Run(ctx, &mcp.StdioTransport{}); err != nil && ctx.Err() == nil { fmt.Fprintln(os.Stderr, err) return 1 } return 0 } -// newMCPServer returns an MCP server whose tools drive srv. -func newMCPServer(srv *dbgp.Server) *mcp.Server { +// newMCPServer returns an MCP server whose tools drive srv. allowCode adds +// the tools that run PHP code in the debugged script. +func newMCPServer(srv *dbgp.Server, allowCode bool) *mcp.Server { + instructions := mcpInstructions + if !allowCode { + instructions += noCodeInstructions + } server := mcp.NewServer(&mcp.Implementation{Name: "dbgp", Title: "PHP debugger", Version: version}, - &mcp.ServerOptions{Instructions: mcpInstructions}) + &mcp.ServerOptions{Instructions: instructions}) t := &tools{srv: srv} mcp.AddTool(server, &mcp.Tool{ @@ -154,6 +170,9 @@ func newMCPServer(srv *dbgp.Server) *mcp.Server { t.addSessionTools(server) t.addInspectTools(server) t.addOutputTools(server) + if allowCode { + t.addCodeTools(server) + } return server } diff --git a/cmd/dbgp/mcp_code.go b/cmd/dbgp/mcp_code.go new file mode 100644 index 0000000..440eeb6 --- /dev/null +++ b/cmd/dbgp/mcp_code.go @@ -0,0 +1,120 @@ +package main + +import ( + "context" + "errors" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" + dbgp "github.com/php-debugger/dbgp-client" +) + +// Code tools: evaluating PHP and changing variables in the debugged +// script. They run code in it, so dbgp mcp -no-eval leaves them out. + +func (t *tools) addCodeTools(server *mcp.Server) { + mcp.AddTool(server, &mcp.Tool{ + Name: "eval", + Description: "Evaluate a PHP expression where a session is stopped, and show the result like variable does: " + + "an array or object with one page of its contents. To look deeper into the result, evaluate the part " + + "you want, e.g. ($expr)[\"key\"]. The expression runs in the script, so it can change its state. " + + "It always runs in the innermost frame, where execution stopped, and sees only that frame's variables: " + + "PHP offers no way to evaluate in a caller's frame. To read a caller's variables, use variables, " + + "variable or variable_value with depth.", + OutputSchema: outputSchema[evalOutput]("children"), + }, t.eval) + mcp.AddTool(server, &mcp.Tool{ + Name: "set_variable", + Description: "Set a variable, or a path into one such as $user[\"name\"], to the value of a PHP expression, " + + "in a stack frame of a stopped session. Returns the variable as it is afterwards.", + }, t.setVariable) +} + +type evalInput struct { + Session int `json:"session,omitempty" jsonschema:"the session id; defaults to the newest session that has not ended"` + Code string `json:"code" jsonschema:"a PHP expression, e.g. count($items) or $user->getName(); it sees the variables of the frame where execution stopped"` + Page int `json:"page,omitempty" jsonschema:"the page of the result's elements or properties, from 0 (the default); see pages"` +} + +type evalOutput struct { + Result *variableInfo `json:"result,omitempty" jsonschema:"the result, named after the expression; absent when it has no value"` + Children []variableInfo `json:"children" jsonschema:"one page of the result's elements or properties, for arrays and objects"` + Page int `json:"page" jsonschema:"this page, from 0"` + Pages int `json:"pages" jsonschema:"how many pages of elements or properties the result has; call eval with page for the others"` +} + +// codeError explains an error from running PHP code. +func codeError(s *dbgp.Session, err error) error { + var engineErr *dbgp.EngineError + if errors.As(err, &engineErr) && engineErr.Code == 206 { + return fmt.Errorf("PHP could not evaluate it: %s", engineErr.Message) + } + return sessionError(s, err) +} + +func (t *tools) eval(_ context.Context, _ *mcp.CallToolRequest, in evalInput) (*mcp.CallToolResult, evalOutput, error) { + s, err := t.session(in.Session) + if err != nil { + return nil, evalOutput{}, err + } + if in.Code == "" { + return nil, evalOutput{}, errors.New("code is required, e.g. count($items)") + } + if in.Page < 0 { + return nil, evalOutput{}, errors.New("page must not be negative") + } + p, err := s.EvalProperty(in.Code, in.Page) + if err != nil { + return nil, evalOutput{}, codeError(s, err) + } + out := evalOutput{Children: []variableInfo{}, Page: in.Page, Pages: 1} + if p == nil { + return nil, out, nil + } + if in.Page >= p.Pages() { + return nil, evalOutput{}, fmt.Errorf("the result has %d pages, numbered from 0", p.Pages()) + } + result := toVariable(*p) + result.Name = in.Code + out.Result, out.Pages = &result, p.Pages() + for _, c := range p.ChildProperties { + out.Children = append(out.Children, toVariable(c)) + } + return nil, out, nil +} + +type setVariableInput struct { + Session int `json:"session,omitempty" jsonschema:"the session id; defaults to the newest session that has not ended"` + Name string `json:"name" jsonschema:"the variable, or a path into one, e.g. $count or $user[\"name\"]"` + Value string `json:"value" jsonschema:"a PHP expression for the new value, e.g. 42, 'text' or $a * 2"` + 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"` +} + +type setVariableOutput struct { + Variable variableInfo `json:"variable" jsonschema:"the variable after it was set"` +} + +func (t *tools) setVariable(_ context.Context, _ *mcp.CallToolRequest, in setVariableInput) (*mcp.CallToolResult, setVariableOutput, error) { + s, err := t.session(in.Session) + if err != nil { + return nil, setVariableOutput{}, err + } + if in.Name == "" || in.Value == "" { + return nil, setVariableOutput{}, errors.New("name and value are both required, e.g. $count and 42") + } + opts := dbgp.PropertyOptions{Depth: in.Depth, Context: in.Context} + if err := s.SetProperty(in.Name, in.Value, opts); err != nil { + var engineErr *dbgp.EngineError + if errors.Is(err, dbgp.ErrRunning) || ended(s.State()) || errors.As(err, &engineErr) { + return nil, setVariableOutput{}, codeError(s, err) + } + // The engine reports a failed assignment without a reason. + return nil, setVariableOutput{}, fmt.Errorf("PHP could not set %s to %s: check that the value is a valid PHP expression", in.Name, in.Value) + } + p, err := s.GetProperty(in.Name, opts) + if err != nil { + return nil, setVariableOutput{}, inspectError(s, in.Name, in.Depth, err) + } + return nil, setVariableOutput{Variable: toVariable(*p)}, nil +} diff --git a/cmd/dbgp/mcp_code_test.go b/cmd/dbgp/mcp_code_test.go new file mode 100644 index 0000000..1a3a2ac --- /dev/null +++ b/cmd/dbgp/mcp_code_test.go @@ -0,0 +1,130 @@ +package main + +import ( + "context" + "strings" + "testing" +) + +func TestMCPCodeToolsCanBeLeftOut(t *testing.T) { + cs, _ := newTestMCPOn(t, "127.0.0.1:0", false) + res, err := cs.ListTools(context.Background(), nil) + if err != nil { + t.Fatal(err) + } + for _, tool := range res.Tools { + if tool.Name == "eval" || tool.Name == "set_variable" { + t.Errorf("%s offered although code tools are off", tool.Name) + } + } + if len(res.Tools) != 21 { + t.Errorf("got %d tools, want the other 21", len(res.Tools)) + } + // Agents are told the tools were turned off, not that they are missing. + if instructions := cs.InitializeResult().Instructions; !strings.Contains(instructions, "-no-eval") { + t.Errorf("instructions do not explain -no-eval: %s", instructions) + } + on, _ := newTestMCPOn(t, "127.0.0.1:0", true) + if strings.Contains(on.InitializeResult().Instructions, "-no-eval") { + t.Error("instructions mention -no-eval although code tools are on") + } +} + +func TestMCPCodeToolsWithoutSession(t *testing.T) { + cs, _ := newTestMCP(t) + for tool, args := range map[string]map[string]any{ + "eval": {"code": "1"}, + "set_variable": {"name": "$a", "value": "1"}, + } { + if msg := callToolError(t, cs, tool, args); !strings.Contains(msg, "no session") { + t.Errorf("%s: %q", tool, msg) + } + } +} + +func TestMCPEval(t *testing.T) { + cs, _, _ := stoppedInAdd(t) + + var out evalOutput + callTool(t, cs, "eval", map[string]any{"code": "$a + $b"}, &out) + if out.Result == nil || !same(*out.Result, variableInfo{Name: "$a + $b", Type: "int", Value: "5"}) || len(out.Children) != 0 { + t.Errorf("eval $a + $b = %+v", out) + } + + callTool(t, cs, "eval", map[string]any{"code": "[$a, 'k' => $b]"}, &out) + if out.Result == nil || out.Result.Type != "array" || *out.Result.Size != 2 || len(out.Children) != 2 || + !same(out.Children[0], variableInfo{Name: "0", Type: "int", Value: "2"}) || + !same(out.Children[1], variableInfo{Name: "k", Type: "int", Value: "3"}) { + t.Errorf("eval array = %+v, children %+v", out.Result, out.Children) + } + + callTool(t, cs, "eval", map[string]any{"code": "range(1, 40)", "page": 1}, &out) + if out.Pages != 2 || out.Page != 1 || len(out.Children) != 8 || out.Children[7].Value != "40" { + t.Errorf("eval page 1 = %d children, page %d of %d", len(out.Children), out.Page, out.Pages) + } + callTool(t, cs, "eval", map[string]any{"code": "null"}, &out) + if out.Result == nil || out.Result.Type != "null" { + t.Errorf("eval null = %+v", out.Result) + } + + for _, tc := range []struct { + args map[string]any + want string + }{ + {map[string]any{"code": "$a +"}, "PHP could not evaluate it"}, + {map[string]any{"code": "range(1, 40)", "page": 2}, "the result has 2 pages"}, + {map[string]any{"code": ""}, "code is required"}, + } { + if msg := callToolError(t, cs, "eval", tc.args); !strings.Contains(msg, tc.want) { + t.Errorf("eval(%v): %q, want %q", tc.args, msg, tc.want) + } + } +} + +func TestMCPSetVariable(t *testing.T) { + cs, _, _ := stoppedInAdd(t) + + var out setVariableOutput + callTool(t, cs, "set_variable", map[string]any{"name": "$b", "value": "$a * 10"}, &out) + if !same(out.Variable, variableInfo{Name: "$b", Type: "int", Value: "20"}) { + t.Errorf("set $b = %+v", out.Variable) + } + // The script carries on with the new value: $sum = $a + $b. + callTool(t, cs, "continue", map[string]any{"command": "step_over"}, &stateOutput{}) + var sum variableOutput + callTool(t, cs, "variable", map[string]any{"name": "$sum"}, &sum) + if sum.Variable.Value != "22" { + t.Errorf("$sum after setting $b = %+v, want 22", sum.Variable) + } + + callTool(t, cs, "set_variable", map[string]any{"name": "$name", "value": "'Mars'", "depth": 1}, &out) + if !same(out.Variable, variableInfo{Name: "$name", Type: "string", Value: "Mars"}) { + t.Errorf("set $name in frame 1 = %+v", out.Variable) + } + + if msg := callToolError(t, cs, "set_variable", map[string]any{"name": "$b", "value": "$a +"}); !strings.Contains(msg, "PHP could not set $b to $a +: check that the value is a valid PHP expression") { + t.Errorf("set_variable with a syntax error: %q", msg) + } + if msg := callToolError(t, cs, "set_variable", map[string]any{"name": "$b"}); !strings.Contains(msg, "required") { + t.Errorf("set_variable without a value: %q", msg) + } +} + +// eval has no stack frame option: the description must say so. +func TestMCPEvalDescriptionExplainsFrames(t *testing.T) { + cs, _ := newTestMCP(t) + res, err := cs.ListTools(context.Background(), nil) + if err != nil { + t.Fatal(err) + } + for _, tool := range res.Tools { + if tool.Name != "eval" { + continue + } + for _, want := range []string{"innermost frame", "no way to evaluate in a caller's frame", "with depth"} { + if !strings.Contains(tool.Description, want) { + t.Errorf("eval description lacks %q", want) + } + } + } +} diff --git a/cmd/dbgp/mcp_test.go b/cmd/dbgp/mcp_test.go index f6665d3..040126f 100644 --- a/cmd/dbgp/mcp_test.go +++ b/cmd/dbgp/mcp_test.go @@ -20,11 +20,12 @@ import ( // is not listening yet. func newTestMCP(t *testing.T) (*mcp.ClientSession, *dbgp.Server) { t.Helper() - return newTestMCPOn(t, "127.0.0.1:0") + return newTestMCPOn(t, "127.0.0.1:0", true) } -// newTestMCPOn is newTestMCP with the server on addr. -func newTestMCPOn(t *testing.T, addr string) (*mcp.ClientSession, *dbgp.Server) { +// newTestMCPOn is newTestMCP with the server on addr, and the code tools +// only if allowCode. +func newTestMCPOn(t *testing.T, addr string, allowCode bool) (*mcp.ClientSession, *dbgp.Server) { t.Helper() srv, err := dbgp.NewServer(dbgp.Config{Addr: addr}) if err != nil { @@ -34,7 +35,7 @@ func newTestMCPOn(t *testing.T, addr string) (*mcp.ClientSession, *dbgp.Server) ctx := context.Background() clientTransport, serverTransport := mcp.NewInMemoryTransports() - ss, err := newMCPServer(srv).Connect(ctx, serverTransport, nil) + ss, err := newMCPServer(srv, allowCode).Connect(ctx, serverTransport, nil) if err != nil { t.Fatal(err) } @@ -96,7 +97,7 @@ func TestMCPTools(t *testing.T) { } names := []string{"status", "listen", "unlisten", "add_breakpoint", "remove_breakpoint", "breakpoints", "add_path_mapping", "path_mappings", "sessions", "wait_for_session", "continue", "wait_for_stop", "stop", "detach", - "stack", "variables", "variable", "variable_value", "source", "output", "warnings"} + "stack", "variables", "variable", "variable_value", "source", "output", "warnings", "eval", "set_variable"} for _, name := range names { if got[name] == nil { t.Errorf("tool %s missing", name) @@ -259,6 +260,7 @@ func TestMCPBinary(t *testing.T) { }{ {[]string{"mcp", "-addr", "127.0.0.1:0"}, false}, {[]string{"mcp", "-addr", "127.0.0.1:0", "-listen"}, true}, + {[]string{"mcp", "-addr", "127.0.0.1:0", "-no-eval"}, false}, } { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) transport := &mcp.CommandTransport{Command: exec.CommandContext(ctx, bin, tc.args...)} @@ -272,6 +274,17 @@ func TestMCPBinary(t *testing.T) { if status.Listening != tc.listening { t.Errorf("%v: listening = %v, want %v", tc.args, status.Listening, tc.listening) } + list, err := cs.ListTools(ctx, nil) + if err != nil { + t.Fatal(err) + } + hasEval := false + for _, tool := range list.Tools { + hasEval = hasEval || tool.Name == "eval" + } + if wantEval := !strings.Contains(strings.Join(tc.args, " "), "-no-eval"); hasEval != wantEval { + t.Errorf("%v: has eval = %v, want %v", tc.args, hasEval, wantEval) + } if err := cs.Close(); err != nil { t.Errorf("%v: close: %v", tc.args, err) } @@ -487,7 +500,7 @@ func TestMCPListenWarnings(t *testing.T) { } defer other.Close() - cs, _ := newTestMCPOn(t, "localhost:"+port) + cs, _ := newTestMCPOn(t, "localhost:"+port, true) var out listenOutput callTool(t, cs, "listen", nil, &out) if !out.Listening || out.Address != "localhost:"+port || len(out.Warnings) != 1 || !strings.Contains(out.Warnings[0], "[::1]:"+port) {