devflow_core/

git.rs

//! Git-flow operations implemented with plain `git` commands.

use crate::config::GitFlowConfig;
use crate::phase_id::PhaseId;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::time::{Duration, Instant};
use tracing::{debug, info, warn};

/// Errors produced by git-flow operations.
#[derive(Debug, thiserror::Error)]
pub enum GitError {
    /// Spawning git failed.
    #[error("failed to execute git: {0}")]
    Io(#[from] std::io::Error),
    /// Git returned a non-success status.
    #[error("git command failed: {0}")]
    Command(String),
}

/// Git's own list of repository-local environment variables, as reported by
/// `git rev-parse --local-env-vars` (15 entries on git 2.55).
///
/// Kept as a constant rather than shelled out per call so building a command
/// stays free of process spawns; `local_env_vars_match_git` asserts it still
/// agrees with the installed git, so a version that adds one fails loudly
/// instead of silently reopening the hole.
pub const REPO_LOCAL_GIT_VARS: &[&str] = &[
    "GIT_ALTERNATE_OBJECT_DIRECTORIES",
    "GIT_CONFIG",
    "GIT_CONFIG_PARAMETERS",
    "GIT_CONFIG_COUNT",
    "GIT_OBJECT_DIRECTORY",
    "GIT_DIR",
    "GIT_WORK_TREE",
    "GIT_IMPLICIT_WORK_TREE",
    "GIT_GRAFT_FILE",
    "GIT_INDEX_FILE",
    "GIT_NO_REPLACE_OBJECTS",
    "GIT_REPLACE_REF_BASE",
    "GIT_PREFIX",
    "GIT_SHALLOW_FILE",
    "GIT_COMMON_DIR",
];

/// Variables that are not repository-local — and so absent from
/// `--local-env-vars` — but still redirect where git reads or writes.
///
/// `GIT_CEILING_DIRECTORIES` is included for completeness rather than
/// because a live path needs it (27-REVIEW WR-02): every production call
/// site passes an explicit `current_dir` that is already a repository root,
/// so git's upward discovery — the only thing this variable constrains —
/// never runs. Scrubbing an unset variable costs nothing, and including it
/// means no future call site that *does* rely on discovery has to
/// rediscover the reasoning.
pub const ALSO_REDIRECTING_GIT_VARS: &[&str] = &[
    "GIT_NAMESPACE",
    "GIT_DISCOVERY_ACROSS_FILESYSTEM",
    "GIT_CEILING_DIRECTORIES",
];

/// A `git` command pinned to `repo` **and** stripped of every inherited
/// variable that could redirect it somewhere else.
///
/// Use this for every production git invocation instead of building
/// `Command::new("git")` directly. `GIT_EXEC_PATH` is deliberately left
/// alone: it only locates git's own helper binaries and cannot change
/// which repository git acts on.
///
/// Clearing `GIT_CONFIG_COUNT` is sufficient to neutralize any inherited
/// `GIT_CONFIG_KEY_n`/`GIT_CONFIG_VALUE_n` pair — git only reads those when
/// the count is set — so they need no separate sweep.
pub fn git_command(repo: &Path) -> Command {
    hermetic_command("git", repo)
}

/// As [`git_command`], for a program that is not `git` itself but will
/// shell out to it — `cargo`, whose build scripts invoke `git`, is the
/// motivating case. The redirecting variables are inherited all the way
/// down a process tree, so scrubbing only the direct `git` calls would
/// leave that path open.
///
/// The scrub is unconditional: there is no bypass parameter, no
/// environment variable, and no config lookup that can turn it back on.
/// There is no legitimate reason a DevFlow-issued command should silently
/// redirect via an inherited variable — an operator who wants DevFlow to
/// act on a different repository passes it a different path (D-01).
pub fn hermetic_command(program: &str, dir: &Path) -> Command {
    let mut cmd = Command::new(program);
    cmd.current_dir(dir);
    for var in REPO_LOCAL_GIT_VARS.iter().chain(ALSO_REDIRECTING_GIT_VARS) {
        cmd.env_remove(var);
    }
    cmd
}

/// Repository helper bound to a project root.
#[derive(Debug, Clone)]
pub struct GitFlow {
    root: PathBuf,
    config: GitFlowConfig,
}

/// Summary of a feature branch for the `devflow list` command.
#[derive(Debug, Clone)]
pub struct BranchInfo {
    /// Branch name (e.g. "feature/phase-05").
    pub name: String,
    /// Number of commits this branch has that develop doesn't.
    pub ahead: usize,
    /// Number of commits develop has that this branch doesn't.
    pub behind: usize,
    /// ISO-8601 date of the last commit on this branch.
    pub last_commit: String,
}

impl GitFlow {
    /// Create a git-flow helper for a project root, using the hardcoded
    /// git-flow constants (`main`, `develop`, `feature/`).
    pub fn new(root: impl AsRef<Path>) -> Self {
        Self {
            root: root.as_ref().to_path_buf(),
            config: GitFlowConfig::default(),
        }
    }

    /// Create a feature branch from the develop branch.
    ///
    /// Returns an error if the branch already exists (use
    /// [`Self::feature_start_force`] to overwrite).
    pub fn feature_start(&self, phase: PhaseId) -> Result<String, GitError> {
        let branch = format!("{}phase-{}", self.config.feature_prefix, phase.padded());
        info!("creating feature branch: {branch}");
        self.git(["checkout", &self.config.develop])?;
        self.git(["checkout", "-b", &branch])?;
        Ok(branch)
    }

    /// Create or reset a feature branch, overwriting it if it already exists.
    pub fn feature_start_force(&self, phase: PhaseId) -> Result<String, GitError> {
        let branch = format!("{}phase-{}", self.config.feature_prefix, phase.padded());
        warn!("force-creating feature branch: {branch}");
        self.git(["checkout", &self.config.develop])?;
        self.git(["checkout", "-B", &branch])?;
        Ok(branch)
    }

    /// Merge a feature branch into develop and delete it.
    pub fn feature_finish(&self, phase: PhaseId) -> Result<String, GitError> {
        let branch = self.merge_feature_into_develop(phase)?;
        self.git(["branch", "-d", &branch])?;
        Ok(branch)
    }

    /// Merge a feature branch into develop without deleting it.
    ///
    /// Default DevFlow runs keep the feature branch checked out in a linked
    /// worktree, so deletion belongs to the later best-effort cleanup hook.
    pub fn merge_feature_into_develop(&self, phase: PhaseId) -> Result<String, GitError> {
        let branch = format!("{}phase-{}", self.config.feature_prefix, phase.padded());
        info!("merging feature branch: {branch}");
        self.git(["checkout", &self.config.develop])?;
        self.git(["merge", "--no-ff", &branch])?;
        Ok(branch)
    }

    /// Whether a phase feature branch has nothing left to merge into develop.
    ///
    /// An absent branch is not proof of a merge. Callers must fail closed
    /// rather than treating a deleted or never-created branch as shipped.
    pub fn is_merged_into_develop(&self, phase: PhaseId) -> bool {
        let branch = format!("{}phase-{}", self.config.feature_prefix, phase.padded());
        if !self.branch_exists(&branch) {
            return false;
        }

        git_command(&self.root)
            .args(["merge-base", "--is-ancestor", &branch, &self.config.develop])
            .output()
            .map(|output| output.status.success())
            .unwrap_or(false)
    }

    /// Create or reset a release branch from the current `HEAD`.
    ///
    /// The release branch is cut from wherever the caller currently is — the
    /// branch being shipped — not from `develop`. `devflow ship` writes the
    /// version bump into the working tree first, so branching from `HEAD`
    /// keeps any commits unique to the shipped branch in the release.
    pub fn release_start(&self, version: &str) -> Result<String, GitError> {
        let branch = format!("release/{version}");
        info!("creating release branch: {branch}");
        self.git(["checkout", "-B", &branch])?;
        Ok(branch)
    }

    /// Merge a release branch into main and develop, tag it, and delete it.
    pub fn release_finish(&self, version: &str) -> Result<String, GitError> {
        let branch = format!("release/{version}");
        info!("finishing release branch: {branch}");
        self.git(["checkout", &self.config.main])?;
        self.git(["merge", "--no-ff", &branch])?;
        // `-c tag.gpgSign=false` scopes the override to this invocation only
        // (never the user's global/repo config) — without it, a global
        // `tag.gpgsign=true` forces this lightweight tag into an
        // annotated+signed one requiring a message, which blocks on
        // `$EDITOR` in what must be a headless, unattended flow (Phase 13
        // dogfood finding).
        self.git(["-c", "tag.gpgSign=false", "tag", &format!("v{version}")])?;
        self.git(["checkout", &self.config.develop])?;
        self.git(["merge", "--no-ff", &branch])?;
        self.git(["branch", "-d", &branch])?;
        Ok(branch)
    }

    /// Create an annotated-free lightweight tag at the current `HEAD`.
    ///
    /// Passes `-c tag.gpgSign=false` scoped to this invocation only — a
    /// global `tag.gpgsign=true` (common for developers who sign their own
    /// tags) otherwise forces this lightweight tag into an annotated+signed
    /// one requiring a message, which blocks on `$EDITOR` in what must be a
    /// headless, unattended flow (Phase 13 dogfood finding: VersionBump hung
    /// on a live `devflow start --mode auto` run).
    pub fn tag(&self, tag: &str) -> Result<(), GitError> {
        info!("tagging {tag}");
        self.git(["-c", "tag.gpgSign=false", "tag", tag])
    }

    /// Delete a single local branch.
    ///
    /// With `force`, uses `git branch -D` (deletes even if unmerged); otherwise
    /// `git branch -d` (refuses to delete unmerged work). Protected branches
    /// (`main`, `develop`) are never deleted.
    pub fn delete_branch(&self, branch: &str, force: bool) -> Result<(), GitError> {
        if branch == self.config.main || branch == self.config.develop {
            return Err(GitError::Command(format!(
                "refusing to delete protected branch `{branch}`"
            )));
        }
        let flag = if force { "-D" } else { "-d" };
        if force {
            warn!("force-deleting branch: {branch}");
        } else {
            info!("deleting branch: {branch}");
        }
        self.git(["branch", flag, branch])
    }

    /// Whether a local branch exists.
    pub fn branch_exists(&self, branch: &str) -> bool {
        git_command(&self.root)
            .args([
                "rev-parse",
                "--verify",
                "--quiet",
                &format!("refs/heads/{branch}"),
            ])
            .output()
            .map(|o| o.status.success())
            .unwrap_or(false)
    }

    /// The commit SHA at the tip of `branch`.
    pub fn branch_tip(&self, branch: &str) -> Result<String, GitError> {
        Ok(self.git_output(["rev-parse", branch])?.trim().to_string())
    }

    /// Create `branch` at `start_point` if it does not already exist, without
    /// checking it out (leaves the current checkout untouched).
    pub fn ensure_branch(&self, branch: &str, start_point: &str) -> Result<(), GitError> {
        if self.branch_exists(branch) {
            return Ok(());
        }
        self.git(["branch", branch, start_point])
    }

    /// Check out an existing branch in the main worktree.
    pub fn checkout(&self, branch: &str) -> Result<(), GitError> {
        debug!("checking out branch: {branch}");
        self.git(["checkout", branch])
    }

    /// Delete `branch` on `origin` (best-effort; errors if no remote/branch).
    pub fn delete_remote_branch(&self, branch: &str) -> Result<(), GitError> {
        info!("deleting remote branch: {branch}");
        self.git(["push", "origin", "--delete", branch])
    }

    /// Whether the repository has at least one configured remote.
    pub fn has_remote(&self) -> bool {
        self.git_output(["remote"])
            .map(|s| !s.trim().is_empty())
            .unwrap_or(false)
    }

    /// Push `branch` to `origin`, setting upstream.
    pub fn push(&self, branch: &str) -> Result<(), GitError> {
        info!("pushing branch: {branch}");
        self.git(["push", "-u", "origin", branch])
    }

    /// Delete local branches already merged into `develop`.
    ///
    /// WR-04 (13-REVIEW.md): passes `develop` explicitly rather than relying
    /// on `git branch --merged`'s default of "whatever HEAD currently is" —
    /// if the main checkout is ever left on a branch other than `develop`
    /// when this runs, an implicit baseline would silently prune branches
    /// merged into that other branch instead.
    ///
    /// Deletion uses `-D`, not `-d`: `-d` verifies merged-into-HEAD, which
    /// contradicts the `--merged develop` listing above in exactly the
    /// checkout-not-on-develop scenario WR-04 targets (every genuinely
    /// merged branch would be refused as "not fully merged"). The listing IS
    /// the merge safety check. A branch git still refuses to delete (e.g.
    /// checked out in a worktree) is logged and skipped so one failure
    /// doesn't abort the rest of the sweep.
    pub fn cleanup_merged(&self) -> Result<Vec<String>, GitError> {
        let output = self.git_output(["branch", "--merged", &self.config.develop])?;
        let protected = [self.config.main.as_str(), self.config.develop.as_str()];
        let mut deleted = Vec::new();
        for line in output.lines() {
            // git's porcelain marker is an exact two-char prefix ("* " for
            // the current branch, "+ " for a worktree checkout, "  "
            // otherwise) — strip it positionally rather than trimming
            // marker CHARACTERS, which would mangle a branch legitimately
            // named e.g. "+foo" (WR-03, revised).
            let branch = line
                .strip_prefix("* ")
                .or_else(|| line.strip_prefix("+ "))
                .unwrap_or(line)
                .trim();
            // Skip blanks, protected trunks, and the detached-HEAD line
            // ("(HEAD detached at ...)"), which is not a branch name.
            if branch.is_empty() || branch.starts_with('(') || protected.contains(&branch) {
                continue;
            }
            info!("cleaning up merged branch: {branch}");
            match self.git(["branch", "-D", branch]) {
                Ok(()) => deleted.push(branch.to_string()),
                Err(err) => warn!("could not delete merged branch {branch}: {err}"),
            }
        }
        Ok(deleted)
    }

    /// Stage all changes and commit with the given message.
    /// Returns Ok(()) whether or not there were changes to commit.
    pub fn commit_all(&self, message: &str) -> Result<(), GitError> {
        debug!("committing all changes: {message}");
        self.git(["add", "."])?;
        // --allow-empty so we don't fail when there are no changes
        match self.git_raw(&["commit", "--allow-empty", "-m", message]) {
            Ok(()) => Ok(()),
            // If the commit produced no changes and we used --allow-empty,
            // this should still succeed. But just in case, ignore "nothing to commit".
            Err(GitError::Command(ref msg)) if msg.contains("nothing to commit") => Ok(()),
            Err(e) => Err(e),
        }
    }

    /// Stage a single relative path and commit with the given message.
    /// Mirrors `commit_all`, but scoped to one path, for hooks that must not
    /// sweep in unrelated dirty state left by other hooks or the workflow.
    /// Returns Ok(()) whether or not the path had changes to commit. Unlike
    /// `commit_all`, a path with no changes produces **no commit** — it is a
    /// genuine no-op, not a forced empty commit, so a caller such as
    /// `hooks::version_bump` can never tag a release on a commit containing
    /// nothing (19b/D-16).
    pub fn commit_path(&self, relative_path: &str, message: &str) -> Result<(), GitError> {
        debug!("committing {relative_path}: {message}");
        // `add` first so a brand-new file is known to git — a pathspec-only
        // commit errors on a path git has never seen. The trailing pathspec is
        // what actually scopes the commit: without it, `commit` writes whatever
        // else is already in the index, which is exactly the sweep-in this
        // function exists to prevent.
        self.git(["add", relative_path])?;
        match self.git_raw_combined(&["commit", "-m", message, "--", relative_path]) {
            Ok(()) => Ok(()),
            // No forcing flag above, so this arm is now the live no-op path:
            // a path with nothing staged makes git exit non-zero with
            // "nothing to commit", and we convert that back to Ok(()) rather
            // than let it propagate as an error (19b/D-16, T-19-11).
            Err(GitError::Command(ref msg)) if msg.contains("nothing to commit") => Ok(()),
            Err(e) => Err(e),
        }
    }

