Skip to content

Add exec-server exec RPC implementation#15090

Open
starr-openai wants to merge 11 commits intomainfrom
starr/exec-server-exec-fresh
Open

Add exec-server exec RPC implementation#15090
starr-openai wants to merge 11 commits intomainfrom
starr/exec-server-exec-fresh

Conversation

@starr-openai
Copy link
Contributor

Stacked PR 2/3, based on the stub PR.

Adds the exec RPC implementation and process/event flow in exec-server only.

starr-openai and others added 5 commits March 18, 2026 10:29
This adds the standalone exec-server stdio JSON-RPC crate and its
smoke tests without wiring it into the CLI or unified-exec yet.

Co-authored-by: Codex <noreply@openai.com>
Document the standalone exec-server crate, its stdio JSON-RPC
transport, and the current request/response and notification
payloads.

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Keep only the standalone handshake transport in the first PR. Leave exec RPC behavior unimplemented here so the working process implementation can land in a separate follow-up.\n\nCo-authored-by: Codex <noreply@openai.com>
Move process-oriented client helpers and exports into the exec follow-up. Keep the first PR focused on spawning the stub server and running the initialize handshake.\n\nCo-authored-by: Codex <noreply@openai.com>
@github-actions
Copy link
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79b46e346d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +452 to +454
if let Some(process) = processes.get_mut(&process_id) {
process.exit_code = Some(exit_code);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove exited processes from the process map

watch_exit only sets exit_code and never evicts the RunningProcess entry. Each finished process keeps retained output (up to RETAINED_OUTPUT_BYTES_PER_PROCESS) and session state until connection shutdown, so long-lived connections that run many short-lived processes can accumulate unbounded memory/state.

Useful? React with 👍 / 👎.

Comment on lines +296 to +299
if let Some(process) = process_map.get(&params.process_id) {
process.session.terminate();
true
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Return running=false for already exited processes

terminate returns running: true whenever process_id exists in the map, without checking whether the process has already exited. Since exited entries are retained, terminate can claim a process is running when it is not, which breaks caller logic that relies on this flag.

Useful? React with 👍 / 👎.

Comment on lines +95 to +101
loop {
let (stream, peer_addr) = listener.accept().await?;
tokio::spawn(async move {
match accept_async(stream).await {
Ok(websocket) => {
run_connection(JsonRpcConnection::from_websocket(
websocket,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require authentication on websocket exec transport

The websocket listener accepts any incoming peer and immediately starts a JSON-RPC exec session. Combined with unauthenticated initialize flow, binding to non-loopback addresses allows arbitrary remote clients to call process/start and execute commands (unauthenticated RCE).

Useful? React with 👍 / 👎.

Introduce API-agnostic server envelope parsing/encoding and a tiny method-registration router in the initialize-only exec-server slice.

Co-authored-by: Codex <noreply@openai.com>
@starr-openai starr-openai force-pushed the starr/exec-server-exec-fresh branch from 79b46e3 to c5071a9 Compare March 18, 2026 21:04
starr-openai and others added 5 commits March 18, 2026 14:30
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
@starr-openai starr-openai force-pushed the starr/exec-server-exec-fresh branch from c5071a9 to 84a6cbe Compare March 18, 2026 21:54
stream,
chunk: chunk.clone(),
});
while process.retained_bytes > RETAINED_OUTPUT_BYTES_PER_PROCESS {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unified exec needs the tail. we might need the same "drop middle strategy" as used in core.

Base automatically changed from starr/exec-server-stub-fresh to main March 19, 2026 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants