Skip to content

fix(http-bridge): retire bridge anchors upstream has denied - #3

Merged
kevinsslin merged 1 commit into
deploy/2026-08-22from
fix/invalidate-denied-bridge-anchor
Aug 21, 2026
Merged

fix(http-bridge): retire bridge anchors upstream has denied#3
kevinsslin merged 1 commit into
deploy/2026-08-22from
fix/invalidate-denied-bridge-anchor

Conversation

@kevinsslin

Copy link
Copy Markdown
Owner

What this is

The fix for the 503 ... cooling down amplification on our own box, stacked on top of the deploy branch that already carries the SQLite startup fix (Soju06#1866).

Opened upstream as Soju06#1872. Kept on a branch off upstream/main so the same commit can serve both, with no fork-only drift.

The bug

When upstream answers with previous_response_not_found for an anchor the proxy injected itself, nothing retires that anchor:

  1. Anchor poisoning only scores reader failures (stream_incomplete, stream_idle_timeout). A denial is a terminal upstream event, so it never counts, at any threshold. This is why the ANCHOR_POISON_FAILURE_THRESHOLD=2 drop-in did not help: it moved durable_anchor_poisoned from 0 to 29 occurrences but left circuit opens per bridge reuse flat at 0.41 per 100.
  2. The dead id survives in the durable row and in the session, and gets re-injected into the next turn.
  3. The store-context trim then strips the resent history against it. Worst case measured on our host: original_items=602 trimmed_to=3.
  4. Upstream never emits response.created, so the attempt looks like an eventless failure. Two of those open the retry circuit, which is the 503.

The fix

Retire the anchor on the first denial, clearing the durable record and the in-memory anchor together. Skip it if a sibling request already advanced the anchor. Never touch client-supplied anchors.

Plus: carry proxy_injected_previous_response_id onto the anchored recovery retry state, without which the above cannot fire on the path that matters. That also fixes the previous_response_source=client_supplied mislabel we saw all over our logs (77 occurrences in 12.7 h, for ids no client ever sent).

No new upstream dispatch is added, so there is no risk of a forked duplicate response.

Verification

uv run ruff check .                              All checks passed!
uv run ruff format --check .                     954 files already formatted
uv run ty check                                  All checks passed!
uv run python scripts/check_proxy_architecture.py    proxy architecture checks passed

pytest tests/integration/test_http_responses_bridge.py
       tests/integration/test_proxy_websocket_responses.py     260 passed
pytest tests/unit/test_proxy_http_bridge.py
       tests/unit/test_http_bridge_cancel_drain.py
       tests/unit/test_bridge_ring_lifecycle.py                1 failed, 705 passed

The single unit failure (..._fails_closed_before_file_affinity_when_previous_response_owner_misses) reproduces identically on a pristine checkout of upstream/main at d4b00fd0, so it is not ours.

The new product-path regression fails without the fix (turn 3 returns 502 because the denied anchor is re-injected) and passes with it.

Deploy note

Not yet deployed to the VPS. Current production is deploy-20260822 (upstream/main + Soju06#1866) with server_anchored_replay_once and ANCHOR_POISON_FAILURE_THRESHOLD=2, healthy and at 0 cooling-down 503s since the 02:39 revert. Merging this gives a build to measure the escalation rate against.

An upstream `previous_response_not_found` against a proxy-injected
`previous_response_id` is a verdict about the anchor, but nothing acts on it.
Anchor poisoning only scores reader failures whose detail is `stream_incomplete`
or `stream_idle_timeout`, and a denial arrives as a terminal upstream event, so
it contributes nothing at any poison threshold. The dead id therefore survives
in the durable row and in the session, the fresh-reattach path injects it into
the next turn, and the store-context trim strips the resent history against it.
Upstream then receives a suffix of the conversation behind an id it has already
refused, never emits `response.created`, and the attempt presents as an
eventless failure. Two of those open the retry circuit and the client gets a
503.

Retire the anchor on the first denial instead, clearing the durable continuity
record and the in-memory anchor together, and skip it when a sibling request has
already advanced the anchor past the denied id. Client-supplied anchors are left
alone.

Also carry `proxy_injected_previous_response_id` onto the anchored recovery
retry state. Without it a denial of the replayed anchor is not attributable to
the proxy, so the retirement above cannot fire on the path that needs it most.
The same gap reports `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.

No new dispatch is added: the following turn is the client's own, with the
history the client sends, so no forked child response can be created against a
parent this proxy cannot observe. The downstream contract is unchanged, so
clients keep their anchor and are not driven into a full-history resend.

Refs Soju06#1852

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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: 3c93df9404

ℹ️ 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 on lines +1047 to +1048
await service._unregister_http_bridge_previous_response_ids(session)
session.last_completed_response_id = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not discard memory when durable anchor clearing fails

When rebind_session_account raises or returns False, _abandon_durable_http_bridge_continuity returns False, but this code ignores that result and still clears the in-memory carrier. The durable row can therefore retain the denied latest_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 👍 / 👎.

@kevinsslin
kevinsslin merged commit b7298f0 into deploy/2026-08-22 Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant