diff --git a/internal/crq/init.go b/internal/crq/init.go index 99420c7..ecc7a56 100644 --- a/internal/crq/init.go +++ b/internal/crq/init.go @@ -43,7 +43,7 @@ func Init(ctx context.Context, cfg Config, gh *ghapi.GitHub, store StateStore) ( if err != nil { return InitResult{}, err } - issue, err := gh.CreateIssue(ctx, cfg.GateRepo, renderTitle(state), body) + issue, err := gh.CreateIssue(ctx, cfg.GateRepo, renderTitle(state, cfg), body) if err != nil { return InitResult{}, err } diff --git a/internal/crq/state.go b/internal/crq/state.go index fd6843c..be37562 100644 --- a/internal/crq/state.go +++ b/internal/crq/state.go @@ -49,6 +49,7 @@ func (c Config) storeConfig() StoreConfig { Timezone: c.Timezone, Scope: c.Scope, CoReviewers: c.coReviewerSummary(), + MinInterval: c.MinInterval, } } @@ -96,7 +97,9 @@ func DefaultState(cfg Config) State { func renderDashboard(st State, cfg Config) string { return crqstate.RenderDashboard(st, cfg.storeConfig()) } -func renderTitle(st State) string { return crqstate.RenderTitle(st) } +func renderTitle(st State, cfg Config) string { + return crqstate.RenderTitle(st, cfg.storeConfig()) +} func issueBody(st State, cfg Config) (string, error) { return crqstate.IssueBody(st, cfg.storeConfig()) } diff --git a/internal/state/dashboard.go b/internal/state/dashboard.go index f02b187..6509b1d 100644 --- a/internal/state/dashboard.go +++ b/internal/state/dashboard.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "sort" + "strconv" "strings" "time" ) @@ -16,6 +17,28 @@ const ( crqProjectURL = "https://github.com/kristofferR/coderabbit-queue" ) +// firstStranded finds an in-flight round that reserved the slot but no longer +// holds it: it cannot receive feedback (no command was posted) and Pump cannot +// advance it (no slot), so it needs naming wherever it sits in the list. +// +// Holding the slot is the whole distinction. Every normal fire passes through +// PhaseReserved WITH a valid slot while the command is being posted, so testing +// the phase alone reported the happy path as stuck — and loudly, since a stranded +// round now outranks every other state. +func firstStranded(st State, inFlight []Round) *Round { + slot := st.SlotRound() + for i := range inFlight { + if inFlight[i].Phase != PhaseReserved { + continue + } + if slot != nil && slot.Repo == inFlight[i].Repo && slot.PR == inFlight[i].PR { + continue // mid-fire, not stranded + } + return &inFlight[i] + } + return nil +} + func joinScope(scope []string) string { return strings.Join(scope, ",") } @@ -44,13 +67,20 @@ func minutesUntil(t time.Time, now time.Time) int { return mins } -// reviewingRounds returns the rounds that fired and are still open (fired or -// reviewing), ordered by fire time — the v3 equivalent of the "awaiting -// feedback" set (a fired round whose slot may already be released). -func reviewingRounds(st State) []Round { +// inFlightRounds returns every round crq has already acted on and is still +// carrying: reserved (slot held, command not yet posted), fired, or reviewing — +// ordered by fire time. +// +// Together with State.Queue (queued + awaiting_retry) this PARTITIONS Active() +// by phase, which is what makes "every active round is on the dashboard" true +// by construction. The previous single "Feedback wait" row showed reviewing[0] +// and silently dropped every round behind it, along with any reserved round +// whose fire slot Normalize had cleared. +func inFlightRounds(st State) []Round { var out []Round for _, r := range st.Rounds { - if r.Phase == PhaseFired || r.Phase == PhaseReviewing { + switch r.Phase { + case PhaseReserved, PhaseFired, PhaseReviewing: out = append(out, r) } } @@ -60,10 +90,40 @@ func reviewingRounds(st State) []Round { return out } +// firedAtOf is when a round entered its in-flight life, for ordering the in-flight +// table. A reserved round has no FiredAt — the command is not posted yet — so +// falling straight through to EnqueuedAt sorted a long-queued PR that was reserved +// seconds ago ahead of reviews fired much earlier, contradicting the table's own +// ordering. +// firedTimeOf is what the in-flight table should print in its "fired" column: the +// time THIS attempt posted its command, or nothing when it has not. +// +// A retry deliberately keeps the previous attempt's FiredAt as history and Reserve +// does not clear it, so a reserved round would otherwise display an earlier +// attempt's timestamp as though the current command had gone out — and if the post +// hangs or the process dies, that misleading value is what stays on the dashboard +// beside the stranded reservation. +func firedTimeOf(r Round) *time.Time { + if r.Phase == PhaseReserved { + return nil + } + return r.FiredAt +} + func firedAtOf(r Round) time.Time { + // A reserved round is BETWEEN attempts: a retry deliberately preserves the + // previous FiredAt as history, so reading it here ordered the round by a fire + // that may be hours old — and printed that stale time in a column describing a + // command this attempt has not posted yet. + if r.Phase == PhaseReserved && r.ReservedAt != nil { + return *r.ReservedAt + } if r.FiredAt != nil { return *r.FiredAt } + if r.ReservedAt != nil { + return *r.ReservedAt + } return r.EnqueuedAt } @@ -95,8 +155,9 @@ func requestedRounds(st State) []Round { } // coBotMarks renders a round's co-reviewer trigger bookkeeping for the -// feedback-wait row: ✓ = trigger posted/adopted, ⏳ = post claimed but not -// yet recorded. Empty (byte-identical row) when the round tracks no co-bots. +// in-flight table's triggers column: ✓ = trigger posted/adopted, ⏳ = post +// claimed but not yet recorded. Empty when the round tracks no co-bots; the +// caller supplies the surrounding decoration. func coBotMarks(r Round) string { if len(r.CoBots) == 0 { return "" @@ -119,7 +180,15 @@ func coBotMarks(r Round) string { if len(parts) == 0 { return "" } - return " · triggers: " + strings.Join(parts, ", ") + return strings.Join(parts, ", ") +} + +// dash renders an empty cell as an em dash so a table row never collapses. +func dash(s string) string { + if s == "" { + return "—" + } + return s } // RenderDashboard renders the human-facing dashboard for v3 state: rounds by @@ -127,23 +196,36 @@ func coBotMarks(r Round) string { func RenderDashboard(st State, cfg StoreConfig) string { loc := dashboardLoc(cfg) now := time.Now().UTC() - queue := st.QueuedRounds(now) - reviewing := reviewingRounds(st) + queue := st.Queue(now, cfg.MinInterval) + inFlight := inFlightRounds(st) slot := st.SlotRound() blocked := st.Account.BlockedUntil != nil && st.Account.BlockedUntil.After(now) var b strings.Builder fmt.Fprintf(&b, "# 🐰 crq — CodeRabbit review queue\n\n") + stranded := firstStranded(st, inFlight) switch { + case stranded != nil: + // Reported before the transient states: a quota window or another PR's + // review clears on its own, but a reserved round with no slot behind it + // cannot be advanced by Pump at all, so hiding it behind them leaves + // permanently stuck work looking ordinary. + fmt.Fprintf(&b, "### 🟠 Stranded reservation on %s#%d — no fire slot backs it\n\n", stranded.Repo, stranded.PR) case blocked: fmt.Fprintf(&b, "### 🔴 Blocked — next review in ~%dm\n\n", minutesUntil(*st.Account.BlockedUntil, now)) case slot != nil: fmt.Fprintf(&b, "### 🟡 Reviewing %s#%d\n\n", slot.Repo, slot.PR) - case len(reviewing) > 0: - fmt.Fprintf(&b, "### 🟡 Awaiting feedback for %s#%d\n\n", reviewing[0].Repo, reviewing[0].PR) + case len(inFlight) > 0: + fmt.Fprintf(&b, "### 🟡 Awaiting feedback for %s#%d\n\n", inFlight[0].Repo, inFlight[0].PR) case len(queue) > 0: - fmt.Fprintf(&b, "### 🟠 %d queued\n\n", len(queue)) + // Nothing ready yet is still queued work, never idle — say when the front + // of the queue opens instead of leaving the reader to guess. + if next := queue[0].ReadyAt; !next.IsZero() { + fmt.Fprintf(&b, "### 🟠 %d queued — next at %s\n\n", len(queue), fmtStamp(&next, loc)) + } else { + fmt.Fprintf(&b, "### 🟠 %d queued\n\n", len(queue)) + } default: fmt.Fprintf(&b, "### 🟢 Idle\n\n") } @@ -172,31 +254,56 @@ func RenderDashboard(st State, cfg StoreConfig) string { fmt.Fprintf(&b, "| **Co-reviewers** | %s |\n", cfg.CoReviewers) } fmt.Fprintf(&b, "| **Last review fired** | %s |\n", fmtStamp(st.LastFired, loc)) - if slot != nil { - fmt.Fprintf(&b, "| **In flight** | [%s#%d](https://github.com/%s/pull/%d) · fired %s · `%s` |\n", - slot.Repo, slot.PR, slot.Repo, slot.PR, fmtStamp(slot.FiredAt, loc), slot.ByHost) - } else { - fmt.Fprintf(&b, "| **In flight** | — |\n") - } - if len(reviewing) > 0 { - r := reviewing[0] - fmt.Fprintf(&b, "| **Feedback wait** | [%s#%d](https://github.com/%s/pull/%d) · `%s` · deadline %s%s |\n", - r.Repo, r.PR, r.Repo, r.PR, r.Head, fmtStamp(r.WaitDeadline, loc), coBotMarks(r)) - } else { - fmt.Fprintf(&b, "| **Feedback wait** | — |\n") - } if st.Warn != "" { fmt.Fprintf(&b, "\n> ⚠️ %s\n", st.Warn) } + fmt.Fprintf(&b, "\n## 🔬 In flight — %d\n\n", len(inFlight)) + if len(inFlight) == 0 { + fmt.Fprintf(&b, "_None._\n") + } else { + fmt.Fprintf(&b, "| PR | commit | phase | fired | deadline | triggers | host |\n|---|---|---|---|---|---|---|\n") + for _, r := range inFlight { + fmt.Fprintf(&b, "| [%s#%d](https://github.com/%s/pull/%d) | `%s` | %s | %s | %s | %s | `%s` |\n", + r.Repo, r.PR, r.Repo, r.PR, r.Head, r.Phase, + fmtStamp(firedTimeOf(r), loc), fmtStamp(r.WaitDeadline, loc), dash(coBotMarks(r)), r.ByHost) + } + } + fmt.Fprintf(&b, "\n## ⏳ Queue — %d waiting\n\n", len(queue)) if len(queue) == 0 { fmt.Fprintf(&b, "_Nothing queued._\n") } else { - fmt.Fprintf(&b, "| # | PR | enqueued | host |\n|--:|---|---|---|\n") - for i, r := range queue { - fmt.Fprintf(&b, "| %d | [%s#%d](https://github.com/%s/pull/%d) | %s | `%s` |\n", - i+1, r.Repo, r.PR, r.Repo, r.PR, fmtStamp(&r.EnqueuedAt, loc), r.ByHost) + fmt.Fprintf(&b, "| # | PR | commit | ready | why | attempts | enqueued | host |\n|--:|---|---|---|---|--:|---|---|\n") + for i, e := range queue { + // Absolute stamps only: a relative "in 11m" would re-hash the dashboard + // on every render, and fmtStamp already honours CRQ_TZ. + // A zero ReadyAt means "now" only when nothing is holding the round. + // With a gate whose end is unknowable — a slot held elsewhere, or a + // round queued behind another — printing "now" contradicts the very + // column next to it. + ready := "now" + switch { + case !e.ReadyAt.IsZero(): + at := e.ReadyAt + ready = fmtStamp(&at, loc) + case e.Why != "": + ready = "unknown" + } + // Only the front has a knowable position. What fires after it depends on + // when its slot releases — the bot acknowledging, or the in-flight + // timeout — so any number past the first is a guess. List them; do not + // rank them. + // A position is claimed only for a round that can fire now (see Queue): + // anything else depends on when a pump runs and which windows have + // opened by then. + position := "—" + if i == 0 && e.ReadyAt.IsZero() && e.Why == "" { + position = strconv.Itoa(1) + } + fmt.Fprintf(&b, "| %s | [%s#%d](https://github.com/%s/pull/%d) | `%s` | %s | %s | %d | %s | `%s` |\n", + position, e.Repo, e.PR, e.Repo, e.PR, e.Head, ready, dash(e.Why), + e.Attempts, fmtStamp(&e.EnqueuedAt, loc), e.ByHost) } } @@ -218,15 +325,22 @@ func RenderDashboard(st State, cfg StoreConfig) string { return b.String() } -func RenderTitle(st State) string { +// RenderTitle summarizes the state for the dashboard issue title. The queue +// count is the WHOLE queue, cooling-down rounds included: a state whose only +// work is not yet fire-eligible is queued, never idle. +func RenderTitle(st State, cfg StoreConfig) string { now := time.Now().UTC() - queue := len(st.QueuedRounds(now)) + queue := len(st.Queue(now, cfg.MinInterval)) switch { + case firstStranded(st, inFlightRounds(st)) != nil: + // Same precedence as the body: permanently stuck work outranks states that + // clear by themselves. + return fmt.Sprintf("🐰 crq — stranded #%d · queue %d", firstStranded(st, inFlightRounds(st)).PR, queue) case st.Account.BlockedUntil != nil && st.Account.BlockedUntil.After(now): return fmt.Sprintf("🐰 crq — blocked · queue %d", queue) case st.SlotRound() != nil: return fmt.Sprintf("🐰 crq — reviewing #%d · queue %d", st.SlotRound().PR, queue) - case len(reviewingRounds(st)) > 0: + case len(inFlightRounds(st)) > 0: return fmt.Sprintf("🐰 crq — awaiting feedback · queue %d", queue) case queue > 0: return fmt.Sprintf("🐰 crq — %d queued", queue) diff --git a/internal/state/dashboard_test.go b/internal/state/dashboard_test.go new file mode 100644 index 0000000..4f309d7 --- /dev/null +++ b/internal/state/dashboard_test.go @@ -0,0 +1,847 @@ +package state + +import ( + "strings" + "testing" + "time" +) + +const nothingQueued = "_Nothing queued._" + +func ptime(t time.Time) *time.Time { return &t } + +// coolingRound is the shape that used to vanish from the dashboard: a waiting +// round whose own RetryAt has not passed yet. +func coolingRound(repo string, pr int, seq int64, now time.Time, retryIn time.Duration) Round { + return Round{ + Repo: repo, + PR: pr, + Head: "a9c688a1c", + Seq: seq, + Phase: PhaseAwaitingRetry, + Attempts: 1, + EnqueuedAt: now.Add(-10 * time.Minute), + FiredAt: ptime(now.Add(-9 * time.Minute)), + RetryAt: ptime(now.Add(retryIn)), + ByHost: "cachyos", + } +} + +func queuedRound(repo string, pr int, seq int64, now time.Time) Round { + return Round{ + Repo: repo, + PR: pr, + Head: "beefbeef1", + Seq: seq, + Phase: PhaseQueued, + EnqueuedAt: now.Add(-time.Minute), + ByHost: "cachyos", + } +} + +func stateWith(rounds ...Round) State { + st := New() + for _, r := range rounds { + st.PutRound(r) + } + return st +} + +// queueSection returns just the "## ⏳ Queue" section of a rendered dashboard, +// so a match cannot come from the in-flight table or the requested history. +func queueSection(t *testing.T, out string) string { + t.Helper() + _, after, ok := strings.Cut(out, "## ⏳ Queue") + if !ok { + t.Fatalf("no queue section:\n%s", out) + } + before, _, _ := strings.Cut(after, "\n## ") + return before +} + +// A queue whose rounds are all cooling down must not render as an empty queue: +// that is the bug this design exists to prevent (a reader concluded their +// enqueue had been dropped). +func TestRenderDashboardCoolingDownOnly(t *testing.T) { + now := time.Now().UTC() + a := coolingRound("kristofferr/ha-adjustable-bed", 480, 1, now, 11*time.Minute) + b := coolingRound("kristofferr/ha-adjustable-bed", 481, 2, now, 12*time.Minute) + st := stateWith(a, b) + + out := RenderDashboard(st, StoreConfig{}) + if strings.Contains(out, nothingQueued) { + t.Fatalf("cooling-down rounds rendered as an empty queue:\n%s", out) + } + if !strings.Contains(out, "## ⏳ Queue — 2 waiting") { + t.Errorf("queue heading does not count cooling-down rounds:\n%s", out) + } + q := queueSection(t, out) + for _, want := range []string{ + "kristofferr/ha-adjustable-bed#480", + "https://github.com/kristofferr/ha-adjustable-bed/pull/480", + "`a9c688a1c`", + fmtStamp(a.RetryAt, time.UTC), // absolute ready time, not a relative "in 11m" + WaitCoolingDown, + "`cachyos`", + } { + if !strings.Contains(q, want) { + t.Errorf("queue section missing %q:\n%s", want, q) + } + } + // The header must say when the front of the queue opens. + if !strings.Contains(out, "2 queued — next at "+fmtStamp(a.RetryAt, time.UTC)) { + t.Errorf("header does not report the next ready time:\n%s", out) + } + + if got, want := RenderTitle(st, StoreConfig{}), "🐰 crq — 2 queued"; got != want { + t.Errorf("RenderTitle = %q, want %q", got, want) + } +} + +// Guard against over-correcting: a genuinely empty state keeps its empty-state +// text and its idle title. +func TestRenderDashboardEmpty(t *testing.T) { + st := New() + + out := RenderDashboard(st, StoreConfig{}) + if !strings.Contains(out, nothingQueued) { + t.Errorf("empty state lost its empty-state text:\n%s", out) + } + if !strings.Contains(out, "## 🔬 In flight — 0\n\n_None._") { + t.Errorf("empty state lost its empty in-flight section:\n%s", out) + } + if got, want := RenderTitle(st, StoreConfig{}), "🐰 crq — idle"; got != want { + t.Errorf("RenderTitle = %q, want %q", got, want) + } +} + +// The queue is ordered the way rounds will actually fire: ready rounds first +// (by Seq), then by when each window opens — regardless of Seq. +func TestQueueOrdersByReadyThenSeq(t *testing.T) { + now := time.Now().UTC() + st := stateWith( + coolingRound("kristofferr/a", 1, 1, now, 20*time.Minute), // lowest Seq, latest window + coolingRound("kristofferr/b", 2, 2, now, 5*time.Minute), + queuedRound("kristofferr/c", 3, 3, now), // ready now, highest Seq + coolingRound("kristofferr/d", 4, 4, now, -2*time.Minute), // window already open + ) + + q := st.Queue(now, 0) + var got []int + for _, e := range q { + got = append(got, e.PR) + } + // Ready now: c (Seq 3) and d (Seq 4, elapsed RetryAt) — Seq order among them. + // Then b (+5m), then a (+20m). + want := []int{3, 4, 2, 1} + if len(got) != len(want) { + t.Fatalf("Queue returned %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("Queue order = %v, want %v", got, want) + } + } + // That d (an ELAPSED RetryAt) sorts among the ready rounds rather than after + // the cooling ones is the point: an expired window is not a wait. + // + // Only the front reports its own gate; everything behind it reports being + // behind, because when it starts depends on when the front finishes. + if q[0].Why != "" || !q[0].ReadyAt.IsZero() { + t.Errorf("the front is ready now: %+v", q[0]) + } + for i := 1; i < len(q); i++ { + if !q[i].ReadyAt.IsZero() { + t.Errorf("q[%d] must carry no time, got %v", i, q[i].ReadyAt) + } + } + // The ready follower has nothing of its own holding it, so it is purely behind. + if q[1].Why != WaitBehind { + t.Errorf("q[1].Why = %q, want %q", q[1].Why, WaitBehind) + } +} + +// The account-wide quota block gates firing too, so a round whose own RetryAt +// falls inside a longer block must display the block's end, not its own — the +// dashboard must not promise a time DecideFire will refuse. +func TestQueueAccountBlockDominatesRetryAt(t *testing.T) { + now := time.Now().UTC() + r := coolingRound("kristofferr/ha-adjustable-bed", 480, 1, now, 11*time.Minute) + st := stateWith(r) + blockedUntil := now.Add(44 * time.Minute) + st.Account.BlockedUntil = &blockedUntil + + q := st.Queue(now, 0) + if len(q) != 1 { + t.Fatalf("Queue returned %d entries, want 1", len(q)) + } + if !q[0].ReadyAt.Equal(blockedUntil.UTC()) { + t.Errorf("ReadyAt = %s, want the account block end %s", q[0].ReadyAt, blockedUntil.UTC()) + } + if q[0].Why != WaitAccountBlocked { + t.Errorf("Why = %q, want %q", q[0].Why, WaitAccountBlocked) + } + + out := queueSection(t, RenderDashboard(st, StoreConfig{})) + if !strings.Contains(out, fmtStamp(&blockedUntil, time.UTC)) { + t.Errorf("queue does not show the account-block end:\n%s", out) + } + if strings.Contains(out, fmtStamp(r.RetryAt, time.UTC)) { + t.Errorf("queue shows the round's own RetryAt, which the fire gate would refuse:\n%s", out) + } +} + +// A round waiting only because another PR holds the fire slot is ready now; the +// slot is what it waits on. +func TestQueueSlotBusy(t *testing.T) { + now := time.Now().UTC() + holder := Round{ + Repo: "kristofferr/a", PR: 1, Head: "aaaaaaaa1", Seq: 1, + Phase: PhaseReserved, EnqueuedAt: now.Add(-2 * time.Minute), + ReservedAt: ptime(now.Add(-time.Minute)), Token: "tok", ByHost: "cachyos", + } + st := stateWith(holder, queuedRound("kristofferr/b", 2, 2, now)) + st.FireSlot = &FireSlot{Key: Key(holder.Repo, holder.PR), Token: "tok"} + + q := st.Queue(now, 0) + if len(q) != 1 || q[0].PR != 2 { + t.Fatalf("Queue = %+v, want only the queued round", q) + } + if q[0].Why != WaitSlotBusy { + t.Errorf("Why = %q, want %q", q[0].Why, WaitSlotBusy) + } +} + +// In flight (reserved/fired/reviewing) and Queue (queued/awaiting_retry) +// partition Active(), so every active round is on the dashboard exactly once. +// The old single "Feedback wait" row hid every reviewing round past the first +// and any reserved round whose fire slot had been cleared. +func TestRenderDashboardPartitionsActiveRounds(t *testing.T) { + now := time.Now().UTC() + fired := Round{ + Repo: "kristofferr/a", PR: 1, Head: "aaaaaaaa1", Seq: 1, + Phase: PhaseFired, EnqueuedAt: now.Add(-5 * time.Minute), + FiredAt: ptime(now.Add(-4 * time.Minute)), ByHost: "cachyos", + } + reviewing := Round{ + Repo: "kristofferr/b", PR: 2, Head: "bbbbbbbb2", Seq: 2, + Phase: PhaseReviewing, EnqueuedAt: now.Add(-3 * time.Minute), + FiredAt: ptime(now.Add(-2 * time.Minute)), ByHost: "cachyos", + } + reserved := Round{ // slot-less reserved round (FireSlot cleared by Normalize) + Repo: "kristofferr/c", PR: 3, Head: "cccccccc3", Seq: 3, + Phase: PhaseReserved, EnqueuedAt: now.Add(-time.Minute), + ReservedAt: ptime(now), ByHost: "cachyos", + } + completed := Round{ // not active: the reviewed-head dedup marker + Repo: "kristofferr/e", PR: 5, Head: "eeeeeeee5", Seq: 5, + Phase: PhaseCompleted, EnqueuedAt: now.Add(-time.Hour), ByHost: "cachyos", + } + st := stateWith(fired, reviewing, reserved, completed, + coolingRound("kristofferr/d", 4, 4, now, 30*time.Minute), + queuedRound("kristofferr/f", 6, 6, now)) + + out := RenderDashboard(st, StoreConfig{}) + // The "Recently requested" history lists fired rounds regardless of phase, so + // it must not count as accounting for a live round. + live, _, _ := strings.Cut(out, "## 📨 Recently requested") + inFlight, queue, ok := strings.Cut(live, "## ⏳ Queue") + if !ok { + t.Fatalf("no queue section:\n%s", out) + } + for _, r := range st.Rounds { + key := Key(r.Repo, r.PR) + inA, inB := strings.Contains(inFlight, key), strings.Contains(queue, key) + if !r.Active() { + if inA || inB { + t.Errorf("inactive round %s rendered as live work", key) + } + continue + } + if inA == inB { + t.Errorf("active round %s is in %d sections, want exactly 1:\n%s", key, btoi(inA)+btoi(inB), live) + } + } + if !strings.Contains(out, "## 🔬 In flight — 3") { + t.Errorf("want 3 in-flight rounds:\n%s", out) + } + if !strings.Contains(out, "## ⏳ Queue — 2 waiting") { + t.Errorf("want 2 waiting rounds:\n%s", out) + } +} + +func btoi(b bool) int { + if b { + return 1 + } + return 0 +} + +// The in-flight table carries each round's co-reviewer trigger marks. +func TestRenderDashboardInFlightTriggers(t *testing.T) { + now := time.Now().UTC() + r := Round{ + Repo: "kristofferr/a", PR: 1, Head: "aaaaaaaa1", Seq: 1, + Phase: PhaseReviewing, EnqueuedAt: now.Add(-3 * time.Minute), + FiredAt: ptime(now.Add(-2 * time.Minute)), WaitDeadline: ptime(now.Add(18 * time.Minute)), + ByHost: "cachyos", + } + r.SetCoCommand("chatgpt-codex-connector", 42, now.Add(-2*time.Minute)) + st := stateWith(r) + + out := RenderDashboard(st, StoreConfig{}) + if !strings.Contains(out, "chatgpt-codex-connector ✓") { + t.Errorf("in-flight row missing trigger marks:\n%s", out) + } + if !strings.Contains(out, fmtStamp(r.WaitDeadline, time.UTC)) { + t.Errorf("in-flight row missing the wait deadline:\n%s", out) + } +} + +// The ready column honours CRQ_TZ via the shared fmtStamp helper. +func TestRenderDashboardQueueHonoursTimezone(t *testing.T) { + now := time.Now().UTC() + r := coolingRound("kristofferr/ha-adjustable-bed", 480, 1, now, 11*time.Minute) + st := stateWith(r) + + loc, err := time.LoadLocation("Europe/Oslo") + if err != nil { + t.Skipf("tzdata unavailable: %v", err) + } + out := RenderDashboard(st, StoreConfig{Timezone: "Europe/Oslo"}) + if !strings.Contains(out, fmtStamp(r.RetryAt, loc)) { + t.Errorf("ready time not rendered in Europe/Oslo:\n%s", out) + } +} + +// The dashboard's "ready" column is a promise about when firing will accept a +// round, so every gate DecideFire applies has to be reflected. These three cases +// each rendered "ready: now" for a round the fire gate would have refused. + +// Pacing: after a fire, DecideFire rejects everything until LastFired + +// MinInterval. Excluding it as "sub-minute churn" was wrong twice over — it is an +// absolute boundary, so it does not churn between renders, and CRQ_MIN_INTERVAL +// can be configured far beyond 90s. +func TestQueueReflectsThePacingGate(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + fired := now.Add(-30 * time.Second) + st.LastFired = &fired + + q := st.Queue(now, 90*time.Second) + if len(q) != 1 { + t.Fatalf("Queue = %d entries, want 1", len(q)) + } + if want := fired.Add(90 * time.Second); !q[0].ReadyAt.Equal(want) { + t.Errorf("ReadyAt = %v, want the pacing boundary %s", q[0].ReadyAt, want) + } + if q[0].Why != WaitPacing { + t.Errorf("Why = %q, want %q", q[0].Why, WaitPacing) + } + + // Once the interval has elapsed it stops being a gate at all. + if q := st.Queue(now.Add(2*time.Minute), 90*time.Second); len(q) != 1 || !q[0].ReadyAt.IsZero() { + t.Errorf("an elapsed interval must not gate: %+v", q) + } +} + +// A slot held by another PR has no knowable release time. Leaving ReadyAt empty +// rendered as "now", which is the one thing that is definitely false. +func TestQueueDoesNotClaimSlotBlockedRoundsAreReadyNow(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now), queuedRound("kristofferr/b", 2, 2, now)) + st.FireSlot = &FireSlot{Key: "kristofferr/b#2", Token: "tok", Since: now.Add(-time.Minute)} + st.Rounds["kristofferr/b#2"] = func() Round { + r := st.Rounds["kristofferr/b#2"] + r.Phase = PhaseFired + r.Token = "tok" + return r + }() + + for _, e := range st.Queue(now, 0) { + if e.Why != WaitSlotBusy { + continue + } + if !e.ReadyAt.IsZero() { + t.Errorf("a slot-blocked entry must not carry a ready time, got %s", e.ReadyAt) + } + } +} + +// A reserved round has not posted its command, so no feedback can be coming — +// and with no FireSlot behind it, Pump cannot advance it either. Reporting a +// feedback wait sent the reader looking for a review nobody requested. +func TestRenderNamesAStrandedReservation(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase = PhaseReserved + reserved := now.Add(-time.Minute) + r.ReservedAt = &reserved + return r + }() + + got := RenderDashboard(st, StoreConfig{}) + if strings.Contains(got, "Awaiting feedback") { + t.Error("a reserved round with no fire slot is not a feedback wait") + } + if !strings.Contains(got, "Stranded reservation") { + t.Errorf("the stranded reservation must be named, got:\n%s", got) + } +} + +// Gates compose: the ready time is the latest of them, and the reason is +// whichever one binds. Taking the first that matched let a short cooldown hide a +// long account block, advertising a time firing would refuse for hours. +func TestQueueTakesTheLatestBindingGate(t *testing.T) { + now := time.Now().UTC() + st := stateWith(coolingRound("kristofferr/a", 1, 1, now, time.Minute)) + blocked := now.Add(2 * time.Hour) + st.Account.BlockedUntil = &blocked + + q := st.Queue(now, 0) + if len(q) != 1 { + t.Fatalf("Queue = %d entries, want 1", len(q)) + } + if !q[0].ReadyAt.Equal(blocked.UTC()) { + t.Errorf("ReadyAt = %v, want the dominating account block %s", q[0].ReadyAt, blocked.UTC()) + } + if q[0].Why != WaitAccountBlocked { + t.Errorf("Why = %q, want %q", q[0].Why, WaitAccountBlocked) + } + + // And the other way round: a pacing boundary beyond a short cooldown binds. + st2 := stateWith(coolingRound("kristofferr/b", 2, 2, now, time.Second)) + fired := now + st2.LastFired = &fired + q2 := st2.Queue(now, 10*time.Minute) + if len(q2) != 1 || q2[0].Why != WaitPacing || !q2[0].ReadyAt.Equal(fired.Add(10*time.Minute).UTC()) { + t.Errorf("pacing must bind past a shorter cooldown, got %+v", q2) + } +} + +// Pacing is serial, so a round behind the front cannot start until the front +// fires — but crq does not know when that is, and a time derived from render +// time would give an unchanged state a different DashboardSHA every minute (the +// churn that argued against surfacing pacing at all). So the queue says "behind +// an earlier round" rather than naming a time it cannot know. +func TestQueueNamesRoundsBehindTheFrontWithoutInventingATime(t *testing.T) { + now := time.Now().UTC() + st := stateWith( + queuedRound("kristofferr/a", 1, 1, now), + queuedRound("kristofferr/b", 2, 2, now), + queuedRound("kristofferr/c", 3, 3, now), + ) + + q := st.Queue(now, 90*time.Second) + if len(q) != 3 { + t.Fatalf("Queue = %d entries, want 3", len(q)) + } + if !q[0].ReadyAt.IsZero() || q[0].Why != "" { + t.Errorf("the front is ready now with no gate, got ready=%v why=%q", q[0].ReadyAt, q[0].Why) + } + for i := 1; i < len(q); i++ { + if !q[i].ReadyAt.IsZero() { + t.Errorf("entry %d invented a ready time %s; only absolute gates may be shown", i, q[i].ReadyAt) + } + if q[i].Why != WaitBehind { + t.Errorf("entry %d Why = %q, want %q", i, q[i].Why, WaitBehind) + } + } + + // Stability is the point: the same state rendered a minute later must produce + // the same queue, or every render becomes a dashboard write. + later := st.Queue(now.Add(time.Minute), 90*time.Second) + for i := range q { + if !later[i].ReadyAt.Equal(q[i].ReadyAt) || later[i].Why != q[i].Why { + t.Errorf("entry %d changed with render time: %v/%q -> %v/%q", + i, q[i].ReadyAt, q[i].Why, later[i].ReadyAt, later[i].Why) + } + } +} + +// Order past the front is not knowable, so the queue stops asserting it. What it +// must still get right is WHICH round is next and that the rest are not ranked: +// slot release comes from the bot acknowledging or from the in-flight timeout, so +// a round cooling now can overtake a ready one with a higher Seq. +func TestQueueRanksOnlyTheFront(t *testing.T) { + now := time.Now().UTC() + st := stateWith( + queuedRound("kristofferr/a", 1, 1, now), // ready, lowest Seq + coolingRound("kristofferr/b", 2, 2, now, 5*time.Minute), // cooling + queuedRound("kristofferr/c", 3, 3, now), // ready, higher Seq + ) + + q := st.Queue(now, 90*time.Second) + if q[0].PR != 1 { + t.Errorf("front = %d, want the lowest-Seq ready round", q[0].PR) + } + if !q[0].ReadyAt.IsZero() || q[0].Why != "" { + t.Errorf("the front is ready now, got ready=%v why=%q", q[0].ReadyAt, q[0].Why) + } + // Behind the front: no time, whatever the reason. A round with a gate of its + // own still reports it; one with nothing holding it is purely behind. + for i := 1; i < len(q); i++ { + if !q[i].ReadyAt.IsZero() { + t.Errorf("entry %d must be untimed, got %v", i, q[i].ReadyAt) + } + } + if q[1].Why != WaitBehind { + t.Errorf("the ready follower has nothing of its own holding it, got %q", q[1].Why) + } + if q[2].Why != WaitCoolingDown { + t.Errorf("the cooling round should still say so, got %q", q[2].Why) + } + + // And the table must not number them. + rendered := RenderDashboard(st, StoreConfig{}) + if strings.Contains(rendered, "| 2 | [") || strings.Contains(rendered, "| 3 | [") { + t.Errorf("only the front may carry a position:\n%s", rendered) + } +} + +// A slot held by another PR has no knowable release time, so it outranks every +// projected timestamp — otherwise a cooldown or pacing boundary is presented as a +// promise while the round cannot fire at all. +func TestQueueLetsSlotBusyDominateATimedGate(t *testing.T) { + now := time.Now().UTC() + st := stateWith(coolingRound("kristofferr/a", 1, 1, now, 5*time.Minute), queuedRound("kristofferr/b", 2, 2, now)) + st.FireSlot = &FireSlot{Key: "kristofferr/b#2", Token: "tok", Since: now.Add(-time.Minute)} + st.Rounds["kristofferr/b#2"] = func() Round { + r := st.Rounds["kristofferr/b#2"] + r.Phase, r.Token = PhaseFired, "tok" + return r + }() + + for _, e := range st.Queue(now, 0) { + if e.Why != WaitSlotBusy { + t.Errorf("entry %d: Why = %q, want %q while the slot is held", e.PR, e.Why, WaitSlotBusy) + } + if !e.ReadyAt.IsZero() { + t.Errorf("entry %d: ReadyAt = %s, want no promised time", e.PR, e.ReadyAt) + } + } +} + +// In flight is ordered by fire time, so an older reviewing round would hide a +// later stranded reservation — and that pairing is the normal shape. +func TestRenderFindsAStrandedReservationBehindAReviewingRound(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now), queuedRound("kristofferr/b", 2, 2, now)) + fired := now.Add(-10 * time.Minute) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase, r.FiredAt = PhaseReviewing, &fired + return r + }() + reserved := now.Add(-time.Minute) + st.Rounds["kristofferr/b#2"] = func() Round { + r := st.Rounds["kristofferr/b#2"] + r.Phase, r.ReservedAt = PhaseReserved, &reserved + return r + }() + + got := RenderDashboard(st, StoreConfig{}) + if !strings.Contains(got, "Stranded reservation on kristofferr/b#2") { + t.Errorf("the stranded reservation must be named even behind a reviewing round, got:\n%s", got) + } +} + +// The title is what a human sees first, so it must not contradict the body: a +// reserved round whose slot was cleared cannot be awaiting feedback. +func TestRenderTitleNamesAStrandedReservation(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + reserved := now.Add(-time.Minute) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase, r.ReservedAt = PhaseReserved, &reserved + return r + }() + + got := RenderTitle(st, StoreConfig{}) + if strings.Contains(got, "awaiting feedback") { + t.Errorf("title claims a feedback wait for a stranded reservation: %q", got) + } + if !strings.Contains(got, "stranded") { + t.Errorf("title = %q, want it to name the stranded reservation", got) + } +} + +// An account block that outlasts a cooldown makes both rounds eligible the moment +// it clears, and NextEligible then takes the lowest Seq. Simulating from render +// time instead put the ready higher-Seq round first. +func TestQueueOrdersFromWhenFiringResumes(t *testing.T) { + now := time.Now().UTC() + st := stateWith( + coolingRound("kristofferr/a", 1, 1, now, time.Hour), // lower Seq, cooling + queuedRound("kristofferr/b", 2, 2, now), // ready now, higher Seq + ) + blocked := now.Add(2 * time.Hour) + st.Account.BlockedUntil = &blocked + + q := st.Queue(now, 90*time.Second) + if len(q) != 2 { + t.Fatalf("Queue = %d entries, want 2", len(q)) + } + if q[0].PR != 1 { + t.Errorf("Queue order = [%d %d], want [1 2]: both are eligible when the block clears, so Seq decides", + q[0].PR, q[1].PR) + } +} + +// The model clears the ready time when a gate's end is unknowable; the renderer +// must not turn that zero back into "now" beside a "slot busy" reason. +func TestRenderShowsUnknownRatherThanNowForAnUnknowableGate(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now), queuedRound("kristofferr/b", 2, 2, now)) + st.FireSlot = &FireSlot{Key: "kristofferr/b#2", Token: "tok", Since: now.Add(-time.Minute)} + st.Rounds["kristofferr/b#2"] = func() Round { + r := st.Rounds["kristofferr/b#2"] + r.Phase, r.Token = PhaseFired, "tok" + return r + }() + + got := RenderDashboard(st, StoreConfig{}) + if strings.Contains(got, "now | slot busy") { + t.Errorf("a slot-blocked round must not read as ready now:\n%s", got) + } + if !strings.Contains(got, "unknown | slot busy") { + t.Errorf("want an unknown ready time beside the slot-busy reason, got:\n%s", got) + } +} + +// Sharing the front's boundary is not a later time: after the front fires, pacing +// pushes the next round out by another interval, so repeating that timestamp +// advertises a moment firing will refuse. +func TestQueueDoesNotRepeatTheFrontsBoundaryForFollowers(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now), queuedRound("kristofferr/b", 2, 2, now)) + blocked := now.Add(time.Hour) + st.Account.BlockedUntil = &blocked + + q := st.Queue(now, 90*time.Second) + if !q[0].ReadyAt.Equal(blocked.UTC()) { + t.Errorf("the front is gated by the account block, got %v", q[0].ReadyAt) + } + // The follower keeps its reason — the block is real — but loses the time, + // because when it actually starts depends on the round ahead of it. + if !q[1].ReadyAt.IsZero() { + t.Errorf("the follower must carry no time: ready=%v", q[1].ReadyAt) + } + if q[1].Why != WaitAccountBlocked { + t.Errorf("the follower should still report why it waits, got %q", q[1].Why) + } +} + +// A stranded reservation cannot be advanced by Pump at all, so a quota window or +// another PR's review — both of which clear by themselves — must not hide it. +func TestStrandedReservationOutranksTransientStates(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + reserved := now.Add(-time.Minute) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase, r.ReservedAt = PhaseReserved, &reserved + return r + }() + blocked := now.Add(time.Hour) + st.Account.BlockedUntil = &blocked + + if got := RenderDashboard(st, StoreConfig{}); !strings.Contains(got, "Stranded reservation") { + t.Errorf("an account block must not hide a stranded reservation:\n%s", got) + } + if got := RenderTitle(st, StoreConfig{}); !strings.Contains(got, "stranded") { + t.Errorf("title = %q, want the stranded reservation named", got) + } +} + +// Every normal fire passes through PhaseReserved while its slot is held and the +// command is posted. Reporting that as stranded turns the happy path into a loud +// false alarm, now that stranded outranks every other state. +func TestReservedRoundHoldingTheSlotIsNotStranded(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + reserved := now.Add(-time.Second) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase, r.ReservedAt, r.Token = PhaseReserved, &reserved, "tok" + return r + }() + st.FireSlot = &FireSlot{Key: "kristofferr/a#1", Token: "tok", Since: reserved} + + if got := RenderDashboard(st, StoreConfig{}); strings.Contains(got, "Stranded") { + t.Errorf("a mid-fire reservation must not read as stranded:\n%s", got) + } + if got := RenderTitle(st, StoreConfig{}); strings.Contains(got, "stranded") { + t.Errorf("title = %q, want no stranded claim mid-fire", got) + } + + // Once the slot is gone it really is stranded. + st.FireSlot = nil + if got := RenderDashboard(st, StoreConfig{}); !strings.Contains(got, "Stranded reservation") { + t.Errorf("a reservation with no slot must still be named:\n%s", got) + } +} + +// A reserved round has no FiredAt yet, so ordering the in-flight table by +// enqueue time put a PR reserved seconds ago ahead of reviews fired much earlier. +func TestInFlightOrdersAReservationByWhenItReserved(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/old", 1, 1, now), queuedRound("kristofferr/new", 2, 2, now)) + firedLongAgo := now.Add(-time.Hour) + st.Rounds["kristofferr/old#1"] = func() Round { + r := st.Rounds["kristofferr/old#1"] + r.Phase, r.FiredAt = PhaseReviewing, &firedLongAgo + return r + }() + enqueuedEvenEarlier := now.Add(-2 * time.Hour) + justReserved := now.Add(-time.Second) + st.Rounds["kristofferr/new#2"] = func() Round { + r := st.Rounds["kristofferr/new#2"] + r.Phase, r.EnqueuedAt, r.ReservedAt = PhaseReserved, enqueuedEvenEarlier, &justReserved + return r + }() + + got := inFlightRounds(st) + if len(got) != 2 || got[0].PR != 1 { + t.Errorf("in flight = %v, want the earlier-fired review first", []int{got[0].PR, got[1].PR}) + } +} + +// While another PR holds the slot, which round fires next depends on whose +// cooldown has elapsed when it releases — and that moment is unknown, so every +// order including Seq is a guess. The table must not number them. +func TestQueueTableDropsPositionsWhileTheSlotIsHeld(t *testing.T) { + now := time.Now().UTC() + st := stateWith(coolingRound("kristofferr/a", 1, 1, now, time.Hour), queuedRound("kristofferr/b", 2, 2, now)) + st.FireSlot = &FireSlot{Key: "kristofferr/c#3", Token: "tok", Since: now.Add(-time.Minute)} + st.Rounds["kristofferr/c#3"] = Round{ + Repo: "kristofferr/c", PR: 3, Head: "ccccccccc", Seq: 3, + Phase: PhaseFired, Token: "tok", EnqueuedAt: now.Add(-2 * time.Minute), FiredAt: &now, + } + + got := RenderDashboard(st, StoreConfig{}) + if strings.Contains(got, "| 1 | [kristofferr/a") || strings.Contains(got, "| 2 | [kristofferr/b") { + t.Errorf("positions must not be claimed while the slot is held:\n%s", got) + } +} + +// A round degraded to its co-reviewers spends no account quota, so DecideFire +// resolves it before the quota gate. Promising it cannot proceed until the window +// closes describes a wait the next observation can end immediately. +func TestQueueDoesNotBlockACoOnlyRoundOnTheAccountWindow(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.CoOnly = true + return r + }() + blocked := now.Add(2 * time.Hour) + st.Account.BlockedUntil = &blocked + + q := st.Queue(now, 0) + if len(q) != 1 { + t.Fatalf("Queue = %d entries, want 1", len(q)) + } + if q[0].Why == WaitAccountBlocked || !q[0].ReadyAt.IsZero() { + t.Errorf("a co-only round must not wait on the account window: ready=%v why=%q", q[0].ReadyAt, q[0].Why) + } +} + +// A co-only round waits for nothing the queue serializes: no account quota, no +// fire slot, and DecideFire resolves it before either gate. Exempting it +// gate-by-gate missed a different spot three rounds running — the account window, +// then the ordering, then the follower pass — so this asserts the whole exemption +// at once, with every queue-wide gate active simultaneously. +func TestCoOnlyRoundIsExemptFromEveryQueueGate(t *testing.T) { + now := time.Now().UTC() + st := stateWith( + queuedRound("kristofferr/metered", 1, 1, now), // lower Seq, ordinary round + queuedRound("kristofferr/free", 2, 2, now), // higher Seq, co-only + ) + st.Rounds["kristofferr/free#2"] = func() Round { + r := st.Rounds["kristofferr/free#2"] + r.CoOnly = true + return r + }() + // Every gate the queue can impose, at once. + blocked := now.Add(2 * time.Hour) + st.Account.BlockedUntil = &blocked + fired := now + st.LastFired = &fired + st.FireSlot = &FireSlot{Key: "kristofferr/other#9", Token: "tok", Since: now.Add(-time.Minute)} + st.Rounds["kristofferr/other#9"] = Round{ + Repo: "kristofferr/other", PR: 9, Head: "999999999", Seq: 9, + Phase: PhaseFired, Token: "tok", EnqueuedAt: now.Add(-2 * time.Minute), FiredAt: &fired, + } + + q := st.Queue(now, 90*time.Second) + if len(q) != 2 { + t.Fatalf("Queue = %d entries, want 2", len(q)) + } + free := q[0] + if free.PR != 2 { + t.Fatalf("the co-only round must lead: got #%d — actionable work cannot sit behind a slot it never needs", free.PR) + } + if !free.ReadyAt.IsZero() || free.Why != "" { + t.Errorf("the co-only round is ready now, got ready=%v why=%q", free.ReadyAt, free.Why) + } + // The ordinary round is still governed by all of it. + if q[1].Why != WaitSlotBusy { + t.Errorf("the metered round must report the held slot, got %q", q[1].Why) + } +} + +// A retry keeps the previous attempt's FiredAt as history, and Reserve does not +// clear it — so a reserved round would print an earlier attempt's timestamp as +// though the current command had gone out. If the post then hangs, that +// misleading value is what stays on the dashboard beside a stranded reservation. +func TestInFlightPrintsNoFireTimeForAReservation(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/a", 1, 1, now)) + previous := now.Add(-3 * time.Hour) + reserved := now.Add(-time.Second) + st.Rounds["kristofferr/a#1"] = func() Round { + r := st.Rounds["kristofferr/a#1"] + r.Phase, r.FiredAt, r.ReservedAt, r.Attempts = PhaseReserved, &previous, &reserved, 1 + return r + }() + + got := RenderDashboard(st, StoreConfig{}) + if strings.Contains(got, fmtStamp(&previous, time.UTC)) { + t.Errorf("the previous attempt's fire time must not be shown for a reservation:\n%s", got) + } +} + +// A held slot stops everything, free-running rounds included — not because they +// need the slot, but because Pump returns as soon as it sees a holder, so the +// quota-free path that would advance them is never reached. +func TestHeldSlotStopsFreeRunningRoundsToo(t *testing.T) { + now := time.Now().UTC() + st := stateWith(queuedRound("kristofferr/free", 1, 1, now)) + st.Rounds["kristofferr/free#1"] = func() Round { + r := st.Rounds["kristofferr/free#1"] + r.CoOnly = true + return r + }() + st.FireSlot = &FireSlot{Key: "kristofferr/other#9", Token: "tok", Since: now.Add(-time.Minute)} + fired := now.Add(-time.Minute) + st.Rounds["kristofferr/other#9"] = Round{ + Repo: "kristofferr/other", PR: 9, Head: "999999999", Seq: 9, + Phase: PhaseFired, Token: "tok", EnqueuedAt: now.Add(-2 * time.Minute), FiredAt: &fired, + } + + q := st.Queue(now, 0) + if len(q) != 1 { + t.Fatalf("Queue = %d entries, want 1", len(q)) + } + if q[0].Why != WaitSlotBusy { + t.Errorf("a co-only round must report the held slot, got %q — the daemon cannot advance it either", q[0].Why) + } + if strings.Contains(RenderDashboard(st, StoreConfig{}), "| 1 | [kristofferr/free") { + t.Error("nothing may be numbered while the slot is held") + } +} diff --git a/internal/state/state.go b/internal/state/state.go index 0e6887a..cc6400f 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -596,6 +596,185 @@ func (s *State) QueuedRounds(now time.Time) []Round { return out } +// Queue-entry wait reasons. A waiting round is held by exactly one of these +// (or by nothing, in which case it is next up). +const ( + WaitCoolingDown = "cooling down" // this round's own RetryAt has not passed + WaitAccountBlocked = "account blocked" // the CodeRabbit account quota window dominates + WaitSlotBusy = "slot busy" // another PR's review holds the fire slot + WaitPacing = "pacing" // LastFired + CRQ_MIN_INTERVAL has not passed + WaitBehind = "behind an earlier round" +) + +// QueueEntry is one round waiting for a fire, plus when it can actually fire +// and what is holding it. It is a VIEW over Rounds + Account — never persisted, +// so it carries no schema obligations. +type QueueEntry struct { + Round + // ReadyAt is the earliest time this round may fire. Zero means "now". + ReadyAt time.Time + // Why is the gate holding it: one of the Wait* constants, or "" when the + // round is simply next up. + Why string +} + +// roundReadyAt is a waiting round's OWN not-before time: RetryAt while it is +// cooling down, zero (ready now) while it is merely queued. +func roundReadyAt(r Round) time.Time { + if r.Phase == PhaseAwaitingRetry && r.RetryAt != nil { + return r.RetryAt.UTC() + } + return time.Time{} +} + +// Queue returns every round waiting for a fire — queued AND awaiting_retry — in +// the order they will actually reach the slot: by ready time, then by Seq. +// +// There is only one queue. A cooling-down round is not a different species of +// work, it is a queued round with a not-before time, and rendering it as a +// separate list left "nothing queued" and "two PRs parked until 00:07Z" looking +// identical. Ordering by (ReadyAt, Seq) reproduces what firing actually does: a +// round whose window has not opened cannot precede a ready one, and among ready +// rounds NextEligible takes the lowest Seq. +// +// ReadyAt folds in the account-wide quota block, because DecideFire gates on it +// too — showing a round's own RetryAt alone promises a time the fire gate will +// not honour once CodeRabbit extends the window. This is the same max() that +// AccountBlockedUntil computes for the wait path. +// +// minInterval is folded in too. It is DecideFire's pacing gate, so leaving it out +// rendered a round "ready: now" that firing would refuse for up to another +// CRQ_MIN_INTERVAL — 90s by default and configurable far longer. It is an +// absolute boundary (LastFired + minInterval), not a countdown, so surfacing it +// does not churn DashboardSHA between renders. +func (s *State) Queue(now time.Time, minInterval time.Duration) []QueueEntry { + var blocked time.Time + if s.Account.BlockedUntil != nil && s.Account.BlockedUntil.After(now) { + blocked = s.Account.BlockedUntil.UTC() + } + slotBusy := s.SlotRound() != nil + // The pacing gate applies to whichever round fires next, so it bounds every + // entry's earliest possible start. + var paced time.Time + if minInterval > 0 && s.LastFired != nil { + if at := s.LastFired.Add(minInterval).UTC(); at.After(now) { + paced = at + } + } + + // Split by whether the round is waiting for anything the queue serializes. + // + // A co-only round is not. It spends no account quota, takes no fire slot, and + // DecideFire resolves it before either gate — so the account window, the slot, + // and its position behind other rounds are all irrelevant to it. Exempting it + // gate-by-gate was tried and missed a different spot three times running; the + // partition makes the exemption structural, so a gate added later cannot + // silently apply to work it does not govern. + var queued, freeRunning []QueueEntry + for _, r := range s.Rounds { + if r.Phase != PhaseQueued && r.Phase != PhaseAwaitingRetry { + continue + } + e := QueueEntry{Round: r} + // Its own cooldown binds either way: that is the round's, not the queue's. + if own := roundReadyAt(r); own.After(now) { + e.ReadyAt, e.Why = own, WaitCoolingDown + } + if r.CoOnly { + freeRunning = append(freeRunning, e) + continue + } + // Every remaining gate is a lower bound on when firing will accept this + // round, so the ready time is the LATEST of them and the reason is + // whichever binds. Taking the first that matched let a one-minute cooldown + // hide a two-hour account block. + for _, g := range []struct { + at time.Time + why string + }{{blocked, WaitAccountBlocked}, {paced, WaitPacing}} { + if g.at.After(now) && g.at.After(e.ReadyAt) { + e.ReadyAt, e.Why = g.at, g.why + } + } + queued = append(queued, e) + } + + // A held slot stops EVERYTHING, free-running rounds included. + // + // Not because they need the slot — they do not — but because Pump returns as + // soon as it sees a slot holder, so the quota-free path that would advance + // them is never reached while one is held. The exemption above is about the + // account window, which genuinely does not apply to them; claiming they are + // ready here would promise action the daemon cannot take until the holder is + // acknowledged. (An agent's own `crq next` can still resolve such a round + // directly, which is why this describes the queue rather than forbidding it.) + if slotBusy { + for i := range queued { + queued[i].ReadyAt, queued[i].Why = time.Time{}, WaitSlotBusy + } + for i := range freeRunning { + freeRunning[i].ReadyAt, freeRunning[i].Why = time.Time{}, WaitSlotBusy + } + } + + // One list, ordered by readiness then Seq — which is NextEligible's own rule + // among rounds that are eligible together. Concatenating the groups instead + // put a cooling co-only round ahead of work that could fire immediately. + out := append(freeRunning, queued...) + // An empty ReadyAt means two different things and they must not sort alike: + // nothing is holding this round, or something is holding it whose end is + // unknowable (a slot another PR holds). Rank them apart, or a slot-blocked + // round sorts as if it were ready and can take the front. + rank := func(e QueueEntry) int { + switch { + case e.ReadyAt.IsZero() && e.Why == "": + return 0 // fire-eligible now + case !e.ReadyAt.IsZero(): + return 1 // waiting until a known time + default: + return 2 // waiting on something with no knowable end + } + } + sort.Slice(out, func(i, j int) bool { + if ri, rj := rank(out[i]), rank(out[j]); ri != rj { + return ri < rj + } + if !out[i].ReadyAt.Equal(out[j].ReadyAt) { + return out[i].ReadyAt.Before(out[j].ReadyAt) + } + return out[i].Seq < out[j].Seq + }) + + // Say which round is next ONLY when one is eligible now — then it is the + // lowest-Seq ready round, exactly what NextEligible picks. With everything + // still cooling, which one fires depends on when a pump happens to run: if + // none runs between two retry times, both are eligible at the next pass and + // the lower Seq wins, not the earlier window. So no front is claimed, and the + // soonest opening is reported without saying whose it is. + // + // Only that front carries a time. Anything behind it starts when the front + // finishes, which is the unknowable part, so naming its own gate there would + // state a lower bound as if it were the answer. + front := len(out) > 0 && rank(out[0]) == 0 + for i := range out { + if front && i == 0 { + continue // the front is the one round whose readiness is knowable + } + if !front && i == 0 { + continue // nothing is eligible; the soonest opening is still worth naming + } + // Drop the TIME but keep the reason. A round's own gate is real and worth + // reporting — "slot busy", "account blocked" — it just is not a start time, + // because the round ahead has to finish first. Only a round with nothing of + // its own holding it is purely behind. + out[i].ReadyAt = time.Time{} + if out[i].Why == "" { + out[i].Why = WaitBehind + } + } + return out +} + // Normalize repairs invariants after load: map init, expired retry windows // (awaiting_retry with a passed RetryAt is simply fire-eligible; nothing to // do), and a FireSlot pointing at a round that no longer holds it. diff --git a/internal/state/store.go b/internal/state/store.go index e67e557..c8d8e5e 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -42,6 +42,9 @@ type StoreConfig struct { // bots ("" hides the dashboard row, keeping co-bot-less dashboards // byte-identical). CoReviewers string + // MinInterval is DecideFire's pacing gate. The dashboard needs it so a queue + // entry is not advertised as ready before firing would actually accept it. + MinInterval time.Duration } func (c StoreConfig) requireState() error { @@ -267,7 +270,7 @@ func (s *GitStateStore) SyncDashboard(ctx context.Context, st State) error { if err != nil { return err } - title := RenderTitle(st) + title := RenderTitle(st, s.cfg) // Held across read-then-write so two concurrent syncs cannot both observe a // stale issue and both write it.