diff --git a/crates/codegraph-cli/src/main.rs b/crates/codegraph-cli/src/main.rs index e0b2c167..564bc482 100644 --- a/crates/codegraph-cli/src/main.rs +++ b/crates/codegraph-cli/src/main.rs @@ -8378,6 +8378,16 @@ mod formatter_and_env_tests { assert!(line.contains("myFunc"), "name must be shown: {line:?}"); } + #[test] + fn search_human_result_line_pads_the_kind_to_twelve_columns() { + // Upstream prints `kind.padEnd(12) + name`; the kind must not run into the name. + let sr = SearchResult { + node: query_test_node("myFunc"), + score: 1.0, + }; + assert_eq!(format_search_result_line(&sr), "function myFunc"); + } + #[test] fn query_json_output_still_carries_raw_score() { // #1045: the percentage is dropped from the HUMAN output only. The diff --git a/crates/codegraph-cli/tests/cli_text_and_errors.rs b/crates/codegraph-cli/tests/cli_text_and_errors.rs index a0ee5b12..707bb8f8 100644 --- a/crates/codegraph-cli/tests/cli_text_and_errors.rs +++ b/crates/codegraph-cli/tests/cli_text_and_errors.rs @@ -207,6 +207,48 @@ fn callers_callees_text_output_render() { ); } +#[test] +fn text_rows_pad_the_kind_to_twelve_columns() { + // search, callers, callees and impact print `{kind:<12}{name}` like upstream's + // `kind.padEnd(12) + name`. Without padding the rows read `functionadd`. + let dir = TestDir::new("kind-column"); + let project = indexed_project(&dir); + let p = project.to_str().unwrap(); + + let rows = |args: &[&str]| -> Vec { + let run = run_in(dir.path(), args); + assert!(run.ok, "{args:?} must succeed: {}", run.stderr); + run.stdout.lines().map(str::to_string).collect() + }; + + let search = rows(&["search", "add", "-p", p]); + assert!( + search.iter().any(|line| line == "function add"), + "search rows: {search:?}" + ); + let callers = rows(&["callers", "add", "-p", p]); + assert!( + callers.iter().any(|line| line == "function runDemo"), + "callers rows: {callers:?}" + ); + let callees = rows(&["callees", "runDemo", "-p", p]); + assert!( + callees.iter().any(|line| line == "function add"), + "callees rows: {callees:?}" + ); + let impact = rows(&["impact", "add", "-p", p]); + for row in [ + " function runDemo:3", + " file app.ts:1", + " method increment:8", + ] { + assert!( + impact.iter().any(|line| line == row), + "impact rows must include {row:?}: {impact:?}" + ); + } +} + #[test] fn lookup_commands_refuse_fuzzy_only_symbol_matches() { let dir = TestDir::new("lookup-fuzzy-refusal"); diff --git a/crates/codegraph-core/src/types.rs b/crates/codegraph-core/src/types.rs index 5ce22bb1..9a214dae 100644 --- a/crates/codegraph-core/src/types.rs +++ b/crates/codegraph-core/src/types.rs @@ -205,7 +205,8 @@ impl NodeKind { impl fmt::Display for NodeKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + // `pad`, not `write_str`: callers lay rows out with a width (`{:<12}`). + f.pad(self.as_str()) } } @@ -273,7 +274,8 @@ impl EdgeKind { impl fmt::Display for EdgeKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + // `pad`, not `write_str`: callers lay rows out with a width (`{:<12}`). + f.pad(self.as_str()) } } @@ -472,7 +474,8 @@ impl Language { impl fmt::Display for Language { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + // `pad`, not `write_str`: callers lay rows out with a width (`{:<12}`). + f.pad(self.as_str()) } } @@ -567,7 +570,8 @@ impl ReferenceSubkind { impl fmt::Display for ReferenceSubkind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + // `pad`, not `write_str`: callers lay rows out with a width (`{:<12}`). + f.pad(self.as_str()) } } @@ -663,6 +667,22 @@ mod tests { assert_eq!(round_tripped, node); } + #[test] + fn display_honours_width_and_alignment() { + // The CLI lays rows out as `{:<12}{}` (upstream's `kind.padEnd(12)`); a Display that + // ignores the formatter's width prints `functionadd`. + assert_eq!(format!("{:<12}|", NodeKind::Function), "function |"); + assert_eq!(format!("{:>12}|", NodeKind::EnumMember), " enum_member|"); + assert_eq!(format!("{:<8}|", EdgeKind::Calls), "calls |"); + assert_eq!(format!("{:^12}|", Language::TypeScript), " typescript |"); + assert_eq!( + format!("{:<10}|", ReferenceSubkind::Autoload), + "autoload |" + ); + // A width narrower than the name never truncates it. + assert_eq!(format!("{:<3}", NodeKind::Function), "function"); + } + #[test] fn node_kind_function_serializes_to_upstream_exact_string() { assert_eq!(NodeKind::Function.as_str(), "function"); diff --git a/crates/codegraph-store/tests/index_lease.rs b/crates/codegraph-store/tests/index_lease.rs index 9c69ab11..64fbd37e 100644 --- a/crates/codegraph-store/tests/index_lease.rs +++ b/crates/codegraph-store/tests/index_lease.rs @@ -17,6 +17,13 @@ use codegraph_store::{ const CHILD_ACTION: &str = "CODEGRAPH_INDEX_LEASE_CHILD_ACTION"; const CHILD_MODE: &str = "CODEGRAPH_INDEX_LEASE_CHILD_MODE"; const CHILD_PROJECT: &str = "CODEGRAPH_INDEX_LEASE_CHILD_PROJECT"; +/// Milliseconds the child may wait for the lock: CHILD_WAIT for a holder, the caller's bound for +/// a probe, so a probe expected to acquire a free lock is not cut short by SHORT_DEADLINE. +const CHILD_LOCK_WAIT_MS: &str = "CODEGRAPH_INDEX_LEASE_CHILD_LOCK_WAIT_MS"; +/// Test-only stand-in for a slow child: milliseconds the child sleeps after setting its +/// deadline and before its first lock attempt, as a loaded Windows runner spends opening and +/// validating the lock. +const CHILD_PRE_ACQUIRE_DELAY_MS: &str = "CODEGRAPH_INDEX_LEASE_CHILD_PRE_ACQUIRE_DELAY_MS"; const LOCK_BYTES: &[u8] = b"permanent-lock-sentinel\nnot-a-pid\n"; const CHILD_WAIT: Duration = Duration::from_secs(5); const SHORT_DEADLINE: Duration = Duration::from_millis(80); @@ -422,6 +429,23 @@ fn an_already_expired_deadline_is_nonmutating_even_when_the_lock_is_free() { assert_eq!(lock_bytes(&paths), LOCK_BYTES); } +#[test] +fn probes_acquire_a_free_lock_from_a_slow_child() { + // A probe expected to ACQUIRE must get the caller's bound for its own lock attempt. On a + // loaded Windows runner the child can take longer than SHORT_DEADLINE to reach its first + // attempt, and then reports TIMED_OUT for a lock nobody holds (Windows CI, 2026-10-03). + let project = TempProject::new("slow-probe"); + let paths = project.paths(); + stage_existing_lock(&paths, LOCK_BYTES); + for mode in ["shared", "exclusive"] { + assert_eq!( + run_probe_after(project.path(), mode, CHILD_WAIT, Duration::from_millis(300)), + "ACQUIRED", + "{mode} probe of a free lock" + ); + } +} + #[test] fn a_clone_keeps_the_single_lock_alive_until_the_final_drop() { let project = TempProject::new("clone"); @@ -474,10 +498,12 @@ fn lease_mode_parent_and_clone_drop_order_are_enforced() { "dropping a non-final shared clone must keep exclusives blocked" ); drop(shared_clone_b); + // A probe expected to ACQUIRE gets CHILD_WAIT: SHORT_DEADLINE would also time the child's + // own open-and-validate work, which a loaded Windows runner can stretch past 80 ms. assert_eq!( - run_probe(shared_project.path(), "exclusive", SHORT_DEADLINE), + run_probe(shared_project.path(), "exclusive", CHILD_WAIT), "ACQUIRED", - "the final shared owner must release immediately" + "the final shared owner must release on drop" ); let exclusive_project = TempProject::new("exclusive-parent-clones"); @@ -510,9 +536,9 @@ fn lease_mode_parent_and_clone_drop_order_are_enforced() { } drop(exclusive_clone_b); assert_eq!( - run_probe(exclusive_project.path(), "shared", SHORT_DEADLINE), + run_probe(exclusive_project.path(), "shared", CHILD_WAIT), "ACQUIRED", - "the final exclusive owner must release immediately" + "the final exclusive owner must release on drop" ); // Build a real Current namespace exclusively through public production APIs, @@ -564,9 +590,9 @@ fn lease_mode_parent_and_clone_drop_order_are_enforced() { } assert_eq!( - run_probe(store_project.path(), "exclusive", SHORT_DEADLINE), + run_probe(store_project.path(), "exclusive", CHILD_WAIT), "ACQUIRED", - "dropping the final Store owner must admit a fresh contender immediately" + "dropping the final Store owner must admit a fresh contender" ); } @@ -613,14 +639,25 @@ fn lease_child_process() { let mode = std::env::var(CHILD_MODE).expect("child mode env"); let paths = IndexPaths::resolve(&project, None).expect("child resolve IndexPaths"); - let acquire = || match mode.as_str() { - "shared" => { - IndexLease::acquire_shared_existing(&paths, deadline_after(SHORT_DEADLINE), || false) + let wait = std::env::var(CHILD_LOCK_WAIT_MS) + .ok() + .and_then(|value| value.parse::().ok()) + .map(Duration::from_millis) + .expect("child lock wait env"); + let delay = std::env::var(CHILD_PRE_ACQUIRE_DELAY_MS) + .ok() + .and_then(|value| value.parse::().ok()) + .map(Duration::from_millis); + let acquire = || { + let deadline = deadline_after(wait); + if let Some(delay) = delay { + std::thread::sleep(delay); } - "exclusive" => { - IndexLease::acquire_exclusive_existing(&paths, deadline_after(SHORT_DEADLINE), || false) + match mode.as_str() { + "shared" => IndexLease::acquire_shared_existing(&paths, deadline, || false), + "exclusive" => IndexLease::acquire_exclusive_existing(&paths, deadline, || false), + other => panic!("unknown child mode {other}"), } - other => panic!("unknown child mode {other}"), }; match action.as_str() { @@ -728,12 +765,27 @@ fn child_command(project: &Path, mode: &str, action: &str) -> Command { .arg("--nocapture") .env(CHILD_ACTION, action) .env(CHILD_MODE, mode) - .env(CHILD_PROJECT, project); + .env(CHILD_PROJECT, project) + .env(CHILD_LOCK_WAIT_MS, CHILD_WAIT.as_millis().to_string()); command } fn run_probe(project: &Path, mode: &str, acquisition_bound: Duration) -> String { + run_probe_after(project, mode, acquisition_bound, Duration::ZERO) +} + +fn run_probe_after( + project: &Path, + mode: &str, + acquisition_bound: Duration, + delay: Duration, +) -> String { let mut child = child_command(project, mode, "probe") + .env( + CHILD_LOCK_WAIT_MS, + acquisition_bound.as_millis().to_string(), + ) + .env(CHILD_PRE_ACQUIRE_DELAY_MS, delay.as_millis().to_string()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .spawn() @@ -748,7 +800,8 @@ fn run_probe(project: &Path, mode: &str, acquisition_bound: Duration) -> String output_tx.send(output).expect("send probe output"); }); let process_bound = acquisition_bound - .checked_add(CHILD_WAIT) + .checked_add(delay) + .and_then(|bound| bound.checked_add(CHILD_WAIT)) .expect("probe process bound"); let status = wait_bounded(&mut child, process_bound); assert!(status.success(), "probe child failed: {status}"); diff --git a/crates/codegraph-store/tests/store_state_gates.rs b/crates/codegraph-store/tests/store_state_gates.rs index b16af68c..20d6ab24 100644 --- a/crates/codegraph-store/tests/store_state_gates.rs +++ b/crates/codegraph-store/tests/store_state_gates.rs @@ -19,6 +19,13 @@ use serde_json::json; const CHILD_ACTION: &str = "CODEGRAPH_STORE_GATE_CHILD_ACTION"; const CHILD_PROJECT: &str = "CODEGRAPH_STORE_GATE_CHILD_PROJECT"; +/// Milliseconds a probe child may wait for the lock: the caller's bound, so a probe expected to +/// acquire a free lock is not cut short by SHORT_DEADLINE. +const CHILD_LOCK_WAIT_MS: &str = "CODEGRAPH_STORE_GATE_CHILD_LOCK_WAIT_MS"; +/// Test-only stand-in for a slow child: milliseconds a probe sleeps after setting its deadline +/// and before its first lock attempt, as a loaded Windows runner spends opening and validating +/// the lock. +const CHILD_PRE_ACQUIRE_DELAY_MS: &str = "CODEGRAPH_STORE_GATE_CHILD_PRE_ACQUIRE_DELAY_MS"; const CHILD_WAIT: Duration = Duration::from_secs(5); const SHORT_DEADLINE: Duration = Duration::from_millis(80); static NEXT_TEMP: AtomicU64 = AtomicU64::new(0); @@ -1580,6 +1587,19 @@ fn missing_state_with_non_utf8_database_sidecar_fails_closed() { } } +#[test] +fn exclusive_probe_acquires_a_free_lock_from_a_slow_child() { + // A probe expected to ACQUIRE must get the caller's bound for its own lock attempt. On a + // loaded Windows runner the child can take longer than SHORT_DEADLINE to reach its first + // attempt, and then reports TIMED_OUT for a lock nobody holds (Windows CI, 2026-10-03). + let project = TempProject::new("slow-probe"); + stage_current(&project, Some(&CURRENT_EXTRACTION_VERSION.to_string())); + assert_eq!( + run_exclusive_probe_after(project.path(), CHILD_WAIT, Duration::from_millis(300)), + "ACQUIRED" + ); +} + /// Child-process entry point used by deterministic lock contention tests. #[test] fn store_gate_child_process() { @@ -1602,18 +1622,28 @@ fn store_gate_child_process() { println!("RELEASED"); std::io::stdout().flush().expect("flush RELEASED"); } - "probe-exclusive" => match IndexLease::acquire_exclusive_existing( - &paths, - deadline_after(SHORT_DEADLINE), - || false, - ) { - Ok(lease) => { - println!("ACQUIRED"); - drop(lease); + "probe-exclusive" => { + let wait = std::env::var(CHILD_LOCK_WAIT_MS) + .ok() + .and_then(|value| value.parse::().ok()) + .map(Duration::from_millis) + .expect("probe lock wait env"); + let deadline = deadline_after(wait); + if let Some(delay) = std::env::var(CHILD_PRE_ACQUIRE_DELAY_MS) + .ok() + .and_then(|value| value.parse::().ok()) + { + std::thread::sleep(Duration::from_millis(delay)); } - Err(codegraph_store::IndexLeaseError::TimedOut { .. }) => println!("TIMED_OUT"), - Err(error) => panic!("unexpected child probe error: {error}"), - }, + match IndexLease::acquire_exclusive_existing(&paths, deadline, || false) { + Ok(lease) => { + println!("ACQUIRED"); + drop(lease); + } + Err(codegraph_store::IndexLeaseError::TimedOut { .. }) => println!("TIMED_OUT"), + Err(error) => panic!("unexpected child probe error: {error}"), + } + } "write-current-wal-and-exit" => { let lease = IndexLease::acquire_exclusive_existing(&paths, deadline(), || false) .expect("crash fixture acquires exclusive lease"); @@ -1721,8 +1751,16 @@ fn child_command(project: &Path, action: &str) -> Command { command } -fn run_exclusive_probe(project: &Path, process_bound: Duration) -> String { +/// Run a child that tries the exclusive lease for `lock_wait` and reports `ACQUIRED` or +/// `TIMED_OUT`. Use SHORT_DEADLINE when the lock is expected to be held, CHILD_WAIT when free. +fn run_exclusive_probe(project: &Path, lock_wait: Duration) -> String { + run_exclusive_probe_after(project, lock_wait, Duration::ZERO) +} + +fn run_exclusive_probe_after(project: &Path, lock_wait: Duration, delay: Duration) -> String { let mut child = child_command(project, "probe-exclusive") + .env(CHILD_LOCK_WAIT_MS, lock_wait.as_millis().to_string()) + .env(CHILD_PRE_ACQUIRE_DELAY_MS, delay.as_millis().to_string()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .spawn() @@ -1738,8 +1776,9 @@ fn run_exclusive_probe(project: &Path, process_bound: Duration) -> String { }); let status = wait_bounded( &mut child, - process_bound - .checked_add(CHILD_WAIT) + lock_wait + .checked_add(delay) + .and_then(|bound| bound.checked_add(CHILD_WAIT)) .expect("probe process bound"), ); assert!(status.success(), "probe child failed: {status}");