console: plans 3b round 2 fixes (strict sidecar listing, rollback checked, exact cron gap)
Astra round 2 (CODEX-FINDINGS-PLANS-3B-CODE-R2.md): always-on on an own fleet needs every sidecar record read (ListSidecarRecordsStrict); CLI, browser and pending rollbacks hold their target to the daemon rule, with an authorize-hook guard for the version actually written; the cron gap is exact over the 400-year Gregorian cycle, so weekly lines are not refused. Census re-proposed; B8 key site for the strict lister.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
13 files changed,
+368
−32
docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md+3 −4docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md+9 −1docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md+41 −0internal/console/apicli.go+20 −1internal/console/pendinghandlers.go+4 −4internal/console/plancaps.go+55 −1internal/console/plancaps_doors_test.go+111 −0internal/console/server.go+22 −1internal/provision/plans3b_census_test.go+4 −0internal/provision/sidecar.go+34 −0internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv+4 −3internal/schedule/mingap.go+50 −13internal/schedule/mingap_test.go+11 −4
CI cancelled — superseded by a newer push — run details
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 129d07384..7e346c492 100644 |
| --- a/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| +++ b/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| @@ -1,19 +1,16 @@ |
| # Census approval: plans-3b-never-sleep |
| -Status: APPROVED by Paul (census owner), 2026-10-25, in session, at tip 7d572577 (CI passed): the approval below and the hand-written decision (census-ruling-plans-3b-fleet-owner). Recorded on #280. Supersedes the 2026-10-25 approval of the earlier card at 9f19e13e. |
| - |
| Ruling `census-ruling-plans-3b-never-sleep`. Every change below keeps the disposition it already has, so it needs an approval, not a decision. |
| | unit | change | read sites | what changed | |
| |---|---|---|---| |
| | `internal/console/apicli.go` `Server.handleAPIShip` | body | unchanged (1) | 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. | |
| -Envelopes re-pinned: `internal/console`, `internal/schedule`. |
| +Envelopes re-pinned: `internal/console`, `internal/provision`, `internal/schedule`. |
| Seal: `74ff021155e1f12ea81cac07f183b528bf5748d6fc06ae7903ed0f91842458f2` → `d62eabd30c03e73ec1348d8e64912678ff8523ac07252c9283ef9f445ca4e944`. |
| Overlay: `internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv`, appended to `internal/provision/testdata/plan14-root-reads-chain.tsv`. |
| - |
| ## Decision (hand-written, census-ruling-plans-3b-fleet-owner) |
| One new read, so it needs a decision rather than an approval. It has its own overlay, `internal/provision/testdata/plan14-root-reads-plans-3b-fleet-owner.tsv`, chained just before this one (seal `86570d13…` → `74ff0211…`). |
| @@ -23,3 +20,5 @@ One new read, so it needs a decision rather than an approval. It has its own ove |
| | `internal/console/plancaps.go` `ownsFleet` | new | `registry:Account`, `whole:Fleet` (one `GetFleet`) | reads who owns the fleet an app, or one of its sidecar records, is placed on. The plan's daemon, always-on and schedule refusals are lifted only when that owner is this account. An unknown or foreign fleet keeps the refusal, and a registry that can't be read is an error, not a pass. Nothing is granted on any box. Classed `legacy`/`accepted-legacy`, like every other console registry read (Astra plans-3b finding 5). | |
| 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. |
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 ee742ec89..cd5707933 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 |
| @@ -80,5 +80,13 @@ Findings in `docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md`. All six we |
| Rulings taken in the fix pass: |
| -- **Rollback and fast track are not checked.** They only re-serve a version that is already on one of this app's channels. Every way onto a channel is now checked: ship, promote, canary and pending promote. So a daemon version can only be there if it got there before plans were on, or while the app was on its fleet. Those leftovers belong to Plan 4, the same as an always-on app that moves from its fleet back to shared boxes. Pending rollbacks and retries are exempt for the same reason. The box's converge step still refuses a daemon whose hibernation is on. |
| +- ~~**Rollback and fast track are not checked.**~~ Rollback reversed in round 2 (below). Fast track and retry still stand. They only re-serve a version that is already on one of this app's channels. Every way onto a channel is now checked: ship, promote, canary and pending promote. So a daemon version can only be there if it got there before plans were on, or while the app was on its fleet. Those leftovers belong to Plan 4, the same as an always-on app that moves from its fleet back to shared boxes. Pending rollbacks and retries are exempt for the same reason. The box's converge step still refuses a daemon whose hibernation is on. |
| - **No new copy.** The promote doors reuse the approved `cli.cap.daemon` string ("…or ship it without daemon" still applies). The browser doors show it through `Browser()`, which falls back to the CLI text the same way `cli.cap.sidecars` does. A failed plan check shows `planCheckFailedCopy`. |
| + |
| +## Astra review, round 2 (2026-10-25) |
| + |
| +Findings in `docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md`. Round 1's findings 1, 3, 5 and 6 are confirmed fixed. Three new Important findings, all fixed: |
| + |
| +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. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md| new file mode 100644 |
| index 000000000..2ab705096 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE-R2.md |
| @@ -0,0 +1,41 @@ |
| +| Round 1 finding | Disposition | Evidence at reviewed HEAD | |
| +|---|---|---| |
| +| 1. Seasonal crons bypass the minimum | **Fixed** | [mingap.go:34](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/schedule/mingap.go:34) examines every permitted hour/minute combination. `TestCronMinGap` includes seasonal, leap-day and combined day-field cases. A new false-refusal issue remains below. | |
| +| 2. Promotion bypasses daemon refusal | **Partly fixed** | [apicli.go:851](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/apicli.go:851) and the browser, canary and pending execution paths now check. Covered by `TestPromoteRefusesADaemonReleaseUnderAPlan`, `TestBrowserPromoteAndCanaryRefuseADaemonReleaseUnderAPlan` and `TestPendingPromoteOfADaemonReleaseRefusedUnderAPlan`. Rollback remains unchecked. | |
| +| 3. Sidecar creation permits new always-on workers | **Fixed** | [apisidecars.go:332](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/apisidecars.go:332) checks an existing `hibernate:false` manifest before either write path. `TestSidecarCreateRefusedUnderAnAlwaysOnManifest` covers refusal. | |
| +| 4. App placement does not prove sidecars moved | **Partly fixed** | [plancaps.go:350](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/plancaps.go:350) checks individual records. `TestAlwaysOnRefusedWhileSidecarsRemainOnSharedBoxes` covers an ordinary shared worker, but enumeration can silently omit workers. | |
| +| 5. Fleet syntax substitutes for ownership | **Fixed** | [plancaps.go:426](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/plancaps.go:426) requires an existing fleet with matching account and propagates lookup errors. `TestOwnBoxesNeedsAFleetTheAccountOwns` covers owned, foreign, missing and unreadable cases. | |
| +| 6. Census card omits indirect reads | **Fixed** | [approval card:9](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md:9) names indirect reads; its decision section identifies `ownsFleet`. Relevant gates are `TestPlan14RootReadCensus` and `TestPlan9B8RecordWriteCensus`. | |
| + |
| +Reviewed `f808aa12b92d522a82ae3deabb4944103886bbed`. Test names above identify coverage inspected; Go tests could not start in this sandbox. |
| + |
| +1. **Important — Incomplete sidecar enumeration grants the always-on exception.** |
| + [plancaps.go:350](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/plancaps.go:350) discards `truncated`, and [`ListSidecarRecords`:389](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/provision/sidecar.go:389) silently skips individual read failures. |
| + |
| + **Failure scenario:** An app is placed on its owned fleet while a hibernating sidecar remains on shared boxes. Listing succeeds, but reading that sidecar temporarily fails. The helper returns no records and no error, so enabling `hibernate:false` succeeds. When the record becomes readable again, shared-box reconciliation applies the always-on manifest. Likewise, a shared worker sorted beyond the first 500 records is never checked. |
| + |
| + **Fix:** Require complete enumeration and successful reads before granting the exception. Refuse on truncation or unresolved record-read failures. Add tests for both conditions. |
| + |
| +2. **Important — The rollback exemption can restore a daemon that no channel currently serves.** |
| + [pendinghandlers.go:549](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/console/pendinghandlers.go:549) explicitly bypasses the check for rollback. Direct API and browser rollback also omit it. |
| + |
| + **Failure scenario:** Before plans are enabled, promote daemon release D, then ordinary release N onto the same channel. Enable plans on shared boxes. A member can now roll back N to D, directly or through an approved proposal. D need exist only in history; it need not remain on any current channel. |
| + |
| + This changes a compliant channel to a prohibited release. It therefore exceeds preserving existing running leftovers. Convergence refusing D does not satisfy §5.2’s required refusal before changing the channel. |
| + |
| + **Fix:** Check the actual rollback target against current limits and placement at execution, including approved rollbacks. Fast-track and retry preserve the current version; that rationale does not cover rollback. |
| + |
| +3. **Important — The cron fix rejects schedules that meet the minimum.** |
| + [mingap.go:49](/home/paulca/.cache/tmp/claude-1001/plans-3b-never-sleep/internal/schedule/mingap.go:49) always includes a next-day wraparound gap, even when consecutive matching days are impossible. |
| + |
| + **Failure scenario:** `0,55 0,23 * * 1` fires Mondays at 00:00, 00:55, 23:00 and 23:55. Its actual shortest interval is **55 minutes**, but the implementation returns **5 minutes**, imagining a Tuesday 00:00 firing. Free accounts therefore receive a cap refusal for a compliant schedule. |
| + |
| + The bound is conservative and closes the seasonal bypass, but violates the requirement to allow schedules meeting the minimum. |
| + |
| + **Fix:** Account for calendar restrictions when evaluating cross-day gaps, while retaining complete seasonal and leap-day coverage. Add this weekly case as an acceptance test. |
| + |
| +I independently recomputed both new census seals, frozen fingerprints, touched function digests and package envelope hashes; they match. The record-write inventory correctly retains the existing writer classification. |
| + |
| +**Declined to judge:** approved wording, excluded operator paths and external approval provenance. Targeted Go tests failed before execution because temporary build-directory creation was denied. HEAD and the clean working tree remained unchanged. |
| + |
| +**Verdict: Changes required — three Important findings.** |
| \ No newline at end of file |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 37bfbad50..4f3baa918 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -1184,6 +1184,15 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| target := cur.History[0] |
| + if err := s.checkDaemonVersion(ctx, acct, app, target); err != nil { |
| + if isPlanCap(err) { |
| + writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| + return |
| + } |
| + log.Printf("console: api rollback %s/%s: plan: %v", acct.Sqid, app, err) |
| + jsonError(w, http.StatusBadGateway, planCheckFailedCopy) |
| + return |
| + } |
| // Rollback moves the same pointer promote moves, so it wears the same |
| // service refusal — a credential barred from promoting the control plane |
| // must not reach the identical effect through the other verb. |
| @@ -1235,7 +1244,17 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| signer = sg |
| authorize = deployScopeAuthorize(p, channel) |
| } |
| - if err := signer.RollbackAuthorized(ctx, s.cfg.Releases, acct.Sqid, app, channel, authorize); err != nil { |
| + guard, gerr := s.rollbackDaemonGuard(ctx, acct, app, channel) |
| + if gerr != nil { |
| + log.Printf("console: api rollback %s/%s: plan: %v", acct.Sqid, app, gerr) |
| + jsonError(w, http.StatusBadGateway, planCheckFailedCopy) |
| + return |
| + } |
| + if err := signer.RollbackAuthorized(ctx, s.cfg.Releases, acct.Sqid, app, channel, bothAuthorize(authorize, guard)); err != nil { |
| + if isPlanCap(err) { |
| + writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| + 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 3df10349f..a8b61bd1b 100644 |
| --- a/internal/console/pendinghandlers.go |
| +++ b/internal/console/pendinghandlers.go |
| @@ -543,10 +543,10 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| } |
| } |
| // A daemon release waiting in a proposal meets the plan as it stands at |
| - // execution, not at propose time (Astra plans-3b finding 2). A rollback |
| - // or retry only re-serves a version this app already served, the same |
| - // line the direct rollback door takes. |
| - if !p.Rollback && !p.Retry { |
| + // 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. |
| + 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) |
diff --git a/internal/console/plancaps.go b/internal/console/plancaps.go| index 7545b156f..539762867 100644 |
| --- a/internal/console/plancaps.go |
| +++ b/internal/console/plancaps.go |
| @@ -317,6 +317,60 @@ func (s *Server) checkDaemonRelease(acct *Account, app string, m *release.Manife |
| return s.checkDaemon(acct, app) |
| } |
| +// checkDaemonVersion is checkDaemonRelease for a version named by its |
| +// channel history, as a rollback target is (Astra plans-3b round 2 |
| +// finding 2). |
| +func (s *Server) checkDaemonVersion(ctx context.Context, acct *Account, app, version string) error { |
| + m, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version) |
| + if err != nil { |
| + return err |
| + } |
| + return s.checkDaemonRelease(acct, app, m) |
| +} |
| + |
| +// 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. |
| +// The hook runs after Rollback's own pointer read and before its |
| +// conditional write, so a pointer that moved in between fails that write, |
| +// and one that did not has the History[0] this re-read sees. nil when plans |
| +// are off or the app is on its own fleet. |
| +func (s *Server) rollbackDaemonGuard(ctx context.Context, acct *Account, app, channel string) (func(*release.Pipeline) error, error) { |
| + l, err := s.EffectiveLimits(acct) |
| + if err != nil || l == nil { |
| + return nil, err |
| + } |
| + if own, err := s.onOwnBoxes(acct, app); err != nil || own { |
| + return nil, err |
| + } |
| + 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 |
| + } |
| + if len(cur.History) == 0 { |
| + return nil // Rollback refuses this itself |
| + } |
| + return s.checkDaemonVersion(ctx, acct, app, cur.History[0]) |
| + }, nil |
| +} |
| + |
| +// bothAuthorize runs a then b, either of which may be nil. |
| +func bothAuthorize(a, b func(*release.Pipeline) error) func(*release.Pipeline) error { |
| + switch { |
| + case a == nil: |
| + return b |
| + case b == nil: |
| + return a |
| + } |
| + return func(p *release.Pipeline) error { |
| + if err := a(p); err != nil { |
| + return err |
| + } |
| + return b(p) |
| + } |
| +} |
| + |
| // checkAlwaysOnCreate refuses a new sidecar that would never sleep (its |
| // app's manifest says hibernate:false) on shared boxes (plans 3b; Astra |
| // plans-3b finding 3). A new sidecar of an app on its own fleet lands there. |
| @@ -347,7 +401,7 @@ func (s *Server) checkAlwaysOn(ctx context.Context, acct *Account, app string) e |
| if l == nil { |
| return nil |
| } |
| - recs, _, err := provision.ListSidecarRecords(ctx, s.cfg.Releases, acct.Sqid, app, sidecarListMax) |
| + recs, err := provision.ListSidecarRecordsStrict(ctx, s.cfg.Releases, acct.Sqid, app) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/console/plancaps_doors_test.go b/internal/console/plancaps_doors_test.go| index 00c6d182f..5f5da5d85 100644 |
| --- a/internal/console/plancaps_doors_test.go |
| +++ b/internal/console/plancaps_doors_test.go |
| @@ -744,3 +744,114 @@ func TestPendingPromoteOfADaemonReleaseRefusedUnderAPlan(t *testing.T) { |
| t.Fatalf("production serves %q after confirming a daemon proposal under a plan, want v2", got.Version) |
| } |
| } |
| + |
| +// Astra plans-3b round 2 finding 1: a sidecar record that cannot be read is |
| +// a worker nobody checked, so always-on is not granted past it. |
| +func TestAlwaysOnRefusedPastAnUnreadableSidecarRecord(t *testing.T) { |
| + fx, token := instanceFixture(t, "code-cap-alwayson-unreadable") |
| + enableSidecars(t, fx, "titogo", titoSidecarManifest()) |
| + if _, err := fx.rel.Put(context.Background(), provision.SidecarRecordKey(release.OpsAccount, "titogo", "bot"), []byte("{not json"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + placeOnFleet(t, fx, "titogo") |
| + status, _, out := rawReq(t, http.MethodPut, sidecarEnableURL(fx, "bab", "titogo"), token, `{"hibernate":false}`) |
| + if status == http.StatusOK || status == http.StatusNoContent { |
| + t.Fatalf("hibernate:false past an unreadable sidecar record = %d %s, want a refusal", status, out) |
| + } |
| + m, err := provision.GetSidecarManifest(context.Background(), fx.rel, release.OpsAccount, "titogo") |
| + if err != nil || m == nil || !m.HibernateOrDefault() { |
| + t.Fatalf("manifest after the refusal = %+v, %v, want hibernation still on", m, err) |
| + } |
| +} |
| + |
| +// daemonInHistory leaves canary/x serving an ordinary release with a daemon |
| +// release one rollback back, the state promotions made before plans were on |
| +// leave. |
| +func daemonInHistory(t *testing.T, fx *consoleFixture, app string) { |
| + t.Helper() |
| + shipDaemonVersion(t, fx, app, "dmn3") |
| + shipVersion(t, fx, app, "plain4") |
| + seedChannel(t, fx, app, "canary/x", "dmn3") |
| + seedChannel(t, fx, app, "canary/x", "plain4") |
| +} |
| + |
| +// Astra plans-3b round 2 finding 2: a rollback can put back a daemon |
| +// release that no channel serves any more, so it meets the rule too. |
| +func TestRollbackToADaemonReleaseRefusedUnderAPlan(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + daemonInHistory(t, fx, "kass") |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-cap-daemon-rollback") |
| + |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/kass/rollback", map[string]string{"channel": "canary/x"}, token) |
| + if status != http.StatusConflict || out["code"] != "cap" { |
| + t.Fatalf("CLI rollback onto a daemon release under a plan = %d %v, want 409 cap", 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 != "plain4" { |
| + t.Fatalf("canary/x = %s after rollbacks refused under a plan, want plain4", p.Version) |
| + } |
| + |
| + placeOnFleet(t, fx, "kass") |
| + 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 onto a daemon release on its own fleet = %d %v, want 200", status, out) |
| + } |
| +} |
| + |
| +// The guard re-reads the pointer at write time, so a target that changed |
| +// after the door's check is still held to the rule. |
| +func TestRollbackDaemonGuardReadsThePointerAtWriteTime(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + shipVersion(t, fx, "kass", "plain2") |
| + seedChannel(t, fx, "kass", "canary/x", "plain2") |
| + acct, _ := fx.st.AccountBySqid(fx.sqid) |
| + ctx := context.Background() |
| + |
| + if guard, err := fx.srv.rollbackDaemonGuard(ctx, acct, "kass", "canary/x"); err != nil || guard != nil { |
| + t.Fatalf("guard with plans off = %v, %v, want none", guard != nil, err) |
| + } |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + guard, err := fx.srv.rollbackDaemonGuard(ctx, acct, "kass", "canary/x") |
| + if err != nil || guard == nil { |
| + t.Fatalf("guard under a plan = %v, %v, want one", guard != nil, err) |
| + } |
| + daemonInHistory(t, fx, "kass") // the pointer moves after the door's check |
| + if err := guard(nil); !isPlanCap(err) { |
| + t.Fatalf("guard after the pointer moved onto a daemon target = %v, want a cap", err) |
| + } |
| + placeOnFleet(t, fx, "kass") |
| + if guard, err := fx.srv.rollbackDaemonGuard(ctx, acct, "kass", "canary/x"); err != nil || guard != nil { |
| + t.Fatalf("guard on its own fleet = %v, %v, want none", guard != nil, err) |
| + } |
| +} |
| + |
| +// A pending rollback onto a daemon release is checked at execution too. |
| +func TestPendingRollbackToADaemonReleaseRefusedUnderAPlan(t *testing.T) { |
| + fx, c, csrf := memberFixture(t) |
| + pipelinedApp(t, fx, "widget", "v2", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}}) |
| + shipDaemonVersion(t, fx, "widget", "dmn5") |
| + seedChannel(t, fx, "widget", "production", "dmn5") |
| + seedChannel(t, fx, "widget", "production", "v2") |
| + |
| + if status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/rollback"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "confirmed": {"1"}}); status != http.StatusSeeOther { |
| + t.Fatalf("rollback proposal = %d -> %q:\n%s", status, loc, body) |
| + } |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil || len(pending) != 1 { |
| + t.Fatalf("pending = %v, %v, want exactly one", pending, err) |
| + } |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + kim := newBrowser(t) |
| + fx.setUser("kim@keymail.dev") |
| + signUp(t, fx, kim, "code-kim-daemon-rollback") |
| + postGated(t, fx, kim, fx.u("/apps/widget/pending/"+pending[0].ID+"/confirm"), |
| + url.Values{"csrf": {sessionToken(t, fx, kim)}}) |
| + if got := fx.channelVersion(t, "widget", "production"); got.Version != "v2" { |
| + t.Fatalf("production serves %q after confirming a daemon rollback under a plan, want v2", got.Version) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index b2e168c3b..2c3ddf026 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -4809,6 +4809,16 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| target := cur.History[0] |
| + if err := s.checkDaemonVersion(ctx, acct, app, target); err != nil { |
| + var planCap *planCapError |
| + if errors.As(err, &planCap) { |
| + back("err", planCap.Browser()) |
| + return |
| + } |
| + log.Printf("console: rollback %s/%s: plan: %v", acct.Sqid, app, err) |
| + back("err", planCheckFailedCopy) |
| + return |
| + } |
| if !confirmed(r) { |
| s.confirmRollback(w, r, addr, raw, acct, role, app, channel, cur.Version, target) |
| return |
| @@ -4865,7 +4875,18 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| } |
| } |
| } |
| - if err := release.Rollback(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, app, channel); err != nil { |
| + guard, gerr := s.rollbackDaemonGuard(ctx, acct, app, channel) |
| + if gerr != nil { |
| + log.Printf("console: rollback %s/%s: plan: %v", acct.Sqid, app, gerr) |
| + back("err", planCheckFailedCopy) |
| + return |
| + } |
| + if err := release.RootSigner(s.cfg.Key).RollbackAuthorized(ctx, s.cfg.Releases, acct.Sqid, app, channel, guard); err != nil { |
| + var planCap *planCapError |
| + if errors.As(err, &planCap) { |
| + back("err", planCap.Browser()) |
| + return |
| + } |
| log.Printf("console: rollback %s %s: %v", app, channel, err) |
| back("err", "Rollback failed: "+err.Error()) |
| return |
diff --git a/internal/provision/plans3b_census_test.go b/internal/provision/plans3b_census_test.go| index 4f5989dcd..6f37bcc4e 100644 |
| --- a/internal/provision/plans3b_census_test.go |
| +++ b/internal/provision/plans3b_census_test.go |
| @@ -7,10 +7,14 @@ package provision |
| // a new sidecar on shared boxes (Astra plans-3b finding 3). Its write is |
| // unchanged: the same CreateSidecarRecord create-only write, or the |
| // host-claim create. No write site is added; the refusal writes nothing. |
| +// ListSidecarRecordsStrict is a new key site, not a writer: it lists the |
| +// app's sidecar records under SidecarsPrefix, as ListSidecarRecords does, |
| +// with no cap and failing on any unreadable record (round 2 finding 1). |
| func plans3bWriteCensusInventory() []censusEntry { |
| entries := append([]censusEntry(nil), wakeLatency3WriteCensusInventory()...) |
| replacements := []censusEntry{ |
| {"internal/console/apisidecars.go", "Server.handleAPISidecarCreate", "record-writer", "key:CreateSidecarRecord", "77b9512efbfee3839265e83e671c00dbd97921460b115ef936eed810fcc409a7"}, |
| + {"internal/provision/sidecar.go", "ListSidecarRecordsStrict", "key-site", "key:SidecarsPrefix", "6f4990138a11b6c45ffce4cb44aa944403b609c672bc74b28a2188ecffabbe46"}, |
| } |
| for _, r := range replacements { |
| found := false |
diff --git a/internal/provision/sidecar.go b/internal/provision/sidecar.go| index aa77c8a2c..6bdbcf8c2 100644 |
| --- a/internal/provision/sidecar.go |
| +++ b/internal/provision/sidecar.go |
| @@ -395,6 +395,40 @@ func ListSidecarRecords(ctx context.Context, s release.Store, acct, app string, |
| return recs, truncated, nil |
| } |
| +// ListSidecarRecordsStrict lists every one of an app's sidecar records, |
| +// with no cap, and fails on any record it cannot read or parse. A record |
| +// deleted between the list and its read is skipped. It is for decisions |
| +// that must see every worker: plans 3b grants always-on on an app's own |
| +// fleet only once each sidecar is there, and a skipped record would be a |
| +// worker nobody checked (Astra plans-3b round 2 finding 1). |
| +func ListSidecarRecordsStrict(ctx context.Context, s release.Store, acct, app string) ([]SidecarRecord, error) { |
| + prefix := SidecarsPrefix(acct, app) |
| + keys, err := s.List(ctx, prefix) |
| + if err != nil { |
| + return nil, err |
| + } |
| + var recs []SidecarRecord |
| + for _, key := range keys { |
| + rest, ok := strings.CutPrefix(key, prefix) |
| + if !ok || rest == "" || strings.Contains(rest, "/") { |
| + continue |
| + } |
| + name, ok := strings.CutSuffix(rest, ".json") |
| + if !ok || name == "" { |
| + continue |
| + } |
| + rec, err := GetSidecarRecord(ctx, s, acct, app, name) |
| + if errors.Is(err, os.ErrNotExist) { |
| + continue |
| + } |
| + if err != nil { |
| + return nil, err |
| + } |
| + recs = append(recs, *rec) |
| + } |
| + return recs, nil |
| +} |
| + |
| // DeleteSidecarRecord retires a sidecar's provisioning authorization. Object |
| // deletion only; Store's Delete is idempotent, so retiring an already-gone |
| // record is not an error. |
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 64c1ea69d..2104ec1f2 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 |
| @@ -1,6 +1,7 @@ |
| # Census overlay plans-3b-never-sleep, written by census-propose (#369). census-ruling-plans-3b-never-sleep, approved by the census owner before the merge; the approval card is docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md. |
| -# 1 body, 0 sites, 2 envelope: each changed unit re-frozen at its existing disposition with its claim appended. |
| +# 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 6a04ed0f7a9bfbaee68f38eb103727999c4bb77e176711af7d62cca533e48362 |
| -@coverage internal/schedule 3952eed7260e7576bddd5c1888b5519b906f1ec2ba8c66998dce09684169bb0d |
| +@coverage internal/console fa8657e7d115ac2970a4ce43dba4fa92b10aaa156094091f736ed762b0743173 |
| +@coverage internal/provision 83a571025e988adf3dd9ffa4afcc4ac0d1cbdc9d41f4555886863ab577ed0212 |
| +@coverage internal/schedule f749883df03257d26076da5bb9add4243e453c488688e7ce4324d1c1df8b221f |
diff --git a/internal/schedule/mingap.go b/internal/schedule/mingap.go| index 21ff6d985..fb5385add 100644 |
| --- a/internal/schedule/mingap.go |
| +++ b/internal/schedule/mingap.go |
| @@ -6,15 +6,15 @@ import ( |
| ) |
| // ShortestInterval is the least time between two runs of s: its every: |
| -// duration, or for a cron line the shortest gap its minute and hour fields |
| -// allow. Plans 3b uses it to hold schedules to a plan's minimum. |
| +// duration, or for a cron line the shortest gap between two of its firings. |
| +// Plans 3b uses it to hold schedules to a plan's minimum. |
| // |
| -// The cron bound deliberately ignores the day fields and assumes a matching |
| -// day can follow a matching day. A sampled window cannot prove a line |
| -// complies: "*/1 * * 4 *" fires every minute in April only, and |
| -// "0,5 0 29 2 *" bursts on a leap day (Astra plans-3b finding 1). The bound |
| -// is conservative: a weekly line whose last firing of the day is close to |
| -// its first is refused even though its days never touch. |
| +// The cron gap is exact over the whole Gregorian calendar, not sampled from |
| +// a window: "*/1 * * 4 *" fires every minute in April only, and |
| +// "0,5 0 29 2 *" bursts on a leap day (Astra plans-3b finding 1), while |
| +// "0,55 0,23 * * 1" never fires on consecutive days, so its gap is 55 |
| +// minutes, not the 5 a day-blind bound would claim (Astra plans-3b round 2 |
| +// finding 3). Cron runs in UTC here, so there is no daylight-saving shift. |
| func (s Schedule) ShortestInterval() (time.Duration, error) { |
| sp, err := s.Spec() |
| if err != nil { |
| @@ -29,8 +29,14 @@ func (s Schedule) ShortestInterval() (time.Duration, error) { |
| return 0, errors.New("schedule has no interval") |
| } |
| -// shortestGap is the smallest gap between two firings within a day, or |
| -// from the day's last firing to the next day's first. |
| +// gregorianCycleDays is the length of the Gregorian calendar's 400-year |
| +// cycle. It is a whole number of weeks, so dates, weekdays and leap days all |
| +// repeat after it. |
| +const gregorianCycleDays = 146097 |
| + |
| +// shortestGap is the smallest gap between two firings: within a matching |
| +// day, or from one matching day's last firing to the next matching day's |
| +// first, with the days taken from a whole 400-year cycle. |
| func (c *CronExpr) shortestGap() time.Duration { |
| var times []int // minutes of the day, ascending |
| for h := 0; h < 24; h++ { |
| @@ -43,10 +49,11 @@ func (c *CronExpr) shortestGap() time.Duration { |
| } |
| } |
| } |
| - if len(times) == 0 { |
| - return 24 * time.Hour |
| + days := c.closestMatchingDays() |
| + if len(times) == 0 || days == 0 { |
| + return 24 * time.Hour // never fires |
| } |
| - shortest := 24*60 - times[len(times)-1] + times[0] // last today to first tomorrow |
| + shortest := days*24*60 - times[len(times)-1] + times[0] // last firing to the next matching day's first |
| for i := 1; i < len(times); i++ { |
| if gap := times[i] - times[i-1]; gap < shortest { |
| shortest = gap |
| @@ -54,3 +61,33 @@ func (c *CronExpr) shortestGap() time.Duration { |
| } |
| return time.Duration(shortest) * time.Minute |
| } |
| + |
| +// closestMatchingDays is the fewest days between two days the line fires on |
| +// (1 for consecutive days), or 0 if it fires on no day at all. A line that |
| +// matches one day per cycle is that cycle's length apart from itself. |
| +func (c *CronExpr) closestMatchingDays() int { |
| + t := time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC) |
| + first, prev, closest := -1, -1, 0 |
| + for i := 0; i < gregorianCycleDays; i, t = i+1, t.Add(24*time.Hour) { |
| + if c.month&(1<<uint(t.Month())) == 0 || !c.dayMatches(t) { |
| + continue |
| + } |
| + if prev >= 0 && (closest == 0 || i-prev < closest) { |
| + closest = i - prev |
| + if closest == 1 { |
| + return 1 |
| + } |
| + } |
| + if first < 0 { |
| + first = i |
| + } |
| + prev = i |
| + } |
| + if first < 0 { |
| + return 0 |
| + } |
| + if wrap := gregorianCycleDays - prev + first; closest == 0 || wrap < closest { |
| + closest = wrap |
| + } |
| + return closest |
| +} |
diff --git a/internal/schedule/mingap_test.go b/internal/schedule/mingap_test.go| index 3685e1f20..60efe6295 100644 |
| --- a/internal/schedule/mingap_test.go |
| +++ b/internal/schedule/mingap_test.go |
| @@ -14,10 +14,17 @@ func TestCronMinGap(t *testing.T) { |
| {Schedule{Name: "a", Cron: "*/15 * * * *", Path: "/x"}, 15 * time.Minute}, |
| {Schedule{Name: "a", Cron: "0,5 * * * *", Path: "/x"}, 5 * time.Minute}, |
| {Schedule{Name: "a", Cron: "0 3 * * *", Path: "/x"}, 24 * time.Hour}, |
| - // Day fields are ignored on purpose: the bound comes from minute and |
| - // hour alone, assuming a matching day can follow a matching day, so no |
| - // seasonal, leap-day or weekday restriction can hide a burst. |
| - {Schedule{Name: "a", Cron: "0 9 * * 1", Path: "/x"}, 24 * time.Hour}, |
| + // Day fields count, over the whole 400-year calendar: seasonal, |
| + // leap-day and weekday lines are held to their real gaps, neither |
| + // shorter (round 2 finding 3) nor longer (round 1 finding 1). |
| + {Schedule{Name: "a", Cron: "0 9 * * 1", Path: "/x"}, 7 * 24 * time.Hour}, |
| + {Schedule{Name: "a", Cron: "0,55 0,23 * * 1", Path: "/x"}, 55 * time.Minute}, |
| + {Schedule{Name: "a", Cron: "0,55 0,23 * * 1,2", Path: "/x"}, 5 * time.Minute}, |
| + {Schedule{Name: "a", Cron: "0,55 0,23 13 * 5", Path: "/x"}, 5 * time.Minute}, // the 13th or a Friday: Thursday 13th, Friday 14th |
| + {Schedule{Name: "a", Cron: "0,59 0,23 28,29 2 *", Path: "/x"}, time.Minute}, // consecutive only in leap years |
| + {Schedule{Name: "a", Cron: "0,59 0,23 29 2 *", Path: "/x"}, 59 * time.Minute}, |
| + {Schedule{Name: "a", Cron: "0,55 0,23 31 * *", Path: "/x"}, 55 * time.Minute}, |
| + {Schedule{Name: "a", Cron: "0 0 30 2 *", Path: "/x"}, 24 * time.Hour}, // never fires |
| {Schedule{Name: "a", Cron: "*/1 * * 4 *", Path: "/x"}, time.Minute}, |
| {Schedule{Name: "a", Cron: "0,5 0 29 2 *", Path: "/x"}, 5 * time.Minute}, |
| {Schedule{Name: "a", Cron: "*/1 * 13 * 5", Path: "/x"}, time.Minute}, |