carlos / platform Public

Astra review P6 of the zero-downtime plans (REJECT: 3 partial, 8 new)

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

claude 2 weeks ago 1dc151631bae33d40cc2a0960683e53726c42ab8 parent a00dfd6

1 file changed, +27 −0

docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P6.md +27 −0

CI passed — run details

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

new file mode 100644

index 000000000..8c7edb1f1

--- /dev/null

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

@@ -0,0 +1,27 @@

+REJECT

+P5 partial P1 #1: **CLOSED** — current-generation stops retain ownership until termination is proven.

+P5 partial P1 #6: **PARTIAL** — worker publication now precedes the failure event; finding 7.

+P5 partial P2 #3: **CLOSED** — tracked stray sidecars prevent lease release.

+P5 partial P4 #1: **CLOSED** — nil-handle retirement, nested workers and failed-wake worker handles are covered.

+P5 partial P4 #3: **CLOSED** — incarnation-ordered publication fixes the reported overwrite race.

+P5 new #1: **PARTIAL** — initial refusal is fixed; subsequent fallback bypasses ownership checks; finding 1.

+P5 new #2: **CLOSED** — strict parents use `Close`, retaining failed handles for retry.

+P5 new #3: **PARTIAL** — stamp/fence errors are covered, but `ErrNoClaim` and claim-read errors still become build failures.

+P5 new #4: **CLOSED** — worker serving writes have one ordered writer.

+1. **P1 — Plan 5, Task 11: worker fallback executes after lease loss.** [`ReplaceWorker`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7037) clears `attemptGate` before observing readiness. If B then crashes after its replicator loses the lease, the failure branch calls `respawnWorkerLocked` with the fallback; its remaining check covers entry/state/shutdown, but not the lease or fences. A therefore executes without ownership, violating **§7.4**. The ownership test loses the lease during initial retirement and misses this later path. **Smallest fix:** require an ownership gate for every respawn, including fallback and restoration, independently of attempt supersession.

+2. **P1 — Plan 3, Task 7: legacy sidecar descendants survive replacement and lease release.** [`discardSidecar` and stray reaping](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:5362) accept `Exited()` as termination. However, the returned `childProc` [signals only its leader](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/backend_exec.go:1557). A forked descendant remains in `role-sidecar`, while the stray record disappears. Replacement can start beside it, and `releasable` can subsequently release the lease. This violates **§§7.4–7.5**. **Smallest fix:** prove the sidecar’s own cgroup empty before replacement or releasing its tracked ownership; retain it when that proof fails.

+3. **P1 — Plan 5, Tasks 9–10: planned wakes do not revalidate before execution.** Planning happens before acquisition/restoration, but [`spawnPlanned` and `spawnPlannedWorker`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5489) call their spawn helpers without a selection gate. A host can select B, pause during restoration, then execute B after a sibling’s failure has held that build. Neither `WakePlan` nor the claimed planned-wake path carries a corresponding recheck. This violates **§§7.4 and 8.1–8.4**. **Smallest fix:** revalidate the selected build and current failure hold at every planned execution boundary; abandon or replan using the already-restored state.

+4. **P1 — Plan 3, Tasks 2/10: claims-on retirement immediately kills in-flight requests.** [`stopMember`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:1664) calls the claimed process’s unchanged [`Stop`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/claimwake.go:311), which immediately invokes `KillAndEmpty`. That writes `cgroup.kill`; it provides no SIGTERM drain. Switching a healthy claimed instance therefore kills the retiring generation’s outstanding requests, contrary to **§7.5 step 6**. **Smallest fix:** add graceful retirement for claimed serving generations, followed by the existing emptiness proof; preserve immediate killing for failed, unready candidates.

+5. **P1 — Plan 5, Task 11: a failed same-build restart can acknowledge config that never ran.** On a spawn error, the existing [`respawnWorkerLocked` error arm](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/activator.go:1957) preserves the old `e.proc` and captured identity. For restart/config, [`sameArtifact`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7050) therefore passes despite the failed spawn. If the incumbent stopped cleanly, readiness observes that old process’s clean exit and returns `SwapDone`; the scheduler acknowledges the new config. This violates **§§6.5 and 8.1**. **Smallest fix:** require a successfully installed new incarnation before readiness observation; the new `workerInc` can distinguish it.

+6. **P1 — Plan 4, Tasks 5/9; Plan 5, Tasks 12/14: flag-off worker recovery artifacts remain unpinned.** Captured-artifact recycling applies regardless of the flag, but [`armRollout`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:8563) returns before enabling generation-file publication when the flag is off. Root pins installed/previous/good versions, without the worker’s captured serving build. If old A is running while B and C are installed within convergence grace, A becomes unpinned. The [sweep measures artifact age](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/release/sweep.go:100), so it can delete A immediately; the next recycle then fails. Restaging can likewise overwrite A. This violates **§§4.2, 7.9 and 8.5**. **Smallest fix:** publish live and pending worker artifact pins independently of the flag, for both sweeping and restaging.

+7. **P1 — Plan 5, Tasks 9/11/14: the new publication callback reintroduces publication before the failure hold.** Task 11 inserts [`publishWorkerServing` before closing readiness](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7185). Task 9 invokes `afterWake` afterward; only that emits `GenFellBack`. Following a failed B and successful worker fallback A, the synchronous serving callback can block on its mutex or registry write while B remains unheld and sibling wakes can select it. Reordering `OnGen` internally does not cover this earlier call. This violates **§8.1**. **Smallest fix:** complete the failure transition before worker serving publication. Test the actual fallback wake with a blocked callback, rather than calling `OnGen` directly.

+8. **P2 — Plan 5, Task 11; Plan 6, Task 13: an unknown-version replacement retains an obsolete serving version.** [`publishWorkerServing`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7170) skips empty versions, although Plan 4 explicitly supports external live-link targets whose captured version is empty. After a previous child published A, a successful replacement from such a target leaves `ServingVersion=A` indefinitely, and Plan 6 displays A as running. This violates **§7.9**. **Smallest fix:** publish empty versions through the same incarnation-ordered callback, clearing the obsolete value.

\ No newline at end of file