fix: address Copilot review — proxy status header and test isolation

- worker.js: add X-Proxy-Status: ok/error header to distinguish
  "upstream confirmed no cheats" from "transient fetch failure",
  allowing clients to skip the direct-scrape fallback on confirmed
  empty results
- GameHackingOrgLookup: fetchFromProxy returns [CheatDatabaseEntry]?
  (nil = proxy error → fallback, [] = proxy ok no results → no fallback)
  uses X-Proxy-Status header to gate the nil vs [] decision
- Tests: add DirectScrapeBlockerProtocol to prevent real gamehacking.org
  network calls; add cannedHeaders support to ProxyCannedProtocol;
  rename proxyReturnsEmpty test to reflect no-fallback semantics

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: github-actions[bot]

PVLibrary/Sources/PVLibrary/Cheat/GameHackingOrgLookup.swift

@@ -94,12 +94,15 @@ public actor GameHackingOrgLookup {
 
         let proxyURL = resolvedProxyURL()
         if !proxyURL.isEmpty, Defaults[.useCheatProxy] {
+            // fetchFromProxy returns nil when the proxy is unreachable or fails (caller should
+            // fallback to direct scraping), or a non-nil array when the proxy successfully
+            // responded — including an empty array meaning "no cheats found" (no fallback needed).
             let proxyResults = await fetchFromProxy(title: title, systemSlug: systemSlug, proxyBaseURL: proxyURL)
-            if !proxyResults.isEmpty {
+            if let proxyResults {
                 DLOG("GameHackingOrgLookup: proxy returned \(proxyResults.count) codes for '\(title)'")
                 results = proxyResults
             } else {
-                DLOG("GameHackingOrgLookup: proxy empty, falling back to direct scrape for '\(title)'")
+                DLOG("GameHackingOrgLookup: proxy failed/unreachable, falling back to direct scrape for '\(title)'")
                 results = await fetchWithFallback(title: title, systemSlug: systemSlug)
             }
         } else {
@@ -128,8 +131,14 @@ public actor GameHackingOrgLookup {
     /// Fetch cheat entries from the Provenance cheat proxy worker.
     ///
     /// The proxy endpoint is `GET <proxyBaseURL>/cheats?title=<title>&system=<slug>`.
-    /// Returns an empty array when the proxy is unreachable or returns no results.
-    private func fetchFromProxy(title: String, systemSlug: String?, proxyBaseURL: String) async -> [CheatDatabaseEntry] {
+    ///
+    /// Returns:
+    /// - `nil` when the proxy is unreachable, returns a non-2xx status, or a network/decode
+    ///   error occurs — the caller should fall back to direct scraping.
+    /// - `[]` when the proxy successfully contacted upstream but found no cheats
+    ///   (signalled by `X-Proxy-Status: ok` in the response) — no fallback needed.
+    /// - `[entries]` when the proxy found results.
+    private func fetchFromProxy(title: String, systemSlug: String?, proxyBaseURL: String) async -> [CheatDatabaseEntry]? {
         var components = URLComponents(string: proxyBaseURL.hasSuffix("/")
             ? proxyBaseURL + "cheats"
             : proxyBaseURL + "/cheats")
@@ -141,7 +150,7 @@ public actor GameHackingOrgLookup {
 
         guard let url = components?.url else {
             WLOG("GameHackingOrgLookup: invalid proxy URL '\(proxyBaseURL)'")
-            return []
+            return nil
         }
 
         do {
@@ -152,10 +161,22 @@ public actor GameHackingOrgLookup {
             guard let http = response as? HTTPURLResponse,
                   (200..<300).contains(http.statusCode) else {
                 WLOG("GameHackingOrgLookup: proxy non-200 for '\(title)'")
-                return []
+                return nil
             }
 
             let raw = try JSONDecoder().decode([ProxyCheatEntry].self, from: data)
+            if raw.isEmpty {
+                // Only trust an empty result as "no cheats found" when the proxy confirms it
+                // successfully contacted upstream via X-Proxy-Status: ok.  Without this header
+                // the empty array may be a transient error — fall back to direct scraping.
+                let proxyStatus = http.value(forHTTPHeaderField: "X-Proxy-Status")
+                guard proxyStatus == "ok" else {
+                    DLOG("GameHackingOrgLookup: proxy returned empty without ok status for '\(title)' — falling back")
+                    return nil
+                }
+                return []
+            }
+
             return raw.enumerated().map { index, entry in
                 CheatDatabaseEntry(
                     id: Self.idOffset + index,
@@ -172,7 +193,7 @@ public actor GameHackingOrgLookup {
             }
         } catch {
             WLOG("GameHackingOrgLookup: proxy fetch error for '\(title)': \(error)")
-            return []
+            return nil
         }
     }
 

PVLibrary/Tests/PVLibraryTests/GameHackingOrgLookupTests.swift

@@ -126,7 +126,9 @@ final class GameHackingOrgLookupTests: XCTestCase {
         let json = #"[{"name":"Infinite Lives","code":"DEADBEEF00000001","category":"General"}]"#
         ProxyCannedProtocol.cannedJSON = Data(json.utf8)
         ProxyCannedProtocol.statusCode = 200
+        ProxyCannedProtocol.cannedHeaders = ["X-Proxy-Status": "ok"]
         ProxyCannedProtocol.lastRequest = nil
+        defer { ProxyCannedProtocol.cannedHeaders = [:] }
 
         Defaults[.useCheatProxy] = true
         Defaults[.cheatProxyURL] = "https://test.proxy.pvemu.invalid"
@@ -151,14 +153,27 @@ final class GameHackingOrgLookupTests: XCTestCase {
         XCTAssertTrue(entries.first?.isOnlineResult ?? false)
     }
 
-    func testSearchCheats_proxyReturnsEmpty_fallsThrough() async {
+    func testSearchCheats_proxyReturnsEmpty_noFallback() async {
+        // Proxy returns [] with X-Proxy-Status: ok — meaning "upstream confirmed no cheats".
+        // searchCheats should trust this and NOT fall back to direct scraping.
+        // DirectScrapeBlockerProtocol is registered to ensure no gamehacking.org request is made.
         URLProtocol.registerClass(ProxyCannedProtocol.self)
-        defer { URLProtocol.unregisterClass(ProxyCannedProtocol.self) }
+        URLProtocol.registerClass(DirectScrapeBlockerProtocol.self)
+        defer {
+            URLProtocol.unregisterClass(ProxyCannedProtocol.self)
+            URLProtocol.unregisterClass(DirectScrapeBlockerProtocol.self)
+        }
 
-        // Proxy returns empty array — direct scraping also yields nothing (no network in CI)
         ProxyCannedProtocol.cannedJSON = Data("[]".utf8)
         ProxyCannedProtocol.statusCode = 200
+        // X-Proxy-Status: ok tells the client the proxy successfully ran and found nothing
+        ProxyCannedProtocol.cannedHeaders = ["X-Proxy-Status": "ok"]
         ProxyCannedProtocol.lastRequest = nil
+        DirectScrapeBlockerProtocol.requestCount = 0
+        defer {
+            ProxyCannedProtocol.cannedHeaders = [:]
+            DirectScrapeBlockerProtocol.requestCount = 0
+        }
 
         Defaults[.useCheatProxy] = true
         Defaults[.cheatProxyURL] = "https://test.proxy.pvemu.invalid"
@@ -167,20 +182,31 @@ final class GameHackingOrgLookupTests: XCTestCase {
             Defaults.reset(.cheatProxyURL)
         }
 
-        let title = "ProxyEmptyFallback_\(UUID().uuidString)"
+        let title = "ProxyEmptyNoFallback_\(UUID().uuidString)"
         let entries = await GameHackingOrgLookup.shared.searchCheats(title: title, systemSlug: nil)
-        // Proxy was contacted but returned empty; direct scraping also fails offline — result is empty
+
+        // Proxy was contacted and returned empty with ok status — no direct scrape should happen
         XCTAssertTrue(entries.isEmpty)
         let intercepted = ProxyCannedProtocol.lastRequest?.url?.absoluteString ?? ""
-        XCTAssertTrue(intercepted.contains("/cheats"), "Proxy should still have been contacted even when empty")
+        XCTAssertTrue(intercepted.contains("/cheats"), "Proxy should have been contacted")
+        XCTAssertEqual(DirectScrapeBlockerProtocol.requestCount, 0, "Direct scrape should NOT occur when proxy confirms no cheats")
     }
 
     func testSearchCheats_proxyDisabled_doesNotContactProxy() async {
+        // Proxy is disabled — only the direct scrape path runs.
+        // DirectScrapeBlockerProtocol intercepts gamehacking.org requests so no real
+        // network call is made; it returns empty HTML so the scrape yields no results.
         URLProtocol.registerClass(ProxyCannedProtocol.self)
-        defer { URLProtocol.unregisterClass(ProxyCannedProtocol.self) }
+        URLProtocol.registerClass(DirectScrapeBlockerProtocol.self)
+        defer {
+            URLProtocol.unregisterClass(ProxyCannedProtocol.self)
+            URLProtocol.unregisterClass(DirectScrapeBlockerProtocol.self)
+        }
 
         ProxyCannedProtocol.cannedJSON = Data()
         ProxyCannedProtocol.lastRequest = nil
+        DirectScrapeBlockerProtocol.requestCount = 0
+        defer { DirectScrapeBlockerProtocol.requestCount = 0 }
 
         Defaults[.useCheatProxy] = false
         Defaults[.cheatProxyURL] = "https://test.proxy.pvemu.invalid"
@@ -202,6 +228,7 @@ final class GameHackingOrgLookupTests: XCTestCase {
 private final class ProxyCannedProtocol: URLProtocol {
     static var cannedJSON: Data = Data()
     static var statusCode: Int = 200
+    static var cannedHeaders: [String: String] = [:]
     static var lastRequest: URLRequest?
 
     override class func canInit(with request: URLRequest) -> Bool {
@@ -212,11 +239,15 @@ private final class ProxyCannedProtocol: URLProtocol {
 
     override func startLoading() {
         ProxyCannedProtocol.lastRequest = request
+        var headers = ["Content-Type": "application/json"]
+        for (key, value) in ProxyCannedProtocol.cannedHeaders {
+            headers[key] = value
+        }
         let response = HTTPURLResponse(
             url: request.url!,
             statusCode: ProxyCannedProtocol.statusCode,
             httpVersion: "HTTP/1.1",
-            headerFields: ["Content-Type": "application/json"]
+            headerFields: headers
         )!
         client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
         client?.urlProtocol(self, didLoad: ProxyCannedProtocol.cannedJSON)
@@ -225,3 +256,30 @@ private final class ProxyCannedProtocol: URLProtocol {
 
     override func stopLoading() {}
 }
+
+/// Intercepts requests to gamehacking.org and returns empty HTML, preventing real network calls
+/// during tests that exercise the direct-scrape fallback path.
+private final class DirectScrapeBlockerProtocol: URLProtocol {
+    static var requestCount: Int = 0
+
+    override class func canInit(with request: URLRequest) -> Bool {
+        request.url?.host?.contains("gamehacking.org") ?? false
+    }
+
+    override class func canonicalRequest(for request: URLRequest) -> URLRequest { request }
+
+    override func startLoading() {
+        DirectScrapeBlockerProtocol.requestCount += 1
+        let response = HTTPURLResponse(
+            url: request.url!,
+            statusCode: 200,
+            httpVersion: "HTTP/1.1",
+            headerFields: ["Content-Type": "text/html"]
+        )!
+        client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
+        client?.urlProtocol(self, didLoad: Data("<html><body></body></html>".utf8))
+        client?.urlProtocolDidFinishLoading(self)
+    }
+
+    override func stopLoading() {}
+}

Scripts/cheat-proxy/worker.js

@@ -75,7 +75,10 @@ export default {
             try {
                 const cached = await env[KV_NAMESPACE].get(cacheKey, { type: "json" });
                 if (cached !== null) {
-                    return jsonResponse(cached, { "X-Cache": "HIT" }, origin, env);
+                    // X-Proxy-Status: ok signals to clients that the proxy successfully
+                    // contacted upstream — an empty array here means "no cheats found",
+                    // not a transient error, so clients should skip the direct-scrape fallback.
+                    return jsonResponse(cached, { "X-Cache": "HIT", "X-Proxy-Status": "ok" }, origin, env);
                 }
             } catch (err) {
                 console.error("KV get error:", err);
@@ -104,7 +107,10 @@ export default {
             );
         }
 
-        return jsonResponse(results, { "X-Cache": "MISS" }, origin, env);
+        // X-Proxy-Status: ok when the upstream fetch succeeded (even with 0 results).
+        // X-Proxy-Status: error when the fetch itself threw (network/parse failure).
+        // Clients use this to decide whether to fall back to direct scraping.
+        return jsonResponse(results, { "X-Cache": "MISS", "X-Proxy-Status": fetchSucceeded ? "ok" : "error" }, origin, env);
     },
 };