diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 2c0cf6fc7..9e3201b0e 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -23729,7 +23729,7 @@ paths: "422": *a4 "429": *a5 /api/search: - get: + post: tags: - Records summary: Search the record @@ -23738,30 +23738,31 @@ paths: notes on mood entries, readings and illness days. Hits are grouped by kind, the best group first, and each carries the in-app path that opens the record. Matching ignores case and accents and reads German umlauts typed as ae, oe, ue or ss. A source whose module the record has switched off is not read. The search reads each source - newest first under a per-source row cap and a time budget; `truncated` says when that cut it short. The query is - never logged. Rate limited per acting account (60 a minute). A grant scoped to some sections of the record does - not open it, like the timeline." - parameters: - - in: query - name: q - schema: - type: string - minLength: 2 - maxLength: 100 - description: What to look for. Matched without regard to case or accents, and German umlauts may be typed as ae, oe, ue - or ss. - required: true - description: What to look for. Matched without regard to case or accents, and German umlauts may be typed as ae, oe, ue - or ss. - - in: query - name: limit - schema: - default: 5 - description: Most hits per group. - type: integer - minimum: 1 - maximum: 20 - description: Most hits per group. + newest first under a per-source row cap and a time budget; `truncated` says when that cut it short. The query + travels in the JSON body, never in the URL, and is never logged. Rate limited per acting account (20 a minute), + and one search at a time: a newer search by the same account stops the one before it, which then answers 409 + `search.superseded`. A grant scoped to some sections of the record does not open it, like the timeline." + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + q: + type: string + minLength: 2 + maxLength: 100 + description: What to look for. Matched without regard to case or accents, and German umlauts may be typed as ae, oe, ue + or ss. + limit: + default: 5 + description: Most hits per group. + type: integer + minimum: 1 + maximum: 20 + required: + - q responses: "200": description: The hits, grouped by kind. diff --git a/e2e/command-palette.spec.ts b/e2e/command-palette.spec.ts index b6ba08930..cae66b948 100644 --- a/e2e/command-palette.spec.ts +++ b/e2e/command-palette.spec.ts @@ -31,11 +31,15 @@ test.use({ storageState: STORAGE_STATE_PATH }); const palette = (page: Page) => page.locator('[data-slot="command-palette"]'); -/** Answer `GET /api/search` with one visit hit, and count the requests. */ +/** Answer `POST /api/search` with one visit hit, and count the requests. */ async function mockRecordSearch(page: Page): Promise { const queries: string[] = []; - await page.route("**/api/search?*", async (route) => { - queries.push(new URL(route.request().url()).searchParams.get("q") ?? ""); + await page.route("**/api/search", async (route) => { + // The query rides the JSON body, never the URL. + expect(route.request().method()).toBe("POST"); + expect(route.request().url()).not.toContain("?"); + const body = route.request().postDataJSON() as { q?: string } | null; + queries.push(body?.q ?? ""); await route.fulfill({ status: 200, contentType: "application/json", diff --git a/src/__tests__/proxy-demo-mode-allowlist.test.ts b/src/__tests__/proxy-demo-mode-allowlist.test.ts index 272865a8e..697cba90e 100644 --- a/src/__tests__/proxy-demo-mode-allowlist.test.ts +++ b/src/__tests__/proxy-demo-mode-allowlist.test.ts @@ -154,6 +154,13 @@ describe("proxy.ts DEMO_MODE mutation allowlist", () => { ).toBe(403); }); + it("permits the record search, a read that travels as a POST", () => { + // The query rides the body so it stays out of URLs and access logs; the + // route writes nothing, so the demo searches like any other instance. + expect(proxy(makeRequest("/api/search", "POST")).status).not.toBe(403); + expect(proxy(makeRequest("/api/search", "DELETE")).status).toBe(403); + }); + it("still blocks a health-data mutation (POST /api/measurements)", () => { const res = proxy(makeRequest("/api/measurements", "POST")); expect(res.status).toBe(403); diff --git a/src/app/api/search/__tests__/route.test.ts b/src/app/api/search/__tests__/route.test.ts index 82e3ae25f..5f59997e4 100644 --- a/src/app/api/search/__tests__/route.test.ts +++ b/src/app/api/search/__tests__/route.test.ts @@ -1,6 +1,6 @@ /** - * `GET /api/search` — admission, validation, the rate limit, and what the - * route hands the search and the log. + * `POST /api/search` — admission, validation, the rate limit, one search in + * flight per actor, and what the route hands the search and the log. * * The search itself is `searchRecords`, covered by its own unit tests and by * `tests/integration/record-search.test.ts` against Postgres; here it is a @@ -49,12 +49,27 @@ vi.mock("@/lib/search/search-records", () => ({ searchRecords: mocks.searchRecords, })); -import { GET } from "../route"; +import { POST } from "../route"; +import { searchesInFlight } from "@/lib/search/in-flight"; -const handler = GET as unknown as (req: NextRequest) => Promise; +const handler = POST as unknown as (req: NextRequest) => Promise; -function request(query: string): NextRequest { - return new NextRequest(`http://localhost/api/search?${query}`); +/** + * A search request. Written as the query string the route used to take, so + * every case reads as before; the route receives it as a JSON body. + */ +function request(query: string, init: { signal?: AbortSignal } = {}) { + const params = new URLSearchParams(query); + const body: Record = {}; + for (const [key, value] of params) { + body[key] = key === "limit" && /^\d+$/.test(value) ? Number(value) : value; + } + return new NextRequest("http://localhost/api/search", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + signal: init.signal, + }); } const OWNER = { id: "owner-1", locale: "de" }; @@ -93,7 +108,7 @@ beforeEach(() => { }); }); -describe("GET /api/search", () => { +describe("POST /api/search", () => { it("admits the whole record only, at read", async () => { await handler(request("q=schulter")); expect(mocks.requireRecordAuth).toHaveBeenCalledWith("read", "record"); @@ -137,6 +152,7 @@ describe("GET /api/search", () => { ["a zero limit", "q=knie&limit=0"], ["a limit above the cap", "q=knie&limit=21"], ["a non-numeric limit", "q=knie&limit=lots"], + ["a fractional limit", "q=knie&limit=2.5"], ])("refuses %s with 422 and never searches", async (_label, query) => { const res = await handler(request(query)); expect(res.status).toBe(422); @@ -173,4 +189,108 @@ describe("GET /api/search", () => { delegated: true, }); }); + + it("keeps the query out of the URL", async () => { + const req = request("q=Psychotherapie"); + expect(req.url).not.toMatch(/psychotherapie/i); + const res = await handler(req); + expect(res.status).toBe(200); + expect(mocks.searchRecords.mock.calls[0][0].query).toBe("Psychotherapie"); + }); + + it("refuses a body that is not JSON", async () => { + const res = await handler( + new NextRequest("http://localhost/api/search", { + method: "POST", + headers: { "Content-Type": "text/plain" }, + body: "q=schulter", + }), + ); + expect(res.status).toBe(415); + expect(mocks.searchRecords).not.toHaveBeenCalled(); + }); + + it("allows 20 searches a minute per actor", async () => { + await handler(request("q=schulter")); + expect(mocks.checkRateLimit).toHaveBeenCalledWith( + "search:actor-1", + 20, + 60_000, + ); + }); + + it("hands the search an abort signal that follows the client", async () => { + const client = new AbortController(); + let seen: AbortSignal | undefined; + mocks.searchRecords.mockImplementationOnce(async (args) => { + seen = args.signal; + client.abort(); + return { + groups: [], + truncated: true, + stats: { sources: 1, rows: 0, candidates: 0, elapsedMs: 1 }, + }; + }); + await handler(request("q=schulter", { signal: client.signal })); + expect(seen?.aborted).toBe(true); + }); + + it("stops an actor's earlier search when a newer one starts", async () => { + let release: () => void = () => {}; + let firstSignal: AbortSignal | undefined; + mocks.searchRecords.mockImplementationOnce( + (args) => + new Promise((resolve) => { + firstSignal = args.signal; + release = () => + resolve({ + groups: [], + truncated: true, + stats: { sources: 1, rows: 0, candidates: 0, elapsedMs: 1 }, + }); + }), + ); + const first = handler(request("q=schul")); + await vi.waitFor(() => expect(firstSignal).toBeDefined()); + expect(firstSignal?.aborted).toBe(false); + + const second = await handler(request("q=schulter")); + expect(second.status).toBe(200); + expect(firstSignal?.aborted).toBe(true); + + release(); + const superseded = await first; + expect(superseded.status).toBe(409); + expect((await superseded.json()).meta?.errorCode).toBe("search.superseded"); + expect(searchesInFlight()).toBe(0); + }); + + it("does not stop another actor's search", async () => { + let firstSignal: AbortSignal | undefined; + let release: () => void = () => {}; + mocks.searchRecords.mockImplementationOnce( + (args) => + new Promise((resolve) => { + firstSignal = args.signal; + release = () => + resolve({ + groups: [], + truncated: false, + stats: { sources: 1, rows: 0, candidates: 0, elapsedMs: 1 }, + }); + }), + ); + const first = handler(request("q=schul")); + await vi.waitFor(() => expect(firstSignal).toBeDefined()); + + mocks.requireRecordAuth.mockResolvedValueOnce({ + user: { id: "owner-2", locale: "de" }, + actor: { id: "actor-2", locale: "de" }, + grantId: null, + }); + await handler(request("q=knie")); + expect(firstSignal?.aborted).toBe(false); + release(); + expect((await first).status).toBe(200); + }); }); diff --git a/src/app/api/search/route.ts b/src/app/api/search/route.ts index 855c9007c..6e79aca2a 100644 --- a/src/app/api/search/route.ts +++ b/src/app/api/search/route.ts @@ -1,5 +1,6 @@ /** - * `GET /api/search?q=&limit=` — one search over the record (Discussion #1117): + * `POST /api/search` with `{ q, limit }` — one search over the record + * (Discussion #1117): * visits and procedures, conditions, documents (title and indexed text), * vaccinations, lab results, medications, symptoms, workouts and the notes on * mood entries, readings and illness days. Each hit carries the in-app path @@ -12,35 +13,49 @@ * switched-off module is not read at all. * * The query is health data ("Schulter", a drug, a diagnosis). It never - * reaches a log: the annotation carries its length and the counts only, and a - * refused query is answered without echoing it. + * reaches a log: it travels in the JSON body rather than the URL, so neither + * this app's request log nor a reverse proxy's access log records it; the + * annotation carries its length and the counts only, and a refused query is + * answered without echoing it. * - * Rate limited per actor: the palette asks once per pause in typing, and a - * manager searching someone's record spends their own allowance. + * Bounded per actor: 20 searches a minute (the palette asks once per pause in + * typing, and a manager searching someone's record spends their own + * allowance), and one at a time. A newer search by the same actor aborts the + * one before it, and a client that goes away aborts its own, so an abandoned + * search stops reading at its next budget check instead of running out its + * time (`in-flight.ts`). */ import { NextRequest } from "next/server"; import { apiHandler, requireRecordAuth } from "@/lib/api-handler"; -import { apiError, apiSuccess, returnAllZodIssues } from "@/lib/api-response"; +import { + apiError, + apiSuccess, + returnAllZodIssues, + safeJson, +} from "@/lib/api-response"; import { prisma } from "@/lib/db"; import { defaultLocale, locales, type Locale } from "@/lib/i18n/config"; import { annotate } from "@/lib/logging/context"; import { resolveModuleMap } from "@/lib/modules/gate"; import { checkRateLimit } from "@/lib/rate-limit"; +import { beginSearch, SUPERSEDED } from "@/lib/search/in-flight"; import { searchRecords } from "@/lib/search/search-records"; import { searchQuerySchema } from "@/lib/search/wire-schemas"; import { actingDomainVisibility } from "@/lib/sharing/acting-domains"; import { resolveUserTimezone } from "@/lib/tz/resolver"; -/** Searches per actor per minute: a few per typed word, with room to spare. */ -const SEARCH_RATE_LIMIT = 60; +/** Searches per actor per minute: one per pause in typing, with room to spare. */ +const SEARCH_RATE_LIMIT = 20; const SEARCH_RATE_WINDOW_MS = 60_000; +/** The body is a query of at most 100 characters and a number. */ +const SEARCH_BODY_MAX_BYTES = 2048; function displayLocale(value: string | null | undefined): Locale { return locales.includes(value as Locale) ? (value as Locale) : defaultLocale; } -export const GET = apiHandler(async (request: NextRequest) => { +export const POST = apiHandler(async (request: NextRequest) => { const { user, actor, grantId } = await requireRecordAuth("read", "record"); const rl = await checkRateLimit( @@ -52,10 +67,14 @@ export const GET = apiHandler(async (request: NextRequest) => { return apiError("Too many searches. Please retry shortly.", 429); } - const params = new URL(request.url).searchParams; + const { data: body, error: jsonError } = await safeJson<{ + q?: unknown; + limit?: unknown; + }>(request, { maxBytes: SEARCH_BODY_MAX_BYTES }); + if (jsonError) return jsonError; const parsed = searchQuerySchema.safeParse({ - q: params.get("q") ?? undefined, - limit: params.get("limit") ?? undefined, + q: body?.q ?? undefined, + limit: body?.limit ?? undefined, }); if (!parsed.success) { annotate({ @@ -67,35 +86,54 @@ export const GET = apiHandler(async (request: NextRequest) => { }); } - const [modules, domainVisible, timezone] = await Promise.all([ - resolveModuleMap(user.id), - actingDomainVisibility(prisma, grantId), - resolveUserTimezone(user.id), - ]); - const result = await searchRecords({ - recordId: user.id, - query: parsed.data.q, - limit: parsed.data.limit, - modules, - domainVisible, - displayLocale: displayLocale(actor.locale), - timezone, - }); - - annotate({ - action: { name: "search.query.run" }, - meta: { - query_length: parsed.data.q.length, + const slot = beginSearch(actor.id); + try { + const [modules, domainVisible, timezone] = await Promise.all([ + resolveModuleMap(user.id), + actingDomainVisibility(prisma, grantId), + resolveUserTimezone(user.id), + ]); + const result = await searchRecords({ + recordId: user.id, + query: parsed.data.q, limit: parsed.data.limit, - sources: result.stats.sources, - rows_scanned: result.stats.rows, - matches: result.stats.candidates, - groups: result.groups.length, - hits: result.groups.reduce((n, g) => n + g.hits.length, 0), - truncated: result.truncated, - elapsed_ms: result.stats.elapsedMs, - delegated: grantId !== null, - }, - }); - return apiSuccess({ groups: result.groups, truncated: result.truncated }); + modules, + domainVisible, + displayLocale: displayLocale(actor.locale), + timezone, + signal: AbortSignal.any([request.signal, slot.signal]), + }); + + if (slot.signal.aborted) { + // A newer search by the same actor took over; this answer is partial + // and nobody is waiting for it. + annotate({ + action: { name: "search.query.superseded" }, + meta: { query_length: parsed.data.q.length }, + }); + return apiError("A newer search replaced this one", 409, { + errorCode: SUPERSEDED, + }); + } + + annotate({ + action: { name: "search.query.run" }, + meta: { + query_length: parsed.data.q.length, + limit: parsed.data.limit, + sources: result.stats.sources, + rows_scanned: result.stats.rows, + matches: result.stats.candidates, + groups: result.groups.length, + hits: result.groups.reduce((n, g) => n + g.hits.length, 0), + truncated: result.truncated, + elapsed_ms: result.stats.elapsedMs, + delegated: grantId !== null, + aborted: request.signal.aborted, + }, + }); + return apiSuccess({ groups: result.groups, truncated: result.truncated }); + } finally { + slot.end(); + } }); diff --git a/src/components/command-palette/command-palette.tsx b/src/components/command-palette/command-palette.tsx index 04564b64b..14b19727f 100644 --- a/src/components/command-palette/command-palette.tsx +++ b/src/components/command-palette/command-palette.tsx @@ -51,7 +51,7 @@ import { useRecordSearch } from "./use-record-search"; * stays open. * * After the local results comes "In your records": the server's search over - * the record (`useRecordSearch`, `GET /api/search`), once the typing pauses. + * the record (`useRecordSearch`, `POST /api/search`), once the typing pauses. * Its hits are options like any other, after the local ones, so the arrows * and Enter work across both; each opens the page that shows the record. * diff --git a/src/components/command-palette/use-record-search.ts b/src/components/command-palette/use-record-search.ts index 80e436730..9c2b56ccf 100644 --- a/src/components/command-palette/use-record-search.ts +++ b/src/components/command-palette/use-record-search.ts @@ -4,7 +4,7 @@ import { keepPreviousData, useQuery } from "@tanstack/react-query"; import { useDebouncedValue } from "@/hooks/use-debounced-value"; import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; -import { apiGet } from "@/lib/api/api-fetch"; +import { apiPost } from "@/lib/api/api-fetch"; import { queryKeys } from "@/lib/query-keys"; import { SEARCH_QUERY_MAX, @@ -37,7 +37,8 @@ export interface RecordSearchState { * record shared whole. A grant over some sections never opens the search, so * it is not asked. The request carries the query's abort signal: when the * next keystroke changes the key, the previous query loses its observer and - * its request is cancelled rather than left to land late. + * its request is cancelled rather than left to land late, and the server + * stops reading for it as well. */ export function useRecordSearch(rawQuery: string): RecordSearchState { const { inSharedRecord, sections } = useRecordCapabilities(); @@ -49,12 +50,12 @@ export function useRecordSearch(rawQuery: string): RecordSearchState { const result = useQuery({ queryKey: queryKeys.recordSearch(query, PALETTE_RECORD_LIMIT), + // A POST with the query in the body, so it never stands in a URL that a + // proxy's access log or the browser history could keep. queryFn: ({ signal }) => - apiGet( - `/api/search?${new URLSearchParams({ - q: query, - limit: String(PALETTE_RECORD_LIMIT), - })}`, + apiPost( + "/api/search", + { q: query, limit: PALETTE_RECORD_LIMIT }, { signal }, ), enabled, diff --git a/src/lib/mcp/__tests__/record-candidates-budget.test.ts b/src/lib/mcp/__tests__/record-candidates-budget.test.ts new file mode 100644 index 000000000..9f8332f1b --- /dev/null +++ b/src/lib/mcp/__tests__/record-candidates-budget.test.ts @@ -0,0 +1,83 @@ +/** + * `loadRecordCandidates` under the in-app search's budget: a kind stops + * building candidates once `shouldStop` says so, keeps its newest rows, and + * says it did not see everything; a kind not started when the budget is + * spent is not read at all. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { prismaMock } = vi.hoisted(() => ({ + prismaMock: { + user: { findUnique: vi.fn(async () => null) }, + encounter: { findMany: vi.fn() }, + illnessEpisode: { findMany: vi.fn(async () => []) }, + }, +})); + +vi.mock("@/lib/db", () => ({ prisma: prismaMock })); + +import { loadRecordCandidates } from "../record-search"; + +function encounters(n: number) { + return Array.from({ length: n }, (_, i) => ({ + id: `e${i}`, + userId: "u1", + occurredAt: new Date(Date.UTC(2026, 0, 30 - i)), + kind: "CHECKUP", + status: "COMPLETED", + reasonEncrypted: null, + outcomeEncrypted: null, + bodySiteEncrypted: null, + laterality: null, + practitioner: null, + })); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("loadRecordCandidates under a budget", () => { + it("stops a kind part-way, newest first, and reports it", async () => { + prismaMock.encounter.findMany.mockResolvedValue(encounters(10)); + let checks = 0; + const truncated: string[] = []; + + const out = await loadRecordCandidates("u1", "visit", "", { + kinds: new Set(["visit"]), + rowsPerKind: 100, + // One check before the kind starts, then one per row: the fifth + // check, before the fourth row, says stop. + shouldStop: () => ++checks > 4, + onTruncated: (kind) => truncated.push(kind), + }); + + expect(out.map((c) => c.id)).toEqual(["visit:e0", "visit:e1", "visit:e2"]); + expect(truncated).toContain("visit"); + }); + + it("does not read a kind once the budget is already spent", async () => { + const truncated: string[] = []; + const out = await loadRecordCandidates("u1", "visit", "", { + kinds: new Set(["visit", "condition"]), + shouldStop: () => true, + onTruncated: (kind) => truncated.push(kind), + }); + expect(out).toEqual([]); + expect(prismaMock.encounter.findMany).not.toHaveBeenCalled(); + expect(prismaMock.illnessEpisode.findMany).not.toHaveBeenCalled(); + expect(truncated.sort()).toEqual(["condition", "visit"]); + }); + + it("reads everything and reports nothing without a budget", async () => { + prismaMock.encounter.findMany.mockResolvedValue(encounters(10)); + const truncated: string[] = []; + const out = await loadRecordCandidates("u1", "visit", "", { + kinds: new Set(["visit"]), + rowsPerKind: 100, + onTruncated: (kind) => truncated.push(kind), + }); + expect(out).toHaveLength(10); + expect(truncated).toEqual([]); + }); +}); diff --git a/src/lib/mcp/record-search.ts b/src/lib/mcp/record-search.ts index 3cbd2d8f7..75f146bc9 100644 --- a/src/lib/mcp/record-search.ts +++ b/src/lib/mcp/record-search.ts @@ -20,7 +20,7 @@ * uses. The document body itself is never read for a search. * * Ranking and folding live in `@/lib/search/match`, shared with the in-app - * search (`GET /api/search`), which reads its record kinds through + * search (`POST /api/search`), which reads its record kinds through * `loadRecordCandidates` here as well: one matcher, one set of record reads. * * Module switches. A record kind whose module the account has off (or the @@ -349,7 +349,7 @@ async function visitCandidates( userId: string, owner: Owner, origin: string, - take: number, + read: KindRead, empty: boolean, ): Promise { const rows = await prisma.encounter.findMany({ @@ -361,10 +361,10 @@ async function visitCandidates( ...(empty ? { occurredAt: { lte: new Date() } } : {}), }, orderBy: [{ occurredAt: "desc" }, { id: "asc" }], - take, + take: read.take, include: { practitioner: PRACTITIONER_SELECT }, }); - return rows.map((row) => { + return mapUntilStopped(rows, read, (row): SearchCandidate => { const practitioner = livePractitioner(row.practitioner); const reason = decryptText(row.reasonEncrypted); const outcome = decryptText(row.outcomeEncrypted); @@ -476,15 +476,15 @@ async function conditionCandidates( userId: string, owner: Owner, origin: string, - take: number, + read: KindRead, ): Promise { const rows = await prisma.illnessEpisode.findMany({ where: { userId, deletedAt: null }, orderBy: [{ onsetAt: "desc" }, { id: "asc" }], - take, + take: read.take, select: CONDITION_SELECT, }); - return rows.map((row) => { + return mapUntilStopped(rows, read, (row): SearchCandidate => { const bodySite = decryptText(row.bodySiteEncrypted); const day = userDayKey(row.onsetAt, owner.timezone); const site = bodySite?.replace(/\s+/g, " ").trim(); @@ -604,7 +604,7 @@ async function documentCandidates( userId: string, owner: Owner, origin: string, - take: number, + read: KindRead, query: string, words: readonly string[], includeAiDeferred: boolean, @@ -615,7 +615,7 @@ async function documentCandidates( where: { userId, deletedAt: null }, select: DOCUMENT_SELECT, orderBy: [{ createdAt: "desc" }, { id: "asc" }], - take, + take: read.take, }), words.length > 0 ? contentMatches(userId, words, includeAiDeferred) @@ -654,66 +654,73 @@ async function documentCandidates( }) : []; - return [...rows, ...extra].map((row) => { - const dated = row.documentDate ?? row.reportDate; - const kindLabel = documentKindLabel(row.kind, "en"); - const name = row.title ?? row.filename ?? kindLabel; - return { - id: `document:${row.id}`, - title: scrubFenceMarkers( - `${name} (${kindLabel}${dated ? `, ${dateOnly(dated)}` : ""})`, - ), - url: recordUrl(origin, "document", row.id), - kindOrder: KIND_ORDER.document, - date: (dated ?? row.createdAt).getTime(), - record: true, - hit: { - kind: "document", - recordId: row.id, - title: - row.title ?? - row.filename ?? - documentKindLabel(row.kind, owner.display), - context: compact([ - documentKindLabel(row.kind, owner.display), - row.title ? row.filename : null, - ]), - day: dated - ? dateOnly(dated) - : userDayKey(row.createdAt, owner.timezone), - path: recordPath("document", row.id), - }, - contentWords: content.get(row.id), - fields: [ - { text: row.title ?? "", weight: WEIGHT.primary }, - { text: row.filename ?? "", weight: WEIGHT.primary }, - { - text: [ - "document", - enumWords(row.kind), - kindLabel, - documentKindLabel(row.kind, owner.locale), - ].join(" "), - weight: WEIGHT.secondary, + // The cap applies to the newest read; the extra rows are content and name + // hits beyond it, already bounded by their own read. + if (rows.length >= read.take) read.onTruncated(); + return mapUntilStopped( + [...rows, ...extra], + { ...read, take: Infinity }, + (row): SearchCandidate => { + const dated = row.documentDate ?? row.reportDate; + const kindLabel = documentKindLabel(row.kind, "en"); + const name = row.title ?? row.filename ?? kindLabel; + return { + id: `document:${row.id}`, + title: scrubFenceMarkers( + `${name} (${kindLabel}${dated ? `, ${dateOnly(dated)}` : ""})`, + ), + url: recordUrl(origin, "document", row.id), + kindOrder: KIND_ORDER.document, + date: (dated ?? row.createdAt).getTime(), + record: true, + hit: { + kind: "document", + recordId: row.id, + title: + row.title ?? + row.filename ?? + documentKindLabel(row.kind, owner.display), + context: compact([ + documentKindLabel(row.kind, owner.display), + row.title ? row.filename : null, + ]), + day: dated + ? dateOnly(dated) + : userDayKey(row.createdAt, owner.timezone), + path: recordPath("document", row.id), }, - ], - }; - }); + contentWords: content.get(row.id), + fields: [ + { text: row.title ?? "", weight: WEIGHT.primary }, + { text: row.filename ?? "", weight: WEIGHT.primary }, + { + text: [ + "document", + enumWords(row.kind), + kindLabel, + documentKindLabel(row.kind, owner.locale), + ].join(" "), + weight: WEIGHT.secondary, + }, + ], + }; + }, + ); } async function vaccinationCandidates( userId: string, owner: Owner, origin: string, - take: number, + read: KindRead, ): Promise { const rows = await prisma.vaccinationRecord.findMany({ where: { userId, deletedAt: null }, orderBy: [{ occurredAt: "desc" }, { id: "asc" }], - take, + take: read.take, select: VACCINATION_SELECT, }); - return rows.map((row) => { + return mapUntilStopped(rows, read, (row): SearchCandidate => { const practitioner = livePractitioner(row.practitioner); const entry = resolveVaccineEntry({ antigenSlug: row.antigenSlug }); const own = ownVaccine(row); @@ -790,6 +797,47 @@ export interface RecordSearchOptions { includeAiDeferredText?: boolean; /** Newest rows read per kind; defaults to `MAX_ROWS_PER_KIND`. */ rowsPerKind?: number; + /** + * Asked before each kind starts and before every row a kind decrypts. + * Once it says stop, the kind keeps what it has (its newest rows) and + * `onTruncated` is told. The in-app search hands its shared time budget + * and the request's abort signal in here. + */ + shouldStop?: () => boolean; + /** + * Told when a kind did not see all of its rows: it filled its newest + * `rowsPerKind` read, or `shouldStop` ended it part-way. + */ + onTruncated?: (kind: RecordKind) => void; +} + +/** How one kind reads: its row cap, and the stop and truncation hooks. */ +interface KindRead { + take: number; + shouldStop: () => boolean; + onTruncated: () => void; +} + +/** + * Map a kind's rows newest first until `shouldStop` says stop. The rows are + * read already; what this bounds is the decrypting and the building, which is + * where a large record spends its time. + */ +function mapUntilStopped( + rows: readonly Row[], + read: KindRead, + build: (row: Row) => Out, +): Out[] { + if (rows.length >= read.take) read.onTruncated(); + const out: Out[] = []; + for (const row of rows) { + if (read.shouldStop()) { + read.onTruncated(); + break; + } + out.push(build(row)); + } + return out; } /** @@ -811,26 +859,40 @@ export async function loadRecordCandidates( options.kinds ? Promise.resolve(options.kinds) : readableKinds(userId), loadOwner(userId, options.displayLocale), ]); + const shouldStop = options.shouldStop ?? (() => false); + const reading = (kind: RecordKind): KindRead | null => { + if (!kinds.has(kind)) return null; + const onTruncated = () => options.onTruncated?.(kind); + if (shouldStop()) { + onTruncated(); + return null; + } + return { take, shouldStop, onTruncated }; + }; + const visit = reading("visit"); + const condition = reading("condition"); + const document = reading("document"); + const vaccination = reading("vaccination"); const groups = await Promise.all([ - kinds.has("visit") - ? visitCandidates(userId, owner, origin, take, empty) + visit + ? visitCandidates(userId, owner, origin, visit, empty) : Promise.resolve([]), - kinds.has("condition") - ? conditionCandidates(userId, owner, origin, take) + condition + ? conditionCandidates(userId, owner, origin, condition) : Promise.resolve([]), - kinds.has("document") + document ? documentCandidates( userId, owner, origin, - take, + document, query, words, options.includeAiDeferredText === true, ) : Promise.resolve([]), - kinds.has("vaccination") - ? vaccinationCandidates(userId, owner, origin, take) + vaccination + ? vaccinationCandidates(userId, owner, origin, vaccination) : Promise.resolve([]), ]); return groups.flat(); diff --git a/src/lib/openapi/routes/search.ts b/src/lib/openapi/routes/search.ts index 4de58c029..94957319c 100644 --- a/src/lib/openapi/routes/search.ts +++ b/src/lib/openapi/routes/search.ts @@ -17,12 +17,15 @@ import { dataEnvelope, recordRefusal, stdResponses } from "./shared"; export const searchPaths: NonNullable = { "/api/search": { - get: { + post: { tags: ["Records"], summary: "Search the record", description: - "One search over the record: visits and procedures, conditions, documents (title, file name and indexed text), vaccinations, lab results (analyte names and notes), medications, symptoms, workouts by sport, and the notes on mood entries, readings and illness days. Hits are grouped by kind, the best group first, and each carries the in-app path that opens the record. Matching ignores case and accents and reads German umlauts typed as ae, oe, ue or ss. A source whose module the record has switched off is not read. The search reads each source newest first under a per-source row cap and a time budget; `truncated` says when that cut it short. The query is never logged. Rate limited per acting account (60 a minute). A grant scoped to some sections of the record does not open it, like the timeline.", - requestParams: { query: searchQuerySchema }, + "One search over the record: visits and procedures, conditions, documents (title, file name and indexed text), vaccinations, lab results (analyte names and notes), medications, symptoms, workouts by sport, and the notes on mood entries, readings and illness days. Hits are grouped by kind, the best group first, and each carries the in-app path that opens the record. Matching ignores case and accents and reads German umlauts typed as ae, oe, ue or ss. A source whose module the record has switched off is not read. The search reads each source newest first under a per-source row cap and a time budget; `truncated` says when that cut it short. The query travels in the JSON body, never in the URL, and is never logged. Rate limited per acting account (20 a minute), and one search at a time: a newer search by the same account stops the one before it, which then answers 409 `search.superseded`. A grant scoped to some sections of the record does not open it, like the timeline.", + requestBody: { + required: true, + content: { "application/json": { schema: searchQuerySchema } }, + }, responses: { "200": { description: "The hits, grouped by kind.", diff --git a/src/lib/query-keys/search.ts b/src/lib/query-keys/search.ts index d9c0aee9a..f5d08a91c 100644 --- a/src/lib/query-keys/search.ts +++ b/src/lib/query-keys/search.ts @@ -1,5 +1,5 @@ /** - * Query keys — the in-app record search (`GET /api/search`, v1.42.2). + * Query keys — the in-app record search (`POST /api/search`, v1.42.2). * Part of the centralized factory; aggregated in `./index.ts`. * * The query text is part of the key, which keeps it in memory only: the @@ -9,7 +9,7 @@ export const searchKeys = { /** Root prefix. */ searchRoot: () => ["search"] as const, - /** `GET /api/search?q=&limit=`. */ + /** `POST /api/search` with `{ q, limit }`. */ recordSearch: (query: string, limit: number) => ["search", "records", query, limit] as const, }; diff --git a/src/lib/search/__tests__/search-records.test.ts b/src/lib/search/__tests__/search-records.test.ts index 297838958..0b9574224 100644 --- a/src/lib/search/__tests__/search-records.test.ts +++ b/src/lib/search/__tests__/search-records.test.ts @@ -19,9 +19,15 @@ import type { SearchCandidate } from "../match"; import { readableSources, scanNewest, + searchRecords, type ScanBudget, } from "../search-records"; +const { loadRecordCandidates } = vi.hoisted(() => ({ + loadRecordCandidates: vi.fn(async () => []), +})); +vi.mock("@/lib/mcp/record-search", () => ({ loadRecordCandidates })); + function budget(deadlineIn = 10_000, clock = { t: 0 }): ScanBudget { return { deadline: clock.t + deadlineIn, @@ -91,6 +97,64 @@ describe("scanNewest", () => { }); }); +describe("an abandoned search", () => { + it("stops a scan at its next batch once the signal aborts", async () => { + const controller = new AbortController(); + const b = { ...budget(), signal: controller.signal }; + const s = source(10_000, 1_000_000); + const read = async (cursor: string | null, take: number) => { + controller.abort(); + return s.read(cursor, take); + }; + await scanNewest({ budget: b, wanted: 3, batch: 10, read, match: s.match }); + expect(s.reads).toEqual([10]); + expect(b.truncated).toBe(true); + }); + + it("starts no source when the request is already gone", async () => { + const controller = new AbortController(); + controller.abort(); + const result = await searchRecords({ + recordId: "u1", + query: "schulter", + limit: 5, + modules: null, + domainVisible: () => true, + displayLocale: "en", + timezone: "UTC", + signal: controller.signal, + }); + // `prisma` is an empty object here: any source that ran would throw. + expect(loadRecordCandidates).not.toHaveBeenCalled(); + expect(result.groups).toEqual([]); + expect(result.truncated).toBe(true); + }); + + it("hands the clinical kinds the shared budget", async () => { + const clock = { t: 0 }; + loadRecordCandidates.mockImplementationOnce(async (...args: unknown[]) => { + const options = args[3] as { shouldStop: () => boolean }; + expect(options.shouldStop()).toBe(false); + clock.t = 2_000; + expect(options.shouldStop()).toBe(true); + return []; + }); + await searchRecords({ + recordId: "u1", + query: "schulter", + limit: 5, + modules: null, + // Only the clinical kinds: every other source would reach `prisma`. + domainVisible: (domain) => domain === "profile", + displayLocale: "en", + timezone: "UTC", + now: () => clock.t, + budgetMs: 1_500, + }); + expect(loadRecordCandidates).toHaveBeenCalledTimes(1); + }); +}); + describe("readableSources", () => { const all = () => true; diff --git a/src/lib/search/contract.ts b/src/lib/search/contract.ts index 40be764de..50a3415dd 100644 --- a/src/lib/search/contract.ts +++ b/src/lib/search/contract.ts @@ -1,5 +1,5 @@ /** - * The in-app record search (`GET /api/search`): its vocabulary and wire + * The in-app record search (`POST /api/search`): its vocabulary and wire * types. Pure constants and types, no validator, so the command palette * imports this file and nothing server-side. The Zod schemas the route parses * with and the OpenAPI table publishes live in `./wire-schemas.ts`. diff --git a/src/lib/search/in-flight.ts b/src/lib/search/in-flight.ts new file mode 100644 index 000000000..e58c01ee7 --- /dev/null +++ b/src/lib/search/in-flight.ts @@ -0,0 +1,42 @@ +/** + * One search in flight per actor. + * + * The palette searches once per pause in typing, and the client cancels the + * previous request when the text changes. A cancelled fetch does not stop the + * server, though: without this, every keystroke-pause left a search reading + * and decrypting on the server until its own budget ran out, so a fast typist + * held several at once. A new search by the same actor aborts the one before + * it, which stops at its next budget check. + * + * Per process: on a multi-instance deploy two searches by one actor on two + * instances both run, each still bounded by its own time budget and by the + * per-actor rate limit, which lives in Postgres. + */ +const inFlight = new Map(); + +/** The reason a search was stopped for a newer one. */ +export const SUPERSEDED = "search.superseded"; + +export interface SearchSlot { + /** Aborted when the same actor starts a newer search. */ + signal: AbortSignal; + /** Release the slot once the search answered (or failed). */ + end: () => void; +} + +export function beginSearch(actorId: string): SearchSlot { + inFlight.get(actorId)?.abort(SUPERSEDED); + const controller = new AbortController(); + inFlight.set(actorId, controller); + return { + signal: controller.signal, + end: () => { + if (inFlight.get(actorId) === controller) inFlight.delete(actorId); + }, + }; +} + +/** Searches currently held, for tests. */ +export function searchesInFlight(): number { + return inFlight.size; +} diff --git a/src/lib/search/match.ts b/src/lib/search/match.ts index 2bc858975..c2d15cb0f 100644 --- a/src/lib/search/match.ts +++ b/src/lib/search/match.ts @@ -1,6 +1,6 @@ /** * Matching and ranking for the record search, shared by the MCP `search` tool - * and the in-app search (`GET /api/search`). Pure: no database, no crypto. + * and the in-app search (`POST /api/search`). Pure: no database, no crypto. * * Folding. A field and a query are compared after the body-sites service's * folding (`normalizeSearchText`: Unicode compatibility form, marks stripped, diff --git a/src/lib/search/search-records.ts b/src/lib/search/search-records.ts index 6a61f84a1..189d665c1 100644 --- a/src/lib/search/search-records.ts +++ b/src/lib/search/search-records.ts @@ -1,5 +1,5 @@ /** - * The in-app record search behind `GET /api/search` (Discussion #1117): one + * The in-app record search behind `POST /api/search` (Discussion #1117): one * query over visits, conditions, documents, vaccinations, lab results, * medications, symptoms, workouts and the free-text notes on mood entries, * readings and illness days, each hit with the in-app path that opens it. @@ -16,7 +16,11 @@ * batches of `SCAN_BATCH`, and stops as soon as it holds more hits than * the caller's limit (newest first, so those are the newest matches); * - one search shares a time budget (`SEARCH_TIME_BUDGET_MS`); a source - * checks it before every batch and stops when it has run out; + * checks it before every batch and stops when it has run out, the + * clinical kinds check it before every row they decrypt, and a source + * that has not started yet when it runs out is not started at all; + * - the request's abort signal ends the search the same way: a client that + * typed on, or a newer search by the same actor, stops this one; * - at most `SOURCE_CONCURRENCY` sources read at once, so one search holds * a few pool connections, not one per table; * - only matching rows are kept: a batch's decrypted text is dropped once @@ -101,12 +105,19 @@ const MAX_DOCUMENT_SNIPPETS = 5; export interface ScanBudget { readonly deadline: number; readonly now: () => number; + /** Aborted when nobody waits for the answer any more. */ + readonly signal?: AbortSignal; /** A cap or the deadline stopped a source before it read everything. */ truncated: boolean; /** Rows read, for the annotation. */ rows: number; } +/** The search may not read any further: out of time, or abandoned. */ +export function budgetSpent(budget: ScanBudget): boolean { + return budget.signal?.aborted === true || budget.now() >= budget.deadline; +} + /** * Read a source newest first in batches until it is exhausted, holds more * than `wanted` hits, has read `maxRows` rows or the budget has run out. @@ -128,7 +139,7 @@ export async function scanNewest(args: { let cursor: string | null = null; let scanned = 0; for (;;) { - if (budget.now() >= budget.deadline) { + if (budgetSpent(budget)) { budget.truncated = true; break; } @@ -161,15 +172,23 @@ function after(cursor: string | null): { return cursor ? { cursor: { id: cursor }, skip: 1 } : {}; } -/** Run `tasks` with at most `limit` in flight. */ +/** + * Run `tasks` with at most `limit` in flight. A task not yet started when + * the budget is spent is not started: the search says `truncated` instead. + */ async function inPool( tasks: ReadonlyArray<() => Promise>, limit: number, + budget: ScanBudget, ): Promise { let next = 0; const worker = async () => { while (next < tasks.length) { const task = tasks[next++]; + if (budgetSpent(budget)) { + budget.truncated = true; + continue; + } await task(); } }; @@ -805,6 +824,12 @@ export interface SearchRecordsArgs { displayLocale: Locale; /** The record's zone, for the calendar day of an instant. */ timezone: string; + /** + * Aborted when the answer is no longer wanted (the client went away, or + * the same actor started a newer search): every source stops at its next + * check. + */ + signal?: AbortSignal; /** Overrides for tests. */ now?: () => number; budgetMs?: number; @@ -837,6 +862,7 @@ export async function searchRecords( const budget: ScanBudget = { deadline: started + (args.budgetMs ?? SEARCH_TIME_BUDGET_MS), now, + signal: args.signal, truncated: false, rows: 0, }; @@ -877,20 +903,22 @@ export async function searchRecords( displayLocale: args.displayLocale, includeAiDeferredText: true, rowsPerKind: RECORD_ROWS_PER_KIND, + // The same budget as every other source: each kind checks it + // before every row it decrypts, so a large record cannot hold the + // search past its time. + shouldStop: () => budgetSpent(budget), + // A kind that holds more than its newest `RECORD_ROWS_PER_KIND`, + // or that the budget stopped part-way, may have older matches the + // search did not see. + onTruncated: () => { + budget.truncated = true; + }, }, ); budget.rows += candidates.length; - // Each kind reads its newest `RECORD_ROWS_PER_KIND`; a kind that - // filled the read may have older rows the search did not see. - const perKind = new Map(); for (const candidate of candidates) { - const kind = candidate.hit?.kind ?? ""; - perKind.set(kind, (perKind.get(kind) ?? 0) + 1); if (keepIfHit(ctx, candidate)) collected.push(candidate); } - if ([...perKind.values()].some((n) => n >= RECORD_ROWS_PER_KIND)) { - budget.truncated = true; - } }); } add("labAnalyte", () => labAnalyteCandidates(ctx)); @@ -903,7 +931,7 @@ export async function searchRecords( add("illnessNote", () => illnessNoteCandidates(ctx)); add("measurementNote", () => measurementNoteCandidates(ctx)); - await inPool(tasks, SOURCE_CONCURRENCY); + await inPool(tasks, SOURCE_CONCURRENCY, budget); const ranked: Ranked[] = collected .filter((candidate) => candidate.hit) diff --git a/src/lib/search/wire-schemas.ts b/src/lib/search/wire-schemas.ts index 6523acf04..7adee6f2c 100644 --- a/src/lib/search/wire-schemas.ts +++ b/src/lib/search/wire-schemas.ts @@ -1,5 +1,5 @@ /** - * Zod schemas for `GET /api/search`. The route parses its query with these + * Zod schemas for `POST /api/search`. The route parses its body with these * and the OpenAPI table publishes them. Server and OpenAPI side only: the * palette imports its types from `./contract.ts`. */ diff --git a/src/proxy.ts b/src/proxy.ts index 6830373aa..b973d8c2f 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -225,6 +225,9 @@ const DEMO_MUTATION_ALLOWLIST: ReadonlyArray<{ path: string; method: string }> = // The per-profile routes (`/api/managed-profiles/{id}`, its guardians) // are different paths and stay closed too. { path: "/api/auth/profile", method: "PUT" }, + // v1.42.2 — the record search is a read that travels as a POST so the + // query stays out of URLs and access logs. It writes nothing. + { path: "/api/search", method: "POST" }, ]; // Legacy route redirects (German → English) diff --git a/tests/integration/record-search.test.ts b/tests/integration/record-search.test.ts index b96c55449..f3f915254 100644 --- a/tests/integration/record-search.test.ts +++ b/tests/integration/record-search.test.ts @@ -1,5 +1,5 @@ /** - * v1.42.2 (#1117) — `GET /api/search` against real Postgres. + * v1.42.2 (#1117) — `POST /api/search` against real Postgres. * * What the unit tests cannot show: that encrypted text is found after the * decrypt whichever way the umlaut is typed, that the reads are narrowed to @@ -79,17 +79,20 @@ async function signIn(userId: string) { } async function search(q: string, limit?: number): Promise { - const { GET } = await import("@/app/api/search/route"); - const params = new URLSearchParams({ q }); - if (limit) params.set("limit", String(limit)); + const { POST } = await import("@/app/api/search/route"); return ( - GET as unknown as ( + POST as unknown as ( req: NextRequest, ctx: { params: Promise }, ) => Promise - )(new NextRequest(`http://localhost/api/search?${params}`), { - params: Promise.resolve({}), - }); + )( + new NextRequest("http://localhost/api/search", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(limit ? { q, limit } : { q }), + }), + { params: Promise.resolve({}) }, + ); } async function hits(q: string, limit?: number): Promise {