feat(debugmcp): declare outputSchema for every tool

debugmcp/schema_test.go

@@ -0,0 +1,212 @@
+package debugmcp
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// toolDef looks a tool up in the tools/list payload.
+func toolDef(t *testing.T, name string) map[string]any {
+	t.Helper()
+	for _, d := range toolDefs() {
+		if d["name"] == name {
+			return d
+		}
+	}
+	t.Fatalf("no tool definition for %q", name)
+	return nil
+}
+
+// schemaOf returns a tool's named schema as a decoded JSON object — decoded
+// rather than read directly so the test sees what a client sees.
+func schemaOf(t *testing.T, tool map[string]any, key string) map[string]any {
+	t.Helper()
+	raw, ok := tool[key]
+	if !ok {
+		t.Fatalf("tool %v: no %s", tool["name"], key)
+	}
+	b, err := json.Marshal(raw)
+	if err != nil {
+		t.Fatalf("tool %v: %s does not marshal: %v", tool["name"], key, err)
+	}
+	var m map[string]any
+	if err := json.Unmarshal(b, &m); err != nil {
+		t.Fatalf("tool %v: %s is not a JSON object: %v", tool["name"], key, err)
+	}
+	return m
+}
+
+// Every tool declares both schemas, both are well formed, and every required
+// key is one the schema actually declares. Handlers and definitions must also
+// agree: a handler with no definition is invisible, a definition with no
+// handler is a dead promise.
+func TestToolSchemasAreComplete(t *testing.T) {
+	defs := toolDefs()
+	seen := map[string]bool{}
+	for _, d := range defs {
+		name, _ := d["name"].(string)
+		if name == "" {
+			t.Fatalf("tool definition without a name: %v", d)
+		}
+		if seen[name] {
+			t.Errorf("duplicate tool definition: %s", name)
+		}
+		seen[name] = true
+		if desc, _ := d["description"].(string); desc == "" {
+			t.Errorf("tool %s: no description", name)
+		}
+		if toolHandlers[name] == nil {
+			t.Errorf("tool %s: declared but has no handler", name)
+		}
+		for _, key := range []string{"inputSchema", "outputSchema"} {
+			s := schemaOf(t, d, key)
+			if s["type"] != "object" {
+				t.Errorf("tool %s: %s type = %v, want object", name, key, s["type"])
+			}
+			props, ok := s["properties"].(map[string]any)
+			if !ok {
+				t.Errorf("tool %s: %s properties = %v, want an object", name, key, s["properties"])
+				continue
+			}
+			req, _ := s["required"].([]any)
+			for _, r := range req {
+				if _, declared := props[r.(string)]; !declared {
+					t.Errorf("tool %s: %s requires %q but does not declare it", name, key, r)
+				}
+			}
+		}
+	}
+	for name := range toolHandlers {
+		if !seen[name] {
+			t.Errorf("handler %s: reachable but absent from tools/list", name)
+		}
+	}
+}
+
+// checkAgainstSchema asserts that a payload a tool actually emitted satisfies
+// the outputSchema that tool advertises: every required key present, every key
+// declared, and every declared type honoured. This is the guard that keeps the
+// declaration honest as the handlers change.
+func checkAgainstSchema(t *testing.T, name string, payload map[string]any) {
+	t.Helper()
+	schema := schemaOf(t, toolDef(t, name), "outputSchema")
+	props, _ := schema["properties"].(map[string]any)
+	req, _ := schema["required"].([]any)
+
+	for _, r := range req {
+		if _, ok := payload[r.(string)]; !ok {
+			t.Errorf("tool %s: emitted payload lacks required key %q: %v", name, r, payload)
+		}
+	}
+	for k, v := range payload {
+		spec, declared := props[k].(map[string]any)
+		if !declared {
+			t.Errorf("tool %s: emitted undeclared key %q", name, k)
+			continue
+		}
+		checkType(t, name+"."+k, spec, v)
+	}
+}
+
+// checkType verifies one value against its schema fragment, descending into
+// arrays and nested objects (the frame and variable item shapes).
+func checkType(t *testing.T, path string, spec map[string]any, v any) {
+	t.Helper()
+	switch spec["type"] {
+	case "string":
+		s, ok := v.(string)
+		if !ok {
+			t.Errorf("%s = %v (%T), want string", path, v, v)
+			return
+		}
+		if enum, ok := spec["enum"].([]any); ok {
+			for _, e := range enum {
+				if s == e {
+					return
+				}
+			}
+			t.Errorf("%s = %q, not in enum %v", path, s, enum)
+		}
+	case "integer":
+		// JSON numbers decode to float64; integrality is what we assert.
+		f, ok := v.(float64)
+		if !ok || f != float64(int64(f)) {
+			t.Errorf("%s = %v (%T), want integer", path, v, v)
+		}
+	case "boolean":
+		if _, ok := v.(bool); !ok {
+			t.Errorf("%s = %v (%T), want boolean", path, v, v)
+		}
+	case "array":
+		items, ok := v.([]any)
+		if !ok {
+			t.Errorf("%s = %v (%T), want array", path, v, v)
+			return
+		}
+		itemSpec, _ := spec["items"].(map[string]any)
+		for _, it := range items {
+			checkType(t, path+"[]", itemSpec, it)
+		}
+	case "object":
+		m, ok := v.(map[string]any)
+		if !ok {
+			t.Errorf("%s = %v (%T), want object", path, v, v)
+			return
+		}
+		props, _ := spec["properties"].(map[string]any)
+		req, _ := spec["required"].([]any)
+		for _, r := range req {
+			if _, ok := m[r.(string)]; !ok {
+				t.Errorf("%s lacks required key %q", path, r)
+			}
+		}
+		for k, mv := range m {
+			sub, declared := props[k].(map[string]any)
+			if !declared {
+				t.Errorf("%s has undeclared key %q", path, k)
+				continue
+			}
+			checkType(t, path+"."+k, sub, mv)
+		}
+	}
+}
+
+// Drive a real session and hold every tool's actual output against its declared
+// outputSchema — including both success shapes of the tools that have two (a
+// run that stops vs one that finishes, a source fetched whole vs as a snippet).
+func TestToolOutputMatchesDeclaredSchema(t *testing.T) {
+	srv := newTestServer()
+	tr := newChanTransport()
+	go srv.Serve(tr)
+	c := &testClient{t: t, tr: tr}
+	c.call("initialize", map[string]any{})
+
+	check := func(name string, args map[string]any) map[string]any {
+		payload := c.callTool(name, args)
+		checkAgainstSchema(t, name, payload)
+		return payload
+	}
+
+	check("set_breakpoints", map[string]any{"source": "loop", "lines": []any{2.0}})
+
+	ev := check("launch", map[string]any{"program": "loop"})
+	if ev["event"] != "stopped" {
+		t.Fatalf("expected a stop inside add(), got %v", ev)
+	}
+	// Paused: the inspection tools and both get_source shapes.
+	check("stack", nil)
+	check("variables", map[string]any{"frame": 0.0})
+	check("evaluate", map[string]any{"expr": "a"})
+	check("get_source", map[string]any{"id": "loop"})                              // whole text
+	check("get_source", map[string]any{"id": "loop", "line": 2.0, "context": 1.0}) // snippet
+	check("pause", nil)
+
+	// Run to the end so the "finished" variant of the control-tool schema is
+	// exercised too, not just "stopped".
+	for ev["event"] == "stopped" {
+		ev = check("continue", nil)
+	}
+	if ev["event"] != "finished" {
+		t.Fatalf("program did not finish: %v", ev)
+	}
+}

