forked from Soju06/codex-lb
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(http-bridge): retire bridge anchors upstream has denied #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
kevinsslin
merged 1 commit into
deploy/2026-08-22
from
fix/invalidate-denied-bridge-anchor
Aug 21, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
openspec/changes/invalidate-denied-bridge-anchor/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-08-22 |
41 changes: 41 additions & 0 deletions
41
openspec/changes/invalidate-denied-bridge-anchor/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Invalidate Bridge Anchors Upstream Has Denied | ||
|
|
||
| ## Why | ||
|
|
||
| The HTTP responses session bridge keeps re-injecting a `previous_response_id` that upstream has already said does not exist (issue #1852). | ||
|
|
||
| When upstream answers an anchored bridge request with `previous_response_not_found`, that is a verdict about the anchor. If the proxy injected the anchor itself, no client asked for the id, it came from the proxy's own durable record, and upstream has now refused it. Nothing in the bridge acts on that verdict: | ||
|
|
||
| 1. Anchor poisoning cannot see it. `_http_bridge_anchor_poison_detail` only scores reader failures whose detail is `stream_incomplete` or `stream_idle_timeout`. A denial arrives as a terminal upstream event, not a reader failure, so it contributes nothing at any value of `http_responses_session_bridge_anchor_poison_failure_threshold`. Lowering the threshold does not help. | ||
| 2. The dead id therefore survives in both carriers, the durable `latest_response_id` row and the in-memory `session.last_completed_response_id`, and the fresh-reattach path injects it into the next turn. | ||
| 3. On that next turn the store-context trim matches the stored prefix and strips it, because the trim consults the stored fingerprint and never whether the anchor is still alive. Upstream then receives a few items instead of the conversation, never emits `response.created`, and the attempt presents as an eventless failure rather than as a stale anchor. | ||
|
|
||
| Two of those eventless failures open the retry circuit, so the client sees `503 ... cooling down`. Measured over 12.7 h on one host: 163 `continuity_fail_closed` rejections, 29 circuit opens, and a worst-case trim of `original_items=602 trimmed_to=3`. | ||
|
|
||
| The second half of this change is why the first half currently could not fire even if it existed. The anchored recovery replays the proxy's own anchor but never copies `proxy_injected_previous_response_id` onto the retry state, so a denial of the replayed anchor is not attributable to the proxy. The same gap misreports `previous_response_source=client_supplied` for ids no client sent and keeps `_http_bridge_request_state_wedged_reattach` from recognising the reattach shape it exists to catch. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Retire a `previous_response_id` on the first explicit upstream denial when the proxy injected it, clearing the durable continuity row and the in-memory anchor together, instead of waiting for a counter that this failure class never increments. The retirement is skipped when a concurrent request has already advanced the anchor past the denied id. | ||
| - Carry `proxy_injected_previous_response_id` onto the anchored recovery retry state, so a denial of the replayed anchor is attributable, diagnostics report the real provenance, and the wedge classifier sees the reattach. Anchor-free recovery paths keep the flag false because they send no anchor. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| None. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `responses-api-compat`: An explicit upstream previous-response denial retires a proxy-injected anchor immediately, and anchored recovery retries retain the provenance of the anchor they replay. | ||
|
|
||
| ## Impact | ||
|
|
||
| - HTTP bridge terminal-event handling (`app/modules/proxy/_service/http_bridge/upstream_events.py`) and anchored recovery retry state (`app/modules/proxy/_service/http_bridge/streaming.py`). | ||
| - No API, schema, migration, dependency, configuration, or dashboard changes. The poison threshold setting and its default of seven are untouched, and the downstream error contract is unchanged: the denial is still masked to `stream_incomplete` and still surfaces as 502, so clients keep their anchor and do not resend full history (the invariant from #397). | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| - Adding another upstream dispatch. This change never resends the turn. The next turn is the client's own, with the history the client sends, so no forked child response can be created against a parent the proxy cannot observe. Retrying the turn server-side without the anchor is what #1857 and #1863 propose and is deliberately out of scope here. | ||
| - Changing the poison threshold arithmetic that issue #1852 is titled after. | ||
| - Exposing the stale-anchor classifier downstream on the bridge path. |
54 changes: 54 additions & 0 deletions
54
...spec/changes/invalidate-denied-bridge-anchor/specs/responses-api-compat/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # responses-api-compat Delta | ||
|
|
||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Explicit upstream previous-response denials retire proxy-injected anchors | ||
|
|
||
| When upstream answers an HTTP bridge request with a `previous_response_not_found` terminal frame, and the `previous_response_id` on that request was injected by the proxy rather than supplied by the client, the proxy MUST retire that anchor on the first denial rather than waiting for the eventless-failure poison threshold. Retirement MUST clear the durable continuity record under the session's owner epoch and the in-memory session anchor together, so no later turn can re-inject the denied id from either carrier. The proxy MUST NOT retire the anchor when the session's current anchor is no longer the denied id, because a concurrent request may have completed and advanced it. A client-supplied `previous_response_id` MUST NOT be retired by this path. | ||
|
|
||
| The downstream error contract is unchanged: the denial is still reported to the client as `stream_incomplete`, so the client retains its own anchor and is not driven into a full-history resend. | ||
|
|
||
| #### Scenario: A denied proxy-injected anchor is retired immediately | ||
|
|
||
| - **GIVEN** an HTTP bridge session whose stored anchor was injected by the proxy | ||
| - **WHEN** upstream answers the anchored request with `previous_response_not_found` | ||
| - **THEN** the proxy clears the durable continuity record under the session's owner epoch | ||
| - **AND** clears the in-memory session anchor and its stored input count and prefix fingerprint | ||
| - **AND** the next turn on that session dispatches without a `previous_response_id` | ||
|
|
||
| #### Scenario: The following turn is not trimmed against a denied anchor | ||
|
|
||
| - **GIVEN** a proxy-injected anchor was denied by upstream on the previous turn | ||
| - **WHEN** the client sends a full resend of the conversation on the next turn | ||
| - **THEN** the request MUST NOT be trimmed against the denied anchor's stored prefix | ||
| - **AND** upstream receives the resent conversation rather than a suffix of it | ||
|
|
||
| #### Scenario: A concurrent completion protects the current anchor | ||
|
|
||
| - **GIVEN** a proxy-injected anchor is denied by upstream | ||
| - **AND** another request on the same session completed first and advanced the session anchor to a different response id | ||
| - **WHEN** the denial is handled | ||
| - **THEN** the proxy MUST NOT clear the session anchor | ||
|
|
||
| #### Scenario: Client-supplied anchors are left alone | ||
|
|
||
| - **GIVEN** an HTTP bridge request carries a `previous_response_id` the client supplied | ||
| - **WHEN** upstream answers it with `previous_response_not_found` | ||
| - **THEN** the proxy MUST NOT retire the anchor on the client's behalf | ||
|
|
||
| ### Requirement: Anchored recovery retries retain the provenance of the anchor they replay | ||
|
|
||
| When the HTTP bridge dispatches an anchored recovery retry that replays a `previous_response_id` the proxy injected, the retry request state MUST record that the anchor is proxy-injected. A recovery path that dispatches without an anchor MUST leave that provenance false, because there is no anchor for it to describe. | ||
|
|
||
| #### Scenario: An anchored recovery retry is attributable to the proxy | ||
|
|
||
| - **GIVEN** a request whose `previous_response_id` was injected by the proxy fails and enters anchored recovery | ||
| - **WHEN** the recovery retry replays the same anchor | ||
| - **THEN** the retry request state records the anchor as proxy-injected | ||
| - **AND** continuity diagnostics for the retry report `previous_response_source=proxy_injected` rather than `client_supplied` | ||
|
|
||
| #### Scenario: Anchor-free recovery retries claim no provenance | ||
|
|
||
| - **GIVEN** a recovery path dispatches without a `previous_response_id` | ||
| - **WHEN** the retry request state is prepared | ||
| - **THEN** it MUST NOT record a proxy-injected anchor |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Tasks | ||
|
|
||
| ## 1. Regression Coverage | ||
|
|
||
| - [x] 1.1 Add a bridge integration regression driving a completed turn, an anchored turn denied with `previous_response_not_found`, then a following client full resend, asserting the third dispatch carries no `previous_response_id` and is not trimmed against the denied anchor. | ||
| - [x] 1.2 Add unit coverage that a denial retires both anchor carriers on the first occurrence, and that it is skipped when a concurrent completion has already advanced the anchor. | ||
| - [x] 1.3 Assert at the product path that no continuity diagnostic reports a proxy-injected anchor as `client_supplied`, which fails before the provenance fix. | ||
|
|
||
| ## 2. Anchor Retirement | ||
|
|
||
| - [x] 2.1 Add `_invalidate_denied_http_bridge_anchor`, clearing durable continuity through the existing fenced `_abandon_durable_http_bridge_continuity` write and the in-memory anchor fields together. | ||
| - [x] 2.2 Call it from the terminal `previous_response_not_found` branch when the denied anchor was proxy-injected. | ||
|
|
||
| ## 3. Recovery Provenance | ||
|
|
||
| - [x] 3.1 Copy `proxy_injected_previous_response_id` onto the anchored recovery retry state, gated on the retry actually carrying an anchor. | ||
|
|
||
| ## 4. Verification | ||
|
|
||
| - [x] 4.1 Run the touched bridge unit and integration suites, ruff, and type checks. | ||
| - [x] 4.2 Run strict OpenSpec validation for this change and review the final diff for unrelated changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
rebind_session_accountraises or returnsFalse,_abandon_durable_http_bridge_continuityreturnsFalse, but this code ignores that result and still clears the in-memory carrier. The durable row can therefore retain the deniedlatest_response_id; after a restart or ownership transfer, that dead anchor is loaded and re-injected again, recreating the failure loop this change is intended to stop. Ensure the durable clear is confirmed, or retire/fail-close the session and arrange retry, rather than treating the two carriers as cleared after a partial failure.AGENTS.md reference: AGENTS.md:L109-L113
Useful? React with 👍 / 👎.