Windows port — Phase 1: paths, scan & clean#7
Conversation
… under User Data)
…; matches temp.rs %WINDIR% pattern)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39bbc4355e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "C:\\Windows", | ||
| "C:\\Program Files", | ||
| "C:\\Program Files (x86)", | ||
| "C:\\ProgramData", |
There was a problem hiding this comment.
On Windows machines where the OS or program directories are not on C: (for example installs on D: or redirected ProgramData), these hard-coded literals fail the subsequent .exists() filter, so system_total() undercounts or can return zero for the dashboard's system footprint. Build these roots from WINDIR, ProgramFiles, ProgramFiles(x86), and ProgramData/SystemDrive instead of fixed C: paths.
Useful? React with 👍 / 👎.
| path: windir.join("Temp"), | ||
| requires_root: true, |
There was a problem hiding this comment.
When the Windows temp scan surfaces %WINDIR%\Temp, marking its children requires_root sends selected items through execute::pkexec_helper; on Windows this code path is still under the non-macOS arm and tries to spawn pkexec, so selecting these entries cannot clean them and only returns a spawn failure. Gate this root out until Windows elevation is implemented, or add the Windows elevation path before marking it root-required.
Useful? React with 👍 / 👎.
| let home = dirs::home_dir() | ||
| .unwrap_or_else(|| PathBuf::from(if cfg!(windows) { "C:\\" } else { "/" })); |
There was a problem hiding this comment.
On Windows, dirs::home_dir() returns a normal C:\... path, while core_trash::validate() canonicalizes each item's parent before checking Path::starts_with against the home zone; std::fs::canonicalize returns verbatim \\?\C:\... paths there. As a result, user-owned scan items under %USERPROFILE% can be reported as outside the allowed zone and no non-privileged cleanup succeeds. Store a canonicalized/normalized home path for the deletion zone, or normalize both sides before the prefix check.
Useful? React with 👍 / 👎.
| roots.push(TempRoot { | ||
| path: home.join("AppData/Local/Temp"), | ||
| requires_root: false, |
There was a problem hiding this comment.
Adding %LOCALAPPDATA%\Temp as a non-root root means TempService::scan() will offer every immediate subdirectory for deletion, but the existing age filter only runs in the meta.is_file() branch. On Windows those subdirectories are often active installer/app temp workspaces, so a user selecting the new temp entries can trash fresh in-use data despite temp_min_age_days = 7; filter directory mtimes too before pushing them.
Useful? React with 👍 / 👎.
| Self::collect(&h.join("AppData/Local"), 5, &mut items); | ||
| Self::collect(&h.join("AppData/Roaming"), 4, &mut items); |
There was a problem hiding this comment.
In redirected-profile or domain Windows setups, %LOCALAPPDATA% and especially %APPDATA% are not guaranteed to be USERPROFILE\AppData\Local/Roaming; they can point to another local path or a network share. Deriving both roots from home means the new Windows app-cache scan silently misses the real browser/Electron caches in those environments, so prefer LOCALAPPDATA/APPDATA with these paths only as fallbacks.
Useful? React with 👍 / 👎.
| roots.push(TempRoot { | ||
| path: home.join("AppData/Local/Temp"), | ||
| requires_root: false, |
There was a problem hiding this comment.
Windows' user temp directory is controlled by %TEMP%/%TMP%, and users or managed environments can move it away from %LOCALAPPDATA%\Temp. With this hard-coded root, those installs scan and clean the wrong location while leaving the actual temp directory untouched, so resolve the user temp root from TEMP/TMP first and fall back to this default only when they are absent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d478196ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| requires_root: false, | ||
| }); | ||
| // Windows: %LOCALAPPDATA%\Temp (user) and %WINDIR%\Temp (privileged). | ||
| #[cfg(target_os = "windows")] |
There was a problem hiding this comment.
When this Windows arm runs, it appends to the shared root list that still contains /tmp and /var/tmp; on Windows those are drive-rooted paths such as C:\tmp, not guaranteed-missing POSIX paths. On machines with MSYS/Cygwin/dev temp directories there, the scan will surface unrelated entries as admin-only temp files, and selecting them routes through the non-macOS pkexec path instead of the Recycle Bin cleanup, so cleanup fails for those results. Build the temp roots per OS, or skip the POSIX roots under cfg(windows).
Useful? React with 👍 / 👎.
| // level deeper than macOS — Google\Chrome\User Data\Default\Cache. | ||
| #[cfg(target_os = "windows")] | ||
| { | ||
| Self::collect(&h.join("AppData/Local"), 5, &mut items); |
There was a problem hiding this comment.
On Windows, the cache paths emitted from this new root serialize with \ separators, while ui/src/lib/reclaim.ts only recognizes ancestry using / boundaries. Since BigFilesService still walks the same home tree and does not prune Cache directories, a browser cache directory returned here and a large descendant file returned by Big Files are no longer considered related, so the dashboard can double-count reclaimable bytes and build plans that try to delete both parent and child. Normalize separators in the overlap logic before adding Windows cache roots.
Useful? React with 👍 / 👎.
1 participant
Phase 1 of the Windows 10/11 port (builds on Phase 0, already merged). Plan:
docs/superpowers/plans/2026-06-26-windows-port-phase-1-paths-scan-clean.md.Goal: On Windows, scans target the real user home, find Windows app/temp caches, and the dashboard shows a real system footprint — non-privileged scan + cleanup (Recycle Bin) functional.
Changes
dirs::home_dir()→%USERPROFILE%on Windows (the foundational fix: every scan readsConfig.home).dirspinned to 6 to reuse the existing transitive version (no duplicate).system_total:duis absent on Windows → sum the system roots (C:\Windows,Program Files,Program Files (x86),ProgramData) with the internal mtime-cached walker; unixdupath unchanged.app_cache: scan Windows caches under%LOCALAPPDATA%(depth 5 — browsers nest underUser Data) and%APPDATA%.temp: scan%LOCALAPPDATA%\Temp(user) +%WINDIR%\Temp(privileged).No Linux/macOS behavior change.
core-trashalready deletes to the Recycle Bin.