debugmcp/tools.go

@@ -26,9 +26,23 @@ var toolHandlers = map[string]toolHandler{
 }
 
 // toolDefs is the tools/list payload: each tool's name, description and JSON
-// Schema for its arguments.
+// Schema for its arguments and for its result.
+//
+// Every tool here reports success as structuredContent, so every tool declares
+// an outputSchema — that is the whole contract a client can rely on, and
+// without it the structured object is opaque. The schemas describe successful
+// results only: a tool-level failure comes back as an isError result carrying
+// {"error": …}, which is a different shape by design.
+//
+// Required keys are the ones present in *every* success. Tools with two success
+// shapes (a run that stops vs finishes, a source fetched whole vs as a snippet)
+// therefore require only the key that discriminates them, and declare the union
+// of the optional rest.
 func toolDefs() []map[string]any {
 	obj := func(props map[string]any, required ...string) map[string]any {
+		if props == nil {
+			props = map[string]any{} // JSON Schema wants an object, not null
+		}
 		schema := map[string]any{"type": "object", "properties": props}
 		if len(required) > 0 {
 			schema["required"] = required
@@ -37,14 +51,47 @@ func toolDefs() []map[string]any {
 	}
 	str := map[string]any{"type": "string"}
 	intt := map[string]any{"type": "integer"}
+	boolt := map[string]any{"type": "boolean"}
+	arr := func(items map[string]any) map[string]any {
+		return map[string]any{"type": "array", "items": items}
+	}
+
+	// runEvent is what the control tools return: the program either stopped or
+	// finished. Only "event" tells the two apart, so it is the only certainty.
+	runEvent := obj(map[string]any{
+		"event":    map[string]any{"type": "string", "enum": []string{"stopped", "finished"}},
+		"reason":   str,
+		"source":   str,
+		"line":     intt,
+		"function": str,
+		"depth":    intt,
+		"results":  arr(str),
+		"error":    str,
+	}, "event")
+	frame := obj(map[string]any{
+		"level": intt, "source": str, "line": intt, "function": str, "what": str,
+	}, "level", "source", "line", "function", "what")
+	variable := obj(map[string]any{"name": str, "value": str, "kind": str},
+		"name", "value", "kind")
+
+	// The control tools differ only in description; their contract is identical.
+	control := func(name, description string) map[string]any {
+		return map[string]any{
+			"name": name, "description": description,
+			"inputSchema": obj(nil), "outputSchema": runEvent,
+		}
+	}
+
 	return []map[string]any{
 		{
 			"name":        "set_breakpoints",
 			"description": "Set the breakpoints for a source (a program id). Replaces any previous set for that source. Safe before or during a run.",
 			"inputSchema": obj(map[string]any{
 				"source": str,
-				"lines":  map[string]any{"type": "array", "items": intt},
+				"lines":  arr(intt),
 			}, "source", "lines"),
+			"outputSchema": obj(map[string]any{"source": str, "lines": arr(intt)},
+				"source", "lines"),
 		},
 		{
 			"name":        "launch",
@@ -53,27 +100,43 @@ func toolDefs() []map[string]any {
 				"program": str,
 				"source":  str,
 			}, "program"),
+			"outputSchema": runEvent,
+		},
+		control("continue", "Resume until the next breakpoint or program end. Returns a 'stopped' or 'finished' event."),
+		control("step_over", "Step to the next line in the current frame (calls run without stopping). Returns a 'stopped' or 'finished' event."),
+		control("step_into", "Step to the next line, descending into calls. Returns a 'stopped' or 'finished' event."),
+		control("step_out", "Run until the current function returns. Returns a 'stopped' or 'finished' event."),
+		{
+			"name":         "pause",
+			"description":  "Request a stop at the next line (asynchronous).",
+			"inputSchema":  obj(nil),
+			"outputSchema": obj(map[string]any{"ok": boolt}, "ok"),
+		},
+		{
+			"name":         "stack",
+			"description":  "List the call stack at the current stop (innermost first).",
+			"inputSchema":  obj(nil),
+			"outputSchema": obj(map[string]any{"frames": arr(frame)}, "frames"),
 		},
-		{"name": "continue", "description": "Resume until the next breakpoint or program end. Returns a 'stopped' or 'finished' event.", "inputSchema": obj(nil)},
-		{"name": "step_over", "description": "Step to the next line in the current frame (calls run without stopping). Returns a 'stopped' or 'finished' event.", "inputSchema": obj(nil)},
-		{"name": "step_into", "description": "Step to the next line, descending into calls. Returns a 'stopped' or 'finished' event.", "inputSchema": obj(nil)},
-		{"name": "step_out", "description": "Run until the current function returns. Returns a 'stopped' or 'finished' event.", "inputSchema": obj(nil)},
-		{"name": "pause", "description": "Request a stop at the next line (asynchronous).", "inputSchema": obj(nil)},
-		{"name": "stack", "description": "List the call stack at the current stop (innermost first).", "inputSchema": obj(nil)},
 		{
-			"name":        "variables",
-			"description": "List the locals, upvalues and varargs of a frame at the current stop.",
-			"inputSchema": obj(map[string]any{"frame": intt}),
+			"name":         "variables",
+			"description":  "List the locals, upvalues and varargs of a frame at the current stop.",
+			"inputSchema":  obj(map[string]any{"frame": intt}),
+			"outputSchema": obj(map[string]any{"variables": arr(variable)}, "variables"),
 		},
 		{
-			"name":        "evaluate",
-			"description": "Evaluate a Lua expression (or statement) in the scope of a frame at the current stop.",
-			"inputSchema": obj(map[string]any{"expr": str, "frame": intt}, "expr"),
+			"name":         "evaluate",
+			"description":  "Evaluate a Lua expression (or statement) in the scope of a frame at the current stop.",
+			"inputSchema":  obj(map[string]any{"expr": str, "frame": intt}, "expr"),
+			"outputSchema": obj(map[string]any{"result": str}, "result"),
 		},
 		{
 			"name":        "get_source",
 			"description": "Fetch source text by program id — the whole text, or a snippet around 'line' when given. Defaults to the current stop's source. Lets a client with no local source show where it is.",
 			"inputSchema": obj(map[string]any{"id": str, "line": intt, "context": intt}),
+			"outputSchema": obj(map[string]any{
+				"id": str, "source": str, "line": intt, "snippet": str,
+			}, "id"),
 		},
 	}
 }