carlos / platform Public

Astra review I1 of the instance-settings plans (REJECT, 12 findings)

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

claude 2 weeks ago 84cd5b34eee50d5d14d4dd4ca9d46666e54b9a55 parent 27525e6

1 file changed, +25 −0

docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I1.md +25 −0

CI cancelled — superseded by a newer push — run details

diff --git a/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I1.md b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I1.md

new file mode 100644

index 000000000..e2782b399

--- /dev/null

+++ b/docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I1.md

@@ -0,0 +1,25 @@

+REJECT

+1. **High — Plan 2, Task 3, step 3: a refused promotion still authorizes the build.** The [record-write insertion](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-2-promotion-records.md:759) precedes the authoritative authorization callback in `origin/main:internal/release/release.go:783`. The existing test `TestAuthorizeSeesAPipelineInstalledBeforeTheAuthoritativeRead` demonstrates the schedule: the final permission check rejects a never-promoted build. Following this plan leaves its signed promotion record behind, making it pinnable despite that rejection. This violates **§4.1**. **Smallest fix:** write the record after the retry guard and successful authorization callback, before publishing the pointer; extend that test to assert record absence.

+2. **High — Plan 2, Task 7: held staging bypasses the digest ratchet and mandatory-digest flag.** [HeldAuthorized](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-2-promotion-records.md:1422) accepts pointers through `Channel` without ratchet admission. A digestless match subsequently uses `materializeHeldStage`. Consequently, a replayed legacy pointer can install substituted bytes during a move even after ordinary adoption ratcheted that channel, or with `CARLOS_REQUIRE_POINTER_DIGEST` enabled. This violates **§4.6**. **Smallest fix:** pass the box’s digest admission policy through held staging and apply it before accepting pointer evidence.

+3. **High — Plan 2, Task 3, step 4: repeat promotions automatically backfill authorization.** [ensurePromotionRecord](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-2-promotion-records.md:780) signs the bucket’s current manifest when an existing head lacks a record, although the command returns `NoOp`. For a legacy head whose bytes were replaced, this blesses the replacement without the required digest confirmation. **§4.1 explicitly forbids automatic backfill.** **Smallest fix:** leave absent records absent on no-op promotions and retry renewals; require `releases authorize` to authorize legacy bytes.

+4. **High — Plan 1, Task 6: a failed mark write leaves the ratchet open.** [DigestRatchet.note](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-1-pointer-digests.md:1540) returns on filesystem errors before setting `seen`, while `Admit` returns success. With an unwritable directory or full disk, the very next digestless replay is accepted—even by the same process. This violates **§4.6’s “once … verified” ratchet**. **Smallest fix:** remember observation immediately and propagate persistence failures so installation/extraction cannot proceed without durable ratchet protection.

+5. **High — Plan 4, Task 5: build state can be written after the executable touches data.** The [legacy-wake instructions](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1186) record the live-link version, execute that mutable link, then repair a mismatch afterward. A repoint followed by a crash leaves durable state naming the wrong build. The same instructions skip the pre-seed write for planned/pinned wakes, although `origin/main:internal/activator/activator.go:1198` seeds before spawning. Both violate **§4.2**. **Smallest fix:** retain one executable identity, record it before the seed and spawn, and execute that identity with swapping either enabled or disabled.

+6. **High — Plan 4, Tasks 4 and 9: unreadable build states permit pruning.** Refresh errors leave unknown or stale registry copies; [buildStatePins](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2215) silently omits missing or malformed states. The resulting fresh, readable box report looks complete to the housekeeper, which can prune the omitted build and promotion record. This violates **§4.4 and §8’s explicit unreadable-build-state refusal**. **Smallest fix:** report build-state-read completeness per affected app/host and refuse pruning while it is incomplete.

+7. **High — Plan 3, Task 4, step 4: unreadable defaults turn a held instance into auto-update.** The [mint instructions](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-3-console-settings.md:717) discard `policyFor`’s `known` result and create default settings on read failure. An instance inheriting `manual`, moving to a fresh box during a manifest-read failure, therefore receives `auto` and adopts the head instead of recovering `last_executed`. This violates **§3.2 and §4.3 rule 3**. **Smallest fix:** defer minting/adoption until the policy is known; distinguish genuine manifest absence from read failure.

+8. **High — Plan 4, Tasks 6 and 7: a baking request disables a major hold’s wake check.** [settingsTarget](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1587) replaces `ReasonMajor` with `ReasonBaking`. `heldTarget` at line 1889 recognizes major holds only from the consumed reason, so it now returns `held=false`. While recovery is pending, a returning instance can execute its stale live link instead of `last_executed`. This violates **§4.3’s unresolved-request and wake rules**. **Smallest fix:** preserve the held decision independently of the request’s baking explanation.

+9. **Medium — Plan 4, Task 6: independent recovery incorrectly requires a readable channel pointer.** The [settings insertion](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1530) leaves `origin/main:internal/release/adopt.go:956`’s early `ptr == nil` return intact. A fresh box cannot install a record-authorized pin, resolved request, or recovery target when the channel pointer is unavailable. **§4.3 rules 1–3 do not require that head.** **Smallest fix:** evaluate independent targets before the head-only guard and condition pointer-dependent reads on pointer availability.

+10. **Medium — Plan 4, Tasks 2, 5 and 6: a request whose target fails after spawning never becomes completed-failed.** [beforeExec](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1242) completes R with `CompletedFailed=false`. Subsequent `Target` calls check T’s failure only while R exceeds the watermark, which is now false. No task propagates the later health/hold failure. This violates **§4.3 rule 2**. **Smallest fix:** update the matching completed request’s outcome after failure, including equal-watermark updates, without reopening the request.

+11. **Medium — Plan 5, Task 4, step 4: the CLI decodes the report from the wrong JSON field.** The [implementation instruction](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-5-surfaces.md:898) uses `json:"updates"` for `*apiUpdatesJoin`; the task’s interface contract and server use `update_report`. `updates` already holds the setting string. Following the step loses the report or fails decoding `"updates":"manual"`, breaking **§5’s instance list**. **Smallest fix:** use `update_report` for the report and retain a separate string field for `updates`.

+12. **Medium — Plan 5, Task 4: inherited holds are deliberately hidden from the list.** The [rendering instructions](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-5-surfaces.md:938) gate reasons on explicit record overrides and expressly omit inherited manual holds. Inherited major holds also lack the effective major number. **§5 requires the reason an instance is behind**, regardless of inheritance. **Smallest fix:** render the effective reported reason and major instead of gating on explicit settings.

\ No newline at end of file