From e91baa32314bcbd59255a109351da0a7e87e8771 Mon Sep 17 00:00:00 2001 From: sunerpy Date: Thu, 20 Aug 2026 16:23:06 +0800 Subject: [PATCH] fix(resolve): bind imported members, cross-module Go calls and initializers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight independent surface defects, batched because each is small and none shares a design with another. An explicit Python import was ignored. `from pkg.mod import real_target` resolved to a same-named function in an unrelated module, and the `as aliased` form produced no edge at all — a false edge and a missing one from the same cause: the import's dotted source was never mapped to a path, and the fallback required an `is_exported` flag our Python extractor never sets, so the reference fell through to name matching. Go calls across module boundaries produced nothing, because a module was identified by path without knowing which directory it lived in. Modules are now located by walking each file's ancestors for a `go.mod`, and the longest matching module path wins, so `ex/abc` is not mistaken for a child of `ex/a`. A call in a declaration initializer was attributed to the file rather than the declarator, so `callers` answered with a filename. It now names the constant. A mutating command that retargets an ancestor index says so; previously `index .` inside a subdirectory rebuilt the parent silently. `callers`, `callees` and `impact` accept `--file` to disambiguate same-named definitions, and an unmatched filter is an error listing the defining files rather than an empty result that would read as "this symbol is dead". The explore note no longer states a call budget nothing enforces, which made agents stop early; it now says there is no limit. MCP instructions list the languages we actually support, generated from the language registry rather than copied prose, which is the one golden this change regenerates. The installer gains VS Code, Copilot CLI and JetBrains targets. --- README.md | 6 +- .../codegraph-cli/src/installer/registry.rs | 24 +- .../src/installer/targets/copilot.rs | 859 ++++++++++++++++++ .../src/installer/targets/mod.rs | 1 + crates/codegraph-cli/src/installer/types.rs | 6 + crates/codegraph-cli/src/main.rs | 239 ++++- .../tests/ancestor_retarget_warning.rs | 179 ++++ .../tests/file_disambiguation.rs | 248 +++++ crates/codegraph-cli/tests/installer.rs | 145 +++ crates/codegraph-extract/src/walker.rs | 115 ++- crates/codegraph-mcp/src/engine.rs | 73 +- crates/codegraph-mcp/src/instructions.rs | 139 ++- crates/codegraph-mcp/src/rmcp_handler.rs | 6 +- crates/codegraph-mcp/src/server.rs | 4 +- crates/codegraph-resolve/src/context.rs | 147 ++- .../codegraph-resolve/src/import_resolver.rs | 190 +++- .../codegraph-resolve/src/snapshot_context.rs | 7 + crates/codegraph-resolve/src/types.rs | 19 +- .../fixtures/tier6/go_multi_module/a/go.mod | 3 + .../tier6/go_multi_module/a/helper.go | 6 + .../fixtures/tier6/go_multi_module/b/go.mod | 7 + .../fixtures/tier6/go_multi_module/b/local.go | 6 + .../fixtures/tier6/go_multi_module/b/main.go | 8 + .../fixtures/tier6/py_from_import/app.py | 5 + .../fixtures/tier6/py_from_import/app2.py | 5 + .../tier6/py_from_import/pkg/__init__.py | 0 .../tier6/py_from_import/pkg/decoy.py | 2 + .../fixtures/tier6/py_from_import/pkg/mod.py | 2 + .../fixtures/tier6/ts_decl_initializer/d.ts | 10 + .../tests/tier6_resolution.rs | 347 +++++++ docs/cli.md | 65 +- docs/mcp.md | 3 +- reference/golden/mcp/initialize.json | 2 +- 33 files changed, 2809 insertions(+), 69 deletions(-) create mode 100644 crates/codegraph-cli/src/installer/targets/copilot.rs create mode 100644 crates/codegraph-cli/tests/ancestor_retarget_warning.rs create mode 100644 crates/codegraph-cli/tests/file_disambiguation.rs create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/go.mod create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/helper.go create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/go.mod create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/local.go create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/main.go create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app.py create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app2.py create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/__init__.py create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/decoy.py create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/mod.py create mode 100644 crates/codegraph-resolve/tests/fixtures/tier6/ts_decl_initializer/d.ts create mode 100644 crates/codegraph-resolve/tests/tier6_resolution.rs diff --git a/README.md b/README.md index c1b28473..3aea8db5 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,8 @@ codegraph serve --mcp --path /path/to/project # optional: pin to a specific pro ``` Auto-register it into your agent's config (Claude Code, Cursor, Codex CLI, -opencode, Hermes, Gemini CLI, Antigravity, Kiro, Trae, Qoder, Zed): +opencode, Hermes, Gemini CLI, Antigravity, Kiro, Trae, Qoder, Zed, VS Code, +Copilot CLI, JetBrains): ```bash codegraph install --yes # detects installed agents and wires them up @@ -217,7 +218,8 @@ directory or `rootUri`/`workspaceFolders`/`roots`. When it can't resolve one, See [`docs/mcp.md`](docs/mcp.md#project-resolution) for the full three-case breakdown. Supported agents: Claude Code, Cursor, Codex CLI, opencode, Hermes Agent, -Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed. +Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed, VS Code (GitHub Copilot), +GitHub Copilot CLI, JetBrains IDEs (GitHub Copilot). ```bash codegraph install --yes # auto-detect installed agents diff --git a/crates/codegraph-cli/src/installer/registry.rs b/crates/codegraph-cli/src/installer/registry.rs index f5ba9d89..e2b17172 100644 --- a/crates/codegraph-cli/src/installer/registry.rs +++ b/crates/codegraph-cli/src/installer/registry.rs @@ -4,14 +4,24 @@ //! Order is load-bearing: it is the order in `--target=all`, in detection, and //! in `--print-config`'s help listing. Matches `ALL_TARGETS` (registry.ts:20): //! claude, cursor, codex, opencode, hermes, gemini, antigravity, kiro, trae, -//! qoder. +//! qoder, zed, plus the three Copilot surfaces appended by this port: vscode, +//! copilot-cli, jetbrains. use anyhow::{Result, bail}; use super::targets::{ - antigravity::ANTIGRAVITY_TARGET, claude::CLAUDE_TARGET, codex::CODEX_TARGET, - cursor::CURSOR_TARGET, gemini::GEMINI_TARGET, hermes::HERMES_TARGET, kiro::KIRO_TARGET, - opencode::OPENCODE_TARGET, qoder::QODER_TARGET, trae::TRAE_TARGET, zed::ZED_TARGET, + antigravity::ANTIGRAVITY_TARGET, + claude::CLAUDE_TARGET, + codex::CODEX_TARGET, + copilot::{COPILOT_CLI_TARGET, JETBRAINS_TARGET, VSCODE_TARGET}, + cursor::CURSOR_TARGET, + gemini::GEMINI_TARGET, + hermes::HERMES_TARGET, + kiro::KIRO_TARGET, + opencode::OPENCODE_TARGET, + qoder::QODER_TARGET, + trae::TRAE_TARGET, + zed::ZED_TARGET, }; use super::types::{AgentTarget, DetectionResult, InstallContext, Location}; @@ -28,6 +38,9 @@ pub fn all_targets() -> Vec<&'static dyn AgentTarget> { &TRAE_TARGET, &QODER_TARGET, &ZED_TARGET, + &VSCODE_TARGET, + &COPILOT_CLI_TARGET, + &JETBRAINS_TARGET, ] } @@ -130,6 +143,9 @@ mod tests { "trae", "qoder", "zed", + "vscode", + "copilot-cli", + "jetbrains", ]; #[test] diff --git a/crates/codegraph-cli/src/installer/targets/copilot.rs b/crates/codegraph-cli/src/installer/targets/copilot.rs new file mode 100644 index 00000000..2dc1e5f2 --- /dev/null +++ b/crates/codegraph-cli/src/installer/targets/copilot.rs @@ -0,0 +1,859 @@ +//! Copilot install targets (upstream `490791c` + follow-up fixes): VS Code, +//! Copilot CLI, and JetBrains. +//! +//! All three are GitHub Copilot MCP surfaces but disagree on both the wrapper +//! key and the available locations, which is why they are three targets rather +//! than one parametrized target: +//! +//! | target | file | wrapper | locations | +//! | ----------- | --------------------------------------------- | ------------ | ------------ | +//! | vscode | `.vscode/mcp.json` (local) | `servers` | local+global | +//! | | `/Code/User/mcp.json` (global) | `servers` | | +//! | copilot-cli | `~/.copilot/mcp-config.json` | `mcpServers` | global only | +//! | jetbrains | `~/.config/github-copilot/intellij/mcp.json` | `servers` | global only | +//! +//! Two contracts are load-bearing and each has a dedicated test: +//! +//! * The VS Code GLOBAL entry must NOT use `${workspaceFolder}`. VS Code +//! substitutes that variable in a WORKSPACE `mcp.json` only; in the user-level +//! file it stays a literal, which would make the server point at a directory +//! named `${workspaceFolder}`. The global entry is therefore BARE (read-only +//! off any existing index, like the Kiro/Qoder global entries) and the LOCAL +//! entry pins an absolute `--path`. +//! * The Copilot CLI entry carries `"tools": ["*"]`. Without it the CLI +//! registers the server but exposes none of its tools. +//! +//! CLI-only and additive: no extraction, resolution, or golden surface is +//! touched. The upsert is JSONC-surgical and only ever writes the `codegraph` +//! key, preserving sibling servers, comments, and key order. + +use std::fs; +use std::path::{Path, PathBuf}; + +use serde_json::{Map, Value, json}; + +use super::super::shared::{ + ConfigRead, mcp_server_config, read_config_file, read_json_file, remove_nested_key_jsonc, + to_upstream_json, upsert_nested_key_jsonc, write_json_file, +}; +use super::super::types::{ + AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, + TargetId, WriteResult, +}; +use super::super::vscode_user::config_base_for; + +/// The VS Code user-level app dir hosting `User/mcp.json`. +const VSCODE_APP_DIR: &str = "Code"; + +const VSCODE_GLOBAL_WHY: &str = "The global VS Code entry is bare `serve --mcp` (read-only off any existing index); VS Code expands ${workspaceFolder} only in a WORKSPACE mcp.json, never in the user-level one."; +const VSCODE_GLOBAL_HOWTO: &str = "For LIVE auto-update run `codegraph init --target=vscode` in each project (writes .vscode/mcp.json with that project's absolute --path)."; + +/// `/Code/User/mcp.json`. +fn vscode_global_mcp_json(ctx: &InstallContext) -> PathBuf { + config_base_for( + &ctx.home, + ctx.app_data.as_deref(), + ctx.xdg_config_home.as_deref(), + std::env::consts::OS, + ) + .join(VSCODE_APP_DIR) + .join("User") + .join("mcp.json") +} + +/// `/.vscode/mcp.json`. +fn vscode_local_mcp_json(ctx: &InstallContext) -> PathBuf { + ctx.cwd.join(".vscode").join("mcp.json") +} + +/// `~/.copilot/mcp-config.json`. +fn copilot_cli_mcp_json(ctx: &InstallContext) -> PathBuf { + ctx.home.join(".copilot").join("mcp-config.json") +} + +/// `/github-copilot/intellij/mcp.json`. +/// +/// JetBrains' Copilot plugin uses this XDG-style path on every OS, so it does +/// NOT go through [`config_base_for`] (which would resolve to +/// `Library/Application Support` on macOS and `AppData/Roaming` on Windows). +fn jetbrains_mcp_json(ctx: &InstallContext) -> PathBuf { + ctx.xdg_config_home + .clone() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| ctx.home.join(".config")) + .join("github-copilot") + .join("intellij") + .join("mcp.json") +} + +/// The entry with an absolute `--path`, for a project-scoped config. +fn entry_with_project_path(ctx: &InstallContext) -> Value { + let mut base = mcp_server_config(); + if let Some(args) = base.get_mut("args").and_then(|a| a.as_array_mut()) { + args.push(json!("--path")); + args.push(json!(ctx.cwd.to_string_lossy().to_string())); + } + base +} + +/// The Copilot CLI entry: the shared stdio block plus `"tools": ["*"]`, without +/// which the CLI registers the server but exposes none of its tools. +fn copilot_cli_entry() -> Value { + let mut base = mcp_server_config(); + if let Value::Object(map) = &mut base { + map.insert("tools".to_string(), json!(["*"])); + } + base +} + +/// Detect via "the config file's wrapper already holds a codegraph key", plus +/// "the agent's own directory exists" for installed-ness. +fn detect_at(file: &Path, marker_dir: &Path, wrapper: &str) -> DetectionResult { + let already_configured = read_json_file(file) + .get(wrapper) + .and_then(|s| s.get("codegraph")) + .is_some(); + DetectionResult { + installed: marker_dir.exists() || file.exists(), + already_configured, + } +} + +/// JSONC-surgical upsert of `.codegraph = entry`, creating the parent +/// directory and the file when absent. +fn write_entry(file: &Path, wrapper: &str, entry: &Value) -> FileWrite { + if let Some(dir) = file.parent() { + let _ = fs::create_dir_all(dir); + } + match read_config_file(file) { + ConfigRead::Unparseable => FileWrite { + path: file.to_path_buf(), + action: FileAction::Skipped, + }, + ConfigRead::Missing => { + let mut config = Map::new(); + let mut servers = Map::new(); + servers.insert("codegraph".to_string(), entry.clone()); + config.insert(wrapper.to_string(), Value::Object(servers)); + let _ = write_json_file(file, &config); + FileWrite { + path: file.to_path_buf(), + action: FileAction::Created, + } + } + ConfigRead::Parsed(_) => { + let action = upsert_nested_key_jsonc(file, wrapper, "codegraph", entry, None) + .unwrap_or(FileAction::Skipped); + FileWrite { + path: file.to_path_buf(), + action, + } + } + } +} + +fn remove_entry(file: &Path, wrapper: &str) -> FileWrite { + let action = + remove_nested_key_jsonc(file, wrapper, "codegraph").unwrap_or(FileAction::NotFound); + FileWrite { + path: file.to_path_buf(), + action, + } +} + +fn print_snippet(file: &Path, wrapper: &str, entry: &Value, why: Option<&str>) -> String { + let snippet = to_upstream_json(&json!({ wrapper: { "codegraph": entry } })); + match why { + Some(why) => format!("# Add to {}\n# {why}\n\n{snippet}\n", file.display()), + None => format!("# Add to {}\n\n{snippet}\n", file.display()), + } +} + +// ============================ VS Code ======================================== + +pub struct VsCodeTarget; + +impl AgentTarget for VsCodeTarget { + fn id(&self) -> TargetId { + TargetId::VsCode + } + fn display_name(&self) -> &'static str { + "VS Code (GitHub Copilot)" + } + fn supports_location(&self, _loc: Location) -> bool { + true + } + + fn detect(&self, ctx: &InstallContext, loc: Location) -> DetectionResult { + match loc { + Location::Global => { + let file = vscode_global_mcp_json(ctx); + let marker = file.parent().map(Path::to_path_buf).unwrap_or_default(); + detect_at(&file, &marker, "servers") + } + Location::Local => { + let file = vscode_local_mcp_json(ctx); + detect_at(&file, &ctx.cwd.join(".vscode"), "servers") + } + } + } + + fn install(&self, ctx: &InstallContext, loc: Location, _opts: InstallOptions) -> WriteResult { + match loc { + Location::Global => WriteResult { + files: vec![write_entry( + &vscode_global_mcp_json(ctx), + "servers", + &mcp_server_config(), + )], + notes: vec![ + VSCODE_GLOBAL_WHY.to_string(), + VSCODE_GLOBAL_HOWTO.to_string(), + "Reload VS Code for MCP changes to take effect.".to_string(), + ], + }, + Location::Local => WriteResult { + files: vec![write_entry( + &vscode_local_mcp_json(ctx), + "servers", + &entry_with_project_path(ctx), + )], + notes: vec![ + format!( + "CodeGraph MCP configured for project {}.", + ctx.cwd.display() + ), + "Reload VS Code for MCP changes to take effect.".to_string(), + ], + }, + } + } + + fn uninstall(&self, ctx: &InstallContext, loc: Location) -> WriteResult { + let file = match loc { + Location::Global => vscode_global_mcp_json(ctx), + Location::Local => vscode_local_mcp_json(ctx), + }; + WriteResult { + files: vec![remove_entry(&file, "servers")], + notes: Vec::new(), + } + } + + fn print_config(&self, ctx: &InstallContext, loc: Location) -> String { + match loc { + Location::Global => print_snippet( + &vscode_global_mcp_json(ctx), + "servers", + &mcp_server_config(), + Some(VSCODE_GLOBAL_WHY), + ), + Location::Local => print_snippet( + &vscode_local_mcp_json(ctx), + "servers", + &entry_with_project_path(ctx), + None, + ), + } + } +} + +pub static VSCODE_TARGET: VsCodeTarget = VsCodeTarget; + +// ========================== Copilot CLI ====================================== + +pub struct CopilotCliTarget; + +impl AgentTarget for CopilotCliTarget { + fn id(&self) -> TargetId { + TargetId::CopilotCli + } + fn display_name(&self) -> &'static str { + "GitHub Copilot CLI" + } + fn supports_location(&self, loc: Location) -> bool { + loc == Location::Global + } + + fn detect(&self, ctx: &InstallContext, loc: Location) -> DetectionResult { + if loc != Location::Global { + return DetectionResult { + installed: false, + already_configured: false, + }; + } + detect_at( + &copilot_cli_mcp_json(ctx), + &ctx.home.join(".copilot"), + "mcpServers", + ) + } + + fn install(&self, ctx: &InstallContext, loc: Location, _opts: InstallOptions) -> WriteResult { + if loc != Location::Global { + return unsupported_local(self.display_name()); + } + WriteResult { + files: vec![write_entry( + &copilot_cli_mcp_json(ctx), + "mcpServers", + &copilot_cli_entry(), + )], + notes: vec![ + "GitHub Copilot CLI supports GLOBAL config only.".to_string(), + "The entry sets \"tools\": [\"*\"] — without it the CLI registers the server but exposes no tools.".to_string(), + ], + } + } + + fn uninstall(&self, ctx: &InstallContext, loc: Location) -> WriteResult { + if loc != Location::Global { + return unsupported_local(self.display_name()); + } + WriteResult { + files: vec![remove_entry(&copilot_cli_mcp_json(ctx), "mcpServers")], + notes: Vec::new(), + } + } + + fn print_config(&self, ctx: &InstallContext, _loc: Location) -> String { + print_snippet( + &copilot_cli_mcp_json(ctx), + "mcpServers", + &copilot_cli_entry(), + None, + ) + } +} + +pub static COPILOT_CLI_TARGET: CopilotCliTarget = CopilotCliTarget; + +// =========================== JetBrains ======================================= + +pub struct JetBrainsTarget; + +impl AgentTarget for JetBrainsTarget { + fn id(&self) -> TargetId { + TargetId::JetBrains + } + fn display_name(&self) -> &'static str { + "JetBrains IDEs (GitHub Copilot)" + } + fn supports_location(&self, loc: Location) -> bool { + loc == Location::Global + } + + fn detect(&self, ctx: &InstallContext, loc: Location) -> DetectionResult { + if loc != Location::Global { + return DetectionResult { + installed: false, + already_configured: false, + }; + } + let file = jetbrains_mcp_json(ctx); + let marker = file.parent().map(Path::to_path_buf).unwrap_or_default(); + detect_at(&file, &marker, "servers") + } + + fn install(&self, ctx: &InstallContext, loc: Location, _opts: InstallOptions) -> WriteResult { + if loc != Location::Global { + return unsupported_local(self.display_name()); + } + WriteResult { + files: vec![write_entry( + &jetbrains_mcp_json(ctx), + "servers", + &mcp_server_config(), + )], + notes: vec![ + "JetBrains Copilot MCP config is GLOBAL only.".to_string(), + "Restart the IDE for MCP changes to take effect.".to_string(), + ], + } + } + + fn uninstall(&self, ctx: &InstallContext, loc: Location) -> WriteResult { + if loc != Location::Global { + return unsupported_local(self.display_name()); + } + WriteResult { + files: vec![remove_entry(&jetbrains_mcp_json(ctx), "servers")], + notes: Vec::new(), + } + } + + fn print_config(&self, ctx: &InstallContext, _loc: Location) -> String { + print_snippet( + &jetbrains_mcp_json(ctx), + "servers", + &mcp_server_config(), + None, + ) + } +} + +pub static JETBRAINS_TARGET: JetBrainsTarget = JetBrainsTarget; + +/// The no-file result for a `--local` request on a global-only target: a note, +/// no error, and nothing written. +fn unsupported_local(display_name: &str) -> WriteResult { + WriteResult { + files: Vec::new(), + notes: vec![format!( + "{display_name} supports --global only; nothing written." + )], + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A temp-rooted context. Sets BOTH `app_data` and `xdg_config_home` under + /// the temp home so `config_base_for` resolves inside it on every OS. + fn temp_ctx(label: &str) -> (InstallContext, PathBuf) { + let base = std::env::temp_dir().join(format!( + "cg-copilot-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let home = base.join("home"); + let ctx = InstallContext { + home: home.clone(), + cwd: base.join("cwd"), + app_data: Some(home.join("AppData").join("Roaming")), + xdg_config_home: Some(home.join(".config")), + hermes_home: None, + }; + (ctx, base) + } + + fn opts() -> InstallOptions { + InstallOptions { + auto_allow: true, + front_load_hook: false, + } + } + + fn args_of(config: &Map, wrapper: &str) -> Vec { + config[wrapper]["codegraph"]["args"] + .as_array() + .expect("codegraph args") + .clone() + } + + // ---------------- VS Code ------------------------------------------------- + + #[test] + fn vscode_global_entry_is_bare_and_never_uses_workspace_folder() { + // The load-bearing contract: VS Code expands ${workspaceFolder} only in a + // WORKSPACE mcp.json, so a global entry using it would point the server at + // a directory literally named "${workspaceFolder}". + let (ctx, base) = temp_ctx("vscode-global-bare"); + let result = VsCodeTarget.install(&ctx, Location::Global, opts()); + assert_eq!(result.files[0].action, FileAction::Created); + let file = vscode_global_mcp_json(&ctx); + let config = read_json_file(&file); + assert_eq!( + args_of(&config, "servers"), + vec![json!("serve"), json!("--mcp")], + "global entry must be bare" + ); + let raw = fs::read_to_string(&file).unwrap(); + assert!( + !raw.contains("${workspaceFolder}"), + "global config must not contain ${{workspaceFolder}}: {raw}" + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_global_path_is_code_user_mcp_json() { + let (ctx, base) = temp_ctx("vscode-global-path"); + let file = vscode_global_mcp_json(&ctx); + assert!( + file.ends_with(PathBuf::from("Code").join("User").join("mcp.json")), + "got {}", + file.display() + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_local_writes_dot_vscode_with_absolute_path() { + let (ctx, base) = temp_ctx("vscode-local"); + let result = VsCodeTarget.install(&ctx, Location::Local, opts()); + assert_eq!(result.files[0].action, FileAction::Created); + let file = vscode_local_mcp_json(&ctx); + assert!( + file.ends_with(PathBuf::from(".vscode").join("mcp.json")), + "got {}", + file.display() + ); + let args = args_of(&read_json_file(&file), "servers"); + assert_eq!( + args, + vec![ + json!("serve"), + json!("--mcp"), + json!("--path"), + json!(ctx.cwd.to_string_lossy().to_string()), + ] + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_uses_the_servers_wrapper_not_mcp_servers() { + let (ctx, base) = temp_ctx("vscode-wrapper"); + VsCodeTarget.install(&ctx, Location::Local, opts()); + let config = read_json_file(&vscode_local_mcp_json(&ctx)); + assert!(config.contains_key("servers"), "must use `servers`"); + assert!( + !config.contains_key("mcpServers"), + "must NOT use `mcpServers`: {config:?}" + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_install_preserves_sibling_servers() { + let (ctx, base) = temp_ctx("vscode-siblings"); + let file = vscode_local_mcp_json(&ctx); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write( + &file, + "{\n \"servers\": {\n \"github\": { \"command\": \"gh-mcp\" }\n }\n}\n", + ) + .unwrap(); + let result = VsCodeTarget.install(&ctx, Location::Local, opts()); + assert_eq!(result.files[0].action, FileAction::Updated); + let config = read_json_file(&file); + assert_eq!(config["servers"]["github"]["command"], json!("gh-mcp")); + assert!(config["servers"].get("codegraph").is_some()); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_install_is_idempotent() { + let (ctx, base) = temp_ctx("vscode-idempotent"); + let file = vscode_local_mcp_json(&ctx); + VsCodeTarget.install(&ctx, Location::Local, opts()); + let first = fs::read_to_string(&file).unwrap(); + let second = VsCodeTarget.install(&ctx, Location::Local, opts()); + assert_eq!(second.files[0].action, FileAction::Unchanged); + assert_eq!(fs::read_to_string(&file).unwrap(), first); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_uninstall_removes_only_codegraph() { + let (ctx, base) = temp_ctx("vscode-uninstall"); + let file = vscode_local_mcp_json(&ctx); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write( + &file, + "{\n \"servers\": {\n \"github\": { \"command\": \"gh-mcp\" },\n \"codegraph\": { \"command\": \"codegraph\" }\n }\n}\n", + ) + .unwrap(); + let result = VsCodeTarget.uninstall(&ctx, Location::Local); + assert_eq!(result.files[0].action, FileAction::Removed); + let config = read_json_file(&file); + assert!(config["servers"].get("github").is_some()); + assert!(config["servers"].get("codegraph").is_none()); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_uninstall_reports_not_found_when_absent() { + let (ctx, base) = temp_ctx("vscode-uninstall-absent"); + let result = VsCodeTarget.uninstall(&ctx, Location::Global); + assert_eq!(result.files[0].action, FileAction::NotFound); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_skips_unparseable_config() { + let (ctx, base) = temp_ctx("vscode-corrupt"); + let file = vscode_local_mcp_json(&ctx); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + let corrupt = "{ not json"; + fs::write(&file, corrupt).unwrap(); + let result = VsCodeTarget.install(&ctx, Location::Local, opts()); + assert_eq!(result.files[0].action, FileAction::Skipped); + assert_eq!(fs::read_to_string(&file).unwrap(), corrupt); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_detect_reflects_configuration() { + let (ctx, base) = temp_ctx("vscode-detect"); + assert!(!VsCodeTarget.detect(&ctx, Location::Local).installed); + VsCodeTarget.install(&ctx, Location::Local, opts()); + let after = VsCodeTarget.detect(&ctx, Location::Local); + assert!(after.installed); + assert!(after.already_configured); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn vscode_print_config_global_warns_about_workspace_folder() { + let (ctx, base) = temp_ctx("vscode-print"); + let out = VsCodeTarget.print_config(&ctx, Location::Global); + assert!(out.contains("servers")); + assert!( + out.contains("${workspaceFolder}"), + "the note must EXPLAIN the omission: {out}" + ); + let entry_line = out + .lines() + .find(|l| l.contains("\"args\"")) + .unwrap_or_default(); + assert!( + !entry_line.contains("${workspaceFolder}"), + "the ENTRY itself must not use it: {entry_line}" + ); + let local = VsCodeTarget.print_config(&ctx, Location::Local); + assert!(local.replace('\\', "/").contains(".vscode/mcp.json")); + assert!(local.contains("--path")); + let _ = fs::remove_dir_all(base); + } + + // ---------------- Copilot CLI -------------------------------------------- + + #[test] + fn copilot_cli_entry_declares_all_tools() { + // Without `"tools": ["*"]` the CLI registers the server but exposes none + // of its tools. + let (ctx, base) = temp_ctx("cli-tools"); + CopilotCliTarget.install(&ctx, Location::Global, opts()); + let config = read_json_file(&copilot_cli_mcp_json(&ctx)); + assert_eq!( + config["mcpServers"]["codegraph"]["tools"], + json!(["*"]), + "the CLI entry must declare tools: [\"*\"]" + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn copilot_cli_path_and_wrapper() { + let (ctx, base) = temp_ctx("cli-path"); + let file = copilot_cli_mcp_json(&ctx); + assert!( + file.ends_with(PathBuf::from(".copilot").join("mcp-config.json")), + "got {}", + file.display() + ); + CopilotCliTarget.install(&ctx, Location::Global, opts()); + let config = read_json_file(&file); + assert!(config.contains_key("mcpServers"), "must use `mcpServers`"); + assert!(!config.contains_key("servers")); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn copilot_cli_is_global_only() { + assert!(CopilotCliTarget.supports_location(Location::Global)); + assert!(!CopilotCliTarget.supports_location(Location::Local)); + } + + #[test] + fn copilot_cli_local_install_writes_nothing() { + let (ctx, base) = temp_ctx("cli-local"); + let result = CopilotCliTarget.install(&ctx, Location::Local, opts()); + assert!(result.files.is_empty(), "no file may be written"); + assert!( + result.notes.iter().any(|n| n.contains("--global only")), + "must explain: {:?}", + result.notes + ); + assert!( + !copilot_cli_mcp_json(&ctx).exists(), + "must not create the global config from a --local request" + ); + let local_result = CopilotCliTarget.uninstall(&ctx, Location::Local); + assert!(local_result.files.is_empty()); + let detect = CopilotCliTarget.detect(&ctx, Location::Local); + assert!(!detect.installed && !detect.already_configured); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn copilot_cli_uninstall_removes_only_codegraph() { + let (ctx, base) = temp_ctx("cli-uninstall"); + let file = copilot_cli_mcp_json(&ctx); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write( + &file, + "{\n \"mcpServers\": {\n \"other\": { \"command\": \"x\" },\n \"codegraph\": { \"command\": \"codegraph\" }\n }\n}\n", + ) + .unwrap(); + assert_eq!( + CopilotCliTarget.uninstall(&ctx, Location::Global).files[0].action, + FileAction::Removed + ); + let config = read_json_file(&file); + assert!(config["mcpServers"].get("other").is_some()); + assert!(config["mcpServers"].get("codegraph").is_none()); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn copilot_cli_print_config_includes_tools() { + let (ctx, base) = temp_ctx("cli-print"); + let out = CopilotCliTarget.print_config(&ctx, Location::Global); + assert!(out.contains("mcpServers")); + assert!(out.contains("\"tools\"")); + let _ = fs::remove_dir_all(base); + } + + // ---------------- JetBrains ---------------------------------------------- + + #[test] + fn jetbrains_path_is_xdg_github_copilot_intellij() { + let (ctx, base) = temp_ctx("jb-path"); + let file = jetbrains_mcp_json(&ctx); + assert!( + file.ends_with( + PathBuf::from("github-copilot") + .join("intellij") + .join("mcp.json") + ), + "got {}", + file.display() + ); + assert!( + file.starts_with(ctx.xdg_config_home.as_ref().unwrap()), + "must sit under xdg_config_home on every OS, got {}", + file.display() + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn jetbrains_falls_back_to_dot_config_when_xdg_unset() { + let (mut ctx, base) = temp_ctx("jb-noxdg"); + ctx.xdg_config_home = None; + let file = jetbrains_mcp_json(&ctx); + assert!( + file.starts_with(ctx.home.join(".config")), + "got {}", + file.display() + ); + ctx.xdg_config_home = Some(PathBuf::new()); + assert!( + jetbrains_mcp_json(&ctx).starts_with(ctx.home.join(".config")), + "an EMPTY xdg value must fall back too" + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn jetbrains_uses_servers_wrapper_and_is_global_only() { + let (ctx, base) = temp_ctx("jb-wrapper"); + assert!(JetBrainsTarget.supports_location(Location::Global)); + assert!(!JetBrainsTarget.supports_location(Location::Local)); + JetBrainsTarget.install(&ctx, Location::Global, opts()); + let config = read_json_file(&jetbrains_mcp_json(&ctx)); + assert!(config.contains_key("servers"), "must use `servers`"); + assert!(!config.contains_key("mcpServers")); + assert_eq!( + args_of(&config, "servers"), + vec![json!("serve"), json!("--mcp")], + "global-only target writes a bare entry" + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn jetbrains_local_install_writes_nothing() { + let (ctx, base) = temp_ctx("jb-local"); + let result = JetBrainsTarget.install(&ctx, Location::Local, opts()); + assert!(result.files.is_empty()); + assert!(!jetbrains_mcp_json(&ctx).exists()); + assert!( + JetBrainsTarget + .uninstall(&ctx, Location::Local) + .files + .is_empty() + ); + let detect = JetBrainsTarget.detect(&ctx, Location::Local); + assert!(!detect.installed); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn jetbrains_uninstall_and_detect_roundtrip() { + let (ctx, base) = temp_ctx("jb-roundtrip"); + assert!(!JetBrainsTarget.detect(&ctx, Location::Global).installed); + JetBrainsTarget.install(&ctx, Location::Global, opts()); + let after = JetBrainsTarget.detect(&ctx, Location::Global); + assert!(after.installed && after.already_configured); + assert_eq!( + JetBrainsTarget.uninstall(&ctx, Location::Global).files[0].action, + FileAction::Removed + ); + assert!( + !JetBrainsTarget + .detect(&ctx, Location::Global) + .already_configured + ); + let _ = fs::remove_dir_all(base); + } + + #[test] + fn jetbrains_print_config_names_the_file() { + let (ctx, base) = temp_ctx("jb-print"); + let out = JetBrainsTarget.print_config(&ctx, Location::Global); + assert!( + out.replace('\\', "/") + .contains("github-copilot/intellij/mcp.json") + ); + assert!(out.contains("servers")); + let _ = fs::remove_dir_all(base); + } + + // ---------------- Cross-target ------------------------------------------- + + #[test] + fn no_copilot_target_declares_skill_support() { + // None of the three has a documented skill directory, so all three keep + // the trait's unsupported default rather than guessing a path. + for (name, supports) in [ + ("vscode", VsCodeTarget.supports_skills(Location::Global)), + ( + "copilot-cli", + CopilotCliTarget.supports_skills(Location::Global), + ), + ( + "jetbrains", + JetBrainsTarget.supports_skills(Location::Global), + ), + ] { + assert!(!supports, "{name} must not claim skill support"); + } + } + + #[test] + fn the_three_targets_write_three_distinct_files() { + let (ctx, base) = temp_ctx("distinct"); + VsCodeTarget.install(&ctx, Location::Global, opts()); + CopilotCliTarget.install(&ctx, Location::Global, opts()); + JetBrainsTarget.install(&ctx, Location::Global, opts()); + let files = [ + vscode_global_mcp_json(&ctx), + copilot_cli_mcp_json(&ctx), + jetbrains_mcp_json(&ctx), + ]; + for file in &files { + assert!(file.is_file(), "missing {}", file.display()); + } + let unique: std::collections::BTreeSet<_> = files.iter().collect(); + assert_eq!(unique.len(), 3, "the three targets must not share a file"); + let _ = fs::remove_dir_all(base); + } +} diff --git a/crates/codegraph-cli/src/installer/targets/mod.rs b/crates/codegraph-cli/src/installer/targets/mod.rs index fd76d9c0..0194a1d7 100644 --- a/crates/codegraph-cli/src/installer/targets/mod.rs +++ b/crates/codegraph-cli/src/installer/targets/mod.rs @@ -3,6 +3,7 @@ pub mod antigravity; pub mod claude; pub mod codex; +pub mod copilot; pub mod cursor; pub mod gemini; pub mod hermes; diff --git a/crates/codegraph-cli/src/installer/types.rs b/crates/codegraph-cli/src/installer/types.rs index 03112d37..bc06c70f 100644 --- a/crates/codegraph-cli/src/installer/types.rs +++ b/crates/codegraph-cli/src/installer/types.rs @@ -40,6 +40,9 @@ pub enum TargetId { Trae, Qoder, Zed, + VsCode, + CopilotCli, + JetBrains, } impl TargetId { @@ -56,6 +59,9 @@ impl TargetId { TargetId::Trae => "trae", TargetId::Qoder => "qoder", TargetId::Zed => "zed", + TargetId::VsCode => "vscode", + TargetId::CopilotCli => "copilot-cli", + TargetId::JetBrains => "jetbrains", } } } diff --git a/crates/codegraph-cli/src/main.rs b/crates/codegraph-cli/src/main.rs index e1e04795..09e1d085 100644 --- a/crates/codegraph-cli/src/main.rs +++ b/crates/codegraph-cli/src/main.rs @@ -319,6 +319,10 @@ enum Command { /// Exit non-zero when no callers are found (default exits 0). #[arg(long)] strict: bool, + /// Disambiguate same-named definitions: keep only the one defined in this + /// file (exact project-relative path or a trailing path suffix). + #[arg(long, value_name = "FILE")] + file: Option, }, // Upstream flags/output shape: upstream bin/codegraph.ts:1280-1284, 1298-1345. Callees { @@ -332,6 +336,10 @@ enum Command { /// Exit non-zero when no callees are found (default exits 0). #[arg(long)] strict: bool, + /// Disambiguate same-named definitions: keep only the one defined in this + /// file (exact project-relative path or a trailing path suffix). + #[arg(long, value_name = "FILE")] + file: Option, }, // Upstream flags/output shape: upstream bin/codegraph.ts:1358-1362, 1374-1439. Impact { @@ -345,6 +353,10 @@ enum Command { /// Exit non-zero when the symbol is not found (default exits 0). #[arg(long)] strict: bool, + /// Disambiguate same-named definitions: keep only the one defined in this + /// file (exact project-relative path or a trailing path suffix). + #[arg(long, value_name = "FILE")] + file: Option, }, // Upstream flags/output shape: upstream bin/codegraph.ts:1462-1469, 1479-1582. Affected { @@ -650,21 +662,24 @@ fn run(cli: Cli) -> Result<()> { limit, json, strict, - } => cmd_callers(symbol, path, limit, json, strict), + file, + } => cmd_callers(symbol, path, limit, json, strict, file), Command::Callees { symbol, path, limit, json, strict, - } => cmd_callees(symbol, path, limit, json, strict), + file, + } => cmd_callees(symbol, path, limit, json, strict, file), Command::Impact { symbol, path, depth, json, strict, - } => cmd_impact(symbol, path, depth, json, strict), + file, + } => cmd_impact(symbol, path, depth, json, strict, file), Command::Affected { files, path, @@ -3788,6 +3803,7 @@ fn cmd_unlock(path: Option) -> Result<()> { let start = absolute_path(path.unwrap_or_else(|| PathBuf::from("."))); index_paths(&start)?; let project = resolve_project_path_optional(&start); + warn_ancestor_index_retarget(&start, &project); let paths = index_paths(&project)?; let daemon_lock = codegraph_daemon::daemon_pid_path(&project)?; let daemon_removed = daemon_lock.exists() && codegraph_daemon::unlock_project(&project); @@ -3839,19 +3855,32 @@ fn cmd_callers( limit: usize, json_output: bool, strict: bool, + file: Option, ) -> Result<()> { let project = resolve_required_project(path)?; let store = open_store(&project)?; - let nodes = related_nodes_for_symbol(&store, &project, &symbol, limit, Related::Callers)?; + let nodes = related_nodes_for_symbol( + &store, + &project, + &symbol, + limit, + Related::Callers, + file.as_deref(), + )?; let godot = godot_honesty_for_symbol(&store, &project, &symbol)?; if json_output { print_json_pretty(&json!({ "symbol": symbol, + "file": file, "callers": nodes, "godotDynamic": godot.as_json(), }))?; } else { - print_related("Callers", &symbol, &nodes); + print_related( + "Callers", + &describe_symbol(&symbol, file.as_deref()), + &nodes, + ); godot.print_cli(nodes.is_empty()); } if strict && nodes.is_empty() { @@ -3866,14 +3895,26 @@ fn cmd_callees( limit: usize, json_output: bool, strict: bool, + file: Option, ) -> Result<()> { let project = resolve_required_project(path)?; let store = open_store(&project)?; - let nodes = related_nodes_for_symbol(&store, &project, &symbol, limit, Related::Callees)?; + let nodes = related_nodes_for_symbol( + &store, + &project, + &symbol, + limit, + Related::Callees, + file.as_deref(), + )?; if json_output { - print_json_pretty(&json!({ "symbol": symbol, "callees": nodes }))?; + print_json_pretty(&json!({ "symbol": symbol, "file": file, "callees": nodes }))?; } else { - print_related("Callees", &symbol, &nodes); + print_related( + "Callees", + &describe_symbol(&symbol, file.as_deref()), + &nodes, + ); } if strict && nodes.is_empty() { bail!("codegraph callees: no callees found for \"{symbol}\""); @@ -3902,6 +3943,7 @@ fn cmd_impact( depth: usize, json_output: bool, strict: bool, + file: Option, ) -> Result<()> { let project = resolve_required_project(path)?; let store = open_store(&project)?; @@ -3919,6 +3961,7 @@ fn cmd_impact( if exact_matches.is_empty() { bail!(lookup_symbol_not_found_message(&symbol)); } + let exact_matches = filter_matches_by_file(exact_matches, &symbol, file.as_deref())?; let traverser = GraphTraverser::new(&store); let mut nodes = HashMap::new(); let mut edge_keys = HashSet::new(); @@ -3963,6 +4006,7 @@ fn cmd_impact( if json_output { print_json_pretty(&json!({ "symbol": symbol, + "file": file, "depth": depth, "nodeCount": affected.len(), "edgeCount": edge_keys.len() + resource_edge_count, @@ -3973,7 +4017,7 @@ fn cmd_impact( } else { println!( "\nImpact of changing \"{}\" - {} affected symbols:\n", - symbol, + describe_symbol(&symbol, file.as_deref()), affected.len() ); print_by_file(&affected); @@ -5196,12 +5240,14 @@ fn related_nodes_for_symbol( symbol: &str, limit: usize, related: Related, + file: Option<&str>, ) -> Result> { let matches = symbol_matches(store, project, symbol)?; let exact_matches = exact_or_top_matches(&matches, symbol); if exact_matches.is_empty() { bail!(lookup_symbol_not_found_message(symbol)); } + let exact_matches = filter_matches_by_file(exact_matches, symbol, file)?; let traverser = GraphTraverser::new(store); let mut seen = HashSet::new(); let mut out = Vec::new(); @@ -5418,6 +5464,60 @@ fn exact_or_top_matches<'a>(matches: &'a [Node], symbol: &str) -> Vec<&'a Node> .collect() } +/// Narrow same-named definitions to the one declared in `file` (#1512). +/// +/// `None` passes every match through, so the unfiltered output of `callers` / +/// `callees` / `impact` is byte-unchanged. A filter is matched against the whole +/// project-relative path OR any trailing path segment boundary, so both +/// `src/deep/mod.ts` and `deep/mod.ts` select the same definition. +/// +/// An unmatched filter is an ERROR listing the files that DO define the symbol. +/// Returning an empty relative-set instead would render as "no callers", which +/// reads as "this symbol is dead" — the exact false negative the flag exists to +/// prevent. +fn filter_matches_by_file<'a>( + matches: Vec<&'a Node>, + symbol: &str, + file: Option<&str>, +) -> Result> { + let Some(want) = file else { + return Ok(matches); + }; + let want = want.replace('\\', "/"); + let want = want.trim_start_matches("./"); + let kept: Vec<&'a Node> = matches + .iter() + .copied() + .filter(|node| path_matches_file_filter(&node.file_path, want)) + .collect(); + if kept.is_empty() { + let mut defined_in: Vec<&str> = matches.iter().map(|n| n.file_path.as_str()).collect(); + defined_in.sort_unstable(); + defined_in.dedup(); + bail!( + "no definition of \"{symbol}\" in \"{want}\"; it is defined in: {}", + defined_in.join(", ") + ); + } + Ok(kept) +} + +/// A file filter matches the whole path or a trailing SEGMENT-aligned suffix, so +/// `other.ts` never selects `my_other.ts`. +fn path_matches_file_filter(file_path: &str, want: &str) -> bool { + let normalized = file_path.replace('\\', "/"); + normalized == want || normalized.ends_with(&format!("/{want}")) +} + +/// Render the queried symbol for human output, naming the applied `--file` +/// filter so a narrowed list is never mistaken for the symbol's full set. +fn describe_symbol(symbol: &str, file: Option<&str>) -> String { + match file { + Some(file) => format!("{symbol}\" in \"{file}"), + None => symbol.to_string(), + } +} + fn lookup_symbol_not_found_message(symbol: &str) -> String { format!( "Symbol \"{symbol}\" not found. Run `codegraph query {symbol}` to search for fuzzy matches." @@ -5634,6 +5734,7 @@ fn resolve_required_rebuild_project(path: Option) -> Result { break; } if has_rebuild_namespace(parent) { + warn_ancestor_index_retarget(&start, parent); return Ok(parent.to_path_buf()); } current = parent; @@ -5641,6 +5742,36 @@ fn resolve_required_rebuild_project(path: Option) -> Result { bail!("CodeGraph not initialized in {}", start.display()) } +/// Announce that a MUTATING command is about to operate on an ANCESTOR's index +/// rather than the directory the user named (#1524). +/// +/// `index .` inside an unindexed `parent/child` rebuilt the PARENT and printed +/// only `Indexed N files` — a count including the parent's own files — while +/// creating no child index. The retarget is intended behaviour (it is how one +/// index serves a whole tree), but performing it silently is what made the +/// outcome unreadable, and for `uninit --force` it deletes an index the user +/// never named. +/// +/// STDERR only: these commands' stdout is machine-readable and pinned by +/// `stdout_purity.rs`. Callers that already resolved the project themselves — +/// [`cmd_unlock`] — call this directly. +fn warn_ancestor_index_retarget(requested: &Path, resolved: &Path) { + if requested == resolved { + return; + } + eprintln!( + "Warning: {} has no CodeGraph index, so this command resolved to an ancestor index at {} \ + and will operate on THAT project, not on {}.", + requested.display(), + resolved.display(), + requested.display() + ); + eprintln!( + " Run `codegraph init {}` first if you meant to give it its own index.", + requested.display() + ); +} + fn resolve_project_path_optional(start: &Path) -> PathBuf { if has_lifecycle_namespace(start) { return start.to_path_buf(); @@ -6436,6 +6567,85 @@ mod pure_helper_tests { assert_eq!(picked.len(), 2); } + fn node_in(name: &str, file: &str) -> Node { + let mut n = node(name); + n.file_path = file.to_string(); + n + } + + #[test] + fn file_filter_matches_whole_path_and_segment_aligned_suffix() { + assert!(path_matches_file_filter( + "src/deep/mod.ts", + "src/deep/mod.ts" + )); + assert!(path_matches_file_filter("src/deep/mod.ts", "deep/mod.ts")); + assert!(path_matches_file_filter("src/deep/mod.ts", "mod.ts")); + } + + #[test] + fn file_filter_rejects_a_suffix_that_splits_a_segment() { + // `other.ts` must not select `my_other.ts` — a plain `ends_with` would. + assert!(!path_matches_file_filter("src/my_other.ts", "other.ts")); + assert!(!path_matches_file_filter("src/deep/mod.ts", "eep/mod.ts")); + } + + #[test] + fn filter_matches_by_file_none_passes_everything_through() { + let alpha = node_in("target", "alpha.ts"); + let beta = node_in("target", "beta.ts"); + let matches = vec![&alpha, &beta]; + let kept = filter_matches_by_file(matches, "target", None).expect("no filter"); + assert_eq!(kept.len(), 2, "unfiltered behaviour must be unchanged"); + } + + #[test] + fn filter_matches_by_file_selects_one_definition() { + let alpha = node_in("target", "alpha.ts"); + let beta = node_in("target", "beta.ts"); + let kept = + filter_matches_by_file(vec![&alpha, &beta], "target", Some("beta.ts")).expect("filter"); + assert_eq!( + kept.iter() + .map(|n| n.file_path.as_str()) + .collect::>(), + vec!["beta.ts"] + ); + } + + #[test] + fn filter_matches_by_file_unmatched_errors_and_lists_the_real_files() { + let alpha = node_in("target", "alpha.ts"); + let beta = node_in("target", "beta.ts"); + let err = filter_matches_by_file(vec![&alpha, &beta], "target", Some("nope.ts")) + .expect_err("an unmatched filter must error, not return empty"); + let text = err.to_string(); + assert!(text.contains("nope.ts"), "must name the filter: {text}"); + assert!( + text.contains("alpha.ts") && text.contains("beta.ts"), + "must list the defining files: {text}" + ); + } + + #[test] + fn filter_matches_by_file_normalizes_windows_separators_and_dot_slash() { + let deep = node_in("target", "src/deep/mod.ts"); + for want in ["src\\deep\\mod.ts", "./src/deep/mod.ts"] { + let kept = + filter_matches_by_file(vec![&deep], "target", Some(want)).expect("normalized"); + assert_eq!(kept.len(), 1, "{want} must select the definition"); + } + } + + #[test] + fn describe_symbol_names_the_applied_filter() { + assert_eq!(describe_symbol("target", None), "target"); + assert_eq!( + describe_symbol("target", Some("alpha.ts")), + "target\" in \"alpha.ts" + ); + } + #[test] fn exact_or_top_matches_refuses_fuzzy_only_matches() { let matches = vec![node("alpha"), node("beta")]; @@ -6604,6 +6814,17 @@ mod pure_helper_tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn ancestor_retarget_warning_is_a_no_op_when_nothing_was_retargeted() { + // The common case is `requested == resolved`; warning there would print + // on every ordinary run. Exercised for its no-panic/no-emit contract — + // the emitting branch is covered end-to-end by + // `tests/ancestor_retarget_warning.rs`, which can actually capture + // stderr. + let same = PathBuf::from("/proj"); + warn_ancestor_index_retarget(&same, &same); + } + #[test] fn normalize_lexical_leading_parentdir_is_preserved() { assert_eq!( diff --git a/crates/codegraph-cli/tests/ancestor_retarget_warning.rs b/crates/codegraph-cli/tests/ancestor_retarget_warning.rs new file mode 100644 index 00000000..978d5b34 --- /dev/null +++ b/crates/codegraph-cli/tests/ancestor_retarget_warning.rs @@ -0,0 +1,179 @@ +//! A mutating command must not silently retarget an ANCESTOR index (#1524). +//! +//! `index .` inside `parent/child`, with only `parent/` indexed, walks up and +//! rebuilds the PARENT — reporting a file count that includes the parent's own +//! files while creating no child index, and printing nothing about it. Upstream's +//! thread widens the diagnostic to `sync` / `uninit` / `unlock`. +//! +//! Each test drives the REAL binary, because the defect is in the CLI's project +//! resolution, and asserts on STDERR: a mutating command's stdout is parsed by +//! scripts, and `stdout_purity.rs` pins that contract. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +struct TestDir { + path: PathBuf, +} + +impl TestDir { + fn new(label: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "codegraph-cli-ancestor-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&path).unwrap(); + Self { path } + } + + fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.path); + } +} + +/// Run the binary with `cwd` as its working directory, so a bare `.` argument +/// exercises the same resolution an interactive user hits. +fn cli_in(cwd: &Path, args: &[&str]) -> (String, String, bool) { + let output = Command::new(env!("CARGO_BIN_EXE_codegraph")) + .args(args) + .current_dir(cwd) + .env("CODEGRAPH_NO_DAEMON", "1") + .env("CODEGRAPH_NO_WATCH", "1") + .output() + .expect("run codegraph binary"); + ( + String::from_utf8_lossy(&output.stdout).into_owned(), + String::from_utf8_lossy(&output.stderr).into_owned(), + output.status.success(), + ) +} + +/// `parent/` indexed, `parent/child/` NOT indexed — the #1524 shape. +fn parent_indexed_child_not(dir: &TestDir) -> (PathBuf, PathBuf) { + let parent = dir.path().join("parent"); + let child = parent.join("child"); + fs::create_dir_all(&child).unwrap(); + fs::write(parent.join("parent_file.ts"), "export const p = 1;\n").unwrap(); + fs::write(child.join("child_file.ts"), "export const c = 2;\n").unwrap(); + let (out, err, ok) = cli_in(dir.path(), &["init", parent.to_str().unwrap()]); + assert!(ok, "init parent failed: stdout={out} stderr={err}"); + (parent, child) +} + +/// The parent path is a PREFIX of the child path, and an unrelated warning (the +/// stdio-MCP index-holder pre-warning) already prints the resolved project — so +/// asserting "stderr mentions the parent" would pass without any retarget +/// diagnostic at all. The assertion therefore isolates the warning's own LINE +/// and requires both paths within it. +fn assert_retarget_warning(stderr: &str, parent: &Path, child: &Path) { + let line = stderr + .lines() + .find(|l| l.contains("resolved to an ancestor")) + .unwrap_or_else(|| { + panic!("no ancestor-retarget warning line in stderr:\n{stderr}"); + }); + assert!( + line.contains(&child.display().to_string()), + "the warning must name the directory the user asked for: {line}" + ); + assert!( + line.contains(&parent.display().to_string()), + "the warning must name the ANCESTOR actually operated on: {line}" + ); + assert!( + stderr.contains("codegraph init"), + "the warning must say how to get a child-local index: {stderr}" + ); +} + +#[test] +fn index_in_an_unindexed_child_warns_that_the_parent_is_the_target() { + let dir = TestDir::new("index"); + let (parent, child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&child, &["index", "."]); + assert!( + ok, + "index must still succeed: stdout={stdout} stderr={stderr}" + ); + assert_retarget_warning(&stderr, &parent, &child); +} + +#[test] +fn sync_in_an_unindexed_child_warns() { + let dir = TestDir::new("sync"); + let (parent, child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&child, &["sync", "."]); + assert!( + ok, + "sync must still succeed: stdout={stdout} stderr={stderr}" + ); + assert_retarget_warning(&stderr, &parent, &child); +} + +#[test] +fn uninit_in_an_unindexed_child_warns_before_deleting_the_parent_index() { + // The most destructive case: `uninit --force` in a child deletes the + // PARENT's index. A silent retarget here removes an index the user never + // named. + let dir = TestDir::new("uninit"); + let (parent, child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&child, &["uninit", ".", "--force"]); + assert!( + ok, + "uninit must still succeed: stdout={stdout} stderr={stderr}" + ); + assert_retarget_warning(&stderr, &parent, &child); +} + +#[test] +fn unlock_in_an_unindexed_child_warns() { + let dir = TestDir::new("unlock"); + let (parent, child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&child, &["unlock", "."]); + assert!( + ok, + "unlock must still succeed: stdout={stdout} stderr={stderr}" + ); + assert_retarget_warning(&stderr, &parent, &child); +} + +#[test] +fn index_at_the_indexed_root_itself_warns_about_nothing() { + // The control. This is the overwhelmingly common case, and a warning here + // would be noise on every single run — so the check must key on "the + // resolved project differs from what was asked for", not on "an ancestor + // walk happened". + let dir = TestDir::new("noop"); + let (parent, _child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&parent, &["index", "."]); + assert!(ok, "index failed: stdout={stdout} stderr={stderr}"); + assert!( + !stderr.contains("resolved to an ancestor"), + "operating on the requested directory must not warn: {stderr}" + ); +} + +#[test] +fn the_retarget_warning_never_reaches_stdout() { + // `stdout_purity.rs` pins machine-readable stdout for these commands; a + // warning printed there would corrupt any script parsing the output. + let dir = TestDir::new("purity"); + let (_parent, child) = parent_indexed_child_not(&dir); + let (stdout, stderr, ok) = cli_in(&child, &["index", "."]); + assert!(ok, "index failed: stderr={stderr}"); + assert!( + !stdout.contains("resolved to an ancestor"), + "the warning must be on stderr only, found on stdout: {stdout}" + ); +} diff --git a/crates/codegraph-cli/tests/file_disambiguation.rs b/crates/codegraph-cli/tests/file_disambiguation.rs new file mode 100644 index 00000000..23190e09 --- /dev/null +++ b/crates/codegraph-cli/tests/file_disambiguation.rs @@ -0,0 +1,248 @@ +//! `--file` disambiguation for `callers` / `callees` / `impact` (upstream #1512). +//! +//! Two same-named definitions merged into ONE unlabeled list before, so an agent +//! could not tell which definition a caller belonged to and had no way to ask +//! about just one. Each test drives the REAL binary over a temp project, because +//! the defect is in the CLI's symbol-selection surface, not in the graph. +//! +//! Every assertion names the FILE of the returned relatives: a test that only +//! counted results would pass with the filter selecting the wrong definition. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +struct TestDir { + path: PathBuf, +} + +impl TestDir { + fn new(label: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "codegraph-cli-file-filter-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&path).unwrap(); + Self { path } + } + + fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.path); + } +} + +fn cli(args: &[&str]) -> (String, String, bool) { + let output = Command::new(env!("CARGO_BIN_EXE_codegraph")) + .args(args) + .env("CODEGRAPH_NO_DAEMON", "1") + .env("CODEGRAPH_NO_WATCH", "1") + .output() + .expect("run codegraph binary"); + ( + String::from_utf8_lossy(&output.stdout).into_owned(), + String::from_utf8_lossy(&output.stderr).into_owned(), + output.status.success(), + ) +} + +/// A project with `target()` defined TWICE — `alpha.ts` and `beta.ts` — each +/// called by its own distinctly-named caller, so a returned caller's NAME +/// identifies which definition was selected. +fn two_definition_project(dir: &TestDir) -> PathBuf { + let project = dir.path().join("proj"); + fs::create_dir_all(&project).unwrap(); + fs::write( + project.join("alpha.ts"), + "export function target(): number {\n return leafAlpha();\n}\n\ + export function leafAlpha(): number {\n return 1;\n}\n\ + export function callerAlpha(): number {\n return target();\n}\n", + ) + .unwrap(); + fs::write( + project.join("beta.ts"), + "export function target(): number {\n return leafBeta();\n}\n\ + export function leafBeta(): number {\n return 2;\n}\n\ + export function callerBeta(): number {\n return target();\n}\n", + ) + .unwrap(); + let p = project.to_str().unwrap(); + let (out, err, ok) = cli(&["init", p]); + assert!(ok, "init failed: stdout={out} stderr={err}"); + project +} + +fn json_of(args: &[&str]) -> serde_json::Value { + let (stdout, err, ok) = cli(args); + assert!(ok, "command failed: stdout={stdout} stderr={err}"); + serde_json::from_str(&stdout).expect("valid JSON on stdout") +} + +fn names(value: &serde_json::Value, key: &str) -> Vec { + let mut out: Vec = value[key] + .as_array() + .unwrap_or_else(|| panic!("`{key}` must be an array in {value}")) + .iter() + .map(|n| n["name"].as_str().unwrap_or_default().to_string()) + .collect(); + out.sort(); + out +} + +#[test] +fn callers_without_file_returns_both_definitions_callers() { + let dir = TestDir::new("callers-all"); + let project = two_definition_project(&dir); + let p = project.to_str().unwrap(); + let v = json_of(&["callers", "target", "-p", p, "--json"]); + assert_eq!( + names(&v, "callers"), + vec!["callerAlpha".to_string(), "callerBeta".to_string()], + "unfiltered behaviour must be unchanged: both definitions' callers" + ); +} + +#[test] +fn callers_with_file_selects_only_that_definition() { + let dir = TestDir::new("callers-filtered"); + let project = two_definition_project(&dir); + let p = project.to_str().unwrap(); + let v = json_of(&["callers", "target", "-p", p, "--file", "alpha.ts", "--json"]); + assert_eq!( + names(&v, "callers"), + vec!["callerAlpha".to_string()], + "--file alpha.ts must return ONLY alpha.ts's caller" + ); + assert_eq!( + v["file"], "alpha.ts", + "the applied filter must be echoed in the JSON" + ); +} + +#[test] +fn callees_with_file_selects_only_that_definition() { + let dir = TestDir::new("callees-filtered"); + let project = two_definition_project(&dir); + let p = project.to_str().unwrap(); + let v = json_of(&["callees", "target", "-p", p, "--file", "beta.ts", "--json"]); + assert_eq!( + names(&v, "callees"), + vec!["leafBeta".to_string()], + "--file beta.ts must return ONLY beta.ts's callee" + ); +} + +#[test] +fn impact_with_file_selects_only_that_definition() { + let dir = TestDir::new("impact-filtered"); + let project = two_definition_project(&dir); + let p = project.to_str().unwrap(); + let all = json_of(&["impact", "target", "-p", p, "--depth", "3", "--json"]); + let filtered = json_of(&[ + "impact", "target", "-p", p, "--depth", "3", "--file", "alpha.ts", "--json", + ]); + let affected = names(&filtered, "affected"); + assert!( + affected.contains(&"callerAlpha".to_string()), + "alpha.ts's impact must include callerAlpha: {affected:?}" + ); + assert!( + !affected.contains(&"callerBeta".to_string()), + "alpha.ts's impact must NOT include beta.ts's caller: {affected:?}" + ); + assert!( + filtered["nodeCount"].as_u64().unwrap() < all["nodeCount"].as_u64().unwrap(), + "filtering must narrow the radius: filtered={} all={}", + filtered["nodeCount"], + all["nodeCount"] + ); +} + +#[test] +fn file_filter_accepts_a_path_suffix() { + let dir = TestDir::new("suffix"); + let project = dir.path().join("proj"); + fs::create_dir_all(project.join("src/deep")).unwrap(); + fs::write( + project.join("src/deep/mod.ts"), + "export function target(): number {\n return 1;\n}\n\ + export function callerDeep(): number {\n return target();\n}\n", + ) + .unwrap(); + fs::write( + project.join("src/other.ts"), + "export function target(): number {\n return 2;\n}\n\ + export function callerOther(): number {\n return target();\n}\n", + ) + .unwrap(); + let p = project.to_str().unwrap(); + let (out, err, ok) = cli(&["init", p]); + assert!(ok, "init failed: stdout={out} stderr={err}"); + + let v = json_of(&[ + "callers", + "target", + "-p", + p, + "--file", + "deep/mod.ts", + "--json", + ]); + assert_eq!( + names(&v, "callers"), + vec!["callerDeep".to_string()], + "a trailing path segment must be enough to disambiguate" + ); +} + +#[test] +fn file_filter_matching_nothing_is_an_explicit_error() { + // Silently returning "no callers" would read as "this symbol is dead" — + // the same false-negative the filter exists to prevent. + let dir = TestDir::new("nomatch"); + let project = two_definition_project(&dir); + let p = project.to_str().unwrap(); + let (stdout, stderr, ok) = cli(&[ + "callers", + "target", + "-p", + p, + "--file", + "nosuch.ts", + "--json", + ]); + assert!( + !ok, + "an unmatched --file must fail, not report an empty result: {stdout}" + ); + let combined = format!("{stdout}{stderr}"); + assert!( + combined.contains("nosuch.ts"), + "the error must name the rejected filter: {combined}" + ); + assert!( + combined.contains("alpha.ts") && combined.contains("beta.ts"), + "the error must list the files that DO define the symbol: {combined}" + ); +} + +#[test] +fn file_filter_is_documented_in_help_for_all_three_commands() { + for command in ["callers", "callees", "impact"] { + let (stdout, _, ok) = cli(&[command, "--help"]); + assert!(ok, "{command} --help failed"); + assert!( + stdout.contains("--file"), + "{command} --help must document --file:\n{stdout}" + ); + } +} diff --git a/crates/codegraph-cli/tests/installer.rs b/crates/codegraph-cli/tests/installer.rs index 7eaef1ff..7a1cc490 100644 --- a/crates/codegraph-cli/tests/installer.rs +++ b/crates/codegraph-cli/tests/installer.rs @@ -399,3 +399,148 @@ fn unknown_target_fails() { assert!(!output.status.success()); assert!(String::from_utf8_lossy(&output.stderr).contains("Unknown --target")); } + +// ===================== Copilot targets (#23) ================================= +// +// The three Copilot surfaces are additive registry entries, so the end-to-end +// contract is "the id resolves and writes the documented file with the documented +// wrapper key". Before this change all three failed with +// `Unknown target "vscode". Known: claude, cursor, …`. + +#[test] +fn vscode_local_install_writes_servers_wrapper_then_uninstalls() { + let fx = Fixture::new("vscode-local"); + let mcp = fx.project.join(".vscode/mcp.json"); + + fx.run(&["install", "--target=vscode", "--local", "--yes"]); + let config = read_json(&mcp); + assert!( + config.get("servers").is_some(), + "VS Code uses the `servers` wrapper, got {config}" + ); + assert!( + config.get("mcpServers").is_none(), + "must NOT write `mcpServers`: {config}" + ); + let args = config["servers"]["codegraph"]["args"].clone(); + let args = args.as_array().unwrap(); + assert!( + args.contains(&Value::String("--path".into())), + "the LOCAL entry pins a project path: {args:?}" + ); + + // A sibling server the user owns must survive. + let mut edited = read_json(&mcp); + edited["servers"]["other"] = serde_json::json!({ "command": "foo" }); + fs::write(&mcp, serde_json::to_string_pretty(&edited).unwrap()).unwrap(); + + fx.run(&["uninstall", "--target=vscode", "--local"]); + let after = read_json(&mcp); + assert!(after["servers"].get("codegraph").is_none()); + assert!(after["servers"].get("other").is_some()); +} + +/// Every `Code/User/mcp.json` written anywhere under `root`. The user-level base +/// is per-OS (`.config` / `AppData/Roaming` / `Library/Application Support`), and +/// the fixture pins HOME + XDG under `root`, so searching the fixture root keeps +/// the assertion OS-agnostic instead of hardcoding one platform's layout. +fn find_written(root: &Path, needle: &Path) -> Vec { + let mut found = Vec::new(); + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(entries) = fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if path.ends_with(needle) { + found.push(path); + } + } + } + found.sort(); + found +} + +#[test] +fn vscode_global_install_never_writes_workspace_folder() { + // VS Code expands ${workspaceFolder} only in a WORKSPACE mcp.json; in the + // user-level file it stays literal and would point the server at a directory + // named "${workspaceFolder}". Asserted against the WRITTEN FILE — reading + // `--print-config` instead would miss a wrong entry in `install`. + let fx = Fixture::new("vscode-global"); + let out = fx.run(&["install", "--target=vscode", "--global", "--yes"]); + assert!( + !out.contains("Unknown target"), + "the vscode id must resolve: {out}" + ); + let wanted = PathBuf::from("Code").join("User").join("mcp.json"); + let written = find_written(&fx.root, &wanted); + assert_eq!( + written.len(), + 1, + "exactly one user-level Code/User/mcp.json must be written, got {written:?}" + ); + let config = read_json(&written[0]); + assert_eq!( + config["servers"]["codegraph"]["args"], + serde_json::json!(["serve", "--mcp"]), + "the global entry must be BARE — no --path, no ${{workspaceFolder}}: {config}" + ); + let raw = fs::read_to_string(&written[0]).unwrap(); + assert!( + !raw.contains("${workspaceFolder}"), + "the global config must not contain ${{workspaceFolder}}: {raw}" + ); +} + +#[test] +fn copilot_cli_global_install_declares_all_tools() { + let fx = Fixture::new("copilot-cli"); + fx.run(&["install", "--target=copilot-cli", "--global", "--yes"]); + let config = read_json(&fx.home.join(".copilot/mcp-config.json")); + assert_eq!( + config["mcpServers"]["codegraph"]["tools"], + serde_json::json!(["*"]), + "without tools:[\"*\"] the CLI exposes no tools: {config}" + ); + + fx.run(&["uninstall", "--target=copilot-cli", "--global"]); + let after = read_json(&fx.home.join(".copilot/mcp-config.json")); + assert!(after.get("mcpServers").is_none() || after["mcpServers"].get("codegraph").is_none()); +} + +#[test] +fn jetbrains_global_install_writes_github_copilot_intellij() { + let fx = Fixture::new("jetbrains"); + fx.run(&["install", "--target=jetbrains", "--global", "--yes"]); + // The fixture pins XDG_CONFIG_HOME, which is where this target resolves on + // every OS. + let file = fx.root.join("xdg/github-copilot/intellij/mcp.json"); + assert!(file.is_file(), "missing {}", file.display()); + let config = read_json(&file); + assert!( + config.get("servers").is_some(), + "JetBrains uses the `servers` wrapper: {config}" + ); + assert_eq!( + config["servers"]["codegraph"]["args"], + serde_json::json!(["serve", "--mcp"]), + "global-only target writes a bare entry" + ); +} + +#[test] +fn all_three_copilot_ids_resolve_in_print_config() { + let fx = Fixture::new("copilot-ids"); + for id in ["vscode", "copilot-cli", "jetbrains"] { + let out = fx.run(&["install", "--print-config", id]); + assert!(!out.contains("Unknown target"), "{id} must resolve: {out}"); + assert!( + out.contains("codegraph"), + "{id} must print a codegraph entry: {out}" + ); + } +} diff --git a/crates/codegraph-extract/src/walker.rs b/crates/codegraph-extract/src/walker.rs index 7d21d553..f3fe8738 100644 --- a/crates/codegraph-extract/src/walker.rs +++ b/crates/codegraph-extract/src/walker.rs @@ -2560,22 +2560,32 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { .collect::(); format!("= {}{}", init, if init.len() >= 100 { "..." } else { "" }) }); - if let Some(var_node) = self.create_node( - kind, - &name, - child, - NodeExtra { - docstring: docstring.clone(), - signature, - is_exported, - ..NodeExtra::default() - }, - ) { - self.extract_variable_type_annotation(child, &var_node.id); - } + let declared_id = self + .create_node( + kind, + &name, + child, + NodeExtra { + docstring: docstring.clone(), + signature, + is_exported, + ..NodeExtra::default() + }, + ) + .map(|var_node| { + self.extract_variable_type_annotation(child, &var_node.id); + var_node.id + }); if let Some(value) = value_node { if value.kind() != "object" && value.kind() != "object_expression" { - self.visit_function_body(value); + match declared_id { + Some(id) => { + self.node_stack.push(id); + self.visit_function_body(value); + self.node_stack.pop(); + } + None => self.visit_function_body(value), + } } } } @@ -5137,6 +5147,83 @@ func main() { _ = counter } assert!(has_node(&nodes, NodeKind::Variable, "counter")); } + /// #1510 / PR #1511 — a file-scope declaration's initializer call must + /// attribute to the DECLARED symbol. It landed on the file node before, so + /// `callers target` answered with a filename. + #[test] + fn declaration_initializer_call_attributes_to_the_declaration() { + let src = r#" +export function target(): number { return 1; } +export const value = target(); +"#; + let (nodes, refs) = run("d.ts", src, Language::TypeScript); + let value_id = node(&nodes, NodeKind::Constant, "value").id.clone(); + let sources: Vec<&str> = refs + .iter() + .filter(|r| r.reference_name == "target" && r.reference_kind == EdgeKind::Calls) + .map(|r| r.from_node_id.as_str()) + .collect(); + assert_eq!( + sources, + vec![value_id.as_str()], + "the initializer call must come from `value`, not the file node" + ); + } + + /// The control for the test above: a declaration initializer INSIDE a + /// function still attributes to that function, so the fix cannot leak past + /// file scope and re-point every local call at its local binding. + #[test] + fn function_local_declaration_initializer_still_attributes_to_the_function() { + let src = r#" +export function target(): number { return 1; } +export function wrapper(): number { + const inner = target(); + return inner; +} +"#; + let (nodes, refs) = run("d.ts", src, Language::TypeScript); + let wrapper_id = node(&nodes, NodeKind::Function, "wrapper").id.clone(); + let sources: Vec<&str> = refs + .iter() + .filter(|r| r.reference_name == "target" && r.reference_kind == EdgeKind::Calls) + .map(|r| r.from_node_id.as_str()) + .collect(); + assert_eq!( + sources, + vec![wrapper_id.as_str()], + "a function-local initializer call must stay attributed to the function" + ); + } + + /// Multi-declarator statements attribute each initializer to ITS OWN + /// declarator, so `const a = f(), b = g()` does not collapse both calls onto + /// whichever node happened to be created last. + #[test] + fn multi_declarator_initializers_attribute_per_declarator() { + let src = r#" +export function f(): number { return 1; } +export function g(): number { return 2; } +const a = f(), b = g(); +"#; + let (nodes, refs) = run("m.ts", src, Language::TypeScript); + let a_id = node(&nodes, NodeKind::Constant, "a").id.clone(); + let b_id = node(&nodes, NodeKind::Constant, "b").id.clone(); + let pairs: Vec<(&str, &str)> = refs + .iter() + .filter(|r| r.reference_kind == EdgeKind::Calls) + .map(|r| (r.from_node_id.as_str(), r.reference_name.as_str())) + .collect(); + assert!( + pairs.contains(&(a_id.as_str(), "f")), + "`a = f()` must attribute to `a`: {pairs:?}" + ); + assert!( + pairs.contains(&(b_id.as_str(), "g")), + "`b = g()` must attribute to `b`: {pairs:?}" + ); + } + #[test] fn php_class_method_and_const_extracted() { let src = r#" **Explore budget: {call_budget} calls for this project ({} files indexed).** Each call covers ~6 files; if your question spans more, spend your remaining calls on the uncovered area BEFORE falling back to Read — another explore is cheaper and more complete than reading those files. Synthesize once you've used {call_budget}.", + "> **{} files indexed.** Each call covers ~6 files; if your question spans more, make ANOTHER `codegraph_explore` targeting the uncovered area BEFORE falling back to Read — another explore is cheaper and more complete than reading those files. There is no call limit.", c.file_count )); lines.push(String::new()); @@ -4502,8 +4508,61 @@ mod tests { assert!(txt.contains("### Relationships"), "got: {txt}"); assert!(txt.contains("line 5")); assert!( - txt.contains("Explore budget"), - "expected budget note, got: {txt}" + txt.contains("files indexed"), + "expected the follow-up-call note, got: {txt}" + ); + } + + /// #1504 — the note must not promise a call CAP. Nothing enforces one, so + /// the upstream "N calls for this project / Synthesize once you've used N" + /// wording only made agents stop exploring early. + #[test] + fn ext_explore_note_promises_no_unenforced_call_budget() { + let mut engine = test_engine(); + put_indexed_source( + &engine, + "svc.rs", + "fn processOrder() {}\n", + Language::Rust, + 1, + ); + for i in 0..5001 { + put_file( + &engine, + &file_rec(&format!("pad/f{i}.rs"), Language::Rust, 0), + ); + } + put_nodes( + &mut engine, + &[node_lang( + "processOrder", + "processOrder", + "svc.rs", + 1, + 1, + NodeKind::Function, + Language::Rust, + )], + ); + let txt = text_of(&engine.execute( + "codegraph_explore", + &serde_json::json!({"query": "processOrder"}), + )); + + for banned in [ + "Explore budget:", + "Synthesize once", + "calls for this project", + "remaining calls", + ] { + assert!( + !txt.contains(banned), + "the explore note must not imply an unenforced call cap, found {banned:?} in: {txt}" + ); + } + assert!( + txt.contains("no call limit"), + "the note must say so explicitly, got: {txt}" ); } diff --git a/crates/codegraph-mcp/src/instructions.rs b/crates/codegraph-mcp/src/instructions.rs index da89ec7e..bb0e38d6 100644 --- a/crates/codegraph-mcp/src/instructions.rs +++ b/crates/codegraph-mcp/src/instructions.rs @@ -1,14 +1,57 @@ //! Server-level instructions emitted in the MCP `initialize` response. //! -//! Verbatim copy of the upstream `SERVER_INSTRUCTIONS` +//! Ported from the upstream `SERVER_INSTRUCTIONS` //! (`upstream mcp/server-instructions.ts:18-78`), captured from the //! LIVE built server (`upstream bin/codegraph.js serve --mcp`) so -//! the byte content is exactly what `initialize` returned (6421 bytes, single -//! trailing newline). The MCP client surfaces this in the agent's system -//! prompt (see that file's doc comment). +//! the byte content matches what `initialize` returned. The MCP client surfaces +//! this in the agent's system prompt (see that file's doc comment). +//! +//! One section is NOT upstream text: "Supported languages" (#671) is GENERATED +//! from this build's own [`Language`] registry by [`server_instructions`], so it +//! can never drift from what the binary actually parses. Upstream hardcodes a +//! "30+" prose count; a copied number would silently go stale the next time a +//! grammar lands. + +use codegraph_core::types::Language; +use std::sync::OnceLock; + +/// The instructions returned in `initialize.result.instructions`: the static +/// body plus the generated supported-language section. +pub fn server_instructions() -> &'static str { + static RENDERED: OnceLock = OnceLock::new(); + RENDERED + .get_or_init(|| format!("{SERVER_INSTRUCTIONS_BODY}{}", render_supported_languages())) + .as_str() +} + +/// The languages this build extracts, in [`Language::ALL`] order. +/// +/// Excludes `Unknown` (the not-a-language sentinel) and the three Godot +/// non-script file kinds (`.tscn` / `.tres` / `project.godot`), which are Godot +/// PROJECT artifacts parsed as part of GDScript support rather than languages a +/// user would name. +pub fn supported_language_names() -> Vec<&'static str> { + Language::ALL + .iter() + .copied() + .filter(|l| *l != Language::Unknown && !l.is_godot_non_script_file()) + .map(Language::as_str) + .collect() +} + +fn render_supported_languages() -> String { + let names = supported_language_names(); + format!( + "\n## Supported languages\n\n\ + Codegraph extracts these {} languages ({}). A file outside this set is \ + not indexed, so no codegraph tool can see it — reach for Read there.\n", + names.len(), + names.join(", ") + ) +} -/// The exact instructions string returned in `initialize.result.instructions`. -pub const SERVER_INSTRUCTIONS: &str = r#"# Codegraph — code intelligence over an indexed knowledge graph +/// The static, upstream-derived body of the instructions. +const SERVER_INSTRUCTIONS_BODY: &str = r#"# Codegraph — code intelligence over an indexed knowledge graph Codegraph is a SQLite knowledge graph of every symbol, edge, and file in the workspace — pre-computed structure you would otherwise re-derive by @@ -69,3 +112,87 @@ typically one to a few calls; a grep/read exploration is dozens. - Cross-file resolution is best-effort name matching; ambiguous calls may return multiple candidates. - No live correctness validation — that's still the TypeScript compiler / test suite / linter's job. Codegraph supplements those with structural context they don't have. "#; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn supported_languages_excludes_unknown_and_godot_artifacts() { + let names = supported_language_names(); + assert!( + !names.contains(&"unknown"), + "the sentinel is not a language" + ); + for artifact in ["godot_scene", "godot_resource", "godot_project"] { + assert!( + !names.contains(&artifact), + "{artifact} is a Godot project artifact, not a language" + ); + } + assert!(names.contains(&"gdscript"), "GDScript itself IS a language"); + assert_eq!( + names.len(), + Language::ALL.len() - 4, + "exactly `unknown` + the three Godot artifact kinds are excluded" + ); + } + + #[test] + fn supported_language_names_are_unique_and_ordered_like_the_registry() { + let names = supported_language_names(); + let mut deduped = names.clone(); + deduped.dedup(); + assert_eq!(names, deduped, "no duplicates"); + let from_registry: Vec<&str> = Language::ALL + .iter() + .copied() + .filter(|l| *l != Language::Unknown && !l.is_godot_non_script_file()) + .map(Language::as_str) + .collect(); + assert_eq!( + names, from_registry, + "the list must be derived from the registry, not a hand-kept copy" + ); + } + + /// #671 — the instructions must NAME the supported languages, and the count + /// must come from this build's registry rather than upstream's "30+" prose. + #[test] + fn instructions_list_the_supported_languages_from_the_registry() { + let text = server_instructions(); + assert!( + text.contains("## Supported languages"), + "instructions must carry a supported-language section" + ); + let names = supported_language_names(); + assert!( + text.contains(&format!("these {} languages", names.len())), + "the count must be the registry's, got: {text}" + ); + for name in names { + assert!(text.contains(name), "language {name} must be named"); + } + assert!( + !text.contains("30+"), + "upstream's hardcoded count must not be copied" + ); + } + + #[test] + fn instructions_keep_the_upstream_body_and_end_with_one_newline() { + let text = server_instructions(); + assert!( + text.starts_with("# Codegraph — code intelligence over an indexed knowledge graph"), + "the upstream body must still lead" + ); + assert!( + text.contains("## Tool selection by intent"), + "the upstream body must be preserved in full" + ); + assert!( + text.ends_with('\n') && !text.ends_with("\n\n"), + "single trailing newline" + ); + } +} diff --git a/crates/codegraph-mcp/src/rmcp_handler.rs b/crates/codegraph-mcp/src/rmcp_handler.rs index db71cd0b..708b0385 100644 --- a/crates/codegraph-mcp/src/rmcp_handler.rs +++ b/crates/codegraph-mcp/src/rmcp_handler.rs @@ -2,7 +2,7 @@ //! running ALONGSIDE the hand-rolled [`crate::McpServer`]. //! //! Reuses the sync engine + rendering layer verbatim: [`CodeGraphEngine`], -//! [`crate::schemas`], [`crate::instructions::SERVER_INSTRUCTIONS`], and the +//! [`crate::schemas`], [`crate::instructions::server_instructions`], and the //! `roots` project resolution. Only the transport/session shell is rmcp. //! //! ## Sync engine bridge @@ -34,7 +34,7 @@ use rmcp::service::{NotificationContext, RequestContext, RoleServer}; use serde_json::{Value, json}; use crate::engine::CodeGraphEngine; -use crate::instructions::SERVER_INSTRUCTIONS; +use crate::instructions::server_instructions; use crate::protocol::ToolResult; use crate::roots::{WorkspaceRoots, db_exists_for, db_path_for}; use crate::schemas; @@ -301,7 +301,7 @@ impl ServerHandler for CodeGraphHandler { InitializeResult::new(ServerCapabilities::builder().enable_tools().build()) .with_protocol_version(ProtocolVersion::V_2024_11_05) .with_server_info(Implementation::new(SERVER_NAME, SERVER_VERSION)) - .with_instructions(SERVER_INSTRUCTIONS) + .with_instructions(server_instructions()) } async fn on_initialized(&self, context: NotificationContext) { diff --git a/crates/codegraph-mcp/src/server.rs b/crates/codegraph-mcp/src/server.rs index db79290b..6fe297b2 100644 --- a/crates/codegraph-mcp/src/server.rs +++ b/crates/codegraph-mcp/src/server.rs @@ -16,7 +16,7 @@ use std::path::{Path, PathBuf}; use serde_json::{Value, json}; use crate::engine::CodeGraphEngine; -use crate::instructions::SERVER_INSTRUCTIONS; +use crate::instructions::server_instructions; use crate::protocol::{JsonRpcRequest, JsonRpcResponse, ToolResult, error_codes}; use crate::roots::{ ROOTS_LIST_REQUEST_ID, WorkspaceRoots, db_exists_for, db_path_for, format_tool_debug_line, @@ -614,7 +614,7 @@ pub fn initialize_result() -> Value { "protocolVersion": PROTOCOL_VERSION, "capabilities": { "tools": {} }, "serverInfo": { "name": SERVER_NAME, "version": SERVER_VERSION }, - "instructions": SERVER_INSTRUCTIONS, + "instructions": server_instructions(), }) } diff --git a/crates/codegraph-resolve/src/context.rs b/crates/codegraph-resolve/src/context.rs index acc9f218..2fc93e07 100644 --- a/crates/codegraph-resolve/src/context.rs +++ b/crates/codegraph-resolve/src/context.rs @@ -14,6 +14,7 @@ use crate::{import_resolver, pathutil}; use codegraph_core::types::{EdgeKind, Language, Node, NodeKind}; use codegraph_store::Store; use std::cell::RefCell; +use std::collections::BTreeSet; use std::path::Path; /// `DEFAULT_CACHE_LIMIT` (`index.ts:54`). Override via env in the upstream; the v1 port @@ -41,6 +42,7 @@ struct Caches { qualified_name_cache: LruCache>, project_aliases: Option>, go_module: Option>, + go_modules: Option>, workspace_packages: Option>, } @@ -59,6 +61,7 @@ impl Caches { qualified_name_cache: LruCache::new(limit), project_aliases: None, go_module: None, + go_modules: None, workspace_packages: None, } } @@ -278,6 +281,15 @@ impl ResolutionContext for StoreResolutionContext<'_> { c.go_module.clone().flatten() } + fn get_go_modules(&self) -> Vec { + if let Some(cached) = self.caches.borrow().go_modules.clone() { + return cached; + } + let modules = load_go_modules(&self.project_root, &self.get_all_files()); + self.caches.borrow_mut().go_modules = Some(modules.clone()); + modules + } + fn get_re_exports(&self, file_path: &str, language: Language) -> Vec { let mut c = self.caches.borrow_mut(); if let Some(cached) = c.re_export_cache.get(&file_path.to_string()) { @@ -322,6 +334,11 @@ pub(crate) fn load_go_module_pub(project_root: &str) -> Option { load_go_module(project_root) } +/// Shares [`load_go_modules`] with the snapshot context. +pub(crate) fn load_go_modules_pub(project_root: &str, files: &[String]) -> Vec { + load_go_modules(project_root, files) +} + /// Order name-lookup candidates by `(file_path, start_line, start_column, id)`. /// /// A full index inserts nodes file-by-file in sorted-path order, each file's @@ -364,7 +381,17 @@ fn is_js_family_path(file_path: &str) -> bool { /// Load `module` path from `go.mod` at the project root /// (mirrors `loadGoModule`, `upstream resolution/go-module.ts`). fn load_go_module(project_root: &str) -> Option { - let text = std::fs::read_to_string(pathutil::resolve(project_root, "go.mod")).ok()?; + load_go_module_in(project_root, "") +} + +/// Read the `module` line of `//go.mod`. +fn load_go_module_in(project_root: &str, dir: &str) -> Option { + let relative = if dir.is_empty() { + "go.mod".to_string() + } else { + format!("{dir}/go.mod") + }; + let text = std::fs::read_to_string(pathutil::resolve(project_root, &relative)).ok()?; for line in text.lines() { let trimmed = line.trim_start(); if let Some(rest) = trimmed.strip_prefix("module") { @@ -372,6 +399,7 @@ fn load_go_module(project_root: &str) -> Option { if !module_path.is_empty() { return Some(GoModule { module_path: module_path.to_string(), + dir: dir.to_string(), }); } } @@ -379,6 +407,47 @@ fn load_go_module(project_root: &str) -> Option { None } +/// Every Go module reachable from the indexed `.go` files, nearest-`go.mod`-first +/// per file (#1521). +/// +/// A multi-module layout (`a/go.mod` + `b/go.mod`, no root `go.mod`) produced NO +/// module at all before, because only `/go.mod` was ever read; with +/// no module path, every cross-module import classified as EXTERNAL and the Go +/// cross-package resolver bailed on its first line, so such a repo got zero +/// `calls` edges across the module boundary. +/// +/// Ordered by `dir` and deduplicated, so the result does not depend on the file +/// iteration order. +fn load_go_modules(project_root: &str, files: &[String]) -> Vec { + let mut dirs: BTreeSet = BTreeSet::new(); + for file in files { + if !file.ends_with(".go") { + continue; + } + let normalized = file.replace('\\', "/"); + let mut dir = pathutil::dirname(&normalized); + loop { + dirs.insert(dir.clone()); + if dir.is_empty() { + break; + } + dir = pathutil::dirname(&dir); + } + } + // A Go-less project keeps the previous behaviour: probe the root only. + if dirs.is_empty() { + return load_go_module(project_root).into_iter().collect(); + } + + let mut modules: Vec = dirs + .iter() + .filter_map(|dir| load_go_module_in(project_root, dir)) + .collect(); + modules.sort_by(|a, b| a.dir.cmp(&b.dir).then(a.module_path.cmp(&b.module_path))); + modules.dedup_by(|a, b| a.dir == b.dir); + modules +} + #[cfg(test)] mod tests { use super::*; @@ -481,6 +550,82 @@ mod tests { std::fs::remove_dir_all(&root).ok(); } + #[test] + fn load_go_modules_finds_every_nested_module_without_a_root_one() { + let root = temp_root("gomods"); + for sub in ["a", "b"] { + std::fs::create_dir_all(root.join(sub)).unwrap(); + std::fs::write( + root.join(sub).join("go.mod"), + format!("module ex/{sub}\n\ngo 1.21\n"), + ) + .unwrap(); + } + let files = vec!["b/main.go".to_string(), "a/helper.go".to_string()]; + let modules = load_go_modules(root.to_str().unwrap(), &files); + assert_eq!( + modules + .iter() + .map(|m| (m.dir.as_str(), m.module_path.as_str())) + .collect::>(), + vec![("a", "ex/a"), ("b", "ex/b")], + "both nested modules must be discovered, ordered by dir" + ); + std::fs::remove_dir_all(&root).ok(); + } + + #[test] + fn load_go_modules_reversed_file_order_is_identical() { + let root = temp_root("gomodsdet"); + for sub in ["a", "b"] { + std::fs::create_dir_all(root.join(sub)).unwrap(); + std::fs::write( + root.join(sub).join("go.mod"), + format!("module ex/{sub}\n\ngo 1.21\n"), + ) + .unwrap(); + } + let forward = load_go_modules( + root.to_str().unwrap(), + &["a/helper.go".to_string(), "b/main.go".to_string()], + ); + let reverse = load_go_modules( + root.to_str().unwrap(), + &["b/main.go".to_string(), "a/helper.go".to_string()], + ); + assert_eq!(forward, reverse, "discovery must not depend on file order"); + std::fs::remove_dir_all(&root).ok(); + } + + #[test] + fn load_go_modules_root_module_keeps_empty_dir() { + let root = temp_root("gomodroot"); + std::fs::write(root.join("go.mod"), "module ex/m\n").unwrap(); + let modules = load_go_modules(root.to_str().unwrap(), &["cmd/main.go".to_string()]); + assert_eq!(modules.len(), 1); + assert_eq!(modules[0].dir, ""); + assert_eq!(modules[0].module_path, "ex/m"); + std::fs::remove_dir_all(&root).ok(); + } + + #[test] + fn load_go_modules_without_go_files_probes_the_root_only() { + let root = temp_root("gomodnogo"); + std::fs::write(root.join("go.mod"), "module ex/m\n").unwrap(); + std::fs::create_dir_all(root.join("sub")).unwrap(); + std::fs::write(root.join("sub").join("go.mod"), "module ex/sub\n").unwrap(); + let modules = load_go_modules(root.to_str().unwrap(), &["src/a.ts".to_string()]); + assert_eq!( + modules + .iter() + .map(|m| m.module_path.as_str()) + .collect::>(), + vec!["ex/m"], + "a project with no .go files must not scan for nested modules" + ); + std::fs::remove_dir_all(&root).ok(); + } + #[test] fn store_context_node_lookups_and_caching() { let root = temp_root("lookups"); diff --git a/crates/codegraph-resolve/src/import_resolver.rs b/crates/codegraph-resolve/src/import_resolver.rs index 6b4c21c6..78c74b1b 100644 --- a/crates/codegraph-resolve/src/import_resolver.rs +++ b/crates/codegraph-resolve/src/import_resolver.rs @@ -12,7 +12,9 @@ use crate::name_matcher::{ }; use crate::path_aliases::apply_aliases; use crate::pathutil; -use crate::types::{ImportMapping, ReExport, RefView, ResolutionContext, ResolvedBy, ResolvedRef}; +use crate::types::{ + GoModule, ImportMapping, ReExport, RefView, ResolutionContext, ResolvedBy, ResolvedRef, +}; use crate::workspace_packages::resolve_workspace_import; use codegraph_core::types::{Language, Node, NodeKind}; use regex::Regex; @@ -424,7 +426,7 @@ fn is_external_import( if import_path.starts_with('.') { return false; } - if let Some(module) = context.get_go_module() { + for module in context.get_go_modules() { if import_path == module.module_path || import_path.starts_with(&format!("{}/", module.module_path)) { @@ -1457,6 +1459,9 @@ pub fn resolve_via_import( if let Some(py_result) = resolve_python_module_member(reference, &imports, context) { return Some(py_result); } + if let Some(py_member) = resolve_python_from_import_member(reference, &imports, context) { + return Some(py_member); + } if let Some(py_mod) = resolve_python_absolute_module(reference, context) { return Some(py_mod); } @@ -1996,6 +2001,30 @@ fn resolve_static_member( Some(candidates.swap_remove(0)) } +/// The module whose `module_path` covers `import_path`, longest match first. +fn owning_go_module<'m>(import_path: &str, modules: &'m [GoModule]) -> Option<&'m GoModule> { + modules + .iter() + .filter(|m| { + import_path == m.module_path || import_path.starts_with(&format!("{}/", m.module_path)) + }) + .max_by_key(|m| m.module_path.len()) +} + +/// The project-relative directory an in-project Go import resolves to: the +/// owning module's own directory, plus the import's path below `module_path`. +fn go_package_dir(import_path: &str, module: &GoModule) -> String { + let below = import_path + .strip_prefix(&module.module_path) + .unwrap_or_default() + .trim_start_matches('/'); + match (module.dir.as_str(), below) { + ("", rest) => rest.to_string(), + (dir, "") => dir.to_string(), + (dir, rest) => format!("{dir}/{rest}"), + } +} + /// Go cross-package call `pkga.FuncX` → the exported member in the package /// directory the import maps to. Ports `resolveGoCrossPackageReference` /// (import-resolver.ts, issue #388). @@ -2004,7 +2033,10 @@ fn resolve_go_cross_package_reference( imports: &[ImportMapping], context: &dyn ResolutionContext, ) -> Option { - let module = context.get_go_module()?; + let modules = context.get_go_modules(); + if modules.is_empty() { + return None; + } let dot_idx = reference.reference_name.find('.')?; if dot_idx == 0 { return None; @@ -2019,17 +2051,13 @@ fn resolve_go_cross_package_reference( if imp.local_name != receiver { continue; } - // Only in-module imports map to a known directory. - if imp.source != module.module_path - && !imp.source.starts_with(&format!("{}/", module.module_path)) - { + // Only in-project imports map to a known directory. With several modules + // the LONGEST matching module path wins, so a module nested inside + // another's path resolves to its own directory rather than its parent's. + let Some(module) = owning_go_module(&imp.source, &modules) else { continue; - } - let pkg_dir = if imp.source == module.module_path { - "" - } else { - &imp.source[module.module_path.len() + 1..] }; + let pkg_dir = go_package_dir(&imp.source, module); for node in context.get_nodes_by_name(member) { if node.language != Language::Go || !node.is_exported { @@ -2122,6 +2150,97 @@ fn resolve_python_module_member( None } +/// Python `from pkg.mod import member [as alias]` → `member` in the file +/// `pkg.mod` names (#1518 / #1453). +/// +/// The generic import loop cannot reach this: it only ever resolves an import's +/// `source` through [`resolve_import_path`], which maps RELATIVE dotted Python +/// paths but not ABSOLUTE ones, and it then requires +/// [`find_exported_symbol`] — which gates on `is_exported`, a flag the Python +/// extractor never sets. Both halves fail, so the reference falls through to the +/// name matcher: with two same-named definitions it picks the proximity-nearest +/// (a DECOY module), and with an `as` alias it finds no node named `alias` at all +/// and emits NO edge. Upstream frames this as a line-wrapped parenthesized +/// import-list parsing bug; the single-line form fails identically, so the fix is +/// module-scoped member resolution rather than a list parser. +/// +/// Deliberately narrow, so it repairs the reported defect without displacing an +/// already-correct resolution chain: +/// * `Imports` references keep their own module/file resolution. +/// * FUNCTION-REF (function-as-value) references keep the strictly-gated #756 +/// path — `reference/golden/python/` pins those as `resolvedBy:function-ref`. +/// * Only the BARE reference form (`member()`) is handled; a `Member.attr()` +/// tail stays with `resolve_python_module_member` and the method matcher. +fn resolve_python_from_import_member( + reference: &RefView, + imports: &[ImportMapping], + context: &dyn ResolutionContext, +) -> Option { + if reference.reference_kind == codegraph_core::types::EdgeKind::Imports + || reference.is_function_ref + || reference.reference_name.contains('.') + || reference.reference_name.is_empty() + { + return None; + } + + for imp in imports { + if imp.local_name != reference.reference_name { + continue; + } + // A namespace/default import binds a MODULE, not a member. + if imp.is_namespace || imp.is_default || imp.exported_name.is_empty() { + continue; + } + // `import mod` yields `local_name == exported_name == source`; that is a + // module binding, handled by `resolve_python_module_member` / + // `resolve_module_import_to_file`. + if imp.source.is_empty() || imp.source == imp.local_name { + continue; + } + + // Relative dotted sources resolve lexically; absolute ones need the + // dotted-module file lookup (`pkg.mod` → `pkg/mod.py`). + let Some(module_file) = resolve_import_path( + &imp.source, + &reference.file_path, + reference.language, + context, + ) + .or_else(|| find_python_module_file(&imp.source, context, &reference.file_path)) else { + continue; + }; + if module_file == reference.file_path { + continue; + } + + // `get_nodes_in_file` is ordered by `start_line`, so a file declaring the + // name twice resolves deterministically to the first declaration. + if let Some(target) = context + .get_nodes_in_file(&module_file) + .into_iter() + .find(|n| { + n.name == imp.exported_name + && matches!( + n.kind, + NodeKind::Function + | NodeKind::Class + | NodeKind::Variable + | NodeKind::Constant + ) + }) + { + return Some(ResolvedRef { + original: reference.clone(), + target_node_id: target.id, + confidence: 0.9, + resolved_by: ResolvedBy::Import, + }); + } + } + None +} + /// Python absolute dotted module import `import a.b.c` → its file (the Django /// `AppConfig.ready()` side-effect-import pattern). Ports /// `resolvePythonAbsoluteModule` (import-resolver.ts). @@ -2395,7 +2514,7 @@ fn resolve_lua_require( mod tests { use super::*; use crate::path_aliases::{AliasMap, AliasPattern}; - use crate::types::{GoModule, ImportMapping, ReExport, RefView, ResolutionContext}; + use crate::types::{ImportMapping, ReExport, RefView, ResolutionContext}; use crate::workspace_packages::WorkspacePackages; use codegraph_core::types::{EdgeKind, Language, Node, NodeKind}; use std::collections::{BTreeMap, BTreeSet}; @@ -3145,6 +3264,7 @@ export default defaultTarget; let ctx = TestContext { go_module: Some(GoModule { module_path: "example.com/proj".to_string(), + dir: String::new(), }), ..Default::default() }; @@ -4010,6 +4130,7 @@ export default defaultTarget; nodes: vec![target], go_module: Some(GoModule { module_path: "example.com/proj".to_string(), + dir: String::new(), }), ..Default::default() }; @@ -4018,6 +4139,49 @@ export default defaultTarget; assert_eq!(resolved.target_node_id, "function:FuncX"); } + fn go_mod(module_path: &str, dir: &str) -> crate::types::GoModule { + crate::types::GoModule { + module_path: module_path.to_string(), + dir: dir.to_string(), + } + } + + #[test] + fn owning_go_module_prefers_the_longest_matching_module_path() { + let modules = vec![go_mod("ex/a", "a"), go_mod("ex/a/sub", "a/sub")]; + assert_eq!( + owning_go_module("ex/a/sub/pkg", &modules).map(|m| m.dir.as_str()), + Some("a/sub"), + "the nested module must win over its prefix" + ); + assert_eq!( + owning_go_module("ex/a/other", &modules).map(|m| m.dir.as_str()), + Some("a") + ); + assert!(owning_go_module("github.com/x/y", &modules).is_none()); + } + + #[test] + fn owning_go_module_rejects_a_shared_path_prefix_that_is_not_a_segment() { + let modules = vec![go_mod("ex/a", "a")]; + assert!( + owning_go_module("ex/abc", &modules).is_none(), + "`ex/abc` is a different module, not a package inside `ex/a`" + ); + } + + #[test] + fn go_package_dir_joins_module_dir_with_the_path_below_it() { + assert_eq!(go_package_dir("ex/a", &go_mod("ex/a", "a")), "a"); + assert_eq!(go_package_dir("ex/a/pkg", &go_mod("ex/a", "a")), "a/pkg"); + assert_eq!(go_package_dir("ex/m/pkg", &go_mod("ex/m", "")), "pkg"); + assert_eq!( + go_package_dir("ex/m", &go_mod("ex/m", "")), + "", + "a root module's own package lives in the project root" + ); + } + #[test] fn resolve_python_module_member_resolves() { let mut existing = BTreeSet::new(); diff --git a/crates/codegraph-resolve/src/snapshot_context.rs b/crates/codegraph-resolve/src/snapshot_context.rs index 6a407612..d619b0fa 100644 --- a/crates/codegraph-resolve/src/snapshot_context.rs +++ b/crates/codegraph-resolve/src/snapshot_context.rs @@ -60,6 +60,7 @@ struct NodeSnapshot { project_aliases: Option, workspace_packages: Option, go_module: Option, + go_modules: Vec, } /// A `Sync`, immutable [`ResolutionContext`] over a precomputed [`NodeSnapshot`] @@ -152,6 +153,7 @@ impl SnapshotResolutionContext { let project_aliases = load_project_aliases(&project_root); let workspace_packages = load_workspace_packages(&project_root); let go_module = crate::context::load_go_module_pub(&project_root); + let go_modules = crate::context::load_go_modules_pub(&project_root, &all_file_paths); Ok(Self { snapshot: Arc::new(NodeSnapshot { @@ -168,6 +170,7 @@ impl SnapshotResolutionContext { project_aliases, workspace_packages, go_module, + go_modules, }), edges: Arc::new(HashMap::new()), }) @@ -323,6 +326,10 @@ impl ResolutionContext for SnapshotResolutionContext { self.snapshot.go_module.clone() } + fn get_go_modules(&self) -> Vec { + self.snapshot.go_modules.clone() + } + fn get_re_exports(&self, file_path: &str, language: Language) -> Vec { match self.read_file(file_path) { Some(text) => { diff --git a/crates/codegraph-resolve/src/types.rs b/crates/codegraph-resolve/src/types.rs index d6826c5f..a96158f1 100644 --- a/crates/codegraph-resolve/src/types.rs +++ b/crates/codegraph-resolve/src/types.rs @@ -360,6 +360,15 @@ pub trait ResolutionContext { None } + /// EVERY Go module in the project, for multi-`go.mod` layouts (#1521). + /// + /// The default lifts [`Self::get_go_module`], so a context that only knows the + /// root module keeps its exact previous behaviour and no implementor is forced + /// to change. + fn get_go_modules(&self) -> Vec { + self.get_go_module().into_iter().collect() + } + /// Re-exports declared by a file (`getReExports?`, `types.ts:119-125`). /// Empty when the file has none. fn get_re_exports(&self, _file_path: &str, _language: Language) -> Vec { @@ -376,11 +385,15 @@ pub trait ResolutionContext { /// Go module info parsed from `go.mod`. /// /// Mirrors the upstream `GoModule` (`upstream resolution/go-module.ts`), -/// referenced by `ResolutionContext.getGoModule?` (`types.ts:104-111`). Only -/// `module_path` is load-bearing for import classification. +/// referenced by `ResolutionContext.getGoModule?` (`types.ts:104-111`). #[derive(Debug, Clone, PartialEq, Eq)] pub struct GoModule { pub module_path: String, + /// Project-relative directory holding this module's `go.mod`, `/`-separated, + /// EMPTY for a module at the project root. An import path is rewritten to a + /// directory as `dir` + the part of the import after `module_path`, so a + /// nested module's packages resolve to their real on-disk location (#1521). + pub dir: String, } #[cfg(test)] @@ -619,9 +632,11 @@ mod tests { fn go_module_constructs_and_derives() { let gm = GoModule { module_path: "example.com/mod".to_string(), + dir: "sub".to_string(), }; assert_eq!(gm.clone(), gm); assert_eq!(gm.module_path, "example.com/mod"); + assert_eq!(gm.dir, "sub"); assert!(format!("{gm:?}").contains("GoModule")); } diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/go.mod b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/go.mod new file mode 100644 index 00000000..c5d7586f --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/go.mod @@ -0,0 +1,3 @@ +module ex/a + +go 1.21 diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/helper.go b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/helper.go new file mode 100644 index 00000000..d1673fd2 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/a/helper.go @@ -0,0 +1,6 @@ +package a + +// Helper is the cross-module target. +func Helper() int { + return 1 +} diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/go.mod b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/go.mod new file mode 100644 index 00000000..48a3a50b --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/go.mod @@ -0,0 +1,7 @@ +module ex/b + +go 1.21 + +require ex/a v0.0.0 + +replace ex/a => ../a diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/local.go b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/local.go new file mode 100644 index 00000000..411d284c --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/local.go @@ -0,0 +1,6 @@ +package main + +// Helper is a same-named decoy in the CALLER's own package. +func Helper() int { + return 99 +} diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/main.go b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/main.go new file mode 100644 index 00000000..7cea9ea5 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/go_multi_module/b/main.go @@ -0,0 +1,8 @@ +package main + +import "ex/a" + +// Caller reaches across the module boundary. +func Caller() int { + return a.Helper() +} diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app.py b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app.py new file mode 100644 index 00000000..0ea9307b --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app.py @@ -0,0 +1,5 @@ +from pkg.mod import real_target + + +def caller(): + return real_target() diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app2.py b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app2.py new file mode 100644 index 00000000..85917212 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/app2.py @@ -0,0 +1,5 @@ +from pkg.mod import real_target as aliased + + +def caller2(): + return aliased() diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/__init__.py b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/decoy.py b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/decoy.py new file mode 100644 index 00000000..45156704 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/decoy.py @@ -0,0 +1,2 @@ +def real_target(): + return 99 diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/mod.py b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/mod.py new file mode 100644 index 00000000..6cd842b7 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/py_from_import/pkg/mod.py @@ -0,0 +1,2 @@ +def real_target(): + return 1 diff --git a/crates/codegraph-resolve/tests/fixtures/tier6/ts_decl_initializer/d.ts b/crates/codegraph-resolve/tests/fixtures/tier6/ts_decl_initializer/d.ts new file mode 100644 index 00000000..61876289 --- /dev/null +++ b/crates/codegraph-resolve/tests/fixtures/tier6/ts_decl_initializer/d.ts @@ -0,0 +1,10 @@ +export function target(): number { + return 1; +} + +export const value = target(); + +export function wrapper(): number { + const inner = target(); + return inner; +} diff --git a/crates/codegraph-resolve/tests/tier6_resolution.rs b/crates/codegraph-resolve/tests/tier6_resolution.rs new file mode 100644 index 00000000..08cee4db --- /dev/null +++ b/crates/codegraph-resolve/tests/tier6_resolution.rs @@ -0,0 +1,347 @@ +//! Tier 6 resolution regression tests (upstream #1518/#1453, #1521, #1510/PR #1511). +//! +//! Each test drives the FULL extract → resolve pipeline over a committed fixture +//! under `tests/fixtures/tier6/`, so it exercises real tree-sitter extraction, +//! real import-mapping extraction and real multi-file resolution — behaviour a +//! mock `ResolutionContext` structurally cannot reach. +//! +//! The three cases are INDEPENDENT code paths and therefore separate fixtures, +//! so an isolated revert of one fix cannot be masked by another: +//! * `py_from_import` — `import_resolver.rs` Python `from`-import member +//! resolution (#1518). A test that only asserted "an edge exists" would pass +//! with the edge still bound to the DECOY, so every assertion here names the +//! TARGET FILE. +//! * `go_multi_module` — `context.rs` multi-`go.mod` module discovery (#1521). +//! * `ts_decl_initializer` — `walker.rs` declaration-initializer attribution +//! (#1510). Asserts the SOURCE of the edge is the declared symbol, not the +//! file node. +//! +//! Fixtures are read in place and never written to; the store lives in a temp +//! file, so the worktree stays clean. + +use std::path::{Path, PathBuf}; + +use codegraph_core::types::{Edge, EdgeKind, FileRecord, Node, NodeKind}; +use codegraph_extract::{detect_language, extract_file}; +use codegraph_resolve::ReferenceResolver; +use codegraph_store::Store; + +fn fixture_root(name: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("fixtures") + .join("tier6") + .join(name) +} + +fn temp_db_path(test_name: &str) -> PathBuf { + let mut path = std::env::temp_dir(); + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock after epoch") + .as_nanos(); + path.push(format!( + "codegraph-tier6-{test_name}-{}-{nanos}.db", + std::process::id() + )); + path +} + +/// Every indexable file under `root`, as project-relative `/`-separated paths, +/// sorted so indexing order is deterministic. +fn fixture_files(root: &Path) -> Vec { + let mut out = Vec::new(); + collect(root, root, &mut out); + out.sort(); + out +} + +fn collect(root: &Path, dir: &Path, out: &mut Vec) { + let entries = std::fs::read_dir(dir).expect("read fixture dir"); + for entry in entries { + let path = entry.expect("dir entry").path(); + if path.is_dir() { + collect(root, &path, out); + } else if let Ok(relative) = path.strip_prefix(root) { + out.push(relative.to_string_lossy().replace('\\', "/")); + } + } +} + +/// A resolved graph over one Tier 6 fixture: the store plus every non-`Contains` +/// edge the pipeline produced. +struct Resolved { + store: Store, + edges: Vec, +} + +impl Resolved { + fn node(&self, id: &str) -> Option { + self.store.node_by_id(id).expect("node lookup") + } + + /// `name@file` for a node id, or the raw id when it is not a node (a `file:` + /// id is a node too, so this only degrades for a dangling id). + fn label(&self, id: &str) -> String { + match self.node(id) { + Some(n) => format!("{:?}|{}@{}:L{}", n.kind, n.name, n.file_path, n.start_line), + None => id.to_string(), + } + } + + fn of_kind(&self, kind: EdgeKind) -> Vec<&Edge> { + self.edges.iter().filter(|e| e.kind == kind).collect() + } + + /// `source -> target at line` for every edge of `kind`, sorted. + fn described(&self, kind: EdgeKind) -> Vec { + let mut rows: Vec = self + .of_kind(kind) + .into_iter() + .map(|e| { + format!( + "{} -> {} at {:?}", + self.label(&e.source), + self.label(&e.target), + e.line + ) + }) + .collect(); + rows.sort(); + rows + } + + /// The FILE PATHS of every `kind` edge whose source node is named + /// `source_name` and whose target node is named `target_name`. Naming the + /// target's file is the whole point for #1518: an edge to the decoy and an + /// edge to the real target are both "an edge to `real_target`". + fn target_files(&self, kind: EdgeKind, source_name: &str, target_name: &str) -> Vec { + let mut out: Vec = self + .of_kind(kind) + .into_iter() + .filter_map(|e| { + let s = self.node(&e.source)?; + let t = self.node(&e.target)?; + (s.name == source_name && t.name == target_name).then_some(t.file_path) + }) + .collect(); + out.sort(); + out + } +} + +fn resolve_fixture(name: &str) -> Resolved { + let root = fixture_root(name); + assert!(root.is_dir(), "missing committed fixture {name}"); + let relative = fixture_files(&root); + assert!(!relative.is_empty(), "fixture {name} has no files"); + + let mut store = Store::open(&temp_db_path(name)).expect("open store"); + for path in &relative { + let language = detect_language(path); + let result = extract_file(&root, path).expect("extract file"); + store + .upsert_file(&FileRecord { + path: path.clone(), + content_hash: "fixture".to_string(), + language, + size: 0, + modified_at: 0, + indexed_at: 0, + node_count: result.nodes.len() as i64, + errors: Vec::new(), + }) + .expect("upsert file"); + store.upsert_nodes(&result.nodes).expect("upsert nodes"); + store + .insert_edges(&result.edges) + .expect("insert contains edges"); + store + .insert_unresolved_refs(&result.unresolved_references) + .expect("insert unresolved refs"); + } + + let root_str = root.to_string_lossy().to_string(); + let mut resolver = ReferenceResolver::new(root_str.clone()); + { + let context = codegraph_resolve::StoreResolutionContext::new(&store, &root_str); + resolver.initialize(&context); + } + resolver + .extract_and_persist_frameworks(&mut store, &relative) + .expect("framework extract"); + resolver + .resolve_and_persist(&mut store) + .expect("resolve and persist"); + resolver + .run_post_extract(&mut store) + .expect("run post extract"); + + let mut edges = Vec::new(); + for kind in NodeKind::ALL { + for node in store.nodes_by_kind(kind).expect("nodes by kind") { + for edge in store + .edges_by_source_kind(&node.id, None) + .expect("edges by source") + { + if edge.kind != EdgeKind::Contains { + edges.push(edge); + } + } + } + } + Resolved { store, edges } +} + +// ============ Item 20 — Python `from`-import member resolution (#1518) ======== + +#[test] +fn python_from_import_binds_to_the_imported_module_not_a_decoy() { + // Given `pkg/mod.py` and `pkg/decoy.py` BOTH defining `real_target`, and + // `app.py` doing `from pkg.mod import real_target`, + // When the pipeline resolves `caller`'s call, + // Then the edge must target the `real_target` in `pkg/mod.py` — the file the + // import explicitly names. Before the fix it bound to `pkg/decoy.py`, which + // an "edge exists" assertion would have accepted. + let g = resolve_fixture("py_from_import"); + let described = g.described(EdgeKind::Calls); + + assert_eq!( + g.target_files(EdgeKind::Calls, "caller", "real_target"), + vec!["pkg/mod.py".to_string()], + "`from pkg.mod import real_target` must bind to pkg/mod.py, not the \ + same-named decoy: {described:?}" + ); +} + +#[test] +fn python_from_import_alias_binds_to_the_imported_module() { + // The `as aliased` form emitted NO call edge at all before the fix, so this + // asserts both that an edge exists AND that it names pkg/mod.py. + let g = resolve_fixture("py_from_import"); + let described = g.described(EdgeKind::Calls); + + assert_eq!( + g.target_files(EdgeKind::Calls, "caller2", "real_target"), + vec!["pkg/mod.py".to_string()], + "`from pkg.mod import real_target as aliased` must bind the aliased call \ + to pkg/mod.py: {described:?}" + ); +} + +#[test] +fn python_from_import_never_reaches_the_decoy() { + // The negative half: no `Calls` edge anywhere in the fixture may land in + // `pkg/decoy.py`. Without this, a fix that ADDED the correct edge while + // KEEPING the wrong one would pass the two tests above. + let g = resolve_fixture("py_from_import"); + let to_decoy: Vec = g + .of_kind(EdgeKind::Calls) + .into_iter() + .filter(|e| { + g.node(&e.target) + .is_some_and(|t| t.file_path == "pkg/decoy.py") + }) + .map(|e| format!("{} -> {}", g.label(&e.source), g.label(&e.target))) + .collect(); + assert!( + to_decoy.is_empty(), + "no call may reach the decoy module: {to_decoy:?}" + ); +} + +// ============ Item 21 — Go cross-module calls (#1521) ========================= + +#[test] +fn go_cross_module_call_resolves_in_multi_go_mod_layout() { + // Given two modules (`a/go.mod` = `ex/a`, `b/go.mod` = `ex/b`) with NO root + // `go.mod`, and `b/main.go` calling `a.Helper()` after `import "ex/a"`, + // When the pipeline resolves, + // Then a `Calls` edge must reach `Helper` in `a/helper.go`. Before the fix + // the whole fixture produced ZERO `calls` edges, because module discovery + // only ever read `/go.mod`. + let g = resolve_fixture("go_multi_module"); + let described = g.described(EdgeKind::Calls); + + assert_eq!( + g.target_files(EdgeKind::Calls, "Caller", "Helper"), + vec!["a/helper.go".to_string()], + "`a.Helper()` across two modules must reach a/helper.go: {described:?}" + ); +} + +#[test] +fn go_cross_module_call_does_not_bind_the_same_named_decoy() { + // `b/local.go` defines its own exported `Helper`. The import names module + // `ex/a`, so the edge must NOT land on the caller's own package. + let g = resolve_fixture("go_multi_module"); + let described = g.described(EdgeKind::Calls); + let files = g.target_files(EdgeKind::Calls, "Caller", "Helper"); + assert!( + !files.contains(&"b/local.go".to_string()), + "the in-package same-named Helper must not win over the imported \ + module's: {described:?}" + ); +} + +// ============ Item 22 — declaration initializers (#1510 / PR #1511) ========== + +#[test] +fn ts_top_level_declaration_initializer_call_attributes_to_the_declaration() { + // Given `export const value = target();` at file scope, + // When the pipeline resolves, + // Then the `Calls` edge SOURCE must be the `value` constant, not `file:d.ts` + // — otherwise `codegraph callers target` answers with a filename. + let g = resolve_fixture("ts_decl_initializer"); + let described = g.described(EdgeKind::Calls); + + let sources: Vec = g + .of_kind(EdgeKind::Calls) + .into_iter() + .filter(|e| g.node(&e.target).is_some_and(|t| t.name == "target")) + .map(|e| { + let s = g.node(&e.source).expect("source node exists"); + format!("{:?}|{}", s.kind, s.name) + }) + .collect(); + + assert!( + sources.contains(&"Constant|value".to_string()), + "the initializer call must attribute to the `value` constant: \ + sources={sources:?} edges={described:?}" + ); + assert!( + !sources.iter().any(|s| s.starts_with("File|")), + "no initializer call may still attribute to the file node: \ + sources={sources:?} edges={described:?}" + ); +} + +#[test] +fn ts_declaration_initializer_fix_leaves_function_local_calls_alone() { + // The control: a `const inner = target()` INSIDE a function already + // attributes to that function. A fix that pushed the declaration node too + // eagerly would re-point this edge at a `constant:inner` node, so this test + // fails if the change leaks past file scope. + let g = resolve_fixture("ts_decl_initializer"); + let described = g.described(EdgeKind::Calls); + + let from_wrapper: Vec = g + .of_kind(EdgeKind::Calls) + .into_iter() + .filter_map(|e| { + let s = g.node(&e.source)?; + (s.name == "wrapper").then(|| self_label(&s)) + }) + .collect(); + assert_eq!( + from_wrapper.len(), + 1, + "the function-local `const inner = target()` must stay attributed to \ + `wrapper`: {described:?}" + ); +} + +fn self_label(n: &Node) -> String { + format!("{:?}|{}", n.kind, n.name) +} diff --git a/docs/cli.md b/docs/cli.md index 151a4991..45c3b63a 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -32,9 +32,9 @@ | `files` | List indexed files (tree/flat/grouped) | `-p`, `--filter `, `--language `, `--pattern`, `--format`, `--max-depth`, `-j` | | `serve` | Start the server; `--mcp` enters MCP stdio mode | `-p`, `--mcp`, `--no-watch` | | `unlock` | Clear a stale daemon lock (keeps live pids) | `[path]` | -| `callers` | Who calls a symbol (along calls/references/imports) | ``, `-p`, `-l`, `-j`, `--strict` | -| `callees` | What a symbol calls | ``, `-p`, `-l`, `-j`, `--strict` | -| `impact` | Blast radius of changing a symbol (incoming deps, transitive) | ``, `-p`, `-d/--depth`, `-j`, `--strict` | +| `callers` | Who calls a symbol (along calls/references/imports) | ``, `-p`, `-l`, `-j`, `--strict`, `--file ` | +| `callees` | What a symbol calls | ``, `-p`, `-l`, `-j`, `--strict`, `--file ` | +| `impact` | Blast radius of changing a symbol (incoming deps, transitive) | ``, `-p`, `-d/--depth`, `-j`, `--strict`, `--file ` | | `affected` | Given changed files, the affected symbol set | `[files...]`, `-p`, `-d/--depth`, `--filter` | | `check` | Detect circular dependencies (each cycle as `a.ts -> b.ts -> a.ts`) | `[path]`, `-j/--json` | | `audit` | Read-only Godot resource audit: orphan resources, dangling references, impact | `-p`, `--orphans`, `--dangling`, `--impact ` (≥1 required), `--verify-plan`, `--include `, `--exclude `, `-j/--json` | @@ -71,7 +71,9 @@ config file; `uninstall` reverses it. No hand-editing of JSON/TOML required. Supported agents (`ALL_TARGETS` order): **Claude Code, Cursor, Codex CLI, -opencode, Hermes Agent, Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed.** +opencode, Hermes Agent, Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed, +VS Code (`vscode`), GitHub Copilot CLI (`copilot-cli`), JetBrains +(`jetbrains`).** The written MCP command launches the Rust binary: `command: "codegraph"`, `args: ["serve", "--mcp"]` (Cursor injects `--path`; Kiro injects `--path` only on a project-local install). @@ -86,6 +88,23 @@ install). > global `--path ${workspaceFolder}` would resolve to a literal, non-existent > directory and break the watcher and catch-up sync. +> **The three GitHub Copilot targets.** They share the Copilot MCP surface but +> disagree on both the wrapper key and the available locations: +> +> | target | file | wrapper | locations | +> | ------------- | -------------------------------------------- | ------------ | -------------- | +> | `vscode` | `.vscode/mcp.json` (local) | `servers` | local + global | +> | | `/Code/User/mcp.json` (global) | `servers` | | +> | `copilot-cli` | `~/.copilot/mcp-config.json` | `mcpServers` | global only | +> | `jetbrains` | `~/.config/github-copilot/intellij/mcp.json` | `servers` | global only | +> +> The VS Code **global** entry is a bare `serve --mcp` and deliberately does NOT +> use `${workspaceFolder}`: VS Code expands that variable only in a WORKSPACE +> `mcp.json`, so in the user-level file it would stay literal and point the server +> at a nonexistent directory. Run `codegraph init --target=vscode` per project for +> live watch. The Copilot CLI entry additionally carries `"tools": ["*"]`, without +> which the CLI registers the server but exposes none of its tools. + ```bash codegraph install --yes # auto-detect installed agents, global codegraph install --target=claude,cursor --yes # explicit list @@ -364,6 +383,44 @@ also exits non-zero. | Exact match with results | Succeeds | Succeeds | | Exact match with zero results | Succeeds and prints the empty result | Fails | +### Ancestor-index retargeting on mutating commands + +`index`, `sync`, `uninit`, and `unlock` walk UP from the given path to find an +index, so running one inside an unindexed subdirectory operates on the nearest +indexed ANCESTOR. That is intentional — one index serves a whole tree — but it is +now announced on **stderr** instead of happening silently: + +``` +Warning: /repo/child has no CodeGraph index, so this command resolved to an + ancestor index at /repo and will operate on THAT project, not on /repo/child. + Run `codegraph init /repo/child` first if you meant to give it its own index. +``` + +It matters most for `uninit --force`, which would otherwise delete an index the +user never named. Stdout stays machine-readable and unchanged. + +--- + +### `--file` — disambiguating same-named definitions + +When two files define the same symbol, `callers` / `callees` / `impact` merge +both definitions' relatives into one list. `--file ` keeps only the +definition declared in that file: + +```bash +codegraph callers target --file src/alpha.ts # only alpha.ts's callers +codegraph callees target --file alpha.ts # a trailing path suffix works +codegraph impact target --file src/alpha.ts --json +``` + +The filter matches the whole project-relative path or any **segment-aligned** +trailing suffix, so `other.ts` never selects `my_other.ts`. Windows separators +and a leading `./` are normalized. + +A filter that matches no definition is an **error** naming the files that do +define the symbol — reporting an empty relative-set instead would read as "this +symbol is dead". In `--json`, the applied filter is echoed as `"file"`. + --- ## `codegraph impact` — edge counts in `--json` diff --git a/docs/mcp.md b/docs/mcp.md index cbd44ed2..330c979e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -85,7 +85,8 @@ regardless of cwd (e.g. `"args": ["serve", "--mcp", "-p", "/abs/path/to/project"]`). Supported agents: Claude Code, Cursor, Codex CLI, opencode, Hermes Agent, -Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed. +Gemini CLI, Antigravity IDE, Kiro, Trae, Qoder, Zed, VS Code (GitHub Copilot), +GitHub Copilot CLI, JetBrains IDEs (GitHub Copilot). --- diff --git a/reference/golden/mcp/initialize.json b/reference/golden/mcp/initialize.json index 807ec33d..08edce81 100644 --- a/reference/golden/mcp/initialize.json +++ b/reference/golden/mcp/initialize.json @@ -24,7 +24,7 @@ "name": "codegraph", "version": "0.12.0" }, - "instructions": "# Codegraph — code intelligence over an indexed knowledge graph\n\nCodegraph is a SQLite knowledge graph of every symbol, edge, and file in\nthe workspace — pre-computed structure you would otherwise re-derive by\nreading files (cached intelligence: thousands of parse/trace decisions you\ndon't pay to re-reason each run). Reads are sub-millisecond; the index lags\nwrites by ~1s through the file watcher. Reach for it BEFORE *and* while\nwriting or editing code — not just for questions: one call returns the\nverbatim source PLUS who calls it and what it affects, so you edit with the\nblast radius in view. More accurate context, in far fewer tokens and\nround-trips than reading files yourself.\n\n## Use codegraph instead of reading files — for questions AND edits\n\nWhether you're answering \"how does X work\" or implementing a change (fixing\na bug, adding a feature), reach for codegraph before you Read. For\nunderstanding, answer DIRECTLY — usually with ONE `codegraph_explore` call.\n`codegraph_explore` takes either a natural-language question or a bag of\nsymbol/file names and returns the verbatim source of the relevant symbols\ngrouped by file, so it is Read-equivalent and most often the ONLY\ncodegraph call you need. Codegraph IS the pre-built search index — so\ndelegating the lookup to a separate file-reading sub-task/agent, or\nrunning your own grep + read loop, repeats work codegraph already did and\ncosts more for the same answer. Reach for raw Read/Grep only to confirm a\nspecific detail codegraph didn't cover. A direct codegraph answer is\ntypically one to a few calls; a grep/read exploration is dozens.\n\n## Tool selection by intent\n\n- **Almost any question — \"how does X work\", architecture, a bug, \"what/where is X\", or surveying an area** → `codegraph_explore` (PRIMARY — call FIRST; ONE capped call returns the verbatim source of the relevant symbols grouped by file; most often the ONLY call you need)\n- **\"How does X reach/become Y? / the flow / the path from X to Y\"** → `codegraph_explore`, naming the symbols that span the flow (e.g. `mutateElement renderScene`) — it surfaces the call path among them, including dynamic-dispatch hops (callbacks, React re-render, JSX children) grep can't follow\n- **\"What is the symbol named X?\" (just its location)** → `codegraph_search`\n- **\"What calls this?\" / \"What does this call?\" / \"What would changing this break?\"** → `codegraph_callers` / `codegraph_callees` / `codegraph_impact`\n- **Reading a source FILE (any time you'd use the `Read` tool)** → `codegraph_node` with a `file` path and no `symbol`. It returns the file's **current source with line numbers — the same `\\t` shape `Read` gives you, safe to `Edit` from** — narrowable with `offset`/`limit` exactly like `Read`, PLUS a one-line note of which files depend on it. Same bytes as `Read`, faster (served from the index), with the blast radius attached. Use it **instead of `Read`** for indexed source files; fall back to `Read` only for what codegraph doesn't index (configs, docs). Pass `symbolsOnly: true` for just the file's structure.\n- **About to read or edit a symbol you can name** → `codegraph_node` with that `symbol` (SECONDARY — the after-explore depth tool): the verbatim source (`includeCode: true`) PLUS its caller/callee trail, so before changing it you see what calls it and what your edit would break. For an OVERLOADED name it returns EVERY matching definition's body in one call, so you never Read a file to find the right overload\n- **\"What's in directory X?\"** → `codegraph_files`\n- **\"Is the index ready / what's its size?\"** → `codegraph_status`\n\n## Common chains\n\n- **Flow / \"how does X reach Y\"**: ONE `codegraph_explore` with the symbol names spanning the flow — it surfaces the call path among them (riding dynamic-dispatch hops) AND returns their source. No need to reconstruct the path with `codegraph_search` + `codegraph_callers`.\n- **Onboarding / understanding any area**: ONE `codegraph_explore` is usually the whole answer. Only follow up — `codegraph_node` for a specific symbol — if something is still unclear.\n- **Refactor planning**: `codegraph_search` → `codegraph_callers` → `codegraph_impact`. The blast-radius answer comes from impact, not from walking callers manually.\n- **Debugging a regression**: `codegraph_callers` of the suspected symbol; widen with `codegraph_impact` if an unexpected call appears.\n\n## Anti-patterns\n\n- **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context.\n- **Don't grep first** when looking up a symbol by name — `codegraph_search` is faster and returns kind + location + signature.\n- **Don't chain `codegraph_search` + `codegraph_node`** to understand an area — ONE `codegraph_explore` returns the relevant symbols' source together in a single round-trip.\n- **Don't loop `codegraph_node` over many symbols** — one `codegraph_explore` call returns them all grouped by file, while each separate call re-reads the whole context and costs far more. Use `codegraph_node` for a single symbol.\n- **Don't reach for the `Read` tool on an indexed source file** — `codegraph_node` with a `file` reads it for you (same `\\t` source, `offset`/`limit` like Read, faster, with its blast radius), and with a `symbol` it returns the source plus the caller/callee trail. Reach for raw `Read` only for what codegraph doesn't index (configs, docs) or when the staleness banner flags a file as pending re-index.\n- **After editing, check the staleness banner.** When a tool response starts with \"⚠️ Some files referenced below were edited since the last index sync…\", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. `codegraph_status` also lists pending files under \"Pending sync\".\n\n## Limitations\n\n- If a tool reports the project isn't initialized, `.codegraph/` doesn't exist yet — offer to run `codegraph init -i` to build the index.\n- Index lags file writes by ~1 second.\n- Cross-file resolution is best-effort name matching; ambiguous calls may return multiple candidates.\n- No live correctness validation — that's still the TypeScript compiler / test suite / linter's job. Codegraph supplements those with structural context they don't have.\n" + "instructions": "# Codegraph — code intelligence over an indexed knowledge graph\n\nCodegraph is a SQLite knowledge graph of every symbol, edge, and file in\nthe workspace — pre-computed structure you would otherwise re-derive by\nreading files (cached intelligence: thousands of parse/trace decisions you\ndon't pay to re-reason each run). Reads are sub-millisecond; the index lags\nwrites by ~1s through the file watcher. Reach for it BEFORE *and* while\nwriting or editing code — not just for questions: one call returns the\nverbatim source PLUS who calls it and what it affects, so you edit with the\nblast radius in view. More accurate context, in far fewer tokens and\nround-trips than reading files yourself.\n\n## Use codegraph instead of reading files — for questions AND edits\n\nWhether you're answering \"how does X work\" or implementing a change (fixing\na bug, adding a feature), reach for codegraph before you Read. For\nunderstanding, answer DIRECTLY — usually with ONE `codegraph_explore` call.\n`codegraph_explore` takes either a natural-language question or a bag of\nsymbol/file names and returns the verbatim source of the relevant symbols\ngrouped by file, so it is Read-equivalent and most often the ONLY\ncodegraph call you need. Codegraph IS the pre-built search index — so\ndelegating the lookup to a separate file-reading sub-task/agent, or\nrunning your own grep + read loop, repeats work codegraph already did and\ncosts more for the same answer. Reach for raw Read/Grep only to confirm a\nspecific detail codegraph didn't cover. A direct codegraph answer is\ntypically one to a few calls; a grep/read exploration is dozens.\n\n## Tool selection by intent\n\n- **Almost any question — \"how does X work\", architecture, a bug, \"what/where is X\", or surveying an area** → `codegraph_explore` (PRIMARY — call FIRST; ONE capped call returns the verbatim source of the relevant symbols grouped by file; most often the ONLY call you need)\n- **\"How does X reach/become Y? / the flow / the path from X to Y\"** → `codegraph_explore`, naming the symbols that span the flow (e.g. `mutateElement renderScene`) — it surfaces the call path among them, including dynamic-dispatch hops (callbacks, React re-render, JSX children) grep can't follow\n- **\"What is the symbol named X?\" (just its location)** → `codegraph_search`\n- **\"What calls this?\" / \"What does this call?\" / \"What would changing this break?\"** → `codegraph_callers` / `codegraph_callees` / `codegraph_impact`\n- **Reading a source FILE (any time you'd use the `Read` tool)** → `codegraph_node` with a `file` path and no `symbol`. It returns the file's **current source with line numbers — the same `\\t` shape `Read` gives you, safe to `Edit` from** — narrowable with `offset`/`limit` exactly like `Read`, PLUS a one-line note of which files depend on it. Same bytes as `Read`, faster (served from the index), with the blast radius attached. Use it **instead of `Read`** for indexed source files; fall back to `Read` only for what codegraph doesn't index (configs, docs). Pass `symbolsOnly: true` for just the file's structure.\n- **About to read or edit a symbol you can name** → `codegraph_node` with that `symbol` (SECONDARY — the after-explore depth tool): the verbatim source (`includeCode: true`) PLUS its caller/callee trail, so before changing it you see what calls it and what your edit would break. For an OVERLOADED name it returns EVERY matching definition's body in one call, so you never Read a file to find the right overload\n- **\"What's in directory X?\"** → `codegraph_files`\n- **\"Is the index ready / what's its size?\"** → `codegraph_status`\n\n## Common chains\n\n- **Flow / \"how does X reach Y\"**: ONE `codegraph_explore` with the symbol names spanning the flow — it surfaces the call path among them (riding dynamic-dispatch hops) AND returns their source. No need to reconstruct the path with `codegraph_search` + `codegraph_callers`.\n- **Onboarding / understanding any area**: ONE `codegraph_explore` is usually the whole answer. Only follow up — `codegraph_node` for a specific symbol — if something is still unclear.\n- **Refactor planning**: `codegraph_search` → `codegraph_callers` → `codegraph_impact`. The blast-radius answer comes from impact, not from walking callers manually.\n- **Debugging a regression**: `codegraph_callers` of the suspected symbol; widen with `codegraph_impact` if an unexpected call appears.\n\n## Anti-patterns\n\n- **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context.\n- **Don't grep first** when looking up a symbol by name — `codegraph_search` is faster and returns kind + location + signature.\n- **Don't chain `codegraph_search` + `codegraph_node`** to understand an area — ONE `codegraph_explore` returns the relevant symbols' source together in a single round-trip.\n- **Don't loop `codegraph_node` over many symbols** — one `codegraph_explore` call returns them all grouped by file, while each separate call re-reads the whole context and costs far more. Use `codegraph_node` for a single symbol.\n- **Don't reach for the `Read` tool on an indexed source file** — `codegraph_node` with a `file` reads it for you (same `\\t` source, `offset`/`limit` like Read, faster, with its blast radius), and with a `symbol` it returns the source plus the caller/callee trail. Reach for raw `Read` only for what codegraph doesn't index (configs, docs) or when the staleness banner flags a file as pending re-index.\n- **After editing, check the staleness banner.** When a tool response starts with \"⚠️ Some files referenced below were edited since the last index sync…\", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. `codegraph_status` also lists pending files under \"Pending sync\".\n\n## Limitations\n\n- If a tool reports the project isn't initialized, `.codegraph/` doesn't exist yet — offer to run `codegraph init -i` to build the index.\n- Index lags file writes by ~1 second.\n- Cross-file resolution is best-effort name matching; ambiguous calls may return multiple candidates.\n- No live correctness validation — that's still the TypeScript compiler / test suite / linter's job. Codegraph supplements those with structural context they don't have.\n\n## Supported languages\n\nCodegraph extracts these 38 languages (typescript, javascript, tsx, jsx, arkts, python, go, rust, java, c, cpp, csharp, razor, php, ruby, swift, kotlin, dart, svelte, vue, astro, liquid, pascal, scala, lua, luau, objc, r, solidity, nix, terraform, erlang, cfml, yaml, twig, xml, properties, gdscript). A file outside this set is not indexed, so no codegraph tool can see it — reach for Read there.\n" } } }