car-server-core 0.52.1

//! Merge-back: deliver the worktree's changes.
//!
//! Three destinations, by session kind:
//! - **Raw repo** ([`publish_branch`]): the worktree gets one squash commit
//!   (authored as `car-coder` — honest attribution) and the only thing that
//!   touches the user's repository is `git branch car/coder/<id> <commit>`.
//!   Branches are shared across worktrees, so this never disturbs the user's
//!   checkout, index, or any existing ref; reverting is `git branch -D`.
//! - **Managed project** ([`commit_to_main`]): the project is fully CAR-owned,
//!   so there is no separate "user working tree" to protect — approve commits
//!   straight to the project's `main` (fast-forward only). The non-dev sees
//!   "Saved", not a branch to merge; reverting is ordinary git history.
//! - **Headless / pull request** ([`deliver_pr`]): nobody is at the approval
//!   gate, and the next round is a fresh session that will only ever see what
//!   is on GitHub. The runtime commits, pushes append-only to a branch that is
//!   stable across sessions, and reconciles exactly one pull request for it.

use std::path::Path;

use super::contract::OutcomeContract;

/// One-line summary of an intent, for a commit subject or a pull-request title.
/// Capped at 72 characters on a char boundary — the git convention, and long
/// enough that the truncation is rare.
fn subject_from_intent(intent: &str) -> String {
    // Line endings normalized FIRST, and that ordering is the whole point. The
    // paragraph split below looks for `\n\n`, which a Windows-authored or
    // Windows-pasted intent never contains — it separates paragraphs with
    // `\r\n\r\n`. The split then matched nothing, `head` became the entire
    // document, and the `replace('\n', " ")` that follows left every `\r` in
    // place: the blank-line handling this function was given was silently off
    // for exactly the callers most likely to paste a long intent, and the
    // resulting commit subject and pull-request title carried embedded carriage
    // returns. `--intent-file` reads bytes, so this is not an exotic path.
    let trimmed = intent.replace("\r\n", "\n");
    let trimmed = trimmed.trim();
    // A blank line is git's own subject/body separator, and a caller that puts
    // one there is saying where the summary ends. Honour it BEFORE flattening:
    // `replace('\n', " ")` first destroyed the break, so a caller who correctly
    // led with a short summary still got the pointer text dragged in behind it
    // and truncated at 72 — the paragraph structure was gone before the length
    // check could act on it. With no blank line, the flatten-and-truncate
    // fallback below behaves exactly as it always has.
    let head = trimmed.split("\n\n").next().unwrap_or(trimmed).trim();
    // A lone `\r` (an old-Mac line ending, or one left by a mixed-ending file)
    // is not a line break to git — it is a control character inside the
    // subject.
    let s = head.replace(['\n', '\r'], " ");
    if s.len() > 72 {
        let mut end = 69;
        while !s.is_char_boundary(end) {
            end -= 1;
        }
        format!("{}...", &s[..end])
    } else {
        s
    }
}

/// What [`commit_worktree`] did, as a value rather than as a phrase inside an
/// error string.
///
/// "The worktree was clean" is a normal outcome on the PR path — a round whose
/// push failed re-delivers the commit it already made — so `deliver_pr_with` has
/// to tell it apart from a commit that genuinely failed. It used to do that with
/// `Err(e) if e.contains("no changes to deliver")`, and that phrase is not the
/// exclusive property of the clean-worktree check: [`git`] formats every failure
/// as `git {args:?} failed: {stderr}`, and the commit argv carries `subject` and
/// `body`, both derived from the caller's `intent`. An intent that merely
/// MENTIONS the phrase — "fix delivery so it reports 'no changes to deliver'
/// correctly" is ordinary work in this repo — turned a real commit failure
/// (`commit.gpgsign` with no agent, a shared `pre-commit` hook) into the
/// re-delivery branch: the previous commit was pushed, the pull-request body was
/// refreshed, and delivery returned `Ok` while the round's actual work sat
/// uncommitted in the worktree. A typed variant cannot be spelled by a caller.
enum CommitOutcome {
    /// A commit was made; the new SHA.
    Made(String),
    /// The worktree had nothing to commit. Not an error here — the callers
    /// decide what it means.
    NothingToCommit,
}

/// Stage all worktree changes and create the CAR-Coder commit. Reports a clean
/// worktree as [`CommitOutcome::NothingToCommit`]; `Err` is reserved for a real
/// failure. Shared by all three delivery paths.
fn commit_worktree(
    worktree: &Path,
    intent: &str,
    contract: &OutcomeContract,
) -> Result<CommitOutcome, String> {
    // `--untracked-files=normal`, pinned rather than inherited: `status
    // --porcelain` honours `status.showUntrackedFiles`, and a repository or
    // global `no` makes a worktree holding nothing BUT new files report clean.
    // Delivery would then take the re-delivery branch and push a stale commit
    // while the round's entire output — every new file it wrote — stayed
    // behind.
    let status = git(
        worktree,
        &["status", "--porcelain", "--untracked-files=normal"],
    )?;
    if status.trim().is_empty() {
        return Ok(CommitOutcome::NothingToCommit);
    }
    git(worktree, &["add", "-A"])?;

    let subject = subject_from_intent(intent);
    let body = format!(
        "Authored by CAR Coder.\n\nIntent:\n{}\n\nOutcome contract (all checks passed):\n{}",
        intent.trim(),
        contract.render()
    );
    git(
        worktree,
        &[
            "-c",
            "user.name=car-coder",
            "-c",
            "user.email=coder@parslee.ai",
            "commit",
            "-m",
            &subject,
            "-m",
            &body,
        ],
    )?;
    Ok(CommitOutcome::Made(
        git(worktree, &["rev-parse", "HEAD"])?.trim().to_string(),
    ))
}

/// The interactive paths' reading of a clean worktree: nothing to publish, and
/// that is an error. Only the PR path has a second, legitimate meaning for it.
fn require_commit(outcome: CommitOutcome) -> Result<String, String> {
    match outcome {
        CommitOutcome::Made(sha) => Ok(sha),
        CommitOutcome::NothingToCommit => {
            Err("no changes to deliver — the worktree is clean".to_string())
        }
    }
}

/// The commit a HEADLESS round delivers when its worktree is already clean.
///
/// A clean worktree is not automatically an error on the headless paths, unlike
/// the interactive ones. The contract keeps a workspace alive across rounds
/// precisely so that a green session whose DELIVERY failed can re-deliver
/// without redoing the work: on that second round the commit already exists and
/// the worktree is clean. Deliver HEAD.
///
/// But "clean" has a second cause that must not be confused with it: a first
/// round where the contract was green with an EMPTY diff (the named check
/// already passed). There HEAD is still the base tip, and delivering it creates
/// a branch and a pull request that prove nothing — over the pull-request path,
/// `gh pr create` then fails with "No commits between …", reported as retriable
/// and requeued forever with junk branches accumulating. Only a genuine
/// re-delivery has something to deliver, and that means HEAD is already AHEAD of
/// the base.
///
/// The base is resolved remote-first with the local branch as an honest stand-in
/// — and the failure to resolve EITHER is an error rather than a `false`.
/// Reading `is_ok()` on an unresolvable ref collapses "not an ancestor" and
/// "could not check" into the same answer, i.e. "there is work to deliver": that
/// fails OPEN, restoring the empty-delivery bug this guard exists to close,
/// exactly when the network is down and the best-effort fetch did nothing.
fn head_beyond_base(worktree: &Path, base_branch: &str) -> Result<String, String> {
    let head = git(worktree, &["rev-parse", "HEAD"])?.trim().to_string();
    let base_ref = ["origin/", ""]
        .iter()
        .map(|p| format!("{p}{base_branch}"))
        .find(|r| git(worktree, &["rev-parse", "--verify", "--quiet", r]).is_ok())
        .ok_or_else(|| {
            format!(
                "the worktree is clean and neither origin/{base_branch} nor {base_branch} \
                 resolves, so whether there is anything to deliver cannot be determined"
            )
        })?;
    if git(worktree, &["merge-base", "--is-ancestor", &head, &base_ref]).is_ok() {
        return Err(format!(
            "nothing to deliver: the worktree is clean and its HEAD ({head}) is already \
             contained in {base_ref}, so there is no work to deliver"
        ));
    }
    Ok(head)
}

/// Result of a publish: the branch name to merge from.
pub fn publish_branch(
    repo: &Path,
    worktree: &Path,
    short_id: &str,
    intent: &str,
    contract: &OutcomeContract,
) -> Result<String, String> {
    let commit = require_commit(commit_worktree(worktree, intent, contract)?)?;
    let branch = format!("car/coder/{short_id}");
    // `git branch` (no checkout) in the original repo: refs are shared with
    // the worktree, so this is pure bookkeeping — no working-tree effects.
    git(repo, &["branch", &branch, &commit])?;
    Ok(branch)
}

/// [`publish_branch`] for the HEADLESS branch mode, where a clean worktree is a
/// legitimate re-delivery rather than an error.
///
/// `car code-task --deliver branch` runs the same kept-workspace lifecycle as
/// `--deliver pr`, and only the pull-request path was ever taught what a clean
/// worktree means there. Branch mode went through `require_commit`, so this
/// sequence parked a healthy goal: round N delivers via `--deliver branch`,
/// committing the work into a kept workspace; round N+1 reuses it, HEAD is ahead
/// of the base so the vacuity guard correctly stands down, the contract is
/// already green so the loop edits nothing — and then the publish failed with
/// "no changes to deliver". [`DeliveryFailure::Commit`] is hard-coded
/// non-retriable, so a run whose contract was green and whose work was intact
/// exited 3, "park it and tell a human", every round.
///
/// The empty-first-round case is still refused, by the same rule the
/// pull-request path uses: see [`head_beyond_base`].
pub fn publish_branch_headless(
    repo: &Path,
    worktree: &Path,
    short_id: &str,
    intent: &str,
    contract: &OutcomeContract,
    base_branch: &str,
) -> Result<String, String> {
    let commit = match commit_worktree(worktree, intent, contract)? {
        CommitOutcome::Made(sha) => sha,
        CommitOutcome::NothingToCommit => head_beyond_base(worktree, base_branch)?,
    };
    let branch = format!("car/coder/{short_id}");
    git(repo, &["branch", &branch, &commit])?;
    Ok(branch)
}

/// Deliver to a managed project's `main`. The worktree was checked out detached
/// at `main`'s tip, so its commit is a direct descendant — a fast-forward
/// updates both the `main` ref and the project's checkout. **ff-only**: if
/// `main` moved since the session started (something committed underneath us),
/// this errors instead of rebasing or forcing, preserving the guarantee that
/// the diff the user approved is exactly what lands. Returns the commit SHA.
pub fn commit_to_main(
    repo: &Path,
    worktree: &Path,
    intent: &str,
    contract: &OutcomeContract,
) -> Result<String, String> {
    let commit = require_commit(commit_worktree(worktree, intent, contract)?)?;
    git(repo, &["merge", "--ff-only", &commit]).map_err(|e| {
        format!("could not fast-forward the project's main branch (it moved since the session started): {e}")
    })?;
    Ok(commit)
}

/// The staged diff as the approval surface needs it.
#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct StagedDiff {
    /// `git diff --cached --stat`, never truncated — it is small, and it is the
    /// one view that stays complete however large the patch gets.
    pub stat: String,
    /// The patch, tail-capped to the configured budget.
    pub patch: String,
    /// Whether `patch` is a tail rather than the whole thing. Carried as a
    /// field, not just the `…[truncated]…` marker inside the string, so a UI can
    /// say so without string-matching (car#706).
    pub truncated: bool,
    /// Size of the untruncated patch, so the surface can report how much was
    /// withheld.
    pub full_bytes: usize,
    /// Every repo-relative path the diff touches, sorted and deduped — for
    /// contract-overlap disclosure and the changed-file summary at the gate.
    ///
    /// Includes BOTH endpoints of a rename. See [`parse_name_status_z`] for why
    /// the destination alone is not an honest answer.
    pub changed_paths: Vec<String>,
}

/// Stage everything and collect the diff for the approval UI. Staging is what
/// `publish_branch` would commit anyway, and it makes untracked files visible.
///
/// `patch_cap_bytes` bounds only the patch body. The review surface used to
/// shrink exactly as the risk grew: the cap was a hardcoded 32 KB tail, so on a
/// long session — the regime where the automated check is weakest — the human
/// approved against a partial patch, with the fact of truncation buried in a
/// marker inside the string.
pub fn stage_and_diff(worktree: &Path, patch_cap_bytes: usize) -> Result<StagedDiff, String> {
    git(worktree, &["add", "-A"])?;
    let stat = git(worktree, &["diff", "--cached", "--stat"])?;
    let patch = git(worktree, &["diff", "--cached"])?;
    let names = git(
        worktree,
        &[
            // Redundant insurance, NOT the mechanism: `-z` below already emits
            // raw bytes, so non-ASCII paths arrive unquoted with or without
            // this. It matters only if someone later drops `-z`, at which point
            // paths would come back C-escaped (`café.txt` as the literal
            // `"caf\303\251.txt"`, quotes included) and match nothing.
            "-c",
            "core.quotepath=false",
            "diff",
            "--cached",
            // `--name-status -z`, NOT `--name-only`. Rename detection reports
            // only the DESTINATION under `--name-only`, so
            // `git mv secrets/key.txt public_key.txt` yielded exactly
            // `["public_key.txt"]` and the fact that `secrets/` was touched
            // vanished. Any consumer reasoning about which paths a session
            // affected was being told a rename is a creation. `-z` additionally
            // makes paths NUL-delimited, so a newline in a filename cannot
            // forge an entry.
            "--name-status",
            "-z",
        ],
    )?;
    let changed_paths = parse_name_status_z(&names);
    let full_bytes = patch.len();
    Ok(StagedDiff {
        patch: super::shell_tool::tail(&patch, patch_cap_bytes),
        truncated: full_bytes > patch_cap_bytes,
        full_bytes,
        stat,
        changed_paths,
    })
}

/// Parse `git diff --cached --name-status -z` into every path the diff touches.
///
/// The `-z` stream is a flat run of NUL-terminated fields. Most entries are two
/// fields — a status letter then a path. Rename (`R`) and copy (`C`) entries are
/// three: the status carries a similarity score, then the SOURCE path, then the
/// destination. **Both endpoints are returned**, because a caller asking "what
/// did this session touch" is asking about the source too: a file moved out of a
/// directory is a change to that directory, and reporting only the destination
/// is how `--name-only` made `git mv secrets/key.txt public_key.txt` look like
/// the creation of an unrelated file.
fn parse_name_status_z(raw: &str) -> Vec<String> {
    let mut out = Vec::new();
    let mut fields = raw.split('\0').filter(|f| !f.is_empty());
    while let Some(status) = fields.next() {
        // Git's status set is closed — A C D M R T U X B — with `R`/`C`
        // carrying a similarity score (`R100`) and `M` optionally a
        // dissimilarity score under `-B`. Validate rather than assume: if a
        // field that is NOT a status reaches here, the loop reads a path as a
        // status and every subsequent field shifts by one, emitting a list of
        // plausible-looking fictional paths — onto a reviewer's approval screen,
        // as fact. A short list plus a warning is recoverable; silent fiction is
        // not, so bail loudly instead of guessing.
        let bytes = status.as_bytes();
        let well_formed = status.len() <= 4
            && matches!(
                bytes[0],
                b'A' | b'C' | b'D' | b'M' | b'R' | b'T' | b'U' | b'X' | b'B'
            )
            && status[1..].bytes().all(|b| b.is_ascii_digit());
        if !well_formed {
            tracing::warn!(
                status = %status,
                "unexpected field in `git diff --name-status -z`; changed-path list truncated \
                 rather than risk a desynchronized parse"
            );
            break;
        }
        // A rename/copy is followed by two paths rather than one.
        let two_paths = bytes[0] == b'R' || bytes[0] == b'C';
        let Some(first) = fields.next() else {
            tracing::warn!(status = %status, "name-status stream ended mid-entry");
            break;
        };
        out.push(first.to_string());
        if two_paths {
            match fields.next() {
                Some(second) => out.push(second.to_string()),
                // A rename with no destination is a corrupt stream, not an
                // entry to swallow silently.
                None => {
                    tracing::warn!(status = %status, "rename/copy entry missing its destination");
                    break;
                }
            }
        }
    }
    out.sort();
    out.dedup();
    out
}

/// Run git in `dir`, with the repository-selecting environment cleared.
///
/// `-C <dir>` selects a directory, not a repository: git resolves the repository
/// from `GIT_DIR`/`GIT_WORK_TREE` FIRST and only walks up from the directory if
/// they are unset. So an inherited `GIT_DIR` silently wins over the `-C` this
/// module relies on, and every caller here — commit, push, `rev-parse HEAD` —
/// would operate on a repository nobody named while the path-based reasoning
/// upstream said the worktree was the right one. That is reachable without
/// anything exotic: a git hook, `git rebase --exec`, or an orchestrator that
/// exported them once for its own bookkeeping. Clearing them costs nothing and
/// makes `-C` mean what the rest of this file assumes it means.
pub(crate) fn git(dir: &Path, args: &[&str]) -> Result<String, String> {
    let mut cmd = std::process::Command::new("git");
    cmd.env_remove("GIT_DIR")
        .env_remove("GIT_WORK_TREE")
        .env_remove("GIT_INDEX_FILE")
        .env_remove("GIT_OBJECT_DIRECTORY")
        .env_remove("GIT_ALTERNATE_OBJECT_DIRECTORIES")
        .env_remove("GIT_COMMON_DIR")
        .arg("-C")
        .arg(dir)
        .args(args);
    no_interactive_prompts(&mut cmd);
    let out = run_capped(cmd).map_err(|e| match e {
        RunFailure::Spawn(io) => format!("git {args:?}: {io}"),
        RunFailure::TimedOut(secs) => format!(
            "git {args:?} timed out after {secs}s and was killed; treat it as a transport failure"
        ),
    })?;
    if out.status.success() {
        Ok(String::from_utf8_lossy(&out.stdout).into_owned())
    } else {
        Err(format!(
            "git {args:?} failed: {}",
            String::from_utf8_lossy(&out.stderr).trim()
        ))
    }
}

/// Refuse every interactive credential prompt, on every child of this module.
///
/// `Command::output()` nulls the child's stdin, and that is NOT enough: git does
/// not prompt on stdin. `git_terminal_prompt` opens `/dev/tty` directly, so a
/// `car code-task --deliver pr` started from a shell, tmux or `nohup` with an
/// inherited controlling terminal — the ordinary way an orchestrator launches
/// it — blocked forever on `Username for 'https://github.com':` against an
/// HTTPS remote with no stored credential. The round never completed, never
/// failed, and emitted no `delivery_failed`, in a command whose entire premise
/// is that nobody is watching. `GIT_ASKPASS` and `SSH_ASKPASS_REQUIRE` close the
/// two GUI-helper doors to the same room.
///
/// With this set the hang becomes `fatal: could not read Username/Password …`
/// or `terminal prompts disabled`, all three of which
/// [`classify_push_error`] now reads as a permanent refusal — which is the
/// truthful answer: no credential is ever going to appear.
fn no_interactive_prompts(cmd: &mut std::process::Command) {
    cmd.env("GIT_TERMINAL_PROMPT", "0")
        .env("GIT_ASKPASS", "")
        .env("SSH_ASKPASS", "")
        .env("SSH_ASKPASS_REQUIRE", "never");
}

/// How long any subprocess on the delivery path may run before it is killed.
///
/// Generous on purpose: a first push of a large repository over a slow link is
/// legitimately minutes, and killing honest work would be worse than the hang
/// this bounds. The point is only that "forever" is not one of the outcomes.
const SUBPROCESS_TIMEOUT_SECS: u64 = 900;

/// Why a subprocess produced no output.
enum RunFailure {
    /// It never started.
    Spawn(std::io::Error),
    /// It started and outlived [`SUBPROCESS_TIMEOUT_SECS`]; it has been killed.
    TimedOut(u64),
}

