Fixed. The reason string now reads: "goal is already active for this thread (use update_goal when the current goal is complete)".

[/tdd] The local shouldRetry mock now checks goalAlreadyActive alone, while the production code checks all three guard flags together via a single detectNonRetryableHarnessGuard call. This creates drift: aiCreditsExceeded and awfAPIProxyBlockingRequests are absent from this mock, so a future reader testing "does aiCreditsExceeded prevent a retry?" via this describe block would find no coverage.

// Replace the single goalAlreadyActive check with the full guard:
const nonRetryableGuard = detectNonRetryableHarnessGuard(result.output);
if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests || nonRetryableGuard.goalAlreadyActive) return false;

Fixed. Updated shouldRetry to mirror the production branch exactly: const nonRetryableGuard = detectNonRetryableHarnessGuard(result.output); if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests || nonRetryableGuard.goalAlreadyActive) return false;

Test exercises a local reimplementation of shouldRetry, not the production retry path — regression in codex_harness.cjs would not be caught here.

The shouldRetry at line 409 is an inline closure used only in this test block. The new check at line 416 correctly adds goalAlreadyActive, but the test verifies the local copy of the logic, not the actual detectNonRetryableHarnessGuard+break path inside main() in codex_harness.cjs. If the production guard on line 442 of that file were accidentally removed, all tests in this describe block would still pass.

Fixed. Updated shouldRetry to mirror the full production guard (aiCreditsExceeded || awfAPIProxyBlockingRequests || goalAlreadyActive), so removing any of those guards from codex_harness.cjs would break the test.

Fixed. Replaced .* with [\s\S]*? so the pattern matches across newlines, and simplified the second anchor to \buse update_goal\b for flexibility. A dedicated multiline test confirms the behavior.

[/diagnose] GOAL_ALREADY_ACTIVE_PATTERNS is more brittle than its sibling arrays. The two-phrase .* requirement means the guard silently fails if: (a) the error spans multiple lines (.* does not match \n), or (b) Codex rephrases either clause. Silent failure here restores the exact token-burn regression this PR is fixing.

const GOAL_ALREADY_ACTIVE_PATTERNS = [
  /\bthis thread already has a goal\b/i,
];

const GOAL_ALREADY_ACTIVE_PATTERNS = [
  /\bthis thread already has a goal\b[\s\S]*\buse update_goal\b/i, // full message, any whitespace
  /\bthis thread already has a goal\b/i,                          // fallback for partial output
];

Fixed. Used Option B ([\s\S]*?) — the pattern now matches across newlines and uses \buse update_goal\b as the second anchor, bringing it in line with the multi-alternative approach of sibling patterns.

Regex is a single compound match with no fallback — any message change silently disables the guard.

Unlike AI_CREDITS_EXCEEDED_PATTERNS (3 alternatives) and AWF_API_PROXY_BLOCKING_REQUESTS_PATTERNS (4 alternatives), GOAL_ALREADY_ACTIVE_PATTERNS has one pattern that requires both phrases to appear in order on the same line. The .* quantifier does not cross newlines, so a multi-line output or a slight rewording of the codex error defeats it silently. The failure mode is invisible: the guard misses, retries continue against a permanently-broken state, and credits are wasted.

const GOAL_ALREADY_ACTIVE_PATTERNS = [
  // prefer the full compound message for precision
  /\bthis thread already has a goal\b.*\buse update_goal only when the existing goal is complete\b/i,
  // fallback: catches rewording or multi-line output
  /\bthis thread already has a goal\b/i,
];

Fixed. Replaced the single compound .* regex with [\s\S]*? so it matches across newlines. The two-phrase requirement is preserved for precision, with the negative test documenting that the first phrase alone is insufficient.

[/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.

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);
});

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.