fix(http-bridge): re-check liveness before retiring a stale pending session

[Komzpa]

_retire_stale_pending_http_bridge_session sampled response_events_seen BEFORE an await (the retry-circuit failure record), then retired the session from that pre-await snapshot. A healthy turn that receives its first response event during the suspension is retired anyway — killing a live turn on hard-strength keys.

Fix: re-sample response liveness under the registry + pending locks immediately before the close decision; a session that has since become healthy is not retired. Genuinely eventless sessions still retire.

Regression fails pre-fix (healthy turn killed) and passes post-fix, plus a control proving a truly-stale session still retires; 473 unit + 119 integration green (the 2 known reconnect failures are the separate bridge_instance_mismatch regression, fixed in its own PR).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1238f4d3e

ℹ️ 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".

[Komzpa]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1966752128

ℹ️ 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".

[Soju06]

Thanks for the fast iterations on the codex P1s — the lock-ordering fix (sampling pending state before _http_bridge_lock) and the upstream_close_attempted guard on the retirement path are the right shape. But the liveness signal added in the last two commits overshoots, and I believe it introduces a worse failure mode than the race being fixed:

1. last_completed_response_id is eternal history, not "became healthy during suspension" (app/modules/proxy/_service/http_bridge/request_submit.py:1836-1841)

became_healthy_during_suspend treats session.last_completed_response_id is not None (with a caller snapshot of 0) as proof that a response arrived during the retry-circuit await. But that field is set on every response.completed (upstream_events.py:2018) and is only cleared on account swap (mixin.py:2369) — it persists across turns for the life of the session. So for any session that has ever completed a turn (the normal case for reused bridge sessions):

• _fail_http_bridge_reader_and_maybe_retire sets session.closed = True because the upstream reader died, fails the pending requests, then calls retirement with response_events_seen=0 (an eventless new turn, e.g. stream_incomplete). The recheck sees the old completed id, flips session.closed = False, clears reconnect_requested/retire_after_drain, and returns without ever reaching _close_http_bridge_session_bounded. The session stays registered as open and reusable with a dead upstream websocket, and its account lease / durable ownership are never released. Each request routed to it fails, re-enters the same path, and gets reopened again — a dead-session leak that persists until account swap or restart.
• The stuck-session path at app/modules/proxy/service.py:1381 (response_create_gate_timeout_stuck_pending) passes no response_events_seen, so it is similarly neutered for any multi-turn session.

The codex finding you were addressing asked for evidence that is new since the caller's snapshot. Snapshotting session.last_completed_response_id at the top of _retire_stale_pending_http_bridge_session (before the retry-circuit await) and comparing it to the value at decision time would give exactly that: completed_at_entry != completed_now ⇒ a terminal response landed during suspension. The current_response_events_seen > caller_response_events_seen term is fine; the bare is not None term is the problem.

2. Split-lock claim of upstream_close_attempted

The close claim moved under _http_bridge_lock (request_submit.py:1866-1868), but _retire_http_bridge_after_drain_if_ready still claims the same flag under session.pending_lock (request_submit.py:1800-1806). Two racing paths can now both observe False and both invoke _close_http_bridge_session_bounded. If bounded close is fully idempotent this may be benign, but the claim used to be serialized by a single lock — either keep the claim under pending_lock or document why a double close is safe.

3. Stale-base artifact: app/modules/api_keys/repository.py

update_last_used was intentionally removed on main by the merged coalesce-api-key-last-used-writes change (its tasks.md 2.2: "remove the now-unused ApiKeysRepository.update_last_used and its protocol entry"). This branch predates that removal and re-adds the method; nothing on current main references it. Please rebase onto current main and drop this hunk. (The db migration label is also stale from an earlier head — the current diff contains no alembic files.)

4. Test gap

Both new tests model callers with response_events_seen=0 and last_completed_response_id=None. A control like "session completed turn 1, upstream reader dies with an eventless turn 2 pending → session must still retire and release its lease" would have caught #1 and would make a good regression for the rework.

One more process note: the last codex review ran on 1966752; the final two commits have not had a codex pass. After the rework, please re-request @codex review so the label gate can go green.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d877cb56a

ℹ️ 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".

[Komzpa]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c009b9592b

ℹ️ 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".

[Soju06]

Thanks — verified at head 378bd09 that all three prior blockers are resolved in code: the liveness signal now compares last_completed_response_id against a baseline snapshotted under pending_lock at entry (no more eternal-history false positive), the upstream_close_attempted claim is serialized under _http_bridge_lock, and the new last_upstream_event_generation counter closes the sampling/close race from the 08-14 review. Three things remain before this can be re-reviewed for merge:

1. Merge conflicts — main has since landed fix(proxy): fence successor bridge claims against the retiring predecessor #1751 (successor-claim fencing), fix(proxy): sweep idle bridge sessions without request traffic #1747 (idle bridge sweep) and fix(http-bridge): preserve goal-restart recovery across reconnects #1680, all touching request_submit.py/support.py/test_proxy_http_bridge.py. Please rebase; note the retirement path itself was refactored by fix(proxy): fence successor bridge claims against the retiring predecessor #1751/fix(proxy): sweep idle bridge sessions without request traffic #1747, so the recheck placement needs care during the rebase, not just mechanical conflict resolution.
2. Unit CI is red — tests/unit/test_otel.py::test_lifespan_drains_actual_audit_and_cancelled_fleet_tasks_before_resource_close timed out on this head. It looks unrelated to your change, but we need a green run on the rebased head to know.
3. Out-of-scope hunk — commit c13f381 (app/modules/api_keys/repository.py last-used touch) is the same stray hunk flagged on fix(ops,config,quota): compose stop grace, refresh timeout bounds, expirable warmup claim #1646 and has no connection to bridge retirement. Please drop it from this branch.

Happy to do a full re-review as soon as the rebased head is green.