diff --git a/crates/pi-iso/src/lib.rs b/crates/pi-iso/src/lib.rs index af778899d..65d02b73c 100644 --- a/crates/pi-iso/src/lib.rs +++ b/crates/pi-iso/src/lib.rs @@ -119,6 +119,22 @@ impl BackendKind { } } +#[cfg(target_os = "macos")] +const MACOS_AUTO_ORDER: &[BackendKind] = &[BackendKind::Apfs, BackendKind::Zfs, BackendKind::Rcopy]; +#[cfg(target_os = "linux")] +const LINUX_AUTO_ORDER: &[BackendKind] = &[ + BackendKind::Btrfs, + BackendKind::Zfs, + BackendKind::LinuxReflink, + BackendKind::Overlayfs, + BackendKind::Rcopy, +]; +#[cfg(windows)] +const WINDOWS_AUTO_ORDER: &[BackendKind] = + &[BackendKind::WindowsBlockClone, BackendKind::Projfs, BackendKind::Rcopy]; +#[cfg(not(any(target_os = "macos", target_os = "linux", windows)))] +const FALLBACK_AUTO_ORDER: &[BackendKind] = &[BackendKind::Rcopy]; + impl fmt::Display for BackendKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(self.as_str()) @@ -261,52 +277,93 @@ pub fn backend_kind() -> BackendKind { default_backend().kind() } -/// Outcome of [`resolve`]. `kind` is the backend that will actually be -/// used; `fell_back` is `true` when a `preferred` choice (or the -/// platform-native pick) was unusable and the resolver downgraded. -/// `reason` carries the original probe's explanation when available. -#[derive(Debug, Clone)] -pub struct Resolution { - pub kind: BackendKind, - pub fell_back: bool, - pub reason: Option, +/// Backend preference order for automatic isolation on this build target. +/// +/// The order is intentionally broader than [`BackendKind::native`]: it tries +/// filesystem-native snapshot/reflink mechanisms first, then mount/projection +/// overlays, and keeps [`BackendKind::Rcopy`] as the universal final fallback. +pub const fn auto_order() -> &'static [BackendKind] { + #[cfg(target_os = "macos")] + { + MACOS_AUTO_ORDER + } + #[cfg(target_os = "linux")] + { + LINUX_AUTO_ORDER + } + #[cfg(windows)] + { + WINDOWS_AUTO_ORDER + } + #[cfg(not(any(target_os = "macos", target_os = "linux", windows)))] + { + FALLBACK_AUTO_ORDER + } } -/// Pick the best backend available right now. +/// Outcome of [`resolve`]. +/// +/// `kind` is the first host-available backend to try. `candidates` contains +/// every host-available backend in fallback order, starting with `kind`, so +/// callers can retry when a backend is unavailable for a specific filesystem +/// path. `fell_back` is `true` when a `preferred` choice (or earlier automatic +/// candidate) was unusable. `reason` carries the first unavailable probe's +/// explanation when available. +#[derive(Debug, Clone)] +pub struct Resolution { + pub kind: BackendKind, + pub candidates: Vec, + pub fell_back: bool, + pub reason: Option, +} + +/// Pick the best backend whose host-level prerequisites are available. /// /// Caller priority: /// 1. If `preferred` is `Some` and its [`probe`](IsolationBackend::probe) /// reports `available`, use it as-is. -/// 2. Otherwise try the platform-native backend ([`BackendKind::native`]); if -/// it differs from `preferred` and probes available, use it. -/// 3. Otherwise fall back to [`BackendKind::Rcopy`], which is always available. +/// 2. Otherwise walk [`auto_order`], skipping `preferred` if present. +/// 3. [`BackendKind::Rcopy`] is the final automatic candidate and is expected +/// to be available on every platform. /// -/// The unavailable probe's `reason` is carried through `Resolution::reason` -/// so callers can surface it to the user instead of guessing. +/// This is only a host-level probe. Some backends still reject a specific +/// `lower`/`merged` pair at [`IsolationBackend::start`] time (cross-device +/// reflinks, non-subvolume btrfs paths, non-ZFS mountpoints). Callers that can +/// recover should retry the remaining automatic candidates when `start` +/// returns [`IsoError::Unavailable`]. pub fn resolve(preferred: Option) -> Resolution { + let mut reason = None; + let mut candidates = Vec::with_capacity(auto_order().len() + usize::from(preferred.is_some())); + if let Some(p) = preferred { let probe = backend(p).probe(); if probe.available { - return Resolution { kind: p, fell_back: false, reason: None }; + candidates.push(p); + } else { + reason = probe.reason; } - let original_reason = probe.reason; - let native = BackendKind::native(); - if native != p { - let np = backend(native).probe(); - if np.available { - return Resolution { kind: native, fell_back: true, reason: original_reason }; - } + } + + for candidate in auto_order() { + if Some(*candidate) == preferred { + continue; + } + let probe = backend(*candidate).probe(); + if probe.available { + candidates.push(*candidate); + } else if reason.is_none() { + reason = probe.reason; } - return Resolution { - kind: BackendKind::Rcopy, - fell_back: true, - reason: original_reason, - }; } - let native = BackendKind::native(); - let probe = backend(native).probe(); - if probe.available { - return Resolution { kind: native, fell_back: false, reason: None }; + + if candidates.is_empty() { + candidates.push(BackendKind::Rcopy); } - Resolution { kind: BackendKind::Rcopy, fell_back: true, reason: probe.reason } + let kind = candidates[0]; + let fell_back = match preferred { + Some(p) => kind != p, + None => kind != auto_order()[0], + }; + + Resolution { kind, candidates, fell_back, reason } } diff --git a/crates/pi-iso/src/overlayfs.rs b/crates/pi-iso/src/overlayfs.rs index f649632e1..49e2332f1 100644 --- a/crates/pi-iso/src/overlayfs.rs +++ b/crates/pi-iso/src/overlayfs.rs @@ -117,6 +117,10 @@ mod imp { let upper = base.join("upper"); let work = base.join("work"); + remove_dir_if_exists(&upper, "stale overlay upper")?; + remove_dir_if_exists(&work, "stale overlay work")?; + remove_dir_if_exists(&merged, "stale overlay merged")?; + fs::create_dir_all(&upper) .map_err(|err| IsoError::other(format!("create upper dir {}: {err}", upper.display())))?; fs::create_dir_all(&work) @@ -152,23 +156,32 @@ mod imp { pub fn stop(merged: &Path) -> IsoResult<()> { let merged = absolutize(merged); - let flavor = ACTIVE_MOUNTS.lock().remove(&merged); - match flavor { - Some(MountFlavor::Fuse) => fuse_umount(&merged), - Some(MountFlavor::Kernel) | None => { - // `None` covers callers that skipped `start` (probe-style flow) - // or processes that re-attached after a crash; try a kernel - // umount first, fall back to fusermount so we don't silently - // leak a mount. - kernel_umount(&merged).or_else(|err| { - if err.is_unavailable() { - fuse_umount(&merged) - } else { - Err(err) - } - }) - }, + let result = { + let flavor = ACTIVE_MOUNTS.lock().remove(&merged); + match flavor { + Some(MountFlavor::Fuse) => fuse_umount(&merged), + Some(MountFlavor::Kernel) | None => { + // `None` covers callers that skipped `start` (probe-style flow) + // or processes that re-attached after a crash; try a kernel + // umount first, fall back to fusermount so we don't silently + // leak a mount. + kernel_umount(&merged).or_else(|err| { + if err.is_unavailable() { + fuse_umount(&merged) + } else { + Err(err) + } + }) + }, + } + }; + result?; + + if let Some(base) = merged.parent() { + remove_dir_if_exists(&base.join("upper"), "overlay upper")?; + remove_dir_if_exists(&base.join("work"), "overlay work")?; } + remove_dir_if_exists(&merged, "overlay merged") } fn kernel_mount(merged: &Path, opts: &str) -> IsoResult<()> { @@ -321,6 +334,14 @@ mod imp { } } + fn remove_dir_if_exists(path: &Path, label: &str) -> IsoResult<()> { + match fs::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(IsoError::other(format!("remove {label} {}: {err}", path.display()))), + } + } + fn to_cstring(bytes: &[u8], label: &str) -> IsoResult { CString::new(bytes) .map_err(|err| IsoError::other(format!("{label} path contains NUL byte: {err}"))) diff --git a/crates/pi-iso/src/projfs.rs b/crates/pi-iso/src/projfs.rs index 8dfe63c35..13f0ed7fb 100644 --- a/crates/pi-iso/src/projfs.rs +++ b/crates/pi-iso/src/projfs.rs @@ -82,17 +82,29 @@ fn x64_under_arm64_emulation() -> bool { if !cfg!(windows) || !cfg!(target_arch = "x86_64") { return false; } - let matches_arm64 = |var: &str| { - std::env::var(var) - .ok() - .is_some_and(|v| v.eq_ignore_ascii_case("ARM64")) - }; - matches_arm64("PROCESSOR_ARCHITEW6432") || matches_arm64("PROCESSOR_ARCHITECTURE") + let env_value = |var: &str| std::env::var(var).ok(); + vars_indicate_arm64_emulation( + env_value("PROCESSOR_ARCHITEW6432").as_deref(), + env_value("PROCESSOR_ARCHITECTURE").as_deref(), + ) +} + +fn vars_indicate_arm64_emulation(wow64_arch: Option<&str>, process_arch: Option<&str>) -> bool { + let matches_arm64 = |value: Option<&str>| value.is_some_and(|v| v.eq_ignore_ascii_case("ARM64")); + matches_arm64(wow64_arch) || matches_arm64(process_arch) } #[cfg(test)] mod tests { - use super::x64_under_arm64_emulation; + use super::{vars_indicate_arm64_emulation, x64_under_arm64_emulation}; + + #[test] + fn detects_windows_arm64_emulation_markers() { + assert!(vars_indicate_arm64_emulation(Some("ARM64"), None)); + assert!(vars_indicate_arm64_emulation(None, Some("arm64"))); + assert!(!vars_indicate_arm64_emulation(Some("AMD64"), Some("x86"))); + assert!(!vars_indicate_arm64_emulation(None, None)); + } #[test] fn returns_false_off_windows_or_non_x64() { @@ -139,8 +151,9 @@ mod imp { Storage::ProjectedFileSystem::{ PRJ_CALLBACK_DATA, PRJ_CALLBACKS, PRJ_CB_DATA_FLAG_ENUM_RESTART_SCAN, PRJ_CB_DATA_FLAG_ENUM_RETURN_SINGLE_ENTRY, PRJ_DIR_ENTRY_BUFFER_HANDLE, - PRJ_FILE_BASIC_INFO, PRJ_NAMESPACE_VIRTUALIZATION_CONTEXT, PRJ_PLACEHOLDER_INFO, - PRJ_STARTVIRTUALIZING_OPTIONS, + PRJ_EXT_INFO_TYPE_SYMLINK, PRJ_EXTENDED_INFO, PRJ_EXTENDED_INFO_0, + PRJ_EXTENDED_INFO_0_0, PRJ_FILE_BASIC_INFO, PRJ_NAMESPACE_VIRTUALIZATION_CONTEXT, + PRJ_PLACEHOLDER_INFO, PRJ_STARTVIRTUALIZING_OPTIONS, }, System::{ Com::CoCreateGuid, @@ -176,7 +189,7 @@ mod imp { PRJ_DIR_ENTRY_BUFFER_HANDLE, PCWSTR, *const PRJ_FILE_BASIC_INFO, - *const c_void, + *const PRJ_EXTENDED_INFO, ) -> HRESULT; type PrjWriteFileDataFn = unsafe extern "system" fn( PRJ_NAMESPACE_VIRTUALIZATION_CONTEXT, @@ -185,11 +198,12 @@ mod imp { u64, u32, ) -> HRESULT; - type PrjWritePlaceholderInfoFn = unsafe extern "system" fn( + type PrjWritePlaceholderInfo2Fn = unsafe extern "system" fn( PRJ_NAMESPACE_VIRTUALIZATION_CONTEXT, PCWSTR, *const PRJ_PLACEHOLDER_INFO, u32, + *const PRJ_EXTENDED_INFO, ) -> HRESULT; struct ProjfsApi { @@ -203,7 +217,7 @@ mod imp { prj_stop_virtualizing: PrjStopVirtualizingFn, prj_fill_dir_entry_buffer2: PrjFillDirEntryBuffer2Fn, prj_write_file_data: PrjWriteFileDataFn, - prj_write_placeholder_info: PrjWritePlaceholderInfoFn, + prj_write_placeholder_info2: PrjWritePlaceholderInfo2Fn, } unsafe impl Send for ProjfsApi {} @@ -263,17 +277,18 @@ mod imp { PrjFillDirEntryBuffer2Fn ), prj_write_file_data: load_symbol!("PrjWriteFileData", PrjWriteFileDataFn), - prj_write_placeholder_info: load_symbol!( - "PrjWritePlaceholderInfo", - PrjWritePlaceholderInfoFn + prj_write_placeholder_info2: load_symbol!( + "PrjWritePlaceholderInfo2", + PrjWritePlaceholderInfo2Fn ), }) } } struct DirectoryEntry { - name_wide: Vec, - basic_info: PRJ_FILE_BASIC_INFO, + name_wide: Vec, + basic_info: PRJ_FILE_BASIC_INFO, + symlink_target: Option>, } #[derive(Default)] @@ -538,12 +553,16 @@ mod imp { }; if matched { + let extended_info = symlink_extended_info(entry.symlink_target.as_deref()); + let extended_info_ptr = extended_info + .as_ref() + .map_or(std::ptr::null(), |info| info as *const _); let hr = unsafe { (context.api.prj_fill_dir_entry_buffer2)( dir_entry_buffer_handle, entry.name_wide.as_ptr(), &raw const entry.basic_info, - std::ptr::null(), + extended_info_ptr, ) }; if is_failed(hr) { @@ -572,10 +591,18 @@ mod imp { let relative_path = callback_relative_path(callback_data); let source_path = context.lower_root.join(relative_path); - let metadata = match fs::metadata(&source_path) { + let metadata = match fs::symlink_metadata(&source_path) { Ok(metadata) => metadata, Err(err) => return io_error_to_hresult(&err), }; + let symlink_target = match symlink_target_wide(&source_path, &metadata) { + Ok(target) => target, + Err(err) => return io_error_to_hresult(&err), + }; + let extended_info = symlink_extended_info(symlink_target.as_deref()); + let extended_info_ptr = extended_info + .as_ref() + .map_or(std::ptr::null(), |info| info as *const _); let placeholder_info = PRJ_PLACEHOLDER_INFO { FileBasicInfo: to_basic_info(&metadata), ..Default::default() }; @@ -587,11 +614,12 @@ mod imp { }; unsafe { - (context.api.prj_write_placeholder_info)( + (context.api.prj_write_placeholder_info2)( callback_data.NamespaceVirtualizationContext, destination, &raw const placeholder_info, mem::size_of::() as u32, + extended_info_ptr, ) } } @@ -706,18 +734,21 @@ mod imp { let mut entries = Vec::new(); for entry in fs::read_dir(&source_dir)? { let entry = entry?; - let metadata = entry.metadata()?; + let path = entry.path(); + let metadata = fs::symlink_metadata(&path)?; + let symlink_target = symlink_target_wide(&path, &metadata)?; let name = entry.file_name(); let mut name_wide = to_wide(name.as_os_str()); if name_wide.is_empty() { continue; } entries.push(DirectoryEntry { - name_wide: { + name_wide: { name_wide.shrink_to_fit(); name_wide }, basic_info: to_basic_info(&metadata), + symlink_target, }); } @@ -742,6 +773,25 @@ mod imp { } } + fn symlink_target_wide(path: &Path, metadata: &fs::Metadata) -> io::Result>> { + if !metadata.file_type().is_symlink() { + return Ok(None); + } + let mut target = to_wide(fs::read_link(path)?.as_os_str()); + target.shrink_to_fit(); + Ok(Some(target)) + } + + fn symlink_extended_info(target: Option<&[u16]>) -> Option { + target.map(|target| PRJ_EXTENDED_INFO { + InfoType: PRJ_EXT_INFO_TYPE_SYMLINK, + NextInfoOffset: 0, + Anonymous: PRJ_EXTENDED_INFO_0 { + Symlink: PRJ_EXTENDED_INFO_0_0 { TargetName: target.as_ptr() }, + }, + }) + } + fn callback_relative_path(callback_data: &PRJ_CALLBACK_DATA) -> PathBuf { if callback_data.FilePathName.is_null() { return PathBuf::new(); diff --git a/crates/pi-iso/src/rcopy.rs b/crates/pi-iso/src/rcopy.rs index 95f72d111..a75edbae7 100644 --- a/crates/pi-iso/src/rcopy.rs +++ b/crates/pi-iso/src/rcopy.rs @@ -35,18 +35,19 @@ impl IsolationBackend for RcopyBackend { fn start(&self, lower: &Path, merged: &Path) -> IsoResult<()> { let lower = canonical_existing_dir(lower)?; - prepare_destination(merged)?; + let merged = absolutize(merged); + prepare_destination(&merged)?; if is_git_worktree(&lower) { - git_worktree_add(&lower, merged)?; + git_worktree_add(&lower, &merged)?; // `worktree add --detach HEAD` lands on a clean checkout. omp // (and friends) expect `merged` to mirror `lower`'s **live** // working tree, so seed the index + working tree + untracked // files exactly as they exist in lower. No applyBaseline call // in the caller — every backend's post-`start` invariant is // the same. - seed_dirty_state(&lower, merged) + seed_dirty_state(&lower, &merged) } else { - recursive_copy(&lower, merged) + recursive_copy(&lower, &merged) } } @@ -83,6 +84,14 @@ fn canonical_existing_dir(path: &Path) -> IsoResult { Ok(std::fs::canonicalize(&resolved).unwrap_or(resolved)) } +fn absolutize(path: &Path) -> PathBuf { + if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir().map_or_else(|_| path.to_path_buf(), |cwd| cwd.join(path)) + } +} + fn prepare_destination(merged: &Path) -> IsoResult<()> { if let Some(parent) = merged.parent() { std::fs::create_dir_all(parent) diff --git a/crates/pi-iso/src/zfs.rs b/crates/pi-iso/src/zfs.rs index bc5d34863..f89abfd14 100644 --- a/crates/pi-iso/src/zfs.rs +++ b/crates/pi-iso/src/zfs.rs @@ -128,10 +128,14 @@ mod imp { match dataset_for_mountpoint(&merged)? { Some(dataset) => { let origin = zfs_get_value("origin", &dataset)?; - run_zfs_other(["destroy", dataset.as_str()])?; - if is_own_snapshot(&origin) { - run_zfs_other(["destroy", origin.as_str()])?; + if !is_own_clone(&dataset, &origin) { + return Err(IsoError::other(format!( + "refusing to destroy unrelated ZFS dataset {dataset} mounted at {}", + merged.display() + ))); } + run_zfs_other(["destroy", dataset.as_str()])?; + run_zfs_other(["destroy", origin.as_str()])?; Ok(()) }, None => match fs::remove_dir_all(&merged) { @@ -306,16 +310,27 @@ mod imp { hash } - fn is_own_snapshot(snapshot: &str) -> bool { - let Some((_, name)) = snapshot.rsplit_once('@') else { - return false; - }; + fn is_own_name(name: &str) -> bool { name.starts_with(SNAP_PREFIX) && name[SNAP_PREFIX.len()..] .bytes() .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b':')) } + fn is_own_snapshot(snapshot: &str) -> bool { + let Some((_, name)) = snapshot.rsplit_once('@') else { + return false; + }; + is_own_name(name) + } + + fn is_own_clone(dataset: &str, origin: &str) -> bool { + let Some(name) = dataset.rsplit('/').next() else { + return false; + }; + is_own_name(name) && is_own_snapshot(origin) + } + fn absolute_path(path: &Path) -> PathBuf { if path.is_absolute() { path.to_path_buf() diff --git a/crates/pi-natives/src/iso.rs b/crates/pi-natives/src/iso.rs index 5f67f7467..30a3c366c 100644 --- a/crates/pi-natives/src/iso.rs +++ b/crates/pi-natives/src/iso.rs @@ -62,13 +62,15 @@ pub struct IsoProbeResult { /// Outcome of [`iso_resolve`]. #[napi(object)] pub struct IsoResolveResult { - /// Backend that will actually be used. - pub kind: IsoBackendKind, + /// Backend that will actually be tried first. + pub kind: IsoBackendKind, + /// Host-available backends in retry order, starting with `kind`. + pub candidates: Vec, /// True when the resolver fell back from `preferred` (or from the - /// platform native) to a different backend. - pub fell_back: bool, + /// first automatic candidate) to a different backend. + pub fell_back: bool, /// Human-readable reason for the fallback, if any. - pub reason: Option, + pub reason: Option, } /// One entry in an [`IsoDiff`]. @@ -113,9 +115,14 @@ pub fn iso_probe(kind: Option) -> IsoProbeResult { pub fn iso_resolve(preferred: Option) -> IsoResolveResult { let resolution = pi_iso::resolve(preferred.map(from_napi_kind)); IsoResolveResult { - kind: to_napi_kind(resolution.kind), - fell_back: resolution.fell_back, - reason: resolution.reason, + kind: to_napi_kind(resolution.kind), + candidates: resolution + .candidates + .into_iter() + .map(to_napi_kind) + .collect(), + fell_back: resolution.fell_back, + reason: resolution.reason, } } diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index acde96d3e..77d0e4df0 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -12,7 +12,6 @@ use std::{ }; use anyhow::{Error, Result}; -use bytes::Bytes; use brush_builtins::{BuiltinSet, default_builtins}; use brush_core::{ ExecutionContext, ExecutionControlFlow, ExecutionExitCode, ExecutionResult, ProcessGroupPolicy, @@ -21,6 +20,7 @@ use brush_core::{ env::EnvironmentScope, openfiles::{self, OpenFile, OpenFiles}, }; +use bytes::Bytes; use clap::Parser; #[cfg(not(unix))] use tokio::io::AsyncReadExt as _; @@ -971,23 +971,21 @@ async fn run_shell_command_streams( if !stdout_finished || !stderr_finished { reader_cancel.cancel(); } - if !stdout_finished { - if time::timeout(READER_SHUTDOWN_TIMEOUT, &mut stdout_handle) + if !stdout_finished + && time::timeout(READER_SHUTDOWN_TIMEOUT, &mut stdout_handle) .await .is_err() - { - stdout_handle.abort(); - let _ = stdout_handle.await; - } + { + stdout_handle.abort(); + let _ = stdout_handle.await; } - if !stderr_finished { - if time::timeout(READER_SHUTDOWN_TIMEOUT, &mut stderr_handle) + if !stderr_finished + && time::timeout(READER_SHUTDOWN_TIMEOUT, &mut stderr_handle) .await .is_err() - { - stderr_handle.abort(); - let _ = stderr_handle.await; - } + { + stderr_handle.abort(); + let _ = stderr_handle.await; } cancel_bridge.abort(); let _ = cancel_bridge.await; diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bee84668c..fbe76cc3f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Added - Added new `task.isolation.mode` values `auto`, `apfs`, `btrfs`, `zfs`, `reflink`, `overlayfs`, `projfs`, `block-clone`, and `rcopy` for native PAL-backed task isolation backends @@ -13,6 +14,8 @@ ### Fixed +- Fixed worktree delta capture to include previously untracked file state by baselining untracked patches for both snapshots +- Fixed task isolation startup to try alternate PAL backends when the preferred one is unavailable, allowing successful fallback instead of immediate failure - Mapped legacy `task.isolation.mode` values `worktree`, `fuse-overlay`, and `fuse-projfs` to their new equivalents during settings migration to preserve behavior with older configs ## [14.9.8] - 2026-05-12 diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index e295b0d01..f79339f67 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -2,10 +2,13 @@ import type { Dirent } from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -import { IsoBackendKind, isoResolve, isoStart, isoStop } from "@oh-my-pi/pi-natives"; +import * as natives from "@oh-my-pi/pi-natives"; import { getWorktreeDir, logger, Snowflake } from "@oh-my-pi/pi-utils"; import * as git from "../utils/git"; +const { IsoBackendKind } = natives; +type IsoBackendKind = natives.IsoBackendKind; + /** Baseline state for a single git repository. */ export interface RepoBaseline { repoRoot: string; @@ -13,6 +16,7 @@ export interface RepoBaseline { staged: string; unstaged: string; untracked: string[]; + untrackedPatch: string; } /** Baseline state for the project, including any nested git repos. */ @@ -79,12 +83,49 @@ async function discoverNestedRepos(repoRoot: string): Promise { return result; } +async function captureUntrackedPatch(repoRoot: string, untracked: readonly string[]): Promise { + if (untracked.length === 0) return ""; + const nullPath = getGitNoIndexNullPath(); + const untrackedDiffs = await Promise.all( + untracked.map(entry => + git.diff(repoRoot, { + allowFailure: true, + binary: true, + noIndex: { left: nullPath, right: entry }, + }), + ), + ); + return untrackedDiffs.filter(diff => diff.trim()).join("\n"); +} + async function captureRepoBaseline(repoRoot: string): Promise { const headCommit = (await git.head.sha(repoRoot)) ?? ""; const staged = await git.diff(repoRoot, { binary: true, cached: true }); const unstaged = await git.diff(repoRoot, { binary: true }); const untracked = await git.ls.untracked(repoRoot); - return { repoRoot, headCommit, staged, unstaged, untracked }; + const untrackedPatch = await captureUntrackedPatch(repoRoot, untracked); + return { repoRoot, headCommit, staged, unstaged, untracked, untrackedPatch }; +} + +async function writeSyntheticTree(repoDir: string, baseTreeish: string, patches: readonly string[]): Promise { + const tempIndex = path.join(os.tmpdir(), `omp-task-index-${Snowflake.next()}`); + try { + await git.readTree(repoDir, baseTreeish, { + env: { GIT_INDEX_FILE: tempIndex }, + }); + for (const patch of patches) { + if (!patch.trim()) continue; + await git.patch.applyText(repoDir, patch, { + cached: true, + env: { GIT_INDEX_FILE: tempIndex }, + }); + } + return await git.writeTree(repoDir, { + env: { GIT_INDEX_FILE: tempIndex }, + }); + } finally { + await fs.rm(tempIndex, { force: true }); + } } export async function captureBaseline(repoRoot: string): Promise { @@ -99,87 +140,23 @@ export async function captureBaseline(repoRoot: string): Promise { - // Check if HEAD advanced (task committed changes) const currentHead = (await git.head.sha(repoDir)) ?? ""; - const headAdvanced = currentHead && currentHead !== rb.headCommit; + const currentStaged = await git.diff(repoDir, { binary: true, cached: true }); + const currentUnstaged = await git.diff(repoDir, { binary: true }); + const currentUntracked = await git.ls.untracked(repoDir); + const currentUntrackedPatch = await captureUntrackedPatch(repoDir, currentUntracked); - if (headAdvanced) { - // HEAD moved: use diff-tree to capture committed changes, plus any uncommitted on top - const parts: string[] = []; + const baselineTree = await writeSyntheticTree(repoDir, rb.headCommit, [rb.staged, rb.unstaged, rb.untrackedPatch]); + const currentTree = await writeSyntheticTree(repoDir, currentHead, [ + currentStaged, + currentUnstaged, + currentUntrackedPatch, + ]); - // Committed changes since baseline - const committedDiff = await git.diff.tree(repoDir, rb.headCommit, currentHead, { - allowFailure: true, - binary: true, - }); - if (committedDiff.trim()) parts.push(committedDiff); - - // Uncommitted changes on top of the new HEAD - const staged = await git.diff(repoDir, { binary: true, cached: true }); - const unstaged = await git.diff(repoDir, { binary: true }); - if (staged.trim()) parts.push(staged); - if (unstaged.trim()) parts.push(unstaged); - - // New untracked files (relative to both baseline and current tracking) - const currentUntracked = await git.ls.untracked(repoDir); - const baselineUntracked = new Set(rb.untracked); - const newUntracked = currentUntracked.filter(entry => !baselineUntracked.has(entry)); - if (newUntracked.length > 0) { - const nullPath = getGitNoIndexNullPath(); - const untrackedDiffs = await Promise.all( - newUntracked.map(entry => - git.diff(repoDir, { - allowFailure: true, - binary: true, - noIndex: { left: nullPath, right: entry }, - }), - ), - ); - parts.push(...untrackedDiffs.filter(d => d.trim())); - } - - return parts.join("\n"); - } - - // HEAD unchanged: use temp index approach (subtracts baseline from delta) - const tempIndex = path.join(os.tmpdir(), `omp-task-index-${Snowflake.next()}`); - try { - await git.readTree(repoDir, rb.headCommit, { - env: { GIT_INDEX_FILE: tempIndex }, - }); - await git.patch.applyText(repoDir, rb.staged, { - cached: true, - env: { GIT_INDEX_FILE: tempIndex }, - }); - await git.patch.applyText(repoDir, rb.unstaged, { - cached: true, - env: { GIT_INDEX_FILE: tempIndex }, - }); - const diff = await git.diff(repoDir, { - binary: true, - env: { GIT_INDEX_FILE: tempIndex }, - }); - - const currentUntracked = await git.ls.untracked(repoDir); - const baselineUntracked = new Set(rb.untracked); - const newUntracked = currentUntracked.filter(entry => !baselineUntracked.has(entry)); - - if (newUntracked.length === 0) return diff; - - const nullPath = getGitNoIndexNullPath(); - const untrackedDiffs = await Promise.all( - newUntracked.map(entry => - git.diff(repoDir, { - allowFailure: true, - binary: true, - noIndex: { left: nullPath, right: entry }, - }), - ), - ); - return `${diff}${diff && !diff.endsWith("\n") ? "\n" : ""}${untrackedDiffs.join("\n")}`; - } finally { - await fs.rm(tempIndex, { force: true }); - } + return git.diff.tree(repoDir, baselineTree, currentTree, { + allowFailure: true, + binary: true, + }); } export interface NestedRepoPatch { @@ -328,6 +305,11 @@ export interface IsolationHandle { * caller learns about that through `IsolationHandle.fellBack` + * `fallbackReason`. */ + +function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + export async function ensureIsolation( baseCwd: string, id: string, @@ -338,28 +320,38 @@ export async function ensureIsolation( const baseDir = getWorktreeDir(encodedProject, id); const mergedDir = path.join(baseDir, "merged"); - const resolution = isoResolve(preferred ?? null); + const resolution = natives.isoResolve(preferred ?? null); + const candidates = resolution.candidates.length > 0 ? resolution.candidates : [resolution.kind]; + let fallbackReason = resolution.reason ?? null; - await fs.rm(baseDir, { recursive: true, force: true }); - try { - await isoStart(resolution.kind, repoRoot, mergedDir); - return { - mergedDir, - backend: resolution.kind, - fellBack: resolution.fellBack, - fallbackReason: resolution.reason ?? null, - }; - } catch (err) { + for (const candidate of candidates) { await fs.rm(baseDir, { recursive: true, force: true }); - throw err; + try { + await natives.isoStart(candidate, repoRoot, mergedDir); + return { + mergedDir, + backend: candidate, + fellBack: candidate !== resolution.kind || resolution.fellBack, + fallbackReason, + }; + } catch (err) { + await fs.rm(baseDir, { recursive: true, force: true }); + const message = errorMessage(err); + if (!natives.isoIsUnavailableError(message)) { + throw err; + } + fallbackReason ??= message; + } } + + throw new Error(fallbackReason ?? "No isolation backend is available."); } /** Tear down a handle returned by {@link ensureIsolation}. */ export async function cleanupIsolation(handle: IsolationHandle): Promise { try { try { - await isoStop(handle.backend, handle.mergedDir); + await natives.isoStop(handle.backend, handle.mergedDir); } catch (err) { logger.warn("isolation backend stop failed during cleanup", { backend: handle.backend, diff --git a/packages/coding-agent/src/utils/git.ts b/packages/coding-agent/src/utils/git.ts index 2ed50f7dc..36745a4f1 100644 --- a/packages/coding-agent/src/utils/git.ts +++ b/packages/coding-agent/src/utils/git.ts @@ -895,6 +895,11 @@ export async function readTree( await runEffect(cwd, ["read-tree", treeish], options); } +/** Write the current index as a tree and return its object id. */ +export async function writeTree(cwd: string, options: Pick = {}): Promise { + return (await runText(cwd, ["write-tree"], options)).trim(); +} + // ════════════════════════════════════════════════════════════════════════════ // API: show // ════════════════════════════════════════════════════════════════════════════ diff --git a/packages/coding-agent/test/task/worktree.test.ts b/packages/coding-agent/test/task/worktree.test.ts index 3b1615501..9fbdca9a7 100644 --- a/packages/coding-agent/test/task/worktree.test.ts +++ b/packages/coding-agent/test/task/worktree.test.ts @@ -2,29 +2,18 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -import { getGitNoIndexNullPath, mergeTaskBranches } from "../../src/task/worktree"; +import * as natives from "@oh-my-pi/pi-natives"; +import { + captureBaseline, + captureDeltaPatch, + ensureIsolation, + getGitNoIndexNullPath, + mergeTaskBranches, + parseIsolationMode, +} from "../../src/task/worktree"; -const isoStartMock = vi.fn(); -const isoStopMock = vi.fn(); -const isoIsUnavailableErrorMock = vi.fn( - (message: string) => typeof message === "string" && message.startsWith("ISO_UNAVAILABLE:"), -); const tempDirs: string[] = []; -// Numeric mirror of the napi-generated const-enum so the production code's -// `IsoBackendKind.Overlayfs` references resolve without loading the addon. -const IsoBackendKind = { Apfs: 0, Overlayfs: 1, Projfs: 2, Rcopy: 3 } as const; - -vi.mock("@oh-my-pi/pi-natives", () => ({ - IsoBackendKind, - isoBackend: vi.fn(() => IsoBackendKind.Rcopy), - isoDiff: vi.fn(), - isoIsUnavailableError: isoIsUnavailableErrorMock, - isoProbe: vi.fn(() => ({ available: true, reason: null, kind: IsoBackendKind.Rcopy })), - isoStart: isoStartMock, - isoStop: isoStopMock, -})); - async function runGit(repo: string, args: string[]): Promise { const proc = Bun.spawn(["git", ...args], { cwd: repo, @@ -70,6 +59,49 @@ describe("worktree isolation helpers", () => { expect(getGitNoIndexNullPath()).toBe(expected); }); + it("maps every isolation mode to the native backend contract", () => { + expect(parseIsolationMode("none")).toBeUndefined(); + expect(parseIsolationMode("auto")).toBeUndefined(); + expect(parseIsolationMode("apfs")).toBe(natives.IsoBackendKind.Apfs); + expect(parseIsolationMode("btrfs")).toBe(natives.IsoBackendKind.Btrfs); + expect(parseIsolationMode("zfs")).toBe(natives.IsoBackendKind.Zfs); + expect(parseIsolationMode("reflink")).toBe(natives.IsoBackendKind.LinuxReflink); + expect(parseIsolationMode("overlayfs")).toBe(natives.IsoBackendKind.Overlayfs); + expect(parseIsolationMode("fuse-overlay")).toBe(natives.IsoBackendKind.Overlayfs); + expect(parseIsolationMode("projfs")).toBe(natives.IsoBackendKind.Projfs); + expect(parseIsolationMode("fuse-projfs")).toBe(natives.IsoBackendKind.Projfs); + expect(parseIsolationMode("block-clone")).toBe(natives.IsoBackendKind.WindowsBlockClone); + expect(parseIsolationMode("rcopy")).toBe(natives.IsoBackendKind.Rcopy); + expect(parseIsolationMode("worktree")).toBe(natives.IsoBackendKind.Rcopy); + }); + + it("retries isoResolve candidates when a backend is path-unavailable", async () => { + const { repo } = await createGitRepo(); + const unavailable = new Error("ISO_UNAVAILABLE: btrfs source is not a subvolume"); + const isoResolve = vi.spyOn(natives, "isoResolve").mockReturnValue({ + kind: natives.IsoBackendKind.Btrfs, + candidates: [natives.IsoBackendKind.Btrfs, natives.IsoBackendKind.Rcopy], + fellBack: false, + reason: undefined, + }); + const isoStart = vi + .spyOn(natives, "isoStart") + .mockRejectedValueOnce(unavailable) + .mockResolvedValueOnce(undefined); + vi.spyOn(natives, "isoIsUnavailableError").mockImplementation(message => message.startsWith("ISO_UNAVAILABLE:")); + + const handle = await ensureIsolation(repo, "retry-path-unavailable"); + + expect(isoResolve).toHaveBeenCalledWith(null); + expect(isoStart.mock.calls.map(call => call[0])).toEqual([ + natives.IsoBackendKind.Btrfs, + natives.IsoBackendKind.Rcopy, + ]); + expect(handle.backend).toBe(natives.IsoBackendKind.Rcopy); + expect(handle.fellBack).toBe(true); + expect(handle.fallbackReason).toBe(unavailable.message); + }); + it("does not pop an unrelated pre-existing stash when the working tree is clean", async () => { const { repo } = await createGitRepo(); await fs.writeFile(path.join(repo, "preexisting.txt"), "user stash\n"); @@ -103,4 +135,25 @@ describe("worktree isolation helpers", () => { expect(await runGit(repo, ["diff", "--cached", "--", "staged.txt"])).toContain("+local staged change"); expect(await runGit(repo, ["stash", "list"])).toBe(""); }); + + it("subtracts baseline dirty state even when the task commits it", async () => { + const { repo } = await createGitRepo(); + await fs.writeFile(path.join(repo, "merged.txt"), "baseline dirty change\n"); + await fs.writeFile(path.join(repo, "preexisting.txt"), "baseline untracked\n"); + const baseline = await captureBaseline(repo); + + await runGit(repo, ["add", "-A"]); + await runGit(repo, ["commit", "-m", "baseline committed inside isolation"]); + await fs.writeFile(path.join(repo, "task.txt"), "task output\n"); + await runGit(repo, ["add", "task.txt"]); + await runGit(repo, ["commit", "-m", "task output"]); + + const delta = await captureDeltaPatch(repo, baseline); + + expect(delta.nestedPatches).toEqual([]); + expect(delta.rootPatch).toContain("task.txt"); + expect(delta.rootPatch).toContain("+task output"); + expect(delta.rootPatch).not.toContain("baseline dirty change"); + expect(delta.rootPatch).not.toContain("preexisting.txt"); + }); }); diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index 366989f91..adb9c6088 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -847,11 +847,13 @@ export declare function isoResolve(preferred?: IsoBackendKind | undefined | null /** Outcome of [`iso_resolve`]. */ export interface IsoResolveResult { - /** Backend that will actually be used. */ + /** Backend that will actually be tried first. */ kind: IsoBackendKind + /** Host-available backends in retry order, starting with `kind`. */ + candidates: Array /** * True when the resolver fell back from `preferred` (or from the - * platform native) to a different backend. + * first automatic candidate) to a different backend. */ fellBack: boolean /** Human-readable reason for the fallback, if any. */