amadan
provision: SetRecordMoveAt always writes under the record sequence
On a record no writer had sequenced, a bound write under a nil judge left
record_seq at 0, so Expect{inc, 0} matched every later version and the
binding reduced to the incarnation. SetRecordMoveAt now wraps its context
with WithRecordSequence: the first bound write introduces record_seq (1)
and a write on the stale observation is ErrCASConflict. It has no callers
yet, so no other path's nil-judge behaviour changes. Census: the
SetRecordMoveAt digest and the internal/provision envelope re-pinned
(PROPOSED-self-serve-moves-2).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+40
−4
internal/provision/offline_queue_test.go+28 −0internal/provision/provision.go+6 −0internal/provision/self_serve_moves2_census_test.go+5 −3internal/provision/testdata/plan14-root-reads-self-serve-moves-2.tsv+1 −1
diff --git a/internal/provision/offline_queue_test.go b/internal/provision/offline_queue_test.go| index ce89be607..936751573 100644 |
| --- a/internal/provision/offline_queue_test.go |
| +++ b/internal/provision/offline_queue_test.go |
| @@ -691,6 +691,34 @@ func TestSetRecordMoveAtIsBoundToTheObservedRecord(t *testing.T) { |
| } |
| }) |
| } |
| + |
| + // A record no writer has sequenced yet, under no judge: the bound write |
| + // introduces record_seq itself, so the observation it was decided on |
| + // (record_seq 0) never matches a later version. |
| + t.Run("never sequenced, no judge", func(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + rec := offlineRecord(t, s) |
| + obs := offlineGet(t, s, rec) |
| + if obs.RecordSeq != 0 { |
| + t.Fatalf("fixture: record_seq %d, want a never-sequenced record", obs.RecordSeq) |
| + } |
| + stale := Expect{Incarnation: obs.Incarnation, Seq: 0} |
| + got, err := SetRecordMoveAt(ctx, s, rec.Account, rec.App, rec.Host, stale, OfflineStart(offID, "", "uk", "o", now, "")) |
| + if err != nil || got.RecordSeq != 1 { |
| + t.Fatalf("first bound write: %v record_seq %d, want 1", err, got.RecordSeq) |
| + } |
| + if stored := offlineGet(t, s, rec); stored.RecordSeq != 1 { |
| + t.Fatalf("stored record_seq %d, want 1", stored.RecordSeq) |
| + } |
| + raw, etag := rawRecord(t, s, rec) |
| + if _, err := SetRecordMoveAt(ctx, s, rec.Account, rec.App, rec.Host, stale, OfflineSeal(offID, offSealer, offBoxes, Receipt{})); !errors.Is(err, release.ErrCASConflict) { |
| + t.Fatalf("a second write on the stale observation: %v, want ErrCASConflict", err) |
| + } |
| + if raw2, etag2 := rawRecord(t, s, rec); !bytes.Equal(raw, raw2) || etag != etag2 { |
| + t.Fatal("the refused stale write changed the record") |
| + } |
| + }) |
| } |
| // The self-serve fields are additive: a journal without them round-trips |
diff --git a/internal/provision/provision.go b/internal/provision/provision.go| index b384a7ac8..17a6db058 100644 |
| --- a/internal/provision/provision.go |
| +++ b/internal/provision/provision.go |
| @@ -1445,7 +1445,13 @@ type Expect struct{ Incarnation, Seq uint64 } |
| // only ever applies to the record the caller decided on (the self-serve |
| // job's persisted request, an operator door's observation). A mutation |
| // returning errRecordUnchanged writes nothing, as under SetRecordMove. |
| +// |
| +// The write always carries the record sequence (WithRecordSequence): on a |
| +// record no writer has sequenced yet the first bound write introduces |
| +// record_seq, so an observation at record_seq 0 matches that version only |
| +// and the binding never reduces to the incarnation alone. |
| func SetRecordMoveAt(ctx context.Context, s release.Store, acct, app, host string, expect Expect, mut func(*Record) error) (*Record, error) { |
| + ctx = WithRecordSequence(ctx) |
| return updateRecordSeeding(ctx, s, acct, app, host, func(rec *Record) error { |
| if err := mut(rec); err != nil { |
| return err |
diff --git a/internal/provision/self_serve_moves2_census_test.go b/internal/provision/self_serve_moves2_census_test.go| index 1fb8df263..8d2234ba6 100644 |
| --- a/internal/provision/self_serve_moves2_census_test.go |
| +++ b/internal/provision/self_serve_moves2_census_test.go |
| @@ -21,8 +21,10 @@ package provision |
| // - provision.go:SetRecordMoveAt is added, a record-writer |
| // (key:SetRecordMoveAt): SetRecordMove bound to an observed incarnation |
| // and record_seq, through updateRecordSeeding and so WriteRecord, one |
| -// attempt. setrecordmoveat joins censusKeyExact so its callers stay |
| -// typed writers in the census. |
| +// attempt, always under WithRecordSequence (fix round 1), so a |
| +// never-sequenced record gains record_seq at the first bound write. |
| +// setrecordmoveat joins censusKeyExact so its callers stay typed |
| +// writers in the census. |
| // - provision.go:updateRecordSeeding is re-pinned: it gains the expect |
| // parameter (nil from every caller but SetRecordMoveAt), which refuses |
| // a read of another incarnation or record_seq with ErrCASConflict before |
| @@ -49,7 +51,7 @@ func plan14SelfServeMoves2WriteCensusInventory() []censusEntry { |
| {"internal/provision/provision.go", "updateRecord", "record-writer", "key:updateRecord", "c02b077b70a5d064bc4a602974fd11b8729365b22353aae31e04978d94f7e122"}, |
| {"internal/provision/provision.go", "updateRecordSeeding", "record-writer", "key:RecordKey,key:WriteRecord", "6c67de6c105824040c297fe23838b9c4de80bdbfce065b1ce80ef46f4f422be0"}, |
| {"internal/provision/seed.go", "SetRecordSeed", "record-writer", "key:SetRecordSeed", "b9b7e479f5217f26356e31e1568e1d1aa0f31c8d0f719633e7f2b15f25119d60"}, |
| - {"internal/provision/provision.go", "SetRecordMoveAt", "record-writer", "key:SetRecordMoveAt", "f44d62d5180d20e67d5459e7578f23fd5ac852d3f8097d14921cd63e33dc7140"}, |
| + {"internal/provision/provision.go", "SetRecordMoveAt", "record-writer", "key:SetRecordMoveAt", "e818d6acd4429185c5fc11db4100a73b2d83409b9f6f32435e88aac7f9012f41"}, |
| {"internal/moves/offline.go", "OpenRecordKey", "key-site", "key:OpenRecordKey", "ebb4f0dcd050b5d183b642829423ba71740bf6d86427af2cba64475d54276be9"}, |
| } |
| entries := plan14SelfServeMoves1WriteCensusInventory() |
diff --git a/internal/provision/testdata/plan14-root-reads-self-serve-moves-2.tsv b/internal/provision/testdata/plan14-root-reads-self-serve-moves-2.tsv| index 0024cdb6d..4a5d1f8fc 100644 |
| --- a/internal/provision/testdata/plan14-root-reads-self-serve-moves-2.tsv |
| +++ b/internal/provision/testdata/plan14-root-reads-self-serve-moves-2.tsv |
| @@ -23,4 +23,4 @@ internal/console/templates/settings.html external legacy accepted-legacy accepte |
| internal/provision/offline_move.go offlineBegin legacy accepted-legacy accepted-legacy;rev3=PROPOSED-self-serve-moves-2;basis=root-input (J4, as OfflineStart); the console (cmd/console serve under console.service, user carlos-console): self-serve moves 1 Task 7's body (OfflineStart's, the state a parameter; a direct start advances move_seq and stamps move.seq, refusing ErrLifetimeExhausted at MaxInt64 before any write), and it now also creates the self-serve move in offline-queued for OfflineQueue (queued_at, staged_at and precheck_at at the caller's stamp, move.seq set to the record's move_seq, no advance) and records return_of; a stage and a queue hold nothing; its one site is still rec.ActiveRun.Open(), a method on the record's run state (RunState.Open), not a file read; self-serve moves 2 Task 2 file:Open@208 0a9b432ddf2f39a977c0651e377258b35c1d05635e9979583eeac6253d6c0ec8 |
| @coverage internal/console 29ae289dc256408f6debca9150987ba5ce7a2e80a0bbf5fc3064ead950d16bd1 |
| @coverage internal/moves 51465341e311dc007d75ed4f68474fe4cec05d51c6ec07ee5c64653936faa410 |
| -@coverage internal/provision 0d327a4972f0a10304fcda5e5ba1ee1612f6e49801554290d88021d75d6d912e |
| +@coverage internal/provision 6e75b92e716fd61f0f70e3071e36dc2be240fb419b70845b3429e8c9990a018a |