New tool: Drive Ejector - #13
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Drive Ejector” menu-bar helper to the MacTools suite, backed by a safety-first DriveEjectorKit that aims to make internal and boot/system volumes impossible to eject while providing a live, user-friendly external-volume eject UI.
Changes:
- Introduces
DriveEjectorKit(eligibility predicate + pre-eject re-check) and a comprehensive synthetic test suite covering boot/internal/system-volume-group edge cases. - Adds the Drive Ejector helper app (status item + popover host) and SwiftUI UI for per-volume eject + Eject All with optional confirmation.
- Wires the new tool into the toolbox catalog, defaults, packaging, README, and changelog.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/DMonteCoreTests/DriveEjectorKitTests.swift | Adds extensive unit tests for ejection eligibility, re-check behavior, and related helpers. |
| Sources/DMonteDriveEjectorApp/main.swift | Helper entrypoint with single-instance guard and “--open” activation notification. |
| Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift | Sets up status item + popover hosting and syncs tray icon with mounted/ejectable volumes. |
| Sources/DMonteCore/ToolboxCatalog.swift | Registers the new Drive Ejector tool in the toolbox catalog. |
| Sources/DMonteCore/DriveEjectorView.swift | Implements the Drive Ejector SwiftUI popover UI and settings overlay. |
| Sources/DMonteCore/DriveEjectorSizing.swift | Adds sizing/scaling logic for the Drive Ejector panel. |
| Sources/DMonteCore/DriveEjectorKit.swift | Adds the core safety rule, volume descriptors, enumeration, and eject implementation. |
| Sources/DMonteCore/DriveEjectorController.swift | Adds the observable controller managing live volume list, busy state, and eject actions. |
| Sources/DMonteCore/AppPreferences.swift | Registers the default for Eject All confirmation. |
| Scripts/package_app.sh | Includes the new helper in the packaging list. |
| README.md | Documents the new tool in the feature table. |
| Packaging/DriveEjectorInfo.plist | Adds the helper app’s Info.plist for packaging/signing. |
| Package.swift | Adds the new executable target and product to SwiftPM. |
| CHANGELOG.md | Notes the Drive Ejector addition under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
havokentity
added a commit
that referenced
this pull request
Jul 18, 2026
1. VolumeDescriptor.id used url.path while the unmount-announcement markers
were keyed on url.standardizedFileURL.path, so a mount point spelled
non-canonically (a "..", a trailing slash, a stray ".") produced two
identities for one volume and the busy marker matched nothing — re-enabling
a row while its unmount was already in flight. On a tool that unmounts
disks that is a real hazard, so identity is now canonical everywhere:
- added DriveEjectorKit.volumeIdentity(for:) as the single definition;
- VolumeDescriptor.init standardises the stored mount-point URL, so id,
the safety rule's path comparison and the URL handed to
unmountAndEjectDevice are all the same string;
- verdict(for:) and the controller's willUnmount marker both route
through volumeIdentity(for:);
- eject(volumeAt:) now unmounts the descriptor's canonical URL rather
than the caller's spelling, so the volume that was cleared is the
volume that gets unmounted.
Re-verified the safety predicate: boot volume, internal disks and every
/System/Volumes group member stay refused, including through
non-canonical paths. Added tests for canonical identity, canonical
descriptor URL, busy-marker collision across spellings, and system-volume
paths reached via "." / ".." / trailing slash.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lists mounted external volumes with their Finder icon, capacity and free space, and ejects them one at a time or all at once. The list updates live from NSWorkspace's mount/unmount notifications, so it never shows a disk that is already gone or hides one just plugged in. Safety is the whole point of the tool, so the eligibility rule lives in a pure DriveEjectorKit and is exercised directly by tests. A volume is offered only when it is demonstrably not the startup disk, not internal, and positively removable or ejectable. The three classification flags are kept as tri-state optionals rather than collapsed to Bool at read time: a filesystem that declines to vend them yields "undetermined", which is excluded. Guessing costs the user a disk; refusing costs a click. The startup disk is blocked by two independent signals — a mount point of "/" and volumeIsRootFileSystemKey — because a Mac booted from an external USB disk reports its startup volume as external, removable and ejectable, which is exactly the combination a naive rule waves through. The listed snapshot is a hint, never authorisation: eject() re-reads the resource values and re-derives the verdict immediately before unmounting, on every volume, including inside Eject All. Between the user seeing a row and pressing it, a disk image can detach and its mount point be reused. Ejection fails often and for good reason, so failures are surfaced verbatim on the inline status line, combining the error description with the recovery suggestion where macOS names the app holding the volume. Nothing force-unmounts and nothing retries: a refusal leaves the volume mounted and the user told why. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adversarial review of the new Drive Ejector tool found four defects, all in the new code. The system volume group could clear the safety rule. On a Mac booted from external media, /System/Volumes/VM, Preboot and Update report isInternal false plus removable and ejectable, and only Data carries the root-filesystem flag — so the siblings passed verdict(for:) outright. The enumeration kept them off the list only via .skipHiddenVolumes, which is an option on one call site, not a property of the rule: eject(volumeAt:) re-derives its verdict from a bare mount point and never went near it. The rule now rejects the system volume group itself, so both paths inherit the same guarantee. The Eject All confirmation shipped switched off. The app delegate built its controller in a stored property initialiser, which runs before applicationDidFinishLaunching calls AppDefaults.registerDefaults(), so the registered default of true was never visible and the read returned false. Made the controller lazy, matching the Focus Timer delegate and its comment. A denied unmount disabled a row permanently. willUnmountNotification is an announcement, not a promise — a dissenting app can veto the unmount and no didUnmount ever follows — but the busy marker was only ever pruned by intersecting with the live mount points, and a vetoed volume is still mounted. Busy state is now the union of this tool's own ejects and foreign unmount announcements, the latter expiring after a grace period. An armed Eject All could act on a list the user never saw. The arming now drops when the volume set changes underneath it, and when the panel reopens. Tests: 451 -> 456, covering the external-boot system volume group, the prefix rule not swallowing user disks named like system paths, and announcement expiry. Not fixed, needs one manual test before merge: unmountAndEjectDevice(at:) is still called off the main actor, which could not be exercised headlessly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1. VolumeDescriptor.id used url.path while the unmount-announcement markers
were keyed on url.standardizedFileURL.path, so a mount point spelled
non-canonically (a "..", a trailing slash, a stray ".") produced two
identities for one volume and the busy marker matched nothing — re-enabling
a row while its unmount was already in flight. On a tool that unmounts
disks that is a real hazard, so identity is now canonical everywhere:
- added DriveEjectorKit.volumeIdentity(for:) as the single definition;
- VolumeDescriptor.init standardises the stored mount-point URL, so id,
the safety rule's path comparison and the URL handed to
unmountAndEjectDevice are all the same string;
- verdict(for:) and the controller's willUnmount marker both route
through volumeIdentity(for:);
- eject(volumeAt:) now unmounts the descriptor's canonical URL rather
than the caller's spelling, so the volume that was cleared is the
volume that gets unmounted.
Re-verified the safety predicate: boot volume, internal disks and every
/System/Volumes group member stay refused, including through
non-canonical paths. Added tests for canonical identity, canonical
descriptor URL, busy-marker collision across spellings, and system-volume
paths reached via "." / ".." / trailing slash.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
havokentity
force-pushed
the
feat/drive-ejector
branch
from
July 18, 2026 10:59
741dd58 to
db6a9d8
Compare
The tool listed nothing at all on a normal Mac. `verdict(for:)` required `isRemovable || isEjectable`, but in DiskArbitration terms "removable" means removable *media* — an SD card, an optical disc — not a drive you can unplug. Measured on a Mac Studio, a USB SSD and a PCIe enclosure both report removable=false AND ejectable=false, so every real external drive fell into `.notRemovable`. The volumes that do report true/true turn out to be iOS Simulator runtime images, which must never be offered. `isInternal == false` is the only flag that actually tracks "this disk is external", so that is now the whole rule. The flags stay on the descriptor as honest display signals but no longer decide eligibility. Safety is unchanged. The startup disk is still caught first by the root-filesystem flag and the mount path, the system volume group is still excluded by its `/System/Volumes` prefix, and a volume with no `isInternal` is still refused — which is what keeps the simulator images out. Network mounts were the legitimate concern hiding behind the old flag check, so they are now excluded by the key that actually identifies them: a volume reporting `isLocal == false` is refused as `.networkVolume`. The old `.notRemovable` case is gone; nothing could reach it. The tests missed all of this because their fixtures encoded the assumption rather than reality — the "network share" fixture was byte-identical to a real USB SSD. They now use values measured from actual hardware, and assert both directions: drives reporting false/false are offered, simulator images reporting true/true are not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sheet is centred over the panel, and its size was a literal 340x330 while the panel's is scaled. On any Mac whose menu bar is thinner than 26pt the panel shrinks and the sheet does not, so it overflows equally on both sides and the clipping eats the first and last characters — "Drive Ejector Settings" loses its outer glyphs and the trailing switch runs off the edge. Measured here: menu bar 22pt gives scale 0.846, a 288x381 panel against a 340pt sheet, 52pt too wide. `settingsSize()` scales like everything else and is capped at the panel width, so the sheet cannot exceed it at any scale; it now measures 288x279 against the 288x381 panel. Putting it in the Sizing enum rather than inline in the view also makes it testable, which a literal buried in a SwiftUI modifier was not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The first version of this fix scaled the sheet by `currentScale` alongside the panel. That is right where the sheet is as wide as its panel, but wrong where it is narrower: Scratchpad's 320pt sheet sits in a 380pt panel, so scaling shrank it to 271 inside a 322 panel — 51pt of inset added to a tool that was never clipped. The sheet's contents are laid out with unscaled padding, so squeezing the frame risks clipping inside the sheet rather than outside it. The requirement is only that the sheet never exceeds the panel, so cap it: `min(designSize, panelSize)`. Identical for the four tools whose sheet matches or exceeds its panel, and strictly better for the two where it does not — Scratchpad keeps its designed 320 and Network Info gains 23pt back. The tests that broke were the ones restating the arithmetic; the ones asserting "the sheet fits the panel" passed through the change untouched. Replaced the former with the actual contract: the sheet uses its design size unless the panel is smaller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The earlier cap-at-panel-width fix was incomplete. PreferencesOverlay wraps the sheet in 18pt of padding on every side, so a sheet sized to the full panel becomes panel+36 once padded and overflows. That over-wide overlay layer then dragged the panel content behind it off both edges — the header's title and gear spilled past the window and the footer pushed below it — reproduced and fixed by eye on the real NSHostingController path. settingsSize now caps at panel minus 36 (2×18), so the padded sheet fits the panel exactly and the content behind it stays put. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # Package.swift # Scripts/package_app.sh # Sources/DMonteCore/ToolboxCatalog.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New menu-bar tool for ejecting external volumes, with capacity/free space per volume, one-click eject, Eject All, and a live list that updates on mount/unmount.
Safety — the whole point of the tool
Internal and boot volumes must be impossible to eject. The eligibility predicate lives in a pure, heavily-tested
DriveEjectorKit, and ejectability is re-checked immediately before the eject, not just when the list was built. If a volume's ejectability cannot be determined, it is excluded.The adversarial review found and closed a real hole here. On a Mac booted from external media,
/System/Volumes/VM,/System/Volumes/Prebootand/System/Volumes/UpdatereportisInternal=falseplus removable and ejectable — and only/System/Volumes/DatacarriesvolumeIsRootFileSystemKey. Those system siblings therefore passed the original check. The predicate now recognises the system volume group as a unit.The safety rule is the most heavily tested thing in this PR: synthetic volume descriptors (internal, boot, system-group sibling, removable, ejectable, indeterminate) assert exactly which are offered.
Behaviour
Formatters.compactBytesStringrather than hand-rolled formatting.Tests: 428 → 456, all passing.
🤖 Generated with Claude Code