Add Codex auth TUI, earned resets, and automatic patch maintenance#1
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds rolling-auth synchronization, generation-aware Codex patching, safer installation and maintenance, a Textual monitoring and autoswitch TUI, deterministic asset generation, expanded documentation, CI, and comprehensive shell/Python test coverage. ChangesCodex Rolling Auth
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CodexAuthTUI
participant AutoSwitchEngine
participant ShellBackend
participant CodexAuthCLI
participant AuthState
User->>CodexAuthTUI: launch watch or auto mode
CodexAuthTUI->>ShellBackend: request snapshot
ShellBackend->>AuthState: read profiles and usage state
ShellBackend-->>CodexAuthTUI: return account snapshot
CodexAuthTUI->>AutoSwitchEngine: start policy loop
AutoSwitchEngine->>ShellBackend: refresh usage
ShellBackend->>CodexAuthCLI: run refresh
CodexAuthCLI->>AuthState: update profile state
AutoSwitchEngine->>ShellBackend: switch selected profile
ShellBackend->>CodexAuthCLI: run guarded switch
CodexAuthCLI->>AuthState: update active auth
AuthState-->>CodexAuthTUI: refreshed active profile state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc9d675226
ℹ️ 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".
| if (( count == 0 )); then | ||
| print_error "no earned resets available for profile: $name" | ||
| return 3 |
There was a problem hiding this comment.
Clear stale reset keys when cached credits reach zero
When a prior reset attempt times out before a terminal outcome, reset-$name.pending is intentionally left behind. If a later usage refresh shows availableCount as 0, this early return reports no credits but leaves that pending key on disk; when the account later earns another reset, the code below will reuse the old key and an alreadyRedeemed response can be treated as a successful reset without spending the new credit. Clear the pending key, or bind it to the current fingerprint/credit epoch, before returning on zero credits.
Useful? React with 👍 / 👎.
| if [[ ! -f "$source_dir/codex-rs/login/src/auth/manager.rs" ]] \ | ||
| || ! grep -Fq 'CODEX_AUTH_ROLLING_ENV_VAR' "$source_dir/codex-rs/login/src/auth/manager.rs"; then | ||
| if [[ "$no_fetch" != "1" ]]; then | ||
| git -C "$source_dir" fetch origin | ||
| fi | ||
| || ! grep -Fq 'maybe_sync_rolling_auth' "$source_dir/codex-rs/login/src/auth/manager.rs"; then | ||
| git -C "$source_dir" checkout --detach "$ref" |
There was a problem hiding this comment.
Always check out the requested patch source ref
When the source directory already contains a previously applied rolling-auth patch, this guard skips git checkout --detach "$ref". Since the default source dir is keyed only by the stock binary, codex-auth patch-codex --force --ref <other-ref> or CODEX_AUTH_PATCH_REF after an earlier build will silently build whatever HEAD was last used while later recording the requested ref in the marker. Check out or verify the requested ref even when the patch is already present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/codex-auth/help.sh (1)
76-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
maintainhas noaction_rows/tone mapping, so its help label falls back to "Repair" instead of a short verb tag.
usage_all()addsprint_help_item "maintain" "Repair shim + queue patch" "$width"(Line 81), but theaction_rowstable inprint_help_item()(Lines 108-136) has no"maintain"pattern. When no pattern matches, the loop's fallback setsaction="${desc%% *}", so the rendered action tag becomes "Repair" — inconsistent with the terse verb labels used everywhere else (reset,best,patch,resume,audit, etc.) and with the tonecasestatement, which also has no branch for it and thus defaults tomuted.🐛 Proposed fix
"export*"$'\t'"write" "help --all"$'\t'"more" + "maintain"$'\t'"tend" )- auth|login|import|wide|refresh|resume|roll|audit|dry|reset) + auth|login|import|wide|refresh|resume|roll|audit|dry|reset|tend) action_tone="warn" ;;Also applies to: 101-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/codex-auth/help.sh` around lines 76 - 91, Add a "maintain" entry to the action_rows mapping in print_help_item(), assigning a concise action tag such as "maintain", and add a corresponding tone case so it uses the intended non-muted styling. Keep usage_all()'s existing help text unchanged and align the new mapping with the established terse labels used by the other commands.
🧹 Nitpick comments (3)
lib/codex-auth/profiles.sh (1)
149-151: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRead-only commands can block up to 30s (and print a mutation error) on lock contention.
sync_active_profile_from_liveruns in a subshell that callsacquire_mutation_lock. When invoked from the mutating paths (cmd_use,cmd_save,cmd_login,cmd_use_if_current) the lock is already held, so re-entry is a no-op. Butcmd_list(here) andcmd_current(Line 364) call it without holding the lock, so the subshell triesflock -w "${CODEX_AUTH_MUTATION_LOCK_WAIT:-30}". Under contention a plainlist/currentwill stall for up to 30s and, on timeout,dieinside the subshell printsauth change already runningbefore the (still contained) subshell exits and listing continues.Consider running the sync with a non-blocking/try wait for these read paths so status commands never hang or surface a mutation error.
♻️ One option: short-circuit the wait for read-only callers
cmd_list() { ensure_dirs - sync_active_profile_from_live + CODEX_AUTH_MUTATION_LOCK_WAIT=0 sync_active_profile_from_live 2>/dev/null || true(and similarly for
cmd_current)Please confirm the intended contention behavior for read-only commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/codex-auth/profiles.sh` around lines 149 - 151, Read-only commands cmd_list and cmd_current can block on the mutation lock and emit an error. Update sync_active_profile_from_live or its read-only call sites to use a non-blocking/try lock acquisition for these commands, while preserving the existing re-entry behavior for mutating commands such as cmd_use, cmd_save, cmd_login, and cmd_use_if_current; on contention, status commands should continue without waiting or printing mutation errors.install.sh (1)
222-223: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReconsider the every-minute maintenance cadence.
* * * * *spawnscodex-auth maintain1440×/day.cmd_maintainshort-circuits quickly (lock/standalone checks), but a per-minute process launch is unusually aggressive for drift recovery; an hourly or few-minute schedule would cut process churn with negligible impact on recovery latency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@install.sh` around lines 222 - 223, Update the cron schedule in the install script’s cron_command definition to run maintenance less frequently than every minute, preferably hourly or at a reasonable multi-minute interval, while preserving the existing PATH and codex-auth maintain --quiet invocation.tui/src/codex_auth_tui/tui/dashboard.py (1)
140-143: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: make the paired iteration explicit with
strict=True.This branch only runs when
names == self._names, so item/account counts already match; addingstrict=Truedocuments that invariant and would surface a future divergence loudly instead of silently truncating.♻️ Optional
- for item, acc in zip(listview.query(AccountItem), snap.accounts): + for item, acc in zip(listview.query(AccountItem), snap.accounts, strict=True):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tui/src/codex_auth_tui/tui/dashboard.py` around lines 140 - 143, Update the paired iteration in the dashboard refresh branch to use strict zip semantics, changing the zip call within the method containing _flash_updated(snap, listview) to pass strict=True and explicitly enforce matching AccountItem and account counts.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 15: Update the actions/checkout@v4 step to set persist-credentials:
false, preventing the GitHub token from being stored in the repository’s Git
configuration.
In `@install.sh`:
- Around line 43-47: In the TUI file-copy loop, update the
`${tui_file#$tui_source/}` parameter expansion to quote the `$tui_source/`
prefix, preventing glob metacharacters in the source path from affecting
relative-path calculation.
---
Outside diff comments:
In `@lib/codex-auth/help.sh`:
- Around line 76-91: Add a "maintain" entry to the action_rows mapping in
print_help_item(), assigning a concise action tag such as "maintain", and add a
corresponding tone case so it uses the intended non-muted styling. Keep
usage_all()'s existing help text unchanged and align the new mapping with the
established terse labels used by the other commands.
---
Nitpick comments:
In `@install.sh`:
- Around line 222-223: Update the cron schedule in the install script’s
cron_command definition to run maintenance less frequently than every minute,
preferably hourly or at a reasonable multi-minute interval, while preserving the
existing PATH and codex-auth maintain --quiet invocation.
In `@lib/codex-auth/profiles.sh`:
- Around line 149-151: Read-only commands cmd_list and cmd_current can block on
the mutation lock and emit an error. Update sync_active_profile_from_live or its
read-only call sites to use a non-blocking/try lock acquisition for these
commands, while preserving the existing re-entry behavior for mutating commands
such as cmd_use, cmd_save, cmd_login, and cmd_use_if_current; on contention,
status commands should continue without waiting or printing mutation errors.
In `@tui/src/codex_auth_tui/tui/dashboard.py`:
- Around line 140-143: Update the paired iteration in the dashboard refresh
branch to use strict zip semantics, changing the zip call within the method
containing _flash_updated(snap, listview) to pass strict=True and explicitly
enforce matching AccountItem and account counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4c61e0a-cd3b-4417-b5a4-fb638114dbb3
⛔ Files ignored due to path filters (9)
assets/codex-auth-auto.pngis excluded by!**/*.pngassets/codex-auth-demo.gifis excluded by!**/*.gifassets/codex-auth-demo.mp4is excluded by!**/*.mp4assets/codex-auth-reset.pngis excluded by!**/*.pngassets/codex-auth-watch.pngis excluded by!**/*.pngassets/usage-selector.gifis excluded by!**/*.gifassets/usage-selector.pngis excluded by!**/*.pngassets/usage-selector.svgis excluded by!**/*.svgtui/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/workflows/ci.yml.gitignoreCHANGELOG.mdPRODUCT.mdREADME.mdbin/codexbin/codex-authbin/codex-auth-tuiinstall.shlib/codex-auth/core.shlib/codex-auth/help.shlib/codex-auth/maintain.shlib/codex-auth/patch.shlib/codex-auth/profiles.shlib/codex-auth/rolling-auth-v2.patchlib/codex-auth/run.shlib/codex-auth/selector.shlib/codex-auth/usage.shscripts/capture_tui.pyscripts/generate-assets.mjstests/run.shtui/README.mdtui/pyproject.tomltui/src/codex_auth_tui/__init__.pytui/src/codex_auth_tui/__main__.pytui/src/codex_auth_tui/backend.pytui/src/codex_auth_tui/cli.pytui/src/codex_auth_tui/engine.pytui/src/codex_auth_tui/models.pytui/src/codex_auth_tui/paths.pytui/src/codex_auth_tui/settings.pytui/src/codex_auth_tui/tui/__init__.pytui/src/codex_auth_tui/tui/app.pytui/src/codex_auth_tui/tui/autoview.pytui/src/codex_auth_tui/tui/codex_auth_tui.tcsstui/src/codex_auth_tui/tui/dashboard.pytui/src/codex_auth_tui/tui/data.pytui/src/codex_auth_tui/tui/modals.pytui/src/codex_auth_tui/tui/theme.pytui/src/codex_auth_tui/tui/widgets.pytui/tests/conftest.pytui/tests/test_backend.pytui/tests/test_engine.pytui/tests/test_policy.pytui/tests/test_tui.py
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on the checkout step.
actions/checkout@v4 persists the GitHub token in .git/config by default. Subsequent shell-script steps in this workflow could inadvertently read it. Add persist-credentials: false to prevent credential leakage.
🛡️ Proposed fix
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 15-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 15, Update the actions/checkout@v4 step to
set persist-credentials: false, preventing the GitHub token from being stored in
the repository’s Git configuration.
Source: Linters/SAST tools
| while IFS= read -r -d '' tui_file; do | ||
| tui_relative="${tui_file#$tui_source/}" | ||
| mkdir -p "$tui_stage/${tui_relative%/*}" | ||
| install -m 0644 "$tui_file" "$tui_stage/$tui_relative" | ||
| done < <(find "$tui_source/src" -type f \( -name '*.py' -o -name '*.tcss' \) -print0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote the prefix inside the parameter expansion.
${tui_file#$tui_source/} treats $tui_source as a glob pattern; if the source path contains pattern metacharacters the strip can misbehave, yielding wrong relative paths under $tui_stage. Quote it.
🐛 Proposed fix
- tui_relative="${tui_file#$tui_source/}"
+ tui_relative="${tui_file#"$tui_source"/}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while IFS= read -r -d '' tui_file; do | |
| tui_relative="${tui_file#$tui_source/}" | |
| mkdir -p "$tui_stage/${tui_relative%/*}" | |
| install -m 0644 "$tui_file" "$tui_stage/$tui_relative" | |
| done < <(find "$tui_source/src" -type f \( -name '*.py' -o -name '*.tcss' \) -print0) | |
| while IFS= read -r -d '' tui_file; do | |
| tui_relative="${tui_file#"$tui_source"/}" | |
| mkdir -p "$tui_stage/${tui_relative%/*}" | |
| install -m 0644 "$tui_file" "$tui_stage/$tui_relative" | |
| done < <(find "$tui_source/src" -type f \( -name '*.py' -o -name '*.tcss' \) -print0) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 44-44: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` around lines 43 - 47, In the TUI file-copy loop, update the
`${tui_file#$tui_source/}` parameter expansion to quote the `$tui_source/`
prefix, preventing glob metacharacters in the source path from affecting
relative-path calculation.
Source: Linters/SAST tools
What changed
rateLimitResetCredits.availableCountper profile and adds a Cancel-default earned-reset flow.alreadyRedeemedas success.Safety boundaries
reports/, local auth files, caches, and capture frames are ignored and were not staged.Verification
--checkaccount/rateLimits/readconfirmed the current saved profile reports 2 earned resets; no reset was consumedSummary by CodeRabbit
New Features
Documentation
Tests