Skip to content

πŸ› review dispatch: promote heads starved of a reviewer and say so in the tracker - #15239

Merged
clubanderson merged 1 commit into
v5from
fix/stalled-review-wrong-base
Oct 29, 2026
Merged

clubanderson merged 1 commit into
v5from
fix/stalled-review-wrong-base

Conversation

@clubanderson

Copy link
Copy Markdown
Member

Fixes #15229

What happened to #15139

After round-1 changes_requested and a fix push (head cf5adb194), only 2/5 perspectives reported; the remaining pending kicks expired at the 45m TTL (23:44Z) and were never re-dispatched because every cycle's free reviewers went to webhook-event PRs that prioritizeEventPRs moves to the front. The only trace was plan.Waiting, consumed by autoscale and never persisted, logged, or shown β€” so the tracker said just stalled.

Base v6 was correct (the file only exists on v6) and CI was green, so no retarget action is added.

Fix

  • pkg/review/dispatch.go: persist per-head review_waits (reason, consecutive cycles, first/last seen); heads waiting β‰₯ ReviewWaitStarvationCycles (3) are moved to the front inside PlanDispatch, ahead of the caller's event promotion; pruned when the head closes.
  • cmd/hive/main.go: log promotions (warn) and waits (info).
  • pkg/github/review_queue.go: review-queue reason explains the wait ("waiting for a free reviewer: N consecutive cycles over Xm … promoted ahead of the queue").
  • pkg/dashboard/stage_tracker.go: review-pending caption names the missing coverage and the wait duration.

Unit tests for planner wait tracking/promotion/threshold/pruning, review-queue reason, and tracker clause; no sleeps.

…tracker

A head that lost its pending review kicks to the 45m TTL could sit
behind webhook-event PRs indefinitely: PlanDispatch handed every free
reviewer to the event-promoted front of the list and the only trace was
plan.Waiting, consumed by autoscale and never persisted. Record the
consecutive wait per head in the dispatch state, move heads starved
for ReviewWaitStarvationCycles to the front inside the planner, and
explain the wait in the review queue and tracker caption.

Fixes #15229

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Oct 29, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign clubanderson for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 29, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this hive's repositories have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

PR Title Relation Files
#15122 πŸ› Write-back containment: per-kind gate for every unsolicited GitHub write, with backfill guard same files src/cmd/hive/main.go
#15134 πŸ› Delegated decision on a capability gap: model the blocker, stop the un-park/re-park loop, name the missing App permission CONFLICT src/pkg/dashboard/stage_tracker.go
#15236 πŸ› Prefer covering PR progress in Activity Monitor messages same files src/pkg/dashboard/stage_tracker.go
#15251 πŸ› tracker: show Merging in the merge gate for at least the toast lifetime before βœ“ same files src/pkg/dashboard/stage_tracker.go
#15223 ✨ Soak countdown on every soak pill, with the live timer on hover same files src/pkg/dashboard/stage_tracker.go
#15186 πŸ› Activity Monitor: one coherent post-decision state for Let hive decide rows; 060 reviewed in CI / Review same files src/pkg/dashboard/stage_tracker.go
#15104 πŸ› Activity Monitor: Rebase, then merge as maintainer; fixer owns parked conflicting PRs; dismiss self-inflicted changes_requested same files src/cmd/hive/main.go
src/pkg/dashboard/stage_tracker.go
#15241 πŸ› tracker: keep a closed linked issue as the row head instead of promoting the PR same files src/pkg/dashboard/stage_tracker.go
#15240 πŸ› Never adopt a person's PR: owner/human-authored PRs return to their author instead of the contributor-offline lane same files src/pkg/dashboard/stage_tracker.go
#15219 πŸ› fix: preserve human and owner PRs during delegated decisions same files src/pkg/dashboard/stage_tracker.go

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Another agent tried to open a PR from scanner/fix-15229, but this PR already closes #15229. No second PR was opened; please continue here, or rebase the new work onto this branch.

@hivecommons-hive hivecommons-hive Bot added author/human A person pushed this PR themselves dco All commits are DCO signed-off role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 29, 2026
@github-actions github-actions Bot removed the conflicts-with-open-pr Open PR has a merge conflict with another open PR label Oct 29, 2026
@hivecommons-hive hivecommons-hive Bot added stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 29, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

@clubanderson this PR is held at stage/071-needs-author and the hive will not merge it yet.

Failing checks:

  • overlayfs-exec-guard

Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path.

@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act labels Oct 29, 2026
@clubanderson
clubanderson merged commit 385cd9a into v5 Oct 29, 2026
91 of 98 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the fix/stalled-review-wrong-base branch October 29, 2026 02:26
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

@github-actions github-actions Bot added the conflicts-with-open-pr Open PR has a merge conflict with another open PR label Oct 29, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read against the tree at 7596be6, diff first. The mechanism is sound and the state lifecycle closes; looks correct to me.

Ordering inside PlanDispatch (src/pkg/review/dispatch.go:655-669): pruneState runs first, so plan.State.Waits passed to promoteStarvedWaits only ever holds records for heads still open; the !RequireApproval || !FanOut early return at :659 leaves the records untouched rather than incrementing them, which is right β€” nothing was passed over that cycle. reconcileWaits (:1018-1051) runs after the slot loop, so a promoted head that got a reviewer disappears from Waiting and its record is dropped; one still waiting gains exactly one cycle. Cycles therefore means "consecutive planner cycles", as the doc comment says.

Promotion (:977-1013) is a stable partition β€” starved heads keep their relative order, then everyone else β€” and it happens on the prs slice before the per-PR gates, so the body's "every gate still applies" claim holds (the head still has to clear ownership, cooldown, TTL etc.). Match is by samePRHead (:2057), so a new push on the PR naturally resets the count because the record keys on HeadSHA.

Pruning (:2034-2040) keys openHeads on fullRepoName, and reconcileWaits stores fullRepoName in the record (:1046), so the two agree and stale records go when the head closes. The queue reason (review_queue.go:538-541) and tracker clause (stage_tracker.go) both try the full name then the bare one, matching how the rest of that file handles pr.Repo.

Body vs diff: every bullet in "Fix" is present; the #15139 narrative matches what Waiting did before (consumed by autoscale only, never persisted).

Small note, not a defect: main.go:11033 logs every waiting head at Info each cycle. On a busy hive with max_parallel_reviews spent that is one line per owed head per cycle; if it gets loud, the Promoted warn alone carries the actionable signal.

β€” hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author/human A person pushed this PR themselves conflicts-with-open-pr Open PR has a merge conflict with another open PR dco All commits are DCO signed-off dco-signoff: yes Indicates the PR's author has signed the DCO. role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) size/L Denotes a PR that changes 100-499 lines, ignoring generated files. stage/050-review-pending Review swarm requested; awaiting a verdict for the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

πŸ› review dispatch: half-reviewed head starves behind webhook-event PRs and the tracker only says 'stalled'

1 participant