feat(mac): proxy config + quota window routing root-cause fix - #58
feat(mac): proxy config + quota window routing root-cause fix#58Cmochance wants to merge 3 commits into
Conversation
Proxy configuration (macOS): - Add Settings UI for HTTP/HTTPS/SOCKS5/SOCKS5h proxy configuration - Store proxy_state.json under per-platform runtime dir (macos/) - Inject proxy into reqwest HTTP client (ChatGPT plan/quota API) - Inject proxy env vars into codex app-server and login subprocesses - Inject proxy env vars into update-check curl subprocess - Invalidate cached HTTP client on proxy change for immediate effect - Validate proxy URL via reqwest::Proxy::all before saving - reqwest: enable socks feature for SOCKS5 support Quota window routing: - Fall back to reset_at distance when window_minutes is missing in /wham/usage payload (observed in production). 5h window resets within hours, weekly resets in days; classify by 6h threshold. - Apply reset_at fallback across all three data paths: chatgpt_api, codex_app_server, session_usage - Show "Xd Yh Zm" reset countdown when reset is more than a day out (previously "115h 20m" for weekly resets)
Surfaced by #57: formatting drift passed CI because no fmt check ran. rustfmt is not in dtolnay/rust-toolchain's minimal profile, so declare the component explicitly.
The raw /wham/usage schema reports window length as limit_window_seconds (seconds) — the window_minutes field this app expected only exists in Codex-CLI-converted payloads (session JSONL / app-server), verified against openai/codex codex-backend-openapi-models and a live capture. HTTP-refreshed quota therefore always fell back to position guessing, which is the real root cause behind weekly quota rendering in the 5h slot. Parse either field (seconds converted exactly like the CLI's window_minutes_from_seconds), keep the reset_at-distance heuristic as a last resort for payloads missing both fields, and route month-scale windows (the free plan's 30-day quota, 43200 min) to the weekly slot instead of the position fallback mislabeling them as 5h. Proxy hardening: set both upper- and lower-case proxy env vars for subprocesses (curl only honors lowercase http_proxy), explicitly exclude loopback via NO_PROXY (reqwest/curl do not exclude it by default), and document that a credentialed proxy URL is stored in plain text.
![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: e757c486cd
ℹ️ 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".
| //! 仅在 macOS 上启用：Windows 与 Linux 走 reqwest 默认行为（读环境 | ||
| //! 变量代理），不做应用层代理配置。schema + IO 是平台无关的中性 | ||
| //! 契约，按 AGENTS.md 平台隔离原则放在 shared/；reqwest 注入与 | ||
| //! `apply_proxy_env` 的 `cfg(target_os = "macos")` 限定负责把行为 | ||
| //! 收窄到 mac。 |
There was a problem hiding this comment.
This new shared module implements explicitly macOS-only persistence, caching, path selection, and subprocess environment mutation rather than an unavoidable cross-platform contract. Keeping this behavior under src-tauri/shared/** couples every target to platform-specific runtime machinery; retain only genuinely neutral payload types in shared code and place the implementation under src-tauri/mac/**.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| // codex login 的 OAuth 浏览器流程本身由浏览器走系统代理，但 codex | ||
| // CLI 本地起 callback server / 走 token exchange 时也可能发请求， | ||
| // 注入代理 env 与 app-server 路径保持一致。 | ||
| crate::shared::proxy::apply_proxy_env(&mut command); |
There was a problem hiding this comment.
When a configured proxy is required to reach OpenAI, only the login command receives apply_proxy_env; fetch_account_via_app_server still creates its child through build_app_server_command, which sets only CODEX_HOME. Consequently, whenever the proxied direct HTTP fast path fails and refresh falls back to app-server, that fallback connects directly and fails in the region-blocked environment this feature targets. Apply the macOS proxy environment while constructing the app-server command as well.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| if delta <= RESET_AT_CLASSIFY_THRESHOLD_SECONDS { | ||
| Some(QuotaSlot::FiveHour) | ||
| } else { | ||
| Some(QuotaSlot::Weekly) |
There was a problem hiding this comment.
For payloads missing both window-length fields, a weekly window enters this branch during the final six hours before its reset and is classified as FiveHour. Remaining time cannot distinguish window duration because any weekly window eventually has fewer than six hours left, so quota data predictably moves into the wrong dashboard slot near every reset; retain the positional fallback or use duration metadata rather than this threshold.
Useful? React with 👍 / 👎.
| let Ok(serialized) = serde_json::to_string_pretty(state) else { | ||
| return; | ||
| }; | ||
| let _ = fs::write(path, format!("{serialized}\n")); |
There was a problem hiding this comment.
If this write fails because the runtime directory is read-only, the disk is full, or another filesystem error occurs, the result is discarded and set_proxy_state still updates its in-memory cache; set_proxy_config therefore returns success and the UI says the proxy was saved, but it silently disappears after restart. Return an AppResult from the persistence path and only update the cache/report success after the file has been written.
Useful? React with 👍 / 👎.
| pub fn apply_proxy_env(command: &mut Command) { | ||
| if let Some(url) = read_proxy_state_cached().effective_url() { |
There was a problem hiding this comment.
When the app was launched from a shell or service that already supplied HTTP_PROXY, HTTPS_PROXY, or ALL_PROXY, clearing this setting leaves those variables untouched: subprocesses inherit them because this function becomes a no-op, and the rebuilt reqwest client can again discover environment proxies. The UI nevertheless reports “direct connection,” so requests continue through the inherited proxy after Clear; represent an explicit disabled state and remove inherited proxy variables from children and disable reqwest environment proxy discovery for that state.
Useful? React with 👍 / 👎.
| export function setProxyConfig(proxyUrl: string): Promise<ProxyConfig> { | ||
| return invokeCommand<ProxyConfig>("set_proxy_config", { | ||
| payload: { proxy_url: proxyUrl }, | ||
| }); |
There was a problem hiding this comment.
In local preview mode, invokeCommand has no cases for get_proxy_config, set_proxy_config, or clear_proxy_config, so these new wrappers reach the default “Unsupported preview command” rejection. Startup silently swallows the read failure, while clicking Save or Clear always displays an error, making the new macOS settings row nonfunctional in the repository's supported front-end preview workflow; add mock state and cases alongside the existing settings command mocks.
Useful? React with 👍 / 👎.
| base64 = "0.22.1" | ||
| chrono = { version = "0.4.44", features = ["serde"] } | ||
| reqwest = { version = "0.12", default-features = false, features = ["blocking", "json", "rustls-tls"] } | ||
| reqwest = { version = "0.12", default-features = false, features = ["blocking", "json", "rustls-tls", "socks"] } |
There was a problem hiding this comment.
The tracked src-tauri/Cargo.lock was not updated with this feature change and contains neither reqwest's optional tokio-socks dependency nor a corresponding dependency edge. Builds that require the committed lockfile, such as cargo build --locked or reproducible/offline packaging, therefore cannot resolve the newly enabled feature without modifying the checkout; regenerate and commit the lockfile with the manifest change.
Useful? React with 👍 / 👎.
2 participants
Supersedes #57. The contributor's fork is intentionally left untouched (maintainer decision) — this PR carries @XLevon's work forward on a maintainer branch instead: the first commit is #57 rebased onto current main with the original authorship preserved.
Commits
f1b4c43— @XLevon's proxy config + quota window routing hardening (feat(mac): add proxy config and harden quota window routing #57), rebased onto main. The one conflict with feat(quota): show reset credit expiry details #56 (quota_summary_from_payload) was resolved keeping both behaviors;cargo fmtapplied on top.19f3255— CI: add acargo fmtgate (the fmt drift above showed CI had no such check;rustfmtcomponent declared explicitly since dtolnay's minimal profile omits it).e757c48— root-cause fix + proxy hardening on top of the rebase (details below).Quota routing: the actual root cause
window_minutesis not flaky on/wham/usage— it never existed there. Verified three ways:codex-backend-openapi-models::RateLimitWindowSnapshot) defines windows asused_percent/limit_window_seconds/reset_after_seconds/reset_at— nowindow_minutes."limit_window_seconds": 2592000on a free-plan account).window_minutes— because it is the Codex CLI's internal field name, converted from the backend's seconds (backend-client'swindow_minutes_from_seconds) before reaching JSONL / app-server surfaces.So HTTP-refreshed quota always fell back to position guessing, and a weekly window arriving in the primary slot rendered as "5h" — the symptom #57 observed. The fix parses
limit_window_seconds(converted exactly like the CLI does) withwindow_minutesstill honored for CLI-shaped payloads; #57'sreset_at-distance heuristic is kept as a last resort for payloads missing both fields.Bonus fix: the free plan's 30-day window (43 200 min, observed live and in local JSONL) now routes to the weekly slot explicitly — previously it fell through to the position fallback and rendered as "5h". Known limitation: the dashboard has no monthly slot, so free-plan quota shows under the weekly label.
Proxy hardening on top of #57
http_proxy).NO_PROXY=localhost,127.0.0.1set explicitly — reqwest/curl do not exclude loopback by default.docs/SECURITY.mdnotes that a credentialed proxy URL (http://user:pass@host) is stored in plain text inproxy_state.json.Verification
cargo test --lib: 153 passed (includes 3 new tests:limit_window_secondsrouting from a raw-shape payload, free-plan monthly window, 43 200 slot routing)cargo fmt --check/tsc --noEmit/vitest run(3 passed) /npm run buildall green🤖 Generated with Claude Code