Improve Copilot organization billing failure guidance - #60877
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
No ADR enforcement needed: PR does not have the 'implementation' label and has <=100 new lines of code in business logic directories.
|
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
There was a problem hiding this comment.
🟡 Changes recommended
Proxy host variants are missed, and billing failures remain categorized as generic agent failures.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds targeted Copilot organization-billing failure detection and remediation.
Changes:
- Detects organization-billed Copilot authorization failures.
- Adds billing/PAT guidance while suppressing conflicting advice.
- Adds focused tests and a patch changeset.
File summaries
| File | Description |
|---|---|
.changeset/patch-copilot-org-billing-agent-failure.md |
Records the patch. |
actions/setup/md/copilot_org_billing_error.md |
Defines remediation guidance. |
actions/setup/md/agent_failure_comment.md |
Includes the new comment context. |
actions/setup/md/agent_failure_issue.md |
Includes the new issue context. |
actions/setup/js/handle_agent_failure.cjs |
Implements detection and rendering. |
actions/setup/js/handle_agent_failure.test.cjs |
Tests detection and guidance. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| const ENGINE_MAX_RUNS_EXCEEDED_RE = /(?:\bmax_runs_exceeded\b|\bmaximum\s+llm\s+invocations\s+exceeded\b)/i; | ||
| const COPILOT_ORG_BILLING_MODE_RE = /API proxy enabled:[^\n]*Copilot=true \(github-token\)/i; | ||
| const COPILOT_ORG_BILLING_ERROR_RE = | ||
| /(?:awf-reflect: models fetch returned 403\b|Copilot requests authentication failed through the gh-aw API proxy \(HTTP 403\b|Authentication failed with provider at (?:https?:\/\/)?(?:api-proxy|(?:172\.(?:1[6-9]|2\d|3[01])|10|192\.168)\.\d+\.\d+)(?::\d+)?[^\n]*\(HTTP 403\)|Access denied by policy settings|invalid access to inference)/i; |
There was a problem hiding this comment.
Aligned in 8dc6865: the provider-auth host alternation now mirrors isLikelyAWFAPIProxyURL (api-proxy, host.docker.internal, localhost, 127.*, 10.*, 192.168.*, 172.16-31.*), so host-bridge 403s are classified as billing failures. Added coverage for each host variant.
| const timeoutMinutes = process.env.GH_AW_TIMEOUT_MINUTES || ""; | ||
| const { aiCredits, maxAICredits, aiCreditsRateLimitError, maxAICreditsExceeded } = resolveAICreditsFailureState(); | ||
| const inferenceAccessError = process.env.GH_AW_INFERENCE_ACCESS_ERROR === "true"; | ||
| const copilotOrgBillingError = detectCopilotOrgBillingErrorFromLog(); |
There was a problem hiding this comment.
Threaded in 8dc6865: copilotOrgBillingError now flows into buildFailureMatchCategories as copilot_org_billing_error (added to the schema filter pattern and the safe-outputs docs table) and into buildFailureIssueTitle. Tests assert the category replaces the agent_failure fallback and that the title is emitted.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — requesting changes on two correctness/consistency gaps in the new Copilot org-billing detection.
📋 Key Themes & Highlights
Key Themes
- Regex overlap risk:
COPILOT_ORG_BILLING_ERROR_REincludes the genericAccess denied by policy settings/invalid access to inferencephrases, which are also the sole triggers for the existing engine-agnosticINFERENCE_ACCESS_ERROR_PATTERN. Combined with the looseCOPILOT_ORG_BILLING_MODE_REproxy-mode check, this can misclassify unrelated inference-access failures as organization-billing failures incopilot-requests: writeruns, surfacing the wrong remediation. - Incomplete wiring: unlike every other specialized signal in this file (
inferenceAccessError,mcpPolicyError, etc.),copilotOrgBillingErrorisn't threaded intobuildFailureIssueTitleorbuildFailureMatchCategories. The failure issue title and dedup marker will fall back to the generic "failed"/agent_failurecategory, weakening the specialized guidance this PR is meant to add.
Positive Highlights
- ✅ Good test coverage for the three documented org-billing 403 signatures and the PAT-isolation negative case.
- ✅ Correct suppression ordering — the new context takes priority over
inferenceAccessErrorContext/credentialAuthErrorContextso guidance isn't duplicated. - ✅ Clear, actionable remediation template with concrete settings path and PAT fallback.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
registry.npmjs.org
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 108.9 AIC · ⌖ 15.2 AIC · ⊞ 10.4K
Comment /matt to run again
| const ENGINE_MAX_RUNS_EXCEEDED_RE = /(?:\bmax_runs_exceeded\b|\bmaximum\s+llm\s+invocations\s+exceeded\b)/i; | ||
| const COPILOT_ORG_BILLING_MODE_RE = /API proxy enabled:[^\n]*Copilot=true \(github-token\)/i; | ||
| const COPILOT_ORG_BILLING_ERROR_RE = | ||
| /(?:awf-reflect: models fetch returned 403\b|Copilot requests authentication failed through the gh-aw API proxy \(HTTP 403\b|Authentication failed with provider at (?:https?:\/\/)?(?:api-proxy|(?:172\.(?:1[6-9]|2\d|3[01])|10|192\.168)\.\d+\.\d+)(?::\d+)?[^\n]*\(HTTP 403\)|Access denied by policy settings|invalid access to inference)/i; |
There was a problem hiding this comment.
[/codebase-design] COPILOT_ORG_BILLING_ERROR_RE reuses the generic Access denied by policy settings / invalid access to inference phrases that already back INFERENCE_ACCESS_ERROR_PATTERN (used across all engines, not just Copilot-with-org-billing). Since COPILOT_ORG_BILLING_MODE_RE only checks that the proxy log mentions Copilot=true (github-token) — not that this specific error line is caused by billing — any generic inference-access rejection that happens to occur in a copilot-requests: write run will be misclassified as an organization-billing failure, showing the wrong remediation (org billing policy / PAT fallback) instead of the actual cause.
💡 Suggested fix
Drop the two generic phrases from COPILOT_ORG_BILLING_ERROR_RE and rely only on the three specific 403 signatures mentioned in the PR description (models fetch returned 403, Copilot requests authentication failed ... (HTTP 403, Authentication failed with provider at <proxy-host> ... (HTTP 403)). These are the actual reported signatures; the generic phrases were designed for a different failure class and their presence undermines the intended disambiguation between PAT failures and org-billing failures.
@copilot please address this.
There was a problem hiding this comment.
Dropped both generic phrases in 8dc6865. COPILOT_ORG_BILLING_ERROR_RE now matches only the three reported 403 signatures, and a test asserts that Access denied by policy settings / invalid access to inference are no longer classified as org-billing failures.
| const timeoutMinutes = process.env.GH_AW_TIMEOUT_MINUTES || ""; | ||
| const { aiCredits, maxAICredits, aiCreditsRateLimitError, maxAICreditsExceeded } = resolveAICreditsFailureState(); | ||
| const inferenceAccessError = process.env.GH_AW_INFERENCE_ACCESS_ERROR === "true"; | ||
| const copilotOrgBillingError = detectCopilotOrgBillingErrorFromLog(); |
There was a problem hiding this comment.
[/codebase-design] copilotOrgBillingError is computed and used to suppress inferenceAccessErrorContext/credentialAuthErrorContext, but it is never added to buildFailureMatchCategories(...) or buildFailureIssueTitle(...) (both called around line 3864-3920). Every other specialized failure signal in this file (inferenceAccessError, mcpPolicyError, modelNotSupportedError, etc.) flows into both the title and the dedup category list.
💡 Impact & suggested fix
Without this wiring:
- The failure issue title falls back to the generic
[aw] {workflow} failedinstead of a specific org-billing title. failureCategorieswon't include ancopilot_org_billing_errorentry, so the dedup marker can't distinguish this failure from a genericagent_failure, and future runs may not reuse/close the right issue.
Add copilotOrgBillingError as an option to both buildFailureIssueTitle (e.g. a title like [aw] {workflow} hit Copilot organization billing error) and buildFailureMatchCategories (e.g. copilot_org_billing_error category), mirroring how inferenceAccessError is threaded through.
@copilot please address this.
There was a problem hiding this comment.
Wired in 8dc6865: copilotOrgBillingError is now passed to both buildFailureIssueTitle ([aw] {workflow} hit Copilot organization billing error) and buildFailureMatchCategories (copilot_org_billing_error), mirroring inferenceAccessError. The category is also allowed by the safe-outputs schema pattern and documented.
|
@copilot run pr-finisher skill |
There was a problem hiding this comment.
Reviewed with Impeccable clarify (docs/copy-focused change) since this PR is primarily new error-guidance markdown and detection logic, no UI. No blocking issues found.
What I checked:
COPILOT_ORG_BILLING_MODE_RE+COPILOT_ORG_BILLING_ERROR_REgating: correctly requires both thegithub-tokenproxy mode marker and one of the known 403/policy signatures, so PAT-based failures (nogithub-tokenmode line) are not misclassified — verified via the added test cases.detectCopilotOrgBillingErrorFromLogis gated onGH_AW_ENGINE_ID === "copilot", consistent with other engine-specific detectors in this file.- New context is correctly wired to suppress the now-superseded
inference_access_error_contextandcredential_auth_error_contextin both call sites, avoiding duplicate/conflicting guidance in the rendered comment. - Copy in
copilot_org_billing_error.mdis clear, actionable, and gives two concrete remediation paths (org policy toggle vs. PAT fallback) with a doc link, matching the tone of sibling templates likecredential_auth_error.mdandinference_access_error.md. - Ran the new
buildCopilotOrgBillingErrorContext/detectCopilotOrgBillingErrorFromLogtest suite locally — all 6 new tests pass. - Template wiring in
agent_failure_comment.md/agent_failure_issue.mdplaceholders looks correct and ordered sensibly (before the generic credential/inference contexts it supersedes).
No actionable issues found — approving.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 124.2 AIC · ⌖ 12.6 AIC · ⊞ 8.4K
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot workflows using
copilot-requests: writecan receive authorization errors when organization billing is unavailable. Existing failure comments incorrectly suggest checking provider credentials.Detection
Remediation
COPILOT_GITHUB_TOKENwithcopilot-requests: noneas the fallback.Coverage