fix: handle Codex turn.failed errors and add model selection#414
Merged
pedramamini merged 1 commit intomainfrom Feb 19, 2026
Merged
fix: handle Codex turn.failed errors and add model selection#414pedramamini merged 1 commit intomainfrom
pedramamini merged 1 commit intomainfrom
Conversation
Codex CLI emits turn.failed events for API errors (model not found, stream disconnections) but the parser didn't handle this type, causing raw JSON to leak into the terminal. Also adds -m/--model support so users can override the model from Maestro's UI instead of relying solely on ~/.codex/config.toml. Closes #392
📝 WalkthroughWalkthroughThis PR extends the Codex agent with support for new GPT-5.x model variants (5.1, 5.2, 5.3), introduces a model override configuration option via command-line argument builder, and enhances error handling to parse Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryFixed Codex agent error handling by adding Key Changes:
Impact:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[Codex CLI Output] --> Parse[parseJsonLine]
Parse --> Transform[transformMessage]
Transform --> CheckType{Check message type}
CheckType -->|turn.failed| ExtractError[Extract error field]
ExtractError --> CheckErrorType{error type?}
CheckErrorType -->|object with message| UseMessage[Use error.message]
CheckErrorType -->|string| UseString[Use error string]
CheckErrorType -->|empty/null| UseDefault[Use 'Turn failed']
UseMessage --> ReturnError[Return error event]
UseString --> ReturnError
UseDefault --> ReturnError
CheckType -->|error| HandleError[Handle error type]
HandleError --> CheckErrorType
CheckType -->|other types| ProcessNormal[Process normally]
ReturnError --> Display[Display clean error in terminal]
ProcessNormal --> Display
subgraph "Model Selection"
Settings[Settings UI] -->|model field| ArgBuilder[argBuilder function]
ArgBuilder -->|if not empty| AddModelArg[Add -m modelId args]
ArgBuilder -->|if empty| UseDefault2[Use config.toml default]
end
AddModelArg --> SpawnCodex[Spawn Codex CLI]
UseDefault2 --> SpawnCodex
Last reviewed commit: 580fd8b |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/parsers/codex-output-parser.ts (1)
490-498: Consider consolidatingturn.failederror extraction for consistency.The condition on line 494 only matches
turn.failedwith object errors (parsed.error?.message), while string errors fall through to the genericelse if (parsed.error)on line 497. This works correctly but the comment on line 495 could mislead readers into thinkingturn.failedonly supports object errors.For clarity, consider handling
turn.failedthe same way as thetransformMessagemethod does:♻️ Optional: Consolidate turn.failed handling
if (parsed.type === 'error' && parsed.error) { errorText = typeof parsed.error === 'string' ? parsed.error : parsed.error?.message || JSON.stringify(parsed.error); - } else if (parsed.type === 'turn.failed' && parsed.error?.message) { - // Handle turn.failed format: {"type":"turn.failed","error":{"message":"..."}} - errorText = parsed.error.message; + } else if (parsed.type === 'turn.failed') { + // Handle turn.failed format: error can be string or { message: "..." } + errorText = + typeof parsed.error === 'object' && parsed.error?.message + ? parsed.error.message + : typeof parsed.error === 'string' + ? parsed.error + : 'Turn failed'; } else if (parsed.error) { errorText = typeof parsed.error === 'string' ? parsed.error : parsed.error?.message || JSON.stringify(parsed.error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/parsers/codex-output-parser.ts` around lines 490 - 498, Consolidate the 'turn.failed' branch so it extracts error text the same way as the generic error handling: when parsed.type === 'turn.failed' treat parsed.error uniformly (support string or object with message) instead of only checking parsed.error?.message; update the conditional around parsed.type === 'turn.failed' in codex-output-parser (where parsed.type is inspected) to mirror the extraction logic used for parsed.type === 'error' (use typeof parsed.error === 'string' ? parsed.error : parsed.error?.message || JSON.stringify(parsed.error)).src/__tests__/main/parsers/codex-output-parser.test.ts (1)
487-498: Consider asserting the specific error type for completeness.The test validates that an error is detected from
turn.failedJSON and thatagentIdis'codex', but doesn't assert the specificerror.typevalue. Adding this assertion would strengthen the test.🧪 Optional: Add error type assertion
it('should detect errors from turn.failed JSON', () => { const line = JSON.stringify({ type: 'turn.failed', error: { message: 'stream disconnected before completion: The model gpt-5.3-codex does not exist or you do not have access to it.', }, }); const error = parser.detectErrorFromLine(line); expect(error).not.toBeNull(); expect(error?.agentId).toBe('codex'); + // The error message should match one of the known patterns or fall through as generic + expect(error?.type).toBeDefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/parsers/codex-output-parser.test.ts` around lines 487 - 498, The test should also assert the parsed error's specific type: after calling parser.detectErrorFromLine(line) and the existing agentId assertion, add an expectation on error?.type (e.g., expect(error?.type).toBe('<expected_error_type>')) so the test verifies the parser returns the correct semantic error type for the 'turn.failed' JSON; update the assertion next to the existing expect(error?.agentId).toBe('codex') and use the concrete error type your parser maps that message to.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/main/parsers/codex-output-parser.test.ts`:
- Around line 487-498: The test should also assert the parsed error's specific
type: after calling parser.detectErrorFromLine(line) and the existing agentId
assertion, add an expectation on error?.type (e.g.,
expect(error?.type).toBe('<expected_error_type>')) so the test verifies the
parser returns the correct semantic error type for the 'turn.failed' JSON;
update the assertion next to the existing expect(error?.agentId).toBe('codex')
and use the concrete error type your parser maps that message to.
In `@src/main/parsers/codex-output-parser.ts`:
- Around line 490-498: Consolidate the 'turn.failed' branch so it extracts error
text the same way as the generic error handling: when parsed.type ===
'turn.failed' treat parsed.error uniformly (support string or object with
message) instead of only checking parsed.error?.message; update the conditional
around parsed.type === 'turn.failed' in codex-output-parser (where parsed.type
is inspected) to mirror the extraction logic used for parsed.type === 'error'
(use typeof parsed.error === 'string' ? parsed.error : parsed.error?.message ||
JSON.stringify(parsed.error)).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
turn.failedevents in the Codex output parser — previously these fell through to a genericsystemevent, causing raw JSON to leak into the terminal instead of a clean error message-m, --modelsupport to Codex agent definition so users can override the model from Settings or per-session, instead of relying solely on~/.codex/config.tomlMODEL_CONTEXT_WINDOWS(gpt-5.1-codex, gpt-5.2-codex, gpt-5.3 family)Closes #392
Details
The reporter's Codex agent was exiting with code 1 and displaying raw JSON like:
Root causes:
CodexRawMessage.typedidn't includeturn.failed— the parser'stransformMessage()fell to the default case, emitting it as a silentsystemeventerrorfield type wasstringbut Codex sends{ message, type }objects forturn.faileddetectErrorFromLine()didn't check forturn.failed(Claude's parser already handled this pattern)modelArgsor model config option existed for Codex despite the CLI supporting-m, --modelChanges:
codex-output-parser.ts: Addedturn.failedto type union, updatederrorfield tostring | { message?, type? }, added handlers in bothtransformMessage()anddetectErrorFromLine()definitions.ts: AddedmodelArgsandmodelconfig option (text field, empty default = use config.toml)turn.failedcoverageTest plan
turn.failedevents display as clean error messages in the terminal~/.codex/config.tomldefaultSummary by CodeRabbit
New Features
Bug Fixes
Tests