mcp: use build-time version instead of hardcoded "0.1.0"#3832
mcp: use build-time version instead of hardcoded "0.1.0"#3832Ankitsinghsisodya wants to merge 6 commits into
Conversation
- Refactor MCP server implementation to retrieve version from the new version package. - Default to "0.0.0+source" if version is not specified. - Clean up version constants for clarity.
- Updated DefaultVersion in root.go to delegate to version.DefaultVers for consistency. - Modified app.go to retrieve the version using version.Get().Original() for accurate versioning. - Adjusted mcp.go to use version.Get().String() for server versioning, ensuring fallback to DefaultVers if necessary. - Added unit tests in version_test.go to verify behavior of version retrieval and fallback logic.
…endency - Added Masterminds/semver/v3 v3.4.0 to the require section for version handling. - Removed the indirect reference to Masterminds/semver/v3 v3.4.0 for clarity.
…ing() - Modified the healthcheckHandler function in tools_healthcheck.go to call version.Get().String() for accurate version reporting. - This change ensures that the version displayed in the health check output is consistent with the new version handling introduced in previous commits.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Introduces a centralized version parsing API and updates MCP/CLI version reporting to use a normalized semver string for machine consumers while preserving the original injected value for human-readable output.
Changes:
- Added
pkg/version.Get()withDefaultVersfallback and semver parsing, plus unit tests. - Updated MCP server implementation and healthcheck tool to report
version.Get().String()instead of a hardcoded constant. - Updated CLI version wiring to use
version.Get().Original()and deduplicated the default version constant.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/version/version.go | Adds semver-backed version parsing with a shared fallback constant. |
| pkg/version/version_test.go | Adds tests for empty, injected, and invalid version behavior. |
| pkg/mcp/mcp.go | Replaces hardcoded MCP server version with parsed version string. |
| pkg/mcp/tools_healthcheck.go | Reports normalized version string in healthcheck output. |
| pkg/app/app.go | Uses the original injected version string for CLI output. |
| cmd/root.go | Delegates default version constant to version.DefaultVers. |
| go.mod | Adds a direct dependency on github.com/Masterminds/semver/v3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Updated the MCP server implementation to store the version as a field in the Server struct, ensuring consistent version reporting across health checks and server initialization. - Removed direct calls to version.Get().String() in favor of using the new s.version field, enhancing clarity and maintainability. - Adjusted the go.mod file to remove the unnecessary dependency on Masterminds/semver/v3, streamlining version management.
- Included Masterminds/semver/v3 v3.4.0 as an indirect dependency to support version handling. - This change ensures compatibility with the latest versioning practices while maintaining existing functionality.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3832 +/- ##
==========================================
- Coverage 57.03% 53.33% -3.70%
==========================================
Files 182 201 +19
Lines 21376 23456 +2080
==========================================
+ Hits 12191 12511 +320
- Misses 7953 9699 +1746
- Partials 1232 1246 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
version = "0.1.0"constant to every MCP client regardless of the actual binary version, causing stale version metadata in clients that display server info.version.Versfrompkg/version, which is already injected at build time via ldflags (-X knative.dev/func/pkg/version.Vers=$(VERS))."0.0.0+source"when the variable is empty (source builds that bypass the Makefile), consistent with howcmd/root.gohandles the same case.Changes
pkg/mcp/mcp.go: removeconst version = "0.1.0", importknative.dev/func/pkg/version, and useversion.Vers(with fallback) when constructing themcp.Implementationpassed tomcp.NewServer.Test plan
go test ./pkg/mcp/...passesfunc versionandfunc mcp startreport the same version stringfunc mcp start