Merge pull request #105 from Hmbown/fix/issue-101-path-routing
fix: @-mention path routing (#101)
This commit is contained in:
@@ -23,11 +23,12 @@
|
||||
|
||||
use std::fmt::Write;
|
||||
use std::io::Read;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::Path;
|
||||
|
||||
use ignore::WalkBuilder;
|
||||
|
||||
use crate::tui::app::App;
|
||||
use crate::working_set::Workspace;
|
||||
|
||||
/// Maximum number of `@`-mentions whose contents are inlined into one user
|
||||
/// message. Beyond this we stop appending blocks but the raw `@token` text
|
||||
@@ -301,17 +302,39 @@ fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option<Str
|
||||
|
||||
let mut blocks = Vec::new();
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
let ws = Workspace::new(workspace.to_path_buf());
|
||||
|
||||
for mention in mentions.into_iter().take(MAX_FILE_MENTIONS_PER_MESSAGE) {
|
||||
let path = resolve_mention_path(&mention, workspace);
|
||||
let display_path = path
|
||||
.canonicalize()
|
||||
.unwrap_or_else(|_| path.clone())
|
||||
.display()
|
||||
.to_string();
|
||||
// `Workspace::resolve` already returns absolute paths when the root
|
||||
// is absolute (TUI always runs from an absolute workspace), so we
|
||||
// skip `canonicalize()` here — it's per-mention I/O on the
|
||||
// message-send hot path. Accept the rare symlink-aliasing dedup
|
||||
// miss as the cost of avoiding a syscall (Gemini code-review).
|
||||
let (path, display_path, exists) = match ws.resolve(&mention) {
|
||||
Ok(p) => {
|
||||
let d = p.display().to_string();
|
||||
(p, d, true)
|
||||
}
|
||||
Err(p) => {
|
||||
let d = p.display().to_string();
|
||||
(p, d, false)
|
||||
}
|
||||
};
|
||||
|
||||
// Gate every block — including <missing-file> — through the dedup
|
||||
// set so a user typing the same non-existent file twice doesn't
|
||||
// waste tokens on duplicate missing-file blocks (Devin code-review).
|
||||
if !seen.insert(display_path.clone()) {
|
||||
continue;
|
||||
}
|
||||
blocks.push(render_file_mention_context(&mention, &path, &display_path));
|
||||
|
||||
if exists {
|
||||
blocks.push(render_file_mention_context(&mention, &path, &display_path));
|
||||
} else {
|
||||
blocks.push(format!(
|
||||
"<missing-file mention=\"@{mention}\" path=\"{display_path}\" />"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
if blocks.is_empty() {
|
||||
@@ -393,28 +416,6 @@ fn trim_unquoted_mention(raw: &str) -> &str {
|
||||
trimmed
|
||||
}
|
||||
|
||||
fn resolve_mention_path(raw_path: &str, workspace: &Path) -> PathBuf {
|
||||
let path = expand_mention_home(raw_path);
|
||||
if path.is_absolute() {
|
||||
path
|
||||
} else {
|
||||
workspace.join(path)
|
||||
}
|
||||
}
|
||||
|
||||
fn expand_mention_home(path: &str) -> PathBuf {
|
||||
if path == "~" {
|
||||
if let Some(home) = std::env::var_os("HOME") {
|
||||
return PathBuf::from(home);
|
||||
}
|
||||
} else if let Some(rest) = path.strip_prefix("~/")
|
||||
&& let Some(home) = std::env::var_os("HOME")
|
||||
{
|
||||
return PathBuf::from(home).join(rest);
|
||||
}
|
||||
PathBuf::from(path)
|
||||
}
|
||||
|
||||
fn render_file_mention_context(raw: &str, path: &Path, display_path: &str) -> String {
|
||||
if !path.exists() {
|
||||
return format!("<missing-file mention=\"@{raw}\" path=\"{display_path}\" />");
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
//! - pinned message indices that compaction should preserve
|
||||
|
||||
use crate::models::{ContentBlock, Message};
|
||||
use ignore::WalkBuilder;
|
||||
use regex::Regex;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::Value;
|
||||
@@ -16,6 +17,122 @@ use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::OnceLock;
|
||||
|
||||
/// Repo-aware resolver for `@`-mentions and file pickers.
|
||||
///
|
||||
/// `cwd` is captured at construction; if the host's current directory changes
|
||||
/// during a session, build a fresh `Workspace`. Fuzzy lookups are backed by a
|
||||
/// lazy basename → paths index built once on first miss and reused for the
|
||||
/// rest of the session — without it, every mis-typed mention triggered a full
|
||||
/// `WalkBuilder` traversal up to depth 6 (Gemini code-review feedback).
|
||||
#[derive(Debug)]
|
||||
pub struct Workspace {
|
||||
pub root: PathBuf,
|
||||
cwd: Option<PathBuf>,
|
||||
file_index: OnceLock<HashMap<String, Vec<PathBuf>>>,
|
||||
}
|
||||
|
||||
impl Workspace {
|
||||
pub fn new(root: PathBuf) -> Self {
|
||||
Self::with_cwd(root, std::env::current_dir().ok())
|
||||
}
|
||||
|
||||
/// Construct with an explicit cwd. Used by tests that need deterministic
|
||||
/// resolution against a known directory without depending on (and
|
||||
/// mutating) the process's real working directory.
|
||||
pub fn with_cwd(root: PathBuf, cwd: Option<PathBuf>) -> Self {
|
||||
Self {
|
||||
root,
|
||||
cwd,
|
||||
file_index: OnceLock::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Two-pass resolution: workspace, then cwd, then fuzzy fallback.
|
||||
pub fn resolve(&self, raw_path: &str) -> Result<PathBuf, PathBuf> {
|
||||
let path = expand_mention_home(raw_path);
|
||||
if path.is_absolute() {
|
||||
if path.exists() {
|
||||
return Ok(path);
|
||||
}
|
||||
return Err(path);
|
||||
}
|
||||
|
||||
let ws_path = self.root.join(&path);
|
||||
if ws_path.exists() {
|
||||
return Ok(ws_path);
|
||||
}
|
||||
|
||||
if let Some(cwd) = self.cwd.as_ref() {
|
||||
let cwd_path = cwd.join(&path);
|
||||
if cwd_path.exists() {
|
||||
return Ok(cwd_path);
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(fuzzy) = self.fuzzy_resolve(&path) {
|
||||
return Ok(fuzzy);
|
||||
}
|
||||
|
||||
Err(ws_path)
|
||||
}
|
||||
|
||||
fn fuzzy_resolve(&self, path: &Path) -> Option<PathBuf> {
|
||||
let needle = path.file_name()?.to_string_lossy().to_lowercase();
|
||||
if needle.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let index = self.file_index.get_or_init(|| self.build_file_index());
|
||||
index.get(&needle).and_then(|paths| paths.first()).cloned()
|
||||
}
|
||||
|
||||
fn build_file_index(&self) -> HashMap<String, Vec<PathBuf>> {
|
||||
let mut index: HashMap<String, Vec<PathBuf>> = HashMap::new();
|
||||
let mut builder = WalkBuilder::new(&self.root);
|
||||
builder.hidden(true).follow_links(false).max_depth(Some(6));
|
||||
|
||||
for entry in builder.build().flatten() {
|
||||
if entry
|
||||
.file_type()
|
||||
.is_some_and(|ft| ft.is_file() || ft.is_dir())
|
||||
{
|
||||
let name = entry.file_name().to_string_lossy().to_lowercase();
|
||||
index
|
||||
.entry(name)
|
||||
.or_default()
|
||||
.push(entry.path().to_path_buf());
|
||||
}
|
||||
}
|
||||
index
|
||||
}
|
||||
}
|
||||
|
||||
impl Clone for Workspace {
|
||||
fn clone(&self) -> Self {
|
||||
// Don't carry the cached file_index — clones get a fresh OnceLock so
|
||||
// they don't pin a stale snapshot of the previous owner's tree.
|
||||
Self {
|
||||
root: self.root.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
file_index: OnceLock::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn expand_mention_home(path: &str) -> PathBuf {
|
||||
if path == "~"
|
||||
&& let Some(home) = std::env::var_os("HOME")
|
||||
{
|
||||
return PathBuf::from(home);
|
||||
}
|
||||
if let Some(rest) = path.strip_prefix("~/")
|
||||
&& let Some(home) = std::env::var_os("HOME")
|
||||
{
|
||||
return PathBuf::from(home).join(rest);
|
||||
}
|
||||
PathBuf::from(path)
|
||||
}
|
||||
|
||||
/// Configuration for working-set tracking.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct WorkingSetConfig {
|
||||
@@ -790,4 +907,55 @@ mod tests {
|
||||
let messages = vec![make_message("user", "src/main.rs")];
|
||||
assert!(estimate_tokens(&messages) > 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workspace_resolve_respects_cwd_and_workspace() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
let sub = tmp.path().join("sub");
|
||||
std::fs::create_dir_all(&sub).unwrap();
|
||||
let bar = sub.join("bar.txt");
|
||||
std::fs::write(&bar, "bar").unwrap();
|
||||
|
||||
let nested = tmp.path().join("nested/deep");
|
||||
std::fs::create_dir_all(&nested).unwrap();
|
||||
let file_md = nested.join("file.md");
|
||||
std::fs::write(&file_md, "md").unwrap();
|
||||
|
||||
// Construct with an explicit cwd so the test doesn't race with other
|
||||
// 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.
|
||||
let res1 = ws.resolve("bar.txt").unwrap();
|
||||
assert_eq!(
|
||||
res1.canonicalize().unwrap_or(res1),
|
||||
bar.canonicalize().unwrap_or(bar)
|
||||
);
|
||||
|
||||
// Test 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),
|
||||
file_md.canonicalize().unwrap_or(file_md)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fuzzy_index_finds_files_and_directories() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
std::fs::create_dir_all(tmp.path().join("a/b/target_dir")).unwrap();
|
||||
std::fs::write(tmp.path().join("a/b/needle.rs"), "fn main(){}").unwrap();
|
||||
|
||||
let ws = Workspace::with_cwd(tmp.path().to_path_buf(), None);
|
||||
|
||||
// Basename-only mention triggers fuzzy fallback for both files and dirs.
|
||||
let f = ws.resolve("needle.rs").unwrap();
|
||||
assert!(f.ends_with("a/b/needle.rs"));
|
||||
let d = ws.resolve("target_dir").unwrap();
|
||||
assert!(d.ends_with("a/b/target_dir"));
|
||||
|
||||
// Index was populated exactly once (subsequent lookups reuse it).
|
||||
assert!(ws.file_index.get().is_some());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user