diff --git a/docs/api.md b/docs/api.md index ba9c9b126..8591a7e85 100644 --- a/docs/api.md +++ b/docs/api.md @@ -38,7 +38,7 @@ REST API at `/api/v1/`. Key endpoints: - `POST /workspaces/{ws}/items/{slug}/copy` — cross-workspace copy; with `archive_source` it is the move. Same request shape as the preflight. **Never retry it automatically** — there is no idempotency key, so a retry duplicates the item - `POST /workspaces/{ws}/items/{slug}/versions/{id}/restore` — restore an item's content to an earlier version. It prunes the whole collaborative op-log and force-refreshes open editors, and its "Restored from…" undo point is built from `items.content`. **Since BUG-3031 it REFUSES with `409 content_pending_flush`** (details `{ref, pending_rows}`, the BUG-3133 code) while content-bearing op-log rows sit above the flush watermark: those are edits no row holds, so a restore over them would leave them in no version. Set-aside rows (BUG-3244) refuse it the same way (`details.set_aside_rows`). The optional body `{"overwrite_pending_edits": true}` discards them (set-aside rows too) and restores, which is the pre-BUG-3031 behaviour; a bare POST with nothing pending is unchanged. The web version card drains its own editor first (BUG-2271), so it sees the refusal only for another session's unsaved edits, and then asks the user before resending with the override. A raw API caller that used to succeed here can now get the 409. There is no CLI or MCP restore, so no tool-surface bump - `GET /workspaces/{ws}/items/{slug}/versions/{id}/diff` — the change one version row records (PLAN-2348 U2): `{version, before, after}` from one reverse-patch chain walk. A create row (`is_create`) holds the body AS CREATED, so its pair is `""` → that body; an update row holds the body BEFORE its edit, so its pair is that body → the next newer row's body (or the current content for the newest). Version rows also carry `user_id` / `actor_name` and `lines_added` / `lines_removed` (the counts of the change the row's write recorded; absent = unknown, every row before migration 103). A body edit the version throttle wrote no row for marks its update activity `body_edited: "true"` -- `GET/DELETE /workspaces/{ws}/items/{slug}/collab-set-aside` — edits a collab schema-version rebuild set aside (BUG-3244). When an editor schema bump makes an item's op-log unreplayable, `maybeRebuildOnSchemaMismatch` moves its unflushed content-bearing rows to `item_yjs_updates_set_aside` instead of deleting them, and the item reads `content_state: superseded_set_aside` until they are recovered (TASK-3246) or discarded. GET returns them as raw Yjs updates (base64 `update_data`, original `op_log_id` order), readable by anyone who can read the item; DELETE (edit permission) discards them and clears the state without writing the body. CLI: `pad item set-aside [--discard]`. Not on MCP. They travel in the workspace bundle (`ItemExport.collab_set_aside`, additive, omitempty) and an import reattaches them, validating each row first; `pad db migrate-to-pg`'s stale-body gate asks about the OP-LOG only, since set-aside rows are carried. The schema version stays frozen until TASK-3246 (see "Tiptap multi-package coordinated bumps") +- `GET/DELETE /workspaces/{ws}/items/{slug}/collab-set-aside` — edits a collab schema-version rebuild set aside (BUG-3244). When an editor schema bump makes an item's op-log unreplayable, `maybeRebuildOnSchemaMismatch` moves its WHOLE op-log to `item_yjs_updates_set_aside` instead of deleting it, whenever the log holds unflushed content-bearing rows (TASK-3574 (4): the unflushed rows build on the rows below the watermark and do not replay alone; a fully flushed log sets aside nothing), and the item reads `content_state: superseded_set_aside` until they are recovered (TASK-3246) or discarded. GET returns them as raw Yjs updates (base64 `update_data`, original `op_log_id` order), readable by anyone who can read the item; DELETE (edit permission) discards them and clears the state without writing the body. CLI: `pad item set-aside [--discard]`. Not on MCP. They travel in the workspace bundle (`ItemExport.collab_set_aside`, additive, omitempty) and an import reattaches them, validating each row first; `pad db migrate-to-pg`'s stale-body gate asks about the OP-LOG only, since set-aside rows are carried. The schema version stays frozen until TASK-3246 (see "Tiptap multi-package coordinated bumps") - `POST /workspaces/{ws}/library/activate {key | title}` — create an item from a convention or playbook LIBRARY entry, server-side (TASK-3462): the fields a template seed of the same entry stores (conventions now carry their typed `convention` metadata through both doors), in the collection that declares the entry's artifact kind, and the item's BUILT-IN ORIGIN recorded. The CLI (against a server advertising `library_activate`), remote MCP `pad_library.activate` and the web Library page all use it; an unknown title answers 404 with the old `not found in convention or playbook library: ` wording. Activating an entry twice is not refused here (the CLI and MCP never refused it; a playbook's unique `invocation_slug` still does) - **Built-in origin** (TASK-3462): every convention and playbook Pad ships (library entries and template-only seeds) has a stable `key` (`playbook/ship`, `convention/<slug>`, `hiring/playbook/<slug>`, …) and a version that is a sha256 of its text (`collections.BuiltinEntry.Hash`: body plus the fields an update writes, never `status` or the title). An item made from one by template seeding or library activation records `{key, seed_hash, seed_content, seed_fields}` in `item_builtin_origin` (a side table, not item columns). Soft delete and restore keep it, a collection move keeps it, a copy (within or across workspaces) gets none, workspace export/import carries it (`ItemExport.builtin_origin`, additive, omitempty; an import validates it and skips a malformed one), and a request body cannot set it - `GET /workspaces/{ws}/items/{ref}/builtin` — the item's state relative to Pad's current text: `current` (nothing to offer), `update_available` (the library changed, the item is unedited), `diverged` (both changed), `unknown_origin` (version unknown, text differs), `unknown_entry` (a key this Pad does not ship). Offers carry `library` and, when known, `seed` text so a client can show what the LIBRARY changed apart from what the user changed, plus `current` (the item's own `{content, fields}`, read in the same request as `seq`, so a client compares against the text the token it sends describes) and `seq`. 404 `not_builtin` for an item made from no built-in. Readable by anyone who can read the item diff --git a/internal/server/bug3244_schema_rebuild_set_aside_test.go b/internal/server/bug3244_schema_rebuild_set_aside_test.go index df8d3dad3..e3ffe26b5 100644 --- a/internal/server/bug3244_schema_rebuild_set_aside_test.go +++ b/internal/server/bug3244_schema_rebuild_set_aside_test.go @@ -137,17 +137,22 @@ func TestBUG3244SchemaRebuildKeepsUnflushedEditsMarked(t *testing.T) { } }) - // Only edits the row never received are kept: a row at or below the flush - // watermark is already in items.content, and a SyncStep1 frame cannot - // change the document. What is kept is the frame byte for byte, with its - // era and original op-log id, because those are what recovery needs. - t.Run("bumped: only unflushed content-bearing rows are set aside, verbatim", func(t *testing.T) { + // TASK-3574 (4), lead ruling: when anything is unflushed, the WHOLE op-log + // is set aside, the flushed base and the SyncStep1 included, because the + // unflushed rows build on structs in the rows below the watermark and alone + // replay to nothing. (This subtest used to assert only the unflushed frame + // was kept.) What is kept is every frame byte for byte, in op-log order, + // with its era and original op-log id, because those are what recovery + // needs. + t.Run("bumped: the whole op-log is set aside, verbatim and in order", func(t *testing.T) { f := newBUG3244Fixture(t) - flushed := f.append(t, []byte{0x00, 0x02, 0x03, 0x09, 0x08, 0x07}, "1") + base := []byte{0x00, 0x02, 0x03, 0x09, 0x08, 0x07} + step1 := []byte{0x00, 0x00, 0x01, 0x00} + flushed := f.append(t, base, "1") if err := f.srv.store.SetItemContentFlushedOpLogIDForTesting(f.itemID, flushed); err != nil { t.Fatalf("set watermark: %v", err) } - f.append(t, []byte{0x00, 0x00, 0x01, 0x00}, "1") // SyncStep1: never content + mid := f.append(t, step1, "1") kept := f.append(t, bug3244Frame, "1") f.joinAt(t, "2") @@ -155,13 +160,18 @@ func TestBUG3244SchemaRebuildKeepsUnflushedEditsMarked(t *testing.T) { if err != nil { t.Fatalf("ListYjsSetAside: %v", err) } - if len(rows) != 1 { - t.Fatalf("set-aside rows = %d, want 1 (only the unflushed content-bearing frame): %+v", len(rows), rows) + want := []struct { + id int64 + data []byte + }{{flushed, base}, {mid, step1}, {kept, bug3244Frame}} + if len(rows) != len(want) { + t.Fatalf("set-aside rows = %d, want the whole log of %d: %+v", len(rows), len(want), rows) } - r := rows[0] - if r.OpLogID != kept || string(r.UpdateData) != string(bug3244Frame) || r.SchemaVersion != "1" { - t.Fatalf("set-aside row = {op_log_id %d, data %v, era %q}, want {%d, %v, %q}", - r.OpLogID, r.UpdateData, r.SchemaVersion, kept, bug3244Frame, "1") + for i, r := range rows { + if r.OpLogID != want[i].id || string(r.UpdateData) != string(want[i].data) || r.SchemaVersion != "1" { + t.Fatalf("set-aside row %d = {op_log_id %d, data %v, era %q}, want {%d, %v, %q}", + i, r.OpLogID, r.UpdateData, r.SchemaVersion, want[i].id, want[i].data, "1") + } } if n := f.opLogRows(t); n != 0 { t.Fatalf("op-log rows = %d, want 0", n) diff --git a/internal/store/task3574_set_aside_whole_test.go b/internal/store/task3574_set_aside_whole_test.go new file mode 100644 index 000000000..ab7a9ebb2 --- /dev/null +++ b/internal/store/task3574_set_aside_whole_test.go @@ -0,0 +1,99 @@ +package store_test + +import ( + "encoding/base64" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" + "github.com/PerpetualSoftware/pad/internal/store" +) + +// TASK-3574 (4): a schema rebuild sets aside the WHOLE op-log, not only the +// unflushed content-bearing suffix. The suffix builds on structs in the rows +// below the watermark, so alone it replays to nothing (measured: pending, +// rendered ""), and no decoder could recover it. A fully flushed log still +// sets aside nothing. +// +// Real Yjs frames (clientID 4242, pinned): u1 "alpha", u2 " beta", u3 " gamma" +// anchored on u2, and a SyncStep2 carrying u1+u2. +var task3574Frames = map[string]string{ + "u1": "AAIsAQOSIQAHAQdkZWZhdWx0AwlwYXJhZ3JhcGgHAJIhAAYEAJIhAQVhbHBoYQA=", + "u2": "AAIQAQGSIQeEkiEGBSBiZXRhAA==", + "u3": "AAIRAQGSIQyEkiELBiBnYW1tYQA=", + "step2": "AAE2AQSSIQAHAQdkZWZhdWx0AwlwYXJhZ3JhcGgHAJIhAAYEAJIhAQVhbHBoYYSSIQYFIGJldGEA", +} + +func task3574Frame(t *testing.T, name string) []byte { + t.Helper() + b, err := base64.StdEncoding.DecodeString(task3574Frames[name]) + if err != nil { + t.Fatal(err) + } + return b +} + +func TestSetAsideKeepsTheWholeOpLog(t *testing.T) { + eachBackend(t, func(t *testing.T, s *store.Store) { + _, _, item := seedStaleItem(t, s) + var want [][]byte + var ids []int64 + for _, name := range []string{"u1", "u2", "step2", "u3"} { + f := task3574Frame(t, name) + id, err := s.AppendYjsUpdate(item.ID, f, "1") + if err != nil { + t.Fatal(err) + } + want = append(want, f) + ids = append(ids, id) + } + // u1 is flushed: the base below the watermark. + if err := s.SetItemContentFlushedOpLogIDForTesting(item.ID, ids[0]); err != nil { + t.Fatal(err) + } + + moved, cleared, err := s.SetAsideAndClearOpLog(item.ID) + if err != nil || cleared != 4 { + t.Fatalf("SetAsideAndClearOpLog = (%d, %d, %v), want 4 cleared", moved, cleared, err) + } + rows, err := s.ListYjsSetAside(item.ID) + if err != nil { + t.Fatal(err) + } + if len(rows) != len(want) { + t.Fatalf("set aside %d rows, want the whole log of %d (the flushed base and the step2 included)", len(rows), len(want)) + } + for i, r := range rows { + if string(r.UpdateData) != string(want[i]) { + t.Fatalf("set-aside row %d is not op-log row %d verbatim, in order", i, i) + } + } + if got, _ := s.GetItem(item.ID); got.ContentState != models.ContentStateSetAside { + t.Fatalf("content_state %q, want set aside", got.ContentState) + } + }) +} + +// Control: with nothing unflushed, nothing is set aside and the item is clean. +func TestSetAsideOfAFlushedLogKeepsNothing(t *testing.T) { + eachBackend(t, func(t *testing.T, s *store.Store) { + _, _, item := seedStaleItem(t, s) + var last int64 + for _, name := range []string{"u1", "u2", "u3"} { + id, err := s.AppendYjsUpdate(item.ID, task3574Frame(t, name), "1") + if err != nil { + t.Fatal(err) + } + last = id + } + if err := s.SetItemContentFlushedOpLogIDForTesting(item.ID, last); err != nil { + t.Fatal(err) + } + moved, cleared, err := s.SetAsideAndClearOpLog(item.ID) + if err != nil || moved != 0 || cleared != 3 { + t.Fatalf("SetAsideAndClearOpLog = (%d moved, %d cleared, %v), want (0, 3)", moved, cleared, err) + } + if got, _ := s.GetItem(item.ID); got.ContentState != "" { + t.Fatalf("content_state %q, want clean", got.ContentState) + } + }) +} diff --git a/internal/store/yjs_set_aside.go b/internal/store/yjs_set_aside.go index 63741d421..11906376b 100644 --- a/internal/store/yjs_set_aside.go +++ b/internal/store/yjs_set_aside.go @@ -14,21 +14,27 @@ import ( // BUG-3244: edits a collab schema-version rebuild takes out of the op-log. // // A join at a new editor schema version cannot replay rows written under the -// old one, so the rebuild empties the item's op-log. The content-bearing rows -// above the flush watermark are edits items.content never received; they are -// MOVED to item_yjs_updates_set_aside first (migration 097 / pg 072), where no -// op-log reader sees them, and they hold content_state at -// models.ContentStateSetAside until they are recovered or explicitly -// discarded. Rows at or below the watermark are already in items.content, and -// non-content-bearing rows cannot change the document (BUG-3124), so neither -// is kept. +// old one, so the rebuild empties the item's op-log. When content-bearing rows +// above the flush watermark exist (edits items.content never received), the +// WHOLE op-log is MOVED to item_yjs_updates_set_aside first (migration 097 / +// pg 072), where no op-log reader sees it, and the item holds content_state at +// models.ContentStateSetAside until it is recovered or explicitly discarded. +// +// The whole log, not only that suffix (TASK-3574 (4), lead ruling): the +// unflushed rows build on structs in the rows below the watermark, so replayed +// alone they leave every edit pending and render nothing (measured), and no +// decoder could recover them. The base exists elsewhere only as markdown in +// items.content, not as Yjs structs. Non-content-bearing rows go too: a +// SyncStep2 can be the only copy of an edit (TASK-3574 (2)), and the others +// replay as no-ops. An item with nothing unflushed sets aside nothing. // setAsideBeforeClearHook, when set by a test, runs inside the transaction // before the DELETE: the gap a concurrent append would land in. var setAsideBeforeClearHook func() -// SetAsideAndClearOpLog moves the item's unflushed content-bearing op-log rows -// to the set-aside table and deletes its whole op-log, in one transaction, so +// SetAsideAndClearOpLog moves the item's WHOLE op-log to the set-aside table +// when it holds unflushed content-bearing rows (and nothing otherwise), and +// deletes its whole op-log, in one transaction, so // a failure leaves the op-log untouched rather than half moved. It returns how // many rows were set aside and how many op-log rows were deleted. // @@ -96,12 +102,17 @@ func (s *Store) SetAsideAndClearOpLog(itemID string) (setAside, cleared int64, e return 0, 0, fmt.Errorf("set aside op-log (watermark): %w", err) } err = nil - var keep []deletedRow + unflushed := false for _, r := range deleted { if r.contentBearing && r.id > watermark { - keep = append(keep, r) + unflushed = true + break } } + var keep []deletedRow + if unflushed { + keep = deleted // the whole log: replayable on its own (TASK-3574 (4)) + } sort.Slice(keep, func(i, j int) bool { return keep[i].id < keep[j].id }) now := time.Now().UTC().Format(time.RFC3339) diff --git a/web/src/lib/collab/materializer/task3574SetAsideReplay.test.ts b/web/src/lib/collab/materializer/task3574SetAsideReplay.test.ts new file mode 100644 index 000000000..9c54dda1b --- /dev/null +++ b/web/src/lib/collab/materializer/task3574SetAsideReplay.test.ts @@ -0,0 +1,32 @@ +// TASK-3574 (4): what a set-aside replays to. The rows the store's tests set +// aside (internal/store/task3574_set_aside_whole_test.go, the same pinned +// frames): the WHOLE log replays to the document; the old unflushed-suffix +// selection replayed to nothing, every edit pending. +import { describe, it, expect } from 'vitest'; +import * as Y from 'yjs'; +import { replayFrames } from './replay'; + +const frames: Record<string, string> = { + u1: 'AAIsAQOSIQAHAQdkZWZhdWx0AwlwYXJhZ3JhcGgHAJIhAAYEAJIhAQVhbHBoYQA=', + u2: 'AAIQAQGSIQeEkiEGBSBiZXRhAA==', + u3: 'AAIRAQGSIQyEkiELBiBnYW1tYQA=', + step2: 'AAE2AQSSIQAHAQdkZWZhdWx0AwlwYXJhZ3JhcGgHAJIhAAYEAJIhAQVhbHBoYYSSIQYFIGJldGEA', +}; +const bytes = (n: string) => new Uint8Array(Buffer.from(frames[n], 'base64')); + +function replay(names: string[]) { + const doc = new Y.Doc(); + replayFrames(doc, names.map(bytes)); + return { pending: doc.store.pendingStructs !== null, text: doc.getXmlFragment('default').toString() }; +} + +describe('TASK-3574 (4): set-aside rows replay on their own', () => { + it('the whole log (what is now set aside) replays to the document', () => { + const r = replay(['u1', 'u2', 'step2', 'u3']); + expect(r.pending).toBe(false); + expect(r.text).toContain('alpha beta gamma'); + }); + it('the old unflushed suffix alone replays to nothing', () => { + expect(replay(['u2', 'u3'])).toEqual({ pending: true, text: '' }); + }); +});