feat(codex): real-time mid-turn input via app-server turn/steer#94
Draft
juacker wants to merge 6 commits into
Draft
feat(codex): real-time mid-turn input via app-server turn/steer#94juacker wants to merge 6 commits into
juacker wants to merge 6 commits into
Conversation
First stage of the Codex app-server migration. Adds a self-contained transport module: process lifecycle for a long-lived `codex app-server` child, newline-delimited JSON-RPC framing (background stdout reader -> channel), and pure request/notification/error builders with unit tests. Not wired into the run path yet (that is the next commit); gated behind the CLAI_CODEX_APP_SERVER env flag so `codex exec` stays the default transport. app-server is the protocol every first-party Codex surface runs on and exposes turn/steer for real-time mid-turn input.
Second stage: wire the app-server transport into the Codex run path, behaviour-parity with codex exec (no steering yet). When CLAI_CODEX_APP_SERVER is set, dispatch routes to run_codex_turn_app_server, which does the initialize -> thread/start|resume -> turn/start handshake, then maps the notification stream (item/started, item/completed, thread/tokenUsage/updated, error, turn/completed) onto the existing CodexStreamState + handle_codex_item helpers by normalizing app-server items into the exec item shape. approvalPolicy=never + danger-full-access mirror the exec bypass; structured error codes are folded into the CLI error classifiers so context/usage recovery still fires. codex exec remains the default. turn_steer_request carries a temporary allow(dead_code) until the next commit wires steering.
Third stage: real-time mid-turn input via turn/steer — the payoff over the exec stop-and-restart model. A 400ms poll in the app-server driver reads pending queued messages for this connection while a turn is active and injects their text into the running turn with turn/steer, guarded by the active expectedTurnId. Delivery is confirmed on the steer response: accepted -> mark delivered + emit QueuedMessagesDelivered; rejected (turn ended / id mismatch) -> leave queued so the existing followup-run path delivers them. Image-bearing messages are deferred to the followup run (which resolves image paths). In-flight tracking prevents double-sending before the response lands.
Fourth stage: unit tests for the pure mapping layer (normalize_app_server_item discriminant mapping, reasoning content/summary flattening, token-usage breakdown -> RunUsage) and an 'Enabling / testing' note in the module rustdoc (repo convention documents env flags in code, not markdown). Default stays codex exec; flipping the default is deferred until the app-server path is validated live.
Addresses the validated findings from the external review of this PR: - aps_request_await now bounds each handshake request (initialize, thread/start|resume) with a 30s deadline, so a wedged `codex app-server` fails fast with a clear message instead of hanging the run until the user cancels. - Stdout closing without `turn/completed` is now surfaced as a run failure instead of finalizing a silently truncated turn as success (found while verifying the review; partial parts are still persisted). - Regression test: agentMessage without a `text` field normalizes to an explicit null in the exec shape. - Stale docstring fixed (steering is in this PR, not a follow-up) and the per-turn/best-effort lifetime of the steer bookkeeping documented. Rejected from the review (verified against the code): the two "major" findings assume pending_steer outlives the turn; it is a local of run_codex_turn_app_server and turn/completed breaks the loop, so it lives exactly one turn and ids cannot collide across turns.
Two issues found running the app-server branch (workspace test session): 1. SECURITY (regression vs exec): the app-server thread/start+resume set approvalPolicy=never + sandbox=danger-full-access but left Codex's built-in shell_tool ENABLED, so Codex ran shell commands through its own native path (observed: pwd/date/ls/python), bypassing CLAI's permission checks and bwrap sandbox entirely. The exec path passes `--disable shell_tool` for exactly this reason. Fix: the thread config now sets `features.shell_tool=false` (the documented equivalent of `--disable shell_tool`) on both start and resume, so Codex has no unmediated execution path — every command routes through the gated MCP `bash_exec`. thread/resume now also re-applies the full config (each turn spawns a fresh app-server process, so it must be re-set). 2. ORDERING: the whole turn accreted into one assistant message whose created_at is frozen at turn start, so a mid-run (steered) user message — created when the user hit send — sorted AFTER the entire reply, showing the handling of a comment before the comment itself. Fix: on steer accept, close the current assistant message and open a fresh one (split_codex_assistant_message), yielding the natural order pre-steer output -> user message -> post-steer output. An empty leading placeholder is dropped instead of left as an empty bubble. Tests: thread_start/thread_resume now assert features.shell_tool=false; full lib suite 742/742, clippy -D warnings clean.
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
1 participant
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.
What
Add real-time mid-turn input for Codex, the way you asked for: instead of the Claude-style stop-and-restart, Codex now runs over the
codex app-serverJSON-RPC transport and injects queued messages into the live turn viaturn/steer— no interrupt, no restart.Gated behind
CLAI_CODEX_APP_SERVER(default off);codex execstays the default transport until this is validated live.Why app-server
codex execconsumes stdin once and closes it, so mid-turn input is impossible without killing the process.codex app-serveris the JSON-RPC protocol every first-party Codex surface (VS Code, desktop, web) runs on, and it exposesturn/steer— input into the currently active turn, guarded by anexpectedTurnIdprecondition. That's the clean primitive for this feature.Commits (staged for commit-by-commit review)
feat(codex): app-server JSON-RPC transport client— self-containedcodex_app_server.rs: process lifecycle, newline-delimited JSON-RPC framing (background stdout reader → channel), pure request/notification/error builders + unit tests. Not wired in yet.feat(codex): run turns through app-server when enabled— dispatch flag +run_codex_turn_app_server:initialize → thread/start|resume → turn/starthandshake, then maps the notification stream onto the existingCodexStreamState+handle_codex_itemby normalizing app-server items into the exec item shape (so tool-call persistence, streaming, usage all reuse the proven path).approvalPolicy=never+sandbox=danger-full-accessmirror the exec--dangerously-bypass-approvals-and-sandbox; structured error codes (contextWindowExceeded/usageLimitExceeded) are folded into the CLI error classifiers so context/usage recovery still fires. Behaviour-parity, no steering yet.feat(codex): steer queued messages into the live turn— the payoff. A 400ms poll injects pending queued messages' text viaturn/steerwhile a turn is active. Delivery is confirmed on the steer response: accepted → mark delivered +QueuedMessagesDelivered; rejected (turn ended / id mismatch) → leave queued so the existing followup-run path delivers them (no message loss). Image-bearing messages are deferred to the followup run (which resolves image paths); steering carries text only. In-flight tracking prevents double-send before the response lands.test(codex): …normalization + document flag— pure-function unit tests for the mapping layer + module rustdoc "Enabling / testing" note.Verification
cargo fmt --check,cargo clippy --lib -D warnings,cargo test --lib→ 740 pass (incl. 7 transport builder/classification tests + 3 normalization/usage tests).initialize,thread/start,turn/started(turn id),item/completed,thread/tokenUsage/updated,error(structuredcodexErrorInfo),turn/completed.Honest caveats (please test before merge)
usageLimitExceeded), so a successful multi-turn/tool-call/steer flow was not exercised end-to-end here — only the envelope + failure paths. Please run with a funded Codex login: (1) a normal turn (tool calls, agent message), (2) send a message mid-turn and confirm it's absorbed without restart (log: "Steered queued user message(s) into the live Codex turn"), (3) resume across turns.-c mcp_servers.clai.*flags but couldn't be fully smoke-tested from the sandbox; the clai tools working under app-server is the key thing to confirm.CLAI_CODEX_APP_SERVER=1. Flipping the default is a deliberate follow-up once you've run it.Backend only; no bindings/frontend changes.