fix(collab): a schema rebuild sets aside the WHOLE op-log, so set-aside edits can be replayed (TASK-3574 (4)) (#2047)

SetAsideAndClearOpLog kept only the content-bearing rows above the flush
watermark. Those rows build on structs in the rows below it, so replayed
alone they leave every edit pending and render nothing (measured: pending,
''), and no decoder could ever recover them from the set-aside rows; the
base existed elsewhere only as markdown in items.content.

When the log holds any unflushed content-bearing row, the rebuild now sets
aside the WHOLE op-log, every row verbatim and in op-log order, the flushed
base and the non-content-bearing frames included (a SyncStep2 can be the
only copy of an edit, TASK-3574 (2); the rest replay as no-ops). A fully
flushed log still sets aside nothing and the item stays clean. Not live
while SCHEMA_VERSION is frozen; TASK-3246's decoder now gets a replayable
quarantine.

Tests: the store sets aside all four rows of [u1 flushed, u2, step2, u3]
verbatim and in order; a flushed log sets aside nothing; a vitest shows
those rows replay to 'alpha beta gamma' while the old suffix replays to ''.
TEST CHANGE (the ruling replaces what it pinned):
TestBUG3244SchemaRebuildKeepsUnflushedEditsMarked's 'only unflushed
content-bearing rows' subtest now asserts the whole log, in order.
Counterfactual: with the old selection the store keeps 2 of 4 rows.
docs/api.md updated.

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

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 <ref> [--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 <ref> [--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: <title>` 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

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)

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)
+		}
+	})
+}

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)

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: '' });
+	});
+});