From 619479905d5c01304d963063e786153c19a5f424 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 11 Oct 2026 16:21:21 +0000 Subject: [PATCH 1/3] fix(mcp): idempotency_target_gone is a structured code on both transports (TASK-3585) A replayed create whose item has since been deleted answers 410 idempotency_target_gone. Remote MCP had no 410 arm, and the CLI wrote no stdio marker for it, so it reached agents as a generic error. It is now allow-listed, mapped on the remote 410 arm, and marked on stdio, with a hint shared between internal/cli and internal/mcp. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS --- internal/cli/client.go | 18 ++++++++----- internal/mcp/errors.go | 17 +++++++++--- internal/mcp/task3585_op_key_test.go | 40 +++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 06cfaf9eb..673bb72c7 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -989,18 +989,20 @@ const ( PatchUnstableCode = "patch_unstable_roundtrip" PatchNotFoundCode = "patch_not_found" // TASK-3585. - PatchOutcomeUnknownCode = "patch_outcome_unknown" - IdempotencyInFlightCode = "idempotency_in_flight" - IdempotencyKeyReusedCode = "idempotency_key_reused" + PatchOutcomeUnknownCode = "patch_outcome_unknown" + IdempotencyInFlightCode = "idempotency_in_flight" + IdempotencyKeyReusedCode = "idempotency_key_reused" + IdempotencyTargetGoneCode = "idempotency_target_gone" ) // PatchOutcomeUnknownHint, IdempotencyInFlightHint and // IdempotencyKeyReusedHint are the recovery guidance for TASK-3585's codes. // Duplicated in internal/mcp; a test asserts they match. const ( - PatchOutcomeUnknownHint = "Nothing new was written. An earlier attempt reached an editor that never confirmed it and is gone, so it is not re-applied: read details.diff (the change since the patch was sent); if your edit is there, it landed. Otherwise send it again with a NEW key." - IdempotencyInFlightHint = "A request with this key is still running. Wait a moment and retry with the SAME key: you will get its answer, not a second write." - IdempotencyKeyReusedHint = "This key was already used for a different request. Use a new key for a new request; reuse a key only to retry the identical request." + PatchOutcomeUnknownHint = "Nothing new was written. An earlier attempt reached an editor that never confirmed it and is gone, so it is not re-applied: read details.diff (the change since the patch was sent); if your edit is there, it landed. Otherwise send it again with a NEW key." + IdempotencyInFlightHint = "A request with this key is still running. Wait a moment and retry with the SAME key: you will get its answer, not a second write." + IdempotencyKeyReusedHint = "This key was already used for a different request. Use a new key for a new request; reuse a key only to retry the identical request." + IdempotencyTargetGoneHint = "The request this key belongs to already ran and created an item that has since been deleted. Nothing was written now. To create it again, send the request with a NEW key." ) // PatchNotFoundHint is the recovery guidance for patch_not_found (PLAN-3578 @@ -1030,7 +1032,7 @@ func IsPatchRefusal(err error) (*APIError, bool) { if errors.As(err, &apiErr) { switch apiErr.Code { case PatchAnchorMissingCode, PatchAnchorAmbiguousCode, PatchRoomOpenCode, PatchUnstableCode, PatchNotFoundCode, - PatchOutcomeUnknownCode, IdempotencyInFlightCode, IdempotencyKeyReusedCode: + PatchOutcomeUnknownCode, IdempotencyInFlightCode, IdempotencyKeyReusedCode, IdempotencyTargetGoneCode: return apiErr, true } } @@ -1057,6 +1059,8 @@ func WritePatchRefusalError(w io.Writer, apiErr *APIError) { hint = IdempotencyInFlightHint case IdempotencyKeyReusedCode: hint = IdempotencyKeyReusedHint + case IdempotencyTargetGoneCode: + hint = IdempotencyTargetGoneHint } body := map[string]any{"code": apiErr.Code, "message": apiErr.Message, "hint": hint} if len(apiErr.Details) > 0 { diff --git a/internal/mcp/errors.go b/internal/mcp/errors.go index 231201121..aec881884 100644 --- a/internal/mcp/errors.go +++ b/internal/mcp/errors.go @@ -226,6 +226,9 @@ const ( ErrPatchOutcomeUnknown ErrorCode = "patch_outcome_unknown" ErrIdempotencyInFlight ErrorCode = "idempotency_in_flight" ErrIdempotencyKeyReused ErrorCode = "idempotency_key_reused" + // ErrIdempotencyTargetGone: a replayed create whose item has since been + // deleted (410). + ErrIdempotencyTargetGone ErrorCode = "idempotency_target_gone" // ErrStoredStateUnreadable fires when an operation is refused because // the ITEM'S STORED STATE cannot be decoded — today, an append to an @@ -511,6 +514,7 @@ var allowedStructuredErrorCodes = map[string]struct{}{ "patch_outcome_unknown": {}, "idempotency_in_flight": {}, "idempotency_key_reused": {}, + "idempotency_target_gone": {}, // BUG-2675. The only entry whose marker is written for a LOCALLY // generated refusal rather than an upstream APIError — the CLI's // append helpers refuse before any request is made (see @@ -1112,6 +1116,12 @@ func classifyHTTPStatusKind( Message: "Validation failed.", Hint: hint, }) + case http.StatusGone: + // TASK-3585: a replayed create whose item was deleted since. Without + // this arm a 410 fell through to the generic other-4xx bucket. + if upstream := extractUpstreamErrorEnvelope(bodyText); upstream.Code == string(ErrIdempotencyTargetGone) { + return NewErrorResult(ErrorPayload{Code: ErrIdempotencyTargetGone, Message: bodyMessage, Hint: IdempotencyTargetGoneHint, Details: upstream.Details}) + } case http.StatusTooManyRequests: // BUG-1430: pre-existing behavior collapsed 429 into the // generic ErrServerError "other 4xx" bucket, which led @@ -1468,9 +1478,10 @@ const PatchNotFoundHint = "Nothing was written. That patch id is not a patch on // TASK-3585's hints. Duplicated in internal/cli; a test asserts they match. const ( - PatchOutcomeUnknownHint = "Nothing new was written. An earlier attempt reached an editor that never confirmed it and is gone, so it is not re-applied: read details.diff (the change since the patch was sent); if your edit is there, it landed. Otherwise send it again with a NEW key." - IdempotencyInFlightHint = "A request with this key is still running. Wait a moment and retry with the SAME key: you will get its answer, not a second write." - IdempotencyKeyReusedHint = "This key was already used for a different request. Use a new key for a new request; reuse a key only to retry the identical request." + PatchOutcomeUnknownHint = "Nothing new was written. An earlier attempt reached an editor that never confirmed it and is gone, so it is not re-applied: read details.diff (the change since the patch was sent); if your edit is there, it landed. Otherwise send it again with a NEW key." + IdempotencyInFlightHint = "A request with this key is still running. Wait a moment and retry with the SAME key: you will get its answer, not a second write." + IdempotencyKeyReusedHint = "This key was already used for a different request. Use a new key for a new request; reuse a key only to retry the identical request." + IdempotencyTargetGoneHint = "The request this key belongs to already ran and created an item that has since been deleted. Nothing was written now. To create it again, send the request with a NEW key." ) // UpdateConflictContentHint replaces the generic re-read-and-retry hint on an diff --git a/internal/mcp/task3585_op_key_test.go b/internal/mcp/task3585_op_key_test.go index 4300e75be..629e7fca0 100644 --- a/internal/mcp/task3585_op_key_test.go +++ b/internal/mcp/task3585_op_key_test.go @@ -1,6 +1,7 @@ package mcp import ( + "bytes" "testing" "github.com/PerpetualSoftware/pad/internal/cli" @@ -44,12 +45,13 @@ func TestTASK3585_CLIAndMCPAgreeOnHints(t *testing.T) { {cli.PatchOutcomeUnknownHint, PatchOutcomeUnknownHint}, {cli.IdempotencyInFlightHint, IdempotencyInFlightHint}, {cli.IdempotencyKeyReusedHint, IdempotencyKeyReusedHint}, + {cli.IdempotencyTargetGoneHint, IdempotencyTargetGoneHint}, } { if p[0] != p[1] { t.Errorf("hints differ:\n cli %q\n mcp %q", p[0], p[1]) } } - for _, c := range []string{cli.PatchOutcomeUnknownCode, cli.IdempotencyInFlightCode, cli.IdempotencyKeyReusedCode} { + for _, c := range []string{cli.PatchOutcomeUnknownCode, cli.IdempotencyInFlightCode, cli.IdempotencyKeyReusedCode, cli.IdempotencyTargetGoneCode} { if _, ok := allowedStructuredErrorCodes[c]; !ok { t.Errorf("%s is not allow-listed", c) } @@ -82,3 +84,39 @@ func TestTASK3585_MCPCreateOpKey(t *testing.T) { t.Fatalf("%d items; the retried create made a twin", n) } } + +// A replayed create whose item has since been deleted is its own structured +// code on both transports, not a generic error. +func TestTASK3585_MCPCreateReplayOfADeletedItem(t *testing.T) { + f := newParentFixture(t) + in := map[string]any{"workspace": f.workspace.Slug, "collection": "tasks", "title": "Then deleted", "op_key": "mcp-gone"} + first := f.dispatchResult2(t, []string{"item", "create"}, in) + m, _ := first.StructuredContent.(map[string]any) + id, _ := m["id"].(string) + if first.IsError || id == "" { + t.Fatalf("create: %s", textOf(first)) + } + if err := f.store.DeleteItem(id); err != nil { + t.Fatal(err) + } + res := f.dispatchResult2(t, []string{"item", "create"}, in) + env, ok := res.StructuredContent.(ErrorEnvelope) + if !res.IsError || !ok || env.Error.Code != ErrIdempotencyTargetGone || env.Error.Hint != IdempotencyTargetGoneHint { + t.Fatalf("remote replay of a deleted item: %s", textOf(res)) + } + + // stdio: main.go writes the marker only for codes IsPatchRefusal admits. + apiErr := &cli.APIError{Code: cli.IdempotencyTargetGoneCode, Message: "m"} + if _, ok := cli.IsPatchRefusal(apiErr); !ok { + t.Fatal("the CLI would not write a marker for idempotency_target_gone") + } + var stderr bytes.Buffer + cli.WritePatchRefusalError(&stderr, apiErr) + r := extractStructuredCLIError(stderr.String()) + if r == nil { + t.Fatalf("stdio marker not lifted: %s", stderr.String()) + } + if e := r.StructuredContent.(ErrorEnvelope).Error; e.Code != ErrIdempotencyTargetGone || e.Hint != IdempotencyTargetGoneHint { + t.Fatalf("stdio: %+v", e) + } +} From db603743cac300a18e799688ff4394b3504c9040 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 11 Oct 2026 16:32:33 +0000 Subject: [PATCH 2/3] feat(items): idempotency keys for comment create (TASK-3585) POST .../items/{ref}/comments accepts op_key (body) or Idempotency-Key. The key's answer (the comment id) is recorded inside CreateCommentWithActivity's transaction, so a retry whose first answer was lost returns that comment instead of a twin. A refused comment releases its key; a replay of a comment deleted since answers 410 idempotency_target_gone. `pad item comment --op-key` (a fresh key per invocation by default) retries once behind the idempotency_keys capability; MCP comment forwards op_key on both transports. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS --- cmd/pad/cmd_item.go | 9 +- cmd/pad/task3585_comment_retry_test.go | 92 ++++++++++++++++++++ internal/cli/client.go | 9 +- internal/mcp/catalog_item.go | 2 +- internal/mcp/dispatch_http_routes.go | 4 + internal/mcp/task3585_op_key_test.go | 27 ++++++ internal/models/comment.go | 10 +++ internal/server/handlers_comments.go | 34 ++++++++ internal/server/idempotency_keys.go | 9 ++ internal/server/task3585_comment_key_test.go | 89 +++++++++++++++++++ internal/store/comments.go | 6 ++ 11 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 cmd/pad/task3585_comment_retry_test.go create mode 100644 internal/server/task3585_comment_key_test.go diff --git a/cmd/pad/cmd_item.go b/cmd/pad/cmd_item.go index 33cde1c7a..2f701a6bd 100644 --- a/cmd/pad/cmd_item.go +++ b/cmd/pad/cmd_item.go @@ -3131,7 +3131,7 @@ Use - to read the artifact from stdin.`, // --- comments --- func commentCmd() *cobra.Command { - var replyTo string + var replyTo, commentOpKey string cmd := &cobra.Command{ Use: "comment ", @@ -3142,9 +3142,15 @@ func commentCmd() *cobra.Command { client, _ := getClient() ws := getWorkspace() + // TASK-3585: one idempotency key per invocation, so a retry of a + // comment whose answer was lost returns it instead of a twin. + if commentOpKey == "" { + commentOpKey = uuid.NewString() + } input := models.CommentCreate{ Body: args[1], ParentID: replyTo, + OpKey: commentOpKey, } comment, err := client.CreateComment(ws, args[0], input) @@ -3162,6 +3168,7 @@ func commentCmd() *cobra.Command { } cmd.Flags().StringVar(&replyTo, "reply-to", "", "reply to a specific comment ID") + cmd.Flags().StringVar(&commentOpKey, "op-key", "", "idempotency key: retrying the SAME comment with the same key returns the comment it made instead of a second one (default: a new key per invocation)") return cmd } diff --git a/cmd/pad/task3585_comment_retry_test.go b/cmd/pad/task3585_comment_retry_test.go new file mode 100644 index 000000000..9c17a81a5 --- /dev/null +++ b/cmd/pad/task3585_comment_retry_test.go @@ -0,0 +1,92 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "testing" +) + +// commentServer answers capabilities (idempotency_keys = honorsKeys) and +// scripts the statuses of the comment POSTs it receives. +func commentServer(t *testing.T, honorsKeys bool, statuses ...int) *[]map[string]any { + t.Helper() + var bodies []map[string]any + setupFormatRoutingTest(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/capabilities") { + _ = json.NewEncoder(w).Encode(map[string]any{"idempotency_keys": honorsKeys}) + return + } + if r.Method != http.MethodPost || !strings.HasSuffix(r.URL.Path, "/comments") { + _ = json.NewEncoder(w).Encode(map[string]any{}) + return + } + raw, _ := io.ReadAll(r.Body) + var b map[string]any + _ = json.Unmarshal(raw, &b) + bodies = append(bodies, b) + status := http.StatusCreated + if n := len(bodies) - 1; n < len(statuses) { + status = statuses[n] + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if status >= 400 { + _ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": "x", "message": "m"}}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"id": "c1", "body": "hi"}) + })) + formatFlag = "table" + return &bodies +} + +func runComment(t *testing.T, args ...string) error { + t.Helper() + cmd := commentCmd() + cmd.SetArgs(append([]string{"TASK-5", "hi"}, args...)) + var err error + _ = captureStdout(t, func() { err = cmd.Execute() }) + return err +} + +func TestTASK3585_CLICommentRetriesOnceWithTheSameKey(t *testing.T) { + bodies := commentServer(t, true, http.StatusBadGateway) + if err := runComment(t); err != nil { + t.Fatalf("a 502 then a 201 must succeed: %v", err) + } + if len(*bodies) != 2 { + t.Fatalf("want exactly one retry, got %d attempts", len(*bodies)) + } + k1, _ := (*bodies)[0]["op_key"].(string) + k2, _ := (*bodies)[1]["op_key"].(string) + if k1 == "" || k1 != k2 { + t.Fatalf("the retry must reuse the key: %q vs %q", k1, k2) + } +} + +func TestTASK3585_CLICommentNoRetryWithoutTheCapability(t *testing.T) { + bodies := commentServer(t, false, http.StatusBadGateway) + if err := runComment(t); err == nil { + t.Fatal("a 502 with no retry must fail") + } + if len(*bodies) != 1 { + t.Fatalf("retried a comment against a server that ignores keys: %d attempts", len(*bodies)) + } +} + +func TestTASK3585_CLICommentKeyIsGivenOrFresh(t *testing.T) { + bodies := commentServer(t, true) + _ = runComment(t) + _ = runComment(t) + _ = runComment(t, "--op-key", "mine") + if len(*bodies) != 3 { + t.Fatalf("attempts = %d", len(*bodies)) + } + k0, _ := (*bodies)[0]["op_key"].(string) + k1, _ := (*bodies)[1]["op_key"].(string) + if k0 == "" || k0 == k1 || (*bodies)[2]["op_key"] != "mine" { + t.Fatalf("keys = %q %q %v", k0, k1, (*bodies)[2]["op_key"]) + } +} diff --git a/internal/cli/client.go b/internal/cli/client.go index 673bb72c7..d74e2470d 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -836,7 +836,14 @@ func (c *Client) ListComments(wsSlug, itemSlug string) ([]models.Comment, error) func (c *Client) CreateComment(wsSlug, itemSlug string, input models.CommentCreate) (*models.Comment, error) { var result models.Comment - err := c.post("/workspaces/"+wsSlug+"/items/"+itemSlug+"/comments", input, &result) + path := "/workspaces/" + wsSlug + "/items/" + itemSlug + "/comments" + err := c.post(path, input, &result) + // TASK-3585: the same single keyed retry as CreateItem, behind the same + // capability gate. + if err != nil && input.OpKey != "" && retryableWithKey(err) && c.serverHonorsKeys() { + result = models.Comment{} + err = c.post(path, input, &result) + } return &result, wrapItemNotFound(err, itemSlug, wsSlug) } diff --git a/internal/mcp/catalog_item.go b/internal/mcp/catalog_item.go index 8671b9366..b2ab8a7b0 100644 --- a/internal/mcp/catalog_item.go +++ b/internal/mcp/catalog_item.go @@ -316,7 +316,7 @@ var padItemSchemaParams = []ParamDef{ }, "required": []any{"old", "new"}, }, Description: "patch: the edits, applied in order, all or nothing. Required for: patch."}, - {Name: "op_key", Type: "string", Description: "patch, create: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused)."}, + {Name: "op_key", Type: "string", Description: "patch, create, comment: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused)."}, {Name: "undo", Type: "string", Description: "patch: invert an earlier patch on this item by its patch_id (instead of edits). Refused, writing nothing (patch_anchor_missing), when the text it changed has changed since; never reverts someone else's later edit. The undo is itself a patch."}, {Name: "lines", Type: "string", Description: "get: return only these body lines, numbered (\"A-B\", \"A-\" or \"A\"; 1-based), with seq and content_state, for targeting a patch. Line numbers are for reading; a patch anchors on text."}, // PLAN-3555: the deliberate-overwrite opt-out. Phase 1 changes no outcome; diff --git a/internal/mcp/dispatch_http_routes.go b/internal/mcp/dispatch_http_routes.go index 4891c76b8..f3d79f949 100644 --- a/internal/mcp/dispatch_http_routes.go +++ b/internal/mcp/dispatch_http_routes.go @@ -1846,6 +1846,10 @@ func mapItemComment(input map[string]any) (string, string, []byte, error) { payload["parent_id"] = s } } + // TASK-3585: op_key is the comment's idempotency key. + if v, ok := input["op_key"].(string); ok && v != "" { + payload["op_key"] = v + } body, err := json.Marshal(payload) if err != nil { return "", "", nil, fmt.Errorf("encode body: %w", err) diff --git a/internal/mcp/task3585_op_key_test.go b/internal/mcp/task3585_op_key_test.go index 629e7fca0..953dd8b26 100644 --- a/internal/mcp/task3585_op_key_test.go +++ b/internal/mcp/task3585_op_key_test.go @@ -120,3 +120,30 @@ func TestTASK3585_MCPCreateReplayOfADeletedItem(t *testing.T) { t.Fatalf("stdio: %+v", e) } } + +// TASK-3585: op_key on comment rides to the server, so a retried comment +// answers the comment it made instead of a twin. +func TestTASK3585_MCPCommentOpKey(t *testing.T) { + f := newParentFixture(t) + in := map[string]any{"workspace": f.workspace.Slug, "ref": f.child.Slug, "message": "Keyed via MCP", "op_key": "mcp-cm"} + first := f.dispatchResult2(t, []string{"item", "comment"}, in) + second := f.dispatchResult2(t, []string{"item", "comment"}, in) + m1, _ := first.StructuredContent.(map[string]any) + m2, _ := second.StructuredContent.(map[string]any) + if first.IsError || second.IsError || m1["id"] == nil || m1["id"] != m2["id"] { + t.Fatalf("replay: %s / %s", textOf(first), textOf(second)) + } + cs, err := f.store.ListComments(f.child.ID) + if err != nil { + t.Fatal(err) + } + n := 0 + for _, c := range cs { + if c.Body == "Keyed via MCP" { + n++ + } + } + if n != 1 { + t.Fatalf("%d comments; the retried comment made a twin", n) + } +} diff --git a/internal/models/comment.go b/internal/models/comment.go index a0d8e18c1..281a62bbd 100644 --- a/internal/models/comment.go +++ b/internal/models/comment.go @@ -1,6 +1,7 @@ package models import ( + "database/sql" "encoding/json" "time" ) @@ -101,6 +102,15 @@ type CommentCreate struct { Source string `json:"source,omitempty"` ParentID string `json:"parent_id,omitempty"` ActivityID string `json:"activity_id,omitempty"` + // OpKey is the idempotency key in the body (TASK-3585), for callers that + // cannot set the Idempotency-Key header. Not part of the request hash; + // the store ignores it. + OpKey string `json:"op_key,omitempty"` + // AfterWriteTx, when set, runs inside CreateCommentWithActivity's + // transaction after the comment is inserted and read back, before COMMIT + // (TASK-3585: the idempotency key's answer). An error rolls the comment + // and its activity back. Internal. + AfterWriteTx func(tx *sql.Tx, created *Comment) error `json:"-"` } // Reaction represents an emoji reaction on a comment. diff --git a/internal/server/handlers_comments.go b/internal/server/handlers_comments.go index 121a35e7a..1ac7e2d4a 100644 --- a/internal/server/handlers_comments.go +++ b/internal/server/handlers_comments.go @@ -2,7 +2,9 @@ package server import ( "database/sql" + "encoding/json" "errors" + "fmt" "net/http" "strings" @@ -100,6 +102,38 @@ func (s *Server) handleCreateComment(w http.ResponseWriter, r *http.Request) { return } + // TASK-3585: an idempotency key, claimed before the write. A replay + // answers with the comment the first request made, as it stands now. + key, kerr := idempotencyKey(r, input.OpKey) + if kerr != nil { + writeError(w, http.StatusBadRequest, "bad_request", kerr.Error()) + return + } + canon := input + canon.OpKey = "" + hold, ok := s.claimKey(w, r, workspaceID, idempotencyRouteComment, item.ID, canon, key, func(status int, stored string) { + var a commentedAnswer + if json.Unmarshal([]byte(stored), &a) != nil { + writeInternalError(w, fmt.Errorf("stored idempotent answer does not decode")) + return + } + c, gerr := s.store.GetComment(a.CommentID) + if gerr != nil || c == nil || c.Deleted { + writeError2(w, http.StatusGone, "idempotency_target_gone", "the comment this key created no longer exists", map[string]interface{}{"comment_id": a.CommentID}) + return + } + writeJSON(w, status, c) + }) + if !ok { + return + } + defer s.release(hold) + if hold != nil { + input.AfterWriteTx = func(tx *sql.Tx, created *models.Comment) error { + return s.completeTx(hold, http.StatusCreated, commentedAnswer{CommentID: created.ID})(tx) + } + } + stampCommentAttribution(r, &input) actor, source := input.CreatedBy, input.Source diff --git a/internal/server/idempotency_keys.go b/internal/server/idempotency_keys.go index 36fa49c5d..9083a1f9a 100644 --- a/internal/server/idempotency_keys.go +++ b/internal/server/idempotency_keys.go @@ -84,3 +84,12 @@ const idempotencyRouteCreate = "item.create" type createdAnswer struct { ItemID string `json:"item_id"` } + +// idempotencyRouteComment names the comment-create route in a key's request +// hash. +const idempotencyRouteComment = "comment.create" + +// commentedAnswer is what a comment create's key stores: the comment it made. +type commentedAnswer struct { + CommentID string `json:"comment_id"` +} diff --git a/internal/server/task3585_comment_key_test.go b/internal/server/task3585_comment_key_test.go new file mode 100644 index 000000000..a183009bb --- /dev/null +++ b/internal/server/task3585_comment_key_test.go @@ -0,0 +1,89 @@ +package server + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// TASK-3585: idempotency keys on comment create. +func commentKeyFixture(t *testing.T) (*Server, string) { + t.Helper() + srv := testServer(t) + slug := createWSWithCollections(t, srv) + rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/tasks/items", map[string]any{"title": "Commented on", "fields": `{"status":"open"}`}) + if rr.Code != http.StatusCreated { + t.Fatalf("item: %d %s", rr.Code, rr.Body.String()) + } + var it models.Item + _ = json.Unmarshal(rr.Body.Bytes(), &it) + return srv, "/api/v1/workspaces/" + slug + "/items/" + it.Slug + "/comments" +} + +func countComments(t *testing.T, srv *Server, path, body string) int { + t.Helper() + rr := doRequest(srv, "GET", path, nil) + var cs []models.Comment + _ = json.Unmarshal(rr.Body.Bytes(), &cs) + n := 0 + for _, c := range cs { + if c.Body == body { + n++ + } + } + return n +} + +func TestTASK3585_CommentReplaysTheCommentItMade(t *testing.T) { + srv, path := commentKeyFixture(t) + body := map[string]any{"body": "Keyed comment", "op_key": "k-1"} + first := doRequest(srv, "POST", path, body) + again := doRequest(srv, "POST", path, body) + var a, b models.Comment + _ = json.Unmarshal(first.Body.Bytes(), &a) + _ = json.Unmarshal(again.Body.Bytes(), &b) + if first.Code != http.StatusCreated || again.Code != http.StatusCreated || a.ID == "" || a.ID != b.ID || again.Header().Get("Idempotent-Replayed") != "true" { + t.Fatalf("replay: %d/%d %q vs %q", first.Code, again.Code, a.ID, b.ID) + } + if n := countComments(t, srv, path, "Keyed comment"); n != 1 { + t.Fatalf("%d comments; the retry made a twin", n) + } + if rr := doRequest(srv, "POST", path, map[string]any{"body": "Different", "op_key": "k-1"}); rr.Code != http.StatusUnprocessableEntity { + t.Fatalf("a reused key: %d", rr.Code) + } +} + +func TestTASK3585_RefusedCommentReleasesItsKey(t *testing.T) { + srv, path := commentKeyFixture(t) + bad := doRequestWithHeaders(srv, "POST", path, map[string]any{"body": "Reply", "parent_id": "no-such-comment"}, map[string]string{"Idempotency-Key": "k-2"}) + if bad.Code < 400 { + t.Fatalf("premise: a reply to a missing parent must be refused; got %d", bad.Code) + } + ok := doRequestWithHeaders(srv, "POST", path, map[string]any{"body": "Reply"}, map[string]string{"Idempotency-Key": "k-2"}) + if ok.Code != http.StatusCreated || ok.Header().Get("Idempotent-Replayed") != "" { + t.Fatalf("corrected retry with the same key: %d %s", ok.Code, ok.Body.String()) + } +} + +func TestTASK3585_CommentReplayOfADeletedComment(t *testing.T) { + srv, path := commentKeyFixture(t) + body := map[string]any{"body": "Then deleted", "op_key": "k-3"} + first := doRequest(srv, "POST", path, body) + var c models.Comment + _ = json.Unmarshal(first.Body.Bytes(), &c) + if first.Code != http.StatusCreated || c.ID == "" { + t.Fatalf("create: %d", first.Code) + } + if rr := doRequest(srv, "DELETE", path+"/"+c.ID, nil); rr.Code >= 300 { + t.Fatalf("delete: %d %s", rr.Code, rr.Body.String()) + } + again := doRequest(srv, "POST", path, body) + if again.Code != http.StatusGone || !json.Valid(again.Body.Bytes()) { + t.Fatalf("replay of a deleted comment: %d %s", again.Code, again.Body.String()) + } + if n := countComments(t, srv, path, "Then deleted"); n != 0 { + t.Fatalf("the replay re-created the comment: %d", n) + } +} diff --git a/internal/store/comments.go b/internal/store/comments.go index d4a2bec27..3a04ee594 100644 --- a/internal/store/comments.go +++ b/internal/store/comments.go @@ -85,6 +85,12 @@ func (s *Store) CreateCommentWithActivity(workspaceID, itemID, userID string, ac if err != nil { return nil, err } + // TASK-3585: the caller's last word inside this transaction. + if input.AfterWriteTx != nil { + if err := input.AfterWriteTx(tx, out); err != nil { + return nil, err + } + } if err := tx.Commit(); err != nil { return nil, fmt.Errorf("commit comment: %w", err) } From 43f1ca3e240c832361a2c44054e875321a571045 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 11 Oct 2026 16:58:17 +0000 Subject: [PATCH 3/3] feat(items): idempotency keys for note and decision appends (TASK-3585) PATCH /workspaces/{ws}/items/{ref} accepts op_key (body) or Idempotency-Key, but only on an append-only update (append_implementation_note and/or append_decision, plus source); a key with any other member is refused 400. The answer (the item id and the minted entries) is recorded in the update's own transaction, so a retry whose first answer was lost echoes the first entries in `appended` instead of appending twice. The built-in and patch callers, which reach this handler with their own key, are untouched. `pad item note|decide --op-key` (fresh per invocation by default) retry once behind the idempotency_keys capability; MCP note/decide forward op_key. ToolSurfaceVersion 0.91 -> 0.92 (additive; also records the 410 now being structured). docs/api.md covers create, comment and append keys. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS --- README.md | 4 +- cmd/pad/notes.go | 17 +++- cmd/pad/task3585_append_retry_test.go | 81 +++++++++++++++++ .../testdata/mcp_wire/remote_initialize.json | 4 +- .../testdata/mcp_wire/remote_tools_list.json | 2 +- .../testdata/mcp_wire/stdio_initialize.json | 4 +- .../testdata/mcp_wire/stdio_tools_list.json | 2 +- docs/api.md | 2 +- internal/cli/client.go | 10 ++- internal/mcp/catalog_item.go | 2 +- internal/mcp/dispatch_http_project.go | 9 ++ internal/mcp/instructions.md | 2 +- internal/mcp/task3585_op_key_test.go | 24 +++++ internal/mcp/version.go | 13 ++- internal/models/item.go | 4 + internal/server/handlers_items.go | 35 ++++++++ internal/server/idempotency_keys.go | 51 +++++++++++ internal/server/task3585_append_key_test.go | 90 +++++++++++++++++++ 18 files changed, 341 insertions(+), 15 deletions(-) create mode 100644 cmd/pad/task3585_append_retry_test.go create mode 100644 internal/server/task3585_append_key_test.go diff --git a/README.md b/README.md index e6313edda..68e5efb96 100644 --- a/README.md +++ b/README.md @@ -402,7 +402,7 @@ 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.91)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server: +**Tool catalog (v0.92)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server: | Tool | Actions | |---|---| @@ -426,7 +426,7 @@ Plus resources at `pad://workspaces`, `pad://workspace/{ws}/dashboard`, and `pad://_meta/version`. **Stability contract.** The catalog is versioned -(`tool_surface_version: "0.91"`) and advertised in the initialize handshake and at +(`tool_surface_version: "0.92"`) and advertised in the initialize handshake and at `pad://_meta/version`, so external agents can pin against it; the per-version changelog lives in [`internal/mcp/version.go`](internal/mcp/version.go). Errors come back as structured envelopes (`{error: {code, message, hint, ...}}`) from a diff --git a/cmd/pad/notes.go b/cmd/pad/notes.go index dc2454078..1b7f0608b 100644 --- a/cmd/pad/notes.go +++ b/cmd/pad/notes.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "github.com/google/uuid" "io" "os" "strings" @@ -16,7 +17,7 @@ import ( ) func noteCmd() *cobra.Command { - var details string + var details, noteOpKey string var readStdin bool cmd := &cobra.Command{ @@ -47,9 +48,15 @@ func noteCmd() *cobra.Command { // BUG-3056: the server appends under its write lock, so a // concurrent write to another key cannot be reverted by this // one. It mints the id, timestamp and attribution. + // TASK-3585: one idempotency key per invocation, so a retry + // of a note whose answer was lost does not append it twice. + if noteOpKey == "" { + noteOpKey = uuid.NewString() + } updated, err = client.UpdateItem(ws, args[0], models.ItemUpdate{ AppendImplementationNote: &models.ItemImplementationNoteAppend{Summary: summary, Details: body}, Source: "cli", + OpKey: noteOpKey, }) if err != nil { return structuredAppendRefusal(err, "note", models.ItemFieldImplementationNotes) @@ -81,11 +88,12 @@ func noteCmd() *cobra.Command { cmd.Flags().StringVar(&details, "details", "", "implementation details") cmd.Flags().BoolVar(&readStdin, "stdin", false, "read note details from stdin") + cmd.Flags().StringVar(¬eOpKey, "op-key", "", "idempotency key: retrying the SAME note with the same key returns the note it appended instead of a second one (default: a new key per invocation)") return cmd } func decideCmd() *cobra.Command { - var rationale string + var rationale, decideOpKey string var readStdin bool cmd := &cobra.Command{ @@ -114,9 +122,13 @@ func decideCmd() *cobra.Command { ) if client.ServerSupportsItemFieldAppend() { // BUG-3056 — see `pad item note`. + if decideOpKey == "" { + decideOpKey = uuid.NewString() + } updated, err = client.UpdateItem(ws, args[0], models.ItemUpdate{ AppendDecision: &models.ItemDecisionLogAppend{Decision: decision, Rationale: body}, Source: "cli", + OpKey: decideOpKey, }) if err != nil { return structuredAppendRefusal(err, "decision", models.ItemFieldDecisionLog) @@ -148,6 +160,7 @@ func decideCmd() *cobra.Command { cmd.Flags().StringVar(&rationale, "rationale", "", "decision rationale") cmd.Flags().BoolVar(&readStdin, "stdin", false, "read decision rationale from stdin") + cmd.Flags().StringVar(&decideOpKey, "op-key", "", "idempotency key: retrying the SAME decision with the same key returns the decision it appended instead of a second one (default: a new key per invocation)") return cmd } diff --git a/cmd/pad/task3585_append_retry_test.go b/cmd/pad/task3585_append_retry_test.go new file mode 100644 index 000000000..a9d189f34 --- /dev/null +++ b/cmd/pad/task3585_append_retry_test.go @@ -0,0 +1,81 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "testing" +) + +// appendServer answers capabilities and scripts the statuses of the item +// PATCHes (appends) it receives. A success echoes the appended entry, as the +// server does, so the command's confirmation check passes. +func appendServer(t *testing.T, honorsKeys bool, statuses ...int) *[]map[string]any { + t.Helper() + var bodies []map[string]any + setupFormatRoutingTest(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/capabilities") { + _ = json.NewEncoder(w).Encode(map[string]any{"item_field_append": true, "idempotency_keys": honorsKeys}) + return + } + if r.Method != http.MethodPatch { + _ = json.NewEncoder(w).Encode(map[string]any{"slug": "s", "ref": "TASK-5", "fields": "{}"}) + return + } + raw, _ := io.ReadAll(r.Body) + var b map[string]any + _ = json.Unmarshal(raw, &b) + bodies = append(bodies, b) + status := http.StatusOK + if n := len(bodies) - 1; n < len(statuses) { + status = statuses[n] + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if status >= 400 { + _ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": "x", "message": "m"}}) + return + } + note := map[string]any{"id": "note_1", "summary": "s"} + fields, _ := json.Marshal(map[string]any{"implementation_notes": []any{note}}) + _ = json.NewEncoder(w).Encode(map[string]any{"slug": "s", "ref": "TASK-5", "fields": string(fields), + "appended": map[string]any{"implementation_note": note}}) + })) + formatFlag = "table" + return &bodies +} + +func runNote(t *testing.T, args ...string) error { + t.Helper() + cmd := noteCmd() + cmd.SetArgs(append([]string{"TASK-5", "s"}, args...)) + var err error + _ = captureStdout(t, func() { err = cmd.Execute() }) + return err +} + +func TestTASK3585_CLINoteRetriesOnceWithTheSameKey(t *testing.T) { + bodies := appendServer(t, true, http.StatusBadGateway) + if err := runNote(t); err != nil { + t.Fatalf("a 502 then a 200 must succeed: %v", err) + } + if len(*bodies) != 2 { + t.Fatalf("want exactly one retry, got %d attempts", len(*bodies)) + } + k1, _ := (*bodies)[0]["op_key"].(string) + k2, _ := (*bodies)[1]["op_key"].(string) + if k1 == "" || k1 != k2 { + t.Fatalf("the retry must reuse the key: %q vs %q", k1, k2) + } +} + +func TestTASK3585_CLINoteNoRetryWithoutTheCapability(t *testing.T) { + bodies := appendServer(t, false, http.StatusBadGateway) + if err := runNote(t); err == nil { + t.Fatal("a 502 with no retry must fail") + } + if len(*bodies) != 1 { + t.Fatalf("retried a note against a server that ignores keys: %d attempts", len(*bodies)) + } +} diff --git a/cmd/pad/testdata/mcp_wire/remote_initialize.json b/cmd/pad/testdata/mcp_wire/remote_initialize.json index c25a59a4a..f7f90f8f9 100644 --- a/cmd/pad/testdata/mcp_wire/remote_initialize.json +++ b/cmd/pad/testdata/mcp_wire/remote_initialize.json @@ -8,7 +8,7 @@ }, "padToolSurface": { "tool_surface_stable": true, - "version": "0.91" + "version": "0.92" } }, "prompts": {}, @@ -21,5 +21,5 @@ "name": "pad-mcp", "version": "" }, - "instructions": "Pad is a project tracker for developers and AI agents — issues (TASK, BUG), plans (PLAN), ideas (IDEA), docs (DOC), conventions, comments, and dependencies. Use this server when a user mentions:\n\n- Issue refs like `TASK-5`, `BUG-12`, `PLAN-3`, `IDEA-8` — they are stable, human-readable IDs and the canonical way to address items. Use them, never slugs; they are what audit trails and PR titles show.\n- Tasks / issues / items / plans / progress / \"what's on my plate\" / \"what to work on next\" / standup / changelog / retrospective.\n- Project conventions, decision records, or \"how should this team do X.\"\n\nIf the user is asking general code questions with no project-management thread, you don't need this server.\n\n## Tool surface (v0.91)\n\nTen resource × action tools, plus `pad_set_workspace`. Eleven tools total.\n\nInputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped.\n\n- `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 / patch. 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. To change PART of a body, prefer `patch`: `edits: [{old, new}]`, where each `old` is exact text occurring once in the current body (read it with `get` and `lines: \"A-B\"` to quote it). The answer is a unified `diff`: read it, because anchors guarantee position, not meaning. A moved `expected_seq` is applied over and reported as `rebased` with the intervening writes. On an item open in a browser editor, the editor applies the patch to its live document, merging with the typing there, and the answer says `content_state: \"applied_pending_flush\"`. `patch_anchor_missing` / `patch_anchor_ambiguous` mean re-read and quote current text; `patch_unstable_roundtrip` means the open editor would also change content you did not edit, so send a whole-body update; `patch_room_open` means an older editor that cannot apply patches holds the item. Every patch answer carries a `patch_id`; `patch` with `undo: \u003cpatch_id\u003e` inverts it, and is refused when the text it changed has changed since. Send an `op_key` (any unique string, e.g. a UUID) with a patch whenever you might retry it: the same request with the same key returns the first answer instead of applying twice. `patch_outcome_unknown` means an earlier attempt's editor never confirmed it and is gone; read `details.diff` before deciding. `content` REPLACES the whole body, so send `expected_seq` with it: `get` the item, edit that body, and send it back with the `seq` you read. An `update_conflict` whose details carry `content_write` means someone changed the item after your read. Do NOT resend your body, because it would undo their change: re-read, apply your edit to the current content, and send that with the new seq. `replace: true` overwrites on purpose instead. A content write with neither carries `warnings.unguarded_content_write`, and a later release refuses it. 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: the server normally recovers the stored copy itself about a minute after the last tab leaves the item, but that is not guaranteed (a recovery can fail, and an operator can turn it off), and otherwise the row catches up only when a tab next opens the item. 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`: claim an item before you work it; a loser's 409 is `lease_held` (pick another, don't retry), and a lease bumps neither `updated_at` nor `seq`. The holder defaults to your agent name, and `pad_project` next / ready leave out items another holder has leased. `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. Deleting a comment that has replies leaves a placeholder (`deleted: true`, empty body) so the replies keep their parent; editing, replying to or reacting to it answers 409 `comment_deleted`. (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.)\n\n**List filters must name a field (v0.75).** A `pad_item` `list` filter on a key no collection in scope declares, and no item stores, is refused with `validation_failed` (\"invalid list filter\"), where it used to return an empty list; a stored undeclared key still filters. Check the schema; don't retry.\n\n**Library words in a blank workspace (v0.67).** A brand-new blank workspace lists one convention trigger (`always`) and one scope (`all`). Activating a library entry, or creating a convention or playbook with a trigger or scope the library itself uses, adds that word to the collection's options and says so in `warnings.options_added`. A trigger you invent is still refused until the workspace owner adds it to the collection's schema.\n\n**App attribution (v0.65).** An item an installed app created, a comment an app wrote and a history row an app's write made carry `via_app` (the install id) and `via_app_name` (the app's name). On an item they name the app that CREATED it, not the last writer. They are absent for anything a person or an agent wrote.\n\n**Relation fields (v0.31).** A `relation` value may be a UUID, an issue ref (`COLO-3`) or the target's EXACT TITLE, tried in that order (a title that is also a ref resolves as the ref). A title matches only inside the declared collection, among items you can see; found only elsewhere, it is refused naming that collection; several matches are `ambiguous`. Full rules: docs/mcp.md.\n\nReads carry a `relation_targets` member beside `fields`: field key → `{id, ref, title}`, so you can render a relation without a request per value. `fields` still holds the canonical id, which is what you write back. An entry with an `id` and NO `ref`/`title` means the target is gone **or** you may not see it — the two are deliberately indistinguishable, so do not render it as either \"deleted\" or \"hidden\"; \"unavailable\" is the honest word. An entry with `stored_as_text: true` differs: the stored value is not UUID-shaped, so it was never an id (an imported title, legacy text). Render it as text, not as unavailable.\n\n**`multi_relation` fields (v0.33).** An ORDERED LIST of references, each resolved like a `relation` (UUID, ref, or exact title in the declared collection). Declare one as `owners:multi_relation:people`; the target collection is required. Rules: `[]` means absent; a blank element is refused by index; order is part of the value; duplicates are refused after resolution (`duplicate_referent`); any failing element refuses the whole write; `required` needs one resolved element; a cross-workspace copy drops the whole value (in `warnings.dropped_fields`). `relation_targets` holds an ARRAY of `{id, ref, title}` in stored order (`id`-only where unresolved); scalar keys stay single objects. Full rules: docs/mcp.md.\n\n- `pad_workspace` — Workspaces: list / members / invite / storage / audit-log / create / claim / deleted / restore.\n- `pad_collection` — Collections: list / create / update / delete / list-archived / restore. `delete` archives: the collection and its items leave every view, and `restore` brings them back. An `update` that removes a field or a select option still in use answers with `warnings.orphaned`: tell the user how many items keep the removed value.\n- `pad_project` — Project intelligence: dashboard / next / ready / stale / standup / changelog / report / activity. Use `ready` for the ranked top items to work on next (blocked work is left out) and `stale` for items needing attention; `activity` to catch up on what other agents/users changed since you last worked (non-streaming feed with item refs + change details).\n- `pad_role` — Agent roles: list / create / update / delete.\n- `pad_search` — Full-text search across items: query.\n- `pad_playbook` — Invokable procedures: list / get / run / match. Use `run` to bind args against a playbook's declared spec and get the rendered body back; side-effect-free. `run` refuses a playbook whose status isn't `active` (a draft still being authored) with a `playbook_not_active` error — pass `allow_draft: true` to override. Both `run` and `get` echo the playbook's `status`. Both may also carry `content_state` (`\"applied_pending_flush\"`, or `\"superseded_set_aside\"` as under `pad_item`), meaning the BODY you are about to execute is behind the item's live collaborative document — an editor holds unflushed edits, so the steps may be superseded. It is a reason to re-read or to ask, not a reason to refuse to run; the body is still the best the server has. `match` finds a playbook from text.\n- `pad_library` — Convention + playbook library (the global catalog of pre-built entries workspaces activate): list / get / activate.\n- `pad_attachment` — Read-only attachment metadata: list / show. `list` enumerates a workspace's attachments (filter by item / category / collection / attached / unattached); `show` returns one attachment's MIME, size, filename, and ETag via a HEAD request. Uploading and general file downloads stay CLI-only; bounded image bytes are available through the attachment resource below.\n- `pad_meta` — Server introspection: server-info / version / tool-surface / bootstrap. The `bootstrap` action returns one-shot workspace context (user + collections + always-on conventions + a metadata-only `convention_index` of every active convention + roles + playbook metadata + dashboard + recent activity).\n- `pad_set_workspace` — Load workspace context; response embeds the bootstrap blob so you load context in one call. Whether it also pins a session default: see Workspace context below. Takes `workspace: \u003cslug\u003e` only (no `action`).\n\nFor the ten resource × action tools, always pass `action` as a top-level field. Per-action required parameters are documented in each tool's description.\n\n## Resources are cheaper than tool calls\n\nRead these directly when you need workspace state (described in the resource listings): `pad://workspaces`, `pad://workspace/{ws}/dashboard`, `.../collections`, `.../items` (use `pad_item` `list` to filter), `.../items/{ref}` (markdown), `.../attachments/{id}` (images only, at most 1 MiB, as base64 `thumb-md`), `.../bootstrap` (the same payload as `pad_meta` `bootstrap`), and `pad://_meta/version`. A host can prefetch them once per session.\n\nLoad the bootstrap resource once per workspace and reuse its collections, conventions, roles, and playbook metadata. Refresh it after switching workspaces, after changing those configuration surfaces, when Pad reports stale schema/context, or when the user asks. For changing work state, read the affected item or use a targeted project tool instead of fetching the whole bootstrap again.\n\n## Workspace context\n\nEvery action that operates within a workspace accepts an optional `workspace` parameter. Resolution order:\n\n1. Explicit `workspace` argument on the call (always wins).\n2. On a single-user local server only: the session default set via `pad_set_workspace`.\n3. On a single-user local server only: the CWD-linked workspace from `.pad.toml`.\n\nA multi-user/remote server does **not** persist a session default — pass `workspace` explicitly on every call. If none resolves, the action returns a structured `no_workspace` error with `available_workspaces`, on both the local and the remote transport.\n\n## Update flow: read first, then patch\n\nFor `pad_item.action: update`, the server merges your patch with the item's current state. Pass only the fields you want to change. When changing `status`, ALWAYS include a `comment` explaining why — it builds the audit trail that helps the team understand history.\n\n## One error code you must not retry\n\nErrors come back as a structured envelope with a `code`. Most failures are worth a retry after you fix something, and a `server_error` may be transient. `stored_state_unreadable` is neither.\n\nIt means the ITEM'S STORED DATA cannot be decoded — your input was fine. Today it fires when `note` / `decide` would append to an `implementation_notes` / `decision_log` value that is not a list of entries. The operation is refused precisely because completing it would overwrite that value. The refusal is fully deterministic, so retrying — immediately or after a backoff — fails identically every time and changes nothing.\n\nWhen you see it: tell the user which item is affected, and stop. Whether you can inspect the bad value depends on which layer is broken — if the item's whole `fields` blob fails to parse, `action: get` hands it back as a raw string; if one structured key is at fault, you won't see it at all, because the blob is normalized on this surface and a value that doesn't decode is dropped from the top-level arrays. A human can always read it with `pad item show \u003cref\u003e --format json`. Do not route around the refusal by writing the field some other way; that is the write that caused it.\n\n## Project conventions\n\nWorkspaces can declare conventions (e.g. \"run `make test` before PR\", \"use conventional commit format\"). The bootstrap blob gives you two views:\n\n- `conventions` — full bodies of the always-on (`trigger=always`) rules. Follow these unconditionally.\n- `convention_index` — METADATA ONLY (`ref`, `title`, `trigger`, `role`; no bodies) for **every** active convention, including the triggered ones whose bodies are NOT in `conventions`. This is your map of what triggered rules exist.\n\nBefore performing meaningful work with a specific trigger (e.g. `on-implement` before writing code), consult `convention_index`: if it lists entries for that trigger, pull their bodies on demand; if it lists none, skip the query.\n\n```\npad_item.action: list, collection: \"conventions\", status: \"active\"\n```\n\nFilter by trigger (`always`, `on-implement`, `on-task-complete`, etc.) when relevant — the `convention_index` triggers tell you which filters are worth running.\n\n`\"conventions\"` above is the DEFAULT collection slug and a workspace may have renamed it, in which case that literal returns nothing while `convention_index` still lists entries — the bootstrap payload resolves by declaration, not by name. When the two disagree, address the items by the `ref`s the index gave you, or find the collection with `pad_collection.action: list`. The same applies to `\"playbooks\"`; `pad_playbook` resolves by declaration and is unaffected.\n\n## Pushes and the session monitor (security note)\n\nPad can push an item into a Claude Code session (`pad push`, or the web item view's push composer). That is terminal instruction injection by design, so receiving is **consent-gated** (PLAN-2613, since v0.15.0): a session receives pushes only after it has connected (`/pad:connect` in the plugin, or a repo-level `push.auto_arm = true` in `.pad.toml` — which a per-user `auto_arm = false` in `~/.pad/config.toml` vetoes), and pushes are self-addressed only — the sender is always the receiving session's own user. This MCP connection is not a session: it cannot arm one, receive a push, or run the monitor, and the `pad_item` / `pad_project` tools expose no push action. If a user asks you to \"push\" something to their terminal, tell them it happens from their own terminal (`pad push \u003cref\u003e -m \"...\"` — the sending session need not be connected, the receiving ones must have consented); do not try to emulate it by writing comments or instructions into items.\n\n## Adding a workspace to this connection\n\nIf the user references a workspace this connection can't see (you'll get a 403 from workspace tools, or the workspace won't appear in `pad_workspace.list`), tell the user you can't see that workspace with your current permissions, then walk them through how to grant access: open Pad in their browser → switch to that workspace → avatar menu → \"Connect project...\" A 6-digit claim code will appear. Have them paste it back in chat, then call `pad_workspace.claim` with `{workspace: \"\u003cslug\u003e\", code: \"\u003c6 digits\u003e\"}`. The workspace joins this connection's allow-list and stays until the user revokes it via `/console/connected-apps`. No re-auth required.\n\nFor brand-new workspaces, `pad_workspace.create` with `{name: \"\u003cname\u003e\"}` (and optional `template`) creates the workspace AND auto-adds it to this connection's allow-list in one call — no claim code needed. Only works when the connection currently carries the \"may create workspaces\" grant — granted at consent time, or enabled later on the connections page. If the grant is present and set to false, the create is **refused with a 403 and no workspace is made** — retrying won't help and neither will the claim flow, since there is nothing to claim. Tell the user this connection isn't permitted to create workspaces, and offer them the two ways forward: create the workspace themselves in the browser and grant access with a claim code, or re-authorize this connection with \"Let this app create new workspaces\" enabled (`/console/connected-apps` can also flip it on an existing connection).\n\n## New workspace: offer to set it up\n\nThe bootstrap blob carries `needs_onboarding: bool` — true when the workspace has zero user-created items (template seeds don't count). When it's true, **lead with an active offer before anything else**: *\"This workspace is brand new and isn't set up yet. Want me to set it up? I'll ask a few quick questions and adapt it to your project.\"*\n\nThis is an **offer, not an auto-run** — wait for the user to say yes before running onboarding. If they accept, run the `onboard` playbook (use the `pad_onboard` prompt, or load the body via `pad_playbook` `action: get`, `ref: onboard`). If they decline, or already declined earlier in the session, respect that and skip the offer. The flag flips to false the moment any item exists, so it won't nag past first setup.\n\n## Built-in updates: mention once\n\nThe bootstrap blob may carry `builtin_updates: int` (absent when there are none): how many of the conventions and playbooks Pad ships have newer text in Pad's library than this workspace's copy. When it is present, mention it **once per session**, in one line, then carry on. **Never apply an update yourself**: a person reviews each one and accepts it or not.\n\n## Multi-step workflows\n\nFour prompts ship with the server: `pad_plan`, `pad_ideate`, `pad_retro`, `pad_onboard`. Use them when the user wants help planning, brainstorming, retrospecting, or onboarding into a workspace — they encode the multi-step Pad-aware playbook for each.\n" + "instructions": "Pad is a project tracker for developers and AI agents — issues (TASK, BUG), plans (PLAN), ideas (IDEA), docs (DOC), conventions, comments, and dependencies. Use this server when a user mentions:\n\n- Issue refs like `TASK-5`, `BUG-12`, `PLAN-3`, `IDEA-8` — they are stable, human-readable IDs and the canonical way to address items. Use them, never slugs; they are what audit trails and PR titles show.\n- Tasks / issues / items / plans / progress / \"what's on my plate\" / \"what to work on next\" / standup / changelog / retrospective.\n- Project conventions, decision records, or \"how should this team do X.\"\n\nIf the user is asking general code questions with no project-management thread, you don't need this server.\n\n## Tool surface (v0.92)\n\nTen resource × action tools, plus `pad_set_workspace`. Eleven tools total.\n\nInputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped.\n\n- `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 / patch. 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. To change PART of a body, prefer `patch`: `edits: [{old, new}]`, where each `old` is exact text occurring once in the current body (read it with `get` and `lines: \"A-B\"` to quote it). The answer is a unified `diff`: read it, because anchors guarantee position, not meaning. A moved `expected_seq` is applied over and reported as `rebased` with the intervening writes. On an item open in a browser editor, the editor applies the patch to its live document, merging with the typing there, and the answer says `content_state: \"applied_pending_flush\"`. `patch_anchor_missing` / `patch_anchor_ambiguous` mean re-read and quote current text; `patch_unstable_roundtrip` means the open editor would also change content you did not edit, so send a whole-body update; `patch_room_open` means an older editor that cannot apply patches holds the item. Every patch answer carries a `patch_id`; `patch` with `undo: \u003cpatch_id\u003e` inverts it, and is refused when the text it changed has changed since. Send an `op_key` (any unique string, e.g. a UUID) with a patch whenever you might retry it: the same request with the same key returns the first answer instead of applying twice. `patch_outcome_unknown` means an earlier attempt's editor never confirmed it and is gone; read `details.diff` before deciding. `content` REPLACES the whole body, so send `expected_seq` with it: `get` the item, edit that body, and send it back with the `seq` you read. An `update_conflict` whose details carry `content_write` means someone changed the item after your read. Do NOT resend your body, because it would undo their change: re-read, apply your edit to the current content, and send that with the new seq. `replace: true` overwrites on purpose instead. A content write with neither carries `warnings.unguarded_content_write`, and a later release refuses it. 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: the server normally recovers the stored copy itself about a minute after the last tab leaves the item, but that is not guaranteed (a recovery can fail, and an operator can turn it off), and otherwise the row catches up only when a tab next opens the item. 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`: claim an item before you work it; a loser's 409 is `lease_held` (pick another, don't retry), and a lease bumps neither `updated_at` nor `seq`. The holder defaults to your agent name, and `pad_project` next / ready leave out items another holder has leased. `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. Deleting a comment that has replies leaves a placeholder (`deleted: true`, empty body) so the replies keep their parent; editing, replying to or reacting to it answers 409 `comment_deleted`. (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.)\n\n**List filters must name a field (v0.75).** A `pad_item` `list` filter on a key no collection in scope declares, and no item stores, is refused with `validation_failed` (\"invalid list filter\"), where it used to return an empty list; a stored undeclared key still filters. Check the schema; don't retry.\n\n**Library words in a blank workspace (v0.67).** A brand-new blank workspace lists one convention trigger (`always`) and one scope (`all`). Activating a library entry, or creating a convention or playbook with a trigger or scope the library itself uses, adds that word to the collection's options and says so in `warnings.options_added`. A trigger you invent is still refused until the workspace owner adds it to the collection's schema.\n\n**App attribution (v0.65).** An item an installed app created, a comment an app wrote and a history row an app's write made carry `via_app` (the install id) and `via_app_name` (the app's name). On an item they name the app that CREATED it, not the last writer. They are absent for anything a person or an agent wrote.\n\n**Relation fields (v0.31).** A `relation` value may be a UUID, an issue ref (`COLO-3`) or the target's EXACT TITLE, tried in that order (a title that is also a ref resolves as the ref). A title matches only inside the declared collection, among items you can see; found only elsewhere, it is refused naming that collection; several matches are `ambiguous`. Full rules: docs/mcp.md.\n\nReads carry a `relation_targets` member beside `fields`: field key → `{id, ref, title}`, so you can render a relation without a request per value. `fields` still holds the canonical id, which is what you write back. An entry with an `id` and NO `ref`/`title` means the target is gone **or** you may not see it — the two are deliberately indistinguishable, so do not render it as either \"deleted\" or \"hidden\"; \"unavailable\" is the honest word. An entry with `stored_as_text: true` differs: the stored value is not UUID-shaped, so it was never an id (an imported title, legacy text). Render it as text, not as unavailable.\n\n**`multi_relation` fields (v0.33).** An ORDERED LIST of references, each resolved like a `relation` (UUID, ref, or exact title in the declared collection). Declare one as `owners:multi_relation:people`; the target collection is required. Rules: `[]` means absent; a blank element is refused by index; order is part of the value; duplicates are refused after resolution (`duplicate_referent`); any failing element refuses the whole write; `required` needs one resolved element; a cross-workspace copy drops the whole value (in `warnings.dropped_fields`). `relation_targets` holds an ARRAY of `{id, ref, title}` in stored order (`id`-only where unresolved); scalar keys stay single objects. Full rules: docs/mcp.md.\n\n- `pad_workspace` — Workspaces: list / members / invite / storage / audit-log / create / claim / deleted / restore.\n- `pad_collection` — Collections: list / create / update / delete / list-archived / restore. `delete` archives: the collection and its items leave every view, and `restore` brings them back. An `update` that removes a field or a select option still in use answers with `warnings.orphaned`: tell the user how many items keep the removed value.\n- `pad_project` — Project intelligence: dashboard / next / ready / stale / standup / changelog / report / activity. Use `ready` for the ranked top items to work on next (blocked work is left out) and `stale` for items needing attention; `activity` to catch up on what other agents/users changed since you last worked (non-streaming feed with item refs + change details).\n- `pad_role` — Agent roles: list / create / update / delete.\n- `pad_search` — Full-text search across items: query.\n- `pad_playbook` — Invokable procedures: list / get / run / match. Use `run` to bind args against a playbook's declared spec and get the rendered body back; side-effect-free. `run` refuses a playbook whose status isn't `active` (a draft still being authored) with a `playbook_not_active` error — pass `allow_draft: true` to override. Both `run` and `get` echo the playbook's `status`. Both may also carry `content_state` (`\"applied_pending_flush\"`, or `\"superseded_set_aside\"` as under `pad_item`), meaning the BODY you are about to execute is behind the item's live collaborative document — an editor holds unflushed edits, so the steps may be superseded. It is a reason to re-read or to ask, not a reason to refuse to run; the body is still the best the server has. `match` finds a playbook from text.\n- `pad_library` — Convention + playbook library (the global catalog of pre-built entries workspaces activate): list / get / activate.\n- `pad_attachment` — Read-only attachment metadata: list / show. `list` enumerates a workspace's attachments (filter by item / category / collection / attached / unattached); `show` returns one attachment's MIME, size, filename, and ETag via a HEAD request. Uploading and general file downloads stay CLI-only; bounded image bytes are available through the attachment resource below.\n- `pad_meta` — Server introspection: server-info / version / tool-surface / bootstrap. The `bootstrap` action returns one-shot workspace context (user + collections + always-on conventions + a metadata-only `convention_index` of every active convention + roles + playbook metadata + dashboard + recent activity).\n- `pad_set_workspace` — Load workspace context; response embeds the bootstrap blob so you load context in one call. Whether it also pins a session default: see Workspace context below. Takes `workspace: \u003cslug\u003e` only (no `action`).\n\nFor the ten resource × action tools, always pass `action` as a top-level field. Per-action required parameters are documented in each tool's description.\n\n## Resources are cheaper than tool calls\n\nRead these directly when you need workspace state (described in the resource listings): `pad://workspaces`, `pad://workspace/{ws}/dashboard`, `.../collections`, `.../items` (use `pad_item` `list` to filter), `.../items/{ref}` (markdown), `.../attachments/{id}` (images only, at most 1 MiB, as base64 `thumb-md`), `.../bootstrap` (the same payload as `pad_meta` `bootstrap`), and `pad://_meta/version`. A host can prefetch them once per session.\n\nLoad the bootstrap resource once per workspace and reuse its collections, conventions, roles, and playbook metadata. Refresh it after switching workspaces, after changing those configuration surfaces, when Pad reports stale schema/context, or when the user asks. For changing work state, read the affected item or use a targeted project tool instead of fetching the whole bootstrap again.\n\n## Workspace context\n\nEvery action that operates within a workspace accepts an optional `workspace` parameter. Resolution order:\n\n1. Explicit `workspace` argument on the call (always wins).\n2. On a single-user local server only: the session default set via `pad_set_workspace`.\n3. On a single-user local server only: the CWD-linked workspace from `.pad.toml`.\n\nA multi-user/remote server does **not** persist a session default — pass `workspace` explicitly on every call. If none resolves, the action returns a structured `no_workspace` error with `available_workspaces`, on both the local and the remote transport.\n\n## Update flow: read first, then patch\n\nFor `pad_item.action: update`, the server merges your patch with the item's current state. Pass only the fields you want to change. When changing `status`, ALWAYS include a `comment` explaining why — it builds the audit trail that helps the team understand history.\n\n## One error code you must not retry\n\nErrors come back as a structured envelope with a `code`. Most failures are worth a retry after you fix something, and a `server_error` may be transient. `stored_state_unreadable` is neither.\n\nIt means the ITEM'S STORED DATA cannot be decoded — your input was fine. Today it fires when `note` / `decide` would append to an `implementation_notes` / `decision_log` value that is not a list of entries. The operation is refused precisely because completing it would overwrite that value. The refusal is fully deterministic, so retrying — immediately or after a backoff — fails identically every time and changes nothing.\n\nWhen you see it: tell the user which item is affected, and stop. Whether you can inspect the bad value depends on which layer is broken — if the item's whole `fields` blob fails to parse, `action: get` hands it back as a raw string; if one structured key is at fault, you won't see it at all, because the blob is normalized on this surface and a value that doesn't decode is dropped from the top-level arrays. A human can always read it with `pad item show \u003cref\u003e --format json`. Do not route around the refusal by writing the field some other way; that is the write that caused it.\n\n## Project conventions\n\nWorkspaces can declare conventions (e.g. \"run `make test` before PR\", \"use conventional commit format\"). The bootstrap blob gives you two views:\n\n- `conventions` — full bodies of the always-on (`trigger=always`) rules. Follow these unconditionally.\n- `convention_index` — METADATA ONLY (`ref`, `title`, `trigger`, `role`; no bodies) for **every** active convention, including the triggered ones whose bodies are NOT in `conventions`. This is your map of what triggered rules exist.\n\nBefore performing meaningful work with a specific trigger (e.g. `on-implement` before writing code), consult `convention_index`: if it lists entries for that trigger, pull their bodies on demand; if it lists none, skip the query.\n\n```\npad_item.action: list, collection: \"conventions\", status: \"active\"\n```\n\nFilter by trigger (`always`, `on-implement`, `on-task-complete`, etc.) when relevant — the `convention_index` triggers tell you which filters are worth running.\n\n`\"conventions\"` above is the DEFAULT collection slug and a workspace may have renamed it, in which case that literal returns nothing while `convention_index` still lists entries — the bootstrap payload resolves by declaration, not by name. When the two disagree, address the items by the `ref`s the index gave you, or find the collection with `pad_collection.action: list`. The same applies to `\"playbooks\"`; `pad_playbook` resolves by declaration and is unaffected.\n\n## Pushes and the session monitor (security note)\n\nPad can push an item into a Claude Code session (`pad push`, or the web item view's push composer). That is terminal instruction injection by design, so receiving is **consent-gated** (PLAN-2613, since v0.15.0): a session receives pushes only after it has connected (`/pad:connect` in the plugin, or a repo-level `push.auto_arm = true` in `.pad.toml` — which a per-user `auto_arm = false` in `~/.pad/config.toml` vetoes), and pushes are self-addressed only — the sender is always the receiving session's own user. This MCP connection is not a session: it cannot arm one, receive a push, or run the monitor, and the `pad_item` / `pad_project` tools expose no push action. If a user asks you to \"push\" something to their terminal, tell them it happens from their own terminal (`pad push \u003cref\u003e -m \"...\"` — the sending session need not be connected, the receiving ones must have consented); do not try to emulate it by writing comments or instructions into items.\n\n## Adding a workspace to this connection\n\nIf the user references a workspace this connection can't see (you'll get a 403 from workspace tools, or the workspace won't appear in `pad_workspace.list`), tell the user you can't see that workspace with your current permissions, then walk them through how to grant access: open Pad in their browser → switch to that workspace → avatar menu → \"Connect project...\" A 6-digit claim code will appear. Have them paste it back in chat, then call `pad_workspace.claim` with `{workspace: \"\u003cslug\u003e\", code: \"\u003c6 digits\u003e\"}`. The workspace joins this connection's allow-list and stays until the user revokes it via `/console/connected-apps`. No re-auth required.\n\nFor brand-new workspaces, `pad_workspace.create` with `{name: \"\u003cname\u003e\"}` (and optional `template`) creates the workspace AND auto-adds it to this connection's allow-list in one call — no claim code needed. Only works when the connection currently carries the \"may create workspaces\" grant — granted at consent time, or enabled later on the connections page. If the grant is present and set to false, the create is **refused with a 403 and no workspace is made** — retrying won't help and neither will the claim flow, since there is nothing to claim. Tell the user this connection isn't permitted to create workspaces, and offer them the two ways forward: create the workspace themselves in the browser and grant access with a claim code, or re-authorize this connection with \"Let this app create new workspaces\" enabled (`/console/connected-apps` can also flip it on an existing connection).\n\n## New workspace: offer to set it up\n\nThe bootstrap blob carries `needs_onboarding: bool` — true when the workspace has zero user-created items (template seeds don't count). When it's true, **lead with an active offer before anything else**: *\"This workspace is brand new and isn't set up yet. Want me to set it up? I'll ask a few quick questions and adapt it to your project.\"*\n\nThis is an **offer, not an auto-run** — wait for the user to say yes before running onboarding. If they accept, run the `onboard` playbook (use the `pad_onboard` prompt, or load the body via `pad_playbook` `action: get`, `ref: onboard`). If they decline, or already declined earlier in the session, respect that and skip the offer. The flag flips to false the moment any item exists, so it won't nag past first setup.\n\n## Built-in updates: mention once\n\nThe bootstrap blob may carry `builtin_updates: int` (absent when there are none): how many of the conventions and playbooks Pad ships have newer text in Pad's library than this workspace's copy. When it is present, mention it **once per session**, in one line, then carry on. **Never apply an update yourself**: a person reviews each one and accepts it or not.\n\n## Multi-step workflows\n\nFour prompts ship with the server: `pad_plan`, `pad_ideate`, `pad_retro`, `pad_onboard`. Use them when the user wants help planning, brainstorming, retrospecting, or onboarding into a workspace — they encode the multi-step Pad-aware playbook for each.\n" } diff --git a/cmd/pad/testdata/mcp_wire/remote_tools_list.json b/cmd/pad/testdata/mcp_wire/remote_tools_list.json index ec4e582c8..3572435d4 100644 --- a/cmd/pad/testdata/mcp_wire/remote_tools_list.json +++ b/cmd/pad/testdata/mcp_wire/remote_tools_list.json @@ -353,7 +353,7 @@ "type": "number" }, "op_key": { - "description": "patch, create: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused).", + "description": "patch, create, comment, note, decide: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused).", "type": "string" }, "overwrite_pending_edits": { diff --git a/cmd/pad/testdata/mcp_wire/stdio_initialize.json b/cmd/pad/testdata/mcp_wire/stdio_initialize.json index c25a59a4a..f7f90f8f9 100644 --- a/cmd/pad/testdata/mcp_wire/stdio_initialize.json +++ b/cmd/pad/testdata/mcp_wire/stdio_initialize.json @@ -8,7 +8,7 @@ }, "padToolSurface": { "tool_surface_stable": true, - "version": "0.91" + "version": "0.92" } }, "prompts": {}, @@ -21,5 +21,5 @@ "name": "pad-mcp", "version": "" }, - "instructions": "Pad is a project tracker for developers and AI agents — issues (TASK, BUG), plans (PLAN), ideas (IDEA), docs (DOC), conventions, comments, and dependencies. Use this server when a user mentions:\n\n- Issue refs like `TASK-5`, `BUG-12`, `PLAN-3`, `IDEA-8` — they are stable, human-readable IDs and the canonical way to address items. Use them, never slugs; they are what audit trails and PR titles show.\n- Tasks / issues / items / plans / progress / \"what's on my plate\" / \"what to work on next\" / standup / changelog / retrospective.\n- Project conventions, decision records, or \"how should this team do X.\"\n\nIf the user is asking general code questions with no project-management thread, you don't need this server.\n\n## Tool surface (v0.91)\n\nTen resource × action tools, plus `pad_set_workspace`. Eleven tools total.\n\nInputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped.\n\n- `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 / patch. 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. To change PART of a body, prefer `patch`: `edits: [{old, new}]`, where each `old` is exact text occurring once in the current body (read it with `get` and `lines: \"A-B\"` to quote it). The answer is a unified `diff`: read it, because anchors guarantee position, not meaning. A moved `expected_seq` is applied over and reported as `rebased` with the intervening writes. On an item open in a browser editor, the editor applies the patch to its live document, merging with the typing there, and the answer says `content_state: \"applied_pending_flush\"`. `patch_anchor_missing` / `patch_anchor_ambiguous` mean re-read and quote current text; `patch_unstable_roundtrip` means the open editor would also change content you did not edit, so send a whole-body update; `patch_room_open` means an older editor that cannot apply patches holds the item. Every patch answer carries a `patch_id`; `patch` with `undo: \u003cpatch_id\u003e` inverts it, and is refused when the text it changed has changed since. Send an `op_key` (any unique string, e.g. a UUID) with a patch whenever you might retry it: the same request with the same key returns the first answer instead of applying twice. `patch_outcome_unknown` means an earlier attempt's editor never confirmed it and is gone; read `details.diff` before deciding. `content` REPLACES the whole body, so send `expected_seq` with it: `get` the item, edit that body, and send it back with the `seq` you read. An `update_conflict` whose details carry `content_write` means someone changed the item after your read. Do NOT resend your body, because it would undo their change: re-read, apply your edit to the current content, and send that with the new seq. `replace: true` overwrites on purpose instead. A content write with neither carries `warnings.unguarded_content_write`, and a later release refuses it. 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: the server normally recovers the stored copy itself about a minute after the last tab leaves the item, but that is not guaranteed (a recovery can fail, and an operator can turn it off), and otherwise the row catches up only when a tab next opens the item. 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`: claim an item before you work it; a loser's 409 is `lease_held` (pick another, don't retry), and a lease bumps neither `updated_at` nor `seq`. The holder defaults to your agent name, and `pad_project` next / ready leave out items another holder has leased. `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. Deleting a comment that has replies leaves a placeholder (`deleted: true`, empty body) so the replies keep their parent; editing, replying to or reacting to it answers 409 `comment_deleted`. (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.)\n\n**List filters must name a field (v0.75).** A `pad_item` `list` filter on a key no collection in scope declares, and no item stores, is refused with `validation_failed` (\"invalid list filter\"), where it used to return an empty list; a stored undeclared key still filters. Check the schema; don't retry.\n\n**Library words in a blank workspace (v0.67).** A brand-new blank workspace lists one convention trigger (`always`) and one scope (`all`). Activating a library entry, or creating a convention or playbook with a trigger or scope the library itself uses, adds that word to the collection's options and says so in `warnings.options_added`. A trigger you invent is still refused until the workspace owner adds it to the collection's schema.\n\n**App attribution (v0.65).** An item an installed app created, a comment an app wrote and a history row an app's write made carry `via_app` (the install id) and `via_app_name` (the app's name). On an item they name the app that CREATED it, not the last writer. They are absent for anything a person or an agent wrote.\n\n**Relation fields (v0.31).** A `relation` value may be a UUID, an issue ref (`COLO-3`) or the target's EXACT TITLE, tried in that order (a title that is also a ref resolves as the ref). A title matches only inside the declared collection, among items you can see; found only elsewhere, it is refused naming that collection; several matches are `ambiguous`. Full rules: docs/mcp.md.\n\nReads carry a `relation_targets` member beside `fields`: field key → `{id, ref, title}`, so you can render a relation without a request per value. `fields` still holds the canonical id, which is what you write back. An entry with an `id` and NO `ref`/`title` means the target is gone **or** you may not see it — the two are deliberately indistinguishable, so do not render it as either \"deleted\" or \"hidden\"; \"unavailable\" is the honest word. An entry with `stored_as_text: true` differs: the stored value is not UUID-shaped, so it was never an id (an imported title, legacy text). Render it as text, not as unavailable.\n\n**`multi_relation` fields (v0.33).** An ORDERED LIST of references, each resolved like a `relation` (UUID, ref, or exact title in the declared collection). Declare one as `owners:multi_relation:people`; the target collection is required. Rules: `[]` means absent; a blank element is refused by index; order is part of the value; duplicates are refused after resolution (`duplicate_referent`); any failing element refuses the whole write; `required` needs one resolved element; a cross-workspace copy drops the whole value (in `warnings.dropped_fields`). `relation_targets` holds an ARRAY of `{id, ref, title}` in stored order (`id`-only where unresolved); scalar keys stay single objects. Full rules: docs/mcp.md.\n\n- `pad_workspace` — Workspaces: list / members / invite / storage / audit-log / create / claim / deleted / restore.\n- `pad_collection` — Collections: list / create / update / delete / list-archived / restore. `delete` archives: the collection and its items leave every view, and `restore` brings them back. An `update` that removes a field or a select option still in use answers with `warnings.orphaned`: tell the user how many items keep the removed value.\n- `pad_project` — Project intelligence: dashboard / next / ready / stale / standup / changelog / report / activity. Use `ready` for the ranked top items to work on next (blocked work is left out) and `stale` for items needing attention; `activity` to catch up on what other agents/users changed since you last worked (non-streaming feed with item refs + change details).\n- `pad_role` — Agent roles: list / create / update / delete.\n- `pad_search` — Full-text search across items: query.\n- `pad_playbook` — Invokable procedures: list / get / run / match. Use `run` to bind args against a playbook's declared spec and get the rendered body back; side-effect-free. `run` refuses a playbook whose status isn't `active` (a draft still being authored) with a `playbook_not_active` error — pass `allow_draft: true` to override. Both `run` and `get` echo the playbook's `status`. Both may also carry `content_state` (`\"applied_pending_flush\"`, or `\"superseded_set_aside\"` as under `pad_item`), meaning the BODY you are about to execute is behind the item's live collaborative document — an editor holds unflushed edits, so the steps may be superseded. It is a reason to re-read or to ask, not a reason to refuse to run; the body is still the best the server has. `match` finds a playbook from text.\n- `pad_library` — Convention + playbook library (the global catalog of pre-built entries workspaces activate): list / get / activate.\n- `pad_attachment` — Read-only attachment metadata: list / show. `list` enumerates a workspace's attachments (filter by item / category / collection / attached / unattached); `show` returns one attachment's MIME, size, filename, and ETag via a HEAD request. Uploading and general file downloads stay CLI-only; bounded image bytes are available through the attachment resource below.\n- `pad_meta` — Server introspection: server-info / version / tool-surface / bootstrap. The `bootstrap` action returns one-shot workspace context (user + collections + always-on conventions + a metadata-only `convention_index` of every active convention + roles + playbook metadata + dashboard + recent activity).\n- `pad_set_workspace` — Load workspace context; response embeds the bootstrap blob so you load context in one call. Whether it also pins a session default: see Workspace context below. Takes `workspace: \u003cslug\u003e` only (no `action`).\n\nFor the ten resource × action tools, always pass `action` as a top-level field. Per-action required parameters are documented in each tool's description.\n\n## Resources are cheaper than tool calls\n\nRead these directly when you need workspace state (described in the resource listings): `pad://workspaces`, `pad://workspace/{ws}/dashboard`, `.../collections`, `.../items` (use `pad_item` `list` to filter), `.../items/{ref}` (markdown), `.../attachments/{id}` (images only, at most 1 MiB, as base64 `thumb-md`), `.../bootstrap` (the same payload as `pad_meta` `bootstrap`), and `pad://_meta/version`. A host can prefetch them once per session.\n\nLoad the bootstrap resource once per workspace and reuse its collections, conventions, roles, and playbook metadata. Refresh it after switching workspaces, after changing those configuration surfaces, when Pad reports stale schema/context, or when the user asks. For changing work state, read the affected item or use a targeted project tool instead of fetching the whole bootstrap again.\n\n## Workspace context\n\nEvery action that operates within a workspace accepts an optional `workspace` parameter. Resolution order:\n\n1. Explicit `workspace` argument on the call (always wins).\n2. On a single-user local server only: the session default set via `pad_set_workspace`.\n3. On a single-user local server only: the CWD-linked workspace from `.pad.toml`.\n\nA multi-user/remote server does **not** persist a session default — pass `workspace` explicitly on every call. If none resolves, the action returns a structured `no_workspace` error with `available_workspaces`, on both the local and the remote transport.\n\n## Update flow: read first, then patch\n\nFor `pad_item.action: update`, the server merges your patch with the item's current state. Pass only the fields you want to change. When changing `status`, ALWAYS include a `comment` explaining why — it builds the audit trail that helps the team understand history.\n\n## One error code you must not retry\n\nErrors come back as a structured envelope with a `code`. Most failures are worth a retry after you fix something, and a `server_error` may be transient. `stored_state_unreadable` is neither.\n\nIt means the ITEM'S STORED DATA cannot be decoded — your input was fine. Today it fires when `note` / `decide` would append to an `implementation_notes` / `decision_log` value that is not a list of entries. The operation is refused precisely because completing it would overwrite that value. The refusal is fully deterministic, so retrying — immediately or after a backoff — fails identically every time and changes nothing.\n\nWhen you see it: tell the user which item is affected, and stop. Whether you can inspect the bad value depends on which layer is broken — if the item's whole `fields` blob fails to parse, `action: get` hands it back as a raw string; if one structured key is at fault, you won't see it at all, because the blob is normalized on this surface and a value that doesn't decode is dropped from the top-level arrays. A human can always read it with `pad item show \u003cref\u003e --format json`. Do not route around the refusal by writing the field some other way; that is the write that caused it.\n\n## Project conventions\n\nWorkspaces can declare conventions (e.g. \"run `make test` before PR\", \"use conventional commit format\"). The bootstrap blob gives you two views:\n\n- `conventions` — full bodies of the always-on (`trigger=always`) rules. Follow these unconditionally.\n- `convention_index` — METADATA ONLY (`ref`, `title`, `trigger`, `role`; no bodies) for **every** active convention, including the triggered ones whose bodies are NOT in `conventions`. This is your map of what triggered rules exist.\n\nBefore performing meaningful work with a specific trigger (e.g. `on-implement` before writing code), consult `convention_index`: if it lists entries for that trigger, pull their bodies on demand; if it lists none, skip the query.\n\n```\npad_item.action: list, collection: \"conventions\", status: \"active\"\n```\n\nFilter by trigger (`always`, `on-implement`, `on-task-complete`, etc.) when relevant — the `convention_index` triggers tell you which filters are worth running.\n\n`\"conventions\"` above is the DEFAULT collection slug and a workspace may have renamed it, in which case that literal returns nothing while `convention_index` still lists entries — the bootstrap payload resolves by declaration, not by name. When the two disagree, address the items by the `ref`s the index gave you, or find the collection with `pad_collection.action: list`. The same applies to `\"playbooks\"`; `pad_playbook` resolves by declaration and is unaffected.\n\n## Pushes and the session monitor (security note)\n\nPad can push an item into a Claude Code session (`pad push`, or the web item view's push composer). That is terminal instruction injection by design, so receiving is **consent-gated** (PLAN-2613, since v0.15.0): a session receives pushes only after it has connected (`/pad:connect` in the plugin, or a repo-level `push.auto_arm = true` in `.pad.toml` — which a per-user `auto_arm = false` in `~/.pad/config.toml` vetoes), and pushes are self-addressed only — the sender is always the receiving session's own user. This MCP connection is not a session: it cannot arm one, receive a push, or run the monitor, and the `pad_item` / `pad_project` tools expose no push action. If a user asks you to \"push\" something to their terminal, tell them it happens from their own terminal (`pad push \u003cref\u003e -m \"...\"` — the sending session need not be connected, the receiving ones must have consented); do not try to emulate it by writing comments or instructions into items.\n\n## Adding a workspace to this connection\n\nIf the user references a workspace this connection can't see (you'll get a 403 from workspace tools, or the workspace won't appear in `pad_workspace.list`), tell the user you can't see that workspace with your current permissions, then walk them through how to grant access: open Pad in their browser → switch to that workspace → avatar menu → \"Connect project...\" A 6-digit claim code will appear. Have them paste it back in chat, then call `pad_workspace.claim` with `{workspace: \"\u003cslug\u003e\", code: \"\u003c6 digits\u003e\"}`. The workspace joins this connection's allow-list and stays until the user revokes it via `/console/connected-apps`. No re-auth required.\n\nFor brand-new workspaces, `pad_workspace.create` with `{name: \"\u003cname\u003e\"}` (and optional `template`) creates the workspace AND auto-adds it to this connection's allow-list in one call — no claim code needed. Only works when the connection currently carries the \"may create workspaces\" grant — granted at consent time, or enabled later on the connections page. If the grant is present and set to false, the create is **refused with a 403 and no workspace is made** — retrying won't help and neither will the claim flow, since there is nothing to claim. Tell the user this connection isn't permitted to create workspaces, and offer them the two ways forward: create the workspace themselves in the browser and grant access with a claim code, or re-authorize this connection with \"Let this app create new workspaces\" enabled (`/console/connected-apps` can also flip it on an existing connection).\n\n## New workspace: offer to set it up\n\nThe bootstrap blob carries `needs_onboarding: bool` — true when the workspace has zero user-created items (template seeds don't count). When it's true, **lead with an active offer before anything else**: *\"This workspace is brand new and isn't set up yet. Want me to set it up? I'll ask a few quick questions and adapt it to your project.\"*\n\nThis is an **offer, not an auto-run** — wait for the user to say yes before running onboarding. If they accept, run the `onboard` playbook (use the `pad_onboard` prompt, or load the body via `pad_playbook` `action: get`, `ref: onboard`). If they decline, or already declined earlier in the session, respect that and skip the offer. The flag flips to false the moment any item exists, so it won't nag past first setup.\n\n## Built-in updates: mention once\n\nThe bootstrap blob may carry `builtin_updates: int` (absent when there are none): how many of the conventions and playbooks Pad ships have newer text in Pad's library than this workspace's copy. When it is present, mention it **once per session**, in one line, then carry on. **Never apply an update yourself**: a person reviews each one and accepts it or not.\n\n## Multi-step workflows\n\nFour prompts ship with the server: `pad_plan`, `pad_ideate`, `pad_retro`, `pad_onboard`. Use them when the user wants help planning, brainstorming, retrospecting, or onboarding into a workspace — they encode the multi-step Pad-aware playbook for each.\n" + "instructions": "Pad is a project tracker for developers and AI agents — issues (TASK, BUG), plans (PLAN), ideas (IDEA), docs (DOC), conventions, comments, and dependencies. Use this server when a user mentions:\n\n- Issue refs like `TASK-5`, `BUG-12`, `PLAN-3`, `IDEA-8` — they are stable, human-readable IDs and the canonical way to address items. Use them, never slugs; they are what audit trails and PR titles show.\n- Tasks / issues / items / plans / progress / \"what's on my plate\" / \"what to work on next\" / standup / changelog / retrospective.\n- Project conventions, decision records, or \"how should this team do X.\"\n\nIf the user is asking general code questions with no project-management thread, you don't need this server.\n\n## Tool surface (v0.92)\n\nTen resource × action tools, plus `pad_set_workspace`. Eleven tools total.\n\nInputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped.\n\n- `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 / patch. 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. To change PART of a body, prefer `patch`: `edits: [{old, new}]`, where each `old` is exact text occurring once in the current body (read it with `get` and `lines: \"A-B\"` to quote it). The answer is a unified `diff`: read it, because anchors guarantee position, not meaning. A moved `expected_seq` is applied over and reported as `rebased` with the intervening writes. On an item open in a browser editor, the editor applies the patch to its live document, merging with the typing there, and the answer says `content_state: \"applied_pending_flush\"`. `patch_anchor_missing` / `patch_anchor_ambiguous` mean re-read and quote current text; `patch_unstable_roundtrip` means the open editor would also change content you did not edit, so send a whole-body update; `patch_room_open` means an older editor that cannot apply patches holds the item. Every patch answer carries a `patch_id`; `patch` with `undo: \u003cpatch_id\u003e` inverts it, and is refused when the text it changed has changed since. Send an `op_key` (any unique string, e.g. a UUID) with a patch whenever you might retry it: the same request with the same key returns the first answer instead of applying twice. `patch_outcome_unknown` means an earlier attempt's editor never confirmed it and is gone; read `details.diff` before deciding. `content` REPLACES the whole body, so send `expected_seq` with it: `get` the item, edit that body, and send it back with the `seq` you read. An `update_conflict` whose details carry `content_write` means someone changed the item after your read. Do NOT resend your body, because it would undo their change: re-read, apply your edit to the current content, and send that with the new seq. `replace: true` overwrites on purpose instead. A content write with neither carries `warnings.unguarded_content_write`, and a later release refuses it. 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: the server normally recovers the stored copy itself about a minute after the last tab leaves the item, but that is not guaranteed (a recovery can fail, and an operator can turn it off), and otherwise the row catches up only when a tab next opens the item. 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`: claim an item before you work it; a loser's 409 is `lease_held` (pick another, don't retry), and a lease bumps neither `updated_at` nor `seq`. The holder defaults to your agent name, and `pad_project` next / ready leave out items another holder has leased. `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. Deleting a comment that has replies leaves a placeholder (`deleted: true`, empty body) so the replies keep their parent; editing, replying to or reacting to it answers 409 `comment_deleted`. (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.)\n\n**List filters must name a field (v0.75).** A `pad_item` `list` filter on a key no collection in scope declares, and no item stores, is refused with `validation_failed` (\"invalid list filter\"), where it used to return an empty list; a stored undeclared key still filters. Check the schema; don't retry.\n\n**Library words in a blank workspace (v0.67).** A brand-new blank workspace lists one convention trigger (`always`) and one scope (`all`). Activating a library entry, or creating a convention or playbook with a trigger or scope the library itself uses, adds that word to the collection's options and says so in `warnings.options_added`. A trigger you invent is still refused until the workspace owner adds it to the collection's schema.\n\n**App attribution (v0.65).** An item an installed app created, a comment an app wrote and a history row an app's write made carry `via_app` (the install id) and `via_app_name` (the app's name). On an item they name the app that CREATED it, not the last writer. They are absent for anything a person or an agent wrote.\n\n**Relation fields (v0.31).** A `relation` value may be a UUID, an issue ref (`COLO-3`) or the target's EXACT TITLE, tried in that order (a title that is also a ref resolves as the ref). A title matches only inside the declared collection, among items you can see; found only elsewhere, it is refused naming that collection; several matches are `ambiguous`. Full rules: docs/mcp.md.\n\nReads carry a `relation_targets` member beside `fields`: field key → `{id, ref, title}`, so you can render a relation without a request per value. `fields` still holds the canonical id, which is what you write back. An entry with an `id` and NO `ref`/`title` means the target is gone **or** you may not see it — the two are deliberately indistinguishable, so do not render it as either \"deleted\" or \"hidden\"; \"unavailable\" is the honest word. An entry with `stored_as_text: true` differs: the stored value is not UUID-shaped, so it was never an id (an imported title, legacy text). Render it as text, not as unavailable.\n\n**`multi_relation` fields (v0.33).** An ORDERED LIST of references, each resolved like a `relation` (UUID, ref, or exact title in the declared collection). Declare one as `owners:multi_relation:people`; the target collection is required. Rules: `[]` means absent; a blank element is refused by index; order is part of the value; duplicates are refused after resolution (`duplicate_referent`); any failing element refuses the whole write; `required` needs one resolved element; a cross-workspace copy drops the whole value (in `warnings.dropped_fields`). `relation_targets` holds an ARRAY of `{id, ref, title}` in stored order (`id`-only where unresolved); scalar keys stay single objects. Full rules: docs/mcp.md.\n\n- `pad_workspace` — Workspaces: list / members / invite / storage / audit-log / create / claim / deleted / restore.\n- `pad_collection` — Collections: list / create / update / delete / list-archived / restore. `delete` archives: the collection and its items leave every view, and `restore` brings them back. An `update` that removes a field or a select option still in use answers with `warnings.orphaned`: tell the user how many items keep the removed value.\n- `pad_project` — Project intelligence: dashboard / next / ready / stale / standup / changelog / report / activity. Use `ready` for the ranked top items to work on next (blocked work is left out) and `stale` for items needing attention; `activity` to catch up on what other agents/users changed since you last worked (non-streaming feed with item refs + change details).\n- `pad_role` — Agent roles: list / create / update / delete.\n- `pad_search` — Full-text search across items: query.\n- `pad_playbook` — Invokable procedures: list / get / run / match. Use `run` to bind args against a playbook's declared spec and get the rendered body back; side-effect-free. `run` refuses a playbook whose status isn't `active` (a draft still being authored) with a `playbook_not_active` error — pass `allow_draft: true` to override. Both `run` and `get` echo the playbook's `status`. Both may also carry `content_state` (`\"applied_pending_flush\"`, or `\"superseded_set_aside\"` as under `pad_item`), meaning the BODY you are about to execute is behind the item's live collaborative document — an editor holds unflushed edits, so the steps may be superseded. It is a reason to re-read or to ask, not a reason to refuse to run; the body is still the best the server has. `match` finds a playbook from text.\n- `pad_library` — Convention + playbook library (the global catalog of pre-built entries workspaces activate): list / get / activate.\n- `pad_attachment` — Read-only attachment metadata: list / show. `list` enumerates a workspace's attachments (filter by item / category / collection / attached / unattached); `show` returns one attachment's MIME, size, filename, and ETag via a HEAD request. Uploading and general file downloads stay CLI-only; bounded image bytes are available through the attachment resource below.\n- `pad_meta` — Server introspection: server-info / version / tool-surface / bootstrap. The `bootstrap` action returns one-shot workspace context (user + collections + always-on conventions + a metadata-only `convention_index` of every active convention + roles + playbook metadata + dashboard + recent activity).\n- `pad_set_workspace` — Load workspace context; response embeds the bootstrap blob so you load context in one call. Whether it also pins a session default: see Workspace context below. Takes `workspace: \u003cslug\u003e` only (no `action`).\n\nFor the ten resource × action tools, always pass `action` as a top-level field. Per-action required parameters are documented in each tool's description.\n\n## Resources are cheaper than tool calls\n\nRead these directly when you need workspace state (described in the resource listings): `pad://workspaces`, `pad://workspace/{ws}/dashboard`, `.../collections`, `.../items` (use `pad_item` `list` to filter), `.../items/{ref}` (markdown), `.../attachments/{id}` (images only, at most 1 MiB, as base64 `thumb-md`), `.../bootstrap` (the same payload as `pad_meta` `bootstrap`), and `pad://_meta/version`. A host can prefetch them once per session.\n\nLoad the bootstrap resource once per workspace and reuse its collections, conventions, roles, and playbook metadata. Refresh it after switching workspaces, after changing those configuration surfaces, when Pad reports stale schema/context, or when the user asks. For changing work state, read the affected item or use a targeted project tool instead of fetching the whole bootstrap again.\n\n## Workspace context\n\nEvery action that operates within a workspace accepts an optional `workspace` parameter. Resolution order:\n\n1. Explicit `workspace` argument on the call (always wins).\n2. On a single-user local server only: the session default set via `pad_set_workspace`.\n3. On a single-user local server only: the CWD-linked workspace from `.pad.toml`.\n\nA multi-user/remote server does **not** persist a session default — pass `workspace` explicitly on every call. If none resolves, the action returns a structured `no_workspace` error with `available_workspaces`, on both the local and the remote transport.\n\n## Update flow: read first, then patch\n\nFor `pad_item.action: update`, the server merges your patch with the item's current state. Pass only the fields you want to change. When changing `status`, ALWAYS include a `comment` explaining why — it builds the audit trail that helps the team understand history.\n\n## One error code you must not retry\n\nErrors come back as a structured envelope with a `code`. Most failures are worth a retry after you fix something, and a `server_error` may be transient. `stored_state_unreadable` is neither.\n\nIt means the ITEM'S STORED DATA cannot be decoded — your input was fine. Today it fires when `note` / `decide` would append to an `implementation_notes` / `decision_log` value that is not a list of entries. The operation is refused precisely because completing it would overwrite that value. The refusal is fully deterministic, so retrying — immediately or after a backoff — fails identically every time and changes nothing.\n\nWhen you see it: tell the user which item is affected, and stop. Whether you can inspect the bad value depends on which layer is broken — if the item's whole `fields` blob fails to parse, `action: get` hands it back as a raw string; if one structured key is at fault, you won't see it at all, because the blob is normalized on this surface and a value that doesn't decode is dropped from the top-level arrays. A human can always read it with `pad item show \u003cref\u003e --format json`. Do not route around the refusal by writing the field some other way; that is the write that caused it.\n\n## Project conventions\n\nWorkspaces can declare conventions (e.g. \"run `make test` before PR\", \"use conventional commit format\"). The bootstrap blob gives you two views:\n\n- `conventions` — full bodies of the always-on (`trigger=always`) rules. Follow these unconditionally.\n- `convention_index` — METADATA ONLY (`ref`, `title`, `trigger`, `role`; no bodies) for **every** active convention, including the triggered ones whose bodies are NOT in `conventions`. This is your map of what triggered rules exist.\n\nBefore performing meaningful work with a specific trigger (e.g. `on-implement` before writing code), consult `convention_index`: if it lists entries for that trigger, pull their bodies on demand; if it lists none, skip the query.\n\n```\npad_item.action: list, collection: \"conventions\", status: \"active\"\n```\n\nFilter by trigger (`always`, `on-implement`, `on-task-complete`, etc.) when relevant — the `convention_index` triggers tell you which filters are worth running.\n\n`\"conventions\"` above is the DEFAULT collection slug and a workspace may have renamed it, in which case that literal returns nothing while `convention_index` still lists entries — the bootstrap payload resolves by declaration, not by name. When the two disagree, address the items by the `ref`s the index gave you, or find the collection with `pad_collection.action: list`. The same applies to `\"playbooks\"`; `pad_playbook` resolves by declaration and is unaffected.\n\n## Pushes and the session monitor (security note)\n\nPad can push an item into a Claude Code session (`pad push`, or the web item view's push composer). That is terminal instruction injection by design, so receiving is **consent-gated** (PLAN-2613, since v0.15.0): a session receives pushes only after it has connected (`/pad:connect` in the plugin, or a repo-level `push.auto_arm = true` in `.pad.toml` — which a per-user `auto_arm = false` in `~/.pad/config.toml` vetoes), and pushes are self-addressed only — the sender is always the receiving session's own user. This MCP connection is not a session: it cannot arm one, receive a push, or run the monitor, and the `pad_item` / `pad_project` tools expose no push action. If a user asks you to \"push\" something to their terminal, tell them it happens from their own terminal (`pad push \u003cref\u003e -m \"...\"` — the sending session need not be connected, the receiving ones must have consented); do not try to emulate it by writing comments or instructions into items.\n\n## Adding a workspace to this connection\n\nIf the user references a workspace this connection can't see (you'll get a 403 from workspace tools, or the workspace won't appear in `pad_workspace.list`), tell the user you can't see that workspace with your current permissions, then walk them through how to grant access: open Pad in their browser → switch to that workspace → avatar menu → \"Connect project...\" A 6-digit claim code will appear. Have them paste it back in chat, then call `pad_workspace.claim` with `{workspace: \"\u003cslug\u003e\", code: \"\u003c6 digits\u003e\"}`. The workspace joins this connection's allow-list and stays until the user revokes it via `/console/connected-apps`. No re-auth required.\n\nFor brand-new workspaces, `pad_workspace.create` with `{name: \"\u003cname\u003e\"}` (and optional `template`) creates the workspace AND auto-adds it to this connection's allow-list in one call — no claim code needed. Only works when the connection currently carries the \"may create workspaces\" grant — granted at consent time, or enabled later on the connections page. If the grant is present and set to false, the create is **refused with a 403 and no workspace is made** — retrying won't help and neither will the claim flow, since there is nothing to claim. Tell the user this connection isn't permitted to create workspaces, and offer them the two ways forward: create the workspace themselves in the browser and grant access with a claim code, or re-authorize this connection with \"Let this app create new workspaces\" enabled (`/console/connected-apps` can also flip it on an existing connection).\n\n## New workspace: offer to set it up\n\nThe bootstrap blob carries `needs_onboarding: bool` — true when the workspace has zero user-created items (template seeds don't count). When it's true, **lead with an active offer before anything else**: *\"This workspace is brand new and isn't set up yet. Want me to set it up? I'll ask a few quick questions and adapt it to your project.\"*\n\nThis is an **offer, not an auto-run** — wait for the user to say yes before running onboarding. If they accept, run the `onboard` playbook (use the `pad_onboard` prompt, or load the body via `pad_playbook` `action: get`, `ref: onboard`). If they decline, or already declined earlier in the session, respect that and skip the offer. The flag flips to false the moment any item exists, so it won't nag past first setup.\n\n## Built-in updates: mention once\n\nThe bootstrap blob may carry `builtin_updates: int` (absent when there are none): how many of the conventions and playbooks Pad ships have newer text in Pad's library than this workspace's copy. When it is present, mention it **once per session**, in one line, then carry on. **Never apply an update yourself**: a person reviews each one and accepts it or not.\n\n## Multi-step workflows\n\nFour prompts ship with the server: `pad_plan`, `pad_ideate`, `pad_retro`, `pad_onboard`. Use them when the user wants help planning, brainstorming, retrospecting, or onboarding into a workspace — they encode the multi-step Pad-aware playbook for each.\n" } diff --git a/cmd/pad/testdata/mcp_wire/stdio_tools_list.json b/cmd/pad/testdata/mcp_wire/stdio_tools_list.json index 40ef07ab4..28c1649b4 100644 --- a/cmd/pad/testdata/mcp_wire/stdio_tools_list.json +++ b/cmd/pad/testdata/mcp_wire/stdio_tools_list.json @@ -353,7 +353,7 @@ "type": "number" }, "op_key": { - "description": "patch, create: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused).", + "description": "patch, create, comment, note, decide: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused).", "type": "string" }, "overwrite_pending_edits": { diff --git a/docs/api.md b/docs/api.md index 226f674e8..7f19a4dfa 100644 --- a/docs/api.md +++ b/docs/api.md @@ -36,7 +36,7 @@ REST API at `/api/v1/`. Key endpoints: - **Refusals** (nothing written): 400 `bad_request` for no edits or an empty `old` (`details.edit`); 409 `patch_anchor_missing` (`details.edit`); 409 `patch_anchor_ambiguous` (`details.edit`, `details.lines`); 409 `patch_room_open` when only an editor that cannot apply patches holds the item (no patch-capable tab, never a whole-document replace). Also every PATCH guard: unflushed editor edits refuse it as `content_pending_flush` token or not, set-aside rows, permissions. - **Open item (step 2):** when a live editor holds the item, the patch goes to a patch-capable tab (one that connected with `?patch_capable=1`; only such a tab is ever sent one, because an older tab would read an edits-only request as an empty body). The tab applies it to its LIVE document as a narrowed replace of exactly the changed top-level blocks, aligning blocks with the ruled equivalence (equal, or differing only in a link's title or bold/italic on whitespace; a trailing empty paragraph is ignored). It acks with the canonical `before`/`after`, from which the server builds the diff. A row write then records the patched body as a version attributed to the request (`ExternalContent`) and moves the seq; the answer adds `content_state: "applied_pending_flush"`. Unflushed editor edits do not refuse it, because the tab applies to the document that holds them. The tab refuses `patch_unstable_roundtrip` (`details.line`, `details.reason`) when a changed block has no equivalent live block. Refusals are counted in `pad_patch_refusals_total{reason}`. - **Undo (step 3):** `{undo: ""}` instead of `edits` inverts a stored patch. Every patch records its applied edits in `item_patches` (`{old, new, ctx_before, ctx_after}`, the context up to 40 characters each side of the replacement as it stood right after that edit, clipped at line ends) in the same transaction as its version row, on both paths. The undo anchors each inverted edit on `ctx_before + new + ctx_after`, in reverse order, under the ordinary rules, so it refuses (`patch_anchor_missing` / `patch_anchor_ambiguous`, nothing written) when that text has changed since: it never reverts someone else's later edit. The undo is itself a patch (its own `patch_id`, version and record) and answers with `undo_of`. 404 `patch_not_found` when the id is not a patch on this item (workspace-scoped, and checked against the item inside the transaction); 400 for `undo` together with `edits`. `item_patches` has no read surface besides undo and is not exported; it goes with its item (`ON DELETE CASCADE`, plus the workspace purge list). - - **Idempotency keys (TASK-3585):** an `Idempotency-Key` header, or `op_key` in the body (not part of the request), makes a retry safe. The key is claimed per (workspace, user, key) BEFORE any write-path guard (so a retry of a write that committed replays it instead of meeting its own success as a conflict), with a 60s lease, and its answer is recorded INSIDE the write's own transaction, guarded by the claim token: `done` holds exactly when the write committed, so a claim whose lease expired committed nothing and may be taken over. A retry of the same request (route + item + canonical body) answers the stored result with `Idempotent-Replayed: true`; another request under the key is 422 `idempotency_key_reused`; a live claim is 409 `idempotency_in_flight` (`Retry-After`). Refusals are not stored: the key is released, so a corrected retry with the same key proceeds. When an open editor received the patch and never confirmed it, the key records the patch id and that connection; a retry goes ONLY to that connection, whose tab remembers the patches it applied and re-acks one it is sent again. With that connection gone it answers 409 `patch_outcome_unknown` (`seq`, `base_seq`, `diff` since then) and re-applies nothing, because an insert-shaped edit would keep its anchor elsewhere. Keys live 24h (swept); capability `idempotency_keys`. The CLI sends a fresh key per invocation and retries once with it on a transport error or a 5xx. + - **Idempotency keys (TASK-3585):** an `Idempotency-Key` header, or `op_key` in the body (not part of the request), makes a retry safe. The key is claimed per (workspace, user, key) BEFORE any write-path guard (so a retry of a write that committed replays it instead of meeting its own success as a conflict), with a 60s lease, and its answer is recorded INSIDE the write's own transaction, guarded by the claim token: `done` holds exactly when the write committed, so a claim whose lease expired committed nothing and may be taken over. A retry of the same request (route + item + canonical body) answers the stored result with `Idempotent-Replayed: true`; another request under the key is 422 `idempotency_key_reused`; a live claim is 409 `idempotency_in_flight` (`Retry-After`). Refusals are not stored: the key is released, so a corrected retry with the same key proceeds. When an open editor received the patch and never confirmed it, the key records the patch id and that connection; a retry goes ONLY to that connection, whose tab remembers the patches it applied and re-acks one it is sent again. With that connection gone it answers 409 `patch_outcome_unknown` (`seq`, `base_seq`, `diff` since then) and re-applies nothing, because an insert-shaped edit would keep its anchor elsewhere. Keys live 24h (swept); capability `idempotency_keys`. The CLI sends a fresh key per invocation and retries once with it on a transport error or a 5xx, and only when the server advertises `idempotency_keys` (an older server ignores the key, so a retry there could write twice). The same key works on item create (`POST .../collections/{c}/items`, which replays the created item as it stands now), comment create (`POST .../items/{ref}/comments`) and the append-only item update (`PATCH .../items/{ref}` with `append_implementation_note` and/or `append_decision`, plus `source`; a key with any other member is refused 400, and a replay echoes the first entries in `appended`). A replay whose item or comment has since been deleted is 410 `idempotency_target_gone`. - **Capability:** `item_patch`; the CLI refuses against a server without it, whose bare 404 would read as a missing item. - **Unguarded writes are counted, and an agent door's unguarded content write is flagged** (PLAN-3555 phase 1). An update that carries neither `expected_seq` / `expected_updated_at` nor `replace: true` is counted in `pad_content_writes_unguarded_total{door,kind}`. `door` is `web` (a browser session), `cli` (the CLI, including local stdio MCP, which runs it), `api` (any other token caller), `mcp_remote` or `chatgpt`. `kind` is `content` or `fields`. Append-only updates (notes, decisions, a bare comment), built-in updates and collab-snapshot flushes are not counted. Such a write with `content` from any door but `web` answers 200 with an additive, `omitempty` `warnings.unguarded_content_write: true`. Nothing is refused in this phase; phase 2 answers 428 for the agent doors. `replace: true` is the deliberate overwrite. It changes no outcome now, a token makes it moot, and it is separate from `overwrite_pending_edits`. A seq `update_conflict` on a content write (the applier route included) gains additive `details.content_write: true`, `changed_at` and `changed_by_kind` (`user`/`agent`, the row's last writer), and a message naming both seqs that says not to resend the body. A field-only conflict and a timestamp conflict are unchanged - **A content write routed to an open tab is REFUSED when that tab holds typing the server has not stored** (BUG-3542): 409 `content_not_applied` with `details.apply_reason: "unconfirmed_edits"` and `content_landed: false`, token or not (Dave's ruling). The check above reads the op-log when the PATCH arrives, so typing still on the wire is invisible to it; the designated applier tab knows (its BUG-3523 barrier has not confirmed every send) and answers `applier_refuse` instead of applying, and the server does not re-elect another tab. Other fields in the request land, as on every `content_not_applied`. Wait a few seconds, re-read and resend; `overwrite_pending_edits: true` sends the request unguarded, and is the only way to replace that typing. A tab that predates the refusal ignores the request's `guarded` flag and applies, as before diff --git a/internal/cli/client.go b/internal/cli/client.go index d74e2470d..4021eb349 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -447,7 +447,15 @@ func (c *Client) GetItem(wsSlug, itemSlug string) (*models.Item, error) { func (c *Client) UpdateItem(wsSlug, itemSlug string, input models.ItemUpdate) (*models.Item, error) { var result models.Item - if err := c.patch("/workspaces/"+wsSlug+"/items/"+itemSlug, input, &result); err != nil { + path := "/workspaces/" + wsSlug + "/items/" + itemSlug + err := c.patch(path, input, &result) + // TASK-3585: a keyed update (an append) is retried once with the same + // key, behind the same capability gate as create and comment. + if err != nil && input.OpKey != "" && retryableWithKey(err) && c.serverHonorsKeys() { + result = models.Item{} + err = c.patch(path, input, &result) + } + if err != nil { return nil, wrapItemNotFound(err, itemSlug, wsSlug) } return &result, nil diff --git a/internal/mcp/catalog_item.go b/internal/mcp/catalog_item.go index b2ab8a7b0..8ee27dab4 100644 --- a/internal/mcp/catalog_item.go +++ b/internal/mcp/catalog_item.go @@ -316,7 +316,7 @@ var padItemSchemaParams = []ParamDef{ }, "required": []any{"old", "new"}, }, Description: "patch: the edits, applied in order, all or nothing. Required for: patch."}, - {Name: "op_key", Type: "string", Description: "patch, create, comment: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused)."}, + {Name: "op_key", Type: "string", Description: "patch, create, comment, note, decide: an idempotency key you choose (e.g. a UUID). Retrying the SAME request with the same key returns the first answer instead of applying it again; use it whenever a retry could follow an unclear outcome. A key reused for a different request is refused (idempotency_key_reused)."}, {Name: "undo", Type: "string", Description: "patch: invert an earlier patch on this item by its patch_id (instead of edits). Refused, writing nothing (patch_anchor_missing), when the text it changed has changed since; never reverts someone else's later edit. The undo is itself a patch."}, {Name: "lines", Type: "string", Description: "get: return only these body lines, numbered (\"A-B\", \"A-\" or \"A\"; 1-based), with seq and content_state, for targeting a patch. Line numbers are for reading; a patch anchors on text."}, // PLAN-3555: the deliberate-overwrite opt-out. Phase 1 changes no outcome; diff --git a/internal/mcp/dispatch_http_project.go b/internal/mcp/dispatch_http_project.go index c0bff20d3..e26bc9579 100644 --- a/internal/mcp/dispatch_http_project.go +++ b/internal/mcp/dispatch_http_project.go @@ -496,6 +496,8 @@ func (d *HTTPHandlerDispatcher) dispatchItemNote( Summary: strings.TrimSpace(summary), Details: details, }, + // TASK-3585: the append's idempotency key, when the agent sent one. + OpKey: opKeyFrom(input), }) if err != nil { return dispatcherErrorResult(cmdKey, "encode body", err), nil @@ -539,6 +541,7 @@ func (d *HTTPHandlerDispatcher) dispatchItemDecide( Decision: strings.TrimSpace(decision), Rationale: rationale, }, + OpKey: opKeyFrom(input), }) if err != nil { return dispatcherErrorResult(cmdKey, "encode body", err), nil @@ -683,3 +686,9 @@ func (d *HTTPHandlerDispatcher) fetchLibraryEndpoint( } return decoded, nil } + +// opKeyFrom is the agent's op_key, if any (TASK-3585). +func opKeyFrom(input map[string]any) string { + k, _ := input["op_key"].(string) + return k +} diff --git a/internal/mcp/instructions.md b/internal/mcp/instructions.md index 4184e392b..a6ecae244 100644 --- a/internal/mcp/instructions.md +++ b/internal/mcp/instructions.md @@ -6,7 +6,7 @@ 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.91) +## Tool surface (v0.92) Ten resource × action tools, plus `pad_set_workspace`. Eleven tools total. diff --git a/internal/mcp/task3585_op_key_test.go b/internal/mcp/task3585_op_key_test.go index 953dd8b26..915236e78 100644 --- a/internal/mcp/task3585_op_key_test.go +++ b/internal/mcp/task3585_op_key_test.go @@ -147,3 +147,27 @@ func TestTASK3585_MCPCommentOpKey(t *testing.T) { t.Fatalf("%d comments; the retried comment made a twin", n) } } + +// TASK-3585: op_key on note and decide rides to the server, so a retried +// append answers the entry it made instead of appending twice. +func TestTASK3585_MCPAppendOpKey(t *testing.T) { + f := newParentFixture(t) + ws, ref := f.workspace.Slug, f.child.Slug + for _, c := range []struct { + action string + in map[string]any + }{ + {"note", map[string]any{"workspace": ws, "ref": ref, "summary": "Keyed note", "op_key": "mcp-n"}}, + {"decide", map[string]any{"workspace": ws, "ref": ref, "decision": "Keyed decision", "op_key": "mcp-d"}}, + } { + first := f.dispatchResult2(t, []string{"item", c.action}, c.in) + second := f.dispatchResult2(t, []string{"item", c.action}, c.in) + if first.IsError || second.IsError { + t.Fatalf("%s: %s / %s", c.action, textOf(first), textOf(second)) + } + } + got, _ := f.store.GetItem(f.child.ID) + if n, d := len(models.ExtractItemImplementationNotes(got.Fields)), len(models.ExtractItemDecisionLog(got.Fields)); n != 1 || d != 1 { + t.Fatalf("notes=%d decisions=%d; a retried append made a twin", n, d) + } +} diff --git a/internal/mcp/version.go b/internal/mcp/version.go index f2942f791..008e75c89 100644 --- a/internal/mcp/version.go +++ b/internal/mcp/version.go @@ -1738,6 +1738,17 @@ const CmdhelpVersion = "0.1" // caller cannot read embeds nothing; a failed fetch stays non-fatal, // as on stdio. Stdio is unchanged. // +// 0.92 — TASK-3585, appends. ADDITIVE bump on the v0.53 grounds: +// `pad_item.action=comment`, `note` and `decide` accept `op_key`. +// Retrying the SAME request with the same key answers the comment or +// entry the first attempt made instead of appending again; the answer is +// recorded in the write's own transaction. On the item-update route a +// key is accepted only with an append (and source); any other member +// alongside it is refused 400. `idempotency_target_gone` (410: the item +// or comment the key wrote has since been deleted) is now a structured +// code on both transports, with its own hint; at 0.91 it reached MCP as +// a generic error. No name, enum or param moved. +// // 0.91 — TASK-3585, create. ADDITIVE bump on the v0.53 grounds: // `pad_item.action=create` accepts `op_key` too. Retrying the SAME // create with the same key answers the item the first attempt created @@ -2107,7 +2118,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.91" +const ToolSurfaceVersion = "0.92" // 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/item.go b/internal/models/item.go index c6bcd72ff..28eb6ea3f 100644 --- a/internal/models/item.go +++ b/internal/models/item.go @@ -1589,6 +1589,10 @@ type ItemUpdate struct { // ServerSupportsItemFieldAppend. AppendImplementationNote *ItemImplementationNoteAppend `json:"append_implementation_note,omitempty"` AppendDecision *ItemDecisionLogAppend `json:"append_decision,omitempty"` + // OpKey is an idempotency key for an APPEND-ONLY update (TASK-3585): + // append_implementation_note and/or append_decision, plus source. Any + // other member alongside it is refused 400. Not part of the request hash. + OpKey string `json:"op_key,omitempty"` // ImplementationNoteToAppend / DecisionToAppend are the MINTED entries // the handler builds from the two request members above, for the store diff --git a/internal/server/handlers_items.go b/internal/server/handlers_items.go index 0060aae47..5febe19f4 100644 --- a/internal/server/handlers_items.go +++ b/internal/server/handlers_items.go @@ -1550,6 +1550,34 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt // write's last_modified_by below, or an in-process caller's label (see // WithStructuredEntryAuthor) — so a caller supplies only the text and // cannot write an entry the extractor would not decode. + // TASK-3585: an idempotency key on this route covers appends only. A key + // on any other update would promise a replay this handler cannot give. + // The built-in and patch callers (b != nil) carry their own key. + var appendHold *idemHold + if b == nil { + key, kerr := idempotencyKey(r, input.OpKey) + if kerr != nil { + writeError(w, http.StatusBadRequest, "bad_request", kerr.Error()) + return + } + if key != "" { + if !isAppendOnlyUpdate(input) { + writeError(w, http.StatusBadRequest, "bad_request", + "an idempotency key on an item update is accepted only with append_implementation_note and/or append_decision (and source)") + return + } + canon := input + canon.OpKey = "" + hold, ok := s.claimKey(w, r, workspaceID, idempotencyRouteAppend, item.ID, canon, key, func(status int, stored string) { + s.replayAppend(w, r, updateVisIDs, status, stored) + }) + if !ok { + return + } + appendHold = hold + defer s.release(hold) + } + } if input.AppendImplementationNote != nil || input.AppendDecision != nil { if input.Fields != nil { writeError(w, http.StatusBadRequest, "bad_request", @@ -1577,6 +1605,13 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt } } } + if appendHold != nil { + answer := appendedAnswer{Note: input.ImplementationNoteToAppend, Decision: input.DecisionToAppend} + input.AfterWriteTx = func(tx *sql.Tx, updated *models.Item) error { + answer.ItemID = updated.ID + return s.completeTx(appendHold, http.StatusOK, answer)(tx) + } + } // BUG-2627 part 2: `fields_patch` is the door every USER field-setter // lowers into — `pad item update --field` (cmd_item.go), the MCP `field` diff --git a/internal/server/idempotency_keys.go b/internal/server/idempotency_keys.go index 9083a1f9a..8e0e1ca5a 100644 --- a/internal/server/idempotency_keys.go +++ b/internal/server/idempotency_keys.go @@ -5,8 +5,10 @@ import ( "database/sql" "encoding/hex" "encoding/json" + "fmt" "net/http" + "github.com/PerpetualSoftware/pad/internal/models" "github.com/PerpetualSoftware/pad/internal/store" ) @@ -93,3 +95,52 @@ const idempotencyRouteComment = "comment.create" type commentedAnswer struct { CommentID string `json:"comment_id"` } + +// idempotencyRouteAppend names the append-only item update in a key's +// request hash. +const idempotencyRouteAppend = "item.append" + +// appendedAnswer is what an append's key stores: the item and the entries +// the write minted. A replay answers with the item as it stands now, echoing +// those entries. +type appendedAnswer struct { + ItemID string `json:"item_id"` + Note *models.ItemImplementationNote `json:"note,omitempty"` + Decision *models.ItemDecisionLogEntry `json:"decision,omitempty"` +} + +// isAppendOnlyUpdate reports whether in carries nothing but appends (and +// source, which every CLI write stamps). It compares against the zero +// update so a member added to ItemUpdate later is covered without a list. +func isAppendOnlyUpdate(in models.ItemUpdate) bool { + if in.AppendImplementationNote == nil && in.AppendDecision == nil { + return false + } + in.AppendImplementationNote, in.AppendDecision, in.OpKey, in.Source = nil, nil, "", "" + got, err1 := json.Marshal(in) + zero, err2 := json.Marshal(models.ItemUpdate{}) + return err1 == nil && err2 == nil && string(got) == string(zero) +} + +// replayAppend answers a replayed append: the item as it stands now, with +// the entries the first request appended. 410 when the item is gone. +// visIDs is the caller's visible collections, as the update's own response +// uses, so a restricted caller's replay names no item it cannot see. +func (s *Server) replayAppend(w http.ResponseWriter, r *http.Request, visIDs []string, status int, stored string) { + var a appendedAnswer + if json.Unmarshal([]byte(stored), &a) != nil { + writeInternalError(w, fmt.Errorf("stored idempotent answer does not decode")) + return + } + it, err := s.store.GetItem(a.ItemID) + if err != nil || it == nil { + writeError2(w, http.StatusGone, "idempotency_target_gone", "the item this key appended to no longer exists", map[string]interface{}{"item_id": a.ItemID}) + return + } + if err := s.enrichItemForResponse(r, it, visIDs); err != nil { + writeInternalError(w, err) + return + } + it.Appended = &models.ItemAppendedEntries{ImplementationNote: a.Note, Decision: a.Decision} + writeJSON(w, status, it) +} diff --git a/internal/server/task3585_append_key_test.go b/internal/server/task3585_append_key_test.go new file mode 100644 index 000000000..1facf737e --- /dev/null +++ b/internal/server/task3585_append_key_test.go @@ -0,0 +1,90 @@ +package server + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// TASK-3585: idempotency keys on the note/decision appends. +func appendKeyFixture(t *testing.T) (*Server, string) { + t.Helper() + srv := testServer(t) + slug := createWSWithCollections(t, srv) + rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/tasks/items", map[string]any{"title": "Noted", "fields": `{"status":"open"}`}) + if rr.Code != http.StatusCreated { + t.Fatalf("item: %d %s", rr.Code, rr.Body.String()) + } + var it models.Item + _ = json.Unmarshal(rr.Body.Bytes(), &it) + return srv, "/api/v1/workspaces/" + slug + "/items/" + it.Slug +} + +func notesOn(t *testing.T, srv *Server, path string) []models.ItemImplementationNote { + t.Helper() + rr := doRequest(srv, "GET", path, nil) + var it models.Item + _ = json.Unmarshal(rr.Body.Bytes(), &it) + return models.ExtractItemImplementationNotes(it.Fields) +} + +func TestTASK3585_AppendReplaysTheEntryItMade(t *testing.T) { + srv, path := appendKeyFixture(t) + body := map[string]any{"append_implementation_note": map[string]any{"summary": "Keyed note"}, "source": "cli", "op_key": "n-1"} + first := doRequest(srv, "PATCH", path, body) + again := doRequest(srv, "PATCH", path, body) + var a, b models.Item + _ = json.Unmarshal(first.Body.Bytes(), &a) + _ = json.Unmarshal(again.Body.Bytes(), &b) + if first.Code != http.StatusOK || again.Code != http.StatusOK || again.Header().Get("Idempotent-Replayed") != "true" { + t.Fatalf("replay: %d/%d %s", first.Code, again.Code, again.Body.String()) + } + if a.Appended == nil || a.Appended.ImplementationNote == nil || b.Appended == nil || b.Appended.ImplementationNote == nil || + a.Appended.ImplementationNote.ID != b.Appended.ImplementationNote.ID { + t.Fatalf("the replay must echo the first entry: %+v vs %+v", a.Appended, b.Appended) + } + if n := len(notesOn(t, srv, path)); n != 1 { + t.Fatalf("%d notes; the retry appended a twin", n) + } + other := map[string]any{"append_implementation_note": map[string]any{"summary": "Other"}, "op_key": "n-1"} + if rr := doRequest(srv, "PATCH", path, other); rr.Code != http.StatusUnprocessableEntity { + t.Fatalf("a reused key: %d", rr.Code) + } +} + +func TestTASK3585_KeyOnANonAppendUpdateIsRefused(t *testing.T) { + srv, path := appendKeyFixture(t) + for _, body := range []map[string]any{ + {"title": "Renamed", "op_key": "u-1"}, + {"append_decision": map[string]any{"decision": "D"}, "fields_patch": map[string]any{"status": "done"}, "op_key": "u-2"}, + } { + rr := doRequest(srv, "PATCH", path, body) + if rr.Code != http.StatusBadRequest { + t.Fatalf("%v: %d %s", body, rr.Code, rr.Body.String()) + } + } + rr := doRequest(srv, "GET", path, nil) + var it models.Item + _ = json.Unmarshal(rr.Body.Bytes(), &it) + if it.Title != "Noted" { + t.Fatalf("a refused keyed update wrote: title %q", it.Title) + } +} + +func TestTASK3585_AppendWithoutAKeyIsUnchanged(t *testing.T) { + srv, path := appendKeyFixture(t) + body := map[string]any{"append_decision": map[string]any{"decision": "Unkeyed"}} + for i := 0; i < 2; i++ { + if rr := doRequest(srv, "PATCH", path, body); rr.Code != http.StatusOK || rr.Header().Get("Idempotent-Replayed") != "" { + t.Fatalf("unkeyed append %d: %d", i, rr.Code) + } + } + rr := doRequest(srv, "GET", path, nil) + var it models.Item + _ = json.Unmarshal(rr.Body.Bytes(), &it) + if n := len(models.ExtractItemDecisionLog(it.Fields)); n != 2 { + t.Fatalf("two unkeyed appends must append twice: %d", n) + } +}