| 1 | | R4 finding | Status | Assessment at `80647eeb` | |
| 2 | |---|---|---| |
| 3 | | 1. Target-selection integration | Partly | Adapter and signatures are now specified; the HTTP fixtures cannot satisfy it, and source selection is incomplete. | |
| 4 | | 2. Early `moves.Check` / strict-spawn fake | Partly | Earlier producer and mock seam added; Task 11 still redeclares `Check`. | |
| 5 | | 3. Signing timestamp | Resolved | The guard’s captured timestamp reaches the signer; pause-across-expiry coverage is specified. | |
| 6 | | 4. Generation-source prefix | Resolved | Separator normalization and a validated-claim fixture address it. | |
| 7 | | 5. Recovery entry points | Partly | Units check a marker, but its asynchronous publication leaves an unsafe recovery window. | |
| 8 | | 6. Privileged materialization / floor persistence | Partly | Floor moved to `/data`; staging relay added, but its error classification and authority checks are incomplete. | |
| 9 | | 7. Default-off behavior | Partly | Ordinary S3 mirroring and console publication are corrected; system-apply dormancy remains contradictory. | |
| 10 | | 8. Archive verification | Resolved | Decoded-tar hashing, sealed digest and terminal authentication are addressed. A separate error-path leak remains. | |
| 11 | | 9. Invalid fixtures | Partly | Earlier bucket, PUT and claim-location defects are corrected; other fixtures still fail. | |
| 12 | | 10. Flat spawn environment | Resolved | Actual spawn composition and optional purge availability are represented. | |
| 13 | | 11. Source readiness probe | Resolved | Explicit cache bypass and a cached-success/failed-wake test are specified. | |
| 14 | | 12. Readiness input producers | Partly | Version-aware loader and archive references added; T3 still contains conflicting loader instructions. | |
| 15 | | 13. Private-storage refusal | Resolved | Checks the instance’s actual binding, fails closed and repeats the check in the worker. | |
| 16 | | 14. Copy inventory | Resolved | Includes M-C1–M-C22 and explicitly imports the copy record into the implementation branch. | |
| 17 | | 15. Legacy/guarded-transition instructions | Resolved | The cited replacement instructions are corrected. A new guarded-call signature error remains. | |
| 18 | |
| 19 | Reviewed the pinned plans, moves spec rev **7.21**, claim rev **6.30** at `73b1b9dc`, and prior reviews. **Fetching was attempted but failed:** the sandbox forbids writing `FETCH_HEAD`; the subsequent remote lookup failed DNS resolution. Source comparisons therefore use cached `origin/main` **`1c43c2ce82f621c6ac84a623db54f0ef63075603`**, whose freshness I cannot establish. No files changed, hosts contacted or tests run. |
| 20 | |
| 21 | **Findings** |
| 22 | |
| 23 | 1. **P1 — The task sequence contains definite build failures.** |
| 24 | Task 3 calls undefined `testHolderKey` at [Plan 1:803](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:803); it exists neither on the inspected main nor in an earlier task. Task 1 creates `moves.Check`, then Task 11 declares it again at [Plan 1:3217](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3217). Task 14 calls `MoveGuardedCut(g, now)` at [Plan 1:3543](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3543), but Task 2 defines `MoveGuardedCut(g MoveGuard)`. |
| 25 | |
| 26 | Define the fixture helper, retain one `Check` declaration and correct the cut call. Task 14 also needs its own or a shared `instanceStoreEnv`: Task 4’s console-local helper is not accessible from package `host`. |
| 27 | |
| 28 | 2. **P1 — The new root staging relay lacks an operation-authority check.** |
| 29 | [Plan 1:2551](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2551) directs root to read `{acct, app, host, version, op}` and call `MaterializeHeld`, which calls `PointLive`. It specifies no verification that the request still names the current operation, incarnation, assigned box and pinned release. The `pub` parameter has no stated verification role. |
| 30 | |
| 31 | A request queued for operation A can run after A is aborted or replaced, repointing the host to A’s build. Request-controlled path components also reach main’s filesystem joins without a specified validation contract. Require validation immediately before the filesystem effect, safe request/path handling, and tests for stale-operation and traversal requests. Keep authority validation in an appropriate host-side layer or callback to avoid introducing a `release`/`hostclaim` import cycle. |
| 32 | |
| 33 | 3. **P1 — The recovery marker still permits execution below the protocol floor.** |
| 34 | [Plan 1:3441](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3441) has the agent publish participation and `hold-prev` asynchronously. Nothing establishes that publication before the console commits a move. A crash after that commit but before the next agent pass leaves recovery free to execute old `carlos.prev` before the agent starts. |
| 35 | |
| 36 | The marker is also removed for nonparticipants even when an armed floor remains and `.prev` is below it. That does not implement §11’s activation floor. The proposed unit test starts with the marker already present and misses both cases. Enforce the floor at executable selection and test the admission-to-marker crash boundary. |
| 37 | |
| 38 | 4. **P2 — Default-off system-apply behavior contradicts the stated dormant boundary.** |
| 39 | [Plan 1:3445](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3445) says absence of agent-written files makes `floorAllows` dormant. But the preceding contract also reads registry holds and the remote floor; the applier hooks are unconditional at [Plan 1:3517](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3517). |
| 40 | |
| 41 | The proposed switch-off test deliberately uses a held registry row, but only checks that the agent writes no files. Applying the specified activation rule to that same fixture refuses recovery. Define and test the root activation boundary, including the separate case of a previously armed, persistent floor. |
| 42 | |
| 43 | 5. **P2 — The staging fallback misses the production read-only-mount error.** |
| 44 | [Plan 1:2551](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2551) recognizes only `errors.Is(err, fs.ErrPermission)`. `ProtectSystem=strict` can produce `EROFS`, which is not `fs.ErrPermission`. Main already handles this distinction in `release.stageBlocked` (`internal/release/adopt.go:250`), covering `EROFS`, `EPERM` and `EACCES`. |
| 45 | |
| 46 | Reuse that classification. The proposed `chmod(0555)` fixture tests discretionary permissions, not read-only mounts; inject `EROFS` explicitly. |
| 47 | |
| 48 | 6. **P2 — Task 4’s supplied HTTP fixtures cannot reach their expected outcomes.** |
| 49 | [Plan 1:1022](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:1022) supplies minimal edges files, while `selectMoveBoxes` now requires a complete live snapshot and verified eligible identities. Matching status snapshots, signed identity/binding fixtures and eligibility inputs are missing. |
| 50 | |
| 51 | Consequently, the claimless test returns incomplete-fleet refusal before its expected claim refusal; the admitted case cannot select its target. Adding only protocol and serving-claim data does not fix this. Reuse a complete production-shaped fixture in the HTTP tests, not just in the separate selector unit test. |
| 52 | |
| 53 | 7. **P2 — Selection permits an absent source.** |
| 54 | At [Plan 1:1319](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:1319), both regions reject more than one candidate, but only the target must produce a candidate. Zero source candidates passes; the source candidate is never bound to the claim’s executor. The source-region floor check can also pass vacuously when no source file exists. |
| 55 | |
| 56 | Require the source required by §7.1 to be present and consistent with the claim. Add missing-source and different-executor tests. |
| 57 | |
| 58 | 8. **P2 — Versioned receipts leave the multipart receipt-match branch unspecified.** |
| 59 | [Plan 1:1555](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:1555) gives concrete changes for equal source/destination ETags. Main has another early return for matching receipts when those ETags differ—the normal multipart-source case. |
| 60 | |
| 61 | That branch also needs to require `DstVersionID` and populate `round.Versions`. Otherwise the specified edits can skip copying while omitting the object’s version from the round. Add a two-round multipart fixture and an old receipt without a version; the supplied single-part case cannot catch this. |
| 62 | |
| 63 | 9. **P2 — Readiness can combine different input generations, and T3’s provenance wiring remains contradictory.** |
| 64 | `Ready` at [Plan 1:3276](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3276) checks age and passing flags, but does not compare the target’s checked files/environment inputs with the current source report. A passing target report for archive A can remain fresh after the source publishes archive B, allowing promotion or auto-cut before B passes T5. |
| 65 | |
| 66 | Separately, Task 7b requires `LoadMergedHostVersions`, while T3 at [Plan 1:3138](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3138) still specifies `LoadMergedHost`, which cannot return both bundle ETags. Require matching report input identities, use the version-aware loader for the actual render, and test two fresh reports with different digests. T3’s specified reporting of host-overlay shadowing also needs an explicit producer and assertion. |
| 67 | |
| 68 | 10. **P2 — Interim abort cannot recover from its claim-release crash boundary.** |
| 69 | [Plan 1:3540](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3540) releases the moving claim before clearing the record. A crash between those writes leaves an aborting record and a serving claim. Task 3’s `ReleaseMove` requires the matching moving operation, so replay cannot complete as specified. |
| 70 | |
| 71 | Provide operation-bound completion evidence or an explicitly safe idempotent completion path. `TestInterimAbortReleasesTheClaim` needs a crash/retry case between these two writes. |
| 72 | |
| 73 | 11. **P2 — The dir-store staging-lease fallback cannot satisfy the signing guard.** |
| 74 | [Plan 1:3538](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3538) falls back to ordinary `lease.Acquire` on the own bucket. Its 15-minute lease cannot provide the required 15-minute signature lifetime plus skew/margin, so the first guarded write is unsafe. Using each participant’s own bucket also fails to identify one shared coordination location. |
| 75 | |
| 76 | Specify a shared fallback with the staging TTL, or explicitly refuse that deployment mode. Test its first guarded write. |
| 77 | |
| 78 | 12. **P2 — Archive verification leaks its producer on early errors.** |
| 79 | [Plan 1:2309](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2309) launches `Open` writing to an `io.Pipe`, but early manifest, tar or decoder errors never close the pipe reader. The producer can remain blocked writing the remaining archive. Repeated T5 failures accumulate blocked goroutines and retained resources. |
| 80 | |
| 81 | Close/cancel the producer on every return and verify termination using an early mismatch in a multi-chunk archive. |
| 82 | |
| 83 | 13. **P2 — Several commit commands omit files required by their own task.** |
| 84 | Examples: Task 1 omits `check.go` at [Plan 1:254](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:254); Task 4 omits its moves prerequisite and console entry-point changes; Task 6 omits its `internal/lease` implementation changes at [Plan 1:1808](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:1808); Task 13 omits units and `movesfloor` files; Task 14 stages only `movespass` files at [Plan 1:3553](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3553). |
| 85 | |
| 86 | A passing working-tree test does not make those commits buildable. Correct the file sets and validate the committed tree at each task boundary. |
| 87 | |
| 88 | 14. **P3 — Competing instructions remain in the index and task bodies.** |
| 89 | The index still identifies older spec/claim revisions at [Index:9](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-0-index.md:9). Task 4 retains older private-policy/selection directions; Task 9’s interface description disagrees with its revised purge treatment; Task 13 prescribes `readEdgesSnapshot` at [Plan 1:3471](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3471), while Task 4 now uses `liveEdges` and full completeness. |
| 90 | |
| 91 | Reconcile these rather than marking the dispositions wholly complete. Also identify `release.LiveVersion` as reuse: it already exists on main at `internal/release/versions.go:290`. |
| 92 | |
| 93 | The task-by-task assessment below is static. “Meaningful tests” means the proposed assertions discriminate the intended behavior; it does **not** mean a red/green run was observed. |
| 94 | |
| 95 | | Task | Main interfaces, ordering and tests | |
| 96 | |---|---| |
| 97 | | 0 — Switch | New configuration is straightforward. Parser tests do not establish actual console/agent wiring. | |
| 98 | | 1 — IDs/shared types | Self-contained; ID tests are meaningful. Retain the early `Check` producer and include it in the commit. | |
| 99 | | 2 — Journal | Existing record/store interfaces match; legacy transitions remain. Guard-conflict tests discriminate. Task 14 must use the actual cut signature. | |
| 100 | | 3 — Claim transitions | Merged guards, kinds, fields and update primitives exist. Undefined fixture helper blocks compilation; abort replay needs coverage. | |
| 101 | | 4 — Door | Existing selector signatures now match, and the adapter is specified locally. HTTP fixtures and source selection remain defective. Legacy dispatch is explicitly gated. | |
| 102 | | 5 — Version receipts | Interfaces are additive and ordinary S3 mirroring stays plain. Multipart receipt/version coverage is missing. | |
| 103 | | 6 — Lease/signing | Existing TTL acquisition internals, `SafeUntil` and signer support the additions. Timestamp test is now discriminating; commit file set is incomplete. | |
| 104 | | 7 — Archive | Crypto interfaces match. Round-trip, authentication and mode tests are useful; producer cleanup on rejection is untested. | |
| 105 | | 7b — Held helpers/strict spawn | Staging, `PointLive`, `PrepareStrict` and configuration primitives exist. Mock seam is viable. Root relay, `EROFS` handling and existing `LiveVersion` reuse need correction. | |
| 106 | | 8 — Timers | Registry timer shape matches; digest and registration round-trip tests are meaningful. No additional blocker found. | |
| 107 | | 9 — Environment | Actual spawn helpers/signatures now align. Composition tests discriminate; stale interface prose and shadow reporting need reconciliation. | |
| 108 | | 10 — Budget | Referenced model at `9c417aaa` and numerical examples align. Boundary tests are meaningful. | |
| 109 | | 11 — Readiness | Duplicate `Check` blocks compilation. Freshness tests miss mismatched input generations. Host work is deferred to Task 14. | |
| 110 | | 12 — Holds | Registry APIs and fixture exist. Conditional release/deletion tests are meaningful; additions are compatible with existing rows. | |
| 111 | | 13 — Floor | Applier hooks exist. Tests miss admission-to-marker crash ordering and actual switch-off activation behavior. | |
| 112 | | 14 — Integration | Worker and harness exist. Wrong call signature, helper placement, abort replay, fallback lease and readiness integration remain unresolved. | |
| 113 | | 15 — Copy gate | Earlier `DefaultEnabled` producer, complete inventory and record import are specified. Negative fixtures meaningfully test the gate. | |
| 114 | | 16 — Final gate | Cannot pass the preceding tasks as written. No gate was run in this review. | |
| 115 | |
| 116 | **Claim independence:** I found no direct Plan 1 dependency on unmerged `hostops`, `hostbox`, `hostwrite`, `hostexec`, `hostfence` or `hostquarantine`. The intended implementation can use merged primitives plus the additions specified here. Claimless-host refusal pending adoption is an explicit operational restriction, not a build dependency on claim Plan 5. The findings above can be repaired without claim Plans 5/6/8. |
| 117 | |
| 118 | **Imports and shippability:** No intended package import cycle was found. Missing/duplicate identifiers, the wrong signature and incomplete commit boundaries prevent buildability after every task. Default-off preservation is substantially improved for the console, worker, mirror and strict spawn, but remains unproven for system activation. |
| 119 | |
| 120 | **Spec coverage:** The remaining gaps include §11’s activation floor, §7.1’s source selection, §7.3’s checked-input consistency and shadow reporting, and operation-bound held materialization. These prevent treating the plan as a complete implementation prescription. |
| 121 | |
| 122 | **PLAN 1: NOT READY TO BUILD.** |
| 123 | |
| 124 | --- |
| 125 | |
| 126 | **FIRST PROMPT:** Read-only review of Plan 1 and its index before implementation. |
| 127 | |
| 128 | **WHAT:** Checked R4 dispositions, main interfaces, claim independence, tests and spec coverage. |
| 129 | |
| 130 | **STATUS:** Review complete; blockers remain. Remote-main freshness unverified. |
| 131 | |
| 132 | **NEEDED:** Correct the findings before implementation. |
| 133 | |
| 134 | **RECOMMENDED:** Re-review the amended plan against successfully fetched `origin/main`. |
| 135 | |
| 136 | **SESSION:** Safe to close. Repository unchanged; no processes started. |
amadan