From bde4c3a2207a0e5579d0cf8e729ec60b39f43cee Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 04:52:24 +0000 Subject: [PATCH 1/2] fix: remediate 2026-07-19 audit findings (inert settings, offline auth, redirect query) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Mark every settings preference no consumer reads yet with an explicit 'Saved for later — not active yet' badge (jurisdiction, population, answer style, landing, home-content toggles, compact citations, and notification toggles); appearance/density/motion stay live and unmarked. A new dom test locks exactly which rows carry the marker. - Treat a retryable getUser() fetch failure as verification-unavailable in the initial auth resolution: an unreachable auth server no longer drops a valid stored session to signed_out on load, while a reachable server that rejects the token still does. Server-side bearer validation is unchanged. - Preserve the sanitized q/focus/run search context through the /medications legacy redirect, matching the root legacy-mode redirect allowlist. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LxzDsyr6FXLG1zcuLGyFsf --- docs/branch-review-ledger.md | 1 + src/app/medications/route.ts | 14 +++- .../clinical-dashboard/settings-dialog.tsx | 30 ++++++++ src/lib/supabase/client.tsx | 23 +++++- .../audit-navigation-auth-regressions.test.ts | 10 ++- tests/private-client-auth.test.ts | 29 +++++++ tests/settings-inert-preferences.dom.test.tsx | 76 +++++++++++++++++++ 7 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 tests/settings-inert-preferences.dom.test.tsx diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 6c754a684..ab2ea50a7 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -621,3 +621,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-18 | claude/clinical-kb-pwa-review-asi3wb (PR #896, plan Phase 5; content commit + this ledger follow-up) | a6c2b4e92374e9002fb00c547eb5677d01ce538c | Design-polish sweep: audit-then-fix (plan Phase 5, final phase) | Audit on post-#890 main: three strict design guards clean; full re-run of the 07-token-adoption-audit grep method shows all July 3 debt resolved (M1–M3 done, L4 reduced to the deliberate theme-aware `ring-white/N dark:ring-white/10` glass idiom, L5/L7 gone; production hex all legitimate print/brand/console/comment classes); 43-capture live sweep across 15 routes × desktop/phone + 320px spots + dark/reduced-motion/forced-colors spots found 0 overflow and 0 console errors. Three defects found and fixed: (1) forced-colors solid-button labels rendered as blank Canvas-on-Canvas backplate boxes (axe-invisible) — command controls flattened to the native HCM ButtonFace/ButtonText pairing and accent glyph tokens flipped to ButtonText inside the existing forced-colors block, regression-locked by a new ui-accessibility test; (2) tools desktop 6-up quick-action rail truncated card titles at 1440×1000 — card metrics tightened, all six titles verified unclipped; (3) privacy page rendered "systemand" from a JSX newline-adjacent-to-tag drop — explicit space, locked by a privacy-ui assertion. Dated July 18 run appended to docs/redesign/07-token-adoption-audit.md (archived design-qa.md not resurrected). | Guards + focused vitest 14/14; `verify:cheap` chain green to the known container-only pdf-extraction-budget artifact (2806/2809); `verify:ui` 220 passed/2 failed (the two long-baselined container artifacts, hosted-CI-green through #826/#835/#872/#890); `test:e2e:accessibility` 8/8 incl. the new forced-colors token test; production build + client-bundle secret scan passed; `check:bundle-budget` within tolerance vs the Phase 4 ratchet (1290.6 vs 1278.6 KiB baseline); `verify:pr-local` runtime/format/lint/typecheck/build/rag-fixtures green with the same sole unit-suite artifact. `verify:release` not run (provider-backed; awaits explicit confirmation). No provider-backed checks run. | | 2026-07-19 | all remote feature branches and registered worktrees against `origin/main` through PR #899 | 8242fa63d5f5b79fc770c9ae4f633e3a784b80e1 | branch/worktree cleanup, useful-work recovery, and protected-main merge closure | Deleted 122 stale or closed remote feature refs with exact SHA leases; four additional merged PR branches were removed by the protected-main PR workflow. Removed 32 obsolete, superseded, or merge-proven worktree registrations. Recovered useful dirty RAG work into PR #901 (deterministic and opt-in semantic reranking) and PR #902 (retrieval phase latency telemetry), preserved follow-up decisions in `docs/process-hardening.md`, and recovered four missing historical review rows. PRs #897, #899, #901, and #902 are merged with green exact-head checks and zero unresolved review threads. A detached full-repo-review worktree is deliberately retained because its ownership/activity could not be safely disproved; one unregistered `node_modules` junction residue is also retained because deletion was denied by local safety policy. | Fresh fetch/prune; full GitHub PR/check/thread inventory; `git worktree list --porcelain`; cherry-pick-aware right-only logs; exact leased remote deletes; exact-old-value local ref deletes; clean-worktree, path, and merged-PR proof before every removal. PR #899 local proof: focused Vitest 31/31, changed-file ESLint, `verify:cheap` 317 files / 2,879 tests, and `verify:ui` 239/239; exact-head hosted checks all passed. PR #901 local proof: `verify:cheap` 316 files / 2,870 tests; PR #902 focused Vitest 8/8 plus ESLint and typecheck. No OpenAI, Supabase, live clinical, deployment, or production-data workflow ran; provider-backed semantic canary evaluation remains approval-gated. | | 2026-07-19 | origin/main 24-hour merged window (d1937d78e..ef042cacd, ~30 PRs incl. #853/#859/#861/#865/#868/#871–#874/#879/#885–#888/#890–#894/#896) | ef042cacd | integrated post-merge regression audit of the full 24h window (code review + design/performance/defect angles) | No P0/P1 regression found at the merged tip. One confirmed P2: the new settings surface (`use-app-preferences.ts` + `settings-dialog.tsx`) presents jurisdiction, population, answer-style, landing, home-content, compact-citations, and all notification preferences as live controls, but no consumer reads them — only density/motion (html attributes + globals.css) and theme are functional; a clinician selecting "Conservative" answer style reasonably but wrongly believes generation changed. One plausible P2/P3: `AuthProvider.initializeSession` now requires a live `getUser()` round-trip, so a transient network failure on load resolves a valid stored session to signed_out (INITIAL_SESSION replay is also skipped); the deliberate stale/tampered-token defense does not distinguish retryable network errors. One P3: `/medications` legacy redirect drops the query string while `/applications` and `/differentials/presentations` preserve theirs. Cleared after inspection: worker image-placement dedupe (key symmetric on both sides), title-word purge/scope migration (matches its replayed review), RPC-layer-only cancellation consolidation (intentional per PR #861), services route-chunk fix intact with no other heavy client value-imports (type-only imports verified), forced-colors ButtonFace/ButtonText flip, codex-autofix/pr-policy workflow changes conform to AGENTS.md (pin retained, rename-aware routing, workflow_sha checkout), audit-metadata allowlist exhaustive over the closed AuditAction union, answer-stream merged abort signals. Environment note (not a repo defect): the session-start hook skips `npm install` when node_modules exists, so this window's dependency bumps left the container stale and `verify:cheap` failed at typecheck until `npm ci`; the hook should compare a lockfile hash. | `npm ci` then typecheck clean and full Vitest 2828 passed / 1 failed / 2 skipped — the sole failure is the long-baselined container-only `tests/pdf-extraction-budget.test.ts` artifact (hosted-CI-green through #826/#835/#872/#890). First `verify:cheap` run passed every static guard (runtime, actions-pin, ci-scope, ci-triage, pr-policy, sitemap, brand, type-scale, icon-scale, function-grants, owner-scope, lint) before the stale-deps typecheck stop. No OpenAI, Supabase, deployment, or provider-backed check ran. | +| 2026-07-19 | claude/audit-recent-changes-kde66i (audit-remediation follow-up to the ef042cacd audit row) | see PR head | remediation of the three 2026-07-19 audit findings | Fixed all three findings from the 24-hour merged-window audit. (1) Inert settings honesty (P2): every preference the app does not yet consume — jurisdiction, population, answer style, landing, both home-content toggles, compact citations, and all three notification toggles — now renders an explicit "Saved for later — not active yet" marker in `settings-dialog.tsx`; the functional appearance/density/motion rows stay unmarked. Wiring the preferences into answer generation was deliberately NOT done here (clinical-behavior change requiring governance review); the new dom test documents the contract for flipping a control live. (2) Auth offline resolution (P2/P3): `resolveInitialAuthState` gains `verificationUnavailable`, set from `isAuthRetryableFetchError(getUser().error)`, so an unreachable auth server keeps the stored session signed in while a reachable server that rejects the token still resolves signed_out; server-side bearer validation is unchanged. (3) `/medications` redirect (P3): now preserves the sanitized q/focus/run search context with the same allowlist as the root legacy-mode redirect. | Focused Vitest 30/30 across `settings-inert-preferences.dom` (new), `private-client-auth` (3 new cases), `audit-navigation-auth-regressions` (query-preservation case), `app-preferences`, and `site-map`. Full `verify:cheap` run recorded on the PR. No OpenAI, Supabase, deployment, or provider-backed check ran. | diff --git a/src/app/medications/route.ts b/src/app/medications/route.ts index 5ebeac9a8..e9284566a 100644 --- a/src/app/medications/route.ts +++ b/src/app/medications/route.ts @@ -1,7 +1,17 @@ -export function GET() { +import { type NextRequest } from "next/server"; + +// Legacy prescribing home. Preserve the search context the same way the root +// legacy-mode redirect does (q plus the focus/run flags, sanitized) so a +// bookmarked /medications?q=lithium link keeps its query after the redirect. +export function GET(request: NextRequest) { + const params = new URLSearchParams({ mode: "prescribing" }); + const query = request.nextUrl.searchParams.get("q")?.trim(); + if (query) params.set("q", query); + if (request.nextUrl.searchParams.get("focus") === "1") params.set("focus", "1"); + if (request.nextUrl.searchParams.get("run") === "1") params.set("run", "1"); return new Response(null, { status: 307, - headers: { Location: "/?mode=prescribing" }, + headers: { Location: `/?${params.toString()}` }, }); } diff --git a/src/components/clinical-dashboard/settings-dialog.tsx b/src/components/clinical-dashboard/settings-dialog.tsx index 720633e96..cca4f9fda 100644 --- a/src/components/clinical-dashboard/settings-dialog.tsx +++ b/src/components/clinical-dashboard/settings-dialog.tsx @@ -517,6 +517,7 @@ export function SettingsDialog({ icon={Globe2} label="Jurisdiction" description="Prioritises guidance relevant to your region." + notYetActive htmlFor="settings-jurisdiction" > option.value === preferences.answerStyle)?.description } + notYetActive labelId="settings-answer-style-label" stacked > @@ -593,6 +596,7 @@ export function SettingsDialog({ icon={PanelTop} label="Default landing view" description="The mode shown when you open the app." + notYetActive labelId="settings-landing-label" stacked > @@ -618,6 +622,7 @@ export function SettingsDialog({ + Saved for later — not active yet + + ); +} + function SettingsField({ icon, label, @@ -788,6 +812,7 @@ function SettingsField({ htmlFor, labelId, stacked = false, + notYetActive = false, children, }: { icon: LucideIcon; @@ -797,6 +822,7 @@ function SettingsField({ htmlFor?: string; labelId?: string; stacked?: boolean; + notYetActive?: boolean; children?: ReactNode; }) { const LabelTag = htmlFor ? "label" : "span"; @@ -821,6 +847,7 @@ function SettingsField({ {description ? (

{description}

) : null} + {notYetActive ? : null} {children ? ( @@ -919,12 +946,14 @@ function SettingsToggleField({ description, checked, onChange, + notYetActive = false, }: { icon: LucideIcon; label: string; description?: string; checked: boolean; onChange: (checked: boolean) => void; + notYetActive?: boolean; }) { return (
{description}

) : null} + {notYetActive ? : null}
diff --git a/src/lib/supabase/client.tsx b/src/lib/supabase/client.tsx index af44bd416..016bed11f 100644 --- a/src/lib/supabase/client.tsx +++ b/src/lib/supabase/client.tsx @@ -1,7 +1,7 @@ "use client"; import { createBrowserClient } from "@supabase/ssr"; -import { type Session, type SupabaseClient } from "@supabase/supabase-js"; +import { isAuthRetryableFetchError, type Session, type SupabaseClient } from "@supabase/supabase-js"; import { createContext, type ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react"; import { clearPersistedAnswerThread } from "@/lib/answer-thread-storage"; import { clearRecentQueries } from "@/components/clinical-dashboard/recent-query-storage"; @@ -90,6 +90,12 @@ export function authorizationHeadersForAccessToken(accessToken: string | null | * local token resolves to signed-out instead of presenting as authenticated. Data * access is already safe — every API route re-validates the bearer token server-side * ([auth.ts](src/lib/supabase/auth.ts)) — so this is defense-in-depth for the client UI. + * + * `verificationUnavailable` covers the case where `getUser()` could not reach the + * auth server at all (offline load, flaky network). That is not evidence the token + * is bad, so the stored session keeps the signed-in UI instead of silently + * presenting as signed out; the server still rejects the token on every data call + * if it truly is invalid. */ export type InitialAuthResolution = { status: "authenticated"; session: Session } | { status: "signed_out"; session: null }; @@ -97,11 +103,15 @@ export type InitialAuthResolution = export function resolveInitialAuthState(args: { verifiedUserId: string | null; session: Session | null; + verificationUnavailable?: boolean; }): InitialAuthResolution { - const { verifiedUserId, session } = args; + const { verifiedUserId, session, verificationUnavailable } = args; if (verifiedUserId && session && session.user.id === verifiedUserId) { return { status: "authenticated", session }; } + if (verificationUnavailable && session) { + return { status: "authenticated", session }; + } return { status: "signed_out", session: null }; } @@ -157,7 +167,14 @@ export function AuthProvider({ children }: { children: ReactNode }) { const [userResult, sessionResult] = await Promise.all([client.auth.getUser(), client.auth.getSession()]); if (!active) return; const verifiedUserId = userResult.error ? null : (userResult.data.user?.id ?? null); - const resolved = resolveInitialAuthState({ verifiedUserId, session: sessionResult.data.session }); + // A retryable fetch error means the auth server was unreachable, not that + // the token was rejected — don't drop a valid stored session for that. + const verificationUnavailable = isAuthRetryableFetchError(userResult.error); + const resolved = resolveInitialAuthState({ + verifiedUserId, + session: sessionResult.data.session, + verificationUnavailable, + }); setSession(resolved.session); setStatus(resolved.status); if (resolved.status === "authenticated") { diff --git a/tests/audit-navigation-auth-regressions.test.ts b/tests/audit-navigation-auth-regressions.test.ts index d2aa7097e..db20a8188 100644 --- a/tests/audit-navigation-auth-regressions.test.ts +++ b/tests/audit-navigation-auth-regressions.test.ts @@ -43,10 +43,18 @@ describe("audit navigation and auth regressions", () => { "/differentials/presentations/acute-confusion-encephalopathy?q=acute+confusion&ids=delirium", ); - const medications = redirectMedications(); + const medications = redirectMedications(new NextRequest("https://clinical-kb.test/medications")); expect(medications.status).toBe(307); expect(medications.headers.get("location")).toBe("/?mode=prescribing"); + // Search context survives the legacy redirect with the same sanitized + // allowlist as the root legacy-mode redirect: trimmed q plus focus/run=1. + const medicationsWithQuery = redirectMedications( + new NextRequest("https://clinical-kb.test/medications?q=+lithium+&focus=1&run=0&mode=ignored&extra=drop"), + ); + expect(medicationsWithQuery.status).toBe(307); + expect(medicationsWithQuery.headers.get("location")).toBe("/?mode=prescribing&q=lithium&focus=1"); + expect([headApplications, headPresentations, headMedications]).toEqual([ redirectApplications, redirectPresentations, diff --git a/tests/private-client-auth.test.ts b/tests/private-client-auth.test.ts index bb960bfdf..0987d8959 100644 --- a/tests/private-client-auth.test.ts +++ b/tests/private-client-auth.test.ts @@ -77,4 +77,33 @@ describe("resolveInitialAuthState — getUser-verified initial auth (audit D3)", session: null, }); }); + + it("keeps the stored session when verification was unavailable (offline/flaky load), not rejected", () => { + // getUser() failed with a retryable fetch error: the auth server was + // unreachable, which is not evidence the token is bad. Dropping to + // signed_out here silently hides a valid signed-in state on a flaky + // connection (audit 2026-07-19). Server-side routes still reject the token + // on every data call if it truly is invalid. + const session = fakeSession("user-1"); + expect(resolveInitialAuthState({ verifiedUserId: null, session, verificationUnavailable: true })).toEqual({ + status: "authenticated", + session, + }); + }); + + it("stays signed out on unavailable verification when no session is stored", () => { + expect(resolveInitialAuthState({ verifiedUserId: null, session: null, verificationUnavailable: true })).toEqual({ + status: "signed_out", + session: null, + }); + }); + + it("still rejects a stored session when the auth server answered and refused the token", () => { + // verificationUnavailable=false is the reachable-server case: an error from + // getUser() means the token was actually rejected, so the stale/tampered + // defense keeps resolving to signed_out. + expect( + resolveInitialAuthState({ verifiedUserId: null, session: fakeSession("user-1"), verificationUnavailable: false }), + ).toEqual({ status: "signed_out", session: null }); + }); }); diff --git a/tests/settings-inert-preferences.dom.test.tsx b/tests/settings-inert-preferences.dom.test.tsx new file mode 100644 index 000000000..db57e42d5 --- /dev/null +++ b/tests/settings-inert-preferences.dom.test.tsx @@ -0,0 +1,76 @@ +import { render, screen, within } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +// The settings surface persists several preferences that nothing in the app +// consumes yet (audit 2026-07-19 P2: inert controls presented as live). Until a +// preference is actually read somewhere, its row must carry the explicit +// "Saved for later — not active yet" marker so a clinician never believes +// answer behavior changed. The functional rows (appearance/density/motion, +// which drive the html theme/data-density/data-motion hooks) must NOT carry it. +// When wiring a preference up, remove it from INERT_ROWS here and drop the +// notYetActive flag from its row in settings-dialog.tsx. + +vi.mock("@/lib/supabase/client", () => ({ + useAuthSession: () => ({ + status: "signed_out", + isConfigured: false, + error: null, + notice: null, + session: null, + signInWithEmail: vi.fn(), + signInWithOAuth: vi.fn(), + signOut: vi.fn(), + }), +})); + +import { SettingsDialog } from "@/components/clinical-dashboard/settings-dialog"; + +const NOT_YET_ACTIVE_TEXT = "Saved for later — not active yet"; + +const INERT_ROWS = [ + "settings-row-jurisdiction", + "settings-row-default-population", + "settings-row-answer-style", + "settings-row-default-landing-view", + "settings-row-recent-searches-on-home", + "settings-row-saved-protocols-on-home", + "settings-row-compact-citations", + "settings-row-guideline-updates", + "settings-row-product-news", + "settings-row-saved-item-changes", +]; + +const FUNCTIONAL_ROWS = ["settings-row-appearance", "settings-row-interface-density", "settings-row-reduce-motion"]; + +function renderDialog() { + return render( + {}} + identity={{ displayName: "Local session", initials: "LS", detail: "Browser only", signedIn: true }} + onSignOut={() => {}} + onOpenGuide={() => {}} + />, + ); +} + +describe("settings dialog inert-preference honesty markers", () => { + it("marks every preference no consumer reads as not yet active", () => { + renderDialog(); + for (const testId of INERT_ROWS) { + const row = screen.getByTestId(testId); + expect(within(row).getByText(NOT_YET_ACTIVE_TEXT)).toBeInTheDocument(); + } + // Exactly the inert rows carry the marker — a count drift means a row was + // added or wired up without updating this contract. + expect(screen.getAllByText(NOT_YET_ACTIVE_TEXT)).toHaveLength(INERT_ROWS.length); + }); + + it("keeps the functional appearance, density, and motion rows unmarked", () => { + renderDialog(); + for (const testId of FUNCTIONAL_ROWS) { + const row = screen.getByTestId(testId); + expect(within(row).queryByText(NOT_YET_ACTIVE_TEXT)).not.toBeInTheDocument(); + } + }); +}); From 694a597cc3bdee817f69b0aed997c3322af3543a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 05:19:30 +0000 Subject: [PATCH 2/2] fix(a11y): announce inert-setting status via aria-describedby MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'Saved for later — not active yet' marker was visual-only; screen-reader users focusing an inert select, radiogroup, or switch heard just the label. Each marked control now references its badge through aria-describedby, and the dom test locks the announcement contract for all ten inert rows. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LxzDsyr6FXLG1zcuLGyFsf --- .../clinical-dashboard/settings-dialog.tsx | 41 ++++++++++++++++--- tests/settings-inert-preferences.dom.test.tsx | 26 ++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/components/clinical-dashboard/settings-dialog.tsx b/src/components/clinical-dashboard/settings-dialog.tsx index cca4f9fda..744211e87 100644 --- a/src/components/clinical-dashboard/settings-dialog.tsx +++ b/src/components/clinical-dashboard/settings-dialog.tsx @@ -522,6 +522,7 @@ export function SettingsDialog({ > setPreference("jurisdiction", value)} options={JURISDICTION_OPTIONS} @@ -536,6 +537,7 @@ export function SettingsDialog({ > setPreference("population", value)} options={POPULATION_OPTIONS} @@ -553,6 +555,7 @@ export function SettingsDialog({ > setPreference("answerStyle", value)} options={ANSWER_STYLE_OPTIONS} @@ -602,6 +605,7 @@ export function SettingsDialog({ > setPreference("landing", value)} options={LANDING_OPTIONS} @@ -794,11 +798,20 @@ function IconBadge({ icon: Icon }: { icon: LucideIcon }) { * Honesty marker for preference controls that persist a choice but are not yet * consumed anywhere in the app (audit 2026-07-19 P2: inert settings presented as * live). Remove the marker from a control only when something actually reads its - * preference and changes behavior. + * preference and changes behavior. The badge carries an id so the control it + * describes can reference it via `aria-describedby` — the marker must be + * announced to assistive tech, not just rendered visually. */ -function NotYetActiveBadge() { +function notYetActiveId(anchor: string) { + return `${anchor}-not-yet-active`; +} + +function NotYetActiveBadge({ id }: { id?: string }) { return ( - + Saved for later — not active yet ); @@ -847,7 +860,9 @@ function SettingsField({ {description ? (

{description}

) : null} - {notYetActive ? : null} + {notYetActive ? ( + + ) : null} {children ? ( @@ -866,16 +881,19 @@ function SegmentedControl({ onChange, options, ariaLabelledBy, + ariaDescribedBy, }: { value: T; onChange: (value: T) => void; options: ReadonlyArray<{ value: T; label: string; icon?: LucideIcon }>; ariaLabelledBy?: string; + ariaDescribedBy?: string; }) { return (
({ value, onChange, options, + describedBy, }: { id: string; value: T; onChange: (value: T) => void; options: ReadonlyArray<{ value: T; label: string }>; + describedBy?: string; }) { return (