Skip to content

Keep HTTP bridge missing-created watchdog armed after prelude - #1580

Open
Komzpa wants to merge 2 commits into
Soju06:mainfrom
Komzpa:fix/bridge-queued-timeout
Open

Keep HTTP bridge missing-created watchdog armed after prelude#1580
Komzpa wants to merge 2 commits into
Soju06:mainfrom
Komzpa:fix/bridge-queued-timeout

Conversation

@Komzpa

@Komzpa Komzpa commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix the HTTP bridge missing-response.created watchdog so upstream prelude events do not disarm it before a response id exists.

Before this change, _http_bridge_eventless_precreated_deadline() returned no deadline as soon as response_event_count was nonzero. That treated any upstream event as progress even when the bridge still had no response.created, no response_id, and no visible downstream output. A client could then see only the initial SSE heartbeat while the bridge stayed occupied until a later idle or stream cleanup path.

This keeps the watchdog armed until real progress is observed: response.created/latency, visible downstream output, terminal cleanup, or the existing explicit disqualifiers.

Validation

  • uv run pytest tests/unit/test_proxy_http_bridge.py -k 'eventless_precreated_deadline or eventless_timeout_force_retires or missing_response_created or reader_wakes_and_retires_lone_eventless_owner_without_keepalives or response_create_gate_timeout_does_not_retire_active_response_progress'
  • uv run pytest tests/unit/test_proxy_http_bridge.py
  • uv run ruff check app/modules/proxy/_service/http_bridge/helpers.py tests/unit/test_proxy_http_bridge.py
  • git diff --check

@Komzpa
Komzpa force-pushed the fix/bridge-queued-timeout branch from 211e6af to 0fc6fce Compare August 3, 2026 20:46
@Komzpa
Komzpa marked this pull request as ready for review August 3, 2026 21:32
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Soju06

Soju06 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

The gap is real: with response_event_count != 0 disarming the deadline (helpers.py:733), a lone owner that sees a reasoning prelude without response.created escapes both watchdogs — _http_bridge_pending_state_is_stale is waiter-triggered and silence-clocked, so the client hangs on heartbeats. Keeping the watchdog armed is the right direction, but as-is this has a regression and misses two chores:

  1. Send-time anchoring now kills live streams. A reattached upstream whose response.created was dropped can stream deferred reasoning for >60s: deltas carry no response id, reasoning isn't downstream-visible, so at sent_at + 60s the timeout fires with fail_all_pending=True and force-retires the whole session (upstream_events.py:806-900). Because upstream_model_output_seen is set, _websocket_request_can_replay_before_visible_output (support.py:1210) refuses replay, so the client gets upstream_request_timeout even though response.completed would have carried the id and settled the request. The stale path already solves this exact case by re-anchoring to last_upstream_activity_at when response_event_count > 0 (helpers.py:709) — do the same here instead of dropping the check outright: keep the deadline armed but clock it from the most recent response-lifecycle event.

  2. The comment directly below the edit (helpers.py:737-742, "intentionally anchored to the send time until a response-lifecycle event is observed") documents the behavior you just removed — update it.

  3. This amends the accepted recover-codex-desktop-idle-bridge requirement, which scopes the owner-side deadline to a send that has "received no matched response.* lifecycle event" (openspec/changes/recover-codex-desktop-idle-bridge/specs/proxy-admission-control/spec.md:7). Needs an openspec delta.

Note overlap with #1543/#1555 (bounded-retry approaches to the same lost-created stall, both superseded by merged #1394) and #1563, which fixes the durable-anchor flavor of #1534 — reconcile anchoring semantics with whichever lands first.

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.

2 participants