oficina / docs Public

Pre-merge fix wave: close the snapshot tail's fifth blind spot, spec the re-seed

Finding 1 (test-only): both store.go's replay and handlers.go's updatesGet
compute the tail as seq > snap.Seq, correctly, but mutating either to
seq > snap.Seq + 1 (dropping the first row after the snapshot) passed every
test on this branch. TestUpdatesGetServesSnapshotThenTail now asserts the
response body byte-for-byte (snapshot State + tail rows), mirroring
TestRawParameterReturnsOnlyLogRows. The equivalence table in
TestSnapshotThenTailRendersIdenticallyToAFullReplay gains a case with the
distinct row at the first tail seq (snapshotCadence+1) rather than inside
the snapshot's range, with its vacuity guard inverted accordingly. Verified
both mutations now fail a test, then reverted; production code unchanged.

Finding 2 (spec drift): documents D8, the in-memory re-seed a tail replay
performs when it meets core.ErrUpdateLogFull past a usable snapshot (an
older-ABI or corrupted snapshot on a log longer than the retained cap) --
implemented and tested on this branch (reseedForReplay) but never written
into the design spec. Adds D8, a failure-table row, a review-record note
that it was added post-review from a task-review finding, and corrects D4,
which read as though ErrUpdateLogFull still ends the story once snapshot
writes fail repeatedly -- it no longer does, for a read or the write riding
the same replay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 61624084669d02e060f580cb2f43131a5277e86c parent bdb7f7c

3 files changed, +135 −11

docs/superpowers/specs/2026-09-01-snapshots-design.md +71 −4
internal/docs/cursor_test.go +34 −0
internal/docs/snapshots_test.go +30 −7

CI passed — run details

diff --git a/docs/superpowers/specs/2026-09-01-snapshots-design.md b/docs/superpowers/specs/2026-09-01-snapshots-design.md

index 3ad05ef..a6d442a 100644

--- a/docs/superpowers/specs/2026-09-01-snapshots-design.md

+++ b/docs/superpowers/specs/2026-09-01-snapshots-design.md

@@ -201,10 +201,16 @@ count in a `Handle` stays around 16× clear of `maxRetainedUpdates`.

**Conditionally, and the condition belongs in writing.** That bound holds only

while snapshot writes keep succeeding. Roughly fifteen consecutive cadence

failures — each logged at Error, so noisy rather than silent — walk the tail back

-to the cliff, and at that point the failure table's "correct, possibly slower"

-bar is no longer met: it ends in `ErrUpdateLogFull` again. The cliff is removed in

-every ordinary case and merely deferred in a persistently failing one, which is a

-weaker claim than "retired" and the true one.

+to the cliff. Before D8 that is where this section ended: the next replay past

+that point met `ErrUpdateLogFull` inside `Store.replay` itself, for a read exactly

+as for a write, and the failure table's "correct, possibly slower" bar was not

+met. D8 closes that: a tail replay that meets the cap re-seeds a fresh handle from

+its own state, in memory, and carries on, so reaching the cliff this way no longer

+ends a read, or the write riding the same replay. What is left of the condition is

+cost, not failure — a re-seed on every replay until a persisted snapshot finally

+succeeds and this section's 16× bound is restored. The cliff is removed in every

+ordinary case and merely deferred to a per-replay cost in a persistently failing

+one, which is a weaker claim than "retired" and the true one.

The cadence distance is read from the snapshot row §D6's replay already loads,

not from a separate query — the same discipline as the live-sync design's D2, for

@@ -286,6 +292,57 @@ rows, and `applyServerLog` refetches with it once when a decode throws. Small, a

it makes the log's authority reachable from the one reader for whom it matters

most.

+### D8 — A tail replay past the cap re-seeds in memory rather than failing

+Found after this design was reviewed, in a task review of its implementation:

+what happens when `Store.replay` has no usable snapshot to start from — D2's

+ordinary "discarded, full replay" story for a corrupt row, a stale one, or (the

+one that will actually happen) every row at once on the deploy that ships the

+next ABI bump, since `newestSnapshot`'s exact-ABI match invalidates them all

+together — and the log it falls back to is longer than

+`core.MaxRetainedUpdates`? Full replay is precisely what §1 opens by naming as

+already unsafe past that count: it is the cliff `Handle.Apply` was built to stop

+at, met this time from inside `Store.replay`'s own tail loop instead of from a

+client's POST, and reached with no snapshot in hand to shorten the log that

+triggers it.

+§3's property closes it the same way it closes everything else here: a snapshot

+is nothing but a handle's own state, exported, and applying it to an empty

+document reproduces the handle. `Store.replay`'s tail loop uses that on itself.

+When applying a row meets `core.ErrUpdateLogFull`, it takes that same export of

+the handle it already has — mid-loop, via `Handle.Snapshot` — opens a fresh

+handle from the document's own stored GUID and client id (never a new identity;

+see Document), applies the export, and continues from the row that was refused,

+which is re-applied into the fresh handle and not dropped. `reseedForReplay` in

+`internal/docs/store.go` is the mechanism; its call site is the one place in the

+tail loop that catches `core.ErrUpdateLogFull` rather than surfacing it. A log of

