diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 4a82c5bc..8606d621 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -583,3 +583,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-17 | codex/performance-latency-remediation-20260717 | f277d13512e85dcaae4f9b94d028a8087e41375a | end-to-end database, network, middleware, client rendering, migration, privacy, and merge-readiness review | Fixed all confirmed performance findings plus three final review blockers: title-vocabulary backfill now installs its privacy trigger first, canonical schema replay preserves hardened trigger-function ACLs, and medication auto-seeding no longer aborts its own owner-cache flight. The isolated worktree also uses Next's supported Webpack fallback only when shared dependencies resolve outside the project. No remaining high-confidence P0-P2 defect was found in the reviewed diff. | Docker scratch replay and drift-manifest regeneration passed; live read-only plans selected lexical, table-fact trigram, and chunk HNSW indexes; exact TypeScript passed; focused Vitest owner-cache 9/9, runner safety 12/12, bundle budget 10/10, offline RAG 291/291, and earlier aggregate unit run 2,657 passed with its sole corrected schema assertion subsequently validated by direct ordering/ACL checks; production build and bundle budget passed at 1,363,382 gzip bytes; targeted Chromium dashboard deferral and NDJSON search passed, while the viewer trace reached all hydration/preview assertions and its corrected download-control selector remains for exact-head hosted UI. No OpenAI calls, live Supabase writes/migrations, deployment, or production mutation ran. | | 2026-07-17 | codex/mobile-search-phone-refresh-20260717 (supersedes PR #700) | 42a3e3ce65dc5a0e1dce386e0b91fccd23d13d6c + reviewed follow-up diff | phone universal-search command-panel recovery and merge-readiness review | Recovered the still-useful behavior from PR #700 onto current `main`, including its hydration fix and wide-touch regression coverage. Hosted Production UI then exposed one desktop focus race: capability state intentionally initializes false for hydration safety, but an input could receive focus before the post-hydration effect synchronized the real browser state. The follow-up recomputes the same guarded predicate synchronously on focus; it requires the placement breakpoint plus either a fine pointer or a zero-touch desktop fallback, so wide touch devices remain suppressed while desktop keeps the first command-panel interaction. No remaining high-confidence P0-P2 defect was found in the scoped diff. | Focused Vitest 7/7; `npm run ensure` verified the project at `http://localhost:3751`; hosted static, safety, coverage, build, advisory UI, Semgrep, Gitleaks, and GitGuardian passed; the first hosted Production UI run isolated the nine desktop regressions. The focused browser proof reproduced the desktop race while the wide-touch regression passed, and exact-head hosted Production UI remains required after the focus fix. `format:changed -- --check` and `git diff --check` passed before the final follow-up. No Supabase/OpenAI/product-provider command ran. | | 2026-07-17 | final historical branch/worktree cleanup against `origin/main` | 5d195d7ca8752b2ae4006725c6b145c5662bb687 | branch-cleanup | Merged PRs #716 and #717, closed superseded PR #700, and removed the three clean task worktrees. Deleted exact remote refs for `codex/mobile-search-phone-fix-20260717` (`590f32b73`), `claude/audit-findings-review-phgz92` (`ea3b8f95b8`), `codex/chat-forms-import-6914` (`b05da82f82`), `codex/chat-supabase-migration-preflight-b463` (`1ef0faee95`), and `codex/dsm-diagnosis-mode` (`f6cda83ca6`); the merged #716/#717 branches were deleted automatically. Deleted 14 unregistered local refs only after direct-main ancestry, exact ledger deletion-pending proof, or exact merged-PR commit provenance. Final inventory found zero remote branches without an open PR or registered worktree, zero locally merged or exact deletion-pending orphan refs, and no retired target refs or paths. Thirteen non-ancestor local refs and 25 registered worktrees remain preserved because they are backups, patch-unique/unresolved, open-PR-owned, or ownership could not be safely disproved. | Fresh fetch/prune; exact GitHub PR/head/merge associations; cherry-pick-aware logs; DSM PR #661 exact commit/file provenance; exact leased remote deletes; exact-old-value local `update-ref` deletes; clean-worktree and path/process checks; worktree prune; final zero-orphan inventory. The Codex task registry lookup timed out, so ambiguous registered worktrees were conservatively retained. No Supabase, OpenAI, production-data, or live clinical workflow ran. | +| 2026-07-17 | codex/supabase-owner-rpc-hardening | 5aa571504ac3f20069e2d61a9b96bf99aa5bcd2e | database/retrieval migration review | Fixed a P2 bounded-resource defect: explicitly null or negative `match_count` reaches PostgreSQL `LIMIT` unbounded semantics in the two versioned retrieval RPC wrappers. The forward migration clamps returned rows to 1–96 while preserving owner-plus-public filtering, signatures, defaults, and service-role-only execute grants. No further high-confidence P0–P2 issue was found in the changed scope. | Static migration-contract audit and `git diff --check` passed; `check:function-grants` passed. Full Vitest and aggregate verification could not start because this worktree lacks `tsx`/Vitest dependencies and runs Node 20 while the repository requires Node 24. No Docker replay, live Supabase, hosted CI, or provider-backed checks ran. | diff --git a/supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql b/supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql new file mode 100644 index 00000000..6eba59a4 --- /dev/null +++ b/supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql @@ -0,0 +1,86 @@ +-- Bound the result cardinality of the two versioned owner-plus-public retrieval +-- entry points. PostgREST permits explicitly-null RPC arguments; PostgreSQL +-- interprets LIMIT NULL (and a negative LIMIT) as no limit. Before this guard, +-- a malformed or compromised service-role caller could turn the final LIMIT +-- into an unbounded result within each helper's candidate window, multiplying +-- JSON hydration and response size on a hot retrieval path. +-- +-- Keep the public signatures and defaults intact for generated types and +-- callers. The inner helpers already cap candidate work; this outer clamp +-- makes the exposed result contract deterministic: 1..96 rows. +-- +-- Rollback / forward repair: re-create these wrappers from +-- 20260717160000_optimize_owner_public_retrieval.sql to restore the former +-- pass-through count behaviour. No data is changed. + +create or replace function public.match_document_chunks_text_v2( + query_text text, + match_count integer default 12, + document_filters uuid[] default null, + owner_filter uuid default '00000000-0000-0000-0000-000000000000'::uuid, + include_public boolean default true +) +returns table ( + id uuid, document_id uuid, title text, file_name text, page_number integer, + chunk_index integer, section_heading text, content text, retrieval_synopsis text, + image_ids uuid[], source_metadata jsonb, document_labels jsonb, document_summary text, + similarity double precision, text_rank double precision, hybrid_score double precision, + lexical_score double precision, images jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + select * + from public.match_document_chunks_text_scoped( + $1, + least(greatest(coalesce($2, 12), 1), 96), + $3, + $4, + $5 + ); +$$; + +create or replace function public.match_document_index_units_hybrid_v2( + query_embedding extensions.vector(1536), + query_text text, + match_count integer default 24, + min_similarity double precision default 0.1, + document_filters uuid[] default null, + owner_filter uuid default '00000000-0000-0000-0000-000000000000'::uuid, + include_public boolean default true +) +returns table ( + id uuid, document_id uuid, source_chunk_id uuid, source_image_id uuid, unit_type text, title text, + content text, page_start integer, page_end integer, heading_path text[], normalized_terms text[], + source_span jsonb, quality_score real, extraction_mode text, similarity double precision, + text_rank double precision, hybrid_score double precision, metadata jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + select * + from public.match_document_index_units_hybrid_scoped( + $1, + $2, + least(greatest(coalesce($3, 24), 1), 96), + $4, + $5, + $6, + $7 + ); +$$; + +-- CREATE OR REPLACE preserves privileges in PostgreSQL, but re-state the +-- service-role-only contract so future function changes cannot reopen it. +revoke all on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean) + from public, anon, authenticated; +grant execute on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean) + to service_role; +revoke all on function public.match_document_index_units_hybrid_v2( + extensions.vector, text, integer, double precision, uuid[], uuid, boolean +) from public, anon, authenticated; +grant execute on function public.match_document_index_units_hybrid_v2( + extensions.vector, text, integer, double precision, uuid[], uuid, boolean +) to service_role; diff --git a/tests/retrieval-access-scope.test.ts b/tests/retrieval-access-scope.test.ts index 0e224333..f49acf10 100644 --- a/tests/retrieval-access-scope.test.ts +++ b/tests/retrieval-access-scope.test.ts @@ -8,6 +8,10 @@ const optimizedRetrievalMigration = readFileSync( join(root, "supabase/migrations/20260717160000_optimize_owner_public_retrieval.sql"), "utf8", ); +const boundedRetrievalMigration = readFileSync( + join(root, "supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql"), + "utf8", +); const canonicalSchema = readFileSync(join(root, "supabase/schema.sql"), "utf8"); function between(source: string, start: string, end: string): string { @@ -111,6 +115,20 @@ describe("owner-plus-public retrieval contract", () => { } }); + it("bounds nullable and negative result counts at the exposed versioned RPCs", () => { + // LIMIT NULL and LIMIT -1 mean "no limit" in PostgreSQL. The wrappers are + // the RPC boundary, so clamp here rather than trusting every service-role + // caller to validate an optional PostgREST argument. + expect(boundedRetrievalMigration).toContain("coalesce($2, 12)"); + expect(boundedRetrievalMigration).toContain("coalesce($3, 24)"); + expect(boundedRetrievalMigration).toContain("least(greatest(coalesce($2, 12), 1), 96)"); + expect(boundedRetrievalMigration).toContain("least(greatest(coalesce($3, 24), 1), 96)"); + expect(boundedRetrievalMigration).toContain( + "revoke all on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean)", + ); + expect(boundedRetrievalMigration).toContain("revoke all on function public.match_document_index_units_hybrid_v2("); + }); + it("merges exact-owner and public rows when a versioned RPC is not deployed yet", async () => { const { callVersionedRetrievalRpc } = await import("../src/lib/rag"); const rpc = vi.fn(async (name: string, args: Record) => {