Skip to content

fix(tui,core): scope auth-profile usage invalidation to the relogged profile#410

Merged
andrei-hasna merged 1 commit into
mainfrom
fix/tui-relogin-profile-scoped-usage-invalidation
Jul 25, 2026
Merged

fix(tui,core): scope auth-profile usage invalidation to the relogged profile#410
andrei-hasna merged 1 commit into
mainfrom
fix/tui-relogin-profile-scoped-usage-invalidation

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Origin

Recovered from a stale worktree (wt_2f47aa47, branch task/2f47aa47-pr211-repair, untouched ~200h) during worktree triage.

That branch's 3 commits duplicated PR #211 "Repair TUI thread routing stack", which already merged to main on 2026-07-21 from repair/pr0-wave2-tui-routing-stack, and its scripts/list-bazel-clippy-targets.sh change is now byte-identical to main. The uncommitted working tree, however, contained two behavior fixes that were never merged anywhere. This PR rebases only those onto current main; everything already upstream or superseded was dropped.

Changes

1. codex-rs/tui/src/chatwidget/usage_limit_reset/state.rs - profile-scoped invalidation

finish_selected_auth_profile_credential_mutation only invalidated usage state when the mutated profile was still the selected one:

if credentials_changed && self.config.selected_auth_profile.as_deref() == Some(profile) {
    self.invalidate_rate_limit_reset_state_after_account_update();
}

Two consequences:

  • A same-name relogin left the profile's auth_profile_usage_exhausted_reset_at_by_profile / heartbeat-suppression entries in place, so the usage heartbeat stayed suppressed for the old (now invalid) reset time even though the credentials changed.
  • If the selection changed while the credential mutation was in flight, the relogged profile's stale state was never invalidated at all.

Invalidation is now scoped via a small AuthProfileUsageInvalidation enum:

  • AllProfiles - account-level update (existing invalidate_rate_limit_reset_state_after_account_update behavior, plus it now also clears the per-profile caches).
  • Profile(name) - a credential mutation: drops only that profile's snapshot / heartbeat-requested / heartbeat-failed / exhausted-reset entries, and clears rate_limit_snapshots_by_limit_id only when the relogged profile is the currently selected one (so an unrelated profile's relogin no longer nukes the live selected-profile snapshot).

2. codex-rs/core/src/auth_profile_usage.rs - Ordered strategy defers Unknown

The Ordered auto-switch branch returned the first Unknown-health candidate immediately, so an earlier unknown-health profile beat a later known-healthy one. It now feeds the existing first_unknown fallback (already used by HighestAvailable and already returned at the end of the function), so a known-healthy candidate wins and SelectedUnknownFallback behaves like the fallback its name implies.

Reviewer note: this is a deliberate behavior change to Ordered. It makes the two strategies consistent, but if Ordered is meant to be strictly positional even across unknown health, say so and I will drop this hunk (the tests are independent).

Tests

  • codex-tui: same_name_relogin_clears_exhausted_profile_heartbeat_suppression, relogin_invalidates_captured_profile_after_selection_changes, profile_relogin_invalidation_preserves_unrelated_profile_usage_state
  • codex-core: ordered_recommendation_prefers_healthy_over_earlier_unknown

Explicitly dropped from the stale worktree

Path Why
codex-rs/tui/src/app/app_server_events.rs Conflicts with main; main solved the pre-primary leak differently (note_agent_metrics_from_notification + is_tracked_thread)
codex-rs/tui/src/app/thread_routing.rs Buffered-event replay paired with the superseded routing approach above
codex-rs/tui/src/app/tests.rs Test for the superseded routing behavior; main already has enqueue_primary_thread_session_* / pre_primary_untracked_request_stays_resolvable_on_addressed_thread
codex-rs/core/src/usage_profile_health.rs Comment-only test cosmetics (/*used_percent*/ labels), no behavior

Full pre-image is archived at ~/.hasna/repos/recovery/wt_2f47aa47/.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna
andrei-hasna force-pushed the fix/tui-relogin-profile-scoped-usage-invalidation branch 2 times, most recently from 8355a6e to a6e18e5 Compare July 25, 2026 04:55
…profile

Recovered from a stale worktree (task/2f47aa47-pr211-repair). The rest of
that branch duplicated PR #211, which already landed on main; only these
two behavior fixes were never merged.

- chatwidget/usage_limit_reset: relogin previously invalidated usage state
  only when the mutated profile was still the selected one, so a same-name
  relogin (or a relogin whose selection changed mid-flight) left stale
  exhausted-reset/heartbeat-suppression entries for that profile and the
  usage heartbeat stayed suppressed until the old reset time. Invalidation
  is now scoped: AllProfiles for account-level updates, Profile(name) for a
  credential mutation. The per-profile snapshot / heartbeat-requested /
  heartbeat-failed / exhausted-reset entries for the profiles in scope are
  always dropped; the global usage-limit reset state (generation, reset
  credits, in-flight and pending reset bookkeeping, limit-id snapshot
  caches) is reset only when the scope covers the currently selected
  profile, so relogging an unrelated profile no longer clears the live
  selected-profile snapshot or cancels its in-flight reset.
