docs: record wave 8 (J-crashsafe) in flight

The user's first scan on the wave-7 build crashed and never reproduced. Their
theory fits structurally — the Google Books Found path had never executed in
production before the key landed — but nine live lookups through the real
repository, including both of their ISBNs, threw nothing, so the parse/merge
code is exonerated by evidence rather than by argument. Written down so no
future worker "fixes" code that was measured working.

What the wave fixes is the defect found while looking: the lookup path catches
only IOException and the ViewModel catches nothing, so any other throwable kills
the process instead of reaching the user as "couldn't be reached" — and the app
has no crash capture at all, which is why one crash left no evidence.

Committed with an explicit pathspec: a wave is in flight (HAZARD #7).

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

docs/HANDOFF.md

@@ -627,3 +627,58 @@ because either alone would have prevented this:
 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.
+
+## Wave 8 — J-crashsafe: IN FLIGHT, launched 2026-09-12 17:50Z
+Prompt: `tasks/J-crashsafe.txt`. Session id in `logs/J-crashsafe.sid`. Lease
+`bookshelf-wave` held by `tasks/wave-guard.sh`, sentinel `logs/WAVE8-DONE`.
+
+**Why this wave exists.** The user installed the wave-7 build and the FIRST barcode
+scanned crashed the app: ISBN decoded and displayed, spinner ran a few seconds, process
+died. Never reproduced, on that book or any other. Their theory — that the scan fell
+through to Google Books and crashed there — is structurally the best fit: keyless GB
+429'd every caller on earth, so `GoogleBooksClient.classify`'s 2xx branch,
+`toBookMetadata()`, `normalizeCoverUrl()` and the two-source merge had NEVER EXECUTED
+in production before 486f6eb. A first crash belongs in a path's first real exercise.
+
+**It does not reproduce off-device, and that is now evidence rather than a guess.**
+`app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt`
+(commit 93546ed, opt-in on `LIVE_METADATA=1`) drives the real `MetadataRepository.lookup`
+against both real APIs with the real key. Nine lookups — both of the user's
+previously-failing ISBNs plus a control, three times each — all returned Found with
+cover art, 254ms to 4.7s, nothing thrown. Static review found no unsafe operation in
+that path either. **Do not let a future worker "fix" this by rewriting the parse/merge
+code; that code was exercised live and is fine.**
+
+**What the wave actually fixes** is the defect found while looking: nothing on that path
+is exception-safe. Both clients' `fetch` catch only `IOException`, `classify` catches
+only `SerializationException`/`IllegalArgumentException`, and `ScanViewModel.runLookup`
+sits inside `viewModelScope.launch { ...collect { } }` with no try/catch at all. So any
+throwable that is not an `IOException` — platform TLS, an OkHttp internal, memory
+pressure, an API-level difference, none of which this JVM reproduces — kills the
+process instead of surfacing as "couldn't be reached". Both clients' KDoc claims "Never
+throws"; that claim is false today. Plus: the app has NO crash capture whatsoever,
+which is why a one-time crash left nothing to work from.
+
+Scope, per the user's decision: (1) both clients catch `Throwable` -> new
+`FailureKind.UNEXPECTED`, non-retryable, reason names the exception class only (never
+its message — that can carry the key); (2) `ScanViewModel.runLookup` guards the same
+way, which also covers the Room call in it; (3) a `CrashReporter` that persists the
+stack trace and chains to the previous handler; (4) a Diagnostics section in settings to
+read and SHARE it off the phone. **`CancellationException` must be rethrown, not
+converted, in every one of those catches** — it is normal control flow here (dismissing
+the sheet cancels the lookup) and is the easiest thing in this wave to get wrong.
+
+**Verify before accepting** (workers self-report optimistically; several waves have
+over-claimed):
+    cd ~/bookshelf && ./tasks/gw assembleDebug && ./tasks/gw testDebugUnitTest \
+      && ./tasks/gw verifyPaparazziDebug && git status --porcelain
+**190 tests, 2 skipped today** (LiveSyncTest + LiveMetadataLookupTest, both opt-in
+live). Count from the `TEST-*.xml` files, not the console. The count must go UP.
+Grep the build log for `always 'false'`. Then eyeball the new settings Paparazzi PNGs.
+Check specifically that `CancellationException` is rethrown in every new catch, and
+that `CrashReporter` calls the previously-installed handler — a handler that does not
+chain leaves the process hung instead of dying.
+
+**Not in scope, deliberately:** finding the original throwing line. Nobody knows what it
+was and the prompt says not to hunt for it. If the guard lands and the user ever sees
+"unexpected: SomeException" on the scan sheet, THAT is when we learn the answer.

tasks/J-crashsafe.txt

@@ -0,0 +1,217 @@
+You are implementing ONE change in the Bookshelf Android app at ~/bookshelf.
+
+READ FIRST, in this order:
+  1. ~/bookshelf/docs/SPEC.md — the authoritative contract. Sections "Book metadata
+     lookup" and "Barcode scanning". Do not contradict it and do not edit it.
+  2. ~/bookshelf/docs/METADATA-SOURCES.md — section "The key landed" and the
+     "Measured again 2026-09-09" section above it. This is the evidence base.
+  3. The files you will change (listed in "What to build").
+
+## The bug, and what is already known — do NOT redo this investigation
+
+On 2026-09-11 the Google Books API key landed (commit 486f6eb). On the user's phone,
+the FIRST barcode scanned after installing that build **crashed the app**. The ISBN
+was decoded and displayed, the spinner ran for a few seconds, then the process died.
+It has never reproduced, on that book or any other.
+
+The orchestrator has already investigated. Do not repeat any of this:
+
+- **The structural suspicion is sound.** Keyless Google Books returned HTTP 429 to
+  every caller on the internet (a shared, exhausted daily quota — METADATA-SOURCES.md),
+  so before that commit `GoogleBooksClient.classify`'s 2xx branch, `toBookMetadata()`,
+  `normalizeCoverUrl()` and the two-source merge in `MetadataMerger` had NEVER ONCE
+  EXECUTED in production. A first crash belongs in a path's first real exercise.
+- **It does not reproduce off-device.** There is now an opt-in live test,
+  `app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt`,
+  which drives the real `MetadataRepository.lookup` against both real APIs with the
+  real key. Nine lookups — both of the user's previously-failing ISBNs plus a control,
+  three times each — all returned Found with cover art and nothing threw. Run it
+  yourself if you like: `LIVE_METADATA=1 GOOGLE_BOOKS_API_KEY=<key> ./tasks/gw
+  testDebugUnitTest --tests '*LiveMetadataLookupTest*'`. (Get the key from the
+  environment if it is set; do NOT read app/local.properties.)
+- Static review found no unsafe operation in that path: no `!!` outside a guarded
+  branch, no date parsing, no unsafe indexing, and `ui/components/BookCover.kt`
+  handles every `AsyncImagePainter.State`.
+
+**So do not go hunting for the throwing line. Nobody knows what it is, and you will
+not find it either.** Your job is to make the crash impossible whatever it was, and to
+make sure the next one leaves evidence. That is the whole wave.
+
+## The actual defect you ARE fixing
+
+The lookup path has no exception safety anywhere along it:
+
+- `GoogleBooksClient.fetch` catches **only `IOException`**. `OpenLibraryClient.fetch`
+  does the same. `classify` catches only `SerializationException` /
+  `IllegalArgumentException`.
+- `ScanViewModel.runLookup` is called from inside
+  `viewModelScope.launch { scannerController.scanResults.collect { ... } }` with **no
+  try/catch at any level**.
+
+Both clients' KDoc claims "Never throws: every outcome, including transport failure,
+comes back as a SourceResult rather than a swallowed null." **That claim is currently
+false** — it is true only for `IOException`. Anything else that escapes on a real
+device (the platform TLS stack, an OkHttp internal, memory pressure, an Android API
+level difference — none of which the JVM here reproduces) takes the whole process
+down instead of surfacing as "couldn't be reached".
+
+That is a real, provable defect regardless of whether it is what bit the user, and it
+is exactly the failure class this project already decided to eliminate in wave 5: a
+failure the user cannot distinguish from anything else.
+
+## What to build
+
+### 1. The clients genuinely never throw
+
+In BOTH `data/metadata/GoogleBooksClient.kt` and `data/metadata/OpenLibraryClient.kt`,
+widen the catch in `fetch` from `IOException` to any `Throwable`, mapping the
+unexpected case to a `SourceResult.Failed`.
+
+- Add a new `FailureKind` for it — `UNEXPECTED` — rather than reusing `TRANSPORT`.
+  It is NOT retryable (`RetryPolicy.isRetryable` must return false for it): we have no
+  idea what went wrong, so repeating it is as likely to repeat the damage as to help.
+- The `reason` must name the exception class, e.g. `"unexpected: IllegalStateException"`.
+  That string is rendered on the scan sheet and is our ONLY diagnostic channel from a
+  real phone. Include the class name; do NOT include the message (it can contain a URL,
+  and therefore the API key). `GoogleBooksClient.redact` still applies on top.
+- **`CancellationException` MUST be rethrown, not converted.** This is the one way to
+  get this wrong. Coroutine cancellation is normal control flow — the user dismissing
+  the sheet or leaving the screen cancels the lookup — and swallowing it turns a
+  cancelled coroutine into a "failed lookup" and breaks structured concurrency.
+  `kotlin.coroutines.cancellation.CancellationException`. Rethrow it first, before any
+  other handling.
+- Keep the existing `IOException` arm and its specific classification
+  (`SourceResult.fromException`) exactly as it is. `UNEXPECTED` is the fallthrough for
+  what that function never sees, not a replacement for it.
+
+### 2. The ViewModel survives anything the layer below does
+
+In `ui/scan/ScanViewModel.kt`, `runLookup` must not be able to kill the process. Wrap
+its body so that any `Throwable` becomes the existing "lookup failed" sheet state
+(`ScanSheetState.LookupFailed` — read `ScanModels.kt`; it already carries a reason
+string and already offers Retry and "Enter by hand", which is exactly the right
+affordance here).
+
+- Again: rethrow `CancellationException` first.
+- Note that `bookRepository.findByIsbn13(isbn13)` is inside this function too, so this
+  guard covers a Room/disk failure as well, not just the network.
+- The reason string should mark it as unexpected and name the exception class, so it is
+  distinguishable on the sheet from an ordinary source failure.
+- This is deliberately belt-and-braces with §1. Keep both: §1 keeps one source's
+  problem from destroying the other source's answer, §2 catches anything that escapes
+  anywhere else in the function.
+
+### 3. A crash leaves evidence
+
+New package `org.modg.bookshelf.diagnostics`. A `CrashReporter` that:
+
+- Installs a `Thread.setDefaultUncaughtExceptionHandler` from
+  `BookshelfApplication.onCreate`.
+- **Chains to the previously-installed handler at the end, always.** If you do not,
+  the process hangs instead of dying and Android never shows its dialog or records the
+  crash. Capture the previous handler before installing, and call it in a `finally`.
+- Writes ONE file per crash into the app's `filesDir`, containing: an ISO-8601
+  timestamp, the app `versionName`/`versionCode`, `Build.MODEL`, `Build.VERSION.SDK_INT`,
+  the crashing thread's name, and the full stack trace INCLUDING the cause chain
+  (`Throwable.printStackTrace` on a `PrintWriter` over a `StringWriter` gives you the
+  causes for free — do not hand-roll the walk).
+- Keeps only the most recent 5 and deletes older ones, so this cannot grow without
+  bound on a phone.
+- Cannot itself throw: the whole body goes in `runCatching`, because an exception
+  inside an uncaught-exception handler is the worst possible place to have one. Must
+  not do I/O on the main thread at startup — writing happens only when a crash
+  happens, and reading happens only when the user opens the screen in §4.
+
+### 4. The user can actually read it
+
+Add a "Diagnostics" section to the settings screen (`ui/settings/`). It shows the most
+recent crash — timestamp and the exception's first line — with a way to see the full
+trace and get it off the phone. Getting it off the phone is the entire point: the user
+is in another country from this machine and screenshots of a stack trace are painful.
+Offer a share action (`Intent.ACTION_SEND`, `text/plain`) and a copy-to-clipboard.
+When there are no crash files, say so plainly — an empty "Diagnostics" section that
+looks broken is worse than none.
+
+Match the existing settings screen's structure and visual language exactly; read it
+before writing. Follow SPEC's "Design language" section. This screen has Paparazzi
+coverage — add snapshots for the new section in both light and dark, in the same style
+as the existing settings snapshots, and record them.
+
+### 5. Update the comments you invalidate
+
+Both clients' class KDoc claims they never throw. After §1 that becomes true — say so
+accurately, and say WHY the `UNEXPECTED` kind exists (the crash above, undiagnosed,
+2026-09-12). This codebase comments with the evidence for a decision, not a restatement
+of the code. Match that.
+
+## Constraints — these are hard
+
+- Kotlin. Match the surrounding code's style, naming and comment density. Read
+  neighbouring files before writing and follow their conventions.
+- DO NOT touch: app/app/build.gradle.kts, gradle/libs.versions.toml, or any other
+  build file. If you believe you need a new dependency, STOP and say so in your report
+  instead — everything above is doable with what is already declared.
+- DO NOT touch anything under data/local, data/remote, data/repo, data/prefs, or
+  server/. You may edit AppContainer.kt and BookshelfApplication.kt only as far as §3
+  requires.
+- DO NOT read, print, cat, grep or echo `app/local.properties`, and never paste a real
+  API key into a source file, a test, or your report. Tests use a fake 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 when it finishes. Three previous workers on this project did
+  exactly that and could never report. A full build takes 1-3 minutes; wait for it.
+- There is no emulator on this box (no KVM). You cannot run the app. Do not claim you
+  did.
+
+## Definition of done
+
+Run these yourself and paste the real output:
+
+  ./tasks/gw assembleDebug          -> exit 0
+  ./tasks/gw testDebugUnitTest      -> exit 0. **190 tests pass today, 2 skipped**
+                                       (LiveSyncTest and LiveMetadataLookupTest, both
+                                       opt-in live tests). The count must go UP and
+                                       nothing may regress. Get the count by summing
+                                       the TEST-*.xml files, not by reading the console.
+  ./tasks/gw recordPaparazziDebug   -> exit 0, for the new settings snapshots (§4)
+  ./tasks/gw verifyPaparazziDebug   -> exit 0 against the re-recorded snapshots
+
+Required new tests, with REAL assertions (assertion-free tests are a spec violation
+on this project):
+  - each client returns `Failed(kind = UNEXPECTED)` — not a crash — when the HTTP call
+    throws a non-IOException `RuntimeException`. An `Interceptor` that throws is the
+    straightforward way to provoke this with the real client.
+  - the reason names the exception class and does NOT contain the exception's message.
+  - `CancellationException` propagates out of a client rather than becoming a `Failed`.
+    Test this explicitly. It is the easiest thing here to get wrong.
+  - `RetryPolicy.isRetryable(UNEXPECTED)` is false, and `withRetry` therefore makes
+    exactly one attempt.
+  - `ScanViewModel.runLookup` puts the sheet into LookupFailed (and does not throw)
+    when the repository throws; and a `CancellationException` is not swallowed.
+  - `CrashReporter` writes a file containing the exception class and its cause's
+    message; retains only the newest 5; and DELEGATES to the previously-installed
+    handler (assert the previous handler was actually invoked).
+
+Also: grep the build log for Kotlin warnings on the 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.
+
+## Report
+
+End with a plain report covering:
+  - what you changed, file by file
+  - the verbatim tail of each gradle command above
+  - the test count before and after, summed from the XML
+  - **your own judgement call on this**: does §1+§2 actually make the lookup path
+    non-crashing, or is there still a way for a throwable to reach the top? Say so
+    plainly if there is. Naming a remaining hole is worth more than a clean report.
+  - 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.