    /// Return divergence from develop: (ahead, behind) commit counts.
    ///
    /// If currently on the develop branch, returns (0, 0).
    /// `ahead` = commits on current branch not yet on develop.
    /// `behind` = commits on develop not yet on current branch.
    pub fn divergence_from_develop(&self) -> Result<(usize, usize), GitError> {
        let current = self
            .git_output(["rev-parse", "--abbrev-ref", "HEAD"])?
            .trim()
            .to_string();
        if current == self.config.develop {
            return Ok((0, 0));
        }
        let ahead = self
            .rev_count(&format!("{}..{current}", self.config.develop))
            .unwrap_or(0);
        let behind = self
            .rev_count(&format!("{current}..{}", self.config.develop))
            .unwrap_or(0);
        Ok((ahead, behind))
    }

    /// List all feature branches with divergence from develop.
    ///
    /// Returns branches matching `feature/phase-*` with ahead/behind counts
    /// and last commit dates. Protected branches (main, develop) are excluded.
    pub fn list_feature_branches(&self) -> Result<Vec<BranchInfo>, GitError> {
        let prefix = &self.config.feature_prefix;
        let branches = self.git_output(["branch", "--format=%(refname:short)"])?;
        let mut result = Vec::new();
        for name in branches.lines().map(|l| l.trim()) {
            if name.is_empty()
                || name == self.config.main
                || name == self.config.develop
                || !name.starts_with(prefix)
            {
                continue;
            }
            let ahead = self
                .rev_count(&format!("{dev}..{name}", dev = self.config.develop))
                .unwrap_or(0);
            let behind = self
                .rev_count(&format!("{name}..{dev}", dev = self.config.develop))
                .unwrap_or(0);
            let last_commit = self
                .git_output(["log", "-1", "--format=%aI", name])
                .map(|s| s.trim().to_string())
                .unwrap_or_default();
            result.push(BranchInfo {
                name: name.to_string(),
                ahead,
                behind,
                last_commit,
            });
        }
        // Sort by phase number so phase-01 comes before phase-10.
        result.sort_by(|a, b| a.name.cmp(&b.name));
        Ok(result)
    }

    /// Count revisions in the given range. Returns None if the command fails.
    fn rev_count(&self, range: &str) -> Option<usize> {
        self.git_output(["rev-list", "--count", range])
            .ok()
            .and_then(|s| s.trim().parse().ok())
    }

    fn git_raw(&self, args: &[&str]) -> Result<(), GitError> {
        debug!("git {}", args.join(" "));
        // Pin the subprocess locale to C (Antigravity review, 19b): commit_path's
        // "nothing to commit" match arm above compares against git's own
        // English-locale output, which a non-English LC_ALL/LANG would
        // localize, silently defeating the match and reopening 19b under a
        // localized environment (T-19-14). Scoped to this one call path only.
        let output = git_command(&self.root)
            .args(args)
            .env("LC_ALL", "C")
            .env("LANG", "C")
            .output()?;
        if output.status.success() {
            Ok(())
        } else {
            Err(GitError::Command(stderr_or_status(&output)))
        }
    }

    /// Like [`git_raw`](Self::git_raw), but the error text combines stdout
    /// with stderr instead of inspecting stderr alone.
    ///
    /// Discovered empirically while implementing 19b: `git commit`'s
    /// "nothing to commit, working tree clean" message is written to
    /// **stdout**, not stderr. `stderr_or_status` only ever inspects
    /// `output.stderr`, so a plain `git_raw` error can never contain that
    /// text — `commit_path`'s `nothing to commit` match arm (immediately
    /// above its call site) would never fire, no matter how the arm itself
    /// is written. This sibling exists solely so `commit_path` can see it;
    /// `commit_all` keeps calling `git_raw` unchanged (D-17 out of scope),
    /// and `git_raw`'s own error-mapping branch is untouched by this
    /// addition.
    fn git_raw_combined(&self, args: &[&str]) -> Result<(), GitError> {
        debug!("git {}", args.join(" "));
        let output = git_command(&self.root)
            .args(args)
            .env("LC_ALL", "C")
            .env("LANG", "C")
            .output()?;
        if output.status.success() {
            Ok(())
        } else {
            let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
            let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string();
            let combined = match (stderr.is_empty(), stdout.is_empty()) {
                (false, false) => format!("{stderr}\n{stdout}"),
                (false, true) => stderr,
                (true, false) => stdout,
                (true, true) => format!("exited with {}", output.status),
            };
            Err(GitError::Command(combined))
        }
    }

    fn git<const N: usize>(&self, args: [&str; N]) -> Result<(), GitError> {
        debug!("git {}", args.iter().copied().collect::<Vec<_>>().join(" "));
        let output = git_command(&self.root).args(args).output()?;
        if output.status.success() {
            Ok(())
        } else {
            Err(GitError::Command(stderr_or_status(&output)))
        }
    }

    fn git_output<const N: usize>(&self, args: [&str; N]) -> Result<String, GitError> {
        let output = git_command(&self.root).args(args).output()?;
        if output.status.success() {
            Ok(String::from_utf8_lossy(&output.stdout).to_string())
        } else {
            Err(GitError::Command(stderr_or_status(&output)))
        }
    }
}

/// Result of checking whether `origin/main` is already an ancestor of
/// `HEAD` — i.e. whether `scripts/sync-main-to-develop.sh` would be a no-op
/// — WITHOUT issuing any `git fetch` (20d, review: Codex HIGH — a
/// "read-only" preflight must not depend on the network).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AncestorStatus {
    /// `origin/main` is an ancestor of `HEAD` — sync would be a no-op.
    Ancestor,
    /// `origin/main` resolves locally but is NOT an ancestor of `HEAD` —
    /// develop has diverged and `scripts/sync-main-to-develop.sh` should be
    /// run before cutting the next release.
    Diverged,
    /// `origin/main` does not resolve locally at all (never fetched, or no
    /// remote configured). Distinct from [`Diverged`](Self::Diverged) so
    /// the caller can degrade to an actionable "run `git fetch` first"
    /// message instead of reporting a false divergence.
    RefAbsent,
}

/// Check whether `origin/main` is an ancestor of `HEAD`, against
/// ALREADY-FETCHED local refs — issues NO `git fetch`. Mirrors
/// `scripts/sync-main-to-develop.sh`'s own `git merge-base --is-ancestor
/// origin/main HEAD` invocation (`:41`), minus the preceding `git fetch`
/// (`:38`), which mutates `.git/FETCH_HEAD`/tracking refs and would make a
/// "read-only" preflight false (20d, review: Codex HIGH).
pub fn origin_main_ancestor_status(project_root: &Path) -> AncestorStatus {
    let ref_exists = git_command(project_root)
        .args(["rev-parse", "--verify", "--quiet", "origin/main"])
        .output()
        .map(|out| out.status.success())
        .unwrap_or(false);
    if !ref_exists {
        return AncestorStatus::RefAbsent;
    }
    let is_ancestor = git_command(project_root)
        .args(["merge-base", "--is-ancestor", "origin/main", "HEAD"])
        .output()
        .map(|out| out.status.success())
        .unwrap_or(false);
    if is_ancestor {
        AncestorStatus::Ancestor
    } else {
        AncestorStatus::Diverged
    }
}

/// Derive the crates.io publish order for a workspace's local-path members
/// (e.g. `devflow-core` before `devflow`) — sourced from the workspace's own
/// `[workspace] members` list and each member's own `[dependencies]`
/// section (which member depends on which), never a hardcoded prose string
/// (20d). Read-only; returns an empty `Vec` (never panics) if the workspace
/// Cargo.toml or a member manifest cannot be read.
pub fn publish_order(project_root: &Path) -> Vec<String> {
    let Ok(root_contents) = std::fs::read_to_string(project_root.join("Cargo.toml")) else {
        return Vec::new();
    };
    let member_paths = workspace_member_paths(&root_contents);

    let mut members: Vec<(String, String)> = Vec::new();
    for path in &member_paths {
        let manifest = project_root.join(path).join("Cargo.toml");
        let Ok(contents) = std::fs::read_to_string(&manifest) else {
            continue;
        };
        let name = package_name(&contents).unwrap_or_else(|| path.clone());
        members.push((name, contents));
    }

    let names: Vec<String> = members.iter().map(|(name, _)| name.clone()).collect();
    let mut edges: Vec<(String, String)> = Vec::new();
    for (name, contents) in &members {
        for other in &names {
            if other != name && member_depends_on(contents, other) {
                edges.push((name.clone(), other.clone()));
            }
        }
    }
    topo_sort(names, edges)
}

/// Extract the `[workspace] members = [...]` array's quoted path entries.
/// Hand-rolled, single-array-only scan (this project deliberately avoids a
/// TOML parser dependency for its version/workspace tooling — see
/// `version.rs`).
fn workspace_member_paths(contents: &str) -> Vec<String> {
    let Some(start) = contents.find("members") else {
        return Vec::new();
    };
    let rest = &contents[start..];
    let Some(open) = rest.find('[') else {
        return Vec::new();
    };
    let Some(close) = rest[open..].find(']') else {
        return Vec::new();
    };
    let inner = &rest[open + 1..open + close];
    inner
        .split(',')
        .filter_map(|fragment| {
            let fragment = fragment.trim();
            let fragment = fragment.strip_prefix('"')?.strip_suffix('"')?;
            (!fragment.is_empty()).then(|| fragment.to_string())
        })
        .collect()
}

/// Extract a member manifest's `[package] name`.
fn package_name(contents: &str) -> Option<String> {
    let mut current = String::new();
    for line in contents.lines() {
        let trimmed = line.trim();
        if let Some(inner) = trimmed.strip_prefix('[').and_then(|s| s.strip_suffix(']')) {
            current = inner.trim().to_string();
            continue;
        }
        if current == "package"
            && let Some((key, value)) = trimmed.split_once('=')
            && key.trim() == "name"
        {
            return Some(value.trim().trim_matches('"').to_string());
        }
    }
    None
}

/// Whether a member manifest's `[dependencies]` section references
/// `dep_name` — either `dep_name.workspace = true` or `dep_name = { ... }`
/// under an inline `[dependencies]` table, OR the equally-valid expanded
/// long-form section `[dependencies.dep_name]` (WR-03, phase 20 review): a
/// manifest may spell a dependency out as its own section (e.g.
/// `[dependencies.devflow-core]\nworkspace = true`), which parses to a
/// section header of `"dependencies.devflow-core"` — never equal to the
/// plain `"dependencies"` the inline-table branch below checks against, so
/// that edge was previously dropped from `publish_order`'s topo-sort
/// entirely.
fn member_depends_on(contents: &str, dep_name: &str) -> bool {
    let mut current = String::new();
    for line in contents.lines() {
        let trimmed = line.trim();
        if let Some(inner) = trimmed.strip_prefix('[').and_then(|s| s.strip_suffix(']')) {
            current = inner.trim().to_string();
            if let Some(name) = current.strip_prefix("dependencies.")
                && name == dep_name
            {
                return true;
            }
            continue;
        }
        if current != "dependencies" {
            continue;
        }
        let key = trimmed.split(['.', '=']).next().unwrap_or("").trim();
        if key == dep_name {
            return true;
        }
    }
    false
}

/// Kahn's-algorithm topological sort: `edges` are `(dependent, dependency)`
/// pairs, meaning `dependent` must be published AFTER `dependency`. Falls
/// back to appending whatever remains (rather than looping forever) if a
/// cycle is present — a genuine cyclic Cargo dependency would already fail
/// `cargo build` long before this check runs.
fn topo_sort(names: Vec<String>, edges: Vec<(String, String)>) -> Vec<String> {
    let mut result = Vec::new();
    let mut published: Vec<String> = Vec::new();
    let mut remaining = names;
    while !remaining.is_empty() {
        let ready: Vec<String> = remaining
            .iter()
            .filter(|name| {
                edges
                    .iter()
                    .filter(|(dependent, _)| dependent == *name)
                    .all(|(_, dep)| published.contains(dep))
            })
            .cloned()
            .collect();
        if ready.is_empty() {
            result.extend(remaining);
            break;
        }
        for name in &ready {
            published.push(name.clone());
            result.push(name.clone());
        }
        remaining.retain(|name| !ready.contains(name));
    }
    result
}

// ---------------------------------------------------------------------------
// tag-signing viability (20d, Pattern 4)
// ---------------------------------------------------------------------------

// REMOVED in v2.5.0 (999.86, D-04/D-08) — `pub enum SigningStatus` and
// `pub fn classify_ssh_add_status` used to live here, immediately below this
// banner. Both were `pub` items of this crate, so their removal is a breaking
// change; it is enumerated in `CHANGELOG.md` under 2.5.0. The private
// `inline_key_fingerprint` helper went with them, orphaned by D-03.
//
// Why they are gone: they PREDICTED tag-signing viability by classifying
// `ssh-add -l`'s exit code and comparing fingerprints — that is, they inferred
// it from the agent's identity list. An agent listing cannot see private key
// material sitting unencrypted on disk, so the predictor tested a condition the
// real signing operation does not require, and reported `NotViable` for a
// perfectly signable key that no agent happened to hold. That is not a
// hypothetical: it false-negatived on two separate release cuts with the
// correct key present.
//
// What replaced them: `check_signing_viability` below, which establishes
// viability by PERFORMING the operation — a bounded, non-interactive
// `ssh-keygen -Y sign` over throwaway bytes in a private per-call workspace,
// whose exit code is the whole verdict. A probe has no independent behaviour to
// drift out of sync with what `git tag -s` actually does, which is the
// structural property the predictor lacked rather than a bug it happened to
// have.
//
// This note exists because a bare absence invites the mistake in reverse. Dead
// public API that still reads like the sanctioned way to judge signing
// viability is how the predictor survived review twice; do not reintroduce an
// agent-membership check here under a new name.

/// Outcome of the tag-signing viability check. Carries only a boolean-ish
/// status plus an optional PUBLIC key fingerprint — never private key
/// material or a full filesystem path (T-20-04, ASVS V6 / WR-02 — mirrors
/// the existing "no path/username" discipline this project already applies
/// elsewhere, e.g. `PhaseFinding`).
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum SigningViability {
    /// Signing is viable. `fingerprint` is the matched public key's
    /// `SHA256:...` fingerprint, when one could be extracted.
    Viable { fingerprint: Option<String> },
    /// Not viable, with an actionable (never key-leaking) reason.
    NotViable { reason: String },
    /// Could not be determined — tool absent, format unset with no key,
    /// etc. Fail-soft: never a crash.
    Unknown { reason: String },
}

/// `git config --get <key>`, scoped to `project_root`. `None` if unset or
/// the command fails (missing `git`, not a repo, etc.) — never panics.
fn git_config(project_root: &Path, key: &str) -> Option<String> {
    let output = git_command(project_root)
        .args(["config", "--get", key])
        .output()
        .ok()?;
    if !output.status.success() {
        return None;
    }
    let value = String::from_utf8_lossy(&output.stdout).trim().to_string();
    (!value.is_empty()).then_some(value)
}

/// `ssh-keygen -lf <pub_key_path>`'s fingerprint (`SHA256:...`) — reads only
/// the PUBLIC key file, never a private key, and returns only the hash
/// token, never a filesystem path.
fn public_key_fingerprint(pub_key_path: &Path) -> Option<String> {
    let path_str = pub_key_path.to_str()?;
    let output = Command::new("ssh-keygen")
        .args(["-lf", path_str])
        .output()
        .ok()?;
    if !output.status.success() {
        return None;
    }
    // Format: "<bits> SHA256:<hash> <comment> (<type>)"
    String::from_utf8_lossy(&output.stdout)
        .split_whitespace()
        .nth(1)
        .map(str::to_string)
}

