Fix #261: port local repository workflows to native Windows

dev-docs/tmux-scenarios/windows-local-repository-prep.json

@@ -0,0 +1,25 @@
+{
+  "config": {
+    "cols": 100,
+    "rows": 32,
+    "history_limit": 2000,
+    "initial_wait_ms": 100,
+    "assert_mode": "strict"
+  },
+  "steps": [
+    { "waitFor": "LLxprt Jefe" },
+    { "key": "i" },
+    { "waitFor": "Issues" },
+    { "key": "Enter" },
+    { "waitFor": "Send to Agent" },
+    { "key": "Enter" },
+    { "waitFor": "Dirty Working Copy" },
+    { "expect": "( Cancel )" },
+    { "capture": "windows-local-repository-dirty-safety" },
+    { "key": "Escape" },
+    { "waitForNot": "Dirty Working Copy" },
+    { "key": "Escape" },
+    { "key": "q" },
+    { "waitForExit": 3000 }
+  ]
+}

project-plans/issue261-plan.md

@@ -0,0 +1,52 @@
+# Issue #261 plan: native Windows repository and GitHub workflows
+
+## Goal
+
+Make local repository, Git, GitHub, issue, and pull-request preparation use
+native executable/argv and filesystem boundaries on Windows, while preserving
+remote Linux shell plans and Unix behavior.
+
+## RED
+
+1. Add a deterministic TUI scenario for the local dirty-copy preparation and
+   confirmation path, with no network operations.
+2. Add executable-resolution tests for Windows `PATH`/`PATHEXT`, paths containing
+   spaces and Unicode, explicit overrides, and missing executables.
+3. Add real-Git integration tests rooted beneath spaces and Unicode that cover
+   clone, refresh/default-branch preparation, linked worktrees, dirty detection,
+   argv-like filenames, and cleanup.
+4. Add pure path-policy tests for mixed/trailing separators, Windows
+   case-insensitive comparison without mutating display values, long paths, and
+   precise UNC rejection.
+5. Add cleanup tests proving no `git clean` is planned, only enumerated untracked
+   paths are removed, owned metadata is retained, and filesystem removal errors
+   stop before tracked changes are reset.
+6. Add GitHub command-boundary tests proving every argument remains a distinct
+   `OsString` and the resolved `gh` executable is used.
+
+## GREEN
+
+1. Introduce a typed local-tool executable resolver shared by Git and GitHub CLI
+   boundaries, including native Windows `PATHEXT` behavior.
+2. Route local Git operations through a typed `GitCommand` boundary with explicit
+   argv, noninteractive environment, typed spawn/exit failures, and no shell.
+3. Route all GitHub CLI operations through a resolved `gh` command constructor;
+   retain existing command-planning/parsing APIs and error categorization.
+4. Introduce a local-path policy for comparison and supported-path validation.
+   Preserve original `PathBuf` values for display and subprocess arguments;
+   reject UNC work directories early with actionable text until they have a
+   dedicated behavioral environment.
+5. Replace `git clean` with `git ls-files --others --exclude-standard -z` plus
+   individually constrained filesystem removals. Surface Windows lock failures
+   and do not proceed to `reset --hard` after partial cleanup failure.
+6. Keep `issue_prep_remote` Unix command strings unchanged and add regression
+   assertions proving that boundary.
+
+## REFACTOR and review
+
+- Keep side effects in Git/GitHub/filesystem boundaries and state transitions
+  deterministic.
+- Verify repository registration, issue and PR handoff call the new boundaries.
+- Audit deletion/reclone ownership and ensure no broad deletion is introduced.
+- Run focused Windows and Unix checks, strict Clippy, locked build/test, and the
+  full CI verification suite.

src/app_input/issue_cleanup.rs

