Greptile Summary

This PR extends isJsonApiInternalServerError() in src/agents/pi-embedded-helpers/errors.ts to recognise OpenAI/Codex transient 500 responses (which use {"type":"server_error","code":"server_error"}) as retryable errors, in addition to the existing Anthropic api_error format. The fix correctly routes these errors through the "timeout" classification in classifyFailoverReason(), triggering model fallback instead of a hard failure.

• The logic change is minimal, well-scoped, and the dual-condition check ("type":"server_error" and "code":"server_error") makes false positives unlikely.
• No test was added for the new code path, even though the existing Anthropic branch has a dedicated test case at line 847 of the test file.
• The .includes() string-matching approach (shared with the Anthropic branch) will silently miss pretty-printed JSON responses where key-value pairs are separated by a space ("type": "server_error").

Confidence Score: 4/5

• Safe to merge; the new branch correctly classifies OpenAI server_error responses as retryable, with no breaking changes.
• The core logic is correct and well-targeted. Score is 4 rather than 5 due to the absence of a test case for the new code path (unlike the existing Anthropic branch which is tested) and the inherited fragility of substring matching on potentially pretty-printed JSON.
• No files require special attention beyond the minor style concerns noted in src/agents/pi-embedded-helpers/errors.ts.

This is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/errors.ts
Line: 859-863

Comment:
**Missing test coverage for new code path**

The existing test file (`src/agents/pi-embedded-helpers.isbillingerrormessage.test.ts`, line 847) has a dedicated test case for the Anthropic `api_error` pattern:

```ts
it("classifies JSON api_error internal server failures as timeout", () => {
  expect(
    classifyFailoverReason(
      '{"type":"error","error":{"type":"api_error","message":"Internal server error"}}',
    ),
  ).toBe("timeout");
});
```

A parallel test for the new OpenAI/Codex `server_error` path should be added alongside it to ensure the new branch stays covered and to document the expected format:

```ts
it("classifies JSON server_error internal server failures as timeout", () => {
  expect(
    classifyFailoverReason(
      '{"type":"error","error":{"type":"server_error","code":"server_error","message":"An error occurred..."}}',
    ),
  ).toBe("timeout");
});
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/errors.ts
Line: 861

Comment:
**Fragile string-matching may miss pretty-printed JSON**

The check uses `value.includes('"type":"server_error"')` (no space after the colon), which will silently fail to match if OpenAI ever sends a pretty-printed response like `"type": "server_error"`. The Anthropic branch has the same pre-existing fragility. Since you're touching this function, consider switching to a safe JSON parse + property access, or at minimum accepting an optional space:

```suggestion
  if (value.includes('"type":"server_error"') && value.includes('"code":"server_error"')) {
```

A more robust alternative would be:
```ts
try {
  const parsed = JSON.parse(raw);
  const errType = parsed?.error?.type;
  const errCode = parsed?.error?.code;
  if (errType === "server_error" && errCode === "server_error") {
    return true;
  }
} catch {
  // not valid JSON, fall through
}
```

This is a pre-existing issue but worth addressing while the function is being modified.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 0b58993}

Closing this as implemented after Codex review.

Current main already classifies OpenAI/Codex server_error payloads as retryable timeout failures, so this PR’s requested behavior is implemented via the broader failover matcher path rather than the older isJsonApiInternalServerError() helper.

What I checked:

• Current matcher recognizes server_error: src/agents/pi-embedded-helpers/failover-matches.ts:83-90 includes "server_error" and "an error occurred while processing" in the serverError patterns used for failover classification. (src/agents/pi-embedded-helpers/failover-matches.ts:83, f03252aaf9cc)
• Failover path maps server errors to timeout: src/agents/pi-embedded-helpers/errors.ts:806-810 returns timeout when isServerErrorMessage(raw) matches, before the older JSON-specific helper runs. (src/agents/pi-embedded-helpers/errors.ts:806, f03252aaf9cc)
• Regression test covers Codex payload: src/agents/pi-embedded-helpers.isbillingerrormessage.test.ts:1074-1078 asserts that a Codex/OpenAI-style payload with error.type="server_error" and error.code="server_error" is classified as timeout. (src/agents/pi-embedded-helpers.isbillingerrormessage.test.ts:1074, f03252aaf9cc)
• Implementation exists on main history: git blame attributes the relevant matcher and test lines to commit 37c37eecfbe4bef413c33b8606db62cf51c2d1d4, which is present in current main. (src/agents/pi-embedded-helpers/failover-matches.ts:83, 37c37eecfbe4)
• Checkout remained clean: Read-only inspection only; git status --short was empty before and after review. (f03252aaf9cc)

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review notes: reviewed against f03252aaf9cc; fix evidence: commit 37c37eecfbe4.