Use the Google Books API key: the fallback source actually answers now

The user obtained a restricted Google Books key. Keyless requests 429 for every
caller on the internet — all anonymous traffic bills to one shared Google Cloud
project whose daily quota is permanently exhausted — so the documented fallback
has never once answered. Because MetadataRepository.combine turns "a source
failed, none found" into Unavailable, that standing failure meant every Open
Library hiccup reached the user as "couldn't be reached". The app has been
effectively single-sourced since it was written.

Build plumbing reads GOOGLE_BOOKS_API_KEY from local.properties (gitignored),
falling back to the environment and then to empty. A blank key is a supported
state: a fresh clone still builds a working app that falls back to the keyless
endpoint, rather than failing to build.

GoogleBooksClient appends the key only when non-blank, building the URL with
HttpUrl.Builder in a pure requestUrl() so it is testable without a socket. The
key is scrubbed from SourceResult.Failed.reason before that string can reach the
scan sheet — it is rendered to the user and is our only diagnostic channel from a
real phone, and some okhttp/JDK IOExceptions embed the full request URL in their
message. Defensive, not a response to an observed leak.

Resolves the RATE_LIMITED decision parked in RetryPolicy's KDoc: a keyed 429 is
the short per-user rate limit and gets exactly one retry, honouring Retry-After
capped at 2s. A keyless 429 is still the dead daily quota and is still never
retried.

Verified against the live API, not only offline: both ISBNs that failed on the
phone (9781883937386, 9781883937676) plus a control return HTTP 200, in the
percent-encoded URL shape HttpUrl actually produces. Both books are in Google
Books, so the restored fallback now covers precisely the Open Library TLS-reset
failure that broke those scans.

189 unit tests (was 172), 0 failures; Paparazzi unchanged; release APK builds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J7WHnTx2Cso4VV245WDAJY

app/app/build.gradle.kts

@@ -21,6 +21,24 @@ val keystoreProperties = Properties().apply {
 }
 val hasReleaseKeystore = keystorePropertiesFile.exists()
 
+// Google Books API key. Lives in `local.properties` (gitignored) as
+// GOOGLE_BOOKS_API_KEY=..., or in the environment for CI. Absent is a supported
+// state: the build stays green and GoogleBooksClient falls back to the keyless
+// endpoint, which is what a fresh clone without the key gets.
+// The key is NOT a secret in the usual sense — it ships inside the APK and can be
+// extracted — but it is restricted to the Books API, and it must never be
+// committed. See docs/METADATA-SOURCES.md.
+val localPropertiesFile = rootProject.file("local.properties")
+val localProperties = Properties().apply {
+    if (localPropertiesFile.exists()) {
+        localPropertiesFile.inputStream().use { load(it) }
+    }
+}
+val googleBooksApiKey: String =
+    (localProperties["GOOGLE_BOOKS_API_KEY"] as String?)
+        ?: System.getenv("GOOGLE_BOOKS_API_KEY")
+        ?: ""
+
 android {
     namespace = "org.modg.bookshelf"
     compileSdk = 37
@@ -34,6 +52,8 @@ android {
         versionName = "1.0"
 
         testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
+
+        buildConfigField("String", "GOOGLE_BOOKS_API_KEY", "\"$googleBooksApiKey\"")
     }
 
     signingConfigs {
@@ -64,6 +84,7 @@ android {
 
     buildFeatures {
         compose = true
+        buildConfig = true
     }
 
     testOptions {

app/app/src/main/java/org/modg/bookshelf/AppContainer.kt

@@ -114,7 +114,7 @@ class AppContainer(private val context: Context) {
             .build()
     }
 
-    val metadataRepository by lazy { MetadataRepository(metadataHttpClient, json) }
+    val metadataRepository by lazy { MetadataRepository(metadataHttpClient, json, BuildConfig.GOOGLE_BOOKS_API_KEY) }
 
     val syncEngine by lazy {
         SyncEngine(

app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksClient.kt

@@ -5,52 +5,83 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import kotlinx.serialization.SerializationException
 import kotlinx.serialization.json.Json
+import okhttp3.HttpUrl
 import okhttp3.OkHttpClient
 import okhttp3.Request
 
 /**
- * Google Books lookup — SPEC.md "Book metadata lookup" fallback source. No API key.
+ * Google Books lookup — SPEC.md "Book metadata lookup" fallback source. Takes an
+ * [apiKey] (see `AppContainer` / `BuildConfig.GOOGLE_BOOKS_API_KEY`) because a
+ * keyless request is permanently rate-limited: every anonymous caller on the
+ * internet shares one exhausted daily quota (docs/METADATA-SOURCES.md). A blank
+ * key (the default) sends the request keyless, unchanged from before — this
+ * matters for a fresh clone with no key configured, which must still build a
+ * working app rather than a broken one.
  * Never throws: every outcome, including transport failure, comes back as a
  * [SourceResult] rather than a swallowed null.
  */
 class GoogleBooksClient(
     private val httpClient: OkHttpClient,
     json: Json,
+    private val apiKey: String = "",
 ) {
     private val json = Json(from = json) { ignoreUnknownKeys = true }
 
     /**
-     * Retries transient failures per [RetryPolicy]. Note this source's standing
+     * Retries transient failures per [RetryPolicy]. A keyless 429 is the exhausted
-     * failure — keyless requests share one exhausted global quota and answer 429,
+     * global daily quota, which [RetryPolicy] deliberately does NOT retry — see its
-     * which [RetryPolicy] deliberately does NOT retry, so today this costs nothing
+     * KDoc. A keyed 429 is the much shorter per-user rate limit and is worth one
-     * and changes nothing here. See docs/METADATA-SOURCES.md.
+     * more ask, so [retryRateLimitedOnce] follows whether a key is configured.
      */
     suspend fun lookup(isbn13: String): SourceResult =
-        withContext(Dispatchers.IO) { withRetry { fetch(isbn13) } }
+        withContext(Dispatchers.IO) {
+            withRetry(retryRateLimitedOnce = apiKey.isNotBlank()) { fetch(isbn13) }
+        }
 
     /** Single un-retried attempt, for tests that need to count calls. */
     internal suspend fun lookupOnce(isbn13: String): SourceResult =
         withContext(Dispatchers.IO) { fetch(isbn13) }
 
-    private fun fetch(isbn13: String): SourceResult = try {
+    private fun fetch(isbn13: String): SourceResult = redact(
-        val request = Request.Builder()
+        try {
-            .url("https://www.googleapis.com/books/v1/volumes?q=isbn:$isbn13")
+            val request = Request.Builder().url(requestUrl(isbn13)).build()
-            .build()
+            httpClient.newCall(request).execute().use { response ->
-        httpClient.newCall(request).execute().use { response ->
+                classify(response.code, response.body.string(), response.header("Retry-After"))
-            classify(response.code, response.body.string())
+            }
+        } catch (e: IOException) {
+            SourceResult.fromException(e)
+        },
+    )
+
+    /**
+     * Package-visible pure function — no socket involved — so it's exhaustively
+     * unit-testable offline: the key is appended (URL-encoded) only when non-blank,
+     * and omitted entirely for the keyless default. Built with [HttpUrl.Builder]
+     * rather than string concatenation so query-parameter encoding is correct by
+     * construction rather than by hand.
+     */
+    internal fun requestUrl(isbn13: String): String {
+        val builder = HttpUrl.Builder()
+            .scheme("https")
+            .host("www.googleapis.com")
+            .addPathSegments("books/v1/volumes")
+            .addQueryParameter("q", "isbn:$isbn13")
+        if (apiKey.isNotBlank()) {
+            builder.addQueryParameter("key", apiKey)
         }
-    } catch (e: IOException) {
+        return builder.build().toString()
-        SourceResult.fromException(e)
     }
 
     /**
      * Package-visible pure function — no socket involved — so it's exhaustively
      * unit-testable offline (2xx-with-record, 2xx-without-record, 404, 429, 500,
      * malformed body). [parseResponse] is defined in terms of this so the two
-     * can never disagree about what a body means.
+     * can never disagree about what a body means. [retryAfterHeader] is the raw
+     * `Retry-After` header value, if any — threaded through so the retry loop can
+     * see it, but only 429 ever consults it (via [SourceResult.fromHttpCode]).
      */
-    internal fun classify(httpCode: Int, body: String?): SourceResult {
+    internal fun classify(httpCode: Int, body: String?, retryAfterHeader: String? = null): SourceResult {
-        if (httpCode !in 200..299) return SourceResult.fromHttpCode(httpCode)
+        if (httpCode !in 200..299) return SourceResult.fromHttpCode(httpCode, retryAfterHeader)
         if (body.isNullOrBlank()) return SourceResult.Failed("empty body", FailureKind.MALFORMED)
         return try {
             val dto = json.decodeFromString(GoogleBooksResponseDto.serializer(), body)
@@ -66,4 +97,20 @@ class GoogleBooksClient(
     /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */
     internal fun parseResponse(body: String): BookMetadata? =
         (classify(200, body) as? SourceResult.Found)?.metadata
+
+    /**
+     * [SourceResult.Failed.reason] is rendered on the scan sheet — our only
+     * diagnostic channel from a real phone (docs/METADATA-SOURCES.md) — so it must
+     * never carry the API key. Some okhttp/JDK IOExceptions embed the full request
+     * URL, key included, in their own message (`UnknownHostException`, SSL errors,
+     * and okhttp's own "Canceled" IOException variants differ by platform), so
+     * this scrubs defensively rather than trusting that no exception type ever
+     * will. Package-visible so the scrub itself is unit-testable without a socket.
+     */
+    internal fun redact(result: SourceResult): SourceResult =
+        if (result is SourceResult.Failed && apiKey.isNotBlank() && result.reason.contains(apiKey)) {
+            result.copy(reason = result.reason.replace(apiKey, "[REDACTED]"))
+        } else {
+            result
+        }
 }

app/app/src/main/java/org/modg/bookshelf/data/metadata/MetadataRepository.kt

@@ -16,9 +16,9 @@ class MetadataRepository(
     private val openLibraryClient: OpenLibraryClient,
     private val googleBooksClient: GoogleBooksClient,
 ) {
-    constructor(httpClient: OkHttpClient, json: Json) : this(
+    constructor(httpClient: OkHttpClient, json: Json, googleBooksApiKey: String = "") : this(
         OpenLibraryClient(httpClient, json),
-        GoogleBooksClient(httpClient, json),
+        GoogleBooksClient(httpClient, json, googleBooksApiKey),
     )
 
     suspend fun lookup(isbn: String): LookupResult {

app/app/src/main/java/org/modg/bookshelf/data/metadata/RetryPolicy.kt

@@ -27,6 +27,15 @@ object RetryPolicy {
     /** One original attempt plus two retries. Beyond this the marginal gain is noise. */
     const val MAX_ATTEMPTS = 3
 
+    /**
+     * Cap on how long we'll wait on a keyed 429's `Retry-After` before treating it
+     * as not worth honouring, and also the fixed backoff used when that header is
+     * absent or unparseable. The user is standing at a bookshelf: a per-user rate
+     * limit clears fast, so there's no reason to wait longer than this for the one
+     * retry [withRetry] grants it (see [isRetryable]'s KDoc on RATE_LIMITED).
+     */
+    const val RATE_LIMIT_RETRY_CAP_MILLIS = 2_000L
+
     /**
      * Stop starting NEW attempts once this much time has gone into a single source.
      * A backstop against pathological cases (every attempt hitting the slow tail),
@@ -41,19 +50,37 @@ object RetryPolicy {
      * [FailureKind.TRANSPORT] and [FailureKind.SERVER_ERROR] are transient and
      * cheap to re-ask. The rest are not:
      *  - TIMEOUT      — the budget is already spent; see the class KDoc.
-     *  - RATE_LIMITED — the source is explicitly asking us to stop. Hammering a
+     *  - RATE_LIMITED — the source is explicitly asking us to stop, and NOT
-     *                   quota is how an intermittent block becomes a permanent one,
+     *                   retryable here either: a keyless 429 is the exhausted
-     *                   and METADATA-SOURCES.md records that happening to this
+     *                   shared daily quota (METADATA-SOURCES.md), which never
-     *                   project's IP during research. When the Google Books API key
+     *                   clears within a session, so hammering it is pure waste and
-     *                   lands, revisit this: a keyed 429 is a per-second rate limit
+     *                   risks turning an intermittent block into a permanent one.
-     *                   and IS worth one Retry-After-respecting retry, unlike
+     *                   A KEYED 429 is a different animal — the much shorter
-     *                   today's keyless daily-quota 429, which never clears.
+     *                   per-user rate limit — and IS worth one retry, but that's a
+     *                   call only [GoogleBooksClient] can make (it knows whether a
+     *                   key is configured), so it opts in per-call via
+     *                   [withRetry]'s `retryRateLimitedOnce` rather than by
+     *                   changing this blanket answer.
      *  - CLIENT_ERROR — an identical request gets an identical answer.
      *  - MALFORMED    — same bytes, same parse failure.
      */
     fun isRetryable(kind: FailureKind): Boolean =
         kind == FailureKind.TRANSPORT || kind == FailureKind.SERVER_ERROR
 
+    /**
+     * Parses an HTTP `Retry-After` header (the plain delta-seconds form; Google
+     * Books does not send the HTTP-date form) into a wait capped at
+     * [RATE_LIMIT_RETRY_CAP_MILLIS]. Returns null — "don't honour this" — if the
+     * header is missing, blank, negative, or not a plain integer; [withRetry]
+     * falls back to [RATE_LIMIT_RETRY_CAP_MILLIS] itself in that case, so either
+     * way the wait stays short and fixed rather than whatever the server asked for.
+     */
+    fun retryAfterMillis(header: String?): Long? {
+        val seconds = header?.trim()?.toLongOrNull() ?: return null
+        if (seconds < 0) return null
+        return (seconds * 1000).coerceAtMost(RATE_LIMIT_RETRY_CAP_MILLIS)
+    }
+
     /**
      * Backoff before attempt number [nextAttempt] (2-based: the wait before the
      * first retry is `backoffMillis(2)`). 250ms then 750ms, plus up to 40% jitter
@@ -82,10 +109,21 @@ object RetryPolicy {
  *
  * [sleep] and [nowMillis] are injectable purely so tests can run the real policy
  * with no wall-clock delay; production callers use the defaults.
+ *
+ * [retryRateLimitedOnce] permits exactly one extra retry of a
+ * [FailureKind.RATE_LIMITED] failure, on top of whatever [RetryPolicy.isRetryable]
+ * already grants — see its KDoc. "Once" is enforced independently of
+ * [maxAttempts]: a second consecutive rate-limited failure always ends the loop,
+ * even if attempts remain in the budget. The wait before that one retry honours
+ * [SourceResult.Failed.retryAfterMillis] when the failure carries one, and
+ * [RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS] otherwise — never the normal
+ * exponential [RetryPolicy.backoffMillis], which is tuned for transient transport
+ * errors, not for a server explicitly asking us to wait.
  */
 suspend fun withRetry(
     maxAttempts: Int = RetryPolicy.MAX_ATTEMPTS,
     budgetMillis: Long = RetryPolicy.TOTAL_BUDGET_MILLIS,
+    retryRateLimitedOnce: Boolean = false,
     random: Random = Random.Default,
     nowMillis: () -> Long = { System.currentTimeMillis() },
     sleep: suspend (Long) -> Unit = { delay(it) },
@@ -94,13 +132,21 @@ suspend fun withRetry(
     val started = nowMillis()
     var last: SourceResult = attempt()
     var attemptsMade = 1
+    var rateLimitedRetryUsed = false
 
     while (attemptsMade < maxAttempts) {
         val failure = last as? SourceResult.Failed ?: return last
-        if (!RetryPolicy.isRetryable(failure.kind)) break
+        val rateLimitedRetry = failure.kind == FailureKind.RATE_LIMITED &&
+            retryRateLimitedOnce && !rateLimitedRetryUsed
+        if (!RetryPolicy.isRetryable(failure.kind) && !rateLimitedRetry) break
         if (nowMillis() - started >= budgetMillis) break
 
-        sleep(RetryPolicy.backoffMillis(attemptsMade + 1, random))
+        if (rateLimitedRetry) {
+            rateLimitedRetryUsed = true
+            sleep(failure.retryAfterMillis ?: RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS)
+        } else {
+            sleep(RetryPolicy.backoffMillis(attemptsMade + 1, random))
+        }
         last = attempt()
         attemptsMade++
     }

app/app/src/main/java/org/modg/bookshelf/data/metadata/SourceResult.kt

@@ -53,9 +53,16 @@ sealed interface SourceResult {
      * to the user as supplementary detail on the scan sheet and is our ONLY
      * diagnostic channel from a real phone — so it names the specific failure, not
      * a generic one. [kind] is the same fact in a form [RetryPolicy] can act on;
-     * nothing should ever parse [reason] to recover it.
+     * nothing should ever parse [reason] to recover it. [retryAfterMillis] is set
+     * only for a [FailureKind.RATE_LIMITED] failure whose response carried a usable
+     * `Retry-After` header (already parsed and capped — see [RetryPolicy]); a
+     * retryable failure without one falls back to a fixed backoff instead.
      */
-    data class Failed(val reason: String, val kind: FailureKind = FailureKind.TRANSPORT) : SourceResult
+    data class Failed(
+        val reason: String,
+        val kind: FailureKind = FailureKind.TRANSPORT,
+        val retryAfterMillis: Long? = null,
+    ) : SourceResult
 
     companion object {
         /**
@@ -89,11 +96,19 @@ sealed interface SourceResult {
         /**
          * Maps a non-2xx HTTP status to a specific failure. 429 is called out
          * separately from the rest of 4xx because it is the one client error that is
-         * about us rather than about the request, and because it is currently
+         * about us rather than about the request, and because it is Google Books'
-         * Google Books' permanent state — see docs/METADATA-SOURCES.md.
+         * permanent keyless state — see docs/METADATA-SOURCES.md. [retryAfterHeader]
+         * is the raw `Retry-After` header value, if any; only a 429 ever uses it, via
+         * [RetryPolicy.retryAfterMillis]. Passing it for another code is harmless
+         * (it's simply not consulted) — the parameter isn't restricted to 429 so
+         * callers don't need to know which codes care.
          */
-        fun fromHttpCode(code: Int): Failed = when {
+        fun fromHttpCode(code: Int, retryAfterHeader: String? = null): Failed = when {
-            code == 429 -> Failed("http 429 (rate limited)", FailureKind.RATE_LIMITED)
+            code == 429 -> Failed(
+                "http 429 (rate limited)",
+                FailureKind.RATE_LIMITED,
+                retryAfterMillis = RetryPolicy.retryAfterMillis(retryAfterHeader),
+            )
             code in 500..599 -> Failed("http $code (server error)", FailureKind.SERVER_ERROR)
             else -> Failed("http $code", FailureKind.CLIENT_ERROR)
         }

app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksClientTest.kt

@@ -3,7 +3,9 @@ package org.modg.bookshelf.data.metadata
 import kotlinx.serialization.json.Json
 import okhttp3.OkHttpClient
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
 import org.junit.Test
 
 /**
@@ -87,6 +89,74 @@ class GoogleBooksClientTest {
         assertEquals(SourceResult.Failed("malformed json", FailureKind.MALFORMED), result)
     }
 
+    // --- requestUrl(): the key must be present (and encoded) only when configured. ---
+
+    @Test
+    fun `requestUrl omits the key entirely when it is blank -- the keyless default`() {
+        val url = client.requestUrl("9780134685991")
+
+        assertTrue(url.contains("9780134685991"))
+        assertFalse(url.contains("key="))
+    }
+
+    @Test
+    fun `requestUrl appends the key when one is configured`() {
+        val keyed = GoogleBooksClient(OkHttpClient(), Json, apiKey = "test-key-123")
+        val url = keyed.requestUrl("9780134685991")
+
+        assertTrue(url.contains("key=test-key-123"))
+    }
+
+    @Test
+    fun `requestUrl url-encodes a key that needs it`() {
+        val key = "a key/with&chars"
+        val keyed = GoogleBooksClient(OkHttpClient(), Json, apiKey = key)
+        val url = keyed.requestUrl("9780134685991")
+
+        assertFalse("raw key must not appear unescaped in the url", url.contains(key))
+        // "key=" is the last query parameter added, so everything after it is the
+        // encoded value -- decode it back and confirm it round-trips to the original,
+        // rather than pinning to one specific percent-encoding scheme.
+        val encodedKey = url.substringAfter("key=")
+        assertEquals(key, java.net.URLDecoder.decode(encodedKey, "UTF-8"))
+    }
+
+    // --- classify() threads the Retry-After header into a 429's retryAfterMillis. ---
+
+    @Test
+    fun `classify carries a parsed Retry-After into the 429 failure`() {
+        val result = client.classify(429, null, retryAfterHeader = "1") as SourceResult.Failed
+        assertEquals(1_000L, result.retryAfterMillis)
+    }
+
+    @Test
+    fun `classify leaves retryAfterMillis null when no header is given`() {
+        val result = client.classify(429, null) as SourceResult.Failed
+        assertNull(result.retryAfterMillis)
+    }
+
+    // --- redact(): the API key must never survive into a shown/logged reason. ---
+
+    @Test
+    fun `redact scrubs the api key out of a reason that leaked the full keyed request url`() {
+        val key = "test-key-123"
+        val keyed = GoogleBooksClient(OkHttpClient(), Json, apiKey = key)
+        val leakedUrl = keyed.requestUrl("9780134685991")
+        check(leakedUrl.contains(key)) { "fixture assumption broken: url doesn't contain the key" }
+
+        val leaking = SourceResult.Failed("network error: connect to $leakedUrl failed", FailureKind.TRANSPORT)
+        val redacted = keyed.redact(leaking) as SourceResult.Failed
+
+        assertFalse("literal key must not survive redaction", redacted.reason.contains(key))
+        assertTrue(redacted.reason.contains("[REDACTED]"))
+    }
+
+    @Test
+    fun `redact is a no-op when there is no key configured or nothing to scrub`() {
+        val clean = SourceResult.Failed("tls connection reset", FailureKind.TRANSPORT)
+        assertEquals(clean, client.redact(clean))
+    }
+
     private fun fixture(name: String): String =
         checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" }
             .bufferedReader()

app/app/src/test/java/org/modg/bookshelf/data/metadata/RetryPolicyTest.kt

@@ -131,6 +131,101 @@ class RetryPolicyTest {
         assertEquals(transport, result)
     }
 
+    // --- a keyed 429 gets exactly one extra retry; a keyless one gets none ---
+
+    @Test
+    fun `a keyed 429 is retried exactly once`() = runTest {
+        var calls = 0
+        val result = withRetry(retryRateLimitedOnce = true, sleep = {}) {
+            calls++
+            if (calls == 1) SourceResult.fromHttpCode(429) else found
+        }
+        assertEquals(found, result)
+        assertEquals(2, calls)
+    }
+
+    @Test
+    fun `a keyless 429 is not retried at all`() = runTest {
+        var calls = 0
+        val result = withRetry(retryRateLimitedOnce = false, sleep = {}) { calls++; SourceResult.fromHttpCode(429) }
+        assertEquals(1, calls)
+        assertEquals(FailureKind.RATE_LIMITED, (result as SourceResult.Failed).kind)
+    }
+
+    @Test
+    fun `a second consecutive 429 ends it -- one retry means one, not up to MAX_ATTEMPTS`() = runTest {
+        var calls = 0
+        val result = withRetry(retryRateLimitedOnce = true, sleep = {}) { calls++; SourceResult.fromHttpCode(429) }
+        assertEquals(2, calls)
+        assertEquals(
+            SourceResult.Failed("http 429 (rate limited), 2 attempts", FailureKind.RATE_LIMITED),
+            result,
+        )
+    }
+
+    @Test
+    fun `the rate-limited retry waits on Retry-After when present and short`() = runTest {
+        val sleeps = mutableListOf<Long>()
+        var calls = 0
+        withRetry(retryRateLimitedOnce = true, sleep = { sleeps.add(it) }) {
+            calls++
+            if (calls == 1) SourceResult.fromHttpCode(429, "1") else found
+        }
+        assertEquals(listOf(1_000L), sleeps)
+    }
+
+    @Test
+    fun `the rate-limited retry caps a long Retry-After instead of honouring it`() = runTest {
+        val sleeps = mutableListOf<Long>()
+        var calls = 0
+        withRetry(retryRateLimitedOnce = true, sleep = { sleeps.add(it) }) {
+            calls++
+            if (calls == 1) SourceResult.fromHttpCode(429, "120") else found
+        }
+        assertEquals(listOf(RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS), sleeps)
+    }
+
+    @Test
+    fun `the rate-limited retry falls back to the cap when Retry-After is absent`() = runTest {
+        val sleeps = mutableListOf<Long>()
+        var calls = 0
+        withRetry(retryRateLimitedOnce = true, sleep = { sleeps.add(it) }) {
+            calls++
+            if (calls == 1) SourceResult.fromHttpCode(429) else found
+        }
+        assertEquals(listOf(RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS), sleeps)
+    }
+
+    @Test
+    fun `the rate-limited retry falls back to the cap when Retry-After is unparseable`() = runTest {
+        val sleeps = mutableListOf<Long>()
+        var calls = 0
+        withRetry(retryRateLimitedOnce = true, sleep = { sleeps.add(it) }) {
+            calls++
+            if (calls == 1) SourceResult.fromHttpCode(429, "not-a-number") else found
+        }
+        assertEquals(listOf(RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS), sleeps)
+    }
+
+    // --- Retry-After parsing itself ---
+
+    @Test
+    fun `retryAfterMillis honours a short header exactly`() {
+        assertEquals(1_000L, RetryPolicy.retryAfterMillis("1"))
+    }
+
+    @Test
+    fun `retryAfterMillis caps a long header at the rate-limit cap`() {
+        assertEquals(RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS, RetryPolicy.retryAfterMillis("120"))
+    }
+
+    @Test
+    fun `retryAfterMillis is null when the header is absent, unparseable, or negative`() {
+        assertEquals(null, RetryPolicy.retryAfterMillis(null))
+        assertEquals(null, RetryPolicy.retryAfterMillis("soon"))
+        assertEquals(null, RetryPolicy.retryAfterMillis("-5"))
+    }
+
     @Test
     fun `backoff is short and jittered, never zero and never seconds long`() {
         val r = Random(1234)

docs/HANDOFF.md

@@ -498,3 +498,66 @@ is our ONLY diagnostic channel from a real phone.** Nothing may parse it.
 - H2 (108 turns, $4.00) followed the brief closely, ran builds in the foreground,
   and reported honestly, including flagging its own stacked-sheet judgement call.
   Cost ratio to H1 ($0.66, 5 turns) is roughly the ratio of work actually done.
+
+## Wave 7 — I-gbkey (Google Books API key): COMPLETE, verified by the orchestrator 2026-09-11
+Prompt: `tasks/I-gbkey.txt`. The user asked for the key on 09-11, which lifts the
+standing "do not add it unasked" instruction recorded in wave 6.
+
+**Split of work, deliberately:** the ORCHESTRATOR did the build plumbing
+(`app/app/build.gradle.kts`: read `GOOGLE_BOOKS_API_KEY` from `local.properties`,
+enable `buildConfig`, emit `BuildConfig.GOOGLE_BOOKS_API_KEY`) and verified it
+green BEFORE launching the worker, because workers on this project are barred from
+build files. The Sonnet worker did the Kotlin against a tree where the key was
+already available. Reuse this pattern for anything needing a build-file change.
+
+| Check | Result |
+|---|---|
+| `./tasks/gw assembleDebug` | exit 0 |
+| `./tasks/gw testDebugUnitTest --rerun-tasks` | exit 0 — **189 tests**, 1 skipped, 0 failures (was 172) |
+| test count source | summed from `TEST-*.xml`, not the console |
+| `./tasks/gw verifyPaparazziDebug` | exit 0 — no pixels moved, as intended |
+| `./tasks/gw assembleRelease` | exit 0 — 41,810,740 bytes |
+| `grep "always 'false'"` on a `--rerun-tasks` build | **0 hits** |
+| boundary check | clean — worker touched only `data/metadata` + the one AppContainer line |
+| key-leak check | `git grep` finds no real key in the tree; tests use `test-key-123` |
+| **live API check** | HTTP 200 for both previously-failing ISBNs and a control |
+
+The worker's report was honest: every claim re-verified, including the test count,
+and it flagged its own judgement calls (how it reconciled the slightly ambiguous
+`Retry-After` cap wording) rather than papering over them. 51 turns, $1.59.
+
+**The live check was not ceremony.** `HttpUrl.Builder` percent-encodes the colon,
+so the app sends `q=isbn%3A...` where every earlier hand-run test sent `q=isbn:...`.
+Offline tests cannot distinguish those. Verified: the API accepts both.
+
+**Both books that failed on the phone are in Google Books** — so the restored
+fallback now covers exactly the Open Library TLS-reset failure mode that actually
+broke those two scans. The app has been effectively single-sourced since it was
+written and is now genuinely two-sourced. Details in `docs/METADATA-SOURCES.md`
+§ "The key landed".
+
+### HAZARD #9 — `run-task.sh` reads the WORKER'S OWN OUTPUT for quota strings
+`run-task.sh`'s quota detector greps the worker's result blob for
+`usage limit|...|429|too many requests|...`. That blob includes `.result` — the
+worker's own prose. **This wave's task was ABOUT HTTP 429**, so the moment the
+worker finished and wrote a report mentioning 429, the runner declared
+`QUOTA hit (wait #1)`, slept 600s, and was about to `--resume` a session that had
+already SUCCEEDED — which would have burned quota redoing finished work and let a
+fresh worker turn loose on a completed tree.
+
+Caught it by checking the log rather than trusting the state line:
+`jq '{is_error, subtype, num_turns}' logs/I-gbkey.json` said
+`is_error:false, subtype:"success", num_turns:51`. The orchestrator killed the
+runner (PID from `ps -o pid,args -p <pid>`, per HAZARD #6 — not `pkill -f`) before
+the sleep elapsed, and appended a note to `logs/<name>.state` saying why.
+
+**Before believing any `QUOTA hit` line, check whether the worker actually
+finished:** a non-empty `logs/<name>.json` with `is_error:false` means it
+SUCCEEDED and the runner is about to waste a session.
+
+The real fix, for whoever next touches the runner (write a NEW file; never edit
+`run-task.sh` while workers are running): the detector must read only the
+transport-level error stream, not `.result` prose — e.g. grep `$ERR` alone, or
+`jq -r 'select(.is_error==true) | .result'`, rather than `cat "$LOG" "$ERR"`.
+Leaving it as-is means any future wave whose subject matter mentions rate limits
+or 429 will loop this way.

docs/METADATA-SOURCES.md

@@ -108,7 +108,7 @@ Worth saying plainly, because it bounds how much weight the numbers carry:
 
 ## The options
 
-### A. Give Google Books an API key
+### A. Give Google Books an API key — **DONE 2026-09-11, see the end of this file**
 Free, 1,000 requests/day, no billing account required. Turns the fallback from
 "silently 429" into a working source. Roughly a dozen lines: a key in
 `local.properties` → `BuildConfig` → `&key=` on the query.
@@ -248,7 +248,8 @@ PERMANENT standing failure, so **every** Open Library hiccup became `Unavailable
 The app has effectively been single-sourced this whole time while reporting
 failures as though two sources had been consulted.
 
-**The user has deliberately deferred the API key.** Do not implement it unasked.
+**The user deferred the API key at the time; they asked for it on 2026-09-11 and it
+is now implemented and verified.** See "The key landed" at the end of this file.
 
 ### Open Library: 13% failure, and our own timeout was manufacturing more
 
@@ -292,3 +293,60 @@ scanning a box of books in sequence stays on the fast path after the first book.
 - The user's own 3-request sample showed 2 failures. That is consistent with 13%
   (p ~ 5%) but does not confirm it. If their phone reports "3 attempts" often,
   their network is worse than this one and the retry count deserves revisiting.
+
+## The key landed — 2026-09-11
+
+The user obtained a restricted Google Books API key and it is wired in
+(commit below). This closes option A, which had been the single biggest
+outstanding win in this file.
+
+### Verified live, not just in tests
+
+The key was tested from this machine against the real API before any code was
+written, and again afterwards in the exact URL shape the app now builds:
+
+| ISBN | Result |
+|---|---|
+| 9781883937386 *Hittite Warrior* | HTTP 200, 1 item |
+| 9781883937676 *Shadow Hawk* | HTTP 200, 1 item |
+| 9780140449136 *Crime and Punishment* (control) | HTTP 200, 1 item |
+
+**Both of the books that failed on the user's phone are in Google Books.** They
+were always in Open Library too — what actually failed was the TLS-stage
+connection reset documented above, not coverage. The significance is that the
+fallback would now cover exactly that failure mode: an Open Library transport
+error no longer leaves the lookup with nothing to fall back to. The app has been
+effectively single-sourced since it was written; it is now genuinely two-sourced.
+
+Note the second URL test was not redundant. `HttpUrl.Builder` percent-encodes the
+colon, so the app sends `q=isbn%3A9781883937386` where every hand-run test in this
+file sent `q=isbn:9781883937386`. Offline unit tests cannot tell those apart and
+the API accepts both — but that is a fact worth having measured rather than
+assumed, because the failure mode would have been a feature that passes every
+test and returns nothing on the phone.
+
+### What was built
+
+- The key lives in `app/local.properties` (gitignored) as `GOOGLE_BOOKS_API_KEY`,
+  read by `app/app/build.gradle.kts` into `BuildConfig.GOOGLE_BOOKS_API_KEY`,
+  falling back to an environment variable of the same name and then to empty.
+  **A blank key is a supported state** — a fresh clone builds a working app that
+  falls back to the keyless (429ing) endpoint rather than failing to build.
+- `GoogleBooksClient` takes the key and appends it only when non-blank, with URL
+  construction in a pure `requestUrl()` so it is testable with no socket.
+- **The key is scrubbed out of `SourceResult.Failed.reason`** before it can reach
+  the scan sheet. That string is rendered to the user and is our only diagnostic
+  channel from a real phone; some okhttp/JDK IOExceptions embed the full request
+  URL in their message, so the scrub is defensive rather than a response to an
+  observed leak. Do not remove it on the grounds that nothing currently leaks.
+- The `RetryPolicy` RATE_LIMITED decision parked in its KDoc is now resolved: a
+  **keyed** 429 is the short per-user rate limit and gets exactly one retry,
+  honouring `Retry-After` capped at 2s; a **keyless** 429 is still the dead daily
+  quota and is still never retried.
+
+### What this does NOT fix
+
+The 17% of books with no cover art anywhere is unchanged, and so is Open
+Library's ~13% transport failure rate. What changes is that a failure of one
+source is now much more likely to be covered by the other instead of surfacing
+as `Unavailable`.

tasks/I-gbkey.txt

@@ -0,0 +1,210 @@
+You are implementing ONE feature in the Bookshelf Android app at ~/bookshelf.
+
+READ FIRST, in this order:
+  1. ~/bookshelf/docs/SPEC.md — the authoritative contract, section "Book metadata
+     lookup". Do not contradict it and do not edit it.
+  2. ~/bookshelf/docs/METADATA-SOURCES.md — why this work exists. Read section
+     "Measured again 2026-09-09" in full; it is the evidence base for every
+     decision below.
+  3. The files you will change: data/metadata/GoogleBooksClient.kt,
+     data/metadata/MetadataRepository.kt, data/metadata/RetryPolicy.kt,
+     data/metadata/SourceResult.kt, AppContainer.kt, and their tests.
+
+## Background — what is already done, do NOT redo it
+
+The user has obtained a Google Books API key and put it in `app/local.properties`
+as `GOOGLE_BOOKS_API_KEY=...`. That file is gitignored.
+
+The ORCHESTRATOR has ALREADY done the build plumbing, and it is verified working:
+`app/app/build.gradle.kts` reads that property (falling back to the environment
+variable of the same name, then to empty string), enables the `buildConfig`
+feature, and emits `BuildConfig.GOOGLE_BOOKS_API_KEY`. `assembleDebug` is green
+and the generated BuildConfig carries the real key.
+
+So: **the key is already available to Kotlin code as
+`org.modg.bookshelf.BuildConfig.GOOGLE_BOOKS_API_KEY`.** You do not need to, and
+MUST NOT, touch any build file. If you think you need to, stop and say so in your
+report instead.
+
+The key has been tested live from this machine against all three of these ISBNs —
+9781883937386, 9781883937676, 9780140449136 — and returned HTTP 200 with the
+right titles. The key works. Your job is to make the app use it.
+
+## Why this matters (so you make the right call on ambiguity)
+
+Keyless Google Books returns HTTP 429 to EVERY caller on the internet, forever:
+all keyless traffic bills to one shared anonymous Google Cloud project whose daily
+quota is permanently exhausted. That is a measured fact, not a guess — see
+METADATA-SOURCES.md.
+
+The knock-on is the part that actually hurt the user. `MetadataRepository.combine`
+turns "at least one source Failed, none Found" into `Unavailable`. Google Books has
+been a PERMANENT standing failure, so **every** Open Library hiccup surfaced to the
+user as "one or more sources couldn't be reached". The app has effectively been
+single-sourced this whole time while reporting as though two sources were
+consulted. Making the key work restores the second source and, with it, the
+honesty of `Unavailable`.
+
+## What to build
+
+### 1. GoogleBooksClient sends the key
+
+Give `GoogleBooksClient` an `apiKey: String` constructor parameter, defaulting to
+`""`. Append `&key=<apiKey>` to the request URL ONLY when the key is non-blank.
+
+A blank key must keep today's exact behaviour (keyless request, which 429s). This
+is not a hypothetical: a fresh clone of this repo has no `local.properties` key,
+and its build must still produce a working app rather than a broken one.
+
+URL-encode the key (`java.net.URLEncoder` or okhttp's `HttpUrl.Builder`). Prefer
+`HttpUrl.Builder` — okhttp is already a dependency and it encodes query parameters
+correctly by construction.
+
+CRITICAL FOR TESTABILITY: there is no MockWebServer in this project and you must
+not add one. Put URL construction in a pure, package-visible function that takes
+the ISBN and returns the URL string, exactly the way `classify()` is already
+factored out for the same reason:
+
+    internal fun requestUrl(isbn13: String): String
+
+`fetch()` then calls it. That makes the whole feature unit-testable offline.
+
+### 2. The key must never leak into anything user-visible or logged
+
+`SourceResult.Failed.reason` is RENDERED ON THE SCAN SHEET and is our only
+diagnostic channel from a real phone. A `reason` that carried the request URL
+would put the API key on the user's screen and into any screenshot they send us.
+
+  - Never build a `reason` from the request URL.
+  - `SourceResult.fromException` derives its reason from exception type/message.
+    Some okhttp/JDK IOExceptions DO include the full URL in their message
+    (`java.net.UnknownHostException`, SSL errors, and okhttp's own
+    `java.io.IOException: Canceled` variants differ by platform). Add a redaction
+    step so that ANY reason string passing through has an API key replaced with a
+    placeholder before it can be stored or shown.
+  - Write a unit test that asserts the key does not appear in the reason for a
+    failure whose exception message contains the full keyed URL. Assert on the
+    literal key string you pass in — a test that only checks for "AIza" is not
+    good enough, because the test key you construct should not look real.
+
+Do not log the URL anywhere either. (`metadataHttpClient` in AppContainer has no
+logging interceptor today — do not add one.)
+
+### 3. Wire the key through (AppContainer + MetadataRepository)
+
+`MetadataRepository`'s convenience constructor `(httpClient, json)` builds both
+clients itself. Add a `googleBooksApiKey: String = ""` parameter to it and pass it
+to `GoogleBooksClient`. Keep the primary constructor (which takes the two clients)
+as it is — the tests use it.
+
+In `AppContainer`, pass `BuildConfig.GOOGLE_BOOKS_API_KEY` when constructing
+`MetadataRepository`. That single line is the ONLY change you may make to
+AppContainer.kt, and AppContainer.kt is the only file outside `data/metadata` you
+may touch at all.
+
+### 4. A keyed 429 is a different animal — make it retryable, once
+
+`RetryPolicy.isRetryable` deliberately refuses to retry `RATE_LIMITED`, and its
+KDoc explains why AND explicitly parks this decision for the wave you are now
+doing. Read that KDoc before writing anything here.
+
+The reasoning: today's keyless 429 is an exhausted DAILY quota that never clears
+within a scanning session, so retrying is pure waste and risks turning an
+intermittent block into a permanent one. A KEYED 429 is usually the per-user
+per-100-seconds rate limit instead, which does clear, and is worth exactly one
+more ask.
+
+Implement it narrowly:
+  - `withRetry` gains a parameter, default `false`, that permits ONE retry of a
+    RATE_LIMITED failure — e.g. `retryRateLimitedOnce: Boolean = false`.
+    Open Library's call site keeps the default and its behaviour must not change.
+  - `GoogleBooksClient` passes `apiKey.isNotBlank()` for it. Keyless stays
+    unretried, which preserves today's measured-correct behaviour.
+  - One retry means one: a second 429 is returned to the caller, never a third
+    attempt, regardless of `MAX_ATTEMPTS`.
+  - Respect a `Retry-After` header if the response carries one, but CAP the wait
+    at ~2 seconds. The user is standing at a bookshelf. If the header is absent,
+    unparseable, or over the cap, use a short fixed backoff instead of honouring
+    it. Note this means `classify()` — currently `(httpCode, body)` — needs the
+    header value to reach the retry decision; thread it through in whatever way
+    keeps `classify` a pure function, and keep its existing tests passing.
+  - Unit-test all of it with the injected `sleep`/`nowMillis` hooks `withRetry`
+    already has. No test may sleep on the real clock.
+
+Keep every existing guarantee in `withRetry` intact, including the "N attempts"
+suffix on a surviving failure's `reason`.
+
+### 5. Update the stale comments you invalidate
+
+Several comments now describe a world that no longer exists — the KDoc at the top
+of `GoogleBooksClient` says "No API key", its `lookup` KDoc explains why retrying
+costs nothing today, and `RetryPolicy`'s RATE_LIMITED bullet parks the decision you
+are implementing. Update each to describe the new behaviour. Do not leave a comment
+that contradicts the code; this project treats that as a defect.
+
+Do NOT edit docs/SPEC.md, docs/HANDOFF.md or docs/METADATA-SOURCES.md — the
+orchestrator owns those.
+
+## Constraints — these are hard
+
+- Kotlin. Match the surrounding code's style, naming and comment density. These
+  files are heavily commented with the EVIDENCE for each decision, not with
+  restatements of the code. Read neighbouring files before writing and follow that
+  convention.
+- DO NOT touch: app/app/build.gradle.kts, gradle/libs.versions.toml, or any other
+  build file. The plumbing is done and verified.
+- DO NOT touch anything under data/local, data/remote, data/repo, data/prefs, or
+  any ui/ package. AppContainer.kt is limited to the one line in §3.
+- DO NOT read, print, cat, grep or echo `app/local.properties` or any file that
+  might contain the key, and never paste a real key into a source file, a test, or
+  your report. Tests must use an obviously-fake key like "test-key-123".
+- DO NOT git commit, git add, or git push. The orchestrator commits. Leave your
+  work in the working tree.
+- Build with `./tasks/gw <task>` — NEVER `./gradlew` directly (tasks/gw is a
+  flock-serialized wrapper).
+- RUN BUILDS IN THE FOREGROUND. Do not background a Gradle build and end your turn
+  saying you will report later — two previous workers did exactly that and could
+  never report. A full build here takes 1-3 minutes; just wait for it.
+- There is no emulator on this box (no KVM). You cannot run the app.
+
+## Definition of done
+
+All of these must pass, and you must run them yourself and paste the real output:
+
+  ./tasks/gw assembleDebug          -> exit 0
+  ./tasks/gw testDebugUnitTest      -> exit 0. 172 tests pass today (1 skipped,
+                                       LiveSyncTest). The count must go UP and
+                                       nothing may regress.
+  ./tasks/gw verifyPaparazziDebug   -> exit 0. This change is not supposed to move
+                                       a single pixel; if a snapshot fails, you
+                                       changed something you should not have.
+
+Required new tests, with REAL assertions:
+  - requestUrl includes `key=` with a non-blank key, and omits it entirely when
+    the key is blank.
+  - a key needing URL-encoding is encoded.
+  - the key is redacted out of a Failed reason built from an exception whose
+    message contains the keyed URL (§2).
+  - a keyed 429 is retried exactly once; a keyless 429 is not retried at all;
+    a second 429 ends it.
+  - Retry-After is honoured when short, capped when long, ignored when absent or
+    unparseable.
+Assertion-free tests are a spec violation on this project.
+
+Also: check the build log for Kotlin warnings on files you touched. A warning
+reading "Check for instance is always 'false'" is NOT cosmetic — that exact
+warning hid a bug that blanked every book cover in this app for months. If you see
+it, you have written dead code; fix it.
+
+## Report
+
+End with a plain report covering:
+  - what you changed, file by file
+  - the verbatim tail of each of the three gradle commands
+  - the test count before and after
+  - anything you could NOT do, or did differently from these instructions, and why
+  - anything you noticed that looks wrong but was out of scope
+
+Be honest. Workers on this project have over-claimed success before, and the
+orchestrator independently re-verifies everything, so an inflated report only
+wastes a round trip.