Range over slice copies each element by value; use index or pointer receiver for large structs to avoid copies
for i, snap := range commits {
1// SPDX-License-Identifier: MIT23package main45import (6 "context"7 "encoding/json"8 "os"9 "path/filepath"10 "strconv"11 "strings"12 "testing"13 "time"1415 "github.com/go-git/go-git/v5"16 "github.com/go-git/go-git/v5/plumbing/object"17 "github.com/mark3labs/mcp-go/mcp"1819 jsoniter "github.com/json-iterator/go"20)2122// makeCouplingRepo initialises a real on-disk git repo whose history couples23// alpha.go and beta.go: they change together in every commit, so the pair24// clears CouplingMinShared. gamma.go is touched once, so its pairs fall below25// the floor and never surface — giving both the all-pairs and per-file views a26// single, predictable coupling to assert on. Returns the repo path.27func makeCouplingRepo(t *testing.T) string {28 t.Helper()29 dir := t.TempDir()3031 repo, err := git.PlainInit(dir, false)32 if err != nil {33 t.Fatalf("init repo: %v", err)34 }35 wt, err := repo.Worktree()36 if err != nil {37 t.Fatalf("worktree: %v", err)38 }3940 commits := []map[string]string{41 {"alpha.go": "package a\n// v0\n", "beta.go": "package b\n// v0\n"},42 {"alpha.go": "package a\n// v1\n", "beta.go": "package b\n// v1\n"},43 {"alpha.go": "package a\n// v2\n", "beta.go": "package b\n// v2\n", "gamma.go": "package g\n// v0\n"},44 }4546 when := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)47 for i, snap := range commits {48 for path, content := range snap {49 full := filepath.Join(dir, path)50 if err := os.WriteFile(full, []byte(content), 0o644); err != nil {51 t.Fatalf("write %s: %v", full, err)52 }53 if _, err := wt.Add(path); err != nil {54 t.Fatalf("add %s: %v", path, err)55 }56 }57 _, err := wt.Commit("commit "+strconv.Itoa(i), &git.CommitOptions{58 Author: &object.Signature{59 Name: "Tester",60 Email: "tester@example.com",61 When: when.Add(time.Duration(i) * time.Hour),62 },63 })64 if err != nil {65 t.Fatalf("commit %d: %v", i, err)66 }67 }68 return dir69}7071// nestedCognitiveSource is a small Go file whose branch keywords are nested,72// so cognitive complexity (nesting-weighted) exceeds plain cyclomatic73// complexity when the metric is enabled.74const nestedCognitiveSource = `package sample7576func deeplyNested(items []int) int {77 total := 078 for _, v := range items {79 if v > 0 {80 for i := 0; i < v; i++ {81 if i%2 == 0 {82 total += i83 }84 }85 }86 }87 return total88}89`9091// callAnalyze runs the MCP analyze handler against dir with the supplied args92// and returns the decoded response.93func callAnalyze(t *testing.T, dir string, args map[string]any) mcpAnalyzeResponse {94 t.Helper()9596 if args == nil {97 args = map[string]any{}98 }99 args["path"] = dir100101 req := mcp.CallToolRequest{102 Params: mcp.CallToolParams{103 Name: "analyze",104 Arguments: args,105 },106 }107108 result, err := mcpAnalyzeHandler(context.Background(), req)109 if err != nil {110 t.Fatalf("mcpAnalyzeHandler returned error: %v", err)111 }112 if result.IsError {113 t.Fatalf("mcpAnalyzeHandler returned tool error: %+v", result.Content)114 }115 if len(result.Content) == 0 {116 t.Fatalf("mcpAnalyzeHandler returned no content")117 }118119 text, ok := result.Content[0].(mcp.TextContent)120 if !ok {121 t.Fatalf("expected TextContent, got %T", result.Content[0])122 }123124 var resp mcpAnalyzeResponse125 if err := json.Unmarshal([]byte(text.Text), &resp); err != nil {126 t.Fatalf("failed to decode analyze response: %v\n%s", err, text.Text)127 }128 return resp129}130131func writeNestedFixture(t *testing.T) string {132 t.Helper()133 dir := t.TempDir()134 if err := os.WriteFile(filepath.Join(dir, "sample.go"), []byte(nestedCognitiveSource), 0o644); err != nil {135 t.Fatalf("failed to write fixture: %v", err)136 }137 return dir138}139140// callCoupling invokes the coupling MCP handler with the given arguments and141// returns the result, failing the test on a transport-level (non-tool) error.142func callCoupling(t *testing.T, args map[string]any) *mcp.CallToolResult {143 t.Helper()144 req := mcp.CallToolRequest{}145 req.Params.Name = "coupling"146 req.Params.Arguments = args147 res, err := mcpCouplingHandler(context.Background(), req)148 if err != nil {149 t.Fatalf("handler returned transport error: %v", err)150 }151 if res == nil {152 t.Fatal("handler returned nil result")153 }154 return res155}156157// resultText concatenates the text content of a tool result.158func resultText(t *testing.T, res *mcp.CallToolResult) string {159 t.Helper()160 var sb strings.Builder161 for _, c := range res.Content {162 if tc, ok := c.(mcp.TextContent); ok {163 sb.WriteString(tc.Text)164 }165 }166 return sb.String()167}168169// TestMCPCouplingAllPairs: no `file` argument returns the repo-wide all-pairs170// report — distinguished by report:"coupling" and a top-level "pairs" array.171func TestMCPCouplingAllPairs(t *testing.T) {172 dir := makeCouplingRepo(t)173174 res := callCoupling(t, map[string]any{"path": dir})175 if res.IsError {176 t.Fatalf("expected success, got error: %s", resultText(t, res))177 }178179 var doc struct {180 Report string `json:"report"`181 Pairs []struct {182 FileA string `json:"fileA"`183 FileB string `json:"fileB"`184 Shared int `json:"shared"`185 } `json:"pairs"`186 }187 if err := jsoniter.Unmarshal([]byte(resultText(t, res)), &doc); err != nil {188 t.Fatalf("unmarshal all-pairs JSON: %v\n%s", err, resultText(t, res))189 }190191 if doc.Report != "coupling" {192 t.Errorf("report = %q, want %q (per-file shape leaked into all-pairs mode)", doc.Report, "coupling")193 }194 if len(doc.Pairs) == 0 {195 t.Fatalf("expected at least one coupled pair, got none: %s", resultText(t, res))196 }197 // alpha.go and beta.go co-change in all three commits.198 p := doc.Pairs[0]199 if !((p.FileA == "alpha.go" && p.FileB == "beta.go") || (p.FileA == "beta.go" && p.FileB == "alpha.go")) {200 t.Errorf("top pair = (%s, %s), want the alpha.go/beta.go pair", p.FileA, p.FileB)201 }202 if p.Shared != 3 {203 t.Errorf("shared = %d, want 3", p.Shared)204 }205}206207// TestMCPCouplingPerFile: with `file` set, the per-file blast-radius report is208// returned unchanged — report:"coupling-for" with a "target" and "partners".209func TestMCPCouplingPerFile(t *testing.T) {210 dir := makeCouplingRepo(t)211212 res := callCoupling(t, map[string]any{"path": dir, "file": "alpha.go"})213 if res.IsError {214 t.Fatalf("expected success, got error: %s", resultText(t, res))215 }216217 var doc struct {218 Report string `json:"report"`219 Target string `json:"target"`220 Partners []struct {221 File string `json:"file"`222 } `json:"partners"`223 }224 if err := jsoniter.Unmarshal([]byte(resultText(t, res)), &doc); err != nil {225 t.Fatalf("unmarshal per-file JSON: %v\n%s", err, resultText(t, res))226 }227228 if doc.Report != "coupling-for" {229 t.Errorf("report = %q, want %q", doc.Report, "coupling-for")230 }231 if doc.Target != "alpha.go" {232 t.Errorf("target = %q, want %q", doc.Target, "alpha.go")233 }234 if len(doc.Partners) != 1 || doc.Partners[0].File != "beta.go" {235 t.Errorf("partners = %+v, want a single beta.go entry", doc.Partners)236 }237}238239// TestMCPCouplingUnknownFile: an unknown `file` still surfaces the existing240// "not in HEAD" error rather than silently falling back to the all-pairs view.241func TestMCPCouplingUnknownFile(t *testing.T) {242 dir := makeCouplingRepo(t)243244 res := callCoupling(t, map[string]any{"path": dir, "file": "does-not-exist.go"})245 if !res.IsError {246 t.Fatalf("expected error for unknown file, got success: %s", resultText(t, res))247 }248 msg := resultText(t, res)249 if !strings.Contains(msg, "not in HEAD") {250 t.Errorf("error = %q, want it to mention the target is not in HEAD", msg)251 }252 // The MCP caller passed a `file` argument and has never seen the CLI flag —253 // no flag name should leak into the message surfaced through MCP.254 if strings.Contains(msg, "--") {255 t.Errorf("error = %q, want no CLI flag names in the MCP-surfaced message", msg)256 }257}258259// TestMcpAnalyzeCognitiveEnabled verifies that requesting cognitive=true260// yields a non-zero cognitive metric at the per-language, per-file and totals261// levels, and that the totals equal the sum of the per-language values.262func TestMcpAnalyzeCognitiveEnabled(t *testing.T) {263 dir := writeNestedFixture(t)264265 resp := callAnalyze(t, dir, map[string]any{266 "cognitive": true,267 "by_file": true,268 })269270 if len(resp.Languages) == 0 {271 t.Fatalf("expected at least one language in response")272 }273274 var sumLang int64275 for _, l := range resp.Languages {276 sumLang += l.Cognitive277 }278279 if resp.Totals.Cognitive <= 0 {280 t.Fatalf("expected non-zero cognitive total, got %d", resp.Totals.Cognitive)281 }282 if sumLang != resp.Totals.Cognitive {283 t.Fatalf("totals cognitive (%d) != sum of per-language cognitive (%d)", resp.Totals.Cognitive, sumLang)284 }285286 // Cognitive should exceed plain cyclomatic complexity for this nested input.287 if resp.Totals.Cognitive <= resp.Totals.Complexity {288 t.Fatalf("expected cognitive (%d) > complexity (%d) for nested source", resp.Totals.Cognitive, resp.Totals.Complexity)289 }290291 // Per-file cognitive must be populated too.292 var fileCognitive int64293 for _, l := range resp.Languages {294 for _, f := range l.FileList {295 fileCognitive += f.Cognitive296 }297 }298 if fileCognitive != resp.Totals.Cognitive {299 t.Fatalf("sum of per-file cognitive (%d) != totals cognitive (%d)", fileCognitive, resp.Totals.Cognitive)300 }301}302303// TestMcpAnalyzeCognitiveDisabled verifies that without the opt-in parameter304// the cognitive field is zero everywhere, leaving default output unchanged.305func TestMcpAnalyzeCognitiveDisabled(t *testing.T) {306 dir := writeNestedFixture(t)307308 resp := callAnalyze(t, dir, map[string]any{309 "by_file": true,310 })311312 if resp.Totals.Cognitive != 0 {313 t.Fatalf("expected zero cognitive total when not requested, got %d", resp.Totals.Cognitive)314 }315 for _, l := range resp.Languages {316 if l.Cognitive != 0 {317 t.Fatalf("expected zero cognitive for language %s, got %d", l.Name, l.Cognitive)318 }319 for _, f := range l.FileList {320 if f.Cognitive != 0 {321 t.Fatalf("expected zero cognitive for file %s, got %d", f.Filename, f.Cognitive)322 }323 }324 }325}326327// TestMcpAnalyzeCognitiveNoLeak guards against per-call state accumulation:328// two consecutive analyze calls over the same input must return identical329// cognitive numbers (cf. the ProcessConstants MCP-leak class of bug).330func TestMcpAnalyzeCognitiveNoLeak(t *testing.T) {331 dir := writeNestedFixture(t)332333 first := callAnalyze(t, dir, map[string]any{"cognitive": true})334 second := callAnalyze(t, dir, map[string]any{"cognitive": true})335336 if first.Totals.Cognitive != second.Totals.Cognitive {337 t.Fatalf("cognitive total changed across calls: %d then %d", first.Totals.Cognitive, second.Totals.Cognitive)338 }339 if first.Totals.Complexity != second.Totals.Complexity {340 t.Fatalf("complexity total changed across calls: %d then %d", first.Totals.Complexity, second.Totals.Complexity)341 }342 if first.Totals.Code != second.Totals.Code {343 t.Fatalf("code total changed across calls: %d then %d", first.Totals.Code, second.Totals.Code)344 }345}346347// TestMcpAnalyzeNoDuplicatesNoLeak guards the duplicate-detection state leaking348// between MCP calls. The hashes live in a package-level accumulator that scc,349// as a one-shot CLI, never needed to reset. In the long-lived server every file350// of the second call matched a hash recorded by the first, so the second and351// every later call returned an empty result.352func TestMcpAnalyzeNoDuplicatesNoLeak(t *testing.T) {353 dir := writeDuplicateFixture(t)354355 first := callAnalyze(t, dir, map[string]any{"no_duplicates": true})356 second := callAnalyze(t, dir, map[string]any{"no_duplicates": true})357358 // The fixture is three copies of one file, so deduplication must leave359 // exactly one on every call — not one then zero.360 if first.Totals.Files != 1 {361 t.Fatalf("expected 1 file after deduplication, got %d", first.Totals.Files)362 }363 if second.Totals.Files != first.Totals.Files {364 t.Fatalf("file count changed across calls: %d then %d", first.Totals.Files, second.Totals.Files)365 }366 if second.Totals.Code != first.Totals.Code {367 t.Fatalf("code total changed across calls: %d then %d", first.Totals.Code, second.Totals.Code)368 }369}370371// writeDuplicateFixture writes three byte-identical Go files into a temp dir.372func writeDuplicateFixture(t *testing.T) string {373 t.Helper()374 dir := t.TempDir()375 for _, name := range []string{"a.go", "b.go", "c.go"} {376 if err := os.WriteFile(filepath.Join(dir, name), []byte(nestedCognitiveSource), 0o644); err != nil {377 t.Fatalf("failed to write fixture: %v", err)378 }379 }380 return dir381}
Code quality findings 11
Range over slice copies each element by value; use index or pointer receiver for large structs to avoid copies
for path, content := range snap {
Can cause issues on Windows consider filepath.Join instead
full := filepath.Join(dir, path)
String to byte slice conversion inside loop allocates a new slice each iteration; convert once before the loop
if err := os.WriteFile(full, []byte(content), 0o644); err != nil {
Deeply nested control structures reduce readability; consider extracting to functions or using early returns
for i := 0; i < v; i++ {
Deeply nested control structures reduce readability; consider extracting to functions or using early returns
if i%2 == 0 {
Uses root context; pass an existing context for cancellation and timeouts
result, err := mcpAnalyzeHandler(context.Background(), req)
Can cause issues on Windows consider filepath.Join instead
if err := os.WriteFile(filepath.Join(dir, "sample.go"), []byte(nestedCognitiveSource), 0o644); err != nil {
Uses root context; pass an existing context for cancellation and timeouts
res, err := mcpCouplingHandler(context.Background(), req)
Can cause issues on Windows consider filepath.Join instead
if err := os.WriteFile(filepath.Join(dir, name), []byte(nestedCognitiveSource), 0o644); err != nil {
String to byte slice conversion inside loop allocates a new slice each iteration; convert once before the loop
if err := os.WriteFile(filepath.Join(dir, name), []byte(nestedCognitiveSource), 0o644); err != nil {