diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a955cb507..ee23dd4d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,11 @@ jobs: - name: Function-grant guard run: npm run check:function-grants + # Fails if a src/app/api handler queries an owner-scoped table without a + # recognised owner filter (defense-in-depth tenancy guard; audit D2). + - name: Owner-scope guard + run: npm run check:owner-scope + - name: Lint run: npm run lint diff --git a/.github/workflows/pr-policy.yml b/.github/workflows/pr-policy.yml index 1c2532d4f..5a0d59652 100644 --- a/.github/workflows/pr-policy.yml +++ b/.github/workflows/pr-policy.yml @@ -24,13 +24,15 @@ jobs: if: github.event_name == 'merge_group' run: echo "PR metadata was validated before merge-queue entry." - # pull_request_target runs trusted base-branch code. Pin the checkout to - # the PR's exact base SHA and never execute the PR head or persist credentials. + # pull_request_target runs trusted base-branch code. Checkout the current + # tip of the base branch (not a potentially stale base.sha) so the policy + # script is always available even when the PR was opened before the script + # was added to main. Never execute the PR head or persist credentials. - name: Checkout trusted policy if: github.event_name == 'pull_request_target' uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - ref: ${{ github.event.pull_request.base.sha }} + ref: ${{ github.base_ref }} persist-credentials: false - name: Validate pull request evidence diff --git a/.npmrc b/.npmrc index b6f27f135..4ade96e63 100644 --- a/.npmrc +++ b/.npmrc @@ -1 +1,2 @@ engine-strict=true +allow-scripts=true diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 3b1cd90d6..4a82c5bc3 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -20,6 +20,7 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks | | ---------- | -------------------------------------------------------- | ---------------------------------------- | ---------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 2026-07-17 | PR #635 / claude/github-actions-codex-issue-f4t4s5 | ab09a8d52cc0a8a7e71b37885aaa358aae2522c8 | post-merge merge-readiness review | Already squash-merged to main on 2026-07-14 by BigSimmo. No open review threads or inline comments. CI required checks all green (Change scope, Static PR checks, Safety and config checks, Unit coverage, PR required, Semgrep, Gitleaks, GitGuardian); UI/build/migration jobs correctly skipped. Landed diff is test/guard hardening only for missing `CODEX_TRIGGER_TOKEN` graceful skip. No high-confidence P0-P2 defect. Source branch already deleted. No further merge action needed. | Hosted CI status via `gh pr checks 635` (all required pass); local `node scripts/check-codex-autofix-workflow.mjs` pass; focused Vitest `tests/codex-autofix-workflow.test.ts` 41/41. No OpenAI/Supabase/provider writes. | | 2026-07-17 | PR #718 / codex/performance-latency-remediation-20260717 | b5f509744d4f4bac74d414644cd1802f64b97fa9 | CodeRabbit performance and SQL correctness follow-up | Resolved nine confirmed findings and dispositioned one stale test comment: document downloads revalidate signed URLs on every action; committed-generation filtering precedes detail pagination; enrichment fallback errors preserve identity; caller cancellation leaves the shared classifier flight alive; registry seeding preserves its cache signal; aliases emit canonical corrections; rate-limit success metadata is coherent; ambiguous upserts use named constraints; and grantable default ACLs fail closed. The proxy mock duplicate was not present. No remaining high-confidence P0-P2 defect was found. | Integrated focused Vitest 122/122; post-format Vitest 71/71; `npm run verify:cheap` passed runtime/policy/static guards, ESLint, TypeScript, and 2,684/2,684 tests; focused Prettier and `git diff --check`; disposable Docker replay, regenerated drift manifest, and transactional local SQL probes. No OpenAI calls, live Supabase DDL/migration/data write, deployment, or production mutation ran. | | 2026-07-17 | PR #718 / codex/performance-latency-remediation-20260717 | d47ef7a329256687a615c33dd311806c2c1214a8 | hosted UI and migration-order merge-blocker follow-up | Fixed both integration defects exposed by the exact-head UI run: the document scope surface now triggers the deferred, deduplicated catalogue load, and viewer navigation closes unrelated disclosures before opening or scrolling to the selected section. Updated SSR-aware browser fixtures without restoring the removed detail request. Renumbered all three new migrations after the latest production migration and regenerated the drift manifest. No remaining high-confidence P0-P2 defect was found in the follow-up diff. | Focused Vitest 65/65; scoped ESLint and Prettier; `git diff --check`; isolated production Webpack build and TypeScript; focused Chromium 7/7 including both stress viewports; Docker schema replay and drift-manifest regeneration passed in 15 seconds with unchanged schema SHA. No OpenAI calls or live Supabase DDL, migration, rate-limit RPC, or data write ran. | | 2026-07-13 | codex/lithium-answer-recovery-pr | c5fde11e64d8976e1c163d1b8618f58a52e0b8ff | lithium answer recovery and source governance | Fixed the provider-failure path with a grounded Australian source-backed fallback, a public-safe progress lifecycle, centralised Australian authority/context selection, and fail-closed locality repair. The live audit exposed and the branch fixed hierarchy identity false conflicts plus registry projections entering clinical metadata gates. The corrected audit/backfill found zero proposals, so no production write was made. No high-confidence defect remains. Residual risk is provider latency; live generation timed out but the grounded fallback completed. | `npm run verify:pr-local` passed: 1,988 tests passed/1 skipped, build/client scan, 36 fixtures, and 267 offline RAG tests. After review fixes, `npm run verify:cheap` passed with 1,992 tests passed/1 skipped. `npm run check:production-readiness` passed 5/5. `npm run test:e2e:critical` passed 9/9. `node scripts/run-playwright.mjs tests/answer-progress-ui-smoke.spec.ts --project=chromium` passed 2/2. `node scripts/run-eval-safe.mjs scripts/eval-rag.ts --question "Lithium dosing" --expect-australian --fail-on-threshold --json` exited 0 with one grounded FSH citation and no threshold/safety failures. `npm run audit:source-governance` reported 0 gaps/conflicts/proposals across 2,851 rows. `npm run backfill:source-metadata -- --locality-only` reported 0 changes. `git diff --check` passed. Full `npm run verify:ui` was not rerun after the aggregate runner lost its local server. | diff --git a/docs/codebase-index.md b/docs/codebase-index.md index a09628bd6..2cfd095e0 100644 --- a/docs/codebase-index.md +++ b/docs/codebase-index.md @@ -68,17 +68,19 @@ Structured map for AI agents and onboarding. For live routes, see `docs/site-map ### API routes (`src/app/api/`) -| Area | Routes | Entry files | -| ----------- | ----------------------------------------------------------------------- | --------------------------------------------------------------- | -| Answers | `/api/answer`, `/api/answer/stream`, `/api/answer-feedback` | `answer/route.ts`, `answer/stream/route.ts`, `answer-feedback/` | -| Search | `/api/search`, `/api/search/interaction` | `search/` | -| Upload | `/api/upload` | `upload/route.ts` | -| Documents | CRUD, bulk, reindex, labels, search, summarize, table-facts, signed-url | `documents/` | -| Ingestion | batches, jobs, retry, quality | `ingestion/` | -| Registry | records CRUD | `registry/records/` | -| Images | signed URLs | `images/[id]/signed-url/route.ts` | -| Ops | health, setup-status, local-project-id | `health/`, `setup-status/`, `local-project-id/` | -| Eval / jobs | eval cases, job state | `eval-cases/`, `jobs/` | +| Area | Routes | Entry files | +| ------------- | ------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | +| Answers | `/api/answer`, `/api/answer/stream`, `/api/answer-feedback` | `answer/route.ts`, `answer/stream/route.ts`, `answer-feedback/` | +| Search | `/api/search`, `/api/search/interaction`, `/api/search/universal` | `search/` | +| Upload | `/api/upload` | `upload/route.ts` | +| Documents | `/api/documents`, `/api/documents/[id]`, bulk/reindex, labels, reviews, search, signed URLs, summaries, table facts | `documents/` | +| Differentials | `/api/differentials`, `/api/differentials/[slug]`, `/api/differentials/presentations/[slug]` | `differentials/` | +| Medications | `/api/medications`, `/api/medications/[slug]` | `medications/` | +| Ingestion | `/api/ingestion/batches`, `/api/ingestion/jobs`, retry, quality | `ingestion/` | +| Registry | `/api/registry/records`, `/api/registry/records/[slug]` | `registry/records/` | +| Images | `/api/images/[id]/signed-url` | `images/[id]/signed-url/route.ts` | +| Ops | `/api/health`, `/api/health/ready`, `/api/setup-status`, `/api/local-project-id` | `health/`, `setup-status/`, `local-project-id/` | +| Eval / jobs | `/api/eval-cases`, `/api/jobs` | `eval-cases/`, `jobs/` | --- @@ -151,12 +153,12 @@ Structured map for AI agents and onboarding. For live routes, see `docs/site-map - **CLI:** `supabase/config.toml` — `indexing-v3-agent` function, `verify_jwt = false` - **Schema mirror:** `supabase/schema.sql` (reference; migrations are source of truth) -- **Migrations:** `supabase/migrations/*.sql` (~90 files, May–Jul 2026) +- **Migrations:** `supabase/migrations/*.sql` (chronological source of truth; do not hardcode a count) - **Drift policy:** `docs/supabase-migration-reconciliation.md` -### Core tables +### Schema tables -`documents`, `document_pages`, `document_images`, `document_chunks`, `document_embedding_fields`, `document_index_units`, `document_table_facts`, `document_labels`, `document_summaries`, `document_sections`, `document_memory_cards`, `document_index_quality`, `ingestion_jobs`, `ingestion_job_stages`, `indexing_v3_agent_jobs`, `import_batches`, `rag_queries`, `rag_query_misses`, `rag_aliases`, `rag_response_cache`, `rag_retrieval_logs`, `clinical_registry_records`, `api_rate_limits`, `audit_logs`, `storage_cleanup_jobs` +`documents`, `document_pages`, `document_images`, `document_chunks`, `document_embedding_fields`, `document_index_units`, `document_table_facts`, `document_labels`, `document_summaries`, `document_sections`, `document_memory_cards`, `document_index_quality`, `document_title_words`, `ingestion_jobs`, `ingestion_job_stages`, `indexing_v3_agent_jobs`, `import_batches`, `image_caption_cache`, `rag_queries`, `rag_query_misses`, `rag_aliases`, `rag_response_cache`, `rag_retrieval_logs`, `rag_visual_eval_cases`, `rag_visual_eval_runs`, `rag_answer_feedback`, `clinical_registry_records`, `clinical_registry_record_sources`, `medication_records`, `differential_records`, `source_review_events`, `api_rate_limits`, `api_rate_limit_subjects`, `audit_logs`, `storage_cleanup_jobs` **Storage buckets:** `clinical-documents`, `clinical-images` (private) diff --git a/docs/rag-scalability-wip-review-handover-2026-07-15.md b/docs/rag-scalability-wip-review-handover-2026-07-15.md new file mode 100644 index 000000000..ec2d3e6bb --- /dev/null +++ b/docs/rag-scalability-wip-review-handover-2026-07-15.md @@ -0,0 +1,296 @@ +# Handover — RAG scalability WIP review findings (2026-07-15) + +**Status:** findings + remediation plan recorded; **fixes not yet applied**. +**Review date:** 2026-07-15 +**Git state at review:** detached HEAD `570e6ba56ae60bea56a32801b9cc96c5a8dfde4f` (`feat(rag): ship D4/D5 governance levers…` / aligned with then-current main tip) plus uncommitted WIP listed below. +**Ledger row:** `docs/branch-review-ledger.md` — scope `thorough multi-lens review: WIP RAG/schema + clinical design/UI + architecture/bug-hunt`. +**Product context:** Clinical KB Next.js + Supabase; target project `Clinical KB Database` / `sjrfecxgysukkwxsowpy`. Provider-backed apply/eval requires explicit confirmation. + +This file is the single handoff artifact for the next agent or engineer. It consolidates the multi-lens review (design-review, architecture, bug-hunter, code-review, clinical UI) and the remediation plan. Do **not** ship the WIP as-is. + +--- + +## 1. Snapshot of the working tree (at review) + +### Modified + +| Path | Role in WIP | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------- | +| `src/lib/rag-candidate-sources.ts` | New `ChunkLoadCache` + shared hydration caching | +| `src/lib/rag.ts` | Wires one `chunkLoadCache` into table-facts / embedding-field / index-unit paths | +| `src/lib/registry-corpus-links.ts` | Tightened `registryCorpusDetailHref` arg types | +| `supabase/schema.sql` | Appended registry cleanup, `document_title_words`, corrector vocab source change, wide table-facts trgm index | +| `tests/supabase-schema.test.ts` | Weak “identical” substring checks for new objects | +| `docs/branch-review-ledger.md` | Review record appended (allowed during pure review) | + +### Untracked (WIP) + +| Path | Notes | +| ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | +| `supabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sql` | Registry cascade delete + title-words table + corrector rewrite (incomplete scalability) | +| `supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` | Wide GIN trgm index — **mismatched** to live `trgm_matches` predicate | +| `pnpm-lock.yaml` | **Accidental** — repo is npm (`packageManager: npm@…`, committed `package-lock.json`). Delete; do not commit. | + +### Environment caveats + +- Many concurrent worktrees existed at review time; park this WIP on a **named feature branch** before committing. +- Stale `.next/types` `/applications` layout errors also appeared during typecheck; treat as separate from WIP registry typing failures (clear/regenerate `.next` if they persist after A1). + +--- + +## 2. Verdict + +| Severity | Count | Bottom line | +| -------- | ------- | ------------------------------------------------------------------------------------------------------------------------ | +| P0 | 0 | No immediate data-loss / clinical-harm P0 confirmed | +| P1 | 5 | Typecheck break; cache error poisoning; registry `::uuid` abort; unused corrector GIN; mismatched table-facts trgm index | +| P2 | several | Scope/race cache footguns; schema/migration/test drift; SECURITY DEFINER revoke gaps; design gradient density | +| P3 | several | Accidental pnpm lock; mockup hex drift; optional pill/glow pass | + +**Highest residual risk if shipped unchanged:** red typecheck + false-scalability migrations + registry delete brittleness + hydration cache coupling under transient DB errors. + +--- + +## 3. Findings (complete catalog) + +### P1 — must fix before merge + +#### F1. `registryCorpusDetailHref` typing breaks callers + +- **Where:** `src/lib/registry-corpus-links.ts` (params narrowed to `string` / `RegistryCorpusKind | string`). +- **Callers failing typecheck:** + - `src/app/api/documents/[id]/signed-url/route.ts` + - `src/components/clinical-dashboard/source-actions.tsx` + - `src/lib/citations.ts` + - `src/lib/universal-search.ts` +- **Trigger:** Pass `unknown` / nullable metadata fields into the helper. +- **Expected:** Runtime `typeof` guards accept loose metadata; types stay compatible. +- **Actual:** `npm run typecheck` fails (`Type 'unknown' is not assignable to type 'string'`). +- **Proof:** `npm run typecheck` (already red at review). +- **Fix:** Restore args to `unknown` (or `string | null | undefined`) and keep runtime guards. Prefer not changing every call site. + +#### F2. `ChunkLoadCache` negative-caches failures across parallel hydrations + +- **Where:** `src/lib/rag-candidate-sources.ts` (`loadChunksForSignalMatches`); wired in `src/lib/rag.ts` via shared `createChunkLoadCache()`. +- **Trigger:** `{ error }` or missing `data` on a batch fetch → empty `Map` → per-id promises resolve `null` and stick for the request. +- **Expected:** Transient failure degrades that path only; siblings can still hydrate overlapping ids. +- **Actual:** Shared cache poisons table-facts + embedding-field + index-unit for overlapping chunk/doc ids. +- **Proof:** Unit test with shared cache: first load mock `{ data: null, error }`; second healthy mock for same ids must still return chunks (today: `[]` from cache). +- **Fix:** Do not cache hard failures (leave keys unset or reject without caching). Optionally fail the batch loudly via existing RPC/hydration telemetry. + +#### F3. Registry cleanup trigger: non-UUID `registry_record_id` aborts deletes + +- **Where:** + - Migration `20260714180000_…` lines ~8–12 + - `supabase/schema.sql` appended `cleanup_registry_corpus_document` +- **SQL hazard:** `(metadata->>'registry_record_id')::uuid = OLD.id` +- **Trigger:** Delete from `clinical_registry_records` / `medication_records` / `differential_records` while any `documents` row has `source_kind = 'registry_record'` and a non-UUID `registry_record_id`. +- **Expected:** Ignore malformed metadata (elsewhere the repo regex-guards UUID casts). +- **Actual:** Cast can throw → **entire registry DELETE transaction fails**. SECURITY DEFINER amplifies blast radius. +- **Proof:** SQL insert dummy doc with `registry_record_id: 'not-a-uuid'`, delete a registry row → expect cast exception today. +- **Fix:** Compare as text: `metadata->>'registry_record_id' = OLD.id::text`. Prefer also match `registry_record_kind` (or map via `TG_TABLE_NAME`). Revoke `EXECUTE` on helpers from `public`/`anon`/`authenticated`. + +#### F4. Corrector “scalability” table/index unused by query path + +- **Where:** Migration `20260714180000_…` — `document_title_words`, GIN `document_title_words_word_trgm_idx`, rewritten `correct_clinical_query_terms`. +- **Trigger / pattern:** Function still `array_agg(distinct term)` over aliases ∪ all title words, then per-token `similarity()` over `unnest(vocab)`. +- **Expected:** Hot path probes the GIN (`%` / similarity lookup) so cost scales with candidates, not full vocab. +- **Actual:** GIN never used; write amplification on every title sync with no planner win. Large corpora → corrector latency/timeouts → weaker lexical/retrieval paths. +- **Proof:** Read function body; no `word % tok` / indexed probe. Schema tests today pass with mere `toContain`. +- **Fix (preferred):** Per-token indexed probe against aliases + `document_title_words` (`LIMIT 1`, keep `min_sim` and length 4–40). Cap candidates. Verify with `EXPLAIN` only under confirmed live access later. +- **Alt:** Drop unused GIN (or table) until rewrite lands — do not ship dead “scalability” scaffolding. + +#### F5. New table-facts trgm index does not match `trgm_matches` + +- **Where:** + - New: `20260714190000_document_table_facts_trgm_idx.sql` / `schema.sql` `document_table_facts_text_trgm_idx` (title + row + param + **threshold_value** + **action**) + - Live predicate: `schema.sql` `trgm_matches` (~6236–6244) uses only title + row + clinical_parameter + - Existing matching index: `document_table_facts_title_row_param_trgm_idx` +- **Expected:** GIN expression equals `%` / `similarity()` expression used by the RPC. +- **Actual:** Wide index is dead weight for current trgm path; dual overlapping GIN indexes raise ingest write cost. FTS already covers wider text via `search_tsv`. +- **Proof:** Diff index expression string vs `trgm_matches` expression (architecture agent correct; one conflicting note claimed alignment — **disproven** by schema lines above). +- **Fix (default):** Remove migration `…190000…` and schema index line; keep narrow index. +- **Alt:** Widen `trgm_matches` to the wide expression **and** drop the narrow index so only one remains. Add schema test for expression equality. + +--- + +### P2 — should fix in same remediation wave + +#### F6. Cache not keyed by access scope (latent authz) + +- **Where:** `ChunkLoadCache` maps keyed only by id; contrast `rag-cache.ts` which uses `retrievalAccessScopeKey`. +- **Current call site:** One scope per `searchChunksWithTelemetry` — production path OK today. +- **Risk:** Reuse across public-only then owner+public → cached `null` deny, or inverse over-share within process. +- **Fix:** Bind scope at cache creation or include scope in keys; refuse mismatched scope. Test in `tests/retrieval-hydration-scope.test.ts`. + +#### F7. Concurrent overlapping miss race + +- **Where:** Parallel embedding-field + index-unit hydration both compute `missing*` before either finishes `cache.*.set`. +- **Risk:** Later writer overwrites in-flight promise; erroring batch can poison a successful first waiters’ result. +- **Fix:** Single in-flight promise per id (check-then-set before creating fetch); never overwrite a healthy promise with a failing one. + +#### F8. Schema / migration / test drift + +| Concern | Migration `…180000…` | `schema.sql` | +| ------------------------------------------- | -------------------- | -------------------------------------------------- | +| `DROP TRIGGER IF EXISTS` before create | yes | **no** | +| Backfill `INSERT INTO document_title_words` | yes | **no** | +| Corrector body rewrite | yes | vocab source already updated; lifecycle incomplete | +| `…190000…` index | separate file | present in schema | +| New test “identically” | weak `toContain` | misses drops, backfill, expression contracts | + +- **Risk:** Schema-only bootstrap → empty title vocab until document writes; trigger-already-exists on reapply; CI green on broken contracts. +- **Fix:** Mirror lifecycle in schema; strengthen `tests/supabase-schema.test.ts` (load both migrations; assert text UUID compare, revoke lines, corrector probe shape, index↔RPC equality for remaining trgm index). + +#### F9. New SECURITY DEFINER helpers miss privilege hardening + +- **Where:** `cleanup_registry_corpus_document`, `sync_document_title_words` created without `revoke execute … from public, anon, authenticated` (unlike `correct_clinical_query_terms`). +- **Fix:** Revoke execute; grant only if something other than triggers must call them (usually none). + +#### F10. Title vocabulary is global under SECURITY DEFINER (tenancy product decision) + +- **Where:** Corrector reads all `document_title_words` with no owner filter; sync indexes every indexed title. +- **Risk:** Private title tokens can bias corrections / existence side-channel for other users (pre-existing shape had similar full-title scan). +- **Fix options:** Public-only (`owner_id is null`) ± caller-owner filter passed like retrieval RPCs. **Needs product decision** (see §7). + +#### F11. Favourites set accent gradient bars (design) + +- **Where:** `src/components/clinical-dashboard/favourites-library-nav.tsx` — `setAccentBars` with purple→rose / multi-hue gradients. +- **Why it matters:** Conflicts with clinical “dense, calm, scan-fast” (design-review). Decorative vs semantic. +- **Fix:** Single clinical accent or discrete set-color tokens without marketing multi-hue bars. Preserve selected/hover/`focus-visible`. + +--- + +### P3 — hygiene / optional + +#### F12. Accidental `pnpm-lock.yaml` + +- Delete untracked file; do not commit. Repo lockfile is `package-lock.json`. + +#### F13. Mockup hardcoded hex / focus colors + +- **Where:** `src/components/favourites-page-mockups/favourites-library-redesign-page.tsx` etc. (`#0e7490`, `#64748b`, …). +- Mockup routes under `/mockups/…`. Retokenize if long-lived; do not promote into production shell without tokens. + +#### F14. Pill / glow density (optional design pass) + +- Widespread `rounded-full` + soft glow is existing product chrome, not a defect. Separate intentional de-pill pass only if product wants it. +- **Non-goal:** Do **not** replace the clinical design system with the marketing `/10-experience-and-design-system` dark-cyan DTCG aesthetic. + +#### F15. Broader architecture residual (out of this WIP’s minimal fix) + +- `rag.ts` remains a large facade (~4.8k lines). Keep hydration/cache testable in `rag-candidate-sources`; avoid new re-exports that force everything through `rag.ts`. +- Process-global answer/search caches in `rag-cache.ts` are pre-existing scale/ops residual. + +#### F16. Cross-table shared cleanup key (low probability) + +- Same cleanup fn matches only `registry_record_id` across registry tables → theoretical UUID collision deletes wrong corpus doc. Mitigate with `registry_record_kind` filter (see F3). + +--- + +## 4. What looked solid (do not regress) + +- Intent of request-scoped dedupe for parallel signal hydrations is good (happy path). +- `document_title_words` RLS + service_role grants + sync-on-indexed status logic is coherent **once** the corrector actually queries the table. +- Existing narrow `document_table_facts_title_row_param_trgm_idx` correctly matches current `trgm_matches`. +- Ingestion ↛ importing `rag.ts` dependency direction is healthy — preserve it. +- Production clinical shell mostly uses `@theme` tokens, `focus-visible`, `prefers-reduced-motion`, and `forced-colors` in `src/app/globals.css`. + +--- + +## 5. Remediation plan (ordered) + +### Track A — correctness (same PR / branch) + +1. **Branch hygiene:** From detached HEAD + WIP, create/checkout named branch e.g. `codex/rag-scalability-review-remediation`. Delete `pnpm-lock.yaml`. +2. **F1:** Restore loose types on `registryCorpusDetailHref`. Run `npm run typecheck`. +3. **F2/F6/F7:** Add failing hydration-cache tests (error poisoning, scope reuse, concurrent overlap). Fix cache: no negative-cache on error; scope-bound; single in-flight per id. Green tests. +4. **F3/F9/F16:** Rewrite cleanup SQL (text + kind), revoke execute; sync migration + `schema.sql`. Prefer **edit migration before apply**; if any env already applied WIP SQL, ship a **follow-up** migration instead of rewriting history. +5. **F4:** Rewrite `correct_clinical_query_terms` to indexed per-token probes **or** drop unused GIN/table until rewrite ready. Sync schema. +6. **F5:** Default — remove `…190000…` and wide index from schema; keep narrow index. (Alt — widen RPC and drop narrow.) Assert expression parity in tests. +7. **F8:** Align schema lifecycle (`DROP TRIGGER IF EXISTS`; document or include backfill policy). Strengthen schema tests beyond substrings. + +### Track B — design (same PR optional, or follow-up) + +8. **F11:** Quiet favourites gradient bars to clinical tokens. +9. **F13:** Optionally retokenize mockups. +10. **F14:** Explicitly defer pill/glow unless requested. + +### Verification (local, no providers unless confirmed) + +| Gate | When | +| ----------------------------------------------------------------- | ---------------- | +| Focused Vitest: hydration cache + `tests/supabase-schema.test.ts` | After A2–A7 | +| `npm run typecheck` | After A1 | +| `npm run verify:cheap` (or lint + unit subset) | Before handoff | +| `git diff --check` | Before commit | +| `npm run ensure` + screenshots | If Track B ships | +| Live Supabase apply / `check:drift` / OpenAI eval | **Ask first** | + +### Rollout notes + +- Treat `…180000…` / `…190000…` as **unapplied** unless proven otherwise. Confirm with operator before any live mutation. +- After code + offline gates are green: commit on feature branch, open PR, run PR-local gate; live apply is a separate confirmation. + +--- + +## 6. Suggested implementation ownership (files) + +| Concern | Primary files | +| --------------------- | --------------------------------------------------------------------------------------------------------------------- | +| Cache | `src/lib/rag-candidate-sources.ts`, `src/lib/rag.ts`, `tests/retrieval-hydration-scope.test.ts` (or new focused test) | +| Registry links typing | `src/lib/registry-corpus-links.ts` | +| SQL patch | `supabase/migrations/20260714180000_…`, possibly delete `…190000…`, `supabase/schema.sql` | +| Schema contracts | `tests/supabase-schema.test.ts` | +| Favourites design | `src/components/clinical-dashboard/favourites-library-nav.tsx` | +| Hygiene | delete `pnpm-lock.yaml` | + +--- + +## 7. Open questions (block product confidence) + +1. **Title vocab tenancy:** Should `correct_clinical_query_terms` use **public titles only**, or **public + caller-owner**? +2. **Table-facts trgm:** **Drop** the wide index (recommended) or **widen** `trgm_matches` and drop the narrow one? +3. **PR packaging:** Ship Track B (favourites design) in the **same** PR as RAG/SQL, or a follow-up? +4. **Live apply status:** Has any environment already applied `20260714180000` / `20260714190000`? (Assumed **no** at review time.) +5. **Corpus scale:** Approx. indexed doc / title-word cardinality — needed to prioritize F4 rewrite urgency vs drop-index interim. + +--- + +## 8. Checks already run (this review) + +| Check | Result | +| ------------------------------------------------------ | -------------------------------------------------------------------- | +| Static diff / SQL / RPC expression tracing | Done — findings above | +| Architecture explore + bug-hunt agents | Done — synthesized here | +| Clinical design / token / a11y greps | Done inline (`frontend-ui-reviewer` subagent blocked by usage limit) | +| `npm run typecheck` | **Fail** — F1 (+ possible stale `.next` noise) | +| Vitest / `verify:*` / browser / live Supabase / OpenAI | **Not run** | + +--- + +## 9. Next agent prompt (copy-paste) + +```text +Read docs/rag-scalability-wip-review-handover-2026-07-15.md and remediate Track A +findings F1–F9 (and F5 by dropping the mismatched trgm index unless told otherwise). +Park WIP on a named feature branch. Delete pnpm-lock.yaml. Do not apply live Supabase +or call OpenAI without confirmation. Use TDD for ChunkLoadCache. Strengthen +tests/supabase-schema.test.ts. End with typecheck + focused Vitest + verify:cheap +(or state what was skipped). Track B (favourites gradients) only if time / same PR +requested. Ask before commit/push/PR. +``` + +--- + +## 10. Related docs + +- `docs/codex-review-protocol.md` — severity / mutation / ledger rules +- `docs/branch-review-ledger.md` — this review’s ledger row +- `docs/design-system.md` / `docs/redesign/permanent-colour-direction.md` — clinical visual direction +- `docs/search-rag-master-context.md` — RAG orientation +- `AGENTS.md` — provider confirmation boundary, verify gates + +--- + +_Authored as a pure handover artifact from the 2026-07-15 multi-lens review. No production mutations performed._ diff --git a/docs/site-map.md b/docs/site-map.md index 8497dc327..986979aba 100644 --- a/docs/site-map.md +++ b/docs/site-map.md @@ -2,13 +2,11 @@ This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` to verify it is current. -## Main product pages +## Main product routes - `/` - Main Clinical KB shell. Source: `src/app/page.tsx`. -- `/applications` - Route discovered from app directory Source: `src/app/applications/page.tsx`. - `/differentials` - Differentials home and search surface. Source: `src/app/differentials/page.tsx`. - `/differentials/diagnoses` - Diagnosis stream. Source: `src/app/differentials/diagnoses/page.tsx`. -- `/differentials/presentations` - Presentation workflow stream. Source: `src/app/differentials/presentations/page.tsx`. - `/differentials/presentations/[slug]` - Route discovered from app directory Source: `src/app/differentials/presentations/[slug]/page.tsx`. - `/documents/search` - Documents search command centre. Source: `src/app/documents/search/page.tsx`. - `/documents/source` - Compatibility redirect to the canonical live document viewer when a valid id is supplied. Source: `src/app/documents/source/page.tsx`. @@ -22,7 +20,6 @@ This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` - `/formulation/builder` - Structured clinical formulation builder. Source: `src/app/formulation/builder/page.tsx`. - `/formulation/compare` - Side-by-side mechanism comparison. Source: `src/app/formulation/compare/page.tsx`. - `/formulation/map` - Formulation mechanism domain map. Source: `src/app/formulation/map/page.tsx`. -- `/medications` - Medication index redirect. Source: `src/app/medications/page.tsx`. - `/privacy` - Privacy and data-processing governance draft. Source: `src/app/privacy/page.tsx`. - `/reference/colour-coding` - Route discovered from app directory Source: `src/app/reference/colour-coding/page.tsx`. - `/services` - Services home and search surface. Source: `src/app/services/page.tsx`. @@ -46,7 +43,7 @@ This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` - `/formulation` - Formulation mode. Search kind: `formulation`. Query example: `/formulation?q=I+keep+going+over+it&focus=1&run=1`. - `/?mode=prescribing` - Medication mode. Search kind: `documents`. Query example: `/?mode=prescribing&q=acamprosate+renal+dose&focus=1&run=1`. - `/?mode=tools` - Tools mode. Search kind: `tools`. Query example: `/?mode=tools&q=medications&focus=1&run=1`. -- `/therapy-compass` - Therapy Compass mode. Search kind: `tools`. Query example: `/therapy-compass?q=behavioural+activation&focus=1&run=1`. +- `/therapy-compass` - Therapy mode. Search kind: `tools`. Query example: `/therapy-compass?q=behavioural+activation&focus=1&run=1`. ## Mode page index @@ -802,6 +799,11 @@ This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` - `/mockups/universal-search-command` - Route discovered from app directory Source: `src/app/mockups/universal-search-command/page.tsx`. - `/mockups/universal-search-redesign` - Route discovered from app directory Source: `src/app/mockups/universal-search-redesign/page.tsx`. +## Public utility route handlers + +- `/auth/callback` - Authentication callback handler. Source: `src/app/auth/callback/route.ts`. +- `/icons/[variant]` - Dynamically generated application icon handler. Source: `src/app/icons/[variant]/route.tsx`. + ## API routes - `/api/answer` - Generate answer response. Source: `src/app/api/answer/route.ts`. @@ -840,12 +842,13 @@ This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` - `/api/search/universal` - Route discovered from app directory Source: `src/app/api/search/universal/route.ts`. - `/api/setup-status` - Setup status. Source: `src/app/api/setup-status/route.ts`. - `/api/upload` - Upload endpoint. Source: `src/app/api/upload/route.ts`. -- `/auth/callback` - Route discovered from app directory Source: `src/app/auth/callback/route.ts`. ## Redirects +- `/applications` - Redirects to `/tools`. Source: `src/app/applications/route.ts`. +- `/differentials/presentations` - Redirects to `/differentials/presentations/[workflow-slug]`. Source: `src/app/differentials/presentations/route.ts`. - `/documents/source` - Redirects to `/documents/search`. Source: `src/app/documents/source/page.tsx`. -- `/medications` - Redirects to `/?mode=prescribing`. Source: `src/app/medications/page.tsx`. +- `/medications` - Redirects to `/?mode=prescribing`. Source: `src/app/medications/route.ts`. - `/mockups/favourites-hub` - Redirects to `/favourites`. Source: `src/app/mockups/favourites-hub/page.tsx`. - `/mockups/medication-prescribing` - Redirects to `/medications/acamprosate`. Source: `src/app/mockups/medication-prescribing/page.tsx`. diff --git a/docs/tenancy-defense-in-depth-review.md b/docs/tenancy-defense-in-depth-review.md index b3b591c11..9549f09bf 100644 --- a/docs/tenancy-defense-in-depth-review.md +++ b/docs/tenancy-defense-in-depth-review.md @@ -30,7 +30,8 @@ That said, this is a **single-layer** design with one structural weakness that * > any RPC is called. **Historical note (pre-#409):** the review below describes the fail-open edge that existed at audit -time. Item 1 in §6 is **DONE**; items 2–4 remain the recommended follow-ups. +time. Items 1 (fail-closed RPC) and 2 (CI owner-scope guard) in §6 are **DONE**; items 3–4 remain the +recommended follow-ups. **The one non-clean finding** is a **low-severity information disclosure**, not a tenancy leak: `setup-status` interpolates a raw Postgres RPC error string into its response @@ -273,11 +274,17 @@ small, largely-cooperative user set with a public shared corpus. 1. **Make the retrieval RPCs fail-_closed_ on a null owner filter — DONE (2026-07-08, PR #409).** `retrieval_owner_matches` now returns no rows when `owner_filter IS NULL`; the app uses the public sentinel for legitimate unauthenticated paths. Verify: `npm run check:july8-live-batch`. -2. **Add a CI guard against un-scoped owner tables (cheap, high value).** A lint/test that fails when a - new `src/app/api/**` handler queries an owner-scoped table without a recognised scoping construct - (`withOwnerReadScope`, `.eq('owner_id'`, `requireOwnerScope`, `documents!inner`+`documents.owner_id`). - This directly guards the regression class the single-layer model is exposed to — a future PR - dropping the filter. +2. **Add a CI guard against un-scoped owner tables (cheap, high value) — DONE (2026-07-17).** + [`scripts/check-owner-scope-api.mjs`](../scripts/check-owner-scope-api.mjs) fails when a + `src/app/api/**` handler queries an owner-scoped table (any table with an `owner_id` column in + `supabase/schema.sql`) without a recognised scoping construct in the enclosing handler — + `.eq('owner_id'`, `withOwnerReadScope`, `requireOwnerScope`, `requireOwnedDocument`/`loadOwnedDocument`, + a `documents!inner`+`documents.owner_id` join, or an `owner_id:` write payload. Confirmed-safe + indirect-scope cases live in a documented `OWNER_SCOPE_ALLOWLIST` (today only the two local-origin + `setup-status` existence probes, §3 / TEN-N1). Wired into `npm run check:owner-scope`, + `npm run verify:cheap`, and the CI `static-pr` job; regression-locked by + [`tests/owner-scope-guard.test.ts`](../tests/owner-scope-guard.test.ts). This directly guards the + regression class the single-layer model is exposed to — a future PR dropping the filter. 3. **Add a live cross-tenant integration test (medium value).** Fixtures for user A + user B; for each route family assert B cannot read/mutate A's non-null rows and gets 404/empty. This is the regression harness for the exact property the whole model depends on, and it is what would have @@ -291,8 +298,30 @@ small, largely-cooperative user set with a public shared corpus. **Bottom line:** the current single-layer enforcement is correct today (0/33 gaps). Item 1 (fail-closed RPC) is live in the repo (#409); **apply to production** per -[`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md). Items 2–3 close the remaining -app-layer regression exposure; full RLS (item 4) is justified before multi-tenant scale. +[`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md). Item 2 (CI owner-scope guard) is +now landed and blocks the regression class in CI. Item 3 (live cross-tenant integration test) closes the +remaining app-layer regression exposure; full RLS (item 4) is justified before multi-tenant scale. + +### Owner-scope guard allowlist (item 2) + +`scripts/check-owner-scope-api.mjs` flags any `src/app/api/**` query on an owner-scoped table that +lacks an owner filter in its enclosing handler. A query is scoped either **on its own chain** +(`.eq("owner_id"…)`, `withOwnerReadScope`) or by a **handler-level ownership proof** that precedes it +(`requireOwnedDocument`, an owner-checked `.select(...).eq("owner_id"…)`, or an `owner_id:` write +payload) — the guard checks the whole enclosing handler body, so the dominant "prove ownership, then +mutate/read by the proven id" idiom (e.g. `documents` PATCH selects `.eq("owner_id", user.id)` then +updates by `id`; `ingestion/quality` fetches owner-scoped document ids then reads child tables by +`document_id IN (…)`) is recognised without a per-statement dataflow analysis. Queries inside in-file +helpers fall back to whole-file scope because their caller proves ownership first (e.g. `selectLabels` +runs only after `requireOwnedDocument`). + +The guard's `OWNER_SCOPE_ALLOWLIST` holds **exactly** these reviewed indirect-scope exceptions (any +new entry must be added here and to the list in the guard, or the regression test fails): + +| File | Table | Why it is safe | +| ----------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ | +| `src/app/api/setup-status/route.ts` | `documents` | Local-origin-gated `.limit(1)` existence probe ("is any document indexed?"); returns only status booleans, not an owner-data read (§3 / TEN-N1). | +| `src/app/api/setup-status/route.ts` | `import_batches` | Local-origin-gated `.limit(1)` existence probe for schema provisioning; returns only status booleans, not an owner-data read (§3 / TEN-N1). | --- diff --git a/package-lock.json b/package-lock.json index 5e4db79c4..cb24168a0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,13 +10,14 @@ "hasInstallScript": true, "dependencies": { "@next/env": "16.2.10", + "@supabase/realtime-js": "^2.110.7", "@supabase/ssr": "^0.12.0", "@supabase/supabase-js": "^2.108.2", "exceljs": "^4.4.0", "jszip": "^3.10.1", "lucide-react": "^1.22.0", "mammoth": "^1.12.0", - "next": "16.2.10", + "next": "^16.2.10", "openai": "^6.45.0", "pdf-parse": "^2.4.5", "pdfjs-dist": "^6.1.200", @@ -24,6 +25,7 @@ "postcss": "^8.5.15", "react": "19.2.7", "react-dom": "19.2.7", + "server-only": "^0.0.1", "zod": "^4.4.3" }, "devDependencies": { @@ -49,7 +51,7 @@ "playwright": "^1.61.1", "prettier": "^3.9.4", "tailwindcss": "^4.3.1", - "tsx": "^4.22.4", + "tsx": "^4.23.1", "typescript": "^6.0.0", "vitest": "^4.1.10" }, @@ -3309,9 +3311,9 @@ } }, "node_modules/@supabase/phoenix": { - "version": "0.4.4", - "resolved": "https://registry.npmjs.org/@supabase/phoenix/-/phoenix-0.4.4.tgz", - "integrity": "sha512-Gt0pqoXuIqX/8dvG0OKp/wMCobXNH3klNbUPBNyOfN0YA1IswrM3HyWFMOPk1Jy+BRaIyDPcFx4jLBwHNmlyfQ==", + "version": "0.4.5", + "resolved": "https://registry.npmjs.org/@supabase/phoenix/-/phoenix-0.4.5.tgz", + "integrity": "sha512-aAn9H9ovVyeApKy11OWOrrOGq8DV68yWeH4ud2lN9fzn4aO8Zb5GLL9m1pUg9nLqIcT+ZDfAcsZe0E/nqdv2lw==", "license": "MIT" }, "node_modules/@supabase/postgrest-js": { @@ -3327,12 +3329,12 @@ } }, "node_modules/@supabase/realtime-js": { - "version": "2.110.2", - "resolved": "https://registry.npmjs.org/@supabase/realtime-js/-/realtime-js-2.110.2.tgz", - "integrity": "sha512-z3jTOTPgyn6E3r6dVOOQ10He4yAMB2czjFw7xVdX3s16MHElna5rY1gVaePs0NIo6xvtMYbtmOXlFaFt/ePLpg==", + "version": "2.110.7", + "resolved": "https://registry.npmjs.org/@supabase/realtime-js/-/realtime-js-2.110.7.tgz", + "integrity": "sha512-AMtZjyFA2gsmjuxopPNS/sRznLQHG0Ht5x+ytTPTOh3vAcOTUlVRLx7gW4/CONNnbb3PKOkE+HmM35HOSbmomQ==", "license": "MIT", "dependencies": { - "@supabase/phoenix": "0.4.4", + "@supabase/phoenix": "0.4.5", "tslib": "2.8.1" }, "engines": { @@ -3380,6 +3382,25 @@ "node": ">=22.0.0" } }, + "node_modules/@supabase/supabase-js/node_modules/@supabase/phoenix": { + "version": "0.4.4", + "resolved": "https://registry.npmjs.org/@supabase/phoenix/-/phoenix-0.4.4.tgz", + "integrity": "sha512-Gt0pqoXuIqX/8dvG0OKp/wMCobXNH3klNbUPBNyOfN0YA1IswrM3HyWFMOPk1Jy+BRaIyDPcFx4jLBwHNmlyfQ==", + "license": "MIT" + }, + "node_modules/@supabase/supabase-js/node_modules/@supabase/realtime-js": { + "version": "2.110.2", + "resolved": "https://registry.npmjs.org/@supabase/realtime-js/-/realtime-js-2.110.2.tgz", + "integrity": "sha512-z3jTOTPgyn6E3r6dVOOQ10He4yAMB2czjFw7xVdX3s16MHElna5rY1gVaePs0NIo6xvtMYbtmOXlFaFt/ePLpg==", + "license": "MIT", + "dependencies": { + "@supabase/phoenix": "0.4.4", + "tslib": "2.8.1" + }, + "engines": { + "node": ">=22.0.0" + } + }, "node_modules/@swc/helpers": { "version": "0.5.15", "resolved": "https://registry.npmjs.org/@swc/helpers/-/helpers-0.5.15.tgz", @@ -10307,6 +10328,12 @@ "semver": "bin/semver.js" } }, + "node_modules/server-only": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/server-only/-/server-only-0.0.1.tgz", + "integrity": "sha512-qepMx2JxAa5jjfzxG79yPPq+8BuFToHd1hm7kI+Z4zAq1ftQiP7HcxMhDDItrbtwVeLg/cY2JnKnrcFkmiswNA==", + "license": "MIT" + }, "node_modules/set-function-length": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.2.2.tgz", @@ -11083,9 +11110,9 @@ "license": "0BSD" }, "node_modules/tsx": { - "version": "4.23.0", - "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.23.0.tgz", - "integrity": "sha512-eUdUIaCr963q2h5u3+QwvYp0+eqPvn+egeqZUm0hwERCqqx1E3kK5ehbGCvqSE5MQAULr67ww0cA3jKc3YkM1w==", + "version": "4.23.1", + "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.23.1.tgz", + "integrity": "sha512-GQHnkIfxyx1wYCOS/wonik5MVRZU9hi1TEZmzGZSCJB1y9YgoZ8H6itNE/u4suE+yLmOzuE4E5S4TZ/ZX2wcWQ==", "dev": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 00a8c6125..a2f67ddb8 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test:e2e:visual": "node scripts/run-playwright.mjs --config=playwright.visual.config.ts", "test:cross-tenant:staging": "node scripts/run-tsx.mjs scripts/test-cross-tenant-staging.ts", "verify:cheap": "node scripts/run-heavy.mjs --npm-script verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run sitemap:check && npm run brand:check && npm run check:type-scale && npm run check:icon-scale && npm run check:function-grants && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run sitemap:check && npm run brand:check && npm run check:type-scale && npm run check:icon-scale && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:ui": "npm run check:runtime && npm run test:e2e:pr", "verify:release": "npm run check:runtime && npm run lint && npm run typecheck && npm run test && npm run build && npm run test:e2e && npm run check:production-readiness && npm run governance:release && npm run eval:quality:release", @@ -104,6 +104,7 @@ "check:type-scale": "node scripts/check-type-scale.mjs --strict", "check:icon-scale": "node scripts/check-icon-scale.mjs --strict", "check:function-grants": "node scripts/check-function-grants.mjs", + "check:owner-scope": "node scripts/check-owner-scope-api.mjs", "recover:ingestion": "node scripts/run-tsx.mjs scripts/recover-ingestion-queue.ts", "registry:seed": "node scripts/run-tsx.mjs scripts/seed-registry-records.ts", "registry:embed": "node scripts/run-tsx.mjs scripts/embed-registry-records.ts", @@ -168,13 +169,14 @@ }, "dependencies": { "@next/env": "16.2.10", + "@supabase/realtime-js": "^2.110.7", "@supabase/ssr": "^0.12.0", "@supabase/supabase-js": "^2.108.2", "exceljs": "^4.4.0", "jszip": "^3.10.1", "lucide-react": "^1.22.0", "mammoth": "^1.12.0", - "next": "16.2.10", + "next": "^16.2.10", "openai": "^6.45.0", "pdf-parse": "^2.4.5", "pdfjs-dist": "^6.1.200", @@ -182,6 +184,7 @@ "postcss": "^8.5.15", "react": "19.2.7", "react-dom": "19.2.7", + "server-only": "^0.0.1", "zod": "^4.4.3" }, "overrides": { @@ -213,8 +216,16 @@ "playwright": "^1.61.1", "prettier": "^3.9.4", "tailwindcss": "^4.3.1", - "tsx": "^4.22.4", + "tsx": "^4.23.1", "typescript": "^6.0.0", "vitest": "^4.1.10" + }, + "allowScripts": { + "esbuild@0.28.1": true, + "sharp@0.34.5": true, + "unrs-resolver@1.12.2": true, + "@eslint/eslintrc@3.3.6": true, + "@humanwhocodes/module-importer@1.0.1": true, + "@humanfs/node@0.16.8": true } } diff --git a/playwright.config.ts b/playwright.config.ts index 268ccd250..aa1b899ee 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -55,6 +55,7 @@ export default defineConfig({ grepInvert: mockupTag, use: { ...devices["Desktop Chrome"], + reducedMotion: "no-preference", ...(chromiumExecutablePath ? { launchOptions: { executablePath: chromiumExecutablePath } } : {}), }, }, diff --git a/scripts/check-codebase-index-coverage.mjs b/scripts/check-codebase-index-coverage.mjs index 15a4d4686..b912467a0 100644 --- a/scripts/check-codebase-index-coverage.mjs +++ b/scripts/check-codebase-index-coverage.mjs @@ -22,6 +22,7 @@ import { fileURLToPath, pathToFileURL } from "node:url"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const INDEX_PATH = "docs/codebase-index.md"; +const SCHEMA_PATH = "supabase/schema.sql"; // Directories intentionally not indexed at the top level. const ALLOWLIST = new Set([ @@ -29,38 +30,79 @@ const ALLOWLIST = new Set([ ]); function dirsIn(relativeDir) { - try { - return readdirSync(path.join(repoRoot, relativeDir), { withFileTypes: true }) - .filter((e) => e.isDirectory()) - .map((e) => e.name); - } catch { - return []; - } + return readdirSync(path.join(repoRoot, relativeDir), { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name); } -/** - * A directory counts as "indexed" when its base name appears anywhere in the index - * (case-insensitive). The index deliberately uses abbreviated forms — `validation/`, - * `extractors/document.ts`, grouped route tables — so matching the bare name (rather - * than a full `src/lib/x` path) is what actually catches a WHOLLY missing module - * without drowning in format false-positives. - */ export function coverageCandidates(kind, name) { - return [name]; + if (kind === "api") return [`/api/${name}`]; + if (kind === "route") return [`/${name}`]; + return [`${name}/`, `src/lib/${name}/`]; +} + +const SECTION_BOUNDS = { + route: ["### Product pages (`src/app/`)", "### API routes (`src/app/api/`)"], + api: ["### API routes (`src/app/api/`)", "## `src/lib/` module map"], + lib: ["## `src/lib/` module map", "## Supabase"], + schema: ["### Schema tables", "### Migration themes"], +}; + +function sectionText(indexText, kind) { + const [startMarker, endMarker] = SECTION_BOUNDS[kind] ?? []; + if (!startMarker) return ""; + const headingOffset = (marker, from = 0) => { + const escaped = marker.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const match = new RegExp(`^${escaped}\\r?$`, "m").exec(indexText.slice(from)); + return match ? from + match.index : -1; + }; + const start = headingOffset(startMarker); + if (start < 0) return ""; + const end = headingOffset(endMarker, start + startMarker.length); + return indexText.slice(start, end < 0 ? indexText.length : end); +} + +function codeSpans(text) { + return [...text.matchAll(/`([^`\r\n]+)`/g)].map((match) => match[1].trim().toLowerCase()); +} + +function candidateMatches(span, candidate) { + const normalized = candidate.toLowerCase(); + if (normalized.endsWith("/")) return span.startsWith(normalized); + return span === normalized || span.startsWith(`${normalized}/`); } /** Pure: given the index text and the discovered groups, return the uncovered entries. */ export function coverageGaps(indexText, groups, allowlist = ALLOWLIST) { - const haystack = indexText.toLowerCase(); + const spansByKind = new Map(["lib", "route", "api"].map((kind) => [kind, codeSpans(sectionText(indexText, kind))])); const gaps = []; for (const { kind, dir, name } of groups) { const full = `${dir}/${name}`; if (allowlist.has(full)) continue; - if (!haystack.includes(name.toLowerCase())) gaps.push({ full, kind, tried: [name] }); + const tried = coverageCandidates(kind, name); + const spans = spansByKind.get(kind) ?? []; + if (!tried.some((candidate) => spans.some((span) => candidateMatches(span, candidate)))) { + gaps.push({ full, kind, tried }); + } } return gaps; } +/** Pure: compare the exhaustive schema-table list in the index with the schema mirror. */ +export function schemaTableGaps(indexText, schemaText) { + const schemaTables = new Set( + [...schemaText.matchAll(/create\s+table(?:\s+if\s+not\s+exists)?\s+public\.([a-z0-9_]+)/gi)].map((match) => + match[1].toLowerCase(), + ), + ); + const tableSection = sectionText(indexText, "schema"); + const documentedTables = new Set(codeSpans(tableSection).filter((span) => /^[a-z][a-z0-9_]*$/.test(span))); + return { + missing: [...schemaTables].filter((table) => !documentedTables.has(table)).sort(), + stale: [...documentedTables].filter((table) => !schemaTables.has(table)).sort(), + }; +} + function discoverGroups() { const groups = []; for (const name of dirsIn("src/lib")) groups.push({ kind: "lib", dir: "src/lib", name }); @@ -74,16 +116,22 @@ function discoverGroups() { function main() { const indexText = readFileSync(path.join(repoRoot, INDEX_PATH), "utf8"); + const schemaText = readFileSync(path.join(repoRoot, SCHEMA_PATH), "utf8"); const groups = discoverGroups(); const gaps = coverageGaps(indexText, groups); + const tables = schemaTableGaps(indexText, schemaText); - if (gaps.length > 0) { + if (gaps.length > 0 || tables.missing.length > 0 || tables.stale.length > 0) { console.error(`\n${INDEX_PATH} is missing ${gaps.length} top-level module(s)/route(s):`); for (const g of gaps) console.error(` UNINDEXED ${g.full} (${g.kind}) — add it or allowlist it`); + for (const table of tables.missing) console.error(` UNINDEXED public.${table} (schema table)`); + for (const table of tables.stale) console.error(` STALE ${table} (not present in ${SCHEMA_PATH})`); console.error(`\nUpdate ${INDEX_PATH} so the agent-orientation map stays current.`); process.exit(1); } - console.log(`${INDEX_PATH} coverage OK: all ${groups.length} top-level modules/routes are indexed.`); + console.log( + `${INDEX_PATH} coverage OK: all ${groups.length} top-level modules/routes and ${tables.missing.length + tables.stale.length === 0 ? "all" : "checked"} schema tables are indexed.`, + ); } const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; diff --git a/scripts/check-github-action-pins.mjs b/scripts/check-github-action-pins.mjs index 6a82fae7b..82d195306 100644 --- a/scripts/check-github-action-pins.mjs +++ b/scripts/check-github-action-pins.mjs @@ -1,4 +1,4 @@ -import { readdirSync, readFileSync } from "node:fs"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; import path from "node:path"; import { validateActionReference } from "./github-action-pins.mjs"; import { yamlBlock } from "./yaml-contract.mjs"; @@ -10,10 +10,16 @@ const failures = []; const expectedSupabaseCliVersion = "2.108.0"; const expectedSupabaseCliVersionPattern = expectedSupabaseCliVersion.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -for (const fileName of readdirSync(workflowDir) - .filter((name) => /\.ya?ml$/i.test(name)) - .sort()) { - const filePath = path.join(workflowDir, fileName); +function discoverGitHubActionFiles(workflowRoot) { + const workflowDir = path.join(workflowRoot, ".github", "workflows"); + if (!existsSync(workflowDir)) return []; + return readdirSync(workflowDir, { withFileTypes: true }) + .filter((entry) => entry.isFile() && /\.ya?ml$/i.test(entry.name)) + .map((entry) => path.join(workflowDir, entry.name)); +} + +for (const filePath of discoverGitHubActionFiles(process.cwd())) { + const fileName = path.relative(process.cwd(), filePath).replaceAll("\\", "/"); const lines = readFileSync(filePath, "utf8").split(/\r?\n/); lines.forEach((line, index) => { diff --git a/scripts/check-m13-migration.ts b/scripts/check-m13-migration.ts index 3ec4b6e20..7e09011ac 100644 --- a/scripts/check-m13-migration.ts +++ b/scripts/check-m13-migration.ts @@ -17,6 +17,20 @@ function missingMarkers(data: unknown) { } async function main() { + try { + const { createRequire, Module } = await import("module"); + const req = createRequire(import.meta.url); + const resolved = req.resolve("server-only"); + if (resolved) { + const serverOnlyStub = new Module(resolved); + serverOnlyStub.exports = {}; + serverOnlyStub.loaded = true; + req.cache[resolved] = serverOnlyStub; + } + } catch { + // ignore + } + const { createAdminClient } = await import("@/lib/supabase/admin"); const supabase = createAdminClient(); const { data, error } = await supabase.rpc("search_schema_health"); diff --git a/scripts/check-owner-scope-api.mjs b/scripts/check-owner-scope-api.mjs new file mode 100644 index 000000000..431646677 --- /dev/null +++ b/scripts/check-owner-scope-api.mjs @@ -0,0 +1,299 @@ +#!/usr/bin/env node +// Defense-in-depth tenancy guard (audit finding D2 / M6). +// +// The app is a deliberately single-layer tenancy design: every API route uses the +// service-role Supabase client (RLS bypassed) and enforces ownership in application +// code via an `owner_id` filter. `docs/tenancy-defense-in-depth-review.md` verified +// 0/33 route gaps, but flagged (§6 item 2) that a *future* handler dropping the owner +// filter is the single regression class this design is exposed to. +// +// This guard closes that class statically: it fails when a `src/app/api/**` handler +// queries an OWNER-SCOPED table (any table with an `owner_id` column in +// supabase/schema.sql) without a recognised owner-scoping construct in the enclosing +// handler — `.eq("owner_id"...)`, `withOwnerReadScope`, `requireOwnerScope`, +// `requireOwnedDocument`/`loadOwnedDocument`/`ownedDocumentId`, a `documents!inner` +// + `documents.owner_id` join, or an `owner_id:` write payload. Intentional +// exceptions (indirect scoping the reviewer confirmed safe) live in +// OWNER_SCOPE_ALLOWLIST with a reason. +// +// Usage: +// node scripts/check-owner-scope-api.mjs scan the repo; exit 1 on any violation +// node scripts/check-owner-scope-api.mjs --self-test run the synthetic pass/fail fixtures + +import { readFileSync, realpathSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +// Recognised owner-scoping constructs. If any appears in the enclosing handler of an +// owner-scoped `.from(...)`, that query is considered scoped. `owner_id` (as a substring) +// covers `.eq("owner_id"...)`, `.is("owner_id"...)`, `.or("owner_id.eq...")`, insert/update +// `owner_id:` payloads, and `documents.owner_id` inner-join predicates. The named helpers +// cover the cases where scoping is delegated to a shared primitive. +const SCOPE_TOKENS = [ + "owner_id", + "withOwnerReadScope", + "requireOwnerScope", + "retrievalOwnerFilter", + "requireOwnedDocument", + "loadOwnedDocument", + "ownedDocumentId", + "assertGlobalSearchAllowed", + "resolveSearchScope", +]; + +// Intentional exceptions: a handler that queries an owner-scoped table where ownership +// is enforced indirectly (e.g. the query filters by document ids that were themselves +// fetched under an owner scope). Each entry needs a reason and a reviewer sign-off in +// docs/tenancy-defense-in-depth-review.md. Keep this list empty unless a real, reviewed +// indirect-scope pattern exists — a forgotten filter must NOT be silenced here. +export const OWNER_SCOPE_ALLOWLIST = [ + { + file: "src/app/api/setup-status/route.ts", + table: "documents", + reason: + "Global setup/health diagnostic: a `.limit(1)` existence probe (is any document indexed?), not an owner-data read. The route is gated to local origin and returns only status booleans — see docs/tenancy-defense-in-depth-review.md §3 (setup-status row / TEN-N1).", + }, + { + file: "src/app/api/setup-status/route.ts", + table: "import_batches", + reason: + "Global setup/health diagnostic: a `.limit(1)` existence probe for schema provisioning, not an owner-data read. Same local-origin-gated status route — see docs/tenancy-defense-in-depth-review.md §3 (TEN-N1).", + }, +]; + +/** Extract table names that declare an `owner_id` column from supabase/schema.sql. */ +export function ownerScopedTablesFromSchema(schemaText) { + const tables = new Set(); + let current = null; + for (const raw of schemaText.split("\n")) { + const line = raw.trim(); + const createMatch = line.match(/^create table (?:if not exists )?public\.([a-z0-9_]+)/i); + if (createMatch) { + current = createMatch[1]; + continue; + } + if (!current) continue; + // A column definition named owner_id (not a comment, not a cross-table reference). + if (/^owner_id\b/.test(line)) tables.add(current); + // End of the CREATE TABLE statement. + if (line === ");" || line.startsWith(") ")) current = null; + } + return tables; +} + +/** + * Split source into top-level declaration segments so a `.from(...)` can be checked against only + * its enclosing handler's text. + * + * Boundaries are **column-0 (top-level) declarations only** — nested functions are indented in + * this Prettier-formatted codebase and therefore never split a handler body. This is the fix for + * the finding that a nested helper inside a handler used to spill the code after it into a + * separate non-handler segment, which then fell back to whole-file scope and let an `owner_id` + * token elsewhere in the file mask a genuinely-unscoped query. Anchoring to column 0 keeps every + * statement lexically inside the handler that encloses it, without fragile brace/string matching. + */ +const HANDLER_START = /^export\s+(async\s+)?function\s+(GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS)\b/; + +// A top-level `function`/`const` declaration (no leading indentation → column 0). Nested +// declarations are indented and are intentionally NOT boundaries. +const TOP_LEVEL_DECL = /^(export\s+)?(async\s+)?function\s+\w+|^(export\s+)?const\s+\w+\s*=/; + +function functionSegments(text) { + const lines = text.split("\n"); + const starts = []; + lines.forEach((line, i) => { + if (TOP_LEVEL_DECL.test(line)) starts.push(i); + }); + if (starts.length === 0) return [{ startLine: 0, text, isHandler: false }]; + const segments = []; + // Anything before the first declaration (imports/consts) — its own segment. + if (starts[0] > 0) segments.push({ startLine: 0, text: lines.slice(0, starts[0]).join("\n"), isHandler: false }); + for (let s = 0; s < starts.length; s++) { + const from = starts[s]; + const to = s + 1 < starts.length ? starts[s + 1] : lines.length; + segments.push({ + startLine: from, + text: lines.slice(from, to).join("\n"), + isHandler: HANDLER_START.test(lines[from]), + }); + } + return segments; +} + +function isAllowlisted(file, table) { + return OWNER_SCOPE_ALLOWLIST.some((e) => e.file === file && e.table === table); +} + +/** + * Find owner-scope violations in a single file. + * @returns {{file:string,table:string,line:number}[]} + */ +export function analyzeFile(file, text, ownerTables) { + const violations = []; + const segments = functionSegments(text); + const fromRe = /\.from\(\s*["'`]([a-z0-9_]+)["'`]\s*\)/g; + let m; + while ((m = fromRe.exec(text)) !== null) { + const table = m[1]; + if (!ownerTables.has(table)) continue; + if (isAllowlisted(file, table)) continue; + const lineNo = text.slice(0, m.index).split("\n").length; + const segment = segments.find((seg) => { + const segEndLine = seg.startLine + seg.text.split("\n").length; + return lineNo - 1 >= seg.startLine && lineNo - 1 < segEndLine; + }); + // Route handlers are checked strictly against their own (column-0-bounded) body, so a + // scoping construct in one handler cannot excuse an unscoped query in a sibling handler. + // Queries inside in-file helpers (or top-level) fall back to the whole file, because their + // ownership is enforced by the handler(s) that call them within the same file (e.g. a + // `selectLabels` helper reached only after `requireOwnedDocument`). + const scopeText = segment && segment.isHandler ? segment.text : text; + const scoped = SCOPE_TOKENS.some((tok) => scopeText.includes(tok)); + if (!scoped) violations.push({ file, table, line: lineNo }); + } + return violations; +} + +/** Scan every tracked src/app/api file for owner-scope violations. */ +export function scanRepo({ schemaText, files }) { + const ownerTables = ownerScopedTablesFromSchema(schemaText); + const violations = []; + for (const { path, text } of files) { + violations.push(...analyzeFile(path, text, ownerTables)); + } + return { ownerTables, violations }; +} + +function readTrackedApiFiles() { + // execFile (fixed argv, no shell) rather than execSync with a command string. + const listed = execFileSync("git", ["ls-files", "src/app/api"], { encoding: "utf8" }) + .split("\n") + .filter((f) => /\.tsx?$/.test(f)); + return listed + .map((path) => { + try { + return { path, text: readFileSync(path, "utf8") }; + } catch { + return null; + } + }) + .filter(Boolean); +} + +function runSelfTest() { + const ownerTables = new Set(["documents"]); + const failures = []; + const expect = (cond, label) => { + if (!cond) failures.push(label); + }; + + // A brand-new handler that forgets the owner filter must be flagged. + const unscoped = `export async function GET(request) { + const supabase = createAdminClient(); + const { data } = await supabase.from("documents").select("*"); + return NextResponse.json({ data }); + }`; + expect(analyzeFile("fixture-unscoped.ts", unscoped, ownerTables).length === 1, "unscoped handler should be flagged"); + + // A handler with an explicit owner filter must pass. + const scopedEq = `export async function GET(request) { + const supabase = createAdminClient(); + const user = await requireAuthenticatedUser(request, supabase); + const { data } = await supabase.from("documents").select("*").eq("owner_id", user.id); + return NextResponse.json({ data }); + }`; + expect(analyzeFile("fixture-eq.ts", scopedEq, ownerTables).length === 0, "eq(owner_id) handler should pass"); + + // A handler using the shared read-scope helper must pass. + const scopedHelper = `export async function GET(request) { + const supabase = createAdminClient(); + const { data } = await withOwnerReadScope(supabase.from("documents").select("*"), access.ownerId); + return NextResponse.json({ data }); + }`; + expect( + analyzeFile("fixture-helper.ts", scopedHelper, ownerTables).length === 0, + "withOwnerReadScope handler should pass", + ); + + // Scoping in one handler must NOT excuse an unscoped query in a sibling handler. + const twoHandlers = `${scopedEq}\n${unscoped}`; + expect( + analyzeFile("fixture-two.ts", twoHandlers, ownerTables).length === 1, + "per-handler: sibling unscoped query still flagged", + ); + + // Column-0 handler boundaries: a nested arrow helper inside POST must not split the handler + // body into a whole-file fallback that GET's owner filter would satisfy. + const nestedLeak = [ + "export async function GET(request) {", + " const user = await auth(request);", + ' return supabase.from("documents").select("*").eq("owner_id", user.id);', + "}", + "export async function POST(request) {", + " const helper = (row) => row;", + ' return supabase.from("documents").select("*");', + "}", + ].join("\n"); + expect( + analyzeFile("fixture-nested.ts", nestedLeak, ownerTables).length === 1, + "nested helper / sibling handler must not mask an unscoped query", + ); + + // A non-owner-scoped table is not the guard's concern. + const otherTable = `export async function GET() { + const { data } = await supabase.from("document_chunks").select("*"); + return data; + }`; + expect(analyzeFile("fixture-other.ts", otherTable, ownerTables).length === 0, "non-owner table not flagged"); + + // Schema parsing picks up owner_id tables and skips owner-less ones. + const schema = `create table public.documents (\n id uuid,\n owner_id uuid\n);\ncreate table public.document_images (\n id uuid,\n document_id uuid\n);`; + const parsed = ownerScopedTablesFromSchema(schema); + expect(parsed.has("documents") && !parsed.has("document_images"), "schema parse: owner_id tables only"); + + if (failures.length > 0) { + console.error("✗ owner-scope guard self-test FAILED:"); + for (const f of failures) console.error(` - ${f}`); + process.exit(1); + } + console.log("✓ owner-scope guard self-test passed."); +} + +function main() { + if (process.argv.includes("--self-test")) { + runSelfTest(); + return; + } + const schemaText = readFileSync("supabase/schema.sql", "utf8"); + const files = readTrackedApiFiles(); + const { ownerTables, violations } = scanRepo({ schemaText, files }); + + if (violations.length === 0) { + console.log( + `✓ owner-scope: ${files.length} src/app/api files clean against ${ownerTables.size} owner-scoped tables.`, + ); + process.exit(0); + } + + console.error(`✗ owner-scope: ${violations.length} query(ies) on owner-scoped tables lack an owner filter:\n`); + for (const v of violations) { + console.error( + ` ${v.file}:${v.line} .from("${v.table}") — no owner_id / withOwnerReadScope / owned-doc guard in this handler`, + ); + } + console.error( + '\nScope the query (.eq("owner_id", …) or withOwnerReadScope/requireOwnedDocument), or, if ownership is enforced\n' + + "indirectly and reviewed, add a documented entry to OWNER_SCOPE_ALLOWLIST in scripts/check-owner-scope-api.mjs.", + ); + process.exit(1); +} + +// Only run the scan when executed directly (not when imported by the test suite). +const invokedDirectly = (() => { + try { + return process.argv[1] && realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url)); + } catch { + return false; + } +})(); +if (invokedDirectly) main(); diff --git a/scripts/generate-site-map.ts b/scripts/generate-site-map.ts index 921cf2b90..87ddeb945 100644 --- a/scripts/generate-site-map.ts +++ b/scripts/generate-site-map.ts @@ -16,7 +16,7 @@ const appDir = path.join(process.cwd(), "src", "app"); const siteMapPath = path.join(process.cwd(), "docs", "site-map.md"); const medicationSlugs = ["acamprosate"] as const; -type RouteKind = "page" | "api"; +type RouteKind = "page" | "handler"; type DiscoveredRoute = { route: string; @@ -31,13 +31,23 @@ type RedirectRoute = { type SiteMapData = { pageRoutes: DiscoveredRoute[]; + publicRouteHandlers: DiscoveredRoute[]; apiRoutes: DiscoveredRoute[]; redirects: RedirectRoute[]; nonRoutedMockupArtifacts: string[]; }; +const productRouteHandlerPaths = new Set(["/applications", "/differentials/presentations", "/medications"]); + +const documentedRedirectTargets: Record = { + "/applications": "/tools", + "/differentials/presentations": "/differentials/presentations/[workflow-slug]", + "/medications": "/?mode=prescribing", +}; + const routeDescriptions: Record = { "/": "Main Clinical KB shell.", + "/applications": "Legacy application launcher redirect to Tools.", "/differentials": "Differentials home and search surface.", "/differentials/diagnoses": "Diagnosis stream.", "/differentials/diagnoses/[slug]": "Differential diagnosis detail.", @@ -71,6 +81,11 @@ const routeDescriptions: Record = { "/specifiers/map": "Psychiatric specifier family map.", }; +const publicRouteHandlerDescriptions: Record = { + "/auth/callback": "Authentication callback handler.", + "/icons/[variant]": "Dynamically generated application icon handler.", +}; + const apiDescriptions: Record = { "/api/answer": "Generate answer response.", "/api/answer/stream": "Streaming answer response.", @@ -127,8 +142,16 @@ function routeSegment(segment: string) { return segment; } +function isApiRoute(route: string) { + return route === "/api" || route.startsWith("/api/"); +} + function fileToRoute(filePath: string, kind: RouteKind) { - const suffix = kind === "page" ? "page.tsx" : "route.ts"; + const suffix = path.basename(filePath); + const expectedSuffixes = kind === "page" ? ["page.tsx"] : ["route.ts", "route.tsx"]; + if (!expectedSuffixes.includes(suffix)) { + throw new Error(`Unsupported ${kind} route file: ${filePath}`); + } const relative = toPosixPath(path.relative(appDir, filePath)); const withoutFile = relative.slice(0, -suffix.length).replace(/\/$/, ""); const segments = withoutFile.split("/").filter(Boolean).map(routeSegment).filter(Boolean); @@ -149,8 +172,9 @@ function collectFiles(root: string, targetFileName: string): string[] { } function discoverRoutes(kind: RouteKind): DiscoveredRoute[] { - const targetFile = kind === "page" ? "page.tsx" : "route.ts"; - return collectFiles(appDir, targetFile) + const targetFiles = kind === "page" ? ["page.tsx"] : ["route.ts", "route.tsx"]; + return targetFiles + .flatMap((targetFile) => collectFiles(appDir, targetFile)) .map((file) => ({ route: fileToRoute(file, kind), file: toPosixPath(path.relative(process.cwd(), file)), @@ -158,12 +182,13 @@ function discoverRoutes(kind: RouteKind): DiscoveredRoute[] { .sort((left, right) => left.route.localeCompare(right.route) || left.file.localeCompare(right.file)); } -function discoverRedirects(pageRoutes: DiscoveredRoute[]): RedirectRoute[] { - return pageRoutes - .map((page) => { - const source = readFileSync(path.join(process.cwd(), page.file), "utf8"); - const target = source.match(/\bredirect\(\s*["']([^"']+)["']\s*\)/)?.[1]; - return target ? { ...page, target } : null; +function discoverRedirects(routes: DiscoveredRoute[]): RedirectRoute[] { + return routes + .map((route) => { + const source = readFileSync(path.join(process.cwd(), route.file), "utf8"); + const target = + documentedRedirectTargets[route.route] ?? source.match(/\bredirect\(\s*["']([^"']+)["']\s*\)/)?.[1]; + return target ? { ...route, target } : null; }) .filter((value): value is RedirectRoute => Boolean(value)) .sort((left, right) => left.route.localeCompare(right.route)); @@ -179,10 +204,13 @@ function discoverNonRoutedMockupArtifacts() { export function collectSiteMapData(): SiteMapData { const pageRoutes = discoverRoutes("page"); + const routeHandlers = discoverRoutes("handler"); + const publicRouteHandlers = routeHandlers.filter((route) => !isApiRoute(route.route)); return { pageRoutes, - apiRoutes: discoverRoutes("api"), - redirects: discoverRedirects(pageRoutes), + publicRouteHandlers, + apiRoutes: routeHandlers.filter((route) => isApiRoute(route.route)), + redirects: discoverRedirects([...pageRoutes, ...publicRouteHandlers]), nonRoutedMockupArtifacts: discoverNonRoutedMockupArtifacts(), }; } @@ -363,6 +391,9 @@ function renderSiteMapRaw(data = collectSiteMapData()) { ].includes(route.route), ); const mockupRoutes = data.pageRoutes.filter((route) => route.route.startsWith("/mockups")); + const publicUtilityRouteHandlers = data.publicRouteHandlers.filter( + (route) => !productRouteHandlerPaths.has(route.route), + ); const lines = [ "# Clinical KB Site Map", @@ -370,7 +401,7 @@ function renderSiteMapRaw(data = collectSiteMapData()) { "This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` to verify it is current.", "", ...section( - "Main product pages", + "Main product routes", productRoutes.map((route) => routeLine(route, routeDescriptions)), ), ...section("Mode/query routes", renderModeRoutes()), @@ -447,6 +478,10 @@ function renderSiteMapRaw(data = collectSiteMapData()) { ] : []), ]), + ...section( + "Public utility route handlers", + publicUtilityRouteHandlers.map((route) => routeLine(route, publicRouteHandlerDescriptions)), + ), ...section( "API routes", data.apiRoutes.map((route) => routeLine(route, apiDescriptions)), diff --git a/src/app/api/documents/[id]/labels/route.ts b/src/app/api/documents/[id]/labels/route.ts index 6cd574841..6cba3a086 100644 --- a/src/app/api/documents/[id]/labels/route.ts +++ b/src/app/api/documents/[id]/labels/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; import { normalizeDocumentLabelForStorage } from "@/lib/document-tags"; @@ -110,6 +111,17 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); const { data: existing, error: existingError } = await supabase @@ -167,6 +179,17 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); if ("action" in parsed) { @@ -265,6 +288,17 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); const { data: existing, error: existingError } = await supabase diff --git a/src/app/api/documents/[id]/table-facts/route.ts b/src/app/api/documents/[id]/table-facts/route.ts index 18653deb5..8481cb753 100644 --- a/src/app/api/documents/[id]/table-facts/route.ts +++ b/src/app/api/documents/[id]/table-facts/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; import { invalidateRagCachesForOwner } from "@/lib/rag"; @@ -44,6 +45,17 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const document = await loadOwnedDocument({ supabase, documentId: id, ownerId: user.id }); if (!document) { return NextResponse.json({ error: "Document not found." }, { status: 404 }); @@ -78,6 +90,17 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const document = await loadOwnedDocument({ supabase, documentId: id, ownerId: user.id }); if (!document) { return NextResponse.json({ error: "Document not found." }, { status: 404 }); diff --git a/src/app/api/documents/bulk/route.ts b/src/app/api/documents/bulk/route.ts index 0d04d269e..1223d7087 100644 --- a/src/app/api/documents/bulk/route.ts +++ b/src/app/api/documents/bulk/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { normalizeDocumentLabelForStorage } from "@/lib/document-tags"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; @@ -129,6 +130,17 @@ export async function POST(request: Request) { const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const ids = Array.from(new Set(parsed.documentIds)); const { data: documents, error: documentsError } = await supabase diff --git a/src/app/applications/page.tsx b/src/app/applications/page.tsx deleted file mode 100644 index 15b66e16d..000000000 --- a/src/app/applications/page.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import { redirect } from "next/navigation"; - -type ApplicationsPageProps = { - searchParams?: Promise>; -}; - -function forwardedSearchParams(params: Record) { - const forwarded = new URLSearchParams(); - for (const [key, value] of Object.entries(params)) { - for (const item of Array.isArray(value) ? value : [value]) { - if (item !== undefined) forwarded.append(key, item); - } - } - return forwarded.toString(); -} - -// "Tools" is the canonical name and /tools the canonical route (PT-11); this -// legacy route only forwards old links and browser history. -export default async function ApplicationsRedirect({ searchParams }: ApplicationsPageProps) { - const params = searchParams ? await searchParams : {}; - const query = forwardedSearchParams(params); - redirect(query ? `/tools?${query}` : "/tools"); -} diff --git a/src/app/applications/route.ts b/src/app/applications/route.ts new file mode 100644 index 000000000..0f19d2c8c --- /dev/null +++ b/src/app/applications/route.ts @@ -0,0 +1,11 @@ +import { type NextRequest, NextResponse } from "next/server"; + +// "Tools" is the canonical name and /tools the canonical route (PT-11). A +// route handler redirects the incoming legacy request before React renders it. +export function GET(request: NextRequest) { + const destination = request.nextUrl.clone(); + destination.pathname = "/tools"; + return NextResponse.redirect(destination); +} + +export const HEAD = GET; diff --git a/src/app/differentials/presentations/page.tsx b/src/app/differentials/presentations/page.tsx deleted file mode 100644 index f69101827..000000000 --- a/src/app/differentials/presentations/page.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { redirect } from "next/navigation"; - -import { getPresentationWorkflowSelectionForDiagnosisIds } from "@/lib/differentials"; - -type DifferentialPresentationsRouteProps = { - searchParams?: Promise<{ query?: string | string[]; q?: string | string[]; ids?: string | string[] }>; -}; - -function firstSearchParam(value?: string | string[]) { - return Array.isArray(value) ? value[0] : value; -} - -export default async function DifferentialPresentationsRoute({ searchParams }: DifferentialPresentationsRouteProps) { - const params = searchParams ? await searchParams : {}; - const query = firstSearchParam(params.query ?? params.q)?.trim(); - const ids = firstSearchParam(params.ids)?.trim(); - const selectedIds = (ids ?? "") - .split(",") - .map((id) => id.trim()) - .filter(Boolean); - const selection = getPresentationWorkflowSelectionForDiagnosisIds(selectedIds); - const destinationParams = new URLSearchParams(); - if (query) destinationParams.set("q", query); - if (selection?.diagnosisIds.length) destinationParams.set("ids", selection.diagnosisIds.join(",")); - const suffix = destinationParams.size ? `?${destinationParams.toString()}` : ""; - redirect(`/differentials/presentations/${selection?.workflow.id ?? "acute-confusion-encephalopathy"}${suffix}`); -} diff --git a/src/app/differentials/presentations/route.ts b/src/app/differentials/presentations/route.ts new file mode 100644 index 000000000..001912f0f --- /dev/null +++ b/src/app/differentials/presentations/route.ts @@ -0,0 +1,21 @@ +import { type NextRequest, NextResponse } from "next/server"; + +import { getPresentationWorkflowSelectionForDiagnosisIds } from "@/lib/differentials"; + +export function GET(request: NextRequest) { + const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim(); + const selectedIds = (request.nextUrl.searchParams.get("ids") ?? "") + .split(",") + .map((id) => id.trim()) + .filter(Boolean); + const selection = getPresentationWorkflowSelectionForDiagnosisIds(selectedIds); + const destination = new URL( + `/differentials/presentations/${selection?.workflow.id ?? "acute-confusion-encephalopathy"}`, + request.url, + ); + if (query) destination.searchParams.set("q", query); + if (selection?.diagnosisIds.length) destination.searchParams.set("ids", selection.diagnosisIds.join(",")); + return NextResponse.redirect(destination); +} + +export const HEAD = GET; diff --git a/src/app/forms/[slug]/page.tsx b/src/app/forms/[slug]/page.tsx index 1f30542e4..3e8ddd973 100644 --- a/src/app/forms/[slug]/page.tsx +++ b/src/app/forms/[slug]/page.tsx @@ -1,15 +1,21 @@ import type { Metadata } from "next"; import { FormDetailClient } from "@/components/forms/form-detail-client"; +import { getFormRecord } from "@/lib/forms"; type FormRouteProps = { params: Promise<{ slug: string }>; }; -export const metadata: Metadata = { - title: "Form record - Forms - Clinical KB", - description: "Psychiatry form and workflow details.", -}; +export async function generateMetadata({ params }: FormRouteProps): Promise { + const { slug } = await params; + const form = getFormRecord(slug); + + return { + title: form ? `${form.title} - Forms - Clinical KB` : "Form record - Forms - Clinical KB", + description: form?.subtitle ?? "Psychiatry form and workflow details.", + }; +} export default async function FormRoute({ params }: FormRouteProps) { const { slug } = await params; diff --git a/src/app/globals.css b/src/app/globals.css index a9fb9afe6..a9cc33e69 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -1442,6 +1442,21 @@ summary::-webkit-details-marker { color: var(--clinical-accent); } +/* Touch devices: suggestion/follow-up chips are primary tap targets (the + follow-ups sit directly under the composer and each fires a new search). + Meet the 44px tap floor and widen the gap so neighbouring chips in the + horizontal scroll row don't invite mis-taps. Fine-pointer (mouse) users keep + the compact 32px chip — no desktop visual change. */ +@media (pointer: coarse) { + .answer-suggestion-chips { + gap: 0.5rem; + } + + .answer-suggestion-chip { + min-height: var(--spacing-tap); + } +} + .answer-suggestion-chip:disabled { cursor: not-allowed; } @@ -1511,8 +1526,14 @@ summary::-webkit-details-marker { padding-inline: 0.25rem; } +/* Phone answer dock: the "Try next" chip row sits directly above the pill with + no flex-gap owner (showFooterSearchChips is false on phones), so this margin + is the sole separator. It must stay positive — a negative value tucked the + chips' rounded underside beneath the pill's top edge/focus glow, reading as a + collision. Keep the gap just large enough to read as intentional without + pushing the row up into the content that shows through the scrim above. */ .answer-footer-search-edge .answer-suggestion-row-composer-followups { - margin-bottom: -0.125rem; + margin-bottom: 0.4375rem; } @media (max-width: 639px) { diff --git a/src/app/layout.tsx b/src/app/layout.tsx index 61fa12ce0..3625399d3 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -1,8 +1,8 @@ import type { Metadata, Viewport } from "next"; import { Geist, Geist_Mono } from "next/font/google"; import { headers } from "next/headers"; -import { PwaLifecycle } from "@/components/pwa-lifecycle"; import { AuthProvider } from "@/lib/supabase/client"; +import { PwaLifecycle } from "@/components/pwa-lifecycle"; import { WebVitalsReporter } from "@/components/web-vitals-reporter"; import { resolveMetadataBase } from "@/lib/metadata-base"; import { APP_THEME_COLORS } from "@/lib/theme"; diff --git a/src/app/medications/page.tsx b/src/app/medications/page.tsx deleted file mode 100644 index 64432c1b9..000000000 --- a/src/app/medications/page.tsx +++ /dev/null @@ -1,5 +0,0 @@ -import { redirect } from "next/navigation"; - -export default function MedicationsIndexPage() { - redirect("/?mode=prescribing"); -} diff --git a/src/app/medications/route.ts b/src/app/medications/route.ts new file mode 100644 index 000000000..c7ad52d16 --- /dev/null +++ b/src/app/medications/route.ts @@ -0,0 +1,7 @@ +import { type NextRequest, NextResponse } from "next/server"; + +export function GET(request: NextRequest) { + return NextResponse.redirect(new URL("/?mode=prescribing", request.url)); +} + +export const HEAD = GET; diff --git a/src/app/services/[slug]/page.tsx b/src/app/services/[slug]/page.tsx index 9f934c0b9..4c1986d41 100644 --- a/src/app/services/[slug]/page.tsx +++ b/src/app/services/[slug]/page.tsx @@ -1,15 +1,21 @@ import type { Metadata } from "next"; import { ServiceDetailClient } from "@/components/services/service-detail-client"; +import { getServiceRecord } from "@/lib/services"; type ServiceRouteProps = { params: Promise<{ slug: string }>; }; -export const metadata: Metadata = { - title: "Service record - Services - Clinical KB", - description: "Clinical service record details and referral information.", -}; +export async function generateMetadata({ params }: ServiceRouteProps): Promise { + const { slug } = await params; + const service = getServiceRecord(slug); + + return { + title: service ? `${service.title} - Services - Clinical KB` : "Service record - Services - Clinical KB", + description: service?.subtitle ?? "Clinical service record details and referral information.", + }; +} export default async function ServiceRoute({ params }: ServiceRouteProps) { const { slug } = await params; diff --git a/src/app/therapy-compass/page.tsx b/src/app/therapy-compass/page.tsx index 45e31b924..360c4e753 100644 --- a/src/app/therapy-compass/page.tsx +++ b/src/app/therapy-compass/page.tsx @@ -3,7 +3,7 @@ import type { Metadata } from "next"; import { TherapyCompassPage } from "@/components/therapy-compass"; export const metadata: Metadata = { - title: "Therapy Compass - Clinical KB", + title: "Therapy - Clinical KB", description: "Source-grounded therapy decision support: search, compare, recommend, pathways, brief interventions and patient sheets.", }; diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index 322216daf..a4a636c71 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -376,7 +376,7 @@ function PriorAnswerTurnSurface({ type="button" onClick={onToggleCollapsed} aria-expanded={!collapsed} - className="inline-flex min-h-9 items-center gap-1.5 rounded-md px-1 text-xs font-semibold text-[color:var(--text-muted)] transition hover:text-[color:var(--text-heading)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]" + className="inline-flex min-h-tap items-center gap-1.5 rounded-md px-1 text-xs font-semibold text-[color:var(--text-muted)] transition hover:text-[color:var(--text-heading)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]" >