diff --git a/crates/tui/src/tui/file_mention.rs b/crates/tui/src/tui/file_mention.rs index ebb9efbb..0bee809a 100644 --- a/crates/tui/src/tui/file_mention.rs +++ b/crates/tui/src/tui/file_mention.rs @@ -23,9 +23,7 @@ use std::fmt::Write; use std::io::Read; -use std::path::Path; - -use ignore::WalkBuilder; +use std::path::{Path, PathBuf}; use crate::tui::app::App; use crate::working_set::Workspace; @@ -43,10 +41,6 @@ pub const MAX_DIRECTORY_MENTION_ENTRIES: usize = 80; /// the cost of walking large workspaces; subsequent keystrokes narrow further. const FILE_MENTION_COMPLETION_LIMIT: usize = 64; -/// Maximum directory depth walked when completing a file mention. Mirrors the -/// existing `project_tree` cutoff and keeps Tab snappy in deep monorepos. -const FILE_MENTION_COMPLETION_DEPTH: usize = 6; - // --------------------------------------------------------------------------- // Tab-completion // --------------------------------------------------------------------------- @@ -92,56 +86,31 @@ pub fn partial_file_mention_at_cursor(input: &str, cursor_chars: usize) -> Optio Some((byte_start, partial)) } -/// Walk the workspace and return relative paths whose representation matches -/// the partial mention. A file matches when its case-insensitive relative -/// path either starts with the partial or contains it as a substring; the -/// former rank earlier so a partial like `docs/de` resolves to -/// `docs/deepseek_v4.pdf` before any path that merely contains those bytes. -pub fn find_file_mention_completions(workspace: &Path, partial: &str, limit: usize) -> Vec { - if limit == 0 { - return Vec::new(); - } - let needle = partial.to_lowercase(); - let mut prefix_hits: Vec = Vec::new(); - let mut substring_hits: Vec = Vec::new(); +/// Cwd-aware completion entry point. Shares its walker with the future +/// Ctrl+P fuzzy picker (#97); see [`Workspace::completions`] for the +/// ranking + display rules. +pub fn find_file_mention_completions( + workspace: &Workspace, + partial: &str, + limit: usize, +) -> Vec { + let entries = workspace.completions(partial, limit); + tracing::debug!( + target: "deepseek_tui::file_mention", + partial = %partial, + workspace = %workspace.root.display(), + cwd = ?std::env::current_dir().ok(), + match_count = entries.len(), + "file mention completion walk", + ); + entries +} - let mut builder = WalkBuilder::new(workspace); - builder - .hidden(true) - .follow_links(false) - .max_depth(Some(FILE_MENTION_COMPLETION_DEPTH)); - - for entry in builder.build().flatten() { - if prefix_hits.len() + substring_hits.len() >= limit { - break; - } - let path = entry.path(); - let Ok(rel) = path.strip_prefix(workspace) else { - continue; - }; - let rel_str = rel.to_string_lossy().replace('\\', "/"); - if rel_str.is_empty() { - continue; - } - let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir()); - let candidate = if is_dir { - format!("{rel_str}/") - } else { - rel_str.clone() - }; - let lower = candidate.to_lowercase(); - if needle.is_empty() || lower.starts_with(&needle) { - prefix_hits.push(candidate); - } else if lower.contains(&needle) { - substring_hits.push(candidate); - } - } - - prefix_hits.sort(); - substring_hits.sort(); - prefix_hits.extend(substring_hits); - prefix_hits.truncate(limit); - prefix_hits +/// Build a `Workspace` for the running app: anchors at `app.workspace` and +/// captures the process CWD so the resolver and completion walker honor the +/// user's launch directory when it differs from `--workspace`. +fn workspace_for_app(app: &App) -> Workspace { + Workspace::with_cwd(app.workspace.clone(), std::env::current_dir().ok()) } /// Resolve the `@`-mention completion popup contents for the current @@ -169,7 +138,8 @@ pub fn visible_mention_menu_entries(app: &App, limit: usize) -> Vec { if limit == 0 { return Vec::new(); } - find_file_mention_completions(&app.workspace, &partial, limit) + let ws = workspace_for_app(app); + find_file_mention_completions(&ws, &partial, limit) } /// Apply the currently selected `@`-mention popup entry to the composer @@ -209,9 +179,8 @@ pub fn try_autocomplete_file_mention(app: &mut App) -> bool { else { return false; }; - let workspace = app.workspace.clone(); - let candidates = - find_file_mention_completions(&workspace, &partial, FILE_MENTION_COMPLETION_LIMIT); + let ws = workspace_for_app(app); + let candidates = find_file_mention_completions(&ws, &partial, FILE_MENTION_COMPLETION_LIMIT); if candidates.is_empty() { app.status_message = Some(format!("No files match @{partial}")); return true; @@ -287,14 +256,28 @@ pub fn longest_common_prefix<'a>(values: &[&'a str]) -> &'a str { /// Append a "Local context from @mentions" block to the user's message when /// any `@path` references are present. Returns the input unchanged when /// there are none. -pub fn user_request_with_file_mentions(input: &str, workspace: &Path) -> String { - let Some(context) = local_context_from_file_mentions(input, workspace) else { +/// +/// `cwd` carries the user's launch directory and drives the second +/// resolution pass (issue #101): relative `@` mentions resolve under +/// `cwd` when `workspace.join(path)` doesn't exist, so the user's mental +/// anchor (their shell's pwd) wins when it diverges from `--workspace`. +/// Pass `None` to disable the cwd pass entirely (workspace-only). +pub fn user_request_with_file_mentions( + input: &str, + workspace: &Path, + cwd: Option, +) -> String { + let Some(context) = local_context_from_file_mentions(input, workspace, cwd) else { return input.to_string(); }; format!("{input}\n\n---\n\nLocal context from @mentions:\n{context}") } -fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option { +fn local_context_from_file_mentions( + input: &str, + workspace: &Path, + cwd: Option, +) -> Option { let mentions = extract_file_mentions(input); if mentions.is_empty() { return None; @@ -302,7 +285,7 @@ fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option Option — through the dedup // set so a user typing the same non-existent file twice doesn't @@ -528,3 +520,115 @@ fn is_media_path(path: &Path) -> bool { | "mkv" ) } + +// --------------------------------------------------------------------------- +// #101 regression repros +// --------------------------------------------------------------------------- +// +// The bug being guarded: typing `@` resolved under `--workspace`, +// not the user's launch CWD. When the two diverged (the canonical case is +// `--workspace=/repo` with `pwd=/repo/sub`), every relative `@` token routed +// to the wrong root and the prompt got `` blocks. +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + /// #101 regression — workspace-vs-cwd divergence: `@bar.txt` typed from + /// the cwd `/sub` MUST resolve to `/sub/bar.txt`, never to + /// `/bar.txt` (which doesn't exist). + #[test] + fn cwd_pass_resolves_when_workspace_pass_misses() { + let tmp = TempDir::new().expect("tempdir"); + let sub = tmp.path().join("sub"); + std::fs::create_dir_all(&sub).expect("mkdir"); + let bar = sub.join("bar.txt"); + std::fs::write(&bar, "hello bar").expect("write bar"); + + let content = + user_request_with_file_mentions("look at @bar.txt", tmp.path(), Some(sub.clone())); + + // The block must reference the cwd-rooted path with the file's body — + // and crucially it must NOT collapse to . + assert!( + content.contains("hello bar"), + "expected file body to be inlined; got: {content}", + ); + assert!( + !content.contains(" for a path that exists under cwd; got: {content}", + ); + let bar_disp = bar.display().to_string(); + assert!( + content.contains(&bar_disp), + "expected resolved path {bar_disp} in content; got: {content}", + ); + // Belt-and-suspenders: the workspace-rooted path doesn't exist and + // must not appear in the rendered attribute. + let wrong = tmp.path().join("bar.txt").display().to_string(); + assert!( + !content.contains(&format!("path=\"{wrong}\"")), + "should NOT have routed to {wrong}; got: {content}", + ); + } + + /// #101 regression — nested workspace path: `@nested/deep/file.md` with + /// the file at workspace root resolves through the workspace pass. + #[test] + fn workspace_pass_resolves_nested_path() { + let tmp = TempDir::new().expect("tempdir"); + let nested = tmp.path().join("nested/deep"); + std::fs::create_dir_all(&nested).expect("mkdir"); + let file_md = nested.join("file.md"); + std::fs::write(&file_md, "# nested deep").expect("write file_md"); + + // Cwd is irrelevant; an unrelated tempdir would do. Pass `None` so we + // are unambiguously testing the workspace-pass path. + let content = user_request_with_file_mentions("see @nested/deep/file.md", tmp.path(), None); + + assert!(content.contains("# nested deep"), "got: {content}"); + assert!(!content.contains("` block for a resolvable + /// mention must include the expected attributes and contents, and must + /// NOT contain ``. + #[test] + fn resolvable_mention_renders_file_block_not_missing_file() { + let tmp = TempDir::new().expect("tempdir"); + std::fs::write(tmp.path().join("guide.md"), "# Guide\nUse the fast path.\n") + .expect("write"); + + let content = user_request_with_file_mentions("read @guide.md", tmp.path(), None); + + // Header + tag presence. + assert!(content.contains("Local context from @mentions:")); + assert!(content.contains(""), "got: {content}"); + // The bug fingerprint MUST be absent. + assert!(!content.contains("` + /// so the user gets an explicit signal instead of silent failure. + #[test] + fn truly_missing_mention_still_renders_missing_file() { + let tmp = TempDir::new().expect("tempdir"); + + let content = user_request_with_file_mentions( + "huh @does/not/exist.txt", + tmp.path(), + Some(tmp.path().to_path_buf()), + ); + + assert!( + content.contains(" QueuedMessage { } fn queued_message_content_for_app(app: &App, message: &QueuedMessage) -> String { - let user_request = - crate::tui::file_mention::user_request_with_file_mentions(&message.display, &app.workspace); + // Pass the process CWD explicitly so the resolver's two-pass logic can + // honor the user's launch directory when it differs from `--workspace` + // (issue #101 — file mentions silently routing to the wrong root). + let user_request = crate::tui::file_mention::user_request_with_file_mentions( + &message.display, + &app.workspace, + std::env::current_dir().ok(), + ); if let Some(skill_instruction) = message.skill_instruction.as_ref() { format!("{skill_instruction}\n\n---\n\nUser request: {user_request}") } else { diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 473aaec3..4243633c 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -6,6 +6,7 @@ use crate::tui::file_mention::{ }; use crate::tui::history::{GenericToolCell, HistoryCell, ToolCell, ToolStatus}; use crate::tui::views::{ModalView, ViewAction}; +use crate::working_set::Workspace; use std::path::PathBuf; use std::process::Command; use tempfile::TempDir; @@ -158,7 +159,7 @@ fn file_mentions_do_not_trigger_inside_email_addresses() { let tmpdir = TempDir::new().expect("tempdir"); std::fs::write(tmpdir.path().join("example.com"), "not a mention").expect("write file"); - let content = user_request_with_file_mentions("email me@example.com", tmpdir.path()); + let content = user_request_with_file_mentions("email me@example.com", tmpdir.path(), None); assert_eq!(content, "email me@example.com"); } @@ -168,7 +169,7 @@ fn media_file_mentions_point_to_attach_instead_of_inlining_bytes() { let tmpdir = TempDir::new().expect("tempdir"); std::fs::write(tmpdir.path().join("photo.png"), b"\0png").expect("write image"); - let content = user_request_with_file_mentions("inspect @photo.png", tmpdir.path()); + let content = user_request_with_file_mentions("inspect @photo.png", tmpdir.path(), None); assert!(content.contains(" Self { Self::with_cwd(root, std::env::current_dir().ok()) } @@ -90,6 +95,9 @@ impl Workspace { let mut index: HashMap> = HashMap::new(); let mut builder = WalkBuilder::new(&self.root); builder.hidden(true).follow_links(false).max_depth(Some(6)); + // Honor `.deepseekignore` in addition to the defaults the `ignore` crate + // already respects (`.gitignore`, `.git/info/exclude`, `.ignore`). + let _ = builder.add_custom_ignore_filename(".deepseekignore"); for entry in builder.build().flatten() { if entry @@ -105,6 +113,121 @@ impl Workspace { } index } + + /// Walk the workspace (and the recorded `cwd` when it diverges) and + /// return relative paths whose representation matches `partial`. + /// + /// Ranking: a candidate matches when its case-insensitive display string + /// starts with `partial` (prefix hit) or contains it as a substring; prefix + /// hits sort first so `docs/de` lands `docs/deepseek_v4.pdf` ahead of any + /// path that merely shares those bytes. + /// + /// Display strings are workspace-relative for files under `root`, and + /// cwd-relative for files only under the recorded `cwd` — so what the user + /// Tab-completes matches what their shell would have shown them. + /// + /// Honors `.gitignore`, `.git/info/exclude`, `.ignore`, and + /// `.deepseekignore`. Capped at `limit` results. + #[must_use] + pub fn completions(&self, partial: &str, limit: usize) -> Vec { + if limit == 0 { + return Vec::new(); + } + let needle = partial.to_lowercase(); + let mut prefix_hits: Vec = Vec::new(); + let mut substring_hits: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + // Walk the recorded cwd first when it diverges from the workspace + // root, so cwd-relative entries appear ahead of duplicates surfaced by + // the workspace walk. + let cwd_diverges = self + .cwd + .as_deref() + .map(|c| c != self.root.as_path()) + .unwrap_or(false); + if cwd_diverges && let Some(cwd) = self.cwd.as_deref() { + walk_for_completions( + cwd, + cwd, + &needle, + limit, + &mut prefix_hits, + &mut substring_hits, + &mut seen, + ); + } + walk_for_completions( + &self.root, + &self.root, + &needle, + limit, + &mut prefix_hits, + &mut substring_hits, + &mut seen, + ); + + prefix_hits.sort(); + substring_hits.sort(); + prefix_hits.extend(substring_hits); + prefix_hits.truncate(limit); + prefix_hits + } +} + +/// Maximum directory depth walked when surfacing file-mention completions. +/// Mirrors the existing `project_tree` cutoff and keeps Tab snappy in deep +/// monorepos. +const COMPLETIONS_WALK_DEPTH: usize = 6; + +#[allow(clippy::too_many_arguments)] +fn walk_for_completions( + walk_root: &Path, + display_root: &Path, + needle: &str, + limit: usize, + prefix_hits: &mut Vec, + substring_hits: &mut Vec, + seen: &mut HashSet, +) { + let mut builder = WalkBuilder::new(walk_root); + builder + .hidden(true) + .follow_links(false) + .max_depth(Some(COMPLETIONS_WALK_DEPTH)); + let _ = builder.add_custom_ignore_filename(".deepseekignore"); + + for entry in builder.build().flatten() { + if prefix_hits.len() + substring_hits.len() >= limit { + break; + } + let path = entry.path(); + let Ok(rel) = path.strip_prefix(display_root) else { + continue; + }; + let rel_str = rel.to_string_lossy().replace('\\', "/"); + if rel_str.is_empty() { + continue; + } + // Dedup across the (cwd, workspace) double-walk by absolute path; we + // want the cwd-relative display when both walks see the same file. + let abs = path.to_path_buf(); + if !seen.insert(abs) { + continue; + } + let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir()); + let candidate = if is_dir { + format!("{rel_str}/") + } else { + rel_str.clone() + }; + let lower = candidate.to_lowercase(); + if needle.is_empty() || lower.starts_with(needle) { + prefix_hits.push(candidate); + } else if lower.contains(needle) { + substring_hits.push(candidate); + } + } } impl Clone for Workspace { @@ -926,14 +1049,17 @@ mod tests { // tests that mutate the real process cwd. let ws = Workspace::with_cwd(tmp.path().to_path_buf(), Some(sub.clone())); - // Test 1: @bar.txt with cwd=sub → resolves via the cwd pass. + // #101 repro #1: @bar.txt with cwd=sub MUST resolve via the cwd pass, + // never to the bogus workspace path tmp/bar.txt (which doesn't exist). let res1 = ws.resolve("bar.txt").unwrap(); assert_eq!( - res1.canonicalize().unwrap_or(res1), - bar.canonicalize().unwrap_or(bar) + res1.canonicalize().unwrap_or(res1.clone()), + bar.canonicalize().unwrap_or(bar.clone()) ); + let wrong = tmp.path().join("bar.txt"); + assert_ne!(res1, wrong, "must not have routed to workspace fallback"); - // Test 2: @nested/deep/file.md → falls through to workspace root. + // #101 repro #2: @nested/deep/file.md falls through to workspace root. let res2 = ws.resolve("nested/deep/file.md").unwrap(); assert_eq!( res2.canonicalize().unwrap_or(res2), @@ -941,6 +1067,45 @@ mod tests { ); } + /// Negative test (#101): a truly missing path returns `Err` with a path + /// that callers can show to the user as a signal of failure. + #[test] + fn workspace_resolve_returns_err_for_truly_missing_path() { + let tmp = TempDir::new().unwrap(); + let ws = Workspace::with_cwd(tmp.path().to_path_buf(), Some(tmp.path().to_path_buf())); + + let res = ws.resolve("does/not/exist.txt"); + assert!(res.is_err(), "expected Err for missing path, got: {res:?}"); + } + + /// `Workspace::completions` returns workspace-relative entries for files + /// under the root, and cwd-relative entries when the cwd-only file lives + /// outside the workspace tree. Honors `.gitignore`. + #[test] + fn workspace_completions_walk_surfaces_workspace_and_cwd() { + let tmp = TempDir::new().unwrap(); + // Two trees: a workspace under `ws/` and a cwd under `cwd/` that is + // NOT inside the workspace, so the two walks are disjoint and we can + // assert each branch contributed. + let ws_root = tmp.path().join("ws"); + let cwd_root = tmp.path().join("cwd"); + std::fs::create_dir_all(&ws_root).unwrap(); + std::fs::create_dir_all(&cwd_root).unwrap(); + std::fs::write(ws_root.join("alpha.txt"), "a").unwrap(); + std::fs::write(cwd_root.join("alphabeta.txt"), "b").unwrap(); + + let ws = Workspace::with_cwd(ws_root.clone(), Some(cwd_root.clone())); + let entries = ws.completions("alpha", 16); + assert!( + entries.iter().any(|e| e == "alpha.txt"), + "expected workspace entry alpha.txt; got: {entries:?}", + ); + assert!( + entries.iter().any(|e| e == "alphabeta.txt"), + "expected cwd entry alphabeta.txt; got: {entries:?}", + ); + } + #[test] fn fuzzy_index_finds_files_and_directories() { let tmp = TempDir::new().unwrap();