fix(proxy): purge stale hard codex_session mappings pinned to a durably unavailable owner

[softkleenex]

Summary

A codex_session-kind sticky mapping is hard-bound and never rebound at
request time, even once its owner account becomes unavailable
(rate-limited/quota-exceeded/paused) — correctly, per
sticky-session-operations, since the mapping can represent live,
unverifiable session state (mid-flight tool calls, account-scoped state)
that isn't safe to move to a different account mid-session.

But that protection has no expiry. If the owner never recovers, every
future request against that session fails closed with
previous_response_owner_unavailable ("Hard affinity owner account is
unavailable") indefinitely. We hit this in production and had to delete
~250 stale mappings by hand directly in the database to unblock it.

This adds a bounded exception, enforced only by a periodic background purge
— never by the hot-path selection logic, and never by rebinding:

• A new repository method,
  StickySessionsRepository.purge_stale_hard_codex_session_mappings, deletes
  codex_session mappings whose account is non-active (PAUSED,
  RATE_LIMITED, or QUOTA_EXCEEDED) and whose mapping hasn't actually
  been reused since well before a cutoff.
• The existing leader-elected StickySessionCleanupScheduler (already
  running every 300s) calls this once per cycle with a fixed 6-hour
  threshold — deliberately far longer than any ordinary quota-reset window,
  so a transient blip never loses its mapping.
• Deletion only, never rebinding: once a stale mapping is gone, the next
  request against that session simply re-resolves fresh, exactly as it
  already does today for a session with no mapping at all.

load_balancer.py's hard_sticky selection branch is untouched — the
correctness invariant it protects (never reallocate mid-flight to an
unverified account) is preserved; this only changes what eventually happens
to an already-abandoned mapping between requests.

An earlier version of this gated on Account.reset_at/blocked_at, which
looked right on paper but doesn't hold up against real account data:
reset_at is frequently unset (upstream hasn't reported fresh quota data),
and blocked_at is explicitly cleared to None on pause. Gating on
StickySession.updated_at instead — "how long since this mapping was
actually last used" — is both simpler and correct across all three
statuses. See design.md in the linked OpenSpec change for the full
rationale.

Full proposal, design rationale, and spec delta:
openspec/changes/purge-stale-hard-codex-session-mappings/.

Test plan

• New repository-level regression: a fresh (recently-used-despite-owner-
  being-down) mapping survives; a durably stale one (owner non-active
  and unused past the cutoff) is purged; a healthy owner's mapping is
  never touched
  (tests/integration/test_proxy_sticky_sessions.py::test_purge_stale_hard_codex_session_mappings_only_drops_durably_unavailable_owners)
• Scheduler-level regression that the new purge call happens once per
  cleanup cycle with the expected cutoff
• Updated the pre-existing scheduler test whose docstring asserted
  codex_session mappings are "never purged" by this job
• uv run pytest — full suite green except one pre-existing, unrelated
  flaky integration test
  (test_quota_planner_warm_now_keeps_bootstrap_for_metadata_less_primary_rows,
  confirmed to fail identically on unmodified origin/main)
• uv run ruff check . / uv run ruff format --check . — clean
• uv run ty check — clean

Note: like my other recent PRs, CI's Lint (ruff) check will likely still
show red here due to the pre-existing, unrelated service.py/
load_balancer.py architecture line-count gate failures on current main
(see #1416 and my comments on #1408/#1401) — unrelated to this diff.

[Komzpa]

Thanks — #1435 has now landed and realigned the proxy architecture ratchets. Please rebase this branch onto current main and rerun CI. The maintainer update-branch path was attempted, but GitHub refused it because the merge touches .github/workflows/codex-review-labels.yml and the current credential cannot update workflow files; that is a control-plane limitation, not a new source finding on this PR.

[softkleenex]

Rebased onto current `main` (post-#1435/#1437), no conflicts — this branch only touches `sticky_repository.py`/`cleanup_scheduler.py`, disjoint from #1437's changes. Architecture check, ruff, ty, and the full test suite are all green (aside from the pre-existing, unrelated flaky `test_quota_planner_warm_now_keeps_bootstrap_for_metadata_less_primary_rows`, and two `test_metrics.py` tests that fail identically on a clean checkout of current `main` in this environment — confirmed via a separate worktree, unrelated to this diff).

[Komzpa]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f79ceb737d

ℹ️ 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]

Ran a local review pass on ad15548 after getting CI green (the two failures were flakes — a Set up uv infra failure on integration-core-1 and an event-loop-teardown flake in test_http_responses_bridge; rerun passed). Two findings to address before merge, both against this PR's own spec delta:

1. purge_stale_hard_codex_session_mappings (app/modules/proxy/sticky_repository.py:273-285) purges at the fixed 6h cutoff even when Account.reset_at is known and still in the future — e.g. a weekly quota exhaustion days out. The spec text requires the owner to be unavailable "well past its own recovery point", so a known future reset_at should extend the gate, with the 6h constant as the fallback when it's unset.
2. Rollout gap: the outage-start refresh (accounts/repository.py, _refresh_hard_sticky_outage_grace) only fires on status transitions after deploy. Mappings whose owners were already PAUSED/RATE_LIMITED/QUOTA_EXCEEDED at upgrade keep their pre-outage updated_at, so the very first cleanup cycle can purge a mapping whose outage began minutes before deployment — violating the "merely transient outage is never purged" scenario for the upgrade window. Seeding the grace clock at startup for already-unavailable owners (or a one-cycle rollout grace) would close it.

The overall shape — background-only purge, delete-never-rebind, hot path untouched — is right, and the earlier updated_at concern is properly fixed by ad15548. Happy to merge once these two are handled.

[softkleenex]

Pushed `0ae21cc9` addressing both findings from the last review:

1. `purge_stale_hard_codex_session_mappings` now excludes any account whose `reset_at` is known and still in the future, regardless of how stale its mapping looks by the flat cutoff — the owner's own stated recovery point takes priority. An absent `reset_at` still falls back to the cutoff-only behavior, so this only ever narrows eligibility (delays a purge), never widens it.
2. Added `AccountsRepository.seed_hard_sticky_outage_grace_on_startup`, called once during app boot (`app/main.py`), to close the rollout-window gap: the status-transition hook only fires on a live transition, so an account already unavailable before this process started would otherwise keep its pre-outage `updated_at`, and the very first cleanup cycle after deploy could mistake a brand-new outage for an ancient one. Safe to call on every restart — re-seeding only ever delays a purge, never causes one.

Updated the OpenSpec proposal/design/spec delta to cover both, and added regression coverage:

• `test_purge_stale_hard_codex_session_mappings_only_drops_durably_unavailable_owners` now includes a future-`reset_at` account that must survive despite a stale mapping.
• New `test_seed_hard_sticky_outage_grace_on_startup_refreshes_only_unavailable_accounts` covers the startup seeding path directly.

Full suite green aside from the three pre-existing, unrelated failures already confirmed to fail identically on a clean `main` checkout in this environment (`test_quota_planner_warm_now_keeps_bootstrap_for_metadata_less_primary_rows`, and two `test_metrics.py` prometheus-availability tests). `ruff`, `ty check`, and the architecture gate are all clean.

[Soju06]

Thanks for 0ae21cc — the future-reset_at gate and the startup grace seeding both address the earlier findings. Local review at this head surfaced two remaining issues before this can land:

1. Purging the sole owner proof can strand turn-state sessions. When the purged mapping is the only owner proof for a request carrying a non-empty conversation and the scoped pool has multiple accounts, _affinity_with_payload_continuity sets require_unambiguous_account and run_sticky_selection_path rejects the mapping miss as ambiguous — so the session doesn't re-resolve fresh, it gets stuck, and stays stuck even after the owner recovers. Please limit purging to mappings that can actually be re-resolved (or record enough metadata to explicitly authorize abandoning continuity for them).

2. Startup seeding resets the grace clock on every boot (app/main.py:390). In a shared-database/multi-replica deployment, each new process start rewrites the grace timestamps for all currently-unavailable owners; with deploys or autoscaling more frequent than the cutoff, durably dead mappings never age out. A durable one-time rollout marker (or persisting the actual outage-observed timestamp) avoids the reseed.

CI is green and the rest looks ready — happy to run the gates again after these two.

[softkleenex]

Thanks for the thorough second pass — both fixed in 966bd61.

1. Purging the sole owner proof strands turn-state sessions

Went with the "record metadata to explicitly authorize abandoning continuity" option, implemented as a tombstone-then-delete purge instead of a one-shot delete:

• purge_stale_hard_codex_session_mappings now sets a new StickySession.continuity_abandoned_at column (migration 20260727_000000_add_sticky_session_continuity_abandoned_at) instead of deleting on the first pass.
• get_account_id_and_abandonment reports a tombstoned row as ownerless (same as a missing row, so hard_sticky is still correctly false) but also flags it as abandoned.
• run_sticky_selection_path uses that flag to exempt the row from the ambiguous-conversation-owner check specifically — not because the pool stopped being ambiguous, but because we know this key's owner was durably unavailable and continuity was deliberately abandoned, which is exactly the authorization that check exists to require. Selection then proceeds through the normal path, picks a fresh account, and the resulting upsert/restore_if_current write clears continuity_abandoned_at, fully restoring hard_sticky for that key.
• A tombstone nobody claims is dropped outright once it's sat unclaimed past a further grace window (aged off continuity_abandoned_at, not the original updated_at), so it doesn't accumulate forever — by then falling back to the same fail-closed default as a key that was never seen is fine.

New coverage: test_tombstoned_hard_owner_lets_conversation_continuity_reselect (unit, exercises the actual bypass end to end with a 2-account pool) plus repository-level tombstone/grace/delete-phase tests.

2. Startup seeding resets the grace clock on every boot

Replaced the every-boot reseed with a one-time-per-database backfill gated on a durable marker in runtime_sentinels — the same insert-if-absent table key_fingerprint.py's encryption-key check already uses. The seeding method atomically inserts a hard_sticky_outage_grace_seeded sentinel with ON CONFLICT DO NOTHING; only the replica whose insert actually lands (first ever, for this database) runs the backfill loop. Every later boot, on any replica, finds the sentinel already stamped and returns 0 immediately, so a fast redeploy/autoscaling cadence can no longer perpetually push out a durably-dead mapping's grace window.

Both OpenSpec docs (proposal/design/tasks/spec) updated to match. Full suite green (ty check, ruff check/format, architecture-check script, and the full test suite — 6476 passed, only the one previously-flagged pre-existing test_quota_planner_warm_now_keeps_bootstrap_for_metadata_less_primary_rows flake, unrelated to this change and reproducible on the unmodified base commit).

[Soju06]

Thanks — 966bd61 resolves both remaining findings, and the designs are the right ones.

1. The tombstone-then-delete purge closes the stranding case cleanly: continuity_abandoned_at gives run_sticky_selection_path exactly the authorization signal the ambiguous-owner check requires, upsert/restore_if_current clearing it on re-pin restores full hard_sticky semantics, and the delayed second-phase delete converging to the same fail-closed default as a never-seen key is an acceptable terminal state. test_tombstoned_hard_owner_lets_conversation_continuity_reselect covers the recovery end-to-end.
2. The runtime_sentinels insert-if-absent marker (ON CONFLICT DO NOTHING ... RETURNING) is the correct once-per-database gate — reusing the same mechanism as the key-fingerprint check, with the second-boot no-op tested.

Two mechanical items before this can land, both from main moving underneath you:

• Rebase conflict with feat(proxy): support Codex Live Voice sideband #1492 (Codex Live Voice sideband). It rewrote StickySessionsRepository.get_account_id in app/modules/proxy/sticky_repository.py — the max-age expiry is now a race-safe predicate-guarded DELETE plus re-read instead of a simple delete-and-return-None. Your get_account_id_and_abandonment split needs to be folded into that new structure (the tombstone check should apply to whichever row survives the re-read), and tests/integration/test_proxy_sticky_sessions.py conflicts as well. Note feat(proxy): support Codex Live Voice sideband #1492 also introduced reserved \ncodex_live_call: keys under CODEX_SESSION kind; your purge can match them, but their own 2h max-age and 5-minute cleanup retire them well before the 6h cutoff, so no change is needed there — just keep the reserved-key tests green after the merge.
• Migration re-parent. Every red CI job is the same root cause: alembic "Multiple head revisions". Your new migration revises 20260720_000000_add_request_log_conversation_id, but main's head is now 20260724_000000_add_request_usage_time_rollups (via 20260722_000000_backfill_request_log_useragent_families). Point down_revision at 20260724_000000_add_request_usage_time_rollups when you rebase and the migration checks and test suites should all come back.

No source-level findings remain — once the rebase lands with CI green I'll run the review gates one more time and merge.

[Soju06]

Friendly ping @softkleenex — nothing new is needed beyond the 07-27 note: fold get_account_id_and_abandonment into the post-#1492 race-safe get_account_id shape (the conflicts are confined to sticky_repository.py and test_proxy_sticky_sessions.py) and re-parent the migration onto 20260724_000000_add_request_usage_time_rollups. This is fix-class, so it's exempt from the beta-soak train and can land ahead of the stable cut — I'll run the final review gates as soon as the rebase is up.

[softkleenex]

Rebase is up (`9bd7354e`).

• Folded `get_account_id_and_abandonment` into feat(proxy): support Codex Live Voice sideband #1492's race-safe predicate-guarded-DELETE-plus-re-read shape: the abandonment check now applies to whichever row survives the re-read (fresh row, deleted-by-race, or a concurrently-rebound row), not just the initial read. Access is by attribute on a small `StickyOwnerLookup` result (never tuple-unpacked), so an unconfigured test double for `sticky_sessions` degrades to its existing safe defaults instead of crashing.
• Left `purge_before_for_key_prefix` (reserved `codex_live_call` namespace cleanup) untouched — confirmed its own 2h max-age + 5-minute cleanup retires those rows well before my 6h cutoff, so no interaction with the tombstone-then-delete purge.
• Re-parented the migration onto `20260725_000000_add_http_bridge_pending_tool_calls`, which is main's actual current head (one more migration landed after the 07-30 note's `20260724_...` pointer).
• Squashed the branch's 4 commits into one for the rebase — happy to re-split if you'd prefer reviewing it that way.

`ty check`, ruff check/format, the architecture-check script, the migration/autogenerate-drift tests, and the full suite (6892 passed, 76 skipped, only the previously-flagged unrelated `test_quota_planner_warm_now_keeps_bootstrap_for_metadata_less_primary_rows` flake deselected) are all green locally. CI just kicked off on the force-pushed branch.

[softkleenex]

One more hardening pass while waiting on review: added test_stale_expiry_race_reread_reports_concurrent_tombstone (47304b33) — covers the one branch the rebase touched that had no direct test yet: when the race-safe expiry path's delete-on-expiry predicate misses (because a concurrent write moved updated_at) and it re-reads, the abandonment check now applies to that re-read row, not just the initial snapshot. Full suite (6893 passed), ty, ruff, and the architecture-check script all still green.

[softkleenex]

Heads up: Tests (pytest, integration-bridge) failed on the latest run (47304b33) with test_v1_responses_http_bridge_preserves_full_resend_before_fresh_bridge_send hitting bridge_instance_mismatch (409 instead of 200). This looks like CI-runner timing flakiness in an unrelated multi-instance-ownership test, not something my last push touched — it doesn't reproduce locally either in isolation or running the full tests/integration/test_http_responses_bridge.py tests/integration/test_proxy_websocket_responses.py group matching that CI job (187 passed). I don't have rerun rights on the workflow run from here; flagging in case a re-run on your end resolves it before the final gates.

[Soju06]

The 08-01 rebase predates #1562 landing (08-01 16:40 UTC), and main's alembic head has moved again: 20260731_000000_add_capability_lineage_markers and 20260803_000000_merge_http_bridge_recovery_and_capability_lineage_heads now sit after this branch's migration parent (20260725_000000_add_http_bridge_pending_tool_calls), so merging as-is recreates the multiple-heads failure on main — the green migration checks on this PR ran against the pre-#1562 merge ref. Please re-parent 20260727_000000_add_sticky_session_continuity_abandoned_at onto 20260803_000000_merge_http_bridge_recovery_and_capability_lineage_heads (or add an alembic merge revision). No other interaction with #1562 — it doesn't touch the sticky-session surface. The integration-bridge failure on 47304b3 matches the known bridge_instance_mismatch flake profile; the re-parent push will trigger a fresh run either way.

[softkleenex]

Re-parented (12c57705) onto 20260803_000000_merge_http_bridge_recovery_and_capability_lineage_heads. Confirmed the rebase itself is conflict-free (matches your read that #1562 doesn't touch the sticky-session surface) and single-head migration checks pass locally.

Also checked the test_proxy_websocket_responses.py failures that showed up in the full local suite after this rebase (test_backend_responses_websocket_masks_anonymous_previous_response_not_found_with_inflight_request, test_backend_responses_websocket_does_not_expire_downstream_while_request_pending) — both reproduce identically on a clean, unmodified upstream/main checkout (currently 0095b6ea), so they're pre-existing failures at main's current tip, unrelated to this branch. Flagging in case that's not already known; not something I've touched or need to fix here.

ty check, ruff check/format, architecture-check, and the full suite (7122 passed, 2 pre-existing-on-main failures noted above, 1 previously-flagged unrelated flake deselected) all otherwise green locally.

[softkleenex]

CI is green (mergeStateStatus: CLEAN) after the re-parent.

[Komzpa]

@codex review

[chatgpt-codex-connector]

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