Fix credit-ceiling handling: exit 0 when AI credits budget is enforced - #49614
Conversation
…iled from daily AIC guardrail Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot refactor, apply fix to other harnesses |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Applied the same fix to both Each harness now sets |
There was a problem hiding this comment.
Pull request overview
Updates AI-credit guardrails so expected budget enforcement can complete workflows successfully.
Changes:
- Makes Copilot, Codex, and Claude harnesses exit successfully on credit-limit signals.
- Makes daily AIC threshold enforcement informational rather than failing activation.
- Updates the credit-ceiling test workflow, exclusions, messaging, and generated lock file.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/copilot_harness.cjs |
Normalizes credit-limit exits. |
actions/setup/js/copilot_harness.test.cjs |
Adds Copilot regression tests. |
actions/setup/js/codex_harness.cjs |
Normalizes credit-limit exits. |
actions/setup/js/codex_harness.test.cjs |
Adds Codex regression tests. |
actions/setup/js/claude_harness.cjs |
Normalizes credit-limit exits. |
actions/setup/js/claude_harness.test.cjs |
Adds Claude regression tests. |
actions/setup/js/check_daily_aic_workflow_guardrail.cjs |
Avoids failing activation on threshold enforcement. |
actions/setup/js/check_daily_aic_workflow_guardrail.test.cjs |
Updates threshold behavior assertions. |
.github/workflows/daily-max-ai-credits-test.md |
Revises test behavior, messages, and issue exclusions. |
.github/workflows/daily-max-ai-credits-test.lock.yml |
Recompiles the workflow changes. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suppressed comments (1)
.github/workflows/daily-max-ai-credits-test.md:38
- The workflow does not assert that the credit-limit output was actually observed. If firewall enforcement regresses, the agent reaches Turn 4, emits the instructed
noop, and exits successfully, so this message falsely reports that the limit was reached and the test still passes. Add a post-agent assertion based on the trustedai_credits_rate_limit_error/max-credit signal so the no-limit path fails instead of silently succeeding.
run-success: "🧪 [{workflow_name}]({run_url}) — expected: the per-run AI credit limit of 1 was reached and the AWF firewall cut off the agent."
- Files reviewed: 10/10 changed files
- Comments generated: 4
- Review effort level: Balanced
| if (nonRetryableGuard.aiCreditsExceeded) { | ||
| log(`attempt ${attempt + 1}: AI credits budget enforced — exiting 0 (budget control, not an error)`); | ||
| lastExitCode = 0; |
| if (nonRetryableGuard.aiCreditsExceeded) { | ||
| log(`attempt ${attempt + 1}: AI credits budget enforced — exiting 0 (budget control, not an error)`); | ||
| lastExitCode = 0; |
| if (nonRetryableGuard.aiCreditsExceeded) { | ||
| log(`attempt ${attempt + 1}: AI credits budget enforced — exiting 0 (budget control, not an error)`); | ||
| lastExitCode = 0; |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the implementation label and has 0 new lines of code in business logic directories (threshold: 100). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Review: Approve ✅
The fix correctly addresses both root causes identified in the PR description.
What was reviewed
-
check_daily_aic_workflow_guardrail.cjs: Replacingcore.setFailed()withcore.info()when the daily AIC limit is exceeded is correct — thedaily_ai_credits_exceededoutput is already set and downstream jobs gate on it. UsingsetFailedhere caused the whole workflow to fail even though hitting the limit is the expected outcome. -
claude_harness.cjs,copilot_harness.cjs,codex_harness.cjs: The addedif (nonRetryableGuard.aiCreditsExceeded) { lastExitCode = 0; }block is placed correctly beforebreak, consistent with other intentional-exit-0 paths in each harness.process.exit(lastExitCode)at the end ofmain()correctly picks up the overridden value. -
Workflow
.md/.lock.yml: Success/failure message inversion corrected;GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTERadded to preventhandle_agent_failurefrom treating budget-enforcement exits as failures; description de-alarmed.
Test coverage
Each harness has three new tests covering: exit 0 on max_ai_credits_exceeded, exit 0 on ai_credits_rate_limit_error, and continued exit 1 for unrelated failures. The check_daily_aic_workflow_guardrail test correctly asserts setFailed is not called.
No issues found. Changes are minimal, correct, and well-tested.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 39.6 AIC · ⌖ 12.2 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Skills-Based Review
Applied /diagnosing-bugs and /tdd — no blocking issues, but two risks worth addressing before merge.
Key Themes
Risk: broad credit-pattern scanning across harnesses
All three harnesses detect credit exhaustion by scanning raw CLI stdout for broad text patterns. Because exit-0 now fires on any matching string in the full output buffer, an unrelated failure whose output includes max_ai_credits_exceeded or ai_credits_rate_limit_error would silently succeed. The existing inline comments detail this precisely.
Risk: intentional-failure: true still set
The .md frontmatter now describes the workflow as concluding success, but features.intentional-failure: true remains (diff line 7). That flag opts the workflow out of fleet-health and prod-main dashboards. If the workflow now genuinely succeeds, it should graduate out of that feature flag.
Minor gap: core.info call not asserted
The updated guardrail test asserts setFailed is not called but does not assert core.info is called with the guardrail-exceeded message, leaving the replacement call untested.
Positive highlights
- Root cause for both exit-code paths correctly identified and fixed
- Regression tests added for all three harnesses
runSuccess/runFailuremessages properly invertedGH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTERadded to suppress spurious failure issues
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 34.1 AIC · ⌖ 8.06 AIC · ⊞ 7.1K
Comment /matt to run again
🧪 Test Quality Sentinel Report✅ Test Quality Score: 87/100 — Excellent
📊 Metrics (10 tests)
Scoring Breakdown
Verdict
|
|
@copilot quick triage for this PR:
Run details: https://github.com/github/gh-aw/actions/runs/30721766175
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in 6ed9554.
Validation run:
|
|
🎉 This pull request is included in a new release. Release: |
Two separate code paths caused scheduled credit-ceiling test workflows to conclude
failurewhen hitting an AI credits limit is the expected, successful outcome.Root causes
Issue 1 — agent job (
copilot_harness.cjs): When the AWF firewall enforcesmax-ai-credits, the Copilot CLI exits non-zero. The harness detectedaiCreditsExceeded, broke the retry loop, but never resetlastExitCode— soprocess.exit(lastExitCode)propagated the non-zero code and failed the agent job.Issue 2 — activation job (
check_daily_aic_workflow_guardrail.cjs): When the daily AIC threshold was exceeded, the script calledcore.setFailed()after already settingdaily_ai_credits_exceeded=true. The downstreamagentjob condition already skips on that output;setFailedwas redundant and caused the activation job — and thus the whole workflow — to fail.Changes
copilot_harness.cjs: SetlastExitCode = 0whennonRetryableGuard.aiCreditsExceededis true. Budget enforcement by the AWF firewall is intentional — it is not a job failure.check_daily_aic_workflow_guardrail.cjs: Replacecore.setFailed()withcore.info()in the threshold-exceeded path. Thedaily_ai_credits_exceededoutput and downstream job conditions are sufficient to handle skipping and reporting.daily-max-ai-credits-test.md: Addreport-failure-as-issueexclusions forai_credits_rate_limit_errorandmax_ai_credits_exceededso that a credit-limit hit does not create a spurious failure issue. Updated description and run-success/run-failure messages to reflect that reaching the limit is the expected success outcome.Tests
check_daily_aic_workflow_guardrail.test.cjs: assertsetFailedis not called when the guardrail is exceeded.copilot_harness.test.cjs: verify exit 0 onmax_ai_credits_exceeded, exit 0 onai_credits_rate_limit_error, and verify unrelated failures (e.g. auth errors) still exit non-zero.branch refresh requested from