feat(disk-maintenance): SSD TRIM page with multi-select and safety rails#150
feat(disk-maintenance): SSD TRIM page with multi-select and safety rails#150dbfx merged 4 commits intomainAdventDevInc/kudu:mainfrom feat/disk-maintenance-trimAdventDevInc/kudu:feat/disk-maintenance-trimCopy head branch name to clipboard
Conversation
…y rails Adds a Disk Maintenance page (sidebar > Disk Tools) that lists every fixed volume with its media type, last-TRIM time, and a status pill explaining whether TRIM is recommended. Multi-select drives and run TRIM in one action. - Windows uses Optimize-Volume -ReTrim; Linux uses fstrim -v. - macOS rows are read-only with a "Managed by macOS" badge — trimforce is never invoked. - Server-side rails: HDDs filtered, 24h throttle, admin elevation, network/removable filtered, sequential mutex. - Last-TRIM stored in trim-history.json (best-effort cross-check against Windows defrag event log). - 20 new tests (1983 total passing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb32b89df4
ℹ️ 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".
| if (drive.mediaType === 'HDD') { | ||
| return failResult(driveId, start, 'TRIM is not applicable to HDDs.') | ||
| } | ||
| if (drive.trimSupport === 'unsupported' || drive.trimSupport === 'disabled') { |
There was a problem hiding this comment.
Block removable volumes before dispatching TRIM
The server-side guard in runTrimForDrive rejects HDD and unsupported states but never rejects drive.isRemovable, so a crafted IPC call can still run TRIM against removable media even though the renderer disables those checkboxes. Because disk:trim:run re-lists drives and trusts this function for enforcement, this is a real defense-in-depth gap that can execute maintenance on external drives the UI explicitly marks as not applicable.
Useful? React with 👍 / 👎.
| // findmnt may be missing on busybox/Alpine — fall back to /proc/mounts (best-effort). | ||
| return [] | ||
| } |
There was a problem hiding this comment.
Add actual fallback when findmnt is unavailable
When findmnt fails, the Linux enumerator immediately returns an empty list despite the comment stating there is a /proc/mounts fallback. On systems where findmnt is not installed (common in minimal/containerized distros), Disk Maintenance will always show no drives and TRIM cannot run at all, so this path silently disables the feature instead of degrading gracefully.
Useful? React with 👍 / 👎.
…roc/mounts fallback, unused import - Server-side reject `drive.isRemovable` in runTrimForDrive (P1 defense-in-depth gap caught by Codex). The renderer already disables removable checkboxes via isSelectable(), but a crafted IPC call could bypass that — now the server rejects too. - Implement actual /proc/mounts fallback when findmnt is unavailable (P2 — minimal Linux distros like Alpine/busybox were silently showing zero drives instead of degrading gracefully). - Drop unused BrowserWindow import. 5 new tests covering the removable rejection and /proc/mounts parser (octal whitespace decode, blank-line handling, missing-file fallback). 1988/1988 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9e3204581
ℹ️ 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".
| const name = devPath.replace(/^\/dev\//, '') | ||
| if (name.startsWith('mapper/')) return name | ||
| // NVMe: nvme0n1p2 -> nvme0n1 | ||
| const nvme = name.match(/^(nvme\d+n\d+)p\d+$/) |
There was a problem hiding this comment.
Normalize findmnt source before lsblk media lookup
deviceBaseName assumes findmnt sources are plain /dev/... nodes, but Linux commonly reports values like /dev/nvme0n1p2[/@] for btrfs subvolumes. Because this suffix is never stripped, findLsblkDevice misses the backing block device, mediaType stays Unknown, and the server-side HDD guard in runTrimForDrive (which only blocks mediaType === 'HDD') can be bypassed for HDD-backed mounts on those systems. This causes drives that should be marked non-applicable to remain selectable/runnable.
Useful? React with 👍 / 👎.
Codex flagged that findmnt reports btrfs subvolumes and bind mounts with a '[/subvol]' suffix on the source field (e.g. '/dev/nvme0n1p2[/@]'). Without stripping it, the lsblk lookup misses the backing block device, mediaType stays 'Unknown', and an HDD-backed btrfs subvolume slips past the HDD safety guard. Fix: strip the trailing '[...]' before extracting the partition base. 4 new tests covering SATA, NVMe, btrfs subvolumes, and device-mapper paths (1992/1992 passing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6018c9fa25
ℹ️ 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".
| // the lookup hits the backing block device and mediaType is detected correctly. | ||
| const stripped = devPath.replace(/\[[^\]]*\]$/, '') | ||
| const name = stripped.replace(/^\/dev\//, '') | ||
| if (name.startsWith('mapper/')) return name |
There was a problem hiding this comment.
Normalize /dev/mapper names before lsblk lookup
Even after the subvolume-suffix fix, mapped Linux sources are still mis-normalized: deviceBaseName() returns mapper/<name>, but lsblk -o NAME entries are plain device names (e.g. cryptroot/dm-0) and never include mapper/, so findLsblkDevice() misses these mounts. In that case mediaType remains Unknown, and because runTrimForDrive() only blocks mediaType === 'HDD', HDD-backed LUKS/LVM mounts can become runnable instead of being rejected by the server-side safety rail.
Useful? React with 👍 / 👎.
Summary
Disk Tools(sidebar) — lists every fixed volume with media type, last-TRIM time, and a status pill explaining why TRIM is recommended (or not).Optimize-Volume -ReTrim, Linux runsfstrim -v, macOS shows drives asManaged by macOS(read-only — Apple SSDs auto-TRIM and we never calltrimforce).Safety rails (all server-side, defense in depth)
trim-history.json— prevents wasted writes.needsAdmin: truewithout spawning anything.runTrimForDrivereturns immediately on darwin. Test assertstrimforceis never spawned.^[A-Za-z]$) and id length cap on all inputs.Status heuristic (the "why" reasoning)
statusReasonis shown as a tooltip on each pill.Files
New:
src/main/ipc/disk-trim.ipc.ts— IPC handler with per-platform enumeration + executionsrc/main/ipc/disk-trim.ipc.test.ts— 13 unit testssrc/main/services/trim-history-store.ts— JSON store for per-drive last-TRIM timestampssrc/main/services/trim-history-store.test.ts— 7 unit testssrc/renderer/src/pages/DiskMaintenancePage.tsx— Page UIsrc/renderer/src/stores/disk-maintenance-store.ts— Zustand store + helpersModified:
src/shared/types.ts— addedTrimDriveInfo,TrimRunResult,TrimProgresssrc/shared/channels.ts— addedDISK_TRIM_LIST/RUN/PROGRESSsrc/main/ipc/index.ts— register handlersrc/preload/index.ts— exposediskTrimList/Run/onDiskTrimProgressonwindow.kudusrc/renderer/src/App.tsx—/disk-maintenanceroute + page titlesrc/renderer/src/components/layout/Sidebar.tsx— sidebar entry under Disk Toolssrc/renderer/src/locales/en/disk.json— i18n stringsTest plan
npm test— 1983/1983 passing (20 new)npx tsc --noEmit— no new TS errorsnpm run validate:rules— passesOptimize-VolumeinvocationneedsAdmintoastbytes were trimmed, history saved