fix: read auth.json directly to avoid macOS sandbox panic#64
fix: read auth.json directly to avoid macOS sandbox panic#64msadikyildiz wants to merge 1 commit intoopenai:mainfrom
Conversation
On macOS, `codex login status` panics inside Claude Code's sandbox because the Rust `system-configuration` crate fails to access SCDynamicStore when PATH updates are blocked. Read ~/.codex/auth.json directly in getCodexLoginStatus() before falling back to the binary call. The file contains the same tokens that the CLI would check, so the result is identical without spawning a process that may panic. Falls back to the existing `codex login status` call if the file is missing or unreadable, preserving current behavior for all other environments.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67787d376e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| // The file-based check should return loggedIn: true without | ||
| // ever calling `codex login status`. | ||
| assert.equal(result.loggedIn, true); |
There was a problem hiding this comment.
getCodexLoginStatus() checks getCodexAvailability() before it reads ~/.codex/auth.json, so this assertion only passes when a real codex binary is installed and runnable on PATH. In CI/local environments without Codex, result.loggedIn remains false, causing the new positive-path tests to fail even if the auth-file parsing logic is correct. Mock or fixture the Codex availability check so the test validates the new file-based behavior instead of host tooling state.
Useful? React with 👍 / 👎.
| // binary check. Since codex may or may not be installed in the | ||
| // test environment, we just verify it did NOT return loggedIn: true | ||
| // from the file-based path. | ||
| assert.equal(result.loggedIn, false); |
There was a problem hiding this comment.
This assertion is nondeterministic because the code path intentionally falls through to codex login status; if the machine running tests is already authenticated, result.loggedIn will be true and the test fails. That makes this test depend on external account state rather than the auth-file fallback behavior it is trying to verify. Stub the binary call (or assert a branch-specific signal) to keep the test deterministic.
Useful? React with 👍 / 👎.
1 participant
Summary
~/.codex/auth.jsondirectly ingetCodexLoginStatus()before spawningcodex login statusCloses #63
Problem
On macOS,
codex login statuspanics inside Claude Code's sandbox:This blocks all
/codex:setup,/codex:review, and/codex:taskcommands despite the user having valid auth tokens.What changed
getCodexLoginStatus()now tries to read~/.codex/auth.jsonfirst. If valid tokens exist (ChatGPT OAuth or API key), it returnsloggedIn: truewithout ever spawning the binary. The existingcodex login statuscall is preserved as a fallback for cases where the file is missing.Test plan