Close the four effectful mutation survivors, and the deadlock behind the three timeouts

agents/reference/testing-gates.md

@@ -92,10 +92,19 @@ Keep the job count low enough that the build dirs fit: each *unscoped* job dir r
 
 - **7 were real gaps and are now closed** -- six operator mutations in `matcher.rs`'s `placeholder_spans` (its own grammar counter-examples were untested) and `lib.rs`'s `prepare_persist` reading `meta_current` as `compatible && lengths match`, where `&&` -> `||` survived. The tests added for them kill 14 mutants at those sites.
 - **7 are equivalent mutants, proved rather than argued.** `|` -> `^` over disjoint bit fields is always a no-op: `function_flags`' `SQLITE_UTF8|SQLITE_DETERMINISTIC` (bits `0x1` and `0x800`), `base64_encode`'s `(b0<<16)|(b1<<8)|b2` (exhaustive over all 16,777,216 inputs), and `base64_decode`'s `(triple<<6)|value_of(b)` (all 67,108,864). In `placeholder_spans`, `j += 1` -> `j *= 1` is re-consumed by the following `while`, and `i = j + 1` -> `i = j * 1` / `i = j - 1` land on the closing brace or a name byte, never a `{` -- zero differing results over all 335,923 strings up to length 7 on `{ } a 1 _ -`. **No exemptions were added**: the gate is diff-scoped, so these only surface if someone edits those lines, and the exemption count stays zero.
-- **4 sit in effectful code no tier reaches** -- `RootedFs::hash` (two), `cli/run.rs`'s `load_state` guard and `wait_for_shutdown`. Tracked separately rather than re-litigating #233 here.
+- **4 sat in effectful code no tier reached** -- `RootedFs::hash` (two), `cli/run.rs`'s `load_state` guard and `wait_for_shutdown`. Tracked as #1012 rather than re-litigating #233 there, and closed by it at the integration and e2e tiers (below).
 
 The headline: nothing merged behind the blind gate turned out to be a silent behavior regression. What it hid was **test debt** -- two pure functions whose own documented edge cases had no assertions.
 
