docs: rewrite README for operators and clients - #32
Conversation
- Lead with purpose, features, and client-agnostic usage - Document session lifecycle, flags, and custom sandbox images - Add high-level architecture focused on scale and security Signed-off-by: Alexey Kazakov <alkazako@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThe README is substantially rewritten into a user- and operator-focused guide covering MCP usage, session routing, configuration, sandbox images, architecture, security, development, and reference links. ChangesREADME guide rewrite
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
fbm3307
left a comment
There was a problem hiding this comment.
LGTM overall — much better operator/client guide.
I had this planned for later, thanks for doing it
Some small details
- pods use runAsNonRoot without a fixed UID (OpenShift SCC).
- Nice-to-haves: document RFC 1123 session ID format, and note that DELETE /sessions/{id} is HTTP-only.
Co-authored-by: Feny Mehta <fmehta@redhat.com>
Co-authored-by: Feny Mehta <fmehta@redhat.com>
Co-authored-by: Feny Mehta <fmehta@redhat.com>
Co-authored-by: Feny Mehta <fmehta@redhat.com>
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 `@README.md`:
- Line 20: Update the README’s HTTP and stdio client guidance and all related
examples to use the deployed API’s canonical Mcp-Session-Id header instead of
X-Session-ID. Ensure every session-routing reference is consistent, or
explicitly document the supported translation layer if X-Session-ID must remain
accepted.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5837c03c-f3af-432a-bcdc-40b90f819e58
📒 Files selected for processing (1)
README.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/mcp-common(manual)codeready-toolchain/mcp-server-devsandbox(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
README.md
🪛 LanguageTool
README.md
[grammar] ~46-~46: Ensure spelling is correct
Context: ...}[a-z0-9])?$`); invalid IDs fail at pod create. Typical client practice (agent harness /...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔀 Multi-repo context codeready-toolchain/mcp-server-devsandbox, codeready-toolchain/mcp-common
Linked repositories findings
codeready-toolchain/mcp-server-devsandbox
- The deployment usage documentation extracts and sends the session identifier using the MCP-standard
Mcp-Session-Idheader indeploy/README.md:184-221, notX-Session-ID. The rewritten README’s explicitX-Session-IDrequirement should be checked for consistency with the deployed API/client behavior. [::codeready-toolchain/mcp-server-devsandbox::]
codeready-toolchain/mcp-common
- Shared middleware obtains session IDs from the MCP SDK session object (
req.GetSession().ID()), confirming that session identity is modeled as an MCP session rather than a custom header-specific contract. [::codeready-toolchain/mcp-common::]
🔇 Additional comments (2)
README.md (2)
112-112: 🔒 Security & PrivacyVerify that the advertised pod hardening is actually enforced.
Before documenting
runAsNonRoot, privilege-drop, capability removal, and resource limits as guarantees, confirm that every sandbox pod—including warm-pool pods—sets these fields in its generated pod spec. Otherwise, this README overstates the security boundary.Source: Path instructions
62-64: 🩺 Stability & AvailabilityKeep the HTTP bind loopback-only. The README matches the intended kube-rbac-proxy deployment model, and non-loopback HTTP addresses are already rejected at startup.
> Likely an incorrect or invalid review comment.
6bb419a
into
codeready-toolchain:master
Summary by CodeRabbit
bashtool, parameters, expected outputs, timeout behavior, and required session routing viaX-Session-ID.