Improve CLAUDE.md with ports, domains, transport modes, and resources clarification#916
Improve CLAUDE.md with ports, domains, transport modes, and resources clarification#916apietosi wants to merge 1 commit intoCoplayDev:betafrom
Conversation
… clarification Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideUpdates CLAUDE.md to more precisely document the Unity MCP architecture, including network ports, transport modes, directory layout, tool domains, resource usage, and branching strategy for PRs. Flow diagram for contributor PR branching strategyflowchart TD
Start[Start new contribution] --> CheckBranch{Which branch to base from?}
CheckBranch -->|main| MainDiscouraged[main is stable release branch
Do not base PRs from main]
CheckBranch -->|beta| BetaPreferred[beta is active development branch
Base all PRs from beta]
MainDiscouraged --> ChooseBeta[Switch to beta]
ChooseBeta --> BetaPreferred
BetaPreferred --> ImplementChanges[Implement changes
and add tests]
ImplementChanges --> OpenPR[Open PR targeting beta]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughCLAUDE.md receives comprehensive documentation updates clarifying transport protocols, directory structure, port configurations for MCP integration, resource handling, and code philosophy. Changes include transport details for HTTP alongside existing protocols, expanded architecture documentation, and explicit domain-to-tool mappings. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Hey - I've left some high level feedback:
- Since port numbers and transport modes are now documented explicitly in CLAUDE.md, consider centralizing these values in a single config or constants file and referencing that in the docs to avoid drift if defaults change.
- The hard-coded list of 20 tool domains in the docs could easily get out of sync with the codebase; consider generating this list from the command modules or adding a simple check to ensure any new domain updates this section.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since port numbers and transport modes are now documented explicitly in CLAUDE.md, consider centralizing these values in a single config or constants file and referencing that in the docs to avoid drift if defaults change.
- The hard-coded list of 20 tool domains in the docs could easily get out of sync with the codebase; consider generating this list from the command modules or adding a simple check to ensure any new domain updates this section.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 12: Update the incorrect port in the documentation: replace the "MCP port
6500" mention for the Python Server entry in CLAUDE.md with the HTTP port 8080
so it matches the Docker exposes and the Server/src/main.py default http_port
(http_port default at main.py:853); ensure any other references in CLAUDE.md to
the server port are consistent with 8080.
| ↓ MCP Protocol (stdio/SSE) | ||
| Python Server (Server/src/) | ||
| ↓ MCP Protocol (stdio / SSE / HTTP) | ||
| Python Server (Server/src/) [MCP port 6500] |
There was a problem hiding this comment.
Fix the documented server port here.
Line 12 says the Python server uses MCP port 6500, but the repo currently documents and implements HTTP on 8080 instead: CLAUDE.md Lines 26 and 50 both say Docker exposes 8080, and Server/src/main.py:853 defaults http_port to 8080. As written, this will send contributors to the wrong port when they follow the architecture diagram.
📝 Proposed doc fix
-Python Server (Server/src/) [MCP port 6500]
+Python Server (Server/src/) [HTTP port 8080 by default]📝 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.
| Python Server (Server/src/) [MCP port 6500] | |
| Python Server (Server/src/) [HTTP port 8080 by default] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` at line 12, Update the incorrect port in the documentation:
replace the "MCP port 6500" mention for the Python Server entry in CLAUDE.md
with the HTTP port 8080 so it matches the Docker exposes and the
Server/src/main.py default http_port (http_port default at main.py:853); ensure
any other references in CLAUDE.md to the server port are consistent with 8080.
Summary
CustomTools/,unity-mcp-skill/,docker-compose.yml,models/,utils/, andservices/resources/services/resources/with concrete examplesbeta(active dev branch) rather thanmain(stable releases)🤖 Generated with Claude Code
Summary by Sourcery
Clarify CLAUDE.md documentation for Unity MCP architecture, transports, structure, and contribution workflow.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit