plans: moves Phase 1 self-review fixes (private storage from the manifest, console edges struct name, ErrNoCgroup, create-only Put, local park destination, operator test helper)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3 files changed,
+9
−9
docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md+5 −5docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md+3 −3docs/superpowers/plans/2026-10-06-moves-phase1-5-cleanup-and-parks.md+1 −1
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md b/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md| index baf615d4c..88047b38c 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md |
| @@ -767,10 +767,10 @@ Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>" |
| - Test: `internal/console/apiinstancemove_test.go` (extend `TestAPIInstanceMoveVerbs`; add `TestMoveDoorRefusesWithoutClaimWrite`, `TestMoveMutationsAreOperatorOnly`) |
| **Interfaces:** |
| -- Consumes: `hostclaim.Read`, `hostclaim.Update`, `hostclaim.BeginMove`, `hostclaim.Incarnation`, `moves.NewMoveID`, `s.store.IsOperator`, `s.cliUser`, `objstore.GetIn`, `provision.GetManifest`. This task creates `internal/moves/floor.go` with the one constant `const ProtocolVersion = 1` and adds `MovesProtocol int \`json:"moves_protocol"\`` to the console's edges struct in `instancesplacement.go`; Task 12 fills the file (the floor object, the box's writer, the activation refusal). |
| +- Consumes: `hostclaim.Read`, `hostclaim.Update`, `hostclaim.BeginMove`, `hostclaim.Incarnation`, `moves.NewMoveID`, `s.store.IsOperator`, `s.store.CLITokenAddress`, `provision.GetManifest` (regions and `PrivateStorage`). This task creates `internal/moves/floor.go` with the one constant `const ProtocolVersion = 1` and adds `MovesProtocol int \`json:"moves_protocol"\`` to the console's edges struct in `instancesplacement.go`; Task 12 fills the file (the floor object, the box's writer, the activation refusal). |
| - Produces: `(s *Server) moveOperatorGate(w, r) (acct *Account, app, host string, rec *provision.Record, addr string, ok bool)`; the door's refusal sentences (copy ids below). |
| -The door's order (§7.1, K2): `moveGate` (class rails), operator (`IsOperator`, 404 otherwise), body, region validity and `placements`, the manifest's `regions` when non-empty, a home with replicas (`len(rec.Replicas) > 0`), private storage bound to the record's environment (`objstore.GetIn(..., rec.Environment)` returning a record whose `Declared != nil && Declared.Private`: read `internal/objstore` for the field that names a private store at implementation time and use it; the refusal is M-C17), then the claim: `hostclaim.Read`; absent or `Holder == nil` → 409 "the instance has no host claim yet. it is adopted on its next reconcile pass"; holder incarnation ≠ `hostclaim.Incarnation(acct, app, host, rec.CreatedAt)` → 409; `Op.Kind != KindServing` → 409 naming the kind; the target box: the lexically-first box publishing the target placement in `readEdgesFiles` (T8 is the target's own check); `hostclaim.Update(ctx, s.cfg.Releases, obs, hostclaim.BeginMove(id, addr, obs.Claim.Op.Exec, targetBox, to, now))`; then `provision.SetRecordMove(... MoveStartTransition ...)`. If the record write fails after the claim CAS, the door answers 502 and logs; the console pass of Plan 5 Task 7 finds a `moving` claim with no record move and releases it (noted there). |
| +The door's order (§7.1, K2): `moveGate` (class rails), operator (`IsOperator`, 404 otherwise), body, region validity and `placements`, the manifest's `regions` when non-empty, a home with replicas (`len(rec.Replicas) > 0`), private storage (the instances manifest's `PrivateStorage`; the refusal is M-C17), then the claim: `hostclaim.Read`; absent or `Holder == nil` → 409 "the instance has no host claim yet. it is adopted on its next reconcile pass"; holder incarnation ≠ `hostclaim.Incarnation(acct, app, host, rec.CreatedAt)` → 409; `Op.Kind != KindServing` → 409 naming the kind; the target box: the lexically-first box publishing the target placement in `readEdgesFiles` (T8 is the target's own check); `hostclaim.Update(ctx, s.cfg.Releases, obs, hostclaim.BeginMove(id, addr, obs.Claim.Op.Exec, targetBox, to, now))`; then `provision.SetRecordMove(... MoveStartTransition ...)`. If the record write fails after the claim CAS, the door answers 502 and logs; the console pass of Plan 5 Task 7 finds a `moving` claim with no record move and releases it (noted there). |
| Copy for the gate (all pending review, Plan 6 Task 7): M-C17 "<host> uses private storage. Instances with private storage cannot move yet."; new strings "a home with replicas does not move. remove its replicas first", "region <to> is not one of this app's regions: <list>", "the instance has no host claim yet. it is adopted on its next reconcile pass", "the instance is under a <kind> operation. wait for it to finish". |
| @@ -873,7 +873,7 @@ func TestMoveDoorRefusesWithoutClaimWrite(t *testing.T) { |
| } |
| ``` |
| -`operatorInstanceFixture` and `writeServingClaim` are test helpers added to `apiinstancemove_test.go`; `operatorInstanceFixture` calls `instanceFixture` then marks the token's address an operator through the same store call the existing operator tests use (`grep -n "makeOperator\|SetOperator\|operators" internal/console/*_test.go` names it; use that helper, do not add a second). |
| +`operatorInstanceFixture` and `writeServingClaim` are test helpers added to `apiinstancemove_test.go`; `operatorInstanceFixture` calls `instanceFixture` then marks the token's address an operator with `fx.st.SetOperator(addr, true)` (`store.go:1952`, the call `alertwatch_test.go:52` and `account_test.go:250` already make), where `addr` is the address `instanceFixture` signed the token for. |
| Update `TestAPIInstanceMoveVerbs` to use `operatorInstanceFixture`, the `moves_protocol` field on its edges file, and a serving claim written before the first request. |
| @@ -965,7 +965,7 @@ func (s *Server) handleAPIInstanceMoveRequest(w http.ResponseWriter, r *http.Req |
| jsonError(w, http.StatusBadRequest, "region "+s.homeRegionOut(to)+" is not one of this app's regions: "+strings.Join(m.Regions, ", ")) |
| return |
| } |
| - if st, _, err := objstore.GetIn(ctx, s.cfg.Releases, acct.Sqid, app, rec.Environment); err == nil && st != nil && objstore.IsPrivate(st) { |
| + if m, err := provision.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app); err == nil && m.PrivateStorage { |
| jsonError(w, http.StatusBadRequest, host+" uses private storage. Instances with private storage cannot move yet.") |
| return |
| } |
| @@ -1048,7 +1048,7 @@ func moveTargetBox(ctx context.Context, s release.Store, to, from string) (box s |
| } |
| ``` |
| -`readEdgesFiles`'s struct in `instancesplacement.go` gains `MovesProtocol int \`json:"moves_protocol"\`` (Task 12 adds the writer's side). `objstore.IsPrivate` is whatever predicate `internal/objstore` already uses to say a store is private (`grep -n "Private" internal/objstore/*.go`); if none is exported, export the existing one rather than writing a second. `handleAPIInstanceMoveCut` and `handleAPIInstanceMoveAbort` switch to `moveOperatorGate`. In `authcli.go`, the three mutation routes keep `svcrole.Allow(svcrole.Instance)` only so a service token meets the operator 404 rather than the role 403 leak; add `s.cliRoute("POST /api/cli/apps/{account}/{app}/instances/{host}/move/resolve", svcrole.Allow(svcrole.Instance), s.handleAPIInstanceMoveResolve)` with a handler that, for this plan, runs `moveOperatorGate` then answers 409 `"nothing to resolve yet"`; Plan 4 Task 10 and Plan 6 Task 2 give it its body. Imports: `crypto/rand`, `slices`, `github.com/carlosframework/platform/internal/hostclaim`, `.../internal/moves`, `.../internal/objstore`, `.../internal/release`. |
| +`edgesFilePlacementView` (`instancesplacement.go:38`, what `readEdgesFiles` decodes) gains `MovesProtocol int \`json:"moves_protocol"\`` (Task 12 adds the writer's side). Private storage is the instances manifest's `PrivateStorage` (`provision.go:253`, the `private_storage` field), read through the `GetManifest` call the regions check already makes: one read, two refusals. `handleAPIInstanceMoveCut` and `handleAPIInstanceMoveAbort` switch to `moveOperatorGate`. In `authcli.go`, the three mutation routes keep `svcrole.Allow(svcrole.Instance)` only so a service token meets the operator 404 rather than the role 403 leak; add `s.cliRoute("POST /api/cli/apps/{account}/{app}/instances/{host}/move/resolve", svcrole.Allow(svcrole.Instance), s.handleAPIInstanceMoveResolve)` with a handler that, for this plan, runs `moveOperatorGate` then answers 409 `"nothing to resolve yet"`; Plan 4 Task 10 and Plan 6 Task 2 give it its body. Imports: `crypto/rand`, `slices`, `github.com/carlosframework/platform/internal/hostclaim`, `.../internal/moves`, `.../internal/release`. |
| - [ ] **Step 4: Run tests to verify they pass** |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md b/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md| index 3a4c3b546..4bfe0dc82 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md |
| @@ -580,7 +580,7 @@ func (a *Service) ReconstructStopEvidence(ctx context.Context, host string, dbPa |
| // 1. Kill and drain the tree. |
| c, err := a.cgroups.OpenCleanup(host) |
| switch { |
| - case errors.Is(err, cgroup.ErrNoGroup): // the group never existed or was removed: an empty tree |
| + case errors.Is(err, cgroup.ErrNoCgroup): // the group never existed or was removed: an empty tree |
| rep.Steps[moves.StepTree] = moves.Check{OK: true, Reason: "no cgroup"} |
| case err != nil: |
| return rep, err |
| @@ -652,7 +652,7 @@ func dbPathsWithSidecars(paths []string) []string { |
| } |
| ``` |
| -(`readProcs` parses one pid per line; `cgroup.ErrNoGroup` is whatever sentinel `openCleanup` returns for a missing directory; name it from `cleanup_linux.go`. The path arithmetic for the agent's own leaf: `Manager.Root()` is the delegated subtree; the agent's leaf is the directory `ParseSelfCgroup` names under `/sys/fs/cgroup`; compute `filepath.Join("/sys/fs/cgroup", leaf, "cgroup.procs")` and test with a fixture.) `stopreconstruct_other.go` returns `ErrStopEvidenceUnsupported`. The cut's step 5 (Task 5) adds `StepReplica` after the checkpoint. |
| +(`readProcs` parses one pid per line; `cgroup.ErrNoCgroup` is `cleanup.go:6`'s sentinel for a missing host cgroup. The path arithmetic for the agent's own leaf: `Manager.Root()` is the delegated subtree; the agent's leaf is the directory `ParseSelfCgroup` names under `/sys/fs/cgroup`; compute `filepath.Join("/sys/fs/cgroup", leaf, "cgroup.procs")` and test with a fixture.) `stopreconstruct_other.go` returns `ErrStopEvidenceUnsupported`. The cut's step 5 (Task 5) adds `StepReplica` after the checkpoint. |
| - [ ] **Step 4: Run**: `nice -n 10 go test ./internal/activator ./internal/lease ./internal/moves -count=1 -p 1; GOOS=darwin go build ./...` → PASS. |
| - [ ] **Step 5: Commit**: `git commit -m "activator: ReconstructStopEvidence from cgroup, proc and lease state (§5.3)" ...` |
| @@ -688,7 +688,7 @@ func dbPathsWithSidecars(paths []string) []string { |
| - `runCut(ctx, s, w, rec, tgt) error`: the phase loop of §7.4: |
| - `requested`: fresh target and source reports (`moves.Ready` with the last round's delta) and `move.inputs` to pin from the target report → `MoveAdmit(g, now, budget, moves.CutDeadline(now, budget), inputs)`; stale → `MoveBackToReady`. |
| - `admitted`: re-check inputs against current bundle etags (`appconfig.LoadMergedHost` etag), overlay etag, release and launch report digest; mismatch → `MoveBackToReady`; else `MoveCutPhase(admitted → stopping)`. |
| - - `stopping`: `SetHold(host, "moving", id)`; fence (`cfg.Fence`); wait `TimersDrained`; `cfg.Evidence`; incomplete → retry next pass until `cut_deadline`, then `MoveRevert(g, "stop evidence budget")` and `hostclaim.Revert`; complete → write `stopped-source.json` create-only (K13: through `lease.VersionedStore.PutIfAbsent` on the deployment bucket when the store is S3, and `release.Store.Put` with an empty `ifMatch` after a `Get` that must return `ErrNotExist` for a dir store; `moveCfg.PutEvidence func(ctx, key string, body []byte) error` wraps the choice; an existing object with equal `Host`, `OpID` and `Role` is a replay and fine) → `MoveCutPhase(stopping → stopped)`. |
| + - `stopping`: `SetHold(host, "moving", id)`; fence (`cfg.Fence`); wait `TimersDrained`; `cfg.Evidence`; incomplete → retry next pass until `cut_deadline`, then `MoveRevert(g, "stop evidence budget")` and `hostclaim.Revert`; complete → write `stopped-source.json` create-only (K13: `release.Store.Put(ctx, key, body, "")` is create-only on both the S3 and the dir store, `store.go:38`, and loses with `release.ErrCASConflict`; on a conflict read the existing object, and equal `Host`, `OpID` and `Role` is a replay and fine) → `MoveCutPhase(stopping → stopped)`. |
| - `stopped`: `Checkpoint` (final `SyncAndWait`); `N = NewestL0`; `Replication`, `NextDue` from the row; timers snapshot → `move-snapshots/<host>/<id>/timers.json` (own bucket, create-only) and `TimersPrefix` (target, `PutVersioned`); `files.plain` via `archive.PlainDigest`; `MoveRecordSource`; `hostclaim.Update(ExecToNone(id, box, stoppedKey, JournalEvidenceSource))` and `SetJournal(cut_session = holder.session)` in the same transition; `MoveCutPhase(stopped → staged)` is written only after the final round below succeeds, so the phase `stopped` repeats the mirror until it does: |
| - the final round: `MirrorPrefix` into `replica/` under the lease; `Deferred > 0` → count in the journal (`moveLedger.DeferredRounds`), three → `MoveRevert`; else seal = `round.Versions` for every current key plus the files and timers versions, `SrcCovered = (N == LocalPosition)`, `FromDB = true` when the stopped database file exists; `SetJournal(seal)` and `SetMovePhase(moving, "" → sealed)` in one claim CAS; `MoveCutPhase(stopped → staged)`. |
| - `staged`: final archive from the stopped directory: `archive.Build` + `Seal` → `move-snapshots/<host>/<id>/files-<sealed>.sealed` create-only (`PutIfAbsent`), manifest beside it, digests into `MoveSource.Files` (`MoveCutPhase(staged → archived, stamp)`); copy into `FilesPrefix` with `PutVersioned`, adding the versions to the seal (`SetJournal(seal)` again, same `N`). |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-5-cleanup-and-parks.md b/docs/superpowers/plans/2026-10-06-moves-phase1-5-cleanup-and-parks.md| index 4e3af7113..e2b571166 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-5-cleanup-and-parks.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-5-cleanup-and-parks.md |
| @@ -94,7 +94,7 @@ Tests: `TestStagingParkAcknowledgesAtSealBoundPlusH` (Review Focus 1, witness `w |
| **Files:** modify `internal/park/local.go` (destination `.parked/superseded-<op>/<host>/<retirement>/` for obligations whose op is a move or rollback id), `internal/park/local_linux_test.go`. |
| -**Interfaces:** `park.Local.RetireArtifacts` and `RetireInstallation` choose the destination by the obligation's op: a `park-local` whose `Op` has `moves.OpIDKind(op) != ""` lands under `.parked/superseded-<op>/<host>/<retirement>/`; everything else keeps today's path. (`internal/park` importing `internal/moves` would invert the dependency: put `OpIDKind`'s regexp in `park` as `supersededOp(op string) bool` with the same shape, and a test in `moves` asserting the two regexps are byte-equal.) |
| +**Interfaces:** `park.Local.RetireArtifacts` and `RetireInstallation` choose the destination by the obligation's op: today `parkLocal` walks `.parked/<incarnation>/<host>/<retirement>/artifacts/` (`local_linux.go:196-201`, `localWalk(parked, []string{want.Incarnation, want.Host, want.Retirement}, true)`); for a `park-local` whose `Op` has `moves.OpIDKind(op) != ""` the first segment becomes `"superseded-" + op` so the event lands under `.parked/superseded-<op>/<host>/<retirement>/artifacts/`; everything else keeps today's path. (`internal/park` importing `internal/moves` would invert the dependency: put `OpIDKind`'s regexp in `park` as `supersededOp(op string) bool` with the same shape, and a test in `moves` asserting the two regexps are byte-equal.) The journal `localJournal` gains no field: the op is already in `want.Op`. |
| - [ ] Test, implement, commit: `git commit -m "park: local retirements of move operations land under .parked/superseded-<op>/ (§6.4)" ...` |