docs: HANDOFF round-2 review followup + move review 2 into notes/

Append a 'Review followup — Round 2' section to HANDOFF.md documenting
the round-2 Gemini-review fixes:

- Critical: request-reorder desync (SnapshotSequencer assumed client send
  order == server processing order; not true on the wire). Fixed by
  promoting the per-hour pendingHours Set to a single global pendingHour
  mutation lock — serializing toggle POSTs eliminates the reorder window
  entirely.
- High: toggle endpoint guessed user intent from server state, which
  combined with any desync to invert clicks. Fixed by requiring an
  explicit action: 'on' | 'off' field on every request.
- Low: round-2 R2-3 (retry storm re-probes succeeded accounts) noted as
  a deliberate trade-off, not fixed.

Move the round-2 review report from the project root into
notes/claude-reset-review-2.md to match the convention established for
the round-1 report.

Net delta vs branch base: 431 tests (was 427 after round 1; +4 contract
tests for the explicit-action endpoint). Biome and svelte-check clean.

2 files changed, 115 insertions, 0 deletions

diff --git a/HANDOFF.md b/HANDOFF.md
index 54fd6c4..3ea711a 100644
--- a/HANDOFF.md
+++ b/HANDOFF.md
@@ -373,3 +373,70 @@ remain as documented in §"Assumptions / known gaps" above:
   traffic. (Documented in gap #8.)
 - **DST drift.** Adding `24h` to an absolute Unix ts ignores DST
   transitions; documented in gap #1.
+
+---
+
+## Review followup — Round 2 (Gemini review pass — `notes/claude-reset-review-2.md`)
+
+After the round-1 fixes shipped, a second Gemini review pass surfaced one
+**Critical** and one **High** finding which together exposed a real desync
+hazard: the round-1 SnapshotSequencer only protected against RESPONSE
+reordering, but the toggle endpoint was vulnerable to REQUEST reordering,
+and the toggle endpoint itself ignored client intent — combining into
+"clicks feel inverted" when the UI got desynced.
+
+Both are now fixed on this branch.
+
+| # | Sev | Where | Symptom | Fix |
+|---|---|---|---|---|
+| R2-1 | Critical | `ClaudeReset.svelte` + `snapshot-sequencer.ts` | If two concurrent toggle POSTs reorder on the WIRE (B reaches server first), the server's truer post-A snapshot carries the OLDER client seq → SnapshotSequencer discards it as stale → UI permanently desyncs from server. Round-1's per-hour-counter fix was replaced with a global sequencer but BOTH had this blind spot. | Replaced per-hour `pendingHours: Set<number>` with a single global `pendingHour: number \| null` mutation lock. While any POST is in flight, ALL toggle buttons are disabled — mutations are now serialized on the client, so the server never sees two concurrent toggle requests. Sequencer retained for the GET-on-mount vs first-click race (which the global lock doesn't cover). |
+| R2-2 | High | `models.ts` POST `/wake-schedule/toggle` | Server decided add-vs-remove from its own in-memory state instead of an explicit request field. Any UI desync (from R2-1 or any future cause) → user clicks to turn on an hour the UI shows off, server sees it on, deletes it → "click inverted" UX, recoverable only by reload. | Toggle endpoint now requires explicit `action: 'on' \| 'off'`. Idempotent: `'off'` on already-off is a no-op success; `'on'` on already-on REPLACES timestamps (so a recovering UI can re-assert wall-clock intent without a delete-then-add round trip). Missing/invalid action → 400. |
+| R2-3 | Low (deferred) | `models.ts` `wakeAllClaudeAccounts` / `processPendingRetry` | If 1 of N accounts fails a probe, the 6 × 5min retry loop re-probes ALL accounts (including the ones that already succeeded). Wastes bandwidth, but the probe payload is tiny (~16 tok) and the constant 30-min budget caps the blast radius. | **Not fixed** — explicit deliberate trade-off; per-account success tracking inside `PendingRetry` would meaningfully complicate the retry path for marginal savings. Noted in §"Assumptions / known gaps". |
+
+### Files changed in round 2
+
+- **Modified:** `packages/api/src/routes/models.ts` — toggle endpoint
+  rewritten to require explicit `action` (`+27 / -6` LoC, idempotency rules
+  documented inline).
+- **Modified:** `packages/api/tests/routes.test.ts` — `toggle()` helper
+  auto-derives `action` from `timestamps` presence so the existing 12 tests
+  stayed terse; one test (`POST toggle rejects missing timestamps on add`)
+  was renamed to `rejects action='on' with missing timestamps` and now
+  passes `action` explicitly. **+4 new contract tests** (29 / 29 routes
+  tests pass):
+  - `POST toggle requires explicit action: 'on' | 'off'` (rejects missing
+    action, rejects non-`'on'/'off'` strings/numbers/`null`).
+  - `POST toggle action='off' is idempotent on an already-off hour`.
+  - `POST toggle action='on' on an already-on hour REPLACES timestamps`
+    (the recovery-from-desync scenario).
+  - `POST toggle action='off' ignores timestamps payload`.
+- **Modified:** `packages/frontend/src/lib/components/ClaudeReset.svelte`
+  — `pendingHours: Set<number>` → `pendingHour: number | null`; all 4 row
+  buttons gated by the global lock; `toggleHour` derives `action` from
+  local state; `postToggle` sends it on the wire. Per-hour
+  `cursor-wait` class is preserved for the in-flight hour as a UX cue.
+- **New:** `notes/claude-reset-review-2.md` — the round-2 review (kept
+  for audit trail, parallel to `notes/claude-reset-review.md`).
+
+### Verification (after round-2 followup)
+
+```
+$ bun run check
+Checked 144 files in 161ms. No fixes applied.
+
+$ bun run test
+Test Files  26 passed (26)
+     Tests  431 passed (431)
+
+$ bun run --cwd packages/frontend typecheck
+svelte-check found 0 errors and 0 warnings
+```
+
+(`+4` tests vs round 1: the four explicit-action contract tests.)
+
+### What's NOT addressed in round 2
+
+- **DST drift** — unchanged design trade-off (gap #1).
+- **No snapshot polling** — unchanged design trade-off (gap #8).
+- **R2-3 (retry storm re-probes succeeded accounts)** — deliberate trade-
+  off, see table above.
diff --git a/notes/claude-reset-review-2.md b/notes/claude-reset-review-2.md
new file mode 100644
index 0000000..31e5789
--- /dev/null
+++ b/notes/claude-reset-review-2.md
@@ -0,0 +1,48 @@
+# Review: Dispatch — Claude Wake Schedule (r1/claude-reset-fix)
+
+## Executive Summary
+
+**HOLD.** While the backend restructuring for the 4-probe coalescing is elegant and the database migrations are sound, the attempt to fix the frontend snapshot race condition is fundamentally flawed. The new `SnapshotSequencer` utility drops server responses based on client-side sequencing, but fails to account for network reordering on the *upstream* path. Under rapid user interaction, this will permanently desync the UI from the server. This desync is severely compounded by a design flaw in the toggle endpoint itself, which ignores explicit client intent. 
+
+## Findings
+
+### 1. `SnapshotSequencer` fails to prevent UI desync under concurrent POSTs (Critical)
+**Location:** `packages/frontend/src/lib/snapshot-sequencer.ts` and `packages/frontend/src/lib/components/ClaudeReset.svelte`
+**Issue:** The `SnapshotSequencer` assumes that a later client request will always contain a strictly fresher view of the server's global state. However, concurrent `fetch` calls can overtake each other on the network. If a user clicks 9 AM (Request A, seq 1) and immediately clicks 10 AM (Request B, seq 2):
+1. Network jitter causes Request B to be received and processed by the server *first*.
+2. The server toggles 10 AM and returns the global state `{10}`.
+3. The server then receives Request A, toggles 9 AM, and returns `{9, 10}`.
+4. Regardless of which response arrives at the client first, the sequencer dictates that seq 2 (the `{10}` snapshot) is the "winner" and seq 1 (the `{9, 10}` snapshot) is dropped.
+**Impact:** The server state is `{9, 10}` but the UI is permanently stuck at `{10}`.
+**Recommended Fix:** The simplest fix is to prevent concurrent mutations entirely. Replace the per-hour `pendingHours` lock with a global `isMutating` boolean that disables all toggle buttons while *any* POST request is in flight. The `SnapshotSequencer` can remain to protect against the `loadFromServer()` vs. user click race, but mutating requests must be sequentially ordered by the client.
+
+### 2. Toggle endpoint ignores client intent, compounding desync (High)
+**Location:** `packages/api/src/routes/models.ts` (~line 924)
+**Issue:** The `POST /wake-schedule/toggle` endpoint blindly determines whether to turn an hour ON or OFF based on its *own* local state (`if (wakeSchedule[hour] !== undefined) ...`). It does not validate the client's intent. If the UI suffers the desync described in Bug 1 (the UI thinks 9 AM is OFF, but the server knows it is ON), the user will naturally click 9 AM to turn it on. The client sends `{ hour: 9, timestamps: {...} }` (clearly intending to turn it ON). The server, seeing `wakeSchedule[9]` is already defined, ignores the timestamps and toggles the hour OFF. 
+**Impact:** The user's clicks feel broken. They try to turn an hour on, but the server invisibly turns it off, keeping the UI looking broken.
+**Recommended Fix:** Make the toggle endpoint idempotent. Require the client to send an explicit intent (e.g., `{ action: 'on', timestamps }` or `{ action: 'off' }`). If the client says "on" but it's already on, simply update the timestamps (or no-op) and return the current snapshot without deleting the hour.
+
+### 3. Retry storm re-wakes successful accounts (Low)
+**Location:** `packages/api/src/routes/models.ts` (`wakeAllClaudeAccounts` and `processPendingRetry`)
+**Issue:** If a user has multiple Anthropic accounts and only *one* of them fails a wake probe (e.g., due to expired credentials), `recordWake` returns `false`. This triggers the scheduler's background retry loop for 30 minutes. Every 5 minutes, `processPendingRetry` calls `wakeAllClaudeAccounts()`, which will re-probe *all* configured accounts, including the ones that already succeeded.
+**Recommended Fix:** Likely acceptable as a known trade-off since the Anthropic probe payload is tiny (16 tokens) and cheap, but it's worth noting as a minor waste of resources. To fix, you would need to track per-account success state in the `pendingRetry` object.
+
+## Verification of prior-review fixes
+
+1. **Clock skew / latency rejects valid toggles (High):** **FIXED.** The server now correctly accepts finite past timestamps and relies on the tick loop to advance them safely.
+2. **Race condition in global snapshot (High):** **NOT FIXED.** The author added `SnapshotSequencer`, which handles out-of-order *responses* correctly, but fails catastrophically when the server processes the *requests* out-of-order (see Finding 1).
+3. **Missing transaction boundary (Medium):** **FIXED.** The `persistSchedule` function now wraps the DELETE and INSERT loop in a synchronous `db.transaction()`.
+4. **Redundant concurrency logic (Nit):** **FIXED.** The unreachable `inFlightSeq` was removed, leaving only `pendingHours`.
+5. **Masked boot recovery reason (Nit):** **FIXED.** The text `" (boot recovery)"` is successfully appended to the reason string.
+
+## Things the author got right
+
+* **Data-Loss Protection:** The `db.transaction()` wrapper safely prevents partial persistence wipes.
+* **Schema Boundary:** The SQLite destructive migration is perfectly executed. Using `PRAGMA table_info` to detect the old schema and dropping *only* the `wake_schedule` table correctly preserves user API keys, settings, and chunks.
+* **Math Purity:** Extracting `nextDailyAfter` and `recoverScheduleEntry` away from the Hono app allows them to be rigorously unit-tested, and those test cases cover the edge cases thoroughly.
+* **Tick Coalescing:** The transition from 1 probe to 4 probes was handled intelligently. Coalescing all due slots into a single API tick prevents wasteful bursts of HTTP requests on boot or when gracefully recovering missed schedules.
+
+## Open design questions / deferred items
+
+* **DST Transition Drift:** The scheduler continues to use `DAILY_INTERVAL_MS` (24h) absolute additions. This means that a scheduled 9:00 AM wake will drift to 10:00 AM or 8:00 AM when the user's local daylight saving time transitions, correcting only when the user manually toggles the UI.
+* **No Snapshot Polling:** As flagged in the previous review, the frontend still has no mechanism (polling or SSE) to react to background state changes. If a retry loop successfully completes in the background, the UI will continue to say "Retrying..." until the user refreshes the page or clicks another button.