Skip to content

Fix Codex login detection when status is written to stderr - #243

Merged
milind-soni merged 1 commit into
milind-soni:mainfrom
NuCl34R:agent/fix-codex-login-status
Aug 19, 2026
Merged

Fix Codex login detection when status is written to stderr#243
milind-soni merged 1 commit into
milind-soni:mainfrom
NuCl34R:agent/fix-codex-login-status

Conversation

@NuCl34R

@NuCl34R NuCl34R commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pass stderr alongside stdout through the shared execCli helper
  • recognize a successful Codex login status on either output stream
  • reproduce the Codex CLI 0.147.0 behavior in the fake CLI
  • retain regression coverage for older versions that wrote the status to stdout

Root cause

codex login status exits successfully, but Codex CLI 0.147.0 writes Logged in using ChatGPT to stderr. OpenMausBot only inspected stdout, so it reported a valid session as authenticated: false and unnecessarily prompted the user to run codex login.

Impact

OpenMausBot now recognizes existing Codex sessions correctly. The check still fails closed when the command returns an error or reports Not logged in.

Validation

  • pnpm exec vitest run server/drivers/codex.test.ts: 19 tests passed
  • pnpm typecheck: passed
  • pnpm exec vitest run --reporter=dot: 106 test files passed, 1,029 tests passed, 8 skipped
  • git diff --check: passed

pnpm lint remains red because of existing anti-slop violations outside the diff. The targeted lint run did not report any line changed by this PR.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Codex login detection across CLI output streams.
    • Correctly recognizes authenticated status from older Codex versions.
    • Improved handling of command-line error output and text encoding.
  • Tests
    • Added coverage for login status reported through standard output.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 50a2aea8-b04e-411e-84de-da0923735cd6

📥 Commits

Reviewing files that changed from the base of the PR and between 4ea27e7 and db0bc9c.

📒 Files selected for processing (4)
  • server/drivers/codex.test.ts
  • server/drivers/codex.ts
  • server/procs.ts
  • server/testing/fake-codex-app-server.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The CLI wrapper now forwards stderr. Codex authentication detection checks both output streams for a standalone login status. The fake CLI and tests cover login status reported through stdout.

Changes

Codex authentication compatibility

Layer / File(s) Summary
Expose CLI stderr
server/procs.ts
execCli now uses UTF-8 output and passes stdout and optional stderr to its callback.
Detect login status across output streams
server/drivers/codex.ts, server/testing/fake-codex-app-server.ts, server/drivers/codex.test.ts
The driver accepts a line beginning with logged in from stdout or stderr. The fake CLI defaults successful login output to stderr and supports stdout output through logged-in-stdout. The test verifies authenticated availability for that mode.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to db0bc

The change makes Codex login detection recognize successful status written to either output stream while preserving the existing failure behavior. It is merge-ready after normal checks, with no actionable merge-blocking risk remaining.

Possibly related PRs

Suggested reviewers: milind-soni, aivsomkar, claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main Codex login detection fix.
Description check ✅ Passed The description covers the changes, root cause, impact, validation results, and known lint limitation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@NuCl34R NuCl34R changed the title Corriger la détection de connexion Codex sur stderr Fix Codex login detection when status is written to stderr Aug 19, 2026
@NuCl34R
NuCl34R marked this pull request as ready for review August 19, 2026 00:41
@milind-soni

Copy link
Copy Markdown
Owner

Looks great, Thanks for the addition

@milind-soni
milind-soni merged commit 31792a3 into milind-soni:main Aug 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants