docs(qa): record /code-review outcome — 8 fixes + 5 backlog refactors (IMP-CODE-08..11, IMP-UX-03)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
null 2026-07-10 22:58:41 -05:00
parent a17ce13f07
commit 25404ec4ed
1 changed files with 6 additions and 1 deletions

View File

@ -164,7 +164,7 @@ until you get a clean cycle.
| 1·data-overhaul | 2026-07-03 | `78ad63d`→`d53a64b` (dev) | **0 defects** (feature work) | — | ✅ **Data page overhaul, 7 batches, all green.** Two-pane goal layout + 5-state connection hero + `?section=` deep-linking + nav badges/health dots + lazy panes (B0B2); **new features** OFX/QFX import (B3), richer export JSON + date-range (B4), "Erase my data" danger zone (B5). Verified: server 139 tests, client 46, build; **axe on `/data` zero critical/serious** (added to `a11y.authed`); full probe suite 17/17. Section internals + the 7 SimpleFIN buttons untouched. | | 1·data-overhaul | 2026-07-03 | `78ad63d`→`d53a64b` (dev) | **0 defects** (feature work) | — | ✅ **Data page overhaul, 7 batches, all green.** Two-pane goal layout + 5-state connection hero + `?section=` deep-linking + nav badges/health dots + lazy panes (B0B2); **new features** OFX/QFX import (B3), richer export JSON + date-range (B4), "Erase my data" danger zone (B5). Verified: server 139 tests, client 46, build; **axe on `/data` zero critical/serious** (added to `a11y.authed`); full probe suite 17/17. Section internals + the 7 SimpleFIN buttons untouched. |
| 2·recon | 2026-07-10 | `d9545d6` (dev) | **6** (1× S2, 4× S3, 1× S4) + 4 IMP | — (find-only) | 🔁 **Blind-spot recon** (deps/dead-code/coverage audit, no UI batches). Logged: high-vuln deps invisible to the critical-only CI gate (QA-B0-02) · package.json 0.40.0 vs HISTORY v0.41.0 drift (QA-B0-03) · **WebAuthn ghost feature** — full backend + api wrappers, zero UI, and a login fall-through that 500-locked any user who enabled it (QA-B1-01, S2) · 38 `@ts-nocheck` files ⇒ vacuous server typecheck (QA-B0-04) · raw `err.message` 5xx leaks (QA-B13-02) · dead duplicate admin routes incl. an unauthenticated `/api/auth/has-users` (QA-B17-01). Verified-clean along the way: SQL building (whitelists+params), no `dangerouslySetInnerHTML`/`as any` in client, ICS feed token-gated, git hygiene, all 29 route files mounted. | | 2·recon | 2026-07-10 | `d9545d6` (dev) | **6** (1× S2, 4× S3, 1× S4) + 4 IMP | — (find-only) | 🔁 **Blind-spot recon** (deps/dead-code/coverage audit, no UI batches). Logged: high-vuln deps invisible to the critical-only CI gate (QA-B0-02) · package.json 0.40.0 vs HISTORY v0.41.0 drift (QA-B0-03) · **WebAuthn ghost feature** — full backend + api wrappers, zero UI, and a login fall-through that 500-locked any user who enabled it (QA-B1-01, S2) · 38 `@ts-nocheck` files ⇒ vacuous server typecheck (QA-B0-04) · raw `err.message` 5xx leaks (QA-B13-02) · dead duplicate admin routes incl. an unauthenticated `/api/auth/has-users` (QA-B17-01). Verified-clean along the way: SQL building (whitelists+params), no `dangerouslySetInnerHTML`/`as any` in client, ICS feed token-gated, git hygiene, all 29 route files mounted. |
| 2·fix | 2026-07-10 | `3c82c30`→HEAD (dev), tags `v0.41.1` | all 6 + 4 IMP fixed | **9 archived** (v0.41.1 + v0.42.0); QA-B0-04 continues as a program (38→29) | ✅ **Fix pass complete, ~30 commits.** Phases: bug fixes (lockout, 401-mid-session redirect, version sync + guard test, 20 err.message leak sites) → **standards unification: all 29 routes on the throw-pattern, eslint-enforced** (no-console + errorFormatter ban; exceptions documented in CODE_STANDARDS) → **passkey UI shipped** (+ virtual-authenticator e2e, WEBAUTHN_RP_ID deploy guard) → tag v0.41.1 → **full dependency modernization** (vite 8 · tailwind 4 · router 7 · openid-client 6 · eslint 10 · nodemailer 9 · SheetJS dist; 0 vulns; audit gate → high; prod-smoke caught & fixed 3 upgrade breakages: send absolute-path, CSP inline script, manualChunks; TS7 deferred on typescript-eslint peer cap) → dead-code sweep (knip clean + blocking; `doingmypart.jpg` kept after user catch — markdown-content lesson added to B0) → @ts-nocheck starter batch (38→29) → **automated control census** (own `census` project) → deploy checklist + renovate.json + weekly deps-audit workflow. Gates at close: ci green (252 server / 52 client), e2e 27, census+probe 33/33 ×3, OIDC smoke 44/44, PROD SMOKE PASS. | | 2·fix | 2026-07-10 | `3c82c30`→HEAD (dev), tags `v0.41.1` | all 6 + 4 IMP fixed | **9 archived** (v0.41.1 + v0.42.0); QA-B0-04 continues as a program (38→29) | ✅ **Fix pass complete, ~30 commits.** Phases: bug fixes (lockout, 401-mid-session redirect, version sync + guard test, 20 err.message leak sites) → **standards unification: all 29 routes on the throw-pattern, eslint-enforced** (no-console + errorFormatter ban; exceptions documented in CODE_STANDARDS) → **passkey UI shipped** (+ virtual-authenticator e2e, WEBAUTHN_RP_ID deploy guard) → tag v0.41.1 → **full dependency modernization** (vite 8 · tailwind 4 · router 7 · openid-client 6 · eslint 10 · nodemailer 9 · SheetJS dist; 0 vulns; audit gate → high; prod-smoke caught & fixed 3 upgrade breakages: send absolute-path, CSP inline script, manualChunks; TS7 deferred on typescript-eslint peer cap) → dead-code sweep (knip clean + blocking; `doingmypart.jpg` kept after user catch — markdown-content lesson added to B0) → @ts-nocheck starter batch (38→29) → **automated control census** (own `census` project) → deploy checklist + renovate.json + weekly deps-audit workflow. Gates at close: ci green (252 server / 52 client), e2e 27, census+probe 33/33 ×3, OIDC smoke 44/44, PROD SMOKE PASS. **Closing /code-review (8 finder angles + library-verified) surfaced and fixed 8 more:** tailwind-4 codemod killed the row-status tints (fractional bare opacity emits no CSS; invisible to the login-only screenshot baseline — `a00013d`) · session-expiry redirect falsely fired on /profile/change-password wrong-password (now keyed off requireAuth's distinct `AUTH_REQUIRED` code, not a path prefix) · OIDC v6 dropped the RFC 9207 `iss` param (every authentik login would fail) · v6 https-only discovery breaks internal http providers (`OIDC_ALLOW_INSECURE_HTTP` opt-in) · 2FA silently downgraded to password on any challenge error (now only the stale-flag case) · burned single-use 2FA challenges left a dead retry button (WebAuthn AND pre-existing TOTP — both reset to sign-in) · three multi-write handlers `db.transaction()`-wrapped (`2a07e76`, `a17ce13`). Larger refactors → IMP-CODE-08…11, IMP-UX-03. |
**Result key:** 🔄 in progress · 🔁 findings fixed, re-run required · ✅ clean (zero findings — QA complete) **Result key:** 🔄 in progress · 🔁 findings fixed, re-run required · ✅ clean (zero findings — QA complete)
@ -233,6 +233,11 @@ graduate to `roadmap.md`/`FUTURE.md`.
| IMP-CODE-05 | Code | `client/pages/SubscriptionsPage.tsx` (2,371 ln), `TrackerPage.tsx` (1,943 ln) | Largest two pages exceed the size BillModal was decomposed at (1,733). Same treatment: extract section components/hooks behind existing tests, behavior-preserving. SpendingPage (1,642) and BillsPage (1,586) next tier. | L | 🔵 Noted | | IMP-CODE-05 | Code | `client/pages/SubscriptionsPage.tsx` (2,371 ln), `TrackerPage.tsx` (1,943 ln) | Largest two pages exceed the size BillModal was decomposed at (1,733). Same treatment: extract section components/hooks behind existing tests, behavior-preserving. SpendingPage (1,642) and BillsPage (1,586) next tier. | L | 🔵 Noted |
| IMP-CODE-06 | Code | `db/data/` | ~~Runtime seed JSONs lived in `docs/`.~~ **Shipped `a31a774`:** advisory-filter + merchant-store JSONs (7 MB) moved to `db/data/` next to their loader in `db/database.cts`; fresh-DB seed verified (5000+5000 rows). | S | ✅ Shipped | | IMP-CODE-06 | Code | `db/data/` | ~~Runtime seed JSONs lived in `docs/`.~~ **Shipped `a31a774`:** advisory-filter + merchant-store JSONs (7 MB) moved to `db/data/` next to their loader in `db/database.cts`; fresh-DB seed verified (5000+5000 rows). | S | ✅ Shipped |
| IMP-CODE-07 | Code | `e2e/control-census.spec.js` | ~~Appendix E never filled manually.~~ **Shipped `92439e7` + `64ab37a`:** automated control census — role + accessible name of every visible interactive control on all 15 authed pages, snapshot-diffed each run; own `census` Playwright project ordered before the mutating probe specs so baselines always see pristine seed data. | M | ✅ Shipped | | IMP-CODE-07 | Code | `e2e/control-census.spec.js` | ~~Appendix E never filled manually.~~ **Shipped `92439e7` + `64ab37a`:** automated control census — role + accessible name of every visible interactive control on all 15 authed pages, snapshot-diffed each run; own `census` Playwright project ordered before the mutating probe specs so baselines always see pristine seed data. | M | ✅ Shipped |
| IMP-CODE-08 | Code | `utils/apiError.cts` + 3 routes | **Shared service-error adapter** (from /code-review): `toMatchError`/`toTransactionServiceError`/`toUserOpError` are byte-identical 4xx-passthrough copies (dataSources' `toSourceError` is a legit sanitizing variant). Hoist one `toServiceApiError(err, fallbackCode)` into `utils/apiError.cts`; the masking boundary should have exactly one implementation + test. | S | 🔵 Noted |
| IMP-CODE-09 | Code | year/month validation ×~8 files | **Shared year/month validator** (from /code-review): the 20002100 bounds + messages are duplicated across summary/calendar/payments/bills/spending (spending drifted to a different message). One `parseYearMonth` in `utils/dates.mts` throwing `ValidationError`, per CODE_STANDARDS "Validation (target)". | S | 🔵 Noted |
| IMP-CODE-10 | Code | `utils/apiError.cts` `formatError` | **Details channel** (from /code-review): `ApiError` already stores `details` but `formatError` never forwards extras, forcing the two documented hand-rolled bodies (payments `DUPLICATE_SUSPECTED` w/ `existing`, transactions bulk-500 w/ `results`). Whitelisting `details` onto the envelope makes those throwable and closes the "one canonical error path" exceptions. | M | 🔵 Noted |
| IMP-CODE-11 | Code | `eslint.config.mjs` error-shape ratchet | **Deepen the ratchet** (from /code-review): it bans the `errorFormatter` require — a proxy — not hand-rolled `res.status().json({error…})` bodies, and its exceptions are whole-file ignores. After IMP-CODE-10 removes the legit exceptions, add a `no-restricted-syntax` selector on error-body `res.json` shapes in `routes/**`. | S | 🔵 Noted |
| IMP-UX-03 | UX | login/profile client | **Small client consolidations** (from /code-review): theme-init.js duplicates ThemeContext's storage key + theme-color hexes (share constants or build-inject); PasskeySection makes two GETs where one endpoint returning `{enabled, credentials}` suffices; the TOTP and passkey second-factor cards in LoginPage could share a `SecondFactorCard`. | M | 🔵 Noted |
--- ---