fix(stella-pipeline): let the verifier read an untracked file's content, not just its name

crates/stella-cli/src/agent.rs

@@ -47,6 +47,7 @@ use crate::{OutputFormat, config::Config, resume_frame};
 use stella_context::EpisodeOutcome;
 
 mod coverage;
+mod diagnostics;
 mod engine;
 mod goal;
 mod graph;

crates/stella-cli/src/agent/diagnostics.rs

@@ -0,0 +1,50 @@
+//! The closed Git diagnostic vocabulary, as argv.
+//!
+//! Split out of [`super::tools`], which sits against the file-size limit, and
+//! coherent on its own terms: this is the one place a
+//! [`DiagnosticInvocation`] becomes a process. The runner's state and
+//! construction stay beside the rest of the workspace ports in `tools`; only
+//! the mapping lives here.
+//!
+//! Every variant maps to fixed argv. Paths are literal arguments and no shell
+//! is involved, which is the property that lets the pipeline hand this
+//! model-supplied path without sanitizing it.
+
+use async_trait::async_trait;
+use stella_pipeline::{CmdOutcome, DiagnosticInvocation, DiagnosticRunner};
+
+use super::tools::{GitDiagnosticRunner, run_command, scrub_model_subprocess};
+
+#[async_trait]
+impl DiagnosticRunner for GitDiagnosticRunner {
+    async fn run_diagnostic(&self, invocation: &DiagnosticInvocation) -> CmdOutcome {
+        let mut cmd = tokio::process::Command::new("git");
+        scrub_model_subprocess(&mut cmd);
+        match invocation {
+            DiagnosticInvocation::GitDiff => match self.baseline_commit() {
+                Some(baseline) => {
+                    cmd.args(["diff", baseline]);
+                }
+                None => {
+                    cmd.args(["diff"]);
+                }
+            },
+            DiagnosticInvocation::UntrackedNumstat { path } => {
+                cmd.args(["diff", "--no-index", "--numstat", "--", "/dev/null", path]);
+            }
+            DiagnosticInvocation::UntrackedPatch { path } => {
+                // The same probe as the numstat above, minus `--numstat`, so
+                // the two answers about one untracked file can never disagree
+                // about which file they read. `--no-color` because a
+                // configured `color.ui = always` would otherwise paint SGR
+                // escapes into a verifier's prompt.
+                cmd.args(["diff", "--no-index", "--no-color", "--", "/dev/null", path]);
+            }
+        }
+        cmd.current_dir(&self.root).env("PWD", &self.root);
+        for var in stella_tools::exec::GIT_REPO_ENV_VARS {
+            cmd.env_remove(var);
+        }
+        run_command(cmd).await
+    }
+}

crates/stella-cli/src/agent/tools.rs

@@ -15,7 +15,7 @@ use stella_pipeline::{
 /// Apply the cross-crate policy shared by every model/repository-controlled
 /// subprocess. Kept as a named seam so the CLI's pipeline-only spawns have a
 /// direct regression test rather than relying only on stella-tools tests.
-fn scrub_model_subprocess(command: &mut tokio::process::Command) {
+pub(super) fn scrub_model_subprocess(command: &mut tokio::process::Command) {
     stella_tools::subprocess_env::scrub_sensitive_env(command);
 }
 
@@ -596,7 +596,7 @@ impl GitDiagnosticRunner {
         Self { root, baseline }
     }
 
-    fn baseline_commit(&self) -> Option<&str> {
+    pub(super) fn baseline_commit(&self) -> Option<&str> {
         self.baseline.as_deref()
     }
 }
@@ -656,33 +656,7 @@ fn test_process(invocation: &TestInvocation, root: &std::path::Path) -> tokio::p
     cmd
 }
 
-#[async_trait::async_trait]
-impl DiagnosticRunner for GitDiagnosticRunner {
-    async fn run_diagnostic(&self, invocation: &DiagnosticInvocation) -> CmdOutcome {
-        let mut cmd = tokio::process::Command::new("git");
-        scrub_model_subprocess(&mut cmd);
-        match invocation {
-            DiagnosticInvocation::GitDiff => match self.baseline_commit() {
-                Some(baseline) => {
-                    cmd.args(["diff", baseline]);
-                }
-                None => {
-                    cmd.args(["diff"]);
-                }
-            },
-            DiagnosticInvocation::UntrackedNumstat { path } => {
-                cmd.args(["diff", "--no-index", "--numstat", "--", "/dev/null", path]);
-            }
-        }
-        cmd.current_dir(&self.root).env("PWD", &self.root);
-        for var in stella_tools::exec::GIT_REPO_ENV_VARS {
-            cmd.env_remove(var);
-        }
-        run_command(cmd).await
-    }
-}
-
-async fn run_command(mut cmd: tokio::process::Command) -> CmdOutcome {
+pub(super) async fn run_command(mut cmd: tokio::process::Command) -> CmdOutcome {
     cmd.stdout(std::process::Stdio::piped());
     cmd.stderr(std::process::Stdio::piped());
     // Cancellation drops this future without unwinding into the timeout arm

crates/stella-pipeline/src/pipeline/tests/verification_honesty.rs

@@ -40,3 +40,120 @@ fn a_real_diff_passes_through_unchanged() {
     assert_eq!(verification_honest_diff(diff.clone(), 0), diff);
     assert_eq!(verification_honest_diff(diff.clone(), 5), diff);
 }
+
+use super::*;
+
+/// The regex the fixture task writes. Distinctive so an assertion that finds
+/// it in a prompt cannot be finding something else.
+const REGEX: &str = r"^\d{4}-\d{2}-\d{2}$";
+
+/// A diagnostics double for the untracked-only shape: `git diff` sees nothing
+/// (the file was never staged), the numstat probe counts one added line, and
+/// the patch probe serves the file under the header preamble real git emits.
+struct PatchRunner;
+
+#[async_trait]
+impl DiagnosticRunner for PatchRunner {
+    async fn run_diagnostic(&self, invocation: &DiagnosticInvocation) -> CmdOutcome {
+        let stdout = match invocation {
+            DiagnosticInvocation::GitDiff => String::new(),
+            DiagnosticInvocation::UntrackedNumstat { .. } => "1\t0\tregex.txt".to_string(),
+            DiagnosticInvocation::UntrackedPatch { .. } => format!(
+                "diff --git a/dev/null b/regex.txt\nnew file mode 100644\n\
+                 index 0000000..1111111\n--- /dev/null\n+++ b/regex.txt\n\
+                 @@ -0,0 +1 @@\n+{REGEX}\n"
+            ),
+        };
+        CmdOutcome {
+            exit_code: 0,
+            stdout_tail: stdout,
+            stderr_tail: String::new(),
+            kind: CmdKind::Completed,
+        }
+    }
+}
+
+/// The witness: a task whose entire deliverable is an untracked file must
+/// reach the verifier as its CONTENT, not as its name.
+///
+/// Before the `UntrackedPatch` probe, `gather_diff` described such a file with
+/// one marker line — the path and an added-line count — because `git diff`
+/// cannot see an unstaged file and nothing else was asked about it. A real run
+/// of exactly this shape returned PASS reasoning that "the unseen content
+/// cannot itself justify a FAIL": the model was grading a filename.
+///
+/// Asserting on the verifier's own prompt is the point. An assertion on the
+/// probe's return value would still pass if the text were dropped anywhere
+/// between the probe and the model.
+#[tokio::test]
+async fn the_verifier_reads_an_untracked_files_content_not_just_its_name() {
+    let provider = ScriptedProvider::new(vec![
+        text_result("CLASS: single\nWITNESS: no\nVERIFIER: yes"),
+        text_result("Wrote the regex to regex.txt."),
+        text_result("PASS the regex handles the stated cases"),
+    ]);
+    let resolver = OneProvider(&provider);
+    let runner = PatchRunner;
+    let tests = ScriptedRunner::new(vec![], "");
+    let tools = EmptyTools;
+    let recall = NoContextRecall;
+    let repo = NoRepoStructure;
+    // Empty before the turn, carrying the new file after it — the fingerprint
+    // delta that makes `gather_diff` bill the file to this turn.
+    let repo_status = SeqRepoStatus::new(vec![vec![], vec![("regex.txt", "sha256:a")]]);
+    let approvals = AutoApproveGate;
+    let sleeper = NoopSleeper;
+    let router = router();
+    let (tx, _rx) = mpsc::unbounded_channel();
+
+    let pipeline = Pipeline::new(
+        PipelinePorts {
+            router: &router,
+            providers: &resolver,
+            tools: &tools,
+            recall: &recall,
+            repo: &repo,
+            repo_status: &repo_status,
+            touches: &NoFileTouches,
+            diagnostics: &runner,
+            tests: &tests,
+            lint: None,
+            mutation: None,
+            coverage: None,
+            approvals: &approvals,
+            sleeper: &sleeper,
+            hooks: None,
+            candidate_workspaces: None,
+            mcp_prefetch: None,
+            steering: None,
+        },
+        tx,
+        PipelineConfig {
+            test_command: None,
+            diff_diagnostic: Some(DiagnosticInvocation::GitDiff),
+            witness_writer: false,
+            ..PipelineConfig::default()
+        },
+    );
+
+    let mut messages = vec![CompletionMessage::system("sys")];
+    let mut budget = BudgetGuard::new(BudgetMode::Off, None, None);
+    pipeline
+        .run("Write a regex to regex.txt", &mut messages, &mut budget)
+        .await
+        .expect("run completes");
+
+    let verifier_prompt = provider
+        .prompts()
+        .into_iter()
+        .find(|p| p.contains("independent code reviewer"))
+        .expect("the verifier was asked");
+    assert!(
+        verifier_prompt.contains(REGEX),
+        "the verifier graded the change without ever seeing it: {verifier_prompt}"
+    );
+    assert!(
+        verifier_prompt.contains("untracked change: regex.txt"),
+        "the marker still names the file the content belongs to: {verifier_prompt}"
+    );
+}

crates/stella-pipeline/src/pipeline/verify_probes.rs

@@ -238,6 +238,33 @@ impl<'a> Pipeline<'a> {
             .unwrap_or(1)
     }
 
+    /// The reviewable body of one untracked file's patch — its hunks, with
+    /// git's file headers removed.
+    ///
+    /// The headers are dropped rather than passed through because
+    /// [`crate::witness::warrant::changed_paths`] reads `+++ `/`--- ` lines to
+    /// decide which paths a change touched, and an untracked path already
+    /// reaches it through the marker line this body sits under. Letting both
+    /// channels name the file would move untracked-only changes off the
+    /// `paths.is_empty()` branch that keeps them
+    /// [`WitnessWarrant::Required`](crate::witness::warrant::WitnessWarrant),
+    /// turning "the verifier can now read the file" into a silent relaxation
+    /// of when a witness is owed. One change, one effect.
+    ///
+    /// A binary file keeps git's `Binary files … differ` sentence instead of
+    /// its bytes: that sentence is the useful evidence (there is nothing here
+    /// a reviewer can read), and it is also what stops a database sidecar
+    /// from pouring escape bytes into a prompt.
+    async fn untracked_patch_body(&self, surface: CandidateSurface<'_>, path: &str) -> String {
+        let out = surface
+            .diagnostics
+            .run_diagnostic(&DiagnosticInvocation::UntrackedPatch {
+                path: path.to_string(),
+            })
+            .await;
+        patch_body(&out.stdout_tail)
+    }
+
     /// Run the diff command and return `(changed_line_count, raw_diff)`.
     ///
     /// `git diff` cannot see untracked files, so a turn whose entire change is
@@ -316,21 +343,35 @@ impl<'a> Pipeline<'a> {
             // the tail would let a large untracked change slip under a budget
             // it should have tripped. `buffered` preserves input order, so
             // the appended evidence stays deterministic.
-            let counted: Vec<(&str, u32)> =
+            let counted: Vec<(&str, u32, String)> =
                 futures_util::stream::iter(fresh.into_iter().map(|path| async move {
-                    (path, self.untracked_added_lines(surface, path).await)
+                    let (added, body) = futures_util::join!(
+                        self.untracked_added_lines(surface, path),
+                        self.untracked_patch_body(surface, path),
+                    );
+                    (path, added, body)
                 }))
                 .buffered(UNTRACKED_NUMSTAT_CONCURRENCY)
                 .collect()
                 .await;
-            for (path, added) in counted {
+            for (path, added, body) in counted {
                 lines += added;
                 // The shared builder, so `witness::warrant` can parse these
                 // paths back out and hold them to the same path rules as
                 // tracked changes — a hand-rolled format here silently
                 // blinded the warrant to every untracked file.
                 text.push('\n');
                 text.push_str(&crate::witness::warrant::untracked_change_line(path, added));
+                // …and the content underneath it, so a verifier grades the
+                // change rather than the filename (#2027). The marker alone
+                // is a line count: a run whose entire deliverable was one
+                // untracked file was graded PASS by a verifier that said so
+                // in as many words — "the unseen content cannot itself
+                // justify a FAIL".
+                if !body.is_empty() {
+                    text.push('\n');
+                    text.push_str(&body);
+                }
             }
         }
         DiffProbe {
@@ -481,3 +522,88 @@ pub(super) struct DiffProbe {
 /// costs roughly one round-trip instead of N, low enough that a turn which
 /// creates hundreds cannot fork an unbounded burst of git processes.
 const UNTRACKED_NUMSTAT_CONCURRENCY: usize = 16;
+
+/// Reduce one `git diff --no-index -- /dev/null <path>` patch to the part a
+/// reviewer can read: the hunks, or git's binary sentence.
+///
+/// Everything above the first `@@` is git's file-header preamble
+/// (`diff --git`, `new file mode`, `index`, `--- `, `+++ `), which
+/// [`Pipeline::untracked_patch_body`] explains must not survive into the
+/// diff text. An output with neither a hunk nor a binary sentence — an empty
+/// probe, a failed one — contributes nothing rather than a confusing
+/// fragment; the marker line above it still states the file changed.
+fn patch_body(raw: &str) -> String {
+    if let Some(start) = raw.find("\n@@ ") {
+        return raw[start + 1..].trim_end().to_string();
+    }
+    if raw.starts_with("@@ ") {
+        return raw.trim_end().to_string();
+    }
+    raw.lines()
+        .find(|line| line.starts_with("Binary files ") && line.ends_with(" differ"))
+        .unwrap_or_default()
+        .to_string()
+}
+
+#[cfg(test)]
+mod patch_body_tests {
+    use super::patch_body;
+
+    /// Exactly what `git diff --no-index -- /dev/null <path>` prints for a
+    /// newly created text file.
+    const CREATED: &str = "diff --git a/dev/null b/regex.txt\n\
+                           new file mode 100644\n\
+                           index 0000000..9f2c1d3\n\
+                           --- /dev/null\n\
+                           +++ b/regex.txt\n\
+                           @@ -0,0 +1,2 @@\n\
+                           +^\\d{4}-\\d{2}-\\d{2}$\n\
+                           +second line\n";
+
+    #[test]
+    fn the_content_survives() {
+        let body = patch_body(CREATED);
+        assert!(body.starts_with("@@ -0,0 +1,2 @@"), "{body}");
+        assert!(body.contains("+^\\d{4}-\\d{2}-\\d{2}$"), "{body}");
+        assert!(body.contains("+second line"), "{body}");
+    }
+
+    /// The restriction that keeps this change from also relaxing the warrant:
+    /// no `+++ `/`--- ` line may survive, because
+    /// [`crate::witness::warrant::changed_paths`] reads those as the set of
+    /// paths a change touched, and the marker line already names this one.
+    #[test]
+    fn no_header_survives_for_changed_paths_to_read() {
+        let body = patch_body(CREATED);
+        assert!(
+            crate::witness::warrant::changed_paths(&body).is_empty(),
+            "the body named a path to the warrant: {body}"
+        );
+        for line in body.lines() {
+            assert!(!line.starts_with("+++ "), "{line}");
+            assert!(!line.starts_with("--- "), "{line}");
+            assert!(!line.starts_with("diff --git"), "{line}");
+        }
+    }
+
+    /// A database sidecar is the case that must never spill bytes into a
+    /// prompt — git says so itself, and that sentence is the evidence.
+    #[test]
+    fn binary_content_is_reported_but_never_rendered() {
+        let raw = "diff --git a/dev/null b/.stella/private/store.db\n\
+                   Binary files /dev/null and b/.stella/private/store.db differ\n";
+        assert_eq!(
+            patch_body(raw),
+            "Binary files /dev/null and b/.stella/private/store.db differ"
+        );
+    }
+
+    /// A probe that failed, or a host with no patch channel, contributes
+    /// nothing rather than a fragment. The marker line above it still states
+    /// the file changed.
+    #[test]
+    fn an_unreadable_probe_contributes_nothing() {
+        assert_eq!(patch_body(""), "");
+        assert_eq!(patch_body("fatal: not a git repository\n"), "");
+    }
+}