Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 150 additions & 10 deletions src/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,31 +56,68 @@ pub fn ensure_project(conn: &Connection, root: &Path) -> anyhow::Result<i64> {

/// 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<std::path::PathBuf> {
let dir = if start.is_file() {
start.parent()?
} else {
start
};

const MARKERS: &[&str] = &[".cora.yaml", "Cargo.toml", "package.json", ".git"];

let mut current = dir.to_path_buf();
let mut fallback: Option<std::path::PathBuf> = 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.
Expand Down Expand Up @@ -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);
}
}
45 changes: 34 additions & 11 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
92 changes: 91 additions & 1 deletion src/mcp/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,20 +707,70 @@ 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,
"db_size_bytes": stats.db_size_bytes,
"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<String> {
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<String> = 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 {
Expand Down Expand Up @@ -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!({}));
Expand Down
Loading