feat(mcp): structured MCP error envelopes with closed taxonomy (TASK-973) (#357)

* feat(mcp): structured MCP error envelopes with closed taxonomy (TASK-973)

Replaces raw stderr / status-text passthrough with a closed-set
ErrorCode taxonomy + structured ErrorEnvelope. Agents can now branch
on `error.code` instead of parsing free-form text:

  {
    "error": {
      "code": "no_workspace",
      "message": "No workspace context. Pass `workspace` explicitly, ...",
      "hint": "Available workspaces: docapp, pad-web",
      "available_workspaces": [{"slug": "docapp", "default": true}, ...]
    }
  }

Taxonomy (8 codes):
- no_workspace, unknown_workspace — populate available_workspaces
- auth_required, permission_denied
- item_not_found, validation_failed, conflict
- server_error (catch-all)

Implementation:
- internal/mcp/errors.go (new): ErrorCode constants, ErrorEnvelope +
  ErrorPayload + WorkspaceHint types, NewErrorResult constructor,
  classifyExecError + classifyHTTPStatus dispatchers, regex pattern
  matchers for stderr classification, WorkspaceLister interface for
  hint enrichment.
- internal/mcp/dispatch.go: ExecDispatcher.Dispatch routes failures
  through classifyExecError (with itself as the WorkspaceLister).
  Adds ListWorkspaces method that shells out to `pad workspace list
  --format json`. Adds RootArgs field so the listing inherits root
  flags (--url etc.).
- internal/mcp/dispatch_http.go: packageHTTPResponse routes 4xx/5xx
  through classifyHTTPStatus. Lookup is intentionally nil here —
  TASK-977 (PLAN-943) owns the privacy-preserving available_workspaces
  filtering by OAuth allow-list.
- cmd/pad/mcp.go: pre-flatten rootFlags into RootArgs at
  dispatcher construction.

NewErrorResult emits BOTH structured content (for Claude Desktop,
Cursor) AND a JSON text body (for older clients). Both decode to the
same envelope so wire-level shape stays uniform.

Tests:
- TestNewErrorResult_Envelope: round-trip the envelope through
  structured + text surfaces.
- TestClassifyExecError: 11 cases covering every taxonomy code via
  stderr patterns.
- TestClassifyExecError_LookupFailureStillReturnsEnvelope: lookup
  failures degrade to empty available_workspaces, never drop the
  whole envelope.
- TestClassifyHTTPStatus: 10 cases covering each HTTP status mapping
  including the workspace-vs-item 404 fork.
- TestParseWorkspaceListJSON: happy path, empty / null / malformed,
  entry-without-slug skipping.
- TestExtractUnknownWorkspaceSlug: regex helper round-trip.

Out of scope (per task description):
- HTTPHandlerDispatcher available_workspaces filtering by OAuth
  allow-list → TASK-977 (PLAN-943).
- item_not_found "recent items" hint enrichment → also TASK-977.
- Per-code docs page on getpad.dev/mcp/local → TASK-976.

Parent: TASK-973 → PLAN-969.

* fix(cli): add JSON output to pad workspace list per Codex review (round 1)

Codex P2: classifyExecError's WorkspaceLister side channel calls
`pad workspace list --format json` to populate available_workspaces
in no_workspace / unknown_workspace error envelopes (TASK-973). The
CLI command silently ignored formatFlag and always printed the
human-readable shape, so parseWorkspaceListJSON would fail and the
hint was effectively never populated.

Add JSON branch to workspacesCmd that emits a {slug, name,
updated_at, default} array. The `default: true` flag marks the
CWD-linked workspace so agents can prefer it without a separate
DetectWorkspace call.

Manual verification:
  $ pad workspace list --format json | jq '.[0]'
  {
    "slug": "docapp",
    "name": "pad",
    "updated_at": "2026-04-14T13:24:51Z",
    "default": true
  }

Parent: TASK-973 → PLAN-969.

* fix(mcp): tighten unknown-workspace slug regex per Codex review (round 2)

Codex finding: extractUnknownWorkspaceSlug's bare-word regex captured
stop-words like "not" out of generic "Workspace not found" messages.
The server emits exactly this generic body in middleware_auth.go and
handlers_workspaces.go, so the resulting envelope would say
`Workspace "not" is not visible to this session.` — pushing agents
toward retrying with a bogus slug.

Tighten the regex to only match QUOTED slug forms ("workspace 'foo'"
or "workspace \"bar\""). Bare-word phrasings yield empty slug, and
unknownWorkspaceResult now emits a generic "Workspace not visible to
this session." instead of the misleading empty-string `Workspace ""`.

Test cases updated:
- "workspace 'foo' does not exist" → "foo" (still works)
- "workspace \"bar\" not found" → "bar" (still works)
- "unknown workspace baz" → "" (was "baz", now intentionally empty)
- "workspace docapp not visible" → "" (was "docapp", now empty)
- "Workspace not found" → "" (the actual server response)

The other taxonomy / hint behavior is unchanged: ErrUnknownWorkspace
still classifies correctly, available_workspaces still populates from
ListWorkspaces, and the body text still appears in Hint via the
classifyHTTPStatus 404 branch's body-append logic.

Parent: TASK-973 → PLAN-969.
This commit is contained in:
xarmian
2026-05-01 18:32:27 -04:00
committed by GitHub
parent 9068e3e7da
commit 1e94fcbd9d
8 changed files with 870 additions and 28 deletions
+28 -3
View File
@@ -2086,13 +2086,38 @@ func workspacesCmd() *cobra.Command {
if err != nil {
return err
}
current, _ := cli.DetectWorkspace(workspaceFlag)
// JSON output: machine-readable shape consumed by the MCP
// server's structured-error side channel (TASK-973's
// classifyExecError populates available_workspaces from
// this output). Each entry includes `default: true` for the
// CWD-linked workspace so agents can prefer it without a
// separate lookup.
if formatFlag == "json" {
type entry struct {
Slug string `json:"slug"`
Name string `json:"name"`
UpdatedAt string `json:"updated_at,omitempty"`
Default bool `json:"default,omitempty"`
}
out := make([]entry, 0, len(workspaces))
for _, ws := range workspaces {
out = append(out, entry{
Slug: ws.Slug,
Name: ws.Name,
UpdatedAt: ws.UpdatedAt.Format(time.RFC3339),
Default: ws.Slug == current,
})
}
return cli.PrintJSON(out)
}
if len(workspaces) == 0 {
fmt.Println("No workspaces. Run 'pad workspace init' to create one.")
return nil
}
current, _ := cli.DetectWorkspace(workspaceFlag)
for _, ws := range workspaces {
marker := " "
if ws.Slug == current {
+13 -1
View File
@@ -279,7 +279,19 @@ Shuts down cleanly on EOF, SIGINT, or SIGTERM.`,
// still consumed at dispatch time (BuildCLIArgs reads
// individual command schemas) but no longer drives the tool
// surface shape.
dispatcher := &mcpserver.ExecDispatcher{Binary: bin}
// Pre-flatten rootFlags into the token list ExecDispatcher
// reuses for its WorkspaceLister side channel — when
// classifyExecError needs to populate available_workspaces,
// it spawns `pad workspace list` and must hit the same
// server endpoint (e.g. --url for non-default servers).
rootArgs := []string{}
for k, v := range rootFlags {
if v == "" {
continue
}
rootArgs = append(rootArgs, "--"+k, v)
}
dispatcher := &mcpserver.ExecDispatcher{Binary: bin, RootArgs: rootArgs}
if _, err := mcpserver.Register(srv.MCP(), mcpserver.RegistryOptions{
Doc: doc,
Workspace: state,
+72 -9
View File
@@ -3,6 +3,7 @@ package mcp
import (
"context"
"encoding/json"
"errors"
"fmt"
"os/exec"
"sort"
@@ -102,11 +103,23 @@ func mergeDispatchInput(input map[string]any, sessionWorkspace string, rootFlags
// ExecDispatcher shells out to the pad binary at Binary. stdout is
// returned as Text content; if it parses as JSON the result is also
// surfaced via StructuredContent so MCP clients can consume it
// natively. Non-zero exit returns an IsError-flagged result with
// stderr as the message.
// natively. Non-zero exit returns an IsError-flagged result with a
// structured ErrorEnvelope (TASK-973) — stderr is classified into a
// closed-set ErrorCode so agents can branch on the code rather than
// parsing free-form text.
type ExecDispatcher struct {
// Binary is the path to the pad executable. Required.
Binary string
// RootArgs are pre-formatted root-flag tokens (e.g. ["--url", X])
// to forward to every spawned subprocess. Same data as the
// rootFlags map carried by RegistryOptions but pre-flattened so
// the dispatcher doesn't re-iterate it on every Dispatch call.
// Used by the WorkspaceLister side-channel — when classifyExecError
// needs to populate available_workspaces, it spawns
// `pad workspace list` with these flags so it hits the same
// endpoint as the original failed call.
RootArgs []string
}
// Dispatch runs `<Binary> <cmdPath...> <cliArgs...>` and packages the
@@ -121,13 +134,11 @@ func (d *ExecDispatcher) Dispatch(ctx context.Context, cmdPath []string, cliArgs
cmd.Stdout = &stdout
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
msg := strings.TrimSpace(stderr.String())
if msg == "" {
msg = err.Error()
}
return mcp.NewToolResultErrorf(
"pad %s failed: %s", strings.Join(cmdPath, " "), msg,
), nil
// Classify into the structured envelope. WorkspaceLister
// uses the same dispatcher (recursive in spirit but a fresh
// subprocess) to enrich no_workspace / unknown_workspace
// errors with available_workspaces.
return classifyExecError(ctx, cmdPath, err, stderr.String(), d), nil
}
out := stdout.String()
// If stdout is JSON, surface it as structured content alongside
@@ -143,6 +154,58 @@ func (d *ExecDispatcher) Dispatch(ctx context.Context, cmdPath []string, cliArgs
return mcp.NewToolResultText(out), nil
}
// ListWorkspaces satisfies WorkspaceLister so error helpers can
// populate available_workspaces hints. Best-effort: if listing fails
// (no auth, network down, etc.) the caller treats an error as "no
// listing available" and surfaces the bare error envelope.
//
// Implementation note: spawns a fresh `pad workspace list --format
// json` subprocess. Adds one extra exec per error path that needs the
// hint — acceptable cost given how rare these errors are. Honors
// RootArgs so the listing hits the same server endpoint as the
// originally-failed call (e.g. --url for non-default servers).
func (d *ExecDispatcher) ListWorkspaces(ctx context.Context) ([]WorkspaceHint, error) {
if d.Binary == "" {
return nil, errors.New("dispatcher: binary path not configured")
}
args := []string{"workspace", "list", "--format", "json"}
args = append(args, d.RootArgs...)
cmd := exec.CommandContext(ctx, d.Binary, args...)
var stdout, stderr strings.Builder
cmd.Stdout = &stdout
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
return nil, fmt.Errorf("workspace list: %s", strings.TrimSpace(stderr.String()))
}
return parseWorkspaceListJSON(stdout.String())
}
// parseWorkspaceListJSON decodes the JSON shape `pad workspace list
// --format json` emits into the WorkspaceHint slice. Lenient about
// optional fields — only `slug` is required; everything else gets
// zero values when absent.
func parseWorkspaceListJSON(body string) ([]WorkspaceHint, error) {
body = strings.TrimSpace(body)
if body == "" || body == "null" {
return nil, nil
}
var raw []map[string]any
if err := json.Unmarshal([]byte(body), &raw); err != nil {
return nil, fmt.Errorf("decode workspace list: %w", err)
}
out := make([]WorkspaceHint, 0, len(raw))
for _, w := range raw {
slug, _ := w["slug"].(string)
if slug == "" {
continue
}
name, _ := w["name"].(string)
isDefault, _ := w["default"].(bool)
out = append(out, WorkspaceHint{Slug: slug, Name: name, Default: isDefault})
}
return out, nil
}
// BuildCLIArgs translates an MCP tool call's JSON arguments into the
// CLI argument list that should be appended after the command path.
// Pure function — no side effects, no subprocess — so it tests cleanly.
+15 -12
View File
@@ -353,7 +353,7 @@ func (d *HTTPHandlerDispatcher) executeRequest(
rec := httptest.NewRecorder()
d.Handler.ServeHTTP(rec, req)
return packageHTTPResponse(cmdKey, rec.Result())
return packageHTTPResponse(ctx, cmdKey, rec.Result())
}
// buildAuthedRequest constructs an in-process HTTP request against
@@ -415,9 +415,12 @@ func buildHTTPRequest(ctx context.Context, method, urlPath string, body []byte,
// packageHTTPResponse turns the recorded handler response into an MCP
// CallToolResult, mirroring ExecDispatcher's "JSON → structured + text
// fallback" behaviour. 4xx/5xx surface as IsError-flagged results so
// MCP clients distinguish protocol vs. tool failures.
func packageHTTPResponse(cmdKey string, resp *http.Response) (*mcp.CallToolResult, error) {
// fallback" behaviour. 4xx/5xx surface as structured ErrorEnvelopes
// (TASK-973) so MCP clients see the same closed-set error codes
// regardless of transport. TASK-977 (PLAN-943) extends this with
// privacy-preserving available_workspaces filtering once the OAuth
// allow-list is in place.
func packageHTTPResponse(ctx context.Context, cmdKey string, resp *http.Response) (*mcp.CallToolResult, error) {
defer resp.Body.Close()
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
@@ -426,14 +429,14 @@ func packageHTTPResponse(cmdKey string, resp *http.Response) (*mcp.CallToolResul
body := string(bodyBytes)
if resp.StatusCode >= 400 {
// Match the CLI's `pad <cmd>` error format from ExecDispatcher
// so MCP clients see a consistent shape regardless of
// transport.
msg := strings.TrimSpace(body)
if msg == "" {
msg = http.StatusText(resp.StatusCode)
}
return mcp.NewToolResultErrorf("pad %s failed: %s", cmdKey, msg), nil
// classifyHTTPStatus does the status → ErrorCode mapping. The
// `lookup` parameter is nil here because TASK-977 owns the
// remote-side workspace listing (it must filter by OAuth
// allow-list to avoid leaking workspaces the agent didn't
// consent to). For now, available_workspaces stays empty on
// the HTTP transport; the rest of the envelope still gives
// the agent enough to branch on `code`.
return classifyHTTPStatus(ctx, cmdKey, resp.StatusCode, bodyBytes, nil), nil
}
if trimmed := strings.TrimSpace(body); strings.HasPrefix(trimmed, "{") || strings.HasPrefix(trimmed, "[") {
+1 -1
View File
@@ -297,7 +297,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemUpdate(
// Mirror the CLI's "not found" UX — the handler's 404 body
// already contains a clear message; package it the same way
// any other tool error would be packaged.
return packageHTTPResponse(cmdKey, prefetchRec.Result())
return packageHTTPResponse(ctx, cmdKey, prefetchRec.Result())
}
var existing struct {
Fields string `json:"fields"`
+2 -2
View File
@@ -499,7 +499,7 @@ func TestPackageHTTPResponse_StructuredJSONOnSuccess(t *testing.T) {
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(`{"ref":"TASK-1"}`)),
}
res, err := packageHTTPResponse("item create", resp)
res, err := packageHTTPResponse(context.Background(), "item create", resp)
if err != nil {
t.Fatalf("packageHTTPResponse: %v", err)
}
@@ -516,7 +516,7 @@ func TestPackageHTTPResponse_TextFallbackOnNonJSON(t *testing.T) {
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader("hello world")),
}
res, err := packageHTTPResponse("item create", resp)
res, err := packageHTTPResponse(context.Background(), "item create", resp)
if err != nil {
t.Fatalf("packageHTTPResponse: %v", err)
}
+441
View File
@@ -0,0 +1,441 @@
package mcp
import (
"context"
"encoding/json"
"fmt"
"net/http"
"regexp"
"strings"
"github.com/mark3labs/mcp-go/mcp"
)
// ─────────────────────────────────────────────────────────────────────
// Structured MCP error envelopes (TASK-973)
//
// Replaces raw CLI stderr passthrough with a closed taxonomy of error
// codes the model can branch on. The agent receives a JSON envelope
// like:
//
// {
// "error": {
// "code": "no_workspace",
// "message": "No workspace context — pass workspace=<slug> or call pad_set_workspace first.",
// "hint": "Available workspaces: docapp, pad-web. Or run pad workspace init.",
// "available_workspaces": [{"slug": "docapp", "default": true}, ...]
// }
// }
//
// instead of an opaque string like "no workspace linked. Run 'pad
// workspace init'". Closed code set means the model can implement
// recovery logic per-code rather than parsing free-form text.
//
// Both ExecDispatcher (stderr classification) and HTTPHandlerDispatcher
// (HTTP status mapping) feed into the same taxonomy. Mirror impl on
// the remote side is TASK-977 — it inherits the type definitions from
// here and adds a privacy-preserving available_workspaces filter.
// ─────────────────────────────────────────────────────────────────────
// ErrorCode is one of the closed-set MCP error codes. Enumerated
// constants below; do not introduce a new code without updating the
// docs (server instructions block + getpad.dev/mcp/local).
type ErrorCode string
const (
// ErrNoWorkspace fires when no workspace context resolves
// (no explicit param, no session default, no CWD .pad.toml).
// Populates available_workspaces from `pad workspace list`.
ErrNoWorkspace ErrorCode = "no_workspace"
// ErrUnknownWorkspace fires when a slug is supplied but doesn't
// match any workspace the user can read. Same available_workspaces
// hint as ErrNoWorkspace.
ErrUnknownWorkspace ErrorCode = "unknown_workspace"
// ErrAuthRequired fires when no valid credentials are present —
// CLI: ~/.pad/credentials.json missing or expired; HTTP: 401.
ErrAuthRequired ErrorCode = "auth_required"
// ErrPermissionDenied fires when authentication succeeds but role
// is insufficient for the operation. HTTP: 403.
ErrPermissionDenied ErrorCode = "permission_denied"
// ErrItemNotFound fires when an item ref / slug doesn't resolve.
// Future enhancement: populate `available_collections` or recent
// items as a hint (deferred to TASK-977).
ErrItemNotFound ErrorCode = "item_not_found"
// ErrValidationFailed fires on bad input — required field missing,
// enum value out of range, malformed JSON. HTTP: 422.
ErrValidationFailed ErrorCode = "validation_failed"
// ErrConflict fires when an operation collides with concurrent
// state (e.g. version mismatch on update). HTTP: 409.
ErrConflict ErrorCode = "conflict"
// ErrServerError is the catch-all for unexpected failures —
// 5xx from HTTP, unknown stderr patterns from exec. The wrapped
// message preserves the underlying detail for debugging without
// promising any structured shape.
ErrServerError ErrorCode = "server_error"
)
// ErrorEnvelope is the wire shape returned to MCP clients on tool
// failures. The outer key is `error` so the JSON unambiguously signals
// "this is the error path"; clients that want to switch on code can do
// so without inspecting IsError separately.
type ErrorEnvelope struct {
Error ErrorPayload `json:"error"`
}
// ErrorPayload is the structured error body. Optional fields use
// pointers / `omitempty` so they only appear when populated, keeping
// success-case-shaped clients happy. Per-code fields documented inline.
type ErrorPayload struct {
// Code is one of the ErrorCode constants. Stable across versions.
Code ErrorCode `json:"code"`
// Message is a short, human-readable summary suitable for direct
// display. Avoid PII / token values here — the message may end up
// in logs.
Message string `json:"message"`
// Hint is a longer suggestion for self-recovery. May reference
// commands, alternate values, or follow-up reads. Optional.
Hint string `json:"hint,omitempty"`
// AvailableWorkspaces is populated for ErrNoWorkspace /
// ErrUnknownWorkspace so the agent can pick a valid slug without
// a human round-trip. Empty array means lookup failed (e.g. no
// auth) — agents should treat this as "no workspace listing
// available" rather than "no workspaces exist."
AvailableWorkspaces []WorkspaceHint `json:"available_workspaces,omitempty"`
// Field / Expected / Got populate ErrValidationFailed when the
// underlying error pinpoints a specific input.
Field string `json:"field,omitempty"`
Expected string `json:"expected,omitempty"`
Got string `json:"got,omitempty"`
// RequiredRole / CurrentRole populate ErrPermissionDenied so
// agents see why the call was rejected.
RequiredRole string `json:"required_role,omitempty"`
CurrentRole string `json:"current_role,omitempty"`
}
// WorkspaceHint is a minimal workspace summary surfaced in the
// no_workspace / unknown_workspace envelopes.
type WorkspaceHint struct {
Slug string `json:"slug"`
Name string `json:"name,omitempty"`
Default bool `json:"default,omitempty"`
}
// NewErrorResult packages an ErrorPayload as an MCP CallToolResult
// 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 {
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)
}
// 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.IsError = true
return res
}
// noWorkspaceResult builds the standard ErrNoWorkspace envelope with
// 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 {
hints := bestEffortWorkspaceHints(ctx, lookup)
return NewErrorResult(ErrorPayload{
Code: ErrNoWorkspace,
Message: "No workspace context. Pass `workspace` explicitly, call pad_set_workspace first, or run from a directory with .pad.toml.",
Hint: workspaceHintLine(hints),
AvailableWorkspaces: hints,
})
}
// unknownWorkspaceResult wraps a "workspace X doesn't exist" failure.
// 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 {
hints := bestEffortWorkspaceHints(ctx, lookup)
message := "Workspace not visible to this session."
if slug != "" {
message = fmt.Sprintf("Workspace %q is not visible to this session.", slug)
}
return NewErrorResult(ErrorPayload{
Code: ErrUnknownWorkspace,
Message: message,
Hint: workspaceHintLine(hints),
AvailableWorkspaces: hints,
})
}
// workspaceHintLine returns a concise comma-joined slug list, or an
// empty string when no hints were resolved (avoids "Available
// workspaces: " trailing nothing).
func workspaceHintLine(hints []WorkspaceHint) string {
if len(hints) == 0 {
return ""
}
slugs := make([]string, 0, len(hints))
for _, h := range hints {
slugs = append(slugs, h.Slug)
}
return "Available workspaces: " + strings.Join(slugs, ", ")
}
// WorkspaceLister is the side-channel a dispatcher exposes so error
// helpers can populate available_workspaces hints. Returning an empty
// slice (rather than an error) when lookup fails is fine — callers
// already treat empty as "no listing available."
type WorkspaceLister interface {
ListWorkspaces(ctx context.Context) ([]WorkspaceHint, error)
}
// bestEffortWorkspaceHints calls lookup.ListWorkspaces and swallows
// errors. The error envelope is more valuable than nothing even when
// the listing failed.
func bestEffortWorkspaceHints(ctx context.Context, lookup WorkspaceLister) []WorkspaceHint {
if lookup == nil {
return nil
}
hints, err := lookup.ListWorkspaces(ctx)
if err != nil {
return nil
}
return hints
}
// envelopeFrom reads the ErrorEnvelope back out of an MCP error
// result's structured content. Used by classifyHTTPStatus's 404
// workspace path so we can layer additional Hint detail on top of the
// 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 {
if res == nil {
return ErrorEnvelope{}
}
if env, ok := res.StructuredContent.(ErrorEnvelope); ok {
return env
}
return ErrorEnvelope{}
}
// ─────────────────────────────────────────────────────────────────────
// ExecDispatcher classification
//
// The local subprocess emits stderr strings that we pattern-match
// against known cases. Unmatched output falls through to ErrServerError
// with the raw stderr preserved in Message.
// ─────────────────────────────────────────────────────────────────────
// 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 {
stderr = strings.TrimSpace(stderr)
lower := strings.ToLower(stderr)
switch {
case execStderrMatchesNoWorkspace(lower):
return noWorkspaceResult(ctx, lookup)
case execStderrMatchesUnknownWorkspace(lower):
// Stderr typically embeds the slug — try to extract it. If
// extraction fails, the envelope still carries the message;
// the slug just won't appear in the hint line.
slug := extractUnknownWorkspaceSlug(stderr)
return unknownWorkspaceResult(ctx, slug, lookup)
case execStderrMatchesAuthRequired(lower):
return NewErrorResult(ErrorPayload{
Code: ErrAuthRequired,
Message: "Authentication required. Run `pad auth login` to sign in.",
Hint: stderr,
})
case execStderrMatchesPermissionDenied(lower):
return NewErrorResult(ErrorPayload{
Code: ErrPermissionDenied,
Message: "Permission denied for this operation.",
Hint: stderr,
})
case execStderrMatchesItemNotFound(lower):
return NewErrorResult(ErrorPayload{
Code: ErrItemNotFound,
Message: "Item not found.",
Hint: stderr,
})
case execStderrMatchesValidation(lower):
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: "Validation failed.",
Hint: stderr,
})
}
// Fallback: unstructured server error. Preserve the original
// "pad <cmd> failed: <stderr>" shape so any agent that special-
// cased the old text still has something to read.
msg := stderr
if msg == "" && runErr != nil {
msg = runErr.Error()
}
if msg == "" {
msg = "unknown error"
}
cmd := strings.Join(cmdPath, " ")
return NewErrorResult(ErrorPayload{
Code: ErrServerError,
Message: fmt.Sprintf("pad %s failed: %s", cmd, msg),
})
}
// Stderr-pattern matchers. Compiled at init for cost-free
// classification. Patterns are case-insensitive against `lower`
// (the caller pre-lowercases for performance).
var (
reNoWorkspace = regexp.MustCompile(`no workspace.*(linked|configured)`)
reUnknownWorkspaceA = regexp.MustCompile(`workspace .* (does not exist|not found)`)
reUnknownWorkspaceB = regexp.MustCompile(`unknown workspace`)
reAuthRequired = regexp.MustCompile(`(not authenticated|authentication required|please log in|run pad auth login|invalid token|expired token)`)
rePermissionDenied = regexp.MustCompile(`(permission denied|forbidden|insufficient (permissions|role))`)
reItemNotFound = regexp.MustCompile(`(item.*not found|no such item|unknown ref)`)
reValidationFailed = regexp.MustCompile(`(invalid|missing required|must be one of|validation)`)
// Only match QUOTED slugs to avoid capturing stop-words like "not"
// in generic "Workspace not found" / "workspace not visible"
// messages. Quoted forms come from CLI stderr ("workspace 'foo'
// does not exist") and from JSON error bodies that explicitly name
// the slug. Unquoted phrasings yield an empty slug — the
// unknownWorkspaceResult message handles that gracefully without
// emitting a misleading `Workspace "not"` line.
reUnknownWorkspaceID = regexp.MustCompile(`workspace ['"]([a-z0-9][a-z0-9-]*)['"]`)
)
func execStderrMatchesNoWorkspace(lower string) bool { return reNoWorkspace.MatchString(lower) }
func execStderrMatchesUnknownWorkspace(lower string) bool {
return reUnknownWorkspaceA.MatchString(lower) || reUnknownWorkspaceB.MatchString(lower)
}
func execStderrMatchesAuthRequired(lower string) bool { return reAuthRequired.MatchString(lower) }
func execStderrMatchesPermissionDenied(lower string) bool {
return rePermissionDenied.MatchString(lower)
}
func execStderrMatchesItemNotFound(lower string) bool { return reItemNotFound.MatchString(lower) }
func execStderrMatchesValidation(lower string) bool { return reValidationFailed.MatchString(lower) }
// extractUnknownWorkspaceSlug pulls the slug from a CLI stderr like
// "workspace 'foo' does not exist" so the envelope can name it.
// Returns empty string if nothing matches; callers fall back to a
// generic "this slug" phrasing.
func extractUnknownWorkspaceSlug(stderr string) string {
m := reUnknownWorkspaceID.FindStringSubmatch(strings.ToLower(stderr))
if len(m) < 2 {
return ""
}
return m[1]
}
// ─────────────────────────────────────────────────────────────────────
// HTTPHandlerDispatcher classification
//
// HTTP status codes map cleanly to the taxonomy. Body parsing is
// best-effort: when the handler returns a structured payload we
// surface its message; otherwise the status text serves as the
// envelope's Message.
// ─────────────────────────────────────────────────────────────────────
// classifyHTTPStatus turns an HTTP error response into a structured
// envelope. body is the raw response body (may be empty); cmdKey is
// the dotted command path for debugging context. lookup is optional
// — supply the dispatcher's WorkspaceLister so 404 (workspace) /
// 403 / etc. can populate available_workspaces when relevant.
func classifyHTTPStatus(ctx context.Context, cmdKey string, status int, body []byte, lookup WorkspaceLister) *mcp.CallToolResult {
bodyText := strings.TrimSpace(string(body))
if bodyText == "" {
bodyText = http.StatusText(status)
}
switch status {
case http.StatusUnauthorized:
return NewErrorResult(ErrorPayload{
Code: ErrAuthRequired,
Message: "Authentication required.",
Hint: bodyText,
})
case http.StatusForbidden:
return NewErrorResult(ErrorPayload{
Code: ErrPermissionDenied,
Message: "Permission denied for this operation.",
Hint: bodyText,
})
case http.StatusNotFound:
// Without inspecting the URL we can't tell workspace-404 vs
// item-404 with certainty; the body usually says. Default to
// item_not_found and let TASK-977 refine when the remote
// transport can provide URL-aware classification.
if strings.Contains(strings.ToLower(bodyText), "workspace") {
// Try to pull the slug out of the body so the message
// names it (e.g. body="workspace 'foo' not visible" →
// "Workspace \"foo\" is not visible..."). Falls back to
// the body in Hint when no slug parses.
slug := extractUnknownWorkspaceSlug(bodyText)
res := unknownWorkspaceResult(ctx, slug, lookup)
// unknownWorkspaceResult uses the available_workspaces
// hint line; preserve the original handler body in Hint
// so debug detail isn't lost. Concatenate when both
// exist so neither hides the other.
env := envelopeFrom(res)
if env.Error.Hint == "" {
env.Error.Hint = bodyText
} else {
env.Error.Hint = bodyText + " — " + env.Error.Hint
}
return NewErrorResult(env.Error)
}
return NewErrorResult(ErrorPayload{
Code: ErrItemNotFound,
Message: "Item not found.",
Hint: bodyText,
})
case http.StatusConflict:
return NewErrorResult(ErrorPayload{
Code: ErrConflict,
Message: "Conflict — current state changed beneath this update.",
Hint: bodyText,
})
case http.StatusUnprocessableEntity, http.StatusBadRequest:
return NewErrorResult(ErrorPayload{
Code: ErrValidationFailed,
Message: "Validation failed.",
Hint: bodyText,
})
}
if status >= 500 {
return NewErrorResult(ErrorPayload{
Code: ErrServerError,
Message: fmt.Sprintf("pad %s failed: %s", cmdKey, bodyText),
})
}
// Other 4xx without a specific mapping — surface as server_error
// with the raw body so debugging is still possible. Avoid silently
// promoting them to validation_failed; that would mislead callers.
return NewErrorResult(ErrorPayload{
Code: ErrServerError,
Message: fmt.Sprintf("pad %s failed (HTTP %d): %s", cmdKey, status, bodyText),
})
}
+298
View File
@@ -0,0 +1,298 @@
package mcp
import (
"context"
"encoding/json"
"errors"
"net/http"
"strings"
"testing"
"github.com/mark3labs/mcp-go/mcp"
)
// TestNewErrorResult_Envelope confirms the wire shape every error
// path produces. The test inspects:
//
// - IsError flag set on the result.
// - StructuredContent contains the envelope as a Go value.
// - Text fallback parses back to the same envelope.
//
// MCP clients pick whichever surface they understand (Claude Desktop
// uses StructuredContent; older clients fall back to text). The
// invariant: both must agree.
func TestNewErrorResult_Envelope(t *testing.T) {
payload := ErrorPayload{
Code: ErrNoWorkspace,
Message: "No workspace context.",
Hint: "Available workspaces: docapp",
AvailableWorkspaces: []WorkspaceHint{
{Slug: "docapp", Name: "Pad", Default: true},
},
}
res := NewErrorResult(payload)
if !res.IsError {
t.Errorf("IsError = false, want true")
}
if res.StructuredContent == nil {
t.Errorf("StructuredContent missing — clients won't see the typed envelope")
}
// Round-trip via the text fallback.
body := textOf(res)
if body == "" {
t.Fatalf("text fallback missing")
}
var got ErrorEnvelope
if err := json.Unmarshal([]byte(body), &got); err != nil {
t.Fatalf("unmarshal envelope: %v\nbody=%s", err, body)
}
if got.Error.Code != ErrNoWorkspace {
t.Errorf("Code = %q, want %q", got.Error.Code, ErrNoWorkspace)
}
if got.Error.Message != payload.Message {
t.Errorf("Message mismatch: got %q, want %q", got.Error.Message, payload.Message)
}
if len(got.Error.AvailableWorkspaces) != 1 {
t.Fatalf("AvailableWorkspaces length = %d, want 1", len(got.Error.AvailableWorkspaces))
}
if got.Error.AvailableWorkspaces[0].Slug != "docapp" {
t.Errorf("workspace slug = %q, want docapp", got.Error.AvailableWorkspaces[0].Slug)
}
}
// fakeWorkspaceLister is a controllable WorkspaceLister for testing.
// Returns the configured slice unless ErrOut is set, in which case it
// errors (best-effort path test).
type fakeWorkspaceLister struct {
hints []WorkspaceHint
errOut error
}
func (f *fakeWorkspaceLister) ListWorkspaces(_ context.Context) ([]WorkspaceHint, error) {
if f.errOut != nil {
return nil, f.errOut
}
return f.hints, nil
}
// TestClassifyExecError covers the full taxonomy via the ExecDispatcher
// classifier. Each subtest pairs a stderr pattern with the expected
// ErrorCode. Hits all 6 patterns the classifier recognizes plus the
// server_error fallback for unmatched output.
func TestClassifyExecError(t *testing.T) {
lookup := &fakeWorkspaceLister{hints: []WorkspaceHint{
{Slug: "docapp"}, {Slug: "pad-web"},
}}
cases := []struct {
name string
stderr string
wantCode ErrorCode
wantHints bool // true if this code populates available_workspaces
}{
{"no_workspace", "Error: no workspace linked. Run 'pad workspace init'.", ErrNoWorkspace, true},
{"unknown_workspace_does_not_exist", "Error: workspace 'foo' does not exist", ErrUnknownWorkspace, true},
{"unknown_workspace_explicit", "Error: unknown workspace bar", ErrUnknownWorkspace, true},
{"auth_required_login", "Error: not authenticated. please run pad auth login.", ErrAuthRequired, false},
{"auth_required_expired", "Error: expired token", ErrAuthRequired, false},
{"permission_denied", "Error: permission denied for this resource", ErrPermissionDenied, false},
{"item_not_found", "Error: item TASK-99 not found", ErrItemNotFound, false},
{"validation_failed_required", "Error: missing required field 'title'", ErrValidationFailed, false},
{"validation_failed_enum", "Error: status must be one of: open, in-progress, done", ErrValidationFailed, false},
{"server_error_unknown", "Error: something went wrong in unexpected ways", ErrServerError, false},
{"server_error_empty", "", ErrServerError, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
runErr := errors.New("exit status 1")
res := classifyExecError(context.Background(),
[]string{"item", "list"}, runErr, tc.stderr, lookup)
if !res.IsError {
t.Errorf("expected IsError, got success")
}
env := decodeEnvelope(t, res)
if env.Error.Code != tc.wantCode {
t.Errorf("Code = %q, want %q", env.Error.Code, tc.wantCode)
}
if tc.wantHints {
if len(env.Error.AvailableWorkspaces) == 0 {
t.Errorf("expected available_workspaces populated for %s", tc.wantCode)
}
} else {
if len(env.Error.AvailableWorkspaces) > 0 {
t.Errorf("did not expect available_workspaces for %s; got %v",
tc.wantCode, env.Error.AvailableWorkspaces)
}
}
})
}
}
// TestClassifyExecError_LookupFailureStillReturnsEnvelope ensures
// best-effort listing — when ListWorkspaces fails, we still surface
// the no_workspace error (just with empty available_workspaces).
// Without this guarantee, a transient lookup failure would degrade
// every workspace-context error into a confusing "couldn't even
// produce an error envelope" path.
func TestClassifyExecError_LookupFailureStillReturnsEnvelope(t *testing.T) {
lookup := &fakeWorkspaceLister{errOut: errors.New("listing exploded")}
res := classifyExecError(context.Background(),
[]string{"item", "list"},
errors.New("exit"),
"Error: no workspace linked",
lookup,
)
env := decodeEnvelope(t, res)
if env.Error.Code != ErrNoWorkspace {
t.Errorf("Code = %q, want no_workspace", env.Error.Code)
}
if len(env.Error.AvailableWorkspaces) != 0 {
t.Errorf("expected empty AvailableWorkspaces on lookup failure; got %v",
env.Error.AvailableWorkspaces)
}
}
// TestClassifyHTTPStatus covers the HTTP status → ErrorCode mapping.
// Each documented status maps to the expected code; unmapped statuses
// fall through to server_error.
func TestClassifyHTTPStatus(t *testing.T) {
cases := []struct {
name string
status int
body string
wantCode ErrorCode
}{
{"401_unauthorized", http.StatusUnauthorized, "token invalid", ErrAuthRequired},
{"403_forbidden", http.StatusForbidden, "role insufficient", ErrPermissionDenied},
{"404_item", http.StatusNotFound, "item TASK-99 not found", ErrItemNotFound},
{"404_workspace", http.StatusNotFound, "workspace foo not visible", ErrUnknownWorkspace},
{"409_conflict", http.StatusConflict, "version mismatch", ErrConflict},
{"422_validation", http.StatusUnprocessableEntity, "title required", ErrValidationFailed},
{"400_validation", http.StatusBadRequest, "bad input", ErrValidationFailed},
{"500_server", http.StatusInternalServerError, "boom", ErrServerError},
{"503_server", http.StatusServiceUnavailable, "down", ErrServerError},
{"418_other", http.StatusTeapot, "weird", ErrServerError},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
res := classifyHTTPStatus(context.Background(),
"pad item list", tc.status, []byte(tc.body), nil)
if !res.IsError {
t.Errorf("expected IsError")
}
env := decodeEnvelope(t, res)
if env.Error.Code != tc.wantCode {
t.Errorf("Code = %q, want %q", env.Error.Code, tc.wantCode)
}
// Body fragment should appear somewhere in the envelope —
// either in Hint (typical) or Message (server_error path).
combined := env.Error.Hint + " " + env.Error.Message
if tc.body != "" && !strings.Contains(combined, tc.body) {
t.Errorf("envelope should preserve body %q somewhere; got message=%q hint=%q",
tc.body, env.Error.Message, env.Error.Hint)
}
})
}
}
// TestParseWorkspaceListJSON locks the JSON shape ExecDispatcher's
// ListWorkspaces consumes. Emits the same fields `pad workspace list
// --format json` would so the parser stays aligned with the CLI as
// the latter evolves.
func TestParseWorkspaceListJSON(t *testing.T) {
t.Run("happy path", func(t *testing.T) {
body := `[{"slug":"docapp","name":"Pad","default":true},{"slug":"pad-web"}]`
got, err := parseWorkspaceListJSON(body)
if err != nil {
t.Fatalf("parse: %v", err)
}
if len(got) != 2 {
t.Fatalf("len = %d, want 2", len(got))
}
if got[0].Slug != "docapp" || !got[0].Default || got[0].Name != "Pad" {
t.Errorf("first hint mismatch: %+v", got[0])
}
if got[1].Slug != "pad-web" {
t.Errorf("second hint slug = %q, want pad-web", got[1].Slug)
}
})
t.Run("empty body", func(t *testing.T) {
got, err := parseWorkspaceListJSON("")
if err != nil || got != nil {
t.Errorf("empty body should return (nil, nil); got (%v, %v)", got, err)
}
})
t.Run("null body", func(t *testing.T) {
got, err := parseWorkspaceListJSON("null")
if err != nil || got != nil {
t.Errorf("null body should return (nil, nil); got (%v, %v)", got, err)
}
})
t.Run("entry without slug skipped", func(t *testing.T) {
body := `[{"name":"orphan"},{"slug":"valid"}]`
got, err := parseWorkspaceListJSON(body)
if err != nil {
t.Fatalf("parse: %v", err)
}
if len(got) != 1 || got[0].Slug != "valid" {
t.Errorf("expected [valid], got %+v", got)
}
})
t.Run("malformed JSON", func(t *testing.T) {
_, err := parseWorkspaceListJSON("not json {")
if err == nil {
t.Errorf("expected error on malformed JSON")
}
})
}
// TestExtractUnknownWorkspaceSlug covers the small regex helper used
// to pull a slug name out of CLI stderr like "workspace 'foo' does
// not exist". Only QUOTED slugs match — Codex review on PR #357
// caught that bare-word matching captured stop-words like "not" out
// of generic "Workspace not found" responses, which would push agents
// toward retrying with a bogus slug. The regex now requires single or
// double quotes around the slug; bare-word phrasings yield empty
// (the caller's message handles that gracefully).
func TestExtractUnknownWorkspaceSlug(t *testing.T) {
cases := map[string]string{
"Error: workspace 'foo' does not exist": "foo",
"Error: workspace \"bar\" not found": "bar",
"workspace 'docapp' is gone": "docapp",
// Bare-word phrasings — slug NOT extractable; empty result.
"unknown workspace baz": "",
"workspace docapp not visible": "",
"Workspace not found": "",
// Unrelated input.
"completely unrelated message": "",
"": "",
}
for stderr, want := range cases {
t.Run(stderr, func(t *testing.T) {
if got := extractUnknownWorkspaceSlug(stderr); got != want {
t.Errorf("got %q, want %q", got, want)
}
})
}
}
// decodeEnvelope helper extracts the ErrorEnvelope from an MCP error
// result. Uses textOf (the shared helper in catalog_test.go) to
// retrieve the JSON body from the result's content blocks.
func decodeEnvelope(t *testing.T, res *mcp.CallToolResult) ErrorEnvelope {
t.Helper()
body := textOf(res)
if body == "" {
t.Fatalf("error result has empty text body")
}
var env ErrorEnvelope
if err := json.Unmarshal([]byte(body), &env); err != nil {
t.Fatalf("decode envelope: %v\nbody=%s", err, body)
}
return env
}