Skip to content

fix(server): detaching a Codex thread unloads it from the shared app-server - #14187

Merged
juliusmarminge merged 2 commits into
t3code/codex-turn-mappingfrom
v2/gi-13331-codex-unload-detached
Sep 29, 2026
Merged

juliusmarminge merged 2 commits into
t3code/codex-turn-mappingfrom
v2/gi-13331-codex-unload-detached

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Addresses #13331 (item 4).

All Codex threads share one codex app-server (supportsMultipleProviderThreadsPerSession: true). Detaching a thread from that shared runtime (settle, archive, workspace change, selection change, handoff) only removed it from attachedThreadIds and loadedProviderThreadKeyByThread. Nothing told the app-server, so the native thread stayed loaded along with its MCP servers until the whole runtime was released. The reporter measured about 80 idle processes and 9 GB over a day.

What changed

  • ProviderAdapterV2SessionRuntime gets an optional unloadThread({ providerThread }). Only runtimes shared by several app threads need it.
  • CodexAdapterV2 implements it with thread/unsubscribe. Codex 0.156 source (codex-rs/app-server/src/request_processors/thread_lifecycle.rs, UnloadingState): a thread with no subscribers that is not Running is shut down after thread_unload_delay (default 60s, core/src/config/mod.rs), which also cancels its pending server requests and emits thread/closed. A running turn is not cut off; Codex waits until it is idle. A later thread/resume reloads the thread, as the existing resume path already does after an idle release.
  • ProviderSessionManager.detach: when the shared runtime stays up after the detach, call unloadThread for the detaching app thread's provider threads on that session (those with a native ref). This runs after the existing interrupt of their running turns. A failed unload is logged (detach-unload-failed) and does not fail the detach. Exclusive runtimes are unchanged: they are released outright when their last thread detaches.

The other adapters are not shared multi-thread runtimes, so they don't implement it and nothing changes for them.

Verification

  • ProviderSessionManager.test.ts: the existing "opens one shared runtime … detaches threads independently" test now records unloadThread calls in its fake runtime. It asserts that the thread with a persisted provider thread is unloaded after its turn is interrupted, and that the one without is not. Fails on the base branch, passes with the fix.
  • CodexAdapterV2.test.ts, new replay test unsubscribes from the native thread when it is unloaded: it expects the exact outbound frame {"method":"thread/unsubscribe","params":{"threadId":…}}. The inbound {"status":"unsubscribed"} response is the one recorded from a live codex app-server 0.156.1. A live probe returned unsubscribed, then notSubscribed on a repeat, then notLoaded for an unknown id. Fails on the base branch, passes with the fix.
  • vp test run src/orchestration-v2/ProviderSessionManager.test.ts src/orchestration-v2/Adapters/CodexAdapterV2.test.ts: 158 passed.
  • vp exec tsc --noEmit -p . in apps/server: no errors. vp lint on the touched files: only two pre-existing unused-variable warnings in CodexAdapterV2.ts. vp run knip:check: clean.
  • Not run: the reporter's end-to-end MCP process count (11 → 0).

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

…server

All Codex threads share one app-server. Detaching a thread (settle, archive,
workspace or selection change) only dropped it from T3's bookkeeping; the
native thread stayed loaded in the app-server with its MCP servers running
until the whole runtime was released, so idle processes piled up over a day.

Add an optional unloadThread to the adapter session runtime. Codex implements
it with thread/unsubscribe, which lets the app-server shut the native thread
down once it is idle and unsubscribed. ProviderSessionManager calls it for the
detached thread's provider threads when a shared runtime stays up for others.
Failures are logged and do not fail the detach.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 28, 2026
Comment thread apps/server/src/orchestration-v2/ProviderSessionManager.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d11ee84

Macroscope's review found this PR approvable — This is a bounded Codex lifecycle fix that unloads detached native threads without closing shared runtimes, with explicit timeout handling and regression coverage for reattachment races. Error propagation is also tested, and no product defaults, schemas, security-sensitive paths, or static-analysis suppressions are changed.

Notes:

  • This verdict was updated automatically after the outstanding correctness findings were resolved. Macroscope did not re-review the code.

You can add or adjust custom eligibility rules. Learn more.

Comment thread apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts Outdated
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.1 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 1 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.7 KiB — 29.3 KiB ✅
Claude Live turn messages — 1 — 8 ✅

Baseline: unavailable · PR result: d11ee84 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Detach removed the attachment, then called unloadThread. The same thread's
next turn could re-attach and resume in between, and the late unload then
tore down the reloaded native thread. Serialize a thread's attach with the
detach's unload on a per-session, per-thread lock: an unload skips a thread
that already re-attached, and an attach that arrives mid-unload waits for it,
so its resume reloads the thread. The unload is bounded so a wedged provider
cannot hold the next attach.

Also keep the app-server failure as the cause of a rejected Codex unload
instead of storing it as the protocol error's payload.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@juliusmarminge
juliusmarminge merged commit a3f967a into t3code/codex-turn-mapping Sep 29, 2026
24 checks passed
@juliusmarminge
juliusmarminge deleted the v2/gi-13331-codex-unload-detached branch September 29, 2026 01:07
Comment thread apps/server/src/orchestration-v2/ProviderSessionManager.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant