Skip to content

fix(proxy): recover goal restarts from unavailable owners - #1679

Open
leventov wants to merge 3 commits into
Soju06:mainfrom
leventov:fix/recover-restarted-conversation-affinity
Open

fix(proxy): recover goal restarts from unavailable owners#1679
leventov wants to merge 3 commits into
Soju06:mainfrom
leventov:fix/recover-restarted-conversation-affinity

Conversation

@leventov

@leventov leventov commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

A Codex conversation restart can resend a self-contained thread under the same
process-session identifier after its legacy owner exhausts quota. Raw legacy
codex_session rows are intentionally hard, so ordinary requests must fail
closed; before this change, that same row also trapped an explicit,
self-contained goal restart on an unavailable account.

This PR adds one proof-gated direct-routing exception. A request may retire an
unavailable raw legacy owner only when it carries Codex's recognized
goal-continuation marker and its canonical upstream Responses payload is
account-neutral and self-contained. Retirement is compare-and-set,
policy-scoped, and request-owned. Ordinary, incremental, file-pinned,
conversation-bound, and unresolved-tool requests remain fail-closed.

HTTP bridge reuse/replacement is deliberately split into the dependent #1680.
No public wire format, setting, schema, or default timeout changes.

Linked issue: none exists for this incident-derived defect; routed regression
coverage exercises the public Codex Responses and direct WebSocket paths.

Behavior and safety

  • Recognize the existing goal-continuation context only after canonicalizing the
    request to the upstream Responses body.
  • Permit guarded retirement only for a raw legacy process-session row whose
    owner is durably PAUSED, RATE_LIMITED, or QUOTA_EXCEEDED.
  • Require the persisted owner to be inside the authenticated request's effective
    account-policy scope.
  • Compare mapping owner and unavailable status atomically; a concurrent owner
    change or recovery wins.
  • Exclude a successfully retired owner from the remainder of the same selection,
    even when selection loaded a stale ACTIVE account snapshot.
  • Preserve request-scoped authority through direct WebSocket selection and strip
    only proxy-generated turn state when the restart changes accounts.
  • Keep every unproved or account-dependent continuation hard owner-bound.

OpenSpec

  • Behavior is specified by the archived, verified change at
    openspec/changes/archive/2026-08-10-recover-restarted-conversation-affinity/.
  • Stable requirements and rationale are synchronized in
    openspec/specs/sticky-session-operations/.
  • Parent requirements cover canonical classification, scoped CAS retirement,
    stale-selection exclusion, and direct WebSocket state provenance.
  • HTTP bridge requirements are isolated in the stacked fix(http-bridge): preserve goal-restart recovery across reconnects #1680 delta.

Origin and concurrent work

Landed lineage:

Concurrent work reviewed for overlap:

Review findings addressed

Concrete review findings incorporated in this object:

  1. retirement must be limited to the authenticated account-policy scope;
  2. successful retirement must override stale account-selection snapshots;
  3. accepted compatibility and transport-envelope fields must classify through
    the canonical upstream request body;
  4. proxy-generated direct-WebSocket turn state must not cross the account
    change, while client-supplied state remains hard.

Validation

relevant unit tests: 1,104 passed
focused Ruff check/format: passed
OpenSpec validation: 49 specs passed, 0 failed
git diff --check: passed

Fresh current-head cloud CI is in progress after the object-level history split.

Screenshots / output

No dashboard-visible change.

Before:

marked self-contained restart + unavailable raw owner -> hard-affinity failure

After:

verified marked restart -> guarded CAS retirement -> eligible replacement owner
ordinary/account-dependent request -> unchanged fail-closed ownership

Simplicity

  • Zero configuration; no setting or environment variable.
  • No migration, setup step, README section, or dashboard surface.
  • Reuses existing replay classification, sticky tombstones, and selection.
  • No new retry budget or timeout.

Checklist

  • Conventional Commit PR title.
  • OpenSpec behavior is present, synchronized, verified, and archived.
  • Direct route, direct WebSocket, scoped-policy, and stale-snapshot
    regressions are included.
  • Simplicity gates P1-P5 reviewed.
  • CHANGELOG.md was not edited.
  • Current-head cloud CI green.
  • Current-head Codex review evidence clean or all findings addressed.

Permit an explicit, account-neutral Codex goal restart to retire an unchanged legacy owner only while its persisted account status is unavailable. Preserve fail-closed routing for all other continuity evidence and guard retirement with one compare-and-set tombstone.
@leventov
leventov force-pushed the fix/recover-restarted-conversation-affinity branch from 2412c6d to a8e8199 Compare August 10, 2026 12:08
@Komzpa

Komzpa commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@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: cd554303b9

ℹ️ 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 on lines +513 to +516
policy.codex_session_source == "session_header"
and _request_allows_unavailable_legacy_owner_abandonment(payload)
):
policy = replace(policy, abandon_unavailable_legacy_owner=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the provenance of legacy rows before tombstoning

When a client supplies a session_id equal to a raw row originally written from an explicit x-codex-turn-state, this condition treats the request as process-session provenance and later permits that row to be tombstoned if its owner is unavailable. The legacy namespace deliberately mixes both sources, and these client-controlled headers can collide within the same effective account-policy scope, so current-request provenance does not prove the persisted row's provenance; one session can consequently retire another session's hard turn-state ownership and break its later continuation. Store or migrate row provenance, or keep ambiguous raw rows fail-closed.

AGENTS.md reference: AGENTS.md:L105-L110

Useful? React with 👍 / 👎.

Comment on lines +1804 to +1808
if (
request_state is not None
and upstream is not None
and account is not None
and request_state.affinity_policy.abandon_unavailable_legacy_owner

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Force goal restarts through selection on the HTTP bridge

This forced reselection covers only direct downstream WebSockets. With the default http_responses_session_bridge_enabled=True, a warm HTTP bridge session can be returned by _get_or_create_http_bridge_session before account selection runs; _http_bridge_session_account_active deliberately excludes RATE_LIMITED and QUOTA_EXCEEDED from its routing-unavailable snapshot, so when another request or replica persists either status after the socket opened, the cached session can still appear active. A marked restart is then submitted on the old account without invoking the new tombstone path or selecting a replacement—the added routed regression avoids this by disabling the bridge. Apply the same pre-dispatch retirement and reselection behavior to HTTP bridge reuse.

AGENTS.md reference: AGENTS.md:L105-L110

Useful? React with 👍 / 👎.

Comment on lines +483 to +486
legacy_owner_in_effective_policy_scope = isinstance(sticky_existing_account_id, str) and any(
account.id == sticky_existing_account_id
for account in selection_inputs.effective_continuity_owner_candidates
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check mutation authority before model eligibility

When unavailable owner A is authorized by the caller but does not support the restart's requested model while replacement B does, _load_selection_inputs builds effective_continuity_owner_candidates by model-filtering scoped_accounts, so this test incorrectly treats A as outside the authenticated policy scope. The raw row therefore remains hard, A is absent from the selectable model pool, and the marked restart returns hard_affinity_saturated instead of moving to B. Derive retirement authority from the authenticated account-assignment/security scope before model and service-tier eligibility are applied; keep those latter filters only for replacement selection.

AGENTS.md reference: AGENTS.md:L105-L110

Useful? React with 👍 / 👎.

@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: needs work [@codex review] raised an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants