docs(codex): log M6 recheck-1 fixes (R1-P1-1..R1-P2-4)

Append the recheck-1 round-2 fix table to the M6 section: atomic key+
source, preview token, fake-mode picker, fatal BYO creds, cold-start
config, overlap rejection, idempotent wizard re-entry, empty PKCE secret.

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

Author: pmaxhogan

design/CODEX_NOTES.md

@@ -761,3 +761,21 @@ confirm against the fake backend, including the B3 phrase-gated Finish and the
 C1 backend folder dialog). A real Playwright/WebDriver run against the built
 Tauri app is deferred to a local pre-release check (no headless Tauri WebDriver
 in the Windows-only PR gate); the vitest walk is the CI proxy.
+
+## M6 codex recheck-1 fixes (round 2: 4 P1 + 4 P2)
+
+The codex recheck-1 (baseline 3af8fc8, M6 @ 25b0b04) raised 4 P1 + 4 P2 - all
+tightenings of the round-1 fixes (atomicity, fatal-not-best-effort, the
+dialog-token rollout that missed `preview_exclusions`) plus a few untouched gaps.
+This is the FINAL fix round (recheck cap = 2); all 8 are fixed below.
+
+| Finding | What was broken | How it was fixed |
+|---|---|---|
+| R1-P1-1 (data-safety, `sources.rs`) | On the FIRST encrypted source, `ensure_master_key()` stored the keychain master key + stamped `accounts.encryption_master_key_id` BEFORE `upsert_source()`. A source-insert failure left the account "provisioned" but the user never got the phrase -> unrestorable encrypted backups, and a retry returned NO phrase. | The account-stamp + source-insert are now ATOMIC: new `StateRepo::insert_source_with_optional_master_key_stamp` does both in ONE sqlx transaction (sqlite override; a default impl covers test doubles). `add_source` splits master-key prep (`prepare_master_key`, which generates + stores the keychain key + encodes the phrase but does NOT stamp) from the atomic DB write; on a DB failure when a key was just generated it DELETES the keychain master key (`delete_master_key`) so the account is left unprovisioned and a retry re-reveals. Net invariant: either the command fully succeeds and returns the phrase, or it fully rolls back. Tests (`sqlite.rs`): forced FK-violation source insert rolls back the account stamp + leaves no orphan; retry succeeds; no-stamp path just inserts. |
+| R1-P1-2 (`sources.rs`) | `preview_exclusions` walked a RAW webview `PathBuf`, so a compromised renderer could enumerate arbitrary readable directories (the round-1 token rollout covered `add_source` + export but MISSED preview). | The DTO now carries `local_path_token` (a backend-minted dialog token) XOR `source_id`. For a NEW candidate the path is resolved by a NON-CONSUMING `AppState::peek_dialog_token` (so the later single-use `add_source` TAKE still works as the user re-runs the preview); for an EXISTING source it is resolved from `backup_sources.local_path` by id. A request with neither / a bad token is rejected. Frontend: `AddSourceWizard.loadPreview` sends the token, `SourceTable.loadEditPreview` sends `sourceId`; `ipc/types.ts` updated. Tests: `peek_dialog_token` is non-consuming + TTL/single-use preserved; the SourceTable vitest asserts preview-by-`sourceId`. |
+| R1-P1-3 (`sources.rs`) | `pick_drive_folder` always built a REAL `GoogleDriveStore`, ignoring `AppState::remote_mode()` - breaking the fake-remote wizard acceptance path and risking real-Google/keychain hits in fake/e2e runs. | Extracted `select_picker_store(remote_mode, account_id)`: `RemoteMode::Fake` builds an `InMemoryRemoteStore` + uses its synthetic root id (NO real store, NO keychain read); `RemoteMode::RealGoogleDrive` builds the live store + uses Drive's `"root"` alias. Test: `select_picker_store(Fake, random_id)` lists the fake root WITHOUT creds (a real-mode build would fail on the missing keychain entry). |
+| R1-P1-4 (`accounts.rs`) | `store_client_creds` was best-effort, so a keychain-write failure still let `finish_add_account` succeed - leaving an account that refreshes with env/default creds after restart and FAILS (the refresh token is bound to the minting client). | `store_client_creds` now returns `CommandResult<()>` (FATAL). Fresh-add: token + creds are stored BEFORE the account row; a creds failure rolls back the just-stored refresh token and returns the error (no half-account). Reauth: creds persist (fatal) BEFORE the account is flipped to `ok`, so it stays needs_reauth on failure. No account may exist that cannot refresh its own token. |
+| R1-P2-1 (`assembly.rs`) | Cold-start orchestrators always used `OrchestratorConfig::default()`, so persisted settings (scan cadence, bandwidth cap, metered/battery gates, VSS mode) only applied after a live edit. | `build_account` now reads `commands::settings::load_orchestrator_config(state)` at assembly time (the SAME loader `update_settings`/`reconfigure_account` use). Test (`assembly.rs`): a persisted non-default scan cadence + cap + gates are reflected in the cold-start config. |
+| R1-P2-2 (`sources.rs`, DESIGN s5.2.2) | Overlapping / nested source roots were not rejected; `add_source` canonicalised the new path but never compared it to existing roots. | New `reject_overlapping_root` canonicalises every existing `backup_sources.local_path` and rejects (stable `local.io_error`) when the candidate is an ancestor of, descendant of, or identical to any existing root (applied GLOBALLY per DESIGN, which does not scope it per-account); siblings are allowed. Checked BEFORE master-key generation so an overlap never provisions a key. Test: nested + ancestor + identical rejected, sibling allowed. |
+| R1-P2-3 (`stores/setup.ts`) | Leaving the encryption step always called `createFirstSource()`; going Back from confirm then Next again re-called it, but the one-shot folder token was already consumed -> the wizard wedged. | `createFirstSource` is now idempotent: it short-circuits when `sourceId` is already set (preserving the staged phrase + ack). Test: a second `createFirstSource` does NOT re-call `add_source` and does not error. |
+| R1-P2-4 (`CredentialsWalkthrough.vue`, DESIGN s6.1) | The UI required a non-empty client secret, but the backend + DESIGN allow an empty secret for PKCE installed-app clients. | `canSubmit` now requires only a non-empty client ID; the (possibly empty) secret is passed through. Tests: submit allowed + the empty secret forwarded with a client ID; still blocked with no client ID. |