mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 18:13:26 +00:00
xarmian
42f6ce96e1
Three independent improvements bundled as one PR because they all touch
the same dispatcher error-emission surface; landing them piecemeal
would churn the same lines repeatedly.
## TASK-1077 — uniform envelope shape
Pre-fix some dispatchers emitted plain-string errors via
`mcp.NewToolResultErrorf("%s: %s failed: %s", ...)`. Same underlying
404 surfaced in three different shapes across the surface (item
lookup → structured envelope; note/decide → "item note: prefetch:
404 ..."; bulk-update per-row → bare error string). Inconsistent
shape made it hard for agents to reason about errors uniformly.
Three new helpers in errors.go:
- validationFailedResult(cmdKey, msg, fixHint) — replaces the
"X is required" / "invalid Y" chain across every dispatcher.
- dispatcherErrorResult(cmdKey, op, err) — replaces the internal
"build request: %s" / "encode body: %s" / "parse current: %s"
chain. Always emits ErrServerError with a programmer-readable
Hint.
- upstreamHTTPErrorResult(...) — wraps every in-handler prefetch /
sub-call HTTP failure through classifyHTTPStatusKind so the shape
matches the main pipeline's responses exactly.
Every NewToolResultErrorf call site in internal/mcp/dispatch_http*.go
+ catalog.go retrofitted. bulk-update's per-row `Error string` field
flipped to `Error *ErrorPayload` so every row failure carries the
same {code, message, hint} shape as a top-level failure.
## TASK-1078 — resource-kind-aware error codes
Pre-fix every 4xx 404 collapsed to ErrItemNotFound regardless of
what was being read; pad_workspace list returning 404 (route
missing) reported `code: "item_not_found"` despite the call having
nothing to do with items. Pre-fix every 5xx collapsed to
ErrServerError, indistinguishable from dispatcher internal failures.
Three new codes in errors.go:
- ErrNotFound — resource-shaped 404s that AREN'T item lookups
(collection, listing endpoint, link target, attachment).
- ErrUpstreamError — 5xx with a structured body (transient backend
failure). Distinct from ErrServerError (catch-all for dispatcher
internal + un-mapped 4xx).
- ErrBackendUnreachable — reserved for transport-level failures
(DNS / connection refused / 5xx with no body); not yet emitted
by classifyHTTPStatus but available for future transport-aware
classification.
- ErrWorkspaceRequired — reserved for the multi-workspace-token
"ambiguous default" case (TASK-1076's deferred sister error;
constant available even though dispatcher doesn't emit it yet).
New ResourceKind enum (item/workspace/collection/listing/link/
attachment/unknown) lets callers tell the classifier what they
were reading. classifyHTTPStatusKind is the new entry point;
classifyHTTPStatus preserved as a legacy adapter for callers that
haven't been retrofitted (pass ResourceUnknown → falls back to
pre-TASK-1078 behaviour).
Every retrofit call site passes its known kind + ref/slug, so 404s
now route through the right code with a contextual message
("Item TASK-7 not found.", "Workspace foo not visible.",
"Collection tasks not found.", etc.).
## TASK-1079 — actionable hints
Pre-fix `hint` was usually `"404 page not found"` (chi's default
NotFound body verbatim) or the upstream JSON envelope re-stringified.
Either way: zero diagnostic value, sometimes outright misleading
(double-stringified JSON in a hint field is hostile).
Per-code hint generators in errors.go:
- itemMissingHint — names the ref + route + suggests pad_item
search / list as recovery.
- workspaceMissingHint — names the slug + route + composes with
the existing available_workspaces enrichment.
- notFoundHintFor — kind-aware: collection 404 → "use pad_collection
list to enumerate"; listing 404 → "verify the route matches the
server's API surface (build version may be stale)"; etc.
- authHintFor / permissionHintFor — point at re-auth / scope check.
- upstreamHintFor — flags 5xx as "usually transient — retry once or
check pad logs."
extractUpstreamMessage parses pad's own structured `{error:{message}}`
envelope when the upstream backend returned one, so hints lift the
inner human-readable message out instead of dumping the literal JSON.
Falls back to the raw body when the JSON shape doesn't match (no
parse failure noise).
## Tests
- TestDispatcher_AllErrorsUseStructuredEnvelope walks every
special-case + link dispatcher's missing-required-input error
path; pins the shape (code, message, hint all set; hint never
just "404 page not found"). Adding a new dispatcher that uses
NewToolResultErrorf will fail this test — it's the regression
gate the DOD wants.
- TestClassifyHTTPStatus_KindAware pins each ResourceKind →
expected ErrorCode mapping for 404s.
- TestClassifyHTTPStatus_HintsAreActionable pins that hints
reference the actual route + ref + recovery tools, AND forbids
the bare "404 page not found" passthrough that triggered Bug 17.
- TestExtractUpstreamMessage covers the 7 input shapes the helper
can see (structured envelope, empty inner, missing inner field,
unparseable, wrong shape, empty, with extra fields).
- Two existing tests updated to reflect the new shapes:
TestClassifyHTTPStatus 5xx cases now expect ErrUpstreamError;
TestMakeFanOutHandler_UnknownAction + TestActionEnv_Dispatch_
UnknownCmdPath substring searches updated for JSON-encoded
quotes.
## Behavior diff agents will observe
Same underlying 404, three example error envelopes:
pad_item show TASK-MISSING:
code: "item_not_found"
message: "Item not found."
hint: "Item \"TASK-MISSING\" not found. Route: /api/v1/.../items/TASK-MISSING. Try `pad_item search` or `pad_item list` to find the right ref."
pad_workspace list (route 404):
code: "unknown_workspace"
message: "Workspace not visible to this session."
hint: "Route: /api/v1/workspaces. Available workspaces: docapp, pad-web."
pad_project dashboard (workspace doesn't exist):
code: "unknown_workspace"
message: "Workspace \"missing\" is not visible to this session."
hint: "Workspace \"missing\" not visible. Route: /api/v1/workspaces/missing/dashboard. Available workspaces: docapp."
Backend 500:
code: "upstream_error"
message: "pad item show failed: backend returned 500"
hint: "Backend returned 500. Usually transient — retry once or check pad logs for the underlying error. Route: ..."