diff --git a/README.md b/README.md index ad3adf931..3645d1657 100644 --- a/README.md +++ b/README.md @@ -390,15 +390,14 @@ pad mcp install claude-desktop # or: cursor, windsurf, claude-code, codex, --a Cursor and Codex installations can opt into compact tool results: ```bash -pad mcp install cursor --structured-only -pad mcp install codex --structured-only +pad mcp install cursor --compact-results +pad mcp install codex --compact-results ``` -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 keeps the result channel each client currently exposes to its model: JSON +text for Cursor and `structuredContent` for Codex. It removes the duplicate +channel only from successful structured results; errors and one-channel results +remain unchanged. Leave the flag off for other clients or compatibility testing. `pad mcp install` writes each client's native config: JSON `mcpServers` for Claude Desktop / Cursor / Windsurf, a **project-local `.mcp.json`** in the current diff --git a/cmd/pad/mcp.go b/cmd/pad/mcp.go index 33c0550f3..e3e4e55da 100644 --- a/cmd/pad/mcp.go +++ b/cmd/pad/mcp.go @@ -47,7 +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 + var compactResults bool cmd := &cobra.Command{ Use: "install [agent]", Short: "Install pad as an MCP server for a client app", @@ -69,6 +69,10 @@ agents (run ` + "`pad mcp status`" + ` for the same view). With --all, installs for every per-user agent, creating config files on demand. +For Cursor or Codex, --compact-results removes the duplicate successful-result +channel while preserving the channel that client exposes to its model. It +configures text-only results for Cursor and structured-only results for Codex. + Existing MCP server entries (other servers configured by the user) are preserved — only the "pad" entry is touched.`, ValidArgs: agentValidArgs(), @@ -85,11 +89,28 @@ are preserved — only the "pad" entry is touched.`, if allFlag && len(args) > 0 { return fmt.Errorf("--all cannot be combined with an agent name") } + if compactResults && (allFlag || len(args) == 0) { + return fmt.Errorf("--compact-results requires an explicit cursor or codex agent") + } binary, err := os.Executable() if err != nil || binary == "" { binary = os.Args[0] } - inst := &mcpserver.Installer{Binary: binary, StructuredOnly: structuredOnly} + inst := &mcpserver.Installer{Binary: binary} + if compactResults { + agent, err := mcpserver.FindAgent(args[0]) + if err != nil { + return err + } + switch agent.Name { + case "cursor": + inst.TextOnly = true + case "codex": + inst.StructuredOnly = true + default: + return fmt.Errorf("--compact-results is supported only for cursor and codex") + } + } switch { case allFlag: return runMCPInstallAll(cmd, inst) @@ -101,7 +122,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") + cmd.Flags().BoolVar(&compactResults, "compact-results", false, "configure one model-visible result channel (Cursor/Codex)") return cmd } @@ -191,6 +212,8 @@ func runMCPInstallOne(cmd *cobra.Command, inst *mcpserver.Installer, agent strin serveArgs := "mcp serve" if inst.StructuredOnly { serveArgs += " --structured-only" + } else if inst.TextOnly { + serveArgs += " --text-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.") @@ -232,6 +255,7 @@ func runMCPInstallAll(cmd *cobra.Command, inst *mcpserver.Installer) error { func mcpServeCmd() *cobra.Command { var debug bool var structuredOnly bool + var textOnly bool cmd := &cobra.Command{ Use: "serve", Short: "Run the MCP server over stdio", @@ -243,7 +267,7 @@ etc.) per its mcp.json configuration. Direct human invocation is rare; when running interactively you'll see an idle process waiting for the client's initialize message. -The tool surface is the hand-curated v0.11 catalog — +The tool surface is the hand-curated v` + mcpserver.ToolSurfaceVersion + ` catalog — ten resource × action tools (` + "`pad_item`, `pad_workspace`, `pad_collection`, `pad_project`, `pad_role`, `pad_search`, `pad_meta`, `pad_playbook`, `pad_library`, `pad_attachment`" + `) plus ` + "`pad_set_workspace`" + ` for session-default workspace pinning. cmdhelp (v0.1) still drives per-command argument schemas at dispatch time, but the historical leaf-walker that @@ -256,6 +280,9 @@ Shuts down cleanly on EOF, SIGINT, or SIGTERM.`, // corrupt the JSON-RPC stream if a client misread the state. SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + if structuredOnly && textOnly { + return fmt.Errorf("--structured-only and --text-only are mutually exclusive") + } srv := mcpserver.NewServer(mcpserver.Options{ Version: fullVersion(), Debug: debug, @@ -329,6 +356,7 @@ Shuts down cleanly on EOF, SIGINT, or SIGTERM.`, PadVersion: fullVersion(), BootstrapFetcher: bootstrapFetcher, StructuredOnly: structuredOnly, + TextOnly: textOnly, }); err != nil { return fmt.Errorf("pad mcp serve: register tools: %w", err) } @@ -363,5 +391,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") + cmd.Flags().BoolVar(&textOnly, "text-only", false, "omit duplicate structuredContent while keeping JSON text") return cmd } diff --git a/cmd/pad/mcp_compact_results_test.go b/cmd/pad/mcp_compact_results_test.go new file mode 100644 index 000000000..a948cbfc7 --- /dev/null +++ b/cmd/pad/mcp_compact_results_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestMCPInstallCompactResultsUsesClientVisibleChannel(t *testing.T) { + tests := []struct { + agent string + path string + want string + drop string + }{ + {agent: "cursor", path: filepath.Join(".cursor", "mcp.json"), want: "--text-only", drop: "--structured-only"}, + {agent: "codex", path: filepath.Join(".codex", "config.toml"), want: "--structured-only", drop: "--text-only"}, + } + for _, tt := range tests { + t.Run(tt.agent, func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + cmd := mcpInstallCmd() + cmd.SetArgs([]string{tt.agent, "--compact-results"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + body, err := os.ReadFile(filepath.Join(home, tt.path)) + if err != nil { + t.Fatalf("read installed config: %v", err) + } + if !strings.Contains(string(body), tt.want) || strings.Contains(string(body), tt.drop) { + t.Fatalf("installed config = %s; want %s and not %s", body, tt.want, tt.drop) + } + }) + } +} + +func TestMCPInstallCompactResultsRejectsUnsupportedClient(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + cmd := mcpInstallCmd() + cmd.SetArgs([]string{"claude-desktop", "--compact-results"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "supported only for cursor and codex") { + t.Fatalf("Execute error = %v, want unsupported-client error", err) + } +} + +func TestMCPServeRejectsConflictingResultModes(t *testing.T) { + cmd := mcpServeCmd() + cmd.SetArgs([]string{"--structured-only", "--text-only"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + t.Fatalf("Execute error = %v, want mutually exclusive modes", err) + } +} diff --git a/internal/mcp/catalog.go b/internal/mcp/catalog.go index 53d34617f..bc5811bbe 100644 --- a/internal/mcp/catalog.go +++ b/internal/mcp/catalog.go @@ -136,8 +136,10 @@ type ActionEnv struct { Catalog []ToolDef // StructuredOnly removes duplicated text fallbacks from successful - // structured results. Opt-in because older MCP clients may only read text. + // structured results. TextOnly removes the structured copy instead for + // clients that expose only text results to the model. StructuredOnly bool + TextOnly bool } // Dispatch is the helper most fan-out actions use. Looks up cmdInfo @@ -208,6 +210,7 @@ type CatalogOptions struct { RootFlags map[string]string PadVersion string StructuredOnly bool + TextOnly bool } // RegisterCatalog installs the v0.2 catalog tools on srv. Returns the @@ -229,6 +232,7 @@ func RegisterCatalog(srv *server.MCPServer, opts CatalogOptions) (int, error) { PadVersion: opts.PadVersion, Catalog: Catalog, StructuredOnly: opts.StructuredOnly, + TextOnly: opts.TextOnly, } count := 0 for _, def := range Catalog { @@ -241,6 +245,9 @@ func RegisterCatalog(srv *server.MCPServer, opts CatalogOptions) (int, error) { } func validateCatalogOptions(opts CatalogOptions) error { + if opts.StructuredOnly && opts.TextOnly { + return &catalogError{msg: "CatalogOptions.StructuredOnly and TextOnly are mutually exclusive"} + } if opts.Doc == nil { return errCatalogMissing("Doc") } @@ -431,21 +438,27 @@ func makeFanOutHandler(def ToolDef, env ActionEnv) server.ToolHandlerFunc { stripped[k] = v } result, err := handler(ctx, stripped, env) - return applyStructuredOnly(result, err, env.StructuredOnly) + return applyResultMode(result, err, env.StructuredOnly, env.TextOnly) } } -// 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 { +// applyResultMode keeps only the result channel the configured client exposes +// to its model. Errors and one-channel results stay untouched so compact mode +// can never turn a useful result into an empty one. +func applyResultMode(result *mcp.CallToolResult, err error, structuredOnly, textOnly bool) (*mcp.CallToolResult, error) { + if err != nil || result == nil || result.IsError || result.StructuredContent == nil { return result, err } trimmed := *result - trimmed.Content = []mcp.Content{} - return &trimmed, nil + if structuredOnly { + trimmed.Content = []mcp.Content{} + return &trimmed, nil + } + if textOnly && len(result.Content) > 0 { + trimmed.StructuredContent = nil + return &trimmed, nil + } + return result, err } // compatAcceptedInputKeys lists input keys that are ACCEPTED without diff --git a/internal/mcp/catalog_test.go b/internal/mcp/catalog_test.go index 0296344c8..7bab9bafd 100644 --- a/internal/mcp/catalog_test.go +++ b/internal/mcp/catalog_test.go @@ -45,6 +45,7 @@ func TestRegisterCatalog_RequiresOptions(t *testing.T) { {"missing Doc", CatalogOptions{Workspace: NewWorkspaceState(""), Dispatcher: &fakeDispatcher{}}, "Doc"}, {"missing Workspace", CatalogOptions{Doc: fixtureDoc(), Dispatcher: &fakeDispatcher{}}, "Workspace"}, {"missing Dispatcher", CatalogOptions{Doc: fixtureDoc(), Workspace: NewWorkspaceState("")}, "Dispatcher"}, + {"conflicting result modes", CatalogOptions{Doc: fixtureDoc(), Workspace: NewWorkspaceState(""), Dispatcher: &fakeDispatcher{}, StructuredOnly: true, TextOnly: true}, "mutually exclusive"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -151,7 +152,7 @@ func TestMakeFanOutHandler_DispatchesPerAction(t *testing.T) { } } -func TestMakeFanOutHandler_StructuredOnlyRemovesDuplicateSuccessText(t *testing.T) { +func TestMakeFanOutHandler_CompactModesKeepTheClientVisibleChannel(t *testing.T) { payload := map[string]any{"body": strings.Repeat("work context ", 400)} raw, err := json.Marshal(payload) if err != nil { @@ -170,30 +171,45 @@ func TestMakeFanOutHandler_StructuredOnlyRemovesDuplicateSuccessText(t *testing. if err != nil { t.Fatal(err) } - modern, err := makeFanOutHandler(def, ActionEnv{StructuredOnly: true})(context.Background(), req) + structured, 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 structured.Content == nil || len(structured.Content) != 0 { + t.Fatalf("structured-only content = %#v, want non-nil empty array", structured.Content) } - if modern.StructuredContent == nil { + if structured.StructuredContent == nil { t.Fatal("structured-only mode dropped structuredContent") } + textOnly, err := makeFanOutHandler(def, ActionEnv{TextOnly: true})(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if len(textOnly.Content) == 0 { + t.Fatal("text-only mode dropped content") + } + if textOnly.StructuredContent != nil { + t.Fatal("text-only mode retained duplicate 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)) + structuredJSON, _ := json.Marshal(structured) + textJSON, _ := json.Marshal(textOnly) + t.Logf("representative structured result: %d bytes default, %d bytes structured-only, %d bytes text-only", len(legacyJSON), len(structuredJSON), len(textJSON)) + for mode, compactLen := range map[string]int{"structured-only": len(structuredJSON), "text-only": len(textJSON)} { + if compactLen*3 >= len(legacyJSON)*2 { + t.Errorf("%s mode should remove roughly half the duplicated payload: default=%d compact=%d", mode, len(legacyJSON), compactLen) + } } 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") + for _, modes := range [][2]bool{{true, false}, {false, true}} { + kept, err := applyResultMode(errorResult, nil, modes[0], modes[1]) + if err != nil || len(kept.Content) == 0 || kept.StructuredContent == nil { + t.Fatal("structured errors must retain both diagnostic channels") + } } } diff --git a/internal/mcp/install.go b/internal/mcp/install.go index 706aedb08..b4d582133 100644 --- a/internal/mcp/install.go +++ b/internal/mcp/install.go @@ -591,6 +591,9 @@ type Installer struct { // StructuredOnly opts installed clients into the token-efficient result // mode by appending --structured-only to `pad mcp serve`. StructuredOnly bool + // TextOnly selects the equivalent text-preserving mode for clients that do + // not expose structured-only results to the model. + TextOnly bool // Home overrides os.UserHomeDir when non-empty (test-only). Home string // CWD overrides os.Getwd when non-empty (test-only). Used to resolve @@ -659,6 +662,9 @@ func (i *Installer) Install(agentName string) (string, bool, error) { if i.Binary == "" { return "", false, errors.New("Installer.Binary is required") } + if i.StructuredOnly && i.TextOnly { + return "", false, errors.New("Installer.StructuredOnly and TextOnly are mutually exclusive") + } agent, err := FindAgent(agentName) if err != nil { return "", false, err @@ -670,6 +676,8 @@ func (i *Installer) Install(agentName string) (string, bool, error) { args := []string{"mcp", "serve"} if i.StructuredOnly { args = append(args, "--structured-only") + } else if i.TextOnly { + args = append(args, "--text-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 e9199b8cc..20dc9d9c3 100644 --- a/internal/mcp/install_test.go +++ b/internal/mcp/install_test.go @@ -301,17 +301,27 @@ 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) { +func TestInstaller_CompactModesConfigureCursorAndCodex(t *testing.T) { + tests := []struct { + agent string + structuredOnly bool + textOnly bool + modeArg any + }{ + {agent: "cursor", textOnly: true, modeArg: "--text-only"}, + {agent: "codex", structuredOnly: true, modeArg: "--structured-only"}, + } + for _, tt := range tests { + t.Run(tt.agent, func(t *testing.T) { tmpHome := t.TempDir() inst := &Installer{ Binary: "/usr/local/bin/pad", Home: tmpHome, GOOS: "linux", - StructuredOnly: true, + StructuredOnly: tt.structuredOnly, + TextOnly: tt.textOnly, } - path, modified, err := inst.Install(agent) + path, modified, err := inst.Install(tt.agent) if err != nil { t.Fatalf("Install: %v", err) } @@ -320,14 +330,14 @@ func TestInstaller_StructuredOnlyConfiguresCursorAndCodex(t *testing.T) { } var args []any - if agent == "codex" { + if tt.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"} + want := []any{"mcp", "serve", tt.modeArg} if !reflect.DeepEqual(args, want) { t.Errorf("args = %#v, want %#v", args, want) } @@ -335,6 +345,19 @@ func TestInstaller_StructuredOnlyConfiguresCursorAndCodex(t *testing.T) { } } +func TestInstaller_RejectsConflictingCompactModes(t *testing.T) { + inst := &Installer{ + Binary: "/usr/local/bin/pad", + Home: t.TempDir(), + GOOS: "linux", + StructuredOnly: true, + TextOnly: true, + } + if _, _, err := inst.Install("cursor"); err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + t.Fatalf("Install error = %v, want mutually exclusive modes", err) + } +} + 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 4dc12bc38..783d18575 100644 --- a/internal/mcp/registry.go +++ b/internal/mcp/registry.go @@ -55,9 +55,10 @@ type RegistryOptions struct { // 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 and TextOnly select one successful result channel for + // clients that expose only that channel to the model. Both are opt-in. StructuredOnly bool + TextOnly bool } // Register installs pad's MCP tools on srv: the built-in @@ -79,6 +80,9 @@ func Register(srv *server.MCPServer, opts RegistryOptions) (int, error) { if opts.Dispatcher == nil { return 0, fmt.Errorf("RegistryOptions.Dispatcher is required") } + if opts.StructuredOnly && opts.TextOnly { + return 0, fmt.Errorf("RegistryOptions.StructuredOnly and TextOnly are mutually exclusive") + } registerSetWorkspaceTool(srv, opts.Workspace, opts.BootstrapFetcher) count := 1 // pad_set_workspace @@ -90,6 +94,7 @@ func Register(srv *server.MCPServer, opts RegistryOptions) (int, error) { RootFlags: opts.RootFlags, PadVersion: opts.PadVersion, StructuredOnly: opts.StructuredOnly, + TextOnly: opts.TextOnly, }) if err != nil { return 0, fmt.Errorf("register catalog: %w", err) diff --git a/internal/mcp/registry_test.go b/internal/mcp/registry_test.go index 91664c588..d2ef31711 100644 --- a/internal/mcp/registry_test.go +++ b/internal/mcp/registry_test.go @@ -120,6 +120,7 @@ func TestRegister_ValidatesRequiredOptions(t *testing.T) { {"missing Doc", RegistryOptions{Workspace: state, Dispatcher: disp}, "Doc"}, {"missing Workspace", RegistryOptions{Doc: doc, Dispatcher: disp}, "Workspace"}, {"missing Dispatcher", RegistryOptions{Doc: doc, Workspace: state}, "Dispatcher"}, + {"conflicting result modes", RegistryOptions{Doc: doc, Workspace: state, Dispatcher: disp, StructuredOnly: true, TextOnly: true}, "mutually exclusive"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) {