fix(proxy): own cancellable usage refresh sessions - #1
Conversation
|
@codex review |
|
Superseded by upstream PR Soju06#1887. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23a487b10d
ℹ️ 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".
| await updater.refresh_accounts( | ||
| accounts, | ||
| latest_usage, | ||
| own_singleflight_sessions=True, |
There was a problem hiding this comment.
When PostgreSQL uses CODEX_LB_DATABASE_POOL_SIZE=1 with zero overflow and the usage API returns 401, this owned path deadlocks until the pool timeout: _refresh_account_if_stale_with_owned_session retains its background session while AuthManager.ensure_fresh opens refresh_repo_factory, which requires a second connection from the same background pool. Thus the newly enabled rate-limit path cannot refresh expired credentials in the exact single-connection configuration the preceding comment promises to support; close the usage read session before the upstream/token-refresh phase or otherwise avoid the nested checkout.
AGENTS.md reference: AGENTS.md:L109-L113
Useful? React with 👍 / 👎.
1 participant
Problem
Client-cancellable rate-limit refreshes used caller-bound repositories while running singleflight work, and joined owned refreshes could leave account state stale.
Solution
Verification
uv run pytest tests/unit/test_usage_updater.py tests/unit/test_proxy_rate_limit.py -q -p no:cacheprovider— 119 passedruff check app/modules/proxy/_service/rate_limit.py tests/unit/test_proxy_rate_limit.py— passedOpenSpec: existing database-backends/session-lifecycle requirements; no new delta required.