Astra review P4 of the zero-downtime plans (REJECT: 5 partial, 4 new)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+20
−0
docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P4.md+20 −0
CI cancelled — superseded by a newer push — run details
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P4.md b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P4.md| new file mode 100644 |
| index 000000000..ffd8a4ffe |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-PLANS-P4.md |
| @@ -0,0 +1,20 @@ |
| +REJECT |
| + |
| +P3 partial P1 #1: PARTIAL — failed-wake leader retention is fixed; claims-on termination still accepts leader exit after `KillAndEmpty` fails. |
| +P3 partial P1 #6: PARTIAL — first-wake `buildFailed` still reads the pointer before publishing the failure hold. |
| +P3 partial P2 #3: PARTIAL — synchronous sidecar discard still forgets an unproven process; the join timeout also permits stop completion. |
| +P3 partial P2 #4: PARTIAL — overlap preparation is covered; claims-on hold wakes still check before admission/restore, without checking again at exec. |
| +P3 #1: CLOSED — good-build and serving-build publication now precedes slot release and checks generation identity. |
| +P3 #2: CLOSED — cold managed hosts materialize and refresh their installed identity before planning. |
| +P3 #3: PARTIAL — claims-on `startAdmitted` still records cancellation through `pinFailed`, bypassing `classifyStep`. |
| +P3 #4: CLOSED — installed-build observation now updates the desired revision during unstable entry states. |
| +P3 #5: CLOSED — retained failed wakes are included in generation pins. |
| +P3 #6: CLOSED — post-connect generation checks and streaming-body retries address both reported cases. |
| + |
| +1. **P1 — Plan 3, Tasks 2/11; Plan 5, Task 9: worker termination proves the wrong cgroup empty.** Workers still spawn in the **host cgroup** ([Plan 4, Task 8](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-4-release-side.md:2537)), but Plan 3 assigns them numbered generations. `genTerminated` consequently calls `KillGen` on `g<n>`, whose absence [counts as successful termination](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-2-containment.md:3931). The actual [`childProc.Stop`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/internal/activator/backend_exec.go:1531) signals only the leader. A forked descendant can therefore survive lease release. Plan 5’s worker `killFailed` likewise checks only leader exit before fallback, allowing two workers to continue work concurrently. This violates §§7.4 and 7.9. **Smallest fix:** prove the actual worker host cgroup empty before replacement, fallback or lease release; retain ownership when that proof fails. |
| + |
| +2. **P2 — Plan 5, Tasks 7–8: config activation can regress after a newer attempt completes.** [`done`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:4414) releases the running slot, reads the registry, and only then calls `configActivated`. If successful config A pauses in that read, config B can complete before A resumes. [`configTrack.activate`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:2561) then unconditionally restores active=A, although B runs and its pending attempt has been cleared. A registry-read failure instead loses activation entirely. Both leave delivered≠active without the required retry, violating §8.1. **Smallest fix:** publish config activation before releasing the slot, independently of the registry read used for event reporting. |
| + |
| +3. **P2 — Plan 6, Task 13; Plan 5, Tasks 8/14: flag-off boxes lose sidecar running-version reporting.** [Task 13](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-6-surfaces.md:4475) unconditionally changes the API to read `ServingVersion`. Its writers belong to the rollout manager, which [`armRollout`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md:8202) leaves unarmed with `CARLOS_SWAP` unset. Running sidecars therefore show “unknown”; disabling the flag after using it can leave an obsolete version displayed indefinitely. This violates §7.9 and the flag-off contract; Plan 4’s exceptions do not authorize losing the serving identity. **Smallest fix:** publish captured worker identity on successful wakes and respawns independently of the rollout flag. |
| + |
| +4. **P2 — Plan 1, Task 5: the no-current-generation branch bypasses response attribution.** [`RoundTrip`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-1-generations.md:2960) waits through an unbound transport when no generation is current, then returns without stamping `X-Carlos-Version` or recording the serving generation. A request admitted before a stop can consequently reach the replacement successfully but return no platform version—or an incorrect application-supplied value. That transport also bypasses the post-connect generation check. This violates §7.2; the plan’s waiting-for-generation test explicitly expects no attribution. **Smallest fix:** wait for `ServingGen` within the retry bound, then use the normal generation-specific transport and attribution path. |
| \ No newline at end of file |