Skip to content

refactor(server): remove the unused V1 Codex session runtime - #13462

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/codex-v1-removal
Sep 24, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/codex-v1-removal

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

makeCodexSessionRuntime (the V1 Codex session runtime, ~2,700 lines) has had no production caller since the V2 Codex adapter took over. It was kept alive by one integration test and by CodexAdapterV2 importing two MCP elicitation helpers from it.

What changed

  • Moved: describeMcpElicitation and toMcpElicitationResponse (plus their private schemas and helpers) now live in apps/server/src/provider/CodexMcpElicitation.ts, next to the other shared Codex modules (CodexDeveloperInstructions.ts, CodexToolPresentation.ts). The code is moved as-is. CodexAdapterV2 imports from the new module.
  • Moved tests: the 12 MCP elicitation tests go to CodexMcpElicitation.test.ts. The 8 developer/browser instruction tests, which only exercised buildCodexDeveloperInstructions, go to CodexDeveloperInstructions.test.ts. Both are unchanged.
  • Deleted:
    • CodexSessionRuntime.ts
    • The V1-only tests in CodexSessionRuntime.test.ts: thread history/pagination, buildTurnStartParams, hasConfiguredMcpServer, the memory-consolidation filter, codexSessionAppServerArgs, isRecoverableThreadResumeError, openCodexThread, plus one test that only restated an effect-codex-app-server error message.
    • CodexCollabRuntime.integration.test.ts, which booted the V1 runtime against a mock peer.
    • CodexCollabWire.test.ts, which tested the V1 child-notification router.
    • codexCollabMockPeer.cmd: only that integration test spawned it on Windows. The registry test that still uses the peer skips Windows.
    • codexSessionAppServerArgs, which only V1 used.
  • Kept: codexCollabMockPeer.sh/.mjs and codexMultiAgentWire.json. ProviderInstanceRegistryLive.test.ts still spawns the peer as a fake codex for the readiness probe. collabAgentToolCall handling in CodexAdapterV2 is not touched: Codex 0.156.1 still emits v1 collab items.
  • Small follow-ons: buildCodexInitializeParams is now private (knip flagged it as an unused export once V1 was gone). The manual-Effect-runner lint ceiling for the deleted test file is removed. The V2 TODO and the lineage doc now describe the missing paginated rollback path directly instead of pointing at V1.

Not ported (V2 behaves differently)

  • Memory-consolidation thread suppression. V1 hid notifications from Codex's internal memory_consolidation subagent threads. V2 has no equivalent filter.
  • Fallback to thread/start when resume fails recoverably. V1 started a fresh thread when thread/resume failed with a "thread not found / no rollout" style error. V2 resumeThread surfaces the error instead.
  • Currency-symbol skill aliases. V1 buildTurnStartParams rewrote skill mentions typed with any currency sigil (€review, £review) into Codex's canonical $review form (fix(skills): support unicode currency symbols as skill aliases #12098). This PR deletes its only test, "sends currency skill aliases in Codex's canonical dollar form". V2 already lacked the rewrite before this PR; fix(server): Codex V2 runs skills typed with any currency sigil #13461 fixes that separately.
  • Surfacing Codex stderr errors. V1 reported ERROR-level lines from Codex's stderr as session errors, skipping known-harmless ones. V2 reads stderr and discards it (packages/effect-codex-app-server/src/client.ts:272).

Verification

  • vp exec tsc --noEmit -p . in apps/server: no error TS.
  • vp lint on every touched TS file: 0 errors. The 2 existing no-unused-vars warnings in CodexAdapterV2.ts were already there before this change.
  • vp fmt on touched files.
  • knip --workspace apps/server --workspace packages/effect-codex-app-server --exports and knip --include files,dependencies: clean after making buildCodexInitializeParams private.
  • vp test run on CodexMcpElicitation.test.ts, CodexDeveloperInstructions.test.ts, codexLaunchArgs.test.ts, CodexProvider.test.ts, CodexAdapterV2.test.ts, ProviderInstanceRegistryLive.test.ts: 6 files, 152 tests passed.
  • git grep over apps, packages, scripts, docs, .github and root config for every deleted symbol and file name: no remaining references.
  • Not run: the full server suite, repo-wide typecheck/lint (CI owns those), or any live Codex session.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

makeCodexSessionRuntime had no production caller since the V2 Codex
adapter took over. Its only live exports were the MCP elicitation
helpers used by CodexAdapterV2, which now live in
provider/CodexMcpElicitation.ts with their tests. The developer
instruction tests that sat in the V1 test file move next to
CodexDeveloperInstructions.ts.

Deletes the runtime, its V1-only tests, the collab integration and wire
routing tests that exercised it, the Windows mock-peer wrapper only that
integration test spawned, and codexSessionAppServerArgs. The .sh/.mjs
mock peer and codexMultiAgentWire.json stay: the provider registry
readiness probe test still spawns them.

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:XXL 1,000+ changed lines (additions + deletions). labels Sep 24, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 5c8928d

Macroscope's review found this PR approvable — This PR removes an unused V1 Codex runtime and its dedicated test harness while relocating shared MCP elicitation helpers without changing their implementation or the active V2 path. The remaining changes are limited to tests, tooling, documentation, and private-symbol cleanup.

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

@github-actions

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.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 5c8928d · 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.

@juliusmarminge
juliusmarminge merged commit 982c612 into t3code/codex-turn-mapping Sep 24, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/codex-v1-removal branch September 24, 2026 19:31
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 25, 2026
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ 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