From 550458ed1f6102cbd2946f6d28e4d8653a46e563 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Sat, 18 Jul 2026 13:47:19 +0530 Subject: [PATCH 1/7] Drive Ejector: add a new menu-bar tool for ejecting external volumes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 8 + Package.swift | 11 + Packaging/DriveEjectorInfo.plist | 32 ++ README.md | 1 + Scripts/package_app.sh | 1 + Sources/DMonteCore/AppPreferences.swift | 3 +- .../DMonteCore/DriveEjectorController.swift | 249 ++++++++++++ Sources/DMonteCore/DriveEjectorKit.swift | 258 ++++++++++++ Sources/DMonteCore/DriveEjectorSizing.swift | 15 + Sources/DMonteCore/DriveEjectorView.swift | 368 ++++++++++++++++++ Sources/DMonteCore/ToolboxCatalog.swift | 3 +- .../DriveEjectorAppDelegate.swift | 98 +++++ Sources/DMonteDriveEjectorApp/main.swift | 24 ++ .../DriveEjectorKitTests.swift | 331 ++++++++++++++++ 14 files changed, 1400 insertions(+), 2 deletions(-) create mode 100644 Packaging/DriveEjectorInfo.plist create mode 100644 Sources/DMonteCore/DriveEjectorController.swift create mode 100644 Sources/DMonteCore/DriveEjectorKit.swift create mode 100644 Sources/DMonteCore/DriveEjectorSizing.swift create mode 100644 Sources/DMonteCore/DriveEjectorView.swift create mode 100644 Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift create mode 100644 Sources/DMonteDriveEjectorApp/main.swift create mode 100644 Tests/DMonteCoreTests/DriveEjectorKitTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f5cf2f..f84f49d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ adheres to [Semantic Versioning](https://semver.org) and the and runs the full test suite on a pinned macOS runner, using the same toolchain the release workflow ships with. Previously only version tags ran CI, so a branch could go unverified until release day. +- **Drive Ejector**: a new menu bar tool that lists mounted external volumes + with their capacity and free space and ejects them individually or all at + once. Only removable, non-internal volumes are ever listed, and every volume + is re-checked against the mount table the moment before it is ejected, so the + startup disk and internal drives cannot be unmounted even if a listed row goes + stale. Nothing is ever force-unmounted: when a file is open or Spotlight is + still indexing, the disk stays mounted and macOS's own reason is shown inline. + The list updates live as disks are plugged in and removed. ## [0.13.0] — 2026-07-12 diff --git a/Package.swift b/Package.swift index e40120a..ada74e0 100644 --- a/Package.swift +++ b/Package.swift @@ -95,6 +95,10 @@ let package = Package( .executable( name: "DMonteWindowManager", targets: ["DMonteWindowManager"] + ), + .executable( + name: "DMonteDriveEjector", + targets: ["DMonteDriveEjector"] ) ], dependencies: [ @@ -253,6 +257,13 @@ let package = Package( ], path: "Sources/DMonteWindowManagerApp" ), + .executableTarget( + name: "DMonteDriveEjector", + dependencies: [ + "DMonteCore" + ], + path: "Sources/DMonteDriveEjectorApp" + ), .testTarget( name: "DMonteCoreTests", dependencies: ["DMonteCore"], diff --git a/Packaging/DriveEjectorInfo.plist b/Packaging/DriveEjectorInfo.plist new file mode 100644 index 0000000..7260e07 --- /dev/null +++ b/Packaging/DriveEjectorInfo.plist @@ -0,0 +1,32 @@ + + + + + CFBundleDevelopmentRegion + en + CFBundleExecutable + DMonteDriveEjector + CFBundleIdentifier + com.havokentity.mactools.driveejector + CFBundleInfoDictionaryVersion + 6.0 + CFBundleDisplayName + DMonte Drive Ejector + CFBundleName + DMonte Drive Ejector + CFBundlePackageType + APPL + CFBundleShortVersionString + 0.13.0 + CFBundleVersion + 1 + LSMinimumSystemVersion + 14.0 + LSMultipleInstancesProhibited + + LSUIElement + + NSHumanReadableCopyright + Copyright © 2026 Yahushad Monte + + diff --git a/README.md b/README.md index 925c765..a8dca1d 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ The app updates itself automatically via [Sparkle](https://sparkle-project.org); | **Audio Router** | Virtual audio routing — install independent loopback cables for app‑to‑app audio, build mirror/aggregate devices with presets, and "listen" to any input through an output | | **Focus Timer** | Pomodoro timer with a live menu bar countdown | | **Calendar** | Menu bar month view with your upcoming events | +| **Drive Ejector** | Eject external volumes one at a time or all at once — internal and startup disks are never listed | | **Keep Awake** | Prevent sleep, optionally for a set duration | | **Maintenance** | Handy Finder/system toggles and cache/index refreshes | | **Dev Tools** | JSON, Base64, URL, hashing, UUID, timestamp, and case utilities | diff --git a/Scripts/package_app.sh b/Scripts/package_app.sh index 9b0f96b..a348aaa 100755 --- a/Scripts/package_app.sh +++ b/Scripts/package_app.sh @@ -36,6 +36,7 @@ HELPERS=( "DMonteGrabText|DMonte Grab Text.app|GrabTextInfo.plist" "DMonteFocusTimer|DMonte Focus Timer.app|FocusTimerInfo.plist" "DMonteWindowManager|DMonte Window Manager.app|WindowManagerInfo.plist" + "DMonteDriveEjector|DMonte Drive Ejector.app|DriveEjectorInfo.plist" ) stamp_version() { diff --git a/Sources/DMonteCore/AppPreferences.swift b/Sources/DMonteCore/AppPreferences.swift index b746647..c7e379e 100644 --- a/Sources/DMonteCore/AppPreferences.swift +++ b/Sources/DMonteCore/AppPreferences.swift @@ -114,7 +114,8 @@ public enum AppDefaults { DefaultsKey.focusTimerFocusMinutes: 25, DefaultsKey.focusTimerShortBreakMinutes: 5, DefaultsKey.focusTimerLongBreakMinutes: 15, - DefaultsKey.focusTimerLongBreakInterval: 4 + DefaultsKey.focusTimerLongBreakInterval: 4, + DefaultsKey.driveEjectorConfirmsEjectAll: true ]) } } diff --git a/Sources/DMonteCore/DriveEjectorController.swift b/Sources/DMonteCore/DriveEjectorController.swift new file mode 100644 index 0000000..e18ea32 --- /dev/null +++ b/Sources/DMonteCore/DriveEjectorController.swift @@ -0,0 +1,249 @@ +import AppKit +import Foundation + +public extension DefaultsKey { + /// Whether Eject All asks for a second press before acting. Ejecting every external disk at + /// once is the one action here that is both irreversible-in-the-moment and easy to hit by + /// accident, so the integrator registers a default of `true` for this key. + static let driveEjectorConfirmsEjectAll = "tool.driveEjector.confirmsEjectAll" +} + +/// One volume queued for ejection, flattened out of the `@MainActor` model so the work can hop +/// to a background task without carrying main-actor state across the boundary. +private struct EjectRequest: Sendable { + let id: String + let name: String + let url: URL +} + +/// Owns the list of ejectable volumes and the ejection attempts against them. +/// +/// It is an `NSObject` because it observes `NSWorkspace`'s mount notifications by selector, the +/// same pattern `ActivationTracker` uses: workspace notifications are posted on the main thread, +/// so a `@MainActor` selector target receives them already on the right actor. +/// +/// The controller never decides what is safe to eject — `DriveEjectorKit` does, both when +/// building the list and again inside every `eject` call. +@MainActor +public final class DriveEjectorController: NSObject, ObservableObject { + /// The volumes currently offered. Only ever assigned from `DriveEjectorKit.ejectableVolumes()`. + @Published public private(set) var volumes: [VolumeDescriptor] = [] + + /// The inline status line: progress while ejecting, then the honest result. + @Published public private(set) var statusMessage: String? + + /// Mount points with an eject in flight, so their rows can show progress and refuse a + /// second press instead of racing a duplicate unmount. + @Published public private(set) var busyVolumeIDs: Set = [] + + /// `true` once Eject All has been requested and is waiting for the confirming press. + @Published public private(set) var isConfirmingEjectAll = false + + /// Persisted preference: require a second press for Eject All. + @Published public private(set) var confirmsEjectAll: Bool + + public override init() { + confirmsEjectAll = AppDefaults.shared.bool(forKey: DefaultsKey.driveEjectorConfirmsEjectAll) + super.init() + refresh() + } + + // No `deinit` cleanup: a nonisolated `deinit` may not touch the main actor under Swift 6, and + // removing a selector-based workspace observer requires it. The app delegate calls + // `stopObserving()` on termination instead. + + // MARK: - Public API + + /// Re-reads the mount table. Cheap enough to call on every mount notification. + public func refresh() { + volumes = DriveEjectorKit.ejectableVolumes() + + // Drop busy markers for volumes that are gone; otherwise a successful eject would leave + // a phantom entry that suppresses a later eject of a disk remounted at the same point. + let liveIDs = Set(volumes.map(\.id)) + busyVolumeIDs.formIntersection(liveIDs) + + if volumes.isEmpty { + isConfirmingEjectAll = false + } + } + + /// Begins observing mount/unmount activity so the list stays truthful without polling. + public func startObserving() { + let center = NSWorkspace.shared.notificationCenter + center.addObserver( + self, + selector: #selector(volumeWillUnmount(_:)), + name: NSWorkspace.willUnmountNotification, + object: nil + ) + center.addObserver( + self, + selector: #selector(volumeDidUnmount(_:)), + name: NSWorkspace.didUnmountNotification, + object: nil + ) + center.addObserver( + self, + selector: #selector(volumeDidMount(_:)), + name: NSWorkspace.didMountNotification, + object: nil + ) + } + + /// Stops observing workspace mount activity (for app-termination cleanup). + public func stopObserving() { + NSWorkspace.shared.notificationCenter.removeObserver(self) + } + + /// Ejects one volume. The verdict is re-derived from fresh resource values inside + /// `DriveEjectorKit.eject(volumeAt:)`, so a stale row cannot authorise anything. + public func eject(_ volume: VolumeDescriptor) { + guard !busyVolumeIDs.contains(volume.id) else { + return + } + + busyVolumeIDs.insert(volume.id) + statusMessage = "Ejecting “\(volume.name)”…" + + let request = EjectRequest(id: volume.id, name: volume.name, url: volume.url) + Task.detached(priority: .userInitiated) { + let outcome = DriveEjectorKit.eject(volumeAt: request.url) + await MainActor.run { [weak self] in + self?.finish(request, outcome: outcome) + } + } + } + + /// Asks to eject every listed volume. With confirmation enabled the first call only arms the + /// action; the second (`confirmEjectAll()`) performs it. + public func requestEjectAll() { + guard !volumes.isEmpty else { + return + } + + if confirmsEjectAll, !isConfirmingEjectAll { + isConfirmingEjectAll = true + return + } + + confirmEjectAll() + } + + /// Performs the batch eject. Every volume goes through the same per-volume safety re-check as + /// a single eject — “all” describes the selection, never a relaxation of the rule. + public func confirmEjectAll() { + isConfirmingEjectAll = false + + let targets = volumes + .filter { !busyVolumeIDs.contains($0.id) } + .map { EjectRequest(id: $0.id, name: $0.name, url: $0.url) } + guard !targets.isEmpty else { + return + } + + targets.forEach { busyVolumeIDs.insert($0.id) } + statusMessage = "Ejecting \(targets.count) volume\(targets.count == 1 ? "" : "s")…" + + Task.detached(priority: .userInitiated) { + // Sequential on purpose: run in parallel, macOS stacks up its own “disk in use” + // panels and the user cannot tell which volume each one belongs to. + var results: [(EjectRequest, EjectionOutcome)] = [] + for target in targets { + results.append((target, DriveEjectorKit.eject(volumeAt: target.url))) + } + + await MainActor.run { [weak self] in + self?.finishBatch(results) + } + } + } + + /// Abandons an armed Eject All. + public func cancelEjectAll() { + isConfirmingEjectAll = false + } + + /// Updates the Eject All confirmation preference, disarming any request already in flight so + /// the pending state cannot outlive the setting that created it. + public func setConfirmsEjectAll(_ newValue: Bool) { + guard newValue != confirmsEjectAll else { + return + } + + confirmsEjectAll = newValue + AppDefaults.shared.set(newValue, forKey: DefaultsKey.driveEjectorConfirmsEjectAll) + isConfirmingEjectAll = false + } + + /// Clears the status line, e.g. when the popover is reopened. + public func clearStatus() { + statusMessage = nil + } + + // MARK: - Workspace notifications + + /// A volume is about to go away — mark it busy so its row reads as in-progress rather than + /// inviting a press that would race the unmount already under way (which may be Finder's). + @objc private func volumeWillUnmount(_ notification: Notification) { + guard let url = notification.volumeURL else { + return + } + busyVolumeIDs.insert(url.path) + } + + @objc private func volumeDidUnmount(_ notification: Notification) { + refresh() + } + + @objc private func volumeDidMount(_ notification: Notification) { + refresh() + } + + // MARK: - Private + + private func finish(_ request: EjectRequest, outcome: EjectionOutcome) { + busyVolumeIDs.remove(request.id) + statusMessage = message(for: outcome, volumeName: request.name) + refresh() + } + + private func finishBatch(_ results: [(EjectRequest, EjectionOutcome)]) { + results.forEach { busyVolumeIDs.remove($0.0.id) } + + let failures = results.filter { $0.1 != .ejected } + + if failures.isEmpty { + let count = results.count + statusMessage = "Ejected \(count) volume\(count == 1 ? "" : "s")" + } else if let (request, outcome) = failures.first, failures.count == 1 { + // One holdout: name it and say exactly why, which is far more actionable than a tally. + statusMessage = message(for: outcome, volumeName: request.name) + } else { + let ejected = results.count - failures.count + let names = failures.map { "“\($0.0.name)”" }.joined(separator: ", ") + statusMessage = "Ejected \(ejected) of \(results.count) — \(names) still mounted" + } + + refresh() + } + + private func message(for outcome: EjectionOutcome, volumeName: String) -> String { + switch outcome { + case .ejected: + "Ejected “\(volumeName)”" + case .blocked(let verdict): + // A blocked eject is a safety refusal, not a failure of the disk — word it as such. + "Didn’t eject “\(volumeName)” — \(verdict.blockedReason ?? "it’s no longer eligible")" + case .failed(let reason): + "Couldn’t eject “\(volumeName)” — \(reason)" + } + } +} + +private extension Notification { + /// The mount point carried by an `NSWorkspace` mount/unmount notification. + var volumeURL: URL? { + userInfo?[NSWorkspace.volumeURLUserInfoKey] as? URL + } +} diff --git a/Sources/DMonteCore/DriveEjectorKit.swift b/Sources/DMonteCore/DriveEjectorKit.swift new file mode 100644 index 0000000..e302004 --- /dev/null +++ b/Sources/DMonteCore/DriveEjectorKit.swift @@ -0,0 +1,258 @@ +import AppKit +import Foundation + +/// A snapshot of the volume resource keys the eject-safety rule depends on. +/// +/// Every flag is a tri-state `Bool?` on purpose. `URLResourceValues` returns `nil` for any key +/// the filesystem declines to vend, and “the system would not tell us” is a materially different +/// answer from “no” — collapsing the two at read time would let an unclassifiable volume inherit +/// the permissive branch. `DriveEjectorKit.verdict(for:)` keeps them apart and refuses anything +/// it cannot positively clear. +public struct VolumeDescriptor: Identifiable, Sendable, Equatable { + /// Mount point. Doubles as the identity: only one volume can occupy a mount point at a time. + public let url: URL + /// Display name (`volumeNameKey`), falling back to the mount point's last path component. + public let name: String + /// `true` when this is the volume the running system booted from. + public let isRootFileSystem: Bool? + /// `true` for disks physically inside the Mac (including the sealed system and Data volumes). + public let isInternal: Bool? + /// `true` for media that can be removed from its drive. + public let isRemovable: Bool? + /// `true` when the volume can be unmounted and ejected by the user. + public let isEjectable: Bool? + /// Total capacity in bytes; `0` when unavailable. + public let totalBytes: UInt64 + /// Free capacity in bytes; `0` when unavailable. + public let freeBytes: UInt64 + + public var id: String { url.path } + + /// Bytes in use, clamped at zero so a stale free-space reading can never produce an underflow. + public var usedBytes: UInt64 { + totalBytes > freeBytes ? totalBytes - freeBytes : 0 + } + + public init( + url: URL, + name: String, + isRootFileSystem: Bool?, + isInternal: Bool?, + isRemovable: Bool?, + isEjectable: Bool?, + totalBytes: UInt64, + freeBytes: UInt64 + ) { + self.url = url + self.name = name + self.isRootFileSystem = isRootFileSystem + self.isInternal = isInternal + self.isRemovable = isRemovable + self.isEjectable = isEjectable + self.totalBytes = totalBytes + self.freeBytes = freeBytes + } +} + +/// Why a volume may or may not be offered for ejection. +/// +/// A separate reason (rather than a bare `Bool`) exists so the re-check performed immediately +/// before unmounting can tell the user *why* a row that looked ejectable a moment ago is not. +public enum VolumeEjectionVerdict: Sendable, Equatable { + /// External, removable, and demonstrably not the startup disk. + case eligible + /// The volume the system booted from. Never ejectable, regardless of its other flags. + case bootVolume + /// A disk inside the Mac (including the sealed system and Data volumes). + case internalDisk + /// External but fixed — nothing about it says it can be detached. + case notRemovable + /// The filesystem did not vend the flags needed to clear it. Excluded by default. + case undetermined + + /// Plain-language explanation for the inline status line; `nil` when the volume is eligible. + public var blockedReason: String? { + switch self { + case .eligible: nil + case .bootVolume: "it’s the startup disk" + case .internalDisk: "it’s an internal disk" + case .notRemovable: "it isn’t a removable volume" + case .undetermined: "macOS wouldn’t confirm it’s safe to eject" + } + } +} + +/// What happened when an eject was attempted. +public enum EjectionOutcome: Sendable, Equatable { + /// The volume was unmounted and the device ejected. + case ejected + /// The safety re-check refused. The volume is untouched and still mounted. + case blocked(VolumeEjectionVerdict) + /// macOS refused the unmount — almost always because something still holds the volume. + /// The volume is still mounted. + case failed(String) +} + +/// The eject-safety layer for the Drive Ejector tool. +/// +/// The entire point of this tool is that it can only ever act on external, removable media, so +/// the eligibility rule lives here as a pure function over `VolumeDescriptor` and is exercised +/// directly by the test target. The I/O entry points (`mountedVolumeDescriptors()`, +/// `eject(volumeAt:)`) are thin: they gather resource values, then defer to the same rule. +public enum DriveEjectorKit { + + // MARK: - The safety rule + + /// Resource keys the tool reads for every mounted volume. Kept in one place so the list + /// enumeration and the pre-eject re-check can never diverge on what they inspect. + public static let resourceKeys: [URLResourceKey] = [ + .volumeNameKey, + .volumeIsRootFileSystemKey, + .volumeIsInternalKey, + .volumeIsRemovableKey, + .volumeIsEjectableKey, + .volumeTotalCapacityKey, + .volumeAvailableCapacityKey + ] + + /// Decides whether a volume may be ejected. Deliberately biased towards refusing. + /// + /// Order matters: the catastrophic case is checked first and by two independent signals, so + /// a filesystem that vends nothing useful still cannot get the startup disk onto the list. + public static func verdict(for descriptor: VolumeDescriptor) -> VolumeEjectionVerdict { + // A mount point of "/" is a fact no missing resource key can hide, and the root-filesystem + // flag catches the same volume when it is reached through a symlinked or relative URL. + // Checked before the removable flags because a Mac booted from an external USB disk reports + // that disk as removable *and* ejectable — exactly the combination that would otherwise + // wave the startup disk straight through. + if descriptor.url.standardizedFileURL.path == "/" || descriptor.isRootFileSystem == true { + return .bootVolume + } + + // All three classification flags must be present. A volume nobody will classify is one + // this tool declines to touch: guessing costs the user a disk, refusing costs a click. + guard let isInternal = descriptor.isInternal, + let isRemovable = descriptor.isRemovable, + let isEjectable = descriptor.isEjectable else { + return .undetermined + } + + if isInternal { + return .internalDisk + } + + // Either flag alone is enough: USB sticks and card media report removable, while disk + // images and most external SSDs report only ejectable. + guard isRemovable || isEjectable else { + return .notRemovable + } + + return .eligible + } + + /// Convenience predicate over `verdict(for:)` for call sites that only need yes/no. + public static func isEligibleForEjection(_ descriptor: VolumeDescriptor) -> Bool { + verdict(for: descriptor) == .eligible + } + + /// The subset of `descriptors` this tool is willing to offer, sorted for stable display. + public static func eligibleVolumes(from descriptors: [VolumeDescriptor]) -> [VolumeDescriptor] { + descriptors + .filter(isEligibleForEjection) + .sorted { $0.name.localizedCaseInsensitiveCompare($1.name) == .orderedAscending } + } + + // MARK: - Enumeration + + /// Every mounted volume the tool is willing to offer, already filtered and sorted. + public static func ejectableVolumes() -> [VolumeDescriptor] { + eligibleVolumes(from: mountedVolumeDescriptors()) + } + + /// Reads a descriptor for every mounted volume. Unreadable volumes are dropped rather than + /// synthesised with default flags, so a read failure can never look like “external and safe”. + public static func mountedVolumeDescriptors() -> [VolumeDescriptor] { + let urls = FileManager.default.mountedVolumeURLs( + includingResourceValuesForKeys: resourceKeys, + options: [.skipHiddenVolumes] + ) ?? [] + + return urls.compactMap(descriptor(forVolumeAt:)) + } + + /// Reads the safety-relevant resource values for one mount point, or `nil` if the volume + /// cannot be inspected at all (it vanished, or the path is not a mount point). + public static func descriptor(forVolumeAt url: URL) -> VolumeDescriptor? { + guard let values = try? url.resourceValues(forKeys: Set(resourceKeys)) else { + return nil + } + + // `volumeAvailableCapacityKey` rather than the “important usage” variant: this is a + // headline figure for an external disk, and the purgeable-space estimate that variant + // adds is a local-snapshot concept that means little on removable media. + let total = UInt64(max(0, values.volumeTotalCapacity ?? 0)) + let free = UInt64(max(0, values.volumeAvailableCapacity ?? 0)) + + return VolumeDescriptor( + url: url, + name: values.volumeName ?? url.lastPathComponent, + isRootFileSystem: values.volumeIsRootFileSystem, + isInternal: values.volumeIsInternal, + isRemovable: values.volumeIsRemovable, + isEjectable: values.volumeIsEjectable, + totalBytes: total, + freeBytes: free + ) + } + + // MARK: - Ejection + + /// Unmounts and ejects the device backing `url`, but only after re-deriving the verdict from + /// fresh resource values. + /// + /// The listed snapshot is treated as a hint, never as authorisation: between the user seeing + /// a row and pressing its button the mount table can change — a disk image detaches and its + /// mount point is reused, a volume is remounted, the machine is rebooted from that very disk. + /// This re-check is the only thing standing between a stale row and an unmounted system disk, + /// so it runs unconditionally on every single volume, including inside Eject All. + /// + /// Never force-unmounts and never retries: a refusal means something is still using the + /// volume, and the honest outcome is a mounted disk plus an explanation. + /// + /// Synchronous and blocking — an unmount can take seconds on a busy disk — so callers run it + /// off the main actor and report progress through their own state. + public static func eject(volumeAt url: URL) -> EjectionOutcome { + guard let descriptor = descriptor(forVolumeAt: url) else { + return .blocked(.undetermined) + } + + let verdict = verdict(for: descriptor) + guard verdict == .eligible else { + return .blocked(verdict) + } + + do { + try NSWorkspace.shared.unmountAndEjectDevice(at: url) + return .ejected + } catch { + return .failed(failureMessage(for: error)) + } + } + + /// Turns an unmount error into a sentence worth showing. + /// + /// macOS puts the useful part — often the name of the app holding the volume — in the + /// recovery suggestion rather than the description, so both are surfaced when they differ. + static func failureMessage(for error: Error) -> String { + let nsError = error as NSError + let description = nsError.localizedDescription.trimmingCharacters(in: .whitespacesAndNewlines) + let suggestion = (nsError.localizedRecoverySuggestion ?? "") + .trimmingCharacters(in: .whitespacesAndNewlines) + + guard !suggestion.isEmpty, suggestion != description else { + return description.isEmpty ? "macOS refused the eject" : description + } + + return description.isEmpty ? suggestion : "\(description) \(suggestion)" + } +} diff --git a/Sources/DMonteCore/DriveEjectorSizing.swift b/Sources/DMonteCore/DriveEjectorSizing.swift new file mode 100644 index 0000000..3bfb475 --- /dev/null +++ b/Sources/DMonteCore/DriveEjectorSizing.swift @@ -0,0 +1,15 @@ +import AppKit + +public enum DriveEjectorSizing { + public static func preferredSize() -> NSSize { + let scale = currentScale + return NSSize(width: (340 * scale).rounded(), height: (450 * scale).rounded()) + } + + static var currentScale: CGFloat { + let visibleFrame = NSScreen.main?.visibleFrame ?? NSRect(x: 0, y: 0, width: 1440, height: 900) + let screenScale = visibleFrame.height / 950 + let menuBarScale = NSStatusBar.system.thickness / 26 + return min(1.0, max(0.82, min(screenScale, menuBarScale))) + } +} diff --git a/Sources/DMonteCore/DriveEjectorView.swift b/Sources/DMonteCore/DriveEjectorView.swift new file mode 100644 index 0000000..7deaee3 --- /dev/null +++ b/Sources/DMonteCore/DriveEjectorView.swift @@ -0,0 +1,368 @@ +import AppKit +import SwiftUI + +/// The floating Drive Ejector popover: a live list of external volumes with capacity and free +/// space, a per-volume eject button, an Eject All action, and an inline status line that reports +/// refusals verbatim. Content is scaled to match the menu-bar/display scale so it fits the scaled +/// panel (same approach as the other tools). +public struct DriveEjectorPopoverView: View { + @ObservedObject var controller: DriveEjectorController + var onQuit: () -> Void + + @State private var isShowingSettings = false + private let scale = DriveEjectorSizing.currentScale + + public init(controller: DriveEjectorController, onQuit: @escaping () -> Void) { + self.controller = controller + self.onQuit = onQuit + } + + private func s(_ value: CGFloat) -> CGFloat { value * scale } + + private var accent: Color { .teal } + + public var body: some View { + ZStack { + VStack(spacing: 0) { + header + Divider().opacity(0.6) + + if controller.volumes.isEmpty { + emptyState + } else { + volumeList + } + + Spacer(minLength: 0) + + if let status = controller.statusMessage { + statusLine(status) + } + + ejectAllRow + footer + } + + if isShowingSettings { + PreferencesOverlay(cornerRadius: 18) { + DriveEjectorSettingsView( + controller: controller, + onQuit: onQuit, + onClose: { isShowingSettings = false } + ) + } + } + } + .frame(width: DriveEjectorSizing.preferredSize().width, height: DriveEjectorSizing.preferredSize().height) + .frostedPanel(cornerRadius: 18) + } + + // MARK: - Header + + private var header: some View { + HStack(spacing: s(8)) { + Image(systemName: "eject.fill") + .font(.system(size: s(15), weight: .semibold)) + .foregroundStyle(controller.volumes.isEmpty ? Color.secondary : accent) + + Text("Drive Ejector") + .font(.system(size: s(15), weight: .bold)) + .foregroundStyle(.primary.opacity(0.9)) + + Spacer() + + Button { + isShowingSettings = true + } label: { + Image(systemName: "gearshape.fill") + .font(.system(size: s(14), weight: .semibold)) + .foregroundStyle(.secondary) + } + .buttonStyle(.plain) + .help("Settings") + } + .padding(.horizontal, s(16)) + .padding(.top, s(14)) + .padding(.bottom, s(10)) + } + + // MARK: - Empty state + + private var emptyState: some View { + VStack(spacing: s(10)) { + Image(systemName: "externaldrive.badge.questionmark") + .font(.system(size: s(34), weight: .regular)) + .foregroundStyle(.secondary) + .symbolRenderingMode(.hierarchical) + + Text("No ejectable volumes") + .font(.system(size: s(14), weight: .semibold)) + .foregroundStyle(.secondary) + + Text("Only external, removable disks appear here. Your startup disk and internal drives are never listed.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + } + .frame(maxWidth: .infinity) + .padding(.horizontal, s(24)) + .padding(.vertical, s(34)) + } + + // MARK: - Volume list + + private var volumeList: some View { + ScrollView { + VStack(spacing: s(8)) { + ForEach(controller.volumes) { volume in + volumeRow(volume) + } + } + .padding(.horizontal, s(16)) + .padding(.vertical, s(12)) + } + } + + private func volumeRow(_ volume: VolumeDescriptor) -> some View { + let isBusy = controller.busyVolumeIDs.contains(volume.id) + + return HStack(spacing: s(10)) { + VolumeIcon(url: volume.url, size: s(30)) + + VStack(alignment: .leading, spacing: s(2)) { + Text(volume.name) + .font(.system(size: s(13), weight: .semibold)) + .lineLimit(1) + .truncationMode(.middle) + + Text(capacityLine(volume)) + .font(.system(size: s(11), weight: .medium)) + .foregroundStyle(.secondary) + .lineLimit(1) + } + + Spacer(minLength: s(6)) + + Button { + controller.eject(volume) + } label: { + Image(systemName: "eject.fill") + .font(.system(size: s(13), weight: .semibold)) + .foregroundStyle(isBusy ? Color.secondary : accent) + .frame(width: s(30), height: s(26)) + .background( + RoundedRectangle(cornerRadius: s(7), style: .continuous) + .fill((isBusy ? Color.secondary : accent).opacity(0.16)) + ) + .contentShape(RoundedRectangle(cornerRadius: s(7), style: .continuous)) + } + .buttonStyle(.plain) + .disabled(isBusy) + .help(isBusy ? "Ejecting…" : "Eject “\(volume.name)”") + } + .padding(.horizontal, s(10)) + .padding(.vertical, s(8)) + .background( + RoundedRectangle(cornerRadius: s(10), style: .continuous) + .fill(Color.secondary.opacity(isBusy ? 0.06 : 0.12)) + ) + .opacity(isBusy ? 0.6 : 1) + } + + /// "128GB free of 500GB" — or just the total when the volume declines to report capacity, + /// which beats printing a confident "0B free". + private func capacityLine(_ volume: VolumeDescriptor) -> String { + guard volume.totalBytes > 0 else { + return "Capacity unavailable" + } + return "\(volume.freeBytes.compactBytesString) free of \(volume.totalBytes.compactBytesString)" + } + + // MARK: - Status + + private func statusLine(_ status: String) -> some View { + Text(status) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.horizontal, s(16)) + .padding(.bottom, s(8)) + } + + // MARK: - Eject All + + @ViewBuilder + private var ejectAllRow: some View { + if controller.isConfirmingEjectAll { + HStack(spacing: s(8)) { + Button { + controller.confirmEjectAll() + } label: { + Text("Eject \(controller.volumes.count) Volume\(controller.volumes.count == 1 ? "" : "s")") + .font(.system(size: s(12), weight: .semibold)) + .foregroundStyle(Color.white) + .frame(maxWidth: .infinity) + .frame(height: s(32)) + .background( + RoundedRectangle(cornerRadius: s(8), style: .continuous) + .fill(accent) + ) + .contentShape(RoundedRectangle(cornerRadius: s(8), style: .continuous)) + } + .buttonStyle(.plain) + + Button { + controller.cancelEjectAll() + } label: { + Text("Cancel") + .font(.system(size: s(12), weight: .semibold)) + .foregroundStyle(.primary) + .frame(width: s(80)) + .frame(height: s(32)) + .background( + RoundedRectangle(cornerRadius: s(8), style: .continuous) + .fill(Color.secondary.opacity(0.14)) + ) + .contentShape(RoundedRectangle(cornerRadius: s(8), style: .continuous)) + } + .buttonStyle(.plain) + } + .padding(.horizontal, s(16)) + .padding(.bottom, s(8)) + } else if !controller.volumes.isEmpty { + Button { + controller.requestEjectAll() + } label: { + HStack(spacing: s(8)) { + Image(systemName: "eject.circle.fill") + .font(.system(size: s(13), weight: .semibold)) + Text("Eject All") + .font(.system(size: s(13), weight: .semibold)) + } + .foregroundStyle(.primary) + .frame(maxWidth: .infinity) + .frame(height: s(34)) + .background( + RoundedRectangle(cornerRadius: s(8), style: .continuous) + .fill(Color.secondary.opacity(0.14)) + ) + .contentShape(RoundedRectangle(cornerRadius: s(8), style: .continuous)) + } + .buttonStyle(.plain) + .padding(.horizontal, s(16)) + .padding(.bottom, s(8)) + } + } + + // MARK: - Footer + + private var footer: some View { + HStack { + Button { + controller.refresh() + } label: { + Label("Refresh", systemImage: "arrow.clockwise") + .font(.system(size: s(12), weight: .semibold)) + } + .buttonStyle(.plain) + .foregroundStyle(.secondary) + + Spacer() + + Button { + onQuit() + } label: { + Label("Quit", systemImage: "power") + .font(.system(size: s(12), weight: .semibold)) + } + .buttonStyle(.plain) + .foregroundStyle(.secondary) + } + .padding(.horizontal, s(16)) + .padding(.top, s(4)) + .padding(.bottom, s(14)) + } +} + +/// The Finder icon for a mount point, which is what users actually recognise a disk by — a +/// branded external SSD, a camera card, a mounted disk image all look different here. +private struct VolumeIcon: View { + var url: URL + var size: CGFloat + + var body: some View { + Image(nsImage: NSWorkspace.shared.icon(forFile: url.path)) + .resizable() + .frame(width: size, height: size) + } +} + +// MARK: - Settings + +private struct DriveEjectorSettingsView: View { + @ObservedObject var controller: DriveEjectorController + var onQuit: () -> Void + var onClose: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: 16) { + HStack { + Text("Drive Ejector Settings") + .font(.system(size: 16, weight: .bold)) + Spacer() + Button { + onClose() + } label: { + Image(systemName: "xmark") + .font(.system(size: 12, weight: .bold)) + .frame(width: 24, height: 24) + } + .buttonStyle(.plain) + } + + settingRow(title: "Confirm before Eject All") { + GreenSwitch(isOn: Binding( + get: { controller.confirmsEjectAll }, + set: { controller.setConfirmsEjectAll($0) } + )) + } + + Text("When on, Eject All asks for a second press before unmounting every listed volume.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + + Divider() + + Text("Only external, removable volumes are ever listed. Your startup disk and internal drives are excluded, and every volume is re-checked the moment before it is ejected. Nothing here force-unmounts: if a file is open or Spotlight is still indexing, the disk stays mounted and the reason is shown.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + + Divider() + + Button(role: .destructive) { + onClose() + onQuit() + } label: { + Label("Quit Drive Ejector", systemImage: "power") + .frame(maxWidth: .infinity, alignment: .leading) + } + + Spacer() + } + .padding(20) + .frame(width: 340, height: 330) + } + + private func settingRow(title: String, @ViewBuilder trailing: () -> Trailing) -> some View { + HStack { + Text(title) + .font(.system(size: 13, weight: .semibold)) + Spacer() + trailing() + } + } +} diff --git a/Sources/DMonteCore/ToolboxCatalog.swift b/Sources/DMonteCore/ToolboxCatalog.swift index 8d1039b..672eb40 100644 --- a/Sources/DMonteCore/ToolboxCatalog.swift +++ b/Sources/DMonteCore/ToolboxCatalog.swift @@ -64,7 +64,8 @@ public enum ToolboxCatalog { ToolboxTool(id: "colorPicker", title: "Color Picker", iconName: "eyedropper.halffull", tint: .mint, bundleID: prefix + "colorpicker", appName: "DMonte Color Picker.app", executableName: "DMonteColorPicker", arguments: ["--open"]), ToolboxTool(id: "grabText", title: "Grab Text", iconName: "text.viewfinder", tint: contrastGreen, bundleID: prefix + "grabtext", appName: "DMonte Grab Text.app", executableName: "DMonteGrabText", arguments: ["--open"]), ToolboxTool(id: "focusTimer", title: "Focus Timer", iconName: "timer", tint: .red, bundleID: prefix + "focustimer", appName: "DMonte Focus Timer.app", executableName: "DMonteFocusTimer", arguments: ["--open"]), - ToolboxTool(id: "windowManager", title: "Window Manager", iconName: "macwindow.on.rectangle", tint: .blue, bundleID: prefix + "windowmanager", appName: "DMonte Window Manager.app", executableName: "DMonteWindowManager", arguments: ["--open"]) + ToolboxTool(id: "windowManager", title: "Window Manager", iconName: "macwindow.on.rectangle", tint: .blue, bundleID: prefix + "windowmanager", appName: "DMonte Window Manager.app", executableName: "DMonteWindowManager", arguments: ["--open"]), + ToolboxTool(id: "driveEjector", title: "Drive Ejector", iconName: "eject.fill", tint: .teal, bundleID: prefix + "driveejector", appName: "DMonte Drive Ejector.app", executableName: "DMonteDriveEjector", arguments: ["--open"]) ] } diff --git a/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift b/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift new file mode 100644 index 0000000..1c5af4c --- /dev/null +++ b/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift @@ -0,0 +1,98 @@ +import AppKit +import Combine +import DMonteCore +import SwiftUI + +/// Distributed notification used to reveal this helper's popover when the Toolbox (or a second +/// launch with `--open`) asks for it. +enum DriveEjectorNotifications { + static let showWindow = Notification.Name("com.havokentity.mactools.driveejector.showWindow") +} + +@MainActor +final class DriveEjectorAppDelegate: NSObject, NSApplicationDelegate { + private let controller = DriveEjectorController() + + private var statusItem: HelperStatusItem? + private var panelHost: HelperPanelHost? + private var cancellables: Set = [] + + func applicationDidFinishLaunching(_ notification: Notification) { + AppDefaults.registerDefaults() + + let host = HelperPanelHost( + configuration: HelperPanelHost.Configuration( + sizing: .preferred({ DriveEjectorSizing.preferredSize() }) + ), + content: .viewController({ [controller, weak self] in + NSHostingController( + rootView: DriveEjectorPopoverView(controller: controller, onQuit: { self?.quit() }) + ) + }), + anchorView: { [weak self] in self?.statusItem?.button } + ) + panelHost = host + host.configure() + + // The mount table changes while the popover is closed, so the list is refreshed on + // every open rather than only on notifications — cheap, and it makes a stale row + // impossible to see even if a notification was missed. + host.onWillShow = { [controller] in + controller.clearStatus() + controller.refresh() + } + + statusItem = HelperStatusItem( + image: Self.statusIcon(hasVolumes: false), + toolTip: "Drive Ejector", + primaryAction: { [weak self] in self?.panelHost?.toggle() }, + quitAction: { [weak self] in self?.quit() } + ) + + // Reflect what is already mounted at launch, before any notification arrives. + updateStatusIcon() + + host.observeShowNotification(named: DriveEjectorNotifications.showWindow) + controller.startObserving() + observeControllerState() + } + + func applicationWillTerminate(_ notification: Notification) { + panelHost?.stopObservingShowNotifications() + panelHost?.removeOutsideClickMonitor() + cancellables.removeAll() + controller.stopObserving() + panelHost?.dismissForTermination() + statusItem?.remove() + } + + /// The two-state tray glyph: a filled eject symbol while external volumes are mounted, an + /// outline when there is nothing to eject. Forced to template so AppKit tints it adaptive + /// white and gives it the native rollover highlight. + private static func statusIcon(hasVolumes: Bool) -> NSImage { + let name = hasVolumes ? "eject.fill" : "eject" + let image = NSImage(systemSymbolName: name, accessibilityDescription: "Drive Ejector") ?? NSImage() + image.isTemplate = true + return image + } + + private func updateStatusIcon() { + statusItem?.button?.image = Self.statusIcon(hasVolumes: !controller.volumes.isEmpty) + } + + /// Keep the tray glyph in sync with what is mounted, so the menu bar answers “is anything + /// plugged in?” without opening the popover. + private func observeControllerState() { + controller.$volumes + .receive(on: RunLoop.main) + .sink { [weak self] _ in + self?.updateStatusIcon() + } + .store(in: &cancellables) + } + + private func quit() { + panelHost?.close() + NSApp.terminate(nil) + } +} diff --git a/Sources/DMonteDriveEjectorApp/main.swift b/Sources/DMonteDriveEjectorApp/main.swift new file mode 100644 index 0000000..fb2edce --- /dev/null +++ b/Sources/DMonteDriveEjectorApp/main.swift @@ -0,0 +1,24 @@ +import AppKit +import DMonteCore + +let singleInstanceGuard = SingleInstanceGuard(identifier: "com.havokentity.mactools.driveejector") + +guard singleInstanceGuard.isPrimary else { + if CommandLine.arguments.contains("--open") { + DistributedNotificationCenter.default().postNotificationName( + DriveEjectorNotifications.showWindow, + object: nil, + userInfo: nil, + deliverImmediately: true + ) + } + + exit(EXIT_SUCCESS) +} + +let app = NSApplication.shared +let delegate = DriveEjectorAppDelegate() + +app.delegate = delegate +app.setActivationPolicy(.accessory) +app.run() diff --git a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift new file mode 100644 index 0000000..b654650 --- /dev/null +++ b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift @@ -0,0 +1,331 @@ +import XCTest +@testable import DMonteCore + +/// The eject-safety rule is the entire reason this tool exists: unmounting someone's system disk +/// is not a bug you apologise for. These tests drive `DriveEjectorKit` with synthetic volume +/// descriptors covering every combination of the resource keys macOS vends — including the ones +/// it declines to vend — and assert exactly which volumes are offered. +/// +/// Everything here is headless-safe: no hardware, no mounts, no network. The two tests that touch +/// the real filesystem only ever ask about "/" and a nonexistent path, both of which the rule +/// rejects *before* any unmount is attempted. +final class DriveEjectorKitTests: XCTestCase { + + // MARK: - Fixtures + + /// Builds a descriptor with the flags spelled out. Defaults are the fully-unknown case so a + /// test that omits a flag is exercising the "macOS wouldn't say" branch on purpose. + private func descriptor( + path: String, + name: String, + isRootFileSystem: Bool? = nil, + isInternal: Bool? = nil, + isRemovable: Bool? = nil, + isEjectable: Bool? = nil, + totalBytes: UInt64 = 500_000_000_000, + freeBytes: UInt64 = 250_000_000_000 + ) -> VolumeDescriptor { + VolumeDescriptor( + url: URL(fileURLWithPath: path), + name: name, + isRootFileSystem: isRootFileSystem, + isInternal: isInternal, + isRemovable: isRemovable, + isEjectable: isEjectable, + totalBytes: totalBytes, + freeBytes: freeBytes + ) + } + + /// The startup disk of a normally-configured Mac. + private var bootVolume: VolumeDescriptor { + descriptor(path: "/", name: "Macintosh HD", isRootFileSystem: true, isInternal: true, isRemovable: false, isEjectable: false) + } + + /// The writable Data volume of an APFS system group. Internal, and catastrophic to unmount. + private var dataVolume: VolumeDescriptor { + descriptor(path: "/System/Volumes/Data", name: "Macintosh HD — Data", isRootFileSystem: false, isInternal: true, isRemovable: false, isEjectable: false) + } + + /// A second internal SSD in a Mac Pro / Studio. + private var internalSecondDisk: VolumeDescriptor { + descriptor(path: "/Volumes/Scratch", name: "Scratch", isRootFileSystem: false, isInternal: true, isRemovable: false, isEjectable: true) + } + + /// A bus-powered external SSD: not internal, ejectable but not removable media. + private var externalSSD: VolumeDescriptor { + descriptor(path: "/Volumes/Backup SSD", name: "Backup SSD", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: true) + } + + /// A USB stick: removable media as well as ejectable. + private var usbStick: VolumeDescriptor { + descriptor(path: "/Volumes/USB Stick", name: "USB Stick", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: true) + } + + /// A mounted disk image. + private var diskImage: VolumeDescriptor { + descriptor(path: "/Volumes/Installer", name: "Installer", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: true) + } + + /// An SMB share: external in the sense of "not inside the Mac", but not detachable media. + private var networkShare: VolumeDescriptor { + descriptor(path: "/Volumes/Studio Share", name: "Studio Share", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: false) + } + + /// A filesystem that vends no classification flags at all (some FUSE mounts behave this way). + private var unclassifiable: VolumeDescriptor { + descriptor(path: "/Volumes/Mystery", name: "Mystery") + } + + // MARK: - The safety rule + + /// THE test. Given the full spread of volume kinds a real Mac can present at once, exactly + /// the detachable external ones are offered — by name, in display order. + func testOnlyExternalRemovableVolumesAreOffered() { + let all = [ + bootVolume, + dataVolume, + internalSecondDisk, + externalSSD, + usbStick, + diskImage, + networkShare, + unclassifiable + ] + + XCTAssertEqual( + DriveEjectorKit.eligibleVolumes(from: all).map(\.name), + ["Backup SSD", "Installer", "USB Stick"], + "Only detachable external volumes may be offered; anything internal, boot, fixed or unclassifiable must be filtered out" + ) + } + + func testBootVolumeIsBlocked() { + XCTAssertEqual(DriveEjectorKit.verdict(for: bootVolume), .bootVolume) + } + + /// The scenario that breaks a naive "not internal && ejectable" rule: a Mac booted from an + /// external USB disk reports its startup volume as external, removable AND ejectable. The + /// root-filesystem flag must win over all three. + func testExternalStartupDiskIsStillBlocked() { + let bootedFromUSB = descriptor( + path: "/", + name: "Field Boot Drive", + isRootFileSystem: true, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + + XCTAssertEqual(DriveEjectorKit.verdict(for: bootedFromUSB), .bootVolume) + XCTAssertTrue(DriveEjectorKit.eligibleVolumes(from: [bootedFromUSB]).isEmpty) + } + + /// The mount point and the root-filesystem flag are independent signals; either one alone is + /// enough to block, so a missing flag cannot expose the startup disk. + func testRootMountPointIsBlockedEvenWhenTheRootFlagIsMissing() { + let flagless = descriptor( + path: "/", + name: "Macintosh HD", + isRootFileSystem: nil, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + + XCTAssertEqual(DriveEjectorKit.verdict(for: flagless), .bootVolume) + } + + func testRootFlagIsBlockedEvenWhenMountedElsewhere() { + let elsewhere = descriptor( + path: "/Volumes/Somewhere", + name: "Somewhere", + isRootFileSystem: true, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + + XCTAssertEqual(DriveEjectorKit.verdict(for: elsewhere), .bootVolume) + } + + /// A non-standardised mount-point URL must not slip past the "/" comparison. + func testUnstandardisedRootPathIsBlocked() { + let messyRoot = VolumeDescriptor( + url: URL(fileURLWithPath: "/Volumes/.."), + name: "Macintosh HD", + isRootFileSystem: nil, + isInternal: false, + isRemovable: true, + isEjectable: true, + totalBytes: 1, + freeBytes: 1 + ) + + XCTAssertEqual(DriveEjectorKit.verdict(for: messyRoot), .bootVolume) + } + + func testInternalDisksAreBlockedEvenWhenEjectable() { + // Internal drive bays report ejectable; "internal" must veto it. + XCTAssertEqual(DriveEjectorKit.verdict(for: internalSecondDisk), .internalDisk) + XCTAssertEqual(DriveEjectorKit.verdict(for: dataVolume), .internalDisk) + } + + func testFixedExternalVolumesAreBlocked() { + XCTAssertEqual(DriveEjectorKit.verdict(for: networkShare), .notRemovable) + } + + /// "Cannot be determined" must never resolve to "safe". Each flag is checked individually so + /// a single missing key is enough to exclude the volume. + func testUndeterminedVolumesAreExcluded() { + XCTAssertEqual(DriveEjectorKit.verdict(for: unclassifiable), .undetermined) + + XCTAssertEqual( + DriveEjectorKit.verdict(for: descriptor(path: "/Volumes/A", name: "A", isRootFileSystem: false, isInternal: nil, isRemovable: true, isEjectable: true)), + .undetermined, + "An unknown internal flag must exclude the volume" + ) + XCTAssertEqual( + DriveEjectorKit.verdict(for: descriptor(path: "/Volumes/B", name: "B", isRootFileSystem: false, isInternal: false, isRemovable: nil, isEjectable: true)), + .undetermined, + "An unknown removable flag must exclude the volume" + ) + XCTAssertEqual( + DriveEjectorKit.verdict(for: descriptor(path: "/Volumes/C", name: "C", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: nil)), + .undetermined, + "An unknown ejectable flag must exclude the volume" + ) + } + + /// Removable and ejectable are alternatives, not a conjunction: card media reports one, a + /// disk image reports the other, and both are legitimately ejectable. + func testEitherRemovableOrEjectableIsEnough() { + XCTAssertEqual(DriveEjectorKit.verdict(for: externalSSD), .eligible) + XCTAssertEqual(DriveEjectorKit.verdict(for: usbStick), .eligible) + XCTAssertEqual(DriveEjectorKit.verdict(for: diskImage), .eligible) + + let cardMedia = descriptor(path: "/Volumes/SD Card", name: "SD Card", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: false) + XCTAssertEqual(DriveEjectorKit.verdict(for: cardMedia), .eligible) + } + + func testIsEligibleAgreesWithVerdict() { + for volume in [bootVolume, dataVolume, internalSecondDisk, externalSSD, usbStick, diskImage, networkShare, unclassifiable] { + XCTAssertEqual( + DriveEjectorKit.isEligibleForEjection(volume), + DriveEjectorKit.verdict(for: volume) == .eligible, + "The convenience predicate must never disagree with the verdict for \(volume.name)" + ) + } + } + + func testEligibleVolumesAreSortedCaseInsensitively() { + let volumes = [ + descriptor(path: "/Volumes/zeta", name: "zeta", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: true), + descriptor(path: "/Volumes/Alpha", name: "Alpha", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: true), + descriptor(path: "/Volumes/beta", name: "beta", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: true) + ] + + XCTAssertEqual(DriveEjectorKit.eligibleVolumes(from: volumes).map(\.name), ["Alpha", "beta", "zeta"]) + } + + func testEmptyInputYieldsNoVolumes() { + XCTAssertTrue(DriveEjectorKit.eligibleVolumes(from: []).isEmpty) + } + + // MARK: - Pre-eject re-check + + /// Safe to run: the rule rejects "/" and returns before `unmountAndEjectDevice` is reached, + /// which is precisely the guarantee under test. + func testEjectingTheStartupDiskIsRefused() { + XCTAssertEqual( + DriveEjectorKit.eject(volumeAt: URL(fileURLWithPath: "/")), + .blocked(.bootVolume), + "Ejecting the startup disk must be refused by the re-check, not merely absent from the list" + ) + } + + /// A path that cannot be inspected yields no descriptor, and "no descriptor" must mean + /// "refuse" rather than "proceed with defaults". + func testEjectingAnUninspectablePathIsRefused() { + let missing = URL(fileURLWithPath: "/Volumes/\(UUID().uuidString)") + XCTAssertEqual(DriveEjectorKit.eject(volumeAt: missing), .blocked(.undetermined)) + } + + // MARK: - Descriptors and formatting + + func testUsedBytesNeverUnderflows() { + // A stale free-space reading larger than the capacity must clamp, not wrap around. + let odd = descriptor(path: "/Volumes/Odd", name: "Odd", totalBytes: 100, freeBytes: 400) + XCTAssertEqual(odd.usedBytes, 0) + + let normal = descriptor(path: "/Volumes/Normal", name: "Normal", totalBytes: 400, freeBytes: 100) + XCTAssertEqual(normal.usedBytes, 300) + } + + func testDescriptorIdentityIsTheMountPoint() { + XCTAssertEqual(usbStick.id, "/Volumes/USB Stick") + } + + func testBlockedReasonsReadAsSentencesAndEligibleHasNone() { + XCTAssertNil(VolumeEjectionVerdict.eligible.blockedReason) + for verdict: VolumeEjectionVerdict in [.bootVolume, .internalDisk, .notRemovable, .undetermined] { + let reason = verdict.blockedReason ?? "" + XCTAssertFalse(reason.isEmpty, "Every blocking verdict needs an explanation to show the user") + XCTAssertFalse(reason.contains("..."), "Typography: use a real ellipsis, never three dots") + } + } + + func testFailureMessageCombinesDescriptionAndRecoverySuggestion() { + // macOS routinely puts the name of the app holding the volume in the recovery suggestion, + // so dropping it would throw away the only actionable part of the failure. + let withSuggestion = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSLocalizedDescriptionKey: "The disk is in use.", + NSLocalizedRecoverySuggestionErrorKey: "Quit Preview and try again." + ]) + XCTAssertEqual( + DriveEjectorKit.failureMessage(for: withSuggestion), + "The disk is in use. Quit Preview and try again." + ) + } + + func testFailureMessageDoesNotRepeatItself() { + let duplicated = NSError(domain: NSCocoaErrorDomain, code: 1, userInfo: [ + NSLocalizedDescriptionKey: "The disk is in use.", + NSLocalizedRecoverySuggestionErrorKey: "The disk is in use." + ]) + XCTAssertEqual(DriveEjectorKit.failureMessage(for: duplicated), "The disk is in use.") + } + + func testFailureMessageAlwaysSaysSomething() { + let blank = NSError(domain: "Test", code: 1, userInfo: [NSLocalizedDescriptionKey: ""]) + XCTAssertFalse( + DriveEjectorKit.failureMessage(for: blank).isEmpty, + "A silent failure would leave the status line blank after a failed eject" + ) + } + + // MARK: - Live enumeration (environment-tolerant) + + /// Whatever this machine happens to have mounted, nothing the tool offers may be internal or + /// the startup disk. Asserts an invariant rather than a count, so it is stable on CI. + func testNothingOfferedOnThisMachineIsInternalOrBoot() { + for volume in DriveEjectorKit.ejectableVolumes() { + XCTAssertNotEqual(volume.url.standardizedFileURL.path, "/", "The startup disk was offered") + XCTAssertNotEqual(volume.isRootFileSystem, true, "The root filesystem was offered") + XCTAssertNotEqual(volume.isInternal, true, "An internal disk was offered: \(volume.name)") + XCTAssertTrue(volume.isRemovable == true || volume.isEjectable == true) + } + } + + func testMountedDescriptorsIncludeTheStartupDiskAndTheRuleRejectsIt() { + let descriptors = DriveEjectorKit.mountedVolumeDescriptors() + XCTAssertFalse(descriptors.isEmpty, "A running Mac always has at least the startup volume mounted") + + guard let root = descriptors.first(where: { $0.url.standardizedFileURL.path == "/" }) else { + // Some sandboxes hide "/" from the volume enumeration; the "/" eject test above still + // covers the refusal path directly. + return + } + XCTAssertEqual(DriveEjectorKit.verdict(for: root), .bootVolume) + } +} From 1e53393150ab7dd63f3b1c7ea0a9e045da83e345 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Sat, 18 Jul 2026 13:56:54 +0530 Subject: [PATCH 2/7] review: fix Drive Ejector safety and lifecycle defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../DMonteCore/DriveEjectorController.swift | 69 +++++++++--- Sources/DMonteCore/DriveEjectorKit.swift | 43 +++++++- .../DriveEjectorAppDelegate.swift | 10 +- .../DriveEjectorKitTests.swift | 100 +++++++++++++++++- 4 files changed, 204 insertions(+), 18 deletions(-) diff --git a/Sources/DMonteCore/DriveEjectorController.swift b/Sources/DMonteCore/DriveEjectorController.swift index e18ea32..b1827c7 100644 --- a/Sources/DMonteCore/DriveEjectorController.swift +++ b/Sources/DMonteCore/DriveEjectorController.swift @@ -32,8 +32,9 @@ public final class DriveEjectorController: NSObject, ObservableObject { /// The inline status line: progress while ejecting, then the honest result. @Published public private(set) var statusMessage: String? - /// Mount points with an eject in flight, so their rows can show progress and refuse a - /// second press instead of racing a duplicate unmount. + /// Mount points whose rows must read as in-progress and refuse a press: the union of the + /// ejects this tool started and the unmounts someone else announced. Derived — never assigned + /// directly — so the two sources can expire on their own schedules. @Published public private(set) var busyVolumeIDs: Set = [] /// `true` once Eject All has been requested and is waiting for the confirming press. @@ -42,6 +43,19 @@ public final class DriveEjectorController: NSObject, ObservableObject { /// Persisted preference: require a second press for Eject All. @Published public private(set) var confirmsEjectAll: Bool + /// Mount points with an eject *this tool* started. Cleared only by the completion handler, + /// which always runs, so a refresh mid-eject cannot re-enable a button that is still working. + private var ejectingVolumeIDs: Set = [] + + /// Mount points someone else (Finder, `diskutil`) announced an unmount for, against the moment + /// the announcement arrived. Expired by `DriveEjectorKit.liveUnmountAnnouncements`. + private var announcedUnmounts: [String: Date] = [:] + + /// How long a foreign unmount announcement keeps a row busy before it is presumed denied. + /// Generous enough to cover a slow unmount of a large disk, short enough that a row which was + /// vetoed un-sticks itself without the user having to relaunch the tool. + private static let unmountAnnouncementGrace: TimeInterval = 15 + public override init() { confirmsEjectAll = AppDefaults.shared.bool(forKey: DefaultsKey.driveEjectorConfirmsEjectAll) super.init() @@ -56,18 +70,34 @@ public final class DriveEjectorController: NSObject, ObservableObject { /// Re-reads the mount table. Cheap enough to call on every mount notification. public func refresh() { + let previousIDs = Set(volumes.map(\.id)) volumes = DriveEjectorKit.ejectableVolumes() + recomputeBusyVolumeIDs() - // Drop busy markers for volumes that are gone; otherwise a successful eject would leave - // a phantom entry that suppresses a later eject of a disk remounted at the same point. - let liveIDs = Set(volumes.map(\.id)) - busyVolumeIDs.formIntersection(liveIDs) - - if volumes.isEmpty { + // An armed Eject All authorises the set of volumes the user was looking at when they armed + // it. If a disk has appeared or vanished since, the confirming press would act on a list + // they never saw, so the arming is dropped and has to be repeated deliberately. + if isConfirmingEjectAll, Set(volumes.map(\.id)) != previousIDs { isConfirmingEjectAll = false } } + /// Rebuilds `busyVolumeIDs` from its two sources, expiring foreign unmount announcements that + /// have outlived their grace period. + private func recomputeBusyVolumeIDs(now: Date = Date()) { + announcedUnmounts = DriveEjectorKit.liveUnmountAnnouncements( + announcedUnmounts, + mountedIDs: Set(volumes.map(\.id)), + now: now, + grace: Self.unmountAnnouncementGrace + ) + + let combined = ejectingVolumeIDs.union(announcedUnmounts.keys) + if combined != busyVolumeIDs { + busyVolumeIDs = combined + } + } + /// Begins observing mount/unmount activity so the list stays truthful without polling. public func startObserving() { let center = NSWorkspace.shared.notificationCenter @@ -103,7 +133,8 @@ public final class DriveEjectorController: NSObject, ObservableObject { return } - busyVolumeIDs.insert(volume.id) + ejectingVolumeIDs.insert(volume.id) + recomputeBusyVolumeIDs() statusMessage = "Ejecting “\(volume.name)”…" let request = EjectRequest(id: volume.id, name: volume.name, url: volume.url) @@ -142,7 +173,8 @@ public final class DriveEjectorController: NSObject, ObservableObject { return } - targets.forEach { busyVolumeIDs.insert($0.id) } + targets.forEach { ejectingVolumeIDs.insert($0.id) } + recomputeBusyVolumeIDs() statusMessage = "Ejecting \(targets.count) volume\(targets.count == 1 ? "" : "s")…" Task.detached(priority: .userInitiated) { @@ -189,7 +221,18 @@ public final class DriveEjectorController: NSObject, ObservableObject { guard let url = notification.volumeURL else { return } - busyVolumeIDs.insert(url.path) + + // Standardised because the row IDs come from `mountedVolumeURLs`, and a marker keyed on a + // differently-spelled path for the same mount point would silently match nothing. + announcedUnmounts[url.standardizedFileURL.path] = Date() + recomputeBusyVolumeIDs() + + // A denied unmount posts nothing further, so this is the only thing that will release the + // marker. Without it a vetoed unmount disables the row until the tool is relaunched. + Task { [weak self] in + try? await Task.sleep(for: .seconds(Self.unmountAnnouncementGrace)) + self?.refresh() + } } @objc private func volumeDidUnmount(_ notification: Notification) { @@ -203,13 +246,13 @@ public final class DriveEjectorController: NSObject, ObservableObject { // MARK: - Private private func finish(_ request: EjectRequest, outcome: EjectionOutcome) { - busyVolumeIDs.remove(request.id) + ejectingVolumeIDs.remove(request.id) statusMessage = message(for: outcome, volumeName: request.name) refresh() } private func finishBatch(_ results: [(EjectRequest, EjectionOutcome)]) { - results.forEach { busyVolumeIDs.remove($0.0.id) } + results.forEach { ejectingVolumeIDs.remove($0.0.id) } let failures = results.filter { $0.1 != .ejected } diff --git a/Sources/DMonteCore/DriveEjectorKit.swift b/Sources/DMonteCore/DriveEjectorKit.swift index e302004..cbec600 100644 --- a/Sources/DMonteCore/DriveEjectorKit.swift +++ b/Sources/DMonteCore/DriveEjectorKit.swift @@ -115,17 +115,33 @@ public enum DriveEjectorKit { .volumeAvailableCapacityKey ] + /// The directory macOS reserves for the running system's own volume group — the Data volume, + /// `VM`, `Preboot`, `Update`, `xarts`, `iSCPreboot`, `Hardware`. + /// + /// The enumeration keeps these off the list via `.skipHiddenVolumes`, but that is an option on + /// one call site rather than a property of the rule, and `eject(volumeAt:)` re-derives its + /// verdict from a bare mount point without going near it. On a Mac booted from external media + /// every one of these reports `isInternal == false` plus removable and ejectable, and only + /// `/System/Volumes/Data` also carries the root-filesystem flag — so the siblings would clear + /// the rule outright. Stated here so both paths inherit the same guarantee. + private static let systemVolumeGroupRoot = "/System/Volumes" + /// Decides whether a volume may be ejected. Deliberately biased towards refusing. /// - /// Order matters: the catastrophic case is checked first and by two independent signals, so - /// a filesystem that vends nothing useful still cannot get the startup disk onto the list. + /// Order matters: the catastrophic case is checked first and by three independent signals, so + /// a filesystem that vends nothing useful still cannot get a system volume onto the list. public static func verdict(for descriptor: VolumeDescriptor) -> VolumeEjectionVerdict { // A mount point of "/" is a fact no missing resource key can hide, and the root-filesystem // flag catches the same volume when it is reached through a symlinked or relative URL. // Checked before the removable flags because a Mac booted from an external USB disk reports // that disk as removable *and* ejectable — exactly the combination that would otherwise // wave the startup disk straight through. - if descriptor.url.standardizedFileURL.path == "/" || descriptor.isRootFileSystem == true { + let path = descriptor.url.standardizedFileURL.path + if path == "/" || descriptor.isRootFileSystem == true { + return .bootVolume + } + + if path == systemVolumeGroupRoot || path.hasPrefix(systemVolumeGroupRoot + "/") { return .bootVolume } @@ -205,6 +221,27 @@ public enum DriveEjectorKit { ) } + // MARK: - Foreign unmount announcements + + /// Filters "someone else started unmounting this" markers down to the ones still worth honouring. + /// + /// `NSWorkspace.willUnmountNotification` is an announcement, not a promise: any app holding the + /// volume can dissent, in which case the unmount is abandoned and no `didUnmountNotification` + /// ever arrives. A marker kept on that basis alone would disable its row — and exclude it from + /// Eject All — for the lifetime of the process, so a volume that is *still mounted* once `grace` + /// has elapsed is released. A marker whose volume has gone is dropped immediately: the unmount + /// succeeded and there is no row left to protect. + static func liveUnmountAnnouncements( + _ announcements: [String: Date], + mountedIDs: Set, + now: Date, + grace: TimeInterval + ) -> [String: Date] { + announcements.filter { id, announcedAt in + mountedIDs.contains(id) && now.timeIntervalSince(announcedAt) < grace + } + } + // MARK: - Ejection /// Unmounts and ejects the device backing `url`, but only after re-deriving the verdict from diff --git a/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift b/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift index 1c5af4c..451e637 100644 --- a/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift +++ b/Sources/DMonteDriveEjectorApp/DriveEjectorAppDelegate.swift @@ -11,7 +11,12 @@ enum DriveEjectorNotifications { @MainActor final class DriveEjectorAppDelegate: NSObject, NSApplicationDelegate { - private let controller = DriveEjectorController() + /// Built lazily, on the first access in `applicationDidFinishLaunching` — i.e. *after* + /// `AppDefaults.registerDefaults()`. The controller reads the Eject All confirmation + /// preference in `init`, and a registration domain only exists once registered in this + /// process, so an eagerly-created controller would read `false` and silently ship the one + /// destructive action here with its confirmation turned off. + private lazy var controller = DriveEjectorController() private var statusItem: HelperStatusItem? private var panelHost: HelperPanelHost? @@ -39,6 +44,9 @@ final class DriveEjectorAppDelegate: NSObject, NSApplicationDelegate { // impossible to see even if a notification was missed. host.onWillShow = { [controller] in controller.clearStatus() + // An armed Eject All must not survive the panel closing: reopening minutes later onto a + // primed "Eject 3 Volumes" button is one stray click from unmounting everything. + controller.cancelEjectAll() controller.refresh() } diff --git a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift index b654650..8e782f4 100644 --- a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift +++ b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift @@ -165,10 +165,55 @@ final class DriveEjectorKitTests: XCTestCase { XCTAssertEqual(DriveEjectorKit.verdict(for: messyRoot), .bootVolume) } + /// The external-boot case one level deeper than the startup volume itself. When a Mac boots + /// from a USB SSD, the firmlinked members of the running system's volume group sit on that same + /// external medium: `isInternal` is false, removable and ejectable are true, and only the Data + /// volume carries the root flag. `VM` and `Preboot` therefore clear every other check, and the + /// enumeration's `.skipHiddenVolumes` does not protect `eject(volumeAt:)`, which re-derives its + /// verdict from a bare mount point. + func testSystemVolumeGroupIsBlockedOnAnExternallyBootedMac() { + for path in ["/System/Volumes/Data", "/System/Volumes/VM", "/System/Volumes/Preboot", "/System/Volumes/Update"] { + let onExternalBootMedia = descriptor( + path: path, + name: (path as NSString).lastPathComponent, + isRootFileSystem: false, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + + XCTAssertEqual( + DriveEjectorKit.verdict(for: onExternalBootMedia), + .bootVolume, + "\(path) belongs to the running system and must never be offered" + ) + } + } + + /// The system-volume guard is a path-prefix rule, so it must match on path *components* and not + /// swallow an ordinary user disk whose mount point merely starts with the same characters. + func testUserVolumesNamedLikeSystemPathsAreStillOffered() { + for path in ["/Volumes/System", "/Volumes/System Volumes Backup"] { + let userDisk = descriptor( + path: path, + name: "Backup", + isRootFileSystem: false, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + XCTAssertEqual(DriveEjectorKit.verdict(for: userDisk), .eligible, "\(path) is an ordinary external disk") + } + } + func testInternalDisksAreBlockedEvenWhenEjectable() { // Internal drive bays report ejectable; "internal" must veto it. XCTAssertEqual(DriveEjectorKit.verdict(for: internalSecondDisk), .internalDisk) - XCTAssertEqual(DriveEjectorKit.verdict(for: dataVolume), .internalDisk) + + // The Data volume is caught earlier, by the system-volume-group rule, so it stays blocked + // even on a Mac where "internal" would be false. Either way it is never offered. + XCTAssertEqual(DriveEjectorKit.verdict(for: dataVolume), .bootVolume) + XCTAssertFalse(DriveEjectorKit.isEligibleForEjection(dataVolume)) } func testFixedExternalVolumesAreBlocked() { @@ -251,6 +296,59 @@ final class DriveEjectorKitTests: XCTestCase { XCTAssertEqual(DriveEjectorKit.eject(volumeAt: missing), .blocked(.undetermined)) } + // MARK: - Foreign unmount announcements + + /// The bug this guards: `willUnmountNotification` fires, the unmount is then *denied* by an app + /// holding the volume, and no further notification ever arrives. Keeping the marker on the + /// grounds that the volume is still mounted would disable its Eject button — and drop it from + /// Eject All — until the tool was relaunched. + func testStillMountedAnnouncementIsReleasedAfterTheGracePeriod() { + let announced = Date() + let announcements = ["/Volumes/USB Stick": announced] + let mounted: Set = ["/Volumes/USB Stick"] + + XCTAssertEqual( + DriveEjectorKit.liveUnmountAnnouncements(announcements, mountedIDs: mounted, now: announced.addingTimeInterval(5), grace: 15).keys.sorted(), + ["/Volumes/USB Stick"], + "An unmount still in progress must keep the row busy" + ) + + XCTAssertTrue( + DriveEjectorKit.liveUnmountAnnouncements(announcements, mountedIDs: mounted, now: announced.addingTimeInterval(15), grace: 15).isEmpty, + "A volume still mounted after the grace period had its unmount denied; the row must become pressable again" + ) + } + + /// The unmount succeeded, so there is no row left to protect and the marker must not linger to + /// suppress a disk later remounted at the same point. + func testAnnouncementForAVanishedVolumeIsDroppedImmediately() { + let announced = Date() + XCTAssertTrue( + DriveEjectorKit.liveUnmountAnnouncements( + ["/Volumes/USB Stick": announced], + mountedIDs: [], + now: announced.addingTimeInterval(1), + grace: 15 + ).isEmpty + ) + } + + func testAnnouncementsAreFilteredIndependently() { + let now = Date() + let live = DriveEjectorKit.liveUnmountAnnouncements( + [ + "/Volumes/Slow": now.addingTimeInterval(-2), + "/Volumes/Vetoed": now.addingTimeInterval(-99), + "/Volumes/Gone": now.addingTimeInterval(-1) + ], + mountedIDs: ["/Volumes/Slow", "/Volumes/Vetoed"], + now: now, + grace: 15 + ) + + XCTAssertEqual(live.keys.sorted(), ["/Volumes/Slow"]) + } + // MARK: - Descriptors and formatting func testUsedBytesNeverUnderflows() { From db6a9d8d40432acd1546367de33fdb9bc9b21ca8 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Sat, 18 Jul 2026 16:04:43 +0530 Subject: [PATCH 3/7] review: address Copilot inline comments on PR #13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../DMonteCore/DriveEjectorController.swift | 7 ++- Sources/DMonteCore/DriveEjectorKit.swift | 29 ++++++++-- .../DriveEjectorKitTests.swift | 55 +++++++++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/Sources/DMonteCore/DriveEjectorController.swift b/Sources/DMonteCore/DriveEjectorController.swift index b1827c7..1e235e1 100644 --- a/Sources/DMonteCore/DriveEjectorController.swift +++ b/Sources/DMonteCore/DriveEjectorController.swift @@ -222,9 +222,10 @@ public final class DriveEjectorController: NSObject, ObservableObject { return } - // Standardised because the row IDs come from `mountedVolumeURLs`, and a marker keyed on a - // differently-spelled path for the same mount point would silently match nothing. - announcedUnmounts[url.standardizedFileURL.path] = Date() + // Through the shared identity because the row IDs come from `mountedVolumeURLs`, and a + // marker keyed on a differently-spelled path for the same mount point would silently match + // nothing — leaving the row pressable while its unmount is already under way. + announcedUnmounts[DriveEjectorKit.volumeIdentity(for: url)] = Date() recomputeBusyVolumeIDs() // A denied unmount posts nothing further, so this is the only thing that will release the diff --git a/Sources/DMonteCore/DriveEjectorKit.swift b/Sources/DMonteCore/DriveEjectorKit.swift index cbec600..b15b3c5 100644 --- a/Sources/DMonteCore/DriveEjectorKit.swift +++ b/Sources/DMonteCore/DriveEjectorKit.swift @@ -9,7 +9,8 @@ import Foundation /// the permissive branch. `DriveEjectorKit.verdict(for:)` keeps them apart and refuses anything /// it cannot positively clear. public struct VolumeDescriptor: Identifiable, Sendable, Equatable { - /// Mount point. Doubles as the identity: only one volume can occupy a mount point at a time. + /// Mount point, always in canonical spelling. Doubles as the identity: only one volume can + /// occupy a mount point at a time. public let url: URL /// Display name (`volumeNameKey`), falling back to the mount point's last path component. public let name: String @@ -26,7 +27,7 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { /// Free capacity in bytes; `0` when unavailable. public let freeBytes: UInt64 - public var id: String { url.path } + public var id: String { DriveEjectorKit.volumeIdentity(for: url) } /// Bytes in use, clamped at zero so a stale free-space reading can never produce an underflow. public var usedBytes: UInt64 { @@ -43,7 +44,11 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { totalBytes: UInt64, freeBytes: UInt64 ) { - self.url = url + // Canonicalised on the way in so that the mount point handed to `unmountAndEjectDevice`, + // the path the safety rule compares, and the identity the busy-marker bookkeeping keys on + // are the same string. A row whose ID disagreed with an in-flight unmount marker would + // re-enable itself mid-unmount, which on this tool means offering to eject a disk twice. + self.url = url.standardizedFileURL self.name = name self.isRootFileSystem = isRootFileSystem self.isInternal = isInternal @@ -126,6 +131,18 @@ public enum DriveEjectorKit { /// the rule outright. Stated here so both paths inherit the same guarantee. private static let systemVolumeGroupRoot = "/System/Volumes" + /// The one definition of “which volume is this”, shared by the descriptors, the safety rule and + /// the controller's busy markers. + /// + /// Mount-point URLs reach the tool from three unrelated sources — `mountedVolumeURLs`, the + /// `NSWorkspace` unmount notifications, and whatever a caller passes to `eject(volumeAt:)` — and + /// nothing guarantees they spell the same mount point identically. Every comparison in the tool + /// goes through here so a `..`, a trailing slash or a stray `.` cannot make two names for one + /// disk look like two disks. + public static func volumeIdentity(for url: URL) -> String { + url.standardizedFileURL.path + } + /// Decides whether a volume may be ejected. Deliberately biased towards refusing. /// /// Order matters: the catastrophic case is checked first and by three independent signals, so @@ -136,7 +153,7 @@ public enum DriveEjectorKit { // Checked before the removable flags because a Mac booted from an external USB disk reports // that disk as removable *and* ejectable — exactly the combination that would otherwise // wave the startup disk straight through. - let path = descriptor.url.standardizedFileURL.path + let path = volumeIdentity(for: descriptor.url) if path == "/" || descriptor.isRootFileSystem == true { return .bootVolume } @@ -269,7 +286,9 @@ public enum DriveEjectorKit { } do { - try NSWorkspace.shared.unmountAndEjectDevice(at: url) + // The descriptor's canonical mount point, not the caller's spelling: the volume that + // was cleared and the volume that gets unmounted must be named by the same string. + try NSWorkspace.shared.unmountAndEjectDevice(at: descriptor.url) return .ejected } catch { return .failed(failureMessage(for: error)) diff --git a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift index 8e782f4..1a7246d 100644 --- a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift +++ b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift @@ -163,6 +163,7 @@ final class DriveEjectorKitTests: XCTestCase { ) XCTAssertEqual(DriveEjectorKit.verdict(for: messyRoot), .bootVolume) + XCTAssertEqual(messyRoot.id, "/", "The startup disk must not acquire a second identity") } /// The external-boot case one level deeper than the startup volume itself. When a Mac boots @@ -364,6 +365,60 @@ final class DriveEjectorKitTests: XCTestCase { XCTAssertEqual(usbStick.id, "/Volumes/USB Stick") } + /// Identity is canonical, so the same mount point spelled three different ways is one volume. + /// This is what lets a busy marker keyed on a `willUnmount` notification's URL match the row + /// built from `mountedVolumeURLs`; without it a row would stay pressable mid-unmount. + func testDescriptorIdentityIsCanonical() { + for spelling in ["/Volumes/USB Stick", "/Volumes/USB Stick/", "/Volumes/./USB Stick", "/Volumes/Other/../USB Stick"] { + XCTAssertEqual( + descriptor(path: spelling, name: "USB Stick").id, + usbStick.id, + "\(spelling) names the same mount point and must produce the same identity" + ) + } + } + + /// The descriptor's own mount point is canonicalised too — it is the URL handed to + /// `unmountAndEjectDevice`, so it must name the same volume the safety rule cleared. + func testDescriptorURLIsCanonicalised() { + XCTAssertEqual(descriptor(path: "/Volumes/Other/../USB Stick", name: "USB Stick").url.path, "/Volumes/USB Stick") + } + + /// A busy marker and a row ID derived from different spellings of one mount point must collide, + /// which is the whole point of routing both through `volumeIdentity(for:)`. + func testBusyMarkerMatchesARowBuiltFromADifferentSpelling() { + let announcement = DriveEjectorKit.volumeIdentity(for: URL(fileURLWithPath: "/Volumes/Other/../USB Stick/")) + XCTAssertTrue( + DriveEjectorKit.liveUnmountAnnouncements( + [announcement: Date()], + mountedIDs: [usbStick.id], + now: Date(), + grace: 15 + ).keys.contains(usbStick.id), + "An unmount announced for a non-canonical path must keep the matching row busy" + ) + } + + /// Canonicalising identity must not weaken the rule: a system-volume-group member reached + /// through a non-canonical path is still refused. + func testNonCanonicalSystemVolumePathsAreStillBlocked() { + for path in ["/System/Volumes/Data/", "/System/Volumes/./VM", "/Volumes/../System/Volumes/Preboot", "/System/Volumes/Update/.."] { + let onExternalBootMedia = descriptor( + path: path, + name: "Data", + isRootFileSystem: false, + isInternal: false, + isRemovable: true, + isEjectable: true + ) + XCTAssertEqual( + DriveEjectorKit.verdict(for: onExternalBootMedia), + .bootVolume, + "\(path) resolves inside the running system's volume group and must never be offered" + ) + } + } + func testBlockedReasonsReadAsSentencesAndEligibleHasNone() { XCTAssertNil(VolumeEjectionVerdict.eligible.blockedReason) for verdict: VolumeEjectionVerdict in [.bootVolume, .internalDisk, .notRemovable, .undetermined] { From eb93e9617a290a39192d32948c556f24c759a25d Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Sat, 18 Jul 2026 17:44:29 +0530 Subject: [PATCH 4/7] Drive Ejector: list real external drives, which report neither flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Sources/DMonteCore/DriveEjectorKit.swift | 57 +++++++++----- .../DriveEjectorKitTests.swift | 75 ++++++++++++++++--- 2 files changed, 103 insertions(+), 29 deletions(-) diff --git a/Sources/DMonteCore/DriveEjectorKit.swift b/Sources/DMonteCore/DriveEjectorKit.swift index b15b3c5..f9565ed 100644 --- a/Sources/DMonteCore/DriveEjectorKit.swift +++ b/Sources/DMonteCore/DriveEjectorKit.swift @@ -18,10 +18,16 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { public let isRootFileSystem: Bool? /// `true` for disks physically inside the Mac (including the sealed system and Data volumes). public let isInternal: Bool? - /// `true` for media that can be removed from its drive. + /// `true` for removable *media* — an SD card or optical disc, not an unpluggable drive. + /// Reported `false` by USB and Thunderbolt SSDs, so it cannot gate eligibility. Kept for + /// display and diagnostics only. public let isRemovable: Bool? - /// `true` when the volume can be unmounted and ejected by the user. + /// `true` when the filesystem advertises itself as ejectable. Also `false` on plenty of real + /// external drives, so likewise not an eligibility signal. See `isRemovable`. public let isEjectable: Bool? + /// `false` for network mounts (SMB, NFS, autofs homes). A drive ejector deals in hardware, + /// so a volume that is known-not-local is refused. + public let isLocal: Bool? /// Total capacity in bytes; `0` when unavailable. public let totalBytes: UInt64 /// Free capacity in bytes; `0` when unavailable. @@ -41,6 +47,7 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { isInternal: Bool?, isRemovable: Bool?, isEjectable: Bool?, + isLocal: Bool? = nil, totalBytes: UInt64, freeBytes: UInt64 ) { @@ -54,6 +61,7 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { self.isInternal = isInternal self.isRemovable = isRemovable self.isEjectable = isEjectable + self.isLocal = isLocal self.totalBytes = totalBytes self.freeBytes = freeBytes } @@ -64,14 +72,15 @@ public struct VolumeDescriptor: Identifiable, Sendable, Equatable { /// A separate reason (rather than a bare `Bool`) exists so the re-check performed immediately /// before unmounting can tell the user *why* a row that looked ejectable a moment ago is not. public enum VolumeEjectionVerdict: Sendable, Equatable { - /// External, removable, and demonstrably not the startup disk. + /// External, local, and demonstrably not the startup disk. case eligible /// The volume the system booted from. Never ejectable, regardless of its other flags. case bootVolume /// A disk inside the Mac (including the sealed system and Data volumes). case internalDisk - /// External but fixed — nothing about it says it can be detached. - case notRemovable + /// A network mount rather than a drive. Unmounting one is a different action with different + /// consequences, so it is not offered here. + case networkVolume /// The filesystem did not vend the flags needed to clear it. Excluded by default. case undetermined @@ -81,7 +90,7 @@ public enum VolumeEjectionVerdict: Sendable, Equatable { case .eligible: nil case .bootVolume: "it’s the startup disk" case .internalDisk: "it’s an internal disk" - case .notRemovable: "it isn’t a removable volume" + case .networkVolume: "it’s a network volume, not a drive" case .undetermined: "macOS wouldn’t confirm it’s safe to eject" } } @@ -116,6 +125,7 @@ public enum DriveEjectorKit { .volumeIsInternalKey, .volumeIsRemovableKey, .volumeIsEjectableKey, + .volumeIsLocalKey, .volumeTotalCapacityKey, .volumeAvailableCapacityKey ] @@ -162,11 +172,16 @@ public enum DriveEjectorKit { return .bootVolume } - // All three classification flags must be present. A volume nobody will classify is one - // this tool declines to touch: guessing costs the user a disk, refusing costs a click. - guard let isInternal = descriptor.isInternal, - let isRemovable = descriptor.isRemovable, - let isEjectable = descriptor.isEjectable else { + // A drive ejector deals in hardware. Network mounts are refused before the hardware + // flags are consulted, since a share can otherwise look external in every other respect. + if descriptor.isLocal == false { + return .networkVolume + } + + // A volume nobody will classify is one this tool declines to touch: guessing costs the + // user a disk, refusing costs a click. Synthetic mounts — the simulator's disk images, + // autofs homes — vend no `isInternal` at all and drop out here. + guard let isInternal = descriptor.isInternal else { return .undetermined } @@ -174,12 +189,19 @@ public enum DriveEjectorKit { return .internalDisk } - // Either flag alone is enough: USB sticks and card media report removable, while disk - // images and most external SSDs report only ejectable. - guard isRemovable || isEjectable else { - return .notRemovable - } - + // `isInternal == false` is the whole rule, because it is the only flag that actually + // tracks "this disk is external" on modern hardware. + // + // Do NOT gate on `isRemovable` / `isEjectable`. In DiskArbitration terms "removable" + // means removable *media* — an SD card or an optical disc — not a drive you can + // unplug, so a USB or Thunderbolt SSD reports removable=false AND ejectable=false and + // a rule keyed on them lists nothing at all. Measured on a Mac Studio: a USB SSD and a + // PCIe enclosure both report false/false, while the iOS Simulator's disk images are + // the volumes that report true/true. The flags are kept on the descriptor because they + // are honest signals for display, but they cannot decide eligibility. + // + // Safety is unaffected: an externally-booted startup disk is caught above by the + // root-filesystem flag and the `/System/Volumes` prefix, both of which run first. return .eligible } @@ -233,6 +255,7 @@ public enum DriveEjectorKit { isInternal: values.volumeIsInternal, isRemovable: values.volumeIsRemovable, isEjectable: values.volumeIsEjectable, + isLocal: values.volumeIsLocal, totalBytes: total, freeBytes: free ) diff --git a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift index 1a7246d..a5f41a7 100644 --- a/Tests/DMonteCoreTests/DriveEjectorKitTests.swift +++ b/Tests/DMonteCoreTests/DriveEjectorKitTests.swift @@ -22,6 +22,7 @@ final class DriveEjectorKitTests: XCTestCase { isInternal: Bool? = nil, isRemovable: Bool? = nil, isEjectable: Bool? = nil, + isLocal: Bool? = true, totalBytes: UInt64 = 500_000_000_000, freeBytes: UInt64 = 250_000_000_000 ) -> VolumeDescriptor { @@ -32,6 +33,7 @@ final class DriveEjectorKitTests: XCTestCase { isInternal: isInternal, isRemovable: isRemovable, isEjectable: isEjectable, + isLocal: isLocal, totalBytes: totalBytes, freeBytes: freeBytes ) @@ -57,6 +59,25 @@ final class DriveEjectorKitTests: XCTestCase { descriptor(path: "/Volumes/Backup SSD", name: "Backup SSD", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: true) } + /// A USB SSD exactly as macOS actually describes one — measured on a Mac Studio, where both + /// the removable and the ejectable flag come back `false`. The tool listed nothing at all + /// until the rule stopped requiring them; this fixture is the regression guard. + private var usbSSDAsMacOSReportsIt: VolumeDescriptor { + descriptor(path: "/Volumes/T9 MAC OUT", name: "T9 MAC OUT", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: false, isLocal: true) + } + + /// A Thunderbolt/PCIe enclosure, likewise measured: false/false, and external only by + /// `isInternal`. + private var pcieEnclosureAsMacOSReportsIt: VolumeDescriptor { + descriptor(path: "/Volumes/XTRM 5 Media", name: "XTRM 5 Media", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: false, isLocal: true) + } + + /// An iOS Simulator runtime image. Reports removable *and* ejectable — the exact combination + /// the old rule keyed on — while vending no `isInternal`, so it must still be refused. + private var simulatorRuntimeImage: VolumeDescriptor { + descriptor(path: "/Library/Developer/CoreSimulator/Volumes/iOS_23E254a", name: "iOS 26.4.1 Simulator", isRootFileSystem: false, isInternal: nil, isRemovable: true, isEjectable: true, isLocal: true) + } + /// A USB stick: removable media as well as ejectable. private var usbStick: VolumeDescriptor { descriptor(path: "/Volumes/USB Stick", name: "USB Stick", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: true) @@ -67,9 +88,11 @@ final class DriveEjectorKitTests: XCTestCase { descriptor(path: "/Volumes/Installer", name: "Installer", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: true) } - /// An SMB share: external in the sense of "not inside the Mac", but not detachable media. + /// An SMB share. Identified by `isLocal == false` and nothing else: its removable and + /// ejectable flags are indistinguishable from a real USB SSD's, which is precisely why the + /// rule cannot be built on them. private var networkShare: VolumeDescriptor { - descriptor(path: "/Volumes/Studio Share", name: "Studio Share", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: false) + descriptor(path: "/Volumes/Studio Share", name: "Studio Share", isRootFileSystem: false, isInternal: false, isRemovable: false, isEjectable: false, isLocal: false) } /// A filesystem that vends no classification flags at all (some FUSE mounts behave this way). @@ -217,12 +240,38 @@ final class DriveEjectorKitTests: XCTestCase { XCTAssertFalse(DriveEjectorKit.isEligibleForEjection(dataVolume)) } - func testFixedExternalVolumesAreBlocked() { - XCTAssertEqual(DriveEjectorKit.verdict(for: networkShare), .notRemovable) + func testNetworkVolumesAreBlocked() { + XCTAssertEqual(DriveEjectorKit.verdict(for: networkShare), .networkVolume) + XCTAssertFalse(DriveEjectorKit.isEligibleForEjection(networkShare)) } - /// "Cannot be determined" must never resolve to "safe". Each flag is checked individually so - /// a single missing key is enough to exclude the volume. + /// The regression this tool actually shipped with: every real external drive was refused. + /// + /// `isRemovable` means removable *media* — an SD card, an optical disc — so USB and + /// Thunderbolt SSDs report it `false`, and report `isEjectable` `false` too. A rule that + /// required either flag listed nothing at all on a normal Mac. These are the values macOS + /// really returned for two attached drives. + func testRealExternalDrivesAreOfferedEvenThoughBothFlagsAreFalse() { + XCTAssertEqual(DriveEjectorKit.verdict(for: usbSSDAsMacOSReportsIt), .eligible) + XCTAssertEqual(DriveEjectorKit.verdict(for: pcieEnclosureAsMacOSReportsIt), .eligible) + XCTAssertTrue(DriveEjectorKit.isEligibleForEjection(usbSSDAsMacOSReportsIt)) + XCTAssertTrue(DriveEjectorKit.isEligibleForEjection(pcieEnclosureAsMacOSReportsIt)) + } + + /// The mirror image: the volumes that *do* claim removable+ejectable on a real Mac are + /// simulator runtime images, which must never be offered. They vend no `isInternal`. + func testSimulatorImagesAreRefusedDespiteClaimingRemovableAndEjectable() { + XCTAssertEqual(DriveEjectorKit.verdict(for: simulatorRuntimeImage), .undetermined) + XCTAssertFalse(DriveEjectorKit.isEligibleForEjection(simulatorRuntimeImage)) + } + + /// "Cannot be determined" must never resolve to "safe" — but only for the flag the rule + /// actually depends on. + /// + /// `isInternal` is that flag: without it nothing distinguishes an external drive from the + /// sealed system volume, so a missing value excludes. `isRemovable` and `isEjectable` are + /// deliberately *not* required — real external drives report them as `false`, and requiring + /// them (present or true) is what made the tool list nothing at all. func testUndeterminedVolumesAreExcluded() { XCTAssertEqual(DriveEjectorKit.verdict(for: unclassifiable), .undetermined) @@ -233,13 +282,13 @@ final class DriveEjectorKitTests: XCTestCase { ) XCTAssertEqual( DriveEjectorKit.verdict(for: descriptor(path: "/Volumes/B", name: "B", isRootFileSystem: false, isInternal: false, isRemovable: nil, isEjectable: true)), - .undetermined, - "An unknown removable flag must exclude the volume" + .eligible, + "A missing removable flag is not disqualifying: the rule does not consult it" ) XCTAssertEqual( DriveEjectorKit.verdict(for: descriptor(path: "/Volumes/C", name: "C", isRootFileSystem: false, isInternal: false, isRemovable: true, isEjectable: nil)), - .undetermined, - "An unknown ejectable flag must exclude the volume" + .eligible, + "A missing ejectable flag is not disqualifying: the rule does not consult it" ) } @@ -421,7 +470,7 @@ final class DriveEjectorKitTests: XCTestCase { func testBlockedReasonsReadAsSentencesAndEligibleHasNone() { XCTAssertNil(VolumeEjectionVerdict.eligible.blockedReason) - for verdict: VolumeEjectionVerdict in [.bootVolume, .internalDisk, .notRemovable, .undetermined] { + for verdict: VolumeEjectionVerdict in [.bootVolume, .internalDisk, .networkVolume, .undetermined] { let reason = verdict.blockedReason ?? "" XCTAssertFalse(reason.isEmpty, "Every blocking verdict needs an explanation to show the user") XCTAssertFalse(reason.contains("..."), "Typography: use a real ellipsis, never three dots") @@ -466,7 +515,9 @@ final class DriveEjectorKitTests: XCTestCase { XCTAssertNotEqual(volume.url.standardizedFileURL.path, "/", "The startup disk was offered") XCTAssertNotEqual(volume.isRootFileSystem, true, "The root filesystem was offered") XCTAssertNotEqual(volume.isInternal, true, "An internal disk was offered: \(volume.name)") - XCTAssertTrue(volume.isRemovable == true || volume.isEjectable == true) + XCTAssertNotEqual(volume.isLocal, false, "A network volume was offered: \(volume.name)") + // Deliberately NOT asserting isRemovable/isEjectable here. Real external drives + // report both as false, so requiring either is what made this tool list nothing. } } From f897416dde8c3d2854c9b916cd852402600ef2a7 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 00:13:55 +0530 Subject: [PATCH 5/7] DriveEjector: stop the settings sheet overflowing the panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Sources/DMonteCore/DriveEjectorSizing.swift | 13 ++++++ Sources/DMonteCore/DriveEjectorView.swift | 2 +- .../DriveEjectorSizingTests.swift | 44 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 Tests/DMonteCoreTests/DriveEjectorSizingTests.swift diff --git a/Sources/DMonteCore/DriveEjectorSizing.swift b/Sources/DMonteCore/DriveEjectorSizing.swift index 3bfb475..ab8f16b 100644 --- a/Sources/DMonteCore/DriveEjectorSizing.swift +++ b/Sources/DMonteCore/DriveEjectorSizing.swift @@ -6,6 +6,19 @@ public enum DriveEjectorSizing { return NSSize(width: (340 * scale).rounded(), height: (450 * scale).rounded()) } + /// The settings overlay, which is centred *over* the panel and therefore must never be wider + /// than it. A hard-coded width looks right only at `currentScale == 1`; on a Mac whose menu + /// bar is thinner than 26pt the panel shrinks and the sheet does not, so it overflows equally + /// on both sides and its first and last characters are clipped. Width is capped at the panel + /// so that cannot happen at any scale. + public static func settingsSize() -> NSSize { + let scale = currentScale + return NSSize( + width: min((340 * scale).rounded(), preferredSize().width), + height: (330 * scale).rounded() + ) + } + static var currentScale: CGFloat { let visibleFrame = NSScreen.main?.visibleFrame ?? NSRect(x: 0, y: 0, width: 1440, height: 900) let screenScale = visibleFrame.height / 950 diff --git a/Sources/DMonteCore/DriveEjectorView.swift b/Sources/DMonteCore/DriveEjectorView.swift index 7deaee3..c4153b9 100644 --- a/Sources/DMonteCore/DriveEjectorView.swift +++ b/Sources/DMonteCore/DriveEjectorView.swift @@ -354,7 +354,7 @@ private struct DriveEjectorSettingsView: View { Spacer() } .padding(20) - .frame(width: 340, height: 330) + .frame(width: DriveEjectorSizing.settingsSize().width, height: DriveEjectorSizing.settingsSize().height) } private func settingRow(title: String, @ViewBuilder trailing: () -> Trailing) -> some View { diff --git a/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift new file mode 100644 index 0000000..bce4365 --- /dev/null +++ b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift @@ -0,0 +1,44 @@ +import AppKit +import XCTest +@testable import DMonteCore + +/// The settings sheet is centred over the panel, so a sheet wider than the panel is clipped on +/// both sides — the title loses its first and last characters and the trailing switch runs off the +/// edge. That is invisible on a Mac where `currentScale` happens to be 1, which is why it shipped: +/// the sheet's size was a literal while the panel's was scaled. +final class DriveEjectorSizingTests: XCTestCase { + + func testSettingsSheetNeverExceedsThePanel() { + let panel = DriveEjectorSizing.preferredSize() + let settings = DriveEjectorSizing.settingsSize() + + XCTAssertLessThanOrEqual( + settings.width, panel.width, + "A settings sheet wider than the panel is clipped on both sides" + ) + XCTAssertLessThanOrEqual( + settings.height, panel.height, + "A settings sheet taller than the panel cannot show its bottom row — on this sheet, Quit" + ) + } + + /// Both sizes have to move together. Pinning only the panel would let a later edit reintroduce + /// a literal sheet width that passes the check above at today's scale and fails at another. + func testBothSizesScaleTogether() { + let scale = DriveEjectorSizing.currentScale + let panel = DriveEjectorSizing.preferredSize() + let settings = DriveEjectorSizing.settingsSize() + + XCTAssertEqual(panel.width, (340 * scale).rounded(), accuracy: 1) + XCTAssertEqual(settings.width, min((340 * scale).rounded(), panel.width), accuracy: 1) + XCTAssertEqual(settings.height, (330 * scale).rounded(), accuracy: 1) + } + + /// The scale is derived from live screen and menu-bar metrics, so the guarantee has to hold + /// across the whole range rather than at whatever this machine reports today. + func testScaleStaysWithinItsDocumentedBounds() { + let scale = DriveEjectorSizing.currentScale + XCTAssertGreaterThanOrEqual(scale, 0.82) + XCTAssertLessThanOrEqual(scale, 1.0) + } +} From d4193746332eee0d0f5982b8fde19004b8eac98f Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 00:17:53 +0530 Subject: [PATCH 6/7] Settings sheet: cap at the panel instead of scaling with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Sources/DMonteCore/DriveEjectorSizing.swift | 19 ++++++++++--------- .../DriveEjectorSizingTests.swift | 14 +++++++------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Sources/DMonteCore/DriveEjectorSizing.swift b/Sources/DMonteCore/DriveEjectorSizing.swift index ab8f16b..38cad3b 100644 --- a/Sources/DMonteCore/DriveEjectorSizing.swift +++ b/Sources/DMonteCore/DriveEjectorSizing.swift @@ -7,16 +7,17 @@ public enum DriveEjectorSizing { } /// The settings overlay, which is centred *over* the panel and therefore must never be wider - /// than it. A hard-coded width looks right only at `currentScale == 1`; on a Mac whose menu - /// bar is thinner than 26pt the panel shrinks and the sheet does not, so it overflows equally - /// on both sides and its first and last characters are clipped. Width is capped at the panel - /// so that cannot happen at any scale. + /// than it. A hard-coded size looks right only at `currentScale == 1`; on a Mac whose menu bar + /// is thinner than 26pt the panel shrinks and the sheet does not, so it overflows equally on + /// both sides and its first and last characters are clipped. + /// + /// Capped at the panel rather than scaled with it. The sheet's contents are laid out at this + /// size with unscaled padding, so shrinking it further than the panel demands would squeeze + /// them for no reason — and on the tools whose sheet is already narrower than their panel, + /// scaling would inset it noticeably while fixing nothing. public static func settingsSize() -> NSSize { - let scale = currentScale - return NSSize( - width: min((340 * scale).rounded(), preferredSize().width), - height: (330 * scale).rounded() - ) + let panel = preferredSize() + return NSSize(width: min(340, panel.width), height: min(330, panel.height)) } static var currentScale: CGFloat { diff --git a/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift index bce4365..cb8d589 100644 --- a/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift +++ b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift @@ -22,16 +22,16 @@ final class DriveEjectorSizingTests: XCTestCase { ) } - /// Both sizes have to move together. Pinning only the panel would let a later edit reintroduce - /// a literal sheet width that passes the check above at today's scale and fails at another. - func testBothSizesScaleTogether() { - let scale = DriveEjectorSizing.currentScale + /// The sheet should be as large as it was designed to be, shrinking only as far as the panel + /// forces. Asserting the contract rather than restating the arithmetic: when this rule changed + /// from "scale with the panel" to "cap at the panel", the tests that restated the formula + /// failed while the ones asserting the relationship kept passing. + func testSettingsUsesItsDesignSizeUnlessThePanelIsSmaller() { let panel = DriveEjectorSizing.preferredSize() let settings = DriveEjectorSizing.settingsSize() - XCTAssertEqual(panel.width, (340 * scale).rounded(), accuracy: 1) - XCTAssertEqual(settings.width, min((340 * scale).rounded(), panel.width), accuracy: 1) - XCTAssertEqual(settings.height, (330 * scale).rounded(), accuracy: 1) + XCTAssertEqual(settings.width, min(340, panel.width), accuracy: 1) + XCTAssertEqual(settings.height, min(330, panel.height), accuracy: 1) } /// The scale is derived from live screen and menu-bar metrics, so the guarantee has to hold From ac6c90512df41a6cfcfba2e6f3e0e2e7569c804f Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 02:39:30 +0530 Subject: [PATCH 7/7] Settings sheet: leave room for the overlay's 18pt padding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Sources/DMonteCore/DriveEjectorSizing.swift | 6 +++++- Tests/DMonteCoreTests/DriveEjectorSizingTests.swift | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Sources/DMonteCore/DriveEjectorSizing.swift b/Sources/DMonteCore/DriveEjectorSizing.swift index 38cad3b..087df69 100644 --- a/Sources/DMonteCore/DriveEjectorSizing.swift +++ b/Sources/DMonteCore/DriveEjectorSizing.swift @@ -17,7 +17,11 @@ public enum DriveEjectorSizing { /// scaling would inset it noticeably while fixing nothing. public static func settingsSize() -> NSSize { let panel = preferredSize() - return NSSize(width: min(340, panel.width), height: min(330, panel.height)) + // Leave room for the 18pt padding PreferencesOverlay adds around the sheet on every + // side: a sheet sized to the full panel becomes panel+36 once padded and spills + // the panel, dragging the content behind it off both edges. + let overlayChrome: CGFloat = 36 + return NSSize(width: min(340, panel.width - overlayChrome), height: min(330, panel.height - overlayChrome)) } static var currentScale: CGFloat { diff --git a/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift index cb8d589..c71a7b2 100644 --- a/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift +++ b/Tests/DMonteCoreTests/DriveEjectorSizingTests.swift @@ -30,8 +30,8 @@ final class DriveEjectorSizingTests: XCTestCase { let panel = DriveEjectorSizing.preferredSize() let settings = DriveEjectorSizing.settingsSize() - XCTAssertEqual(settings.width, min(340, panel.width), accuracy: 1) - XCTAssertEqual(settings.height, min(330, panel.height), accuracy: 1) + XCTAssertEqual(settings.width, min(340, panel.width - 36), accuracy: 1) + XCTAssertEqual(settings.height, min(330, panel.height - 36), accuracy: 1) } /// The scale is derived from live screen and menu-bar metrics, so the guarantee has to hold