Skip to content
Merged
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
7 changes: 4 additions & 3 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD

## Review Records

| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks |
| ---------- | -------------- | ------------- | -------------- | ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- |
| 2026-07-09 | example/branch | abc1234 | branch-cleanup | Example: already merged into `main`; no unique patch content. | `git log --right-only --cherry-pick main...example/branch`; `git diff --name-status main...example/branch` |
| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks |
| ---------- | --------------------------------------- | ---------------------------------------- | -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 2026-07-09 | example/branch | abc1234 | branch-cleanup | Example: already merged into `main`; no unique patch content. | `git log --right-only --cherry-pick main...example/branch`; `git diff --name-status main...example/branch` |
| 2026-07-11 | PR #481 / claude/perf-r2-auth-roundtrip | a21a2a9690e3a5c64d672255052c7fbf9b20f950 | open-PR review and unresolved comments | Two P2 findings fixed: cookie-authenticated API requests again return rotated SSR cookies, and route-boundary payload trimming preserves full source content required by client safety scanning while still removing server-only fields. Added focused regression coverage. No additional high-confidence defect was found in the six-file diff. | Focused proxy, payload, and clinical-safety Vitest (17/17); TypeScript; focused Prettier; `git diff --check`. Production readiness ran fail-closed with provider variables cleared and reported only expected missing provider configuration. |
5 changes: 4 additions & 1 deletion src/app/api/answer/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
sourceGovernanceWarnings,
} from "@/lib/source-governance";
import { parseJsonBody } from "@/lib/validation/body";
import { toClientAnswerPayload } from "@/lib/answer-client-payload";
import { answerServerTimingEntries, buildServerTimingHeader } from "@/lib/server-timing";
import { createAdminClient } from "@/lib/supabase/admin";
import { logAnswerDiagnostics } from "@/lib/answer-telemetry";
Expand Down Expand Up @@ -166,7 +167,9 @@ export async function POST(request: Request) {
);
return NextResponse.json(
{
...answer,
// Boundary trim only — governance warnings and diagnostics above
// consumed the full answer (see answer-client-payload.ts).
...toClientAnswerPayload(answer),
degradedMode: answerDegradedModeSignal(answer),
scope: { ...scope, queryMode: answerBody.queryMode },
sourceGovernanceWarnings: warnings,
Expand Down
5 changes: 4 additions & 1 deletion src/app/api/answer/stream/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type ApiRateLimitResult,
} from "@/lib/api-rate-limit";
import { publicAccessContext } from "@/lib/public-api-access";
import { toClientAnswerPayload } from "@/lib/answer-client-payload";
import { answerQuestionWithScope, type AnswerProgressEvent } from "@/lib/rag";
import { classifyRagQuery } from "@/lib/clinical-search";
import { annotateSearchResults, buildEvidenceRelevance } from "@/lib/evidence-relevance";
Expand Down Expand Up @@ -231,7 +232,9 @@ function streamAnswer(
}

send("final", {
...answer,
// Boundary trim only — governance warnings and diagnostics above
// consumed the full answer (see answer-client-payload.ts).
...toClientAnswerPayload(answer),
degradedMode: answerDegradedModeSignal(answer),
scope: scope ? { ...scope, queryMode: body.queryMode } : undefined,
sourceGovernanceWarnings: warnings,
Expand Down
33 changes: 33 additions & 0 deletions src/lib/answer-client-payload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type { RagAnswer, SearchResult } from "@/lib/types";

// Route-boundary trim of the answer payload. The retrieval pipeline carries
// full chunk text plus server-only context on every source (adjacent_context
// for generation packing, memory cards, table facts, index-unit matches,
// document summaries), but the client renders only a snippet
// (retrieval_synopsis ?? content) plus identity, scoring, governance, and
// label fields. Trimming at the route boundary — never inside rag.ts — keeps
// caches, generation inputs, and eval behavior byte-identical while cutting
// the final SSE/JSON event the user waits on after the prose has streamed.
//
// Ordering contract: sourceGovernanceWarnings and logAnswerDiagnostics consume
// the FULL answer and must run before this trim (both routes do). Full source
// content remains client-visible because the safety panel scans it for clinical
// warnings that may occur beyond the display synopsis.

function trimSourceForClient(source: SearchResult): SearchResult {
const trimmed: SearchResult = {
...source,
content: source.content ?? "",
};
delete trimmed.adjacent_context;
delete trimmed.memory_cards;
delete trimmed.table_facts;
delete trimmed.index_unit;
delete trimmed.document_summary;
return trimmed;
}

export function toClientAnswerPayload<T extends Pick<RagAnswer, "sources">>(answer: T): T {
if (!answer.sources?.length) return answer;
return { ...answer, sources: answer.sources.map(trimSourceForClient) };
}
13 changes: 8 additions & 5 deletions src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import { buildContentSecurityPolicy, resolveRuntimeFlags } from "@/lib/security-
// opts pages into dynamic rendering (app/layout.tsx reads the nonce), which
// is inherent to nonce-based CSP.
// 2. Session refresh. Keep the user's @supabase/ssr session cookie fresh on
// navigation and API calls so persistent logins survive refreshes. It is a
// no-op unless the public Supabase env is configured AND an `sb-` auth
// cookie is present, so demo / local-no-auth traffic is untouched.
// page navigations so persistent logins survive refreshes. It is a no-op
// unless the public Supabase env is configured AND an `sb-` auth cookie is
// present, so demo / local-no-auth traffic is untouched. Cookie-authenticated
// API requests still pass through this refresh path because route handlers
// cannot write rotated SSR cookies back to the browser themselves.

const documentFlowRedirects: Record<string, string> = {
"/mockups/document-search-command": "/documents/search",
Expand Down Expand Up @@ -96,7 +98,8 @@ export async function proxy(request: NextRequest) {
}

export const config = {
// Run on everything except static assets and image files. API routes are
// intentionally included so cookie-based sessions refresh for them too.
// Run on everything except static assets and image files. API routes stay in
// the matcher so cookie-authenticated requests can return rotated cookies and
// every response carries the CSP header.
matcher: ["/((?!_next/static|_next/image|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp|ico)$).*)"],
};
92 changes: 92 additions & 0 deletions tests/answer-client-payload.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { describe, expect, it } from "vitest";

import { toClientAnswerPayload } from "@/lib/answer-client-payload";
import { extractSafetyFindings } from "@/lib/clinical-safety";
import type { RagAnswer, SearchResult } from "@/lib/types";

function fullSource(overrides: Partial<SearchResult> = {}): SearchResult {
return {
id: "chunk-1",
document_id: "doc-1",
title: "Clozapine monitoring guideline",
file_name: "clozapine.pdf",
page_number: 4,
chunk_index: 7,
section_heading: "Monitoring",
content: "Full blood count weekly for 18 weeks. ".repeat(60),
retrieval_synopsis: "FBC weekly for 18 weeks, then monthly.",
image_ids: [],
similarity: 0.82,
source_metadata: { document_status: "current" } as SearchResult["source_metadata"],
adjacent_context: "Preceding paragraph context. ".repeat(20),
document_summary: "A long document summary. ".repeat(30),
memory_cards: [{ id: "m1" } as never],
table_facts: [{ id: "t1" } as never],
index_unit: { unit_type: "table" } as never,
...overrides,
} as SearchResult;
}

function answerWith(sources: SearchResult[]): Pick<RagAnswer, "sources"> {
return { sources };
}

describe("toClientAnswerPayload", () => {
it("drops server-only per-source fields the client never renders", () => {
const trimmed = toClientAnswerPayload(answerWith([fullSource()])).sources![0];
expect(trimmed.adjacent_context).toBeUndefined();
expect(trimmed.memory_cards).toBeUndefined();
expect(trimmed.table_facts).toBeUndefined();
expect(trimmed.index_unit).toBeUndefined();
expect(trimmed.document_summary).toBeUndefined();
});

it("keeps identity, snippet, scoring, and governance fields intact", () => {
const trimmed = toClientAnswerPayload(answerWith([fullSource()])).sources![0];
expect(trimmed.id).toBe("chunk-1");
expect(trimmed.title).toBe("Clozapine monitoring guideline");
expect(trimmed.retrieval_synopsis).toBe("FBC weekly for 18 weeks, then monthly.");
expect(trimmed.similarity).toBe(0.82);
expect(trimmed.source_metadata).toEqual({ document_status: "current" });
expect(trimmed.page_number).toBe(4);
});

it("preserves full source content so client-side safety scanning cannot miss later warnings", () => {
const source = fullSource({ content: `${"Routine context. ".repeat(60)}Contraindicated in severe disease.` });
const payload = toClientAnswerPayload({
answer: "Review the source.",
grounded: true,
confidence: "medium",
citations: [],
sources: [source],
} as RagAnswer);

expect(payload.sources![0].content).toBe(source.content);
expect(extractSafetyFindings(payload)).toHaveLength(1);
});

it("leaves short content untouched", () => {
const short = fullSource({ content: "Short snippet." });
expect(toClientAnswerPayload(answerWith([short])).sources![0].content).toBe("Short snippet.");
});

it("does not mutate the original answer (caches keep the full sources)", () => {
const source = fullSource();
const answer = answerWith([source]);
toClientAnswerPayload(answer);
expect(answer.sources![0].adjacent_context).toBeTruthy();
expect(answer.sources![0].content.length).toBeGreaterThan(700);
});

it("passes through answers without sources", () => {
const empty = answerWith([]);
expect(toClientAnswerPayload(empty)).toBe(empty);
});

it("materially shrinks a representative payload", () => {
const answer = answerWith(Array.from({ length: 8 }, (_, index) => fullSource({ id: `chunk-${index}` })));
const fullBytes = JSON.stringify(answer).length;
const trimmedBytes = JSON.stringify(toClientAnswerPayload(answer)).length;
expect(trimmedBytes).toBeLessThan(fullBytes * 0.8);
});
});
78 changes: 78 additions & 0 deletions tests/proxy-session-refresh.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { NextRequest } from "next/server";

// The proxy's second job (session-cookie refresh via supabase.auth.getUser)
// must run on page navigations and cookie-authenticated API routes. Route
// handlers can validate an SSR cookie, but their read-only adapter cannot return
// rotated Set-Cookie headers to the browser.

const getUser = vi.fn(async () => ({ data: { user: null }, error: null }));

vi.mock("@supabase/ssr", async (importOriginal) => {
const actual = await importOriginal<typeof import("@supabase/ssr")>();
return {
...actual,
createServerClient: vi.fn((_url, _key, options: { cookies: { setAll: (cookies: never[]) => void } }) => ({
auth: {
getUser: async () => {
options.cookies.setAll([
{
name: "sb-unit-test-auth-token",
value: "rotated-session",
options: { path: "/", httpOnly: true },
},
] as never[]);
return getUser();
},
},
})),
};
});

vi.mock("@/lib/env", async (importOriginal) => {
const actual = await importOriginal<typeof import("../src/lib/env")>();
return {
...actual,
env: {
...actual.env,
NEXT_PUBLIC_SUPABASE_URL: "https://unit-test.supabase.co",
NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY: "sb_publishable_unit_test",
},
};
});

function requestWithSessionCookie(path: string): NextRequest {
const request = new NextRequest(new URL(`http://localhost${path}`));
request.cookies.set("sb-unit-test-auth-token", "base64-opaque-session");
return request;
}

beforeEach(() => {
getUser.mockClear();
});

describe("proxy session refresh scoping", () => {
it("refreshes SSR cookies for cookie-authenticated API routes and stamps the CSP", async () => {
const { proxy } = await import("../src/proxy");
const response = await proxy(requestWithSessionCookie("/api/answer"));

expect(getUser).toHaveBeenCalledTimes(1);
expect(response.cookies.get("sb-unit-test-auth-token")?.value).toBe("rotated-session");
expect(response.headers.get("content-security-policy")).toBeTruthy();
});

it("refreshes the session on page navigations that carry an sb- cookie", async () => {
const { proxy } = await import("../src/proxy");
const response = await proxy(requestWithSessionCookie("/documents/some-id"));

expect(getUser).toHaveBeenCalledTimes(1);
expect(response.headers.get("content-security-policy")).toBeTruthy();
});

it("never calls getUser without an sb- cookie", async () => {
const { proxy } = await import("../src/proxy");
await proxy(new NextRequest(new URL("http://localhost/documents/some-id")));

expect(getUser).not.toHaveBeenCalled();
});
});