Skip to content

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

Merged
mrcfps merged 1 commit into
nexu-io:mainfrom
VIVAAN-DHAWAN:codex/handle-codex-stream-reconnect-2268
May 22, 2026
Merged

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

Conversation

@VIVAAN-DHAWAN
Copy link
Copy Markdown
Contributor

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
Contributor Author

I will check on it

@mrcfps mrcfps merged commit 6273874 into nexu-io:main May 22, 2026
14 checks passed
@open-design-bot
Copy link
Copy Markdown
Contributor

🎉 📡 You just leveled up to Giotto

Giotto card for @VIVAAN-DHAWAN

📡 ✨ Sending steady signals.

🙌 Your contributions are sending a clear signal across the network: you care about making Open Design better. Keep transmitting.

💛 Thanks for helping Open Design move forward. Keep building in the open. 🚀


📊 Rank #196 among 100+ contributors

🔗 Share on X (English) · 分享到 X(中文)

@VIVAAN-DHAWAN
Copy link
Copy Markdown
Contributor Author

Thanks

@VIVAAN-DHAWAN VIVAAN-DHAWAN mentioned this pull request May 22, 2026
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

4 participants