From 9a3e2f8ad2a64c6268e55b2f5838b0d9cbfb5fbf Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Mon, 29 Jun 2026 18:03:02 +0000 Subject: [PATCH 1/3] Fix code diagnostics LSP startup states --- .../code-diagnostics/src/CodeDiagnostics.tsx | 1 + dashboard/code-diagnostics/src/styles.css | 3 +- dashboard/code-diagnostics/src/types.ts | 1 + src/dashboard/code_diagnostics_api.rs | 150 +++++++++++++++++- src/diagnostics/lsp/broker.rs | 70 +++++++- src/diagnostics/lsp/client.rs | 57 ++++++- tests/dashboard_code_diagnostics_api_test.rs | 5 + tests/lsp_code_diagnostics_test.rs | 43 +++++ 8 files changed, 322 insertions(+), 8 deletions(-) diff --git a/dashboard/code-diagnostics/src/CodeDiagnostics.tsx b/dashboard/code-diagnostics/src/CodeDiagnostics.tsx index 31887832..7da40d00 100644 --- a/dashboard/code-diagnostics/src/CodeDiagnostics.tsx +++ b/dashboard/code-diagnostics/src/CodeDiagnostics.tsx @@ -14,6 +14,7 @@ import type { const STATE_LABELS: Record = { unavailable: "Unavailable", disabled: "Disabled", + inactive: "Inactive", starting: "Starting", indexing: "Indexing", ready: "Ready", diff --git a/dashboard/code-diagnostics/src/styles.css b/dashboard/code-diagnostics/src/styles.css index 766de1b9..8b8b2970 100644 --- a/dashboard/code-diagnostics/src/styles.css +++ b/dashboard/code-diagnostics/src/styles.css @@ -117,7 +117,8 @@ color: var(--color-destructive); } -.tdcd-state-disabled { +.tdcd-state-disabled, +.tdcd-state-inactive { color: var(--color-muted-foreground); } diff --git a/dashboard/code-diagnostics/src/types.ts b/dashboard/code-diagnostics/src/types.ts index 6863795a..a88c48ba 100644 --- a/dashboard/code-diagnostics/src/types.ts +++ b/dashboard/code-diagnostics/src/types.ts @@ -2,6 +2,7 @@ export type IdleBackfillMode = "off" | "idle"; export type EngineState = | "unavailable" | "disabled" + | "inactive" | "starting" | "indexing" | "ready" diff --git a/src/dashboard/code_diagnostics_api.rs b/src/dashboard/code_diagnostics_api.rs index b6fb67f0..0b672b82 100644 --- a/src/dashboard/code_diagnostics_api.rs +++ b/src/dashboard/code_diagnostics_api.rs @@ -12,6 +12,7 @@ use serde_json::{json, Value}; use super::util::{http_detail, JsonError}; use super::DashboardState; use crate::diagnostics::lsp::adapters::LspAdapterDefinition; +use crate::diagnostics::lsp::broker::EngineState; use crate::diagnostics::lsp::client::LspDocument; use crate::diagnostics::lsp::settings::{save_settings, IdleBackfillMode}; @@ -44,6 +45,7 @@ enum CommandOverridePatch { } pub(crate) async fn overview(State(state): State) -> ApiResult { + sync_project_language_activity(&state).await?; maybe_spawn_idle_backfill(&state).await; let snapshot = state.code_diagnostics.read().await.snapshot(); Ok(Json(json!(snapshot))) @@ -85,10 +87,14 @@ pub(crate) async fn patch_settings( let mut broker = state.code_diagnostics.write().await; broker.update_adapters(adapters); broker.update_settings(settings); + drop(broker); + sync_project_language_activity(&state).await?; + let broker = state.code_diagnostics.read().await; Ok(Json(json!(broker.snapshot()))) } pub(crate) async fn refresh_all(State(state): State) -> ApiResult { + sync_project_language_activity(&state).await?; let languages: Vec = state .code_diagnostics .read() @@ -96,6 +102,7 @@ pub(crate) async fn refresh_all(State(state): State) -> ApiResul .snapshot() .engines .into_iter() + .filter(|engine| engine.state != EngineState::Inactive) .map(|engine| engine.language) .collect(); for language in languages { @@ -109,6 +116,7 @@ pub(crate) async fn refresh_language( State(state): State, AxumPath(language): AxumPath, ) -> ApiResult { + sync_project_language_activity(&state).await?; refresh_one(&state, &language).await?; let snapshot = state.code_diagnostics.read().await.snapshot(); Ok(Json(json!(snapshot))) @@ -218,7 +226,7 @@ async fn maybe_spawn_idle_backfill(state: &DashboardState) { .snapshot() .engines .into_iter() - .filter(|engine| engine.enabled) + .filter(|engine| engine.enabled && engine.state != EngineState::Inactive) .map(|engine| engine.language) .collect(); for language in languages { @@ -228,6 +236,34 @@ async fn maybe_spawn_idle_backfill(state: &DashboardState) { }); } +async fn sync_project_language_activity( + state: &DashboardState, +) -> std::result::Result<(), JsonError> { + let files = indexed_files(&state.graph_conn) + .await + .map_err(|err| internal_error(&err))?; + let adapters = { + let broker = state.code_diagnostics.read().await; + broker + .snapshot() + .engines + .into_iter() + .filter_map(|engine| broker.adapter_for(&engine.language)) + .collect::>() + }; + let active_languages = adapters + .iter() + .filter(|adapter| adapter_has_project_documents(&state.project_root, adapter, &files)) + .map(|adapter| adapter.language.clone()) + .collect::>(); + state + .code_diagnostics + .write() + .await + .update_project_languages(active_languages); + Ok(()) +} + async fn indexed_files(conn: &libsql::Connection) -> crate::errors::Result> { let mut rows = conn .query("SELECT path FROM files ORDER BY path ASC", ()) @@ -251,6 +287,9 @@ async fn documents_for_adapter( if !matches_adapter_extension(adapter, &file) { continue; } + if !file_has_adapter_root_marker(project_root, adapter, &file) { + continue; + } let path = project_root.join(&file); let Ok(text) = tokio::fs::read_to_string(&path).await else { continue; @@ -265,6 +304,43 @@ async fn documents_for_adapter( Ok(documents) } +fn adapter_has_project_documents( + project_root: &Path, + adapter: &LspAdapterDefinition, + files: &[String], +) -> bool { + files.iter().any(|file| { + matches_adapter_extension(adapter, file) + && file_has_adapter_root_marker(project_root, adapter, file) + }) +} + +fn file_has_adapter_root_marker( + project_root: &Path, + adapter: &LspAdapterDefinition, + file: &str, +) -> bool { + if adapter.root_markers.is_empty() { + return true; + } + let path = project_root.join(file); + let mut current = path.parent(); + while let Some(dir) = current { + if adapter + .root_markers + .iter() + .any(|marker| dir.join(marker).is_file()) + { + return true; + } + if dir == project_root { + break; + } + current = dir.parent(); + } + false +} + fn language_id_for_file(adapter: &LspAdapterDefinition, file: &str) -> String { let extension = Path::new(file) .extension() @@ -316,3 +392,75 @@ fn internal_error(err: &impl ToString) -> JsonError { Json(http_detail(&err.to_string())), ) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::diagnostics::lsp::adapters::{DiagnosticMode, LspAdapterDefinition}; + + #[tokio::test] + async fn documents_for_adapter_requires_a_matching_root_marker() { + let temp = tempfile::tempdir().expect("tempdir"); + let project_root = temp.path(); + let source_path = project_root.join("src/lib.fake"); + tokio::fs::create_dir_all(source_path.parent().expect("source parent")) + .await + .expect("create source dir"); + tokio::fs::write(&source_path, "fake source") + .await + .expect("write source"); + let adapter = fake_adapter("fake-root"); + + let documents = + documents_for_adapter(project_root, &adapter, vec!["src/lib.fake".to_string()]) + .await + .expect("documents"); + + assert!( + documents.is_empty(), + "adapter without a root marker should not open project documents" + ); + } + + #[tokio::test] + async fn documents_for_adapter_accepts_files_under_a_matching_root_marker() { + let temp = tempfile::tempdir().expect("tempdir"); + let project_root = temp.path(); + let package_root = project_root.join("package"); + let source_path = package_root.join("src/lib.fake"); + tokio::fs::create_dir_all(source_path.parent().expect("source parent")) + .await + .expect("create source dir"); + tokio::fs::write(package_root.join("fake-root"), "") + .await + .expect("write marker"); + tokio::fs::write(&source_path, "fake source") + .await + .expect("write source"); + let adapter = fake_adapter("fake-root"); + + let documents = documents_for_adapter( + project_root, + &adapter, + vec!["package/src/lib.fake".to_string()], + ) + .await + .expect("documents"); + + assert_eq!(documents.len(), 1); + assert_eq!(documents[0].relative_path, "package/src/lib.fake"); + } + + fn fake_adapter(root_marker: &str) -> LspAdapterDefinition { + LspAdapterDefinition { + language: "fake".to_string(), + language_id: "fake".to_string(), + command: "fake-ls".to_string(), + args: Vec::new(), + extensions: vec!["fake".to_string()], + root_markers: vec![root_marker.to_string()], + install_options: Vec::new(), + diagnostics: DiagnosticMode::Push, + } + } +} diff --git a/src/diagnostics/lsp/broker.rs b/src/diagnostics/lsp/broker.rs index dae08946..c7c212cd 100644 --- a/src/diagnostics/lsp/broker.rs +++ b/src/diagnostics/lsp/broker.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -42,6 +42,7 @@ pub enum DiagnosticSeverity { pub enum EngineState { Unavailable, Disabled, + Inactive, Starting, Indexing, Ready, @@ -176,6 +177,7 @@ pub struct DiagnosticBroker { clients: BTreeMap>>>, engine_overrides: BTreeMap, engine_errors: BTreeMap, + project_languages: BTreeSet, backfill: BTreeMap, } @@ -185,6 +187,10 @@ impl DiagnosticBroker { adapters: Vec, settings: CodeDiagnosticsSettings, ) -> Self { + let project_languages = adapters + .iter() + .map(|adapter| adapter.language.clone()) + .collect(); Self { project_root: project_root.into(), adapters, @@ -193,6 +199,7 @@ impl DiagnosticBroker { clients: BTreeMap::new(), engine_overrides: BTreeMap::new(), engine_errors: BTreeMap::new(), + project_languages, backfill: BTreeMap::new(), } } @@ -226,6 +233,30 @@ impl DiagnosticBroker { self.clients.clear(); } + pub fn update_project_languages(&mut self, languages: BTreeSet) { + self.project_languages = languages; + let active_languages: Vec = self.project_languages.iter().cloned().collect(); + for language in active_languages { + if self.engine_overrides.get(&language) == Some(&EngineState::Inactive) { + self.engine_overrides.remove(&language); + } + } + let inactive_languages: Vec = self + .adapters + .iter() + .filter(|adapter| !self.project_languages.contains(&adapter.language)) + .map(|adapter| adapter.language.clone()) + .collect(); + for language in inactive_languages { + self.remove_language_clients(&language); + self.clear_language(&language); + self.engine_errors.remove(&language); + if self.engine_overrides.get(&language) != Some(&EngineState::Disabled) { + self.engine_overrides.remove(&language); + } + } + } + pub fn set_language_enabled(&mut self, language: &str, enabled: bool) { self.settings.set_language_enabled(language, enabled); if enabled { @@ -250,6 +281,14 @@ impl DiagnosticBroker { self.clear_language(language); return Ok(None); } + if !self.project_languages.contains(language) { + self.engine_overrides + .insert(language.to_string(), EngineState::Inactive); + self.engine_errors.remove(language); + self.remove_language_clients(language); + self.clear_language(language); + return Ok(None); + } let adapter = self .adapters .iter() @@ -348,7 +387,7 @@ impl DiagnosticBroker { Err(message) => { self.engine_errors.insert(language.clone(), message.clone()); self.engine_overrides - .insert(language.clone(), EngineState::Crashed); + .insert(language.clone(), engine_state_for_error(&message)); self.remove_language_clients(&language); Err(TraceDecayError::Config { message }) } @@ -449,7 +488,13 @@ impl DiagnosticBroker { .engine_overrides .get(&adapter.language) .copied() - .unwrap_or_else(|| default_state(enabled, &command)); + .unwrap_or_else(|| { + default_state( + enabled, + self.project_languages.contains(&adapter.language), + &command, + ) + }); let last_diagnostic_update = self .diagnostics .iter() @@ -473,10 +518,13 @@ impl DiagnosticBroker { } } -fn default_state(enabled: bool, command: &str) -> EngineState { +fn default_state(enabled: bool, active: bool, command: &str) -> EngineState { if !enabled { return EngineState::Disabled; } + if !active { + return EngineState::Inactive; + } if command_available(command) { EngineState::Ready } else { @@ -484,6 +532,20 @@ fn default_state(enabled: bool, command: &str) -> EngineState { } } +fn engine_state_for_error(message: &str) -> EngineState { + let lower = message.to_ascii_lowercase(); + if lower.contains("not available on path") + || lower.contains("unknown binary") + || lower.contains("not installed") + || lower.contains("no such file or directory") + || lower.contains("could not execute process") + { + EngineState::Unavailable + } else { + EngineState::Crashed + } +} + fn workspace_root_for_document( project_root: &Path, adapter: &LspAdapterDefinition, diff --git a/src/diagnostics/lsp/client.rs b/src/diagnostics/lsp/client.rs index 8ed0ef02..dd269a2e 100644 --- a/src/diagnostics/lsp/client.rs +++ b/src/diagnostics/lsp/client.rs @@ -2,11 +2,14 @@ use std::collections::BTreeMap; use std::fmt::Write as _; use std::path::{Path, PathBuf}; use std::process::Stdio; +use std::sync::Arc; use std::time::Duration; use serde::Deserialize; use serde_json::{json, Value}; use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; +use tokio::sync::Mutex; +use tokio::task::JoinHandle; use crate::diagnostics::lsp::broker::{CodeDiagnostic, DiagnosticSeverity}; use crate::errors::{Result, TraceDecayError}; @@ -38,6 +41,7 @@ pub struct StdioLspClient { stdin: tokio::process::ChildStdin, reader: BufReader, child: tokio::process::Child, + stderr_task: JoinHandle<()>, } impl StdioLspClient { @@ -47,7 +51,7 @@ impl StdioLspClient { .current_dir(project_root) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::null()) + .stderr(Stdio::piped()) .kill_on_drop(true) .spawn() .map_err(|e| TraceDecayError::Config { @@ -60,7 +64,12 @@ impl StdioLspClient { let stdout = child.stdout.take().ok_or_else(|| TraceDecayError::Config { message: format!("failed to open stdout for LSP server '{command}'"), })?; + let stderr = child.stderr.take().ok_or_else(|| TraceDecayError::Config { + message: format!("failed to open stderr for LSP server '{command}'"), + })?; let mut reader = BufReader::new(stdout); + let stderr_capture = Arc::new(Mutex::new(Vec::new())); + let stderr_task = spawn_stderr_capture(stderr, Arc::clone(&stderr_capture)); write_message( &mut stdin, @@ -87,7 +96,13 @@ impl StdioLspClient { }), ) .await?; - wait_for_initialize(&mut reader).await?; + if let Err(err) = wait_for_initialize(&mut reader).await { + let _ = child.start_kill(); + let _ = child.wait().await; + let stderr = captured_stderr(&stderr_capture).await; + stderr_task.abort(); + return Err(enrich_start_error(command, err, stderr)); + } write_message( &mut stdin, json!({ @@ -104,6 +119,7 @@ impl StdioLspClient { stdin, reader, child, + stderr_task, }) } @@ -199,6 +215,43 @@ impl StdioLspClient { impl Drop for StdioLspClient { fn drop(&mut self) { let _ = self.child.start_kill(); + self.stderr_task.abort(); + } +} + +fn spawn_stderr_capture( + mut stderr: tokio::process::ChildStderr, + capture: Arc>>, +) -> JoinHandle<()> { + tokio::spawn(async move { + let mut buffer = [0_u8; 1024]; + loop { + let Ok(bytes_read) = stderr.read(&mut buffer).await else { + break; + }; + if bytes_read == 0 { + break; + } + let mut captured = capture.lock().await; + let remaining = 8192_usize.saturating_sub(captured.len()); + if remaining > 0 { + captured.extend_from_slice(&buffer[..bytes_read.min(remaining)]); + } + } + }) +} + +async fn captured_stderr(capture: &Arc>>) -> String { + let captured = capture.lock().await; + String::from_utf8_lossy(&captured).trim().to_string() +} + +fn enrich_start_error(command: &str, err: TraceDecayError, stderr: String) -> TraceDecayError { + if stderr.is_empty() { + return err; + } + TraceDecayError::Config { + message: format!("{command} failed during initialize: {err}; stderr: {stderr}"), } } diff --git a/tests/dashboard_code_diagnostics_api_test.rs b/tests/dashboard_code_diagnostics_api_test.rs index 92e69ce2..5f025db3 100644 --- a/tests/dashboard_code_diagnostics_api_test.rs +++ b/tests/dashboard_code_diagnostics_api_test.rs @@ -24,6 +24,11 @@ fn code_diagnostics_dashboard_api_exposes_engines_and_applies_settings() { .any(|engine| engine["language"] == "rust"), "rust engine should be advertised" ); + assert_eq!( + engine(&initial, "rust")["state"], + "inactive", + "fixture has .rs files but no Cargo.toml, so rust-analyzer should not auto-start" + ); let (status, patched) = patch_json_body( &agent, diff --git a/tests/lsp_code_diagnostics_test.rs b/tests/lsp_code_diagnostics_test.rs index 42458da6..a40bf081 100644 --- a/tests/lsp_code_diagnostics_test.rs +++ b/tests/lsp_code_diagnostics_test.rs @@ -260,6 +260,49 @@ async fn broker_marks_missing_lsp_command_unavailable_after_refresh_failure() { .contains("not available on PATH")); } +#[tokio::test] +async fn broker_marks_install_proxy_exit_during_initialize_unavailable() { + let temp = tempfile::tempdir().unwrap(); + let script_path = temp.path().join("missing_component_lsp.py"); + std::fs::write( + &script_path, + r#" +import sys + +sys.stderr.write("error: unknown binary 'rust-analyzer' in toolchain 'test-toolchain'\n") +sys.stderr.flush() +"#, + ) + .unwrap(); + let mut broker = lsp::broker::DiagnosticBroker::new_for_test( + temp.path(), + vec![fake_python_adapter(FAKE_LANGUAGE, "fake", &script_path)], + ); + + let err = broker + .refresh_documents( + FAKE_LANGUAGE, + vec![fake_document(FAKE_LANGUAGE, FAKE_PATH, "let nope")], + std::time::Duration::from_millis(50), + ) + .await + .unwrap_err(); + + assert!(err.to_string().contains("unknown binary")); + let snapshot = broker.snapshot(); + let status = snapshot + .engines + .iter() + .find(|engine| engine.language == FAKE_LANGUAGE) + .expect("fake engine status should be listed"); + assert_eq!(status.state, lsp::broker::EngineState::Unavailable); + assert!(status + .last_error + .as_deref() + .unwrap_or_default() + .contains("unknown binary")); +} + #[tokio::test] async fn broker_reuses_warm_lsp_client_between_refreshes() { let temp = tempfile::tempdir().unwrap(); From c87d758f7f5b19c0422bb34e6e020904216e4ded Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Mon, 29 Jun 2026 18:05:27 +0000 Subject: [PATCH 2/3] Improve tracedecay follow-up diagnostics --- src/automation/fact_proposals.rs | 18 ++ src/doctor.rs | 214 ++++++++++++++++++ src/extraction/typescript_extractor.rs | 52 ++++- src/hooks/tool_hints.rs | 171 +++++++++++++- src/mcp/server.rs | 76 +++++++ src/mcp/tools/handlers/graph.rs | 89 +++++++- ...utomation_session_reflector_runner_test.rs | 63 ++++++ tests/mcp_handler_test.rs | 46 ++++ tests/mcp_server_test.rs | 75 ++++++ tests/typescript_extraction_test.rs | 32 +++ 10 files changed, 828 insertions(+), 8 deletions(-) diff --git a/src/automation/fact_proposals.rs b/src/automation/fact_proposals.rs index 6f17f3c4..ce6a3727 100644 --- a/src/automation/fact_proposals.rs +++ b/src/automation/fact_proposals.rs @@ -166,6 +166,9 @@ pub async fn record_session_fact_proposals( .ok_or_else(|| config_error("accepted fact proposal missing add_fact_request"))?; let add_fact_request = serde_json::from_value::(add_fact_request) .map_err(|e| config_error(format!("invalid accepted fact add_fact_request: {e}")))?; + if pending_add_fact_request_exists(&store, &add_fact_request) { + continue; + } let proposal = value.get("proposal").cloned(); let validation = value.get("validation").cloned(); let record = FactProposalRecord { @@ -214,6 +217,21 @@ pub async fn record_session_fact_proposals( Ok(records) } +fn pending_add_fact_request_exists(store: &FactProposalStore, request: &AddFactRequest) -> bool { + let content = normalize_fact_content(&request.content); + store.proposals.iter().any(|proposal| { + proposal.state == FactProposalState::PendingApproval + && proposal.add_fact_request.as_ref().is_some_and(|existing| { + existing.category == request.category + && normalize_fact_content(&existing.content) == content + }) + }) +} + +fn normalize_fact_content(content: &str) -> String { + content.split_whitespace().collect::>().join(" ") +} + pub async fn apply_fact_proposal( dashboard_root: &Path, conn: &Connection, diff --git a/src/doctor.rs b/src/doctor.rs index 2e866f0e..8cf22fae 100644 --- a/src/doctor.rs +++ b/src/doctor.rs @@ -179,6 +179,31 @@ async fn check_stale_stores(dc: &mut DoctorCounters) { check_orphan_store_manifests(dc, &project_paths); check_stale_code_projects(dc, &gdb).await; + if let Some(profile_root) = profile_root.as_deref() { + let drift = registry_drift_findings(&gdb, profile_root).await; + if drift.is_empty() { + dc.pass("No registry/store manifest identity drift"); + } else { + dc.warn(&format!( + "{} registry/store manifest identity drift finding(s):", + drift.len() + )); + for finding in drift.iter().take(10) { + dc.info(&format!( + " • {} {} {}: registry={} manifest={} ({})", + finding.project_id, + finding.store_id, + finding.field, + finding.registry_value, + finding.manifest_value, + finding.manifest_path.display() + )); + } + if drift.len() > 10 { + dc.info(&format!(" … and {} more", drift.len() - 10)); + } + } + } if stale.is_empty() { dc.pass("No stale projects in global DB"); return; @@ -287,6 +312,76 @@ fn code_project_root_exists(project: &crate::global_db::CodeProjectRecord) -> bo Path::new(&project.canonical_root).exists() || Path::new(&project.display_root).exists() } +#[derive(Debug, Clone, PartialEq, Eq)] +struct RegistryDriftFinding { + project_id: String, + store_id: String, + field: &'static str, + registry_value: String, + manifest_value: String, + manifest_path: PathBuf, +} + +async fn registry_drift_findings( + global_db: &crate::global_db::GlobalDb, + profile_root: &Path, +) -> Vec { + let mut findings = Vec::new(); + for project in global_db.list_code_projects(usize::MAX).await { + let Some(context) = global_db + .project_registry_context_by_id(&project.project_id) + .await + else { + continue; + }; + for store_context in context.stores { + let store = store_context.store; + let Some(manifest_path) = resolve_registry_manifest_path(profile_root, &store) else { + continue; + }; + let Ok(manifest) = crate::storage::read_store_manifest(&manifest_path) else { + continue; + }; + let manifest_project_id = manifest + .project_id + .as_deref() + .unwrap_or("") + .to_string(); + if manifest_project_id != store.project_id { + findings.push(RegistryDriftFinding { + project_id: project.project_id.clone(), + store_id: store.store_id.clone(), + field: "project_id", + registry_value: store.project_id.clone(), + manifest_value: manifest_project_id, + manifest_path: manifest_path.clone(), + }); + } + + let registry_project_root = comparable_path(Path::new(&project.canonical_root)); + let manifest_project_root = comparable_path(&manifest.project_root); + if registry_project_root != manifest_project_root { + findings.push(RegistryDriftFinding { + project_id: project.project_id.clone(), + store_id: store.store_id.clone(), + field: "project_root", + registry_value: registry_project_root, + manifest_value: manifest_project_root, + manifest_path, + }); + } + } + } + findings +} + +fn comparable_path(path: &Path) -> String { + path.canonicalize() + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .to_string() +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum DoctorStorageStatus { RepoLocal, @@ -386,6 +481,45 @@ fn classify_registry_storage( } } +fn resolve_registry_manifest_path( + profile_root: &Path, + store: &crate::global_db::StoreInstanceRecord, +) -> Option { + if store.storage_mode != "profile_sharded" { + return None; + } + let store_relpath = registry_relpath(&store.store_relpath); + let manifest_relpath = store + .manifest_relpath + .as_ref() + .map(|relpath| registry_relpath(relpath)); + for profile_root in registry_profile_roots(profile_root) { + let Ok(data_root) = + crate::storage::StoreArtifactPath::resolve(&profile_root, &store_relpath) + else { + continue; + }; + let data_root = data_root.absolute_path(); + if let Some(relpath) = manifest_relpath.as_ref() { + for root in [&profile_root, &data_root] { + let Ok(path) = crate::storage::StoreArtifactPath::resolve(root, relpath) else { + continue; + }; + let path = path.absolute_path(); + if path.is_file() { + return Some(path); + } + } + } else { + let path = data_root.join(crate::storage::STORE_MANIFEST_FILENAME); + if path.is_file() { + return Some(path); + } + } + } + None +} + fn registry_relpath(value: &str) -> PathBuf { let path = Path::new(value); if path.is_absolute() @@ -549,6 +683,10 @@ fn print_summary(dc: &DoctorCounters) { mod tests { use super::*; use crate::global_db::StoreInstanceUpsert; + use crate::storage::{ + StorageMode, StoreKind, StoreManifest, STORE_MANIFEST_FILENAME, + STORE_MANIFEST_SCHEMA_VERSION, + }; #[cfg(unix)] use std::os::unix::fs::symlink; @@ -729,4 +867,80 @@ mod tests { ); Ok(()) } + + #[tokio::test] + async fn registry_drift_findings_report_manifest_identity_mismatches( + ) -> std::result::Result<(), Box> { + let dir = tempfile::TempDir::new()?; + let profile_root = canonical_temp_path(&dir.path().join("profile")); + let registry_root = canonical_temp_path(&dir.path().join("registry-repo")); + let manifest_root = canonical_temp_path(&dir.path().join("manifest-repo")); + let shard_relpath = Path::new("projects").join("proj_registry"); + let shard_root = profile_root.join(&shard_relpath); + std::fs::create_dir_all(®istry_root)?; + std::fs::create_dir_all(&manifest_root)?; + std::fs::create_dir_all(&shard_root)?; + std::fs::write(shard_root.join("tracedecay.db"), b"graph")?; + std::fs::write(shard_root.join("sessions.db"), b"sessions")?; + std::fs::write(shard_root.join("branch-meta.json"), b"{}")?; + let manifest = StoreManifest { + schema_version: STORE_MANIFEST_SCHEMA_VERSION, + project_id: Some("proj_manifest".to_string()), + store_kind: StoreKind::CodeProject, + storage_mode: StorageMode::ProfileSharded, + project_root: manifest_root.clone(), + data_root: shard_root.clone(), + graph_db_relpath: "tracedecay.db".into(), + sessions_db_relpath: "sessions.db".into(), + branch_meta_relpath: "branch-meta.json".into(), + }; + std::fs::write( + shard_root.join(STORE_MANIFEST_FILENAME), + serde_json::to_string_pretty(&manifest)?, + )?; + + let db = crate::global_db::GlobalDb::open_at(&dir.path().join("global.db")) + .await + .ok_or_else(|| std::io::Error::other("could not open global db"))?; + db.upsert_code_project("proj_registry", ®istry_root, None, None, Some("main")) + .await + .ok_or_else(|| std::io::Error::other("could not upsert project"))?; + db.upsert_store_instance(StoreInstanceUpsert { + store_id: "store:proj_registry:profile_sharded".to_string(), + project_id: "proj_registry".to_string(), + store_kind: "code_project".to_string(), + storage_mode: "profile_sharded".to_string(), + store_relpath: shard_relpath.to_string_lossy().to_string(), + manifest_relpath: Some( + shard_relpath + .join(STORE_MANIFEST_FILENAME) + .to_string_lossy() + .to_string(), + ), + last_verified_at: Some(1_800_000_000), + last_write_at: Some(1_800_000_000), + }) + .await + .ok_or_else(|| std::io::Error::other("could not upsert store"))?; + + let findings = registry_drift_findings(&db, &profile_root).await; + let fields: Vec<_> = findings.iter().map(|finding| finding.field).collect(); + assert!( + fields.contains(&"project_id"), + "expected project_id drift finding, got {findings:#?}" + ); + assert!( + fields.contains(&"project_root"), + "expected project_root drift finding, got {findings:#?}" + ); + assert!( + findings + .iter() + .any(|finding| finding.registry_value == "proj_registry" + && finding.manifest_value == "proj_manifest"), + "project_id finding should include registry and manifest values: {findings:#?}" + ); + + Ok(()) + } } diff --git a/src/extraction/typescript_extractor.rs b/src/extraction/typescript_extractor.rs index 91beb6b8..d4db343a 100644 --- a/src/extraction/typescript_extractor.rs +++ b/src/extraction/typescript_extractor.rs @@ -1080,13 +1080,25 @@ impl TypeScriptExtractor { // Unresolved Uses reference. state.unresolved_refs.push(UnresolvedRef { - from_node_id: id, + from_node_id: id.clone(), reference_name: name, reference_kind: EdgeKind::Uses, line: start_line, column: start_column, file_path: state.file_path.clone(), }); + if let Some(module_path) = module_path.as_deref() { + for candidate in Self::ignored_dependency_import_candidates(&text, module_path) { + state.unresolved_refs.push(UnresolvedRef { + from_node_id: id.clone(), + reference_name: candidate, + reference_kind: EdgeKind::Uses, + line: start_line, + column: start_column, + file_path: state.file_path.clone(), + }); + } + } } /// Extract a namespace (`internal_module`) declaration. @@ -1292,6 +1304,44 @@ impl TypeScriptExtractor { None } + fn ignored_dependency_import_candidates(import_text: &str, module_path: &str) -> Vec { + if Self::is_project_relative_import(module_path) || !Self::is_type_only_import(import_text) + { + return Vec::new(); + } + let Some((_, after_open)) = import_text.split_once('{') else { + return Vec::new(); + }; + let Some((named_imports, _)) = after_open.split_once('}') else { + return Vec::new(); + }; + named_imports + .split(',') + .filter_map(Self::named_import_exported_name) + .map(|name| format!("npm:{module_path}#{name}")) + .collect() + } + + fn is_type_only_import(import_text: &str) -> bool { + import_text.trim_start().starts_with("import type ") + } + + fn is_project_relative_import(module_path: &str) -> bool { + module_path.starts_with('.') || module_path.starts_with('/') + } + + fn named_import_exported_name(raw: &str) -> Option { + let without_type_prefix = raw.trim().strip_prefix("type ").unwrap_or(raw.trim()); + let exported = without_type_prefix + .split_once(" as ") + .map_or(without_type_prefix, |(name, _)| name) + .trim(); + if exported.is_empty() { + return None; + } + Some(exported.to_string()) + } + /// Recursively find `call_expression` nodes inside a node and create /// unresolved Calls references. fn extract_call_sites(state: &mut ExtractionState, node: TsNode<'_>, fn_node_id: &str) { diff --git a/src/hooks/tool_hints.rs b/src/hooks/tool_hints.rs index b1e73a32..91665afd 100644 --- a/src/hooks/tool_hints.rs +++ b/src/hooks/tool_hints.rs @@ -36,6 +36,8 @@ pub enum HintCategory { Impact, SymbolLookup, FileLookup, + ProjectContext, + SessionRecall, ExploreSubagent, SubagentStartContext, } @@ -51,6 +53,8 @@ impl HintCategory { HintCategory::Impact => "impact", HintCategory::SymbolLookup => "symbol_lookup", HintCategory::FileLookup => "file_lookup", + HintCategory::ProjectContext => "project_context", + HintCategory::SessionRecall => "session_recall", HintCategory::ExploreSubagent => "explore_subagent", HintCategory::SubagentStartContext => "subagent_start_context", } @@ -66,6 +70,8 @@ impl HintCategory { "impact" => Some(HintCategory::Impact), "symbol_lookup" => Some(HintCategory::SymbolLookup), "file_lookup" => Some(HintCategory::FileLookup), + "project_context" => Some(HintCategory::ProjectContext), + "session_recall" => Some(HintCategory::SessionRecall), "explore_subagent" => Some(HintCategory::ExploreSubagent), "subagent_start_context" => Some(HintCategory::SubagentStartContext), _ => None, @@ -197,6 +203,30 @@ pub fn decide_hint(input: &ToolHintInput) -> Option { )); } + let text = combined_text(input); + if asks_for_session_recall(&text) { + return Some(hint( + HintCategory::SessionRecall, + "For prior conversation context, consider TraceDecay session search.", + "tracedecay_message_search searches ingested agent transcripts across providers; tracedecay_lcm_grep can search bounded raw-message snippets and summaries when you need session-level recall before re-discovering context.", + false, + )); + } + + if asks_for_project_context(&text) + || input + .command + .as_deref() + .is_some_and(is_project_discovery_command) + { + return Some(hint( + HintCategory::ProjectContext, + "For other repos or registered projects, consider TraceDecay project registry tools.", + "tracedecay_project_list shows known projects; tracedecay_project_search can find a sibling repo by name/path/remote; pass project_path or project_id to tracedecay_context/search for cross-project code context before scanning parent directories.", + false, + )); + } + if input .command .as_deref() @@ -245,7 +275,6 @@ pub fn decide_hint(input: &ToolHintInput) -> Option { )); } - let text = combined_text(input); if text.is_empty() { return None; } @@ -377,6 +406,34 @@ fn is_shell_search_command(command: &str) -> bool { } } +fn is_project_discovery_command(command: &str) -> bool { + let tokens = super::shell_words(command); + let Some(first) = tokens.first() else { + return false; + }; + let program = first.trim_start_matches('(').to_ascii_lowercase(); + match program.as_str() { + "find" | "fd" | "fdfind" => tokens + .iter() + .skip(1) + .any(|token| is_parent_or_projects_path(token)), + "rg" | "ripgrep" | "grep" => tokens + .iter() + .skip(1) + .any(|token| is_parent_or_projects_path(token)), + _ => false, + } +} + +fn is_parent_or_projects_path(token: &str) -> bool { + let token = token.trim_matches(|c| matches!(c, '(' | ')' | '"' | '\'')); + token == ".." + || token.starts_with("../") + || token.contains("/../") + || token.contains("/projects/") + || token.ends_with("/projects") +} + fn is_recursive_grep_flag(token: &str) -> bool { if token == "--recursive" { return true; @@ -442,6 +499,67 @@ fn asks_for_broad_read(text: &str) -> bool { ) } +fn asks_for_project_context(text: &str) -> bool { + contains_any( + text, + &[ + "another repo", + "another repository", + "other repo", + "other repository", + "external repo", + "external repository", + "sibling repo", + "sibling repository", + "neighbor repo", + "neighbor repository", + "nearby repo", + "nearby repository", + "next door", + "registered project", + "project registry", + "project listing", + "project list", + "project search", + "cross-project", + "cross project", + ], + ) || (contains_any(text, &[" repo", " repository", "workspace"]) + && contains_any( + text, + &[ + "find", + "locate", + "search", + "where", + "defined", + "lives", + "orchestrator", + ], + )) +} + +fn asks_for_session_recall(text: &str) -> bool { + contains_any( + text, + &[ + "where did we", + "what did we", + "when did we", + "did we talk", + "talk about", + "discuss before", + "mentioned before", + "prior conversation", + "previous conversation", + "earlier conversation", + "session search", + "session recall", + "conversation history", + ], + ) +} + fn asks_for_symbol_lookup(text: &str) -> bool { contains_any( text, @@ -497,6 +615,57 @@ mod tests { } } + #[test] + fn parent_directory_find_gets_project_registry_hint() { + let hint = decide_hint(&ToolHintInput { + tool_name: Some("shell".to_string()), + command: Some("find .. -maxdepth 3 -type f -iname '*runner*'".to_string()), + prompt: Some( + "Find where the clean-ci Windows runner orchestrator is defined".to_string(), + ), + session_id: Some("session-1".to_string()), + ..ToolHintInput::default() + }) + .unwrap(); + + assert_eq!(hint.category.as_key(), "project_context"); + assert!(hint.context.contains("tracedecay_project_list")); + assert!(hint.context.contains("tracedecay_project_search")); + } + + #[test] + fn external_repo_shell_search_prefers_project_registry_hint() { + let hint = decide_hint(&ToolHintInput { + tool_name: Some("shell".to_string()), + command: Some("rg -n \"proxmox|windows|runner|clean-ci\" .".to_string()), + prompt: Some( + "Find the runner orchestrator repo and update its Windows boxes".to_string(), + ), + session_id: Some("session-1".to_string()), + ..ToolHintInput::default() + }) + .unwrap(); + + assert_eq!(hint.category.as_key(), "project_context"); + assert!(hint.message.contains("registered projects")); + } + + #[test] + fn prior_conversation_prompt_gets_session_recall_hint() { + let hint = decide_hint(&ToolHintInput { + prompt: Some( + "Where did we talk about clean-ci and the runner orchestrator before?".to_string(), + ), + session_id: Some("session-1".to_string()), + ..ToolHintInput::default() + }) + .unwrap(); + + assert_eq!(hint.category.as_key(), "session_recall"); + assert!(hint.context.contains("tracedecay_message_search")); + assert!(hint.context.contains("tracedecay_lcm_grep")); + } + #[test] fn single_file_read_gets_a_soft_outline_hint() { let mut input = input_for_tool("Read"); diff --git a/src/mcp/server.rs b/src/mcp/server.rs index dd0d28e5..486c6d6b 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -1792,6 +1792,7 @@ impl McpServer { timestamp: ts, request_id: &request_id, arguments: &analytics_arguments, + response: Some(&result.value), }); self.spawn_observed_ledger_write(async move { gdb.record_savings( @@ -1973,6 +1974,7 @@ impl McpServer { timestamp: crate::tracedecay::current_timestamp(), request_id, arguments, + response: None, }); self.spawn_observed_ledger_write(async move { if let Err(e) = gdb.append_analytics_event(&event).await { @@ -2094,16 +2096,22 @@ struct McpToolAnalyticsEvent<'a> { timestamp: i64, request_id: &'a Value, arguments: &'a Value, + response: Option<&'a Value>, } fn mcp_tool_analytics_event(input: McpToolAnalyticsEvent<'_>) -> AnalyticsEventInsert { let category = crate::accounting::classifier::classify(&[input.tool_name], &[]); let mut metadata = json!({ "request_id": input.request_id, + "transport": "mcp", + "tool_kind": "mcp_tool", "before_tokens": input.raw_file_tokens, "after_tokens": input.response_tokens, "tokens_saved": input.net_saved_tokens, }); + if input.outcome == "error" { + metadata["failure_reason"] = json!("tool_dispatch_error"); + } if crate::analytics::is_skill_view_tool(input.tool_name) { metadata["arguments"] = input.arguments.clone(); metadata["function"] = json!({ @@ -2111,6 +2119,12 @@ fn mcp_tool_analytics_event(input: McpToolAnalyticsEvent<'_>) -> AnalyticsEventI "arguments": input.arguments, }); } + append_tool_response_analytics( + input.tool_name, + input.arguments, + input.response, + &mut metadata, + ); AnalyticsEventInsert { provider: "mcp".to_string(), project_id: GlobalDb::canonical_project_key(input.project_root), @@ -2128,6 +2142,68 @@ fn mcp_tool_analytics_event(input: McpToolAnalyticsEvent<'_>) -> AnalyticsEventI } } +fn append_tool_response_analytics( + tool_name: &str, + arguments: &Value, + response: Option<&Value>, + metadata: &mut Value, +) { + if tool_name != "tracedecay_context" { + return; + } + let include_memory = arguments + .get("include_memory") + .and_then(Value::as_bool) + .unwrap_or(true); + let limit = arguments + .get("memory_limit") + .and_then(Value::as_u64) + .unwrap_or(3) + .clamp(1, 10); + let min_trust = arguments + .get("memory_min_trust") + .and_then(Value::as_f64) + .unwrap_or(0.5) + .clamp(0.0, 1.0); + let payload = response.and_then(tool_result_json_payload); + let memory_matches = payload + .as_ref() + .and_then(|payload| payload.get("memory_matches")) + .and_then(Value::as_array); + let fact_ids: Vec = memory_matches + .into_iter() + .flatten() + .filter_map(|hit| { + hit.get("fact") + .and_then(|fact| fact.get("fact_id")) + .and_then(Value::as_i64) + }) + .map(Value::from) + .collect(); + let match_count = fact_ids.len(); + let memory_error = payload + .as_ref() + .and_then(|payload| payload.get("memory_matches_error")) + .and_then(Value::as_str); + metadata["context_memory"] = json!({ + "include_memory": include_memory, + "limit": limit, + "min_trust": min_trust, + "match_count": match_count, + "fact_ids": fact_ids, + "error": memory_error, + }); +} + +fn tool_result_json_payload(response: &Value) -> Option { + response + .get("content") + .and_then(Value::as_array)? + .iter() + .filter_map(|item| item.get("text").and_then(Value::as_str)) + .find_map(|text| serde_json::from_str::(text).ok()) +} + fn json_rpc_request_id_string(id: &Value) -> Option { match id { Value::String(id) => Some(id.clone()), diff --git a/src/mcp/tools/handlers/graph.rs b/src/mcp/tools/handlers/graph.rs index c1877918..2f48d4e9 100644 --- a/src/mcp/tools/handlers/graph.rs +++ b/src/mcp/tools/handlers/graph.rs @@ -2,7 +2,7 @@ //! `impact`, `node`, `similar`, `rename_preview`, `callers_for`, `by_qualified_name`, //! `signature`. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt::Write as _; use serde_json::{json, Value}; @@ -72,6 +72,7 @@ pub(super) async fn handle_search( let results = cg.search(query, limit).await?; let results = filter_by_scope(results, scope_prefix, |r| &r.node.file_path); let coverage_hint = cg.index_coverage_hint(results.len()); + let ignored_dependency_hint = ignored_dependency_hint(cg, query, limit).await?; let touched_files = unique_file_paths(results.iter().map(|r| r.node.file_path.as_str())); @@ -90,11 +91,15 @@ pub(super) async fn handle_search( }) .collect(); - let output_value = if let Some(hint) = coverage_hint { - json!({ - "results": items, - "index_coverage_hint": hint, - }) + let output_value = if coverage_hint.is_some() || ignored_dependency_hint.is_some() { + let mut value = json!({ "results": items }); + if let Some(hint) = coverage_hint { + value["index_coverage_hint"] = json!(hint); + } + if let Some(hint) = ignored_dependency_hint { + value["ignored_dependency_hint"] = hint; + } + value } else { json!(items) }; @@ -107,6 +112,60 @@ pub(super) async fn handle_search( )) } +async fn ignored_dependency_hint( + cg: &TraceDecay, + query: &str, + limit: usize, +) -> Result> { + let query = query.trim().to_ascii_lowercase(); + if query.is_empty() { + return Ok(None); + } + let db = cg.open_project_store_db().await?; + let refs = db.get_unresolved_refs().await?; + let mut seen = BTreeSet::new(); + let mut candidates = Vec::new(); + for unresolved in refs { + let Some((module, symbol)) = parse_ignored_dependency_candidate(&unresolved.reference_name) + else { + continue; + }; + let haystack = format!("{module} {symbol}").to_ascii_lowercase(); + if !haystack.contains(&query) { + continue; + } + if !seen.insert(( + module.to_string(), + symbol.to_string(), + unresolved.file_path.clone(), + unresolved.line, + )) { + continue; + } + candidates.push(json!({ + "module": module, + "symbol": symbol, + "import_file": unresolved.file_path, + "line": user_line(unresolved.line), + })); + if candidates.len() >= limit.clamp(1, 20) { + break; + } + } + if candidates.is_empty() { + return Ok(None); + } + Ok(Some(json!({ + "message": "No indexed symbol matched, but project imports reference matching symbols from an ignored dependency. Keep node_modules ignored for normal sync; use bounded lazy dependency indexing for the listed module if this symbol is needed.", + "candidates": candidates, + "suggested_action": "lazy_index_ignored_dependency", + }))) +} + +fn parse_ignored_dependency_candidate(reference_name: &str) -> Option<(&str, &str)> { + reference_name.strip_prefix("npm:")?.split_once('#') +} + fn render_search_md(value: &Value) -> String { let items = if value.is_array() { value.as_array() @@ -146,6 +205,24 @@ fn render_search_md(value: &Value) -> String { { md.blank().heading(3, "Index Coverage Hint").line(msg); } + if let Some(hint) = value.get("ignored_dependency_hint") { + let msg = hint + .get("message") + .and_then(Value::as_str) + .unwrap_or("Matching ignored dependency candidates were found."); + md.blank().heading(3, "Ignored Dependency Hint").line(msg); + if let Some(candidates) = hint.get("candidates").and_then(Value::as_array) { + for candidate in candidates { + let module = render::field_str(candidate, "module"); + let symbol = render::field_str(candidate, "symbol"); + let file = render::field_str(candidate, "import_file"); + let line = render::field_i64(candidate, "line"); + md.bullet(&format!( + "`{module}` exports `{symbol}` referenced at {file}:{line}" + )); + } + } + } md.render() } diff --git a/tests/automation_session_reflector_runner_test.rs b/tests/automation_session_reflector_runner_test.rs index 29152be2..6d98d7c4 100644 --- a/tests/automation_session_reflector_runner_test.rs +++ b/tests/automation_session_reflector_runner_test.rs @@ -1,6 +1,7 @@ mod automation_runner_support; use automation_runner_support::*; +use tracedecay::automation::fact_proposals::record_session_fact_proposals; #[tokio::test] async fn session_reflector_runner_skips_when_task_is_disabled() { @@ -314,6 +315,68 @@ async fn session_reflector_runner_validates_fact_proposals_without_applying() { assert!(records[0].applied_ops.is_none()); } +#[tokio::test] +async fn session_fact_proposals_dedupe_repeated_pending_facts_across_runs() { + let temp = tempdir().unwrap(); + let dashboard_root = temp.path().join("dashboard"); + let accepted = json!({ + "add_fact_request": { + "content": "Repeated session evidence should produce one pending fact proposal", + "category": "project", + "source": "session_reflector", + "tags": ["session-reflector"], + "entities": ["session reflector"], + "trust": 0.91, + "metadata": { + "source_span": { + "session_id": "session-a", + "message_id": "message-a" + }, + "trust_reason": "same durable fact repeated" + } + }, + "proposal": { + "content": "Repeated session evidence should produce one pending fact proposal" + }, + "validation": { + "dedupe": { + "nearest_existing_fact_id": null + } + } + }); + + let first = record_session_fact_proposals( + &dashboard_root, + "run-a", + Some("evidence-a"), + std::slice::from_ref(&accepted), + &[], + ) + .await + .unwrap(); + let second = record_session_fact_proposals( + &dashboard_root, + "run-b", + Some("evidence-b"), + std::slice::from_ref(&accepted), + &[], + ) + .await + .unwrap(); + let proposals = list_fact_proposals( + &dashboard_root, + Some(FactProposalState::PendingApproval), + 10, + ) + .await + .unwrap(); + + assert_eq!(first.len(), 1); + assert_eq!(second.len(), 0); + assert_eq!(proposals.len(), 1); + assert_eq!(proposals[0].run_id, "run-a"); +} + #[tokio::test] async fn session_reflector_runner_reads_hermes_profile_lcm_with_filters() { let temp = tempdir().unwrap(); diff --git a/tests/mcp_handler_test.rs b/tests/mcp_handler_test.rs index f5253bf1..55dfa993 100644 --- a/tests/mcp_handler_test.rs +++ b/tests/mcp_handler_test.rs @@ -1653,6 +1653,52 @@ async fn test_search_returns_index_coverage_hint_for_skipped_generated_dirs() { ); } +#[tokio::test] +async fn test_search_hints_at_ignored_dependency_candidates() { + let dir = test_temp_dir(); + let project = dir.path(); + fs::create_dir_all(project.join("src")).unwrap(); + fs::create_dir_all(project.join("node_modules/pkg")).unwrap(); + fs::write( + project.join("src/app.ts"), + r#"import type { Foo } from "pkg"; +export const value = 1; +"#, + ) + .unwrap(); + fs::write( + project.join("node_modules/pkg/index.d.ts"), + "export interface Foo { value: string }\n", + ) + .unwrap(); + + let (cg, _env) = init_test_project(project).await; + cg.index_all().await.unwrap(); + + let result = handle_tool_call( + &cg, + "tracedecay_search", + json!({"query": "Foo", "limit": 5, "format": "json"}), + None, + None, + ) + .await + .unwrap(); + let payload: Value = serde_json::from_str(extract_text(&result.value)).unwrap(); + assert_eq!( + payload["ignored_dependency_hint"]["candidates"][0]["module"].as_str(), + Some("pkg") + ); + assert_eq!( + payload["ignored_dependency_hint"]["candidates"][0]["symbol"].as_str(), + Some("Foo") + ); + assert!(payload["ignored_dependency_hint"]["message"] + .as_str() + .unwrap_or_default() + .contains("ignored dependency")); +} + #[tokio::test] async fn test_context_appends_index_coverage_hint_for_skipped_generated_dirs() { let (cg, _env, _dir) = setup_generated_dir_project(false).await; diff --git a/tests/mcp_server_test.rs b/tests/mcp_server_test.rs index a1fdee88..daf1d66a 100644 --- a/tests/mcp_server_test.rs +++ b/tests/mcp_server_test.rs @@ -2119,6 +2119,78 @@ async fn search_call_writes_mcp_runtime_analytics_event() { assert_eq!(metadata["before_tokens"], before); assert_eq!(metadata["after_tokens"], after); assert_eq!(metadata["tokens_saved"], before - after); + assert_eq!(metadata["transport"], "mcp"); + assert_eq!(metadata["tool_kind"], "mcp_tool"); +} + +#[tokio::test] +// Intentional: serializes env-mutating savings tests; #[tokio::test] +// defaults to a current-thread runtime, so no executor thread blocks. +#[allow(clippy::await_holding_lock)] +async fn context_call_writes_memory_match_analytics_without_fact_bodies() { + let _env_guard = SAVINGS_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp_global = persistent_temp_dir(); + let _env = isolated_savings_env(&tmp_global.join("global.db")); + + let (server, _proj_tmp) = setup_server().await; + let server_handle = server.clone(); + let db_path = locked_global_db_path(); + + let fact_content = + "Durable context memory analytics should report counts without leaking fact bodies."; + let added = call_tool( + server.clone(), + 9005, + "tracedecay_fact_store", + json!({ + "action": "add", + "content": fact_content, + "category": "decision", + "entity": "context memory analytics", + "trust": 0.94, + "source": "mcp-server-memory-analytics-test" + }), + ) + .await; + assert!(added["error"].is_null(), "fact add should not error"); + + let resp = call_tool( + server, + 9006, + "tracedecay_context", + json!({ + "task": "context memory analytics", + "format": "json", + "session_id": "mcp-session-9006" + }), + ) + .await; + assert!(resp["error"].is_null(), "context should not error"); + + server_handle.ledger_writes_settled().await; + let event = expect_mcp_runtime_event( + &db_path, + "tracedecay_context", + "mcp-session-9006", + "durable context MCP runtime analytics event", + ) + .await; + let metadata = analytics_metadata(&event); + + assert_eq!(event.outcome.as_deref(), Some("success")); + assert_eq!(metadata["context_memory"]["include_memory"], true); + assert!( + metadata["context_memory"]["match_count"] + .as_u64() + .is_some_and(|count| count >= 1), + "context analytics should expose bounded memory match counts: {metadata}" + ); + assert!( + !metadata.to_string().contains(fact_content), + "analytics metadata must not persist fact bodies" + ); } #[tokio::test] @@ -2166,6 +2238,9 @@ async fn failed_tool_call_writes_mcp_runtime_analytics_event() { assert_eq!(metadata["before_tokens"], 0); assert_eq!(metadata["after_tokens"], 0); assert_eq!(metadata["tokens_saved"], 0); + assert_eq!(metadata["transport"], "mcp"); + assert_eq!(metadata["tool_kind"], "mcp_tool"); + assert_eq!(metadata["failure_reason"], "tool_dispatch_error"); } #[tokio::test] diff --git a/tests/typescript_extraction_test.rs b/tests/typescript_extraction_test.rs index e24c54d9..fe53388f 100644 --- a/tests/typescript_extraction_test.rs +++ b/tests/typescript_extraction_test.rs @@ -305,6 +305,38 @@ import * as path from 'path'; assert_eq!(use_refs.len(), 2); } +#[test] +fn test_ts_import_type_records_ignored_dependency_candidates() { + let source = r#" +import type { Foo, Bar as Baz } from "pkg"; +import { localThing } from "./local"; +"#; + let extractor = TypeScriptExtractor; + let result = extractor.extract("imports.ts", source); + assert!(result.errors.is_empty(), "errors: {:?}", result.errors); + + let use_refs: Vec<_> = result + .unresolved_refs + .iter() + .filter(|r| r.reference_kind == EdgeKind::Uses) + .collect(); + + assert!( + use_refs.iter().any(|r| r.reference_name == "npm:pkg#Foo"), + "expected npm dependency candidate for Foo, got {use_refs:#?}" + ); + assert!( + use_refs.iter().any(|r| r.reference_name == "npm:pkg#Bar"), + "expected npm dependency candidate to use exported name before alias, got {use_refs:#?}" + ); + assert!( + !use_refs + .iter() + .any(|r| r.reference_name == "npm:./local#localThing"), + "relative project imports should not be dependency candidates" + ); +} + #[test] fn test_ts_async_function() { let source = r#" From 50c353201299121ea989fcf445637ae65e962295 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Mon, 29 Jun 2026 18:48:14 +0000 Subject: [PATCH 3/3] Address TraceDecay diagnostics review findings --- src/dashboard/code_diagnostics_api.rs | 169 +------------------- src/db/search.rs | 78 +++++++++ src/dependency_imports.rs | 108 +++++++++++++ src/diagnostics/lsp/activity.rs | 177 +++++++++++++++++++++ src/diagnostics/lsp/broker.rs | 14 +- src/diagnostics/lsp/mod.rs | 1 + src/doctor.rs | 115 +------------ src/doctor/registry_drift.rs | 110 +++++++++++++ src/extraction/typescript_extractor.rs | 52 +----- src/hooks/tool_hints.rs | 59 +++++-- src/lib.rs | 1 + src/mcp/mod.rs | 1 + src/mcp/server.rs | 122 +------------- src/mcp/tool_analytics.rs | 122 ++++++++++++++ src/mcp/tools/handlers/dependency_hints.rs | 64 ++++++++ src/mcp/tools/handlers/graph.rs | 85 ++-------- src/mcp/tools/handlers/mod.rs | 1 + tests/db_query_test.rs | 28 ++++ tests/typescript_extraction_test.rs | 14 +- 19 files changed, 768 insertions(+), 553 deletions(-) create mode 100644 src/dependency_imports.rs create mode 100644 src/diagnostics/lsp/activity.rs create mode 100644 src/doctor/registry_drift.rs create mode 100644 src/mcp/tool_analytics.rs create mode 100644 src/mcp/tools/handlers/dependency_hints.rs diff --git a/src/dashboard/code_diagnostics_api.rs b/src/dashboard/code_diagnostics_api.rs index 0b672b82..cfd67711 100644 --- a/src/dashboard/code_diagnostics_api.rs +++ b/src/dashboard/code_diagnostics_api.rs @@ -1,5 +1,4 @@ use std::collections::{BTreeMap, BTreeSet}; -use std::path::Path; use std::sync::atomic::Ordering; use std::time::Duration; @@ -11,9 +10,9 @@ use serde_json::{json, Value}; use super::util::{http_detail, JsonError}; use super::DashboardState; +use crate::diagnostics::lsp::activity::{active_languages_for_files, documents_for_adapter}; use crate::diagnostics::lsp::adapters::LspAdapterDefinition; use crate::diagnostics::lsp::broker::EngineState; -use crate::diagnostics::lsp::client::LspDocument; use crate::diagnostics::lsp::settings::{save_settings, IdleBackfillMode}; type ApiResult = std::result::Result, JsonError>; @@ -251,11 +250,7 @@ async fn sync_project_language_activity( .filter_map(|engine| broker.adapter_for(&engine.language)) .collect::>() }; - let active_languages = adapters - .iter() - .filter(|adapter| adapter_has_project_documents(&state.project_root, adapter, &files)) - .map(|adapter| adapter.language.clone()) - .collect::>(); + let active_languages = active_languages_for_files(&state.project_root, &adapters, &files); state .code_diagnostics .write() @@ -277,94 +272,6 @@ async fn indexed_files(conn: &libsql::Connection) -> crate::errors::Result, -) -> crate::errors::Result> { - let mut documents = Vec::new(); - for file in files { - if !matches_adapter_extension(adapter, &file) { - continue; - } - if !file_has_adapter_root_marker(project_root, adapter, &file) { - continue; - } - let path = project_root.join(&file); - let Ok(text) = tokio::fs::read_to_string(&path).await else { - continue; - }; - documents.push(LspDocument { - language: adapter.language.clone(), - language_id: language_id_for_file(adapter, &file), - relative_path: file, - text, - }); - } - Ok(documents) -} - -fn adapter_has_project_documents( - project_root: &Path, - adapter: &LspAdapterDefinition, - files: &[String], -) -> bool { - files.iter().any(|file| { - matches_adapter_extension(adapter, file) - && file_has_adapter_root_marker(project_root, adapter, file) - }) -} - -fn file_has_adapter_root_marker( - project_root: &Path, - adapter: &LspAdapterDefinition, - file: &str, -) -> bool { - if adapter.root_markers.is_empty() { - return true; - } - let path = project_root.join(file); - let mut current = path.parent(); - while let Some(dir) = current { - if adapter - .root_markers - .iter() - .any(|marker| dir.join(marker).is_file()) - { - return true; - } - if dir == project_root { - break; - } - current = dir.parent(); - } - false -} - -fn language_id_for_file(adapter: &LspAdapterDefinition, file: &str) -> String { - let extension = Path::new(file) - .extension() - .and_then(|extension| extension.to_str()) - .unwrap_or_default(); - match (adapter.language.as_str(), extension) { - ("typescript", "tsx") => "typescriptreact".to_string(), - ("javascript", "jsx") => "javascriptreact".to_string(), - _ => adapter.language_id.clone(), - } -} - -fn matches_adapter_extension(adapter: &LspAdapterDefinition, file: &str) -> bool { - Path::new(file) - .extension() - .and_then(|extension| extension.to_str()) - .is_some_and(|extension| { - adapter - .extensions - .iter() - .any(|candidate| candidate == extension) - }) -} - fn bad_request(err: &impl ToString) -> JsonError { ( StatusCode::BAD_REQUEST, @@ -392,75 +299,3 @@ fn internal_error(err: &impl ToString) -> JsonError { Json(http_detail(&err.to_string())), ) } - -#[cfg(test)] -mod tests { - use super::*; - use crate::diagnostics::lsp::adapters::{DiagnosticMode, LspAdapterDefinition}; - - #[tokio::test] - async fn documents_for_adapter_requires_a_matching_root_marker() { - let temp = tempfile::tempdir().expect("tempdir"); - let project_root = temp.path(); - let source_path = project_root.join("src/lib.fake"); - tokio::fs::create_dir_all(source_path.parent().expect("source parent")) - .await - .expect("create source dir"); - tokio::fs::write(&source_path, "fake source") - .await - .expect("write source"); - let adapter = fake_adapter("fake-root"); - - let documents = - documents_for_adapter(project_root, &adapter, vec!["src/lib.fake".to_string()]) - .await - .expect("documents"); - - assert!( - documents.is_empty(), - "adapter without a root marker should not open project documents" - ); - } - - #[tokio::test] - async fn documents_for_adapter_accepts_files_under_a_matching_root_marker() { - let temp = tempfile::tempdir().expect("tempdir"); - let project_root = temp.path(); - let package_root = project_root.join("package"); - let source_path = package_root.join("src/lib.fake"); - tokio::fs::create_dir_all(source_path.parent().expect("source parent")) - .await - .expect("create source dir"); - tokio::fs::write(package_root.join("fake-root"), "") - .await - .expect("write marker"); - tokio::fs::write(&source_path, "fake source") - .await - .expect("write source"); - let adapter = fake_adapter("fake-root"); - - let documents = documents_for_adapter( - project_root, - &adapter, - vec!["package/src/lib.fake".to_string()], - ) - .await - .expect("documents"); - - assert_eq!(documents.len(), 1); - assert_eq!(documents[0].relative_path, "package/src/lib.fake"); - } - - fn fake_adapter(root_marker: &str) -> LspAdapterDefinition { - LspAdapterDefinition { - language: "fake".to_string(), - language_id: "fake".to_string(), - command: "fake-ls".to_string(), - args: Vec::new(), - extensions: vec!["fake".to_string()], - root_markers: vec![root_marker.to_string()], - install_options: Vec::new(), - diagnostics: DiagnosticMode::Push, - } - } -} diff --git a/src/db/search.rs b/src/db/search.rs index 212fd05c..50d8d6c4 100644 --- a/src/db/search.rs +++ b/src/db/search.rs @@ -4,6 +4,7 @@ use libsql::params; use super::connection::Database; use super::rows::row_to_node; use super::sql::{build_qmark_placeholders, collect_rows}; +use crate::dependency_imports::{candidates_from_type_only_import, DependencyImportCandidate}; use crate::errors::{Result, TraceDecayError}; use crate::types::*; @@ -201,6 +202,83 @@ impl Database { collect_rows(&mut rows, row_to_node, "search_nodes_by_exact_name").await } + pub async fn dependency_import_candidates( + &self, + query: &str, + limit: usize, + ) -> Result> { + let query = query.trim(); + if query.is_empty() || limit == 0 { + return Ok(Vec::new()); + } + let query_lower = query.to_ascii_lowercase(); + let like_pattern = format!("%{query}%"); + let mut rows = self + .conn() + .query( + "SELECT name, signature, file_path, start_line + FROM nodes + WHERE kind = 'use' + AND signature LIKE ?1 + AND name NOT LIKE './%' + AND name NOT LIKE '../%' + AND name NOT LIKE '/%' + ORDER BY file_path ASC, start_line ASC + LIMIT ?2", + params![ + like_pattern.as_str(), + limit.saturating_mul(4).max(limit) as i64 + ], + ) + .await + .map_err(|e| TraceDecayError::Database { + message: format!("failed to query dependency import candidates: {e}"), + operation: "dependency_import_candidates".to_string(), + })?; + + let mut candidates = Vec::new(); + while let Some(row) = rows.next().await.map_err(|e| TraceDecayError::Database { + message: format!("failed to read dependency import candidate: {e}"), + operation: "dependency_import_candidates".to_string(), + })? { + let module = row + .get::(0) + .map_err(|e| TraceDecayError::Database { + message: format!("failed to read dependency import module: {e}"), + operation: "dependency_import_candidates".to_string(), + })?; + let signature = row + .get::(1) + .map_err(|e| TraceDecayError::Database { + message: format!("failed to read dependency import signature: {e}"), + operation: "dependency_import_candidates".to_string(), + })?; + let file_path = row + .get::(2) + .map_err(|e| TraceDecayError::Database { + message: format!("failed to read dependency import file path: {e}"), + operation: "dependency_import_candidates".to_string(), + })?; + let line = row.get::(3).map_err(|e| TraceDecayError::Database { + message: format!("failed to read dependency import line: {e}"), + operation: "dependency_import_candidates".to_string(), + })?; + candidates.extend( + candidates_from_type_only_import(&signature, &module, &file_path, line) + .into_iter() + .filter(|candidate| { + candidate.symbol.to_ascii_lowercase().contains(&query_lower) + || candidate.module.to_ascii_lowercase().contains(&query_lower) + }), + ); + if candidates.len() >= limit { + candidates.truncate(limit); + break; + } + } + Ok(candidates) + } + /// Returns `true` if the error indicates `SQLite` database corruption. pub fn is_corruption_error(e: &TraceDecayError) -> bool { match e { diff --git a/src/dependency_imports.rs b/src/dependency_imports.rs new file mode 100644 index 00000000..158f15fd --- /dev/null +++ b/src/dependency_imports.rs @@ -0,0 +1,108 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DependencyImportCandidate { + pub module: String, + pub symbol: String, + pub import_file: String, + pub line: u32, +} + +pub fn candidates_from_type_only_import( + import_text: &str, + module_path: &str, + import_file: &str, + line: u32, +) -> Vec { + if is_project_relative_import(module_path) || !is_type_only_import(import_text) { + return Vec::new(); + } + let Some((_, after_open)) = import_text.split_once('{') else { + return Vec::new(); + }; + let Some((named_imports, _)) = after_open.split_once('}') else { + return Vec::new(); + }; + named_imports + .split(',') + .filter_map(named_import_exported_name) + .map(|symbol| DependencyImportCandidate { + module: module_path.to_string(), + symbol, + import_file: import_file.to_string(), + line, + }) + .collect() +} + +fn is_type_only_import(import_text: &str) -> bool { + import_text.trim_start().starts_with("import type ") +} + +fn is_project_relative_import(module_path: &str) -> bool { + module_path.starts_with('.') || module_path.starts_with('/') +} + +fn named_import_exported_name(raw: &str) -> Option { + let trimmed = raw.trim(); + let without_type_prefix = trimmed.strip_prefix("type ").unwrap_or(trimmed); + let exported = without_type_prefix + .split_once(" as ") + .map_or(without_type_prefix, |(name, _)| name) + .trim(); + if exported.is_empty() { + return None; + } + Some(exported.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn type_only_named_imports_become_dependency_candidates() { + let candidates = candidates_from_type_only_import( + "import type { Foo, Bar as Baz } from \"pkg\";", + "pkg", + "src/app.ts", + 7, + ); + + assert_eq!( + candidates, + vec![ + DependencyImportCandidate { + module: "pkg".to_string(), + symbol: "Foo".to_string(), + import_file: "src/app.ts".to_string(), + line: 7, + }, + DependencyImportCandidate { + module: "pkg".to_string(), + symbol: "Bar".to_string(), + import_file: "src/app.ts".to_string(), + line: 7, + }, + ] + ); + } + + #[test] + fn relative_and_value_imports_are_not_dependency_candidates() { + assert!(candidates_from_type_only_import( + "import type { Local } from \"./local\";", + "./local", + "src/app.ts", + 1, + ) + .is_empty()); + assert!(candidates_from_type_only_import( + "import { Foo } from \"pkg\";", + "pkg", + "src/app.ts", + 1, + ) + .is_empty()); + } +} diff --git a/src/diagnostics/lsp/activity.rs b/src/diagnostics/lsp/activity.rs new file mode 100644 index 00000000..8e8191a5 --- /dev/null +++ b/src/diagnostics/lsp/activity.rs @@ -0,0 +1,177 @@ +use std::collections::BTreeSet; +use std::path::Path; + +use crate::diagnostics::lsp::adapters::LspAdapterDefinition; +use crate::diagnostics::lsp::client::LspDocument; + +pub fn active_languages_for_files( + project_root: &Path, + adapters: &[LspAdapterDefinition], + files: &[String], +) -> BTreeSet { + adapters + .iter() + .filter(|adapter| adapter_has_project_documents(project_root, adapter, files)) + .map(|adapter| adapter.language.clone()) + .collect() +} + +pub fn adapter_has_project_documents( + project_root: &Path, + adapter: &LspAdapterDefinition, + files: &[String], +) -> bool { + files.iter().any(|file| { + matches_adapter_extension(adapter, file) + && file_has_adapter_root_marker(project_root, adapter, file) + }) +} + +pub async fn documents_for_adapter( + project_root: &Path, + adapter: &LspAdapterDefinition, + files: Vec, +) -> crate::errors::Result> { + let mut documents = Vec::new(); + for file in files { + if !matches_adapter_extension(adapter, &file) { + continue; + } + if !file_has_adapter_root_marker(project_root, adapter, &file) { + continue; + } + let path = project_root.join(&file); + let Ok(text) = tokio::fs::read_to_string(&path).await else { + continue; + }; + documents.push(LspDocument { + language: adapter.language.clone(), + language_id: language_id_for_file(adapter, &file), + relative_path: file, + text, + }); + } + Ok(documents) +} + +fn language_id_for_file(adapter: &LspAdapterDefinition, file: &str) -> String { + let extension = Path::new(file) + .extension() + .and_then(|extension| extension.to_str()) + .unwrap_or_default(); + match (adapter.language.as_str(), extension) { + ("typescript", "tsx") => "typescriptreact".to_string(), + ("javascript", "jsx") => "javascriptreact".to_string(), + _ => adapter.language_id.clone(), + } +} + +fn matches_adapter_extension(adapter: &LspAdapterDefinition, file: &str) -> bool { + Path::new(file) + .extension() + .and_then(|extension| extension.to_str()) + .is_some_and(|extension| { + adapter + .extensions + .iter() + .any(|candidate| candidate == extension) + }) +} + +fn file_has_adapter_root_marker( + project_root: &Path, + adapter: &LspAdapterDefinition, + file: &str, +) -> bool { + if adapter.root_markers.is_empty() { + return true; + } + let path = project_root.join(file); + let mut current = path.parent(); + while let Some(dir) = current { + if adapter + .root_markers + .iter() + .any(|marker| dir.join(marker).is_file()) + { + return true; + } + if dir == project_root { + break; + } + current = dir.parent(); + } + false +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::diagnostics::lsp::adapters::DiagnosticMode; + + #[tokio::test] + async fn documents_for_adapter_requires_a_matching_root_marker() { + let temp = tempfile::tempdir().expect("tempdir"); + let project_root = temp.path(); + let source_path = project_root.join("src/lib.fake"); + tokio::fs::create_dir_all(source_path.parent().expect("source parent")) + .await + .expect("create source dir"); + tokio::fs::write(&source_path, "fake source") + .await + .expect("write source"); + let adapter = fake_adapter("fake-root"); + + let documents = + documents_for_adapter(project_root, &adapter, vec!["src/lib.fake".to_string()]) + .await + .expect("documents"); + + assert!( + documents.is_empty(), + "adapter without a root marker should not open project documents" + ); + } + + #[tokio::test] + async fn documents_for_adapter_accepts_files_under_a_matching_root_marker() { + let temp = tempfile::tempdir().expect("tempdir"); + let project_root = temp.path(); + let package_root = project_root.join("package"); + let source_path = package_root.join("src/lib.fake"); + tokio::fs::create_dir_all(source_path.parent().expect("source parent")) + .await + .expect("create source dir"); + tokio::fs::write(package_root.join("fake-root"), "") + .await + .expect("write marker"); + tokio::fs::write(&source_path, "fake source") + .await + .expect("write source"); + let adapter = fake_adapter("fake-root"); + + let documents = documents_for_adapter( + project_root, + &adapter, + vec!["package/src/lib.fake".to_string()], + ) + .await + .expect("documents"); + + assert_eq!(documents.len(), 1); + assert_eq!(documents[0].relative_path, "package/src/lib.fake"); + } + + fn fake_adapter(root_marker: &str) -> LspAdapterDefinition { + LspAdapterDefinition { + language: "fake".to_string(), + language_id: "fake".to_string(), + command: "fake-ls".to_string(), + args: Vec::new(), + extensions: vec!["fake".to_string()], + root_markers: vec![root_marker.to_string()], + install_options: Vec::new(), + diagnostics: DiagnosticMode::Push, + } + } +} diff --git a/src/diagnostics/lsp/broker.rs b/src/diagnostics/lsp/broker.rs index c7c212cd..eeaf320c 100644 --- a/src/diagnostics/lsp/broker.rs +++ b/src/diagnostics/lsp/broker.rs @@ -187,10 +187,6 @@ impl DiagnosticBroker { adapters: Vec, settings: CodeDiagnosticsSettings, ) -> Self { - let project_languages = adapters - .iter() - .map(|adapter| adapter.language.clone()) - .collect(); Self { project_root: project_root.into(), adapters, @@ -199,7 +195,7 @@ impl DiagnosticBroker { clients: BTreeMap::new(), engine_overrides: BTreeMap::new(), engine_errors: BTreeMap::new(), - project_languages, + project_languages: BTreeSet::new(), backfill: BTreeMap::new(), } } @@ -208,7 +204,13 @@ impl DiagnosticBroker { project_root: impl Into, adapters: Vec, ) -> Self { - Self::new(project_root, adapters, CodeDiagnosticsSettings::default()) + let project_languages = adapters + .iter() + .map(|adapter| adapter.language.clone()) + .collect(); + let mut broker = Self::new(project_root, adapters, CodeDiagnosticsSettings::default()); + broker.update_project_languages(project_languages); + broker } pub fn snapshot(&self) -> DiagnosticsSnapshot { diff --git a/src/diagnostics/lsp/mod.rs b/src/diagnostics/lsp/mod.rs index 094c99dd..957ad8b6 100644 --- a/src/diagnostics/lsp/mod.rs +++ b/src/diagnostics/lsp/mod.rs @@ -1,5 +1,6 @@ //! Dashboard-owned LSP diagnostics support. +pub mod activity; pub mod adapters; pub mod broker; pub mod client; diff --git a/src/doctor.rs b/src/doctor.rs index 8cf22fae..6fea6dca 100644 --- a/src/doctor.rs +++ b/src/doctor.rs @@ -9,6 +9,8 @@ use crate::agents::{self, DoctorCounters, HealthcheckContext}; use crate::display::{format_bytes, format_token_count}; use crate::tracedecay::TraceDecay; +mod registry_drift; + /// Runs a comprehensive health check of the tracedecay installation. pub async fn run_doctor(agent_filter: Option<&str>) { debug_assert!( @@ -180,7 +182,7 @@ async fn check_stale_stores(dc: &mut DoctorCounters) { check_orphan_store_manifests(dc, &project_paths); check_stale_code_projects(dc, &gdb).await; if let Some(profile_root) = profile_root.as_deref() { - let drift = registry_drift_findings(&gdb, profile_root).await; + let drift = registry_drift::registry_drift_findings(&gdb, profile_root).await; if drift.is_empty() { dc.pass("No registry/store manifest identity drift"); } else { @@ -312,76 +314,6 @@ fn code_project_root_exists(project: &crate::global_db::CodeProjectRecord) -> bo Path::new(&project.canonical_root).exists() || Path::new(&project.display_root).exists() } -#[derive(Debug, Clone, PartialEq, Eq)] -struct RegistryDriftFinding { - project_id: String, - store_id: String, - field: &'static str, - registry_value: String, - manifest_value: String, - manifest_path: PathBuf, -} - -async fn registry_drift_findings( - global_db: &crate::global_db::GlobalDb, - profile_root: &Path, -) -> Vec { - let mut findings = Vec::new(); - for project in global_db.list_code_projects(usize::MAX).await { - let Some(context) = global_db - .project_registry_context_by_id(&project.project_id) - .await - else { - continue; - }; - for store_context in context.stores { - let store = store_context.store; - let Some(manifest_path) = resolve_registry_manifest_path(profile_root, &store) else { - continue; - }; - let Ok(manifest) = crate::storage::read_store_manifest(&manifest_path) else { - continue; - }; - let manifest_project_id = manifest - .project_id - .as_deref() - .unwrap_or("") - .to_string(); - if manifest_project_id != store.project_id { - findings.push(RegistryDriftFinding { - project_id: project.project_id.clone(), - store_id: store.store_id.clone(), - field: "project_id", - registry_value: store.project_id.clone(), - manifest_value: manifest_project_id, - manifest_path: manifest_path.clone(), - }); - } - - let registry_project_root = comparable_path(Path::new(&project.canonical_root)); - let manifest_project_root = comparable_path(&manifest.project_root); - if registry_project_root != manifest_project_root { - findings.push(RegistryDriftFinding { - project_id: project.project_id.clone(), - store_id: store.store_id.clone(), - field: "project_root", - registry_value: registry_project_root, - manifest_value: manifest_project_root, - manifest_path, - }); - } - } - } - findings -} - -fn comparable_path(path: &Path) -> String { - path.canonicalize() - .unwrap_or_else(|_| path.to_path_buf()) - .to_string_lossy() - .to_string() -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum DoctorStorageStatus { RepoLocal, @@ -481,45 +413,6 @@ fn classify_registry_storage( } } -fn resolve_registry_manifest_path( - profile_root: &Path, - store: &crate::global_db::StoreInstanceRecord, -) -> Option { - if store.storage_mode != "profile_sharded" { - return None; - } - let store_relpath = registry_relpath(&store.store_relpath); - let manifest_relpath = store - .manifest_relpath - .as_ref() - .map(|relpath| registry_relpath(relpath)); - for profile_root in registry_profile_roots(profile_root) { - let Ok(data_root) = - crate::storage::StoreArtifactPath::resolve(&profile_root, &store_relpath) - else { - continue; - }; - let data_root = data_root.absolute_path(); - if let Some(relpath) = manifest_relpath.as_ref() { - for root in [&profile_root, &data_root] { - let Ok(path) = crate::storage::StoreArtifactPath::resolve(root, relpath) else { - continue; - }; - let path = path.absolute_path(); - if path.is_file() { - return Some(path); - } - } - } else { - let path = data_root.join(crate::storage::STORE_MANIFEST_FILENAME); - if path.is_file() { - return Some(path); - } - } - } - None -} - fn registry_relpath(value: &str) -> PathBuf { let path = Path::new(value); if path.is_absolute() @@ -923,7 +816,7 @@ mod tests { .await .ok_or_else(|| std::io::Error::other("could not upsert store"))?; - let findings = registry_drift_findings(&db, &profile_root).await; + let findings = registry_drift::registry_drift_findings(&db, &profile_root).await; let fields: Vec<_> = findings.iter().map(|finding| finding.field).collect(); assert!( fields.contains(&"project_id"), diff --git a/src/doctor/registry_drift.rs b/src/doctor/registry_drift.rs new file mode 100644 index 00000000..a269dde5 --- /dev/null +++ b/src/doctor/registry_drift.rs @@ -0,0 +1,110 @@ +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct RegistryDriftFinding { + pub(super) project_id: String, + pub(super) store_id: String, + pub(super) field: &'static str, + pub(super) registry_value: String, + pub(super) manifest_value: String, + pub(super) manifest_path: PathBuf, +} + +pub(super) async fn registry_drift_findings( + global_db: &crate::global_db::GlobalDb, + profile_root: &Path, +) -> Vec { + let mut findings = Vec::new(); + for project in global_db.list_code_projects(usize::MAX).await { + let Some(context) = global_db + .project_registry_context_by_id(&project.project_id) + .await + else { + continue; + }; + for store_context in context.stores { + let store = store_context.store; + let Some(manifest_path) = resolve_registry_manifest_path(profile_root, &store) else { + continue; + }; + let Ok(manifest) = crate::storage::read_store_manifest(&manifest_path) else { + continue; + }; + let manifest_project_id = manifest + .project_id + .as_deref() + .unwrap_or("") + .to_string(); + if manifest_project_id != store.project_id { + findings.push(RegistryDriftFinding { + project_id: project.project_id.clone(), + store_id: store.store_id.clone(), + field: "project_id", + registry_value: store.project_id.clone(), + manifest_value: manifest_project_id, + manifest_path: manifest_path.clone(), + }); + } + + let registry_project_root = comparable_path(Path::new(&project.canonical_root)); + let manifest_project_root = comparable_path(&manifest.project_root); + if registry_project_root != manifest_project_root { + findings.push(RegistryDriftFinding { + project_id: project.project_id.clone(), + store_id: store.store_id.clone(), + field: "project_root", + registry_value: registry_project_root, + manifest_value: manifest_project_root, + manifest_path, + }); + } + } + } + findings +} + +fn resolve_registry_manifest_path( + profile_root: &Path, + store: &crate::global_db::StoreInstanceRecord, +) -> Option { + if store.storage_mode != "profile_sharded" { + return None; + } + let store_relpath = super::registry_relpath(&store.store_relpath); + let manifest_relpath = store + .manifest_relpath + .as_ref() + .map(|relpath| super::registry_relpath(relpath)); + for profile_root in super::registry_profile_roots(profile_root) { + let Ok(data_root) = + crate::storage::StoreArtifactPath::resolve(&profile_root, &store_relpath) + else { + continue; + }; + let data_root = data_root.absolute_path(); + if let Some(relpath) = manifest_relpath.as_ref() { + for root in [&profile_root, &data_root] { + let Ok(path) = crate::storage::StoreArtifactPath::resolve(root, relpath) else { + continue; + }; + let path = path.absolute_path(); + if path.is_file() { + return Some(path); + } + } + } else { + let path = data_root.join(crate::storage::STORE_MANIFEST_FILENAME); + if path.is_file() { + return Some(path); + } + } + } + None +} + +fn comparable_path(path: &Path) -> String { + path.canonicalize() + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .to_string() +} diff --git a/src/extraction/typescript_extractor.rs b/src/extraction/typescript_extractor.rs index d4db343a..91beb6b8 100644 --- a/src/extraction/typescript_extractor.rs +++ b/src/extraction/typescript_extractor.rs @@ -1080,25 +1080,13 @@ impl TypeScriptExtractor { // Unresolved Uses reference. state.unresolved_refs.push(UnresolvedRef { - from_node_id: id.clone(), + from_node_id: id, reference_name: name, reference_kind: EdgeKind::Uses, line: start_line, column: start_column, file_path: state.file_path.clone(), }); - if let Some(module_path) = module_path.as_deref() { - for candidate in Self::ignored_dependency_import_candidates(&text, module_path) { - state.unresolved_refs.push(UnresolvedRef { - from_node_id: id.clone(), - reference_name: candidate, - reference_kind: EdgeKind::Uses, - line: start_line, - column: start_column, - file_path: state.file_path.clone(), - }); - } - } } /// Extract a namespace (`internal_module`) declaration. @@ -1304,44 +1292,6 @@ impl TypeScriptExtractor { None } - fn ignored_dependency_import_candidates(import_text: &str, module_path: &str) -> Vec { - if Self::is_project_relative_import(module_path) || !Self::is_type_only_import(import_text) - { - return Vec::new(); - } - let Some((_, after_open)) = import_text.split_once('{') else { - return Vec::new(); - }; - let Some((named_imports, _)) = after_open.split_once('}') else { - return Vec::new(); - }; - named_imports - .split(',') - .filter_map(Self::named_import_exported_name) - .map(|name| format!("npm:{module_path}#{name}")) - .collect() - } - - fn is_type_only_import(import_text: &str) -> bool { - import_text.trim_start().starts_with("import type ") - } - - fn is_project_relative_import(module_path: &str) -> bool { - module_path.starts_with('.') || module_path.starts_with('/') - } - - fn named_import_exported_name(raw: &str) -> Option { - let without_type_prefix = raw.trim().strip_prefix("type ").unwrap_or(raw.trim()); - let exported = without_type_prefix - .split_once(" as ") - .map_or(without_type_prefix, |(name, _)| name) - .trim(); - if exported.is_empty() { - return None; - } - Some(exported.to_string()) - } - /// Recursively find `call_expression` nodes inside a node and create /// unresolved Calls references. fn extract_call_sites(state: &mut ExtractionState, node: TsNode<'_>, fn_node_id: &str) { diff --git a/src/hooks/tool_hints.rs b/src/hooks/tool_hints.rs index 91665afd..5de05e0a 100644 --- a/src/hooks/tool_hints.rs +++ b/src/hooks/tool_hints.rs @@ -500,6 +500,10 @@ fn asks_for_broad_read(text: &str) -> bool { } fn asks_for_project_context(text: &str) -> bool { + mentions_external_project_scope(text) || asks_for_repo_discovery(text) +} + +fn mentions_external_project_scope(text: &str) -> bool { contains_any( text, &[ @@ -524,19 +528,33 @@ fn asks_for_project_context(text: &str) -> bool { "cross-project", "cross project", ], - ) || (contains_any(text, &[" repo", " repository", "workspace"]) - && contains_any( - text, - &[ - "find", - "locate", - "search", - "where", - "defined", - "lives", - "orchestrator", - ], - )) + ) +} + +fn asks_for_repo_discovery(text: &str) -> bool { + !mentions_current_project_scope(text) + && contains_any(text, &[" repo", " repository"]) + && contains_any(text, &["find", "locate", "where", "which"]) +} + +fn mentions_current_project_scope(text: &str) -> bool { + contains_any( + text, + &[ + "this repo", + "this repository", + "current repo", + "current repository", + "current workspace", + "this workspace", + "in repo", + "in repository", + "in the repo", + "in the repository", + "inside repo", + "inside the repo", + ], + ) } fn asks_for_session_recall(text: &str) -> bool { @@ -650,6 +668,21 @@ mod tests { assert!(hint.message.contains("registered projects")); } + #[test] + fn current_repo_shell_search_keeps_normal_search_hint() { + let hint = decide_hint(&ToolHintInput { + tool_name: Some("shell".to_string()), + command: Some("rg -n \"runner\" .".to_string()), + prompt: Some("Search this repo for the runner implementation".to_string()), + session_id: Some("session-1".to_string()), + ..ToolHintInput::default() + }) + .unwrap(); + + assert_eq!(hint.category.as_key(), "search"); + assert!(hint.context.contains("tracedecay_search")); + } + #[test] fn prior_conversation_prompt_gets_session_recall_hint() { let hint = decide_hint(&ToolHintInput { diff --git a/src/lib.rs b/src/lib.rs index ddaf566d..8e191f12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,6 +29,7 @@ pub mod context; pub mod daemon; pub mod dashboard; pub mod db; +mod dependency_imports; pub mod derive_table; pub mod diagnose; pub mod diagnostics; diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index cd113273..5a98da7e 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -7,6 +7,7 @@ /// MCP server implementation. pub mod response_handles; pub mod server; +mod tool_analytics; /// Tool definitions and dispatch. pub mod tools; diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 486c6d6b..935580bc 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -13,10 +13,11 @@ use std::time::{Duration, Instant}; use serde_json::{json, Value}; use crate::errors::{Result, TraceDecayError}; -use crate::global_db::{AnalyticsEventInsert, GlobalDb}; +use crate::global_db::GlobalDb; use crate::mcp::response_handles::{ cleanup_expired_response_handles, response_handle_stats_json, RESPONSE_RETRIEVE_TOOL, }; +use crate::mcp::tool_analytics::{mcp_tool_analytics_event, McpToolAnalyticsEvent}; use crate::path_tree::format_compact_annotated_path_list; use crate::tracedecay::TraceDecay; @@ -2085,125 +2086,6 @@ fn mcp_analytics_session_id(arguments: &Value) -> Option { }) } -struct McpToolAnalyticsEvent<'a> { - project_root: &'a std::path::Path, - session_id: Option, - tool_name: &'a str, - outcome: &'a str, - raw_file_tokens: u64, - response_tokens: u64, - net_saved_tokens: u64, - timestamp: i64, - request_id: &'a Value, - arguments: &'a Value, - response: Option<&'a Value>, -} - -fn mcp_tool_analytics_event(input: McpToolAnalyticsEvent<'_>) -> AnalyticsEventInsert { - let category = crate::accounting::classifier::classify(&[input.tool_name], &[]); - let mut metadata = json!({ - "request_id": input.request_id, - "transport": "mcp", - "tool_kind": "mcp_tool", - "before_tokens": input.raw_file_tokens, - "after_tokens": input.response_tokens, - "tokens_saved": input.net_saved_tokens, - }); - if input.outcome == "error" { - metadata["failure_reason"] = json!("tool_dispatch_error"); - } - if crate::analytics::is_skill_view_tool(input.tool_name) { - metadata["arguments"] = input.arguments.clone(); - metadata["function"] = json!({ - "name": input.tool_name, - "arguments": input.arguments, - }); - } - append_tool_response_analytics( - input.tool_name, - input.arguments, - input.response, - &mut metadata, - ); - AnalyticsEventInsert { - provider: "mcp".to_string(), - project_id: GlobalDb::canonical_project_key(input.project_root), - session_id: input.session_id, - timestamp: input.timestamp, - event_kind: "mcp_tool_call".to_string(), - hook_name: None, - tool_name: Some(input.tool_name.to_string()), - tool_category: Some(category.as_str().to_string()), - skill_name: None, - hint_category: None, - hint_id: None, - outcome: Some(input.outcome.to_string()), - metadata_json: Some(metadata.to_string()), - } -} - -fn append_tool_response_analytics( - tool_name: &str, - arguments: &Value, - response: Option<&Value>, - metadata: &mut Value, -) { - if tool_name != "tracedecay_context" { - return; - } - let include_memory = arguments - .get("include_memory") - .and_then(Value::as_bool) - .unwrap_or(true); - let limit = arguments - .get("memory_limit") - .and_then(Value::as_u64) - .unwrap_or(3) - .clamp(1, 10); - let min_trust = arguments - .get("memory_min_trust") - .and_then(Value::as_f64) - .unwrap_or(0.5) - .clamp(0.0, 1.0); - let payload = response.and_then(tool_result_json_payload); - let memory_matches = payload - .as_ref() - .and_then(|payload| payload.get("memory_matches")) - .and_then(Value::as_array); - let fact_ids: Vec = memory_matches - .into_iter() - .flatten() - .filter_map(|hit| { - hit.get("fact") - .and_then(|fact| fact.get("fact_id")) - .and_then(Value::as_i64) - }) - .map(Value::from) - .collect(); - let match_count = fact_ids.len(); - let memory_error = payload - .as_ref() - .and_then(|payload| payload.get("memory_matches_error")) - .and_then(Value::as_str); - metadata["context_memory"] = json!({ - "include_memory": include_memory, - "limit": limit, - "min_trust": min_trust, - "match_count": match_count, - "fact_ids": fact_ids, - "error": memory_error, - }); -} - -fn tool_result_json_payload(response: &Value) -> Option { - response - .get("content") - .and_then(Value::as_array)? - .iter() - .filter_map(|item| item.get("text").and_then(Value::as_str)) - .find_map(|text| serde_json::from_str::(text).ok()) -} - fn json_rpc_request_id_string(id: &Value) -> Option { match id { Value::String(id) => Some(id.clone()), diff --git a/src/mcp/tool_analytics.rs b/src/mcp/tool_analytics.rs new file mode 100644 index 00000000..f6e5bfd6 --- /dev/null +++ b/src/mcp/tool_analytics.rs @@ -0,0 +1,122 @@ +use serde_json::{json, Value}; + +use crate::global_db::{AnalyticsEventInsert, GlobalDb}; + +pub(super) struct McpToolAnalyticsEvent<'a> { + pub(super) project_root: &'a std::path::Path, + pub(super) session_id: Option, + pub(super) tool_name: &'a str, + pub(super) outcome: &'a str, + pub(super) raw_file_tokens: u64, + pub(super) response_tokens: u64, + pub(super) net_saved_tokens: u64, + pub(super) timestamp: i64, + pub(super) request_id: &'a Value, + pub(super) arguments: &'a Value, + pub(super) response: Option<&'a Value>, +} + +pub(super) fn mcp_tool_analytics_event(input: McpToolAnalyticsEvent<'_>) -> AnalyticsEventInsert { + let category = crate::accounting::classifier::classify(&[input.tool_name], &[]); + let mut metadata = json!({ + "request_id": input.request_id, + "transport": "mcp", + "tool_kind": "mcp_tool", + "before_tokens": input.raw_file_tokens, + "after_tokens": input.response_tokens, + "tokens_saved": input.net_saved_tokens, + }); + if input.outcome == "error" { + metadata["failure_reason"] = json!("tool_dispatch_error"); + } + if crate::analytics::is_skill_view_tool(input.tool_name) { + metadata["arguments"] = input.arguments.clone(); + metadata["function"] = json!({ + "name": input.tool_name, + "arguments": input.arguments, + }); + } + append_tool_response_analytics( + input.tool_name, + input.arguments, + input.response, + &mut metadata, + ); + AnalyticsEventInsert { + provider: "mcp".to_string(), + project_id: GlobalDb::canonical_project_key(input.project_root), + session_id: input.session_id, + timestamp: input.timestamp, + event_kind: "mcp_tool_call".to_string(), + hook_name: None, + tool_name: Some(input.tool_name.to_string()), + tool_category: Some(category.as_str().to_string()), + skill_name: None, + hint_category: None, + hint_id: None, + outcome: Some(input.outcome.to_string()), + metadata_json: Some(metadata.to_string()), + } +} + +fn append_tool_response_analytics( + tool_name: &str, + arguments: &Value, + response: Option<&Value>, + metadata: &mut Value, +) { + if tool_name != "tracedecay_context" { + return; + } + let include_memory = arguments + .get("include_memory") + .and_then(Value::as_bool) + .unwrap_or(true); + let limit = arguments + .get("memory_limit") + .and_then(Value::as_u64) + .unwrap_or(3) + .clamp(1, 10); + let min_trust = arguments + .get("memory_min_trust") + .and_then(Value::as_f64) + .unwrap_or(0.5) + .clamp(0.0, 1.0); + let payload = response.and_then(tool_result_json_payload); + let memory_matches = payload + .as_ref() + .and_then(|payload| payload.get("memory_matches")) + .and_then(Value::as_array); + let fact_ids: Vec = memory_matches + .into_iter() + .flatten() + .filter_map(|hit| { + hit.get("fact") + .and_then(|fact| fact.get("fact_id")) + .and_then(Value::as_i64) + }) + .map(Value::from) + .collect(); + let match_count = fact_ids.len(); + let memory_error = payload + .as_ref() + .and_then(|payload| payload.get("memory_matches_error")) + .and_then(Value::as_str); + metadata["context_memory"] = json!({ + "include_memory": include_memory, + "limit": limit, + "min_trust": min_trust, + "match_count": match_count, + "fact_ids": fact_ids, + "error": memory_error, + }); +} + +fn tool_result_json_payload(response: &Value) -> Option { + response + .get("content") + .and_then(Value::as_array)? + .iter() + .filter_map(|item| item.get("text").and_then(Value::as_str)) + .find_map(|text| serde_json::from_str::(text).ok()) +} diff --git a/src/mcp/tools/handlers/dependency_hints.rs b/src/mcp/tools/handlers/dependency_hints.rs new file mode 100644 index 00000000..d611bd5a --- /dev/null +++ b/src/mcp/tools/handlers/dependency_hints.rs @@ -0,0 +1,64 @@ +use serde_json::{json, Value}; + +use crate::errors::Result; +use crate::mcp::tools::render::{self, Md}; +use crate::tracedecay::TraceDecay; + +pub(super) async fn ignored_dependency_hint( + cg: &TraceDecay, + query: &str, + limit: usize, +) -> Result> { + let query = query.trim().to_ascii_lowercase(); + if query.is_empty() { + return Ok(None); + } + let db = cg.open_project_store_db().await?; + let candidates = db + .dependency_import_candidates(&query, limit.clamp(1, 20)) + .await? + .into_iter() + .map(|candidate| { + json!({ + "module": candidate.module, + "symbol": candidate.symbol, + "import_file": candidate.import_file, + "line": user_line(candidate.line), + }) + }) + .collect::>(); + if candidates.is_empty() { + return Ok(None); + } + Ok(Some(json!({ + "message": "No indexed symbol matched, but project imports reference matching symbols from an ignored dependency. Keep node_modules ignored for normal sync; use bounded lazy dependency indexing for the listed module if this symbol is needed.", + "candidates": candidates, + "suggested_action": "lazy_index_ignored_dependency", + }))) +} + +pub(super) fn append_ignored_dependency_hint_md(md: &mut Md, value: &Value) { + let Some(hint) = value.get("ignored_dependency_hint") else { + return; + }; + let msg = hint + .get("message") + .and_then(Value::as_str) + .unwrap_or("Matching ignored dependency candidates were found."); + md.blank().heading(3, "Ignored Dependency Hint").line(msg); + if let Some(candidates) = hint.get("candidates").and_then(Value::as_array) { + for candidate in candidates { + let module = render::field_str(candidate, "module"); + let symbol = render::field_str(candidate, "symbol"); + let file = render::field_str(candidate, "import_file"); + let line = render::field_i64(candidate, "line"); + md.bullet(&format!( + "`{module}` exports `{symbol}` referenced at {file}:{line}" + )); + } + } +} + +fn user_line(line: u32) -> u32 { + line.saturating_add(1) +} diff --git a/src/mcp/tools/handlers/graph.rs b/src/mcp/tools/handlers/graph.rs index 2f48d4e9..f71e22cd 100644 --- a/src/mcp/tools/handlers/graph.rs +++ b/src/mcp/tools/handlers/graph.rs @@ -2,7 +2,7 @@ //! `impact`, `node`, `similar`, `rename_preview`, `callers_for`, `by_qualified_name`, //! `signature`. -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::fmt::Write as _; use serde_json::{json, Value}; @@ -20,6 +20,7 @@ const CONTEXT_MEMORY_SNIPPET_CHARS: usize = 240; use super::super::render::{self, Md}; use super::super::ToolResult; +use super::dependency_hints; use super::support::{ effective_path, filter_by_scope, require_node_id, string_array_values, unique_file_paths, }; @@ -72,7 +73,14 @@ pub(super) async fn handle_search( let results = cg.search(query, limit).await?; let results = filter_by_scope(results, scope_prefix, |r| &r.node.file_path); let coverage_hint = cg.index_coverage_hint(results.len()); - let ignored_dependency_hint = ignored_dependency_hint(cg, query, limit).await?; + let has_symbol_result = results + .iter() + .any(|result| result.node.kind != NodeKind::Use); + let ignored_dependency_hint = if has_symbol_result { + None + } else { + dependency_hints::ignored_dependency_hint(cg, query, limit).await? + }; let touched_files = unique_file_paths(results.iter().map(|r| r.node.file_path.as_str())); @@ -112,60 +120,6 @@ pub(super) async fn handle_search( )) } -async fn ignored_dependency_hint( - cg: &TraceDecay, - query: &str, - limit: usize, -) -> Result> { - let query = query.trim().to_ascii_lowercase(); - if query.is_empty() { - return Ok(None); - } - let db = cg.open_project_store_db().await?; - let refs = db.get_unresolved_refs().await?; - let mut seen = BTreeSet::new(); - let mut candidates = Vec::new(); - for unresolved in refs { - let Some((module, symbol)) = parse_ignored_dependency_candidate(&unresolved.reference_name) - else { - continue; - }; - let haystack = format!("{module} {symbol}").to_ascii_lowercase(); - if !haystack.contains(&query) { - continue; - } - if !seen.insert(( - module.to_string(), - symbol.to_string(), - unresolved.file_path.clone(), - unresolved.line, - )) { - continue; - } - candidates.push(json!({ - "module": module, - "symbol": symbol, - "import_file": unresolved.file_path, - "line": user_line(unresolved.line), - })); - if candidates.len() >= limit.clamp(1, 20) { - break; - } - } - if candidates.is_empty() { - return Ok(None); - } - Ok(Some(json!({ - "message": "No indexed symbol matched, but project imports reference matching symbols from an ignored dependency. Keep node_modules ignored for normal sync; use bounded lazy dependency indexing for the listed module if this symbol is needed.", - "candidates": candidates, - "suggested_action": "lazy_index_ignored_dependency", - }))) -} - -fn parse_ignored_dependency_candidate(reference_name: &str) -> Option<(&str, &str)> { - reference_name.strip_prefix("npm:")?.split_once('#') -} - fn render_search_md(value: &Value) -> String { let items = if value.is_array() { value.as_array() @@ -205,24 +159,7 @@ fn render_search_md(value: &Value) -> String { { md.blank().heading(3, "Index Coverage Hint").line(msg); } - if let Some(hint) = value.get("ignored_dependency_hint") { - let msg = hint - .get("message") - .and_then(Value::as_str) - .unwrap_or("Matching ignored dependency candidates were found."); - md.blank().heading(3, "Ignored Dependency Hint").line(msg); - if let Some(candidates) = hint.get("candidates").and_then(Value::as_array) { - for candidate in candidates { - let module = render::field_str(candidate, "module"); - let symbol = render::field_str(candidate, "symbol"); - let file = render::field_str(candidate, "import_file"); - let line = render::field_i64(candidate, "line"); - md.bullet(&format!( - "`{module}` exports `{symbol}` referenced at {file}:{line}" - )); - } - } - } + dependency_hints::append_ignored_dependency_hint_md(&mut md, value); md.render() } diff --git a/src/mcp/tools/handlers/mod.rs b/src/mcp/tools/handlers/mod.rs index 2ae67889..ba3b4223 100644 --- a/src/mcp/tools/handlers/mod.rs +++ b/src/mcp/tools/handlers/mod.rs @@ -6,6 +6,7 @@ pub mod analysis; pub mod dashboard; +mod dependency_hints; pub mod edit; pub mod git; pub mod graph; diff --git a/tests/db_query_test.rs b/tests/db_query_test.rs index f4173b1b..883b670d 100644 --- a/tests/db_query_test.rs +++ b/tests/db_query_test.rs @@ -87,6 +87,34 @@ async fn assert_can_start_new_transaction(db: &Database) { .expect("test transaction rollback should succeed"); } +#[tokio::test] +async fn test_dependency_import_candidates_query_use_nodes_without_unresolved_refs() { + let db = setup_db().await; + let mut import_node = sample_node("dep-import", "pkg", "src/app.ts"); + import_node.kind = NodeKind::Use; + import_node.start_line = 4; + import_node.signature = Some("import type { Foo, Bar as Baz } from \"pkg\";".to_string()); + let mut relative_import = sample_node("relative-import", "./local", "src/app.ts"); + relative_import.kind = NodeKind::Use; + relative_import.start_line = 5; + relative_import.signature = Some("import type { Foo } from \"./local\";".to_string()); + + db.insert_nodes(&[import_node, relative_import]) + .await + .expect("insert_nodes failed"); + + let candidates = db + .dependency_import_candidates("Foo", 5) + .await + .expect("dependency_import_candidates failed"); + + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].module, "pkg"); + assert_eq!(candidates[0].symbol, "Foo"); + assert_eq!(candidates[0].import_file, "src/app.ts"); + assert_eq!(candidates[0].line, 4); +} + // ------------------------------------------------------------------------- // public db module exports // ------------------------------------------------------------------------- diff --git a/tests/typescript_extraction_test.rs b/tests/typescript_extraction_test.rs index fe53388f..a1546488 100644 --- a/tests/typescript_extraction_test.rs +++ b/tests/typescript_extraction_test.rs @@ -306,7 +306,7 @@ import * as path from 'path'; } #[test] -fn test_ts_import_type_records_ignored_dependency_candidates() { +fn test_ts_import_type_does_not_pollute_unresolved_refs_with_dependency_candidates() { let source = r#" import type { Foo, Bar as Baz } from "pkg"; import { localThing } from "./local"; @@ -321,19 +321,11 @@ import { localThing } from "./local"; .filter(|r| r.reference_kind == EdgeKind::Uses) .collect(); - assert!( - use_refs.iter().any(|r| r.reference_name == "npm:pkg#Foo"), - "expected npm dependency candidate for Foo, got {use_refs:#?}" - ); - assert!( - use_refs.iter().any(|r| r.reference_name == "npm:pkg#Bar"), - "expected npm dependency candidate to use exported name before alias, got {use_refs:#?}" - ); assert!( !use_refs .iter() - .any(|r| r.reference_name == "npm:./local#localThing"), - "relative project imports should not be dependency candidates" + .any(|r| r.reference_name.starts_with("npm:")), + "dependency candidates should not be encoded as unresolved refs: {use_refs:#?}" ); }