Skip to content

fix(rate-limits): keep Codex PTY reset text for weekly-only plans#8643

Open
moseoh wants to merge 1 commit into
stablyai:mainfrom
moseoh:fix/codex-pty-weekly-reset-text
Open

fix(rate-limits): keep Codex PTY reset text for weekly-only plans#8643
moseoh wants to merge 1 commit into
stablyai:mainfrom
moseoh:fix/codex-pty-weekly-reset-text

Conversation

@moseoh

@moseoh moseoh commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

When both the backend and app-server RPC paths fail, Orca falls back to spawning the Codex CLI in a hidden PTY, sending /status, and parsing the rendered output. That parser reads the 5h limit / Weekly limit lines by label, but the extracted reset text was only ever attached to the session window:

if (resetMatch && session) {
  session.resetDescription = resetMatch[1].trim()
}

Codex plans without a 5h session bucket (e.g. the current Pro plan) have no 5h limit line in the Codex CLI's /status output, so the parse yields session === null — and the Resets in … text the Codex CLI did print was silently dropped, leaving the weekly window without any reset info. This fix falls back to the weekly window when no session window exists.

This is independent of (and complementary to) #8493: the PTY path classifies windows by their text labels, so weekly-only output already lands in the weekly slot today — only the reset text was lost.

Screenshots

No visual change beyond the weekly usage row regaining its reset text when the PTY fallback serves a weekly-only plan.

Testing

  • pnpm lint (pre-commit oxlint/react-doctor/oxfmt on changed files; repository lint config)
  • pnpm typecheck
  • Focused Vitest: codex-fetcher-pty-settle.test.ts, codex-fetcher.test.ts — 20/20 pass
  • pnpm build (verified on the same workstation earlier today; this change is a 4-line main-process patch with no build-surface impact)
  • Added a regression test: weekly-only PTY output keeps resetDescription on the weekly window

AI Review Report

Reviewed the change against the enclosing parsePtyStatus flow and its two callers (settle-timer path and exit path):

  • Both call sites consume { session, weekly } from the same parser, so the fallback applies uniformly.
  • The reset regex captures the first resets… text in the output; with both windows present the behavior is unchanged (session keeps priority), so plans with a 5h bucket are unaffected.
  • Cross-platform: no platform-specific code touched; the PTY fallback itself already handles win32/WSL spawn differences upstream of this parser. No shortcuts, labels, or path handling involved.

Security Audit

No new input surfaces: the change reorders which already-parsed window object receives an already-extracted string from the existing regex. No command execution, path, auth, or IPC changes.

Notes

Follow-up candidate (intentionally not included here): RateLimitsService.refreshForCodexAccountChange gates the inactive-account snapshot cache on session existing, which will skip weekly-only snapshots once #8493 lands. That belongs with/after #8493, not this PR.

The PTY /status fallback parses '5h limit' and 'Weekly limit' lines by
label, but the extracted reset text was only ever attached to the
session window. Codex plans without a 5h session bucket (e.g. current
Pro) produce a weekly-only parse, so the reset time the CLI printed was
silently dropped. Fall back to the weekly window when no session window
exists.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

PTY rate-limit parsing now assigns reset descriptions to the available session or weekly window, preserving reset text for weekly-only plans. A new test exercises the PTY fallback, advances the settle timer, and verifies the weekly usage, reset description, null session, and ok status.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: preserving Codex PTY reset text for weekly-only plans.
Description check ✅ Passed The description includes all required sections and covers summary, testing, review, security, screenshots, and notes.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants