Files
pad/internal/mcp/dispatch_http_project.go
xarmian 449ac109e9 fix(server,mcp,cli): refuse system-metadata keys in fields_patch + retry-hostile error code (BUG-2627 part 2, BUG-2675) (#1166)
* fix(server,mcp,cli): refuse system-metadata keys in fields_patch + retry-hostile error code (BUG-2627 part 2, BUG-2675)

Part 2 of BUG-2627 closes the door that mints the defect parts 1 and 3
dealt with: `--field implementation_notes=<json>` stored the entries as a
JSON-ENCODED STRING, which is invisible to every reader and — since part
3's guard — disables `pad item note` on that item until the row is
repaired.

Refused SERVER-SIDE in `fields_patch`, not at the CLI as the item's scope
line proposed. The deviation is deliberate and recorded on the trail: the
CLI is one of three clients, and all three lower a user field-setter into
the same key (`pad item update --field` at cmd_item.go, the MCP `field`
param via dispatch_http_advanced.go on remote, and stdio by shelling out
to that CLI). One gate closes all three; a CLI-only refusal would have
left remote MCP writing the key. Both call sites were read, and the CLI's
lowering is now pinned by a test rather than left as an assumption.

Scope, stated because it is deliberate: this closes UPDATE only. The full
`fields` blob stays open because that door is SHARED — `pad item note` /
`decide` / `github link` send one, and so does convention activation via
BuildConventionItemFields -> ItemCreate. Closing it would break the system
writers the gate exists to protect. Item create therefore remains a mint
site, tracked with the rest of that surface in BUG-2685.

The refusal message is per-key: implementation_notes -> `pad item note`,
decision_log -> `pad item decide`, github_pr -> the GitHub link flow, and
`convention` refuses WITHOUT naming a command, because none writes it.
PATTE-135 wants a remedy that works in the failing state; a single
"use pad item note" line would have been wrong for three of the four keys.

BUG-2675 rides along on one ToolSurfaceVersion bump, as ruled. The append
refusal from part 3 reached MCP agents as `server_error` — not our fault,
and not transient, so agents could reasonably retry a failure that is
deterministic forever. New closed-set code `stored_state_unreadable`,
emitted on BOTH transports: HTTP classifies the sentinel error directly,
stdio via a `pad-structured-error/v1:` marker the CLI now writes for its
own local refusal (the first marker generated without an upstream
APIError). v0.16-then-v0.17 is what a one-transport fix costs.

Also here:
- items.ReservedOverrideKeys -> ReservedFieldKeysIn. The second caller
  passes a patch, not an override map, and the old doc comment said
  fields_patch was an open exposure — true until this commit.
- `Extract* returns nil for THREE reasons` -> FOUR. The comment listed
  four; the count was corrected everywhere except the code.
- Consumer-read artifacts updated where the claim is ACTED on, not only
  where it is documented: instructions.md (incl. a "do not retry this
  code" section), the catalog `field` param description, `pad item update
  --help`, README.

Gates: build · make lint · go test ./... · make test-pg · Codex.
Eleven-mutation matrix run against the new tests; every one killed by an
assertion (two were rewritten after killing by compile error / surviving,
which proves nothing).

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(server,mcp): honest remedy when the stored value is already unreadable; name the MCP-facing code (Codex round 1)

Three findings from the pre-push review, all real:

P2 — the refusal named `pad item note` unconditionally, but on an item
whose stored value is ALREADY undecodable that command refuses too (part
3's guard). The caller was routed in a circle: field write refused -> run
the note -> refused -> back again. That is exactly the failure PATTE-135
exists to prevent, and my own trail had reasoned the remedy was safe on
the strength of the HEALTHY case only. The message now inspects the
item's stored value and, when the key is unparseable, says so and points
at the one action that works in that state (inspection), noting that the
repair needs a full `fields` write no CLI flag exposes.

P2 — two doc claims were false where an actor reads them. The catalog
said reserved keys are refused "on every action that accepts field",
which includes CREATE, and create is deliberately NOT gated; and both the
catalog and instructions.md named `validation_error` (the HTTP code)
where an MCP client actually receives `validation_failed`. Both corrected,
and the create exception is now stated rather than implied by omission —
an agent that reads only "refused on update" will otherwise assume create
is fine, which is how a hole gets used.

nit — the destructive-downstream sentence claimed every reserved key
becomes unreadable and trips an append guard. True only for the two
append-backed keys; github_pr and convention are simply overwritten. The
clause is now per-key, because a confident wrong explanation is worse
than a vague right one.

Two more mutations run against the new branch: always-readable (the
circular remedy returns) and never-readable (the working remedy
disappears) — both killed by assertions.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(models,mcp,cli): one appendability predicate, per-key docs, stdio hint parity (Codex round 2)

Five findings, all real.

P2 — the message's readability check and the guard it describes were two
different decodes. Mine unmarshalled into []json.RawMessage; the guard
uses []ItemImplementationNote. A stored `[1]` passed mine and fails the
guard, so the message would again have prescribed a command that refuses
— the same circularity round 1 caught, through a narrower door. Replaced
with models.StructuredFieldIsAppendable, which ASKS the guard rather than
re-deriving it, plus an agreement test over 12 shapes x 2 keys that
compares the predicate against the real Append* helpers. Verified by
restoring the RawMessage version: the table catches it on `[1]`.

P2 — stdio lost the new code's hint. Remote MCP told the agent retrying
is pointless and how to inspect; stdio got the code with an empty hint,
because the CLI's marker envelope carried none and the classifier parsed
none. Both fixed, with the hint hoisted into paired constants (the same
duplication StructuredErrorMarker already uses) and the test comparing
the two TRANSPORTS' envelopes rather than either against a literal.

P2 — doc text was still false for `convention`: the catalog, the
instructions and `--help` all said reserved keys are maintained by
note/decide/the GitHub flow, which is true of three of the four. Each key
now names its own writer, and `convention` names library activation.
Also dropped the `malformed_override` advertisement — that is the
SERVER's code; an MCP client sees validation_failed for both refusals.

nit — the classification test called structuredAppendErrorResult
directly, so deleting either dispatcher call site left it green.
Added dispatcher-level tests driving the real server + store, asserting
the code, the hint, and that the item's stored fields are byte-identical
afterwards. Mutation-verified by reverting the note call site.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(items,models,mcp): github_pr stays writable through fields_patch; no nil-map panic (Codex round 3)

P1 — the gate refused `github_pr`, and that was wrong. My model was
"system writers use the full fields blob, user setters use fields_patch",
which holds for three of the four reserved keys and fails for this one:
`pad github link` needs a local git checkout and the `gh` CLI, so it is
excluded from remote MCP BY NAME, and internal/mcp/dispatch_http.go's
noRemoteEquivalent map tells remote agents in so many words to use
`item update --field github_pr=...` instead. For that audience the patch
door is not a bypass of the writer — it IS the writer.

So the refusal deleted a documented capability from remote agents, and
answered with a message naming a command they cannot run: the same
circular remedy round 1 caught, aimed this time at the people the gate
was meant to help. items.PatchRefusedFieldKeysIn now exempts the key and
records the rule being applied — refuse a raw write where a real writer
exists — rather than the list it produces. Whether remote agents should
get a proper PR-link action, so the key can be closed too, is a product
question and is left as one.

P2 — the hint told agents to read the bad value with `pad_item action=get`.
They cannot: stripDuplicatedFieldsKeys removes implementation_notes and
decision_log from every MCP response's fields blob, and the top-level
arrays come from the extractor, which returns nil for exactly this shape.
The value is invisible on the whole surface. The hint now says so and
routes to a human, who can read it with `pad item show --format json`.

P2 — `fields` holding a literal `null` unmarshals into a NIL map with no
error, and both Append* helpers assign into what they get back, so
`pad item note` PANICKED ("assignment to entry in nil map") instead of
appending. Reproduced, fixed in parseMutableItemFields, and pinned by a
test that fails on a panic rather than taking the process down. An absent
blob and a null blob mean the same thing to every caller. Pre-existing,
but it sits in the function family this bug is about and the message was
about to recommend the command that panics.

nit — README claimed a "closed eight-code taxonomy" (17 codes, and I had
just added one) and read as if create lowers into fields_patch.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(models,mcp): predicate matches the append on malformed blobs; stop promising a broken workaround (Codex round 4)

P1 — round 3 exempted `github_pr` from the update gate on the strength
of noRemoteEquivalent's documented workaround. That workaround does not
work: ingestFieldKVP (remote) and parseFieldFlag (CLI, and so stdio) both
store a `field` value as a STRING, so the PR data lands double-encoded
and no link appears — the BUG-2627 shape one key over. Filed as BUG-2696
with the three candidate fixes; NOT folded in, because the narrowest of
them changes how every field value is typed.

The exemption stands regardless: refusing would leave remote agents with
strictly less than a broken door. What changes is what we may PROMISE.
The catalog, instructions.md, version.go and README said "this is how you
link a PR"; they now say the door is open and broken, and to hand PR
linking to a human. Advertising a capability that isn't there is the
failure mode this whole unit keeps circling.

P2 — StructuredFieldIsAppendable returned TRUE when the whole fields blob
was unparseable, on the reasoning that a broken outer blob is a different
problem. True of the cause, irrelevant to the caller: the Append* helpers
bail on that same parse, so the message again named a command that fails.
It now returns false, which is simply the honest answer to the question
asked, and the agreement table grew a malformed-outer-blob leg — the gap
that let the disagreement through.

P2 — the message claimed a raw field write always stores something Pad
cannot read back. That holds for the CLI and MCP (a `--field` value is
typed by schema lookup and these keys are in no schema) but not for a
direct REST caller sending a valid array, who is refused for ownership
reasons alone. Reworded to say both parts.

nit — a misplaced parenthetical in the README read as if item CREATE
lowers into fields_patch. It does not; it sends the full blob.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(mcp,models): stop the remote hint advertising the broken PR workaround; classify an unparseable blob as retry-hostile (Codex round 5)

P1 — I corrected four artifacts that pointed agents at the github_pr
field write and missed the fifth: noRemoteEquivalent's own text, which IS
the message a remote agent receives when it calls `github link`, and
which Codex had quoted at me in round 3 to establish the workaround
existed. The nearest artifact to the actor was the one I did not open.
Both entries now say there is no working remote path and name BUG-2696,
with a test pinning the negative so a future edit cannot quietly
reinstate the advice while the write is still broken.

P2 — a fields blob that will not parse at all produced a bare parse
error, so `note` / `decide` reached agents as `server_error`: transient-
looking, and therefore retried, for a failure that is as deterministic as
the per-key one BUG-2675 exists for. Both Append* helpers now wrap that
parse failure in ErrStructuredFieldUnreadable, which both transports
already classify, and the malformed-blob test asserts the sentinel rather
than just an error.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* docs(mcp,cli): qualify what an agent can actually see when the state is unreadable (Codex round 6 nit)

Round 5 widened stored_state_unreadable to cover a fields blob that
fails to parse outright, which made half of its own hint false: MCP's
normalization strips a broken structured KEY (so `get` hides it), but
leaves an unparseable BLOB as a raw string (so `get` shows it). The hint
and instructions.md asserted the first case for both.

Now stated per layer, in the two paired constants and the instructions.
The reason it is worth the words rather than being cut: an agent told
'you cannot see this' does not look, and would have missed a value that
was in fact right there in the response it already had.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(mcp): classify the move/copy reserved-key refusal as validation on stdio too (Codex round 7)

P2 — carried over from v0.22, surfaced because THIS bump documents the
two reserved-key refusals as agreeing across transports. The move/copy
message ("Field(s) reserved for system metadata and not settable here")
matched none of the stdio validation patterns, so the same deterministic
400 arrived as validation_failed on remote and server_error on stdio —
and server_error reads as transient, so an agent retries a refusal that
can never pass. One pattern added, plus a test that drives both real
classifiers with the real server message text for both refusals, so a
reworded message that stops matching fails here rather than in the field.

nit — the github_pr exemption is UPDATE-only; move and copy still refuse
it, because there the argument is BUG-2674's (an override reintroduces
the key the migration just dropped), not this one's. The catalog and
instructions said "not refused" without that qualifier.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* fix(mcp): cover the copy path's own refusal wording in the stdio classifier (Codex round 8)

P2 — round 7 fixed the MOVE wording; the copy path words the same class
of refusal differently ("Destination collection has no field(s): ..."),
so it kept arriving as server_error on stdio and validation_failed on
remote. Third message in one family, and the round-7 test used the move
text for every case, which is why it missed this.

The parity table now carries all three real messages plus a control leg
using one the pattern list already covered — without it the table could
pass by matching everything.

Recorded in the pattern list's comment rather than left implicit:
matching prose is a stopgap, the structural fix is the
pad-structured-error/v1 marker that carries the code instead of inferring
it, and until a refusal emits one, this test is where a new wording has
to be added.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* test(mcp): use the real upstream codes in the parity fixtures (Codex round 9 nit)

The copy legs carried `validation_error` where the handlers actually
emit `malformed_override` and `invalid_override`. The 400 branch ignores
the body code today, so the test passed either way — which is exactly why
the fixture mattered: it was quietly recording a wrong contract, and a
future code-aware classifier would regress against a table that agrees
with it.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* docs(mcp): the upstream code is not forwarded to MCP clients (Codex round 10 nit)

The catalog said the server's own code (validation_error /
malformed_override) appears in the MCP message. It does not: the 400
branch emits code=validation_failed with a fixed "Validation failed."
message and the server's text in the HINT, discarding the finer-grained
code. Reworded to say what an agent actually receives, and to say that
telling the two refusals apart means reading the message.

Also carried the update-only qualifier on the github_pr exemption into
the README, matching the catalog and instructions.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V

* docs(items): state the exemption predicate, not the exemption list (lead ruling)

The lead's ruling on the github_pr reversal: make the REASON what the code
says, so the next key added to reserved metadata is evaluated against
'does this audience have a real writer?' rather than pattern-matched onto
a list that happened to be wrong for one key.

Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
2026-08-19 23:17:24 -04:00

778 lines
28 KiB
Go

package mcp
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"sort"
"strings"
"time"
"github.com/mark3labs/mcp-go/mcp"
"github.com/PerpetualSoftware/pad/internal/models"
)
// dispatchProjectReady reproduces `pad project ready --format json` —
// the CLI returns `{count, results}` extracted from the dashboard's
// SuggestedNext slice, NOT the full dashboard payload. (Compare with
// `pad project next` which returns the raw dashboard JSON; both surface
// the same suggestions but with different framing.)
//
// Aliasing to /dashboard would be a behavioural divergence: the
// agent would see an unexpected wrapper shape and have to know to dig
// into `suggested_next`. Mirroring the CLI's `{count, results}` shape
// keeps the MCP transport equivalent to ExecDispatcher.
func (d *HTTPHandlerDispatcher) dispatchProjectReady(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "project ready"
dash, errRes := d.fetchDashboardJSON(ctx, input, user, cmdKey)
if errRes != nil {
return errRes, nil
}
suggestions := dashboardArrayField(dash, "suggested_next")
return packageStructuredResponse(cmdKey, map[string]any{
"count": len(suggestions),
"results": suggestions,
})
}
// dispatchProjectStale reproduces `pad project stale --format json` —
// CLI filters the dashboard's Attention slice to "interesting" types
// (stalled / blocked / overdue / orphaned_task) before returning
// `{count, results}`. Sorting matches cmd/pad/query.go's
// filterAgentAttention: type, ItemRef, ItemTitle.
//
// Operates on the raw map[string]any decoded from the dashboard JSON
// so any field server.DashboardAttention adds in future versions
// (collection, plus anything not yet wired) flows through unchanged.
// Codex review on PR #348 round 1 caught the previous typed-struct
// approach dropping `collection` from the response.
func (d *HTTPHandlerDispatcher) dispatchProjectStale(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "project stale"
dash, errRes := d.fetchDashboardJSON(ctx, input, user, cmdKey)
if errRes != nil {
return errRes, nil
}
attention := filterAgentAttention(dashboardArrayField(dash, "attention"))
return packageStructuredResponse(cmdKey, map[string]any{
"count": len(attention),
"results": attention,
})
}
// dispatchProjectNext proxies `project next` to GET
// /workspaces/{ws}/next — handleGetProjectNext
// (internal/server/handlers_project_intel.go), the browser/REST
// surface PLAN-1888 / TASK-1894 added. Before TASK-1916 this method
// fetched the dashboard itself and sliced out suggested_next (there
// was no REST endpoint to route to yet — see BUG-987 bug 6: without
// that slicing, `project next` was indistinguishable from `project
// dashboard`); now the REST endpoint already returns exactly that
// slice, so this is a thin validate-then-forward.
//
// Kept as a hand-written method rather than a routeTable entry so the
// missing-workspace error keeps the pad_set_workspace hint every other
// workspace-required dispatcher uses (a plain routeSpec's missing
// {workspace} placeholder produces a more generic "check your input
// shape" hint instead — see expandPath in dispatch_http_routes.go).
//
// No re-marshaling needed for the BUG-985 array-wrap: executeRequest's
// packageHTTPResponse already wraps top-level JSON arrays as
// {items: [...]} for every proxied GET, and /next returns a bare
// array.
func (d *HTTPHandlerDispatcher) dispatchProjectNext(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "project next"
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
path := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/next"
return d.executeRequest(ctx, cmdKey, user, http.MethodGet, path, nil)
}
// dispatchProjectStandup proxies `project standup` to GET
// /workspaces/{ws}/standup?days=N — handleGetProjectStandup
// (internal/server/handlers_project_intel.go). Before TASK-1916 this
// method reproduced the CLI's standupCmd JSON branch itself (dashboard
// fetch + per-terminal-status ListItems loop + in-progress fetch);
// that reshaping, its days=1 default, and its per-status best-effort
// error handling (a failed status is skipped, not fatal) now all live
// server-side in handleGetProjectStandup / listTerminalItemsSince,
// so this method only validates workspace (same pad_set_workspace
// hint as dispatchProjectNext above, same routeTable-vs-custom-method
// reasoning) and forwards `days` as-is via the shared buildQuery
// helper (dispatch_http_routes.go) — parseDaysParam server-side
// applies the identical n<=0-falls-back-to-default gate the
// in-dispatcher `numericInput(...) && n > 0` check used to.
func (d *HTTPHandlerDispatcher) dispatchProjectStandup(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "project standup"
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
path := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/standup"
if q := buildQuery(input, map[string]string{"days": "days"}); q != "" {
path += "?" + q
}
return d.executeRequest(ctx, cmdKey, user, http.MethodGet, path, nil)
}
// dispatchProjectChangelog proxies `project changelog` to GET
// /workspaces/{ws}/changelog?days=N&since=YYYY-MM-DD&parent=REF —
// handleGetProjectChangelog (internal/server/handlers_project_intel.go).
// Same consolidation as dispatchProjectStandup above: the per-status
// listing loop, days=7 default, since-overrides-days precedence,
// parent filtering, and collection grouping all now live server-side.
//
// One observable behavior change from consolidation: a malformed
// `since` used to get a bespoke dispatcher-side hint ("Pass
// since=YYYY-MM-DD..."); now it flows through the REST endpoint's 400
// and the generic HTTP-error classifier (classifyHTTPStatusKind) like
// every other proxied validation error in this codebase — same
// ErrValidationFailed code, but the hint text becomes the generic
// "Backend: invalid 'since' date...Adjust the input shape and retry."
// rather than the old bespoke wording. Accepted as part of TASK-1916's
// consolidation (keeping the bespoke hint here would mean re-parsing
// `since` twice and defeats the point of proxying).
func (d *HTTPHandlerDispatcher) dispatchProjectChangelog(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "project changelog"
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
path := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/changelog"
if q := buildQuery(input, map[string]string{"days": "days", "since": "since", "parent": "parent"}); q != "" {
path += "?" + q
}
return d.executeRequest(ctx, cmdKey, user, http.MethodGet, path, nil)
}
// fetchDashboardJSON hits the workspace dashboard endpoint and decodes
// the response into a generic map[string]any so the dispatcher
// preserves every field the server emits — no maintenance burden when
// new fields land on DashboardAttention / DashboardSuggestion.
//
// Returns the raw object so callers can pull specific fields
// (suggested_next, attention) via dashboardArrayField without the
// typed-struct round-trip.
func (d *HTTPHandlerDispatcher) fetchDashboardJSON(
ctx context.Context,
input map[string]any,
user *models.User,
cmdKey string,
) (map[string]any, *mcp.CallToolResult) {
workspace, _ := input["workspace"].(string)
if workspace == "" {
return nil, validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace.")
}
path := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/dashboard"
req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
if err != nil {
return nil, dispatcherErrorResult(cmdKey, "build dashboard request", err)
}
rec := httptest.NewRecorder()
d.Handler.ServeHTTP(rec, req)
if rec.Code >= 400 {
return nil, upstreamHTTPErrorResult(ctx, cmdKey, "fetch dashboard", path,
rec.Code, rec.Body.Bytes(), d.Lister, ResourceWorkspace, workspace)
}
var dash map[string]any
if err := json.Unmarshal(rec.Body.Bytes(), &dash); err != nil {
return nil, dispatcherErrorResult(cmdKey, "parse dashboard", err)
}
return dash, nil
}
// dashboardArrayField pulls a named array out of a decoded dashboard
// payload, returning a typed []map[string]any so the callers can
// filter / sort by string fields without the json.Number / interface{}
// dance per element. Missing/empty/non-array values normalize to an
// empty slice so the {count, results} responses always emit a usable
// shape.
func dashboardArrayField(dash map[string]any, key string) []map[string]any {
raw, ok := dash[key].([]any)
if !ok {
return []map[string]any{}
}
out := make([]map[string]any, 0, len(raw))
for _, e := range raw {
if m, ok := e.(map[string]any); ok {
out = append(out, m)
}
}
return out
}
// filterAgentAttention mirrors cmd/pad/query.go's helper of the same
// name — keeps only the attention types agents care about (stalled,
// blocked, overdue, orphaned_task) and sorts deterministically by
// (type, item_ref, item_title). Same stable ordering as the CLI so
// `--format json` outputs match between transports.
//
// Operates on map[string]any (not a typed struct) so attention
// entries pass through to the response with EVERY field the server
// emitted, not just the ones we knew to declare. Codex review on PR
// #348 caught the previous typed approach dropping `collection`.
func filterAgentAttention(attention []map[string]any) []map[string]any {
interesting := map[string]bool{
"stalled": true,
"blocked": true,
"overdue": true,
"orphaned_task": true,
}
results := make([]map[string]any, 0, len(attention))
for _, item := range attention {
typ, _ := item["type"].(string)
if interesting[typ] {
results = append(results, item)
}
}
sort.SliceStable(results, func(i, j int) bool {
ti, _ := results[i]["type"].(string)
tj, _ := results[j]["type"].(string)
if ti != tj {
return ti < tj
}
ri, _ := results[i]["item_ref"].(string)
rj, _ := results[j]["item_ref"].(string)
if ri != rj {
return ri < rj
}
titI, _ := results[i]["item_title"].(string)
titJ, _ := results[j]["item_title"].(string)
return titI < titJ
})
return results
}
// --- item bulk-update ---
// dispatchItemBulkUpdate iterates the input's `ref` array and applies
// --status / --priority via the same read-modify-write semantics the
// item.update path uses (so existing fields survive). Mirrors the
// CLI's bulkUpdateCmd: at-least-one-of-status-or-priority gating, per-
// item GET → field merge → PATCH, and a per-item success/error report.
//
// The cmdhelp surface marks `ref` as required AND repeatable — agents
// pass it as either []any (typical JSON array) or []string. Anything
// else is rejected so the dispatcher doesn't silently iterate over
// nothing.
//
// Per-item failures don't abort the bulk operation; they get
// individually reported in the response so an agent can inspect what
// succeeded vs. failed without having to retry the whole batch.
func (d *HTTPHandlerDispatcher) dispatchItemBulkUpdate(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "item bulk-update"
workspace, _ := input["workspace"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
refs, err := bulkUpdateRefs(input["ref"])
if err != nil {
return validationFailedResult(cmdKey, err.Error(),
"Pass `ref` as a string or array of TASK-N / BUG-N / etc. refs."), nil
}
if len(refs) == 0 {
return validationFailedResult(cmdKey, "at least one ref is required",
"Pass at least one item ref (e.g. ref=[\"TASK-7\",\"TASK-8\"])."), nil
}
status, _ := input["status"].(string)
priority, _ := input["priority"].(string)
if status == "" && priority == "" {
return validationFailedResult(cmdKey, "at least one of status or priority is required",
"Pass `status=<value>` and/or `priority=<value>` to specify what to update."), nil
}
type bulkResult struct {
Ref string `json:"ref"`
Updated bool `json:"updated"`
Error *ErrorPayload `json:"error,omitempty"`
}
results := make([]bulkResult, 0, len(refs))
successes := 0
// rowError builds a per-row ErrorPayload. Reuses the same shape
// the top-level error envelope uses — agents see consistent
// {code, message, hint} per failed row instead of bare strings
// (BUG-1077 / Bug 15: bulk-update was the partially-fixed case
// that surfaced this requirement).
rowError := func(code ErrorCode, msg, hint string) *ErrorPayload {
return &ErrorPayload{Code: code, Message: msg, Hint: hint}
}
rowDispatcherError := func(op string, err error) *ErrorPayload {
return rowError(ErrServerError, fmt.Sprintf("%s failed", op),
fmt.Sprintf("Internal: %s — %s", op, err.Error()))
}
rowUpstreamError := func(op, route string, status int, body []byte, ref string) *ErrorPayload {
res := classifyHTTPStatusKind(ctx, cmdKey, route, status, body, d.Lister, ResourceItem, ref)
env := envelopeFrom(res)
if op != "" && env.Error.Message != "" {
env.Error.Message = fmt.Sprintf("%s (%s)", env.Error.Message, op)
}
return &env.Error
}
for _, ref := range refs {
// Per-item RMW: GET, merge fields, PATCH. Same shape
// dispatchItemUpdate uses, but inlined here so a per-item
// failure produces a {ref, error} entry instead of aborting.
itemPath := "/api/v1/workspaces/" + url.PathEscape(workspace) +
"/items/" + url.PathEscape(ref)
getReq, err := d.buildAuthedRequest(ctx, http.MethodGet, itemPath, nil, user)
if err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("build request", err)})
continue
}
getRec := httptest.NewRecorder()
d.Handler.ServeHTTP(getRec, getReq)
if getRec.Code >= 400 {
results = append(results, bulkResult{
Ref: ref,
Error: rowUpstreamError("read item", itemPath, getRec.Code, getRec.Body.Bytes(), ref),
})
continue
}
var existing struct {
Fields string `json:"fields"`
}
if err := json.Unmarshal(getRec.Body.Bytes(), &existing); err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("parse item", err)})
continue
}
merged := map[string]any{}
if existing.Fields != "" && existing.Fields != "{}" {
if err := json.Unmarshal([]byte(existing.Fields), &merged); err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("parse existing fields", err)})
continue
}
}
if status != "" {
merged["status"] = status
}
if priority != "" {
merged["priority"] = priority
}
fieldsJSON, err := json.Marshal(merged)
if err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("encode fields", err)})
continue
}
fieldsStr := string(fieldsJSON)
patchPayload := map[string]any{"fields": fieldsStr}
// IDEA-1494: forward the open-children guard override per-row.
// Same flag shape as `pad item bulk-update --force` so the
// override travels through both transports identically.
if b, ok := input["force"].(bool); ok && b {
patchPayload["force"] = true
}
patchBody, err := json.Marshal(patchPayload)
if err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("encode body", err)})
continue
}
patchReq, err := d.buildAuthedRequest(ctx, http.MethodPatch, itemPath, patchBody, user)
if err != nil {
results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("build PATCH", err)})
continue
}
patchRec := httptest.NewRecorder()
d.Handler.ServeHTTP(patchRec, patchReq)
if patchRec.Code >= 400 {
results = append(results, bulkResult{
Ref: ref,
Error: rowUpstreamError("update item", itemPath, patchRec.Code, patchRec.Body.Bytes(), ref),
})
continue
}
results = append(results, bulkResult{Ref: ref, Updated: true})
successes++
}
payload := map[string]any{
"updated": successes,
"total": len(refs),
"results": results,
}
return packageStructuredResponse(cmdKey, payload)
}
// bulkUpdateRefs canonicalizes the `ref` input — accepts repeatable
// shapes the cmdhelp registry generates (string for a single value,
// []any from JSON arrays, []string from typed callers) into a clean
// []string. Empty / non-string entries are rejected so we don't
// silently skip elements an agent expected to be processed.
func bulkUpdateRefs(raw any) ([]string, error) {
switch v := raw.(type) {
case nil:
return nil, nil
case string:
if v == "" {
return nil, nil
}
return []string{v}, nil
case []string:
out := make([]string, 0, len(v))
for i, s := range v {
if s == "" {
return nil, fmt.Errorf("ref[%d] is empty", i)
}
out = append(out, s)
}
return out, nil
case []any:
out := make([]string, 0, len(v))
for i, e := range v {
s, ok := e.(string)
if !ok {
return nil, fmt.Errorf("ref[%d] must be a string, got %T", i, e)
}
if s == "" {
return nil, fmt.Errorf("ref[%d] is empty", i)
}
out = append(out, s)
}
return out, nil
default:
return nil, fmt.Errorf("ref must be a string or array of strings, got %T", raw)
}
}
// --- item note + decide (RMW append) ---
// dispatchItemNote handles `pad item note <ref> <summary>
// [--details ...]` — appends an implementation-note entry to the
// item's structured-fields blob, then PATCHes.
//
// Same RMW shape as dispatchItemUpdate but using
// models.AppendImplementationNote so the entry gets the right shape
// + ID + timestamp the CLI applies.
//
// Emits the updated item (the PATCH response) like every other
// dispatcher — agents see the same shape they'd get from a follow-up
// `item show`.
func (d *HTTPHandlerDispatcher) dispatchItemNote(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "item note"
workspace, _ := input["workspace"].(string)
ref, _ := input["ref"].(string)
summary, _ := input["summary"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
if ref == "" {
return validationFailedResult(cmdKey, "ref is required",
"Pass `ref=<TASK-N>` (or whichever item ref the note targets)."), nil
}
if summary == "" {
return validationFailedResult(cmdKey, "summary is required",
"Pass `summary=<short text>` describing the note."), nil
}
details, _ := input["details"].(string)
details = strings.TrimSpace(details)
itemPath := "/api/v1/workspaces/" + url.PathEscape(workspace) +
"/items/" + url.PathEscape(ref)
currentFields, errRes := d.prefetchItemFields(ctx, user, cmdKey, itemPath, ref)
if errRes != nil {
return errRes, nil
}
updated, err := models.AppendImplementationNote(currentFields, models.ItemImplementationNote{
ID: newStructuredEntryID("note"),
Summary: strings.TrimSpace(summary),
Details: details,
CreatedAt: time.Now().UTC().Format(time.RFC3339),
CreatedBy: userActorLabel(user),
})
if err != nil {
return structuredAppendErrorResult(cmdKey, "append note", err), nil
}
body, err := json.Marshal(map[string]any{"fields": updated})
if err != nil {
return dispatcherErrorResult(cmdKey, "encode body", err), nil
}
return d.executeRequest(ctx, cmdKey, user, http.MethodPatch, itemPath, body)
}
// dispatchItemDecide is the decision-log analogue of
// dispatchItemNote — same RMW shape, just using
// AppendDecisionLogEntry on a different fields slot.
func (d *HTTPHandlerDispatcher) dispatchItemDecide(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "item decide"
workspace, _ := input["workspace"].(string)
ref, _ := input["ref"].(string)
decision, _ := input["decision"].(string)
if workspace == "" {
return validationFailedResult(cmdKey, "workspace is required",
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
}
if ref == "" {
return validationFailedResult(cmdKey, "ref is required",
"Pass `ref=<TASK-N>` (or whichever item ref the decision targets)."), nil
}
if decision == "" {
return validationFailedResult(cmdKey, "decision is required",
"Pass `decision=<short text>` describing the decision."), nil
}
rationale, _ := input["rationale"].(string)
rationale = strings.TrimSpace(rationale)
itemPath := "/api/v1/workspaces/" + url.PathEscape(workspace) +
"/items/" + url.PathEscape(ref)
currentFields, errRes := d.prefetchItemFields(ctx, user, cmdKey, itemPath, ref)
if errRes != nil {
return errRes, nil
}
updated, err := models.AppendDecisionLogEntry(currentFields, models.ItemDecisionLogEntry{
ID: newStructuredEntryID("decision"),
Decision: strings.TrimSpace(decision),
Rationale: rationale,
CreatedAt: time.Now().UTC().Format(time.RFC3339),
CreatedBy: userActorLabel(user),
})
if err != nil {
return structuredAppendErrorResult(cmdKey, "append decision", err), nil
}
body, err := json.Marshal(map[string]any{"fields": updated})
if err != nil {
return dispatcherErrorResult(cmdKey, "encode body", err), nil
}
return d.executeRequest(ctx, cmdKey, user, http.MethodPatch, itemPath, body)
}
// prefetchItemFields GETs the item at itemPath and returns its
// `fields` JSON string. Surfaces 404s and parse errors as
// IsError-flagged tool results so the dispatcher's caller can return
// them directly without further wrapping.
//
// Used by note/decide which append into the existing fields blob —
// they need the current value so AppendImplementationNote /
// AppendDecisionLogEntry can preserve other entries.
//
// ref is the item ref the caller was looking up; threaded through to
// the error envelope so agents see e.g. "Item TASK-7 not found"
// rather than a bare 404 (TASK-1078 / TASK-1079).
func (d *HTTPHandlerDispatcher) prefetchItemFields(
ctx context.Context,
user *models.User,
cmdKey, itemPath, ref string,
) (string, *mcp.CallToolResult) {
req, err := d.buildAuthedRequest(ctx, http.MethodGet, itemPath, nil, user)
if err != nil {
return "", dispatcherErrorResult(cmdKey, "build prefetch", err)
}
rec := httptest.NewRecorder()
d.Handler.ServeHTTP(rec, req)
if rec.Code >= 400 {
return "", upstreamHTTPErrorResult(ctx, cmdKey, "prefetch item", itemPath,
rec.Code, rec.Body.Bytes(), d.Lister, ResourceItem, ref)
}
var existing struct {
Fields string `json:"fields"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &existing); err != nil {
return "", dispatcherErrorResult(cmdKey, "parse current item", err)
}
return existing.Fields, nil
}
// newStructuredEntryID mirrors the CLI's helper for note/decision
// IDs (cmd/pad/notes.go). The actual collision-avoidance is handled
// by combining the prefix + a unix-nano timestamp — same shape so
// CLI-created and MCP-created entries are indistinguishable in
// downstream consumers.
func newStructuredEntryID(prefix string) string {
return fmt.Sprintf("%s-%d", prefix, time.Now().UTC().UnixNano())
}
// userActorLabel produces a stable string label for the actor that
// created a structured entry. Mirrors the CLI's "user" label for
// CLI-driven entries; for MCP we use the requesting user's name (or
// email fallback) so audit-log review can tell who appended what
// when multiple users share the same MCP server.
func userActorLabel(user *models.User) string {
if user == nil {
return "user"
}
if user.Name != "" {
return user.Name
}
if user.Email != "" {
return user.Email
}
return "user"
}
// dispatchLibraryList composes the /convention-library and
// /playbook-library endpoints to mirror `pad library list --format
// json`. The CLI's JSON output shape varies on --type:
//
// - --type conventions → returns the convention library (lib).
// - --type playbooks → returns the playbook library (plib).
// - (no --type) → returns {conventions: lib, playbooks: plib}.
//
// Inputs honored (PLAN-1560 / TASK-1561+TASK-1563):
// - type: restricts to conventions or playbooks (above).
// - category: server-side category filter; case-sensitive exact match.
// Forwarded to BOTH endpoints when set.
// - full: when true, playbooks come back with full bodies. Default
// (full=false) passes ?summary=true so MCP agents browse
// the catalog without blowing their context budget.
//
// The endpoints are global (no workspace), so we don't read
// `workspace` from input. Both endpoints require an authenticated
// user; the route table-level Apply hook handles that uniformly.
func (d *HTTPHandlerDispatcher) dispatchLibraryList(
ctx context.Context,
input map[string]any,
user *models.User,
) (*mcp.CallToolResult, error) {
const cmdKey = "library list"
typ, _ := input["type"].(string)
typ = strings.ToLower(strings.TrimSpace(typ))
wantConventions := typ == "" || typ == "conventions"
wantPlaybooks := typ == "" || typ == "playbooks"
if !wantConventions && !wantPlaybooks {
return validationFailedResult(cmdKey,
fmt.Sprintf("unknown --type %q", typ),
"Pass `type=conventions`, `type=playbooks`, or omit for both."), nil
}
category, _ := input["category"].(string)
full, _ := input["full"].(bool)
// Build query strings per endpoint. Convention endpoint takes only
// category; playbook endpoint takes both category and summary.
conventionQuery := url.Values{}
playbookQuery := url.Values{}
if category != "" {
conventionQuery.Set("category", category)
playbookQuery.Set("category", category)
}
if !full {
// Default: summary mode for playbooks. MCP callers want compact
// payloads; CLI default already does this too. Opt back in via
// full=true.
playbookQuery.Set("summary", "true")
}
conventionPath := "/api/v1/convention-library"
if encoded := conventionQuery.Encode(); encoded != "" {
conventionPath += "?" + encoded
}
playbookPath := "/api/v1/playbook-library"
if encoded := playbookQuery.Encode(); encoded != "" {
playbookPath += "?" + encoded
}
var conventions any
var playbooks any
if wantConventions {
v, errRes := d.fetchLibraryEndpoint(ctx, user, cmdKey, conventionPath)
if errRes != nil {
return errRes, nil
}
conventions = v
}
if wantPlaybooks {
v, errRes := d.fetchLibraryEndpoint(ctx, user, cmdKey, playbookPath)
if errRes != nil {
return errRes, nil
}
playbooks = v
}
// Single-type mode returns the library payload directly (matches
// the CLI). Both-types mode wraps in {conventions, playbooks}.
switch {
case wantConventions && wantPlaybooks:
return packageStructuredResponse(cmdKey, map[string]any{
"conventions": conventions,
"playbooks": playbooks,
})
case wantConventions:
return packageStructuredResponse(cmdKey, conventions)
default:
return packageStructuredResponse(cmdKey, playbooks)
}
}
// fetchLibraryEndpoint GETs one of the library endpoints and decodes
// the JSON body into a generic any so the caller can stuff it into
// the composed response without losing the wire shape.
func (d *HTTPHandlerDispatcher) fetchLibraryEndpoint(
ctx context.Context,
user *models.User,
cmdKey, path string,
) (any, *mcp.CallToolResult) {
req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
if err != nil {
return nil, dispatcherErrorResult(cmdKey, "build "+path, err)
}
rec := httptest.NewRecorder()
d.Handler.ServeHTTP(rec, req)
if rec.Code >= 400 {
return nil, upstreamHTTPErrorResult(ctx, cmdKey, "fetch "+path, path,
rec.Code, rec.Body.Bytes(), d.Lister, ResourceListing, "")
}
var decoded any
if err := json.Unmarshal(rec.Body.Bytes(), &decoded); err != nil {
return nil, dispatcherErrorResult(cmdKey, "parse "+path, err)
}
return decoded, nil
}