From 4c0f0aa8793dd00abf5c4b298a0b8c7ad004fb84 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 11:49:54 -0600 Subject: [PATCH 01/21] fix: exit codes & CI gating (batch 1: changelog cmd, coverage cmd, path confinement, test module wiring) --- src/commands/changelog.rs | 33 ++++++++++++++++++++++++++++- src/util.rs | 44 +++++++++++++++++++++++++++++++++++++++ tests/integration.rs | 3 +++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/commands/changelog.rs b/src/commands/changelog.rs index 8cbfeda3..0df0ccc3 100644 --- a/src/commands/changelog.rs +++ b/src/commands/changelog.rs @@ -7,7 +7,7 @@ use crate::config::load_config; use crate::types; pub fn cmd_changelog(root: &Path, range: &str, format: types::OutputFormat) { - let (from_ref, to_ref) = match changelog::parse_range(range) { + let (from_ref, to_ref, three_dot) = match changelog::parse_range_full(range) { Some(r) => r, None => { eprintln!( @@ -18,6 +18,37 @@ pub fn cmd_changelog(root: &Path, range: &str, format: types::OutputFormat) { } }; + // Validate BOTH endpoints before comparing: an unresolvable ref used to be + // silently treated as an empty tree, fabricating a plausible changelog + // with exit 0 — exactly the wrong output for CI (#418). + let to_ref = match changelog::resolve_ref(root, &to_ref) { + Ok(_) => to_ref, + Err(e) => { + eprintln!("{} {e}", "Error:".red().bold()); + process::exit(1); + } + }; + let from_ref = match changelog::resolve_ref(root, &from_ref) { + Ok(_) => from_ref, + Err(e) => { + eprintln!("{} {e}", "Error:".red().bold()); + process::exit(1); + } + }; + + // Three-dot range (A...B): compare from the real merge-base, not from A. + let from_ref = if three_dot { + match changelog::merge_base(root, &from_ref, &to_ref) { + Ok(base) => base, + Err(e) => { + eprintln!("{} {e}", "Error:".red().bold()); + process::exit(1); + } + } + } else { + from_ref + }; + let config = load_config(root); let report = changelog::generate_changelog(root, &config.specs_dir, &from_ref, &to_ref); diff --git a/src/util.rs b/src/util.rs index f89eb73d..03dd76f2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -35,6 +35,27 @@ pub fn safe_regex(pattern: &str) -> Option { .ok() } +/// Confine a user-supplied path (e.g. a `depends_on` entry) to the project +/// root. Returns the joined path when `rel` is a relative path with no `..` +/// escape and no absolute/prefix component; `None` otherwise. Use this to +/// keep dependency declarations from validating (or reading) files outside +/// the project — `/etc/passwd` must never pass as a spec dependency. +pub fn confine_path_to_root(root: &std::path::Path, rel: &str) -> Option { + use std::path::Component; + let path = std::path::Path::new(rel); + if rel.is_empty() || path.is_absolute() { + return None; + } + for component in path.components() { + match component { + Component::Normal(_) | Component::CurDir => {} + // ParentDir could escape the root; RootDir/Prefix are absolute. + _ => return None, + } + } + Some(root.join(path)) +} + #[cfg(test)] mod tests { use super::*; @@ -58,4 +79,27 @@ mod tests { fn test_safe_regex_invalid() { assert!(safe_regex(r"[invalid").is_none()); } + + #[test] + fn test_confine_path_to_root_accepts_relative() { + let root = std::path::Path::new("/proj"); + assert_eq!( + confine_path_to_root(root, "specs/a/a.spec.md"), + Some(root.join("specs/a/a.spec.md")) + ); + assert_eq!( + confine_path_to_root(root, "./specs/a.spec.md"), + Some(root.join("./specs/a.spec.md")) + ); + } + + #[test] + fn test_confine_path_to_root_rejects_escapes() { + let root = std::path::Path::new("/proj"); + // Absolute paths and `..` traversal must never validate (#444). + assert_eq!(confine_path_to_root(root, "/etc/passwd"), None); + assert_eq!(confine_path_to_root(root, "../outside.spec.md"), None); + assert_eq!(confine_path_to_root(root, "specs/../../etc/passwd"), None); + assert_eq!(confine_path_to_root(root, ""), None); + } } diff --git a/tests/integration.rs b/tests/integration.rs index 2ffe272a..14cb5dd2 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -25,3 +25,6 @@ mod change; #[path = "integration/comment.rs"] mod comment; + +#[path = "integration/regression_w1.rs"] +mod regression_w1; From d670526d5740f795937e0c549210ad98b57fd7b9 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 11:52:35 -0600 Subject: [PATCH 02/21] fix: coverage gate + missing-file reporting (batch 2) --- src/commands/coverage.rs | 6 ++++-- src/output.rs | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/commands/coverage.rs b/src/commands/coverage.rs index ef04d95c..c5ff5586 100644 --- a/src/commands/coverage.rs +++ b/src/commands/coverage.rs @@ -6,7 +6,8 @@ use crate::types; use crate::validator::{compute_coverage, get_schema_table_names}; use super::{ - build_schema_columns, compute_exit_code, exit_with_status, load_and_discover, run_validation, + build_schema_columns, compute_exit_code, default_enforcement, exit_with_status, + load_and_discover, run_validation, }; pub fn cmd_coverage( @@ -26,7 +27,7 @@ pub fn cmd_coverage( let enforcement = enforcement.unwrap_or(if strict { types::EnforcementMode::Strict } else { - config.enforcement + default_enforcement(&config) }); let schema_tables = get_schema_table_names(root, &config); let schema_columns = build_schema_columns(root, &config); @@ -79,6 +80,7 @@ pub fn cmd_coverage( "loc_total": coverage.total_loc, "modules": modules, "uncovered_files": uncovered_files, + "missing_files": coverage.missing_files, }); println!("{}", serde_json::to_string_pretty(&output).unwrap()); // Gate the exit code for machine consumers too (was an unconditional diff --git a/src/output.rs b/src/output.rs index 2021f4d9..1b80668a 100644 --- a/src/output.rs +++ b/src/output.rs @@ -70,9 +70,21 @@ pub fn print_coverage_report(coverage: &types::CoverageReport) { } } - if coverage.unspecced_files.is_empty() { + // Files a spec references but that do not exist on disk must never sit + // under a green "all referenced" line — name them as failures instead. + if !coverage.missing_files.is_empty() { + println!( + "\n Referenced by specs but missing on disk ({}):", + coverage.missing_files.len() + ); + for file in &coverage.missing_files { + println!(" {} {file}", "✗".red()); + } + } + + if coverage.unspecced_files.is_empty() && coverage.missing_files.is_empty() { println!(" {} All source files referenced by specs", "✓".green()); - } else { + } else if !coverage.unspecced_files.is_empty() { let uncovered_loc: usize = coverage.unspecced_file_loc.iter().map(|(_, l)| l).sum(); println!( "\n Files not in any spec ({}, {} LOC uncovered):", @@ -244,6 +256,7 @@ mod tests { specced_loc: 1, loc_coverage_percent: loc_pct, unspecced_file_loc: Vec::new(), + missing_files: Vec::new(), } } From a14d8f5d63c1040b259acd1ce227a8094cb3072d Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 11:53:44 -0600 Subject: [PATCH 03/21] fix: comment command gated via compute_exit_code (batch 3) --- src/commands/comment.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/commands/comment.rs b/src/commands/comment.rs index a835f2ec..79b60868 100644 --- a/src/commands/comment.rs +++ b/src/commands/comment.rs @@ -9,7 +9,9 @@ use crate::ignore::IgnoreRules; use crate::types; use crate::validator::{compute_coverage, get_schema_table_names}; -use super::{build_schema_columns, compute_exit_code, load_and_discover, run_validation}; +use super::{ + build_schema_columns, compute_exit_code, default_enforcement, load_and_discover, run_validation, +}; pub fn cmd_comment( root: &Path, @@ -29,7 +31,7 @@ pub fn cmd_comment( let enforcement = enforcement.unwrap_or(if strict { types::EnforcementMode::Strict } else { - config.enforcement + default_enforcement(&config) }); // Use the same validation pipeline as `check` for consistent results From 20fba42e13a8331f31af49a2c54472cd2c2c6d1a Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 11:56:43 -0600 Subject: [PATCH 04/21] fix: deps gates (--strict warnings, --require-coverage, visualization paths) (batch 4) --- src/commands/deps.rs | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/commands/deps.rs b/src/commands/deps.rs index 67eaa507..3c616bc5 100644 --- a/src/commands/deps.rs +++ b/src/commands/deps.rs @@ -6,7 +6,17 @@ use crate::config::load_config; use crate::deps; use crate::types; -pub fn cmd_deps(root: &Path, strict: bool, format: types::OutputFormat, mermaid: bool, dot: bool) { +use super::{compute_exit_code, default_enforcement, load_and_discover}; + +pub fn cmd_deps( + root: &Path, + strict: bool, + enforcement: Option, + require_coverage: Option, + format: types::OutputFormat, + mermaid: bool, + dot: bool, +) { let config = load_config(root); // --mermaid or --dot: output graph visualization and exit @@ -140,6 +150,37 @@ pub fn cmd_deps(root: &Path, strict: bool, format: types::OutputFormat, mermaid: if !report.errors.is_empty() || strict_fail { process::exit(1); } + + // --require-coverage gate (#419): was completely inert. Evaluate the same + // coverage computation `specsync coverage` uses and fail below the + // threshold. Honored in every output format; JSON/machine formats gate + // silently via the exit code (handled by not printing here when Json). + if let Some(req) = require_coverage { + let (_, spec_files) = load_and_discover(root, true); + let coverage = crate::validator::compute_coverage(root, &spec_files, &config); + let enforcement = + enforcement.unwrap_or_else(|| default_enforcement(&config)); + let code = compute_exit_code(0, 0, strict, enforcement, &coverage, Some(req)); + if format != types::OutputFormat::Json { + if code == 0 { + println!( + " {} Coverage {}% meets --require-coverage {req}%", + "✓".green(), + coverage.coverage_percent + ); + } else { + eprintln!( + "{} {req}%: actual coverage is {}% ({} file(s) missing specs)", + "--require-coverage".red(), + coverage.coverage_percent, + coverage.unspecced_files.len() + ); + } + } + if code != 0 { + process::exit(code); + } + } } /// Render the dependency graph as a Mermaid flowchart diagram. From 47be188095cac4fa9d345b9dee0334dcf6fdc69b Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 11:59:26 -0600 Subject: [PATCH 05/21] fix: generate/score honor gate flags on all output paths (batch 5) --- src/commands/generate.rs | 7 ++++--- src/commands/score.rs | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/commands/generate.rs b/src/commands/generate.rs index d934cb50..dad9fc55 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -10,7 +10,8 @@ use crate::types; use crate::validator::{compute_coverage, get_schema_table_names}; use super::{ - build_schema_columns, compute_exit_code, exit_with_status, load_and_discover, run_validation, + build_schema_columns, compute_exit_code, default_enforcement, exit_with_status, + load_and_discover, run_validation, }; #[allow(clippy::too_many_arguments)] @@ -50,7 +51,7 @@ fn cmd_generate_all( let enforcement = enforcement.unwrap_or(if strict { types::EnforcementMode::Strict } else { - config.enforcement + default_enforcement(&config) }); let schema_tables = get_schema_table_names(root, &config); let schema_columns = build_schema_columns(root, &config); @@ -212,7 +213,7 @@ fn cmd_generate_batch( let enforcement = enforcement.unwrap_or(if strict { types::EnforcementMode::Strict } else { - config.enforcement + default_enforcement(&config) }); let coverage = compute_coverage(root, &spec_files, &config); diff --git a/src/commands/score.rs b/src/commands/score.rs index 2e6ee808..81392045 100644 --- a/src/commands/score.rs +++ b/src/commands/score.rs @@ -6,7 +6,8 @@ use crate::types; use crate::validator::compute_coverage; use super::{ - compute_exit_code, exit_with_status, filter_by_status, filter_specs, load_and_discover, + compute_exit_code, default_enforcement, exit_with_status, filter_by_status, filter_specs, + load_and_discover, }; #[allow(clippy::too_many_arguments)] @@ -34,12 +35,15 @@ pub fn cmd_score( if strict { types::EnforcementMode::Strict } else { - crate::config::load_config(root).enforcement + default_enforcement(&crate::config::load_config(root)) } }); let gate_requested = require_coverage.is_some() || !matches!(enforcement, types::EnforcementMode::Warn); - let (config, all_spec_files) = load_and_discover(root, gate_requested); + // JSON mode must ALSO bypass the no-spec early-exit: that path prints plain + // human text ("No spec files found…"), breaking CI parsers exactly in the + // bootstrap case (#441). The JSON below already handles zero specs. + let (config, all_spec_files) = load_and_discover(root, gate_requested || json); let spec_files = filter_specs(root, &all_spec_files, spec_filters); let spec_files = filter_by_status(&spec_files, exclude_status, only_status); let scores: Vec = spec_files @@ -107,6 +111,31 @@ pub fn cmd_score( } } + // --strict quality gate (#441): F-grade specs — or nothing scored at all, + // where the bar is unenforceable — must fail the process. JSON/CSV gate + // silently via the exit code to keep stdout machine-parseable. + if strict { + let f_grades = project + .spec_scores + .iter() + .filter(|s| s.grade == "F") + .count(); + if project.total_specs == 0 || f_grades > 0 { + if !json && !matches!(format, types::OutputFormat::Csv) { + eprintln!( + "{}: {} — failing the quality gate", + "--strict mode".red(), + if project.total_specs == 0 { + "no specs were scored".to_string() + } else { + format!("{f_grades} spec(s) scored F") + } + ); + } + std::process::exit(1); + } + } + // Honor the global gate flags. `score` is advisory by default (Warn config → // exit 0), but `--require-coverage`, `--enforcement enforce-new`, and a strict // config now gate the exit code — previously they were silent no-ops here. From 88b61d10b03d41a5929e4f600a64ca7441eadd6d Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:01:04 -0600 Subject: [PATCH 06/21] fix: report gating (--strict/--enforcement/--require-coverage), stale+incomplete as failures (batch 6) --- src/commands/report.rs | 95 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/commands/report.rs b/src/commands/report.rs index b14cca39..3f761130 100644 --- a/src/commands/report.rs +++ b/src/commands/report.rs @@ -7,15 +7,25 @@ use crate::parser; use crate::types; use crate::validator::compute_coverage; -use super::{filter_by_status, load_and_discover}; +use super::{compute_exit_code, default_enforcement, filter_by_status, load_and_discover}; +#[allow(clippy::too_many_arguments)] pub fn cmd_report( root: &Path, format: types::OutputFormat, stale_threshold: usize, exclude_status: &[String], only_status: &[String], + strict: bool, + enforcement: Option, + require_coverage: Option, ) { + let enforcement = enforcement.unwrap_or(if strict { + types::EnforcementMode::Strict + } else { + let config = crate::config::load_config(root); + default_enforcement(&config) + }); let (config, all_spec_files) = load_and_discover(root, true); let spec_files = filter_by_status(&all_spec_files, exclude_status, only_status); let coverage = compute_coverage(root, &spec_files, &config); @@ -26,6 +36,7 @@ pub fn cmd_report( module_name: String, source_files: Vec, coverage_pct: f64, + status: Option, stale: bool, stale_commits_behind: usize, incomplete: bool, @@ -81,9 +92,11 @@ pub fn cmd_report( continue; } let behind = git_commits_since(root, &spec_commit, source_file); + // Always track the real drift: `commits_behind` must reflect + // sub-threshold drift too, not only once the module is stale. + max_behind = max_behind.max(behind); if behind >= stale_threshold { stale = true; - max_behind = max_behind.max(behind); } } } @@ -135,6 +148,7 @@ pub fn cmd_report( module_name, source_files: fm.files.clone(), coverage_pct: cov, + status: fm.status.clone(), stale, stale_commits_behind: max_behind, incomplete, @@ -163,6 +177,7 @@ pub fn cmd_report( serde_json::json!({ "module": m.module_name, "spec_path": m.spec_path, + "status": m.status, "source_files": m.source_files, "coverage_pct": (m.coverage_pct * 100.0).round() / 100.0, "stale": m.stale, @@ -185,6 +200,57 @@ pub fn cmd_report( "modules": module_json, }); println!("{}", serde_json::to_string_pretty(&output).unwrap()); + // Machine consumers get the gate via the exit code alone (printing + // the human gate message would corrupt the JSON on stdout). + std::process::exit(compute_exit_code( + stale_count + incomplete_count, + 0, + strict, + enforcement, + &coverage, + require_coverage, + )); + } + types::OutputFormat::Csv => { + println!("module,spec_path,status,coverage_pct,stale,commits_behind,incomplete"); + for m in &modules { + println!( + "{},{},{},{:.0},{},{},{}", + m.module_name, + m.spec_path, + m.status.as_deref().unwrap_or(""), + m.coverage_pct, + m.stale, + m.stale_commits_behind, + m.incomplete, + ); + } + println!( + "SUMMARY,,,{overall_coverage:.1},{stale_count},,{incomplete_count}", + ); + } + types::OutputFormat::Markdown | types::OutputFormat::Github => { + println!("## Spec Coverage Report\n"); + println!( + "**Overall:** {}/{} files covered ({:.1}%) ", + coverage.specced_file_count, coverage.total_source_files, overall_coverage, + ); + println!( + "**Modules:** {total_modules} total, {stale_count} stale, {incomplete_count} incomplete\n" + ); + println!("| Module | Status | Coverage | Stale | Commits Behind | Incomplete |"); + println!("|--------|--------|----------|-------|----------------|------------|"); + for m in &modules { + println!( + "| {} | {} | {:.0}% | {} | {} | {} |", + m.module_name, + m.status.as_deref().unwrap_or(""), + m.coverage_pct, + if m.stale { "yes" } else { "no" }, + m.stale_commits_behind, + if m.incomplete { "yes" } else { "no" }, + ); + } } _ => { println!( @@ -275,4 +341,29 @@ pub fn cmd_report( println!(); } } + + // CI gating (#430): every enforcement path must be able to fail the + // process. Stale and incomplete modules count as failures; `--enforcement + // warn` still exits 0, `enforce-new` gates on unspecced files, and + // `--require-coverage` gates on real file coverage. CSV is a machine + // format — gate silently via the exit code like JSON (handled above). + let failures = stale_count + incomplete_count; + if matches!(format, types::OutputFormat::Csv) { + std::process::exit(compute_exit_code( + failures, + 0, + strict, + enforcement, + &coverage, + require_coverage, + )); + } + super::exit_with_status( + failures, + 0, + strict, + enforcement, + &coverage, + require_coverage, + ); } From f6ba585125ff9d85b177a2abb3ac1b53e47e41c9 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:04:03 -0600 Subject: [PATCH 07/21] fix: diff explicit --base wins, loud HEAD fallback, --strict refuses fallback, fail loud on git errors (batch 7) --- src/commands/diff.rs | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/commands/diff.rs b/src/commands/diff.rs index a1b08054..7542c542 100644 --- a/src/commands/diff.rs +++ b/src/commands/diff.rs @@ -9,15 +9,37 @@ use crate::types; use super::load_and_discover; -pub fn cmd_diff(root: &Path, base: &str, format: types::OutputFormat) { +pub fn cmd_diff(root: &Path, base: Option<&str>, format: types::OutputFormat, strict: bool) { let (config, spec_files) = load_and_discover(root, false); - // Auto-detect PR context: when base is default "HEAD" and we're in a GitHub - // Actions pull_request event, compare against the PR's base branch instead. - let effective_base = if base == "HEAD" { - detect_pr_base().unwrap_or_else(|| base.to_string()) - } else { - base.to_string() + // An explicit --base always wins over environment auto-detection (a literal + // `--base HEAD` used to be hijacked by the PR-context fallback, since HEAD + // doubled as the "not specified" sentinel). + let effective_base = match base { + Some(b) => b.to_string(), + None => match detect_pr_base() { + Some(b) => b, + None => { + // Silent HEAD fallback = silent no-op in the default CI states + // (detached HEAD, push events): the working tree vs HEAD is + // empty by construction in a clean checkout, so nothing is + // ever compared while the run greens. Say so loudly. + eprintln!( + "specsync: no PR base detected (GITHUB_EVENT_NAME is not a pull_request* event); comparing against HEAD" + ); + eprintln!( + " In a clean CI checkout this compares nothing — pass --base to choose a real comparison base." + ); + if strict { + eprintln!( + "{} no usable base ref and none given — refusing to silently diff HEAD", + "--strict mode".red() + ); + process::exit(1); + } + "HEAD".to_string() + } + }, }; let base = effective_base.as_str(); @@ -318,7 +340,8 @@ pub fn cmd_diff(root: &Path, base: &str, format: types::OutputFormat) { /// or `None` otherwise. fn detect_pr_base() -> Option { let event = std::env::var("GITHUB_EVENT_NAME").ok()?; - if event != "pull_request" && event != "pull_request_target" { + // GitHub sets GITHUB_BASE_REF for all three PR-flavored events. + if event != "pull_request" && event != "pull_request_target" && event != "pull_request_review" { return None; } let base_ref = std::env::var("GITHUB_BASE_REF").ok()?; From 900326cf78e175efa56a32ccbb4b862f0f0bf032 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:06:22 -0600 Subject: [PATCH 08/21] fix: remote registry fetch (v4 path first, token auth) + W1 regression tests (batch 8) --- src/registry.rs | 79 +++++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/src/registry.rs b/src/registry.rs index 3fed0ab2..f23bd4f7 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -63,28 +63,31 @@ pub struct RemoteSpec { pub body: String, } -/// Fetch a spec file's raw content from a GitHub repo. -/// -/// `repo` is `owner/repo`, `spec_path` is the relative path from the registry. -pub fn fetch_remote_spec(repo: &str, spec_path: &str) -> Result { - let url = format!("https://raw.githubusercontent.com/{repo}/HEAD/{spec_path}"); - +/// GET a raw file from GitHub, attaching `GITHUB_TOKEN` (or `GH_TOKEN`) as an +/// Authorization header when set — documented private-repo support, previously +/// never actually sent (#422). +fn github_raw_get(url: &str) -> Result { let agent = ureq::Agent::new_with_config( ureq::config::Config::builder() .timeout_global(Some(Duration::from_secs(10))) .build(), ); - let mut response = agent - .get(&url) + let mut request = agent.get(url); + if let Ok(token) = std::env::var("GITHUB_TOKEN") + .or_else(|_| std::env::var("GH_TOKEN")) + .map(|t| t.trim().to_string()) + && !token.is_empty() + { + request = request.header("Authorization", &format!("Bearer {token}")); + } + + let mut response = request .call() .map_err(|e| format!("HTTP request failed: {e}"))?; if response.status() != 200 { - return Err(format!( - "HTTP {} — could not fetch {spec_path} from {repo}", - response.status() - )); + return Err(format!("HTTP {}", response.status())); } response @@ -93,6 +96,15 @@ pub fn fetch_remote_spec(repo: &str, spec_path: &str) -> Result .map_err(|e| format!("Failed to read response body: {e}")) } +/// Fetch a spec file's raw content from a GitHub repo. +/// +/// `repo` is `owner/repo`, `spec_path` is the relative path from the registry. +pub fn fetch_remote_spec(repo: &str, spec_path: &str) -> Result { + let url = format!("https://raw.githubusercontent.com/{repo}/HEAD/{spec_path}"); + github_raw_get(&url) + .map_err(|e| format!("{e} — could not fetch {spec_path} from {repo}")) +} + /// Parse a fetched spec into its relevant metadata for verification. pub fn parse_remote_spec(module: &str, content: &str) -> Option { use crate::parser; @@ -112,35 +124,26 @@ pub fn parse_remote_spec(module: &str, content: &str) -> Option { }) } -/// Fetch `specsync-registry.toml` from a GitHub repo's default branch. +/// Fetch the registry from a GitHub repo's default branch. /// /// `repo` is in `owner/repo` format (e.g. `corvid-labs/algochat`). -/// Tries the GitHub raw content URL for the file at repo root. +/// Fetches the canonical v4 location `.specsync/registry.toml` first (what +/// `init-registry` publishes and the docs promise), falling back to the legacy +/// root `specsync-registry.toml` for un-migrated repos (#422 — previously only +/// the legacy path was fetched, so every tool-published registry 404'd). pub fn fetch_remote_registry(repo: &str) -> Result { - let url = format!("https://raw.githubusercontent.com/{repo}/HEAD/{REGISTRY_FILENAME}"); - - let agent = ureq::Agent::new_with_config( - ureq::config::Config::builder() - .timeout_global(Some(Duration::from_secs(10))) - .build(), - ); - - let mut response = agent - .get(&url) - .call() - .map_err(|e| format!("HTTP request failed: {e}"))?; - - if response.status() != 200 { - return Err(format!( - "HTTP {} — {repo} may not have a {REGISTRY_FILENAME}", - response.status() - )); - } - - let body = response - .body_mut() - .read_to_string() - .map_err(|e| format!("Failed to read response body: {e}"))?; + let v4_url = format!("https://raw.githubusercontent.com/{repo}/HEAD/{V4_REGISTRY_RELATIVE}"); + let legacy_url = format!("https://raw.githubusercontent.com/{repo}/HEAD/{REGISTRY_FILENAME}"); + + let body = match github_raw_get(&v4_url) { + Ok(body) => body, + Err(v4_err) => github_raw_get(&legacy_url).map_err(|legacy_err| { + format!( + "{v4_err} — {repo} may not have a {V4_REGISTRY_RELATIVE} \ + (legacy {REGISTRY_FILENAME} also failed: {legacy_err})" + ) + })?, + }; let entry = parse_registry(&body).ok_or_else(|| format!("Failed to parse registry from {repo}"))?; From 5eaf683d8ddf78d97830365995a88aef7b9dfa83 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:08:10 -0600 Subject: [PATCH 09/21] test: W1 integration regression tests for exit-code/CI-gating fixes (batch 9) --- tests/integration/regression_w1.rs | 466 +++++++++++++++++++++++++++++ 1 file changed, 466 insertions(+) create mode 100644 tests/integration/regression_w1.rs diff --git a/tests/integration/regression_w1.rs b/tests/integration/regression_w1.rs new file mode 100644 index 00000000..195ee186 --- /dev/null +++ b/tests/integration/regression_w1.rs @@ -0,0 +1,466 @@ +//! Regression tests for the W1 exit-code / CI-gating workstream. +//! +//! Issues: #418 (changelog), #419 (deps), #422 (resolve --remote), +//! #425 (coverage), #430 (report), #431 (diff), #441 (score), #444 (resolve local). + +use crate::helpers::*; +use predicates::prelude::*; +use std::fs; +use tempfile::TempDir; + +fn git_init(root: &std::path::Path) { + for args in [ + vec!["init"], + vec!["config", "user.email", "test@test.com"], + vec!["config", "user.name", "Test"], + ] { + std::process::Command::new("git") + .args(&args) + .current_dir(root) + .output() + .unwrap(); + } +} + +fn git_commit_all(root: &std::path::Path, msg: &str) { + std::process::Command::new("git") + .args(["add", "."]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["commit", "-m", msg]) + .current_dir(root) + .output() + .unwrap(); +} + +// ─── #425: coverage gates on missing referenced files ──────────────────── + +#[test] +fn coverage_missing_referenced_file_exits_nonzero_by_default() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + // Spec references a file that does not exist on disk. + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/service.ts", "src/auth/ghost.ts"]), + ) + .unwrap(); + + specsync() + .args(["coverage", "--root"]) + .arg(root) + .assert() + .failure(); +} + +#[test] +fn coverage_missing_referenced_file_warn_enforcement_exits_zero() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/service.ts", "src/auth/ghost.ts"]), + ) + .unwrap(); + + specsync() + .args(["coverage", "--enforcement", "warn", "--root"]) + .arg(root) + .assert() + .success(); +} + +#[test] +fn coverage_json_includes_missing_files() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/service.ts", "src/auth/ghost.ts"]), + ) + .unwrap(); + + let output = specsync() + .args(["coverage", "--json", "--root"]) + .arg(root) + .output() + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let missing = json["missing_files"] + .as_array() + .expect("missing_files must be an array"); + assert!( + missing.iter().any(|f| f.as_str().unwrap_or("").contains("ghost.ts")), + "missing_files should list the referenced-but-absent file: {json}" + ); +} + +// ─── #430: report honors --require-coverage ────────────────────────────── + +#[test] +fn report_require_coverage_above_actual_exits_1() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + + specsync() + .args(["report", "--require-coverage", "101", "--root"]) + .arg(&root) + .assert() + .failure(); +} + +#[test] +fn report_require_coverage_at_actual_exits_0() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + + specsync() + .args(["report", "--require-coverage", "100", "--root"]) + .arg(&root) + .assert() + .success(); +} + +// ─── #419: deps scalar depends_on + dedupe + --require-coverage ───────── + +#[test] +fn deps_scalar_depends_on_is_not_silently_dropped() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + // Scalar depends_on (invalid YAML shape) pointing at a module with no spec. + // Before #419 it was silently dropped; now it must surface as a missing dep. + let spec = valid_spec("auth", &["src/auth/service.ts"]) + .replace("depends_on: []", "depends_on: nonexistent-module"); + fs::write(root.join("specs/auth/auth.spec.md"), spec).unwrap(); + + let output = specsync() + .args(["deps", "--root"]) + .arg(root) + .output() + .unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + combined.contains("nonexistent-module"), + "scalar depends_on must be honored, got: {combined}" + ); + assert!(!output.status.success(), "missing dep must fail deps"); +} + +#[test] +fn deps_duplicate_depends_on_entries_deduped() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + let spec = valid_spec("auth", &["src/auth/service.ts"]).replace( + "depends_on: []", + "depends_on:\n - nonexistent-module\n - nonexistent-module", + ); + fs::write(root.join("specs/auth/auth.spec.md"), spec).unwrap(); + + let output = specsync() + .args(["deps", "--root"]) + .arg(root) + .output() + .unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let occurrences = combined.matches("nonexistent-module").count(); + assert_eq!( + occurrences, 1, + "duplicate depends_on entries must be deduped, got: {combined}" + ); +} + +#[test] +fn deps_require_coverage_gate_enforced() { + let tmp = TempDir::new().unwrap(); + let root = setup_minimal_project(&tmp); + // Add an unspecced source file so coverage drops below 100%. + fs::write(root.join("src/extra.ts"), "export function extra() {}\n").unwrap(); + + specsync() + .args(["deps", "--require-coverage", "100", "--root"]) + .arg(&root) + .assert() + .failure(); +} + +// ─── #444: resolve local deps gate the exit code ───────────────────────── + +fn resolve_project(dep: &str) -> (TempDir, std::path::PathBuf) { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().to_path_buf(); + write_config(&root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write( + root.join("src/auth/service.ts"), + "export function login() {}\n", + ) + .unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + let spec = valid_spec("auth", &["src/auth/service.ts"]) + .replace("depends_on: []", &format!("depends_on:\n - {dep}")); + fs::write(root.join("specs/auth/auth.spec.md"), spec).unwrap(); + (tmp, root) +} + +#[test] +fn resolve_local_missing_path_exits_1_without_strict() { + let (_tmp, root) = resolve_project("src/auth/nonexistent.ts"); + specsync() + .args(["resolve", "--root"]) + .arg(&root) + .assert() + .failure(); +} + +#[test] +fn resolve_local_outside_root_traversal_exits_1() { + let (_tmp, root) = resolve_project("../outside.ts"); + let output = specsync() + .args(["resolve", "--root"]) + .arg(&root) + .output() + .unwrap(); + assert!(!output.status.success()); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + combined.contains("outside") || combined.contains("escapes"), + "traversal dep must be reported, got: {combined}" + ); +} + +#[test] +fn resolve_malformed_cross_project_ref_exits_1() { + let (_tmp, root) = resolve_project("owner/repo@"); + let output = specsync() + .args(["resolve", "--root"]) + .arg(&root) + .output() + .unwrap(); + assert!(!output.status.success()); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + combined.contains("malformed") || combined.contains("owner/repo@"), + "malformed ref must be reported, got: {combined}" + ); +} + +#[test] +fn resolve_all_ok_exits_0() { + let (_tmp, root) = resolve_project("src/auth/service.ts"); + specsync() + .args(["resolve", "--root"]) + .arg(&root) + .assert() + .success(); +} + +// ─── #431: diff base-ref fallback ──────────────────────────────────────── + +#[test] +fn diff_head_fallback_prints_loud_stderr_warning() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + git_init(root); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/service.ts"]), + ) + .unwrap(); + git_commit_all(root, "initial"); + + let output = specsync() + .args(["diff", "--root"]) + .arg(root) + .output() + .unwrap(); + assert!(output.status.success(), "diff fallback should still succeed"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("HEAD") || stderr.to_lowercase().contains("fallback"), + "HEAD fallback must print a loud stderr warning, got: {stderr}" + ); +} + +#[test] +fn diff_strict_refuses_head_fallback() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + git_init(root); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/service.ts"]), + ) + .unwrap(); + git_commit_all(root, "initial"); + + specsync() + .args(["diff", "--strict", "--root"]) + .arg(root) + .assert() + .failure(); +} + +// ─── #418: changelog validates refs ────────────────────────────────────── + +#[test] +fn changelog_bogus_ref_exits_1() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + git_init(root); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/a.ts"), "export function a() {}\n").unwrap(); + git_commit_all(root, "initial"); + + specsync() + .args(["changelog", "HEAD..bogus-ref-that-does-not-exist", "--root"]) + .arg(root) + .assert() + .failure(); +} + +#[test] +fn changelog_valid_range_exits_0() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + git_init(root); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/a.ts"), "export function a() {}\n").unwrap(); + git_commit_all(root, "initial"); + fs::write(root.join("src/a.ts"), "export function a() {}\nexport function b() {}\n").unwrap(); + git_commit_all(root, "second"); + + specsync() + .args(["changelog", "HEAD~1..HEAD", "--root"]) + .arg(root) + .assert() + .success(); +} + +// ─── #441: score zero-spec JSON ────────────────────────────────────────── + +#[test] +fn score_json_with_zero_specs_outputs_valid_json() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/a.ts"), "export function a() {}\n").unwrap(); + + let output = specsync() + .args(["score", "--json", "--root"]) + .arg(root) + .output() + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&output.stdout) + .expect("score --json with zero specs must emit valid JSON"); + assert!(json["specs"].is_array(), "JSON must include specs array"); + assert_eq!(json["specs"].as_array().unwrap().len(), 0); +} + +#[test] +fn score_strict_with_zero_specs_exits_1() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/a.ts"), "export function a() {}\n").unwrap(); + + specsync() + .args(["score", "--strict", "--json", "--root"]) + .arg(root) + .assert() + .failure(); +} + +// ─── Default enforcement: errors gate even without flags ──────────────── + +#[test] +fn check_validation_errors_exit_nonzero_by_default() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + // Spec references a file that does not exist → validation error. + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/ghost.ts"]), + ) + .unwrap(); + + specsync() + .args(["check", "--root"]) + .arg(root) + .assert() + .failure(); +} + +#[test] +fn check_validation_errors_warn_enforcement_exits_zero() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/auth")).unwrap(); + fs::write(root.join("src/auth/service.ts"), "export function login() {}\n").unwrap(); + fs::create_dir_all(root.join("specs/auth")).unwrap(); + fs::write( + root.join("specs/auth/auth.spec.md"), + valid_spec("auth", &["src/auth/ghost.ts"]), + ) + .unwrap(); + + specsync() + .args(["check", "--enforcement", "warn", "--root"]) + .arg(root) + .assert() + .success(); +} From bd5147ef8b501171f30fc34b57f888fc8b97fcc1 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:10:22 -0600 Subject: [PATCH 10/21] fix: main wiring (enforcement/require-coverage threading, compute_exit_code unit tests) (batch 10) --- src/main.rs | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 495b7c25..2e329d72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -172,8 +172,17 @@ fn run() { remote, verify, cache_ttl, - } => commands::resolve::cmd_resolve(&root, remote || verify, verify, cache_ttl), - Command::Diff { base } => commands::diff::cmd_diff(&root, &base, format), + } => commands::resolve::cmd_resolve( + &root, + remote || verify, + verify, + cache_ttl, + format, + cli.strict, + ), + Command::Diff { base } => { + commands::diff::cmd_diff(&root, base.as_deref(), format, cli.strict) + } Command::Hooks { action } => commands::hooks::cmd_hooks(&root, action), Command::Agents { action } => commands::agents::cmd_agents(&root, action), Command::Compact { keep, dry_run } => commands::compact::cmd_compact(&root, keep, dry_run), @@ -185,9 +194,15 @@ fn run() { Command::Issues { create } => commands::issues::cmd_issues(&root, format, create), Command::New { name, full } => commands::new::cmd_new(&root, &name, full), Command::Wizard => commands::wizard::cmd_wizard(&root), - Command::Deps { mermaid, dot } => { - commands::deps::cmd_deps(&root, cli.strict, format, mermaid, dot) - } + Command::Deps { mermaid, dot } => commands::deps::cmd_deps( + &root, + cli.strict, + cli.enforcement, + cli.require_coverage, + format, + mermaid, + dot, + ), Command::Import { source, id, @@ -217,6 +232,9 @@ fn run() { stale_threshold, &cli.exclude_status, &cli.only_status, + cli.strict, + cli.enforcement, + cli.require_coverage, ), Command::Comment { pr, base } => commands::comment::cmd_comment( &root, @@ -314,6 +332,7 @@ mod tests { specced_loc: 0, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], } } @@ -341,6 +360,7 @@ mod tests { specced_loc: 0, loc_coverage_percent: 0, unspecced_file_loc: vec![], + missing_files: vec![], } } From a1e2816e0269ddf453b764e79e219e4c2b6b6d70 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:13:13 -0600 Subject: [PATCH 11/21] fix: comment rendering updated for CoverageReport.missing_files (batch 11) --- src/comment.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/comment.rs b/src/comment.rs index a6146656..c7e095d2 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -447,6 +447,7 @@ mod tests { specced_loc: 1000, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], }; let output = render_check_comment( 5, @@ -479,6 +480,7 @@ mod tests { specced_loc: 800, loc_coverage_percent: 80, unspecced_file_loc: vec![], + missing_files: vec![], }; let errors = vec![ "specs/auth.spec.md: Missing required section: Purpose".to_string(), @@ -523,6 +525,7 @@ mod tests { specced_loc: 0, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], }; let output = render_check_comment(0, 0, 0, 0, &[], &[], &[], &coverage, true, None, None); assert!(output.contains("Generated by [specsync]")); @@ -542,6 +545,7 @@ mod tests { specced_loc: 0, loc_coverage_percent: 0, unspecced_file_loc: vec![], + missing_files: vec![], }; let output = render_check_comment(0, 0, 0, 0, &[], &[], &[], &coverage, true, None, None); assert!(output.contains("...and 5 more")); @@ -559,6 +563,7 @@ mod tests { specced_loc: 1, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], }; let errors = vec![format!( "specs/large.spec.md: {}", @@ -592,6 +597,7 @@ mod tests { specced_loc: 500, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], }; let body = render_comment_body(&violations, &coverage, Some("owner/repo"), Some("main")); assert!(body.contains("SpecSync: Failed")); @@ -616,6 +622,7 @@ mod tests { specced_loc: 500, loc_coverage_percent: 100, unspecced_file_loc: vec![], + missing_files: vec![], }; let body = render_comment_body(&violations, &coverage, Some("owner/repo"), Some("main")); assert!(body.contains("SpecSync: Passed")); From 8ec60e3e2d75272478a7ed0ad0bec2af72d30b69 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:16:34 -0600 Subject: [PATCH 12/21] =?UTF-8?q?fix:=20types=20=E2=80=94=20CoverageReport?= =?UTF-8?q?.missing=5Ffiles,=20SpecSyncConfig.enforcement=5Fset=20(batch?= =?UTF-8?q?=2012)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/types.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/types.rs b/src/types.rs index afe86b22..d0452369 100644 --- a/src/types.rs +++ b/src/types.rs @@ -193,6 +193,10 @@ pub struct CoverageReport { pub loc_coverage_percent: usize, /// (file_path, line_count) sorted by LOC descending. pub unspecced_file_loc: Vec<(String, usize)>, + /// Files referenced by a spec's `files:` list that do not exist on disk. + /// They count toward the coverage denominator but can never be covered, + /// so a `--require-coverage` gate cannot pass vacuously over broken specs. + pub missing_files: Vec, } /// Controls export extraction granularity. @@ -316,6 +320,14 @@ pub struct SpecSyncConfig { #[serde(default)] pub enforcement: EnforcementMode, + /// Whether `enforcement` was explicitly set in the loaded config file + /// (not serialized — set at runtime by the config loader). Lets gate + /// commands distinguish an explicit opt-in `warn` (exit 0 on failures) + /// from an unset enforcement, which must gate on errors so failures are + /// not silently green in CI. + #[serde(skip)] + pub enforcement_set: bool, + /// Lifecycle transition guards — configurable rules that must pass before /// a spec can be promoted/transitioned. #[serde(default)] @@ -813,6 +825,7 @@ impl Default for SpecSyncConfig { task_archive_days: None, github: None, enforcement: EnforcementMode::default(), + enforcement_set: false, lifecycle: LifecycleConfig::default(), companions: CompanionConfig::default(), config_path: None, From 997309917aaeb30de34863085f1197463a02e933 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:19:11 -0600 Subject: [PATCH 13/21] =?UTF-8?q?fix:=20resolve=20=E2=80=94=20local=20deps?= =?UTF-8?q?=20gate=20exit=20code,=20malformed=20refs=20surfaced,=20one=20v?= =?UTF-8?q?erdict/exit=20(closes=20#444,=20part=20of=20#422)=20(batch=2013?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/resolve.rs | 294 ++++++++++++++++++++++++++++++++-------- 1 file changed, 238 insertions(+), 56 deletions(-) diff --git a/src/commands/resolve.rs b/src/commands/resolve.rs index 490a8096..6b34c4c6 100644 --- a/src/commands/resolve.rs +++ b/src/commands/resolve.rs @@ -79,10 +79,42 @@ impl SpecCache { } } -pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { +/// Verdict for a single local dependency entry. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LocalVerdict { + /// The path exists and is confined to the project root. + Ok, + /// The path does not exist. + NotFound, + /// The path escapes the project root (absolute or `..` traversal). + OutsideRoot, +} + +impl LocalVerdict { + fn as_str(self) -> &'static str { + match self { + LocalVerdict::Ok => "ok", + LocalVerdict::NotFound => "not-found", + LocalVerdict::OutsideRoot => "outside-root", + } + } +} + +pub fn cmd_resolve( + root: &Path, + remote: bool, + verify: bool, + cache_ttl: u64, + format: crate::types::OutputFormat, + strict: bool, +) { + let json = matches!(format, crate::types::OutputFormat::Json); let (_config, spec_files) = load_and_discover(root, false); let mut cross_refs: Vec<(String, String, String)> = Vec::new(); - let mut local_refs: Vec<(String, String, bool)> = Vec::new(); + let mut local_refs: Vec<(String, String, LocalVerdict)> = Vec::new(); + // depends_on entries that look like refs but don't parse (e.g. `owner/repo@` + // with no module) — previously these vanished silently (#444). + let mut malformed_refs: Vec<(String, String)> = Vec::new(); // Track local spec exports for bidirectional checking let mut local_exports: HashMap> = HashMap::new(); @@ -114,43 +146,93 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { for dep in &parsed.frontmatter.depends_on { if validator::is_cross_project_ref(dep) { - if let Some((repo, module)) = validator::parse_cross_project_ref(dep) { - cross_refs.push((spec_path.clone(), repo.to_string(), module.to_string())); + match validator::parse_cross_project_ref(dep) { + Some((repo, module)) => { + cross_refs.push((spec_path.clone(), repo.to_string(), module.to_string())) + } + // Looks cross-project (`owner/repo@…`) but doesn't parse — + // surface it instead of dropping it silently. + None => malformed_refs.push((spec_path.clone(), dep.clone())), } + } else if dep.contains('@') { + // `repo@` / `@module` shapes: not local paths, not valid refs. + malformed_refs.push((spec_path.clone(), dep.clone())); } else { - let exists = root.join(dep).exists(); - local_refs.push((spec_path.clone(), dep.clone(), exists)); + // Confine to the project root: absolute paths and `..` + // traversal entries must never validate (#444). + let verdict = match crate::util::confine_path_to_root(root, dep) { + Some(path) if path.exists() => LocalVerdict::Ok, + Some(_) => LocalVerdict::NotFound, + None => LocalVerdict::OutsideRoot, + }; + local_refs.push((spec_path.clone(), dep.clone(), verdict)); } } } - println!( - "\n--- {} ------------------------------------------------", - "Dependency Resolution".bold() - ); + if !json { + println!( + "\n--- {} ------------------------------------------------", + "Dependency Resolution".bold() + ); + } - if local_refs.is_empty() && cross_refs.is_empty() { - println!("\n No dependencies declared in any spec."); + if local_refs.is_empty() && cross_refs.is_empty() && malformed_refs.is_empty() { + if json { + println!("{\"dependencies\":[],\"passed\":true}"); + } else { + println!("\n No dependencies declared in any spec."); + } return; } - if !local_refs.is_empty() { - println!("\n {} Local dependencies:", "Local".bold()); - for (spec, dep, exists) in &local_refs { - if *exists { - println!(" {} {spec} -> {dep}", "✓".green()); - } else { - println!(" {} {spec} -> {dep} (not found)", "✗".red()); + if !json && !local_refs.is_empty() { + println!("\n {}", "Local dependencies:".bold()); + for (spec, dep, verdict) in &local_refs { + match verdict { + LocalVerdict::Ok => println!(" {} {spec} -> {dep}", "✓".green()), + LocalVerdict::NotFound => { + println!(" {} {spec} -> {dep} (not found)", "✗".red()) + } + LocalVerdict::OutsideRoot => println!( + " {} {spec} -> {dep} (path escapes the project root)", + "✗".red() + ), } } } + if !json && !malformed_refs.is_empty() { + for (spec, dep) in &malformed_refs { + println!( + " {} {spec} -> {dep} (malformed reference — expected `owner/repo@module` or a project-relative path)", + "✗".red() + ); + } + } + + // Local verdicts are computed up-front so both the text and JSON paths — + // and the final exit code — share one source of truth (#444: unresolved + // local deps must fail the process even without --strict). + let unresolved_local = local_refs + .iter() + .filter(|(_, _, v)| *v != LocalVerdict::Ok) + .count() + + malformed_refs.len(); + + // Remote verification state — shared by the text report, the JSON report, + // and the final exit code so there is exactly one verdict (#422). + let mut remote_errors = 0usize; + let mut fetch_failures: Vec = Vec::new(); + let mut drift_issues: Vec = Vec::new(); + if !cross_refs.is_empty() { - println!("\n {} Cross-project references:", "Remote".bold()); + if !json { + println!("\n {} Cross-project references:", "Remote".bold()); + } if remote { // Fetch remote registries to verify cross-project refs - let mut remote_errors = 0; // Group refs by repo to avoid duplicate fetches let mut repos: HashMap> = HashMap::new(); @@ -160,10 +242,16 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { .or_insert_with(|| match registry::fetch_remote_registry(repo) { Ok(reg) => Some(reg), Err(e) => { - eprintln!( - " {} Failed to fetch registry for {repo}: {e}", - "!".yellow() - ); + // A failed fetch is a failed verification, not a + // neutral "?" — the ref cannot be confirmed, so the + // run must not pass (#422). + fetch_failures.push(repo.clone()); + if !json { + eprintln!( + " {} Failed to fetch registry for {repo}: {e}", + "!".yellow() + ); + } None } }); @@ -174,40 +262,48 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { match repos.get(repo) { Some(Some(reg)) => { if reg.has_spec(module) { - println!(" {} {spec} -> {repo}@{module}", "✓".green()); + if !json { + println!(" {} {spec} -> {repo}@{module}", "✓".green()); + } } else { - println!( - " {} {spec} -> {repo}@{module} (module not in registry)", - "✗".red() - ); + if !json { + println!( + " {} {spec} -> {repo}@{module} (module not in registry)", + "✗".red() + ); + } remote_errors += 1; } } Some(None) => { - println!( - " {} {spec} -> {repo}@{module} (registry fetch failed)", - "?".yellow() - ); + if !json { + println!( + " {} {spec} -> {repo}@{module} (registry fetch failed)", + "✗".red() + ); + } } None => { - println!( - " {} {spec} -> {repo}@{module} (no registry)", - "?".yellow() - ); + if !json { + println!( + " {} {spec} -> {repo}@{module} (no registry)", + "?".yellow() + ); + } } } } - if remote_errors > 0 { + if remote_errors > 0 && !json { println!( "\n {} {remote_errors} cross-project ref(s) could not be verified", - "Warning:".yellow() + "Error:".red().bold() ); } // Phase 2: Deep content verification (--verify) if verify { - let drift_issues = verify_remote_specs( + let drift_issues_found = verify_remote_specs( &cross_refs, &repos, &local_exports, @@ -215,14 +311,13 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { root, cache_ttl, ); + drift_issues = drift_issues_found; - if !drift_issues.is_empty() { + if !drift_issues.is_empty() && !json { println!("\n {} Content verification:", "Verify".bold()); - let mut drift_count = 0; for result in &drift_issues { for issue in &result.issues { - drift_count += 1; match issue { DriftIssue::Deprecated { status } => { println!( @@ -270,7 +365,7 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { } } - let drift_errors: usize = drift_issues + let drift_error_count: usize = drift_issues .iter() .flat_map(|r| &r.issues) .filter(|i| { @@ -281,26 +376,37 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { }) .count(); - if drift_errors > 0 { + if drift_error_count > 0 { println!( - "\n {} {drift_errors} drift issue(s) detected — specs have diverged from remote", + "\n {} {drift_error_count} drift issue(s) detected — specs have diverged from remote", "Error:".red().bold() ); - std::process::exit(1); } else { println!( - "\n {} {drift_count} warning(s), no breaking drift", - "Info:".cyan() + "\n {} {} warning(s), no breaking drift", + "Info:".cyan(), + drift_issues.iter().flat_map(|r| &r.issues).count() + ); + } + } else if !json { + // Never print this when zero registries were actually + // fetched — "verified" over zero successful fetches is a + // false green (#422). + if fetch_failures.is_empty() { + println!( + "\n {} All cross-project references verified — no drift detected", + "✓".green() + ); + } else { + println!( + "\n {} could not verify {} repo(s) — registry fetch failed", + "✗".red(), + fetch_failures.len() ); } - } else { - println!( - "\n {} All cross-project references verified — no drift detected", - "✓".green() - ); } } - } else { + } else if !json { for (spec, repo, module) in &cross_refs { println!(" {} {spec} -> {repo}@{module}", "→".cyan()); } @@ -312,6 +418,82 @@ pub fn cmd_resolve(root: &Path, remote: bool, verify: bool, cache_ttl: u64) { println!(" Use --verify for deep content verification and drift detection."); } } + + // ─── One verdict, one exit code (#422/#444) ──────────────────────── + let drift_errors: usize = drift_issues + .iter() + .flat_map(|r| &r.issues) + .filter(|i| matches!(i, DriftIssue::Deprecated { .. } | DriftIssue::MissingExport { .. })) + .count(); + let drift_warnings: usize = drift_issues.iter().flat_map(|r| &r.issues).count() - drift_errors; + + let mut failure_reasons: Vec = Vec::new(); + if unresolved_local > 0 { + failure_reasons.push(format!("{unresolved_local} unresolved local dependency(ies)")); + } + if remote_errors > 0 { + failure_reasons.push(format!( + "{remote_errors} cross-project ref(s) not found in remote registry" + )); + } + if !fetch_failures.is_empty() { + failure_reasons.push(format!( + "registry fetch failed for {} repo(s): {}", + fetch_failures.len(), + fetch_failures.join(", ") + )); + } + if drift_errors > 0 { + failure_reasons.push(format!("{drift_errors} drift issue(s) detected")); + } + // --strict makes verification warnings (non-bidirectional, unparsable or + // unfetchable remote spec content) fatal, matching `check --strict`. + if strict && drift_warnings > 0 { + failure_reasons.push(format!( + "{drift_warnings} verification warning(s) treated as errors (--strict)" + )); + } + let passed = failure_reasons.is_empty(); + + if json { + let local_json: Vec = local_refs + .iter() + .map(|(spec, dep, verdict)| { + serde_json::json!({"spec": spec, "dep": dep, "status": verdict.as_str()}) + }) + .collect(); + let cross_json: Vec = cross_refs + .iter() + .map(|(spec, repo, module)| { + serde_json::json!({"spec": spec, "repo": repo, "module": module}) + }) + .collect(); + let malformed_json: Vec = malformed_refs + .iter() + .map(|(spec, dep)| serde_json::json!({"spec": spec, "dep": dep})) + .collect(); + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "passed": passed, + "local": local_json, + "cross_project": cross_json, + "malformed": malformed_json, + "fetch_failures": fetch_failures, + "failures": failure_reasons, + })) + .unwrap() + ); + } else if !passed { + eprintln!("\n{}", "Dependency resolution FAILED:".red().bold()); + for reason in &failure_reasons { + eprintln!(" {} {reason}", "✗".red()); + } + } + + if !passed { + std::process::exit(1); + } } /// Deep-verify remote spec content for drift. From 475a3197ba9636929521563aee6ff5e166512af5 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:22:11 -0600 Subject: [PATCH 14/21] =?UTF-8?q?fix:=20cli=20=E2=80=94=20global=20--enfor?= =?UTF-8?q?cement=20flag,=20--base=20Option=20for=20diff=20(batch=2014)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 68e30105..70421062 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -148,11 +148,11 @@ pub enum Command { }, /// Show export changes since last commit (useful for CI/PR comments) Diff { - /// Git ref to compare against (default: HEAD). - /// In GitHub Actions PR context, auto-detects the base branch - /// from GITHUB_BASE_REF when set to HEAD. - #[arg(long, default_value = "HEAD")] - base: String, + /// Git ref to compare against (default: auto-detect the PR base from + /// GITHUB_BASE_REF in GitHub Actions pull_request* events, else HEAD). + /// An explicit value always wins over environment auto-detection. + #[arg(long)] + base: Option, }, /// Manage agent instruction files and git hooks for spec awareness Hooks { From f74fc28180bdca329fa5ab0a63fc17850c5cc980 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:25:18 -0600 Subject: [PATCH 15/21] =?UTF-8?q?fix:=20commands=20core=20=E2=80=94=20defa?= =?UTF-8?q?ult=5Fenforcement,=20compute=5Fexit=5Fcode,=20exit=5Fwith=5Fsta?= =?UTF-8?q?tus=20(batch=2015)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/mod.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 1b976378..d0ac0526 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -669,6 +669,24 @@ fn colorize_subscore(score: u32) -> String { } } +/// Resolve the enforcement mode when the user did NOT pass `--enforcement`. +/// +/// Consistent convention across all gate commands (`check`, `coverage`, +/// `report`, `score`, `generate`, `comment`): +/// - an explicit `enforcement` key in the config file is honored as-is +/// (including `warn`, which always exits 0); +/// - when enforcement is not configured anywhere, the default GATES ON +/// ERRORS (`Strict`-like): validation errors exit 1. Warnings remain +/// non-blocking unless `--strict` is also passed. This is the only safe +/// default for CI — a command that prints "1 failed" must not exit 0. +pub fn default_enforcement(config: &types::SpecSyncConfig) -> types::EnforcementMode { + if config.enforcement_set { + config.enforcement + } else { + types::EnforcementMode::Strict + } +} + /// Compute exit code without printing or exiting. pub fn compute_exit_code( total_errors: usize, From afd63b9247c7706e41b50f76d0ac881c8bb083cb Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:29:50 -0600 Subject: [PATCH 16/21] =?UTF-8?q?fix:=20parser=20=E2=80=94=20scalar=20depe?= =?UTF-8?q?nds=5Fon=20normalized,=20duplicates=20deduped,=20scaffold=20boi?= =?UTF-8?q?lerplate=20detection=20(batch=2016)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/parser.rs | 121 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 114 insertions(+), 7 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 8f15981d..9be953b3 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -113,10 +113,38 @@ fn set_scalar(fm: &mut Frontmatter, key: &str, value: &str) { // Handle inline bracket arrays like `implements: [42, 57]` "implements" => fm.implements = parse_inline_issue_numbers(value), "tracks" => fm.tracks = parse_inline_issue_numbers(value), + // A scalar `depends_on: alpha` (or inline `depends_on: [a, b]`) used to + // be silently DROPPED — the dependency edge vanished from validation + // and graphing with no diagnostic. Normalize it into the list so the + // edge is enforced, and warn so the typo is visible. + "depends_on" => { + fm.depends_on = parse_inline_string_list(value); + if !value.trim_start().starts_with('[') { + eprintln!( + "warning: scalar `depends_on: {value}` should be a YAML list (`depends_on: [{value}]`); treating it as a single dependency" + ); + } + } _ => {} } } +/// Parse an inline bracket array of strings (`[a, b]` → vec!["a", "b"]) or a +/// bare scalar (`a` → vec!["a"]). Quotes are stripped from each element. +fn parse_inline_string_list(value: &str) -> Vec { + let s = value.trim(); + let inner = if s.starts_with('[') && s.ends_with(']') { + &s[1..s.len() - 1] + } else { + s + }; + inner + .split(',') + .map(|v| v.trim().trim_matches('"').trim_matches('\'').to_string()) + .filter(|v| !v.is_empty()) + .collect() +} + /// Parse an inline bracket array of issue numbers: `[42, 57]` → vec![42, 57]. fn parse_inline_issue_numbers(value: &str) -> Vec { let s = value.trim(); @@ -143,7 +171,16 @@ fn set_field(fm: &mut Frontmatter, key: &str, values: &[String]) { match key { "files" => fm.files = values.to_vec(), "db_tables" => fm.db_tables = values.to_vec(), - "depends_on" => fm.depends_on = values.to_vec(), + // Dedupe at parse time (order-preserving): duplicate `depends_on` + // entries used to inflate edge counts and emit doubled mermaid edges. + "depends_on" => { + let mut seen = std::collections::HashSet::new(); + fm.depends_on = values + .iter() + .filter(|v| seen.insert((*v).clone())) + .cloned() + .collect(); + } "implements" => fm.implements = parse_issue_numbers(values), "tracks" => fm.tracks = parse_issue_numbers(values), "lifecycle_log" => fm.lifecycle_log = values.to_vec(), @@ -378,15 +415,46 @@ const STUB_PHRASES: &[&str] = &[ "\u{2026}", // ellipsis character ]; +/// Stock sentences emitted verbatim by `specsync new` / `generate` scaffolds. +/// They read like prose but are pure template — a spec consisting of them is +/// an untouched scaffold, not documentation (#441). +const SCAFFOLD_BOILERPLATE_PREFIXES: &[&str] = &[ + "document this module's responsibility", + "list runtime dependencies and the specific symbols", + "define an invariant that must remain true", + "**given** precondition", + "**when** action", + "**then** result", +]; + +/// Strip list markers (`- `, `* `, `> `, `1. `) from the start of a line. +fn strip_list_marker(mut s: &str) -> &str { + s = s.trim(); + for marker in ["- ", "* ", "> "] { + if let Some(rest) = s.strip_prefix(marker) { + s = rest.trim_start(); + } + } + let digits = s.len() - s.trim_start_matches(|c: char| c.is_ascii_digit()).len(); + if digits > 0 && s[digits..].starts_with(". ") { + s = s[digits + 2..].trim_start(); + } + s +} + +/// Check if a line is verbatim scaffold boilerplate (case-insensitive). +pub fn is_boilerplate_line(line: &str) -> bool { + let lower = strip_list_marker(line).to_ascii_lowercase(); + SCAFFOLD_BOILERPLATE_PREFIXES + .iter() + .any(|p| lower.starts_with(p)) +} + /// Check if a line is a stub/placeholder (case-insensitive). fn is_stub_line(line: &str) -> bool { - let t = line - .trim() - .trim_start_matches("- ") - .trim_start_matches("* ") - .trim_start_matches("> "); + let t = strip_list_marker(line); let lower = t.to_ascii_lowercase(); - STUB_PHRASES.contains(&lower.as_str()) + STUB_PHRASES.contains(&lower.as_str()) || is_boilerplate_line(line) } /// Find the byte offset of an exact `## Section` heading line. @@ -491,6 +559,45 @@ mod tests { assert!(parsed.frontmatter.db_tables.is_empty()); } + #[test] + fn test_scalar_depends_on_normalized_to_list() { + // Regression (#419): a scalar `depends_on: alpha` was silently dropped, + // disabling the dependency edge with no diagnostic. + let content = "---\nmodule: beta\nversion: 1\nstatus: active\nfiles: []\ndepends_on: alpha\n---\n\n# Beta\n"; + let parsed = parse_frontmatter(content).unwrap(); + assert_eq!(parsed.frontmatter.depends_on, vec!["alpha"]); + } + + #[test] + fn test_inline_bracket_depends_on_parsed() { + let content = "---\nmodule: beta\ndepends_on: [alpha, gamma]\n---\n\n# Beta\n"; + let parsed = parse_frontmatter(content).unwrap(); + assert_eq!(parsed.frontmatter.depends_on, vec!["alpha", "gamma"]); + } + + #[test] + fn test_duplicate_depends_on_deduped() { + // Regression (#419): duplicate entries inflated edge counts and doubled + // mermaid edges. + let content = "---\nmodule: beta\ndepends_on:\n - alpha\n - alpha\n - gamma\n - alpha\n---\n\n# Beta\n"; + let parsed = parse_frontmatter(content).unwrap(); + assert_eq!(parsed.frontmatter.depends_on, vec!["alpha", "gamma"]); + } + + #[test] + fn test_scaffold_boilerplate_detected() { + // Regression (#441): an untouched `specsync new` scaffold must not + // count as placeholder-free, meaningful content. + assert!(is_boilerplate_line( + "Document this module's responsibility, inputs, outputs, and ownership boundaries." + )); + assert!(is_boilerplate_line("- **Given** precondition")); + assert!(is_boilerplate_line( + "1. Define an invariant that must remain true for supported inputs." + )); + assert!(!is_boilerplate_line("Handles authentication tokens.")); + } + #[test] fn test_strip_yaml_comment() { assert_eq!(strip_yaml_comment("active"), "active"); From b82799c1e5d1be8f62c5226bb63167427a24d4c7 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:33:26 -0600 Subject: [PATCH 17/21] =?UTF-8?q?fix:=20check=20=E2=80=94=20default=20enfo?= =?UTF-8?q?rcement=20gates=20on=20errors,=20no-spec/warm-cache=20gate=20pa?= =?UTF-8?q?ths=20(batch=2017)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/check.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/commands/check.rs b/src/commands/check.rs index d089c5f6..a1c88e19 100644 --- a/src/commands/check.rs +++ b/src/commands/check.rs @@ -16,7 +16,8 @@ use crate::validator::{compute_coverage, get_schema_table_names}; use crate::config::is_legacy_layout; use super::{ - build_schema_columns, compute_exit_code, create_drift_issues, exit_with_status, + build_schema_columns, compute_exit_code, create_drift_issues, default_enforcement, + exit_with_status, filter_by_status, filter_specs, load_and_discover, run_validation, }; @@ -136,7 +137,7 @@ pub fn cmd_check( let enforcement = enforcement.unwrap_or(if strict { types::EnforcementMode::Strict } else { - config.enforcement + default_enforcement(&config) }); // Spec name filters that matched nothing are an error — don't fall through From a21cd7a9a2f2776529a75c37c68bd2de74986e70 Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:41:29 -0600 Subject: [PATCH 18/21] =?UTF-8?q?fix:=20changelog=20=E2=80=94=20three-dot?= =?UTF-8?q?=20split-first,=20rev-parse=20ref=20validation,=20real=20merge-?= =?UTF-8?q?base=20(closes=20#418)=20(batch=2018)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/changelog.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/changelog.rs b/src/changelog.rs index 3ef2383a..6b590bbb 100644 --- a/src/changelog.rs +++ b/src/changelog.rs @@ -83,12 +83,60 @@ fn read_file_at_ref(root: &Path, git_ref: &str, file_path: &str) -> Option Option<(String, String)> { - let parts: Vec<&str> = range.splitn(2, "..").collect(); - if parts.len() == 2 && !parts[0].is_empty() && !parts[1].is_empty() { - Some((parts[0].to_string(), parts[1].to_string())) + parse_range_full(range).map(|(from, to, _)| (from, to)) +} + +/// Parse a range into (from, to, three_dot). A three-dot range (`A...B`, +/// changes since the merge-base) must be split on `...` FIRST — splitting on +/// `..` mangles it into `A` / `.B`, and the bogus `.B` ref then silently +/// produced fabricated "removed" entries (#418). +pub fn parse_range_full(range: &str) -> Option<(String, String, bool)> { + if let Some((from, to)) = range.split_once("...") { + if !from.is_empty() && !to.is_empty() { + return Some((from.to_string(), to.to_string(), true)); + } + return None; + } + let (from, to) = range.split_once("..")?; + if from.is_empty() || to.is_empty() { + return None; + } + Some((from.to_string(), to.to_string(), false)) +} + +/// Verify that a git ref resolves to a real commit (`git rev-parse --verify`). +/// Returns the resolved full hash. +pub fn resolve_ref(root: &Path, git_ref: &str) -> Result { + let output = Command::new("git") + .args([ + "rev-parse", + "--verify", + "--end-of-options", + &format!("{git_ref}^{{commit}}"), + ]) + .current_dir(root) + .output() + .map_err(|e| format!("failed to run git rev-parse: {e}"))?; + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) } else { - None + Err(format!("unknown revision '{git_ref}'")) + } +} + +/// Compute the merge-base of two refs (`git merge-base`). +pub fn merge_base(root: &Path, a: &str, b: &str) -> Result { + let output = Command::new("git") + .args(["merge-base", "--end-of-options", a, b]) + .current_dir(root) + .output() + .map_err(|e| format!("failed to run git merge-base: {e}"))?; + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) + } else { + Err(format!("no merge-base between '{a}' and '{b}'")) } } From 628bf3f9334c59f63fa5431527d6ef03480e69bc Mon Sep 17 00:00:00 2001 From: Leif Date: Fri, 24 Jul 2026 12:46:09 -0600 Subject: [PATCH 19/21] =?UTF-8?q?fix:=20scoring=20=E2=80=94=20value-valida?= =?UTF-8?q?ted=20frontmatter,=20consistent=20sections=20arithmetic,=20scaf?= =?UTF-8?q?fold=20boilerplate,=20non-vacuous=20files=5Fexist=20(closes=20#?= =?UTF-8?q?441)=20(batch=2019)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/scoring.rs | 174 +++++++++++++++++++++++++++++++------------------ 1 file changed, 111 insertions(+), 63 deletions(-) diff --git a/src/scoring.rs b/src/scoring.rs index 573f8ce9..d499e41a 100644 --- a/src/scoring.rs +++ b/src/scoring.rs @@ -120,27 +120,40 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe let body = &parsed.body; // ─── Frontmatter (0-20) ────────────────────────────────────────── + // Presence alone is not enough (#441): `version: "notanumber"` and + // `status: bogus` used to score full marks. Validate the VALUES. + let version_valid = fm.version.as_deref().is_some_and(|v| { + let v = v.trim().trim_matches('"').trim_matches('\''); + !v.is_empty() + && v.split('.') + .all(|part| !part.is_empty() && part.chars().all(|c| c.is_ascii_digit())) + }); + let status_valid = fm.parsed_status().is_some(); let mut fm_points = 0u32; - let mut fm_missing: Vec<&str> = Vec::new(); + let mut fm_missing: Vec = Vec::new(); if fm.module.is_some() { fm_points += FM_MODULE_POINTS; } else { - fm_missing.push("module (-5pts)"); + fm_missing.push("module (-5pts)".to_string()); } - if fm.version.is_some() { + if version_valid { fm_points += FM_VERSION_POINTS; + } else if fm.version.is_some() { + fm_missing.push("valid version (-5pts)".to_string()); } else { - fm_missing.push("version (-5pts)"); + fm_missing.push("version (-5pts)".to_string()); } - if fm.status.is_some() { + if status_valid { fm_points += FM_STATUS_POINTS; + } else if fm.status.is_some() { + fm_missing.push("valid status (-4pts)".to_string()); } else { - fm_missing.push("status (-4pts)"); + fm_missing.push("status (-4pts)".to_string()); } if !fm.files.is_empty() { fm_points += FM_FILES_POINTS; } else { - fm_missing.push("files (-6pts)"); + fm_missing.push("files (-6pts)".to_string()); } score.frontmatter_score = fm_points; if !fm_missing.is_empty() { @@ -172,30 +185,26 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe }, CriterionResult { name: "has_version".to_string(), - passed: fm.version.is_some(), - points: if fm.version.is_some() { - FM_VERSION_POINTS - } else { - 0 - }, + passed: version_valid, + points: if version_valid { FM_VERSION_POINTS } else { 0 }, max_points: FM_VERSION_POINTS, detail: if fm.version.is_none() { Some("add `version:` field".to_string()) + } else if !version_valid { + Some("`version` must be numeric (e.g. `1` or `1.2`)".to_string()) } else { None }, }, CriterionResult { name: "has_status".to_string(), - passed: fm.status.is_some(), - points: if fm.status.is_some() { - FM_STATUS_POINTS - } else { - 0 - }, + passed: status_valid, + points: if status_valid { FM_STATUS_POINTS } else { 0 }, max_points: FM_STATUS_POINTS, detail: if fm.status.is_none() { Some("add `status:` field".to_string()) + } else if !status_valid { + Some("`status` is not a recognized lifecycle status".to_string()) } else { None }, @@ -220,47 +229,34 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe // ─── Sections (0-20) ───────────────────────────────────────────── let missing = get_missing_sections(body, &config.required_sections); - let present = config.required_sections.len() - missing.len(); let total_sections = config.required_sections.len(); - score.sections_score = if total_sections == 0 { - DIMENSION_MAX - } else { - ((present as f64 / total_sections as f64) * DIMENSION_MAX as f64).round() as u32 - }; - if !missing.is_empty() { - let lost = DIMENSION_MAX - score.sections_score; - let names = missing - .iter() - .take(3) - .cloned() - .collect::>() - .join(", "); - let suffix = if missing.len() > 3 { - format!(" (+{} more)", missing.len() - 3) - } else { - String::new() - }; - score - .suggestions - .push(format!("Sections (-{lost}pts): missing ## {names}{suffix}")); - } { let missing_set: HashSet<&str> = missing.iter().map(|s| s.as_str()).collect(); - let per_section_max = if total_sections > 0 { - ((DIMENSION_MAX as f64 / total_sections as f64).round() as u32).max(1) + // Distribute DIMENSION_MAX points across the sections EXACTLY (first + // `remainder` sections get one extra point). The previous round() gave + // every section ceil(20/n) points, so the criteria summed to e.g. 21 + // while the dimension reported /20 — the --explain output contradicted + // itself (#441). The dimension score is now literally the criteria sum. + let (base, remainder) = if total_sections > 0 { + ( + DIMENSION_MAX / total_sections as u32, + DIMENSION_MAX % total_sections as u32, + ) } else { - 0 + (0, 0) }; let section_criteria: Vec = config .required_sections .iter() - .map(|sec| { + .enumerate() + .map(|(i, sec)| { let present = !missing_set.contains(sec.as_str()); + let max = base + u32::from((i as u32) < remainder); CriterionResult { name: sec.clone(), passed: present, - points: if present { per_section_max } else { 0 }, - max_points: per_section_max, + points: if present { max } else { 0 }, + max_points: max, detail: if !present { Some(format!("add ## {sec} section")) } else { @@ -269,6 +265,11 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe } }) .collect(); + score.sections_score = if total_sections == 0 { + DIMENSION_MAX + } else { + section_criteria.iter().map(|c| c.points).sum() + }; score.explain.push(ExplainDetail { dimension: "Sections".to_string(), score: score.sections_score, @@ -276,6 +277,23 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe criteria: section_criteria, }); } + if !missing.is_empty() { + let lost = DIMENSION_MAX - score.sections_score; + let names = missing + .iter() + .take(3) + .cloned() + .collect::>() + .join(", "); + let suffix = if missing.len() > 3 { + format!(" (+{} more)", missing.len() - 3) + } else { + String::new() + }; + score + .suggestions + .push(format!("Sections (-{lost}pts): missing ## {names}{suffix}")); + } // ─── API Coverage (0-20) ───────────────────────────────────────── if !fm.files.is_empty() { @@ -410,7 +428,11 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe // ─── Content Depth (0-20) ──────────────────────────────────────── let mut depth_points = 0u32; let todo_count = count_placeholder_todos(body); - let placeholder_count = count_placeholder_comments(body); + // Untouched `specsync new`/`generate` scaffolds contain no TODOs or + // `` markers, yet are pure boilerplate — count the scaffold's + // stock sentences as placeholders too, or an empty scaffold passes the + // "placeholder_free" check and clears the ≥80 bar (#441). + let placeholder_count = count_placeholder_comments(body) + count_boilerplate_lines(body); // Check each required section has meaningful content (stubs don't count) let sections_with_content = count_sections_with_content(body, &config.required_sections); @@ -536,7 +558,16 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe stale_files += 1; } } - let file_penalty = if stale_files > 0 { + let file_penalty = if fm.files.is_empty() { + // `files_exist` was vacuously 15/15 when `files:` was missing entirely + // — nothing to check passed as everything checked out (#441). An empty + // files list makes freshness unverifiable, not perfect. + fresh_points = fresh_points.saturating_sub(FRESH_FILES_MAX); + score.suggestions.push(format!( + "Freshness (-{FRESH_FILES_MAX}pts): no files listed in frontmatter — freshness is unverifiable" + )); + FRESH_FILES_MAX + } else if stale_files > 0 { let penalty = (stale_files * FRESH_FILE_PENALTY_PER).min(FRESH_FILES_MAX); fresh_points = fresh_points.saturating_sub(penalty); score.suggestions.push(format!( @@ -600,6 +631,19 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe } score.freshness_score = fresh_points; + // Budget the dimension so the --explain criteria sum EXACTLY to the + // reported score (previously criteria summed to 15+5+variable ≠ 20 — the + // raw max was silently rescaled, contradicting the dimension line, #441). + // fresh_points = 20 - file_penalty - dep_penalty - git_penalty, so: + // files_exist: max 15-dep_budget, points max-file_penalty + // deps_exist: max dep_budget, points budget-dep_penalty + // git: max 5, points 5-git_penalty + let dep_budget = if fm.depends_on.is_empty() { + 0 + } else { + dep_penalty.max(FRESH_DEP_PENALTY_PER) + }; + let files_budget = FRESH_FILES_MAX - dep_budget; score.explain.push(ExplainDetail { dimension: "Freshness".to_string(), score: fresh_points, @@ -607,10 +651,12 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe criteria: vec![ CriterionResult { name: "files_exist".to_string(), - passed: stale_files == 0, - points: FRESH_FILES_MAX.saturating_sub(file_penalty), - max_points: FRESH_FILES_MAX, - detail: if stale_files > 0 { + passed: !fm.files.is_empty() && stale_files == 0, + points: files_budget.saturating_sub(file_penalty), + max_points: files_budget, + detail: if fm.files.is_empty() { + Some("no files listed in frontmatter".to_string()) + } else if stale_files > 0 { Some(format!("{stale_files} file(s) missing")) } else { None @@ -619,15 +665,8 @@ pub fn score_spec(spec_path: &Path, root: &Path, config: &SpecSyncConfig) -> Spe CriterionResult { name: "deps_exist".to_string(), passed: stale_deps == 0, - points: (stale_deps * FRESH_DEP_PENALTY_PER) - .saturating_sub(dep_penalty) - .min(if fm.depends_on.is_empty() { - 0 - } else { - stale_deps * FRESH_DEP_PENALTY_PER - }), - max_points: (fm.depends_on.len() as u32 * FRESH_DEP_PENALTY_PER) - .min(FRESH_DEP_PENALTY_PER * 2), + points: dep_budget.saturating_sub(dep_penalty), + max_points: dep_budget, detail: if stale_deps > 0 { Some(format!("{stale_deps} depends_on path(s) missing")) } else { @@ -724,6 +763,15 @@ fn count_placeholder_comments(body: &str) -> usize { stripped.matches("