From 0f1069e9df011ce10fd8d829ec699c8aa73160ca Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 27 Sep 2026 07:59:11 -0400 Subject: [PATCH] feat(comments): agents can edit and delete comments from the CLI and every MCP transport (TASK-2695) (#1607) Web users could edit and delete comments since PLAN-1662. Agents, who write most comments, could only append corrections. - Server: new item-scoped routes, PATCH/DELETE /workspaces/{ws}/items/{ref}/comments/{id}. They answer 404 unless the comment is on {ref}, then run the existing handlers unchanged (edit is author-only, delete needs item-edit). GET /server/capabilities advertises them as item_scoped_comment_writes. - CLI: `pad item comment-edit [message|--stdin]` and `pad item comment-delete `. Both refuse to send to a server without the capability, whose bare 404 would otherwise read as "comment not found". `pad item comments` now prints each comment's id, plus "edited" once its body has changed. - models.Comment serialises a derived `edited` bool, true when updated_at > created_at (the web timeline's rule). - MCP 0.57: pad_item edit-comment / delete-comment and a comment_id param, on stdio, remote /mcp and WebMCP. All three use the item-scoped routes, so the check that the comment is on ref happens on the server for every transport. Remote packages a delete's 204 as the CLI's JSON. Found along the way, and out of scope: deleting a comment that has replies answers 500, because parent_id has no ON DELETE. Filed as BUG-3252. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 --- CLAUDE.md | 7 +- README.md | 6 +- cmd/pad/cmd_item.go | 107 +++++++++++- cmd/pad/comment_edit_delete_test.go | 98 +++++++++++ cmd/pad/groups.go | 2 + internal/cli/client.go | 32 +++- internal/cli/format.go | 6 +- internal/mcp/catalog_item.go | 16 +- internal/mcp/catalog_item_fields_test.go | 3 +- internal/mcp/catalog_readonly_test.go | 103 +++++++----- internal/mcp/dispatch_http.go | 1 + internal/mcp/dispatch_http_parity_test.go | 1 + internal/mcp/dispatch_http_routes.go | 64 ++++++++ internal/mcp/instructions.md | 4 +- .../mcp/task2695_comment_edit_delete_test.go | 153 ++++++++++++++++++ internal/mcp/version.go | 20 ++- internal/models/comment.go | 27 +++- internal/models/comment_test.go | 47 ++++++ internal/server/handlers_capabilities.go | 8 +- internal/server/handlers_comments.go | 40 +++++ .../handlers_comments_item_scoped_test.go | 142 ++++++++++++++++ internal/server/server.go | 2 + skills/pad/SKILL.md | 4 +- web/src/lib/api/client.ts | 15 ++ web/src/lib/types/index.ts | 2 + web/src/lib/webmcp/dispatch.test.ts | 41 ++++- web/src/lib/webmcp/dispatch.ts | 12 ++ 27 files changed, 897 insertions(+), 66 deletions(-) create mode 100644 cmd/pad/comment_edit_delete_test.go create mode 100644 internal/mcp/task2695_comment_edit_delete_test.go create mode 100644 internal/models/comment_test.go create mode 100644 internal/server/handlers_comments_item_scoped_test.go diff --git a/CLAUDE.md b/CLAUDE.md index c3c8c069e..142dca169 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,7 @@ REST API at `/api/v1/`. Key endpoints: - `GET /workspaces/{ws}/dashboard` — computed project overview (active items, plans, attention, blockers). Also carries `pending_reminders`: fired-but-unacknowledged reminders, which is the delivery path on any instance with no webhook configured. With a decision provider, `attention` also carries `needs_human` entries (the item's latest `needs_human_decision` answer is >= 0.7), and a text-derived `blocked` entry for an item whose `blocked` answer is >= 0.7 and which the dependency graph has NOT already flagged — the graph stays primary. Only CURRENT answers count, the same rule the decisions endpoint applies (the item's present state, the question as registered now, the model pinned now), so an item edited or commented since its last evaluation drops off until the tick re-asks. The state check costs one item-plus-trail read per ABOVE-threshold item, never per workspace item. A failing provider (any owed `attention` job with a recorded error) marks the section `attention.decisions` in `degraded_sections`. Text-derived `blocked` entries therefore also reach `pad project stale` (which filters on type); `needs_human` does not - `GET /workspaces/{ws}/activity` — workspace activity feed (enriched with item titles + change details) - `GET/POST/DELETE /workspaces/{ws}/webhooks` — webhook management +- `PATCH/DELETE /workspaces/{ws}/items/{slug}/comments/{id}` — comment edit/delete addressed through its item (TASK-2695): 404 unless the comment is on that item, then the same handler, ACL and events as `/workspaces/{ws}/comments/{id}` (edit author-only, delete item-edit). The CLI and every MCP transport use this form; `GET /server/capabilities` advertises it as `item_scoped_comment_writes`. Every serialised comment carries a derived `edited` bool - `GET /workspaces/{ws}/items/{slug}/children` — child items linked to a parent - `GET /workspaces/{ws}/items/{slug}/progress` — child item completion progress - `GET/POST /workspaces/{ws}/items/{slug}/links` — item relationships (blocks/blocked-by, parent/child) @@ -316,7 +317,7 @@ Collection names accept singular forms: `task`→`tasks`, `idea`→`ideas`, `doc ## MCP server -Pad runs as a local Model Context Protocol server so Claude Desktop / Cursor / Windsurf can call non-interactive `pad` commands as tools. The tool surface is a **hand-curated catalog** (currently v0.56) in `internal/mcp/catalog_*.go` — one ToolDef per resource (`pad_item`, `pad_workspace`, `pad_collection`, `pad_project`, `pad_role`, `pad_search`, `pad_meta`, `pad_playbook`, `pad_library`, `pad_attachment`) with an `action` enum dispatching to underlying CLI commands. v0.33 (PLAN-2857 U4 / TASK-2999) adds a **`multi_relation`** field type — an ORDERED LIST of references, each element resolving through the same UUID→ref→exact-title ladder a scalar `relation` uses, declared through the DSL as `owners:multi_relation:people` (third part = target collection; a bare `owners:multi_relation` is refused at parse time rather than minting a field no write can satisfy). The TYPE is additive — every rule is reachable only through a field whose schema says `multi_relation`, which no existing schema can contain — so the bump is owed by the READ shape: `relation_targets[key]` now carries EITHER the scalar object v0.31 emitted OR a JSON **ARRAY** of those objects, in stored order and one per stored element (an `id`-only entry for an element that names nothing, so a position lines up with the same position in `fields`). A scalar entry is byte-identical to v0.31's — pinned against a literal, not against another call into the same code — so a consumer with no such fields sees nothing new, while one that acquires such a field must handle the array. ONE map, not a parallel `multi_relation_targets` key: the field's type already says which shape to expect. Value rules, all NEW rather than inherited because scalar `relation` had nothing coherent to inherit (see BUG-3028 — a required scalar relation WAS satisfied by `""`, with three stored spellings of "no target", until v0.40 converged it on this same key-absent rule): exactly ONE stored form for none, the key ABSENT, with `[]` normalising to it at the write door; an empty or whitespace-only ELEMENT REFUSED rather than skipped, naming its index; `required` meaning >=1 RESOLVED element; order part of the value; duplicates REFUSED with a new `duplicate_referent` reason, detected AFTER resolution because two elements naming one item are usually different strings. Any failing element refuses the WHOLE write — never a partial list — and a cross-workspace copy drops the value whole, reported once, so an import can never change an element count. **0.32 is RESERVED for PR #1337**, which claims it and was opened first; this unit took 0.33 rather than contest the number, so 0.32 may be skipped if #1337 is abandoned — a gap is cheaper than a collision resolved under merge pressure. v0.31 (PLAN-2857 U6 / TASK-2996) lets a `relation` value be an EXACT TITLE, scoped to the collection that field declares, alongside the UUID and ref v0.29 accepted; adds a `relation_targets` member to reads hydrating each stored id to `{id, ref, title}`; and makes the `fields` DSL take the target collection slug as a relation's third part. The first two are additive — a title is a THIRD spelling, and `relation_targets` is `omitempty` — so the bump is owed by the DSL: `fields="owner:relation"` used to be ACCEPTED and is now REFUSED, because the old parser put the third part into `Options` for every type and therefore built a relation with NO target, a field every subsequent write refused with `target_missing`. Scoping is the point: a title unique only workspace-wide refuses and names the collection searched, and two matches inside the declared collection get their own `ambiguous` reason rather than `not_found`, which would state the opposite of what happened. The ladder is UUID, then ref, then title, so an item literally TITLED `COLO-3` is unreachable by title while the ref resolves. An id-only `relation_targets` entry means the target is GONE **or** the caller may not SEE it — the two are made indistinguishable deliberately, matching the write side's `wrong_collection`→`not_found` collapse, so no consumer may render it as either. v0.30 (BUG-2870) makes one `--field key=value` entry mean ONE thing at every door. Six sites parsed that entry independently — `item create`, `item list`, `item update`, `item move` and `item copy` in `cmd/pad`, plus `ingestFieldKVP` on the remote door — in four spellings, so `field:[" effort=l"]` stored an undeclared field literally named `" effort"` through the CLI while the remote door trimmed and wrote `effort`: the same call storing two different keys, decided by the transport. All six now call `items.SplitFieldEntry`, under two deliberately asymmetric rules — a padded KEY is REFUSED everywhere (trimming silently retargets the write to a field the caller did not type), a VALUE is carried VERBATIM everywhere (trimming reinterprets a caller's bytes, and on a text field the padding is content). The catalog's conflict pass is re-grounded on the same change, since its rules were derived from that trimming: comparisons are RAW, a padded entry is refused in the pass rather than skipped, `parseFieldArray`'s refusal is PROPAGATED rather than swallowed (it had no second owner on the no-`fields` path, where four existing refusals were landing as successes), and canonicalization and the re-emission path are gone — nothing rewrites a caller's key any more. v0.29 (PLAN-2857 / TASK-2878) makes a `relation` field value have to NAME A LIVE ITEM in the collection that field declares. `internal/items` only ever checked the SHAPE of a relation ("must be a string") because that package is DB-free, so any string at all was accepted and stored and no client could render it honestly. Every write door now refuses a value that names nothing, names an item in the WRONG collection, sits in a field declaring no target collection, or is a SLUG (deliberate divergence from `ResolveItem` — a slug is neither an ID nor stable). A CARRIED value, already on the item and asserted by nobody, is never refused: within a workspace it resolves and survives; across a boundary it is dropped without a lookup and reported in `warnings.dropped_fields`. v0.28 (IDEA-2641) adds two ADDITIVE `pad_item` actions — `remind` (arm a one-shot reminder at an RFC3339 `remind_at` instant) and `ack-reminder` (acknowledge a fired one by `reminder_id`). Agents already RECEIVED reminders, since the poll surface is `pad_project.next` / `ready`; what was missing is the other half — deferring work is exactly when an agent knows it wants to be asked again. A bare date is refused rather than read as midnight. Re-arm and disarm stay CLI-only until a listing action exists to discover an id. v0.27 (BUG-2850) typed field values server-side, carried the `fields` object with its JSON types intact, named undeclared keys back in `warnings.undeclared_fields`, and replaced the per-site conflict guards with one check that refuses two names for one target in a single call. v0.26 (IDEA-2756) makes `pad_workspace.create` REFUSE with a 403 when the calling OAuth connection's grant carries `may_create_workspaces=false` — that consent checkbox previously gated only the post-creation auto-add, so a connection whose user declined it created workspaces anyway — invisible to it when the connection carried an explicit allow-list, visible when it carried the `all_current_workspaces` wildcard; the consent mismatch is the defect in both cases. The same gate covers `POST /workspaces/import` (a second door onto `store.ImportWorkspace` → `CreateWorkspace`, with no MCP action today). No escape-hatch param, deliberately: the gate expresses the USER's consent decision, so only the user can lift it — by re-authorizing, or by enabling the flag on the existing connection at `/console/connected-apps`. v0.25 (TASK-2657 / BUG-2702) makes `pad_library.activate` resolve its destination collection from the target's declared artifact kind rather than the literal `conventions` / `playbooks` slugs, and surfaces a lookup ERROR instead of falling back. v0.24 (#1066) makes the `pad_item` `fields` OBJECT a real write form on create/update — reads return `fields` as a native object (BUG-991 normalization), and writing that shape back was a silent no-op: not a declared param, no `additionalProperties`, so it was accepted, never mapped by `BuildCLIArgs`, and dropped while the PATCH still bumped `updated_at`. The alias merges into the same path as `field: ["key=value"]` / the dedicated params (`catalog_item_fields.go`), refusing the same key in two places with conflicting values; and input validation is now STRICT across all catalog tools — an undeclared top-level key fails with a structured `validation_failed` naming it, instead of being silently dropped (a small documented compat list survives: pad_item's v0.16 `assigned_user_id` / `agent_role_id` remote clear form). One bump covers both halves — they are one contract change. v0.23 (BUG-2627 part 2 + BUG-2675, PR #1166) refuses raw `field` setters naming system-metadata keys in `fields_patch` on every transport (`github_pr` exempt on UPDATE only — the sole remote writer, itself broken — until v0.47 closed it: BUG-2696) and adds the retry-hostile `stored_state_unreadable` error code. v0.22 (BUG-2674, PR #1165) makes reserved metadata survive a move and refuses `field` setters naming those keys on move/copy — see `internal/mcp/version.go` for both full entries. v0.21 (BUG-2608) bounds `pad_item.action=history`, which was unbounded on every surface: the `limit` param now covers it (default 50, max 300 — the NEWEST N versions, with no `offset`, because reverse-patch storage makes only a newest-end window cheap to reconstruct), applied in the CATALOG action so it lands on both transports, and summary mode now asks the server to skip patch resolution (`?summary=true`) instead of resolving every body and discarding it. Additive param bump — `limit` already existed and nothing changed shape. v0.20 (BUG-2302 + BUG-2305, one bump) adds explicit MCP tool annotations (`readOnlyHint`/`destructiveHint`/`idempotentHint` derived from the catalog's own write-shape knowledge, fixing read-only tools that advertised `destructiveHint:true`) and makes `pad_item.list` summary-shaped on the REMOTE /mcp transport too (the hand-written `dispatchItemList` projects via `cli.ToItemSummaries`; `full=true` opts back into complete bodies) — see `internal/mcp/version.go` for the authoritative per-version changelog. Post-0.20 without a bump (BUG-2304): `item backlinks` / `item history` / `project report` gained HTTP route coverage — they were advertised but answered "not yet implemented over HTTP transport" — and a catalog↔route parity test (`dispatch_http_parity_test.go`) now drives every catalog action and fails on any future advertised-but-unrouted action; no names, enums, or shapes changed, hence no bump. v0.19 adds a `clear_parent` boolean to `pad_item` — the canonical, schema-discoverable way to detach an item from its parent, backed by a new `--clear-parent` bareword flag on `pad item update` (BUG-2078). v0.18 adds `clear_assigned_user` / `clear_agent_role` booleans to `pad_item` — the canonical, schema-discoverable way to unassign, backed by new `--clear-assigned-user` / `--clear-agent-role` bareword flags on `pad item update` (IDEA-2584). Update-only, deliberately asymmetric with create. v0.17 carries the empty-string clear to the LOCAL STDIO transport, which shells out to the CLI — `cmd/pad/cmd_item.go` now lifts `assigned_user_id` / `agent_role_id` onto their columns instead of into the fields blob, on create and update (BUG-2583). v0.16 makes an empty-string `assigned_user_id` / `agent_role_id` CLEAR the assignment instead of being silently dropped, so an MCP agent can finally unassign an item (TASK-2571). v0.15 adds the `pad_item.list` `unparented` boolean, mutually exclusive with `parent`, for items with no parent or implements relationship (TASK-2096). v0.2 introduced the catalog (PLAN-969 / TASK-981); v0.3 added `pad_playbook`, `pad_meta.action: bootstrap`, `pad_set_workspace`'s embedded-bootstrap response, and the `pad://workspace/{ws}/bootstrap` resource (PLAN-1377 / TASK-1380); v0.4 trimmed the bootstrap payload by ~40% (PLAN-1410) — slim `BootstrapCollection` + `BootstrapRole` projections (no UUIDs/timestamps/settings; nested `schema` object; redundant labels omitted), removed top-level `recent_activity` duplicate, dropped convention `slug`, and added a `BootstrapDashboard` wrapper that caps five sub-arrays (`attention`, `recent_activity`, `active_items`, `active_plans`, `by_role`) at 5 entries each with parallel `*_overflow_count` fields. The pre-catalog v0.1 cmdhelp leaf walker is retired. +Pad runs as a local Model Context Protocol server so Claude Desktop / Cursor / Windsurf can call non-interactive `pad` commands as tools. The tool surface is a **hand-curated catalog** (currently v0.57) in `internal/mcp/catalog_*.go` — one ToolDef per resource (`pad_item`, `pad_workspace`, `pad_collection`, `pad_project`, `pad_role`, `pad_search`, `pad_meta`, `pad_playbook`, `pad_library`, `pad_attachment`) with an `action` enum dispatching to underlying CLI commands. v0.33 (PLAN-2857 U4 / TASK-2999) adds a **`multi_relation`** field type — an ORDERED LIST of references, each element resolving through the same UUID→ref→exact-title ladder a scalar `relation` uses, declared through the DSL as `owners:multi_relation:people` (third part = target collection; a bare `owners:multi_relation` is refused at parse time rather than minting a field no write can satisfy). The TYPE is additive — every rule is reachable only through a field whose schema says `multi_relation`, which no existing schema can contain — so the bump is owed by the READ shape: `relation_targets[key]` now carries EITHER the scalar object v0.31 emitted OR a JSON **ARRAY** of those objects, in stored order and one per stored element (an `id`-only entry for an element that names nothing, so a position lines up with the same position in `fields`). A scalar entry is byte-identical to v0.31's — pinned against a literal, not against another call into the same code — so a consumer with no such fields sees nothing new, while one that acquires such a field must handle the array. ONE map, not a parallel `multi_relation_targets` key: the field's type already says which shape to expect. Value rules, all NEW rather than inherited because scalar `relation` had nothing coherent to inherit (see BUG-3028 — a required scalar relation WAS satisfied by `""`, with three stored spellings of "no target", until v0.40 converged it on this same key-absent rule): exactly ONE stored form for none, the key ABSENT, with `[]` normalising to it at the write door; an empty or whitespace-only ELEMENT REFUSED rather than skipped, naming its index; `required` meaning >=1 RESOLVED element; order part of the value; duplicates REFUSED with a new `duplicate_referent` reason, detected AFTER resolution because two elements naming one item are usually different strings. Any failing element refuses the WHOLE write — never a partial list — and a cross-workspace copy drops the value whole, reported once, so an import can never change an element count. **0.32 is RESERVED for PR #1337**, which claims it and was opened first; this unit took 0.33 rather than contest the number, so 0.32 may be skipped if #1337 is abandoned — a gap is cheaper than a collision resolved under merge pressure. v0.31 (PLAN-2857 U6 / TASK-2996) lets a `relation` value be an EXACT TITLE, scoped to the collection that field declares, alongside the UUID and ref v0.29 accepted; adds a `relation_targets` member to reads hydrating each stored id to `{id, ref, title}`; and makes the `fields` DSL take the target collection slug as a relation's third part. The first two are additive — a title is a THIRD spelling, and `relation_targets` is `omitempty` — so the bump is owed by the DSL: `fields="owner:relation"` used to be ACCEPTED and is now REFUSED, because the old parser put the third part into `Options` for every type and therefore built a relation with NO target, a field every subsequent write refused with `target_missing`. Scoping is the point: a title unique only workspace-wide refuses and names the collection searched, and two matches inside the declared collection get their own `ambiguous` reason rather than `not_found`, which would state the opposite of what happened. The ladder is UUID, then ref, then title, so an item literally TITLED `COLO-3` is unreachable by title while the ref resolves. An id-only `relation_targets` entry means the target is GONE **or** the caller may not SEE it — the two are made indistinguishable deliberately, matching the write side's `wrong_collection`→`not_found` collapse, so no consumer may render it as either. v0.30 (BUG-2870) makes one `--field key=value` entry mean ONE thing at every door. Six sites parsed that entry independently — `item create`, `item list`, `item update`, `item move` and `item copy` in `cmd/pad`, plus `ingestFieldKVP` on the remote door — in four spellings, so `field:[" effort=l"]` stored an undeclared field literally named `" effort"` through the CLI while the remote door trimmed and wrote `effort`: the same call storing two different keys, decided by the transport. All six now call `items.SplitFieldEntry`, under two deliberately asymmetric rules — a padded KEY is REFUSED everywhere (trimming silently retargets the write to a field the caller did not type), a VALUE is carried VERBATIM everywhere (trimming reinterprets a caller's bytes, and on a text field the padding is content). The catalog's conflict pass is re-grounded on the same change, since its rules were derived from that trimming: comparisons are RAW, a padded entry is refused in the pass rather than skipped, `parseFieldArray`'s refusal is PROPAGATED rather than swallowed (it had no second owner on the no-`fields` path, where four existing refusals were landing as successes), and canonicalization and the re-emission path are gone — nothing rewrites a caller's key any more. v0.29 (PLAN-2857 / TASK-2878) makes a `relation` field value have to NAME A LIVE ITEM in the collection that field declares. `internal/items` only ever checked the SHAPE of a relation ("must be a string") because that package is DB-free, so any string at all was accepted and stored and no client could render it honestly. Every write door now refuses a value that names nothing, names an item in the WRONG collection, sits in a field declaring no target collection, or is a SLUG (deliberate divergence from `ResolveItem` — a slug is neither an ID nor stable). A CARRIED value, already on the item and asserted by nobody, is never refused: within a workspace it resolves and survives; across a boundary it is dropped without a lookup and reported in `warnings.dropped_fields`. v0.28 (IDEA-2641) adds two ADDITIVE `pad_item` actions — `remind` (arm a one-shot reminder at an RFC3339 `remind_at` instant) and `ack-reminder` (acknowledge a fired one by `reminder_id`). Agents already RECEIVED reminders, since the poll surface is `pad_project.next` / `ready`; what was missing is the other half — deferring work is exactly when an agent knows it wants to be asked again. A bare date is refused rather than read as midnight. Re-arm and disarm stay CLI-only until a listing action exists to discover an id. v0.27 (BUG-2850) typed field values server-side, carried the `fields` object with its JSON types intact, named undeclared keys back in `warnings.undeclared_fields`, and replaced the per-site conflict guards with one check that refuses two names for one target in a single call. v0.26 (IDEA-2756) makes `pad_workspace.create` REFUSE with a 403 when the calling OAuth connection's grant carries `may_create_workspaces=false` — that consent checkbox previously gated only the post-creation auto-add, so a connection whose user declined it created workspaces anyway — invisible to it when the connection carried an explicit allow-list, visible when it carried the `all_current_workspaces` wildcard; the consent mismatch is the defect in both cases. The same gate covers `POST /workspaces/import` (a second door onto `store.ImportWorkspace` → `CreateWorkspace`, with no MCP action today). No escape-hatch param, deliberately: the gate expresses the USER's consent decision, so only the user can lift it — by re-authorizing, or by enabling the flag on the existing connection at `/console/connected-apps`. v0.25 (TASK-2657 / BUG-2702) makes `pad_library.activate` resolve its destination collection from the target's declared artifact kind rather than the literal `conventions` / `playbooks` slugs, and surfaces a lookup ERROR instead of falling back. v0.24 (#1066) makes the `pad_item` `fields` OBJECT a real write form on create/update — reads return `fields` as a native object (BUG-991 normalization), and writing that shape back was a silent no-op: not a declared param, no `additionalProperties`, so it was accepted, never mapped by `BuildCLIArgs`, and dropped while the PATCH still bumped `updated_at`. The alias merges into the same path as `field: ["key=value"]` / the dedicated params (`catalog_item_fields.go`), refusing the same key in two places with conflicting values; and input validation is now STRICT across all catalog tools — an undeclared top-level key fails with a structured `validation_failed` naming it, instead of being silently dropped (a small documented compat list survives: pad_item's v0.16 `assigned_user_id` / `agent_role_id` remote clear form). One bump covers both halves — they are one contract change. v0.23 (BUG-2627 part 2 + BUG-2675, PR #1166) refuses raw `field` setters naming system-metadata keys in `fields_patch` on every transport (`github_pr` exempt on UPDATE only — the sole remote writer, itself broken — until v0.47 closed it: BUG-2696) and adds the retry-hostile `stored_state_unreadable` error code. v0.22 (BUG-2674, PR #1165) makes reserved metadata survive a move and refuses `field` setters naming those keys on move/copy — see `internal/mcp/version.go` for both full entries. v0.21 (BUG-2608) bounds `pad_item.action=history`, which was unbounded on every surface: the `limit` param now covers it (default 50, max 300 — the NEWEST N versions, with no `offset`, because reverse-patch storage makes only a newest-end window cheap to reconstruct), applied in the CATALOG action so it lands on both transports, and summary mode now asks the server to skip patch resolution (`?summary=true`) instead of resolving every body and discarding it. Additive param bump — `limit` already existed and nothing changed shape. v0.20 (BUG-2302 + BUG-2305, one bump) adds explicit MCP tool annotations (`readOnlyHint`/`destructiveHint`/`idempotentHint` derived from the catalog's own write-shape knowledge, fixing read-only tools that advertised `destructiveHint:true`) and makes `pad_item.list` summary-shaped on the REMOTE /mcp transport too (the hand-written `dispatchItemList` projects via `cli.ToItemSummaries`; `full=true` opts back into complete bodies) — see `internal/mcp/version.go` for the authoritative per-version changelog. Post-0.20 without a bump (BUG-2304): `item backlinks` / `item history` / `project report` gained HTTP route coverage — they were advertised but answered "not yet implemented over HTTP transport" — and a catalog↔route parity test (`dispatch_http_parity_test.go`) now drives every catalog action and fails on any future advertised-but-unrouted action; no names, enums, or shapes changed, hence no bump. v0.19 adds a `clear_parent` boolean to `pad_item` — the canonical, schema-discoverable way to detach an item from its parent, backed by a new `--clear-parent` bareword flag on `pad item update` (BUG-2078). v0.18 adds `clear_assigned_user` / `clear_agent_role` booleans to `pad_item` — the canonical, schema-discoverable way to unassign, backed by new `--clear-assigned-user` / `--clear-agent-role` bareword flags on `pad item update` (IDEA-2584). Update-only, deliberately asymmetric with create. v0.17 carries the empty-string clear to the LOCAL STDIO transport, which shells out to the CLI — `cmd/pad/cmd_item.go` now lifts `assigned_user_id` / `agent_role_id` onto their columns instead of into the fields blob, on create and update (BUG-2583). v0.16 makes an empty-string `assigned_user_id` / `agent_role_id` CLEAR the assignment instead of being silently dropped, so an MCP agent can finally unassign an item (TASK-2571). v0.15 adds the `pad_item.list` `unparented` boolean, mutually exclusive with `parent`, for items with no parent or implements relationship (TASK-2096). v0.2 introduced the catalog (PLAN-969 / TASK-981); v0.3 added `pad_playbook`, `pad_meta.action: bootstrap`, `pad_set_workspace`'s embedded-bootstrap response, and the `pad://workspace/{ws}/bootstrap` resource (PLAN-1377 / TASK-1380); v0.4 trimmed the bootstrap payload by ~40% (PLAN-1410) — slim `BootstrapCollection` + `BootstrapRole` projections (no UUIDs/timestamps/settings; nested `schema` object; redundant labels omitted), removed top-level `recent_activity` duplicate, dropped convention `slug`, and added a `BootstrapDashboard` wrapper that caps five sub-arrays (`attention`, `recent_activity`, `active_items`, `active_plans`, `by_role`) at 5 entries each with parallel `*_overflow_count` fields. The pre-catalog v0.1 cmdhelp leaf walker is retired. cmdhelp is still consumed at dispatch time — `BuildCLIArgs` reads individual command schemas to translate the catalog's snake_case input map into CLI args. cmdhelp no longer drives tool naming or count. @@ -330,7 +331,7 @@ pad mcp status # Install state across supported clients ``` Surface: -- **Tools:** the v0.56 catalog — ten resource × action tools (`pad_item`, `pad_workspace`, `pad_collection`, `pad_project`, `pad_role`, `pad_search`, `pad_meta`, `pad_playbook`, `pad_library`, `pad_attachment`) plus `pad_set_workspace` (takes a `workspace` slug only — no action enum). The ten resource × action tools take `action: ` to choose what they do. `pad_item` (v0.19) exposes `clear_parent` as the canonical parent-detach (update only); (v0.18) exposes `clear_assigned_user` / `clear_agent_role` booleans as the canonical unassign (update only); (v0.17) treats an empty-string `assigned_user_id` / `agent_role_id` as a clear on BOTH transports via `field: ["assigned_user_id="]` — the direct param form is remote-only, since it isn't schema-declared and stdio's BuildCLIArgs drops unknown keys (IDEA-2584); v0.16 fixed remote only; (v0.15) adds the `unparented` list parameter; v0.14 added `history` + `expected_updated_at`. `pad_project` (v0.13) adds `ready` (actionable backlog) + `stale` (items needing attention); `pad_project.activity` (v0.12) is the non-streaming, bounded activity feed — catch up on what other agents/users changed since you last worked. `pad_attachment` is the read-only attachment-metadata surface — `list`/`show` (upload/download/view stay CLI-only). `pad_library` is the convention+playbook library surface — `list`/`get`/`activate`. `pad_playbook` is the playbook surface from PLAN-1377 — `list`/`get`/`run` mirror the CLI's `pad playbook` subcommands; `run` is side-effect-free and returns the body + bound args for the agent to execute. v0.4 (PLAN-1410) didn't change the tool/action surface; it trimmed the bootstrap JSON those tools/resources return — see the Stability contract subsection below for details. +- **Tools:** the v0.57 catalog — ten resource × action tools (`pad_item`, `pad_workspace`, `pad_collection`, `pad_project`, `pad_role`, `pad_search`, `pad_meta`, `pad_playbook`, `pad_library`, `pad_attachment`) plus `pad_set_workspace` (takes a `workspace` slug only — no action enum). The ten resource × action tools take `action: ` to choose what they do. `pad_item` (v0.19) exposes `clear_parent` as the canonical parent-detach (update only); (v0.18) exposes `clear_assigned_user` / `clear_agent_role` booleans as the canonical unassign (update only); (v0.17) treats an empty-string `assigned_user_id` / `agent_role_id` as a clear on BOTH transports via `field: ["assigned_user_id="]` — the direct param form is remote-only, since it isn't schema-declared and stdio's BuildCLIArgs drops unknown keys (IDEA-2584); v0.16 fixed remote only; (v0.15) adds the `unparented` list parameter; v0.14 added `history` + `expected_updated_at`. `pad_project` (v0.13) adds `ready` (actionable backlog) + `stale` (items needing attention); `pad_project.activity` (v0.12) is the non-streaming, bounded activity feed — catch up on what other agents/users changed since you last worked. `pad_attachment` is the read-only attachment-metadata surface — `list`/`show` (upload/download/view stay CLI-only). `pad_library` is the convention+playbook library surface — `list`/`get`/`activate`. `pad_playbook` is the playbook surface from PLAN-1377 — `list`/`get`/`run` mirror the CLI's `pad playbook` subcommands; `run` is side-effect-free and returns the body + bound args for the agent to execute. v0.4 (PLAN-1410) didn't change the tool/action surface; it trimmed the bootstrap JSON those tools/resources return — see the Stability contract subsection below for details. - **Resources:** `pad://workspace/{ws}/items/{ref}`, `pad://workspace/{ws}/items`, `pad://workspace/{ws}/dashboard`, `pad://workspace/{ws}/collections`, `pad://workspace/{ws}/attachments/{id}` (bounded base64 image via `thumb-md`; non-images and image bytes over 1 MiB (pre-base64) rejected), `pad://workspace/{ws}/bootstrap` (one-shot workspace overview — user + collections + always-on conventions + roles + playbook metadata + dashboard + recent activity), plus the server-wide `pad://_meta/version`. - **Prompts:** `pad_plan`, `pad_ideate`, `pad_retro`, `pad_onboard` — multi-step workflows lifted from `skills/pad/SKILL.md`. @@ -338,7 +339,7 @@ Surface: **Stability contract.** Two version constants live in `internal/mcp/version.go`, advertised in the handshake under `capabilities.experimental.padCmdhelp` and `capabilities.experimental.padToolSurface`: - `CmdhelpVersion` (currently `"0.1"`) — the cmdhelp CLI help-tree contract. Bump when CLI flag/arg schemas change incompatibly. -- `ToolSurfaceVersion` (currently `"0.56"`) — the MCP tool catalog contract. Bump when tool names, action enums, or parameter shapes change incompatibly. **v0.56** (BUG-3244) gives `content_state` a second value, `superseded_set_aside`, on every MCP door that carries it (`pad_item` get / full list, `pad_playbook` run, the item resource's line above the body): edits an editor schema-version rebuild set aside, which opening the item will NOT restore, so the resource line and the `content_pending_flush` hint (chosen by `details.set_aside_rows`) name `pad item set-aside` and `overwrite_pending_edits` instead. ADDITIVE bump: no name, enum or param changed, but an agent branching on the one-word vocabulary meets a word whose remedy differs. Reading or discarding the rows alone is not on the catalog. **v0.55** (BUG-2659) sends `pad_item.action=search`'s `collection` to /search as typed, and /search resolves it exact-first per workspace in scope (the item routes' resolver). In a workspace holding both `task` and `tasks`, `collection: "task"` now answers from `task` instead of `tasks`; every other workspace is unchanged, because the shorthand still resolves server-side. Stdio reaches it through the CLI, which sends the name as typed only to a server advertising `search_collection_resolution`. BEHAVIOR bump on the v0.36 / v0.40 grounds. **v0.54** (BUG-3217) keeps a `fields` number's literal through MCP on both transports (mcp-go's argument decode rounded anything above 2^53, and stdio's `--field` stringify carried the rounded value), and REFUSES a `fields` number beside a `field` entry for the same key when they differ as exact numbers — two values that differ only above 2^53 used to collapse into one rounded write; a matching pair (3.0 beside n=3) still collapses. BEHAVIOR bump on the v0.48 grounds. **v0.53** (BUG-2819) adds `filename_source` (caller / normalised / substituted / derived / unknown) to every `pad_attachment.action=list` row on both transports, because a stored filename cannot say whether the caller sent it or the server substituted it. ADDITIVE, the v0.28 / v0.13 disposition; `show` does not carry it, and `mcp_audit_log.tool_name_source` reaches no catalog action. Rows written before it read `unknown`, never `caller`. **v0.52** (BUG-2367 item 4) refuses a move or copy (every migrate door) that would change an item between open, done and abandoned unless the caller names the destination done field. Only the STATE is compared, so two values that both mean done carry. Move answers 400 `state_change_requires_value` with details `{field, options, from, to}`; bulk move puts it in a `failed[]` row; the copy answers 400 `validation_error` with the same sentence; the preflight emits a needs_value row with reason `state_change`, which the web dialog picker renders. A destination with no done select field is exempt. BEHAVIOR bump. **v0.51** (BUG-2367) stops every migrate door (single move, bulk move, cross-workspace copy and its preflight) carrying a value into a field the destination marks `computed` (preflight drop reason `target_computed`) or a value that collides on a destination `unique_scope` field. Provenance decides, the TASK-2878 rule: a CARRIED collision is dropped and reported (`not_unique`), a supplied override or injected default that collides is refused 409 `conflict`, and a required field emptied by the drop is refused as required, which is the existing needs_value route. Before it, single move answered 500 on the indexed `invocation_slug`, bulk move leaked the SQL error, and an unindexed unique field stored a duplicate. Reporting is additive and omitempty: `warnings.not_unique` [{key, value, holder?, message}] on the move response, bulk `updated[]` rows and the copy response, and a `detail` sentence on the preflight's dropped row. `holder` is present only when the caller may see that item, so the drop is not an existence oracle. BEHAVIOR bump on the v0.43 / v0.29 grounds. **v0.50** (BUG-3014) adds an ADDITIVE `omitempty` `stored_as_text: true` to a `relation_targets` entry whose stored value is not UUID-shaped (a title a bundle import carried verbatim, or legacy free text). Such a value was never an id and used to hydrate as a plain id-only entry, which reads as "gone or hidden"; an id-only entry without the flag keeps that meaning, and a UUID-shaped value, resolved or not, is byte-identical to v0.49. The predicate is syntactic, decided from the stored bytes and never from a lookup, so it discloses nothing, and deliberately not coupled to the resolver ladder. The CLI prints `"" (text, not a reference)` and the web chip says the same. **v0.49** (BUG-2410) makes `pad_project.action=report` with no `collections` param leave out SYSTEM collections (`Collection.IsSystem`: Conventions and Playbooks), which were counted in Insights' created, WIP and status totals; naming one explicitly still includes it. BEHAVIOR bump on the v0.40 / v0.42 grounds: a default call's content changed for every caller (web Insights, print, `pad project report`, both MCP transports, all through the one store rule). **v0.48** (BUG-3163) makes `pad_item.action=create` REFUSE a `field` / `fields` entry naming any reserved metadata key (`implementation_notes`, `decision_log`, `github_pr`, `convention`) on every transport, with 400 `validation_error`, creating nothing: create was the last door that stored one, as a string no extractor reads. Library activation, its one system writer, moves to an additive typed `convention` create member (not on the catalog; `pad_library.activate` is unchanged), and the CLI verifies from the stored fields that it landed, because an older server ignores the member and answers 201. BEHAVIOR bump on the v0.47 grounds. **v0.47** (BUG-2696) makes `pad_item.action=update` REFUSE a field setter naming `github_pr` on every transport (400 `validation_error` naming `pad github link` / `unlink`), closing the exemption v0.23 left: that door stored the PR as an unreadable string, so no link ever rendered. The writer is a new additive, typed update member (`github_pr` object + `clear_github_pr`) used by `pad github link` / `unlink` / `project reconcile`, deliberately NOT on the catalog (a remote PR-link action waits on the catalog-trim decision). BEHAVIOR bump; item create is BUG-3163. **v0.46** (BUG-3156) makes `pad_item.action=bulk-update` over stdio and remote REFUSE, per item, a status or priority change on an item whose collection does not declare that field, writing nothing for that item: the answer WebMCP has given since v0.44, so the three transports agree. Rows keep each transport's shape (stdio `failed[]` with `validation_error`, remote `results[]` with `validation_failed` and the server message in `hint`). The mechanism is the update door's new `refuse_undeclared_fields` member, which both bulk-update transports set; `pad_item.update` does not, and still accepts-and-warns. A server that predates the member ignores it, which is the old accept-and-warn. BEHAVIOR bump on the v0.44 / v0.43 grounds. **v0.45** (BUG-2829) reports an HTTP 413 as `too_large` on BOTH transports, with the server's own code in `details.reason`, where it used to be `server_error`: three deliberate caps are reachable from the catalog (a title rename's link cascade, an oversized outbox event, an oversized artifact import), and the old code read as transient and invited a retry that fails identically. Remote maps 413 in `classifyHTTPStatusKind`; stdio gets it from a structured marker the CLI root writes for any 413, and `too_large` joins `allowedStructuredErrorCodes`, which a test now proves contains every code any CLI marker writer emits. BEHAVIOR bump on the v0.42 grounds; no name, enum or param shape changed. **v0.44** (BUG-3154) makes `pad_item.action=bulk-update` over the WebMCP browser transport REFUSE, per item, a status or priority change on an item whose collection does not declare that field: the item lands in the bulk response's `failed` array with `validation_error` naming the collection and field, nothing is written, and the rest of the call applies. The door is `POST /items/bulk` (`set-priority`, and `move` with a `status`, with or without a `collection`, where the target's schema decides), where the request carries an operation and a value and the SERVER chooses the key; it used to store an orphan field and report the item updated. BEHAVIOR bump on the v0.43 / v0.40 grounds. The transports disagreed after it, stated rather than implied: stdio and remote issued a per-item PATCH, the single-item update door, which ACCEPTED the key with `warnings.undeclared_fields` (v0.27), until v0.46 converged them. **v0.43** (BUG-2379) makes `pad_item.action=move` REFUSE a field override naming a field the destination schema does not declare, with 400 `malformed_override` — the same check, code and message the cross-workspace copy already used. BEHAVIOR bump on the v0.40 / v0.29 grounds: a write door refuses a call it used to accept, where it used to store the key as an orphan field no schema renders. Create and update still ACCEPT undeclared keys with `warnings.undeclared_fields` (v0.27), because callers round-trip the whole fields blob; an override is a fresh assertion about the destination schema, not round-tripped state. **v0.42** (BUG-3142 + BUG-3147) changes the error CODE a LOCAL STDIO caller receives for two failure families that used to arrive as retryable `server_error`: an argv refusal (cobra/pflag: unknown flag, arg count, …) is now `validation_failed` with its message kept, and a 429 on any command is now `rate_limited` with `details.retry_after_seconds`, via the structured marker line the CLI writes at its root. It also stops the refusal that caused most of the first family: `BuildCLIArgs` now emits flags first and the positionals behind `--`, so free text starting with `-` (`item search "-ship it"`) is no longer parsed as a flag. BEHAVIOR bump; no name, enum or param shape changed, and the remote transport is untouched. **v0.41** (PLAN-3114 U5 / TASK-3120) adds a `match` action to `pad_playbook`: given free `text`, a Choice question over the workspace's ACTIVE playbooks plus a reserved `"none"` option returns the best match's ref (or `"none"`), confidence, per-option probabilities and the model. No provider configured → 404 `decision_provider_unavailable`; a provider error or an out-of-set answer → 502 `decision_provider_error`. Pure ADDITIVE bump (new action + param, nothing existing moved), read-only and side-effect-free. **v0.40** (BUG-3028) gives a scalar `relation` ONE stored form for "no target", the key ABSENT, converging on multi_relation's rule. A blank (`""` or whitespace) a write SETS — create, `fields_patch`, a changed value in full `fields`, a move/copy override — is normalised to absent before validation, so a `required` relation is refused as required (a patch blanking it now gets the answer deleting it always got). A legacy blank a write only CARRIES — an untouched key under `fields_patch` (normalised by the store under its write lock, via `ItemUpdate.BlankRelationKeys`), an unchanged value in full `fields`, move, copy, artifact import — is normalised after validation and never refused. An empty list filter on a scalar relation key of a collection-scoped list (`owner=`) matches all three spellings. BEHAVIOR bump; no shape change. Workspace import (`store.ImportWorkspace`) still stores blobs verbatim; its legacy blanks normalise on the next write. **v0.39** (BUG-3133) refuses `pad_item.update` that sets content WITH a version token (`expected_seq` / `expected_updated_at`) while the item's op-log holds content-bearing rows above its flush watermark, with 409 `content_pending_flush` (details `ref`, `pending_rows`). BEHAVIOR bump on the v0.35 / v0.36 grounds, carrying one additive param: `overwrite_pending_edits` lifts it, on the API, the CLI (`--overwrite-pending-edits`; `pad item edit --force` sets it on the save) and MCP alike, because an agent must be able to clear the refusal without a browser. The token guards the ROW and a tab's unflushed typing is not in it, so the write used to replace those edits — through the applier's setContent with a tab open, and by the direct path's op-log prune, which DELETES them, with none. Its own code rather than `update_conflict` because re-reading returns the same row and seq. A write without a token, without content, or to an item with nothing pending is unchanged, except that a direct write now reports `warnings.pruned_pending_edits` (additive, omitempty) when its prune deleted such rows. The check runs in the write's own tx; on the direct path it is under the item lock and appendMu and exact, on the applier path a tab can still type in the one server hop between the check and setContent. (v0.37 and v0.38 — the `agent` projection on `pad_item.get` and `claim` / `release` — are in `internal/mcp/version.go`; this paragraph had stopped at v0.36.) **v0.36** (BUG-3082) stops a REF-SHAPED `relation` value falling back to matching by item NUMBER when its prefix names a collection that is LIVE in the workspace; such a value is now `not_found`. BEHAVIOR bump on the v0.35 / v0.30 / v0.29 / v0.16 grounds — no tool name, action enum or param shape changed, but a write door refuses a value it used to accept. It finishes v0.29: that entry closed free-text corruption ("red" resolving to whatever is slugged `red` today) and left the same corruption open for anything shaped like a REF, which is the spelling the docs tell callers to prefer. Item numbers are workspace-unique and sequential ACROSS collections, so `CONVE-1` and `SECRE-1` are never both real — the fallback dropped the prefix, matched on the number alone, and landed on an item that was usually inside the field's declared collection, so the wrong-collection check above it passed and NOTHING was raised: a caller wrote `CONVE-1`, the item stored a different item's UUID, and the write answered 201 with the substitution visible only in `relation_targets`. The predicate is about the PREFIX rather than the item because a collection RENAME changes its prefix and a relation already written as the old ref must keep resolving (BUG-2873) — that ref's prefix names nothing afterwards, so the fallback still fires for it, while a prefix that IS live and simply holds no item at that number is the caller naming a real collection that does not contain what they said. A SOFT-DELETED collection counts as absent IN THAT PREDICATE, for the same reason a renamed one does — a claim about the fallback only: the exact-prefix lookup ahead of it never filtered on the collection’s own `deleted_at`, so an item whose collection was soft-deleted still matches THERE and never reaches the predicate. That is left alone rather than made consistent, because it cannot retarget — the downstream collection check refuses it. RESIDUAL, stated rather than implied and pinned by a test: a ref pasted from ANOTHER workspace whose prefix also names no collection here still resolves by number, because nothing in the value distinguishes it from a rename's leftover. No escape hatch, for v0.29's reason. Out of scope and unchanged: `GetItemByRef`, the navigational read path, where landing on the moved item is the helpful answer. **v0.35** (BUG-3079) makes an injected schema DEFAULT take the same `validateFieldType` check a caller-supplied value takes; one that fails is DISCARDED and named in `warnings.dropped_fields` instead of being stored. BEHAVIOR bump on the v0.30 / v0.29 / v0.16 grounds — no tool name, action enum or param shape changed, but a write door stores something different from what it stored before. Unlike v0.29, which turned an accepted value into a refusal, this turns a stored value into a reported drop: a create that used to land WITH a bad field still lands, without it. What it closes is two doors disagreeing about one value, decided only by who put it there. It also widens `dropped_fields` from the single relation case v0.29 introduced to a field of ANY type. No escape hatch, for v0.29's reason. ONE exception, which is the rule finishing its sentence rather than a carve-out: dropping leaves a REQUIRED field absent, which is exactly what the required check reports, so such a write is refused — naming the default as the cause, because a bare "field is required" points its reader at a request that never mentioned the field. **v0.34** (BUG-3037) adds an `expected_seq` param to `pad_item.action=update` — the STRONG optimistic-concurrency token, preferred over `expected_updated_at`. ADDITIVE (the v0.13 / v0.11 / v0.8 disposition): nothing existing moved and `expected_updated_at` still works. It is needed rather than nice because `updated_at` is stored at ONE-SECOND resolution, so two writes to a row inside one second both match the token and NEITHER conflicts — the loser is accepted and silently overwrites the winner, both callers seeing success. That is the shape of agent traffic, so the token agents were offered could not refuse the race it exists to refuse. `seq` is bumped on every mutation of the row under the write lock. Raising `updated_at`'s resolution instead was measured and REJECTED: the column is TEXT on both dialects and compared LEXICALLY in SQL (the since-cursor read plus eight `ORDER BY` sites), and Go's `RFC3339Nano` omits trailing zeros, so a sub-second value sorts BEFORE a whole-second value of the same second and the cursor would start skipping rows. The read side is what makes the token reachable: `seq` now serialises without `omitempty` on `models.Item` AND on the item SUMMARY shape (`cli.ItemSummary`), which is what `pad_item.list` returns by default since v0.9 — a token absent from the shape a caller reads is a token that caller cannot send. A value below 1 is a 400, not a 409: the server never issues one, so a 409 would read as contention that never clears. **v0.30** (BUG-2870) is a BEHAVIOR bump on the v0.29/v0.27/v0.26/v0.16/v0.10/v0.9 grounds — no tool name, action enum or param shape changed. **Every door refuses a padded KEY now**, and each was accepting it differently: /mcp trimmed it and wrote the declared field, the CLI stored a ghost field beside it — so both refuse something they used to accept. What is /mcp-only is the VALUE half, which it used to trim and type and now passes through to the same validation the CLI has always applied. A caller writing canonical entries sees no difference at either door. A second, separately-noticeable fix rides with it: `detectFieldConflicts` swallowed `parseFieldArray`'s refusal as "the caller owns this error surface", which held only while the sole possible error was a shape error — `reshapeItemFields` returns early with no `fields` object, so on the no-`fields` path (this bug's own path) four existing refusals were landing as successes. No escape hatch, for v0.29's reason: there is no legitimate call this refuses, only calls whose two readings a door used to choose between silently. **v0.29** (PLAN-2857 U1 / TASK-2878) is a BEHAVIOR bump on the v0.27/v0.26/v0.16/v0.10/v0.9 grounds (NOT v0.28's, which was purely additive) — no tool name, action enum, or param shape changed, but a `relation` field value must now name a live item in the collection that field declares, so every write door refuses values it used to store. Refused: names nothing (`not_found`), names an item in the wrong collection, the field declares no target collection (`target_missing`), or the value is a SLUG — a deliberate divergence from `ResolveItem`'s UUID→ref→slug ladder, since a slug is neither an ID nor stable and free text like "red" resolving to whatever is slugged `red` today is exactly the corruption this closes. Ordinary `validation_error`, no new code and no new details key, because stdio classifies errors by matching CLI stderr prose. A CARRIED value — already on the item, asserted by nobody — is never refused, since refusing would make every legacy item un-updatable, un-movable and un-copyable: within a workspace it resolves and SURVIVES, across a workspace boundary it is dropped without a lookup (a source-workspace id cannot mean anything in the destination) and reported through the same `warnings.dropped_fields` channel BUG-2674 established. No escape hatch, deliberately: unlike v0.10's `allow_draft` there is no legitimate call this refuses, and the case with a real claim to leniency is already exempt by provenance rather than by a flag. **v0.28** (IDEA-2641 / GitHub #1010) adds two ADDITIVE `pad_item` actions and two optional params: `remind` arms a one-shot reminder at an RFC3339 `remind_at` INSTANT, and `ack-reminder` acknowledges a fired one by `reminder_id`. Purely additive — nothing existing moved, and a v0.27 consumer that enumerates neither action is unaffected; same disposition as v0.13 / v0.11 / v0.8, which likewise wired existing CLI verbs onto the catalog. Agents already RECEIVED reminders (the poll surface is `pad_project.next` / `ready`, long exposed); what was missing is the half where an agent that defers work can say when it wants to be asked again. `remind_at` REFUSES a bare date rather than reading it as midnight — the `date` schema type accepts `YYYY-MM-DD` so a caller will try it, but a bare date names a 24-hour span and choosing an hour inside it would fire at a time nobody picked. Re-arm and disarm stay CLI-only: both address a reminder by an id the agent would have to list first, and no listing action exists on this surface yet — a door with no handle. **v0.27** (BUG-2850) types field values SERVER-SIDE at all eight validate sites, carries the `fields` object to the remote door with its JSON types intact, accepts undeclared keys while NAMING them in `warnings.undeclared_fields`, and replaces five accreted conflict guards with one canonical pass; the merge refuses several ambiguities it used to resolve silently. (This entry was missing from CLAUDE.md — the 0.27 unit swept `instructions.md` and `README.md` and not this file.) **v0.27** (BUG-2850) types field values server-side at all eight `Validate*` call sites so a declared number/json field is writable from the remote transport at all, carries the `fields` OBJECT with its JSON types intact, names undeclared keys back in `warnings.undeclared_fields` (accepted rather than refused — a census of 1012 items found 14 such keys across 168 live values, so refusing would have broken read-modify-write on items nobody had edited wrongly), and replaces the accreted per-site conflict guards with ONE check over a canonical view of every source; that check refuses ambiguities v0.26 resolved silently, chiefly two names for one target in a single call (`parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id`), refused even when the values match because the names address one thing through incomparable vocabularies and the two doors resolved them differently. **v0.26** (IDEA-2756) is a BEHAVIOR bump on the v0.9/v0.16/v0.25 grounds — no tool name, action enum, or param shape changed, but `pad_workspace.create` now refuses a call it used to permit. Closest precedent is v0.10, which likewise turned a server-side gate into a structured refusal; unlike v0.10 there is no `allow_draft`-style override, because the gate encodes a decision the USER made at consent time and a bypass param would be the app overriding its own grant. `POST /workspaces/import` is gated by the same shared helper (import mints a workspace through `store.ImportWorkspace`), though it has no MCP action today. **v0.25** (TASK-2657 / BUG-2702) resolves `pad_library.activate`'s destination collection from the target's declared artifact kind rather than the literal `conventions` / `playbooks` slugs, so activating into a workspace that renamed either collection lands correctly; a lookup ERROR is surfaced rather than silently falling back. **v0.24** (#1066) adds the `fields` OBJECT param to `pad_item` create/update — an alias merging into the same path as `field`/the dedicated params, so the shape reads return is finally a valid write shape; the same key supplied twice with conflicting values is REFUSED (refuse-on-ambiguity, the v0.18/v0.19 disposition), equal duplicates collapse to one write, and non-writer actions refuse a `fields` param loudly. It also makes input validation STRICT for every catalog tool: undeclared top-level keys are rejected with a structured error naming them, instead of being accepted and silently dropped by `BuildCLIArgs` — which is the mechanism that made the `fields` object a session-scoped silent no-op in the first place. Compat carve-out: `pad_item`'s v0.16 `assigned_user_id` / `agent_role_id` remote-transport clear form stays accepted (documented, undeprecated, deliberately never schema-declared). The strict half changes behaviour for inputs that previously "succeeded", but that reliance was indistinguishable from a caller bug (the key never did anything), so the break is the fix; one bump covers both halves. **v0.23** (BUG-2627 part 2 + BUG-2675) refuses system-metadata keys through `fields_patch` on all three doors at once, `github_pr` exempt on update (move/copy still refuse it), and adds the retry-hostile `stored_state_unreadable` code. **v0.22** (BUG-2674) stops `pad_item.action=move` destroying system metadata and refuses `field` setters naming the reserved keys there. **v0.21** bounds `pad_item.action=history` (BUG-2608): the `limit` param now covers it, default 50 / max 300, applied in the CATALOG action so it reaches both transports (HTTP reads the input; stdio gets the CLI's new `--limit` via BuildCLIArgs). The window is the NEWEST N and there is deliberately no `offset` — versions are reverse patches, so only a newest-end window is cheap to reconstruct. Additive param bump; a v0.20 consumer sending no limit now receives the newest 50 rather than every version, which is the fix. Summary mode additionally asks the server to skip patch resolution rather than resolving bodies the dispatcher discards. **v0.19** adds a `clear_parent` boolean to `pad_item` (BUG-2078) — an ADDITIVE param bump, same grounds as v0.18; nothing existing changed shape. The server has supported clearing a parent since BUG-2013 (`extractParentLink` treats a present-but-empty `parent` key in `fields_patch` as detach), but neither client surface could reach it — `--parent ""` was a silent no-op on the CLI and the MCP `parent` param has the same "empty means not provided" convention every other declared string on the tool has. Boolean rather than overloading the empty string, same two reasons as v0.18: keeps that invariant intact for every other param, and only a boolean reaches LOCAL STDIO via `BuildCLIArgs`, mapping to a new `--clear-parent` bareword flag exactly as `clear_assigned_user` maps to `--clear-assigned-user`. Update-only, same asymmetry as v0.18. A simultaneous `parent` + `clear_parent` — including via `field: ["parent=..."]` or the `plan` alias `extractParentLink` also accepts — is REFUSED on both transports, not silently resolved (codex round 1). Also refused, not silently applied: `clear_parent` against a collection whose schema declares its own `parent`/`plan` field — `extractParentLink` skips hierarchy handling entirely for a schema-shadowed key and lets it fall through as an ordinary field write, so the wire shape `{"parent":""}` can no longer distinguish clear-hierarchy intent from a legitimate blank-a-real-field write once it reaches the server; the ambiguity is created at the client surface that accepted `clear_parent`, so that surface refuses rather than guessing (codex round 2). **v0.18** adds `clear_assigned_user` / `clear_agent_role` booleans to `pad_item` (IDEA-2584) — an ADDITIVE param bump (v0.5/v0.6 precedent); nothing existing changed shape and v0.16/v0.17's empty-string forms still work, undeprecated. v0.16 and v0.17 made the clear WORK; nothing advertised it, because the params that do it were never in the catalog, so an agent reading the schema reached for `assign: ""` (a no-op, and it stays one). Booleans rather than declaring the string params, for two reasons: an empty DECLARED string is inert everywhere else on the tool, so giving one a destructive meaning would let a param-padding client silently unassign everything; and only a boolean can reach LOCAL STDIO, since `BuildCLIArgs` emits the CLI's real flags and a param with no flag behind it is dropped — these map to new `--clear-assigned-user` / `--clear-agent-role` bareword flags, exactly as `allow_draft` maps to `--allow-draft`. Update-only, deliberately asymmetric with create (clearing at create has no honest behaviour but a no-op; a test fails if someone adds them there). Server-side it is wiring, not new semantics: `models.ItemUpdate.ClearAssignedUser`/`ClearAgentRole` already existed with store support since BUG-2566. **v0.17** closes the transport gap v0.16 documented: local stdio MCP shells out to the CLI, which wrote `--field assigned_user_id=` into the item's FIELDS BLOB while the column stayed stale and then printed "Updated TASK-9". `cmd/pad/cmd_item.go` now lifts `columnFieldKeys` onto the columns on create AND update, mirroring `liftFieldsToColumns` and its INVARIANT. Two compat changes, ruled separately: non-empty values move to the column and stop writing the blob key (relying on the old behaviour is relying on a shadowing defect), and empty values clear (falls out of the lift, inherits BUG-2566). Existing stray blob keys are left alone — the fix stops minting new ones. Another behaviour-only bump (BUG-2583). **v0.16** lets an MCP agent UNASSIGN an item over the REMOTE transport (TASK-2571). No tool/action/param shape changed — this is a BEHAVIOR bump on the same grounds as v0.9: an empty-string `assigned_user_id` / `agent_role_id`, passed at the top level or as `field: ["assigned_user_id="]`, was silently dropped by two dispatch-path filters (`mapItemUpdate`, `liftFieldsToColumns`) and is now forwarded as a clear-to-NULL. The store has had defined clear semantics for exactly these two columns since BUG-2566 and HTTP inherited them, so this is uniformity restoration — MCP was the only surface with no way to unassign. Compat posture accepted deliberately: today's `""` senders get a no-op, and a no-op is the surprising reading. The empty-string filter on `tags` at the same call site STAYS (codex #547 r3 P2) — `tags: ""` is a corrupt JSONB/TEXT write, not a clear; same-looking guard, opposite justification. `clear_assigned_user` / `clear_agent_role` schema flags (option (b)) deliberately skipped as additive sugar, though codex review reopened the case — the catalog exposes `assign` / `role`, NOT the ID params, so an agent reading the schema still can't discover the clear (IDEA-2584); an empty `assign` is deliberately left inert because every other schema-declared string on that mapper treats empty as not-provided. **Transport scope:** v0.16 fixed the REMOTE /mcp transport only; v0.17 (BUG-2583) closed the local-stdio half at the CLI. **v0.15** adds the `unparented` boolean to `pad_item.list`, mutually exclusive with `parent`, for structural loose-item filtering (TASK-2096). **v0.14** added a `history` action to `pad_item` (read-only item version history — newest-first metadata; content body omitted for token thrift) and an `expected_updated_at` param for optimistic concurrency on `update` (round-trip the `updated_at` you last read; a stale value fails with a structured 409 `code=update_conflict`). The `update` action's field writes are now a server-side field-level MERGE (only the keys you set change) rather than a full-blob replace, closing the concurrent-update lost-write race (IDEA-1480 / TASK-2022) — pure addition to the action enum + param vocabulary; existing `pad_item` actions/params are unchanged and backwards-compatible. **v0.13** adds `ready` + `stale` actions to `pad_project`, mirroring the existing CLI `pad project ready` / `pad project stale` (TASK-2019): `ready` (read-only) returns the actionable backlog — the query-oriented counterpart to `next`, reusing the dashboard's suggested-next logic; `stale` (read-only) lists items needing attention (stalled, blocked, overdue, or out of the active workflow). Both HTTP dispatchers already existed (`dispatch_http_project.go`); this just wires them onto the catalog. `pad project reconcile` stays CLI-only (shells out to `gh` for live PR state — a local-git dependency MCP agents lack). Pure addition of two read-only actions — existing actions unchanged; backwards-compatible for v0.12 consumers that don't enumerate the new actions. **v0.12** adds an `activity` action to `pad_project`, mirroring the new CLI `pad project activity [--limit N] [--actor user|agent] [--since DATE]` (TASK-2018) — the non-streaming, bounded query counterpart to the CLI-only `pad project watch` SSE stream. Read-only snapshot of the workspace's enriched activity feed (item refs, titles, field-level change details) backed by the existing `GET /workspaces/{ws}/activity` endpoint (previously web-UI-only, now extended with a server-side `since` date filter so `limit`/`actor`/`since` behave identically across CLI, stdio MCP, and cloud HTTP), so agents can catch up on what other agents/users did since they last worked. Adds `actor` + `limit` params to the `pad_project` vocabulary (`since` already existed for changelog); pure addition — existing actions unchanged; backwards-compatible for v0.11 consumers that don't enumerate the new action. **v0.11** adds the read-only `pad_attachment` tool (the tenth resource × action tool) with `list` + `show` actions, mirroring the CLI `pad attachment list` / `pad attachment show` (TASK-2017): `list` enumerates a workspace's attachments (optional filters: item / category / collection / attached / unattached / sort / limit / offset); `show` returns one attachment's metadata (MIME, size, filename, ETag, last-modified) via a HEAD request without transferring bytes. Both HTTP dispatchers already existed (`dispatch_http_attachments.go`); this just wires them onto the catalog. Upload / download / view stay CLI-only (filesystem-bound, excluded per the catalog's exclusion rules). Pure addition — existing tools/actions unchanged; backwards-compatible for v0.10 consumers that don't enumerate the new tool. The base64 image RESOURCE for multimodal agents (`pad://workspace/{ws}/attachments/{id}`) shipped later in TASK-2077 (PR #930) as a bounded, image-only resource; TASK-2101 brought it — and the full read-only resource set — to the remote /mcp transport via the in-process `HTTPResourceFetcher`, so resources are no longer local-stdio-only. **v0.10** enforces the draft-playbook gate server-side: `pad_playbook.run` (and the underlying `POST /playbooks/{ref}/run`) now refuses a playbook whose `status` isn't `active` with a structured `playbook_not_active` error, adds an `allow_draft` boolean param (bareword `--allow-draft` on the CLI) as the escape hatch, and echoes the playbook `status` on both the `run` and `get` responses (BUG-2020). **v0.9** makes `pad_item.list` summary-shaped by default (drops item `content`, adds a default result limit of 50 / hard max 300 on MCP; CLI `--full` restores the complete shape) — a behavior change to the tool's return shape, hence the bump, though tool names, action enums, and parameter shapes are unchanged (TASK-2000). **v0.8** adds `restore` + `deleted` actions to `pad_workspace`, mirroring the CLI `pad workspace restore` / `pad workspace deleted` (TASK-1972): `deleted` (read-only) lists the caller's soft-deleted workspaces still inside the 30-day restore window; `restore` (mutating, not destructive, owner-only) un-soft-deletes a workspace by `slug` while it's still restorable. Both reuse the existing `slug` param — no new params; pure addition. **v0.7** adds `export` + `import` actions to `pad_item`, mirroring the CLI `pad item export` / `pad item import` (covers playbooks AND conventions). `export` (read-only) takes `ref` and returns the portable artifact text — it forces the CLI's stdout sink (`-o -`) so the bytes come back as the result instead of a file. `import` (mutating, not destructive) takes a new `artifact` param (the full artifact text) and returns `{ref, slug, warnings}`; the ExecDispatcher can't pipe stdin, so it spills the artifact to a temp file and dispatches `item import `. v0.6 added the `pad_item.backlinks` action; v0.5 added `pad_library`. v0.3 (PLAN-1377 / TASK-1380) introduced `pad_meta.action: bootstrap`, `pad_set_workspace`'s embedded-bootstrap response, and the `pad://workspace/{ws}/bootstrap` resource. **v0.4 (PLAN-1410)** is a comprehensive bootstrap-payload trim — same tool catalog, slimmer JSON shape inside bootstrap responses: `BootstrapCollection` projection drops `id`/`workspace_id`/timestamps/`settings` and emits `schema` as a nested object; `BootstrapRole` projection drops UUIDs/timestamps/`tools`; convention `slug` dropped; top-level `recent_activity` (a duplicate of `dashboard.recent_activity`) removed; new `BootstrapDashboard` wrapper caps five sub-arrays (`attention`, `recent_activity`, `active_items`, `active_plans`, `by_role`) at 5 entries each with parallel `*_overflow_count` fields; redundant schema labels omitted when `label == TitleCase(key)`. Cumulative size reduction: ~40% on a representative workspace, ~54% on the fixture (see PLAN-1410's Result section for per-section deltas). Compatibility: most changes are subtractive (dropped fields) or additive (overflow counts), but **one type change is breaking**: `collections[].schema` went from a JSON-encoded string to a nested JSON object — clients that JSON.parse()'d the string need to consume it directly as an object now. The dropped fields (UUIDs, timestamps, settings, duplicate `recent_activity`, convention `slug`) have canonical alternatives (slugs for addressing; `pad collection list` / `pad role list` for the full models when needed). +- `ToolSurfaceVersion` (currently `"0.57"`) — the MCP tool catalog contract. Bump when tool names, action enums, or parameter shapes change incompatibly. **v0.57** (TASK-2695) adds `pad_item` `edit-comment` (ref, comment_id, message) and `delete-comment` (ref, comment_id) on all three transports, backed by the item-scoped routes `PATCH/DELETE /workspaces/{ws}/items/{ref}/comments/{id}`, which answer 404 unless the comment is on `ref` and otherwise run the workspace routes' handlers (edit author-only, delete item-edit). CLI: `pad item comment-edit` / `comment-delete`, refused against a server without the `item_scoped_comment_writes` capability. Every serialised comment gains a derived `edited` bool (updated_at > created_at), and `pad item comments` prints each comment's id. ADDITIVE. **v0.56** (BUG-3244) gives `content_state` a second value, `superseded_set_aside`, on every MCP door that carries it (`pad_item` get / full list, `pad_playbook` run, the item resource's line above the body): edits an editor schema-version rebuild set aside, which opening the item will NOT restore, so the resource line and the `content_pending_flush` hint (chosen by `details.set_aside_rows`) name `pad item set-aside` and `overwrite_pending_edits` instead. ADDITIVE bump: no name, enum or param changed, but an agent branching on the one-word vocabulary meets a word whose remedy differs. Reading or discarding the rows alone is not on the catalog. **v0.55** (BUG-2659) sends `pad_item.action=search`'s `collection` to /search as typed, and /search resolves it exact-first per workspace in scope (the item routes' resolver). In a workspace holding both `task` and `tasks`, `collection: "task"` now answers from `task` instead of `tasks`; every other workspace is unchanged, because the shorthand still resolves server-side. Stdio reaches it through the CLI, which sends the name as typed only to a server advertising `search_collection_resolution`. BEHAVIOR bump on the v0.36 / v0.40 grounds. **v0.54** (BUG-3217) keeps a `fields` number's literal through MCP on both transports (mcp-go's argument decode rounded anything above 2^53, and stdio's `--field` stringify carried the rounded value), and REFUSES a `fields` number beside a `field` entry for the same key when they differ as exact numbers — two values that differ only above 2^53 used to collapse into one rounded write; a matching pair (3.0 beside n=3) still collapses. BEHAVIOR bump on the v0.48 grounds. **v0.53** (BUG-2819) adds `filename_source` (caller / normalised / substituted / derived / unknown) to every `pad_attachment.action=list` row on both transports, because a stored filename cannot say whether the caller sent it or the server substituted it. ADDITIVE, the v0.28 / v0.13 disposition; `show` does not carry it, and `mcp_audit_log.tool_name_source` reaches no catalog action. Rows written before it read `unknown`, never `caller`. **v0.52** (BUG-2367 item 4) refuses a move or copy (every migrate door) that would change an item between open, done and abandoned unless the caller names the destination done field. Only the STATE is compared, so two values that both mean done carry. Move answers 400 `state_change_requires_value` with details `{field, options, from, to}`; bulk move puts it in a `failed[]` row; the copy answers 400 `validation_error` with the same sentence; the preflight emits a needs_value row with reason `state_change`, which the web dialog picker renders. A destination with no done select field is exempt. BEHAVIOR bump. **v0.51** (BUG-2367) stops every migrate door (single move, bulk move, cross-workspace copy and its preflight) carrying a value into a field the destination marks `computed` (preflight drop reason `target_computed`) or a value that collides on a destination `unique_scope` field. Provenance decides, the TASK-2878 rule: a CARRIED collision is dropped and reported (`not_unique`), a supplied override or injected default that collides is refused 409 `conflict`, and a required field emptied by the drop is refused as required, which is the existing needs_value route. Before it, single move answered 500 on the indexed `invocation_slug`, bulk move leaked the SQL error, and an unindexed unique field stored a duplicate. Reporting is additive and omitempty: `warnings.not_unique` [{key, value, holder?, message}] on the move response, bulk `updated[]` rows and the copy response, and a `detail` sentence on the preflight's dropped row. `holder` is present only when the caller may see that item, so the drop is not an existence oracle. BEHAVIOR bump on the v0.43 / v0.29 grounds. **v0.50** (BUG-3014) adds an ADDITIVE `omitempty` `stored_as_text: true` to a `relation_targets` entry whose stored value is not UUID-shaped (a title a bundle import carried verbatim, or legacy free text). Such a value was never an id and used to hydrate as a plain id-only entry, which reads as "gone or hidden"; an id-only entry without the flag keeps that meaning, and a UUID-shaped value, resolved or not, is byte-identical to v0.49. The predicate is syntactic, decided from the stored bytes and never from a lookup, so it discloses nothing, and deliberately not coupled to the resolver ladder. The CLI prints `"" (text, not a reference)` and the web chip says the same. **v0.49** (BUG-2410) makes `pad_project.action=report` with no `collections` param leave out SYSTEM collections (`Collection.IsSystem`: Conventions and Playbooks), which were counted in Insights' created, WIP and status totals; naming one explicitly still includes it. BEHAVIOR bump on the v0.40 / v0.42 grounds: a default call's content changed for every caller (web Insights, print, `pad project report`, both MCP transports, all through the one store rule). **v0.48** (BUG-3163) makes `pad_item.action=create` REFUSE a `field` / `fields` entry naming any reserved metadata key (`implementation_notes`, `decision_log`, `github_pr`, `convention`) on every transport, with 400 `validation_error`, creating nothing: create was the last door that stored one, as a string no extractor reads. Library activation, its one system writer, moves to an additive typed `convention` create member (not on the catalog; `pad_library.activate` is unchanged), and the CLI verifies from the stored fields that it landed, because an older server ignores the member and answers 201. BEHAVIOR bump on the v0.47 grounds. **v0.47** (BUG-2696) makes `pad_item.action=update` REFUSE a field setter naming `github_pr` on every transport (400 `validation_error` naming `pad github link` / `unlink`), closing the exemption v0.23 left: that door stored the PR as an unreadable string, so no link ever rendered. The writer is a new additive, typed update member (`github_pr` object + `clear_github_pr`) used by `pad github link` / `unlink` / `project reconcile`, deliberately NOT on the catalog (a remote PR-link action waits on the catalog-trim decision). BEHAVIOR bump; item create is BUG-3163. **v0.46** (BUG-3156) makes `pad_item.action=bulk-update` over stdio and remote REFUSE, per item, a status or priority change on an item whose collection does not declare that field, writing nothing for that item: the answer WebMCP has given since v0.44, so the three transports agree. Rows keep each transport's shape (stdio `failed[]` with `validation_error`, remote `results[]` with `validation_failed` and the server message in `hint`). The mechanism is the update door's new `refuse_undeclared_fields` member, which both bulk-update transports set; `pad_item.update` does not, and still accepts-and-warns. A server that predates the member ignores it, which is the old accept-and-warn. BEHAVIOR bump on the v0.44 / v0.43 grounds. **v0.45** (BUG-2829) reports an HTTP 413 as `too_large` on BOTH transports, with the server's own code in `details.reason`, where it used to be `server_error`: three deliberate caps are reachable from the catalog (a title rename's link cascade, an oversized outbox event, an oversized artifact import), and the old code read as transient and invited a retry that fails identically. Remote maps 413 in `classifyHTTPStatusKind`; stdio gets it from a structured marker the CLI root writes for any 413, and `too_large` joins `allowedStructuredErrorCodes`, which a test now proves contains every code any CLI marker writer emits. BEHAVIOR bump on the v0.42 grounds; no name, enum or param shape changed. **v0.44** (BUG-3154) makes `pad_item.action=bulk-update` over the WebMCP browser transport REFUSE, per item, a status or priority change on an item whose collection does not declare that field: the item lands in the bulk response's `failed` array with `validation_error` naming the collection and field, nothing is written, and the rest of the call applies. The door is `POST /items/bulk` (`set-priority`, and `move` with a `status`, with or without a `collection`, where the target's schema decides), where the request carries an operation and a value and the SERVER chooses the key; it used to store an orphan field and report the item updated. BEHAVIOR bump on the v0.43 / v0.40 grounds. The transports disagreed after it, stated rather than implied: stdio and remote issued a per-item PATCH, the single-item update door, which ACCEPTED the key with `warnings.undeclared_fields` (v0.27), until v0.46 converged them. **v0.43** (BUG-2379) makes `pad_item.action=move` REFUSE a field override naming a field the destination schema does not declare, with 400 `malformed_override` — the same check, code and message the cross-workspace copy already used. BEHAVIOR bump on the v0.40 / v0.29 grounds: a write door refuses a call it used to accept, where it used to store the key as an orphan field no schema renders. Create and update still ACCEPT undeclared keys with `warnings.undeclared_fields` (v0.27), because callers round-trip the whole fields blob; an override is a fresh assertion about the destination schema, not round-tripped state. **v0.42** (BUG-3142 + BUG-3147) changes the error CODE a LOCAL STDIO caller receives for two failure families that used to arrive as retryable `server_error`: an argv refusal (cobra/pflag: unknown flag, arg count, …) is now `validation_failed` with its message kept, and a 429 on any command is now `rate_limited` with `details.retry_after_seconds`, via the structured marker line the CLI writes at its root. It also stops the refusal that caused most of the first family: `BuildCLIArgs` now emits flags first and the positionals behind `--`, so free text starting with `-` (`item search "-ship it"`) is no longer parsed as a flag. BEHAVIOR bump; no name, enum or param shape changed, and the remote transport is untouched. **v0.41** (PLAN-3114 U5 / TASK-3120) adds a `match` action to `pad_playbook`: given free `text`, a Choice question over the workspace's ACTIVE playbooks plus a reserved `"none"` option returns the best match's ref (or `"none"`), confidence, per-option probabilities and the model. No provider configured → 404 `decision_provider_unavailable`; a provider error or an out-of-set answer → 502 `decision_provider_error`. Pure ADDITIVE bump (new action + param, nothing existing moved), read-only and side-effect-free. **v0.40** (BUG-3028) gives a scalar `relation` ONE stored form for "no target", the key ABSENT, converging on multi_relation's rule. A blank (`""` or whitespace) a write SETS — create, `fields_patch`, a changed value in full `fields`, a move/copy override — is normalised to absent before validation, so a `required` relation is refused as required (a patch blanking it now gets the answer deleting it always got). A legacy blank a write only CARRIES — an untouched key under `fields_patch` (normalised by the store under its write lock, via `ItemUpdate.BlankRelationKeys`), an unchanged value in full `fields`, move, copy, artifact import — is normalised after validation and never refused. An empty list filter on a scalar relation key of a collection-scoped list (`owner=`) matches all three spellings. BEHAVIOR bump; no shape change. Workspace import (`store.ImportWorkspace`) still stores blobs verbatim; its legacy blanks normalise on the next write. **v0.39** (BUG-3133) refuses `pad_item.update` that sets content WITH a version token (`expected_seq` / `expected_updated_at`) while the item's op-log holds content-bearing rows above its flush watermark, with 409 `content_pending_flush` (details `ref`, `pending_rows`). BEHAVIOR bump on the v0.35 / v0.36 grounds, carrying one additive param: `overwrite_pending_edits` lifts it, on the API, the CLI (`--overwrite-pending-edits`; `pad item edit --force` sets it on the save) and MCP alike, because an agent must be able to clear the refusal without a browser. The token guards the ROW and a tab's unflushed typing is not in it, so the write used to replace those edits — through the applier's setContent with a tab open, and by the direct path's op-log prune, which DELETES them, with none. Its own code rather than `update_conflict` because re-reading returns the same row and seq. A write without a token, without content, or to an item with nothing pending is unchanged, except that a direct write now reports `warnings.pruned_pending_edits` (additive, omitempty) when its prune deleted such rows. The check runs in the write's own tx; on the direct path it is under the item lock and appendMu and exact, on the applier path a tab can still type in the one server hop between the check and setContent. (v0.37 and v0.38 — the `agent` projection on `pad_item.get` and `claim` / `release` — are in `internal/mcp/version.go`; this paragraph had stopped at v0.36.) **v0.36** (BUG-3082) stops a REF-SHAPED `relation` value falling back to matching by item NUMBER when its prefix names a collection that is LIVE in the workspace; such a value is now `not_found`. BEHAVIOR bump on the v0.35 / v0.30 / v0.29 / v0.16 grounds — no tool name, action enum or param shape changed, but a write door refuses a value it used to accept. It finishes v0.29: that entry closed free-text corruption ("red" resolving to whatever is slugged `red` today) and left the same corruption open for anything shaped like a REF, which is the spelling the docs tell callers to prefer. Item numbers are workspace-unique and sequential ACROSS collections, so `CONVE-1` and `SECRE-1` are never both real — the fallback dropped the prefix, matched on the number alone, and landed on an item that was usually inside the field's declared collection, so the wrong-collection check above it passed and NOTHING was raised: a caller wrote `CONVE-1`, the item stored a different item's UUID, and the write answered 201 with the substitution visible only in `relation_targets`. The predicate is about the PREFIX rather than the item because a collection RENAME changes its prefix and a relation already written as the old ref must keep resolving (BUG-2873) — that ref's prefix names nothing afterwards, so the fallback still fires for it, while a prefix that IS live and simply holds no item at that number is the caller naming a real collection that does not contain what they said. A SOFT-DELETED collection counts as absent IN THAT PREDICATE, for the same reason a renamed one does — a claim about the fallback only: the exact-prefix lookup ahead of it never filtered on the collection’s own `deleted_at`, so an item whose collection was soft-deleted still matches THERE and never reaches the predicate. That is left alone rather than made consistent, because it cannot retarget — the downstream collection check refuses it. RESIDUAL, stated rather than implied and pinned by a test: a ref pasted from ANOTHER workspace whose prefix also names no collection here still resolves by number, because nothing in the value distinguishes it from a rename's leftover. No escape hatch, for v0.29's reason. Out of scope and unchanged: `GetItemByRef`, the navigational read path, where landing on the moved item is the helpful answer. **v0.35** (BUG-3079) makes an injected schema DEFAULT take the same `validateFieldType` check a caller-supplied value takes; one that fails is DISCARDED and named in `warnings.dropped_fields` instead of being stored. BEHAVIOR bump on the v0.30 / v0.29 / v0.16 grounds — no tool name, action enum or param shape changed, but a write door stores something different from what it stored before. Unlike v0.29, which turned an accepted value into a refusal, this turns a stored value into a reported drop: a create that used to land WITH a bad field still lands, without it. What it closes is two doors disagreeing about one value, decided only by who put it there. It also widens `dropped_fields` from the single relation case v0.29 introduced to a field of ANY type. No escape hatch, for v0.29's reason. ONE exception, which is the rule finishing its sentence rather than a carve-out: dropping leaves a REQUIRED field absent, which is exactly what the required check reports, so such a write is refused — naming the default as the cause, because a bare "field is required" points its reader at a request that never mentioned the field. **v0.34** (BUG-3037) adds an `expected_seq` param to `pad_item.action=update` — the STRONG optimistic-concurrency token, preferred over `expected_updated_at`. ADDITIVE (the v0.13 / v0.11 / v0.8 disposition): nothing existing moved and `expected_updated_at` still works. It is needed rather than nice because `updated_at` is stored at ONE-SECOND resolution, so two writes to a row inside one second both match the token and NEITHER conflicts — the loser is accepted and silently overwrites the winner, both callers seeing success. That is the shape of agent traffic, so the token agents were offered could not refuse the race it exists to refuse. `seq` is bumped on every mutation of the row under the write lock. Raising `updated_at`'s resolution instead was measured and REJECTED: the column is TEXT on both dialects and compared LEXICALLY in SQL (the since-cursor read plus eight `ORDER BY` sites), and Go's `RFC3339Nano` omits trailing zeros, so a sub-second value sorts BEFORE a whole-second value of the same second and the cursor would start skipping rows. The read side is what makes the token reachable: `seq` now serialises without `omitempty` on `models.Item` AND on the item SUMMARY shape (`cli.ItemSummary`), which is what `pad_item.list` returns by default since v0.9 — a token absent from the shape a caller reads is a token that caller cannot send. A value below 1 is a 400, not a 409: the server never issues one, so a 409 would read as contention that never clears. **v0.30** (BUG-2870) is a BEHAVIOR bump on the v0.29/v0.27/v0.26/v0.16/v0.10/v0.9 grounds — no tool name, action enum or param shape changed. **Every door refuses a padded KEY now**, and each was accepting it differently: /mcp trimmed it and wrote the declared field, the CLI stored a ghost field beside it — so both refuse something they used to accept. What is /mcp-only is the VALUE half, which it used to trim and type and now passes through to the same validation the CLI has always applied. A caller writing canonical entries sees no difference at either door. A second, separately-noticeable fix rides with it: `detectFieldConflicts` swallowed `parseFieldArray`'s refusal as "the caller owns this error surface", which held only while the sole possible error was a shape error — `reshapeItemFields` returns early with no `fields` object, so on the no-`fields` path (this bug's own path) four existing refusals were landing as successes. No escape hatch, for v0.29's reason: there is no legitimate call this refuses, only calls whose two readings a door used to choose between silently. **v0.29** (PLAN-2857 U1 / TASK-2878) is a BEHAVIOR bump on the v0.27/v0.26/v0.16/v0.10/v0.9 grounds (NOT v0.28's, which was purely additive) — no tool name, action enum, or param shape changed, but a `relation` field value must now name a live item in the collection that field declares, so every write door refuses values it used to store. Refused: names nothing (`not_found`), names an item in the wrong collection, the field declares no target collection (`target_missing`), or the value is a SLUG — a deliberate divergence from `ResolveItem`'s UUID→ref→slug ladder, since a slug is neither an ID nor stable and free text like "red" resolving to whatever is slugged `red` today is exactly the corruption this closes. Ordinary `validation_error`, no new code and no new details key, because stdio classifies errors by matching CLI stderr prose. A CARRIED value — already on the item, asserted by nobody — is never refused, since refusing would make every legacy item un-updatable, un-movable and un-copyable: within a workspace it resolves and SURVIVES, across a workspace boundary it is dropped without a lookup (a source-workspace id cannot mean anything in the destination) and reported through the same `warnings.dropped_fields` channel BUG-2674 established. No escape hatch, deliberately: unlike v0.10's `allow_draft` there is no legitimate call this refuses, and the case with a real claim to leniency is already exempt by provenance rather than by a flag. **v0.28** (IDEA-2641 / GitHub #1010) adds two ADDITIVE `pad_item` actions and two optional params: `remind` arms a one-shot reminder at an RFC3339 `remind_at` INSTANT, and `ack-reminder` acknowledges a fired one by `reminder_id`. Purely additive — nothing existing moved, and a v0.27 consumer that enumerates neither action is unaffected; same disposition as v0.13 / v0.11 / v0.8, which likewise wired existing CLI verbs onto the catalog. Agents already RECEIVED reminders (the poll surface is `pad_project.next` / `ready`, long exposed); what was missing is the half where an agent that defers work can say when it wants to be asked again. `remind_at` REFUSES a bare date rather than reading it as midnight — the `date` schema type accepts `YYYY-MM-DD` so a caller will try it, but a bare date names a 24-hour span and choosing an hour inside it would fire at a time nobody picked. Re-arm and disarm stay CLI-only: both address a reminder by an id the agent would have to list first, and no listing action exists on this surface yet — a door with no handle. **v0.27** (BUG-2850) types field values SERVER-SIDE at all eight validate sites, carries the `fields` object to the remote door with its JSON types intact, accepts undeclared keys while NAMING them in `warnings.undeclared_fields`, and replaces five accreted conflict guards with one canonical pass; the merge refuses several ambiguities it used to resolve silently. (This entry was missing from CLAUDE.md — the 0.27 unit swept `instructions.md` and `README.md` and not this file.) **v0.27** (BUG-2850) types field values server-side at all eight `Validate*` call sites so a declared number/json field is writable from the remote transport at all, carries the `fields` OBJECT with its JSON types intact, names undeclared keys back in `warnings.undeclared_fields` (accepted rather than refused — a census of 1012 items found 14 such keys across 168 live values, so refusing would have broken read-modify-write on items nobody had edited wrongly), and replaces the accreted per-site conflict guards with ONE check over a canonical view of every source; that check refuses ambiguities v0.26 resolved silently, chiefly two names for one target in a single call (`parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id`), refused even when the values match because the names address one thing through incomparable vocabularies and the two doors resolved them differently. **v0.26** (IDEA-2756) is a BEHAVIOR bump on the v0.9/v0.16/v0.25 grounds — no tool name, action enum, or param shape changed, but `pad_workspace.create` now refuses a call it used to permit. Closest precedent is v0.10, which likewise turned a server-side gate into a structured refusal; unlike v0.10 there is no `allow_draft`-style override, because the gate encodes a decision the USER made at consent time and a bypass param would be the app overriding its own grant. `POST /workspaces/import` is gated by the same shared helper (import mints a workspace through `store.ImportWorkspace`), though it has no MCP action today. **v0.25** (TASK-2657 / BUG-2702) resolves `pad_library.activate`'s destination collection from the target's declared artifact kind rather than the literal `conventions` / `playbooks` slugs, so activating into a workspace that renamed either collection lands correctly; a lookup ERROR is surfaced rather than silently falling back. **v0.24** (#1066) adds the `fields` OBJECT param to `pad_item` create/update — an alias merging into the same path as `field`/the dedicated params, so the shape reads return is finally a valid write shape; the same key supplied twice with conflicting values is REFUSED (refuse-on-ambiguity, the v0.18/v0.19 disposition), equal duplicates collapse to one write, and non-writer actions refuse a `fields` param loudly. It also makes input validation STRICT for every catalog tool: undeclared top-level keys are rejected with a structured error naming them, instead of being accepted and silently dropped by `BuildCLIArgs` — which is the mechanism that made the `fields` object a session-scoped silent no-op in the first place. Compat carve-out: `pad_item`'s v0.16 `assigned_user_id` / `agent_role_id` remote-transport clear form stays accepted (documented, undeprecated, deliberately never schema-declared). The strict half changes behaviour for inputs that previously "succeeded", but that reliance was indistinguishable from a caller bug (the key never did anything), so the break is the fix; one bump covers both halves. **v0.23** (BUG-2627 part 2 + BUG-2675) refuses system-metadata keys through `fields_patch` on all three doors at once, `github_pr` exempt on update (move/copy still refuse it), and adds the retry-hostile `stored_state_unreadable` code. **v0.22** (BUG-2674) stops `pad_item.action=move` destroying system metadata and refuses `field` setters naming the reserved keys there. **v0.21** bounds `pad_item.action=history` (BUG-2608): the `limit` param now covers it, default 50 / max 300, applied in the CATALOG action so it reaches both transports (HTTP reads the input; stdio gets the CLI's new `--limit` via BuildCLIArgs). The window is the NEWEST N and there is deliberately no `offset` — versions are reverse patches, so only a newest-end window is cheap to reconstruct. Additive param bump; a v0.20 consumer sending no limit now receives the newest 50 rather than every version, which is the fix. Summary mode additionally asks the server to skip patch resolution rather than resolving bodies the dispatcher discards. **v0.19** adds a `clear_parent` boolean to `pad_item` (BUG-2078) — an ADDITIVE param bump, same grounds as v0.18; nothing existing changed shape. The server has supported clearing a parent since BUG-2013 (`extractParentLink` treats a present-but-empty `parent` key in `fields_patch` as detach), but neither client surface could reach it — `--parent ""` was a silent no-op on the CLI and the MCP `parent` param has the same "empty means not provided" convention every other declared string on the tool has. Boolean rather than overloading the empty string, same two reasons as v0.18: keeps that invariant intact for every other param, and only a boolean reaches LOCAL STDIO via `BuildCLIArgs`, mapping to a new `--clear-parent` bareword flag exactly as `clear_assigned_user` maps to `--clear-assigned-user`. Update-only, same asymmetry as v0.18. A simultaneous `parent` + `clear_parent` — including via `field: ["parent=..."]` or the `plan` alias `extractParentLink` also accepts — is REFUSED on both transports, not silently resolved (codex round 1). Also refused, not silently applied: `clear_parent` against a collection whose schema declares its own `parent`/`plan` field — `extractParentLink` skips hierarchy handling entirely for a schema-shadowed key and lets it fall through as an ordinary field write, so the wire shape `{"parent":""}` can no longer distinguish clear-hierarchy intent from a legitimate blank-a-real-field write once it reaches the server; the ambiguity is created at the client surface that accepted `clear_parent`, so that surface refuses rather than guessing (codex round 2). **v0.18** adds `clear_assigned_user` / `clear_agent_role` booleans to `pad_item` (IDEA-2584) — an ADDITIVE param bump (v0.5/v0.6 precedent); nothing existing changed shape and v0.16/v0.17's empty-string forms still work, undeprecated. v0.16 and v0.17 made the clear WORK; nothing advertised it, because the params that do it were never in the catalog, so an agent reading the schema reached for `assign: ""` (a no-op, and it stays one). Booleans rather than declaring the string params, for two reasons: an empty DECLARED string is inert everywhere else on the tool, so giving one a destructive meaning would let a param-padding client silently unassign everything; and only a boolean can reach LOCAL STDIO, since `BuildCLIArgs` emits the CLI's real flags and a param with no flag behind it is dropped — these map to new `--clear-assigned-user` / `--clear-agent-role` bareword flags, exactly as `allow_draft` maps to `--allow-draft`. Update-only, deliberately asymmetric with create (clearing at create has no honest behaviour but a no-op; a test fails if someone adds them there). Server-side it is wiring, not new semantics: `models.ItemUpdate.ClearAssignedUser`/`ClearAgentRole` already existed with store support since BUG-2566. **v0.17** closes the transport gap v0.16 documented: local stdio MCP shells out to the CLI, which wrote `--field assigned_user_id=` into the item's FIELDS BLOB while the column stayed stale and then printed "Updated TASK-9". `cmd/pad/cmd_item.go` now lifts `columnFieldKeys` onto the columns on create AND update, mirroring `liftFieldsToColumns` and its INVARIANT. Two compat changes, ruled separately: non-empty values move to the column and stop writing the blob key (relying on the old behaviour is relying on a shadowing defect), and empty values clear (falls out of the lift, inherits BUG-2566). Existing stray blob keys are left alone — the fix stops minting new ones. Another behaviour-only bump (BUG-2583). **v0.16** lets an MCP agent UNASSIGN an item over the REMOTE transport (TASK-2571). No tool/action/param shape changed — this is a BEHAVIOR bump on the same grounds as v0.9: an empty-string `assigned_user_id` / `agent_role_id`, passed at the top level or as `field: ["assigned_user_id="]`, was silently dropped by two dispatch-path filters (`mapItemUpdate`, `liftFieldsToColumns`) and is now forwarded as a clear-to-NULL. The store has had defined clear semantics for exactly these two columns since BUG-2566 and HTTP inherited them, so this is uniformity restoration — MCP was the only surface with no way to unassign. Compat posture accepted deliberately: today's `""` senders get a no-op, and a no-op is the surprising reading. The empty-string filter on `tags` at the same call site STAYS (codex #547 r3 P2) — `tags: ""` is a corrupt JSONB/TEXT write, not a clear; same-looking guard, opposite justification. `clear_assigned_user` / `clear_agent_role` schema flags (option (b)) deliberately skipped as additive sugar, though codex review reopened the case — the catalog exposes `assign` / `role`, NOT the ID params, so an agent reading the schema still can't discover the clear (IDEA-2584); an empty `assign` is deliberately left inert because every other schema-declared string on that mapper treats empty as not-provided. **Transport scope:** v0.16 fixed the REMOTE /mcp transport only; v0.17 (BUG-2583) closed the local-stdio half at the CLI. **v0.15** adds the `unparented` boolean to `pad_item.list`, mutually exclusive with `parent`, for structural loose-item filtering (TASK-2096). **v0.14** added a `history` action to `pad_item` (read-only item version history — newest-first metadata; content body omitted for token thrift) and an `expected_updated_at` param for optimistic concurrency on `update` (round-trip the `updated_at` you last read; a stale value fails with a structured 409 `code=update_conflict`). The `update` action's field writes are now a server-side field-level MERGE (only the keys you set change) rather than a full-blob replace, closing the concurrent-update lost-write race (IDEA-1480 / TASK-2022) — pure addition to the action enum + param vocabulary; existing `pad_item` actions/params are unchanged and backwards-compatible. **v0.13** adds `ready` + `stale` actions to `pad_project`, mirroring the existing CLI `pad project ready` / `pad project stale` (TASK-2019): `ready` (read-only) returns the actionable backlog — the query-oriented counterpart to `next`, reusing the dashboard's suggested-next logic; `stale` (read-only) lists items needing attention (stalled, blocked, overdue, or out of the active workflow). Both HTTP dispatchers already existed (`dispatch_http_project.go`); this just wires them onto the catalog. `pad project reconcile` stays CLI-only (shells out to `gh` for live PR state — a local-git dependency MCP agents lack). Pure addition of two read-only actions — existing actions unchanged; backwards-compatible for v0.12 consumers that don't enumerate the new actions. **v0.12** adds an `activity` action to `pad_project`, mirroring the new CLI `pad project activity [--limit N] [--actor user|agent] [--since DATE]` (TASK-2018) — the non-streaming, bounded query counterpart to the CLI-only `pad project watch` SSE stream. Read-only snapshot of the workspace's enriched activity feed (item refs, titles, field-level change details) backed by the existing `GET /workspaces/{ws}/activity` endpoint (previously web-UI-only, now extended with a server-side `since` date filter so `limit`/`actor`/`since` behave identically across CLI, stdio MCP, and cloud HTTP), so agents can catch up on what other agents/users did since they last worked. Adds `actor` + `limit` params to the `pad_project` vocabulary (`since` already existed for changelog); pure addition — existing actions unchanged; backwards-compatible for v0.11 consumers that don't enumerate the new action. **v0.11** adds the read-only `pad_attachment` tool (the tenth resource × action tool) with `list` + `show` actions, mirroring the CLI `pad attachment list` / `pad attachment show` (TASK-2017): `list` enumerates a workspace's attachments (optional filters: item / category / collection / attached / unattached / sort / limit / offset); `show` returns one attachment's metadata (MIME, size, filename, ETag, last-modified) via a HEAD request without transferring bytes. Both HTTP dispatchers already existed (`dispatch_http_attachments.go`); this just wires them onto the catalog. Upload / download / view stay CLI-only (filesystem-bound, excluded per the catalog's exclusion rules). Pure addition — existing tools/actions unchanged; backwards-compatible for v0.10 consumers that don't enumerate the new tool. The base64 image RESOURCE for multimodal agents (`pad://workspace/{ws}/attachments/{id}`) shipped later in TASK-2077 (PR #930) as a bounded, image-only resource; TASK-2101 brought it — and the full read-only resource set — to the remote /mcp transport via the in-process `HTTPResourceFetcher`, so resources are no longer local-stdio-only. **v0.10** enforces the draft-playbook gate server-side: `pad_playbook.run` (and the underlying `POST /playbooks/{ref}/run`) now refuses a playbook whose `status` isn't `active` with a structured `playbook_not_active` error, adds an `allow_draft` boolean param (bareword `--allow-draft` on the CLI) as the escape hatch, and echoes the playbook `status` on both the `run` and `get` responses (BUG-2020). **v0.9** makes `pad_item.list` summary-shaped by default (drops item `content`, adds a default result limit of 50 / hard max 300 on MCP; CLI `--full` restores the complete shape) — a behavior change to the tool's return shape, hence the bump, though tool names, action enums, and parameter shapes are unchanged (TASK-2000). **v0.8** adds `restore` + `deleted` actions to `pad_workspace`, mirroring the CLI `pad workspace restore` / `pad workspace deleted` (TASK-1972): `deleted` (read-only) lists the caller's soft-deleted workspaces still inside the 30-day restore window; `restore` (mutating, not destructive, owner-only) un-soft-deletes a workspace by `slug` while it's still restorable. Both reuse the existing `slug` param — no new params; pure addition. **v0.7** adds `export` + `import` actions to `pad_item`, mirroring the CLI `pad item export` / `pad item import` (covers playbooks AND conventions). `export` (read-only) takes `ref` and returns the portable artifact text — it forces the CLI's stdout sink (`-o -`) so the bytes come back as the result instead of a file. `import` (mutating, not destructive) takes a new `artifact` param (the full artifact text) and returns `{ref, slug, warnings}`; the ExecDispatcher can't pipe stdin, so it spills the artifact to a temp file and dispatches `item import `. v0.6 added the `pad_item.backlinks` action; v0.5 added `pad_library`. v0.3 (PLAN-1377 / TASK-1380) introduced `pad_meta.action: bootstrap`, `pad_set_workspace`'s embedded-bootstrap response, and the `pad://workspace/{ws}/bootstrap` resource. **v0.4 (PLAN-1410)** is a comprehensive bootstrap-payload trim — same tool catalog, slimmer JSON shape inside bootstrap responses: `BootstrapCollection` projection drops `id`/`workspace_id`/timestamps/`settings` and emits `schema` as a nested object; `BootstrapRole` projection drops UUIDs/timestamps/`tools`; convention `slug` dropped; top-level `recent_activity` (a duplicate of `dashboard.recent_activity`) removed; new `BootstrapDashboard` wrapper caps five sub-arrays (`attention`, `recent_activity`, `active_items`, `active_plans`, `by_role`) at 5 entries each with parallel `*_overflow_count` fields; redundant schema labels omitted when `label == TitleCase(key)`. Cumulative size reduction: ~40% on a representative workspace, ~54% on the fixture (see PLAN-1410's Result section for per-section deltas). Compatibility: most changes are subtractive (dropped fields) or additive (overflow counts), but **one type change is breaking**: `collections[].schema` went from a JSON-encoded string to a nested JSON object — clients that JSON.parse()'d the string need to consume it directly as an object now. The dropped fields (UUIDs, timestamps, settings, duplicate `recent_activity`, convention `slug`) have canonical alternatives (slugs for addressing; `pad collection list` / `pad role list` for the full models when needed). Post-0.30 without a bump (BUG-2995): a successful content write through the designated applier used to answer with the item's PREVIOUS content — on that path the markdown goes to a live tab's Y.Doc and the row write runs with `Content` nil, so the response described the item as it stood before the request. It now carries the content as SENT plus an additive `omitempty` `warnings.content_outcome: "applied_pending_flush"`. No bump, on the BUG-2304 rather than the v0.9 grounds: v0.9 bumped because list rows LOST fields consumers read, whereas here nothing is removed or retyped — what changed is the VALUE of one field on one path, which a caller does observe. The claim is that no DOCUMENTED OR SUPPORTED reliance breaks, not that none can exist: a consumer could have detected the applier path by noticing the response echoed something other than what it sent, and that stops working — but it was never documented, a genuinely lost write produced the same mismatch, and `warnings.content_outcome` answers that question properly in the same change. Reads are unchanged: during the window a `get` (or `list` with `full: true`) still answers from the row, so the warning means re-read later, not re-send. diff --git a/README.md b/README.md index 584ee628b..5e7909dbf 100644 --- a/README.md +++ b/README.md @@ -406,11 +406,11 @@ directory for `claude-code`, and an `[mcp_servers.pad]` table in project-scoped, it's install-on-request only — `--all` and `pad mcp status` cover the per-user clients (including Codex) and skip it. -**Tool catalog (v0.56)** — ten resource × action tools plus `pad_set_workspace` (eleven total), no flat verb explosion. Undeclared input keys are rejected with a structured error rather than silently dropped. `pad_item` create/update accept field values as a `fields` object (the same shape reads return) as an equivalent to the dedicated params / `field: ["key=value"]`, and its values keep their JSON types where the transport can carry them. Field values are typed against the collection schema server-side, so a declared number or json field is writable from the remote transport (which sends every value as a string). Keys the schema does not declare are stored and NAMED back in `warnings.undeclared_fields`. One key supplied through two doors is adjudicated once: differing values are refused, equal ones collapse, and two names for the same target — `parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id` — are refused even when the values match. `pad_item.get` accepts `agent: true` for the full body and work-relevant metadata without internal UUID plumbing; `pad_item.list` accepts `unparented: true` (mutually exclusive with `parent`) to select items with no parent or implements relationship, and is summary-shaped by default on both transports (`full: true` opts into complete content bodies): +**Tool catalog (v0.57)** — ten resource × action tools plus `pad_set_workspace` (eleven total), no flat verb explosion. Undeclared input keys are rejected with a structured error rather than silently dropped. `pad_item` create/update accept field values as a `fields` object (the same shape reads return) as an equivalent to the dedicated params / `field: ["key=value"]`, and its values keep their JSON types where the transport can carry them. Field values are typed against the collection schema server-side, so a declared number or json field is writable from the remote transport (which sends every value as a string). Keys the schema does not declare are stored and NAMED back in `warnings.undeclared_fields`. One key supplied through two doors is adjudicated once: differing values are refused, equal ones collapse, and two names for the same target — `parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id` — are refused even when the values match. `pad_item.get` accepts `agent: true` for the full body and work-relevant metadata without internal UUID plumbing; `pad_item.list` accepts `unparented: true` (mutually exclusive with `parent`) to select items with no parent or implements relationship, and is summary-shaped by default on both transports (`full: true` opts into complete content bodies): | Tool | Actions | |---|---| -| `pad_item` | `create`, `update`, `delete`, `get`, `list`, `move`, `restore`, `link`, `unlink`, `deps`, `star`, `unstar`, `starred`, `comment`, `list-comments`, `backlinks`, `bulk-update`, `note`, `decide`, `export`, `import`, `history`, `remind`, `ack-reminder`, `claim`, `release` | +| `pad_item` | `create`, `update`, `delete`, `get`, `list`, `move`, `restore`, `link`, `unlink`, `deps`, `star`, `unstar`, `starred`, `comment`, `list-comments`, `edit-comment`, `delete-comment`, `backlinks`, `bulk-update`, `note`, `decide`, `export`, `import`, `history`, `remind`, `ack-reminder`, `claim`, `release` | | `pad_workspace` | `list`, `members`, `invite`, `storage`, `audit-log`, `create`, `claim`, `deleted`, `restore` | | `pad_collection` | `list`, `create`, `update`, `delete` | | `pad_project` | `dashboard`, `next`, `ready`, `stale`, `standup`, `changelog`, `report`, `activity` | @@ -435,7 +435,7 @@ initialize handshake under `capabilities.experimental.padCmdhelp` and `pad://_meta/version`): - `cmdhelp_version: "0.1"` — CLI help-tree contract (used at dispatch time) -- `tool_surface_version: "0.56"` — MCP tool catalog contract (v0.5 added `pad_library`; v0.6 `pad_item.backlinks`; v0.7 `pad_item` `export`/`import`; v0.8 `pad_workspace` `deleted`/`restore`; v0.9 made `pad_item.list` summary-shaped by default with a default+max result cap; v0.10 enforced the draft-playbook gate server-side on `pad_playbook.run` with an `allow_draft` escape hatch; v0.11 added the read-only `pad_attachment` tool (`list`/`show`); v0.12 added `pad_project.activity` (agent-accessible non-streaming activity feed); v0.13 added `pad_project` `ready`/`stale` (agent-oriented backlog + attention queries); v0.14 added `pad_item` `history` + optimistic concurrency (TASK-2022); v0.15 added the `pad_item.list` `unparented` parameter (TASK-2096); v0.16 made an empty-string `assigned_user_id` / `agent_role_id` CLEAR the assignment instead of being silently dropped, so an agent can finally unassign an item (TASK-2571); v0.17 carried that to the LOCAL STDIO transport by teaching the CLI to lift those keys onto their columns instead of into the fields blob (BUG-2583); v0.18 added `clear_assigned_user` / `clear_agent_role` booleans — the canonical, schema-discoverable way to unassign, backed by new `--clear-assigned-user` / `--clear-agent-role` flags on `pad item update` (IDEA-2584); v0.19 added a `clear_parent` boolean — the canonical, schema-discoverable way to detach an item from its parent, backed by a new `--clear-parent` flag on `pad item update` (BUG-2078); v0.20 gave every tool an explicit annotation block derived from the catalog’s read-only knowledge — fully-read-only tools advertise `readOnlyHint: true` / `destructiveHint: false`, all-additive-write tools (`pad_workspace`, `pad_library`) drop `destructiveHint`, overwrite/delete-capable tools stay conservatively destructive, `openWorldHint: false` everywhere — replacing mcp-go’s defaults that marked every tool destructive (BUG-2302), and made `pad_item.list` summary-shaped on the remote HTTP transport too, with a declared `full` boolean as the opt-in for complete bodies on both transports (BUG-2305); v0.21 bounded `pad_item.history`, which was unbounded on every surface — `limit` now covers it (default 50, max 300, the NEWEST N; no `offset`, because reverse-patch storage makes only a newest-end window cheap), applied in the catalog action so it lands on both transports, and summary mode now asks the server to skip patch resolution rather than resolving bodies the dispatcher discards (BUG-2608); v0.22 stopped `pad_item.move` destroying an item’s system metadata — implementation notes, decision log, linked PR and convention data now survive a move, any field the destination schema has no home for is REPORTED in the move’s activity entry rather than vanishing, and a `field` setter naming one of those reserved keys is refused with `malformed_override` instead of writing it (BUG-2674); v0.23 closed the same door on the ordinary update — a `field` setter naming `implementation_notes`, `decision_log` or `convention` is now refused on every transport at once (`validation_error` on HTTP, surfaced to MCP clients as `validation_failed`); the one gate covers the CLI, remote MCP and stdio MCP at once because all three lower a `field` setter into the same `fields_patch`; `github_pr` was exempt on update until v0.47 refused it too, because that door stored the PR as an unreadable string (BUG-2696), and `pad github link` / `unlink` now write it through a typed, validated update member; item CREATE stayed open until v0.48 refused the keys there too (BUG-3163), once library activation, the one system writer using that door, moved to a typed `convention` create member. v0.23 also added the retry-hostile `stored_state_unreadable` error code so an agent told its target item’s stored data is unreadable stops instead of retrying a permanent failure (BUG-2627 / BUG-2675); v0.24 made the `pad_item` `fields` object a real write form on create/update — reads return `fields` as a native object, and writing that shape back was a silent no-op (accepted, never mapped, dropped while the PATCH still bumped `updated_at`) — merging it into the same path as `field`/the dedicated params with conflicting duplicate keys refused, and made input validation strict across all catalog tools: undeclared top-level keys now fail with a structured error instead of being silently dropped (#1066); v0.25 made `pad_library.activate` resolve its DESTINATION collection from the target’s declared artifact kind (SPEC-5 collection traits) rather than the literal `conventions` / `playbooks` slugs, so activating into a workspace that renamed either collection lands correctly instead of failing not-found with the collection sitting right there (BUG-2702); a lookup ERROR is now surfaced rather than silently falling back to the canonical slug, because falling back on an error means writing to a slug nothing was confirmed about (TASK-2657); v0.26 made `pad_workspace.create` REFUSE with a 403 when the calling OAuth connection's grant has `may_create_workspaces=false` — that checkbox previously gated only the post-creation auto-add, so a connection whose user declined it could still create workspaces — and on a connection with an explicit workspace allow-list, could not then see them (a wildcard `all_current_workspaces` connection could, which is why the consent mismatch rather than the invisibility is the defect); the same gate covers `POST /workspaces/import`, which mints a workspace through a second door. There is deliberately no escape-hatch parameter: the gate expresses the USER's consent decision, so only the user can lift it — by re-authorizing, or by enabling the flag on the existing connection at `/console/connected-apps` (IDEA-2756); v0.27 typed field values server-side so a declared number/json field is writable from the remote transport at all, carried the `fields` object with its JSON types intact, named undeclared keys back in `warnings.undeclared_fields` (accepted rather than refused — a census of 1012 items found 14 such keys across 168 live values, so refusing would have broken read-modify-write on items nobody had edited wrongly), and replaced the accreted per-site conflict guards with ONE check over a canonical view of every source; that check refuses several ambiguities v0.26 resolved silently, chiefly two names for one target in a single call (`parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id`), refused even when the values match because the names address one thing through incomparable vocabularies and the two doors resolved them differently (BUG-2850); v0.28 added two ADDITIVE `pad_item` actions — `remind`, which arms a one-shot reminder at an RFC3339 instant (`remind_at`), and `ack-reminder`, which acknowledges a fired one by id (`reminder_id`); a bare `YYYY-MM-DD` is refused rather than read as midnight, since a date names a 24-hour span and picking an hour inside it would be the server choosing a time nobody did (IDEA-2641); v0.29 made a `relation` field value have to NAME A LIVE ITEM in the collection that field declares — `internal/items` only ever checked the SHAPE of a relation ("must be a string"), because deciding whether a string names an item is a database question and that package is DB-free, so any string at all was accepted and stored and no client could render it honestly; every write door now refuses a value that names nothing, names an item in the WRONG collection, sits in a field whose schema declares no target collection, or is a SLUG (a deliberate divergence from `ResolveItem`: a slug is neither an ID nor stable, and free text like "red" resolving to whatever is slugged `red` today is exactly the corruption this closes). A CARRIED value — one already on the item, asserted by nobody — is never refused, because refusing would make every legacy item un-updatable, un-movable and un-copyable: within a workspace it resolves and survives, across a workspace boundary it is dropped without a lookup and reported in `warnings.dropped_fields`, so `pad_item.action=copy` now names a drop where v0.28 silently landed a dangling reference (PLAN-2857 / TASK-2878); v0.30 made one `--field key=value` entry mean ONE thing at every door — six sites parsed it independently (`item create`, `item list`, `item update`, `item move`, `item copy`, and the remote door’s own ingest) in four spellings, so `field:[" effort=l"]` stored an undeclared field literally named " effort" through the CLI and wrote `effort` through /mcp, the same call storing two different keys depending only on the transport. All six now share one parse, with two deliberately asymmetric rules: a padded KEY is REFUSED everywhere rather than trimmed anywhere (trimming silently retargets the write to a field the caller did not type), and a VALUE is carried VERBATIM everywhere (trimming reinterprets a caller’s bytes, and on a text field the padding is content) — a padded value against a typed field is refused one layer down by validation, naming the field, which is the same answer at both doors since v0.27 types declared fields server-side. Both doors refuse something they used to accept, and they were accepting it differently — /mcp trimmed the padded key and wrote the declared field, the CLI stored a ghost field beside it; what is /mcp-only is the value half, which it used to trim and type and now passes through to the same validation the CLI has always applied. A caller writing canonical entries sees no difference at either door (BUG-2870); v0.31 let a `relation` field value be an EXACT TITLE, scoped to the collection that field declares, alongside the UUID and ref v0.29 accepted — a THIRD accepted spelling, so nothing a v0.30 caller sends stops working — added a `relation_targets` member to reads hydrating each stored id to `{id, ref, title}` so a client can render a relation without a request per value, and made the `fields` DSL take the target collection slug as a relation's third part. The bump is owed by that last part: `fields="owner:relation"` used to be ACCEPTED and is now REFUSED, because the old parser put the third part into Options for every type and so built a relation with no target — a field every subsequent write then refused with `target_missing`, which is to say a field that could never accept a value. Title resolution is COLLECTION-SCOPED on purpose: a title unique only workspace-wide refuses and names the collection searched, and two matches inside the declared collection get their own `ambiguous` reason rather than `not_found`, which would state the opposite of what happened. The ladder is UUID, then ref, then title, so an item literally TITLED "COLO-3" is unreachable by title while the ref resolves. An id-only `relation_targets` entry means the target is gone OR the caller may not see it, and consumers must not render it as either — the two are made indistinguishable deliberately (PLAN-2857 / TASK-2996); v0.33 added a `multi_relation` field type — an ORDERED LIST of references, each element resolving through the same UUID → ref → exact-title ladder a scalar relation uses, declared through the DSL as `owners:multi_relation:people` with the target collection as its third part (a bare `owners:multi_relation` is refused at parse time rather than minting a field no write can satisfy, exactly as v0.31 did for `relation`). The type itself is additive — every rule it introduces is reachable only through a field whose schema says `multi_relation`, which no existing schema can contain — but the READ shape is why this is a bump: `relation_targets[key]` now carries EITHER the scalar object v0.31 emitted OR a JSON ARRAY of those objects, in stored order and one per stored element, when the key names a `multi_relation`. A scalar entry is byte-identical to v0.31's, so a consumer with no such fields sees nothing new; one that acquires such a field must handle the array, and a strictly-typed client will fail to decode it. Value rules, all NEW rather than inherited because scalar `relation` had nothing coherent to inherit (BUG-3028): exactly ONE stored form for "none", the key being ABSENT, with `[]` normalising to it; an empty or whitespace-only ELEMENT refused rather than skipped; `required` meaning at least one RESOLVED element; order part of the value; and duplicates refused with a new `duplicate_referent` reason, detected AFTER resolution because two elements naming one item are usually different strings — a UUID and a ref, a ref and a title. Any failing element refuses the WHOLE write, and a cross-workspace copy drops the value whole and reports it once, so an import can never change an element count (PLAN-2857 / TASK-2999); v0.34 added an `expected_seq` param to `pad_item.action=update` — the STRONG optimistic-concurrency token, preferred over `expected_updated_at`, which is stored at one-second resolution and therefore cannot tell two writes inside one second apart: both match it, neither conflicts, and the loser silently overwrites the winner. `seq` is bumped on every mutation of the row under the write lock, and is now serialised on every item AND every item summary (without `omitempty`) so the shape a caller reads always carries the token it must send back. Raising `updated_at`'s resolution instead was measured and rejected: the column is TEXT on both dialects and compared lexically in SQL, and Go's RFC3339Nano omits trailing zeros, so a sub-second value sorts BEFORE a whole-second value of the same second and the since-cursor read would start skipping rows (BUG-3037); ; v0.35 made an injected schema DEFAULT take the same `validateFieldType` check a caller-supplied value takes. It used to be assigned and skipped past that check, so the SAME BYTES were refused through one door and stored through the other, decided only by who put them there: on a `status` retyped to `multi_select` whose scalar default survived the retype, `{"status":"open"}` answered 400 and `{}` answered 201 storing `{"status":"open"}` — a scalar sitting in a list field, which is the corruption BUG-3016, BUG-3057, BUG-3067, BUG-3068 and BUG-3074 have each been defending one surface at a time against. A failing default is now DISCARDED and named in `warnings.dropped_fields` rather than stored, which widens that key from the single relation case v0.29 introduced to a field of any type. Dropped rather than refused because nobody in the request typed the value and refusing would make every write into that collection fail on a schema defect its author must fix elsewhere; the one exception is a REQUIRED field, where dropping leaves it absent and the write is refused as required, with a message naming the default as the cause rather than a bare "field is required" that points at a request which never mentioned it (BUG-3079); v0.36 stopped a REF-SHAPED `relation` value falling back to matching by item NUMBER when its prefix names a collection that is LIVE in the workspace. Item numbers are workspace-unique and sequential ACROSS collections, so `CONVE-1` and `SECRE-1` are never both real — and the fallback dropped the prefix, matched on the number alone, and found an item that was usually inside the field’s declared collection, so the wrong-collection check above it passed and nothing was raised: a caller wrote `CONVE-1`, the item stored a different item’s UUID, and the write answered 201 with the substitution visible only in `relation_targets`. That is the corruption v0.29 closed for free text, left open for the spelling the docs tell callers to prefer. The predicate is about the PREFIX rather than the item because a collection RENAME changes its prefix and a relation already written as the old ref has to keep resolving (BUG-2873): that ref’s prefix names nothing afterwards, so the fallback still fires for it, while a prefix that IS live and simply has no item at that number is the caller naming a real collection that does not contain what they said, and is refused. Residual, stated rather than implied: a ref pasted from ANOTHER workspace whose prefix also names no collection here still resolves by number, because nothing in the value distinguishes it from a rename’s leftover. `GetItemByRef`, the navigational read path, is unchanged (BUG-3082); v0.37 added the opt-in `agent` projection to `pad_item.get`; v0.38 added two ADDITIVE `pad_item` actions from PR #1238 (b4rk13) — `claim`, an atomic lease on an item (`holder`, `ttl` as a Go duration; 409 `lease_held` carrying the holder and expiry when another live holder has it, a re-claim by the live holder extends the expiry), and `release`, which drops it idempotently; a `ttl` under one second is refused at the mapper because whole seconds cross the wire and a positive sub-second value truncated to the `ttl_seconds: 0` the server reads as absent (IDEA-2815); v0.39 refuses a `pad_item.update` that sets content WITH a version token (`expected_seq` / `expected_updated_at`) while a browser tab holds edits not yet saved back, with 409 `content_pending_flush` — the token guards the stored row and those edits are not in it, so the write used to replace them and, with no tab open, delete them outright. Re-reading does not clear it; the new `overwrite_pending_edits` boolean does, and a write without a token is unchanged apart from `warnings.pruned_pending_edits` naming how many such edits it deleted (BUG-3133); v0.40 gives a scalar `relation` field one stored form for "no target", the key absent: a blank a write SETS is normalised to absent before validation, so a REQUIRED relation can no longer be satisfied by `""`, while a legacy blank a write only carries is normalised without refusing it, and an empty list filter on a relation (`owner=`) matches every spelling of none (BUG-3028); v0.41 adds a `match` action to `pad_playbook` — given free `text`, a Choice question over the workspace's ACTIVE playbooks (ref/title/summary/trigger/invocation_slug) plus a reserved `"none"` option returns the best match's ref (or `"none"`), confidence, per-option probabilities and the model; no provider configured answers 404 `decision_provider_unavailable`, a provider error or an out-of-set answer answers 502 `decision_provider_error`; pure ADDITIVE bump, read-only and side-effect-free (PLAN-3114 / TASK-3120); v0.42 changes the error code a LOCAL STDIO caller receives: an argv refusal is `validation_failed` with its message, a 429 is `rate_limited` with `details.retry_after_seconds`, and free text starting with `-` is no longer parsed as a flag, since positionals now follow `--` (BUG-3142 / BUG-3147); v0.43 makes `pad_item.action=move` refuse a field override the destination schema does not declare, with 400 `malformed_override`, as the copy does (BUG-2379); v0.44 makes `pad_item.action=bulk-update` over WebMCP refuse, per item into `failed`, a status or priority change on an item whose collection does not declare that field, while stdio and remote, which issue a per-item PATCH, still accepted it with `warnings.undeclared_fields` (BUG-3154) until v0.46 made them refuse it too, per item, via the update door's new `refuse_undeclared_fields` member (BUG-3156); v0.45 reports an HTTP 413 as `too_large` on both transports, with the server's own code in `details.reason`, where it used to be `server_error` (BUG-2829); v0.48 makes `pad_item.action=create` refuse a `field` naming any reserved metadata key, as update already did (BUG-3163); v0.49 makes `pad_project.action=report` with no `collections` param leave out system collections (Conventions, Playbooks), keyed on `is_system`, while naming one still includes it (BUG-2410); v0.50 adds an additive `stored_as_text: true` to a `relation_targets` entry whose stored value is not UUID-shaped (an imported title, legacy text), so it renders as the text it is rather than as an unavailable target, while a UUID-shaped entry is byte-identical to v0.49 (BUG-3014); v0.51 stops a move or copy carrying a value into a COMPUTED field or a value that collides on a destination unique field — a carried collision is dropped and named in `warnings.not_unique`, a supplied or default one is refused 409 (BUG-2367); v0.52 refuses a move or copy that would change an item between open, done and abandoned unless the destination done field is named (`state_change_requires_value`; a `state_change` needs_value row on the preflight) (BUG-2367); v0.53 adds `filename_source` to every `pad_attachment.list` row (BUG-2819); v0.54 keeps a `fields` number's literal through MCP, so an integer above 2^53 is no longer rounded, and refuses a `fields` number beside a `field` entry for the same key when the two differ as exact numbers — a pair that used to collapse into one rounded write (BUG-3217); v0.55 resolves `pad_item.search`'s `collection` exact-first per workspace on the server, so where both `task` and `tasks` exist `task` means `task`, and every other workspace answers as before (BUG-2659); v0.56 `content_state` gains `superseded_set_aside`, edits an editor upgrade set aside that opening the item will not restore (BUG-3244); see `internal/mcp/version.go` for the full changelog) +- `tool_surface_version: "0.57"` — MCP tool catalog contract (v0.5 added `pad_library`; v0.6 `pad_item.backlinks`; v0.7 `pad_item` `export`/`import`; v0.8 `pad_workspace` `deleted`/`restore`; v0.9 made `pad_item.list` summary-shaped by default with a default+max result cap; v0.10 enforced the draft-playbook gate server-side on `pad_playbook.run` with an `allow_draft` escape hatch; v0.11 added the read-only `pad_attachment` tool (`list`/`show`); v0.12 added `pad_project.activity` (agent-accessible non-streaming activity feed); v0.13 added `pad_project` `ready`/`stale` (agent-oriented backlog + attention queries); v0.14 added `pad_item` `history` + optimistic concurrency (TASK-2022); v0.15 added the `pad_item.list` `unparented` parameter (TASK-2096); v0.16 made an empty-string `assigned_user_id` / `agent_role_id` CLEAR the assignment instead of being silently dropped, so an agent can finally unassign an item (TASK-2571); v0.17 carried that to the LOCAL STDIO transport by teaching the CLI to lift those keys onto their columns instead of into the fields blob (BUG-2583); v0.18 added `clear_assigned_user` / `clear_agent_role` booleans — the canonical, schema-discoverable way to unassign, backed by new `--clear-assigned-user` / `--clear-agent-role` flags on `pad item update` (IDEA-2584); v0.19 added a `clear_parent` boolean — the canonical, schema-discoverable way to detach an item from its parent, backed by a new `--clear-parent` flag on `pad item update` (BUG-2078); v0.20 gave every tool an explicit annotation block derived from the catalog’s read-only knowledge — fully-read-only tools advertise `readOnlyHint: true` / `destructiveHint: false`, all-additive-write tools (`pad_workspace`, `pad_library`) drop `destructiveHint`, overwrite/delete-capable tools stay conservatively destructive, `openWorldHint: false` everywhere — replacing mcp-go’s defaults that marked every tool destructive (BUG-2302), and made `pad_item.list` summary-shaped on the remote HTTP transport too, with a declared `full` boolean as the opt-in for complete bodies on both transports (BUG-2305); v0.21 bounded `pad_item.history`, which was unbounded on every surface — `limit` now covers it (default 50, max 300, the NEWEST N; no `offset`, because reverse-patch storage makes only a newest-end window cheap), applied in the catalog action so it lands on both transports, and summary mode now asks the server to skip patch resolution rather than resolving bodies the dispatcher discards (BUG-2608); v0.22 stopped `pad_item.move` destroying an item’s system metadata — implementation notes, decision log, linked PR and convention data now survive a move, any field the destination schema has no home for is REPORTED in the move’s activity entry rather than vanishing, and a `field` setter naming one of those reserved keys is refused with `malformed_override` instead of writing it (BUG-2674); v0.23 closed the same door on the ordinary update — a `field` setter naming `implementation_notes`, `decision_log` or `convention` is now refused on every transport at once (`validation_error` on HTTP, surfaced to MCP clients as `validation_failed`); the one gate covers the CLI, remote MCP and stdio MCP at once because all three lower a `field` setter into the same `fields_patch`; `github_pr` was exempt on update until v0.47 refused it too, because that door stored the PR as an unreadable string (BUG-2696), and `pad github link` / `unlink` now write it through a typed, validated update member; item CREATE stayed open until v0.48 refused the keys there too (BUG-3163), once library activation, the one system writer using that door, moved to a typed `convention` create member. v0.23 also added the retry-hostile `stored_state_unreadable` error code so an agent told its target item’s stored data is unreadable stops instead of retrying a permanent failure (BUG-2627 / BUG-2675); v0.24 made the `pad_item` `fields` object a real write form on create/update — reads return `fields` as a native object, and writing that shape back was a silent no-op (accepted, never mapped, dropped while the PATCH still bumped `updated_at`) — merging it into the same path as `field`/the dedicated params with conflicting duplicate keys refused, and made input validation strict across all catalog tools: undeclared top-level keys now fail with a structured error instead of being silently dropped (#1066); v0.25 made `pad_library.activate` resolve its DESTINATION collection from the target’s declared artifact kind (SPEC-5 collection traits) rather than the literal `conventions` / `playbooks` slugs, so activating into a workspace that renamed either collection lands correctly instead of failing not-found with the collection sitting right there (BUG-2702); a lookup ERROR is now surfaced rather than silently falling back to the canonical slug, because falling back on an error means writing to a slug nothing was confirmed about (TASK-2657); v0.26 made `pad_workspace.create` REFUSE with a 403 when the calling OAuth connection's grant has `may_create_workspaces=false` — that checkbox previously gated only the post-creation auto-add, so a connection whose user declined it could still create workspaces — and on a connection with an explicit workspace allow-list, could not then see them (a wildcard `all_current_workspaces` connection could, which is why the consent mismatch rather than the invisibility is the defect); the same gate covers `POST /workspaces/import`, which mints a workspace through a second door. There is deliberately no escape-hatch parameter: the gate expresses the USER's consent decision, so only the user can lift it — by re-authorizing, or by enabling the flag on the existing connection at `/console/connected-apps` (IDEA-2756); v0.27 typed field values server-side so a declared number/json field is writable from the remote transport at all, carried the `fields` object with its JSON types intact, named undeclared keys back in `warnings.undeclared_fields` (accepted rather than refused — a census of 1012 items found 14 such keys across 168 live values, so refusing would have broken read-modify-write on items nobody had edited wrongly), and replaced the accreted per-site conflict guards with ONE check over a canonical view of every source; that check refuses several ambiguities v0.26 resolved silently, chiefly two names for one target in a single call (`parent`/`plan`, `assign`/`assigned_user_id`, `role`/`agent_role_id`), refused even when the values match because the names address one thing through incomparable vocabularies and the two doors resolved them differently (BUG-2850); v0.28 added two ADDITIVE `pad_item` actions — `remind`, which arms a one-shot reminder at an RFC3339 instant (`remind_at`), and `ack-reminder`, which acknowledges a fired one by id (`reminder_id`); a bare `YYYY-MM-DD` is refused rather than read as midnight, since a date names a 24-hour span and picking an hour inside it would be the server choosing a time nobody did (IDEA-2641); v0.29 made a `relation` field value have to NAME A LIVE ITEM in the collection that field declares — `internal/items` only ever checked the SHAPE of a relation ("must be a string"), because deciding whether a string names an item is a database question and that package is DB-free, so any string at all was accepted and stored and no client could render it honestly; every write door now refuses a value that names nothing, names an item in the WRONG collection, sits in a field whose schema declares no target collection, or is a SLUG (a deliberate divergence from `ResolveItem`: a slug is neither an ID nor stable, and free text like "red" resolving to whatever is slugged `red` today is exactly the corruption this closes). A CARRIED value — one already on the item, asserted by nobody — is never refused, because refusing would make every legacy item un-updatable, un-movable and un-copyable: within a workspace it resolves and survives, across a workspace boundary it is dropped without a lookup and reported in `warnings.dropped_fields`, so `pad_item.action=copy` now names a drop where v0.28 silently landed a dangling reference (PLAN-2857 / TASK-2878); v0.30 made one `--field key=value` entry mean ONE thing at every door — six sites parsed it independently (`item create`, `item list`, `item update`, `item move`, `item copy`, and the remote door’s own ingest) in four spellings, so `field:[" effort=l"]` stored an undeclared field literally named " effort" through the CLI and wrote `effort` through /mcp, the same call storing two different keys depending only on the transport. All six now share one parse, with two deliberately asymmetric rules: a padded KEY is REFUSED everywhere rather than trimmed anywhere (trimming silently retargets the write to a field the caller did not type), and a VALUE is carried VERBATIM everywhere (trimming reinterprets a caller’s bytes, and on a text field the padding is content) — a padded value against a typed field is refused one layer down by validation, naming the field, which is the same answer at both doors since v0.27 types declared fields server-side. Both doors refuse something they used to accept, and they were accepting it differently — /mcp trimmed the padded key and wrote the declared field, the CLI stored a ghost field beside it; what is /mcp-only is the value half, which it used to trim and type and now passes through to the same validation the CLI has always applied. A caller writing canonical entries sees no difference at either door (BUG-2870); v0.31 let a `relation` field value be an EXACT TITLE, scoped to the collection that field declares, alongside the UUID and ref v0.29 accepted — a THIRD accepted spelling, so nothing a v0.30 caller sends stops working — added a `relation_targets` member to reads hydrating each stored id to `{id, ref, title}` so a client can render a relation without a request per value, and made the `fields` DSL take the target collection slug as a relation's third part. The bump is owed by that last part: `fields="owner:relation"` used to be ACCEPTED and is now REFUSED, because the old parser put the third part into Options for every type and so built a relation with no target — a field every subsequent write then refused with `target_missing`, which is to say a field that could never accept a value. Title resolution is COLLECTION-SCOPED on purpose: a title unique only workspace-wide refuses and names the collection searched, and two matches inside the declared collection get their own `ambiguous` reason rather than `not_found`, which would state the opposite of what happened. The ladder is UUID, then ref, then title, so an item literally TITLED "COLO-3" is unreachable by title while the ref resolves. An id-only `relation_targets` entry means the target is gone OR the caller may not see it, and consumers must not render it as either — the two are made indistinguishable deliberately (PLAN-2857 / TASK-2996); v0.33 added a `multi_relation` field type — an ORDERED LIST of references, each element resolving through the same UUID → ref → exact-title ladder a scalar relation uses, declared through the DSL as `owners:multi_relation:people` with the target collection as its third part (a bare `owners:multi_relation` is refused at parse time rather than minting a field no write can satisfy, exactly as v0.31 did for `relation`). The type itself is additive — every rule it introduces is reachable only through a field whose schema says `multi_relation`, which no existing schema can contain — but the READ shape is why this is a bump: `relation_targets[key]` now carries EITHER the scalar object v0.31 emitted OR a JSON ARRAY of those objects, in stored order and one per stored element, when the key names a `multi_relation`. A scalar entry is byte-identical to v0.31's, so a consumer with no such fields sees nothing new; one that acquires such a field must handle the array, and a strictly-typed client will fail to decode it. Value rules, all NEW rather than inherited because scalar `relation` had nothing coherent to inherit (BUG-3028): exactly ONE stored form for "none", the key being ABSENT, with `[]` normalising to it; an empty or whitespace-only ELEMENT refused rather than skipped; `required` meaning at least one RESOLVED element; order part of the value; and duplicates refused with a new `duplicate_referent` reason, detected AFTER resolution because two elements naming one item are usually different strings — a UUID and a ref, a ref and a title. Any failing element refuses the WHOLE write, and a cross-workspace copy drops the value whole and reports it once, so an import can never change an element count (PLAN-2857 / TASK-2999); v0.34 added an `expected_seq` param to `pad_item.action=update` — the STRONG optimistic-concurrency token, preferred over `expected_updated_at`, which is stored at one-second resolution and therefore cannot tell two writes inside one second apart: both match it, neither conflicts, and the loser silently overwrites the winner. `seq` is bumped on every mutation of the row under the write lock, and is now serialised on every item AND every item summary (without `omitempty`) so the shape a caller reads always carries the token it must send back. Raising `updated_at`'s resolution instead was measured and rejected: the column is TEXT on both dialects and compared lexically in SQL, and Go's RFC3339Nano omits trailing zeros, so a sub-second value sorts BEFORE a whole-second value of the same second and the since-cursor read would start skipping rows (BUG-3037); ; v0.35 made an injected schema DEFAULT take the same `validateFieldType` check a caller-supplied value takes. It used to be assigned and skipped past that check, so the SAME BYTES were refused through one door and stored through the other, decided only by who put them there: on a `status` retyped to `multi_select` whose scalar default survived the retype, `{"status":"open"}` answered 400 and `{}` answered 201 storing `{"status":"open"}` — a scalar sitting in a list field, which is the corruption BUG-3016, BUG-3057, BUG-3067, BUG-3068 and BUG-3074 have each been defending one surface at a time against. A failing default is now DISCARDED and named in `warnings.dropped_fields` rather than stored, which widens that key from the single relation case v0.29 introduced to a field of any type. Dropped rather than refused because nobody in the request typed the value and refusing would make every write into that collection fail on a schema defect its author must fix elsewhere; the one exception is a REQUIRED field, where dropping leaves it absent and the write is refused as required, with a message naming the default as the cause rather than a bare "field is required" that points at a request which never mentioned it (BUG-3079); v0.36 stopped a REF-SHAPED `relation` value falling back to matching by item NUMBER when its prefix names a collection that is LIVE in the workspace. Item numbers are workspace-unique and sequential ACROSS collections, so `CONVE-1` and `SECRE-1` are never both real — and the fallback dropped the prefix, matched on the number alone, and found an item that was usually inside the field’s declared collection, so the wrong-collection check above it passed and nothing was raised: a caller wrote `CONVE-1`, the item stored a different item’s UUID, and the write answered 201 with the substitution visible only in `relation_targets`. That is the corruption v0.29 closed for free text, left open for the spelling the docs tell callers to prefer. The predicate is about the PREFIX rather than the item because a collection RENAME changes its prefix and a relation already written as the old ref has to keep resolving (BUG-2873): that ref’s prefix names nothing afterwards, so the fallback still fires for it, while a prefix that IS live and simply has no item at that number is the caller naming a real collection that does not contain what they said, and is refused. Residual, stated rather than implied: a ref pasted from ANOTHER workspace whose prefix also names no collection here still resolves by number, because nothing in the value distinguishes it from a rename’s leftover. `GetItemByRef`, the navigational read path, is unchanged (BUG-3082); v0.37 added the opt-in `agent` projection to `pad_item.get`; v0.38 added two ADDITIVE `pad_item` actions from PR #1238 (b4rk13) — `claim`, an atomic lease on an item (`holder`, `ttl` as a Go duration; 409 `lease_held` carrying the holder and expiry when another live holder has it, a re-claim by the live holder extends the expiry), and `release`, which drops it idempotently; a `ttl` under one second is refused at the mapper because whole seconds cross the wire and a positive sub-second value truncated to the `ttl_seconds: 0` the server reads as absent (IDEA-2815); v0.39 refuses a `pad_item.update` that sets content WITH a version token (`expected_seq` / `expected_updated_at`) while a browser tab holds edits not yet saved back, with 409 `content_pending_flush` — the token guards the stored row and those edits are not in it, so the write used to replace them and, with no tab open, delete them outright. Re-reading does not clear it; the new `overwrite_pending_edits` boolean does, and a write without a token is unchanged apart from `warnings.pruned_pending_edits` naming how many such edits it deleted (BUG-3133); v0.40 gives a scalar `relation` field one stored form for "no target", the key absent: a blank a write SETS is normalised to absent before validation, so a REQUIRED relation can no longer be satisfied by `""`, while a legacy blank a write only carries is normalised without refusing it, and an empty list filter on a relation (`owner=`) matches every spelling of none (BUG-3028); v0.41 adds a `match` action to `pad_playbook` — given free `text`, a Choice question over the workspace's ACTIVE playbooks (ref/title/summary/trigger/invocation_slug) plus a reserved `"none"` option returns the best match's ref (or `"none"`), confidence, per-option probabilities and the model; no provider configured answers 404 `decision_provider_unavailable`, a provider error or an out-of-set answer answers 502 `decision_provider_error`; pure ADDITIVE bump, read-only and side-effect-free (PLAN-3114 / TASK-3120); v0.42 changes the error code a LOCAL STDIO caller receives: an argv refusal is `validation_failed` with its message, a 429 is `rate_limited` with `details.retry_after_seconds`, and free text starting with `-` is no longer parsed as a flag, since positionals now follow `--` (BUG-3142 / BUG-3147); v0.43 makes `pad_item.action=move` refuse a field override the destination schema does not declare, with 400 `malformed_override`, as the copy does (BUG-2379); v0.44 makes `pad_item.action=bulk-update` over WebMCP refuse, per item into `failed`, a status or priority change on an item whose collection does not declare that field, while stdio and remote, which issue a per-item PATCH, still accepted it with `warnings.undeclared_fields` (BUG-3154) until v0.46 made them refuse it too, per item, via the update door's new `refuse_undeclared_fields` member (BUG-3156); v0.45 reports an HTTP 413 as `too_large` on both transports, with the server's own code in `details.reason`, where it used to be `server_error` (BUG-2829); v0.48 makes `pad_item.action=create` refuse a `field` naming any reserved metadata key, as update already did (BUG-3163); v0.49 makes `pad_project.action=report` with no `collections` param leave out system collections (Conventions, Playbooks), keyed on `is_system`, while naming one still includes it (BUG-2410); v0.50 adds an additive `stored_as_text: true` to a `relation_targets` entry whose stored value is not UUID-shaped (an imported title, legacy text), so it renders as the text it is rather than as an unavailable target, while a UUID-shaped entry is byte-identical to v0.49 (BUG-3014); v0.51 stops a move or copy carrying a value into a COMPUTED field or a value that collides on a destination unique field — a carried collision is dropped and named in `warnings.not_unique`, a supplied or default one is refused 409 (BUG-2367); v0.52 refuses a move or copy that would change an item between open, done and abandoned unless the destination done field is named (`state_change_requires_value`; a `state_change` needs_value row on the preflight) (BUG-2367); v0.53 adds `filename_source` to every `pad_attachment.list` row (BUG-2819); v0.54 keeps a `fields` number's literal through MCP, so an integer above 2^53 is no longer rounded, and refuses a `fields` number beside a `field` entry for the same key when the two differ as exact numbers — a pair that used to collapse into one rounded write (BUG-3217); v0.55 resolves `pad_item.search`'s `collection` exact-first per workspace on the server, so where both `task` and `tasks` exist `task` means `task`, and every other workspace answers as before (BUG-2659); v0.56 `content_state` gains `superseded_set_aside`, edits an editor upgrade set aside that opening the item will not restore (BUG-3244); v0.57 `pad_item` `edit-comment` / `delete-comment` and an `edited` flag on every comment (TASK-2695); see `internal/mcp/version.go` for the full changelog) v0.40 refuses a blank value on a REQUIRED relation field and stores an empty relation as an absent key; see `internal/mcp/version.go` for the full diff --git a/cmd/pad/cmd_item.go b/cmd/pad/cmd_item.go index ae4a81591..775ade9ce 100644 --- a/cmd/pad/cmd_item.go +++ b/cmd/pad/cmd_item.go @@ -3077,6 +3077,98 @@ func commentsCmd() *cobra.Command { } } +// errItemScopedCommentWritesUnsupported is returned before sending when the +// server does not advertise item_scoped_comment_writes (TASK-2695). An older +// build has no such route, and its bare 404 would read as "comment not found". +var errItemScopedCommentWritesUnsupported = errors.New("this server does not support editing or deleting comments from the CLI " + + "(it does not advertise item_scoped_comment_writes); upgrade the server, or use the web UI") + +func commentEditCmd() *cobra.Command { + var useStdin bool + + cmd := &cobra.Command{ + Use: "comment-edit [message]", + Short: "Replace the body of a comment on an item", + Long: `Replace the body of a comment on an item. + +Only the comment's author may edit it; an admin may too, but only from a +browser session. Get comment IDs from "pad item comments ". An edited +comment is shown with "edited" in that listing. + + pad item comment-edit TASK-5 3f2a... "corrected threshold is 0.7" + pad item comment-edit TASK-5 3f2a... --stdin < note.md`, + Args: func(cmd *cobra.Command, args []string) error { + if useStdin { + return cobra.ExactArgs(2)(cmd, args) + } + return cobra.ExactArgs(3)(cmd, args) + }, + RunE: func(cmd *cobra.Command, args []string) error { + var body string + if useStdin { + b, err := readStdinBody("comment edit") + if err != nil { + return err + } + body = b + } else { + body = args[2] + } + + client, _ := getClient() + ws := getWorkspace() + if !client.ServerSupportsItemScopedCommentWrites() { + return errItemScopedCommentWritesUnsupported + } + + comment, err := client.UpdateComment(ws, args[0], args[1], body) + if err != nil { + return err + } + + if formatFlag == "json" { + return cli.PrintJSON(comment) + } + + fmt.Printf("Comment %s on %s updated\n", args[1], args[0]) + return nil + }, + } + + cmd.Flags().BoolVar(&useStdin, "stdin", false, "read the new body from stdin instead of the message argument (a blank read is refused)") + return cmd +} + +func commentDeleteCmd() *cobra.Command { + return &cobra.Command{ + Use: "comment-delete ", + Short: "Delete a comment from an item", + Long: `Delete a comment from an item. + +Anyone who may edit the item may delete its comments. Get comment IDs from +"pad item comments ".`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + client, _ := getClient() + ws := getWorkspace() + if !client.ServerSupportsItemScopedCommentWrites() { + return errItemScopedCommentWritesUnsupported + } + + if err := client.DeleteComment(ws, args[0], args[1]); err != nil { + return err + } + + if formatFlag == "json" { + return cli.PrintJSON(map[string]any{"deleted": true, "ref": args[0], "comment_id": args[1]}) + } + + fmt.Printf("Comment %s deleted from %s\n", args[1], args[0]) + return nil + }, + } +} + // renderCommentsMarkdown is the markdown counterpart of cli.PrintCommentTable. // Comments are not tabular — the terminal form prints an attribution line then // the body — so the markdown form keeps that shape rather than forcing a table. @@ -3095,10 +3187,16 @@ func renderCommentsMarkdown(w io.Writer, comments []models.Comment) { if c.Author != "" && c.Author != c.CreatedBy { badge = c.Author + " (" + c.CreatedBy + ")" } - fmt.Fprintf(w, "**%s** · %s via %s\n\n", + edited := "" + if c.IsEdited() { + edited = " · edited" + } + fmt.Fprintf(w, "**%s** · %s via %s%s · id `%s`\n\n", cli.SanitizeMarkdownText(badge), cli.SanitizeMarkdownText(cli.RelativeTime(c.CreatedAt)), - cli.SanitizeMarkdownText(c.Source)) + cli.SanitizeMarkdownText(c.Source), + edited, + strings.ReplaceAll(c.ID, "`", "")) fmt.Fprintln(w, c.Body) if i < len(comments)-1 { fmt.Fprintln(w) @@ -4363,8 +4461,11 @@ func readStdinBody(verb string) (string, error) { } if strings.TrimSpace(string(data)) == "" { hint := "pass a body, or drop --stdin" - if verb == "update" { + switch verb { + case "update": hint = "pass a body, or use --clear-content to empty the body on purpose" + case "comment edit": + hint = "pass a body, or use comment-delete to remove the comment" } return "", fmt.Errorf("--stdin: the body read from stdin is empty or whitespace-only, so the %s was refused and nothing was sent (a lost heredoc looks like this); %s", verb, hint) } diff --git a/cmd/pad/comment_edit_delete_test.go b/cmd/pad/comment_edit_delete_test.go new file mode 100644 index 000000000..a001fd1a5 --- /dev/null +++ b/cmd/pad/comment_edit_delete_test.go @@ -0,0 +1,98 @@ +package main + +import ( + "encoding/json" + "net/http" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" +) + +// TASK-2695: `pad item comment-edit` / `comment-delete` send only to a server +// that advertises item_scoped_comment_writes. An older build has no such +// route and would answer a bare 404, which reads as "comment not found". +// Every leg counts the writes, since what the gate prevents is a write. + +type commentStub struct { + caps string // "new": advertises; "old": 404; "flaky": 500 + + mu sync.Mutex + writes []string // "METHOD path" + bodies []map[string]any +} + +func (s *commentStub) handler(w http.ResponseWriter, r *http.Request) { + s.mu.Lock() + defer s.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/server/capabilities") { + switch s.caps { + case "old": + http.NotFound(w, r) + case "flaky": + w.WriteHeader(http.StatusInternalServerError) + default: + _ = json.NewEncoder(w).Encode(map[string]any{"item_scoped_comment_writes": true}) + } + return + } + s.writes = append(s.writes, r.Method+" "+r.URL.Path) + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusNoContent) + return + } + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + s.bodies = append(s.bodies, body) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "c-1", "body": body["body"]}) +} + +func runCommentCmd(t *testing.T, stub *commentStub, cmd *cobra.Command, args ...string) error { + t.Helper() + setupPushTest(t, http.HandlerFunc(stub.handler)) + cmd.SetArgs(args) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + var err error + captureStdout(t, func() { err = cmd.Execute() }) + return err +} + +func TestCommentEditDelete_NewServer_ItemScopedRoute(t *testing.T) { + stub := &commentStub{caps: "new"} + if err := runCommentCmd(t, stub, commentEditCmd(), "TASK-5", "c-1", "fixed text"); err != nil { + t.Fatal(err) + } + if err := runCommentCmd(t, stub, commentDeleteCmd(), "TASK-5", "c-1"); err != nil { + t.Fatal(err) + } + if len(stub.writes) != 2 { + t.Fatalf("writes = %v, want one PATCH and one DELETE", stub.writes) + } + for i, method := range []string{"PATCH", "DELETE"} { + if !strings.HasPrefix(stub.writes[i], method+" ") || !strings.HasSuffix(stub.writes[i], "/items/TASK-5/comments/c-1") { + t.Errorf("write %d = %q, want %s to the item-scoped route", i, stub.writes[i], method) + } + } + if len(stub.bodies) != 1 || stub.bodies[0]["body"] != "fixed text" { + t.Errorf("PATCH body = %v, want {body: fixed text}", stub.bodies) + } +} + +func TestCommentEditDelete_UnadvertisedServer_SendsNothing(t *testing.T) { + for _, caps := range []string{"old", "flaky"} { + stub := &commentStub{caps: caps} + errEdit := runCommentCmd(t, stub, commentEditCmd(), "TASK-5", "c-1", "fixed text") + errDelete := runCommentCmd(t, stub, commentDeleteCmd(), "TASK-5", "c-1") + for name, err := range map[string]error{"edit": errEdit, "delete": errDelete} { + if err == nil || !strings.Contains(err.Error(), "item_scoped_comment_writes") { + t.Errorf("%s/%s: want the capability refusal, got %v", caps, name, err) + } + } + if len(stub.writes) != 0 { + t.Errorf("%s: sent %v to a server that did not advertise the routes", caps, stub.writes) + } + } +} diff --git a/cmd/pad/groups.go b/cmd/pad/groups.go index fecdea2a4..8eb23d3e0 100644 --- a/cmd/pad/groups.go +++ b/cmd/pad/groups.go @@ -124,6 +124,8 @@ func itemCmd() *cobra.Command { bulkUpdateCmd(), commentCmd(), commentsCmd(), + commentEditCmd(), + commentDeleteCmd(), remindCmd(), remindersCmd(), setAsideCmd(), diff --git a/internal/cli/client.go b/internal/cli/client.go index 734b89c7e..4aca9ab97 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -52,6 +52,7 @@ type serverCapabilityFlags struct { CollectionResolution bool `json:"collection_resolution"` ItemFieldAppend bool `json:"item_field_append"` SearchCollectionResolution bool `json:"search_collection_resolution"` + ItemScopedCommentWrites bool `json:"item_scoped_comment_writes"` } func NewClient(host string, port int) *Client { @@ -782,8 +783,35 @@ func (c *Client) CreateComment(wsSlug, itemSlug string, input models.CommentCrea return &result, wrapItemNotFound(err, itemSlug, wsSlug) } -func (c *Client) DeleteComment(wsSlug, commentID string) error { - return c.delete("/workspaces/" + wsSlug + "/comments/" + commentID) +// UpdateComment replaces a comment's body through the item-scoped route +// (TASK-2695), which refuses a comment that is not on itemRef. The server's +// not_found is passed through as-is: it already says whether the ITEM or the +// COMMENT was missing, and wrapItemNotFound would relabel both as the item. +func (c *Client) UpdateComment(wsSlug, itemRef, commentID, body string) (*models.Comment, error) { + var result models.Comment + err := c.patch(itemCommentPath(wsSlug, itemRef, commentID), map[string]string{"body": body}, &result) + if err != nil { + return nil, err + } + return &result, nil +} + +// DeleteComment removes a comment through the item-scoped route (TASK-2695). +func (c *Client) DeleteComment(wsSlug, itemRef, commentID string) error { + return c.delete(itemCommentPath(wsSlug, itemRef, commentID)) +} + +func itemCommentPath(wsSlug, itemRef, commentID string) string { + return "/workspaces/" + wsSlug + "/items/" + url.PathEscape(itemRef) + "/comments/" + url.PathEscape(commentID) +} + +// ServerSupportsItemScopedCommentWrites reports whether this server routes +// comment PATCH/DELETE under the item (TASK-2695). An indeterminate probe +// answers false: sending anyway would turn an older server's unrouted 404 +// into a false "comment not found". +func (c *Client) ServerSupportsItemScopedCommentWrites() bool { + caps, definitive := c.serverCapabilities() + return definitive && caps.ItemScopedCommentWrites } // --- Dashboard --- diff --git a/internal/cli/format.go b/internal/cli/format.go index 4640ff5fb..9a7feb9fd 100644 --- a/internal/cli/format.go +++ b/internal/cli/format.go @@ -644,7 +644,11 @@ func PrintCommentTable(comments []models.Comment) { if c.Author != "" && c.Author != c.CreatedBy { badge = c.Author + " (" + c.CreatedBy + ")" } - fmt.Printf("💬 %s • %s via %s\n", badge, RelativeTime(c.CreatedAt), c.Source) + edited := "" + if c.IsEdited() { + edited = " • edited" + } + fmt.Printf("💬 %s • %s via %s%s • id %s\n", badge, RelativeTime(c.CreatedAt), c.Source, edited, c.ID) fmt.Println(c.Body) if i < len(comments)-1 { fmt.Println() diff --git a/internal/mcp/catalog_item.go b/internal/mcp/catalog_item.go index a96f8d55e..b8ef167a3 100644 --- a/internal/mcp/catalog_item.go +++ b/internal/mcp/catalog_item.go @@ -107,6 +107,10 @@ var padItemTool = ToolDef{ // the `reply_to` parameter. "comment": passThrough([]string{"item", "comment"}), "list-comments": passThrough([]string{"item", "comments"}), + // TASK-2695: edit/delete ride the item-scoped comment routes, which + // refuse a comment_id that is not on `ref` on every transport. + "edit-comment": passThrough([]string{"item", "comment-edit"}), + "delete-comment": passThrough([]string{"item", "comment-delete"}), // Backlinks ("Mentioned in") — PLAN-1593 / TASK-1596. // Returns inbound `[[...]]` references to the given item. @@ -156,7 +160,7 @@ var padItemTool = ToolDef{ // keeping the schema simple to maintain. var padItemSchemaParams = []ParamDef{ // ── Targeting ── - {Name: "ref", Type: "string", Description: "Item reference (e.g. TASK-5, IDEA-12, PLAYB-3, CONVE-7). Required for: update, delete, restore, get, move, link, unlink, deps, star, unstar, comment, list-comments, note, decide, export, remind, claim, release. NOT used for ack-reminder (which addresses a REMINDER by `reminder_id`, since an item can carry several) and NOT used for bulk-update — pass `refs` (array) instead."}, + {Name: "ref", Type: "string", Description: "Item reference (e.g. TASK-5, IDEA-12, PLAYB-3, CONVE-7). Required for: update, delete, restore, get, move, link, unlink, deps, star, unstar, comment, list-comments, edit-comment, delete-comment, note, decide, export, remind, claim, release. NOT used for ack-reminder (which addresses a REMINDER by `reminder_id`, since an item can carry several) and NOT used for bulk-update — pass `refs` (array) instead."}, {Name: "refs", Type: "array", Description: "Item references for batch operations. Required for: bulk-update (one or more refs)."}, {Name: "target", Type: "string", Description: "The OTHER end of a relationship. Required for: link, unlink (paired with `ref` and `link_type`). For link_type=blocks, target is the item being blocked; for blocked-by it's the blocker; for supersedes it's the superseded item; etc."}, {Name: "link_type", Type: "string", Description: "Type of relationship for action=link/unlink.", Enum: []string{"blocks", "blocked-by", "supersedes", "implements", "split-from"}}, @@ -270,7 +274,8 @@ var padItemSchemaParams = []ParamDef{ {Name: "target_collection", Type: "string", Description: "Destination collection slug for action=move. Required for: move."}, // ── Comments ── - {Name: "message", Type: "string", Description: "Comment body. Required for: comment."}, + {Name: "message", Type: "string", Description: "Comment body. Required for: comment, edit-comment."}, + {Name: "comment_id", Type: "string", Description: "ID of a comment on `ref` (from list-comments). Required for: edit-comment, delete-comment."}, {Name: "reply_to", Type: "string", Description: "Parent comment ID for threading replies. Optional for: comment."}, {Name: "comment", Type: "string", Description: "Audit comment explaining the change. Optional for: update."}, @@ -411,8 +416,13 @@ Actions: comment — Add a comment. Required: ref, message. Optional: reply_to (comment ID for threaded reply). - list-comments — List comments on an item. + list-comments — List comments on an item. Each carries its id and + edited (true once its body has been changed). Required: ref. + edit-comment — Replace a comment's body. Only its author may. + Required: ref, comment_id, message. + delete-comment — Delete a comment. Anyone who may edit the item may. + Required: ref, comment_id. backlinks — List inbound [[...]] references to an item ("Mentioned in"). Required: ref. Optional: limit (default 50, max 300), offset. diff --git a/internal/mcp/catalog_item_fields_test.go b/internal/mcp/catalog_item_fields_test.go index a2d656d5e..4b5884357 100644 --- a/internal/mcp/catalog_item_fields_test.go +++ b/internal/mcp/catalog_item_fields_test.go @@ -1710,7 +1710,8 @@ func TestFieldConflictProperty_SourcesDerivedFromTheDeclaredSchema(t *testing.T) "summary": true, "details": true, // action=note "decision": true, "rationale": true, // action=decide "message": true, "reply_to": true, // action=comment - "comment": true, // the audit note on update + "comment_id": true, // action=edit-comment / delete-comment (TASK-2695): addresses a comment row, never an item field + "comment": true, // the audit note on update // action=remind / ack-reminder (IDEA-2641). Neither writes an item // FIELD: a reminder is a row in its own table addressed by its own diff --git a/internal/mcp/catalog_readonly_test.go b/internal/mcp/catalog_readonly_test.go index af09beb8d..cde6005c5 100644 --- a/internal/mcp/catalog_readonly_test.go +++ b/internal/mcp/catalog_readonly_test.go @@ -115,28 +115,30 @@ func TestReadOnlyCatalog_ActionsMatchCmdhelp(t *testing.T) { // pad_item passThrough actions. link/unlink are excluded — // they fan out to per-link_type cmdPaths and are tested in // catalog_item_test.go instead. - {"pad_item", "create"}: {"item", "create"}, - {"pad_item", "update"}: {"item", "update"}, - {"pad_item", "delete"}: {"item", "delete"}, - {"pad_item", "get"}: {"item", "show"}, - {"pad_item", "list"}: {"item", "list"}, - {"pad_item", "move"}: {"item", "move"}, - {"pad_item", "restore"}: {"item", "restore"}, - {"pad_item", "deps"}: {"item", "deps"}, - {"pad_item", "remind"}: {"item", "remind"}, - {"pad_item", "ack-reminder"}: {"item", "ack"}, - {"pad_item", "claim"}: {"item", "claim"}, - {"pad_item", "release"}: {"item", "release"}, - {"pad_item", "star"}: {"item", "star"}, - {"pad_item", "unstar"}: {"item", "unstar"}, - {"pad_item", "starred"}: {"item", "starred"}, - {"pad_item", "comment"}: {"item", "comment"}, - {"pad_item", "list-comments"}: {"item", "comments"}, - {"pad_item", "bulk-update"}: {"item", "bulk-update"}, - {"pad_item", "note"}: {"item", "note"}, - {"pad_item", "decide"}: {"item", "decide"}, - {"pad_item", "backlinks"}: {"item", "backlinks"}, - {"pad_item", "history"}: {"item", "history"}, + {"pad_item", "create"}: {"item", "create"}, + {"pad_item", "update"}: {"item", "update"}, + {"pad_item", "delete"}: {"item", "delete"}, + {"pad_item", "get"}: {"item", "show"}, + {"pad_item", "list"}: {"item", "list"}, + {"pad_item", "move"}: {"item", "move"}, + {"pad_item", "restore"}: {"item", "restore"}, + {"pad_item", "deps"}: {"item", "deps"}, + {"pad_item", "remind"}: {"item", "remind"}, + {"pad_item", "ack-reminder"}: {"item", "ack"}, + {"pad_item", "claim"}: {"item", "claim"}, + {"pad_item", "release"}: {"item", "release"}, + {"pad_item", "star"}: {"item", "star"}, + {"pad_item", "unstar"}: {"item", "unstar"}, + {"pad_item", "starred"}: {"item", "starred"}, + {"pad_item", "comment"}: {"item", "comment"}, + {"pad_item", "list-comments"}: {"item", "comments"}, + {"pad_item", "edit-comment"}: {"item", "comment-edit"}, + {"pad_item", "delete-comment"}: {"item", "comment-delete"}, + {"pad_item", "bulk-update"}: {"item", "bulk-update"}, + {"pad_item", "note"}: {"item", "note"}, + {"pad_item", "decide"}: {"item", "decide"}, + {"pad_item", "backlinks"}: {"item", "backlinks"}, + {"pad_item", "history"}: {"item", "history"}, // pad_playbook actions (PLAN-1377 / TASK-1381). All three // passThrough to `pad playbook `. @@ -275,27 +277,29 @@ func TestReadOnlyCatalog_ActionsDispatchExpectedCmdPath(t *testing.T) { // pad_item passThrough actions. link/unlink excluded — see // catalog_item_test.go for their dispatch test. - {"pad_item", "create"}: {"item", "create"}, - {"pad_item", "update"}: {"item", "update"}, - {"pad_item", "delete"}: {"item", "delete"}, - {"pad_item", "get"}: {"item", "show"}, - {"pad_item", "list"}: {"item", "list"}, - {"pad_item", "move"}: {"item", "move"}, - {"pad_item", "restore"}: {"item", "restore"}, - {"pad_item", "deps"}: {"item", "deps"}, - {"pad_item", "remind"}: {"item", "remind"}, - {"pad_item", "ack-reminder"}: {"item", "ack"}, - {"pad_item", "claim"}: {"item", "claim"}, - {"pad_item", "release"}: {"item", "release"}, - {"pad_item", "star"}: {"item", "star"}, - {"pad_item", "unstar"}: {"item", "unstar"}, - {"pad_item", "starred"}: {"item", "starred"}, - {"pad_item", "comment"}: {"item", "comment"}, - {"pad_item", "list-comments"}: {"item", "comments"}, - {"pad_item", "bulk-update"}: {"item", "bulk-update"}, - {"pad_item", "note"}: {"item", "note"}, - {"pad_item", "decide"}: {"item", "decide"}, - {"pad_item", "backlinks"}: {"item", "backlinks"}, + {"pad_item", "create"}: {"item", "create"}, + {"pad_item", "update"}: {"item", "update"}, + {"pad_item", "delete"}: {"item", "delete"}, + {"pad_item", "get"}: {"item", "show"}, + {"pad_item", "list"}: {"item", "list"}, + {"pad_item", "move"}: {"item", "move"}, + {"pad_item", "restore"}: {"item", "restore"}, + {"pad_item", "deps"}: {"item", "deps"}, + {"pad_item", "remind"}: {"item", "remind"}, + {"pad_item", "ack-reminder"}: {"item", "ack"}, + {"pad_item", "claim"}: {"item", "claim"}, + {"pad_item", "release"}: {"item", "release"}, + {"pad_item", "star"}: {"item", "star"}, + {"pad_item", "unstar"}: {"item", "unstar"}, + {"pad_item", "starred"}: {"item", "starred"}, + {"pad_item", "comment"}: {"item", "comment"}, + {"pad_item", "list-comments"}: {"item", "comments"}, + {"pad_item", "edit-comment"}: {"item", "comment-edit"}, + {"pad_item", "delete-comment"}: {"item", "comment-delete"}, + {"pad_item", "bulk-update"}: {"item", "bulk-update"}, + {"pad_item", "note"}: {"item", "note"}, + {"pad_item", "decide"}: {"item", "decide"}, + {"pad_item", "backlinks"}: {"item", "backlinks"}, // pad_playbook actions (PLAN-1377 / TASK-1381). {"pad_playbook", "list"}: {"playbook", "list"}, @@ -335,6 +339,9 @@ func TestReadOnlyCatalog_ActionsDispatchExpectedCmdPath(t *testing.T) { "remind_at": "2026-08-01T09:00:00Z", // pad_playbook.match needs a `text` positional (TASK-3120). "text": "test text", + // pad_item.edit-comment / delete-comment address a COMMENT on `ref` + // (TASK-2695). + "comment_id": "c-1", } for _, def := range Catalog { @@ -708,6 +715,16 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document { Args: mkArgs("ref"), Flags: mkFlags("workspace"), }, + "item comment-edit": { + Summary: "edit comment", + Args: append(mkArgs("ref", "comment-id"), cmdhelp.Arg{Name: "message"}), + Flags: mkFlags("workspace"), + }, + "item comment-delete": { + Summary: "delete comment", + Args: mkArgs("ref", "comment-id"), + Flags: mkFlags("workspace"), + }, "item backlinks": { Summary: "list inbound [[...]] references", Args: mkArgs("ref"), diff --git a/internal/mcp/dispatch_http.go b/internal/mcp/dispatch_http.go index cdea8d833..c69ee1586 100644 --- a/internal/mcp/dispatch_http.go +++ b/internal/mcp/dispatch_http.go @@ -453,6 +453,7 @@ func (d *HTTPHandlerDispatcher) buildSpecialRoutes() map[string]specialDispatchF "item note": d.dispatchItemNote, "item decide": d.dispatchItemDecide, "item import": d.dispatchItemImport, + "item comment-delete": d.dispatchItemCommentDelete, "project ready": d.dispatchProjectReady, "project stale": d.dispatchProjectStale, "project next": d.dispatchProjectNext, diff --git a/internal/mcp/dispatch_http_parity_test.go b/internal/mcp/dispatch_http_parity_test.go index 54fbc1e82..2cf26cd69 100644 --- a/internal/mcp/dispatch_http_parity_test.go +++ b/internal/mcp/dispatch_http_parity_test.go @@ -49,6 +49,7 @@ func parityFixtureInput() map[string]any { "title": "test title", "target_collection": "ideas", "message": "test message", + "comment_id": "c-1", "summary": "test summary", "decision": "test decision", "code": "123456", diff --git a/internal/mcp/dispatch_http_routes.go b/internal/mcp/dispatch_http_routes.go index 66359353f..83a250184 100644 --- a/internal/mcp/dispatch_http_routes.go +++ b/internal/mcp/dispatch_http_routes.go @@ -258,6 +258,10 @@ func init() { method: http.MethodGet, pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}/comments", }.toRouteMapper(), + // TASK-2695: the item-scoped route, so the comment-is-on-ref check + // is the server's on this transport as on the CLI's. Delete is a + // special route (dispatchItemCommentDelete) for its 204. + "item comment-edit": mapItemCommentEdit, // `item backlinks` (BUG-2304) — the reverse [[...]] index // (PLAN-1593). The CLI's --format json emits the endpoint's @@ -1714,6 +1718,66 @@ func cloneStringMap(m map[string]any) map[string]any { return out } +// itemCommentTarget reads the workspace, ref and comment_id every +// item-scoped comment write needs and builds its path (TASK-2695). +func itemCommentTarget(input map[string]any) (string, error) { + workspace, _ := input["workspace"].(string) + ref, _ := input["ref"].(string) + commentID, _ := input["comment_id"].(string) + if workspace == "" { + return "", fmt.Errorf("workspace is required") + } + if ref == "" { + return "", fmt.Errorf("ref is required") + } + if commentID == "" { + return "", fmt.Errorf("comment_id is required") + } + return fmt.Sprintf("/api/v1/workspaces/%s/items/%s/comments/%s", + url.PathEscape(workspace), url.PathEscape(ref), url.PathEscape(commentID)), nil +} + +// mapItemCommentEdit dispatches `pad item comment-edit +// ` as PATCH {body: }, the rename mapItemComment also makes. +func mapItemCommentEdit(input map[string]any) (string, string, []byte, error) { + urlPath, err := itemCommentTarget(input) + if err != nil { + return "", "", nil, err + } + message, _ := input["message"].(string) + if message == "" { + return "", "", nil, fmt.Errorf("message is required") + } + body, err := json.Marshal(map[string]any{"body": message}) + if err != nil { + return "", "", nil, fmt.Errorf("encode body: %w", err) + } + return http.MethodPatch, urlPath, body, nil +} + +// dispatchItemCommentDelete handles `pad item comment-delete +// `. The handler answers 204 with no body, which would reach the +// agent as an empty result, so a success is packaged as the CLI's +// `--format json` object instead (the dispatchDeleteItemLink precedent). +func (d *HTTPHandlerDispatcher) dispatchItemCommentDelete( + ctx context.Context, + input map[string]any, + user *models.User, +) (*mcp.CallToolResult, error) { + const cmdKey = "item comment-delete" + urlPath, err := itemCommentTarget(input) + if err != nil { + return validationFailedResult(cmdKey, err.Error(), "Pass ref and comment_id (ids come from list-comments)."), nil + } + res, err := d.executeRequest(ctx, cmdKey, user, http.MethodDelete, urlPath, nil) + if err != nil || res.IsError { + return res, err + } + ref, _ := input["ref"].(string) + commentID, _ := input["comment_id"].(string) + return packageStructuredResponse(cmdKey, map[string]any{"deleted": true, "ref": ref, "comment_id": commentID}) +} + // mapItemComment dispatches `pad item comment `. // // POST /api/v1/workspaces/{ws}/items/{ref}/comments with body shape diff --git a/internal/mcp/instructions.md b/internal/mcp/instructions.md index b8117f7b7..a756473bc 100644 --- a/internal/mcp/instructions.md +++ b/internal/mcp/instructions.md @@ -6,13 +6,13 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl If the user is asking general code questions with no project-management thread, you don't need this server. -## Tool surface (v0.56) +## Tool surface (v0.57) Ten resource × action tools, plus `pad_set_workspace`. Eleven tools total. Inputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped. -- `pad_item` — Items: create / update / delete / get / list / move / restore / link / unlink / deps / star / unstar / starred / comment / list-comments / backlinks / bulk-update / note / decide / export / import / history / remind / ack-reminder / claim / release. On create/update, field values may be passed as a `fields` OBJECT (the same shape reads return, e.g. `{"status":"done","effort":"l"}`) — it merges into the same path as the dedicated params and `field: ["key=value"]`; the same key in two places with CONFLICTING values is refused, not silently resolved. `list` accepts `unparented: true` to keep items with no parent or implements relationship (mutually exclusive with `parent`). `list` results are SUMMARY-shaped by default on both transports — no content bodies; pass `full: true` for complete bodies (token-expensive), or prefer `get` for a single item's body. `update` field writes are a server-side field-level merge (only the keys you set change); pass `expected_seq` for optimistic concurrency — the `seq` you last read, which is on every item AND every list summary — and a stale value fails with a structured 409 `update_conflict` whose details carry `actual_seq` to retry with. PREFER IT over `expected_updated_at`, which cannot tell two writes inside one second apart; that one still works, and if you send both, seq decides. A token cannot see a browser tab's unsaved edits, so `content` sent WITH one is refused with 409 `content_pending_flush` while any exist; re-reading will not clear it. Wait for the editor to save, or pass `overwrite_pending_edits: true` to replace them. `content_state: "superseded_set_aside"` is different and wins when both apply: an editor upgrade set aside edits no tab can restore, so EVERY content write is refused (token or not; `details.set_aside_rows`) and waiting never clears it. Tell the user, or pass `overwrite_pending_edits: true` to discard them. Writing `content` while a browser tab has the item open routes the markdown through that live document, and the stored copy is updated only by a later collab-snapshot flush — usually from the tab that applied it, though any such write updates the row, and nothing guarantees one happens: the 200 echoes the content you SENT and carries `warnings.content_outcome: "applied_pending_flush"`, which describes that WRITE — the content went to the document, not the row — rather than being a live reading of the row, which a concurrent flush may already have updated. A `get` (or a `list` with `full: true` — a default list carries no content at all) before such a flush lands reads the stored copy and answers with the PREVIOUS content, which is the lag rather than a failed write, so do not re-send on the strength of it. You do not have to infer that from the write, and you will also see it when someone ELSE's tab holds the unflushed edits and you wrote nothing: a read that serves a body the server knows is behind the live document carries `content_state: "applied_pending_flush"` — same value as the write warning, different question (what the ROW is, not what your REQUEST did). Treat it as re-read later, never as re-send, and do not wait on it in a loop: nothing server-side moves that content into the stored copy, so the row catches up only when a tab next opens the item, which may never happen. The stored form may also not be byte-identical to what you sent, since the markdown round-trips through the editor (bullet markers, emphasis characters, list numbering and blank-line runs normalise; already-canonical markdown survives unchanged), so compare on meaning rather than bytes. `move` changes an item's COLLECTION within its workspace: system metadata (implementation notes, decision log, linked PR) survives it, and any field value the target schema has no home for is dropped AND reported in the move's activity entry — check there rather than assuming a move is lossless. A carried unique-field collision is dropped, named in `warnings.not_unique`. Three system keys — `implementation_notes`, `decision_log`, `convention` — cannot be set through `field` on update or move; the call is refused with `validation_failed`, naming the key and the write path that does maintain it (`note`, `decide`, and library activation respectively). `github_pr` is refused as well (BUG-2696): a `field` value arrived as a string, so the PR data was stored double-encoded and no link appeared. Its writer is `pad github link` / `unlink`, which needs a local git checkout, and there is no PR-link action on this surface yet, so treat linking a PR as something to hand to a human rather than a call to retry. On `create` they are refused too (BUG-3163), with the same `validation_failed`: create the item first, then use `note` / `decide` on it; convention metadata comes from library activation. `history` returns read-only item version metadata (newest-first), bounded to the NEWEST 50 versions by default (max 300 — pass `limit` to change the window); pass `full: true` to include each version's resolved content body (token-expensive). There is no `offset`: versions are stored as reverse patches, so only a newest-end window is cheap to reconstruct. Unassign or detach with the `clear_assigned_user` / `clear_agent_role` / `clear_parent` booleans; an empty `assign` / `role` / `parent` does NOT clear. `remind` / `ack-reminder`: a fired reminder shows in `pad_project` next / ready (the delivery path on an instance with no webhook) until acknowledged, and one on a completed item is hidden there. `claim` / `release`: a loser's 409 is `lease_held`, and a lease bumps neither `updated_at` nor `seq`. (Two older forms still work and are not deprecated: `field: ["assigned_user_id="]` on either transport, and a direct `assigned_user_id: ""` param over remote `/mcp` only — prefer the boolean, which is the only one this schema advertises.) +- `pad_item` — Items: create / update / delete / get / list / move / restore / link / unlink / deps / star / unstar / starred / comment / list-comments / edit-comment / delete-comment / backlinks / bulk-update / note / decide / export / import / history / remind / ack-reminder / claim / release. On create/update, field values may be passed as a `fields` OBJECT (the same shape reads return, e.g. `{"status":"done","effort":"l"}`) — it merges into the same path as the dedicated params and `field: ["key=value"]`; the same key in two places with CONFLICTING values is refused, not silently resolved. `list` accepts `unparented: true` to keep items with no parent or implements relationship (mutually exclusive with `parent`). `list` results are SUMMARY-shaped by default on both transports — no content bodies; pass `full: true` for complete bodies (token-expensive), or prefer `get` for a single item's body. `update` field writes are a server-side field-level merge (only the keys you set change); pass `expected_seq` for optimistic concurrency — the `seq` you last read, which is on every item AND every list summary — and a stale value fails with a structured 409 `update_conflict` whose details carry `actual_seq` to retry with. PREFER IT over `expected_updated_at`, which cannot tell two writes inside one second apart; that one still works, and if you send both, seq decides. A token cannot see a browser tab's unsaved edits, so `content` sent WITH one is refused with 409 `content_pending_flush` while any exist; re-reading will not clear it. Wait for the editor to save, or pass `overwrite_pending_edits: true` to replace them. `content_state: "superseded_set_aside"` is different and wins when both apply: an editor upgrade set aside edits no tab can restore, so EVERY content write is refused (token or not; `details.set_aside_rows`) and waiting never clears it. Tell the user, or pass `overwrite_pending_edits: true` to discard them. Writing `content` while a browser tab has the item open routes the markdown through that live document, and the stored copy is updated only by a later collab-snapshot flush — usually from the tab that applied it, though any such write updates the row, and nothing guarantees one happens: the 200 echoes the content you SENT and carries `warnings.content_outcome: "applied_pending_flush"`, which describes that WRITE — the content went to the document, not the row — rather than being a live reading of the row, which a concurrent flush may already have updated. A `get` (or a `list` with `full: true` — a default list carries no content at all) before such a flush lands reads the stored copy and answers with the PREVIOUS content, which is the lag rather than a failed write, so do not re-send on the strength of it. You do not have to infer that from the write, and you will also see it when someone ELSE's tab holds the unflushed edits and you wrote nothing: a read that serves a body the server knows is behind the live document carries `content_state: "applied_pending_flush"` — same value as the write warning, different question (what the ROW is, not what your REQUEST did). Treat it as re-read later, never as re-send, and do not wait on it in a loop: nothing server-side moves that content into the stored copy, so the row catches up only when a tab next opens the item, which may never happen. The stored form may also not be byte-identical to what you sent, since the markdown round-trips through the editor (bullet markers, emphasis characters, list numbering and blank-line runs normalise; already-canonical markdown survives unchanged), so compare on meaning rather than bytes. `move` changes an item's COLLECTION within its workspace: system metadata (implementation notes, decision log, linked PR) survives it, and any field value the target schema has no home for is dropped AND reported in the move's activity entry — check there rather than assuming a move is lossless. A carried unique-field collision is dropped, named in `warnings.not_unique`. Three system keys — `implementation_notes`, `decision_log`, `convention` — cannot be set through `field` on update or move; the call is refused with `validation_failed`, naming the key and the write path that does maintain it (`note`, `decide`, and library activation respectively). `github_pr` is refused as well (BUG-2696): a `field` value arrived as a string, so the PR data was stored double-encoded and no link appeared. Its writer is `pad github link` / `unlink`, which needs a local git checkout, and there is no PR-link action on this surface yet, so treat linking a PR as something to hand to a human rather than a call to retry. On `create` they are refused too (BUG-3163), with the same `validation_failed`: create the item first, then use `note` / `decide` on it; convention metadata comes from library activation. `history` returns read-only item version metadata (newest-first), bounded to the NEWEST 50 versions by default (max 300 — pass `limit` to change the window); pass `full: true` to include each version's resolved content body (token-expensive). There is no `offset`: versions are stored as reverse patches, so only a newest-end window is cheap to reconstruct. Unassign or detach with the `clear_assigned_user` / `clear_agent_role` / `clear_parent` booleans; an empty `assign` / `role` / `parent` does NOT clear. `remind` / `ack-reminder`: a fired reminder shows in `pad_project` next / ready (the delivery path on an instance with no webhook) until acknowledged, and one on a completed item is hidden there. `claim` / `release`: a loser's 409 is `lease_held`, and a lease bumps neither `updated_at` nor `seq`. `edit-comment` / `delete-comment` take a `comment_id` from `list-comments` (whose rows carry `edited`); only a comment's author may edit it. Correct a comment in place rather than appending a correction. (Two older forms still work and are not deprecated: `field: ["assigned_user_id="]` on either transport, and a direct `assigned_user_id: ""` param over remote `/mcp` only — prefer the boolean, which is the only one this schema advertises.) **Relation fields (v0.31).** A `relation` value may be a UUID, an issue ref (`COLO-3`), or the target item's EXACT TITLE — in that order, so an item literally titled `COLO-3` is unreachable by title while the ref resolves. Title matching is scoped to the collection the field declares: a title that is unique only workspace-wide is REFUSED, naming the collection searched, and a title matching two or more items inside the declared collection is refused as `ambiguous` rather than `not_found` — it matched too much, not too little. Only items YOU can see count towards any of that, so a match you have no access to never changes your answer. diff --git a/internal/mcp/task2695_comment_edit_delete_test.go b/internal/mcp/task2695_comment_edit_delete_test.go new file mode 100644 index 000000000..5d94fffc6 --- /dev/null +++ b/internal/mcp/task2695_comment_edit_delete_test.go @@ -0,0 +1,153 @@ +package mcp + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" + "github.com/PerpetualSoftware/pad/internal/server" + "github.com/PerpetualSoftware/pad/internal/store/storetest" + mcpserver "github.com/mark3labs/mcp-go/server" +) + +// TASK-2695. edit-comment / delete-comment through the REMOTE door, end to +// end: raw JSON-RPC → catalog → HTTPHandlerDispatcher → the real handler +// chain → the store. The wrong-item rows are the point: the comment-is-on-ref +// check lives on the server, so a dispatcher that addressed the +// workspace-scoped route instead would pass the happy rows and fail these. +func TestCommentEditDelete_Remote(t *testing.T) { + s := storetest.NewSQLite(t) + srv := server.New(s) + t.Cleanup(srv.Stop) + owner, err := s.CreateUser(models.UserCreate{Email: "o@example.com", Name: "O", Password: "correct-horse-battery-staple"}) + if err != nil { + t.Fatal(err) + } + ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "W", Slug: "w2695", OwnerID: owner.ID}) + if err != nil { + t.Fatal(err) + } + if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil { + t.Fatal(err) + } + coll, err := s.CreateCollection(ws.ID, models.CollectionCreate{Name: "Tasks", Slug: "tasks", Prefix: "TASK", Schema: `{"fields":[]}`}) + if err != nil { + t.Fatal(err) + } + itemA, err := s.CreateItem(ws.ID, coll.ID, models.ItemCreate{Title: "A"}) + if err != nil { + t.Fatal(err) + } + itemB, err := s.CreateItem(ws.ID, coll.ID, models.ItemCreate{Title: "B"}) + if err != nil { + t.Fatal(err) + } + onA, err := s.CreateComment(ws.ID, itemA.ID, owner.ID, models.CommentCreate{Body: "on A", CreatedBy: "agent"}) + if err != nil { + t.Fatal(err) + } + onB, err := s.CreateComment(ws.ID, itemB.ID, owner.ID, models.CommentCreate{Body: "on B", CreatedBy: "agent"}) + if err != nil { + t.Fatal(err) + } + + m := mcpserver.NewMCPServer("t", "1", mcpserver.WithToolCapabilities(true)) + d := &HTTPHandlerDispatcher{Handler: srv, UserResolver: func(context.Context) *models.User { return owner }} + if _, err := RegisterCatalog(m, CatalogOptions{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState(ws.Slug), Dispatcher: d, PadVersion: "test"}); err != nil { + t.Fatal(err) + } + call := func(id int, args string) string { + t.Helper() + return respText(t, m.HandleMessage(context.Background(), toolsCall(id, args))) + } + body := func(id string) string { + t.Helper() + c, err := s.GetComment(id) + if err != nil { + t.Fatal(err) + } + if c == nil { + return "" + } + return c.Body + } + + refA := fmt.Sprintf("TASK-%d", *itemA.ItemNumber) + + resp := call(1, fmt.Sprintf(`{"action":"edit-comment","ref":%q,"comment_id":%q,"message":"on A, fixed"}`, refA, onA.ID)) + if strings.Contains(resp, `"isError":true`) { + t.Fatalf("edit through the right item failed: %s", resp) + } + if got := body(onA.ID); got != "on A, fixed" { + t.Fatalf("edit did not land: body = %q", got) + } + + resp = call(2, fmt.Sprintf(`{"action":"edit-comment","ref":%q,"comment_id":%q,"message":"hijacked"}`, refA, onB.ID)) + if !strings.Contains(resp, `"isError":true`) { + t.Fatalf("edit of B's comment through A succeeded: %s", resp) + } + if got := body(onB.ID); got != "on B" { + t.Fatalf("B's comment changed through A: %q", got) + } + + resp = call(3, fmt.Sprintf(`{"action":"delete-comment","ref":%q,"comment_id":%q}`, refA, onB.ID)) + if !strings.Contains(resp, `"isError":true`) { + t.Fatalf("delete of B's comment through A succeeded: %s", resp) + } + if got := body(onB.ID); got != "on B" { + t.Fatalf("B's comment deleted through A: %q", got) + } + + resp = call(4, fmt.Sprintf(`{"action":"list-comments","ref":%q}`, refA)) + if !strings.Contains(resp, `\"edited\":`) { + t.Fatalf("list-comments rows carry no edited key: %s", resp) + } + + resp = call(5, fmt.Sprintf(`{"action":"delete-comment","ref":%q,"comment_id":%q}`, refA, onA.ID)) + if strings.Contains(resp, `"isError":true`) { + t.Fatalf("delete through the right item failed: %s", resp) + } + if !strings.Contains(resp, `\"deleted\":true`) || !strings.Contains(resp, onA.ID) { + t.Fatalf("delete result is not the CLI's JSON shape: %s", resp) + } + if got := body(onA.ID); got != "" { + t.Fatalf("comment still present after delete: %q", got) + } +} + +// TestCommentEditDelete_Stdio pins the local door's argv: the actions reach +// the CLI verbs with ref, comment id and body as positionals behind `--`, so +// a body that starts with "-" is not parsed as a flag (the v0.42 rule). +func TestCommentEditDelete_Stdio(t *testing.T) { + fd := &fakeDispatcher{} + m := mcpserver.NewMCPServer("t", "1", mcpserver.WithToolCapabilities(true)) + if _, err := RegisterCatalog(m, CatalogOptions{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("ws"), Dispatcher: fd, PadVersion: "test"}); err != nil { + t.Fatal(err) + } + + resp := respText(t, m.HandleMessage(context.Background(), + toolsCall(1, `{"action":"edit-comment","ref":"TASK-1","comment_id":"c-1","message":"- corrected"}`))) + if strings.Contains(resp, `"isError":true`) { + t.Fatalf("edit-comment failed: %s", resp) + } + if got := strings.Join(fd.gotPath, " "); got != "item comment-edit" { + t.Fatalf("edit-comment dispatched to %q", got) + } + if got := strings.Join(fd.gotArgs, "\x00"); !strings.HasSuffix(got, strings.Join([]string{"--", "TASK-1", "c-1", "- corrected"}, "\x00")) { + t.Fatalf("edit-comment argv = %q", fd.gotArgs) + } + + resp = respText(t, m.HandleMessage(context.Background(), + toolsCall(2, `{"action":"delete-comment","ref":"TASK-1","comment_id":"c-1"}`))) + if strings.Contains(resp, `"isError":true`) { + t.Fatalf("delete-comment failed: %s", resp) + } + if got := strings.Join(fd.gotPath, " "); got != "item comment-delete" { + t.Fatalf("delete-comment dispatched to %q", got) + } + if got := strings.Join(fd.gotArgs, "\x00"); !strings.HasSuffix(got, strings.Join([]string{"--", "TASK-1", "c-1"}, "\x00")) { + t.Fatalf("delete-comment argv = %q", fd.gotArgs) + } +} diff --git a/internal/mcp/version.go b/internal/mcp/version.go index c6d96a46f..758a4310e 100644 --- a/internal/mcp/version.go +++ b/internal/mcp/version.go @@ -1651,6 +1651,24 @@ const CmdhelpVersion = "0.1" // this one either, and it is not a promise that the row will catch // up at all. // +// 0.57 — TASK-2695. ADDITIVE bump (the v0.28 / v0.13 disposition): two +// `pad_item` actions, `edit-comment` (ref, comment_id, message) and +// `delete-comment` (ref, comment_id), and the `comment_id` param. Web +// users could edit and delete comments since PLAN-1662; agents could +// only append corrections. Both ride new item-scoped routes, +// PATCH/DELETE /items/{ref}/comments/{id}, which answer 404 when the +// comment is not on `ref`. That check is the server's on every +// transport, and the ACL is the workspace route's unchanged: edit is +// author-only (a bearer admin gets no bypass, BUG-1919), delete is +// anyone who may edit the item. Remote packages a delete's 204 as the +// CLI's JSON, `{deleted, ref, comment_id}`. Every serialised comment +// (list-comments on all transports, and the API) now carries a derived +// `edited` bool, true once the body changed (updated_at > created_at, +// the web timeline's rule). That key is new, not a change to an +// existing one. Stdio reaches the actions through the new CLI verbs +// `pad item comment-edit` / `comment-delete`, which refuse against a +// server that does not advertise `item_scoped_comment_writes`. +// // Post-0.37, deliberately NO bump (BUG-3098): `workspace_member_limit` // joins the structured 403 allow-list, as ErrWorkspaceMemberLimit with // its own hint (not the plan-limit "upgrade" hint). The server emits it @@ -1659,7 +1677,7 @@ const CmdhelpVersion = "0.1" // this surface can receive it; the entry exists so a future action does // not collapse it to permission_denied. When an action that can reach // it is added, that addition is the contract change and owns the bump. -const ToolSurfaceVersion = "0.56" +const ToolSurfaceVersion = "0.57" // MetaVersionURI is the canonical URI of the queryable version document. // Lives outside the pad://workspace/{ws}/... namespace because it's a diff --git a/internal/models/comment.go b/internal/models/comment.go index 24730e356..03afdbb18 100644 --- a/internal/models/comment.go +++ b/internal/models/comment.go @@ -1,6 +1,9 @@ package models -import "time" +import ( + "encoding/json" + "time" +) // Comment represents a comment on an item. type Comment struct { @@ -44,6 +47,28 @@ type Comment struct { Reactions []Reaction `json:"reactions,omitempty"` } +// IsEdited reports whether the body changed after creation: create stamps +// created_at and updated_at with one value, and only an edit moves updated_at +// (reactions live in their own table). Timestamps are second-precision, so an +// edit inside the creation second is not seen. Same rule as the web timeline's +// isEdited (TimelineCommentCard.svelte); TASK-2695 brought it to the CLI. +func (c Comment) IsEdited() bool { + return !c.CreatedAt.IsZero() && c.UpdatedAt.After(c.CreatedAt) +} + +// MarshalJSON adds a derived, read-only `edited` (IsEdited) to every +// serialised comment, so an agent reading JSON on either MCP transport sees +// the same marker the CLI table and the web timeline show without comparing +// timestamps itself (TASK-2695). It is not stored and not read back: Comment +// has no field for it, so a decode ignores it. +func (c Comment) MarshalJSON() ([]byte, error) { + type plain Comment + return json.Marshal(struct { + plain + Edited bool `json:"edited"` + }{plain(c), c.IsEdited()}) +} + // CommentCreate is the input for creating a new comment. type CommentCreate struct { Author string `json:"author,omitempty"` diff --git a/internal/models/comment_test.go b/internal/models/comment_test.go new file mode 100644 index 000000000..04a719ceb --- /dev/null +++ b/internal/models/comment_test.go @@ -0,0 +1,47 @@ +package models + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +// TASK-2695: `edited` is derived from the timestamps and emitted on every +// serialised comment, nested replies included. +func TestCommentEditedMarker(t *testing.T) { + created := time.Date(2026, 9, 27, 12, 0, 0, 0, time.UTC) + fresh := Comment{ID: "c1", CreatedAt: created, UpdatedAt: created} + edited := Comment{ID: "c2", CreatedAt: created, UpdatedAt: created.Add(time.Second)} + + if fresh.IsEdited() { + t.Error("a comment whose timestamps match is not edited") + } + if !edited.IsEdited() { + t.Error("a comment updated after creation is edited") + } + if (Comment{}).IsEdited() { + t.Error("a zero comment is not edited") + } + + fresh.Replies = []Comment{edited} + b, err := json.Marshal(fresh) + if err != nil { + t.Fatal(err) + } + got := string(b) + if !strings.Contains(got, `"edited":false`) || !strings.Contains(got, `"edited":true`) { + t.Fatalf("want edited on the comment and on its reply, got %s", got) + } + if !strings.Contains(got, `"id":"c1"`) || !strings.Contains(got, `"replies":[`) { + t.Fatalf("the ordinary fields must still serialise: %s", got) + } + + var back Comment + if err := json.Unmarshal(b, &back); err != nil { + t.Fatalf("a serialised comment must still decode: %v", err) + } + if back.ID != "c1" || len(back.Replies) != 1 { + t.Fatalf("round trip lost fields: %+v", back) + } +} diff --git a/internal/server/handlers_capabilities.go b/internal/server/handlers_capabilities.go index 425aacfcd..f065f1e1e 100644 --- a/internal/server/handlers_capabilities.go +++ b/internal/server/handlers_capabilities.go @@ -43,6 +43,12 @@ type serverCapabilities struct { // literal slug, so a shorthand (`task` for `tasks`) finds nothing and the // client must keep normalising. SearchCollectionResolution bool `json:"search_collection_resolution"` + + // ItemScopedCommentWrites is true when PATCH and DELETE are routed at + // /items/{ref}/comments/{id} (TASK-2695). An older build has no such + // route and answers a bare 404, which a client would read as "comment + // not found", so the CLI refuses instead of sending when this is absent. + ItemScopedCommentWrites bool `json:"item_scoped_comment_writes"` } // WHAT A BUILD THAT CANNOT DECODE A FORMAT ACTUALLY COSTS THE READER @@ -85,7 +91,7 @@ type serverCapabilities struct { // rather than 500-ing — that signals to the editor "uploads still work, // but disable transformation tools." func (s *Server) handleServerCapabilities(w http.ResponseWriter, r *http.Request) { - resp := serverCapabilities{CollectionResolution: true, ItemFieldAppend: true, SearchCollectionResolution: true} + resp := serverCapabilities{CollectionResolution: true, ItemFieldAppend: true, SearchCollectionResolution: true, ItemScopedCommentWrites: true} if s.imageProcessor != nil { resp.Image = s.imageProcessor.Capabilities() } else { diff --git a/internal/server/handlers_comments.go b/internal/server/handlers_comments.go index 147aa21bb..2c65a33d0 100644 --- a/internal/server/handlers_comments.go +++ b/internal/server/handlers_comments.go @@ -259,6 +259,46 @@ func (s *Server) handleUpdateComment(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, updated) } +// itemScopedComment adapts a /comments/{commentID} handler to the item-scoped +// route /items/{itemSlug}/comments/{commentID} (TASK-2695). It resolves the +// item and answers 404 unless the comment is on it, then runs next unchanged, +// so the ACL, validation and events stay the workspace-scoped handler's own. +// +// The item-scoped form exists for the agent doors (CLI, both MCP transports), +// which name a comment by item ref plus id: checking the pairing here, rather +// than in each client, is what makes the three doors land on one check. The +// remote MCP dispatcher maps an action to a single HTTP call, so a +// client-side check could not reach it. +// +// A comment that exists on another item answers the same 404 as one that +// does not exist, so the route says nothing about comments elsewhere. +func (s *Server) itemScopedComment(next http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + item, err := s.store.ResolveItemIncludeDeleted(workspaceID, chi.URLParam(r, "itemSlug")) + if err != nil { + writeInternalError(w, err) + return + } + if item == nil { + writeError(w, http.StatusNotFound, "not_found", "Item not found") + return + } + if !s.requireItemVisible(w, r, workspaceID, item) { + return + } + comment, cerr := s.store.GetComment(chi.URLParam(r, "commentID")) + if cerr != nil || comment == nil || comment.WorkspaceID != workspaceID || comment.ItemID != item.ID { + writeError(w, http.StatusNotFound, "not_found", "Comment not found") + return + } + next(w, r) + } +} + // canEditComment reports whether the requester may edit the given comment: // the authenticated author (matching user_id) or a platform admin. A comment // with an empty user_id has no provable author, so only admins can edit it. diff --git a/internal/server/handlers_comments_item_scoped_test.go b/internal/server/handlers_comments_item_scoped_test.go new file mode 100644 index 000000000..1fca2f26f --- /dev/null +++ b/internal/server/handlers_comments_item_scoped_test.go @@ -0,0 +1,142 @@ +package server + +import ( + "net/http" + "strings" + "testing" +) + +// TestItemScopedCommentWrites pins the item-scoped comment routes the agent +// doors use (TASK-2695): PATCH and DELETE /items/{ref}/comments/{id} refuse a +// comment that is not on {ref}, and otherwise behave exactly like the +// workspace-scoped routes they delegate to, ACL included. +func TestItemScopedCommentWrites(t *testing.T) { + t.Parallel() + env := setupRBACEnv(t) + ws := "/api/v1/workspaces/" + env.wsSlug + + newItem := func(title string) string { + t.Helper() + rr := doRequestWithCookie(env.srv, "POST", ws+"/collections/docs/items", + map[string]any{"title": title}, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("create item: %d %s", rr.Code, rr.Body.String()) + } + var item map[string]any + parseJSON(t, rr, &item) + return item["slug"].(string) + } + newComment := func(slug, body, token string) string { + t.Helper() + rr := doRequestWithCookie(env.srv, "POST", ws+"/items/"+slug+"/comments", + map[string]any{"body": body}, token) + if rr.Code != http.StatusCreated { + t.Fatalf("post comment: %d %s", rr.Code, rr.Body.String()) + } + var c map[string]any + parseJSON(t, rr, &c) + if c["edited"] != false { + t.Fatalf("fresh comment: want edited=false, got %v", c["edited"]) + } + return c["id"].(string) + } + listBodies := func(slug string) map[string]map[string]any { + t.Helper() + rr := doRequestWithCookie(env.srv, "GET", ws+"/items/"+slug+"/comments", nil, env.ownerToken) + if rr.Code != http.StatusOK { + t.Fatalf("list comments: %d %s", rr.Code, rr.Body.String()) + } + var list []map[string]any + parseJSON(t, rr, &list) + out := map[string]map[string]any{} + for _, c := range list { + out[c["id"].(string)] = c + } + return out + } + + itemA := newItem("Scoped A") + itemB := newItem("Scoped B") + onA := newComment(itemA, "on A", env.editorToken) + onB := newComment(itemB, "on B", env.editorToken) + + t.Run("edit through the right item", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "PATCH", ws+"/items/"+itemA+"/comments/"+onA, + map[string]any{"body": "on A, corrected"}, env.editorToken) + if rr.Code != http.StatusOK { + t.Fatalf("want 200, got %d: %s", rr.Code, rr.Body.String()) + } + got := listBodies(itemA)[onA] + if got["body"] != "on A, corrected" { + t.Fatalf("body not updated: %v", got["body"]) + } + }) + + t.Run("edit through the wrong item is refused and writes nothing", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "PATCH", ws+"/items/"+itemA+"/comments/"+onB, + map[string]any{"body": "hijacked"}, env.editorToken) + if rr.Code != http.StatusNotFound || !strings.Contains(rr.Body.String(), "Comment not found") { + t.Fatalf("want 404 Comment not found, got %d: %s", rr.Code, rr.Body.String()) + } + if got := listBodies(itemB)[onB]["body"]; got != "on B" { + t.Fatalf("comment on B changed through A: %v", got) + } + }) + + t.Run("unknown item is an item 404", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "PATCH", ws+"/items/no-such-item/comments/"+onA, + map[string]any{"body": "x"}, env.editorToken) + if rr.Code != http.StatusNotFound || !strings.Contains(rr.Body.String(), "Item not found") { + t.Fatalf("want 404 Item not found, got %d: %s", rr.Code, rr.Body.String()) + } + }) + + t.Run("the edit ACL is the workspace route's", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "PATCH", ws+"/items/"+itemA+"/comments/"+onA, + map[string]any{"body": "not mine"}, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Fatalf("non-author edit: want 403, got %d: %s", rr.Code, rr.Body.String()) + } + }) + + t.Run("delete through the wrong item is refused and deletes nothing", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "DELETE", ws+"/items/"+itemA+"/comments/"+onB, nil, env.editorToken) + if rr.Code != http.StatusNotFound { + t.Fatalf("want 404, got %d: %s", rr.Code, rr.Body.String()) + } + if _, ok := listBodies(itemB)[onB]; !ok { + t.Fatal("comment on B was deleted through A") + } + }) + + t.Run("the delete ACL is the workspace route's", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "DELETE", ws+"/items/"+itemB+"/comments/"+onB, nil, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Fatalf("viewer delete: want 403, got %d: %s", rr.Code, rr.Body.String()) + } + }) + + t.Run("delete through the right item", func(t *testing.T) { + rr := doRequestWithCookie(env.srv, "DELETE", ws+"/items/"+itemB+"/comments/"+onB, nil, env.editorToken) + if rr.Code != http.StatusNoContent { + t.Fatalf("want 204, got %d: %s", rr.Code, rr.Body.String()) + } + if _, ok := listBodies(itemB)[onB]; ok { + t.Fatal("comment still listed after delete") + } + }) +} + +func TestServerCapabilitiesAdvertisesItemScopedCommentWrites(t *testing.T) { + t.Parallel() + srv := testServer(t) + rr := doRequest(srv, "GET", "/api/v1/server/capabilities", nil) + if rr.Code != http.StatusOK { + t.Fatalf("capabilities: %d %s", rr.Code, rr.Body.String()) + } + var caps map[string]any + parseJSON(t, rr, &caps) + if caps["item_scoped_comment_writes"] != true { + t.Fatalf("want item_scoped_comment_writes=true, got %v", caps["item_scoped_comment_writes"]) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index d40a9460b..39089d53f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1933,6 +1933,8 @@ func (s *Server) setupRouter() { r.Post("/links", s.handleCreateItemLink) r.Get("/comments", s.handleListComments) r.Post("/comments", s.handleCreateComment) + r.Patch("/comments/{commentID}", s.itemScopedComment(s.handleUpdateComment)) + r.Delete("/comments/{commentID}", s.itemScopedComment(s.handleDeleteComment)) r.Get("/timeline", s.handleListItemTimeline) r.Get("/children", s.handleGetItemChildren) r.Get("/progress", s.handleGetItemProgress) diff --git a/skills/pad/SKILL.md b/skills/pad/SKILL.md index cb167b198..fc8aa65df 100644 --- a/skills/pad/SKILL.md +++ b/skills/pad/SKILL.md @@ -206,7 +206,9 @@ pad item remind TASK-5 --remind-at 2026-08-01T09:00:00Z # arm a one-shot remin pad item reminders TASK-5 # armed / fired / acknowledged pad item ack # acknowledge a fired reminder pad item unremind -pad item comments TASK-5 +pad item comments TASK-5 # each row shows its id, and "edited" once changed +pad item comment-edit TASK-5 "..." [--stdin] # correct your own comment in place instead of appending +pad item comment-delete TASK-5 pad item note TASK-5 "what you did" [--details "..." | --stdin] pad item decide TASK-5 "what you chose" [--rationale "..." | --stdin] pad item bulk-update --status X TASK-5 TASK-8 ... diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index 3ad67de6f..d33ca8061 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -2108,6 +2108,21 @@ export const api = { method: 'DELETE' }), + // Item-scoped edit/delete (TASK-2695): the server answers 404 unless + // the comment is on itemRef. WebMCP uses these so its edit-comment / + // delete-comment land on the same check as the CLI and /mcp. + updateOnItem: (ws: string, itemRef: string, commentId: string, data: { body: string }) => + request( + `/workspaces/${ws}/items/${encodeURIComponent(itemRef)}/comments/${encodeURIComponent(commentId)}`, + { method: 'PATCH', body: JSON.stringify(data) } + ), + + deleteOnItem: (ws: string, itemRef: string, commentId: string) => + request( + `/workspaces/${ws}/items/${encodeURIComponent(itemRef)}/comments/${encodeURIComponent(commentId)}`, + { method: 'DELETE' } + ), + reply: (ws: string, commentId: string, data: CommentCreate) => request(`/workspaces/${ws}/comments/${commentId}/replies`, { method: 'POST', diff --git a/web/src/lib/types/index.ts b/web/src/lib/types/index.ts index 895c51ffb..ad62f866f 100644 --- a/web/src/lib/types/index.ts +++ b/web/src/lib/types/index.ts @@ -1574,6 +1574,8 @@ export interface Comment { parent_id?: string; created_at: string; updated_at: string; + /** Derived server-side: the body changed after creation (TASK-2695). */ + edited?: boolean; item_title?: string; item_slug?: string; /** diff --git a/web/src/lib/webmcp/dispatch.test.ts b/web/src/lib/webmcp/dispatch.test.ts index 8e486c45e..9ef5c933d 100644 --- a/web/src/lib/webmcp/dispatch.test.ts +++ b/web/src/lib/webmcp/dispatch.test.ts @@ -31,6 +31,8 @@ function mockApi() { comments: { list: vi.fn(async () => []), create: vi.fn(async () => ({ id: 'c-1' })), + updateOnItem: vi.fn(async () => ({ id: 'c-1', body: 'fixed', edited: true })), + deleteOnItem: vi.fn(async () => undefined), }, dashboard: { get: vi.fn(async () => ({ ok: true })) }, next: vi.fn(async () => [{ item_slug: 'task-1', item_title: 'Do it', collection: 'tasks', reason: 'high priority' }]), @@ -104,7 +106,7 @@ const READ = new Set([ const isReadOnly = (tool: string, action: string): boolean | undefined => { if (READ.has(`${tool}:${action}`)) return true; if ( - ['create', 'update', 'delete', 'import', 'comment', 'link', 'unlink', 'move', + ['create', 'update', 'delete', 'import', 'comment', 'edit-comment', 'delete-comment', 'link', 'unlink', 'move', 'restore', 'star', 'unstar', 'bulk-update', 'activate'].includes(action) ) return false; @@ -359,6 +361,43 @@ describe('dispatch — pad_item writes', () => { ); }); + // TASK-2695: the item-scoped client methods, so the server refuses a + // comment_id that is not on `ref`, as it does for the CLI and /mcp. + it('edit-comment maps ref, comment_id and message to the item-scoped update', async () => { + const api = mockApi(); + const res = parse( + await run(api, 'pad_item', { + action: 'edit-comment', + ref: 'TASK-1', + comment_id: 'c-1', + message: 'fixed', + }), + ); + expect(res.isError).toBe(false); + expect(api.comments.updateOnItem).toHaveBeenCalledWith(WS, 'TASK-1', 'c-1', { body: 'fixed' }); + }); + + it('edit-comment refuses a missing comment_id or message without calling the api', async () => { + const api = mockApi(); + const noId = parse(await run(api, 'pad_item', { action: 'edit-comment', ref: 'TASK-1', message: 'x' })); + const noBody = parse(await run(api, 'pad_item', { action: 'edit-comment', ref: 'TASK-1', comment_id: 'c-1' })); + expect(noId.isError).toBe(true); + expect(noId.text).toContain("'comment_id'"); + expect(noBody.isError).toBe(true); + expect(noBody.text).toContain("'message'"); + expect(api.comments.updateOnItem).not.toHaveBeenCalled(); + }); + + it('delete-comment uses the item-scoped delete and returns the CLI-shaped result', async () => { + const api = mockApi(); + const res = parse( + await run(api, 'pad_item', { action: 'delete-comment', ref: 'TASK-1', comment_id: 'c-1' }), + ); + expect(res.isError).toBe(false); + expect(api.comments.deleteOnItem).toHaveBeenCalledWith(WS, 'TASK-1', 'c-1'); + expect(JSON.parse(res.text)).toEqual({ deleted: true, ref: 'TASK-1', comment_id: 'c-1' }); + }); + it('star/unstar map to the right client methods', async () => { const api = mockApi(); await run(api, 'pad_item', { action: 'star', ref: 'TASK-1' }); diff --git a/web/src/lib/webmcp/dispatch.ts b/web/src/lib/webmcp/dispatch.ts index 992701876..0074545aa 100644 --- a/web/src/lib/webmcp/dispatch.ts +++ b/web/src/lib/webmcp/dispatch.ts @@ -522,6 +522,18 @@ const HANDLERS: Record = { parent_id: str(args, 'reply_to'), source: 'web', }), + // TASK-2695: item-scoped, so a comment_id not on `ref` is refused here + // exactly as on the CLI and /mcp. Delete mirrors their JSON result. + 'pad_item:edit-comment': (api, ws, args) => + api.comments.updateOnItem(ws, requireRef(args), requireArg(args, 'comment_id'), { + body: requireArg(args, 'message'), + }), + 'pad_item:delete-comment': async (api, ws, args) => { + const ref = requireRef(args); + const commentId = requireArg(args, 'comment_id'); + await api.comments.deleteOnItem(ws, ref, commentId); + return { deleted: true, ref, comment_id: commentId }; + }, 'pad_item:bulk-update': (api, ws, args) => { const ids = strArray(args, 'refs'); if (!ids || ids.length === 0) {