amadan
console: the host-claim move door checks the roster before it registers, and a later refusal no longer blames the roster (E-switch fix round 1)
Review minor (apiinstancemove.go:691): BeginClaimMove answers ErrIdentity
for roster failures and for record races (incarnation, a move already
journaled) alike. The door now checks its own roster observation (the one
E2 read) against BeginClaimMove's roster precondition before RegisterMove:
both boxes active and enrolled, the source in the record's region, the
target in to, no termination fence on the target. A roster that fails is
MV-N177 and writes no registration. An ErrIdentity or ErrRoster from
BeginClaimMove itself is now the neutral MV-N178 ("the instance or its
boxes changed under this request. read its state and try again").
Review important (the compensation leaves the move-start effect uncertain)
is NOT fixed here: hostops exposes no API that resolves an uncertain
record effect. TestMoveDoorHostClaimRetryAfterAFailedRecordStart records
the requirement (fail-once Put, retry succeeds, registration drains) and
is skipped as a KNOWN GAP until the claim code provides that API.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+152
−14
approved_moves_copy_test.go+1 −1docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+2 −1internal/console/apiinstancemove.go+40 −7internal/console/apiinstancemove_claim_test.go+109 −5
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index 6e0229d6a..6756eef9e 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -69,7 +69,7 @@ var movesCopyIDs = []string{ |
| "MV-N157", "MV-N158", "MV-N159", "MV-N160", "MV-N161", "MV-N162", |
| "MV-N163", "MV-N164", "MV-N165", "MV-N166", "MV-N167", "MV-N168", |
| "MV-N169", "MV-N170", "MV-N171", "MV-N172", "MV-N173", "MV-N174", |
| - "MV-N175", "MV-N176", "MV-N177", |
| + "MV-N175", "MV-N176", "MV-N177", "MV-N178", |
| } |
| // copyRow is one row of the record. The File cell is the file the sentence is |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md| index 6b56cb561..f35c797c2 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| @@ -210,7 +210,7 @@ Columns: id, file, text, status. |
| ## Plan 2 |
| -(Task 10 rows, then task E-switch's MV-N173 to MV-N177; the rest appended by Plan 2 Task 11) |
| +(Task 10 rows, then task E-switch's MV-N173 to MV-N178; the rest appended by Plan 2 Task 11) |
| | id | file | text | status | |
| |---|---|---|---| |
| @@ -224,6 +224,7 @@ Columns: id, file, text, status. |
| | MV-N175 | internal/console/apiinstancemove.go | another operation holds this app. try again when it finishes | pending | |
| | MV-N176 | internal/console/apiinstancemove.go | could not register the move. try again | pending | |
| | MV-N177 | internal/console/apiinstancemove.go | the host roster does not allow this move. the source and target must be active boxes in <from> and <to> | pending | |
| +| MV-N178 | internal/console/apiinstancemove.go | the instance or its boxes changed under this request. read its state and try again | pending | |
| ## Plan 3 |
diff --git a/internal/console/apiinstancemove.go b/internal/console/apiinstancemove.go| index 40ba5d3c2..b1c5b21c9 100644 |
| --- a/internal/console/apiinstancemove.go |
| +++ b/internal/console/apiinstancemove.go |
| @@ -614,7 +614,7 @@ func (s *Server) handleAPIInstanceMoveRequest(w http.ResponseWriter, r *http.Req |
| } |
| exec := obs.Claim.Op.Exec |
| if s.cfg.HostClaim { |
| - s.startClaimModeMove(w, r, acct, app, host, obs, ms, sel.Target, from) |
| + s.startClaimModeMove(w, r, acct, app, host, obs, ms, sel.Target, from, rec.Region, dir, roster) |
| return |
| } |
| moving, err := hostclaim.Update(ctx, s.cfg.Releases, obs, hostclaim.BeginMove(id, addr, exec, sel.Target, to, now)) |
| @@ -656,14 +656,36 @@ func moveClaimCASFailed(w http.ResponseWriter, acct *Account, app, host, to stri |
| // and StartClaimMoveRecord journals the move on the record as one effect |
| // of that registration. The door never builds an authority of its own. |
| // |
| -// Every refusal before the claim CAS writes nothing but the registration, |
| -// which the admission sweep settles once its TTL passes. A failed record |
| -// start after the CAS is settled as on the plain path, with the release |
| -// written under the registration's lifecycle grant. |
| -func (s *Server) startClaimModeMove(w http.ResponseWriter, r *http.Request, acct *Account, app, host string, obs hostclaim.Observed, ms provision.MoveStart, target, from string) { |
| +// Before anything is registered, the door's own roster observation (the |
| +// one E2 read) must place the move as BeginClaimMove will require: both |
| +// boxes active and enrolled, the source in the record's region, the target |
| +// in to, and no termination fence on the target. A roster that does not is |
| +// MV-N177. BeginClaimMove rereads the roster and the record; if it still |
| +// refuses (ErrIdentity, ErrRoster), something changed under the request |
| +// and the door does not guess what (MV-N178). |
| +// |
| +// Every refusal after RegisterMove and before the claim CAS writes nothing |
| +// but the registration, which the admission sweep settles once its TTL |
| +// passes. A failed record start after the CAS is settled as on the plain |
| +// path, with the release written under the registration's lifecycle grant. |
| +func (s *Server) startClaimModeMove(w http.ResponseWriter, r *http.Request, acct *Account, app, host string, obs hostclaim.Observed, ms provision.MoveStart, target, from, recRegion string, dir hostactors.Directory, roster hostactors.Observed) { |
| ctx := r.Context() |
| a := s.cfg.ClaimAuthority |
| to := ms.To |
| + if !claimMoveRosterPlaces(roster.Roster, obs.Claim.Op.Exec, target, recRegion, to) { |
| + jsonError(w, http.StatusConflict, "the host roster does not allow this move. the source and target must be active boxes in "+from+" and "+to) |
| + return |
| + } |
| + fenced, err := dir.TerminationFence(ctx, roster, target, time.Now()) |
| + if err != nil { |
| + log.Printf("console: api instance move %s/%s/%s: termination fence of %s: %v", acct.Sqid, app, host, target, err) |
| + jsonError(w, http.StatusServiceUnavailable, "could not read the host roster. try again") |
| + return |
| + } |
| + if fenced { |
| + jsonError(w, http.StatusConflict, "the host roster does not allow this move. the source and target must be active boxes in "+from+" and "+to) |
| + return |
| + } |
| appID, err := s.store.ClaimAppID(acct.ID, app) |
| if errors.Is(err, sql.ErrNoRows) || errors.Is(err, hostops.ErrIdentity) { |
| jsonError(w, http.StatusConflict, "this app has no host-claim admission yet. try again shortly") |
| @@ -692,7 +714,7 @@ func (s *Server) startClaimModeMove(w http.ResponseWriter, r *http.Request, acct |
| switch { |
| case errors.Is(err, hostops.ErrIdentity), errors.Is(err, hostactors.ErrRoster): |
| log.Printf("console: api instance move %s/%s/%s: begin move %s: %v", acct.Sqid, app, host, ms.ID, err) |
| - jsonError(w, http.StatusConflict, "the host roster does not allow this move. the source and target must be active boxes in "+from+" and "+to) |
| + jsonError(w, http.StatusConflict, "the instance or its boxes changed under this request. read its state and try again") |
| case errors.Is(err, hostops.ErrPending): |
| jsonError(w, http.StatusConflict, "another operation holds this app. try again when it finishes") |
| default: |
| @@ -884,6 +906,17 @@ func phase1Conflict(err error) string { |
| return "this move's state does not allow that. read its state and try again" |
| } |
| +// claimMoveRosterPlaces is BeginClaimMove's roster precondition, read from |
| +// the door's own observation: both boxes have active, enrolled entries |
| +// under their box references, they differ, the source is in the record's |
| +// region and the target in to, which is not the source's region. |
| +func claimMoveRosterPlaces(r hostactors.Roster, source, target, recRegion, to string) bool { |
| + src, sourceOK := r.Boxes[source] |
| + dst, targetOK := r.Boxes[target] |
| + return sourceOK && targetOK && source != target && src.State == "active" && dst.State == "active" && src.Enrollment != "" && dst.Enrollment != "" && |
| + src.Region != "" && src.Region == recRegion && dst.Region == to && src.Region != to |
| +} |
| + |
| // sourceEnrollment is the enrollment of the box that executes the claim, and |
| // whether the roster lets a move start from it: an entry under that box |
| // reference (never a label), in state "active", with a non-empty |
diff --git a/internal/console/apiinstancemove_claim_test.go b/internal/console/apiinstancemove_claim_test.go| index d27eb4482..30b76319a 100644 |
| --- a/internal/console/apiinstancemove_claim_test.go |
| +++ b/internal/console/apiinstancemove_claim_test.go |
| @@ -278,21 +278,44 @@ func TestMoveDoorStartIsWhatC7Converts(t *testing.T) { |
| } |
| } |
| -// failRecordPut refuses every write of one record key; reads pass. |
| +// failRecordPut refuses writes of one record key; reads pass. With fails |
| +// > 0 it refuses that many writes and then passes; 0 refuses every write. |
| type failRecordPut struct { |
| release.Store |
| - key string |
| + key string |
| + fails *int |
| } |
| func (f failRecordPut) Put(ctx context.Context, key string, body []byte, match string) (string, error) { |
| - if key == f.key { |
| + if key == f.key && (f.fails == nil || *f.fails > 0) { |
| + if f.fails != nil { |
| + *f.fails-- |
| + } |
| return "", errors.New("secret-detail: record put failed") |
| } |
| return f.Store.Put(ctx, key, body, match) |
| } |
| +// hookFirstGet runs hook once, on the first read of key, before the read. |
| +type hookFirstGet struct { |
| + release.Store |
| + key string |
| + hook *func() |
| +} |
| + |
| +func (h hookFirstGet) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == h.key && *h.hook != nil { |
| + f := *h.hook |
| + *h.hook = nil |
| + f() |
| + } |
| + return h.Store.Get(ctx, key) |
| +} |
| + |
| // Every host-claim refusal answers its recorded sentence before the claim |
| -// CAS: the claim and the record are as they were. |
| +// CAS: the claim and the record are as they were. A roster that does not |
| +// place both boxes is refused from the door's own roster observation, before |
| +// any registration is written. |
| func TestMoveDoorHostClaimRefusals(t *testing.T) { |
| const roster = "the host roster does not allow this move. the source and target must be active boxes in ie and uk" |
| for _, tc := range []struct { |
| @@ -301,6 +324,7 @@ func TestMoveDoorHostClaimRefusals(t *testing.T) { |
| setup func(t *testing.T, fx *consoleFixture, w *claimMoveWorld) |
| status int |
| msg string |
| + registered bool // the refusal comes after RegisterMove |
| }{ |
| {name: "app-not-bound", admission: true, status: http.StatusConflict, msg: "this app has no host-claim admission yet. try again shortly"}, |
| {name: "no-admission", bind: true, status: http.StatusConflict, msg: "this app has no host-claim admission yet. try again shortly"}, |
| @@ -321,10 +345,38 @@ func TestMoveDoorHostClaimRefusals(t *testing.T) { |
| setup: func(t *testing.T, fx *consoleFixture, _ *claimMoveWorld) { |
| putMoveRoster(t, fx, hostactors.Roster{Boxes: map[string]hostactors.Box{"ie-box": moveRosterBox(ieBox("ie-box"))}}) |
| }}, |
| + {name: "target-in-another-region", bind: true, admission: true, status: http.StatusConflict, msg: roster, |
| + setup: func(t *testing.T, fx *consoleFixture, _ *claimMoveWorld) { |
| + uk := moveRosterBox(ukBox("uk-box")) |
| + uk.Region = "fr" |
| + putMoveRoster(t, fx, hostactors.Roster{Boxes: map[string]hostactors.Box{"ie-box": moveRosterBox(ieBox("ie-box")), "uk-box": uk}}) |
| + }}, |
| + {name: "target-fenced", bind: true, admission: true, status: http.StatusConflict, msg: roster, |
| + setup: func(t *testing.T, fx *consoleFixture, _ *claimMoveWorld) { |
| + writeMoveFence(t, fx, moveRosterBox(ukBox("uk-box")), nil) |
| + }}, |
| + // The door's roster observation placed both boxes; the roster changed |
| + // before BeginClaimMove's own read (here: the target quarantined while |
| + // the move registered). The refusal does not guess which part of the |
| + // world changed. |
| + {name: "changed-under-the-request", bind: true, admission: true, status: http.StatusConflict, registered: true, |
| + msg: "the instance or its boxes changed under this request. read its state and try again"}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| fx, token, moveURL := moveDoorWithClaim(t, "code-inst-move-refuse") |
| - w := armClaimMoves(t, fx, tc.bind, tc.admission, nil) |
| + var raw release.Store |
| + if tc.name == "changed-under-the-request" { |
| + var hook func() |
| + hook = func() { |
| + uk := moveRosterBox(ukBox("uk-box")) |
| + uk.State, uk.QuarantineRevision, uk.LossIDs = "quarantined", 1, []string{"loss"} |
| + putMoveRoster(t, fx, hostactors.Roster{Boxes: map[string]hostactors.Box{"ie-box": moveRosterBox(ieBox("ie-box")), "uk-box": uk}}) |
| + } |
| + // RegisterMove reads the admission through the data handle, |
| + // after the door's own roster checks and before BeginClaimMove. |
| + raw = hookFirstGet{Store: fx.rel, key: "apps/bab/titogo/admission.json", hook: &hook} |
| + } |
| + w := armClaimMoves(t, fx, tc.bind, tc.admission, raw) |
| if tc.setup != nil { |
| tc.setup(t, fx, w) |
| } |
| @@ -343,6 +395,16 @@ func TestMoveDoorHostClaimRefusals(t *testing.T) { |
| if rec.Move != nil { |
| t.Fatalf("a refused request journaled a move: %+v", rec.Move) |
| } |
| + if !tc.admission { |
| + return |
| + } |
| + a, err := hostops.ReadAdmission(context.Background(), fx.rel, "bab", "titogo", claimMoveAppID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := len(a.Value.Registrations) > 0; got != tc.registered { |
| + t.Fatalf("registrations after the refusal = %+v, want registered=%v", a.Value.Registrations, tc.registered) |
| + } |
| }) |
| } |
| } |
| @@ -386,3 +448,45 @@ func TestMoveDoorHostClaimFailedRecordStartReleasesTheClaim(t *testing.T) { |
| t.Fatalf("record move = %+v", rec.Move) |
| } |
| } |
| + |
| +// The failed record start's "try again" must be true: after the storage |
| +// failure clears, a retry starts the move, and once the moves settle and the |
| +// registrations pass their TTL, the failed attempt's registration drains |
| +// (its effect does not stay unsettled). |
| +// |
| +// KNOWN GAP (task E-switch fix round 1, NEEDS_CONTEXT): this fails today. The |
| +// failed attempt's move-start effect is left "uncertain" (the Put was issued), |
| +// the retry's effect on the same record is refused by prepareEffect, and the |
| +// registration cannot drain while its effect is unsettled. hostops has no |
| +// API that resolves an uncertain record effect; the claim code owns that. |
| +// Remove the skip when it lands and the compensation calls it. |
| +func TestMoveDoorHostClaimRetryAfterAFailedRecordStart(t *testing.T) { |
| + t.Skip("KNOWN GAP: no hostops API resolves an uncertain move-start effect (task E-switch fix round 1)") |
| + ctx := context.Background() |
| + fx, token, moveURL := moveDoorWithClaim(t, "code-inst-move-retry") |
| + fails := 1 |
| + w := armClaimMoves(t, fx, true, true, failRecordPut{Store: fx.rel, key: provision.RecordKey("bab", "titogo", "acme.go.tito.io"), fails: &fails}) |
| + if code, _, out := rawReq(t, http.MethodPost, moveURL, token, `{"region":"uk"}`); code != http.StatusBadGateway || errorText(out) != "could not record the move. try again" { |
| + t.Fatalf("first request = %d %s", code, out) |
| + } |
| + a, err := hostops.ReadAdmission(ctx, fx.rel, "bab", "titogo", claimMoveAppID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var failed string |
| + for id := range a.Value.Registrations { |
| + failed = id |
| + } |
| + if code, _, out := rawReq(t, http.MethodPost, moveURL, token, `{"region":"uk"}`); code != http.StatusAccepted { |
| + t.Fatalf("retry after the failure cleared = %d %s (failed attempt's effect: %+v)", code, out, a.Value.Effects[failed]) |
| + } |
| + if err := w.control.SettleRegistration(ctx, "bab", "titogo", claimMoveAppID, failed, time.Now().Add(hostops.RegistrationTTL+2*hostops.RegistrationSkew)); err != nil { |
| + t.Fatalf("the failed attempt's registration does not drain: %v", err) |
| + } |
| + if a, err = hostops.ReadAdmission(ctx, fx.rel, "bab", "titogo", claimMoveAppID); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := a.Value.Registrations[failed]; ok { |
| + t.Fatal("the failed attempt's registration is still held") |
| + } |
| +} |