fix(refresh): replace codex exec fallback with app-server JSON-RPC#33
Merged
Conversation
The per-card Refresh button's fallback path used to spawn `codex exec
"Reply with the single word OK."` whenever the direct ChatGPT HTTP
refresh failed. That path:
* burned ~30–90 s on the user's clock (real LLM round-trip)
* consumed real ChatGPT quota
* relied on parsing the resulting session JSONL to extract rate
limits, which only worked because codex happened to write that
data on every request
It is replaced by `codex app-server`'s JSON-RPC interface:
`account/read` (refreshToken=true) for plan tier + auth.json refresh,
followed by `account/rateLimits/read` for the live primary/secondary
rate-limit windows. Same backend endpoint, no LLM round-trip, returns
within seconds.
Implementation lives in `shared/runtime/codex_app_server.rs`: a stdio
JSON-RPC 2.0 client driving the upstream protocol verified against
`openai/codex` `codex-rs/app-server` (newline-delimited JSON,
mandatory `initialize` + `initialized` handshake, monotonic id
matching with per-request and overall session deadlines, ChildGuard
that always kills + reaps on every exit path, dedicated reader and
stderr-drain threads to avoid pipe back-pressure deadlocks).
Mac and Windows `process.rs` build the `Command` (Windows hides the
console window via the existing `hide_console_window` helper); the
shared client runs the rest. The `PlatformHooks` trait method
renamed `run_codex_auth_refresh` → `fetch_account_via_app_server`
with a return type that surfaces both plan + quota in one shot, so
both `mac/runtime/refresh_runtime.rs` and
`win/runtime/refresh_runtime.rs` drop the post-spawn JSONL scan and
sandbox-cleanup dance.
Requires `codex` ≥ 0.130.0 on the fallback path; older CLIs surface
the new `APP_SERVER_METHOD_UNSUPPORTED` error so the user can upgrade
instead of hitting an opaque hang.
Also: hardened three `discover_real_codex_cli_path_*` macOS tests
that mutate `HOME` / `PATH` against parallel-execution races by
routing them through the existing `env_guard()` mutex (these were
flaky before this PR; touching the same file made it cheap to fix).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
用户手测清单(push 后请在本机跑一遍)PR-1 替换的是"刷新按钮失败兜底"的路径,最常见触发条件是网络抖一下让直连 HTTP 失败。建议测试: 1. 正常路径(未变)
2. 兜底路径(核心改动)
3. 老版 codex CLI 兼容性
4. 已登录账号 token 失效
5. 没装 codex CLI
CI 全绿后,等你手测确认 → squash merge。 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d493819846
ℹ️ 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".
`handleRefreshProfile` was missing the symmetric counterpart of `handleLoginProfile`'s `isRefreshPending(profile)` guard. With the old code, clicking Refresh on a profile whose login was still in flight would push it onto the refresh queue; once the worker drained, refresh and login could race on writing the same per-profile `auth.json`. Atomic writes prevented torn data, but whichever lost the race got silently overwritten. Add `state.loginActiveProfile === profile` to the early-return. Cross-profile Refresh during a login is still allowed (different sandboxes, different `auth.json`s). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
追加 commit:refresh ↔ login 同 profile 互锁补一条 3 行守卫到 增量手测建议
|
Codex bot raised a P2 on PR #33: the new fallback's hard 8 s deadline on every JSON-RPC call could fail on the exact "slow network" scenarios it is meant to recover from, since `account/read` with `refreshToken: true` chains an OAuth refresh round-trip + account read on the server side, and either leg can legitimately take 5–15 s on a high-latency link. Split the per-method budget so `account/read` gets 25 s (covers chained OAuth refresh + read) while the lighter `account/rateLimits/read` GET keeps a 15 s ceiling — matching `chatgpt_api.rs:109` `HTTP_TIMEOUT`. Session ceiling raised to 60 s with margin to cover handshake + both calls at their per-method maxima. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Cmochance
added a commit
that referenced
this pull request
May 10, 2026
Bumps `package.json` to 1.5.8 (`version-sync.mjs` propagates to Cargo.toml + lockfiles) and stamps the 1.5.8 CHANGELOG entry. Patches landing as 1.5.8: - **#33** — Replace the legacy `codex exec "Reply with the single word OK."` refresh fallback with `codex app-server`'s JSON-RPC `account/read` + `account/rateLimits/read`. No more burning user quota for an LLM round-trip just to provoke a session-file write. Also closes a same-profile Refresh-vs-Login race and tunes the RPC timeouts so slow-network users still benefit from the fallback instead of tripping `APP_SERVER_TIMEOUT`. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
codex exec "Reply with the single word OK."fallback in the per-card Refresh button withcodex app-server's JSON-RPCaccount/read+account/rateLimits/read. Same backend data, no LLM round-trip, no quota consumed, returns within seconds instead of 30–90 s.codex exechad to provoke a session write to leak rate-limit data.src-tauri/shared/runtime/codex_app_server.rsimplements a stdio JSON-RPC 2.0 client (newline-delimited frames, mandatoryinitialize+initializedhandshake, monotonic id matching, per-request + overall session deadlines,ChildGuardthat always kills + reaps, dedicated reader + stderr-drain threads to avoid pipe back-pressure deadlocks). Verified againstopenai/codexcodex-rs/app-serverupstream protocol.PlatformHooks::run_codex_auth_refresh(AppResult<()>) renamed tofetch_account_via_app_server(AppResult<AppServerSnapshot>); both mac and Windowsprocess.rsbuild aCommandand the shared client drives it.discover_real_codex_cli_path_*tests that mutateHOME/PATHagainst parallel-execution races by threading them through the existingenv_guard()mutex.Compatibility
Requires
codex≥ 0.130.0 on the fallback path (the primary HTTP path is unchanged and works on all versions). Older CLIs surface a newAPP_SERVER_METHOD_UNSUPPORTEDerror so users get an upgrade hint instead of an opaque hang.Why
For users in slow-network regions (or whenever the direct HTTP refresh hit a transient 401 / GFW interference), every Refresh click that fell through to the legacy path wasted real ChatGPT quota and made the user wait 30–90 s for a model to reply with one token. The app-server protocol exposes the same data without the model spend.
Test plan
cargo test --libmac: 84/84 pass (10 + 3 new tests on the JSON-RPC client cover routing, plan extraction, error mapping, partial-data branches).sudo pfctlor unplug Wi-Fi briefly), verify fallback now completes in seconds without a quota deduction.codex< 0.130.0 — verify the user seesAPP_SERVER_METHOD_UNSUPPORTEDtoast hinting at upgrade, not a hang.Follow-ups (not in this PR)
drain_stderr_in_backgroundwrites the child's stderr straight to/dev/null; on a child crash mid-handshake the user seesAPP_SERVER_PIPE_CLOSEDwith no context. Buffering the last N KB and appending to error messages is a small structural improvement.try_refresh_via_chatgpt_apiswallows every HTTP error and falls through. For some cases (e.g. matching the relogin-required token signatures) it could short-circuit toAUTH_REFRESH_RELOGIN_REQUIREDdirectly without a second app-server round-trip.mac/runtime/refresh_runtime.rsandwin/runtime/refresh_runtime.rsare now near-mirror images of each other — candidate for a shared helper in a separate refactor.🤖 Generated with Claude Code