Repository navigation
🐛 fix(fleetreport): exclude expected agent GitHub quota warnings - #14347
Conversation
Keep bounded quota enforcement notifications in logs without reporting them as repeated runtime defects. Preserve reporting for other budget failures. Hive-Run: hivecommons#14346 Hive-Plan: hourly-quota-selfreport Hive-Spec: issue-14346#expected-quota-backpressure Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
There was a problem hiding this comment.
Read the tree, not just the diff — this looks correct to me.
src/pkg/fleetreport/fleetreport.go:765lowercasesmsgbefore the newswitch strings.TrimSpace(m), so the lower-case case labels match the mixed-case strings the proxy actually logs:"agent GitHub API hourly budget above soft threshold"(src/crux/proxy/agent_github_budget.go:178) and"agent GitHub API hourly cap reached"(agent_github_budget.go:207). The exact-match form (rather than abenignSubstringsentry) is a reasonable choice here: it keeps"agent GitHub API hourly budget accounting failed"reportable, which the new test asserts.src/pkg/fleetreport/slogfeed/slogfeed.go:93and:108both consultIsBenignMessagebeforeLogFedClass, so the suppression applies on both the keyed and the explicit-class path; the record is still forwarded to the wrapped handler (feedonly gates the observer), which is what the test's 16-line count checks.- Checked
Classeskeywords insrc/pkg/fleetreport/runtime.go:160-188: none containbudget/accounting/hourly, so the Error control line in the test falls through toClassUnclassifiedRepeatedas asserted. - Quota enforcement itself is untouched — the diff is confined to the fleet-report classifier and its test, matching the body's claim.
Changelog fragment name fixed-14346-hourly-cap-selfreport.md satisfies the fragment guard. No findings; a maintainer can merge once test (rest …) finishes.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read this against the v5 tree rather than just the diff — it looks correct to me.
Why the exact-match is right:
IsBenignMessagelower-cases first (src/pkg/fleetreport/fleetreport.go:765), and the two new strings are exactly the lower-cased forms of the only two emitters:crux/proxy/agent_github_budget.go:178("agent GitHub API hourly budget above soft threshold") and:207("agent GitHub API hourly cap reached"). Nothing else insrc/logs those messages.- The
:207site already demotes repeats to DEBUG and only WARNs once per agent per window, so the accumulation the body describes can only come from many agents / many windows — exact-matching the notification (instead of abenignSubstringsentry) is the narrowest fix that covers it. - The test's control line,
"agent GitHub API hourly budget accounting failed", does not contain anyClasseskeyword (src/pkg/fleetreport/runtime.go:154-191), so it goes throughKey()→ClassUnclassifiedRepeatedas the test asserts (slogfeed.go:92-104). Behaviour preserved for real failures.
Body and diff agree; the changelog fragment name follows the fixed-<pr-or-slug>.md convention.
One optional nit, not a defect: the IsBenignMessage doc comment (fleetreport.go:760-763) enumerates what it covers and doesn't mention the new quota case. Fine to leave.
CI: the test (rest …) shards that run pkg/fleetreport were still in progress when I read this; I did not wait on them.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Ready for a human merge: verdict approved @0ca9f2d, 51/51 checks green, DCO yes, soak 30m0s elapsed. Blocked only by The hive never approves on your behalf: |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
clubanderson
left a comment
There was a problem hiding this comment.
Fix is right: the exact hourly-cap / soft-budget notices are expected, not errors. Thanks @thatguywob. Test-file overlap with #14365 is sibling-only (both add cases to slogfeed_test.go) — whichever lands second rebases.
|
/hive decision merge; if #14365 lands first, rebase this one onto it (test-only overlap in slogfeed_test.go). |
|
LGTM label has been added. DetailsGit tree hash: 5c4bf3341d1c2dd31e51e3f3a8a0428238948195 |
clubanderson
left a comment
There was a problem hiding this comment.
Reviewed: scoped fix with regression test, CI green. Approved.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Expected per-agent GitHub quota enforcement warnings can accumulate across agents or cap windows and trigger
unclassified_error_repeated. Treat the exact hourly-cap and soft-budget notification messages as benign for fleet reporting while preserving the original logs and quota behavior.Adds regression coverage for repeated notifications, retained log diagnostics, and continued reporting of budget accounting failures, plus a changelog fragment.
Validation:
gofmt -lreturned no files andgit diff --checkpassed. Tests were not run locally per AGENTS.md; CI runs the Go test suite.Fixes #14346
— hive: backend=codex model=gpt-6.1-sol
🐝 Hive Agent:
contributor| SHA:0ca9f2d25