Replace test-only package globals with per-instance injection

Three package-level mutable var seams existed only so tests could swap
behavior, and reassigning a package global in a long-running MCP server
is not goroutine-safe:

- testBuildBatchReleaseHook and resolveBuildEdges (service) become
  GraphService fields (onBatchRelease, resolveEdges); edgeResolver()
  defaults to edgeresolve.ResolveWithOptions when unset.
- refreshSearchDocuments (mcp) becomes a Deps field; the handler method
  defaults to service.RefreshSearchDocuments when unset.

Tests now set the field on the instance instead of mutating a global.
The production var literally named testBuildBatchReleaseHook is gone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: tae2089

internal/mcp/deps.go

@@ -183,4 +183,8 @@ type Deps struct {
 
 	MaxFileBytes        int64
 	MaxTotalParsedBytes int64
+
+	// RefreshSearchDocuments overrides the search-document refresh used after a build; nil uses
+	// service.RefreshSearchDocuments. Kept as an injectable field so tests need no package globals.
+	RefreshSearchDocuments func(ctx context.Context, db *gorm.DB) (int, error)
 }

internal/mcp/handler_parse.go

@@ -18,7 +18,13 @@ import (
 	"github.com/tae2089/code-context-graph/internal/service"
 )
 
-var refreshSearchDocuments = service.RefreshSearchDocuments
+// @intent refresh search documents through the injected override, defaulting to the service impl.
+func (h *handlers) refreshSearchDocuments(ctx context.Context) (int, error) {
+	if h.deps.RefreshSearchDocuments != nil {
+		return h.deps.RefreshSearchDocuments(ctx, h.deps.DB)
+	}
+	return service.RefreshSearchDocuments(ctx, h.deps.DB)
+}
 
 // @intent serialize build_or_update_graph results with a fixed JSON schema without changing the wire format.
 type buildOrUpdateGraphResponse struct {
@@ -250,7 +256,7 @@ func (h *handlers) buildOrUpdateGraph(ctx context.Context, request mcp.CallToolR
 		}
 		// search rebuild
 		if h.deps.SearchBackend != nil && h.deps.DB != nil {
-			if _, err := refreshSearchDocuments(ctx, h.deps.DB); err != nil {
+			if _, err := h.refreshSearchDocuments(ctx); err != nil {
 				if failClosed {
 					failClosedErr = err
 					failedSteps = append(failedSteps, "search_documents")
@@ -274,7 +280,7 @@ func (h *handlers) buildOrUpdateGraph(ctx context.Context, request mcp.CallToolR
 		skippedSteps = appendUniqueStrings(skippedSteps, "communities", "flows")
 		// search only rebuild
 		if h.deps.SearchBackend != nil && h.deps.DB != nil {
-			if _, err := refreshSearchDocuments(ctx, h.deps.DB); err != nil {
+			if _, err := h.refreshSearchDocuments(ctx); err != nil {
 				if failClosed {
 					failClosedErr = err
 					failedSteps = append(failedSteps, "search_documents")
@@ -433,7 +439,7 @@ func (h *handlers) runPostprocess(ctx context.Context, request mcp.CallToolReque
 
 	if doFTS {
 		if h.deps.SearchBackend != nil && h.deps.DB != nil {
-			if _, err := refreshSearchDocuments(ctx, h.deps.DB); err != nil {
+			if _, err := h.refreshSearchDocuments(ctx); err != nil {
 				if failClosed {
 					failClosedErr = err
 					failedSteps = append(failedSteps, "search_documents")

internal/mcp/handlers_test.go

@@ -1404,9 +1404,7 @@ func TestBuildOrUpdateGraph_DegradedOnSearchDocumentRefreshFailure(t *testing.T)
 	deps := setupTestDeps(t)
 	backend := &failSearchBackend{}
 	deps.SearchBackend = backend
-	origRefresh := refreshSearchDocuments
-	defer func() { refreshSearchDocuments = origRefresh }()
-	refreshSearchDocuments = func(ctx context.Context, db *gorm.DB) (int, error) {
+	deps.RefreshSearchDocuments = func(ctx context.Context, db *gorm.DB) (int, error) {
 		return 0, errors.New("search document refresh boom")
 	}
 
@@ -1451,9 +1449,7 @@ func TestBuildOrUpdateGraph_MinimalSkipsFTSRebuildOnSearchDocumentRefreshFailure
 	deps := setupTestDeps(t)
 	backend := &failSearchBackend{}
 	deps.SearchBackend = backend
-	origRefresh := refreshSearchDocuments
-	defer func() { refreshSearchDocuments = origRefresh }()
-	refreshSearchDocuments = func(ctx context.Context, db *gorm.DB) (int, error) {
+	deps.RefreshSearchDocuments = func(ctx context.Context, db *gorm.DB) (int, error) {
 		return 0, errors.New("search document refresh boom")
 	}
 

internal/service/build.go

@@ -100,8 +100,8 @@ func (s *GraphService) bindAndReleaseNodeBatch(ctx context.Context, txStore stor
 
 	parsed.tsComments = nil
 	parsed.sourceLines = nil
-	if testBuildBatchReleaseHook != nil {
-		testBuildBatchReleaseHook(batches, idx)
+	if s.onBatchRelease != nil {
+		s.onBatchRelease(batches, idx)
 	}
 	return nil
 }
@@ -450,7 +450,7 @@ func (s *GraphService) flushBuildEdges(ctx context.Context, txStore store.GraphS
 			return err
 		}
 		end := min(start+buildEdgeResolveChunkSize, len(implementsEdges))
-		resolved, err := resolveBuildEdges(ctx, txStore, implementsEdges[start:end], resolveOptions)
+		resolved, err := s.edgeResolver()(ctx, txStore, implementsEdges[start:end], resolveOptions)
 		if err != nil {
 			s.logger().ErrorContext(ctx, "resolve deferred implements edges failed", append(obs.TraceLogArgs(ctx), "start", start, "end", end, "error", err)...)
 			return trace.Wrap(err, "resolve deferred implements edges")
@@ -474,7 +474,7 @@ func (s *GraphService) flushBuildEdges(ctx context.Context, txStore store.GraphS
 			end := min(start+buildEdgeResolveChunkSize, len(parsed.edges))
 			chunk := parsed.edges[start:end]
 			resolveInput := chunkWithImportWarmup(chunk, importsByPath[parsed.relPath])
-			resolved, err := resolveBuildEdges(ctx, txStore, resolveInput, resolveOptions)
+			resolved, err := s.edgeResolver()(ctx, txStore, resolveInput, resolveOptions)
 			if err != nil {
 				s.logger().ErrorContext(ctx, "resolve deferred edges failed", append(obs.TraceLogArgs(ctx), "file", parsed.relPath, "error", err)...)
 				return trace.Wrap(err, "resolve deferred edges for "+parsed.relPath)

internal/service/indexer.go

@@ -18,12 +18,7 @@ import (
 
 type languagePackageInfo = treesitter.PackageInfo
 
-var (
-	testBuildBatchReleaseHook func([]parsedBuildNodeBatch, int)
-	resolveBuildEdges         resolveBuildEdgesFn = edgeresolve.ResolveWithOptions
-)
-
-// @intent abstract build-time edge resolution so tests and build paths can swap resolver behavior without rewiring callers.
+// @intent abstract build-time edge resolution so tests can inject resolver behavior per GraphService.
 type resolveBuildEdgesFn func(ctx context.Context, lookup edgeresolve.NodeLookup, edges []model.Edge, options edgeresolve.ResolveOptions) ([]model.Edge, error)
 
 const (
@@ -89,6 +84,20 @@ type GraphService struct {
 	Walkers       map[string]*treesitter.Walker
 	Parsers       map[string]Parser
 	Logger        *slog.Logger
+
+	// resolveEdges overrides build-time edge resolution; nil uses edgeresolve.ResolveWithOptions.
+	// onBatchRelease, when set, is notified after each node batch is persisted and its buffers
+	// are released. Both are test seams kept per-instance so the server has no mutable globals.
+	resolveEdges   resolveBuildEdgesFn
+	onBatchRelease func([]parsedBuildNodeBatch, int)
+}
+
+// @intent resolve build edges through the injected resolver, defaulting to the production resolver.
+func (s *GraphService) edgeResolver() resolveBuildEdgesFn {
+	if s.resolveEdges != nil {
+		return s.resolveEdges
+	}
+	return edgeresolve.ResolveWithOptions
 }
 
 // BuildOptions configures one graph build run.

internal/service/indexer_test.go

@@ -498,22 +498,20 @@ func TestFlushBuildEdges_ResolvesAndUpsertsBoundedBatches(t *testing.T) {
 	}
 
 	var resolveSizes []int
-	oldResolve := resolveBuildEdges
-	resolveBuildEdges = func(ctx context.Context, lookup edgeresolve.NodeLookup, edges []model.Edge, options edgeresolve.ResolveOptions) ([]model.Edge, error) {
+	resolver := func(ctx context.Context, lookup edgeresolve.NodeLookup, edges []model.Edge, options edgeresolve.ResolveOptions) ([]model.Edge, error) {
 		resolveSizes = append(resolveSizes, len(edges))
 		if len(edges) > buildEdgeResolveChunkSize {
 			t.Fatalf("resolve batch exceeded limit: got %d want <= %d", len(edges), buildEdgeResolveChunkSize)
 		}
-		return oldResolve(ctx, lookup, edges, options)
+		return edgeresolve.ResolveWithOptions(ctx, lookup, edges, options)
 	}
-	t.Cleanup(func() { resolveBuildEdges = oldResolve })
 
 	batches := []parsedBuildEdgeBatch{
 		{relPath: "cmd/main.go", edges: []model.Edge{{Kind: model.EdgeKindImportsFrom, FilePath: "cmd/main.go", Line: 1, Fingerprint: "imports_from:cmd/main.go:github.com/example/project/mcp:1"}, {Kind: model.EdgeKindCalls, FilePath: "cmd/main.go", Line: 2, Fingerprint: "calls:cmd/main.go:h.deps.FlowTracer.TraceFlow:2"}}},
 		{relPath: "flows/tracer.go", edges: []model.Edge{{Kind: model.EdgeKindImplements, FilePath: "flows/tracer.go", Line: 7, Fingerprint: "implements:flows/tracer.go:flows.Tracer:mcp.FlowTracer"}}},
 	}
 
-	svc := &GraphService{}
+	svc := &GraphService{resolveEdges: resolver}
 	if err := svc.flushBuildEdges(ctx, st, batches, nil, edgeresolve.ResolveOptions{}); err != nil {
 		t.Fatalf("flushBuildEdges: %v", err)
 	}
@@ -543,26 +541,24 @@ func TestFlushBuildEdges_ResolvesImplementsOnlyOnce(t *testing.T) {
 	st.nodesByFP["flows/tracer.go"] = []model.Node{{ID: 30, QualifiedName: "flows/tracer.go", Name: "flows/tracer.go", Kind: model.NodeKindFile, FilePath: "flows/tracer.go", Language: "go"}, {ID: 3, QualifiedName: "flows.Tracer", Name: "Tracer", Kind: model.NodeKindClass, FilePath: "flows/tracer.go", StartLine: 7, EndLine: 7, Language: "go"}, {ID: 4, QualifiedName: "flows.Tracer.TraceFlow", Name: "TraceFlow", Kind: model.NodeKindFunction, FilePath: "flows/tracer.go", StartLine: 9, EndLine: 11, Language: "go"}}
 
 	var implementsSeen []int
-	oldResolve := resolveBuildEdges
-	resolveBuildEdges = func(ctx context.Context, lookup edgeresolve.NodeLookup, edges []model.Edge, options edgeresolve.ResolveOptions) ([]model.Edge, error) {
+	resolver := func(ctx context.Context, lookup edgeresolve.NodeLookup, edges []model.Edge, options edgeresolve.ResolveOptions) ([]model.Edge, error) {
 		count := 0
 		for _, edge := range edges {
 			if edge.Kind == model.EdgeKindImplements {
 				count++
 			}
 		}
 		implementsSeen = append(implementsSeen, count)
-		return oldResolve(ctx, lookup, edges, options)
+		return edgeresolve.ResolveWithOptions(ctx, lookup, edges, options)
 	}
-	t.Cleanup(func() { resolveBuildEdges = oldResolve })
 
 	batches := []parsedBuildEdgeBatch{
 		{relPath: "flows/tracer.go", edges: []model.Edge{{Kind: model.EdgeKindImplements, FilePath: "flows/tracer.go", Line: 7, Fingerprint: "implements:flows/tracer.go:flows.Tracer:mcp.FlowTracer"}}},
 		{relPath: "cmd/main.go", edges: []model.Edge{{Kind: model.EdgeKindImportsFrom, FilePath: "cmd/main.go", Line: 1, Fingerprint: "imports_from:cmd/main.go:github.com/example/project/mcp:1"}, {Kind: model.EdgeKindCalls, FilePath: "cmd/main.go", Line: 2, Fingerprint: "calls:cmd/main.go:h.deps.FlowTracer.TraceFlow:2"}}},
 		{relPath: "cmd/main.go", edges: []model.Edge{{Kind: model.EdgeKindContains, FilePath: "cmd/main.go", Line: 1, Fingerprint: "contains:cmd/main.go:main.Run"}}},
 	}
 
-	svc := &GraphService{}
+	svc := &GraphService{resolveEdges: resolver}
 	if err := svc.flushBuildEdges(ctx, st, batches, nil, edgeresolve.ResolveOptions{}); err != nil {
 		t.Fatalf("flushBuildEdges: %v", err)
 	}
@@ -2100,8 +2096,7 @@ func TestBuild_ReleasesBatchCommentStateAfterBinding(t *testing.T) {
 		tsCommentsNil bool
 		sourceNil     bool
 	}
-	prevHook := testBuildBatchReleaseHook
-	testBuildBatchReleaseHook = func(batches []parsedBuildNodeBatch, idx int) {
+	recordRelease := func(batches []parsedBuildNodeBatch, idx int) {
 		snapshots = append(snapshots, struct {
 			batch         int
 			tsCommentsNil bool
@@ -2112,15 +2107,15 @@ func TestBuild_ReleasesBatchCommentStateAfterBinding(t *testing.T) {
 			sourceNil:     batches[idx].sourceLines == nil,
 		})
 	}
-	defer func() { testBuildBatchReleaseHook = prevHook }()
 
 	fakeStore := newRecordingGraphStore(t)
 	svc := &GraphService{
 		Store: fakeStore,
 		Walkers: map[string]*treesitter.Walker{
 			".go": treesitter.NewWalker(treesitter.GoSpec),
 		},
-		Logger: slog.Default(),
+		Logger:         slog.Default(),
+		onBatchRelease: recordRelease,
 	}
 
 	dir := t.TempDir()