feat(install): check free disk space before SDK downloads and extraction

[rominf]

Summary

Check free disk space before starting a multi-gigabyte download or extraction, and report an out-of-space failure in plain language instead of a raw OS error.

Root cause

Nothing in the codebase looked at free space, and nothing handled the resulting failure. A user with a nearly-full disk got a low-level write error partway through an install rather than an upfront statement of what was needed. Verified by searching apps/, crates/, engines/, and xtask/ for available_space, free_space, statvfs, ENOSPC, StorageFull, disk_space, fs2, and prose variants — zero hits, and nothing mapped ErrorKind::StorageFull to a user-facing message.

What this adds

A new crates/rocm-core/src/disk_space.rs:

• mount_for_path / available_space_for_path resolve the filesystem that will actually hold a path — walking up to the nearest existing ancestor, so a not-yet-created destination still resolves correctly, then picking the longest matching mount point.
• ensure_space_for (hard fail) and warn_if_low_space (advisory).
• map_write_error for direct writes, and subprocess_full_disk_error for a helper process such as tar, whose out-of-space failure arrives as stderr text rather than an io::Error.

The SDK tarball install path gains a preflight before download and extraction, plus a best-effort Content-Length probe.

Technical decisions

Free space is only reported when the filesystem is positively identified. Longest-prefix mount matching is only correct if every mount is listed, and it is not: sysinfo omits tmpfs by default and skips NFS/CIFS unless opted in. When the real mount is missing the prefix filter does not fail — it falls through to the nearest listed ancestor, in practice /, and reports an unrelated filesystem's free space. Since the download check hard-fails, that refuses a valid install citing a filesystem the download never touches. A device-ID cross-check between the resolved path and the selected mount point rejects that case, reporting the space as unknown instead.

Unknown space never blocks. If the filesystem cannot be identified, or the HEAD probe fails or omits Content-Length, the check passes silently. This is what makes the check above safe: it converts "confidently wrong" into "no opinion".

linux-tmpfs on, linux-netdevs off. Enabling tmpfs enumeration measures a cache or install root under a tmpfs /tmp (the Fedora and Arch default) rather than merely declining to guess. Network filesystems stay off: statvfs on a hard-mounted share can block indefinitely, which is not an acceptable cost for a preflight. NFS and CIFS paths therefore report unknown and fail open.

Hard-fail on the download, warn on the extraction. Content-Length is exact, so refusing up front saves a doomed multi-gigabyte transfer. The extracted size is only an estimate, so a shortfall there is a warning. Because that warning can be right, tar's out-of-space stderr is mapped to the same plain-language message.

An implausible Content-Length is ignored rather than acted on. The header is unauthenticated and is never cross-checked against the body the GET delivers, so an inflated value would refuse an install that would succeed. Past a ceiling the preflight is skipped; download_file still checks the real buffered body length before writing.

The margin is proportional, not flat. max(payload / 20, 32 MiB). download_file_to_path is a general-purpose helper — its other callers fetch the uv binary and similar — and a flat 256 MiB margin turned a 20 MiB download into a 276 MiB requirement, refusing it on small volumes. At SDK-tarball scale the proportional part lands in the same range as the old constant.

Extracted size is estimated at 4x compressed. No manifest or index field carries the uncompressed size. Observed gzip ratios on these tarballs run about 2-3x, so 4x is a deliberate conservative upper bound. Documented on the constant.

Windows

canonicalize returns verbatim paths (\\?\C:\...) whose Prefix variant never equals a mount point's C:\, so component-wise starts_with failed for every mount and the preflight was a silent no-op on Windows. Verbatim prefixes are now stripped before matching, and comparison is case-insensitive there since volumes are.

I have no Windows host, so this is reasoned from the std source and covered by unit tests, not observed at runtime. The prefix-stripping is pure string handling and is exercised on every platform; the end-to-end mount selection against a C:\-style table is a #[cfg(windows)] test that runs in CI but not locally. Mapped network drives remain invisible on Windows for the same reason as NFS on Linux, and fail open.

Non-goals

