Compare commits

..

1 Commits

Author SHA1 Message Date
Lunny Xiao 6694ec6873 fix(main): only trust build version when major matches module path
debug.ReadBuildInfo() can report a version whose major version does
not match the module path's major version suffix (e.g. a v2 tag on a
module without a /v2 path), which is unreliable per Go's module
versioning rules. Extract the decision into resolveVersion() and only
accept the build version when its major matches the module path's
implied major version, otherwise fall back to the dev version.

Fixes #231.

Co-Authored-By: Codet <codet@commitgo.dev> (GPT-5-Codex)
2026-08-23 23:35:02 -07:00
5 changed files with 120 additions and 142 deletions
+57 -2
View File
@@ -1,7 +1,10 @@
package main package main
import ( import (
"path"
"runtime/debug" "runtime/debug"
"strconv"
"strings"
"gitea.com/gitea/gitea-mcp/cmd" "gitea.com/gitea/gitea-mcp/cmd"
"gitea.com/gitea/gitea-mcp/pkg/flag" "gitea.com/gitea/gitea-mcp/pkg/flag"
@@ -11,13 +14,65 @@ var Version = "dev"
func init() { func init() {
if Version == "dev" { if Version == "dev" {
if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "" && info.Main.Version != "(devel)" { if info, ok := debug.ReadBuildInfo(); ok {
Version = info.Main.Version Version = resolveVersion(Version, info)
} }
} }
flag.Version = Version flag.Version = Version
} }
// resolveVersion returns the version reported by debug.ReadBuildInfo when its
// major version matches the major version encoded in the module path (e.g.
// "/v2" suffix). Otherwise it falls back to devVersion, since Go's module
// versioning rules make a mismatched major version untrustworthy (see #231).
func resolveVersion(devVersion string, info *debug.BuildInfo) string {
if info == nil {
return devVersion
}
buildVersion := info.Main.Version
if buildVersion == "" || buildVersion == "(devel)" {
return devVersion
}
buildMajor := majorVersionOf(buildVersion)
pathMajor := majorVersionFromModulePath(info.Main.Path)
if buildMajor != pathMajor {
return devVersion
}
return buildVersion
}
// majorVersionOf extracts the numeric major version from a semver-like
// string such as "v1.2.3", returning 0 if it cannot be parsed.
func majorVersionOf(version string) int {
version = strings.TrimPrefix(version, "v")
dot := strings.IndexByte(version, '.')
if dot >= 0 {
version = version[:dot]
}
major, err := strconv.Atoi(version)
if err != nil {
return 0
}
return major
}
// majorVersionFromModulePath returns the major version encoded in a module
// path's "/vN" suffix, or 1 if the module path has no such suffix (as is the
// case for v0 and v1 modules).
func majorVersionFromModulePath(modulePath string) int {
suffix := path.Base(modulePath)
if len(suffix) < 2 || suffix[0] != 'v' {
return 1
}
major, err := strconv.Atoi(suffix[1:])
if err != nil {
return 1
}
return major
}
func main() { func main() {
cmd.Execute() cmd.Execute()
} }
+61
View File
@@ -0,0 +1,61 @@
package main
import (
"runtime/debug"
"testing"
)
func TestResolveVersion(t *testing.T) {
cases := []struct {
name string
dev string
info *debug.BuildInfo
want string
}{
{
name: "nil build info falls back to dev version",
dev: "dev",
info: nil,
want: "dev",
},
{
name: "devel version falls back to dev version",
dev: "dev",
info: &debug.BuildInfo{Main: debug.Module{Path: "gitea.com/gitea/gitea-mcp", Version: "(devel)"}},
want: "dev",
},
{
name: "empty version falls back to dev version",
dev: "dev",
info: &debug.BuildInfo{Main: debug.Module{Path: "gitea.com/gitea/gitea-mcp", Version: ""}},
want: "dev",
},
{
name: "v1 version accepted for module path without major suffix",
dev: "dev",
info: &debug.BuildInfo{Main: debug.Module{Path: "gitea.com/gitea/gitea-mcp", Version: "v1.2.3"}},
want: "v1.2.3",
},
{
name: "v2 version rejected when module path has no /v2 suffix",
dev: "dev",
info: &debug.BuildInfo{Main: debug.Module{Path: "gitea.com/gitea/gitea-mcp", Version: "v2.0.0"}},
want: "dev",
},
{
name: "v2 version accepted when module path has /v2 suffix",
dev: "dev",
info: &debug.BuildInfo{Main: debug.Module{Path: "gitea.com/gitea/gitea-mcp/v2", Version: "v2.0.0"}},
want: "v2.0.0",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := resolveVersion(tc.dev, tc.info)
if got != tc.want {
t.Errorf("resolveVersion(%q, %+v) = %q, want %q", tc.dev, tc.info, got, tc.want)
}
})
}
}
+2 -41
View File
@@ -20,13 +20,6 @@ import (
var Tool = tool.New("pull_request") var Tool = tool.New("pull_request")
// commentWithAssets wraps the SDK Comment to capture the `assets` field that
// the SDK currently drops on the issue comments endpoint.
type commentWithAssets struct {
gitea_sdk.Comment
Assets []*gitea_sdk.Attachment `json:"assets"`
}
const ( const (
ListRepoPullRequestsToolName = "list_pull_requests" ListRepoPullRequestsToolName = "list_pull_requests"
PullRequestReadToolName = "pull_request_read" PullRequestReadToolName = "pull_request_read"
@@ -50,9 +43,9 @@ var (
PullRequestReadTool = tool.NewDefinition( PullRequestReadTool = tool.NewDefinition(
PullRequestReadToolName, PullRequestReadToolName,
"Read pull request: details, diff, changed files, head commit status, reviews, review comments, discussion comments.", "Read pull request: details, diff, changed files, head commit status, reviews, review comments.",
annotation.ReadOnly("Read pull request details"), annotation.ReadOnly("Read pull request details"),
tool.String("method", tool.Required(), tool.Enum("get", "get_diff", "get_files", "get_status", "get_reviews", "get_review", "get_review_comments", "get_comments")), tool.String("method", tool.Required(), tool.Enum("get", "get_diff", "get_files", "get_status", "get_reviews", "get_review", "get_review_comments")),
tool.String("owner", tool.Required(), tool.Description(params.OwnerDesc)), tool.String("owner", tool.Required(), tool.Description(params.OwnerDesc)),
tool.String("repo", tool.Required(), tool.Description(params.RepoDesc)), tool.String("repo", tool.Required(), tool.Description(params.RepoDesc)),
tool.Number("pull_number", tool.Required()), tool.Number("pull_number", tool.Required()),
@@ -158,8 +151,6 @@ func pullRequestReadFn(ctx context.Context, args map[string]any) (*mcp.CallToolR
return getPullRequestReviewFn(ctx, args) return getPullRequestReviewFn(ctx, args)
case "get_review_comments": case "get_review_comments":
return listPullRequestReviewCommentsFn(ctx, args) return listPullRequestReviewCommentsFn(ctx, args)
case "get_comments":
return listPullRequestCommentsFn(ctx, args)
default: default:
return to.ErrorResult(fmt.Errorf("unknown method: %s", method)) return to.ErrorResult(fmt.Errorf("unknown method: %s", method))
} }
@@ -612,36 +603,6 @@ func listPullRequestReviewCommentsFn(ctx context.Context, args map[string]any) (
return to.TextResult(slimReviewComments(comments)) return to.TextResult(slimReviewComments(comments))
} }
func listPullRequestCommentsFn(ctx context.Context, args map[string]any) (*mcp.CallToolResult, error) {
owner, err := params.GetString(args, "owner")
if err != nil {
return to.ErrorResult(err)
}
repo, err := params.GetString(args, "repo")
if err != nil {
return to.ErrorResult(err)
}
index, err := params.GetIndex(args, "pull_number")
if err != nil {
return to.ErrorResult(err)
}
// PRs are issues internally, so the regular discussion comments live on
// the issue comments endpoint rather than a pull-specific one.
var comments []commentWithAssets
path := fmt.Sprintf("repos/%s/%s/issues/%d/comments", url.PathEscape(owner), url.PathEscape(repo), index)
if _, err := gitea.DoJSON(ctx, "GET", path, nil, nil, &comments); err != nil {
return to.ErrorResult(fmt.Errorf("get %v/%v/pr/%v comments err: %v", owner, repo, index, err))
}
out := make([]map[string]any, 0, len(comments))
for i := range comments {
m := slimComment(&comments[i].Comment)
m["body"] = slim.BodyWithAttachments(comments[i].Body, comments[i].Assets)
out = append(out, m)
}
return to.TextResult(out)
}
func createPullRequestReviewFn(ctx context.Context, args map[string]any) (*mcp.CallToolResult, error) { func createPullRequestReviewFn(ctx context.Context, args map[string]any) (*mcp.CallToolResult, error) {
owner, err := params.GetString(args, "owner") owner, err := params.GetString(args, "owner")
if err != nil { if err != nil {
-85
View File
@@ -943,91 +943,6 @@ func Test_closePullRequestFn(t *testing.T) {
} }
} }
func Test_listPullRequestCommentsFn_missingArgs(t *testing.T) {
result, err := listPullRequestCommentsFn(context.Background(), map[string]any{
"owner": "octo",
"repo": "demo",
})
if err != nil {
t.Fatalf("listPullRequestCommentsFn() error = %v", err)
}
if result == nil || !result.IsError {
t.Fatalf("listPullRequestCommentsFn() result = %#v, want an error result for missing pull_number", result)
}
}
func Test_listPullRequestCommentsFn_apiError(t *testing.T) {
const (
owner = "octo"
repo = "demo"
index = 7
)
serveStub(t, func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/comments", owner, repo, index) {
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
w.WriteHeader(http.StatusNotFound)
return
}
http.Error(w, "boom", http.StatusInternalServerError)
})
args := map[string]any{
"owner": owner, "repo": repo, "pull_number": float64(index),
}
result, err := listPullRequestCommentsFn(context.Background(), args)
if err != nil {
t.Fatalf("listPullRequestCommentsFn() error = %v", err)
}
if result == nil || !result.IsError {
t.Fatalf("listPullRequestCommentsFn() result = %#v, want an error result on API failure", result)
}
}
func Test_listPullRequestCommentsFn_decodesComments(t *testing.T) {
const (
owner = "octo"
repo = "demo"
index = 7
)
serveStub(t, func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/comments", owner, repo, index) {
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
w.WriteHeader(http.StatusNotFound)
return
}
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`[
{"id": 1, "body": "see this", "assets": [
{"id": 9, "name": "log.txt", "size": 200, "browser_download_url": "https://example/log.txt"}
]},
{"id": 2, "body": "no attachment", "assets": []}
]`))
})
args := map[string]any{
"method": "get_comments", "owner": owner, "repo": repo, "pull_number": float64(index),
}
result, err := pullRequestReadFn(context.Background(), args)
if err != nil {
t.Fatalf("pullRequestReadFn() error = %v", err)
}
if result.IsError {
t.Fatalf("unexpected error result: %v", result.Content)
}
body := result.Content[0].(*mcp.TextContent).Text
if !strings.Contains(body, `[log.txt](https://example/log.txt)`) {
t.Fatalf("expected attachment markdown inlined in body, got: %s", body)
}
if !strings.Contains(body, `"no attachment"`) {
t.Fatalf("expected second comment body preserved, got: %s", body)
}
if strings.Contains(body, `"assets"`) {
t.Fatalf("assets should be inlined into body, not a separate field: %s", body)
}
}
func Test_reopenPullRequestFn(t *testing.T) { func Test_reopenPullRequestFn(t *testing.T) {
const ( const (
owner = "octo" owner = "octo"
-14
View File
@@ -164,17 +164,3 @@ func slimReviewComments(comments []*gitea_sdk.PullReviewComment) []map[string]an
} }
return out return out
} }
func slimComment(c *gitea_sdk.Comment) map[string]any {
if c == nil {
return nil
}
return map[string]any{
"id": c.ID,
"body": c.Body,
"user": slim.UserLogin(c.Poster),
"html_url": c.HTMLURL,
"created_at": c.Created,
"updated_at": c.Updated,
}
}