fix(http-bridge): stabilize silent and clean-close recovery - #1394
Conversation
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c752dc3a55
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfb79a763d
ℹ️ 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".
f38723b to
c4bb264
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4bb264381
ℹ️ 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".
9313538 to
179c092
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 179c092050
ℹ️ 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".
f463cda to
ae956ef
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae956efed6
ℹ️ 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".
ae956ef to
817d59e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 817d59ed8d
ℹ️ 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".
Rebased onto main after Soju06#1394 ("stabilize silent and clean-close recovery") landed its own bounded eventless response.created watchdog, which already covers this change's original owner-side stuck-gate failover (see recover-fresh-hard-bridge-timeouts's "Fresh hard bridge requests may recover across accounts"). Re-scoped to what's still open on the separate waiter-side gate-replacement path (_http_bridge_can_replace_retired_gate_session): - A waiter whose client already reconnected once (replay_count > 0) is no longer disqualified from transparent replacement on its own — replay count reflects client reconnects, not upstream progress on the current bridge attempt. - A waiter's replacement bridge now excludes the account whose gate it was just waiting behind, so it can't legally land back on the exact account that just proved stuck.
main merged Soju06#1394, adding 20260730_000000_add_http_bridge_recovery_attempts and the no-op merge revision 20260803_000000_merge_http_bridge_recovery_and_capability_lineage_heads, which is now the Alembic head. Re-parenting keeps the merged tree single-headed, per review. The round-trip test's parent constant moves with it, and the test itself relocates to sit directly above the rollups round-trip it mirrors — its previous end-of-file position is where main now appends its own migration tests, so co-locating the round-trips also removes that recurring merge-conflict surface.
* fix(usage): confirm workspace-less paid to free plan downgrades A workspace-less account whose paid subscription expired stayed stored and displayed as paid forever: `_payload_mismatches_account_slot` trusted only workspace-less transitions *into* a recognized paid plan, so every `free` payload was discarded as an identity mismatch. Routing kept honoring the stale paid label and then failed with `no_plan_support_for_model`. Accepting the first `free` payload would give up the degraded/wrong-identity protection that rejection was written to provide, so the downgrade is now confirmed instead of trusted outright: the first workspace-less `free` observation is recorded without mutating the account, and a second consecutive one persists the change. Any recognized paid payload clears the pending state, unrecognized plan values never accumulate, and the differing-`workspace_id` slot-conflict guard stays unconditional. The confirmation threshold is a hardcoded constant, so there is no new setting, schema change, or migration. Also aligns `StubUsageRepository` with the production UTC-naive `recorded_at` contract; its tz-aware fallback raised `TypeError` in the freshness check as soon as a test wrote a usage row and then refreshed the same account again. Fixes #1456 * test(usage): harden workspace-less downgrade confirmation coverage Mutation testing on the confirmation guard surfaced three gaps where a real defect would have shipped green: - The pending-reset test drove the scheduled refresh path, which skips an account whose usage is still fresh. Only two of its three payloads were ever fetched, so the trailing observation went unexercised and a mutant that never cleared the pending state survived. It now drives `force_refresh` and asserts the observation counter directly at each step. - Nothing asserted that the counter is cleared *after* a confirmed downgrade, so a leaked counter survived undetected. - Nothing covered `force_refresh` with an `access_token_override`, the path used by the Codex usage-identity refresh in `app/modules/proxy/api.py`. A mutant that skipped confirmation for exactly that caller survived. Adds `test_force_refresh_confirms_free_downgrade_with_access_token_override` and tightens the two existing tests. No production behavior change. * fix(usage): scope downgrade confirmation to workspace-less accounts Self-review of this branch found that the confirmation path applied to workspace-BOUND accounts too, which the change's own spec never intended: the requirement is written for "a workspace-less account". The outer guard only rejects a payload whose `workspace_id` *conflicts* with the stored one. A payload that omits `workspace_id` entirely falls through to the plan-transition branch even when the account is bound to a workspace, so two such payloads reporting `free` were enough to demote a Team/Business/Enterprise seat to `free` — and that in turn strips plan-based routing eligibility. This is a regression this branch introduced: on `main` the same account is rejected on every observation, because no workspace-less transition into `free` was trusted at all. A payload that never names a workspace cannot establish that it describes a bound account's slot, so confirmation is now refused outright when the stored account has a `workspace_id`, regardless of repetition. Adds the matching spec scenario and a regression test that fails without the guard. * test(usage): assert the pending-downgrade reset is per account A second mutation pass found that making the reset clear *every* account's pending observation, instead of only the reporting account's, survived every existing test. That defect would let one account's recovery silently discard another account's in-progress confirmation, delaying a real downgrade indefinitely. Adds a two-account regression test: both hold a pending observation, only the account that reports a recognized paid plan is reset, and the untouched account still confirms on its own next free observation. * test(usage): isolate downgrade state and pin degraded/casing behaviour Three findings from a deeper self-review pass. 1. The pending-downgrade dict was only cleared by the autouse fixture inside tests/unit/test_usage_updater.py. Integration tests that drive Force probe never cleared it, so a test could leave a pending observation behind and give a later test a head start toward a downgrade. Demonstrated with a two-test integration probe: the second test saw the first test's state. It is now cleared in the shared `_reset_global_state` helper alongside the other process-global caches, which fixes leakage for every suite rather than one file. 2. An unrecognized plan value between two `free` observations does not discard the pending downgrade, so `free -> garbage -> free` still confirms. That is deliberate — a degraded response is absence of evidence, not evidence the account is still paid, and treating it as a reset would let a flapping upstream block a real expiry indefinitely — but it was undocumented and untested. Now pinned by a test. 3. Upstream casing and surrounding whitespace (`FREE`, ` free `) normalize to the same confirmation path and still require two observations. Now covered by a parametrized test. No production behavior change. * docs(usage): specify the confirmation behaviours that only tests covered An audit mapping every spec scenario to a test, and every test back to a scenario, found four behaviours this change relies on that existed only in tests. OpenSpec is this repo's merge gate, so behaviour a reviewer cannot read in the delta is effectively undocumented. Adds scenarios for: - a degraded (unrecognized) payload between two `free` observations, which does not reset the pending downgrade — absence of evidence is not evidence the account is still paid, and treating it as a reset would let a flapping upstream block a real expiry indefinitely; - plan casing and surrounding whitespace, which normalize to the same confirmation path; - clearing one account's pending downgrade leaving another account's intact; - confirmation applying to a refresh performed with an access-token override (the Codex usage-identity path). Also adds a test asserting the shared `_reset_global_state` helper clears the pending-downgrade dict. Mutation testing showed that removing the isolation fix added earlier in this branch was undetectable, so the fix could have been dropped silently. No production behavior change. * fix(usage): make plan-downgrade confirmation cluster-coherent Review follow-up on #1456: the pending observation lived in process memory, so the confirmation sequence diverged whenever replicas shared a database. Replica A recording `free`, replica B observing a paid payload that should clear the evidence, then A observing `free` again confirmed a downgrade the cluster had already contradicted. Conversely two genuine `free` samples split across replicas each stalled at one observation, so a real expiry never converged. Both were reproduced directly against the previous head before this change. Pending observations now live in `account_plan_downgrade_observations`, one row per account, modelled on the existing `account_refresh_claims` table used for cross-replica refresh coordination. Every replica reads and advances the same count; single-replica behavior is unchanged, and the two-observation threshold is untouched. The row also pins evidence to the credential that produced it. Account ids are deterministic and `upsert_account_slot` updates the existing row, so a delete-and-re-import or an in-place reauthentication reuses the id with new token material — previously the new credential inherited the old one's pending observation and downgraded on its own first `free` payload, which is exactly the single-sample trust the guard exists to prevent. The fingerprint is an HMAC over the already-encrypted refresh-token bytes, so no token material is stored, and `ondelete="CASCADE"` removes evidence when the account is deleted. The store degrades to process-local state when the table is absent, so a replica running this code against a not-yet-migrated database keeps refreshing usage instead of failing every cycle; unrelated database errors still propagate. The migration is DDL-only, guarded and idempotent in both directions, and sits on the current single head with no backfill. Fixes #1456 * fix(usage): compare decrypted credentials and record observations atomically Two defects in the persisted downgrade evidence added earlier on this branch, both found by re-auditing it and both reproduced before being fixed. `credential_fingerprint` hashed the stored ciphertext. `TokenEncryptor` wraps Fernet, which embeds a random IV, so the same refresh token encrypts to different bytes every time: any path that re-encrypted an unchanged credential looked like a credential *replacement*, discarded the pending evidence, and reset the count to one. A genuinely expired account could then never accumulate two observations and never converge -- reintroducing the bug this change exists to fix, through a different door. The digest is now taken over the decrypted material, mirroring `_refresh_token_material_fingerprint` in the accounts auth manager, which decrypts for exactly this reason; undecryptable material still falls back to the raw bytes so equality comparisons keep working. The existing stability test compared one `Account` object twice and so never re-encrypted, which is why this went unnoticed; the new test encrypts the same token twice. Recording an observation was a read followed by a write with an `await` between the halves, so two refreshes of one account could both read the same prior count and write the same value, losing an increment. That contradicts the "consecutive observations" property the spec asserts. It is now one atomic operation: a single `INSERT .. ON CONFLICT DO UPDATE` whose `CASE` chooses between incrementing and restarting at one, with the fingerprint comparison inside the same statement, and an in-memory equivalent that updates without an intervening suspension point. Note for future concurrency tests here: `asyncio.gather` alone cannot demonstrate the lost update, because each task runs to its first suspension point and so completes a whole read-write before the next starts -- a non-atomic implementation passes. The regression test uses a barrier released only once every task has entered `observe`, which does expose it. * fix(db): re-parent downgrade-observations migration onto the bridge tool-call revision main gained 20260725_000000_add_http_bridge_pending_tool_calls after this branch forked, leaving two Alembic heads on the PR merge ref that the migration-check jobs validate. Re-parenting keeps the chain single-headed on the merged tree; verified with upgrade head + check on SQLite and PostgreSQL 16 against a merge of current main. * fix(usage): key downgrade evidence to credential lineage and gate hot-path clears Addresses the review on #1491: - The credential fingerprint digests stable seat identity (chatgpt_account_id, chatgpt_user_id, email, codex_installation_id) under a fixed public salt (v2) instead of decrypted refresh-token material. Routine rotation rewrites every token ciphertext on each successful refresh, so the old digest restarted the confirmation count whenever rotation interleaved with usage refresh and a real expiry could be postponed indefinitely. - With no token material in the digest there is nothing to decrypt, so the ciphertext-HMAC decrypt-failure fallback (random IV, never converges) is deleted rather than patched. - Credential replacement resets evidence at the replacement seam: every in-place re-import or reauthentication flows through AccountsRepository._apply_account_replacement, which discards the account's pending evidence in the same transaction, inside a SAVEPOINT so an unmigrated database degrades to a warning instead of poisoning the caller's transaction. Delete + re-import stays covered by ondelete=CASCADE. - PlanDowngradeObservationStore.clear() reads first and takes the writer lock, DELETE, and COMMIT only when a pending row exists, so healthy paid accounts stay read-only on the refresh hot path. - The observe upsert carries explicit String/DateTime bind types, and the new store, probe, and repository tests join POSTGRES_PYTEST_TARGETS so the raw SQL, datetime binds, concurrency, and the missing-schema degrade run on PostgreSQL in CI. All new entries pass against a local PostgreSQL 16.2. - Two-replica unit tests state plainly that they share one in-process store via the autouse fixture; the database-backed evidence lives in tests/integration/test_plan_downgrade_observation_store.py. * docs(openspec): record the lineage fingerprint and hot-path decisions The invalidation requirement now distinguishes credential replacement from routine rotation, a rotation scenario replaces the re-encryption scenario (moot once no token material is digested), and the security notes state exactly what credential_fingerprint is: a non-reversible HMAC under a fixed public versioned salt over stable seat-identity fields, stored outside the encrypted columns, derived without decryption. * docs(contributors): add mrnaturalmonopoly-prog and mrunnaturalmonopoly-svg Both accounts author commits on #1491 (the PR was opened from the first and is being completed from the second), and the Contributors attribution job requires every PR commit author to be listed in .all-contributorsrc. Entries are appended chronologically per the convention restored in #1518; the README grid cells are hand-built to match the regenerated layout because the all-contributors CLI (npm) was not reachable from this environment. * test(db): round-trip the plan-downgrade observations migration Codex review P1 on #1491: the store tests build their schema through Base.metadata.create_all, so nothing executed this revision's upgrade and downgrade functions. The new round-trip test drives Alembic itself: parent -> revision creates the table with the expected shape, downgrade removes it, the guarded upgrade tolerates a table created by a pre-merge build of this same revision, and a final upgrade to head proves the re-parented revision sits on the single-head path. Added to POSTGRES_PYTEST_TARGETS alongside the rollups round-trip it mirrors. * fix(db): re-parent downgrade-observations migration onto capability lineage markers main gained 20260731_000000_add_capability_lineage_markers (#1562) after the last base sync, so the PR merge ref carried two Alembic heads again and every migration-machinery test failed with 'Multiple head revisions are present' — the CI integration-core failure on 4570163. Re-parenting restores the single head; the round-trip test's parent constant moves with it. Verified on a merge of current main: the 22 previously failing migration tests pass, upgrade head + check report a single head with no drift on SQLite and PostgreSQL 16.2, and the round-trip test passes on both engines. The two remaining integration-core failures in that environment reproduce on pure main and are unrelated to this change. * fix(db): track main's migration head onto the 08-03 merge revision main merged #1394, adding 20260730_000000_add_http_bridge_recovery_attempts and the no-op merge revision 20260803_000000_merge_http_bridge_recovery_and_capability_lineage_heads, which is now the Alembic head. Re-parenting keeps the merged tree single-headed, per review. The round-trip test's parent constant moves with it, and the test itself relocates to sit directly above the rollups round-trip it mirrors — its previous end-of-file position is where main now appends its own migration tests, so co-locating the round-trips also removes that recurring merge-conflict surface. * docs(contributors): restore main's contributors after the conflict resolution The web conflict editor for the main sync could only keep one whole side per conflicted JSON object (git interleaved both sides inside single entries), and the branch side won — dropping the five contributors main gained meanwhile. This restores the full union, chronological order, with the PR authors last. --------- Co-authored-by: mrunnaturalmonopoly-svg <mrunnaturalmonopoly@gmail.com> Co-authored-by: Soju06 <qlskssk@gmail.com>
…nt main Maintainer takeover follow-up on top of @mhooooo's original commit: - Reconcile with the post-#1394 streaming flow: keep the recovery-attempt journal state alongside the sealed full-resend proof, and keep the unit test's account-neutral classifier assertion conditional (main now computes neutrality eagerly for the recovery journal on the preserved path). - Update the stale anchor-injection comment: with the sealed proof, durable anchor injection can now also run for a full resend that failed proof verification, not only for non-full-resend payloads. - Add bridge-level failover coverage (codex P1 on #1484): complete a turn on account A through the real create/response-create gate, pause A, and assert the verified session-header full resend fails over to account B with the complete history and no previous_response_id. - Satisfy the current openspec validator by moving the MUST clause onto the first body line of the modified requirement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(proxy): reconcile durable full resend owner * fix(proxy): restack durable full-resend owner reconciliation on current main Maintainer takeover follow-up on top of @mhooooo's original commit: - Reconcile with the post-#1394 streaming flow: keep the recovery-attempt journal state alongside the sealed full-resend proof, and keep the unit test's account-neutral classifier assertion conditional (main now computes neutrality eagerly for the recovery journal on the preserved path). - Update the stale anchor-injection comment: with the sealed proof, durable anchor injection can now also run for a full resend that failed proof verification, not only for non-full-resend payloads. - Add bridge-level failover coverage (codex P1 on #1484): complete a turn on account A through the real create/response-create gate, pause A, and assert the verified session-header full resend fails over to account B with the complete history and no previous_response_id. - Satisfy the current openspec validator by moving the MUST clause onto the first body line of the modified requirement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: mhooooo <n.tanyawit@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Rebased onto main after Soju06#1394 ("stabilize silent and clean-close recovery") landed its own bounded eventless response.created watchdog, which already covers this change's original owner-side stuck-gate failover (see recover-fresh-hard-bridge-timeouts's "Fresh hard bridge requests may recover across accounts"). Re-scoped to what's still open on the separate waiter-side gate-replacement path (_http_bridge_can_replace_retired_gate_session): - A waiter whose client already reconnected once (replay_count > 0) is no longer disqualified from transparent replacement on its own — replay count reflects client reconnects, not upstream progress on the current bridge attempt. - A waiter's replacement bridge now excludes the account whose gate it was just waiting behind, so it can't legally land back on the exact account that just proved stuck.
#1630) * fix(proxy): quarantine silent HTTP bridge sessions 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> * fix(proxy): reset session quarantine flag on clear and expire stale eventless 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> * fix(proxy): close quarantine bypass paths flagged by review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: WangErgouaaaa <117421439+WangErgouaaaa@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Fixes #1393
Summary
Design notes
Recovery is deliberately bounded: one clean-close replay, a two-failure circuit threshold, exponential cooldown, and a shorter cap for clean-close failures. These are internal safety defaults rather than seven new public settings, keeping the settings-surface ratchet unchanged.
The retry circuit uses a durable migration rebased on the current upstream Alembic head. The implementation is split into HTTP-bridge components to remain within repository architecture limits.
The clean-close jitter maximum is intentionally exposed as one bounded public setting because operators may need to tune burst spreading for their upstream deployment; it is capped at 30 seconds and defaults to 2 seconds, while all other recovery limits remain fixed safety defaults.
Validation
The equivalent deployed recovery series also completed a 12-minute soak with 188/188 successful requests, five recovery handoffs, no circuit opens, no incomplete streams or timeouts, and no container restarts.
OpenSpec
openspec/changes/recover-repeated-clean-close
Related work