Astra review P3 of the zero-downtime plan set (REJECT: 4 partial, 6 new)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+25
−0
docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P3.md+25 −0
CI passed — run details
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P3.md b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P3.md| new file mode 100644 |
| index 000000000..9ab41ca04 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P3.md |
| @@ -0,0 +1,25 @@ |
| +REJECT |
| + |
| +P2 partial P1 #1: PARTIAL — failed-wake cleanup still releases the lease when the leader has not exited. |
| +P2 partial P1 #6: PARTIAL — blocking pointer reads still precede failure publication; sibling wakes remain unguarded during that interval. |
| +P2 partial P1 #13: CLOSED. |
| +P2 #1: CLOSED. |
| +P2 #2: CLOSED. |
| +P2 #3: PARTIAL — terminal-stop completion branches still use asynchronous `stopDiscardedSidecar`. |
| +P2 #4: PARTIAL — claims-on `Read`/`Prepare` still run after the last newest-attempt check, with no subsequent execution-time check. |
| +P2 #5: CLOSED. |
| +P2 #6: CLOSED. |
| +P2 #7: CLOSED. |
| +P2 #8: CLOSED. |
| + |
| +1. **P1 — Plan 5, Task 8: an older successful completion can overwrite a newer good build.** [`done`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:4228) frees the host’s running slot before calling `served`. Another kick can therefore admit and complete C while B’s completion is delayed. B’s subsequent [`writeGood`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:3981) reads C’s record and replaces it with B; its retry explicitly permits overwriting a concurrent winner. `ServingVersion` can regress too. This violates §8.5’s expected-current requirement and can make later fallback select an older, migration-incompatible build. **Smallest fix:** retain the running slot until successful state publication completes, and make all good-build writes, including wake events, conditional on the generation being acknowledged. |
| + |
| +2. **P1 — Plan 5, Tasks 9–10: cold materialization bypasses failure holds and first-wake canaries.** [`decideWake`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5920) returns an empty plan when `rt.Version` is empty. Task 9 consequently takes the ordinary `startGen` path. The existing [`ensureLiveLink`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/backend_exec.go:794) can then materialize the channel head through [`MaterializeHost`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/release/materialize.go:47) and execute it without replanning. A newly provisioned host can execute a build already held for its app; its first wake also lacks the planned failure reporting and settle observation. This violates §§6.4 and 8.4. **Smallest fix:** materialize and refresh the installed identity before planning, or defer the wake until installation is observed; never execute a managed cold build through an empty plan. |
| + |
| +3. **P1 — Plan 5, Task 9: cancelling a wake records a build failure.** [`spawnPlanned`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:5170) sets `w.failed` for every error returned with a process, including cancellation of the wake context by maintenance or `Shutdown`. `spawnPlannedWorker` does the same when its readiness observation is cancelled. `afterWake` then emits `GenFailed`, halting the cohort and, in hold mode, persisting `HeldDown`. An intentional lifecycle stop therefore makes a healthy release require explicit recovery, contrary to §§6.4 and 7.4. **Smallest fix:** distinguish lifecycle cancellation from health/readiness failure before setting `w.failed`; clean up and report cancellation without fallback or a failure record. |
| + |
| +4. **P1 — Plan 5, Task 8: installed-build changes are ignored while a hold is stopping or waking.** [`observe`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:3670) skips both serving-state observation and `wantBuild` whenever `Current` is unavailable but `Owns` remains true. During a hold’s drain, root can install C and the pass can read that stamp, yet the scheduler revision still names B. Plan 3’s new `pinCheck` consequently passes and executes superseded B. This violates §6.3 despite the added re-checks. **Smallest fix:** always update the desired installed build and its revision; defer only serving-state updates and admission while the entry is unstable. |
| + |
| +5. **P1 — Plan 3, Task 2 / Plan 5, Task 12: the new retained failed wakes disappear from root’s pin set.** The P2 fix retains an unproven generation in [`unprovenWakes`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md:1911), but the existing failed-wake transition deletes its normal entry. [`generationsLocked`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:7051) enumerates only normal entries. Its next publication removes the retained generation’s pin while descendants may still run. Once install-history pins advance, root may sweep or restage that artifact, violating §§4.2 and 8.5. **Smallest fix:** include `unprovenWakes` in generation snapshots and retain their pins until cleanup proves termination. |
| + |
| +6. **P2 — Plan 1, Task 5: a switch can turn an ordinary POST into a 502 before any request bytes are sent.** When the chosen generation disappears, [`RoundTrip`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-1-generations.md:2911) retries only requests without a body or with `GetBody`. Incoming proxied POSTs normally have neither replay mechanism, so a pre-send `genMovedError` becomes a proxy failure. Separately, [`dialGen`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-1-generations.md:2781) accepts an exclusive old socket after detecting the switch, contrary to rev 17 §7.2’s explicit retry rule. **Smallest fix:** finish generation selection and switch retries before handing the request body to a transport; reject a connection whose generation changed before dial completion. Test with a streaming POST lacking `GetBody`. |
| \ No newline at end of file |