From b58821fd0b472cebb5bf28f28d385370684faf0d Mon Sep 17 00:00:00 2001 From: Heznpc Date: Thu, 13 Aug 2026 21:50:50 +0900 Subject: [PATCH 1/2] Fix accuracy defects from the review follow-up batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lower-severity half of the same adversarial review as #74. None of these crashes or claims a false success, but each reports something that is not quite true. Login item names are now read one per line instead of relying on osascript's ", " list serialization. That default is ambiguous: an item named "Backup, Inc." serialized exactly like two items "Backup" and "Inc.", so it could never be matched and was permanently unremovable through this tool; and a surviving "Foo, Bar" kept producing a "Foo" fragment, so removing a genuinely separate "Foo" re-matched on the post-delete recheck and reported failed after a successful removal. lsof escapes every byte it deems unprintable, not just the space that motivated the earlier fix -- a tab arrives as \x09 and a non-ASCII name as a run of \xNN. Both the awk and the JXA decoders now handle whole runs, so a multi-byte UTF-8 name reassembles instead of leaking literal escapes into the UI and into anything matching on the name; control bytes become a space because they would break the TSV. Pinned Homebrew packages were dropped outright by the devtoolUpdates regex, which anchored the latest version to end-of-line while brew appends " [pinned at X]". The row vanished while the collector's line count still included it, so the displayed total disagreed with the list and a pinned package's update was invisible. Rows now survive and say they are pinned. SpaceGoalSelection's documented "same set regardless of scan order" promise did not hold when size and label both tie -- label falls back to kind, so two same-size rows of one kind tied completely and the result depended on emit order. Path breaks the remaining tie. The goal comparison also gained a tolerance: 2.4 + 0.3 + 0.3 is exactly 3.0 in decimal but accumulates to 2.9999999999999996, so the greedy walked past the set that met the goal, took an extra item, and then called the result short. A non-finite sizeGB is clamped at the parse boundary, where it used to poison every sum and trip a range precondition. Login item removal and the observation window now respect isBusy in both directions: removal could previously start a second concurrent ScanPipeline over the same output files during a scan (untracked by scanTask, so 검사 취소 could not stop it), and a scan started under an observation landed in that observation's own results. Mothball used scan() rather than scanReport(), discarding inspection failures -- MothballCore's own API comment warns that this makes "found but could not inspect" indistinguishable from "nothing found". The page now says which it is. --- .../Sources/Modore/Models/ScanModels.swift | 4 + .../Sources/Modore/Models/StorageModels.swift | 13 ++- .../Modore/Services/LoginItemService.swift | 11 ++- .../Modore/Services/MothballService.swift | 20 +++-- .../Modore/Services/ObservationService.swift | 8 +- .../Sources/Modore/Services/ScanModel.swift | 7 ++ .../Sources/Modore/Views/ActivityView.swift | 11 ++- .../Sources/Modore/Views/MothballView.swift | 15 +++- .../Sources/Modore/Views/SecurityView.swift | 6 +- .../Sources/Modore/Views/SpaceGoalView.swift | 16 +++- .../ModoreTests/DevtoolUpdateRowTests.swift | 21 +++++ .../ModoreTests/SpaceGoalSelectionTests.swift | 69 +++++++++++++++- scripts/login_items.sh | 25 ++++-- scripts/network_watch.sh | 52 ++++++++++-- scripts/scanner_helper.jxa.js | 33 +++++++- tests/test_macos_devtool_updates.py | 45 +++++++++++ tests/test_macos_login_items.py | 80 ++++++++++++++----- 17 files changed, 373 insertions(+), 63 deletions(-) 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..2a6731c 100644 --- a/tests/test_macos_devtool_updates.py +++ b/tests/test_macos_devtool_updates.py @@ -112,3 +112,48 @@ 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" + + +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 From 03f1b9ffb2012328ab08163be31959d5d7b6c3a7 Mon Sep 17 00:00:00 2001 From: Heznpc Date: Thu, 13 Aug 2026 21:58:44 +0900 Subject: [PATCH 2/2] Skip the JXA-driven pinned-line test on non-macOS CI Same class of miss as the network_watch skipif: the probe runs the real regex through /usr/bin/osascript, which Linux runners lack. --- tests/test_macos_devtool_updates.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_macos_devtool_updates.py b/tests/test_macos_devtool_updates.py index 2a6731c..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" @@ -114,6 +117,7 @@ def test_never_disables_auto_update_opt_out(project_root, tmp_path): 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