From 209ed00fddac186935c7033807bbe28eea776f4f Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 2 Nov 2026 15:53:26 -0800 Subject: [PATCH 1/2] Stop the migration manifest overflowing on a large /workspace get_container_staleness failed on every start/stop of a real container with "OUTPUT_LIMIT: Command output exceeded 8388608 bytes": the manifest's flat `find /workspace` walk alone was 8.6 MB (94k entries of worktrees, node_modules, vendored trees in the writable layer). Pruning node_modules and .git by name was measured on that container and only reached 6.3 MB. - Walk /workspace two levels deep; each non-empty directory at depth 2 is one `S` entry, sized from a new ###SUMMARY section (`du -sbx`). The entry comes from find, so a failed du costs the size, never the dir. Measured on the same container: 8.6 MB -> ~25 kB. - compute_verbatim_paths: summaries count as content (not an empty tree) and are exempt from presence-in-base, since a summary hides which paths are inside; restore's --skip-old-files keeps the per-file guarantee. The verbatim set is unchanged: it already pruned to /workspace/. - manifest_from_container gets its own 32 MiB budget via a new exec_oneshot_as_limited, and an OUTPUT_LIMIT is reported as an actionable sentence (describe_manifest_failure) instead of the buffer message. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 9 + app/src-tauri/src/docker/exec.rs | 13 + app/src-tauri/src/docker/migration.rs | 367 +++++++++++++++++++++++++- 3 files changed, 383 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 56f6cba..9c9e17c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -579,6 +579,15 @@ security update. Migration is the non-destructive way out; Reset is the destruct the last commit. Migration diffs two filesystem manifests instead, filtered through dpkg ownership and presence-in-the-new-base. Measured on a real project, that turns 8,677 raw path differences into 2 genuinely user-authored ones. +- **`/workspace` is walked two levels deep, never file by file** (`SUMMARISED_ROOTS`). Below that, + each non-empty directory is one `S` entry sized by `du -sbx`. A user's worktrees and + `node_modules` made a flat walk 8.6 MB — past the exec cap, so every staleness check failed on + start and stop — and pruning `node_modules`/`.git` by name only reached 6.3 MB. Summaries lose + nothing: below `/workspace/` every per-path filter is vacuous, and `tar` copies a directory + recursively. Two rules: the `S` line comes from `find`, never from `du`, so a failed size costs + the estimate and not the directory; and a summary is exempt from presence-in-the-base (it hides + which paths are inside — `--skip-old-files` on restore is the per-file answer). Depth 2, not 1, + because `du -x` on a first-level bind-mount point would walk the host repository. ### Authentication diff --git a/app/src-tauri/src/docker/exec.rs b/app/src-tauri/src/docker/exec.rs index a285b4b..b64a86c 100644 --- a/app/src-tauri/src/docker/exec.rs +++ b/app/src-tauri/src/docker/exec.rs @@ -642,6 +642,19 @@ pub async fn exec_oneshot_as( exec_oneshot_inner(container_id, user, cmd, env, MAX_ONESHOT_OUTPUT).await } +/// [`exec_oneshot_as`] with a caller-chosen output ceiling, for a reader that +/// knows its legitimate output can pass [`MAX_ONESHOT_OUTPUT`] — the migration +/// manifest of a container with a large `/opt` is the one today. +pub async fn exec_oneshot_as_limited( + container_id: &str, + user: &str, + cmd: Vec, + env: Vec, + limit: usize, +) -> Result<(String, i64), String> { + exec_oneshot_inner(container_id, user, cmd, env, limit).await +} + /// What a one-shot exec printed, with the two streams still tellable apart. /// /// `combined` is stdout and stderr interleaved in arrival order — the shape diff --git a/app/src-tauri/src/docker/migration.rs b/app/src-tauri/src/docker/migration.rs index 67c242d..23299b9 100644 --- a/app/src-tauri/src/docker/migration.rs +++ b/app/src-tauri/src/docker/migration.rs @@ -94,6 +94,51 @@ pub const MANIFEST_ROOTS: &[&str] = &[ "/var/www", ]; +/// [`MANIFEST_ROOTS`] entries walked only to [`SUMMARY_DEPTH`], with every +/// non-empty directory at that depth reported as one [`SUMMARY_KIND`] entry +/// carrying the subtree's total size. +/// +/// **Why `/workspace`, and why it has to be.** It is the one root whose size is +/// the user's rather than the image's: git worktrees, `node_modules`, a pnpm +/// store and vendored trees all sit in the writable layer beside the bind +/// mounts. A real container listed 94k entries — 8.6 MB of `find` output — +/// under `/workspace` alone, which overflowed +/// [`crate::docker::exec::MAX_ONESHOT_OUTPUT`] and failed the staleness check +/// on every start and stop. Pruning by name (`node_modules`, `.git`) was +/// measured on that container and only got it to 6.3 MB: most of it was a +/// vendored phpMyAdmin in each of eight worktrees. Depth is what bounds it. +/// +/// **Why summarising loses nothing.** Every per-path filter in +/// [`compute_verbatim_paths`] is vacuous below `/workspace/`: dpkg owns +/// nothing there, no [`COPY_EXCLUSIONS`] entry is there, bind mounts are +/// exactly `/workspace/` (a mount name cannot hold a `/`), and the +/// base ships an empty `/workspace`. So the verbatim set was already pruned to +/// first-level entries; a summary just stops listing what was going to be +/// pruned away. Migration's `tar` still archives the directory recursively. +/// +/// **Why depth 2 and not 1.** The first level holds the bind-mount points. +/// `find -xdev` lists a mount point without descending, but measuring one with +/// `du -x` would walk the whole host repository — so summaries start a level +/// below, where `-xdev` has already refused to enter any mount. +pub const SUMMARISED_ROOTS: &[&str] = &["/workspace"]; + +/// Depth below a [`SUMMARISED_ROOTS`] entry at which directories are +/// summarised instead of walked. See there for why it is 2. +pub const SUMMARY_DEPTH: usize = 2; + +/// [`ManifestEntry::kind`] of a summarised directory. Not a `find` `%y` letter. +pub const SUMMARY_KIND: char = 'S'; + +/// Output budget for one manifest exec against a running container. +/// +/// Above [`crate::docker::exec::MAX_ONESHOT_OUTPUT`] on purpose. With +/// [`SUMMARISED_ROOTS`] a real container's manifest is a few MB (`/usr/local` +/// ~0.8 MB, `/var/lib` ~0.1 MB, the dpkg section a few hundred kB), but the +/// non-summarised roots are still walked file by file and a user can install +/// anything into `/opt`. Failing at 8 MiB cost the user every staleness check; +/// 32 MiB keeps that rare while still bounding a container-controlled read. +pub const MANIFEST_OUTPUT_LIMIT: usize = 32 * 1024 * 1024; + /// Roots holding **state a base-image swap destroys and no replay can put /// back**. Reported by [`unpreserved_data`], never copied. /// @@ -173,16 +218,27 @@ pub const PROBE_LABEL_MIGRATION: &str = "migration"; /// One entry from the filesystem walk. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ManifestEntry { - /// `find`'s `%y`: `f` regular, `d` directory, `l` symlink, … + /// `find`'s `%y`: `f` regular, `d` directory, `l` symlink, … — or + /// [`SUMMARY_KIND`] for a directory under a [`SUMMARISED_ROOTS`] entry that + /// was not walked file by file. pub kind: char, + /// Bytes on disk; for a summary, the whole subtree's (`du -sb`), or 0 when + /// that could not be measured. pub size: u64, pub path: String, } impl ManifestEntry { + /// A directory reported as itself, with nothing implied about content. + /// A summary is deliberately *not* one: it stands for a non-empty subtree. pub fn is_dir(&self) -> bool { self.kind == 'd' } + + /// A non-empty directory whose contents were aggregated rather than listed. + pub fn is_summary(&self) -> bool { + self.kind == SUMMARY_KIND + } } /// Everything one probe run learned about an image or a running container. @@ -226,9 +282,15 @@ pub fn manifest_script() -> String { .join(" "); let roots = MANIFEST_ROOTS .iter() + .filter(|p| !SUMMARISED_ROOTS.contains(p)) .map(|p| shell_single_quote(p)) .collect::>() .join(" "); + let summarised = SUMMARISED_ROOTS + .iter() + .map(|r| summarised_walk(r)) + .collect::>() + .join("\necho '###PATHS'\n"); // The dpkg grep is anchored to the manifest roots so the section stays a // few hundred kB instead of the ~40 MB a full ownership dump would be. let dpkg_filter = MANIFEST_ROOTS @@ -240,6 +302,7 @@ pub fn manifest_script() -> String { r#" echo '###PATHS' find {roots} -xdev -printf '%y\t%s\t%p\n' 2>/dev/null +{summarised} echo '###DPKG' cat /var/lib/dpkg/info/*.list 2>/dev/null | grep -E '^/({dpkg_filter})(/|$)' echo '###APT' @@ -260,6 +323,26 @@ exit 0 ) } +/// The `###PATHS` lines for one [`SUMMARISED_ROOTS`] entry, followed by a +/// `###SUMMARY` section of `du -sb` lines (`bytespath`) for its summaries. +/// +/// The summary entry itself comes from `find`, not `du`: the directory's +/// presence must never depend on the size measurement succeeding, or a `du` +/// failure would make a whole subtree vanish from the migration. A missing size +/// costs only the disk estimate. Empty directories at the boundary are printed +/// as ordinary `d` entries, so the empty-tree filter still sees them as empty. +fn summarised_walk(root: &str) -> String { + let q = shell_single_quote(root); + let boundary = shell_single_quote(&format!("{}{}", root, "/*".repeat(SUMMARY_DEPTH))); + format!( + "find {q} -xdev -maxdepth {d} \\( -type d -path {boundary} ! -empty -printf '{k}\\t0\\t%p\\n' \\) -o -printf '%y\\t%s\\t%p\\n' 2>/dev/null\n\ + echo '###SUMMARY'\n\ + find {q} -xdev -mindepth {d} -maxdepth {d} -type d ! -empty -exec du -sbx {{}} + 2>/dev/null", + d = SUMMARY_DEPTH, + k = SUMMARY_KIND, + ) +} + /// Parse the output of [`manifest_script`]. /// /// Unknown sections and malformed lines are skipped rather than failing: the @@ -268,11 +351,15 @@ exit 0 pub fn parse_manifest(raw: &str) -> Manifest { let mut m = Manifest::default(); let mut section = ""; + // `du -sb` lines for summary entries, applied once everything has been + // read so the two halves cannot depend on output order. + let mut summary_sizes: HashMap = HashMap::new(); for line in raw.lines() { let line = line.strip_suffix('\r').unwrap_or(line); if let Some(name) = line.strip_prefix("###") { section = match name { - "PATHS" | "DPKG" | "APT" | "NPM" | "FEATURES" | "ETC" | "PKGVER" | "END" => name, + "PATHS" | "SUMMARY" | "DPKG" | "APT" | "NPM" | "FEATURES" | "ETC" | "PKGVER" + | "END" => name, _ => "", }; continue; @@ -286,6 +373,13 @@ pub fn parse_manifest(raw: &str) -> Manifest { m.paths.push(entry); } } + "SUMMARY" => { + if let Some((bytes, path)) = line.split_once('\t') { + if let Ok(bytes) = bytes.trim().parse::() { + summary_sizes.insert(path.to_string(), bytes); + } + } + } "DPKG" => { m.dpkg_owned.insert(line.to_string()); } @@ -313,6 +407,11 @@ pub fn parse_manifest(raw: &str) -> Manifest { _ => {} } } + for entry in m.paths.iter_mut().filter(|e| e.is_summary()) { + if let Some(bytes) = summary_sizes.get(&entry.path) { + entry.size = *bytes; + } + } m } @@ -416,7 +515,8 @@ pub fn prune_to_roots(paths: &BTreeSet) -> Vec { /// * it lives under a [`COPY_ROOTS`] entry, /// * it is not under a [`COPY_EXCLUSIONS`] entry or a bind-mount target, /// * neither image's dpkg database owns it, -/// * the current base image does not already have it. +/// * the current base image does not already have it (a [`SUMMARY_KIND`] +/// entry is exempt from this one — see the comment at the check). /// /// The result is then pruned to subtree roots. An empty result means the copy /// step is skipped entirely. @@ -446,7 +546,12 @@ pub fn compute_verbatim_paths( if from.dpkg_owned.contains(p) || base.dpkg_owned.contains(p) { continue; } - if base_paths.contains(p) { + // Not for a summary: "the base already has this path" is a per-path + // judgment, and a summary hides which paths are inside it. Skipping one + // would drop every user file under any subtree the base happens to ship + // too. Copying it is safe — the restore's `--skip-old-files` keeps the + // base's copy of anything both have, which is the per-file answer. + if base_paths.contains(p) && !entry.is_summary() { continue; } candidates.insert(entry.path.clone()); @@ -1064,6 +1169,26 @@ pub async fn manifest_from_image(image: &str) -> Result { Ok(parse_manifest(&out.stdout)) } +/// Turn a manifest exec's failure into something the user can act on. +/// +/// The one case worth naming is the output ceiling: what reached the banner was +/// "OUTPUT_LIMIT: Command output exceeded 8388608 bytes and was abandoned", +/// true of a buffer and no help about a container. Same idea as +/// `file_commands::describe_listing_failure`. +pub fn describe_manifest_failure(error: String) -> String { + if error.starts_with(super::exec::OUTPUT_LIMIT_MARKER) { + return format!( + "This container has too many files for Triple-C to inspect for an update (its \ + file listing passed {} MiB). The usual cause is a very large tree installed \ + under /usr/local, /opt, /srv or /var/lib, or a huge number of files directly \ + in a top-level /workspace folder. Removing or moving it into a mounted \ + project folder will let the check run.", + MANIFEST_OUTPUT_LIMIT / (1024 * 1024) + ); + } + error +} + /// Capture a [`Manifest`] from a *running* container. /// /// Preferred over [`manifest_from_image`] for the "from" side whenever the @@ -1071,13 +1196,15 @@ pub async fn manifest_from_image(image: &str) -> Result { /// installed since the last commit, and a verbatim set computed from a stale /// manifest would silently fail to carry that work across. pub async fn manifest_from_container(container_id: &str) -> Result { - let (out, code) = super::exec::exec_oneshot_as( + let (out, code) = super::exec::exec_oneshot_as_limited( container_id, "root", vec!["/bin/sh".to_string(), "-c".to_string(), manifest_script()], Vec::new(), + MANIFEST_OUTPUT_LIMIT, ) - .await?; + .await + .map_err(describe_manifest_failure)?; if !out.contains("###END") { return Err(format!( "Probe of the running container did not complete (exit {})", @@ -2220,6 +2347,234 @@ mod tests { assert_eq!(shell_single_quote("/opt/a'b"), r#"'/opt/a'\''b'"#); } + // ── /workspace summarisation (the 8 MiB overflow) ─────────────────────── + + #[test] + fn a_summarised_subtree_takes_its_size_from_the_summary_section() { + let raw = "###PATHS\n\ + d\t4096\t/workspace/wt\n\ + f\t10\t/workspace/wt/README.md\n\ + S\t0\t/workspace/wt/node_modules\n\ + ###SUMMARY\n\ + 123456\t/workspace/wt/node_modules\n\ + ###DPKG\n\ + ###END\n"; + let m = parse_manifest(raw); + assert_eq!(m.paths.len(), 3); + let s = &m.paths[2]; + assert!(s.is_summary()); + assert!(!s.is_dir(), "a summary stands for content, not an empty directory"); + assert_eq!(s.size, 123456); + } + + #[test] + fn a_summary_with_no_size_line_is_kept_rather_than_dropped() { + // `du` failing must cost the disk estimate, never the directory. + let raw = "###PATHS\nS\t0\t/workspace/wt/vendor\n###SUMMARY\n###END\n"; + let m = parse_manifest(raw); + assert_eq!(m.paths.len(), 1); + assert_eq!(m.paths[0].path, "/workspace/wt/vendor"); + assert_eq!(m.paths[0].size, 0); + } + + #[test] + fn a_summarised_subtree_is_copied_whole_and_counted_in_the_payload() { + // `/workspace/node_modules` holds nothing at its own level but deeper + // summaries: it must not look like an empty tree and be dropped. + let from = manifest( + &[ + ('d', 4096, "/workspace/node_modules"), + ('S', 50_000, "/workspace/node_modules/typescript"), + ('S', 7_000, "/workspace/node_modules/.bin"), + ('d', 4096, "/workspace/wt"), + ('f', 10, "/workspace/wt/README.md"), + ('S', 1_000, "/workspace/wt/src"), + ], + &[], + &[], + ); + let verbatim = compute_verbatim_paths(&from, &Manifest::default(), &[]); + assert_eq!( + verbatim, + vec!["/workspace/node_modules".to_string(), "/workspace/wt".to_string()] + ); + assert_eq!(verbatim_payload_bytes(&from, &verbatim), 58_010); + } + + #[test] + fn a_summary_the_base_also_has_is_still_copied() { + // The presence-in-base filter is a per-path judgment, and a summary + // hides which paths are inside it. Dropping it would lose the user's + // files under any subtree the base happens to ship; copying it is safe + // because the restore's `--skip-old-files` never clobbers the base. + let from = manifest( + &[ + ('d', 4096, "/workspace/tools"), + ('S', 900, "/workspace/tools/lib"), + ], + &[], + &[], + ); + let base = manifest( + &[ + ('d', 4096, "/workspace/tools"), + ('S', 100, "/workspace/tools/lib"), + ], + &[], + &[], + ); + assert_eq!( + compute_verbatim_paths(&from, &base, &[]), + vec!["/workspace/tools/lib".to_string()] + ); + } + + #[test] + fn the_workspace_is_walked_to_a_bounded_depth_and_summarised_below_it() { + let s = manifest_script(); + assert!( + !s.contains("/workspace' -xdev -printf") && !s.contains("'/workspace' '/var/lib'"), + "/workspace must not be walked file-by-file:\n{}", + s + ); + assert!(s.contains("###SUMMARY")); + assert!(s.contains("-maxdepth 2")); + } + + /// Runs the real walk against a scratch tree shaped like the container that + /// overflowed: a few large dependency trees under a writable-layer + /// directory. GNU `find`/`du` are what the image has; the Linux CI runner + /// has the same. + #[cfg(target_os = "linux")] + #[test] + fn the_summarised_walk_stays_small_on_a_tree_of_dependencies() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("workspace"); + for wt in 0..4 { + let nm = root.join(format!("wt{}", wt)).join("node_modules"); + for pkg in 0..50 { + let p = nm.join(format!("pkg{}", pkg)).join("lib"); + std::fs::create_dir_all(&p).unwrap(); + for f in 0..10 { + std::fs::write(p.join(format!("f{}.js", f)), b"0123456789").unwrap(); + } + } + std::fs::write(root.join(format!("wt{}", wt)).join("README.md"), b"hi").unwrap(); + } + std::fs::create_dir_all(root.join("wt0").join("empty")).unwrap(); + std::fs::write(root.join("loose.txt"), b"x").unwrap(); + let root_s = root.to_string_lossy().to_string(); + + let script = format!( + "echo '###PATHS'\n{}\n###END", + summarised_walk(&root_s).replace("###END", "") + ); + let out = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(&script) + .output() + .unwrap(); + let text = String::from_utf8_lossy(&out.stdout).to_string(); + let m = parse_manifest(&text); + + // 2,000 files below; one line per first- and second-level entry above. + assert!(m.paths.len() < 20, "walk was not summarised: {}", text); + let nm0 = format!("{}/wt0/node_modules", root_s); + let entry = m.paths.iter().find(|e| e.path == nm0).expect("summary present"); + assert!(entry.is_summary()); + assert!(entry.size >= 500 * 10, "du size missing: {:?}", entry); + // An empty second-level directory is reported as one, not as content. + let empty = format!("{}/wt0/empty", root_s); + assert!(m.paths.iter().any(|e| e.path == empty && e.is_dir())); + assert!(m.paths.iter().any(|e| e.path == format!("{}/loose.txt", root_s))); + } + + /// The overflow end to end, against a scratch container off the real base: + /// 80k files in a writable-layer `/workspace` tree (past 8 MiB unsummarised) + /// beside a bind-mounted project folder. + /// + /// ```text + /// cargo test -- --ignored --nocapture large_workspace + /// ``` + #[cfg(unix)] + #[tokio::test] + #[ignore = "needs a Docker daemon; creates and removes a throwaway container"] + async fn a_large_workspace_is_probed_without_overflowing_and_still_copied() { + fn docker_cli(args: &[&str]) -> String { + let out = std::process::Command::new("docker") + .args(args) + .output() + .expect("docker CLI"); + assert!( + out.status.success(), + "docker {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + let image = std::env::var("TRIPLE_C_TEST_IMAGE") + .unwrap_or_else(|_| "ghcr.io/shadowdao/triple-c-sandbox:latest".to_string()); + let host = tempfile::tempdir().unwrap(); + for i in 0..50 { + std::fs::write(host.path().join(format!("host-{}.txt", i)), b"h").unwrap(); + } + let mount = format!("{}:/workspace/proj", host.path().display()); + let id = docker_cli(&[ + "run", "-d", "--label", "triple-c.managed=true", + "-v", &mount, + "--entrypoint", "/bin/sh", + &image, "-c", "sleep 300", + ]); + // 80k files with long names, three levels down: ~9.2 MB as a flat walk. + let populate = "for w in 0 1 2 3 4 5 6 7; do for p in $(seq 1 100); do \ + d=/workspace/worktree-number-$w/node_modules/package-with-a-long-name-$p/lib; \ + mkdir -p $d; cd $d && seq -f 'module-file-with-a-long-name-%g.js' 1 100 | xargs touch; \ + done; done; echo x > /workspace/worktree-number-0/README.md; \ + head -c 4096 /dev/zero > /workspace/worktree-number-0/node_modules/real.js"; + docker_cli(&["exec", "-u", "root", &id, "sh", "-c", populate]); + + let result = manifest_from_container(&id).await; + let _ = std::process::Command::new("docker").args(["rm", "-f", &id]).output(); + + let m = result.expect("the probe must not overflow"); + let ws: Vec<_> = m.paths.iter().filter(|e| is_under(&e.path, "/workspace")).collect(); + assert!(ws.len() < 100, "/workspace was walked file by file: {} entries", ws.len()); + assert!( + !ws.iter().any(|e| is_under(&e.path, "/workspace/proj") && e.path != "/workspace/proj"), + "the walk crossed into the bind mount" + ); + let nm = m + .paths + .iter() + .find(|e| e.path == "/workspace/worktree-number-0/node_modules") + .expect("summary entry"); + assert!(nm.is_summary() && nm.size > 0, "{:?}", nm); + + let excl = bind_mount_exclusions(&[pp("proj")]); + let verbatim = compute_verbatim_paths(&m, &Manifest::default(), &excl); + let ws_verbatim: Vec<_> = + verbatim.iter().filter(|p| is_under(p, "/workspace")).cloned().collect(); + assert_eq!( + ws_verbatim, + (0..8).map(|w| format!("/workspace/worktree-number-{}", w)).collect::>() + ); + } + + #[test] + fn an_overflowing_probe_says_what_to_do_rather_than_quoting_a_buffer() { + let raw = format!( + "{}: Command output exceeded 33554432 bytes and was abandoned", + crate::docker::exec::OUTPUT_LIMIT_MARKER + ); + let msg = describe_manifest_failure(raw); + assert!(!msg.contains(crate::docker::exec::OUTPUT_LIMIT_MARKER), "{}", msg); + assert!(!msg.contains("33554432"), "{}", msg); + assert!(msg.contains("/usr/local"), "{}", msg); + // Anything else passes through untouched. + assert_eq!(describe_manifest_failure("boom".into()), "boom"); + } + // ── Stale rollback pins (A5) ───────────────────────────────────────────── -- 2.52.0 From 89908b3b3d91e53aa238ec1a221721853780fc87 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 2 Nov 2026 16:07:13 -0800 Subject: [PATCH 2/2] Address review of the /workspace manifest summarisation - One find pass piped through awk replaces find + du: listing and sizes can no longer disagree under concurrent writes, and /workspace is walked once. A summary's size is now the sum of %s over its non-directories, exactly what verbatim_payload_bytes summed per file, so the disk gate neither under-counts (no du-missing -> 0) nor over-counts dir inodes. - A depth-2 dir with no file at any depth is reported as a plain `d`, so an empty scaffold (a/b/c with no files) is dropped as before instead of becoming a verbatim root. - The pipeline prints ###SUMMARISED-OK only when awk succeeds; check_manifest_complete refuses a manifest without it in both manifest_from_container and manifest_from_image, so a failed walk can never read as an empty /workspace. - The presence-in-base exemption for summaries now logs a warning if the base ever ships a summarised path, and an ignored live test pins that the base ships nothing under /workspace. - plain_walk() emits nothing for no roots instead of `find -xdev` (= /). - Overflow message names /var/www and /etc too. - exec_oneshot_limited takes a user and returns the exit code; the duplicate exec_oneshot_as_limited is gone. MAX_ONESHOT_OUTPUT doc fixed. - The Linux walk test now echoes a following section and asserts it parses, asserts exact sizes and the empty-scaffold case. Measured on the container that overflowed: whole manifest 0.99 MB. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 27 ++- app/src-tauri/src/auth_bridge/mod.rs | 4 +- app/src-tauri/src/docker/exec.rs | 41 ++-- app/src-tauri/src/docker/migration.rs | 332 +++++++++++++++++++------- 4 files changed, 287 insertions(+), 117 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9c9e17c..0a80b45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -579,15 +579,24 @@ security update. Migration is the non-destructive way out; Reset is the destruct the last commit. Migration diffs two filesystem manifests instead, filtered through dpkg ownership and presence-in-the-new-base. Measured on a real project, that turns 8,677 raw path differences into 2 genuinely user-authored ones. -- **`/workspace` is walked two levels deep, never file by file** (`SUMMARISED_ROOTS`). Below that, - each non-empty directory is one `S` entry sized by `du -sbx`. A user's worktrees and - `node_modules` made a flat walk 8.6 MB — past the exec cap, so every staleness check failed on - start and stop — and pruning `node_modules`/`.git` by name only reached 6.3 MB. Summaries lose - nothing: below `/workspace/` every per-path filter is vacuous, and `tar` copies a directory - recursively. Two rules: the `S` line comes from `find`, never from `du`, so a failed size costs - the estimate and not the directory; and a summary is exempt from presence-in-the-base (it hides - which paths are inside — `--skip-old-files` on restore is the per-file answer). Depth 2, not 1, - because `du -x` on a first-level bind-mount point would walk the host repository. +- **`/workspace` is reported two levels deep, never file by file** (`SUMMARISED_ROOTS`). One + `find -xdev` pass is piped through `awk`, which turns each depth-2 directory into one `S` entry + sized as the sum of the files beneath it (the same number the per-file sum gave the disk gate), + or a plain `d` if it holds no file at any depth (so empty scaffolds stay empty to the filter). A + user's worktrees and `node_modules` made a flat walk 8.6 MB — past the exec cap, so every + staleness check failed on start and stop — and pruning `node_modules`/`.git` by name only + reached 6.3 MB; now the whole manifest is ~1 MB. Summaries lose nothing: below `/workspace/` + every per-path filter is vacuous, and `tar` copies a directory recursively. Three rules: + - **A failed `awk` is an error, never an empty `/workspace`.** The pipeline prints + `###SUMMARISED-OK` only on success and `check_manifest_complete` refuses a manifest without + it — otherwise the swap would destroy what the walk failed to see. + - **A summary is exempt from presence-in-the-base**, because it hides which paths are inside; + `--skip-old-files` on restore is the per-file answer. That rests on the base shipping nothing + under `/workspace` — pinned by the ignored live test + `the_base_ships_nothing_under_a_summarised_root`, and logged as a warning if a base breaks it. + - **Only `/workspace`.** The other roots stay per file: dpkg ownership and presence-in-the-base + are exactly what make their delta honest (`/usr/local/lib/node_modules`, base content in + `/opt`), and they are under 1 MB together. ### Authentication diff --git a/app/src-tauri/src/auth_bridge/mod.rs b/app/src-tauri/src/auth_bridge/mod.rs index 7473cbc..88a0824 100644 --- a/app/src-tauri/src/auth_bridge/mod.rs +++ b/app/src-tauri/src/auth_bridge/mod.rs @@ -327,7 +327,9 @@ async fn poll_loop( // bridge or stopping the container doesn't wait out an in-flight poll. let discovery = tokio::select! { _ = cancel.changed() => break, - res = exec_oneshot_limited(&container_id, cmd, PROC_NET_OUTPUT_LIMIT) => res, + res = exec_oneshot_limited(&container_id, "claude", cmd, PROC_NET_OUTPUT_LIMIT) => { + res.map(|(out, _)| out) + } }; match discovery { diff --git a/app/src-tauri/src/docker/exec.rs b/app/src-tauri/src/docker/exec.rs index b64a86c..f9f1e91 100644 --- a/app/src-tauri/src/docker/exec.rs +++ b/app/src-tauri/src/docker/exec.rs @@ -521,8 +521,9 @@ pub fn now_epoch_secs() -> u64 { /// container. Falls back to 1000:1000 (the image's build-time ids) if the exec /// fails, which is strictly better than the 0:0 a default tar header carries. pub async fn container_user_ids(container_id: &str) -> (u64, u64) { - let out = exec_oneshot_limited( + let (out, _) = exec_oneshot_limited( container_id, + "claude", vec!["sh".to_string(), "-c".to_string(), "id -u; id -g".to_string()], 256, ) @@ -543,9 +544,12 @@ pub async fn container_user_ids(container_id: &str) -> (u64, u64) { /// caller sees a byte, and what it is reading is *container-controlled* — the /// scheduler notifications reader `cat`s up to 50 files with no size cap, and /// the auth bridge reads `/proc/net/tcp` every two seconds. Neither has an -/// upstream bound, so this is where the bound goes. Generous enough that no -/// legitimate reader (the largest is a package manifest of a full image) comes -/// close. +/// upstream bound, so this is where the bound goes. It is the default, not a +/// promise that every reader fits: the migration manifest of a running +/// container did pass it (8.6 MB from a busy `/workspace`), and now runs under +/// its own `migration::MANIFEST_OUTPUT_LIMIT` through [`exec_oneshot_limited`]. +/// A reader that can legitimately grow with the user's files should do the +/// same rather than raise this. pub const MAX_ONESHOT_OUTPUT: usize = 8 * 1024 * 1024; /// The auth bridge's per-tick budget. It reads two procfs files whose rows are @@ -586,16 +590,20 @@ pub async fn exec_oneshot(container_id: &str, cmd: Vec) -> Result, limit: usize, -) -> Result { - exec_oneshot_inner(container_id, "claude", cmd, Vec::new(), limit) - .await - .map(|(output, _)| output) +) -> Result<(String, i64), String> { + exec_oneshot_inner(container_id, user, cmd, Vec::new(), limit).await } /// Like `exec_oneshot`, but passes additional environment variables to the exec @@ -642,19 +650,6 @@ pub async fn exec_oneshot_as( exec_oneshot_inner(container_id, user, cmd, env, MAX_ONESHOT_OUTPUT).await } -/// [`exec_oneshot_as`] with a caller-chosen output ceiling, for a reader that -/// knows its legitimate output can pass [`MAX_ONESHOT_OUTPUT`] — the migration -/// manifest of a container with a large `/opt` is the one today. -pub async fn exec_oneshot_as_limited( - container_id: &str, - user: &str, - cmd: Vec, - env: Vec, - limit: usize, -) -> Result<(String, i64), String> { - exec_oneshot_inner(container_id, user, cmd, env, limit).await -} - /// What a one-shot exec printed, with the two streams still tellable apart. /// /// `combined` is stdout and stderr interleaved in arrival order — the shape diff --git a/app/src-tauri/src/docker/migration.rs b/app/src-tauri/src/docker/migration.rs index 23299b9..20f5cf5 100644 --- a/app/src-tauri/src/docker/migration.rs +++ b/app/src-tauri/src/docker/migration.rs @@ -94,9 +94,12 @@ pub const MANIFEST_ROOTS: &[&str] = &[ "/var/www", ]; -/// [`MANIFEST_ROOTS`] entries walked only to [`SUMMARY_DEPTH`], with every -/// non-empty directory at that depth reported as one [`SUMMARY_KIND`] entry -/// carrying the subtree's total size. +/// [`MANIFEST_ROOTS`] entries reported only to [`SUMMARY_DEPTH`], with every +/// directory at that depth that holds at least one non-directory (at any depth) +/// reported as one [`SUMMARY_KIND`] entry whose size is the sum of those +/// files' sizes — exactly what [`verbatim_payload_bytes`] would have summed +/// from a per-file listing. A directory holding no file is reported as a plain +/// `d`, so the empty-tree filter still drops it. /// /// **Why `/workspace`, and why it has to be.** It is the one root whose size is /// the user's rather than the image's: git worktrees, `node_modules`, a pnpm @@ -116,16 +119,30 @@ pub const MANIFEST_ROOTS: &[&str] = &[ /// first-level entries; a summary just stops listing what was going to be /// pruned away. Migration's `tar` still archives the directory recursively. /// -/// **Why depth 2 and not 1.** The first level holds the bind-mount points. -/// `find -xdev` lists a mount point without descending, but measuring one with -/// `du -x` would walk the whole host repository — so summaries start a level -/// below, where `-xdev` has already refused to enter any mount. +/// **One `find` pass, aggregated by `awk`.** The listing and the sizes come +/// from the same walk, so they cannot disagree under concurrent writes, and +/// `-xdev` keeps it off the bind mounts (a mount point is listed, never +/// entered). Because the walk's presence then depends on `awk`, the pipeline +/// prints `###SUMMARISED-OK` only when `awk` succeeded, and +/// [`check_manifest_complete`] refuses a manifest without it — a missing +/// `/workspace` must never read as an empty one. +/// +/// **Why the base-presence exemption is safe.** See the check in +/// [`compute_verbatim_paths`]; it rests on the base shipping nothing under +/// these roots, which `the_base_ships_nothing_under_a_summarised_root` pins and +/// which is logged loudly if it ever stops being true. pub const SUMMARISED_ROOTS: &[&str] = &["/workspace"]; /// Depth below a [`SUMMARISED_ROOTS`] entry at which directories are -/// summarised instead of walked. See there for why it is 2. +/// summarised instead of listed. 1 would be just as correct for migration (the +/// verbatim set prunes to first-level entries anyway); 2 keeps the immediate +/// contents of each top-level folder visible in the manifest for debugging, at +/// a few hundred lines on the container that overflowed. pub const SUMMARY_DEPTH: usize = 2; +/// Marker the summarised walk prints after a successful pass, once per root. +const SUMMARISED_OK: &str = "SUMMARISED-OK"; + /// [`ManifestEntry::kind`] of a summarised directory. Not a `find` `%y` letter. pub const SUMMARY_KIND: char = 'S'; @@ -220,22 +237,23 @@ pub const PROBE_LABEL_MIGRATION: &str = "migration"; pub struct ManifestEntry { /// `find`'s `%y`: `f` regular, `d` directory, `l` symlink, … — or /// [`SUMMARY_KIND`] for a directory under a [`SUMMARISED_ROOTS`] entry that - /// was not walked file by file. + /// was not listed file by file. pub kind: char, - /// Bytes on disk; for a summary, the whole subtree's (`du -sb`), or 0 when - /// that could not be measured. + /// `find`'s `%s`; for a summary, the sum of `%s` over every non-directory + /// beneath it, from the same pass. pub size: u64, pub path: String, } impl ManifestEntry { /// A directory reported as itself, with nothing implied about content. - /// A summary is deliberately *not* one: it stands for a non-empty subtree. + /// A summary is deliberately *not* one: it stands for a subtree that holds + /// at least one file. pub fn is_dir(&self) -> bool { self.kind == 'd' } - /// A non-empty directory whose contents were aggregated rather than listed. + /// A directory whose contents were aggregated rather than listed. pub fn is_summary(&self) -> bool { self.kind == SUMMARY_KIND } @@ -258,6 +276,9 @@ pub struct Manifest { pub etc_paths: BTreeSet, /// `package -> version` for every installed dpkg package. pub dpkg_versions: BTreeMap, + /// How many [`SUMMARISED_ROOTS`] walks reported finishing. See + /// [`check_manifest_complete`]. + pub summarised_roots_complete: usize, } impl Manifest { @@ -273,19 +294,21 @@ impl Manifest { /// Sections are separated by sentinel lines so one exec answers every question; /// on a 5.49 GB image the whole thing takes about three seconds. Every command /// is failure-tolerant (`2>/dev/null`, no `set -e`) because a missing `npm` or -/// an unreadable directory must degrade one section, not the run. +/// an unreadable directory must degrade one section, not the run — with one +/// exception, the summarised walk, whose failure [`check_manifest_complete`] +/// turns into an error because degrading it would lose data. pub fn manifest_script() -> String { let feature_paths = FEATURE_PROBES .iter() .map(|(p, _)| shell_single_quote(p)) .collect::>() .join(" "); - let roots = MANIFEST_ROOTS + let plain_roots: Vec<&str> = MANIFEST_ROOTS .iter() + .copied() .filter(|p| !SUMMARISED_ROOTS.contains(p)) - .map(|p| shell_single_quote(p)) - .collect::>() - .join(" "); + .collect(); + let plain = plain_walk(&plain_roots); let summarised = SUMMARISED_ROOTS .iter() .map(|r| summarised_walk(r)) @@ -301,7 +324,8 @@ pub fn manifest_script() -> String { format!( r#" echo '###PATHS' -find {roots} -xdev -printf '%y\t%s\t%p\n' 2>/dev/null +{plain} +echo '###PATHS' {summarised} echo '###DPKG' cat /var/lib/dpkg/info/*.list 2>/dev/null | grep -E '^/({dpkg_filter})(/|$)' @@ -323,26 +347,90 @@ exit 0 ) } -/// The `###PATHS` lines for one [`SUMMARISED_ROOTS`] entry, followed by a -/// `###SUMMARY` section of `du -sb` lines (`bytespath`) for its summaries. +/// A per-file `find` over `roots`, or nothing at all when there are none. /// -/// The summary entry itself comes from `find`, not `du`: the directory's -/// presence must never depend on the size measurement succeeding, or a `du` -/// failure would make a whole subtree vanish from the migration. A missing size -/// costs only the disk estimate. Empty directories at the boundary are printed -/// as ordinary `d` entries, so the empty-tree filter still sees them as empty. +/// The empty case is not cosmetic: `find -xdev …` with no starting point walks +/// `.`, which in a probe container is `/` — the whole filesystem, straight into +/// an output-capped manifest. +fn plain_walk(roots: &[&str]) -> String { + if roots.is_empty() { + return String::new(); + } + let roots = roots + .iter() + .map(|p| shell_single_quote(p)) + .collect::>() + .join(" "); + format!("find {} -xdev -printf '%y\\t%s\\t%p\\n' 2>/dev/null", roots) +} + +/// Aggregates a per-file `find` listing of one root (`kindsizepath`) +/// into [`SUMMARY_DEPTH`]-limited `###PATHS` lines. See [`SUMMARISED_ROOTS`]. +/// +/// Entries above the boundary are printed as they arrive; a directory *at* the +/// boundary is held back and printed at the end — as `S` with the summed size +/// of every non-directory beneath it, or as its own `d` line when it has none, +/// so an empty scaffold stays empty to the filter. A deeper entry whose +/// boundary ancestor was never seen is printed rather than dropped (it cannot +/// happen in a pre-order walk; erring towards over-listing is the safe side). +/// The path is everything after the second tab, so a tab in a name survives. +/// POSIX `awk` only: the image has `mawk`. +const SUMMARY_AWK: &str = r#"BEGIN { FS = "\t"; rl = length(root) } +{ + kind = $1; size = $2 + p = $0; sub(/^[^\t]*\t[^\t]*\t/, "", p) + if (p == root || substr(p, 1, rl + 1) != root "/") { print; next } + n = split(substr(p, rl + 2), c, "/") + if (n < depth) { print; next } + key = root + for (i = 1; i <= depth; i++) key = key "/" c[i] + if (n == depth) { + if (kind != "d") { print; next } + order[++nb] = key; dsize[key] = size; files[key] = 0; bytes[key] = 0 + next + } + if (!(key in files)) { print; next } + if (kind != "d") { files[key]++; bytes[key] += size } +} +END { + for (i = 1; i <= nb; i++) { + k = order[i] + if (files[k] > 0) printf "S\t%.0f\t%s\n", bytes[k], k + else printf "d\t%s\t%s\n", dsize[k], k + } +}"#; + +/// The `###PATHS` lines for one [`SUMMARISED_ROOTS`] entry, from one `find` +/// pass piped through [`SUMMARY_AWK`], then `###SUMMARISED-OK` only if `awk` +/// succeeded. fn summarised_walk(root: &str) -> String { - let q = shell_single_quote(root); - let boundary = shell_single_quote(&format!("{}{}", root, "/*".repeat(SUMMARY_DEPTH))); format!( - "find {q} -xdev -maxdepth {d} \\( -type d -path {boundary} ! -empty -printf '{k}\\t0\\t%p\\n' \\) -o -printf '%y\\t%s\\t%p\\n' 2>/dev/null\n\ - echo '###SUMMARY'\n\ - find {q} -xdev -mindepth {d} -maxdepth {d} -type d ! -empty -exec du -sbx {{}} + 2>/dev/null", + "find {q} -xdev -printf '%y\\t%s\\t%p\\n' 2>/dev/null | awk -v root={q} -v depth={d} {prog} && echo '###{ok}'", + q = shell_single_quote(root), d = SUMMARY_DEPTH, - k = SUMMARY_KIND, + prog = shell_single_quote(SUMMARY_AWK), + ok = SUMMARISED_OK, ) } +/// Refuse a manifest whose summarised walk did not finish. +/// +/// Every other section degrades on failure, and that is right for them — a +/// missing `npm` costs the npm delta. This one cannot: the summarised walk is +/// the *only* record of what lives under `/workspace`, so a silently absent one +/// reads as "nothing there" and the container swap would destroy it. +pub fn check_manifest_complete(m: &Manifest) -> Result<(), String> { + if m.summarised_roots_complete < SUMMARISED_ROOTS.len() { + return Err(format!( + "Could not list {} inside the container (the probe's `awk` step did not finish), \ + so Triple-C cannot tell what an update would need to carry across.", + SUMMARISED_ROOTS.join(", ") + )); + } + Ok(()) +} + + /// Parse the output of [`manifest_script`]. /// /// Unknown sections and malformed lines are skipped rather than failing: the @@ -351,15 +439,14 @@ fn summarised_walk(root: &str) -> String { pub fn parse_manifest(raw: &str) -> Manifest { let mut m = Manifest::default(); let mut section = ""; - // `du -sb` lines for summary entries, applied once everything has been - // read so the two halves cannot depend on output order. - let mut summary_sizes: HashMap = HashMap::new(); for line in raw.lines() { let line = line.strip_suffix('\r').unwrap_or(line); if let Some(name) = line.strip_prefix("###") { + if name == SUMMARISED_OK { + m.summarised_roots_complete += 1; + } section = match name { - "PATHS" | "SUMMARY" | "DPKG" | "APT" | "NPM" | "FEATURES" | "ETC" | "PKGVER" - | "END" => name, + "PATHS" | "DPKG" | "APT" | "NPM" | "FEATURES" | "ETC" | "PKGVER" | "END" => name, _ => "", }; continue; @@ -373,13 +460,6 @@ pub fn parse_manifest(raw: &str) -> Manifest { m.paths.push(entry); } } - "SUMMARY" => { - if let Some((bytes, path)) = line.split_once('\t') { - if let Ok(bytes) = bytes.trim().parse::() { - summary_sizes.insert(path.to_string(), bytes); - } - } - } "DPKG" => { m.dpkg_owned.insert(line.to_string()); } @@ -407,11 +487,6 @@ pub fn parse_manifest(raw: &str) -> Manifest { _ => {} } } - for entry in m.paths.iter_mut().filter(|e| e.is_summary()) { - if let Some(bytes) = summary_sizes.get(&entry.path) { - entry.size = *bytes; - } - } m } @@ -551,6 +626,18 @@ pub fn compute_verbatim_paths( // would drop every user file under any subtree the base happens to ship // too. Copying it is safe — the restore's `--skip-old-files` keeps the // base's copy of anything both have, which is the per-file answer. + // What it cannot do is notice base content the new base *removed*, so + // this rests on the base shipping nothing under a summarised root — + // pinned by `the_base_ships_nothing_under_a_summarised_root`, and said + // out loud here if a base ever breaks it. + if base_paths.contains(p) && entry.is_summary() { + log::warn!( + "The base image ships {}, which this manifest only summarises — copying the \ + container's whole subtree and relying on --skip-old-files to keep the base's \ + files. SUMMARISED_ROOTS assumes the base ships nothing there; revisit it.", + p + ); + } if base_paths.contains(p) && !entry.is_summary() { continue; } @@ -1166,7 +1253,9 @@ pub async fn manifest_from_image(image: &str) -> Result { } )); } - Ok(parse_manifest(&out.stdout)) + let m = parse_manifest(&out.stdout); + check_manifest_complete(&m)?; + Ok(m) } /// Turn a manifest exec's failure into something the user can act on. @@ -1180,9 +1269,9 @@ pub fn describe_manifest_failure(error: String) -> String { return format!( "This container has too many files for Triple-C to inspect for an update (its \ file listing passed {} MiB). The usual cause is a very large tree installed \ - under /usr/local, /opt, /srv or /var/lib, or a huge number of files directly \ - in a top-level /workspace folder. Removing or moving it into a mounted \ - project folder will let the check run.", + under /usr/local, /opt, /srv, /var/lib, /var/www or /etc, or a huge number of \ + files directly in a top-level /workspace folder. Removing it, or moving it \ + into a mounted project folder, will let the check run.", MANIFEST_OUTPUT_LIMIT / (1024 * 1024) ); } @@ -1196,11 +1285,10 @@ pub fn describe_manifest_failure(error: String) -> String { /// installed since the last commit, and a verbatim set computed from a stale /// manifest would silently fail to carry that work across. pub async fn manifest_from_container(container_id: &str) -> Result { - let (out, code) = super::exec::exec_oneshot_as_limited( + let (out, code) = super::exec::exec_oneshot_limited( container_id, "root", vec!["/bin/sh".to_string(), "-c".to_string(), manifest_script()], - Vec::new(), MANIFEST_OUTPUT_LIMIT, ) .await @@ -1211,7 +1299,9 @@ pub async fn manifest_from_container(container_id: &str) -> Result= 500 * 10, "du size missing: {:?}", entry); - // An empty second-level directory is reported as one, not as content. - let empty = format!("{}/wt0/empty", root_s); - assert!(m.paths.iter().any(|e| e.path == empty && e.is_dir())); + assert_eq!(entry.size, 5_000, "{:?}", entry); + for empty in [format!("{}/wt0/empty", root_s), format!("{}/scaffold/a", root_s)] { + let e = m.paths.iter().find(|e| e.path == empty).expect("listed"); + assert!(e.is_dir(), "a tree with no files is not content: {:?}", e); + } assert!(m.paths.iter().any(|e| e.path == format!("{}/loose.txt", root_s))); + // The walk terminates its section: the next one still parses. + assert_eq!(m.apt_manual, strs(&["socat"])); + assert_eq!(m.summarised_roots_complete, 1, "{}", text); + + // And the migration sees the same thing it would have per file. + let from = Manifest { + paths: m + .paths + .iter() + .map(|e| ManifestEntry { + path: e.path.replacen(&root_s, "/workspace", 1), + ..e.clone() + }) + .collect(), + ..Default::default() + }; + let verbatim = compute_verbatim_paths(&from, &Manifest::default(), &[]); + assert!(!verbatim.contains(&"/workspace/scaffold".to_string()), "{:?}", verbatim); + assert_eq!( + verbatim_payload_bytes(&from, &["/workspace/wt0".to_string()]), + 5_000 + 2 + ); + } + + /// Pins the assumption that makes summaries safe to exempt from the + /// presence-in-base check: the base ships nothing under a summarised root. + /// If a future Dockerfile puts content in /workspace this fails, and + /// `compute_verbatim_paths` needs revisiting before it ships. + /// + /// ```text + /// cargo test -- --ignored --nocapture base_ships_nothing + /// ``` + #[tokio::test] + #[ignore = "needs a Docker daemon and the base image"] + async fn the_base_ships_nothing_under_a_summarised_root() { + let image = std::env::var("TRIPLE_C_TEST_IMAGE") + .unwrap_or_else(|_| "ghcr.io/shadowdao/triple-c-sandbox:latest".to_string()); + let m = manifest_from_image(&image).await.expect("base probe"); + for root in SUMMARISED_ROOTS { + let inside: Vec<_> = m + .paths + .iter() + .filter(|e| is_under(&e.path, root) && e.path != *root) + .map(|e| e.path.as_str()) + .collect(); + assert!(inside.is_empty(), "the base ships {:?} under {}", inside, root); + } } /// The overflow end to end, against a scratch container off the real base: @@ -2549,7 +2704,10 @@ mod tests { .iter() .find(|e| e.path == "/workspace/worktree-number-0/node_modules") .expect("summary entry"); - assert!(nm.is_summary() && nm.size > 0, "{:?}", nm); + // Every file below is empty but `real.js`: the per-file sum, exactly, + // with no directory inode sizes in it — computed by the image's `mawk`. + assert!(nm.is_summary() && nm.size == 4096, "{:?}", nm); + assert_eq!(m.summarised_roots_complete, SUMMARISED_ROOTS.len()); let excl = bind_mount_exclusions(&[pp("proj")]); let verbatim = compute_verbatim_paths(&m, &Manifest::default(), &excl); @@ -2570,7 +2728,13 @@ mod tests { let msg = describe_manifest_failure(raw); assert!(!msg.contains(crate::docker::exec::OUTPUT_LIMIT_MARKER), "{}", msg); assert!(!msg.contains("33554432"), "{}", msg); - assert!(msg.contains("/usr/local"), "{}", msg); + // Every root still walked file by file, so the advice is never wrong + // about where to look. + for root in MANIFEST_ROOTS.iter().chain(["/etc"].iter()) { + if !SUMMARISED_ROOTS.contains(root) { + assert!(msg.contains(root), "message omits {}: {}", root, msg); + } + } // Anything else passes through untouched. assert_eq!(describe_manifest_failure("boom".into()), "boom"); } -- 2.52.0