@@ -0,0 +1,188 @@
+//! Ownership-scoped local dirty-worktree cleanup.
+
+use std::collections::HashSet;
+use std::path::{Path, PathBuf};
+
+#[cfg(unix)]
+use std::ffi::OsString;
+#[cfg(unix)]
+use std::os::unix::ffi::OsStringExt;
+
+use super::{git_capture, git_require_success, require_success};
+
+pub(in crate::app_input) fn discard_workdir_changes(work_dir: &Path) -> Result<(), String> {
+    let output = git_capture(
+        work_dir,
+        [
+            "-c",
+            "core.quotepath=false",
+            "ls-files",
+            "--others",
+            "--exclude-standard",
+            "-z",
+        ],
+    )?;
+    require_success(&output, "ls-files --others --exclude-standard -z")?;
+    let untracked = parse_changed_paths(&output.stdout)?;
+    for relative in &untracked {
+        remove_untracked_path(work_dir, relative)?;
+    }
+    remove_empty_untracked_parents(work_dir, &untracked)?;
+    restore_tracked_paths(work_dir)
+}
+
+fn restore_tracked_paths(work_dir: &Path) -> Result<(), String> {
+    let output = git_capture(
+        work_dir,
+        [
+            "-c",
+            "core.quotepath=false",
+            "diff",
+            "--name-only",
+            "-z",
+            "HEAD",
+        ],
+    )?;
+    require_success(&output, "diff --name-only -z HEAD")?;
+    for relative in parse_changed_paths(&output.stdout)? {
+        git_require_success(
+            work_dir,
+            [
+                std::ffi::OsStr::new("restore"),
+                std::ffi::OsStr::new("--source=HEAD"),
+                std::ffi::OsStr::new("--staged"),
+                std::ffi::OsStr::new("--worktree"),
+                std::ffi::OsStr::new("--"),
+                relative.as_os_str(),
+            ],
+        )?;
+    }
+    Ok(())
+}
+
+fn parse_changed_paths(stdout: &[u8]) -> Result<Vec<PathBuf>, String> {
+    let mut paths = parse_paths(stdout)?;
+    paths.retain(|path| !is_owned_path(path));
+    Ok(paths)
+}
+
+fn is_owned_path(path: &Path) -> bool {
+    path.components().next().is_some_and(|component| {
+        let value = component.as_os_str().to_string_lossy();
+        value.eq_ignore_ascii_case(".jefe") || value.eq_ignore_ascii_case(".llxprt")
+    })
+}
+
+fn parse_paths(stdout: &[u8]) -> Result<Vec<PathBuf>, String> {
+    stdout
+        .split(|byte| *byte == 0)
+        .filter(|raw| !raw.is_empty())
+        .map(|raw| {
+            let path = path_from_git_bytes(raw)?;
+            let safe = !path.is_absolute()
+                && path
+                    .components()
+                    .all(|component| matches!(component, std::path::Component::Normal(_)));
+            if safe {
+                Ok(path)
+            } else {
+                Err(format!("git returned an unsafe path: {}", path.display()))
+            }
+        })
+        .collect()
+}
+
+#[cfg(unix)]
+fn path_from_git_bytes(raw: &[u8]) -> Result<PathBuf, String> {
+    if raw.is_empty() {
+        return Err("git returned an empty path".to_owned());
+    }
+    Ok(PathBuf::from(OsString::from_vec(raw.to_vec())))
+}
+
+#[cfg(not(unix))]
+fn path_from_git_bytes(raw: &[u8]) -> Result<PathBuf, String> {
+    std::str::from_utf8(raw)
+        .map(PathBuf::from)
+        .map_err(|error| format!("git returned a non-UTF-8 path: {error}"))
+}
+
+fn remove_untracked_path(work_dir: &Path, relative: &Path) -> Result<(), String> {
+    let path = work_dir.join(relative);
+    let metadata = std::fs::symlink_metadata(&path).map_err(|error| {
+        format!(
+            "Failed to inspect untracked path {} before cleanup: {error}. Close programs using the file and retry.",
+            path.display()
+        )
+    })?;
+    let result = if metadata.is_dir() && !metadata.file_type().is_symlink() {
+        std::fs::remove_dir(&path)
+    } else {
+        std::fs::remove_file(&path)
+    };
+    result.map_err(|error| {
+        format!(
+            "Failed to remove untracked path {}: {error}. Close programs using the file and retry.",
+            path.display()
+        )
+    })
+}
+
+fn remove_empty_untracked_parents(work_dir: &Path, paths: &[PathBuf]) -> Result<(), String> {
+    let mut candidates = HashSet::new();
+    for relative in paths {
+        let mut parent = relative.parent();
+        while let Some(candidate) = parent.filter(|value| !value.as_os_str().is_empty()) {
+            candidates.insert(candidate.to_path_buf());
+            parent = candidate.parent();
+        }
+    }
+    let mut candidates = candidates.into_iter().collect::<Vec<_>>();
+    candidates.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
+    for candidate in candidates {
+        let directory = work_dir.join(candidate);
+        match std::fs::remove_dir(&directory) {
+            Ok(()) => {}
+            Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
+            Err(error) if is_directory_not_empty(&error, &directory) => {}
+            Err(error) => {
+                return Err(format!(
+                    "Failed to remove empty untracked directory {}: {error}",
+                    directory.display()
+                ));
+            }
+        }
+    }
+    Ok(())
+}
+
+fn is_directory_not_empty(error: &std::io::Error, _path: &Path) -> bool {
+    matches!(error.raw_os_error(), Some(39 | 66 | 145))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn owned_metadata_names_are_ascii_case_insensitive() {
+        assert!(is_owned_path(Path::new(".JEFE/cache.json")));
+        assert!(is_owned_path(Path::new(".LlXpRt/LLXPRT.md")));
+        assert!(!is_owned_path(Path::new("nested/.JEFE/cache.json")));
+    }
+
+    #[test]
+    fn shared_parent_already_removed_is_successful() {
+        let root = tempfile::tempdir()
+            .unwrap_or_else(|error| panic!("create cleanup test directory: {error}"));
+        let shared = root.path().join("shared");
+        std::fs::create_dir(&shared)
+            .unwrap_or_else(|error| panic!("create shared directory: {error}"));
+        let paths = vec![PathBuf::from("shared/one"), PathBuf::from("shared/two")];
+
+        let result = remove_empty_untracked_parents(root.path(), &paths);
+
+        assert!(result.is_ok(), "shared-parent cleanup failed: {result:?}");
+        assert!(!shared.exists());
+    }
+}

src/app_input/issue_git_prep.rs

@@ -15,15 +15,18 @@
 //! `git`.
 
 use std::path::Path;
-use std::process::{Command, Stdio};
+use std::process::Stdio;
 
+#[path = "issue_cleanup.rs"]
+mod issue_cleanup;
 /// Safety guards for the destructive force-reclone path (issue #190).
 ///
 /// Re-exported here so callers can reference
 /// `super::issue_git_prep::validate_reclone_target` regardless of the internal
 /// module split.
 #[path = "reclone_safety.rs"]
 mod reclone_safety;
+pub(super) use issue_cleanup::discard_workdir_changes;
 pub(super) use reclone_safety::validate_reclone_target;
 
 /// Paths that jefe/llxprt own and that must never count as "dirty" working
@@ -524,72 +527,15 @@ pub(super) fn prepare_issue_workdir(work_dir: &Path) -> PrepResult {
     checkout_and_pull(work_dir, &branch)
 }
 
-/// Discard uncommitted/untracked changes in the working copy, preserving
-/// jefe/ and llxprt-owned paths and all `.gitignore`-ed files.
-///
-/// Runs `git clean -fd` first (remove untracked files, respecting
-/// `.gitignore`) then `git reset --hard` (discard tracked changes). Running
-/// `clean` first means if it fails, the user's tracked modifications are
-/// still intact — only untracked files would have been affected. Exclusions
-/// for `.jefe/` and `.llxprt/` are derived from [`IGNORED_PREFIXES`].
-///
-/// Does **not** use `-x`, so gitignored files like `.env`, `node_modules/`,
-/// and build artifacts are preserved.
-///
-/// # Non-atomicity
-///
-/// These two operations are not atomic. If `reset --hard` fails after
-/// `clean -fd` has already removed untracked files, those untracked files
-/// are gone. The user has explicitly confirmed this destructive operation
-/// via the `ConfirmIssueDirtyCopy` modal.
-///
-/// # Limitation: tracked `.jefe/` files
-///
-/// While `git clean -e` prevents removal of *untracked* `.jefe/` files,
-/// `git reset --hard` will revert any *tracked* `.jefe/` files (e.g.,
-/// committed metadata that was locally modified) to their committed state.
-/// In practice this is not an issue because the issue prompt file is
-/// written fresh after cleanup and is never tracked by git.
-pub(super) fn discard_workdir_changes(work_dir: &Path) -> Result<(), String> {
-    // Build clean exclusion args from IGNORED_PREFIXES so the porcelain
-    // dirty-check and the cleanup step can never drift. Each prefix is
-    // added twice: as the directory itself (`.jefe/`) and as a glob for
-    // nested contents (`.jefe/**`).
-    let mut clean_args: Vec<String> = vec!["clean".into(), "-fd".into()];
-    for prefix in IGNORED_PREFIXES {
-        clean_args.push("-e".into());
-        clean_args.push(prefix.into());
-        let glob = format!("{prefix}**");
-        clean_args.push("-e".into());
-        clean_args.push(glob);
-    }
-    let output = git_capture(work_dir, &clean_args)?;
-    // Check exit status before logging so we don't report a partial/failed
-    // clean as if it succeeded.
-    require_success(&output, "clean -fd")?;
-    // Log what was removed so the destructive operation is auditable.
-    let removed = String::from_utf8_lossy(&output.stdout);
-    if !removed.trim().is_empty() {
-        tracing::info!(
-            work_dir = %work_dir.display(),
-            removed = %removed.trim(),
-            "discard_workdir_changes: git clean removed paths"
-        );
-    }
-    // Now discard tracked modifications. Running after clean so if clean
-    // fails, tracked changes are still intact.
-    git_require_success(work_dir, ["reset", "--hard"])?;
-    Ok(())
-}
-
 /// Run `git` with the given args in `work_dir`, capturing output. Returns an
 /// error string on spawn failure (does not inspect exit status).
