Repository navigation
fix(rate-limits): stop driving a hidden Codex TUI to read usage - #23806
brennanb2025 merged 6 commits into
Conversation
When the headless Codex usage call failed, Orca opened a hidden interactive Codex, typed /status and pressed Enter without reading the screen, then killed it after 15 s. If Codex showed its "Update available" prompt, that Enter picked "Update now", Codex started its installer, and the 15 s kill interrupted it, leaving the global install broken. Drop the hidden-terminal fallback. When the headless call fails with a non-sign-in error, read the same usage from the HTTP endpoint Orca already calls on WSL and for the 5-hour window, so a usage check can no longer answer any Codex startup screen. Fixes #17415
Also drop comments that still described the removed hidden-terminal fallback.
The changed-code gate rejects the new type assertions this PR added.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (options?.signal?.aborted) { | ||
| return abortedCodexRateLimitResult() | ||
| } | ||
| return backendResult ?? rpcFailure |
There was a problem hiding this comment.
If the RPC check fails and the HTTP endpoint returns a plan type but no rate-limit windows, the backend reader marks that response as successful. This line then returns empty usage instead of the RPC error, so the service replaces the last good numbers rather than retaining them during a failed refresh.
…ground-rate-limit-pty-interrupts-codex-s
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 11 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (22)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe Codex rate-limit fetcher removes the PTY probe and parser. For WSL homes, it tries HTTP usage retrieval before RPC. For non-WSL homes, it tries RPC first and then HTTP after an RPC failure. The fetch options and service callers no longer include Priority: ⬆️ High Severity of issue fixed: High Merge Risk: ⚪ Minimal · up to The change removes the hidden interactive Codex probe and replaces its fallback with HTTP usage retrieval. No concrete merge-blocking issue is established; merge after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to This removes automatic interaction with Codex update prompts. Background usage requests now run in more account and platform contexts, but selected-account and cancellation safeguards remain. No material security regression was established in the examined paths. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Deletes the hidden Codex TUI probe — removes
codex-pty-rate-limit-probe.ts,codex-pty-status-parser.ts, and their tests, plus the Codex-onlyallowPtyFallbackoption andshouldAllowCodexPtyFallback(). Claude's identically-named option is untouched. - Replaces the fallback with the existing HTTP usage reader — on a non-sign-in headless failure,
fetchCodexRateLimitsnow callsfetchBackendUsage(thechatgpt.com/backend-api/wham/usagereader, renamed fromfetchWslBackend) instead of driving codex. WSL homes keep their HTTP-first ordering and skip the second read; non-WSL homes returnbackendResult ?? rpcFailure. - Widens the retry to Windows and inactive-account previews, which previously had no fallback. Disclosed in the PR body.
- Fallout cleanups —
buildWslCodexCommanddrops its now-single-valuedisolateRpcStdioparameter, the auth-error comment incodex-auth-errors.tsis updated, and callers in the rate-limit service shed the removed option.
The core flow checks out: auth errors short-circuit before the fallback, abort paths return abortedCodexRateLimitResult() before any HTTP read, and a failed HTTP read falls back to the RPC failure. pnpm tc:node and the seven codex/rate-limit suites (57 tests) pass locally; tests that exercise a failing headless call use a temp home or stubbed fetch, so none touch a real login or the network.
ℹ️ The HTTP fallback cannot refresh an expired token
The removed interactive codex would refresh the ChatGPT token in auth.json before reading usage; getCodexBackendAuthHeaders reads the stored access_token verbatim. This only diverges when the headless app-server fails before it can refresh (for example an older codex rejecting the probe flags) and the stored token is stale — then the web read returns 401 and the RPC error surfaces instead of usage. The blast radius is bounded (the status bar keeps the last good numbers for 30 minutes and retries), and this is a defensible trade against the install-corruption bug, so I am not asking for a change — just flagging it since the PR body discusses the flag-incompatibility case but not token freshness.
Technical details
# Token freshness in the Codex HTTP fallback
## Affected sites
- `src/main/rate-limits/codex-fetcher.ts` — non-WSL branch reaches `fetchBackendUsage` after the RPC failure.
- `src/main/rate-limits/codex-backend-auth.ts:74` — `getCodexBackendAuthHeaders` only reads `auth.json`; it never refreshes.
- `codex-rpc-rate-limit-probe.ts:161` — the headless app-server refreshes tokens itself, so the gap is only the "app-server never started" failure mode.
## Why it is only a note
- `fetchViaRpc` failures that happen after startup carry a refreshed token, so HTTP succeeds.
- When the app-server cannot start, the fallback may read a stale token; the result is a failed refresh, not a wrong number.
- The PR already bounds the user impact (30-minute last-good retention) and the alternative (running the TUI) is exactly what this PR removes.
## Open question for the human (optional)
- Is a stale-token 401 worth a distinct, non-auth error message so the user knows a usage refresh failed vs. their install being broken?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
649f07aamergesorigin/maininto the branch. The net PR diff against the new merge base (fed1eca4) is byte-identical to the version reviewed at3ba125da— no files, behavior, or tests changed.
No new concerns. The prior review's observations (the HTTP fallback's inability to refresh an expired auth.json token, and the disclosed stale comment in the content-hashed relay asset) still stand and were not re-raised.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Review status: ready for reviewThe problem (#17415): when Orca's headless Codex usage call failed for a reason other than sign-in, Orca opened a hidden interactive Codex, typed What changes for the user: Orca's background usage check never opens an interactive Codex, so it can never answer a Codex prompt or start an update. If the headless call fails, Orca reads the same usage over the web with the token Codex already keeps in Fixed during review (commits befeca1 and 3ba125d, no production code change):
Deferred (minor, none user-visible except the first):
Found, but not caused by this PR: Claude's usage check has the same kind of hidden-terminal fallback ( What was verified:
Before: the usage menu shows "PTY timeout". Behind it, each refresh started a Codex update that was killed partway through.
After: the same failure shows the headless call's own error, and nothing else was launched.
After, with the headless call still refused but a ChatGPT token present: the web read fills in usage (Plus, 5h 37%, weekly 12%) in 87 ms. The app's two web requests (usage and reset credits) were answered inside the test app, so nothing reached the network.
What was not verified: Windows, Linux, WSL and SSH were not tested live. The web read was never tested against the real chatgpt.com with a real account token. The success case above answered the app's web request locally, and no fake token was ever sent to chatgpt.com. |
…ground-rate-limit-pty-interrupts-codex-s # Conflicts: # src/main/rate-limits/codex-fetcher.test.ts # src/main/rate-limits/codex-fetcher.ts
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Adopted the shared
spawnProcesschokepoint for the Codex RPC probe —fetchViaRpcnow hands the resolved CLI toresolveSpawninstead of pre-wrapping an npm.cmdshim incmd.exeviagetSpawnArgsForWindows; the explicitwindowsHideis dropped because the chokepoint pins it. WSL still spawnswsl.exethroughbuildWslExecArgs. - Updated the probe-termination comment to say the Windows launcher may now be
node(resolved shim) orcmd.exe(unresolved shim), not alwayscmd.exe. - Retargeted the Codex fetcher tests to the
spawnProcessseam —codex-fetcher*.test.tsnow mock../../shared/child-process/run-process(the budget/process-contract tests run the node stub through the real chokepoint) and drop thewin32-utils/getCmdExePathstubs, with the win32 spawn assertion now expecting the bare CLI rather thancmd.exe.
This delta comes from merging origin/main, which had already moved fetchViaRpc to the spawn chokepoint while retaining the PTY fallback; the merge combines that with this PR's removal of the PTY path. Behavior for the RPC probe is unchanged: POSIX spawns the resolved CLI exactly as before, and Windows now lets spawnProcess resolve an npm/pnpm .cmd shim to its real target (falling back to a properly quoted cmd.exe line for unrecognised shims). The ['pipe','pipe','pipe'] stdio the RPC reader depends on is preserved. No new concerns.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…ground-rate-limit-pty-interrupts-codex-s
What this fixes
Orca's background Codex usage check could break the user's Codex install. When a newer Codex was out, the check selected "Update now" in Codex's update prompt and then killed the installer 15 seconds later, leaving
codexmissing or failing withMissing optional dependency @openai/codex-<platform>. Nothing in Orca reported it; the status bar only said "Refresh failed".Who hits it: anyone signed in to Codex on macOS or Linux whose installed Codex is older than the latest release, whenever Orca's headless usage call fails for a reason other than sign-in (a slow or dropped network, a timeout, a Codex process that exits early). It fires on window focus, show and restore, and on the 15-minute timer while Orca is focused. It repeats on every check, because the install never finishes so Codex shows the prompt again. It breaks npm installs (
npm install -g @openai/codex) and would equally interrupt the standalone installer (curl … install.sh | sh) orbrew upgrade.Not affected: Windows, including WSL homes, which never ran this fallback. Terminal and chat panes, where the user answers Codex's prompts themselves. Claude, Gemini and other providers' usage checks.
This is not a regression from a recent PR: the hidden-terminal fallback has existed since the usage status bar was first added (#483).
ELI5
To show how much Codex quota you have left, Orca asks Codex quietly in the background. If that quiet question failed, Orca used to open an invisible Codex window, type
/status, and press Enter without looking at the screen. When Codex was showing "Update available — 1. Update now", that blind Enter started the update, and Orca then closed the invisible window mid-install. Now Orca never opens that invisible window; if the quiet question fails, it asks the same usage server directly over the web instead.What Changed
Before: usage check → headless
codex app-servercall → on a non-sign-in failure, a hidden interactive Codex in a hidden terminal →/status, Enter at +350 ms and again at +3 s → terminal killed at 15 s. With an update pending, Codex exits its screen and runs its installer inside that hidden terminal, and the 15 s kill interrupts it.After: usage check → headless
codex app-servercall → on a non-sign-in failure, a read of the HTTP usage endpoint (chatgpt.com/backend-api/wham/usage) with the access token Codex keeps inauth.json→ if there is no token or the request fails, the headless call's error is shown. No Codex screen is ever opened by a background check, so no Codex prompt can be answered by it.For the user: Codex is never updated, trusted, or otherwise changed by Orca's usage check. A failed check no longer spends up to 15 s in a hidden terminal. When the headless call fails but the web read succeeds, usage still shows, as it did when the hidden terminal succeeded.
Mechanism:
codex-fetcher.ts: removed the hidden-terminal path. A non-sign-in headless failure now falls back tofetchBackendUsage, the existing HTTP reader that WSL homes already try first (it wasfetchWslBackend). WSL homes skip the second HTTP attempt, since they already made it.codex-pty-rate-limit-probe.tsandcodex-pty-status-parser.ts(the screen driver and the/statustext parser) and their tests.allowPtyFallbackoption andshouldAllowCodexPtyFallback(). Claude's option of the same name is untouched.Why
Nothing needs the hidden terminal. Codex's own
/statusscreen gets its numbers by sending the sameaccount/rateLimits/readrequest Orca's headless call already makes, so the hidden terminal was a slow second try of the same request, not a different source. That request has been in Codex since 0.48 (October 2025), older than Orca's usage bar. Sign-in failures never reached the fallback, and Windows never had it. After a failed check the status bar keeps the last good numbers for 30 minutes while it retries, so a brief failure does not blank it.The HTTP read keeps the one case the fallback did cover. That case is Orca's headless Codex process failing while a fresh Codex would have succeeded. Orca already has this reader: WSL homes use it first, and checks already call it when the headless call returns no 5-hour window. It reads the same numbers without starting Codex.
Alternatives considered:
-c check_for_update_on_startup=false(Codex's own switch; the workaround in the issue). It removes only the update prompt. The same blind Enter still answers Codex's other startup screens: its "trust this folder?" screen defaults to Trust, which writes to the user's Codex config. An admin-managed Codex config can also force the update check back on.Deliberate widening, disclosed: Windows and the switcher's background previews of inactive accounts previously got no fallback at all (the hidden terminal could crash on Windows, and previews shouldn't start processes). Neither reason applies to an HTTP request, so both now get the HTTP retry too. It uses the same token and endpoint Orca already uses for WSL homes and for a missing 5-hour window.
Linked Issue
Fixes #17415
Visual Proof
Reproduced live in an isolated dev Orca. It used the real Codex 0.157.0 showing its update prompt, a fake
npmthat only logs and sleeps, a separateHOME,CODEX_HOMEand profile, and the headless call forced to fail. Nothing on the real machine was touched: the real~/.codexwas byte-identical afterwards.The status bar reads "Refresh failed" in both builds, because the test forces the headless call to fail and has no HTTP token. The difference is what happened behind it:
npm install -g @openai/codexstartedPTY timeoutBefore, from the fake npm's log (the parent chain is the hidden Codex started by Orca's main process, in Orca's
rate-limit-pty-cwdfolder):After: the fake npm's log is empty, and the Codex log shows only the headless call for each refresh.
Before:
After:
Testing
I manually tested these changes locally (macOS arm64, isolated dev app, before and after as above)
Automated tests added/updated
Rate-limit suites: 54 files, 649 tests pass. The hidden-terminal cases were replaced with HTTP-fallback cases, and each asserts that no hidden terminal is spawned. Coverage: the web read serves usage when the headless call fails; the headless error stays when there is no token; aborting skips the web read; WSL homes don't repeat the web read; sign-in errors don't fall back.
Run against the previous fetcher, the 4 new or changed fallback tests fail.
Tests where the headless call fails now use a temp Codex home or a mocked
fetch, so none can read the developer's realauth.jsonor reach the network.tscfor the main-process project passes. oxlint is clean on the changed files.Not tested live: Windows, Linux, WSL and SSH. Also not tested live: the success path where the web read returns usage after a headless failure (that needs a real account token); it is covered by the unit test above.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
auth.jsontoken and endpoint Orca already uses for WSL homes and for a missing 5-hour window. One process-spawning path is removed.config/relay-assets/node-pty-1.1.0-windows-pty-teardown-patch.cjsstill mentions the deleted Codex probe in a comment. It is a hashed relay asset, so editing it would make every SSH host reinstall its relay, and the mention was already inaccurate because that probe never ran on Windows. The matching test comment is updated.npm install -g @openai/codex@latest. Detecting and repairing a broken install is a possible follow-up, not part of this PR.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred). Scoped lint, main-process typecheck and rate-limit tests pass locally; the full runs are left to CI.