run-task.sh: stop reading the worker's own prose as a quota error

The quota detector grepped `cat "$LOG" "$ERR"` for strings like "429" and "usage
limit". $LOG holds the worker's final JSON, including its .result prose — so a
worker whose TASK was about HTTP 429 finished successfully, wrote a report saying
so, and the runner read its own worker's words, logged "QUOTA hit", slept 600s
and was about to --resume a session that had already succeeded. That would have
burned quota redoing finished work with a fresh worker loose on a completed tree.

Two independent defences, because either alone suffices:
  - the blob is now stderr plus the result text ONLY when is_error is true,
    falling back to the whole log when it isn't valid JSON (a hard crash writes
    no JSON, and has no prose to be confused by).
  - the success check runs BEFORE the quota and session-vanished checks. A
    finished worker is finished regardless of what strings its output contains.

Regression-checked against the real logs/I-gbkey.json that triggered this: the
old logic matches the quota pattern, the new logic yields SUCCESS.

Recorded as HAZARD #9 in docs/HANDOFF.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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,73 @@ 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.
+
+**FIXED 2026-09-12 in `run-task.sh` itself** (safe: no workers were running —
+the standing rule is only about editing it *while* a wave is live). Two defences,
+because either alone would have prevented this:
+1. The detector blob is now stderr plus `jq -r 'select(.is_error==true) | .result'`
+   — the worker's prose reaches it ONLY when the run actually errored. If the log
+   isn't valid JSON at all (a hard crash), it falls back to the whole log, where
+   there is no prose to be confused by.
+2. The success check moved ABOVE the quota and session-vanished checks. A finished
+   worker is finished regardless of what strings appear in its output.
+
+Regression-checked against the real `logs/I-gbkey.json` that caused this: the old
+logic matches the quota pattern, the new logic yields SUCCESS and feeds the
+detector an empty blob.

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.

tasks/run-task.sh

@@ -57,7 +57,27 @@ while [ "$(date +%s)" -lt "$deadline" ]; do
   fi
   rc=$?
 
-  blob="$(cat "$LOG" "$ERR" 2>/dev/null | head -c 20000)"
+  # HAZARD #9 — the detectors below must NEVER see the worker's own prose. The
+  # wave-7 worker's task was ABOUT HTTP 429; it finished successfully, wrote a
+  # report mentioning 429, and this runner read its own worker's words, declared
+  # a quota hit, and was about to --resume a session that had already SUCCEEDED.
+  # So: stderr always, plus the result text ONLY when the run actually errored.
+  # A hard crash may write no JSON at all, in which case fall back to the whole
+  # log — there is no prose to be confused by in that case.
+  if jq -e . "$LOG" >/dev/null 2>&1; then
+    blob="$( { cat "$ERR"; jq -r 'select(.is_error==true) | (.result // "")' "$LOG"; } 2>/dev/null | head -c 20000)"
+  else
+    blob="$(cat "$LOG" "$ERR" 2>/dev/null | head -c 20000)"
+  fi
+
+  # 0) success -> accept it before any error-string matching can second-guess it.
+  # This ordering is itself a guard: a finished worker is finished no matter what
+  # strings appear anywhere in its output.
+  isErr="$(jq -r '.is_error // false' "$LOG" 2>/dev/null)"
+  if [ "$rc" -eq 0 ] && [ "$isErr" != "true" ]; then
+    say "SUCCESS after $attempt attempt(s), $quota_waits quota wait(s)"
+    break
+  fi
 
   # 1) quota / rate limit -> wait it out, do NOT burn a hard-fail
   if printf '%s' "$blob" | grep -qiE 'usage limit|limit will reset|limit resets|rate_limit_error|rate limit exceeded|429|too many requests|overloaded_error'; then
@@ -74,14 +94,7 @@ while [ "$(date +%s)" -lt "$deadline" ]; do
     continue
   fi
 
-  # 3) success
+  # 3) genuine failure
-  isErr="$(jq -r '.is_error // false' "$LOG" 2>/dev/null)"
-  if [ "$rc" -eq 0 ] && [ "$isErr" != "true" ]; then
-    say "SUCCESS after $attempt attempt(s), $quota_waits quota wait(s)"
-    break
-  fi
-
-  # 4) genuine failure
   hard=$((hard+1))
   say "hard failure #$hard (rc=$rc is_error=$isErr)"
   if [ "$hard" -ge "$MAX_HARD_FAILS" ]; then say "GIVING UP after $hard hard failures"; break; fi