Skip to content

fix(proxy): quarantine silent HTTP bridge sessions (takeover of #1405) - #1630

Merged
Soju06 merged 4 commits into
mainfrom
fix/quarantine-silent-bridge-sessions
Aug 7, 2026
Merged

fix(proxy): quarantine silent HTTP bridge sessions (takeover of #1405)#1630
Soju06 merged 4 commits into
mainfrom
fix/quarantine-silent-bridge-sessions

Conversation

@Soju06

@Soju06 Soju06 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

First-party takeover of #1405 by @WangErgouaaaa — quarantine is the maintainer-chosen direction for silent HTTP-bridge sessions (decided 08-04, over the account-health penalty alternative #1574), and the author has been unresponsive since 07-20 across two review rounds and a takeover offer. Supersedes #1405. The original commit's quarantine-window concept, per-key deadline registry with pruning/size-cap eviction, and event wording are carried forward with attribution (Co-authored-by: WangErgouaaaa <117421439+WangErgouaaaa@users.noreply.github.com>); the rejected pieces (5s response.created timeout, raw-HTTP fallback window, two new settings) are replaced per the 07-24/08-06 review findings.

Related: #1534 (production evidence; its reported fully eventless full-resend flavor was already fixed by the merged #1563 — this PR covers the remainder that #1563 and #1394 leave open: the events-flowing wedge, the in-memory reuse path, and the repeated-wedge case), #1580 (the deferred-reasoning P1 is honored: quarantine never keys on mere event silence during a live owned turn), #1410.

Trigger (proof-based, never a live-turn timeout)

Evaluated only when a request is already being failed or its session retired:

  1. Wedged reattach (immediate): a pending request with a proxy-injected previous_response_id that sent response.create, observed upstream response events, but never had response.created assigned. This is the shape that actually wedged production (HTTP-bridge reattach stalls with no response.created and needs a client restart to recover #1534/fix(http-bridge): unstarve the stuck-gate watchdog on silent upstreams (follow-up to #1394) #1410) — the response_event_count == 0 gates in the eventless watchdog (fix(http-bridge): stabilize silent and clean-close recovery #1394) and the fenced anchor clear (fix(proxy): clear durable bridge anchor after stuck eventless timeout #1563) never trip on it, and replay is refused because model output was seen. Hooked at the reader failure/retire funnel and the stale-gate-holder cleanup.
  2. Repeated eventless timeouts: two consecutive missing_response_created_timeout retires for the same session key. The first stays on the merged fix(http-bridge): stabilize silent and clean-close recovery #1394 recovery path (bounded pre-created retry + fix(proxy): clear durable bridge anchor after stuck eventless timeout #1563 fenced anchor clear).

Any request whose response.created was observed is excluded by construction — a deferred-reasoning stream with long legitimate gaps can never be quarantined (unit-tested negative).

Effect (session-scoped, account-neutral)

Expiry (bounded, no leak)

In-memory only — no durable rows, no janitor, no migration, no settings (fix-class, default-on). Cleared on a completed response for the key (alongside the retry-circuit clear); otherwise a 600s TTL (aligned with the retry circuit's max backoff), prune-on-touch, and a 1024-entry size cap with oldest-first eviction.

OpenSpec

openspec/changes/quarantine-silent-bridge-sessions/ (responses-api-compat delta, 7 scenarios incl. the deferred-reasoning negative and the delta-only-anchor protection). openspec validate quarantine-silent-bridge-sessions --strict and openspec validate --specs --strict (49/49) pass.

Test evidence

  • Integration regression modeling HTTP-bridge reattach stalls with no response.created and needs a client restart to recover #1534 (tests/integration/test_http_responses_bridge.py): reattach injects the durable anchor (resp_bridge_custom_1), upstream streams reasoning deltas but never response.created, the turn fails and the key is quarantined; the next full-resend request is sent with no previous_response_id and completes 200 on a fresh connect, and the completed response clears the quarantine. Verified the test fails with quarantine neutralized — the third attempt rebuilds the identical wedged reattach and fails again, reproducing the HTTP-bridge reattach stalls with no response.created and needs a client restart to recover #1534 loop.
  • Unit coverage: predicate truth table (8 shapes), TTL expiry, size bound, 2-strike threshold with completion reset, reuse-gate exclusion, reader-failure trigger (positive + created-assigned negative), stale-gate-holder trigger, and a real reader-loop missing_response_created_timeout run recording exactly one strike without quarantining.
  • Gates: ruff check/format, ty (clean), scripts/check_proxy_architecture.py (no budget raised — new logic lives in un-ratcheted quarantine.py; mixin.py/service.py untouched), tests/unit/test_proxy_http_bridge.py 488 passed, adjacent bridge suites 298 passed, full tests/unit 5684 passed, tests/integration/test_http_responses_bridge.py 122 passed.

🤖 Generated with Claude Code

Post-review hardening (b473212)

Two fixes from the adversarial verification pass, each with a regression test: the session-object quarantine flag now resets on clear (a surviving session becomes reusable once a completed response disproves the wedge), and a TTL-expired first eventless strike can no longer be resurrected into a "consecutive" second strike (prune runs before increment).

Soju06 and others added 2 commits August 6, 2026 09:49
A bridge session that has proven silent/wedged must stop attracting new
attach attempts. Two shapes prove it: a reattached stream (proxy-injected
previous_response_id) that delivers upstream response events but never gets
response.created assigned (the #1534 production wedge, which the
response_event_count == 0 gates in the eventless watchdog and the fenced
durable-anchor clear never trip on), and a session key that hits two
consecutive eventless missing_response_created_timeout retires.

Quarantine is bounded, in-memory, session-scoped, and account-neutral:
- Excluded from re-attach/session-reuse selection so later requests take
  a fresh session.
- Fresh-reattach durable-anchor injection is skipped for full-resend
  payloads while the key is quarantined; delta-only payloads keep the
  anchor (same boundary as the fenced anchor clear).
- Cleared on a completed response for the key, a 600s TTL, and a registry
  size cap; no durable rows, no account-health writes, no new settings.

Triggers are only ever evaluated when a request is already being failed or
its session retired — never against a live owned turn — so deferred-
reasoning streams with long legitimate event gaps can never be quarantined.

First-party takeover of #1405 (quarantine direction chosen over the #1574
account-health alternative), rebuilt on the merged #1394/#1563/#1600
recovery machinery with the 07-24 review findings addressed.

Co-authored-by: WangErgouaaaa <117421439+WangErgouaaaa@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ventless strikes

Two hardening fixes from adversarial review: a surviving session becomes
reusable again once a completed response disproves the wedge (the session
flag now resets alongside the registry clear), and a TTL-expired first
eventless strike can no longer be resurrected into a "consecutive" second
strike (prune runs before the increment).

Co-Authored-By: Claude Fable 5 <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: b4732123a8

ℹ️ 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 +1417 to +1420
if (
fresh_reattach_can_use_durable_anchor
and payload_looks_like_full_resend
and _http_bridge_session_key_quarantined(self, bridge_session_key)

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 Treat quarantined local sessions as absent during anchor planning

When stale-gate cleanup quarantines a session while a sibling request keeps it live, _http_bridge_has_live_local_session still returns true because its checks in owner_forwarding.py ignore session.quarantined. That makes fresh_reattach_can_use_durable_anchor false before this quarantine check is reached; the later lookup then rejects and detaches the session, so a delta-only continuation is sent on a fresh connection without the durable anchor and loses its prior conversation context. Make the live-local preflight use the same quarantine eligibility as session reuse.

AGENTS.md reference: AGENTS.md:L129-L132

Useful? React with 👍 / 👎.

Comment on lines +63 to +69
for key, entry in list(registry.items()):
if entry.last_touched_monotonic <= expiry and entry.quarantined_until <= now:
registry.pop(key, None)
overflow = len(registry) - _HTTP_BRIDGE_QUARANTINE_MAX_ENTRIES
if overflow > 0:
for stale_key in sorted(registry, key=lambda candidate: registry[candidate].last_touched_monotonic)[:overflow]:
registry.pop(stale_key, 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.

P2 Badge Clear live session flags when pruning quarantine entries

When the 600-second TTL elapses—or the size cap evicts an entry—this removes only the registry record and leaves the corresponding live session's quarantined flag set. A session that survived stale-gate cleanup without a later completion therefore remains permanently ineligible for reuse even though the documented quarantine window expired; this is observable for Codex sessions whose configured idle TTL can exceed 600 seconds. Expiry/eviction must also clear or stop relying on the per-session flag.

AGENTS.md reference: AGENTS.md:L24-L27

Useful? React with 👍 / 👎.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Komzpa pushed a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Aug 6, 2026
Komzpa added a commit that referenced this pull request Aug 6, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
Komzpa pushed a commit that referenced this pull request Aug 7, 2026
The five new pre-response-start regressions were appended to the end of
tests/unit/test_proxy_http_bridge.py, which collides with PR #1630 appending
its quarantine tests at the same spot. Move them (and their local
_eventless_settings/_make_bridge_session helpers) into
tests/unit/test_http_bridge_eventless_semantics.py so both branches can land.

The two in-place assertion updates in test_proxy_http_bridge.py stay where
they are; that file's tail is now byte-identical to its pre-branch state, so
there is no append surface left to collide on.
@Soju06
Soju06 merged commit 3aabe99 into main Aug 7, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: needs work [@codex review] raised an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants