carlos / platform Public

zdd-readiness: Astra's whole-branch final review, round 1 (ACCEPT WITH FIXES dormant; REJECT flag-on)

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

claude 1 week ago 379e8ead9178d1353b0982469c56f1748aa24ef8 parent 78ef3b8

1 file changed, +126 −0

docs/superpowers/plans/CODEX-FINDINGS-ZDD-READINESS.md +126 −0

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

new file mode 100644

index 000000000..7893d3fd9

--- /dev/null

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

@@ -0,0 +1,126 @@

+**ACCEPT WITH FIXES for a dormant merge; REJECT for flag-on.** Keep `CARLOS_SWAP` unset. Several claimed closures remain incomplete, and some deferred “minors” are safety or recovery blockers.

+Reviewed `5da1cc9b..78ef3b81` against rev17, the corrected plan, earlier findings and execution ledger. No files changed or tests run; test results below are reported evidence, not independently reproduced.

+1. **High — T3/4/5: execution identities repeat after restart. — Flag**

+ [manager.go:881](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/manager.go:881) starts wake and attempt counters afresh. Persisted markers retain identities such as `w1`. [failures.go:654](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/failures.go:654) treats the same build and identity as a duplicate, retaining its old timestamp and epoch.

+ Sequence: B executes and leaves a marker without a failure record; restart the agent; its first wake executes B again and leaves another marker. Both executions can be `w1`. An earlier signed retry still matches, and a rollback timestamp between the two executions can incorrectly authorize clearing the newer execution’s restriction.

+ **Smallest fix:** add a unique manager incarnation to execution identities, or persist their allocation. Test reopening the book with a new manager, including stale rollback and retry grants. N6 and rollback-provenance closure are incomplete.

+2. **High — T9: the claims move launch gate excludes actual move rows. — Flag**

+ [claimmoveaccess.go:168](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/host/claimmoveaccess.go:168) invokes the new gate, but [rolloutdeclined.go:163](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/host/rolloutdeclined.go:163) returns unrestricted when `rollout.Managed(rt)` is false. That predicate excludes held statuses.

+ Actual move probes require a held row: [movesprobeowners.go:255](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/host/movesprobeowners.go:255). Their launch therefore bypasses an inherited hold-down on a declined claims box. The new test uses a ready row and misses this.

+ **Smallest fix:** evaluate restriction eligibility independently of transient move status, preserving move authorization separately. Test held landing/restoring rows against failure records and markers. Declining claims-on swaps does not justify ignoring existing restrictions.

+3. **High — T8: signed retry epochs are not unique across books. — Flag**

+ [hostretry.go:21](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/release/hostretry.go:21) signs account/app/host/build and a numeric epoch, without a book or restriction incarnation. [failures.go:867](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/failures.go:867) accepts numeric equality.

+ Independent books can assign the same epoch to different restrictions. Following relocation or recreation of the same host, an earlier valid grant can clear a later restriction within its 24-hour window. Root signing prevents forgery, but does not establish the claimed replay protection.

+ **Smallest fix:** sign and verify a persistent unpredictable restriction identity, or book incarnation plus epoch. Test two independent books with matching numeric counters. Also cover failed-save/crash counter reuse before declaring replay closed.

+4. **Medium — T4/8: a successful marker-only retry records no retry boundary. — Flag**

+ [failures.go:871](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/failures.go:871) removes a matching marker, but `retryHostLocked` returns without `clearedLocked` when no failure record exists.

+ A B execution begun before that successful retry can subsequently report failure and create a record/halt the cohort. It is not classified `PreRetry`. Retaining a safety marker for the late hold outcome is intentional; undoing retry ordering is not.

+ **Smallest fix:** advance the host clear boundary when a marker is actually removed, including without a record. Preserve no-op retry behavior. Test both scheduled and planned-wake late outcomes.

+5. **Medium — T8: retrying a non-current build loses its canary entitlement. — Flag**

+ [manager.go:826](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/manager.go:826) adds the host to B’s `Retried` set, but binds its queue retry only when the slot currently wants B.

+ If it wants A/C and later returns to B while B’s cohort halt remains, the host has a book exemption but no forced queue admission. Another host retry cannot recreate the entitlement because the record already says retried. App-wide retry or restart may recover it; the successful host retry itself does not.

+ **Smallest fix:** retain a host/build canary entitlement until that build becomes desired. Do not spend it on another build.

+6. **Medium — T15: failed-wake cleanup can infer containment from a changed layout. — Flag**

+ [opset.go:329](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/activator/opset.go:329) skips host-empty proof when `hostCgroupIsTheWakes` returns false. Its classifier reads the *current* layout at [opset.go:371](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/activator/opset.go:371).

+ A flat failed wake whose layout becomes fenced/conflicting before cleanup can release its lease without proving its descendants gone. Neither classification proves that the original launch had separate containment.

+ **Smallest fix:** retain the launch’s containment identity; at minimum, unknown/fenced/conflicting layouts must not count as proof. F6’s failed-wake closure remains conditional.

+7. **Medium — T17: the new status tests change dependencies after starting readers. — Merge**

+ [status_test.go:270](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/status_test.go:270) and [status_test.go:353](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/internal/rollout/status_test.go:353) assign `h.m.d.Act` after `h.pass()` starts scheduler activity. Readers have no corresponding synchronization.

+ **Smallest fix:** install the probe adapter before starting schedulers, then attach its test state safely. Run those focused tests with `-race`.

+**Rulings.** References below are line numbers in the [execution ledger](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/zdd-flag-readiness/.superpowers/sdd/2026-10-11-zero-downtime-flag-readiness/progress.md). “Conditional” means agree with the stated condition.

+| Lines | Judgment and reason |

+|---|---|

+| 9, 10 | Agree: claims-on declines; it must not turn the flag into an outage switch. |

+| 11 | Conditional: root signing is right; findings 1, 3 and 4 prevent the claimed restriction binding. |

+| 12 | Agree: unresolved intents conservatively become failures. |

+| 13 | Conditional: schema-2 downgrade refusal is sound; deleting authoritative restrictions must not become routine recovery advice. |

+| 14 | Conditional: signed rollback provenance needs execution identities that survive restart. |

+| 15 | Conditional: coverage/replay work can be distinguished from code defects; real-box evidence remains a flag gate. |

+| 16 | Agree: one full sweep per relevant branch revision is reasonable; rerun after these gate fixes. |

+| 41 | **Disagree:** manager-local counters are not durable execution identities. |

+| 47 | Agree: explicit authorization covers missing-record waiting even flag off; document this additional behavior change. |

+| 49, 51 | Agree: rerunning the installed marked build uses the served-build exemption. |

+| 52 | Conditional: the accepted dispatch-time rollback tradeoff still requires unique identities and explicit clock-skew handling. |

+| 53 | Agree: adoption onto good must respect the standing marker. |

+| 58 | Agree: gate by execution cause, not queue class. |

+| 59 | Conditional: postponing restart/config has supported recovery, but its invisible waiting needs follow-up. |

+| 63 | Agree: cgroup recreation is an authorized flag-off exception. |

+| 65 | Agree: the end-of-Plan snapshot chooses the conservative ordering. |

+| 66 | Conditional: no-op retries must not advance ordering; actual marker removal is not a no-op. |

+| 67, 68 | Agree: late hold outcomes retain database safety; marker time must not regress. |

+| 70 | Conditional: existing scheduler-locked saves may remain; document their latency, including indirect activator-lock waits. |

+| 71 | Agree: unchanged records can reassert memory-only halts. |

+| 74 | Agree: intent conflict uses the existing accurate refusal. |

+| 75 | Agree: join after activator shutdown; the implementation shares the shutdown deadline rather than necessarily adding 60 seconds. |

+| 81 | Agree: abandon dispatch when ownership/status is no longer eligible. |

+| 90 | Conditional: sidecar gating is correct; retained-incumbent sidecar recovery remains unresolved. |

+| 91 | Agree: gate before stopping the serving process. |

+| 92 | Conditional: acceptable only for dormant merge; nested identity race remains Flag. |

+| 93 | Conditional: marker/path changes are sound; the move-launch fix fails finding 2. |

+| 97 | Agree: hashing has an identity-binding purpose on those paths. |

+| 98 | Conditional: seed bypass remains Flag, despite its narrow preconditions. |

+| 101 | Conditional: CLI recovery permits dormant merge; the explicitly owed browser surface remains Flag absent waiver. |

+| 105 | Agree: counting plans/wakes/coalesced execution closes shutdown gaps; caller cancellation must not poison shared policy cache. |

+| 110 | Conditional: clean one-shot exit is success, but good publication must still validate ownership/fencing. |

+| 115 | Conditional: populated-parent fix is useful; other uncapped fallbacks remain Flag. |

+| 116 | Conditional: manual recovery needs an accurate operator procedure; do not claim automatic lifting. |

+| 120 | Conditional: retaining seeding and box-condition classification are sound; preserve claim evidence and obtain kernel proof. |

+| 124 | Agree: retaining seeding falls within the authorized recreation exception. |

+| 127 | Agree for undelegated legacy operation only; leader exit is not descendant containment. |

+| 128 | Conditional: widening F6 is justified; finding 6 defeats its universal claim. |

+| 130 | Conditional: scope may defer implementation, but failed-seed and normal-stop proof remain owed Flag gates. |

+| 135 | Agree: a named unit test can kill M12; add restart-incarnation coverage. |

+| 138 | Conditional: transient reporting is right; remove finding 7 and exercise `Owns` in the lock probe. |

+**Closure assessment.** The blanket statement that all flag-path code defects are closed is inaccurate. N6/provenance/retry ordering, declined move launches and F6 remain incomplete as above. Clean-one-shot publication also retains an ownership gap.

+I found the intended implementations for startup intent reconciliation, uncertain nested-start treatment, bounded policy/read/ack work, shutdown joining, worker execution revalidation, GoodVersion retention, pre-stop hold publication, transient reporting and `/run` sweeping. The populated-parent and killed-cgroup fixes exist but lack real-kernel confirmation. The reported simulation sweep does not cover the restart/replay sequences above.

+I found no new direct disk write under `activator.Service.mu`, demonstrated lock-order inversion, or unintended incompatible external wire shape. A declined gate can nevertheless wait under that mutex behind a book fsync. Local schema-2 downgrade refusal is intentional. New unsigned-retry rejection and missing-record waiting are explicitly ruled behavior changes beyond the four enumerated exceptions.

+**Deferred-item triage.**

+| Classification | Items |

+|---|---|

+| **Merge** | Finding 7; approve the three pending no-signing-key strings; correct overclaimed closures and misleading recovery instructions; protect T19’s duplicated claim-name format with a shared definition/test because recreation runs flag off; complete the normal gate on the final integrated tip. |

+| **Flag** | Findings 1–6; T8 failed-save/crash replay coverage; T9 nested identity race, ungated seeds and stranded legacy sidecar recovery; T10 grants consumed while rows are moving/landing; T12 good publication after fencing/lost lease and the duplicate-wake window; T14 all uncapped error fallbacks and positive-control kernel coverage; T15 failed-seed descendants and normal-stop proof; declined browser hold/retry surface unless waived; host-retry spec correction; mixed-version retry deployment sequencing. |

+| **Defer** | Claims-only intent/default-crash gaps until claims-on swaps; T8 overbroad epoch invalidation and API/browser no-key ordering; policy single-flight; harmless Pass-after-Close behavior; scheduling/log verbosity; extra hashing; kill-mark/lock-map growth; optional inode/reservation hardening; wording/comment/test-name cleanup; redundant kills/pins; stale documentation line numbers. |

+| **Defer, retain explicit coverage debt** | Remaining T1/2/4/5/6/7/9/10/12/13/16/17 unit/model omissions, rollback liveness/replay tooling, thin M8 detection and root-disabled I8. New regression tests for findings are not deferrable. Keep known flakes tracked; do not normalize a failing gate. |

+T10’s delayed `retryBuild` concern is already addressed by T11’s `onCleared` callback. T19’s `RecoverStopped` concern is resolved by inspecting containment names: they include controller/boot/session identity. The pre-existing watch-finalized-as-asleep gap remains reporting debt and should not be presented as solved by transient status.

+Pointer re-signing versus explicit rollback, timestamp skew, stale outcomes after pruning, effective-mode races and failed pinned-wake publication deserve explicit recovery tests before the flag gate; none establishes a new merge regression here. The unrequested `git worktree prune` recorded at ledger line 56 should also remain noted as a process deviation.

+**Remaining flag blockers.** Code must satisfy the Flag list, with corrected closure claims and regression coverage, followed by the relevant simulation/mutant sweep. Claims-on swaps remain a separate future feature; claims-on boxes must continue declining.

+Separately, obtain scoped approval and run the delegated arm64 kernel suite as `User=carlos`, including `TestGenerationStartRefusedBesideADirectParentMember` and `TestClaimContainmentCleanupRespawn`; verify killed-parent children, retained directories and systemd-stop behavior. Complete real-unit control/sidecar checks, dev rollout checks and the flagship probe gates. D1’s daemon-reload measurement is already reported complete. Every other owed gate requires evidence or Paul’s explicit waiver before flag-on.

\ No newline at end of file