Repository navigation
Pass a valid empty MCP configuration to the auto-naming summarizer - #9473
Conversation
Extracts the claude summarizer argv into
AutoNamingEnvironmentPolicy.claudeSummarizerArguments and asserts the
--mcp-config value is a valid MCP configuration object. It currently
emits a bare {}, which Claude Code rejects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude Code 2.1.220 validates --mcp-config against a schema requiring an
mcpServers record, so the bare {} cmux passed made the summarizer exit
on argument validation and every workspace auto-naming attempt recorded
category: failed. Emit {"mcpServers":{}} instead.
Fixes #9457
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughClaude auto-naming now builds arguments through a centralized environment policy. The invocation uses a valid empty MCP configuration and disables tools, slash commands, and session persistence. New tests verify these arguments and model selection. ChangesClaude auto-naming
Estimated code review effort: 3 (Moderate) | ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (24 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@cmuxTests/AutoNamingClaudeArgumentsTests.swift`:
- Around line 31-33: Update the assertions for parsed in
AutoNamingClaudeArgumentsTests so the mcpServers value is unwrapped as a
dictionary and explicitly asserted to be empty, rather than only checking that
it exists. Preserve the existing object.count assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96cacabb-47d1-4c78-84e3-0442c28ad658
📒 Files selected for processing (4)
CLI/CMUXCLI+AutoNaming.swiftCLI/CMUXCLI+AutoNamingDispatch.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AutoNamingClaudeArgumentsTests.swift
| let object = try #require(parsed) | ||
| #expect(object["mcpServers"] as? [String: Any] != nil) | ||
| #expect(object.count == 1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that mcpServers is empty.
The current assertion accepts a non-empty mcpServers dictionary. That permits a future change to enable an MCP server while this isolation test still passes. Unwrap the dictionary and assert isEmpty.
Proposed fix
- `#expect`(object["mcpServers"] as? [String: Any] != nil)
+ let mcpServers = try `#require`(object["mcpServers"] as? [String: Any])
+ `#expect`(mcpServers.isEmpty)
`#expect`(object.count == 1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let object = try #require(parsed) | |
| #expect(object["mcpServers"] as? [String: Any] != nil) | |
| #expect(object.count == 1) | |
| let object = try `#require`(parsed) | |
| let mcpServers = try `#require`(object["mcpServers"] as? [String: Any]) | |
| `#expect`(mcpServers.isEmpty) | |
| `#expect`(object.count == 1) |
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 32-32: Prefer a is X to a as? X != nil
(prefer_type_checking)
🤖 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 `@cmuxTests/AutoNamingClaudeArgumentsTests.swift` around lines 31 - 33, Update
the assertions for parsed in AutoNamingClaudeArgumentsTests so the mcpServers
value is unwrapped as a dictionary and explicitly asserted to be empty, rather
than only checking that it exists. Preserve the existing object.count assertion.
Workspace auto-naming failed on every attempt with a Claude Code session. cmux spawned the summarizer with
--strict-mcp-config --mcp-config {}, and Claude Code 2.1.220 validates that JSON against a schema requiring anmcpServersrecord. The subprocess exited on argument validation in under a second (mcpServers: Invalid input: expected record, received undefined), so no title was ever produced and every attempt recordedautoNamingLastStatus.category: "failed".The fix emits
{"mcpServers":{}}instead, which is the same semantics (no MCP servers) in a form the current schema accepts. The argv is now built byAutoNamingEnvironmentPolicy.claudeSummarizerArguments(from:)so it is covered by a test rather than living inline in the dispatch path.Fixes #9457
Not addressed here: the reporter's secondary observations (the summarizer still loads the user's global
CLAUDE.md, and the failure is invisible from outside because the subprocess stderr and exit code are discarded). Both are real but separate from the argument bug; leaving them out keeps this change minimal.Testing
Regression test:
cmuxTests/AutoNamingClaudeArgumentsTests.swiftasserts the--mcp-configvalue is not{}and parses to an object whose only key is anmcpServersrecord, plus the tool/session-isolation flags and theANTHROPIC_SMALL_FAST_MODELoverride. Two commits,ad2dc5cadds the test against the current{}argument and5d7d7eaapplies the fix.CI could not run that test on either commit.
ci.ymlis dispatch-only right now, and both dispatched runs (30855956954 on the test-only commit, 30855966921 on the fix) failed before the macOStestsjob at a preexisting gate onmain:workflow-guard-testsreportsscripts/ghosttykit-checksums.txt is missing an entry for ghostty f0f8273b700e754b3f0052cea033789aab902fd1, which is the SHAmainpins.linux-preflightgatestestson that guard, so the same failure is hitting other branches. That pin is a separate fix.Verified at runtime instead, on macOS 26 with Claude Code 2.1.220 installed locally:
claude -p --model haiku --tools "" --disable-slash-commands --no-session-persistence --strict-mcp-config --mcp-config '{}'printsError: Invalid MCP configuration: mcpServers: Invalid input: expected record, received undefined. The same command with'{"mcpServers":{}}'returns a title../scripts/reload.sh --tag sym9457and confirmed the bundled CLI carries the new literal.workspaceAutoNamingEnabledwithautoNamingAgent: claude, ran an interactive Claude Code session in a workspace over/tmp/cmux-debug-sym9457.sock, and completed a turn so theStophook fired. The workspace was renamed tocustom_title: "OAuth Refresh Tokens"withhas_custom_title: true, andautoNamingLastStatusdid not exist afterward, which is the success signature the reporter described. Before this change the same flow leftcustom_title: nulland recordedcategory: "failed".The tagged app was quit and its temporary
defaultswere removed after verification.🤖 Generated with Claude Code