amadan

oficina / sheets Public

test: reproduce regressing versions after clearing cells and reopening

Paul Campbell pushed by paul@keymail.dev 32ba4498749931ce054b2f0eab92db7d0a4dfa6a parent 37c8055
3 files changed, +271 −0
  • docs/superpowers/specs/2026-09-07-durable-tab-versions.md +51 −0
  • internal/sheets/store/handle_version_migration_test.go +110 −0
  • internal/sheets/store/handle_version_watermark_test.go +110 −0

CI failed — run details

Intent

full ledger: fix/durable-tab-versions

diff --git a/docs/superpowers/specs/2026-09-07-durable-tab-versions.md b/docs/superpowers/specs/2026-09-07-durable-tab-versions.md
new file mode 100644
index 0000000..b8399c6
--- /dev/null
+++ b/docs/superpowers/specs/2026-09-07-durable-tab-versions.md
@@ -0,0 +1,51 @@
+# Durable tab version watermark
+
+Status: proposed prerequisite; independent Fable review pending. No implementation is approved yet. Branch fix/durable-tab-versions starts at atomic batch candidate 37c8055; Amadan discussion 19/task 3. Atomic production deployment is blocked until this defect is fixed and reviewed.
+
+## Reproduction
+
+A 500-cell paste returns version 503. Release/reopen the tab handle, clear those 500 cells, and the clear returns 1004. Release/reopen again and write one new cell: version 4. The browser correctly refuses version 4 as older than 1004, so the newly committed edit is absent from the display. The same defect affects a single-cell edit after clearing the last surviving cell.
+
+buildHandle derives maxVersion only from current cells' InputVersion and CacheVersion. Deleting the newest cell rows destroys that ordering evidence. The in-memory handle counter survives only while some caller retains the handle; SSE subscribers deliberately do not retain it. An event count or timestamp is not a reliable substitute: neither is the previously issued version.
+
+## Invariant and schema
+
+For each tab, every committed cell input or persisted calculation stamp is no greater than a durable tab watermark. Opening a handle starts above both that watermark and all surviving input/cache versions. Clearing cells, recovering stale caches, and releasing every handle must never make a future edit's version go backwards.
+
+Add Tab.ContentVersion as a non-null integer with default zero through an additive generated migration. It is metadata of the same class as the existing cell InputVersion/CacheVersion, not authored content. Existing tabs start at zero and retain all content, names, history, formats, rows and columns. At open, the maximum of watermark and existing cell versions preserves older populated databases without a destructive migration/backfill. Already-empty pre-upgrade databases have no reconstructable historic version; deployment must reconnect/refetch browsers, and all future edits gain the durable invariant.
+
+SQLite integers are signed 64-bit. Refuse an exhausted/corrupt watermark before allocating a wrapped stamp; do not silently return to zero. No user-facing strings are added; existing fixed storage failure responses remain.
+
+## Mutation paths
+
+| Path | Required handling |
+| --- | --- |
+| SetReport -> setContent | Advance watermark to at least v in the same first transaction as sealed input and AppendInput |
+| SetReport -> setBlank | Advance watermark to at least v in the same transaction as deletion, dirty dependent markers and AppendInput |
+| persistTouched | Advance watermark to at least v in the same transaction as sealed cache updates and AppendRecalc |
+| recoverStale | Uses persistTouched on the transaction-scoped handle; recovery version is also preserved even if its cells are later cleared |
+| SetBatch | Every nested update uses staged.db=tx; outer rollback undoes watermark changes too; successful live swap retains staged.nextVersion |
+| Table edit, XLSX import, restore, undo | Already call Set or SetReport; inherit the same durable handling without parallel logic |
+| SetFormat, widths/heights, saved views | Existing behavior does not allocate content versions; retain it. This fix does not pretend to solve concurrent ordering of distinct equal-version formatting responses |
+| EnsureRow/EnsureCol | Allocate stable axes only, not a content change; no new version |
+| Row/column insert/delete/move | Not implemented in this repository; future structural operations must allocate and persist a tab stamp in the same mutation transaction. Do not delete a watermark with cell/axis rows |
+| Tab add/rename/reorder | Future metadata events have separate ordering semantics; do not masquerade as cell history |
+
+Implement one small transaction-scoped helper for monotonic advancement using an allowlisted update of content_version on Tab by ID. It never lowers a stored value. Both input write paths and persistTouched call it. No outer db calls occur while a transaction holds the one writer connection. A missing tab row must fail rather than silently discarding the watermark.
+
+Read the tab watermark while building a handle, before allocating nextVersion. A failed input/recalculation transaction must not commit its watermark; the existing dirty-marker behavior after a committed input transaction remains valid. A failed atomic batch must leave both the live handle and persisted watermark unchanged.
+
+## Regression coverage
+
+1. Real release/reopen sequence for paste500, clear500, new edit; require strictly increasing report versions and the correct final value. Existing candidate fails 503 ->1004 ->4.
+2. Single-cell last-cell clear followed by new edit after reopen; require the new stamp to exceed the clear stamp.
+3. Blank crash seam after the first transaction: release/reopen and require the next edit to exceed the committed blank's allocated stamp.
+4. Overlapping formula/dependent recomputation, removal of the newest cached cells, and reopen preserve ordering and correct values.
+5. Existing failure injection into a late SetBatch event must leave Tab.ContentVersion unchanged alongside inputs, history and axes. A subsequent successful edit must remain usable.
+6. Migrate a database built with BootSchema through the pre-watermark migration set; seed encrypted names/content and history, then apply the new migration. Verify byte-for-byte ciphertext retention, default watermark, schema agreement and monotonic first post-upgrade edits.
+7. A high watermark in an otherwise empty tab survives Open; the next write uses a higher stamp. A lower attempted advance cannot reduce it. Exhaustion refuses without a write.
+8. Run existing crash/recovery, plain SQLite/WAL leakage and race tests. The watermark contains no plaintext values and adds no logs.
+
+## Review and deploy
+
+Keep this prerequisite independently attributable in discussion 19. Fable reviews the design before implementation and the final code before merge. Integrate with atomic batch only after passing full make test, migration agreement, exact-head CI and deployed canary browser regression (paste, clear, new paste, reload, second observer). A migration-only deploy must not erase or replace any user data. Root coordinates production promotion and /api/version proof.
diff --git a/internal/sheets/store/handle_version_migration_test.go b/internal/sheets/store/handle_version_migration_test.go
new file mode 100644
index 0000000..24dacbb
--- /dev/null
+++ b/internal/sheets/store/handle_version_migration_test.go
@@ -0,0 +1,110 @@
+package store_test
+
+import (
+ "bytes"
+ "context"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "amadan.net/oficina/sheets/internal/sheets"
+ "amadan.net/oficina/sheets/internal/sheets/calc"
+ "amadan.net/oficina/sheets/internal/sheets/seal"
+ "amadan.net/oficina/sheets/internal/sheets/store"
+ rdb "github.com/carlosframework/rastrillo/db"
+ "github.com/carlosframework/rastrillo/migrate"
+)
+
+func TestVersionWatermarkMigrationPreservesExistingContent(t *testing.T) {
+ d, err := rdb.Open(filepath.Join(t.TempDir(), "old.db"), nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer d.Close()
+ old := &migrate.Set{}
+ for _, m := range sheets.BootSchema.All() {
+ if !strings.Contains(m.SQL, "content_version") {
+ old.Add(m)
+ }
+ }
+ if _, err := migrate.Apply(context.Background(), d, old); err != nil {
+ t.Fatal(err)
+ }
+ s, err := seal.New(make32())
+ if err != nil {
+ t.Fatal(err)
+ }
+ name, err := s.Seal("tab.name", []byte("Original tab"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ raw, err := s.Seal("cell.input", []byte("5"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ cache, err := s.Seal("cell.cached", store.EncodeValueForTest(calc.Value{Kind: calc.KindNum, Num: 5}))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, q := range []struct {
+ sql string
+ args []any
+ }{
+ {"INSERT INTO tabs (id,sheet_id,name_sealed,position) VALUES (?,?,?,?)", []any{tabID, 1, name, 0}},
+ {"INSERT INTO rows (id,tab_id,position) VALUES (?,?,?)", []any{rowA, tabID, 0}},
+ {"INSERT INTO cols (id,tab_id,position) VALUES (?,?,?)", []any{colA, tabID, 0}},
+ {"INSERT INTO cells (tab_id,row_id,col_id,input_sealed,cached_sealed,input_version,cache_version) VALUES (?,?,?,?,?,?,?)", []any{tabID, rowA, colA, raw, cache, 700, 700}},
+ } {
+ if err := d.G.Exec(q.sql, q.args...).Error; err != nil {
+ t.Fatal(err)
+ }
+ }
+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {
+ t.Fatal(err)
+ }
+ var mark uint64
+ if err := d.G.Raw("SELECT content_version FROM tabs WHERE id = ?", tabID).Scan(&mark).Error; err != nil {
+ t.Fatal(err)
+ }
+ if mark != 0 {
+ t.Fatalf("new watermark default=%d", mark)
+ }
+ var cell store.Cell
+ if err := d.G.First(&cell).Error; err != nil {
+ t.Fatal(err)
+ }
+ if !bytes.Equal(cell.InputSealed, raw) || !bytes.Equal(cell.CachedSealed, cache) {
+ t.Fatal("migration altered encrypted cell")
+ }
+ var tab store.Tab
+ if err := d.G.First(&tab, tabID).Error; err != nil {
+ t.Fatal(err)
+ }
+ if !bytes.Equal(tab.NameSealed, name) {
+ t.Fatal("migration altered encrypted name")
+ }
+ h, err := store.Open(d.G, s, tabID)
+ if err != nil {
+ t.Fatal(err)
+ }
+ clear, err := h.SetReport(rowA, colA, "", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ h.Release()
+ if clear.Version <= 700 {
+ t.Fatalf("ignored old cell stamp: %d", clear.Version)
+ }
+ h, err = store.Open(d.G, s, tabID)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer h.Release()
+ next, err := h.SetReport(rowA, colA, "6", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if next.Version <= clear.Version {
+ t.Fatalf("post-upgrade versions %d -> %d", clear.Version, next.Version)
+ }
+}
diff --git a/internal/sheets/store/handle_version_watermark_test.go b/internal/sheets/store/handle_version_watermark_test.go
new file mode 100644
index 0000000..54283be
--- /dev/null
+++ b/internal/sheets/store/handle_version_watermark_test.go
@@ -0,0 +1,110 @@
+package store_test
+
+import (
+ "amadan.net/oficina/sheets/internal/sheets/store"
+ "errors"
+ "testing"
+)
+
+func TestBatchVersionsSurviveClearingAndReopen(t *testing.T) {
+ db, s, id := seeded(t)
+ h, e := store.Open(db, s, id)
+ if e != nil {
+ t.Fatal(e)
+ }
+ jobs := make([]store.CellWrite, 500)
+ for i := range jobs {
+ jobs[i] = store.CellWrite{Row: i, Col: 1, Raw: "1"}
+ }
+ a, e := h.SetBatch(jobs, actor)
+ if e != nil {
+ t.Fatal(e)
+ }
+ h.Release()
+ h, e = store.Open(db, s, id)
+ if e != nil {
+ t.Fatal(e)
+ }
+ for i := range jobs {
+ jobs[i].Raw = ""
+ }
+ b, e := h.SetBatch(jobs, actor)
+ if e != nil {
+ t.Fatal(e)
+ }
+ h.Release()
+ h, e = store.Open(db, s, id)
+ if e != nil {
+ t.Fatal(e)
+ }
+ defer h.Release()
+ c, e := h.SetBatch([]store.CellWrite{{Row: 0, Col: 1, Raw: "2"}}, actor)
+ if e != nil {
+ t.Fatal(e)
+ }
+ if c.Version <= b.Version {
+ t.Fatalf("version regression: paste=%d clear=%d next edit=%d; browser discards latest edit", a.Version, b.Version, c.Version)
+ }
+}
+
+func TestSingleCellVersionSurvivesLastCellClear(t *testing.T) {
+ db, s, id := seeded(t)
+ h, err := store.Open(db, s, id)
+ if err != nil {
+ t.Fatal(err)
+ }
+ first, err := h.SetReport(rowA, colA, "8", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ h.Release()
+ h, err = store.Open(db, s, id)
+ if err != nil {
+ t.Fatal(err)
+ }
+ cleared, err := h.SetReport(rowA, colA, "", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ h.Release()
+ h, err = store.Open(db, s, id)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer h.Release()
+ next, err := h.SetReport(rowA, colA, "9", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if cleared.Version <= first.Version || next.Version <= cleared.Version {
+ t.Fatalf("versions regressed: first=%d cleared=%d next=%d", first.Version, cleared.Version, next.Version)
+ }
+}
+
+func TestBlankCrashRetainsCommittedVersion(t *testing.T) {
+ db, s, id := seeded(t)
+ h, err := store.Open(db, s, id)
+ if err != nil {
+ t.Fatal(err)
+ }
+ allocated := h.Version()
+ reset := store.InterruptBlankAfterFirstTransaction(func() error { return errors.New("simulated crash") })
+ _, err = h.SetReport(rowA, colA, "", actor)
+ reset()
+ h.Release()
+ if err == nil {
+ t.Fatal("crash injection failed")
+ }
+ h, err = store.Open(db, s, id)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer h.Release()
+ next, err := h.SetReport(rowA, colA, "9", actor)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if next.Version <= allocated {
+ t.Fatalf("committed blank stamp=%d next=%d", allocated, next.Version)
+ }
+}