-fn git_capture<I, S>(work_dir: &Path, args: I) -> Result<std::process::Output, String>
+pub(super) fn git_capture<I, S>(work_dir: &Path, args: I) -> Result<std::process::Output, String>
 where
     I: IntoIterator<Item = S>,
     S: AsRef<std::ffi::OsStr>,
 {
-    Command::new("git")
+    jefe::local_command::command(jefe::local_command::LocalTool::Git)
+        .map_err(|error| error.to_string())?
         .current_dir(work_dir)
         // Fail fast instead of hanging on an interactive credential prompt
         // for private repos over HTTPS (stdout/stderr are piped, not a TTY).
@@ -637,7 +583,7 @@ where
 
 /// Inspect a captured `git` output and return `Err` with stderr/stdout detail
 /// when the exit status was non-zero.
-fn require_success(output: &std::process::Output, context: &str) -> Result<(), String> {
+pub(super) fn require_success(output: &std::process::Output, context: &str) -> Result<(), String> {
     if output.status.success() {
         Ok(())
     } else {

src/app_input/issue_prep.rs

@@ -216,6 +216,7 @@ fn prepare_local(
     policy: DirtyPolicy,
     prompt: &str,
 ) -> Result<PrepOutcome, String> {
+    jefe::services::validate_local_path(work_dir)?;
     let owned_url = identity.map(CloneIdentity::clone_url);
     let expected = identity.map(CloneIdentity::expected_shortform);
     match ensure_workdir_with_origin(work_dir, owned_url.as_deref(), expected)? {