[Danathar]

Summary

When the reviewer hits its own GitHub API cap (agents.github_api_hourly_cap, 300/h), the proxy refuses its reads and the review comes back requires_human in whatever words the model picks. The existing guard (Refs #16994, #17082) only recognises "could not read … 429"-style wording, so these reviews were recorded as verdicts and parked PRs at 085 on 2026-11-09. This PR makes the decision on the cap event itself: the proxy remembers when it last refused each agent, and the review relay sets aside a requires_human from an agent that was refused after the perspective was dispatched. The perspective is asked again after review.pending_ttl, the same path as the 429 case. A follow-up commit makes sure those retries don't use up review.max_redispatch, so a reviewer capped for a few hours no longer ends with a person being asked anyway.

What went wrong

The live log shows agent GitHub API hourly cap reached agent=reviewer count=300 cap=300 at 01:33, 03:32 and 05:37Z. The warning is logged once per agent per hour, so the reviewer was capped for longer than those three moments. Reviews that ran into the cap came back requires_human and asked for a person:

• projectbluefin/utah-packages#959 (085 comment 01:39Z): "GitHub reads stopped at the user's request after the hourly cap; … could not be verified."
• projectbluefin/fsdk-containers#491 (review 05:38Z): "GitHub's hourly limit blocked access, and further reads were stopped as requested."
• projectbluefin/finpilot#659 (00:16Z): "HUMAN DECISION NEEDED … obtain a review with access to the proposed changes". It quotes the proxy's own body, hive: agent GitHub API hourly cap reached (300/300).
• projectbluefin/utah#1161 (11:25Z): "Correctness not assessed: GitHub returned HTTP 429 before PR body or diff could be read."

None of these contains a phrase from unreadableReviewTerms, so review.UnreadableReviewReason passes them through. The "at the user's request" wording comes from the proxy's own nudge, which tells the agent to "stop all GitHub reads for this session".

What changes

• Proxy (crux/proxy/agent_github_budget.go): every refusal from githubBudgetRefusal (the hourly cap or the reserve floor) records the time against the agent. New GitHubProxy.AgentGitHubRefusedSince(agent, since) answers whether the agent was refused at or after since. The record is in memory only, like the counts.
• Relay (pkg/github/review_verdict_agent_cap.go, review_verdict_relay.go): after the existing wording check, recordReviewVerdict asks the proxy whether the posting agent was refused since the open dispatch row for that PR and perspective was sent. If no row is found, it looks back one hour. If the agent was refused, a qualifying requires_human is not recorded and the watcher logs why.
• Which reports qualify (review.CapStoppedReviewReason): requires_human with no in-scope finding, no hive-decision-risk: high marker and no already-landed PR. An approve or changes_requested is still recorded. A decision question does not keep the report, because a cut-short session asks for "a review with access to the changes" (finpilot#659). A real question comes back from the re-review that can read the PR. This differs on purpose from the wording check, which keeps decision questions because it has no other evidence.
• Wiring (cmd/hive/main.go): github.SetAgentGitHubRefusalFunc(b.githubProxy.AgentGitHubRefusal) is installed where the proxy's budget config is set. It is a process-wide hook, like SetIssueReopenRecorder, because the proxy is built after the GitHub client. The review planner reads the same hook (github.AgentGitHubRefusalFunc(), cmd/hive/main_sweeps.go).

The test errs toward asking again. If the cap hit the lane's previous session after this row was dispatched, the report is still set aside, which costs one extra review and never records a verdict that wasn't read.

Follow-up commit: cap retries don't use up max_redispatch

The first commit set the report aside, but the expiry after it still counted as one of the three review.max_redispatch tries. Three 45-minute TTLs is about 2h15m, so a reviewer capped that long on one head still ended in a review-dispatch-exhausted hold.

• Proxy: each refusal now also records when it said the agent could read again, using its Retry-After. That is when the sliding hour frees a slot, or when the shared bucket resets. GitHubProxy.AgentGitHubRefusal(agent, since) returns that time. AgentGitHubRefusedSince is now a thin wrapper around it.
• Planner (pkg/review/dispatch_agent_cap.go): when the pending rows expire, the planner checks whether the proxy refused the agent of every expired row after that row was sent. If so, the cap caused the expiry.
  • It waits until the refusal's Retry-After has passed, for at most one hour past pending_ttl. Asking sooner would just be refused again. The rows stay in place while it waits, so the perspective is not asked twice.
  • Then it re-asks as usual, but adds the expiry to a new cap_redispatches[] count instead of redispatches[]. The governor's review pending expired line now carries cap_caused=true.
  • That count has its own limit, DefaultMaxCapRedispatch = 8. That's about a working day at the default TTL, and it isn't a config key. When it is reached, the planner sets the same review-dispatch-exhausted hold, with a reason that names the cap. So a reviewer whose cap is too low to ever finish a review still reaches a person and does not loop forever.
• Why a separate count and not a refund: the existing count stays honest, it is just as easy to reason about, and the limit for cap retries shows up in the state file. A refund with no limit could loop forever, and a time budget would need a second clock beside pending_ttl.
• Anything else counts as before: an expiry still uses up max_redispatch if any expired row's agent was not refused, if the refusal came before the row was sent, if the refusal hit another agent, or if no proxy record exists. A restart forgets the proxy record, so an expiry right after a restart counts as ordinary.

Files

• src/crux/proxy/agent_github_budget.go, agent_github_budget_test.go
• src/pkg/review/unreadable.go, unreadable_test.go
• src/pkg/review/dispatch.go, dispatch_agent_cap.go, dispatch_agent_cap_test.go (follow-up)
• src/pkg/github/review_verdict_agent_cap.go, review_verdict_agent_cap_test.go, review_verdict_relay.go
• src/cmd/hive/main.go, main_sweeps.go
• src/docs/review-swarm.md, changelog.d/fixed-review-agent-cap-no-verdict.md

Tests

None were run locally (AGENTS.md); CI is the check.

• TestAgentGitHubRefusedSinceRecordsCapAndReserveRefusals: a cap refusal and a reserve-floor refusal are both recorded, scoped by agent and time. Under the cap nothing is recorded. The hive's own calls and a nil proxy never read as refused.
• TestCapStoppedReviewReasonDoesNotDependOnWording / TestCapStoppedReviewReasonKeepsJudgements: the utah-packages#959 and utah#1161 summaries are set aside, and the test asserts that the wording check misses them. The finpilot#659 decision question is set aside too. Approve, changes_requested, an in-scope finding, the high-risk marker and an already-landed PR are all kept.
• TestRecordReviewVerdictCapStoppedSessionIsNoVerdict: the three utah-packages#959 reports are posted after a refusal that falls inside the session. Nothing is written, the refusal window starts at the dispatch time, and after the pending TTL the planner re-asks without asking a person.
• TestRecordReviewVerdictKeepsRequiresHumanWithoutARefusalInTheSession: the same reports are recorded when no proxy is wired, when the refusal came before the dispatch, when another agent was refused, and when the verdict is approve.

Follow-up commit:

• TestPlanDispatchCapCausedExpiriesDoNotSpendRedispatchBudget: eight cap-caused expiries in a row. After the third, and up to the seventh, the head is not held and is asked again, and redispatches stays at 0. The eighth sets the review-dispatch-exhausted hold, once, with a reason that names the cap.
• TestPlanDispatchOrdinaryExpiriesStillExhaust: no proxy record, a refusal before the row was sent, another agent refused, or only one expired row's agent refused. In each case the head is still held after three expiries, as before.
• TestPlanDispatchCapCausedExpiryWaitsForTheRefusalToClear: no re-ask and no count before the refusal clears. One re-ask after it clears. No wait more than an hour past the TTL.
• TestPlanDispatchCapRedispatchesResetOnNewHead: a new push drops the cap count.
• TestCapStoppedSessionsInARowDoNotExhaustTheRedispatchBudget: the full path, relay plus planner. Four utah-packages#959 sessions in a row, each refused by the proxy, never set a hold. The testsuite#1116 "could not read … 429" wording with no proxy refusal still sets the hold after the third session.
• TestAgentGitHubRefusedSinceRecordsCapAndReserveRefusals now also checks the clear time: one hour after the oldest counted request for the cap, the bucket reset for the reserve floor.

On the v6 tip and on the first commit, the follow-up tests don't compile, because AgentGitHubRefusal, CapRedispatchCountFor and SetAgentGitHubRefusalFunc are new. Without the planner change, the cap loop sets the hold on its third cycle and fails.

On the v6 tip the first three tests don't compile (AgentGitHubRefusedSince, CapStoppedReviewReason and SetAgentGitHubRefusedSinceFunc don't exist). Without the relay change, the utah-packages#959 reports would be recorded and the first relay test would fail on recorded 3 verdicts.

Not in this PR

• Verdicts already stored on the four PRs above stay as they are. The cap record is new and in memory, so nothing can tell those stored reports apart afterwards. They need a new head or a manual re-review.
• The nudge text ("stop all GitHub reads for this session") is unchanged.
• With review.review_heads on, a head dispatch that expires still counts toward review.head_max_retries whatever the cause. The PR-level max_redispatch fallback after that does get the cap rule.
• The review queue's redispatch_count shows only the ordinary count. Cap-caused expiries are in the state file's cap_redispatches[] and the governor log, not on the dashboard.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1

[kubestellar-prow]

[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 hanthor 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:

• OWNERS

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

[github-actions]

Open PR overlap check

Base branch: v6

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

PR	Title	Relation	Files
#17385	✨ feat(settings): the GitHub API hourly caps per agent can be set from the dashboard and the admin MCP	same files	src/cmd/hive/main.go
#17378	🐛 fix(proxy): An agent refused because the hive's shared GitHub budget is low is told so, with the reset time, and keeps working	same files	src/crux/proxy/agent_github_budget.go src/crux/proxy/agent_github_budget_test.go src/pkg/review/unreadable_test.go

[hivecommons-hive]

Read against v6 at the merge-base, including the callers. The change looks correct to me; one open question and one behavioural note, neither a defect.

Verified

• src/crux/proxy/agent_github_budget.go — githubBudgetRefusal is called only from github_proxy_mitm.go:294, and the new defer marks the agent refused on both the cap and reserve returns. markRefused/refusedSince guard a nil receiver, so the reserve-floor path with p.githubBudget == nil is safe (it just records nothing). agentCapSessionLookback (1h) matches agentGitHubBudgetWindow at agent_github_budget.go:19.
• src/pkg/github/review_verdict_agent_cap.go — reviewPerspectiveDispatchedAt reads PendingReview.Repo/Number/Perspective/Dispatched as they exist in pkg/review/dispatch.go:398-429; LoadDispatchState("") is the existing default-path entry at dispatch.go:631. The continue in review_verdict_relay.go mirrors the unreadable-wording branch directly above it, so the pending row survives and is re-asked on the same TTL path.
• review.CapStoppedReviewReason uses InScopeFinding (review.go:1415) and the existing DecisionRiskHigh/RedundantOf fields; it never fires for approve/changes_requested.
• cmd/hive/main.go:4333 installs the hook right after SetAgentGitHubBudgetConfig, so the proxy exists when it is wired.
• Body and diff agree, including the "Not in this PR" items.

Open question (not verified either way)

capStoppedReview keys the lookup by req.Agent (the name hive-review writes, or the sandbox manifest's agent at review_request_watcher.go:288), while the proxy keys refused by the MITM connection's agentName. If those two names ever differ for the same lane (e.g. a sandbox spool), the check is silently inert — it would never set a verdict aside, which fails safe but defeats the fix. Worth a one-line confirmation from whoever knows the proxy's agent identity path that they are the same string.

Behavioural note

The dispatch-row lookup does not filter by PendingReview.Agent, and a refusal that lands after the agent posts its verdict but before the watcher tick also counts. Both err toward re-asking, as the body says, and each costs one max_redispatch slot; fine as designed, just naming it.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

Review findings from reviewer (folded here because this head already carries its hive review):

The code does what the body says, and I found no defect in the mechanism. The question I cannot settle is a policy one about which requires_human reports it discards.

What I verified

• src/crux/proxy/agent_github_budget.go:190-199 — the defer on the named return records markRefused for every non-nil refusal (cap and reserve floor), and markRefused/refusedSince are nil-receiver safe, so the cap > 0 && p.githubBudget != nil branch below cannot panic. The call site is the single one at github_proxy_mitm.go:294.
• Both sides key on the configured agent name: the proxy from the connection (proxyHTTPHost agentName), the relay from ReviewRequest.Agent (review_request_watcher.go:71, overridden by the sandbox manifest at :288). Same identifier, so the lookup matches.
• review_verdict_relay.go:126-137 — the new branch sits after the existing UnreadableReviewReason branch and before writeOneVerdict, and the comment itself is still posted first (review_request_watcher.go:700 calls recordReviewVerdict after created), i.e. identical shape to the 429 path. Dispatch lookup uses the default state path (review/dispatch.go:631-635) and falls back to a one-hour lookback when no row is found.

The policy question (correctness/intent, medium)

src/pkg/review/unreadable.go:108-122 — CapStoppedReviewReason deliberately does not keep a report because it carries a decision question; only DecisionRiskHigh, RedundantOf or an in-scope finding keeps it. Combined with capStoppedReview (review_verdict_agent_cap.go:56-75) using the dispatch time or a full hour as the window, any refusal to the reviewer in that window discards every finding-less requires_human in the batch, including a legitimate trust-surface route ("OWNERS change, owner must decide") that the reviewer policy tells the agent to emit without the risk marker. The body's own evidence shows the reviewer capped at 01:33, 03:32 and 05:37Z, so on that hive most windows contain a refusal, and such a route would be re-asked after pending_ttl indefinitely rather than parking at 085 — the one class of PR that must reach a person. The body says the design "errs toward asking again" but does not name this consequence.

Two cheap ways to narrow it if you agree it matters: keep the report when DecisionQuestion != "", or require the refusal to have happened after this PR's dispatch row only (drop the one-hour fallback). Either is a one-line change to CapStoppedReviewReason / capStoppedReview; the tests in unreadable_test.go and review_verdict_agent_cap_test.go would need one case each.

I have not closed or changed anything.

Evidence bundle `#1737681bcae9` sha256:087abe065092684ad3ddc4ddf2071bc0273334389de80735ba135487b7089705 — download from the dashboard or `hivectl review evidence #17376`.

[hivecommons-hive]

Decision needed: @clubanderson should decide whether the relay may set aside a requires_human that carries a hive-decision-question (no findings) on proxy-refusal evidence alone, including the 1-hour fallback lookback used when no dispatch row exists for the perspective.

The mechanics read correctly; the open point is a routing policy choice, not a defect.

Verified

• src/crux/proxy/agent_github_budget.go (PR head): the defer in githubBudgetRefusal runs after both the reason=cap and reason=reserve returns, so both refusals are recorded; markRefused is nil-receiver-safe, so a proxy built without a budget (p.githubBudget == nil) does not panic. refusedSince compares last.at.Before(since) under the same mutex as the other maps.
• src/pkg/review/dispatch.go expirePending: the cap-caused branch is taken before Redispatches is touched, so cap-caused expiries neither increment nor reset redispatches[], as the doc claims. pruneState drops CapRedispatches for closed heads alongside Redispatches.
• src/pkg/review/dispatch_agent_cap.go capCausedExpiry: one expired row whose agent was not refused makes it an ordinary expiry (returns false,false on the first miss), and the wait is bounded at earliest + ttl + 1h, so a refusal record with a far-future clears cannot park a head indefinitely.
• src/pkg/github/review_verdict_relay.go:126-137 mirrors the existing unreadable set-aside: the comment is still posted, only the verdict write is skipped.
• review-swarm.md and the changelog fragment describe exactly this behaviour, including the restart caveat.

The policy question

src/pkg/review/unreadable.go CapStoppedReviewReason keeps a report only on DecisionRiskHigh, RedundantOf or an in-scope finding. A requires_human whose whole content is a hive-decision-question — the normal shape for "a maintainer must choose intent/scope/ownership" — is set aside whenever the posting agent had any refusal after since. And src/pkg/github/review_verdict_agent_cap.go falls back to since = now - 1h when no pending row matches, which is the case for reviewer kicks that did not come from the planner. Reviewer lanes that sit near their 300/h cap most of the day would then have their needs-decision verdicts deferred up to MaxCapRedispatch (8) × pending_ttl before converging to a hold, with a fresh comment on each re-ask. The PR body states this trade-off ("errs toward asking again"); I am not saying it is wrong, only that it is a routing policy decision an approver should make knowingly, because it changes when 085-needs-decision gets reached on a capped hive.

Two cheaper alternatives if the answer is "too broad": (a) keep a report whose DecisionQuestion is non-empty unless the summary also matches rateLimitCauseRe; or (b) drop the 1-hour fallback and only set aside when a dispatch row pins since.

One nit, not blocking: reviewPerspectiveDispatchedAt picks the latest row across heads for the PR+perspective, so when an older head's row is still open and a newer head was dispatched later, a refusal between the two dispatches is missed. Conservative direction (records the verdict), so fine.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88