carlos / platform Public

Astra review P2 of the zero-downtime plan set (REJECT: 3 partial, 8 new)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago 13ab01a131f621d491232e5ea2f0bd78054b1731 parent cf29e61

1 file changed, +34 −0

docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P2.md +34 −0

CI passed — run details

diff --git a/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P2.md b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P2.md

new file mode 100644

index 000000000..c9f150773

--- /dev/null

+++ b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P2.md

@@ -0,0 +1,34 @@

+REJECT

+P1 #1: PARTIAL — member termination now proves cgroup emptiness; current-generation stops and failed-wake cleanup still bypass that proof.

+P1 #2: CLOSED.

+P1 #3: CLOSED.

+P1 #4: CLOSED.

+P1 #5: CLOSED.

+P1 #6: PARTIAL — admission halts atomically, but wakes can still select the failed build before the failure record is written; a failed registry read skips that write entirely.

+P1 #7: CLOSED.

+P1 #8: CLOSED.

+P1 #9: CLOSED.

+P1 #10: CLOSED.

+P1 #11: CLOSED.

+P1 #12: CLOSED.

+P1 #13: PARTIAL — foreign generations reuse one socket path, so an old pool can connect to the replacement and attribute its response to the old generation.

+P1 #14: CLOSED.

+P1 #15: CLOSED.

+P1 #16: CLOSED.

+1. **P1 — Plan 5, Task 14: initialization failure bypasses persisted failure holds.** [ `armRollout` ](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7538) returns nil when the store, key or failure book cannot be loaded. The agent then continues without `Planner`, using live-link wakes and legacy convergence. An unreadable failure book therefore permits exactly what its error message says it prevents: executing the failed installed build again, including after a hold migration. This violates spec §§6.4, 8.3–8.4. **Smallest fix:** when the flag is enabled, fail initialization or refuse managed starts until the required state is available; do not select legacy behavior.

+2. **P1 — Plan 3, Task 9, against Plan 2, Task 9: nested sidecar starts can collapse the overlap limit.** The new [staged sidecar start](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:5422) still enters the original `/start` path, whose [`handleStart`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/instancectl/server.go:309) unconditionally sets the shared host slice to one tier. A supervised sidecar restart during overlap lowers the raised parent beneath both generations and can OOM the incumbent. It also reads the mutable live-link tier after fallback. This violates §7.3. **Smallest fix:** generation-aware sidecar starts must preserve the parent limit and leave its restoration to the swap owner; retain the original behavior for flag-off starts.

+3. **P1 — Plan 3, Task 7: joining the supervisor does not join its discarded process.** The new [stale-generation completion branch](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:4396) calls [`stopDiscardedSidecar`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/activator.go:517), which launches cleanup asynchronously. The supervisor then closes `sidecarStartDone`, allowing `replaceSidecar` to start the replacement while the discarded sidecar still runs. A terminal stop can likewise pass this join and release the lease. This violates §§7.4–7.5. **Smallest fix:** stop discarded sidecars synchronously outside `a.mu`, retain unproven processes, and close the joined operation only after termination is proven.

+4. **P1 — Plan 3, Tasks 4, 6 and 10: instance attempts still have missing execution-time re-checks.** In [overlap](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:2798), registration checks precede the parent-limit operation and pin publication; [`spawnSuccessor`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:6145) performs no entry/newest re-check afterward. Supersession or lease loss during those operations still permits exec. The [hold path](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:3591) checks revision before constructing the waking entry, but never again before accepting health or settle success. A cohort halt during that wake can therefore still produce `SwapDone` and a good-build write. This violates §§6.3, 7.4 and 8.1. **Smallest fix:** carry the attempt checks through both spawn paths and re-check a hold before accepting its result.

+5. **P1 — Plan 5, Task 9: first-wake observation is registered after the generation becomes available.** [`afterWake`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5107) runs after `e.ready` closes and synchronously emits `GenHealthy` before setting `e.firstWatch`. During that interval, `Current` exposes the generation as stable. If it crashes and the sweep starts stopping it, `watchFirstWake` sees a non-awake entry and emits `GenCancelled`, suppressing a real build failure. This violates §§6.4 and 8.1. **Smallest fix:** register the observation under the awake-transition lock before exposing readiness, and preserve unexpected-exit classification if the entry stops before its observer runs.

+6. **P1 — Plan 3, Task 6 / Plan 5, Task 8: the foreign-layout conversion loses the effective hold mode.** [`Swap`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:3675) executes an overlap attempt on a foreign row through `swapHold`, but the scheduler receives the original `ModeOverlap`. Its [failure handling](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:4181) consequently omits `HeldDown`. After the hold fails, the next wake automatically boots the good build. This contradicts rev 17 §2.9 together with §§6.4 and 8.3. **Smallest fix:** propagate the effective mode to the scheduler and persist the hold-down before another wake can select fallback.

+7. **P2 — Plan 5, Task 9: socketless first wakes now undergo two readiness windows.** [`spawnPlannedWorker`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5046) waits ten seconds through `plannedWorkerReady`. Afterward, `afterWake` starts another ten-second observation because `FirstWake` remains true. A worker that survives twelve seconds and then crashes is recorded as a failed release, although §6.5 made it ready at ten seconds and §8.6 requires ordinary crash recovery afterward. **Smallest fix:** report planned worker readiness as settled once the first window completes; do not start the HTTP first-wake observer for it.

+8. **P2 — Plan 5, Task 12: the claimed-wake pin-publication insertion does not compile.** The [instruction](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:6903) copies `if plan.planned() || e.pin != nil` into `doClaimWake` after its planning block. Task 9 declares [`plan := a.Planner(...)`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5361) inside that block, so `plan` is out of scope. **Smallest fix:** use the stored `e.plan` in the claimed-wake insertion, or declare the local in the enclosing scope.

\ No newline at end of file