carlos / platform Public

Astra review I3 of the instance-settings plans (REJECT: 1 partial, 8 new)

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

claude 2 weeks ago cc9e17782cb18a6038e68b95d971ba19ad277d74 parent b027eef

1 file changed, +33 −0

docs/superpowers/plans/CODEX-FINDINGS-SETTINGS-PLANS-I3.md +33 −0

CI cancelled — superseded by a newer push — run details

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

new file mode 100644

index 000000000..6aee4f53b

--- /dev/null

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

@@ -0,0 +1,33 @@

+REJECT

+I1-2: CLOSED

+I1-4: CLOSED

+I1-5: CLOSED

+I1-6: CLOSED

+I2-1: CLOSED

+I2-2: CLOSED

+I2-3: CLOSED

+I2-4: PARTIAL — the replacement depends on a scheduler absent with `CARLOS_SWAP` unset.

+I2-5: CLOSED

+I2-6: CLOSED

+I2-7: CLOSED

+I2-8: CLOSED

+I2-9: CLOSED

+New findings:

+1. **High — Plan 4, Tasks 4–5: initialization failure silently disables build-state enforcement.** [Task 4 explicitly says to log and continue when `newBuildStates` fails](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1102). Task 5 installs `BeforeExec` only when `bs != nil`, and its nil-hook branch permits execution. Thus an initialization error leaves the agent able to spawn without recording `last_executed`; Task 10 still advertises settings support. This violates **§4.2’s mandatory write-before-execution guarantee**. **Smallest fix:** when replica-backed execution is configured, fail initialization or install a refusing hook until the build-state service is available. Reserve the nil hook for configurations outside that requirement.

+2. **High — Plan 4, Task 7: failed builds can execute with swapping disabled.** [`heldTarget` returns `held=false` when root reports the target failed](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2407), relying on the scheduler to select recovery or refuse. The consumed scheduler interface leaves `Service.Planner` unset when `CARLOS_SWAP` is off. If the failed target remains installed, `LiveIdentity` and `StartExe` consequently run it again. This violates **§4.3’s failure precedence and §6**. **Smallest fix:** preserve an explicit failed-target refusal independently of the scheduler; permit recovery only after failure-aware admission. Test a nil planner with the failed target still at the live link.

+3. **High — Plan 4, Task 5: the promised unit-backend bypass cannot work through `SplitBackend`.** [The task adds identity methods to `SplitBackend`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:1352), then selects the new wake path using `a.backend.(identityBackend)` at line 1408. Production constructs the service with a `SplitBackend` (`origin/main:internal/host/agent.go:382`), which dispatches unit routes through `pick` (`internal/activator/backend_unit.go:292`). The assertion therefore succeeds for unit-backed routes too, although their selected backend cannot supply the required identity. Their existing wake now fails or enters the wrong execution path. This breaks **§2 decision 4’s unchanged default behavior**. **Smallest fix:** select identity capability per route, returning no identity backend for unit-backed routes; test a production-shaped `SplitBackend` unit wake.

+4. **High — Plan 1, Task 10: `resign` bypasses the emission marker.** [The task explicitly exempts resign from the marker](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-1-pointer-digests.md:2420), and `Resign` signs digest fields without checking it. This contradicts **§4.6’s writers-before-readers rollout and §8’s marker test**. Resigning during the writer upgrade arms reader ratchets prematurely; subsequent digestless writes from remaining old writers are refused. Confirming particular digests does not establish that every writer has been upgraded. **Smallest fix:** require a verified emission marker at signing time and refuse resign while it is absent or invalid.

+5. **Medium — Plan 4, Task 7: retained-build recovery now unnecessarily requires a root repoint.** [`wakeCheck` waits exclusively for the live link to become the target](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:2346). It never uses an already staged target. A held instance with verified A staged and stale B installed therefore times out when root cannot complete its pass, although **§4.3 rule 3 explicitly requires running the retained staged copy in this case**. The I2 fix removed that behavior along with the unsafe shortcut. **Smallest fix:** submit the retained identity through failure-aware wake admission; use the stage relay only when that copy is unavailable.

+6. **Medium — Plan 4, Task 2: pins suppress request resolution and failure completion.** [`Target` returns from the pin branch before processing requests](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-4-root-applies.md:658). **§4.3 rule 2 requires unresolved requests to be resolved at the start of every pass, before the selection rules.** Request R can wait on bake, then coexist with a newly set pin: once H becomes eligible, the plan still leaves R unresolved. Clearing the pin after the head advances resolves R to a different build. The same early return suppresses recording a request target’s failure. **Smallest fix:** compute request resolution/completion before target selection and carry those updates through the pin decision.

+7. **Medium — Plan 1, Tasks 6 and 8: `SharedDigestRatchet` introduces a data race.** [Every repeated lookup writes `r.require` under `r.mu`](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-1-pointer-digests.md:2078), but `Admit` reads it without synchronization at line 1522. Concurrent materializations, adoption passes, and static reads now share that object. The write occurs even when the flag’s value remains unchanged. **Smallest fix:** synchronize the read with the same mutex or use an atomic flag; add a concurrent shared-lookup/admission race test.

+8. **Medium — Plan 5, Task 5: the browser prevents safe removal of settings during a mixed-version rollout.** [The form disables auto, major, and pin whenever `.Refusing` is true](/home/paulca/amadan.net/carlos/platform-design-zero-downtime-deploys/docs/superpowers/plans/2026-10-07-instance-settings-5-surfaces.md:1131). Disabled fields are omitted, while the handler interprets absent `auto` as `manual`. Members consequently cannot switch manual to auto or clear a pin/major hold when enforcement is unavailable. **§4.4 only gates enabling these settings**, and Plan 3 expressly permits disabling them. **Smallest fix:** keep disabling changes expressible and distinguish omitted controls from an explicit manual selection; test all three clearing operations with the fleet gate closed.

\ No newline at end of file