+any length gets through in as many re-seeds as it needs, and what a replay

+retains at once stays bounded by the cap regardless — the same bound §D4 relies

+on, just re-established mid-replay instead of assumed for it.

+**It writes nothing**, deliberately, and this is the one place that needs saying

+twice: `Store.replay` is the read path (Text, HTML and JSON all reach it), and a

+read that persisted a row would be a materially different change — the one place

+today's design allows a snapshot to originate is a request that has already

+proved the instance awake (D3), which a plain read has not. The persisted

+snapshot this document is still owed arrives the ordinary way, on its next

+`ApplyUpdate`: crossing the cap here means the gap already exceeds

+`snapshotCadence` many times over, so that write's own cadence check (D3, D4)

+passes trivially and it snapshots at the current ABI — which is exactly what an

+ABI bump needs repaired, and the reason this stays self-healing rather than

+becoming a second, competing writer to reason about.

+**It logs at Warn, once per re-seed** — not Error, and not silent either, and

+both halves of that were argued from evidence rather than assumed. Not Error,

+because nothing is wrong with the document: it is complete and correct, only

+slower to read, exactly the distinction D4 draws between a cliff and a cost. Not

+silent, which is what this was at first, on the reasoning that the document's

+next write repairs it — reasoning that assumes a write arrives. Reads write

+nothing, so a long document that is finished and only ever read again, which is

+most of what an ABI bump invalidates at once, would re-seed on every single read,

+forever, with degraded latency as the only trace. One line per read is the price

+of that not being invisible to an operator.

## 5. What does not change

The wire format (a snapshot is an ordinary Yjs update); the browser bundle; the

@@ -301,6 +358,7 @@ which keeps every row it has.

| D5's verification fails | Snapshot not written, log untouched, logged at Error |

| Snapshot write fails after the update committed | Update stands, no snapshot, next Nth write tries again |

| Core call traps producing the snapshot | Same as above; the update is already durable |

+| A full replay (no usable snapshot) crosses `core.MaxRetainedUpdates` mid-tail | Re-seeds a fresh handle from its own state, in memory, and continues; writes nothing, logged at Warn per re-seed (D8) |

Every row of that table ends in "correct, possibly slower", which is the bar this

design sets for itself.

@@ -380,3 +438,12 @@ keeps this from making storage worse than the problem it solves (§2).

The review also confirmed what §3 rests on: against an empty state vector,

`encode_state_as_update_v1` is merge-equivalent to the log and not merely

render-equivalent, so a snapshot is safe to build future concurrent merges on.

+**§D8 was added after this review**, not folded into it: a finding from a later

+task review of the implementation, that a full replay falling back to no usable

+snapshot on a log already past `core.MaxRetainedUpdates` met `ErrUpdateLogFull`

+inside `Store.replay` itself with no way out, which is the ABI-bump case this

+same design's D2 and §9's first risk both name as the thing that will actually

+happen. The fix — re-seeding a fresh handle from the replay's own state, in

+memory, mid-tail — was implemented and tested before this section existed; §D8

+is this spec catching up to what the code already does, not a proposal.

diff --git a/internal/docs/cursor_test.go b/internal/docs/cursor_test.go

index 4d44b1b..0dbcce3 100644

--- a/internal/docs/cursor_test.go

+++ b/internal/docs/cursor_test.go

