diff --git a/Cargo.lock b/Cargo.lock index eda7f82..f2360a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -86,9 +86,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] name = "arrow-array" @@ -502,9 +502,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] diff --git a/README.md b/README.md index cefe645..6cd680b 100644 --- a/README.md +++ b/README.md @@ -60,9 +60,12 @@ Add the server to your agent. For Claude Code (`~/.claude.json`): ``` Once wired, have the agent call `oxcode_watch` once: it builds the index and -keeps it current as files change. Across multiple agents on one repo a file lock -elects a single writer (the one watcher/re-indexer) while the rest serve reads, so -you can run as many as you like. Then ask questions with `oxcode_explore`. +keeps it current as files change. When `path` is omitted, the project root comes +from `OXCODE_ROOT`, the client's MCP roots, `CLAUDE_PROJECT_DIR`, or +`WORKSPACE_FOLDER_PATHS` — not from the MCP process cwd (hosts often start servers in +`$HOME`). Across multiple agents on one repo a file lock elects a single writer +(the one watcher/re-indexer) while the rest serve reads, so you can run as many +as you like. Then ask questions with `oxcode_explore`. Optionally auto-allow the tools in `~/.claude/settings.json` (the query tools are read-only; `oxcode_watch` only builds/maintains the local index): diff --git a/crates/oxcode-cli/src/mcp/integration_tests.rs b/crates/oxcode-cli/src/mcp/integration_tests.rs new file mode 100644 index 0000000..73b0560 --- /dev/null +++ b/crates/oxcode-cli/src/mcp/integration_tests.rs @@ -0,0 +1,487 @@ +//! In-process MCP integration tests for OxcodeServer. + +//! In-process integration tests: a real `OxcodeServer` and an MCP client wired +//! over `tokio::io::duplex`, exercising the full JSON-RPC stack. These cover +//! tool registration, writer election + the read path, auto re-index on change, +//! and the task lifecycle. The cross-process guarantee is proven separately by +//! `tests/multiprocess.rs` (real spawned processes). + +use std::{sync::MutexGuard, time::Duration}; + +use rmcp::{ + ClientHandler, RoleClient, + model::{ + CallToolRequestParams, ClientCapabilities, ClientInfo, ClientRequest, GetTaskInfoParams, + GetTaskResultParams, Implementation, ListRootsResult, Request, Root, ServerResult, + TaskStatus, TaskSupport, + }, + service::{RequestContext, RunningService}, +}; + +use super::{ + project_root::{PROJECT_ROOT_ENV_LOCK, canonicalize_root, oxcode_root_override}, + *, +}; + +/// Env keys consulted for omitted-`path` defaults, for save/restore around tests. +const PROJECT_ROOT_ENV_KEYS: &[&str] = &[ + "OXCODE_ROOT", + "CLAUDE_PROJECT_DIR", + "WORKSPACE_FOLDER_PATHS", +]; + +/// Clears project-root env vars for the duration of a test; restores on drop. +struct ProjectRootEnvGuard { + _lock: MutexGuard<'static, ()>, + previous: Vec<(&'static str, Option)>, +} + +impl ProjectRootEnvGuard { + fn clear() -> Self { + let lock = PROJECT_ROOT_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let previous = PROJECT_ROOT_ENV_KEYS + .iter() + .map(|&key| (key, std::env::var_os(key))) + .collect::>(); + // SAFETY: held exclusively via PROJECT_ROOT_ENV_LOCK for this process. + unsafe { + for key in PROJECT_ROOT_ENV_KEYS { + std::env::remove_var(key); + } + } + Self { + _lock: lock, + previous, + } + } + + fn set(&self, key: &str, value: impl AsRef) { + // SAFETY: guard holds PROJECT_ROOT_ENV_LOCK. + unsafe { + std::env::set_var(key, value); + } + } +} + +impl Drop for ProjectRootEnvGuard { + fn drop(&mut self) { + // SAFETY: guard still holds PROJECT_ROOT_ENV_LOCK until drop completes. + unsafe { + for (key, value) in self.previous.drain(..) { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + } +} + +/// Minimal MCP client; the server is what these tests exercise. +#[derive(Clone, Default)] +struct TestClient; + +impl ClientHandler for TestClient {} + +/// MCP client that advertises workspace roots (Cursor / Claude Code do this). +#[derive(Clone)] +struct RootsClient { + roots: Vec, +} + +impl ClientHandler for RootsClient { + fn get_info(&self) -> ClientInfo { + ClientInfo::new( + ClientCapabilities::builder().enable_roots().build(), + Implementation::from_build_env(), + ) + } + + fn list_roots( + &self, + _context: RequestContext, + ) -> impl std::future::Future> + Send + '_ { + let roots = self + .roots + .iter() + .map(|path| Root::new(format!("file://{}", path.display()))) + .collect(); + std::future::ready(Ok(ListRootsResult::new(roots))) + } +} + +/// Wires a fresh `OxcodeServer` (with the given intervals) to a `TestClient` +/// over an in-memory duplex pipe and returns the connected client service. +async fn connect(debounce: Duration, poll: Duration) -> RunningService { + let (server_transport, client_transport) = tokio::io::duplex(4096); + tokio::spawn(async move { + let server = OxcodeServer::new_with(debounce, poll) + .serve(server_transport) + .await + .expect("server serve"); + let _ = server.waiting().await; + }); + TestClient + .serve(client_transport) + .await + .expect("client connect") +} + +/// Writes a minimal two-function Rust project into a fresh temp dir. +fn rust_project() -> tempfile::TempDir { + let temp = tempfile::TempDir::new().expect("temp dir"); + std::fs::create_dir_all(temp.path().join("src")).expect("mkdir src"); + std::fs::write( + temp.path().join("src/lib.rs"), + "pub fn helper() {}\npub fn entry() {\n helper();\n}\n", + ) + .expect("write lib.rs"); + temp +} + +/// Builds a tool-call params object for `name` with JSON `arguments`. +fn tool_call(name: &'static str, arguments: serde_json::Value) -> CallToolRequestParams { + let mut params = CallToolRequestParams::new(name); + params.arguments = arguments.as_object().cloned(); + params +} + +/// Extracts the single text content block from a tool result. +fn result_text(result: &CallToolResult) -> &str { + result + .content + .first() + .and_then(|content| content.as_text()) + .map(|text| text.text.as_str()) + .expect("text content") +} + +/// Calls `oxcode_watch` for `path` and returns the parsed JSON response. +async fn watch(client: &RunningService, path: &str) -> serde_json::Value { + let result = client + .call_tool(tool_call( + "oxcode_watch", + serde_json::json!({ "path": path }), + )) + .await + .expect("watch call"); + serde_json::from_str(result_text(&result)).expect("watch json") +} + +/// Polls `oxcode_search` (bounded) until `name` actually appears as a match. +/// Inspects the parsed `matches` array — not a substring of the JSON, which +/// would falsely match the echoed `query` field. +async fn poll_symbol_indexed( + client: &RunningService, + path: &str, + name: &str, +) -> bool { + for _ in 0..100 { + let searched = client + .call_tool(tool_call( + "oxcode_search", + serde_json::json!({ "path": path, "query": name }), + )) + .await + .expect("search call"); + let report: serde_json::Value = + serde_json::from_str(result_text(&searched)).expect("search json"); + // Keyword search is fuzzy, so check for an exact-named match rather + // than "any match" (which would falsely fire on weak candidates). + let matched = report["matches"] + .as_array() + .is_some_and(|matches| matches.iter().any(|entry| entry["symbol"]["name"] == name)); + if matched { + return true; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + false +} + +/// Polls `tasks/get` until the task reaches a terminal status (or times out). +async fn poll_until_terminal( + client: &RunningService, + task_id: &str, +) -> TaskStatus { + let mut status = TaskStatus::Working; + for _ in 0..200 { + let info = client + .send_request(ClientRequest::GetTaskInfoRequest(Request::new( + GetTaskInfoParams { + meta: None, + task_id: task_id.to_owned(), + }, + ))) + .await + .expect("tasks/get"); + if let ServerResult::GetTaskResult(result) = info { + status = result.task.status; + } + if status != TaskStatus::Working { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + status +} + +#[test] +fn oxcode_root_override_wins_over_host_env() { + let project = tempfile::TempDir::new().expect("temp"); + let host = tempfile::TempDir::new().expect("host"); + let guard = ProjectRootEnvGuard::clear(); + guard.set("CLAUDE_PROJECT_DIR", host.path()); + guard.set("OXCODE_ROOT", project.path()); + let resolved = oxcode_root_override().expect("OXCODE_ROOT"); + assert_eq!( + canonicalize_root(resolved), + canonicalize_root(project.path().to_path_buf()) + ); +} + +#[tokio::test] +async fn omitted_path_uses_client_mcp_roots_not_process_cwd() { + let _env = ProjectRootEnvGuard::clear(); + let project = rust_project(); + let (server_transport, client_transport) = tokio::io::duplex(4096); + tokio::spawn(async move { + let server = OxcodeServer::new_with(Duration::from_millis(50), Duration::from_millis(150)) + .serve(server_transport) + .await + .expect("server serve"); + let _ = server.waiting().await; + }); + let client = RootsClient { + roots: vec![project.path().to_path_buf()], + } + .serve(client_transport) + .await + .expect("client connect"); + + // No `path` argument: `on_initialized` already cached roots/list; resolve + // must use that workspace root (not process cwd) without nested roots/list. + let result = client + .call_tool(tool_call("oxcode_watch", serde_json::json!({}))) + .await + .expect("watch without path"); + let body: serde_json::Value = serde_json::from_str(result_text(&result)).expect("watch json"); + assert_eq!(body["role"], "writer"); + assert_eq!( + canonicalize_root(PathBuf::from(body["root"].as_str().expect("root"))), + canonicalize_root(project.path().to_path_buf()), + "omitted path must use the client's MCP root, not the server process cwd" + ); +} + +/// `flock` is per open-file-description on macOS/Linux: a second independent +/// open of the same path cannot take the lock the first holds. This pins the +/// platform behavior the writer election depends on. +#[test] +fn watch_lock_is_exclusive_per_handle() { + let temp = tempfile::TempDir::new().expect("temp dir"); + let path = temp.path().join("watch.lock"); + let first = OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(&path) + .expect("open first"); + first.try_lock().expect("first acquires"); + let second = OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .expect("open second"); + assert!( + matches!(second.try_lock(), Err(TryLockError::WouldBlock)), + "a second handle cannot take the held lock" + ); +} + +#[tokio::test] +async fn lists_tools_with_watch_and_explore_task_support() { + let client = connect(DEFAULT_DEBOUNCE, DEFAULT_POLL).await; + let tools = client.list_all_tools().await.expect("list tools"); + + assert!( + tools.iter().any(|tool| tool.name == "oxcode_watch"), + "oxcode_watch is registered" + ); + assert!( + tools.iter().all(|tool| tool.name != "oxcode_index"), + "the old write tool is gone" + ); + + let task_support = |name: &str| { + tools + .iter() + .find(|tool| tool.name == name) + .and_then(|tool| tool.execution.as_ref()) + .and_then(|execution| execution.task_support) + }; + assert_eq!(task_support("oxcode_watch"), Some(TaskSupport::Optional)); + assert_eq!(task_support("oxcode_explore"), Some(TaskSupport::Optional)); + assert_eq!(task_support("oxcode_search"), None); + assert_eq!(task_support("oxcode_status"), None); +} + +#[tokio::test] +async fn watch_elects_writer_and_serves_queries() { + let project = rust_project(); + let path = project.path().to_string_lossy().into_owned(); + let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; + + let watched = watch(&client, &path).await; + assert_eq!(watched["role"], "writer", "first watcher is the writer"); + assert_eq!(watched["watching"], true); + + let explored = client + .call_tool(tool_call( + "oxcode_explore", + serde_json::json!({ "path": path, "query": "entry" }), + )) + .await + .expect("explore call"); + assert!( + result_text(&explored).contains("entry"), + "writer's index is queryable" + ); +} + +#[tokio::test] +async fn second_watcher_on_same_root_is_standby() { + let project = rust_project(); + let path = project.path().to_string_lossy().into_owned(); + let writer_client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; + let standby_client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; + + assert_eq!(watch(&writer_client, &path).await["role"], "writer"); + // Second server, same root: the lock is held, so it becomes a standby. + assert_eq!(watch(&standby_client, &path).await["role"], "standby"); + + // The standby still answers queries off the shared on-disk index. + let explored = standby_client + .call_tool(tool_call( + "oxcode_explore", + serde_json::json!({ "path": path, "query": "entry" }), + )) + .await + .expect("reader explore"); + assert!(result_text(&explored).contains("entry")); +} + +#[tokio::test] +async fn query_without_watch_errors_when_no_index() { + let project = rust_project(); + let path = project.path().to_string_lossy().into_owned(); + let client = connect(DEFAULT_DEBOUNCE, DEFAULT_POLL).await; + + // No oxcode_watch, no prior index: a query must not build; it hints instead. + let result = client + .call_tool(tool_call( + "oxcode_explore", + serde_json::json!({ "path": path, "query": "entry" }), + )) + .await; + assert!( + result.is_err(), + "query before oxcode_watch errors with a hint, never silently builds" + ); +} + +#[tokio::test] +async fn writer_auto_reindexes_on_change() { + let project = rust_project(); + let path = project.path().to_string_lossy().into_owned(); + let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; + + assert_eq!(watch(&client, &path).await["role"], "writer"); + + // Let the FS-event stream establish before the change: FSEvents (and + // other backends) have a startup window where a change can land as + // initial state and go unreported. + tokio::time::sleep(Duration::from_millis(300)).await; + std::fs::write( + project.path().join("src/extra.rs"), + "pub fn brand_new_symbol() {}\n", + ) + .expect("write extra.rs"); + + let found = poll_symbol_indexed(&client, &path, "brand_new_symbol").await; + assert!( + found, + "the writer's watcher re-indexed and surfaced the symbol" + ); + + let status: serde_json::Value = serde_json::from_str(result_text( + &client + .call_tool(tool_call( + "oxcode_status", + serde_json::json!({ "path": path }), + )) + .await + .expect("status call"), + )) + .expect("status json"); + assert_eq!(status["watch"]["role"], "writer"); + assert!( + status["watch"]["reindexes"].as_u64().unwrap_or(0) >= 2, + "writer reindexed at least the initial build and the change" + ); +} + +#[tokio::test] +async fn task_augmented_watch_completes() { + let project = rust_project(); + let path = project.path().to_string_lossy().into_owned(); + let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; + + // Task-augment the call: typed `call_tool` cannot carry a task field, so + // send the request directly and expect an immediate CreateTaskResult. + let mut params = tool_call("oxcode_watch", serde_json::json!({ "path": path })); + params.task = serde_json::json!({ "ttl": 60_000 }).as_object().cloned(); + let created = client + .send_request(ClientRequest::CallToolRequest(Request::new(params))) + .await + .expect("enqueue task"); + let task_id = match created { + ServerResult::CreateTaskResult(result) => { + assert_eq!(result.task.status, TaskStatus::Working); + result.task.task_id + } + other => panic!("expected CreateTaskResult, got {other:?}"), + }; + + let status = poll_until_terminal(&client, &task_id).await; + assert_eq!( + status, + TaskStatus::Completed, + "watch task ran to completion" + ); + + let payload = client + .send_request(ClientRequest::GetTaskResultRequest(Request::new( + GetTaskResultParams { + meta: None, + task_id, + }, + ))) + .await + .expect("tasks/result"); + let text = match payload { + ServerResult::CallToolResult(result) => result_text(&result).to_owned(), + ServerResult::GetTaskPayloadResult(payload) => payload.0["content"][0]["text"] + .as_str() + .expect("tool result text") + .to_owned(), + other => panic!("expected the deferred tool result, got {other:?}"), + }; + assert!( + text.contains("writer"), + "deferred watch result reports the elected writer role" + ); +} diff --git a/crates/oxcode-cli/src/mcp.rs b/crates/oxcode-cli/src/mcp/mod.rs similarity index 62% rename from crates/oxcode-cli/src/mcp.rs rename to crates/oxcode-cli/src/mcp/mod.rs index b4dd607..8d4da07 100644 --- a/crates/oxcode-cli/src/mcp.rs +++ b/crates/oxcode-cli/src/mcp/mod.rs @@ -6,6 +6,9 @@ //! at one folder, a `.oxcode/watch.lock` file lock elects exactly one writer (the //! process that watches and re-indexes); the rest serve reads. +mod project_root; +mod roots; + use std::{ collections::{HashMap, HashSet}, fs::{File, OpenOptions, TryLockError}, @@ -22,15 +25,19 @@ use notify_debouncer_full::{ notify::{RecommendedWatcher, RecursiveMode}, }; use oxcode_core::{GraphDirection, IndexStats, NodeKind, ProjectIndex}; +use project_root::{OptionalProjectRoot, resolve_project_root}; use rmcp::{ - ErrorData as McpError, ServerHandler, ServiceExt, + ErrorData as McpError, RoleServer, ServerHandler, ServiceExt, handler::server::{router::tool::ToolRouter, wrapper::Parameters}, model::{CallToolResult, Content, ServerCapabilities, ServerInfo, TasksCapability}, - schemars, task_handler, + schemars, + service::NotificationContext, + task_handler, task_manager::OperationProcessor, tool, tool_handler, tool_router, transport::stdio, }; +use roots::{RootsCache, RootsWait}; use serde::Deserialize; use tokio::sync::{ Mutex, @@ -70,17 +77,19 @@ pub(crate) fn serve() -> anyhow::Result<()> { } /// Server instructions steering agents to `oxcode_watch` then `oxcode_explore`. -const INSTRUCTIONS: &str = "This server answers questions about the code repository in the working \ -directory. First call `oxcode_watch` (optional `path`, defaults to the working directory): it builds \ -the index if needed and keeps it current as files change. Only one MCP instance watches a given \ -folder at a time — a file lock elects a single writer; other instances serve reads and take over \ -automatically if the writer exits. Then, for almost any code-understanding question, call \ -`oxcode_explore` first with the user's question verbatim: it returns the most relevant symbols \ -(ranked by graph centrality), their source, the relationships among them, the n-ary hyperedges they \ -belong to (trait impl groups and container/module membership, ranked by hypergraph PageRank — the \ -architecture-altitude layer), the blast radius, and the call flow — in one call. Use \ -`oxcode_callers`/`oxcode_callees`/`oxcode_symbol` to follow specific edges, and \ -`oxcode_search`/`oxcode_files` only when explore did not surface the target. Prefer these query \ +const INSTRUCTIONS: &str = "This server answers questions about the code repository in the current \ +project. First call `oxcode_watch` (optional `path`): it builds the index if needed and keeps it \ +current as files change. When `path` is omitted, the project root is taken from OXCODE_ROOT, \ +the client's MCP roots, CLAUDE_PROJECT_DIR, or WORKSPACE_FOLDER_PATHS — not from this process's \ +cwd, which MCP hosts often set to $HOME. Pass `path` explicitly when in doubt. Only one MCP \ +instance watches a given folder at a time — a file lock elects a single writer; other instances \ +serve reads and take over automatically if the writer exits. Then, for almost any \ +code-understanding question, call `oxcode_explore` first with the user's question verbatim: it \ +returns the most relevant symbols (ranked by graph centrality), their source, the relationships \ +among them, the n-ary hyperedges they belong to (trait impl groups and container/module membership, \ +ranked by hypergraph PageRank — the architecture-altitude layer), the blast radius, and the call \ +flow — in one call. Use `oxcode_callers`/`oxcode_callees`/`oxcode_symbol` to follow specific edges, \ +and `oxcode_search`/`oxcode_files` only when explore did not surface the target. Prefer these query \ tools over shelling out to grep or reading files. Every tool except `oxcode_watch` is read-only; do \ not edit source files."; @@ -103,6 +112,8 @@ pub(crate) struct OxcodeServer { writers: Arc>>>, /// Roots this process is a standby for (lost the lock; a failover task polls). standbys: Arc>>, + /// Client MCP workspace roots (fetched outside tool handlers). + roots: RootsCache, /// File-watcher debounce window. debounce: Duration, /// Failover poll interval for standbys. @@ -128,8 +139,8 @@ struct WriterState { pub(crate) struct ExploreParams { /// The task or question about the codebase, in natural language. pub query: String, - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, /// Maximum source characters to render (default 20000). pub max_bytes: Option, } @@ -139,8 +150,8 @@ pub(crate) struct ExploreParams { pub(crate) struct SearchParams { /// Keywords matched against symbol names, signatures, and docs. pub query: String, - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, /// Maximum number of matches (default 30). pub limit: Option, /// Restrict to these symbol kinds (e.g. function, method, struct, trait). @@ -152,8 +163,8 @@ pub(crate) struct SearchParams { pub(crate) struct CallParams { /// Selector: a qualified name, `name:`, `element:`, or `file::`. pub selector: String, - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, /// Maximum hop depth (default 2). pub depth: Option, /// Maximum discovered symbol count (default 50). @@ -165,8 +176,8 @@ pub(crate) struct CallParams { pub(crate) struct SymbolParams { /// Selector: a qualified name, `name:`, `element:`, or `file::`. pub selector: String, - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, } /// A keyword search over indexed files. @@ -174,8 +185,8 @@ pub(crate) struct SymbolParams { pub(crate) struct FilesParams { /// Keywords matched against file paths and their symbols. pub query: String, - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, /// Maximum number of files (default 30). pub limit: Option, } @@ -183,15 +194,15 @@ pub(crate) struct FilesParams { /// A project-root pointer. #[derive(Debug, Deserialize, schemars::JsonSchema)] pub(crate) struct StatusParams { - /// Project root; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, } /// A project root to watch and keep indexed. #[derive(Debug, Deserialize, schemars::JsonSchema)] pub(crate) struct WatchParams { - /// Project root to watch; defaults to the server's working directory. - pub path: Option, + #[serde(flatten)] + pub root: OptionalProjectRoot, } #[tool_router] @@ -216,20 +227,21 @@ impl OxcodeServer { operations: Arc::new(Mutex::new(OperationProcessor::new())), writers: Arc::new(std::sync::Mutex::new(HashMap::new())), standbys: Arc::new(std::sync::Mutex::new(HashSet::new())), + roots: RootsCache::new(), debounce, poll, } } #[tool( - description = "Start (or join) watching a project so its index is built and kept current as files change. Exactly one MCP instance per folder becomes the writer (it holds a file lock and re-indexes on changes); other instances become readers that just serve queries and automatically take over if the writer exits. Call this once before querying. Optional `path` defaults to the working directory.", + description = "Start (or join) watching a project so its index is built and kept current as files change. Exactly one MCP instance per folder becomes the writer (it holds a file lock and re-indexes on changes); other instances become readers that just serve queries and automatically take over if the writer exits. Call this once before querying. Optional `path` defaults to the workspace (OXCODE_ROOT / MCP roots / CLAUDE_PROJECT_DIR / WORKSPACE_FOLDER_PATHS); never silently to $HOME.", execution(task_support = "optional") )] async fn oxcode_watch( &self, Parameters(params): Parameters, ) -> Result { - let root = resolve_root(params.path); + let root = self.resolve_root(params.root.path).await?; // Idempotent: already participating for this root. if self.is_writer(&root) { @@ -287,7 +299,7 @@ impl OxcodeServer { &self, Parameters(params): Parameters, ) -> Result { - let index = self.index_for(params.path).await?; + let index = self.index_for(params.root.path).await?; let query = params.query; let max_bytes = params.max_bytes.unwrap_or(20_000); let report = blocking(move || index.context(&query, 8, 1, max_bytes)).await?; @@ -301,7 +313,7 @@ impl OxcodeServer { &self, Parameters(params): Parameters, ) -> Result { - let index = self.index_for(params.path).await?; + let index = self.index_for(params.root.path).await?; let query = params.query; let limit = params.limit.unwrap_or(30); let kinds = parse_kinds(params.kinds.as_deref()); @@ -332,7 +344,7 @@ impl OxcodeServer { &self, Parameters(params): Parameters, ) -> Result { - let index = self.index_for(params.path).await?; + let index = self.index_for(params.root.path).await?; let selector = params.selector; let value = blocking(move || resolve_symbol(&index, &selector)).await?; json_result(&value) @@ -343,7 +355,7 @@ impl OxcodeServer { &self, Parameters(params): Parameters, ) -> Result { - let index = self.index_for(params.path).await?; + let index = self.index_for(params.root.path).await?; let query = params.query; let limit = params.limit.unwrap_or(30); let report = blocking(move || index.search_files(&query, limit)).await?; @@ -357,7 +369,7 @@ impl OxcodeServer { &self, Parameters(params): Parameters, ) -> Result { - let root = resolve_root(params.path); + let root = self.resolve_root(params.root.path).await?; let (role, watching, reindexes) = self.watch_state(&root); let status_root = root.clone(); let database = blocking(move || oxcode_core::project_status(&status_root)).await?; @@ -374,7 +386,7 @@ impl OxcodeServer { params: CallParams, direction: GraphDirection, ) -> Result { - let index = self.index_for(params.path).await?; + let index = self.index_for(params.root.path).await?; let selector = params.selector; let depth = params.depth.unwrap_or(2); let limit = params.limit.unwrap_or(50); @@ -382,12 +394,12 @@ impl OxcodeServer { json_result(&report) } - /// Opens the index for `path` (default cwd). If this process is the writer for - /// the root, the opened reader is cached and evicted on each reindex; any other - /// process opens fresh per query so it reflects the writer's latest commit. A - /// missing index is not built here — call `oxcode_watch` first. + /// Opens the index for `path` (default: workspace root). If this process is the + /// writer for the root, the opened reader is cached and evicted on each reindex; + /// any other process opens fresh per query so it reflects the writer's latest + /// commit. A missing index is not built here — call `oxcode_watch` first. async fn index_for(&self, path: Option) -> Result, McpError> { - let root = resolve_root(path); + let root = self.resolve_root(path).await?; if self.is_writer(&root) { if let Some(index) = self.indexes.lock().await.get(&root) { return Ok(Arc::clone(index)); @@ -562,6 +574,36 @@ impl OxcodeServer { } ("reader", false, 0) } + + /// Resolves the project root from an optional `path`, preferring workspace + /// signals over this process's cwd. MCP hosts often start servers with + /// `cwd=$HOME` even when the agent is in a project folder; omitting `path` + /// must not silently index the home directory. + /// + /// Never calls `roots/list` here — nested client requests during tool handling + /// can hang on some hosts. Roots are fetched from `on_initialized` / + /// `on_roots_list_changed` only; this waits briefly for that in-flight fetch + /// when the cache is still cold. + async fn resolve_root(&self, path: Option) -> Result { + // Explicit path / OXCODE_ROOT pin win without waiting on MCP roots. + // Host env snapshots (CLAUDE_PROJECT_DIR / WORKSPACE_FOLDER_PATHS) do + // not — they are fixed at process start and must lose to a refreshed + // roots cache after a folder switch. + if path + .as_deref() + .map(str::trim) + .is_some_and(|value| !value.is_empty()) + || project_root::oxcode_root_override().is_some() + { + return resolve_project_root(path, None, true); + } + match self.roots.wait_ready().await { + RootsWait::Ready(mcp_root) => resolve_project_root(path, mcp_root, true), + // Refresh timed out: do not fall through to sticky host env, which + // can be older than the MCP root being replaced. + RootsWait::RefreshInFlight => resolve_project_root(path, None, false), + } + } } /// Re-indexes `root` on each debounced change tick until the watcher stops. @@ -671,15 +713,14 @@ impl ServerHandler for OxcodeServer { ) .with_instructions(INSTRUCTIONS) } -} -/// Resolves the project root from an optional path argument, canonicalizing -/// best-effort so the reader cache, the writer registry, and the lock file all key -/// on the same absolute path (FS events report canonical paths). Falls back to the -/// raw path when it does not exist yet. -fn resolve_root(path: Option) -> PathBuf { - let raw = PathBuf::from(path.unwrap_or_else(|| ".".to_owned())); - std::fs::canonicalize(&raw).unwrap_or(raw) + async fn on_initialized(&self, context: NotificationContext) { + self.roots.fetch(&context.peer).await; + } + + async fn on_roots_list_changed(&self, context: NotificationContext) { + self.roots.fetch(&context.peer).await; + } } /// Parses caller-supplied kind strings into `NodeKind`, dropping unknown ones. @@ -723,358 +764,5 @@ fn json_result(value: &T) -> Result RunningService { - let (server_transport, client_transport) = tokio::io::duplex(4096); - tokio::spawn(async move { - let server = OxcodeServer::new_with(debounce, poll) - .serve(server_transport) - .await - .expect("server serve"); - let _ = server.waiting().await; - }); - TestClient - .serve(client_transport) - .await - .expect("client connect") - } - - /// Writes a minimal two-function Rust project into a fresh temp dir. - fn rust_project() -> tempfile::TempDir { - let temp = tempfile::TempDir::new().expect("temp dir"); - std::fs::create_dir_all(temp.path().join("src")).expect("mkdir src"); - std::fs::write( - temp.path().join("src/lib.rs"), - "pub fn helper() {}\npub fn entry() {\n helper();\n}\n", - ) - .expect("write lib.rs"); - temp - } - - /// Builds a tool-call params object for `name` with JSON `arguments`. - fn tool_call(name: &'static str, arguments: serde_json::Value) -> CallToolRequestParams { - let mut params = CallToolRequestParams::new(name); - params.arguments = arguments.as_object().cloned(); - params - } - - /// Extracts the single text content block from a tool result. - fn result_text(result: &CallToolResult) -> &str { - result - .content - .first() - .and_then(|content| content.as_text()) - .map(|text| text.text.as_str()) - .expect("text content") - } - - /// Calls `oxcode_watch` for `path` and returns the parsed JSON response. - async fn watch( - client: &RunningService, - path: &str, - ) -> serde_json::Value { - let result = client - .call_tool(tool_call( - "oxcode_watch", - serde_json::json!({ "path": path }), - )) - .await - .expect("watch call"); - serde_json::from_str(result_text(&result)).expect("watch json") - } - - /// Polls `oxcode_search` (bounded) until `name` actually appears as a match. - /// Inspects the parsed `matches` array — not a substring of the JSON, which - /// would falsely match the echoed `query` field. - async fn poll_symbol_indexed( - client: &RunningService, - path: &str, - name: &str, - ) -> bool { - for _ in 0..100 { - let searched = client - .call_tool(tool_call( - "oxcode_search", - serde_json::json!({ "path": path, "query": name }), - )) - .await - .expect("search call"); - let report: serde_json::Value = - serde_json::from_str(result_text(&searched)).expect("search json"); - // Keyword search is fuzzy, so check for an exact-named match rather - // than "any match" (which would falsely fire on weak candidates). - let matched = report["matches"] - .as_array() - .is_some_and(|matches| matches.iter().any(|entry| entry["symbol"]["name"] == name)); - if matched { - return true; - } - tokio::time::sleep(Duration::from_millis(50)).await; - } - false - } - - /// Polls `tasks/get` until the task reaches a terminal status (or times out). - async fn poll_until_terminal( - client: &RunningService, - task_id: &str, - ) -> TaskStatus { - let mut status = TaskStatus::Working; - for _ in 0..200 { - let info = client - .send_request(ClientRequest::GetTaskInfoRequest(Request::new( - GetTaskInfoParams { - meta: None, - task_id: task_id.to_owned(), - }, - ))) - .await - .expect("tasks/get"); - if let ServerResult::GetTaskResult(result) = info { - status = result.task.status; - } - if status != TaskStatus::Working { - break; - } - tokio::time::sleep(Duration::from_millis(20)).await; - } - status - } - - /// `flock` is per open-file-description on macOS/Linux: a second independent - /// open of the same path cannot take the lock the first holds. This pins the - /// platform behavior the writer election depends on. - #[test] - fn watch_lock_is_exclusive_per_handle() { - let temp = tempfile::TempDir::new().expect("temp dir"); - let path = temp.path().join("watch.lock"); - let first = OpenOptions::new() - .create(true) - .truncate(false) - .read(true) - .write(true) - .open(&path) - .expect("open first"); - first.try_lock().expect("first acquires"); - let second = OpenOptions::new() - .read(true) - .write(true) - .open(&path) - .expect("open second"); - assert!( - matches!(second.try_lock(), Err(TryLockError::WouldBlock)), - "a second handle cannot take the held lock" - ); - } - - #[tokio::test] - async fn lists_tools_with_watch_and_explore_task_support() { - let client = connect(DEFAULT_DEBOUNCE, DEFAULT_POLL).await; - let tools = client.list_all_tools().await.expect("list tools"); - - assert!( - tools.iter().any(|tool| tool.name == "oxcode_watch"), - "oxcode_watch is registered" - ); - assert!( - tools.iter().all(|tool| tool.name != "oxcode_index"), - "the old write tool is gone" - ); - - let task_support = |name: &str| { - tools - .iter() - .find(|tool| tool.name == name) - .and_then(|tool| tool.execution.as_ref()) - .and_then(|execution| execution.task_support) - }; - assert_eq!(task_support("oxcode_watch"), Some(TaskSupport::Optional)); - assert_eq!(task_support("oxcode_explore"), Some(TaskSupport::Optional)); - assert_eq!(task_support("oxcode_search"), None); - assert_eq!(task_support("oxcode_status"), None); - } - - #[tokio::test] - async fn watch_elects_writer_and_serves_queries() { - let project = rust_project(); - let path = project.path().to_string_lossy().into_owned(); - let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; - - let watched = watch(&client, &path).await; - assert_eq!(watched["role"], "writer", "first watcher is the writer"); - assert_eq!(watched["watching"], true); - - let explored = client - .call_tool(tool_call( - "oxcode_explore", - serde_json::json!({ "path": path, "query": "entry" }), - )) - .await - .expect("explore call"); - assert!( - result_text(&explored).contains("entry"), - "writer's index is queryable" - ); - } - - #[tokio::test] - async fn second_watcher_on_same_root_is_standby() { - let project = rust_project(); - let path = project.path().to_string_lossy().into_owned(); - let writer_client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; - let standby_client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; - - assert_eq!(watch(&writer_client, &path).await["role"], "writer"); - // Second server, same root: the lock is held, so it becomes a standby. - assert_eq!(watch(&standby_client, &path).await["role"], "standby"); - - // The standby still answers queries off the shared on-disk index. - let explored = standby_client - .call_tool(tool_call( - "oxcode_explore", - serde_json::json!({ "path": path, "query": "entry" }), - )) - .await - .expect("reader explore"); - assert!(result_text(&explored).contains("entry")); - } - - #[tokio::test] - async fn query_without_watch_errors_when_no_index() { - let project = rust_project(); - let path = project.path().to_string_lossy().into_owned(); - let client = connect(DEFAULT_DEBOUNCE, DEFAULT_POLL).await; - - // No oxcode_watch, no prior index: a query must not build; it hints instead. - let result = client - .call_tool(tool_call( - "oxcode_explore", - serde_json::json!({ "path": path, "query": "entry" }), - )) - .await; - assert!( - result.is_err(), - "query before oxcode_watch errors with a hint, never silently builds" - ); - } - - #[tokio::test] - async fn writer_auto_reindexes_on_change() { - let project = rust_project(); - let path = project.path().to_string_lossy().into_owned(); - let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; - - assert_eq!(watch(&client, &path).await["role"], "writer"); - - // Let the FS-event stream establish before the change: FSEvents (and - // other backends) have a startup window where a change can land as - // initial state and go unreported. - tokio::time::sleep(Duration::from_millis(300)).await; - std::fs::write( - project.path().join("src/extra.rs"), - "pub fn brand_new_symbol() {}\n", - ) - .expect("write extra.rs"); - - let found = poll_symbol_indexed(&client, &path, "brand_new_symbol").await; - assert!( - found, - "the writer's watcher re-indexed and surfaced the symbol" - ); - - let status: serde_json::Value = serde_json::from_str(result_text( - &client - .call_tool(tool_call( - "oxcode_status", - serde_json::json!({ "path": path }), - )) - .await - .expect("status call"), - )) - .expect("status json"); - assert_eq!(status["watch"]["role"], "writer"); - assert!( - status["watch"]["reindexes"].as_u64().unwrap_or(0) >= 2, - "writer reindexed at least the initial build and the change" - ); - } - - #[tokio::test] - async fn task_augmented_watch_completes() { - let project = rust_project(); - let path = project.path().to_string_lossy().into_owned(); - let client = connect(Duration::from_millis(50), Duration::from_millis(150)).await; - - // Task-augment the call: typed `call_tool` cannot carry a task field, so - // send the request directly and expect an immediate CreateTaskResult. - let mut params = tool_call("oxcode_watch", serde_json::json!({ "path": path })); - params.task = serde_json::json!({ "ttl": 60_000 }).as_object().cloned(); - let created = client - .send_request(ClientRequest::CallToolRequest(Request::new(params))) - .await - .expect("enqueue task"); - let task_id = match created { - ServerResult::CreateTaskResult(result) => { - assert_eq!(result.task.status, TaskStatus::Working); - result.task.task_id - } - other => panic!("expected CreateTaskResult, got {other:?}"), - }; - - let status = poll_until_terminal(&client, &task_id).await; - assert_eq!( - status, - TaskStatus::Completed, - "watch task ran to completion" - ); - - let payload = client - .send_request(ClientRequest::GetTaskResultRequest(Request::new( - GetTaskResultParams { - meta: None, - task_id, - }, - ))) - .await - .expect("tasks/result"); - let text = match payload { - ServerResult::CallToolResult(result) => result_text(&result).to_owned(), - ServerResult::GetTaskPayloadResult(payload) => payload.0["content"][0]["text"] - .as_str() - .expect("tool result text") - .to_owned(), - other => panic!("expected the deferred tool result, got {other:?}"), - }; - assert!( - text.contains("writer"), - "deferred watch result reports the elected writer role" - ); - } -} +#[path = "integration_tests.rs"] +mod tests; diff --git a/crates/oxcode-cli/src/mcp/project_root.rs b/crates/oxcode-cli/src/mcp/project_root.rs new file mode 100644 index 0000000..c52f676 --- /dev/null +++ b/crates/oxcode-cli/src/mcp/project_root.rs @@ -0,0 +1,334 @@ +//! Resolve the project root for omitted MCP `path` arguments. +//! +//! MCP hosts often start the server process with `cwd=$HOME` even when the +//! agent is in a project folder. Prefer a live MCP roots cache (and then +//! host-injected env snapshots) over process cwd, and refuse `$HOME` unless +//! `path` was explicit. + +use std::path::{Path, PathBuf}; + +use rmcp::{ErrorData as McpError, schemars}; +use serde::Deserialize; + +/// Shared optional `path` field for every tool that accepts a project root. +/// +/// Flattened into each tool's params struct so the wire shape stays `path?` +/// while the defaulting docs live in one place. +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +pub(crate) struct OptionalProjectRoot { + /// Project root; defaults to the workspace (`OXCODE_ROOT` / MCP roots / + /// `CLAUDE_PROJECT_DIR` / `WORKSPACE_FOLDER_PATHS`), never silently to + /// `$HOME`. + pub path: Option, +} + +/// Resolves an optional tool `path` against env defaults and a ready MCP root. +/// +/// Order for omitted `path`: `OXCODE_ROOT` (explicit pin) → `mcp_root` (live +/// client roots) → host env snapshots (`CLAUDE_PROJECT_DIR` / +/// `WORKSPACE_FOLDER_PATHS`) → process cwd. Host env sits below MCP roots +/// because those vars are fixed at process start while roots refresh on +/// `roots/list_changed`. +/// +/// When `allow_host_env` is false (roots refresh timed out in flight), host +/// snapshots are skipped so a sticky pre-switch env cannot win over an +/// in-progress roots update. +/// +/// `mcp_root` is the first client workspace root already fetched outside the +/// tool handler (see [`super::roots::RootsCache`]); this never calls +/// `roots/list`. +pub(crate) fn resolve_project_root( + path: Option, + mcp_root: Option, + allow_host_env: bool, +) -> Result { + let explicit = path + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(PathBuf::from); + let raw = if let Some(explicit_path) = explicit.clone() { + explicit_path + } else if let Some(pin) = oxcode_root_override() { + pin + } else if let Some(from_roots) = mcp_root { + from_roots + } else if allow_host_env { + if let Some(from_host) = host_env_project_root() { + from_host + } else { + std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")) + } + } else { + std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")) + }; + let root = canonicalize_root(raw); + if explicit.is_none() && is_home_directory(&root) { + return Err(McpError::invalid_params( + format!( + "refusing to use home directory {} as the project root — MCP hosts often \ + start this server with cwd=$HOME even when your workspace is elsewhere. \ + Pass `path` (the project folder), or set OXCODE_ROOT / CLAUDE_PROJECT_DIR \ + / WORKSPACE_FOLDER_PATHS.", + root.display() + ), + None, + )); + } + Ok(root) +} + +/// Explicit user/config pin. Wins over live MCP roots and host env snapshots. +pub(crate) fn oxcode_root_override() -> Option { + non_empty_env("OXCODE_ROOT") +} + +/// Host-injected workspace snapshots (Claude Code / Cursor). +/// +/// These are set at process start and typically do not update when the client +/// switches folders via `roots/list_changed`, so callers should prefer a ready +/// MCP root when one is available. +pub(crate) fn host_env_project_root() -> Option { + if let Some(path) = non_empty_env("CLAUDE_PROJECT_DIR") { + return Some(path); + } + std::env::var("WORKSPACE_FOLDER_PATHS") + .ok() + .and_then(|value| first_workspace_folder(&value)) +} + +fn non_empty_env(key: &str) -> Option { + std::env::var(key).ok().and_then(|value| { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(PathBuf::from(trimmed)) + } + }) +} + +/// First folder from a `WORKSPACE_FOLDER_PATHS` value (single path or CSV). +pub(crate) fn first_workspace_folder(value: &str) -> Option { + let trimmed = value.trim(); + if trimmed.is_empty() { + return None; + } + let as_path = PathBuf::from(trimmed); + if as_path.is_dir() { + return Some(as_path); + } + // Multi-root workspaces: comma-separated. Do not split on `:` — that breaks + // Windows drive letters (`C:\...`). + trimmed + .split(',') + .map(str::trim) + .find(|part| !part.is_empty()) + .map(PathBuf::from) +} + +/// Whether `path` is the current user's home directory (best-effort). +pub(crate) fn is_home_directory(path: &Path) -> bool { + let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")) else { + return false; + }; + let home = PathBuf::from(home); + canonicalize_root(path.to_path_buf()) == canonicalize_root(home) +} + +/// Canonicalizes best-effort so reader cache / writer registry / lock file key on +/// the same absolute path (FS events report canonical paths). Falls back to the +/// raw path when it does not exist yet. +pub(crate) fn canonicalize_root(path: PathBuf) -> PathBuf { + std::fs::canonicalize(&path).unwrap_or(path) +} + +/// Converts a `file://` MCP root URI into a filesystem path. +pub(crate) fn file_uri_to_path(uri: &str) -> Option { + let rest = uri.strip_prefix("file://")?; + let path = if let Some(path) = rest.strip_prefix("localhost") { + path + } else if rest.starts_with('/') { + rest + } else { + return None; + }; + let decoded = percent_decode(path); + if decoded.is_empty() { + return None; + } + // `file:///C:/Users/...` percent-decodes to `/C:/Users/...`. On Windows the + // usable path is `C:/Users/...` — same as `Url::to_file_path`. + let normalized = match decoded.as_bytes() { + [b'/', drive, b':', ..] if drive.is_ascii_alphabetic() => decoded[1..].to_owned(), + _ => decoded, + }; + Some(PathBuf::from(normalized)) +} + +/// Decodes `%XX` sequences in a URI path; returns the input unchanged when none. +fn percent_decode(input: &str) -> String { + if !input.contains('%') { + return input.to_owned(); + } + let bytes = input.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + if bytes[index] == b'%' + && index + 2 < bytes.len() + && let (Some(high), Some(low)) = + (hex_nibble(bytes[index + 1]), hex_nibble(bytes[index + 2])) + { + out.push((high << 4) | low); + index += 3; + continue; + } + out.push(bytes[index]); + index += 1; + } + String::from_utf8_lossy(&out).into_owned() +} + +fn hex_nibble(byte: u8) -> Option { + match byte { + b'0'..=b'9' => Some(byte - b'0'), + b'a'..=b'f' => Some(byte - b'a' + 10), + b'A'..=b'F' => Some(byte - b'A' + 10), + _ => None, + } +} + +/// Serializes tests that mutate project-root / HOME env vars. +#[cfg(test)] +pub(crate) static PROJECT_ROOT_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +#[cfg(test)] +fn restore_env(key: &str, previous: Option) { + // SAFETY: caller holds [`PROJECT_ROOT_ENV_LOCK`]. + unsafe { + match previous { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn first_workspace_folder_accepts_csv_and_single_path() { + let project = tempfile::TempDir::new().expect("temp"); + let path = project.path().to_string_lossy().into_owned(); + assert_eq!( + first_workspace_folder(&path), + Some(PathBuf::from(&path)), + "existing single path wins without splitting" + ); + assert_eq!( + first_workspace_folder(&format!("{path},/does/not/exist")), + Some(PathBuf::from(&path)) + ); + assert_eq!( + first_workspace_folder("/missing/a,/missing/b"), + Some(PathBuf::from("/missing/a")) + ); + } + + #[test] + fn resolve_project_root_prefers_mcp_roots_over_host_env() { + let _lock = PROJECT_ROOT_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let host = tempfile::TempDir::new().expect("host"); + let live = tempfile::TempDir::new().expect("live"); + let previous_claude = std::env::var_os("CLAUDE_PROJECT_DIR"); + let previous_oxcode = std::env::var_os("OXCODE_ROOT"); + let previous_workspace = std::env::var_os("WORKSPACE_FOLDER_PATHS"); + // SAFETY: held exclusively via PROJECT_ROOT_ENV_LOCK. + unsafe { + std::env::set_var("CLAUDE_PROJECT_DIR", host.path()); + std::env::remove_var("OXCODE_ROOT"); + std::env::remove_var("WORKSPACE_FOLDER_PATHS"); + } + let with_roots = resolve_project_root(None, Some(live.path().to_path_buf()), true) + .expect("mcp roots win"); + assert_eq!( + with_roots, + canonicalize_root(live.path().to_path_buf()), + "live MCP roots must beat sticky CLAUDE_PROJECT_DIR" + ); + let without_roots = resolve_project_root(None, None, true).expect("host env fallback"); + assert_eq!( + without_roots, + canonicalize_root(host.path().to_path_buf()), + "host env is used only when MCP roots are absent" + ); + let suppressed = resolve_project_root(None, None, false).expect("no host env"); + assert_ne!( + suppressed, + canonicalize_root(host.path().to_path_buf()), + "refresh-in-flight must not fall back to sticky host env" + ); + restore_env("CLAUDE_PROJECT_DIR", previous_claude); + restore_env("OXCODE_ROOT", previous_oxcode); + restore_env("WORKSPACE_FOLDER_PATHS", previous_workspace); + } + + #[test] + fn resolve_project_root_oxcode_root_pins_over_mcp_roots() { + let _lock = PROJECT_ROOT_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let pin = tempfile::TempDir::new().expect("pin"); + let live = tempfile::TempDir::new().expect("live"); + let previous = std::env::var_os("OXCODE_ROOT"); + // SAFETY: held exclusively via PROJECT_ROOT_ENV_LOCK. + unsafe { + std::env::set_var("OXCODE_ROOT", pin.path()); + } + let resolved = + resolve_project_root(None, Some(live.path().to_path_buf()), true).expect("pin"); + assert_eq!( + resolved, + canonicalize_root(pin.path().to_path_buf()), + "OXCODE_ROOT is an explicit pin above live MCP roots" + ); + restore_env("OXCODE_ROOT", previous); + } + + #[test] + fn file_uri_to_path_decodes_file_roots() { + assert_eq!( + file_uri_to_path("file:///Users/snowmead/opt/jinttai"), + Some(PathBuf::from("/Users/snowmead/opt/jinttai")) + ); + assert_eq!( + file_uri_to_path("file://localhost/tmp/project%20name"), + Some(PathBuf::from("/tmp/project name")) + ); + assert_eq!( + file_uri_to_path("file:///C:/Users/snowmead/opt/jinttai"), + Some(PathBuf::from("C:/Users/snowmead/opt/jinttai")) + ); + assert_eq!(file_uri_to_path("https://example.com"), None); + } + + #[test] + fn is_home_directory_matches_home_env() { + let _lock = PROJECT_ROOT_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let home = tempfile::TempDir::new().expect("home"); + let previous_home = std::env::var_os("HOME"); + // SAFETY: held exclusively via PROJECT_ROOT_ENV_LOCK. + unsafe { + std::env::set_var("HOME", home.path()); + } + assert!(is_home_directory(home.path())); + assert!(!is_home_directory(&home.path().join("opt/jinttai"))); + restore_env("HOME", previous_home); + } +} diff --git a/crates/oxcode-cli/src/mcp/roots.rs b/crates/oxcode-cli/src/mcp/roots.rs new file mode 100644 index 0000000..ee7d796 --- /dev/null +++ b/crates/oxcode-cli/src/mcp/roots.rs @@ -0,0 +1,209 @@ +//! Client MCP workspace roots, fetched outside tool handlers. +//! +//! Nested `roots/list` during a tool call can hang some hosts. This cache is +//! populated from `on_initialized` / `on_roots_list_changed` only; tool +//! handlers wait for a completed publication. + +use std::{path::PathBuf, sync::Arc, time::Duration}; + +use rmcp::{Peer, RoleServer, model::Root}; +use tokio::sync::{Mutex, watch}; + +use super::project_root::file_uri_to_path; + +/// How long an omitted-`path` resolve will wait for a roots fetch before +/// falling through. +const ROOTS_READY_WAIT: Duration = Duration::from_millis(500); + +/// Published state of the client's MCP workspace roots. +#[derive(Debug, Clone, PartialEq, Eq)] +enum RootsState { + /// No fetch has completed yet (cold start). + Cold, + /// A refresh is in flight; holds the last completed root for failure restore. + Refreshing(Option), + /// Last completed fetch. `None` means the client has no usable root. + Ready(Option), +} + +/// Outcome of waiting for the roots cache from a tool handler. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum RootsWait { + /// A completed fetch is available (`None` = client has no usable root). + Ready(Option), + /// Timed out while replacing a previously ready root. Callers must not fall + /// back to sticky host env snapshots (those can be older than the root being + /// replaced). Cold-start / `Refreshing(None)` timeouts are [`Ready`]`(None)` + /// instead, so host env remains available. + RefreshInFlight, +} + +/// Outcome of one `roots/list` attempt. +enum LoadOutcome { + /// Client advertised no roots capability, or the RPC failed. + Unavailable, + /// Client returned an empty roots array. + Empty, + /// Client returned URIs but none parsed to a filesystem path. + Unparseable, + /// First usable filesystem path. + Parsed(PathBuf), +} + +/// Single watch-backed cache for MCP roots. +/// +/// Fetches are serialized so overlapping `initialized` / `roots/list_changed` +/// notifications cannot wipe or restore the wrong root. +#[derive(Clone)] +pub(crate) struct RootsCache { + tx: Arc>, + rx: watch::Receiver, + fetch_lock: Arc>, +} + +impl RootsCache { + #[must_use] + pub(crate) fn new() -> Self { + let (tx, rx) = watch::channel(RootsState::Cold); + Self { + tx: Arc::new(tx), + rx, + fetch_lock: Arc::new(Mutex::new(())), + } + } + + /// Waits briefly for a completed fetch. + /// + /// Never issues `roots/list`. On timeout during a refresh, returns + /// [`RootsWait::RefreshInFlight`] so callers do not use a sticky host env + /// snapshot that may predate the workspace being refreshed. + pub(crate) async fn wait_ready(&self) -> RootsWait { + if let RootsState::Ready(root) = &*self.rx.borrow() { + return RootsWait::Ready(root.clone()); + } + let mut rx = self.rx.clone(); + let finished = tokio::time::timeout( + ROOTS_READY_WAIT, + rx.wait_for(|state| matches!(state, RootsState::Ready(_))), + ) + .await; + match finished { + Ok(Ok(state)) => match &*state { + RootsState::Ready(root) => RootsWait::Ready(root.clone()), + RootsState::Cold | RootsState::Refreshing(_) => RootsWait::Ready(None), + }, + _ => match &*self.rx.borrow() { + RootsState::Ready(root) => RootsWait::Ready(root.clone()), + // Only suppress host env when replacing a previously ready root. + // Cold start / Refreshing(None) still allows CLAUDE_PROJECT_DIR + // and WORKSPACE_FOLDER_PATHS if roots/list is slow. + RootsState::Refreshing(Some(_)) => RootsWait::RefreshInFlight, + RootsState::Refreshing(None) | RootsState::Cold => RootsWait::Ready(None), + }, + } + } + + /// Fetches `roots/list` and publishes [`RootsState::Ready`]. + /// + /// Serialized: only one fetch runs at a time. Marks + /// [`RootsState::Refreshing`] first so concurrent waiters observe the + /// refresh. RPC failures and unparseable URI lists restore the previous + /// ready root; an explicitly empty list clears it. + pub(crate) async fn fetch(&self, peer: &Peer) { + let _guard = self.fetch_lock.lock().await; + let previous = match &*self.rx.borrow() { + RootsState::Ready(root) | RootsState::Refreshing(root) => root.clone(), + RootsState::Cold => None, + }; + let _ = self.tx.send(RootsState::Refreshing(previous.clone())); + let published = match self.load_root(peer).await { + LoadOutcome::Empty => None, + LoadOutcome::Parsed(path) => Some(path), + LoadOutcome::Unavailable | LoadOutcome::Unparseable => previous, + }; + let _ = self.tx.send(RootsState::Ready(published)); + } + + async fn load_root(&self, peer: &Peer) -> LoadOutcome { + if peer + .peer_info() + .and_then(|info| info.capabilities.roots.as_ref()) + .is_none() + { + return LoadOutcome::Unavailable; + } + let Ok(result) = peer.list_roots().await else { + return LoadOutcome::Unavailable; + }; + classify_roots_list(result.roots) + } +} + +/// Classifies a `roots/list` payload. +fn classify_roots_list(roots: Vec) -> LoadOutcome { + if roots.is_empty() { + return LoadOutcome::Empty; + } + match roots + .iter() + .filter_map(|root| file_uri_to_path(&root.uri)) + .next() + { + Some(path) => LoadOutcome::Parsed(std::fs::canonicalize(&path).unwrap_or(path)), + None => LoadOutcome::Unparseable, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn classify_roots_list_empty_unparseable_and_parsed() { + assert!(matches!(classify_roots_list(vec![]), LoadOutcome::Empty)); + assert!(matches!( + classify_roots_list(vec![Root::new("https://example.com")]), + LoadOutcome::Unparseable + )); + let project = tempfile::TempDir::new().expect("temp"); + let uri = format!("file://{}", project.path().display()); + assert!(matches!( + classify_roots_list(vec![Root::new(uri)]), + LoadOutcome::Parsed(path) if path == project.path() + )); + } + + #[tokio::test] + async fn wait_ready_timeout_during_refresh_is_in_flight() { + let cache = RootsCache::new(); + let _ = cache + .tx + .send(RootsState::Refreshing(Some(PathBuf::from("/old")))); + assert_eq!(cache.wait_ready().await, RootsWait::RefreshInFlight); + } + + #[tokio::test] + async fn wait_ready_timeout_on_first_fetch_allows_host_env() { + let cache = RootsCache::new(); + let _ = cache.tx.send(RootsState::Refreshing(None)); + assert_eq!( + cache.wait_ready().await, + RootsWait::Ready(None), + "cold-start Refreshing(None) must not suppress host env" + ); + } + + #[tokio::test] + async fn wait_ready_timeout_while_cold_allows_host_env_fallback() { + let cache = RootsCache::new(); + assert_eq!(cache.wait_ready().await, RootsWait::Ready(None)); + } + + #[tokio::test] + async fn wait_ready_returns_ready_root() { + let cache = RootsCache::new(); + let root = PathBuf::from("/project"); + let _ = cache.tx.send(RootsState::Ready(Some(root.clone()))); + assert_eq!(cache.wait_ready().await, RootsWait::Ready(Some(root))); + } +} diff --git a/prompts/arms/oxcode-mcp.md b/prompts/arms/oxcode-mcp.md index 31cbbf7..7051d04 100644 --- a/prompts/arms/oxcode-mcp.md +++ b/prompts/arms/oxcode-mcp.md @@ -12,6 +12,6 @@ Available tools: - `oxcode_files { query, path?, limit? }` — keyword search over indexed files. - `oxcode_status { path? }` — index status (element/relation counts). -Selectors may be qualified names, `name:`, `element:`, or `file::`. The `path` argument defaults to the indexed repository, so you can omit it. +Selectors may be qualified names, `name:`, `element:`, or `file::`. The `path` argument defaults to the workspace project root (`OXCODE_ROOT` / MCP roots / `CLAUDE_PROJECT_DIR` / `WORKSPACE_FOLDER_PATHS`), so you can omit it — pass it explicitly if the host did not advertise a workspace. Tool results are JSON with definition paths, line ranges, signatures, docstrings, source previews, and relationship call sites. Use those fields as evidence; do not open files just to recover line numbers or a short definition already present in the tool output. After `oxcode_explore`, use at most two targeted follow-up tool calls unless you are stuck.