use crate::docker::{CommandRunner, RunOptions};
use crate::instance::runtime_slug;
use crate::paths::JackinPaths;
use crate::repo::{CachedRepo, validate_role_repo};
use crate::selector::RoleSelector;
use anyhow::Context;
use fs2::FileExt;
use std::io::IsTerminal;

use super::identity::try_capture;

/// Typed errors raised by role-repo resolution.
///
/// Surfaced through `anyhow::Error` chains so the editor can downcast and
/// pick a friendly translation without substring-matching free text. Add
/// new variants here together with their `friendly_role_resolution_error`
/// arm in `console::manager::input::editor`.
#[derive(Debug, thiserror::Error)]
pub enum RepoError {
    /// `git clone` failed for any reason — host unreachable, auth required,
    /// repo missing, server-side error. The original anyhow chain is kept
    /// as the `#[source]` so `--debug` still surfaces it.
    #[error("repository is not available or cannot be accessed")]
    CloneFailed(#[source] anyhow::Error),

    /// Cached repo's `origin` remote points at a different URL than the
    /// configured source and the operator declined removal.
    #[error("cached role repo remote mismatch — aborting")]
    RemoteMismatch,

    /// `validate_role_repo` rejected the cloned repo. The inner typed
    /// error carries the structural reason; `friendly_role_resolution_error`
    /// matches on its variants for richer messages.
    #[error("invalid role repo: {0}")]
    InvalidRoleRepo(#[from] crate::repo::RoleRepoValidationError),
}

/// Extract `owner/repo` from a git remote URL.
pub(super) fn parse_repo_name(url: &str) -> Option<String> {
    let url = url.trim();
    let stripped = url.strip_suffix(".git").unwrap_or(url);
    // HTTPS: https://github.com/owner/repo
    if let Some(rest) = stripped
        .strip_prefix("https://")
        .or_else(|| stripped.strip_prefix("http://"))
    {
        return rest.find('/').map(|i| rest[i + 1..].to_string());
    }
    // SSH: git@github.com:owner/repo
    stripped.rsplit_once(':').map(|(_, p)| p.to_string())
}

pub(super) fn repo_matches(expected: &str, actual: &str) -> bool {
    match (parse_repo_name(expected), parse_repo_name(actual)) {
        (Some(expected_repo), Some(actual_repo)) => expected_repo == actual_repo,
        _ => expected.trim() == actual.trim(),
    }
}

fn role_cache_root(paths: &JackinPaths, selector: &RoleSelector) -> std::path::PathBuf {
    selector.namespace.as_ref().map_or_else(
        || paths.roles_dir.join(&selector.name),
        |namespace| paths.roles_dir.join(namespace).join(&selector.name),
    )
}

fn migrate_legacy_default_cache(
    paths: &JackinPaths,
    selector: &RoleSelector,
    cached_repo: &CachedRepo,
) -> anyhow::Result<()> {
    let root = role_cache_root(paths, selector);
    if cached_repo.repo_dir != root.join("default")
        || cached_repo.repo_dir.exists()
        || !root.join(".git").is_dir()
    {
        return Ok(());
    }

    let parent = root
        .parent()
        .ok_or_else(|| anyhow::anyhow!("role cache path has no parent: {}", root.display()))?;
    let root_name = root
        .file_name()
        .and_then(|name| name.to_str())
        .unwrap_or("role");
    let suffix = std::time::SystemTime::now()
        .duration_since(std::time::UNIX_EPOCH)
        .map(|duration| duration.as_nanos())
        .unwrap_or_default();
    let legacy_root = parent.join(format!(".{root_name}.legacy-cache-{suffix}"));

    std::fs::rename(&root, &legacy_root).with_context(|| {
        format!(
            "failed to move legacy role cache {} before migrating to default/",
            root.display()
        )
    })?;
    std::fs::create_dir_all(&root)?;

    let legacy_branches = legacy_root.join("branches");
    if legacy_branches.exists() {
        std::fs::rename(&legacy_branches, root.join("branches")).with_context(|| {
            format!(
                "failed to move legacy branch cache {}",
                legacy_branches.display()
            )
        })?;
    }

    std::fs::rename(&legacy_root, &cached_repo.repo_dir).with_context(|| {
        format!(
            "failed to move legacy role cache into {}",
            cached_repo.repo_dir.display()
        )
    })?;

    Ok(())
}

fn role_root_has_unexpected_entries(root: &std::path::Path) -> anyhow::Result<bool> {
    if !root.exists() {
        return Ok(false);
    }

    for entry in std::fs::read_dir(root)? {
        let entry = entry?;
        if entry.file_name() != "branches" {
            return Ok(true);
        }
    }

    Ok(false)
}

/// Derive a short repository name from a git remote URL (e.g. `jackin-project/jackin`).
pub(super) async fn git_repo_name(
    dir: &std::path::Path,
    runner: &mut impl CommandRunner,
) -> Option<String> {
    let dir_str = dir.display().to_string();
    let url = try_capture(
        runner,
        "git",
        &["-C", &dir_str, "remote", "get-url", "origin"],
    )
    .await?;
    parse_repo_name(&url)
}

/// Get the current branch name for a git directory.
pub(super) async fn git_branch(
    dir: &std::path::Path,
    runner: &mut impl CommandRunner,
) -> Option<String> {
    let dir_str = dir.display().to_string();
    try_capture(
        runner,
        "git",
        &["-C", &dir_str, "rev-parse", "--abbrev-ref", "HEAD"],
    )
    .await
}

/// Check whether a path is inside a git work tree.
pub(super) async fn is_git_dir(dir: &std::path::Path, runner: &mut impl CommandRunner) -> bool {
    let dir_str = dir.display().to_string();
    try_capture(
        runner,
        "git",
        &["-C", &dir_str, "rev-parse", "--is-inside-work-tree"],
    )
    .await
    .is_some()
}

/// Resolve the role repository: clone if missing, pull if already present.
/// Returns the validated repo metadata and cached repo paths.
/// Prompt the user to confirm cached-repo removal when running in an
/// interactive terminal.  Returns `true` when the user accepts.
pub(super) fn confirm_repo_removal_interactive() -> anyhow::Result<bool> {
    if !std::io::stdin().is_terminal() {
        return Ok(false);
    }
    Ok(dialoguer::Confirm::new()
        .with_prompt("Remove the cached repo and re-clone from the configured source?")
        .default(false)
        .interact()?)
}

pub(super) async fn resolve_agent_repo(
    paths: &JackinPaths,
    selector: &RoleSelector,
    git_url: &str,
    runner: &mut impl CommandRunner,
    debug: bool,
    branch_override: Option<&str>,
) -> anyhow::Result<(CachedRepo, crate::repo::ValidatedRoleRepo, std::fs::File)> {
    // URL is normalized once inside `resolve_agent_repo_with` — single
    // chokepoint for the SSH→HTTPS rewrite, no double-allocation.
    resolve_agent_repo_with(
        paths,
        selector,
        git_url,
        runner,
        debug,
        branch_override,
        confirm_repo_removal_interactive,
    )
    .await
}

/// Resolve a role repo into the cache, registering it on success.
///
/// Two paths:
/// 1. Cache hit (`.git` already exists): delegate to
///    `resolve_agent_repo_with` which fetch+merges, then run
///    `persist_registration` if validation passes.
/// 2. Cache miss: clone into a temp dir under `data_dir`, validate,
///    `rename` into the cache, then run `persist_registration`. Rename
///    happens before persist so a failed rename leaves the role
///    *un-registered* (clean state) rather than registered without an
///    on-disk repo (broken state).
///
/// `persist_registration` is the single commit point — it must be
/// idempotent so retries after a transient failure are safe.
pub(super) async fn register_agent_repo(
    paths: &JackinPaths,
    selector: &RoleSelector,
    git_url: &str,
    runner: &mut impl CommandRunner,
    debug: bool,
    persist_registration: impl FnOnce() -> anyhow::Result<()>,
) -> anyhow::Result<(CachedRepo, crate::repo::ValidatedRoleRepo)> {
    let normalized = normalize_github_url(git_url);
    let git_url = normalized.as_str();
    let cached_repo = CachedRepo::new(paths, selector);
    let legacy_root = role_cache_root(paths, selector);
    if cached_repo.repo_dir.join(".git").is_dir() || legacy_root.join(".git").is_dir() {
        let (cached_repo, validated_repo, _lock_file) =
            resolve_agent_repo_with(paths, selector, git_url, runner, debug, None, || Ok(false))
                .await?;
        persist_registration()?;
        return Ok((cached_repo, validated_repo));
    }

    let repo_parent = cached_repo.repo_dir.parent().ok_or_else(|| {
        anyhow::anyhow!(
            "role repo path has no parent: {}",
            cached_repo.repo_dir.display()
        )
    })?;
    std::fs::create_dir_all(repo_parent)?;
    std::fs::create_dir_all(&paths.data_dir)?;

    let role_root = role_cache_root(paths, selector);
    if !cached_repo.repo_dir.exists()
        && !role_root.join(".git").is_dir()
        && role_root_has_unexpected_entries(&role_root)?
    {
        anyhow::bail!(
            "cached role path exists but is not a git repository: {}",
            role_root.display()
        );
    }

    if cached_repo.repo_dir.exists() {
        anyhow::bail!(
            "cached role path exists but is not a git repository: {}",
            cached_repo.repo_dir.display()
        );
    }

    let lock_path = paths
        .data_dir
        .join(format!("{}.repo.lock", runtime_slug(selector)));
    let lock_file = std::fs::File::create(&lock_path)?;
    lock_file
        .lock_exclusive()
        .map_err(|e| anyhow::anyhow!("failed to acquire repo lock for {}: {e}", selector.key()))?;

    let temp_dir = tempfile::Builder::new()
        .prefix("role-resolve-")
        .tempdir_in(&paths.data_dir)?;
    let temp_repo = temp_dir.path().join("repo");
    let temp_repo_path = temp_repo.display().to_string();
    let git_run_opts = RunOptions {
        quiet: !debug,
        ..RunOptions::default()
    };
    runner
        .run(
            "git",
            &["clone", git_url, &temp_repo_path],
            None,
            &git_run_opts,
        )
        .await
        .map_err(RepoError::CloneFailed)?;

    let validated_repo = validate_role_repo(&temp_repo).map_err(RepoError::InvalidRoleRepo)?;
    // Install the repo into the cache before persisting registration: if
    // rename fails the role stays unregistered and the user can retry from a
    // clean state. Persisting first would leave config.toml referencing a
    // role that has no on-disk repo, surfacing as "failed to install" once
    // and then silently registered every load thereafter.
    std::fs::rename(&temp_repo, &cached_repo.repo_dir).with_context(|| {
        format!(
            "failed to install role repository at {}",
            cached_repo.repo_dir.display()
        )
    })?;
    persist_registration().with_context(|| {
        format!(
            "role repo installed at {} but registration could not be persisted",
            cached_repo.repo_dir.display()
        )
    })?;

    Ok((cached_repo, validated_repo))
}

/// Normalize SSH-form GitHub URLs to HTTPS so containers without an
/// SSH key can still clone a role repo when the operator's
/// `[roles.<name>].git` is in the SCP/`ssh://` form.
///
/// Non-GitHub URLs (e.g. self-hosted gitlab) pass through unchanged —
/// substituting their SSH form for HTTPS would risk hitting an
/// HTTPS endpoint that doesn't exist on the operator's SCM.
// Reachable crate-wide via the `pub(crate) use` re-export in
// `runtime/mod.rs`. `redundant_pub_crate` fires because the parent
// module is private; the attribute keeps the visibility honest at
// the symbol.
#[allow(clippy::redundant_pub_crate)]
pub(crate) fn normalize_github_url(url: &str) -> String {
    if let Some(rest) = url.strip_prefix("git@github.com:") {
        return format!("https://github.com/{rest}");
    }
    if let Some(rest) = url.strip_prefix("ssh://git@github.com/") {
        return format!("https://github.com/{rest}");
    }
    url.to_string()
}

/// Build the argument list for `git clone`, optionally scoped to a single branch.
/// `git clone -b <branch>` fetches only that branch, making the clone faster and
/// leaving the working tree on the right branch without a separate checkout step.
fn clone_args<'a>(git_url: &'a str, dest: &'a str, branch: Option<&'a str>) -> Vec<&'a str> {
    branch.map_or_else(
        || vec!["clone", git_url, dest],
        |b| vec!["clone", "-b", b, git_url, dest],
    )
}

