[3/4] Add executor-backed RMCP HTTP client#18583
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f411733c
ℹ️ 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".
1a1b28d to
afcf571
Compare
d2f4117 to
f11defd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f11defd27c
ℹ️ 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".
afcf571 to
81ff713
Compare
f11defd to
48f6619
Compare
81ff713 to
d6a38e3
Compare
48f6619 to
c02a707
Compare
d6a38e3 to
0a24aa2
Compare
c02a707 to
4bd6bdf
Compare
0a24aa2 to
4690cae
Compare
4bd6bdf to
7914b4b
Compare
4690cae to
b85c91f
Compare
7914b4b to
f4d2712
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4d2712d0b
ℹ️ 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".
b85c91f to
f14bc16
Compare
f4d2712 to
c84554f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c84554f5b8
ℹ️ 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".
| let metadata_client = build_http_client(&default_headers)?; | ||
| let mut oauth_state = OAuthState::new(url.to_string(), Some(metadata_client)).await?; |
There was a problem hiding this comment.
Fallback to token when environment OAuth discovery is unreachable
create_environment_oauth_transport_and_runtime performs OAuth metadata discovery with a local reqwest client (OAuthState::new(...)). For environment transport, the MCP URL can be reachable only from exec-server, so this call fails before any MCP request is proxied. create_pending_transport only falls back on NoAuthorizationSupport, so network-unreachable discovery errors abort startup even with valid stored tokens.
Useful? React with 👍 / 👎.
f14bc16 to
dbee178
Compare
c84554f to
cedd12a
Compare
dbee178 to
5baf804
Compare
cedd12a to
6c3949a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cedd12a13f
ℹ️ 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".
| /// Starts a local exec-server and connects an initialized `ExecServerClient`. | ||
| async fn spawn_exec_server() -> anyhow::Result<ExecServerProcess> { | ||
| let codex_home = TempDir::new()?; | ||
| let mut child = Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) |
There was a problem hiding this comment.
Resolve codex binary before spawning exec-server test
spawn_exec_server hard-fails on cargo_bin("codex")?. In a clean cargo test -p codex-rmcp-client run, that binary is not present (could not locate binary "codex"), so the new integration test fails before exercising RMCP behavior. This makes the test non-hermetic; skip when unavailable or use a harness that guarantees the binary path.
Useful? React with 👍 / 👎.
5baf804 to
941b42e
Compare
6c3949a to
abc0092
Compare
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b902e49baf
ℹ️ 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".
| backend: Arc<dyn StreamableHttpTransportBackend>, | ||
| } | ||
|
|
||
| #[derive(Clone)] |
There was a problem hiding this comment.
| map_streamable_http_error(error, StreamableHttpTransportClientError::Remote) | ||
| }) | ||
| } | ||
| .boxed() |
There was a problem hiding this comment.
| StreamableHttpClientTransport<AuthClient<StreamableHttpTransportClient>>, | ||
| OAuthPersistor, | ||
| )> { | ||
| let http_client = build_http_client(&default_headers)?; |
There was a problem hiding this comment.
Define or import build_http_client in OAuth setup
create_oauth_transport_and_runtime calls build_http_client, but this function is no longer defined/imported in rmcp_client.rs after the refactor. That leaves an unresolved symbol at compile time and prevents codex-rmcp-client from building.
Useful? React with 👍 / 👎.
Co-authored-by: Codex <noreply@openai.com>
…-http-client-full-ci
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d40346010
ℹ️ 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".
| /*http_headers*/ None, | ||
| /*env_http_headers*/ None, | ||
| OAuthCredentialsStoreMode::File, | ||
| exec_client, |
There was a problem hiding this comment.
Wrap exec client in Arc before calling remote constructor
create_remote_client passes an ExecServerClient directly to RmcpClient::new_remote_streamable_http_client, but that API requires Arc<dyn ExecutorHttpClient>. This yields E0308 during test builds, so the new remote integration tests do not compile (cargo check -p codex-rmcp-client --tests fails).
Useful? React with 👍 / 👎.
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b30bb4c614
ℹ️ 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".
| http_headers, | ||
| env_http_headers, | ||
| store_mode, | ||
| runtime_environment.environment().get_http_client(), |
There was a problem hiding this comment.
Honor experimental_environment for Streamable HTTP placement
make_rmcp_client now always injects runtime_environment.environment().get_http_client() into new_streamable_http_client, regardless of experimental_environment. If the runtime default environment is remote, servers configured as local (experimental_environment unset/local) are still routed through exec-server HTTP, which can break localhost/private-network MCP endpoints and changes documented placement behavior.
Useful? React with 👍 / 👎.
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>
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>
Co-authored-by: Codex <noreply@openai.com>
Why
The RMCP layer needs a Streamable HTTP client that can talk either directly over
reqwestor through the executor HTTP runner without duplicating MCP session logic higher in the stack. This PR adds that client-side transport boundary so remote Streamable HTTP MCP can reuse the same RMCP flow as the local path.What
rmcp-client/src/streamable_http/module with:transport_client.rsfor the local-or-remote transport enumlocal_client.rsfor the directreqwestimplementationremote_client.rsfor the executor-backed implementationcommon.rsfor the small shared Streamable HTTP helpersRmcpClientto build Streamable HTTP transports in either local or remote mode while keeping the existing OAuth ownership in RMCP.http/requestcalls.rmcp-client/tests/streamable_http_remote.rsand keep the shared test support inrmcp-client/tests/streamable_http_test_support.rs.Verification
cargo check -p codex-rmcp-clientStack