Bug #373: ReaderActivity.saveNote reads the popover colour inside the app-scope coroutine — pins the Activity and can persist the wrong colour

[lllyys]

Problem

ReaderActivity.saveNote reads the popover's colour inside the app-scope coroutine:

// ReaderActivity.kt:682
container.appScope.launch { annotations.updateHighlight(id, popoverVm.state.value.activeColor, note) }

popoverVm.state.value.activeColor is evaluated at execution time, not at tap time. Two consequences.

1. Leak. Reaching the popoverVm property from inside the lambda captures this@ReaderActivity, so an app-scope job pins a finished Activity for the duration of the write. That is exactly the class feature #165 WI-7's round-4 audit fixed one week earlier.

2. Correctness — the interesting one. In the EDIT branch clearSelectionAndDismiss() has already run by the time the coroutine is scheduled, so whatever that dismiss (or a fast re-tap) writes to the popover VM is what gets persisted. A user saves a note on a yellow highlight and can get whatever colour the VM holds a moment later.

Why this reads as unintentional

Its own siblings in the same file do it correctly:

• editHighlightColor (:672-676) reads popoverVm.state.value.noteDraft into a local before launch, then launches with only id, color, note.
• createHighlight (:660-668) snapshots inputs and current first.

Only saveNote's id != null branch reads inside the lambda.

Repro

Deterministic with a paused dispatcher: tap save-note on a highlight of colour A, mutate the popover VM's activeColor to B before the coroutine runs, let it run, read the persisted row → colour B.

Fix

One line — snapshot popoverVm.state.value.activeColor into a local before launch, matching :674. But it needs a RED test that fails on the current code first: assert the persisted colour equals the colour at tap time after mutating the VM between the tap and the coroutine running.

Found by

Feature #142's Gate-2 round 3, outside that plan's write-set — the plan author was pinning its own coroutine-scope rules against local precedent and noticed the precedent was wrong. Orchestrator-verified by reading :660-692; the three-way contrast with the two correct siblings is in the source.

#142's plan carries a binding WI-5 brief instruction not to copy this pattern into the AZW3 host, and its Gate 4 should read that diff against it specifically.

Source of truth: docs/bugs.md