#[allow(clippy::too_many_lines)]
pub(super) async fn resolve_agent_repo_with(
    paths: &JackinPaths,
    selector: &RoleSelector,
    git_url: &str,
    runner: &mut impl CommandRunner,
    debug: bool,
    branch_override: Option<&str>,
    confirm_removal: impl FnOnce() -> anyhow::Result<bool>,
) -> anyhow::Result<(CachedRepo, crate::repo::ValidatedRoleRepo, std::fs::File)> {
    let normalized = normalize_github_url(git_url);
    let git_url = normalized.as_str();
    let cached_repo = branch_override.map_or_else(
        || CachedRepo::new(paths, selector),
        |branch| CachedRepo::for_branch(paths, selector, branch),
    );
    let repo_parent = cached_repo.repo_dir.parent().ok_or_else(|| {
        anyhow::anyhow!(
            "role repo path has no parent: {}",
            cached_repo.repo_dir.display()
        )
    })?;
    std::fs::create_dir_all(repo_parent)?;

    // Short-lived lock around git operations on the shared repo directory.
    // Multiple `jackin load` commands may run in parallel for the same
    // role class (spawning clones); the lock serializes only the git
    // clone/fetch/merge so they don't race on the same working tree.
    // The lock is released as soon as the git section completes.
    //
    // Lock path mirrors the repo_dir path under data_dir so that a branch
    // named `feat/my-pr` and one named `feat-my-pr` (slug collision) always
    // get distinct lock files. The roles_dir prefix is stripped to produce
    // the relative path; `std::fs::create_dir_all` creates intermediate dirs.
    let selector_base = role_cache_root(paths, selector);
    let rel = cached_repo
        .repo_dir
        .strip_prefix(&selector_base)
        .unwrap_or_else(|_| std::path::Path::new(""));
    // Build lock path: data_dir/<container>.locks/<rel>.repo.lock
    let lock_path = {
        let rel_str = rel.to_string_lossy();
        let file_name = if rel_str.is_empty() {
            "default.repo.lock".to_string()
        } else {
            format!("{rel_str}.repo.lock")
        };
        paths
            .data_dir
            .join(format!("{}.locks", runtime_slug(selector)))
            .join(file_name)
    };
    std::fs::create_dir_all(lock_path.parent().unwrap_or(&paths.data_dir))?;
    let lock_file = std::fs::File::create(&lock_path)?;
    lock_file
        .lock_exclusive()
        .map_err(|e| anyhow::anyhow!("failed to acquire repo lock for {}: {e}", selector.key()))?;

    let git_run_opts = RunOptions {
        quiet: !debug,
        ..RunOptions::default()
    };

    if branch_override.is_none() {
        migrate_legacy_default_cache(paths, selector, &cached_repo)?;
    }

    let repo_path = cached_repo.repo_dir.display().to_string();
    if cached_repo.repo_dir.join(".git").is_dir() {
        let remote_url = runner
            .capture(
                "git",
                &["-C", &repo_path, "remote", "get-url", "origin"],
                None,
            )
            .await?;
        if !repo_matches(git_url, &remote_url) {
            // Route diagnostics through the buffered debug channel rather
            // than `eprintln!` — the latter corrupts the alt-screen render
            // when called from inside the TUI session. Operators with
            // `--debug` still get the full expected/found/path trio.
            crate::debug_log!(
                "repo_cache",
                "cached role repo remote mismatch: expected={git_url:?} \
                 found={remote_url:?} path={}",
                cached_repo.repo_dir.display()
            );

            if confirm_removal()? {
                std::fs::remove_dir_all(&cached_repo.repo_dir)?;
                let clone_args = clone_args(git_url, &repo_path, branch_override);
                runner
                    .run("git", &clone_args, None, &git_run_opts)
                    .await
                    .map_err(RepoError::CloneFailed)?;
                let validated_repo = validate_role_repo(&cached_repo.repo_dir)
                    .map_err(RepoError::InvalidRoleRepo)?;
                return Ok((cached_repo, validated_repo, lock_file));
            }

            return Err(anyhow::Error::new(RepoError::RemoteMismatch));
        }

        let status = runner
            .capture(
                "git",
                &[
                    "-C",
                    &repo_path,
                    "status",
                    "--porcelain",
                    "--ignored=matching",
                    "--untracked-files=all",
                ],
                None,
            )
            .await?;
        anyhow::ensure!(
            status.is_empty(),
            "cached role repo contains local changes or extra files: {}. Remove the cached repo or clean it before loading.",
            cached_repo.repo_dir.display()
        );

        // Fetch + merge instead of pull to avoid "Cannot fast-forward to
        // multiple branches" errors that occur with `git pull` when the
        // remote has multiple branches. When a branch is pinned via
        // `--branch`, use it directly; otherwise derive from HEAD.
        let branch_val = git_branch(&cached_repo.repo_dir, runner).await;
        let branch = branch_override.map_or_else(
            || {
                branch_val.ok_or_else(|| {
                    anyhow::anyhow!(
                        "could not determine current branch of cached role repo at {}",
                        cached_repo.repo_dir.display()
                    )
                })
            },
            |b| Ok(b.to_string()),
        )?;
        runner
            .run(
                "git",
                &["-C", &repo_path, "fetch", "origin", &branch],
                None,
                &git_run_opts,
            )
            .await?;
        let ff_result = runner
            .run(
                "git",
                &["-C", &repo_path, "merge", "--ff-only", "FETCH_HEAD"],
                None,
                &git_run_opts,
            )
            .await;
        if ff_result.is_err() {
            eprintln!(
                "        cached role branch diverged (remote may have been force-pushed) — resetting to origin/{branch}"
            );
            runner
                .run(
                    "git",
                    &["-C", &repo_path, "reset", "--hard", "FETCH_HEAD"],
                    None,
                    &git_run_opts,
                )
                .await?;
        }
    } else {
        let clone_args = clone_args(git_url, &repo_path, branch_override);
        runner
            .run("git", &clone_args, None, &git_run_opts)
            .await
            .map_err(RepoError::CloneFailed)?;
    }

    let validated_repo =
        validate_role_repo(&cached_repo.repo_dir).map_err(RepoError::InvalidRoleRepo)?;

    // Return the repo lock so the caller can hold it until the build
    // context (a snapshot copy of the repo) is created.  This prevents
    // a parallel load from fast-forwarding the shared repo between
    // validation and context creation.
    Ok((cached_repo, validated_repo, lock_file))
}

#[cfg(test)]
mod tests {
    use super::super::test_support::FakeRunner;
    use super::*;
    use crate::paths::JackinPaths;
    use crate::selector::RoleSelector;
    use tempfile::tempdir;

    #[test]
    fn normalize_github_url_rewrites_scp_form() {
        assert_eq!(
            normalize_github_url("git@github.com:jackin-project/jackin.git"),
            "https://github.com/jackin-project/jackin.git"
        );
    }

    #[test]
    fn normalize_github_url_rewrites_ssh_url_form() {
        assert_eq!(
            normalize_github_url("ssh://git@github.com/jackin-project/jackin.git"),
            "https://github.com/jackin-project/jackin.git"
        );
    }

    #[test]
    fn normalize_github_url_passes_https_through_unchanged() {
        assert_eq!(
            normalize_github_url("https://github.com/jackin-project/jackin.git"),
            "https://github.com/jackin-project/jackin.git"
        );
    }

    #[test]
    fn normalize_github_url_leaves_non_github_urls_alone() {
        // Non-GitHub SSH URLs must NOT be rewritten — substituting an
        // HTTPS URL would risk hitting an endpoint that doesn't exist
        // on the operator's SCM.
        assert_eq!(
            normalize_github_url("git@gitlab.example.com:team/repo.git"),
            "git@gitlab.example.com:team/repo.git"
        );
        assert_eq!(
            normalize_github_url("ssh://git@gitlab.example.com/team/repo.git"),
            "ssh://git@gitlab.example.com/team/repo.git"
        );
    }

