carlos / platform Public

Download this file

1	Reviewed `cc69376d`.
2
3	| Round 3 finding | Disposition | Assessment |
4	|---|---|---|
5	| P2: acknowledgement guard applied too broadly | Resolved | `PinFailover` limits the pre-pin acknowledgement barrier to initial failovers and source loss during `starting`. |
6	| P3: incorrect completion-cleanup attribution | Resolved | The plan now distinguishes its immediate park abstraction from the at-rest sweep and retained-object cleanup. |
7
8	1. **P2 — Task 5C, Step 1: converted moves still pin before naming a target.**
9
10	In [relocPinning](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha-two-box/docs/superpowers/plans/2026-10-07-region-ha-hostlife-two-box.md:8131), target selection precedes pinning only when `PinFailover` is set. A converted move instead selects its pin before `relocNameTarget`.
11
12	1b-next rev 19’s `Worker.pinning` checks for a missing target and calls `NameTarget` **before** selecting any pin (lines 7029–7048). If no target is eligible while a newer manifest lands, production waits and later includes that manifest; the model can retain an earlier pin.
13
14	**Fix:** name the target before pin selection on every entry path, retaining the acknowledgement barrier only for initial failovers. Adjust the converted-move test to hold an already-named target’s controller unavailable while another worker pins. Add the no-eligible-target interleaving.
15
16	2. **P2 — Task 5B, Step 1, after Task 5C: the one-box phase matrix misses actual `pinning`.**
17
18	The [termination matrix](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha-two-box/docs/superpowers/plans/2026-10-07-region-ha-hostlife-two-box.md:6448) stops once the failover target is named. Its subsequent `atPhase("pinning")` succeeds immediately, although `PinFailover` remains true—representing production’s `starting` phase.
19
20	Consequently, the one-box “pinning” case terminates the target before `PinFailover`, not after it. The test also explicitly skips `starting`, so its labels conceal the missing phase boundary.
21
22	**Fix:** make phase predicates and assertions understand `PinFailover`. Exercise target termination both while the flag is set and after it clears, before installation.
23
24	3. **P2 — Task 3, Step 1: initial failovers bypass relocation-start guards.**
25
26	The [serving-executor fence](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha-two-box/docs/superpowers/plans/2026-10-07-region-ha-hostlife-two-box.md:2801) calls `startFailover` unconditionally. Neither that path nor later target naming checks app-admission closure or the start guard’s same-region `park-replica` exclusion.
27
28	In 1b-next rev 19, failovers pass through `Worker.Start`’s admission registration/recheck and `StartRelocation`’s `RelocationStartGuard`. The model therefore starts recovery in states production refuses. Existing tests exercise the drain path or call the guard helper directly; they cannot catch these missing failover checks.
29
30	**Fix:** apply the corresponding guards to serving-to-failover starts, preserving the fenced-source exception for local chores. Add termination cases with closed admission and an outstanding same-region `park-replica`.
31
32	The four new Go files reconstructed from their sequential patches and passed `gofmt` syntax checks. Compilation, runtime tests and mutations were not independently reproduced in this read-only workspace. No files changed or sweeps ran.
33
34	---
35
36	**FIRST PROMPT:**
37	“Follow the review brief docs/superpowers/plans/CODEX-PLAN-REGION-HA-TWO-BOX-BRIEF-4.md. Review the plan at commit cc69376d.”
38
39	**WHAT:**
40	Review prior dispositions and report new findings.
41
42	**STATUS:**
43	Both prior findings resolved; three new P2 findings.
44
45	**NEEDED:**
46	Nothing for this review.
47
48	**RECOMMENDED:**
49	Correct target-selection order, phase coverage and failover-start guards.
50
51	**SESSION:**
52	Safe to close — no changes or processes left running.
53
54	**Verdict: not ready.**