diff --git a/README.md b/README.md index 539b1096f..ad3adf931 100644 --- a/README.md +++ b/README.md @@ -387,6 +387,19 @@ pad mcp install claude-desktop # or: cursor, windsurf, claude-code, codex, --a # Restart the client; pad shows up as the "pad" MCP server. ``` +Cursor and Codex installations can opt into compact tool results: + +```bash +pad mcp install cursor --structured-only +pad mcp install codex --structured-only +``` + +This keeps successful `structuredContent` and sends an empty required `content` +array instead of repeating the same JSON as text. Errors and text-only results +still carry text. Leave the flag off for clients that only read compatibility +fallbacks; MCP recommends the duplicate for backward compatibility but does not +require it. + `pad mcp install` writes each client's native config: JSON `mcpServers` for Claude Desktop / Cursor / Windsurf, a **project-local `.mcp.json`** in the current directory for `claude-code`, and an `[mcp_servers.pad]` table in diff --git a/cmd/pad/mcp.go b/cmd/pad/mcp.go index 74b1ba469..33c0550f3 100644 --- a/cmd/pad/mcp.go +++ b/cmd/pad/mcp.go @@ -47,6 +47,7 @@ See https://getpad.dev/mcp/local for client configuration.`, // for every supported agent (creating config dirs as needed). func mcpInstallCmd() *cobra.Command { var allFlag bool + var structuredOnly bool cmd := &cobra.Command{ Use: "install [agent]", Short: "Install pad as an MCP server for a client app", @@ -88,7 +89,7 @@ are preserved — only the "pad" entry is touched.`, if err != nil || binary == "" { binary = os.Args[0] } - inst := &mcpserver.Installer{Binary: binary} + inst := &mcpserver.Installer{Binary: binary, StructuredOnly: structuredOnly} switch { case allFlag: return runMCPInstallAll(cmd, inst) @@ -100,6 +101,7 @@ are preserved — only the "pad" entry is touched.`, }, } cmd.Flags().BoolVar(&allFlag, "all", false, "install for every supported agent") + cmd.Flags().BoolVar(&structuredOnly, "structured-only", false, "omit duplicate structured-result JSON text") return cmd } @@ -186,7 +188,11 @@ func runMCPInstallOne(cmd *cobra.Command, inst *mcpserver.Installer, agent strin } w := cmd.OutOrStdout() if modified { - fmt.Fprintf(w, "Installed pad MCP entry for %s\n config: %s\n command: %s mcp serve\n", agent, path, inst.Binary) + serveArgs := "mcp serve" + if inst.StructuredOnly { + serveArgs += " --structured-only" + } + fmt.Fprintf(w, "Installed pad MCP entry for %s\n config: %s\n command: %s %s\n", agent, path, inst.Binary, serveArgs) fmt.Fprintln(w, " → Restart the client to pick up the new server entry.") } else { fmt.Fprintf(w, "%s already up to date\n config: %s\n", agent, path) @@ -225,6 +231,7 @@ func runMCPInstallAll(cmd *cobra.Command, inst *mcpserver.Installer) error { // drive them directly. func mcpServeCmd() *cobra.Command { var debug bool + var structuredOnly bool cmd := &cobra.Command{ Use: "serve", Short: "Run the MCP server over stdio", @@ -321,6 +328,7 @@ Shuts down cleanly on EOF, SIGINT, or SIGTERM.`, RootFlags: rootFlags, PadVersion: fullVersion(), BootstrapFetcher: bootstrapFetcher, + StructuredOnly: structuredOnly, }); err != nil { return fmt.Errorf("pad mcp serve: register tools: %w", err) } @@ -354,5 +362,6 @@ Shuts down cleanly on EOF, SIGINT, or SIGTERM.`, }, } cmd.Flags().BoolVar(&debug, "debug", false, "verbose logging on stderr (development)") + cmd.Flags().BoolVar(&structuredOnly, "structured-only", false, "omit duplicate structured-result JSON text") return cmd } diff --git a/internal/mcp/catalog.go b/internal/mcp/catalog.go index 619a3f745..53d34617f 100644 --- a/internal/mcp/catalog.go +++ b/internal/mcp/catalog.go @@ -134,6 +134,10 @@ type ActionEnv struct { // Catalog is the live catalog slice — supplied so pad_meta.action: // tool-surface can introspect the catalog without an import cycle. Catalog []ToolDef + + // StructuredOnly removes duplicated text fallbacks from successful + // structured results. Opt-in because older MCP clients may only read text. + StructuredOnly bool } // Dispatch is the helper most fan-out actions use. Looks up cmdInfo @@ -198,11 +202,12 @@ func appendToCatalog(def ToolDef) { // CatalogOptions configures RegisterCatalog. Mirrors RegistryOptions // for the cmdhelp-walk path but adds PadVersion (needed by pad_meta). type CatalogOptions struct { - Doc *cmdhelp.Document - Workspace *WorkspaceState - Dispatcher Dispatcher - RootFlags map[string]string - PadVersion string + Doc *cmdhelp.Document + Workspace *WorkspaceState + Dispatcher Dispatcher + RootFlags map[string]string + PadVersion string + StructuredOnly bool } // RegisterCatalog installs the v0.2 catalog tools on srv. Returns the @@ -217,12 +222,13 @@ func RegisterCatalog(srv *server.MCPServer, opts CatalogOptions) (int, error) { return 0, err } env := ActionEnv{ - Doc: opts.Doc, - Workspace: opts.Workspace, - Dispatcher: opts.Dispatcher, - RootFlags: opts.RootFlags, - PadVersion: opts.PadVersion, - Catalog: Catalog, + Doc: opts.Doc, + Workspace: opts.Workspace, + Dispatcher: opts.Dispatcher, + RootFlags: opts.RootFlags, + PadVersion: opts.PadVersion, + Catalog: Catalog, + StructuredOnly: opts.StructuredOnly, } count := 0 for _, def := range Catalog { @@ -424,10 +430,24 @@ func makeFanOutHandler(def ToolDef, env ActionEnv) server.ToolHandlerFunc { } stripped[k] = v } - return handler(ctx, stripped, env) + result, err := handler(ctx, stripped, env) + return applyStructuredOnly(result, err, env.StructuredOnly) } } +// applyStructuredOnly keeps MCP's required content array but removes the +// serialized JSON fallback when a modern client has explicitly opted into +// structuredContent. Errors and text-only results retain their text so an +// agent can still diagnose and recover from a failed call. +func applyStructuredOnly(result *mcp.CallToolResult, err error, enabled bool) (*mcp.CallToolResult, error) { + if !enabled || err != nil || result == nil || result.IsError || result.StructuredContent == nil { + return result, err + } + trimmed := *result + trimmed.Content = []mcp.Content{} + return &trimmed, nil +} + // compatAcceptedInputKeys lists input keys that are ACCEPTED without // being schema-declared — documented compatibility forms that predate // strict input validation (#1066 / ToolSurfaceVersion 0.24). Additions diff --git a/internal/mcp/catalog_test.go b/internal/mcp/catalog_test.go index d9f90f17f..0296344c8 100644 --- a/internal/mcp/catalog_test.go +++ b/internal/mcp/catalog_test.go @@ -151,6 +151,52 @@ func TestMakeFanOutHandler_DispatchesPerAction(t *testing.T) { } } +func TestMakeFanOutHandler_StructuredOnlyRemovesDuplicateSuccessText(t *testing.T) { + payload := map[string]any{"body": strings.Repeat("work context ", 400)} + raw, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + def := ToolDef{ + Name: "pad_test", + Actions: map[string]ActionFn{ + "get": func(_ context.Context, _ map[string]any, _ ActionEnv) (*mcp.CallToolResult, error) { + return mcp.NewToolResultStructured(payload, string(raw)), nil + }, + }, + } + req := callToolRequest(map[string]any{"action": "get"}) + legacy, err := makeFanOutHandler(def, ActionEnv{})(context.Background(), req) + if err != nil { + t.Fatal(err) + } + modern, err := makeFanOutHandler(def, ActionEnv{StructuredOnly: true})(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if len(legacy.Content) == 0 { + t.Fatal("default mode must retain the compatibility text fallback") + } + if modern.Content == nil || len(modern.Content) != 0 { + t.Fatalf("structured-only content = %#v, want non-nil empty array", modern.Content) + } + if modern.StructuredContent == nil { + t.Fatal("structured-only mode dropped structuredContent") + } + legacyJSON, _ := json.Marshal(legacy) + modernJSON, _ := json.Marshal(modern) + t.Logf("representative structured result: %d bytes default, %d bytes structured-only", len(legacyJSON), len(modernJSON)) + if len(modernJSON)*3 >= len(legacyJSON)*2 { + t.Errorf("structured-only mode should remove roughly half the duplicated payload: default=%d modern=%d", len(legacyJSON), len(modernJSON)) + } + + errorResult := NewErrorResult(ErrorPayload{Code: ErrValidationFailed, Message: "fix the input"}) + kept, err := applyStructuredOnly(errorResult, nil, true) + if err != nil || len(kept.Content) == 0 { + t.Fatal("structured errors must retain their diagnostic text fallback") + } +} + // TestMakeFanOutHandler_MissingAction returns a structured error // listing valid actions so the agent can self-correct without a // human round-trip. Wire-level guarantee: IsError = true, message diff --git a/internal/mcp/install.go b/internal/mcp/install.go index f0b91c559..706aedb08 100644 --- a/internal/mcp/install.go +++ b/internal/mcp/install.go @@ -209,6 +209,10 @@ func codexPathFor(home, _ string) (string, error) { // Returns (modified=true, nil) when the on-disk content actually // changed, (false, nil) when the file was already up to date. func AddPadEntry(path, binary string) (bool, error) { + return addPadEntryJSON(path, binary, []string{"mcp", "serve"}) +} + +func addPadEntryJSON(path, binary string, args []string) (bool, error) { if binary == "" { return false, errors.New("AddPadEntry: binary path is required") } @@ -218,7 +222,7 @@ func AddPadEntry(path, binary string) (bool, error) { } wantedEntry := map[string]any{ "command": binary, - "args": []any{"mcp", "serve"}, + "args": stringsToAny(args), } servers, _ := cfg["mcpServers"].(map[string]any) if servers == nil { @@ -403,6 +407,10 @@ func jsonEqual(a, b map[string]any) bool { // unrelated keys and other mcp_servers entries are preserved. Returns // (modified=true, nil) only when the on-disk content actually changed. func addPadEntryTOML(path, binary string) (bool, error) { + return addPadEntryTOMLWithArgs(path, binary, []string{"mcp", "serve"}) +} + +func addPadEntryTOMLWithArgs(path, binary string, args []string) (bool, error) { if binary == "" { return false, errors.New("addPadEntryTOML: binary path is required") } @@ -412,7 +420,7 @@ func addPadEntryTOML(path, binary string) (bool, error) { } wantedEntry := map[string]any{ "command": binary, - "args": []any{"mcp", "serve"}, + "args": stringsToAny(args), } // If mcp_servers exists but isn't a table (e.g. a scalar left by a // hand-edit or an incompatible tool), refuse rather than silently @@ -543,11 +551,19 @@ func writeTOMLConfig(path string, cfg map[string]any) error { // read and written in its native format. JSON is the default; TOML is // Codex-only for now. -func addEntry(agent *Agent, path, binary string) (bool, error) { +func addEntry(agent *Agent, path, binary string, args []string) (bool, error) { if agent.Format == formatTOML { - return addPadEntryTOML(path, binary) + return addPadEntryTOMLWithArgs(path, binary, args) } - return AddPadEntry(path, binary) + return addPadEntryJSON(path, binary, args) +} + +func stringsToAny(values []string) []any { + out := make([]any, len(values)) + for i, value := range values { + out[i] = value + } + return out } func removeEntry(agent *Agent, path string) (bool, error) { @@ -572,6 +588,9 @@ func hasEntry(agent *Agent, path string) (bool, string, error) { type Installer struct { // Binary is the pad executable to register. Required for Install. Binary string + // StructuredOnly opts installed clients into the token-efficient result + // mode by appending --structured-only to `pad mcp serve`. + StructuredOnly bool // Home overrides os.UserHomeDir when non-empty (test-only). Home string // CWD overrides os.Getwd when non-empty (test-only). Used to resolve @@ -648,7 +667,11 @@ func (i *Installer) Install(agentName string) (string, bool, error) { if err != nil { return "", false, err } - modified, err := addEntry(agent, path, i.Binary) + args := []string{"mcp", "serve"} + if i.StructuredOnly { + args = append(args, "--structured-only") + } + modified, err := addEntry(agent, path, i.Binary, args) return path, modified, err } diff --git a/internal/mcp/install_test.go b/internal/mcp/install_test.go index a14192add..e9199b8cc 100644 --- a/internal/mcp/install_test.go +++ b/internal/mcp/install_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" @@ -300,6 +301,40 @@ func TestInstaller_Install_RoundTripWithTempHome(t *testing.T) { } } +func TestInstaller_StructuredOnlyConfiguresCursorAndCodex(t *testing.T) { + for _, agent := range []string{"cursor", "codex"} { + t.Run(agent, func(t *testing.T) { + tmpHome := t.TempDir() + inst := &Installer{ + Binary: "/usr/local/bin/pad", + Home: tmpHome, + GOOS: "linux", + StructuredOnly: true, + } + path, modified, err := inst.Install(agent) + if err != nil { + t.Fatalf("Install: %v", err) + } + if !modified { + t.Fatal("fresh install should modify config") + } + + var args []any + if agent == "codex" { + cfg := readTOML(t, path) + args = cfg[codexServersKey].(map[string]any)[MCPServerKey].(map[string]any)["args"].([]any) + } else { + cfg := readConfig(t, path) + args = cfg["mcpServers"].(map[string]any)[MCPServerKey].(map[string]any)["args"].([]any) + } + want := []any{"mcp", "serve", "--structured-only"} + if !reflect.DeepEqual(args, want) { + t.Errorf("args = %#v, want %#v", args, want) + } + }) + } +} + func TestInstaller_Install_MissingBinaryErrors(t *testing.T) { inst := &Installer{Home: t.TempDir(), GOOS: "linux"} if _, _, err := inst.Install("cursor"); err == nil { diff --git a/internal/mcp/registry.go b/internal/mcp/registry.go index 4f6b9fb2f..4dc12bc38 100644 --- a/internal/mcp/registry.go +++ b/internal/mcp/registry.go @@ -54,6 +54,10 @@ type RegistryOptions struct { // fetch the blob separately via pad_meta.action=bootstrap or // pad://workspace/{ws}/bootstrap. PLAN-1377 / TASK-1380. BootstrapFetcher BootstrapFetcher + + // StructuredOnly removes text copies of successful structured tool + // results. It is opt-in for clients that consume structuredContent. + StructuredOnly bool } // Register installs pad's MCP tools on srv: the built-in @@ -80,11 +84,12 @@ func Register(srv *server.MCPServer, opts RegistryOptions) (int, error) { count := 1 // pad_set_workspace catalogCount, err := RegisterCatalog(srv, CatalogOptions{ - Doc: opts.Doc, - Workspace: opts.Workspace, - Dispatcher: opts.Dispatcher, - RootFlags: opts.RootFlags, - PadVersion: opts.PadVersion, + Doc: opts.Doc, + Workspace: opts.Workspace, + Dispatcher: opts.Dispatcher, + RootFlags: opts.RootFlags, + PadVersion: opts.PadVersion, + StructuredOnly: opts.StructuredOnly, }) if err != nil { return 0, fmt.Errorf("register catalog: %w", err)