amadan
plan: claim Plan 4 rev 2, answers the independent review (one P1, three P2)
Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+314
−19
docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md+314 −19
CI passed — run details
diff --git a/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md b/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md| index 38c72a4ff..9d0fa520f 100644 |
| --- a/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md |
| +++ b/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md |
| @@ -1,5 +1,7 @@ |
| # Host Lifecycle Claim, Plan 4: the park engine |
| +Rev 2 answers the independent review (one P1, three P2). |
| + |
| > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| **Goal:** add `internal/park`, a library that archives exact remote versions, waits for the late-completion horizon, acknowledges durable chores, decides and executes at-rest sweeps, and retires local artifacts under the host guard. No production path calls it. |
| @@ -8,7 +10,7 @@ |
| **Tech Stack:** Go 1.26; existing `internal/hostclaim`, `internal/hostguard`, `internal/lease`, `internal/lease/leasetest`, `internal/release`; existing `golang.org/x/sys/unix`. No new dependency. |
| -**Spec:** `docs/superpowers/specs/2026-10-02-host-lifecycle-claim-design.md`, **rev 6.14**: §2, C3, C3a, C5, C6, §5.2/§5.3, §6.2, §6.2a, §6.3, §6.4 and §13. Plan 1's roadmap assigns exactly these parking mechanisms to Plan 4. |
| +**Spec:** `docs/superpowers/specs/2026-10-02-host-lifecycle-claim-design.md`, **rev 6.15**: §2, C3, C3a, C5, C6, §5.2/§5.3, §6.2, §6.2a, §6.3, §6.4 and §13. Plan 1's roadmap assigns exactly these parking mechanisms to Plan 4. |
| **Depends on:** Plans **1, 2 and 3**. Use the real `hostclaim` package from `feat/hostclaim-library`, Plan 2's implementation on `origin/main`, and Plan 3's committed `feat/hostclaim-boxguard`. Plan 3's implementation was still being built when this plan was authored; the code below uses its published and present `Guard`, `Captured`, `With`, `Activate`, `RetireInstallation`, `locked`, `fresh`, `recordDir` and `record.retire` names. |
| @@ -24,7 +26,7 @@ |
| - **Complete discovery.** List all pages under both `instances/<host>/` and `moves/<host>/`. The existing `VersionedStore` has no delimiter API; using its full recursive version listing and deriving the first child gives the same location partition, also retaining marker-only locations. Exactly 64 hex characters followed by `/` form an incarnation location. Every other immediate child belongs to the legacy location. Never discover locations through the capped 32-entry `retired_incarnations` history. |
| - **The live set is regional.** Use the moves-owned authoritative region, holder location and generation under the same claim ETag. In that bucket, retain the installed generation, every unfinished cold-admission predecessor back to the marker, every pinned noncurrent version and every version of the marker key. For simplicity retain an entire generation containing a pinned version. Other regions' copies of the same holder location are residue. An unrelated marker disagreement returns an alert and suppresses that live location. A matching unfinished run raises no alert. An operation other than serving/free suppresses new sweeps. A stopped `parking` operation has no executing live installation, but only the mandatory authorization callback can establish the stop/fence evidence that permits its work. |
| - **Authority and leases are explicit dependencies.** `Authorize` must validate the current operation/phase, chore owner, original installation/scope and durable stop evidence or lost-box fence from Plan 3/8; for generation work it also validates moves' sealing/publication conditions. A no-op callback appears only in tests. `View` must perform complete marker/selection reads for the supplied observation and return that exact ETag. `Leases.Acquire(bucket, location)` is an **aggregate lease acquisition**: hold and renew the replica leases for every readable `instances`/`moves` location plus any moves-owned staging-work lease required for that scope. It is not permission to hold only the instance lock while mutating staging. `Check` refuses cancellation, expiry and lost ownership. `Close` stops renewal; only successful acknowledgement calls conditional `Release`, which removes only this worker's held lock version(s). Plans 5/6 supply these adapters; no production adapter is invented here. Each engine is bound to its own region's bucket; no sibling-bucket mutation (N13). |
| -- **Local identity and order.** Persist the original `hostguard.Captured{Incarnation, Stamp}` when selecting retirement work and reuse it on every retry. `RetireArtifacts(..., S)` parks only inventory entries tagged S, while the caller's captured stamp is the installation active during this operation; it changes no stamp. `RetireInstallation` persists the inactive stamp before the first artifact move, then parks all artifacts under the same flock. An inactive retry must still match the last retired stamp. Activation is still `hostguard.Activate`; never use installation retirement to clean a predecessor. |
| +- **Local identity and order.** Persist the original `hostguard.Captured{Incarnation, Stamp}` when selecting retirement work and reuse it on every retry. `RetireArtifacts(..., S)` parks only inventory entries tagged S, while the caller's captured stamp is the installation active during this operation; it changes no stamp. `RetireInstallation` durably persists the inactive stamp before the first artifact move on every attempt, including an already-inactive retry, then parks all artifacts under the same flock. An inactive retry must still match the last retired stamp. Activation is still `hostguard.Activate`; never use installation retirement to clean a predecessor. |
| - **Inventory and journal.** Inventory paths are relative to DataDir and are disjoint roots; a directory entry owns its entire subtree at the recorded stamp. Flat layouts enroll the host directory, DB/WAL/SHM, `<host>.env`, `<host>.config.env`, `<host>-sidecar.env`, generation/lineage markers and every other host-bound operational path. Nested layouts enroll the resolved instance directory. Use `RecordInventory` before materialization, including on layout changes; retire old paths before changing a populated layout. Missing inventory refuses; an explicitly empty entry can acknowledge. A never-installed box uses `AcknowledgeEmpty`, which additionally proves the local record is absent under the guard. |
| - **Local destination and R9:** `<DataDir>/.parked/<incarnation>/<host>/<retirement-id>/artifacts/<inventory-relative-path>`, with `.journal.json` beside `artifacts/`. `.parked`, DataDir and Plan 3's guard directories must be provisioned with trusted ownership; `.parked` uses setgid group `carlos`, mode **2770**, like the guard directories. Plan 6 owns that provisioning. This library uses `Mkdirat` only for descendants of the pinned `.parked` descriptor; it does not change pathname ownership/mode. Every traversal uses directory descriptors and `O_NOFOLLOW`, every artifact inspection uses `AT_SYMLINK_NOFOLLOW`, and artifact moves use **`RENAME_NOREPLACE`**. Journal temporaries use exclusive, no-follow opens and fd-based mode/fsync; journal replacement is atomic. Fsync source/destination and journal directories before completion. `EXDEV`, conflicting destinations, changed source inodes, symlinks and any other error keep the chore open. Artifacts are never deleted. |
| - **Portable build, fail-closed local operation.** Linux implements `Renameat2`; Darwin and Windows compile a local-retirement refusal stub that runs before stamp mutation. Remote parking remains portable. Do not create Go caches under `/tmp`; use the default shared cache. If the sandbox refuses writes there, use the already-existing shared on-disk `$TMPDIR/gocache`, never a per-task cache. |
| @@ -36,8 +38,9 @@ |
| 3. An exact-horizon verification refuses; a late PUT causes a new retirement event, and listing that began too early cannot be rescued by the clock moving during it. |
| 4. A stopped authoritative replica can be parked; a serving authoritative replica, unfinished chain, pinned noncurrent version or marker cannot. |
| 5. A free host with residue can hold a durable sweep chore without inventing a holder; its start and completion preserve ordinary holderless validation and quiet-after history. |
| -6. Installation retirement holds one flock through inactive-record persistence, journaled renames and completion; stamp-only `RetireInstallation` followed by `With` would be an invalid composition. |
| -7. A crash after rename recognizes the recorded destination inode, never a same-path successor; source/destination/ancestor symlinks refuse. |
| +6. Installation retirement re-establishes inactive-record durability on retries and holds one flock through persistence, journaled renames and completion; stamp-only `RetireInstallation` followed by `With` would be an invalid composition. |
| +7. A crash after rename recognizes the recorded destination inode, never a same-path successor; source/destination/ancestor symlinks refuse. Missing journal paths are rechecked on every retry. |
| +8. A serving sweep uses `session:<holder.session>`; only a holderless sweep generates an operation ID. Repeated scope binding survives JSON timestamp serialization. |
| ## File Structure |
| @@ -49,6 +52,7 @@ |
| | `internal/park/sweep.go` | moves-owned input contract, pure decision, namespace discovery and executor | |
| | `internal/park/remote_test.go`, `internal/park/sweep_test.go` | fault injection, horizon, live-set, marker and sweep tests | |
| | `internal/hostguard/retirement.go` | guarded installation-retirement callback and absent-installation callback | |
| +| `internal/hostguard/retirement_test.go` | record rename/fsync failure and inactive-retry regression | |
| | `internal/park/local.go` | inventory registration, local retirement modes and acknowledgement | |
| | `internal/park/local_linux.go` | descriptor-relative journal and no-replace renames | |
| | `internal/park/local_other.go` | unsupported-platform refusal | |
| @@ -528,6 +532,53 @@ func TestBoundScopeAndQuietAfter(t *testing.T) { |
| t.Fatal("quiet_after regressed") |
| } |
| } |
| +func TestBindSurvivesSerialization(t *testing.T) { |
| + f := newFixture(t) |
| + obs, err := hostclaim.Read(t.Context(), f.e.Claims, f.w.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + _, err = hostclaim.Update(t.Context(), f.e.Claims, obs, func(c *hostclaim.Claim) error { |
| + c.Obligations[0].Park = nil |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + scope := *f.ob.Park |
| + scope.EvidenceAt = time.Now() // Carries a monotonic reading that JSON drops. |
| + if err := f.e.Bind(t.Context(), f.w, scope); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, err := hostclaim.Read(t.Context(), f.e.Claims, f.w.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, at := range []time.Time{scope.EvidenceAt, scope.EvidenceAt.UTC()} { |
| + same := scope |
| + same.EvidenceAt = at |
| + if err := f.e.Bind(t.Context(), f.w, same); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + for _, change := range []func(*hostclaim.ParkScope){ |
| + func(p *hostclaim.ParkScope) { p.Location += "/other" }, |
| + func(p *hostclaim.ParkScope) { p.Incarnation = strings.Repeat("f", 64) }, |
| + func(p *hostclaim.ParkScope) { p.RequestLimit += time.Second }, |
| + func(p *hostclaim.ParkScope) { p.EvidenceAt = p.EvidenceAt.Add(time.Nanosecond) }, |
| + } { |
| + bad := scope |
| + change(&bad) |
| + if err := f.e.Bind(t.Context(), f.w, bad); !errors.Is(err, ErrChanged) { |
| + t.Fatal(err) |
| + } |
| + } |
| + after, err := hostclaim.Read(t.Context(), f.e.Claims, f.w.Host) |
| + if err != nil || after.ETag != before.ETag { |
| + t.Fatalf("bind changed persisted scope: %v", err) |
| + } |
| +} |
| + |
| func TestLegacyLocationDoesNotIncludeIncarnationChildren(t *testing.T) { |
| f := newFixture(t) |
| key := f.ob.Park.Location + "/0000/data" |
| @@ -648,6 +699,7 @@ func sweepFixture(t *testing.T) (*fixture, hostclaim.Observed, View, []lease.Obj |
| c.Obligations = nil |
| c.Op = hostclaim.Op{Kind: hostclaim.KindServing, Exec: "box"} |
| c.Holder.Generation = "g3" |
| + c.Holder.Session = 3 |
| c.Holder.Admissions = []hostclaim.AdmissionRecord{ |
| {Session: 2, Generation: "g2", Predecessor: "g1", Mode: "cold", Region: "ie", PredecessorRegion: "ie"}, |
| {Session: 3, Generation: "g3", Predecessor: "g2", Mode: "cold", Region: "ie", PredecessorRegion: "ie"}, |
| @@ -754,6 +806,76 @@ func TestSweepFreeHostQueuesAndAcknowledges(t *testing.T) { |
| t.Fatalf("%+v %v", obs, err) |
| } |
| } |
| +func TestSweepOperationOwnership(t *testing.T) { |
| + for _, serving := range []bool{true, false} { |
| + name := "holderless" |
| + if serving { |
| + name = "serving" |
| + } |
| + t.Run(name, func(t *testing.T) { |
| + f, obs, v, _ := sweepFixture(t) |
| + if !serving { |
| + _, err := hostclaim.Update(t.Context(), f.e.Claims, obs, func(c *hostclaim.Claim) error { |
| + c.Op = hostclaim.Op{Kind: hostclaim.KindParking, ID: "free"} |
| + return hostclaim.FreeAfterPark("free")(c) |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + f.e.View = func(_ context.Context, o hostclaim.Observed) (View, error) { |
| + x := v |
| + x.ClaimETag = o.ETag |
| + return x, nil |
| + } |
| + ids := []string{"generated-op", "chore", "retirement"} |
| + wantOp, wantCalls := "generated-op", 3 |
| + if serving { |
| + ids, wantOp, wantCalls = ids[1:], "session:3", 2 |
| + } |
| + calls := 0 |
| + f.e.NewID = func() string { |
| + if calls >= len(ids) { |
| + t.Fatal("unexpected generated operation id") |
| + } |
| + id := ids[calls] |
| + calls++ |
| + return id |
| + } |
| + authorized := false |
| + f.e.Authorize = func(_ context.Context, _ hostclaim.Observed, o hostclaim.Obligation) error { |
| + authorized = true |
| + if o.Op != wantOp { |
| + return ErrUnsafe |
| + } |
| + return nil |
| + } |
| + // Stop after queueing, before Run can acknowledge the persisted chore. |
| + f.held.failed = true |
| + if _, err := f.e.Sweep(t.Context(), f.w.Host, f.ob.Park.Incarnation, f.ob.Park.EvidenceAt, time.Second); !errors.Is(err, ErrChanged) { |
| + t.Fatal(err) |
| + } |
| + if !authorized || calls != wantCalls { |
| + t.Fatalf("authorized=%v ids=%d", authorized, calls) |
| + } |
| + queued, err := hostclaim.Read(t.Context(), f.e.Claims, f.w.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(queued.Claim.Obligations) != 1 || queued.Claim.Obligations[0].Op != wantOp { |
| + t.Fatalf("wrong chore owner: %+v", queued.Claim) |
| + } |
| + if serving { |
| + if queued.Claim.Op.Kind != hostclaim.KindServing || queued.Claim.Holder.Session != 3 { |
| + t.Fatal("serving state changed") |
| + } |
| + } else if queued.Claim.Holder != nil || queued.Claim.Op.ID != wantOp || queued.Claim.Op.Intent["park_sweep"] != "v1" { |
| + t.Fatal("wrong holderless owner") |
| + } |
| + }) |
| + } |
| +} |
| + |
| func TestSweepPageFailureAndStaleView(t *testing.T) { |
| f, obs, v, vs := sweepFixture(t) |
| before := f.b.Snapshot() |
| @@ -907,7 +1029,7 @@ func (e Engine) Bind(ctx context.Context, w Work, p hostclaim.ParkScope) error { |
| return ErrChanged |
| } |
| if o.Park != nil { |
| - if reflect.DeepEqual(*o.Park, p) { |
| + if o.Park.Location == p.Location && o.Park.Incarnation == p.Incarnation && o.Park.RequestLimit == p.RequestLimit && o.Park.EvidenceAt.Equal(p.EvidenceAt) { |
| return nil |
| } |
| return ErrChanged |
| @@ -1557,9 +1679,14 @@ func (e Engine) Sweep(ctx context.Context, host, legacyIncarnation string, evide |
| if err != nil || len(d.Candidates) == 0 { |
| return d, err |
| } |
| - op := e.NewID() |
| - if !token(op) { |
| - return d, ErrUnsafe |
| + var op string |
| + if obs.Claim.Holder != nil { |
| + op = fmt.Sprintf("session:%d", obs.Claim.Holder.Session) |
| + } else { |
| + op = e.NewID() |
| + if !token(op) { |
| + return d, ErrUnsafe |
| + } |
| } |
| chores := make([]hostclaim.Obligation, 0, len(d.Candidates)) |
| for _, x := range d.Candidates { |
| @@ -1610,7 +1737,7 @@ Expected: all tests pass. The crash table covers each of two archive copies, the |
| ### Task 3: Park local inventory under the same guard |
| -**Files:** create `internal/hostguard/retirement.go`, `internal/park/local.go`, `internal/park/local_linux.go`, `internal/park/local_other.go`, `internal/park/local_linux_test.go`. |
| +**Files:** create `internal/hostguard/retirement.go`, `internal/hostguard/retirement_test.go`, `internal/park/local.go`, `internal/park/local_linux.go`, `internal/park/local_other.go`, `internal/park/local_linux_test.go`. |
| **Interfaces:** consumes Plan 3's `Guard.With`, `Captured`, private guard/record helpers and the real `hostclaim.InventoryEntry{Layout, Paths map[string]uint64}`. Adds `RetireInstallationWith` as the callback form of `RetireInstallation`'s stamp retirement, sharing `record.retire` and record persistence; it retains the flock through the callback and also resumes when the captured installation is already inactive. The existing stamp-only method remains available. Adds `WithNoInstallation`, which permits only an authoritatively absent local record and never bypasses an existing stamp. |
| @@ -1937,6 +2064,80 @@ func TestLocalArtifactResumeRefusesReusedSource(t *testing.T) { |
| } |
| } |
| +func TestLocalMissingPathRecreatedAfterCrash(t *testing.T) { |
| + for _, kind := range []string{"file", "directory", "leaf-symlink", "ancestor-symlink", "still-missing"} { |
| + t.Run(kind, func(t *testing.T) { |
| + missing := "a-missing" |
| + if kind == "ancestor-symlink" { |
| + missing += "/child" |
| + } |
| + f := localFixtureFor(t, map[string]uint64{missing: 5, "z-present": 5}) |
| + f.write(t, "z-present", "old") |
| + crash := errors.New("crash after missing check and artifact rename") |
| + f.l.afterRename = func(string) error { return crash } |
| + if err := f.l.RetireArtifacts(t.Context(), f.host, f.captured, "local", "delete", 5); !errors.Is(err, crash) { |
| + t.Fatal(err) |
| + } |
| + f.l.afterRename = nil |
| + outside := t.TempDir() |
| + target := filepath.Join(outside, "child") |
| + if err := os.WriteFile(target, []byte("outside"), 0600); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(f.l.DataDir, "a-missing") |
| + switch kind { |
| + case "file": |
| + f.write(t, missing, "new") |
| + case "directory": |
| + if err := os.Mkdir(path, 0770); err != nil { |
| + t.Fatal(err) |
| + } |
| + case "leaf-symlink": |
| + if err := os.Symlink(target, path); err != nil { |
| + t.Fatal(err) |
| + } |
| + case "ancestor-symlink": |
| + if err := os.Symlink(outside, path); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + err := f.l.RetireArtifacts(t.Context(), f.host, f.captured, "local", "delete", 5) |
| + if kind == "still-missing" { |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + } else if err == nil { |
| + t.Fatal("acknowledged a recreated missing path") |
| + } |
| + obs, readErr := hostclaim.Read(t.Context(), f.l.Claims, f.host) |
| + if readErr != nil { |
| + t.Fatal(readErr) |
| + } |
| + if kind == "still-missing" { |
| + if len(obs.Claim.Obligations) != 0 || len(obs.Claim.Inventory["box"].Paths) != 0 { |
| + t.Fatal("missing path did not acknowledge") |
| + } |
| + } else { |
| + if len(obs.Claim.Obligations) != 1 || len(obs.Claim.Inventory["box"].Paths) != 2 { |
| + t.Fatal("refusal changed inventory or chore") |
| + } |
| + if _, err := os.Lstat(path); err != nil { |
| + t.Fatal("recreated artifact lost", err) |
| + } |
| + } |
| + if got, err := hostguard.Capture(f.l.DataDir, f.host); err != nil || got != f.captured { |
| + t.Fatal("artifact retirement changed installation", err) |
| + } |
| + if b, err := os.ReadFile(target); err != nil || string(b) != "outside" { |
| + t.Fatal("followed planted symlink", err) |
| + } |
| + if b, err := os.ReadFile(f.parked("z-present")); err != nil || string(b) != "old" { |
| + t.Fatal("lost parked artifact", err) |
| + } |
| + }) |
| + } |
| +} |
| + |
| func TestLocalResolvedDirectoryAndDestinationConflict(t *testing.T) { |
| t.Run("nested directory", func(t *testing.T) { |
| f := localFixtureFor(t, map[string]uint64{"nested/instance": 5}) |
| @@ -2076,16 +2277,79 @@ func TestLocalUntouchedBoxAcknowledgement(t *testing.T) { |
| } |
| ``` |
| +Write `internal/hostguard/retirement_test.go` exactly as follows. The injected writer uses the real descriptor-relative record writer, performs its rename, then returns an error before its directory fsync. A second failed write on the inactive retry must still suppress the callback. |
| + |
| +```go |
| +//go:build unix |
| + |
| +package hostguard |
| + |
| +import ( |
| + "errors" |
| + "testing" |
| + |
| + "golang.org/x/sys/unix" |
| +) |
| + |
| +func TestRetireInstallationWithRepairsDurability(t *testing.T) { |
| + f := newGuardFixture(t) |
| + if err := f.g.Activate(t.Context(), f.obs.Host, "uk", nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + captured, err := Capture(f.g.DataDir, f.obs.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + crash := errors.New("crash after record rename before directory fsync") |
| + callbacks, writes := 0, 0 |
| + callback := func() error { callbacks++; return nil } |
| + persist := func(d *recordDir, r record) error { |
| + writes++ |
| + if r.ActiveStamp != 0 || len(r.RetiredStamps) != 1 || r.RetiredStamps[0] != captured.Stamp { |
| + t.Fatal("incorrect inactive record", r) |
| + } |
| + return d.writeUsing(r, ".retire-crash", func(a int, old string, b int, new string) error { |
| + if err := unix.Renameat(a, old, b, new); err != nil { |
| + return err |
| + } |
| + return crash |
| + }) |
| + } |
| + for attempt := 0; attempt < 2; attempt++ { |
| + if err := f.g.retireInstallationWith(t.Context(), f.obs.Host, captured, allow, callback, persist); !errors.Is(err, crash) { |
| + t.Fatalf("attempt %d: %v", attempt, err) |
| + } |
| + if callbacks != 0 || writes != attempt+1 { |
| + t.Fatal("callback ran without durable record", callbacks, writes) |
| + } |
| + visible, err := Capture(f.g.DataDir, f.obs.Host) |
| + if err != nil || visible.Stamp != 0 { |
| + t.Fatal("did not reach rename boundary", visible, err) |
| + } |
| + } |
| + // Exercise the public entry point with its real writer after both failures. |
| + if err := f.g.RetireInstallationWith(t.Context(), f.obs.Host, captured, allow, callback); err != nil { |
| + t.Fatal(err) |
| + } |
| + if callbacks != 1 { |
| + t.Fatal("durable retry did not invoke callback") |
| + } |
| +} |
| +``` |
| + |
| - [ ] **Step 2: Run the red tests on Linux** |
| ```bash |
| go test ./internal/park -run '^TestLocal' -count=1 |
| +go test ./internal/hostguard -run '^TestRetireInstallationWithRepairsDurability$' -count=1 |
| ``` |
| -Expected: undefined `Local` and its retirement methods. These tests create and own every fixture file, including symlinks; they never touch the deployment's DataDir or guard directory. |
| +Expected: undefined `Local` and its retirement methods, and missing guard callback methods. These tests create and own every fixture file, including symlinks; they never touch the deployment's DataDir or guard directory. |
| - [ ] **Step 3: Add the guard callback forms** |
| +The cached `origin/feat/hostclaim-boxguard` at `01ed2bd9` does not expose the review fix or a durability helper. Network access prevented checking for a newer remote tip. This plan therefore implements the repair locally in `hostguard/retirement.go` using the existing `recordDir.write`, which fsyncs the replacement file and the pinned record directory. Recheck the branch before implementation; if Plan 3 has since exposed a helper for the same validated record, reuse it and retain the failure-boundary test. |
| + |
| Write `internal/hostguard/retirement.go` exactly as follows: |
| ```go |
| @@ -2098,6 +2362,11 @@ import ( |
| ) |
| func (g Guard) RetireInstallationWith(ctx context.Context, host string, captured Captured, check func(*hostclaim.Claim) error, fn func() error) error { |
| + return g.retireInstallationWith(ctx, host, captured, check, fn, (*recordDir).write) |
| +} |
| + |
| +// persist is injected by package tests to interrupt the record's durable write. |
| +func (g Guard) retireInstallationWith(ctx context.Context, host string, captured Captured, check func(*hostclaim.Claim) error, fn func() error, persist func(*recordDir, record) error) error { |
| if captured.Stamp == 0 || check == nil || fn == nil { |
| return ErrInvalidInput |
| } |
| @@ -2124,12 +2393,6 @@ func (g Guard) RetireInstallationWith(ctx context.Context, host string, captured |
| return ErrStampMismatch |
| } |
| r.retire() |
| - if err := ctx.Err(); err != nil { |
| - return err |
| - } |
| - if err := d.write(r); err != nil { |
| - return err |
| - } |
| } else { |
| n := len(r.RetiredStamps) |
| if n == 0 || r.RetiredStamps[n-1] != captured.Stamp { |
| @@ -2139,6 +2402,14 @@ func (g Guard) RetireInstallationWith(ctx context.Context, host string, captured |
| if err := ctx.Err(); err != nil { |
| return err |
| } |
| + // An inactive record can be visible after rename without being durable. |
| + // Rewrite it on retries too, before the callback may move any artifact. |
| + if err := persist(d, r); err != nil { |
| + return err |
| + } |
| + if err := ctx.Err(); err != nil { |
| + return err |
| + } |
| return fn() |
| }) |
| } |
| @@ -2702,6 +2973,18 @@ func (l Local) parkLocal(ctx context.Context, want *localJournal) error { |
| } |
| it := &want.Items[i] |
| if it.Missing { |
| + // A still-active installation can recreate an absent path between attempts. |
| + sp, sn, err := localParent(root, it.Path, false) |
| + if err == nil { |
| + _, err = localStat(sp, sn) |
| + unix.Close(sp) |
| + } |
| + if !errors.Is(err, unix.ENOENT) { |
| + if err != nil { |
| + return err |
| + } |
| + return ErrLocalJournal |
| + } |
| it.Done = true |
| continue |
| } |
| @@ -2778,7 +3061,7 @@ func (l Local) moveLocalItem(sp int, sn string, dp int, dn string, it *localItem |
| } |
| ``` |
| -The first journal records each artifact's device, inode and type before any rename. A resumed journal must match operation, obligation, retirement, mode, original captured installation, selected artifact stamp and inventory exactly. An existing destination must match that journaled identity and the source must be absent; a reused source refuses, even if its inventory stamp was not changed. This favors refusal over moving a possible successor. `Missing` artifacts were authoritatively absent under the guard at journal creation and have no object to rename. |
| +The first journal records each artifact's device, inode and type before any rename. A resumed journal must match operation, obligation, retirement, mode, original captured installation, selected artifact stamp and inventory exactly. An existing destination must match that journaled identity and the source must be absent; a reused source refuses, even if its inventory stamp was not changed. This favors refusal over moving a possible successor. `Missing` records must still be absent on every attempt, including when `Done` is already true. Reopen their parents descriptor-relatively and inspect the leaf without following symlinks; a newly present artifact or any symlink refuses retirement and leaves the inventory and chore unchanged. |
| - [ ] **Step 6: Write the portable refusal stub** |
| @@ -2799,7 +3082,7 @@ func localSupported() bool { return false } |
| - [ ] **Step 7: Run local, guard and cross-platform checks** |
| ```bash |
| -gofmt -w internal/park internal/hostguard/retirement.go |
| +gofmt -w internal/park internal/hostguard/retirement.go internal/hostguard/retirement_test.go |
| go test -race ./internal/park ./internal/hostguard ./internal/hostclaim -count=1 |
| go vet ./internal/park ./internal/hostguard ./internal/hostclaim |
| GOOS=darwin GOARCH=amd64 go build ./internal/park ./internal/hostguard |
| @@ -2861,6 +3144,15 @@ Plans 5–7 remain behind `CARLOS_HOST_CLAIM` until Plan 8 publishes the shared |
| ## Self-review |
| +Rev 2 resolves the independent review's findings: |
| + |
| +| Finding | Change and regression | |
| +|---|---| |
| +| P1 inactive retry lacks durability | `RetireInstallationWith` rewrites the validated inactive record before each callback; `TestRetireInstallationWithRepairsDurability` interrupts after record rename, repeats the failure on an inactive retry, then resumes through the public method | |
| +| P2 missing journal path recreated | Reopen and recheck each `Missing` path on every attempt; `TestLocalMissingPathRecreatedAfterCrash` covers files, directories, leaf/ancestor symlinks and continued absence while preserving the active stamp | |
| +| P2 serving sweep op owner | Serving chores use the holder session; `TestSweepOperationOwnership` asserts authorization, persisted ownership and generated-ID counts for serving and holderless sweeps | |
| +| P2 scope equality after JSON | Compare fields explicitly and `EvidenceAt` with `time.Time.Equal`; `TestBindSurvivesSerialization` repeats a monotonic timestamp and UTC representation and refuses any changed scope field | |
| + |
| | Requirement | Evidence | |
| |---|---| |
| | R1 library, no production caller | File Structure; Task 4 call-surface check; rollout note | |
| @@ -2888,9 +3180,12 @@ Plans 5–7 remain behind `CARLOS_HOST_CLAIM` until Plan 8 publishes the shared |
| | §5 touched and free | existing `WriteSet`/`ToParking` unchanged, durable chores block free; rollout caller contracts | |
| | §13 S3 facts | production VersionedStore interface plus its existing fake/wire contract; no bare-key primitive | |
| -Plan-author verification: all **10 complete Go blocks** were extracted from this Markdown, and its exact claim-edit script was executed against the real Plan 1 source in on-disk scratch. The resulting claim file matched the compiled version byte-for-byte after gofmt. An overlay combined those files with `origin/main` storage code, the real `feat/hostclaim-library` package and the present Plan 3 guard implementation, without creating implementation files in this repository. The extracted park test binary compiled with `-race` and all its remote, sweep and local tests passed. The complete hostclaim test suite also passed with the additive scope changes. Tests-only overlays failed with the expected missing scope and engine symbols before implementation. |
| +Original plan-author verification: all **10 complete Go blocks** were extracted from this Markdown, and its exact claim-edit script was executed against the real Plan 1 source in on-disk scratch. The resulting claim file matched the compiled version byte-for-byte after gofmt. An overlay combined those files with `origin/main` storage code, the real `feat/hostclaim-library` package and the present Plan 3 guard implementation, without creating implementation files in this repository. The extracted park test binary compiled with `-race` and all its remote, sweep and local tests passed. The complete hostclaim test suite also passed with the additive scope changes. Tests-only overlays failed with the expected missing scope and engine symbols before implementation. |
| The extracted park, hostguard and hostclaim packages built successfully for Darwin/amd64, Windows/amd64 and Linux/arm64; Linux code compiled and ran through the race test binary. Local retirement and guard code additionally passed their independent test/vet checks in the reviewer's scratch workspace. The default Go cache was not writable in this sandbox, so verification used the already-existing shared on-disk `$TMPDIR/gocache`. Test compilation used `-vet=off` for the virtual package directories and then ran the resulting binary directly; that is not a claim that the full repository vet/test gate passed. Task 4's full implementation-branch gate remains required. |
| A separate **gpt-6-astra** authority review, used because Opus was unavailable, checked the code and final plan. Its findings about stopped authoritative replicas, foreign `@root` chores, all marker-key versions, aggregate lease ownership, authorization rechecks and empty boxes were addressed and rechecked. The final review reported no blockers. Production lease/evidence/moves adapters, caller wiring, provisioning and activation remain explicitly with the later plans. This planning session changed only this Markdown file and staged or committed nothing. |
| +Rev 2 verification: extracted all **11 complete Go blocks** and applied the exact claim-edit script to a scratch checkout of cached `origin/feat/hostclaim-boxguard` (`01ed2bd9`, including the storage and claim dependencies). `go test ./internal/park ./internal/hostguard ./internal/hostclaim -count=1`, the same suite with `-race`, and `go vet` for those packages all passed. All three packages built for Darwin/amd64, Windows/amd64 and Linux/arm64. Restoring each reported defect separately made its new regression fail; the revised sources were then restored and checked against the Markdown. Verification used the existing shared on-disk cache after the default cache returned a read-only-filesystem error. The full repository gate remains an implementation task. |
| + |
| +A separate **gpt-6-astra** authority review of the rev 2 changes, used because Opus was unavailable, reported no blockers. The cached Plan 3 branch had no durability repair helper, and network access prevented checking a newer remote tip; Task 3 therefore includes the local durable-rewrite implementation and the instruction to reuse a suitable Plan 3 helper when available. Only this plan was changed; nothing was staged or committed. |