fix(collab): a REST content replacement fences tabs seeded from the old body, as a restore does (BUG-3746) (#2134)

* fix(collab): a REST content replacement fences tabs seeded from the old body, as a restore does (BUG-3746)

A prune-and-replace content write (content route and patch route, via
PruneAndApply) pruned the op-log and replaced items.content, but stamped
no seed boundary. A browser that fetched body A before the write and
joined after it, with since=0 and its old content_seq, was admitted,
seeded A into the empty document and flushed it over B. Join already
force-refreshes a client whose content_seq predates the last restore
(BUG-2264); replacements now stamp that same boundary: MarkRestoreBoundary
on the write (durable last_restore_seq, in its tx) and SetLastRestoreSeq
in memory, under the item lock Join takes, so no join lands in between.

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

* test(collab): the BUG-3746 boundary test uses a collab-enabled server (BUG-3746)

testServer has no collab manager, so the PATCH took the ordinary write
path, never reached PruneAndApply, and the test failed with the fix in
place (CI on #2134). Production always runs with collab; the test now
does too. The seeded op-log row is dropped: the boundary does not depend
on it.

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

* test(collab): the patch route has its own seed-boundary leg (BUG-3746)

The patch route prunes and replaces through its own PruneAndApply call,
wired separately from the content route, so a future edit dropping
fenceSeedsBefore there would pass the content-route test. Its twin
asserts the boundary moves past the seeded body's seq to the patch's.

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

* test(collab): a REST replacement admits a tab seeded at the new seq and refuses one seeded before it, through a real join (BUG-3746)

Heron, GO-blocking on #2134: the fence must not over-fence, or every fresh
tab after a REST replace would be force-refreshed, possibly in a loop. Per
route (CONVE-19), after the replacement a tab joining with
content_seq = the new seq gets op_log_cursor, and one at seq - 1 gets
force_refresh.

Mutant: the in-memory boundary stamped one past the write fails both admit
legs ("the fence over-fences"). Counterfactual: with the fence removed, both
refusal legs fail.

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

internal/models/item.go

@@ -1405,7 +1405,8 @@ type ItemUpdate struct {
 	ActorUserID string `json:"-"`
 	// MarkRestoreBoundary stamps items.last_restore_seq with this update's newly
 	// assigned seq, INSIDE the same transaction as the content write + op-log
-	// prune (BUG-2264). Set only by version RESTORE. That durable per-item
+	// prune (BUG-2264). Set by version RESTORE and, since BUG-3746, by every
+	// prune-and-replace content write (fenceSeedsBefore). That durable per-item
 	// boundary is what Join reads to force_refresh a client whose ?content_seq
 	// seed predates the restore — surviving a server restart, unlike the
 	// in-memory fast-path. Internal-only (`json:"-"`); no HTTP client can set it.

internal/server/bug3746_prune_replace_fence_test.go

@@ -0,0 +1,161 @@
+package server
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/gorilla/websocket"
+
+	"github.com/PerpetualSoftware/pad/internal/collab"
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// BUG-3746: a REST content replacement prunes the op-log and replaces
+// items.content, but did not stamp the seed boundary a restore stamps. A tab
+// that fetched the old body and joined afterwards with since=0 and its old
+// content_seq was admitted, seeded the old body into the empty document, and
+// flushed it over the new one. Join's fence (contentSeq < last restore seq →
+// force_refresh) is unchanged and already tested for restores; this pins that
+// a replacement now moves the boundary, durably and in memory.
+func TestBUG3746_ContentReplacementMovesTheSeedBoundary(t *testing.T) {
+	srv := testServerWithCollab(t)
+	slug := createWSWithCollections(t, srv)
+	rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/tasks/items", map[string]interface{}{
+		"title":   "BUG-3746",
+		"content": "A",
+		"fields":  `{"status":"open"}`,
+	})
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("create: %d %s", rr.Code, rr.Body.String())
+	}
+	var created models.Item
+	parseJSON(t, rr, &created)
+	seqA := created.Seq
+
+	rr = doRequest(srv, "PATCH", "/api/v1/workspaces/"+slug+"/items/"+created.Slug,
+		map[string]interface{}{"content": "B"})
+	if rr.Code != http.StatusOK {
+		t.Fatalf("replace: %d %s", rr.Code, rr.Body.String())
+	}
+	var updated models.Item
+	parseJSON(t, rr, &updated)
+	if updated.Content != "B" || updated.Seq <= seqA {
+		t.Fatalf("precondition: replacement did not land (content %q, seq %d after %d)", updated.Content, updated.Seq, seqA)
+	}
+
+	durable, ok, err := srv.store.ItemLastRestoreSeq(created.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !ok || durable != updated.Seq {
+		t.Fatalf("durable seed boundary = (%d, %v), want (%d, true): a tab seeded at seq %d would be admitted", durable, ok, updated.Seq, seqA)
+	}
+	if mem, ok := srv.collab.LastRestoreSeq(created.ID); !ok || mem != updated.Seq {
+		t.Fatalf("in-memory seed boundary = (%d, %v), want (%d, true): Join reads it before the column", mem, ok, updated.Seq)
+	}
+}
+
+// The patch route's twin. It prunes and replaces through its own
+// PruneAndApply call (handlers_items_patch.go), so it is wired separately
+// and needs its own leg (CONVE-19). The fixture seeds the body through the
+// content route, which already stamps a boundary, so this asserts the
+// boundary moves past the seed's seq to the patch's.
+func TestBUG3746_PatchReplacementMovesTheSeedBoundary(t *testing.T) {
+	srv := testServerWithCollab(t)
+	f := newPatchFixture(t, srv, "alpha\nbeta\ngamma\n")
+	seeded := f.current(t).Seq
+
+	code, out := f.patch(t, map[string]any{"edits": edits("beta", "BETA")})
+	if code != http.StatusOK {
+		t.Fatalf("patch: %d %v", code, out)
+	}
+	after := f.current(t)
+	if after.Seq <= seeded {
+		t.Fatalf("precondition: the patch did not land (seq %d after %d)", after.Seq, seeded)
+	}
+
+	durable, ok, err := srv.store.ItemLastRestoreSeq(f.item.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !ok || durable != after.Seq {
+		t.Fatalf("durable seed boundary = (%d, %v), want (%d, true): a tab seeded before the patch (seq %d) would be admitted", durable, ok, after.Seq, seeded)
+	}
+	if mem, ok := srv.collab.LastRestoreSeq(f.item.ID); !ok || mem != after.Seq {
+		t.Fatalf("in-memory seed boundary = (%d, %v), want (%d, true)", mem, ok, after.Seq)
+	}
+}
+
+// joinSeededAt joins the item's room as a tab whose document was seeded from
+// items.content at contentSeq (since=0), and returns the first control frame:
+// op_log_cursor for an admitted tab, force_refresh for a refused seed.
+func joinSeededAt(t *testing.T, srv *Server, itemID string, contentSeq int64) string {
+	t.Helper()
+	ts := httptest.NewServer(srv)
+	defer ts.Close()
+	u, _ := url.Parse(ts.URL)
+	q := url.Values{}
+	q.Set("schema_version", collab.DefaultSchemaVersion)
+	q.Set("content_seq", fmt.Sprint(contentSeq))
+	c, _, err := (&websocket.Dialer{HandshakeTimeout: 3 * time.Second}).Dial("ws://"+u.Host+"/api/v1/collab/"+itemID+"?"+q.Encode(), nil)
+	if err != nil {
+		t.Fatalf("dial: %v", err)
+	}
+	defer c.Close()
+	return firstControl(t, c).Type
+}
+
+// The fence must not OVER-fence (Heron, GO-blocking on #2134): a tab seeded
+// from the replacement itself, at the new seq, is admitted normally, or every
+// fresh tab after a REST replace would be force-refreshed, possibly in a loop.
+// A tab one generation older is refused, through an actual join. One leg per
+// route (CONVE-19).
+func TestBUG3746_ContentReplacementAdmitsTheNewSeedAndRefusesTheOld(t *testing.T) {
+	srv := testServerWithCollab(t)
+	slug := createWSWithCollections(t, srv)
+	rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/tasks/items", map[string]interface{}{
+		"title":   "BUG-3746 admit",
+		"content": "A",
+		"fields":  `{"status":"open"}`,
+	})
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("create: %d %s", rr.Code, rr.Body.String())
+	}
+	var created models.Item
+	parseJSON(t, rr, &created)
+	rr = doRequest(srv, "PATCH", "/api/v1/workspaces/"+slug+"/items/"+created.Slug,
+		map[string]interface{}{"content": "B"})
+	if rr.Code != http.StatusOK {
+		t.Fatalf("replace: %d %s", rr.Code, rr.Body.String())
+	}
+	var updated models.Item
+	parseJSON(t, rr, &updated)
+
+	if got := joinSeededAt(t, srv, created.ID, updated.Seq); got != collab.ControlMessageOpLogCursor {
+		t.Fatalf("a tab seeded from the replacement (seq %d) got %q, want %q: the fence over-fences", updated.Seq, got, collab.ControlMessageOpLogCursor)
+	}
+	if got := joinSeededAt(t, srv, created.ID, updated.Seq-1); got != collab.ControlMessageForceRefresh {
+		t.Fatalf("a tab seeded before the replacement (seq %d) got %q, want %q", updated.Seq-1, got, collab.ControlMessageForceRefresh)
+	}
+}
+
+func TestBUG3746_PatchReplacementAdmitsTheNewSeedAndRefusesTheOld(t *testing.T) {
+	srv := testServerWithCollab(t)
+	f := newPatchFixture(t, srv, "alpha\nbeta\ngamma\n")
+	code, out := f.patch(t, map[string]any{"edits": edits("beta", "BETA")})
+	if code != http.StatusOK {
+		t.Fatalf("patch: %d %v", code, out)
+	}
+	after := f.current(t)
+
+	if got := joinSeededAt(t, srv, f.item.ID, after.Seq); got != collab.ControlMessageOpLogCursor {
+		t.Fatalf("a tab seeded from the patch (seq %d) got %q, want %q: the fence over-fences", after.Seq, got, collab.ControlMessageOpLogCursor)
+	}
+	if got := joinSeededAt(t, srv, f.item.ID, after.Seq-1); got != collab.ControlMessageForceRefresh {
+		t.Fatalf("a tab seeded before the patch (seq %d) got %q, want %q", after.Seq-1, got, collab.ControlMessageForceRefresh)
+	}
+}

