spec: add openspec for MCP server setup (SANDBOX-1814) - #20
Merged
fbm3307 merged 3 commits intoJul 17, 2026
Merged
Conversation
- Capture proposal, design, tasks, and specs for mcp.Server wiring, HTTP mux, Cobra entry point, and graceful shutdown - Align with mcp-server-devsandbox mcpinit and design/implementation docs Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis change adds OpenSpec documentation for an MCP server control plane, covering HTTP and stdio transports, routes, middleware, CLI validation, session lifecycle, health probes, background workers, graceful shutdown, dependencies, and acceptance checks. ChangesMCP server setup
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
the
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@openspec/changes/2026-07-16-mcp-server-setup/design.md`:
- Around line 52-70: Resolve the HTTP statelessness contract consistently across
the design, proposal, specs, Cobra --stateless handling, and deployment
manifests. Ensure production HTTP cannot unintentionally run with Stateless
false—either require/force true for HTTP while preserving documented local
behavior, or update all related contracts to explicitly support stateful mode,
including Tools.ListChanged expectations.
- Around line 145-161: Align the required-flag rules in Decision 8 with
runServer validation: define whether sandbox-image and hmac-key-file are
required for both stdio and HTTP transports, ensure the documented default
command satisfies those rules or explicitly requires the needed flags, and
update server-entry-point/spec.md plus tests to cover both transport paths.
- Around line 3-5: Update the deployment and configuration requirements around
--address and DisableLocalhostProtection to enforce the trust boundary before
disabling localhost protection: require a loopback bind address, a verified
trusted-proxy deployment, or explicit authentication configuration. Document
that non-loopback addresses must not expose /mcp or DELETE /sessions/{id}
without authentication, and define startup validation that rejects unsafe
combinations.
In `@openspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.md`:
- Around line 42-64: Update the mux configuration for session deletion to
explicitly handle DELETE requests to /sessions/ with an empty ID, returning HTTP
400 without calling CleanupSession. Preserve the existing DELETE /sessions/{id}
route and its handler behavior for non-empty IDs, while ensuring the fallback
does not redirect or produce a 404.
In `@openspec/changes/2026-07-16-mcp-server-setup/tasks.md`:
- Around line 24-31: The HTTP transport flow in task 3.7 must observe
ListenAndServe failures instead of waiting only for signals. Capture the
goroutine’s serve result, treat non-http.ErrServerClosed errors as fatal by
canceling the shared context and returning cleanly, and preserve normal shutdown
handling for expected closure. Add an acceptance test covering an unexpected
HTTP serve failure and confirming shutdown occurs.
🪄 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: 49d260e7-850b-4e38-96dd-48418c5dac74
📒 Files selected for processing (6)
openspec/changes/2026-07-16-mcp-server-setup/.openspec.yamlopenspec/changes/2026-07-16-mcp-server-setup/design.mdopenspec/changes/2026-07-16-mcp-server-setup/proposal.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.mdopenspec/changes/2026-07-16-mcp-server-setup/tasks.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:
openspec/changes/2026-07-16-mcp-server-setup/proposal.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.mdopenspec/changes/2026-07-16-mcp-server-setup/tasks.mdopenspec/changes/2026-07-16-mcp-server-setup/design.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.md
🪛 LanguageTool
openspec/changes/2026-07-16-mcp-server-setup/design.md
[grammar] ~75-~75: Ensure spelling is correct
Context: ...thout Cobra) - cmd/server owns flags, clientset construction, SessionManager lifecycle,...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (5)
openspec/changes/2026-07-16-mcp-server-setup/.openspec.yaml (1)
1-2: LGTM!openspec/changes/2026-07-16-mcp-server-setup/proposal.md (1)
1-45: LGTM!openspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.md (1)
3-40: LGTM!Also applies to: 65-97
openspec/changes/2026-07-16-mcp-server-setup/tasks.md (1)
1-20: LGTM!Also applies to: 33-51
openspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.md (1)
3-74: LGTM!
- Require --stateless and loopback --address for HTTP; reject unsafe binds - Require sandbox-image and hmac-key-file for both transports - Add DELETE /sessions/ empty-id fallback (400, not 404) - Observe ListenAndServe failures and shut down background workers Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@openspec/changes/2026-07-16-mcp-server-setup/design.md`:
- Around line 97-100: Update the session deletion flow using
SessionManager.CleanupSession so cleanup failures return a stable generic HTTP
500 response without exposing the underlying error text. Log the wrapped cleanup
error server-side with request and session correlation details, while preserving
the 204 No Content success response.
In
`@openspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.md`:
- Around line 16-24: Update runServer’s HMAC key validation to reject readable
files whose loaded contents are zero-length, before constructing SessionManager
or starting either transport. Preserve the existing empty-path and
unreadable-file errors, and cover the non-empty-content requirement for both
stdio and http transports.
- Around line 44-55: Expand the runServer requirement to define stdio behavior:
specify how stdio blocks while serving, how transport errors are returned or
surfaced, and that shutdown stops the stale-session cleanup and warm-pool
reconciler workers. Keep the existing HTTP shutdown and fatal serve-error
semantics unchanged.
🪄 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: fd1f9528-23e2-4c2f-b918-a13b379a75b9
📒 Files selected for processing (5)
openspec/changes/2026-07-16-mcp-server-setup/design.mdopenspec/changes/2026-07-16-mcp-server-setup/proposal.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.mdopenspec/changes/2026-07-16-mcp-server-setup/tasks.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)
🚧 Files skipped from review as they are similar to previous changes (3)
- openspec/changes/2026-07-16-mcp-server-setup/proposal.md
- openspec/changes/2026-07-16-mcp-server-setup/specs/mcp-server-setup/spec.md
- openspec/changes/2026-07-16-mcp-server-setup/tasks.md
📜 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:
openspec/changes/2026-07-16-mcp-server-setup/design.mdopenspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.md
🪛 LanguageTool
openspec/changes/2026-07-16-mcp-server-setup/design.md
[grammar] ~154-~154: Ensure spelling is correct
Context: ...iler on either signal or unexpected serve failure. Treat http.ErrServerClosed a...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
openspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.md
[grammar] ~55-~55: Ensure spelling is correct
Context: .... Wait for shutdown signal or fatal serve error #### Scenario: Warm pool skipped...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~89-~89: Ensure spelling is correct
Context: ...cenario: ErrServerClosed is not a fatal serve failure - WHEN ListenAndServe ret...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
openspec/changes/2026-07-16-mcp-server-setup/design.md (1)
1-96: LGTM!Also applies to: 101-220
openspec/changes/2026-07-16-mcp-server-setup/specs/server-entry-point/spec.md (1)
1-15: LGTM!Also applies to: 25-43, 56-98
- Return generic 500 on session cleanup failure; log details server-side - Reject zero-byte HMAC key files for both transports - Define stdio Run blocking, cancel, and worker shutdown semantics Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assisted by : Cursor
Summary by CodeRabbit
/mcp, session deletion routes with status code rules,/metrics,/live, and/health(including timeout and payload expectations).