+**The four effectful survivors (#1012).** Three were real gaps at a tier that *can* reach them; the fourth is unreachable by construction. None of them needed #233 revisited, and none needed an exemption.
+
+- **`RootedFs::hash` -> `None` and `-> Some([0; 32])`** are both observable through the parsed path-table's racy window, which `packages/rust/tests/persist_parsed_path_table.rs` now drives on purpose. Pin a file's mtime *ahead* of the cache's `snapshot_ns` (`pin_inside_the_racy_window`) and `is_trusted` can no longer settle it on the stat tuple, so every reuse decision runs through the hash. Two tests fall out of that. A corpus pinned into the window and left alone must still parse nothing -- a hash that cannot be computed cannot confirm, so `None` re-parses all of it. And an in-place edit of the *same length* with the mtime restored leaves the stat tuple byte-identical (`fs::write` reuses the inode, and `FileStat`'s `ctime_ns` is `created()` -- a birth time no rewrite moves), so only a hash that varies with content re-parses it: that kills both constants, `Some([1; 32])` included.
+- **`load_state`'s `delete !`** (the `--extension` override guard) is wrong in *both* directions, and each is a CLI behavior worth pinning. With the flag present the mutant drops the launcher's resolved extensions *and* stops suppressing the config's own; with the flag absent it suppresses a config's `[[dirsql.extension]]` entries that nothing overrode. `packages/rust/tests/extensions_e2e.rs` spawns the real binary over the `testext` fixture once per direction -- the builder tests in `extensions.rs` cannot see this decision, because it is the CLI's, not the builder's.
+- **`wait_for_shutdown` at `cli/run.rs:651`**, the unix arm, was already caught -- but only by `wait_until_ready`'s timeout, since a server that returns from its own signal wait exits before any readiness poll can see it. That is a 10s never-became-ready panic standing in for a contract. `cli_e2e.rs`'s `the_server_runs_until_signalled_and_then_exits_zero` reads the startup banner instead (the mutant still prints it), waits a beat, and asserts the process is alive -- killing it in 0.6s against the contract `docs/reference/cli.md` states.
+- **`wait_for_shutdown` at `cli/run.rs:664`** is the `#[cfg(not(unix))]` twin, and **no test on this gate can kill it**: the gate runs on Linux, where that item is never compiled. Proof rather than argument -- plant a `compile_error!` in its body and `cargo build --features cli` still succeeds. The test above would kill it on a Windows runner. No exemption was added, on the same reasoning as the seven equivalents: mutation is diff-scoped, so it only resurfaces if someone edits that line.
+
+**The three scanner timeouts were one deadlocked test (#1012).** `walk` (two) and `scan_directory` reported `Timeout` -- inconclusive rather than survivors -- and the mutants were telling the truth about the suite. `async_sdk.rs`'s `sync_backed_methods_before_ready_error` parked background init on a `std::sync::Barrier::new(2)` whose second arrival *is* the extract closure; a scan that matches no file never calls the closure, so the test's own release waited forever and wedged the whole test binary (every other binary in the tree failed fast under the same mutant -- `async_sdk` was the only hang). The barrier is now a `Mutex<Gate>` + `Condvar` bounded on both sides: a starved scan now fails in 30s naming the cause, and the three mutants become ordinary kills. When a mutant times out, look for a wait with no deadline before assuming the engine is slow.
+
 **Survivors.** The fix is almost always a **new assertion** that kills the mutant. Only a genuinely *equivalent* or intentionally-defensive mutant is lifted, via a `[[<language>.exempt]]` entry whose `rules` includes `"mutation"` -- in the config that lane actually reads (`packages/python/testing-conventions.toml` / `packages/ts/testing-conventions.toml` since #933; the root file for the rust core) (with a real `path` and a `reason`) -- never weaken a test to make a survivor pass. There are none today.
 ## Coverage Floor
 

packages/rust/tests/async_sdk.rs

@@ -202,23 +202,45 @@ async fn start_watching_and_poll_events_forward() {
     );
 }
 
+/// Deadlock backstop for the parked-init handshake below. Generous: nothing
+/// here is a timing assertion, it is the bound that turns a scan which never
+/// reaches the extract closure into a failure instead of a wedged test binary.
+const PARK_PATIENCE: Duration = Duration::from_secs(30);
+
+/// The parked-init handshake: `parked` is raised by the extract closure on
+/// arrival, `released` by the test once its assertions are done.
+#[derive(Default)]
+struct Gate {
+    parked: bool,
+    released: bool,
+}
+
 #[tokio::test]
 async fn sync_backed_methods_before_ready_error() {
     let root = TempDir::new().unwrap();
     fs::write(root.path().join("gate.txt"), "apple").unwrap();
 
     // Background init runs eagerly, so merely "not awaiting ready()" races
     // it. Instead, park init deterministically: the extract closure blocks
-    // on a barrier released only after the assertions, so init cannot
-    // complete during the assertion window.
-    let gate = std::sync::Arc::new(std::sync::Barrier::new(2));
+    // until the assertions release it, so init cannot complete during the
+    // assertion window.
+    let gate = std::sync::Arc::new((
+        std::sync::Mutex::new(Gate::default()),
+        std::sync::Condvar::new(),
+    ));
     let gate_in_extract = gate.clone();
     let gated_table = Table::new(
         "items",
         "CREATE TABLE items (name TEXT)",
         "**/*.txt",
         move |_| {
-            gate_in_extract.wait();
+            let (lock, cvar) = &*gate_in_extract;
+            let mut state = lock.lock().unwrap();
+            state.parked = true;
+            cvar.notify_all();
+            let _ = cvar
+                .wait_timeout_while(state, PARK_PATIENCE, |s| !s.released)
+                .unwrap();
             vec![HashMap::from([(
                 "name".into(),
                 Value::Text("apple".into()),
@@ -260,9 +282,21 @@ async fn sync_backed_methods_before_ready_error() {
         "got: {watch_err}"
     );
 
-    // Release init and let it finish so the background thread does not
-    // outlive the TempDir.
-    gate.wait();
+    // Init parks in the extract closure, which is reached only if the scan
+    // matched `gate.txt`. Prove it arrived, then release it and let it finish
+    // so the background thread does not outlive the TempDir.
+    let (lock, cvar) = &*gate;
+    let (mut state, wait) = cvar
+        .wait_timeout_while(lock.lock().unwrap(), PARK_PATIENCE, |s| !s.parked)
+        .unwrap();
+    assert!(
+        !wait.timed_out(),
+        "init never reached the extract closure: the scan matched no file, so \
+         this test never exercised a parked init",
+    );
+    state.released = true;
+    cvar.notify_all();
+    drop(state);
     db.ready().await.unwrap();
 }
 

packages/rust/tests/cli_e2e.rs

@@ -1541,3 +1541,75 @@ fn persist_flag_with_path_writes_the_cache_there() {
         "the default cache must not be written when a path is given"
     );
 }
+
+/// Send `sig` to a live child. Mirrors [`kill_and_wait`]'s signalling, which is
+/// the only way to ask a `dirsql server` to stop.
+#[cfg(unix)]
+fn signal_child(child: &Child, sig: i32) {
+    #[expect(
+        unsafe_code,
+        reason = "no safe std API sends a signal to another process"
+    )]
+    unsafe {
+        libc::kill(i32::try_from(child.id()).unwrap(), sig);
+    }
+}
+
+/// The child's exit code, or `None` if it outlives `patience`.
+#[cfg(unix)]
+fn exit_code_within(child: &mut Child, patience: Duration) -> Option<i32> {
+    let deadline = Instant::now() + patience;
+    while Instant::now() < deadline {
+        if let Some(status) = child.try_wait().unwrap() {
+            return status.code();
+        }
+        std::thread::sleep(Duration::from_millis(50));
+    }
+    None
+}
+
+/// `docs/reference/cli.md`: the server "runs until it receives `SIGINT`
+/// (Ctrl-C) or `SIGTERM`", and that shutdown is exit code `0`.
+///
+/// Both halves are load-bearing and neither is visible from a request. A
+/// server that never waited would announce its port, serve whatever raced in,
+/// and exit 0 — which reads as a clean run from the outside.
+#[cfg(unix)]
+#[test]
+fn the_server_runs_until_signalled_and_then_exits_zero() {
+    let root = blog_fixture();
+    let port = free_port();
+    let mut child = spawn_dirsql_with_args(root.path(), port, &["-c", ".dirsql.toml"]);
+
+    // Wait on the banner rather than on a request: it is printed the moment
+    // the listener is bound, so a server that leaves straight afterwards is
+    // still observed here instead of timing out as never-ready.
+    let stdout = child.stdout.take().expect("stdout piped");
+    let mut line = String::new();
+    BufReader::new(stdout)
+        .read_line(&mut line)
+        .expect("expected a startup line");
+    assert!(
+        line.contains(&format!("localhost:{port}")),
+        "banner: {line:?}"
+    );
+
+    std::thread::sleep(Duration::from_millis(500));
+    assert!(
+        child.try_wait().unwrap().is_none(),
+        "the server must stay up until it is signalled",
+    );
+
+    signal_child(&child, libc::SIGTERM);
+    let code = exit_code_within(&mut child, Duration::from_secs(10));
+    if code.is_none() {
+        let _ = child.kill();
+        let _ = child.wait();
+    }
+
+    assert_eq!(
+        code,
+        Some(0),
+        "a signalled server drains and exits 0, never 143",
+    );
+}

packages/rust/tests/extensions_e2e.rs

@@ -0,0 +1,130 @@
+//! End-to-end tests for the CLI's two extension surfaces: a config's own
+//! `[[dirsql.extension]]` entries and the launcher-facing `--extension` flag
+//! that overrides them.
+//!
+//! Spawns the real compiled `dirsql` binary against a real temp directory, a
+//! real `.dirsql.toml` and a real loadable extension built from
+//! `tests/fixtures/testext`. Nothing is mocked (real process, real filesystem,
+//! real SQLite, real `dlopen`).
+//!
+//! `packages/rust/tests/extensions.rs` covers the same two behaviors at the
+//! builder surface. They are covered again here because the CLI has a
+//! decision of its own to make -- which of the two sources of extensions to
+//! hand the builder -- and the builder tests cannot see it.
+//!
+//! Gated behind `--features cli` (the `dirsql` bin needs it) and Unix (the
+//! fixture config shells out to `cat`); the Rust CI test job runs on Linux.
+
+#![cfg(all(feature = "cli", unix))]
+
+mod common;
+
+use std::fs;
+use std::path::Path;
+use std::process::Output;
+
+use assert_cmd::prelude::*;
+use common::build_fixture_extension;
+use serde_json::Value;
+use tempfile::TempDir;
+
+/// What `dirsql_testext_answer()` returns once the fixture extension is loaded
+/// onto the connection. An unloaded extension is a `no such function` error,
+/// not a wrong answer.
+const ANSWER: i64 = 42;
+
+const SQL: &str = "SELECT dirsql_testext_answer() AS a";
+
+/// A tempdir holding one parseable file and a `.dirsql.toml` that declares
+/// `extension` (verbatim TOML, possibly empty) ahead of an ordinary table, so
+/// the index has something to build either way.
+fn fixture(extension: &str) -> TempDir {
+    let root = TempDir::new().unwrap();
+    fs::write(root.path().join("a.json"), r#"[{"name": "a"}]"#).unwrap();
+    fs::write(
+        root.path().join(".dirsql.toml"),
+        format!(
+            r#"{extension}
+[[table]]
+name = "files"
+ddl = "CREATE TABLE files (name TEXT)"
+glob = "*.json"
+on-file = "cat {{path}}"
+"#
+        ),
+    )
+    .unwrap();
+    root
+}
+
+/// A `[[dirsql.extension]]` entry naming `path`.
+fn extension_entry(path: &Path) -> String {
+    format!(
+        "[[dirsql.extension]]\npath = \"{}\"\nentrypoint = \"sqlite3_extension_init\"\n",
+        path.display(),
+    )
+}
+
+/// Run `dirsql query <SQL> -c .dirsql.toml` in `root`, with `extra` appended.
+fn query(root: &TempDir, extra: &[String]) -> Output {
+    std::process::Command::cargo_bin("dirsql")
+        .expect("`dirsql` binary must be built with --features cli")
+        .arg("query")
+        .arg(SQL)
+        .arg("--config")
+        .arg(root.path().join(".dirsql.toml"))
+        .args(extra)
+        .current_dir(root.path())
+        .output()
+        .expect("spawning `dirsql query` failed")
+}
+
+fn rows(out: &Output) -> Vec<Value> {
+    assert!(
+        out.status.success(),
+        "query must succeed, stderr: {}",
+        String::from_utf8_lossy(&out.stderr)
+    );
+    serde_json::from_slice(&out.stdout).expect("stdout must be JSON")
+}
+
+#[test]
+fn a_config_declared_extension_loads_when_no_flag_overrides_it() {
+    // The ordinary user path: no `--extension` anywhere, so the config's own
+    // entries are the only source of extensions and must be honored.
+    let ext = build_fixture_extension();
+    let root = fixture(&extension_entry(&ext));
+
+    let out = query(&root, &[]);
+
+    assert_eq!(
+        rows(&out)[0]["a"],
+        Value::from(ANSWER),
+        "the config's extension must be loaded onto the connection",
+    );
+}
+
+#[test]
+fn an_extension_flag_replaces_the_config_entries_rather_than_adding_to_them() {
+    // The launcher path: it has already resolved the config's entries (package
+    // names and all) and hands the resolved paths back through `--extension`,
+    // so the config's own entries must not be loaded a second time. The config
+    // here names a path that does not exist, which would fail the build if it
+    // were honored -- a green run is the proof it was not.
+    let ext = build_fixture_extension();
+    let root = fixture("[[dirsql.extension]]\npath = \"does/not/exist.so\"\n");
+
+    let out = query(
+        &root,
+        &[
+            "--extension".to_string(),
+            format!("{}::sqlite3_extension_init", ext.display()),
+        ],
+    );
+
+    assert_eq!(
+        rows(&out)[0]["a"],
+        Value::from(ANSWER),
+        "the flag's extension must load and the config's must be suppressed",
+    );
+}