mcp: use build-time version instead of hardcoded "0.1.0"#3662
mcp: use build-time version instead of hardcoded "0.1.0"#3662Ankitsinghsisodya wants to merge 3 commits intoknative:mainfrom
Conversation
knative-prow
Bot
added
size/S
|
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
This PR updates the MCP server’s advertised implementation version to reflect the actual build-time pkg/version.Vers value (instead of a hardcoded "0.1.0"), improving accuracy of version metadata shown by MCP clients.
Changes:
- Remove the hardcoded MCP server version constant.
- Import
knative.dev/func/pkg/versionand useversion.Verswhen constructing the MCPImplementation. - Add a source-build fallback version string when
version.Versis empty.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vers := version.Vers | ||
| if vers == "" { | ||
| vers = "0.0.0+source" | ||
| } |
| vers := version.Vers | ||
| if vers == "" { | ||
| vers = "0.0.0+source" | ||
| } | ||
|
|
||
| i := mcp.NewServer( | ||
| &mcp.Implementation{ | ||
| Name: name, | ||
| Title: title, | ||
| Version: version}, | ||
| Version: vers}, |
|
/ok-to-test |
knative-prow
Bot
added
ok-to-test
|
cc @gauron99 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3662 +/- ##
==========================================
+ Coverage 54.42% 56.89% +2.46%
==========================================
Files 181 182 +1
Lines 20926 20936 +10
==========================================
+ Hits 11389 11911 +522
+ Misses 8423 7814 -609
- Partials 1114 1211 +97
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:
|
|
I wonder if mcp version should be different to func cli version. Underneath we just run 'func' as a command so technically its possible to run I like more that the mcp has its own versioning to be fair but we can have it be the same for the sake of identifying what func is used to run the mcp server. or we can display both the mcp version and func version? |
| vers := version.Vers | ||
| if vers == "" { | ||
| vers = "0.0.0+source" | ||
| } |
There was a problem hiding this comment.
This splits the logic of the default version into two places.
Let's consolidate into a single Get() *semver.Version.
Then it's all in one place, AND we can decide on the leading v depending on the situation: human-readable output prefers leading v, whereas machine-readable (like the MCP server) perfer a proper semver format which does not.
Quick sketch:
// pkg/version/version.go
package version
import "github.com/Masterminds/semver/v3"
var Vers, Kver, Hash string
const DefaultVers = "v0.0.0+source"
// Get returns the parsed semver, with a fallback when no
// build-time version was injected.
func Get() *semver.Version {
s := Vers
if s == "" {
s = DefaultVers
}
v, err := semver.NewVersion(s) // permissive: accepts leading 'v'
if err != nil {
v, _ = semver.NewVersion(DefaultVers)
}
return v
}
Callers:
// cmd/version.go (human form):
fmt.Printf("v%s\n", version.Get().String()) // String() returns "0.0.0+source"; we add the v
// or use .Original() to round-trip whatever was injected, including the v.
// pkg/mcp/mcp.go (machine form):
Version: version.Get().String(), // "0.0.0+source", clean semverThere was a problem hiding this comment.
done sir!!
That's a good point about perhaps the MCP server having a different version than func, but for simplicity let's just keep them the same for now so we don't forget to bump versions! |
- 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.
Ankitsinghsisodya
force-pushed
the
fix/mcp-hardcoded-version
branch
from
e9d77f9 to
1510117
Compare
|
[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 |
knative-prow
Bot
added
size/L
…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.
Summary
version = "0.1.0"constant to everyMCP client regardless of the actual binary version, causing stale version
metadata in clients that display server info.
version.Versfrompkg/version, which isalready injected at build time via ldflags
(
-X knative.dev/func/pkg/version.Vers=$(VERS))."0.0.0+source"when the variable is empty (source buildsthat bypass the Makefile), consistent with how
cmd/root.gohandles thesame case.
Changes
pkg/mcp/mcp.go: removeconst version = "0.1.0", importknative.dev/func/pkg/version, and useversion.Vers(with fallback)when constructing the
mcp.Implementationpassed tomcp.NewServer.Test plan
go test ./pkg/mcp/...passesmake testpasses for mcp packagefunc versionandfunc mcp startreport the same version stringfunc mcp start