Skip to content

Add exec-server filesystem RPC implementation#15091

Open
starr-openai wants to merge 9 commits intostarr/exec-server-exec-freshfrom
starr/exec-server-filesystem-fresh
Open

Add exec-server filesystem RPC implementation#15091
starr-openai wants to merge 9 commits intostarr/exec-server-exec-freshfrom
starr/exec-server-filesystem-fresh

Conversation

@starr-openai
Copy link
Contributor

Stacked PR 3/3, based on the exec PR.

Adds filesystem RPCs and the client/backend cleanup on top of the exec implementation.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2026

All contributors have signed the CLA ✍️ ✅
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: bfdd8bcc65

ℹ️ 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 +42 to +46
tokio::spawn(async move {
if let Err(err) = handler.handle_message(message).await {
warn!("exec-server request failed after protocol error: {err}");
}
});
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 Handle inbound RPCs in-order per connection

run_connection now spawns every inbound message on a detached task. That allows initialized and the following process/start to run out of order, so exec can observe initialized=false and reject a valid handshake sequence with client must send initialized.... This makes correctly ordered client traffic nondeterministically fail.

Useful? React with 👍 / 👎.

handler.shutdown().await;
drop(handler);
drop(outgoing_tx);
let _ = outbound_task.await;
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 Track spawned handler tasks before shutdown

Per-message tasks are detached and never joined/aborted. During disconnect, run_connection waits for outbound_task, but in-flight handler tasks still hold Arc<ExecServerHandler>/outbound_tx. A long process/read can keep teardown hanging, and an in-flight exec can start a process after shutdown() has already run, leaving it unmanaged.

Useful? React with 👍 / 👎.

@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 starr-openai force-pushed the starr/exec-server-filesystem-fresh branch 2 times, most recently from 67fa0dc to 6742da0 Compare March 18, 2026 21:54
@starr-openai starr-openai force-pushed the starr/exec-server-exec-fresh branch from c5071a9 to 84a6cbe Compare March 18, 2026 21:54
@starr-openai starr-openai force-pushed the starr/exec-server-filesystem-fresh branch from b892b77 to 6245099 Compare March 19, 2026 04:39
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