fix: include exitCode on Codex completion events

[CoderLuii]

Summary

• Add an explicit exitCode: 0 to successful Codex complete events.
• Keep aborted Codex streams out of the normal success completion path.
• Add exitCode: 0 to Codex turn_complete provider normalization so live and history paths agree.
• Keep the existing actualSessionId, sessionId, and provider fields unchanged for successful completions.

Context

Codex was the only provider sending the final successful complete event without an explicit success exit code. This is the server-side version of the session finalization problem discussed in #614 and CoderLuii/HolyClaude#19.

This supersedes #614. Current main already handles session replacement through actualSessionId; this PR makes the provider lifecycle payload consistent at the source.

Abort handling stays separate from success handling. The abort request path already sends an aborted complete payload for client cleanup, so queryCodex now avoids sending a second normal success completion after an aborted stream.

Testing

• node --check server/openai-codex.js
• git diff --check
• npm ci completed; npm audit reported 38 existing advisories
• npm run typecheck
• npm run build passed with existing CSS/chunk/Browserslist warnings
• Compiled CodexSessionsProvider turn_complete normalization check returned kind: complete with exitCode: 0

Not live-tested: live Codex abort against the upstream service.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved completion event handling so “complete” messages are only sent when runs finish normally.
  • Added an explicit exit code (0) to completion payloads to make stream termination clearer.

• Chores

  • Enhanced abort detection so sessions are consistently marked as aborted when an abort occurs.

[coderabbitai]

📝 Walkthrough

Walkthrough

queryCodex now tracks abort state across streaming, error, and finalization phases via a wasAborted flag set when the abort signal fires or session status is observed as aborted. The "complete" WebSocket message is emitted conditionally only when not aborted and now includes exitCode: 0. Session status is finalized based on the tracked abort state. The provider normalizes the complete message to propagate the exitCode field downstream.

Changes

Abort-aware Completion Signaling

Layer / File(s)	Summary
Abort state tracking and detection server/openai-codex.js	A wasAborted flag is introduced and initialized to false, then set to true when the abort signal fires during streaming or when the session is detected as aborted. The catch block also accumulates abort detection into this shared flag.
Conditional completion with exitCode and session finalization server/openai-codex.js	The "complete" message is now emitted only when !terminalFailure && !wasAborted, with payload extended to include exitCode: 0. The finally block assigns session status to aborted or completed based on the wasAborted flag or existing session status.
Provider message normalization with exitCode server/modules/providers/list/codex/codex-sessions.provider.ts	The provider's normalizeMessage method now includes exitCode: 0 when normalizing turn_complete into a complete-kind NormalizedMessage.

Possibly Related PRs

• siteboon/claudecodeui#738: The main PR's server-side Codex WebSocket "complete" payload now conditionally emits and includes exitCode: 0, which directly aligns with the retrieved PR's client-side complete message handling logic that determines completion/cleanup based on exitCode (treating undefined/0 as successful).

Suggested Reviewers

• viper151

Poem

A rabbit tracks the abort with careful grace,
wasAborted flags race through cyberspace,
When signals fire and sessions go dark,
The "complete" stays silent—no finishing spark,
But when all runs smooth, exitCode: 0 marks the mark! 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: adding exitCode to Codex completion events. It is concise, specific, and clearly summarizes the primary modification made across the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

server/openai-codex.js (1)

313-320: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t emit exitCode: 0 for aborted sessions.

At Line 313–320 an abort only breaks the stream loop, but Line 356 still sends kind: 'complete' with exitCode: 0. That marks aborted runs as success and bypasses the client’s aborted-complete path (msg.aborted). Please branch completion payload by abort state (for example: aborted: true, non-zero exitCode, and non-completed stop reason for aborted runs).

