Repository navigation
feat: add ACP v2 support again - #620
Merged
Merged
Conversation
2 tasks done
EugeneTheDev
added a commit
that referenced
this pull request
Oct 13, 2026
## Summary The E2E run on #620 ([run](https://github.com/agentclientprotocol/codex-acp/actions/runs/38395294465)) failed on two model-dependent tests. #620 didn't cause it: the run before it shows the same behavior and passed only by luck. This PR changes test code only. - **Default e2e model `gpt-5.2` → `gpt-6-luna[low]`.** `gpt-5.2` isn't in the Codex model list anymore, so Codex used fallback metadata. In 0.160.1 that has `apply_patch_tool_type: None`, so the model ran `apply_patch` through the shell. A malformed patch came in as an `execute` approval, not `edit`, and broke `applies an approved workspace file edit`. - **Shell approval prompts ask for elevated permissions on the first try.** In read-only mode the model sometimes ran the command in the sandbox, hit `Read-only file system`, and ended the turn without asking for approval (`cancels every command when cancel is selected` got `end_turn`). This applies only to the tests that expect an approval prompt. - **Cancellation test follows how Codex handles interrupts.** `gpt-6-luna` runs commands through unified exec, and `turn/interrupt` doesn't kill those processes: they keep running as background terminals. The test now: - waits until the client has seen the command's tool call; - cancels and asserts the turn is `cancelled` and the process is still running; - stops the process with `_session/async_task/stop` and asserts it exits. The ACP SDK client rejects the `async_task_spawned` update because its schema doesn't include it, so the test uses the tool call id as the task id. For a root command they're the same. - Fixture helpers: the fixture records tool calls, and `createAuthenticatedFixture` can declare AIR capabilities. ## Test plan - [x] `npx tsc --noEmit` - [x] E2E workflow on this branch: [run 38406199843](https://github.com/agentclientprotocol/codex-acp/actions/runs/38406199843). 33 passed, 2 skipped. One retry, in the exec-policy amendment test: the model mistyped the long temp path, so the saved approval rule didn't match.
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
initializeand keeps it:acpV2.agentProtocolRouter().withV1(v1Agent).withV2(v2Agent).connect(stream)(src/index.ts).session updates fork by version in one place:
toV2SessionUpdate(src/AcpV2SessionUpdate.ts), called fromACPSessionConnection.Implemented (v2)
The v2 method set is implemented as specified:
initialize,session/new|list|close|delete|resume|fork,session/prompt,session/cancel,$/cancel_request,session/set_config_option,auth/login|logout,session/request_permission,elicitation/create|complete,state_update, plus the extensions below. Only the partsthat are project-specific or not obvious from the spec are described here.
Prompts and turns
session/promptis answered with{messageId}once the user message is actually inserted into a Codex turn, notwhen the request arrives.
ends. They are not steered into the running turn; steering stays a separate
_session/steeringrequest.session/cancelinterrupts the running turn and rejects every queued, not-yet-inserted prompt with -32800. Thoseprompts get no
state_update.session/neworsession/resume). These getrunning/idlelike any other turn, with no matchingsession/prompt. Clients should not assume everyrunning/idlepair belongs to one of their prompts.requires_action/runningbracket a permission or elicitation request only while a turn is running. Requests sentwhile the session is idle (e.g. MCP OAuth re-auth when a session opens) go out without the bracket.
user_message_chunk(withmessageId) per prompt block, the same shape resumereplay produces. An empty prompt sends no user message update.
/compactand/goalprompts render live only; Codex persists no user message for them, so replay doesn't showthem.
Tool calls and terminals
terminal_output_chunks without Codex's shell-wrapper framing. Completion doesn't resendthe full output: the streamed chunks are the only copy.
changes[]; adds and deletes get a synthesized git patch.Resume replay
session/resumewithreplayFrom: {type: "start"}replays the full history (messages, tool calls, plans, subagentand async-task updates) with stable ids. Each replayed message starts with a
content: []primer before its firstchunk.
recovers. Its message chunks are used to position those tool calls and are never sent.
Extensions
providers/list|set|disablereuse the v1 logic.providers/set|disablerestarts the Codex app-server and resumesevery session the client has loaded; a turn that was running at restart ends with
idle/cancelled._session/goal(set/pause/resume/clear) and_session/async_task/stop.subagent_spawned/subagent_state_updatebecome one_subagent_update(sent when AIR declaresnativeSubagentSessions);async_task_spawned/async_task_state_updatebecome
_async_task_spawned/_async_task_state_update(sent when AIR declaresasyncTasks). Payloads areunchanged.
Compatibility details
rejected.
session/listandauth/logoutaccept a request withparamsomitted. The ACP SDK (1.5.0) rejects that eventhough every field is optional, so a stream transform fills in
params: {}for these two methods only.The full client contract, including what AIR relies on, is in
readme-dev.md("ACP v2 support", "AIR v2 clientcontract").
v1 compatibility
v1 wire behavior is unchanged apart from the fixes below. Verified by:
behavior.
CONFORMANT, 50 pass / 1 fail / 5 skip, identical to the baseline before this work. The singlefailure is the pre-existing advisory
ACP-SCHEMA-002.v1Agentinsrc/index.ts) is built exactly as before, and the SDK'sprotocol router binds each connection to one chain for its lifetime.
Fixes
Pre-existing bugs, fixed on both v1 and v2 unless stated otherwise.
A goal turn that Codex starts right after a session is loaded loses its opening content
Codex auto-continues an active goal a few milliseconds after
thread/resume. The turn tracker was registered onlyafter
model/listandaccount/read, and the app-server client drops notifications that have no handler yet. Thetracker is now registered before
thread/resume/thread/loadand held until the session is ready. Paths that didn'thit the race send exactly the same frames as before.
Turns Codex starts on its own are invisible
A turn no
session/promptowns (e.g. a goal auto-continuation) was tracked but never rendered. It now renders throughthe same session-scoped path a prompt uses. A prompt-owned turn can't be rendered twice: a session has only one current
dispatcher.
Closing or cancelling a session can leave Codex running a turn
When
turn/interruptraced the turn's registration, the retry only covered cancel, not close, and it computed the turnid once instead of on each attempt. Close now gets the same retry, and the id is recomputed on every attempt.
/reviewcompletion goes to the wrong turn, and a review that fails to start hangs the promptreview/startresponds with the parent turn id, whileturn/startedcarries the reviewer's child turn id, andturn/interruptneeds the child id. The two ids are now tracked separately, so completion and errors are attributed tothe review's own turn, and a review that fails before starting ends the prompt. A successful review reports the same as
before.
Cancel leaves a pending permission or elicitation request open, and a cancelled review reports
end_turnsession/cancelonly aborted work that hadn't started a turn yet. Each prompt now has an interaction signal that canceland close abort. That withdraws any permission or elicitation request still waiting on the client (
$/cancel_request),so a late answer can't be applied. A cancelled plan-implementation review now reports
stopReason: cancelled.Replay resurrects tool calls from a rolled-back history
The response-item history fallback for old rollouts ignored
thread_rolled_back. It now bails out when the rolloutshows a rollback. Only that legacy replay path is affected.
codex-acp starts its own goal continuations on top of Codex's
codex-acp had a fallback that continued an active goal after a turn ended. Codex 0.156.1 does this itself, so the
fallback could start an unchecked turn (even for a goal whose budget was exhausted) or steer into a turn Codex had just
started. The fallback is removed; goal continuation is left to Codex.
An unknown permission outcome is treated as a selection
Permission responses were read as "selected" whenever the outcome wasn't
cancelled, but the outcome is an open union(
ACP-ENUM-203). The readers now checkoutcome === "selected". Every valid outcome behaves as before.Provider restart re-resumes forks the client hasn't loaded yet
After a fork, the session waits for the client's
session/load, but a provider restart resumed it anyway. Restart nowskips such sessions; their
session/loadresumes them as usual.Provider restart leaves the cut-off turn open forever
providers/set|disablekills the old app-server while a turn may still be running, so that turn'sturn/completedanditem/completednever arrive. Before, the resumed session also lost its turn tracker, so turns after the restartweren't rendered. Now, per session, the restart:
failedand closes its terminal;idle/cancelled;The order matters. Codex can start a goal turn about 3 ms after
thread/resume, and closing out after the resume wouldcancel that new turn instead of the old one. The list of open tool calls is cleared when a turn completes, so a later
restart can't fail a tool call from an earlier turn.
On v1 the only new frames are the
failedtool call updates and the terminal exit.session/newhas no fallback for a missingmcpServersThe other session-creation paths default it to
[];session/newnow does too.Known issues (not fixed)
A session that was never prompted is lost on provider restart
Codex writes a thread's rollout only after its first turn. If
providers/set|disableruns before a session's firstprompt,
thread/resumefails with "no rollout found", and so does thethread/readfallback. Theproviders/*requestfails with -32603 "Failed to resume 1 session(s)", and the next
session/promptfails with "thread not found". Thisalready happens on
main. Deferred: it needs a separate design for restarting threads that were never persisted. AFIXMEmarks the resume loop, andreadme-dev.md"Known gaps" describes it.A disabled
openaiprovider is still reported as currentAfter
providers/disable("openai"),providers/liststill reports native OpenAI routing ascurrent; the providersRFD says it should be
null. Left as-is deliberately and documented inreadme-dev.md"Known gaps".Fork points can't target a user message sent with a client id
AIR's fork points match Codex item ids, but on v2 a user message's id is the client's
clientIdwhen the clientsupplied one. A fork at such a message wouldn't resolve. This is an open question for AIR, documented in
readme-dev.md"Known gaps".stopReasonis only everend_turnorcancelledcodex-acp never reports
refusal,max_tokensormax_turn_requests. This predates the PR; v2 only needed themapping moved, not made more precise.
The title generator keeps using the old client after a provider restart
Title generation after a restart goes to the app-server that was killed. Out of scope for this PR.
Races with a pending provider update
A session opened while
providers/setis in flight may run on the client that is about to be killed._session/goaland_session/steeringlikely don't wait for a pending provider update either (not verified). Out ofscope.
Idle-time permission and elicitation requests can't be cancelled
session/cancelwithdraws requests through the current prompt's interaction signal. A request sent while no prompt isrunning reuses the finished prompt's signal, so cancel doesn't reach it. Low impact: these are rare (e.g. MCP OAuth
re-auth).
Extra
runningafter a late permission answer on v2If a v2 client cancels during
requires_actionand then answers the permission request anyway, it gets a redundantrunningbeforeidle. The final state is correct.Closing a session during a turn Codex started gets no interrupt retry
The registration-race retry covers prompt-owned turns only. Closing during a self-started turn that hasn't registered
yet can leave it running.
Testing
New test areas:
initialize-v2,session-lifecycle-v2(including fork and resume/replay),session-update-v2,cancel-v2,elicitation-v2,providers-v2,session-goal-v2,async-task-stop-v2,steering-v2,prompt-v2,agent-initiated-turns-v1/v2,resume-goal-turn-timing,provider-restart-turn-tracking,v2-resume-replay(including subagents),
subagent-and-async-task-updates-v2,session-config-options-v2.TCK, full run for both protocol versions against the same build:
ACP-SCHEMA-002, pre-existing advisory)Live checks against real Codex 0.156.1 (isolated
CODEX_HOME, raw JSON-RPC driver): a goal turn right aftersession/new, cancel during a real command approval, resume with an active goal in a fresh process, and a providerrestart with an active goal. The restart check found the last provider-restart fix above and the unfixed
never-prompted-session issue.
Reviewer notes
Suggested reading order:
src/index.ts: where the v1/v2 protocol router is wired up.src/AcpAgentRouter.ts: the v2 agent chain (method registrations).src/ACPSessionConnection.ts+src/AcpV2SessionUpdate.ts: thetoV2SessionUpdatefork point. This pair explainshow the rest of the code avoids version-specific logic.
src/CodexAcpServer.ts: shared business logic (turn tracking, session lifecycle, provider restart). The largest andmost-changed file.
src/CodexSessionToolCalls.ts: small new tracker of open tool calls, used by provider restart.Worth extra attention:
enqueueProviderUpdateinCodexAcpServer.ts). The per-session order (drain, closeout, then register the tracker and resume) is load-bearing and easy to break.
acquireTurnStartReservation,promptAfterReservation,promptV2).This is the one new concurrency mechanism; the rest mostly reuses existing v1 logic.