console: plans 3b round 3 fixes (no manifest read with plans off, guard failures are plan-check failures)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9 files changed,
+135
−11
docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md+1 −0docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md+7 −0docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R3.md+29 −0internal/console/apicli.go+6 −0internal/console/pendinghandlers.go+2 −6internal/console/plancaps.go+32 −4internal/console/plancaps_doors_test.go+51 −0internal/console/server.go+6 −0internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv+1 −1
CI cancelled — superseded by a newer push — run details
CI log
=== gate === .amadan/test-gate.sh amadan: job cancelled — a newer push superseded it
diff --git a/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md b/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md| index 7e346c492..41d3ff681 100644 |
| --- a/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| +++ b/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| @@ -22,3 +22,4 @@ One new read, so it needs a decision rather than an approval. It has its own ove |
| Also reachable now, with no new census site: CLI, browser, canary and pending promote read the target release's manifest (each already did) and, when it declares daemon, the account plan and the app's placement (Astra plans-3b finding 2). The record-write census gains `plans3bWriteCensusInventory`: `handleAPISidecarCreate` has a new refusal but writes nothing new (finding 3). |
| Round 2 (Astra) adds no census decision. `internal/provision` is re-pinned for `ListSidecarRecordsStrict`, a new key site in the record-write census (it lists sidecar records under `SidecarsPrefix`, uncapped, and fails on any it can't read). CLI and browser rollback now read the target release's manifest before writing, and again inside the rollback's authorize hook. Neither is a census unit. The earlier approval at 7d572577 covered the card before these changes. It needs approving again at the new tip. |
| +Round 3 adds no census change beyond the console envelope: with plans off, rollback and pending execution read no manifest, and a guard read failure is answered as a failed plan check. |
diff --git a/docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md b/docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md| index cd5707933..a87a581fd 100644 |
| --- a/docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md |
| +++ b/docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md |
| @@ -90,3 +90,10 @@ Findings in `docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md`. Round 1 |
| 1. **Sidecar listing.** `checkAlwaysOn` now uses `provision.ListSidecarRecordsStrict`. It has no cap, and it fails on any record it can't read, so a skipped worker can no longer earn the own-fleet exemption. A record that disappears between the list and the read is skipped. Covered by `TestAlwaysOnRefusedPastAnUnreadableSidecarRecord`. |
| 2. **Rollback is checked.** A rollback target need not be on any channel now, so CLI rollback, browser rollback and pending rollback all apply the daemon rule to it. The direct doors check the target they read. They also pass `rollbackDaemonGuard` as Rollback's authorize hook. That hook re-reads the pointer after Rollback's own read and before its conditional write, so the version actually written is the one checked. Fast track and retry stay unchecked: they keep the version the channel already serves. Covered by `TestRollbackToADaemonReleaseRefusedUnderAPlan`, `TestRollbackDaemonGuardReadsThePointerAtWriteTime` and `TestPendingRollbackToADaemonReleaseRefusedUnderAPlan`. |
| 3. **Exact cron gap.** `ShortestInterval` now walks a whole 400-year Gregorian cycle, which is a whole number of weeks, to find the closest pair of matching days. Weekly and monthly lines get their real gaps (`0,55 0,23 * * 1` is 55 minutes), and seasonal and leap-day bursts are still caught. Cron runs in UTC, so there is no daylight-saving shift. |
| + |
| +## Astra review, round 3 (2026-10-25) |
| + |
| +Findings in `docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R3.md`. All three round-2 findings are confirmed fixed. Two new findings, both fixed: |
| + |
| +1. **Important: with plans off, rollback read a manifest.** `checkDaemonVersion` now checks the plan and the placement before it reads anything. Pending execution goes through it too. So with plans off, neither rollback nor a pending change reads a manifest it didn't read before. Covered by `TestRollbackWithoutACatalogNeedsNoManifest` (mutation-checked). |
| +2. **Minor: guard read failures looked like deploy refusals.** A guard that can't read now returns `*planGuardError`. Both rollback doors answer it as a failed plan check: a 502 with `planCheckFailedCopy` on the API and the same text as a browser flash. Covered by `TestRollbackGuardReadFailureIsAPlanCheckFailure`. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R3.md b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R3.md| new file mode 100644 |
| index 000000000..a66f8ac29 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R3.md |
| @@ -0,0 +1,29 @@ |
| +| Round 2 finding | Disposition | Evidence at reviewed HEAD | |
| +|---|---|---| |
| +| Incomplete sidecar enumeration | **Fixed** | [sidecar.go:404](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/provision/sidecar.go:404) reads the complete listing and propagates record errors, skipping only deleted records. `TestAlwaysOnRefusedPastAnUnreadableSidecarRecord` covers unreadable data. | |
| +| Rollback restores prohibited daemon releases | **Fixed** | [plancaps.go:338](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/plancaps.go:338) adds the execution-time guard. Coverage: `TestRollbackToADaemonReleaseRefusedUnderAPlan`, `TestRollbackDaemonGuardReadsThePointerAtWriteTime`, and `TestPendingRollbackToADaemonReleaseRefusedUnderAPlan`. | |
| +| Compliant weekly crons refused | **Fixed** | [mingap.go:67](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/schedule/mingap.go:67) includes calendar restrictions and cycle wraparound. `TestCronMinGap` covers weekly, seasonal, leap-day and combined day-field cases. | |
| + |
| +Reviewed **`eb0c40ceb92f23bc5fc4a65d02e1688e68f00d45`**. Test names above identify inspected coverage, not successful executions. |
| + |
| +1. **Important — Rollback gains a manifest dependency even with plans off.** |
| + [plancaps.go:323](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/plancaps.go:323) |
| + |
| + `checkDaemonVersion` reads and validates the manifest before checking whether plans apply. Both direct rollback doors call it unconditionally. |
| + |
| + **Failure scenario:** With `PlanCatalog == nil`, the channel and history remain readable, but fetching the historical manifest fails. Previously, rollback could swap the signed pointer using its history; now the API returns 502 and the browser refuses. Returning a nil `rollbackDaemonGuard` does not help because this earlier check already failed. This violates “plans off changes nothing.” |
| + |
| + **Fix:** Short-circuit the plan check before fetching the manifest when no effective limits apply. Add plans-off rollback tests that fail manifest reads while allowing pointer operations. |
| + |
| +2. **Minor — Guard read failures are handled as deploy-authorization failures.** |
| + [apicli.go:1258](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/apicli.go:1258), [server.go:4884](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/server.go:4884) |
| + |
| + **Failure scenario:** The initial checks succeed, but the guard’s pointer or manifest reread fails. `RollbackAuthorized` wraps that failure in `AuthorizationError`. The API forwards it to `writeDeployRefusal`, which returns an unlogged generic 500 because it is not a deploy refusal. The browser instead exposes the underlying error through its generic rollback flash. Neither uses the required plan-check failure handling. |
| + |
| + **Fix:** Distinguish guard failures from deploy-scope refusals. Preserve cap responses and genuine deploy 403s; log other guard failures and use `planCheckFailedCopy` with API status 502. |
| + |
| +No additional finding emerged from listing pagination/deletion semantics, deploy-scope composition, cron month/OR rules, or bounded calendar cost. Strict listing costs one complete listing plus one read per record. An independent calendar calculation matched all 17 cron expectations. Census digests, envelope hashes and seals match; the card correctly states that renewed approval is needed. |
| + |
| +**Declined to judge:** approved wording and external approval provenance. Targeted Go tests could not start because the sandbox denied temporary build-directory creation. HEAD and the clean working tree remained unchanged. |
| + |
| +**Verdict: Changes required — one Important and one Minor finding.** |
| \ No newline at end of file |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 4f3baa918..77c6e47dc 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -1255,6 +1255,12 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| return |
| } |
| + var ge *planGuardError |
| + if errors.As(err, &ge) { |
| + log.Printf("console: api rollback %s/%s: plan: %v", acct.Sqid, app, ge.err) |
| + jsonError(w, http.StatusBadGateway, planCheckFailedCopy) |
| + return |
| + } |
| var ae *release.AuthorizationError |
| if errors.As(err, &ae) { |
| s.writeDeployRefusal(w, ae.Err) |
diff --git a/internal/console/pendinghandlers.go b/internal/console/pendinghandlers.go| index a8b61bd1b..c28257532 100644 |
| --- a/internal/console/pendinghandlers.go |
| +++ b/internal/console/pendinghandlers.go |
| @@ -546,13 +546,9 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| // execution, not at propose time (Astra plans-3b finding 2). Rollbacks |
| // too: the target need be on no channel now (round 2 finding 2). A retry |
| // only renews the version the channel already serves. |
| + // With plans off this reads nothing (checkDaemonVersion). |
| if !p.Retry { |
| - man, merr := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, p.Version) |
| - if merr != nil { |
| - log.Printf("console: execute promote %s/%s: read manifest %s: %v", acct.Sqid, app, p.Version, merr) |
| - return "", errors.New("could not read release " + p.Version) |
| - } |
| - if cerr := s.checkDaemonRelease(acct, app, man); cerr != nil { |
| + if cerr := s.checkDaemonVersion(ctx, acct, app, p.Version); cerr != nil { |
| var planCap *planCapError |
| if errors.As(cerr, &planCap) { |
| return "", errors.New(planCap.Browser()) |
diff --git a/internal/console/plancaps.go b/internal/console/plancaps.go| index 539762867..996e381e7 100644 |
| --- a/internal/console/plancaps.go |
| +++ b/internal/console/plancaps.go |
| @@ -319,15 +319,36 @@ func (s *Server) checkDaemonRelease(acct *Account, app string, m *release.Manife |
| // checkDaemonVersion is checkDaemonRelease for a version named by its |
| // channel history, as a rollback target is (Astra plans-3b round 2 |
| -// finding 2). |
| +// finding 2). It reads the manifest only when a plan applies to an app on |
| +// shared boxes, so with plans off a rollback depends on nothing it did not |
| +// before (round 3 finding 1). |
| func (s *Server) checkDaemonVersion(ctx context.Context, acct *Account, app, version string) error { |
| + l, err := s.EffectiveLimits(acct) |
| + if err != nil || l == nil { |
| + return err |
| + } |
| + if own, err := s.onOwnBoxes(acct, app); err != nil || own { |
| + return err |
| + } |
| m, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version) |
| if err != nil { |
| return err |
| } |
| - return s.checkDaemonRelease(acct, app, m) |
| + if m.Capabilities == nil || !m.Capabilities.Daemon { |
| + return nil |
| + } |
| + return &planCapError{msg: capMessage("cli.cap.daemon")} |
| } |
| +// planGuardError is a rollback guard that could not read what it checks. |
| +// Rollback wraps every hook failure in an AuthorizationError, so the doors |
| +// tell this apart from a deploy-scope refusal and answer it as a failed |
| +// plan check (round 3 finding 2). |
| +type planGuardError struct{ err error } |
| + |
| +func (e *planGuardError) Error() string { return e.err.Error() } |
| +func (e *planGuardError) Unwrap() error { return e.err } |
| + |
| // rollbackDaemonGuard is the authorize hook a rollback door passes so the |
| // daemon rule holds for the version the rollback actually writes. The door |
| // checked the target it read, but the pointer can move before the write. |
| @@ -346,12 +367,19 @@ func (s *Server) rollbackDaemonGuard(ctx context.Context, acct *Account, app, ch |
| return func(*release.Pipeline) error { |
| cur, err := release.Channel(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, channel) |
| if err != nil { |
| - return err |
| + return &planGuardError{err} |
| } |
| if len(cur.History) == 0 { |
| return nil // Rollback refuses this itself |
| } |
| - return s.checkDaemonVersion(ctx, acct, app, cur.History[0]) |
| + m, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, cur.History[0]) |
| + if err != nil { |
| + return &planGuardError{err} |
| + } |
| + if m.Capabilities == nil || !m.Capabilities.Daemon { |
| + return nil |
| + } |
| + return &planCapError{msg: capMessage("cli.cap.daemon")} |
| }, nil |
| } |
diff --git a/internal/console/plancaps_doors_test.go b/internal/console/plancaps_doors_test.go| index 5f5da5d85..b8238cd82 100644 |
| --- a/internal/console/plancaps_doors_test.go |
| +++ b/internal/console/plancaps_doors_test.go |
| @@ -2,6 +2,7 @@ package console |
| import ( |
| "context" |
| + "errors" |
| "fmt" |
| "net/http" |
| "net/url" |
| @@ -855,3 +856,53 @@ func TestPendingRollbackToADaemonReleaseRefusedUnderAPlan(t *testing.T) { |
| t.Fatalf("production serves %q after confirming a daemon rollback under a plan, want v2", got.Version) |
| } |
| } |
| + |
| +// Astra plans-3b round 3 finding 1: with plans off a rollback reads no |
| +// manifest, so one that cannot be read does not stop it. |
| +func TestRollbackWithoutACatalogNeedsNoManifest(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + shipVersion(t, fx, "kass", "old1") |
| + shipVersion(t, fx, "kass", "new2") |
| + seedChannel(t, fx, "kass", "canary/x", "old1") |
| + seedChannel(t, fx, "kass", "canary/x", "new2") |
| + if err := fx.rel.Delete(context.Background(), release.ManifestKey(release.OpsAccount, "kass", "old1")); err != nil { |
| + t.Fatal(err) |
| + } |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-rollback-nomanifest") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/kass/rollback", map[string]string{"channel": "canary/x"}, token); status != http.StatusOK { |
| + t.Fatalf("CLI rollback with plans off and the target's manifest unreadable = %d %v, want 200", status, out) |
| + } |
| + c := signInPaul(t, fx) |
| + postForm(t, c, fx.u("/apps/kass/rollback"), url.Values{"csrf": {sessionToken(t, fx, c)}, "channel": {"canary/x"}, "confirmed": {"1"}}) |
| + if p := fx.channelVersion(t, "kass", "canary/x"); p.Version != "new2" { |
| + t.Fatalf("canary/x = %s after a second rollback with plans off, want new2", p.Version) |
| + } |
| +} |
| + |
| +// Round 3 finding 2: a guard that cannot read is a failed plan check (502), |
| +// not a deploy refusal. |
| +func TestRollbackGuardReadFailureIsAPlanCheckFailure(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + shipVersion(t, fx, "kass", "old1") |
| + shipVersion(t, fx, "kass", "new2") |
| + seedChannel(t, fx, "kass", "canary/x", "old1") |
| + seedChannel(t, fx, "kass", "canary/x", "new2") |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + if err := fx.rel.Delete(context.Background(), release.ManifestKey(release.OpsAccount, "kass", "old1")); err != nil { |
| + t.Fatal(err) |
| + } |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-rollback-guardfail") |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/kass/rollback", map[string]string{"channel": "canary/x"}, token) |
| + if status != http.StatusBadGateway || out["error"] != planCheckFailedCopy { |
| + t.Fatalf("CLI rollback with an unreadable target manifest under a plan = %d %v, want 502 %q", status, out, planCheckFailedCopy) |
| + } |
| + acct, _ := fx.st.AccountBySqid(fx.sqid) |
| + guard, err := fx.srv.rollbackDaemonGuard(context.Background(), acct, "kass", "canary/x") |
| + if err != nil || guard == nil { |
| + t.Fatalf("guard = %v, %v", guard != nil, err) |
| + } |
| + var ge *planGuardError |
| + if gerr := guard(nil); !errors.As(gerr, &ge) { |
| + t.Fatalf("guard read failure = %v, want a *planGuardError", gerr) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index 2c3ddf026..49a1022e3 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -4887,6 +4887,12 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| back("err", planCap.Browser()) |
| return |
| } |
| + var ge *planGuardError |
| + if errors.As(err, &ge) { |
| + log.Printf("console: rollback %s/%s: plan: %v", acct.Sqid, app, ge.err) |
| + back("err", planCheckFailedCopy) |
| + return |
| + } |
| log.Printf("console: rollback %s %s: %v", app, channel, err) |
| back("err", "Rollback failed: "+err.Error()) |
| return |
diff --git a/internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv b/internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv| index 2104ec1f2..35b5934e9 100644 |
| --- a/internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv |
| +++ b/internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv |
| @@ -2,6 +2,6 @@ |
| # 1 body, 0 sites, 3 envelope: each changed unit re-frozen at its existing disposition with its claim appended. |
| @legacy 8d3bb7b6f3218fb051765cc3d958f58c32f1b2f43735b891f723d05d2f7e26ad internal/console/apicli.go Server.handleAPIShip legacy accepted-legacy accepted-legacy;rev3=J4;basis=root-input;rev3=census-ruling-plans-3b-never-sleep;basis=after the capability block is validated, a release declaring daemon is refused (409 cap) for an app on shared boxes under a plan; an app on a fleet the account owns, or any app with plans off, ships as before. No new direct read at this site; the plan check it calls reads the account plan, the app placement and (through ownsFleet) the owner of the fleet named. file:ReadAll@11378 c431049c96beaa5858c1aaf58e23fa2f32c40c2319297cac2b42527a3b879359 |
| internal/console/apicli.go Server.handleAPIShip legacy accepted-legacy accepted-legacy;rev3=J4;basis=root-input;rev3=census-ruling-plans-3b-never-sleep;basis=after the capability block is validated, a release declaring daemon is refused (409 cap) for an app on shared boxes under a plan; an app on a fleet the account owns, or any app with plans off, ships as before. No new direct read at this site; the plan check it calls reads the account plan, the app placement and (through ownsFleet) the owner of the fleet named. file:ReadAll@11378 c431049c96beaa5858c1aaf58e23fa2f32c40c2319297cac2b42527a3b879359 |
| -@coverage internal/console fa8657e7d115ac2970a4ce43dba4fa92b10aaa156094091f736ed762b0743173 |
| +@coverage internal/console 9dca8ef71869e2f8700f06d96f0f74049f3bcc569a171b5e8e6aef8e1f39fc7d |
| @coverage internal/provision 83a571025e988adf3dd9ffa4afcc4ac0d1cbdc9d41f4555886863ab577ed0212 |
| @coverage internal/schedule f749883df03257d26076da5bb9add4243e453c488688e7ce4324d1c1df8b221f |