offline-move: fix round 1 (manifest Owner and Source, park roles, refusal rows, no-write repeat abort)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
7 files changed,
+458
−125
approved_moves_copy_test.go+2 −0docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+16 −1internal/moves/offline.go+115 −55internal/moves/offline_test.go+140 −32internal/provision/offline_move.go+75 −24internal/provision/offline_move_test.go+107 −13internal/provision/provision.go+3 −0
CI cancelled — superseded by a newer push — run details
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index 73c8186e4..d9336e645 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -95,6 +95,7 @@ var movesCopyIDs = []string{ |
| "MV-O1", "MV-O2", "MV-O3", "MV-O4", "MV-O5", "MV-O6", "MV-O7", "MV-O8", "MV-O9", "MV-O10", |
| "MV-O20", "MV-O21", "MV-O22", "MV-O23", "MV-O24", "MV-O25", "MV-O26", "MV-O27", "MV-O28", "MV-O29", "MV-O30", "MV-O31", |
| "MV-O32", "MV-O33", |
| + "MV-O34", "MV-O35", "MV-O36", "MV-O37", "MV-O38", "MV-O39", "MV-O40", "MV-O41", "MV-O42", "MV-O43", |
| } |
| // copyRow is one row of the record. The File cell is the file the sentence is |
| @@ -1311,6 +1312,7 @@ var movesScanFiles = []string{ |
| "internal/moves/lease.go", |
| "internal/moves/log.go", |
| "internal/moves/ltxbuild.go", |
| + "internal/moves/offline.go", |
| "internal/moves/operatorcopy.go", |
| "internal/moves/parkscope.go", |
| "internal/moves/probe.go", |
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 d1547ff60..264d6103d 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| @@ -487,7 +487,7 @@ Task 5 withdraws MV-N21 ("moves to the home region are not supported yet. the ho |
| ## Offline operator move |
| -Plan `docs/superpowers/plans/2026-10-20-offline-operator-move.md`. The rows take MV-O1 to MV-O33, each `pending` until Paul reviews them, and merge waits for that. MV-O1 to MV-O10 are `carlos move offline` output (`move.go`, Task 7); MV-O20 to MV-O31 are the console doors' refusals (`internal/console/apiinstancemove_offline.go`, Task 6); MV-O32 and MV-O33 are the two paragraphs of the docs page (Task 7). MV-O11 to MV-O19 are not used. The sentences the offline verbs share with the move already reviewed keep their ids and approved text, and only their fills change: M-C1 (Moving <host> from <from> to <to>.), M-C10 with `<from>` filled in (Waiting for <box> in <from> to confirm it has stopped <host>. <host> is offline.), M-C6, M-C7, and M-C9. `<from>` is the home's configured name, or the word `home` when none is configured. Door sentences are lower case, as the move door's are. |
| +Plan `docs/superpowers/plans/2026-10-20-offline-operator-move.md`. The rows take MV-O1 to MV-O43, each `pending` until Paul reviews them, and merge waits for that. MV-O1 to MV-O10 are `carlos move offline` output (`move.go`, Task 7); MV-O20 to MV-O31 are the console doors' refusals (`internal/console/apiinstancemove_offline.go`, Task 6); MV-O32 and MV-O33 are the two paragraphs of the docs page (Task 7). MV-O34 to MV-O43 are the refusals of the record's offline transitions (`internal/provision/offline_move.go`), which a console door maps onto its own rows or sends as they are; they are not free text. MV-O11 to MV-O19 are not used. The sentences the offline verbs share with the move already reviewed keep their ids and approved text, and only their fills change: M-C1 (Moving <host> from <from> to <to>.), M-C10 with `<from>` filled in (Waiting for <box> in <from> to confirm it has stopped <host>. <host> is offline.), M-C6, M-C7, and M-C9. `<from>` is the home's configured name, or the word `home` when none is configured. Door sentences are lower case, as the move door's are. |
| | id | file | text | status | |
| |---|---|---|---| |
| @@ -520,3 +520,18 @@ The docs page (`docs/site/cli.md`, under `### carlos move`) gets two paragraphs |
| |---|---|---|---| |
| | MV-O32 | docs/site/cli.md | `carlos move offline` moves one instance with planned downtime, one step at a time. It is for operators, works with host claims and Phase 1 moves off, and never acts on its own: each verb does one step and says which verb comes next. | pending | |
| | MV-O33 | docs/site/cli.md | The instance is offline from `start` until `open`. Until `open`, `abort` puts it back where it was with nothing lost. After `open`, moving it back is a new move. | pending | |
| + |
| +The offline transitions' refusals (fix round 1, review M4). A refusal that names no value is a whole sentence; the ones that name a value are one format. The no-move, same-region, malformed-start, replica and in-flight refusals reuse the existing rows (MV-N45, MV-N16, MV-N14, MV-N44, MV-N43). |
| + |
| +| id | file | text | status | |
| +|---|---|---|---| |
| +| MV-O34 | internal/provision/offline_move.go | this is not an offline move | pending | |
| +| MV-O35 | internal/provision/offline_move.go | the record carries a different offline move | pending | |
| +| MV-O36 | internal/provision/offline_move.go | this offline move is <state>. this step follows <state> | pending | |
| +| MV-O37 | internal/provision/offline_move.go | a seal names the box that seals and the boxes that stopped the instance. the box that seals is one of them | pending | |
| +| MV-O38 | internal/provision/offline_move.go | this offline move is sealed by <box> | pending | |
| +| MV-O39 | internal/provision/offline_move.go | the record's region is <region>, which this step does not expect. the instance changed under the move | pending | |
| +| MV-O40 | internal/provision/offline_move.go | this offline move is <state>. it cannot be aborted once it is open | pending | |
| +| MV-O41 | internal/provision/offline_move.go | this offline move is <state>. it finishes from finishing or reverting | pending | |
| +| MV-O42 | internal/provision/offline_move.go | this is an offline move. use carlos move offline | pending | |
| +| MV-O43 | internal/provision/offline_move.go | the instance is in <region>, not <from> | pending | |
diff --git a/internal/moves/offline.go b/internal/moves/offline.go| index c95790aaf..a51bdbbe1 100644 |
| --- a/internal/moves/offline.go |
| +++ b/internal/moves/offline.go |
| @@ -16,51 +16,61 @@ import ( |
| // OfflineItem is one row of the dependency list: a thing the instance needs |
| // that the move must carry, rebuild or consciously leave behind. ID names the |
| // item in a report's Items. Disposition says what the move does with it. |
| -// Owner says who decides it is in place: |
| +// |
| +// Owner is the step that proves the item where it must end up, and the |
| +// door that step gates requires that step's OK: |
| // |
| // door a door refuses the move up front (nothing to check later) |
| -// seal the sealer's report carries its Check; flip needs it OK |
| -// landing the target's landing report carries its Check; open needs it OK |
| -// none not checked by a report; the Disposition says why |
| -type OfflineItem struct{ ID, Item, Disposition, Owner string } |
| - |
| -// The owners. |
| +// seal the sealer's report proves it (the source's own state) |
| +// landing the target's landing report proves it; the open door requires it |
| +// none no report proves it; the Disposition says why |
| +// |
| +// Source is the step that reports the expected value on the source side: the |
| +// sealer's report ("seal"), which the flip door requires, or "" when the item |
| +// has no source-side value. A carried item is Owner landing and Source seal: |
| +// the sealer records what it copied, the target proves it arrived equal. |
| +// Exactly one Owner per row; Source is independent of it. |
| +type OfflineItem struct{ ID, Item, Disposition, Owner, Source string } |
| + |
| +// The owners and sources. |
| const ( |
| OfflineOwnerDoor = "door" |
| OfflineOwnerSeal = "seal" |
| OfflineOwnerLanding = "landing" |
| OfflineOwnerNone = "none" |
| + |
| + OfflineSourceSeal = "seal" |
| ) |
| // OfflineManifest is the plan's dependency list, one row per item. It is the |
| -// checklist: the doors refuse flip and open unless every item their stage |
| -// owns is OK, a report may carry only items listed here, and |
| -// TestOfflineManifestIsCovered fails if a row has no owner. A Disposition |
| -// begins with its kind (carried, rebuilt, refused, parked, not carried); an |
| -// item no report checks (owner none) says why after the first colon. |
| +// checklist: flip needs every Source "seal" item OK in the sealer's report, |
| +// open needs every Owner "landing" item OK in the target's, a report may |
| +// carry only items listed here, and TestOfflineManifestIsCovered fails if a |
| +// row has no owner. A Disposition begins with its kind (carried, rebuilt, |
| +// refused, parked, not carried) and a reason after the first colon. |
| var OfflineManifest = []OfflineItem{ |
| - {"db", "SQLite DB at its final position", "carried: instances/<host>/ is mirrored to the target's moves/<host>/ and promoted by the target's first wake after open", OfflineOwnerSeal}, |
| - {"db-tail", "writes the source's stop failed to checkpoint", "carried: the sealer checkpoints first when a source report is ahead of the replica", OfflineOwnerSeal}, |
| - {"build-state", "instances/<host>/build-state.json", "carried: inside the replica prefix, after CarryRequest", OfflineOwnerSeal}, |
| - {"lease", "instances/<host>/lock.json", "not carried: the source's lock becomes the tombstone until finish; a stale tombstone of another move in the target bucket is lifted at landing", OfflineOwnerSeal}, |
| - {"files", "flat <DataDir>/<host>/ and the nested instance directory minus platform artifacts", "carried: sealed archive at moves/@offline/<host>/<id>/files.sealed in the target bucket", OfflineOwnerSeal}, |
| - {"env", "<host>.env: instance key, admin token and overrides", "carried: byte for byte inside the archive", OfflineOwnerSeal}, |
| - {"key-adoption", "<host>.instance-key-adoption.json", "not carried: written only for claim launches; a copy on the target is parked", OfflineOwnerLanding}, |
| - {"config", "<host>.config.env: the app's settings, store binding and secrets", "rebuilt: rendered on the target from the bucket bundle", OfflineOwnerLanding}, |
| - {"env-pin", "the composed launch environment", "rebuilt: app settings must be equal on both rows; box settings are not compared", OfflineOwnerLanding}, |
| - {"release", "the build the source runs", "rebuilt: not copied, ordinary adoption stages the channel's build for the ready row after open", OfflineOwnerLanding}, |
| - {"store", "object store bucket and its credentials", "not carried: the bucket stays where it is and its binding travels in config, so env-pin covers it", OfflineOwnerNone}, |
| - {"timers", "timer rows, once-timers the app armed included", "carried: a timers snapshot beside the archive, replaced on the held row", OfflineOwnerSeal}, |
| - {"next-due", "the row's NextDue", "carried: in the journal's source baselines", OfflineOwnerSeal}, |
| - {"replication", "the row's replication rung", "carried: in the journal's source baselines", OfflineOwnerSeal}, |
| - {"row", "the registry row", "rebuilt: reconcile mints it from the record, held at flat paths", OfflineOwnerLanding}, |
| - {"routing", "edges file, backhaul, certificates, derived aliases", "not carried: produced by the existing passes once the row is ready, and the runbook confirms a certificate path before the move", OfflineOwnerNone}, |
| - {"source-retire", "everything the source box keeps for the host after open", "parked: renamed, never deleted, and listed path by path in the source's parked report", OfflineOwnerNone}, |
| - {"sidecar", "<host>-sidecar.env", "refused: a source report that sees one fails the seal", OfflineOwnerSeal}, |
| - {"restore-verify", "control/restore-verify.json entry", "not carried: the next drill writes a fresh verdict, and until then the old one describes the source copy", OfflineOwnerNone}, |
| - {"warm-cache", "local DB, meta dirs, files and env a box holds for the host", "parked: renamed under .offline-moves on the source after open and on a target that already has them", OfflineOwnerLanding}, |
| - {"receipts", "move-receipts/<host>.json in the source bucket", "not carried: cleared at finish, and self-checking", OfflineOwnerNone}, |
| - {"class", "kind, addr, fleet, pool, hibernate, replica, replicas, steering, unit, seed and open run", "refused: the start door and the start transition refuse every class the move cannot carry", OfflineOwnerDoor}, |
| + {"db", "SQLite DB at its final position", "carried: instances/<host>/ is mirrored to the target's moves/<host>/ and promoted by the target's first wake after open", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"db-tail", "writes the source's stop failed to checkpoint", "carried: the sealer checkpoints first when a source report is ahead of the replica", OfflineOwnerSeal, OfflineSourceSeal}, |
| + {"build-state", "instances/<host>/build-state.json", "carried: inside the replica prefix, after CarryRequest", OfflineOwnerSeal, OfflineSourceSeal}, |
| + {"lease", "instances/<host>/lock.json", "not carried: the source's lock becomes the tombstone until finish; a stale tombstone of another move in the target bucket is lifted at landing", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"files", "flat <DataDir>/<host>/ and the nested instance directory minus platform artifacts", "carried: sealed archive at moves/@offline/<host>/<id>/files.sealed in the target bucket", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"env", "<host>.env: instance key, admin token and overrides", "carried: byte for byte inside the archive", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"key-adoption", "<host>.instance-key-adoption.json", "not carried: written only for claim launches; a copy on the target is parked", OfflineOwnerLanding, ""}, |
| + {"config", "<host>.config.env: the app's settings, store binding and secrets", "rebuilt: rendered on the target from the bucket bundle", OfflineOwnerLanding, ""}, |
| + {"env-pin", "the composed launch environment", "rebuilt: app settings must be equal on both rows; box settings are not compared", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"release", "the build the source runs", "rebuilt: not copied, ordinary adoption stages the channel's build for the ready row after open", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"store", "object store bucket and its credentials", "not carried: the bucket stays where it is and its binding travels in config, so env-pin covers it", OfflineOwnerNone, ""}, |
| + {"timers", "timer rows, once-timers the app armed included", "carried: a timers snapshot beside the archive, replaced on the held row", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"next-due", "the row's NextDue", "carried: in the journal's source baselines", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"replication", "the row's replication rung", "carried: in the journal's source baselines", OfflineOwnerLanding, OfflineSourceSeal}, |
| + {"row", "the registry row", "rebuilt: reconcile mints it from the record, held at flat paths", OfflineOwnerLanding, ""}, |
| + {"routing", "edges file, backhaul, certificates, derived aliases", "not carried: produced by the existing passes once the row is ready, and the runbook confirms a certificate path before the move", OfflineOwnerNone, ""}, |
| + {"source-retire", "everything the source box keeps for the host after open", "parked: renamed, never deleted, and listed path by path in the source's parked report", OfflineOwnerNone, ""}, |
| + {"sidecar", "<host>-sidecar.env", "refused: a source report that sees one fails the seal", OfflineOwnerSeal, OfflineSourceSeal}, |
| + {"restore-verify", "control/restore-verify.json entry", "not carried: the next drill writes a fresh verdict, and until then the old one describes the source copy", OfflineOwnerNone, ""}, |
| + {"warm-cache", "local DB, meta dirs, files and env a box holds for the host", "parked: renamed under .offline-moves on the source after open and on a target that already has them", OfflineOwnerLanding, ""}, |
| + {"receipts", "move-receipts/<host>.json in the source bucket", "not carried: cleared at finish, and self-checking", OfflineOwnerNone, ""}, |
| + {"class", "kind, addr, fleet, pool, hibernate, replica, replicas, steering, unit, seed and open run", "refused: the start door and the start transition refuse every class the move cannot carry", OfflineOwnerDoor, ""}, |
| } |
| // OfflineItemOwner is the owner of manifest item id. |
| @@ -73,6 +83,15 @@ func OfflineItemOwner(id string) (owner string, ok bool) { |
| return "", false |
| } |
| +func offlineItem(id string) (OfflineItem, bool) { |
| + for _, it := range OfflineManifest { |
| + if it.ID == id { |
| + return it, true |
| + } |
| + } |
| + return OfflineItem{}, false |
| +} |
| + |
| // OfflineOwned lists the manifest items an owner decides, in manifest order. |
| func OfflineOwned(owner string) []OfflineItem { |
| var out []OfflineItem |
| @@ -84,6 +103,18 @@ func OfflineOwned(owner string) []OfflineItem { |
| return out |
| } |
| +// OfflineSourced lists the manifest items whose expected value the source |
| +// side reports (Source "seal"), in manifest order. |
| +func OfflineSourced() []OfflineItem { |
| + var out []OfflineItem |
| + for _, it := range OfflineManifest { |
| + if it.Source == OfflineSourceSeal { |
| + out = append(out, it) |
| + } |
| + } |
| + return out |
| +} |
| + |
| // The roles a report names: a source-region box that stopped the host, the |
| // one source box that seals, and the target box that lands. |
| const ( |
| @@ -173,7 +204,7 @@ type OfflineReport struct { |
| NextDue int64 `json:"next_due,omitempty"` |
| TombstoneOwner string `json:"tombstone_owner,omitempty"` |
| - TombstoneUntil time.Time `json:"tombstone_until"` |
| + TombstoneUntil time.Time `json:"tombstone_until,omitzero"` |
| Procs int `json:"procs,omitempty"` |
| LayoutJournal bool `json:"layout_journal,omitempty"` |
| @@ -186,15 +217,22 @@ var ( |
| offlineRoles = map[string]bool{OfflineRoleSource: true, OfflineRoleSealer: true, OfflineRoleTarget: true} |
| offlinePhases = map[string]bool{OfflinePhaseStopped: true, OfflinePhaseSealed: true, OfflinePhaseLanded: true, OfflinePhaseOpened: true, |
| OfflinePhaseParked: true, OfflinePhaseGone: true, OfflinePhaseLifted: true, OfflinePhaseReverted: true, OfflinePhaseFailed: true} |
| - // The owners whose Checks a role's report may carry. The target also |
| - // re-verifies what the seal recorded, so it may carry the seal's items. |
| - offlineRoleOwners = map[string][]string{ |
| - OfflineRoleSource: {OfflineOwnerSeal}, |
| - OfflineRoleSealer: {OfflineOwnerSeal}, |
| - OfflineRoleTarget: {OfflineOwnerLanding, OfflineOwnerSeal}, |
| - } |
| + // The Checks a role's report may carry: the source side's (the sealer |
| + // and the stopped source boxes) carry items with Source "seal", the target |
| + // carries items it owns ("landing"). |
| ) |
| +// offlineCarries reports whether a role's report may carry the item. |
| +func offlineCarries(role string, it OfflineItem) bool { |
| + switch role { |
| + case OfflineRoleSource, OfflineRoleSealer: |
| + return it.Source == OfflineSourceSeal |
| + case OfflineRoleTarget: |
| + return it.Owner == OfflineOwnerLanding |
| + } |
| + return false |
| +} |
| + |
| // ValidateOfflineReport reports why a report cannot be trusted as a report: a |
| // wrong version, an unknown role or phase, an unusable key part, or an item |
| // the manifest does not list or that this role does not carry. A reader |
| @@ -210,18 +248,12 @@ func ValidateOfflineReport(r OfflineReport) error { |
| return fmt.Errorf("offline report: unknown phase %q", r.Phase) |
| } |
| for id := range r.Items { |
| - owner, ok := OfflineItemOwner(id) |
| + it, ok := offlineItem(id) |
| if !ok { |
| return fmt.Errorf("offline report: item %q is not in the manifest", id) |
| } |
| - allowed := false |
| - for _, o := range offlineRoleOwners[r.Role] { |
| - if o == owner { |
| - allowed = true |
| - } |
| - } |
| - if !allowed { |
| - return fmt.Errorf("offline report: item %q (owner %s) is not carried by a %s report", id, owner, r.Role) |
| + if !offlineCarries(r.Role, it) { |
| + return fmt.Errorf("offline report: item %q (owner %s, source %q) is not carried by a %s report", id, it.Owner, it.Source, r.Role) |
| } |
| } |
| return nil |
| @@ -240,6 +272,18 @@ func (r OfflineReport) ItemsOK(owner string) (ok bool, bad []string) { |
| return len(bad) == 0, bad |
| } |
| +// SourceItemsOK is ItemsOK for the flip door: whether the report holds an OK |
| +// Check for every item the source side reports (Source "seal"), and the ones |
| +// that are missing or not OK. |
| +func (r OfflineReport) SourceItemsOK() (ok bool, bad []string) { |
| + for _, it := range OfflineSourced() { |
| + if c, found := r.Items[it.ID]; !found || !c.OK { |
| + bad = append(bad, it.ID) |
| + } |
| + } |
| + return len(bad) == 0, bad |
| +} |
| + |
| // offlineSegment reports a string fit to be one path or key segment: not |
| // empty, not a dot name, and made only of letters, digits and _ . - so it can |
| // never escape the directory or prefix it is joined under. |
| @@ -298,12 +342,28 @@ func TombstoneOwner(op string) string { return "moved:" + op } |
| // re-asserts it every pass and finish lifts it. |
| const TombstoneTTL = 7 * 24 * time.Hour |
| +// The roles OfflineParkDir accepts besides the report roles: the target parks |
| +// what it finds before landing under "target-before", and the landing's |
| +// verify scratch under "verify" (Task 5). Nothing else is accepted; a numbered |
| +// suffix ("target-before-2") is the caller's, built by renaming within the |
| +// directory this returns, never passed through the helper. |
| +const ( |
| + OfflineParkTargetBefore = "target-before" |
| + OfflineParkVerify = "verify" |
| +) |
| + |
| +var offlineParkRoles = map[string]bool{ |
| + OfflineRoleSource: true, OfflineRoleSealer: true, OfflineRoleTarget: true, |
| + OfflineParkTargetBefore: true, OfflineParkVerify: true, |
| +} |
| + |
| // OfflineParkDir is where a box parks what it renames for the host: |
| -// <dataDir>/.offline-moves/<host>/<op>/<role>/. "" when dataDir is empty, |
| -// host fails provision.ValidHostShape, role is not source, sealer or target, |
| -// or op is not a plain name; a caller must not rename into "". |
| +// <dataDir>/.offline-moves/<host>/<op>/<role>/. role is source, sealer, |
| +// target, OfflineParkTargetBefore or OfflineParkVerify. "" when dataDir is |
| +// empty, host fails provision.ValidHostShape, role is none of those, or op is |
| +// not a plain name; a caller must not rename into "". |
| func OfflineParkDir(dataDir, host, op, role string) string { |
| - if dataDir == "" || !provision.ValidHostShape(host) || !offlineSegment(op) || !offlineRoles[role] { |
| + if dataDir == "" || !provision.ValidHostShape(host) || !offlineSegment(op) || !offlineParkRoles[role] { |
| return "" |
| } |
| return filepath.Join(dataDir, ".offline-moves", host, op, role) |
diff --git a/internal/moves/offline_test.go b/internal/moves/offline_test.go| index d7ac75a52..635eb9e23 100644 |
| --- a/internal/moves/offline_test.go |
| +++ b/internal/moves/offline_test.go |
| @@ -26,6 +26,24 @@ var planManifestIDs = []string{ |
| "warm-cache", "receipts", "class", |
| } |
| +// planManifestPairs pins each row's Owner (who proves it where it must end |
| +// up) and Source (who reports its expected value on the source side). A |
| +// reassignment fails here, so it is a decision and not a drift. Rows the |
| +// sealer alone produces (db-tail, build-state, sidecar) are owned by the seal; |
| +// every carried outage item is owned by the landing and sourced by the seal. |
| +var planManifestPairs = map[string][2]string{ |
| + "db": {OfflineOwnerLanding, OfflineSourceSeal}, "db-tail": {OfflineOwnerSeal, OfflineSourceSeal}, |
| + "build-state": {OfflineOwnerSeal, OfflineSourceSeal}, "lease": {OfflineOwnerLanding, OfflineSourceSeal}, |
| + "files": {OfflineOwnerLanding, OfflineSourceSeal}, "env": {OfflineOwnerLanding, OfflineSourceSeal}, |
| + "key-adoption": {OfflineOwnerLanding, ""}, "config": {OfflineOwnerLanding, ""}, |
| + "env-pin": {OfflineOwnerLanding, OfflineSourceSeal}, "release": {OfflineOwnerLanding, OfflineSourceSeal}, |
| + "store": {OfflineOwnerNone, ""}, "timers": {OfflineOwnerLanding, OfflineSourceSeal}, |
| + "next-due": {OfflineOwnerLanding, OfflineSourceSeal}, "replication": {OfflineOwnerLanding, OfflineSourceSeal}, |
| + "row": {OfflineOwnerLanding, ""}, "routing": {OfflineOwnerNone, ""}, "source-retire": {OfflineOwnerNone, ""}, |
| + "sidecar": {OfflineOwnerSeal, OfflineSourceSeal}, "restore-verify": {OfflineOwnerNone, ""}, |
| + "warm-cache": {OfflineOwnerLanding, ""}, "receipts": {OfflineOwnerNone, ""}, "class": {OfflineOwnerDoor, ""}, |
| +} |
| + |
| var offlineDispositionKinds = []string{"carried", "rebuilt", "refused", "parked", "not carried"} |
| // validateOfflineManifest is the checklist rule: every row has an id, a thing |
| @@ -44,6 +62,8 @@ func validateOfflineManifest(rows []OfflineItem) error { |
| return fmt.Errorf("row %q names no item", it.ID) |
| case !owners[it.Owner]: |
| return fmt.Errorf("row %q has owner %q, which is not door, seal, landing or none", it.ID, it.Owner) |
| + case it.Source != "" && it.Source != OfflineSourceSeal: |
| + return fmt.Errorf("row %q has source %q, which is neither seal nor empty", it.ID, it.Source) |
| } |
| seen[it.ID] = true |
| kind := "" |
| @@ -74,6 +94,34 @@ func TestOfflineManifestIsCovered(t *testing.T) { |
| t.Fatalf("the manifest's rows are not the plan's:\n got %v\nwant %v", ids, planManifestIDs) |
| } |
| + // Owner and Source of every row are the pinned pair, and the pins cover |
| + // every row. |
| + if len(planManifestPairs) != len(OfflineManifest) { |
| + t.Fatalf("%d pinned pairs for %d rows", len(planManifestPairs), len(OfflineManifest)) |
| + } |
| + for _, it := range OfflineManifest { |
| + want, ok := planManifestPairs[it.ID] |
| + if !ok || it.Owner != want[0] || it.Source != want[1] { |
| + t.Errorf("%s: owner %q source %q, pinned %v", it.ID, it.Owner, it.Source, want) |
| + } |
| + } |
| + // The outage items: every carried item the target must have is required |
| + // at open (owner landing) and reported at flip (source seal). |
| + for _, id := range []string{"db", "lease", "files", "env", "timers", "next-due", "replication"} { |
| + it, _ := offlineItem(id) |
| + if it.Owner != OfflineOwnerLanding || it.Source != OfflineSourceSeal { |
| + t.Errorf("%s is not proven on the target and reported by the seal: %+v", id, it) |
| + } |
| + } |
| + var sourced []string |
| + for _, it := range OfflineSourced() { |
| + sourced = append(sourced, it.ID) |
| + } |
| + wantSourced := []string{"db", "db-tail", "build-state", "lease", "files", "env", "env-pin", "release", "timers", "next-due", "replication", "sidecar"} |
| + if !reflect.DeepEqual(sourced, wantSourced) { |
| + t.Fatalf("OfflineSourced: %v, want %v", sourced, wantSourced) |
| + } |
| + |
| // Every row is owned by exactly one of the four: OfflineOwned partitions |
| // the manifest. |
| total := 0 |
| @@ -106,20 +154,21 @@ func TestOfflineManifestIsCovered(t *testing.T) { |
| // The checklist rule must bite: a row added with no owner, a duplicate, an |
| // unknown owner or a disposition with no reason fails. |
| func TestOfflineManifestRuleBites(t *testing.T) { |
| - good := OfflineItem{"extra", "something", "carried: because", OfflineOwnerSeal} |
| + good := OfflineItem{"extra", "something", "carried: because", OfflineOwnerSeal, OfflineSourceSeal} |
| if err := validateOfflineManifest(append(append([]OfflineItem(nil), OfflineManifest...), good)); err != nil { |
| t.Fatalf("a well-formed extra row: %v", err) |
| } |
| for name, bad := range map[string]OfflineItem{ |
| - "no owner": {"extra", "something", "carried: because", ""}, |
| - "unknown owner": {"extra", "something", "carried: because", "operator"}, |
| - "no id": {"", "something", "carried: because", OfflineOwnerSeal}, |
| - "duplicate id": {"db", "something", "carried: because", OfflineOwnerSeal}, |
| - "no item": {"extra", "", "carried: because", OfflineOwnerSeal}, |
| - "no reason": {"extra", "something", "carried", OfflineOwnerSeal}, |
| - "empty reason": {"extra", "something", "carried: ", OfflineOwnerSeal}, |
| - "unknown kind": {"extra", "something", "forgotten: because", OfflineOwnerSeal}, |
| - "em dash in a row": {"extra", "something", "carried: because \u2014 reasons", OfflineOwnerSeal}, |
| + "no owner": {"extra", "something", "carried: because", "", ""}, |
| + "unknown owner": {"extra", "something", "carried: because", "operator", ""}, |
| + "no id": {"", "something", "carried: because", OfflineOwnerSeal, ""}, |
| + "duplicate id": {"db", "something", "carried: because", OfflineOwnerSeal, ""}, |
| + "no item": {"extra", "", "carried: because", OfflineOwnerSeal, ""}, |
| + "no reason": {"extra", "something", "carried", OfflineOwnerSeal, ""}, |
| + "empty reason": {"extra", "something", "carried: ", OfflineOwnerSeal, ""}, |
| + "unknown kind": {"extra", "something", "forgotten: because", OfflineOwnerSeal, ""}, |
| + "unknown source": {"extra", "something", "carried: because", OfflineOwnerSeal, "operator"}, |
| + "em dash in a row": {"extra", "something", "carried: because \u2014 reasons", OfflineOwnerSeal, ""}, |
| } { |
| if err := validateOfflineManifest(append(append([]OfflineItem(nil), OfflineManifest...), bad)); err == nil { |
| t.Errorf("%s: the rule accepted the row", name) |
| @@ -128,41 +177,66 @@ func TestOfflineManifestRuleBites(t *testing.T) { |
| } |
| // A report may carry only items the manifest lists, and only the ones its |
| -// role carries. |
| +// role carries: the source side carries what the seal reports, the target |
| +// what the landing owns. |
| func TestOfflineReportItemsAreTheManifests(t *testing.T) { |
| base := func(role string) OfflineReport { |
| return OfflineReport{V: OfflineReportVersion, Op: offTestOp, Host: offTestHost, Box: "box-a", Role: role, Phase: OfflinePhaseSealed, |
| Items: map[string]Check{}, At: time.Date(2026, 10, 21, 9, 0, 0, 0, time.UTC)} |
| } |
| - // Every owned item, in a report of the right role, validates. |
| sealer := base(OfflineRoleSealer) |
| - for _, it := range OfflineOwned(OfflineOwnerSeal) { |
| + for _, it := range OfflineSourced() { |
| sealer.Items[it.ID] = Check{OK: true} |
| } |
| if err := ValidateOfflineReport(sealer); err != nil { |
| t.Fatalf("a full sealer report: %v", err) |
| } |
| - if ok, bad := sealer.ItemsOK(OfflineOwnerSeal); !ok || len(bad) != 0 { |
| - t.Fatalf("seal items: %v %v", ok, bad) |
| + if ok, bad := sealer.SourceItemsOK(); !ok || len(bad) != 0 { |
| + t.Fatalf("source items: %v %v", ok, bad) |
| } |
| - if ok, bad := sealer.ItemsOK(OfflineOwnerLanding); ok || len(bad) != len(OfflineOwned(OfflineOwnerLanding)) { |
| + // A sealer report does not satisfy the open door, whatever it carries. |
| + if ok, bad := sealer.ItemsOK(OfflineOwnerLanding); ok || !reflect.DeepEqual(bad, []string{"key-adoption", "config", "row", "warm-cache"}) { |
| t.Fatalf("a sealer report satisfies the landing's items: %v %v", ok, bad) |
| } |
| + // A not-OK seal item is named, in manifest order. |
| + sealer.Items["files"] = Check{OK: false, Reason: "no archive"} |
| + delete(sealer.Items, "db") |
| + if ok, bad := sealer.SourceItemsOK(); ok || !reflect.DeepEqual(bad, []string{"db", "files"}) { |
| + t.Fatalf("SourceItemsOK: %v %v", ok, bad) |
| + } |
| + |
| + // The open door's set includes every outage item, so a target with no |
| + // files directory cannot pass it. |
| target := base(OfflineRoleTarget) |
| target.Phase = OfflinePhaseLanded |
| for _, it := range OfflineOwned(OfflineOwnerLanding) { |
| target.Items[it.ID] = Check{OK: true} |
| } |
| - target.Items["db"] = Check{OK: true} // the target re-verifies what the seal recorded |
| if err := ValidateOfflineReport(target); err != nil { |
| t.Fatalf("a full target report: %v", err) |
| } |
| - // A not-OK item, and a missing one, are named in manifest order. |
| - target.Items["row"] = Check{OK: false, Reason: "no row"} |
| - delete(target.Items, "config") |
| - ok, bad := target.ItemsOK(OfflineOwnerLanding) |
| - if ok || !reflect.DeepEqual(bad, []string{"config", "row"}) { |
| - t.Fatalf("ItemsOK: %v %v", ok, bad) |
| + if ok, bad := target.ItemsOK(OfflineOwnerLanding); !ok || len(bad) != 0 { |
| + t.Fatalf("a full target report: %v %v", ok, bad) |
| + } |
| + for _, id := range []string{"db", "lease", "files", "env", "timers", "next-due", "replication"} { |
| + missing := target |
| + missing.Items = map[string]Check{} |
| + for k, v := range target.Items { |
| + missing.Items[k] = v |
| + } |
| + delete(missing.Items, id) |
| + if ok, bad := missing.ItemsOK(OfflineOwnerLanding); ok || !reflect.DeepEqual(bad, []string{id}) { |
| + t.Errorf("open passes a target report without %s: %v %v", id, ok, bad) |
| + } |
| + failed := missing |
| + failed.Items = map[string]Check{} |
| + for k, v := range target.Items { |
| + failed.Items[k] = v |
| + } |
| + failed.Items[id] = Check{OK: false, Reason: "not there"} |
| + if ok, bad := failed.ItemsOK(OfflineOwnerLanding); ok || !reflect.DeepEqual(bad, []string{id}) { |
| + t.Errorf("open passes a target report with %s failed: %v %v", id, ok, bad) |
| + } |
| } |
| // An item the manifest does not list fails the report. |
| @@ -171,18 +245,30 @@ func TestOfflineReportItemsAreTheManifests(t *testing.T) { |
| if err := ValidateOfflineReport(rogue); err == nil || !strings.Contains(err.Error(), "made-up") { |
| t.Fatalf("an unlisted item: %v", err) |
| } |
| - // An item whose owner is not the report's role. |
| - wrongRole := base(OfflineRoleSealer) |
| - wrongRole.Items["row"] = Check{OK: true} // landing's |
| - if err := ValidateOfflineReport(wrongRole); err == nil { |
| - t.Fatal("a sealer report carried a landing item") |
| + // An item the role does not carry: landing-only items on the source |
| + // side, source-only items on the target. |
| + for _, id := range []string{"row", "config", "key-adoption", "warm-cache"} { |
| + r := base(OfflineRoleSealer) |
| + r.Items[id] = Check{OK: true} |
| + if err := ValidateOfflineReport(r); err == nil { |
| + t.Errorf("a sealer report carried %s", id) |
| + } |
| + } |
| + for _, id := range []string{"db-tail", "build-state", "sidecar"} { |
| + r := base(OfflineRoleTarget) |
| + r.Items[id] = Check{OK: true} |
| + if err := ValidateOfflineReport(r); err == nil { |
| + t.Errorf("a target report carried %s", id) |
| + } |
| } |
| for _, owner := range []string{OfflineOwnerDoor, OfflineOwnerNone} { |
| for _, it := range OfflineOwned(owner) { |
| - r := base(OfflineRoleTarget) |
| - r.Items[it.ID] = Check{OK: true} |
| - if err := ValidateOfflineReport(r); err == nil { |
| - t.Errorf("a report carried %s, which is %s-owned and checked by no report", it.ID, owner) |
| + for _, role := range []string{OfflineRoleSource, OfflineRoleSealer, OfflineRoleTarget} { |
| + r := base(role) |
| + r.Items[it.ID] = Check{OK: true} |
| + if err := ValidateOfflineReport(r); err == nil { |
| + t.Errorf("a %s report carried %s, which is %s-owned and checked by no report", role, it.ID, owner) |
| + } |
| } |
| } |
| } |
| @@ -236,6 +322,10 @@ func TestOfflineReportRoundTrips(t *testing.T) { |
| t.Errorf("a zero report omits %s: %s", k, empty) |
| } |
| } |
| + // The tombstone's time is written only when there is one. |
| + if strings.Contains(string(empty), "tombstone_until") { |
| + t.Errorf("a zero report writes a tombstone time: %s", empty) |
| + } |
| } |
| func TestOfflineKeyHelpers(t *testing.T) { |
| @@ -254,6 +344,24 @@ func TestOfflineKeyHelpers(t *testing.T) { |
| if got := OfflineParkDir("/data", offTestHost, offTestOp, OfflineRoleSource); got != "/data/.offline-moves/"+offTestHost+"/"+offTestOp+"/source" { |
| t.Fatalf("park dir %q", got) |
| } |
| + for _, role := range []string{OfflineParkTargetBefore, OfflineParkVerify} { |
| + if got := OfflineParkDir("/data", offTestHost, offTestOp, role); got != "/data/.offline-moves/"+offTestHost+"/"+offTestOp+"/"+role { |
| + t.Errorf("park dir for %s: %q", role, got) |
| + } |
| + // A park role is not a report role: the report key does not take it. |
| + if OfflineReportKey(offTestHost, offTestOp, role, "box-a") != "" { |
| + t.Errorf("a report key for park role %s", role) |
| + } |
| + } |
| + if OfflineParkTargetBefore != "target-before" || OfflineParkVerify != "verify" { |
| + t.Fatal("the park role names are the plan's") |
| + } |
| + // Nothing else is accepted, and a numbered suffix is the caller's. |
| + for _, role := range []string{"target-before-2", "scratch", "verify/x"} { |
| + if OfflineParkDir("/data", offTestHost, offTestOp, role) != "" { |
| + t.Errorf("park role %q accepted", role) |
| + } |
| + } |
| if TombstoneTTL != 7*24*time.Hour { |
| t.Fatalf("tombstone ttl %v", TombstoneTTL) |
| } |
diff --git a/internal/provision/offline_move.go b/internal/provision/offline_move.go| index 238c33f76..94b257b27 100644 |
| --- a/internal/provision/offline_move.go |
| +++ b/internal/provision/offline_move.go |
| @@ -1,7 +1,9 @@ |
| package provision |
| import ( |
| + "errors" |
| "fmt" |
| + "slices" |
| "time" |
| ) |
| @@ -50,8 +52,36 @@ const ( |
| OfflineOutcomeAborted = "aborted" |
| ) |
| -// refusedOfflineKind is MoveGuard.check's refusal of an offline move. |
| -const refusedOfflineKind = "an offline move has its own transitions" |
| +// The offline transitions' refusals that can reach an operator. Each is a |
| +// recorded sentence (copy rows MV-O34 to MV-O43); a console door maps an |
| +// offline ErrMoveConflict onto them or onto its own rows (MV-O24, MV-O25, |
| +// MV-O27) and never sends err.Error() through unchecked. The refusals that |
| +// name a value are written in place as one format. |
| +const ( |
| + // refusedOfflineKind is MoveGuard.check's refusal of an offline move |
| + // (MV-O42). |
| + refusedOfflineKind = "this is an offline move. use carlos move offline" |
| + // refusedNotOffline: an offline transition met a legacy or Phase 1 move |
| + // (MV-O34). |
| + refusedNotOffline = "this is not an offline move" |
| + // refusedOtherOfflineMove: the record's offline move has another id |
| + // (MV-O35). |
| + refusedOtherOfflineMove = "the record carries a different offline move" |
| + // refusedOfflineSeal: a seal that names no sealer, no boxes, or a sealer |
| + // that is not one of them (MV-O37). |
| + refusedOfflineSeal = "a seal names the box that seals and the boxes that stopped the instance. the box that seals is one of them" |
| + // refusedOfflineAfterOpen is OfflineAbort past the point where the target |
| + // serves (MV-O40 is the format: this offline move is <state>. ...). |
| + refusedOfflineAfterOpen = "it cannot be aborted once it is open" |
| +) |
| + |
| +// errRecordUnchanged is what a transition returns to say "the record already |
| +// says this": updateRecord then writes nothing and returns the record as read |
| +// (no record_seq advance, no updated_at stamp). Only OfflineAbort from |
| +// reverting uses it. Applied outside updateRecord the mut returns it as an |
| +// error, so a transition that can return it is called only through |
| +// SetRecordMove. |
| +var errRecordUnchanged = errors.New("provision: the record already says this") |
| // Offline reports whether this is an offline operator move. Nil-safe, so a |
| // record at rest answers false. |
| @@ -65,7 +95,7 @@ func offlineAt(rec *Record, id, from string) error { |
| return err |
| } |
| if rec.Move.State != from { |
| - return fmt.Errorf("%w: the offline move is %s, this step follows %s", ErrMoveConflict, rec.Move.State, from) |
| + return fmt.Errorf("%w: this offline move is %s. this step follows %s", ErrMoveConflict, rec.Move.State, from) |
| } |
| return nil |
| } |
| @@ -76,10 +106,10 @@ func offlineIs(rec *Record, id string) error { |
| return fmt.Errorf("%w: %s", ErrMoveConflict, refusedNoMove) |
| } |
| if !rec.Move.Offline() { |
| - return fmt.Errorf("%w: the record's move is not an offline move", ErrMoveConflict) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, refusedNotOffline) |
| } |
| if rec.Move.ID != id { |
| - return fmt.Errorf("%w: the record's offline move is %s", ErrMoveConflict, rec.Move.ID) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, refusedOtherOfflineMove) |
| } |
| return nil |
| } |
| @@ -104,8 +134,11 @@ func OfflineStart(id, from, to, by string, now time.Time) func(*Record) error { |
| if !moveIDShape.MatchString(id) || now.IsZero() { |
| return fmt.Errorf("%w: %s", ErrMoveConflict, refusedMalformedStart) |
| } |
| - if rec.Region == to || from != rec.Region { |
| - return fmt.Errorf("%w: the instance is already in %s", ErrMoveConflict, rec.Region) |
| + if rec.Region == to { |
| + return fmt.Errorf("%w: the instance is already in %s", ErrMoveConflict, regionWord(rec.Region)) |
| + } |
| + if from != rec.Region { |
| + return fmt.Errorf("%w: the instance is in %s, not %s", ErrMoveConflict, regionWord(rec.Region), regionWord(from)) |
| } |
| if rec.Role != "" { |
| return fmt.Errorf("%w: a %s does not move. delete it and create one in the new region", ErrMoveConflict, rec.Role) |
| @@ -123,6 +156,15 @@ func OfflineStart(id, from, to, by string, now time.Time) func(*Record) error { |
| } |
| } |
| +// regionWord is a region as a sentence names it: the home placement's empty |
| +// code reads "home". |
| +func regionWord(r string) string { |
| + if r == "" { |
| + return "home" |
| + } |
| + return r |
| +} |
| + |
| // OfflineSeal is stopping to sealing: every source box has confirmed the stop |
| // and the operator asked for the copy. It names the sealer (the one box that |
| // copies) and the source boxes whose reports the later doors wait for. |
| @@ -131,8 +173,8 @@ func OfflineSeal(id, sealer string, boxes []string) func(*Record) error { |
| if err := offlineAt(rec, id, OfflineStopping); err != nil { |
| return err |
| } |
| - if sealer == "" || len(boxes) == 0 { |
| - return fmt.Errorf("%w: a seal names the sealer and the boxes that stopped the instance", ErrMoveConflict) |
| + if sealer == "" || len(boxes) == 0 || !slices.Contains(boxes, sealer) { |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, refusedOfflineSeal) |
| } |
| rec.Move.State = OfflineSealing |
| rec.Move.Sealer = sealer |
| @@ -150,7 +192,7 @@ func OfflineSealedBy(id, sealer string, src MoveSource) func(*Record) error { |
| return err |
| } |
| if sealer == "" || rec.Move.Sealer != sealer { |
| - return fmt.Errorf("%w: the offline move is sealed by %s", ErrMoveConflict, rec.Move.Sealer) |
| + return fmt.Errorf("%w: this offline move is sealed by %s", ErrMoveConflict, rec.Move.Sealer) |
| } |
| s := src |
| if src.Files != nil { |
| @@ -172,7 +214,7 @@ func OfflineFlip(id string) func(*Record) error { |
| return err |
| } |
| if rec.Region != rec.Move.From { |
| - return fmt.Errorf("%w: the record's region is %s, not the move's source", ErrMoveConflict, rec.Region) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, offlineRegionMoved(rec.Region)) |
| } |
| rec.Region = rec.Move.To |
| rec.Move.State = OfflineLanding |
| @@ -189,7 +231,7 @@ func OfflineLandedBy(id, to string) func(*Record) error { |
| return err |
| } |
| if rec.Region != to || rec.Move.To != to { |
| - return fmt.Errorf("%w: the record's region is %s, not %s", ErrMoveConflict, rec.Region, to) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, offlineRegionMoved(rec.Region)) |
| } |
| rec.Move.State = OfflineLanded |
| return nil |
| @@ -197,21 +239,22 @@ func OfflineLandedBy(id, to string) func(*Record) error { |
| } |
| // OfflineOpenTransition is landed to open: the operator opens the target. The |
| -// region must be the target's. It stamps opened_at, which is when the instance |
| -// came back and so what the finished move's offline time is counted to. |
| +// region must be the target's. It stamps opened_at from now, which is when the |
| +// instance came back and so what the finished move's offline time is counted |
| +// to. |
| // |
| // The brief named this OfflineOpen, which is also the name of the state |
| // constant; Go has one namespace, so the two transitions that clash with a |
| // state (this one and OfflineFinishingTransition) carry a suffix. |
| -func OfflineOpenTransition(id string) func(*Record) error { |
| +func OfflineOpenTransition(id string, now time.Time) func(*Record) error { |
| return func(rec *Record) error { |
| if err := offlineAt(rec, id, OfflineLanded); err != nil { |
| return err |
| } |
| if rec.Region != rec.Move.To { |
| - return fmt.Errorf("%w: the record's region is %s, not the move's target", ErrMoveConflict, rec.Region) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, offlineRegionMoved(rec.Region)) |
| } |
| - t := time.Now().UTC() |
| + t := now.UTC() |
| rec.Move.State, rec.Move.OpenedAt = OfflineOpen, &t |
| return nil |
| } |
| @@ -233,8 +276,10 @@ func OfflineFinishingTransition(id string) func(*Record) error { |
| // sealed, landing or landed becomes reverting. From landing and landed the |
| // region goes back to the source in the same write (the flip is undone with |
| // the state, never separately). From open and finishing it refuses, because |
| -// the target already serves; from reverting it is a no-op success, so a |
| -// second abort is not an error. |
| +// the target already serves. From reverting it changes nothing and writes |
| +// nothing (updateRecord sees errRecordUnchanged and returns the record as |
| +// read), so a second abort is a success that leaves record_seq and updated_at |
| +// alone. Call it through SetRecordMove. |
| func OfflineAbort(id string) func(*Record) error { |
| return func(rec *Record) error { |
| if err := offlineIs(rec, id); err != nil { |
| @@ -242,12 +287,12 @@ func OfflineAbort(id string) func(*Record) error { |
| } |
| switch rec.Move.State { |
| case OfflineReverting: |
| - return nil |
| + return errRecordUnchanged |
| case OfflineStopping, OfflineSealing, OfflineSealed: |
| case OfflineLanding, OfflineLanded: |
| rec.Region = rec.Move.From |
| default: |
| - return fmt.Errorf("%w: the offline move is %s. it cannot be aborted once it is open", ErrMoveConflict, rec.Move.State) |
| + return fmt.Errorf("%w: this offline move is %s. %s", ErrMoveConflict, rec.Move.State, refusedOfflineAfterOpen) |
| } |
| rec.Move.State = OfflineReverting |
| return nil |
| @@ -270,7 +315,7 @@ func OfflineFinish(id string, now time.Time) func(*Record) error { |
| switch m.State { |
| case OfflineFinishing: |
| if rec.Region != m.To { |
| - return fmt.Errorf("%w: the record's region is %s, not the move's target", ErrMoveConflict, rec.Region) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, offlineRegionMoved(rec.Region)) |
| } |
| out.Outcome = OfflineOutcomeDone |
| if m.OpenedAt != nil { |
| @@ -278,11 +323,11 @@ func OfflineFinish(id string, now time.Time) func(*Record) error { |
| } |
| case OfflineReverting: |
| if rec.Region != m.From { |
| - return fmt.Errorf("%w: the record's region is %s, not the move's source", ErrMoveConflict, rec.Region) |
| + return fmt.Errorf("%w: %s", ErrMoveConflict, offlineRegionMoved(rec.Region)) |
| } |
| out.Outcome = OfflineOutcomeAborted |
| default: |
| - return fmt.Errorf("%w: the offline move is %s. it finishes from finishing or reverting", ErrMoveConflict, m.State) |
| + return fmt.Errorf("%w: this offline move is %s. it finishes from finishing or reverting", ErrMoveConflict, m.State) |
| } |
| if s := end.Sub(m.RequestedAt).Seconds(); s > 0 { |
| out.OfflineS = s |
| @@ -291,3 +336,9 @@ func OfflineFinish(id string, now time.Time) func(*Record) error { |
| return nil |
| } |
| } |
| + |
| +// offlineRegionMoved is the refusal for a record whose region is not where |
| +// the step expects it (MV-O39): something wrote the region under the move. |
| +func offlineRegionMoved(region string) string { |
| + return fmt.Sprintf("the record's region is %s, which this step does not expect. the instance changed under the move", regionWord(region)) |
| +} |
diff --git a/internal/provision/offline_move_test.go b/internal/provision/offline_move_test.go| index e0da0f0cf..238599cf9 100644 |
| --- a/internal/provision/offline_move_test.go |
| +++ b/internal/provision/offline_move_test.go |
| @@ -1,6 +1,7 @@ |
| package provision |
| import ( |
| + "bytes" |
| "context" |
| "encoding/json" |
| "errors" |
| @@ -55,7 +56,7 @@ func offlineSteps(now time.Time) []offlineStep { |
| {"sealed", func() func(*Record) error { return OfflineSealedBy(offID, offSealer, src) }, OfflineSealing}, |
| {"flip", func() func(*Record) error { return OfflineFlip(offID) }, OfflineSealed}, |
| {"landed", func() func(*Record) error { return OfflineLandedBy(offID, "uk") }, OfflineLanding}, |
| - {"open", func() func(*Record) error { return OfflineOpenTransition(offID) }, OfflineLanded}, |
| + {"open", func() func(*Record) error { return OfflineOpenTransition(offID, now.Add(30*time.Minute)) }, OfflineLanded}, |
| {"finishing", func() func(*Record) error { return OfflineFinishingTransition(offID) }, OfflineOpen}, |
| {"finish", func() func(*Record) error { return OfflineFinish(offID, now.Add(time.Hour)) }, OfflineFinishing}, |
| } |
| @@ -153,13 +154,20 @@ func TestOfflineFullLegalPathThroughTheWriter(t *testing.T) { |
| func TestOfflineRecordDiffersOnlyWhereTheMoveSaysSo(t *testing.T) { |
| for _, via := range []string{"done", "aborted after the flip", "aborted before the flip"} { |
| t.Run(via, func(t *testing.T) { |
| + // The doors call the transitions with a plain context. A counter that |
| + // is already present advances without any context asking for it, so |
| + // the record starts with one: a first write under the sequence rule |
| + // puts record_seq into presence, as on any record a writer has touched. |
| ctx := context.Background() |
| s := release.NewDirStore(t.TempDir()) |
| rec := offlineRecord(t, s) |
| - // Advance record_seq into presence first, as it is on any record a |
| - // writer has touched under the sequence rule. |
| - ctx = WithRecordSequence(ctx) |
| + if _, err := updateRecord(WithRecordSequence(ctx), s, rec.Account, rec.App, rec.Host, func(*Record) error { return nil }); err != nil { |
| + t.Fatal(err) |
| + } |
| before := offlineGet(t, s, rec) |
| + if before.RecordSeq == 0 { |
| + t.Fatal("the record starts without record_seq") |
| + } |
| beforeMap := asMap(t, before) |
| now := time.Date(2026, 10, 21, 9, 0, 0, 0, time.UTC) |
| @@ -294,7 +302,7 @@ func TestOfflineWrongIDIsRefusedEverywhere(t *testing.T) { |
| "sealed": func() func(*Record) error { return OfflineSealedBy(otherOff, offSealer, MoveSource{}) }, |
| "flip": func() func(*Record) error { return OfflineFlip(otherOff) }, |
| "landed": func() func(*Record) error { return OfflineLandedBy(otherOff, "uk") }, |
| - "open": func() func(*Record) error { return OfflineOpenTransition(otherOff) }, |
| + "open": func() func(*Record) error { return OfflineOpenTransition(otherOff, now) }, |
| "finishing": func() func(*Record) error { return OfflineFinishingTransition(otherOff) }, |
| "finish": func() func(*Record) error { return OfflineFinish(otherOff, now) }, |
| "abort": func() func(*Record) error { return OfflineAbort(otherOff) }, |
| @@ -456,6 +464,10 @@ func TestOfflineSealAndSealedByChecks(t *testing.T) { |
| t.Errorf("%s: %v", name, err) |
| } |
| } |
| + // The sealer is one of the boxes. |
| + if _, err := SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, OfflineSeal(offID, "box-z", offBoxes)); !errors.Is(err, ErrMoveConflict) || !strings.Contains(err.Error(), refusedOfflineSeal) { |
| + t.Errorf("a sealer outside the boxes: %v", err) |
| + } |
| // The seal copies the caller's slice. |
| boxes := []string{"box-a", "box-b"} |
| if _, err := SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, OfflineSeal(offID, offSealer, boxes)); err != nil { |
| @@ -544,8 +556,18 @@ func TestOfflineAbort(t *testing.T) { |
| if got.Move.State != OfflineReverting || got.Region != "" || !got.Move.Offline() || got.Move.ID != offID { |
| t.Fatalf("abort from %s: %+v region %q", c.state, got.Move, got.Region) |
| } |
| - // A second abort is a no-op success. |
| - again, err := SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, OfflineAbort(offID)) |
| + // A second abort is a success that writes nothing: the stored record |
| + // is the same bytes, record_seq and updated_at included. |
| + key := RecordKey(rec.Account, rec.App, rec.Host) |
| + rawBefore, etagBefore, err := s.Get(ctx, key) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + again, err := SetRecordMove(WithRecordSequence(ctx), s, rec.Account, rec.App, rec.Host, OfflineAbort(offID)) |
| + rawAfter, etagAfter, gerr := s.Get(ctx, key) |
| + if gerr != nil || !bytes.Equal(rawBefore, rawAfter) || etagBefore != etagAfter { |
| + t.Fatalf("second abort from %s wrote the record: %v", c.state, gerr) |
| + } |
| if err != nil || again.Move.State != OfflineReverting || again.Region != "" { |
| t.Fatalf("second abort from %s: %v %+v", c.state, err, again) |
| } |
| @@ -573,9 +595,8 @@ func TestOfflineAbort(t *testing.T) { |
| func TestOfflineFinishFromBothAndTheRegionGuard(t *testing.T) { |
| ctx := context.Background() |
| - // The request was 90 minutes ago; open stamps opened_at from the clock. |
| - now := time.Now().UTC().Add(-90 * time.Minute) |
| - steps := offlineSteps(now) |
| + now := time.Date(2026, 10, 21, 9, 0, 0, 0, time.UTC) |
| + steps := offlineSteps(now) // open at now+30m, finish at now+1h |
| // Done: offline time runs from the request to the open. |
| s := release.NewDirStore(t.TempDir()) |
| @@ -584,8 +605,8 @@ func TestOfflineFinishFromBothAndTheRegionGuard(t *testing.T) { |
| if got.LastMove.Outcome != "done" { |
| t.Fatalf("outcome %q", got.LastMove.Outcome) |
| } |
| - if got.LastMove.OfflineS < 5400 || got.LastMove.OfflineS > 5400+120 { |
| - t.Fatalf("offline_s %v, want about 5400", got.LastMove.OfflineS) |
| + if got.LastMove.OfflineS != 1800 || got.LastMove.At != now.Add(time.Hour) { |
| + t.Fatalf("offline_s %v at %v, want 1800 (request to open) at the finish", got.LastMove.OfflineS, got.LastMove.At) |
| } |
| // finish refuses a region that is not where the outcome says. |
| @@ -689,11 +710,20 @@ func TestEveryLegacyAndPhase1TransitionRefusesAnOfflineMove(t *testing.T) { |
| "MoveCommit": func() func(*Record) error { return MoveCommit(g, now) }, |
| "MoveClose": func() func(*Record) error { return MoveClose(g, now) }, |
| "MoveRevert": func() func(*Record) error { return MoveRevert(g, "r") }, |
| - "MoveMarkStuck": func() func(*Record) error { return MoveMarkStuck(g, "r") }, |
| + "MoveMarkStuck": func() func(*Record) error { return MoveMarkStuck(g, StuckBaselineMismatch) }, |
| "MoveRetry": func() func(*Record) error { return MoveRetry(g) }, |
| "MoveRollBack": func() func(*Record) error { return MoveRollBack(g, "rb_abcdefghijklmnopqrstuvwxyz", "r", now) }, |
| "guarded, real incarnation": func() func(*Record) error { return MoveGuardedAbort(gInc) }, |
| } |
| + // The guard itself refuses, whatever the transition would say after it: |
| + // this pins MoveGuard.check, not each transition's own state check. |
| + for _, st := range []string{OfflineStopping, OfflineLanding, OfflineOpen, OfflineReverting} { |
| + offRec := Record{Move: &MoveState{Kind: MoveKindOffline, ID: offID, State: st}} |
| + err := g.check(&offRec) |
| + if !errors.Is(err, ErrMoveConflict) || !strings.Contains(err.Error(), refusedOfflineKind) { |
| + t.Errorf("MoveGuard.check on an offline move in %s: %v", st, err) |
| + } |
| + } |
| // Walk one record through every offline state and try everything on it. |
| s := release.NewDirStore(t.TempDir()) |
| rec := offlineRecord(t, s) |
| @@ -759,3 +789,67 @@ func TestMoveStateOfflineIsNilSafe(t *testing.T) { |
| t.Fatal("Offline reads the wrong field") |
| } |
| } |
| + |
| +// The refusals an operator can read are the recorded sentences (copy rows |
| +// MV-O34 to MV-O43 and the existing MV-N rows), not free text, and none has an |
| +// em dash. A door maps them; this holds the provision side to its rows. |
| +func TestOfflineRefusalsAreTheRecordedSentences(t *testing.T) { |
| + now := time.Date(2026, 10, 21, 9, 0, 0, 0, time.UTC) |
| + steps := offlineSteps(now) |
| + ctx := context.Background() |
| + refuse := func(t *testing.T, s release.Store, rec Record, mut func(*Record) error, want string) { |
| + t.Helper() |
| + _, err := SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, mut) |
| + if !errors.Is(err, ErrMoveConflict) { |
| + t.Fatalf("err %v, want ErrMoveConflict", err) |
| + } |
| + if got := strings.TrimPrefix(err.Error(), ErrMoveConflict.Error()+": "); got != want { |
| + t.Errorf("refusal %q, want %q", got, want) |
| + } |
| + if strings.ContainsRune(err.Error(), '\u2014') { |
| + t.Errorf("em dash in %q", err) |
| + } |
| + } |
| + fresh := func(t *testing.T, walked int) (release.Store, Record) { |
| + s := release.NewDirStore(t.TempDir()) |
| + rec := offlineRecord(t, s) |
| + offlineWalk(t, s, rec, steps[:walked]) |
| + return s, rec |
| + } |
| + |
| + s, rec := fresh(t, 0) |
| + refuse(t, s, rec, OfflineSeal(offID, offSealer, offBoxes), refusedNoMove) |
| + refuse(t, s, rec, OfflineStart(offID, "us", "uk", "ops", now), "the instance is in home, not us") |
| + refuse(t, s, rec, OfflineStart(offID, "", "", "ops", now), "the instance is already in home") |
| + refuse(t, s, rec, OfflineStart("mv_short", "", "uk", "ops", now), refusedMalformedStart) |
| + |
| + s, rec = fresh(t, 1) |
| + refuse(t, s, rec, OfflineSeal(otherOff, offSealer, offBoxes), refusedOtherOfflineMove) |
| + refuse(t, s, rec, OfflineFlip(offID), "this offline move is offline-stopping. this step follows offline-sealed") |
| + refuse(t, s, rec, OfflineSeal(offID, "", offBoxes), refusedOfflineSeal) |
| + refuse(t, s, rec, OfflineFinish(offID, now), "this offline move is offline-stopping. it finishes from finishing or reverting") |
| + |
| + s, rec = fresh(t, 2) |
| + refuse(t, s, rec, OfflineSealedBy(offID, "box-b", MoveSource{}), "this offline move is sealed by box-a") |
| + |
| + s, rec = fresh(t, 6) |
| + refuse(t, s, rec, OfflineAbort(offID), "this offline move is offline-open. it cannot be aborted once it is open") |
| + |
| + s, rec = fresh(t, 3) |
| + if _, err := updateRecord(ctx, s, rec.Account, rec.App, rec.Host, func(r *Record) error { r.Region = "us"; return nil }); err != nil { |
| + t.Fatal(err) |
| + } |
| + refuse(t, s, rec, OfflineFlip(offID), "the record's region is us, which this step does not expect. the instance changed under the move") |
| + |
| + // A legacy move meets an offline step. |
| + s = release.NewDirStore(t.TempDir()) |
| + rec = offlineRecord(t, s) |
| + if _, err := SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, MoveRequest("uk", now)); err != nil { |
| + t.Fatal(err) |
| + } |
| + refuse(t, s, rec, OfflineSeal(offID, offSealer, offBoxes), refusedNotOffline) |
| + |
| + // And the other way: the Phase 1 guard on an offline move. |
| + s, rec = fresh(t, 1) |
| + refuse(t, s, rec, MoveGuardedCut(MoveGuard{ID: offID}), refusedOfflineKind) |
| +} |
diff --git a/internal/provision/provision.go b/internal/provision/provision.go| index 91155922b..30a4bf1b3 100644 |
| --- a/internal/provision/provision.go |
| +++ b/internal/provision/provision.go |
| @@ -953,6 +953,9 @@ func updateRecordSeeding(ctx context.Context, s release.Store, acct, app, host s |
| before.Maintenance = append(MaintenanceProjection(nil), rec.Maintenance...) |
| before.Seed = cloneSeed(rec.Seed) // deep: mut edits rec.Seed through its pointer |
| if err := mut(&rec); err != nil { |
| + if errors.Is(err, errRecordUnchanged) { |
| + return &rec, nil // the record already says it: no write, no sequence advance |
| + } |
| return nil, err |
| } |
| if !protectedFieldsEqual(before, rec) { |