Repository navigation
Say why a refused Keychain read can't be fixed by signing in again - #599
Conversation
macOS's security tool exits 36 (errSecInteractionNotAllowed) when the Keychain won't hand an item to a process outside the logged-in desktop session: one started over SSH, or by a background server launched from there, as Codex's app-server can be. hey reported that as "exit status 36" with no hint, and the skill then told the person to run `hey auth login`, which writes to the same Keychain and changes nothing. - The store marks an exit status 36 from the keyring as ErrKeyringRefused, keeping the keyring's own message. - A refused read now carries a hint that rules out a new login and names the fix: run hey from Terminal on the Mac, or restart Codex's background server from there. Inside Codex's sandbox (CODEX_SANDBOX set), the hint says to let Codex run hey outside it. - A missing login inside Codex's sandbox says the sandbox may be hiding a saved one, instead of only "Run: hey auth login". - The skill no longer sends a person to `hey auth login` when `hey auth status` fails with a keyring error. - docs/agents.md explains both cases, and why `hey mcp` shows up as a closed connection.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟢 Approved
The narrowly scoped diagnostic changes preserve error compatibility, include focused regression coverage, and have no identified blocking issues.
No open issues 🎉
What changed in this PR
Improves hey-cli’s authentication diagnostics so agents distinguish refused Keychain access from a missing login, avoiding repeated sign-in attempts that cannot fix the problem.
Changes:
- Classifies Keychain exit status 36 while preserving existing error messages and exit codes.
- Adds recovery hints for refused reads and missing credentials inside Codex’s sandbox, with regression coverage.
- Updates agent guidance and documentation to explain recovery and MCP startup failures.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
skills/hey/SKILL.md |
Relays storage-error hints instead of recommending another login. |
internal/cmd/root.go |
Uses sandbox-aware guidance for missing credentials. |
internal/cmd/root_test.go |
Covers sandbox hints and isolates sandbox environment state. |
internal/cmd/auth.go |
Adds refusal hints to authentication-status errors. |
internal/auth/store.go |
Classifies Keychain refusal without changing its error text. |
internal/auth/store_test.go |
Tests refusal classification with simulated and real exit errors. |
internal/auth/auth.go |
Centralizes storage-error and missing-login hints. |
internal/auth/auth_test.go |
Covers refusal hints and unchanged handling of other storage failures. |
docs/agents.md |
Documents Keychain recovery and MCP connection-closed errors. |
Review effort: Balanced
![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: 306174085f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Exit status 36 comes from the desktop session, not the sandbox, so letting Codex run hey outside its sandbox can't clear it on its own. Inside the sandbox the hint now names that step first, then the restart.
|
Not doing this — from our own pre-merge review (gpt-6.1-sol, high), on 3061740: it's true that a refusal hitting the SDK path now comes back as |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Review threads: 2 resolved (1 fixed, 1 declined with the reasoning in the thread). |
Why
A customer on macOS can sign in and use hey in Terminal, but inside Codex every
heycommand fails withread credentials from system keyring: exit status 36, andhey mcpfails Codex's startup withhandshaking with MCP server failed: connection closed. The skill then tells them to runhey auth login, so they loop. Allowing hey outside Codex's sandbox with~/.codex/rules(as #596 proposed and Support sent by hand) didn't help:codex execpolicy checksaidallow, and the command still got exit status 36.Exit status 36 is
errSecInteractionNotAllowed. The Keychain gives it to a process outside the Mac's logged-in desktop session, whether or not it's sandboxed. Codex 0.163+ runs every command and MCP server from a shared background server (codex app-server, started withsetsid, kept running after Codex quits). If that server was started from such a session, Codex can't read the Keychain from anywhere. A rules file can't fix that, and neither can a new login or restarting Codex.Reproduced on macOS 27 with Codex 0.164.0, against a scratch
CODEX_HOMEand a scratch keychain item, never the real HEY login:security find-generic-password -wrunsssh localhost)prefix_rule(runs unsandboxed)codex app-server daemon restartfrom the desktop sessioncodex --no-daemonWith the server started from SSH, the same TUI session got the customer's exact errors:
hey auth status --jsongavecould not read authentication status: read credentials from system keyring: exit status 36, and Codex loggedMCP server startup failed server_name="hey" error=handshaking with MCP server failed: connection closed.hey mcp's stderr, which Codex keeps in~/.codex/logs_2.sqlite, saidcould not read stored credentials: … exit status 36. After a restart from the desktop session,hey mcploggedMCP server running on stdio tools=6 read_only=true.What changes
Classify the refusal.
store.loadFromKeyringmarks a keyring error whose exit status is 36 asErrKeyringRefused. The keyring's own message is kept unchanged, so existing scripts and the skill'scould not read stored credentialsmatch still work.Hint instead of a login.
CredentialStorageErrorturns a refused read into anapierr.Error. It is used by data commands, throughcredentialLoadError, and byhey auth status. Its code isunknown, exit 1: unchanged where the preflight read caught it. On the rarer path where an SDK request's re-read is refused, it wasapi/ 7 before. The hint rules out a new login and names the fix:codex app-server daemon restartin Terminal, then restart Codex.Inside Codex's sandbox (
CODEX_SANDBOXset), it first says to let Codex run hey outside the sandbox, then gives the restart, since the refusal comes from the session. Any other storage failure keeps today's plain error.A missing login inside Codex's sandbox. On macOS 27 a sandboxed read gets exit status 44, which reads as "Not logged in". With
CODEX_SANDBOXset, the hint now says the sandbox may be hiding a saved login: checkhey auth statusin Terminal first. Outside the sandbox it still saysRun: hey auth login. This is the misdiagnosis Let Codex run hey's read commands outside its sandbox #596's verification showed.Skill. It no longer tells the person to run
hey auth loginwhenhey auth statusitself fails with a keyring error. It relays the error's hint and stops.docs/agents.md. A short "Keychain access from agents" section covers both cases, and explains why
hey mcpshows up as a closed connection.Not done
hey mcpstill checks the login before the MCP handshake, so a client still sees only "connection closed". Serving the error from each tool call would surface the hint in the client. That is the area Give hey mcp its own sign-in, and send a draft only on the person's confirmation #598 reworks, so it belongs there or after it.hey setup codexstill writes no rules file. Let Codex run hey's read commands outside its sandbox #596 was closed in favour of a hand-made~/.codex/rules/hey.rules. Rules are still needed for sandboxed CLI commands, but they don't help with exit status 36.Verification
TestKeyringFailures/refusal, which covers a fake exit status 36 and a real*exec.ExitErrorfromsh -c 'exit 36', with exit 1 as the control;TestAKeychainRefusalSaysSigningInAgainWontHelp, through the status and access-token paths, inside and outside the sandbox;TestNotLoggedInHintInsideCodexSandbox;TestRequireAuthPromptsOnlyWhenInteractiveAndStyled/inside_Codex's_sandbox….TestAnUnclassifiedStorageFailureCarriesNoHintpins that only a refusal is classified.CODEX_SANDBOX="", so it still passes when run inside Codex.hey auth status --jsonandhey search. Under Codex's seatbelt withCODEX_SANDBOX=seatbelt,hey searchprints the sandbox hint, exit 3.make test,make lint,fmt-check,vet,check-surface,test-sync-skillsandcheck-release-locksteppass locally.