diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml index e1ddf66d..809cbb10 100644 --- a/.github/workflows/security-audit.yml +++ b/.github/workflows/security-audit.yml @@ -99,7 +99,8 @@ jobs: # G302: File permissions (0644 is standard for config/log files) # G304: File path from variable (paths come from trusted sources) # G306: File write permissions (same as G302) - exclude_rules: 'G104,G301,G302,G304,G306' + # G703: Path traversal taint analysis (same as G304 but taint-based; all paths are internal) + exclude_rules: 'G104,G301,G302,G304,G306,G703' sast-python: name: Python SAST diff --git a/cmd/ckb/daemon.go b/cmd/ckb/daemon.go index 446958c8..5e19c6a5 100644 --- a/cmd/ckb/daemon.go +++ b/cmd/ckb/daemon.go @@ -226,7 +226,7 @@ func runDaemonBackground() error { return fmt.Errorf("failed to create daemon directory: %w", dirErr) } - logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) // #nosec G703 -- path is internally constructed if err != nil { return fmt.Errorf("failed to open log file: %w", err) } @@ -324,7 +324,7 @@ func runDaemonLogs(cmd *cobra.Command, args []string) error { } func showLastLines(path string, n int) error { - file, err := os.Open(path) + file, err := os.Open(path) // #nosec G703 -- path is internally constructed if err != nil { return err } @@ -349,7 +349,7 @@ func showLastLines(path string, n int) error { func followLogs(path string) error { // Open file - file, err := os.Open(path) + file, err := os.Open(path) // #nosec G703 -- path is internally constructed if err != nil { return err } diff --git a/cmd/ckb/diag.go b/cmd/ckb/diag.go index f4507f89..53451ca5 100644 --- a/cmd/ckb/diag.go +++ b/cmd/ckb/diag.go @@ -181,7 +181,7 @@ func sanitizeConfig(cfg *config.Config) *config.Config { // createDiagnosticZip creates a zip file with diagnostic information func createDiagnosticZip(bundle *DiagnosticBundle, outPath string) error { // Create output file - outFile, err := os.Create(outPath) + outFile, err := os.Create(outPath) // #nosec G703 -- path from CLI flag if err != nil { return fmt.Errorf("failed to create output file: %w", err) } diff --git a/cmd/ckb/diff.go b/cmd/ckb/diff.go index c9bf455f..95625203 100644 --- a/cmd/ckb/diff.go +++ b/cmd/ckb/diff.go @@ -114,7 +114,7 @@ func runDiff(cmd *cobra.Command, args []string) { } if diffOutputPath != "" { - if err := os.WriteFile(diffOutputPath, data, 0644); err != nil { + if err := os.WriteFile(diffOutputPath, data, 0644); err != nil { // #nosec G703 -- non-sensitive output file fmt.Fprintf(os.Stderr, "Error writing output: %v\n", err) os.Exit(1) } @@ -140,7 +140,7 @@ func runDiff(cmd *cobra.Command, args []string) { func runDiffValidate(path string, logger *slog.Logger) { // Read delta file - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) // #nosec G703 -- path from CLI arg if err != nil { fmt.Fprintf(os.Stderr, "Error reading delta file: %v\n", err) os.Exit(1) diff --git a/cmd/ckb/log.go b/cmd/ckb/log.go index 768d98dc..6a3514a7 100644 --- a/cmd/ckb/log.go +++ b/cmd/ckb/log.go @@ -170,7 +170,7 @@ func showLogLines(path string, n int) error { } func followLogFile(path string) error { - file, err := os.Open(path) + file, err := os.Open(path) // #nosec G703 -- path is internally constructed if err != nil { return err } diff --git a/cmd/ckb/mcp.go b/cmd/ckb/mcp.go index 65a95d9e..78268fc3 100644 --- a/cmd/ckb/mcp.go +++ b/cmd/ckb/mcp.go @@ -118,10 +118,6 @@ func runMCP(cmd *cobra.Command, args []string) error { repoRoot = entry.Path repoName = mcpRepo fmt.Fprintf(os.Stderr, "Repository: %s (%s) [%s]\n", repoName, repoRoot, state) - - // Skip multi-repo mode - use lazy loading path instead - // TODO: Add lazy loading support to multi-repo mode - _ = registry // silence unused warning } } else { // No --repo flag - use smart resolution @@ -160,9 +156,6 @@ func runMCP(cmd *cobra.Command, args []string) error { } } - // Skip multi-repo mode for now - use lazy loading path instead - // TODO: Add lazy loading support to multi-repo mode - _ = repos.LoadRegistry // silence unused warning } else { // No repo found - fall back to current directory repoRoot = mustGetRepoRoot() @@ -370,6 +363,11 @@ func triggerReindex(repoRoot, ckbDir string, trigger index.RefreshTrigger, trigg logger.Error("Failed to save index metadata", "error", err.Error()) } + // Populate incremental tracking tables so subsequent incremental updates work + if project.SupportsIncrementalIndexing(config.Language) { + populateIncrementalTracking(repoRoot, config.Language) + } + logger.Info("Reindex complete", "trigger", string(trigger), "duration", duration.String(), diff --git a/cmd/ckb/refresh.go b/cmd/ckb/refresh.go index 80212590..44b465c6 100644 --- a/cmd/ckb/refresh.go +++ b/cmd/ckb/refresh.go @@ -12,6 +12,7 @@ import ( "github.com/SimplyLiz/CodeMCP/internal/backends/scip" "github.com/SimplyLiz/CodeMCP/internal/config" + "github.com/SimplyLiz/CodeMCP/internal/index" "github.com/SimplyLiz/CodeMCP/internal/repostate" "github.com/spf13/cobra" @@ -208,6 +209,22 @@ func runRefresh(cmd *cobra.Command, args []string) error { RefreshedAt: time.Now(), } + // Update index metadata so freshness check stays in sync + ckbDir := filepath.Join(repoRoot, ".ckb") + meta := &index.IndexMeta{ + CreatedAt: time.Now(), + FileCount: result.FilesIndexed, + Duration: fmt.Sprintf("%dms", result.Duration), + Indexer: "scip-go", + } + if rs != nil { + meta.CommitHash = rs.HeadCommit + meta.RepoStateID = rs.RepoStateID + } + if saveErr := meta.Save(ckbDir); saveErr != nil { + fmt.Fprintf(os.Stderr, "Warning: Could not save index metadata: %v\n", saveErr) + } + return outputRefreshResult(result, refreshFormat, logger) } @@ -249,7 +266,7 @@ func outputRefreshResult(result *RefreshResult, format string, logger *slog.Logg return fmt.Errorf("refresh failed: %s", result.Error) } - return nil + return nil // #nosec G703 -- paths are internally constructed from repo root } func findScipGo() (string, error) { diff --git a/cmd/ckb/setup.go b/cmd/ckb/setup.go index edc25a25..46f74204 100644 --- a/cmd/ckb/setup.go +++ b/cmd/ckb/setup.go @@ -469,7 +469,7 @@ func getConfigPath(toolID string, global bool) string { } } for _, path := range candidates { - if _, err := os.Stat(path); err == nil { + if _, err := os.Stat(path); err == nil { // #nosec G703 -- path is internally constructed return path } } @@ -513,7 +513,7 @@ func writeMcpServersConfigWithEnv(path, command string, args []string, env map[s McpServers: make(map[string]mcpServer), } - if data, err := os.ReadFile(path); err == nil { + if data, err := os.ReadFile(path); err == nil { // #nosec G703 -- path is internally constructed if jsonErr := json.Unmarshal(data, &config); jsonErr != nil { fmt.Printf("Warning: existing config is invalid, will overwrite\n") config.McpServers = make(map[string]mcpServer) @@ -536,7 +536,7 @@ func writeMcpServersConfigWithEnv(path, command string, args []string, env map[s return fmt.Errorf("failed to marshal config: %w", err) } - return os.WriteFile(path, data, 0644) + return os.WriteFile(path, data, 0644) // #nosec G703 -- non-sensitive config file } func writeVSCodeConfig(path, command string, args []string) error { @@ -545,7 +545,7 @@ func writeVSCodeConfig(path, command string, args []string) error { Servers: make(map[string]vsCodeServer), } - if data, err := os.ReadFile(path); err == nil { + if data, err := os.ReadFile(path); err == nil { // #nosec G703 -- path is internally constructed if jsonErr := json.Unmarshal(data, &config); jsonErr != nil { fmt.Printf("Warning: existing config is invalid, will overwrite\n") config.Servers = make(map[string]vsCodeServer) @@ -565,7 +565,7 @@ func writeVSCodeConfig(path, command string, args []string) error { return fmt.Errorf("failed to marshal config: %w", err) } - return os.WriteFile(path, data, 0644) + return os.WriteFile(path, data, 0644) // #nosec G703 -- non-sensitive config file } func writeOpenCodeConfig(path, command string, args []string, useNpx bool) error { @@ -574,7 +574,7 @@ func writeOpenCodeConfig(path, command string, args []string, useNpx bool) error Mcp: make(map[string]openCodeMcpEntry), } - if data, err := os.ReadFile(path); err == nil { + if data, err := os.ReadFile(path); err == nil { // #nosec G703 -- path is internally constructed if jsonErr := json.Unmarshal(data, &config); jsonErr != nil { fmt.Printf("Warning: existing config is invalid, will overwrite\n") config.Mcp = make(map[string]openCodeMcpEntry) @@ -602,13 +602,13 @@ func writeOpenCodeConfig(path, command string, args []string, useNpx bool) error return fmt.Errorf("failed to marshal config: %w", err) } - return os.WriteFile(path, data, 0644) + return os.WriteFile(path, data, 0644) // #nosec G703 -- non-sensitive config file } func writeGrokConfig(path, command string, args []string) error { // Read existing config preserving other fields var raw map[string]json.RawMessage - if data, err := os.ReadFile(path); err == nil { + if data, err := os.ReadFile(path); err == nil { // #nosec G703 -- path is internally constructed if jsonErr := json.Unmarshal(data, &raw); jsonErr != nil { fmt.Printf("Warning: existing config is invalid, will overwrite\n") raw = make(map[string]json.RawMessage) @@ -644,7 +644,7 @@ func writeGrokConfig(path, command string, args []string) error { return fmt.Errorf("failed to marshal config: %w", err) } - return os.WriteFile(path, data, 0644) + return os.WriteFile(path, data, 0644) // #nosec G703 -- non-sensitive config file } func configureGrokGlobal(ckbCommand string, ckbArgs []string) (bool, error) { @@ -836,7 +836,7 @@ func getClaudeMcpConfig() (*claudeConfigEntry, error) { } configPath := filepath.Join(home, ".claude.json") - data, err := os.ReadFile(configPath) + data, err := os.ReadFile(configPath) // #nosec G703 -- path is internally constructed if err != nil { return nil, err // File doesn't exist or can't read } @@ -862,7 +862,7 @@ func getGrokMcpConfig() (*grokMcpEntry, error) { } configPath := filepath.Join(home, ".grok", "user-settings.json") - data, err := os.ReadFile(configPath) + data, err := os.ReadFile(configPath) // #nosec G703 -- path is internally constructed if err != nil { return nil, err // File doesn't exist or can't read } @@ -917,7 +917,7 @@ func getVSCodeGlobalMcpConfig() (*vsCodeMcpEntry, error) { return nil, fmt.Errorf("unsupported platform: %s", runtime.GOOS) } - data, err := os.ReadFile(settingsPath) + data, err := os.ReadFile(settingsPath) // #nosec G703 -- path is internally constructed if err != nil { return nil, err // File doesn't exist or can't read } diff --git a/cmd/ckb/setup_hooks.go b/cmd/ckb/setup_hooks.go index c7d3d3b8..ab2c90bf 100644 --- a/cmd/ckb/setup_hooks.go +++ b/cmd/ckb/setup_hooks.go @@ -92,7 +92,7 @@ func runSetupHooks(cmd *cobra.Command, args []string) { hookContent := buildPreCommitHook(installSecrets, installImpact, existingHook) // Write hook - if err := os.WriteFile(preCommitPath, []byte(hookContent), 0755); err != nil { + if err := os.WriteFile(preCommitPath, []byte(hookContent), 0755); err != nil { // #nosec G703 -- git hook must be executable fmt.Fprintf(os.Stderr, "Error writing pre-commit hook: %v\n", err) os.Exit(1) } diff --git a/cmd/ckb/status.go b/cmd/ckb/status.go index a2851f05..57a162b1 100644 --- a/cmd/ckb/status.go +++ b/cmd/ckb/status.go @@ -278,7 +278,16 @@ func convertStatusResponse(resp *query.StatusResponse) *StatusResponseCLI { // getIndexStatus retrieves index freshness information func getIndexStatus(ckbDir, repoRoot string) *IndexStatusCLI { + // Respect configured index path instead of hardcoding index.scip indexPath := filepath.Join(repoRoot, "index.scip") + if cfg, loadErr := config.LoadConfig(repoRoot); loadErr == nil && cfg.Backends.Scip.IndexPath != "" { + cfgPath := cfg.Backends.Scip.IndexPath + if filepath.IsAbs(cfgPath) { + indexPath = cfgPath + } else { + indexPath = filepath.Join(repoRoot, cfgPath) + } + } // Check if index file exists if _, err := os.Stat(indexPath); os.IsNotExist(err) { @@ -463,7 +472,7 @@ func detectCodeowners(repoRoot string) *CodeownersStatusCLI { for _, relPath := range codeownersLocations { fullPath := filepath.Join(repoRoot, relPath) - content, err := os.ReadFile(fullPath) + content, err := os.ReadFile(fullPath) // #nosec G703 -- path is internally constructed if err == nil { status.Found = true status.Path = relPath @@ -501,20 +510,20 @@ func formatDuration(d time.Duration) string { return "just now" } if d < time.Hour { - mins := int(d.Minutes()) + mins := int(d.Minutes()) // #nosec G115 -- duration fits in int if mins == 1 { return "1 minute ago" } return fmt.Sprintf("%d minutes ago", mins) } if d < 24*time.Hour { - hours := int(d.Hours()) + hours := int(d.Hours()) // #nosec G115 -- duration fits in int if hours == 1 { return "1 hour ago" } return fmt.Sprintf("%d hours ago", hours) } - days := int(d.Hours() / 24) + days := int(d.Hours() / 24) // #nosec G115 -- duration fits in int if days == 1 { return "1 day ago" } diff --git a/cmd/ckb/token.go b/cmd/ckb/token.go index ea3f441a..29d1dcda 100644 --- a/cmd/ckb/token.go +++ b/cmd/ckb/token.go @@ -423,11 +423,11 @@ func formatTimeAgo(t time.Time) string { case d < time.Minute: return "just now" case d < time.Hour: - return fmt.Sprintf("%dm ago", int(d.Minutes())) + return fmt.Sprintf("%dm ago", int(d.Minutes())) // #nosec G115 -- duration fits in int case d < 24*time.Hour: - return fmt.Sprintf("%dh ago", int(d.Hours())) + return fmt.Sprintf("%dh ago", int(d.Hours())) // #nosec G115 -- duration fits in int case d < 7*24*time.Hour: - return fmt.Sprintf("%dd ago", int(d.Hours()/24)) + return fmt.Sprintf("%dd ago", int(d.Hours()/24)) // #nosec G115 -- duration fits in int default: return t.Format("Jan 2") } diff --git a/cmd/ckb/use.go b/cmd/ckb/use.go index 4ebe84cf..069e6ca7 100644 --- a/cmd/ckb/use.go +++ b/cmd/ckb/use.go @@ -174,20 +174,20 @@ func formatRelativeTime(t time.Time) string { return "just now" } if d < time.Hour { - mins := int(d.Minutes()) + mins := int(d.Minutes()) // #nosec G115 -- duration fits in int if mins == 1 { return "1m ago" } return fmt.Sprintf("%dm ago", mins) } if d < 24*time.Hour { - hours := int(d.Hours()) + hours := int(d.Hours()) // #nosec G115 -- duration fits in int if hours == 1 { return "1h ago" } return fmt.Sprintf("%dh ago", hours) } - days := int(d.Hours() / 24) + days := int(d.Hours() / 24) // #nosec G115 -- duration fits in int if days == 1 { return "1d ago" } diff --git a/go.mod b/go.mod index e000a3e9..0f19955b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/SimplyLiz/CodeMCP -go 1.24.12 +go 1.26.0 require ( github.com/BurntSushi/toml v1.6.0 diff --git a/internal/api/handlers_quality.go b/internal/api/handlers_quality.go index 0c5f3f92..f0b9dba5 100644 --- a/internal/api/handlers_quality.go +++ b/internal/api/handlers_quality.go @@ -65,7 +65,7 @@ func (s *Server) handleLanguageQuality(w http.ResponseWriter, r *http.Request) { for lang, lq := range report.Languages { languages[string(lang)] = &LanguageQualityInfo{ DisplayName: lq.DisplayName, - Tier: int(lq.Tier), + Tier: int(lq.Tier), // #nosec G115 -- tier is a small enum value TierName: lq.TierName, Quality: string(lq.Quality), SymbolCount: lq.SymbolCount, diff --git a/internal/api/handlers_upload_delta.go b/internal/api/handlers_upload_delta.go index ab2a54d6..136757ea 100644 --- a/internal/api/handlers_upload_delta.go +++ b/internal/api/handlers_upload_delta.go @@ -148,7 +148,7 @@ func (s *Server) HandleIndexDeltaUpload(w http.ResponseWriter, r *http.Request) // Check if we should suggest full upload based on changed percentage if result.TotalFiles > 0 { changedPercent := float64(len(deltaMeta.ChangedFiles)) / float64(result.TotalFiles) * 100 - if int(changedPercent) > threshold { + if int(changedPercent) > threshold { // #nosec G115 -- percentage 0-100 suggestFull = true suggestReason = fmt.Sprintf("%.1f%% of files changed (threshold: %d%%)", changedPercent, threshold) } diff --git a/internal/api/index_processor.go b/internal/api/index_processor.go index f7690165..132705ec 100644 --- a/internal/api/index_processor.go +++ b/internal/api/index_processor.go @@ -659,13 +659,13 @@ func isCallableSymbol(symbolID string, info map[string]*scip.SymbolInformation) func parseRange(r []int32) (line, col, endCol int) { if len(r) >= 1 { - line = int(r[0]) + 1 // Convert to 1-indexed + line = int(r[0]) + 1 // #nosec G115 -- SCIP int32 fits in int } if len(r) >= 2 { - col = int(r[1]) + 1 + col = int(r[1]) + 1 // #nosec G115 -- SCIP int32 fits in int } if len(r) >= 4 { - endCol = int(r[3]) + 1 + endCol = int(r[3]) + 1 // #nosec G115 -- SCIP int32 fits in int } return } @@ -693,10 +693,10 @@ func buildLocation(path string, occ *scip.Occurrence) string { } if len(occ.Range) >= 1 { - loc["line"] = int(occ.Range[0]) + 1 + loc["line"] = int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 fits in int } if len(occ.Range) >= 2 { - loc["col"] = int(occ.Range[1]) + 1 + loc["col"] = int(occ.Range[1]) + 1 // #nosec G115 -- SCIP int32 fits in int } data, _ := json.Marshal(loc) @@ -728,7 +728,7 @@ func resolveCallerFromDoc(doc *scip.Document, callLine int, info map[string]*sci continue } - line := int(occ.Range[0]) + 1 + line := int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 fits in int // Find the closest function definition before the call if line <= callLine && line > bestLine { bestMatch = occ.Symbol diff --git a/internal/api/index_queries.go b/internal/api/index_queries.go index add7ad2e..34aaeaef 100644 --- a/internal/api/index_queries.go +++ b/internal/api/index_queries.go @@ -403,7 +403,7 @@ func (h *IndexRepoHandle) QueryCallgraph(cursor *CursorData, limit int, filters edge.CallerID = callerID.String } if endCol.Valid { - edge.EndCol = int(endCol.Int64) + edge.EndCol = int(endCol.Int64) // #nosec G115 -- column number fits in int } edges = append(edges, edge) @@ -494,7 +494,7 @@ func (h *IndexRepoHandle) QueryRefs(cursor *CursorData, limit int, filters RefFi Language: detectLanguage(fromFile), } if endCol.Valid { - ref.EndCol = int(endCol.Int64) + ref.EndCol = int(endCol.Int64) // #nosec G115 -- column number fits in int } refs = append(refs, ref) diff --git a/internal/api/index_storage.go b/internal/api/index_storage.go index 5950e602..c2a56e5a 100644 --- a/internal/api/index_storage.go +++ b/internal/api/index_storage.go @@ -210,7 +210,7 @@ func (s *IndexStorage) SaveMeta(repoID string, meta *RepoMeta) error { return fmt.Errorf("failed to marshal metadata: %w", err) } - if err := os.WriteFile(s.MetaPath(repoID), data, 0644); err != nil { + if err := os.WriteFile(s.MetaPath(repoID), data, 0644); err != nil { // #nosec G703 -- non-sensitive metadata return fmt.Errorf("failed to write metadata: %w", err) } @@ -254,7 +254,7 @@ func (s *IndexStorage) CleanupUpload(path string) error { if !strings.HasPrefix(path, s.uploadDir) { return fmt.Errorf("invalid upload path: %s", path) } - return os.Remove(path) + return os.Remove(path) // #nosec G703 -- path validated against uploadDir above } // CleanupOldUploads removes uploads older than the given duration diff --git a/internal/auth/store.go b/internal/auth/store.go index bb0aa1b9..b321f79c 100644 --- a/internal/auth/store.go +++ b/internal/auth/store.go @@ -353,7 +353,7 @@ func (s *KeyStore) scanKey(row *sql.Row) (*APIKey, error) { // Parse optional fields if rateLimit.Valid { - rl := int(rateLimit.Int64) + rl := int(rateLimit.Int64) // #nosec G115 -- rate limit is a small positive integer key.RateLimit = &rl } @@ -438,7 +438,7 @@ func (s *KeyStore) scanKeys(rows *sql.Rows) ([]*APIKey, error) { // Parse optional fields if rateLimit.Valid { - rl := int(rateLimit.Int64) + rl := int(rateLimit.Int64) // #nosec G115 -- rate limit is a small positive integer key.RateLimit = &rl } diff --git a/internal/backends/scip/callgraph.go b/internal/backends/scip/callgraph.go index e21a3005..17443868 100644 --- a/internal/backends/scip/callgraph.go +++ b/internal/backends/scip/callgraph.go @@ -63,7 +63,7 @@ func (idx *SCIPIndex) FindCallees(symbolId string) ([]*CallGraphNode, error) { for _, occ := range doc.Occurrences { if occ.Symbol == symbolId && occ.SymbolRoles&SymbolRoleDefinition != 0 { funcDoc = doc - funcDefLine = int(occ.Range[0]) + funcDefLine = int(occ.Range[0]) // #nosec G115 -- SCIP int32 fits in int break } } @@ -97,7 +97,7 @@ func (idx *SCIPIndex) FindCallees(symbolId string) ([]*CallGraphNode, error) { continue } - occLine := int(occ.Range[0]) + occLine := int(occ.Range[0]) // #nosec G115 -- SCIP int32 fits in int // Check if this occurrence is within the function's body if occLine < funcRange.start || occLine > funcRange.end { @@ -155,7 +155,7 @@ func (idx *SCIPIndex) FindCallers(symbolId string) ([]*CallGraphNode, error) { continue } - occLine := int(occ.Range[0]) + occLine := int(occ.Range[0]) // #nosec G115 -- SCIP int32 fits in int // Find which function contains this occurrence for funcSymbol, lineRange := range funcRanges { @@ -216,7 +216,7 @@ func buildFunctionRanges(doc *Document) map[string]lineRange { if occ.Symbol == sym.Symbol && occ.SymbolRoles&SymbolRoleDefinition != 0 { funcs = append(funcs, funcDef{ symbol: sym.Symbol, - startLine: int(occ.Range[0]), + startLine: int(occ.Range[0]), // #nosec G115 -- SCIP int32 fits in int }) break } diff --git a/internal/backends/scip/symbols.go b/internal/backends/scip/symbols.go index a941cd21..3be0d996 100644 --- a/internal/backends/scip/symbols.go +++ b/internal/backends/scip/symbols.go @@ -233,18 +233,18 @@ func parseOccurrenceRange(occ *Occurrence, filePath string) *Location { // SCIP range format: [startLine, startChar, endChar] for single-line // or [startLine, startChar, endLine, endChar] for multi-line - startLine := int(occ.Range[0]) - startColumn := int(occ.Range[1]) + startLine := int(occ.Range[0]) // #nosec G115 -- SCIP int32 fits in int + startColumn := int(occ.Range[1]) // #nosec G115 -- SCIP int32 fits in int var endLine, endColumn int if len(occ.Range) == 3 { // Single-line range endLine = startLine - endColumn = int(occ.Range[2]) + endColumn = int(occ.Range[2]) // #nosec G115 -- SCIP int32 fits in int } else if len(occ.Range) >= 4 { // Multi-line range - endLine = int(occ.Range[2]) - endColumn = int(occ.Range[3]) + endLine = int(occ.Range[2]) // #nosec G115 -- SCIP int32 fits in int + endColumn = int(occ.Range[3]) // #nosec G115 -- SCIP int32 fits in int } return &Location{ diff --git a/internal/compression/truncation.go b/internal/compression/truncation.go index e801f998..45d5d5c6 100644 --- a/internal/compression/truncation.go +++ b/internal/compression/truncation.go @@ -1,5 +1,7 @@ package compression +import "strconv" + // TruncationReason indicates why data was truncated in a response type TruncationReason string @@ -72,5 +74,5 @@ func (t *TruncationInfo) String() string { return "no truncation" } - return string(t.Reason) + ": dropped " + string(rune(t.DroppedCount)) + " of " + string(rune(t.OriginalCount)) + " items" + return string(t.Reason) + ": dropped " + strconv.Itoa(t.DroppedCount) + " of " + strconv.Itoa(t.OriginalCount) + " items" } diff --git a/internal/config/config.go b/internal/config/config.go index 4acfd5c2..2e78dcc3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -525,7 +525,7 @@ func LoadConfigWithDetails(repoRoot string) (*LoadResult, error) { // loadConfigFromPath loads a config file from a specific path func loadConfigFromPath(path string) (*Config, error) { - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) // #nosec G703 -- path is internally constructed if err != nil { return nil, err } diff --git a/internal/diff/scipadapter.go b/internal/diff/scipadapter.go index 9a0b5933..a1060c24 100644 --- a/internal/diff/scipadapter.go +++ b/internal/diff/scipadapter.go @@ -86,21 +86,21 @@ func convertOccurrence(occ *scip.Occurrence) *OccurrenceInfo { return nil } - startLine := int(occ.Range[0]) + 1 // Convert to 1-indexed + startLine := int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 fits in int endLine := startLine startCol := 0 endCol := 0 if len(occ.Range) >= 2 { - startCol = int(occ.Range[1]) + startCol = int(occ.Range[1]) // #nosec G115 -- SCIP int32 fits in int } if len(occ.Range) >= 3 { - endCol = int(occ.Range[2]) + endCol = int(occ.Range[2]) // #nosec G115 -- SCIP int32 fits in int // If only 3 elements, end is on same line } if len(occ.Range) >= 4 { - endLine = int(occ.Range[2]) + 1 // Convert to 1-indexed - endCol = int(occ.Range[3]) + endLine = int(occ.Range[2]) + 1 // #nosec G115 -- SCIP int32 fits in int + endCol = int(occ.Range[3]) // #nosec G115 -- SCIP int32 fits in int } isDefinition := (occ.SymbolRoles & scip.SymbolRoleDefinition) != 0 @@ -126,13 +126,13 @@ func convertSymbolDef(sym *scip.SymbolInformation, doc *scip.Document) *SymbolDe for _, occ := range doc.Occurrences { if occ.Symbol == sym.Symbol && (occ.SymbolRoles&scip.SymbolRoleDefinition) != 0 { if len(occ.Range) >= 1 { - startLine = int(occ.Range[0]) + 1 // Convert to 1-indexed + startLine = int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 fits in int } // Use enclosing range for end line if available if len(occ.EnclosingRange) >= 3 { - endLine = int(occ.EnclosingRange[2]) + 1 // Convert to 1-indexed + endLine = int(occ.EnclosingRange[2]) + 1 // #nosec G115 -- SCIP int32 fits in int } else if len(occ.Range) >= 3 { - endLine = int(occ.Range[2]) + 1 + endLine = int(occ.Range[2]) + 1 // #nosec G115 -- SCIP int32 fits in int } else { endLine = startLine + 10 // Default assumption for body } diff --git a/internal/federation/queries.go b/internal/federation/queries.go index 3803d305..5b687ac4 100644 --- a/internal/federation/queries.go +++ b/internal/federation/queries.go @@ -321,7 +321,7 @@ func (f *Federation) GetHotspots(opts GetHotspotsOptions) (*GetHotspotsResult, e } if churn.Valid { - h.ChurnCommits30d = int(churn.Int64) + h.ChurnCommits30d = int(churn.Int64) // #nosec G115 -- commit count fits in int } if complexity.Valid { h.ComplexityCyclomatic = complexity.Float64 diff --git a/internal/graph/builder.go b/internal/graph/builder.go index 8e66a037..32abc459 100644 --- a/internal/graph/builder.go +++ b/internal/graph/builder.go @@ -44,8 +44,8 @@ func BuildFromSCIP(ctx context.Context, idx *scip.SCIPIndex, weights EdgeWeights if occ.SymbolRoles&scip.SymbolRoleDefinition != 0 { symbolDefs[occ.Symbol] = &scip.Location{ FileId: doc.RelativePath, - StartLine: int(occ.Range[0]), - EndLine: int(occ.Range[0]), + StartLine: int(occ.Range[0]), // #nosec G115 -- SCIP int32 fits in int + EndLine: int(occ.Range[0]), // #nosec G115 -- SCIP int32 fits in int } } } diff --git a/internal/incremental/extractor.go b/internal/incremental/extractor.go index fadff79b..5c8baecc 100644 --- a/internal/incremental/extractor.go +++ b/internal/incremental/extractor.go @@ -200,10 +200,10 @@ func (e *SCIPExtractor) extractFileDelta(doc *scip.Document, change ChangedFile) // Parse range (SCIP is 0-indexed, we use 1-indexed) if len(occ.Range) >= 1 { - sym.StartLine = int(occ.Range[0]) + 1 + sym.StartLine = int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } if len(occ.Range) >= 3 { - sym.EndLine = int(occ.Range[2]) + 1 + sym.EndLine = int(occ.Range[2]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } else { sym.EndLine = sym.StartLine } @@ -242,7 +242,7 @@ func (e *SCIPExtractor) extractFileDelta(doc *scip.Document, change ChangedFile) } if len(occ.Range) >= 1 { - ref.FromLine = int(occ.Range[0]) + 1 + ref.FromLine = int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } delta.Refs = append(delta.Refs, ref) @@ -273,13 +273,13 @@ func (e *SCIPExtractor) extractFileDelta(doc *scip.Document, change ChangedFile) // Parse location (SCIP is 0-indexed, we use 1-indexed) if len(occ.Range) >= 1 { - edge.Line = int(occ.Range[0]) + 1 + edge.Line = int(occ.Range[0]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } if len(occ.Range) >= 2 { - edge.Column = int(occ.Range[1]) + 1 + edge.Column = int(occ.Range[1]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } if len(occ.Range) >= 4 { - edge.EndColumn = int(occ.Range[3]) + 1 + edge.EndColumn = int(occ.Range[3]) + 1 // #nosec G115 -- SCIP int32 coordinates fit in int } // Resolve caller symbol (may be empty for top-level calls) diff --git a/internal/incremental/store.go b/internal/incremental/store.go index 7b6aa227..56e987db 100644 --- a/internal/incremental/store.go +++ b/internal/incremental/store.go @@ -251,7 +251,7 @@ func (s *Store) SetLastIndexedCommit(commit string) error { // GetSchemaVersion returns the stored schema version func (s *Store) GetSchemaVersion() int { - return int(s.GetMetaInt(MetaKeySchemaVersion)) + return int(s.GetMetaInt(MetaKeySchemaVersion)) // #nosec G115 -- schema version is a small integer } // GetIndexState retrieves the full index state for display @@ -265,7 +265,7 @@ func (s *Store) GetIndexState() IndexState { state.LastFull = s.GetMetaInt(MetaKeyLastFull) state.LastIncremental = s.GetMetaInt(MetaKeyLastIncremental) - state.FilesSinceFull = int(s.GetMetaInt(MetaKeyFilesSinceFull)) + state.FilesSinceFull = int(s.GetMetaInt(MetaKeyFilesSinceFull)) // #nosec G115 -- file count fits in int state.Commit = s.GetLastIndexedCommit() state.State = baseState diff --git a/internal/index/lock.go b/internal/index/lock.go index 33d1f85e..49e6990a 100644 --- a/internal/index/lock.go +++ b/internal/index/lock.go @@ -34,7 +34,7 @@ func AcquireLock(ckbDir string) (*Lock, error) { } // Try to acquire exclusive lock (non-blocking) - err = syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) + err = syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) // #nosec G115 -- fd fits in int if err != nil { _ = file.Close() @@ -48,19 +48,19 @@ func AcquireLock(ckbDir string) (*Lock, error) { // Write our PID to the lock file if err := file.Truncate(0); err != nil { - _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) + _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) // #nosec G115 -- fd fits in int _ = file.Close() return nil, fmt.Errorf("truncating lock file: %w", err) } if _, err := file.Seek(0, 0); err != nil { - _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) + _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) // #nosec G115 -- fd fits in int _ = file.Close() return nil, fmt.Errorf("seeking lock file: %w", err) } if _, err := file.WriteString(strconv.Itoa(os.Getpid())); err != nil { - _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) + _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) // #nosec G115 -- fd fits in int _ = file.Close() return nil, fmt.Errorf("writing PID to lock file: %w", err) } @@ -75,7 +75,7 @@ func (l *Lock) Release() { } // Release the flock - _ = syscall.Flock(int(l.file.Fd()), syscall.LOCK_UN) + _ = syscall.Flock(int(l.file.Fd()), syscall.LOCK_UN) // #nosec G115 -- fd fits in int // Close the file _ = l.file.Close() diff --git a/internal/mcp/engine_cache.go b/internal/mcp/engine_cache.go new file mode 100644 index 00000000..78a46a5f --- /dev/null +++ b/internal/mcp/engine_cache.go @@ -0,0 +1,119 @@ +package mcp + +import ( + "path/filepath" + "sync" + "time" +) + +// getOrCreateEngine returns a cached engine for the given repo root, creating one if needed. +// Thread-safe: uses s.mu for synchronization. +func (s *MCPServer) getOrCreateEngine(repoRoot string) (*engineEntry, error) { + normalized := normalizePath(repoRoot) + + // Fast path: check cache with read lock + s.mu.RLock() + if entry, ok := s.engines[normalized]; ok { + entry.lastUsed = time.Now() + s.mu.RUnlock() + return entry, nil + } + s.mu.RUnlock() + + // Slow path: upgrade to write lock, double-check, create + s.mu.Lock() + defer s.mu.Unlock() + + // Double-check after acquiring write lock + if entry, ok := s.engines[normalized]; ok { + entry.lastUsed = time.Now() + return entry, nil + } + + // Evict LRU if at capacity + if len(s.engines) >= maxEngines { + s.evictLRULocked() + } + + // Create new engine + engine, err := s.createEngineForRoot(normalized) + if err != nil { + return nil, err + } + + entry := &engineEntry{ + engine: engine, + repoPath: normalized, + repoName: filepath.Base(normalized), + loadedAt: time.Now(), + lastUsed: time.Now(), + } + s.engines[normalized] = entry + + s.logger.Info("Created engine for repo", + "path", normalized, + "totalLoaded", len(s.engines), + ) + + return entry, nil +} + +// ensureActiveEngine switches the active engine to the repo at repoRoot, if needed. +// No-op if repoRoot is empty or already the active repo. +// MCP over stdio is sequential, so no race on legacyEngine. +func (s *MCPServer) ensureActiveEngine(repoRoot string) error { + if repoRoot == "" { + return nil + } + + normalized := normalizePath(repoRoot) + + // Check if current engine already points here + if eng := s.engine(); eng != nil { + currentRoot := normalizePath(eng.GetRepoRoot()) + if currentRoot == normalized { + return nil + } + } + + entry, err := s.getOrCreateEngine(normalized) + if err != nil { + s.logger.Warn("Auto-resolve failed, keeping current engine", + "targetRoot", normalized, + "error", err.Error(), + ) + return err + } + + // Swap the active engine pointer + s.mu.Lock() + s.legacyEngine = entry.engine + s.activeRepo = entry.repoName + s.activeRepoPath = entry.repoPath + s.engineOnce = sync.Once{} // mark as loaded + s.engineErr = nil + s.mu.Unlock() + + // Wire up metrics persistence + if entry.engine.DB() != nil { + SetMetricsDB(entry.engine.DB()) + } + + s.logger.Info("Auto-resolved active repo", + "repo", entry.repoName, + "path", entry.repoPath, + ) + + return nil +} + +// normalizePath cleans and resolves symlinks for a path. +// Always returns a usable path — falls back to filepath.Clean if symlink resolution fails. +func normalizePath(path string) string { + cleaned := filepath.Clean(path) + resolved, err := filepath.EvalSymlinks(cleaned) + if err != nil { + return cleaned + } + return resolved +} diff --git a/internal/mcp/engine_cache_test.go b/internal/mcp/engine_cache_test.go new file mode 100644 index 00000000..e4e93210 --- /dev/null +++ b/internal/mcp/engine_cache_test.go @@ -0,0 +1,213 @@ +package mcp + +import ( + "log/slog" + "os" + "path/filepath" + "testing" + "time" +) + +func TestEnsureActiveEngine_EmptyRoot(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + server := &MCPServer{ + logger: logger, + engines: make(map[string]*engineEntry), + roots: newRootsManager(), + } + + // Should be a no-op for empty root + err := server.ensureActiveEngine("") + if err != nil { + t.Errorf("ensureActiveEngine('') returned error: %v", err) + } +} + +func TestEnsureActiveEngine_SameRepoNoSwitch(t *testing.T) { + // Test that ensureActiveEngine is a no-op when current engine points to same repo + // This tests the early return path, not the full engine creation + + tmpDir, err := os.MkdirTemp("", "engine-cache-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + + // Create server without legacyEngine - ensureActiveEngine will try to create one + // and fail, but this tests the path normalization logic + server := &MCPServer{ + logger: logger, + engines: make(map[string]*engineEntry), + roots: newRootsManager(), + } + + // This will fail because there's no .ckb directory, but won't panic + err = server.ensureActiveEngine(tmpDir) + + // Error expected - we can't create a real engine without setup + // Just verify it doesn't panic and returns an error gracefully + if err == nil { + t.Log("ensureActiveEngine succeeded (temp dir may be in a git repo)") + } +} + +func TestGetOrCreateEngine_CacheHit(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "engine-cache-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + + normalized := normalizePath(tmpDir) + originalTime := time.Now().Add(-time.Hour) + entry := &engineEntry{ + repoPath: normalized, + repoName: filepath.Base(normalized), + loadedAt: originalTime, + lastUsed: originalTime, + } + + server := &MCPServer{ + logger: logger, + engines: map[string]*engineEntry{normalized: entry}, + roots: newRootsManager(), + } + + // Should hit cache and update lastUsed + result, err := server.getOrCreateEngine(tmpDir) + if err != nil { + t.Fatalf("getOrCreateEngine returned error: %v", err) + } + + if result != entry { + t.Error("getOrCreateEngine should return cached entry") + } + + if !result.lastUsed.After(originalTime) { + t.Error("getOrCreateEngine should update lastUsed timestamp") + } +} + +func TestGetOrCreateEngine_NormalizedPath(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "engine-cache-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + + // Store with normalized path + normalized := normalizePath(tmpDir) + entry := &engineEntry{ + repoPath: normalized, + repoName: filepath.Base(normalized), + loadedAt: time.Now(), + lastUsed: time.Now(), + } + + server := &MCPServer{ + logger: logger, + engines: map[string]*engineEntry{normalized: entry}, + roots: newRootsManager(), + } + + // Query with unnormalized path (trailing slash, etc.) + pathWithSlash := tmpDir + "/" + result, err := server.getOrCreateEngine(pathWithSlash) + if err != nil { + t.Fatalf("getOrCreateEngine returned error: %v", err) + } + + if result != entry { + t.Error("getOrCreateEngine should find entry regardless of trailing slash") + } +} + +func TestEvictLRULocked_PreservesActiveRepo(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + + now := time.Now() + activeEntry := &engineEntry{ + repoPath: "/active/repo", + repoName: "active", + lastUsed: now.Add(-time.Hour), // Oldest, but should not be evicted + } + otherEntry := &engineEntry{ + repoPath: "/other/repo", + repoName: "other", + lastUsed: now, // Newer + } + + server := &MCPServer{ + logger: logger, + engines: map[string]*engineEntry{ + "/active/repo": activeEntry, + "/other/repo": otherEntry, + }, + activeRepoPath: "/active/repo", + roots: newRootsManager(), + } + + // Evict should remove other, not active (even though active is older) + server.evictLRULocked() + + if _, ok := server.engines["/active/repo"]; !ok { + t.Error("evictLRULocked should not evict active repo") + } + + if _, ok := server.engines["/other/repo"]; ok { + t.Error("evictLRULocked should evict non-active repo") + } +} + +func TestEvictLRULocked_EvictsOldest(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + + now := time.Now() + entry1 := &engineEntry{ + repoPath: "/repo1", + repoName: "repo1", + lastUsed: now.Add(-2 * time.Hour), // Oldest + } + entry2 := &engineEntry{ + repoPath: "/repo2", + repoName: "repo2", + lastUsed: now.Add(-time.Hour), + } + entry3 := &engineEntry{ + repoPath: "/repo3", + repoName: "repo3", + lastUsed: now, // Newest + } + + server := &MCPServer{ + logger: logger, + engines: map[string]*engineEntry{ + "/repo1": entry1, + "/repo2": entry2, + "/repo3": entry3, + }, + activeRepoPath: "/repo3", // Active is newest + roots: newRootsManager(), + } + + server.evictLRULocked() + + // repo1 should be evicted (oldest non-active) + if _, ok := server.engines["/repo1"]; ok { + t.Error("evictLRULocked should evict oldest repo") + } + + // repo2 and repo3 should remain + if _, ok := server.engines["/repo2"]; !ok { + t.Error("repo2 should not be evicted") + } + if _, ok := server.engines["/repo3"]; !ok { + t.Error("repo3 should not be evicted") + } +} diff --git a/internal/mcp/handler.go b/internal/mcp/handler.go index 1ae8bdea..58621403 100644 --- a/internal/mcp/handler.go +++ b/internal/mcp/handler.go @@ -338,6 +338,13 @@ func (s *MCPServer) handleCallTool(params map[string]interface{}) (interface{}, "params", toolParams, ) + // v8.1: Auto-resolve active repository from file paths in params + if pathHint := extractPathHint(toolParams); pathHint != "" { + if repoRoot := s.resolveRepoForPath(pathHint); repoRoot != "" { + _ = s.ensureActiveEngine(repoRoot) + } + } + // v8.0: Check for streaming request if streamResp, err := s.wrapForStreaming(toolName, toolParams); streamResp != nil || err != nil { if err != nil { diff --git a/internal/mcp/repo_resolver.go b/internal/mcp/repo_resolver.go new file mode 100644 index 00000000..29f35f66 --- /dev/null +++ b/internal/mcp/repo_resolver.go @@ -0,0 +1,68 @@ +package mcp + +import ( + "os" + "path/filepath" + "strings" + + "github.com/SimplyLiz/CodeMCP/internal/repos" +) + +// pathParams is the ordered list of tool parameter names that may contain file paths. +// We check these in priority order and return the first path-like value found. +var pathParams = []string{"filePath", "path", "targetPath", "target", "moduleId"} + +// extractPathHint extracts a file path hint from tool parameters. +// Returns the first path-like value found, or "" if none. +func extractPathHint(toolParams map[string]interface{}) string { + for _, key := range pathParams { + val, ok := toolParams[key].(string) + if !ok || val == "" { + continue + } + + // "target" is overloaded: sometimes a symbol name like "MCPServer.GetEngine". + // Only treat it as a path if it contains a path separator. + if key == "target" && !strings.Contains(val, "/") && !strings.Contains(val, "\\") { + continue + } + + return val + } + return "" +} + +// resolveRepoForPath resolves a path hint to a git repo root. +// Returns the repo root path or "" if the hint cannot be resolved. +func (s *MCPServer) resolveRepoForPath(pathHint string) string { + if pathHint == "" { + return "" + } + + // Absolute path: resolve directly + if filepath.IsAbs(pathHint) { + return repos.FindGitRoot(pathHint) + } + + // Relative path: try against each client root + for _, root := range s.GetRootPaths() { + candidate := filepath.Join(root, pathHint) + if _, err := os.Stat(candidate); err == nil { + if gitRoot := repos.FindGitRoot(candidate); gitRoot != "" { + return gitRoot + } + } + } + + // Try against current engine's repo root + if eng := s.engine(); eng != nil { + candidate := filepath.Join(eng.GetRepoRoot(), pathHint) + if _, err := os.Stat(candidate); err == nil { + if gitRoot := repos.FindGitRoot(candidate); gitRoot != "" { + return gitRoot + } + } + } + + return "" +} diff --git a/internal/mcp/repo_resolver_test.go b/internal/mcp/repo_resolver_test.go new file mode 100644 index 00000000..0c4f366a --- /dev/null +++ b/internal/mcp/repo_resolver_test.go @@ -0,0 +1,220 @@ +package mcp + +import ( + "os" + "path/filepath" + "testing" +) + +func TestExtractPathHint(t *testing.T) { + tests := []struct { + name string + params map[string]interface{} + expected string + }{ + { + name: "empty params", + params: map[string]interface{}{}, + expected: "", + }, + { + name: "filePath param", + params: map[string]interface{}{"filePath": "internal/mcp/server.go"}, + expected: "internal/mcp/server.go", + }, + { + name: "path param", + params: map[string]interface{}{"path": "cmd/ckb/main.go"}, + expected: "cmd/ckb/main.go", + }, + { + name: "targetPath param", + params: map[string]interface{}{"targetPath": "/absolute/path/file.go"}, + expected: "/absolute/path/file.go", + }, + { + name: "target with path separator treated as path", + params: map[string]interface{}{"target": "internal/query/engine.go"}, + expected: "internal/query/engine.go", + }, + { + name: "target without separator skipped (symbol name)", + params: map[string]interface{}{"target": "MCPServer.GetEngine"}, + expected: "", + }, + { + name: "moduleId param", + params: map[string]interface{}{"moduleId": "internal/mcp"}, + expected: "internal/mcp", + }, + { + name: "priority order - filePath wins", + params: map[string]interface{}{"filePath": "first.go", "path": "second.go"}, + expected: "first.go", + }, + { + name: "empty string value skipped", + params: map[string]interface{}{"filePath": "", "path": "fallback.go"}, + expected: "fallback.go", + }, + { + name: "non-string value skipped", + params: map[string]interface{}{"filePath": 123, "path": "fallback.go"}, + expected: "fallback.go", + }, + { + name: "target with backslash treated as path", + params: map[string]interface{}{"target": "internal\\mcp\\server.go"}, + expected: "internal\\mcp\\server.go", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractPathHint(tt.params) + if result != tt.expected { + t.Errorf("extractPathHint(%v) = %q, want %q", tt.params, result, tt.expected) + } + }) + } +} + +func TestNormalizePath(t *testing.T) { + tests := []struct { + name string + input string + }{ + { + name: "simple path", + input: "/Users/test/project", + }, + { + name: "path with dots", + input: "/Users/test/../test/project", + }, + { + name: "path with double slashes", + input: "/Users//test/project", + }, + { + name: "relative path", + input: "internal/mcp", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizePath(tt.input) + // Should return a cleaned path + if result == "" { + t.Errorf("normalizePath(%q) returned empty string", tt.input) + } + // Should be cleaned (no double slashes, no . or ..) + cleaned := filepath.Clean(tt.input) + // Result should be at least as clean as filepath.Clean + if filepath.Clean(result) != result { + t.Errorf("normalizePath(%q) = %q is not clean", tt.input, result) + } + _ = cleaned // used for documentation + }) + } +} + +func TestNormalizePath_NonexistentPath(t *testing.T) { + // normalizePath should handle nonexistent paths gracefully + result := normalizePath("/nonexistent/path/that/does/not/exist") + if result == "" { + t.Error("normalizePath should return cleaned path even for nonexistent paths") + } + expected := filepath.Clean("/nonexistent/path/that/does/not/exist") + if result != expected { + t.Errorf("normalizePath returned %q, expected %q", result, expected) + } +} + +func TestNormalizePath_Symlink(t *testing.T) { + // Create a temp directory with a symlink + tmpDir, err := os.MkdirTemp("", "normalizepath-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create actual directory + actualDir := filepath.Join(tmpDir, "actual") + if err := os.Mkdir(actualDir, 0755); err != nil { + t.Fatalf("Failed to create actual dir: %v", err) + } + + // Create symlink + linkDir := filepath.Join(tmpDir, "link") + if err := os.Symlink(actualDir, linkDir); err != nil { + t.Skipf("Symlinks not supported: %v", err) + } + + // normalizePath should resolve the symlink + result := normalizePath(linkDir) + + // Result should point to actual directory (after resolving symlinks) + resultResolved, _ := filepath.EvalSymlinks(result) + actualResolved, _ := filepath.EvalSymlinks(actualDir) + + if resultResolved != actualResolved { + t.Errorf("normalizePath(%q) = %q, expected to resolve to %q", linkDir, result, actualDir) + } +} + +func TestResolveRepoForPath_EmptyHint(t *testing.T) { + server := &MCPServer{ + roots: newRootsManager(), + } + + result := server.resolveRepoForPath("") + if result != "" { + t.Errorf("resolveRepoForPath('') = %q, want empty string", result) + } +} + +func TestResolveRepoForPath_AbsolutePath(t *testing.T) { + // Use the current repo as a test case + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get cwd: %v", err) + } + + server := &MCPServer{ + roots: newRootsManager(), + } + + // This file exists in a git repo + result := server.resolveRepoForPath(cwd) + + // Should return a git root (non-empty if we're in a git repo) + // Don't fail if not in a git repo, just skip + if result == "" { + t.Skip("Not running in a git repository") + } + + // The result should be a parent of cwd + if !isParentOrEqual(result, cwd) { + t.Errorf("resolveRepoForPath(%q) = %q, expected a parent directory", cwd, result) + } +} + +// isParentOrEqual checks if parent is a parent directory of child (or equal) +func isParentOrEqual(parent, child string) bool { + parent = filepath.Clean(parent) + child = filepath.Clean(child) + + if parent == child { + return true + } + + rel, err := filepath.Rel(parent, child) + if err != nil { + return false + } + + // If relative path doesn't start with "..", parent is an ancestor + return len(rel) > 0 && rel[0] != '.' +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 6a7052eb..dea4390c 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -76,6 +76,7 @@ func NewMCPServer(version string, engine *query.Engine, logger *slog.Logger) *MC logger: logger, version: version, legacyEngine: engine, + engines: make(map[string]*engineEntry), tools: make(map[string]ToolHandler), resources: make(map[string]ResourceHandler), activePreset: DefaultPreset, @@ -96,6 +97,23 @@ func NewMCPServer(version string, engine *query.Engine, logger *slog.Logger) *MC SetMetricsDB(engine.DB()) } + // Store initial engine in cache for auto-resolution + if engine != nil { + repoRoot := engine.GetRepoRoot() + normalized := normalizePath(repoRoot) + if normalized != "" { + server.engines[normalized] = &engineEntry{ + engine: engine, + repoPath: normalized, + repoName: filepath.Base(normalized), + loadedAt: time.Now(), + lastUsed: time.Now(), + } + server.activeRepoPath = normalized + server.activeRepo = filepath.Base(normalized) + } + } + return server } @@ -120,6 +138,7 @@ func NewMCPServerLazy(version string, loader EngineLoader, logger *slog.Logger) logger: logger, version: version, engineLoader: loader, + engines: make(map[string]*engineEntry), tools: make(map[string]ToolHandler), resources: make(map[string]ResourceHandler), activePreset: DefaultPreset, @@ -187,6 +206,26 @@ func (s *MCPServer) engine() *query.Engine { if engine != nil && engine.DB() != nil { SetMetricsDB(engine.DB()) } + // Store in engine cache for auto-resolution + if engine != nil { + repoRoot := engine.GetRepoRoot() + normalized := normalizePath(repoRoot) + if normalized != "" { + s.mu.Lock() + s.engines[normalized] = &engineEntry{ + engine: engine, + repoPath: normalized, + repoName: filepath.Base(normalized), + loadedAt: time.Now(), + lastUsed: time.Now(), + } + if s.activeRepoPath == "" { + s.activeRepoPath = normalized + s.activeRepo = filepath.Base(normalized) + } + s.mu.Unlock() + } + } s.logger.Info("Engine loaded successfully") }) return s.legacyEngine @@ -456,75 +495,36 @@ func (s *MCPServer) createEngineForRoot(repoRoot string) (*query.Engine, error) // switchToClientRoot switches the engine to the client's root directory if different. // This fixes repo confusion when using a binary from a different location. -// -// IMPORTANT: Only switches in legacy single-engine mode. In multi-repo mode, -// users have explicit control via switchRepo tool, so we don't override that. +// Uses the engine cache so old engines are retained for auto-resolution. func (s *MCPServer) switchToClientRoot(clientRoot string) { if clientRoot == "" { return } - // Only switch in legacy single-engine mode - // Multi-repo mode users have explicit control via switchRepo - if s.legacyEngine == nil { - s.logger.Debug("Multi-repo mode active, not auto-switching to client root", - "clientRoot", clientRoot, - ) - return - } - - currentRoot := s.legacyEngine.GetRepoRoot() - - // Normalize paths for comparison clientRootClean := filepath.Clean(clientRoot) - currentRootClean := filepath.Clean(currentRoot) - // Check if they're the same - if clientRootClean == currentRootClean { - s.logger.Debug("Client root matches current repo, no switch needed", - "root", clientRootClean, - ) - return + // Check if current engine already points here + if eng := s.engine(); eng != nil { + currentRootClean := filepath.Clean(eng.GetRepoRoot()) + if clientRootClean == currentRootClean { + s.logger.Debug("Client root matches current repo, no switch needed", + "root", clientRootClean, + ) + return + } } s.logger.Info("Client root differs from server repo, switching to client's project", "clientRoot", clientRootClean, - "serverRoot", currentRootClean, ) - // Create a new engine for the client's root - newEngine, err := s.createEngineForRoot(clientRootClean) - if err != nil { - s.logger.Warn("Failed to create engine for client root, keeping current repo", + // Use ensureActiveEngine which handles caching and swapping + if err := s.ensureActiveEngine(clientRootClean); err != nil { + s.logger.Warn("Failed to switch to client root, keeping current repo", "clientRoot", clientRootClean, "error", err.Error(), ) - return } - - // Close the old engine's database to avoid resource leaks - oldEngine := s.legacyEngine - if oldEngine != nil && oldEngine.DB() != nil { - if err := oldEngine.DB().Close(); err != nil { - s.logger.Warn("Failed to close old engine database", - "error", err.Error(), - ) - } - } - - // Switch to the new engine - s.mu.Lock() - s.legacyEngine = newEngine - s.mu.Unlock() - - // Wire up metrics persistence for the new engine - if newEngine.DB() != nil { - SetMetricsDB(newEngine.DB()) - } - - s.logger.Info("Switched to client root", - "root", clientRootClean, - ) } // enrichNotFoundError adds repo context to "not found" errors when the client diff --git a/internal/mcp/tool_impls_multirepo.go b/internal/mcp/tool_impls_multirepo.go index 3335ad44..d734d79f 100644 --- a/internal/mcp/tool_impls_multirepo.go +++ b/internal/mcp/tool_impls_multirepo.go @@ -2,34 +2,18 @@ package mcp import ( "fmt" + "path/filepath" "time" - "github.com/SimplyLiz/CodeMCP/internal/config" "github.com/SimplyLiz/CodeMCP/internal/envelope" "github.com/SimplyLiz/CodeMCP/internal/errors" - "github.com/SimplyLiz/CodeMCP/internal/query" "github.com/SimplyLiz/CodeMCP/internal/repos" - "github.com/SimplyLiz/CodeMCP/internal/storage" ) -// toolListRepos lists all registered repositories +// toolListRepos lists all registered repositories and loaded engines func (s *MCPServer) toolListRepos(params map[string]interface{}) (*envelope.Response, error) { s.logger.Debug("Executing listRepos") - if !s.IsMultiRepoMode() { - return nil, &MCPError{ - Code: InvalidRequest, - Message: "Multi-repo mode not enabled. Start MCP server with a registry.", - } - } - - registry, err := repos.LoadRegistry() - if err != nil { - return nil, errors.NewOperationError("load registry", err) - } - - activeRepo, _ := s.GetActiveRepo() - type repoInfo struct { Name string `json:"name"` Path string `json:"path"` @@ -39,28 +23,58 @@ func (s *MCPServer) toolListRepos(params map[string]interface{}) (*envelope.Resp IsLoaded bool `json:"is_loaded"` } + activeRepo, _ := s.GetActiveRepo() var repoList []repoInfo - for _, entry := range registry.List() { - state := registry.ValidateState(entry.Name) + var defaultName string - s.mu.RLock() - _, isLoaded := s.engines[entry.Path] - s.mu.RUnlock() + // Include repos from registry if available + registry, err := repos.LoadRegistry() + if err == nil && len(registry.List()) > 0 { + defaultName = registry.Default + for _, entry := range registry.List() { + state := registry.ValidateState(entry.Name) + + s.mu.RLock() + _, isLoaded := s.engines[entry.Path] + s.mu.RUnlock() + + repoList = append(repoList, repoInfo{ + Name: entry.Name, + Path: entry.Path, + State: string(state), + IsDefault: entry.Name == registry.Default, + IsActive: entry.Name == activeRepo, + IsLoaded: isLoaded, + }) + } + } - repoList = append(repoList, repoInfo{ - Name: entry.Name, - Path: entry.Path, - State: string(state), - IsDefault: entry.Name == registry.Default, - IsActive: entry.Name == activeRepo, - IsLoaded: isLoaded, - }) + // Also include any loaded engines not in the registry + s.mu.RLock() + for path, entry := range s.engines { + found := false + for _, r := range repoList { + if r.Path == path { + found = true + break + } + } + if !found { + repoList = append(repoList, repoInfo{ + Name: entry.repoName, + Path: entry.repoPath, + State: "valid", + IsActive: entry.repoPath == s.activeRepoPath, + IsLoaded: true, + }) + } } + s.mu.RUnlock() return OperationalResponse(map[string]interface{}{ "repos": repoList, "activeRepo": activeRepo, - "default": registry.Default, + "default": defaultName, }), nil } @@ -70,13 +84,6 @@ func (s *MCPServer) toolSwitchRepo(params map[string]interface{}) (*envelope.Res "params", params, ) - if !s.IsMultiRepoMode() { - return nil, &MCPError{ - Code: InvalidRequest, - Message: "Multi-repo mode not enabled. Start MCP server with a registry.", - } - } - name, ok := params["name"].(string) if !ok || name == "" { return nil, &MCPError{ @@ -85,104 +92,67 @@ func (s *MCPServer) toolSwitchRepo(params map[string]interface{}) (*envelope.Res } } + // Try registry first registry, err := repos.LoadRegistry() - if err != nil { - return nil, errors.NewOperationError("load registry", err) - } - - entry, state, err := registry.Get(name) - if err != nil { - return nil, &MCPError{ - Code: InvalidParams, - Message: fmt.Sprintf("Repository not found: %s", name), + if err == nil { + entry, state, getErr := registry.Get(name) + if getErr == nil { + switch state { + case repos.RepoStateMissing: + return nil, &MCPError{ + Code: InvalidParams, + Message: fmt.Sprintf("Path does not exist: %s", entry.Path), + Data: map[string]string{"hint": fmt.Sprintf("Run: ckb repo remove %s", name)}, + } + case repos.RepoStateUninitialized: + return nil, &MCPError{ + Code: InvalidParams, + Message: fmt.Sprintf("Repository not initialized: %s", entry.Path), + Data: map[string]string{"hint": fmt.Sprintf("Run: cd %s && ckb init", entry.Path)}, + } + } + + // Use ensureActiveEngine for the switch + if switchErr := s.ensureActiveEngine(entry.Path); switchErr != nil { + return nil, errors.NewOperationError("switch to "+name, switchErr) + } + + // Update the repo name (ensureActiveEngine uses filepath.Base) + s.mu.Lock() + s.activeRepo = name + s.mu.Unlock() + + _ = registry.TouchLastUsed(name) + + return OperationalResponse(map[string]interface{}{ + "success": true, + "activeRepo": name, + "path": entry.Path, + }), nil } } - switch state { - case repos.RepoStateMissing: - return nil, &MCPError{ - Code: InvalidParams, - Message: fmt.Sprintf("Path does not exist: %s", entry.Path), - Data: map[string]string{"hint": fmt.Sprintf("Run: ckb repo remove %s", name)}, - } - case repos.RepoStateUninitialized: - return nil, &MCPError{ - Code: InvalidParams, - Message: fmt.Sprintf("Repository not initialized: %s", entry.Path), - Data: map[string]string{"hint": fmt.Sprintf("Run: cd %s && ckb init", entry.Path)}, - } + // Not in registry — treat name as a path + return nil, &MCPError{ + Code: InvalidParams, + Message: fmt.Sprintf("Repository not found: %s", name), } - - // Load or switch engine - s.mu.Lock() - defer s.mu.Unlock() - - // Check if already loaded - if existingEntry, ok := s.engines[entry.Path]; ok { - existingEntry.lastUsed = time.Now() - s.activeRepo = name - s.activeRepoPath = entry.Path - s.logger.Info("Switched to existing engine", - "repo", name, - "path", entry.Path, - ) - return OperationalResponse(map[string]interface{}{ - "success": true, - "activeRepo": name, - "path": entry.Path, - }), nil - } - - // Need to create new engine - check if we're at max - if len(s.engines) >= maxEngines { - s.evictLRULocked() - } - - // Create new engine - engine, err := s.createEngineForRepo(entry.Path) - if err != nil { - return nil, errors.NewOperationError("create engine for "+name, err) - } - - s.engines[entry.Path] = &engineEntry{ - engine: engine, - repoPath: entry.Path, - repoName: name, - loadedAt: time.Now(), - lastUsed: time.Now(), - } - s.activeRepo = name - s.activeRepoPath = entry.Path - - // Update last used in registry - _ = registry.TouchLastUsed(name) - - s.logger.Info("Created new engine and switched", - "repo", name, - "path", entry.Path, - "totalLoaded", len(s.engines), - ) - - return OperationalResponse(map[string]interface{}{ - "success": true, - "activeRepo": name, - "path": entry.Path, - }), nil } // toolGetActiveRepo returns information about the currently active repository func (s *MCPServer) toolGetActiveRepo(params map[string]interface{}) (*envelope.Response, error) { s.logger.Debug("Executing getActiveRepo") - if !s.IsMultiRepoMode() { - return nil, &MCPError{ - Code: InvalidRequest, - Message: "Multi-repo mode not enabled. Start MCP server with a registry.", + name, path := s.GetActiveRepo() + + // Fall back to current engine info if no explicit active repo + if name == "" && path == "" { + if eng := s.engine(); eng != nil { + path = eng.GetRepoRoot() + name = filepath.Base(path) } } - name, path := s.GetActiveRepo() - if name == "" { return OperationalResponse(map[string]interface{}{ "name": nil, @@ -191,17 +161,18 @@ func (s *MCPServer) toolGetActiveRepo(params map[string]interface{}) (*envelope. }), nil } - registry, err := repos.LoadRegistry() - if err != nil { - return nil, errors.NewOperationError("load registry", err) + // Try to get state from registry + state := "valid" + if registry, err := repos.LoadRegistry(); err == nil { + if rs := registry.ValidateState(name); rs != "" { + state = string(rs) + } } - state := registry.ValidateState(name) - return OperationalResponse(map[string]interface{}{ "name": name, "path": path, - "state": string(state), + "state": state, }), nil } @@ -238,31 +209,6 @@ func (s *MCPServer) evictLRULocked() { } } -// createEngineForRepo creates a new query engine for a repository -func (s *MCPServer) createEngineForRepo(repoPath string) (*query.Engine, error) { - // Load config from repo - cfg, err := config.LoadConfig(repoPath) - if err != nil { - // Use default config - cfg = config.DefaultConfig() - } - - // Open storage for this repo - db, err := storage.Open(repoPath, s.logger) - if err != nil { - return nil, errors.NewOperationError("open database", err) - } - - // Create engine - engine, err := query.NewEngine(repoPath, db, s.logger, cfg) - if err != nil { - _ = db.Close() - return nil, errors.NewOperationError("create engine", err) - } - - return engine, nil -} - // CloseAllEngines closes all loaded engines (for graceful shutdown) func (s *MCPServer) CloseAllEngines() { s.mu.Lock() diff --git a/internal/query/prepare_move.go b/internal/query/prepare_move.go index 13089685..20b0b49c 100644 --- a/internal/query/prepare_move.go +++ b/internal/query/prepare_move.go @@ -138,7 +138,7 @@ func (e *Engine) findAffectedImportsHeuristic(sourceDir, targetDir string) []Mov return nil } - f, err := os.Open(path) + f, err := os.Open(path) // #nosec G122 -- path from filepath.WalkDir in trusted repo if err != nil { return nil } diff --git a/internal/repos/lock_unix.go b/internal/repos/lock_unix.go index 4585582f..52d818c4 100644 --- a/internal/repos/lock_unix.go +++ b/internal/repos/lock_unix.go @@ -8,9 +8,9 @@ import ( ) func lockFile(f *os.File) error { - return syscall.Flock(int(f.Fd()), syscall.LOCK_EX) + return syscall.Flock(int(f.Fd()), syscall.LOCK_EX) // #nosec G115 -- fd fits in int } func unlockFile(f *os.File) error { - return syscall.Flock(int(f.Fd()), syscall.LOCK_UN) + return syscall.Flock(int(f.Fd()), syscall.LOCK_UN) // #nosec G115 -- fd fits in int } diff --git a/internal/repos/registry.go b/internal/repos/registry.go index fb8b86fb..3421a338 100644 --- a/internal/repos/registry.go +++ b/internal/repos/registry.go @@ -64,7 +64,7 @@ func LoadRegistry() (*Registry, error) { return nil, err } - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) // #nosec G703 -- path is internally constructed if os.IsNotExist(err) { // Return empty registry return &Registry{ @@ -129,11 +129,11 @@ func (r *Registry) Save() error { // Write atomically tmpPath := path + ".tmp" - if err := os.WriteFile(tmpPath, data, 0644); err != nil { + if err := os.WriteFile(tmpPath, data, 0644); err != nil { // #nosec G703 -- non-sensitive registry file return fmt.Errorf("failed to write registry: %w", err) } if err := os.Rename(tmpPath, path); err != nil { - _ = os.Remove(tmpPath) + _ = os.Remove(tmpPath) // #nosec G703 -- path is internally constructed return fmt.Errorf("failed to rename registry: %w", err) } @@ -356,7 +356,7 @@ func acquireLock(path string) (*FileLock, error) { return nil, err } - f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0644) + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0644) // #nosec G703 -- path is internally constructed if err != nil { return nil, err } diff --git a/internal/telemetry/matcher.go b/internal/telemetry/matcher.go index 96fd5c56..609a2aae 100644 --- a/internal/telemetry/matcher.go +++ b/internal/telemetry/matcher.go @@ -1,6 +1,7 @@ package telemetry import ( + "strconv" "strings" ) @@ -217,7 +218,7 @@ func (idx *SCIPSymbolIndex) AddSymbol(symbol *IndexedSymbol) { } func locationKey(file string, line int) string { - return file + ":" + string(rune(line)) + return file + ":" + strconv.Itoa(line) } // FindByLocation implements SymbolIndex diff --git a/internal/webhooks/manager.go b/internal/webhooks/manager.go index c4207da6..974ee18b 100644 --- a/internal/webhooks/manager.go +++ b/internal/webhooks/manager.go @@ -1174,7 +1174,7 @@ func (s *Store) scanDeliveryFromRows(rows *sql.Rows) (*Delivery, error) { delivery.LastError = lastError.String if responseCode.Valid { - delivery.ResponseCode = int(responseCode.Int64) + delivery.ResponseCode = int(responseCode.Int64) // #nosec G115 -- HTTP status code fits in int } if t, err := time.Parse(time.RFC3339, createdAt); err == nil {