• The default --format wheel path gets no preflight — Free-space preflight does not cover the default --format wheel install path #191. uv pip install resolves the wheel set at run time, so there is no cheap upfront size signal comparable to a tarball's Content-Length; it needs its own approach. Because the default path is uncovered this is Refs [Issue]: No free-space check before multi-GB downloads and extractions #159, not a fix for it.
• A cached copy of the tarball is not counted in the requirement — Tarball space preflight under-asks when a cached download is being replaced #192. Under-asks rather than over-asks, so the failure mode is a late error, not a false refusal.
• Extraction on Windows keeps the raw error. run_command discards subprocess stderr on Windows, so tar's out-of-space text is not available to map there.

Dependencies

sysinfo was promoted to [workspace.dependencies] with the linux-tmpfs feature, and rocm-core now uses it. No new package enters the graph — the Cargo.lock diff is a single edge — so THIRD_PARTY_NOTICES.txt and about.toml are unchanged (sysinfo 0.34.2 is already listed). I verified this by inspecting the lock diff rather than by running cargo xtask tpn --check, since cargo-about was not installed here; CI's notices check confirms.

Tests

Unit tests covering byte formatting, the proportional margin at both uv and SDK scale, saturating arithmetic, mount selection (longest prefix, no match, siblings, verbatim Windows prefixes), the device-ID rejection of a mount that does not own the path, ancestor walking, message content, StorageFull mapping versus passthrough, tar ENOSPC recognition versus passthrough, and a preflight that skips when the HEAD probe is unreachable.

The refusal policy is driven through an internal seam that takes the resolved free-space figure, so both the refusal and the fail-open paths are asserted against synthetic values rather than whatever the host happens to have free.

Verification

cargo fmt --all -- --check, cargo clippy --locked --workspace --all-targets -- -D warnings, and cargo test --workspace --all-targets --no-fail-fast all pass locally, except two proc_lifecycle failures that are pre-existing on this WSL2 host and unrelated to this change — tracked in #168, with #169 open to address them.

The mis-attribution bug was reproduced before fixing: a path under a tmpfs /dev/shm reported the root filesystem's free space (hundreds of GB against a real 64 MiB). After the change that path reports 64 MiB, and a path on a filesystem that remains unlisted reports unknown and does not block.

Refs #159

[volen-silo]

Good problem to solve and the module is cleanly built — select_mount split out as a pure function is the right call, and the arithmetic is genuinely careful (saturating_* throughout, verified against u64::MAX inputs).

But the safety argument reasons about the wrong side of the comparison. "Content-Length is exact, so hard-failing is safe" is true of the required side. The available side is not exact — available_space_for_path can silently return a different filesystem's free space, and the hard fail then refuses a valid install with a confident, specific, wrong number. "Unknown space never blocks" is structurally true (I verified there is exactly one bail! in the module, at disk_space.rs:191, reachable only from Insufficient) — but it guards the harmless failure mode, not the dangerous one.

---

Blocking

1. select_mount silently attributes another filesystem's free space when the real mount isn't in sysinfo's list

crates/rocm-core/src/disk_space.rs:96-102 picks the longest mount point that is a prefix of the path. If the path's actual mount is missing from sysinfo::Disks::list(), the filter doesn't fail — it falls through to the nearest listed ancestor, virtually always /, and reports that filesystem's free space as the target's. SpaceCheck::Unknown only happens when nothing matches, which on Linux essentially never occurs because / is always listed.

sysinfo 0.34.2 excludes real, writable mounts by default. From the vendored source, src/unix/linux/disk.rs:395-397:

"tmpfs" => !cfg!(feature = "linux-tmpfs"),
// calling statvfs on a mounted CIFS or NFS may hang, when they are mounted with option: hard
"cifs" | "nfs" | "nfs4" => !cfg!(feature = "linux-netdevs"),

Its default feature set is ["component", "disk", "network", "system", "user"] — neither linux-tmpfs nor linux-netdevs. Root Cargo.toml:48 declares sysinfo = "0.34" with no features, and crates/rocm-core/Cargo.toml:26 takes it via workspace = true. So NFS, NFS4, CIFS and tmpfs mounts are invisible to this code.

Demonstrated with a scratch binary replicating nearest_existing_ancestor + select_mount verbatim against real sysinfo 0.34.2:

/dev/shm/rocm-sdk.tar.gz  -> Some(("/", 966154645504))    # code says 966 GB
df /dev/shm               -> tmpfs, 67108864              # reality: 64 MiB

Wrong filesystem, off by ~15,000×, no signal anything is wrong.