internal/server/handlers_items_content_route.go

@@ -153,10 +153,11 @@ func (s *Server) routeContentUpdate(
 		func() error {
 			return s.collab.PruneAndApply(item.ID, func() error {
 				precheck := composePruneWithPrecheck(s, item.ID, openChildrenPrecheck, pruned)
-				u, uerr := s.store.UpdateItemWithParentLink(item.ID, *input, precheck, parentLink)
+				u, uerr := s.store.UpdateItemWithParentLink(item.ID, fenceSeedsBefore(*input), precheck, parentLink)
 				if uerr != nil {
 					return uerr
 				}
+				s.noteSeedBoundary(item.ID, u)
 				updated = u
 				return nil
 			})
@@ -377,6 +378,28 @@ func classifyApplyOutcome(err error) string {
 	}
 }
 
+// fenceSeedsBefore marks a prune-and-replace content write as a seed boundary
+// (BUG-3746). The write prunes the op-log and replaces items.content, so a tab
+// that fetched the old body and has not joined yet would, on a since=0 join,
+// seed that old body into the now-empty document and flush it over the new
+// one. A restore had the same hole and closed it with last_restore_seq
+// (BUG-2264); every prune-and-replace now stamps it too, so Join
+// force-refreshes a client whose ?content_seq predates the write.
+func fenceSeedsBefore(input models.ItemUpdate) models.ItemUpdate {
+	input.MarkRestoreBoundary = true
+	return input
+}
+
+// noteSeedBoundary records the write's seq in the collab manager's in-memory
+// fence, which Join reads before the durable column. Called inside
+// PruneAndApply, under the item lock Join also takes, so no join lands between
+// the commit and this. Monotonic: an older seq never lowers it.
+func (s *Server) noteSeedBoundary(itemID string, updated *models.Item) {
+	if updated != nil {
+		s.collab.SetLastRestoreSeq(itemID, updated.Seq)
+	}
+}
+
 // composePruneWithPrecheck rides the op-log prune inside the write's own transaction
 // (BUG-2840 half B) by composing it onto the precheck hook UpdateItemWithParentLink
 // runs there, so a refusal from either rolls the prune back.

internal/server/handlers_items_patch.go

@@ -319,10 +319,11 @@ func (s *Server) patchRoute(
 		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)
+				u, uerr := s.store.UpdateItemWithParentLink(item.ID, fenceSeedsBefore(*input), pc, parentLink)
 				if uerr != nil {
 					return uerr
 				}
+				s.noteSeedBoundary(item.ID, u)
 				updated = u
 				return nil
 			})