/// Classifies a `user.signingkey` value the way `git` itself does (mirrors
/// `man git-config`'s `user.signingKey` precedence, D-01): a `key::`-prefixed
/// value is inline with the prefix stripped; otherwise a value starting with
/// the deprecated raw `ssh-` compat form is inline as-is; otherwise the value
/// is a filesystem path. Pure — no I/O, no `Path`, no `.exists()` — so the
/// classification never depends on the host's filesystem.
///
/// The prefix decides unconditionally (D-02): a value that also happens to
/// name an existing file (e.g. `ssh-key.pub`) is still classified inline,
/// because git never stats the value. The raw allowlist is `ssh-` only
/// (D-03) — `ecdsa-`/`sk-` bare forms are NOT added here; git treats those as
/// paths, and they only reach the inline branch through the `key::` prefix.
fn inline_signing_key_blob(signingkey: &str) -> Option<&str> {
    let trimmed = signingkey.trim();
    if let Some(remainder) = trimmed.strip_prefix("key::") {
        Some(remainder)
    } else if trimmed.starts_with("ssh-") {
        Some(trimmed)
    } else {
        None
    }
}

/// The SSHSIG namespace `git` itself writes into a tag signature.
///
/// Decoded byte-for-byte out of a real git-produced SSHSIG blob — this
/// repository's own `v2.4.0` signed tag. After the `SSHSIG` magic and the
/// uint32 version come the length-prefixed public key and then the
/// namespace field, which reads `\0\0\0\x03git`; the following `sha512`
/// hash-algorithm field lands exactly where that length says it should,
/// which is what makes the offset reading self-checking rather than a
/// guess.
///
/// Do NOT re-derive this value from documentation or from memory. The
/// probe's entire worth is that it performs the operation git performs
/// rather than approximating it, and a namespace that differs from git's
/// would silently make the probe measure something git never does.
const SSH_SIGN_NAMESPACE: &str = "git";

/// Wall-clock ceiling for the signing probe (D-01).
///
/// `SSH_ASKPASS_REQUIRE=never` closes the passphrase-prompt route; this
/// closes the rest. Both are required: the env var alone leaves non-askpass
/// blocking routes open (a wedged `ssh-agent`, a stalled PKCS11 provider —
/// reasoned, not measured), and a timeout alone would turn a working
/// graphical askpass into a false `NotViable`.
const SSH_SIGN_PROBE_TIMEOUT: Duration = Duration::from_secs(10);

/// Poll interval while waiting for the probe child to exit.
const SSH_SIGN_PROBE_POLL: Duration = Duration::from_millis(25);

/// A probe-workspace directory name unique to each individual CALL (F-8).
///
/// Per-*process* uniqueness is not enough: `cargo test` runs tests as
/// parallel threads inside one process, so a name derived from the process
/// id alone is shared by every concurrent probe. Two probes would collide,
/// the loser's non-recursive `create_dir` would fail, and it would fail
/// soft to `Unknown` — a flaky result that points at the probe rather than
/// at the caller.
///
/// Three parts, `std` only (this crate adds no dependency): the process id,
/// a process-wide counter incremented on every call, and a sub-millisecond
/// time component. Extracted as its own function so the uniqueness property
/// can be asserted directly rather than inferred from a probe result.
fn probe_workspace_name() -> String {
    use std::sync::atomic::{AtomicU64, Ordering};
    static PROBE_SEQ: AtomicU64 = AtomicU64::new(0);

    let seq = PROBE_SEQ.fetch_add(1, Ordering::Relaxed);
    let nanos = std::time::SystemTime::now()
        .duration_since(std::time::UNIX_EPOCH)
        .map(|since| since.as_nanos())
        .unwrap_or(0);
    format!(
        "devflow-sign-probe-{}-{}-{}",
        std::process::id(),
        seq,
        nanos
    )
}

/// What one run of the signing probe established. Five outcomes, mapped to
/// fixed reason strings by class at the single call site — never composed
/// from `ssh-keygen`'s own output (D-02, D-08).
enum SignProbeOutcome {
    /// The child exited zero: this key really can sign.
    Signed,
    /// The child exited non-zero: this key really cannot sign.
    Rejected,
    /// The child outlived [`SSH_SIGN_PROBE_TIMEOUT`] and was killed and
    /// reaped.
    TimedOut,
    /// The child could not be spawned — `ssh-keygen` is absent.
    ToolMissing,
    /// The probe could not be set up or supervised at all (its workspace
    /// could not be created, the payload could not be written, or the child
    /// could not be polled). Fail-soft: an infrastructure problem is not
    /// evidence about the key.
    NotRun,
}

/// Removes its directory when dropped, so the workspace goes away on EVERY
/// exit path from [`run_ssh_sign_probe`] — including an unwind (WR-07,
/// 35-REVIEW).
///
/// The plain `remove_dir_all` statement this replaces covered every `return`
/// inside `sign_probe_within`, which is what its comment was written for, but a
/// panic anywhere in that function skipped it. Over many `release --check` runs
/// on a long-lived host that is unbounded accumulation of
/// `devflow-sign-probe-*` directories in `/tmp`.
struct ProbeWorkspace(PathBuf);

impl Drop for ProbeWorkspace {
    fn drop(&mut self) {
        let _ = std::fs::remove_dir_all(&self.0);
    }
}

/// Sign throwaway bytes with the configured key and report only how that
/// went. Creates a private workspace, runs the probe inside it, and removes
/// the workspace on every exit path — including the timeout path, since
/// `ssh-keygen -Y sign` writes its signature as a sibling of the payload
/// (T-35-13: both live and die inside this one directory) — and including an
/// unwind, via [`ProbeWorkspace`]'s `Drop`.
fn run_ssh_sign_probe(key_path: &Path) -> SignProbeOutcome {
    let workspace = std::env::temp_dir().join(probe_workspace_name());

    // Non-recursive on purpose (T-35-12). `DirBuilder::create` FAILS when the
    // path already exists — `recursive(true)` is never set — so a pre-planted
    // directory or symlink cannot redirect where the payload is written;
    // `create_dir_all` would accept one silently. `tempfile` is a
    // dev-dependency of this crate and is unavailable to production code, and
    // no dependency may be added, so this is `std` only.
    //
    // WR-07: mode 0o700, so "private" is implemented rather than merely
    // claimed. `std::fs::create_dir` applies `0o777 & !umask` — typically
    // 0o755, world-readable and world-traversable inside a shared
    // `std::env::temp_dir()`. Nothing secret lands here (the payload is fixed
    // bytes and `payload.sig` signs those bytes), so this was not an exposure
    // of key material; it is that a future author extending the probe would
    // read the comment rather than the mode bits.
    if !create_probe_workspace(&workspace) {
        return SignProbeOutcome::NotRun;
    }
    let _cleanup = ProbeWorkspace(workspace.clone());

    sign_probe_within(&workspace, key_path)
}

/// Create the probe's workspace directory, owner-only and non-recursively.
/// `false` means it could not be created — including because something was
/// already there.
///
/// Split from [`run_ssh_sign_probe`] so both properties can be asserted on a
/// directory that still exists: the probe removes its workspace before
/// returning, so nothing downstream can inspect the mode bits (WR-07).
fn create_probe_workspace(workspace: &Path) -> bool {
    let mut builder = std::fs::DirBuilder::new();
    #[cfg(unix)]
    std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
    builder.create(workspace).is_ok()
}

/// The probe proper, with `workspace` already created and guaranteed to be
/// removed by the caller.
fn sign_probe_within(workspace: &Path, key_path: &Path) -> SignProbeOutcome {
    // Bytes the probe generated itself, inside its own private directory
    // (T-35-13). A viability check must never become an unauthorised
    // signing operation over real content, so nothing from the operator's
    // working tree is ever signed or even read here.
    let payload = workspace.join("payload");
    if std::fs::write(&payload, b"devflow signing viability probe\n").is_err() {
        return SignProbeOutcome::NotRun;
    }

    let (Some(key_arg), Some(payload_arg)) = (key_path.to_str(), payload.to_str()) else {
        return SignProbeOutcome::NotRun;
    };

    let mut command = Command::new("ssh-keygen");
    command
        .args([
            "-Y",
            "sign",
            "-n",
            SSH_SIGN_NAMESPACE,
            "-f",
            key_arg,
            payload_arg,
        ])
        // D-01: closes the ASKPASS route, so an encrypted key cannot park an
        // unattended preflight on an askpass helper. It does NOT close the
        // /dev/tty route — see the `setsid` call below.
        .env("SSH_ASKPASS_REQUIRE", "never")
        .stdin(Stdio::null())
        .stdout(Stdio::null())
        // The child's stderr is discarded here and read by nobody: it
        // embeds the configured key path verbatim (`Couldn't load public
        // key ./does-not-exist.pub`), so reproducing any part of it in a
        // reason string would violate D-08's redaction contract. The exit
        // code is the sole verdict (D-02).
        .stderr(Stdio::null());

    // SAFETY: `setsid` is a bare syscall and async-signal-safe, which is the
    // only requirement `pre_exec` imposes.
    //
    // Detach from any controlling terminal before exec. `SSH_ASKPASS_REQUIRE
    // =never` alone is NOT sufficient: OpenSSH only consults it once
    // `open("/dev/tty")` has failed, so on a host that HAS a controlling
    // terminal `ssh-keygen` prompts for the passphrase on the terminal
    // regardless of the variable and blocks there until the ceiling expires.
    // Measured on this host with a real pty: 10.06s (i.e. the whole
    // SSH_SIGN_PROBE_TIMEOUT) before the fix, 0.02s after. Dropping the
    // controlling terminal makes that `open` fail, which is the condition
    // the variable is gated on.
    //
    // The failure is ignored deliberately. `setsid` only fails when the
    // caller is already a process-group leader, which a freshly forked child
    // is not; and if it somehow did fail, the probe degrades to exactly the
    // pre-fix behaviour, which the wall-clock ceiling already bounds. Turning
    // it into a spawn error would be worse: `spawn` failure is classified as
    // absent tooling, so it would surface as a false "ssh-keygen not found".
    unsafe {
        std::os::unix::process::CommandExt::pre_exec(&mut command, || {
            libc::setsid();
            Ok(())
        });
    }

    let mut child = match command.spawn() {
        Ok(child) => child,
        Err(_) => return SignProbeOutcome::ToolMissing,
    };

    // Bounded wait, following `canary.rs`'s `reap` shape: poll until the
    // deadline, then kill and wait so no child is left behind.
    let deadline = Instant::now() + SSH_SIGN_PROBE_TIMEOUT;
    loop {
        match child.try_wait() {
            Ok(Some(status)) => {
                return if status.success() {
                    SignProbeOutcome::Signed
                } else {
                    SignProbeOutcome::Rejected
                };
            }
            Ok(None) => {}
            Err(_) => {
                // Could not poll: nothing was established about the key, so
                // reap the child and degrade rather than inventing a verdict.
                let _ = child.kill();
                let _ = child.wait();
                return SignProbeOutcome::NotRun;
            }
        }
        if Instant::now() >= deadline {
            break;
        }
        std::thread::sleep(SSH_SIGN_PROBE_POLL);
    }
    let _ = child.kill();
    let _ = child.wait();
    SignProbeOutcome::TimedOut
}

/// `gpg.format == "ssh"` branch (Pattern 4): `user.signingkey` must be set.
/// Its value is classified by git's own prefix rules (D-01) into either an
/// inline key blob or a filesystem path; only a path value is required to
/// exist, and only a path value is probed (D-03).
///
/// Viability is then established by **performing the operation** — signing
/// throwaway bytes with `ssh-keygen -Y sign` — rather than by predicting it
/// from `ssh-add -l`. The predictor this replaced inferred viability from
/// agent membership, which is not a necessary condition for `git tag -s` to
/// succeed: an unencrypted private key sitting beside the configured public
/// key signs fine with no agent at all. That gap false-negatived live on
/// two separate release cuts (999.86). A probe cannot drift out of sync
/// with git's real behaviour because it has no independent behaviour.
///
/// The probe's exit code is the sole verdict (D-02) and its stderr is never
/// re-emitted; on success only the public key's `SHA256:` fingerprint is
/// reported, never the configured value in any form (D-08's redaction
/// contract, unchanged).
fn check_ssh_signing_viability(project_root: &Path) -> SigningViability {
    let Some(signingkey) = git_config(project_root, "user.signingkey") else {
        return SigningViability::NotViable {
            reason: "gpg.format=ssh but user.signingkey is not set".into(),
        };
    };

    // Mirrors `man git-config`'s user.signingKey precedence (D-01): key::
    // form, then deprecated raw ssh- form, else a path. Never stat a path
    // for a prefix-matched value (D-02). Classification runs BEFORE the
    // value is treated as a filesystem path, so an inline value never
    // reaches the `.exists()` check below.
    if inline_signing_key_blob(&signingkey).is_some() {
        // D-03/A-17: inline values are not probed at all. Probing one would
        // mean materialising the blob to a temp file — measured to work,
        // declined on surface cost. The operator gets no verdict here
        // rather than a wrong one.
        return SigningViability::Unknown {
            reason: "cannot verify signing viability — an inline user.signingkey is not probed"
                .into(),
        };
    }

    // Path branch keeps today's early return, byte-for-byte (D-12): the
    // `.exists()` check runs first and a missing file still returns the
    // existing missing-key-file `NotViable` before anything is spawned.
    let key_path = Path::new(&signingkey);
    if !key_path.exists() {
        return SigningViability::NotViable {
            reason: "user.signingkey is set but the key file does not exist".into(),
        };
    }

    sign_probe_verdict(run_ssh_sign_probe(key_path), key_path)
}

/// The probe outcome → operator-facing verdict mapping, split out from
/// [`check_ssh_signing_viability`] so it can be asserted for every variant
/// without spawning anything (WR-01). Forcing a real `TimedOut` needs a
/// 10-second wedged `ssh-keygen`; the classification that was wrong is right
/// here, and this is the level it can be pinned at.
///
/// Fixed reason strings keyed by failure class (D-02) — none is composed
/// from `ssh-keygen`'s output, and none names the configured key, a path,
/// or any part of the child's stderr. Every fail-soft class keeps the
/// file's existing "cannot verify signing viability — " prefix.
fn sign_probe_verdict(outcome: SignProbeOutcome, key_path: &Path) -> SigningViability {
    match outcome {
        SignProbeOutcome::Signed => SigningViability::Viable {
            fingerprint: public_key_fingerprint(key_path),
        },
        SignProbeOutcome::Rejected => SigningViability::NotViable {
            reason: "the configured signing key could not sign a test payload".into(),
        },
        // WR-01 (35-REVIEW): a timeout is a MEASUREMENT failure, and this
        // file argues that twice already — `NotRun`'s doc comment ("an
        // infrastructure problem is not evidence about the key") and 20d/D-06
        // (an unavailable tool yields `Unknown`, never a hard-fail
        // `NotViable`). D-01's own justification for the ceiling names a
        // wedged `ssh-agent` and a stalled PKCS11 provider; both are
        // infrastructure, and neither says anything about whether the key
        // signs. A FIDO/`sk-` key is the concrete case: `ssh-keygen -Y sign`
        // waits for a physical touch, the prompt reaches nobody (stdio is
        // nulled and `setsid` dropped the terminal), and ten seconds later a
        // key that `git tag -s` signs with fine would have hard-failed a
        // release cut — the 999.86 defect class reintroduced by the
        // replacement.
        SignProbeOutcome::TimedOut => SigningViability::Unknown {
            reason: "cannot verify signing viability — the signing probe did not finish \
                     within its time limit"
                .into(),
        },
        SignProbeOutcome::ToolMissing => SigningViability::Unknown {
            reason: "cannot verify signing viability — ssh-keygen not found".into(),
        },
        SignProbeOutcome::NotRun => SigningViability::Unknown {
            reason: "cannot verify signing viability — the signing probe could not be run".into(),
        },
    }
}

