fix(proxy): scope backend Codex affinity by thread identity - #1703
Merged
Conversation
|
To use Codex here, create a Codex account and connect to github. |
Contributor
Author
Contributor
Author
Komzpa
force-pushed
the
fix/scope-codex-affinity-by-thread
branch
from
August 14, 2026 17:56
268f1db to
26b1134
Compare
17 tasks
Collaborator
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This was referenced Aug 15, 2026
This was referenced Aug 15, 2026
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Codex now sends three deliberately different identities on backend Responses traffic:
session-ididentifies the root agent tree and is shared by root and child threads;thread-ididentifies one actual Codex thread and changes for a child or fork while remaining stable across turns, compact, and resume;prompt_cache_keyintentionally defaults to the sharedsession-id, so it is a cache hint for the root tree rather than a thread identifier.codex-lb still treats the first accepted session header as all three things at once. Because
session-idhas precedence overthread-id, independent root and child threads can share account-affinity, HTTP-bridge, direct-WebSocket replay/tool-continuity, and thread-goal routing scope. The bridge's historical(session header, prompt_cache_key)composition no longer separates those threads because current Codex sends(S, S)for every thread in the tree.This PR separates those roles. Backend Responses, compact, direct WebSocket continuity, HTTP bridge lanes, and thread-goal requests use a versioned, source-separated internal thread key derived from
(process session, thread-id)whenthread-idis available. Requests withoutthread-idretain the existing process-session compatibility behavior. The client-suppliedprompt_cache_keycontinues upstream unchanged, and every exact hard owner remains authoritative.The new account-affinity row is bounded soft locality. A live or durable bridge using the same logical thread identity is hard continuity for the life of that bridge. Those are deliberately different strengths even though they originate from the same request identity.
Type of change
fix:— bug fix (no behavior change beyond the bug)feat:— new user-facing feature or capabilityrefactor:— internal refactor (no behavior change, no API change)docs:— documentation onlychore:/ci:/build:— tooling, CI, packagingtest:— test-only changeLinked issue: Fixes #1702
OpenSpec
Change directory:
openspec/changes/scope-codex-affinity-by-thread/This change does not introduce a competing affinity capability. Its delta spec modifies the existing backend-session, bare-process-session, and unanchored bridge-concurrency requirements. In particular, it removes the now-false statement that an explicit
prompt_cache_keydistinguishes Codex agent threads.Upstream identity contract
The design follows the contract established by upstream Codex rather than inferring identity from subagent markers:
session-idthread-idprompt_cache_keysession-id; explicit overrides remain allowedx-codex-turn-stateprevious_response_idThe relevant upstream history is:
SessionIdspecifically so every thread under one root can share a session ID while retaining a distinct thread ID.prompt_cache_keyfrom thread ID to session ID. Its regression test asserts that root and child have differentthread-idvalues but the same session-based prompt-cache key.session-idfor a resumed child while retaining the child's ownthread-id. Remote compact tests keep the thread identity stable, and fork creates a different thread identity.Consequently, changing or rewriting the upstream
prompt_cache_keyto make it thread-specific would be a protocol divergence, not a fix.Regression origin and relationship to earlier codex-lb work
This is protocol-evolution drift across otherwise valid earlier changes:
prompt_cache_keyfor sequential child isolation. Upstream #33035 landed afterwards and collapsed that composition from(S, thread-cache-key)to(S, S).thread_headerthrough hard-kind, owner-forwarding, durable-operation, and account-neutral full-resend recovery paths without weakening its fences.thread-id.Confirmed defects
Session-header parsing hides thread identity
The shared session-header parser currently accepts these names in order:
session_id,session-id,x-codex-session-id,x-codex-conversation-id, andthread-id. A normal current Codex request has bothsession-idandthread-id, so the process session always wins. Callers cannot tell whether the returned string is tree identity or thread identity.HTTP bridge composition has collapsed
The bridge comments and OpenSpec requirement assume every child has a stable, child-specific explicit
prompt_cache_key. Current Codex deliberately sends the shared process session instead. Hashing(session-id, prompt_cache_key)therefore gives the same canonical key to root and children, including sequential requests that do not trigger the concurrent request-scoped fork path.Direct WebSocket continuity is process-scoped
The direct WebSocket replay/tool-continuity cache is indexed through the same session-header parser. Independent threads under one Codex process can therefore read or replace one process-wide retained continuity state. Fixing only account selection or bridge keying would leave this correctness leak.
Responses and compact account locality are process-scoped
Both paths classify a bare backend session before considering any other soft locality. Every thread in a root tree therefore competes for one process-level mapping. Exact turn state, response, file, and bridge owners are already stronger and remain so.
Thread-goal routing ignores its exact payload identity
/backend-api/codex/thread/goal/{get,set,clear}carriesthreadIdin the payload but currently selects an account using only generic request headers. Once distinct threads can establish different account locality, goal access must use the payload's exact thread identity rather than the broader process session.Request-log conversation grouping is already correct
Codex request logs use the raw
thread-idasconversation_id. This PR does not change that behavior.RequestLog.session_idand internal owner-lookup scope are separate concerns and must not be rewritten merely to make the dashboard grouping look uniform.Production evidence and its limits
Read-only aggregate analysis of one long-running root-tree session found:
thread-idvalues sharing one process session;This proves that an account change is expensive. It does not prove that every status-line account change is caused by stale identity granularity:
stream_incomplete,websocket_connection_limit_reached,previous_response_not_found, or a client disconnect;The acceptance claim for this PR is therefore bounded: it removes cross-thread identity collisions and makes reconnect/account locality stable per thread. It does not promise that health failover, quota exhaustion, explicit restart, or concurrent completion can never expose another account.
Changes
1. Parse identities independently
thread-idinto separate typed fields.thread-idas merely the last alias in a generic session header list.(process session, thread-id)when both are present.thread-idis present, derive a separate thread-only key rather than falling back to a client-reproducible raw identity.thread-idis absent, preserve the current process-session behavior.The derivation is namespacing, not secrecy. Its encoded source/version must be structurally unreachable by normalized client headers, matching the safety property introduced for bare process-session keys in #1382.
2. Use bounded thread locality for backend Responses and compact
prompt_cache_keyunchanged in the upstream body.thread-id.Initial placement and cache-sharing boundary
Upstream deliberately gives sibling threads the same prompt-cache key, so a new thread should not be distributed to another account merely because its thread-local row does not exist yet. Initial thread placement must prefer an eligible existing process-session soft owner, then persist the admitted account under the bounded thread key. If the process-session owner cannot be used and the request is genuinely movable, ordinary safe selection can seed the thread on an alternate account without rewriting the process row.
Current Codex sends
thread-idon the first root request, so no earlier bare process request may exist to initialize that preference. A normal first admission therefore inserts the process preference if absent and upserts the initiating thread row in one repository transaction. Insert-if-absent is essential because concurrent siblings may race; atomicity is essential because a process default without its initiating thread is false affinity evidence. A recovery-probe reservation remains reversible until its runtime CAS commits, so it persists only the thread row and does not publish an immutable process default that cannot safely be compensating-deleted after another sibling sees it.After a thread row exists, that row supplies its reconnect locality. A sibling that later moves because of a justified failure must not move every other thread sharing the process session. This two-level behavior preserves session-wide cache colocation by default while containing necessary divergence to one thread.
The existing selection architecture expresses this without a second policy engine, new setting, schema, or durable child row.
3. Scope direct WebSocket continuity by thread
thread-idto the old process-wide in-memory continuity entry: that state may belong to a sibling.No durable migration is needed for this in-memory cache. A process replacement already drops it.
4. Scope HTTP bridge lanes by thread
Existing bridge compatibility
Do not blindly fall back to the old
(session-id, prompt_cache_key)canonical key whenthread-idis present. With current Codex that key is shared(S, S)and can identify a sibling's bridge.An old bridge may be recovered only through exact hard evidence already tied to that bridge, such as a client turn-state or previous-response alias. A request with only soft thread identity creates/uses the new canonical lane; unaliased old lanes expire through existing idle/durable cleanup. Mixed-version tests must prove that old and new canonical rows may coexist without one thread attaching to another thread's history, that trusted forwarded keys are not re-derived, and that an exact old alias remains usable until natural retirement.
5. Route thread-goal requests by payload thread identity
threadIdas the exact thread identity for goal get/set/clear account locality.6. Preserve every hard owner and legacy check
The following remain authoritative and are outside soft affinity mobility:
x-codex-turn-state;codex_sessionrows that may represent historical turn-state ownership.Raw legacy process-session lookup must run separately from the new thread selection key. A raw hit wins conservatively; disagreement with another hard owner fails closed. The implementation must not globally repurpose the current
_owner_lookup_session_id_from_headers()helper, because its callers mix request-log scope, response-owner lookup, direct-WebSocket continuity, and legacy compatibility. Introduce role-specific typed helpers instead.Rejected alternatives
Adapt #1309's marker/TTL/schema design
Rejected. It adds a setting, migration,
is_subagentstate, dashboard surface, and special cleanup lifecycle to infer information now present directly inthread-id. It also applies only to recognized child markers, while root threads, resumes, forks, and clients without those markers still need correct identity. The open PR's review history illustrates the resulting keying, forwarding, reuse, cleanup, and parent-row coupling.Detect only
x-openai-subagentorx-codex-parent-thread-idRejected. Those headers describe request role/provenance, not the child's identity. Marker presence can be missing, values can identify the parent, and ordinary root/fork/resume traffic still has thread identity. Marker values must remain compatibility metadata, not affinity keys.
Use
x-codex-parent-thread-idas the child keyRejected. It groups children by their parent and therefore reproduces the same collision one level down. It also changes meaning across nesting depth and cannot identify the root thread.
Treat explicit
prompt_cache_keyas the thread identifierRejected after upstream archaeology. This was true when #1169 was designed, but upstream #33035 intentionally changed the default to shared
session-idand tests that root and child use the same cache key.Derive affinity from
(session-id, prompt_cache_key)Rejected. For current Codex it reduces to
(S, S)for every root and child, so it is only a more opaque spelling of the existing process-wide collision.Rewrite
prompt_cache_keytothread-idRejected. It contradicts current upstream behavior and discards deliberate root-tree cache sharing. codex-lb must forward an explicit client cache key unchanged.
Use
thread-idalone everywhereRejected as the general representation. Source/version separation prevents a client-controlled raw value from colliding with historical hard rows, and including process session preserves namespace separation between process trees. Thread-only derivation is only the compatibility fallback when no process session exists.
Use one thread key with one strength and lifetime everywhere
Rejected. Identity and ownership are not the same concept. Sticky account locality is bounded and soft; a live/durable bridge, exact response, file, or turn state is hard. Sharing a derivation helper must not flatten these capabilities.
Keep account/cache locality process-wide and change only bridge/replay state
Rejected as the complete fix, though it is the minimum correctness subset. It would stop cross-thread history/replay contamination but leave every thread reconnect competing for one process-wide soft mapping. The selected design instead seeds new threads from the process preference to preserve default cache colocation, then gives each thread stable bounded locality so a justified divergence does not move its siblings.
If implementation cannot preserve that initial seeding without significant new machinery, this narrower alternative must be reconsidered explicitly; it must not reappear accidentally through a missing-row fallback.
Independently balance every previously unseen thread
Rejected. It would distribute siblings across accounts even while the shared process owner is healthy, defeating the cache-sharing intent behind upstream
prompt_cache_key=session-id. A thread key is a containment boundary, not a reason to force immediate distribution.Create durable per-thread
CODEX_SESSIONrowsRejected. Durable rows are not automatically cleaned up and would turn every short-lived child into permanent routing state. The ordinary locality row must use an existing bounded lifecycle; only proven bridge/object ownership is durable/hard.
Add a new sticky-session kind, database schema, or setting
Rejected as unnecessary. Existing typed source provenance, bounded sticky lifecycle, and hard-owner machinery can express the behavior. A new enum, migration, dashboard control, or operator knob would add compatibility and cleanup obligations without resolving identity more accurately.
Rebind the shared process-session row after spill/failover
Rejected. Moving that row moves every sibling, requires publication and rollback across transport admission, and recreates the distributed settlement problem rejected by #1382. A selected alternate may establish one thread's bounded row without rewriting the process default.
Rely only on #1169 request-scoped parallel fork lanes
Rejected. Those lanes correctly handle concurrent unanchored collisions but do not separate sequential root/child reuse once
(session-id, prompt_cache_key)has collapsed.Fall back to the legacy bridge canonical key during migration
Rejected for requests with
thread-id. The old key can denote any sibling in the tree. Only exact hard aliases can safely recover an old bridge.Change request-log conversation grouping or owner lookup wholesale
Rejected. Conversation grouping already uses
thread-id. Request logging, response-object ownership, soft selection, bridge identity, and in-memory replay are different roles. A global helper replacement would create migration and ownership regressions unrelated to the defect.Non-goals
Safety invariants
prompt_cache_keyis forwarded unchanged.Expected implementation footprint
The change is conceptually narrow but crosses the places that currently reuse the generic session parser:
It should not require schema, settings, dashboard, API response-shape, or upstream payload changes. If implementation expands into those areas or needs a generalized owner-resolution rewrite, stop and reassess scope. The bridge migration is the only expected deep compatibility boundary; if it alone makes the PR exceed one reviewable object-level concern, split it as a stacked PR over the typed identity/account-affinity change rather than broadening the base PR.
Test plan
This PR includes focused product-path regressions for:
thread-idonly, process session only, blank values, mixed casing, and values that resemble internal keys;(session-id, prompt_cache_key)but receiving distinct internal thread keys while the upstream payload remains unchanged;thread-idnever reconstructing the shared process lane after a missing generated turn alias;threadId;thread-idis absent;conversation_id=thread-idremaining unchanged;Commands run on the final local tree:
Validation
tests/unit/test_proxy_utils.py: 1007 passed.tests/unit/test_proxy_http_bridge.py: 548 passed on the rebased head.ty, all 49 main OpenSpecs, and the strict change validation passed.Repository-wide
ty checkcurrently reports four diagnostics in untouchedwebsocket/helpers.pyandtool_call_dedupe.py; changed-file type checking is clean. GitHub CI and current-head cloud Codex review remain merge gates after publication and are not inferred from local results.Simplicity
CODEX_LB_*setting..env.exampleentry, dashboard navigation, or UI.Screenshots / output
Not applicable: no dashboard-visible change. The product-path regression
test_backend_thread_rows_route_sibling_responses_and_compact_independentlydemonstrates distinct root/child account locality while both upstream calls retain the same process-wideprompt_cache_key.Checklist
Fixes #N/Closes #N, or stated partial coverage explicitly.openspec validate --specsand/opsx:verifyare clean.@codex reviewis clean or every finding is addressed.mergeable=CLEAN.CHANGELOG.mdis not edited manually.