From b05aa8de982a2145b035645f4241d65530b3b061 Mon Sep 17 00:00:00 2001 From: htjulia Date: Wed, 12 Aug 2026 08:48:31 +0900 Subject: [PATCH] feat(debugmcp): declare outputSchema for every tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every tool already reports success as structuredContent, but none declared an outputSchema. A client therefore had no contract for the structured object it receives — it could only guess the shape or fall back to parsing the text block. A tool that emits structured output should describe it. - declare outputSchema on all 11 tools; the control tools share one schema, since a run either stops or finishes - require only the key present in every success (event, id) and declare the union of the rest, so the tools with two success shapes stay honest rather than promising keys they sometimes omit - emit an empty object rather than null for the no-argument inputSchema properties, which was not valid JSON Schema The declarations are held to the handlers by tests: one asserts every tool declares both schemas with required a subset of properties, the other drives a real debug session and validates each emitted payload against its declared schema, covering both success shapes. Tags: #lua-pure #debugmcp #mcp #schema #contract Co-Authored-By: htjulia --- debugmcp/schema_test.go | 212 ++++++++++++++++++++++++++++++++++++++++ debugmcp/tools.go | 91 ++++++++++++++--- 2 files changed, 289 insertions(+), 14 deletions(-) create mode 100644 debugmcp/schema_test.go diff --git a/debugmcp/schema_test.go b/debugmcp/schema_test.go new file mode 100644 index 0000000..9d252a8 --- /dev/null +++ b/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) + } +} diff --git a/debugmcp/tools.go b/debugmcp/tools.go index 9171bb3..8bae0f0 100644 --- a/debugmcp/tools.go +++ b/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"), }, } }