/// `gpg.format` unset or `"openpgp"` branch (Pattern 4): verify a secret
/// key exists for `user.signingkey` via `gpg --list-secret-keys`.
fn check_gpg_signing_viability(project_root: &Path) -> SigningViability {
    let Some(signingkey) = git_config(project_root, "user.signingkey") else {
        return SigningViability::Unknown {
            reason: "cannot verify signing viability — user.signingkey is not set".into(),
        };
    };
    let output = match Command::new("gpg")
        .args(["--list-secret-keys", &signingkey])
        .output()
    {
        Ok(out) => out,
        Err(_) => {
            return SigningViability::Unknown {
                reason: "cannot verify signing viability — gpg not found".into(),
            };
        }
    };
    if output.status.success() {
        SigningViability::Viable {
            fingerprint: Some(signingkey),
        }
    } else {
        SigningViability::NotViable {
            reason: "no secret key found for the configured user.signingkey".into(),
        }
    }
}

/// Tag-signing viability check (20d): branches on `git config gpg.format`
/// since the check is a genuinely different code path per format — a
/// GPG-only check would miss the `ssh_askpass` failure this project's own
/// release actually hit (Pattern 4). Fail-soft throughout: an absent tool
/// or unset config degrades to an actionable [`SigningViability::Unknown`],
/// never a crash.
pub fn check_signing_viability(project_root: &Path) -> SigningViability {
    match git_config(project_root, "gpg.format").as_deref() {
        Some("ssh") => check_ssh_signing_viability(project_root),
        _ => check_gpg_signing_viability(project_root),
    }
}

fn stderr_or_status(output: &std::process::Output) -> String {
    let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
    if stderr.is_empty() {
        format!("exited with {}", output.status)
    } else {
        stderr
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use tempfile::TempDir;

    /// Run a git command in `root`, asserting success.
    fn git(root: &Path, args: &[&str]) {
        let output = crate::test_support::git_command(root)
            .args(args)
            .output()
            .expect("spawn git");
        assert!(
            output.status.success(),
            "git {args:?} failed: {}",
            String::from_utf8_lossy(&output.stderr)
        );
    }

    fn current_branch(root: &Path) -> String {
        let output = crate::test_support::git_command(root)
            .args(["rev-parse", "--abbrev-ref", "HEAD"])
            .output()
            .expect("rev-parse");
        String::from_utf8_lossy(&output.stdout).trim().to_string()
    }

    fn commit_file(root: &Path, name: &str) {
        std::fs::write(root.join(name), name).unwrap();
        git(root, &["add", "."]);
        git(root, &["commit", "-q", "-m", &format!("add {name}")]);
    }

    /// Initialize a repo with `main` and `develop` branches and one commit.
    fn init_repo() -> TempDir {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        git(root, &["init", "-q"]);
        git(root, &["config", "user.email", "test@example.com"]);
        git(root, &["config", "user.name", "Test"]);
        git(root, &["config", "commit.gpgsign", "false"]);
        git(root, &["config", "tag.gpgsign", "false"]);
        // Disable any globally-configured hooks (e.g. gitleaks) for isolation.
        git(root, &["config", "core.hooksPath", "/dev/null"]);
        commit_file(root, "README.md");
        git(root, &["branch", "-M", "main"]);
        git(root, &["checkout", "-q", "-b", "develop"]);
        dir
    }

    fn flow(root: &Path) -> GitFlow {
        GitFlow::new(root)
    }

    #[test]
    fn feature_start_branches_from_develop() {
        let repo = init_repo();
        let root = repo.path();
        let branch = flow(root)
            .feature_start(PhaseId::new(3))
            .expect("feature_start");
        assert_eq!(branch, "feature/phase-03");
        assert_eq!(current_branch(root), "feature/phase-03");
    }

    #[test]
    fn list_feature_branches_reports_ahead_and_behind_semantics() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        gf.feature_start(PhaseId::new(12)).expect("feature_start");
        commit_file(root, "feature-one.txt");
        commit_file(root, "feature-two.txt");
        git(root, &["checkout", "-q", "develop"]);
        commit_file(root, "develop-only.txt");

        let branches = gf.list_feature_branches().unwrap();
        let branch = branches
            .iter()
            .find(|branch| branch.name == "feature/phase-12")
            .unwrap();

        assert_eq!(branch.ahead, 2);
        assert_eq!(branch.behind, 1);
    }

    #[test]
    fn feature_finish_merges_into_develop_and_deletes() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        gf.feature_start(PhaseId::new(1)).expect("start");
        commit_file(root, "feature.txt");

        let branch = gf.feature_finish(PhaseId::new(1)).expect("finish");
        assert_eq!(branch, "feature/phase-01");
        assert_eq!(current_branch(root), "develop");

        // Branch is deleted and its work is now on develop.
        let branches = crate::test_support::git_command(root)
            .args(["branch"])
            .output()
            .unwrap();
        let listing = String::from_utf8_lossy(&branches.stdout);
        assert!(!listing.contains("feature/phase-01"));
        assert!(root.join("feature.txt").exists());
    }

    #[test]
    fn release_start_and_finish_tags_main_and_merges_both() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // Add work on develop so the release has content.
        commit_file(root, "work.txt");
        let branch = gf.release_start("1.2.0").expect("release_start");
        assert_eq!(branch, "release/1.2.0");

        gf.release_finish("1.2.0").expect("release_finish");
        assert_eq!(current_branch(root), "develop");

        // Tag exists.
        let tags = crate::test_support::git_command(root)
            .args(["tag"])
            .output()
            .unwrap();
        assert!(String::from_utf8_lossy(&tags.stdout).contains("v1.2.0"));

        // Release branch deleted.
        let branches = crate::test_support::git_command(root)
            .args(["branch"])
            .output()
            .unwrap();
        assert!(!String::from_utf8_lossy(&branches.stdout).contains("release/1.2.0"));
    }

    /// A global/repo `tag.gpgsign=true` must not turn `tag()`'s lightweight
    /// tag into an annotated+signed one — that would require a tag message
    /// and block on `$EDITOR`, silently hanging a headless, unattended run
    /// (Phase 13 dogfood finding: VersionBump hung on a live
    /// `devflow start --mode auto` run because the operator's global
    /// gitconfig sets `tag.gpgsign=true`).
    #[test]
    fn tag_stays_lightweight_when_gpgsign_is_forced_on() {
        let repo = init_repo();
        let root = repo.path();
        // Simulate an operator whose global config signs tags by default —
        // override the test harness's own `tag.gpgsign false` to prove
        // `tag()`'s per-invocation `-c` override wins regardless.
        git(root, &["config", "tag.gpgsign", "true"]);

        flow(root)
            .tag("v9.9.9")
            .expect("tag must not block on $EDITOR");

        let tags = crate::test_support::git_command(root)
            .args(["tag", "-l"])
            .output()
            .unwrap();
        assert!(String::from_utf8_lossy(&tags.stdout).contains("v9.9.9"));

        // Confirm it's a lightweight tag (points directly at the commit),
        // not an annotated tag object (which `cat-file -t` would report as
        // "tag" rather than "commit").
        let obj_type = crate::test_support::git_command(root)
            .args(["cat-file", "-t", "v9.9.9"])
            .output()
            .unwrap();
        assert_eq!(
            String::from_utf8_lossy(&obj_type.stdout).trim(),
            "commit",
            "tag() must stay lightweight even when tag.gpgsign=true"
        );
    }

    #[test]
    fn commit_path_stages_only_the_given_path_leaving_other_dirt_uncommitted() {
        // The property that distinguishes commit_path from commit_all
        // (17-12, Task 2b): a hook using commit_path must never sweep in
        // unrelated dirty state.
        let repo = init_repo();
        let root = repo.path();
        std::fs::write(root.join("CHANGELOG.md"), "# Changelog\n").unwrap();
        std::fs::write(root.join("unrelated.txt"), "not part of this commit\n").unwrap();

        // Stage the unrelated file BEFORE calling commit_path. An untracked
        // file is excluded by any implementation and so proves nothing; an
        // already-staged one is the real failure mode — a bare `git commit`
        // writes the whole index and would sweep it in.
        crate::test_support::git_command(root)
            .args(["add", "unrelated.txt"])
            .status()
            .unwrap();

        flow(root)
            .commit_path("CHANGELOG.md", "docs: add changelog entry")
            .expect("commit_path");

        let committed = crate::test_support::git_command(root)
            .args(["log", "-1", "--name-only", "--pretty=format:"])
            .output()
            .unwrap();
        let committed_files = String::from_utf8_lossy(&committed.stdout);
        assert!(committed_files.contains("CHANGELOG.md"));
        assert!(!committed_files.contains("unrelated.txt"));

        let status = crate::test_support::git_command(root)
            .args(["status", "--porcelain"])
            .output()
            .unwrap();
        let status = String::from_utf8_lossy(&status.stdout);
        assert!(
            status.contains("A  unrelated.txt"),
            "unrelated.txt must remain staged-but-uncommitted, got: {status}"
        );
    }

    /// `git rev-list --count HEAD`, parsed. Shared by the three tests below
    /// so a failure reports both counts instead of a bare assertion.
    fn rev_list_count(root: &Path) -> u32 {
        let output = crate::test_support::git_command(root)
            .args(["rev-list", "--count", "HEAD"])
            .output()
            .unwrap();
        assert!(output.status.success(), "git rev-list --count HEAD failed");
        String::from_utf8_lossy(&output.stdout)
            .trim()
            .parse::<u32>()
            .expect("rev-list --count HEAD must print an integer")
    }

    /// 19b/D-16: `hooks::version_bump` (hooks.rs:242) calls `commit_path` and
    /// then tags whatever commit it last produced (hooks.rs:249). If a
    /// terminal-batch retry calls `commit_path` again with byte-identical
    /// content (the file untouched since the first call), a forced commit
    /// here means the release tag can end up naming a commit that contains
    /// nothing new. This pins the exact retry scenario: two calls, unchanged
    /// content, `git rev-list --count HEAD` must not move between them.
    #[test]
    fn commit_path_twice_with_identical_content_creates_only_one_commit() {
        let repo = init_repo();
        let root = repo.path();
        std::fs::write(root.join("CHANGELOG.md"), "# Changelog\n").unwrap();

        flow(root)
            .commit_path("CHANGELOG.md", "docs: add changelog entry")
            .expect("first commit_path call");
        let n1 = rev_list_count(root);

        // The file is not touched again -- this is the retry scenario, not
        // a second genuine change.
        flow(root)
            .commit_path("CHANGELOG.md", "docs: add changelog entry")
            .expect("second commit_path call");
        let n2 = rev_list_count(root);

        assert_eq!(
            n2, n1,
            "a repeat commit_path call on unchanged content must not add a \
             commit: n1={n1}, n2={n2}"
        );
    }

    /// 19b/D-16, T-19-11: separates the "no commit" claim from the "no
    /// error" claim so a future change can't satisfy one by breaking the
    /// other. `hooks.rs` propagates `commit_path`'s `Result` with `?` at both
    /// call sites (changelog_append:225, version_bump:242) -- turning a
    /// genuine no-op into `Err` would stall the terminal hook batch (see
    /// T-19-11 in this plan's threat model), so both properties must hold
    /// simultaneously.
    #[test]
    fn commit_path_with_no_changes_returns_ok_without_committing() {
        let repo = init_repo();
        let root = repo.path();
        std::fs::write(root.join("CHANGELOG.md"), "# Changelog\n").unwrap();
        flow(root)
            .commit_path("CHANGELOG.md", "docs: add changelog entry")
            .expect("initial commit_path");
        let n1 = rev_list_count(root);

        // CHANGELOG.md is already committed and unmodified -- a single call
        // here has nothing to commit.
        let result = flow(root).commit_path("CHANGELOG.md", "docs: add changelog entry");
        let n2 = rev_list_count(root);

        assert!(
            result.is_ok(),
            "no-op call must return Ok(()), got: {result:?}"
        );
        assert_eq!(
            n2, n1,
            "no-op call must not create a commit: n1={n1}, n2={n2}"
        );
    }

    /// Edge case the fix must NOT change: `commit_path` on a path that does
    /// not exist on disk still errors at the staging step (`git add` fails
    /// on an unknown pathspec). Asserted explicitly so the fix for the
    /// no-change case above cannot be over-applied into "commit_path never
    /// fails".
    #[test]
    fn commit_path_on_nonexistent_path_still_errors() {
        let repo = init_repo();
        let root = repo.path();

        let result = flow(root).commit_path("does-not-exist.md", "docs: add changelog entry");

        assert!(
            result.is_err(),
            "commit_path on an unknown pathspec must still error, got: {result:?}"
        );
    }

    #[test]
    fn release_start_branches_from_current_head_not_develop() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // Ship from a feature branch carrying a commit that is NOT on develop.
        gf.feature_start(PhaseId::new(5)).expect("feature_start");
        commit_file(root, "feature-only.txt");
        let feature_tip = gf.branch_tip("feature/phase-05").expect("feature tip");

        let branch = gf.release_start("2.0.0").expect("release_start");
        assert_eq!(branch, "release/2.0.0");
        assert_eq!(current_branch(root), "release/2.0.0");

        // The release branch tip must descend from the feature commit — i.e.
        // the feature-only work is present, not dropped to develop's HEAD.
        let release_tip = gf.branch_tip("release/2.0.0").expect("release tip");
        let is_ancestor = crate::test_support::git_command(root)
            .args(["merge-base", "--is-ancestor", &feature_tip, &release_tip])
            .output()
            .unwrap()
            .status
            .success();
        assert!(
            is_ancestor,
            "release branch must descend from the shipped feature commit"
        );
        assert!(root.join("feature-only.txt").exists());
    }

    #[test]
    fn cleanup_merged_removes_merged_but_keeps_protected() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // Create and merge a feature branch into develop.
        gf.feature_start(PhaseId::new(2)).expect("start");
        commit_file(root, "f.txt");
        gf.feature_finish(PhaseId::new(2)).expect("finish");

        // Create an already-merged stray branch off develop.
        git(root, &["branch", "stale-merged"]);

        let deleted = gf.cleanup_merged().expect("cleanup");
        assert!(deleted.contains(&"stale-merged".to_string()));
        // Protected branches survive.
        assert!(!deleted.contains(&"develop".to_string()));
        assert!(!deleted.contains(&"main".to_string()));
    }

    /// WR-04 (13-REVIEW.md): `cleanup_merged` must compute "merged" relative
    /// to `develop` explicitly, not whatever the main checkout's current
    /// HEAD happens to be. If the main checkout is left on a divergent
    /// branch, an implicit-HEAD baseline would wrongly identify (and
    /// delete) a branch that's merged into that other branch but was never
    /// actually merged into `develop`.
    #[test]
    fn cleanup_merged_is_relative_to_develop_not_current_head() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // `topic` diverges from develop with a unique commit develop never
        // sees, then `premature` branches off `topic`'s tip — so
        // `premature` is merged into `topic` but NOT into `develop`.
        git(root, &["checkout", "-q", "-b", "topic", "develop"]);
        commit_file(root, "topic-only.txt");
        git(root, &["checkout", "-q", "-b", "premature", "topic"]);

        // Leave the main checkout on `topic` — NOT `develop` — before
        // calling cleanup_merged, mirroring an operator who forgot to
        // check out develop first. (`topic` itself is also technically
        // "merged into HEAD" under an implicit baseline since it IS HEAD,
        // which git's own `-d` correctly refuses as the checked-out branch
        // — so the call's overall Ok/Err is not itself decisive here; check
        // the actual side effect on `premature` instead.)
        git(root, &["checkout", "-q", "topic"]);

        let _ = gf.cleanup_merged();
        assert!(
            gf.branch_exists("premature"),
            "premature is merged into topic (current HEAD) but not into \
             develop — it must survive cleanup_merged when the baseline is develop"
        );
    }

    /// WR-03 (13-REVIEW.md), revised: `git branch --merged` prefixes a
    /// branch checked out in a linked worktree with `+ `. The prefix must be
    /// stripped positionally (not by trimming marker characters, which would
    /// mangle a branch legitimately named "+foo"), and a branch git refuses
    /// to delete — a worktree checkout can never be deleted, by design —
    /// must be skipped with a warning rather than aborting the sweep before
    /// the remaining merged branches.
    #[test]
    fn cleanup_merged_skips_worktree_branch_and_continues_sweep() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // Merge a branch into develop WITHOUT deleting it (feature_finish
        // deletes on merge, which would leave nothing to check out).
        git(
            root,
            &["checkout", "-q", "-b", "worktree-merged", "develop"],
        );
        commit_file(root, "g.txt");
        git(root, &["checkout", "-q", "develop"]);
        git(root, &["merge", "-q", "--no-ff", "worktree-merged"]);

        // Check the merged branch out in a linked worktree so
        // `git branch --merged` reports it with a `+ ` prefix.
        let wt_dir = tempfile::tempdir().unwrap();
        git(
            root,
            &[
                "worktree",
                "add",
                wt_dir.path().to_str().unwrap(),
                "worktree-merged",
            ],
        );

        // A second merged branch that sorts after "worktree-merged" would be
        // reached only if the sweep survives the worktree refusal; "zz-" also
        // guards against luck in iteration order via the branch before it.
        git(root, &["branch", "aa-stale"]);
        git(root, &["branch", "zz-stale"]);

        let deleted = gf
            .cleanup_merged()
            .expect("a skipped worktree branch must not abort the sweep");
        assert!(deleted.contains(&"aa-stale".to_string()));
        assert!(deleted.contains(&"zz-stale".to_string()));
        assert!(
            !deleted.contains(&"worktree-merged".to_string()),
            "worktree checkout cannot be deleted"
        );
        assert!(gf.branch_exists("worktree-merged"));
    }

    /// The delete side must agree with the `--merged develop` listing: `-d`
    /// verifies merged-into-HEAD, so with the main checkout parked on a
    /// stale branch every genuinely-merged branch was refused as "not fully
    /// merged" — in exactly the scenario WR-04 exists for.
    #[test]
    fn cleanup_merged_deletes_when_head_is_not_on_develop() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // `old` is parked before the merge below, so nothing merged later is
        // reachable from HEAD while it's checked out.
        git(root, &["checkout", "-q", "-b", "old", "develop"]);
        git(root, &["checkout", "-q", "develop"]);
        git(root, &["checkout", "-q", "-b", "merged-feature", "develop"]);
        commit_file(root, "h.txt");
        git(root, &["checkout", "-q", "develop"]);
        git(root, &["merge", "-q", "--no-ff", "merged-feature"]);
        git(root, &["checkout", "-q", "old"]);

        let deleted = gf.cleanup_merged().expect("cleanup");
        assert!(
            deleted.contains(&"merged-feature".to_string()),
            "merged-into-develop branch must be deleted even when HEAD is elsewhere: {deleted:?}"
        );
        assert!(!gf.branch_exists("merged-feature"));
    }

    #[test]
    fn delete_branch_removes_unmerged_with_force_and_protects_trunk() {
        let repo = init_repo();
        let root = repo.path();
        let gf = flow(root);

        // Create a feature branch with an unmerged commit.
        gf.feature_start(PhaseId::new(8)).expect("start");
        commit_file(root, "unmerged.txt");
        // Switch back to develop so the branch isn't checked out.
        git(root, &["checkout", "-q", "develop"]);

        // -d would refuse (unmerged); force deletes it.
        assert!(gf.delete_branch("feature/phase-08", false).is_err());
        gf.delete_branch("feature/phase-08", true)
            .expect("force delete");
        let branches = crate::test_support::git_command(root)
            .args(["branch"])
            .output()
            .unwrap();
        assert!(!String::from_utf8_lossy(&branches.stdout).contains("feature/phase-08"));

        // Protected branches are never deleted.
        assert!(gf.delete_branch("develop", true).is_err());
        assert!(gf.delete_branch("main", true).is_err());
    }

    #[test]
    fn merge_of_missing_branch_is_an_error() {
        let repo = init_repo();
        let root = repo.path();
        // feature_finish for a phase that was never started: checkout develop
        // succeeds, but merging the nonexistent feature branch fails.
        let err = flow(root).feature_finish(PhaseId::new(99)).unwrap_err();
        assert!(matches!(err, GitError::Command(_)));
    }

    // -----------------------------------------------------------------
    // 20d: publish-order helpers (pure, no I/O)
    // -----------------------------------------------------------------

    #[test]
    fn workspace_member_paths_parses_multiline_array() {
        let contents = "[workspace]\nresolver = \"2\"\nmembers = [\n    \"crates/devflow-core\",\n    \"crates/devflow-cli\",\n]\n";
        assert_eq!(
            workspace_member_paths(contents),
            vec![
                "crates/devflow-core".to_string(),
                "crates/devflow-cli".to_string()
            ]
        );
    }

    #[test]
    fn package_name_reads_the_package_section() {
        let contents = "[package]\nname = \"devflow-core\"\nversion.workspace = true\n";
        assert_eq!(package_name(contents), Some("devflow-core".to_string()));
    }

    #[test]
    fn member_depends_on_matches_dotted_workspace_shorthand() {
        let contents = "[package]\nname = \"devflow\"\n\n[dependencies]\ndevflow-core.workspace = true\nclap.workspace = true\n";
        assert!(member_depends_on(contents, "devflow-core"));
        assert!(!member_depends_on(contents, "serde"));
    }

    /// WR-03 (phase 20 review): the equally-valid expanded long-form TOML
    /// section syntax (`[dependencies.NAME]`) parses to a section header of
    /// `"dependencies.NAME"`, never equal to the plain `"dependencies"` the
    /// inline-table branch checks against — this must still be recognized
    /// as a dependency edge.
    #[test]
    fn member_depends_on_matches_long_form_dependency_section() {
        let contents = "[package]\nname = \"devflow\"\n\n[dependencies.devflow-core]\nworkspace = true\n\n[dependencies.clap]\nversion = \"4\"\n";
        assert!(member_depends_on(contents, "devflow-core"));
        assert!(member_depends_on(contents, "clap"));
        assert!(!member_depends_on(contents, "serde"));
    }

    #[test]
    fn topo_sort_orders_dependency_before_dependent() {
        let names = vec!["devflow".to_string(), "devflow-core".to_string()];
        let edges = vec![("devflow".to_string(), "devflow-core".to_string())];
        assert_eq!(
            topo_sort(names, edges),
            vec!["devflow-core".to_string(), "devflow".to_string()]
        );
    }

    #[test]
    fn topo_sort_falls_back_to_input_order_on_a_cycle() {
        // A genuine cyclic dependency would already fail `cargo build`
        // long before this check runs — this just proves no infinite loop.
        let names = vec!["a".to_string(), "b".to_string()];
        let edges = vec![
            ("a".to_string(), "b".to_string()),
            ("b".to_string(), "a".to_string()),
        ];
        let result = topo_sort(names, edges);
        assert_eq!(result.len(), 2);
    }

    #[test]
    fn publish_order_derives_core_before_cli_from_a_fixture_workspace() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        std::fs::write(
            root.join("Cargo.toml"),
            "[workspace]\nmembers = [\n    \"crates/devflow-core\",\n    \"crates/devflow-cli\",\n]\n",
        )
        .unwrap();
        std::fs::create_dir_all(root.join("crates/devflow-core")).unwrap();
        std::fs::write(
            root.join("crates/devflow-core/Cargo.toml"),
            "[package]\nname = \"devflow-core\"\n\n[dependencies]\n",
        )
        .unwrap();
        std::fs::create_dir_all(root.join("crates/devflow-cli")).unwrap();
        std::fs::write(
            root.join("crates/devflow-cli/Cargo.toml"),
            "[package]\nname = \"devflow\"\n\n[dependencies]\ndevflow-core.workspace = true\n",
        )
        .unwrap();

        assert_eq!(
            publish_order(root),
            vec!["devflow-core".to_string(), "devflow".to_string()]
        );
    }

    /// WR-03 (phase 20 review): a workspace member manifest written with
    /// the long-form `[dependencies.devflow-core]` section (rather than the
    /// inline `[dependencies]\ndevflow-core.workspace = true` form) must
    /// still contribute its dependency edge to `publish_order`'s topo-sort
    /// — the release-safety-critical crates.io publish order this
    /// self-pin regression would otherwise silently get wrong.
    #[test]
    fn publish_order_recognizes_long_form_dependency_section_self_dependency() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        std::fs::write(
            root.join("Cargo.toml"),
            "[workspace]\nmembers = [\n    \"crates/devflow-core\",\n    \"crates/devflow-cli\",\n]\n",
        )
        .unwrap();
        std::fs::create_dir_all(root.join("crates/devflow-core")).unwrap();
        std::fs::write(
            root.join("crates/devflow-core/Cargo.toml"),
            "[package]\nname = \"devflow-core\"\n\n[dependencies]\n",
        )
        .unwrap();
        std::fs::create_dir_all(root.join("crates/devflow-cli")).unwrap();
        std::fs::write(
            root.join("crates/devflow-cli/Cargo.toml"),
            "[package]\nname = \"devflow\"\n\n[dependencies.devflow-core]\nworkspace = true\n",
        )
        .unwrap();

        assert_eq!(
            publish_order(root),
            vec!["devflow-core".to_string(), "devflow".to_string()],
            "the long-form dependency section must still order devflow-core before devflow"
        );
    }

    // -----------------------------------------------------------------
    // 20d: origin/main ancestor check (no fetch)
    // -----------------------------------------------------------------

    #[test]
    fn origin_main_ancestor_status_is_ref_absent_without_a_remote() {
        let repo = init_repo();
        let root = repo.path();
        assert_eq!(origin_main_ancestor_status(root), AncestorStatus::RefAbsent);
    }

    #[test]
    fn origin_main_ancestor_status_is_ancestor_when_head_is_up_to_date() {
        let repo = init_repo();
        let root = repo.path();
        let head = crate::test_support::git_command(root)
            .args(["rev-parse", "HEAD"])
            .output()
            .unwrap();
        let head_sha = String::from_utf8_lossy(&head.stdout).trim().to_string();
        git(root, &["update-ref", "refs/remotes/origin/main", &head_sha]);
        assert_eq!(origin_main_ancestor_status(root), AncestorStatus::Ancestor);
    }

    // -----------------------------------------------------------------
    // 27-01 (D-03): the scrubbing constructor holds under a hostile GIT_DIR
    // -----------------------------------------------------------------

    /// D-03: a real spawned `git` process built through the constructor
    /// resolves the caller-supplied root even when `GIT_DIR` points at an
    /// unrelated repository — proven by a subprocess test, not by
    /// inspecting the `Command` object alone.
    #[test]
    fn hermetic_command_resolves_caller_root_even_under_a_hostile_git_dir() {
        let real_repo = init_repo();
        let real_root = real_repo.path();

        let foreign_repo = TempDir::new().unwrap();
        git(foreign_repo.path(), &["init", "-q"]);

        let output = git_command(real_root)
            .args(["rev-parse", "--show-toplevel"])
            // Hostile injection chained AFTER the constructor — the
            // strongest form of the claim: `--show-toplevel` must still
            // resolve `real_root`, not `foreign_repo`.
            .env("GIT_DIR", foreign_repo.path().join(".git"))
            .output()
            .expect("spawn git");
        assert!(
            output.status.success(),
            "rev-parse --show-toplevel failed: {}",
            String::from_utf8_lossy(&output.stderr)
        );

        let resolved = std::fs::canonicalize(String::from_utf8_lossy(&output.stdout).trim())
            .expect("canonicalize resolved toplevel");
        let expected = std::fs::canonicalize(real_root).expect("canonicalize real_root");
        assert_eq!(
            resolved, expected,
            "hermetic_command must resolve real_root even with a foreign GIT_DIR set"
        );
    }

    /// D-03: `origin_main_ancestor_status` produces the correct answer
    /// under a hostile `GIT_DIR` where it previously did not. Setting a
    /// process-global env var is forbidden (Rust 2024 `unsafe`, unsound
    /// under threaded tests — Phase 25 D-14), so this proves the property
    /// the way the constructor guarantees it, in two parts: (a) the
    /// `Command` this code path builds via `git_command` is
    /// unconditionally scrubbed — no bypass parameter, no env-var check,
    /// no config lookup (D-01), asserted directly on the built `Command`;
    /// (b) the actual mechanism `origin_main_ancestor_status` now depends
    /// on — scrubbed, with nothing in production code re-adding `GIT_DIR`
    /// afterward — reaches the correct answer for a real spawn. (A literal
    /// unscrubbed `Command::new("git")` reproduction chaining a hostile
    /// `.env("GIT_DIR", foreign)` on top was deliberately NOT added here:
    /// verified empirically against this machine's git 2.55.0 that doing
    /// so genuinely redirects `merge-base --is-ancestor`'s ref resolution
    /// to the foreign repo — unlike `--show-toplevel` above, which falls
    /// back to cwd when `GIT_WORK_TREE` is unset — so re-adding it here
    /// would both prove nothing new beyond (a) and inflate git.rs's
    /// unscrubbed-call-site count past the 7 sites this task deliberately
    /// leaves for 27-02.)
    #[test]
    fn origin_main_ancestor_status_holds_under_a_hostile_git_dir() {
        let repo = init_repo();
        let root = repo.path();
        let head = crate::test_support::git_command(root)
            .args(["rev-parse", "HEAD"])
            .output()
            .unwrap();
        let head_sha = String::from_utf8_lossy(&head.stdout).trim().to_string();
        git(root, &["update-ref", "refs/remotes/origin/main", &head_sha]);

        // (a) unconditionally scrubbed.
        let cmd = git_command(root);
        assert!(
            cmd.get_envs()
                .any(|(key, value)| key == "GIT_DIR" && value.is_none()),
            "origin_main_ancestor_status's own Command must mark GIT_DIR for removal"
        );

        // (b) the actual, scrubbed mechanism reaches the correct answer.
        assert_eq!(origin_main_ancestor_status(root), AncestorStatus::Ancestor);
    }

    // -----------------------------------------------------------------
    // 27-01: hermetic git command construction (moved from test_support,
    // now the canonical, always-compiled home — 999.37/999.39/27-01)
    // -----------------------------------------------------------------

    /// The contract callers depend on, asserted on the built command rather
    /// than inferred: every redirecting variable is marked for removal.
    #[test]
    fn git_command_marks_every_redirecting_var_for_removal() {
        let cmd = git_command(Path::new("/tmp"));
        let removed: Vec<&str> = cmd
            .get_envs()
            .filter(|(_, value)| value.is_none())
            .filter_map(|(key, _)| key.to_str())
            .collect();

        for var in REPO_LOCAL_GIT_VARS.iter().chain(ALSO_REDIRECTING_GIT_VARS) {
            assert!(
                removed.contains(var),
                "{var} is not cleared by git_command — a fixture inheriting it \
                 would operate on that repository instead of its tempdir"
            );
        }
    }

    /// GIT_EXEC_PATH must survive: clearing it can break git's own helper
    /// lookup on installations that rely on it, and it cannot redirect
    /// repository resolution.
    #[test]
    fn git_command_preserves_git_exec_path() {
        let cmd = git_command(Path::new("/tmp"));
        assert!(
            !cmd.get_envs()
                .any(|(key, value)| key == "GIT_EXEC_PATH" && value.is_none()),
            "GIT_EXEC_PATH must not be cleared"
        );
    }

    /// Guards the hard-coded list against a git upgrade that adds a
    /// repository-local variable. If this fails, add the new name to
    /// `REPO_LOCAL_GIT_VARS` — do not delete the assertion.
    #[test]
    fn local_env_vars_match_git() {
        let output = git_command(Path::new("/tmp"))
            .args(["rev-parse", "--local-env-vars"])
            .output()
            .expect("run `git rev-parse --local-env-vars`");
        assert!(
            output.status.success(),
            "`git rev-parse --local-env-vars` failed"
        );

        let mut from_git: Vec<String> = String::from_utf8_lossy(&output.stdout)
            .lines()
            .map(str::trim)
            .filter(|line| !line.is_empty())
            .map(str::to_string)
            .collect();
        let mut ours: Vec<String> = REPO_LOCAL_GIT_VARS
            .iter()
            .map(|v| (*v).to_string())
            .collect();
        from_git.sort();
        ours.sort();

        assert_eq!(
            ours, from_git,
            "REPO_LOCAL_GIT_VARS has drifted from `git rev-parse --local-env-vars`"
        );
    }

    // -----------------------------------------------------------------
    // 20d: signing-viability helpers
    // -----------------------------------------------------------------

    /// Guards tests that temporarily override the process-global `HOME`
    /// env var (same idiom as `config.rs`'s test-local `ENV_MUTEX`) — this
    /// project's own dev machine sets `gpg.format=ssh` / `user.signingkey`
    /// GLOBALLY (the exact Pattern 4 research finding), so a hermetic test
    /// of the "unset" branch must isolate `$HOME/.gitconfig`, not just the
    /// repo-local config.
    static HOME_ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());

    #[test]
    fn check_signing_viability_degrades_when_gpg_format_unset_and_no_signingkey() {
        // 20d/empty: no gpg.format, no user.signingkey — must degrade to an
        // actionable message, never panic.
        let _lock = HOME_ENV_MUTEX.lock().unwrap();
        let repo = init_repo();
        let root = repo.path();
        let fake_home = tempfile::tempdir().unwrap();
        let original_home = std::env::var_os("HOME");
        // SAFETY: serialized under HOME_ENV_MUTEX; restored below before
        // the guard drops.
        unsafe { std::env::set_var("HOME", fake_home.path()) };

        let result = check_signing_viability(root);

        // SAFETY: still serialized under HOME_ENV_MUTEX.
        match original_home {
            Some(home) => unsafe { std::env::set_var("HOME", home) },
            None => unsafe { std::env::remove_var("HOME") },
        }

        match result {
            SigningViability::Unknown { reason } => {
                assert!(
                    reason.contains("user.signingkey"),
                    "unexpected reason: {reason}"
                );
            }
            other => panic!("expected Unknown (fail-soft), got: {other:?}"),
        }
    }

    /// D-01/D-02/D-10: an inline `user.signingkey` value — either the
    /// `key::`-prefixed form or the raw deprecated `ssh-` compat form — must
    /// never be classified as a missing filesystem path. Git never stats an
    /// inline value, so this must never return the missing-key-file
    /// `NotViable`.
    ///
    /// This test deliberately keeps its narrow assertion — only that the
    /// missing-file reason is absent. It used to be narrow because the
    /// outcome depended on the host's ssh-agent state (D-10); it is narrow
    /// now because that is the single property it exists to guard, and
    /// `inline_signing_key_returns_unknown_without_probing` pins the exact
    /// arm. Leaving the assertion here narrow keeps one falsifier per test.
    #[test]
    fn check_signing_viability_never_reports_key_file_missing_for_inline_key() {
        const MISSING_FILE_REASON: &str = "user.signingkey is set but the key file does not exist";
        let inline_values = [
            "key::ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFAKEFIXTUREKEYMATERIALZZZZZZZZZZZZZZZZZZZZZZ devflow-fixture",
            "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFAKEFIXTUREKEYMATERIALZZZZZZZZZZZZZZZZZZZZZZ devflow-fixture",
        ];
        for value in inline_values {
            let repo = init_repo();
            let root = repo.path();
            git(root, &["config", "gpg.format", "ssh"]);
            git(root, &["config", "user.signingkey", value]);

            let result = check_signing_viability(root);

            if let SigningViability::NotViable { reason } = &result {
                assert_ne!(
                    reason, MISSING_FILE_REASON,
                    "inline signingkey value {value:?} incorrectly classified as a \
                     missing file: {result:?}"
                );
            }
        }
    }

    /// D-01/D-02/D-03: a flat table over the pure classifier proving git's
    /// own prefix precedence — `key::` strip first, then the raw `ssh-`
    /// compat form, else a path. Non-`ssh-` algorithms (`ecdsa-`, `sk-`)
    /// reach the inline branch ONLY through `key::` (D-03) — a bare form of
    /// either is a path, matching git.
    #[test]
    fn inline_signing_key_blob_follows_git_prefix_precedence() {
        assert_eq!(
            inline_signing_key_blob("key::ssh-rsa AAAAB3 id"),
            Some("ssh-rsa AAAAB3 id")
        );
        assert_eq!(
            inline_signing_key_blob("key::ssh-ed25519 AAAAC3 id"),
            Some("ssh-ed25519 AAAAC3 id")
        );
        assert_eq!(
            inline_signing_key_blob("key::ecdsa-sha2-nistp256 AAAAE2 id"),
            Some("ecdsa-sha2-nistp256 AAAAE2 id")
        );
        assert_eq!(inline_signing_key_blob("key::"), Some(""));
        assert_eq!(
            inline_signing_key_blob("ssh-ed25519 AAAAC3 id"),
            Some("ssh-ed25519 AAAAC3 id")
        );
        assert_eq!(
            inline_signing_key_blob("  key::ssh-ed25519 AAAAC3 id  "),
            Some("ssh-ed25519 AAAAC3 id")
        );
        // D-02: a value that plausibly names an existing file is STILL
        // inline, because the classifier never stats it.
        assert_eq!(inline_signing_key_blob("ssh-key.pub"), Some("ssh-key.pub"));
        assert_eq!(
            inline_signing_key_blob("/home/operator/.ssh/id_ed25519.pub"),
            None
        );
        // D-03: bare, no `key::` prefix, so git treats these as paths and so
        // must DevFlow.
        assert_eq!(
            inline_signing_key_blob("ecdsa-sha2-nistp256 AAAAE2 id"),
            None
        );
        assert_eq!(
            inline_signing_key_blob("sk-ssh-ed25519@openssh.com AAAAG id"),
            None
        );
        assert_eq!(inline_signing_key_blob("ABCD1234"), None);
    }

    /// D-03/D-12: values that neither start with `key::` nor `ssh-` still
    /// take the path branch and keep today's byte-for-byte behavior — the
    /// early `.exists()` return, which still fires before anything is
    /// spawned. What follows that early return is now the signing probe
    /// rather than the deleted `ssh-add` predictor; the guarantee under
    /// test is that a missing file is answered without reaching it at all.
    /// This is the D-03 falsifier: bare `ecdsa-`/`sk-` forms must NOT be
    /// treated as inline.
    #[test]
    fn check_signing_viability_still_reports_missing_file_for_a_path_value() {
        const MISSING_FILE_REASON: &str = "user.signingkey is set but the key file does not exist";
        let path_values = [
            "/nonexistent/path/to/a/signing/key/that/does/not/exist",
            "ecdsa-sha2-nistp256 AAAAE2 devflow-fixture",
            "sk-ssh-ed25519@openssh.com AAAAG devflow-fixture",
        ];
        for value in path_values {
            let repo = init_repo();
            let root = repo.path();
            git(root, &["config", "gpg.format", "ssh"]);
            git(root, &["config", "user.signingkey", value]);

            let result = check_signing_viability(root);

            assert_eq!(
                result,
                SigningViability::NotViable {
                    reason: MISSING_FILE_REASON.to_string(),
                },
                "value {value:?} did not take the path branch: {result:?}"
            );
        }
    }

    /// D-06: every inline-branch failure mode must degrade to `Unknown`,
    /// never a NEW hard fail introduced by this phase. That guarantee is
    /// what this test preserves; only the reasons it accepts changed.
    ///
    /// It used to pin a two-reason set built from the predictor's no-agent
    /// and agent-empty strings, neither of which the code can produce any
    /// more — a test that referenced the deleted mechanism through its
    /// output rather than its symbols, so nothing but a run would have
    /// caught it. Under D-03 an unparseable inline value classifies as
    /// inline and returns the single fixed inline reason without being
    /// probed at all.
    ///
    /// Its agent-independence note survives, now true by construction
    /// rather than by argument: these values never reach a probe, so no
    /// host's agent state can reach this result.
    #[test]
    fn check_signing_viability_never_hard_fails_on_an_unparseable_inline_key() {
        const INLINE_REASON: &str =
            "cannot verify signing viability — an inline user.signingkey is not probed";
        let unparseable_values = ["key::", "key::this is not a key at all"];
        for value in unparseable_values {
            let repo = init_repo();
            let root = repo.path();
            git(root, &["config", "gpg.format", "ssh"]);
            git(root, &["config", "user.signingkey", value]);

            let result = check_signing_viability(root);

            assert_eq!(
                result,
                SigningViability::Unknown {
                    reason: INLINE_REASON.into(),
                },
                "value {value:?} produced an unexpected hard fail: {result:?}"
            );
        }
    }

    // -----------------------------------------------------------------
    // 35-03: the `ssh-keygen -Y sign` probe
    // -----------------------------------------------------------------

    /// F-8: the probe workspace name must be unique per CALL, not per
    /// process. `cargo test` runs tests as parallel THREADS inside a single
    /// process, so a name derived from the process id is shared by every
    /// concurrent probe: two probes collide, the loser's non-recursive
    /// `create_dir` fails with an already-exists error, and it fails soft to
    /// `Unknown` — a flaky test whose failure points at the probe rather
    /// than at the harness.
    ///
    /// The two-thread half is load-bearing. The single-thread half alone
    /// passes against a name built from the process id plus a thread id,
    /// which is the near-miss fix this test exists to reject.
    #[test]
    fn probe_workspace_name_is_unique_per_call() {
        let first = probe_workspace_name();
        let second = probe_workspace_name();
        assert_ne!(
            first, second,
            "two successive calls on one thread produced the same probe workspace name"
        );

        const PER_THREAD: usize = 64;
        let handles: Vec<_> = (0..2)
            .map(|_| {
                std::thread::spawn(|| {
                    (0..PER_THREAD)
                        .map(|_| probe_workspace_name())
                        .collect::<Vec<_>>()
                })
            })
            .collect();
        let mut names: Vec<String> = handles
            .into_iter()
            .flat_map(|handle| handle.join().expect("probe-name thread panicked"))
            .collect();
        let total = names.len();
        assert_eq!(total, 2 * PER_THREAD, "fixture did not produce every name");
        names.sort();
        names.dedup();
        assert_eq!(
            names.len(),
            total,
            "two concurrently spawned threads produced duplicate probe workspace names"
        );
    }

    /// WR-07 (35-REVIEW): "creates a **private** workspace" must be
    /// implemented, not merely claimed. `std::fs::create_dir` applies
    /// `0o777 & !umask` — typically 0o755 — inside a shared
    /// `std::env::temp_dir()`, leaving the directory world-readable and
    /// world-traversable. Nothing secret lands in it, so this was never an
    /// exposure of key material; the hazard is a future author extending the
    /// probe on the strength of the comment.
    ///
    /// **The umask is neutralized, and that is the whole measurement.** Asserted
    /// naively this test is VACUOUS on any host whose umask is already 0o077 —
    /// `0o777 & !0o077` is 0o700, so a plain `std::fs::create_dir` produces the
    /// expected mode and the assertion passes against the unfixed code. That was
    /// observed here, not reasoned about: the first version of this test passed
    /// with the `DirBuilderExt::mode` call removed. Setting the umask to 0 makes
    /// a plain creation 0o777, so the two really do differ, and the sibling
    /// created that way is the negative control.
    ///
    /// The window spans two `create_dir` calls with no I/O between them. A
    /// concurrent test creating a file inside it would get a laxer mode than
    /// usual — every such file is a tempdir artifact in a test process, so there
    /// is no consequence beyond the mode bits themselves.
    ///
    /// The non-recursive refusal is asserted here too: `create_dir_all` would
    /// accept a pre-planted directory or symlink silently and redirect where the
    /// payload is written (T-35-12).
    #[test]
    fn the_probe_workspace_is_owner_only_and_refuses_an_existing_path() {
        use std::os::unix::fs::PermissionsExt;

        let dir = tempfile::tempdir().unwrap();
        let workspace = dir.path().join("probe");
        let plain = dir.path().join("plain");

        // SAFETY: `umask` is a plain syscall with no preconditions. Restored
        // immediately below, before any assertion can unwind past it.
        let previous_umask = unsafe { libc::umask(0) };
        let created = create_probe_workspace(&workspace);
        let plain_created = std::fs::create_dir(&plain).is_ok();
        // SAFETY: restoring the value the call above returned.
        unsafe {
            libc::umask(previous_umask);
        }

        assert!(created, "the fixture needs the creation to succeed");
        assert!(plain_created, "the fixture needs the control to be created");

        let control = std::fs::metadata(&plain).unwrap().permissions().mode() & 0o777;
        assert_eq!(
            control, 0o777,
            "NEGATIVE CONTROL: with the umask neutralized a default creation must be wide \
             open. If it is not, the umask window did not take and the assertion below \
             cannot distinguish the fix from the default"
        );

        let mode = std::fs::metadata(&workspace).unwrap().permissions().mode() & 0o777;
        assert_eq!(
            mode, 0o700,
            "the workspace must be owner-only by request, not by whatever the umask happened \
             to strip"
        );

        // The path now exists, so the same call must refuse it.
        assert!(
            !create_probe_workspace(&workspace),
            "a pre-planted directory or symlink must not be adopted — that is how a payload \
             gets written somewhere the probe did not choose"
        );
    }

    /// WR-07's second half: "removes the workspace on **every** exit path" was
    /// a plain statement after the call, which an unwind skips. Repeated over
    /// many `release --check` runs on a long-lived host that is unbounded
    /// accumulation of `devflow-sign-probe-*` directories in `/tmp`.
    ///
    /// Driven through a real `catch_unwind` rather than by calling `drop`:
    /// dropping the guard by hand proves only that `Drop` removes a directory,
    /// which was never in question. The claim is that the removal survives a
    /// panic, and only an unwind establishes that.
    ///
    /// The control is the directory's existence before the panic — without it a
    /// test that never created anything would pass.
    ///
    /// # What this does NOT establish
    ///
    /// Its subject is [`ProbeWorkspace`], not [`run_ssh_sign_probe`]. Measured,
    /// not assumed: reverting `run_ssh_sign_probe` to the trailing
    /// `remove_dir_all` statement leaves this test PASSING. Nothing here can
    /// panic on demand inside `sign_probe_within`, so the link from the
    /// production function to the guard rests on there being exactly one
    /// construction site, checked by reading. A future refactor that stops
    /// binding the guard would not be caught here.
    #[test]
    fn the_probe_workspace_guard_removes_its_directory_on_unwind() {
        let dir = tempfile::tempdir().unwrap();
        let workspace = dir.path().join("probe");
        assert!(create_probe_workspace(&workspace));
        assert!(
            workspace.exists(),
            "premise: the directory must exist before the panic, or its later absence \
             establishes nothing"
        );

        let panicked = std::panic::catch_unwind({
            let workspace = workspace.clone();
            move || {
                let _cleanup = ProbeWorkspace(workspace);
                panic!("the probe panicked mid-flight");
            }
        })
        .is_err();

        assert!(panicked, "the fixture must actually unwind");
        assert!(
            !workspace.exists(),
            "a panic inside the probe must not leak its workspace into the shared temp dir"
        );
    }

    /// WR-01 (35-REVIEW): a probe timeout is a measurement failure, so it
    /// must land on `Unknown`/`warn` beside the other two non-verdicts — not
    /// on a hard `NotViable`, which asserts something about the key and
    /// attaches `release --check`'s "resolve before attempting the signed
    /// release tag" hint to a key that may sign perfectly well.
    ///
    /// `Rejected` is the NC-4 negative control and is checked in the same
    /// function on purpose here: it is the one outcome that genuinely IS
    /// evidence about the key, so a mapping that returned `Unknown`
    /// unconditionally — the obvious over-correction — fails on it. If both
    /// halves agreed, this test would be measuring nothing.
    ///
    /// Asserted on the classification rather than by wedging a real
    /// `ssh-keygen` for ten seconds: the defect was in the mapping, and a
    /// wall-clock probe would make this a slow test of the timeout mechanism
    /// (which `SSH_SIGN_PROBE_TIMEOUT`'s own tests already cover) instead of
    /// a fast test of the verdict.
    #[test]
    fn a_probe_timeout_is_unknown_while_a_rejection_stays_not_viable() {
        // Never read: no arm below reaches `public_key_fingerprint`.
        let unused_key = Path::new("/nonexistent/devflow-wr01");

        let timed_out = sign_probe_verdict(SignProbeOutcome::TimedOut, unused_key);
        match &timed_out {
            SigningViability::Unknown { reason } => assert!(
                reason.starts_with("cannot verify signing viability — "),
                "a non-verdict must carry the file's fail-soft prefix, got: {reason:?}"
            ),
            other => panic!(
                "a timeout establishes nothing about the key and must not be a hard \
                 verdict, got: {other:?}"
            ),
        }

        let rejected = sign_probe_verdict(SignProbeOutcome::Rejected, unused_key);
        assert!(
            matches!(rejected, SigningViability::NotViable { .. }),
            "NEGATIVE CONTROL: a key that ran the probe and could not sign IS evidence \
             about the key and must stay a hard verdict, got: {rejected:?}"
        );

        // The other two fail-soft classes, pinned in the same place so the
        // three "could not establish anything" outcomes cannot drift apart
        // again.
        for outcome in [SignProbeOutcome::ToolMissing, SignProbeOutcome::NotRun] {
            assert!(
                matches!(
                    sign_probe_verdict(outcome, unused_key),
                    SigningViability::Unknown { .. }
                ),
                "every measurement failure maps to Unknown"
            );
        }
    }

    /// Generate a real ed25519 keypair at `stem`, with `passphrase` (empty
    /// for an unencrypted key). Returns the public half's path.
    fn generate_keypair(stem: &Path, passphrase: &str) -> PathBuf {
        let keygen = Command::new("ssh-keygen")
            .args([
                "-t",
                "ed25519",
                "-f",
                stem.to_str().unwrap(),
                "-N",
                passphrase,
                "-q",
            ])
            .output()
            .expect("spawn ssh-keygen");
        assert!(
            keygen.status.success(),
            "ssh-keygen fixture setup failed: {}",
            String::from_utf8_lossy(&keygen.stderr)
        );
        let pub_path = stem.with_extension("pub");
        assert!(pub_path.exists(), "ssh-keygen wrote no public key");
        pub_path
    }

    /// Point a repo's `user.signingkey` at `key` under `gpg.format=ssh`.
    fn configure_ssh_signing(root: &Path, key: &Path) {
        git(root, &["config", "gpg.format", "ssh"]);
        git(root, &["config", "user.signingkey", key.to_str().unwrap()]);
    }

    /// D-08's redaction contract, asserted on the rendered result: neither
    /// the reason nor the fingerprint may carry the configured key path, any
    /// private key material, or any fragment of `ssh-keygen`'s own stderr
    /// (which embeds the path verbatim — see the probe's own comment).
    fn assert_no_leak(result: &SigningViability, secret_dir: &Path) {
        let rendered = format!("{result:?}");
        assert!(
            !rendered.contains(secret_dir.to_str().unwrap()),
            "signing viability leaked a filesystem path: {rendered}"
        );
        for fragment in [
            "PRIVATE KEY",
            "No private key found",
            "Couldn't load public key",
            "Enter passphrase",
            "incorrect passphrase",
        ] {
            assert!(
                !rendered.contains(fragment),
                "signing viability leaked key material or ssh-keygen stderr ({fragment:?}): \
                 {rendered}"
            );
        }
    }

    /// The headline case, and the exact live false negative 999.86 was filed
    /// for twice: a configured signing key whose unencrypted private sibling
    /// is on disk signs fine with NO agent involvement at all. The predictor
    /// this replaced asked `ssh-add -l` whether the agent held the key and
    /// reported `NotViable` when it did not — agent membership is simply not
    /// a necessary condition for `git tag -s` to succeed.
    ///
    /// This test therefore reads, sets and depends on NO agent state. The
    /// fixture key is generated fresh into a temporary directory, so no
    /// agent on any host can be holding it; a `Viable` verdict here is only
    /// reachable through the on-disk private key.
    #[test]
    fn ssh_signing_probe_reports_viable_with_on_disk_private_key() {
        let repo = init_repo();
        let root = repo.path();
        let keys = tempfile::tempdir().unwrap();
        let pub_key = generate_keypair(&keys.path().join("probe-key"), "");
        configure_ssh_signing(root, &pub_key);

        let result = check_signing_viability(root);

        match &result {
            SigningViability::Viable { fingerprint } => {
                let fingerprint = fingerprint
                    .as_deref()
                    .expect("Viable must carry the public key fingerprint");
                assert!(
                    fingerprint.starts_with("SHA256:"),
                    "unexpected fingerprint shape: {fingerprint}"
                );
            }
            other => panic!("expected Viable for an on-disk private key, got: {other:?}"),
        }
        assert_no_leak(&result, keys.path());
    }

    /// NC-9, the negative control for the test above. Same fixture with the
    /// private half deleted, leaving only the `.pub` file: the verdict must
    /// FLIP to `NotViable`. Without this the positive assertion is vacuously
    /// true — a probe that returned `Viable` unconditionally would pass the
    /// positive case and fail here.
    #[test]
    fn ssh_signing_probe_reports_not_viable_without_a_private_key() {
        let repo = init_repo();
        let root = repo.path();
        let keys = tempfile::tempdir().unwrap();
        let stem = keys.path().join("probe-key");
        let pub_key = generate_keypair(&stem, "");
        std::fs::remove_file(&stem).expect("remove the private half");
        assert!(!stem.exists(), "fixture still has a private key");
        configure_ssh_signing(root, &pub_key);

        let result = check_signing_viability(root);

        assert_eq!(
            result,
            SigningViability::NotViable {
                reason: "the configured signing key could not sign a test payload".into(),
            },
            "expected the verdict to flip without a private key, got: {result:?}"
        );
        assert_no_leak(&result, keys.path());
    }

    /// Build one raw `ssh-keygen -Y sign` invocation for NC-10.
    ///
    /// Deliberately drives the raw command rather than the probe: the
    /// observation must be of `SSH_ASKPASS_REQUIRE`'s effect, and a run
    /// through the probe would have its duration pinned by
    /// [`SSH_SIGN_PROBE_TIMEOUT`] instead — measuring the constant, not the
    /// variable.
    fn askpass_arm(dir: &Path, askpass_require: Option<&str>) -> std::process::Child {
        use std::os::unix::process::CommandExt;

        let payload = dir.join(probe_workspace_name());
        std::fs::write(&payload, b"nc-10 payload\n").expect("write nc-10 payload");

        let mut command = Command::new("ssh-keygen");
        command
            .args([
                "-Y",
                "sign",
                "-n",
                SSH_SIGN_NAMESPACE,
                "-f",
                dir.join("encrypted-key.pub").to_str().unwrap(),
                payload.to_str().unwrap(),
            ])
            .env("SSH_ASKPASS", dir.join("askpass.sh"))
            // `read_passphrase` only consults SSH_ASKPASS when DISPLAY or
            // SSH_ASKPASS is set; both arms get the same askpass route so
            // the ONLY difference between them is the variable under test.
            .env("DISPLAY", ":0")
            // Agent state is deliberately neither read nor cleared here.
            // The fixture key is generated microseconds earlier into a fresh
            // temporary directory, so no agent on any host can hold it, and
            // a test that reaches for agent state near a signing assertion
            // has reproduced the very premise that produced 999.86.
            .stdin(Stdio::null())
            .stdout(Stdio::null())
            .stderr(Stdio::null());
        match askpass_require {
            Some(value) => command.env("SSH_ASKPASS_REQUIRE", value),
            None => command.env_remove("SSH_ASKPASS_REQUIRE"),
        };

        // SAFETY: `setsid` is a bare syscall and async-signal-safe, which is
        // the only requirement `pre_exec` imposes.
        //
        // This is load-bearing, not hygiene. With a controlling terminal
        // available, `ssh-keygen` prompts for the passphrase on `/dev/tty`
        // regardless of SSH_ASKPASS_REQUIRE, so BOTH arms would block and
        // the control would agree with its positive case for a reason that
        // has nothing to do with the variable. Dropping the terminal forces
        // the askpass route, which is the route the variable governs. It
        // also stops a killed child from leaving an operator's terminal
        // with echo disabled.
        unsafe {
            command.pre_exec(|| {
                if libc::setsid() == -1 {
                    return Err(std::io::Error::last_os_error());
                }
                Ok(())
            });
        }
        command.spawn().expect("spawn ssh-keygen for NC-10")
    }

    /// Write the NC-10 fixture: an encrypted ed25519 key and an askpass
    /// helper that takes far longer than any observation window here.
    fn encrypted_key_fixture() -> tempfile::TempDir {
        let dir = tempfile::tempdir().unwrap();
        generate_keypair(&dir.path().join("encrypted-key"), "devflow-nc10-passphrase");
        let askpass = dir.path().join("askpass.sh");
        std::fs::write(
            &askpass,
            "#!/bin/sh\nsleep 5\necho devflow-nc10-passphrase\n",
        )
        .unwrap();
        let mut perms = std::fs::metadata(&askpass).unwrap().permissions();
        std::os::unix::fs::PermissionsExt::set_mode(&mut perms, 0o755);
        std::fs::set_permissions(&askpass, perms).unwrap();
        dir
    }

    /// Poll `child` for up to `window`, returning how long it took to exit
    /// or `None` if it was still running when the window closed.
    fn wait_bounded(child: &mut std::process::Child, window: Duration) -> Option<Duration> {
        let started = Instant::now();
        let deadline = started + window;
        loop {
            match child.try_wait().expect("poll nc-10 child") {
                Some(_) => return Some(started.elapsed()),
                None => {
                    if Instant::now() >= deadline {
                        return None;
                    }
                    std::thread::sleep(Duration::from_millis(5));
                }
            }
        }
    }

    /// NC-10, positive arm: with `SSH_ASKPASS_REQUIRE=never` an encrypted
    /// key does NOT park on the askpass helper — it gives up promptly.
    #[test]
    fn ssh_signing_probe_does_not_block_on_an_encrypted_key() {
        let dir = encrypted_key_fixture();
        let mut child = askpass_arm(dir.path(), Some("never"));
        let elapsed = wait_bounded(&mut child, SSH_SIGN_PROBE_TIMEOUT / 2);
        if elapsed.is_none() {
            let _ = child.kill();
            let _ = child.wait();
        }
        let elapsed = elapsed.expect(
            "SSH_ASKPASS_REQUIRE=never did not stop ssh-keygen blocking on the askpass helper",
        );
        eprintln!("NC-10 non-blocking arm exited in {elapsed:?}");
        assert!(
            elapsed < Duration::from_secs(2),
            "the non-blocking arm took {elapsed:?}, which is too slow to calibrate a control"
        );
    }

    /// NC-10's control (D-01). The env var, not the fixture and not the
    /// timeout, is what prevents the hang — so the SAME fixture is run with
    /// the variable OMITTED and must still be alive when the window closes.
    ///
    /// **The window is calibrated, not assumed (F-9).** The non-blocking arm
    /// runs first and its wall-clock exit is measured; the window is derived
    /// from that measurement at a stated multiple. An uncalibrated window
    /// shorter than the time `ssh-keygen` ordinarily takes to give up would
    /// report "blocked" for a reason that has nothing to do with the
    /// variable, and the control would pass while measuring the wrong thing.
    /// The window is also held well under [`SSH_SIGN_PROBE_TIMEOUT`], so the
    /// measurement is of the variable's effect rather than of the ceiling.
    ///
    /// A control that agrees with its positive case is a broken measurement,
    /// not evidence: if the blocking arm does NOT block, this test fails
    /// loudly rather than passing.
    #[test]
    fn encrypted_key_blocks_without_the_askpass_require_env_var() {
        const CALIBRATION_MULTIPLE: u32 = 8;
        const MIN_WINDOW: Duration = Duration::from_millis(1000);

        let dir = encrypted_key_fixture();

        // Arm 1 — measure. Same fixture, variable set.
        let mut baseline_child = askpass_arm(dir.path(), Some("never"));
        let baseline = wait_bounded(&mut baseline_child, SSH_SIGN_PROBE_TIMEOUT / 2);
        if baseline.is_none() {
            let _ = baseline_child.kill();
            let _ = baseline_child.wait();
        }
        let baseline = baseline.expect(
            "control uncalibrated: the non-blocking arm never exited, so there is no baseline \
             to derive an observation window from",
        );

        // Derive the window from the measurement rather than assuming one.
        let window = std::cmp::max(baseline * CALIBRATION_MULTIPLE, MIN_WINDOW);
        assert!(
            window >= baseline * 4,
            "control uncalibrated: observation window {window:?} is not at least four times \
             the measured non-blocking exit of {baseline:?}"
        );
        assert!(
            window < SSH_SIGN_PROBE_TIMEOUT / 2,
            "control uncalibrated: observation window {window:?} is not comfortably under the \
             probe's own {SSH_SIGN_PROBE_TIMEOUT:?} ceiling, so it would measure the ceiling \
             rather than SSH_ASKPASS_REQUIRE"
        );

        // Arm 2 — the control. Same fixture, variable omitted.
        let mut blocking_child = askpass_arm(dir.path(), None);
        let blocked = wait_bounded(&mut blocking_child, window);
        let _ = blocking_child.kill();
        let _ = blocking_child.wait();

        eprintln!(
            "NC-10 calibration: non-blocking exit {baseline:?}, observation window {window:?} \
             ({CALIBRATION_MULTIPLE}x, floored at {MIN_WINDOW:?}), blocking arm {blocked:?}"
        );
        assert!(
            blocked.is_none(),
            "NC-10 control FAILED: with SSH_ASKPASS_REQUIRE omitted the child still exited in \
             {blocked:?}, inside the {window:?} window. A control that agrees with its positive \
             case is a broken measurement, not evidence — nothing here supports the conclusion \
             that the environment variable is what prevents the hang"
        );
    }

    // ------------------------------------------------------------------
    // D8 / HARDEN-05 — the PRODUCTION probe drops its controlling terminal.
    //
    // The two NC-10 arms above install their OWN `setsid`, so both would pass
    // byte-unchanged if the production `pre_exec` were deleted: 35-03's
    // SUMMARY records exactly that, as `human_judgment: true`. The test below
    // is the missing guard. It runs the production probe from a child that has
    // ACQUIRED a pty as its controlling terminal, which is the only condition
    // under which the production `setsid` does anything observable at all.
    // ------------------------------------------------------------------

    /// Carries the fixture key into the re-executed child.
    const TTY_PROBE_KEY_ENV: &str = "DEVFLOW_TTY_PROBE_KEY";

    /// The child entrypoint's name, as libtest's `--exact` filter sees it.
    const TTY_PROBE_CHILD: &str = "git::tests::ssh_sign_probe_tty_child_entrypoint";

    // Exit codes for that child. **None of them is 0, deliberately.** `cargo
    // test --exact <name>` exits 0 when the name matches nothing (CLAUDE.md;
    // this repo has already paid for it), so a renamed entrypoint would make
    // the measuring arm below "succeed" in milliseconds while running no probe
    // whatsoever. A 0 exit therefore means "the child never ran" and is
    // asserted against explicitly.
    const EXIT_PROBE_REJECTED: i32 = 42;
    const EXIT_PROBE_TIMED_OUT: i32 = 43;
    const EXIT_PROBE_OTHER: i32 = 44;
    const EXIT_NO_CONTROLLING_TTY: i32 = 97;

    /// A pty pair whose fds are closed on every exit path, including unwind.
    ///
    /// Closing the master hangs up the line, which is also the backstop that
    /// stops anything still sitting on a passphrase prompt from outliving this
    /// test: the session leader's death sends `SIGHUP` to the foreground
    /// process group.
    struct Pty {
        master: libc::c_int,
        slave: libc::c_int,
    }

    impl Pty {
        /// Allocate a pty pair. `O_NOCTTY` throughout — *this* process must
        /// not acquire the terminal; only the child spawned by
        /// [`spawn_owning_controlling_tty`] may, and only via an explicit
        /// `TIOCSCTTY`.
        fn open() -> Pty {
            // SAFETY: each call below is a bare libc entry point with
            // in-bounds arguments (`name` is sized and its length passed), and
            // the `Pty` is constructed as soon as the first fd exists, so its
            // `Drop` owns every descriptor from that point on — including
            // across the assertion unwinds between here and the return.
            unsafe {
                let master = libc::posix_openpt(libc::O_RDWR | libc::O_NOCTTY);
                assert!(
                    master >= 0,
                    "posix_openpt failed: {}",
                    std::io::Error::last_os_error()
                );
                let mut pty = Pty { master, slave: -1 };
                assert!(
                    libc::grantpt(master) == 0,
                    "grantpt failed: {}",
                    std::io::Error::last_os_error()
                );
                assert!(
                    libc::unlockpt(master) == 0,
                    "unlockpt failed: {}",
                    std::io::Error::last_os_error()
                );
                let mut name = [0 as libc::c_char; 128];
                assert!(
                    libc::ptsname_r(master, name.as_mut_ptr(), name.len()) == 0,
                    "ptsname_r failed: {}",
                    std::io::Error::last_os_error()
                );
                let slave = libc::open(name.as_ptr(), libc::O_RDWR | libc::O_NOCTTY);
                assert!(
                    slave >= 0,
                    "opening the pty slave failed: {}",
                    std::io::Error::last_os_error()
                );
                pty.slave = slave;
                pty
            }
        }
    }

    impl Drop for Pty {
        fn drop(&mut self) {
            // SAFETY: both descriptors were opened by `Pty::open` and are
            // closed exactly once, here.
            unsafe {
                if self.slave >= 0 {
                    libc::close(self.slave);
                }
                libc::close(self.master);
            }
        }
    }

    /// Spawn `command` as the leader of a NEW session that has acquired
    /// `pty`'s slave as its **controlling terminal**.
    ///
    /// Both syscalls are checked here, unlike the production probe's
    /// deliberate ignore. A silently failed `TIOCSCTTY` would leave the child
    /// with no controlling terminal — the single condition under which every
    /// assertion in this test passes for the wrong reason — so a failure is
    /// returned as an error and surfaces as a loud `spawn` failure instead.
    fn spawn_owning_controlling_tty(command: &mut Command, pty: &Pty) -> std::process::Child {
        use std::os::unix::process::CommandExt;

        let slave = pty.slave;
        // SAFETY: `setsid` and `ioctl` are bare syscalls and async-signal-safe,
        // which is the only requirement `pre_exec` imposes. `slave` is owned by
        // the `Pty` the caller holds for the child's whole lifetime, and it is
        // inherited across the fork because it was opened without `CLOEXEC`.
        unsafe {
            command.pre_exec(move || {
                if libc::setsid() == -1 {
                    return Err(std::io::Error::last_os_error());
                }
                if libc::ioctl(slave, libc::TIOCSCTTY, 0) == -1 {
                    return Err(std::io::Error::last_os_error());
                }
                Ok(())
            });
        }
        command
            .spawn()
            .expect("spawn a child owning the pty as its controlling terminal")
    }

    /// Spawn a child GUARANTEED to have no controlling terminal, by putting it
    /// in a fresh session and giving it no pty to acquire.
    ///
    /// Arm 0 must not merely *assume* the ambient environment lacks a terminal
    /// — that assumption is environment-dependent and it is false under the
    /// pre-push gate, which runs `docker run --rm -t` (`check-in-container.sh`)
    /// and therefore hands the test binary a pty as its controlling terminal.
    /// Inheriting it made the control arm block on `/dev/tty`, which the
    /// calibration guard correctly reported as `control uncalibrated` rather
    /// than mis-attributing it to `setsid` — a hard red in the container while
    /// this same test passed on a terminal-less host.
    ///
    /// Detaching explicitly makes the arm mean the same thing in both places.
    fn spawn_detached_from_terminal(command: &mut Command) -> std::process::Child {
        use std::os::unix::process::CommandExt;

        // SAFETY: `setsid` is a bare syscall and async-signal-safe, which is
        // the only requirement `pre_exec` imposes. A freshly forked child is
        // never already a process-group leader, so the call cannot fail for
        // the one reason `setsid` fails; it is still checked rather than
        // ignored, because a silent failure here would leave the child holding
        // an inherited terminal and turn this control back into the very
        // environment-dependent arm it exists to replace.
        unsafe {
            command.pre_exec(|| {
                if libc::setsid() == -1 {
                    return Err(std::io::Error::last_os_error());
                }
                Ok(())
            });
        }
        command
            .spawn()
            .expect("spawn a child detached from any controlling terminal")
    }

    /// One raw `ssh-keygen -Y sign` against an encrypted key, mirroring the
    /// production probe's environment and stdio EXACTLY — and deliberately
    /// **not** calling `setsid` itself. Each arm decides its own session: arm 0
    /// via [`spawn_detached_from_terminal`] (fresh session, no pty), arm 1 via
    /// [`spawn_owning_controlling_tty`] (fresh session, then `TIOCSCTTY`). The
    /// only thing that differs between the two arms built from it is therefore
    /// whether the child holds a controlling terminal — and now that holds by
    /// construction rather than by inheritance from whatever spawned the tests.
    fn tty_control_arm(key_pub: &Path, payload: &Path) -> Command {
        let mut command = Command::new("ssh-keygen");
        command
            .args([
                "-Y",
                "sign",
                "-n",
                SSH_SIGN_NAMESPACE,
                "-f",
                key_pub.to_str().unwrap(),
                payload.to_str().unwrap(),
            ])
            .env("SSH_ASKPASS_REQUIRE", "never")
            .stdin(Stdio::null())
            .stdout(Stdio::null())
            .stderr(Stdio::null());
        command
    }

    /// Re-entry point for the D8 test below: runs the **production** probe
    /// inside whatever session its caller placed this process in, and reports
    /// the verdict as a process exit code.
    ///
    /// A no-op unless [`TTY_PROBE_KEY_ENV`] is set, so an ordinary `cargo test`
    /// run spawns nothing and costs nothing here.
    ///
    /// The `/dev/tty` open is a **premise check, not hygiene**: without it a
    /// `TIOCSCTTY` that silently failed would leave this child with no
    /// terminal, the probe would return promptly for a reason having nothing to
    /// do with `setsid`, and the test would pass while measuring nothing.
    #[test]
    fn ssh_sign_probe_tty_child_entrypoint() {
        let Ok(key) = std::env::var(TTY_PROBE_KEY_ENV) else {
            return;
        };

        let tty_path = std::ffi::CString::new("/dev/tty").unwrap();
        // SAFETY: `tty_path` is a valid NUL-terminated C string that outlives
        // the call, and the descriptor is closed on the one path that opens it.
        let tty = unsafe { libc::open(tty_path.as_ptr(), libc::O_RDWR) };
        if tty < 0 {
            std::process::exit(EXIT_NO_CONTROLLING_TTY);
        }
        // SAFETY: `tty` was just opened by this thread and is closed once.
        unsafe { libc::close(tty) };

        std::process::exit(match run_ssh_sign_probe(Path::new(&key)) {
            SignProbeOutcome::Rejected => EXIT_PROBE_REJECTED,
            SignProbeOutcome::TimedOut => EXIT_PROBE_TIMED_OUT,
            _ => EXIT_PROBE_OTHER,
        });
    }

    /// **D8 (HARDEN-05): the production signing probe is not captured by a
    /// controlling terminal's `/dev/tty` passphrase prompt.**
    ///
    /// `SSH_ASKPASS_REQUIRE=never` is not sufficient on its own — OpenSSH only
    /// consults it after `open("/dev/tty")` has already failed. The production
    /// `pre_exec`/`setsid` is what makes that open fail. Delete it and this
    /// test fails: the probe blocks on the terminal until its own 10 s ceiling.
    ///
    /// Three arms, and the first two are a **matched pair that must disagree**:
    ///
    /// | arm | terminal | `setsid` | required result |
    /// |---|---|---|---|
    /// | 0 — baseline/control | none | none | exits promptly |
    /// | 1 — premise | pty, acquired | none | still blocked when the window closes |
    /// | 2 — measurement | pty, acquired | production's | exits promptly, with a real verdict |
    ///
    /// If arms 0 and 1 agreed, the harness would have established nothing —
    /// either the terminal never took effect, or this OpenSSH build does not
    /// use it — and arm 2 would be fast for a reason unrelated to `setsid`.
    /// Arm 1 therefore runs BEFORE the measurement and fails as a PREMISE
    /// failure, not as a regression.
    ///
    /// The observation window is derived from arm 0's measured exit, at a
    /// stated multiple, following NC-10's calibration shape; every wait is
    /// bounded and every child is killed and reaped on every path.
    #[test]
    fn the_signing_probe_is_not_captured_by_a_controlling_terminal() {
        const CALIBRATION_MULTIPLE: u32 = 8;
        const MIN_WINDOW: Duration = Duration::from_millis(1000);
        /// Bound for the production arm: far above a real verdict (tens of
        /// milliseconds plus one process spawn) and far below the probe's own
        /// [`SSH_SIGN_PROBE_TIMEOUT`], so exceeding it means "ran to the
        /// ceiling on the terminal", not "this host is slow".
        const PROBE_ARM_CAP: Duration = Duration::from_millis(3000);

        let dir = tempfile::tempdir().unwrap();
        let key_pub = generate_keypair(&dir.path().join("tty-key"), "devflow-d8-passphrase");
        let payload = dir.path().join("payload");
        std::fs::write(&payload, b"devflow d8 tty payload\n").unwrap();

        // --- Arm 0: the paired control. Same command, same environment, NO
        // controlling terminal. `readpassphrase` falls back to a nulled stdin
        // and gives up at once. The child is detached into its own session
        // explicitly rather than trusting the ambient environment to lack a
        // terminal — see `spawn_detached_from_terminal`.
        let mut baseline_child =
            spawn_detached_from_terminal(&mut tty_control_arm(&key_pub, &payload));
        let baseline = wait_bounded(&mut baseline_child, SSH_SIGN_PROBE_TIMEOUT / 2);
        if baseline.is_none() {
            let _ = baseline_child.kill();
            let _ = baseline_child.wait();
        }
        let baseline = baseline.expect(
            "control uncalibrated: ssh-keygen blocked with NO controlling terminal, so nothing \
             measured below can be attributed to the terminal",
        );

        let window = std::cmp::max(baseline * CALIBRATION_MULTIPLE, MIN_WINDOW);
        assert!(
            window >= baseline * 4,
            "control uncalibrated: observation window {window:?} is not at least four times the \
             measured no-terminal exit of {baseline:?}"
        );
        assert!(
            window < SSH_SIGN_PROBE_TIMEOUT / 2,
            "control uncalibrated: observation window {window:?} is not comfortably under the \
             probe's own {SSH_SIGN_PROBE_TIMEOUT:?} ceiling, so it would measure the ceiling"
        );

        // --- Arm 1: the premise, and the other half of the pair. Same command
        // again, WITH the pty acquired as a controlling terminal and no
        // `setsid`: it must still be blocked when the window closes.
        let blocked = {
            let pty = Pty::open();
            let mut child =
                spawn_owning_controlling_tty(&mut tty_control_arm(&key_pub, &payload), &pty);
            let blocked = wait_bounded(&mut child, window);
            let _ = child.kill();
            let _ = child.wait();
            blocked
        };
        assert!(
            blocked.is_none(),
            "PREMISE FAILED: with a controlling terminal and no setsid, ssh-keygen exited in \
             {blocked:?} — the same result as the no-terminal control ({baseline:?}). Either the \
             pty was never acquired or this build does not consult /dev/tty, and either way the \
             arm below would be fast for a reason unrelated to the production setsid. A control \
             that agrees with its positive case is a broken measurement, not evidence"
        );

        // --- Arm 2: the measurement. The PRODUCTION probe, same fixture, same
        // kind of controlling terminal — re-executed as a child because only a
        // separate process can be made a session leader.
        let (elapsed, status) = {
            let pty = Pty::open();
            let mut command = Command::new(
                std::env::current_exe().expect("locate this test binary for re-execution"),
            );
            command
                .args([TTY_PROBE_CHILD, "--exact", "--test-threads=1"])
                .env(TTY_PROBE_KEY_ENV, &key_pub)
                .stdin(Stdio::null())
                .stdout(Stdio::null())
                .stderr(Stdio::null());
            let mut child = spawn_owning_controlling_tty(&mut command, &pty);
            let elapsed = wait_bounded(&mut child, PROBE_ARM_CAP);
            if elapsed.is_none() {
                let _ = child.kill();
                let _ = child.wait();
            }
            let status = elapsed.map(|_| child.wait().expect("reap the production probe arm"));
            (elapsed, status)
        };
        let code = status.and_then(|status| status.code());
        eprintln!(
            "D8: no-terminal baseline {baseline:?}; window {window:?} \
             ({CALIBRATION_MULTIPLE}x, floored at {MIN_WINDOW:?}); with-terminal control \
             {blocked:?}; production probe {elapsed:?} exiting {code:?}"
        );

        let elapsed = elapsed.unwrap_or_else(|| {
            panic!(
                "REGRESSION: the production signing probe did not return within {PROBE_ARM_CAP:?} \
                 while its caller held a controlling terminal. That is the pre-setsid behaviour — \
                 it is parked on /dev/tty waiting for a passphrase nobody can type, and will stay \
                 there until SSH_SIGN_PROBE_TIMEOUT ({SSH_SIGN_PROBE_TIMEOUT:?}) expires. The \
                 no-terminal control exited in {baseline:?}, so the fixture and the environment \
                 are not what changed"
            )
        });
        assert_ne!(
            code,
            Some(EXIT_NO_CONTROLLING_TTY),
            "PREMISE FAILED: the re-executed child could not open /dev/tty, so it never held a \
             controlling terminal and its {elapsed:?} says nothing about setsid"
        );
        assert_ne!(
            code,
            Some(0),
            "the child exited 0, which no path in ssh_sign_probe_tty_child_entrypoint does: \
             `--exact {TTY_PROBE_CHILD}` matched no test, so no probe ran at all"
        );
        assert_eq!(
            code,
            Some(EXIT_PROBE_REJECTED),
            "the probe returned in {elapsed:?} but with the wrong verdict: {} means it hit its \
             own ceiling and {} means it never reached one",
            EXIT_PROBE_TIMED_OUT,
            EXIT_PROBE_OTHER
        );
    }

    /// D-03/A-17: an inline `user.signingkey` returns `Unknown` with the
    /// fixed inline reason and is never probed.
    ///
    /// Run with a NORMAL `PATH`, so `ssh-keygen` is present throughout. That
    /// is what makes this a proof of "never probed" rather than of "failed
    /// to probe" — the surface test that removes the tooling cannot tell the
    /// two apart.
    #[test]
    fn inline_signing_key_returns_unknown_without_probing() {
        const INLINE_REASON: &str =
            "cannot verify signing viability — an inline user.signingkey is not probed";
        let keys = tempfile::tempdir().unwrap();
        let pub_key = generate_keypair(&keys.path().join("inline-key"), "");
        let blob = std::fs::read_to_string(&pub_key)
            .unwrap()
            .trim()
            .to_string();

        for value in [format!("key::{blob}"), blob.clone()] {
            let repo = init_repo();
            let root = repo.path();
            git(root, &["config", "gpg.format", "ssh"]);
            git(root, &["config", "user.signingkey", &value]);

            let result = check_signing_viability(root);

            assert_eq!(
                result,
                SigningViability::Unknown {
                    reason: INLINE_REASON.into(),
                },
                "inline value {value:?} was not routed to the unprobed Unknown arm: {result:?}"
            );
            assert_no_leak(&result, keys.path());
        }
    }
}