Astra review I2 of the instance-settings plans (REJECT: 4 partial, 9 new)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+34
−0
docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I2.md+34 −0
CI passed — run details
diff --git a/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I2.md b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I2.md| new file mode 100644 |
| index 000000000..1fc654874 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I2.md |
| @@ -0,0 +1,34 @@ |
| +REJECT |
| + |
| +I1-1: CLOSED |
| +I1-2: PARTIAL — ratchet admission added, but its implementation signature is broken. |
| +I1-3: CLOSED |
| +I1-4: PARTIAL — observation is remembered only within a ratchet object that callers recreate. |
| +I1-5: PARTIAL — nested and claimed wakes are not covered correctly. |
| +I1-6: PARTIAL — completeness covers reported hosts, but not omitted hosts. |
| +I1-7: CLOSED |
| +I1-8: CLOSED |
| +I1-9: CLOSED |
| +I1-10: CLOSED |
| +I1-11: CLOSED |
| +I1-12: CLOSED |
| + |
| +New findings: |
| + |
| +1. **High — Plan 1, Task 8: recreating ratchets discards the new failure protection.** [The call-site instructions](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-1-pointer-digests.md:1997) construct a new `DigestRatchet` inside each materialization/adoption call. After `ErrRatchetUnpersisted`, the channel is remembered only in the discarded object’s `seen` map. The next pass in the same agent can therefore accept a digestless replay. This violates **§4.6’s once-observed ratchet**, despite the corrected unit test passing. **Smallest fix:** construct and share one ratchet per process across its adoption, materialization and held-stage paths; test persistence failure followed by a replay through successive production calls. |
| + |
| +2. **High — Plan 4, Task 5, step 4: nested `StartExe` does not execute the recorded identity.** [The new wrapper](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1348) passes a staged executable to `spawnUnitFrom` with generation **0**. The consumed `startRole` interface retains its generation-zero `Control.Start` branch; the staged-executable support is in its generation branch. The root API also rejects an explicit executable for a generation-zero run role. Thus the prescribed path still executes the live link, allowing a repoint after recording A to execute B. Nested `SeedExe` likewise has no corresponding staged-seed API. This violates **§4.2**. **Smallest fix:** add explicit staged-executable support for the original run and seed units, including root validation, and test both across a concurrent repoint. |
| + |
| +3. **High — Plan 4, Task 5, steps 5–6: the claimed primary spawn has no implementable pre-exec insertion.** [“Make the same edits in `doClaimWake`”](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1389) names a restore/seed/spawn sequence that is not there. On `origin/main`, `doClaimWake` delegates at `internal/activator/claimwake.go:131`; the primary execution occurs through `s.RoleLauncher.Start` at line 224. The task explicitly covers only `startAdmitted`’s **fallback**. Following those insertions leaves the claimed primary execution outside the build-state guarantee in **§4.2**. **Smallest fix:** specify the primary claimed start’s identity and hook explicitly, composing the write into its pre-start callback after materialization, under the admitted ownership, and test primary starts and retries. |
| + |
| +4. **High — Plan 4, Task 7: the staged wake shortcut bypasses failure records.** [The shortcut](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2099) creates `e.pin` whenever the requested file exists. The instructions then place this before planning, whose interface skips planning when `e.pin != nil`. Neither `heldTarget` nor this shortcut checks the failure book. Consequently, root can refuse a failed pin while the wake executes its staged copy through `startPinnedChecked` with no `pinCheck`. This violates **§4.3’s failure precedence and §6**. **Smallest fix:** submit the held identity through the scheduler’s failure-aware wake decision, or apply the same failure/hold-down checks before installing this internal pin. |
| + |
| +5. **High — Plan 4, Task 9: an omitted build-state entry still permits pruning.** [The report collector](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2537) returns `nil` when `reg.List()` fails. [The housekeeper](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2629) checks completeness only for hosts appearing in `BuildStates`; it never requires an entry for each relevant provisioned host. A fresh report with the map omitted therefore permits deletion of that host’s recovery build and promotion record. This violates **§4.4 and §8’s incomplete-state refusal**. **Smallest fix:** report collection failure explicitly and validate expected host coverage before pruning; test an omitted map, not only an entry with `Complete=false`. |
| + |
| +6. **Medium — Plan 4, Task 5: the identity fix removes cold-wake materialization.** [`wakeIdentity`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1373) resolves the live link before calling the backend and immediately fails if it is absent. Today, `origin/main:internal/activator/backend_exec.go:1146` calls `ensureLiveLink`, which invokes the materialization hook when needed. With swapping off, a default instance now fails before reaching that hook or its root-stage relay. This regresses **§4.3 rule 4**. **Smallest fix:** perform the existing materialization step before capturing the identity, then record and execute that captured file. |
| + |
| +7. **Medium — Plan 4, Task 7: wake precedence contradicts root’s pin precedence.** [`heldTarget`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2145) returns a pending request’s T **before** checking `u.Pin`. A user can request T, then pin P before the sleeping instance wakes. Root correctly installs P, but the wake selects T; with T staged it runs T, otherwise it waits for a repoint root will not perform. This violates **§4.3 rules 1–2**. **Smallest fix:** check the pin before pending requests in the wake decision and test pinning after request resolution. |
| + |
| +8. **Medium — Plan 4, Task 6: downstream proof checks examine the head instead of the selected build.** [The adoption rewrite](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1773) changes `ver` but retains the real head pointer whenever one exists. `origin/main:internal/release/proofgate.go:255` derives proof evidence from `ptr.Version`, and line 337 hashes that version’s staged file. A valid recovery onto B can therefore be blocked by an unrelated head H lacking proof or a staged artifact; conversely, H’s proof is checked when B is installed. **§4 requires the existing adoption gates to apply to the selected build.** **Smallest fix:** carry a separate selected-build pointer/context into the proof checks, preserving the actual head separately for head-specific bake and refusal logic. |
| + |
| +9. **Medium — Plan 2, Task 7, step 3: `HeldAuthorized` does not compile.** [Its implementation signature](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-2-promotion-records.md:1457) omits `r *DigestRatchet`, while its body references `r` and the new caller supplies it. This contradicts the task’s corrected interface block and prevents the I1-2 fix from building. **Smallest fix:** add the ratchet parameter after `pub`, matching the interface and caller. |
| \ No newline at end of file |