Skip to content

[4/6] Abstract MCP stdio server launching#17986

Open
aibrahim-oai wants to merge 3 commits intoopenai:dev/remote-mcp-exec-eventsfrom
aibrahim-oai:dev/remote-mcp-stdio-runtime
Open

[4/6] Abstract MCP stdio server launching#17986
aibrahim-oai wants to merge 3 commits intoopenai:dev/remote-mcp-exec-eventsfrom
aibrahim-oai:dev/remote-mcp-stdio-runtime

Conversation

@aibrahim-oai
Copy link
Copy Markdown
Collaborator

  • Move local MCP stdio process startup behind a runtime trait.
  • Preserve existing local stdio behavior while making transport creation explicit.

@aibrahim-oai aibrahim-oai changed the title Abstract rmcp stdio transport runtime [3/5] Abstract rmcp stdio transport runtime Apr 15, 2026
@aibrahim-oai aibrahim-oai changed the title [3/5] Abstract rmcp stdio transport runtime [3/5] Abstract MCP stdio server launching Apr 15, 2026
aibrahim-oai and others added 3 commits April 15, 2026 15:45
Move local stdio process startup behind a runtime trait while preserving the existing local MCP stdio behavior.

Co-authored-by: Codex <noreply@openai.com>
Rename the stdio runtime abstraction around launching a server so the PR boundary is about process placement, not a parallel stdio/executor runtime shape.\n\nCo-authored-by: Codex <noreply@openai.com>
Keep the shared launcher API before the local implementation and move local launch helpers onto LocalStdioServerLauncher.\n\nCo-authored-by: Codex <noreply@openai.com>
@aibrahim-oai aibrahim-oai force-pushed the dev/remote-mcp-stdio-runtime branch from 595325e to 3842a64 Compare April 15, 2026 22:47
@aibrahim-oai aibrahim-oai changed the title [3/5] Abstract MCP stdio server launching [4/6] Abstract MCP stdio server launching Apr 15, 2026
@aibrahim-oai aibrahim-oai changed the base branch from dev/remote-mcp-exec-stdin to dev/remote-mcp-exec-events April 15, 2026 22:47
@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

let (event_tx, _event_rx) = broadcast::channel(PROCESS_EVENT_CHANNEL_CAPACITY);

P1 Badge Buffer local process events before subscribers attach

The event channel is created with _event_rx immediately dropped, so there are initially zero receivers. stream_output/watch_exit ignore broadcast::Sender::send errors, which means early Output/Exited/Closed events are discarded for short-lived commands. A caller that subscribes right after start() can miss terminal events and wait forever.


let (event_tx, _event_rx) = broadcast::channel(PROCESS_EVENT_CHANNEL_CAPACITY);

P1 Badge Preserve remote events emitted before first subscription

SessionState::new also drops the initial broadcast receiver, leaving no listeners until user code calls subscribe_events(). Notifications published in that window are silently lost because publish_event ignores send failures. Fast remote processes can emit Closed/Exited before subscription, causing event-driven consumers to miss termination and hang.

ℹ️ 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".

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.

1 participant