WIP: Add protocol version observability and update CRD godoc - #255
WIP: Add protocol version observability and update CRD godoc #255matzew wants to merge 2 commits into
Conversation
Upgrades github.com/modelcontextprotocol/go-sdk from v1.6.1 to v1.7.0-pre.1. Adds support for MCP protocol version 2026-07-28 (stateless MCP, server/discover). The SDK handles protocol negotiation transparently - no functional code changes required. The only code change is adding nolint:staticcheck directives on the LoggingCapabilities field, which the SDK now marks as deprecated per SEP-2577. The field remains functional during the 12-month deprecation window. Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
✅ Deploy Preview for mcp-lifecycle-operator ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Log the negotiated MCP protocol version on successful handshake in reconcileHandshake. Update CRD godoc to reflect that the handshake may use either initialize or server/discover, and add deprecation notice on the Logging capability per SEP-2577. Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
591bad0 to
26c7434
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
📝 WalkthroughWalkthroughThis PR updates documentation comments describing the MCP protocol handshake phase (broadening "initialize" to "initialize or server/discover") across API types, generated apply-configurations, and CRD schema. It adds a deprecation notice to the Logging capability field, bumps the go-sdk dependency, and adds protocolVersion logging plus staticcheck suppressions in the handshake controller and tests. ChangesHandshake docs and Logging deprecation
Estimated code review effort: 2 (Simple) | ~10 minutes Related PRs: None identified. Suggested labels: documentation, dependencies Suggested reviewers: None identified. 🐰 A handshake now speaks of two doors, initialize or discover's floor, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #255 +/- ##
=======================================
Coverage ? 84.54%
=======================================
Files ? 13
Lines ? 1482
Branches ? 0
=======================================
Hits ? 1253
Misses ? 183
Partials ? 46 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go.mod (1)
8-8: 🗄️ Data Integrity & Integration | 🔵 TrivialRevisit when v1.7.0 lands.
v1.7.0-pre.1is still the latest published SDK release, so this pre-release pin is the only available option for now.🤖 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 `@go.mod` at line 8, No code change is needed right now: the go.mod dependency pin to github.com/modelcontextprotocol/go-sdk v1.7.0-pre.1 is intentional because it is currently the latest published SDK release. Revisit the version in go.mod once v1.7.0 is available, and then update the dependency pin accordingly.
🤖 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 `@config/crd/bases/mcp.x-k8s.io_mcpservers.yaml`:
- Line 1685: The change was made in the generated CRD manifest, which should not
be edited directly; regenerate the CRD from the source API types instead. Update
the schema or annotations in mcpserver_types.go (and any related API
definitions), then run make manifests so
config/crd/bases/mcp.x-k8s.io_mcpservers.yaml is recreated consistently. This
keeps the generated YAML aligned with the source types and avoids manual drift.
---
Nitpick comments:
In `@go.mod`:
- Line 8: No code change is needed right now: the go.mod dependency pin to
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.1 is intentional because it is
currently the latest published SDK release. Revisit the version in go.mod once
v1.7.0 is available, and then update the dependency pin accordingly.
🪄 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
Run ID: a0233b46-3c85-4c77-87a5-9278582d25df
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
api/v1alpha1/applyconfiguration/api/v1alpha1/mcpservercapabilities.goapi/v1alpha1/applyconfiguration/api/v1alpha1/mcpserverinfo.goapi/v1alpha1/applyconfiguration/api/v1alpha1/mcpserverstatus.goapi/v1alpha1/mcpserver_types.goconfig/crd/bases/mcp.x-k8s.io_mcpservers.yamlgo.modinternal/controller/mcpserver_controller_handshake.gointernal/controller/mcpserver_controller_handshake_test.go
| description: |- | ||
| ServerInfo contains identity and capability information reported by the | ||
| MCP server during the protocol initialize handshake. | ||
| MCP server during the protocol handshake (initialize or server/discover). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Regenerate the CRD instead of editing the generated YAML directly.
This manifest lives under config/crd/bases/*.yaml, so hand-editing it risks the next make manifests run overwriting the change and drifting from api/v1alpha1/mcpserver_types.go. As per coding guidelines, config/crd/bases/*.yaml: Do not edit auto-generated CRD manifests: config/crd/bases/*.yaml - these are created by make manifests.
Also applies to: 1697-1701, 1726-1729
🤖 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 `@config/crd/bases/mcp.x-k8s.io_mcpservers.yaml` at line 1685, The change was
made in the generated CRD manifest, which should not be edited directly;
regenerate the CRD from the source API types instead. Update the schema or
annotations in mcpserver_types.go (and any related API definitions), then run
make manifests so config/crd/bases/mcp.x-k8s.io_mcpservers.yaml is recreated
consistently. This keeps the generated YAML aligned with the source types and
avoids manual drift.
Source: Coding guidelines
|
Matthias, is this ready for review, or still gated behind #254 landing first? CI is green across the board — happy to take a pass once you drop the WIP. |
Log the negotiated MCP protocol version on successful handshake in reconcileHandshake. Update CRD godoc to reflect that the handshake may use either initialize or server/discover, and add deprecation
notice on the Logging capability per SEP-2577.
https://modelcontextprotocol.io/seps/2577-deprecate-roots-sampling-and-logging
Summary by CodeRabbit
Bug Fixes
initializeandserver/discoverflows.Documentation