diff --git a/AGENTS.develop.md b/AGENTS.develop.md index b68a6d40..b3b9bf18 100644 --- a/AGENTS.develop.md +++ b/AGENTS.develop.md @@ -241,6 +241,7 @@ Never `unwrap_or_default()` a store error on a search path — "no results" and ## Changelog highlights (recent) +- **v1.5.33 (pending)** — /status + MCP status answer from a background snapshot (never measure inline): always responsive under rebuild waves, with `snapshot_age_secs` + a `load` summary for calling agents (`CODESEARCH_STATUS_REFRESH_SECS`, default 2s) - **v1.5.31 (pending)** — scip-csharp VS-MSBuild restore fallback: NETSDK1045-class CLI SDK gaps (hollow SDK dir, net10-only-in-VS) retry via vswhere + `MSBuild -t:restore`, and MSBuild registration picks a VS instance when no CLI SDK can target the repo - **v1.5.31 (pending)** — serve TUI status: `Indexing` only for real text indexing (pure SCIP rebuilds drive the C#/TS columns only); resting repos read `Idle` again - **v1.5.16 (pending)** — C# restore-pending retries 20×60s → 3 attempts with exponential backoff; restore exhaustion renders as yellow C#⚠ partial on every serve path instead of a red error diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f1d1f1d..6930d7a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ more PRs land; when the release is actually tagged, the same section is finalized in place with a date — no renaming/migration step needed. --> +## [1.5.33] + +### Fixed + +- **`GET /status` and the MCP `status` tool always answer, even under a heavy rebuild wave.** They used to measure inline in the request path: per registered repo several filesystem probes plus a SCIP LMDB env open and a sysinfo refresh, synchronous on async workers that a rebuild wave (dotnet restore + scip-csharp children) starves — with 28 repos one call took tens of seconds and polling clients (remote TUI, calling agents) piled up and timed out; the project list died the same death. Now a single background task measures (on the blocking pool, one pass at a time — under load the cadence stretches instead of piling measuring tasks onto a starved machine) and stores a snapshot whose readers answer in microseconds. Every answer carries `snapshot_age_secs` (staleness is visible, never a hang) and a `load` summary (`text_indexing` / `symbol_indexing` / `errored` / `refresh_interval_secs`) so calling agents can throttle themselves; before the first pass completes they answer `"starting"` immediately. Cadence: `CODESEARCH_STATUS_REFRESH_SECS`, default 2s. + ## [1.5.31] ### Changed diff --git a/src/constants.rs b/src/constants.rs index 5f7dbc34..197609a1 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -616,6 +616,12 @@ pub const REMOTE_PEER_RETRY_BACKOFF_MS: &[u64] = &[3000, 8000]; /// single millisecond value (test hook so retry tests don't sleep for real). pub const REMOTE_PEER_RETRY_BACKOFF_ENV: &str = "CODESEARCH_REMOTE_RETRY_BACKOFF_MS"; +/// Cadence of the background status-snapshot refresher (seconds). Read +/// endpoints answer from the stored snapshot, so this only trades staleness +/// (`snapshot_age_secs`) for probing cost — under load the cadence +/// stretches on its own; this knob only sets the idle pace. +pub const STATUS_REFRESH_SECS_ENV: &str = "CODESEARCH_STATUS_REFRESH_SECS"; + /// Cooperative join window (seconds) for `await_index_task` and /// `await_fsw_shutdown`: how long a background indexing / file-watcher task /// is given to observe its `CancellationToken` and exit on its own before it diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index ac0a38a7..530e1b1e 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -121,6 +121,8 @@ pub(crate) use helpers::*; pub(crate) use instructions::*; pub(crate) use responses::*; pub use runtime::*; +/// The projects-measure pass for the serve status-snapshot background task. +pub(crate) use status::compute_projects_snapshot; /// Re-order lexical `usages` hits so source-code paths come before everything /// else (docs, configs, markdown). Stable: score order is preserved within diff --git a/src/mcp/status.rs b/src/mcp/status.rs index 18e2efca..29dba823 100644 --- a/src/mcp/status.rs +++ b/src/mcp/status.rs @@ -16,6 +16,102 @@ const LOCK_STATUS_HELD_IN_PROCESS: &str = "held-in-process"; const LOCK_STATUS_LOCKED_EXTERNALLY: &str = "locked-externally"; const LOCK_STATUS_AVAILABLE: &str = "available"; +/// One full projects-measure pass for the background status-snapshot task +/// — the ONLY place these per-repo probes (metadata reads, try_read stats, +/// lock detection) run. `list_projects` answers from the stored result and +/// must never call this on the request path. +pub(crate) fn compute_projects_snapshot( + serve_state: &crate::serve::ServeState, +) -> (Vec, Vec) { + let config = serve_state.config_snapshot(); + let project_groups = config.project_groups(); + let mut repos_info = Vec::new(); + let mut list_warnings: Vec = Vec::new(); + + for (alias, path) in &config.repos { + let db_path = path.join(crate::constants::DB_DIR_NAME); + + let (total_chunks, total_files, model, lock_status, error) = if db_path.exists() { + let (model_name, _dims) = read_model_metadata(&db_path); + + // For repos already opened in DashMap, use the live SharedStores for stats + // WITHOUT opening a new VectorStore connection. + // For unopened repos, just report metadata — do NOT open the DB. + let opened = serve_state.get_opened_stores(alias); + // Never wait on the store lock: an indexing run holds it for + // whole batches, and status must answer immediately. + let stats_result = opened + .as_ref() + .and_then(|stores| stores.vector_store.try_read().ok().map(|vs| vs.stats())); + if let Some(stats_result) = stats_result { + // `0 chunks` alone reads exactly like "not indexed yet" — the + // repo may in fact be full and simply failing to answer (the + // read-only-incident shape this branch exists for). Attribute + // the failure to THIS repo rather than a top-level channel: + // list_projects returns one entry per repo, so per-item is the + // shape that actually matches the fan-out. + // `repo_stats_from_result` carries only the part of this + // decision that varies by Ok/Err — see its doc comment. + // `record_stats_or_warn` wraps it so this call site cannot + // silently drop the warning half without also breaking the + // counts it returns — see its own doc comment. + let (total_chunks, total_files, error) = + record_stats_or_warn(stats_result, alias, &mut list_warnings); + ( + total_chunks, + total_files, + model_name, + serve_state + .repo_lock_status(alias) + .unwrap_or("unknown") + .to_string(), + error, + ) + } else { + // Not opened, or its store is busy — persisted stats from + // metadata.json; do NOT open the DB. Our own holders are + // checked before the file-lock probe, which cannot tell + // this process apart from another one. + let (md_chunks, md_files) = read_metadata_stats(&db_path); + let lock_status = if serve_state.is_indexing(alias) { + LOCK_STATUS_INDEXING + } else if opened.is_some() { + LOCK_STATUS_BUSY + } else if serve_state.is_held_in_process(alias, &db_path) { + LOCK_STATUS_HELD_IN_PROCESS + } else if crate::index::is_database_locked(&db_path) { + LOCK_STATUS_LOCKED_EXTERNALLY + } else { + LOCK_STATUS_AVAILABLE + }; + ( + md_chunks, + md_files, + model_name, + lock_status.to_string(), + None, + ) + } + } else { + (0, 0, "not indexed".to_string(), "unknown".to_string(), None) + }; + + repos_info.push(RepoInfo { + alias: alias.clone(), + project_path: path.display().to_string(), + database_path: db_path.display().to_string(), + total_chunks, + total_files, + model, + lock_status, + groups: project_groups.get(alias).cloned().unwrap_or_default(), + error, + }); + } + + (repos_info, list_warnings) +} + #[tool_router(router = status_router, vis = "pub(crate)")] impl CodesearchService { /// Unified status tool — dispatches based on `kind`. @@ -78,7 +174,32 @@ impl CodesearchService { // Count the virtual "all" group when repos are registered, so the // summary doesn't read "0 group(s)" while `all` is actually available. let group_count = config.groups.len() + if config.repos.is_empty() { 0 } else { 1 }; - let statuses = serve_state.repo_statuses_lightweight(); + // From the background snapshot, never an inline measure: under a + // rebuild wave the snapshot task is the ONLY thing allowed to + // touch the filesystem (the GET /status timeout fix) — and this + // tool must always answer too. Pre-first-pass answers "starting". + let Some(snap) = serve_state.latest_status_snapshot() else { + let response = IndexStatusResponse { + indexed: repo_count > 0, + status: "starting".to_string(), + status_message: + "serve is computing its first status snapshot; retry in a few seconds" + .to_string(), + total_chunks: 0, + total_files: 0, + model: self.model_type.short_name().to_string(), + dimensions: 0, + max_chunk_id: 0, + db_path: format!("({} repos)", repo_count), + project_path: format!("serve mode — {} repo(s)", repo_count), + error_message: None, + mode: self.mcp_mode(), + }; + let json = + serde_json::to_string(&response).unwrap_or_else(|_| "{}".to_string()); + return Ok(CallToolResult::success(vec![ContentBlock::text(json)])); + }; + let statuses = &snap.repos; let open_count = statuses .iter() .filter(|(_, r)| matches!(r.status, crate::serve::RepoStateLabel::Open)) @@ -351,90 +472,23 @@ impl CodesearchService { // When serve is active, use ServeState as source of truth for lock status if let Some(ref serve_state) = self.serve_state { let config = serve_state.config_snapshot(); - let project_groups = config.project_groups(); - let mut repos_info = Vec::new(); - let mut list_warnings: Vec = Vec::new(); - - for (alias, path) in &config.repos { - let db_path = path.join(crate::constants::DB_DIR_NAME); - - let (total_chunks, total_files, model, lock_status, error) = if db_path.exists() { - let (model_name, _dims) = read_model_metadata(&db_path); - - // For repos already opened in DashMap, use the live SharedStores for stats - // WITHOUT opening a new VectorStore connection. - // For unopened repos, just report metadata — do NOT open the DB. - let opened = serve_state.get_opened_stores(alias); - // Never wait on the store lock: an indexing run holds it for - // whole batches, and status must answer immediately. - let stats_result = opened.as_ref().and_then(|stores| { - stores.vector_store.try_read().ok().map(|vs| vs.stats()) - }); - if let Some(stats_result) = stats_result { - // `0 chunks` alone reads exactly like "not indexed yet" — the - // repo may in fact be full and simply failing to answer (the - // read-only-incident shape this branch exists for). Attribute - // the failure to THIS repo rather than a top-level channel: - // list_projects returns one entry per repo, so per-item is the - // shape that actually matches the fan-out. - // `repo_stats_from_result` carries only the part of this - // decision that varies by Ok/Err — see its doc comment. - // `record_stats_or_warn` wraps it so this call site cannot - // silently drop the warning half without also breaking the - // counts it returns — see its own doc comment. - let (total_chunks, total_files, error) = - record_stats_or_warn(stats_result, alias, &mut list_warnings); - ( - total_chunks, - total_files, - model_name, - serve_state - .repo_lock_status(alias) - .unwrap_or("unknown") - .to_string(), - error, - ) - } else { - // Not opened, or its store is busy — persisted stats from - // metadata.json; do NOT open the DB. Our own holders are - // checked before the file-lock probe, which cannot tell - // this process apart from another one. - let (md_chunks, md_files) = read_metadata_stats(&db_path); - let lock_status = if serve_state.is_indexing(alias) { - LOCK_STATUS_INDEXING - } else if opened.is_some() { - LOCK_STATUS_BUSY - } else if serve_state.is_held_in_process(alias, &db_path) { - LOCK_STATUS_HELD_IN_PROCESS - } else if crate::index::is_database_locked(&db_path) { - LOCK_STATUS_LOCKED_EXTERNALLY - } else { - LOCK_STATUS_AVAILABLE - }; - ( - md_chunks, - md_files, - model_name, - lock_status.to_string(), - None, - ) - } - } else { - (0, 0, "not indexed".to_string(), "unknown".to_string(), None) - }; - - repos_info.push(RepoInfo { - alias: alias.clone(), - project_path: path.display().to_string(), - database_path: db_path.display().to_string(), - total_chunks, - total_files, - model, - lock_status, - groups: project_groups.get(alias).cloned().unwrap_or_default(), - error, - }); - } + // From the background snapshot, never an inline measure — the same + // contract as /status and status(kind=index): under a rebuild wave + // the measuring task is the only thing that touches the filesystem, + // and the project list must still answer (it was the second timeout + // in the 2026-10-16 field case). Pre-first-pass answers an empty + // list with an explicit warming warning instead of hanging. + let (repos_info, list_warnings) = match serve_state.latest_status_snapshot() { + Some(snap) => (snap.projects.clone(), snap.projects_warnings.clone()), + None => ( + Vec::new(), + vec![ + "status snapshot still warming up — the project list is empty for a few \ + seconds after serve startup; retry shortly" + .to_string(), + ], + ), + }; let response = ListProjectsResponse { repos: repos_info, diff --git a/src/mcp/types.rs b/src/mcp/types.rs index e113eff8..67df7f68 100644 --- a/src/mcp/types.rs +++ b/src/mcp/types.rs @@ -526,7 +526,9 @@ pub struct RemoteProjectInfo { } /// Information about a single registered project/repo. -#[derive(Debug, Serialize)] +/// `Clone`: the snapshot stores the computed list and every projects answer +/// clones it out (the slot lock is never held across response building). +#[derive(Debug, Clone, Serialize)] pub struct RepoInfo { pub alias: String, pub project_path: String, diff --git a/src/serve/mod.rs b/src/serve/mod.rs index 65d77c26..d62aeb19 100644 --- a/src/serve/mod.rs +++ b/src/serve/mod.rs @@ -10,6 +10,7 @@ //! Holds a `DashMap>` keyed by repo alias. //! Lazy-opens stores on first query. Conflicted repos are isolated. +mod status_snapshot; mod tui; mod tui_common; mod tui_remote; @@ -464,6 +465,12 @@ pub(crate) struct ServeState { reload_count: std::sync::atomic::AtomicUsize, /// Instant when ServeState was created — used to compute uptime for TUI header. started_at: std::time::Instant, + /// Precomputed status snapshot the read endpoints answer from: a + /// background task measures (filesystem probes, sysinfo), handlers + /// never do. Under a heavy rebuild wave the measuring workers are + /// OS-starved; inline measurement made /status itself take tens of + /// seconds across the fleet (the "status must always answer" fix). + status_snapshot: Arc, } impl std::fmt::Debug for ServeState { @@ -548,6 +555,7 @@ impl ServeState { #[cfg(test)] reload_count: std::sync::atomic::AtomicUsize::new(0), started_at: std::time::Instant::now(), + status_snapshot: Arc::new(status_snapshot::StatusSnapshotHandle::new()), } } @@ -706,6 +714,13 @@ impl ServeState { .is_none() } + /// The latest precomputed status snapshot, or `None` before the first + /// background pass completes. Read endpoints answer from this — never + /// by measuring inline (the /status timeout fix). + pub(crate) fn latest_status_snapshot(&self) -> Option> { + self.status_snapshot.get() + } + /// Return the instant when serve started, used to compute uptime. pub(crate) fn started_at(&self) -> std::time::Instant { self.started_at @@ -3807,6 +3822,54 @@ impl ServeState { self.end_indexing(alias, IndexingOwner::Reindex); } + /// Compute one full status pass for the background snapshot task — + /// the ONLY place that measures (filesystem probes, sysinfo refresh). + /// Runs on the blocking pool via [`status_snapshot::spawn`]; the + /// read endpoints answer from the stored result and must never call + /// this on the request path. + pub(crate) fn compute_status_snapshot(&self) -> status_snapshot::StatusSnapshot { + let repos = self.repo_statuses_lightweight(); + let active_sessions = self.active_session_count(); + + // CPU usage — reuse shared System instance so cpu_usage() can compute delta + let cpu = (|| -> Option { + use sysinfo::ProcessesToUpdate; + let pid = sysinfo::get_current_pid().ok()?; + let mut sys = self.sysinfo_system.lock().ok()?; + sys.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); + let proc = sys.process(pid)?; + let num_cpus = sys.cpus().len().max(1) as f32; + Some(format!("{:.0}%", proc.cpu_usage() / num_cpus)) + })() + .unwrap_or_else(|| "—".to_string()); + + let csharp_helper = self + .symbol_registry + .get(LANG_CSHARP) + .map(|i| i.is_available()) + .unwrap_or(false); + + let ts_helper = self + .symbol_registry + .get(LANG_TYPESCRIPT) + .map(|i| i.is_available()) + .unwrap_or(false); + + let (snapshot_projects, snapshot_projects_warnings) = + crate::mcp::compute_projects_snapshot(self); + + status_snapshot::StatusSnapshot { + repos, + projects: snapshot_projects, + projects_warnings: snapshot_projects_warnings, + active_sessions, + cpu_percent: cpu, + csharp_helper, + ts_helper, + generated_at: std::time::Instant::now(), + } + } + /// Get lightweight repo statuses WITHOUT opening any databases. /// Returns a list of (alias, status_info) where status is derived from DashMap state only. pub(crate) fn repo_statuses_lightweight(&self) -> Vec<(String, RepoStatusInfo)> { @@ -4356,15 +4419,33 @@ async fn indexing_handler( /// Status handler: GET /status /// -/// Returns a JSON snapshot of all repo states, active sessions, and CPU usage. -/// Used by the standalone TUI (`codesearch serve tui`) to poll server state. +/// Answers from the precomputed [`status_snapshot::StatusSnapshot`] — the +/// measuring runs on a background task, so this handler is O(1) no matter +/// how loaded the machine is (the old inline version took tens of seconds +/// across the fleet during a rebuild wave; clients timed out). First ticks +/// after startup get an immediate minimal answer with `"status": "starting"`. +/// Every answer carries `snapshot_age_secs`; the `load` object counts the +/// busy repos so calling agents can throttle themselves. async fn status_handler( axum::extract::State(state): axum::extract::State>, ) -> AxumJson { - let repos = state.repo_statuses_lightweight(); - let active_sessions = state.active_session_count(); + let uptime_secs = state.started_at().elapsed().as_secs(); + let version = env!("CARGO_PKG_VERSION"); + + let Some(snap) = state.status_snapshot.get() else { + return AxumJson(json!({ + "version": version, + "status": "starting", + "repos": [], + "active_sessions": 0, + "cpu_percent": "—", + "snapshot_age_secs": serde_json::Value::Null, + "uptime_secs": uptime_secs, + })); + }; - let repo_json: Vec = repos + let repo_json: Vec = snap + .repos .iter() .map(|(alias, info)| { let status_str = match info.status { @@ -4409,73 +4490,44 @@ async fn status_handler( }) .collect(); - let uptime_secs = state.started_at().elapsed().as_secs(); - - // Serve-wide default model for newly created indexes (`serve --model`). - // `null` means the built-in default. - let default_model = state.default_model().map(|m| m.short_name()); - - // CPU usage — reuse shared System instance so cpu_usage() can compute delta - let cpu = { - use sysinfo::ProcessesToUpdate; - let pid = match sysinfo::get_current_pid() { - Ok(p) => p, - Err(_) => { - return AxumJson(json!({ - "version": env!("CARGO_PKG_VERSION"), - "repos": repo_json, - "active_sessions": active_sessions, - "default_model": default_model, - "cpu_percent": "—", - "uptime_secs": uptime_secs, - })); - } - }; - let mut sys = match state.sysinfo_system.lock() { - Ok(s) => s, - Err(_) => { - return AxumJson(json!({ - "version": env!("CARGO_PKG_VERSION"), - "repos": repo_json, - "active_sessions": active_sessions, - "default_model": default_model, - "cpu_percent": "—", - "uptime_secs": uptime_secs, - })); - } - }; - sys.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); - match sys.process(pid) { - Some(proc) => { - let num_cpus = sys.cpus().len().max(1) as f32; - let pct = proc.cpu_usage() / num_cpus; - format!("{:.0}%", pct) - } - None => "—".to_string(), - } - }; - - let csharp_helper = state - .symbol_registry - .get(LANG_CSHARP) - .map(|i| i.is_available()) - .unwrap_or(false); - - let ts_helper = state - .symbol_registry - .get(LANG_TYPESCRIPT) - .map(|i| i.is_available()) - .unwrap_or(false); + // Load summary for calling agents: how busy is this serve right now. + // Symbol columns count separately from the text label — a pure SCIP + // wave never lights "indexing" at the repo level (v1.5.31 semantics). + let text_indexing = snap + .repos + .iter() + .filter(|(_, i)| i.status == RepoStateLabel::Indexing) + .count(); + let symbol_indexing = snap + .repos + .iter() + .filter(|(_, i)| { + matches!(i.csharp_index, CSharpIndexStatus::Indexing) + || matches!(i.typescript_index, CSharpIndexStatus::Indexing) + }) + .count(); + let errored = snap + .repos + .iter() + .filter(|(_, i)| i.status == RepoStateLabel::Error) + .count(); AxumJson(json!({ - "version": env!("CARGO_PKG_VERSION"), + "version": version, "repos": repo_json, - "active_sessions": active_sessions, - "default_model": default_model, - "cpu_percent": cpu, - "csharp_helper": csharp_helper, - "ts_helper": ts_helper, + "active_sessions": snap.active_sessions, + "default_model": state.default_model().map(|m| m.short_name()), + "cpu_percent": snap.cpu_percent, + "csharp_helper": snap.csharp_helper, + "ts_helper": snap.ts_helper, "uptime_secs": uptime_secs, + "snapshot_age_secs": snap.age_secs(), + "load": { + "text_indexing": text_indexing, + "symbol_indexing": symbol_indexing, + "errored": errored, + "refresh_interval_secs": status_snapshot::refresh_interval().as_secs(), + }, })) } @@ -6592,6 +6644,12 @@ pub async fn run_serve( // — it never polls a federated peer on a timer at all. let serve_state = Arc::new(ServeState::new(config, None).with_default_model(default_model)); + // The status snapshot refresher: only this task measures. Read + // endpoints (GET /status, MCP status) answer from its stored result, + // so they respond in microseconds even while a rebuild wave starves + // the machine — the failure mode that made /status itself time out. + status_snapshot::spawn(serve_state.clone()); + // One line that answers "why is it slower / less parallel than before?" // without grepping the code: the effective heavy-work budget. info!( diff --git a/src/serve/status_snapshot.rs b/src/serve/status_snapshot.rs new file mode 100644 index 00000000..99ff02b0 --- /dev/null +++ b/src/serve/status_snapshot.rs @@ -0,0 +1,112 @@ +//! Always-answering status snapshot for serve mode. +//! +//! GET /status and the MCP `status` tool used to run their probes inline: +//! per registered repo several filesystem stats, a SCIP LMDB env open on +//! cache miss, and a sysinfo refresh — all synchronous on async workers. +//! Under a heavy rebuild wave those workers are OS-starved by the dotnet +//! and scip-csharp children, and one "lightweight" call took tens of +//! seconds across 28 repos; every polling client piled on and timed out. +//! +//! The fix is architectural: only a background task measures; handlers +//! read a precomputed [`StatusSnapshot`] out of a lock-held-only-to-swap +//! slot and answer in microseconds, no matter how loaded the machine is. +//! `snapshot_age_secs` in every answer tells the caller exactly how stale +//! what it is reading is — degraded freshness is visible, never a hang. + +use std::sync::{Arc, RwLock}; +use std::time::Duration; + +use super::{RepoStatusInfo, ServeState}; + +/// One computed status pass. Cheap to clone (handlers get an `Arc`). +pub(crate) struct StatusSnapshot { + pub(crate) repos: Vec<(String, RepoStatusInfo)>, + /// Precomputed per-repo project-list payload (metadata reads, store + /// stats via try_read, lock probes) — list_projects answers from this + /// for the same reason /status does: the request path never measures. + pub(crate) projects: Vec, + /// Warnings captured while computing `projects` (per-repo stats + /// failures). Re-emitted verbatim on each projects answer so a repo + /// that failed to report stays visible. + pub(crate) projects_warnings: Vec, + pub(crate) active_sessions: u64, + pub(crate) cpu_percent: String, + pub(crate) csharp_helper: bool, + pub(crate) ts_helper: bool, + pub(crate) generated_at: std::time::Instant, +} + +impl StatusSnapshot { + /// Age of this snapshot in whole seconds. + pub(crate) fn age_secs(&self) -> u64 { + self.generated_at.elapsed().as_secs() + } +} + +/// The slot handlers read from. The inner RwLock is only ever held to swap +/// an `Arc` — never across computation — so reader latency is bound by the +/// swap itself, not by how long the machine takes to measure. +#[derive(Default)] +pub(crate) struct StatusSnapshotHandle { + slot: RwLock>>, +} + +impl StatusSnapshotHandle { + pub(crate) fn new() -> Self { + Self::default() + } + + /// The latest snapshot, or `None` before the first background pass + /// completes. Handlers must answer something immediately in that case, + /// never fall back to measuring inline. + pub(crate) fn get(&self) -> Option> { + self.slot.read().ok().and_then(|s| s.clone()) + } + + pub(crate) fn store(&self, snapshot: StatusSnapshot) { + if let Ok(mut slot) = self.slot.write() { + *slot = Some(Arc::new(snapshot)); + } + } +} + +/// Refresh cadence: `CODESEARCH_STATUS_REFRESH_SECS` when a positive +/// integer, otherwise 2s. Unparseable values fall back to the default. +pub(crate) fn refresh_interval() -> Duration { + match std::env::var(crate::constants::STATUS_REFRESH_SECS_ENV) + .ok() + .and_then(|v| v.parse::().ok()) + { + Some(secs) if secs > 0 => Duration::from_secs(secs), + _ => Duration::from_secs(2), + } +} + +/// Spawn the background refresher. One pass runs at a time: the sleep +/// starts only after the previous pass completed, so under heavy load the +/// cadence stretches (visible as a growing `snapshot_age_secs`) instead of +/// piling measuring tasks onto a starved machine. The computation runs on +/// the blocking pool — it is filesystem work, not async work. +pub(crate) fn spawn(state: Arc) { + let handle = state.status_snapshot.clone(); + let interval = refresh_interval(); + tokio::spawn(async move { + loop { + let snap = tokio::task::spawn_blocking({ + let state = state.clone(); + move || state.compute_status_snapshot() + }) + .await; + match snap { + Ok(snapshot) => handle.store(snapshot), + Err(err) => { + // Join failure means the runtime is shutting down; the + // next loop iteration would also fail, but sleeping + // keeps shutdown semantics simple. + tracing::debug!("status snapshot pass failed: {err}"); + } + } + tokio::time::sleep(interval).await; + } + }); +} diff --git a/src/serve/tests.rs b/src/serve/tests.rs index 44e9897e..c1db245c 100644 --- a/src/serve/tests.rs +++ b/src/serve/tests.rs @@ -5010,3 +5010,144 @@ async fn sweeper_reasserts_cancellation_for_a_removed_live_task() { } } } + +// ── status snapshot ────────────────────────────────────────────────────── + +#[test] +fn status_snapshot_handle_starts_empty_and_roundtrips() { + let handle = status_snapshot::StatusSnapshotHandle::new(); + // Before the first background pass handlers must see None and answer + // their "starting" shape — never measure inline. + assert!(handle.get().is_none()); + + let snap = status_snapshot::StatusSnapshot { + repos: Vec::new(), + projects: Vec::new(), + projects_warnings: Vec::new(), + active_sessions: 0, + cpu_percent: "0%".to_string(), + csharp_helper: false, + ts_helper: false, + generated_at: std::time::Instant::now(), + }; + handle.store(snap); + + let got = handle.get().expect("snapshot readable after store"); + assert_eq!(got.cpu_percent, "0%"); + assert!(got.age_secs() <= 1, "fresh snapshot must have ~0 age"); +} + +#[test] +fn compute_status_snapshot_reports_empty_fleet_without_state() { + let state = ServeState::new(ReposConfig::default(), None); + assert!(state.latest_status_snapshot().is_none()); + + let snap = state.compute_status_snapshot(); + assert!(snap.repos.is_empty()); + assert_eq!(snap.active_sessions, 0); + assert!(!snap.cpu_percent.is_empty(), "cpu is always a string"); + assert!(snap.age_secs() <= 1); +} + +#[test] +#[serial] +fn status_refresh_interval_env_overrides() { + { + let _env = + crate::testing::EnvRestore::set(&[(crate::constants::STATUS_REFRESH_SECS_ENV, "7")]); + assert_eq!(status_snapshot::refresh_interval(), Duration::from_secs(7)); + } + { + // Unparseable falls back to the default rather than failing. + let _env = + crate::testing::EnvRestore::set(&[(crate::constants::STATUS_REFRESH_SECS_ENV, "abc")]); + assert_eq!(status_snapshot::refresh_interval(), Duration::from_secs(2)); + } + { + // Zero would busy-loop the measuring task; the default floor applies. + let _env = + crate::testing::EnvRestore::set(&[(crate::constants::STATUS_REFRESH_SECS_ENV, "0")]); + assert_eq!(status_snapshot::refresh_interval(), Duration::from_secs(2)); + } + { + let _env = + crate::testing::EnvRestore::set(&[(crate::constants::STATUS_REFRESH_SECS_ENV, "")]); + assert_eq!(status_snapshot::refresh_interval(), Duration::from_secs(2)); + } +} + +#[tokio::test] +async fn status_handler_answers_starting_then_snapshot_with_load() { + let state = Arc::new(ServeState::new(ReposConfig::default(), None)); + + // Before the first background pass: immediate "starting" answer, no + // measurement, null age. + let res = status_handler(axum::extract::State(state.clone())).await; + assert_eq!(res.0["status"], "starting"); + assert!(res.0["snapshot_age_secs"].is_null()); + assert!(res.0["repos"].is_array()); + + // After a pass is stored: the full contract callers depend on — legacy + // fields preserved, load counters present, age numeric. + state.status_snapshot.store(state.compute_status_snapshot()); + let res = status_handler(axum::extract::State(state)).await; + let v = res.0; + assert!( + v["status"].is_null(), + "stored pass must not read 'starting'" + ); + assert_eq!(v["load"]["text_indexing"], 0); + assert_eq!(v["load"]["symbol_indexing"], 0); + assert_eq!(v["load"]["errored"], 0); + assert!(v["load"]["refresh_interval_secs"].is_u64()); + assert!(v["snapshot_age_secs"].is_u64()); + assert!(v["version"].is_string()); + assert!(v["uptime_secs"].is_u64()); +} + +#[tokio::test] +async fn mcp_status_and_projects_answer_starting_pre_first_pass() { + let state = Arc::new(ServeState::new(ReposConfig::default(), None)); + let service = crate::mcp::CodesearchService::new_for_serve(state).unwrap(); + + // kind=index, no snapshot yet: structured "starting" answer. + let res = service + .status(rmcp::handler::server::wrapper::Parameters( + crate::mcp::types::StatusRequest { + kind: Some("index".to_string()), + project: None, + group: None, + }, + )) + .await + .expect("status must not error"); + let text = match res.content.first() { + Some(rmcp::model::ContentBlock::Text(t)) => t.text.clone(), + other => panic!("expected text content, got {other:?}"), + }; + assert!( + text.contains("\"starting\""), + "scope-free index status must answer starting pre-first-pass, got: {text}" + ); + + // kind=projects, no snapshot yet: empty list + explicit warming warning + // (the warnings channel terminates — never a silent empty answer). + let res = service + .status(rmcp::handler::server::wrapper::Parameters( + crate::mcp::types::StatusRequest { + kind: Some("projects".to_string()), + project: None, + group: None, + }, + )) + .await + .expect("projects must not error"); + let text = match res.content.first() { + Some(rmcp::model::ContentBlock::Text(t)) => t.text.clone(), + other => panic!("expected text content, got {other:?}"), + }; + assert!( + text.contains("warming up"), + "projects pre-first-pass must carry the warming warning, got: {text}" + ); +}