From 1287785ee2939d479f11f4c5e5393820272a09a7 Mon Sep 17 00:00:00 2001 From: Emre Camdere Date: Wed, 29 Jul 2026 02:39:06 +0300 Subject: [PATCH] fix(gates): witness the ingest so truncated checkouts cannot pass silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shallow fetch-depth checkout whose tip is a merge commit ingests zero history under the default merge filter, leaving every quality gate with nothing to violate — `codelore check` printed PASS (0 files evaluated), wrote result=pass, and exited 0. Three related gate-integrity holes are closed as one class: - Witness the ingest on both gate paths (`codelore check` and the check_gates MCP tool): a real HEAD over a zero-commit store is now a hard error (exit 3, the shallow/corrupted-repo bucket) that names fetch-depth, never a pass. A shallow checkout that did ingest partial history is warned about loudly, and the code_health_min degraded sentinel now witnesses "the repository has analyzable source" by reading the HEAD tree directly rather than counting complexity_metrics — which derives from the same changes-commits join as the health rows and so could not fire for the very ingest-blindness it guards. A source-less tree stays a vacuous pass. - Disclose the [new_code] shallow-history skip as structured data on the check_gates MCP path (a {gate, reason} entry in skipped_gates, unified with the structural check-only skips, which gain reasons too) and make the reason discriminate a shallow checkout (names fetch-depth) from a genuinely young repository on both the CLI and MCP paths. - Write .codelore-ratchet.toml atomically through the shared write-to-temp + fsync + rename helper, so an interrupted write can no longer leave a 0-byte file; and reject a present-but-empty ratchet file as corrupt instead of parsing it to an all-None snapshot and silently rebaselining the gate. No file (initialize), a well-formed empty table (no floors configured), and a destroyed file (loud error) are now distinct states. --- CHANGELOG.md | 6 + crates/codelore-cli/src/check.rs | 52 +++++-- crates/codelore-cli/src/mcp.rs | 134 ++++++++++++++++-- crates/codelore-cli/tests/mcp_test.rs | 14 +- crates/codelore-lib/src/facts/mod.rs | 119 ++++++++++++++++ .../src/quality_gates/evaluators.rs | 39 +++++ crates/codelore-lib/src/quality_gates/mod.rs | 2 +- .../codelore-lib/src/quality_gates/ratchet.rs | 101 ++++++++++++- crates/codelore-lib/src/repo/gix_repo/mod.rs | 7 + crates/codelore-lib/src/repo/mod.rs | 17 +++ 10 files changed, 460 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7daf8c85..8ddf8329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ Conventional Commits format. All notable changes documented here. - **The SPA dashboard rendered only its header on browsers without `scheduler.yield` — every widget below the fold stayed blank on all Safari and iOS Safari, Chrome and Edge before 129, and Firefox before 142.** The cooperative widget-boot loop yields between widgets through a `yieldToMain` helper that prefers `scheduler.yield()` and otherwise falls back to a `MessageChannel`. The fallback's lazy state lived in two module-scope `let` bindings in a later-concatenated script, but the boot loop — defined in an earlier script of the same shared IIFE — calls `yieldToMain` during its first synchronous pass, before those bindings' declarations have run. On engines that expose `scheduler.yield()` the helper returned before touching them, which hid the defect on the common desktop-Chrome path; everywhere else the first yield read a binding inside its temporal dead zone, the un-awaited boot loop rejected, and only the first widget ever rendered — the header and quality-dimension tiles sat above a blank page. The fallback state now lives on the `yieldToMain` function object itself, a hoisted declaration reachable from the IIFE's first statement, so no dead-zone read can occur under any concatenation order. A headless regression test boots the dashboard with `scheduler` removed and asserts the full widget set renders. +- **`codelore check` and the `check_gates` MCP tool no longer pass green over an empty fact store.** A shallow `fetch-depth` checkout whose tip is a merge commit ingests zero history under the default merge filter (`include_merges = false`), leaving every gate with nothing to violate — the run printed `PASS (0 files evaluated)`, wrote `result=pass`, and exited 0, and under `--format sarif` or a `steps.*.outputs.result` read the "0 files" tell was invisible. Both gate paths now **witness the ingest**: a real HEAD over a zero-commit store is a hard error (exit 3 — the shallow/corrupted-repo bucket) that names `fetch-depth` as the likely cause, never a pass. As defence in depth, a shallow checkout that *did* ingest partial history is warned about loudly (naming `fetch-depth`) on the check output, and the `code_health_min` degraded sentinel now witnesses "the repository has analyzable source" by reading the HEAD tree directly rather than counting `complexity_metrics` — which derives from the same `changes ⋈ commits` join as the health rows and so could not fire for the very ingest-blindness it guards. A source-less tree (docs or config only) stays the honest vacuous pass it always was. + +- **The `[new_code]` gate's shallow-history skip is now disclosed as structured data on the `check_gates` MCP path and names the true cause on both paths.** When the repository's history does not reach past the `[new_code]` window there is no legacy baseline, so the gate skips — but a truncated `fetch-depth` checkout is indistinguishable from a genuinely young repository at that query, and the truncated case is the CI default. The MCP tool previously disclosed nothing, so an agent could not tell "the new-code gate passed" from "it did not run"; it now reports the skip in the `skipped_gates` array as a `{gate, reason}` entry alongside the structural check-only skips (which gain reasons too — one unified shape). Both the CLI disclosure and the MCP reason now **discriminate**: when the checkout is shallow (`.git/shallow` present) the reason names `fetch-depth` and advises re-fetching full history; otherwise it names a genuinely young repository. Exit codes and verdicts are unchanged. + +- **`.codelore-ratchet.toml` is written atomically, and a destroyed (empty) ratchet file is rejected loudly instead of silently rebaselining the regression gate.** The committed ratchet snapshot was written with a plain truncate-then-write, so a crash, OOM kill, or ENOSPC mid-write could leave a 0-byte file; that empty file parses cleanly to an all-`None` snapshot, which the ratchet reads as "everything improved" and then rewrites the floor from the current — possibly already regressed — run, disabling the regression gate for the whole team once the reset is committed. The write now routes through the same write-to-temp + fsync + atomic-rename helper every `analyze` output uses, so an interrupted write leaves the previous good file untouched. And `read_snapshot` now treats a present-but-empty (0-byte or whitespace-only) file as **corrupt** with a loud restore-or-delete error, keeping three states distinct that were previously conflated: no file (initialize), a well-formed empty `[ratchet]` table (no floors configured), and a destroyed file (the loud error). + ## [0.23.0] - 2026-07-28 ### Added diff --git a/crates/codelore-cli/src/check.rs b/crates/codelore-cli/src/check.rs index 9c2efa24..eb0069ac 100644 --- a/crates/codelore-cli/src/check.rs +++ b/crates/codelore-cli/src/check.rs @@ -101,6 +101,28 @@ pub(crate) fn run_check_cmd(args: &args::CheckArgs) -> Result<()> { let head_sha = repo.head_sha().context("get HEAD sha")?; let db = FactsDb::open_or_ingest_with_cache_root(&opts, &repo, &cache_root).context("ingest")?; + // Witness the ingest before any gate runs: a real HEAD over an empty commit + // store is the truncated-checkout signature (a shallow merge-tip fetch under + // the default merge filter ingests zero history), on which every gate would + // pass over no data. Turn that silent green into a hard, distinct error. + db.ensure_ingest_witnessed(&head_sha)?; + // Defence in depth: a shallow checkout that DID ingest a commit or two still + // carries only partial history, which quietly weakens every behavioral gate. + // Warn loudly and name the cause; SARIF mode keeps stdout a clean document. + // The same signal discriminates the `new_code` skip disclosure below (a + // truncated checkout reads identically to a young repository at that query). + let shallow_checkout = repo.is_shallow(); + if shallow_checkout { + let warning = "⚠ codelore check: shallow checkout detected (.git/shallow present) — \ + history is truncated by fetch-depth, so the behavioral gates (hotspots, \ + effort-exposure, new-code) evaluate only partial history. Re-run against full \ + history (fetch-depth: 0) for an authoritative verdict."; + if matches!(args.format, CheckFormat::Sarif) { + eprintln!("{warning}"); + } else { + println!("{warning}"); + } + } let ts = now_utc_ts(); // Open the external-findings sidecar without creating it, only when it @@ -131,7 +153,7 @@ pub(crate) fn run_check_cmd(args: &args::CheckArgs) -> Result<()> { // branches so both paths surface them; stderr keeps stdout a clean SARIF // document in --format sarif; suppressed under --quiet. if !args.quiet { - emit_gate_notices(&ledger_records); + emit_gate_notices(&ledger_records, shallow_checkout); } // One-per-run hint when the corpus lens is inactive — the check path always // computes code-health rows, which carry no corpus_percentile without an @@ -318,6 +340,7 @@ pub(crate) fn run_check_cmd(args: &args::CheckArgs) -> Result<()> { /// beside the rest of `run_check_cmd`'s reporting. fn emit_gate_notices( ledger_records: &[codelore_lib::cli_api::quality_gates::ledger::GateRunRecord], + shallow_checkout: bool, ) { for r in ledger_records { match (r.gate.as_str(), r.verdict.as_str()) { @@ -333,8 +356,12 @@ fn emit_gate_notices( ("code_health_min", "degraded") => eprintln!( " ⚠ code_health_min: degraded — health scan returned no rows on a non-empty repo" ), + ("new_code", "skipped") if shallow_checkout => eprintln!( + " ⚠ new_code: skipped — the checkout is shallow (fetch-depth): its history is truncated to within the {:.0}-day window, so there is no pre-window baseline to contrast the working set against. This is the checkout, not the repository — re-run against full history (fetch-depth: 0).", + r.threshold + ), ("new_code", "skipped") => eprintln!( - " ⚠ new_code: skipped — repository history is shallower than the {:.0}-day window (no pre-window baseline to contrast the working set against)", + " ⚠ new_code: skipped — the repository's history is shallower than the {:.0}-day window (a genuinely young repository), so there is no pre-window baseline to contrast the working set against.", r.threshold ), _ => {} @@ -435,6 +462,7 @@ fn eval_hotspot_gates( fn eval_code_health_gate( thresholds: &codelore_lib::cli_api::quality_gates::Thresholds, db: &codelore_lib::cli_api::facts::FactsDb, + repo: &impl codelore_lib::cli_api::repo::Repo, opts: &codelore_lib::cli_api::Options, ts: &str, head_sha: &str, @@ -457,14 +485,15 @@ fn eval_code_health_gate( return Ok(((Vec::new(), Vec::new()), code_health)); }; let ch_violations = evaluate_code_health_gate(thresholds, &code_health); - // Degraded: empty result when the repo has scorable files. - let degraded = code_health.is_empty() && { - db.query_row("SELECT COUNT(*) FROM complexity_metrics", [], |r| { - r.get::<_, i64>(0) - }) - .unwrap_or(0) - > 0 - }; + // Degraded: the health scan returned nothing, yet the repository actually + // carries analyzable source. The witness reads the HEAD tree directly rather + // than counting `complexity_metrics`, which derives from the same + // changes⋈commits join as the (empty) health set and so empties in lockstep + // — it cannot witness an ingest that went blind. A source-less tree (docs or + // config only) legitimately yields no rows and stays a vacuous pass. The `&&` + // short-circuit keeps the tree walk off every healthy run. + let degraded = code_health.is_empty() + && codelore_lib::cli_api::quality_gates::head_has_scorable_source(repo, opts); let worst = code_health .iter() .map(|r| r.score) @@ -576,7 +605,8 @@ fn evaluate_all_gates( violations.extend(hs_v); recs.extend(hs_r); - let ((ch_v, ch_r), code_health) = eval_code_health_gate(thresholds, db, opts, ts, head_sha)?; + let ((ch_v, ch_r), code_health) = + eval_code_health_gate(thresholds, db, repo, opts, ts, head_sha)?; violations.extend(ch_v); recs.extend(ch_r); diff --git a/crates/codelore-cli/src/mcp.rs b/crates/codelore-cli/src/mcp.rs index a97c7aa3..67d097b5 100644 --- a/crates/codelore-cli/src/mcp.rs +++ b/crates/codelore-cli/src/mcp.rs @@ -469,10 +469,26 @@ struct GateSummary { violation_count: usize, /// Individual gate violations, if any. violations: Vec, - /// Configured `[gates]` gates that this tool did NOT evaluate, so its - /// verdict is a subset of `codelore check` (which evaluates all of them). - /// Empty when nothing is skipped. See [`skipped_check_gates`]. - skipped_gates: Vec<&'static str>, + /// Configured gates that produced no verdict in this response, each with the + /// reason — so an empty `violations` list is distinguishable from "did not + /// run". Empty when nothing is skipped. See [`SkippedGate`]. + skipped_gates: Vec, +} + +/// One gate `check_gates` produced no verdict for, paired with why. A skip is +/// either *structural* — a gate this tool never evaluates because its +/// committed-tree input (the corpus lens, the external-findings sidecar, +/// degraded-gate handling) is `codelore check`-only — or *runtime*: a +/// `[new_code]` gate with no pre-window baseline in this checkout. Carrying the +/// reason beside the name is what lets a caller tell an empty `violations` list +/// ("all gates passed") apart from "this gate did not run". +#[derive(Debug, Serialize)] +struct SkippedGate { + /// The gate that produced no verdict. + gate: &'static str, + /// Why it produced none — a structural limit of this tool, or a runtime + /// condition of this repository. + reason: String, } /// The `[gates]` gates configured in `thresholds` that `check_gates` does not @@ -481,7 +497,7 @@ struct GateSummary { /// the corpus lens), so the disclosure is (configured gates) − (the set /// evaluated here). Returned in a stable declaration order; empty when nothing /// is skipped. -fn skipped_check_gates(thresholds: &Thresholds) -> Vec<&'static str> { +fn skipped_check_gates(thresholds: &Thresholds) -> Vec { // Gates this tool evaluates, kept beside the evaluation calls in // `check_gates`. A configured `[gates]` gate absent from this set is // disclosed as skipped, so a gate added to the config surfaces here until @@ -539,10 +555,50 @@ fn skipped_check_gates(thresholds: &Thresholds) -> Vec<&'static str> { configured .into_iter() .filter(|(name, set)| *set && !EVALUATED_HERE.contains(name)) - .map(|(name, _)| name) + .map(|(gate, _)| SkippedGate { + gate, + reason: structural_skip_reason(gate).to_owned(), + }) .collect() } +/// Why a configured `[gates]` gate is not evaluated by `check_gates`: its +/// committed-tree input is `codelore check`-only. +fn structural_skip_reason(gate: &str) -> &'static str { + match gate { + "max_findings_in_hot_files" => { + "reads the external-findings sidecar, which is `codelore check`-only \ + (run `codelore ingest-sarif`, then `codelore check`)" + } + "corpus_percentile_max" | "hotspot_anchored_max" => { + "depends on the calibration-corpus lens, which this tool does not carry; \ + `codelore check` is authoritative" + } + "fail_on_degraded" => "degraded-gate handling is `codelore check`-only", + _ => "evaluated only by `codelore check`, which is authoritative for it", + } +} + +/// The discriminating reason a `[new_code]` gate skipped at runtime. A shallow +/// checkout (fetch-depth truncated the pre-window history) and a genuinely young +/// repository both leave the window with no pre-window baseline and read +/// identically at the window-start query — but they are different causes with +/// different fixes, so the reason names fetch-depth when the checkout is shallow. +fn new_code_skip_reason(window_days: u32, shallow_checkout: bool) -> String { + if shallow_checkout { + format!( + "the checkout is shallow (fetch-depth): history is truncated to within the \ + {window_days}-day window, so there is no pre-window baseline. This is the checkout, \ + not the repository — re-run against full history (fetch-depth: 0)." + ) + } else { + format!( + "the repository's history is shallower than the {window_days}-day window (a genuinely \ + young repository), so there is no pre-window baseline." + ) + } +} + #[tool_router] impl CodeLoreServer { // ── repo_overview ───────────────────────────────────────────────────────── @@ -888,10 +944,14 @@ impl CodeLoreServer { name = "check_gates", description = "Evaluate `.codelore-thresholds.toml` quality gates at HEAD and return a JSON \ summary with verdict (pass/fail/no_thresholds), violation count, individual violations, \ - and a `skipped_gates` array naming any configured gate this tool did not evaluate. \ + and a `skipped_gates` array of `{gate, reason}` for every configured gate that produced \ + no verdict — so an empty `violations` list is distinguishable from a gate that did not run. \ This tool evaluates a subset of `codelore check`: the `max_findings_in_hot_files` and \ `corpus_percentile_max` gates, `--ratchet`, and degraded-gate handling remain check-only, \ so a config using those can make this verdict diverge — `codelore check` is authoritative. \ + A configured `[new_code]` gate that finds no pre-window baseline (a young repository, or a \ + shallow fetch-depth checkout) is reported here too, with a reason that names fetch-depth \ + when the checkout is shallow. \ Returns `no_thresholds` verdict when no config file is found. \ First call on a cold cache triggers history ingest." )] @@ -930,6 +990,12 @@ impl CodeLoreServer { let repo = GixRepo::open(&repo_path).map_err(|e| map_lib_err(&e))?; let db = FactsDb::open_or_ingest_with_cache_root(&opts, &repo, &default_cache_root()) .map_err(|e| map_lib_err(&e))?; + // Same ingest witness as `codelore check`: a real HEAD over an empty + // commit store is a truncated checkout on which every gate passes over + // no data. An agent must get a hard error, never a spurious green. + let head_sha = repo.head_sha().map_err(|e| map_lib_err(&e))?; + db.ensure_ingest_witnessed(&head_sha) + .map_err(|e| map_lib_err(&e))?; let mut violations: Vec = Vec::new(); @@ -969,9 +1035,11 @@ impl CodeLoreServer { // [new_code] two-band period gate — mirrors `codelore check`: reuses // the code-health rows and the effort-exposure window-start // machinery, evaluating the born + touched bands over the committed - // HEAD. A run whose history is shallower than the window produces no - // new_code violations here; the authoritative `codelore check` - // discloses that skip. + // HEAD. A run whose history is shallower than the window is disclosed + // as a runtime skip in `skipped_gates` below, with a reason that names + // fetch-depth when the checkout is shallow — so an empty violations + // list is never mistaken for "the new-code gate passed". + let mut new_code_skip: Option = None; if let Some(nc) = &thresholds.new_code { use codelore_lib::cli_api::analyses::new_code; let scope = new_code::run_new_code_scope(&db, &repo, &opts, nc.window_days, &ch) @@ -980,6 +1048,11 @@ impl CodeLoreServer { violations.extend( codelore_lib::cli_api::quality_gates::evaluate_new_code_rows(nc, &scope), ); + } else { + new_code_skip = Some(SkippedGate { + gate: "new_code", + reason: new_code_skip_reason(nc.window_days, repo.is_shallow()), + }); } } @@ -1008,11 +1081,13 @@ impl CodeLoreServer { } else { "fail" }; + let mut skipped_gates = skipped_check_gates(&thresholds); + skipped_gates.extend(new_code_skip); let summary = GateSummary { verdict: verdict.into(), violation_count: violations.len(), violations, - skipped_gates: skipped_check_gates(&thresholds), + skipped_gates, }; serde_json::to_string(&summary).map_err(internal) }) @@ -1505,8 +1580,8 @@ pub fn run_mcp_server( mod tests { use super::{ CodeHealthParams, DEFAULT_ROW_CAP, DeltaHealthParams, MAX_ROW_CAP, MEMO_CAPACITY, - ResultMemo, map_lib_err, memo_key, resolve_row_cap, serialize_capped_rows, - skipped_check_gates, + ResultMemo, map_lib_err, memo_key, new_code_skip_reason, resolve_row_cap, + serialize_capped_rows, skipped_check_gates, }; use codelore_lib::CodeLoreError; use codelore_lib::cli_api::quality_gates::Thresholds; @@ -1662,14 +1737,21 @@ mod tests { "[gates]\ncode_health_min = 50.0\nmax_findings_in_hot_files = 5\ncorpus_percentile_max = 0.9\n", ) .expect("parse thresholds"); + let skipped = skipped_check_gates(&thresholds); assert_eq!( - skipped_check_gates(&thresholds), + skipped.iter().map(|s| s.gate).collect::>(), vec![ "max_findings_in_hot_files", "corpus_percentile_max", "fail_on_degraded" ], ); + // Every skip carries a non-empty reason, so an empty violation list is + // distinguishable from "did not run". + assert!( + skipped.iter().all(|s| !s.reason.is_empty()), + "each skipped gate must disclose a reason" + ); } #[test] @@ -1693,7 +1775,29 @@ mod tests { // pass here is never mistaken for a full `codelore check` pass. let thresholds = Thresholds::from_text("[gates]\ncode_health_min = 50.0\n").expect("parse thresholds"); - assert_eq!(skipped_check_gates(&thresholds), vec!["fail_on_degraded"]); + let skipped = skipped_check_gates(&thresholds); + assert_eq!( + skipped.iter().map(|s| s.gate).collect::>(), + vec!["fail_on_degraded"] + ); + } + + #[test] + fn new_code_skip_reason_names_fetch_depth_only_when_shallow() { + // A shallow checkout and a genuinely young repository both leave the + // window without a baseline, but the disclosure must tell them apart: + // only the shallow case is about the checkout (fetch-depth), and only + // it should advise re-fetching full history. + let shallow = new_code_skip_reason(90, true); + assert!( + shallow.contains("fetch-depth") && shallow.contains("shallow"), + "shallow-checkout reason must name fetch-depth: {shallow}" + ); + let young = new_code_skip_reason(90, false); + assert!( + young.contains("young repository") && !young.contains("fetch-depth"), + "young-repository reason must not blame fetch-depth: {young}" + ); } #[test] diff --git a/crates/codelore-cli/tests/mcp_test.rs b/crates/codelore-cli/tests/mcp_test.rs index c9655cf3..51041b2e 100644 --- a/crates/codelore-cli/tests/mcp_test.rs +++ b/crates/codelore-cli/tests/mcp_test.rs @@ -393,7 +393,7 @@ fn mcp_check_gates_returns_verdict() { .as_array() .expect("check_gates: skipped_gates array") .iter() - .filter_map(|v| v.as_str()) + .filter_map(|v| v["gate"].as_str()) .collect(); assert_eq!( skipped, @@ -428,7 +428,7 @@ fn mcp_check_gates_discloses_skipped_gates() { .as_array() .expect("check_gates: skipped_gates array") .iter() - .filter_map(|v| v.as_str()) + .filter_map(|v| v["gate"].as_str()) .collect(); assert!( skipped.contains(&"max_findings_in_hot_files"), @@ -442,6 +442,16 @@ fn mcp_check_gates_discloses_skipped_gates() { !skipped.contains(&"fail_on_degraded"), "explicitly disabled degraded semantics must not be disclosed as skipped: {parsed}" ); + // Every disclosed skip carries a non-empty reason string, so a caller can + // tell an empty `violations` list ("all gates passed") from "did not run". + assert!( + parsed["skipped_gates"] + .as_array() + .expect("skipped_gates array") + .iter() + .all(|v| v["reason"].as_str().is_some_and(|r| !r.is_empty())), + "each skipped gate must carry a reason: {parsed}" + ); drop(stdin); let _ = child.wait(); diff --git a/crates/codelore-lib/src/facts/mod.rs b/crates/codelore-lib/src/facts/mod.rs index 842c48f2..826c3b33 100644 --- a/crates/codelore-lib/src/facts/mod.rs +++ b/crates/codelore-lib/src/facts/mod.rs @@ -546,6 +546,53 @@ impl FactsDb { .map_err(|e| CodeLoreError::Analysis(format!("query_row: {e}"))) } + /// Number of commits in the fact store — the persisted, cache-safe form of + /// [`ingest::IngestStats::commits_ingested`] (one row per ingested commit). + /// Unlike that in-memory counter it is readable after a cache HIT as well as + /// a fresh ingest; and unlike `complexity_metrics` / `changes` it is the raw + /// output of the commit walk — it does not derive from the `changes ⋈ + /// commits` join, so a blind walk that empties that join still leaves this + /// readable (and zero). That independence is what makes it a witness. + /// + /// # Errors + /// + /// [`CodeLoreError::Analysis`] on query failure. + pub fn commit_count(&self) -> Result { + self.query_row("SELECT COUNT(*) FROM commits", [], |r| r.get::<_, i64>(0)) + } + + /// Fail loudly when the walk ingested no commits while HEAD names a real + /// commit — the signature of a truncated checkout. A shallow `fetch-depth` + /// clone whose tip is a merge commit ingests zero commits under the default + /// merge filter, leaving an empty fact store on which every quality gate + /// finds nothing to violate and `codelore check` reports a green pass over + /// no data. Gating on the ingest count turns that silent pass into a hard, + /// distinct error. + /// + /// An empty `head_sha` (an unborn HEAD — `git init` with nothing committed) + /// is deliberately not this case and passes through: that is a genuinely + /// empty repository, the province of the empty-repository preflight, not a + /// truncated one. + /// + /// # Errors + /// + /// [`CodeLoreError::Repo`] (spec §6.6 exit 3 — the shallow/corrupted-repo + /// bucket that [`CodeLoreError::BlobNotFound`] also occupies) when HEAD is + /// real but no commits were ingested. + pub fn ensure_ingest_witnessed(&self, head_sha: &str) -> Result<()> { + if !head_sha.is_empty() && self.commit_count()? == 0 { + return Err(CodeLoreError::Repo( + "HEAD names a real commit but the walk ingested no history — the repository \ + checkout is truncated. A shallow clone (git fetch-depth, e.g. \ + actions/checkout's default fetch-depth: 1) whose tip is a merge commit ingests \ + zero commits under the default merge filter, so every quality gate would pass \ + over an empty fact store. Re-run against full history (fetch-depth: 0)." + .to_string(), + )); + } + Ok(()) + } + /// Internal raw-connection accessor. `pub(crate)` so the rest of /// `codelore-lib` (kamei, `quality_gates`, `output::spa`, ingest, etc.) /// can still reach the underlying `duckdb::Connection` for @@ -559,3 +606,75 @@ impl FactsDb { &self.conn } } + +#[cfg(test)] +mod ingest_witness_tests { + use super::FactsDb; + + /// Insert one inert commit row — the witness reads only the row count. + fn seed_commit(db: &FactsDb, rev: &str) { + db.conn() + .execute( + &format!( + "INSERT INTO commits (rev, author_email, author_name, committer_email, \ + canonical_author, date, committer_date, message, is_merge, parent_count) \ + VALUES ('{rev}', 'a@b.com', 'A', 'a@b.com', 'A', \ + TIMESTAMP '2026-01-01', TIMESTAMP '2026-01-01', 'm', false, 1)" + ), + [], + ) + .expect("insert commit"); + } + + #[test] + fn commit_count_reflects_ingested_rows() { + let db = FactsDb::new_in_memory().expect("db"); + assert_eq!(db.commit_count().expect("count"), 0); + seed_commit(&db, "c1"); + seed_commit(&db, "c2"); + assert_eq!(db.commit_count().expect("count"), 2); + } + + #[test] + fn witness_errors_on_real_head_with_zero_commits() { + // The truncated-checkout signature: HEAD resolves to a real commit but + // the walk ingested nothing (a shallow merge-tip checkout under the + // default merge filter). Must be a hard repo error, never a pass. + let db = FactsDb::new_in_memory().expect("db"); + let err = db + .ensure_ingest_witnessed("af53d17d1e3d64679d1691e75f82b65a2edb397a") + .expect_err("zero commits + real HEAD must be a hard error"); + assert_eq!( + err.exit_code(), + 3, + "a truncated checkout maps to the repo-error exit bucket" + ); + let msg = err.to_string(); + assert!( + msg.contains("fetch-depth"), + "the message must name the likely cause: {msg}" + ); + assert!( + msg.contains("truncated"), + "the message must name the condition: {msg}" + ); + } + + #[test] + fn witness_passes_when_commits_present() { + let db = FactsDb::new_in_memory().expect("db"); + seed_commit(&db, "c1"); + db.ensure_ingest_witnessed("af53d17d") + .expect("a store with history passes the witness"); + } + + #[test] + fn witness_ignores_unborn_head() { + // An empty head_sha is an unborn HEAD (git init, nothing committed) — a + // genuinely empty repository, not a truncated one. The witness must not + // fire so the empty-repository preflight owns that case. + let db = FactsDb::new_in_memory().expect("db"); + db.ensure_ingest_witnessed("") + .expect("an unborn HEAD is not a truncated checkout"); + } +} diff --git a/crates/codelore-lib/src/quality_gates/evaluators.rs b/crates/codelore-lib/src/quality_gates/evaluators.rs index 409140ca..def076fb 100644 --- a/crates/codelore-lib/src/quality_gates/evaluators.rs +++ b/crates/codelore-lib/src/quality_gates/evaluators.rs @@ -398,6 +398,45 @@ pub fn evaluate_code_health_gate( out } +/// Whether the HEAD tree carries at least one Tier-1 source file the code-health +/// scan should have scored — the independent witness for the `code_health_min` +/// degraded sentinel. +/// +/// The sentinel fires when the scan returned nothing yet the repository actually +/// carries analyzable source. It must decide "carries source" from a signal that +/// does NOT share the scan's failure mode: `complexity_metrics` (the obvious +/// proxy) is populated from the same `changes ⋈ commits` join the health rows +/// derive from, so a blind ingest empties it in lockstep with the health set and +/// a count over it cannot witness the very blindness the sentinel exists to +/// catch. Reading the HEAD tree straight from the repository sidesteps that: it +/// is non-zero whenever real source exists, and legitimately zero for a +/// source-less tree (docs / config only), which stays the honest vacuous pass it +/// should be rather than a spurious degraded verdict. +/// +/// The same `--exclude` / gitignore / git-metadata filter the ingest applies is +/// used here so the witnessed file universe matches the scan's own. Any read +/// failure yields `false` (treated as no scorable source): a missed degraded +/// flag beats a hard failure on a tree-walk edge case, matching the repo layer's +/// hint-not-contract convention for [`crate::repo::Repo::is_worktree_dirty`]. +/// +/// Call it only on the empty-health path — the caller's `&&` short-circuit keeps +/// the tree walk off every healthy run. +#[must_use] +pub fn head_has_scorable_source(repo: &R, opts: &crate::Options) -> bool { + let Ok(paths) = repo.tracked_paths_at_head() else { + return false; + }; + let Ok(filter) = crate::paths_filter::PathsFilter::from_opts(opts) else { + return false; + }; + paths.iter().any(|p| { + let rel = std::path::Path::new(p); + !crate::paths_filter::is_git_metadata(rel) + && !filter.is_excluded(rel, false) + && crate::complexity::Tier1Language::from_path(p).is_some() + }) +} + /// Pure inner comparison for the `corpus_percentile_max` gate. /// /// One violation per file whose `corpus_percentile` exceeds `max` (strictly diff --git a/crates/codelore-lib/src/quality_gates/mod.rs b/crates/codelore-lib/src/quality_gates/mod.rs index 02cb3d1f..620bf568 100644 --- a/crates/codelore-lib/src/quality_gates/mod.rs +++ b/crates/codelore-lib/src/quality_gates/mod.rs @@ -51,5 +51,5 @@ pub use evaluators::{ evaluate_diff_gate, evaluate_effort_exposure_gate, evaluate_effort_exposure_rows, evaluate_effort_exposure_rows_exempt, evaluate_familiarity_gate, evaluate_familiarity_rows, evaluate_finding_overlap_rows, evaluate_full_tree, evaluate_gate_thresholds, - evaluate_hotspot_anchored_rows, evaluate_new_code_rows, + evaluate_hotspot_anchored_rows, evaluate_new_code_rows, head_has_scorable_source, }; diff --git a/crates/codelore-lib/src/quality_gates/ratchet.rs b/crates/codelore-lib/src/quality_gates/ratchet.rs index ba3050f3..bae9614e 100644 --- a/crates/codelore-lib/src/quality_gates/ratchet.rs +++ b/crates/codelore-lib/src/quality_gates/ratchet.rs @@ -128,6 +128,25 @@ pub fn read_snapshot(repo_root: &Path) -> Result> { let raw = fs::read_to_string(&path).map_err(|e| { CodeLoreError::Analysis(format!("read ratchet file {}: {e}", path.display())) })?; + // A present-but-empty file is corrupt, not absent. An empty string parses + // cleanly to an all-`None` snapshot (every field is `#[serde(default)]`), + // which `evaluate_ratchet` reads as "everything improved" and then silently + // rebaselines the committed floor from the current — possibly regressed — + // run. Three states must stay distinct: NO file (`Ok(None)`, initialize); a + // DESTROYED file (0-byte or whitespace — a torn write, an empty merge + // resolution, manual truncation — the loud error here); and a well-formed + // file (parsed below, where an empty `[ratchet]` table legitimately means + // "no floors configured"). "The floors were destroyed" must never look like + // "no floors were configured". + if raw.trim().is_empty() { + return Err(CodeLoreError::Analysis(format!( + "ratchet file {} is present but empty — the committed baseline was destroyed \ + (a truncated write, an empty merge resolution, or manual truncation). An empty \ + ratchet must not silently rebaseline the gate: restore the file from version \ + control, or delete it to re-initialize the baseline from this run.", + path.display() + ))); + } // The typed parse rejects truncated/corrupt TOML and schema mismatches // alike — no separate untyped pre-parse is needed. let snap = toml::from_str::(&raw).map_err(|e| { @@ -150,8 +169,19 @@ pub fn write_snapshot(repo_root: &Path, snap: &RatchetSnapshot) -> Result<()> { # Ratchet tightens automatically on improvement; edit manually to relax.\n\n"; let body = toml::to_string_pretty(snap) .map_err(|e| CodeLoreError::Analysis(format!("serialize ratchet snapshot: {e}")))?; - fs::write(&path, format!("{header}{body}")) - .map_err(|e| CodeLoreError::Analysis(format!("write ratchet file {}: {e}", path.display()))) + let payload = format!("{header}{body}"); + // A committed baseline is a shared floor for the whole team, so the write + // must be all-or-nothing. Plain `fs::write` truncates then writes; a crash, + // OOM kill, or ENOSPC between the two would leave a 0-byte file that parses + // back to "everything improved" and silently rebaselines the gate. Route + // through the same write-to-temp + fsync + atomic-rename helper every + // analyze output uses, so an interrupted write leaves the previous good file + // untouched rather than a torn one. + crate::output::atomic_publish(&path, |tmp| { + fs::write(tmp, payload.as_bytes()).map_err(|e| { + CodeLoreError::Analysis(format!("write ratchet file {}: {e}", path.display())) + }) + }) } /// Compare current metrics against the snapshot and determine the outcome. @@ -314,6 +344,73 @@ mod tests { ); } + // ── destroyed baseline: 0-byte / whitespace-only → corrupt, not improved ── + + #[test] + fn empty_ratchet_file_is_corrupt_not_improved() { + // The torn-write signature: a 0-byte file. It parses to an all-`None` + // snapshot that would read as "everything improved" and silently + // rebaseline the committed floor. read_snapshot must instead fail loudly + // — distinct from a missing file (which legitimately initializes) and a + // well-formed no-floors file. + let dir = tmp(); + fs::write(dir.path().join(RATCHET_FILENAME), b"").unwrap(); + let err = read_snapshot(dir.path()).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("empty") && msg.contains("destroyed"), + "expected a loud destroyed-baseline error, got: {msg}" + ); + } + + #[test] + fn whitespace_only_ratchet_file_is_corrupt() { + // Whitespace-only content trims to empty and parses to all-`None` just + // like a 0-byte file — same corruption, same loud error. + let dir = tmp(); + fs::write(dir.path().join(RATCHET_FILENAME), b"\n \n").unwrap(); + assert!( + read_snapshot(dir.path()).is_err(), + "a whitespace-only ratchet file must be rejected as corrupt" + ); + } + + #[test] + fn well_formed_empty_table_is_no_floors_not_corrupt() { + // A serialized snapshot with no metrics set is the legitimate "no floors + // configured" state — distinct from destroyed — and must round-trip as + // an all-`None` snapshot rather than erroring. + let dir = tmp(); + fs::write(dir.path().join(RATCHET_FILENAME), b"[ratchet]\n").unwrap(); + let snap = read_snapshot(dir.path()) + .expect("a well-formed empty table must not be corrupt") + .expect("a present file returns Some"); + assert!(snap.ratchet.code_health_min_observed.is_none()); + assert!(snap.ratchet.red_effort_pct_observed.is_none()); + assert!(snap.ratchet.dependency_cycles_observed.is_none()); + } + + #[test] + fn write_snapshot_leaves_no_temp_stray() { + // Proves the write routes through atomic_publish: its write-to-temp + + // rename contract removes the process-unique `.tmp.` sibling on + // success, so a completed write leaves only the destination. + let dir = tmp(); + write_snapshot(dir.path(), &snapshot_from_metrics(&metrics_good())).unwrap(); + let strays: Vec<_> = fs::read_dir(dir.path()) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.contains(".tmp.")) + .collect(); + assert!( + strays.is_empty(), + "an atomic write must not orphan a temp file: {strays:?}" + ); + // And the published file is the real snapshot, readable back. + assert!(read_snapshot(dir.path()).unwrap().is_some()); + } + // ── regression: snapshot has BETTER values than current → exit-1 scenario ─ #[test] diff --git a/crates/codelore-lib/src/repo/gix_repo/mod.rs b/crates/codelore-lib/src/repo/gix_repo/mod.rs index 30bb666e..fa443bb0 100644 --- a/crates/codelore-lib/src/repo/gix_repo/mod.rs +++ b/crates/codelore-lib/src/repo/gix_repo/mod.rs @@ -277,6 +277,13 @@ impl Repo for GixRepo { repo.is_dirty().unwrap_or(false) } + fn is_shallow(&self) -> bool { + // `Repository::is_shallow()` reports a non-empty `shallow` grafts file in + // the repository's common git dir (worktree-correct via `common_dir()`), + // exactly the truncated-history signal a `fetch-depth` clone leaves. + self.inner.to_thread_local().is_shallow() + } + fn merge_or_rebase_in_progress(&self) -> bool { // `Repository::state()` reproduces git's own `wt-status` probe: it // inspects `rebase-apply/`, `rebase-merge/`, `CHERRY_PICK_HEAD`, diff --git a/crates/codelore-lib/src/repo/mod.rs b/crates/codelore-lib/src/repo/mod.rs index 43cb2e74..6f1f5deb 100644 --- a/crates/codelore-lib/src/repo/mod.rs +++ b/crates/codelore-lib/src/repo/mod.rs @@ -90,6 +90,23 @@ pub trait Repo: Send + Sync { false } + /// Whether the repository is a shallow clone — history truncated at a depth + /// boundary, with a non-empty `.git/shallow` grafts list, so commits beyond + /// the boundary (and the parents of the boundary commits) are absent. + /// + /// The gate paths consult this to warn that a verdict was computed over + /// partial history: a shallow `fetch-depth` checkout can leave the fact + /// store empty, or the new-code window without a pre-window baseline, and + /// the operator otherwise has no signal that the *checkout* — not the + /// repository — is the cause. + /// + /// Default impl returns `false` so backends without a cheap check can opt + /// out, mirroring [`is_worktree_dirty`](Self::is_worktree_dirty): a missed + /// warning is better than a hard failure on a detection edge case. + fn is_shallow(&self) -> bool { + false + } + /// Read the blob bytes at revision `rev` for `path` (POSIX- /// separated, repo-relative). `rev` is any git revision the backend /// can resolve — a commit SHA, `"HEAD"`, a tag, etc. Returns