/// `Command::output()` with a ceiling.
///
/// `output()` waits forever. Every network-touching call on this path — push,
/// fetch, every `gh` round trip — can hang indefinitely on a black-holed
/// connection or a credential prompt, and this module's callers have no other
/// clock: `car code-task` reports a hung round as nothing at all, not even an
/// empty failure. A killed child at least becomes a classified error.
///
/// stdout and stderr are drained on their own threads because a child that
/// fills a pipe buffer blocks on the write while we block on `try_wait`, which
/// is a deadlock no timeout could observe. On the kill path the threads are
/// deliberately NOT joined: a grandchild (ssh, a credential helper) can hold the
/// inherited pipe open after its parent dies, and joining would reintroduce
/// exactly the unbounded wait this exists to remove.
fn run_capped(cmd: std::process::Command) -> Result<std::process::Output, RunFailure> {
    run_capped_for(cmd, std::time::Duration::from_secs(SUBPROCESS_TIMEOUT_SECS))
}

/// [`run_capped`] with the ceiling injected, so a test can prove the kill path
/// in a second rather than in fifteen minutes. Production always passes
/// [`SUBPROCESS_TIMEOUT_SECS`].
fn run_capped_for(
    mut cmd: std::process::Command,
    timeout: std::time::Duration,
) -> Result<std::process::Output, RunFailure> {
    use std::io::Read as _;
    use std::process::Stdio;

    let mut child = cmd
        .stdin(Stdio::null())
        .stdout(Stdio::piped())
        .stderr(Stdio::piped())
        .spawn()
        .map_err(RunFailure::Spawn)?;

    let mut child_out = child.stdout.take().expect("stdout piped");
    let mut child_err = child.stderr.take().expect("stderr piped");
    let out_reader = std::thread::spawn(move || {
        let mut buf = Vec::new();
        let _ = child_out.read_to_end(&mut buf);
        buf
    });
    let err_reader = std::thread::spawn(move || {
        let mut buf = Vec::new();
        let _ = child_err.read_to_end(&mut buf);
        buf
    });

    let deadline = std::time::Instant::now() + timeout;
    let status = loop {
        match child.try_wait() {
            Ok(Some(status)) => break status,
            Ok(None) => {}
            Err(e) => return Err(RunFailure::Spawn(e)),
        }
        if std::time::Instant::now() >= deadline {
            let _ = child.kill();
            let _ = child.wait();
            return Err(RunFailure::TimedOut(timeout.as_secs()));
        }
        std::thread::sleep(std::time::Duration::from_millis(25));
    };

    Ok(std::process::Output {
        status,
        stdout: out_reader.join().unwrap_or_default(),
        stderr: err_reader.join().unwrap_or_default(),
    })
}

// ---------------------------------------------------------------------------
// PR delivery — the third mode
// ---------------------------------------------------------------------------
//
// `publish_branch` and `commit_to_main` deliver INTO the local repository and
// stop there; a human is standing at the approval gate. PR delivery is the
// headless mode: nobody is watching, the work has to leave this machine, and
// the next round is a fresh session that will only ever see what is on GitHub.
// That changes what "deliver" has to guarantee, and every rule below is one of
// those guarantees rather than a preference.

/// Which of the two things happened to the pull request for the target
/// branch. Mirrors the `pr_action` field of the delivery event stream.
///
/// There is deliberately no `Reopened`. The runtime never closes a pull
/// request, so it is never the party entitled to undo a close — see
/// [`closed_pr_refusal`].
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum PrAction {
    /// No pull request existed for this branch, so one was created.
    Opened,
    /// An open pull request already existed; the push landed on it.
    Updated,
}

impl PrAction {
    /// The wire spelling used by the `delivery_completed` event.
    pub fn as_str(&self) -> &'static str {
        match self {
            PrAction::Opened => "opened",
            PrAction::Updated => "updated",
        }
    }
}

impl std::fmt::Display for PrAction {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.write_str(self.as_str())
    }
}

/// GitHub's three terminal states for a pull request, as this module needs to
/// distinguish them. `Merged` is deliberately NOT folded into `ClosedUnmerged`:
/// a merge is this branch's work LANDING, while a close is somebody's decision
/// against it, so the two demand opposite responses — the first lets the next
/// round open a fresh pull request, the second parks the round.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PrState {
    /// Open — a push to its head branch updates it in place.
    Open,
    /// Closed without being merged. Somebody decided against it; the runtime
    /// does not reopen one (see [`closed_pr_refusal`]).
    ClosedUnmerged,
    /// Merged — its work landed, and a new pull request carries the next round.
    Merged,
}

/// One pull request as GitHub reports it for a head branch.
#[derive(Debug, Clone, PartialEq)]
pub struct PrRecord {
    pub number: u64,
    pub state: PrState,
    pub url: String,
    pub is_draft: bool,
    /// The branch this pull request merges INTO, as `gh` reports `baseRefName`.
    ///
    /// Carried because GitHub's one-open-pull-request constraint is per (head,
    /// base) PAIR, not per head — two open pull requests from the same branch
    /// into different bases are entirely legal. Reconciliation without this
    /// field picked the highest-numbered open pull request for the head and
    /// rewrote its body, so a second pull request opened from the same branch
    /// into `release/2.1` (by a human, or by a round invoked with a different
    /// `--pr-base`) captured every later round: its description — which this run
    /// did not author — was replaced wholesale, and the pull request the
    /// orchestrator actually tracks kept a stale body forever.
    pub base: String,
}

/// Everything PR delivery needs. Borrowed rather than owned because every field
/// already exists in the caller's session state.
pub struct PrDelivery<'a> {
    /// The repository. `gh` runs here, so this must be a checkout with the
    /// GitHub remote configured.
    pub repo: &'a Path,
    /// The session's worktree — where the changes are and where the commit is
    /// made. It shares `repo`'s config, so it can push to the same remote.
    pub worktree: &'a Path,
    /// The delivery branch. **Stable across sessions**: round N+1 pushes to the
    /// same branch, so a second branch is never created for the same goal.
    pub target_branch: &'a str,
    /// The base the pull request merges into.
    pub base_branch: &'a str,
    /// Open the pull request as a draft. Only ever consulted when a pull
    /// request is CREATED — see [`deliver_pr`] on why this never flips an
    /// existing one.
    pub draft: bool,
    /// The run intent; becomes the commit subject and the pull-request title.
    pub intent: &'a str,
    /// The contract that passed. Embedded in the commit body by
    /// [`commit_worktree`].
    pub contract: &'a OutcomeContract,
    /// The substantive pull-request description: what was done, what remains,
    /// what the contract proved. It is the next fresh session's context and
    /// must stand alone without the diff.
    pub body: &'a str,
}

/// A successful PR delivery.
#[derive(Debug, Clone, PartialEq)]
pub struct PrDeliveryOutcome {
    /// The target branch the commit landed on.
    pub branch: String,
    /// The delivered commit SHA.
    pub commit: String,
    /// Whether the push actually ran (always `true` on success today; carried
    /// as a field because the event stream reports it).
    pub pushed: bool,
    pub pr_number: u64,
    pub pr_url: String,
    pub pr_action: PrAction,
    /// The pull request's ACTUAL draft state, not the requested one. On
    /// `Opened` these agree; on `Updated` this reports what GitHub says, because
    /// a car worker may have marked the pull request ready in an earlier round
    /// and delivery must not misreport that.
    pub draft: bool,
}

/// Why a PR delivery stopped, at which stage, and whether the next round should
/// simply try again.
///
/// The stage is not decoration: it is the difference between "the work is safe
/// on disk, re-push it" and "this goal cannot progress without a human". It
/// maps 1:1 onto the `delivery_failed` event's `stage` field.
#[derive(Debug, Clone, PartialEq)]
pub enum DeliveryFailure {
    /// Refused before touching anything — a missing GitHub credential, a branch
    /// name git/`gh` would misread, a target branch equal to the base, or either
    /// half of the delivery-head policy ([`delivery_head_refusal`]): an
    /// **ambiguous head**, where the target branch already carries an open pull
    /// request into some other base that the push would silently add this
    /// round's commits to ([`ambiguous_head_refusal`]); or a **closed pull
    /// request** into this run's own base, which is somebody's decision that
    /// this branch should stop ([`closed_pr_refusal`]). Never retriable: nothing
    /// about running again changes any of them. Those two are the causes here
    /// that depend on remote STATE rather than on the invocation, so they are
    /// also the ones a `stage: "preflight"` reader is most likely to misread as
    /// a configuration mistake — clearing either means acting on the other pull
    /// request (closing the ambiguous one, reopening the closed one) or picking
    /// a different `--target-branch`.
    Preflight { reason: String },
    /// The worktree could not be committed. Not retriable — the same worktree
    /// will fail the same way.
    Commit { reason: String },
    /// The push was refused. `retriable` is `true` for the ordinary case (the
    /// branch moved, so the next round re-cuts from its head and replays) and
    /// `false` for credential/permission refusals.
    Push { reason: String, retriable: bool },
    /// A `gh` pull-request call failed. Usually retriable (a GitHub API blip);
    /// the work is not lost either way. Reached from two places, and the state
    /// of the remote differs between them: reconciliation runs after the push,
    /// so the commit is safely on the branch, while the head-ambiguity listing
    /// in preflight runs before it, so nothing has been committed or pushed at
    /// all. Both are the same verdict — retry the delivery — which is why they
    /// share a variant.
    Pr { reason: String, retriable: bool },
}

impl DeliveryFailure {
    /// The wire spelling of the stage — `preflight` | `commit` | `push` | `pr`.
    pub fn stage(&self) -> &'static str {
        match self {
            DeliveryFailure::Preflight { .. } => "preflight",
            DeliveryFailure::Commit { .. } => "commit",
            DeliveryFailure::Push { .. } => "push",
            DeliveryFailure::Pr { .. } => "pr",
        }
    }

    /// Whether the next round should just try the same delivery again.
    /// Preflight and commit failures are definitionally not retriable.
    pub fn retriable(&self) -> bool {
        match self {
            DeliveryFailure::Preflight { .. } | DeliveryFailure::Commit { .. } => false,
            DeliveryFailure::Push { retriable, .. } | DeliveryFailure::Pr { retriable, .. } => {
                *retriable
            }
        }
    }

    /// The human-readable reason, without the stage prefix.
    pub fn reason(&self) -> &str {
        match self {
            DeliveryFailure::Preflight { reason }
            | DeliveryFailure::Commit { reason }
            | DeliveryFailure::Push { reason, .. }
            | DeliveryFailure::Pr { reason, .. } => reason,
        }
    }
}

impl std::fmt::Display for DeliveryFailure {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{} failed: {}", self.stage(), self.reason())
    }
}

impl std::error::Error for DeliveryFailure {}

/// The GitHub operations PR delivery needs, behind a seam.
///
/// Delivery's interesting behavior — reconciliation, append-only pushes,
/// preflight refusal — is testable against a real temp git repo, but only if
/// the GitHub half can be faked. Same idiom as [`super::contract::derive_contract`],
/// which takes its `generate` closure injected for exactly this reason.
pub trait GitHubApi: Send + Sync {
    /// Fail with a message NAMING the missing credential when `gh` has no
    /// usable login.
    fn auth_status(&self) -> Result<(), GhError>;
    /// Every pull request whose head is `head_branch`, in any state.
    fn list_prs_for_head(&self, dir: &Path, head_branch: &str) -> Result<Vec<PrRecord>, GhError>;
    fn create_pr(
        &self,
        dir: &Path,
        head_branch: &str,
        base_branch: &str,
        title: &str,
        body: &str,
        draft: bool,
    ) -> Result<PrRecord, GhError>;
    /// Replace a pull request's body wholesale.
    fn set_pr_body(&self, dir: &Path, number: u64, body: &str) -> Result<(), GhError>;
}

/// The real `gh` CLI.
pub struct GhCli;

/// `gh auth status` args. A separate fn so the guard tests can read the exact
/// argv without a network round trip.
fn gh_auth_status_args() -> Vec<String> {
    vec!["auth".into(), "status".into()]
}

/// The repository `gh` must act on: the one [`push_args`] pushes to.
///
/// Every `gh` call used to run with only `current_dir(dir)`, leaving `gh` to
/// pick a base repository through its OWN remote preference order — `upstream`
/// before `github` before `origin`. On the ordinary contributor layout that
/// `gh repo fork --remote` creates (`origin = you/repo`,
/// `upstream = acme/repo`), the two halves of delivery then addressed different
/// repositories: the commit went to `you/repo`, the listing queried
/// `acme/repo` and found nothing, and `gh pr create` opened a pull request on
/// `acme/repo` — a repository the operator never named. Round N+1 queried the
/// wrong repository again, so "exactly one pull request per branch" did not hold
/// either.
///
/// `None` when `origin` is not a GitHub URL — a local path, as every test in
/// this file uses — in which case `gh`'s own resolution is left alone, because
/// there is nothing better to say.
fn gh_repo_args(dir: &Path) -> Vec<String> {
    match git(dir, &["remote", "get-url", "origin"])
        .ok()
        .and_then(|url| parse_github_repo_spec(url.trim()))
    {
        Some(spec) => vec!["--repo".into(), spec],
        None => Vec::new(),
    }
}

/// `<owner>/<name>` for github.com, `<host>/<owner>/<name>` elsewhere — the
/// `[HOST/]OWNER/REPO` spelling `gh --repo` accepts. `None` for anything that is
/// not a remote URL naming exactly one repository.
fn parse_github_repo_spec(url: &str) -> Option<String> {
    // `scheme://[user@]host[:port]/owner/name[.git]`, or scp-style
    // `[user@]host:owner/name[.git]`.
    let after_scheme = url.split_once("://").map(|(_, rest)| rest);
    let (host_part, path) = match after_scheme {
        Some(rest) => rest.split_once('/')?,
        // No scheme. A leading `/` or `.` is a local path, not a URL.
        None if url.starts_with('/') || url.starts_with('.') => return None,
        None => url.split_once(':')?,
    };
    let host = host_part
        .rsplit('@')
        .next()?
        .split(':')
        .next()?
        .to_ascii_lowercase();
    if host.is_empty() {
        return None;
    }
    let path = path
        .trim_matches('/')
        .strip_suffix(".git")
        .unwrap_or(path.trim_matches('/'));
    let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect();
    // Exactly owner/name. Anything else is a shape this does not understand, and
    // guessing at it would aim delivery somewhere nobody asked for.
    let [owner, name] = segments[..] else {
        return None;
    };
    if host == "github.com" {
        Some(format!("{owner}/{name}"))
    } else {
        Some(format!("{host}/{owner}/{name}"))
    }
}

/// `gh pr list` args for one head branch, all states.
fn gh_pr_list_args(head_branch: &str) -> Vec<String> {
    vec![
        "pr".into(),
        "list".into(),
        "--head".into(),
        head_branch.to_string(),
        "--state".into(),
        "all".into(),
        "--json".into(),
        // `isCrossRepository` is requested so `parse_pr_list` can DROP forks'
        // pull requests. `--head` filters on the head ref NAME alone, so an
        // outside contributor's `them/repo:goalpool/g_abc` is returned
        // alongside — or instead of — ours. Goal-derived branch names collide
        // trivially and forks are the normal contribution path on a public
        // repo, and the consequences are not cosmetic: the update path would
        // `gh pr edit` a third party's description away, and a stranger's
        // CLOSED pull request would park every future round on this branch.
        //
        // `baseRefName` is requested for the neighbouring reason: `--head`
        // filters on the head ref alone, and GitHub allows several open pull
        // requests from one head into DIFFERENT bases. See [`PrRecord::base`].
        "number,state,url,isDraft,isCrossRepository,baseRefName".into(),
        // `gh pr list` pages at 30 by default. That was cosmetic while this
        // listing only chose which pull request to reconcile; it is not now,
        // because the same listing is the sole input to
        // [`delivery_head_refusal`] — a truncated page hides an open pull
        // request into another base, or a closed one into this base, and the
        // harmful push proceeds exactly as it did before the guard existed.
        // Few pull requests share one head branch, so this is unlikely, but the
        // failure is silent and it defeats the guard entirely, which is the
        // wrong pair of properties to leave to a default.
        "--limit".into(),
        "100".into(),
    ]
}

/// `gh pr create` args. `--draft` appears exactly when `draft` is set.
fn gh_pr_create_args(
    head_branch: &str,
    base_branch: &str,
    title: &str,
    body: &str,
    draft: bool,
) -> Vec<String> {
    let mut args = vec![
        "pr".into(),
        "create".into(),
        "--head".into(),
        head_branch.to_string(),
        "--base".into(),
        base_branch.to_string(),
        "--title".into(),
        title.to_string(),
        "--body".into(),
        body.to_string(),
    ];
    if draft {
        args.push("--draft".into());
    }
    args
}

/// Git's force marker in a refspec, named exactly once.
///
/// Named so the append-only source guard can ban the bare char literal
/// everywhere else in this file. `validate_branch_name` legitimately has to talk
/// about `+` — it exists to REFUSE names starting with one — and a scanner that
/// trips on the code refusing a force marker is the failure this guard family
/// keeps rediscovering. The previous carve-out was "only complain when the same
/// LINE also says `format!`", and rustfmt splits `format!(` from its string
/// routinely (it does so in `validate_branch_name` itself), so the one spelling
/// a real force refspec would take — bind the plus to a name, interpolate it —
/// walked straight through. One allowlisted definition line is narrower and has
/// no such hole.
const FORCE_MARKER: char = '+';

/// The push refspec, built as a function so a test can assert on it directly.
///
/// `<sha>:refs/heads/<branch>` with **no leading `+`**. A leading plus is git's
/// force marker; without it git refuses any update that is not a fast-forward,
/// which is precisely the guarantee this delivery path sells. The full
/// `refs/heads/` prefix is spelled out so a branch name that also matches a tag
/// cannot redirect the push.
fn push_args(commit: &str, target_branch: &str) -> Vec<String> {
    vec![
        "push".into(),
        "origin".into(),
        format!("{commit}:refs/heads/{target_branch}"),
    ]
}

/// A failed `gh` invocation, with the two halves deliberately kept apart.
///
/// They are separated because one of them is model output and the other is not.
/// `gh` used to fail as a single string built from the whole argv —
/// `format!("gh {} failed: {}", args.join(" "), stderr)` — and for `create_pr`
/// and `set_pr_body` that argv carries `--title <subject_from_intent(intent)>`
/// and `--body <body>`, both derived from what the model wrote. That string went
/// straight to [`classify_pr_error`], which decides PERMANENCE by substring. So
/// a goal whose intent was `fix the 'no commits between' error on empty
/// deliveries` — ordinary work in this repo — produced a pull-request body
/// containing that exact phrase; an unrelated GitHub 502 on `gh pr edit` then
/// matched it inside the echoed `--body` argument, was classified permanent, and
/// parked a goal whose commit was already safely on the remote. That is this
/// file's oldest defect class (a predicate firing on a NAME) arriving through
/// the pull-request body, and no amount of care in the phrase list can close it
/// while the classifier is allowed to read the argv at all.
///
/// So: [`stderr`](Self::stderr) is what GitHub and `gh` said, and it is the only
/// thing the classifier is given. [`message`](Self::message) is for a human and
/// for the event stream, and may name whatever is useful.
#[derive(Debug, Clone)]
pub struct GhError {
    /// Human-readable, names the failing command. Never classified.
    pub message: String,
    /// The child's stderr, alone. The ONLY text [`classify_pr_error`] reads.
    pub stderr: String,
}

impl std::fmt::Display for GhError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.write_str(&self.message)
    }
}

impl GhError {
    /// A failure this module authored itself — `gh` missing, a timeout, a
    /// malformed response. There is no remote stderr, and the text is safe to
    /// classify precisely because nothing outside this process wrote it.
    fn local(message: impl Into<String>) -> Self {
        let message = message.into();
        Self {
            stderr: message.clone(),
            message,
        }
    }
}

