Repository navigation
Expose GitHub search quotas and MCP 429 retry hints - #68728
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The parser can misclassify echoed query text and discard valid HTTP-date retry hints.
2 open findings
What changed in this PR
Improves diagnostics for GitHub MCP search quota exhaustion through rate-limit snapshots, surfaced retry hints, and documented alternatives.
Changes:
- Adds nonfatal start/end quota snapshots using GitHub tool credential precedence.
- Summarizes MCP 429 responses and retry hints.
- Documents sparse checkout as an alternative to remote code search.
No confirmed vulnerabilities found.
| File | Description |
|---|---|
pkg/workflow/compiler_yaml_main_job_test.go |
Tests snapshot ordering and credentials. |
pkg/workflow/compiler_yaml_ai_execution.go |
Generates quota snapshot steps. |
docs/src/content/docs/reference/github-tools.md |
Documents quotas and local search. |
actions/setup/js/parse_mcp_gateway_log.test.cjs |
Tests 429 diagnostics. |
actions/setup/js/parse_mcp_gateway_log.cjs |
Extracts and summarizes retry hints. |
actions/setup/js/github_rate_limit_logger.test.cjs |
Tests visible quota logging. |
actions/setup/js/github_rate_limit_logger.cjs |
Logs selected quota buckets. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /grill-with-docs (fallback triage: pr-triage succeeded; see below). One actionable, non-blocking issue found.
📋 Key Themes & Highlights
Key Theme
- Unaddressed prior feedback:
pelikhan's existing review comment asking for progressive disclosure (<details>/<summary>) on the new "MCP rate limits (HTTP 429)" summary block was not applied in this revision — the block still renders as an always-expanded heading + table, inconsistent with the token-steering/DIFC-filtered sections right next to it ingenerateGatewayLogSummary.
Positive Highlights
- ✅
getMcpRateLimitHint/getRpcRateLimitHintcarefully scrub query text and URLs before surfacing retry hints — verified with targeted regex tests against adjacent 4xx/5xx noise,Retry-Aftervalues that look like status codes, and legacytool_callerror shapes. - ✅ Strong
/tddcoverage: parametrized tests cover duration formats, Retry-After edge cases, HTML-escaping of tool labels, and an end-to-endmain()test acrossgateway.jsonl/rpc-messages.jsonl/legacy log combinations. - ✅
compiler_yaml_ai_execution.goreusescollectMCPEnvironmentVariablesfor token precedence rather than duplicating resolution logic, andcompiler_yaml_main_job_test.golocks down step ordering (start snapshot before execution, end snapshot withif: always()before failure handling, artifact path included). - ✅ Docs update clearly explains the
code_searchvscorequota distinction and links the sparse-checkout alternative with a concrete config example.
No security or correctness blockers identified; the review comment above is a consistency/maintainability nit that should be quick to apply.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 81.8 AIC · ⌖ 14.7 AIC · ⊞ 10.4K
Comment /matt to run again
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
There was a problem hiding this comment.
Verdict
Useful diagnostics overall, but I found two correctness holes that make the new output easier to misread than it should be.
Highlights
- The new 429 parser can still promote echoed query text into a fake rate-limit error when the backend message does not carry a later transport status.
- Agent start and end quota snapshots currently run even when the GitHub tool is disabled, which burns quota and makes the logs look authoritative for workflows that never had GitHub MCP access.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 82.2 AIC · ⌖ 6.09 AIC · ⊞ 21.3K
Comment /review to run again
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: a6f94b7
|
…ch-rate-limit-buckets Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Harden echoed-query masking, resolve quota probes from the actual MCP tool set, and persist sanitized HTTP 429 retry hints alongside GitHub quota snapshots in the unified session. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: e70ec77
|
Keep unified session normalization independent of gateway summary logging shims, and record the intended agent-start/end quota steps in WASM fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Main is merged locally, and the remaining parser and golden-fixture fixes are committed as
The reported failures are fixed locally in |

GitHub MCP
search_codecan exhaust its smaller search budget whilecorequota remains available. Missing bucket snapshots and buried 429 retry hints make these failures difficult to diagnose.core,code_search, andsearchlimit, remaining, and reset values before and after agent execution, using GitHub tools credential precedence. End snapshots run on failure; logging remains nonfatal.Retry-Aftervalues as HTTP statuses.checkout:withsparse-checkoutfor local code searches.Example summary entry: