fix(tests): verify the fixture git template before handing it out

crates/batten/tests/it/common/mod.rs

@@ -807,6 +807,15 @@ pub(crate) fn git_command(dir: &Path, args: &[&str]) -> Command {
 /// of redundant `git init`s is the wrong trade for a once-per-filesystem cost,
 /// and a lock file is one more thing to leak.
 ///
+/// # `None` is an answer, not a failure (CLOUD-1832)
+///
+/// Every reading of a failed publish that does NOT leave a complete template
+/// behind ends here, and the honest report is that this process has no template
+/// — not a path it hopes is one. The caller's fallback is the `git init` fork
+/// this template exists to avoid, which is a cost rather than a defect, so the
+/// suite stays green on a filesystem where the publish cannot land. The state is
+/// printed, not swallowed: the next run that hits it says why.
+///
 /// # The stamp is the `git` binary's own metadata
 ///
 /// A hand-bumped version constant would leave a stale template behind whenever
@@ -826,35 +835,119 @@ pub(crate) fn git_command(dir: &Path, args: &[&str]) -> Command {
 /// `.git/config` is the same values by a cheaper route. A fixture that wants a
 /// DIFFERENT identity still sets it after the copy, and one whose subject is an
 /// UNSET identity unsets it — `attribution.rs` already does exactly that.
-fn git_init_template() -> &'static Path {
-    static TEMPLATE: std::sync::OnceLock<PathBuf> = std::sync::OnceLock::new();
-    TEMPLATE.get_or_init(|| {
-        let published = target_tmp().join(format!("git-init-template-{}", git_stamp()));
-        if published.join("HEAD").is_file() && published.join("config").is_file() {
-            return published;
-        }
-        let staging = target_tmp().join(format!(
-            "git-init-template-{}.staging-{}",
-            git_stamp(),
-            std::process::id()
-        ));
+fn git_init_template() -> Option<&'static Path> {
+    static TEMPLATE: std::sync::OnceLock<Option<PathBuf>> = std::sync::OnceLock::new();
+    TEMPLATE.get_or_init(build_git_init_template).as_deref()
+}
+
+/// Establish the template once per process, or report that this process could
+/// not — see [`git_init_template`] for why the answer is an `Option`.
+#[allow(clippy::print_stderr)]
+fn build_git_init_template() -> Option<PathBuf> {
+    let published = target_tmp().join(format!("git-init-template-{}", git_stamp()));
+    if is_template(&published) {
+        return Some(published);
+    }
+    let staging = target_tmp().join(format!(
+        "git-init-template-{}.staging-{}",
+        git_stamp(),
+        std::process::id()
+    ));
+    let _ = fs::remove_dir_all(&staging);
+    fs::create_dir_all(&staging).expect("create the template staging directory");
+    fork_the_template_into(&staging);
+    // Publish the `.git` itself rather than the work tree around it: what a
+    // fixture copies is a repository directory, and lifting it here keeps
+    // `init_repo` from having to know the template's internal layout.
+    if let Err(error) = fs::rename(staging.join(".git"), &published) {
+        // A FAILED RENAME IS NOT A REPORT THAT SOMEBODY ELSE WON (CLOUD-1832).
+        // That is the LIKELY reading — `rename(2)` onto a non-empty directory
+        // is `ENOTEMPTY` and the winner's copy is complete — but it is not the
+        // only one. A `staging/.git` that was never created, a cross-device
+        // error, a publish interrupted mid-flight: every one of them arrives
+        // here, and returning the path unasked turns "I could not establish a
+        // template" into "here is a template". `init_repo` then copies
+        // whatever is at that path into a fixture, and the first thing to
+        // notice is a `git add -A` three calls later reporting a directory
+        // that is not a repository — a could-not-look wearing a result's
+        // clothes, which is the one thing this repository refuses.
+        //
+        // So the loser's branch reports what it could not establish rather
+        // than assuming it. It does NOT fail the case: the template is a
+        // COST optimisation over a `git init` this module still knows how to
+        // fork, so the sound answer is `None` and one fork for this process.
+        // Failing here would red a suite whose subject is elsewhere for a
+        // reason that is purely about how the fixture was built — the
+        // measured CI failure of exactly that shape is why this is a
+        // fallback and not an assertion.
+        let listing = fs::read_dir(&published).map_or_else(
+            |error| format!("unreadable: {error}"),
+            |entries| {
+                let mut names: Vec<String> = entries
+                    .filter_map(Result::ok)
+                    .map(|entry| entry.file_name().to_string_lossy().into_owned())
+                    .collect();
+                names.sort();
+                format!("[{}]", names.join(" "))
+            },
+        );
+        let staged_git = staging.join(".git").is_dir();
         let _ = fs::remove_dir_all(&staging);
-        fs::create_dir_all(&staging).expect("create the template staging directory");
-        git_in(&staging, &["init", "-q"]);
-        git_in(&staging, &["config", "user.email", "t@example.com"]);
-        git_in(&staging, &["config", "user.name", "t"]);
-        // Publish the `.git` itself rather than the work tree around it: what a
-        // fixture copies is a repository directory, and lifting it here keeps
-        // `init_repo` from having to know the template's internal layout.
-        if fs::rename(staging.join(".git"), &published).is_err() {
-            // Another process published first, which is the whole point of the
-            // rename. Its copy is complete; ours is not needed.
-            let _ = fs::remove_dir_all(&staging);
-            return published;
+        if is_template(&published) {
+            return Some(published);
         }
-        let _ = fs::remove_dir_all(&staging);
-        published
-    })
+        // Diagnostics rather than a verdict: this names the state that the
+        // assertion used to only assert, so the next run that hits it says
+        // WHY instead of that it happened.
+        //
+        // `print_stderr` is allowed here and only here. The lint exists so the
+        // BINARY never writes an ungoverned line to a channel its output
+        // contract owns (house-style §6); this is the test harness, whose
+        // stderr nextest already captures per process and prints on failure.
+        // The alternative is a fallback that leaves no trace at all — a silent
+        // change of route, which is the shape that made this defect expensive
+        // to see. The allowance sits on the function rather than here because
+        // a statement-scoped one does not reach this macro — clippy reports it
+        // as an unused attribute.
+        eprintln!(
+            "note: the template publish at {} failed ({error}; staged .git \
+                 present: {staged_git}) and {} is not a complete template \
+                 (contents: {listing}), so this process forks `git init` per \
+                 fixture instead — see CLOUD-1832",
+            staging.display(),
+            published.display(),
+        );
+        return None;
+    }
+    let _ = fs::remove_dir_all(&staging);
+    Some(published)
+}
+
+/// Build the repository [`git_init_template`] publishes, by forking, in `dir`.
+///
+/// The route [`init_repo`] falls back to when no template could be established,
+/// and the reason it is a named function rather than three lines inside that
+/// `else`: the two `config` calls are not decoration. The template bakes the
+/// identity into its own `config`, the binary under test reads it through
+/// `git::config_value`, and a fallback that forked `init` alone would hand the
+/// fixture an UNSET identity — a difference no fixture asks about and every
+/// attribution case would feel. Same commands, same order, same values as the
+/// staging build above.
+pub(crate) fn fork_the_template_into(dir: &Path) {
+    git_in(dir, &["init", "-q"]);
+    git_in(dir, &["config", "user.email", "t@example.com"]);
+    git_in(dir, &["config", "user.name", "t"]);
+}
+
+/// Whether `dir` is a published template this module may copy from.
+///
+/// The same two files [`git_init_template`] checks before it stages, named once
+/// so the pre-check and the post-check cannot drift into disagreeing about what
+/// "published" means. Deliberately cheap and structural rather than a `git`
+/// fork: the question is whether the directory is a repository at all, and a
+/// fork per fixture is the cost this template exists to remove.
+pub(crate) fn is_template(dir: &Path) -> bool {
+    dir.join("HEAD").is_file() && dir.join("config").is_file()
 }
 
 /// The resolved `git` binary's length and mtime, as one path-safe token.
@@ -931,10 +1024,33 @@ pub(crate) fn init_repo(dir: &Path) {
          template copy is not, so a second initialisation has to be deliberate",
         dir.display()
     );
-    if dir.starts_with(target_tmp()) {
-        copy_tree(git_init_template(), &dir.join(".git"));
-    } else {
+    if !dir.starts_with(target_tmp()) {
         git_in(dir, &["init", "-q"]);
+        return;
+    }
+    let Some(template) = git_init_template() else {
+        // No template could be established (CLOUD-1832): fork what it stands
+        // for.
+        fork_the_template_into(dir);
+        return;
+    };
+    {
+        copy_tree(template, &dir.join(".git"));
+        // CHECK WHAT THE COPY PRODUCED, HERE (CLOUD-1832). `copy_tree` creates
+        // its destination and copies whatever it finds, so a template that was
+        // not a repository yields a `.git` that is not one either — silently.
+        // The failure then surfaces wherever the fixture first runs git, which
+        // in the measured case was `base_commit`'s `git add -A` reporting
+        // "not a git repository" in a case about config loading. Asserting at
+        // the point of creation is the difference between a defect that names
+        // itself and one that reads as an unrelated test being broken.
+        assert!(
+            is_template(&dir.join(".git")),
+            "copying the template {} into {} did not produce a repository — see \
+             CLOUD-1832",
+            template.display(),
+            dir.display(),
+        );
     }
 }
 

crates/batten/tests/it/primitives.rs

@@ -1900,3 +1900,112 @@ fn no_case_names_the_template_directory() {
          `make_empty` would wipe it: {named:?}"
     );
 }
+
+// THE PREDICATE BOTH TEMPLATE ASSERTIONS REST ON (CLOUD-1832).
+//
+// `git_init_template` publishes by atomic rename and, when that rename failed,
+// used to return the published path unasked — reading "rename failed" as
+// "somebody else won and their copy is complete". Every other cause of a failed
+// rename arrived at the same `return`, so a process that could not establish a
+// template handed one back anyway, and `init_repo` copied it into a fixture. The
+// first thing to notice was a `git add -A` several calls later reporting a
+// directory that is not a repository: a could-not-look rendered as a result.
+//
+// Both halves of the repair — the loser's branch, and `init_repo` checking what
+// its copy produced — are straight-line uses of `common::is_template`, so this is
+// where the discrimination has to be shown. Poisoning the REAL published template
+// would be the stronger end-to-end arm and is deliberately not taken: that path
+// is shared by every test process under one `CARGO_TARGET_TMPDIR`, so a case that
+// corrupted it would red its concurrent siblings — which is precisely the
+// cross-process hazard this row is about.
+#[test]
+fn a_directory_is_a_template_only_when_it_carries_a_repositorys_own_files() {
+    let root = common::scratch("is-template");
+
+    assert!(
+        !common::is_template(&root.join("never-created")),
+        "a path that does not exist is not a template"
+    );
+
+    let empty = root.join("empty");
+    std::fs::create_dir_all(&empty).expect("create the empty candidate");
+    assert!(
+        !common::is_template(&empty),
+        "an empty directory is not a template — that is the shape a failed \
+         publish leaves behind, and the one that used to be handed back as \
+         though it were a repository"
+    );
+
+    let headless = root.join("no-head");
+    std::fs::create_dir_all(&headless).expect("create the headless candidate");
+    std::fs::write(headless.join("config"), "[core]\n").expect("write config");
+    assert!(
+        !common::is_template(&headless),
+        "config alone is not a repository"
+    );
+
+    let configless = root.join("no-config");
+    std::fs::create_dir_all(&configless).expect("create the configless candidate");
+    std::fs::write(configless.join("HEAD"), "ref: refs/heads/main\n").expect("write HEAD");
+    assert!(
+        !common::is_template(&configless),
+        "HEAD alone is not a repository"
+    );
+
+    // The positive case comes from a repository this suite actually built, not
+    // from two files this test wrote: a predicate checked only against hand-made
+    // directories could agree with itself and disagree with git.
+    let real = common::Fixture::new("is-template-real")
+        .file("a.txt", "a\n")
+        .git()
+        .base_commit()
+        .build();
+    assert!(
+        common::is_template(&real.join(".git")),
+        "a repository this suite just built must satisfy the predicate its own \
+         fixtures are checked against"
+    );
+}
+
+// CLOUD-1832, the other half: what a process does when it could NOT establish a
+// template.
+//
+// The first repair asserted, and that is what CI then failed on — the assertion
+// fired on a cold musl scratch root and reddened a case whose subject was
+// elsewhere. A missing template is a COST, not a defect: `common` still knows
+// how to fork the repository the template stands for, so the sound answer is to
+// fork it. `init_repo` takes that route whenever `git_init_template` returns
+// `None`, and this pins what the route has to produce.
+//
+// Driving it through `init_repo` itself is not available — the template is a
+// process-wide `OnceLock` and a case cannot un-establish it without redding its
+// concurrent siblings, the same cross-process hazard the case above declines. So
+// the fallback's BODY is the named function both routes call, and it is checked
+// directly: a repository, with the identity the template bakes into its config.
+// A fallback that forked `init` alone would pass `is_template` and still hand
+// every fixture an unset identity, so the identity is asserted, not assumed.
+#[test]
+fn the_route_taken_when_no_template_exists_builds_the_repository_the_template_would_have() {
+    let dir = common::scratch("template-fallback").join("repo");
+    std::fs::create_dir_all(&dir).expect("create the fallback candidate");
+
+    common::fork_the_template_into(&dir);
+
+    assert!(
+        common::is_template(&dir.join(".git")),
+        "the fallback has to produce a repository — it stands in for a template \
+         whose whole content is one"
+    );
+    assert_eq!(
+        common::git_in(&dir, &["config", "user.email"]),
+        "t@example.com",
+        "the template bakes the identity into its own config, so a fallback \
+         that left it unset would differ from the copy route in a way the \
+         binary under test can read"
+    );
+    assert_eq!(
+        common::git_in(&dir, &["config", "user.name"]),
+        "t",
+        "both halves of the identity, for the same reason"
+    );
+}