mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 14:30:13 +00:00
feat(items): content-anchored patch writes on an item with no live editor (PLAN-3578 step 1) (#2055)
POST /workspaces/{ws}/items/{ref}/patch {edits:[{old,new,line_hint?}],
expected_seq?} replaces exact text in the body. It runs through updateItem,
so it takes every PATCH guard; the body is computed inside the store
transaction from the row it replaces. Each old must occur exactly once;
edits apply in order, all or nothing; every patch writes a version. The
answer carries a unified diff of what changed.
expected_seq does not refuse: a moved seq is applied over when the
anchors still match, and answered rebased with the intervening writes,
read from a seq chain stamped into item activity entries ({prev_seq, seq,
version_id}). complete is decided at runtime, so any write that left no
stamp reads incomplete; a guard test classifies every seq-moving store
statement. Refusals write nothing: patch_anchor_missing,
patch_anchor_ambiguous, patch_room_open (an open editor; step 2).
CLI: pad item patch, pad item show --lines. MCP 0.87 (additive):
pad_item.patch, get lines, three allow-listed codes. The timeline treats
the chain and patch keys as bookkeeping, not a change.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
@@ -402,11 +402,11 @@ directory for `claude-code`, and an `[mcp_servers.pad]` table in
|
||||
project-scoped, it's install-on-request only — `--all` and `pad mcp status` cover
|
||||
the per-user clients (including Codex) and skip it.
|
||||
|
||||
**Tool catalog (v0.86)** — 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.87)** — 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 |
|
||||
|---|---|
|
||||
| `pad_item` | `create`, `update`, `delete`, `get`, `list`, `move`, `restore`, `link`, `unlink`, `deps`, `star`, `unstar`, `starred`, `comment`, `list-comments`, `edit-comment`, `delete-comment`, `backlinks`, `bulk-update`, `note`, `decide`, `export`, `import`, `history`, `remind`, `ack-reminder`, `claim`, `release` |
|
||||
| `pad_item` | `create`, `update`, `delete`, `get`, `list`, `move`, `restore`, `link`, `unlink`, `deps`, `star`, `unstar`, `starred`, `comment`, `list-comments`, `edit-comment`, `delete-comment`, `backlinks`, `bulk-update`, `note`, `decide`, `export`, `import`, `history`, `remind`, `ack-reminder`, `claim`, `release`, `patch` |
|
||||
| `pad_workspace` | `list`, `members`, `invite`, `storage`, `audit-log`, `create`, `claim`, `deleted`, `restore` |
|
||||
| `pad_collection` | `list`, `create`, `update`, `delete`, `list-archived`, `restore` |
|
||||
| `pad_project` | `dashboard`, `next`, `ready`, `stale`, `standup`, `changelog`, `report`, `activity` |
|
||||
@@ -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.86"`) and advertised in the initialize handshake and at
|
||||
(`tool_surface_version: "0.87"`) 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
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/PerpetualSoftware/pad/internal/bodypatch"
|
||||
"io"
|
||||
"math"
|
||||
"net/url"
|
||||
@@ -738,6 +739,7 @@ func printItemsGroupedByCollection(items []models.Item) {
|
||||
|
||||
func showCmd() *cobra.Command {
|
||||
var agentOutput bool
|
||||
var linesFlag string
|
||||
cmd := &cobra.Command{
|
||||
Use: "show <ref>",
|
||||
Aliases: []string{"read"},
|
||||
@@ -756,6 +758,18 @@ on stderr instead. --format json carries it as the content_state field.`,
|
||||
client, _ := getClient()
|
||||
ws := getWorkspace()
|
||||
|
||||
// PLAN-3578: numbered lines, for targeting a patch. Parsed before the
|
||||
// read so a bad range costs no request.
|
||||
var lineStart, lineEnd int
|
||||
if linesFlag != "" {
|
||||
if agentOutput {
|
||||
return fmt.Errorf("--lines conflicts with --agent; use one")
|
||||
}
|
||||
var perr error
|
||||
if lineStart, lineEnd, perr = bodypatch.ParseLineRange(linesFlag); perr != nil {
|
||||
return perr
|
||||
}
|
||||
}
|
||||
item, err := client.GetItem(ws, args[0])
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -763,6 +777,22 @@ on stderr instead. --format json carries it as the content_state field.`,
|
||||
if agentOutput {
|
||||
return cli.PrintJSONCompact(cli.ToItemAgentView(*item))
|
||||
}
|
||||
if linesFlag != "" {
|
||||
lines, from, to, total := bodypatch.SliceBodyLines(item.Content, lineStart, lineEnd)
|
||||
out := bodypatch.BodyLines{Ref: cli.ItemRef(*item), Seq: item.Seq, ContentState: item.ContentState,
|
||||
Start: from, End: to, TotalLines: total, Lines: lines}
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(out)
|
||||
}
|
||||
fmt.Printf("%s · seq %d · lines %d-%d of %d\n", out.Ref, out.Seq, from, to, total)
|
||||
if item.ContentState != "" {
|
||||
fmt.Printf("content_state: %s (the stored body is behind the live document)\n", item.ContentState)
|
||||
}
|
||||
for _, l := range lines {
|
||||
fmt.Printf("%5d %s\n", l.N, l.Text)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// PLAN-1593 / TASK-1596: fetch the top 5 backlinks
|
||||
// alongside the item so the show command always carries
|
||||
@@ -1005,6 +1035,7 @@ on stderr instead. --format json carries it as the content_state field.`,
|
||||
},
|
||||
}
|
||||
cmd.Flags().BoolVar(&agentOutput, "agent", false, "return compact agent-oriented JSON")
|
||||
cmd.Flags().StringVar(&linesFlag, "lines", "", "show only these body lines, numbered (A-B, A- or A; 1-based), with the seq: for targeting pad item patch. Line numbers are for reading; a patch anchors on text")
|
||||
return cmd
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/bodypatch"
|
||||
"github.com/PerpetualSoftware/pad/internal/cli"
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
// itemPatchCmd is `pad item patch` (PLAN-3578): content-anchored edits to an
|
||||
// item's body, answered with the diff of exactly what changed.
|
||||
func itemPatchCmd() *cobra.Command {
|
||||
var (
|
||||
olds, news []string
|
||||
editsFlag string
|
||||
expectedSeq int64
|
||||
)
|
||||
cmd := &cobra.Command{
|
||||
Use: "patch <ref> (--old TEXT --new TEXT)... | --edits @FILE",
|
||||
Short: "Replace exact text in an item's body, and print the diff",
|
||||
Long: `Edit part of an item's body without sending the whole of it. Each edit
|
||||
names text (--old) that must occur EXACTLY ONCE in the current body and the
|
||||
text that replaces it (--new). Edits apply in order, all or nothing. The
|
||||
answer is a unified diff of what changed.
|
||||
|
||||
Anchors are text, not line numbers: line N when you read is not line N when
|
||||
the write lands. Quote enough surrounding text to be unique. Anchors
|
||||
guarantee position, not meaning: if someone rewrote the paragraph around your
|
||||
anchor, the edit still lands; read the diff.
|
||||
|
||||
--expected-seq does not refuse a moved item. If every anchor still matches,
|
||||
the patch applies and reports rebased, with the writes in between and the
|
||||
diff; if an anchor no longer matches, nothing is written.
|
||||
|
||||
For several or multi-line edits use --edits with a JSON array of
|
||||
{"old","new","line_hint"} (line_hint, 1-based, only chooses among several
|
||||
exact matches): --edits @edits.json, or --edits - to read stdin.
|
||||
|
||||
An item open in an editor is refused for now (patch_room_open).`,
|
||||
Example: ` pad item patch TASK-5 --old "Status: draft" --new "Status: ready"
|
||||
pad item patch DOC-3 --old "old line" --new "new line" --expected-seq 41
|
||||
pad item patch DOC-3 --edits @edits.json`,
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
edits, err := patchEditsFromFlags(olds, news, editsFlag, os.Stdin)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
client, _ := getClient()
|
||||
if !client.ServerSupportsItemPatch() {
|
||||
return fmt.Errorf("this Pad server does not support patch writes; upgrade it, or use pad item update with --expected-seq")
|
||||
}
|
||||
var seq *int64
|
||||
if cmd.Flags().Changed("expected-seq") {
|
||||
seq = &expectedSeq
|
||||
}
|
||||
res, err := client.PatchItem(getWorkspace(), args[0], edits, seq)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(res)
|
||||
}
|
||||
fmt.Printf("Patched %s (%d edit(s), seq %d)\n", res.Ref, len(edits), res.Seq)
|
||||
if res.Rebased && res.Intervening != nil {
|
||||
iv := res.Intervening
|
||||
fmt.Printf("Rebased: applied over %d write(s) since seq %d", len(iv.Changes), iv.FromSeq)
|
||||
if !iv.Complete {
|
||||
fmt.Print(" (the list is incomplete: some writes in between left no record)")
|
||||
}
|
||||
fmt.Println(". Read the diff.")
|
||||
}
|
||||
fmt.Print(res.Diff)
|
||||
return nil
|
||||
},
|
||||
}
|
||||
cmd.Flags().StringArrayVar(&olds, "old", nil, "text to replace; must occur exactly once in the current body (repeatable, paired in order with --new)")
|
||||
cmd.Flags().StringArrayVar(&news, "new", nil, "replacement text (repeatable, paired in order with --old; may be empty to delete)")
|
||||
cmd.Flags().StringVar(&editsFlag, "edits", "", `edits as a JSON array of {"old","new","line_hint"}: @FILE, - for stdin, or the JSON itself`)
|
||||
cmd.Flags().Int64Var(&expectedSeq, "expected-seq", 0, "the seq you read; a moved item is still patched when every anchor matches, and reported as rebased")
|
||||
return cmd
|
||||
}
|
||||
|
||||
// patchEditsFromFlags builds the edit list from --old/--new pairs or --edits,
|
||||
// refusing a mix, an unpaired flag, or an empty anchor before any request.
|
||||
func patchEditsFromFlags(olds, news []string, editsFlag string, stdin io.Reader) ([]bodypatch.Edit, error) {
|
||||
if editsFlag != "" {
|
||||
if len(olds) > 0 || len(news) > 0 {
|
||||
return nil, fmt.Errorf("--edits conflicts with --old/--new; use one")
|
||||
}
|
||||
raw := []byte(editsFlag)
|
||||
switch {
|
||||
case editsFlag == "-":
|
||||
b, err := io.ReadAll(stdin)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read --edits from stdin: %w", err)
|
||||
}
|
||||
raw = b
|
||||
case strings.HasPrefix(editsFlag, "@"):
|
||||
b, err := os.ReadFile(editsFlag[1:])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read --edits file: %w", err)
|
||||
}
|
||||
raw = b
|
||||
}
|
||||
var edits []bodypatch.Edit
|
||||
if err := json.Unmarshal(raw, &edits); err != nil {
|
||||
return nil, fmt.Errorf(`--edits must be a JSON array of {"old","new","line_hint"}: %w`, err)
|
||||
}
|
||||
return checkEdits(edits)
|
||||
}
|
||||
if len(olds) == 0 {
|
||||
return nil, fmt.Errorf("give at least one --old/--new pair, or --edits")
|
||||
}
|
||||
if len(olds) != len(news) {
|
||||
return nil, fmt.Errorf("%d --old but %d --new: each --old needs its --new", len(olds), len(news))
|
||||
}
|
||||
edits := make([]bodypatch.Edit, len(olds))
|
||||
for i := range olds {
|
||||
edits[i] = bodypatch.Edit{Old: olds[i], New: news[i]}
|
||||
}
|
||||
return checkEdits(edits)
|
||||
}
|
||||
|
||||
func checkEdits(edits []bodypatch.Edit) ([]bodypatch.Edit, error) {
|
||||
if len(edits) == 0 {
|
||||
return nil, fmt.Errorf("no edits")
|
||||
}
|
||||
for i, e := range edits {
|
||||
if e.Old == "" {
|
||||
return nil, fmt.Errorf("edit %d: old is empty; every edit must quote the text it replaces", i)
|
||||
}
|
||||
}
|
||||
return edits, nil
|
||||
}
|
||||
@@ -116,6 +116,7 @@ func itemCmd() *cobra.Command {
|
||||
showCmd(),
|
||||
itemOpenCmd(),
|
||||
updateCmd(),
|
||||
itemPatchCmd(),
|
||||
historyCmd(),
|
||||
deleteCmd(),
|
||||
restoreCmd(),
|
||||
|
||||
@@ -90,6 +90,11 @@ func writeRootStructuredError(w io.Writer, err error) {
|
||||
// BUG-3496: a claim on an item another holder leases.
|
||||
if apiErr, ok := cli.IsLeaseHeld(err); ok {
|
||||
cli.WriteLeaseHeldError(w, apiErr)
|
||||
return
|
||||
}
|
||||
// PLAN-3578: a patch whose anchor no longer matches, or an open item.
|
||||
if apiErr, ok := cli.IsPatchRefusal(err); ok {
|
||||
cli.WritePatchRefusalError(w, apiErr)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// PLAN-3578: the CLI builds a patch's edits from --old/--new pairs or --edits
|
||||
// and refuses a bad combination before any request.
|
||||
func TestPLAN3578_PatchEditsFromFlags(t *testing.T) {
|
||||
got, err := patchEditsFromFlags([]string{"a", "b"}, []string{"A", ""}, "", nil)
|
||||
if err != nil || len(got) != 2 || got[1].Old != "b" || got[1].New != "" {
|
||||
t.Fatalf("pairs: %+v %v", got, err)
|
||||
}
|
||||
got, err = patchEditsFromFlags(nil, nil, `[{"old":"x","new":"y","line_hint":3}]`, nil)
|
||||
if err != nil || len(got) != 1 || got[0].LineHint != 3 {
|
||||
t.Fatalf("json: %+v %v", got, err)
|
||||
}
|
||||
got, err = patchEditsFromFlags(nil, nil, "-", strings.NewReader(`[{"old":"x","new":"y"}]`))
|
||||
if err != nil || len(got) != 1 {
|
||||
t.Fatalf("stdin: %+v %v", got, err)
|
||||
}
|
||||
for name, c := range map[string]struct {
|
||||
olds, news []string
|
||||
edits string
|
||||
want string
|
||||
}{
|
||||
"unpaired": {[]string{"a"}, nil, "", "each --old needs its --new"},
|
||||
"none": {nil, nil, "", "at least one"},
|
||||
"mixed": {[]string{"a"}, []string{"b"}, `[]`, "conflicts"},
|
||||
"empty anchor": {[]string{""}, []string{"b"}, "", "old is empty"},
|
||||
"bad json": {nil, nil, `{`, "JSON array"},
|
||||
"empty json old": {nil, nil, `[{"old":"","new":"z"}]`, "old is empty"},
|
||||
} {
|
||||
if _, err := patchEditsFromFlags(c.olds, c.news, c.edits, nil); err == nil || !strings.Contains(err.Error(), c.want) {
|
||||
t.Errorf("%s: err = %v, want %q", name, err, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+31
-1
File diff suppressed because one or more lines are too long
+2
-2
File diff suppressed because one or more lines are too long
+31
-1
File diff suppressed because one or more lines are too long
@@ -28,6 +28,13 @@ REST API at `/api/v1/`. Key endpoints:
|
||||
- **A convention's reserved `convention` copy follows its ordinary metadata keys** (TASK-2257). A convention stores its metadata twice: the ordinary fields `trigger`, `scope` / `surfaces`, `enforcement` / `priority`, `category` and `commands` (what agents obey; bootstrap and `--field` filters read them) and the reserved `convention` key (what `item.convention` prefers). When an update CHANGES one of those ordinary keys on an item that HAS a reserved copy, the server rebuilds the copy from the merged fields in the same transaction, after the carry check above: `surfaces` from `surfaces`, else `[scope]`; `enforcement` from `enforcement`, else `priority`; untouched keys keep their stored value. An item without a reserved copy is left as it was. The one new refusal is deliberate: a write whose merged values would leave the copy empty answers 400 `validation_error` and writes nothing. Before this, `pad item update --field trigger=X` left `item.convention` on the old trigger
|
||||
- **`refuse_undeclared_fields: true` is a strict opt-in on item update** (BUG-3156): a write that would store a field key the collection does not declare is REFUSED with 400 `validation_error`, and nothing is written, instead of being accepted and named in `warnings.undeclared_fields`. It refuses EXACTLY the keys that warning would have named: on a `fields_patch` write the patched keys only; on a full `fields` write the whole blob, stray stored keys included. It is additive and omitted by default, so a write without it is unchanged. **Skew:** a server that predates it ignores the member, which is the accept-and-warn, so a strict caller confirms by the ABSENCE of the warning rather than trusting a 200. `pad item bulk-update` and the remote bulk-update dispatcher set it; `pad item update` / `pad_item.update` do not
|
||||
- **A content write carrying a version token is REFUSED while a tab holds unflushed edits** (BUG-3133): 409 `content_pending_flush`, lifted by `overwrite_pending_edits: true`. The token guards the row, and those edits live only in the op-log until a tab flushes, so re-reading does not clear it. A tokenless content write still replaces them (edits the server has STORED; typing it has not received yet is the next bullet, and refuses tokenless writes too); with no tab open that deletes them from the op-log, and the response names how many in `warnings.pruned_pending_edits` (additive, omitempty; the CLI prints it to stderr). Rows a schema rebuild SET ASIDE (BUG-3244) refuse EVERY content write, token or not, with the same code plus `details.set_aside_rows`, because no tab will ever store them; `overwrite_pending_edits` deletes them in the write's own transaction and counts them into `warnings.pruned_pending_edits`. A collab-snapshot flush is exempt and leaves them alone. A writer that cannot carry a token asks for the same refusal with `refuse_pending_edits: true` (BUG-3230 U0): no row version is compared, and `overwrite_pending_edits` still lifts it. The web item pane's raw-markdown saves send it, because their debounced saves overlap and their unload flush is a teardown write (BUG-3080). Web-only: not on the CLI or the MCP catalog, so no tool-surface bump, and a server that predates it ignores it and replaces
|
||||
- **`POST /workspaces/{ws}/items/{ref}/patch {edits:[{old, new, line_hint?}], expected_seq?}`** (PLAN-3578 step 1) replaces exact text in the body without sending all of it.
|
||||
- **Rules:** each `old` must occur EXACTLY ONCE in the body as it stands inside the write's own transaction (overlapping occurrences count), and `line_hint` only chooses among several exact matches. Edits apply in order, all or nothing, at most 100 per patch.
|
||||
- **Answer:** `{ref, seq, patch_id, version_id, rebased, intervening?, diff}`, where `diff` is a unified diff of the body before → after. Every patch writes a version (past the throttle), and its activity entry carries `patch_id` and `patch_edits`.
|
||||
- **`expected_seq` does NOT refuse;** the anchors are the precondition. When the row's seq moved, the patch still applies if every anchor matches and answers `rebased: true` with `intervening: {from_seq, to_seq, changes:[{activity_id, seq, prev_seq, action, actor, source, at, version_id?}], complete, truncated?}` (at most 50 changes).
|
||||
- **`complete`** is decided by walking the activity seq chain. Every item `created`/`updated` activity entry carries `metadata.{prev_seq, seq, version_id?}`. A write that left no stamped entry (a path that writes none, a failed activity write, anything before the stamps existed) breaks the chain, and then `complete` is false. A debounced merge keeps the chain only across contiguous runs, and marks the entry `seq_chain_broken` otherwise. `internal/store/plan3578_seq_site_guard_test.go` classifies every seq-moving store statement.
|
||||
- **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 a live editor holds the item (the open-room path is a later step, and a whole-document replace is never the fallback). Also every PATCH guard: unflushed editor edits refuse it as `content_pending_flush` token or not, set-aside rows, permissions.
|
||||
- **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
|
||||
- **Write responses may also carry `warnings.dropped_fields`** (TASK-2878, widened by BUG-3079) — schema-declared keys the write DISCARDED. The general case is an injected schema DEFAULT that fails the same `validateFieldType` a caller-supplied value takes: it used to be assigned and skipped past that check, so the SAME BYTES were refused when supplied and stored when injected, decided only by who put them there — a `status` retyped to `multi_select` whose scalar default survived answered 400 to `{"status":"open"}` and 201 to `{}`, storing `{"status":"open"}`. It is now dropped and named here. The relation case keeps its own later pass, because "is this a string" and "does this string name a live, visible item" are different questions and only the first is answerable in the DB-free validator. Dropped rather than refused because nobody in the request typed it — EXCEPT on a `required` field, where dropping leaves it absent and the write is refused as required, with a message naming the default as the cause. Additive and `omitempty`
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
@@ -0,0 +1,120 @@
|
||||
// Package bodypatch applies content-anchored edits to an item body
|
||||
// (PLAN-3578): each edit names text that must occur EXACTLY ONCE in the body
|
||||
// at the moment it applies, and replaces it. Anchors are text, never line
|
||||
// numbers: the body is markdown serialized from a collaborative document, so
|
||||
// line N at read time need not be line N at write time. A line hint may only
|
||||
// choose among several exact matches; it never stands in for one.
|
||||
package bodypatch
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Edit is one replacement: Old must occur exactly once (or exactly once on
|
||||
// LineHint's line when it occurs more than once), and becomes New.
|
||||
type Edit struct {
|
||||
Old string `json:"old"`
|
||||
New string `json:"new"`
|
||||
// LineHint, when > 0, is the 1-based line on which an ambiguous Old
|
||||
// starts. Ignored when Old occurs once.
|
||||
LineHint int `json:"line_hint,omitempty"`
|
||||
}
|
||||
|
||||
// MaxEdits bounds one patch.
|
||||
const MaxEdits = 100
|
||||
|
||||
// EmptyAnchorError refuses an edit whose Old is empty: an empty anchor
|
||||
// matches everywhere, so it says nothing about where the edit goes.
|
||||
type EmptyAnchorError struct{ Index int }
|
||||
|
||||
func (e *EmptyAnchorError) Error() string {
|
||||
return fmt.Sprintf("edit %d: old is empty; an anchor must name text in the body", e.Index)
|
||||
}
|
||||
|
||||
// AnchorMissingError: Old does not occur in the body as it stands when the
|
||||
// edit applies (after the edits before it).
|
||||
type AnchorMissingError struct{ Index int }
|
||||
|
||||
func (e *AnchorMissingError) Error() string {
|
||||
return fmt.Sprintf("edit %d: old text not found in the current body", e.Index)
|
||||
}
|
||||
|
||||
// AnchorAmbiguousError: Old occurs more than once and no line hint picks
|
||||
// exactly one. Lines are the 1-based start lines of every match.
|
||||
type AnchorAmbiguousError struct {
|
||||
Index int
|
||||
Lines []int
|
||||
}
|
||||
|
||||
func (e *AnchorAmbiguousError) Error() string {
|
||||
return fmt.Sprintf("edit %d: old text occurs %d times (lines %v); quote more context, or pass line_hint", e.Index, len(e.Lines), e.Lines)
|
||||
}
|
||||
|
||||
// Apply runs edits in order against body, each against the result of the
|
||||
// ones before it, and returns the new body. All or nothing: the first edit
|
||||
// that cannot apply returns its error and no body.
|
||||
func Apply(body string, edits []Edit) (string, error) {
|
||||
if len(edits) == 0 {
|
||||
return "", fmt.Errorf("no edits")
|
||||
}
|
||||
if len(edits) > MaxEdits {
|
||||
return "", fmt.Errorf("%d edits; at most %d per patch", len(edits), MaxEdits)
|
||||
}
|
||||
for i, e := range edits {
|
||||
if e.Old == "" {
|
||||
return "", &EmptyAnchorError{Index: i}
|
||||
}
|
||||
}
|
||||
for i, e := range edits {
|
||||
at := matches(body, e.Old)
|
||||
switch {
|
||||
case len(at) == 0:
|
||||
return "", &AnchorMissingError{Index: i}
|
||||
case len(at) > 1:
|
||||
pick := -1
|
||||
if e.LineHint > 0 {
|
||||
for _, p := range at {
|
||||
if lineOf(body, p) == e.LineHint {
|
||||
if pick >= 0 {
|
||||
pick = -2 // two matches start on the hinted line
|
||||
break
|
||||
}
|
||||
pick = p
|
||||
}
|
||||
}
|
||||
}
|
||||
if pick < 0 {
|
||||
lines := make([]int, len(at))
|
||||
for k, p := range at {
|
||||
lines[k] = lineOf(body, p)
|
||||
}
|
||||
return "", &AnchorAmbiguousError{Index: i, Lines: lines}
|
||||
}
|
||||
at = []int{pick}
|
||||
}
|
||||
body = body[:at[0]] + e.New + body[at[0]+len(e.Old):]
|
||||
}
|
||||
return body, nil
|
||||
}
|
||||
|
||||
// matches returns every start offset of sub in s, overlapping ones included:
|
||||
// "aa" occurs twice in "aaa", and a uniqueness check that counted it once
|
||||
// would let an edit land on whichever one a scan happened to find first.
|
||||
func matches(s, sub string) []int {
|
||||
var out []int
|
||||
for from := 0; from <= len(s)-len(sub); {
|
||||
k := strings.Index(s[from:], sub)
|
||||
if k < 0 {
|
||||
break
|
||||
}
|
||||
out = append(out, from+k)
|
||||
from += k + 1
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// lineOf is the 1-based line on which byte offset p lies.
|
||||
func lineOf(s string, p int) int {
|
||||
return strings.Count(s[:p], "\n") + 1
|
||||
}
|
||||
@@ -0,0 +1,57 @@
|
||||
package bodypatch
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestApply(t *testing.T) {
|
||||
body := "alpha\nbeta\ngamma beta\n"
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
edits []Edit
|
||||
want string
|
||||
err any
|
||||
}{
|
||||
{"single unique", []Edit{{Old: "alpha", New: "ALPHA"}}, "ALPHA\nbeta\ngamma beta\n", nil},
|
||||
{"edits apply in order to the evolving body", []Edit{{Old: "alpha", New: "x"}, {Old: "x\nbeta", New: "y"}}, "y\ngamma beta\n", nil},
|
||||
{"missing", []Edit{{Old: "delta", New: "d"}}, "", &AnchorMissingError{}},
|
||||
{"ambiguous without hint", []Edit{{Old: "beta", New: "b"}}, "", &AnchorAmbiguousError{}},
|
||||
{"hint picks a match", []Edit{{Old: "beta", New: "B", LineHint: 3}}, "alpha\nbeta\ngamma B\n", nil},
|
||||
{"hint on no match is still ambiguous", []Edit{{Old: "beta", New: "B", LineHint: 1}}, "", &AnchorAmbiguousError{}},
|
||||
{"hint ignored when unique", []Edit{{Old: "gamma", New: "G", LineHint: 1}}, "alpha\nbeta\nG beta\n", nil},
|
||||
{"empty old refused", []Edit{{Old: "alpha", New: "a"}, {Old: "", New: "z"}}, "", &EmptyAnchorError{}},
|
||||
{"overlapping occurrences are ambiguous", []Edit{{Old: "aa", New: "b"}}, "", &AnchorAmbiguousError{}},
|
||||
{"a later failure applies nothing", []Edit{{Old: "alpha", New: "a"}, {Old: "nope", New: "z"}}, "", &AnchorMissingError{}},
|
||||
} {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
in := body
|
||||
if c.name == "overlapping occurrences are ambiguous" {
|
||||
in = "aaa"
|
||||
}
|
||||
got, err := Apply(in, c.edits)
|
||||
if c.err != nil {
|
||||
target := reflect.New(reflect.TypeOf(c.err).Elem()).Interface()
|
||||
if err == nil || !errors.As(err, &target) {
|
||||
t.Fatalf("err = %v, want %T", err, c.err)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("a refused patch returned a body: %q", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil || got != c.want {
|
||||
t.Fatalf("got %q, %v; want %q", got, err, c.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAmbiguousNamesEveryLine(t *testing.T) {
|
||||
_, err := Apply("x\ny x\nx", []Edit{{Old: "x", New: "z"}})
|
||||
var amb *AnchorAmbiguousError
|
||||
if !errors.As(err, &amb) || !reflect.DeepEqual(amb.Lines, []int{1, 2, 3}) {
|
||||
t.Fatalf("err = %v", err)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,68 @@
|
||||
package bodypatch
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// BodyLines is a numbered slice of an item's body (PLAN-3578), for targeting
|
||||
// and quoting a patch. Line numbers are 1-based over the body as stored,
|
||||
// split on "\n". They are for reading only: a patch anchors on text.
|
||||
type BodyLines struct {
|
||||
Ref string `json:"ref"`
|
||||
Seq int64 `json:"seq"`
|
||||
ContentState string `json:"content_state,omitempty"`
|
||||
Start int `json:"start"`
|
||||
End int `json:"end"`
|
||||
TotalLines int `json:"total_lines"`
|
||||
Lines []BodyLine `json:"lines"`
|
||||
}
|
||||
|
||||
// BodyLine is one numbered line.
|
||||
type BodyLine struct {
|
||||
N int `json:"n"`
|
||||
Text string `json:"text"`
|
||||
}
|
||||
|
||||
// ParseLineRange reads "A-B", "A-" (to the end) or "A". A range past the
|
||||
// body's end is clipped by SliceBodyLines, not refused here.
|
||||
func ParseLineRange(s string) (start, end int, err error) {
|
||||
bad := fmt.Errorf("lines wants A-B, A- or A (1-based), got %q", s)
|
||||
a, b, isRange := strings.Cut(strings.TrimSpace(s), "-")
|
||||
start, err = strconv.Atoi(a)
|
||||
if err != nil || start < 1 {
|
||||
return 0, 0, bad
|
||||
}
|
||||
switch {
|
||||
case !isRange:
|
||||
return start, start, nil
|
||||
case b == "":
|
||||
return start, 0, nil // 0: to the end
|
||||
}
|
||||
end, err = strconv.Atoi(b)
|
||||
if err != nil || end < start {
|
||||
return 0, 0, bad
|
||||
}
|
||||
return start, end, nil
|
||||
}
|
||||
|
||||
// SliceBodyLines numbers content's lines start..end (end 0 = to the end),
|
||||
// clipped to the body. A body that ends in "\n" has no empty last line.
|
||||
func SliceBodyLines(content string, start, end int) (lines []BodyLine, from, to, total int) {
|
||||
all := strings.Split(strings.TrimSuffix(content, "\n"), "\n")
|
||||
if content == "" {
|
||||
all = nil
|
||||
}
|
||||
total = len(all)
|
||||
if end == 0 || end > total {
|
||||
end = total
|
||||
}
|
||||
for n := start; n <= end; n++ {
|
||||
lines = append(lines, BodyLine{N: n, Text: all[n-1]})
|
||||
}
|
||||
if len(lines) == 0 {
|
||||
lines = []BodyLine{}
|
||||
}
|
||||
return lines, start, end, total
|
||||
}
|
||||
@@ -0,0 +1,32 @@
|
||||
package bodypatch
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestParseLineRange(t *testing.T) {
|
||||
for in, want := range map[string][2]int{"3": {3, 3}, "2-5": {2, 5}, "4-": {4, 0}, " 1-1 ": {1, 1}} {
|
||||
s, e, err := ParseLineRange(in)
|
||||
if err != nil || s != want[0] || e != want[1] {
|
||||
t.Errorf("%q: %d %d %v", in, s, e, err)
|
||||
}
|
||||
}
|
||||
for _, bad := range []string{"", "0", "-3", "5-2", "a-b", "2-x"} {
|
||||
if _, _, err := ParseLineRange(bad); err == nil {
|
||||
t.Errorf("%q accepted", bad)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSliceBodyLines(t *testing.T) {
|
||||
lines, from, to, total := SliceBodyLines("a\nb\nc\n", 2, 0)
|
||||
if total != 3 || from != 2 || to != 3 || len(lines) != 2 || lines[1].N != 3 || lines[1].Text != "c" {
|
||||
t.Fatalf("got %+v %d %d %d", lines, from, to, total)
|
||||
}
|
||||
lines, _, to, total = SliceBodyLines("a\nb", 1, 9)
|
||||
if total != 2 || to != 2 || len(lines) != 2 {
|
||||
t.Fatalf("clip: %+v %d %d", lines, to, total)
|
||||
}
|
||||
lines, _, _, total = SliceBodyLines("", 1, 0)
|
||||
if total != 0 || len(lines) != 0 {
|
||||
t.Fatalf("empty body: %+v %d", lines, total)
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/PerpetualSoftware/pad/internal/bodypatch"
|
||||
"io"
|
||||
"mime"
|
||||
"mime/multipart"
|
||||
@@ -56,6 +57,7 @@ type serverCapabilityFlags struct {
|
||||
AttachmentAttach bool `json:"attachment_attach"`
|
||||
LibraryActivate bool `json:"library_activate"`
|
||||
BuiltinUpdate bool `json:"builtin_update"`
|
||||
ItemPatch bool `json:"item_patch"`
|
||||
}
|
||||
|
||||
func NewClient(host string, port int) *Client {
|
||||
@@ -894,6 +896,87 @@ func (c *Client) BuiltinUpdateSupport() (supported, definitive bool) {
|
||||
return caps.BuiltinUpdate, definitive
|
||||
}
|
||||
|
||||
// ServerSupportsItemPatch reports whether this server serves POST
|
||||
// /workspaces/{ws}/items/{ref}/patch (PLAN-3578). An older build answers that
|
||||
// route with a bare 404 that reads exactly like a missing item, so the CLI
|
||||
// refuses instead of sending when this is absent.
|
||||
func (c *Client) ServerSupportsItemPatch() bool {
|
||||
caps, _ := c.serverCapabilities()
|
||||
return caps.ItemPatch
|
||||
}
|
||||
|
||||
// PatchResult is the answer to a patch write (PLAN-3578).
|
||||
type PatchResult struct {
|
||||
Ref string `json:"ref"`
|
||||
Seq int64 `json:"seq"`
|
||||
PatchID string `json:"patch_id"`
|
||||
VersionID string `json:"version_id,omitempty"`
|
||||
Rebased bool `json:"rebased"`
|
||||
Intervening *models.PatchIntervening `json:"intervening,omitempty"`
|
||||
Diff string `json:"diff"`
|
||||
}
|
||||
|
||||
// PatchItem applies content-anchored edits to the item's body.
|
||||
func (c *Client) PatchItem(wsSlug, itemSlug string, edits []bodypatch.Edit, expectedSeq *int64) (*PatchResult, error) {
|
||||
body := map[string]any{"edits": edits}
|
||||
if expectedSeq != nil {
|
||||
body["expected_seq"] = *expectedSeq
|
||||
}
|
||||
var result PatchResult
|
||||
if err := c.post("/workspaces/"+wsSlug+"/items/"+itemSlug+"/patch", body, &result); err != nil {
|
||||
return nil, wrapItemNotFound(err, itemSlug, wsSlug)
|
||||
}
|
||||
return &result, nil
|
||||
}
|
||||
|
||||
// Patch refusal codes (PLAN-3578). Keep in lockstep with internal/mcp's
|
||||
// allowedStructuredErrorCodes and its Patch*Hint constants.
|
||||
const (
|
||||
PatchAnchorMissingCode = "patch_anchor_missing"
|
||||
PatchAnchorAmbiguousCode = "patch_anchor_ambiguous"
|
||||
PatchRoomOpenCode = "patch_room_open"
|
||||
)
|
||||
|
||||
// PatchAnchorHint is the recovery guidance for a patch whose anchor no
|
||||
// longer matches exactly once. Duplicated in internal/mcp; a test asserts
|
||||
// the two match.
|
||||
const PatchAnchorHint = "Nothing was written. Re-read the item and quote, as old, text that occurs exactly once in the CURRENT body " +
|
||||
"(add surrounding lines to make it unique, or pass line_hint to choose among exact matches)."
|
||||
|
||||
// PatchRoomOpenHint is the recovery guidance for patch_room_open. Duplicated
|
||||
// in internal/mcp; a test asserts the two match.
|
||||
const PatchRoomOpenHint = "Nothing was written. The item is open in an editor, and patching an open item is not supported yet. " +
|
||||
"Retry after the editor closes, or send a whole-body update with expected_seq."
|
||||
|
||||
// IsPatchRefusal reports a patch refusal the server answered.
|
||||
func IsPatchRefusal(err error) (*APIError, bool) {
|
||||
var apiErr *APIError
|
||||
if errors.As(err, &apiErr) {
|
||||
switch apiErr.Code {
|
||||
case PatchAnchorMissingCode, PatchAnchorAmbiguousCode, PatchRoomOpenCode:
|
||||
return apiErr, true
|
||||
}
|
||||
}
|
||||
return nil, false
|
||||
}
|
||||
|
||||
// WritePatchRefusalError writes the structured marker line for a patch
|
||||
// refusal, so the stdio MCP transport reports its code and details instead
|
||||
// of inferring one from the prose.
|
||||
func WritePatchRefusalError(w io.Writer, apiErr *APIError) {
|
||||
hint := PatchAnchorHint
|
||||
if apiErr.Code == PatchRoomOpenCode {
|
||||
hint = PatchRoomOpenHint
|
||||
}
|
||||
body := map[string]any{"code": apiErr.Code, "message": apiErr.Message, "hint": hint}
|
||||
if len(apiErr.Details) > 0 {
|
||||
body["details"] = apiErr.Details
|
||||
}
|
||||
if data, err := json.Marshal(map[string]any{"error": body}); err == nil {
|
||||
fmt.Fprintln(w, StructuredErrorMarker+string(data))
|
||||
}
|
||||
}
|
||||
|
||||
// BuiltinText is a built-in's text: the body and the fields an update writes.
|
||||
type BuiltinText struct {
|
||||
Content string `json:"content"`
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
package diff
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
difflib "github.com/sergi/go-diff/diffmatchpatch"
|
||||
)
|
||||
|
||||
// Unified renders a line-based unified diff of oldText → newText with the
|
||||
// given lines of context (PLAN-3578: what a patch write hands back so the
|
||||
// caller can verify exactly what changed). Empty when the texts are equal.
|
||||
// Headers are fixed ("--- before" / "+++ after"); hunk ranges follow the
|
||||
// unified format (1-based start, count; a zero-count range starts at the
|
||||
// line before it).
|
||||
func Unified(oldText, newText string, context int) string {
|
||||
if oldText == newText {
|
||||
return ""
|
||||
}
|
||||
a, b, lines := dmp.DiffLinesToChars(oldText, newText)
|
||||
diffs := dmp.DiffCharsToLines(dmp.DiffMain(a, b, false), lines)
|
||||
|
||||
type line struct {
|
||||
op difflib.Operation
|
||||
text string
|
||||
}
|
||||
var all []line
|
||||
for _, d := range diffs {
|
||||
for _, l := range splitKeepEnd(d.Text) {
|
||||
all = append(all, line{d.Type, l})
|
||||
}
|
||||
}
|
||||
|
||||
var out strings.Builder
|
||||
out.WriteString("--- before\n+++ after\n")
|
||||
oldNo, newNo := 1, 1
|
||||
for i := 0; i < len(all); {
|
||||
if all[i].op == difflib.DiffEqual {
|
||||
oldNo++
|
||||
newNo++
|
||||
i++
|
||||
continue
|
||||
}
|
||||
// A change starts at i: back up over up to `context` equal lines.
|
||||
start := i
|
||||
for k := 0; k < context && start > 0 && all[start-1].op == difflib.DiffEqual; k++ {
|
||||
start--
|
||||
}
|
||||
hOld, hNew := oldNo-(i-start), newNo-(i-start)
|
||||
// Extend through changes separated by at most 2*context equal lines.
|
||||
end := i
|
||||
for end < len(all) {
|
||||
if all[end].op != difflib.DiffEqual {
|
||||
end++
|
||||
continue
|
||||
}
|
||||
run := end
|
||||
for run < len(all) && all[run].op == difflib.DiffEqual {
|
||||
run++
|
||||
}
|
||||
if run < len(all) && run-end <= 2*context {
|
||||
end = run
|
||||
continue
|
||||
}
|
||||
end = min(end+context, run)
|
||||
break
|
||||
}
|
||||
var body strings.Builder
|
||||
oc, nc := 0, 0
|
||||
for _, l := range all[start:end] {
|
||||
switch l.op {
|
||||
case difflib.DiffEqual:
|
||||
body.WriteString(" " + l.text)
|
||||
oc++
|
||||
nc++
|
||||
case difflib.DiffDelete:
|
||||
body.WriteString("-" + l.text)
|
||||
oc++
|
||||
case difflib.DiffInsert:
|
||||
body.WriteString("+" + l.text)
|
||||
nc++
|
||||
}
|
||||
if !strings.HasSuffix(l.text, "\n") {
|
||||
body.WriteString("\n\\ No newline at end of file\n")
|
||||
}
|
||||
}
|
||||
fmt.Fprintf(&out, "@@ -%s +%s @@\n", hunkRange(hOld, oc), hunkRange(hNew, nc))
|
||||
out.WriteString(body.String())
|
||||
// Advance the counters over the lines of this hunk that lie past i.
|
||||
for _, l := range all[i:end] {
|
||||
if l.op != difflib.DiffInsert {
|
||||
oldNo++
|
||||
}
|
||||
if l.op != difflib.DiffDelete {
|
||||
newNo++
|
||||
}
|
||||
}
|
||||
i = end
|
||||
}
|
||||
return out.String()
|
||||
}
|
||||
|
||||
func hunkRange(start, count int) string {
|
||||
if count == 0 {
|
||||
return fmt.Sprintf("%d,0", start-1)
|
||||
}
|
||||
if count == 1 {
|
||||
return fmt.Sprintf("%d", start)
|
||||
}
|
||||
return fmt.Sprintf("%d,%d", start, count)
|
||||
}
|
||||
|
||||
// splitKeepEnd splits s into lines, each keeping its "\n"; a last line
|
||||
// without one is kept as is.
|
||||
func splitKeepEnd(s string) []string {
|
||||
var out []string
|
||||
for s != "" {
|
||||
k := strings.IndexByte(s, '\n')
|
||||
if k < 0 {
|
||||
out = append(out, s)
|
||||
break
|
||||
}
|
||||
out = append(out, s[:k+1])
|
||||
s = s[k+1:]
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
package diff
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestUnified(t *testing.T) {
|
||||
for _, c := range []struct{ name, a, b, want string }{
|
||||
{"equal", "x\n", "x\n", ""},
|
||||
{"one line changed in the middle",
|
||||
"1\n2\n3\n4\n5\n6\n7\n8\n9\n", "1\n2\n3\n4\nFIVE\n6\n7\n8\n9\n",
|
||||
"--- before\n+++ after\n@@ -2,7 +2,7 @@\n 2\n 3\n 4\n-5\n+FIVE\n 6\n 7\n 8\n"},
|
||||
{"insert at start",
|
||||
"a\nb\n", "z\na\nb\n",
|
||||
"--- before\n+++ after\n@@ -1,2 +1,3 @@\n+z\n a\n b\n"},
|
||||
{"two far changes make two hunks",
|
||||
"1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n", "ONE\n2\n3\n4\n5\n6\n7\n8\n9\nTEN\n",
|
||||
"--- before\n+++ after\n@@ -1,4 +1,4 @@\n-1\n+ONE\n 2\n 3\n 4\n@@ -7,4 +7,4 @@\n 7\n 8\n 9\n-10\n+TEN\n"},
|
||||
{"no trailing newline",
|
||||
"a\nb", "a\nc",
|
||||
"--- before\n+++ after\n@@ -1,2 +1,2 @@\n a\n-b\n\\ No newline at end of file\n+c\n\\ No newline at end of file\n"},
|
||||
} {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
if got := Unified(c.a, c.b, 3); got != c.want {
|
||||
t.Fatalf("got:\n%s\nwant:\n%s", got, c.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -88,9 +88,12 @@ type ToolSchema struct {
|
||||
// WithArray, WithObject). Enum, when non-empty, constrains string parameters.
|
||||
type ParamDef struct {
|
||||
Name string
|
||||
Type string // "string" | "number" | "bool" | "array<string>" | "object"
|
||||
Type string // "string" | "number" | "bool" | "array<string>" | "array<object>" | "object"
|
||||
Description string
|
||||
Enum []string
|
||||
// ItemSchema is the JSON schema of one element of an "array<object>"
|
||||
// param (PLAN-3578's patch edits), so the host sees its shape.
|
||||
ItemSchema map[string]any
|
||||
}
|
||||
|
||||
// ActionFn handles one tool action. Receives the call context, the raw
|
||||
@@ -450,6 +453,11 @@ func paramDefToToolOption(p ParamDef) mcp.ToolOption {
|
||||
return mcp.WithBoolean(p.Name, propOpts...)
|
||||
case "array<string>":
|
||||
return mcp.WithArray(p.Name, append(propOpts, mcp.WithStringItems())...)
|
||||
case "array<object>":
|
||||
if p.ItemSchema != nil {
|
||||
propOpts = append(propOpts, mcp.Items(p.ItemSchema))
|
||||
}
|
||||
return mcp.WithArray(p.Name, propOpts...)
|
||||
case "object":
|
||||
// Structured JSON parameter. The MCP host sees a generic object;
|
||||
// per-tool semantics are encoded in the Description. Callers
|
||||
|
||||
@@ -2,7 +2,9 @@ package mcp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"github.com/PerpetualSoftware/pad/internal/bodypatch"
|
||||
"os"
|
||||
"sort"
|
||||
"strings"
|
||||
@@ -55,7 +57,10 @@ var padItemTool = ToolDef{
|
||||
"create": actionItemCreate,
|
||||
"update": actionItemUpdate,
|
||||
"delete": passThrough([]string{"item", "delete"}),
|
||||
"get": passThrough([]string{"item", "show"}),
|
||||
"get": actionItemGet,
|
||||
|
||||
// PLAN-3578: content-anchored body edits, answered with the diff.
|
||||
"patch": passThrough([]string{"item", "patch"}),
|
||||
// list is CUSTOM (not passThrough) so a bare agent list stays
|
||||
// bounded: it injects a default limit and clamps an oversized one
|
||||
// before dispatch, mirroring the backlinks default/max. See
|
||||
@@ -301,6 +306,17 @@ var padItemSchemaParams = []ParamDef{
|
||||
// to catch. `seq` is bumped on every mutation of the row, is returned on
|
||||
// every item AND every item summary, and is an integer.
|
||||
{Name: "overwrite_pending_edits", Type: "bool", Description: "update: true replaces a tab's unsaved edits instead of refusing token-guarded content (content_pending_flush)."},
|
||||
// ── Patch (PLAN-3578) ──
|
||||
{Name: "edits", Type: "array<object>", ItemSchema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"old": map[string]any{"type": "string", "description": "Exact text that must occur EXACTLY ONCE in the current body. Never empty."},
|
||||
"new": map[string]any{"type": "string", "description": "Replacement text; empty deletes."},
|
||||
"line_hint": map[string]any{"type": "number", "description": "1-based line on which `old` starts; only chooses among several exact matches."},
|
||||
},
|
||||
"required": []any{"old", "new"},
|
||||
}, Description: "patch: the edits, applied in order, all or nothing. Required for: 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;
|
||||
// phase 2 refuses a body write that carries neither this nor a token.
|
||||
{Name: "replace", Type: "bool", Description: "update: true overwrites the content on purpose, whatever changed since you read it (no version token). A content write with neither this nor expected_seq is flagged warnings.unguarded_content_write now and will be refused later. Conflicts with expected_seq."},
|
||||
@@ -362,7 +378,20 @@ Actions:
|
||||
restore — Un-archive (restore) a soft-deleted item by ref.
|
||||
Required: ref.
|
||||
get — Read an item.
|
||||
Required: ref.
|
||||
Required: ref. Optional: lines ("40-80": numbered body lines plus
|
||||
seq, for quoting a patch's anchors).
|
||||
patch — Replace exact text in an item's body without sending all of it.
|
||||
Required: ref, edits ([{old, new, line_hint?}]). Optional: expected_seq.
|
||||
Each old must occur EXACTLY ONCE in the current body (quote more
|
||||
context, or pass line_hint to pick among exact matches); edits apply
|
||||
in order, all or nothing. The answer is a unified diff of what changed:
|
||||
read it. Anchors guarantee position, not meaning: if the text around
|
||||
your anchor was rewritten, the edit still lands. A moved expected_seq
|
||||
does not refuse: the patch applies and returns rebased=true with the
|
||||
intervening writes (complete=false when some left no record). Refusals
|
||||
write nothing: patch_anchor_missing / patch_anchor_ambiguous (re-read,
|
||||
quote current text) and patch_room_open (the item is open in an
|
||||
editor; not supported yet).
|
||||
list — List items, optionally filtered.
|
||||
Optional: collection, status, priority, parent, unparented, role, assign, all, limit, full.
|
||||
parent and unparented are mutually exclusive. Results are
|
||||
@@ -941,3 +970,42 @@ func actionItemImport(ctx context.Context, input map[string]any, env ActionEnv)
|
||||
out["file"] = tmpPath
|
||||
return env.Dispatch(ctx, []string{"item", "import"}, out)
|
||||
}
|
||||
|
||||
// actionItemGet handles pad_item.action=get. With `lines` it slices the body
|
||||
// into numbered lines HERE, the one place both transports pass through, so
|
||||
// the two answer identically (PLAN-3578).
|
||||
func actionItemGet(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
|
||||
spec, has := input["lines"]
|
||||
if !has {
|
||||
return env.Dispatch(ctx, []string{"item", "show"}, input)
|
||||
}
|
||||
s, _ := spec.(string)
|
||||
start, end, err := bodypatch.ParseLineRange(s)
|
||||
if err != nil {
|
||||
return errStructured("pad_item.get", err), nil
|
||||
}
|
||||
out := make(map[string]any, len(input))
|
||||
for k, v := range input {
|
||||
if k != "lines" {
|
||||
out[k] = v
|
||||
}
|
||||
}
|
||||
res, err := env.Dispatch(ctx, []string{"item", "show"}, out)
|
||||
if err != nil || res == nil || res.IsError {
|
||||
return res, err
|
||||
}
|
||||
m, ok := res.StructuredContent.(map[string]any)
|
||||
if !ok {
|
||||
return errStructured("pad_item.get", fmt.Errorf("lines: the item read returned no structured body")), nil
|
||||
}
|
||||
content, _ := m["content"].(string)
|
||||
lines, from, to, total := bodypatch.SliceBodyLines(content, start, end)
|
||||
view := bodypatch.BodyLines{Start: from, End: to, TotalLines: total, Lines: lines}
|
||||
view.Ref, _ = m["ref"].(string)
|
||||
if n, ok := numericInput(m["seq"]); ok {
|
||||
view.Seq = int64(n)
|
||||
}
|
||||
view.ContentState, _ = m["content_state"].(string)
|
||||
text, _ := json.Marshal(view)
|
||||
return structuredResult(view, string(text)), nil
|
||||
}
|
||||
|
||||
@@ -1697,7 +1697,7 @@ func TestFieldConflictProperty_SourcesDerivedFromTheDeclaredSchema(t *testing.T)
|
||||
// expected_seq joins expected_updated_at here for the same reason
|
||||
// (BUG-3037): it is a concurrency TOKEN, not a field write, so the
|
||||
// field-conflict machinery has nothing to say about it.
|
||||
"expected_updated_at": true, "expected_seq": true, "overwrite_pending_edits": true, "replace": true, "force": true, "allow_draft": true,
|
||||
"expected_updated_at": true, "expected_seq": true, "overwrite_pending_edits": true, "replace": true, "edits": true, "lines": true, "force": true, "allow_draft": true,
|
||||
"clear_parent": true, "clear_assigned_user": true, "clear_agent_role": true,
|
||||
"artifact": true, "raw_args": true,
|
||||
|
||||
|
||||
@@ -130,6 +130,7 @@ func TestReadOnlyCatalog_ActionsMatchCmdhelp(t *testing.T) {
|
||||
{"pad_item", "ack-reminder"}: {"item", "ack"},
|
||||
{"pad_item", "claim"}: {"item", "claim"},
|
||||
{"pad_item", "release"}: {"item", "release"},
|
||||
{"pad_item", "patch"}: {"item", "patch"},
|
||||
{"pad_item", "star"}: {"item", "star"},
|
||||
{"pad_item", "unstar"}: {"item", "unstar"},
|
||||
{"pad_item", "starred"}: {"item", "starred"},
|
||||
@@ -295,6 +296,7 @@ func TestReadOnlyCatalog_ActionsDispatchExpectedCmdPath(t *testing.T) {
|
||||
{"pad_item", "ack-reminder"}: {"item", "ack"},
|
||||
{"pad_item", "claim"}: {"item", "claim"},
|
||||
{"pad_item", "release"}: {"item", "release"},
|
||||
{"pad_item", "patch"}: {"item", "patch"},
|
||||
{"pad_item", "star"}: {"item", "star"},
|
||||
{"pad_item", "unstar"}: {"item", "unstar"},
|
||||
{"pad_item", "starred"}: {"item", "starred"},
|
||||
@@ -700,6 +702,11 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
|
||||
Args: mkArgs("ref"),
|
||||
Flags: mkFlags("workspace", "holder"),
|
||||
},
|
||||
"item patch": {
|
||||
Summary: "replace exact text in an item's body",
|
||||
Args: mkArgs("ref"),
|
||||
Flags: mkFlags("workspace", "edits", "expected-seq"),
|
||||
},
|
||||
"item star": {
|
||||
Summary: "star item",
|
||||
Args: mkArgs("ref"),
|
||||
|
||||
@@ -317,6 +317,7 @@ var ChatGPTExclusions = map[ChatGPTSource]string{
|
||||
{"pad_item", "unlink"}: "deferred to v2",
|
||||
{"pad_item", "claim"}: "agent lease semantics, not a chat concept",
|
||||
{"pad_item", "release"}: "agent lease semantics, not a chat concept",
|
||||
{"pad_item", "patch"}: "update_item already versions every content write so a chat edit can be undone; anchored patching is an agent workflow (PLAN-3578)",
|
||||
{"pad_item", "remind"}: "deferred to v2",
|
||||
{"pad_item", "ack-reminder"}: "deferred to v2",
|
||||
{"pad_item", "star"}: "personal UI state",
|
||||
|
||||
@@ -412,6 +412,12 @@ func init() {
|
||||
// duration (the CLI flag's vocabulary) while the handler's body
|
||||
// takes ttl_seconds. `item release` fits the declarative shape.
|
||||
"item claim": mapItemClaim,
|
||||
// PLAN-3578: content-anchored patch writes.
|
||||
"item patch": routeSpec{
|
||||
method: http.MethodPost,
|
||||
pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}/patch",
|
||||
bodyKeys: []string{"edits", "expected_seq"},
|
||||
}.toRouteMapper(),
|
||||
"item release": routeSpec{
|
||||
method: http.MethodPost,
|
||||
pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}/release",
|
||||
|
||||
@@ -210,6 +210,13 @@ const (
|
||||
// refuses identically.
|
||||
ErrLeaseHeld ErrorCode = "lease_held"
|
||||
|
||||
// Patch refusals (PLAN-3578): nothing was written. Anchor codes mean the
|
||||
// quoted text no longer occurs exactly once; room_open means the item is
|
||||
// open in an editor, which a patch does not support yet.
|
||||
ErrPatchAnchorMissing ErrorCode = "patch_anchor_missing"
|
||||
ErrPatchAnchorAmbiguous ErrorCode = "patch_anchor_ambiguous"
|
||||
ErrPatchRoomOpen ErrorCode = "patch_room_open"
|
||||
|
||||
// ErrStoredStateUnreadable fires when an operation is refused because
|
||||
// the ITEM'S STORED STATE cannot be decoded — today, an append to an
|
||||
// implementation_notes / decision_log field whose value is not a list
|
||||
@@ -485,6 +492,10 @@ var allowedStructuredErrorCodes = map[string]struct{}{
|
||||
// BUG-3496: a claim on an item someone else leases. details.holder and
|
||||
// details.expires_at say who and until when.
|
||||
"lease_held": {},
|
||||
// PLAN-3578: patch refusals.
|
||||
"patch_anchor_missing": {},
|
||||
"patch_anchor_ambiguous": {},
|
||||
"patch_room_open": {},
|
||||
// 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
|
||||
@@ -1029,6 +1040,12 @@ func classifyHTTPStatusKind(
|
||||
// The lease stands until it expires or its holder releases it.
|
||||
hint = LeaseHeldHint
|
||||
}
|
||||
if upstream.Code == string(ErrPatchAnchorMissing) || upstream.Code == string(ErrPatchAnchorAmbiguous) {
|
||||
hint = PatchAnchorHint
|
||||
}
|
||||
if upstream.Code == string(ErrPatchRoomOpen) {
|
||||
hint = PatchRoomOpenHint
|
||||
}
|
||||
if upstream.Code == string(ErrStoredStateUnreadable) {
|
||||
// Same reason: the stored value stays undecodable, so every
|
||||
// retry refuses identically. Since BUG-3056 the note/decide
|
||||
@@ -1391,6 +1408,16 @@ const LeaseHeldHint = "Another holder (details.holder) has claimed this item unt
|
||||
const ContentPendingFlushHint = "Re-reading will not clear this: the stored item is unchanged until the open editor saves its edits. " +
|
||||
"Wait for that and re-read, or resend with overwrite_pending_edits=true to replace them."
|
||||
|
||||
// PatchAnchorHint is the recovery guidance for a patch anchor refusal
|
||||
// (PLAN-3578). Duplicated in internal/cli; a test asserts the two match.
|
||||
const PatchAnchorHint = "Nothing was written. Re-read the item and quote, as old, text that occurs exactly once in the CURRENT body " +
|
||||
"(add surrounding lines to make it unique, or pass line_hint to choose among exact matches)."
|
||||
|
||||
// PatchRoomOpenHint is the recovery guidance for patch_room_open (PLAN-3578).
|
||||
// Duplicated in internal/cli; a test asserts the two match.
|
||||
const PatchRoomOpenHint = "Nothing was written. The item is open in an editor, and patching an open item is not supported yet. " +
|
||||
"Retry after the editor closes, or send a whole-body update with expected_seq."
|
||||
|
||||
// UpdateConflictContentHint replaces the generic re-read-and-retry hint on an
|
||||
// update_conflict whose details carry content_write (PLAN-3555): a resent body
|
||||
// would undo the change the caller missed. Duplicated in internal/cli, whose
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -0,0 +1,116 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/cli"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// PLAN-3578 step 1 at the MCP doors.
|
||||
|
||||
func TestPLAN3578_MCPPatchLinesAndRefusals(t *testing.T) {
|
||||
f := newParentFixture(t)
|
||||
ws, ref := f.workspace.Slug, f.child.Slug
|
||||
f.dispatch(t, []string{"item", "update"}, map[string]any{"workspace": ws, "ref": ref, "content": "alpha\nbeta\ngamma beta\n", "replace": true})
|
||||
|
||||
// get with lines: numbered lines and the seq, on the catalog path both
|
||||
// transports share.
|
||||
res, err := actionItemGet(t.Context(), map[string]any{"workspace": ws, "ref": ref, "lines": "2-3"}, f.env(t))
|
||||
if err != nil || res.IsError {
|
||||
t.Fatalf("get lines: %v %s", err, textOf(res))
|
||||
}
|
||||
var view struct {
|
||||
Seq int64 `json:"seq"`
|
||||
Lines []struct {
|
||||
N int `json:"n"`
|
||||
Text string `json:"text"`
|
||||
} `json:"lines"`
|
||||
TotalLines int `json:"total_lines"`
|
||||
}
|
||||
b, _ := json.Marshal(res.StructuredContent)
|
||||
_ = json.Unmarshal(b, &view)
|
||||
if len(view.Lines) != 2 || view.Lines[0].N != 2 || view.Lines[0].Text != "beta" || view.TotalLines != 3 || view.Seq == 0 {
|
||||
t.Fatalf("lines view = %+v", view)
|
||||
}
|
||||
|
||||
// A patch lands and answers with the diff.
|
||||
res = f.dispatchResult2(t, []string{"item", "patch"}, map[string]any{"workspace": ws, "ref": ref,
|
||||
"edits": []any{map[string]any{"old": "alpha", "new": "ALPHA"}}})
|
||||
if res.IsError {
|
||||
t.Fatalf("patch: %s", textOf(res))
|
||||
}
|
||||
m, _ := res.StructuredContent.(map[string]any)
|
||||
if d, _ := m["diff"].(string); !strings.Contains(d, "+ALPHA") {
|
||||
t.Fatalf("patch answer = %v", m)
|
||||
}
|
||||
|
||||
// An ambiguous anchor: its own code, the lines, the shared hint.
|
||||
res = f.dispatchResult2(t, []string{"item", "patch"}, map[string]any{"workspace": ws, "ref": ref,
|
||||
"edits": []any{map[string]any{"old": "beta", "new": "B"}}})
|
||||
env, ok := res.StructuredContent.(ErrorEnvelope)
|
||||
if !res.IsError || !ok || env.Error.Code != ErrPatchAnchorAmbiguous || env.Error.Hint != PatchAnchorHint {
|
||||
t.Fatalf("ambiguous: %s", textOf(res))
|
||||
}
|
||||
var d map[string]any
|
||||
_ = json.Unmarshal(env.Error.Details, &d)
|
||||
if lines, _ := d["lines"].([]any); len(lines) != 2 {
|
||||
t.Fatalf("details = %v", d)
|
||||
}
|
||||
}
|
||||
|
||||
// Stdio: the CLI's marker carries the code, details and hint.
|
||||
func TestPLAN3578_StdioPatchRefusal(t *testing.T) {
|
||||
for code, hint := range map[string]string{
|
||||
"patch_anchor_missing": PatchAnchorHint,
|
||||
"patch_anchor_ambiguous": PatchAnchorHint,
|
||||
"patch_room_open": PatchRoomOpenHint,
|
||||
} {
|
||||
var stderr bytes.Buffer
|
||||
cli.WritePatchRefusalError(&stderr, &cli.APIError{Code: code, Message: "m", Details: json.RawMessage(`{"ref":"TASK-1","edit":0}`)})
|
||||
res := extractStructuredCLIError(stderr.String())
|
||||
if res == nil {
|
||||
t.Fatalf("%s: marker not recognised (not in the allow-list?)", code)
|
||||
}
|
||||
env := res.StructuredContent.(ErrorEnvelope)
|
||||
if string(env.Error.Code) != code || env.Error.Hint != hint {
|
||||
t.Fatalf("%s: got %s %q", code, env.Error.Code, env.Error.Hint)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPLAN3578_CLIAndMCPAgreeOnPatchHints(t *testing.T) {
|
||||
if cli.PatchAnchorHint != PatchAnchorHint || cli.PatchRoomOpenHint != PatchRoomOpenHint {
|
||||
t.Fatal("patch hints differ between internal/cli and internal/mcp")
|
||||
}
|
||||
for _, c := range []string{cli.PatchAnchorMissingCode, cli.PatchAnchorAmbiguousCode, cli.PatchRoomOpenCode} {
|
||||
if _, ok := allowedStructuredErrorCodes[c]; !ok {
|
||||
t.Fatalf("%s is not in allowedStructuredErrorCodes", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (f *parentFixture) httpDispatcher() *HTTPHandlerDispatcher {
|
||||
return &HTTPHandlerDispatcher{
|
||||
Handler: f.srv,
|
||||
UserResolver: func(context.Context) *models.User { return f.owner },
|
||||
}
|
||||
}
|
||||
|
||||
func (f *parentFixture) env(t *testing.T) ActionEnv {
|
||||
t.Helper()
|
||||
return ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState(f.workspace.Slug), Dispatcher: f.httpDispatcher()}
|
||||
}
|
||||
|
||||
func (f *parentFixture) dispatchResult2(t *testing.T, cmd []string, input map[string]any) *CallToolResult {
|
||||
t.Helper()
|
||||
res, err := f.httpDispatcher().Dispatch(WithDispatchInput(context.Background(), input), cmd, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Dispatch(%v): %v", cmd, err)
|
||||
}
|
||||
return res
|
||||
}
|
||||
@@ -202,6 +202,9 @@ func TestEveryCLIMarkerCodeIsInTheStdioVocabulary(t *testing.T) {
|
||||
"WriteLeaseHeldError": func(w *bytes.Buffer) {
|
||||
cli.WriteLeaseHeldError(w, api("lease_held"))
|
||||
},
|
||||
"WritePatchRefusalError": func(w *bytes.Buffer) {
|
||||
cli.WritePatchRefusalError(w, api("patch_anchor_missing"))
|
||||
},
|
||||
}
|
||||
|
||||
for name, write := range writers {
|
||||
|
||||
+16
-1
@@ -1738,6 +1738,21 @@ const CmdhelpVersion = "0.1"
|
||||
// caller cannot read embeds nothing; a failed fetch stays non-fatal,
|
||||
// as on stdio. Stdio is unchanged.
|
||||
//
|
||||
// 0.87 — PLAN-3578 step 1. ADDITIVE bump on the v0.38 grounds (a new
|
||||
// action is the contract change): `pad_item.action=patch {ref, edits:
|
||||
// [{old, new, line_hint?}], expected_seq?}` replaces exact text in an
|
||||
// item's body. Each old must occur exactly once; edits apply in order,
|
||||
// all or nothing; the answer is {ref, seq, patch_id, version_id,
|
||||
// rebased, intervening?, diff}. A moved expected_seq does not refuse:
|
||||
// the patch applies and reports rebased with the intervening writes
|
||||
// ({from_seq, to_seq, changes, complete, truncated}). New structured
|
||||
// codes on both transports: patch_anchor_missing,
|
||||
// patch_anchor_ambiguous (details.lines), patch_room_open (the item is
|
||||
// open in an editor; not supported yet). `pad_item.action=get` gains
|
||||
// `lines` ("A-B"), which answers numbered body lines with seq and
|
||||
// content_state instead of the item. The catalog gains the
|
||||
// `array<object>` param type for `edits`.
|
||||
//
|
||||
// 0.86 — PLAN-3555 phase 1. ADDITIVE bump on the v0.53 grounds: a new
|
||||
// param and new response members, and no call that was accepted is
|
||||
// refused. `pad_item.action=update` gains `replace` (bool), the
|
||||
@@ -2049,7 +2064,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.86"
|
||||
const ToolSurfaceVersion = "0.87"
|
||||
|
||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||
|
||||
@@ -267,3 +267,30 @@ func AgentNameFromMetadata(metadata string) string {
|
||||
name, _ := m["agent"].(string)
|
||||
return name
|
||||
}
|
||||
|
||||
// PatchIntervening is what a rebased patch reports about the writes that
|
||||
// landed between the seq its caller read and the seq it applied over
|
||||
// (PLAN-3578). Changes are walked newest first along the activity seq chain
|
||||
// (metadata seq / prev_seq). Complete is true only when that chain is
|
||||
// unbroken from ToSeq back to FromSeq: a write that recorded no stamped entry
|
||||
// (a path that writes none, an entry that failed, a write from before the
|
||||
// stamps existed) breaks it, and then Complete is false.
|
||||
type PatchIntervening struct {
|
||||
FromSeq int64 `json:"from_seq"`
|
||||
ToSeq int64 `json:"to_seq"`
|
||||
Changes []PatchInterveningChange `json:"changes"`
|
||||
Complete bool `json:"complete"`
|
||||
Truncated bool `json:"truncated,omitempty"`
|
||||
}
|
||||
|
||||
// PatchInterveningChange is one stamped write in that range.
|
||||
type PatchInterveningChange struct {
|
||||
ActivityID string `json:"activity_id"`
|
||||
Seq int64 `json:"seq"`
|
||||
PrevSeq int64 `json:"prev_seq"`
|
||||
Action string `json:"action"`
|
||||
Actor string `json:"actor"`
|
||||
Source string `json:"source"`
|
||||
At time.Time `json:"at"`
|
||||
VersionID string `json:"version_id,omitempty"`
|
||||
}
|
||||
|
||||
@@ -566,6 +566,11 @@ type Item struct {
|
||||
// NOT silently fall back to its own stale snapshot — that fallback IS the
|
||||
// defect this field exists to remove.
|
||||
PreUpdate *Item `json:"-"`
|
||||
|
||||
// CreatedVersionID is the id of the version row this update wrote, empty
|
||||
// when the throttle wrote none (PLAN-3578: the activity entry names it).
|
||||
// Internal, like PreUpdate.
|
||||
CreatedVersionID string `json:"-"`
|
||||
// BodyEditedWithoutVersion is set by UpdateItem when the update changed
|
||||
// the body and the per-actor throttle wrote no version row, so the edit
|
||||
// has no version to show it (PLAN-2348 U2, for U3's "edited the body").
|
||||
@@ -1510,6 +1515,12 @@ type ItemUpdate struct {
|
||||
// ignores the member, which is the old replace-and-prune.
|
||||
RefusePendingEdits bool `json:"refuse_pending_edits,omitempty"`
|
||||
|
||||
// ContentTransform, when set, computes the new body from the row as it
|
||||
// stands inside the update's transaction (PLAN-3578 patch writes) and
|
||||
// replaces Content with it. An error aborts the write. Internal: no
|
||||
// request body can set it.
|
||||
ContentTransform func(existing *Item) (string, error) `json:"-"`
|
||||
|
||||
// Replace is the deliberate-overwrite opt-out (PLAN-3555): the caller means
|
||||
// to replace the body whatever was written since it read it, so the write
|
||||
// carries no version token on purpose. It changes no outcome in phase 1; it
|
||||
|
||||
@@ -50,6 +50,7 @@ var bug3407Exempt = map[string]bug3407Exemption{
|
||||
"handlers_items_bulk.go::bulkTagUpdate": {1, "bulk tag/untag: tags only, no fields"},
|
||||
"handlers_items_content_route.go::routeContentUpdate": {1, "passes the update handler's own input, which carries ValidatedSchema"},
|
||||
"handlers_items_content_route.go::applierFirstWrite": {1, "passes the update handler's own input (rowInput := *input)"},
|
||||
"handlers_items_patch.go::patchRoute": {1, "passes the update handler's own input, which carries ValidatedSchema (PLAN-3578 patch)"},
|
||||
"handlers_item_versions.go::handleRestoreItemVersion": {2, "version restore: content only, no fields"},
|
||||
}
|
||||
|
||||
|
||||
@@ -74,6 +74,12 @@ type serverCapabilities struct {
|
||||
// binary. It is here for any non-web caller, which against an older build
|
||||
// falls back to one PATCH per item.
|
||||
ItemSortOrderBulk bool `json:"item_sort_order_bulk"`
|
||||
|
||||
// ItemPatch is true when POST /workspaces/{ws}/items/{ref}/patch is served
|
||||
// (PLAN-3578). An older build answers that route with a bare 404 that
|
||||
// reads exactly like a missing item, so the CLI refuses instead of sending
|
||||
// when this is absent.
|
||||
ItemPatch bool `json:"item_patch"`
|
||||
}
|
||||
|
||||
// WHAT A BUILD THAT CANNOT DECODE A FORMAT ACTUALLY COSTS THE READER
|
||||
@@ -116,7 +122,7 @@ type serverCapabilities struct {
|
||||
// rather than 500-ing — that signals to the editor "uploads still work,
|
||||
// but disable transformation tools."
|
||||
func (s *Server) handleServerCapabilities(w http.ResponseWriter, r *http.Request) {
|
||||
resp := serverCapabilities{CollectionResolution: true, ItemFieldAppend: true, SearchCollectionResolution: true, ItemScopedCommentWrites: true, AttachmentAttach: true, LibraryActivate: true, BuiltinUpdate: true, ItemSortOrderBulk: true}
|
||||
resp := serverCapabilities{CollectionResolution: true, ItemFieldAppend: true, SearchCollectionResolution: true, ItemScopedCommentWrites: true, AttachmentAttach: true, LibraryActivate: true, BuiltinUpdate: true, ItemSortOrderBulk: true, ItemPatch: true}
|
||||
if s.imageProcessor != nil {
|
||||
resp.Image = s.imageProcessor.Capabilities()
|
||||
} else {
|
||||
|
||||
@@ -1059,9 +1059,9 @@ func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *mo
|
||||
"options_added": formatOptionsAdded(optionsAdded),
|
||||
"options_added_collection": coll.Name,
|
||||
})
|
||||
s.logActivityWithMeta(workspaceID, item.ID, "created", r, string(meta))
|
||||
s.logActivityWithMeta(workspaceID, item.ID, "created", r, stampSeqChain(string(meta), 0, item.Seq, ""))
|
||||
} else {
|
||||
s.logActivity(workspaceID, item.ID, "created", r)
|
||||
s.logActivityWithMeta(workspaceID, item.ID, "created", r, stampSeqChain("", 0, item.Seq, ""))
|
||||
}
|
||||
s.publishItemEventWithName(sseItemCreated, workspaceID, item.ID, item.Title, coll.Slug, actor, actorNameForCreate, source, item.Seq)
|
||||
|
||||
@@ -1381,6 +1381,9 @@ type builtinItemUpdate struct {
|
||||
// clearConvention removes the convention metadata (the library dropped
|
||||
// it), through the same lowering.
|
||||
clearConvention bool
|
||||
// patch is set only by handlePatchItem (PLAN-3578): a content-anchored
|
||||
// patch, whose body input.ContentTransform computes in the transaction.
|
||||
patch *itemPatch
|
||||
}
|
||||
|
||||
// updateItem is handleUpdateItem, or a built-in update when b is set.
|
||||
@@ -2468,7 +2471,7 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt
|
||||
// the direct one: no tab holds them, so the applier replacing the live
|
||||
// document does not touch them.
|
||||
var discardedSetAside int
|
||||
if input.Content != nil && !collabSnapshot {
|
||||
if (input.Content != nil || input.ContentTransform != nil) && !collabSnapshot {
|
||||
if input.OverwritePendingEdits {
|
||||
openChildrenPrecheck = composeSetAsideDiscard(s, item.ID, openChildrenPrecheck, &discardedSetAside)
|
||||
} else {
|
||||
@@ -2479,6 +2482,16 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt
|
||||
// BUG-3133 D: how many unflushed edit rows the direct path's prune deleted.
|
||||
var prunedPendingEdits int
|
||||
|
||||
// PLAN-3578: a patch takes the direct path only; an open editor refuses it.
|
||||
if input.ContentTransform != nil && s.collab != nil && !collabSnapshot {
|
||||
route, updated := s.patchRoute(w, r, item, &input, openChildrenPrecheck, parentLink, &prunedPendingEdits)
|
||||
if route == contentRouteHandled {
|
||||
return
|
||||
}
|
||||
fullWriteHandled = true
|
||||
fullWriteUpdated = updated
|
||||
}
|
||||
|
||||
if input.Content != nil && s.collab != nil && !collabSnapshot {
|
||||
// PLAN-2975: WRITE FIRST, APPLY SECOND on the applier path.
|
||||
//
|
||||
@@ -2526,6 +2539,10 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt
|
||||
updated, err = s.store.UpdateItemWithParentLink(item.ID, input, openChildrenPrecheck, parentLink)
|
||||
}
|
||||
if err != nil {
|
||||
// PLAN-3578: an anchor that no longer matches, when collab is off.
|
||||
if writePatchRefusal(w, itemRefOrSlug(*item), err) {
|
||||
return
|
||||
}
|
||||
// IDEA-1494 R2: surface the open-children guard rejection as
|
||||
// the structured 409 BEFORE the UNIQUE-constraint / generic
|
||||
// internal-error paths can swallow it. Same shape the
|
||||
@@ -2674,6 +2691,15 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt
|
||||
if updated.BodyEditedWithoutVersion {
|
||||
meta = withActivityMetaKey(meta, "body_edited", "true")
|
||||
}
|
||||
// PLAN-3578: the seq chain a rebased patch reports its intervening
|
||||
// writes from. Without a pre-image the link is unknown, so the entry
|
||||
// carries no stamp and the chain reads broken there.
|
||||
if before != nil {
|
||||
meta = stampSeqChain(meta, before.Seq, updated.Seq, updated.CreatedVersionID)
|
||||
}
|
||||
if b != nil && b.patch != nil {
|
||||
meta = b.patch.activityMeta(meta)
|
||||
}
|
||||
actor, source := actorFromRequest(r)
|
||||
// The error is CHECKED here (BUG-2779). The activity write is
|
||||
// best-effort — a failed audit entry must not fail the user's update,
|
||||
@@ -2820,6 +2846,10 @@ func (s *Server) updateItem(w http.ResponseWriter, r *http.Request, b *builtinIt
|
||||
updated.Warnings = warnings
|
||||
}
|
||||
|
||||
if b != nil && b.patch != nil {
|
||||
s.writePatchResponse(w, updated, b.patch)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, updated)
|
||||
}
|
||||
|
||||
|
||||
@@ -488,6 +488,9 @@ func (s *Server) writeTypedItemRefusal(w http.ResponseWriter, item *models.Item,
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
if writePatchRefusal(w, itemRefOrSlug(*item), err) {
|
||||
return true
|
||||
}
|
||||
if details, ok := asOpenChildrenGuardError(err); ok {
|
||||
writeOpenChildrenError(w, itemRefOrSlug(*item), details)
|
||||
return true
|
||||
|
||||
@@ -0,0 +1,230 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/bodypatch"
|
||||
"github.com/PerpetualSoftware/pad/internal/diff"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// PLAN-3578 step 1: content-anchored patch writes on an item with no live
|
||||
// editor. POST /workspaces/{ws}/items/{ref}/patch {edits, expected_seq?}.
|
||||
//
|
||||
// A patch is an ordinary PATCH underneath (updateItem, through the same hook a
|
||||
// built-in update uses), so it takes every guard a PATCH takes. What differs:
|
||||
// - the body is computed inside the store transaction from the row it
|
||||
// replaces (models.ItemUpdate.ContentTransform), so anchors match the
|
||||
// committed body, never an earlier read;
|
||||
// - expected_seq does not refuse: anchors are the precondition (ruling iii).
|
||||
// A moved seq is applied over and reported as rebased, with the
|
||||
// intervening writes and the diff;
|
||||
// - an item with a live editor is refused (patch_room_open). The open-room
|
||||
// path is step 2; a whole-document replace is never the fallback;
|
||||
// - unflushed editor edits refuse it, token or not: items.content is then
|
||||
// behind the document, and the anchors would match stale text.
|
||||
|
||||
type patchRequest struct {
|
||||
Edits []bodypatch.Edit `json:"edits"`
|
||||
ExpectedSeq *int64 `json:"expected_seq,omitempty"`
|
||||
}
|
||||
|
||||
// itemPatch carries one patch through updateItem. The transform fills the
|
||||
// in-transaction fields; the response and the activity read them.
|
||||
type itemPatch struct {
|
||||
id string
|
||||
edits []bodypatch.Edit
|
||||
expectedSeq *int64
|
||||
|
||||
baseSeq int64 // the row's seq inside the transaction, before the write
|
||||
before string // the body the edits applied to
|
||||
after string // the body they produced
|
||||
}
|
||||
|
||||
// maxPatchIntervening caps the intervening writes a rebased patch lists
|
||||
// (lead ruling); the diff is the real check.
|
||||
const maxPatchIntervening = 50
|
||||
|
||||
func (s *Server) handlePatchItem(w http.ResponseWriter, r *http.Request) {
|
||||
var req patchRequest
|
||||
if err := decodeJSON(r, &req); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "bad_request", err.Error())
|
||||
return
|
||||
}
|
||||
if len(req.Edits) == 0 {
|
||||
writeError(w, http.StatusBadRequest, "bad_request", "edits is required: at least one {old, new}")
|
||||
return
|
||||
}
|
||||
if len(req.Edits) > bodypatch.MaxEdits {
|
||||
writeError(w, http.StatusBadRequest, "bad_request", fmt.Sprintf("%d edits; at most %d per patch", len(req.Edits), bodypatch.MaxEdits))
|
||||
return
|
||||
}
|
||||
for i, e := range req.Edits {
|
||||
if e.Old == "" {
|
||||
writeError2(w, http.StatusBadRequest, "bad_request",
|
||||
fmt.Sprintf("edit %d: old is empty; every edit must quote the text it replaces", i),
|
||||
map[string]interface{}{"edit": i})
|
||||
return
|
||||
}
|
||||
}
|
||||
p := &itemPatch{id: uuid.NewString(), edits: req.Edits, expectedSeq: req.ExpectedSeq}
|
||||
input := models.ItemUpdate{
|
||||
ContentTransform: func(existing *models.Item) (string, error) {
|
||||
after, err := bodypatch.Apply(existing.Content, p.edits)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
p.baseSeq, p.before, p.after = existing.Seq, existing.Content, after
|
||||
return after, nil
|
||||
},
|
||||
// Every patch is a version, past the throttle, so it can be undone
|
||||
// from History (as the ChatGPT door's content writes are).
|
||||
ForceVersion: true,
|
||||
// Refuse over unflushed editor edits whether or not a token came.
|
||||
RefusePendingEdits: true,
|
||||
ChangeSummary: fmt.Sprintf("patch: %d edit(s)", len(req.Edits)),
|
||||
}
|
||||
s.updateItem(w, r, &builtinItemUpdate{input: input, patch: p})
|
||||
}
|
||||
|
||||
// writePatchRefusal answers a bodypatch refusal with its typed 409; false
|
||||
// when err is not one.
|
||||
func writePatchRefusal(w http.ResponseWriter, ref string, err error) bool {
|
||||
var missing *bodypatch.AnchorMissingError
|
||||
if errors.As(err, &missing) {
|
||||
writeError2(w, http.StatusConflict, "patch_anchor_missing",
|
||||
fmt.Sprintf("%s: edit %d's old text is not in the current body; re-read it and quote text that is there", ref, missing.Index),
|
||||
map[string]interface{}{"ref": ref, "edit": missing.Index})
|
||||
return true
|
||||
}
|
||||
var amb *bodypatch.AnchorAmbiguousError
|
||||
if errors.As(err, &amb) {
|
||||
writeError2(w, http.StatusConflict, "patch_anchor_ambiguous",
|
||||
fmt.Sprintf("%s: edit %d's old text occurs %d times; quote more surrounding text, or pass line_hint", ref, amb.Index, len(amb.Lines)),
|
||||
map[string]interface{}{"ref": ref, "edit": amb.Index, "lines": amb.Lines})
|
||||
return true
|
||||
}
|
||||
var empty *bodypatch.EmptyAnchorError
|
||||
if errors.As(err, &empty) {
|
||||
writeError2(w, http.StatusBadRequest, "bad_request", empty.Error(), map[string]interface{}{"edit": empty.Index})
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func writePatchRoomOpen(w http.ResponseWriter, ref string) {
|
||||
writeError2(w, http.StatusConflict, "patch_room_open",
|
||||
fmt.Sprintf("%s is open in an editor; a patch to an open item is not supported yet. Retry once the editor closes, or send a whole-body update", ref),
|
||||
map[string]interface{}{"ref": ref})
|
||||
}
|
||||
|
||||
// patchRoute is routeContentUpdate for a patch: the same settle, but a room
|
||||
// with an electable applier is refused rather than handed the content.
|
||||
func (s *Server) patchRoute(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
item *models.Item,
|
||||
input *models.ItemUpdate,
|
||||
precheck func(*sql.Tx, *models.Item) error,
|
||||
parentLink *store.ParentLinkUpdate,
|
||||
pruned *int,
|
||||
) (contentRoute, *models.Item) {
|
||||
var updated *models.Item
|
||||
outcome, paErr := settleContentRoute(
|
||||
r.Context(),
|
||||
func() bool { return s.collab.HasElectableApplier(item.ID) },
|
||||
func() error {
|
||||
return s.collab.PruneAndApply(item.ID, func() error {
|
||||
pc := composePruneWithPrecheck(s, item.ID, precheck, pruned)
|
||||
u, uerr := s.store.UpdateItemWithParentLink(item.ID, *input, pc, parentLink)
|
||||
if uerr != nil {
|
||||
return uerr
|
||||
}
|
||||
updated = u
|
||||
return nil
|
||||
})
|
||||
},
|
||||
applierSettleBudget, applierSettlePoll,
|
||||
)
|
||||
switch outcome {
|
||||
case settleElectApplier:
|
||||
writePatchRoomOpen(w, itemRefOrSlug(*item))
|
||||
return contentRouteHandled, nil
|
||||
case settleDirectWrote:
|
||||
return contentRouteDirectWrote, updated
|
||||
case settleUnsettled:
|
||||
writeRoomSettlingError(w, itemRefOrSlug(*item))
|
||||
return contentRouteHandled, nil
|
||||
default:
|
||||
if s.writeTypedItemRefusal(w, item, paErr, input.ChatGPTDoor) {
|
||||
return contentRouteHandled, nil
|
||||
}
|
||||
writeInternalError(w, paErr)
|
||||
return contentRouteHandled, nil
|
||||
}
|
||||
}
|
||||
|
||||
// patchActivityMeta adds the patch's own keys to the update's activity
|
||||
// entry: its id, how many edits, and whether it rebased.
|
||||
func (p *itemPatch) activityMeta(meta string) string {
|
||||
meta = withActivityMetaKey(meta, "patch_id", p.id)
|
||||
meta = withActivityMetaKey(meta, "patch_edits", strconv.Itoa(len(p.edits)))
|
||||
if p.rebased() {
|
||||
meta = withActivityMetaKey(meta, "patch_rebased_from_seq", strconv.FormatInt(*p.expectedSeq, 10))
|
||||
}
|
||||
return meta
|
||||
}
|
||||
|
||||
func (p *itemPatch) rebased() bool {
|
||||
return p.expectedSeq != nil && *p.expectedSeq != p.baseSeq
|
||||
}
|
||||
|
||||
// patchResponse is the answer to a patch.
|
||||
type patchResponse struct {
|
||||
Ref string `json:"ref"`
|
||||
Seq int64 `json:"seq"`
|
||||
PatchID string `json:"patch_id"`
|
||||
VersionID string `json:"version_id,omitempty"`
|
||||
Rebased bool `json:"rebased"`
|
||||
Intervening *models.PatchIntervening `json:"intervening,omitempty"`
|
||||
Diff string `json:"diff"`
|
||||
}
|
||||
|
||||
func (s *Server) writePatchResponse(w http.ResponseWriter, updated *models.Item, p *itemPatch) {
|
||||
resp := patchResponse{
|
||||
Ref: itemRefOrSlug(*updated),
|
||||
Seq: updated.Seq,
|
||||
PatchID: p.id,
|
||||
VersionID: updated.CreatedVersionID,
|
||||
Rebased: p.rebased(),
|
||||
Diff: diff.Unified(p.before, p.after, 3),
|
||||
}
|
||||
if resp.Rebased {
|
||||
iv, err := s.store.ItemSeqChain(updated.ID, *p.expectedSeq, p.baseSeq, maxPatchIntervening)
|
||||
if err != nil {
|
||||
// The write committed; an unreadable chain is reported as
|
||||
// incomplete rather than failing an answer about a done write.
|
||||
iv = &models.PatchIntervening{FromSeq: *p.expectedSeq, ToSeq: p.baseSeq}
|
||||
}
|
||||
resp.Intervening = iv
|
||||
}
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
// stampSeqChain records on an item activity entry the write's place in the
|
||||
// item's seq chain (PLAN-3578): it moved the item from prev to seq, and wrote
|
||||
// versionID when non-empty.
|
||||
func stampSeqChain(meta string, prev, seq int64, versionID string) string {
|
||||
meta = withActivityMetaKey(meta, store.ActivityMetaPrevSeq, strconv.FormatInt(prev, 10))
|
||||
meta = withActivityMetaKey(meta, store.ActivityMetaSeq, strconv.FormatInt(seq, 10))
|
||||
if versionID != "" {
|
||||
meta = withActivityMetaKey(meta, store.ActivityMetaVersionID, versionID)
|
||||
}
|
||||
return meta
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package server
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
"net/http"
|
||||
"sort"
|
||||
"strconv"
|
||||
@@ -854,7 +855,7 @@ func activityRecordsMore(metadata string) bool {
|
||||
// wrote no row for. It counts: the History tab (U3) renders it as
|
||||
// "edited the body", grouped into the nearest version's event, so a
|
||||
// row carrying only the marker is the record of that edit.
|
||||
if k == "agent" {
|
||||
if k == "agent" || activityBookkeepingKeys[k] {
|
||||
continue
|
||||
}
|
||||
if s := strings.TrimSpace(string(v)); s != `""` && s != "null" {
|
||||
@@ -864,6 +865,20 @@ func activityRecordsMore(metadata string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// activityBookkeepingKeys are metadata keys that record WHERE a write sits,
|
||||
// not WHAT it changed (PLAN-3578): the seq chain a rebased patch reports
|
||||
// from, and a patch's id. A row carrying only these has nothing to show; the
|
||||
// version card is the body edit's record.
|
||||
var activityBookkeepingKeys = map[string]bool{
|
||||
store.ActivityMetaSeq: true,
|
||||
store.ActivityMetaPrevSeq: true,
|
||||
store.ActivityMetaVersionID: true,
|
||||
store.ActivitySeqChainBroken: true,
|
||||
"patch_id": true,
|
||||
"patch_edits": true,
|
||||
"patch_rebased_from_seq": true,
|
||||
}
|
||||
|
||||
// activityChangesText returns the "changes" string an activity's metadata
|
||||
// carries, or "" when it has none or the metadata does not parse.
|
||||
func activityChangesText(metadata string) string {
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// PLAN-3578 step 1: the no-room patch.
|
||||
|
||||
type patchFixture struct {
|
||||
srv *Server
|
||||
slug string
|
||||
item models.Item
|
||||
path string
|
||||
}
|
||||
|
||||
func newPatchFixture(t *testing.T, srv *Server, body string) *patchFixture {
|
||||
t.Helper()
|
||||
slug := createWSWithCollections(t, srv)
|
||||
item := createTaskWithFields(t, srv, slug, "Patched", `{"status":"open"}`)
|
||||
f := &patchFixture{srv: srv, slug: slug, item: item, path: "/api/v1/workspaces/" + slug + "/items/" + item.Slug}
|
||||
if rr := doRequest(srv, "PATCH", f.path, map[string]any{"content": body, "replace": true}); rr.Code != http.StatusOK {
|
||||
t.Fatalf("seed body: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
return f
|
||||
}
|
||||
|
||||
func (f *patchFixture) current(t *testing.T) *models.Item {
|
||||
t.Helper()
|
||||
it, err := f.srv.store.GetItem(f.item.ID)
|
||||
if err != nil || it == nil {
|
||||
t.Fatalf("GetItem: %v", err)
|
||||
}
|
||||
return it
|
||||
}
|
||||
|
||||
func (f *patchFixture) patch(t *testing.T, body map[string]any) (int, map[string]any) {
|
||||
t.Helper()
|
||||
rr := doRequest(f.srv, "POST", f.path+"/patch", body)
|
||||
var out map[string]any
|
||||
_ = json.Unmarshal(rr.Body.Bytes(), &out)
|
||||
return rr.Code, out
|
||||
}
|
||||
|
||||
func edits(pairs ...string) []map[string]any {
|
||||
var out []map[string]any
|
||||
for i := 0; i+1 < len(pairs); i += 2 {
|
||||
out = append(out, map[string]any{"old": pairs[i], "new": pairs[i+1]})
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func TestPLAN3578_PatchAppliesAndReturnsTheDiff(t *testing.T) {
|
||||
f := newPatchFixture(t, testServer(t), "alpha\nbeta\ngamma\n")
|
||||
before := f.current(t)
|
||||
code, out := f.patch(t, map[string]any{"edits": edits("beta", "BETA"), "expected_seq": before.Seq})
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("patch: %d %v", code, out)
|
||||
}
|
||||
if got := f.current(t).Content; got != "alpha\nBETA\ngamma\n" {
|
||||
t.Fatalf("body = %q", got)
|
||||
}
|
||||
d, _ := out["diff"].(string)
|
||||
if !strings.Contains(d, "-beta\n") || !strings.Contains(d, "+BETA\n") {
|
||||
t.Fatalf("diff = %q", d)
|
||||
}
|
||||
if out["rebased"] != false || out["intervening"] != nil {
|
||||
t.Fatalf("an unmoved seq reported a rebase: %v", out)
|
||||
}
|
||||
if v, _ := out["version_id"].(string); v == "" {
|
||||
t.Fatalf("a patch must write a version (ForceVersion): %v", out)
|
||||
}
|
||||
if int64(out["seq"].(float64)) != f.current(t).Seq {
|
||||
t.Fatalf("seq in the answer is not the row's")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPLAN3578_PatchRefusals(t *testing.T) {
|
||||
f := newPatchFixture(t, testServer(t), "one\ntwo two\n")
|
||||
body := f.current(t).Content
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
edits []map[string]any
|
||||
status int
|
||||
code string
|
||||
}{
|
||||
{"missing anchor", edits("three", "3"), http.StatusConflict, "patch_anchor_missing"},
|
||||
{"ambiguous anchor", edits("two", "2"), http.StatusConflict, "patch_anchor_ambiguous"},
|
||||
{"empty anchor", edits("", "x"), http.StatusBadRequest, "bad_request"},
|
||||
{"later edit fails, earlier one not applied", edits("one", "1", "nope", "x"), http.StatusConflict, "patch_anchor_missing"},
|
||||
} {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
code, out := f.patch(t, map[string]any{"edits": c.edits})
|
||||
e, _ := out["error"].(map[string]any)
|
||||
if code != c.status || e["code"] != c.code {
|
||||
t.Fatalf("got %d %v", code, out)
|
||||
}
|
||||
if got := f.current(t).Content; got != body {
|
||||
t.Fatalf("a refused patch changed the body: %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
code, _ := f.patch(t, map[string]any{"edits": []map[string]any{}})
|
||||
if code != http.StatusBadRequest {
|
||||
t.Fatalf("no edits: %d", code)
|
||||
}
|
||||
}
|
||||
|
||||
// A moved seq is applied over when the anchors still match, and reported:
|
||||
// the intervening write, stamped in the chain, and complete.
|
||||
func TestPLAN3578_RebaseReportsTheInterveningWrites(t *testing.T) {
|
||||
f := newPatchFixture(t, testServer(t), "alpha\nbeta\n")
|
||||
read := f.current(t)
|
||||
// Someone else edits another part of the body, through the PATCH door.
|
||||
if rr := doRequestWithHeaders(f.srv, "PATCH", f.path, map[string]any{"content": "alpha\nbeta\nomega\n", "replace": true}, map[string]string{"X-Pad-Agent": "other"}); rr.Code != http.StatusOK {
|
||||
t.Fatalf("intervening write: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
mid := f.current(t)
|
||||
code, out := f.patch(t, map[string]any{"edits": edits("alpha", "ALPHA"), "expected_seq": read.Seq})
|
||||
if code != http.StatusOK || out["rebased"] != true {
|
||||
t.Fatalf("rebase: %d %v", code, out)
|
||||
}
|
||||
if got := f.current(t).Content; got != "ALPHA\nbeta\nomega\n" {
|
||||
t.Fatalf("the rebase lost the other write: %q", got)
|
||||
}
|
||||
iv, _ := out["intervening"].(map[string]any)
|
||||
changes, _ := iv["changes"].([]any)
|
||||
if iv["complete"] != true || len(changes) != 1 {
|
||||
t.Fatalf("intervening = %v", iv)
|
||||
}
|
||||
c := changes[0].(map[string]any)
|
||||
if int64(c["seq"].(float64)) != mid.Seq || int64(c["prev_seq"].(float64)) != read.Seq || c["actor"] != "agent" {
|
||||
t.Fatalf("change = %v (read %d, mid %d)", c, read.Seq, mid.Seq)
|
||||
}
|
||||
}
|
||||
|
||||
// A write that recorded no stamped entry breaks the chain: complete is false.
|
||||
func TestPLAN3578_UnstampedWriteBreaksTheChain(t *testing.T) {
|
||||
f := newPatchFixture(t, testServer(t), "alpha\nbeta\n")
|
||||
read := f.current(t)
|
||||
// A store write with no handler records no activity entry at all.
|
||||
if _, err := f.srv.store.UpdateItem(f.item.ID, models.ItemUpdate{FieldsPatch: map[string]interface{}{"status": "in-progress"}}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
code, out := f.patch(t, map[string]any{"edits": edits("beta", "B"), "expected_seq": read.Seq})
|
||||
if code != http.StatusOK || out["rebased"] != true {
|
||||
t.Fatalf("patch: %d %v", code, out)
|
||||
}
|
||||
iv, _ := out["intervening"].(map[string]any)
|
||||
if iv["complete"] != false {
|
||||
t.Fatalf("a write with no stamped entry read as complete: %v", iv)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPLAN3578_PatchRefusedWhileAnEditorIsOpen(t *testing.T) {
|
||||
srv := testServerWithCollab(t)
|
||||
ts := httptest.NewServer(srv)
|
||||
t.Cleanup(ts.Close)
|
||||
f := newPatchFixture(t, srv, "alpha\n")
|
||||
conn, resp, err := dialCollab(t, ts.URL, f.item.ID, nil, "")
|
||||
if err != nil {
|
||||
status := ""
|
||||
if resp != nil {
|
||||
status = resp.Status
|
||||
}
|
||||
t.Fatalf("dialCollab: %v (%s)", err, status)
|
||||
}
|
||||
stop := applierEcho(t, conn)
|
||||
t.Cleanup(stop)
|
||||
waitForApplierPath(t, srv, f.slug, f.item.Slug, f.item.ID)
|
||||
body := f.current(t).Content
|
||||
code, out := f.patch(t, map[string]any{"edits": edits("alpha", "A")})
|
||||
e, _ := out["error"].(map[string]any)
|
||||
if code != http.StatusConflict || e["code"] != "patch_room_open" {
|
||||
t.Fatalf("got %d %v", code, out)
|
||||
}
|
||||
if got := f.current(t).Content; got != body {
|
||||
t.Fatalf("a refused patch changed the body: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPLAN3578_PatchRefusedOverUnflushedEdits(t *testing.T) {
|
||||
srv := testServerWithCollab(t)
|
||||
f := newPatchFixture(t, srv, "alpha\n")
|
||||
if _, err := srv.store.AppendYjsUpdate(f.item.ID, []byte{0x00, 0x02, 0x04, 0x01, 0x9A, 0x7C, 0x00}, "1"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if f.current(t).ContentState != models.ContentOutcomeAppliedPendingFlush {
|
||||
t.Fatal("premise: the item must read pending")
|
||||
}
|
||||
code, out := f.patch(t, map[string]any{"edits": edits("alpha", "A")})
|
||||
e, _ := out["error"].(map[string]any)
|
||||
if code != http.StatusConflict || e["code"] != "content_pending_flush" {
|
||||
t.Fatalf("got %d %v", code, out)
|
||||
}
|
||||
}
|
||||
|
||||
// The seq-chain and patch keys are bookkeeping: an agent's body-only patch
|
||||
// renders its version card, never an empty activity card.
|
||||
func TestPLAN3578_PatchLeavesNoEmptyTimelineCard(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
token, ws, slug := debounceFixture(t, srv)
|
||||
rr := authedAgentRequest(t, srv, token, "wren", "PATCH", "/api/v1/workspaces/"+ws+"/items/"+slug,
|
||||
map[string]any{"content": "alpha\nbeta\n", "replace": true})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("seed: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr = authedAgentRequest(t, srv, token, "wren", "POST", "/api/v1/workspaces/"+ws+"/items/"+slug+"/patch",
|
||||
map[string]any{"edits": edits("beta", "BETA")})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("patch: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
for _, e := range updatedActivityEntries(fetchTimelineAuthed(t, srv, token, ws, slug).Entries) {
|
||||
if changesOf(t, e.Activity.Metadata) == "" {
|
||||
t.Fatalf("an updated entry with no change rendered: metadata %s", e.Activity.Metadata)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2208,6 +2208,8 @@ func (s *Server) setupRouter() {
|
||||
r.Route("/items/{itemSlug}", func(r chi.Router) {
|
||||
r.Get("/", s.handleGetItem)
|
||||
r.Patch("/", s.handleUpdateItem)
|
||||
// PLAN-3578: content-anchored patch writes.
|
||||
r.Post("/patch", s.handlePatchItem)
|
||||
r.Delete("/", s.handleDeleteItem)
|
||||
r.Post("/restore", s.handleRestoreItem)
|
||||
r.Post("/move", s.handleMoveItem)
|
||||
|
||||
@@ -534,6 +534,14 @@ func mergeActivityMeta(existing, incoming string) string {
|
||||
existChanges, _ := existMap["changes"].(string)
|
||||
newChanges, _ := newMap["changes"].(string)
|
||||
|
||||
// PLAN-3578: decide the seq chain from both sides BEFORE the overlay.
|
||||
chainExist := map[string]interface{}{}
|
||||
for _, k := range []string{ActivityMetaSeq, ActivityMetaPrevSeq} {
|
||||
if v, ok := existMap[k]; ok {
|
||||
chainExist[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
// Start with existing map as base, overlay new fields
|
||||
for k, v := range newMap {
|
||||
if k == "changes" {
|
||||
@@ -541,6 +549,18 @@ func mergeActivityMeta(existing, incoming string) string {
|
||||
}
|
||||
existMap[k] = v
|
||||
}
|
||||
if _, stamped := chainExist[ActivityMetaSeq]; stamped || newMap[ActivityMetaSeq] != nil {
|
||||
broken := existMap[ActivitySeqChainBroken]
|
||||
mergeSeqChain(chainExist, newMap)
|
||||
if v, ok := chainExist[ActivitySeqChainBroken]; ok {
|
||||
existMap[ActivitySeqChainBroken] = v
|
||||
} else if broken != nil {
|
||||
existMap[ActivitySeqChainBroken] = broken
|
||||
}
|
||||
if v, ok := chainExist[ActivityMetaPrevSeq]; ok && chainExist[ActivitySeqChainBroken] == nil {
|
||||
existMap[ActivityMetaPrevSeq] = v
|
||||
}
|
||||
}
|
||||
|
||||
if newChanges != "" {
|
||||
if existChanges != "" {
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strconv"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// Activity metadata keys that link an item's writes into a chain
|
||||
// (PLAN-3578). An entry stamped {seq: S, prev_seq: P} records a write that
|
||||
// moved the item from P to S. ActivitySeqChainBroken marks a merged entry
|
||||
// whose runs were not contiguous, so the chain cannot pass through it.
|
||||
const (
|
||||
ActivityMetaSeq = "seq"
|
||||
ActivityMetaPrevSeq = "prev_seq"
|
||||
ActivityMetaVersionID = "version_id"
|
||||
ActivitySeqChainBroken = "seq_chain_broken"
|
||||
itemSeqChainActivityWindow = 500
|
||||
)
|
||||
|
||||
// ItemSeqChain walks the item's stamped activity entries from toSeq back to
|
||||
// fromSeq and returns them newest first, at most max. Complete is true only
|
||||
// when the walk reaches fromSeq exactly without a gap.
|
||||
func (s *Store) ItemSeqChain(itemID string, fromSeq, toSeq int64, max int) (*models.PatchIntervening, error) {
|
||||
out := &models.PatchIntervening{FromSeq: fromSeq, ToSeq: toSeq, Changes: []models.PatchInterveningChange{}}
|
||||
if toSeq <= fromSeq {
|
||||
out.Complete = toSeq == fromSeq
|
||||
return out, nil
|
||||
}
|
||||
acts, err := s.ListDocumentActivity(itemID, models.ActivityListParams{Limit: itemSeqChainActivityWindow})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
bySeq := map[int64]models.PatchInterveningChange{}
|
||||
broken := map[int64]bool{}
|
||||
for _, a := range acts {
|
||||
seq, prev, vid, isBroken, ok := seqStamp(a.Metadata)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if _, seen := bySeq[seq]; seen {
|
||||
continue // newest first: the first one listed wins
|
||||
}
|
||||
bySeq[seq] = models.PatchInterveningChange{
|
||||
ActivityID: a.ID, Seq: seq, PrevSeq: prev, Action: a.Action,
|
||||
Actor: a.Actor, Source: a.Source, At: a.CreatedAt, VersionID: vid,
|
||||
}
|
||||
broken[seq] = isBroken
|
||||
}
|
||||
for cursor := toSeq; ; {
|
||||
if cursor == fromSeq {
|
||||
out.Complete = true
|
||||
return out, nil
|
||||
}
|
||||
c, ok := bySeq[cursor]
|
||||
// PrevSeq strictly below cursor keeps the walk finite on a malformed
|
||||
// entry. A walk that steps past fromSeq needs no check of its own:
|
||||
// seqs only fall, so it never meets fromSeq and ends at a missing
|
||||
// entry, incomplete.
|
||||
if !ok || broken[cursor] || c.PrevSeq >= cursor {
|
||||
return out, nil
|
||||
}
|
||||
if len(out.Changes) < max {
|
||||
out.Changes = append(out.Changes, c)
|
||||
} else {
|
||||
out.Truncated = true
|
||||
}
|
||||
cursor = c.PrevSeq
|
||||
}
|
||||
}
|
||||
|
||||
// seqStamp reads an entry's chain stamp. Values are strings (the handlers'
|
||||
// metadata maps are string-valued) or numbers; ok is false without both.
|
||||
func seqStamp(meta string) (seq, prev int64, versionID string, broken, ok bool) {
|
||||
var m map[string]interface{}
|
||||
if json.Unmarshal([]byte(meta), &m) != nil || m == nil {
|
||||
return 0, 0, "", false, false
|
||||
}
|
||||
seq, ok1 := metaInt(m[ActivityMetaSeq])
|
||||
prev, ok2 := metaInt(m[ActivityMetaPrevSeq])
|
||||
if !ok1 || !ok2 {
|
||||
return 0, 0, "", false, false
|
||||
}
|
||||
versionID, _ = m[ActivityMetaVersionID].(string)
|
||||
b, _ := m[ActivitySeqChainBroken].(string)
|
||||
return seq, prev, versionID, b == "true", true
|
||||
}
|
||||
|
||||
func metaInt(v interface{}) (int64, bool) {
|
||||
switch x := v.(type) {
|
||||
case string:
|
||||
n, err := strconv.ParseInt(x, 10, 64)
|
||||
return n, err == nil
|
||||
case float64:
|
||||
return int64(x), true
|
||||
}
|
||||
return 0, false
|
||||
}
|
||||
|
||||
// mergeSeqChain fixes the chain keys of a debounced merge: the merged entry
|
||||
// spans the first run's prev_seq to the latest seq, which is true only when
|
||||
// the runs were contiguous (the existing seq is the incoming prev_seq).
|
||||
// Otherwise a write between them recorded no stamp here, and the merged
|
||||
// entry is marked broken, so the chain can never read through the gap.
|
||||
func mergeSeqChain(existMap, newMap map[string]interface{}) {
|
||||
es, eok := metaInt(existMap[ActivityMetaSeq])
|
||||
ep, epok := metaInt(existMap[ActivityMetaPrevSeq])
|
||||
_, nok := metaInt(newMap[ActivityMetaSeq])
|
||||
np, npok := metaInt(newMap[ActivityMetaPrevSeq])
|
||||
if !nok || !npok {
|
||||
// The incoming write carried no stamp: the merged entry cannot
|
||||
// vouch for the span it now covers.
|
||||
if eok {
|
||||
existMap[ActivitySeqChainBroken] = "true"
|
||||
}
|
||||
return
|
||||
}
|
||||
if !eok || !epok || es != np {
|
||||
existMap[ActivitySeqChainBroken] = "true"
|
||||
return
|
||||
}
|
||||
// Contiguous: keep the first run's prev_seq (the overlay set the
|
||||
// incoming one).
|
||||
existMap[ActivityMetaPrevSeq] = strconv.FormatInt(ep, 10)
|
||||
}
|
||||
@@ -2825,6 +2825,18 @@ func (s *Store) updateItemWithParentLinkOnce(
|
||||
}
|
||||
}
|
||||
|
||||
// PLAN-3578: a content-anchored patch computes the new body from the row
|
||||
// as it stands INSIDE this transaction, after the lock, so its anchors are
|
||||
// matched against the body it replaces and not against an earlier read.
|
||||
// Its refusal (an anchor that no longer matches) aborts the write.
|
||||
if input.ContentTransform != nil {
|
||||
body, terr := input.ContentTransform(existing)
|
||||
if terr != nil {
|
||||
return nil, terr
|
||||
}
|
||||
input.Content = &body
|
||||
}
|
||||
|
||||
// IDEA-1494 round 2: run the caller's invariant check (open-children
|
||||
// guard) against the same in-tx snapshot the UPDATE will write. Closing
|
||||
// the guard-vs-write TOCTOU window relies on this ordering — every
|
||||
@@ -2919,6 +2931,7 @@ func (s *Store) updateItemWithParentLinkOnce(
|
||||
// rather than the row (BUG-3327): either way the body before the edit
|
||||
// is what a version must keep.
|
||||
bodyEditedWithoutVersion := false
|
||||
createdVersionID := ""
|
||||
newBody := input.Content
|
||||
if newBody == nil {
|
||||
newBody = input.ExternalContent
|
||||
@@ -2983,6 +2996,7 @@ func (s *Store) updateItemWithParentLinkOnce(
|
||||
|
||||
if shouldVersion {
|
||||
vid := newID()
|
||||
createdVersionID = vid
|
||||
versionContent := existing.Content
|
||||
isDiff := false
|
||||
patch := diff.CreateReversePatch(existing.Content, *newBody)
|
||||
@@ -3409,6 +3423,7 @@ func (s *Store) updateItemWithParentLinkOnce(
|
||||
// to set it" at exactly the call site that must not guess (BUG-2776).
|
||||
updated.PreUpdate = &preUpdate
|
||||
updated.BodyEditedWithoutVersion = bodyEditedWithoutVersion
|
||||
updated.CreatedVersionID = createdVersionID
|
||||
|
||||
// The choke point (SPEC-3 / TASK-2658). Emitted here — after the in-tx
|
||||
// read-back and after the status/assignment deltas are computed, before
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
func chainMeta(prev, seq string) string {
|
||||
b, _ := json.Marshal(map[string]string{ActivityMetaPrevSeq: prev, ActivityMetaSeq: seq})
|
||||
return string(b)
|
||||
}
|
||||
|
||||
// PLAN-3578: a debounced merge keeps the chain only when its runs are
|
||||
// contiguous; otherwise the merged entry is marked broken.
|
||||
func TestPLAN3578_MergeKeepsTheChainOnlyWhenContiguous(t *testing.T) {
|
||||
read := func(m string) map[string]interface{} {
|
||||
var out map[string]interface{}
|
||||
_ = json.Unmarshal([]byte(m), &out)
|
||||
return out
|
||||
}
|
||||
contiguous := read(mergeActivityMeta(chainMeta("10", "12"), chainMeta("12", "15")))
|
||||
if contiguous[ActivityMetaPrevSeq] != "10" || contiguous[ActivityMetaSeq] != "15" || contiguous[ActivitySeqChainBroken] != nil {
|
||||
t.Fatalf("contiguous merge = %v, want prev 10 seq 15 unbroken", contiguous)
|
||||
}
|
||||
gap := read(mergeActivityMeta(chainMeta("10", "12"), chainMeta("13", "15")))
|
||||
if gap[ActivitySeqChainBroken] != "true" {
|
||||
t.Fatalf("a merge across a gap was not marked broken: %v", gap)
|
||||
}
|
||||
unstamped := read(mergeActivityMeta(chainMeta("10", "12"), `{"changes":"status: a → b"}`))
|
||||
if unstamped[ActivitySeqChainBroken] != "true" {
|
||||
t.Fatalf("merging an unstamped write did not break the chain: %v", unstamped)
|
||||
}
|
||||
stays := read(mergeActivityMeta(mergeActivityMeta(chainMeta("10", "12"), chainMeta("13", "15")), chainMeta("15", "16")))
|
||||
if stays[ActivitySeqChainBroken] != "true" {
|
||||
t.Fatalf("a broken entry healed on a later contiguous merge: %v", stays)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPLAN3578_ItemSeqChainWalk(t *testing.T) {
|
||||
s := testStore(t)
|
||||
ws := createTestWorkspace(t, s, "Chain")
|
||||
col := createTestCollection(t, s, ws.ID, "Tasks")
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Chained", "")
|
||||
add := func(meta string) {
|
||||
t.Helper()
|
||||
if _, err := s.CreateActivity(models.Activity{WorkspaceID: ws.ID, DocumentID: item.ID, Action: "updated", Actor: "user", Source: "cli", Metadata: meta}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
add(chainMeta("5", "7"))
|
||||
add(chainMeta("7", "9"))
|
||||
add(chainMeta("9", "12"))
|
||||
|
||||
got, err := s.ItemSeqChain(item.ID, 5, 12, 50)
|
||||
if err != nil || !got.Complete || len(got.Changes) != 3 || got.Changes[0].Seq != 12 {
|
||||
t.Fatalf("full walk = %+v, %v", got, err)
|
||||
}
|
||||
got, _ = s.ItemSeqChain(item.ID, 5, 12, 2)
|
||||
if !got.Complete || !got.Truncated || len(got.Changes) != 2 {
|
||||
t.Fatalf("capped walk = %+v", got)
|
||||
}
|
||||
got, _ = s.ItemSeqChain(item.ID, 3, 12, 50)
|
||||
if got.Complete {
|
||||
t.Fatalf("a walk past the first stamp (a pre-deploy range) read complete: %+v", got)
|
||||
}
|
||||
got, _ = s.ItemSeqChain(item.ID, 6, 12, 50)
|
||||
if got.Complete {
|
||||
t.Fatalf("a from-seq inside a write's span read complete: %+v", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,97 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// PLAN-3578 (lead ruling, condition 1): a rebased patch reports its
|
||||
// intervening writes as complete only when the activity seq chain is
|
||||
// unbroken, and a write that moves an item's seq without a stamped entry
|
||||
// breaks it. Every statement that moves items.seq is classified here: STAMPED
|
||||
// (its door writes a {prev_seq, seq} entry) or BREAKS (it writes none, so a
|
||||
// patch rebased across it reads complete:false, which is the honest answer).
|
||||
// A NEW seq-moving site fails this test until it is classified, so the
|
||||
// decision is made rather than defaulted.
|
||||
var seqMovingSites = map[string]string{
|
||||
"items.go:updateItemWithParentLinkOnce": "STAMPED via updateItem (PATCH, the collab-snapshot flush, built-in update, patch). Store-direct callers (recovery flush, bulk update, internal writes) write no stamp and BREAK",
|
||||
"items.go:insertItemTx": "STAMPED as the chain root by createItemChecked (prev_seq 0); other creators start an item with no root, which no rebase can reach back past anyway",
|
||||
"fenced_items.go:CreateItem": "BREAKS: an app's item create (no stamped entry)",
|
||||
"fenced_items.go:UpdateItemFields": "BREAKS: an app's field write",
|
||||
"agent_roles.go:UpdateRoleSortOrder": "BREAKS: role-board reorder, many items at once",
|
||||
"item_sort_order.go:UpdateItemSortOrders": "BREAKS: drag reorder, many items at once",
|
||||
"attachments.go:RemapAttachmentReferencesInWorkspace": "BREAKS: attachment reference rewrite across a workspace",
|
||||
"collections.go:applyFieldMigrationsTx": "BREAKS: schema migration across a collection",
|
||||
"field_option_rename.go:writeRenamedArraysTx": "BREAKS: option rename across a collection",
|
||||
"wiki_links.go:cascadeTitleRename": "BREAKS: wiki-link rewrite in other items on a rename",
|
||||
"export.go:importWorkspace": "BREAKS: workspace import creates items",
|
||||
"items.go:DeleteItem": "BREAKS: archive",
|
||||
"items.go:restoreItemOnce": "BREAKS: restore",
|
||||
"items.go:bumpStructuralLinkSourceTx": "BREAKS: a link change bumps the source item",
|
||||
"items.go:moveItemWithPreCheckOnce": "BREAKS: collection move",
|
||||
"items_cross_workspace_copy.go:archiveItemForCopyTx": "BREAKS: archive-source on a cross-workspace move",
|
||||
}
|
||||
|
||||
func TestPLAN3578_EverySeqMovingSiteIsClassified(t *testing.T) {
|
||||
files, err := filepath.Glob("*.go")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
funcRe := regexp.MustCompile(`\nfunc (?:\([^)]*\) )?(\w+)`)
|
||||
found := map[string]bool{}
|
||||
for _, f := range files {
|
||||
if strings.HasSuffix(f, "_test.go") {
|
||||
continue
|
||||
}
|
||||
b, err := os.ReadFile(f)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
src := string(b)
|
||||
for off := 0; ; {
|
||||
k := strings.Index(src[off:], "nextWorkspaceSeqSubquery")
|
||||
if k < 0 {
|
||||
break
|
||||
}
|
||||
at := off + k
|
||||
off = at + 1
|
||||
lineStart := strings.LastIndex(src[:at], "\n") + 1
|
||||
lineEnd := strings.Index(src[at:], "\n")
|
||||
if lineEnd < 0 {
|
||||
lineEnd = len(src) - at
|
||||
}
|
||||
line := src[lineStart : at+lineEnd]
|
||||
if strings.HasPrefix(strings.TrimSpace(line), "//") || strings.HasPrefix(strings.TrimSpace(line), "const nextWorkspaceSeqSubquery") {
|
||||
continue
|
||||
}
|
||||
fns := funcRe.FindAllStringSubmatch(src[:at], -1)
|
||||
if len(fns) == 0 {
|
||||
t.Fatalf("%s: a seq-moving use outside any function", f)
|
||||
}
|
||||
found[f+":"+fns[len(fns)-1][1]] = true
|
||||
}
|
||||
}
|
||||
var unclassified, stale []string
|
||||
for k := range found {
|
||||
if _, ok := seqMovingSites[k]; !ok {
|
||||
unclassified = append(unclassified, k)
|
||||
}
|
||||
}
|
||||
for k := range seqMovingSites {
|
||||
if !found[k] {
|
||||
stale = append(stale, k)
|
||||
}
|
||||
}
|
||||
sort.Strings(unclassified)
|
||||
sort.Strings(stale)
|
||||
if len(unclassified) > 0 {
|
||||
t.Errorf("seq-moving sites with no classification (decide: does its door write a stamped activity entry, or does it break the chain?): %v", unclassified)
|
||||
}
|
||||
if len(stale) > 0 {
|
||||
t.Errorf("classified sites that no longer move seq (remove them): %v", stale)
|
||||
}
|
||||
}
|
||||
@@ -116,7 +116,7 @@ Interpret the user's intent and route to the appropriate action. Here are common
|
||||
|
||||
**Updating:** `pad item update <ref> --status X --comment "..."` — **always** include `--comment` on status changes to explain *why*. The audit trail is the whole point. Same pattern for priority/role/assign changes.
|
||||
|
||||
**Editing a body:** `--stdin` / `--content` REPLACE the whole body, so guard the write with the version you read. Read with `pad item show <ref> --agent` (it carries `seq` and `content`), edit that text, then write it back with `pad item update <ref> --stdin --expected-seq <that seq>`. A conflict means someone changed the item after your read. Re-read it and re-apply your edit to the CURRENT body; never resend yours, because that would undo their change. `--replace` overwrites on purpose. An unguarded body write prints a warning now, and a future release refuses it.
|
||||
**Editing a body:** to change PART of a body, prefer `pad item patch <ref> --old "exact text" --new "replacement"` (repeatable; `--edits @file.json` for many or multi-line edits). Each `--old` must occur exactly once in the current body, so quote enough context, and `pad item show <ref> --lines 40-80` numbers the lines to quote from. It prints the diff of exactly what changed: read it, because anchors guarantee position, not meaning. A patch rebases over other writes when its anchors still match. An anchor refusal means re-read and quote current text. `patch_room_open` means the item is open in an editor; use a guarded whole-body update instead. For a whole-body rewrite, `--stdin` / `--content` REPLACE the body, so guard the write with the version you read. Read with `pad item show <ref> --agent` (it carries `seq` and `content`), edit that text, then write it back with `pad item update <ref> --stdin --expected-seq <that seq>`. A conflict means someone changed the item after your read. Re-read it and re-apply your edit to the CURRENT body; never resend yours, because that would undo their change. `--replace` overwrites on purpose. An unguarded body write prints a warning now, and a future release refuses it.
|
||||
|
||||
**Working with attachments:** items reference attachments as `` (images) or `[label](pad-attachment:<uuid>)` (files). To inspect or read bytes, always use `pad attachment {list|show|view|upload|download}`. `view <uuid>` writes the bytes to a temp file and prints the path — compose with `IMG=$(pad attachment view <uuid>)`, then open it with whatever's available on the platform (`open "$IMG"` on macOS, `xdg-open "$IMG"` on Linux) or just read/describe the file directly.
|
||||
|
||||
@@ -189,6 +189,8 @@ pad item create <collection> "title" [--status X] [--priority X] [--parent REF]
|
||||
pad item list [collection] [--status X] [--role X] [--assign X] [--parent REF] [--all] [--field key=value]
|
||||
pad item show TASK-5 [--agent] # --agent: title, ref, status, fields AND body in one read. (--format markdown is the body only, for an edit round-trip with update --stdin; empty for a title-only item)
|
||||
pad item update TASK-5 [--status X] [--role X] [--assign X] [--comment "..."] [--stdin | --clear-content] [--expected-seq N | --replace]
|
||||
pad item patch TASK-5 --old "exact text" --new "replacement" [--expected-seq N] # part of a body; prints the diff
|
||||
pad item show TASK-5 --lines 40-80 # numbered body lines + seq, to quote a patch anchor
|
||||
pad item delete TASK-5
|
||||
pad item search "query"
|
||||
pad item comment TASK-5 "..." [--reply-to <comment-id>]
|
||||
|
||||
+3
-1
@@ -133,7 +133,7 @@ Interpret the user's intent and route to the appropriate action. Here are common
|
||||
|
||||
**Updating:** `pad item update <ref> --status X --comment "..."` — **always** include `--comment` on status changes to explain *why*. The audit trail is the whole point. Same pattern for priority/role/assign changes.
|
||||
|
||||
**Editing a body:** `--stdin` / `--content` REPLACE the whole body, so guard the write with the version you read. Read with `pad item show <ref> --agent` (it carries `seq` and `content`), edit that text, then write it back with `pad item update <ref> --stdin --expected-seq <that seq>`. A conflict means someone changed the item after your read. Re-read it and re-apply your edit to the CURRENT body; never resend yours, because that would undo their change. `--replace` overwrites on purpose. An unguarded body write prints a warning now, and a future release refuses it.
|
||||
**Editing a body:** to change PART of a body, prefer `pad item patch <ref> --old "exact text" --new "replacement"` (repeatable; `--edits @file.json` for many or multi-line edits). Each `--old` must occur exactly once in the current body, so quote enough context, and `pad item show <ref> --lines 40-80` numbers the lines to quote from. It prints the diff of exactly what changed: read it, because anchors guarantee position, not meaning. A patch rebases over other writes when its anchors still match. An anchor refusal means re-read and quote current text. `patch_room_open` means the item is open in an editor; use a guarded whole-body update instead. For a whole-body rewrite, `--stdin` / `--content` REPLACE the body, so guard the write with the version you read. Read with `pad item show <ref> --agent` (it carries `seq` and `content`), edit that text, then write it back with `pad item update <ref> --stdin --expected-seq <that seq>`. A conflict means someone changed the item after your read. Re-read it and re-apply your edit to the CURRENT body; never resend yours, because that would undo their change. `--replace` overwrites on purpose. An unguarded body write prints a warning now, and a future release refuses it.
|
||||
|
||||
**Claiming work before you start:** when you are about to work an item (not just discuss it), run `pad item claim <ref>` first (`pad_item action=claim` via MCP). If it fails with `lease_held`, another agent is on it: pick something else and don't retry in a loop. Release it with `pad item release <ref>` when you finish or get blocked. A lease expires on its own (15m by default: pass `--ttl 2h` for longer work, or re-claim to extend it), and `pad project next` / `ready` leave out items another holder has leased. The lease is held under this session's agent name, so two sessions on one account only get distinct leases when each is registered under its own name (`pad session register --agent <name>`).
|
||||
|
||||
@@ -204,6 +204,8 @@ pad item create <collection> "title" [--status X] [--priority X] [--parent REF]
|
||||
pad item list [collection] [--status X] [--role X] [--assign X] [--parent REF] [--all] [--field key=value]
|
||||
pad item show TASK-5 [--agent] # --agent: title, ref, status, fields AND body in one read. (--format markdown is the body only, for an edit round-trip with update --stdin; empty for a title-only item)
|
||||
pad item update TASK-5 [--status X] [--role X] [--assign X] [--comment "..."] [--stdin | --clear-content] [--expected-seq N | --replace]
|
||||
pad item patch TASK-5 --old "exact text" --new "replacement" [--expected-seq N] # part of a body; prints the diff
|
||||
pad item show TASK-5 --lines 40-80 # numbered body lines + seq, to quote a patch anchor
|
||||
pad item delete TASK-5
|
||||
pad item search "query"
|
||||
pad item comment TASK-5 "..." [--reply-to <comment-id>]
|
||||
|
||||
Reference in New Issue
Block a user