    #[test]
    fn normalize_github_url_handles_missing_git_suffix() {
        assert_eq!(
            normalize_github_url("git@github.com:jackin-project/jackin"),
            "https://github.com/jackin-project/jackin"
        );
        assert_eq!(
            normalize_github_url("ssh://git@github.com/jackin-project/jackin"),
            "https://github.com/jackin-project/jackin"
        );
    }

    #[test]
    fn repo_matches_cross_protocol_for_same_owner_repo() {
        // After SSH→HTTPS normalize, a repo cloned years ago via SSH
        // and a config that now says HTTPS must agree at the
        // remote-URL match check.
        assert!(repo_matches(
            "https://github.com/jackin-project/jackin.git",
            "git@github.com:jackin-project/jackin.git"
        ));
        assert!(repo_matches(
            "git@github.com:jackin-project/jackin.git",
            "https://github.com/jackin-project/jackin.git"
        ));
    }

    #[test]
    fn parse_repo_name_extracts_owner_repo_from_ssh_url() {
        assert_eq!(
            parse_repo_name("git@github.com:jackin-project/jackin.git"),
            Some("jackin-project/jackin".to_string())
        );
    }

    #[test]
    fn parse_repo_name_extracts_owner_repo_from_https_url() {
        assert_eq!(
            parse_repo_name("https://github.com/jackin-project/jackin.git"),
            Some("jackin-project/jackin".to_string())
        );
    }

