diff --git a/cli/src/commands/charter/audit.rs b/cli/src/commands/charter/audit.rs index ef66fe1..1aaa9d6 100644 --- a/cli/src/commands/charter/audit.rs +++ b/cli/src/commands/charter/audit.rs @@ -33,7 +33,46 @@ use crate::audit_schema::AuditOutputSchema; use crate::charter::{self, Charter}; use crate::utils; -const DEFAULT_RANGE: &str = "HEAD~1..HEAD"; +/// Last-resort fallback when no upstream branch is reachable. Issued with a +/// warning that explains why the operator may want `--range` explicitly. +const FALLBACK_RANGE: &str = "HEAD~1..HEAD"; + +/// Resolve the default git range when `--range` is not provided. +/// +/// Tries upstream branches in priority order (`origin/main` → `origin/master`) +/// to bound the audit at the point where the feature branch diverged from the +/// project's main line. Falls back to `HEAD~1..HEAD` (the v0 default) when no +/// upstream is reachable, with a warning to stderr — that path covers +/// freshly-cloned repos without remotes, disconnected branches, or repos +/// where the operator hasn't run `git fetch` yet. +/// +/// Issue #102 R11(A): Sentinel CHARTER-07 was implemented as 8 commits on a +/// feature branch; the previous default `HEAD~1..HEAD` only sent the last +/// (metadata-only) commit to the auditors, which converged on "0 substantive +/// findings" vacuously because they never saw the migrations, SQLC, scaffolding, +/// or PII guard test. `origin/main..HEAD` captures the full implementation set. +fn resolve_default_range(project_root: &Path) -> String { + for candidate in ["origin/main", "origin/master"] { + let probe = std::process::Command::new("git") + .args(["rev-parse", "--verify", "--quiet", candidate]) + .current_dir(project_root) + .output(); + if let Ok(out) = probe { + if out.status.success() { + return format!("{candidate}..HEAD"); + } + } + } + eprintln!( + "{} no upstream branch reachable (tried origin/main, origin/master); \ + falling back to {}. For multi-commit feature branches, pass \ + --range explicitly so the auditors see the full \ + implementation set, not just the last commit.", + "warn:".yellow().bold(), + FALLBACK_RANGE + ); + FALLBACK_RANGE.to_string() +} pub fn run( path: &str, @@ -70,7 +109,10 @@ pub fn run( let prompts_dir = audit_dir.join("prompts"); utils::ensure_dir(&prompts_dir)?; - let range = range.unwrap_or(DEFAULT_RANGE).to_string(); + let range = match range { + Some(r) => r.to_string(), + None => resolve_default_range(project_root), + }; if finalize { return run_finalize( diff --git a/cli/tests/charter_audit_test.rs b/cli/tests/charter_audit_test.rs index efdda61..efe091d 100644 --- a/cli/tests/charter_audit_test.rs +++ b/cli/tests/charter_audit_test.rs @@ -675,3 +675,148 @@ fn audit_merge_into_requires_finalize() { .assert() .failure(); } + +// ── R11(A) regression tests (issue #102) ─────────────────────────────────── +// +// Sentinel CHARTER-07 was implemented as 8 commits on a feature branch; the +// previous default `HEAD~1..HEAD` only sent the last (metadata-only) commit +// to the auditors, who converged on "0 substantive findings" vacuously. The +// fix prefers `origin/main..HEAD` (or `origin/master..HEAD` on legacy repos) +// when an upstream is reachable, falling back to `HEAD~1..HEAD` otherwise. +// The resolved Git range appears in the prompt body via `Git range: `, +// which is what these tests assert against. + +/// Set up a working tree where `origin/main` is reachable: a bare repo as +/// the remote (in its own tempdir to avoid collisions when tests run in +/// parallel), an initial commit on `main`, push to remote, then a feature +/// branch with two additional commits. The current branch is the feature +/// branch when this returns. The returned `TempDir` MUST be kept alive by +/// the caller for the duration of the test — dropping it removes the bare +/// remote and breaks subsequent git operations on the working tree. +fn init_repo_with_remote_main(dir: &Path) -> TempDir { + let remote = TempDir::new().unwrap(); + let status = StdCommand::new("git") + .args(["init", "--bare", "-q", "-b", "main"]) + .current_dir(remote.path()) + .status() + .expect("git init --bare failed"); + assert!(status.success()); + + std::fs::create_dir_all(dir.join("src")).unwrap(); + std::fs::write(dir.join("src/foo.rs"), "// initial\n").unwrap(); + git(dir, &["init", "-q", "-b", "main"]); + git(dir, &["add", "."]); + git(dir, &["commit", "-q", "-m", "initial on main"]); + git(dir, &["remote", "add", "origin", remote.path().to_str().unwrap()]); + git(dir, &["push", "-q", "origin", "main"]); + + // Feature branch with multiple commits — this is what `origin/main..HEAD` + // is supposed to capture in full. + git(dir, &["checkout", "-q", "-b", "feature/multi"]); + std::fs::write(dir.join("src/foo.rs"), "// edited 1\n").unwrap(); + git(dir, &["add", "."]); + git(dir, &["commit", "-q", "-m", "feature: first"]); + std::fs::write(dir.join("src/foo.rs"), "// edited 2\n").unwrap(); + git(dir, &["add", "."]); + git(dir, &["commit", "-q", "-m", "feature: second"]); + + remote +} + +#[test] +fn audit_default_range_uses_origin_main_when_available() { + if !bash_available() { + return; + } + let dir = TempDir::new().unwrap(); + setup_devtrail(dir.path()); + write_charter(dir.path()); + let _remote = init_repo_with_remote_main(dir.path()); + + Command::cargo_bin("devtrail") + .unwrap() + .args(["charter", "audit", "CHARTER-01", "--path"]) + .arg(dir.path().to_str().unwrap()) + .assert() + .success(); + + let prompt = std::fs::read_to_string( + dir.path() + .join("audit/charters/CHARTER-01/prompts/auditor-primary.prompt.md"), + ) + .unwrap(); + assert!( + prompt.contains("origin/main..HEAD"), + "default range must resolve to origin/main..HEAD when remote is reachable; \ + prompt did not contain that string. Excerpt:\n{}", + prompt.lines().take(80).collect::>().join("\n") + ); + assert!( + !prompt.contains("Git range: `HEAD~1..HEAD`"), + "default range must NOT fall back to HEAD~1..HEAD when origin/main exists" + ); +} + +#[test] +fn audit_default_range_falls_back_to_head_minus_one_without_remote() { + if !bash_available() { + return; + } + let dir = TempDir::new().unwrap(); + setup_devtrail(dir.path()); + write_charter(dir.path()); + init_repo_with_diff(dir.path()); // no remote configured + + Command::cargo_bin("devtrail") + .unwrap() + .args(["charter", "audit", "CHARTER-01", "--path"]) + .arg(dir.path().to_str().unwrap()) + .assert() + .success() + .stderr(predicate::str::contains("no upstream branch reachable")) + .stderr(predicate::str::contains("HEAD~1..HEAD")) + .stderr(predicate::str::contains("--range")); + + let prompt = std::fs::read_to_string( + dir.path() + .join("audit/charters/CHARTER-01/prompts/auditor-primary.prompt.md"), + ) + .unwrap(); + assert!( + prompt.contains("HEAD~1..HEAD"), + "fallback range must be HEAD~1..HEAD when no upstream is reachable" + ); + assert!( + !prompt.contains("origin/main..HEAD"), + "no origin/main exists, fallback must not claim it does" + ); +} + +#[test] +fn audit_explicit_range_overrides_default_resolution() { + // Backwards-compat sanity: --range still wins over the new defaulting + // logic (no upstream probe attempted). + if !bash_available() { + return; + } + let dir = TempDir::new().unwrap(); + setup_devtrail(dir.path()); + write_charter(dir.path()); + init_repo_with_diff(dir.path()); + + Command::cargo_bin("devtrail") + .unwrap() + .args([ + "charter", + "audit", + "CHARTER-01", + "--range", + "HEAD~1..HEAD", + "--path", + ]) + .arg(dir.path().to_str().unwrap()) + .assert() + .success() + // No fallback warning when --range was passed explicitly. + .stderr(predicate::str::contains("no upstream branch reachable").not()); +}