Add MCP eval and set_variable tools, with -no-eval to leave them out

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
 

cmd/dbgp/mcp.go

@@ -40,13 +40,24 @@ 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 {
 	fs := flag.NewFlagSet("dbgp mcp", flag.ContinueOnError)
 	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
 }
 

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
+}

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)
+			}
+		}
+	}
+}

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) {