Conversation
…ewrite `unlockRateLimitGate` reads every gate field in both spellings it arrives in: rate_limit and rateLimit, limit_reached and limitReached, spend_control and spendControlReached, rate_limit_reached_type and rateLimitReachedType. The usage window was the exception: only the JSON-RPC `usedPercent` counted as plain-quota evidence. A web usage snapshot spells it `used_percent`, so a genuinely exhausted snapshot never showed the plain quota as the reason and its flags stayed closed. Both spellings now count; below 100% and with a reached spend control the flags still stay as sent. On dev the only caller is the app-server RPC path, whose payloads use `usedPercent`, so its behaviour is unchanged. The web snapshot shape matters to the send-unblock intercept, which runs the same rewrite over `/backend-api/wham/usage`.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
ChangesQuota gate usage detection
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change adds snake_case quota handling while preserving existing gate protections, and tests cover the helper’s threshold behavior. The upstream notification shape is not established in the repository, but no concrete merge-blocking issue is evidenced; this appears mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change recognizes another spelling of quota usage without adding a new caller or credential access. Spend-control and other explicit restrictions still prevent gate opening. Remaining uncertainty concerns upstream message shapes, not an established security regression. 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)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
@coderabbitai review |
|
…un#6463) Read used_percent as exhaustion evidence alongside usedPercent. Preserve usage values and existing spend-control and non-quota blockers. Carries lidge-jun#6463 by @lcxhh521. Co-authored-by: lcxhh521 <59329914+lcxhh521@users.noreply.github.com>
|
Closing as landed on dev via f5572a0. Thanks @lidge-jun for carrying it and keeping the credit. |
Brings the branch level with dev (100 commits) and resolves four conflicts: - `src/cli/chatgpt-command.ts`: dev's carry of lidge-jun#6453 checks the discovered bundle's trust before either relaunch path. It now runs after the intercept listener probe and the shim's binary resolution, and before the restore watcher guard, so the intercept relaunch is validated too. An intercept-only launch reports "launch ChatGPT" rather than "launch the shim". - `src/chatgpt/app-server-shim/gate-rewrite.ts`: dev's carry of lidge-jun#6463 makes the same `used_percent` change; only the comment differed, and dev's wording is kept. - `structure/config.md` and the ChatGPT desktop guide: our `chatgptDesktop` fields alongside dev's `claudeCode.subagentModelForce` text and restore trust paragraphs. The command-child fixture from lidge-jun#6453 now stubs the intercept status and watcher modules, so its status and restore scenarios never probe this machine's listener, launchd agent, keychain or running proxy. A new scenario covers restore refusing while the watcher is loaded. The desktop-unblock layout entries share lines, which keeps `tests/fixtures/test-layout-expected.json` under the 2000-line ratchet as dev's packed entries already do.
Brings the branch level with dev (100 commits) and resolves four conflicts: - `src/cli/chatgpt-command.ts`: dev's bundle trust check before either relaunch path (0358e72, from lidge-jun#6453) now runs after the intercept listener probe and the shim's binary resolution, and before the restore watcher guard, so the intercept relaunch is validated too. An intercept-only launch reports "launch ChatGPT" rather than "launch the shim". - `src/chatgpt/app-server-shim/gate-rewrite.ts`: dev already has the same `used_percent` change (f5572a0, from lidge-jun#6463); only the comment differed, and dev's wording is kept. - `structure/config.md` and the ChatGPT desktop guide: our `chatgptDesktop` fields alongside dev's `claudeCode.subagentModelForce` text and restore trust paragraphs. The bundle-trust command-child fixture now stubs the intercept status and watcher modules, so its status and restore scenarios never probe this machine's listener, launchd agent, keychain or running proxy. A new scenario covers restore refusing while the watcher is loaded. The desktop-unblock layout entries share lines, which keeps `tests/fixtures/test-layout-expected.json` under the 2000-line ratchet as dev's packed entries already do.
1 participant
Summary
unlockRateLimitGate(src/chatgpt/app-server-shim/gate-rewrite.ts) reads every gate field in both spellings it can arrive in:rate_limit/rateLimit,limit_reached/limitReached,spend_control/spendControlReached,rate_limit_reached_type/rateLimitReachedType. The usage window was the one exception: only the JSON-RPCusedPercentcounted as plain-quota evidence.A web usage snapshot (
/backend-api/wham/usage) spells the windowused_percent. Since the flags open only with plain-quota evidence, a genuinely exhausted snapshot never showed the plain quota as the reason and itsrate_limitflags stayed closed. This PR counts both spellings. Below 100%, or with a reached spend control, the flags still stay as the server sent them.devthe only caller is the app-server RPC path, whose payloads useusedPercent, so its behaviour does not change.devwith that intercept makes its "exhausted usage snapshot opens the gate" tests fail without this line.Verification
bun run typecheck,bun run structure:check,bun run privacy:scan: pass.bun test ./tests/clients/desktop-app-server-shim.test.ts ./tests/clients/desktop-app-server-shim-launcher.test.ts ./tests/clients/desktop-chatgpt-config.test.ts: 43 pass, 0 fail.unlockRateLimitGateon a web-shaped snapshot:used_percent: 100opens the flags and leaves the window as sent;used_percent: 42stays closed; a reachedspend_controlkeeps them closed at 100%. Removing the new line fails the first one.Checklist
structure/clients/chatgpt-desktop.mdnames both spellings.)Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
used_percentnow correctly identify exhausted quotas and open rate-limit gates when no other blocking condition applies.