carlos / platform Public

PLAN 1: NOT READY TO BUILD.

Reviewed d5716bf4 against cached origin/main 1c43c2ce82f621c6ac84a623db54f0ef63075603, spec rev 7.19, claim rev 6.27 at 989ee349, and both prior reviews. Fetch failed on the read-only filesystem; a read-only remote-tip lookup then failed DNS resolution. Consequently, remote freshness is unverified. No files changed, hosts contacted, or tests run.

1. P1 — The switch does not protect the intermediate commits. Plan 1:585 removes the existing cut implementation in Task 2; the worker switch only arrives in Task 14. Task 4 references s.cfg.MovesEnabled, which neither main nor an earlier task defines, and only explicitly checks it in the request handler—not cut, abort, or resolve. moves.DefaultEnabled likewise has no producer before Task 15 uses it. Moreover, index D13:155 disables main’s existing move behavior when unset, so it does not meet the literal “off = no behavior change” requirement. Introduce and test the complete compatibility/switch boundary before Task 2 changes behavior.

1. P1 — Tasks 2 and 3 cannot pass their commit gates. Plan 1:585 deletes MoveRequest and MoveComplete but omits internal/provision/provision_test.go: main still calls them at lines 916, 922, 928, 946, 954 and 1005. Plan 1:626 adds servingClaim() *Claim; main already defines servingClaim(t, ctx, store) Observed in internal/hostclaim/delete_test.go:19. Go cannot overload it. Update all removed-function consumers and rename the new fixture.

1. P1 — The specified copy presigner does not exist in the pinned SDK. Plan 1:1334 calls s3.PresignClient.PresignCopyObject. Main pins S3 v1.105.0, whose locally installed source has no such method. Task 6 needs a concrete supported signing implementation, including copy-source conditional headers, response/error decoding, and the captured signing time. This is not an implementation-ready interface.

1. P1 — Task 5 breaks existing non-versioned mirroring. Plan 1:1298 replaces equal-ETag skipping with a requirement for a receipt containing DstVersionID. A plain MirrorStore cannot supply that field, so unchanged objects are recopied every round. Main’s TestMirrorPrefix already requires the second identical round to copy nothing. Apply the stronger receipt requirement only when version evidence is required; preserve the existing ordinary-mirror path.

1. P1 — The source copy is not bound to the claim’s location and generation. Plan 1:3025 changes staging destinations but leaves “own replica prefix” unspecified. Main constructs that prefix with activator.HibernatePrefix(cfg.OwnPrefix, host) at movespass.go:366. That scans the host namespace, not the claim-selected generation. With incarnation directories or g-<op>/ generations, it copies unrelated generations and retains their directory nesting under replica/. Define selection from holder.location and holder.generation, including legacy-root handling, using the already-merged claim/genselect primitives. Test multiple incarnations and generations.

1. P1 — Some staging writes bypass the promised signing guard. Plan 1:3025 sends move.json through ordinary VersionedStore.PutIfAbsent; only the mirror’s copy/put/delete methods explicitly use Guarded.Before. Main’s versioned implementation calls the normal SDK directly. Task 11’s measurement writes under the staging probe/ prefix also lack this contract. Additionally, the round validates the claim before acquiring the staging lease, whereas §7.3 requires revalidation afterward. Specify one guarded mutation path for every staging writer and test each request class across expiry/retry.

1. P1 — Plan 1’s worker transitions still lack operation-identity guards. Plan 1:3025 calls MovePromoteReady and MoveMarkCutting; Task 2 only adds legacy/state checks. Neither compares the expected move ID and incarnation. SetRecordMove rereads and retries, so a delayed round for A can promote or cut a replacement move B in the same state. MoveGuard arrives only in Plan 2. Bring the guards forward for every Plan 1 worker transition, including abort completion, and test replacement between observation and CAS.

1. P2 — Several proposed tests remain red after correct implementation. These are fixture/expectation failures, not useful regression failures: - Plan 1:275: UpdateRecord(..., "") is explicitly rejected by main; the legacy fixture never reaches its assertions. - Plan 1:1209: fakeMirror.put creates zero-size objects, so the expected successful recopy still has BytesCopied == 0. - Plan 1:1382: lease.TestEndpoint does not exist; the fixture never calls AddBucket; leasetest.NewS3Server supports create-only PUTs and version-specific deletes, not the full lease/update behavior these tests need. - Plan 1:890: the existing API test needs more than operator/claim setup. It clears the record without releasing the claim, then expects another request to succeed; it also expects abort from newly requested cutting to fail, contrary to Task 2.

1. P2 — Launch-environment composition does not match main’s spawn contract. Plan 1:2528 proposes mergeEnv(fileEnv, agentOwnedEnv(rt)). Main already has agentOwnedVars(rt, fileEnv, haveControl), which removes tenant-provided agent-owned names when unavailable and depends on the actual control-listener outcome. Merging an agent-only map cannot express those removals. The proposed launch test would pass even if the entire agent-owned layer were omitted. Reuse the existing composition contract and test removals, region, cache presence, and control availability. Separately, CompareEnv at line 2485 accepts a missing target CARLOS_REGION, and never checks layout names absent from both reports.

1. P2 — T5 does not verify the archive requirements it claims to cover. Plan 1:2636 specifies archive.Open to io.Discard and “compare plain.” Open returns only an error and emits compressed tar bytes; this establishes decryption, not the plaintext tar digest, manifest entries, allowed entry types, or uncompressed cap. Add a streaming verifier and negative tests for correctly encrypted archives with wrong manifests, special entries, and excessive content.

1. P2 — Archive extraction does not preserve modes. Plan 1:1954 permanently ORs directory modes with 0700; file creation is also subject to umask. A valid 0555 directory becomes 0755, changing the reconstructed plaintext digest and violating §8.1. Use temporary writable directory modes, then restore exact modes after extraction; explicitly restore file modes too. The existing round-trip fixture does not exercise this.

1. P2 — Task 12 has a forward dependency and reintroduces the rejected completeness check. Plan 1:2825 requires registry hold_op inspection and held-row tests before Task 13 creates the field and schema. Move that prerequisite earlier. Plan 1:2851 again requires readEdgesSnapshot(...).complete(). That reader never sets BoxesComplete; this contradicts Task 4 and the R2-1 disposition and would refuse every request.

1. P2 — Target selection still omits approved rev 7.19 behavior. Plan 1:1072 defers eligible-box selection and uses lexical order. Main now contains same-package eligibleBoxes and chooseBox in internal/console/boxchoice.go; neither needs exporting for this caller. Main’s helper still needs reconciliation with the approved SHA-256 tie-break, but the plan must specify that work now. Also bind target reporting to op.intent.target; “every box whose placement equals move.to” is insufficient for the approved named-target contract.

1. P2 — Task 14 contains mutually incompatible acceptance criteria. Plan 1:3028 requires an existing move.json with no ledger to stop copying, while the implementation and lost-ledger test require accepting ErrExists without reading the target object. These source-visible states are indistinguishable. Keep the corrected target-validation rule and replace the obsolete source-refusal test. The self-review at line 3067 also still claims source-side staging deletion, contradicting Task 14’s target-owned cleanup disposition.

The task-by-task result is:

| Task | Build and test assessment | |---|---| | 1 — IDs | New package/functions; tests have meaningful positive and negative assertions. No blocker found. | | 2 — Journal | Existing production interfaces exist; deleted consumers, invalid fixture, switch ordering and missing identity guards block it. | | 3 — Claim transitions | Consumed claim APIs exist on main. Duplicate helper blocks tests; ReleaseMove needs its stated implementation/test. | | 4 — Door | Most APIs exist; console switch producer is missing and existing tests need transition-aware migration. | | 5 — Copy versions | Existing mirror types match; plain-store regression and zero-size fixture prevent green tests. | | 6 — Lease/signing | acquireTTL exists and can be exported. Copy presigner and test endpoint do not; fixtures are incompatible. | | 7 — Archive | Crypto signatures match main. New tests cover basic round-trip/tampering, but miss mode preservation. | | 8 — Timers | registry.Timer fields match exactly. No import cycle or substantive blocker found. | | 9 — Environment | Existing spawn composition differs from the proposed extraction; tests do not protect that contract. | | 10 — Budget | Referenced model exists at 9c417aaa; arithmetic and expected values align. No blocker found. | | 11 — Readiness | Pure dependencies exist by this task. Host work is explicitly deferred to Task 14; T5 verification is incomplete. | | 12 — Floor | Main applier/edges interfaces exist. Depends on Task 13 and contradicts the corrected completeness rule. | | 13 — Holds | Registry interfaces and fixture exist. No claim dependency; add a negative test for DeleteIfHold, which currently has none. | | 14 — Integration | Existing harness exists; source selection, guarded writes, identity CAS and contradictory tests block completion. | | 15 — Copy gate | Requires the missing DefaultEnabled producer and explicit inclusion of the copy record when building from main. | | 16 — Gate | Cannot pass with the preceding failures. No gate was executed during this review. |

Claim dependency verdict: I found no direct Plan 1 call into unmerged hostops, hostbox, hostwrite, or lease-controller packages. The identified build fixes can be made against merged primitives. However, the plan explicitly substitutes manifest existence for T7 materialization and cgroup delegation for S2 strict-next-spawn. Those are scaffolding, not completed readiness requirements. The direct-socket S1 probe also needs an explicit wake path for an already-hibernating source.

No new package import cycle was found in Plan 1’s stated dependency graph. The failure is concrete build/test mismatches, incomplete integration contracts, and ineffective intermediate gating.

---

FIRST PROMPT: Focused read-only review of moves Phase 1 Plan 1 and its index before implementation.

WHAT: Check buildability on main, claim independence, switch-off safety, tests and spec coverage.

STATUS: Review complete. PLAN 1: NOT READY TO BUILD. Remote freshness unverified.

NEEDED: Nothing for this review.

RECOMMENDED: Fix the build blockers and establish the switch boundary first; then revise staging and readiness contracts before re-review.

SESSION: Safe to close. Repository unchanged; nothing running; no handoff file written.