Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Original file line number Diff line number Diff line change
@@ -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;
18 changes: 18 additions & 0 deletions tests/retrieval-access-scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(");
Comment on lines +126 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Assert the complete ACL contract.

These prefix checks still pass if authenticated is omitted from REVOKE, or if the service_role grants are removed. Assert each full revoke clause and both GRANT ... TO service_role statements.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/retrieval-access-scope.test.ts` around lines 126 - 129, Update the ACL
assertions in the boundedRetrievalMigration test to verify each complete REVOKE
clause, including the authenticated role, rather than using prefix-only checks.
Also assert both full GRANT statements targeting service_role, preserving the
exact function signatures and privileges defined by the migration.

});

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<string, unknown>) => {
Expand Down
Loading