carlos / platform Public

Reviewed cc69376d.

| Round 3 finding | Disposition | Assessment | |---|---|---| | P2: acknowledgement guard applied too broadly | Resolved | PinFailover limits the pre-pin acknowledgement barrier to initial failovers and source loss during starting. | | P3: incorrect completion-cleanup attribution | Resolved | The plan now distinguishes its immediate park abstraction from the at-rest sweep and retained-object cleanup. |

1. P2 — Task 5C, Step 1: converted moves still pin before naming a target.

In relocPinning, target selection precedes pinning only when PinFailover is set. A converted move instead selects its pin before relocNameTarget.

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.

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.

1. P2 — Task 5B, Step 1, after Task 5C: the one-box phase matrix misses actual pinning.

The termination matrix stops once the failover target is named. Its subsequent atPhase("pinning") succeeds immediately, although PinFailover remains true—representing production’s starting phase.

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.

Fix: make phase predicates and assertions understand PinFailover. Exercise target termination both while the flag is set and after it clears, before installation.

1. P2 — Task 3, Step 1: initial failovers bypass relocation-start guards.

The serving-executor fence calls startFailover unconditionally. Neither that path nor later target naming checks app-admission closure or the start guard’s same-region park-replica exclusion.

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.

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.

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.

---

FIRST PROMPT: “Follow the review brief docs/superpowers/plans/CODEX-PLAN-REGION-HA-TWO-BOX-BRIEF-4.md. Review the plan at commit cc69376d.”

WHAT: Review prior dispositions and report new findings.

STATUS: Both prior findings resolved; three new P2 findings.

NEEDED: Nothing for this review.

RECOMMENDED: Correct target-selection order, phase coverage and failover-start guards.

SESSION: Safe to close — no changes or processes left running.

Verdict: not ready.