code-mode: add process-owned session runtime - #29805
Conversation
| self.state.pending.lock().await.remove(&id); | ||
| return Err(err); | ||
| } | ||
| response_rx |
There was a problem hiding this comment.
This isn’t cancellation-safe: once the frame is sent, dropping the caller only drops response_rx
| let Some(message) = message else { | ||
| break; | ||
| }; | ||
| if let Err(err) = writer.write(&message).await { |
There was a problem hiding this comment.
This makes an oversized client→host message connection-fatal. A nested-tool result can exceed the frame cap, so one large MCP/image/exec result tears down every session sharing this host
Is this on purpose? (not sure what's the latest decision here)
| for (_, sender) in self.initial_responses.lock().await.drain() { | ||
| let _ = sender.send(Err(reason.clone())); | ||
| } | ||
| self.delegates.lock().await.clear(); |
There was a problem hiding this comment.
Clearing the delegates skips cell_closed for every live remote cell
| let response = self | ||
| .request(HostRequest::OpenSession { | ||
| session_id: session_id.clone(), | ||
| }) |
There was a problem hiding this comment.
There’s an open-session leak here if create_session() is cancelled: neither cleanup arm runs, so the host can retain the session and this map retains the delegate after the only local handle is gone
| .state | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner); | ||
| match std::mem::replace(&mut *state, SessionState::Shutdown) { |
There was a problem hiding this comment.
Shutdown is published before the host acknowledges cleanup... this is race prone
| .write(&ClientToHost::ClientHello(hello)) | ||
| .await | ||
| .map_err(|err| format!("failed to write code-mode host hello: {err}"))?; | ||
| match reader |
There was a problem hiding this comment.
Can we put the startup hello exchange behind a finite deadline and start draining stderr before awaiting it?
| }); | ||
| let writer_task = tokio::spawn(async move { | ||
| while let Some(message) = outgoing_rx.recv().await { | ||
| writer |
There was a problem hiding this comment.
(found by Codex)
The reverse direction has the same blast radius: JS can build an arbitrarily large tool argument or notify() string, which becomes one DelegateRequest. If it exceeds MAX_FRAME_BYTES, this stops the writer and tears down the shared host
0f7bda8 to
537cda9
Compare
e0fac5a to
125a59b
Compare
537cda9 to
42691b9
Compare
125a59b to
c52c04b
Compare
42691b9 to
1886ad6
Compare
## Why The process-owned code mode implementation needs an explicit, bounded wire contract before either side depends on it. Keeping framing and message semantics in `codex-code-mode-protocol` gives the client and sidecar one shared source of truth and makes compatibility failures detectable during connection setup. ## What changed - adds a versioned client/host handshake with required and optional capabilities - defines operation requests and responses for session lifecycle and cell control - defines reverse delegate request, response, cancellation, and cell-closure messages - adds a four-byte little-endian length-prefixed JSON codec with a hard frame cap - rejects malformed frames, unknown fields, invalid identifiers, and unsupported protocol states - locks the wire representation down with explicit JSON round-trip tests ## Testing - `just test -p codex-code-mode-protocol` ## Stack Part 1 of 6. Followed by [#29805](#29805).
## Why The process-owned code mode implementation needs an explicit, bounded wire contract before either side depends on it. Keeping framing and message semantics in `codex-code-mode-protocol` gives the client and sidecar one shared source of truth and makes compatibility failures detectable during connection setup. ## What changed - adds a versioned client/host handshake with required and optional capabilities - defines operation requests and responses for session lifecycle and cell control - defines reverse delegate request, response, cancellation, and cell-closure messages - adds a four-byte little-endian length-prefixed JSON codec with a hard frame cap - rejects malformed frames, unknown fields, invalid identifiers, and unsupported protocol states - locks the wire representation down with explicit JSON round-trip tests ## Testing - `just test -p codex-code-mode-protocol` ## Stack Part 1 of 6. Followed by [openai#29805](openai#29805).
Why
Code mode needs a process-owned implementation that preserves the existing
CodeModeSessioncontract while moving JavaScript execution and stored session values out of the client process. The host and client are introduced together here so the transport is reviewable as one end-to-end path rather than as disconnected process halves.What changed
codex-code-mode-hostsidecar binaryInProcessCodeModeSessionThis does not activate the process-owned provider in the production tool path. Packaging and binary discovery remain follow-up work.
Testing
just test -p codex-code-modejust test -p codex-code-mode-hostStack
Part 2 of 6. Depends on #29804; followed by #29806.