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
21 changes: 19 additions & 2 deletions crates/uffs-daemon/src/index/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,25 @@ impl IndexManager {
/// Get current daemon status.
///
/// Includes `has_drives` and `total_records` for completeness.
///
/// # Concurrency
///
/// Snapshots the `DaemonStatus` upfront via `.read().await.clone()`
/// rather than holding the read guard across the inner awaits
/// below. Without the snapshot, the guard would be held across
/// [`Self::has_drives`], [`Self::total_records`], and
/// [`Self::snapshot`] — three independent `self.index.read().await`
/// acquisitions — blocking any concurrent
/// [`Self::set_ready`] / [`Self::set_loading_progress`] /
/// [`crate::index::refresh::IndexManager::refresh`] writer on
/// `self.status` for the duration of the status RPC (which on a
/// many-drive box with a slow snapshot path can be tens of
/// milliseconds). `DaemonStatus` is a small `Clone` enum
/// (`Loading { usize, usize }` or `Refreshing { Vec<DriveLetter> }`
/// in the worst case), so the snapshot is microseconds and never
/// crosses an inner `.await`. See Phase 10b audit findings.
pub(crate) async fn status(&self, connections: usize) -> StatusResponse {
let status = self.status.read().await;
let status_snapshot = self.status.read().await.clone();
let loaded = self.has_drives().await;
let records = self.total_records().await;
tracing::trace!(
Expand Down Expand Up @@ -115,7 +132,7 @@ impl IndexManager {
});

StatusResponse {
status: status.clone(),
status: status_snapshot,
uptime_secs: self.start_time.elapsed().as_secs(),
connections,
pid: std::process::id(),
Expand Down
19 changes: 18 additions & 1 deletion crates/uffs-mcp/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,23 @@ impl UffsMcpServer {
///
/// Separated from `call_tool` so the retry-on-reconnect logic can
/// call it a second time with the same arguments.
///
/// # Concurrency
///
/// Snapshots the [`RootsState`] upfront via `.read().await.clone()`
/// rather than holding the read guard across the daemon-RPC tool
/// dispatch awaits below. Without the snapshot, the guard would
/// be held for the duration of every tool call (potentially
/// seconds for a large `uffs_search` / `uffs_aggregate`), blocking
/// any concurrent [`ServerHandler::on_roots_list_changed`] writer
/// — which acquires `self.roots.write().await` to refresh the
/// state — for the duration of every in-flight tool call across
/// the whole MCP session. Cloning `RootsState` is cheap (small
/// `Vec<RootScope>` of typically `< 10` short-string entries; see
/// the type's rustdoc). See Phase 10b audit findings.
///
/// [`RootsState`]: crate::roots::RootsState
/// [`ServerHandler::on_roots_list_changed`]: rmcp::handler::server::ServerHandler::on_roots_list_changed
async fn dispatch_tool(
&self,
tool_name: &str,
Expand All @@ -253,7 +270,7 @@ impl UffsMcpServer {
Self::readiness_gate(&mut client).await?;
}

let roots_state = self.roots.read().await;
let roots_state = self.roots.read().await.clone();

match tool_name {
"uffs_search" => {
Expand Down
9 changes: 8 additions & 1 deletion crates/uffs-mcp/src/roots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ pub struct RootScope {
}

/// Shared roots state held by the MCP server.
#[derive(Debug, Default)]
///
/// `Clone` is derived so the MCP handler can snapshot the current
/// state under a brief read guard and then release the lock before
/// dispatching tools that await on the daemon RPC. Cloning is cheap:
/// the `roots` and `warnings` `Vec`s are typically `< 10` short
/// `String`s each (one per workspace root). See Phase 10b audit
/// findings and `crate::handler::UffsMcpServer::call_tool_with_args`.
#[derive(Debug, Default, Clone)]
pub struct RootsState {
/// Whether the client has advertised roots at least once.
pub advertised: bool,
Expand Down
Loading