From 4ca6369924cada2b9262696595da70bb7b3b516d Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Wed, 14 Oct 2026 23:15:03 +0000 Subject: [PATCH 1/8] Wait for the watch set in git engine tests instead of reading it once A status snapshot can reflect an index written after its pass reconciled the watch set; the set catches up on the pass that index event triggers. Tests that asserted the set right after the snapshot raced that pass. The debug hook now signals each published set, and the tests wait on it. --- crates/git/src/lib.rs | 4 +++- crates/git/src/state.rs | 29 +++++++++++++++++++++++------ crates/git/tests/engine.rs | 31 +++++++++++++++++-------------- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/crates/git/src/lib.rs b/crates/git/src/lib.rs index 2e0fbdbb..59b09caf 100644 --- a/crates/git/src/lib.rs +++ b/crates/git/src/lib.rs @@ -28,7 +28,9 @@ mod state; pub use state::{StateHandle, StateOptions}; #[doc(hidden)] -pub use state::{debug_engine_refs, debug_status_recomputes, debug_worktree_watches}; +pub use state::{ + debug_engine_refs, debug_status_recomputes, debug_wait_worktree_watches, debug_worktree_watches, +}; /// The slash-separated Git path of `abs` relative to `workdir`, if any. /// On Windows this converts backslashes so that ignore rules and index diff --git a/crates/git/src/state.rs b/crates/git/src/state.rs index 13b51336..33ae7bf1 100644 --- a/crates/git/src/state.rs +++ b/crates/git/src/state.rs @@ -15,7 +15,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{Receiver, RecvTimeoutError, SyncSender}; -use std::sync::{Arc, Mutex, OnceLock, Weak}; +use std::sync::{Arc, Condvar, Mutex, OnceLock, Weak}; use std::time::{Duration, Instant}; use crate::model::{ @@ -223,11 +223,27 @@ fn status_recomputes() -> &'static Mutex> { /// arm. Test/diagnostic hook. #[doc(hidden)] pub fn debug_worktree_watches(gitdir: &Path) -> Option> { - worktree_watch_sets().lock().unwrap().get(gitdir).cloned() + worktree_watch_sets().0.lock().unwrap().get(gitdir).cloned() } -fn worktree_watch_sets() -> &'static Mutex>> { - static SETS: OnceLock>>> = OnceLock::new(); +/// Blocks until the engine publishes a watch set for `gitdir` that +/// satisfies `done`. Test/diagnostic hook. +#[doc(hidden)] +pub fn debug_wait_worktree_watches(gitdir: &Path, mut done: impl FnMut(&[PathBuf]) -> bool) { + let (sets, published) = worktree_watch_sets(); + drop( + published + .wait_while(sets.lock().unwrap(), |sets| { + !done(sets.get(gitdir).map_or(&[], Vec::as_slice)) + }) + .unwrap(), + ); +} + +type WatchSets = (Mutex>>, Condvar); + +fn worktree_watch_sets() -> &'static WatchSets { + static SETS: OnceLock = OnceLock::new(); SETS.get_or_init(Default::default) } @@ -1241,10 +1257,11 @@ impl Engine { } arms.watch_set_changed = false; let dirs = arms.worktree_dirs.iter().cloned().collect(); - worktree_watch_sets() - .lock() + let (sets, published) = worktree_watch_sets(); + sets.lock() .unwrap() .insert((*self.repo.gitdir).clone(), dirs); + published.notify_all(); } /// Directories to arm for the ignore sources that live *outside* every diff --git a/crates/git/tests/engine.rs b/crates/git/tests/engine.rs index 4e60fd2d..d8069321 100644 --- a/crates/git/tests/engine.rs +++ b/crates/git/tests/engine.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use std::path::PathBuf; use std::process::Command; use std::sync::atomic::{AtomicU64, Ordering}; -use std::time::{Duration, Instant}; +use std::time::Duration; use yas_git::native::{ self, BlameRecord, DiffRecord, DiffRequest, Endpoint, LogRequest, PatchRecord, PatchRequest, @@ -79,6 +79,19 @@ impl Repository { .map(|path| std::fs::canonicalize(path).unwrap()) .collect() } + + /// Waits for the engine to arm or disarm `dir`. The status snapshot can + /// reflect an index written after its pass reconciled the watch set, so + /// the set catches up on the pass that index event triggers. + fn wait_for_watch(&self, dir: &str, armed: bool) { + let path = self.0.join(dir); + yas_git::debug_wait_worktree_watches(&self.0.join(".git"), |watches| { + watches + .iter() + .any(|watch| std::fs::canonicalize(watch).is_ok_and(|watch| watch == path)) + == armed + }); + } } impl Drop for Repository { @@ -466,13 +479,13 @@ fn index_changes_reconcile_tracked_exceptions_to_ignore_pruning() { repository.git(&["add", "-f", "generated/nested/tracked"]); assert_eq!(state.status()["generated/nested/tracked"], (b'A', b' ')); - assert!(watches().contains(&repository.0.join("generated/nested"))); + repository.wait_for_watch("generated/nested", true); repository.write("generated/nested/tracked", b"modified after force-add\n"); assert_eq!(state.status()["generated/nested/tracked"], (b'A', b'M')); repository.git(&["rm", "--cached", "-f", "generated/nested/tracked"]); assert!(state.status().is_empty()); - assert!(!watches().contains(&repository.0.join("generated"))); + repository.wait_for_watch("generated", false); } #[test] @@ -594,17 +607,7 @@ fn reattached_status_selection_observes_changes_made_while_pruned() { // Wait for the asynchronous detach to disarm this directory. Writes // inside it then cannot invalidate a cached ignored-status segment. - let deadline = Instant::now() + Duration::from_secs(5); - while repository - .worktree_watches() - .contains(&repository.0.join("ignored")) - { - assert!( - Instant::now() < deadline, - "ignored directory was not pruned" - ); - std::thread::sleep(Duration::from_millis(5)); - } + repository.wait_for_watch("ignored", false); repository.write("ignored/second", b"new ignored file\n"); let broader = StateStream::new(&handle, status_options(true, true)); assert_eq!(broader.status().len(), 2); From 592560fa89d3c0adf2f8ae8f55d1bc86a493072c Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Thu, 15 Oct 2026 01:13:16 +0000 Subject: [PATCH 2/8] Wait for PowerShell's first output without a deadline in residue tests Windows PowerShell 5.1 can take over 20 s to print its first line when the runner starts it cold under the parallel suite, which failed read_until's 20 s timeout. read_until now waits for the next event; an early exit still fails the test. The residue grace is measured from the first output, so a slow start no longer counts against it. --- crates/server/src/yas_process.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/server/src/yas_process.rs b/crates/server/src/yas_process.rs index 712c1959..c027e6aa 100644 --- a/crates/server/src/yas_process.rs +++ b/crates/server/src/yas_process.rs @@ -2704,11 +2704,7 @@ mod windows_tests { ) -> T { let mut output = String::new(); loop { - match tokio::time::timeout(Duration::from_secs(20), attachment.next()) - .await - .expect("output in time") - .expect("the attachment is open") - { + match attachment.next().await.expect("the attachment is open") { Event::Output { stream, data, .. } => { *acked += data.len() as u64; attachment.acknowledge_output(stream, *acked).unwrap(); @@ -2955,7 +2951,6 @@ mod windows_tests { let server = Server::new(false, true); let runtime = Runtime::new(server.clone()); let session = runtime.session([34; 16], None).unwrap(); - let started = std::time::Instant::now(); let mut attachment = session .spawn( &with_residue_grace( @@ -2975,6 +2970,7 @@ mod windows_tests { .unwrap(); let mut acked = 0; let pid = read_until(&mut attachment, &mut acked, residue_pid).await; + let started = std::time::Instant::now(); let exit = exit_of(&mut attachment, &mut acked, Duration::from_secs(20)).await; assert_eq!( (exit.kind, exit.code), From d9a4396f546d233c87b5c26e08de443d189cfd1c Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 20:50:59 +0000 Subject: [PATCH 3/8] git: send the first real state as the WATCH snapshot run_git_watch opened every subscription with an empty snapshot and only delivered repository state as a later Reset. One-shot `yas git status` printed that empty snapshot, so it always said clean. The first native snapshot now is the initial snapshot. --- crates/server/src/yas.rs | 152 +++++++++++++++++++++++++++++++++------ 1 file changed, 132 insertions(+), 20 deletions(-) diff --git a/crates/server/src/yas.rs b/crates/server/src/yas.rs index 95fba9de..8dc44835 100644 --- a/crates/server/src/yas.rs +++ b/crates/server/src/yas.rs @@ -32440,25 +32440,7 @@ async fn run_git_watch( journal: Option, ) { let mut sent_bytes = 0u64; - if send_git_snapshot( - subscription_id, - revision, - Vec::new(), - event_limit, - &mut sent_bytes, - &control, - &out, - &cancellation, - journal.as_ref(), - ) - .await - .is_err() - { - return; - } - if let Some(journal) = &journal { - journal.state(revision, 0, sent_bytes); - } + let mut initial = true; loop { let event = tokio::select! { event = watch.next() => event, @@ -32482,9 +32464,32 @@ async fn run_git_watch( return; } Ok(super::yas_git_adapter::WatchEvent::Snapshot { state_id, records }) => { - let next = revision.saturating_add(1).max(1); let before = sent_bytes; let count = records.len(); + if std::mem::take(&mut initial) { + if send_git_snapshot( + subscription_id, + revision, + records, + event_limit, + &mut sent_bytes, + &control, + &out, + &cancellation, + journal.as_ref(), + ) + .await + .is_err() + { + return; + } + if let Some(journal) = &journal { + journal.state(revision, count, sent_bytes - before); + } + watch.acknowledge(state_id); + continue; + } + let next = revision.saturating_add(1).max(1); if send_git_state_event( subscription_id, Phase::Reset, @@ -40662,6 +40667,113 @@ mod tests { (client, codec, server_hello, task) } + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn git_watch_initial_snapshot_carries_worktree_status() { + let root = tempfile::tempdir().unwrap(); + let git = |args: &[&str]| { + let status = std::process::Command::new("git") + .args(["-c", "user.name=t", "-c", "user.email=t@t"]) + .args(args) + .current_dir(root.path()) + .status() + .unwrap(); + assert!(status.success()); + }; + git(&["init", "-q"]); + std::fs::write(root.path().join("hello.txt"), b"hello\n").unwrap(); + git(&["add", "hello.txt"]); + git(&["commit", "-qm", "init"]); + std::fs::write(root.path().join("hello.txt"), b"changed\n").unwrap(); + + let state = super::super::tests::process_transport::test_state( + super::super::process::Server::new(false, true), + ); + let (mut client, codec, _, server_task) = + start_registered_session(state, &[family::TRANSFER, family::GIT]).await; + write_request( + &mut client, + &codec, + family::GIT, + yas_git_wire::request_kind::OPEN, + 10, + &yas_git_wire::Open { + source: yas_git_wire::RepositorySource::PlatformPath( + root.path().to_string_lossy().as_bytes().to_vec(), + ), + extensions: Extensions::default(), + }, + ) + .await; + let opened = next_sensitive_result( + &mut client, + &codec, + family::GIT, + yas_git_wire::request_kind::OPEN, + 10, + ) + .await; + assert_eq!(opened.status, Status::Ok); + let repository_handle = yas_git_wire::OpenResult::decode(&opened.body) + .unwrap() + .repository_handle; + write_request( + &mut client, + &codec, + family::GIT, + yas_git_wire::request_kind::WATCH, + 11, + &yas_git_wire::Watch { + repository_handle, + datasets: (yas_wire::schema::git::WATCH_HEAD | yas_wire::schema::git::WATCH_STATUS) + as u16, + state: Watch { + initial_credit: 1024 * 1024, + resume: None, + extensions: Extensions::default(), + }, + }, + ) + .await; + let watched = next_sensitive_result( + &mut client, + &codec, + family::GIT, + yas_git_wire::request_kind::WATCH, + 11, + ) + .await; + assert_eq!(watched.status, Status::Ok); + let mut entities = Vec::new(); + loop { + let frame = timeout(TEST_TIMEOUT, next_frame(&mut client, &codec)) + .await + .unwrap(); + assert_eq!(frame.header.kind, yas_git_wire::event_kind::STATE); + let event = StateEvent::decode(&frame.payload).unwrap(); + assert_ne!(event.phase, Phase::Reset); + for record in &event.records { + entities.push(yas_git_wire::EntityRecord::decode(&record.body).unwrap()); + } + if event.phase == Phase::SnapshotEnd { + break; + } + } + assert!( + entities + .iter() + .any(|entity| matches!(entity.body, yas_git_wire::EntityBody::Head(_))) + ); + assert!( + entities + .iter() + .any(|entity| matches!(entity.body, yas_git_wire::EntityBody::Status(_))) + ); + + drop(client); + timeout(TEST_TIMEOUT, server_task).await.unwrap().unwrap(); + } + #[cfg(unix)] #[tokio::test] async fn extension_installation_support_matches_server_policy() { From 47fe408afc5bcec0b8d767cfb916228ed489d13e Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 20:50:59 +0000 Subject: [PATCH 4/8] git: text PATCH between index and worktree A PATCH_TEXT answer needs a content object and neither INDEX nor WORKTREE names one, so `yas git diff -p` failed with Unsupported. Use the null object ID for patches between two mutable endpoints. --- crates/server/src/yas_git.rs | 51 +++++++++++++++++++++++++++++++----- docs/design/git.md | 5 +++- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/crates/server/src/yas_git.rs b/crates/server/src/yas_git.rs index 9f629be9..1790b7a9 100644 --- a/crates/server/src/yas_git.rs +++ b/crates/server/src/yas_git.rs @@ -1081,14 +1081,12 @@ fn run_query( .map_err(native_error)?; let records = match response { yas_git::native::PatchResult::Text(data) => { - let object = endpoint_content_object(right) + let object = match endpoint_content_object(right) .or_else(|| endpoint_content_object(left)) - .ok_or_else(|| { - Error::new( - Status::Unsupported, - "text patch between mutable endpoints has no content object", - ) - })?; + { + Some(object) => object, + None => null_object(object_algorithm)?, + }; let byte_len = data.len() as u64; return Ok(query_page( vec![QueryItem::Content(OwnedContent { @@ -1896,6 +1894,20 @@ fn native_endpoint(endpoint: &wire::QueryEndpoint) -> Result Result { + let len = match object_algorithm { + value if value == yas_wire::schema::git::OBJECT_SHA1 as u8 => 20, + value if value == yas_wire::schema::git::OBJECT_SHA256 as u8 => 32, + _ => return Err(Error::new(Status::Internal, "unknown Git object algorithm")), + }; + Ok(wire::ObjectId { + algorithm: object_algorithm, + bytes: vec![0; len], + }) +} + fn endpoint_content_object(endpoint: &wire::QueryEndpoint) -> Option { match endpoint { wire::QueryEndpoint::Commit(object) @@ -2274,6 +2286,31 @@ mod tests { .any(|record| matches!(record, QueryItem::Record(wire::QueryRecord::PatchRow(_)))) ); + let text = run_query( + Some(&repository), + algorithm, + &query(wire::QueryBody::Patch { + left: wire::QueryEndpoint::Index, + right: wire::QueryEndpoint::Worktree, + path: None, + context_lines: 3, + rename_threshold: 50, + max_bytes: 1024 * 1024, + flags: yas_wire::schema::git::PATCH_TEXT as u16, + }), + None, + &cancel, + ) + .unwrap(); + match text.records.as_slice() { + [QueryItem::Content(OwnedContent { object, bytes, .. })] => { + assert!(object.bytes.iter().all(|byte| *byte == 0)); + let text = String::from_utf8_lossy(bytes); + assert!(text.contains("-hello\n+hello native YAS\n"), "{text}"); + } + other => panic!("unexpected index/worktree patch page: {other:?}"), + } + for body in [ wire::QueryBody::Index { path: None, diff --git a/docs/design/git.md b/docs/design/git.md index 7e24b825..936bf93f 100644 --- a/docs/design/git.md +++ b/docs/design/git.md @@ -205,7 +205,10 @@ the tree via gix) with intraline span refinement on modified line pairs β€” word- or character-granular, over raw or whitespace-normalized text, per request flags; binary detection short-circuits to `BINARY`. The row records are engine-agnostic by design, so a syntax-aware engine can -replace the alignment later, purely server-side. +replace the alignment later, purely server-side. A `PATCH_TEXT` answer's +content object is the right endpoint's object, else the left's; between two +mutable endpoints (INDEX, WORKTREE, EMPTY) it is the null object ID, as +`git diff` prints for the worktree side. Nothing runs under the session mutex; Results and Events interleave with Terminal, Surface, Media, and FS traffic through the per-session writer. From 8e40ef7d06c650dd53a09d3e8e99ece44e4a5ad6 Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 20:50:59 +0000 Subject: [PATCH 5/8] cli: git show without a path, and client-relative --repo `yas git show REV` sent a BLOB query for the commit and failed with Invalid. It now prints the commit in `git cat-file commit` layout (from the LOG record), and lists a directory for REV:DIR and REV:. --repo and `git discover PATH` were sent verbatim, so relative paths (including the default `.`) resolved against the server's cwd. They now resolve against the client's cwd, like fs and lsp --root. --- crates/cli/src/cli.rs | 29 +++--- crates/cli/src/learn.md | 6 +- crates/cli/src/yas_git.rs | 186 +++++++++++++++++++++++++++++++++++--- 3 files changed, 194 insertions(+), 27 deletions(-) diff --git a/crates/cli/src/cli.rs b/crates/cli/src/cli.rs index a5fef7f7..19201616 100644 --- a/crates/cli/src/cli.rs +++ b/crates/cli/src/cli.rs @@ -2198,7 +2198,7 @@ pub enum FsCommand { pub enum GitCommand { /// Branch, ahead/behind, stash, and working-tree status Status { - /// Repository location on the server (default: server cwd) + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2228,7 +2228,7 @@ pub enum GitCommand { #[arg(last = true)] pathspec: Vec, - /// Repository location on the server (default: server cwd) + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2284,7 +2284,7 @@ pub enum GitCommand { #[arg(last = true)] pathspec: Vec, - /// Repository location on the server (default: server cwd) + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2318,12 +2318,15 @@ pub enum GitCommand { /// yas git show HEAD:src/main.rs # a file at a revision /// yas git show v1.0:Cargo.toml /// yas git show HEAD # the commit object itself + /// yas git show HEAD:src # a directory, listed like ls-tree Show { - /// REV[:PATH]. Omit PATH for the commit object; omit REV - /// (`:path`) for HEAD. + /// REV[:PATH]. REV names a commit; reach trees and blobs by PATH. + /// Omit PATH for the commit object (in `git cat-file commit` + /// layout); a directory PATH is listed like ls-tree, and `REV:` + /// lists the root tree. Omit REV (`:path`) for HEAD. spec: String, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2340,7 +2343,7 @@ pub enum GitCommand { /// REV[:PATH]; omit PATH for the root tree spec: String, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2358,7 +2361,7 @@ pub enum GitCommand { #[arg(default_value = "")] path: String, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2375,7 +2378,7 @@ pub enum GitCommand { #[arg(required = true, num_args = 2..)] revs: Vec, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2393,7 +2396,7 @@ pub enum GitCommand { /// File to blame path: String, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2428,7 +2431,7 @@ pub enum GitCommand { #[arg(default_value = "")] ref_name: String, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, @@ -2450,7 +2453,7 @@ pub enum GitCommand { /// TSV: WORKDIRGITDIR. Deduped by gitdir, so several paths /// resolving to one repository report once. Discover { - /// Directory to search (default: server cwd) + /// Directory to search (relative to the client's cwd) #[arg(default_value = ".")] path: String, @@ -2484,7 +2487,7 @@ pub enum GitCommand { /// Refspecs to fetch (default: the remote's configured ones) refspecs: Vec, - /// Repository path on the server + /// Repository path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] repo: String, diff --git a/crates/cli/src/learn.md b/crates/cli/src/learn.md index dc85b32f..ee992bb1 100644 --- a/crates/cli/src/learn.md +++ b/crates/cli/src/learn.md @@ -293,7 +293,8 @@ yas fs ln -s target link # symlink (omit -s for a hard link) ## Git Read-only introspection of repositories on the server. `--repo` picks the -worktree (default: cwd); `--json` emits NDJSON. +worktree; a relative path, and the default `.`, resolve against the client's +cwd, as `--root` does for `fs` and `lsp`. `--json` emits NDJSON. ```bash yas git status # branch, ahead/behind, stash, worktree @@ -302,7 +303,7 @@ yas git log # history, newest first yas git log v1.0 # from a tag yas git log main..feature # a range yas git log --follow -- src/main.rs -yas git diff # unstaged +yas git diff # unstaged (-p for hunks) yas git diff --staged # staged yas git diff main dev # between two commits yas git diff main...dev # since they diverged (from the merge base) @@ -310,6 +311,7 @@ yas git diff --merge-base main # worktree vs where main forked (a `base` lin yas git diff HEAD~2 -p -- src # with hunks, limited to a path yas git show HEAD:src/main.rs # a file's bytes at a revision yas git show HEAD # the commit object itself +yas git show HEAD:src # a directory, listed like ls-tree yas git ls-tree HEAD # one tree level (MODE TYPE OIDNAME) yas git ls-tree HEAD:src # descend by passing a path yas git merge-base main feature # best common ancestors (exit 1 if unrelated) diff --git a/crates/cli/src/yas_git.rs b/crates/cli/src/yas_git.rs index d4a60d71..57ff4413 100644 --- a/crates/cli/src/yas_git.rs +++ b/crates/cli/src/yas_git.rs @@ -40,7 +40,7 @@ impl Repository { family::GIT, git::request_kind::OPEN, &Open { - source: RepositorySource::PlatformPath(path.as_bytes().to_vec()), + source: RepositorySource::PlatformPath(client_path(path)), extensions: Extensions::default(), }, true, @@ -612,10 +612,29 @@ async fn cmd_show( let mut repository = Repository::open(on, hub, &repo).await?; let (revision, path) = split_revision_path(&spec); let object = repository.resolve_one(revision).await?; - let records = repository + match path { + None => { + let commit = commit_object(&mut repository, &object) + .await + .ok_or_else(|| format!("'{revision}' did not resolve to a commit"))?; + std::io::stdout() + .write_all(&commit) + .map_err(|error| format!("writing stdout: {error}"))?; + repository.close().await?; + return Ok(0); + } + Some("") => { + let records = tree_entries(&mut repository, object, "").await?; + print_tree_entries(records, false); + repository.close().await?; + return Ok(0); + } + Some(_) => {} + } + let records = match repository .query( QueryBody::Blob { - object, + object: object.clone(), path: path.map(fs_path).transpose()?, offset: 0, max_bytes: max_len, @@ -624,7 +643,21 @@ async fn cmd_show( 1, 1, ) - .await?; + .await + { + Ok(records) => records, + Err(error) => { + let Some(path) = path else { + return Err(error); + }; + let Ok(records) = tree_entries(&mut repository, object, path).await else { + return Err(error); + }; + print_tree_entries(records, false); + repository.close().await?; + return Ok(0); + } + }; let content = records.into_iter().find_map(|record| match record { QueryRecord::Blob(content) => Some(content), _ => None, @@ -651,18 +684,30 @@ async fn cmd_ls_tree( let mut repository = Repository::open(on, hub, &repo).await?; let (revision, path) = split_revision_path(&spec); let tree = repository.resolve_one(revision).await?; - let records = repository + let records = tree_entries(&mut repository, tree, path.unwrap_or("")).await?; + print_tree_entries(records, json); + repository.close().await?; + Ok(0) +} + +async fn tree_entries( + repository: &mut Repository, + tree: ObjectId, + path: &str, +) -> Result, String> { + repository .query( QueryBody::Tree { tree, - path: path.map(fs_path).transpose()?.unwrap_or(FsPath { - components: Vec::new(), - }), + path: fs_path(path)?, }, git::MAX_QUERY_RECORDS as u16, MAX_COLLECTED_RECORDS, ) - .await?; + .await +} + +fn print_tree_entries(records: Vec, json: bool) { for record in records { if let QueryRecord::TreeEntry(entry) = record { let kind = match entry.entry_kind { @@ -685,8 +730,70 @@ async fn cmd_ls_tree( } } } - repository.close().await?; - Ok(0) +} + +/// The commit object in `git cat-file commit` layout, rebuilt from the LOG +/// record because BLOB serves only blobs. +async fn commit_object(repository: &mut Repository, object: &ObjectId) -> Option> { + let records = repository + .query( + QueryBody::Log { + spec: object_hex(object).into_bytes(), + tips: Vec::new(), + hides: Vec::new(), + path: None, + flags: yas_wire::schema::git::LOG_FULL_MESSAGE as u16, + }, + 1, + 1, + ) + .await + .ok()?; + records.into_iter().find_map(|record| match record { + QueryRecord::Commit(commit) => Some(render_commit(&commit)), + _ => None, + }) +} + +fn render_commit(commit: &git::CommitRecord) -> Vec { + fn signature(out: &mut Vec, role: &str, name: &[u8], email: &[u8], time: i64, tz: i16) { + out.extend_from_slice(role.as_bytes()); + out.push(b' '); + out.extend_from_slice(name); + out.extend_from_slice(b" <"); + out.extend_from_slice(email); + let sign = if tz < 0 { '-' } else { '+' }; + let minutes = tz.unsigned_abs(); + out.extend_from_slice( + format!("> {time} {sign}{:02}{:02}\n", minutes / 60, minutes % 60).as_bytes(), + ); + } + let mut out = format!("tree {}\n", object_hex(&commit.tree)).into_bytes(); + for parent in &commit.parents { + out.extend_from_slice(format!("parent {}\n", object_hex(parent)).as_bytes()); + } + signature( + &mut out, + "author", + &commit.author_name, + &commit.author_email, + commit.authored_unix_seconds, + commit.author_timezone_minutes, + ); + signature( + &mut out, + "committer", + &commit.committer_name, + &commit.committer_email, + commit.committed_unix_seconds, + commit.committer_timezone_minutes, + ); + out.push(b'\n'); + out.extend_from_slice(&commit.message); + if !commit.message.ends_with(b"\n") { + out.push(b'\n'); + } + out } async fn cmd_ls_files( @@ -886,7 +993,7 @@ async fn cmd_discover( &mut client, 0, QueryBody::Discover { - source: RepositorySource::PlatformPath(path.into_bytes()), + source: RepositorySource::PlatformPath(client_path(&path)), max_depth: u16::from(depth), flags, }, @@ -1179,6 +1286,15 @@ fn print_diff(diff: &git::DiffRecord, json: bool) { } } +/// Relative repository paths name the client's working directory, as FS and +/// LSP roots do. +fn client_path(path: &str) -> Vec { + std::path::absolute(path) + .map(|path| path.to_string_lossy().into_owned()) + .unwrap_or_else(|_| path.to_string()) + .into_bytes() +} + fn fs_path(path: &str) -> Result { let normalized = path.replace('\\', "/"); if normalized.starts_with('/') { @@ -1266,6 +1382,52 @@ fn wire_error(error: impl std::fmt::Display) -> String { mod tests { use super::*; + #[test] + fn commit_object_uses_cat_file_layout() { + let oid = |byte| ObjectId { + algorithm: yas_wire::schema::git::OBJECT_SHA1 as u8, + bytes: vec![byte; 20], + }; + let commit = git::CommitRecord { + flags: 0, + object: oid(1), + tree: oid(0xab), + parents: vec![oid(0xcd)], + authored_unix_seconds: 1_700_000_000, + author_timezone_minutes: -330, + committed_unix_seconds: 1_700_000_100, + committer_timezone_minutes: 60, + author_name: b"A U".to_vec(), + author_email: b"a@example.com".to_vec(), + committer_name: b"C".to_vec(), + committer_email: b"c@example.com".to_vec(), + message: b"subject\n\nbody".to_vec(), + }; + assert_eq!( + String::from_utf8(render_commit(&commit)).unwrap(), + format!( + "tree {}\nparent {}\nauthor A U 1700000000 -0530\n\ + committer C 1700000100 +0100\n\nsubject\n\nbody\n", + "ab".repeat(20), + "cd".repeat(20) + ) + ); + } + + #[test] + fn relative_repository_paths_resolve_against_client_cwd() { + let cwd = std::env::current_dir().unwrap(); + assert_eq!(client_path("."), cwd.to_string_lossy().as_bytes()); + assert_eq!( + client_path("sub"), + cwd.join("sub").to_string_lossy().as_bytes() + ); + #[cfg(unix)] + assert_eq!(client_path("/abs/repo"), b"/abs/repo"); + #[cfg(windows)] + assert_eq!(client_path(r"C:\abs\repo"), br"C:\abs\repo"); + } + #[test] fn revision_path_and_ranges_match_git_cli_spellings() { assert_eq!( From c03729dda5532882a31b8737d0c8de339311db52 Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 21:08:48 +0000 Subject: [PATCH 6/8] lsp: project the hover LOCATION before its MARKUP The server family pairs a native LOCATION with the MARKUP that follows it into one HoverRecord, but translate::hover pushed the markup first, so every hover failed with "LSP markup has no target location" (surfaced as Internal: no detail). Push the location first, as one group, and fall back to the empty range at the queried position when the language server reports no range. --- crates/lsp/src/backend.rs | 8 ++- crates/lsp/src/tests.rs | 100 ++++++++++++++++++++++++++++++++++++ crates/lsp/src/translate.rs | 41 ++++++++++----- 3 files changed, 134 insertions(+), 15 deletions(-) diff --git a/crates/lsp/src/backend.rs b/crates/lsp/src/backend.rs index c06ee018..e29db36a 100644 --- a/crates/lsp/src/backend.rs +++ b/crates/lsp/src/backend.rs @@ -564,6 +564,8 @@ struct PendingQuery { sub: u64, nonce: u16, kind: u8, + line: u32, + col: u32, path: Option, sink: native::QuerySink, } @@ -2096,6 +2098,8 @@ impl Engine { sub, nonce, kind, + line, + col, path: path.clone(), sink, }; @@ -2546,7 +2550,7 @@ fn project_query(job: ProjectionJob) { } LSP_QUERY_HOVER => { let path = q.path.clone().unwrap_or_default(); - translate::hover(&mut sink, &mut src, &path, &result, enc) + translate::hover(&mut sink, &mut src, &path, (q.line, q.col), &result, enc) } LSP_QUERY_DOC_SYMBOLS => { let path = q.path.clone().unwrap_or_default(); @@ -2695,6 +2699,8 @@ mod tests { sub: 1, nonce, kind: LSP_QUERY_DEFINITION, + line: 0, + col: 0, path: None, sink, } diff --git a/crates/lsp/src/tests.rs b/crates/lsp/src/tests.rs index 1d5743ee..82c0df39 100644 --- a/crates/lsp/src/tests.rs +++ b/crates/lsp/src/tests.rs @@ -959,6 +959,106 @@ fn definition_transcodes_utf16_to_bytes() { } } +/// HOVER projects the described LOCATION before its MARKUP; the server +/// family pairs them in that order. Without a reported range the target +/// is the empty range at the queried position. +#[test] +fn hover_location_precedes_markup() { + let root = tmp_root("hover"); + std::fs::write(root.join("a.rs"), "x\naΓ©π„žb\n").unwrap(); + let serve = |mut reader: BufReader>, mut writer: Box| { + while let Some(msg) = rpc::read_msg(&mut reader) { + match msg { + rpc::RpcMsg::Request { id, method, params } => { + let reply = match method.as_str() { + "initialize" => rpc::response( + &id, + json!({ "capabilities": { + "positionEncoding": "utf-16", + "hoverProvider": true, + } }), + ), + "shutdown" => rpc::response(&id, Value::Null), + "textDocument/hover" if params["position"]["line"] == 1 => rpc::response( + &id, + json!({ + "contents": { "kind": "markdown", "value": "**Γ©**" }, + "range": { "start": { "line": 1, "character": 1 }, + "end": { "line": 1, "character": 2 } }, + }), + ), + "textDocument/hover" => { + rpc::response(&id, json!({ "contents": "plain x" })) + } + _ => rpc::error_response(&id, -32601, "unhandled in fake"), + }; + let _ = rpc::write_msg(writer.as_mut(), &reply); + } + rpc::RpcMsg::Notification { method, .. } => { + if method == "exit" { + return; + } + } + rpc::RpcMsg::Response { .. } => {} + } + } + }; + let backend = testutil::pipe_backend(test_spec(), root.clone(), test_budgets(), serve); + wait_ready(&backend); + let att = attach(&root, &backend, 0, dummy_sink()); + + let (sink, rx) = collector(); + run_query(&att, 5, LSP_QUERY_HOVER, 0, 1, 1, "a.rs", "", &sink); + let records = wait_for(&rx, |msg| { + query_response(msg) + .filter(|r| r.nonce == 5) + .map(|r| r.records) + }); + match &query_records(&records).collect::>()[..] { + [ + QueryRecord::Location { + line, + column, + end_line, + end_column, + path, + .. + }, + QueryRecord::Markup { markdown, text }, + ] => { + assert_eq!((*line, *column, *end_line, *end_column), (1, 1, 1, 3)); + assert_eq!(path, &root.join("a.rs")); + assert!(*markdown); + assert_eq!(text, "**Γ©**"); + } + other => panic!("unexpected ranged hover records: {other:?}"), + } + + let (sink, rx) = collector(); + run_query(&att, 6, LSP_QUERY_HOVER, 0, 0, 0, "a.rs", "", &sink); + let records = wait_for(&rx, |msg| { + query_response(msg) + .filter(|r| r.nonce == 6) + .map(|r| r.records) + }); + match &query_records(&records).collect::>()[..] { + [ + QueryRecord::Location { + line, + column, + end_line, + end_column, + .. + }, + QueryRecord::Markup { text, .. }, + ] => { + assert_eq!((*line, *column, *end_line, *end_column), (0, 0, 0, 0)); + assert_eq!(text, "plain x"); + } + other => panic!("unexpected rangeless hover records: {other:?}"), + } +} + /// yas advertises `definition.linkSupport`, so rust-analyzer and gopls /// answer with `LocationLink[]` (`targetUri` + `targetSelectionRange`, /// with `targetRange` the fallback) rather than plain `Location[]`. That diff --git a/crates/lsp/src/translate.rs b/crates/lsp/src/translate.rs index 914074b5..4cf7da52 100644 --- a/crates/lsp/src/translate.rs +++ b/crates/lsp/src/translate.rs @@ -369,12 +369,14 @@ pub fn locations( } /// `textDocument/hover`: `contents` is `MarkupContent | MarkedString | -/// MarkedString[]`; everything becomes one markup record (plus a -/// `LOCATION` for the hovered range when the server reports one). +/// MarkedString[]`; everything becomes one markup record, preceded by the +/// `LOCATION` it describes: the hovered range when the server reports one, +/// else the empty range at the queried position `(line, col)`. pub fn hover( sink: &mut RecordSink<'_>, src: &mut TextSource, query_path: &Path, + (line, col): (u32, u32), result: &Value, enc: PositionEncoding, ) { @@ -412,24 +414,35 @@ pub fn hover( if body.is_empty() { return; } - sink.push(&ProjectedRecord::Markup { - format, - text: &body, - }); - if result["range"].is_object() - && let Some(s) = src.lookup(query_path) - { - let range = range_to_native(&result["range"], s, enc); - sink.push(&ProjectedRecord::Location { + let here = NativeRange { + line, + col, + end_line: line, + end_col: col, + }; + let reported = result["range"].is_object().then(|| &result["range"]); + let (range, hash) = match src.lookup(query_path) { + Some(s) => ( + reported.map_or(here, |range| range_to_native(range, s, enc)), + s.hash(), + ), + None => (reported.map_or(here, raw_range), LSP_HASH_NONE), + }; + sink.push_group(&[ + ProjectedRecord::Location { flags: 0, - hash: s.hash(), + hash, line: range.line, col: range.col, end_line: range.end_line, end_col: range.end_col, path: query_path, - }); - } + }, + ProjectedRecord::Markup { + format, + text: &body, + }, + ]); } fn symbol_flags(item: &Value) -> u8 { From 9c56bd1e8aae739fc318b8499eadfd5c9f8804c6 Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 21:08:48 +0000 Subject: [PATCH 7/8] lsp: diagnostics one-shot and --wait report published diagnostics run_lsp_watch sent a buffer-only initial snapshot and delivered the engine's retained diagnostics only as a later Reset, so one-shot `yas lsp diagnostics` always printed nothing and exited 0. The initial snapshot of a diagnostics subscription now waits for the engine's first diagnostics replay. --wait now waits for indexing, then for the next published diagnostics, ending after 5 seconds without any server update so a clean workspace still returns. Help: lsp --root is relative to the client's cwd, not the server's; learn.md names the actual `Busy: Warming` error. --- crates/cli/src/cli.rs | 21 +++++++++--------- crates/cli/src/learn.md | 11 ++++++---- crates/cli/src/yas_lsp.rs | 42 ++++++++++++++++++++++++++++++++---- crates/server/src/yas.rs | 40 +++++++++++++++++++++++++++++++--- crates/server/src/yas_lsp.rs | 4 ++++ docs/design/lsp.md | 4 +++- 6 files changed, 100 insertions(+), 22 deletions(-) diff --git a/crates/cli/src/cli.rs b/crates/cli/src/cli.rs index 19201616..d37c68e1 100644 --- a/crates/cli/src/cli.rs +++ b/crates/cli/src/cli.rs @@ -2602,7 +2602,7 @@ pub enum LspCommand { /// Position, 1-based (e.g. src/main.rs:10:4) spec: String, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2620,7 +2620,7 @@ pub enum LspCommand { #[arg(long)] declaration: bool, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2634,7 +2634,7 @@ pub enum LspCommand { /// Position, 1-based (e.g. src/main.rs:10:4) spec: String, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2651,7 +2651,7 @@ pub enum LspCommand { /// Position, 1-based (e.g. src/main.rs:10:4) spec: String, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2668,7 +2668,7 @@ pub enum LspCommand { /// Position, 1-based (e.g. src/main.rs:10:4) spec: String, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2687,7 +2687,7 @@ pub enum LspCommand { #[arg(long)] file: Option, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2708,11 +2708,12 @@ pub enum LspCommand { #[arg(long)] watch: bool, - /// Wait for language servers to finish indexing first + /// Wait for language servers to finish indexing, then for the next + /// published diagnostics (or 5 seconds without any server update) #[arg(long)] wait: bool, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2730,7 +2731,7 @@ pub enum LspCommand { /// The new name new_name: String, - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, @@ -2741,7 +2742,7 @@ pub enum LspCommand { /// Block until the workspace's language servers are ready Wait { - /// Workspace location on the server (default: server cwd) + /// Workspace path on the server (relative to the client's cwd) #[arg(long, default_value = ".")] root: String, diff --git a/crates/cli/src/learn.md b/crates/cli/src/learn.md index ee992bb1..120e2147 100644 --- a/crates/cli/src/learn.md +++ b/crates/cli/src/learn.md @@ -337,12 +337,13 @@ having refused one refspec of several). Language servers (rust-analyzer, gopls, clangd, …) are discovered by project markers, spawned on the server, and stay warm across invocations. Positions are 1-based PATH:LINE:COL; all commands take -`--root` (default: server cwd) and `--json` (NDJSON). +`--root` (default: the client's cwd; relative paths resolve against it) and +`--json` (NDJSON). ```bash yas lsp wait # block until servers finish indexing yas lsp diag # current diagnostics (exit 1 if any) -yas lsp diag --wait # settle first β€” use after editing files +yas lsp diag --wait # wait for the next publish β€” use after editing files yas lsp def src/main.rs:10:4 # definition of the symbol at 10:4 yas lsp refs src/main.rs:10:4 # references (--declaration to include it) yas lsp hover src/main.rs:10:4 # type and docs @@ -354,9 +355,11 @@ yas lsp rename src/main.rs:10:4 nm # rename plan (prints edits, never applies) yas lsp list # running servers (ref, phase, memory) ``` -A first call in a fresh workspace may exit 2 with "warming up" β€” run +A first call in a fresh workspace may exit 2 with `Busy: Warming` β€” run `yas lsp wait` once, then query. The edit loop: change files, then -`yas lsp diag --wait` to see resulting errors. +`yas lsp diag --wait` to see resulting errors. `--wait` waits for indexing, +then for the next published diagnostics, or 5 seconds without any server +update when nothing changes. ## Key/value store diff --git a/crates/cli/src/yas_lsp.rs b/crates/cli/src/yas_lsp.rs index 0fadc81e..26c7bedb 100644 --- a/crates/cli/src/yas_lsp.rs +++ b/crates/cli/src/yas_lsp.rs @@ -24,6 +24,7 @@ use crate::{cli::LspCommand, yas_native::NativeClient}; const STATE_CREDIT: u64 = yas_wire::schema::transport::RECOMMENDED_BUFFERED; const QUERY_CREDIT: u64 = lsp::MAX_QUERY_BYTES as u64; const MAX_COLLECTED_RECORDS: usize = 65_536; +const DIAGNOSTICS_LULL: Duration = Duration::from_secs(5); struct Workspace { client: NativeClient, @@ -431,7 +432,14 @@ async fn cmd_diagnostics( lsp::request_kind::WATCH, &Watch { workspace_handle: workspace.handle, - datasets: yas_wire::schema::lsp::WATCH_DIAGNOSTICS as u16, + // Backend updates carry indexing and check progress, which + // keep --wait from mistaking a running check for a lull. + datasets: if wait { + (yas_wire::schema::lsp::WATCH_DIAGNOSTICS + | yas_wire::schema::lsp::WATCH_BACKEND) as u16 + } else { + yas_wire::schema::lsp::WATCH_DIAGNOSTICS as u16 + }, state: yas_wire::state::Watch { initial_credit: STATE_CREDIT, resume: None, @@ -444,11 +452,25 @@ async fn cmd_diagnostics( let mut diagnostics = BTreeMap::::new(); let mut snapshot_done = false; let mut cumulative_credit = STATE_CREDIT; + let mut awaiting_publish = wait && !watch_forever; + let mut baseline: Option> = None; loop { - let frame = workspace + let next = workspace .client - .next_matching_event(family::LSP, lsp::event_kind::STATE) - .await?; + .next_matching_event(family::LSP, lsp::event_kind::STATE); + let frame = if awaiting_publish && snapshot_done { + match tokio::time::timeout(DIAGNOSTICS_LULL, next).await { + Ok(frame) => frame?, + // A clean workspace publishes nothing distinguishable, so a + // lull with no updates at all ends the wait. + Err(_) => { + let count = print_diagnostics(&diagnostics, filter.as_deref(), json); + return Ok(if count == 0 { 0 } else { 1 }); + } + } + } else { + next.await? + }; let event = StateEvent::decode(&frame.payload).map_err(wire_error)?; if event.subscription_id != result.subscription_id { continue; @@ -503,6 +525,18 @@ async fn cmd_diagnostics( } if event.phase == Phase::SnapshotEnd || (snapshot_done && event.phase == Phase::Delta) { snapshot_done = true; + if awaiting_publish { + // Retained diagnostics may predate the edit being checked; + // wait for a publish that changes them. + match &baseline { + None => { + baseline = Some(diagnostics.clone()); + continue; + } + Some(baseline) if *baseline == diagnostics => continue, + Some(_) => awaiting_publish = false, + } + } let count = print_diagnostics(&diagnostics, filter.as_deref(), json); if !watch_forever { workspace diff --git a/crates/server/src/yas.rs b/crates/server/src/yas.rs index 8dc44835..66e1d566 100644 --- a/crates/server/src/yas.rs +++ b/crates/server/src/yas.rs @@ -32549,12 +32549,43 @@ async fn run_lsp_watch( let mut revision = 1u64; let mut backend = Vec::new(); let mut diagnostics = Vec::new(); + // The engine replays its retained diagnostics as the first Diagnostics + // update, so waiting for it lets the initial snapshot carry them rather + // than an empty set that a one-shot reader would take as clean. + let mut initial_acks = Vec::new(); + if watch.wants_diagnostics() { + loop { + let event = tokio::select! { + event = watch.next() => event, + _ = cancellation.cancelled() => return, + }; + let Some(Ok(super::yas_lsp_adapter::WatchEvent::Snapshot { + stream, + update_id, + entities, + })) = event + else { + return; + }; + initial_acks.push((stream, update_id)); + match stream { + super::yas_lsp_adapter::WatchStream::Backend => backend = entities, + super::yas_lsp_adapter::WatchStream::Diagnostics => { + diagnostics = entities; + break; + } + } + } + } + let mut initial = session + .buffer_snapshot(workspace_handle) + .unwrap_or_default(); + initial.extend(backend.iter().cloned()); + initial.extend(diagnostics.iter().cloned()); if send_lsp_snapshot( subscription_id, revision, - session - .buffer_snapshot(workspace_handle) - .unwrap_or_default(), + initial, event_limit, &mut sent_bytes, &control, @@ -32566,6 +32597,9 @@ async fn run_lsp_watch( { return; } + for (stream, update_id) in initial_acks { + watch.acknowledge(stream, update_id); + } enum Input { Backend(Option>), Buffer(Result<(), tokio::sync::watch::error::RecvError>), diff --git a/crates/server/src/yas_lsp.rs b/crates/server/src/yas_lsp.rs index 65e5846e..ff98884f 100644 --- a/crates/server/src/yas_lsp.rs +++ b/crates/server/src/yas_lsp.rs @@ -607,6 +607,10 @@ impl Session { } impl Watch { + pub(crate) fn wants_diagnostics(&self) -> bool { + self.datasets & yas_wire::schema::lsp::WATCH_DIAGNOSTICS as u16 != 0 + } + pub(crate) async fn next(&mut self) -> Option> { while let Some(event) = self.receiver.as_mut()?.recv().await { match event { diff --git a/docs/design/lsp.md b/docs/design/lsp.md index 9eb30f86..b6db0941 100644 --- a/docs/design/lsp.md +++ b/docs/design/lsp.md @@ -98,7 +98,9 @@ loss with an exact reason. `WATCH` subscribes independently to backend lifecycle, diagnostics, and buffer overlays. Backend records carry phase, progress, epoch, capabilities, stable identity, resource data, and failure detail. Diagnostic records are complete -per-path replacements, including an explicit empty set to clear a file. State +per-path replacements, including an explicit empty set to clear a file. A +diagnostics subscription's initial snapshot carries the diagnostics the +backends have already published. State credit and staged snapshots make reconnect and slow-consumer recovery exact. Every document target combines a component-vector path, revision, and From c29b04416e465df07f91e616053ddc4aa74c035f Mon Sep 17 00:00:00 2001 From: Max Gasner Date: Mon, 28 Sep 2026 21:19:01 +0000 Subject: [PATCH 8/8] fs: prune .git from grep and find; make grep -m count files enumerate_paths walked with hidden(false), which keeps dotfiles as the design wants but also descended into .git, so grep and find searched repository metadata. .git is now pruned like an ignored directory, and with ignored paths included it is walked and marked ignored (docs/design/fs-grep.md). GREP's default per-file cap was the page record limit and counted from the first match, so a file with more matches than one page could hold lost the rest even though the cursor resumed inside it. Zero now caps nothing. `yas fs grep -m N` sent N as the page record limit and kept paging, so it never limited anything, -m 1 failed with Invalid, and small pages hit the dropped-match bug above. It now uses the default page size and stops after N matching files; a file split across pages counts and prints once, and multi-page searches no longer warn that results were truncated. --- crates/cli/src/yas_fs.rs | 92 ++++++++++++++++--------------- crates/server/src/yas_fs.rs | 106 +++++++++++++++++++++++++++++++++++- docs/design/fs-grep.md | 4 +- docs/design/fs-search.md | 3 +- 4 files changed, 158 insertions(+), 47 deletions(-) diff --git a/crates/cli/src/yas_fs.rs b/crates/cli/src/yas_fs.rs index 91aaadde..0d6a73f7 100644 --- a/crates/cli/src/yas_fs.rs +++ b/crates/cli/src/yas_fs.rs @@ -867,11 +867,12 @@ async fn cmd_grep( flags |= yas_wire::schema::fs::GREP_INCLUDE_IGNORED as u16; } + let file_limit = (max_matches != 0).then_some(usize::from(max_matches)); let mut cursor = Vec::new(); let mut matched_files = 0usize; - let mut hits = 0usize; - let mut truncated = false; - loop { + let mut last_path = None; + let mut truncated; + 'pages: loop { let page: QueryPage = root .client .request_typed( @@ -880,7 +881,7 @@ async fn cmd_grep( &Grep { root_handle: root.handle, flags, - max_results: max_matches, + max_results: 0, max_per_file: 0, query: pattern.as_bytes().to_vec(), cursor: cursor.clone(), @@ -890,7 +891,7 @@ async fn cmd_grep( true, ) .await?; - truncated |= page.flags & yas_wire::schema::fs::PAGE_TRUNCATED as u16 != 0; + truncated = page.flags & yas_wire::schema::fs::PAGE_TRUNCATED as u16 != 0; let next = page.next_cursor.clone(); let records = page_records(&mut root.client, page).await?; let files: BTreeMap = records @@ -900,39 +901,49 @@ async fn cmd_grep( _ => None, }) .collect(); - if files_with_matches { - for file in files.values().filter(|file| file.match_count != 0) { - matched_files += 1; - print_grep_file(file, json); - } - } else { - for record in records { - let QueryRecord::GrepMatch(found) = record else { - continue; - }; - let file = files - .get(&found.file_index) - .ok_or_else(|| "FS GREP match referenced a missing file".to_string())?; - hits += 1; - let path = display_path(&file.path); - let ignored = - file.flags & yas_wire::schema::fs::QUERY_GREP_FILE_IGNORED as u16 != 0; - if json { - println!( - "{}", - serde_json::json!({ - "path": path, - "ignored": ignored, - "line": found.line + 1, - "col": found.column, - "endLine": found.end_line + 1, - "endCol": found.end_column, - "text": found.text, - }) - ); - } else { - println!("{path}:{}:{}", found.line + 1, found.text); + for record in records { + match record { + QueryRecord::GrepFile(file) if file.match_count != 0 => { + // A file whose matches span a page boundary is sent again + // at the top of the next page. + if last_path.as_ref() == Some(&file.path) { + continue; + } + if file_limit.is_some_and(|limit| matched_files >= limit) { + truncated = false; + break 'pages; + } + matched_files += 1; + if files_with_matches { + print_grep_file(&file, json); + } + last_path = Some(file.path); } + QueryRecord::GrepMatch(found) if !files_with_matches => { + let file = files + .get(&found.file_index) + .ok_or_else(|| "FS GREP match referenced a missing file".to_string())?; + let path = display_path(&file.path); + let ignored = + file.flags & yas_wire::schema::fs::QUERY_GREP_FILE_IGNORED as u16 != 0; + if json { + println!( + "{}", + serde_json::json!({ + "path": path, + "ignored": ignored, + "line": found.line + 1, + "col": found.column, + "endLine": found.end_line + 1, + "endCol": found.end_column, + "text": found.text, + }) + ); + } else { + println!("{path}:{}:{}", found.line + 1, found.text); + } + } + _ => {} } } if next.is_empty() || next == cursor { @@ -944,12 +955,7 @@ async fn cmd_grep( eprintln!("yas: results truncated β€” a budget was reached"); } root.close().await?; - let count = if files_with_matches { - matched_files - } else { - hits - }; - Ok(if count == 0 { 1 } else { 0 }) + Ok(if matched_files == 0 { 1 } else { 0 }) } async fn page_records( diff --git a/crates/server/src/yas_fs.rs b/crates/server/src/yas_fs.rs index 7e57113d..47c2d60e 100644 --- a/crates/server/src/yas_fs.rs +++ b/crates/server/src/yas_fs.rs @@ -1916,7 +1916,11 @@ fn enumerate_paths( } let visible = if include_ignored { let mut builder = WalkBuilder::new(&root.path); - builder.hidden(false).follow_links(false).require_git(false); + builder + .hidden(false) + .follow_links(false) + .require_git(false) + .filter_entry(not_git_dir); let mut visible = BTreeSet::new(); for entry in builder.build() { let entry = entry.map_err(|error| Error::Io(error.to_string()))?; @@ -1937,6 +1941,8 @@ fn enumerate_paths( .git_ignore(false) .git_global(false) .git_exclude(false); + } else { + builder.filter_entry(not_git_dir); } let mut paths = Vec::new(); for entry in builder.build() { @@ -1958,6 +1964,13 @@ fn enumerate_paths( Ok(paths) } +/// Dotfiles stay visible; `.git` is pruned like an ignored directory +/// (docs/design/fs-grep.md). With ignored paths included it is walked and +/// marked ignored. +fn not_git_dir(entry: &ignore::DirEntry) -> bool { + entry.depth() == 0 || entry.file_name() != ".git" +} + fn path_flat(path: &wire::Path) -> Vec { let mut bytes = Vec::new(); for (index, component) in path.components.iter().enumerate() { @@ -2125,8 +2138,10 @@ fn grep_root(root: &Root, request: &wire::Grep) -> Result { "FS GREP max_results cannot hold a file and match record", )); } + // Zero caps nothing: the page budget bounds each page and the cursor + // resumes inside a file, so no match is silently dropped. let max_per_file = if request.max_per_file == 0 { - max_results + usize::MAX } else { request.max_per_file as usize }; @@ -4083,4 +4098,91 @@ mod tests { let error = write(&root, &[b"target.txt", b"file"], 0, b"x", [10; 16]).unwrap_err(); assert_eq!(os_of(&error), (libc::ENOTDIR, "ENOTDIR", "open")); } + + #[test] + fn grep_default_per_file_cap_keeps_every_match_across_pages() { + let directory = TestDir::new(); + fs::write( + directory.0.join("many.txt"), + b"needle one\nneedle two\nneedle three\n", + ) + .unwrap(); + let root = test_root(&directory); + let mut cursor = Vec::new(); + let mut lines = Vec::new(); + loop { + let page = grep_root( + &root, + &wire::Grep { + root_handle: 1, + flags: 0, + max_results: 2, + max_per_file: 0, + query: b"needle".to_vec(), + cursor, + initial_receive_credit: 0, + extensions: Extensions::default(), + }, + ) + .unwrap(); + lines.extend(page.records.iter().filter_map(|record| match record { + wire::QueryRecord::GrepMatch(found) => Some(found.line), + _ => None, + })); + if page.next_cursor.is_empty() { + break; + } + cursor = page.next_cursor; + } + assert_eq!(lines, [0, 1, 2]); + } + + #[test] + fn grep_and_search_prune_git_metadata_unless_ignored_paths_are_included() { + let directory = TestDir::new(); + fs::create_dir(directory.0.join(".git")).unwrap(); + fs::write(directory.0.join(".git").join("config"), b"needle\n").unwrap(); + fs::write(directory.0.join(".env"), b"needle\n").unwrap(); + let root = test_root(&directory); + let grep = |flags: u16| { + grep_root( + &root, + &wire::Grep { + root_handle: 1, + flags, + max_results: 0, + max_per_file: 0, + query: b"needle".to_vec(), + cursor: Vec::new(), + initial_receive_credit: 0, + extensions: Extensions::default(), + }, + ) + .unwrap() + .records + .into_iter() + .filter_map(|record| match record { + wire::QueryRecord::GrepFile(file) => Some((path_flat(&file.path), file.flags)), + _ => None, + }) + .collect::>() + }; + assert_eq!(grep(0), [(b".env".to_vec(), 0)]); + assert_eq!( + grep(schema::fs::GREP_INCLUDE_IGNORED as u16), + [ + (b".env".to_vec(), 0), + ( + b".git/config".to_vec(), + schema::fs::QUERY_GREP_FILE_IGNORED as u16 + ), + ] + ); + let paths = enumerate_paths(&root, false).unwrap(); + assert!( + paths + .iter() + .all(|(path, _, _)| path.components.first().map(Vec::as_slice) != Some(b".git")) + ); + } } diff --git a/docs/design/fs-grep.md b/docs/design/fs-grep.md index 55e13d7a..31aef352 100644 --- a/docs/design/fs-grep.md +++ b/docs/design/fs-grep.md @@ -101,7 +101,9 @@ list before a between-files check looked at it, and the pattern comes from the client. 65,535 matches per file is the Request field's `u16` ceiling. A zero request -uses the page record limit; a nonzero value is applied per candidate file. +applies no per-file cap: the page limits bound each page and the cursor resumes +inside a file. A nonzero value caps the matches reported for each candidate +file across all pages. The size check happens before a file read. A remaining file is read in full up to the 4 MiB ceiling and then accepted only if it is UTF-8, so an unpruned walk diff --git a/docs/design/fs-search.md b/docs/design/fs-search.md index a18bbae9..140ac2de 100644 --- a/docs/design/fs-search.md +++ b/docs/design/fs-search.md @@ -19,7 +19,8 @@ with **no watch**: recency to break otherwise equal matches. Both use the same `ignore`-crate walk. Normal ignore rules filter candidates by -default; request flags can include ignored paths. Dotfiles are included, +default; request flags can include ignored paths. Dotfiles are included, but +`.git` is pruned like an ignored directory unless ignored paths are included, symlinked directories are not followed, and each PATH record says whether it is a directory and whether it was ignored. `INDEX` independently selects files and directories; the TypeScript `indexFiles` facade requests files only.