Skip to content

Commit f9fb164

Browse files
pmaxhoganclaude
andcommitted
docs(m6): record recheck-3 fixes (R3-P1-1..R3-P2-2); round-4 ran, M6 closes after recheck-4
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent 2a03b39 commit f9fb164

1 file changed

Lines changed: 14 additions & 0 deletions

File tree

design/CODEX_NOTES.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,3 +803,17 @@ R2-P2-3).
803803
| R2-P2-4 (`settings.rs` + `state/mod.rs` + `sqlite.rs`, SPEC s18) | `schema.txt` only counted `accounts` + `backup_sources`. | New authoritative `KNOWN_STATE_TABLES` (every migration-defined table: accounts, backup_sources, file_state, file_state_fts, pending_ops, activity_log, settings, file_checksum_mismatch) + a `StateRepo::table_row_count(table)` method (allow-list guarded, since a table name cannot be a bound parameter). `build_schema_summary` now counts EVERY table. Tests: schema.txt contains a count line for every known table incl. file_state + pending_ops. |
804804

805805
Cross-cutting: backend/frontend contracts stayed in sync (the only UI change is the new `errors.internal.invalid_input` `en-US` locale entry; the DTO shapes are unchanged). The new sqlx `query!` (CAS SELECT) regenerated the workspace `.sqlx` offline cache (0 drift). All gates green: `cargo build/clippy(-D warnings)/test --workspace`, `build -p driven-app`, `deny check`, `fmt --check`; `pnpm lint/test:unit/build` (vue-tsc clean). Anti-fake-green stub sweep on the M6 non-test surface: zero `todo!`/`unimplemented!`/`unreachable!` (the planner/scanner `unimplemented!()` are pre-existing `#[cfg(test)]` FakeStateRepo doubles).
806+
807+
## M6 codex recheck-3 fixes (round 4: 3 P1 + 2 P2 - FINAL; M6 review CLOSES after recheck-4)
808+
809+
The codex recheck-3 (baseline 3af8fc8, M6 @ df46a0e) raised 3 P1 + 2 P2, all localized to the diagnostic redactor, the recovery-phrase reveal gate, and two scalar validators - mostly incomplete/regressed round-3 fixes. Round 4 is the FINAL M6 fix round (user-approved past the round-3 hard-stop). After this push, codex RECHECK-4 runs and the M6 review CLOSES regardless - whatever recheck-4 finds is documented as a residual; there is no round-5 / recheck-5. All 5 are fixed below.
810+
811+
| Finding | What was broken | How it was fixed |
812+
|---|---|---|
813+
| R3-P1-1 (`RecoveryPhraseReveal.vue` + `stores/setup.ts` + `SetupWizard.vue` + `AddSourceWizard.vue`) | `canFinish` gated only on `phraseAcknowledged`, so the confirm checkbox could be ticked while the phrase was still HIDDEN - a user could start encrypted backups they could never restore. | The reveal component now tracks `everRevealed` (latches true the first time the user reveals a present phrase), DISABLES the acknowledge checkbox until `everRevealed && hasPhrase` (with a "reveal first" hint via the new `recoveryPhrase.revealFirstHint` i18n key), and emits `update:revealed` so the parent gates Finish. The setup store adds `phraseRevealed` + `markPhraseRevealed(value)`; `canFinish = !hasRecoveryPhrase || (phraseRevealed && phraseAcknowledged)`; both are reset on a new phrase (`createFirstSource`) and on `reset()`, and `markPhraseRevealed(false)` force-clears the ack. `AddSourceWizard` mirrors this with a local `phraseRevealed` ref gating the reveal-step "Done" button. When the phrase prop changes the component re-locks (emits `revealed=false` + `confirmed=false`). Tests: dedicated `recovery-phrase-reveal.test.ts` (checkbox disabled until reveal; confirm only emitted after reveal+check; re-lock on phrase change; empty phrase never enables); setup-wizard store + walk tests assert acknowledge-without-reveal leaves Finish disabled, reveal+ack enables it, and a re-lock clears both. |
814+
| R3-P1-2 (`settings.rs:redact_token`, SPEC s18) | Redaction only caught an OAuth token when the WHOLE whitespace token started with `ya29.` / `1//`, so `refresh_token=1//...`, `"access_token":"ya29...."`, and `file_id=<id>` leaked secrets into the shareable bundle. | `redact_token` now splits each whitespace token on VALUE separators (`= : " ' , { } [ ] ( ) < > ; & ?`) and redacts each VALUE segment via a new `redact_value` helper (email / `ya29.` access token / `1//` refresh token / long opaque drive-id), re-emitting the separators + key names verbatim so the `key=` structure survives for debugging. Tests: a key=value line and a JSON snippet both redact the embedded refresh/access tokens + file id while keeping the key names + an adjacent `op=upload` field. |
815+
| R3-P1-3 (`settings.rs:replace_ci`, SPEC s18, no-panic-in-non-test) | `replace_ci` found match offsets in `haystack.to_lowercase()` then sliced the ORIGINAL `haystack` with them; a Unicode case fold that changes byte length yielded wrong spans -> mis-redaction or a PANIC on a non-char-boundary slice (a non-ASCII username/path during export). | `replace_ci` now walks the ORIGINAL char boundaries and, at each, attempts a case-insensitive char-by-char match (`ci_match_at`) of the (already-lowercased) needle - lowercasing each haystack char on the fly and handling multi-char case-fold expansion - so every returned span is a valid ORIGINAL-string byte range. Documented the caller contract (every call site passes a pre-lowercased needle; source roots are stored lowercased). Test (non-ASCII written as `\u{}` escapes to keep the source ASCII): a source root with an accented `e` (U+00E9) + sharp-s (U+00DF) redacts correctly and never panics; a dotted-capital-I (U+0130, a length-changing fold) input does not panic; an ASCII CI replace returns original-span slices. |
816+
| R3-P2-1 (`exclude.rs:validate_patterns`, DESIGN 18.8) | Caps were 1000 include + 1000 exclude and 4096 BYTES per pattern, but DESIGN 18.8 caps TOTAL patterns at 256 and per-pattern length at 512 CHARS. | Replaced `MAX_PATTERNS_PER_SIDE=1000` with `MAX_PATTERNS_TOTAL=256` (the COMBINED include+exclude count) and `MAX_PATTERN_LEN=4096`->`512` measured in CHARS (`check_one_pattern` now counts `chars()`). Tests: exactly the total cap + exactly the length cap accepted; one past the combined total (split across both sides) and one past the char length rejected on both sides. |
817+
| R3-P2-2 (`sources.rs:update_source`, DESIGN 18.8) | `update_source` accepted any `deep_verify_interval_secs` (0 = constant churn, `u32::MAX` = suppress for decades). | `settings.rs` exposes `DEEP_VERIFY_MIN`/`DEEP_VERIFY_MAX` (`pub(crate)`) and a `validate_deep_verify_interval(value)` helper sharing the SAME `check_range` bound the global settings validator uses (3600..=31_536_000), returning the stable `internal.invalid_input` s24 code; `update_source` calls it on the patch value BEFORE persisting. Test: 0, `u32::MAX`, just-below-min, and just-above-max rejected; the 7-day default + both inclusive bounds accepted. |
818+
819+
Cross-cutting: backend/frontend contracts stayed in sync (the only UI additions are the `recoveryPhrase.revealFirstHint` `en-US` key + the store's `phraseRevealed`/`markPhraseRevealed` surface; no DTO shape changed, no new sqlx query/migration). All gates green: `cargo build/clippy(-D warnings)/test --workspace` (539 passed, google_e2e + elevation honest gate-skip), `build -p driven-app`, `deny check`, `fmt --check`; `pnpm install` (lockfile unchanged), `pnpm lint/test:unit (43 passed)/build` (vue-tsc clean). Anti-fake-green stub sweep on the M6 non-test surface: zero `todo!`/`unimplemented!`/`unreachable!` (the planner/scanner/orchestrator `unimplemented!()` are pre-existing `#[cfg(test)]` Fake doubles, outside the touched surface).

0 commit comments

Comments
 (0)