- core/auth_profile_usage: the Ordered auto-switch strategy returned the
  first Unknown candidate immediately, so an earlier unknown-health profile
  beat a later known-healthy one. It now defers to the existing
  first_unknown fallback, matching HighestAvailable.

Adds three TUI tests and one core test covering the new behavior.
@andrei-hasna
andrei-hasna force-pushed the fix/tui-relogin-profile-scoped-usage-invalidation branch from a6e18e5 to c32f719 Compare July 25, 2026 05:28
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Rebased onto current main + proven green on a Blacksmith testbox

Head: c32f71940 — rebased onto b71a05ee9 (i.e. after #393 and #406 landed, both of which touch chatwidget/tests/popups_and_settings.rs). Clean rebase, no conflicts.

One review-driven change to the diff

invalidate_rate_limit_reset_state(Profile(name)) previously ran the entire global usage-limit-reset invalidation (generation bump, rate_limit_reset_credits, rate_limit_reset_in_flight, pending picker / auto-reset-check / post-reset-refresh, auto-switch snapshot cache, prepare_for_usage_limit_reset) for any relogged profile — including one that is not selected. On main that path did nothing at all when the mutated profile was not selected, so as written the PR would have let an unrelated profile's relogin cancel the selected profile's in-flight usage-limit reset.

Now:

  • the per-profile caches (auth_profile_rate_limit_snapshots_by_profile, ..._heartbeat_requested_at_..., ..._heartbeat_failed_at_..., ..._exhausted_reset_at_...) are always dropped for the profiles in scope — this is the actual bug being fixed (same-name relogin, and relogin whose selection changed mid-flight);
  • the global reset state is reset only when the scope covers the currently selected profile (AllProfiles, or Profile(p) where p is selected).

This matches the convention in the caller (finish_auth_profile_relogin also only re-applies the turn-context auth profile when the relogged profile is still selected). All three TUI tests already in the PR pass unchanged — including profile_relogin_invalidation_preserves_unrelated_profile_usage_state, which asserts the selected-profile snapshot survives.

Reviewer note in the description: keep the Ordered hunk

The description offered to drop the core/auth_profile_usage.rs Ordered change. Keep it — it is the correct direction, corroborated in-tree: the canonical auto-switch selector choose_profile_for_auto_switch in core/src/usage_profile_health.rs already implements exactly this (first_healthy wins over first_unknown) with a comment saying "never optimistically switch to an Unknown profile when a later candidate is known to be healthy". recommend_auth_profile was the odd one out. (It is currently pub with no in-repo callers besides its own tests, so the change is behaviour-neutral for the shipped auto-switch path and purely aligns the two implementations.)

Also: the PR as pushed was not rustfmt-clean (popups_and_settings.rs, two hunks) and would have failed cargo fmt --all -- --check. Fixed.

Blacksmith testbox evidence

Runner: blacksmith-16vcpu-ubuntu-2404, workflow blacksmith-testbox.yml.

1. Green gate — run 30145816927conclusion=success, on the rebased head c32f71940:

(cd codex-rs && cargo fmt --all -- --check)
&& just clippy -p codex-tui -p codex-core -- -D warnings
&& just test -p codex-tui
&& (cd codex-rs && cargo nextest run --no-fail-fast -p codex-core \
      -E 'test(auth_profile_usage) or test(usage_profile_health)')
  • codex-tui: 3513 tests run, 3513 passed, 4 skipped — incl. same_name_relogin_clears_exhausted_profile_heartbeat_suppression, relogin_invalidates_captured_profile_after_selection_changes, profile_relogin_invalidation_preserves_unrelated_profile_usage_state
  • codex-core (auth-profile usage/health): 41 tests run, 41 passed — incl. ordered_recommendation_prefers_healthy_over_earlier_unknown
  • cargo fmt --check clean; clippy -D warnings clean on both crates.

2. Why the full -p codex-core suite is not part of the gate — it is red on main too.

Same command run on the branch (30144865253) and on main (30145308591, ref main):

tests run failed
branch 6497 57
main baseline 6493 57

The two failing sets are identical (comm diff empty in both directions): pre-existing environment failures in codex-core::allsuite::cli_stream::*, suite::rmcp_client::*, suite::code_mode::*, suite::hooks_mcp::*, suite::shell_snapshot::*, suite::request_permissions::*, suite::truncation::mcp_*. This PR adds zero new failures.

Merge-result gate

git merge-tree --write-tree origin/main HEAD990bd7fdf7d66c0d1fcd4bec2502901129a6bfb3, byte-identical to HEAD^{tree}. Because the branch is rebased directly onto current origin/main (b71a05ee9), the merge is a fast-forward and the merge result is exactly the tree that run 30145816927 tested — the branch numbers above are the merge-result numbers.

@andrei-hasna
andrei-hasna merged commit 23a664c into main Jul 25, 2026
28 checks passed
@andrei-hasna
andrei-hasna deleted the fix/tui-relogin-profile-scoped-usage-invalidation branch July 25, 2026 05:54
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant