fix(rmcp-client): preserve OAuth metadata queries (#5953)#383
fix(rmcp-client): preserve OAuth metadata queries (#5953)#383
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| for (key, value) in preserved_vars { | ||
| env_map.entry(key).or_insert(value); | ||
| } |
There was a problem hiding this comment.
Respect inherit=none when restoring PATH/NVM
The new logic unconditionally reinserts any discovered PATH and NVM_DIR entries after filtering. When ShellEnvironmentPolicyInherit::None is selected, the caller expects a completely clean environment except for explicit overrides (see test_inherit_none). The loop re‑adding preserved vars ignores that contract and now leaks the parent process’s PATH/NVM_DIR into supposedly isolated executions. This both regresses existing behaviour and breaks environments that relied on inherit: None for sandboxing. Consider skipping the preservation step when the policy forbids inheritance or when the variables were deliberately excluded.
Useful? React with 👍 / 👎.
Summary
/authorizeURLsrmcpclient to retain the original query string when probing/.well-known/oauth-authorization-serverand add a regression test that fails without itccdependency so metadata discovery and downstream builds succeedTesting
Closes openai#5953.