From 0d7b605990d211766d9ff197d25889fa1914619b Mon Sep 17 00:00:00 2001 From: boris Date: Wed, 16 Sep 2026 18:17:13 +0300 Subject: [PATCH] fix: stamp update-check cooldown only after a successful fetch A failed GitHub lookup was setting lastChecked first, so opening the popover again stayed silent for 30 minutes even though no release was seen. --- Sources/PokeTokenBar/Core/UpdateChecker.swift | 61 ++++++++++++----- .../UpdateCheckerTests.swift | 65 +++++++++++++++++++ docs/reference/defect-log.md | 9 +++ 3 files changed, 120 insertions(+), 15 deletions(-) diff --git a/Sources/PokeTokenBar/Core/UpdateChecker.swift b/Sources/PokeTokenBar/Core/UpdateChecker.swift index 12f2c947..d4352972 100644 --- a/Sources/PokeTokenBar/Core/UpdateChecker.swift +++ b/Sources/PokeTokenBar/Core/UpdateChecker.swift @@ -8,43 +8,74 @@ import Observation final class UpdateChecker { struct Available: Equatable { let version: String; let url: String } + /// Tag + release-page URL from the GitHub latest endpoint (or a test double). + struct LatestRelease: Equatable, Sendable { + let tag: String + let url: String + } + private(set) var available: Available? private(set) var isUpdating = false let currentVersion: String - private let repo = "chattymin/PokeTokenBar" private let clock: () -> Date + /// Injected so tests can fail or succeed without hitting the network. + private let fetchLatest: () async -> LatestRelease? private var lastChecked: Date? + /// Overlapping popover opens must not stack concurrent GitHub calls once the early + /// cooldown stamp is gone (a failed check no longer blocks the next attempt). + private var checkInFlight = false - init(currentVersion: String? = nil, clock: @escaping () -> Date = Date.init) { + init(currentVersion: String? = nil, + clock: @escaping () -> Date = Date.init, + fetchLatest: (() async -> LatestRelease?)? = nil) { self.currentVersion = currentVersion ?? (Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String) ?? "0" self.clock = clock + self.fetchLatest = fetchLatest ?? { await Self.liveLatestRelease(repo: "chattymin/PokeTokenBar") } } /// 최신 릴리스 조회 → 새 버전이고 사용자가 그 버전을 'skip' 하지 않았으면 available 설정. - /// minInterval 보다 자주 호출되면 무시(레이트리밋 보호). + /// minInterval 은 **성공한** 조회 사이에만 적용한다. 실패(네트워크·비정상 응답·불안전 URL)는 + /// 쿨다운을 시작하지 않아서, 팝오버를 다시 열면 곧장 재시도할 수 있다. func check(minInterval: TimeInterval = 1800) async { + if checkInFlight { return } if let last = lastChecked, clock().timeIntervalSince(last) < minInterval { return } + checkInFlight = true + defer { checkInFlight = false } + + guard let release = await fetchLatest(), + Self.isTrustedReleaseURL(release.url) else { return } lastChecked = clock() - guard let url = URL(string: "https://api.github.com/repos/\(repo)/releases/latest") else { return } + + let latest = release.tag.hasPrefix("v") ? String(release.tag.dropFirst()) : release.tag + let skipped = UserDefaults.standard.string(forKey: "skippedUpdateVersion") + if Self.isNewer(latest, than: currentVersion), latest != skipped { + available = Available(version: latest, url: release.url) + } else { + available = nil + } + } + + /// NSWorkspace.open 으로 가는 릴리스 URL — https + github.com 만 허용(스킴 하이재킹 방지). + nonisolated static func isTrustedReleaseURL(_ string: String) -> Bool { + guard let url = URL(string: string), + url.scheme == "https", + url.host == "github.com" else { return false } + return true + } + + /// Live GitHub latest-release fetch. Returns nil on any transport or payload failure. + private nonisolated static func liveLatestRelease(repo: String) async -> LatestRelease? { + guard let url = URL(string: "https://api.github.com/repos/\(repo)/releases/latest") else { return nil } var req = URLRequest(url: url, timeoutInterval: 15) req.setValue("application/vnd.github+json", forHTTPHeaderField: "Accept") guard let (data, resp) = try? await URLSession.shared.data(for: req), (resp as? HTTPURLResponse)?.statusCode == 200, let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let tag = json["tag_name"] as? String, - let html = json["html_url"] as? String, - // 응답 필드가 NSWorkspace.open 으로 가므로 https + github.com 만 허용(스킴 하이재킹 방지) - let htmlURL = URL(string: html), htmlURL.scheme == "https", htmlURL.host == "github.com" - else { return } - let latest = tag.hasPrefix("v") ? String(tag.dropFirst()) : tag - let skipped = UserDefaults.standard.string(forKey: "skippedUpdateVersion") - if Self.isNewer(latest, than: currentVersion), latest != skipped { - available = Available(version: latest, url: html) - } else { - available = nil - } + let html = json["html_url"] as? String else { return nil } + return LatestRelease(tag: tag, url: html) } /// 이 버전은 다시 알리지 않음. diff --git a/Tests/PokeTokenBarTests/UpdateCheckerTests.swift b/Tests/PokeTokenBarTests/UpdateCheckerTests.swift index 8a22ae5f..b8bc7d6b 100644 --- a/Tests/PokeTokenBarTests/UpdateCheckerTests.swift +++ b/Tests/PokeTokenBarTests/UpdateCheckerTests.swift @@ -25,6 +25,71 @@ final class UpdateCheckerTests: XCTestCase { XCTAssertFalse(UpdateChecker.isNewer("2.0", than: "2.0.0")) // 동일 } + // MARK: - Cooldown stamps only after a successful fetch + + /// A failed GitHub lookup must not start the 30-minute cooldown: otherwise opening the + /// popover again stays silent until the timer expires, even though no release was seen. + @MainActor + func testFailedCheckDoesNotStartTheCooldown() async { + var now = Date(timeIntervalSince1970: 1_700_000_000) + var fetches = 0 + let checker = UpdateChecker(currentVersion: "2.5.3", clock: { now }) { + fetches += 1 + return nil + } + + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 1) + XCTAssertNil(checker.available) + + now = now.addingTimeInterval(5) + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 2, "a failed check must not suppress the next attempt") + } + + @MainActor + func testSuccessfulCheckStartsTheCooldownAndAppliesTheRelease() async { + var now = Date(timeIntervalSince1970: 1_700_000_000) + var fetches = 0 + let url = "https://github.com/chattymin/PokeTokenBar/releases/tag/v2.5.5" + let checker = UpdateChecker(currentVersion: "2.5.3", clock: { now }) { + fetches += 1 + return UpdateChecker.LatestRelease(tag: "v2.5.5", url: url) + } + + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 1) + XCTAssertEqual(checker.available?.version, "2.5.5") + XCTAssertEqual(checker.available?.url, url) + + now = now.addingTimeInterval(60) + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 1, "a successful check must honour minInterval") + + now = now.addingTimeInterval(1_800) + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 2, "after the cooldown the next check must fetch again") + } + + @MainActor + func testRejectedReleaseUrlDoesNotStartTheCooldown() async { + var fetches = 0 + let checker = UpdateChecker( + currentVersion: "2.5.3", + clock: { Date(timeIntervalSince1970: 1_700_000_000) } + ) { + fetches += 1 + // Live fetch rejects non-https github.com URLs before applying. A poisoned + // payload must not count as a successful check either. + return UpdateChecker.LatestRelease(tag: "v2.5.5", url: "http://evil.example/x") + } + + await checker.check(minInterval: 1_800) + XCTAssertNil(checker.available) + await checker.check(minInterval: 1_800) + XCTAssertEqual(fetches, 2, "an unsafe URL is a failed check, not a cooldown start") + } + // MARK: - Detached upgrade script wait loop (#175) func testDetachedUpgradeScriptWaitsOnPidNotProcessName() { diff --git a/docs/reference/defect-log.md b/docs/reference/defect-log.md index 6bcd5ae4..faa80b58 100644 --- a/docs/reference/defect-log.md +++ b/docs/reference/defect-log.md @@ -757,6 +757,15 @@ read_when: ## 프로세스 제어·업데이트 +- **쿨다운 스탬프는 성공한 조회에만 찍는다.** `UpdateChecker.check` 가 GitHub 호출 *전에* + `lastChecked` 를 쓰면, 네트워크·파싱 실패도 30분 쿨다운을 시작한다. 팝오버를 다시 열어도 + (`minInterval` 디바운스) 재시도가 막혀 배너가 조용히 안 뜬다. 실패·불안전 URL 은 스탬프하지 + 않고, 성공한 페이로드만 스탬프한다. 조기 스탬프가 없어지면 겹친 `check()` 가 동시에 나갈 수 + 있으므로 in-flight 가드도 둔다. 회귀: + `testFailedCheckDoesNotStartTheCooldown`, + `testSuccessfulCheckStartsTheCooldownAndAppliesTheRelease`, + `testRejectedReleaseUrlDoesNotStartTheCooldown`. + - **`pgrep -x ` 은 실행 파일의 정체성 검사이지, 기다리는 특정 프로세스에 대한 검사가 아니다.** 중복 인스턴스가 떠 있는 동안 실행될 수 있는 모든 wait-for-exit 루프는 PID를 받아야 한다. `UpdateChecker`가 자동 업데이트 시 앱 종료를 기다릴 때 `pgrep -x PokeTokenBar`를 쓰면, 중복 인스턴스가 살아있는 동안 루프를