fix(reset-credits): critical consume hardening#2
Conversation
|
Thanks for the PR, but I would prefer a zero-db migration ATM as I have a Soju06#1015 implementation which includes a db-stored info I think it will be good to insert history record to db after the redeem and handle it on ui (just like the audit log) |
you're right. one question I have is if you've figured out how banking resets works? I've tried reversing the codex app but had no luck. |
Checkout: https://github.com/aaamosh/codex-reset |
I am using this repo as the logic base. |
I have not looked into it deeply myself yet, but I can help with this if you would like. From quickly checking it out it looks like you just use these two: GET using the existing auth header. |
Can you verify that these endpoints actually show resets? I have a feeling a person must first go to the codex app, receive the reset and only then does the endpoint actually report the reset. I could absolutely be wrong here though, feel free to correct me and test this yourself. |
In my build, one of the account does not login to Codex desktop, but still correctly show I have 2 resets remaining Does not test the actual reset tho |
|
added api functionality to fetch and redeem the pooled account's reset credit |
currently out swimming will do later today |
e23d2cf to
49f35e7
Compare
|
if you need any other changes then please tell me. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db33f9f602
ℹ️ 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".
|
addressed the codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83affefcf1
ℹ️ 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".
…rent cache invalidation and redeem UX behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d396d99d0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45bee550c1
ℹ️ 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".
…credits Reset-credit polling must not mutate account status (spec rate-limit-reset-credits: polling failures including 401 log and retain the prior snapshot; usage refresh owns token refresh and deactivation). The forced auth_manager.ensure_fresh(force=True) retry on a 401 flowed into AuthManager.refresh_account, which persists REAUTH_REQUIRED / DEACTIVATED on permanent refresh failure. Remove the auth_manager retry loop from _refresh_account_reset_credits so a 401 logs and keeps the cache in a single attempt. This also covers the dashboard GET cache-miss path that reuses the helper. Add REAUTH_REQUIRED to the account-summary ineligible set so a reauth-required account with a stale cached snapshot no longer surfaces available_reset_credits in list/header/dashboard badges, matching the scheduler skip set and the GET/consume eligibility checks.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 837ea46bac
ℹ️ 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".
| credits_response = await effective_fetch_fn( | ||
| access_token, | ||
| redeem_account.chatgpt_account_id, | ||
| route=route, | ||
| allow_direct_egress=route is None, |
There was a problem hiding this comment.
Preserve the cached snapshot gate before redeeming
The OpenSpec for this change still says POST /consume must return a 409 without calling upstream when this replica has no cached snapshot or the cached available_count <= 0, but this path fetches live credits before consulting the store and then selects from that response. In a cold replica or after local cache invalidation, a write-capable caller can redeem an irreversible reset credit even though the account summary exposes 0/hides the affordance, and it bypasses the cached-count guard intended to serialize local state.
Useful? React with 👍 / 👎.
| return None | ||
|
|
||
| async def refresh_usage(account: Account) -> None: | ||
| await usage_updater.force_refresh(account) |
There was a problem hiding this comment.
Treat false force-refresh results as failures
UsageUpdater.force_refresh() catches refresh exceptions and returns False, so this callback currently falls through as if the post-consume usage refresh succeeded. When /wham/usage is disabled or transiently fails after a credit was consumed, the snapshot has already been invalidated and the restore path is never run, leaving remaining reset credits hidden until a later reset-credit poll and the account status stale until a successful usage refresh; branch on the boolean or raise to trigger the existing restore logic.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b659b9ed84
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5b1c6f902
ℹ️ 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 credit is None: | ||
| raise DashboardConflictError("No available reset credit", code="no_available_reset_credit") |
There was a problem hiding this comment.
Invalidate stale cache when live fetch finds no credit
When the account summary still has a cached positive count but the new live fetch at consume time returns available_count: 0 (for example the credit expired or was redeemed outside this process), this branch returns 409 without updating or invalidating the stored snapshot. Because the frontend does not invalidate queries on consume failure, the same stale reset button/details can remain visible until a later scheduler tick; clear or replace the snapshot from credits_response before returning the conflict.
Useful? React with 👍 / 👎.
| if account.status in _RESET_CREDITS_SKIP_STATUSES: | ||
| continue |
There was a problem hiding this comment.
Clear skipped account snapshots before they can be reused
For accounts that are paused, require reauth, or are deactivated while a reset-credit snapshot is cached, this continue now leaves the old snapshot in memory. If the account is later resumed before the next successful poll, account summaries start reading that old store.get(account.id) again and can advertise a credit that expired or was spent while the account was ineligible; the previous implementation invalidated skipped accounts, so either keep that invalidation or force a fresh fetch before reused snapshots become visible.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f380c5f296
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Thanks for the contribution |
follow-up for Soju06#1053 — ports critical fixes from overlapping Soju06#1056 / Soju06#1014 testing.
backend
frontend
tests
happy to fold into Soju06#1053 or adjust anything.