refactor(mcp): route tool results through a Pad-owned seam (TASK-2306 unit 2) (#1611)

internal/mcp/result.go declares CallToolResult as an alias of mcp-go's type,
plus textResult / structuredResult / errorResult / errorResultf wrapping the
SDK's four constructors. Every non-test site now goes through it: 78 type
references and 18 constructor calls across 17 files. 15 files no longer
import the SDK's mcp package at all.

This is behavior-neutral by construction (an alias and pure delegation). The
initialize and tools/list goldens from unit 1 are byte-unchanged, and the mcp
package tests, which cover the tools/call path the goldens do not, pass.

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
This commit is contained in:
xarmian
2026-09-27 11:00:34 -04:00
committed by GitHub
parent c29af5731c
commit 6b85f6881e
18 changed files with 132 additions and 125 deletions
+8 -8
View File
@@ -106,7 +106,7 @@ type ParamDef struct {
// 2. Inline (pad_meta + similar): build the CallToolResult directly
// from in-memory state (ToolSurfaceVersion, the catalog itself,
// etc.) without dispatching anywhere.
type ActionFn func(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error)
type ActionFn func(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error)
// ActionEnv carries everything an action handler needs to either
// dispatch to a CLI cmdPath or build a result inline. Wired up once
@@ -151,7 +151,7 @@ type ActionEnv struct {
// Unknown cmdPath produces a "registry bug" error rather than panicking
// — catches catalog/cmdhelp drift early in tests. A clean run on
// startup ensures every action's cmdPath maps to a real CLI command.
func (env ActionEnv) Dispatch(ctx context.Context, cmdPath []string, input map[string]any) (*mcp.CallToolResult, error) {
func (env ActionEnv) Dispatch(ctx context.Context, cmdPath []string, input map[string]any) (*CallToolResult, error) {
if input == nil {
input = map[string]any{}
}
@@ -184,7 +184,7 @@ func (env ActionEnv) Dispatch(ctx context.Context, cmdPath []string, input map[s
// reshapes the input or dispatches on a parameter (see actionItemLink
// for the link_type → cmdPath dispatch pattern).
func passThrough(cmdPath []string) ActionFn {
return func(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
return func(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
return env.Dispatch(ctx, cmdPath, input)
}
}
@@ -413,7 +413,7 @@ func paramDefToToolOption(p ParamDef) mcp.ToolOption {
// for that collision.
func makeFanOutHandler(def ToolDef, env ActionEnv) server.ToolHandlerFunc {
declared := declaredInputKeys(def)
return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return func(ctx context.Context, req mcp.CallToolRequest) (*CallToolResult, error) {
input := withFieldNumberLiterals(req.GetArguments(), req.Params.RawArguments)
if input == nil {
input = map[string]any{}
@@ -491,7 +491,7 @@ func withFieldNumberLiterals(input map[string]any, raw json.RawMessage) map[stri
// 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) {
func applyResultMode(result *CallToolResult, err error, structuredOnly, textOnly bool) (*CallToolResult, error) {
if err != nil || result == nil || result.IsError || result.StructuredContent == nil {
return result, err
}
@@ -561,7 +561,7 @@ func declaredInputKeys(def ToolDef) map[string]bool {
// keys, so a typo'd or misplaced param was accepted, did nothing, and
// still returned success — the worst version of wrong. Nil result
// means the input is clean.
func rejectUndeclaredKeys(def ToolDef, declared map[string]bool, input map[string]any) *mcp.CallToolResult {
func rejectUndeclaredKeys(def ToolDef, declared map[string]bool, input map[string]any) *CallToolResult {
var unknown []string
for k := range input {
if !declared[k] {
@@ -601,7 +601,7 @@ func sortedActionNames(def ToolDef) []string {
// errMissingAction is the structured result returned when the input
// has no `action` field. Lists the valid actions inline so agents can
// retry with a correct value.
func errMissingAction(def ToolDef) *mcp.CallToolResult {
func errMissingAction(def ToolDef) *CallToolResult {
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: fmt.Sprintf("%s: missing required field 'action'", def.Name),
@@ -612,7 +612,7 @@ func errMissingAction(def ToolDef) *mcp.CallToolResult {
// errUnknownAction is the structured result returned when `action` is
// set but not in the tool's action table. Same listing as
// errMissingAction so agents can self-correct.
func errUnknownAction(def ToolDef, action string) *mcp.CallToolResult {
func errUnknownAction(def ToolDef, action string) *CallToolResult {
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: fmt.Sprintf("%s: unknown action %q", def.Name, action),
+8 -10
View File
@@ -6,8 +6,6 @@ import (
"os"
"sort"
"strings"
"github.com/mark3labs/mcp-go/mcp"
)
// padItemTool is the v0.2 tool that consolidates the ~20 v0.1 verb
@@ -569,7 +567,7 @@ var itemLinkRoutes = map[string]itemLinkRoute{
// the valid options, same shape as makeFanOutHandler's errMissingAction.
// - missing ref or target → BuildCLIArgs catches it as a missing-
// positional error.
func actionItemLink(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemLink(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
cmdPath, dispatchInput, err := resolveItemLink(input, true)
if err != nil {
return errStructured("pad_item.link", err), nil
@@ -579,7 +577,7 @@ func actionItemLink(ctx context.Context, input map[string]any, env ActionEnv) (*
// actionItemUnlink is the symmetric un-create operation. Same routing
// rules; uses route.unlinkCmdPath instead of route.linkCmdPath.
func actionItemUnlink(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemUnlink(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
cmdPath, dispatchInput, err := resolveItemLink(input, false)
if err != nil {
return errStructured("pad_item.unlink", err), nil
@@ -677,7 +675,7 @@ func joinSorted(ss []string) string {
// link_type, etc.). If a future call site needs a different code,
// give it its own helper rather than overloading this one — keeps
// the code-per-call-site mapping explicit.
func errStructured(prefix string, err error) *mcp.CallToolResult {
func errStructured(prefix string, err error) *CallToolResult {
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: fmt.Sprintf("%s: %s", prefix, err.Error()),
@@ -695,7 +693,7 @@ func errStructured(prefix string, err error) *mcp.CallToolResult {
// `ref` (which is scalar across every other action) so the schema
// stays consistent — agents see a single shape per param name. The
// rename happens here at dispatch time.
func actionItemBulkUpdate(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemBulkUpdate(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
rawRefs, ok := input["refs"]
if !ok || rawRefs == nil {
return errStructured("pad_item.bulk-update",
@@ -793,7 +791,7 @@ const (
// projection parameter). `full: true` opts into complete bodies on
// both. MCP callers that need one item's body should prefer
// action=get.
func actionItemList(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemList(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
// Early MCP-side feedback for the parent/unparented conflict; canonical
// enforcement lives in validateUnparentedListRequest
// (internal/server/handlers_items.go). Keep the two in sync.
@@ -838,7 +836,7 @@ const (
// The catalog is the right home for the default (rather than either
// dispatcher) for the reason actionItemList already documents — it is the one
// place both transports pass through.
func actionItemHistory(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemHistory(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
out := make(map[string]any, len(input)+1)
for k, v := range input {
out[k] = v
@@ -871,7 +869,7 @@ func actionItemHistory(ctx context.Context, input map[string]any, env ActionEnv)
//
// Read-only / side-effect-free: the server's export endpoint only
// reads the item.
func actionItemExport(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemExport(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
if ref, _ := input["ref"].(string); strings.TrimSpace(ref) == "" {
return errStructured("pad_item.export",
fmt.Errorf("ref is required (PLAYB-N, CONVE-N, or slug)")), nil
@@ -903,7 +901,7 @@ func actionItemExport(ctx context.Context, input map[string]any, env ActionEnv)
// Mutating but not destructive — the server imports the artifact as a
// draft item (same risk profile as action=create), returning
// {ref, slug, warnings}.
func actionItemImport(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemImport(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
artifact, _ := input["artifact"].(string)
if strings.TrimSpace(artifact) == "" {
return errStructured("pad_item.import",
+6 -8
View File
@@ -10,8 +10,6 @@ import (
"strconv"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/items"
)
@@ -208,7 +206,7 @@ func topLevelConflictKeys() []string {
// If you are about to add a condition here, say its QUANTIFIER out loud
// first. Per-key or per-request, this key class or all of them: rounds 15,
// 16, 17, 19, 20 and 21 were each that question answered by assumption.
func detectFieldConflicts(prefix string, input map[string]any) *mcp.CallToolResult {
func detectFieldConflicts(prefix string, input map[string]any) *CallToolResult {
// Parsed here rather than passed in, so this pass sees the same inputs
// whether or not a `fields` object exists (codex round 14). It used to
// run only from inside reshapeItemFields, which returns early without
@@ -576,7 +574,7 @@ var padItemPromotedFieldKeys = map[string]bool{
// the two writer actions: reshape the `fields` object (when present)
// into the dedicated-param + `field`-array paths, then dispatch as
// before. Calls without `fields` are byte-for-byte unchanged.
func actionItemCreate(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemCreate(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
// ONE conflict decision, over the canonical view of every source, run
// whether or not a `fields` object is present (lead ruling after round
// 13; reach corrected after round 14).
@@ -590,7 +588,7 @@ func actionItemCreate(ctx context.Context, input map[string]any, env ActionEnv)
return env.Dispatch(ctx, []string{"item", "create"}, out)
}
func actionItemUpdate(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionItemUpdate(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
// ONE conflict decision, over the canonical view of every source, run
// whether or not a `fields` object is present (lead ruling after round
// 13; reach corrected after round 14).
@@ -638,7 +636,7 @@ func dropInertEmptyParent(input map[string]any) {
// no-op this contract removes (an agent trying `fields` as a list
// filter would be the obvious casualty).
func rejectFieldsParam(prefix string, fn ActionFn) ActionFn {
return func(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
return func(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
if _, present := input["fields"]; present {
return errStructured(prefix, fmt.Errorf(
"fields is only accepted for action=create and action=update; for list filtering use the dedicated params (status, priority, ...)")), nil
@@ -651,7 +649,7 @@ func rejectFieldsParam(prefix string, fn ActionFn) ActionFn {
// `field`-array paths. Returns the reshaped input, or a structured
// error result (second return) that the caller surfaces as-is. Input
// without a `fields` key is returned unchanged.
func reshapeItemFields(prefix string, input map[string]any) (map[string]any, *mcp.CallToolResult) {
func reshapeItemFields(prefix string, input map[string]any) (map[string]any, *CallToolResult) {
raw, present := input["fields"]
if !present {
return input, nil
@@ -944,7 +942,7 @@ const fieldsNativeKey = "__fields_native"
// plus a key→value index. Entries the CLI would reject anyway (no '=')
// are passed through unindexed rather than pre-empting the CLI's own
// error surface.
func parseFieldArray(prefix string, raw any) ([]string, map[string]string, *mcp.CallToolResult) {
func parseFieldArray(prefix string, raw any) ([]string, map[string]string, *CallToolResult) {
byKey := map[string]string{}
if raw == nil {
return nil, byKey, nil
+10 -12
View File
@@ -3,8 +3,6 @@ package mcp
import (
"context"
"encoding/json"
"github.com/mark3labs/mcp-go/mcp"
)
// padMetaTool is the server-introspection tool (v0.4 catalog). Four actions —
@@ -82,7 +80,7 @@ or build documentation. For runtime task work, use pad_item / pad_workspace / et
// actionMetaServerInfo returns the minimal {name, version} pair.
// Roughly equivalent to what the MCP handshake exposes, but available
// post-handshake without a full reconnect.
func actionMetaServerInfo(_ context.Context, _ map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionMetaServerInfo(_ context.Context, _ map[string]any, env ActionEnv) (*CallToolResult, error) {
version := env.PadVersion
if version == "" {
version = FallbackVersion
@@ -95,20 +93,20 @@ func actionMetaServerInfo(_ context.Context, _ map[string]any, env ActionEnv) (*
if err != nil {
// Marshalling a string-valued map can't realistically fail;
// surface as an error result rather than panicking.
return mcp.NewToolResultErrorf("pad_meta.server-info: marshal: %s", err.Error()), nil
return errorResultf("pad_meta.server-info: marshal: %s", err.Error()), nil
}
return mcp.NewToolResultStructured(payload, string(body)), nil
return structuredResult(payload, string(body)), nil
}
// actionMetaVersion returns the same MetaPayload that pad://_meta/version
// serves as a resource. Two surfaces, one source of truth (BuildMetaPayload).
func actionMetaVersion(_ context.Context, _ map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionMetaVersion(_ context.Context, _ map[string]any, env ActionEnv) (*CallToolResult, error) {
payload := BuildMetaPayload(env.PadVersion)
body, err := json.Marshal(payload)
if err != nil {
return mcp.NewToolResultErrorf("pad_meta.version: marshal: %s", err.Error()), nil
return errorResultf("pad_meta.version: marshal: %s", err.Error()), nil
}
return mcp.NewToolResultStructured(payload, string(body)), nil
return structuredResult(payload, string(body)), nil
}
// actionMetaBootstrap dispatches to `pad bootstrap` so MCP callers get
@@ -120,7 +118,7 @@ func actionMetaVersion(_ context.Context, _ map[string]any, env ActionEnv) (*mcp
// (Server.BuildAgentBootstrap) the single source of truth for shape,
// validation, and visibility filtering. The MCP surface contributes
// only the discovery affordance, not a divergent implementation.
func actionMetaBootstrap(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionMetaBootstrap(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
return env.Dispatch(ctx, []string{"bootstrap"}, input)
}
@@ -149,7 +147,7 @@ func actionMetaBootstrap(ctx context.Context, input map[string]any, env ActionEn
// and the per-tool ParamDefs. This makes the dump self-contained for
// docs generators: they don't have to reproduce buildToolFromDef's
// implicit-param logic separately.
func actionMetaToolSurface(_ context.Context, _ map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionMetaToolSurface(_ context.Context, _ map[string]any, env ActionEnv) (*CallToolResult, error) {
// Shared serializer (TASK-1891): buildToolSurfacePayload is the
// single source of truth for the tool-surface shape, consumed by
// both this MCP action and the cycle-free ToolSurfaceJSON() that
@@ -161,7 +159,7 @@ func actionMetaToolSurface(_ context.Context, _ map[string]any, env ActionEnv) (
payload := buildToolSurfacePayload(env.Catalog)
body, err := json.Marshal(payload)
if err != nil {
return mcp.NewToolResultErrorf("pad_meta.tool-surface: marshal: %s", err.Error()), nil
return errorResultf("pad_meta.tool-surface: marshal: %s", err.Error()), nil
}
return mcp.NewToolResultStructured(payload, string(body)), nil
return structuredResult(payload, string(body)), nil
}
+1 -3
View File
@@ -4,8 +4,6 @@ import (
"context"
"fmt"
"sort"
"github.com/mark3labs/mcp-go/mcp"
)
// padPlaybookTool exposes the first-class playbook surface from
@@ -105,7 +103,7 @@ var padPlaybookTool = ToolDef{
// supports bareword flag PRESENCE (no flag=false form), so a
// local-stdio caller cannot override a flag-default-true value;
// route through HTTP/in-process MCP for that rare case.
func actionPlaybookRun(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionPlaybookRun(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
ref, _ := input["ref"].(string)
if ref == "" {
// Use the structured validation envelope (matches what
+1 -3
View File
@@ -2,8 +2,6 @@ package mcp
import (
"context"
"github.com/mark3labs/mcp-go/mcp"
)
// padWorkspaceTool exposes workspace-level operations: discovery
@@ -109,7 +107,7 @@ var padWorkspaceTool = ToolDef{
// `action_filter` in the schema and rename here before dispatch.
//
// All other audit-log flags (actor, days, limit) flow through unchanged.
func actionWorkspaceAuditLog(ctx context.Context, input map[string]any, env ActionEnv) (*mcp.CallToolResult, error) {
func actionWorkspaceAuditLog(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
if v, ok := input["action_filter"]; ok {
// Clone so the caller's map (and req.Params.Arguments) stays
// untouched. Same defensive copy makeFanOutHandler does.
+9 -11
View File
@@ -9,13 +9,11 @@ import (
"sort"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/cmdhelp"
)
// Dispatcher executes a pad CLI command on behalf of a tool call. It
// always returns a *mcp.CallToolResult — error paths set IsError on
// always returns a *CallToolResult — error paths set IsError on
// the result so MCP clients see structured stderr without having to
// distinguish protocol errors from tool errors.
//
@@ -37,7 +35,7 @@ import (
// registry attaches that input via WithDispatchInput before calling
// Dispatch.
type Dispatcher interface {
Dispatch(ctx context.Context, cmdPath []string, cliArgs []string) (*mcp.CallToolResult, error)
Dispatch(ctx context.Context, cmdPath []string, cliArgs []string) (*CallToolResult, error)
}
// dispatchInputKey is the unexported context-key type used to forward
@@ -124,9 +122,9 @@ type ExecDispatcher struct {
// Dispatch runs `<Binary> <cmdPath...> <cliArgs...>` and packages the
// output for an MCP client.
func (d *ExecDispatcher) Dispatch(ctx context.Context, cmdPath []string, cliArgs []string) (*mcp.CallToolResult, error) {
func (d *ExecDispatcher) Dispatch(ctx context.Context, cmdPath []string, cliArgs []string) (*CallToolResult, error) {
if d.Binary == "" {
return mcp.NewToolResultError("dispatcher: binary path not configured"), nil
return errorResult("dispatcher: binary path not configured"), nil
}
// THIS dispatcher is the stdio door, and it cannot express a structured
// field value: it runs the CLI with `--field key=value` arguments, and a
@@ -176,14 +174,14 @@ func (d *ExecDispatcher) Dispatch(ctx context.Context, cmdPath []string, cliArgs
// Shared between ExecDispatcher and HTTPHandlerDispatcher's
// packageHTTPResponse so both transports produce the same wire
// shape.
func packageJSONResult(out string) *mcp.CallToolResult {
func packageJSONResult(out string) *CallToolResult {
trimmed := strings.TrimSpace(out)
if !strings.HasPrefix(trimmed, "{") && !strings.HasPrefix(trimmed, "[") {
return mcp.NewToolResultText(out)
return textResult(out)
}
var parsed any
if err := json.Unmarshal([]byte(trimmed), &parsed); err != nil {
return mcp.NewToolResultText(out)
return textResult(out)
}
parsed = normalizeStringEncodedJSONFields(parsed)
if arr, ok := parsed.([]any); ok {
@@ -192,9 +190,9 @@ func packageJSONResult(out string) *mcp.CallToolResult {
// array. The original JSON text is still returned as the text
// fallback so clients that don't parse structured content keep
// seeing the raw shape they used to.
return mcp.NewToolResultStructured(map[string]any{"items": arr}, out)
return structuredResult(map[string]any{"items": arr}, out)
}
return mcp.NewToolResultStructured(parsed, out)
return structuredResult(parsed, out)
}
// normalizeStringEncodedJSONFields recursively walks a parsed JSON
+5 -6
View File
@@ -14,7 +14,6 @@ import (
"sync"
"github.com/go-chi/chi/v5"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/items"
"github.com/PerpetualSoftware/pad/internal/models"
@@ -275,7 +274,7 @@ var noRemoteEquivalent = map[string]string{
// dispatcher does the merge).
// 6. Otherwise, look up a RouteMapper in routeTable, build the
// synthesized request, and execute through the handler chain.
func (d *HTTPHandlerDispatcher) Dispatch(ctx context.Context, cmdPath, _ []string) (*mcp.CallToolResult, error) {
func (d *HTTPHandlerDispatcher) Dispatch(ctx context.Context, cmdPath, _ []string) (*CallToolResult, error) {
if d.Handler == nil {
return NewErrorResult(ErrorPayload{
Code: ErrServerError,
@@ -417,7 +416,7 @@ func (d *HTTPHandlerDispatcher) Dispatch(ctx context.Context, cmdPath, _ []strin
// specialDispatchFn is the signature shared by the dispatcher's
// special-case route methods.
type specialDispatchFn func(context.Context, map[string]any, *models.User) (*mcp.CallToolResult, error)
type specialDispatchFn func(context.Context, map[string]any, *models.User) (*CallToolResult, error)
// specialRoutes maps cmdKeys to the dispatcher methods that handle
// them in place of a routeTable mapper — commands whose HTTP path
@@ -482,7 +481,7 @@ func (d *HTTPHandlerDispatcher) executeRequest(
user *models.User,
method, urlPath string,
body []byte,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
req, err := d.buildAuthedRequest(ctx, method, urlPath, body, user)
if err != nil {
return buildRequestErrorResult(cmdKey, err), nil
@@ -511,7 +510,7 @@ func (d *HTTPHandlerDispatcher) executeRequest(
// surface the SAME permission_denied codes as the routeTable path —
// without the extraction, a scope rejection on those paths degraded
// to a generic server_error.
func buildRequestErrorResult(cmdKey string, err error) *mcp.CallToolResult {
func buildRequestErrorResult(cmdKey string, err error) *CallToolResult {
// Most build-request failures are scope-rejection from
// buildAuthedRequest's TokenScopeAllows check (PATCH on a
// read-only token, etc.). Surface as permission_denied so
@@ -722,7 +721,7 @@ func buildHTTPRequest(ctx context.Context, method, urlPath string, body []byte,
// user's workspaces by the token's allow-list — leaking workspace
// slugs the user didn't consent to expose would be a privacy bug.
// nil → empty available_workspaces (legacy behaviour).
func packageHTTPResponse(ctx context.Context, cmdKey string, resp *http.Response, lister WorkspaceLister) (*mcp.CallToolResult, error) {
func packageHTTPResponse(ctx context.Context, cmdKey string, resp *http.Response, lister WorkspaceLister) (*CallToolResult, error) {
defer resp.Body.Close()
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
+1 -3
View File
@@ -10,8 +10,6 @@ import (
"net/url"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
)
@@ -281,7 +279,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemUpdate(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item update"
workspace, _ := input["workspace"].(string)
+1 -3
View File
@@ -7,8 +7,6 @@ import (
"net/url"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
)
@@ -44,7 +42,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemImport(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item import"
workspace, _ := input["workspace"].(string)
+2 -4
View File
@@ -10,8 +10,6 @@ import (
"path/filepath"
"strconv"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
)
@@ -36,7 +34,7 @@ func (d *HTTPHandlerDispatcher) dispatchAttachmentList(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "attachment list"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -117,7 +115,7 @@ func (d *HTTPHandlerDispatcher) dispatchAttachmentShow(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "attachment show"
workspace, _ := input["workspace"].(string)
if workspace == "" {
+2 -4
View File
@@ -8,8 +8,6 @@ import (
"net/http/httptest"
"net/url"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/artifact"
"github.com/PerpetualSoftware/pad/internal/collections"
"github.com/PerpetualSoftware/pad/internal/models"
@@ -44,7 +42,7 @@ func (d *HTTPHandlerDispatcher) dispatchLibraryActivate(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "library activate"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -159,7 +157,7 @@ func (d *HTTPHandlerDispatcher) postLibraryItem(
user *models.User,
workspace, collection, cmdKey, title, content, fieldsJSON string,
convention *models.ItemConventionMetadata,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
payload := map[string]any{
"title": title,
"fields": fieldsJSON,
+8 -10
View File
@@ -9,8 +9,6 @@ import (
"net/url"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
)
@@ -183,7 +181,7 @@ func (d *HTTPHandlerDispatcher) dispatchCreateItemLink(
input map[string]any,
user *models.User,
spec itemLinkSpec,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(spec.cmdKey, "workspace is required",
@@ -242,7 +240,7 @@ func (d *HTTPHandlerDispatcher) dispatchDeleteItemLink(
input map[string]any,
user *models.User,
spec itemLinkSpec,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(spec.cmdKey, "workspace is required",
@@ -332,7 +330,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemDeps(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item deps"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -383,7 +381,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemRelated(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item related"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -430,7 +428,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemImplementedBy(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item implemented-by"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -472,7 +470,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemImplementedBy(
// This matches packageHTTPResponse's pattern: that helper json-decodes
// the handler's response body into `any` for the structured channel,
// so synthesized responses use the same path here for shape parity.
func packageStructuredResponse(cmdKey string, payload any) (*mcp.CallToolResult, error) {
func packageStructuredResponse(cmdKey string, payload any) (*CallToolResult, error) {
body, err := json.Marshal(payload)
if err != nil {
return dispatcherErrorResult(cmdKey, "encode response", err), nil
@@ -483,9 +481,9 @@ func packageStructuredResponse(cmdKey string, payload any) (*mcp.CallToolResult,
// payload — but if it ever happens, fall back to the typed
// payload + raw body so the caller still gets something
// usable instead of an error.
return mcp.NewToolResultStructured(payload, string(body)), nil
return structuredResult(payload, string(body)), nil
}
return mcp.NewToolResultStructured(decoded, string(body)), nil
return structuredResult(decoded, string(body)), nil
}
// fetchItem retrieves a full models.Item via the
+11 -13
View File
@@ -10,8 +10,6 @@ import (
"sort"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
"github.com/PerpetualSoftware/pad/internal/server"
)
@@ -30,7 +28,7 @@ func (d *HTTPHandlerDispatcher) dispatchProjectReady(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "project ready"
dash, errRes := d.fetchDashboardJSON(ctx, input, user, cmdKey)
if errRes != nil {
@@ -58,7 +56,7 @@ func (d *HTTPHandlerDispatcher) dispatchProjectStale(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "project stale"
dash, errRes := d.fetchDashboardJSON(ctx, input, user, cmdKey)
if errRes != nil {
@@ -95,7 +93,7 @@ func (d *HTTPHandlerDispatcher) dispatchProjectNext(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "project next"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -124,7 +122,7 @@ func (d *HTTPHandlerDispatcher) dispatchProjectStandup(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "project standup"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -159,7 +157,7 @@ func (d *HTTPHandlerDispatcher) dispatchProjectChangelog(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "project changelog"
workspace, _ := input["workspace"].(string)
if workspace == "" {
@@ -186,7 +184,7 @@ func (d *HTTPHandlerDispatcher) fetchDashboardJSON(
input map[string]any,
user *models.User,
cmdKey string,
) (map[string]any, *mcp.CallToolResult) {
) (map[string]any, *CallToolResult) {
workspace, _ := input["workspace"].(string)
if workspace == "" {
return nil, validationFailedResult(cmdKey, "workspace is required",
@@ -292,7 +290,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemBulkUpdate(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item bulk-update"
workspace, _ := input["workspace"].(string)
@@ -470,7 +468,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemNote(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item note"
workspace, _ := input["workspace"].(string)
@@ -518,7 +516,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemDecide(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item decide"
workspace, _ := input["workspace"].(string)
@@ -595,7 +593,7 @@ func (d *HTTPHandlerDispatcher) dispatchLibraryList(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "library list"
typ, _ := input["type"].(string)
typ = strings.ToLower(strings.TrimSpace(typ))
@@ -675,7 +673,7 @@ func (d *HTTPHandlerDispatcher) fetchLibraryEndpoint(
ctx context.Context,
user *models.User,
cmdKey, path string,
) (any, *mcp.CallToolResult) {
) (any, *CallToolResult) {
req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
if err != nil {
return nil, dispatcherErrorResult(cmdKey, "build "+path, err)
+4 -6
View File
@@ -12,8 +12,6 @@ import (
"strings"
"time"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/cli"
"github.com/PerpetualSoftware/pad/internal/collections"
"github.com/PerpetualSoftware/pad/internal/models"
@@ -1405,7 +1403,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemList(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item list"
method, urlPath, _, err := mapItemList(input)
if err != nil {
@@ -1458,7 +1456,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemShow(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item show"
mapper := routeTable[cmdKey]
method, urlPath, body, err := mapper(input)
@@ -1522,7 +1520,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemHistory(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item history"
workspace, _ := input["workspace"].(string)
ref, _ := input["ref"].(string)
@@ -1763,7 +1761,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemCommentDelete(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
) (*CallToolResult, error) {
const cmdKey = "item comment-delete"
urlPath, err := itemCommentTarget(input)
if err != nil {
+15 -17
View File
@@ -7,8 +7,6 @@ import (
"net/http"
"regexp"
"strings"
"github.com/mark3labs/mcp-go/mcp"
)
// ─────────────────────────────────────────────────────────────────────
@@ -283,20 +281,20 @@ type WorkspaceHint struct {
// with IsError=true. Both the JSON envelope and a human-readable
// summary are returned: the envelope as structured content for clients
// that parse it (Claude Desktop, Cursor), the summary as text fallback.
func NewErrorResult(p ErrorPayload) *mcp.CallToolResult {
func NewErrorResult(p ErrorPayload) *CallToolResult {
envelope := ErrorEnvelope{Error: p}
body, err := json.Marshal(envelope)
if err != nil {
// Marshal of a struct with only string + bool + slice fields
// can't realistically fail; defensive fallback returns a plain
// errorf so the agent at least sees something.
return mcp.NewToolResultErrorf("%s: %s", p.Code, p.Message)
return errorResultf("%s: %s", p.Code, p.Message)
}
// NewToolResultStructured returns a result with content blocks
// PLUS structured content. The IsError flag has to be set after
// because the structured constructor doesn't accept it as a
// parameter — set it here so MCP clients see both.
res := mcp.NewToolResultStructured(envelope, string(body))
res := structuredResult(envelope, string(body))
res.IsError = true
return res
}
@@ -305,7 +303,7 @@ func NewErrorResult(p ErrorPayload) *mcp.CallToolResult {
// available_workspaces populated by the supplied lookup. Lookup is
// best-effort: failures (e.g. no auth) yield an envelope with empty
// AvailableWorkspaces rather than dropping the whole error.
func noWorkspaceResult(ctx context.Context, lookup WorkspaceLister) *mcp.CallToolResult {
func noWorkspaceResult(ctx context.Context, lookup WorkspaceLister) *CallToolResult {
hints := bestEffortWorkspaceHints(ctx, lookup)
return NewErrorResult(ErrorPayload{
Code: ErrNoWorkspace,
@@ -319,7 +317,7 @@ func noWorkspaceResult(ctx context.Context, lookup WorkspaceLister) *mcp.CallToo
// Same available_workspaces enrichment as no_workspace. Empty slug
// emits a generic message rather than a misleading `Workspace ""`
// — happens when the source error doesn't explicitly name the slug.
func unknownWorkspaceResult(ctx context.Context, slug string, lookup WorkspaceLister) *mcp.CallToolResult {
func unknownWorkspaceResult(ctx context.Context, slug string, lookup WorkspaceLister) *CallToolResult {
hints := bestEffortWorkspaceHints(ctx, lookup)
message := "Workspace not visible to this session."
if slug != "" {
@@ -375,7 +373,7 @@ func bestEffortWorkspaceHints(ctx context.Context, lookup WorkspaceLister) []Wor
// envelope unknownWorkspaceResult already built. Returns a zero
// envelope when the structured content is missing or malformed —
// callers fall back gracefully.
func envelopeFrom(res *mcp.CallToolResult) ErrorEnvelope {
func envelopeFrom(res *CallToolResult) ErrorEnvelope {
if res == nil {
return ErrorEnvelope{}
}
@@ -489,7 +487,7 @@ var allowedStructuredErrorCodes = map[string]struct{}{
// can't pre-empt it.
// - Marker must start the line after trimming whitespace; markers
// embedded mid-line (e.g. in a quoted log message) are ignored.
func extractStructuredCLIError(stderr string) *mcp.CallToolResult {
func extractStructuredCLIError(stderr string) *CallToolResult {
if !strings.Contains(stderr, structuredErrorMarker) {
return nil
}
@@ -540,7 +538,7 @@ func extractStructuredCLIError(stderr string) *mcp.CallToolResult {
// classifyExecError turns an exec.Cmd failure (err + stderr) into a
// structured envelope. lookup is optional — when supplied, no_workspace
// errors get available_workspaces enrichment.
func classifyExecError(ctx context.Context, cmdPath []string, runErr error, stderr string, lookup WorkspaceLister) *mcp.CallToolResult {
func classifyExecError(ctx context.Context, cmdPath []string, runErr error, stderr string, lookup WorkspaceLister) *CallToolResult {
// IDEA-1494 R2: the CLI emits a single `pad-error: {json}` line
// on stderr when surfacing the open-children rejection (see
// internal/cli/client.go::WriteOpenChildrenError). Detect it
@@ -725,7 +723,7 @@ func extractUnknownWorkspaceSlug(stderr string) string {
// BUG-987 bug 12: previously BuildCLIArgs failures came out of
// env.Dispatch as bare mcp.NewToolResultErrorf strings, breaking the
// structured-envelope invariant that every other error path follows.
func validationFailedFromBuildErr(cmdPath string, err error) *mcp.CallToolResult {
func validationFailedFromBuildErr(cmdPath string, err error) *CallToolResult {
msg := err.Error()
field := extractValidationField(msg)
payload := ErrorPayload{
@@ -884,7 +882,7 @@ const (
// classifyHTTPStatus is the legacy entry point preserved for callers
// that don't know their resource kind. New callers should use
// classifyHTTPStatusKind directly.
func classifyHTTPStatus(ctx context.Context, cmdKey string, status int, body []byte, lookup WorkspaceLister) *mcp.CallToolResult {
func classifyHTTPStatus(ctx context.Context, cmdKey string, status int, body []byte, lookup WorkspaceLister) *CallToolResult {
return classifyHTTPStatusKind(ctx, cmdKey, "", status, body, lookup, ResourceUnknown, "")
}
@@ -917,7 +915,7 @@ func classifyHTTPStatusKind(
lookup WorkspaceLister,
kind ResourceKind,
refOrSlug string,
) *mcp.CallToolResult {
) *CallToolResult {
bodyText := strings.TrimSpace(string(body))
bodyMessage := extractUpstreamMessage(bodyText)
if bodyText == "" {
@@ -1075,7 +1073,7 @@ func classify404(
lookup WorkspaceLister,
kind ResourceKind,
refOrSlug string,
) *mcp.CallToolResult {
) *CallToolResult {
// Workspace-shaped 404s: route through the existing
// unknown_workspace path so available_workspaces enrichment fires.
// Body-string sniffing is the legacy fallback for callers that
@@ -1496,7 +1494,7 @@ func tooLargeHintFor(route string) string {
// which tool was invoked; msg is the human-readable issue (e.g.
// "workspace is required", "ref is required"); fixHint is the
// recovery suggestion.
func validationFailedResult(cmdKey, msg, fixHint string) *mcp.CallToolResult {
func validationFailedResult(cmdKey, msg, fixHint string) *CallToolResult {
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: fmt.Sprintf("%s: %s", cmdKey, msg),
@@ -1514,7 +1512,7 @@ func validationFailedResult(cmdKey, msg, fixHint string) *mcp.CallToolResult {
// request", "encode body", "parse current item"); err is the
// underlying error — its message goes in the hint so debugging
// info isn't lost.
func dispatcherErrorResult(cmdKey, op string, err error) *mcp.CallToolResult {
func dispatcherErrorResult(cmdKey, op string, err error) *CallToolResult {
hint := fmt.Sprintf("Internal: %s — %s", op, err.Error())
return NewErrorResult(ErrorPayload{
Code: ErrServerError,
@@ -1538,7 +1536,7 @@ func upstreamHTTPErrorResult(
lookup WorkspaceLister,
kind ResourceKind,
refOrSlug string,
) *mcp.CallToolResult {
) *CallToolResult {
res := classifyHTTPStatusKind(ctx, cmdKey, route, status, body, lookup, kind, refOrSlug)
// Layer the per-call op verb into the message so a "prefetch"
// failure surfaces distinct from a top-level "execute" failure.
+36
View File
@@ -0,0 +1,36 @@
package mcp
import "github.com/mark3labs/mcp-go/mcp"
// This file is the SDK seam for tool results (TASK-2306). Every non-test
// dispatcher names the result type and builds results through what is
// declared here, so a future SDK swap (the modelcontextprotocol/go-sdk port)
// changes this file rather than every call site. The wire bytes are pinned
// by cmd/pad/mcp_wire_golden_test.go.
// CallToolResult is the result every tool handler and dispatcher returns.
// It is an alias, not a new type, so values move to and from mcp-go
// without conversion.
type CallToolResult = mcp.CallToolResult
// textResult is a successful result carrying plain text.
func textResult(text string) *CallToolResult {
return mcp.NewToolResultText(text)
}
// structuredResult is a successful result carrying structured content plus
// its text rendering, for clients that read only one of the two.
func structuredResult(structured any, text string) *CallToolResult {
return mcp.NewToolResultStructured(structured, text)
}
// errorResult is an error result carrying plain text. Prefer NewErrorResult
// (errors.go), which carries a structured envelope, wherever a code applies.
func errorResult(text string) *CallToolResult {
return mcp.NewToolResultError(text)
}
// errorResultf is errorResult with formatting.
func errorResultf(format string, a ...any) *CallToolResult {
return mcp.NewToolResultErrorf(format, a...)
}
+4 -4
View File
@@ -163,10 +163,10 @@ func SetWorkspaceTool(state *WorkspaceState, bootstrapFetcher BootstrapFetcher)
mcp.Required(),
),
)
handler := func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
handler := func(ctx context.Context, req mcp.CallToolRequest) (*CallToolResult, error) {
ws, err := req.RequireString("workspace")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
return errorResult(err.Error()), nil
}
// On a shared multi-user state (cloud /mcp), a persisted session
// default would bleed across users / concurrent sessions
@@ -191,7 +191,7 @@ func SetWorkspaceTool(state *WorkspaceState, bootstrapFetcher BootstrapFetcher)
}
}
b, _ := json.Marshal(out)
return mcp.NewToolResultText(string(b)), nil
return textResult(string(b)), nil
}
state.Set(ws)
out := map[string]any{"workspace": ws, "status": "ok"}
@@ -213,7 +213,7 @@ func SetWorkspaceTool(state *WorkspaceState, bootstrapFetcher BootstrapFetcher)
// don't fail the whole call on a bootstrap glitch.
}
b, _ := json.Marshal(out)
return mcp.NewToolResultText(string(b)), nil
return textResult(string(b)), nil
}
return tool, handler
}