-
Notifications
You must be signed in to change notification settings - Fork 432
Stop harness retries on AI-credit exhaustion and AWF proxy blocking signals #38018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // @ts-check | ||
|
|
||
| "use strict"; | ||
|
|
||
| const AI_CREDITS_EXCEEDED_PATTERNS = [/\bmax[\s_-]*ai[\s_-]*credits[\s_-]*exceeded\b/i, /\bai[\s_-]*credits[\s_-]*rate[\s_-]*limit[\s_-]*error\b/i, /ai[\s_-]*credits?.*(?:rate[\s-]*limit|limit exceeded|budget exceeded|exceeded)/i]; | ||
|
|
||
| const AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS = [/\bawf\b.*\bapi[\s_-]*proxy\b.*\bblocking requests\b/i, /\bapi[\s_-]*proxy\b.*\bblocking requests\b/i, /\bapi[\s_-]*proxy\b.*\bblocked requests?\b/i, /\bDIFC_FILTERED\b/]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/diagnose] Same readability issue: all four AWF proxy patterns are on one line. Splitting to one-per-line makes each pattern independently reviewable and auditable. 💡 Suggested formattingconst AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS = [
/\bawf\b.*\bapi[\s_-]*proxy\b.*\bblocking requests\b/i,
/\bapi[\s_-]*proxy\b.*\bblocking requests\b/i,
/\bapi[\s_-]*proxy\b.*\bblocked requests?\b/i,
/\bDIFC_FILTERED\b/,
]; |
||
|
|
||
| /** | ||
| * Detect retry guard conditions that should stop harness retries immediately. | ||
| * @param {unknown} output | ||
| * @returns {{ aiCreditsExceeded: boolean, awfAPIProxyBlockingRequests: boolean }} | ||
| */ | ||
| function detectNonRetryableHarnessGuard(output) { | ||
| const safeOutput = typeof output === "string" ? output : ""; | ||
|
Comment on lines
+9
to
+15
|
||
| return { | ||
| aiCreditsExceeded: AI_CREDITS_EXCEEDED_PATTERNS.some(pattern => pattern.test(safeOutput)), | ||
| awfAPIProxyBlockingRequests: AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS.some(pattern => pattern.test(safeOutput)), | ||
| }; | ||
| } | ||
|
|
||
| if (typeof module !== "undefined" && module.exports) { | ||
| module.exports = { | ||
| detectNonRetryableHarnessGuard, | ||
| AI_CREDITS_EXCEEDED_PATTERNS, | ||
| AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // @ts-check | ||
|
|
||
| import { describe, expect, it } from "vitest"; | ||
| import { createRequire } from "node:module"; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
| const { detectNonRetryableHarnessGuard } = require("./harness_retry_guard.cjs"); | ||
|
|
||
| describe("harness_retry_guard.cjs", () => { | ||
| it("detects AI credits exceeded markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("error: max_ai_credits_exceeded=true"); | ||
| expect(result.aiCreditsExceeded).toBe(true); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] Only the first AI-credits pattern ( 💡 Add tests for the other two patternsit("detects ai_credits_rate_limit_error signal", () => {
const result = detectNonRetryableHarnessGuard("error: ai_credits_rate_limit_error=true");
expect(result.aiCreditsExceeded).toBe(true);
});
it("detects broad ai-credits-budget-exceeded signal", () => {
const result = detectNonRetryableHarnessGuard("ai credits budget exceeded");
expect(result.aiCreditsExceeded).toBe(true);
});Each regex in |
||
| }); | ||
|
|
||
| it("detects AI credits rate-limit markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("error: ai_credits_rate_limit_error=true"); | ||
| expect(result.aiCreditsExceeded).toBe(true); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(false); | ||
| }); | ||
|
|
||
| it("detects AI credits budget markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("error: ai credits budget exceeded"); | ||
| expect(result.aiCreditsExceeded).toBe(true); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(false); | ||
| }); | ||
|
|
||
| it("detects AWF API proxy blocking request markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("awf api proxy is blocking requests for this run"); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] Only the first AWF proxy pattern (full phrase with "awf") is tested in the positive direction; the second ( 💡 Cover the remaining AWF proxy patternsit("detects api-proxy blocking requests without awf prefix", () => {
const result = detectNonRetryableHarnessGuard("api-proxy is blocking requests");
expect(result.awfAPIProxyBlockingRequests).toBe(true);
});
it("detects api-proxy blocked requests variant", () => {
const result = detectNonRetryableHarnessGuard("api proxy blocked request");
expect(result.awfAPIProxyBlockingRequests).toBe(true);
});Driving one test per pattern (rather than one test per signal type) keeps each regex independently verifiable and makes failures self-diagnosing. |
||
| expect(result.awfAPIProxyBlockingRequests).toBe(true); | ||
| }); | ||
|
|
||
| it("detects API proxy blocking request markers without AWF prefix", () => { | ||
| const result = detectNonRetryableHarnessGuard("api-proxy is blocking requests"); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(true); | ||
| }); | ||
|
|
||
| it("detects API proxy blocked request markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("api proxy blocked request"); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(true); | ||
| }); | ||
|
|
||
| it("detects DIFC filtered proxy block markers", () => { | ||
| const result = detectNonRetryableHarnessGuard('{"type":"DIFC_FILTERED","reason":"blocked"}'); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false for non-string input", () => { | ||
| const result = detectNonRetryableHarnessGuard(null); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(false); | ||
| }); | ||
|
|
||
| it("detects both flags when output contains both signals", () => { | ||
| const result = detectNonRetryableHarnessGuard("max_ai_credits_exceeded=true DIFC_FILTERED"); | ||
| expect(result.aiCreditsExceeded).toBe(true); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false when output has no guard markers", () => { | ||
| const result = detectNonRetryableHarnessGuard("transient network timeout"); | ||
| expect(result.aiCreditsExceeded).toBe(false); | ||
| expect(result.awfAPIProxyBlockingRequests).toBe(false); | ||
| }); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] No test covers non-string input (e.g. 💡 Add boundary testsit("returns false for null input", () => {
const result = detectNonRetryableHarnessGuard(null);
expect(result.aiCreditsExceeded).toBe(false);
expect(result.awfAPIProxyBlockingRequests).toBe(false);
});
it("returns both flags true when output contains both signals", () => {
const combined = "max_ai_credits_exceeded=true; DIFC_FILTERED";
const result = detectNonRetryableHarnessGuard(combined);
expect(result.aiCreditsExceeded).toBe(true);
expect(result.awfAPIProxyBlockingRequests).toBe(true);
});The combined-signals test also validates that the log message in each harness correctly emits both reasons separated by " and ". |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/diagnose] All three AI-credits patterns are on one long line, making them very hard to review for correctness and nearly impossible to diff in future PRs.
💡 One pattern per line
Note also that the third pattern lacks a leading
\bword-boundary unlike the first two — worth confirming that is intentional (it broadens the match surface).