diff --git a/internal/mcp/tools_analyze_hotspots_scope_reads_test.go b/internal/mcp/tools_analyze_hotspots_scope_reads_test.go new file mode 100644 index 000000000..e5f47f1ae --- /dev/null +++ b/internal/mcp/tools_analyze_hotspots_scope_reads_test.go @@ -0,0 +1,199 @@ +package mcp + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/analysis" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" +) + +type hotspotScopeReader struct { + graph.Reader + lookedUp []string +} + +func (r *hotspotScopeReader) GetNode(id string) *graph.Node { + r.lookedUp = append(r.lookedUp, id) + return r.Reader.GetNode(id) +} + +// Real indexing supplies the repo registry and verifies the source-ID invariant +// for both callable kinds. Extra ranked rows exercise exact totals past the cap. +func hotspotScopeFixture(t *testing.T, sqlite bool) (*Server, []analysis.HotspotEntry) { + t.Helper() + body := "package main\ntype T struct{}\nfunc F() {}\nfunc (T) M() {}\n" + s, _ := newAnalyzeServer(t, true, + analyzeRepoSpec{name: "gortex", workspace: "shared", body: body}, + analyzeRepoSpec{name: "gortex2", workspace: "shared", body: body}, + analyzeRepoSpec{name: "gortex@feature", workspace: "feature", body: body}, + ) + kinds := map[graph.NodeKind]bool{} + for _, n := range s.graph.AllNodes() { + if n.Kind == graph.KindFunction || n.Kind == graph.KindMethod { + require.Equal(t, n.RepoPrefix, graph.RepoPrefixOfID(n.ID), n.ID) + kinds[n.Kind] = true + } + } + require.True(t, kinds[graph.KindFunction]) + require.True(t, kinds[graph.KindMethod]) + if sqlite { + store, err := store_sqlite.Open(filepath.Join(t.TempDir(), "hotspots.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, store.Close()) }) + store.AddBatch(s.graph.AllNodes(), s.graph.AllEdges()) + s.graph = store + } + var entries []analysis.HotspotEntry + for i := range 23 { + for _, repo := range []string{"gortex2", "gortex@feature", "gortex"} { + ws := "shared" + if repo == "gortex@feature" { + ws = "feature" + } + kind := graph.KindFunction + if i%2 != 0 { + kind = graph.KindMethod + } + n := &graph.Node{ID: fmt.Sprintf("%s/pkg/hot.go::F%02d", repo, i), Name: fmt.Sprintf("F%02d", i), Kind: kind, + FilePath: repo + "/pkg/hot.go", RepoPrefix: repo, WorkspaceID: ws} + s.graph.AddNode(n) + entries = append(entries, analysis.HotspotEntry{ID: n.ID, Name: n.Name, Kind: string(kind), FilePath: n.FilePath, ComplexityScore: float64(100 - len(entries))}) + } + } + // Stub and unknown-prefix rows cannot safely use a slash-derived repo key. + for _, n := range []*graph.Node{ + {ID: "gortex@feature::stdlib::net/http::Get", Kind: graph.KindFunction, RepoPrefix: "gortex@feature", WorkspaceID: "feature", FilePath: "external::net/http"}, + {ID: "unknown/pkg/legacy.go::F", Kind: graph.KindFunction, RepoPrefix: "gortex", WorkspaceID: "shared", FilePath: "unknown/pkg/legacy.go"}, + } { + s.graph.AddNode(n) + entries = append(entries, analysis.HotspotEntry{ID: n.ID, Kind: string(n.Kind), FilePath: n.FilePath}) + } + for repo, callers := range map[string]int{"gortex": 22, "gortex2": 20, "gortex@feature": 18} { + for i := 1; i <= callers; i++ { + s.graph.AddEdge(&graph.Edge{From: fmt.Sprintf("%s/pkg/hot.go::F%02d", repo, i), To: repo + "/pkg/hot.go::F00", Kind: graph.EdgeCalls}) + } + } + installCommunitiesForTest(s, &analysis.CommunityResult{}) + s.hotspots = entries + s.hotspotsReady = true + return s, entries +} + +func TestHotspotsScopeSkipsInvisibleRepoReads(t *testing.T) { + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "0") + for _, backend := range []string{"memory", "sqlite"} { + t.Run(backend, func(t *testing.T) { + s, cached := hotspotScopeFixture(t, backend == "sqlite") + for _, tc := range []struct { + name, workspace, repo, mode string + bound bool + threshold float64 + reads, total int + }{ + {name: "repo", repo: "gortex", reads: 25, total: 24}, + {name: "workspace", workspace: "shared", bound: true, reads: 48, total: 47}, + {name: "workspace_repo", workspace: "shared", repo: "gortex2", bound: true, reads: 25, total: 23}, + {name: "worktree", repo: "gortex@feature", reads: 25, total: 24}, + {name: "unbound", reads: 0, total: 71}, + {name: "deny_all", workspace: "missing", bound: true, reads: 2, total: 0}, + {name: "novelty", repo: "gortex", mode: "novelty", reads: 25, total: 24}, + {name: "threshold", repo: "gortex", threshold: 50, reads: 1, total: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := WithSessionID(context.Background(), backend+"-"+tc.name) + if tc.bound { + ss := s.sessionFor(ctx) + ss.scopeResolved, ss.scopeBound, ss.scopeWorkspaceID = true, true, tc.workspace + } + if tc.repo != "" { + ctx = withRepoAllow(ctx, map[string]bool{tc.repo: true}) + } + args := map[string]any{"kind": "hotspots", "format": "json", "max_bytes": float64(0), "mode": tc.mode, "threshold": tc.threshold} + req := makeReq("analyze", args) + entries := cached + if tc.threshold != 0 { + entries = analysis.FindHotspots(s.graph, s.getCommunities(), tc.threshold) + require.NotEmpty(t, entries) + } + if tc.mode != "" { + entries = rerankHotspots(entries, s.graph, tc.mode, "adds", 30) + } + // The base implementation hydrates every entry before filtering. + kept := entries + if s.scopeFiltersActive(ctx) { + kept = make([]analysis.HotspotEntry, 0, len(entries)) + for _, e := range entries { + if s.analyzeNodeVisible(ctx, s.graph.GetNode(e.ID)) { + kept = append(kept, e) + } + } + } + total := len(kept) + require.Equal(t, tc.total, total) + if total > 20 { + kept = kept[:20] + } + want, err := s.respondJSONOrTOON(ctx, req, map[string]any{"hotspots": kept, "total": total, "truncated": total > 20}) + require.NoError(t, err) + reader := &hotspotScopeReader{Reader: s.graph} + callCtx := withRequestView(ctx, &requestView{reader: reader}) + got, err := s.handleFindHotspots(callCtx, req) + require.NoError(t, err) + require.False(t, got.IsError, toolResultText(got)) + require.Equal(t, toolResultText(want), toolResultText(got), "complete response must match the base filter") + require.Len(t, reader.lookedUp, tc.reads) + for _, id := range reader.lookedUp { + if id == "gortex@feature::stdlib::net/http::Get" || id == "unknown/pkg/legacy.go::F" { + continue + } + require.True(t, s.analyzeNodeVisible(ctx, s.graph.GetNode(id)), "invisible source entry was hydrated: %s", id) + } + if tc.threshold == 0 && tc.name != "unbound" { + require.Contains(t, reader.lookedUp, "gortex@feature::stdlib::net/http::Get") + require.Contains(t, reader.lookedUp, "unknown/pkg/legacy.go::F") + } + }) + } + }) + } +} + +func TestHotspotsScopeSingleRepoKeepsUnprefixedReads(t *testing.T) { + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "0") + for _, backend := range []string{"memory", "sqlite"} { + t.Run(backend, func(t *testing.T) { + s, _ := newAnalyzeServer(t, true, analyzeRepoSpec{name: "solo", workspace: "solo", body: structuralBody("solo")}) + require.False(t, s.multiIndexer.IsMultiRepo()) + if backend == "sqlite" { + store, err := store_sqlite.Open(filepath.Join(t.TempDir(), "solo.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, store.Close()) }) + store.AddBatch(s.graph.AllNodes(), s.graph.AllEdges()) + s.graph = store + } + n := &graph.Node{ID: "internal/pkg.go::F", Kind: graph.KindFunction, WorkspaceID: "solo"} + s.graph.AddNode(n) + require.NotEqual(t, n.RepoPrefix, graph.RepoPrefixOfID(n.ID), "single-repo IDs do not encode repo ownership") + installCommunitiesForTest(s, &analysis.CommunityResult{}) + s.hotspots = []analysis.HotspotEntry{{ID: n.ID}} + s.hotspotsReady = true + ctx := outlineBoundContext(s, "solo", "solo", nil) + req := makeReq("analyze", map[string]any{"format": "json"}) + want, err := s.respondJSONOrTOON(ctx, req, map[string]any{"hotspots": s.hotspots, "total": 1, "truncated": false}) + require.NoError(t, err) + reader := &hotspotScopeReader{Reader: s.graph} + ctx = withRequestView(ctx, &requestView{reader: reader}) + result, err := s.handleFindHotspots(ctx, req) + require.NoError(t, err) + require.False(t, result.IsError) + require.Equal(t, toolResultText(want), toolResultText(result)) + require.Equal(t, []string{n.ID}, reader.lookedUp) + }) + } +} diff --git a/internal/mcp/tools_enhancements.go b/internal/mcp/tools_enhancements.go index 65077ef5f..bfaf2486e 100644 --- a/internal/mcp/tools_enhancements.go +++ b/internal/mcp/tools_enhancements.go @@ -2734,8 +2734,25 @@ func (s *Server) handleFindHotspots(ctx context.Context, req mcp.CallToolRequest // Strict no-op for an unbound session with no RepoAllow. if s.scopeFiltersActive(ctx) { reader := s.readerFor(ctx) + var excludedRepos map[string]bool + if s.multiIndexer != nil && s.multiIndexer.IsMultiRepo() { + workspaceRepos, bound := s.sessionWorkspaceRepoSet(ctx) + allow := repoAllowFromContext(ctx) + excludedRepos = make(map[string]bool) + for _, prefix := range s.multiIndexer.RepoPrefixes() { + if (bound && !workspaceRepos[prefix]) || (len(allow) > 0 && !allow[prefix]) { + excludedRepos[prefix] = true + } + } + } kept := make([]analysis.HotspotEntry, 0, len(entries)) for _, e := range entries { + // Only multi-repo source IDs encode ownership as prefix/path::symbol. + // Unprefixed, stub and unknown IDs still need the node visibility check. + path, symbol, sourceID := strings.Cut(e.ID, "::") + if sourceID && symbol != "" && excludedRepos[graph.RepoPrefixOfID(path)] { + continue + } if s.analyzeNodeVisible(ctx, reader.GetNode(e.ID)) { kept = append(kept, e) }