Skip to content

fix(daemon): handle Codex stream reconnect events#2274

Open
VIVAAN-DHAWAN wants to merge 1 commit into
nexu-io:mainfrom
VIVAAN-DHAWAN:codex/handle-codex-stream-reconnect-2268
Open

fix(daemon): handle Codex stream reconnect events#2274
VIVAAN-DHAWAN wants to merge 1 commit into
nexu-io:mainfrom
VIVAAN-DHAWAN:codex/handle-codex-stream-reconnect-2268

Conversation

@VIVAAN-DHAWAN
Copy link
Copy Markdown

Summary

  • Treat Codex Reconnecting... JSON error frames for stream disconnected before completion as recoverable status events instead of fatal adapter errors.
  • Keep existing timeout-reconnect handling intact.
  • Add regression coverage for the reported reconnect shape and a guard that non-reconnect errors mentioning reconnect text still remain fatal.

Fixes #2268.

Verification

  • PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.ts
  • PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.ts -t "stream disconnect|non-reconnect"
  • PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon run typecheck
  • git diff --check

Notes

I also tried the full daemon suite. It initially failed because better-sqlite3 had been installed under Node 25 while the repo requires Node 24. After rebuilding better-sqlite3 under Node 24, the full suite command stopped producing progress and had to be stopped manually, so I am not listing it as passing.

@lefarcen lefarcen requested a review from Siri-Ray May 19, 2026 13:56
@lefarcen lefarcen added size/S PR changes 20-100 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix labels May 19, 2026
@VIVAAN-DHAWAN VIVAAN-DHAWAN marked this pull request as ready for review May 19, 2026 14:16
Copy link
Copy Markdown
Contributor

@Siri-Ray Siri-Ray left a comment

Choose a reason for hiding this comment

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

@VIVAAN-DHAWAN I reviewed the Codex JSON stream reconnect handling change and the added daemon parser coverage. The new helper keeps the existing timeout reconnect behavior, treats the reported stream disconnected before completion reconnect frame as recoverable status, and preserves fatal handling for non-reconnect errors that merely mention reconnect text. I also ran pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.ts, which passed with 23 tests. Thanks for the focused fix and the regression coverage.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Copy link
Copy Markdown
Contributor

@lefarcen lefarcen left a comment

Choose a reason for hiding this comment

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

Hey @VIVAAN-DHAWAN — the reconnect failure mode and targeted parser coverage are clearly described here. One small PR-body cleanup before this moves through maintainer review: could you add the ## Surface area checklist and a short ## Bug fix verification note for the red→green path, e.g. that the reported Codex reconnect frame now stays recoverable while the non-reconnect error case remains fatal? Your existing Summary and Verification sections already cover the why/what/validation, so no need to rename those.

@VIVAAN-DHAWAN
Copy link
Copy Markdown
Author

I will check on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/S PR changes 20-100 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex adapter treats transient reconnect JSON event as fatal run failure

3 participants