amadan
zdd-5: Astra's final review, round 2 (REJECT; wave 1 re-review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+75
−0
docs/superpowers/plans/CODEX-FINDINGS-ZDD-IMPL-5-R2.md+75 −0
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-ZDD-IMPL-5-R2.md b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-IMPL-5-R2.md| new file mode 100644 |
| index 000000000..825a7e78b |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-IMPL-5-R2.md |
| @@ -0,0 +1,75 @@ |
| +REJECT |
| + |
| +Reviewed `db81e734..e0f231c4` against round 1 and spec rev 17. No files or git state changed; no tests or simulations run. Findings 1–4 and 8 remain incomplete. I accept the explicit parking of 5 and the three declined-box residuals **for dormant merge only**. |
| + |
| +**1. Round-1 findings** |
| + |
| +| # | Status | Evidence | |
| +|---|---|---| |
| +| 1 | **PARTIAL** | Gates now reach both backends, but the flat gate at [backend_exec.go:1457](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/backend_exec.go:1457) still precedes cgroup preparation and launch retries. [cgroup.go:312](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/cgroup/cgroup.go:312) applies limits and logs before starting; line 328 retries a failed launch without rechecking. Ownership can change during that work. Carry the gate to **each actual start**, after preparation. | |
| +| 2 | **PARTIAL** | Execution evidence prevents the direct restore, but [workerswap.go:121](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/workerswap.go:121) stops an executed, superseded hold worker without preserving its recovery restriction. Its next wake can restore the predecessor: N2 below. | |
| +| 3 | **PARTIAL** | The fence is installed before entry removal, and failed observations retain `firstWatch` through delivery. However, [wakeplan.go:579](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/wakeplan.go:579) deliberately permits planning after five seconds without the outcome: N1. | |
| +| 4 | **PARTIAL** | Token-matched deletion now protects newer planned/observing entries at [rollout/wakeplan.go:286](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/wakeplan.go:286). Config activation checks a cached `newest` result after unlocking and doing other work: N3. | |
| +| 5 | **PARKED-AGREE** | The hole remains at [failures.go:100](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/failures.go:100): failed persistence leaves only memory. Acceptable solely as an explicit enablement blocker; generations-file publication is not a substitute for durable failure state. | |
| +| 6 | **PARTIAL** | [rollout.go:300](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/host/rollout.go:300) now loads declined-mode protection; [rolloutdeclined.go:107](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/host/rolloutdeclined.go:107) checks both stamp and live link. Worker respawns, execution-time validation, and retry refresh remain explicitly parked. Agree with parking until enablement; this is not complete failure-book enforcement. | |
| +| 7 | **ADDRESSED** | Managed sidecars retain `Delivered`, and unmanaged socket routes regain their restart at [syncexec.go:623](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/appconfig/syncexec.go:623). Refused sleepers return the delivery-on-wake sentinel at [rollout.go:420](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/host/rollout.go:420), mapped to `Hibernating` at [syncexec.go:1295](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/appconfig/syncexec.go:1295). | |
| +| 8 | **PARTIAL** | [scheduler.go:208](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/scheduler.go:208) restores an unspent forced allowance, build-scoped by [queue.go:455](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/queue.go:455). The new execution evidence can incorrectly report “never executed”: N4. | |
| +| 9 | **ADDRESSED** | Original worker wording restored at [workerbuild.go:38](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/workerbuild.go:38); the staged sentinel has a separate member-safe mapping at [logsink.go:51](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/logsink.go:51). | |
| + |
| +**2. New failure paths in the fix diff** |
| + |
| +**Flag off:** No new runtime defect identified in this diff. The execution gate is a no-op without its context value; the config split requires the new callbacks. |
| + |
| +**Flag on:** |
| + |
| +- **N1 — High: the outcome fence fails open.** |
| + [wakeplan.go:574](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/wakeplan.go:574), [activator.go:1704](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/activator.go:1704). Hold B executes and fails; its outcome remains undelivered; another wake waits five seconds, then plans and can execute B without `HeldDown`. The new [bounded-fence test:126](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/outcome_fence_test.go:126) explicitly expects this reopening. **Smallest fix:** return a refusal/cancellation on timeout, retain the fence, and never invoke the planner until delivery completes; test that timeout launches nothing. |
| + |
| +- **N2 — High: stopping the superseded hold worker permits a later automatic fallback.** |
| + [workerswap.go:121](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/workerswap.go:121). Worker H serves good A; hold B executes. A sibling fails B, causing [queue.go:410](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/queue.go:410) to supersede H’s attempt. The new branch stops H and returns `SwapAbandoned`, so H receives no hold-down. On its next ordinary wake, B’s sibling failure blocks B and [rollout/wakeplan.go:74](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/wakeplan.go:74) selects A—against a database B may have migrated. **Smallest fix:** preserve a host-specific recovery restriction before releasing the attempt, including for executed hold abandonment; subsequent planning must honour it without misreporting cancellation as a build failure. |
| + |
| +- **N3 — Medium: config activation uses a token check that can become stale.** |
| + [rollout/wakeplan.go:283](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/wakeplan.go:283), [rollout/wakeplan.go:316](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/wakeplan.go:316). Old wake W1 passes the token check, then pauses during or after `served`. W2 plans and activates c2. W1 resumes with cached `newest=true` and activates c1. [config.go:65](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/rollout/config.go:65) regresses `active`, potentially leaving permanent “config pending” after c2’s queued work has already cleared. **Smallest fix:** validate the wake token in the same manager critical section that changes config activation state; keep acknowledgement I/O outside that lock. |
| + |
| +- **N4 — Medium: unknown execution is treated as an unused retry.** |
| + [workerswap.go:101](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/workerswap.go:101) returns on context cancellation before reading `attemptExec`, leaving the new result false even when the independent respawn has executed or can still execute. Separately, [swap.go:482](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/swap.go:482) equates a nil process handle with no execution, although [backend_exec.go:1678](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/internal/activator/backend_exec.go:1678) can return nil after a nested launch whose execution/termination is uncertain. An abandoned forced attempt therefore regains an allowance it may have spent. **Smallest fix:** distinguish proven-unexecuted from executed/unknown; join or conservatively classify unfinished respawns, and re-arm only proven-unexecuted attempts. |
| + |
| +**3. New ledger rulings** |
| + |
| +References below are to [progress.md:178](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/.superpowers/sdd/2026-10-07-zero-downtime-deploys-5-scheduler/progress.md:178). |
| + |
| +| Line / ruling | Judgment | |
| +|---|---| |
| +| 178 — sequential dispatches | **Agree:** package-scoped sequential fixes are reasonable. | |
| +| 179 — F1 launch gates | **Agree with conditions:** “after all preparation” must include cgroup preparation and every launch retry. | |
| +| 180 — F2 executed hold stops | **Disagree:** stopping without retaining a recovery restriction still permits automatic fallback on the next wake. | |
| +| 181 — F3 bounded outcome fence | **Agree with conditions:** bound the waiting request, never the safety restriction. | |
| +| 182 — F4 wake tokens | **Agree with conditions:** activation must atomically validate its token when changing state. | |
| +| 183 — F5 parked | **Agree with conditions:** dormant merge only; a transient failed write followed by restart remains an enablement blocker regardless of generations-file checks. | |
| +| 184 — F6 declined protection | **Agree with conditions:** its promised enforcement requires closing the three residuals before any enablement. | |
| +| 185 — F7 config split | **Agree:** delivery, restart ownership, and sleeper acknowledgement now follow their respective routes. | |
| +| 186 — F8 retry re-arming | **Agree with conditions:** only proof of no execution authorizes another allowance. | |
| +| 187 — F9 worker wording | **Agree:** the separate sentinel preserves flag-off behavior. | |
| +| 191 — record older wake failures | **Agree with conditions:** retain real failures, but respect explicit retry/promotion ordering so delayed pre-retry outcomes cannot undo newer recovery authorization. | |
| +| 193 — declined bootstrap | **Agree:** a sibling record alone must not suppress the no-good bootstrap exception. | |
| +| 194 — park worker converge/recycle | **Agree with conditions:** dormant-only deferral; loss of delegation must not become permission to execute held builds. | |
| +| 194 — park plan-time-only refusal | **Agree with conditions:** validate the selected executable at launch before enabling. | |
| +| 194 — park startup-only records | **Agree with conditions:** implement supported retry refresh before enabling; restarting while still declined merely rereads the file and does not itself process a newer signed retry pointer. | |
| + |
| +**4. What remains owed** |
| + |
| +**Before dormant merge:** |
| + |
| +- Complete F1’s actual-start checks and fix N1–N4, with deterministic regressions for those interleavings. |
| +- Keep F5 and the three declined-mode residuals explicitly open as enablement blockers. |
| +- Obtain the final-tip repository gate and cross-build evidence. The ledger’s full gate is for `5b6c95ed`; the fixer reports do not establish a complete final-tip gate. I ran none. |
| +- The bounded concurrency test, short-window refusal regression, and mutation-script validation/cleanup changes are present; mutation cleanup execution remains unverified in the report. |
| + |
| +**Before `CARLOS_SWAP` is enabled anywhere:** |
| + |
| +- All merge fixes above; durable unresolved-attempt/outcome recovery for F5; complete declined-box launch, worker, and retry enforcement. |
| +- Remaining round-1 **Flag** items remain binding in [the original triage](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/docs/superpowers/plans/CODEX-FINDINGS-ZDD-IMPL-5.md:120): policy-cache ordering/backoff; retry-generation boundaries and unrelated retry consumption; eligibility revalidation; bounded store/ack operations and shutdown; worker readiness/fallback/one-shot behavior; good-artifact retention; publication-before-stop; truthful status; relay bounds; independent simulation assertions and reproducible replay. |
| +- Complete remaining config delivery/removal coverage and blocked-publication tests. Wave 1’s sidecar/unmanaged/sleeper tests do not close that whole list. |
| +- Finish plan 6’s member-safe error handling, reason classification, copy, and migration guidance. |
| +- Prove containment and the populated-legacy-parent guard, including crash-surviving children; complete D1, real-unit control/`KillSidecar`, and nondefault-path checks recorded at [ledger:19](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-5-scheduler/.superpowers/sdd/2026-10-07-zero-downtime-deploys-5-scheduler/progress.md:19). |
| +- Keep claims-backed launches excluded until isolated launch, selected config restart, and the deferred ownership/classification obligations are completed. |
| \ No newline at end of file |