Preserve legacy Codex MCP opt-out#1043
Conversation
|
CodeAnt AI is reviewing your PR. |
📝 WalkthroughWalkthroughThe PR extends MCP configuration detection in the Codex auto-config path to recognize both the modern ChangesAgent Relay MCP Config Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
codeant-ai
Bot
added
the
size:S
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| && !existing_args | ||
| .iter() | ||
| .any(|a| a.contains("mcp_servers.agent-relay")) | ||
| .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast")) |
There was a problem hiding this comment.
Suggestion: The new legacy opt-out check uses substring matching (contains("mcp_servers.relaycast")), which can falsely match unrelated config keys like mcp_servers.relaycast_backup... and incorrectly skip MCP injection. This can leave Codex without Agent Relay MCP configured. Match the config key boundary instead (for example exact server key/prefix such as mcp_servers.relaycast. or mcp_servers.relaycast=) to avoid accidental opt-out. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Codex workers may start without Agent Relay MCP configured.
- ⚠️ Relay messaging/channel tools unavailable in affected Codex sessions.Steps of Reproduction ✅
1. Spawn a Codex worker through the broker so `WorkerRegistry::build_mcp_args` is called
(see `crates/broker/src/worker.rs:175-205`), passing `cli_name = "codex"` and some
`existing_args` taken from the Codex CLI invocation.
2. Start Codex with a custom MCP server whose config key contains the legacy prefix but is
not actually the `relaycast` server, e.g. pass `--config
mcp_servers.relaycast_backup.command=custom` so `existing_args` includes the string
`"mcp_servers.relaycast_backup.command=custom"`.
3. `build_mcp_args` forwards `existing_args` into `configure_agent_relay_mcp_with_result`
in `crates/broker/src/snippets.rs:750-761`, which reaches the Codex auto-config block at
lines 815-820; the condition `!existing_args.iter().any(|a|
a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast"))` (line 818)
evaluates to false because `"mcp_servers.relaycast_backup.command=custom"` contains the
substring `"mcp_servers.relaycast"`, even though there is no `mcp_servers.agent-relay` or
`mcp_servers.relaycast` server configured.
4. Because that condition fails, the block at `snippets.rs:35-124` that appends
`mcp_servers.agent-relay.command=...`, `mcp_servers.agent-relay.args=...`, and
`mcp_servers.agent-relay.env.*` is skipped, so `build_mcp_args` returns without Agent
Relay MCP configuration and the Codex worker starts without the expected Agent
Relay/relaycast tools.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/broker/src/snippets.rs
**Line:** 818:818
**Comment:**
*Incorrect Condition Logic: The new legacy opt-out check uses substring matching (`contains("mcp_servers.relaycast")`), which can falsely match unrelated config keys like `mcp_servers.relaycast_backup...` and incorrectly skip MCP injection. This can leave Codex without Agent Relay MCP configured. Match the config key boundary instead (for example exact server key/prefix such as `mcp_servers.relaycast.` or `mcp_servers.relaycast=`) to avoid accidental opt-out.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bd95fc2cb
ℹ️ 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".
| && !existing_args | ||
| .iter() | ||
| .any(|a| a.contains("mcp_servers.agent-relay")) | ||
| .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast")) |
There was a problem hiding this comment.
When a Codex worker is spawned with legacy inline args such as --config mcp_servers.relaycast.command=..., this condition now suppresses all canonical mcp_servers.agent-relay args instead of migrating or adding the new server. The rest of the broker still instructs agents to use the canonical tool namespace (mcp__agent-relay__... / agent-relay.<tool> in crates/broker/src/broker/injection_format.rs), so these legacy-configured Codex workers are left with only a relaycast-named MCP server while being prompted to call tools under agent-relay. Please either rewrite/alias legacy args to agent-relay or continue injecting the canonical server while avoiding duplicate legacy setup.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/broker/src/snippets.rs (1)
1858-1877: 💤 Low valueRegression test correctly validates legacy opt-out behavior.
The test structure and assertions are correct. The legacy
mcp_servers.relaycastconfig is properly detected and triggers the opt-out path, returning only the update suppression flag.📝 Optional: Add assertion message for consistency
For consistency with the similar test on lines 1852-1855, consider adding an explanatory message to the assertion:
- assert_eq!(args, vec!["--config", "check_for_update_on_startup=false"]); + assert_eq!( + args, + vec!["--config", "check_for_update_on_startup=false"], + "should only return update suppression when user already provided mcp_servers.relaycast config" + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/broker/src/snippets.rs` around lines 1858 - 1877, The regression test function codex_opt_out_when_legacy_relaycast_config_already_in_args validates legacy opt-out behavior but lacks an assertion message; update the final assert_eq comparing args to vec!["--config", "check_for_update_on_startup=false"] to include a descriptive failure message (matching the style used in the nearby test) so failures clearly state that the legacy relaycast config should trigger the codex opt-out path; the change should be made in the codex_opt_out_when_legacy_relaycast_config_already_in_args test that calls configure_agent_relay_mcp.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/broker/src/snippets.rs`:
- Around line 1858-1877: The regression test function
codex_opt_out_when_legacy_relaycast_config_already_in_args validates legacy
opt-out behavior but lacks an assertion message; update the final assert_eq
comparing args to vec!["--config", "check_for_update_on_startup=false"] to
include a descriptive failure message (matching the style used in the nearby
test) so failures clearly state that the legacy relaycast config should trigger
the codex opt-out path; the change should be made in the
codex_opt_out_when_legacy_relaycast_config_already_in_args test that calls
configure_agent_relay_mcp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8379b486-26ed-40ed-b556-3d8d7b0da9e0
📒 Files selected for processing (1)
crates/broker/src/snippets.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/broker/src/snippets.rs">
<violation number="1" location="crates/broker/src/snippets.rs:818">
P2: The substring check `contains("mcp_servers.relaycast")` is too broad — it will falsely match unrelated config keys like `mcp_servers.relaycast_backup` or `mcp_servers.relaycast_v2`, causing the canonical agent-relay injection to be skipped when it shouldn't be. Use a boundary-aware match (e.g., check for `mcp_servers.relaycast.` or `mcp_servers.relaycast=`) to only match the exact legacy server key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| && !existing_args | ||
| .iter() | ||
| .any(|a| a.contains("mcp_servers.agent-relay")) | ||
| .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast")) |
There was a problem hiding this comment.
P2: The substring check contains("mcp_servers.relaycast") is too broad — it will falsely match unrelated config keys like mcp_servers.relaycast_backup or mcp_servers.relaycast_v2, causing the canonical agent-relay injection to be skipped when it shouldn't be. Use a boundary-aware match (e.g., check for mcp_servers.relaycast. or mcp_servers.relaycast=) to only match the exact legacy server key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/broker/src/snippets.rs, line 818:
<comment>The substring check `contains("mcp_servers.relaycast")` is too broad — it will falsely match unrelated config keys like `mcp_servers.relaycast_backup` or `mcp_servers.relaycast_v2`, causing the canonical agent-relay injection to be skipped when it shouldn't be. Use a boundary-aware match (e.g., check for `mcp_servers.relaycast.` or `mcp_servers.relaycast=`) to only match the exact legacy server key.</comment>
<file context>
@@ -815,7 +815,7 @@ pub async fn configure_agent_relay_mcp_with_result(
&& !existing_args
.iter()
- .any(|a| a.contains("mcp_servers.agent-relay"))
+ .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast"))
{
let mcp_command = agent_relay_mcp_command();
</file context>
| .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast")) | |
| .any(|a| a.contains("mcp_servers.agent-relay") || a.contains("mcp_servers.relaycast.") || a.contains("mcp_servers.relaycast=")) |
1 participant
User description
Summary
Test
CodeAnt-AI Description
Preserve legacy Codex MCP opt-out
What Changed
mcp_servers.relaycastconfig is already present.Impact
✅ Fewer duplicate Codex MCP setups✅ Smoother migration from legacy relaycast config✅ Fewer startup prompts for existing users💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.