amadan
zdd-readiness: Astra's final review round 2 (ACCEPT WITH FIXES dormant; REJECT flag-on)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+46
−0
docs/superpowers/plans/CODEX-FINDINGS-ZDD-READINESS-R2.md+46 −0
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-ZDD-READINESS-R2.md b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-READINESS-R2.md| new file mode 100644 |
| index 000000000..d08f20c1c |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-ZDD-READINESS-R2.md |
| @@ -0,0 +1,46 @@ |
| +**ACCEPT WITH FIXES for dormant merge; REJECT for flag-on.** Finding 5 remains partial. Keep `CARLOS_SWAP` unset. |
| + |
| +Reviewed `379e8ead..4d93f6bc`, the controller’s brief, implementer’s report, ledger and roll notes. No files changed or tests run. Reported test and simulation results were inspected, not independently reproduced. |
| + |
| +1. **Finding 1 — ADDRESSED.** [manager.go:913](internal/rollout/manager.go:913) creates an incarnation; attempt and wake identities include it. `wakeIntent` still checks only the `w` prefix, so old persisted identities remain readable. I found no production consumer parsing the former numeric suffix. The restart regression covers renewed marker identity, sequence and timestamp, and rejection of the earlier grant and rollback. |
| + |
| +2. **Finding 2 — ADDRESSED.** [policy.go:177](internal/rollout/policy.go:177) evaluates restriction eligibility independently of transient status. [rolloutdeclined.go:176](internal/host/rolloutdeclined.go:176) uses it for the launch gate, and `retryHost` uses it before applying grants. The tests exercise landing/restoring rows with records and markers, plus retry during a move. This also closes the declined-box T10 consumed-grant defect. |
| + |
| +3. **Finding 3 — ADDRESSED for cross-book binding and the tested crash sequence.** [failures.go:940](internal/rollout/failures.go:940) requires matching nonempty book identity and epoch. Status, console joining, signing and box application carry that identity. Open advances the sequence at [failures.go:385](internal/rollout/failures.go:385), and armed startup flushes before constructing the manager. |
| + |
| + **Qualification:** the `1<<20` advance is a bounded mitigation, not an unconditional uniqueness guarantee. There is no enforced limit on unsaved increments. If those exhaust the gap, a later incarnation can reuse an epoch. The roll notes’ claim that reopening passes *any* previously issued epoch is stronger than the implementation. Before flag-on, enforce the bound, use a durable reservation/incarnation mechanism, or obtain an explicit ruling accepting this limit. |
| + |
| +4. **Finding 4 — ADDRESSED.** [failures.go:950](internal/rollout/failures.go:950) records a clear boundary when only the marker was removed. No-op retries still return before that boundary. Both scheduled-attempt and planned-wake late failures have regressions. |
| + |
| +5. **Finding 5 — PARTIAL, Medium, Flag.** The original single-retry sequence works, and unrelated attempts no longer spend its entitlement. But [queue.go:470](internal/rollout/queue.go:470) retains only one `forceBuild`. |
| + |
| + **Still-failing sequence:** B’s cohort is halted; retry host H for B while H wants A. Before H adopts B, retry H for another restricted build C. The second retry overwrites B’s entitlement with C. Later H wants B: its book record already lists H as retried, but B’s halted cohort rejects admission. Retrying B again cannot restore the entitlement because `retryHostLocked` returns false for an already-retried host ([failures.go:883](internal/rollout/failures.go:883)). |
| + |
| + Retain entitlements by host **and build**, or define and implement explicit supersession that leaves a recoverable book state. Add the two-build regression. |
| + |
| +6. **Finding 6 — ADDRESSED for the production classifier.** [activator.go:1887](internal/activator/activator.go:1887) captures the pre-launch layout; retained cleanup carries it. [opset.go:368](internal/activator/opset.go:368) requires host proof for unknown, flat, foreign, fenced or conflicting launch layouts, and for disagreement at cleanup. |
| + |
| + The backend classifies again, but flat versus nested is determined by the captured route’s paths ([layout.go:103](internal/activator/layout.go:103)); mutable fence/conflict inputs refuse launch. I did not establish a production nested→flat→nested bypass. Failed-seed and normal-stop proof remain separate debts. |
| + |
| +7. **Finding 7 — ADDRESSED.** [fake_test.go:91](internal/rollout/fake_test.go:91) installs the adapter before manager construction and swaps hooks atomically. The status probe now exercises both `Current` and `Owns`. The report records successful focused `-race -count=50` runs. |
| + |
| +**Merge items** |
| + |
| +- **Item 8 — ADDRESSED.** [cgroup.go:54](internal/cgroup/cgroup.go:54) owns the shared naming constants, builder and recognizer. Host construction and retaining setup use them; the cross-package test checks host and cgroup names. Existing generated names remain unchanged. |
| +- **Item 9 — PARTIAL.** The “Still owed” list retains the unfixed Flag categories, and correctly records T10 as closed. Recovery now preserves the failure book, recommends upgrading after schema refusal, and accurately describes manual populated-parent recovery. However, the closure at [roll-notes.md:62](docs/superpowers/plans/2026-10-07-zero-downtime-deploys-6-roll-notes.md:62) must reflect finding 5’s remaining sequence. Qualify the unconditional epoch claim at line 173 too. |
| + |
| +**Other diff checks and ledger rulings** |
| + |
| +`held_book` and grant `book` are additive JSON fields. New boxes reject empty/mismatched book grants; older boxes do not acquire that protection merely by receiving an extra field. Consequently, [roll-notes.md:264](docs/superpowers/plans/2026-10-07-zero-downtime-deploys-6-roll-notes.md:264), “Nothing is cleared that should not be,” overstates mixed-version safety. Retain sequencing as a flag gate. |
| + |
| +I found no new lock inversion or direct disk write under `Service.mu`. Scheduler-held book saves and indirect activator waits remain. Flag-off changes are confined to the authorized failed-wake proof work and shared containment-name implementation. |
| + |
| +**Scope ruling: agree**, provided deferred Flag items remain blockers. **Declined-box save ruling: conditional agreement.** Ordinary declined operation clears existing restrictions rather than recording fresh executions. But its rationale is too broad: `epochOlderLocked` also allocates epochs, and reconciliation merely attempts a save; failure is logged and operation continues ([rolloutdeclined.go:87](internal/host/rolloutdeclined.go:87)). Do not describe persistence as guaranteed. |
| + |
| +**Remaining gates** |
| + |
| +For **dormant merge**, correct the overclaims and complete the normal integrated-tip gate. The reported focused checks and 3000-seed/M1–M12 sweep are useful evidence, but do not substitute for the repository’s full landing gate. |
| + |
| +For **flag-on, code and contract work** remains: finding 5; a sound bound or explicit ruling for crash-epoch allocation; declined nested identity binding, ungated seeds and stranded sidecar recovery; one-shot good publication after fencing/lease loss and duplicate wakes; uncapped cgroup error paths; failed-seed and normal-stop containment; declined browser hold/retry UI unless waived; host-retry spec correction; and mixed-version sequencing. Rerun relevant regressions and simulation after gate changes. |
| + |
| +For **real-box evidence**, obtain scoped approval and complete delegated arm64 tests as `User=carlos`, including populated-parent refusal with positive controls, claim-containment respawn, killed-parent children, retained directories and systemd-stop behavior; real-unit control/sidecar checks; dev rollout and retry sequencing; and flagship probe gates. D1 remains reported complete. Claims-on swaps remain a separate future feature; those boxes must continue declining. |
| \ No newline at end of file |