    #[test]
    fn parse_repo_name_handles_url_without_git_suffix() {
        assert_eq!(
            parse_repo_name("https://github.com/jackin-project/jackin"),
            Some("jackin-project/jackin".to_string())
        );
        assert_eq!(
            parse_repo_name("git@github.com:jackin-project/jackin"),
            Some("jackin-project/jackin".to_string())
        );
    }

    #[tokio::test]
    async fn resolve_agent_repo_rejects_cached_repo_with_wrong_remote() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        let mut runner =
            FakeRunner::with_capture_queue(["git@github.com:evil/agent-smith.git".to_string()]);
        let error = resolve_agent_repo(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
        )
        .await
        .unwrap_err();

        assert!(
            error
                .to_string()
                .contains("cached role repo remote mismatch")
        );
    }

    #[tokio::test]
    async fn resolve_agent_repo_recovers_when_user_confirms_removal() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        // The capture queue provides: 1) the wrong remote URL, then 2) a
        // successful clone response (empty output).  After the user confirms,
        // the function removes the stale dir and re-clones.
        let mut runner = FakeRunner::with_capture_queue([
            "git@github.com:evil/agent-smith.git".to_string(),
            String::new(), // clone output
        ]);

        // Simulate what `git clone` would produce on disk: recreate the repo
        // files when the clone command is captured by FakeRunner.
        let repo_dir_clone = repo_dir;
        runner.side_effects.push((
            "clone".to_string(),
            Box::new(move || {
                std::fs::create_dir_all(repo_dir_clone.join(".git")).unwrap();
                std::fs::write(
                    repo_dir_clone.join("Dockerfile"),
                    "FROM projectjackin/construct:0.1-trixie\n",
                )
                .unwrap();
                std::fs::write(
                    repo_dir_clone.join("jackin.role.toml"),
                    r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
                )
                .unwrap();
            }),
        ));

        let result = resolve_agent_repo_with(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
            || Ok(true), // user confirms removal
        )
        .await;

        assert!(result.is_ok(), "expected recovery to succeed: {result:?}");
        assert!(
            runner.recorded.iter().any(|c| c.contains("clone")),
            "expected a git clone after removal"
        );
    }

    #[tokio::test]
    async fn resolve_agent_repo_aborts_when_user_declines_removal() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        let mut runner =
            FakeRunner::with_capture_queue(["git@github.com:evil/agent-smith.git".to_string()]);
        let error = resolve_agent_repo_with(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
            || Ok(false), // user declines
        )
        .await
        .unwrap_err();

        assert!(
            error
                .to_string()
                .contains("cached role repo remote mismatch")
        );
        // The cached repo directory should still exist
        assert!(repo_dir.join(".git").is_dir());
    }

    #[tokio::test]
    async fn resolve_agent_repo_rejects_cached_repo_with_local_changes() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        let mut runner = FakeRunner::with_capture_queue([
            "git@github.com:jackin-project/jackin-agent-smith.git".to_string(),
            "?? scratch.txt".to_string(),
        ]);
        let error = resolve_agent_repo(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
        )
        .await
        .unwrap_err();

        assert!(error.to_string().contains("contains local changes"));
    }

    #[tokio::test]
    async fn resolve_agent_repo_uses_run_for_clone_after_recovery() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        let mut runner =
            FakeRunner::with_capture_queue(["git@github.com:evil/agent-smith.git".to_string()]);
        let repo_dir_clone = repo_dir;
        runner.side_effects.push((
            "clone".to_string(),
            Box::new(move || {
                std::fs::create_dir_all(repo_dir_clone.join(".git")).unwrap();
                std::fs::write(
                    repo_dir_clone.join("Dockerfile"),
                    "FROM projectjackin/construct:0.1-trixie\n",
                )
                .unwrap();
                std::fs::write(
                    repo_dir_clone.join("jackin.role.toml"),
                    r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
                )
                .unwrap();
            }),
        ));

        let result = resolve_agent_repo_with(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
            || Ok(true),
        )
        .await;

        assert!(result.is_ok(), "expected recovery to succeed: {result:?}");
        assert!(runner.run_recorded.iter().any(|call| {
            call.contains("git clone https://github.com/jackin-project/jackin-agent-smith.git")
        }));
    }

    #[tokio::test]
    async fn resolve_agent_repo_uses_run_for_pull_on_clean_repo() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let repo_dir = CachedRepo::new(&paths, &selector).repo_dir;
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();

        let mut runner = FakeRunner::with_capture_queue([
            "git@github.com:jackin-project/jackin-agent-smith.git".to_string(),
            String::new(),      // git status --porcelain (clean)
            "main".to_string(), // git rev-parse --abbrev-ref HEAD
        ]);

        let result = resolve_agent_repo(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
        )
        .await;

        assert!(
            result.is_ok(),
            "expected clean repo update to succeed: {result:?}"
        );
        assert!(
            runner
                .run_recorded
                .iter()
                .any(|call| call.contains("git -C") && call.contains("fetch origin")),
            "expected a git fetch: {:?}",
            runner.run_recorded
        );
        assert!(
            runner
                .run_recorded
                .iter()
                .any(|call| call.contains("git -C") && call.contains("merge --ff-only")),
            "expected a git merge --ff-only: {:?}",
            runner.run_recorded
        );
    }

    #[tokio::test]
    async fn resolve_agent_repo_migrates_legacy_root_repo_to_default_sibling_layout() {
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        let selector = RoleSelector::new(None, "agent-smith");
        let legacy_root = paths.roles_dir.join("agent-smith");
        seed_valid_role_repo(&legacy_root);
        std::fs::create_dir_all(legacy_root.join("branches/feat/caveman-all-install")).unwrap();
        std::fs::write(
            legacy_root.join("branches/feat/caveman-all-install/README.md"),
            "branch cache\n",
        )
        .unwrap();

        let mut runner = FakeRunner::with_capture_queue([
            "git@github.com:jackin-project/jackin-agent-smith.git".to_string(),
            String::new(),      // git status --porcelain (clean)
            "main".to_string(), // git rev-parse --abbrev-ref HEAD
        ]);

        let (cached_repo, _, _) = resolve_agent_repo(
            &paths,
            &selector,
            "https://github.com/jackin-project/jackin-agent-smith.git",
            &mut runner,
            false,
            None,
        )
        .await
        .unwrap();

        assert_eq!(cached_repo.repo_dir, legacy_root.join("default"));
        assert!(legacy_root.join("default/.git").is_dir());
        assert!(!legacy_root.join(".git").exists());
        assert!(
            legacy_root
                .join("branches/feat/caveman-all-install/README.md")
                .is_file()
        );
    }

    /// Materialise a valid role repo at `repo_dir` — `.git`, `Dockerfile`,
    /// and `jackin.role.toml` are all required by `validate_role_repo`.
    fn seed_valid_role_repo(repo_dir: &std::path::Path) {
        std::fs::create_dir_all(repo_dir.join(".git")).unwrap();
        std::fs::write(
            repo_dir.join("Dockerfile"),
            "FROM projectjackin/construct:0.1-trixie\n",
        )
        .unwrap();
        std::fs::write(
            repo_dir.join("jackin.role.toml"),
            r#"version = "v1alpha3"
dockerfile = "Dockerfile"

[claude]
plugins = []
"#,
        )
        .unwrap();
    }

    /// Find the `repo` subdir under the first `role-resolve-*` temp dir
    /// `register_agent_repo` creates inside `data_dir`. Used inside
    /// `git clone` side-effect callbacks where the path isn't known
    /// until the function runs.
    fn first_temp_role_repo(data_dir: &std::path::Path) -> std::path::PathBuf {
        std::fs::read_dir(data_dir)
            .unwrap()
            .filter_map(Result::ok)
            .map(|entry| entry.path())
            .find(|path| {
                path.is_dir()
                    && path
                        .file_name()
                        .and_then(|name| name.to_str())
                        .is_some_and(|name| name.starts_with("role-resolve-"))
            })
            .expect("role registration temp dir should exist before git clone side-effect")
            .join("repo")
    }

    #[tokio::test]
    async fn register_agent_repo_cleans_up_temp_dir_on_validate_failure() {
        // When validation rejects the cloned repo (here: no Dockerfile,
        // no jackin.role.toml — just a `.git` dir), `register_agent_repo`
        // must NOT rename the temp dir into the cache and must NOT call
        // `persist_registration`. The temp dir is cleaned up by tempfile's
        // Drop, so the only assertion is that the cache slot is empty.
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        paths.ensure_base_dirs().unwrap();
        let selector = RoleSelector::new(None, "agent-broken");
        let cached_dir = CachedRepo::new(&paths, &selector).repo_dir;

        let data_dir = paths.data_dir.clone();
        let mut runner = FakeRunner::default();
        runner.side_effects.push((
            "git clone".to_string(),
            // Materialise a `.git` dir but skip the manifest files so
            // `validate_role_repo` rejects the clone.
            Box::new(move || {
                let temp_repo = first_temp_role_repo(&data_dir);
                std::fs::create_dir_all(temp_repo.join(".git")).unwrap();
            }),
        ));

        let persist_called = std::cell::Cell::new(false);
        let err = register_agent_repo(
            &paths,
            &selector,
            "https://github.com/example/agent-broken.git",
            &mut runner,
            false,
            || {
                persist_called.set(true);
                Ok(())
            },
        )
        .await
        .unwrap_err();

        assert!(
            err.downcast_ref::<RepoError>()
                .is_some_and(|e| matches!(e, RepoError::InvalidRoleRepo(_))),
            "expected RepoError::InvalidRoleRepo, got {err:?}"
        );
        assert!(
            !persist_called.get(),
            "persist must not run on validate failure"
        );
        assert!(
            !cached_dir.exists(),
            "cache slot must remain empty when validate fails: {}",
            cached_dir.display()
        );
    }

    #[tokio::test]
    async fn register_agent_repo_aborts_when_persist_registration_fails() {
        // Persist runs after rename, so a persist failure leaves the
        // cache populated but registration un-persisted. Verify the
        // error surfaces with the diagnostic context that points the
        // operator at the inconsistency.
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        paths.ensure_base_dirs().unwrap();
        let selector = RoleSelector::new(None, "agent-persist-fail");
        let cached_dir = CachedRepo::new(&paths, &selector).repo_dir;

        let data_dir = paths.data_dir.clone();
        let mut runner = FakeRunner::default();
        runner.side_effects.push((
            "git clone".to_string(),
            Box::new(move || seed_valid_role_repo(&first_temp_role_repo(&data_dir))),
        ));

        let err = register_agent_repo(
            &paths,
            &selector,
            "https://github.com/example/agent-persist-fail.git",
            &mut runner,
            false,
            || anyhow::bail!("simulated config write failure"),
        )
        .await
        .unwrap_err();

        let chain = format!("{err:?}");
        assert!(
            chain.contains("registration could not be persisted"),
            "error chain must surface persist failure: {chain}"
        );
        assert!(
            cached_dir.join(".git").is_dir(),
            "cache must be populated before persist runs (rename-then-persist invariant)",
        );
    }

    #[tokio::test]
    async fn register_agent_repo_rejects_stale_non_git_directory() {
        // A pre-existing directory at the cache slot that is *not* a
        // git repo must bail rather than overwrite or skip — the
        // operator likely has unsynced work there.
        let temp = tempdir().unwrap();
        let paths = JackinPaths::for_tests(temp.path());
        paths.ensure_base_dirs().unwrap();
        let selector = RoleSelector::new(None, "agent-stale");
        let cached_dir = paths.roles_dir.join("agent-stale");
        std::fs::create_dir_all(&cached_dir).unwrap();
        std::fs::write(cached_dir.join("README"), "operator's lost work\n").unwrap();

        let mut runner = FakeRunner::default();
        let err = register_agent_repo(
            &paths,
            &selector,
            "https://github.com/example/agent-stale.git",
            &mut runner,
            false,
            || Ok(()),
        )
        .await
        .unwrap_err();

        assert!(
            err.to_string()
                .contains("cached role path exists but is not a git repository"),
            "expected stale-non-git bail, got: {err}"
        );
        // Operator's file remains untouched.
        assert_eq!(
            std::fs::read_to_string(cached_dir.join("README")).unwrap(),
            "operator's lost work\n"
        );
    }
}