/// Run `gh <args>` in `dir`, returning trimmed stdout.
///
/// Argument array only — never a shell string. Branch names, titles and bodies
/// all originate outside this process, and an argv has no quoting to escape.
pub(super) fn gh(dir: &Path, args: &[String]) -> Result<String, GhError> {
    let mut cmd = std::process::Command::new("gh");
    cmd.current_dir(dir).args(args);
    // `gh` shells out to git for some operations, and it has its own prompting.
    // Same reasoning as [`no_interactive_prompts`]: a headless round must fail,
    // not wait.
    no_interactive_prompts(&mut cmd);
    cmd.env("GH_PROMPT_DISABLED", "1");
    let out = run_capped(cmd).map_err(|e| match e {
        RunFailure::Spawn(io) if io.kind() == std::io::ErrorKind::NotFound => GhError::local(
            "`gh` not found on PATH — install the GitHub CLI (https://cli.github.com) \
             and authenticate it",
        ),
        // The argv is deliberately NOT interpolated: only the subcommand words,
        // which this module chose, reach the message.
        RunFailure::Spawn(io) => GhError::local(format!(
            "failed to run `gh {}`: {io}",
            gh_subcommand_shape(args)
        )),
        RunFailure::TimedOut(secs) => GhError::local(format!(
            "`gh {}` timed out after {secs}s and was killed",
            gh_subcommand_shape(args)
        )),
    })?;
    if out.status.success() {
        Ok(String::from_utf8_lossy(&out.stdout).trim().to_string())
    } else {
        let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string();
        Err(GhError {
            message: format!("gh {} failed: {stderr}", gh_subcommand_shape(args)),
            stderr,
        })
    }
}

/// The shape of a `gh` argv for a human: flags kept, operand VALUES elided.
///
/// Even the human-readable half declines to echo `--title` and `--body`. Their
/// values are model output, they can be thousands of lines, and a delivery
/// failure surfaces in an event stream and a `run_end.error` an orchestrator
/// logs. The flags themselves say everything a reader needs about which call
/// failed.
fn gh_subcommand_shape(args: &[String]) -> String {
    let mut out: Vec<String> = Vec::with_capacity(args.len());
    let mut elide_next = false;
    for arg in args {
        if std::mem::take(&mut elide_next) {
            out.push("<…>".to_string());
            continue;
        }
        if arg.starts_with("--") {
            elide_next = matches!(arg.as_str(), "--title" | "--body");
        }
        out.push(arg.clone());
    }
    out.join(" ")
}

/// Parse `gh pr list --json number,state,url,isDraft,isCrossRepository` output.
///
/// **Cross-repository entries are dropped here**, at the parse boundary, so that
/// no downstream branch can reach one. Reconciliation's whole vocabulary —
/// update the body, park on a close — is about a pull request whose head branch
/// is the one we just pushed to on `origin`. A fork's pull request merely shares
/// the ref NAME, which is all `--head` matches on; acting on it would rewrite a
/// stranger's description, or let a stranger's close park this branch, while the
/// branch we actually pushed still has no pull request at all.
fn parse_pr_list(raw: &str) -> Result<Vec<PrRecord>, String> {
    let value: serde_json::Value = serde_json::from_str(raw.trim())
        .map_err(|e| format!("could not parse `gh pr list` JSON: {e}"))?;
    let items = value
        .as_array()
        .ok_or_else(|| "`gh pr list` did not return a JSON array".to_string())?;
    let mut out = Vec::with_capacity(items.len());
    for item in items {
        // Absent field ⇒ same-repo. `gh` omits nothing it was asked for, and an
        // older `gh` that does not know the field would otherwise refuse the
        // whole call rather than answer without it.
        if item
            .get("isCrossRepository")
            .and_then(|c| c.as_bool())
            .unwrap_or(false)
        {
            continue;
        }
        let number = item
            .get("number")
            .and_then(|n| n.as_u64())
            .ok_or_else(|| "pull request entry has no numeric `number`".to_string())?;
        let raw_state = item
            .get("state")
            .and_then(|s| s.as_str())
            .ok_or_else(|| "pull request entry has no `state`".to_string())?;
        // gh reports MERGED distinctly from CLOSED. An unknown state is not
        // guessed at: treating it as closed would park a run that should have
        // proceeded.
        let state = match raw_state.to_ascii_uppercase().as_str() {
            "OPEN" => PrState::Open,
            "CLOSED" => PrState::ClosedUnmerged,
            "MERGED" => PrState::Merged,
            other => return Err(format!("unrecognized pull request state `{other}`")),
        };
        out.push(PrRecord {
            number,
            state,
            url: item
                .get("url")
                .and_then(|u| u.as_str())
                .unwrap_or_default()
                .to_string(),
            is_draft: item
                .get("isDraft")
                .and_then(|d| d.as_bool())
                .unwrap_or(false),
            // Required, not defaulted. A missing base cannot be guessed at:
            // every default is a claim that some pull request merges into the
            // branch this round targets, and acting on the wrong one is the
            // failure this field was added to stop. `gh` returns every field it
            // was asked for.
            base: item
                .get("baseRefName")
                .and_then(|b| b.as_str())
                .ok_or_else(|| "pull request entry has no `baseRefName`".to_string())?
                .to_string(),
        });
    }
    Ok(out)
}

/// Recover a pull request number from the URL `gh pr create` prints. `gh`
/// prints only the URL, and the number is its last path segment.
fn pr_number_from_url(url: &str) -> Result<u64, String> {
    url.trim()
        .rsplit('/')
        .find(|seg| !seg.is_empty())
        .and_then(|seg| seg.parse::<u64>().ok())
        .ok_or_else(|| format!("could not read a pull request number out of `{url}`"))
}

impl GitHubApi for GhCli {
    fn auth_status(&self) -> Result<(), GhError> {
        gh(Path::new("."), &gh_auth_status_args())
            .map(|_| ())
            .map_err(|e| GhError {
                message: format!(
                    "no usable GitHub credential: `gh auth status` failed. Authenticate with \
                 `gh auth login`, or set GH_TOKEN (or GITHUB_TOKEN) in this process's \
                 environment. Underlying error: {e}"
                ),
                stderr: e.stderr,
            })
    }

    fn list_prs_for_head(&self, dir: &Path, head_branch: &str) -> Result<Vec<PrRecord>, GhError> {
        let mut args = gh_repo_args(dir);
        args.extend(gh_pr_list_args(head_branch));
        parse_pr_list(&gh(dir, &args)?).map_err(GhError::local)
    }

    fn create_pr(
        &self,
        dir: &Path,
        head_branch: &str,
        base_branch: &str,
        title: &str,
        body: &str,
        draft: bool,
    ) -> Result<PrRecord, GhError> {
        let mut args = gh_repo_args(dir);
        args.extend(gh_pr_create_args(
            head_branch,
            base_branch,
            title,
            body,
            draft,
        ));
        let url = gh(dir, &args)?;
        Ok(PrRecord {
            number: pr_number_from_url(&url).map_err(GhError::local)?,
            state: PrState::Open,
            url: url.trim().to_string(),
            is_draft: draft,
            // `gh pr create --base <b>` succeeded, so this is what it created.
            base: base_branch.to_string(),
        })
    }

    fn set_pr_body(&self, dir: &Path, number: u64, body: &str) -> Result<(), GhError> {
        let mut args = gh_repo_args(dir);
        args.extend([
            "pr".to_string(),
            "edit".to_string(),
            number.to_string(),
            "--body".to_string(),
            body.to_string(),
        ]);
        gh(dir, &args).map(|_| ())
    }
}

/// Reject a branch name git or `gh` would misread.
///
/// Argument arrays already close command injection, but they do NOT stop a name
/// beginning with `-` from being parsed as a flag, and a name beginning with `+`
/// is a force marker in a refspec. Both are refused here rather than sanitized:
/// a delivery to a silently-renamed branch is worse than a delivery that stops.
/// Reject a branch name git or `gh` would misread, BEFORE it reaches a command
/// line.
///
/// Public because the check has to run at the caller's preflight, not only
/// inside `deliver_pr_with`. `car code-task` hands the raw `--target-branch`
/// and `--pr-base` to `git fetch`, `git worktree add` and `git merge` long
/// before delivery — and `git fetch` accepts `--upload-pack=<cmd>`, so a name of
/// that shape is executed on local and ssh transports. Validating only at
/// delivery also spends an entire model session first, which defeats the
/// early-refusal design the preflight exists for.
pub fn validate_branch_name(label: &str, name: &str) -> Result<(), String> {
    if name.is_empty() {
        return Err(format!("{label} is empty"));
    }
    if name.starts_with('-') {
        return Err(format!(
            "{label} `{name}` starts with '-', which git and gh would read as a flag"
        ));
    }
    if name.starts_with(FORCE_MARKER) {
        return Err(format!(
            "{label} `{name}` starts with `{FORCE_MARKER}`, git's force marker in a refspec"
        ));
    }
    if let Some(bad) = name
        .chars()
        .find(|c| c.is_whitespace() || c.is_control() || "~^:?*[]\\".contains(*c))
    {
        return Err(format!(
            "{label} `{name}` contains `{bad}`, which is not legal in a git ref name"
        ));
    }
    // Forms git rejects that the checks above let through. Without these they
    // surface as `fatal: invalid refspec` at push time, which matches neither
    // the refused nor the moved list in `classify_push_error` and so lands on
    // the retriable default — retried forever against a name that can never
    // work.
    if name.contains("..")
        || name.ends_with('/')
        || name.starts_with('/')
        || name.ends_with(".lock")
        // Per COMPONENT, not just the whole name: git rejects `a.lock/b` too,
        // and a name that only fails at push time lands on the retriable
        // default and is retried forever against something that can never work.
        || name.split('/').any(|c| c.ends_with(".lock"))
        || name.ends_with('.')
        || name.contains("//")
        || name.contains("@{")
        || name.split('/').any(|c| c.is_empty() || c.starts_with('.'))
    {
        return Err(format!("{label} `{name}` is not a legal git ref name"));
    }
    Ok(())
}

/// Turn a raw push error into a reason plus a retriability verdict.
///
/// The two verdicts drive opposite behaviour in the orchestrator — retriable
/// means "requeue, this is not a no-progress cycle", non-retriable means "park
/// the goal and quote the failure class" — so a misread in either direction is
/// expensive, and both were happening against git's real output:
///
/// - A bare `contains("403")` matched the digits inside an echoed commit SHA, so
///   an ordinary lost race was parked. A parked goal is never relaunched, which
///   also meant the round-N+1 recovery (merging `origin/<target>` back in) never
///   got the chance to run. Now anchored on HTTP phrasing, which a branch or
///   repository name cannot produce.
/// - Git's actual permission wording is `Permission to <repo> denied to <user>`,
///   which does NOT contain the contiguous phrase `permission denied`, so a
///   genuine refusal matched nothing and fell through to the retriable default —
///   retried forever. The old test passed only because it used wording git does
///   not emit.
/// - Git's lost-race wording (`cannot lock ref …`, `! [remote rejected] …`)
///   matched none of the retriable keywords either. Note `[remote rejected]`
///   does not contain `[rejected]` — the bracket sits before `remote`. The
///   verdict was right by accident, and the reason text said nothing useful.
///
/// **Refusals are tested first**, because git prints refusal and rejection
/// wording on the same line — `! [remote rejected] main -> main (pre-receive
/// hook declined)` is a branch-protection refusal, not a race — and "do not
/// retry" has to win when both are present. Getting that order wrong is how
/// adding the race patterns would have introduced a fresh misclassification.
fn classify_push_error(err: &str) -> (String, bool) {
    let low = err.to_ascii_lowercase();

    let refused = low.contains("permission denied")
        || (low.contains("permission to") && low.contains("denied"))
        // Anchored on HTTP phrasing. Token-anchoring stopped the digits inside
        // a commit SHA matching, but a branch or repository name is delimited
        // by `-` `/` `_` `.` — so `feature-403`,
        // `goalpool/g_403` and `repo-403.git` all present `403` as a standalone
        // token, and git echoes branch and remote in every push error. Since
        // refusals are tested before races, an ordinary lost race on such a name
        // was parked as a permission refusal.
        || low.contains("returned error: 403")
        || low.contains("status code 403")
        || low.contains("http 403")
        || low.contains("error 403")
        || low.contains("authentication failed")
        // Git's credential layer emits these two SYMMETRICALLY, and which one
        // you get depends on whether the remote URL already carries a username:
        // `https://github.com/...` asks for a Username, and
        // `https://someuser@github.com/...` — the form `gh auth setup-git` and
        // most CI clones leave behind — asks for a Password. Only the first was
        // matched, so on the second shape a headless box with no credential
        // helper produced `fatal: could not read Password for
        // 'https://someuser@github.com'`, which matched nothing in `refused`
        // and nothing in `moved` and landed on the retriable default: a full
        // model session requeued every round, forever, against a credential
        // that will never appear. `gh auth status` cannot catch it either — gh
        // holds its own token, which git does not use. Whole phrases, so no
        // branch or repository name can synthesise one.
        || low.contains("could not read username")
        || low.contains("could not read password")
        // What git says once `GIT_TERMINAL_PROMPT=0` is set (see
        // [`no_interactive_prompts`]): the prompt that used to hang forever now
        // returns instantly, and it has to be read as the permanent refusal it
        // is rather than retried.
        || low.contains("terminal prompts disabled")
        // SSH's own version of the same permanence: the host key is unknown or
        // has changed, and no number of retries alters that. Without it the
        // failure fell through to the retriable default.
        || low.contains("host key verification failed")
        // 401 — a revoked or expired token. Same HTTP anchoring as 403 above,
        // for the same reason: the bare digits appear inside SHAs and branch
        // names, the phrasings do not.
        || low.contains("returned error: 401")
        || low.contains("status code 401")
        || low.contains("http 401")
        || low.contains("error 401")
        // 404 — the ordinary shape of a token that cannot see this private
        // repository. GitHub says `remote: Repository not found.` over HTTPS
        // and `ERROR: Repository not found.` over SSH, and the `gh auth status`
        // preflight cannot catch it: gh IS authenticated, it just has no access
        // HERE. It matched nothing above and nothing in `moved`, so it landed
        // on the retriable default and the orchestrator requeued a full model
        // session, forever, against a wall that cannot move. The whole phrase is
        // matched, not `not found` alone, so `fatal: pathspec … not found` and
        // friends cannot claim it.
        || low.contains("repository not found")
        // A remote that is missing or is not a repository at all. Permanent by
        // the same argument, and `classify_pr_error` already calls the same
        // condition (`no such remote`) permanent — so leaving it retriable here
        // meant the two classifiers disagreed about one fact.
        || low.contains("does not appear to be a git repository")
        // Branch protection / server-side policy. Arrives wrapped in rejection
        // wording, so it must be recognised before the race patterns below.
        || low.contains("pre-receive hook declined")
        || low.contains("protected branch")
        // `! [remote rejected]` is git's GENERIC server-refusal line; the real
        // reason is in the parentheses. These are permanent, and classifying
        // them as races told the orchestrator "requeue, nothing was judged" —
        // a full model session per round against an identical wall, forever.
        || low.contains("refusing to allow")
        || low.contains("workflow' scope")
        || low.contains("shallow update not allowed")
        || low.contains("file size limit")
        // A directory/file ref-namespace collision emits BOTH `cannot lock ref`
        // and `failed to update ref`, so without this it matches the race set
        // and retries forever. It matters specifically here: goalpool branches
        // live under a `goalpool/` prefix, so one remote branch literally named
        // `goalpool` poisons every goal. Captured from git 2.50.1:
        //   remote: error: cannot lock ref 'refs/heads/goalpool/g_1':
        //   'refs/heads/goalpool' exists; cannot create 'refs/heads/goalpool/g_1'
        || low.contains("exists; cannot create")
        // A ruleset or secret-scanning block. GitHub prints this reason inside
        // the `[remote rejected]` parentheses, and it is permanent in the
        // strongest sense — the same commit can never be pushed.
        || low.contains("push declined")
        || mentions_github_policy_code(&low);
    if refused {
        return (
            format!("push refused for credential/permission reasons: {err}"),
            false,
        );
    }

    let moved = low.contains("non-fast-forward")
        || low.contains("fetch first")
        || low.contains("[rejected]")
        || low.contains("remote rejected")
        || low.contains("cannot lock ref")
        || low.contains("failed to update ref");
    if moved {
        return (
            format!(
                "non-fast-forward: the target branch moved since this worktree was cut \
                 (lost a push race) — {err}"
            ),
            true,
        );
    }

    // Everything else (DNS, TLS, a transient 5xx from the host) is a transport
    // problem the next round may well get past.
    (err.to_string(), true)
}

/// Whether the text carries one of GitHub's `GHNNN:` push-policy codes.
///
/// This was a hand-written list that stopped at `GH008:` — which left `GH013`,
/// the code for a repository-rule violation and therefore the one secret-
/// scanning push protection emits, matching nothing in the refusal set. It then
/// fell through to `remote rejected` in the race set and was reported to the
/// orchestrator as "the branch moved, requeue": a full model session per round,
/// re-pushing the identical commit at the identical wall, forever. That is the
/// precise failure the `[remote rejected]` handling above exists to prevent, and
/// push protection is on by default for public repositories, so the gap was not
/// on an exotic path. Matching the SHAPE closes the family — including whatever
/// code GitHub adds next — rather than the members someone happened to list.
///
/// Two anchors keep it off ordinary text, because this file has twice shipped a
/// predicate that fired on a name. The digits must be followed by `:`, which
/// `validate_branch_name` rejects in a ref name, so no branch, tag or remote can
/// synthesise one; and they must be preceded by a non-alphanumeric, so no longer
/// word ending in `gh` can either.
fn mentions_github_policy_code(low: &str) -> bool {
    let bytes = low.as_bytes();
    bytes.windows(6).enumerate().any(|(i, w)| {
        w[0] == b'g'
            && w[1] == b'h'
            && w[2..5].iter().all(u8::is_ascii_digit)
            && w[5] == b':'
            && (i == 0 || !bytes[i - 1].is_ascii_alphanumeric())
    })
}

/// Classify a `gh` reconciliation failure the way [`classify_push_error`]
/// classifies a push.
///
/// Every `map_err` on the reconciliation path used to construct
/// `DeliveryFailure::Pr { retriable: true }` without looking at the reason, so
/// the stage's retriability field carried no information — the variant's own doc
/// said "usually retriable", but no code path could produce the "usually not"
/// case. Permanent GitHub refusals therefore returned exit 2, which the
/// orchestrator reads as "re-run me, nothing was judged": a whole fresh session
/// and another push to reach the identical error.
///
/// The permanent set is deliberately narrow — an unrecognised failure stays
/// retriable, because a GitHub API blip genuinely is the common case and
/// wrongly parking a healthy goal is the worse error of the two.
fn classify_pr_error(err: &str) -> (String, bool) {
    let low = err.to_ascii_lowercase();
    // NOT `already exists`. GitHub returns
    // `a pull request for branch "X" into branch "Y" already exists: #7`
    // precisely when a pull request DOES exist for that head — and reaching
    // `create_pr` at all means the listing returned nothing, so the two
    // statements contradict each other: a list/create race, replication lag
    // right after a push, or a `--head` that did not match. In every one of
    // those the commit is already pushed and a usable pull request is open, and
    // the error even names its number. It is the one candidate whose own
    // trigger condition proves the goal can progress, so parking on it strands
    // a goal at the exact moment it has succeeded.
    let permanent = low.contains("no commits between")
        || low.contains("draft pull requests are not supported")
        || low.contains("must be a collaborator")
        || low.contains("no such remote")
        || low.contains("could not resolve to a repository");
    (err.to_string(), !permanent)
}

/// Turn a failed `gh` call into a typed delivery failure.
///
/// The one place the split in [`GhError`] is spent: the verdict is taken from
/// `stderr` — what GitHub actually said — and the human-readable reason from
/// `message`. Every reconciliation `map_err` goes through here so no call site
/// can quietly hand the classifier the argv again.
fn pr_failure(e: GhError) -> DeliveryFailure {
    let (_, retriable) = classify_pr_error(&e.stderr);
    DeliveryFailure::Pr {
        reason: e.message,
        retriable,
    }
}

