From b514d6e4e38fcbdf3bba758729dc78ea6cf19e1a Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:30:49 -0700 Subject: [PATCH 1/3] refactor: remove dead ProgressBar view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unreferenced SwiftUI view in StatusView.swift — zero call sites across Sources/Tests/Resources (progress UI uses IndeterminateBar / LowSpaceBar / count-up heroes). Surfaced by a /prune audit. --- Sources/StatusView.swift | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Sources/StatusView.swift b/Sources/StatusView.swift index e37fe16d..40455960 100644 --- a/Sources/StatusView.swift +++ b/Sources/StatusView.swift @@ -432,21 +432,6 @@ struct LowSpaceBar: View { } } -struct ProgressBar: View { - let fraction: Double - let color: Color - var body: some View { - GeometryReader { g in - ZStack(alignment: .leading) { - Capsule().fill(Brand.trackFill) - Capsule().fill(color) - .frame(width: g.size.width * CGFloat(max(0, min(fraction, 1)))) - } - } - .frame(height: 6) - } -} - // MARK: - Bluetooth /// Connected Bluetooth devices with their battery — surfaced from mo's From 55c8be0066c401143fd09d5fd804fb097cc6e624 Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:30:49 -0700 Subject: [PATCH 2/3] refactor: dedupe size-string parsing into Fmt.parseSize MoleClient.parseSize and CleanList.parseSize were near-identical 1024-based size-string parsers. Consolidate into one canonical Fmt.parseSize (the inverse of Fmt.bytes); both keep their public names as thin forwarders, so all call sites and the MoleClient/CleanList test suites are unchanged. The canonical is the superset (raw-number fallback), so every existing assertion still holds. Surfaced by a /prune audit. --- Sources/CleanList.swift | 21 ++++++--------------- Sources/Format.swift | 15 +++++++++++++++ Sources/MoleClient.swift | 17 +++++------------ 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/Sources/CleanList.swift b/Sources/CleanList.swift index a361f436..fc4a7a01 100644 --- a/Sources/CleanList.swift +++ b/Sources/CleanList.swift @@ -107,21 +107,12 @@ struct CleanList: Equatable { summaryItemCount: summaryItems) } - /// "2.24GB" → bytes (1024-based, matching the engine's humanized - /// sizes closely enough for selection totals). - static func parseSize(_ text: String) -> Int64 { - let t = text.trimmingCharacters(in: .whitespaces).uppercased() - let units: [(String, Double)] = [ - ("TB", 1_099_511_627_776), ("GB", 1_073_741_824), - ("MB", 1_048_576), ("KB", 1024), ("B", 1), - ] - for (suffix, multiplier) in units where t.hasSuffix(suffix) { - let number = t.dropLast(suffix.count).trimmingCharacters(in: .whitespaces) - guard let value = Double(number) else { return 0 } - return Int64(value * multiplier) - } - return 0 - } + /// "2.24GB" → bytes (1024-based, matching the engine's humanized sizes + /// closely enough for selection totals). Forwards to the shared + /// `Fmt.parseSize` — the single source of truth, also used by + /// `MoleClient.parseSize`; kept as a named entry the streamed-line and + /// selection-total call sites (and `CleanListTests`) read by intent. + static func parseSize(_ text: String) -> Int64 { Fmt.parseSize(text) } /// Live count-up support (design 2.1): the bytes one streamed dry-run /// line contributes. Only per-item "…, dry" lines count; diff --git a/Sources/Format.swift b/Sources/Format.swift index 694cd3cb..a090efa7 100644 --- a/Sources/Format.swift +++ b/Sources/Format.swift @@ -21,6 +21,21 @@ enum Fmt { let s = (i == 0) ? "\(Int(v))" : String(format: v < 10 ? "%.2f" : "%.1f", v) return "\(s) \(units[i])" } + /// Inverse of `bytes`: parse a humanized size string ("1.5GB", "250MB", + /// "--", "junk") into bytes, using the same 1024-based multipliers. A + /// unit-suffixed value scales by its unit; a bare number is read as raw + /// bytes; anything unparseable (including "--" / "") yields 0. The single + /// home of the size parser shared by `MoleClient` and `CleanList` — pinned + /// by MoleClientTests + CleanListTests. + static func parseSize(_ s: String) -> Int64 { + let t = s.trimmingCharacters(in: .whitespaces).uppercased() + let units: [(String, Double)] = [("TB", 1_099_511_627_776), ("GB", 1_073_741_824), + ("MB", 1_048_576), ("KB", 1024), ("B", 1)] + for (u, mult) in units where t.hasSuffix(u) { + return Int64((Double(t.dropLast(u.count).trimmingCharacters(in: .whitespaces)) ?? 0) * mult) + } + return Int64(Double(t) ?? 0) + } /// Bytes → binary gigabytes. The only home of the 1_073_741_824 constant. static func gib(_ bytes: Double) -> Double { bytes / 1_073_741_824 } static func gib(_ bytes: UInt64) -> Double { Double(bytes) / 1_073_741_824 } diff --git a/Sources/MoleClient.swift b/Sources/MoleClient.swift index ce9935b3..342d8325 100644 --- a/Sources/MoleClient.swift +++ b/Sources/MoleClient.swift @@ -48,18 +48,11 @@ enum MoleClient { } } - /// Parse a human size string ("1.5GB", "250MB", "--") into bytes. - static func parseSize(_ s: String) -> Int64 { - let t = s.trimmingCharacters(in: .whitespaces).uppercased() - if t == "--" || t.isEmpty { return 0 } - let units: [(String, Double)] = [("TB", 1_099_511_627_776), ("GB", 1_073_741_824), - ("MB", 1_048_576), ("KB", 1024), ("B", 1)] - for (u, mult) in units where t.hasSuffix(u) { - let num = Double(t.dropLast(u.count).trimmingCharacters(in: .whitespaces)) ?? 0 - return Int64(num * mult) - } - return Int64(Double(t) ?? 0) - } + /// Parse a human size string ("1.5GB", "250MB", "--") into bytes. Forwards + /// to the shared `Fmt.parseSize` (single source of truth, shared with + /// `CleanList.parseSize`); kept as a named entry so the typed-row decode + /// above and `MoleClientTests` read by intent at the call site. + static func parseSize(_ s: String) -> Int64 { Fmt.parseSize(s) } // MARK: - Other commands (delegate to the existing tested parsers) From 8e15bfc185d0e3fa54698923e850f7cfb3fc24d7 Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:30:49 -0700 Subject: [PATCH 3/3] refactor(mo): drop unused MoEngine.stream()/runElevatedClassified() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both facade methods were unused in production: OperationFlow takes the streaming port via the exposed MoEngine.shared.streamPort, and elevation runs through MoleCLI.runElevatedClassified (SettingsView) — they were exercised only by MoEngineTests. Remove them, plus the now-orphaned privilegeBroker dependency and the MoLocator.locateTrusted() seam they alone used, so no new dead code is left behind. The three deleted MoEngine elevation tests are fully covered against the production path by PrivilegeBrokerTests (.launchFailed / authCancelled / exited) and MoleCLITests (trustedExecutable never resolves from PATH). Header/docs updated to match. Surfaced by a /prune audit. --- Sources/MoEngine.swift | 101 +++++++++++++------------------------- Tests/MoEngineTests.swift | 87 +++++++------------------------- 2 files changed, 50 insertions(+), 138 deletions(-) diff --git a/Sources/MoEngine.swift b/Sources/MoEngine.swift index f0331386..a0279a06 100644 --- a/Sources/MoEngine.swift +++ b/Sources/MoEngine.swift @@ -2,29 +2,29 @@ // MoEngine.swift // Burrow // -// The single entry point to the `mo` runners (issue #48). Burrow grew three +// The single entry point to the `mo` runners (issue #48). Burrow grew several // process shapes — capture (small one-shot commands), streaming (clean / -// optimize), and PTY (purge / installer) — plus a one-shot elevated path, -// each found and spawned at its own call site. `MoEngine` is the ONE facade -// callers reach for so "how do I run mo?" has a single answer. +// optimize), and interactive PTY (purge / installer) — each found and spawned +// at its own call site. `MoEngine` is the ONE facade callers reach for so +// "how do I run mo?" has a single answer. // -// All FOUR process shapes now hang off this one facade (issue #48 complete): -// CAPTURE + ELEVATED + DISCOVERY (slice 2) plus STREAMING (clean / optimize) -// and interactive PTY (purge / installer), added here. Every shape delegates -// to the existing, tested port — the facade does NOT reimplement any spawn, -// elevation, or PTY internals; it just funnels "how do I run mo?" to one type. +// Three shapes hang off the facade as methods — CAPTURE, DISCOVERY, and +// interactive PTY — each delegating to the existing, tested port; the facade +// does NOT reimplement any spawn or PTY internals. STREAMING is reached +// through the EXPOSED `streamPort` (not a wrapper method): `OperationFlow` +// holds the `any ProcessPort` to drive its own reduce/notify/auth-cancel loop, +// so the facade hands it the production port rather than the stream. The +// one-shot ELEVATED path is NOT on the facade — it stays in +// `MoleCLI.runElevatedClassified` (trusted-location resolution + the shared +// `PrivilegeBroker`), the path production has always used. // // Behavior is preserved exactly: a `capture(_:)` call produces the same argv, -// stdin, environment, timeout, and result fields that `MoleCLI.run` did; -// `runElevatedClassified` routes through the same `PrivilegeBroker` against -// the same trusted-location resolution; `stream(_:)` hands back the SAME -// `AsyncStream` `SystemProcessPort` already produced (the -// elevated-osascript-tail, auth-cancel classification, and line-splitting are -// untouched); and `interactive()` vends a FRESH `PTYTask` whose raw, -// escape-preserving output the `SelectionSession` reducer depends on. The -// ports are injected (production defaults are the real ones) so every shape is -// testable with scripted fakes, matching the seams `MoleCLI`/`MoleProcess`/ -// `PrivilegeBroker`/`ProcessPort`/`PTYPort` already expose. +// stdin, environment, timeout, and result fields that `MoleCLI.run` did, and +// `interactive()` vends a FRESH `PTYTask` whose raw, escape-preserving output +// the `SelectionSession` reducer depends on. The ports are injected +// (production defaults are the real ones) so every shape is testable with +// scripted fakes, matching the seams `MoleCLI`/`MoleProcess`/`ProcessPort`/ +// `PTYPort` already expose. // import Foundation @@ -89,37 +89,32 @@ enum Availability: Equatable { } /// Discovery seam. Production resolves through `MoleCLI` (PATH + known -/// locations, cached + revalidated for normal lookups; trusted-locations-only -/// for elevated runs); tests inject a fake to drive resolution deterministically. +/// locations, cached + revalidated); tests inject a fake to drive resolution +/// deterministically. protocol MoLocator: Sendable { /// The `mo` for a normal (unelevated) run — PATH allowed. func locate() -> String? - /// The `mo` for an ELEVATED run — known install locations ONLY, never a - /// user-writable PATH entry (running an attacker-shadowed binary as root is - /// the whole threat model). - func locateTrusted() -> String? } /// The production locator: delegates to `MoleCLI`'s existing discovery so the -/// caching/revalidation and trusted-only semantics stay in one place. +/// caching/revalidation stays in one place. struct SystemMoLocator: MoLocator { func locate() -> String? { MoleCLI.findExecutable() } - func locateTrusted() -> String? { MoleCLI.trustedExecutable() } } // MARK: - Facade -/// The one runner facade. All four `mo` process shapes — capture, elevated -/// one-shot, streaming, and interactive PTY — plus discovery hang off this -/// type; the ports are injected so every path is testable in memory. +/// The one runner facade. The `mo` process shapes callers reach for — capture, +/// discovery, and interactive PTY — hang off this type as methods, and the +/// streaming port is exposed for `OperationFlow`; the ports are injected so +/// every path is testable in memory. final class MoEngine { private let processPort: MoleProcessPort - private let privilegeBroker: PrivilegeBroker private let locator: MoLocator - /// The streaming-op spawn port (clean / optimize). Exposed (not just used - /// by `stream(_:)`) so `OperationFlow`, which holds an `any ProcessPort` - /// and drives its own reduce/notify/auth-cancel loop, can take the facade's - /// production port as its default without the facade reaching into that loop. + /// The streaming-op spawn port (clean / optimize). Exposed so + /// `OperationFlow`, which holds an `any ProcessPort` and drives its own + /// reduce/notify/auth-cancel loop, can take the facade's production port as + /// its default without the facade reaching into that loop. let streamPort: ProcessPort /// Vends a FRESH interactive PTY session per call. A factory, not a shared /// instance, because a `PTYTask` is stateful per launch and each selection @@ -127,19 +122,16 @@ final class MoEngine { /// would stomp each other's child and keystrokes. private let makePTY: @Sendable () -> PTYPort - /// Production singleton. Wraps the real capture runner, the real osascript - /// broker, `MoleCLI` discovery, the real streaming port, and the real PTY — - /// the exact spawn paths the migrated call sites used before, just funneled - /// through one type. + /// Production singleton. Wraps the real capture runner, `MoleCLI` discovery, + /// the real streaming port, and the real PTY — the exact spawn paths the + /// migrated call sites used before, just funneled through one type. static let shared = MoEngine() init(processPort: MoleProcessPort = SystemMoleProcess(), - privilegeBroker: PrivilegeBroker = SystemPrivilegeBroker(), locator: MoLocator = SystemMoLocator(), streamPort: ProcessPort = SystemProcessPort(), makePTY: @escaping @Sendable () -> PTYPort = { PTYTask() }) { self.processPort = processPort - self.privilegeBroker = privilegeBroker self.locator = locator self.streamPort = streamPort self.makePTY = makePTY @@ -189,33 +181,6 @@ final class MoEngine { ) } - // MARK: Elevated one-shot - - /// Run `mo ` ONCE with administrator rights, returning the classified - /// outcome (`.authCancelled` distinguished from a command that ran and - /// failed). Resolves through the TRUSTED locations only — never PATH — and - /// routes through the same `PrivilegeBroker` the one-shot config commands - /// (`touchid enable/disable`) already use. No `mo` in a trusted spot → - /// `.launchFailed`, matching the old guard. - func runElevatedClassified(args: [String]) -> ElevatedOutcome { - guard let mo = locator.locateTrusted() else { return .launchFailed } - return privilegeBroker.openElevated(executable: mo, args: args) - } - - // MARK: Streaming (clean / optimize) - - /// Stream a long-running `mo` op line-by-line. Hands the spec straight to - /// the injected streaming port and returns its `AsyncStream` - /// unchanged — the plain/elevated spawn, the osascript temp-log tail, the - /// ANSI strip + line split, and the auth-cancel classification all stay in - /// `SystemProcessPort`. Cancelling the consuming task still terminates the - /// child via the stream's `onTermination`, exactly as before. The caller - /// (`OperationFlow`) owns the reduce/notify/cancel loop; this is only the - /// spawn source. - func stream(_ spec: ProcessSpec) -> AsyncStream { - streamPort.events(spec) - } - // MARK: Interactive PTY (purge / installer) /// Open a FRESH interactive PTY session for a selection TUI (`mo purge` / diff --git a/Tests/MoEngineTests.swift b/Tests/MoEngineTests.swift index ad941dc3..0bf6d86d 100644 --- a/Tests/MoEngineTests.swift +++ b/Tests/MoEngineTests.swift @@ -2,20 +2,20 @@ // MoEngineTests.swift // BurrowTests // -// Boundary tests for the unified runner facade (issue #48, slice 2). The -// capture + elevated + discovery entry points all delegate to injected ports, -// so they're driven here with scripted fakes — same seam style as -// MoleProcessTests (capture port) and PrivilegeBrokerTests (elevation port). +// Boundary tests for the unified runner facade (issue #48). The capture + +// discovery entry points delegate to injected ports, so they're driven here +// with scripted fakes — same seam style as MoleProcessTests (capture port). +// (The one-shot elevated path is NOT on the facade; its wiring is covered by +// PrivilegeBrokerTests against `MoleCLI.runElevatedClassified`.) // // The point of these tests is the WIRING: that a `MoCommand` lands on the -// capture runner as the exact argv/stdin/env/timeout it described, that a +// capture runner as the exact argv/stdin/env/timeout it described, and that a // `.mo` target resolves through the locator (and degrades to a clean nonzero -// exit when `mo` is missing), and that the elevated path routes the TRUSTED -// binary to the broker — never a PATH lookup. The streaming and PTY shapes -// (slice 3, completing #48) are wired the same way: `stream(_:)` forwards the -// spec to the injected `ProcessPort` and returns its events untouched, and -// `interactive()` vends a FRESH session from the injected PTY factory each -// call — so two hosts can never share one stateful pty. +// exit when `mo` is missing). The streaming and PTY shapes are wired the same +// way: the exposed `streamPort` forwards the spec to the injected +// `ProcessPort` and returns its events untouched, and `interactive()` vends a +// FRESH session from the injected PTY factory each call — so two hosts can +// never share one stateful pty. // import XCTest @@ -52,26 +52,11 @@ final class MoEngineTests: XCTestCase { } } - /// Canned discovery: the normal lookup and the trusted-only lookup are - /// separately settable so a test can prove the elevated path takes the - /// trusted one. + /// Canned discovery: the normal lookup is settable so a test can drive + /// resolution deterministically. private struct FakeLocator: MoLocator { var located: String? - var trusted: String? func locate() -> String? { located } - func locateTrusted() -> String? { trusted } - } - - /// In-memory stand-in for osascript: records the elevated (exe, args) and - /// replays a canned outcome — no auth dialog. - private final class FakeBroker: PrivilegeBroker, @unchecked Sendable { - private(set) var calls: [(executable: String, args: [String])] = [] - var outcome: ElevatedOutcome - init(outcome: ElevatedOutcome) { self.outcome = outcome } - func openElevated(executable: String, args: [String]) -> ElevatedOutcome { - calls.append((executable, args)) - return outcome - } } /// Records the streamed spec and replays a canned event script — no real @@ -213,44 +198,6 @@ final class MoEngineTests: XCTestCase { XCTAssertEqual(engine.availability(), .missing) } - // MARK: - Elevated one-shot - - func testRunElevatedClassified_routesTrustedBinaryToTheBroker() { - let broker = FakeBroker(outcome: .exited(0)) - // The normal lookup points one place; the elevated path must use the - // TRUSTED lookup — never PATH — so a shadowed binary can't get root. - let engine = MoEngine(privilegeBroker: broker, - locator: FakeLocator(located: "/untrusted/mo", - trusted: "/opt/homebrew/bin/mo")) - - let outcome = engine.runElevatedClassified(args: ["touchid", "enable"]) - - XCTAssertEqual(outcome, .exited(0)) - XCTAssertEqual(broker.calls.count, 1) - XCTAssertEqual(broker.calls.first?.executable, "/opt/homebrew/bin/mo", - "elevated runs resolve through the trusted list, never the normal lookup") - XCTAssertEqual(broker.calls.first?.args, ["touchid", "enable"]) - } - - func testRunElevatedClassified_noTrustedBinaryIsLaunchFailedAndNeverReachesTheBroker() { - let broker = FakeBroker(outcome: .exited(0)) - let engine = MoEngine(privilegeBroker: broker, - locator: FakeLocator(located: "/untrusted/mo", trusted: nil)) - - XCTAssertEqual(engine.runElevatedClassified(args: ["touchid", "enable"]), .launchFailed) - XCTAssertTrue(broker.calls.isEmpty, "a missing trusted mo must never reach the elevation spawn") - } - - func testRunElevatedClassified_authCancelIsDistinctFromCommandFailure() { - let cancel = MoEngine(privilegeBroker: FakeBroker(outcome: .authCancelled), - locator: FakeLocator(trusted: "/opt/homebrew/bin/mo")) - XCTAssertEqual(cancel.runElevatedClassified(args: ["touchid", "enable"]), .authCancelled) - - let failed = MoEngine(privilegeBroker: FakeBroker(outcome: .exited(2)), - locator: FakeLocator(trusted: "/opt/homebrew/bin/mo")) - XCTAssertEqual(failed.runElevatedClassified(args: ["touchid", "disable"]), .exited(2)) - } - // MARK: - Streaming (clean / optimize) func testStream_forwardsTheSpecToThePortUntouched() { @@ -260,7 +207,7 @@ final class MoEngineTests: XCTestCase { let spec = ProcessSpec(executable: "/usr/local/bin/mo", arguments: ["clean", "--dry-run"], stdin: nil, elevated: false, timeout: 30) - _ = engine.stream(spec) + _ = engine.streamPort.events(spec) // The facade is a pass-through: the port sees the exact spec, never a // rewritten one. (Argv/elevation are destructive-path inputs — they @@ -279,7 +226,7 @@ final class MoEngineTests: XCTestCase { var lines: [String] = [] var exit: Int32? - for await event in engine.stream(ProcessSpec(executable: "/usr/local/bin/mo", + for await event in engine.streamPort.events(ProcessSpec(executable: "/usr/local/bin/mo", arguments: ["clean"], stdin: nil, elevated: false, timeout: nil)) { switch event { @@ -300,7 +247,7 @@ final class MoEngineTests: XCTestCase { let engine = MoEngine(streamPort: port) var sawAuthCancel = false - for await event in engine.stream(ProcessSpec(executable: "/usr/local/bin/mo", + for await event in engine.streamPort.events(ProcessSpec(executable: "/usr/local/bin/mo", arguments: ["clean"], stdin: nil, elevated: true, timeout: nil)) { if case .authCancelled = event { sawAuthCancel = true } @@ -315,7 +262,7 @@ final class MoEngineTests: XCTestCase { let engine = MoEngine() var lines: [String] = [] var exit: Int32? - for await event in engine.stream(ProcessSpec(executable: "/bin/sh", + for await event in engine.streamPort.events(ProcessSpec(executable: "/bin/sh", arguments: ["-c", "printf 'a\\nb\\n'; exit 0"], stdin: nil, elevated: false, timeout: nil)) { switch event {