diff --git a/src/index/mod.rs b/src/index/mod.rs index 8b08428..f6259b6 100644 --- a/src/index/mod.rs +++ b/src/index/mod.rs @@ -56,8 +56,15 @@ pub fn ensure_project(conn: &Connection, root: &Path) -> anyhow::Result { /// Detect the project root by walking up from `start` looking for marker files. /// -/// Search order: `.cora.yaml` → `Cargo.toml` → `package.json` → `.git` (dir or file). -/// Returns the directory containing the first marker found, or `None` if none is found. +/// Resolution order: +/// 1. `.cora.yaml` — explicit user override, always wins immediately. +/// 2. A `Cargo.toml` declaring a `[workspace]` section — a Rust workspace root +/// beats a nested member crate's plain `Cargo.toml`, so indexing from inside +/// `crates/*` and resolving from the repo root land on the same project (#522). +/// 3. The first plain marker (`Cargo.toml`, `package.json`, `.git`) as fallback. +/// +/// The walk never climbs past a git repository boundary, so an unrelated +/// `[workspace]` outside the repo cannot hijack resolution. pub fn resolve_project_root(start: &Path) -> Option { let dir = if start.is_file() { start.parent()? @@ -65,22 +72,52 @@ pub fn resolve_project_root(start: &Path) -> Option { start }; - const MARKERS: &[&str] = &[".cora.yaml", "Cargo.toml", "package.json", ".git"]; - let mut current = dir.to_path_buf(); + let mut fallback: Option = None; loop { - for marker in MARKERS { - let candidate = current.join(marker); - if candidate.exists() { - debug!(root = %current.display(), marker, "detected project root"); - return Some(current); + // 1. Explicit cora config wins immediately. + if current.join(".cora.yaml").is_file() { + debug!(root = %current.display(), marker = ".cora.yaml", "detected project root"); + return Some(current); + } + + // 2. Cargo workspace root beats a nested member crate manifest. + let cargo_toml = current.join("Cargo.toml"); + if cargo_toml.is_file() + && std::fs::read_to_string(&cargo_toml) + .map(|s| s.contains("[workspace")) + .unwrap_or(false) + { + debug!(root = %current.display(), marker = "[workspace] Cargo.toml", "detected project root"); + return Some(current); + } + + // 3. First plain marker is the fallback (original behavior). + if fallback.is_none() { + const MARKERS: &[&str] = &["Cargo.toml", "package.json", ".git"]; + for marker in MARKERS { + if current.join(marker).exists() { + fallback = Some(current.clone()); + break; + } } } + + // Repo boundary: stop AFTER giving this directory its own chance to + // match above (a repo root can legitimately be the workspace root). + if current.join(".git").exists() { + break; + } match current.parent() { Some(parent) if parent != current => current = parent.to_path_buf(), - _ => return None, + _ => break, } } + + if let Some(root) = &fallback { + debug!(root = %root.display(), "detected project root"); + } + fallback } /// Resolve `project_id` from the current directory, using project root detection. @@ -806,4 +843,107 @@ pub struct AuthService { "resolved root should contain Cargo.toml" ); } + + /// Regression (#522): running `cora index` from inside a workspace member + /// crate must resolve to the WORKSPACE root (the member's plain + /// `Cargo.toml` is not the project root), so CLI and MCP agree on one + /// project_id instead of silently creating two. + #[test] + fn test_resolve_project_root_prefers_workspace_root() { + let tmp = tempfile::TempDir::new().unwrap(); + let ws = tmp.path().join("ws"); + let member = ws.join("crates").join("app"); + std::fs::create_dir_all(&member).unwrap(); + + std::fs::write( + ws.join("Cargo.toml"), + "[workspace]\nmembers = [\"crates/*\"]\n", + ) + .unwrap(); + std::fs::write(member.join("Cargo.toml"), "[package]\nname = \"app\"\n").unwrap(); + std::fs::write(member.join("src.rs"), "fn main() {}\n").unwrap(); + + let resolved = resolve_project_root(&member); + assert_eq!( + resolved.as_deref(), + Some(ws.as_path()), + "workspace root should win over a member crate's plain Cargo.toml" + ); + } + + /// An explicit `.cora.yaml` anywhere along the walk always wins — it is a + /// deliberate user override of project-root detection. + #[test] + fn test_resolve_project_root_cora_yaml_wins_over_workspace() { + let tmp = tempfile::TempDir::new().unwrap(); + let ws = tmp.path().join("ws"); + let member = ws.join("crates").join("app"); + std::fs::create_dir_all(&member).unwrap(); + + std::fs::write( + ws.join("Cargo.toml"), + "[workspace]\nmembers = [\"crates/*\"]\n", + ) + .unwrap(); + std::fs::write(ws.join(".cora.yaml"), "version: 1\n").unwrap(); + std::fs::write(member.join("Cargo.toml"), "[package]\nname = \"app\"\n").unwrap(); + std::fs::write(member.join(".cora.yaml"), "version: 1\n").unwrap(); + + let resolved = resolve_project_root(&member); + assert_eq!(resolved.as_deref(), Some(member.as_path())); + } + + /// Root detection must not climb above a git repository boundary: an + /// unrelated `[workspace]` Cargo.toml outside the repo must never hijack + /// resolution. + #[test] + fn test_resolve_project_root_stops_at_git_boundary() { + let tmp = tempfile::TempDir::new().unwrap(); + let outer_ws = tmp.path().join("outer"); + let repo = outer_ws.join("myrepo"); + std::fs::create_dir_all(repo.join(".git")).unwrap(); + + std::fs::write( + outer_ws.join("Cargo.toml"), + "[workspace]\nmembers = [\"*\"]\n", + ) + .unwrap(); + // Repo itself has no markers other than .git and one plain file dir. + let deep = repo.join("src"); + std::fs::create_dir_all(&deep).unwrap(); + + let resolved = resolve_project_root(&deep); + assert_eq!( + resolved.as_deref(), + Some(repo.as_path()), + ".git must stop the upward walk" + ); + } + + /// Regression (#522): an incremental no-op re-index (all fingerprints + /// match) must keep reporting the STORED symbol count — DB state must + /// survive untouched re-runs. + #[test] + fn test_incremental_index_preserves_counts() { + let conn = mem_conn(); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().to_path_buf(); + std::fs::write(root.join("lib.rs"), "pub fn alpha() {} pub fn beta() {}\n").unwrap(); + + let first = index_project(&conn, &root, false).unwrap(); + assert_eq!(first.files_indexed, 1); + assert!(first.symbols_indexed > 0); + + // Second run: everything unchanged → skipped, nothing wiped. + let second = index_project(&conn, &root, false).unwrap(); + assert_eq!(second.files_skipped, 1); + assert_eq!(second.files_indexed, 0); + + let summary = index_stats(&conn, ensure_project(&conn, &root).unwrap()).unwrap(); + assert_eq!( + summary.total_symbols as usize, first.symbols_indexed, + "stored symbols must survive an incremental no-op re-run" + ); + assert_eq!(summary.total_files, 1); + } } diff --git a/src/main.rs b/src/main.rs index 507da76..83ab980 100644 --- a/src/main.rs +++ b/src/main.rs @@ -787,17 +787,40 @@ async fn main() -> Result<()> { verbose || cli.global.verbose, skip_patterns.as_deref(), )?; - eprintln!( - "{}", - format!( - "✅ Indexed {} symbols from {} files ({} skipped, {} errors)", - stats.symbols_indexed, - stats.files_indexed, - stats.files_skipped, - stats.errors - ) - .green() - ); + if stats.files_indexed == 0 && stats.errors == 0 { + // Incremental no-op: fingerprints all matched. Report the + // STORED totals instead of a confusing zeros line (#522). + eprintln!( + "{}", + format!( + "✓ Index up to date ({} files unchanged)", + stats.files_skipped + ) + .green() + ); + if let Ok(summary) = index::index_stats(&conn, project_id) { + eprintln!( + "{}", + format!( + " {} symbols across {} files", + summary.total_symbols, summary.total_files + ) + .dimmed() + ); + } + } else { + eprintln!( + "{}", + format!( + "✅ Indexed {} symbols from {} files ({} skipped, {} errors)", + stats.symbols_indexed, + stats.files_indexed, + stats.files_skipped, + stats.errors + ) + .green() + ); + } eprintln!( "{}", format!( diff --git a/src/mcp/tools.rs b/src/mcp/tools.rs index c93d780..8d5fddb 100644 --- a/src/mcp/tools.rs +++ b/src/mcp/tools.rs @@ -707,7 +707,7 @@ fn handle_index_status() -> ToolResult { match crate::index::index_stats(&conn, project_id) { Ok(stats) => { - let json = serde_json::json!({ + let mut json = serde_json::json!({ "exists": true, "total_symbols": stats.total_symbols, "total_files": stats.total_files, @@ -715,12 +715,62 @@ fn handle_index_status() -> ToolResult { "symbols_by_kind": stats.symbols_by_kind, "symbols_by_language": stats.symbols_by_language, }); + if let Some(hint) = project_root_mismatch_hint(&conn, project_id, stats.total_symbols) { + json["hint"] = serde_json::json!(hint); + } ToolResult::text(serde_json::to_string_pretty(&json).unwrap_or_default()) } Err(e) => ToolResult::error(format!("Failed to get stats: {e}")), } } +/// Surface a silent project-root mismatch (#522): the resolved project has no +/// indexed symbols while other indexed projects in the same global DB do — +/// usually because CLI and MCP resolved different project roots. +fn project_root_mismatch_hint( + conn: &rusqlite::Connection, + project_id: i64, + total_symbols: usize, +) -> Option { + if total_symbols > 0 { + return None; + } + + let mut stmt = conn + .prepare( + "SELECT p.root_path, COUNT(s.id) + FROM projects p + JOIN symbols s ON s.project_id = p.id + WHERE p.id != ?1 + GROUP BY p.id + ORDER BY COUNT(s.id) DESC + LIMIT 3", + ) + .ok()?; + let rows: Vec<(String, i64)> = stmt + .query_map([project_id], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) + }) + .ok()? + .filter_map(|r| r.ok()) + .collect(); + + if rows.is_empty() { + return None; + } + + let list: Vec = rows + .iter() + .map(|(root, n)| format!("{root} ({n} symbols)")) + .collect(); + Some(format!( + "This project root has 0 indexed symbols, but the global index holds data for \ + other roots: {}. Likely a project-root mismatch between where 'cora index' ran \ + and where this session resolved the root. Run 'cora index' at your project root.", + list.join(", ") + )) +} + // ─── Review Pipeline Handlers (Phase 2) ─── fn handle_review_diff(params: &serde_json::Value) -> ToolResult { @@ -1160,6 +1210,46 @@ mod tests { assert!(result.is_error || result.content[0].text.contains("total_symbols")); } + /// Regression (#522): when the resolved project has zero symbols but the + /// global DB holds data for other roots, index_status must carry a hint + /// naming those roots instead of silently reporting zeros. + #[test] + fn project_root_mismatch_hint_on_zero_symbol_project() { + let conn = rusqlite::Connection::open_in_memory().unwrap(); + crate::index::schema::run_migrations(&conn).unwrap(); + + let indexed_pid = + crate::index::schema::get_or_create_project(&conn, "/workspace/uteke").unwrap(); + conn.execute( + "INSERT INTO symbols (name, kind, file, line, signature, language, project_id) + VALUES ('alpha', 'function', 'lib.rs', 1, '', 'rust', ?1)", + [indexed_pid], + ) + .unwrap(); + let empty_pid = + crate::index::schema::get_or_create_project(&conn, "/workspace/uteke/crates/app") + .unwrap(); + + let hint = project_root_mismatch_hint(&conn, empty_pid, 0); + assert!( + hint.is_some(), + "zero-symbol project beside an indexed one must hint" + ); + let hint = hint.unwrap(); + assert!( + hint.contains("/workspace/uteke"), + "hint should name the root that actually holds data: {hint}" + ); + assert!( + hint.contains("(1 symbols)"), + "hint should include counts: {hint}" + ); + + // Happy paths produce no hint. + assert!(project_root_mismatch_hint(&conn, empty_pid, 5).is_none()); + assert!(project_root_mismatch_hint(&conn, indexed_pid, 1).is_none()); + } + #[test] fn handle_search_symbols_missing_query() { let result = handle_tool_call("cora.search_symbols", &serde_json::json!({}));