/// The ambiguous-delivery-head refusal, as a reason string — `None` when the
/// head is unambiguous and delivery may proceed.
///
/// `prs` is everything `gh` reports for the head branch (see
/// [`GitHubApi::list_prs_for_head`]); a pull request parks delivery when it is
/// **open** into a base other than this run's, because pushing to
/// `target_branch` adds this round's commits to it as well.
///
/// Half of [`delivery_head_refusal`], which is what callers should use; kept
/// separate so each half can be read and tested against its own rule.
///
/// OPEN pull requests only, and only into a FOREIGN base. A merged one is inert
/// — it cannot gain commits. A closed one is handled by the other half,
/// [`closed_pr_refusal`], on a different rule: it is not about where the push
/// lands but about whose decision a close is. Cross-repository pull requests
/// never reach here — [`parse_pr_list`] drops them — so a fork whose branch
/// happens to share this name cannot park a legitimate run.
pub fn ambiguous_head_refusal(
    prs: &[PrRecord],
    target_branch: &str,
    base_branch: &str,
) -> Option<String> {
    let foreign_open: Vec<&PrRecord> = prs
        .iter()
        .filter(|p| p.state == PrState::Open && p.base != base_branch)
        .collect();
    if foreign_open.is_empty() {
        return None;
    }
    let described = foreign_open
        .iter()
        .map(|p| format!("#{} into `{}`", p.number, p.base))
        .collect::<Vec<_>>()
        .join(", ");
    let numbers = foreign_open
        .iter()
        .map(|p| format!("#{}", p.number))
        .collect::<Vec<_>>()
        .join(", ");
    let plural = if foreign_open.len() == 1 { "" } else { "s" };
    Some(format!(
        "branch `{target_branch}` already has open pull request{plural} {described} — not into \
         `{base_branch}`, this run's base. Pushing this round's commits to `{target_branch}` \
         would add them to {numbers} as well, because a pull request tracks its head branch. \
         Close {numbers}, or deliver to a different --target-branch"
    ))
}

/// The closed-pull-request refusal, as a reason string — `None` when no pull
/// request into this run's base is closed-unmerged and delivery may proceed.
///
/// Half of [`delivery_head_refusal`]. Scoped to `base_branch` because that is
/// the pull request this run would otherwise reconcile: a pull request from
/// this branch into some OTHER base being closed says nothing about this run,
/// and letting it park delivery would hand any stale pull request a veto over a
/// base it does not merge into.
///
/// **An OPEN pull request into this base suppresses the veto entirely.** GitHub
/// allows at most one open pull request per (head, base) pair, so when one
/// exists it is unambiguously the one this run reconciles, and its existence is
/// a later human decision than any close: a reviewer who closes #40 as the
/// wrong approach and opens #55 from the same branch into the same base has
/// carried the work forward, not stopped it. Vetoing there would park every
/// subsequent round on the number the reviewer deliberately superseded while
/// the live pull request went stale, and the only remedies offered would be
/// reopening the dead one or renaming `--target-branch`. This keeps the rule
/// that predates car#1055 — an open pull request into the base always wins —
/// and narrows the change to the arm that used to reopen.
///
/// **The runtime never reopens a pull request it did not close** — and it never
/// closes one, so it is never the party entitled to undo a close. Until
/// car#1055 the closed case was a reopen: a reviewer who read the pull request,
/// edited the body and closed it got it reopened on the next round with their
/// edits replaced wholesale, every round, and closing it did not stop the
/// runtime. Nothing available at this seam distinguishes "closed because it
/// went stale" from "closed by a human who read it and said no", and the second
/// reading is the one that must win: a close is the cheapest stop signal a
/// person has on an agent acting under their account, and it has to hold
/// without them deleting a branch.
///
/// Cross-repository pull requests never reach here — [`parse_pr_list`] drops
/// them — so a fork's closed pull request cannot park a legitimate run.
pub fn closed_pr_refusal(
    prs: &[PrRecord],
    target_branch: &str,
    base_branch: &str,
) -> Option<String> {
    // Superseded: reconciliation would update the open one, and that is a
    // later decision than the close. See the doc comment.
    if prs
        .iter()
        .any(|p| p.state == PrState::Open && p.base == base_branch)
    {
        return None;
    }
    let closed: Vec<&PrRecord> = prs
        .iter()
        .filter(|p| p.state == PrState::ClosedUnmerged && p.base == base_branch)
        .collect();
    if closed.is_empty() {
        return None;
    }
    let numbers = closed
        .iter()
        .map(|p| format!("#{}", p.number))
        .collect::<Vec<_>>()
        .join(", ");
    let plural = if closed.len() == 1 { "" } else { "s" };
    let was = if closed.len() == 1 { "was" } else { "were" };
    Some(format!(
        "pull request{plural} {numbers} from `{target_branch}` into `{base_branch}` {was} \
         closed — the runtime does not reopen a pull request it did not close. Reopen {numbers} \
         yourself to continue on this branch, or deliver to a different --target-branch"
    ))
}

/// The whole delivery-head policy: [`ambiguous_head_refusal`] first, then
/// [`closed_pr_refusal`]. `None` when the head is clear and delivery may
/// proceed.
///
/// `prs` is everything `gh` reports for the head branch (see
/// [`GitHubApi::list_prs_for_head`]).
///
/// Split out of [`deliver_pr_with`] so `car code-task`'s own preflight can apply
/// exactly this policy — same inputs, same words — before the model session
/// starts. Both call sites are needed and neither is redundant: the early one
/// makes an already-parked head cost no session; the delivery-time one is what
/// actually stands between the push and a pull request opened or closed DURING
/// the session. Callers take this function rather than either half, so the two
/// preflights cannot drift apart one rule at a time.
pub fn delivery_head_refusal(
    prs: &[PrRecord],
    target_branch: &str,
    base_branch: &str,
) -> Option<String> {
    ambiguous_head_refusal(prs, target_branch, base_branch)
        .or_else(|| closed_pr_refusal(prs, target_branch, base_branch))
}

/// Deliver the worktree as a pull request on a stable branch.
///
/// This is the headless third delivery mode, alongside [`publish_branch`] (raw
/// repo, local branch) and [`commit_to_main`] (managed project, ff-only into
/// `main`). It commits the worktree with the same `car-coder` authorship, pushes
/// that commit onto `target_branch` on `origin`, and reconciles exactly one pull
/// request for that branch.
///
/// **This is host code and it is NOT a hole in the coder's tool policy.** The
/// model's own shell tool still refuses `git push` through
/// [`super::policy`]'s `DenyGitRemoteMutation` inspector, and that inspector is
/// deliberately untouched by this function. The distinction is who is acting:
/// the inspector chain gates commands the MODEL proposes, which is why it must
/// deny remote mutation — a model that can push can exfiltrate a repository and
/// can escape every gate downstream of it. `deliver_pr` runs after
/// `evaluate_contract` has re-executed the checks and observed their exit codes
/// itself, on a commit the runtime made, to a branch the runtime named. Nothing
/// the model said is trusted here; only what the runtime verified. Weakening the
/// inspector to let the model push would be a hole. Pushing from the runtime,
/// after verification, is the mechanism the inspector exists to protect.
///
/// The guarantees, in order of how badly their absence would hurt:
///
/// 1. **Preflight before work.** A GitHub credential is checked before anything
///    is committed or pushed. Missing ⇒ [`DeliveryFailure::Preflight`] naming
///    the credential. This never returns success without a pull request.
/// 2. **Append/fast-forward only.** The push is a plain `<sha>:refs/heads/<b>`
///    refspec with no force flag and no leading `+` anywhere on this path (a
///    test asserts that against this file's own source text). A non-fast-forward
///    rejection is a retriable [`DeliveryFailure::Push`], and the remote is left
///    exactly as it was.
/// 3. **Base-branch update policy: MERGE.** When the base moves and the branch
///    needs updating, the policy for this delivery path is `git merge
///    origin/<base>` in the worktree — **never rebase, never force**. Rebase
///    rewrites commits a reviewer may already have read and a reviewer may
///    already have commented on; force-updating the branch can silently discard
///    a round's work. Merge is additive and cannot lose a commit. Note that
///    `deliver_pr` does not itself run the merge: it delivers what is in the
///    worktree, and bringing the base in is the round orchestrator's step
///    before the session starts. The policy is stated here because this is the
///    function whose invariants it protects.
/// 4. **One pull request per (branch, base), and at most one OPEN pull request
///    per branch.** Among the pull requests whose head is `target_branch` AND
///    whose base is `base_branch`: an open one receives the push
///    ([`PrAction::Updated`]); otherwise one is created ([`PrAction::Opened`]).
///    The pair, not the head alone, is GitHub's own constraint — see
///    [`PrRecord::base`] — and it is the right granularity for choosing WHICH
///    pull request to reconcile. It is not sufficient to decide whether
///    delivering is safe at all, because a pull request tracks its HEAD: the
///    push lands in every open pull request whose head is `target_branch`,
///    whatever base each merges into, and no base filter applied afterwards can
///    take that back. So a clear head is a PRECONDITION of delivery
///    ([`delivery_head_refusal`]), checked at preflight
///    ([`DeliveryFailure::Preflight`]) before anything is committed or pushed:
///    an open pull request from this branch into any OTHER base is refused,
///    naming each number and its base. A run with a changed `--pr-base`
///    therefore delivers only once the previous pull request is closed or
///    merged; with it still open the run is refused rather than quietly adding
///    this round's commits to it.
///
///    **A CLOSED pull request into this run's own base also parks the round,
///    and the runtime never reopens it.** The runtime never closes a pull
///    request, so it is never the party entitled to undo a close, and nothing
///    at this seam tells "closed because it went stale" from "closed by a
///    reviewer who read it and said no". Until car#1055 this path reopened the
///    pull request and replaced its body, so closing one did not stop the
///    runtime; now a close is a stop, and a human reopens it to continue. A
///    MERGED pull request parks nothing: it is inert, it cannot gain commits,
///    and its branch gets a fresh pull request for the next round.
/// 5. **Draft is a create-time decision only.** `draft` is honored when a pull
///    request is created and ignored otherwise. This function NEVER marks a
///    pull request ready for review — that is a car worker's judgment in a
///    later round, and a runtime that could flip it would be publishing
///    unreviewed work on the reviewer's behalf.
/// 6. **The body is refreshed, not appended.** On update the pull request body
///    is REPLACED with `body`. The body is the next fresh session's standing
///    context (it must describe the branch as it is now), and appending would
///    grow an unbounded log of stale round-by-round descriptions with the
///    current truth buried at the bottom.
///
/// `gh` runs in `repo`; git runs in `worktree`. They share the same remote
/// configuration, and the remote is `origin`.
pub fn deliver_pr(d: PrDelivery<'_>) -> Result<PrDeliveryOutcome, DeliveryFailure> {
    deliver_pr_with(d, &GhCli)
}

/// [`deliver_pr`] with the GitHub half injected. Tests drive this with a fake;
/// production goes through [`deliver_pr`].
pub fn deliver_pr_with(
    d: PrDelivery<'_>,
    gh_api: &dyn GitHubApi,
) -> Result<PrDeliveryOutcome, DeliveryFailure> {
    // --- 1. Preflight: nothing is touched until this passes. ---------------
    validate_branch_name("target branch", d.target_branch)
        .map_err(|reason| DeliveryFailure::Preflight { reason })?;
    validate_branch_name("base branch", d.base_branch)
        .map_err(|reason| DeliveryFailure::Preflight { reason })?;
    // The two names were validated independently and never compared, so `main`
    // passed as BOTH. `push_args` then builds `<sha>:refs/heads/main` and the
    // model's unreviewed output is published to the base branch; reconciliation
    // only fails afterwards (`gh pr create --head main --base main` → "no
    // commits between", which `classify_pr_error` marks permanent), so the goal
    // parked with the code already on `main` and an append-only path has no way
    // to take it back. `--pr-base` defaults to the repository's default branch,
    // so `--target-branch main` alone is enough to reach it. This is the one
    // preflight refusal that protects the whole point of the mode.
    if d.target_branch == d.base_branch {
        return Err(DeliveryFailure::Preflight {
            reason: format!(
                "target branch and base branch are both `{}`; delivering would push \
                 unreviewed work directly onto the base instead of opening a pull request",
                d.target_branch
            ),
        });
    }
    gh_api
        .auth_status()
        .map_err(|e| DeliveryFailure::Preflight {
            reason: e.to_string(),
        })?;

    // --- 2. Preflight, continued: the head must be unambiguous. ------------
    // The listing that reconciliation needs, taken BEFORE the push, because a
    // pull request tracks its HEAD branch: every commit pushed to
    // `target_branch` appears in every open pull request whose head it is,
    // whatever base each merges into. The base filter at step 5 stops this run
    // from rewriting a foreign pull request's body, but it cannot stop the
    // push, which by then has already landed the model's commits on it — a
    // human's open pull request from this branch into `release/2.1` silently
    // gained unreviewed work while the run reported opening a different one
    // into `main`. GitHub offers no way to push to a branch without updating
    // every open pull request tracking it, so refusal is the only thing that
    // prevents it; a warning would go to the JSONL stream a machine
    // orchestrator reads, not to the person whose release pull request moved.
    //
    // The same listing also carries the CLOSED case, on a different rule: a
    // closed pull request into this run's own base is somebody's decision that
    // this branch should stop, and the runtime does not reopen it (car#1055).
    // That refusal belongs HERE, at preflight, and not at reconciliation —
    // parking at step 5 would leave the round's commits pushed onto the branch
    // with no pull request describing them.
    //
    // This is the LOAD-BEARING call: `car code-task` applies the same policy in
    // its own preflight so an already-parked head costs no model session, but a
    // pull request can be opened — or closed — while the session runs, and only
    // this one is between that pull request and the push.
    let listed = gh_api
        .list_prs_for_head(d.repo, d.target_branch)
        .map_err(pr_failure)?;
    if let Some(reason) = delivery_head_refusal(&listed, d.target_branch, d.base_branch) {
        return Err(DeliveryFailure::Preflight { reason });
    }

    // --- 3. Commit. --------------------------------------------------------
    let commit = match commit_worktree(d.worktree, d.intent, d.contract) {
        Ok(CommitOutcome::Made(c)) => c,
        Ok(CommitOutcome::NothingToCommit) => head_beyond_base(d.worktree, d.base_branch)
            .map_err(|reason| DeliveryFailure::Commit { reason })?,
        Err(reason) => return Err(DeliveryFailure::Commit { reason }),
    };

    // --- 4. Push: append-only. ---------------------------------------------
    let args = push_args(&commit, d.target_branch);
    let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect();
    if let Err(e) = git(d.worktree, &arg_refs) {
        let (reason, retriable) = classify_push_error(&e);
        return Err(DeliveryFailure::Push { reason, retriable });
    }

    // --- 5. Reconcile exactly one pull request for the branch. -------------
    // Over the `listed` vector step 2 already holds. The push cannot create a
    // pull request, so re-listing here would return the same set.

    // Restricted to this run's BASE. GitHub's one-open-pull-request constraint
    // is per (head, base) pair, so `--head <target>` alone can return several
    // legal pull requests that merge into different branches. Reconciling over
    // all of them took the highest-numbered one and replaced its body: a pull
    // request opened from this same branch into `release/2.1` — by a human, or
    // by a round invoked with a different `--pr-base` — captured every
    // subsequent round, had a description this run did not author overwritten,
    // and left the pull request the orchestrator tracks with a permanently stale
    // body. Step 2 now refuses the OPEN case outright, but the filter is still
    // load-bearing for the merged one: without it a pull request merged into
    // ANOTHER base would suppress creating this run's own. Filtering also makes
    // `d.base_branch` mean something on the update path, where it was validated
    // in preflight and then silently ignored.
    let existing: Vec<&PrRecord> = listed.iter().filter(|p| p.base == d.base_branch).collect();

    // No closed arm: step 2 refused every closed pull request into this base
    // before the commit, except where an OPEN one into the same base supersedes
    // it — and that one is picked up by the open arm just below, which is the
    // behaviour the close rule was never meant to change (car#1055).

    // Highest number wins when several match: it is the most recent.
    let open = existing
        .iter()
        .filter(|p| p.state == PrState::Open)
        .max_by_key(|p| p.number);

    let (record, action) = if let Some(pr) = open {
        // The push already landed on it. Refresh the body; do NOT touch its
        // draft state.
        gh_api
            .set_pr_body(d.repo, pr.number, d.body)
            .map_err(pr_failure)?;
        ((*pr).clone(), PrAction::Updated)
    } else {
        // None, or only merged ones — a merged pull request's work has landed,
        // so the next round needs one of its own.
        let created = gh_api
            .create_pr(
                d.repo,
                d.target_branch,
                d.base_branch,
                &subject_from_intent(d.intent),
                d.body,
                d.draft,
            )
            .map_err(pr_failure)?;
        (created, PrAction::Opened)
    };

    Ok(PrDeliveryOutcome {
        branch: d.target_branch.to_string(),
        commit,
        pushed: true,
        pr_number: record.number,
        pr_url: record.url,
        pr_action: action,
        draft: record.is_draft,
    })
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::coder::contract::ContractCheck;

    fn contract() -> OutcomeContract {
        OutcomeContract {
            description: "x exists".into(),
            checks: vec![ContractCheck {
                name: "exists".into(),
                command: "test -f x.txt".into(),
                expect_exit_zero: true,
                output_contains: None,
                timeout_secs: 10,
            }],
        }
    }

    fn init_repo(dir: &Path) {
        for args in [
            vec!["init", "-q", "-b", "main"],
            vec![
                "-c",
                "user.name=t",
                "-c",
                "user.email=t@t",
                "commit",
                "-q",
                "--allow-empty",
                "-m",
                "init",
            ],
        ] {
            let out = std::process::Command::new("git")
                .arg("-C")
                .arg(dir)
                .args(&args)
                .output()
                .unwrap();
            assert!(
                out.status.success(),
                "{}",
                String::from_utf8_lossy(&out.stderr)
            );
        }
    }

    #[test]
    fn publishes_branch_without_touching_user_checkout() {
        let repo_dir = tempfile::tempdir().unwrap();
        let repo = repo_dir.path();
        init_repo(repo);

        // Provision a worktree the way a session does.
        let wt_base = tempfile::tempdir().unwrap();
        let config = car_multi::WorkspaceConfig::git_worktree_at(repo, wt_base.path());
        let ws = car_multi::AgentWorkspace::provision(&config, "coder-merge-test").unwrap();

        std::fs::write(ws.path().join("x.txt"), "made by coder").unwrap();
        let branch = publish_branch(
            repo,
            ws.path(),
            "abc12345",
            "create x.txt with content",
            &contract(),
        )
        .unwrap();
        assert_eq!(branch, "car/coder/abc12345");

        // The branch exists in the user's repo and contains the file…
        let show = git(repo, &["show", &format!("{branch}:x.txt")]).unwrap();
        assert_eq!(show, "made by coder");
        // …attributed to the coder…
        let author = git(repo, &["log", "-1", "--format=%an", &branch]).unwrap();
        assert_eq!(author.trim(), "car-coder");
        // …and the user's checkout is untouched.
        let status = git(repo, &["status", "--porcelain"]).unwrap();
        assert!(status.is_empty(), "user checkout dirtied: {status}");
        assert!(!repo.join("x.txt").exists());
    }

    #[test]
    fn clean_worktree_refuses_to_publish() {
        let repo_dir = tempfile::tempdir().unwrap();
        init_repo(repo_dir.path());
        let wt_base = tempfile::tempdir().unwrap();
        let config = car_multi::WorkspaceConfig::git_worktree_at(repo_dir.path(), wt_base.path());
        let ws = car_multi::AgentWorkspace::provision(&config, "coder-clean-test").unwrap();

        let err =
            publish_branch(repo_dir.path(), ws.path(), "def", "noop", &contract()).unwrap_err();
        assert!(err.contains("no changes"), "{err}");
    }

    #[test]
    fn long_intent_is_truncated_in_subject() {
        let repo_dir = tempfile::tempdir().unwrap();
        let repo = repo_dir.path();
        init_repo(repo);
        let wt_base = tempfile::tempdir().unwrap();
        let config = car_multi::WorkspaceConfig::git_worktree_at(repo, wt_base.path());
        let ws = car_multi::AgentWorkspace::provision(&config, "coder-long-test").unwrap();
        std::fs::write(ws.path().join("y.txt"), "y").unwrap();

        let long_intent = "a very ".repeat(40) + "long intent";
        let branch = publish_branch(repo, ws.path(), "fff", &long_intent, &contract()).unwrap();
        let subject = git(repo, &["log", "-1", "--format=%s", &branch]).unwrap();
        assert!(subject.trim().len() <= 72);
        assert!(subject.contains("..."));
    }

    #[test]
    fn commit_to_main_fast_forwards_the_checkout() {
        let repo_dir = tempfile::tempdir().unwrap();
        let repo = repo_dir.path();
        init_repo(repo);
        let wt_base = tempfile::tempdir().unwrap();
        let config = car_multi::WorkspaceConfig::git_worktree_at(repo, wt_base.path());
        let ws = car_multi::AgentWorkspace::provision(&config, "coder-main-test").unwrap();
        std::fs::write(ws.path().join("z.txt"), "managed").unwrap();

        let commit = commit_to_main(repo, ws.path(), "add z", &contract()).unwrap();
        // main fast-forwarded to the commit; the file is in the repo checkout.
        let head = git(repo, &["rev-parse", "HEAD"]).unwrap();
        assert_eq!(head.trim(), commit);
        assert_eq!(
            std::fs::read_to_string(repo.join("z.txt")).unwrap(),
            "managed"
        );
        // No coder branch.
        assert!(git(repo, &["branch", "--list", "car/coder/*"])
            .unwrap()
            .is_empty());
    }

    #[test]
    fn commit_to_main_errors_when_main_moved() {
        let repo_dir = tempfile::tempdir().unwrap();
        let repo = repo_dir.path();
        init_repo(repo);
        let wt_base = tempfile::tempdir().unwrap();
        let config = car_multi::WorkspaceConfig::git_worktree_at(repo, wt_base.path());
        let ws = car_multi::AgentWorkspace::provision(&config, "coder-moved-test").unwrap();
        std::fs::write(ws.path().join("a.txt"), "from session").unwrap();

        // Something commits to main AFTER the worktree was provisioned, so the
        // worktree's commit is no longer a fast-forward of main.
        std::fs::write(repo.join("b.txt"), "concurrent").unwrap();
        for args in [
            vec!["-c", "user.name=t", "-c", "user.email=t@t", "add", "-A"],
            vec![
                "-c",
                "user.name=t",
                "-c",
                "user.email=t@t",
                "commit",
                "-q",
                "-m",
                "concurrent",
            ],
        ] {
            assert!(std::process::Command::new("git")
                .arg("-C")
                .arg(repo)
                .args(&args)
                .output()
                .unwrap()
                .status
                .success());
        }

        let err = commit_to_main(repo, ws.path(), "add a", &contract()).unwrap_err();
        assert!(err.contains("fast-forward"), "{err}");
    }

    // --- Rename-aware, byte-safe changed paths ---------------------------

    /// **The bypass this parser exists to close.** Under `--name-only`, git
    /// reports only a rename's destination, so moving a file OUT of a directory
    /// erased every trace of that directory from the change record. Anything
    /// reasoning about which paths a session touched was told a rename is a
    /// creation.
    #[test]
    fn a_rename_reports_both_endpoints_not_just_the_destination() {
        let paths = parse_name_status_z("R100\0secrets/key.txt\0public_key.txt\0");
        assert!(
            paths.contains(&"secrets/key.txt".to_string()),
            "the source directory must not vanish: {paths:?}"
        );
        assert!(paths.contains(&"public_key.txt".to_string()), "{paths:?}");
        assert_eq!(paths.len(), 2);
    }

    /// A copy carries the same three-field shape as a rename.
    #[test]
    fn a_copy_also_reports_both_endpoints() {
        let paths = parse_name_status_z("C75\0src/a.rs\0src/b.rs\0");
        assert_eq!(paths, vec!["src/a.rs".to_string(), "src/b.rs".to_string()]);
    }

    /// Ordinary two-field entries, and the mixed stream — a rename's extra
    /// field must not desynchronize the ones that follow it.
    #[test]
    fn mixed_entries_stay_in_sync_after_a_rename() {
        let paths = parse_name_status_z("M\0src/a.rs\0R100\0old/x.rs\0new/x.rs\0A\0src/z.rs\0");
        assert_eq!(
            paths,
            vec![
                "new/x.rs".to_string(),
                "old/x.rs".to_string(),
                "src/a.rs".to_string(),
                "src/z.rs".to_string(),
            ]
        );
    }

    /// A newline inside a filename must not be able to forge an entry — the
    /// reason for `-z` over line-splitting.
    #[test]
    fn a_newline_in_a_filename_does_not_forge_an_entry() {
        let paths = parse_name_status_z("A\0we\nird.txt\0");
        assert_eq!(paths, vec!["we\nird.txt".to_string()]);
    }

    #[test]
    fn an_empty_diff_yields_no_paths() {
        assert!(parse_name_status_z("").is_empty());
    }

    /// `T` (type change, e.g. file -> symlink) and `U` (unmerged) are
    /// single-path entries. Verified against real git; pinned here because the
    /// whole parser rests on "only R and C carry two paths".
    #[test]
    fn type_change_and_unmerged_are_single_path_entries() {
        assert_eq!(
            parse_name_status_z("T\0src/link.txt\0M\0src/after.rs\0"),
            vec!["src/after.rs".to_string(), "src/link.txt".to_string()]
        );
        assert_eq!(
            parse_name_status_z("U\0conflict.txt\0"),
            vec!["conflict.txt".to_string()]
        );
    }

    /// **The desync guard.** A field that is not a status means git's format
    /// moved under us; continuing would read paths as statuses and emit
    /// plausible-looking fiction onto a reviewer's approval screen. Bail.
    #[test]
    fn an_unrecognized_status_bails_instead_of_desynchronizing() {
        // `Z9` is not in git's closed status set.
        assert!(parse_name_status_z("Z9\0a.txt\0b.txt\0").is_empty());
        // A well-formed prefix is kept; the garbage tail is dropped, not guessed.
        assert_eq!(
            parse_name_status_z("M\0good.rs\0Z9\0a.txt\0"),
            vec!["good.rs".to_string()]
        );
    }

    /// A rename whose destination field never arrived is a corrupt stream, not
    /// an entry to swallow.
    #[test]
    fn a_rename_missing_its_destination_bails() {
        assert_eq!(
            parse_name_status_z("R100\0only-one.txt\0"),
            vec!["only-one.txt".to_string()]
        );
    }

    /// End-to-end against real git: the rename bypass, reproduced and closed.
    #[test]
    fn stage_and_diff_sees_a_renamed_out_of_directory_source() {
        let dir = tempfile::tempdir().unwrap();
        let repo = dir.path();
        for args in [
            vec!["init", "-q", "."],
            vec!["config", "user.email", "t@t"],
            vec!["config", "user.name", "t"],
        ] {
            git(repo, &args).unwrap();
        }
        std::fs::create_dir(repo.join("secrets")).unwrap();
        std::fs::write(repo.join("secrets/key.txt"), "k").unwrap();
        git(repo, &["add", "-A"]).unwrap();
        git(repo, &["commit", "-qm", "init"]).unwrap();
        std::fs::rename(repo.join("secrets/key.txt"), repo.join("public_key.txt")).unwrap();

        let diff = stage_and_diff(repo, 64 * 1024).unwrap();
        assert!(
            diff.changed_paths.iter().any(|p| p.starts_with("secrets/")),
            "the source directory must appear: {:?}",
            diff.changed_paths
        );
        // Exactly the two endpoints — a parser that emitted status letters as
        // paths would also satisfy the assertion above.
        assert_eq!(
            diff.changed_paths,
            vec!["public_key.txt".to_string(), "secrets/key.txt".to_string()],
            "both endpoints, and nothing else"
        );
    }

    // --- PR delivery -----------------------------------------------------

    use std::path::PathBuf;
    use std::sync::Mutex;

    /// This file's own source text, for the guard tests below. The point of a
    /// source-level guard is that it fails on the WRITING of a forbidden token,
    /// not on some execution path a future test might not cover.
    const MERGE_RS_SOURCE: &str = include_str!("merge.rs");

    /// A scriptable stand-in for the `gh` CLI. Records every call so a test can
    /// assert on what delivery asked GitHub to do, not just on what came back.
    struct FakeGh {
        auth: Result<(), GhError>,
        prs: Mutex<Vec<PrRecord>>,
        calls: Mutex<Vec<String>>,
        next_number: Mutex<u64>,
        /// Per-method failure knobs. Reconciliation has three distinct `gh`
        /// seams and only `auth_status` could be made to fail, so the
        /// retriability each seam attaches to a failure — the whole reason
        /// `DeliveryFailure::Pr` carries the flag — was never exercised end to
        /// end. Each holds the STDERR the fake reports; the fake builds the
        /// human-readable half itself, so a test cannot accidentally hand the
        /// classifier a message.
        fail_list: Mutex<Option<String>>,
        fail_create: Mutex<Option<String>>,
        fail_set_body: Mutex<Option<String>>,
    }

    /// Production lines carrying a char-literal plus — a force refspec waiting
    /// to be interpolated. Returns 1-based line numbers.
    ///
    /// A FUNCTION rather than an inline loop so it can be pointed at sources
    /// that do violate it. Lives in the test module because its own needle is
    /// spelled with the very character it bans, and a production copy would
    /// report itself.
    ///
    /// Scoped to production: the test module below legitimately constructs
    /// `+`-prefixed names to prove `validate_branch_name` rejects them. The one
    /// production line allowed to name the character is [`FORCE_MARKER`]'s
    /// definition.
    fn force_char_offenders(src: &str) -> Vec<usize> {
        let production = src.split_once("mod tests {").map(|(h, _)| h).unwrap_or(src);
        let needle: String = ['\'', '+', '\''].iter().collect();
        production
            .lines()
            .enumerate()
            .filter(|(_, line)| line.contains(needle.as_str()))
            .filter(|(_, line)| !line.contains("FORCE_MARKER: char"))
            .map(|(i, _)| i + 1)
            .collect()
    }

    /// The error shape a real `gh` failure has: a message that names the
    /// command (and may echo argv) plus the remote's own stderr.
    fn gh_err(message: &str, stderr: &str) -> GhError {
        GhError {
            message: message.to_string(),
            stderr: stderr.to_string(),
        }
    }

    impl FakeGh {
        fn ok() -> Self {
            Self {
                auth: Ok(()),
                prs: Mutex::new(Vec::new()),
                calls: Mutex::new(Vec::new()),
                next_number: Mutex::new(101),
                fail_list: Mutex::new(None),
                fail_create: Mutex::new(None),
                fail_set_body: Mutex::new(None),
            }
        }

        fn no_credential() -> Self {
            Self {
                auth: Err(gh_err(
                    "no usable GitHub credential: `gh auth status` failed. Authenticate with \
                     `gh auth login`, or set GH_TOKEN (or GITHUB_TOKEN)",
                    "gh: To get started with GitHub CLI, please run: gh auth login",
                )),
                ..Self::ok()
            }
        }

        fn with_prs(prs: Vec<PrRecord>) -> Self {
            Self {
                prs: Mutex::new(prs),
                ..Self::ok()
            }
        }

        /// `gh pr edit --body` fails with this stderr.
        fn failing_set_body(stderr: &str) -> Self {
            let me = Self::ok();
            *me.fail_set_body.lock().unwrap() = Some(stderr.to_string());
            me
        }

        fn calls(&self) -> Vec<String> {
            self.calls.lock().unwrap().clone()
        }
    }

    /// The message half a real `gh` failure carries — command shape plus
    /// stderr. Built here so the fake's message is never the bare stderr, which
    /// would make "the classifier read stderr" trivially true.
    fn fake_gh_failure(command: &str, stderr: &str, body: &str) -> GhError {
        gh_err(
            &format!("gh {command} --body {body} failed: {stderr}"),
            stderr,
        )
    }

    impl GitHubApi for FakeGh {
        fn auth_status(&self) -> Result<(), GhError> {
            self.calls.lock().unwrap().push("auth_status".into());
            self.auth.clone().map_err(|e| e.clone())
        }

        fn list_prs_for_head(&self, _dir: &Path, head: &str) -> Result<Vec<PrRecord>, GhError> {
            self.calls.lock().unwrap().push(format!("list {head}"));
            if let Some(stderr) = self.fail_list.lock().unwrap().clone() {
                return Err(gh_err(&format!("gh pr list failed: {stderr}"), &stderr));
            }
            Ok(self.prs.lock().unwrap().clone())
        }

        fn create_pr(
            &self,
            _dir: &Path,
            head: &str,
            base: &str,
            title: &str,
            body: &str,
            draft: bool,
        ) -> Result<PrRecord, GhError> {
            self.calls.lock().unwrap().push(format!(
                "create head={head} base={base} draft={draft} title={title} body={body}"
            ));
            if let Some(stderr) = self.fail_create.lock().unwrap().clone() {
                return Err(fake_gh_failure("pr create", &stderr, body));
            }
            let mut n = self.next_number.lock().unwrap();
            let record = PrRecord {
                number: *n,
                state: PrState::Open,
                url: format!("https://github.com/acme/repo/pull/{n}"),
                is_draft: draft,
                base: base.to_string(),
            };
            *n += 1;
            self.prs.lock().unwrap().push(record.clone());
            Ok(record)
        }

        fn set_pr_body(&self, _dir: &Path, number: u64, body: &str) -> Result<(), GhError> {
            self.calls
                .lock()
                .unwrap()
                .push(format!("set_body {number} {body}"));
            if let Some(stderr) = self.fail_set_body.lock().unwrap().clone() {
                return Err(fake_gh_failure("pr edit", &stderr, body));
            }
            Ok(())
        }
    }

    /// A bare `origin` plus a working clone that pushes to it — the smallest
    /// thing that can tell a fast-forward from a rejection for real.
    struct Fixture {
        origin: PathBuf,
        repo: PathBuf,
        wt_base: PathBuf,
        _dirs: Vec<tempfile::TempDir>,
    }

    fn fixture() -> Fixture {
        let origin_dir = tempfile::tempdir().unwrap();
        let repo_dir = tempfile::tempdir().unwrap();
        let wt_dir = tempfile::tempdir().unwrap();
        let origin = origin_dir.path().to_path_buf();
        let repo = repo_dir.path().to_path_buf();

        git(&origin, &["init", "-q", "--bare", "-b", "main"]).unwrap();
        git(&repo, &["init", "-q", "-b", "main"]).unwrap();
        git(&repo, &["config", "user.name", "t"]).unwrap();
        git(&repo, &["config", "user.email", "t@t"]).unwrap();
        std::fs::write(repo.join("README.md"), "seed").unwrap();
        git(&repo, &["add", "-A"]).unwrap();
        git(&repo, &["commit", "-qm", "seed"]).unwrap();
        git(
            &repo,
            &["remote", "add", "origin", origin.to_str().unwrap()],
        )
        .unwrap();
        git(&repo, &["push", "-q", "origin", "main"]).unwrap();

        Fixture {
            origin,
            repo,
            wt_base: wt_dir.path().to_path_buf(),
            _dirs: vec![origin_dir, repo_dir, wt_dir],
        }
    }

    impl Fixture {
        /// Cut a session worktree from `from_ref`, the way a round does.
        fn cut(&self, name: &str, from_ref: &str) -> PathBuf {
            let path = self.wt_base.join(name);
            git(
                &self.repo,
                &[
                    "worktree",
                    "add",
                    "--detach",
                    "-q",
                    path.to_str().unwrap(),
                    from_ref,
                ],
            )
            .unwrap();
            path
        }

        /// The commit `origin` has on a branch, or `None` when it has no such
        /// branch.
        fn origin_head(&self, branch: &str) -> Option<String> {
            git(
                &self.origin,
                &["rev-parse", "--verify", &format!("refs/heads/{branch}")],
            )
            .ok()
            .map(|s| s.trim().to_string())
        }
    }

    fn delivery<'a>(
        f: &'a Fixture,
        worktree: &'a Path,
        contract: &'a OutcomeContract,
        target: &'a str,
        draft: bool,
        body: &'a str,
    ) -> PrDelivery<'a> {
        PrDelivery {
            repo: &f.repo,
            worktree,
            target_branch: target,
            base_branch: "main",
            draft,
            intent: "make x exist",
            contract,
            body,
        }
    }

    const TARGET: &str = "goalpool/g_abc123";

    #[test]
    fn a_green_delivery_pushes_the_commit_and_opens_one_pr() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "made by coder").unwrap();

        let gh = FakeGh::ok();
        let out =
            deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "round 1 body"), &gh).unwrap();

        assert!(out.pushed);
        assert_eq!(out.branch, TARGET);
        assert_eq!(out.pr_action, PrAction::Opened);
        assert_eq!(out.pr_number, 101);
        assert!(out.draft, "a draft was requested at create time");

        // The remote really has the commit, with the file and the coder identity.
        assert_eq!(f.origin_head(TARGET).as_deref(), Some(out.commit.as_str()));
        assert_eq!(
            git(&f.origin, &["show", &format!("refs/heads/{TARGET}:x.txt")]).unwrap(),
            "made by coder"
        );
        assert_eq!(
            git(
                &f.origin,
                &[
                    "log",
                    "-1",
                    "--format=%an <%ae>",
                    &format!("refs/heads/{TARGET}")
                ]
            )
            .unwrap()
            .trim(),
            "car-coder <coder@parslee.ai>"
        );
        // Preflight ran before anything else.
        assert_eq!(gh.calls()[0], "auth_status");
    }

    #[test]
    fn a_second_delivery_appends_to_the_same_branch_and_the_same_pr() {
        let f = fixture();
        let c = contract();

        let wt1 = f.cut("s1", "main");
        std::fs::write(wt1.join("x.txt"), "round one").unwrap();
        let gh1 = FakeGh::ok();
        let first =
            deliver_pr_with(delivery(&f, &wt1, &c, TARGET, true, "round 1 body"), &gh1).unwrap();

        // Round 2 cuts from the branch's current head, as the contract requires.
        git(&f.repo, &["fetch", "-q", "origin"]).unwrap();
        let wt2 = f.cut("s2", &format!("origin/{TARGET}"));
        std::fs::write(wt2.join("y.txt"), "round two").unwrap();

        // GitHub already has the open PR from round 1.
        let gh2 = FakeGh::with_prs(vec![PrRecord {
            number: 101,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/101".into(),
            is_draft: true,
            base: "main".into(),
        }]);
        let second =
            deliver_pr_with(delivery(&f, &wt2, &c, TARGET, true, "round 2 body"), &gh2).unwrap();

        assert_eq!(second.pr_action, PrAction::Updated);
        assert_eq!(second.pr_number, 101);
        assert_ne!(second.commit, first.commit);
        assert!(
            !gh2.calls().iter().any(|c| c.starts_with("create")),
            "a second PR must never be created for the same branch: {:?}",
            gh2.calls()
        );
        assert!(gh2.calls().iter().any(|c| c == "set_body 101 round 2 body"));

        // One linear history: seed -> round 1 -> round 2, no merges.
        assert_eq!(
            git(
                &f.origin,
                &["rev-list", "--count", &format!("refs/heads/{TARGET}")]
            )
            .unwrap()
            .trim(),
            "3"
        );
        assert_eq!(
            git(
                &f.origin,
                &[
                    "rev-list",
                    "--count",
                    "--merges",
                    &format!("refs/heads/{TARGET}")
                ]
            )
            .unwrap()
            .trim(),
            "0"
        );
        // Round 1's commit is still an ancestor — nothing was replaced.
        assert!(git(
            &f.origin,
            &["merge-base", "--is-ancestor", &first.commit, &second.commit]
        )
        .is_ok());
        // And exactly two branches exist on the remote: main and the target.
        let mut branches: Vec<String> = git(
            &f.origin,
            &["for-each-ref", "--format=%(refname:short)", "refs/heads/"],
        )
        .unwrap()
        .lines()
        .map(|l| l.to_string())
        .collect();
        branches.sort();
        assert_eq!(branches, vec![TARGET.to_string(), "main".to_string()]);
    }

    #[test]
    fn a_non_fast_forward_is_retriable_and_leaves_the_remote_alone() {
        let f = fixture();
        let c = contract();

        let wt1 = f.cut("s1", "main");
        std::fs::write(wt1.join("x.txt"), "round one").unwrap();
        let first =
            deliver_pr_with(delivery(&f, &wt1, &c, TARGET, true, "b1"), &FakeGh::ok()).unwrap();

        // A session that was cut from the OLD base — its commit is not a
        // descendant of what the branch now points at.
        let wt2 = f.cut("stale", "main");
        std::fs::write(wt2.join("z.txt"), "stale round").unwrap();
        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 101,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/101".into(),
            is_draft: true,
            base: "main".into(),
        }]);
        let err = deliver_pr_with(delivery(&f, &wt2, &c, TARGET, true, "b2"), &gh).unwrap_err();

        assert_eq!(err.stage(), "push");
        assert!(err.retriable(), "{err}");
        assert!(
            matches!(
                err,
                DeliveryFailure::Push {
                    retriable: true,
                    ..
                }
            ),
            "{err:?}"
        );
        assert!(
            err.reason().contains("non-fast-forward"),
            "the reason must name the condition: {}",
            err.reason()
        );
        // The remote still points at round 1.
        assert_eq!(
            f.origin_head(TARGET).as_deref(),
            Some(first.commit.as_str())
        );
        // And no PR work was attempted after the push failed.
        assert!(
            !gh.calls().iter().any(|c| c.starts_with("create")),
            "{:?}",
            gh.calls()
        );
    }

    #[test]
    fn a_missing_credential_fails_preflight_and_touches_nothing() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "never delivered").unwrap();

        let gh = FakeGh::no_credential();
        let err = deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "body"), &gh).unwrap_err();

        assert_eq!(err.stage(), "preflight");
        assert!(!err.retriable(), "a missing credential is not retriable");
        assert!(
            err.reason().contains("GH_TOKEN") || err.reason().contains("gh auth"),
            "the failure must name the missing credential: {}",
            err.reason()
        );
        // Nothing committed, nothing pushed.
        assert!(
            f.origin_head(TARGET).is_none(),
            "the remote gained a branch"
        );
        assert!(
            !git(&wt, &["status", "--porcelain"])
                .unwrap()
                .trim()
                .is_empty(),
            "the worktree was committed despite the preflight failure"
        );
        assert_eq!(gh.calls(), vec!["auth_status".to_string()]);
    }

    /// car#1055: a closed pull request into this run's base is somebody's
    /// decision that this branch should stop. It used to be reopened; now it
    /// parks the round at PREFLIGHT, before the commit and before the push,
    /// because parking at reconciliation would leave the round's commits on the
    /// branch with no pull request describing them.
    #[test]
    fn a_closed_unmerged_pull_request_parks_the_round() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "again").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 55,
            state: PrState::ClosedUnmerged,
            url: "https://github.com/acme/repo/pull/55".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        let err = deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "revived"), &gh).unwrap_err();

        assert!(
            matches!(err, DeliveryFailure::Preflight { .. }),
            "a closed pull request is refused before the commit: {err:?}"
        );
        assert!(
            !err.retriable(),
            "retrying changes nothing — a human reopens #55 or picks another target branch"
        );
        // Actionable: it names the number and says who reopens it.
        assert!(
            err.reason().contains("#55") && err.reason().contains("Reopen"),
            "{}",
            err.reason()
        );

        // The listing is all it asked GitHub for: no reopen, no create, no body
        // rewrite.
        assert_eq!(
            gh.calls(),
            vec!["auth_status".to_string(), format!("list {TARGET}")]
        );
        // And nothing local moved either.
        assert_eq!(f.origin_head(TARGET), None, "nothing may be pushed");
        assert!(
            !git(&wt, &["status", "--porcelain"])
                .unwrap()
                .trim()
                .is_empty(),
            "the worktree was committed despite the preflight failure"
        );
    }

    /// The filed scenario (car#1055): a reviewer read the pull request, EDITED
    /// its body, and closed it. The next round must not hand `set_pr_body` this
    /// round's `body` and wipe those edits — which is what a reopen did, every
    /// round, for as long as the branch existed.
    #[test]
    fn a_reviewers_edited_body_survives_the_next_round() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "round two").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 40,
            state: PrState::ClosedUnmerged,
            url: "https://github.com/acme/repo/pull/40".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        let err = deliver_pr_with(
            delivery(&f, &wt, &c, TARGET, false, "round two's generated body"),
            &gh,
        )
        .unwrap_err();

        assert_eq!(err.stage(), "preflight");
        assert!(
            !gh.calls().iter().any(|c| c.starts_with("set_body")),
            "the reviewer's description was rewritten: {:?}",
            gh.calls()
        );
        // Stronger than "no set_body": this round's generated body never
        // reached GitHub on ANY call, so nothing could have replaced the
        // reviewer's text by another route. That the pull request is also never
        // made live again is asserted by the exact-calls check in
        // `a_closed_unmerged_pull_request_parks_the_round` — there is no reopen
        // seam left on `GitHubApi` for a call to come from.
        assert!(
            !gh.calls()
                .iter()
                .any(|c| c.contains("round two's generated body")),
            "this round's body reached GitHub: {:?}",
            gh.calls()
        );
    }

    /// car#1055 must not veto a pull request a human has already SUPERSEDED.
    /// A reviewer closes #40 ("wrong approach") and opens #55 from the same
    /// branch into the same base to carry the work forward. `gh` reports both.
    /// The close is the older decision; the open one is what this round
    /// reconciles, exactly as it did before the close rule existed. Parking
    /// here would name a number the reviewer deliberately retired and leave #55
    /// permanently stale.
    #[test]
    fn a_superseding_open_pull_request_beats_the_closed_one() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "carried forward").unwrap();

        let gh = FakeGh::with_prs(vec![
            PrRecord {
                number: 40,
                state: PrState::ClosedUnmerged,
                url: "https://github.com/acme/repo/pull/40".into(),
                is_draft: false,
                base: "main".into(),
            },
            PrRecord {
                number: 55,
                state: PrState::Open,
                url: "https://github.com/acme/repo/pull/55".into(),
                is_draft: false,
                base: "main".into(),
            },
        ]);

        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "round body"), &gh).unwrap();

        assert_eq!(out.pr_action, PrAction::Updated);
        assert_eq!(out.pr_number, 55, "the live pull request receives the push");
        assert!(out.pushed);
        assert!(
            gh.calls().iter().any(|c| c.starts_with("set_body 55")),
            "{:?}",
            gh.calls()
        );
        // #40 is somebody's closed decision and stays that way: nothing is
        // created to paper over it and nothing touches it.
        assert!(
            !gh.calls().iter().any(|c| c.starts_with("create")),
            "{:?}",
            gh.calls()
        );
        assert!(
            !gh.calls().iter().any(|c| c.contains(" 40 ")),
            "{:?}",
            gh.calls()
        );
    }

    /// The unit-level statement of the same rule, on the half that carries it:
    /// the close veto is suppressed by an open pull request into THIS base, and
    /// only that base.
    #[test]
    fn the_close_veto_is_suppressed_only_by_an_open_pr_into_the_same_base() {
        let closed = PrRecord {
            number: 40,
            state: PrState::ClosedUnmerged,
            url: "u40".into(),
            is_draft: false,
            base: "main".into(),
        };
        let open_same = PrRecord {
            number: 55,
            state: PrState::Open,
            url: "u55".into(),
            is_draft: false,
            base: "main".into(),
        };
        let open_other = PrRecord {
            base: "release/2.1".into(),
            ..open_same.clone()
        };

        assert!(
            closed_pr_refusal(std::slice::from_ref(&closed), TARGET, "main").is_some(),
            "a lone closed pull request still parks the round"
        );
        assert!(
            closed_pr_refusal(&[closed.clone(), open_same], TARGET, "main").is_none(),
            "the open pull request into `main` supersedes the close"
        );
        assert!(
            closed_pr_refusal(&[closed, open_other], TARGET, "main").is_some(),
            "an open pull request into ANOTHER base says nothing about this base"
        );
    }

    #[test]
    fn a_merged_pr_does_not_block_a_new_one() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "next chapter").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 9,
            state: PrState::Merged,
            url: "https://github.com/acme/repo/pull/9".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "fresh"), &gh).unwrap();

        assert_eq!(out.pr_action, PrAction::Opened);
        // A merge is this branch's work landing, not a decision against it: it
        // neither parks the round nor suppresses the next pull request.
        assert!(gh.calls().iter().any(|c| c.starts_with("create")));
    }

    #[test]
    fn an_updated_pr_never_has_its_draft_state_flipped() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "more work").unwrap();

        // A car worker marked it ready in an earlier round.
        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 77,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/77".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        // Delivery still asks for a draft — it must be ignored on an existing PR.
        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "body"), &gh).unwrap();

        assert_eq!(out.pr_action, PrAction::Updated);
        assert!(
            !out.draft,
            "delivery must report the PR's real state, not re-draft a ready PR"
        );
    }

    #[test]
    fn a_clean_worktree_redelivers_head_after_an_earlier_push_failure() {
        let f = fixture();
        let c = contract();

        // Round 1: commit lands locally, push is impossible (no such remote).
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();
        git(
            &f.repo,
            &["remote", "set-url", "origin", "/nonexistent/nope.git"],
        )
        .unwrap();
        let err =
            deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "b"), &FakeGh::ok()).unwrap_err();
        assert_eq!(err.stage(), "push");
        // A remote that is not a repository is permanent: re-running spends a
        // whole model session to reach the identical wall.
        assert!(
            !err.retriable(),
            "a missing remote cannot be fixed by trying again: {err}"
        );
        let committed = git(&wt, &["rev-parse", "HEAD"]).unwrap().trim().to_string();

        // Round 2 reuses the kept workspace. It is now CLEAN, but the work is
        // real and must be delivered rather than refused.
        git(
            &f.repo,
            &["remote", "set-url", "origin", f.origin.to_str().unwrap()],
        )
        .unwrap();
        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "b"), &FakeGh::ok()).unwrap();
        assert_eq!(out.commit, committed);
        assert_eq!(f.origin_head(TARGET).as_deref(), Some(committed.as_str()));
    }

    #[test]
    fn a_branch_name_that_looks_like_a_flag_is_refused_at_preflight() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "x").unwrap();

        for bad in [
            "--upload-pack=touch /tmp/pwn",
            "goalpool/../../etc",
            "has space",
        ] {
            let err =
                deliver_pr_with(delivery(&f, &wt, &c, bad, true, "b"), &FakeGh::ok()).unwrap_err();
            assert_eq!(err.stage(), "preflight", "for `{bad}`: {err}");
        }
        // A leading '+' would be a force marker in a refspec.
        let plus = format!("{}goalpool/x", '+');
        let err =
            deliver_pr_with(delivery(&f, &wt, &c, &plus, true, "b"), &FakeGh::ok()).unwrap_err();
        assert_eq!(err.stage(), "preflight", "{err}");
    }

    // --- Source-level guards ---------------------------------------------

    /// **The guarantee this delivery path sells.** A force-push on a shared
    /// delivery branch silently discards a round's work and rewrites commits a
    /// reviewer already read. The needles are assembled at runtime so this
    /// test's own source cannot trip it.
    #[test]
    fn no_force_push_token_appears_anywhere_in_this_file() {
        let dashes = "-".repeat(2);
        let plain = format!("{dashes}{}", "force");
        let lease = format!("{plain}-with-lease");
        // A force refspec is any string literal whose FIRST character is `+`.
        //
        // This needle used to be the literal `+refs`, which cannot see the force
        // refspec this file would actually produce: refspecs here are built by
        // interpolation ([`push_args`] formats the commit and branch in), so a
        // forced one is a format string beginning with a plus followed by
        // `{commit}:refs/heads/...` — no `+refs` substring anywhere in it. The
        // guard read as rigorous and could not fail on the one spelling that
        // matters. Anchoring on quote-then-plus catches both.
        //
        // Assembled from chars, like the needles above, so it does not appear
        // verbatim in this file and match its own definition.
        let plus_refspec: String = ['"', '+'].iter().collect();
        // `-f` is the commonest spelling of a force push and was not covered.
        // Quoted, so the bare two characters cannot match inside prose.
        let short_flag: String = ['"', '-', 'f', '"'].iter().collect();
        for needle in [
            plain.as_str(),
            lease.as_str(),
            plus_refspec.as_str(),
            short_flag.as_str(),
        ] {
            assert!(
                !MERGE_RS_SOURCE.contains(needle),
                "`{needle}` must appear nowhere on the delivery path"
            );
        }

        // A char-literal plus reaches the same force refspec by another route —
        // interpolating `'+'` as the first field of a format string — which the
        // quote-then-plus needle cannot see.
        //
        // Scoped to PRODUCTION lines that build a string. Both narrowings are
        // load-bearing, and each was found by this check firing on correct code:
        // a bare `'+'` is how `the_push_refspec_is_append_only_and_fully_
        // qualified` asserts no arg starts with one, and the branch-validation
        // test deliberately CONSTRUCTS a `+`-prefixed name to prove preflight
        // rejects it. Banning the spelling everywhere would fail on the two
        // tests that prove the property — the failure mode this guard family
        // keeps rediscovering.
        let production = MERGE_RS_SOURCE
            .split_once("mod tests {")
            .map(|(head, _)| head)
            .unwrap_or(MERGE_RS_SOURCE);
        // Rebase, in BOTH spellings. merge.rs's delivery path must never
        // rebase either — the base-update policy is merge precisely so a
        // reviewer's already-read commits are not rewritten — and a bare
        // `"rebase"` needle cannot see `"--rebase"`, two dashes sitting between
        // the quote and the `r`.
        for tok in [
            ['"', 'r', 'e', 'b', 'a', 's', 'e', '"']
                .iter()
                .collect::<String>(),
            ['"', '-', '-', 'r', 'e', 'b', 'a', 's', 'e', '"']
                .iter()
                .collect::<String>(),
        ] {
            assert!(
                !production.contains(tok.as_str()),
                "delivery must never rebase: `{tok}`"
            );
        }

        assert_eq!(
            force_char_offenders(MERGE_RS_SOURCE),
            Vec::<usize>::new(),
            "a char-literal plus on the delivery path is a force refspec"
        );
    }

    /// The guard above, run against sources that DO contain the violation.
    ///
    /// This is the half that was missing, and its absence is the defect: the
    /// scanner used to require `format!` and the char literal on the SAME line,
    /// and rustfmt routinely splits `format!(` from its string — it does so in
    /// this very file, in `validate_branch_name`. So the one spelling a real
    /// force refspec would take here (bind the plus to a name, interpolate it)
    /// tripped no needle, and no test could have noticed, because the guard was
    /// only ever run against a file that does not violate it. A green
    /// assertion over one input says nothing about whether the check can fail.
    #[test]
    fn the_force_char_scanner_catches_the_spellings_that_slipped_past_it() {
        // The exact bypass: rustfmt splits the macro from its string, and the
        // plus is bound to a name on a line with no `format!` on it.
        let wrapped = "fn f() {\n    let plus = '+';\n    let refspec = format!(\n        \
                       \"{plus}{commit}:refs/heads/{branch}\"\n    );\n}\n";
        assert!(
            !force_char_offenders(wrapped).is_empty(),
            "a plus bound to a name and interpolated is still a force refspec"
        );
        // The single-line spelling the old scanner did catch, still caught.
        let inline = "fn f() { let r = format!(\"{}{commit}:refs/heads/{b}\", '+'); }\n";
        assert!(!force_char_offenders(inline).is_empty());
        // And it stops at the test module, which legitimately constructs
        // `+`-prefixed names to prove `validate_branch_name` rejects them.
        let only_in_tests = "fn f() {}\nmod tests {\n    let plus = '+';\n}\n";
        assert!(force_char_offenders(only_in_tests).is_empty());
    }

    /// Marking a pull request ready for review is a car worker's judgment in a
    /// later round, never the runtime's. A string literal `ready` in this file
    /// would be a `gh pr ready` argument.
    #[test]
    fn delivery_never_marks_a_pull_request_ready_for_review() {
        // Spelled in pieces so this test's own source does not contain the token.
        let ready_arg = String::from('"') + "read" + "y" + "\"";
        assert!(
            !MERGE_RS_SOURCE.contains(&ready_arg),
            "the runtime must not flip a pull request out of draft"
        );
    }

    /// Every subprocess on this path is an argument array. A shell string would
    /// let a branch name carry a command.
    #[test]
    fn no_shell_invocation_appears_on_the_delivery_path() {
        for needle in ["Command::new(\"sh\")", "Command::new(\"bash\")"] {
            assert!(
                !MERGE_RS_SOURCE.contains(needle),
                "`{needle}` would reintroduce shell interpolation"
            );
        }
    }

    // --- Argument builders -------------------------------------------------

    #[test]
    fn draft_adds_the_draft_flag_and_nothing_else_does() {
        let with = gh_pr_create_args("h", "main", "t", "b", true);
        assert!(with.contains(&"--draft".to_string()), "{with:?}");
        let without = gh_pr_create_args("h", "main", "t", "b", false);
        assert!(!without.contains(&"--draft".to_string()), "{without:?}");
        // The body and title are passed as their own argv entries.
        assert!(with.windows(2).any(|w| w[0] == "--body" && w[1] == "b"));
        assert!(with.windows(2).any(|w| w[0] == "--title" && w[1] == "t"));
    }

    #[test]
    fn the_push_refspec_is_append_only_and_fully_qualified() {
        let args = push_args("abc123", "goalpool/g_1");
        assert_eq!(
            args,
            vec![
                "push".to_string(),
                "origin".to_string(),
                "abc123:refs/heads/goalpool/g_1".to_string(),
            ]
        );
        let plus = '+';
        assert!(
            !args.iter().any(|a| a.starts_with(plus)),
            "a leading plus is git's force marker: {args:?}"
        );
    }

    /// The listing is the sole input to [`ambiguous_head_refusal`], so an
    /// explicit `--limit` above `gh`'s default page of 30 is part of the guard:
    /// a truncated page hides an open pull request into another base and the
    /// push it exists to prevent goes ahead silently.
    #[test]
    fn pr_list_asks_for_every_state_of_one_head_branch() {
        let args = gh_pr_list_args("goalpool/g_1");
        assert!(args
            .windows(2)
            .any(|w| w[0] == "--head" && w[1] == "goalpool/g_1"));
        assert!(args.windows(2).any(|w| w[0] == "--state" && w[1] == "all"));
        let limit: u32 = args
            .windows(2)
            .find(|w| w[0] == "--limit")
            .map(|w| w[1].parse().expect("--limit is a number"))
            .expect("an explicit --limit, or gh silently pages at 30");
        assert!(
            limit >= 100,
            "the head listing must not be truncated below 100: {args:?}"
        );
    }

    // --- gh output parsing -------------------------------------------------

    #[test]
    fn pr_list_json_maps_merged_apart_from_closed() {
        let prs = parse_pr_list(
            r#"[{"number":1,"state":"OPEN","url":"u1","isDraft":true,"baseRefName":"main"},
                {"number":2,"state":"CLOSED","url":"u2","isDraft":false,"baseRefName":"main"},
                {"number":3,"state":"MERGED","url":"u3","isDraft":false,"baseRefName":"main"}]"#,
        )
        .unwrap();
        assert_eq!(prs[0].state, PrState::Open);
        assert!(prs[0].is_draft);
        assert_eq!(prs[1].state, PrState::ClosedUnmerged);
        assert_eq!(prs[2].state, PrState::Merged);
    }

    #[test]
    fn an_unknown_pr_state_is_an_error_not_a_guess() {
        assert!(
            parse_pr_list(r#"[{"number":1,"state":"WAT","url":"u","baseRefName":"main"}]"#)
                .is_err()
        );
        // A missing base is an error too, for the same reason: every default
        // would be a claim that this pull request merges into the branch this
        // round targets.
        assert!(
            parse_pr_list(r#"[{"number":1,"state":"OPEN","url":"u","isDraft":false}]"#).is_err(),
            "a pull request with no baseRefName cannot be reconciled against a base"
        );
        assert!(parse_pr_list("not json").is_err());
        assert!(parse_pr_list("[]").unwrap().is_empty());
    }

    #[test]
    fn a_pr_number_is_read_off_the_created_url() {
        assert_eq!(
            pr_number_from_url("https://github.com/acme/repo/pull/4821\n").unwrap(),
            4821
        );
        assert!(pr_number_from_url("https://github.com/acme/repo").is_err());
    }

    #[test]
    fn push_errors_split_into_retriable_and_not() {
        let (reason, retriable) = classify_push_error("! [rejected] abc -> b (non-fast-forward)");
        assert!(retriable);
        assert!(reason.contains("non-fast-forward"));

        let (_, retriable) = classify_push_error("remote: Permission denied to car-coder.");
        assert!(!retriable, "a permission refusal must not be retried");

        // git's REAL output, trailer included. "Please make sure you have the
        // correct access rights / and the repository exists." is the generic
        // die_initial_contact trailer printed on EVERY transport failure — a
        // DNS miss, an ssh timeout, a laptop off wifi — so matching it as a
        // credential refusal parked green rounds on ordinary network blips.
        // Reproduced against a scratch repo with an unresolvable ssh remote.
        let (_, retriable) = classify_push_error(
            "ssh: Could not resolve hostname github.com: nodename nor servname provided, \
             or not known\nfatal: Could not read from remote repository.\n\nPlease make \
             sure you have the correct access rights\nand the repository exists.",
        );
        assert!(retriable, "transport failures are worth another round");
    }

    /// A commit SHA that happens to contain the digits `403` must NOT be read as
    /// an HTTP 403.
    ///
    /// This is not a hypothetical. Git echoes the competing SHA on a lost push
    /// race, a 40-char hex string contains `403` roughly 0.9% of the time, and
    /// git prints two of them — so on the order of 2% of push failures. A bare
    /// `contains("403")` turns those into `retriable: false`, which exits 3, the
    /// park-the-goal path. The consequence is worse than the rate suggests: a
    /// parked goal is never relaunched, so the round-N+1 recovery that merges
    /// `origin/<target>` back in never runs, and a perfectly healthy race
    /// becomes a dead goal needing a human.
    #[test]
    fn a_sha_containing_403_is_not_mistaken_for_a_permission_refusal() {
        let (reason, retriable) = classify_push_error(
            "! [remote rejected] goalpool/g_1 -> goalpool/g_1 (cannot lock ref \
             'refs/heads/goalpool/g_1': is at a4973f07ba3815b8d45b86a7e9633d9fbc5e4403 \
             but expected b1c2d3e4f5061728394a5b6c7d8e9f0011223344)",
        );
        assert!(
            retriable,
            "a lost push race is retriable; the digits 403 inside a SHA are not an HTTP status"
        );
        assert!(
            reason.contains("race") || reason.contains("moved"),
            "the reason must name what actually happened: {reason}"
        );
    }

    /// Git's lost-race wording must be classified because it is recognised, not
    /// because it happens to fall through to the retriable default.
    ///
    /// Note `[remote rejected]` does NOT contain the substring `[rejected]` —
    /// the bracket sits before `remote`. Until this was added, none of the
    /// retriable keywords matched git's actual lost-race output, so the verdict
    /// was right by luck and the reason text said nothing useful.
    #[test]
    fn gits_lost_race_wording_is_recognised_rather_than_defaulted() {
        for message in [
            "! [remote rejected] main -> main (failed to update ref)",
            "error: cannot lock ref 'refs/heads/goalpool/g_1': is at aaa but expected bbb",
            "! [rejected] abc -> b (fetch first)",
        ] {
            let (reason, retriable) = classify_push_error(message);
            assert!(retriable, "{message}");
            assert!(
                reason.contains("moved") || reason.contains("race"),
                "the reason must tell an operator the branch moved, not echo git: {reason}"
            );
        }
    }

    /// `403` in a BRANCH or REPOSITORY name is not an HTTP status.
    ///
    /// Token-anchoring stopped the digits inside a commit SHA matching, but a
    /// name is delimited by `-` `/` `_` `.`, so `feature-403` presents `403` as
    /// a standalone token — and git echoes the branch and the remote URL in
    /// every push error. Since refusals are tested before races, an ordinary
    /// lost race on such a branch was parked as a permission refusal with a
    /// reason that was factually wrong.
    #[test]
    fn a_403_in_a_branch_or_repo_name_is_not_a_permission_refusal() {
        for message in [
            "! [rejected] abc1234 -> feature-403 (non-fast-forward)",
            "! [rejected] abc -> goalpool/g_403 (fetch first)",
            "! [remote rejected] x -> release_403 (failed to update ref)",
        ] {
            let (reason, retriable) = classify_push_error(message);
            assert!(retriable, "must stay a retriable race: {message}");
            assert!(
                reason.contains("moved") || reason.contains("race"),
                "{reason}"
            );
        }
        // A repository name carrying 403 must not poison an unrelated failure.
        let (_, retriable) =
            classify_push_error("fatal: unable to access 'https://github.com/org/repo-403.git/'");
        assert!(retriable, "a repo name is not a status code");
    }

    /// Permanent refusals that arrive inside `[remote rejected]` are not races.
    ///
    /// `! [remote rejected]` is git's GENERIC server-refusal line; the reason is
    /// in the parentheses. Classifying the whole class as a lost race returned
    /// exit 2 — "requeue, nothing was judged" — so each of these burned a full
    /// model session per round against an identical wall, forever.
    #[test]
    fn permanent_server_refusals_inside_remote_rejected_are_not_retried() {
        for message in [
            "! [remote rejected] b -> b (refusing to allow an OAuth App to create or update \
             workflow '.github/workflows/x.yml' without 'workflow' scope)",
            "! [remote rejected] b -> b (shallow update not allowed)",
            "remote: error: GH001: Large files detected. File exceeds GitHub's file size limit \
             of 100.00 MB",
            // Directory/file ref collision — captured from git 2.50.1 against a
            // scratch bare remote, not paraphrased. Emits BOTH `cannot lock
            // ref` and `failed to update ref`, so it reads as a lost race
            // unless refusals are matched first.
            "remote: error: cannot lock ref 'refs/heads/goalpool/g_1': \
             'refs/heads/goalpool' exists; cannot create 'refs/heads/goalpool/g_1'\n \
             ! [remote rejected] HEAD -> goalpool/g_1 (failed to update ref)",
        ] {
            let (_, retriable) = classify_push_error(message);
            assert!(!retriable, "permanent, must not be retried: {message}");
        }
    }

    /// A repository-rule violation — which is what secret-scanning push
    /// protection emits — is permanent, and the enumerated code list stopped
    /// short of it.
    ///
    /// `GH013` is the code GitHub returns for a ruleset block, and push
    /// protection is on by default for public repositories, so this is not an
    /// exotic path: a test fixture that reads like a token is enough. Walking
    /// the old predicates against the text below, nothing in the refusal set
    /// matched — the list ended at `GH008:` — so it fell through to
    /// `remote rejected` in the race set and was reported to the orchestrator as
    /// "the branch moved, requeue". Every following round then burned a full
    /// model session to re-push the identical commit at the identical wall,
    /// which is the exact failure `classify_push_error` says it exists to stop.
    ///
    /// Captured shape, not paraphrased — the `remote:` lines and the trailing
    /// `! [remote rejected]` are what git prints for a declined push.
    #[test]
    fn a_repository_rule_violation_is_permanent_not_a_lost_race() {
        let (reason, retriable) = classify_push_error(
            "remote: error: GH013: Repository rule violations found for \
             refs/heads/goalpool/g_1.\nremote:\nremote: - GITHUB PUSH PROTECTION\nremote:   \
             —— GitHub Personal Access Token ————————————————\nremote:\n \
             ! [remote rejected] goalpool/g_1 -> goalpool/g_1 (push declined due to \
             repository rule violations)\nerror: failed to push some refs to \
             'https://github.com/o/r.git'",
        );
        assert!(
            !retriable,
            "a ruleset block cannot be got past by pushing the same commit again"
        );
        assert!(
            !reason.contains("race") && !reason.contains("moved"),
            "and it must not be described as a lost push race: {reason}"
        );

        // The generic decline line carries the same verdict on its own — a
        // caller that only kept git's stderr summary still gets it right.
        let (_, retriable) = classify_push_error(
            "! [remote rejected] b -> b (push declined due to repository rule violations)",
        );
        assert!(!retriable);

        // Codes above the old ceiling are the family, not one member.
        for code in ["GH009", "GH011", "GH013"] {
            let (_, retriable) =
                classify_push_error(&format!("remote: error: {code}: blocked by policy"));
            assert!(!retriable, "{code} must be read as a policy refusal");
        }
    }

    /// …and the code shape must not fire on a NAME.
    ///
    /// This file has twice shipped a predicate that matched text git echoes for
    /// unrelated reasons — `403` inside a commit SHA, then inside a branch name.
    /// The colon is the anchor: `validate_branch_name` rejects `:` in a ref, so
    /// `GH013:` cannot arrive from a branch, a tag, or a remote name.
    #[test]
    fn a_github_code_in_a_branch_name_is_not_a_policy_refusal() {
        for message in [
            "! [rejected] abc -> fix-gh013-secret-scanning (non-fast-forward)",
            "! [remote rejected] x -> gh001 (failed to update ref)",
            "fatal: unable to access 'https://github.com/org/gh013.git/'",
        ] {
            let (_, retriable) = classify_push_error(message);
            assert!(retriable, "a name is not a status code: {message}");
        }
    }

    /// A permanent `gh` failure must not be reported as a retriable blip.
    #[test]
    fn permanent_pr_reconciliation_failures_are_not_retriable() {
        for message in [
            "GraphQL: No commits between main and goalpool/g_1",
            "GraphQL: Draft pull requests are not supported in this repository",
        ] {
            let (_, retriable) = classify_pr_error(message);
            assert!(!retriable, "permanent, must not be retried: {message}");
        }

        // `already exists` is RETRIABLE, and this assertion is the whole point:
        // the error is only ever returned when a pull request for that head
        // exists, while reaching `create_pr` means the listing found none — a
        // race, replication lag, or a head mismatch, all of which resolve on the
        // next round's re-list. Parking there strands a goal whose commit is
        // already pushed and whose pull request number is IN THE ERROR TEXT.
        let (_, retriable) = classify_pr_error(
            "a pull request for branch \"goalpool/g_1\" into branch \"main\" already exists: #7",
        );
        assert!(
            retriable,
            "the error proves a usable pull request exists; the next round adopts it"
        );
        // An unrecognised failure stays retriable: an API blip is the common
        // case, and wrongly parking a healthy goal is the worse error.
        let (_, retriable) = classify_pr_error("502 Bad Gateway");
        assert!(retriable);
    }

    /// **The E2E defect.** A caller's paragraph break decides the commit subject.
    ///
    /// goalpool writes a short summary, a blank line, then a pointer to the goal
    /// brief. `subject_from_intent` flattened newlines to spaces BEFORE the
    /// length check, so the break was gone before it could do anything and the
    /// subject read `"<summary>  Read the goal brief /tmp/..."`, truncated at 72.
    /// Leading with a short first line was not enough, because the flattening
    /// happened first.
    ///
    /// Asserted on the delivered COMMIT — `%s` and `%b` — rather than on the
    /// helper, because the subject a reviewer sees is the thing that matters and
    /// the helper is only how it gets there.
    #[test]
    fn a_paragraph_break_in_the_intent_ends_the_commit_subject() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("subject-check", "main");
        std::fs::write(wt.join("greet.js"), "console.log('hi');\n").unwrap();

        let summary = "Goal: implement greet() in the scratch repo";
        let body_text = "Read the goal brief /tmp/car-e2e-2/gp-home/logs/brief.md and \
                         follow it exactly. This pointer text must not reach the subject.";
        let intent = format!("{summary}\n\n{body_text}");

        let out = deliver_pr_with(
            PrDelivery {
                repo: &f.repo,
                worktree: &wt,
                target_branch: "goalpool/g_subject",
                base_branch: "main",
                draft: true,
                intent: &intent,
                contract: &c,
                body: "b",
            },
            &FakeGh::ok(),
        )
        .expect("delivery succeeds");

        let subject = git(&f.repo, &["log", "-1", "--format=%s", &out.commit]).unwrap();
        assert_eq!(
            subject.trim(),
            summary,
            "the subject must be exactly the first paragraph"
        );
        assert!(
            !subject.contains("goal brief"),
            "the pointer text must not ride along: {subject}"
        );

        // Nothing is lost — the rest is in the body, where git expects it.
        let body = git(&f.repo, &["log", "-1", "--format=%b", &out.commit]).unwrap();
        assert!(
            body.contains("goal brief"),
            "the remainder belongs in the body: {body}"
        );
    }

    /// An intent with no blank line still flattens and truncates at 72, exactly
    /// as before — a single-paragraph caller sees no change.
    #[test]
    fn a_single_paragraph_intent_still_truncates_as_before() {
        let short = "Add a --verbose flag to the CLI";
        assert_eq!(subject_from_intent(short), short);

        let long = "Add a --verbose flag to the export subcommand and thread it through \
                    every downstream call site so the whole pipeline reports progress";
        let subject = subject_from_intent(long);
        assert!(subject.ends_with("..."), "{subject}");
        assert!(subject.len() <= 72, "len {}: {subject}", subject.len());
        assert!(!subject.contains('\n'));

        // A multi-line first paragraph is still one line in the subject.
        assert_eq!(
            subject_from_intent("wrapped over\ntwo lines\n\nbody here"),
            "wrapped over two lines"
        );
    }

    /// **The E2E defect.** Runtime bookkeeping must not reach the delivered tree.
    ///
    /// The ownership marker used to be written into the WORKING TREE, and
    /// `commit_worktree` stages with `git add -A`, so every pull request this
    /// command opened carried a `.car-code-task` file in its diff for a human to
    /// review. It now lives in the worktree's gitdir, which `git add` cannot
    /// reach — so this asserts against the pushed tree rather than against the
    /// mechanism, and would catch any future bookkeeping file the same way.
    #[test]
    fn no_runtime_bookkeeping_reaches_the_delivered_tree() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("marker-check", "main");

        // Whatever the CLI writes to claim a workspace lives here, alongside
        // anything else git keeps per-worktree.
        let gitdir = std::fs::read_to_string(wt.join(".git"))
            .ok()
            .and_then(|m| {
                m.trim()
                    .strip_prefix("gitdir:")
                    .map(|g| g.trim().to_string())
            })
            .map(PathBuf::from)
            .expect("a worktree's .git is a file holding a gitdir pointer");
        std::fs::write(gitdir.join("car-code-task"), "claimed\n").unwrap();

        std::fs::write(wt.join("greet.js"), "console.log('hi');\n").unwrap();
        let out = deliver_pr_with(
            delivery(&f, &wt, &c, "goalpool/g_marker", true, "b"),
            &FakeGh::ok(),
        )
        .expect("delivery succeeds");

        let tree = git(&f.origin, &["ls-tree", "-r", "--name-only", &out.commit]).unwrap();
        assert!(
            tree.contains("greet.js"),
            "the actual work must be there: {tree}"
        );
        for bookkeeping in ["car-code-task", ".car-code-task"] {
            assert!(
                !tree.contains(bookkeeping),
                "`{bookkeeping}` is runtime bookkeeping and must not reach a reviewed diff: {tree}"
            );
        }
    }

    /// A clean worktree whose HEAD is still the base has nothing to deliver.
    ///
    /// The fallback exists for a re-delivery — a green round whose PUSH failed,
    /// re-pushing its existing commit. It could not tell that from a FIRST round
    /// that was green with an empty diff, where HEAD is the base tip: the push
    /// then succeeded, created a stray remote branch pointing at the base, and
    /// `gh pr create` failed with "No commits between …" — retried forever, with
    /// junk branches accumulating on the remote.
    #[test]
    fn a_clean_worktree_at_the_base_is_refused_rather_than_pushed_empty() {
        let f = fixture();
        let c = contract();
        // Cut from main and change NOTHING: HEAD is still the base tip, which is
        // exactly the first-round-green-with-an-empty-diff case.
        let wt = f.cut("empty-round", "main");
        let base_tip = git(&wt, &["rev-parse", "HEAD"]).unwrap().trim().to_string();

        let err = deliver_pr_with(
            delivery(&f, &wt, &c, "goalpool/g_empty", true, "b"),
            &FakeGh::ok(),
        )
        .expect_err("an empty round must not open a pull request");
        assert_eq!(err.stage(), "commit", "{err}");
        assert!(
            err.reason().contains("nothing to deliver"),
            "{}",
            err.reason()
        );
        assert!(err.reason().contains(&base_tip), "{}", err.reason());
        assert!(!err.retriable(), "an empty round will be empty again");

        // Nothing reached the remote: no stray branch pointing at the base.
        assert!(
            git(
                &f.origin,
                &[
                    "rev-parse",
                    "--verify",
                    "--quiet",
                    "refs/heads/goalpool/g_empty"
                ]
            )
            .is_err(),
            "no stray branch may be created for an empty round"
        );
    }

    /// A refusal wrapped in rejection wording must read as a refusal.
    ///
    /// Branch protection arrives as `! [remote rejected] … (pre-receive hook
    /// declined)`, which matches the lost-race patterns. Adding those patterns
    /// without ordering refusals first would have turned every protected-branch
    /// rejection into an infinite retry — a fresh bug introduced by the fix.
    #[test]
    fn a_policy_refusal_wrapped_in_rejection_wording_is_not_a_race() {
        let (_, retriable) =
            classify_push_error("! [remote rejected] main -> main (pre-receive hook declined)");
        assert!(!retriable, "branch protection is not worth retrying");
    }

    /// The permission signals still fire on the real thing.
    #[test]
    fn a_genuine_403_is_still_a_non_retriable_refusal() {
        for message in [
            "fatal: unable to access 'https://github.com/o/r/': The requested URL returned error: 403",
            "remote: Permission to o/r.git denied to car-coder.",
            "fatal: Authentication failed for 'https://github.com/o/r/'",
            "fatal: could not read Username for 'https://github.com'",
        ] {
            let (_, retriable) = classify_push_error(message);
            assert!(!retriable, "must not be retried: {message}");
        }
    }

    /// **The blocker.** `main` passed `validate_branch_name` as both the target
    /// and the base, and nothing compared them — so the refspec became
    /// `<sha>:refs/heads/main` and unreviewed model output was published to the
    /// base branch. Reconciliation only failed afterwards, by which time an
    /// append-only path has no way to take it back.
    #[test]
    fn delivering_onto_the_base_branch_is_refused_before_anything_is_pushed() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "unreviewed").unwrap();

        let gh = FakeGh::ok();
        let err = deliver_pr_with(delivery(&f, &wt, &c, "main", true, "b"), &gh).unwrap_err();

        assert_eq!(err.stage(), "preflight", "{err}");
        assert!(!err.retriable());
        assert!(err.reason().contains("base"), "{err}");
        // Nothing was committed, nothing was pushed, and `gh` was never even
        // asked for a credential — the refusal costs no session.
        assert!(
            git(&wt, &["log", "-1", "--format=%an"])
                .unwrap()
                .trim()
                .ne("car-coder"),
            "the worktree must not have been committed"
        );
        assert!(gh.calls().is_empty(), "{:?}", gh.calls());
    }

    /// **The stringly sentinel.** A commit that fails for an ordinary reason
    /// must never be mistaken for a clean worktree just because the caller's
    /// intent happens to contain the phrase the clean-worktree check used to
    /// report.
    #[test]
    fn a_commit_failure_is_not_mistaken_for_a_clean_worktree() {
        let f = fixture();
        let c = contract();

        // Round 1 delivers for real, so HEAD is ahead of the base and the
        // re-delivery branch would have something to push.
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "round one").unwrap();
        let first =
            deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "b"), &FakeGh::ok()).unwrap();

        // Round 2 has real work — and a commit that cannot succeed. Signing with
        // a program that does not exist is the portable stand-in for the real
        // causes (a gpg agent that is not running, a shared `pre-commit` hook).
        std::fs::write(
            wt.join("y.txt"),
            "round two — the work that must not be lost",
        )
        .unwrap();
        git(&f.repo, &["config", "commit.gpgsign", "true"]).unwrap();
        git(&f.repo, &["config", "gpg.program", "/nonexistent/gpg"]).unwrap();

        let intent = "fix delivery so it reports 'no changes to deliver' correctly";
        let d = PrDelivery {
            intent,
            ..delivery(&f, &wt, &c, TARGET, true, "b")
        };
        let err = deliver_pr_with(d, &FakeGh::ok()).unwrap_err();

        assert_eq!(err.stage(), "commit", "{err}");
        // The decisive assertion: round 1's commit must NOT have been re-pushed
        // and reported as this round's delivery.
        assert_eq!(
            f.origin_head(TARGET).as_deref(),
            Some(first.commit.as_str()),
            "the stale commit must not be re-delivered as if it were round 2"
        );
    }

    /// A fork's pull request shares only the head ref NAME. Adopting it would
    /// overwrite a stranger's description, or reopen their closed pull request,
    /// while our own branch still had none.
    #[test]
    fn a_cross_repository_pull_request_is_not_adopted() {
        let raw = r#"[
            {"number":200,"state":"OPEN","url":"https://github.com/acme/repo/pull/200",
             "isDraft":false,"isCrossRepository":true,"baseRefName":"main"},
            {"number":7,"state":"OPEN","url":"https://github.com/acme/repo/pull/7",
             "isDraft":false,"isCrossRepository":false,"baseRefName":"main"}
        ]"#;
        let prs = parse_pr_list(raw).unwrap();
        assert_eq!(
            prs.iter().map(|p| p.number).collect::<Vec<_>>(),
            vec![7],
            "only the same-repository pull request may be reconciled"
        );
        // The field is actually requested, or the filter has nothing to read.
        assert!(gh_pr_list_args("b")
            .iter()
            .any(|a| a == "number,state,url,isDraft,isCrossRepository,baseRefName"));
    }

    /// `gh` must be pointed at the repository the push went to. Its own
    /// resolution prefers `upstream`, which on a fork clone is a different
    /// repository entirely.
    #[test]
    fn the_github_repository_is_taken_from_the_same_remote_the_push_uses() {
        for (url, expect) in [
            ("https://github.com/acme/repo.git", Some("acme/repo")),
            ("https://github.com/acme/repo", Some("acme/repo")),
            ("git@github.com:acme/repo.git", Some("acme/repo")),
            ("ssh://git@github.com/acme/repo.git", Some("acme/repo")),
            (
                "https://x-token@github.com/acme/repo.git",
                Some("acme/repo"),
            ),
            // A GitHub Enterprise host must keep its host, or `--repo` would
            // aim at github.com.
            (
                "git@github.example.com:acme/repo.git",
                Some("github.example.com/acme/repo"),
            ),
            // Not a remote URL naming one repository: say nothing and leave
            // `gh`'s own resolution alone.
            ("/srv/mirrors/repo.git", None),
            ("../sibling", None),
        ] {
            assert_eq!(
                parse_github_repo_spec(url).as_deref(),
                expect,
                "for `{url}`"
            );
        }

        // And it reaches the argv: the fixture's origin is a local path, so
        // nothing is added there, while a GitHub origin adds `--repo`.
        let f = fixture();
        assert!(gh_repo_args(&f.repo).is_empty());
        git(
            &f.repo,
            &[
                "remote",
                "set-url",
                "origin",
                "git@github.com:acme/repo.git",
            ],
        )
        .unwrap();
        assert_eq!(
            gh_repo_args(&f.repo),
            vec!["--repo".to_string(), "acme/repo".to_string()]
        );
    }

    /// The two permanent push failures the refusal set had no spelling for.
    /// Both landed on the retriable default, so the orchestrator requeued a full
    /// model session against a wall that can never move.
    #[test]
    fn a_404_or_401_push_failure_is_permanent_not_a_race() {
        for message in [
            "remote: Repository not found.\nfatal: repository \
             'https://github.com/o/private.git/' not found",
            "ERROR: Repository not found.\nfatal: Could not read from remote repository.",
            "fatal: unable to access 'https://github.com/o/r/': The requested URL returned \
             error: 401",
            "fatal: 'origin' does not appear to be a git repository",
        ] {
            let (_, retriable) = classify_push_error(message);
            assert!(!retriable, "must not be retried forever: {message}");
        }
        // And the loosening test: an ordinary lost race is still retriable.
        let (_, retriable) =
            classify_push_error("! [rejected] goalpool/g_404 -> goalpool/g_404 (non-fast-forward)");
        assert!(retriable, "a moved branch is still worth another round");
    }

    /// A repository-selecting environment variable overrides `-C`, so every
    /// path-based guard upstream can say yes while git acts somewhere else.
    /// Asserted against the source text: the point is that the clearing is
    /// WRITTEN, not that some execution path happened to cover it.
    #[test]
    fn git_does_not_inherit_the_repository_from_the_environment() {
        let production = MERGE_RS_SOURCE
            .split_once("mod tests {")
            .map(|(head, _)| head)
            .unwrap_or(MERGE_RS_SOURCE);
        for var in ["GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE"] {
            assert!(
                production.contains(&format!(".env_remove(\"{var}\")")),
                "`git()` must clear {var}, which otherwise overrides `-C`"
            );
        }
    }

    /// `status --porcelain` honours `status.showUntrackedFiles`, and a
    /// repository or global `no` makes a worktree holding nothing but NEW files
    /// look clean — so delivery took the re-delivery branch and pushed a stale
    /// commit while the round's whole output stayed behind.
    #[test]
    fn a_worktree_holding_only_untracked_work_is_not_read_as_clean() {
        let f = fixture();
        let c = contract();
        git(&f.repo, &["config", "status.showUntrackedFiles", "no"]).unwrap();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("brand-new.txt"), "a day of work").unwrap();

        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, true, "b"), &FakeGh::ok()).unwrap();

        assert_eq!(
            git(
                &f.origin,
                &["show", &format!("refs/heads/{TARGET}:brand-new.txt")]
            )
            .unwrap(),
            "a day of work",
            "the untracked work must be in the delivered commit"
        );
        assert_eq!(out.pr_action, PrAction::Opened);
    }

    // --- Round-5 findings --------------------------------------------------

    /// The pull-request body is MODEL output, and it must never be able to
    /// decide whether a failure is permanent.
    ///
    /// Reproduces the reported chain exactly: a goal whose intent names one of
    /// the permanent phrases produces a body containing it, the push succeeds,
    /// and `gh pr edit --body …` then hits an ordinary 502. Before the split in
    /// [`GhError`] the classifier read the whole argv, matched the phrase inside
    /// the echoed body, and returned `retriable: false` — exit 3, a goal parked
    /// forever with its commit already safely on the remote.
    #[test]
    fn a_permanent_phrase_in_the_body_cannot_make_a_transient_failure_permanent() {
        let f = fixture();
        let c = contract();

        // Round 1 opens the pull request.
        let wt1 = f.cut("s1", "main");
        std::fs::write(wt1.join("x.txt"), "one").unwrap();
        deliver_pr_with(delivery(&f, &wt1, &c, TARGET, false, "one"), &FakeGh::ok()).unwrap();

        // Round 2 updates it, and `gh pr edit` fails transiently.
        git(&f.repo, &["fetch", "-q", "origin"]).unwrap();
        let wt2 = f.cut("s2", &format!("origin/{TARGET}"));
        std::fs::write(wt2.join("y.txt"), "two").unwrap();

        let body = "This round fixes the 'no commits between' error on empty deliveries.";
        let gh = FakeGh::failing_set_body("HTTP 502: Bad Gateway (https://api.github.com/…)");
        *gh.prs.lock().unwrap() = vec![PrRecord {
            number: 101,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/101".into(),
            is_draft: false,
            base: "main".into(),
        }];

        let err = deliver_pr_with(delivery(&f, &wt2, &c, TARGET, false, body), &gh).unwrap_err();
        assert!(
            err.retriable(),
            "a 502 is retriable however the body is worded: {err:?}"
        );

        // And the classifier still calls a REAL permanent failure permanent,
        // so the fix is not "everything is retriable now".
        let gh2 = FakeGh::failing_set_body("GraphQL: No commits between main and goalpool/g_1");
        *gh2.prs.lock().unwrap() = vec![PrRecord {
            number: 101,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/101".into(),
            is_draft: false,
            base: "main".into(),
        }];
        let wt3 = f.cut("s3", &format!("origin/{TARGET}"));
        std::fs::write(wt3.join("z.txt"), "three").unwrap();
        let err2 =
            deliver_pr_with(delivery(&f, &wt3, &c, TARGET, false, "plain"), &gh2).unwrap_err();
        assert!(!err2.retriable(), "{err2:?}");
    }

    /// GitHub's one-open-pull-request rule is per (head, base) PAIR, so two
    /// open pull requests from one head into different bases are legal — but a
    /// push cannot be aimed at only one of them. The head is ambiguous, and
    /// delivery refuses before it commits anything rather than reconciling one
    /// pull request while quietly appending to the other.
    ///
    /// This fixture used to assert the opposite — that reconciliation picked
    /// the same-base pull request and left the other alone — which is the
    /// behavior car#1054 overturned. Reconciliation's base filter is still
    /// there and still load-bearing for the closed and merged cases; its
    /// coverage now lives in
    /// [`a_changed_base_opens_its_own_pull_request_once_the_old_one_is_closed`].
    #[test]
    fn an_open_pull_request_into_another_base_is_refused_rather_than_reconciled() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();

        // #10 into `main` — ours. #12 into `release/2.1` — somebody else's,
        // which the push would have landed on all the same.
        let gh = FakeGh::with_prs(vec![
            PrRecord {
                number: 10,
                state: PrState::Open,
                url: "https://github.com/acme/repo/pull/10".into(),
                is_draft: false,
                base: "main".into(),
            },
            PrRecord {
                number: 12,
                state: PrState::Open,
                url: "https://github.com/acme/repo/pull/12".into(),
                is_draft: false,
                base: "release/2.1".into(),
            },
        ]);

        let err =
            deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "round body"), &gh).unwrap_err();

        assert!(
            matches!(err, DeliveryFailure::Preflight { .. }),
            "an ambiguous head is refused before the commit: {err:?}"
        );
        assert!(
            !err.retriable(),
            "retrying changes nothing — a human closes #12 or picks another target branch"
        );
        // Actionable: it names the offending pull request and its base.
        assert!(
            err.reason().contains("#12") && err.reason().contains("release/2.1"),
            "{}",
            err.reason()
        );

        // Nothing was written anywhere: no branch on the remote, and no pull
        // request touched — including the one into our own base.
        assert_eq!(f.origin_head(TARGET), None, "nothing may be pushed");
        assert!(
            !gh.calls()
                .iter()
                .any(|c| c.starts_with("set_body") || c.starts_with("create")),
            "{:?}",
            gh.calls()
        );
    }

    /// The issue's scenario verbatim: a human's pull request into `release/2.1`
    /// is open on the delivery branch and the round runs `--pr-base main`. The
    /// PUSH is what lands the model's commits on the human's pull request, so
    /// the assertion that matters is that the remote branch never moves.
    #[test]
    fn a_human_pull_request_into_another_base_parks_delivery_before_the_push() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "unreviewed model output").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 200,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/200".into(),
            is_draft: false,
            base: "release/2.1".into(),
        }]);

        let err =
            deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "round body"), &gh).unwrap_err();

        assert_eq!(err.stage(), "preflight");
        assert!(!err.retriable(), "{err:?}");
        assert!(err.reason().contains("#200"), "{}", err.reason());
        assert_eq!(
            f.origin_head(TARGET),
            None,
            "the model's commits must never reach a branch #200 tracks"
        );
        assert!(
            !gh.calls().iter().any(|c| c.starts_with("create")),
            "no second pull request is opened to paper over the refusal: {:?}",
            gh.calls()
        );
    }

    /// A changed `--pr-base` no longer opens a second pull request while the
    /// first is open: the push would land on both.
    #[test]
    fn a_changed_base_is_refused_while_the_old_pull_request_is_open() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 10,
            state: PrState::Open,
            url: "https://github.com/acme/repo/pull/10".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        let mut d = delivery(&f, &wt, &c, TARGET, false, "body");
        d.base_branch = "release/2.1";

        let err = deliver_pr_with(d, &gh).unwrap_err();
        assert_eq!(err.stage(), "preflight");
        assert!(err.reason().contains("#10"), "{}", err.reason());
        assert_eq!(f.origin_head(TARGET), None);
    }

    /// …and once that pull request is closed the same run proceeds and opens
    /// its own, which is what keeps the base filter and the create path live.
    #[test]
    fn a_changed_base_opens_its_own_pull_request_once_the_old_one_is_closed() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();

        let gh = FakeGh::with_prs(vec![PrRecord {
            number: 10,
            state: PrState::ClosedUnmerged,
            url: "https://github.com/acme/repo/pull/10".into(),
            is_draft: false,
            base: "main".into(),
        }]);
        let mut d = delivery(&f, &wt, &c, TARGET, false, "body");
        d.base_branch = "release/2.1";

        let out = deliver_pr_with(d, &gh).unwrap();
        assert_eq!(out.pr_action, PrAction::Opened);
        assert_ne!(out.pr_number, 10);
        assert!(
            gh.calls()
                .iter()
                .any(|c| c.contains("create head=") && c.contains("base=release/2.1")),
            "{:?}",
            gh.calls()
        );
        // #10 is closed but into ANOTHER base, so it neither parks this run
        // nor gets touched: [`closed_pr_refusal`] is base-scoped, or any stale
        // pull request would hold a veto over a base it does not merge into.
        assert!(
            !gh.calls().iter().any(|c| c.starts_with("set_body")),
            "{:?}",
            gh.calls()
        );
    }

    /// Only OPEN pull requests park a run. A merged one into another base is
    /// inert — it cannot gain commits — so it is not ambiguity.
    #[test]
    fn a_merged_pull_request_into_another_base_does_not_park_delivery() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();

        let gh = FakeGh::with_prs(vec![
            PrRecord {
                number: 10,
                state: PrState::Open,
                url: "https://github.com/acme/repo/pull/10".into(),
                is_draft: false,
                base: "main".into(),
            },
            PrRecord {
                number: 12,
                state: PrState::Merged,
                url: "https://github.com/acme/repo/pull/12".into(),
                is_draft: false,
                base: "release/2.1".into(),
            },
        ]);

        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "round body"), &gh).unwrap();
        assert_eq!(out.pr_action, PrAction::Updated);
        assert_eq!(out.pr_number, 10);
    }

    /// A fork's pull request that happens to share the head branch NAME cannot
    /// park a legitimate run — `parse_pr_list` drops cross-repository entries
    /// before delivery ever sees them, and the new preflight must not
    /// reintroduce them by reading the raw `gh` output itself.
    #[test]
    fn a_cross_repository_pull_request_does_not_park_delivery() {
        let parsed = parse_pr_list(
            r#"[
              {"number": 77, "state": "OPEN", "url": "u77", "isDraft": false,
               "baseRefName": "release/2.1", "isCrossRepository": true},
              {"number": 10, "state": "OPEN", "url": "u10", "isDraft": false,
               "baseRefName": "main", "isCrossRepository": false}
            ]"#,
        )
        .unwrap();
        assert_eq!(parsed.len(), 1, "the fork entry is dropped: {parsed:?}");

        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();
        let gh = FakeGh::with_prs(parsed);

        let out = deliver_pr_with(delivery(&f, &wt, &c, TARGET, false, "round body"), &gh).unwrap();
        assert_eq!(out.pr_action, PrAction::Updated);
        assert_eq!(out.pr_number, 10);
    }

    /// `could not read Password` is git's message when the remote URL already
    /// carries a username — the shape `gh auth setup-git` leaves behind — and it
    /// is exactly as permanent as `could not read Username`.
    #[test]
    fn credential_prompts_that_can_never_be_answered_are_permanent() {
        for message in [
            "fatal: could not read Username for 'https://github.com': No such device or address",
            "fatal: could not read Password for 'https://someuser@github.com': \
             No such device or address",
            "fatal: could not read Username for 'https://github.com': terminal prompts disabled",
            "Host key verification failed.\nfatal: Could not read from remote repository.",
        ] {
            let (_, retriable) = classify_push_error(message);
            assert!(
                !retriable,
                "a credential that will never appear must not be retried: {message}"
            );
        }
        // Unchanged: an ordinary lost race on a branch whose name contains none
        // of those words is still retriable.
        let (_, retriable) =
            classify_push_error("! [rejected] goalpool/g_pw -> goalpool/g_pw (non-fast-forward)");
        assert!(retriable);
    }

    /// git prompts on `/dev/tty`, not stdin, so nulling stdin is not enough:
    /// a delivery launched with an inherited controlling terminal hung forever.
    /// Probed through git itself rather than by reading this file.
    #[test]
    fn git_children_cannot_open_a_terminal_prompt() {
        let dir = tempfile::tempdir().unwrap();
        git(dir.path(), &["init", "-q", "-b", "main"]).unwrap();
        // A `!` alias runs through the shell, so it can report what git handed
        // its children.
        let seen = git(
            dir.path(),
            &[
                "-c",
                "alias.envprobe=!printf %s \"${GIT_TERMINAL_PROMPT-unset}\"",
                "envprobe",
            ],
        )
        .unwrap();
        assert_eq!(
            seen.trim(),
            "0",
            "GIT_TERMINAL_PROMPT must be 0 for every git this module runs"
        );
    }

    /// A subprocess that never exits is killed and reported, rather than
    /// hanging a round that nobody is watching.
    #[test]
    fn a_subprocess_that_never_finishes_is_killed_and_reported() {
        let mut cmd = std::process::Command::new("sleep");
        cmd.arg("60");
        let started = std::time::Instant::now();
        let err = run_capped_for(cmd, std::time::Duration::from_millis(300)).err();
        assert!(
            matches!(err, Some(RunFailure::TimedOut(_))),
            "the child must be killed, not waited on"
        );
        assert!(
            started.elapsed() < std::time::Duration::from_secs(20),
            "the kill must not wait for the child's own exit"
        );
    }

    /// The headless branch mode's clean-worktree path: round N committed the
    /// work, round N+1 reuses the workspace with nothing left to commit, and
    /// that is a re-delivery rather than an error.
    #[test]
    fn branch_mode_re_delivers_a_clean_worktree_whose_head_is_ahead_of_the_base() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        std::fs::write(wt.join("x.txt"), "work").unwrap();
        // Round N: commits and names a branch.
        let first =
            publish_branch_headless(&f.repo, &wt, "r1", "make x exist", &c, "main").unwrap();
        assert_eq!(first, "car/coder/r1");

        // Round N+1: the same worktree, now clean, HEAD ahead of the base.
        let second =
            publish_branch_headless(&f.repo, &wt, "r2", "make x exist", &c, "main").unwrap();
        assert_eq!(second, "car/coder/r2");
        assert_eq!(
            git(&f.repo, &["rev-parse", "car/coder/r1"]).unwrap().trim(),
            git(&f.repo, &["rev-parse", "car/coder/r2"]).unwrap().trim(),
            "the same commit is re-delivered, not redone"
        );
    }

    /// The empty-first-round case is still refused: a clean worktree sitting AT
    /// the base has nothing to deliver, and naming a branch for it would prove
    /// nothing.
    #[test]
    fn branch_mode_still_refuses_a_clean_worktree_that_holds_no_work() {
        let f = fixture();
        let c = contract();
        let wt = f.cut("s1", "main");
        let err = publish_branch_headless(&f.repo, &wt, "r1", "noop", &c, "main").unwrap_err();
        assert!(err.contains("nothing to deliver"), "{err}");
    }

    /// A Windows-authored intent separates paragraphs with `\r\n\r\n`, which the
    /// blank-line split never matched — so the whole document was flattened and
    /// truncated at 72, and carriage returns rode into the commit subject.
    #[test]
    fn a_crlf_intent_still_stops_at_its_blank_line() {
        let intent =
            "Add the retry shim\r\n\r\nPointers:\r\n- see src/net.rs\r\n- and the docs\r\n";
        let subject = subject_from_intent(intent);
        assert_eq!(subject, "Add the retry shim");
        assert!(!subject.contains('\r'), "{subject:?}");
        // Mixed endings, and a lone `\r` inside the summary, are flattened too.
        assert!(!subject_from_intent("a\rb\r\nc").contains('\r'));
    }

    /// The human-readable half never echoes the model's title or body either.
    /// It reaches an event stream and a `run_end.error` an orchestrator logs,
    /// and those values are unbounded model output.
    #[test]
    fn the_gh_command_shape_elides_the_title_and_the_body() {
        let args = gh_pr_create_args("head", "main", "a title", "a very long body", false);
        let shape = gh_subcommand_shape(&args);
        assert!(!shape.contains("a title"), "{shape}");
        assert!(!shape.contains("a very long body"), "{shape}");
        assert!(shape.contains("--title"), "{shape}");
        assert!(shape.contains("--head head"), "{shape}");
    }
}