@@ -228,11 +228,22 @@ func pastCadence(t *testing.T, app *App, title string) *Document {

// A client far behind the snapshot receives the snapshot plus the tail

// rather than the whole log, and the header still names the highest seq

// the bytes actually reach.

+//

+// Byte-exact, mirroring TestRawParameterReturnsOnlyLogRows's shape for

+// the raw path: a size and Docs-Seq comparison alone cannot tell a

+// correct tail (`seq > snapshot.Seq`) from one that drops exactly the

+// first row after the snapshot (`seq > snapshot.Seq + 1`) — every tail

+// row here is validUpdate's idempotent duplicate bytes, so dropping one

+// changes neither the response's length nor its Docs-Seq. Comparing the

+// body against the snapshot's own State plus the concatenation of every

+// doc_updates row with seq > snapshot.Seq is what a dropped row cannot

+// survive.

func TestUpdatesGetServesSnapshotThenTail(t *testing.T) {

app, err := NewForTest(t)

if err != nil {

t.Fatalf("NewForTest: %v", err)

}

+ ctx := context.Background()

doc := pastCadence(t, app, "Snapshot then tail")

snap := serve(app, app.TestRequest(t, "owner@example.com", http.MethodGet,

@@ -250,6 +261,29 @@ func TestUpdatesGetServesSnapshotThenTail(t *testing.T) {

if got := docsSeq(t, snap.Result()); got != int64(snapshotCadence+3) {

t.Errorf("Docs-Seq = %d, want %d", got, snapshotCadence+3)

}

+ snapshotRow, err := app.Store().newestSnapshot(ctx, doc.ID)

+ if err != nil {

+ t.Fatalf("newestSnapshot: %v", err)

+ }

+ if snapshotRow == nil {

+ t.Fatal("no snapshot exists past the cadence, so this test is not exercising snapshot-then-tail")

+ }

+ var rows []DocUpdate

+ if err := app.Store().db.WithContext(ctx).

+ Where("doc_id = ? AND seq > ?", doc.ID, snapshotRow.Seq).

+ Order("seq ASC").Find(&rows).Error; err != nil {

+ t.Fatalf("load tail rows: %v", err)

+ }

+ want := append([]byte(nil), snapshotRow.State...)

+ for _, r := range rows {

+ want = append(want, r.UpdateBin...)

+ }

+ if !bytes.Equal(snap.Body.Bytes(), want) {

+ t.Error("response body is not exactly the snapshot's State followed by the tail rows after snapshot.Seq")

+ }

}

// raw=1 is the escape hatch a browser needs when a snapshot will not

diff --git a/internal/docs/snapshots_test.go b/internal/docs/snapshots_test.go

index be2164e..f7b4284 100644

--- a/internal/docs/snapshots_test.go

+++ b/internal/docs/snapshots_test.go

@@ -112,7 +112,8 @@ func TestNewestSnapshotIsNilWhenThereAreNone(t *testing.T) {

// exactly what a stale snapshot makes look right.

// Property-shaped rather than one fixture at one cadence, as §8 asks:

-// two documents, one snapshot apart and two snapshots apart, and each

+// three documents — one snapshot apart, two snapshots apart, and one

+// whose distinct row sits exactly at the tail's first seq — and each

// carries one update whose content nothing else in its log holds.

// That last part is what gives the comparison teeth, and leaving it out

@@ -120,9 +121,16 @@ func TestNewestSnapshotIsNilWhenThereAreNone(t *testing.T) {

// simple.bin bytes, which Yjs merges idempotently, so a log of nothing

// but those reconstructs the whole document from the tail alone: an

// empty snapshot, or one stranding any row at all, would render

-// identically and this test would pass. The distinct row sits in the

-// range only the snapshot can carry — everything at or below its seq —

-// so if the snapshot does not hold it, nothing does.

+// identically and this test would pass. The first two cases put the

+// distinct row in the range only the snapshot can carry — everything at

+// or below its seq — so if the snapshot does not hold it, nothing does.

+//

+// The third case inverts that on purpose: its distinct row sits one

+// past the snapshot, at the tail's first seq, so it is the tail's

+// content that must carry it — a snapshot dropping the first tail row

+// would render identically to a full replay here on every existing

+// case, which is exactly the shape of tail query that `seq >

+// snapshot.Seq + 1` would produce without a single test noticing.

func TestSnapshotThenTailRendersIdenticallyToAFullReplay(t *testing.T) {

// distinctAt is a 0-based index into the run of updates; the row it

// lands in gets seq distinctAt+1.

@@ -130,15 +138,25 @@ func TestSnapshotThenTailRendersIdenticallyToAFullReplay(t *testing.T) {

name string

updates int

distinctAt int

+ // inTail is true when the distinct row must land in the tail

+ // (after the snapshot) rather than inside the snapshot itself

+ // — the vacuity guard below is inverted for such a case.

+ inTail bool

}{

// One snapshot at seq snapshotCadence and a five-row tail. The

// distinct row is deep inside the snapshot's range.

- {"one cadence, the distinct row deep inside the snapshot", snapshotCadence + 5, 3},

+ {"one cadence, the distinct row deep inside the snapshot", snapshotCadence + 5, 3, false},

// Two cadences: the snapshot at seq snapshotCadence is

// superseded by one at 2*snapshotCadence, and the distinct row

// is the very last row that surviving snapshot must contain —

// the boundary itself.

- {"two cadences, the distinct row on the boundary", 2*snapshotCadence + 3, 2*snapshotCadence - 1},

+ {"two cadences, the distinct row on the boundary", 2*snapshotCadence + 3, 2*snapshotCadence - 1, false},

+ // One snapshot at seq snapshotCadence, and the distinct row is

+ // the very first row of the tail (seq snapshotCadence+1) rather

+ // than anything the snapshot covers. Only the tail query can

+ // carry this content, which is what makes it the case that

+ // catches a tail query narrowed to `seq > snapshot.Seq + 1`.

+ {"one cadence, the distinct row is the first tail row", snapshotCadence + 3, snapshotCadence, true},

}

for _, tc := range cases {

@@ -173,7 +191,12 @@ func TestSnapshotThenTailRendersIdenticallyToAFullReplay(t *testing.T) {

if snap == nil {

t.Fatal("no snapshot exists past the cadence, so nothing below exercises snapshot-then-tail")

}

- if want := int64(tc.distinctAt + 1); snap.Seq < want {

+ want := int64(tc.distinctAt + 1)

+ if tc.inTail {

+ if snap.Seq >= want {

+ t.Fatalf("newest snapshot is at seq %d, at or past the distinct row at seq %d — the snapshot would carry that content on its own and this test would prove nothing", snap.Seq, want)

+ }

+ } else if snap.Seq < want {

t.Fatalf("newest snapshot is at seq %d, below the distinct row at seq %d — the tail would carry that content on its own and this test would prove nothing", snap.Seq, want)

}