Skip to content

fix(proxy): retry accepted capacity bridge failures - #1384

Open
Komzpa wants to merge 34 commits into
Soju06:mainfrom
Komzpa:fix/selected-model-capacity-retry-20260716
Open

fix(proxy): retry accepted capacity bridge failures#1384
Komzpa wants to merge 34 commits into
Soju06:mainfrom
Komzpa:fix/selected-model-capacity-retry-20260716

Conversation

@Komzpa

@Komzpa Komzpa commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • retry accepted-but-output-free HTTP bridge overload, selected-model-capacity, and abrupt-close failures before model output is visible
  • retry direct non-streaming Responses API overload responses and initial upstream stream EOFs through the existing bounded retry path
  • preserve owner-pinned quota handling while keeping local bridge/websocket close failures account-neutral
  • bound accepted retries by downstream attachment, request deadline, and visible-output state; retain the pending close failure for the reader when replay cannot proceed
  • update OpenSpec coverage and CI label-helper tests for retry-server-is-overloaded

Root cause

The bridge handled failures before response.created, but later accepted responses could still fail with overload/capacity or an abrupt close before any model output became visible. Those failures were surfaced as terminal even though the narrow output-free request was safe to replay. The direct non-streaming and initial-stream-EOF paths similarly returned before entering their bounded transient retry path.

The retry path now preserves downstream continuity and refuses replay after downstream detach, deadline expiry, visible output, or an unsafe continuation. Local transport and reader closes remain account-neutral; real upstream account errors retain their existing health penalties.

Validation

  • Head: 4ee8538421eaa6a46cfa199055b435dd019deb45
  • uv run pytest tests/unit/test_proxy_http_bridge.py tests/unit/test_proxy_utils.py -q -> 1239 passed
  • uv run pytest tests/integration/test_http_responses_bridge.py tests/integration/test_proxy_api_extended.py tests/integration/test_proxy_transient_retry.py -q -> 183 passed
  • uv run ruff check app/modules/proxy tests/unit/test_proxy_http_bridge.py tests/unit/test_proxy_utils.py tests/integration/test_http_responses_bridge.py tests/integration/test_proxy_api_extended.py tests/integration/test_proxy_transient_retry.py -> passed
  • uv run ty check app/modules/proxy -> passed
  • openspec validate retry-server-is-overloaded --strict -> passed
  • git diff --check origin/main...HEAD -> passed

@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from 251c570 to 4d3a04d Compare July 16, 2026 21:11
@Komzpa

Komzpa commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aabc8da834

ℹ️ 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".

Comment thread app/modules/proxy/_service/streaming/mixin.py Outdated
Comment thread app/modules/proxy/_service/http_bridge/upstream_events.py Outdated
@Komzpa Komzpa added 🤖 codex: ok [@codex review] says no issues found. simplicity-budget-approved Maintainer-approved exception to a simplicity budget (PRINCIPLES.md P3) labels Jul 16, 2026
@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from b119d6c to 8062265 Compare July 16, 2026 23:22
@Komzpa

Komzpa commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8062265485

ℹ️ 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".

Comment thread .github/scripts/fetch_pr_labels.py
Comment thread .github/scripts/detect_changed_areas.py
Comment thread .github/scripts/check_all_contributors.py Outdated
@Komzpa Komzpa added 🤖 codex: needs work [@codex review] raised an issue and removed 🤖 codex: ok [@codex review] says no issues found. 🤖 codex: needs work [@codex review] raised an issue labels Jul 16, 2026
@Komzpa

Komzpa commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 8043fdc51e

ℹ️ 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".

@Komzpa Komzpa added the 🤖 codex: ok [@codex review] says no issues found. label Jul 17, 2026
@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from 8043fdc to 7563c32 Compare July 17, 2026 08:52
@Komzpa Komzpa removed the 🤖 codex: ok [@codex review] says no issues found. label Jul 17, 2026
@Komzpa

Komzpa commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 7563c322f7

ℹ️ 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".

@Komzpa Komzpa added the 🤖 codex: ok [@codex review] says no issues found. label Jul 17, 2026
@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from abcd853 to fc86a9d Compare July 19, 2026 11:27
@Komzpa Komzpa removed the 🤖 codex: ok [@codex review] says no issues found. label Jul 19, 2026
@Komzpa

Komzpa commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0148a2f96a

ℹ️ 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".

Comment thread app/modules/proxy/_service/http_bridge/request_submit.py
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Jul 22, 2026
@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from a8fca26 to c896bb9 Compare July 22, 2026 08:42
Komzpa added a commit to Komzpa/codex-lb that referenced this pull request Jul 22, 2026
@Komzpa Komzpa removed the 🤖 codex: needs work [@codex review] raised an issue label Jul 22, 2026
@Komzpa

Komzpa commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@Komzpa
Komzpa force-pushed the fix/selected-model-capacity-retry-20260716 branch from b4d20d3 to b2f0839 Compare August 14, 2026 17:55
@Komzpa Komzpa added 🤖 codex: needs work [@codex review] raised an issue and removed needs rebase Needs rebase or conflict repair against current main 🤖 codex: needs work [@codex review] raised an issue labels Aug 14, 2026
@Komzpa Komzpa removed the 🤖 codex: needs work [@codex review] raised an issue label Aug 15, 2026
@Soju06

Soju06 commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Thanks for the 08-14 rebase — conflicts are cleared, all review threads are resolved, and the eventless-deadline reset landed in _prepare_http_bridge_terminal_capacity_replay. Remaining blockers before this can go to a fresh review pass:

  1. CI Required is red on head 7202460a: ty fails in tests/unit/test_key_fingerprint.py:34 (Expected AsyncSession, found _FakeSession).
  2. Out-of-scope changes folded in by the rebase: app/core/config/key_fingerprint.py (sqlite busy-retry) + its test, and the websocket/mixin.py reader-await/cancellation hunks carried over from the now-closed fix(proxy): retry websocket EOF before visible output #1388. Please split these into their own PR(s) — the key_fingerprint change is also what breaks CI here.
  3. Half of the 07-30 item 1 is still open: the reset is in the prepare helper, but latency_response_created_ms is not saved/restored in the finally block of _retry_http_bridge_terminal_capacity_request (a failed replay now leaves it None while the rest of the state is rolled back), and the requested regression test (replayed socket accepts the send and goes silent -> eventless deadline fires) is missing.
  4. Fixes #N line is still absent from the PR body.
  5. Once 1-4 are done, trigger a fresh automated review on the rebased head — the last review predates the rebase, and note the repo is moving review to CodeRabbit (chore(github): retire codex review label gate in favor of CodeRabbit #1763).

Also flagging: main's #1713 archived retry-server-is-overloaded, so this PR now edits docs under openspec/changes/archive/2026-08-13-retry-server-is-overloaded/ — maintainer should confirm that editing the archived change dir is the intended reconciliation.

@Soju06

Soju06 commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Rebase nudge: this was one review pass from mergeable on 08-16; now needs the rebase plus the ty fix. Still wanted.

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

Labels

simplicity-budget-approved Maintainer-approved exception to a simplicity budget (PRINCIPLES.md P3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants