diff --git a/macos/Modore/Sources/Modore/Models/ScanModels.swift b/macos/Modore/Sources/Modore/Models/ScanModels.swift index 70b6ee2..ecc9c9c 100644 --- a/macos/Modore/Sources/Modore/Models/ScanModels.swift +++ b/macos/Modore/Sources/Modore/Models/ScanModels.swift @@ -330,11 +330,15 @@ struct DevtoolUpdateRow: Identifiable { let name: String let current: String let latest: String + /// `brew pin`ned: an update exists but the owner deliberately held this + /// package back, so listing it without saying so reads as a missed update. + let pinned: Bool init?(json: [String: Any]) { name = JsonRead.string(json, "name") current = JsonRead.string(json, "current") latest = JsonRead.string(json, "latest") + pinned = JsonRead.bool(json, "pinned") ?? false guard !name.isEmpty, !current.isEmpty, !latest.isEmpty else { return nil } } } diff --git a/macos/Modore/Sources/Modore/Models/StorageModels.swift b/macos/Modore/Sources/Modore/Models/StorageModels.swift index a8cc6af..5ba9fc3 100644 --- a/macos/Modore/Sources/Modore/Models/StorageModels.swift +++ b/macos/Modore/Sources/Modore/Models/StorageModels.swift @@ -209,13 +209,20 @@ struct StorageItem: Identifiable { risk = json["risk"] as? String ?? "unknown" kind = json["kind"] as? String ?? "unknown" label = json["label"] as? String ?? kind + // A non-finite size propagates: it poisons every sum it enters, makes + // the goal slider's range comparison false (`1 <= NaN`), and trips + // ClosedRange's precondition -- a full-screen crash traceable to one + // field. `Double("1e999")` and a bare 1e999 in JSON both produce one, + // so treat it as unmeasured rather than trusting the producer. + let rawSize: Double if let number = json["sizeGB"] as? NSNumber { - sizeGB = number.doubleValue + rawSize = number.doubleValue } else if let string = json["sizeGB"] as? String { - sizeGB = Double(string) ?? 0 + rawSize = Double(string) ?? 0 } else { - sizeGB = 0 + rawSize = 0 } + sizeGB = rawSize.isFinite ? rawSize : 0 path = json["path"] as? String ?? "" action = json["action"] as? String ?? "확인 필요" note = json["note"] as? String ?? "" diff --git a/macos/Modore/Sources/Modore/Services/LoginItemService.swift b/macos/Modore/Sources/Modore/Services/LoginItemService.swift index 9a48acb..22e9ad1 100644 --- a/macos/Modore/Sources/Modore/Services/LoginItemService.swift +++ b/macos/Modore/Sources/Modore/Services/LoginItemService.swift @@ -126,8 +126,15 @@ enum LoginItemService { } extension ScanModel { + // `isBusy` as well as the per-action flag: the Security page stays + // interactive during a scan, and confirming a removal kicks off its own + // rescan. Without this a right-click removal mid-scan started a second + // ScanPipeline writing the same scan_result.json/report files as the + // first, with two finishRun()s racing over whichever mix survived -- + // and the second run isn't held in `scanTask`, so 검사 취소 could not + // stop it. prepareCleanup already guards this way. func previewLoginItemRemoval(_ name: String) { - guard loginItemActionInFlight == nil else { return } + guard !isBusy, loginItemActionInFlight == nil else { return } loginItemActionInFlight = name errorMessage = nil let root = projectRoot @@ -145,7 +152,7 @@ extension ScanModel { } func confirmLoginItemRemoval() { - guard let pending = pendingLoginItemRemoval, loginItemActionInFlight == nil else { return } + guard !isBusy, let pending = pendingLoginItemRemoval, loginItemActionInFlight == nil else { return } pendingLoginItemRemoval = nil loginItemActionInFlight = pending.name let root = projectRoot diff --git a/macos/Modore/Sources/Modore/Services/MothballService.swift b/macos/Modore/Sources/Modore/Services/MothballService.swift index 5fe8526..58b0511 100644 --- a/macos/Modore/Sources/Modore/Services/MothballService.swift +++ b/macos/Modore/Sources/Modore/Services/MothballService.swift @@ -43,11 +43,19 @@ enum MothballService { .sorted { $0.repo.sizeBytes > $1.repo.sizeBytes } } - static func scanCandidates(lineagePaths: [ScreeLineagePath]) async -> [ArchiveCandidate] { + /// `scanReport`, not `scan`: the latter drops the inspection failures, + /// and MothballCore's own API comment warns why that matters -- a repo + /// found but not inspectable (corrupt .git, permission denied, git + /// timeout) would otherwise be indistinguishable from no repo at all, and + /// the page would state "nothing worth archiving" when the truth is that + /// it could not look. + static func scanCandidates( + lineagePaths: [ScreeLineagePath] + ) async -> (candidates: [ArchiveCandidate], failureCount: Int) { let roots = candidateRoots(from: lineagePaths) - guard !roots.isEmpty else { return [] } - let repos = await RepoScanner().scan(roots: roots) - return rankCandidates(repos: repos) + guard !roots.isEmpty else { return ([], 0) } + let report = await RepoScanner().scanReport(roots: roots) + return (rankCandidates(repos: report.repos), report.failures.count) } } @@ -63,7 +71,9 @@ extension ScanModel { let paths = report.lineagePaths Task { defer { archiveLoading = false } - archiveCandidates = await MothballService.scanCandidates(lineagePaths: paths) + let outcome = await MothballService.scanCandidates(lineagePaths: paths) + archiveCandidates = outcome.candidates + archiveInspectionFailures = outcome.failureCount } } } diff --git a/macos/Modore/Sources/Modore/Services/ObservationService.swift b/macos/Modore/Sources/Modore/Services/ObservationService.swift index 0401f71..911ad59 100644 --- a/macos/Modore/Sources/Modore/Services/ObservationService.swift +++ b/macos/Modore/Sources/Modore/Services/ObservationService.swift @@ -126,7 +126,9 @@ enum ObservationService { extension ScanModel { func observeNow(windowSeconds: Int) { - guard !observationInFlight else { return } + // Symmetric with isBusy including observationInFlight: neither side + // may run underneath the other, or each measures the other's work. + guard !isBusy, !observationInFlight else { return } observationInFlight = true observationErrorMessage = nil let root = projectRoot @@ -136,6 +138,10 @@ extension ScanModel { case .ready(let result): observationResult = result case .failure(let message): + // Drop the previous run's rows: keeping them left the header + // reporting "N초 관찰됨" from an older window while the body + // showed this run's failure. + observationResult = nil observationErrorMessage = message } } diff --git a/macos/Modore/Sources/Modore/Services/ScanModel.swift b/macos/Modore/Sources/Modore/Services/ScanModel.swift index 4549e7b..619624c 100644 --- a/macos/Modore/Sources/Modore/Services/ScanModel.swift +++ b/macos/Modore/Sources/Modore/Services/ScanModel.swift @@ -34,6 +34,7 @@ final class ScanModel: ObservableObject { @Published var screeError: String? @Published var screePreserveInFlightSource: String? @Published var archiveCandidates: [ArchiveCandidate]? + @Published var archiveInspectionFailures = 0 @Published var archiveLoading = false @Published var pendingLoginItemRemoval: PendingLoginItemRemoval? @Published var loginItemActionInFlight: String? @@ -67,6 +68,12 @@ final class ScanModel: ObservableObject { || cleanupInFlight || browserAutomationStopInFlight || storageWatchInFlight + // An observation measures what this Mac is doing on its own. A + // scan or cleanup started underneath it lands in its own results + // -- the scanner's du/lsof become the top "real CPU use" rows and + // VirusTotal lookups appear as new connections -- so the window + // would report the app's own work as the finding. + || observationInFlight || resultLoading } var logText: String { logStore.text } diff --git a/macos/Modore/Sources/Modore/Views/ActivityView.swift b/macos/Modore/Sources/Modore/Views/ActivityView.swift index f177453..106a800 100644 --- a/macos/Modore/Sources/Modore/Views/ActivityView.swift +++ b/macos/Modore/Sources/Modore/Views/ActivityView.swift @@ -79,6 +79,9 @@ private struct StorageWatchActivitySection: View { private struct ContinuousObservationSection: View { @EnvironmentObject private var model: ScanModel @State private var windowSeconds = 60 + /// The window the in-flight run was actually started with, so the + /// progress caption cannot drift from it. + @State private var runningWindowSeconds = 60 var body: some View { Section { @@ -91,19 +94,23 @@ private struct ContinuousObservationSection: View { } .labelsHidden() .frame(maxWidth: 140) + // Left enabled mid-run, the picker rewrote the caption below + // to a window the run in progress is not actually using. + .disabled(model.observationInFlight) Spacer() Button(model.observationInFlight ? "관찰 중…" : "지금 관찰하기") { + runningWindowSeconds = windowSeconds model.observeNow(windowSeconds: windowSeconds) } .buttonStyle(.bordered) - .disabled(model.observationInFlight) + .disabled(model.isBusy || model.observationInFlight) } if model.observationInFlight { HStack(spacing: 8) { ProgressView() .controlSize(.small) - Text("\(windowSeconds)초 동안 CPU와 네트워크를 관찰하는 중입니다…") + Text("\(runningWindowSeconds)초 동안 CPU와 네트워크를 관찰하는 중입니다…") .font(.callout) .foregroundStyle(.secondary) } diff --git a/macos/Modore/Sources/Modore/Views/MothballView.swift b/macos/Modore/Sources/Modore/Views/MothballView.swift index 02cb736..b532811 100644 --- a/macos/Modore/Sources/Modore/Views/MothballView.swift +++ b/macos/Modore/Sources/Modore/Views/MothballView.swift @@ -56,7 +56,10 @@ struct MothballPage: View { } if let candidates = model.archiveCandidates { - MothballCandidateSection(candidates: candidates) + MothballCandidateSection( + candidates: candidates, + inspectionFailures: model.archiveInspectionFailures + ) } } .macSettingsFormStyle() @@ -65,11 +68,19 @@ struct MothballPage: View { private struct MothballCandidateSection: View { let candidates: [ArchiveCandidate] + let inspectionFailures: Int var body: some View { Section { if candidates.isEmpty { - Text("보관할 만한 저장소가 없습니다.") + // "None found" and "could not look" are different answers. + Text(inspectionFailures > 0 + ? "저장소 \(inspectionFailures)개를 검사하지 못해 보관 후보를 판단할 수 없습니다." + : "보관할 만한 저장소가 없습니다.") + .foregroundStyle(.secondary) + } else if inspectionFailures > 0 { + Text("저장소 \(inspectionFailures)개는 검사하지 못했습니다. 아래 목록은 확인된 것만입니다.") + .font(.callout) .foregroundStyle(.secondary) } ForEach(candidates) { candidate in diff --git a/macos/Modore/Sources/Modore/Views/SecurityView.swift b/macos/Modore/Sources/Modore/Views/SecurityView.swift index bb48072..68b58ce 100644 --- a/macos/Modore/Sources/Modore/Views/SecurityView.swift +++ b/macos/Modore/Sources/Modore/Views/SecurityView.swift @@ -174,9 +174,11 @@ struct SecurityPage: View { DisclosureGroup(isExpanded: $showsDevtoolUpdates) { ForEach(model.devtoolUpdateRows) { row in SecurityDetailRow( - symbol: "shippingbox", + symbol: row.pinned ? "pin" : "shippingbox", title: row.name, - detail: "\(row.current) → \(row.latest)" + detail: row.pinned + ? "\(row.current) → \(row.latest) · 고정해 둔 패키지입니다" + : "\(row.current) → \(row.latest)" ) } } label: { diff --git a/macos/Modore/Sources/Modore/Views/SpaceGoalView.swift b/macos/Modore/Sources/Modore/Views/SpaceGoalView.swift index df7162d..acb8aaa 100644 --- a/macos/Modore/Sources/Modore/Views/SpaceGoalView.swift +++ b/macos/Modore/Sources/Modore/Views/SpaceGoalView.swift @@ -7,18 +7,30 @@ import SwiftUI /// regardless of the scan's original ordering -- not an exact/optimal subset /// sum, just a simple, explainable greedy-largest-first rule. enum SpaceGoalSelection { + /// Sizes arrive rounded to a tenth of a GB, and a tenth is not exact in + /// binary: four items truly summing to 3.0 add up to 2.9999999999999996, + /// so a bare `>=` walked past the exact-match set and appended one more + /// item than the goal needed, then reported the result as short of it. + private static let goalTolerance = 0.000_001 + static func select(from candidates: [StorageItem], targetGB: Double) -> [StorageItem] { guard targetGB > 0 else { return [] } let eligible = candidates .filter(\.canCleanup) .sorted { lhs, rhs in if lhs.sizeGB != rhs.sizeGB { return lhs.sizeGB > rhs.sizeGB } - return lhs.label < rhs.label + if lhs.label != rhs.label { return lhs.label < rhs.label } + // Same size and same label still has to resolve to one fixed + // order, or the "same set regardless of scan order" promise + // above is only true until two rows collide -- which they do: + // label falls back to `kind`, so two same-size rows of one + // kind tie. Paths are unique per row. + return lhs.path < rhs.path } var selected: [StorageItem] = [] var total = 0.0 for item in eligible { - if total >= targetGB { break } + if total >= targetGB - goalTolerance { break } selected.append(item) total += item.sizeGB } diff --git a/macos/Modore/Tests/ModoreTests/DevtoolUpdateRowTests.swift b/macos/Modore/Tests/ModoreTests/DevtoolUpdateRowTests.swift index 79cda1d..3c71b73 100644 --- a/macos/Modore/Tests/ModoreTests/DevtoolUpdateRowTests.swift +++ b/macos/Modore/Tests/ModoreTests/DevtoolUpdateRowTests.swift @@ -28,4 +28,25 @@ final class DevtoolUpdateRowTests: XCTestCase { XCTAssertNil(DevtoolUpdateRow(json: ["name": "foo", "current": "1.0"])) XCTAssertNil(DevtoolUpdateRow(json: [:])) } + + // brew appends " [pinned at X]" for a pinned formula or cask. The row is + // kept (dropping it made the collector's count disagree with the list) + // and flagged, since "an update exists" reads differently for a package + // the owner deliberately held back. + func testDecodesAPinnedPackage() throws { + let row = try XCTUnwrap(DevtoolUpdateRow(json: [ + "name": "node", "current": "18.0.0", "latest": "20.0.0", "pinned": true, + ])) + + XCTAssertTrue(row.pinned) + XCTAssertEqual(row.latest, "20.0.0") + } + + func testDefaultsToNotPinnedWhenTheFieldIsAbsent() throws { + let row = try XCTUnwrap(DevtoolUpdateRow(json: [ + "name": "ada-url", "current": "3.4.4", "latest": "4.0.0", + ])) + + XCTAssertFalse(row.pinned) + } } diff --git a/macos/Modore/Tests/ModoreTests/SpaceGoalSelectionTests.swift b/macos/Modore/Tests/ModoreTests/SpaceGoalSelectionTests.swift index 38a8400..8b6b510 100644 --- a/macos/Modore/Tests/ModoreTests/SpaceGoalSelectionTests.swift +++ b/macos/Modore/Tests/ModoreTests/SpaceGoalSelectionTests.swift @@ -6,14 +6,15 @@ final class SpaceGoalSelectionTests: XCTestCase { label: String, sizeGB: Double, cleanupID: String = "npm_cache", - measureStatus: String = "ok" + measureStatus: String = "ok", + path: String? = nil ) -> StorageItem { StorageItem(json: [ "risk": "warning", "kind": "cache", "label": label, "sizeGB": sizeGB, - "path": "/tmp/\(label)", + "path": path ?? "/tmp/\(label)", "action": "정리", "note": "", "measureStatus": measureStatus, @@ -101,4 +102,68 @@ final class SpaceGoalSelectionTests: XCTestCase { XCTAssertEqual(SpaceGoalSelection.select(from: items, targetGB: 0).count, 0) XCTAssertEqual(SpaceGoalSelection.select(from: items, targetGB: -1).count, 0) } + + // The doc comment promises the same candidate set yields the same + // selection regardless of scan order. Size and label alone do not + // guarantee that: `label` falls back to `kind`, so two same-size rows of + // one kind tie completely and the result then depended on emit order. + func testIdenticalSizeAndLabelStillResolveToOneFixedOrder() { + let first = item(label: "cache", sizeGB: 0.5, path: "/tmp/a") + let second = item(label: "cache", sizeGB: 0.5, path: "/tmp/b") + + let forward = SpaceGoalSelection.select(from: [first, second], targetGB: 1) + let reversed = SpaceGoalSelection.select(from: [second, first], targetGB: 1) + + XCTAssertEqual(forward.map(\.path), reversed.map(\.path)) + XCTAssertEqual(forward.map(\.path), ["/tmp/a", "/tmp/b"]) + } + + func testSelectionOrderIsIndependentOfInputOrderAtTheGoalBoundary() { + // Only one of the two tied 0.5GB rows is needed to cross the goal, so + // which one gets picked is exactly where input order used to leak. + let big = item(label: "big", sizeGB: 2.5) + let tiedA = item(label: "cache", sizeGB: 0.5, path: "/tmp/a") + let tiedB = item(label: "cache", sizeGB: 0.5, path: "/tmp/b") + + let forward = SpaceGoalSelection.select(from: [big, tiedA, tiedB], targetGB: 3) + let reversed = SpaceGoalSelection.select(from: [tiedB, tiedA, big], targetGB: 3) + + XCTAssertEqual(forward.count, 2) + XCTAssertEqual(forward.map(\.path), reversed.map(\.path)) + } + + // Sizes arrive rounded to a tenth, and a tenth is not exact in binary. + // 2.4 + 0.3 + 0.3 is exactly 3.0 in decimal but accumulates to + // 2.9999999999999996 in Double, so a bare `>=` walked past the set that + // actually meets the goal and appended a fourth item -- then reported the + // result as short of the goal it had in fact reached. + func testExactlyMetGoalDoesNotPickUpAnExtraItemFromFloatError() { + let items = [ + item(label: "a", sizeGB: 2.4), + item(label: "b", sizeGB: 0.3), + item(label: "c", sizeGB: 0.3), + item(label: "d", sizeGB: 0.2), + ] + + let selected = SpaceGoalSelection.select(from: items, targetGB: 3) + + XCTAssertEqual(selected.map(\.label), ["a", "b", "c"]) + } + + // A non-finite size poisons every sum it enters and makes the goal + // slider's range precondition trap; it is treated as unmeasured instead. + func testNonFiniteSizeIsTreatedAsUnmeasured() { + let poisoned = StorageItem(json: [ + "risk": "warning", + "kind": "cache", + "label": "poisoned", + "sizeGB": "1e999", + "path": "/tmp/poisoned", + "measureStatus": "ok", + "cleanupId": "npm_cache", + ])! + + XCTAssertEqual(poisoned.sizeGB, 0) + XCTAssertTrue(poisoned.sizeGB.isFinite) + } } diff --git a/scripts/login_items.sh b/scripts/login_items.sh index 7b30184..23b2c93 100755 --- a/scripts/login_items.sh +++ b/scripts/login_items.sh @@ -75,8 +75,19 @@ applescript_escape() { /usr/bin/printf '%s' "$value" } +# One name per line, not osascript's default ", " list serialization. That +# default is ambiguous: a single item named "Backup, Inc." serializes exactly +# like two items named "Backup" and "Inc.", so such an item could never be +# matched (permanently unremovable), and a surviving "Foo, Bar" made a +# genuinely successful removal of a separate "Foo" report as failed. A +# newline delimiter cannot occur inside a name -- names carrying tab/newline +# are rejected outright before they ever reach this. +LOGIN_ITEM_NAMES_SCRIPT='set text item delimiters to linefeed +tell application "System Events" to set itemNames to name of every login item +return itemNames as text' + current_login_item_names() { - "$OSASCRIPT_BIN" -e 'tell application "System Events" to get the name of every login item' 2>/dev/null + "$OSASCRIPT_BIN" -e "$LOGIN_ITEM_NAMES_SCRIPT" 2>/dev/null } # 0 = present, 1 = confirmed absent, 2 = could not determine. @@ -91,14 +102,12 @@ current_login_item_names() { login_item_exists() { local target="$1" names entry names="$(current_login_item_names)" || return 2 - IFS=',' read -ra parts <<< "$names" - # A Mac with zero login items yields an empty array, and macOS's bash 3.2 - # treats "${parts[@]}" on an empty array as unbound under set -u. - [[ "${#parts[@]}" -gt 0 ]] || return 1 - for entry in "${parts[@]}"; do - entry="$(/usr/bin/printf '%s' "$entry" | /usr/bin/sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" + # A Mac with zero login items yields an empty string from the coercion + # above -- a real answer (nothing is registered), not a failed read. + [[ -n "$names" ]] || return 1 + while IFS= read -r entry; do [[ "$entry" == "$target" ]] && return 0 - done + done <<< "$names" return 1 } diff --git a/scripts/network_watch.sh b/scripts/network_watch.sh index 9823f74..09acf24 100755 --- a/scripts/network_watch.sh +++ b/scripts/network_watch.sh @@ -137,10 +137,28 @@ fi # 인자 사이의 변수 대입(POSIX)은 파일이 비어도 순서대로 적용된다. new_established() { /usr/bin/awk ' - function command_name(value) { - gsub(/\\x20/, " ", value) - sub(/ +$/, "", value) - return value + function hex_digit(c) { + return index("0123456789abcdef", tolower(c)) - 1 + } + # lsof escapes every byte it considers unprintable, not just the space + # that motivated this: a tab arrives as \\x09 and a non-ASCII name as a + # run of \\xNN bytes. Decoding only \\x20 left those literal in the + # reported name. Bytes are emitted verbatim, so a multi-byte UTF-8 name + # reassembles correctly; control bytes become a space because they would + # otherwise break the TSV this protocol is carried on. + function command_name(value, out, rest, code) { + out = "" + rest = value + while (match(rest, /\\x[0-9A-Fa-f][0-9A-Fa-f]/)) { + out = out substr(rest, 1, RSTART - 1) + code = hex_digit(substr(rest, RSTART + 2, 1)) * 16 \ + + hex_digit(substr(rest, RSTART + 3, 1)) + out = out ((code < 32 || code == 127) ? " " : sprintf("%c", code)) + rest = substr(rest, RSTART + 4) + } + out = out rest + sub(/ +$/, "", out) + return out } FNR == 1 { next } building == 1 { @@ -169,10 +187,28 @@ new_established() { new_listen() { /usr/bin/awk ' - function command_name(value) { - gsub(/\\x20/, " ", value) - sub(/ +$/, "", value) - return value + function hex_digit(c) { + return index("0123456789abcdef", tolower(c)) - 1 + } + # lsof escapes every byte it considers unprintable, not just the space + # that motivated this: a tab arrives as \\x09 and a non-ASCII name as a + # run of \\xNN bytes. Decoding only \\x20 left those literal in the + # reported name. Bytes are emitted verbatim, so a multi-byte UTF-8 name + # reassembles correctly; control bytes become a space because they would + # otherwise break the TSV this protocol is carried on. + function command_name(value, out, rest, code) { + out = "" + rest = value + while (match(rest, /\\x[0-9A-Fa-f][0-9A-Fa-f]/)) { + out = out substr(rest, 1, RSTART - 1) + code = hex_digit(substr(rest, RSTART + 2, 1)) * 16 \ + + hex_digit(substr(rest, RSTART + 3, 1)) + out = out ((code < 32 || code == 127) ? " " : sprintf("%c", code)) + rest = substr(rest, RSTART + 4) + } + out = out rest + sub(/ +$/, "", out) + return out } FNR == 1 { next } building == 1 { diff --git a/scripts/scanner_helper.jxa.js b/scripts/scanner_helper.jxa.js index 0767aab..1398601 100644 --- a/scripts/scanner_helper.jxa.js +++ b/scripts/scanner_helper.jxa.js @@ -785,7 +785,26 @@ raw.sections.gpu = []; // 9-character truncation ("Codex\x20" -> "Codex ") is invisible in the UI // while still splitting dedup keys, so it is dropped too. function lsofCommandName(value) { - return String(value || "").replace(/\\x20/g, " ").replace(/ +$/, ""); + // lsof escapes every byte it considers unprintable, not just the space that + // motivated this: a tab arrives as \x09 and a non-ASCII name as a run of + // \xNN bytes. Decoding only \x20 left those literal in the reported name + // and in anything matching on it. A run is decoded as UTF-8 so multi-byte + // names reassemble instead of turning into per-byte mojibake; control bytes + // become a space, and an undecodable run degrades to a space rather than + // throwing mid-scan. + return String(value || "") + .replace(/(?:\\x[0-9A-Fa-f]{2})+/g, run => { + const bytes = run.match(/[0-9A-Fa-f]{2}/g).map(hex => parseInt(hex, 16)); + if (bytes.every(byte => byte < 32 || byte === 127)) return " "; + try { + return decodeURIComponent(bytes.map(byte => + "%" + (byte < 16 ? "0" : "") + byte.toString(16) + ).join("")); + } catch (error) { + return " "; + } + }) + .replace(/ +$/, ""); } const connections = []; @@ -832,11 +851,17 @@ raw.sections.privacyPermissions = tmp("privacy.tsv").trim().split(/\r?\n/).filte // Casks compare with "!=" instead of "<" since cask versions aren't always // strictly ordered; a formula can also list multiple installed versions // comma-separated inside the parens (e.g. "sqlite (3.53.2, 3.53.3) < 3.53.4"). +// A pinned formula or cask appends " [pinned at X]" after the latest version, +// which an end-anchored latest-version capture silently dropped: the row +// vanished from this list while the collector's own line count still included +// it, so the "N개 업데이트" total disagreed with the rows shown, and a pinned +// package's available update was invisible. The suffix is captured instead so +// the row survives and can say why it is being held back. raw.sections.devtoolUpdates = tmp("devtool_updates.txt").trim().split(/\r?\n/).filter(Boolean).map(line => { - const m = line.match(/^(\S+)\s+\((.+?)\)\s+(?:<|!=)\s+(\S+)\s*$/); + const m = line.match(/^(\S+)\s+\((.+?)\)\s+(?:<|!=)\s+(\S+)(\s+\[pinned at [^\]]*\])?\s*$/); if (!m) return null; - const [, name, current, latest] = m; - return { name, current, latest }; + const [, name, current, latest, pinned] = m; + return { name, current, latest, pinned: !!pinned }; }).filter(Boolean); // codesign -dv writes its verdict to stderr, and exits non-zero for a diff --git a/tests/test_macos_devtool_updates.py b/tests/test_macos_devtool_updates.py index 2db72f8..b84e292 100644 --- a/tests/test_macos_devtool_updates.py +++ b/tests/test_macos_devtool_updates.py @@ -9,8 +9,11 @@ import os import subprocess +import sys import textwrap +import pytest + def run_collector(project_root, tmp_path, brew_script: str): module = project_root / "scripts" / "modules" / "macos" / "devtool_updates.sh" @@ -112,3 +115,49 @@ def test_never_disables_auto_update_opt_out(project_root, tmp_path): assert result.returncode == 0, result.stderr fields = status.strip("\n").split("\t") assert fields[2] == "ok" + + +@pytest.mark.skipif(sys.platform != "darwin", reason="drives the regex through the real JXA engine") +def test_pinned_brew_lines_survive_the_scan_parser(project_root): + """`brew outdated --verbose` appends " [pinned at X]" for a pinned + formula or cask. An end-anchored latest-version capture dropped those rows + entirely, so a pinned package's available update was invisible while the + collector's own line count still included it -- the "N개 업데이트" total + disagreed with the rows actually shown. The regex lives in the JXA + scanner, so it is exercised through the real JavaScriptCore engine here + rather than re-implemented in Python. + """ + import re + + source = (project_root / "scripts" / "scanner_helper.jxa.js").read_text(encoding="utf-8") + pattern = re.search(r"const m = line\.match\((/\^\(\\S\+\).*?)\);", source) + assert pattern, "could not find the devtoolUpdates line regex" + + probe = f""" + var lines = [ + "node (18.0.0) < 20.0.0 [pinned at 18.0.0]", + "firefox (139.0) != 140.0.1 [pinned at 139.0]", + "sqlite (3.53.2, 3.53.3) < 3.53.4", + "virtualbox (7.2.8,173730) != 7.2.14,174565", + "ada-url (3.4.4) < 4.0.0" + ]; + var out = lines.map(function (line) {{ + var m = line.match({pattern.group(1)}); + if (!m) return "DROP"; + return m[1] + "|" + m[3] + "|" + !!m[4]; + }}); + console.log(out.join("\\n")); + """ + result = subprocess.run( + ["/usr/bin/osascript", "-l", "JavaScript", "-e", probe], + capture_output=True, text=True, encoding="utf-8", + ) + assert result.returncode == 0, result.stderr + rows = result.stderr.strip().splitlines() or result.stdout.strip().splitlines() + + assert "DROP" not in rows, rows + assert rows[0] == "node|20.0.0|true" + assert rows[1] == "firefox|140.0.1|true" + assert rows[2] == "sqlite|3.53.4|false" + assert rows[3] == "virtualbox|7.2.14,174565|false" + assert rows[4] == "ada-url|4.0.0|false" diff --git a/tests/test_macos_login_items.py b/tests/test_macos_login_items.py index bd5a327..f5da34d 100644 --- a/tests/test_macos_login_items.py +++ b/tests/test_macos_login_items.py @@ -31,7 +31,7 @@ def _failing_query_osascript(tmp_path, *, initial_items, healthy_queries): revoked or System Events going unscriptable between preview and execute. Deletes always fail, so the item genuinely survives.""" state_file = tmp_path / "login-items-state.txt" - state_file.write_text(initial_items, encoding="utf-8") + state_file.write_text("\n".join(initial_items), encoding="utf-8") counter = tmp_path / "query-count.txt" counter.write_text("0", encoding="utf-8") stub = tmp_path / "osascript-failing-stub" @@ -39,7 +39,7 @@ def _failing_query_osascript(tmp_path, *, initial_items, healthy_queries): f"""#!/bin/bash script="$2" case "$script" in - *"get the name of every login item"*) + *"name of every login item"*) n=$(( $(cat "{counter}") + 1 )) printf '%s' "$n" > "{counter}" [[ "$n" -gt {healthy_queries} ]] && exit 1 @@ -64,7 +64,7 @@ def _fake_osascript(tmp_path, *, initial_items, delete_is_noop=False): but changes nothing -- exactly the gap the post-delete recheck exists to catch).""" state_file = tmp_path / "login-items-state.txt" - state_file.write_text(initial_items, encoding="utf-8") + state_file.write_text("\n".join(initial_items), encoding="utf-8") calls_log = tmp_path / "osascript-calls.log" stub = tmp_path / "osascript-stub" noop_flag = "1" if delete_is_noop else "0" @@ -73,7 +73,7 @@ def _fake_osascript(tmp_path, *, initial_items, delete_is_noop=False): script="$2" printf '%s\\n' "$script" >> "{calls_log}" case "$script" in - *"get the name of every login item"*) + *"name of every login item"*) cat "{state_file}" ;; *"delete login item"*) @@ -81,9 +81,7 @@ def _fake_osascript(tmp_path, *, initial_items, delete_is_noop=False): exit 0 fi name=$(printf '%s' "$script" | sed -E 's/.*delete login item "(.*)".*/\\1/') - current=$(cat "{state_file}") - new=$(printf '%s' "$current" | tr ',' '\\n' | sed 's/^ *//;s/ *$//' \\ - | grep -v -F -x "$name" | paste -sd, -) + new=$(grep -v -F -x "$name" "{state_file}" || true) printf '%s' "$new" > "{state_file}" ;; esac @@ -140,7 +138,7 @@ def _execute(project_root, tmp_path, name, token, *, osascript_stub, home): def test_preview_issues_token_for_an_existing_item(project_root, tmp_path): home = tmp_path / "home" - stub, _, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar, Baz") + stub, _, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar", "Baz"]) result, payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) @@ -158,7 +156,7 @@ def test_preview_issues_token_for_an_existing_item(project_root, tmp_path): def test_preview_refuses_a_name_that_is_not_actually_a_login_item(project_root, tmp_path): home = tmp_path / "home" - stub, _, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar") + stub, _, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"]) result, payload = _preview(project_root, tmp_path, "NotThere", osascript_stub=stub, home=home) @@ -170,7 +168,7 @@ def test_preview_refuses_a_name_that_is_not_actually_a_login_item(project_root, def test_execute_removes_the_item_and_confirms_it_is_actually_gone(project_root, tmp_path): home = tmp_path / "home" - stub, state_file, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar, Baz") + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar", "Baz"]) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) result, payload = _execute( @@ -179,7 +177,7 @@ def test_execute_removes_the_item_and_confirms_it_is_actually_gone(project_root, assert result.returncode == 0, result.stderr assert payload["status"] == "ok" - remaining = [n.strip() for n in state_file.read_text(encoding="utf-8").split(",")] + remaining = state_file.read_text(encoding="utf-8").splitlines() assert "Bar" not in remaining assert "Foo" in remaining and "Baz" in remaining @@ -189,7 +187,7 @@ def test_execute_reports_failed_when_the_item_survives_the_delete_call(project_r real recheck of the live list can tell "deleted" from "command accepted, nothing actually changed".""" home = tmp_path / "home" - stub, state_file, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar", delete_is_noop=True) + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"], delete_is_noop=True) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) result, payload = _execute( @@ -203,7 +201,7 @@ def test_execute_reports_failed_when_the_item_survives_the_delete_call(project_r def test_execute_rejects_reusing_an_already_consumed_token(project_root, tmp_path): home = tmp_path / "home" - stub, _, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar") + stub, _, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"]) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) token = preview_payload["approvalToken"] @@ -218,7 +216,7 @@ def test_execute_rejects_reusing_an_already_consumed_token(project_root, tmp_pat def test_execute_rejects_a_token_approved_for_a_different_name(project_root, tmp_path): home = tmp_path / "home" - stub, state_file, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar") + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"]) _, preview_payload = _preview(project_root, tmp_path, "Foo", osascript_stub=stub, home=home) result, payload = _execute( @@ -227,13 +225,13 @@ def test_execute_rejects_a_token_approved_for_a_different_name(project_root, tmp assert result.returncode == 1 assert payload["status"] == "mismatch" - remaining = [n.strip() for n in state_file.read_text(encoding="utf-8").split(",")] + remaining = state_file.read_text(encoding="utf-8").splitlines() assert "Foo" in remaining and "Bar" in remaining def test_execute_rejects_an_expired_token(project_root, tmp_path): home = tmp_path / "home" - stub, state_file, _ = _fake_osascript(tmp_path, initial_items="Foo, Bar") + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"]) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) token = preview_payload["approvalToken"] @@ -256,7 +254,7 @@ def test_execute_rejects_an_expired_token(project_root, tmp_path): def test_execute_reports_already_gone_when_item_vanished_before_execute(project_root, tmp_path): home = tmp_path / "home" - stub, state_file, calls_log = _fake_osascript(tmp_path, initial_items="Foo, Bar") + stub, state_file, calls_log = _fake_osascript(tmp_path, initial_items=["Foo", "Bar"]) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) # Simulate the item being removed some other way (System Settings, the @@ -281,7 +279,7 @@ def test_execute_does_not_call_a_failed_query_already_gone(project_root, tmp_pat successful read may assert absence.""" home = tmp_path / "home" # Query 1 is the preview; every query from the execute onward fails. - stub, state_file = _failing_query_osascript(tmp_path, initial_items="Foo, Bar", healthy_queries=1) + stub, state_file = _failing_query_osascript(tmp_path, initial_items=["Foo", "Bar"], healthy_queries=1) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) assert preview_payload["status"] == "ready" @@ -303,7 +301,7 @@ def test_execute_does_not_report_ok_when_the_post_delete_recheck_fails(project_r home = tmp_path / "home" # Queries 1 (preview) and 2 (pre-delete check) succeed; the post-delete # recheck fails, and the delete itself failed too, so the item survives. - stub, state_file = _failing_query_osascript(tmp_path, initial_items="Foo, Bar", healthy_queries=2) + stub, state_file = _failing_query_osascript(tmp_path, initial_items=["Foo", "Bar"], healthy_queries=2) _, preview_payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) result, payload = _execute( @@ -319,7 +317,7 @@ def test_preview_does_not_call_a_failed_query_not_found(project_root, tmp_path): """not_found means "this is not a login item". A failed read means we do not know, and must not issue an approval token off it either way.""" home = tmp_path / "home" - stub, _ = _failing_query_osascript(tmp_path, initial_items="Foo, Bar", healthy_queries=0) + stub, _ = _failing_query_osascript(tmp_path, initial_items=["Foo", "Bar"], healthy_queries=0) result, payload = _preview(project_root, tmp_path, "Bar", osascript_stub=stub, home=home) @@ -328,9 +326,47 @@ def test_preview_does_not_call_a_failed_query_not_found(project_root, tmp_path): assert "approvalToken" not in payload +def test_a_name_containing_a_comma_is_matched_and_removed(project_root, tmp_path): + """osascript's default list serialization joins names with ", ", so a + single item named "Backup, Inc." was indistinguishable from two items + "Backup" and "Inc." -- the real item could never be matched, and was + therefore permanently unremovable through this tool.""" + home = tmp_path / "home" + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Backup, Inc.", "Mos"]) + + _, preview_payload = _preview(project_root, tmp_path, "Backup, Inc.", osascript_stub=stub, home=home) + assert preview_payload["status"] == "ready", preview_payload + + result, payload = _execute( + project_root, tmp_path, "Backup, Inc.", preview_payload["approvalToken"], + osascript_stub=stub, home=home, + ) + + assert result.returncode == 0, result.stderr + assert payload["status"] == "ok" + assert state_file.read_text(encoding="utf-8").splitlines() == ["Mos"] + + +def test_removing_one_item_is_not_reported_failed_by_a_similar_surviving_name(project_root, tmp_path): + """Under the old comma split, a surviving "Foo, Bar" still produced a + "Foo" fragment, so removing a genuinely separate "Foo" re-matched on the + recheck and reported failed even though the removal succeeded.""" + home = tmp_path / "home" + stub, state_file, _ = _fake_osascript(tmp_path, initial_items=["Foo", "Foo, Bar"]) + + _, preview_payload = _preview(project_root, tmp_path, "Foo", osascript_stub=stub, home=home) + result, payload = _execute( + project_root, tmp_path, "Foo", preview_payload["approvalToken"], osascript_stub=stub, home=home + ) + + assert result.returncode == 0, result.stderr + assert payload["status"] == "ok" + assert state_file.read_text(encoding="utf-8").splitlines() == ["Foo, Bar"] + + def test_execute_requires_owner_approved_flag(project_root, tmp_path): home = tmp_path / "home" - stub, _, _ = _fake_osascript(tmp_path, initial_items="Foo") + stub, _, _ = _fake_osascript(tmp_path, initial_items=["Foo"]) token_file = tmp_path / "token" token_file.write_text("0" * 64, encoding="utf-8") @@ -347,7 +383,7 @@ def test_execute_requires_owner_approved_flag(project_root, tmp_path): def test_execute_requires_an_approval_token_file(project_root, tmp_path): home = tmp_path / "home" - stub, _, _ = _fake_osascript(tmp_path, initial_items="Foo") + stub, _, _ = _fake_osascript(tmp_path, initial_items=["Foo"]) result = _run( project_root, tmp_path, ["--execute", "Foo", "--owner-approved"], osascript_stub=stub, home=home