The scenario that matters for this audience: an enterprise or HPC workstation with $HOME on NFS — very common for ROCm SDK users on shared infrastructure — and a conventional root partition with 3 GB free. ~/.cache/rocm/ has terabytes. sysinfo omits the NFS mount, select_mount falls back to /, and rocm install sdk --format tarball gives:

Error: not enough free disk space to download the SDK tarball: need about 5.3 GiB
but only 3.1 GiB is available on the filesystem holding
/home/user/.cache/rocm/therock/therock-dist-....tar.gz. Free up 2.2 GiB and retry.

df -h ~/.cache shows 2 TB free. The install is now impossible — there's no bypass flag, and the only remedy the message suggests is freeing space on a filesystem the download won't touch. Same shape where /tmp is tmpfs (Fedora, Arch defaults) with TMPDIR/ROCM_CLI_CACHE_DIR pointing into it.

Suggested fix: before letting Insufficient escalate to a hard failure, verify the selected mount actually owns the path — compare MetadataExt::dev() of the resolved ancestor against the mount point's, and on mismatch return None so it degrades to Unknown and fails open. Enabling sysinfo's linux-netdevs and linux-tmpfs features would cover the common cases, but the dev() cross-check is the load-bearing part: it turns "confidently wrong" back into "unknown", which the existing design already handles correctly.

2. The feature is a silent no-op on Windows

crates/rocm-core/src/disk_space.rs:83-87 — nearest_existing_ancestor returns candidate.canonicalize(). On Windows that's a verbatim path (\\?\C:\Users\...), whose first component parses as Prefix::VerbatimDisk(b'C'). sysinfo reports mount points as C:\ → Prefix::Disk(b'C'). Path::starts_with compares component-wise including the prefix, and Prefix derives PartialEq — two different variants are never equal regardless of payload. So starts_with fails at the first component for every mount, select_mount returns None, and every Windows path yields Unknown.

Net: on a supported platform per AGENTS.md §6, no preflight ever runs. Fails open, so not a false refusal — but the PR ships a feature that does nothing on half its supported platforms while claiming to close #159.

windows-build-and-test passing doesn't contradict this: every select_mount test (disk_space.rs:257-292) uses synthetic Unix-style mount tables, and nothing asserts mount_for_path returns Some for a real path. The gap is invisible to the suite by construction.

Two related Windows issues in the same area: Component::Normal compares case-sensitively, so a volume mounted at C:\Data won't match a path C:\data\...; and mapped network drives are filtered out by sysinfo (src/windows/disk.rs:285-288 keeps only DRIVE_FIXED/DRIVE_REMOVABLE).

This is cheap for a Windows reviewer to settle directly: a temporary dbg!(mount_for_path(Path::new("C:\\Users"))) and see whether it's Some or None. I confirmed the Prefix inequality from Rust 1.96.0 std source but could not execute on Windows.

3. The flat 256 MiB margin makes a general-purpose helper hard-fail small downloads

crates/rocm-core/src/lib.rs:139-143 applies ensure_space_for(..., with_margin(content_length)) inside download_file_to_path, and disk_space.rs:69-71 adds a flat SPACE_MARGIN_BYTES (256 MiB, :28) to every requirement regardless of payload.

download_file_to_path isn't an SDK-tarball function. Its production callers are crates/rocm-core/src/uv.rs:129 (the uv binary, ~15-35 MB, on the default --format wheel path), apps/rocm/src/comfyui.rs:1352, and engines/lemonade/src/lib.rs:1245.

On a machine with 200 MB free, downloading the 20 MB uv binary now hard-fails: "need about 276.0 MiB but only 190.7 MiB is available". The 20 MB it actually needs fits fine. Reachable in small VMs, CI containers, constrained scratch volumes. Suggest a proportional margin with a floor — max(bytes / 20, 32 MiB) — or take the margin as a parameter so the SDK path can ask for 256 MiB without a 20 MB helper download inheriting it.

4. The one operation that only warns is also the one with no ENOSPC message

apps/rocm/src/therock.rs:2141-2147 deliberately downgrades the extraction check to a warning, on the grounds that a late failure is acceptable because the message will be clear. But extract_tarball (therock.rs:2265-2274) shells out to tar via run_command, and map_write_error (disk_space.rs:220-231) only handles std::io::ErrorKind::StorageFull from direct Rust I/O — a subprocess exit code never reaches it.

So when the warning is right and extraction runs out of space, the user gets exactly what #159 asked to eliminate:

extract TheRock tarball artifact: tar: ...: No space left on device

Either map tar's ENOSPC stderr, or say in the PR that extraction still surfaces the raw error.

5. The default install format gets no preflight at all

apps/rocm/src/main.rs:503: #[arg(long, default_value = "wheel")]. grep -n "disk_space::" apps/rocm/src/therock.rs hits only download_file (:2093), preflight_tarball_space (:2129-2144), and write_file_atomically (:2256) — all tarball-path. install_wheel_runtime, the default path and the larger download once PyTorch/torchvision/torchaudio come through uv pip install, has zero preflight and zero ENOSPC mapping. Either extend coverage or state the gap and downgrade to "Refs #159".

Non-blocking

• The hard fail trusts an unauthenticated Content-Length with no ceiling. therock.rs:2106-2115 → :2129-2133. A HEAD response is never cross-checked against the subsequent GET. A CDN or proxy returning an inflated length blocks the install outright even though the GET would deliver the correct smaller body. A bogus 2^63 header yields "need about 8.0 EiB" and a refusal.
• Tests don't cover the false-refusal paths, and two pass vacuously. disk_space.rs:337-341 (zero_requirement_never_fails_on_a_real_path) — with required = 0, available >= 0 always holds, so it never exercises the Unknown branch; nothing would fail if "unknown never blocks" regressed for a nonzero requirement. disk_space.rs:366-374 puts every assertion inside if let Some(warning), so a None return passes asserting nothing. Root cause of both, and of #1 slipping through: check_space_for_path calls the real available_space_for_path directly (:137), so the policy layer — the part that decides to hard-fail — can't be tested against a synthetic mount table. Threading a mount-table parameter through would make all three testable.
• therock.rs:3248-3253 makes a real network connect to 127.0.0.1:1. Fast here (~0.04 s) because the port refuses, but a host that blackholes will sit for ureq's connect timeout — and ureq 2.12.1 documents .timeout_connect() (default 30 s) as taking precedence over .timeout(), so THEROCK_HEAD_PROBE_TIMEOUT_SECS = 10 (therock.rs:36) isn't an airtight ceiling.
• Output-ordering wart: therock.rs:2146 calls progress_line(warning), an unconditional println!, while every other line in install_tarball_runtime accumulates into a local output: String (:1034-1049) printed once by the caller. The warning lands before the block it belongs to, and in apply_runtime_update's dry-run branch appears unindented and detached.
• Redundant work: therock.rs:2093-2097 re-runs a hard check preflight_tarball_space already did on the same path moments earlier. And one preflight performs four independent Disks::new_with_refreshed_list() sweeps, each a full re-enumeration that also re-reads /proc/diskstats and /sys/block/*/queue/rotational — neither used here. 3.6-33 ms measured, so not a perf problem, but new_with_refreshed_list_specifics(DiskRefreshKind::nothing().with_storage(true)) called once would be cheaper and would fix the max_by_key-returns-last tie-break nondeterminism on duplicate mount points.
• Retrying a cached download under-estimates. No cache-hit check in install_tarball_runtime or download_file, so a re-run always re-downloads; write_file_atomically writes a sibling temp then renames, so true peak is archive + existing cached copy + margin while the preflight asks only archive + margin. Under-asks, so late failure, not a false refusal. engines/lemonade/src/lib.rs:976-979 already implements the cache-hit pattern.
• The body says the proc_lifecycle failures are "fixed by #169", but #169 is an open unmerged PR — reads as landed. Also says 15 unit tests; the diff adds 17.

Verified clean

• available_space() uses f_bavail, not f_bfree — the safe direction. sysinfo-0.34.2/src/unix/linux/disk.rs:205-229. Confirmed byte-exact against stat -f here: / reports 975,423,778,816 (bavail·bsize 975,420,985,344) versus bfree·bsize 1,030,413,340,672 — ~55 GB apart. Root-reserved blocks correctly excluded.
• Path::starts_with is component-wise, so the /data vs /database trap doesn't apply — verified empirically (/database/file against [("/",…),("/data",…)] correctly picks /; /mnt/data against [("/mnt/d",…)] returns None). Nothing guards against a future refactor to string comparison, though.
• Arithmetic is overflow-safe — estimated_extracted_size(u64::MAX) == u64::MAX, with_margin(u64::MAX) == u64::MAX, both tested.
• nearest_existing_ancestor edge cases are sound — relative paths resolve against cwd; a deleted cwd makes current_dir() fail → Unknown → fails open; a .. tail can't smuggle a mismatched filesystem past select_mount; a mode-000 directory stops the walk at that directory rather than skipping past it.
• The dependency claim is correct. The Cargo.lock diff is genuinely one line (+ "sysinfo", under rocm-core) — no new [[package]], no version change, no transitive additions. sysinfo 0.34.2 is already at THIRD_PARTY_NOTICES.txt:10873; xtask/src/tpn.rs:100-109 runs cargo about generate --workspace and about.toml has no member allowlist, so scope is the whole workspace and crates/rocm-dash-collectors already held the dep. Nothing new enters the covered graph.
• Merging with #165 conflicts loudly, not silently. git merge-tree --write-tree against #165's live head 8c1ec0c gives a hard content conflict in both apps/rocm/src/therock.rs and crates/rocm-core/src/lib.rs — so no clean-merge-but-broken hazard. Resolution still needs care: #165 replaces the exact lines this PR edits with a call to stream_to_path_atomically, and .map_err(|e| map_write_error(e, destination)) won't typecheck against its anyhow::Error return, so the mapping has to move onto the real io::copy — and whoever resolves must pass &partial, not destination, or the message names sdk.tar.gz when the file that failed is sdk.tar.gz.part-<pid>-<ts>. The preflight target is fine either way since the .part file is a same-directory sibling. Worth landing #165 first — rebasing this onto the .part flow is a same-author fixup; the reverse hands an unfamiliar feature to #165's author.
• Locally: cargo fmt --all -- --check clean; cargo clippy --locked --workspace --all-targets -- -D warnings clean; cargo test -p rocm-core --lib disk_space 13/13; cargo test --workspace --all-targets --no-fail-fast green everywhere except the two known proc_lifecycle WSL2 failures. Leak scan clean (one hit, a pre-existing public endpoint in unchanged context). 21/21 CI checks green.

Not verified: Windows runtime behaviour (no host; mingw link fails, no wine) — see the cheap check suggested under #2; cross-filesystem symlink resolution (no writable second filesystem in this sandbox); and cargo xtask tpn --check (cargo-about not installed — the licensing conclusion rests on reading tpn.rs, about.toml, the notices file and the lock diff, plus CI's passing notices job).

[rominf]

Thanks — the framing was the useful part. You're right that the safety argument only covered the exact side of the comparison; the available side was the one that could refuse a valid install, and I hadn't checked it.

Addressed in 8ae4d52.

1. select_mount attributing another filesystem's free space — fixed

Reproduced first. On this host a path under /dev/shm (tmpfs, 64 MiB real) resolved to ("/", 372307722240) — 372 GB, the same shape as your 966 GB, with st_dev 146 vs 104 for /.

mount_for_path now cross-checks MetadataExt::dev() of the resolved ancestor against the selected mount point's and returns None on mismatch, so it degrades to Unknown and fails open. Verified after the change: a path on a filesystem that is still unlisted returns None, and ensure_space_for(..., u64::MAX) on it succeeds rather than refusing.

I also enabled sysinfo's linux-tmpfs, so tmpfs is now measured rather than merely declined — the /dev/shm repro reports the correct 67108864 instead of falling back. linux-netdevs is deliberately off: it is gated precisely because statvfs on a hard-mounted NFS/CIFS share can block indefinitely, and I'd rather a network home report unknown and fail open than risk hanging a preflight. So your NFS scenario ends in Unknown, not a refusal.

The regression test uses /proc rather than /dev/shm as the stand-in unlisted filesystem: it is a distinct device on every Linux system and is never enumerated, so the test stays deterministic regardless of which optional sysinfo features are on — otherwise enabling linux-tmpfs would have made a /dev/shm-based test silently vacuous, which is the failure mode you flagged elsewhere.

2. Windows no-op — fixed in code, not verified at runtime

Verbatim prefixes are stripped before matching (\\?\C:\... → C:\..., \\?\UNC\server\share → \\server\share), and comparison is case-insensitive on Windows so C:\Data matches C:\data\....

Being explicit about what is what, since you asked:

• Executed: the prefix-stripping is pure string handling and its unit test runs on every platform, including here. The #[cfg(windows)] test that selects from a C:\-style mount table compiled and ran in CI's windows-build-and-test (green), but I did not observe it locally.
• Reasoned from std source only: that Prefix::VerbatimDisk never equals Prefix::Disk and that this was the cause. I have no Windows host and did not run the dbg!(mount_for_path(...)) check you suggested — that still wants a Windows reviewer's eyes, since a passing unit test against a synthetic table is exactly the kind of by-construction coverage you called out.

Mapped network drives stay invisible on Windows for the same reason NFS does on Linux, and now fail open rather than mis-attributing.

3. Flat 256 MiB margin — fixed

with_margin is now payload + max(payload / 20, 32 MiB). The 20 MiB uv download asks for ~52 MiB instead of 276 MiB and fits on your 200 MB machine; at SDK scale (~5 GiB) the proportional part lands at ~256 MiB, so the tarball path keeps the margin it had. Tested at both ends.

4. Extraction ENOSPC — fixed on Linux/macOS, gap remains on Windows

extract_tarball now maps tar's out-of-space stderr to the same plain-language message as direct writes, so the case where the advisory warning turns out to be right no longer ends in the raw error #159 asked to remove.

The gap I'm stating rather than closing: run_command discards subprocess stderr on Windows, so there is no text to match there and extraction keeps the raw error. Noted in the PR description.

5. Default --format wheel path — scoped out, now Refs #159

Filed #191 and changed the PR to Refs #159, as you asked. uv pip install only knows the wheel set after resolving, so there is no Content-Length equivalent — it needs a resolve-only sizing pass, a conservative floor, or post-hoc mapping of uv's output. Different enough that bolting it on here seemed worse than deciding it separately.

Non-blocking

• Tests can't reach the policy layer / two pass vacuously — fixed. The resolved free-space figure is threaded through an internal seam, so the refusal path and the fail-open path are now driven by synthetic values. zero_requirement_never_fails_on_a_real_path is replaced by one that asserts u64::MAX against None; the if let Some(warning) test now asserts the branch it lands in matches whether the filesystem resolved, with the per-branch content assertions moved to a deterministic test.
• Unauthenticated Content-Length with no ceiling — an implausible size is now ignored rather than acted on: past a ceiling the preflight is skipped, and download_file still checks the real buffered body length before writing. So a bogus 2^63 header skips the check instead of refusing.
• timeout_connect — set explicitly on the HEAD probe, so a blackholing host no longer outlives the intended 10s ceiling via the 30s default.
• Output-ordering wart — preflight_tarball_space returns the warning instead of printing it; the caller writes it into the same accumulated block as the rest of the report.
• Four Disks sweeps / rotational reads — now new_with_refreshed_list_specifics(DiskRefreshKind::nothing().with_storage()), so the unused /proc/diskstats and rotational reads are skipped.
• Redundant re-check in download_file — kept deliberately. It is not the same check: the preflight uses the HEAD estimate, this one uses the actual buffered body length, which is what makes ignoring an inflated header safe.
• Cached download under-estimates — filed Tarball space preflight under-asks when a cached download is being replaced #192 rather than fixed here. Under-asks, so late failure rather than false refusal, and it wants the cache-hit pattern from the lemonade engine, which is its own change.
• PR description — fixed both: the proc_lifecycle note no longer reads as though test(proc-lifecycle): stop counting a zombie as a running process #169 landed, and the test count is gone rather than restated wrongly.

#165

Still open as I write this, so I did not rebase — your recommendation to land it first stands, and the resolution hazards you spelled out (mapping moves onto the real io::copy, and it must be &partial, not destination) are noted for when I do it.

Verification

Ran here: cargo fmt --all -- --check clean; cargo clippy --locked --workspace --all-targets -- -D warnings clean (sources touched first, since post--- args aren't fingerprinted); cargo test --workspace --all-targets --no-fail-fast green except the two known proc_lifecycle WSL2 failures. Leak scan clean.

CI: everything green except E2E tests, which failed once on dash-managed-service-metrics — unrelated to this change (sysinfo::Disks is still only a TODO in the dash collectors, so the feature addition can't reach it). Re-ran with no code change and it passed, so I'm calling it a flake rather than a regression. E2E tests (GPU) is still queued on a self-hosted runner.

[volen-silo]

Re-reviewed at 8ae4d52. All five blocking items are addressed, and the framing you took from #1 — turning "confidently wrong" into "no opinion" — is applied consistently.

Confirmed fixed

1. The device-ID cross-check is the right shape, and it can only ever fail open: a btrfs subvolume or ZFS dataset that is not separately mounted also fails it, and lands on Unknown, which is the safe side. Reproduced your repro here — with linux-tmpfs on, mount_for_path("/dev/shm/rocm.tar.gz") returns Some(("/dev/shm", 67108864)), byte-exact against df -B1, where before it returned the root filesystem. The /proc stand-in is a better choice than /dev/shm for the reason you give.

2. Better than you claim for yourself: windows_mount_selection_matches_a_canonicalized_path did run and pass on the Windows runner — it is in the windows-build-and-test log alongside verbatim_windows_prefixes_are_stripped_before_matching. I also checked the two Windows sub-cases I raised and they are not holes: sysinfo 0.34.2 enumerates via FindFirstVolumeW + GetVolumePathNamesForVolumeNameW, so a volume mounted at a folder is listed, and a mapped drive is a disjoint root (Z:\), so it yields no prefix match rather than a wrong one. mount_owns_path returning true on non-Unix is therefore fine as written.

3. max(payload / 20, 32 MiB) — 20 MiB uv asks 52 MiB, 5 GiB SDK asks 5 GiB + 256 MiB, u64::MAX still saturates.

4. Checked the mechanism rather than the claim: on non-Windows run_command bails with "{context}: {stderr}", so tar's text really is in the anyhow chain that subprocess_full_disk_error matches against. The Windows branch does set stderr(Stdio::null()), so your stated gap is exactly right. Worth noting for whoever closes it: capture_command_output_with_temp_files, directly below, already captures Windows stderr to temp files — the null-pipe branch in run_command may just predate it.

5. Refs #159, #191 and #192 filed. #165 still open, so deferring the rebase is right.

Verification I reproduced

cargo fmt --all -- --check clean, cargo clippy --locked --workspace --all-targets -- -D warnings clean (so the lock is consistent with the feature change), cargo test --workspace --all-targets --no-fail-fast green except the two known proc_lifecycle WSL2 failures; disk_space 20/20. Third-party notices current is green in CI, which settles the dependency claim. E2E tests is green on the current run. GPU is still queued.

Three small things, none blocking

1. The enospc match doesn't do what its comment says. GNU tar's out-of-space diagnostic comes from strerror(ENOSPC), which glibc localizes, and tar ships its own gettext catalogues — what a non-English host prints is the translated message, not the errno name. So text.contains("enospc") does not cover the localized case, and nothing in run_command / capture_command_output pins a locale. On a LANG=de_DE.UTF-8 machine the mapping falls through to the raw tar: error, which is what #159 asked to remove. .env("LC_ALL", "C") on the tar invocation would settle it — nothing else consumes that output. I could not demonstrate this here (no non-English locales installed on this host), so it is from the mechanism, not a repro.

2. disk quota exceeded produces a self-contradictory message. output_reports_full_disk matches EDQUOT, but subprocess_full_disk_error appends (N free) from the filesystem's real free space, so a quota hit on a large volume reads:

ran out of disk space while writing to /install/rocm (2.0 TiB free). Free up space on that filesystem and retry.

Right to catch it; the advice is then wrong. Dropping the (N free) suffix when the quota string is what matched, or giving quota its own sentence, fixes it.

3. The Windows verification gap is one test away from closed. windows_mount_selection_matches_a_canonicalized_path still runs against a synthetic table, and warn_if_low_space_returns_a_warning_not_an_error_for_huge_estimates compares two calls into the same code path, so neither can distinguish "resolves" from "silently resolves to nothing" on a real path. A #[cfg(windows)] assert!(mount_for_path(&std::env::temp_dir()).is_some()) would have failed before this commit and passes after, and windows-build-and-test already runs that suite — so the thing you correctly say you cannot verify without a Windows host is verifiable in CI without one.

Nothing blocking left from my side. My CHANGES_REQUESTED is stale — happy to clear it once #165 lands and this is rebased, or now if you would rather rebase after approval.