Fix Docs MCP deployment reliability - #446
Merged
Merged
Conversation
✅ Deploy Preview for genlayer-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThe PR adds an MCP SSE handshake smoke test with local fixtures, integrates it into build and scheduled health workflows, improves SQLite index failure diagnostics and rebuilding, and documents deployment, health-check, and incident-response procedures. ChangesDocs MCP health and deployment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant SmokeTest
participant DocsMCP
GitHubActions->>SmokeTest: Run MCP canary
SmokeTest->>DocsMCP: Open SSE endpoint
DocsMCP-->>SmokeTest: Return messages URL
SmokeTest->>DocsMCP: Send initialize and tools/list
DocsMCP-->>SmokeTest: Return server info and tools
SmokeTest-->>GitHubActions: Report pass or failure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
MuncleUscles
marked this pull request as ready for review
July 29, 2026 10:36
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/docs-mcp.yml (1)
32-32: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable persisted checkout credentials.
Both workflows retain the
GITHUB_TOKENin the workspace Git configuration after checkout, although neither needs Git authentication afterward.
.github/workflows/docs-mcp.yml#L32-L32: setpersist-credentials: falseon checkout before running tests and the image build..github/workflows/docs-mcp-health.yml#L20-L20: setpersist-credentials: falseon checkout before running the canary.🤖 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 @.github/workflows/docs-mcp.yml at line 32, Disable persisted checkout credentials in the checkout steps of .github/workflows/docs-mcp.yml at lines 32-32 and .github/workflows/docs-mcp-health.yml at lines 20-20 by setting persist-credentials to false before the subsequent tests, image build, and canary steps.Source: Linters/SAST tools
🤖 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 @.github/workflows/docs-mcp-health.yml:
- Around line 22-23: Update the workflow step named “Initialize MCP and list
tools” to invoke the repository’s established failure-alert integration when the
scheduled canary fails, following the configuration and usage required by
docs-mcp/OPERATIONS.md. Preserve the existing smoke-test command and ensure
alerts are emitted only for workflow failure.
In `@docs-mcp/smoke-test.mjs`:
- Around line 84-90: Update the JSON-RPC response validation around the payload
handling in the smoke test to reject malformed responses: require
payload.jsonrpc to equal "2.0" and require exactly one of payload.result or
payload.error to be present. Preserve existing error propagation for valid error
responses, and add a regression fixture covering an incomplete response such as
{"id":1} so the canary cannot pass without a valid initialization exchange.
- Line 147: Validate the URL created in the smoke test’s messagesUrl flow before
any JSON-RPC POST, ensuring endpointEvent.data remains same-origin with sseUrl
or matches an explicit approved-origin allowlist. Reject disallowed origins and
do not send requests to them; preserve the existing flow for validated message
URLs.
---
Nitpick comments:
In @.github/workflows/docs-mcp.yml:
- Line 32: Disable persisted checkout credentials in the checkout steps of
.github/workflows/docs-mcp.yml at lines 32-32 and
.github/workflows/docs-mcp-health.yml at lines 20-20 by setting
persist-credentials to false before the subsequent tests, image build, and
canary steps.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ccc4fee-e86c-48d9-8a5e-cef13a351bb7
📒 Files selected for processing (6)
.github/workflows/docs-mcp-health.yml.github/workflows/docs-mcp.ymldocs-mcp/OPERATIONS.mddocs-mcp/entrypoint.shdocs-mcp/smoke-test.mjsdocs-mcp/smoke-test.test.mjs
Comment on lines
+22
to
+23
| - name: Initialize MCP and list tools | ||
| run: node docs-mcp/smoke-test.mjs https://docs-mcp.genlayer.com/sse |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Alert when the scheduled canary fails.
This only creates a failed Actions run; it does not notify an owning channel. Add the repository’s failure-alert integration so outages are actionable, as required by docs-mcp/OPERATIONS.md lines 47-55.
🤖 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 @.github/workflows/docs-mcp-health.yml around lines 22 - 23, Update the
workflow step named “Initialize MCP and list tools” to invoke the repository’s
established failure-alert integration when the scheduled canary fails, following
the configuration and usage required by docs-mcp/OPERATIONS.md. Preserve the
existing smoke-test command and ensure alerts are emitted only for workflow
failure.
Comment on lines
+84
to
+90
| const payload = JSON.parse(event.data); | ||
| if (payload.error) { | ||
| throw new Error( | ||
| `JSON-RPC response ${id} failed: ${JSON.stringify(payload.error)}`, | ||
| ); | ||
| } | ||
| return payload; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject malformed JSON-RPC responses.
{"id":1} is accepted as a successful initialize response. If tools/list then succeeds, this canary passes without a valid initialization exchange. Require jsonrpc: "2.0" plus exactly one of result/error, and add a regression fixture.
🤖 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 `@docs-mcp/smoke-test.mjs` around lines 84 - 90, Update the JSON-RPC response
validation around the payload handling in the smoke test to reject malformed
responses: require payload.jsonrpc to equal "2.0" and require exactly one of
payload.result or payload.error to be present. Preserve existing error
propagation for valid error responses, and add a regression fixture covering an
incomplete response such as {"id":1} so the canary cannot pass without a valid
initialization exchange.
| (candidate) => candidate.event === "endpoint", | ||
| "the MCP message endpoint", | ||
| ); | ||
| const messagesUrl = new URL(endpointEvent.data, sseUrl); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not POST to arbitrary SSE-advertised origins.
endpointEvent.data is remote-controlled. A compromised public SSE service can make the scheduled GitHub runner POST JSON-RPC requests to an arbitrary URL. Require the message URL to remain same-origin, or validate it against an explicit allowlist.
Suggested guard
const messagesUrl = new URL(endpointEvent.data, sseUrl);
+ if (messagesUrl.origin !== sseUrl.origin) {
+ throw new Error("MCP message endpoint must use the SSE endpoint origin");
+ }📝 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.
Suggested change
| const messagesUrl = new URL(endpointEvent.data, sseUrl); | |
| const messagesUrl = new URL(endpointEvent.data, sseUrl); | |
| if (messagesUrl.origin !== sseUrl.origin) { | |
| throw new Error("MCP message endpoint must use the SSE endpoint origin"); | |
| } |
🤖 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 `@docs-mcp/smoke-test.mjs` at line 147, Validate the URL created in the smoke
test’s messagesUrl flow before any JSON-RPC POST, ensuring endpointEvent.data
remains same-origin with sseUrl or matches an explicit approved-origin
allowlist. Reject disallowed origins and do not send requests to them; preserve
the existing flow for validated message URLs.
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.
What changed
initializeandtools/list/web/jobs/scrapedeployment stepRoot cause
The public nginx sidecar could report
/healthzas healthy while the MCP container was unavailable, so infrastructure health was green even as MCP initialization returned HTTP 502. The image workflow also called a route that the current server does not expose and swallowed both failures, producing a false-success deployment signal. Production additionally uses a mutablelatestimage and keeps derived index state on a persistent volume, which makes rollouts non-reproducible and can preserve a bad index across restarts.Impact
This makes broken MCP handshakes fail visibly in CI and on a schedule, ensures unusable derived index state is rebuilt, and gives the production rollout an immutable, testable artifact.
Validation
node --test docs-mcp/smoke-test.test.mjsshellcheck docs-mcp/entrypoint.shactionlint .github/workflows/docs-mcp.yml .github/workflows/docs-mcp-health.ymlnode --check docs-mcp/smoke-test.mjsnode --check docs-mcp/smoke-test.test.mjsSummary by CodeRabbit
New Features
Bug Fixes
Tests