console: plans 3b review fixes (promote doors, seasonal crons, sidecar create, fleet ownership)
Astra's whole-branch review (CODEX-FINDINGS-PLANS-3B-CODE.md): daemon releases are refused at every member promote door, cron gaps are bounded over minute and hour only, a new sidecar of an always-on app is refused on shared boxes, enabling always-on needs every sidecar on an owned fleet, and own-boxes means a fleet the account owns. Census: ownsFleet as a decision (plans-3b-fleet-owner overlay), plans-3b card re-proposed, record-write layer plans3bWriteCensusInventory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
16 files changed,
+464
−55
docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md+12 −4docs/superpowers/plans/2026-10-23-plans-3b-never-sleep.md+16 −0docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md+25 −0internal/console/apicli.go+11 −1internal/console/apisidecars.go+12 −1internal/console/pendinghandlers.go+19 −0internal/console/plancaps.go+85 −11internal/console/plancaps_doors_test.go+179 −1internal/console/server.go+24 −2internal/provision/plan9_b8_census_test.go+1 −1internal/provision/plans3b_census_test.go+27 −0internal/provision/testdata/plan14-root-reads-chain.tsv+2 −1internal/provision/testdata/plan14-root-reads-plans-3b-fleet-owner.tsv+3 −0internal/provision/testdata/plan14-root-reads-plans-3b-never-sleep.tsv+4 −4internal/schedule/mingap.go+35 −28internal/schedule/mingap_test.go+9 −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 ef207b406..856232e31 100644 |
| --- a/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| +++ b/docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md |
| @@ -1,15 +1,23 @@ |
| # Census approval: plans-3b-never-sleep |
| -Status: APPROVED by Paul (census owner), 2026-10-25, in session; recorded on #280. |
| - |
| 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 the account's own fleet, or any app with plans off, ships as before. No new read. | |
| +| `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`. |
| -Seal: `86570d1326baad6d866b4066b6bf65218434b201fb1f2c05ebc6e3c79f9c5a98` → `3e22132e02fdb3acc157f95f261d43b16b8ebbcd0f7636739ed50695a02b84b6`. |
| +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…`). |
| + |
| +| unit | change | reads | what it does | |
| +|---|---|---|---| |
| +| `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). |
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 082341fe7..ee742ec89 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 |
| @@ -66,3 +66,19 @@ Asked "Should we not just have plans that include daemons and sidecars? Or do we |
| ## Done when |
| Every task's tests pass, Review Focus 1–4 are pinned, CI is green after merging current main, and Astra's whole-branch review has no open Critical or Important finding. |
| + |
| +## Astra review (2026-10-25) |
| + |
| +Findings in `docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md`. All six were fixed: |
| + |
| +1. Cron gap: `ShortestInterval` is now a conservative bound over the minute and hour fields. It ignores the day fields, so seasonal and leap-day crons can't slip through. |
| +2. Promotion: CLI promote, browser promote, new canaries and pending promote all refuse a daemon release on shared boxes under a plan (`checkDaemonRelease`). The pending check runs at execution, against the plan as it stands then. |
| +3. Sidecar create: refused on shared boxes when the app's manifest says `hibernate: false` (`checkAlwaysOnCreate`). |
| +4. Always-on enable on a fleet: every existing sidecar record must already be on a fleet the account owns. |
| +5. Fleet ownership: `ownsFleet` checks that the fleet exists and that `Account == acct.Sqid`. Lookup failures are errors. |
| +6. Census card: names the new indirect reads, and records `ownsFleet` as a hand-written decision. |
| + |
| +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. |
| +- **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`. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md| new file mode 100644 |
| index 000000000..bafe3c464 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-PLANS-3B-CODE.md |
| @@ -0,0 +1,25 @@ |
| +1. **Important — Seasonal crons bypass the minimum interval.** [internal/schedule/mingap.go:30](internal/schedule/mingap.go:30) |
| + `*/1 * * 4 *` returns 62 days because its first firing falls outside the fixed January–March window. The console therefore accepts it on free, although it fires every minute throughout April. Leap-day bursts such as `0,5 0 29 2 *` escape too. **Fix:** establish the shortest gap across the expression’s permitted calendar patterns; absence from this window cannot prove compliance. Add seasonal, leap-day, and combined day-of-month/day-of-week cases. |
| + |
| +2. **Important — Promotion bypasses the daemon refusal.** [internal/console/apicli.go:846](internal/console/apicli.go:846) |
| + Promotion reads an existing manifest but discards its capabilities. A daemon release shipped before plans were enabled can still be promoted on shared boxes. Browser promotion and approved-proposal execution also lack the check. Even where convergence subsequently refuses the daemon, the channel has already changed, violating §5.2’s explicit promotion-time refusal. **Fix:** check the target manifest’s daemon capability against current limits and placement before every member-facing promotion executes. |
| + |
| +3. **Important — Creating a sidecar can introduce a new always-on worker.** [internal/console/apisidecars.go:323](internal/console/apisidecars.go:323) |
| + An older `hibernate:false` manifest survives enabling plans, as intended. But a member below the worker-count cap can then create a new sidecar on shared boxes: creation checks only the count, and reconciliation applies the manifest’s `false` setting to the new worker. Preserving existing workers does not justify creating additional always-on workers. **Fix:** check the effective manifest hibernation setting during creation, before either record-write path. Preserve the required omitted-field behavior for enable. |
| + |
| +4. **Important — App placement does not prove existing sidecars moved to own boxes.** [internal/console/plancaps.go:315](internal/console/plancaps.go:315) |
| + With `HostClaim` off, place an app containing hibernating sidecars onto an owned fleet, then enable `hibernate:false`. The new check permits this from stored app placement. However, `placeAppTarget` rewrites instance records, not sidecar records; shared-box reconciliation still selects those sidecars by their original record region and applies the new hibernation setting. This creates new always-on behavior on shared boxes, rather than preserving the explicitly deferred fleet-to-shared leftovers. **Fix:** verify affected sidecar records actually target owned fleets before granting the exception, or complete their relocation first. |
| + |
| +5. **Minor — Fleet syntax is treated as ownership evidence.** [internal/console/plancaps.go:360](internal/console/plancaps.go:360) |
| + `FleetPlacement` only parses the string. A stored or fallback placement naming a nonexistent fleet or another account’s fleet receives all three exemptions. The new test helper even creates no fleet record. Normal placement APIs validate ownership, which limits ordinary reachability, but this helper does not establish what it claims. **Fix:** resolve the fleet, require `fleet.Account == acct.Sqid`, and propagate lookup failures. Test owned, foreign, missing, and unreadable fleets. |
| + |
| +6. **Minor — The census card omits new indirect reads.** [docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md:7](docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md:7), at the reviewed commit. |
| + The unchanged direct read-site count is consistent with the scanner, but “No new read” does not describe the added execution path: shipping now reads account-plan and app-placement state and can open/list the registry. **Fix:** distinguish unchanged direct census sites from these newly reachable reads so the approval describes the actual change. |
| + |
| +**Declined to judge** |
| + |
| +- Approved copy, section 3 decisions, operator-only paths, and the explicitly deferred fleet-to-shared leftovers. |
| +- Test success and census approval provenance. Targeted Go tests could not start because the sandbox refused the temporary build directory. Static inspection supports plans-off behavior, identical daemon re-ship refusal, and preservation of omitted hibernation settings; `git diff --check` passed. |
| +- The checkout advanced externally to `9f19e13` during review, adding only census approval notes. Findings remain scoped to `4d2b80284b80165b8dc7a253495303cd7c5ad12f`. |
| + |
| +**Verdict:** Changes required: four Important findings remain. I made no repository changes. |
| \ No newline at end of file |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index f0f347547..37bfbad50 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -843,10 +843,20 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| - if _, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version); err != nil { |
| + man, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version) |
| + if err != nil { |
| jsonError(w, http.StatusNotFound, "Version "+version+" is not a shipped release of "+app+".") |
| return |
| } |
| + if err := s.checkDaemonRelease(acct, app, man); err != nil { |
| + if isPlanCap(err) { |
| + writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| + return |
| + } |
| + log.Printf("console: api promote %s/%s: plan: %v", acct.Sqid, app, err) |
| + jsonError(w, http.StatusBadGateway, planCheckFailedCopy) |
| + return |
| + } |
| // The pipeline is read ONCE, for the gate below and for the held answer |
| // further down. An unreadable one refuses outright: GetPipeline's error |
| // means "not safely nil", and treating it as "no pipeline" would drop |
diff --git a/internal/console/apisidecars.go b/internal/console/apisidecars.go| index d803c88bd..b56d45c93 100644 |
| --- a/internal/console/apisidecars.go |
| +++ b/internal/console/apisidecars.go |
| @@ -329,6 +329,17 @@ func (s *Server) handleAPISidecarCreate(w http.ResponseWriter, r *http.Request) |
| jsonError(w, http.StatusBadGateway, "could not create the sidecar — try again") |
| return |
| } |
| + if m.Hibernate != nil && !*m.Hibernate { |
| + if err := s.checkAlwaysOnCreate(acct, app); err != nil { |
| + if isPlanCap(err) { |
| + writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| + return |
| + } |
| + log.Printf("console: api sidecars create %s/%s: plan: %v", acct.Sqid, app, err) |
| + jsonError(w, http.StatusBadGateway, planCheckFailedCopy) |
| + return |
| + } |
| + } |
| // The leftover-replica rail (#199, createleftovers.go): a sidecar's host |
| // is derived from its name, so a name reused after a delete lands on the |
| // deleted sidecar's replica. Refused, after every validation above. |
| @@ -609,7 +620,7 @@ func (s *Server) handleAPISidecarEnable(w http.ResponseWriter, r *http.Request) |
| // asks for hibernate:false is refused; one that omits the field leaves an |
| // existing manifest as it was. |
| if req.Hibernate != nil && !*req.Hibernate { |
| - if err := s.checkAlwaysOn(acct, app); err != nil { |
| + if err := s.checkAlwaysOn(ctx, acct, app); err != nil { |
| if isPlanCap(err) { |
| writeJSON(w, http.StatusConflict, map[string]any{"error": err.Error(), "code": "cap"}) |
| return |
diff --git a/internal/console/pendinghandlers.go b/internal/console/pendinghandlers.go| index a28a1247a..3df10349f 100644 |
| --- a/internal/console/pendinghandlers.go |
| +++ b/internal/console/pendinghandlers.go |
| @@ -542,6 +542,25 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| return "", staleProposal(aerr.Error()) |
| } |
| } |
| + // 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 { |
| + 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 { |
| + var planCap *planCapError |
| + if errors.As(cerr, &planCap) { |
| + return "", errors.New(planCap.Browser()) |
| + } |
| + log.Printf("console: execute promote %s/%s: plan: %v", acct.Sqid, app, cerr) |
| + return "", errors.New(planCheckFailedCopy) |
| + } |
| + } |
| res, err := release.PromoteOnLadder(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, app, p.Channel, p.Version, |
| release.PromoteOptions{Actor: actor, Hotfix: p.Hotfix, Retry: p.Retry}) |
| if err != nil { |
diff --git a/internal/console/plancaps.go b/internal/console/plancaps.go| index 1e12d290c..7545b156f 100644 |
| --- a/internal/console/plancaps.go |
| +++ b/internal/console/plancaps.go |
| @@ -10,6 +10,7 @@ import ( |
| "github.com/carlosframework/platform/internal/provision" |
| "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| "github.com/carlosframework/platform/internal/schedule" |
| ) |
| @@ -305,9 +306,21 @@ func (s *Server) checkDaemon(acct *Account, app string) error { |
| return &planCapError{msg: capMessage("cli.cap.daemon")} |
| } |
| -// checkAlwaysOn refuses settings that would keep a sidecar awake for good |
| -// (plans 3b; spec §5.2: no plan offers always-on). |
| -func (s *Server) checkAlwaysOn(acct *Account, app string) error { |
| +// checkDaemonRelease is checkDaemon for a release already shipped: a |
| +// daemon release shipped before plans were on must not be promoted onto |
| +// shared boxes under one (Astra plans-3b finding 2). A release without the |
| +// capability passes. |
| +func (s *Server) checkDaemonRelease(acct *Account, app string, m *release.Manifest) error { |
| + if m == nil || m.Capabilities == nil || !m.Capabilities.Daemon { |
| + return nil |
| + } |
| + return s.checkDaemon(acct, app) |
| +} |
| + |
| +// 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. |
| +func (s *Server) checkAlwaysOnCreate(acct *Account, app string) error { |
| l, err := s.EffectiveLimits(acct) |
| if err != nil || l == nil { |
| return err |
| @@ -318,6 +331,47 @@ func (s *Server) checkAlwaysOn(acct *Account, app string) error { |
| return &planCapError{msg: capMessage("cli.cap.always_on")} |
| } |
| +// checkAlwaysOn refuses settings that would keep an app's sidecars awake |
| +// for good (plans 3b; spec §5.2). On its own fleet an app may, but only once |
| +// every existing sidecar runs there too: placing an app does not move its |
| +// sidecar records (Astra plans-3b finding 4). |
| +func (s *Server) checkAlwaysOn(ctx context.Context, acct *Account, app string) error { |
| + if err := s.checkAlwaysOnCreate(acct, app); !isPlanCap(err) { |
| + if err != nil { |
| + return err |
| + } |
| + if s.cfg.Releases == nil { |
| + return nil |
| + } |
| + l, _ := s.EffectiveLimits(acct) |
| + if l == nil { |
| + return nil |
| + } |
| + recs, _, err := provision.ListSidecarRecords(ctx, s.cfg.Releases, acct.Sqid, app, sidecarListMax) |
| + if err != nil { |
| + return err |
| + } |
| + if len(recs) == 0 { |
| + return nil |
| + } |
| + reg, err := openRegistry(s.cfg.Registry) |
| + if err != nil { |
| + return err |
| + } |
| + for _, rec := range recs { |
| + own, err := ownsFleet(reg, acct, rec.Region) |
| + if err != nil { |
| + return err |
| + } |
| + if !own { |
| + return &planCapError{msg: capMessage("cli.cap.always_on")} |
| + } |
| + } |
| + return nil |
| + } |
| + return &planCapError{msg: capMessage("cli.cap.always_on")} |
| +} |
| + |
| // checkSchedule refuses a schedule that would run more often than the |
| // plan's minimum interval (plans 3b; spec §5.2). Cron lines are held to |
| // their shortest gap between firings. |
| @@ -342,21 +396,41 @@ func (s *Server) checkSchedule(acct *Account, app string, sc schedule.Schedule) |
| // onOwnBoxes reports whether app is placed on one of the account's own |
| // fleets. Always-on work there costs the deployment nothing, so the |
| // never-sleep and schedule checks pass for it (Paul, 2026-10-24: always-on |
| -// on your own boxes). Sidecars of a placed app inherit its fleet region. |
| +// on your own boxes). New sidecars of a placed app inherit its fleet region; |
| +// existing ones keep theirs, which checkAlwaysOn reads. A console with no |
| +// registry has no fleets, so nothing it serves is on own boxes. |
| func (s *Server) onOwnBoxes(acct *Account, app string) (bool, error) { |
| placement, err := s.store.AppPlacement(acct.ID, app) |
| if err != nil { |
| return false, err |
| } |
| - if placement == "" && s.cfg.Registry != "" { |
| - reg, err := openRegistry(s.cfg.Registry) |
| - if err != nil { |
| - return false, err |
| - } |
| + if s.cfg.Registry == "" { |
| + return false, nil |
| + } |
| + reg, err := openRegistry(s.cfg.Registry) |
| + if err != nil { |
| + return false, err |
| + } |
| + if placement == "" { |
| if placement, err = s.appPlacement(reg, acct, app); err != nil { |
| return false, err |
| } |
| } |
| - _, ok := registry.FleetPlacement(placement) |
| - return ok, nil |
| + return ownsFleet(reg, acct, placement) |
| +} |
| + |
| +// ownsFleet reports whether placement names a fleet acct itself owns. The |
| +// "fleet/<name>" shape alone proves nothing: fleet names are one namespace |
| +// across accounts, so an unknown or foreign fleet is shared boxes as far as |
| +// a plan is concerned (Astra plans-3b finding 5). |
| +func ownsFleet(reg consoleRegistry, acct *Account, placement string) (bool, error) { |
| + name, ok := registry.FleetPlacement(placement) |
| + if !ok { |
| + return false, nil |
| + } |
| + f, err := reg.GetFleet(name) |
| + if err != nil { |
| + return false, err |
| + } |
| + return f != nil && f.Account == acct.Sqid, nil |
| } |
diff --git a/internal/console/plancaps_doors_test.go b/internal/console/plancaps_doors_test.go| index be1a883f5..00c6d182f 100644 |
| --- a/internal/console/plancaps_doors_test.go |
| +++ b/internal/console/plancaps_doors_test.go |
| @@ -5,10 +5,13 @@ import ( |
| "fmt" |
| "net/http" |
| "net/url" |
| + "path/filepath" |
| "strings" |
| "sync" |
| "testing" |
| + "time" |
| + "github.com/carlosframework/platform/internal/caps" |
| "github.com/carlosframework/platform/internal/plans" |
| "github.com/carlosframework/platform/internal/provision" |
| "github.com/carlosframework/platform/internal/registry" |
| @@ -532,13 +535,67 @@ func TestScheduleWithoutACatalogUnchanged(t *testing.T) { |
| // ---- Plans 3b: own boxes (Paul, 2026-10-24) ---- |
| func placeOnFleet(t *testing.T, fx *consoleFixture, app string) { |
| + t.Helper() |
| + addFleet(t, fx, "ownbox", fx.sqid) |
| + placeOnNamedFleet(t, fx, app, "ownbox") |
| +} |
| + |
| +// addFleet registers a fleet owned by account in the fixture's registry. |
| +func addFleet(t *testing.T, fx *consoleFixture, name, account string) { |
| + t.Helper() |
| + reg, err := registry.Open(fx.regPath) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer reg.Close() |
| + if err := reg.UpsertFleet(registry.Fleet{Name: name, Account: account, Bucket: name + "-bucket"}); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// placeOnNamedFleet stores app's placement as fleet/<name>, whether or not |
| +// that fleet exists or is the account's own. |
| +func placeOnNamedFleet(t *testing.T, fx *consoleFixture, app, name string) { |
| t.Helper() |
| ops, _ := fx.st.AccountBySqid(fx.sqid) |
| - if err := fx.st.SetAppPlacement(ops.ID, app, registry.PlacementTarget("ownbox")); err != nil { |
| + if err := fx.st.SetAppPlacement(ops.ID, app, registry.PlacementTarget(name)); err != nil { |
| t.Fatal(err) |
| } |
| } |
| +// Astra plans-3b finding 5: the fleet/<name> shape is not ownership. Only a |
| +// fleet the account owns exempts an app; a missing or foreign one does not, |
| +// and a registry that cannot be read is an error, not a pass. |
| +func TestOwnBoxesNeedsAFleetTheAccountOwns(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + acct, _ := fx.st.AccountBySqid(fx.sqid) |
| + addFleet(t, fx, "mine", fx.sqid) |
| + addFleet(t, fx, "theirs", "zzz") |
| + for _, tc := range []struct { |
| + fleet string |
| + own bool |
| + }{{"mine", true}, {"theirs", false}, {"nowhere", false}} { |
| + placeOnNamedFleet(t, fx, "kass", tc.fleet) |
| + got, err := fx.srv.onOwnBoxes(acct, "kass") |
| + if err != nil || got != tc.own { |
| + t.Errorf("onOwnBoxes on fleet/%s = %v, %v, want %v", tc.fleet, got, err, tc.own) |
| + } |
| + if err := fx.srv.checkDaemon(acct, "kass"); (err == nil) != tc.own { |
| + t.Errorf("checkDaemon on fleet/%s = %v, want refused=%v", tc.fleet, err, !tc.own) |
| + } |
| + } |
| + |
| + placeOnNamedFleet(t, fx, "kass", "mine") |
| + fx.srv.cfg.Registry = filepath.Join(t.TempDir(), "missing", "registry.db") |
| + if _, err := fx.srv.onOwnBoxes(acct, "kass"); err == nil { |
| + t.Error("onOwnBoxes with an unreadable registry = nil error, want the failure") |
| + } |
| + if err := fx.srv.checkDaemon(acct, "kass"); err == nil || isPlanCap(err) { |
| + t.Errorf("checkDaemon with an unreadable registry = %v, want a non-cap error", err) |
| + } |
| +} |
| + |
| func TestDaemonAllowedOnOwnBoxes(t *testing.T) { |
| fx, token := daemonFixture(t, "code-cap-daemon-fleet") |
| fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| @@ -566,3 +623,124 @@ func TestFrequentSchedulesAllowedOnOwnBoxes(t *testing.T) { |
| t.Fatalf("every 1m for an app on its own fleet = %d, want 200", status) |
| } |
| } |
| + |
| +// Astra plans-3b finding 3: a manifest set to stay awake before plans were |
| +// on must not let a new always-on worker be created on shared boxes. |
| +func TestSidecarCreateRefusedUnderAnAlwaysOnManifest(t *testing.T) { |
| + fx, token := instanceFixture(t, "code-cap-create-alwayson") |
| + m := titoSidecarManifest() |
| + off := false |
| + m.Hibernate = &off |
| + enableSidecars(t, fx, "titogo", m) |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + status, _, out := rawReq(t, http.MethodPost, sidecarsURL(fx, "bab", "titogo"), token, `{"name":"bot","region":"ie"}`) |
| + if status != http.StatusConflict || !strings.Contains(out, `"code":"cap"`) { |
| + t.Fatalf("new worker under an always-on manifest on shared boxes = %d %s, want 409 cap", status, out) |
| + } |
| +} |
| + |
| +// Astra plans-3b finding 4: an app placed on its fleet whose existing |
| +// sidecars still run on shared boxes may not be switched to stay awake. |
| +func TestAlwaysOnRefusedWhileSidecarsRemainOnSharedBoxes(t *testing.T) { |
| + fx, token := instanceFixture(t, "code-cap-alwayson-moved") |
| + enableSidecars(t, fx, "titogo", titoSidecarManifest()) |
| + if status, _, out := rawReq(t, http.MethodPost, sidecarsURL(fx, "bab", "titogo"), token, `{"name":"bot","region":"ie"}`); status != http.StatusCreated { |
| + t.Fatalf("setup sidecar on shared boxes = %d %s", status, out) |
| + } |
| + 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.StatusConflict || !strings.Contains(out, `"code":"cap"`) { |
| + t.Fatalf("hibernate:false while a sidecar runs on shared boxes = %d %s, want 409 cap", status, out) |
| + } |
| +} |
| + |
| +// Astra plans-3b finding 2: a daemon release shipped before plans were on |
| +// must not be promoted on shared boxes under a plan. |
| +func TestPromoteRefusesADaemonReleaseUnderAPlan(t *testing.T) { |
| + fx, token := daemonFixture(t, "code-cap-daemon-promote") |
| + if status, out := shipDaemon(t, fx, token, "dmn00020"); status != http.StatusCreated { |
| + t.Fatalf("setup daemon ship with no catalog = %d %v", status, out) |
| + } |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/relayapp/promote", |
| + map[string]string{"version": "dmn00020", "channel": "canary/x"}, token) |
| + if status != http.StatusConflict || out["code"] != "cap" { |
| + t.Fatalf("promote a daemon release under a plan = %d %v, want 409 cap", status, out) |
| + } |
| + placeOnFleet(t, fx, "relayapp") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/relayapp/promote", |
| + map[string]string{"version": "dmn00020", "channel": "canary/x"}, token); status != http.StatusOK { |
| + t.Fatalf("promote a daemon release on its own fleet = %d %v, want 200", status, out) |
| + } |
| +} |
| + |
| +// shipDaemonVersion writes a release declaring the daemon capability |
| +// straight to the bucket, the state a ship from before plans were on leaves. |
| +func shipDaemonVersion(t *testing.T, fx *consoleFixture, app, version string) { |
| + t.Helper() |
| + m := release.Manifest{ |
| + Schema: 1, App: app, Version: version, Kind: "binary", |
| + BuiltAt: "2026-07-03T10:00:00Z", |
| + Artifacts: map[string]release.Artifact{"linux-amd64": {File: app}}, |
| + Capabilities: &caps.Set{V: caps.Version, Daemon: true}, |
| + } |
| + if err := release.Ship(context.Background(), fx.rel, release.OpsAccount, m, |
| + map[string][]byte{"linux-amd64": []byte("binary " + version)}); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +func TestBrowserPromoteAndCanaryRefuseADaemonReleaseUnderAPlan(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + shipDaemonVersion(t, fx, "kass", "dmn1") |
| + fx.srv.cfg.PlanCatalog = examplePlanCatalog(t) |
| + c := signInPaul(t, fx) |
| + csrf := sessionToken(t, fx, c) |
| + |
| + postForm(t, c, fx.u("/apps/kass/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"canary/x"}, "version": {"dmn1"}, "confirmed": {"1"}}) |
| + if p := fx.channelVersion(t, "kass", "canary/x"); p.Version == "dmn1" { |
| + t.Fatal("the browser promoted a daemon release onto shared boxes under a plan") |
| + } |
| + postForm(t, c, fx.u("/apps/kass/canary"), url.Values{"csrf": {csrf}, "name": {"rc"}, "version": {"dmn1"}}) |
| + if _, _, err := fx.rel.Get(context.Background(), release.ChannelKey(release.OpsAccount, "kass", "canary/rc")); err == nil { |
| + t.Fatal("a canary of a daemon release was created on shared boxes under a plan") |
| + } |
| + |
| + placeOnFleet(t, fx, "kass") |
| + postForm(t, c, fx.u("/apps/kass/canary"), url.Values{"csrf": {csrf}, "name": {"rc"}, "version": {"dmn1"}}) |
| + if p := fx.channelVersion(t, "kass", "canary/rc"); p.Version != "dmn1" { |
| + t.Fatalf("canary/rc = %+v on its own fleet, want dmn1", p) |
| + } |
| +} |
| + |
| +// A daemon proposal made before plans were on meets the plan at execution. |
| +func TestPendingPromoteOfADaemonReleaseRefusedUnderAPlan(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", "dmn2") |
| + seedChannel(t, fx, "widget", "edge", "dmn2") |
| + seedChannel(t, fx, "widget", "production", "v2") |
| + |
| + if status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"dmn2"}, "confirmed": {"1"}}); status != http.StatusSeeOther { |
| + t.Fatalf("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-pending") |
| + 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 proposal under a plan, want v2", got.Version) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index 433d08e15..b2e168c3b 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -4532,10 +4532,21 @@ func (s *Server) handlePromote(w http.ResponseWriter, r *http.Request) { |
| back("err", "Invalid version.") |
| return |
| } |
| - if _, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version); err != nil { |
| + man, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version) |
| + if err != nil { |
| back("err", "Version "+version+" is not a shipped release of "+app+".") |
| return |
| } |
| + if err := s.checkDaemonRelease(acct, app, man); err != nil { |
| + var planCap *planCapError |
| + if errors.As(err, &planCap) { |
| + back("err", planCap.Browser()) |
| + return |
| + } |
| + log.Printf("console: promote %s/%s: plan: %v", acct.Sqid, app, err) |
| + back("err", planCheckFailedCopy) |
| + return |
| + } |
| // Which gate this promotion meets is the pipeline's answer (release |
| // pipelines v2 §5), so the pipeline is read before any of them. An |
| // unreadable one refuses: "not safely nil" must never fall back to the |
| @@ -5122,10 +5133,21 @@ func (s *Server) handleNewCanary(w http.ResponseWriter, r *http.Request) { |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| - if _, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version); err != nil { |
| + man, err := release.GetManifest(ctx, s.cfg.Releases, acct.Sqid, app, version) |
| + if err != nil { |
| back("err", "Version "+version+" is not a shipped release of "+app+".") |
| return |
| } |
| + if err := s.checkDaemonRelease(acct, app, man); err != nil { |
| + var planCap *planCapError |
| + if errors.As(err, &planCap) { |
| + back("err", planCap.Browser()) |
| + return |
| + } |
| + log.Printf("console: canary %s/%s: plan: %v", acct.Sqid, app, err) |
| + back("err", planCheckFailedCopy) |
| + return |
| + } |
| // New means new: an existing canary is promoted to (releases table) or |
| // retired, never quietly re-created. |
| if _, _, err := s.cfg.Releases.Get(ctx, release.ChannelKey(acct.Sqid, app, channel)); err == nil { |
diff --git a/internal/provision/plan9_b8_census_test.go b/internal/provision/plan9_b8_census_test.go| index 7a4016832..678cca769 100644 |
| --- a/internal/provision/plan9_b8_census_test.go |
| +++ b/internal/provision/plan9_b8_census_test.go |
| @@ -366,7 +366,7 @@ func TestPlan9B8RecordWriteCensus(t *testing.T) { |
| if len(got) < 100 { |
| t.Fatalf("census found only %d units under %s; the walk is broken", len(got), root) |
| } |
| - inventory := wakeLatency3WriteCensusInventory() |
| + inventory := plans3bWriteCensusInventory() |
| problems := append(censusDiff(inventory, got), censusPolicy(inventory)...) |
| if len(problems) > 0 { |
| t.Fatalf("record-write census rejected %d site(s); update the explicit inventory only after reviewing the source:\n %s", len(problems), strings.Join(problems, "\n ")) |
diff --git a/internal/provision/plans3b_census_test.go b/internal/provision/plans3b_census_test.go| new file mode 100644 |
| index 000000000..4f5989dcd |
| --- /dev/null |
| +++ b/internal/provision/plans3b_census_test.go |
| @@ -0,0 +1,27 @@ |
| +package provision |
| + |
| +// Record-write census entries for plans 3b (plans-3b-never-sleep), layered |
| +// on wakeLatency3WriteCensusInventory (main's last at the branch's merge). |
| +// handleAPISidecarCreate gains one early return after the sidecar-count cap: |
| +// under a plan, an app whose sidecar manifest says hibernate:false is refused |
| +// 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. |
| +func plans3bWriteCensusInventory() []censusEntry { |
| + entries := append([]censusEntry(nil), wakeLatency3WriteCensusInventory()...) |
| + replacements := []censusEntry{ |
| + {"internal/console/apisidecars.go", "Server.handleAPISidecarCreate", "record-writer", "key:CreateSidecarRecord", "77b9512efbfee3839265e83e671c00dbd97921460b115ef936eed810fcc409a7"}, |
| + } |
| + for _, r := range replacements { |
| + found := false |
| + for i, e := range entries { |
| + if e.File == r.File && e.Unit == r.Unit { |
| + entries[i], found = r, true |
| + } |
| + } |
| + if !found { |
| + entries = append(entries, r) |
| + } |
| + } |
| + return entries |
| +} |
diff --git a/internal/provision/testdata/plan14-root-reads-chain.tsv b/internal/provision/testdata/plan14-root-reads-chain.tsv| index 2debd0b5f..0b34d0857 100644 |
| --- a/internal/provision/testdata/plan14-root-reads-chain.tsv |
| +++ b/internal/provision/testdata/plan14-root-reads-chain.tsv |
| @@ -10,4 +10,5 @@ plan14-root-reads-edge-asset-cache-2.tsv de15b849fcb93b76caca55be924cde41512f8c4 |
| plan14-root-reads-pro-6214-service-runs-decisions.tsv ec2d8daaec385b34b3be9c8c6430427c082ac36cd40150e1c6f520e20e3da72d |
| plan14-root-reads-pro-6214-service-runs.tsv 86570d1326baad6d866b4066b6bf65218434b201fb1f2c05ebc6e3c79f9c5a98 |
| plan14-root-reads-wake-latency-3.tsv 86570d1326baad6d866b4066b6bf65218434b201fb1f2c05ebc6e3c79f9c5a98 |
| -plan14-root-reads-plans-3b-never-sleep.tsv 3e22132e02fdb3acc157f95f261d43b16b8ebbcd0f7636739ed50695a02b84b6 |
| +plan14-root-reads-plans-3b-fleet-owner.tsv 74ff021155e1f12ea81cac07f183b528bf5748d6fc06ae7903ed0f91842458f2 |
| +plan14-root-reads-plans-3b-never-sleep.tsv d62eabd30c03e73ec1348d8e64912678ff8523ac07252c9283ef9f445ca4e944 |
diff --git a/internal/provision/testdata/plan14-root-reads-plans-3b-fleet-owner.tsv b/internal/provision/testdata/plan14-root-reads-plans-3b-fleet-owner.tsv| new file mode 100644 |
| index 000000000..c2bcaaada |
| --- /dev/null |
| +++ b/internal/provision/testdata/plan14-root-reads-plans-3b-fleet-owner.tsv |
| @@ -0,0 +1,3 @@ |
| +# Census overlay plans-3b-fleet-owner, written by hand: a new read (the owner of a fleet), a decision for the census owner. census-ruling-plans-3b-fleet-owner; the card is docs/superpowers/notes/census-proposal-plans-3b-never-sleep.md. |
| +@legacy c041bcf8d643e5a7178342ee7834942f7aae28ff583c4e40094269b9e0fd5a28 internal/console/plancaps.go ownsFleet legacy accepted-legacy accepted-legacy;rev3=census-ruling-plans-3b-fleet-owner;basis=the console (cmd/console serve under console.service, user carlos-console): reads the owning account of the fleet an app or sidecar record is placed on, to decide whether a plan's never-sleep and schedule refusals apply; an owned fleet only lifts a console-side refusal, an unknown or foreign fleet keeps it, and nothing is granted on any box (Astra plans-3b finding 5) registry:Account@256,whole:Fleet@244 1ce3c80b1ed62a205c1ca8ca0d9482421165a1b621d1a5e573d155d235bd6ff5 |
| +internal/console/plancaps.go ownsFleet legacy accepted-legacy accepted-legacy;rev3=census-ruling-plans-3b-fleet-owner;basis=the console (cmd/console serve under console.service, user carlos-console): reads the owning account of the fleet an app or sidecar record is placed on, to decide whether a plan's never-sleep and schedule refusals apply; an owned fleet only lifts a console-side refusal, an unknown or foreign fleet keeps it, and nothing is granted on any box (Astra plans-3b finding 5) registry:Account@256,whole:Fleet@244 1ce3c80b1ed62a205c1ca8ca0d9482421165a1b621d1a5e573d155d235bd6ff5 |
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 13e5488ab..64c1ea69d 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,6 @@ |
| # 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. |
| -@legacy 880722e2fa8808e3cdfe94f9c1cdc450720642aaceec805e12feee9a55593e62 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 the account's own fleet, or any app with plans off, ships as before. No new read. 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 the account's own fleet, or any app with plans off, ships as before. No new read. file:ReadAll@11378 c431049c96beaa5858c1aaf58e23fa2f32c40c2319297cac2b42527a3b879359 |
| -@coverage internal/console d9f98933506fb069e345fd23baaeb182e04091dede0b7a45b8239324b1af594f |
| -@coverage internal/schedule e126a4d723fb88cebbba0899167ff1c82be592d240da5f0ce94d1d600555ed26 |
| +@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 |
diff --git a/internal/schedule/mingap.go b/internal/schedule/mingap.go| index 38b1476e8..21ff6d985 100644 |
| --- a/internal/schedule/mingap.go |
| +++ b/internal/schedule/mingap.go |
| @@ -5,19 +5,16 @@ import ( |
| "time" |
| ) |
| -// shortestWindow is how far ShortestInterval looks for a cron line's |
| -// firings: long enough to see two of a monthly line's. |
| -const shortestWindow = 62 * 24 * time.Hour |
| - |
| -// shortestFrom is a fixed starting instant (a Monday at midnight UTC), so |
| -// the answer never depends on when it is asked. |
| -var shortestFrom = time.Date(2026, 1, 5, 0, 0, 0, 0, time.UTC) |
| - |
| // ShortestInterval is the least time between two runs of s: its every: |
| -// duration, or for a cron line the smallest gap between consecutive firings |
| -// across shortestWindow. A line that fires at most once in the window |
| -// answers the window. Plans 3b uses it to hold schedules to a plan's |
| -// minimum. |
| +// 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. |
| +// |
| +// 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. |
| func (s Schedule) ShortestInterval() (time.Duration, error) { |
| sp, err := s.Spec() |
| if err != nil { |
| @@ -27,23 +24,33 @@ func (s Schedule) ShortestInterval() (time.Duration, error) { |
| case sp.Every > 0: |
| return sp.Every, nil |
| case sp.Cron != nil: |
| - shortest := shortestWindow |
| - end := shortestFrom.Add(shortestWindow) |
| - prev := sp.Cron.Next(shortestFrom) |
| - for !prev.IsZero() && prev.Before(end) { |
| - next := sp.Cron.Next(prev) |
| - if next.IsZero() || !next.Before(end) { |
| - break |
| - } |
| - if gap := next.Sub(prev); gap < shortest { |
| - shortest = gap |
| - if shortest <= MinEvery { |
| - break |
| - } |
| + return sp.Cron.shortestGap(), nil |
| + } |
| + 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. |
| +func (c *CronExpr) shortestGap() time.Duration { |
| + var times []int // minutes of the day, ascending |
| + for h := 0; h < 24; h++ { |
| + if c.hour&(1<<uint(h)) == 0 { |
| + continue |
| + } |
| + for m := 0; m < 60; m++ { |
| + if c.min&(1<<uint(m)) != 0 { |
| + times = append(times, h*60+m) |
| } |
| - prev = next |
| } |
| - return shortest, nil |
| } |
| - return 0, errors.New("schedule has no interval") |
| + if len(times) == 0 { |
| + return 24 * time.Hour |
| + } |
| + shortest := 24*60 - times[len(times)-1] + times[0] // last today to first tomorrow |
| + for i := 1; i < len(times); i++ { |
| + if gap := times[i] - times[i-1]; gap < shortest { |
| + shortest = gap |
| + } |
| + } |
| + return time.Duration(shortest) * time.Minute |
| } |
diff --git a/internal/schedule/mingap_test.go b/internal/schedule/mingap_test.go| index 20fbf3b14..3685e1f20 100644 |
| --- a/internal/schedule/mingap_test.go |
| +++ b/internal/schedule/mingap_test.go |
| @@ -14,7 +14,15 @@ 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}, |
| - {Schedule{Name: "a", Cron: "0 9 * * 1", Path: "/x"}, 7 * 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}, |
| + {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}, |
| + {Schedule{Name: "a", Cron: "55 23,0 * * *", Path: "/x"}, 60 * time.Minute}, |
| + {Schedule{Name: "a", Cron: "5,55 0,23 * * *", Path: "/x"}, 10 * time.Minute}, |
| {Schedule{Name: "a", Every: "10m", Path: "/x"}, 10 * time.Minute}, |
| } { |
| got, err := c.sc.ShortestInterval() |