diff --git a/src-tauri/src/acp/delegation/companion.rs b/src-tauri/src/acp/delegation/companion.rs index 1be8ae5e00..60fc841626 100644 --- a/src-tauri/src/acp/delegation/companion.rs +++ b/src-tauri/src/acp/delegation/companion.rs @@ -5803,9 +5803,10 @@ mod tests { use tokio::net::UnixListener; // `/tmp`, not `$TMPDIR`: a socket path has ~104 bytes of `sun_path` to - // live in, and codeg exports a 72-byte per-session `TMPDIR` to the - // agents it launches. Under `tempdir()` this lands at 92 bytes there — - // green, but with very little left for a deeper nesting. + // live in, and `$TMPDIR` is whatever the environment hands us. Under + // the ~72-byte per-session `TMPDIR` codeg exported before #754 this + // landed at 92 bytes — green, but with very little left for a deeper + // nesting — and a hand-set one can be longer still. let dir = tempfile::tempdir_in("/tmp").unwrap(); let sock = dir.path().join("fb.sock").to_string_lossy().to_string(); let listener = UnixListener::bind(&sock).unwrap(); @@ -5876,9 +5877,10 @@ mod tests { use tokio::net::UnixListener; // `/tmp`, not `$TMPDIR`: a socket path has ~104 bytes of `sun_path` to - // live in, and codeg exports a 72-byte per-session `TMPDIR` to the - // agents it launches. Under `tempdir()` this lands at 92 bytes there — - // green, but with very little left for a deeper nesting. + // live in, and `$TMPDIR` is whatever the environment hands us. Under + // the ~72-byte per-session `TMPDIR` codeg exported before #754 this + // landed at 92 bytes — green, but with very little left for a deeper + // nesting — and a hand-set one can be longer still. let dir = tempfile::tempdir_in("/tmp").unwrap(); let sock = dir.path().join("fb.sock").to_string_lossy().to_string(); let listener = UnixListener::bind(&sock).unwrap(); diff --git a/src-tauri/src/acp/delegation/listener.rs b/src-tauri/src/acp/delegation/listener.rs index 71fcd1ecd3..35c8d0e572 100644 --- a/src-tauri/src/acp/delegation/listener.rs +++ b/src-tauri/src/acp/delegation/listener.rs @@ -44,7 +44,7 @@ use crate::acp::delegation::types::{ use crate::acp::feedback::{PendingFeedback, SessionFeedbackAccess}; use crate::acp::question::{QuestionOutcome, SessionQuestionAccess}; #[cfg(unix)] -use crate::acp::scratch_dir::SUN_PATH_CAP; +use crate::acp::scratch_dir::{is_inside_sweep_scope, SUN_PATH_CAP}; use crate::acp::chat_authoring::{AuthoringContext, AuthoringOutcome, ChatAuthoringAccess}; use crate::acp::session_info::{SessionInfo, SessionInfoAccess}; use crate::acp::work_task_tools::{TaskReportAck, WorkTaskToolAccess}; @@ -1594,12 +1594,24 @@ fn short_socket_dir() -> PathBuf { /// codeg instances on the same machine don't collide. /// /// Unix: a `.sock` file inside `temp_dir`, or — when that would not fit in -/// `sun_path` — inside [`short_socket_dir`]. The fallback is not hypothetical -/// housekeeping: codeg exports a ~72-byte per-session `TMPDIR` to the agents it -/// launches, so a codeg started from inside one is already within a few bytes -/// of the macOS cap, and a container or a hand-set `TMPDIR` clears it outright. -/// Without the fallback that produced a socket nobody could dial and no error -/// anywhere — see [`DelegationListener::bind`]. +/// `sun_path`, or would put the socket inside a scratch root a startup sweep +/// deletes wholesale ([`is_inside_sweep_scope`]) — inside [`short_socket_dir`]. +/// Neither fallback is hypothetical housekeeping: +/// +/// * The length budget: a container's or a hand-set `TMPDIR` can clear the +/// `sun_path` cap outright, and a codeg session's own `TMPDIR` used to come +/// close — ~72 bytes under `/var/folders/…/T/` until #754 moved it to the +/// short scratch root, so a codeg started inside one sat a few bytes under +/// the macOS cap. Without the fallback that produced a socket nobody could +/// dial and no error anywhere — see [`DelegationListener::bind`]. +/// * The sweep scope: an app launched from inside a codeg-managed session +/// inherits that session's scratch `TMPDIR`, whose directory is named like a +/// `-<8 hex>` launch dir and is deleted wholesale — by the codeg that +/// owns it when that launch ends, or by any codeg's startup sweep once the +/// pid in the name is dead. The socket FILE never matches that pattern — +/// the danger is its PARENT being deleted with it, after which the listener +/// keeps serving an unlinked vnode and every `delegate_to_agent` fails +/// `ENOENT` (2026-10-15, twice in a row on this machine). /// /// Windows: a named pipe address `\\.\pipe\codeg-delegation-`. Windows /// named pipes live in their own kernel namespace and ignore `temp_dir`; the @@ -1608,22 +1620,43 @@ fn short_socket_dir() -> PathBuf { pub fn default_socket_path(temp_dir: &Path) -> PathBuf { let name = format!("codeg-delegation-{}.sock", std::process::id()); let preferred = temp_dir.join(&name); - if fits_sun_path(&preferred) { + // The guard gets the full `preferred` path rather than `temp_dir`: that is + // strictly stronger — it also refuses the corner where `temp_dir` IS a + // sweep root, whose socket inside would be just as deletable. + let in_sweep_scope = is_inside_sweep_scope(&preferred); + if fits_sun_path(&preferred) && !in_sweep_scope { return preferred; } // No third candidate, because there is no third case to handle: this is // `/tmp/codeg-` + at most 10 digits of euid + `/codeg-delegation-` + at // most 10 digits of pid + `.sock` — 54 bytes at its absolute widest, or - // half the smallest `sun_path` any of these platforms has. + // half the smallest `sun_path` any of these platforms has. It is also + // outside every sweep root by construction, which is the other half of + // what the guard above has to be able to promise about its result. let short = short_socket_dir().join(&name); - tracing::info!( - "[delegation] {} is {} bytes, past the {}-byte AF_UNIX limit; \ - binding {} instead", - preferred.display(), - preferred.as_os_str().len(), - SUN_PATH_CAP - 1, - short.display(), - ); + if !fits_sun_path(&preferred) { + tracing::info!( + "[delegation] {} is {} bytes, past the {}-byte AF_UNIX limit; \ + binding {} instead", + preferred.display(), + preferred.as_os_str().len(), + SUN_PATH_CAP - 1, + short.display(), + ); + } + if in_sweep_scope { + tracing::warn!( + "[delegation] TMPDIR {} sits inside codeg's per-launch scratch \ + namespace (`-<8 hex>` directories codeg deletes wholesale when \ + their launch ends or their owner dies), so the broker socket is bound \ + at {} instead — bound there it would silently disappear with that \ + directory. This usually happens when the app is launched from inside \ + a codeg-managed session environment and inherits its scratch TMPDIR; \ + if that was not intentional, restart it from a clean environment.", + temp_dir.display(), + short.display(), + ); + } short } @@ -1635,6 +1668,8 @@ pub fn default_socket_path(_temp_dir: &Path) -> PathBuf { #[cfg(test)] mod tests { use super::*; + #[cfg(unix)] + use crate::acp::scratch_dir::short_root; use crate::acp::delegation::broker::{ConversationDepthLookup, DelegationConfig}; use crate::acp::delegation::spawner::{ mock::MockSpawner, ConnectionSpawner, ResumedSpawn, SpawnerError, @@ -3664,17 +3699,22 @@ mod tests { #[test] fn an_over_long_temp_dir_falls_back_to_the_short_socket_dir() { let name = format!("codeg-delegation-{}.sock", std::process::id()); - // A macOS `/var/folders/…/T` with codeg's own per-session nesting under - // it — the shape that actually produces this — padded so the composed - // path lands exactly ONE byte past what `sun_path` can hold. Sized from - // the cap rather than hard-coded, so it keeps straddling the boundary - // if either side of it moves. + // A macOS `/var/folders/…/T` with a per-session directory nested under + // it, padded so the composed path lands exactly ONE byte past what + // `sun_path` can hold. Sized from the cap rather than hard-coded, so it + // keeps straddling the boundary if either side of it moves. NOT codeg's + // own `codeg-acp/-` nesting: that is refused for its sweep + // scope as well, and this has to pin the length budget alone. let prefix = "/var/folders/hl/"; - let suffix = "/T/codeg-acp/12345-deadbeef"; + let suffix = "/T/agent-tmp/12345-deadbeef"; let pad = SUN_PATH_CAP - 1 - name.len() - prefix.len() - suffix.len(); let ambient = PathBuf::from(format!("{prefix}{}{suffix}", "z".repeat(pad))); assert_eq!(ambient.as_os_str().len() + 1 + name.len(), SUN_PATH_CAP); assert!(!fits_sun_path(&ambient.join(&name))); + assert!( + !is_inside_sweep_scope(&ambient.join(&name)), + "the sweep guard must NOT be what refuses this" + ); let path = default_socket_path(&ambient); assert_eq!(path.parent(), Some(short_socket_dir().as_path())); @@ -3687,6 +3727,111 @@ mod tests { ); } + /// The second refusal reason, and the one that actually bit (2026-10-15): + /// a `temp_dir` inside codeg's own scratch namespace. Such a path is + /// perfectly bindable on length, so only the sweep guard can be what + /// refuses it — and it must, because a startup sweep deletes the whole + /// `-<8 hex>` directory it sits in once the owner is dead, leaving + /// the listener serving an unlinked vnode while every dial gets `ENOENT`. + #[cfg(unix)] + #[test] + fn a_temp_dir_inside_the_sweep_scope_falls_back_to_the_short_socket_dir() { + let ambient = short_root().join(format!("{}-deadbeef", std::process::id())); + assert!(is_inside_sweep_scope(&ambient)); + let composed = ambient.join(format!("codeg-delegation-{}.sock", std::process::id())); + assert!( + fits_sun_path(&composed), + "the length budget must NOT be what refuses this: {}", + composed.display() + ); + + let path = default_socket_path(&ambient); + assert_eq!(path.parent(), Some(short_socket_dir().as_path())); + assert!( + !is_inside_sweep_scope(&path), + "the fallback itself must stay out of scope: {}", + path.display() + ); + } + + /// ...and so must a launch dir under a root that is NOT among this + /// process's own sweep roots. Its owner deletes it when that launch ends, + /// and any codeg whose root it is sweeps it at startup — neither needs to + /// be this process. Linux's default `/tmp/codeg-acp` is the common shape: + /// a `TMPDIR` under it pushes this process to the short root instead. + #[cfg(unix)] + #[test] + fn a_launch_dir_under_another_codegs_root_falls_back_too() { + let resolved_short_root = Path::new("/private").join( + short_root() + .strip_prefix("/") + .expect("the short root is absolute"), + ); + for ambient in [ + PathBuf::from("/tmp/codeg-acp/4242-deadbeef"), + PathBuf::from("/srv/elsewhere/codeg-acp/4242-deadbeef"), + resolved_short_root.join("4242-deadbeef"), + ] { + let path = default_socket_path(&ambient); + assert_eq!( + path.parent(), + Some(short_socket_dir().as_path()), + "{} must not host the socket", + ambient.display() + ); + } + } + + /// ...and a launch dir reached through an alias that hides every root + /// component — here a symlink; a relative `TMPDIR=.` taken from inside a + /// launch dir is the same case. The directory behind it is deleted all the + /// same. + #[cfg(unix)] + #[test] + fn an_alias_into_a_launch_dir_falls_back_too() { + let holder = tempfile::tempdir_in("/tmp").expect("tempdir"); + let leaf = holder.path().join("codeg-acp").join("4242-deadbeef"); + std::fs::create_dir_all(&leaf).expect("create leaf"); + let alias = holder.path().join("session-tmp"); + std::os::unix::fs::symlink(&leaf, &alias).expect("symlink"); + + let path = default_socket_path(&alias); + assert_eq!(path.parent(), Some(short_socket_dir().as_path())); + } + + /// Property rather than example: for ANY temp directory, the socket this + /// picks must sit outside every sweep root — a parent inside one can be + /// deleted wholesale by a startup sweep while the listener is live, which + /// is the failure the guard exists to prevent. Every operand is read + /// through the same helpers production uses, so it holds whether or not + /// these tests themselves run inside a codeg-managed session. + #[cfg(unix)] + #[test] + fn no_chosen_socket_path_is_ever_inside_the_sweep_scope() { + for temp_dir in [ + // The ordinary short case, an over-long macOS one, a scratch + // directory of our own, the ambient scratch root, one under + // another codeg's root, and TMPDIR. + PathBuf::from("/tmp"), + PathBuf::from(format!("/var/folders/hl/{}/T", "z".repeat(30))), + short_root().join(format!("{}-deadbeef", std::process::id())), + std::env::temp_dir().join("codeg-acp").join("4242-deadbeef"), + PathBuf::from("/tmp/codeg-acp/4242-deadbeef"), + std::env::temp_dir(), + ] { + let path = default_socket_path(&temp_dir); + let parent = path + .parent() + .expect("a socket path always names a directory"); + assert!( + !is_inside_sweep_scope(parent), + "{} would put the socket inside a sweep root: {}", + temp_dir.display(), + path.display() + ); + } + } + /// The STAGED path is measured too, not assumed shorter than the real one. /// /// `.stg--<8 hex>` is shorter than `codeg-delegation-.sock`, which diff --git a/src-tauri/src/acp/delegation/service.rs b/src-tauri/src/acp/delegation/service.rs index 9c70df9e05..f2499a3a58 100644 --- a/src-tauri/src/acp/delegation/service.rs +++ b/src-tauri/src/acp/delegation/service.rs @@ -439,11 +439,12 @@ mod tests { /// /// `tempfile::tempdir()` honours `$TMPDIR`, and these tests then add /// `/.tmpXXXXXX/codeg-delegation-*.sock` on top — about 38 bytes. That is - /// fine against the ~20-byte default and fatal against codeg's own - /// per-session `TMPDIR`, which is 72 bytes (`/var/folders/…/T/codeg-acp/ - /// -`): the composed path reaches 110 and `sun_path` caps at 104 - /// on macOS. The whole suite went red inside a codeg session and green - /// outside it, for reasons having nothing to do with the code under test. + /// fine against a stock temp directory and was fatal against the 72-byte + /// per-session `TMPDIR` codeg exported before #754 (`/var/folders/…/T/ + /// codeg-acp/-`): the composed path reached 110 and `sun_path` + /// caps at 104 on macOS. The whole suite went red inside a codeg session + /// and green outside it, for reasons having nothing to do with the code + /// under test — and a hand-set `TMPDIR` can still do the same. /// /// Rooting in `/tmp` — the same move `scratch_dir`'s socket tests make — /// keeps the result near 40 bytes no matter what the environment says. diff --git a/src-tauri/src/acp/delegation/transport.rs b/src-tauri/src/acp/delegation/transport.rs index e38b41167f..6695584939 100644 --- a/src-tauri/src/acp/delegation/transport.rs +++ b/src-tauri/src/acp/delegation/transport.rs @@ -1025,9 +1025,10 @@ mod tests { use tokio::net::UnixListener; // `/tmp`, not `$TMPDIR`: a socket path has ~104 bytes of `sun_path` to - // live in, and codeg exports a 72-byte per-session `TMPDIR` to the - // agents it launches. Under `tempdir()` this lands at 99 bytes there — - // green, but one directory level from an unexplainable red. + // live in, and `$TMPDIR` is whatever the environment hands us. Under + // the ~72-byte per-session `TMPDIR` codeg exported before #754 this + // landed at 99 bytes — green, but one directory level from an + // unexplainable red — and a hand-set one can be longer still. let dir = tempfile::tempdir_in("/tmp").unwrap(); let path = dir.path().join("codeg-mcp.sock"); let listener = UnixListener::bind(&path).unwrap(); diff --git a/src-tauri/src/acp/scratch_dir.rs b/src-tauri/src/acp/scratch_dir.rs index 919197c53f..9b3c9a8164 100644 --- a/src-tauri/src/acp/scratch_dir.rs +++ b/src-tauri/src/acp/scratch_dir.rs @@ -169,8 +169,11 @@ fn leaves_room_for_a_child_socket(root: &Path) -> bool { /// account to run codeg would own `codeg-acp` and every other account would be /// unable to create inside it. [`create`] makes it `0700` so the contents stay /// as private as they were in the per-user directory this replaces. +/// +/// `pub(crate)` so [`crate::acp::delegation::listener`]'s socket tests can +/// construct a temp directory inside the sweep scope this root belongs to. #[cfg(unix)] -fn short_root() -> PathBuf { +pub(crate) fn short_root() -> PathBuf { PathBuf::from(format!( "/tmp/{SCRATCH_NAMESPACE}-{}", // Always succeeds; `geteuid` has no failure mode. @@ -277,6 +280,68 @@ fn sweep_roots() -> Vec { roots } +/// Whether `path` sits inside (or equals) a scratch root — i.e. it or an +/// ancestor may be deleted wholesale as a `-<8hex>` launch dir. +/// Consumer: the delegation listener's `default_socket_path`, which must never +/// place the long-lived broker socket where that can happen. +/// +/// Three tests, any one enough: +/// +/// * Under one of THIS process's [`sweep_roots`], as written. +/// * Under any directory NAMED like a scratch root ([`is_scratch_root_name`]), +/// wherever it sits. This process is not the only thing that deletes launch +/// dirs: the codeg that owns one removes it when that launch ends, and every +/// other codeg on the machine sweeps its own roots at startup. A launch dir +/// inherited as `TMPDIR` hangs off THAT process's root, which this process's +/// list need not contain — Linux's default `/tmp/codeg-acp` drops out of it +/// as soon as the inherited `TMPDIR` pushes [`choose_root`] to the short +/// root, and a `/private/tmp` spelling of the macOS short root never matches +/// the literal one. The name test sees both. +/// * The same name test on the path RESOLVED through its nearest existing +/// ancestor, because an alias can hide every root component from the two +/// tests above: a symlink into a launch dir, or a relative `TMPDIR=.` taken +/// from inside one. +/// +/// Resolution only ever adds a way in; it never replaces the literal tests. +/// The roots are compared as written because that is how the sweeps enumerate +/// them, and on macOS a resolved `/tmp` becomes `/private/tmp`, which would +/// stop matching — the same convention the listener's own path guards follow. +/// +/// Unix-only because its consumer is: the Windows `default_socket_path` is a +/// named pipe and ignores the temp directory, so an unused copy here would be +/// dead code on that target. +#[cfg(unix)] +pub(crate) fn is_inside_sweep_scope(path: &Path) -> bool { + let named = |path: &Path| { + path.ancestors() + .any(|dir| dir.file_name().is_some_and(is_scratch_root_name)) + }; + sweep_roots().iter().any(|root| path.starts_with(root)) + || named(path) + || path + .ancestors() + .find_map(|dir| std::fs::canonicalize(dir).ok()) + .is_some_and(|resolved| named(&resolved)) +} + +/// The last component of every root [`scratch_root`] can pick, under whatever +/// parent: `codeg-acp` (nested under the ambient temp directory or under +/// [`ROOT_ENV`]), or [`short_root`]'s `codeg-acp-` — any euid, since +/// which account's root it is changes nothing about whether it gets swept. +#[cfg(unix)] +fn is_scratch_root_name(name: &std::ffi::OsStr) -> bool { + let Some(name) = name.to_str() else { + return false; + }; + match name.strip_prefix(SCRATCH_NAMESPACE) { + Some("") => true, + Some(rest) => rest + .strip_prefix('-') + .is_some_and(|euid| !euid.is_empty() && euid.bytes().all(|b| b.is_ascii_digit())), + None => false, + } +} + /// A scratch directory owned by one agent launch. /// /// Cleanup lives in `Drop`, and that is load-bearing rather than idiomatic @@ -574,7 +639,12 @@ pub(crate) fn apply_to_env(env: &mut std::collections::BTreeMap, /// Exact set arithmetic, no heuristic: a live launch is in the registry by /// construction (invariant 1), so it can never appear in the diff. pub fn sweep_own_orphans() { - let roots = sweep_roots(); + sweep_own_orphans_in(&sweep_roots(), &std::env::temp_dir()); +} + +/// [`sweep_own_orphans`] over explicit roots, so tests can point it somewhere +/// other than the machine's real scratch roots. +fn sweep_own_orphans_in(roots: &[PathBuf], temp_dir: &Path) { let our_pid = std::process::id(); // INVARIANT 3: enumerate + diff under the lock... @@ -596,6 +666,9 @@ pub fn sweep_own_orphans() { }; // ...delete outside it. for path in claimed { + if holds_temp_dir(&path, temp_dir) { + continue; + } if remove_dir_robust(&path).is_ok() { tracing::info!("[ACP][scratch] reclaimed orphan {}", path.display()); } @@ -609,8 +682,14 @@ pub fn sweep_own_orphans() { /// see [`probe_pid`] for why "the probe failed" must not be read as "the owner /// is gone". pub fn sweep_foreign_orphans() { + sweep_foreign_orphans_in(&sweep_roots(), &std::env::temp_dir()); +} + +/// [`sweep_foreign_orphans`] over explicit roots, so tests can point it +/// somewhere other than the machine's real scratch roots. +fn sweep_foreign_orphans_in(roots: &[PathBuf], temp_dir: &Path) { let our_pid = std::process::id(); - for entry in sweep_roots() + for entry in roots .iter() .filter_map(|root| std::fs::read_dir(root).ok()) .flatten() @@ -629,6 +708,15 @@ pub fn sweep_foreign_orphans() { continue; } let path = entry.path(); + if holds_temp_dir(&path, temp_dir) { + tracing::info!( + "[ACP][scratch] leaving orphan {} from dead pid {pid} in place: \ + this process's own temp dir {} is inside it", + path.display(), + temp_dir.display() + ); + continue; + } if remove_dir_robust(&path).is_ok() { tracing::info!( "[ACP][scratch] reclaimed orphan {} from dead pid {pid}", @@ -638,6 +726,33 @@ pub fn sweep_foreign_orphans() { } } +/// Whether reclaiming `dir` would delete `temp_dir`, this process's own temp +/// directory, along with it. +/// +/// A codeg launched from inside an agent session inherits that session's +/// scratch `TMPDIR` — another codeg's `-<8 hex>` launch dir — and adopts +/// it as its own `std::env::temp_dir()`. Once that owner is dead it is an +/// orphan by every rule above, and on macOS it sits under the short root this +/// process sweeps as well, so the startup sweep would delete the temp directory +/// the process has just started using: the parent of its broker socket, the +/// directory a pi catalog query creates, the `TMPDIR` its terminals inherit. +/// It is adopted, not orphaned. A sweep from a process that does not live in it +/// reclaims it later. +/// +/// Literal first, then resolved: the sweeps enumerate roots as written, while +/// `TMPDIR` may spell the same directory another way (`/private/tmp` for +/// `/tmp`). Only ever consulted to SKIP a delete, so a resolution that fails or +/// surprises can cost a reclaim but never widen one. +fn holds_temp_dir(dir: &Path, temp_dir: &Path) -> bool { + if temp_dir.starts_with(dir) { + return true; + } + match (std::fs::canonicalize(dir), std::fs::canonicalize(temp_dir)) { + (Ok(dir), Ok(temp_dir)) => temp_dir.starts_with(dir), + _ => false, + } +} + /// Long-running task: [`sweep_own_orphans`] on a fixed interval. Spawned by /// both runtimes at startup; never returns. pub async fn scratch_sweep_task() { @@ -960,6 +1075,184 @@ mod tests { "and the ambient one, which a previous version may have filled" ); assert!(roots.contains(&short_root()), "and the short fallback"); + // Every root is inside the sweep scope by definition — the scope IS + // this list — which is what lets the socket guard refuse them all. + for root in &roots { + assert!( + is_inside_sweep_scope(root), + "{} is a sweep root, so it must be in scope", + root.display() + ); + } + } + + /// The guard the broker socket leans on: exactly the paths a startup sweep + /// may delete wholesale, and nothing else. Both sides are built from the + /// same helpers the sweeps use, so this encodes the scope rather than this + /// machine's current temp layout. + #[cfg(unix)] + #[test] + fn the_sweep_scope_covers_scratch_dirs_and_nothing_else() { + // Equal to a root, and a `-<8 hex>` leaf under one. + assert!(is_inside_sweep_scope(&short_root())); + assert!(is_inside_sweep_scope( + &short_root().join(format!("{}-deadbeef", std::process::id())) + )); + // The ambient root, wherever this machine's temp_dir points. + assert!(is_inside_sweep_scope( + &std::env::temp_dir().join(SCRATCH_NAMESPACE).join("4242-deadbeef") + )); + // A plain per-user temp path is NOT in scope: a socket there must keep + // the ordinary placement. Constructed, not read, so it holds on every + // machine. + assert!(!is_inside_sweep_scope(Path::new( + "/var/folders/hl/26lfwjvs4s760g9fm1nk15xm0000gn/T/codeg-delegation-4242.sock" + ))); + } + + /// The scope is every codeg's roots, not just this process's: the launch + /// dir a codeg inherits as `TMPDIR` is deleted by its OWNER when that launch + /// ends, and swept at startup by any codeg whose root it sits under — and + /// the root it hangs off need not be among this process's own. + #[cfg(unix)] + #[test] + fn the_sweep_scope_covers_any_codeg_root_by_name() { + for path in [ + // Linux's default root. A codeg launched with a `TMPDIR` under it + // derives the short root instead, so it is not in that list. + "/tmp/codeg-acp/4242-deadbeef/codeg-delegation-1.sock", + // Another codeg's `CODEG_ACP_TMP_ROOT`, which this process lacks. + "/srv/elsewhere/codeg-acp/4242-deadbeef", + // The macOS short root as a resolved `/tmp` spells it. + "/private/tmp/codeg-acp-501/4242-deadbeef/codeg-delegation-1.sock", + ] { + assert!(is_inside_sweep_scope(Path::new(path)), "{path} is in scope"); + } + for path in [ + // The broker socket's own fallback directory. + "/tmp/codeg-501/codeg-delegation-1.sock", + // Names that merely resemble a root. + "/tmp/codeg-acpx/4242-deadbeef/codeg-delegation-1.sock", + "/tmp/codeg-acp-/4242-deadbeef/codeg-delegation-1.sock", + "/tmp/codeg-acp-50a/4242-deadbeef/codeg-delegation-1.sock", + ] { + assert!(!is_inside_sweep_scope(Path::new(path)), "{path} is not"); + } + } + + /// A pid that is positively dead with no reuse window: past the largest + /// `pid_max` Linux allows (2^22) and macOS's 99999, so no process can ever + /// hold it. A spawned-and-reaped child would not do — its number can be + /// handed to another process before the sweep gets to probe it. + #[cfg(unix)] + fn a_dead_pid() -> u32 { + let pid = 999_999_999; + assert_eq!(probe_pid(pid), PidState::Dead); + pid + } + + /// A codeg launched from inside an agent session lives in that session's + /// launch dir. Its owner being dead makes that dir an orphan by name, but + /// reclaiming it would delete this process's own temp directory — the + /// startup sweep must leave it, and only it. + /// + /// The root is enumerated through a symlink while the temp dir is handed + /// over by its real path, so the two spellings never match as written — + /// the way a `/private/tmp` `TMPDIR` meets the literal `/tmp` short root on + /// macOS, but on every platform. Only the resolved comparison can keep it. + #[cfg(unix)] + #[test] + fn a_startup_sweep_leaves_the_launch_dir_this_process_lives_in() { + let holder = tempfile::tempdir().expect("tempdir"); + let root = holder.path().join("root"); + std::fs::create_dir(&root).expect("create root"); + let alias = holder.path().join("alias"); + std::os::unix::fs::symlink(&root, &alias).expect("symlink"); + let dead = a_dead_pid(); + let adopted = root.join(format!("{dead}-0000feed")); + let orphan = root.join(format!("{dead}-0000dead")); + for dir in [&adopted, &orphan] { + std::fs::create_dir(dir).expect("create"); + } + + sweep_foreign_orphans_in(&[alias], &adopted); + + assert!(adopted.exists(), "the dir this process lives in must survive"); + assert!( + !orphan.exists(), + "and the guard must not cost a real orphan its reclaim" + ); + } + + /// The scope checked against what a sweep ACTUALLY deletes rather than + /// against its own reasoning: everything a sweep takes down must have been + /// in scope beforehand, including through an alias that hides the root's + /// name. The bystander is the control — it survives and is out of scope, so + /// the scope is not simply answering yes. + /// + /// Rooted in `/tmp` rather than `TMPDIR`: inside a codeg-managed session + /// `TMPDIR` is itself a launch dir, which would put the bystander in scope. + #[cfg(unix)] + #[test] + fn everything_a_sweep_deletes_was_in_scope() { + let holder = tempfile::tempdir_in("/tmp").expect("tempdir"); + let dead = a_dead_pid(); + let mut roots = Vec::new(); + let mut probes = Vec::new(); + for root_name in ["codeg-acp", "codeg-acp-4242"] { + let root = holder.path().join(root_name); + let leaf = root.join(format!("{dead}-deadbeef")); + std::fs::create_dir_all(&leaf).expect("create leaf"); + let socket = leaf.join("codeg-delegation-1.sock"); + std::fs::write(&socket, b"").expect("write"); + let alias = holder.path().join(format!("alias-{root_name}")); + std::os::unix::fs::symlink(&leaf, &alias).expect("symlink"); + let bystander = holder.path().join(format!("{root_name}-bystander.sock")); + std::fs::write(&bystander, b"").expect("write"); + probes.extend([socket, alias.join("codeg-delegation-1.sock"), bystander]); + roots.push(root); + } + // Asked BEFORE the sweep, the way the broker asks at startup. + let in_scope: Vec = probes.iter().map(|p| is_inside_sweep_scope(p)).collect(); + + sweep_foreign_orphans_in(&roots, Path::new("/nonexistent-temp-dir")); + + for (probe, in_scope) in probes.iter().zip(in_scope) { + let deleted = !probe.exists(); + let is_bystander = probe.to_string_lossy().ends_with("-bystander.sock"); + assert_eq!(deleted, !is_bystander, "{} deleted: {deleted}", probe.display()); + assert_eq!( + in_scope, + deleted, + "{} was {}deleted but scoped {in_scope}", + probe.display(), + if deleted { "" } else { "not " } + ); + } + } + + /// The in-session sweep gets the same guard. Only a recycled pid can put + /// this process inside a dir named for its own pid, but that sweep runs + /// every few minutes for the life of the process. + #[test] + fn the_in_session_sweep_leaves_the_launch_dir_this_process_lives_in() { + let root = tempfile::tempdir().expect("tempdir"); + // Never registered, so both are orphans to `sweep_own_orphans`. + let adopted = root.path().join(new_dir_name(std::process::id())); + let orphan = root.path().join(new_dir_name(std::process::id())); + for dir in [&adopted, &orphan] { + std::fs::create_dir(dir).expect("create"); + } + let temp_dir = adopted.join("nested"); + std::fs::create_dir(&temp_dir).expect("create"); + + sweep_own_orphans_in(&[root.path().to_path_buf()], &temp_dir); + + assert!(adopted.exists(), "the dir this process lives in must survive"); + assert!( + !orphan.exists(), + "and the guard must not cost a real orphan its reclaim" + ); } #[test] @@ -988,6 +1281,10 @@ mod tests { "123", // no suffix at all "", // empty "-deadbeef", // no pid + // The broker socket's own names never match; the danger is the + // PARENT directory they sit in going with a sweep. + "codeg-delegation-16527.sock", + ".stg-16527-f41c5659", ] { assert_eq!(pid_from_dir_name(name), None, "{name} must not parse"); } diff --git a/src-tauri/tests/delegation_e2e_uds.rs b/src-tauri/tests/delegation_e2e_uds.rs index 18e4217c23..6057b67fdc 100644 --- a/src-tauri/tests/delegation_e2e_uds.rs +++ b/src-tauri/tests/delegation_e2e_uds.rs @@ -39,12 +39,12 @@ use tokio::sync::oneshot; /// `$TMPDIR` happens to be. /// /// `tempfile::tempdir()` honours `$TMPDIR`, and these tests then add -/// `/.tmpXXXXXX/codeg-e2e-*.sock` on top. That is fine against the ~20-byte -/// default and fatal against codeg's own per-session `TMPDIR`, which is 72 -/// bytes: `codeg-e2e-batch.sock` composed to exactly 104 there, and `sun_path` -/// caps at 104 on macOS. Three of these tests went red inside a codeg session -/// and green outside it, for reasons having nothing to do with the code under -/// test. +/// `/.tmpXXXXXX/codeg-e2e-*.sock` on top. That is fine against a stock temp +/// directory and was fatal against the 72-byte per-session `TMPDIR` codeg +/// exported before #754: `codeg-e2e-batch.sock` composed to exactly 104 there, +/// and `sun_path` caps at 104 on macOS. Three of these tests went red inside a +/// codeg session and green outside it, for reasons having nothing to do with +/// the code under test — and a hand-set `TMPDIR` can still do the same. /// /// Rooting in `/tmp` keeps the result near 40 bytes whatever the environment /// says. Still unique per test: `tempdir_in` mints a fresh name.