diff --git a/hubs/cli-check.md b/hubs/cli-check.md index 589f944..25c4a9c 100644 --- a/hubs/cli-check.md +++ b/hubs/cli-check.md @@ -19,7 +19,7 @@ anchors: divergence (blocking the run) rather than being silently skipped, so a frontmatter typo can't pass as clean. at: surf-cli/src/check.rs > check_workspace - hash: c29434a58059 + hash: df30746fb6b2 refs: [] --- diff --git a/hubs/cli-for.md b/hubs/cli-for.md index 5f209e1..cf8b8b5 100644 --- a/hubs/cli-for.md +++ b/hubs/cli-for.md @@ -2,11 +2,14 @@ summary: surf for — reverse lookup of hubs/claims anchored into a file; read-only query. anchors: - claim: > - run normalizes the queried path to workspace-root-relative form, finds the matching claims, - and prints them grouped by hub (human) or as a versioned {version, path, matches} envelope - (JSON). It is a query, not a gate, so it always exits 0 whether or not anything matched. + run normalizes the queried path to workspace-root-relative form, then verifies it is a + regular file on disk — a nonexistent/mistyped path, a directory, or a trailing slash errors + (exit 1) rather than reporting "no hubs anchor", so a typo can't read as safe-to-edit. For a + real file it finds the matching claims and prints them grouped by hub (human) or as a + versioned {version, path, matches} envelope (JSON), always exiting 0 whether or not anything + matched. at: surf-cli/src/for_path.rs > run - hash: 0e15525b1340 + hash: 3ffb208cc1db - claim: > find collects every claim whose anchored file equals the queried path (matched on path only — no source parse), optionally narrowed to anchors whose first segment is the given symbol. diff --git a/hubs/cli-stats.md b/hubs/cli-stats.md index 728115a..3dc9f78 100644 --- a/hubs/cli-stats.md +++ b/hubs/cli-stats.md @@ -13,9 +13,10 @@ anchors: rubber-stamp numerator only when the claim's prose was unchanged. A claim-touch event is a commit that changed a file the claim anchors; it counts toward the in-place numerator when the claim's stored hash was updated in that same commit. Claim identity is its at: site(s), - and missing git history is a hard error rather than a silent zero. + and missing git history or an invalid hub glob in surf.toml is a hard error rather than a + silent zero or a quietly-narrowed hub set. at: surf-cli/src/stats.rs > compute - hash: 55561222d721 + hash: c58b950866ba refs: ["../docs/guides/stats.md"] --- diff --git a/surf-cli/src/check.rs b/surf-cli/src/check.rs index 7c836a4..478ae28 100644 --- a/surf-cli/src/check.rs +++ b/surf-cli/src/check.rs @@ -42,7 +42,7 @@ fn check_workspace( base: Option<&str>, files: &[String], ) -> Result> { - let scope = Scope::build(ws, base, files); + let scope = Scope::build(ws, base, files)?; // Enrichment always needs a ref; an explicit --base doubles as the diff base, else HEAD. let enrich_base = base.unwrap_or("HEAD"); @@ -93,15 +93,20 @@ struct Scope { } impl Scope { - fn build(ws: &Workspace, base: Option<&str>, files: &[String]) -> Scope { + fn build(ws: &Workspace, base: Option<&str>, files: &[String]) -> Result { // A bad ref / non-repo yields None — we fall back to a full check rather than // silently checking nothing. let changed = base.and_then(|b| git::changed_files(&ws.root, b)); + // Invalid glob *syntax* must fail loudly: silently dropping a `--files` pattern + // changes the gate's scope with no signal (#38). Zero *matches* stay fine. let globs = files .iter() - .filter_map(|p| glob::Pattern::new(p).ok()) - .collect(); - Scope { changed, globs } + .map(|p| { + glob::Pattern::new(p) + .map_err(|e| anyhow::anyhow!("invalid --files glob \"{p}\": {e}")) + }) + .collect::>>()?; + Ok(Scope { changed, globs }) } fn includes(&self, claim: &surf_core::Claim) -> bool { @@ -328,6 +333,17 @@ mod tests { .is_empty()); } + #[test] + fn invalid_files_glob_syntax_errors() { + // A malformed `--files` pattern must fail loudly, not silently widen/narrow scope (#38). + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write(root, "surf.toml", ""); + let err = check_workspace(&ws_at(root.to_path_buf()), None, &["src/[".to_string()]) + .unwrap_err(); + assert!(err.to_string().contains("src/["), "got: {err}"); + } + #[test] fn per_symbol_not_per_file() { // Anchor `add`; modify the *other* function in the same file. Must stay clean. diff --git a/surf-cli/src/for_path.rs b/surf-cli/src/for_path.rs index f1a509a..7023f46 100644 --- a/surf-cli/src/for_path.rs +++ b/surf-cli/src/for_path.rs @@ -1,11 +1,13 @@ //! `surf for [symbol]` — reverse lookup: which hubs/claims anchor into a file (#31). //! The inverse of authoring — pull up the claims governing a file before you edit it. Reuses the //! hub/anchor machinery and only matches on the anchored *path*, so it stays deterministic with -//! no model, network, or source parse. A query, not a gate: it always exits 0. +//! no model, network, or source parse. A query, not a gate: for an existing file it always exits +//! 0 (matched or not), but a path that isn't a regular file errors (exit 1) so a typo can't read +//! as "nothing anchors here, safe to edit" (#53). use crate::format::Format; use crate::workspace::Workspace; -use anyhow::Result; +use anyhow::{bail, Result}; use serde::Serialize; use std::process::ExitCode; use surf_core::{parse_anchor, REPORT_VERSION}; @@ -25,7 +27,15 @@ struct ForReport { } pub fn run(ws: &Workspace, path: &str, symbol: Option<&str>, format: Format) -> Result { + // A pre-edit safety check that can't tell "no claims" from "wrong path" is worse than + // useless — it greenlights editing a file that isn't the one you meant. Stat first, like + // `suggest` does for its globs (#30, #53). Resolve against the root so a root-relative + // query is checked where anchors actually live, not against the cwd. let query = normalize(ws, path); + if !ws.root.join(&query).is_file() { + bail!("no such file: {path} (path does not exist or is not a regular file)"); + } + let matches = find(ws, &query, symbol)?; match format { @@ -162,6 +172,26 @@ mod tests { assert_eq!(normalize(&ws, abs.to_str().unwrap()), "src/x.rs"); } + #[test] + fn run_errors_on_nonexistent_path() { + // A typo must not look like "nothing anchors here, safe to edit". + let (_t, ws) = ws_with(&[("hubs/a.md", HUB)]); + assert!(run(&ws, "src/x.rs", None, Format::Human).is_err()); + } + + #[test] + fn run_errors_on_directory() { + let (_t, ws) = ws_with(&[("hubs/a.md", HUB), ("src/x.rs", "fn foo() {}\n")]); + assert!(run(&ws, "src", None, Format::Human).is_err()); + } + + #[test] + fn run_succeeds_on_real_file_even_when_unanchored() { + // Genuinely-existing-but-unanchored keeps exit 0; only the wrong-path case errors. + let (_t, ws) = ws_with(&[("hubs/a.md", HUB), ("src/lonely.rs", "fn solo() {}\n")]); + assert!(run(&ws, "src/lonely.rs", None, Format::Human).is_ok()); + } + #[test] fn json_envelope_is_versioned() { let (_t, ws) = ws_with(&[("hubs/a.md", HUB)]); diff --git a/surf-cli/src/stats.rs b/surf-cli/src/stats.rs index b975bb1..170533b 100644 --- a/surf-cli/src/stats.rs +++ b/surf-cli/src/stats.rs @@ -77,12 +77,17 @@ pub fn run( } fn compute(ws: &Workspace, since: Option<&str>, until: Option<&str>) -> Result { + // A bad hub glob in surf.toml must fail loudly: silently dropping it excludes hubs from the + // metrics with no signal (#38). stats already fails loudly when git can't answer; do the same + // for malformed config rather than reporting on a quietly-narrowed hub set. let patterns: Vec = ws .config .hubs .iter() - .filter_map(|p| glob::Pattern::new(p).ok()) - .collect(); + .map(|p| { + glob::Pattern::new(p).map_err(|e| anyhow!("invalid hub glob \"{p}\" in surf.toml: {e}")) + }) + .collect::>>()?; // Unlike check's advisory git, stats *is* a history report: if git can't answer, fail loudly // rather than printing a misleading zero. @@ -244,6 +249,16 @@ mod tests { Workspace::discover(root).unwrap() } + #[test] + fn invalid_hub_glob_syntax_errors() { + // A malformed hub glob in surf.toml must fail loudly, not silently exclude hubs (#38). + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write(root, "surf.toml", "hubs = [\"hubs/[.md\"]\n"); + let err = compute(&ws(root), None, None).unwrap_err(); + assert!(err.to_string().contains("hubs/["), "got: {err}"); + } + #[test] fn rubber_stamp_when_hash_changes_but_prose_does_not() { let tmp = tempfile::tempdir().unwrap();