Skip to content

fix(proxy): recover dead durable bridge anchors fast - #1625

Merged
Soju06 merged 5 commits into
mainfrom
bridge-restart-recovery
Aug 10, 2026
Merged

fix(proxy): recover dead durable bridge anchors fast#1625
Soju06 merged 5 commits into
mainfrom
bridge-restart-recovery

Conversation

@Komzpa

@Komzpa Komzpa commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

After a backend process is killed mid-drain (deploy SIGKILL, crash), durable bridge sessions survive in SQLite and — because a restarted container can reuse the same instance id — startup purge treats dead-owner rows as self-owned. Clients then reattach to dead continuity anchors and loop on retryable stream_idle_timeout 503s for tens of minutes (live incident 2026-08-06: 49 post-restart orphan failures in a 40-minute window; interactive sessions reconnected fruitlessly for 29-44 minutes; codex resume could not cure it).

The maintainer's law this implements: tears are acceptable — slow recovery is the bug.

Fix 1 — owner process epoch: durable bridge ownership now includes a per-process epoch (additive migration). Startup retires rows owned by any previous epoch of the same instance id, so a same-container restart no longer masks dead owners.

Fix 2 — dead-anchor semantics: when the durable owner is provably dead (stale epoch), the client-facing terminal is non-retryable fresh-turn guidance instead of retryable stream_idle_timeout — reconnect loops against dead anchors end immediately.

Fix 3 — anchor poisoning: repeated zero-event idle-timeout failures on the same bridge key (configurable threshold, default 7 — the live incident's observed circuit count) clear/abandon durable continuity even when admission waiters would otherwise defer retirement indefinitely.

Verification: regression tests for all three fixes fail on unfixed origin/main (verified in a throwaway worktree) and pass here; focused bridge suite 538 passed; full suite has no new failures vs baseline (the single failure is the pre-existing warm-now timezone bug fixed in #1623); openspec change recover-bridge-restart-anchors validates strictly.

@github-actions github-actions Bot added the db migration PR changes Alembic database migrations; maintainer must coordinate merge order label Aug 6, 2026

@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: 38e6e1ca2e

ℹ️ 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 thread app/modules/proxy/_service/http_bridge/streaming.py
Comment thread app/modules/proxy/_service/http_bridge/upstream_events.py Outdated
Comment thread app/modules/proxy/durable_bridge_coordinator.py Outdated
Comment thread app/modules/proxy/durable_bridge_repository.py
Komzpa and others added 5 commits August 10, 2026 02:58
The rebase onto current main made 20260808_000000_tune_usage_history_autovacuum
the single head; parent the bridge owner-process-epoch revision on it instead
of the stale 20260730 fair-share head so the graph stays linear without a
merge revision.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Soju06
Soju06 force-pushed the bridge-restart-recovery branch from 5ce37c5 to da562a3 Compare August 10, 2026 03:08
@Soju06
Soju06 merged commit 85f1ee4 into main Aug 10, 2026
38 checks passed
Soju06 added a commit that referenced this pull request Aug 10, 2026
…ess check (#1673)

lease_expires_at is a timestamptz column, so PostgreSQL yields it
offset-aware while utcnow() and SQLite are naive UTC. The dead-owner
classifier introduced in #1625 was the first Python-side comparison of
the two and raised TypeError on every anchored durable lookup in
production (v1.23.0-beta.5, ~12 failed requests/min). Normalize both
sides with to_utc_naive; regression covers aware/naive combinations.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ayusavin added a commit to effective-dev-os/codex-lb that referenced this pull request Aug 10, 2026
websocket_connect()'s asyncio.TimeoutError (open_timeout) carries no
socket/DNS errno, so is_process_network_failure() never classified it
as a process-network error and it never reached
_rotate_after_websocket_network_failure - unlike the post-connect
send/receive path, which already rotates on network failure
(rotate_shared_http_transport, see the existing test at
test_direct_websocket_network_send_and_receive_are_typed_and_rotate_without_reconnect).

Reproduced live on prod 2026-08-10/11: the same pooled account got
stuck 3 times in a few hours, each recurrence 17-40 min after a
process restart - unanchored_parallel_fork followed by repeating
upstream_unavailable/"Request to upstream timed out" on every
subsequent request for that account, while sibling accounts in the
same pool kept succeeding. Traced to the connect-phase 8s
open_timeout at the websocket_connect() call site never triggering
any shared-transport rotation, so if the underlying SOCKS5-backed
transport itself is the stale resource, nothing discards it short of
a full process restart.

Now the connect-timeout except block also calls
rotate_shared_http_transport(transport="websocket", ...) before
raising the same 502 upstream_unavailable it already raised -
best-effort and wrapped in its own try/except (mirrors
_rotate_after_websocket_network_failure's existing pattern) so a
rotation failure can never replace the credential-safe timeout the
owning request must surface. rotate_shared_http_transport's own
cooldown (network_recovery.py) already protects against over-rotation
on repeated timeouts, so this is safe to call unconditionally rather
than only after a confirmed network-error classification.

No file overlap with this branch's own retry_circuit.py commit
(1418bcf) - that one only touches
_http_bridge_precreated_retry_cooldown_seconds's half-open-probe
cooldown math.

Confirmed unrelated to origin/bridge-restart-recovery (PR Soju06#1625,
already merged/deployed, already an ancestor of this branch via
85f1ee4) - that fix targets an already-connected zero-event-idle
wedge, a disjoint code path from this connect-phase timeout.
Soju06 added a commit that referenced this pull request Aug 11, 2026
…1694)

Verify -> sync -> archive for eight OpenSpec changes whose implementation
PRs are merged on main:

- probe-additional-usage-latest-reads (#1614)
- bound-projection-history-fetch (#1613)
- serve-request-log-count-from-rollups (#1615)
- add-conversation-presence-rollup (#1616)
- coalesce-api-key-last-used-writes (#1627, #1667)
- async-commit-telemetry-writes (#1628, #1665)
- usage-history-bulk-covering-index (#1629, #1666)
- recover-bridge-restart-anchors (#1625, #1673)

Delta specs merged into main specs (12 requirements added, 3 modified);
openspec validate --specs --strict passes (49/49).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

db migration PR changes Alembic database migrations; maintainer must coordinate merge order

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants