fix: stamp update-check cooldown only after a successful fetch

Sources/PokeTokenBar/Core/UpdateChecker.swift

@@ -15,24 +15,36 @@ final class UpdateChecker {
         case current
     }
 
+    /// 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?
     /// Newer release the user chose to skip. Hidden from the banner, still shown in Settings.
     private(set) var skipped: Available?
     private(set) var isUpdating = false
 
     let currentVersion: String
-    private let repo = "chattymin/PokeTokenBar"
     private let clock: () -> Date
     private let defaults: UserDefaults
+    /// 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
 
     static let skippedVersionKey = "skippedUpdateVersion"
 
-    init(currentVersion: String? = nil, clock: @escaping () -> Date = Date.init, defaults: UserDefaults = .standard) {
+    init(currentVersion: String? = nil, clock: @escaping () -> Date = Date.init, defaults: UserDefaults = .standard,
+         fetchLatest: (() async -> LatestRelease?)? = nil) {
         self.currentVersion = currentVersion
             ?? (Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String) ?? "0"
         self.clock = clock
         self.defaults = defaults
+        self.fetchLatest = fetchLatest ?? { await Self.liveLatestRelease(repo: "chattymin/PokeTokenBar") }
     }
 
     var settingsNotice: SettingsNotice {
@@ -45,22 +57,53 @@ final class UpdateChecker {
     var updateTarget: Available? { available ?? skipped }
 
     /// 최신 릴리스 조회. 스킵한 버전은 배너(`available`)에 안 올리고 Settings(`skipped`)에만 남긴다.
-    /// 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),
+              let version = Self.normalizedReleaseVersion(release.tag) else { return }
         lastChecked = clock()
-        guard let url = URL(string: "https://api.github.com/repos/\(repo)/releases/latest") else { return }
+        consider(latest: version, url: release.url)
+    }
+
+    /// 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
+    }
+
+    /// Release tag → comparable version, or nil when the tag is not a plain `MAJOR.MINOR.PATCH`
+    /// (optionally `v`-prefixed). `isNewer` reads non-numeric parts as 0, so a prerelease such as
+    /// `v2.6.0-beta.1` would compare equal to `2.6.0` and garbage would compare as `0.0.0`; neither
+    /// may be applied or counted as a successful check.
+    nonisolated static func normalizedReleaseVersion(_ tag: String) -> String? {
+        var version = Substring(tag.trimmingCharacters(in: .whitespacesAndNewlines))
+        if version.first == "v" || version.first == "V" { version = version.dropFirst() }
+        let parts = version.split(separator: ".", omittingEmptySubsequences: false)
+        guard parts.count == 3,
+              parts.allSatisfy({ part in
+                  !part.isEmpty && part.count <= 9 && part.allSatisfy { $0.isASCII && $0.isNumber }
+              }) else { return nil }
+        return parts.map { String(Int($0)!) }.joined(separator: ".")
+    }
+
+    /// Live GitHub latest-release fetch. 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 }
-        consider(latest: tag, url: html)
+              let html = json["html_url"] as? String else { return nil }
+        return LatestRelease(tag: tag, url: html)
     }
 
     /// Apply one fetched release. `latest` may be a tag (`v2.5.4`) or a bare version.

Tests/PokeTokenBarTests/UpdateCheckerTests.swift

@@ -39,6 +39,132 @@ final class UpdateCheckerTests: XCTestCase {
         )
     }
 
+    // MARK: - Cooldown stamps only after a successful, validated check
+
+    private static let releaseURL = "https://github.com/chattymin/PokeTokenBar/releases/tag/v2.5.5"
+
+    @MainActor
+    private func isolatedDefaults() -> UserDefaults {
+        let suite = "UpdateCheckerTests.check.\(UUID().uuidString)"
+        addTeardownBlock { UserDefaults().removePersistentDomain(forName: suite) }
+        return UserDefaults(suiteName: suite)!
+    }
+
+    /// 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 }, defaults: isolatedDefaults()) {
+            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 checker = UpdateChecker(currentVersion: "2.5.3", clock: { now }, defaults: isolatedDefaults()) {
+            fetches += 1
+            return UpdateChecker.LatestRelease(tag: "v2.5.5", url: Self.releaseURL)
+        }
+
+        await checker.check(minInterval: 1_800)
+        XCTAssertEqual(fetches, 1)
+        XCTAssertEqual(checker.available?.version, "2.5.5")
+        XCTAssertEqual(checker.available?.url, Self.releaseURL)
+
+        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) },
+                                    defaults: isolatedDefaults()) {
+            fetches += 1
+            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")
+    }
+
+    /// Malformed tags and unsupported prerelease tags are failed checks: nothing is applied, and the
+    /// very next attempt fetches again and applies a valid release instead of waiting 30 minutes.
+    @MainActor
+    func testMalformedOrPrereleaseTagIsAFailedCheckAndDoesNotBlockTheNextOne() async {
+        let badTags = ["", "latest", "v", "v2.5", "2.5.5.1", "v2..5", "2.5.x", "release-2.5.5", "v 2.5.5",
+                       "v2.6.0-beta.1", "2.6.0-rc1", "v2.6.0+build.5", "v2.6.0.beta", "v-2.6.0",
+                       "v1234567890.0.0", "v٢.٥.٥"]
+        for bad in badTags {
+            var tag = bad
+            var fetches = 0
+            let checker = UpdateChecker(currentVersion: "2.5.3", clock: { Date(timeIntervalSince1970: 1_700_000_000) },
+                                        defaults: isolatedDefaults()) {
+                fetches += 1
+                return UpdateChecker.LatestRelease(tag: tag, url: Self.releaseURL)
+            }
+
+            await checker.check(minInterval: 1_800)
+            XCTAssertNil(checker.available, "\(bad.debugDescription) must not be offered")
+            XCTAssertEqual(checker.settingsNotice, .current, "\(bad.debugDescription)")
+
+            tag = "v2.5.5"
+            await checker.check(minInterval: 1_800)
+            XCTAssertEqual(fetches, 2, "\(bad.debugDescription) must not start the cooldown")
+            XCTAssertEqual(checker.available?.version, "2.5.5", "the next valid release applies at once")
+        }
+    }
+
+    func testNormalizedReleaseVersion() {
+        XCTAssertEqual(UpdateChecker.normalizedReleaseVersion("v2.5.5"), "2.5.5")
+        XCTAssertEqual(UpdateChecker.normalizedReleaseVersion("2.5.10"), "2.5.10")
+        XCTAssertEqual(UpdateChecker.normalizedReleaseVersion(" V2.05.10\n"), "2.5.10", "trimmed, prefix and leading zeros dropped")
+        XCTAssertNil(UpdateChecker.normalizedReleaseVersion("v2.6.0-beta.1"))
+        XCTAssertNil(UpdateChecker.normalizedReleaseVersion("v2.5"))
+        XCTAssertNil(UpdateChecker.normalizedReleaseVersion("v1234567890.0.0"), "component would overflow comparisons")
+    }
+
+    /// With no early stamp, overlapping popover opens would each hit GitHub. One fetch at a time.
+    @MainActor
+    func testOverlappingChecksShareOneFetch() async {
+        var fetches = 0
+        var release: CheckedContinuation<Void, Never>?
+        let checker = UpdateChecker(currentVersion: "2.5.3", defaults: isolatedDefaults()) {
+            fetches += 1
+            if fetches == 1 { await withCheckedContinuation { release = $0 } }
+            return nil
+        }
+        let first = Task { await checker.check(minInterval: 1_800) }
+        while release == nil { await Task.yield() }
+        await checker.check(minInterval: 1_800)
+        XCTAssertEqual(fetches, 1, "a check while one is in flight returns without fetching")
+
+        release?.resume()
+        await first.value
+        await checker.check(minInterval: 1_800)
+        XCTAssertEqual(fetches, 2, "once the in-flight check finished (failed), the next one fetches")
+    }
+
     /// "Skip this version" hides the banner, but a later check must still know
     /// the release exists. Settings must not treat that as "already latest".
     @MainActor

docs/reference/defect-log.md

@@ -1102,6 +1102,16 @@ read_when:
   더 새 태그는 배너로 돌아온다. 회귀: `testSkippedReleaseStaysVisibleAndANewerOneReturnsToTheBanner`,
   `testShowAgainRestoresTheBannerAndUpdateUsesTheSkippedRelease`.
 
+- **쿨다운 스탬프는 *검증까지 통과한* 조회에만 찍는다.** `UpdateChecker.check` 가 GitHub 호출 *전에*
+  `lastChecked` 를 쓰면 네트워크·파싱 실패도 30분 쿨다운을 시작해, 팝오버를 다시 열어도(`minInterval`
+  디바운스) 재시도가 막히고 배너가 조용히 안 뜬다. "응답을 받았다"도 성공이 아니다 — 태그가 `MAJOR.MINOR.PATCH`
+  가 아니면(`v2.6.0-beta.1`·`latest`·`v2.5`) `isNewer` 가 숫자 아닌 부분을 0 으로 읽어 prerelease 가 정식과
+  같게, 쓰레기가 `0.0.0` 으로 비교된다. 실패·불안전 URL·지원하지 않는 태그는 스탬프하지 않고, 정규화
+  (`normalizedReleaseVersion`)까지 통과한 릴리스만 스탬프한다. 조기 스탬프가 없어지면 겹친 `check()` 가
+  동시에 나갈 수 있으므로 in-flight 가드도 둔다. 회귀(세 가드 각각 결함 주입으로 실패 확인):
+  `testFailedCheckDoesNotStartTheCooldown`, `testRejectedReleaseUrlDoesNotStartTheCooldown`,
+  `testMalformedOrPrereleaseTagIsAFailedCheckAndDoesNotBlockTheNextOne`, `testOverlappingChecksShareOneFetch`.
+
 - **`pgrep -x <name>` 은 실행 파일의 정체성 검사이지, 기다리는 특정 프로세스에 대한 검사가 아니다.**
   중복 인스턴스가 떠 있는 동안 실행될 수 있는 모든 wait-for-exit 루프는 PID를 받아야 한다. `UpdateChecker`가
   자동 업데이트 시 앱 종료를 기다릴 때 `pgrep -x PokeTokenBar`를 쓰면, 중복 인스턴스가 살아있는 동안 루프를