Also applies to: 356-363

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/openai-codex.js` around lines 313 - 320, The abort checks at lines
313-320 break the stream loop when an abort is detected, but the completion
payload sent at lines 356-363 unconditionally includes `kind: 'complete'` with
`exitCode: 0`, which marks the run as successful. Track whether an abort
occurred during the stream loop (by detecting either
abortController.signal.aborted or session status being aborted), and then at
lines 356-363 conditionally construct the completion payload based on this abort
state. For aborted runs, set `aborted: true`, use a non-zero `exitCode`, and use
a non-`completed` stop reason instead of the normal success completion payload.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@server/openai-codex.js`:
- Around line 313-320: The abort checks at lines 313-320 break the stream loop
when an abort is detected, but the completion payload sent at lines 356-363
unconditionally includes `kind: 'complete'` with `exitCode: 0`, which marks the
run as successful. Track whether an abort occurred during the stream loop (by
detecting either abortController.signal.aborted or session status being
aborted), and then at lines 356-363 conditionally construct the completion
payload based on this abort state. For aborted runs, set `aborted: true`, use a
non-zero `exitCode`, and use a non-`completed` stop reason instead of the normal
success completion payload.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fa74b8f6-8f79-40a1-bbc2-248856740c9a

📥 Commits

Reviewing files that changed from the base of the PR and between 86f6479 and 164f039.

📒 Files selected for processing (1)

• server/openai-codex.js

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

server/openai-codex.js (1)

405-410: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Failure paths are being finalized as completed status

Line 409 still maps all non-aborted terminal states to 'completed'. When terminalFailure is set (for example after turn.failed), this creates a contradictory lifecycle: failure notifications are emitted, but session state says completed.

Use an explicit failed state in finalization.

Suggested fix

-        session.status = wasAborted || session.status === 'aborted' ? 'aborted' : 'completed';
+        session.status = wasAborted || session.status === 'aborted'
+          ? 'aborted'
+          : terminalFailure
+            ? 'failed'
+            : 'completed';

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/openai-codex.js` around lines 405 - 410, The session status
finalization logic in the activeCodexSessions update block only accounts for
aborted states and defaults all other terminal states to completed, which
contradicts failure notifications. Modify the conditional assignment for
session.status to check for terminalFailure in addition to wasAborted, and
assign a failed status when terminalFailure is true, an aborted status when
wasAborted or session.status is already aborted, and completed status otherwise.
This ensures the session lifecycle is consistent between the emitted failure
notifications and the recorded status.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@server/openai-codex.js`:
- Around line 405-410: The session status finalization logic in the
activeCodexSessions update block only accounts for aborted states and defaults
all other terminal states to completed, which contradicts failure notifications.
Modify the conditional assignment for session.status to check for
terminalFailure in addition to wasAborted, and assign a failed status when
terminalFailure is true, an aborted status when wasAborted or session.status is
already aborted, and completed status otherwise. This ensures the session
lifecycle is consistent between the emitted failure notifications and the
recorded status.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8923e5e5-e930-4766-91a7-e34ccb1b6db0

📥 Commits

Reviewing files that changed from the base of the PR and between 164f039 and b1b6e36.

📒 Files selected for processing (2)

• server/modules/providers/list/codex/codex-sessions.provider.ts
• server/openai-codex.js

[blackmammoth]

Hey @CoderLuii, thanks for the update. I completely revamped the websocket logic and this issue should be completely resolved now. As a result, I'm closing this PR. Lmk if the issue still occurs tho.

[CoderLuii]

@blackmammoth thanks for the websocket rewrite. I checked current main at e885391, and the general lifecycle contract is better now. There is still one Codex-specific gap, though.

The current live path still lets turn.completed become the first terminal message:

1. server/openai-codex.js transforms SDK turn.completed into raw turn_complete inside the stream loop.
2. server/modules/providers/list/codex/codex-sessions.provider.ts maps turn_complete to kind: 'complete' without exitCode, success, or aborted.
3. server/modules/websocket/services/chat-run-registry.service.ts marks the first complete as terminal and drops the later createCompleteMessage({ exitCode: 0 }) from queryCodex.

I verified that sequence with a small registry probe: first send the Codex-normalized complete with no exitCode, then send the final helper-built complete with exitCode: 0. The registry forwards only the first one, so the client never sees the helper-built success payload.

The smallest fix I see is to keep live Codex turn_complete out of the terminal websocket path and let queryCodex send the one final createCompleteMessage() after token budget/status handling. A regression test should cover that ordering directly: Codex sees turn.completed, token/status handling runs, and the client receives exactly one final complete with exitCode: 0, success: true, and aborted: false.

No need to reopen this PR if you would rather handle it as a small follow-up. I just wanted to flag that current main still has this edge case.