Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion actions/setup/js/codex_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,11 @@ async function main() {
}

const nonRetryableGuard = detectNonRetryableHarnessGuard(result.output);
if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests) {
if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests || nonRetryableGuard.goalAlreadyActive) {
const reasons = [];
if (nonRetryableGuard.aiCreditsExceeded) reasons.push("AI credits budget exceeded");
if (nonRetryableGuard.awfAPIProxyBlockingRequests) reasons.push("AWF API proxy is blocking requests");
if (nonRetryableGuard.goalAlreadyActive) reasons.push("goal is already active for this thread (use update_goal when the current goal is complete)");
log(`attempt ${attempt + 1}: ${reasons.join(" and ")} — not retrying (non-retryable guard condition)`);
break;
}
Expand Down
12 changes: 12 additions & 0 deletions actions/setup/js/codex_harness.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
validateCodexOpenAIBaseURLFromReflect,
hasNoopInSafeOutputs,
} = require("./codex_harness.cjs");
const { detectNonRetryableHarnessGuard } = require("./harness_retry_guard.cjs");

const agentTempDir = "/tmp/gh-aw/agent";

Expand Down Expand Up @@ -412,6 +413,8 @@ env_key = "OPENAI_API_KEY"
if (attempt === 0 && isAuthenticationFailedError(result.output)) return false;
if (isMissingApiKeyError(result.output)) return false;
if (hasNumerousPermissionDeniedIssues(result.output)) return false;
const nonRetryableGuard = detectNonRetryableHarnessGuard(result.output);
if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests || nonRetryableGuard.goalAlreadyActive) return false;
const isTransient = RATE_LIMIT_ERROR_PATTERN.test(result.output) || SERVER_ERROR_PATTERN.test(result.output);
return attempt < MAX_RETRIES && (result.hasOutput || isTransient);
}
Expand Down Expand Up @@ -461,6 +464,15 @@ env_key = "OPENAI_API_KEY"
const result = { exitCode: 1, hasOutput: true, output: "permission denied\npermission denied\npermission denied" };
expect(shouldRetry(result, 0)).toBe(false);
});

it("does not retry when codex reports an existing active goal", () => {
const result = {
exitCode: 1,
hasOutput: true,
output: "cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete",
};
expect(shouldRetry(result, 0)).toBe(false);
});
});

describe("noop pre-flight and retry guard", () => {
Expand Down
5 changes: 4 additions & 1 deletion actions/setup/js/harness_retry_guard.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@
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/];
const GOAL_ALREADY_ACTIVE_PATTERNS = [/\bthis thread already has a goal\b[\s\S]*?\buse update_goal\b/i];

/**
* Detect retry guard conditions that should stop harness retries immediately.
* @param {unknown} output
* @returns {{ aiCreditsExceeded: boolean, awfAPIProxyBlockingRequests: boolean }}
* @returns {{ aiCreditsExceeded: boolean, awfAPIProxyBlockingRequests: boolean, goalAlreadyActive: boolean }}
*/
function detectNonRetryableHarnessGuard(output) {
const safeOutput = typeof output === "string" ? output : "";
return {
aiCreditsExceeded: AI_CREDITS_EXCEEDED_PATTERNS.some(pattern => pattern.test(safeOutput)),
awfAPIProxyBlockingRequests: AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS.some(pattern => pattern.test(safeOutput)),
goalAlreadyActive: GOAL_ALREADY_ACTIVE_PATTERNS.some(pattern => pattern.test(safeOutput)),
};
}

Expand All @@ -24,5 +26,6 @@ if (typeof module !== "undefined" && module.exports) {
detectNonRetryableHarnessGuard,
AI_CREDITS_EXCEEDED_PATTERNS,
AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS,
GOAL_ALREADY_ACTIVE_PATTERNS,
};
}
25 changes: 25 additions & 0 deletions actions/setup/js/harness_retry_guard.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,42 @@ describe("harness_retry_guard.cjs", () => {
const result = detectNonRetryableHarnessGuard(null);
expect(result.aiCreditsExceeded).toBe(false);
expect(result.awfAPIProxyBlockingRequests).toBe(false);
expect(result.goalAlreadyActive).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);
expect(result.goalAlreadyActive).toBe(false);
});

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);
expect(result.goalAlreadyActive).toBe(false);
});

it("detects goal already active markers", () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The new test only validates the exact canonical message. Consider adding a negative case that confirms a partial first-phrase match alone does NOT trigger detection — this would make the two-phrase requirement an explicit, documented contract rather than an implicit one.

💡 Suggested additions
it("does not detect goal active from first phrase alone", () => {
  const result = detectNonRetryableHarnessGuard("this thread already has a goal");
  expect(result.goalAlreadyActive).toBe(false); // adjust to true if single-phrase is desired
});

it("detects goal already active when embedded in longer output", () => {
  const result = detectNonRetryableHarnessGuard(
    "[codex] cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete\nExit code: 1"
  );
  expect(result.goalAlreadyActive).toBe(true);
});

Also: the existing null input test (line 52) and "both flags" test (line 58) were not updated to assert goalAlreadyActive: false — worth adding for completeness.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Added: (1) negative case asserting the first phrase alone does not trigger detection, (2) embedded-output multiline test, and (3) goalAlreadyActive: false assertions in the null input and both flags tests.

const result = detectNonRetryableHarnessGuard("cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete");
expect(result.aiCreditsExceeded).toBe(false);
expect(result.awfAPIProxyBlockingRequests).toBe(false);
expect(result.goalAlreadyActive).toBe(true);
});

it("detects goal already active markers across newlines", () => {
const result = detectNonRetryableHarnessGuard("this thread already has a goal\nuse update_goal to update it");
expect(result.goalAlreadyActive).toBe(true);
});

it("does not detect goal active from first phrase alone", () => {
const result = detectNonRetryableHarnessGuard("this thread already has a goal");
expect(result.goalAlreadyActive).toBe(false);
});

it("detects goal already active when embedded in longer output", () => {
const result = detectNonRetryableHarnessGuard("[codex] cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete\nExit code: 1");
expect(result.goalAlreadyActive).toBe(true);
});
});