From cf3ea045e99e3d4ea7310f983f60844c46414048 Mon Sep 17 00:00:00 2001 From: Rene Leonhardt <65483435+reneleonhardt@users.noreply.github.com> Date: Sun, 2 Aug 2026 08:46:10 +0200 Subject: [PATCH] fix(mcp): Bound cancellable project scans Propagate cancellation through scanner, Git, handoff, and MCP traversal paths. Bound project and manifest discovery without changing legacy fallback behavior. Signed-off-by: GPT-5.6 Sol Co-Authored-By: GPT-5.6 Sol --- docs/MCP.md | 6 + handoff/build.go | 153 ++++++++--- handoff/context_test.go | 179 +++++++++++++ handoff/detail.go | 50 +++- mcp/cancellation_test.go | 496 +++++++++++++++++++++++++++++++++++ mcp/main.go | 334 ++++++++++++++++++----- scanner/astgrep.go | 18 +- scanner/cancellation_test.go | 253 ++++++++++++++++++ scanner/deps.go | 128 +++++++-- scanner/filegraph.go | 75 +++++- scanner/git.go | 59 ++++- scanner/walker.go | 62 ++++- 12 files changed, 1666 insertions(+), 147 deletions(-) create mode 100644 handoff/context_test.go create mode 100644 mcp/cancellation_test.go create mode 100644 scanner/cancellation_test.go diff --git a/docs/MCP.md b/docs/MCP.md index e2fcc66..13d27b9 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -138,6 +138,12 @@ Surface behavior note: Output and budget notes: - text responses are byte-budgeted and line-truncated to protect context +- project discovery reads bounded batches, returns at most 50 sorted projects, examines at most 200 parent entries, and reports truncation separately from cancellation; when the examination cap is exceeded it returns only the truncation notice rather than a filesystem-order-dependent partial list +- MCP dependency discovery skips individual manifests larger than 1 MiB; CLI dependency and blast-radius callers retain their legacy unbounded manifest behavior - prefix file counts and size-based handoff budgets honor the active `.codemap/config.json` filters - handoff payload includes deterministic hashes (`prefix_hash`, `delta_hash`, `combined_hash`) - handoff payload includes cache metrics (`reuse_ratio`, `unchanged_bytes`, etc.) + +Cancellation notes: +- project scans, dependency graphs, ast-grep, Git diff/impact work, project discovery, and generated handoff/detail work stop with the MCP request context +- with `save=true`, cancellation observed after generation but before persistence prevents `WriteLatest`; once persistence begins, the existing multi-file write completes or reports its existing error without rollback diff --git a/handoff/build.go b/handoff/build.go index 4c4e7cc..34014d2 100644 --- a/handoff/build.go +++ b/handoff/build.go @@ -2,6 +2,7 @@ package handoff import ( "bytes" + "context" "crypto/sha256" "encoding/hex" "encoding/json" @@ -58,6 +59,15 @@ func normalizeOptions(opts BuildOptions, fileCount int) BuildOptions { // Build creates a multi-agent handoff artifact from git + daemon state. func Build(root string, opts BuildOptions) (*Artifact, error) { + return BuildContext(context.Background(), root, opts) +} + +// BuildContext creates a handoff artifact while honoring caller cancellation +// across filesystem scans, dependency analysis, and Git subprocesses. +func BuildContext(ctx context.Context, root string, opts BuildOptions) (*Artifact, error) { + if err := ctx.Err(); err != nil { + return nil, err + } absRoot, err := filepath.Abs(root) if err != nil { return nil, err @@ -67,16 +77,22 @@ func Build(root string, opts BuildOptions) (*Artifact, error) { if state == nil { state = watch.ReadState(absRoot) } + if err := ctx.Err(); err != nil { + return nil, err + } - fileCount := resolveRepoFileCount(absRoot) + fileCount, err := resolveRepoFileCountContext(ctx, absRoot) + if err != nil { + return nil, err + } opts = normalizeOptions(opts, fileCount) - branch, err := gitCurrentBranch(absRoot) + branch, err := gitCurrentBranchContext(ctx, absRoot) if err != nil { return nil, fmt.Errorf("failed to read git branch: %w", err) } - entries, diffErr := collectChangedEntries(absRoot, opts.BaseRef) + entries, diffErr := collectChangedEntriesContext(ctx, absRoot, opts.BaseRef) if diffErr != nil { return nil, diffErr } @@ -92,12 +108,18 @@ func Build(root string, opts BuildOptions) (*Artifact, error) { } } - importers := dependencyImportersForHandoff(absRoot, state, fileCount) + importers, err := dependencyImportersForHandoffContext(ctx, absRoot, state, fileCount) + if err != nil { + return nil, err + } riskFiles := summarizeRiskFiles(changedAll, importers, opts.MaxRisk) selectedPaths := prioritizeChangedPaths(changedAll, riskFiles, opts.MaxChanged) entries = selectEntries(entries, selectedPaths) - changedStubs := buildFileStubs(absRoot, entries) + changedStubs, err := buildFileStubsContext(ctx, absRoot, entries) + if err != nil { + return nil, err + } hubs := summarizeHubs(importers, opts.MaxHubs) nextSteps, openQuestions := deriveGuidance(selectedPaths, riskFiles, recentEvents, opts.BaseRef, state != nil, len(importers) > 0) @@ -128,6 +150,9 @@ func Build(root string, opts BuildOptions) (*Artifact, error) { if previous == nil { previous, _ = ReadLatest(absRoot) } + if err := ctx.Err(); err != nil { + return nil, err + } metrics := buildCacheMetrics(previous, prefixHash, deltaHash, prefixBytes, deltaBytes) generatedAt := time.Now() if previous != nil && previous.PrefixHash == prefixHash && previous.DeltaHash == deltaHash && !previous.GeneratedAt.IsZero() { @@ -157,25 +182,37 @@ func Build(root string, opts BuildOptions) (*Artifact, error) { }, nil } -func collectChangedEntries(root, baseRef string) ([]changedEntry, error) { +func collectChangedEntriesContext(ctx context.Context, root, baseRef string) ([]changedEntry, error) { changed := make(map[string]changedEntry) - branchLines, branchErr := runGitLines(root, "diff", "--name-only", baseRef+"...HEAD") + branchLines, branchErr := runGitLinesContext(ctx, root, "diff", "--name-only", baseRef+"...HEAD") + if err := ctx.Err(); err != nil { + return nil, err + } for _, line := range branchLines { addChangedEntry(changed, root, line, "branch") } - workingLines, _ := runGitLines(root, "diff", "--name-only") + workingLines, _ := runGitLinesContext(ctx, root, "diff", "--name-only") + if err := ctx.Err(); err != nil { + return nil, err + } for _, line := range workingLines { addChangedEntry(changed, root, line, "modified") } - stagedLines, _ := runGitLines(root, "diff", "--name-only", "--cached") + stagedLines, _ := runGitLinesContext(ctx, root, "diff", "--name-only", "--cached") + if err := ctx.Err(); err != nil { + return nil, err + } for _, line := range stagedLines { addChangedEntry(changed, root, line, "staged") } - untrackedLines, _ := runGitLines(root, "ls-files", "--others", "--exclude-standard") + untrackedLines, _ := runGitLinesContext(ctx, root, "ls-files", "--others", "--exclude-standard") + if err := ctx.Err(); err != nil { + return nil, err + } for _, line := range untrackedLines { addChangedEntry(changed, root, line, "untracked") } @@ -254,13 +291,16 @@ func isLikelyBinary(root, relPath string) bool { return bytes.IndexByte(buf[:n], 0) >= 0 } -func buildFileStubs(root string, changed []changedEntry) []FileStub { +func buildFileStubsContext(ctx context.Context, root string, changed []changedEntry) ([]FileStub, error) { if len(changed) == 0 { - return []FileStub{} + return []FileStub{}, ctx.Err() } stubs := make([]FileStub, 0, len(changed)) for _, entry := range changed { + if err := ctx.Err(); err != nil { + return nil, err + } stub := FileStub{ Path: entry.Path, Status: entry.Status, @@ -270,25 +310,49 @@ func buildFileStubs(root string, changed []changedEntry) []FileStub { info, err := os.Stat(absPath) if err == nil && !info.IsDir() { stub.Size = info.Size() - stub.Hash = fileSHA256(absPath) + stub.Hash, err = fileSHA256Context(ctx, absPath) + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + if err != nil { + stub.Hash = "" + } } stubs = append(stubs, stub) } - return stubs + return stubs, ctx.Err() } -func fileSHA256(path string) string { +func fileSHA256Context(ctx context.Context, path string) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } f, err := os.Open(path) if err != nil { - return "" + return "", err } defer f.Close() h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return "" + buf := make([]byte, 32*1024) + for { + if err := ctx.Err(); err != nil { + return "", err + } + n, readErr := f.Read(buf) + if n > 0 { + if _, err := h.Write(buf[:n]); err != nil { + return "", err + } + } + if readErr == io.EOF { + break + } + if readErr != nil { + return "", readErr + } } - return hex.EncodeToString(h.Sum(nil)) + return hex.EncodeToString(h.Sum(nil)), ctx.Err() } func summarizeHubs(importersByFile map[string][]string, maxHubs int) []HubSummary { @@ -562,11 +626,18 @@ func buildCacheMetrics(previous *Artifact, prefixHash, deltaHash string, prefixB return metrics } -func runGitLines(root string, args ...string) ([]string, error) { - cmd := exec.Command("git", args...) +func runGitLinesContext(ctx context.Context, root string, args ...string) ([]string, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + cmd := exec.CommandContext(ctx, "git", args...) + cmd.WaitDelay = 100 * time.Millisecond cmd.Dir = root out, err := cmd.Output() if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } return nil, err } @@ -585,40 +656,56 @@ func runGitLines(root string, args ...string) ([]string, error) { return lines, nil } -func gitCurrentBranch(root string) (string, error) { - cmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD") +func gitCurrentBranchContext(ctx context.Context, root string) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } + cmd := exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "HEAD") + cmd.WaitDelay = 100 * time.Millisecond cmd.Dir = root out, err := cmd.Output() if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } return "", err } return strings.TrimSpace(string(out)), nil } -func resolveRepoFileCount(root string) int { +func resolveRepoFileCountContext(ctx context.Context, root string) (int, error) { gitCache := scanner.NewGitIgnoreCache(root) - files, err := scanner.ScanConfiguredFiles(root, gitCache) + files, err := scanner.ScanConfiguredFilesContext(ctx, root, gitCache) if err != nil { - return 0 + if ctxErr := ctx.Err(); ctxErr != nil { + return 0, ctxErr + } + return 0, nil } - return len(files) + return len(files), ctx.Err() } -func dependencyImportersForHandoff(root string, state *watch.State, fileCount int) map[string][]string { +func dependencyImportersForHandoffContext(ctx context.Context, root string, state *watch.State, fileCount int) (map[string][]string, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if state != nil && len(state.Importers) > 0 { - return state.Importers + return state.Importers, nil } // Skip fallback graph construction for configured large repositories. if fileCount > limits.LargeRepoFileCount { - return nil + return nil, nil } - fg, err := scanner.BuildFileGraph(root) + fg, err := scanner.BuildFileGraphContext(ctx, root) if err != nil { - return nil + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, nil } - return fg.Importers + return fg.Importers, ctx.Err() } func nonNilStrings(items []string) []string { diff --git a/handoff/context_test.go b/handoff/context_test.go new file mode 100644 index 0000000..bd78257 --- /dev/null +++ b/handoff/context_test.go @@ -0,0 +1,179 @@ +package handoff + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "codemap/scanner" +) + +type cancelAfterHandoffChecks struct { + context.Context + remaining int +} + +func (c *cancelAfterHandoffChecks) Err() error { + if c.remaining <= 0 { + return context.Canceled + } + c.remaining-- + return nil +} + +func TestBuildContextPreCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := BuildContext(ctx, t.TempDir(), BuildOptions{}); !errors.Is(err, context.Canceled) { + t.Fatalf("BuildContext error = %v, want context.Canceled", err) + } +} + +func TestBuildContextTerminatesGitSubprocess(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("requires shell script execution") + } + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + binDir := t.TempDir() + marker := filepath.Join(t.TempDir(), "started") + fakeGit := filepath.Join(binDir, "git") + script := `#!/bin/sh +if [ "$1" = "rev-parse" ]; then + echo feature/test + exit 0 +fi +printf '%s\n' "$$" > "$CODEMAP_HANDOFF_GIT_MARKER" +exec sleep 10 +` + if err := os.WriteFile(fakeGit, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("CODEMAP_HANDOFF_GIT_MARKER", marker) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + _, err := BuildContext(ctx, root, BuildOptions{BaseRef: "main"}) + done <- err + }() + var blockerPID string + deadline := time.Now().Add(5 * time.Second) + for { + if data, err := os.ReadFile(marker); err == nil && len(data) > 0 { + blockerPID = strings.TrimSpace(string(data)) + break + } + select { + case err := <-done: + t.Fatalf("BuildContext exited before fake handoff git subprocess started: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("fake handoff git subprocess did not start") + } + time.Sleep(5 * time.Millisecond) + } + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Fatalf("BuildContext error = %v, want context.Canceled", err) + } + case <-time.After(2 * time.Second): + t.Fatal("BuildContext did not terminate cancelled git subprocess") + } + if err := exec.Command("/bin/kill", "-0", blockerPID).Run(); err == nil { + t.Fatalf("cancelled fake handoff git left blocker process %s running", blockerPID) + } +} + +func TestBuildFileDetailContextPreCancellation(t *testing.T) { + artifact := &Artifact{Delta: DeltaSnapshot{Changed: []FileStub{{Path: "main.go"}}}} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := BuildFileDetailContext(ctx, t.TempDir(), artifact, "main.go", nil); !errors.Is(err, context.Canceled) { + t.Fatalf("BuildFileDetailContext error = %v, want context.Canceled", err) + } + + inFlight := &cancelAfterHandoffChecks{Context: context.Background(), remaining: 1} + if _, err := BuildFileDetailContext(inFlight, t.TempDir(), artifact, "main.go", nil); !errors.Is(err, context.Canceled) { + t.Fatalf("in-flight BuildFileDetailContext error = %v, want context.Canceled", err) + } +} + +func TestFileSHA256ContextInFlightCancellation(t *testing.T) { + path := filepath.Join(t.TempDir(), "large.go") + if err := os.WriteFile(path, make([]byte, 2<<20), 0o644); err != nil { + t.Fatal(err) + } + ctx := &cancelAfterHandoffChecks{Context: context.Background(), remaining: 1} + if _, err := fileSHA256Context(ctx, path); !errors.Is(err, context.Canceled) { + t.Fatalf("fileSHA256Context error = %v, want context.Canceled", err) + } +} + +func TestBuildAndFileDetailPreserveBestEffortScanFailures(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission fixture and shell script are Unix-specific") + } + + root := t.TempDir() + blocked := filepath.Join(root, "blocked") + if err := os.MkdirAll(blocked, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(blocked, "main.go"), []byte("package blocked\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(blocked, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(blocked, 0o755) }) + if _, err := scanner.ScanConfiguredFilesContext(context.Background(), root, scanner.NewGitIgnoreCache(root)); err == nil { + t.Skip("fixture did not produce a scanner failure") + } + + binDir := t.TempDir() + fakeGit := filepath.Join(binDir, "git") + script := `#!/bin/sh +if [ "$1" = "rev-parse" ]; then + echo feature/test +fi +exit 0 +` + if err := os.WriteFile(fakeGit, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", binDir) + + t.Run("Build", func(t *testing.T) { + artifact, err := BuildContext(context.Background(), root, BuildOptions{BaseRef: "HEAD"}) + if err != nil { + t.Fatalf("ordinary scanner failure escaped BuildContext: %v", err) + } + if artifact == nil || artifact.Prefix.FileCount != 0 { + t.Fatalf("BuildContext artifact = %#v, want zero-count best-effort artifact", artifact) + } + }) + + t.Run("BuildFileDetail", func(t *testing.T) { + artifact := &Artifact{Delta: DeltaSnapshot{Changed: []FileStub{{Path: "blocked/main.go"}}}} + detail, err := BuildFileDetailContext(context.Background(), root, artifact, "blocked/main.go", nil) + if err != nil { + t.Fatalf("ordinary scanner failure escaped BuildFileDetailContext: %v", err) + } + if detail == nil || len(detail.Importers) != 0 || len(detail.Imports) != 0 { + t.Fatalf("BuildFileDetailContext detail = %#v, want empty best-effort dependency context", detail) + } + }) +} diff --git a/handoff/detail.go b/handoff/detail.go index f80af0e..ca3fd0f 100644 --- a/handoff/detail.go +++ b/handoff/detail.go @@ -1,6 +1,7 @@ package handoff import ( + "context" "fmt" "path/filepath" "sort" @@ -13,6 +14,14 @@ import ( // BuildFileDetail resolves detailed context for one changed file stub. func BuildFileDetail(root string, artifact *Artifact, targetPath string, state *watch.State) (*FileDetail, error) { + return BuildFileDetailContext(context.Background(), root, artifact, targetPath, state) +} + +// BuildFileDetailContext resolves detailed context while honoring caller cancellation. +func BuildFileDetailContext(ctx context.Context, root string, artifact *Artifact, targetPath string, state *watch.State) (*FileDetail, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if artifact == nil { return nil, fmt.Errorf("handoff artifact is nil") } @@ -25,6 +34,9 @@ func BuildFileDetail(root string, artifact *Artifact, targetPath string, state * var selected *FileStub for i := range artifact.Delta.Changed { + if err := ctx.Err(); err != nil { + return nil, err + } if artifact.Delta.Changed[i].Path == target { selected = &artifact.Delta.Changed[i] break @@ -41,13 +53,22 @@ func BuildFileDetail(root string, artifact *Artifact, targetPath string, state * if state == nil { state = watch.ReadState(absRoot) } + if err := ctx.Err(); err != nil { + return nil, err + } - importers, imports := dependencyContextForFile(absRoot, state, target) + importers, imports, err := dependencyContextForFileContext(ctx, absRoot, state, target) + if err != nil { + return nil, err + } importers = uniqueSorted(importers) imports = uniqueSorted(imports) events := make([]EventSummary, 0, len(artifact.Delta.RecentEvents)) for _, event := range artifact.Delta.RecentEvents { + if err := ctx.Err(); err != nil { + return nil, err + } if event.Path == target { events = append(events, event) } @@ -66,19 +87,34 @@ func BuildFileDetail(root string, artifact *Artifact, targetPath string, state * } func dependencyContextForFile(root string, state *watch.State, path string) ([]string, []string) { + importers, imports, _ := dependencyContextForFileContext(context.Background(), root, state, path) + return importers, imports +} + +func dependencyContextForFileContext(ctx context.Context, root string, state *watch.State, path string) ([]string, []string, error) { + if err := ctx.Err(); err != nil { + return nil, nil, err + } if state != nil && (len(state.Importers) > 0 || len(state.Imports) > 0) { - return append([]string{}, state.Importers[path]...), append([]string{}, state.Imports[path]...) + return append([]string{}, state.Importers[path]...), append([]string{}, state.Imports[path]...), nil } - if resolveRepoFileCount(root) > limits.LargeRepoFileCount { - return nil, nil + fileCount, err := resolveRepoFileCountContext(ctx, root) + if err != nil { + return nil, nil, err + } + if fileCount > limits.LargeRepoFileCount { + return nil, nil, nil } - fg, err := scanner.BuildFileGraph(root) + fg, err := scanner.BuildFileGraphContext(ctx, root) if err != nil { - return nil, nil + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, nil, ctxErr + } + return nil, nil, nil } - return append([]string{}, fg.Importers[path]...), append([]string{}, fg.Imports[path]...) + return append([]string{}, fg.Importers[path]...), append([]string{}, fg.Imports[path]...), ctx.Err() } func uniqueSorted(items []string) []string { diff --git a/mcp/cancellation_test.go b/mcp/cancellation_test.go new file mode 100644 index 0000000..3182b6f --- /dev/null +++ b/mcp/cancellation_test.go @@ -0,0 +1,496 @@ +package codemapmcp + +import ( + "context" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "codemap/handoff" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +type cancelAfterMCPChecks struct { + context.Context + remaining int +} + +type scriptedProjectDirReader struct { + entries []os.DirEntry + offset int +} + +func (r *scriptedProjectDirReader) ReadDir(n int) ([]os.DirEntry, error) { + if r.offset >= len(r.entries) { + return nil, io.EOF + } + end := min(r.offset+n, len(r.entries)) + batch := r.entries[r.offset:end] + r.offset = end + return batch, nil +} + +type namedProjectDirEntry string + +func (e namedProjectDirEntry) Name() string { return string(e) } +func (e namedProjectDirEntry) IsDir() bool { return true } +func (e namedProjectDirEntry) Type() fs.FileMode { return fs.ModeDir } +func (e namedProjectDirEntry) Info() (fs.FileInfo, error) { return nil, errors.New("unused") } + +func (c *cancelAfterMCPChecks) Err() error { + if c.remaining <= 0 { + return context.Canceled + } + c.remaining-- + return nil +} + +func TestMCPTraversalHandlersReportPreCancellation(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + call func(context.Context) (bool, string, error) + }{ + {"structure", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetStructure(ctx, nil, StructureInput{Path: root}) + return resultState(t, r, e) + }}, + {"dependencies", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetDependencies(ctx, nil, PathInput{Path: root}) + return resultState(t, r, e) + }}, + {"diff", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetDiff(ctx, nil, DiffInput{Path: root, Ref: "HEAD"}) + return resultState(t, r, e) + }}, + {"find", func(ctx context.Context) (bool, string, error) { + r, _, e := handleFindFile(ctx, nil, FindInput{Path: root, Pattern: "main"}) + return resultState(t, r, e) + }}, + {"importers", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetImporters(ctx, nil, ImportersInput{Path: root, File: "main.go"}) + return resultState(t, r, e) + }}, + {"hubs", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetHubs(ctx, nil, PathInput{Path: root}) + return resultState(t, r, e) + }}, + {"file context", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetFileContext(ctx, nil, ImportersInput{Path: root, File: "main.go"}) + return resultState(t, r, e) + }}, + {"handoff", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetHandoff(ctx, nil, HandoffInput{Path: root}) + return resultState(t, r, e) + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + isError, text, err := tt.call(ctx) + if err != nil { + t.Fatalf("handler returned transport error instead of explicit result: %v", err) + } + if !isError || !strings.Contains(strings.ToLower(text), "cancel") { + t.Fatalf("cancelled handler result: IsError=%v text=%q", isError, text) + } + }) + } +} + +func TestMCPTraversalHandlersReportInFlightCancellation(t *testing.T) { + root := t.TempDir() + for i := 0; i < 20; i++ { + if err := os.WriteFile(filepath.Join(root, fmt.Sprintf("file-%02d.go", i)), []byte("package fixture\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + tests := []struct { + name string + call func(context.Context) (bool, string, error) + }{ + {"structure", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetStructure(ctx, nil, StructureInput{Path: root}) + return resultState(t, r, e) + }}, + {"dependencies", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetDependencies(ctx, nil, PathInput{Path: root}) + return resultState(t, r, e) + }}, + {"diff", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetDiff(ctx, nil, DiffInput{Path: root, Ref: "HEAD"}) + return resultState(t, r, e) + }}, + {"find", func(ctx context.Context) (bool, string, error) { + r, _, e := handleFindFile(ctx, nil, FindInput{Path: root, Pattern: "file"}) + return resultState(t, r, e) + }}, + {"importers", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetImporters(ctx, nil, ImportersInput{Path: root, File: "file-00.go"}) + return resultState(t, r, e) + }}, + {"hubs", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetHubs(ctx, nil, PathInput{Path: root}) + return resultState(t, r, e) + }}, + {"file context", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetFileContext(ctx, nil, ImportersInput{Path: root, File: "file-00.go"}) + return resultState(t, r, e) + }}, + {"handoff", func(ctx context.Context) (bool, string, error) { + r, _, e := handleGetHandoff(ctx, nil, HandoffInput{Path: root}) + return resultState(t, r, e) + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := &cancelAfterMCPChecks{Context: context.Background(), remaining: 1} + isError, text, err := tt.call(ctx) + if err != nil { + t.Fatalf("handler returned transport error instead of explicit result: %v", err) + } + if !isError || !strings.Contains(strings.ToLower(text), "cancel") { + t.Fatalf("cancelled handler result: IsError=%v text=%q", isError, text) + } + }) + } +} + +func resultState(t *testing.T, result interface{}, err error) (bool, string, error) { + t.Helper() + if err != nil { + return false, "", err + } + if result == nil { + return false, "", errors.New("nil MCP result") + } + r, ok := result.(*mcp.CallToolResult) + if !ok { + return false, "", fmt.Errorf("unexpected MCP result type %T", result) + } + return r.IsError, resultText(t, r), nil +} + +func TestListProjectsUsesSeparateExaminedAndResultCaps(t *testing.T) { + t.Run("exact result cap is complete", func(t *testing.T) { + parent := t.TempDir() + createProjectDirs(t, parent, 50, "project") + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: parent}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + if result.IsError || strings.Contains(out, "truncated") || strings.Count(out, "project-") != 50 { + t.Fatalf("exact-cap result: IsError=%v count=%d output=%q", result.IsError, strings.Count(out, "project-"), out) + } + }) + + t.Run("over result cap truncates", func(t *testing.T) { + parent := t.TempDir() + createProjectDirs(t, parent, 51, "project") + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: parent}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + if result.IsError || strings.Count(out, "project-") != 50 || !strings.Contains(out, "truncated") { + t.Fatalf("over-cap result: IsError=%v count=%d output=%q", result.IsError, strings.Count(out, "project-"), out) + } + }) + + t.Run("examined cap bounds nonmatches", func(t *testing.T) { + parent := t.TempDir() + createProjectDirs(t, parent, 240, "nonmatch") + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: parent, Pattern: "wanted"}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + want := fmt.Sprintf("Project discovery matching 'wanted' in %s was truncated after examining 200 entries; results are unavailable because the directory exceeds the examination cap.", parent) + if result.IsError || out != want { + t.Fatalf("patterned examined-cap result: IsError=%v output=%q, want %q", result.IsError, out, want) + } + if strings.Contains(out, "No projects matching") { + t.Fatalf("patterned examined-cap result made a false exhaustive claim: %q", out) + } + }) + + t.Run("examined cap does not claim parent has no projects", func(t *testing.T) { + parent := t.TempDir() + createProjectDirs(t, parent, 201, "project") + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: parent}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + want := fmt.Sprintf("Project discovery in %s was truncated after examining 200 entries; results are unavailable because the directory exceeds the examination cap.", parent) + if result.IsError || out != want { + t.Fatalf("unpatterned examined-cap result: IsError=%v output=%q, want %q", result.IsError, out, want) + } + if strings.Contains(out, "No project directories found") { + t.Fatalf("unpatterned examined-cap result made a false exhaustive claim: %q", out) + } + }) +} + +func TestSelectProjectCandidatesSortsAcrossBatchesAndCapsDeterministically(t *testing.T) { + makeEntries := func(count int) []os.DirEntry { + entries := make([]os.DirEntry, 0, count) + for i := count - 1; i >= 0; i-- { + entries = append(entries, namedProjectDirEntry(fmt.Sprintf("project-%03d", i))) + } + return entries + } + + tests := []struct { + name string + count int + wantCount int + wantExamined int + wantResultCap bool + wantExaminationCap bool + }{ + {name: "exact result cap", count: 50, wantCount: 50, wantExamined: 50}, + {name: "over result cap", count: 51, wantCount: 50, wantExamined: 51, wantResultCap: true}, + {name: "over examination cap", count: 201, wantCount: 0, wantExamined: 200, wantExaminationCap: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := &scriptedProjectDirReader{entries: makeEntries(tt.count)} + candidates, examined, byResults, byExamined, err := selectProjectCandidates(context.Background(), reader, "") + if err != nil { + t.Fatal(err) + } + if len(candidates) != tt.wantCount || examined != tt.wantExamined || byResults != tt.wantResultCap || byExamined != tt.wantExaminationCap { + t.Fatalf("selection = count %d, examined %d, result cap %v, examination cap %v", len(candidates), examined, byResults, byExamined) + } + for i, candidate := range candidates { + want := fmt.Sprintf("project-%03d", tt.count-tt.wantExamined+i) + if candidate != want { + t.Fatalf("candidate[%d] = %q, want %q; selection was not sorted across batches", i, candidate, want) + } + } + }) + } +} + +func TestSelectProjectCandidatesAboveExamCapIsIndependentOfEnumerationOrder(t *testing.T) { + forward := make([]os.DirEntry, 0, maxExaminedProjectEntries+1) + reverse := make([]os.DirEntry, 0, maxExaminedProjectEntries+1) + for i := 0; i <= maxExaminedProjectEntries; i++ { + forward = append(forward, namedProjectDirEntry(fmt.Sprintf("project-%03d", i))) + } + for i := maxExaminedProjectEntries; i >= 0; i-- { + reverse = append(reverse, namedProjectDirEntry(fmt.Sprintf("project-%03d", i))) + } + + for name, entries := range map[string][]os.DirEntry{"forward": forward, "reverse": reverse} { + t.Run(name, func(t *testing.T) { + candidates, examined, byResults, byExamined, err := selectProjectCandidates(context.Background(), &scriptedProjectDirReader{entries: entries}, "") + if err != nil { + t.Fatal(err) + } + if len(candidates) != 0 || examined != maxExaminedProjectEntries || byResults || !byExamined { + t.Fatalf("oversized selection leaked filesystem order: candidates=%v examined=%d result cap=%v examination cap=%v", candidates, examined, byResults, byExamined) + } + }) + } +} + +func TestListProjectsCancellationIsNotTruncation(t *testing.T) { + parent := t.TempDir() + createProjectDirs(t, parent, 80, "project") + ctx := &cancelAfterMCPChecks{Context: context.Background(), remaining: 1} + result, _, err := handleListProjects(ctx, nil, ListProjectsInput{Path: parent}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + if !result.IsError || !strings.Contains(strings.ToLower(out), "cancel") || strings.Contains(strings.ToLower(out), "truncated") { + t.Fatalf("cancelled discovery result: IsError=%v output=%q", result.IsError, out) + } +} + +func TestListProjectsPreservesPerChildScanErrors(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission fixture is Unix-specific") + } + parent := t.TempDir() + project := filepath.Join(parent, "restricted") + blocked := filepath.Join(project, "blocked") + if err := os.MkdirAll(blocked, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(blocked, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(blocked, 0o755) }) + + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: parent}) + if err != nil { + t.Fatal(err) + } + out := resultText(t, result) + if result.IsError || !strings.Contains(out, "restricted/") || !strings.Contains(out, "(error scanning)") { + t.Fatalf("per-child scan failure was not preserved: IsError=%v output=%q", result.IsError, out) + } +} + +func TestTraversalRootsFailClosed(t *testing.T) { + notDir := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(notDir, []byte("not a project"), 0o644); err != nil { + t.Fatal(err) + } + tests := []struct { + name string + call func() (bool, string, error) + }{ + {"structure", func() (bool, string, error) { + r, _, e := handleGetStructure(context.Background(), nil, StructureInput{Path: notDir}) + return resultState(t, r, e) + }}, + {"dependencies", func() (bool, string, error) { + r, _, e := handleGetDependencies(context.Background(), nil, PathInput{Path: notDir}) + return resultState(t, r, e) + }}, + {"diff", func() (bool, string, error) { + r, _, e := handleGetDiff(context.Background(), nil, DiffInput{Path: notDir, Ref: "HEAD"}) + return resultState(t, r, e) + }}, + {"find", func() (bool, string, error) { + r, _, e := handleFindFile(context.Background(), nil, FindInput{Path: notDir, Pattern: "x"}) + return resultState(t, r, e) + }}, + {"importers", func() (bool, string, error) { + r, _, e := handleGetImporters(context.Background(), nil, ImportersInput{Path: notDir, File: "x.go"}) + return resultState(t, r, e) + }}, + {"hubs", func() (bool, string, error) { + r, _, e := handleGetHubs(context.Background(), nil, PathInput{Path: notDir}) + return resultState(t, r, e) + }}, + {"file context", func() (bool, string, error) { + r, _, e := handleGetFileContext(context.Background(), nil, ImportersInput{Path: notDir, File: "x.go"}) + return resultState(t, r, e) + }}, + {"handoff", func() (bool, string, error) { + r, _, e := handleGetHandoff(context.Background(), nil, HandoffInput{Path: notDir}) + return resultState(t, r, e) + }}, + {"list projects", func() (bool, string, error) { + r, _, e := handleListProjects(context.Background(), nil, ListProjectsInput{Path: notDir}) + return resultState(t, r, e) + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isError, text, err := tt.call() + if err != nil { + t.Fatal(err) + } + if !isError || text == "" { + t.Fatalf("invalid explicit root did not fail closed: IsError=%v text=%q", isError, text) + } + }) + } + + result, _, err := handleListProjects(context.Background(), nil, ListProjectsInput{Path: string([]byte{'b', 'a', 'd', 0, 'p', 'a', 't', 'h'})}) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatalf("malformed root did not fail closed: %q", resultText(t, result)) + } +} + +func TestHandoffCancellationBeforePersistenceCommitPoint(t *testing.T) { + oldBuild := buildHandoffForMCP + oldWrite := writeLatestForMCP + t.Cleanup(func() { + buildHandoffForMCP = oldBuild + writeLatestForMCP = oldWrite + }) + + ctx, cancel := context.WithCancel(context.Background()) + buildHandoffForMCP = func(context.Context, string, handoff.BuildOptions) (*handoff.Artifact, error) { + cancel() + return &handoff.Artifact{}, nil + } + writeCalled := false + writeLatestForMCP = func(string, *handoff.Artifact) error { + writeCalled = true + return nil + } + + result, _, err := handleGetHandoff(ctx, nil, HandoffInput{Path: t.TempDir(), Save: true}) + if err != nil { + t.Fatal(err) + } + if writeCalled { + t.Fatal("WriteLatest began after cancellation was observed before persistence commit point") + } + if !result.IsError || !strings.Contains(strings.ToLower(resultText(t, result)), "cancel") { + t.Fatalf("pre-persistence cancellation result: IsError=%v text=%q", result.IsError, resultText(t, result)) + } +} + +func TestGetDiffPreservesBestEffortUnavailableImpactScan(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake Git script is Unix-specific") + } + + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + binDir := t.TempDir() + fakeGit := filepath.Join(binDir, "git") + script := `#!/bin/sh +if [ "$1" = "diff" ] && [ "$2" = "--numstat" ]; then + printf '1\t0\tmain.go\n' +fi +exit 0 +` + if err := os.WriteFile(fakeGit, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + // Keep Git available while making ast-grep unavailable. + t.Setenv("PATH", binDir) + + result, _, err := handleGetDiff(context.Background(), nil, DiffInput{Path: root, Ref: "HEAD"}) + if err != nil { + t.Fatal(err) + } + if result.IsError || !strings.Contains(resultText(t, result), "main.go") { + t.Fatalf("ordinary impact scan failure escaped get_diff: IsError=%v output=%q", result.IsError, resultText(t, result)) + } +} + +func createProjectDirs(t *testing.T, parent string, count int, prefix string) { + t.Helper() + for i := 0; i < count; i++ { + root := filepath.Join(parent, fmt.Sprintf("%s-%03d", prefix, i)) + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + } +} diff --git a/mcp/main.go b/mcp/main.go index dae04f6..6459ae0 100644 --- a/mcp/main.go +++ b/mcp/main.go @@ -32,6 +32,10 @@ import ( var ( watchers = make(map[string]*watch.Daemon) watchersMu sync.RWMutex + + buildHandoffForMCP = handoff.BuildContext + buildHandoffDetailMCP = handoff.BuildFileDetailContext + writeLatestForMCP = handoff.WriteLatest ) const ( @@ -289,15 +293,40 @@ func errorResult(text string) *mcp.CallToolResult { } } -func handleGetStructure(ctx context.Context, req *mcp.CallToolRequest, input StructureInput) (*mcp.CallToolResult, any, error) { - absRoot, err := filepath.Abs(input.Path) +func traversalRoot(path string) (string, *mcp.CallToolResult) { + absRoot, err := filepath.Abs(path) if err != nil { - return errorResult("Invalid path: " + err.Error()), nil, nil + return "", errorResult("Invalid path: " + err.Error()) + } + info, err := os.Stat(absRoot) + if err != nil || !info.IsDir() { + return "", errorResult("Invalid project path: path is not an accessible directory") + } + return absRoot, nil +} + +func cancellationResult(ctx context.Context, operation string) *mcp.CallToolResult { + if err := ctx.Err(); err != nil { + return errorResult(operation + " cancelled: " + err.Error()) + } + return nil +} + +func handleGetStructure(ctx context.Context, req *mcp.CallToolRequest, input StructureInput) (*mcp.CallToolResult, any, error) { + if cancelled := cancellationResult(ctx, "Structure scan"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil } - gitCache := scanner.NewGitIgnoreCache(input.Path) - files, err := scanner.ScanConfiguredFiles(input.Path, gitCache) + gitCache := scanner.NewGitIgnoreCache(absRoot) + files, err := scanner.ScanConfiguredFilesContext(ctx, absRoot, gitCache) if err != nil { + if cancelled := cancellationResult(ctx, "Structure scan"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Scan error: " + err.Error()), nil, nil } fileCount := len(files) @@ -337,7 +366,10 @@ func handleGetStructure(ctx context.Context, req *mcp.CallToolRequest, input Str hubs = append(hubs, state.Hubs...) importers = state.Importers } else if fileCount <= limits.LargeRepoFileCount { - fg, err := scanner.BuildFileGraph(absRoot) + fg, err := scanner.BuildFileGraphContext(ctx, absRoot) + if cancelled := cancellationResult(ctx, "Structure graph analysis"); cancelled != nil { + return cancelled, nil, nil + } if err == nil { hubs = fg.HubFiles() importers = fg.Importers @@ -364,21 +396,34 @@ func handleGetStructure(ctx context.Context, req *mcp.CallToolRequest, input Str } func handleGetDependencies(ctx context.Context, req *mcp.CallToolRequest, input PathInput) (*mcp.CallToolResult, any, error) { - absRoot, err := filepath.Abs(input.Path) - if err != nil { - return errorResult("Invalid path: " + err.Error()), nil, nil + if cancelled := cancellationResult(ctx, "Dependency scan"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil } - analyses, err := scanner.ScanForDeps(input.Path) + analyses, err := scanner.ScanForDepsContext(ctx, absRoot) if err != nil { + if cancelled := cancellationResult(ctx, "Dependency scan"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Scan error: " + err.Error()), nil, nil } + externalDeps, err := scanner.ReadExternalDepsContext(ctx, absRoot, scanner.MCPManifestByteBudget) + if err != nil { + if cancelled := cancellationResult(ctx, "External dependency scan"); cancelled != nil { + return cancelled, nil, nil + } + return errorResult("External dependency scan error: " + err.Error()), nil, nil + } depsProject := scanner.DepsProject{ Root: absRoot, Mode: "deps", Files: analyses, - ExternalDeps: scanner.ReadExternalDeps(absRoot), + ExternalDeps: externalDeps, } var buf bytes.Buffer @@ -389,18 +434,24 @@ func handleGetDependencies(ctx context.Context, req *mcp.CallToolRequest, input } func handleGetDiff(ctx context.Context, req *mcp.CallToolRequest, input DiffInput) (*mcp.CallToolResult, any, error) { + if cancelled := cancellationResult(ctx, "Diff analysis"); cancelled != nil { + return cancelled, nil, nil + } ref := input.Ref if ref == "" { ref = "main" } - absRoot, err := filepath.Abs(input.Path) - if err != nil { - return errorResult("Invalid path: " + err.Error()), nil, nil + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil } - diffInfo, err := scanner.GitDiffInfo(absRoot, ref) + diffInfo, err := scanner.GitDiffInfoContext(ctx, absRoot, ref) if err != nil { + if cancelled := cancellationResult(ctx, "Diff analysis"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Git diff error: " + err.Error() + "\nMake sure '" + ref + "' is a valid branch/ref"), nil, nil } @@ -408,14 +459,23 @@ func handleGetDiff(ctx context.Context, req *mcp.CallToolRequest, input DiffInpu return textResult("No files changed vs " + ref), nil, nil } - gitCache := scanner.NewGitIgnoreCache(input.Path) - files, err := scanner.ScanConfiguredFiles(input.Path, gitCache) + gitCache := scanner.NewGitIgnoreCache(absRoot) + files, err := scanner.ScanConfiguredFilesContext(ctx, absRoot, gitCache) if err != nil { + if cancelled := cancellationResult(ctx, "Diff scan"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Scan error: " + err.Error()), nil, nil } files = scanner.FilterToChangedWithInfo(files, diffInfo) - impact := scanner.AnalyzeImpact(absRoot, files) + impact, err := scanner.AnalyzeImpactContext(ctx, absRoot, files) + if err != nil { + if cancelled := cancellationResult(ctx, "Impact analysis"); cancelled != nil { + return cancelled, nil, nil + } + return errorResult("Impact scan error: " + err.Error()), nil, nil + } project := scanner.Project{ Root: absRoot, @@ -433,14 +493,24 @@ func handleGetDiff(ctx context.Context, req *mcp.CallToolRequest, input DiffInpu } func handleFindFile(ctx context.Context, req *mcp.CallToolRequest, input FindInput) (*mcp.CallToolResult, any, error) { - gitCache := scanner.NewGitIgnoreCache(input.Path) - files, err := scanner.ScanFiles(input.Path, gitCache, nil, nil) + if cancelled := cancellationResult(ctx, "File search"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil + } + gitCache := scanner.NewGitIgnoreCache(absRoot) + files, err := scanner.ScanFilesContext(ctx, absRoot, gitCache, nil, nil) if err != nil { + if cancelled := cancellationResult(ctx, "File search"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Scan error: " + err.Error()), nil, nil } // Filter files matching pattern (case-insensitive) - matches, filteredMatches, hintsEnabled := findConfiguredMatches(input.Path, input.Pattern, files) + matches, filteredMatches, hintsEnabled := findConfiguredMatches(absRoot, input.Pattern, files) if len(matches) == 0 { if hintsEnabled && len(filteredMatches) > 0 { @@ -510,6 +580,9 @@ func statusHandler(guidance string) func(context.Context, *mcp.CallToolRequest, } func handleListProjects(ctx context.Context, req *mcp.CallToolRequest, input ListProjectsInput) (*mcp.CallToolResult, any, error) { + if cancelled := cancellationResult(ctx, "Project discovery"); cancelled != nil { + return cancelled, nil, nil + } // Expand ~ to home directory path := input.Path if strings.HasPrefix(path, "~/") { @@ -517,47 +590,51 @@ func handleListProjects(ctx context.Context, req *mcp.CallToolRequest, input Lis path = filepath.Join(home, path[2:]) } - absPath, err := filepath.Abs(path) - if err != nil { - return errorResult("Invalid path: " + err.Error()), nil, nil + absPath, invalid := traversalRoot(path) + if invalid != nil { + return invalid, nil, nil } - entries, err := os.ReadDir(absPath) + dir, err := os.Open(absPath) if err != nil { return errorResult("Cannot read directory: " + err.Error()), nil, nil } + defer dir.Close() pattern := strings.ToLower(input.Pattern) - var projects []string - - for _, entry := range entries { - if !entry.IsDir() { - continue - } - name := entry.Name() - - // Skip hidden directories and common non-project dirs - if strings.HasPrefix(name, ".") { - continue + candidates, examined, truncatedByResults, truncatedByExamined, err := selectProjectCandidates(ctx, dir, pattern) + if err != nil { + if cancelled := cancellationResult(ctx, "Project discovery"); cancelled != nil { + return cancelled, nil, nil } + return errorResult("Cannot read directory: " + err.Error()), nil, nil + } - // Filter by pattern if provided - if pattern != "" && !strings.Contains(strings.ToLower(name), pattern) { - continue + projects := make([]string, 0, len(candidates)) + for _, name := range candidates { + if cancelled := cancellationResult(ctx, "Project discovery"); cancelled != nil { + return cancelled, nil, nil } - - // Get project stats projectPath := filepath.Join(absPath, name) - stats := getProjectStats(projectPath) - + stats, scanErr := getProjectStatsContext(ctx, projectPath) + if scanErr != nil { + if cancelled := cancellationResult(ctx, "Project discovery"); cancelled != nil { + return cancelled, nil, nil + } + stats = "(error scanning)" + } projects = append(projects, fmt.Sprintf("%-30s %s", name+"/", stats)) } + if truncatedByExamined && len(candidates) == 0 { + return textResult(projectDiscoveryExaminationCap(input.Pattern, absPath, examined)), nil, nil + } if len(projects) == 0 { + truncation := projectDiscoveryTruncation(truncatedByResults, truncatedByExamined, examined) if pattern != "" { - return textResult(fmt.Sprintf("No projects matching '%s' in %s", input.Pattern, absPath)), nil, nil + return textResult(fmt.Sprintf("No projects matching '%s' in %s%s", input.Pattern, absPath, truncation)), nil, nil } - return textResult("No project directories found in " + absPath), nil, nil + return textResult("No project directories found in " + absPath + truncation), nil, nil } header := fmt.Sprintf("Projects in %s", absPath) @@ -565,21 +642,114 @@ func handleListProjects(ctx context.Context, req *mcp.CallToolRequest, input Lis header = fmt.Sprintf("Projects matching '%s' in %s", input.Pattern, absPath) } - return textResult(fmt.Sprintf("%s:\n\n%s", header, strings.Join(projects, "\n"))), nil, nil + output := fmt.Sprintf("%s:\n\n%s", header, strings.Join(projects, "\n")) + output += projectDiscoveryTruncation(truncatedByResults, truncatedByExamined, examined) + return textResult(output), nil, nil +} + +func projectDiscoveryExaminationCap(pattern, path string, examined int) string { + if pattern != "" { + return fmt.Sprintf("Project discovery matching '%s' in %s was truncated after examining %d entries; results are unavailable because the directory exceeds the examination cap.", pattern, path, examined) + } + return fmt.Sprintf("Project discovery in %s was truncated after examining %d entries; results are unavailable because the directory exceeds the examination cap.", path, examined) +} + +const ( + maxListedProjects = 50 + maxExaminedProjectEntries = 200 + projectDiscoveryBatchSize = 32 +) + +type projectDirectoryReader interface { + ReadDir(int) ([]os.DirEntry, error) +} + +// selectProjectCandidates deterministically selects the lexicographically +// first result-cap entries within the bounded examination window. +func selectProjectCandidates(ctx context.Context, dir projectDirectoryReader, pattern string) ([]string, int, bool, bool, error) { + candidates := make([]string, 0, maxListedProjects+1) + examined := 0 + truncatedByExamined := false + discoveryDone := false + + for !discoveryDone { + if err := ctx.Err(); err != nil { + return nil, examined, false, truncatedByExamined, err + } + entries, readErr := dir.ReadDir(projectDiscoveryBatchSize) + for _, entry := range entries { + if examined >= maxExaminedProjectEntries { + truncatedByExamined = true + discoveryDone = true + break + } + examined++ + if !entry.IsDir() { + continue + } + name := entry.Name() + if strings.HasPrefix(name, ".") { + continue + } + if pattern != "" && !strings.Contains(strings.ToLower(name), pattern) { + continue + } + candidates = append(candidates, name) + } + if readErr == io.EOF { + discoveryDone = true + } else if readErr != nil { + return nil, examined, false, truncatedByExamined, readErr + } + } + + if truncatedByExamined { + // The bounded prefix is filesystem-order dependent. Do not emit a + // partial selection when the examination cap prevents observing the + // complete directory; only the stable truncation notice is returned. + return []string{}, examined, false, true, ctx.Err() + } + sort.Strings(candidates) + truncatedByResults := len(candidates) > maxListedProjects + if truncatedByResults { + candidates = candidates[:maxListedProjects] + } + return candidates, examined, truncatedByResults, truncatedByExamined, ctx.Err() +} + +func projectDiscoveryTruncation(byResults, byExamined bool, examined int) string { + switch { + case byResults && byExamined: + return fmt.Sprintf("\n... truncated after %d projects and examining %d entries", maxListedProjects, examined) + case byResults: + return fmt.Sprintf("\n... truncated after %d projects", maxListedProjects) + case byExamined: + return fmt.Sprintf("\n... truncated after examining %d entries", examined) + default: + return "" + } } // getProjectStats returns a brief summary of a project directory // Uses the same scanner logic as the main codemap command (respects nested .gitignore files) func getProjectStats(path string) string { + stats, _ := getProjectStatsContext(context.Background(), path) + return stats +} + +func getProjectStatsContext(ctx context.Context, path string) (string, error) { gitCache := scanner.NewGitIgnoreCache(path) - files, err := scanner.ScanConfiguredFiles(path, gitCache) + files, err := scanner.ScanConfiguredFilesContext(ctx, path, gitCache) if err != nil { - return "(error scanning)" + return "(error scanning)", err } // Count files by language langCounts := make(map[string]int) for _, f := range files { + if err := ctx.Err(); err != nil { + return "(error scanning)", err + } lang := scanner.DetectLanguage(f.Path) if lang != "" { langCounts[lang]++ @@ -603,14 +773,24 @@ func getProjectStats(path string) string { } if lang, ok := scanner.LangDisplay[primaryLang]; ok { - return fmt.Sprintf("(%d files, %s%s)", len(files), lang, isGit) + return fmt.Sprintf("(%d files, %s%s)", len(files), lang, isGit), nil } - return fmt.Sprintf("(%d files%s)", len(files), isGit) + return fmt.Sprintf("(%d files%s)", len(files), isGit), ctx.Err() } func handleGetImporters(ctx context.Context, req *mcp.CallToolRequest, input ImportersInput) (*mcp.CallToolResult, any, error) { - fg, err := scanner.BuildFileGraph(input.Path) + if cancelled := cancellationResult(ctx, "Importer analysis"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil + } + fg, err := scanner.BuildFileGraphContext(ctx, absRoot) if err != nil { + if cancelled := cancellationResult(ctx, "Importer analysis"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Failed to build file graph: " + err.Error()), nil, nil } @@ -639,16 +819,20 @@ func mcpCoverageText(fg *scanner.FileGraph) string { } func handleGetHandoff(ctx context.Context, req *mcp.CallToolRequest, input HandoffInput) (*mcp.CallToolResult, any, error) { + if cancelled := cancellationResult(ctx, "Handoff generation"); cancelled != nil { + return cancelled, nil, nil + } if input.Prefix && input.Delta { return errorResult("prefix and delta options are mutually exclusive"), nil, nil } - absRoot, err := filepath.Abs(input.Path) - if err != nil { - return errorResult("Invalid path: " + err.Error()), nil, nil + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil } var artifact *handoff.Artifact + var err error if input.Latest { artifact, err = handoff.ReadLatest(absRoot) if err != nil { @@ -673,23 +857,35 @@ func handleGetHandoff(ctx context.Context, req *mcp.CallToolRequest, input Hando } } - artifact, err = handoff.Build(absRoot, handoff.BuildOptions{ + artifact, err = buildHandoffForMCP(ctx, absRoot, handoff.BuildOptions{ BaseRef: baseRef, Since: since, }) if err != nil { + if cancelled := cancellationResult(ctx, "Handoff generation"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Failed to build handoff: " + err.Error()), nil, nil } if input.Save { - if err := handoff.WriteLatest(absRoot, artifact); err != nil { + // Persistence commit point: cancellation observed before WriteLatest + // prevents all writes. Once WriteLatest begins, its established + // non-contextual multi-file semantics remain unchanged. + if cancelled := cancellationResult(ctx, "Handoff generation"); cancelled != nil { + return cancelled, nil, nil + } + if err := writeLatestForMCP(absRoot, artifact); err != nil { return errorResult("Failed to save handoff: " + err.Error()), nil, nil } } } if input.File != "" { - detail, err := handoff.BuildFileDetail(absRoot, artifact, input.File, nil) + detail, err := buildHandoffDetailMCP(ctx, absRoot, artifact, input.File, nil) if err != nil { + if cancelled := cancellationResult(ctx, "Handoff detail generation"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Failed to load handoff detail: " + err.Error()), nil, nil } if input.JSON { @@ -989,8 +1185,18 @@ The user may be: // === FILE GRAPH HANDLERS === func handleGetHubs(ctx context.Context, req *mcp.CallToolRequest, input PathInput) (*mcp.CallToolResult, any, error) { - fg, err := scanner.BuildFileGraph(input.Path) + if cancelled := cancellationResult(ctx, "Hub analysis"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil + } + fg, err := scanner.BuildFileGraphContext(ctx, absRoot) if err != nil { + if cancelled := cancellationResult(ctx, "Hub analysis"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Failed to build file graph: " + err.Error()), nil, nil } @@ -1025,8 +1231,18 @@ func handleGetHubs(ctx context.Context, req *mcp.CallToolRequest, input PathInpu } func handleGetFileContext(ctx context.Context, req *mcp.CallToolRequest, input ImportersInput) (*mcp.CallToolResult, any, error) { - fg, err := scanner.BuildFileGraph(input.Path) + if cancelled := cancellationResult(ctx, "File context analysis"); cancelled != nil { + return cancelled, nil, nil + } + absRoot, invalid := traversalRoot(input.Path) + if invalid != nil { + return invalid, nil, nil + } + fg, err := scanner.BuildFileGraphContext(ctx, absRoot) if err != nil { + if cancelled := cancellationResult(ctx, "File context analysis"); cancelled != nil { + return cancelled, nil, nil + } return errorResult("Failed to build file graph: " + err.Error()), nil, nil } diff --git a/scanner/astgrep.go b/scanner/astgrep.go index ceb86b8..7881468 100644 --- a/scanner/astgrep.go +++ b/scanner/astgrep.go @@ -206,6 +206,15 @@ func findNestedGitRepos(root string) []string { // ScanDirectory analyzes all files in a directory using sg scan func (s *AstGrepScanner) ScanDirectory(root string) ([]FileAnalysis, error) { + return s.ScanDirectoryContext(context.Background(), root) +} + +// ScanDirectoryContext analyzes all files in a directory while honoring caller cancellation. +// The existing internal timeout remains a best-effort empty-result safety cap. +func (s *AstGrepScanner) ScanDirectoryContext(parent context.Context, root string) ([]FileAnalysis, error) { + if err := parent.Err(); err != nil { + return nil, err + } if !s.Available() { return nil, nil } @@ -234,12 +243,16 @@ func (s *AstGrepScanner) ScanDirectory(root string) ([]FileAnalysis, error) { } args = append(args, root) - ctx, cancel := context.WithTimeout(context.Background(), astGrepScanTimeout) + ctx, cancel := context.WithTimeout(parent, astGrepScanTimeout) defer cancel() cmd := exec.CommandContext(ctx, s.binary, args...) + cmd.WaitDelay = 100 * time.Millisecond out, err := cmd.Output() if err != nil { + if parentErr := parent.Err(); parentErr != nil { + return nil, parentErr + } if errors.Is(ctx.Err(), context.DeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) { fmt.Fprintf(os.Stderr, "warning: ast-grep timed out after %s in %s; skipping ast-grep results\n", astGrepScanTimeout, root) return nil, nil @@ -258,6 +271,9 @@ func (s *AstGrepScanner) ScanDirectory(root string) ([]FileAnalysis, error) { return nil, nil } } + if err := parent.Err(); err != nil { + return nil, err + } // Extract JSON array from output (handles debug output before JSON, e.g. ast-grep 0.40.2 bug) jsonData := extractJSONArray(out) diff --git a/scanner/cancellation_test.go b/scanner/cancellation_test.go new file mode 100644 index 0000000..9e2abad --- /dev/null +++ b/scanner/cancellation_test.go @@ -0,0 +1,253 @@ +package scanner + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +type cancelAfterScannerChecks struct { + context.Context + remaining int +} + +func (c *cancelAfterScannerChecks) Err() error { + if c.remaining <= 0 { + return context.Canceled + } + c.remaining-- + return nil +} + +func TestScanContextCancellation(t *testing.T) { + root := t.TempDir() + for i := 0; i < 40; i++ { + if err := os.WriteFile(filepath.Join(root, fmt.Sprintf("file-%02d.go", i)), []byte("package fixture\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := ScanFilesContext(cancelled, root, NewGitIgnoreCache(root), nil, nil); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled ScanFilesContext error = %v, want context.Canceled", err) + } + + inFlight := &cancelAfterScannerChecks{Context: context.Background(), remaining: 8} + if _, err := ScanConfiguredFilesContext(inFlight, root, NewGitIgnoreCache(root)); !errors.Is(err, context.Canceled) { + t.Fatalf("in-flight ScanConfiguredFilesContext error = %v, want context.Canceled", err) + } +} + +func TestDependencyAndGraphContextCancellation(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + analyses := make([]FileAnalysis, 100) + for i := range analyses { + analyses[i] = FileAnalysis{Path: "main.go", Language: "go", Imports: []string{"example.com/dependency"}} + } + + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := ScanForDepsContext(cancelled, root); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled ScanForDepsContext error = %v, want context.Canceled", err) + } + if _, err := ScanForDepsWithFiltersContext(cancelled, root, Filters{}); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled ScanForDepsWithFiltersContext error = %v, want context.Canceled", err) + } + if _, err := BuildFileGraphContext(cancelled, root); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled BuildFileGraphContext error = %v, want context.Canceled", err) + } + if _, err := BuildFileGraphFromAnalysesContext(cancelled, root, analyses); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled BuildFileGraphFromAnalysesContext error = %v, want context.Canceled", err) + } + + inFlight := &cancelAfterScannerChecks{Context: context.Background(), remaining: 12} + if _, err := BuildFileGraphFromAnalysesContext(inFlight, root, analyses); !errors.Is(err, context.Canceled) { + t.Fatalf("in-flight graph build error = %v, want context.Canceled", err) + } +} + +func TestAstGrepCallerContextOutcomes(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("requires shell script execution") + } + root := t.TempDir() + fakeBinary := filepath.Join(root, "fake-sg.sh") + marker := filepath.Join(t.TempDir(), "started") + script := `#!/bin/sh +printf '%s\n' "$$" > "$CODEMAP_AST_GREP_TEST_MARKER" +exec sleep 10 +` + if err := os.WriteFile(fakeBinary, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("CODEMAP_AST_GREP_TEST_MARKER", marker) + s := &AstGrepScanner{rulesDir: root, binary: fakeBinary} + + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := s.ScanDirectoryContext(cancelled, root); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled ScanDirectoryContext error = %v, want context.Canceled", err) + } + + deadline, stop := context.WithTimeout(context.Background(), time.Second) + defer stop() + started := time.Now() + done := make(chan error, 1) + go func() { + _, err := s.ScanDirectoryContext(deadline, root) + done <- err + }() + var blockerPID string + startupDeadline := time.Now().Add(5 * time.Second) + for { + if data, err := os.ReadFile(marker); err == nil && len(data) > 0 { + blockerPID = strings.TrimSpace(string(data)) + break + } + select { + case err := <-done: + t.Fatalf("ScanDirectoryContext exited before fake ast-grep subprocess started: %v", err) + default: + } + if time.Now().After(startupDeadline) { + t.Fatal("fake ast-grep subprocess did not start") + } + time.Sleep(5 * time.Millisecond) + } + select { + case err := <-done: + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("caller-deadline ScanDirectoryContext error = %v, want context.DeadlineExceeded", err) + } + case <-time.After(2 * time.Second): + t.Fatal("ScanDirectoryContext did not terminate deadline-exceeded subprocess") + } + if elapsed := time.Since(started); elapsed > 2*time.Second { + t.Fatalf("caller deadline did not terminate ast-grep promptly: %s", elapsed) + } + if err := exec.Command("/bin/kill", "-0", blockerPID).Run(); err == nil { + t.Fatalf("deadline-exceeded fake ast-grep left blocker process %s running", blockerPID) + } +} + +func TestGitDiffInfoContextTerminatesSubprocess(t *testing.T) { + cancelled, cancelNow := context.WithCancel(context.Background()) + cancelNow() + if _, err := GitDiffInfoContext(cancelled, t.TempDir(), "main"); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled GitDiffInfoContext error = %v, want context.Canceled", err) + } + + if runtime.GOOS == "windows" { + t.Skip("requires shell script execution") + } + binDir := t.TempDir() + marker := filepath.Join(t.TempDir(), "started") + fakeGit := filepath.Join(binDir, "git") + script := `#!/bin/sh +printf '%s\n' "$$" > "$CODEMAP_GIT_TEST_MARKER" +exec sleep 10 +` + if err := os.WriteFile(fakeGit, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("CODEMAP_GIT_TEST_MARKER", marker) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + gitRoot := t.TempDir() + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + _, err := GitDiffInfoContext(ctx, gitRoot, "main") + done <- err + }() + var blockerPID string + deadline := time.Now().Add(5 * time.Second) + for { + if data, err := os.ReadFile(marker); err == nil && len(data) > 0 { + blockerPID = strings.TrimSpace(string(data)) + break + } + select { + case err := <-done: + t.Fatalf("GitDiffInfoContext exited before fake git subprocess started: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("fake git subprocess did not start") + } + time.Sleep(5 * time.Millisecond) + } + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Fatalf("GitDiffInfoContext error = %v, want context.Canceled", err) + } + case <-time.After(2 * time.Second): + t.Fatal("GitDiffInfoContext did not terminate cancelled subprocess") + } + if err := exec.Command("/bin/kill", "-0", blockerPID).Run(); err == nil { + t.Fatalf("cancelled fake git left blocker process %s running", blockerPID) + } +} + +func TestAnalyzeImpactContextPreCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := AnalyzeImpactContext(ctx, t.TempDir(), []FileInfo{{Path: "changed.go"}}) + if !errors.Is(err, context.Canceled) { + t.Fatalf("AnalyzeImpactContext error = %v, want context.Canceled", err) + } +} + +func TestReadExternalDepsContextBudgetAndCompatibility(t *testing.T) { + root := t.TempDir() + large := strings.Repeat("x", int(MCPManifestByteBudget)+1) + largeManifest := "require (\nexample.com/large v1.0.0\n)\n" + large + if err := os.WriteFile(filepath.Join(root, "go.mod"), []byte(largeManifest), 0o644); err != nil { + t.Fatal(err) + } + smallDir := filepath.Join(root, "nested") + if err := os.Mkdir(smallDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(smallDir, "requirements.txt"), []byte("small-dependency==1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + + deps, err := ReadExternalDepsContext(context.Background(), root, MCPManifestByteBudget) + if err != nil { + t.Fatalf("ReadExternalDepsContext error: %v", err) + } + if got := deps["go"]; len(got) != 0 { + t.Fatalf("bounded MCP read parsed oversized go.mod: %v", got) + } + if got := deps["python"]; len(got) != 1 || got[0] != "small-dependency" { + t.Fatalf("bounded MCP read lost valid small manifest: %v", got) + } + legacy := ReadExternalDeps(root) + if got := legacy["go"]; len(got) != 1 || got[0] != "example.com/large" { + t.Fatalf("legacy wrapper no longer reads oversized manifest: %v", got) + } + + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := ReadExternalDepsContext(cancelled, root, MCPManifestByteBudget); !errors.Is(err, context.Canceled) { + t.Fatalf("pre-cancelled dependency read error = %v, want context.Canceled", err) + } + inFlight := &cancelAfterScannerChecks{Context: context.Background(), remaining: 2} + if _, err := ReadExternalDepsContext(inFlight, root, MCPManifestByteBudget); !errors.Is(err, context.Canceled) { + t.Fatalf("in-flight dependency read error = %v, want context.Canceled", err) + } +} diff --git a/scanner/deps.go b/scanner/deps.go index 90b322a..c4ddbb2 100644 --- a/scanner/deps.go +++ b/scanner/deps.go @@ -1,17 +1,42 @@ package scanner import ( + "context" + "errors" + "io" "os" "path/filepath" "strings" ) +// MCPManifestByteBudget bounds each manifest read performed for one MCP request. +const MCPManifestByteBudget int64 = 1 << 20 + +var errManifestBudgetExceeded = errors.New("manifest exceeds byte budget") + // ReadExternalDeps reads manifest files (go.mod, requirements.txt, package.json) func ReadExternalDeps(root string) map[string][]string { + deps, _ := ReadExternalDepsContext(context.Background(), root, 0) + if deps == nil { + return make(map[string][]string) + } + return deps +} + +// ReadExternalDepsContext reads external dependencies with caller cancellation. +// A positive manifestByteBudget skips individual oversized manifests; zero keeps +// the legacy unbounded behavior used by CLI and blast-radius callers. +func ReadExternalDepsContext(ctx context.Context, root string, manifestByteBudget int64) (map[string][]string, error) { + if err := ctx.Err(); err != nil { + return nil, err + } deps := make(map[string][]string) // Walk tree to find all manifest files - filepath.Walk(root, func(path string, info os.FileInfo, _ error) error { + err := filepath.Walk(root, func(path string, info os.FileInfo, _ error) error { + if err := ctx.Err(); err != nil { + return err + } if info == nil { return nil } @@ -21,38 +46,35 @@ func ReadExternalDeps(root string) map[string][]string { } return nil } + if !isDependencyManifest(info.Name()) { + return nil + } + content, err := readManifestContext(ctx, path, info.Size(), manifestByteBudget) + if errors.Is(err, errManifestBudgetExceeded) { + return nil + } + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + return nil + } switch info.Name() { case "go.mod": - if c, err := os.ReadFile(path); err == nil { - deps["go"] = append(deps["go"], parseGoMod(string(c))...) - } + deps["go"] = append(deps["go"], parseGoMod(string(content))...) case "requirements.txt": - if c, err := os.ReadFile(path); err == nil { - deps["python"] = append(deps["python"], parseRequirements(string(c))...) - } + deps["python"] = append(deps["python"], parseRequirements(string(content))...) case "package.json": - if c, err := os.ReadFile(path); err == nil { - deps["javascript"] = append(deps["javascript"], parsePackageJson(string(c))...) - } + deps["javascript"] = append(deps["javascript"], parsePackageJson(string(content))...) case "Podfile": - if c, err := os.ReadFile(path); err == nil { - deps["swift"] = append(deps["swift"], parsePodfile(string(c))...) - } + deps["swift"] = append(deps["swift"], parsePodfile(string(content))...) case "Package.swift": - if c, err := os.ReadFile(path); err == nil { - deps["swift"] = append(deps["swift"], parsePackageSwift(string(c))...) - } + deps["swift"] = append(deps["swift"], parsePackageSwift(string(content))...) case "packages.config": - if c, err := os.ReadFile(path); err == nil { - deps["csharp"] = append(deps["csharp"], parsePackagesConfig(string(c))...) - } + deps["csharp"] = append(deps["csharp"], parsePackagesConfig(string(content))...) default: - // .csproj files have project-specific names, so they must be matched - // by extension rather than a fixed case above. if strings.HasSuffix(info.Name(), ".csproj") { - if c, err := os.ReadFile(path); err == nil { - deps["csharp"] = append(deps["csharp"], parseCsproj(string(c))...) - } + deps["csharp"] = append(deps["csharp"], parseCsproj(string(content))...) } } return nil @@ -61,7 +83,63 @@ func ReadExternalDeps(root string) map[string][]string { for k, v := range deps { deps[k] = dedupe(v) } - return deps + if err != nil { + return nil, err + } + if err := ctx.Err(); err != nil { + return nil, err + } + return deps, nil +} + +func isDependencyManifest(name string) bool { + switch name { + case "go.mod", "requirements.txt", "package.json", "Podfile", "Package.swift", "packages.config": + return true + default: + return strings.HasSuffix(name, ".csproj") + } +} + +func readManifestContext(ctx context.Context, path string, size, budget int64) ([]byte, error) { + if budget > 0 && size > budget { + return nil, errManifestBudgetExceeded + } + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + reader := io.Reader(f) + if budget > 0 { + reader = io.LimitReader(f, budget+1) + } + data := make([]byte, 0, min(max(size, 0), budgetCapacity(budget))) + buf := make([]byte, 32*1024) + for { + if err := ctx.Err(); err != nil { + return nil, err + } + n, readErr := reader.Read(buf) + data = append(data, buf[:n]...) + if budget > 0 && int64(len(data)) > budget { + return nil, errManifestBudgetExceeded + } + if errors.Is(readErr, io.EOF) { + return data, nil + } + if readErr != nil { + return nil, readErr + } + } +} + +func budgetCapacity(budget int64) int64 { + if budget <= 0 { + return 64 * 1024 + } + return budget } func parseGoMod(c string) (deps []string) { diff --git a/scanner/filegraph.go b/scanner/filegraph.go index 2514ff5..aef3f6a 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -33,31 +33,61 @@ type fileIndex struct { // BuildFileGraph analyzes a project with its configured filters. func BuildFileGraph(root string) (*FileGraph, error) { + return BuildFileGraphContext(context.Background(), root) +} + +// BuildFileGraphContext analyzes a configured project while honoring caller cancellation. +func BuildFileGraphContext(ctx context.Context, root string) (*FileGraph, error) { + if err := ctx.Err(); err != nil { + return nil, err + } cfg := config.Load(root) - return BuildFileGraphWithFilters(root, Filters{Only: cfg.Only, Exclude: cfg.Exclude}) + return BuildFileGraphWithFiltersContext(ctx, root, Filters{Only: cfg.Only, Exclude: cfg.Exclude}) } // BuildFileGraphWithFilters analyzes a project with explicit filters. func BuildFileGraphWithFilters(root string, filters Filters) (*FileGraph, error) { - analyses, err := ScanForDepsWithFilters(root, filters) + return BuildFileGraphWithFiltersContext(context.Background(), root, filters) +} + +// BuildFileGraphWithFiltersContext analyzes a project with explicit filters and caller cancellation. +func BuildFileGraphWithFiltersContext(ctx context.Context, root string, filters Filters) (*FileGraph, error) { + analyses, err := ScanForDepsWithFiltersContext(ctx, root, filters) if err != nil { return nil, err } - return BuildFileGraphFromFilteredAnalyses(root, analyses, filters) + return BuildFileGraphFromFilteredAnalysesContext(ctx, root, analyses, filters) } // BuildFileGraphFromAnalyses builds a file graph from pre-computed analyses // using the configured project filters. func BuildFileGraphFromAnalyses(root string, analyses []FileAnalysis) (*FileGraph, error) { + return BuildFileGraphFromAnalysesContext(context.Background(), root, analyses) +} + +// BuildFileGraphFromAnalysesContext builds a configured graph from existing analyses with caller cancellation. +func BuildFileGraphFromAnalysesContext(ctx context.Context, root string, analyses []FileAnalysis) (*FileGraph, error) { + if err := ctx.Err(); err != nil { + return nil, err + } cfg := config.Load(root) filters := Filters{Only: cfg.Only, Exclude: cfg.Exclude} - return buildFileGraphFromFilteredAnalysesWithCargoMetadata(context.Background(), root, filterAnalyses(analyses, filters), filters, loadCargoMetadata) + filtered, err := filterAnalysesContext(ctx, analyses, filters) + if err != nil { + return nil, err + } + return buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx, root, filtered, filters, loadCargoMetadata) } // BuildFileGraphFromFilteredAnalyses builds a file graph from analyses that // already match the supplied filters. func BuildFileGraphFromFilteredAnalyses(root string, analyses []FileAnalysis, filters Filters) (*FileGraph, error) { - return buildFileGraphFromFilteredAnalysesWithCargoMetadata(context.Background(), root, analyses, filters, loadCargoMetadata) + return BuildFileGraphFromFilteredAnalysesContext(context.Background(), root, analyses, filters) +} + +// BuildFileGraphFromFilteredAnalysesContext builds a graph from filtered analyses with caller cancellation. +func BuildFileGraphFromFilteredAnalysesContext(ctx context.Context, root string, analyses []FileAnalysis, filters Filters) (*FileGraph, error) { + return buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx, root, analyses, filters, loadCargoMetadata) } // buildFileGraphFromAnalysesWithCargoMetadata is the testable configuration @@ -93,7 +123,7 @@ func buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx context.Context, ro // Scan all files with the same filters used for the analyses. gitCache := NewGitIgnoreCache(root) - files, err := ScanFiles(root, gitCache, filters.Only, filters.Exclude) + files, err := ScanFilesContext(ctx, root, gitCache, filters.Only, filters.Exclude) if err != nil { return nil, err } @@ -103,9 +133,15 @@ func buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx context.Context, ro } // Build file index for fast fuzzy matching - idx := buildFileIndex(files, fg.Module) + idx, err := buildFileIndexContext(ctx, files, fg.Module) + if err != nil { + return nil, err + } fg.Packages = idx.goPkgs for _, file := range files { + if err := ctx.Err(); err != nil { + return nil, err + } if strings.EqualFold(filepath.Ext(file.Path), ".rs") { fg.Coverage = GraphCoverage{Status: rustCoverageStatus, Notes: []string{rustCoverageNote}} break @@ -114,12 +150,18 @@ func buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx context.Context, ro // Resolve imports to files using universal fuzzy matching for _, a := range analyses { + if err := ctx.Err(); err != nil { + return nil, err + } var resolvedImports []string if a.Language == "rust" { resolvedImports = resolveRustReferences(absRoot, a, idx, rustWorkspace) } else { for _, imp := range a.Imports { + if err := ctx.Err(); err != nil { + return nil, err + } resolved := fuzzyResolve(imp, a.Path, idx, fg.Module, fg.PathAliases, fg.BaseURL) // Exclude multi-file Go package imports to avoid inflating hub counts. // Go package imports start with the module prefix and resolve to all @@ -143,11 +185,19 @@ func buildFileGraphFromFilteredAnalysesWithCargoMetadata(ctx context.Context, ro } } + if err := ctx.Err(); err != nil { + return nil, err + } return fg, nil } // buildFileIndex creates a multi-key index for fast import resolution func buildFileIndex(files []FileInfo, goModule string) *fileIndex { + idx, _ := buildFileIndexContext(context.Background(), files, goModule) + return idx +} + +func buildFileIndexContext(ctx context.Context, files []FileInfo, goModule string) (*fileIndex, error) { idx := &fileIndex{ byExact: make(map[string][]string), bySuffix: make(map[string][]string), @@ -156,6 +206,9 @@ func buildFileIndex(files []FileInfo, goModule string) *fileIndex { } for _, f := range files { + if err := ctx.Err(); err != nil { + return nil, err + } path := f.Path dir := filepath.Dir(path) if dir == "." { @@ -177,6 +230,9 @@ func buildFileIndex(files []FileInfo, goModule string) *fileIndex { // - "config.py" parts := strings.Split(path, string(filepath.Separator)) for i := 1; i < len(parts); i++ { + if err := ctx.Err(); err != nil { + return nil, err + } suffix := strings.Join(parts[i:], string(filepath.Separator)) idx.bySuffix[suffix] = append(idx.bySuffix[suffix], path) // Also without extension @@ -195,7 +251,10 @@ func buildFileIndex(files []FileInfo, goModule string) *fileIndex { } } - return idx + if err := ctx.Err(); err != nil { + return nil, err + } + return idx, nil } // fuzzyResolve converts an import path to compatible local file paths. diff --git a/scanner/git.go b/scanner/git.go index cac1fd5..58dabee 100644 --- a/scanner/git.go +++ b/scanner/git.go @@ -1,13 +1,17 @@ package scanner import ( + "context" "fmt" "os/exec" "path/filepath" "sort" "strings" + "time" ) +const gitContextWaitDelay = 100 * time.Millisecond + // DiffInfo holds all diff-related data for changed files type DiffInfo struct { Changed map[string]bool // all changed files (modified + untracked) @@ -17,6 +21,14 @@ type DiffInfo struct { // GitDiffInfo returns comprehensive diff information for the repo func GitDiffInfo(root, ref string) (*DiffInfo, error) { + return GitDiffInfoContext(context.Background(), root, ref) +} + +// GitDiffInfoContext returns diff information while honoring cancellation of Git subprocesses. +func GitDiffInfoContext(ctx context.Context, root, ref string) (*DiffInfo, error) { + if err := ctx.Err(); err != nil { + return nil, err + } info := &DiffInfo{ Changed: make(map[string]bool), Untracked: make(map[string]bool), @@ -24,10 +36,14 @@ func GitDiffInfo(root, ref string) (*DiffInfo, error) { } // Get modified files vs ref with stats - cmd := exec.Command("git", "diff", "--numstat", ref) + cmd := exec.CommandContext(ctx, "git", "diff", "--numstat", ref) + cmd.WaitDelay = gitContextWaitDelay cmd.Dir = root output, err := cmd.Output() if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } return nil, err } @@ -51,9 +67,19 @@ func GitDiffInfo(root, ref string) (*DiffInfo, error) { } // Get untracked files (new files) - cmd2 := exec.Command("git", "ls-files", "--others", "--exclude-standard") + if err := ctx.Err(); err != nil { + return nil, err + } + cmd2 := exec.CommandContext(ctx, "git", "ls-files", "--others", "--exclude-standard") + cmd2.WaitDelay = gitContextWaitDelay cmd2.Dir = root - output2, _ := cmd2.Output() + output2, err := cmd2.Output() + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + if err != nil { + output2 = nil + } for _, line := range strings.Split(strings.TrimSpace(string(output2)), "\n") { if line != "" { info.Changed[line] = true @@ -67,7 +93,12 @@ func GitDiffInfo(root, ref string) (*DiffInfo, error) { // GitDiffFiles returns files changed between current HEAD and the given branch/ref // Also includes untracked files (new files not yet committed) func GitDiffFiles(root, ref string) (map[string]bool, error) { - info, err := GitDiffInfo(root, ref) + return GitDiffFilesContext(context.Background(), root, ref) +} + +// GitDiffFilesContext returns changed files while honoring cancellation. +func GitDiffFilesContext(ctx context.Context, root, ref string) (map[string]bool, error) { + info, err := GitDiffInfoContext(ctx, root, ref) if err != nil { return nil, err } @@ -163,16 +194,28 @@ type ImpactInfo struct { // AnalyzeImpact checks which changed files are imported by other files // Uses ast-grep to extract actual imports for accuracy func AnalyzeImpact(root string, changedFiles []FileInfo) []ImpactInfo { + impact, _ := AnalyzeImpactContext(context.Background(), root, changedFiles) + return impact +} + +// AnalyzeImpactContext analyzes impact while honoring caller cancellation. +func AnalyzeImpactContext(ctx context.Context, root string, changedFiles []FileInfo) ([]ImpactInfo, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if len(changedFiles) == 0 { - return nil + return nil, nil } // Scan all files to get their imports using ast-grep - analyses, err := ScanForDeps(root) + analyses, err := ScanForDepsContext(ctx, root) if err != nil { - return nil + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, nil } - return AnalyzeImpactFromAnalyses(changedFiles, analyses) + return AnalyzeImpactFromAnalyses(changedFiles, analyses), ctx.Err() } // AnalyzeImpactFromAnalyses computes impact using a pre-computed ast-grep scan, diff --git a/scanner/walker.go b/scanner/walker.go index 9d4e28a..4e54261 100644 --- a/scanner/walker.go +++ b/scanner/walker.go @@ -2,6 +2,7 @@ package scanner import ( "bufio" + "context" "os" "path/filepath" "strings" @@ -227,10 +228,21 @@ func LoadGitignore(root string) *ignore.GitIgnore { // only: list of extensions to include (empty = all) // exclude: list of patterns to exclude func ScanFiles(root string, cache *GitIgnoreCache, only []string, exclude []string) ([]FileInfo, error) { + return ScanFilesContext(context.Background(), root, cache, only, exclude) +} + +// ScanFilesContext walks the directory tree and honors caller cancellation. +func ScanFilesContext(ctx context.Context, root string, cache *GitIgnoreCache, only []string, exclude []string) ([]FileInfo, error) { + if err := ctx.Err(); err != nil { + return nil, err + } var files []FileInfo absRoot, _ := filepath.Abs(root) err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } if err != nil { return err } @@ -290,6 +302,9 @@ func ScanFiles(root string, cache *GitIgnoreCache, only []string, exclude []stri return nil }) + if err == nil { + err = ctx.Err() + } return files, err } @@ -302,23 +317,42 @@ type Filters struct { // ScanConfiguredFiles scans using the active setup root's project filters. func ScanConfiguredFiles(root string, cache *GitIgnoreCache) ([]FileInfo, error) { + return ScanConfiguredFilesContext(context.Background(), root, cache) +} + +// ScanConfiguredFilesContext scans configured files and honors caller cancellation. +func ScanConfiguredFilesContext(ctx context.Context, root string, cache *GitIgnoreCache) ([]FileInfo, error) { + if err := ctx.Err(); err != nil { + return nil, err + } cfg := config.Load(root) - return ScanFiles(root, cache, cfg.Only, cfg.Exclude) + return ScanFilesContext(ctx, root, cache, cfg.Only, cfg.Exclude) } func filterAnalyses(analyses []FileAnalysis, filters Filters) []FileAnalysis { + filtered, _ := filterAnalysesContext(context.Background(), analyses, filters) + return filtered +} + +func filterAnalysesContext(ctx context.Context, analyses []FileAnalysis, filters Filters) ([]FileAnalysis, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if len(filters.Only) == 0 && len(filters.Exclude) == 0 { - return analyses + return analyses, ctx.Err() } filtered := make([]FileAnalysis, 0, len(analyses)) for _, analysis := range analyses { + if err := ctx.Err(); err != nil { + return nil, err + } path := filepath.ToSlash(analysis.Path) if MatchesFilters(path, filepath.Ext(path), filters.Only, filters.Exclude) { filtered = append(filtered, analysis) } } - return filtered + return filtered, ctx.Err() } func filterConfiguredAnalyses(root string, analyses []FileAnalysis) []FileAnalysis { @@ -328,12 +362,28 @@ func filterConfiguredAnalyses(root string, analyses []FileAnalysis) []FileAnalys // ScanForDeps uses the configured project filters for batched dependency analysis. func ScanForDeps(root string) ([]FileAnalysis, error) { + return ScanForDepsContext(context.Background(), root) +} + +// ScanForDepsContext performs configured dependency analysis with caller cancellation. +func ScanForDepsContext(ctx context.Context, root string) ([]FileAnalysis, error) { + if err := ctx.Err(); err != nil { + return nil, err + } cfg := config.Load(root) - return ScanForDepsWithFilters(root, Filters{Only: cfg.Only, Exclude: cfg.Exclude}) + return ScanForDepsWithFiltersContext(ctx, root, Filters{Only: cfg.Only, Exclude: cfg.Exclude}) } // ScanForDepsWithFilters uses ast-grep for batched dependency analysis with explicit filters. func ScanForDepsWithFilters(root string, filters Filters) ([]FileAnalysis, error) { + return ScanForDepsWithFiltersContext(context.Background(), root, filters) +} + +// ScanForDepsWithFiltersContext performs dependency analysis with explicit filters and caller cancellation. +func ScanForDepsWithFiltersContext(ctx context.Context, root string, filters Filters) ([]FileAnalysis, error) { + if err := ctx.Err(); err != nil { + return nil, err + } astScanner, err := NewAstGrepScanner() if err != nil { return nil, err @@ -344,9 +394,9 @@ func ScanForDepsWithFilters(root string, filters Filters) ([]FileAnalysis, error return nil, ErrAstGrepNotFound } - analyses, err := astScanner.ScanDirectory(root) + analyses, err := astScanner.ScanDirectoryContext(ctx, root) if err != nil { return nil, err } - return filterAnalyses(analyses, filters), nil + return filterAnalysesContext(ctx, analyses, filters) }