Skip to content

fix(http-bridge): keep idle retirements out of retry circuit - #1677

Open
leventov wants to merge 2 commits into
Soju06:mainfrom
leventov:fix/ignore-idle-bridge-retirement-circuit
Open

fix(http-bridge): keep idle retirements out of retry circuit#1677
leventov wants to merge 2 commits into
Soju06:mainfrom
leventov:fix/ignore-idle-bridge-retirement-circuit

Conversation

@leventov

Copy link
Copy Markdown
Contributor

Summary

Routine idle HTTP Responses bridge retirement could consume a hard-key retry
circuit strike even when the bridge owned no pending request. One later genuine
pre-response timeout was then misclassified as the second consecutive failure,
opening the cooldown and returning 503 HTTP responses session bridge is cooling down to the client.

This PR makes circuit evidence request-owned: retirement records a strike only
when it still owns at least one pending request and that lifecycle has observed
zero response events. Direct retirement derives omitted event evidence from the
same locked ownership snapshot, so an eventful stale-gate request cannot be
misclassified as eventless merely because its caller omitted an optional
handoff. A reader failure hands its explicit pre-drain evidence to the ordinary
retirement path so draining the deque cannot erase genuine failure evidence.
Idle socket churn remains visible in diagnostics but is neutral to the durable
retry circuit.

Type of change

  • fix: — bug fix (no behavior change beyond the bug)

Linked issue: none exists for this incident-derived regression; the exact
idle-retirement-then-timeout sequence is covered at the routed endpoint.

OpenSpec

  • This PR includes / updates an OpenSpec change
  • This PR touches the codex-faithful Responses bridge and preserves the
    upstream request and downstream SSE wire formats

Change directory:
openspec/changes/archive/2026-08-10-recover-repeated-clean-close/

#1394 originally landed the recover-repeated-clean-close change as an active
artifact. This PR adds its missing design record, syncs the already-landed
stable requirements into openspec/specs/responses-api-compat/, appends the
post-deploy retirement-accounting requirement, verifies it, and archives the
same change. The comparatively large spec diff is lineage closure for #1394;
the new production correction in this PR is the focused retirement guard.

Origin and concurrent work

Landed lineage:

Concurrent work reviewed for overlap:

Changes

  • Snapshot the requests actually owned by the shared retirement boundary.
  • Carry a reader failure's pre-drain pending count into ordinary retirement so
    a genuine pre-response failure still records exactly one strike after its
    requests have been failed and removed.
  • Keep that ownership count out of deferred/poison handling, which has its own
    settlement path, so one failure cannot be charged twice.
  • Record a retry-circuit failure only when that snapshot is non-empty and the
    lifecycle is still pre-response.
  • Derive omitted response-event evidence from the exact locked retirement
    snapshot while preserving explicit pre-drain evidence from reader failures.
  • Keep genuine eventless pending retirement at exactly one strike.
  • Keep midstream retirement excluded from the pre-response circuit.
  • Add a production comment explaining why socket lifecycle and request
    lifecycle evidence must not drift together again.
  • Add unit coverage for idle, eventless-pending, midstream, and direct eventful
    retirement with omitted event evidence, plus a routed
    /backend-api/codex/responses regression for idle retirement followed by one
    real failure on the same hard key.

Test plan

uv run pytest tests/unit/test_proxy_http_bridge.py -q
# 517 passed

uv run pytest tests/integration/test_http_responses_bridge.py -q
# 126 passed

uv run ruff check <changed Python files>
uv run ruff format --check <changed Python files>
uv run ty check
uv run python scripts/check_proxy_architecture.py
npx --yes @fission-ai/openspec@1.6.0 validate --all --strict --no-interactive
git diff --check origin/main...HEAD
# all passed; OpenSpec: 170 passed, 0 failed

Screenshots / output

No dashboard-visible changes and no wire-format changes.

Before:

idle bridge retires with pending=0 -> phantom strike 1
one real missing_response_created_timeout -> strike 2 -> cooldown 503

After:

idle bridge retires with pending=0 -> no circuit change
one real missing_response_created_timeout -> strike 1 -> retry remains allowed
eventful direct retirement with omitted handoff -> no pre-response strike

Two genuine consecutive eventless request failures still open the configured
durable cooldown.

Simplicity

  • Works with zero configuration
  • Adds no setting, migration, setup step, README section, environment
    variable, or dashboard surface
  • Changes no default timeout or replay budget

Checklist

  • Title is in Conventional Commits format.
  • No related public issue exists; the bug and reproduction are documented
    inline.
  • Added unit and integration regressions at the externally failing path.
  • Ran the relevant local CI subsets and static gates.
  • Strict OpenSpec validation and change verification are clean.
  • Simplicity gates P1-P5 reviewed.
  • CHANGELOG.md was not edited.

Advance a hard-key retry circuit only when bridge retirement owns at least one pending request and that request has emitted no response event. Routine idle socket retirement remains diagnostic but no longer creates phantom failures or premature cooldowns.
@leventov

Copy link
Copy Markdown
Contributor Author

CI follow-up: the first integration-bridge run failed one parameter of test_v1_responses_http_bridge_classifies_responses_lite_developer_interleaved_full_resend with a transient 409 bridge_instance_mismatch. The exact CI target passed locally in one process (236 passed), and all other cloud jobs passed. I attempted to rerun failed jobs, but fork-contributor credentials do not have the required upstream admin permission. Please rerun the failed job.

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