| 1 | **Verdict: REJECT** |
| 2 | |
| 3 | Reviewed commit `087e3380`. Seven findings follow. |
| 4 | |
| 5 | **Findings** |
| 6 | |
| 7 | 1. **P1 — A failed refresh lets a held instance execute a stale build.** |
| 8 | [internal/host/buildstate.go:641](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/buildstate.go:641) |
| 9 | |
| 10 | A recovered instance can have cached `last_executed=A` and live A while its durable build state names B. If the wake’s refresh fails transiently, it logs the error and selects A from the old copy. The subsequent pre-exec GET can succeed, but [beforeExec:488](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/buildstate.go:488) blindly merges A into that freshly read state. A then runs against B’s data. |
| 11 | |
| 12 | This violates §4.3’s stale-link refusal. The later successful write does not validate the earlier selection. |
| 13 | |
| 14 | **Smallest fix:** refuse held wakes when their required refresh fails. Revalidate the held target against the state read under the lease before recording execution. |
| 15 | |
| 16 | 2. **P1 — Settings protect recovery copies from deletion, but not replacement.** |
| 17 | [internal/host/versionsweep.go:82](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/versionsweep.go:82), [internal/release/pins.go:124](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/pins.go:124) |
| 18 | |
| 19 | The new settings references reach the deletion sweep only. Staging’s `BoxPins` still reads just `Version`, `PrevVersion`, and `GoodVersion`. |
| 20 | |
| 21 | Consider a sleeper that last executed V1, then received cold installs V2 and V3. With no `GoodVersion` or active generation, only its build state names V1. Another instance adopting a legitimately promoted reuse of V1 causes [stageFrom:1994](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/adopt.go:1994) to consider the old copy unreferenced and replace both executable and manifest sibling. Later retained recovery validates the replacement bytes; the original recovery copy is gone. |
| 22 | |
| 23 | This breaks §4.1’s retained identity and the plan index’s explicit requirement to include settings references in the restaging pin set. |
| 24 | |
| 25 | **Smallest fix:** include pins, `LastExecuted`, and resolved targets in `BoxPins`, qualified by account/app. Refuse replacement when those references cannot be read completely. |
| 26 | |
| 27 | 3. **P1 — A graceful move can reopen a completed-failed request.** |
| 28 | [internal/release/adopt.go:2209](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/adopt.go:2209), [internal/host/movescut.go:986](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/movescut.go:986) |
| 29 | |
| 30 | A sleeping manual instance has `last_executed=A`. Root resolves request R to an already-failed T and records R as completed-failed in its local consumed file. Nothing executes, so nothing mirrors that completion into the build state. A graceful move seals the unchanged bucket object. On the destination, R is above the watermark and resolves again, potentially installing a newer head without another “Update now.” |
| 31 | |
| 32 | This violates §4.3’s completed-request rule and §4.3a’s preservation of request state. |
| 33 | |
| 34 | **Smallest fix:** merge local request metadata into the durable state under the lease before transfer/sealing, preserving `last_executed`. Cover legacy moves and return lanes too; successful completion must remain exclusive to pre-exec writes. |
| 35 | |
| 36 | 4. **P1 — A major hold is bypassed until root writes a matching reason.** |
| 37 | [internal/host/buildstate.go:587](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/buildstate.go:587) |
| 38 | |
| 39 | `heldTarget` recognizes a major hold through root’s consumed reason. An absent file or stale `ReasonAdopt` yields `Held=false`. |
| 40 | |
| 41 | For example, a sleeper last ran B from major N but has a cold-installed A from major N+1. The member enables major N. Before root corrects the installation and writes its reason, a flag-off wake reads the new setting and fresh build state but still runs A. `Target` given the same current settings and head would choose B. |
| 42 | |
| 43 | This violates §4.3 rule 3 and the required agreement between root and wake selection. |
| 44 | |
| 45 | **Smallest fix:** determine current major eligibility in the wake decision, or hold and request root until a decision for the current settings/head is available. Missing or stale root output cannot establish that the instance is unheld. |
| 46 | |
| 47 | 5. **P1 — A failed installed build can run when no build-state object exists.** |
| 48 | [internal/host/buildstate.go:597](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/buildstate.go:597) |
| 49 | |
| 50 | For a legacy manual instance with `Known=true`, no state object, installed T, and root’s `ReasonFailed/Detail=T`, the final branch returns `Held=false`. With no planner, the wake executes T. |
| 51 | |
| 52 | Root correctly applies failure precedence to the installed-build fallback at [updatetarget.go:156](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/updatetarget.go:156). The wake never checks that fallback through `rootFailed`. |
| 53 | |
| 54 | This violates §4.3’s failure precedence and §6, including the explicitly supported legacy-state case. |
| 55 | |
| 56 | **Smallest fix:** select and failure-check the installed build when the state has no `last_executed`; return an explicit failed held target instead of falling through. |
| 57 | |
| 58 | 6. **P1 — The local sweep proceeds with an explicitly incomplete recovery set.** |
| 59 | [internal/host/versionsweep.go:149](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/host/versionsweep.go:149) |
| 60 | |
| 61 | `settingsPins` ignores `ReadErr` when `Known` remains true. After a failed refresh, an old cached state naming A therefore permits deletion of other staged builds. If the recovered durable state names B, and no live link, version column, or generation names B, the sweep can delete its only recovery copy. |
| 62 | |
| 63 | Keeping A does not make the protected set complete. This violates §4.4’s incomplete-set rule and the required preservation of local recovery copies. |
| 64 | |
| 65 | **Smallest fix:** refuse sweeping when a relevant build-state copy has `ReadErr`, while retaining its previously known references. Add a test containing both cached A and an otherwise unreferenced recovery B. |
| 66 | |
| 67 | 7. **P2 — Verified local recovery unnecessarily depends on remote release objects.** |
| 68 | [internal/release/adopt.go:2252](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/adopt.go:2252) |
| 69 | |
| 70 | `stageAuthorized` tries the retained copy only after `ErrNoPromotionRecord`. An existing promotion record whose manifest GET fails—or a failed record GET—therefore blocks recovery despite a valid local executable and sibling. |
| 71 | |
| 72 | A held instance with another live version cannot get its retained target installed. With a planner, its wake waits for root and times out. §4.1 permits recovery from the box’s verified staged copy independently of remote availability. |
| 73 | |
| 74 | **Smallest fix:** accept verified retained evidence independently; require remote digest-bound evidence whenever downloading or replacing bytes. |
| 75 | |
| 76 | **Rulings** |
| 77 | |
| 78 | - **R-SPEC-43A:** disagree for completed-failed requests. The lost-request exception concerns a lost resolution before execution. It does not permit a graceful move to discard a completed outcome and reopen the request. |
| 79 | - **R-INV1:** disagree with excluding departed-box failed completions from the completion invariant. That makes finding 3 pass the gate. Lost resolutions and completed requests need distinct expectations. |
| 80 | - The Task 7 note claiming failed refreshes are effectively fail-closed is also incorrect: the refresh can fail while the subsequent pre-exec GET and PUT succeed, as in finding 1. |
| 81 | |
| 82 | The simulation exercises the production decision and merge functions, but does **not** faithfully cover wake enforcement. [Its wake:946](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/updatesim_test.go:946) runs the live build directly, bypassing `heldTarget` and `wakeCheck`; [its consistency check:993](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/settings-4-root-applies/internal/release/updatesim_test.go:993) excludes stale known copies that production preserves. The reported 3,000-seed result therefore does not establish these guarantees. |
| 83 | |
| 84 | Tests could not start because the read-only sandbox refused Go’s build directory. Findings are based on verified code paths; no files were changed. |
amadan