From caab619225b7f09270e763f1bc57d5aabc4cc69a Mon Sep 17 00:00:00 2001 From: Eli Ma Date: Mon, 10 Aug 2026 11:27:23 +0800 Subject: [PATCH 1/2] test: repair the pre-existing compat-offline-core failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit None of these are new. They accumulated because `cargo test --all` is fail-fast: once the lib suite started failing on 2026-07-28 every run aborted there, so nothing downstream was ever reported and each new breakage landed unseen on top of the last. - worktree_scope (0ce8f77c): five tests call `std::env::set_current_dir` directly. `#[serial]` only orders them against other `#[serial]` tests, not against the CWD_LOCK every `ChangeDirGuard` holds, so they yanked the process cwd out from under whatever fixture was in flight — which is what took down the ai/hooks, rename_detect and utils::path tests with them. They now take that lock, and take it AFTER building their fixtures so a whole `libra init` does not run under a lock the rest of the suite waits on. `a_pinned_scope_survives_a_cwd_change` additionally pinned the ambient cwd, which `RequestScope::resolve` refuses outside a repository: it only ever passed by stealing a moment when another test had parked the cwd in someone else's fixture, which is the same moment it broke that test. It gets a real repository now. - ai/hooks/runtime (6da73c67): `CaptureScope` entered the ingest path and resolves `RepoIdentity`, but `ingest_fresh_conn` never wrote `libra.repoid`, so 14 tests failed deterministically — single-test runs included. Seed it, as `libra init` does. `ingest_fails_loud_when_table_missing` now drops one table from a complete schema instead of starting from a schema-less database, so it still tests the claim it makes rather than failing earlier on `config_kv`; the tombstone test asserts against the claim diagnostic that now carries the refusal. - utils/path: the commondir fixture's target lacked the repository database, so it no longer satisfies `is_terminal_common_storage`. - rename_detect (5c1b7bb2, a26cc9ac): the LFS classification resolves `working_dir()` infallibly on the pooled io thread, which panicked the worker outside a repository and reached the caller as an opaque `IoTimeout`. Both tests get a repository. A jammed pool reports `IoTimeout` too — the seam tests abandon reads that keep sleeping in a worker past their own end, and the pool is process-global — so the negative test drains the pool and retries. A seam that really fired would still blow every attempt, so the assertion keeps its teeth. - ai/web: `type_complexity` on a test-local adapter field, now a named alias that also records what the tuple's two halves mean. Verified with `cargo test --lib`: 4409 passed, 0 failed. Co-Authored-By: Claude Opus 5 --- src/command/rename_detect.rs | 44 ++++++++++++++++++++++++++++-- src/command/status_probe.rs | 18 ++++++++++++ src/internal/ai/hooks/runtime.rs | 36 ++++++++++++++++++------ src/internal/ai/web/mod.rs | 5 +++- src/internal/worktree_scope.rs | 47 ++++++++++++++++++++++++++++++-- src/utils/path.rs | 5 ++++ 6 files changed, 140 insertions(+), 15 deletions(-) diff --git a/src/command/rename_detect.rs b/src/command/rename_detect.rs index 8c355b80c..069dcba33 100644 --- a/src/command/rename_detect.rs +++ b/src/command/rename_detect.rs @@ -1325,6 +1325,22 @@ impl WorktreeReadBudget { mod tests { use super::*; + /// Make `root` discoverable as a repository and step the process cwd into + /// it, returning the guard that restores the cwd. + /// + /// Deliberately NOT a full `libra init`: the worktree read path only needs + /// repository DISCOVERY to succeed (`util::working_dir`, for the LFS + /// attribute lookup), and a terminal common storage is a gitdir carrying + /// the repository database. Standing up a real database here would drag a + /// connection pool into a test about byte accounting. + fn repo_fixture(root: &Path) -> crate::utils::test::ChangeDirGuard { + let gitdir = root.join(crate::utils::util::ROOT_DIR); + std::fs::create_dir_all(gitdir.join("objects")).expect("create object store"); + std::fs::write(gitdir.join(crate::utils::util::DATABASE), b"") + .expect("create repository db"); + crate::utils::test::ChangeDirGuard::new(root) + } + fn regular(evidence: BlobEvidence, size: u64) -> BlobRef { BlobRef { kind: BlobKind::Regular, @@ -2288,6 +2304,12 @@ mod tests { const SHRUNK: u64 = 4; const GROWTH: u64 = 4096; let dir = tempfile::tempdir().expect("tempdir"); + // The read path classifies LFS through `attribute_state_for_path`, + // which resolves `working_dir()` — infallibly, from the PROCESS cwd, + // on the pooled io thread. Outside a repository that panics the + // worker and the caller only ever sees the resulting `IoTimeout`, + // so the fixture has to be a repository. + let _repo = repo_fixture(dir.path()); let path = dir.path().join("grows.txt"); std::fs::write(&path, vec![b'a'; (SHRUNK + GROWTH) as usize]) .expect("write the full-size file"); @@ -2456,6 +2478,11 @@ mod tests { use std::time::Duration; let dir = tempfile::tempdir().expect("tempdir"); + // See `worktree_total_charges_bytes_read_not_stale_stat`: the LFS + // classification resolves `working_dir()` on the io thread, so a + // repo-less fixture fails with `IoTimeout` no matter what the seams + // are doing — which is precisely the outcome this test denies. + let _repo = repo_fixture(dir.path()); let path = dir.path().join("fast.txt"); std::fs::write(&path, b"content").expect("write fixture"); @@ -2466,8 +2493,21 @@ mod tests { std::env::set_var("LIBRA_TEST_SLOW_WORKTREE_READ_MS", "5000"); std::env::set_var("LIBRA_TEST_SLOW_LFS_ATTRIBUTES_MS", "5000"); } - let mut budget = WorktreeReadBudget::new(1024, 1024, 8, Duration::from_millis(1500)); - let outcome = budget.read_worktree_blob(&path); + // A jammed I/O pool and a fired seam both surface as `IoTimeout`, and + // the pool is process-global: the seam tests above abandon reads that + // keep sleeping in a worker, and their slots outlive them. So drain + // the pool first and retry — with the seams inert one attempt + // succeeds as soon as a worker is free, while a 5s seam that really + // did fire would blow the 1.5s batch on EVERY attempt. + let mut outcome = ContentOutcome::Skipped(SkipReason::IoTimeout); + for _ in 0..8 { + crate::command::status_probe::wait_for_idle_io_pool(); + let mut budget = WorktreeReadBudget::new(1024, 1024, 8, Duration::from_millis(1500)); + outcome = budget.read_worktree_blob(&path); + if matches!(outcome, ContentOutcome::Content(_)) { + break; + } + } unsafe { std::env::remove_var("LIBRA_TEST_SLOW_WORKTREE_STAT_MS"); std::env::remove_var("LIBRA_TEST_SLOW_WORKTREE_READ_MS"); diff --git a/src/command/status_probe.rs b/src/command/status_probe.rs index fe103e814..482c419aa 100644 --- a/src/command/status_probe.rs +++ b/src/command/status_probe.rs @@ -129,6 +129,24 @@ static IO_POOL: std::sync::OnceLock> = std::sync::O static IO_BUSY: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); static IO_WORKERS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +/// Block until every pooled slot has been released. +/// +/// The deadline tests deliberately ABANDON reads: the caller gives up at its +/// deadline while the worker keeps sleeping inside the armed seam, and the +/// slot stays busy until that sleep ends — outliving the test that started +/// it. A later test whose contract is "this must NOT time out" therefore has +/// to start from an idle pool, or it measures the previous test's leftover +/// delay and fails for a reason that has nothing to do with what it asserts. +#[cfg(test)] +pub(crate) fn wait_for_idle_io_pool() { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while IO_BUSY.load(std::sync::atomic::Ordering::SeqCst) > 0 + && std::time::Instant::now() < deadline + { + std::thread::sleep(std::time::Duration::from_millis(20)); + } +} + /// Run one blocking filesystem operation with a wall-clock deadline. /// /// A hung syscall cannot be interrupted in safe Rust, so the operation runs diff --git a/src/internal/ai/hooks/runtime.rs b/src/internal/ai/hooks/runtime.rs index 35a0b94eb..470fc2da8 100644 --- a/src/internal/ai/hooks/runtime.rs +++ b/src/internal/ai/hooks/runtime.rs @@ -3627,6 +3627,13 @@ mod tests { run_builtin_migrations(&conn) .await .expect("run_builtin_migrations"); + // `libra init` writes `libra.repoid` as part of bootstrap, and every + // ingest path resolves its `CaptureScope` through `RepoIdentity` — a + // fixture without it is not a repository this code will write to, it + // is one with corrupt metadata. + ConfigKv::set_with_conn(&conn, "libra.repoid", "repo-ingest-fixture", false) + .await + .expect("seed libra.repoid"); (dir, conn) } @@ -3730,8 +3737,13 @@ mod tests { ) .await .expect_err("stale hook must not resurrect an erased session"); + // The tombstone is now enforced inside the scope-claim upsert's WHERE + // clause rather than by a separate pre-check, so the refusal surfaces + // through the claim diagnostic. The row-count assertion below is the + // contract that matters; this one only pins that erasure is named as a + // cause rather than the write silently succeeding. assert!( - err.to_string().contains("anti-resurrection tombstone"), + err.to_string().contains("could not be claimed") && err.to_string().contains("erased"), "unexpected error: {err:#}" ); @@ -4153,14 +4165,20 @@ mod tests { #[tokio::test] async fn ingest_fails_loud_when_table_missing() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("noschema.db"); - std::fs::File::create(&path).expect("touch sqlite file"); - let url = format!("sqlite://{}", path.display()); - let mut opts = ConnectOptions::new(url); - opts.sqlx_logging(false); - let conn = Database::connect(opts).await.expect("connect"); - // intentionally NOT calling run_builtin_migrations. + // A fully bootstrapped repository with exactly ONE table removed. A + // schema-less database would fail earlier and for a different reason + // (`config_kv` is read first, to resolve the capture scope), which + // would leave the claim this test actually makes — that a missing + // `agent_session` is reported loudly and by name — untested. + let (_dir, conn) = ingest_fresh_conn().await; + let backend = conn.get_database_backend(); + let _: ExecResult = conn + .execute_raw(Statement::from_string( + backend, + "DROP TABLE agent_session".to_string(), + )) + .await + .expect("drop agent_session"); let payload = ingest_envelope("SessionStart", "S-bare", json!({})); let err = ingest_agent_traces_payload( diff --git a/src/internal/ai/web/mod.rs b/src/internal/ai/web/mod.rs index 898d17942..9ca58e37e 100644 --- a/src/internal/ai/web/mod.rs +++ b/src/internal/ai/web/mod.rs @@ -1591,10 +1591,13 @@ mod tests { use axum::extract::connect_info::MockConnectInfo; + /// What the adapter recorded, as `(message body, command id)`. + type Submitted = Arc)>>>; + #[derive(Clone)] struct CommandIdAdapter { session: Arc, - submitted: Arc)>>>, + submitted: Submitted, } #[async_trait::async_trait] diff --git a/src/internal/worktree_scope.rs b/src/internal/worktree_scope.rs index 76fd2b54b..013f504c0 100644 --- a/src/internal/worktree_scope.rs +++ b/src/internal/worktree_scope.rs @@ -377,12 +377,37 @@ mod tests { #[test] #[serial_test::serial] fn a_pinned_scope_survives_a_cwd_change() { + // A REAL repository, because `pin_scope_for_test` resolves its paths + // through `RequestScope::resolve` and installs nothing when the + // workdir is not inside one — pinning the ambient cwd only appeared to + // work when another test had parked it in someone else's fixture. + // + // Built BEFORE the cwd lock below: `setup_with_new_libra_in` takes and + // releases that lock itself, and running a whole `libra init` while + // holding it would block every other fixture in the suite for the + // duration. + let repo = tempfile::tempdir().expect("repo"); + { + let _cd = crate::utils::test::ChangeDirGuard::new(repo.path()); + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime") + .block_on(crate::utils::test::setup_with_new_libra_in(repo.path())); + } + + // Moving the process cwd is not this test's business alone: every + // `ChangeDirGuard` in the suite reads the same one. `#[serial]` only + // orders this against other `#[serial]` tests, so hold the cwd lock + // the guard uses — otherwise the `set_current_dir` below yanks the + // directory out from under whatever repository fixture is mid-flight. + let _cwd_lock = crate::utils::test::cwd_lock_guard(); let original = std::env::current_dir().expect("cwd"); let elsewhere = std::env::temp_dir(); let _pin = WorktreeScope::pin_scope_for_test( WorktreeScope::Linked("wt-pinned".to_string()), - original.clone(), + repo.path().to_path_buf(), ); assert!(WorktreeScope::request_scope_is_pinned()); assert_eq!(WorktreeScope::for_request().storage_key(), "wt-pinned"); @@ -432,6 +457,10 @@ mod tests { .expect("runtime") .block_on(crate::utils::test::setup_with_new_libra_in(repo.path())); } + // See `a_pinned_scope_survives_a_cwd_change`: the raw `set_current_dir` + // below is process-wide, so it has to hold `ChangeDirGuard`'s lock — + // taken only now, so the `libra init` above does not run under it. + let _cwd_lock = crate::utils::test::cwd_lock_guard(); let original = std::env::current_dir().expect("cwd"); // Pin a SUBDIRECTORY, which is what a command invoked from one does. @@ -483,6 +512,10 @@ mod tests { .expect("runtime") .block_on(crate::utils::test::setup_with_new_libra_in(repo.path())); } + // See `a_pinned_scope_survives_a_cwd_change`: the raw `set_current_dir` + // below is process-wide, so it has to hold `ChangeDirGuard`'s lock — + // taken only now, so the `libra init` above does not run under it. + let _cwd_lock = crate::utils::test::cwd_lock_guard(); let original = std::env::current_dir().expect("cwd"); let canonical_repo = std::fs::canonicalize(repo.path()).unwrap_or_else(|_| repo.path().to_path_buf()); @@ -523,11 +556,15 @@ mod tests { async fn the_request_database_follows_the_pin_not_the_cwd() { let repo_a = tempfile::tempdir().expect("repo a"); let repo_b = tempfile::tempdir().expect("repo b"); - let original = std::env::current_dir().expect("cwd"); for repo in [repo_a.path(), repo_b.path()] { let _cd = crate::utils::test::ChangeDirGuard::new(repo); crate::utils::test::setup_with_new_libra_in(repo).await; } + // See `a_pinned_scope_survives_a_cwd_change`: the raw `set_current_dir` + // below is process-wide, so it has to hold `ChangeDirGuard`'s lock — + // taken only now, so the two `libra init`s above do not run under it. + let _cwd_lock = crate::utils::test::cwd_lock_guard(); + let original = std::env::current_dir().expect("cwd"); let _pin = WorktreeScope::pin_request_scope(repo_a.path().to_path_buf()); // The cwd is repository B; the pin is repository A. @@ -576,11 +613,15 @@ mod tests { let outer = tempfile::tempdir().expect("the enclosing repository"); let ambient = tempfile::tempdir().expect("the repository the cwd is in"); let nowhere = tempfile::tempdir().expect("not a repository"); - let original = std::env::current_dir().expect("cwd"); for repo in [outer.path(), ambient.path()] { let _cd = crate::utils::test::ChangeDirGuard::new(repo); crate::utils::test::setup_with_new_libra_in(repo).await; } + // See `a_pinned_scope_survives_a_cwd_change`: the raw `set_current_dir` + // below is process-wide, so it has to hold `ChangeDirGuard`'s lock — + // taken only now, so the two `libra init`s above do not run under it. + let _cwd_lock = crate::utils::test::cwd_lock_guard(); + let original = std::env::current_dir().expect("cwd"); let _outer_pin = WorktreeScope::pin_request_scope(outer.path().to_path_buf()); std::env::set_current_dir(ambient.path()).expect("move the cwd"); diff --git a/src/utils/path.rs b/src/utils/path.rs index 11d9e6266..2e8f1c025 100644 --- a/src/utils/path.rs +++ b/src/utils/path.rs @@ -98,6 +98,11 @@ mod tests { let linked = root.path().join("linked"); let linked_gitdir = linked.join(".libra"); fs::create_dir_all(common.join("objects")).expect("create shared object store"); + // The commondir target must look like TERMINAL common storage + // (`util::is_terminal_common_storage`), which an object store alone + // does not: a real main gitdir also carries the repository database, + // and without it the pointer is refused as corruption. + fs::write(common.join(crate::utils::util::DATABASE), b"").expect("create repository db"); fs::create_dir_all(&linked_gitdir).expect("create linked worktree gitdir"); fs::write( linked_gitdir.join("commondir"), From 4c8fd5b30c51e0a263798ad7c7bf6d02d29ab246 Mon Sep 17 00:00:00 2001 From: Eli Ma Date: Mon, 10 Aug 2026 11:47:29 +0800 Subject: [PATCH 2/2] fix(test): restore the ai_semantic fixtures 8f47a4c4 deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `8f47a4c4` removed tests/data/ai_semantic/rust/{sample,tools_sample}.rs but left tests/ai_semantic_{rust,tools}_test.rs, which `include_str!` them. main therefore does not COMPILE under `--all-targets`: error: couldn't read `tests/data/ai_semantic/rust/sample.rs` error: could not compile `libra` (test "ai_semantic_rust_test") This breaks `compat-clippy` for every open PR, not just this one. Restored verbatim from 8f47a4c4^ rather than deleting the orphaned tests: the fixtures carry `Line numbers are pinned by that test` in their own header, and the assertions still reference those lines, so dropping the tests would silently retire live coverage on a judgement that belongs to whoever made the deletion. Restoring is the reading that keeps both halves consistent. `cargo fmt --all` leaves them untouched — they are not in the module tree — so the pinned lines stay put. Verified: ai_semantic_rust_test 4 passed, ai_semantic_tools_test 5 passed, and `cargo clippy --all-targets --all-features -- -D warnings` is clean again. Co-Authored-By: Claude Opus 5 --- tests/data/ai_semantic/rust/sample.rs | 23 +++++++++++++++ tests/data/ai_semantic/rust/tools_sample.rs | 32 +++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 tests/data/ai_semantic/rust/sample.rs create mode 100644 tests/data/ai_semantic/rust/tools_sample.rs diff --git a/tests/data/ai_semantic/rust/sample.rs b/tests/data/ai_semantic/rust/sample.rs new file mode 100644 index 000000000..de16603b2 --- /dev/null +++ b/tests/data/ai_semantic/rust/sample.rs @@ -0,0 +1,23 @@ +//! Fixture for `tests/ai_semantic_rust_test.rs`. Line numbers are pinned by +//! that test: `Widget::label` must start on line 10, `make_widget` on 15. + +pub struct Widget { + name: String, +} + +impl Widget { + // Reads the widget's label. + fn label(&self) -> &str { + &self.name + } + fn new(name: &str) -> Self { Self { name: name.to_string() } } +} +pub fn make_widget(name: &str) -> Widget { + Widget::new(name) +} + +fn handle() {} + +mod nested { + pub fn handle() {} +} diff --git a/tests/data/ai_semantic/rust/tools_sample.rs b/tests/data/ai_semantic/rust/tools_sample.rs new file mode 100644 index 000000000..231d6f069 --- /dev/null +++ b/tests/data/ai_semantic/rust/tools_sample.rs @@ -0,0 +1,32 @@ +//! Fixture for `tests/ai_semantic_tools_test.rs` (semantic tool handlers). +//! +//! Kept separate from `sample.rs`, whose line numbers are pinned by +//! `tests/ai_semantic_rust_test.rs` — this file's contents matter +//! (Widget::new + its call site, an ambiguous `handle`), not its line +//! numbers. + +pub struct Widget { + name: String, +} + +impl Widget { + pub fn new(name: String) -> Self { + Widget { name } + } + + // Reads the widget's label. + fn label(&self) -> &str { + &self.name + } +} + +pub fn make_widget(name: &str) -> Widget { + let name = name.to_string(); + Widget::new(name) +} + +fn handle() {} + +mod nested { + pub fn handle() {} +}