| 1 | **PLAN 1: NOT READY TO BUILD.** |
| 2 | |
| 3 | Reviewed `296807a2a337` against fetched `origin/main` at `1c43c2ce82f6`, spec 7.22, claim 6.30, and the five prior reviews. This was a static source and plan review: no modifications, fetches, host access, or test runs. |
| 4 | |
| 5 | I found **4 P1, 4 P2 and 1 P3 findings**. |
| 6 | |
| 7 | 1. **P1 — Two tasks consume fields before the task that creates them.** |
| 8 | [Plan 1:1355](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:1355) reads `edgesFilePlacementView.PrevRelease` and `.MovesFloorMinRelease`. Neither exists on main; Task 4 adds only `MovesProtocol`. Task 13 creates the other fields. Task 4 therefore cannot pass its build gate. |
| 9 | |
| 10 | Separately, [Plan 1:2611](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2611) requires Task 7b’s `heldStageAuthority` to read `hold_op`, but `Route.HoldOp` and its migration arrive in Task 12. |
| 11 | |
| 12 | **Fix:** introduce the console reader fields in Task 4 and move the registry prerequisite before Task 7b. Also resolve two smaller symbol omissions: Task 3 names `ErrOpMismatch`, which main does not define, and Task 4 uses `fleetBox.FloorArmed` without including it in the declared fixture shape. |
| 13 | |
| 14 | 2. **P1 — The shared box-identity test helper introduces an import cycle.** |
| 15 | [Plan 1:961](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:961) creates `boxid/boxidtest`, whose API returns `boxid.Record`, and rewrites the existing `boxid_test.go` to use it. That file is currently `package boxid`. |
| 16 | |
| 17 | The resulting test dependency is `boxid → boxidtest → boxid`. |
| 18 | **Fix:** explicitly migrate those tests to external package `boxid_test`, or retain an internal helper that does not import the child package. Console tests can use the exported helper. |
| 19 | |
| 20 | 3. **P1 — T7 preparation requires state that cannot exist until after readiness.** |
| 21 | [Plan 1:2611](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2611) authorizes root staging only when: |
| 22 | - the target registry row already carries `hold_op == req.Op`; |
| 23 | - `rec.Move.Inputs.Release == req.Version`. |
| 24 | |
| 25 | But [Plan 1:3198](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3198) needs that relay during **preparing**, before T7 can pass and the move become ready. Plan 1 supplies neither prerequisite. Plan 2 writes `Move.Inputs` at cut admission ([Plan 2:185](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md:185)); its target held row appears after the flip ([Plan 2:719](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-2-cut-flip-landing.md:719)). |
| 26 | |
| 27 | On the hardened production unit, preparation therefore cannot complete. A nil `Inputs` also needs explicit handling. |
| 28 | |
| 29 | **Fix:** define preparing-stage authority and the producer of its release identity within Plan 1. Add an integration test starting from an actual `MoveStart` record, with no target registry row, exercising `ErrStageBlocked → root relay → T7 pass`. The current readiness test stubs `ReleaseReady`, so it misses this cycle. |
| 30 | |
| 31 | 4. **P1 — The recovery command is incorrect under systemd, and its test bypasses the failure.** |
| 32 | [Plan 1:3507](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3507) embeds `printf "%s\n%s\n"` directly in `ExecStart`. Systemd expands `%s` to the service manager user’s shell; literal percent signs require escaping. Shell quotes do not prevent systemd’s expansion. |
| 33 | |
| 34 | With an armed floor, this can reject a valid `.prev` and select the failed current executable. The proposed test at line 3515 extracts the command and executes it through `sh`, so it passes without exercising systemd’s processing. |
| 35 | |
| 36 | There is also a comparison mismatch: `sort -V` orders `v0.30.0` before `v0.30.0-rc.1`, admitting that prerelease against a final-release floor, unlike semantic-version comparison. |
| 37 | |
| 38 | **Fix:** use correctly escaped unit syntax or a dedicated helper, enforce the same version semantics as `ReleaseAtOrAbove`, and test the command after unit expansion. |
| 39 | |
| 40 | 5. **P2 — The held-operation downgrade prohibition covers only `rollback`.** |
| 41 | [Plan 1:3519](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3519) adds the held-row refusal to `rollback`, while `install` and `restoreFiles` check only the minimum release. |
| 42 | |
| 43 | With floor `v0.30.0`, current `v0.32.0`, and a held operation, installing or recovering `v0.31.0` still passes. Main’s `recoverIfNeeded` calls `restoreFiles` directly, bypassing `rollback`. This misses [spec:3531](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/specs/2026-10-02-move-any-app-design.md:3531): **any downgrade of a held participant is refused**. |
| 44 | |
| 45 | **Fix:** cover each activating downgrade path. Add armed-floor tests where the candidate remains above the floor but below the current release. |
| 46 | |
| 47 | 6. **P2 — Archive verification accepts bytes after the authenticated final chunk.** |
| 48 | [Plan 1:2570](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2570) returns from `Open` immediately on the final chunk, without requiring ciphertext EOF. `Verify` hashes through a `TeeReader` consumed by `Open`; its subsequent drains consume the decoded stream, not the remaining ciphertext. |
| 49 | |
| 50 | Consequently, a valid sealed object with an appended suffix can pass against the original manifest’s sealed digest, although the complete object has a different digest. |
| 51 | |
| 52 | **Fix:** reject trailing ciphertext and add an appended-byte/concatenated-stream test. The existing corruption and truncation cases do not establish this property. |
| 53 | |
| 54 | 7. **P2 — T3 reports shadowing at the wrong configuration layer.** |
| 55 | [Plan 1:2609](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:2609) collects `MergedVersions.Shadowed` from the remote instance-config overlay. T3 tests that overlay overriding bundle variables. |
| 56 | |
| 57 | The required report concerns the carried **`<host>.env` overriding rendered configuration** ([spec:2537](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/specs/2026-10-02-move-any-app-design.md:2537)). Main applies that file later, in `ExecBackend.instanceEnv`. The proposed collection point cannot observe it. |
| 58 | |
| 59 | **Fix:** collect shadow names during final launch-environment composition. Test a carried `.env` override with no remote instance overlay. |
| 60 | |
| 61 | 8. **P2 — Commit commands still omit required changes and contradict task boundaries.** |
| 62 | [Plan 1:3544](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3544) adds `lease.MirrorPrefixKeep`, but Task 14’s `git add` at line 3555 excludes `internal/lease`. Following it leaves callers committed without their implementation. |
| 63 | |
| 64 | Task 11’s command at [line 3386](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-1-copy-and-staging.md:3386) also stages host files explicitly deferred to Task 14. Neither command includes the required `.amadan/ci.d/30-minio-contract` amendment. |
| 65 | |
| 66 | **Fix:** align file lists, gates and staging commands with the actual task boundary. Verify the staged tree, not just the working tree. |
| 67 | |
| 68 | 9. **P3 — The index retains superseded implementation instructions.** |
| 69 | [Index:166](/home/paulca/amadan.net/carlos/platform-moves-design/docs/superpowers/plans/2026-10-06-moves-phase1-0-index.md:166), D3, still describes SDK signing immediately after the guard. Task 6 now correctly requires the guard’s captured timestamp to be passed to the signer. D4 still says the S3 mirror implements `VersionedCopier`, while the detailed plan preserves the ordinary mirror and introduces a guarded staging adapter. |
| 70 | |
| 71 | Task 4 also still says it creates the constant-only `floor.go`, although Task 1 now owns that file. The cache instructions at index line 183 contradict the supplied machine rule to retain Go’s default cache. |
| 72 | |
| 73 | **Fix:** remove these competing instructions and update the affected “Fixed” dispositions. |
| 74 | |
| 75 | The task-by-task audit is: |
| 76 | |
| 77 | | Task | Build/API assessment | Test assessment from inspection | |
| 78 | |---|---|---| |
| 79 | | 0 — Switch | Additive configuration; legacy delegation is explicit. | New switch assertions discriminate; unchanged legacy tests are intentionally baseline checks. | |
| 80 | | 1 — Shared types/floor | Definitions now precede most consumers; no duplicate `Check` remains. | ID, floor and comparison cases target new behavior. | |
| 81 | | 2 — Record journal | Main’s record/transition APIs support the additions; guarded-cut signature is corrected. | Identity-conflict and additive-wire tests are meaningful. | |
| 82 | | 3 — Claim helpers | Required claim primitives exist on main; clarify `ErrOpMismatch`. | Admission, touch and released-move evidence cases discriminate. | |
| 83 | | 4 — Request door | **Blocked by findings 1–2.** Selector signatures and `liveEdges` exist as cited. | Fleet fixtures now exercise real identity verification, subject to the helper fixes. | |
| 84 | | 5 — Versioned receipts | Existing mirror/receipt interfaces support the extension. | Multipart and missing-version cases cover the prior gap. | |
| 85 | | 6 — Signing guard | Existing lease and signer APIs match; explicit `signAt` addresses the prior timing defect. | Pause and retry cases test the intended timestamp property. | |
| 86 | | 7 — Archive | No additional structural build blocker identified. | **Missing trailing-ciphertext case: finding 6.** | |
| 87 | | 7b — Held helpers | Existing render, staging and strict-process APIs fit. **Ordering/runtime blockers: findings 1, 3.** | Local helper tests miss the preparing-state integration failure. | |
| 88 | | 8 — Timers | Registry timer shape supports the proposed canonical snapshot. | Snapshot/digest cases are meaningful. | |
| 89 | | 9 — Environment | Existing launch composition supports extraction. | Classification tests are meaningful; T3 shadow reporting remains incomplete. | |
| 90 | | 10 — Budget | Referenced model and arithmetic are available. | Arithmetic/boundary cases are specified; not executed here. | |
| 91 | | 11 — Readiness | Pure package can precede Task 14; host implementation is explicitly deferred. | Freshness/provenance checks discriminate; stubbed T7 masks finding 3. | |
| 92 | | 12 — Holds | Migration and conditional operations are explicit additions. | Wrong-operation release/delete tests are meaningful; task must move earlier. | |
| 93 | | 13 — Floor | Existing applier hooks exist. **Findings 4–5 prevent approval.** | Shell-only and below-floor tests miss the identified production paths. | |
| 94 | | 14 — Integration | Legacy worker is retained; guarded transitions and generation filtering are specified. | **Blocked by T7 and incomplete commit contents.** | |
| 95 | | 15 — Copy gate | Existing gate is available; copying the design record into the implementation tree is explicit. | Separate inventory and pending/default-off checks discriminate. | |
| 96 | | 16 — Final gate | Cannot certify until preceding blockers are fixed. | No gate was run during this review. | |
| 97 | |
| 98 | **Claim independence:** I found no direct build dependency on unmerged claim Plans 5, 6 or 8. The claim primitives used by Plan 1 are present on main, and the shared `MoveInputs` definition has an explicit ownership rule. The preparation dependency in finding 3 is on later **moves** work and must still be removed. |
| 99 | |
| 100 | **Shippability:** D13 now correctly preserves the legacy door and worker and explicitly names the independently armed floor exception. However, the plan cannot keep every intermediate commit shippable because of the build-order/import-cycle failures and incomplete commits. The armed-floor implementation also remains incorrect. |
| 101 | |
| 102 | **Test confidence:** this review establishes concrete gaps in the proposed tests, not an executed red/green result. Existing legacy tests should already pass without the new code; that is intentional. The missing integration and negative cases above are needed before claiming the new tests establish the required behavior. |
| 103 | |
| 104 | --- |
| 105 | |
| 106 | **FIRST PROMPT:** |
| 107 | Focused read-only review of moves Phase 1 Plan 1 and its index before implementation. |
| 108 | |
| 109 | **WHAT:** |
| 110 | Check buildability, claim independence, dormant behavior, tests and spec coverage. |
| 111 | |
| 112 | **STATUS:** |
| 113 | Review complete: PLAN 1 NOT READY TO BUILD; 4 P1, 4 P2, 1 P3. |
| 114 | |
| 115 | **NEEDED:** |
| 116 | Nothing for this review. |
| 117 | |
| 118 | **RECOMMENDED:** |
| 119 | Correct the prerequisites and preparing-stage authority first, then repair floor enforcement and its tests. |
| 120 | |
| 121 | **SESSION:** |
| 122 | Safe to close. No files changed, tests run, or background processes started. |
amadan