diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index b35dfaf1b..bbf7e832f 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -70,7 +70,18 @@ paths: schema: $ref: "#/components/schemas/CycleCustomSymptomListEnvelope" "401": &a1 - description: Authentication required or invalid credentials. + description: "Authentication required or invalid credentials. The generic auth gates — the ones every route passes + through before its own body runs — name the reason in `meta.errorCode`, and that is the field to branch on + there; the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session + cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has + passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is + gone; discard the credential and sign in again — retrying will not help. `auth.mfa.code_invalid`: a second + factor presented on a step-up flow did not verify; re-prompt for a code and keep the session. The related + 403 refusals carry `auth.scope.insufficient` (the credential is valid but its scope does not reach this + route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that + is not an admin's). A route that checks a credential of its own — sign-in, the MFA challenge exchange, a + password confirmation — may still answer 401 with prose and no code, so a missing `meta.errorCode` means the + route refused the credential it was handed, not that the response is malformed." content: application/json: schema: @@ -101,7 +112,32 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "429": &a5 - description: Rate limit exceeded. + description: Rate limit exceeded. The response carries `Retry-After` (whole seconds, rounded up and never below 1 — wait + at least that long before retrying), `X-RateLimit-Limit` (the bucket's cap), `X-RateLimit-Remaining` and + `X-RateLimit-Reset` (the reset instant, ISO-8601, not epoch seconds). Back off on `Retry-After` rather than + guessing. A 429 raised by a ceiling that is not one of these buckets — a daily AI budget, an upstream + provider's own refusal relayed onward — carries none of them, because there is no bucket to describe. + headers: + Retry-After: + description: Whole seconds to wait before retrying. Rounded up, never below 1. + schema: + type: integer + minimum: 1 + X-RateLimit-Limit: + description: Requests the bucket allows per window. + schema: + type: integer + minimum: 1 + X-RateLimit-Remaining: + description: Requests left in the current window; 0 on a refusal. + schema: + type: integer + minimum: 0 + X-RateLimit-Reset: + description: Instant the current window rolls over, ISO-8601 — not epoch seconds. + schema: + type: string + format: date-time content: application/json: schema: @@ -3328,10 +3364,12 @@ paths: get: tags: - Sync - summary: Measurements delta feed - description: "Incremental catch-up after the first-pair backfill (never a replacement for it). Pages over an opaque - keyset cursor; each page carries `tombstones` (soft-deleted rows, keyed on `externalId`) and `upserts` (live - rows). Apply tombstones before upserts within a page. `cursorExpired: true` forces a clean re-init." + summary: Multi-domain delta feed + description: "Incremental catch-up after the first-pair backfill (never a replacement for it). Pages over one opaque + keyset cursor spanning five domains — measurements, mood, medication intakes, cycle day logs and cycles. Each + domain carries `tombstones` (soft-deleted rows) and `upserts` (live rows); apply tombstones before upserts + within a domain. Tombstone identity is `externalId` for measurements and the server `id` everywhere else. + `cursorExpired: true` forces a clean re-init and the page is empty in every domain." parameters: - in: query name: cursor @@ -15423,9 +15461,10 @@ paths: returns a `{ title, kind, documentDate }` DRAFT for the edit form. AI-consent / rate / budget gated (shares the per-user document-AI bucket with extract/summary/index — default 6/hour, operator-tunable via `DOCUMENT_AI_LIMIT_PER_HOUR`; a slot is only consumed when the request reaches the provider, and the 429 carries - the reset instant in `X-RateLimit-Reset` and `meta.retryAt`). WRITES NOTHING — never stages facts, never flips - status; the user reviews and saves. 422 (`documents.inbound.providerUnsupported`) with no provider configured. - Never interprets or diagnoses; the title is a neutral filing label." + `Retry-After`, the `X-RateLimit-Limit` / `-Remaining` / `-Reset` triple, and `meta.retryAt`). WRITES NOTHING — + never stages facts, never flips status; the user reviews and saves. 422 + (`documents.inbound.providerUnsupported`) with no provider configured. Never interprets or diagnoses; the title + is a neutral filing label." parameters: - name: id in: path @@ -19894,9 +19933,9 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "429": - description: More than 60 requests in a minute for this token — or, when no valid token was presented, from this client - IP. `meta.errorCode` = `ingest.rate_limited`, plus the standard `X-RateLimit-Remaining` / - `X-RateLimit-Reset` headers. + description: "More than 60 requests in a minute for this token — or, when no valid token was presented, from this client + IP. `meta.errorCode` = `ingest.rate_limited`, plus the standard rate-limit headers: `Retry-After` (whole + seconds, at least 1), `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset`." content: application/json: schema: @@ -29242,19 +29281,63 @@ components: type: "null" error: type: string + details: + description: "Present on a 422 raised by the multi-issue validator: every field the request got wrong, not only the + first. Absent on every other status." + type: object + properties: + issues: + type: array + items: + $ref: "#/components/schemas/ValidationIssue" + required: + - issues + additionalProperties: false meta: type: object properties: requestId: type: string errorCode: + description: Stable machine code for this refusal. Branch on it rather than on `error`, which is prose and may be + reworded. type: string - additionalProperties: false + additionalProperties: {} required: - data - error additionalProperties: false - description: "Standard error response: data is null, error is human prose." + description: "Standard error response: `data` is null, `error` is human prose. `meta.errorCode` carries the stable + machine code where one exists, and `meta` may carry further per-refusal context. A 422 from the multi-issue + validator additionally carries `details.issues` — every field the request got wrong, so a client can fix them in + one round-trip." + ValidationIssue: + type: object + properties: + path: + type: string + description: Dot-joined path to the rejected field, e.g. `entries.3.measuredAt`. Empty string when the whole body was + rejected. + code: + type: string + description: Zod issue code, e.g. `invalid_type`, `too_small`, `unrecognized_keys`. Treat an unfamiliar code as a + generic rejection of `path`. + message: + type: string + description: English sentence describing the rejection. Safe to log; show the user your own wording keyed on `path`. + keys: + description: "Only on `unrecognized_keys`: the refused key names, bounded to 10 and 64 characters each. Zod reports + every unknown key of one object in a single issue whose `path` is the object, so the names live here and + nowhere in `path`." + type: array + items: + type: string + required: + - path + - code + - message + additionalProperties: false + description: "One rejected field in a multi-issue 422. Sanitised: no rejected VALUE is echoed back." CycleCustomSymptomCreatedEnvelope: type: object properties: @@ -37507,10 +37590,45 @@ components: - upserts - tombstones additionalProperties: false + cycleDays: + type: object + properties: + upserts: + type: array + items: + $ref: "#/components/schemas/CycleDayLogDTO" + tombstones: + type: array + items: + $ref: "#/components/schemas/SyncCycleDayTombstone" + required: + - upserts + - tombstones + additionalProperties: false + description: Cycle day logs. Present on every page, empty arrays when the account has the cycle module off. + cycles: + type: object + properties: + upserts: + type: array + items: + $ref: "#/components/schemas/MenstrualCycleDTO" + tombstones: + type: array + items: + $ref: "#/components/schemas/SyncCycleTombstone" + required: + - upserts + - tombstones + additionalProperties: false + description: Menstrual cycles, observed and forward-predicted. Present on every page, empty arrays when the account has + the cycle module off. required: - measurements - mood - intakes + - cycleDays + - cycles additionalProperties: false required: - serverNow @@ -37519,9 +37637,10 @@ components: - cursorExpired - changes additionalProperties: false - description: "Multi-domain delta page (v1.7.0): measurements + mood + intakes. One opaque multi-domain keyset cursor; - tombstones apply before upserts within each domain. Tombstone identity: measurements key on externalId, mood + - intakes on server id. The iOS consumer is measurements-only this cycle; mood + intakes are forward-prep." + description: "Multi-domain delta page: measurements, mood, intakes, cycle day logs and cycles. One opaque multi-domain + keyset cursor; tombstones apply before upserts within each domain. Tombstone identity: measurements key on + externalId, every other domain on server id. All five keys are present on every response, including the + `cursorExpired` short-circuit — a domain with nothing to report sends empty arrays rather than being omitted." SyncMeasurementUpsert: type: object properties: @@ -37863,6 +37982,60 @@ components: - updatedAt additionalProperties: false description: A soft-deleted medication intake, keyed on server `id`. Apply before upserts within the domain page. + SyncCycleDayTombstone: + type: object + properties: + id: + type: string + externalId: + anyOf: + - type: string + - type: "null" + description: The client's own key for the day log, when it set one. + syncVersion: + type: integer + minimum: 0 + maximum: 9007199254740991 + deletedAt: + type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + updatedAt: + type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + required: + - id + - externalId + - syncVersion + - deletedAt + - updatedAt + additionalProperties: false + description: A soft-deleted cycle day log, keyed on server `id`. Apply before upserts within the domain page. + SyncCycleTombstone: + type: object + properties: + id: + type: string + syncVersion: + type: integer + minimum: 0 + maximum: 9007199254740991 + deletedAt: + type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + updatedAt: + type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + required: + - id + - syncVersion + - deletedAt + - updatedAt + additionalProperties: false + description: A soft-deleted menstrual cycle, keyed on server `id`. Apply before upserts within the domain page. GetMeasurementsSeriesBatchResponse: type: object properties: diff --git a/src/__tests__/api/auth/check-user/route.test.ts b/src/__tests__/api/auth/check-user/route.test.ts index 7cd4e5eae..8ae3cbd40 100644 --- a/src/__tests__/api/auth/check-user/route.test.ts +++ b/src/__tests__/api/auth/check-user/route.test.ts @@ -27,6 +27,7 @@ vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, }), @@ -34,6 +35,7 @@ vi.mock("@/lib/rate-limit", () => ({ // wrapper. Default to a clean per-IP result. checkAuthSurfaceRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", @@ -84,11 +86,13 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, }); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", @@ -175,6 +179,7 @@ describe("POST /api/auth/check-user", () => { it("returns 429 when the per-IP rate-limit is exhausted", async () => { vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", diff --git a/src/app/api/auth/me/diabetes/__tests__/route.test.ts b/src/app/api/auth/me/diabetes/__tests__/route.test.ts index 5919ff386..0cee31013 100644 --- a/src/app/api/auth/me/diabetes/__tests__/route.test.ts +++ b/src/app/api/auth/me/diabetes/__tests__/route.test.ts @@ -32,6 +32,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -192,6 +193,7 @@ describe("PATCH /api/auth/me/diabetes", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/disable-coach/__tests__/route.test.ts b/src/app/api/auth/me/disable-coach/__tests__/route.test.ts index 8042a65c6..5ed9ad5b2 100644 --- a/src/app/api/auth/me/disable-coach/__tests__/route.test.ts +++ b/src/app/api/auth/me/disable-coach/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -247,6 +248,7 @@ describe("PATCH /api/auth/me/disable-coach", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts b/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts index 943f37294..a699059cb 100644 --- a/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts +++ b/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts @@ -36,6 +36,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -197,6 +198,7 @@ describe("PATCH /api/auth/me/glucose-unit", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/health-score-config/__tests__/route.test.ts b/src/app/api/auth/me/health-score-config/__tests__/route.test.ts index 75d4ff20f..da967879f 100644 --- a/src/app/api/auth/me/health-score-config/__tests__/route.test.ts +++ b/src/app/api/auth/me/health-score-config/__tests__/route.test.ts @@ -35,6 +35,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -397,6 +398,7 @@ describe("PATCH — envelope and transport", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts index 7884d3f52..6639267e6 100644 --- a/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts +++ b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts @@ -147,6 +147,10 @@ describe("POST /api/auth/me/mfa/disable", () => { } as never); const res = await POST(req()); expect(res.status).toBe(401); + // The code is what separates "re-prompt for a digit" from "this session is + // gone"; a client that branches on prose here signs the user out on a typo. + const body = (await res.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("auth.mfa.code_invalid"); expect(destroyOtherSessions).not.toHaveBeenCalled(); }); }); diff --git a/src/app/api/auth/me/mfa/disable/route.ts b/src/app/api/auth/me/mfa/disable/route.ts index 50d22d12a..8e9718220 100644 --- a/src/app/api/auth/me/mfa/disable/route.ts +++ b/src/app/api/auth/me/mfa/disable/route.ts @@ -13,6 +13,7 @@ */ import { apiHandler, + AUTH_ERROR_CODES, requireMfaManagementAuth, HttpError, } from "@/lib/api-handler"; @@ -81,7 +82,7 @@ export const POST = apiHandler(async (req: Request) => { }, }); annotate({ action: { name: "auth.mfa.disable.invalid_factor" } }); - throw new HttpError(401, "Invalid code"); + throw new HttpError(401, "Invalid code", AUTH_ERROR_CODES.mfaCode); } // Every cheap check has passed and the teardown is next, so spend the diff --git a/src/app/api/auth/me/mfa/totp/confirm/route.ts b/src/app/api/auth/me/mfa/totp/confirm/route.ts index 8e701bf04..1251829d0 100644 --- a/src/app/api/auth/me/mfa/totp/confirm/route.ts +++ b/src/app/api/auth/me/mfa/totp/confirm/route.ts @@ -18,6 +18,7 @@ */ import { apiHandler, + AUTH_ERROR_CODES, requireMfaManagementAuth, HttpError, } from "@/lib/api-handler"; @@ -99,7 +100,7 @@ export const POST = apiHandler(async (req: Request) => { action: { name: "auth.mfa.totp.confirm.invalid" }, meta: { replay: result.replay }, }); - throw new HttpError(401, "Invalid code"); + throw new HttpError(401, "Invalid code", AUTH_ERROR_CODES.mfaCode); } const recoveryCodes = generateRecoveryCodes(); diff --git a/src/app/api/auth/me/modules/__tests__/route.test.ts b/src/app/api/auth/me/modules/__tests__/route.test.ts index 1e23957ac..7a14aef25 100644 --- a/src/app/api/auth/me/modules/__tests__/route.test.ts +++ b/src/app/api/auth/me/modules/__tests__/route.test.ts @@ -35,6 +35,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -300,6 +301,7 @@ describe("PATCH /api/auth/me/modules", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts b/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts index 088cecd53..c8d503649 100644 --- a/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts +++ b/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts @@ -26,6 +26,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -587,6 +588,7 @@ describe("PATCH /api/auth/me/notification-prefs", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/sessions/route.ts b/src/app/api/auth/me/sessions/route.ts index 3d447e3d9..ee4ade61c 100644 --- a/src/app/api/auth/me/sessions/route.ts +++ b/src/app/api/auth/me/sessions/route.ts @@ -13,7 +13,12 @@ */ import { NextRequest } from "next/server"; import { headers } from "next/headers"; -import { apiHandler, HttpError, requireAuth } from "@/lib/api-handler"; +import { + apiHandler, + AUTH_ERROR_CODES, + HttpError, + requireAuth, +} from "@/lib/api-handler"; import { apiSuccess } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; @@ -139,7 +144,7 @@ export const DELETE = apiHandler(async (request: NextRequest) => { async function presentedBearerToken(): Promise { const authHeader = (await headers()).get("authorization"); if (!authHeader?.startsWith("Bearer ")) { - throw new HttpError(401, "Not authenticated"); + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); } return authHeader.slice(7); } diff --git a/src/app/api/c/[token]/unlock/__tests__/route.test.ts b/src/app/api/c/[token]/unlock/__tests__/route.test.ts index 7c0828da3..e7cf2ef56 100644 --- a/src/app/api/c/[token]/unlock/__tests__/route.test.ts +++ b/src/app/api/c/[token]/unlock/__tests__/route.test.ts @@ -68,6 +68,7 @@ beforeEach(() => { vi.stubEnv("API_TOKEN_HMAC_KEY", TEST_HMAC_KEY); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 60_000, ip: "1.2.3.4", @@ -137,6 +138,7 @@ describe("POST /api/c/[token]/unlock", () => { }); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, ip: "1.2.3.4", diff --git a/src/app/api/consent/ai/__tests__/route.test.ts b/src/app/api/consent/ai/__tests__/route.test.ts index cfb9c52c3..4a9cb42f3 100644 --- a/src/app/api/consent/ai/__tests__/route.test.ts +++ b/src/app/api/consent/ai/__tests__/route.test.ts @@ -55,7 +55,12 @@ import { getSession } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { checkConsentRateLimit } from "@/lib/rate-limit"; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; const $transaction = vi.mocked(prisma.$transaction) as unknown as { mockImplementation: (impl: (fn: TxFn) => unknown) => void; @@ -109,6 +114,7 @@ describe("POST /api/consent/ai", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 20, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/consent/ai/latest/__tests__/route.test.ts b/src/app/api/consent/ai/latest/__tests__/route.test.ts index b21a15eeb..b486eb6b7 100644 --- a/src/app/api/consent/ai/latest/__tests__/route.test.ts +++ b/src/app/api/consent/ai/latest/__tests__/route.test.ts @@ -54,7 +54,12 @@ import { getSession } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { checkConsentRateLimit } from "@/lib/rate-limit"; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; const $transaction = vi.mocked(prisma.$transaction) as unknown as { mockImplementation: (impl: (fn: TxFn) => unknown) => void; @@ -99,6 +104,7 @@ describe("GET /api/consent/ai/latest", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 20, remaining: 0, resetAt: Date.now() + 60_000, }); @@ -197,6 +203,7 @@ describe("DELETE /api/consent/ai/latest", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 20, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/consent/ai/web/__tests__/route.test.ts b/src/app/api/consent/ai/web/__tests__/route.test.ts index d646dcb1f..baecb7a59 100644 --- a/src/app/api/consent/ai/web/__tests__/route.test.ts +++ b/src/app/api/consent/ai/web/__tests__/route.test.ts @@ -43,7 +43,12 @@ const SESSION_OK = { user: { id: "user-1", username: "testuser", role: "USER" as const }, }; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; /** The mount heal posts no body and no content-type. */ function mkHeal(): Request { @@ -79,6 +84,7 @@ describe("POST /api/consent/ai/web", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 20, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/custom-metrics/[id]/entries/__tests__/route.test.ts b/src/app/api/custom-metrics/[id]/entries/__tests__/route.test.ts index d02cd4a08..c13967317 100644 --- a/src/app/api/custom-metrics/[id]/entries/__tests__/route.test.ts +++ b/src/app/api/custom-metrics/[id]/entries/__tests__/route.test.ts @@ -90,10 +90,18 @@ describe("GET /api/custom-metrics/[id]/entries", () => { expect(body.data.meta.total).toBe(5); expect(body.data.meta.limit).toBe(1); const findCall = vi.mocked(prisma.customMetricEntry.findMany).mock - .calls[0][0] as { where: Record; take: number }; + .calls[0][0] as { + where: Record; + take: number; + orderBy: unknown; + }; expect(findCall.where.userId).toBe("user-1"); expect(findCall.where.customMetricId).toBe("cm-1"); expect(findCall.take).toBe(1); + // `measuredAt` is not unique on a custom metric — a batch of readings can + // share one instant — so offset paging over it alone can repeat a row on + // one page and drop another. + expect(findCall.orderBy).toEqual([{ measuredAt: "desc" }, { id: "desc" }]); }); it("404s a cross-user / unknown metric", async () => { diff --git a/src/app/api/custom-metrics/[id]/entries/route.ts b/src/app/api/custom-metrics/[id]/entries/route.ts index 71a54f7e0..5fdd9ff4b 100644 --- a/src/app/api/custom-metrics/[id]/entries/route.ts +++ b/src/app/api/custom-metrics/[id]/entries/route.ts @@ -62,7 +62,9 @@ export const GET = apiHandler( const [rows, total] = await Promise.all([ prisma.customMetricEntry.findMany({ where, - orderBy: { measuredAt: sortDir }, + // Unique tiebreaker — see the measurements list: offset paging over + // a non-unique sort key can repeat and drop rows between pages. + orderBy: [{ measuredAt: sortDir }, { id: sortDir }], take: limit, skip: offset, }), diff --git a/src/app/api/daily/digest/dismiss/__tests__/route.test.ts b/src/app/api/daily/digest/dismiss/__tests__/route.test.ts index 8c2ab3e05..795f7ef8c 100644 --- a/src/app/api/daily/digest/dismiss/__tests__/route.test.ts +++ b/src/app/api/daily/digest/dismiss/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/modules/gate", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 39, resetAt: Date.now() + 60_000, })), @@ -159,6 +160,7 @@ describe("POST /api/daily/digest/dismiss", () => { const { checkRateLimit } = await import("@/lib/rate-limit"); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts b/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts index c5b3ba303..6acb80659 100644 --- a/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts @@ -168,6 +168,7 @@ beforeEach(() => { vi.mocked(prisma.extractedFact.count).mockResolvedValue(0 as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 5, resetAt: Date.now() + 60 * 60 * 1000, }); @@ -304,6 +305,7 @@ describe("POST /api/documents/inbound/[id]/extract — stored mode", () => { it("consumes the shared document-AI bucket honestly", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30 * 60_000, }); diff --git a/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts b/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts index 5b6540011..aa5b6c7d5 100644 --- a/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts @@ -128,6 +128,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 5, resetAt: Date.now() + 60 * 60 * 1000, }); @@ -222,6 +223,7 @@ describe("POST /api/documents/inbound/[id]/index — bucket honesty", () => { const resetAt = Date.now() + 47 * 60_000; vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt, }); @@ -309,6 +311,7 @@ describe("POST /api/documents/inbound/[id]/index — staging continuation", () = it("does not reach staging when the index itself was refused", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/export/__tests__/measurement-streaming.test.ts b/src/app/api/export/__tests__/measurement-streaming.test.ts index 3fedbe375..aa674b460 100644 --- a/src/app/api/export/__tests__/measurement-streaming.test.ts +++ b/src/app/api/export/__tests__/measurement-streaming.test.ts @@ -71,6 +71,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); diff --git a/src/app/api/export/__tests__/per-type-routes.test.ts b/src/app/api/export/__tests__/per-type-routes.test.ts index f29671ba1..f3ca60494 100644 --- a/src/app/api/export/__tests__/per-type-routes.test.ts +++ b/src/app/api/export/__tests__/per-type-routes.test.ts @@ -190,6 +190,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -229,6 +230,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -258,6 +260,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -289,6 +292,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -327,6 +331,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -370,6 +375,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -390,6 +396,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -425,6 +432,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -452,6 +460,7 @@ describe("GET /api/export/mood", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -492,6 +501,7 @@ describe("GET /api/export/full-backup", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -562,6 +572,7 @@ describe("GET /api/export/full-backup", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); diff --git a/src/app/api/export/__tests__/soft-delete-filter.test.ts b/src/app/api/export/__tests__/soft-delete-filter.test.ts index 329405629..bd4a5669c 100644 --- a/src/app/api/export/__tests__/soft-delete-filter.test.ts +++ b/src/app/api/export/__tests__/soft-delete-filter.test.ts @@ -130,6 +130,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); diff --git a/src/app/api/illness/episodes/[id]/day-logs/__tests__/route.test.ts b/src/app/api/illness/episodes/[id]/day-logs/__tests__/route.test.ts index 659d66159..6c2ae4d52 100644 --- a/src/app/api/illness/episodes/[id]/day-logs/__tests__/route.test.ts +++ b/src/app/api/illness/episodes/[id]/day-logs/__tests__/route.test.ts @@ -105,7 +105,10 @@ describe("GET /api/illness/episodes/{id}/day-logs — date-less list", () => { const findArgs = vi.mocked(prisma.illnessDayLog.findMany).mock.calls[0][0]; expect(findArgs).toMatchObject({ where: { episodeId: "ep-1", deletedAt: null }, - orderBy: { date: "desc" }, + // `date` is a calendar day and repeats within an episode, so the + // unique tiebreaker is what keeps `skip`-based paging from repeating + // one row and dropping another between pages. + orderBy: [{ date: "desc" }, { id: "desc" }], take: 60, skip: 0, }); @@ -126,7 +129,7 @@ describe("GET /api/illness/episodes/{id}/day-logs — date-less list", () => { const findArgs = vi.mocked(prisma.illnessDayLog.findMany).mock.calls[0][0]; expect(findArgs).toMatchObject({ - orderBy: { date: "asc" }, + orderBy: [{ date: "asc" }, { id: "asc" }], take: 10, skip: 20, }); diff --git a/src/app/api/illness/episodes/[id]/day-logs/route.ts b/src/app/api/illness/episodes/[id]/day-logs/route.ts index 9d01977df..2202f338f 100644 --- a/src/app/api/illness/episodes/[id]/day-logs/route.ts +++ b/src/app/api/illness/episodes/[id]/day-logs/route.ts @@ -88,7 +88,8 @@ export const GET = apiHandler( const [rows, total] = await Promise.all([ prisma.illnessDayLog.findMany({ where, - orderBy: { date: sortDir }, + // Unique tiebreaker — `date` is a calendar day and repeats. + orderBy: [{ date: sortDir }, { id: sortDir }], take: limit, skip: offset, include: dayLogSymptomInclude, diff --git a/src/app/api/insights/comprehensive/__tests__/route.test.ts b/src/app/api/insights/comprehensive/__tests__/route.test.ts index 372a313ea..e6dde951a 100644 --- a/src/app/api/insights/comprehensive/__tests__/route.test.ts +++ b/src/app/api/insights/comprehensive/__tests__/route.test.ts @@ -117,6 +117,7 @@ beforeEach(() => { // v1.15.20 — default to an allowing analytics-read budget. vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: true, + limit: 120, remaining: 119, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/insights/correlations/__tests__/route.test.ts b/src/app/api/insights/correlations/__tests__/route.test.ts index 0a110f9ed..aa9ddb6d4 100644 --- a/src/app/api/insights/correlations/__tests__/route.test.ts +++ b/src/app/api/insights/correlations/__tests__/route.test.ts @@ -103,6 +103,7 @@ beforeEach(() => { // v1.15.20 — default to an allowing analytics-read budget. vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: true, + limit: 120, remaining: 119, resetAt: Date.now() + 60_000, }); @@ -229,6 +230,7 @@ describe("GET /api/insights/correlations", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: false, + limit: 120, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index 55516a66d..5289fa0d1 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -188,6 +188,7 @@ beforeEach(() => { selfContextText.mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -366,6 +367,7 @@ describe("POST /api/insights/generate — rate limit (v1.4.16 A7.1)", () => { expect(limit).toBe(10); return { allowed: callCount <= 10, + limit: 10, remaining: Math.max(0, 10 - callCount), resetAt: Date.now() + 3600_000, }; @@ -392,6 +394,7 @@ describe("POST /api/insights/generate — rate limit (v1.4.16 A7.1)", () => { expect(limit).toBe(3); return { allowed: callCount <= 3, + limit: 3, remaining: Math.max(0, 3 - callCount), resetAt: Date.now() + 3600_000, }; @@ -673,6 +676,7 @@ describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", mockBriefinglessFreshCache(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 3600_000, }); @@ -691,6 +695,7 @@ describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", mockBriefinglessFreshCache(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 3600_000, }); diff --git a/src/app/api/internal/web-vitals/__tests__/route.test.ts b/src/app/api/internal/web-vitals/__tests__/route.test.ts index 81a792abf..e3e1ac303 100644 --- a/src/app/api/internal/web-vitals/__tests__/route.test.ts +++ b/src/app/api/internal/web-vitals/__tests__/route.test.ts @@ -50,6 +50,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -114,6 +115,7 @@ describe("POST /api/internal/web-vitals", () => { it("returns 429 when the rate-limit fires", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/labs/route.ts b/src/app/api/labs/route.ts index b420b745e..97fc344ce 100644 --- a/src/app/api/labs/route.ts +++ b/src/app/api/labs/route.ts @@ -105,7 +105,11 @@ export const GET = apiHandler(async (request: NextRequest) => { const [rows, total] = await Promise.all([ prisma.labResult.findMany({ where, - orderBy: { takenAt: sortDir }, + // Unique tiebreaker: `takenAt` is a day-resolution timestamp, so a + // panel drawn in one sitting ties across every analyte in it. Offset + // paging over a non-unique sort key can repeat a row on one page and + // drop another, invisibly to both sides. + orderBy: [{ takenAt: sortDir }, { id: sortDir }], take: limit, skip: offset, include: { diff --git a/src/app/api/mcp/oauth/__tests__/token.test.ts b/src/app/api/mcp/oauth/__tests__/token.test.ts index 54bf4e451..df17975e9 100644 --- a/src/app/api/mcp/oauth/__tests__/token.test.ts +++ b/src/app/api/mcp/oauth/__tests__/token.test.ts @@ -37,12 +37,14 @@ vi.mock("@/lib/auth/issue-token", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkAuthSurfaceRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 100, resetAt: Date.now() + 60_000, ip: "1.2.3.4", })), checkRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, })), @@ -103,6 +105,7 @@ beforeEach(() => { vi.clearAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); @@ -215,6 +218,7 @@ describe("authorization_code grant", () => { it("rejects a replayed code (single-use jti already claimed)", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/mcp/oauth/authorize/route.ts b/src/app/api/mcp/oauth/authorize/route.ts index 1abc2c847..04de08df8 100644 --- a/src/app/api/mcp/oauth/authorize/route.ts +++ b/src/app/api/mcp/oauth/authorize/route.ts @@ -302,11 +302,7 @@ export async function GET(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } @@ -407,11 +403,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/mcp/oauth/register/route.ts b/src/app/api/mcp/oauth/register/route.ts index 046453158..aa5d76a83 100644 --- a/src/app/api/mcp/oauth/register/route.ts +++ b/src/app/api/mcp/oauth/register/route.ts @@ -80,11 +80,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/mcp/oauth/token/route.ts b/src/app/api/mcp/oauth/token/route.ts index e9493db20..a82b732a5 100644 --- a/src/app/api/mcp/oauth/token/route.ts +++ b/src/app/api/mcp/oauth/token/route.ts @@ -222,11 +222,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/measurement-reminders/[id]/__tests__/skip-snooze-history.test.ts b/src/app/api/measurement-reminders/[id]/__tests__/skip-snooze-history.test.ts index 8483ec1f6..ecafa0b85 100644 --- a/src/app/api/measurement-reminders/[id]/__tests__/skip-snooze-history.test.ts +++ b/src/app/api/measurement-reminders/[id]/__tests__/skip-snooze-history.test.ts @@ -303,7 +303,10 @@ describe("GET /api/measurement-reminders/[id]/history", () => { }); expect(body.data.meta).toEqual({ total: 2, limit: 10, offset: 0 }); const query = eventFindManyMock.mock.calls[0][0]; - expect(query.orderBy).toEqual({ occurredAt: "desc" }); + // The trailing `id` is the unique tiebreaker: several reminder events + // can share one `occurredAt`, and offset paging over a tied key repeats + // and drops rows between pages. + expect(query.orderBy).toEqual([{ occurredAt: "desc" }, { id: "desc" }]); expect(query.where).toEqual({ reminderId: "r1", userId: "u1" }); }); diff --git a/src/app/api/measurement-reminders/[id]/history/route.ts b/src/app/api/measurement-reminders/[id]/history/route.ts index 19872c4fc..df16f4a93 100644 --- a/src/app/api/measurement-reminders/[id]/history/route.ts +++ b/src/app/api/measurement-reminders/[id]/history/route.ts @@ -45,7 +45,8 @@ export const GET = apiHandler( const [rows, total] = await Promise.all([ prisma.measurementReminderEvent.findMany({ where, - orderBy: { occurredAt: "desc" }, + // Unique tiebreaker — several reminder events can share one instant. + orderBy: [{ occurredAt: "desc" }, { id: "desc" }], take: limit, skip: offset, }), diff --git a/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts b/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts index a46987f29..3f766d9fb 100644 --- a/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts +++ b/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts @@ -189,6 +189,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts b/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts index b5e36298c..7597199f4 100644 --- a/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts +++ b/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts @@ -186,6 +186,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts b/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts index 9b551c5ac..e7eb79f74 100644 --- a/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts +++ b/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts @@ -159,6 +159,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(session as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts b/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts index 01f5cb72d..a36974a0c 100644 --- a/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts +++ b/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts @@ -114,6 +114,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts b/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts index fa834286f..83a7416d5 100644 --- a/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts +++ b/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts @@ -194,6 +194,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -271,6 +272,7 @@ describe("POST /api/measurements/batch — syncTrigger diagnostic (iOS #66)", () vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts b/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts index 5c79d949e..8fdeb46da 100644 --- a/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts +++ b/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts @@ -148,6 +148,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts b/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts index e7369d833..a68fdd012 100644 --- a/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts +++ b/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts @@ -151,6 +151,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/route.ts b/src/app/api/measurements/route.ts index d8f76ada0..6dcfe5456 100644 --- a/src/app/api/measurements/route.ts +++ b/src/app/api/measurements/route.ts @@ -190,7 +190,12 @@ export const GET = apiHandler(async (request: NextRequest) => { measuredAt: { gte: dayStart, lt: dayEnd }, deletedAt: null, }, - orderBy: { measuredAt: sortDir }, + // Same tiebreaker as the paged list. This one slices in memory rather + // than with `skip`, so there is no page drift to prevent — but two + // identical requests would still be free to render a tied block in + // different orders, and a drill-down that reshuffles on refresh reads + // as data changing. + orderBy: [{ measuredAt: sortDir }, { id: sortDir }], // v1.4.38 — the 1000-row cap now lives on the validator (refine // on `(limit, dayKey)` returning 422 when a caller asks for // more). The route reads `limit` straight through because the @@ -544,7 +549,17 @@ export const GET = apiHandler(async (request: NextRequest) => { const [measurements, total] = await Promise.all([ prisma.measurement.findMany({ where, - orderBy: { [sortBy]: sortDir }, + // `{ id: sortDir }` is the tiebreaker, not decoration. `measuredAt` + // is not unique here and ties are the normal case. Only the exact + // tuple `(userId, type, measuredAt, source, sleepStage)` is unique, + // so a tie survives across `type` (the `stats::YYYY-MM-DD` + // daily-rollup convention puts every daily total at one instant), + // across `source` (two writers landing on the same instant) and + // across `sleepStage`. Without a unique secondary key Postgres is + // free to order the tied block differently between the query for page + // N and the query for page N+1, so a client paging its own history + // silently gets one row twice and never gets another. + orderBy: [{ [sortBy]: sortDir }, { id: sortDir }], take: limit, skip: offset, }), diff --git a/src/app/api/medications/[id]/compliance/__tests__/route.test.ts b/src/app/api/medications/[id]/compliance/__tests__/route.test.ts index 4b6de773f..128bcfe87 100644 --- a/src/app/api/medications/[id]/compliance/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/compliance/__tests__/route.test.ts @@ -46,6 +46,7 @@ vi.mock("@/lib/cache/server-cache", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -139,6 +140,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -416,6 +418,7 @@ describe("GET /api/medications/[id]/compliance — caching and limits", () => { it("returns 429 when the per-user rate limit is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts b/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts index ccc4f707d..4a5c8fed8 100644 --- a/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/medications/route-guards", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -92,6 +93,7 @@ beforeEach(() => { vi.mocked(assertMedicationOwnership).mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -134,6 +136,7 @@ describe("GET /api/medications/[id]/dose-history", () => { it("returns 429 when the per-user cap is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/glp1/__tests__/route.test.ts b/src/app/api/medications/[id]/glp1/__tests__/route.test.ts index 5e6bbad29..3d52191ee 100644 --- a/src/app/api/medications/[id]/glp1/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/glp1/__tests__/route.test.ts @@ -38,6 +38,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -85,6 +86,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -251,6 +253,7 @@ describe("POST /api/medications/[id]/glp1", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/intake/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/__tests__/route.test.ts index 2555f8d7b..60569dcb6 100644 --- a/src/app/api/medications/[id]/intake/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/__tests__/route.test.ts @@ -208,6 +208,27 @@ describe("GET /api/medications/[id]/intake — status filter", () => { }); }); + it("orders on a unique tiebreaker, keeping NULLS LAST on the takenAt arm", async () => { + // Scheduled slots share an instant by construction — a twice-daily + // medication puts every morning dose on the same `scheduledFor` — so + // offset paging over the sort key alone can repeat a row on one page and + // drop another. The `takenAt` arm keeps its NULLS LAST pin as well: the + // tiebreaker must not push skipped rows back to the top. + await GET(makeRequest("sortBy=takenAt&sortDir=desc"), ROUTE_PARAMS); + expect( + vi.mocked(prisma.medicationIntakeEvent.findMany).mock.calls[0][0] + ?.orderBy, + ).toEqual([{ takenAt: { sort: "desc", nulls: "last" } }, { id: "desc" }]); + + vi.mocked(prisma.medicationIntakeEvent.findMany).mockClear(); + + await GET(makeRequest("sortBy=scheduledFor&sortDir=asc"), ROUTE_PARAMS); + expect( + vi.mocked(prisma.medicationIntakeEvent.findMany).mock.calls[0][0] + ?.orderBy, + ).toEqual([{ scheduledFor: "asc" }, { id: "asc" }]); + }); + it("rejects unknown status values with a 422", async () => { const res = await GET(makeRequest("status=junk"), ROUTE_PARAMS); expect(res.status).toBe(422); diff --git a/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts index 381421305..c087df6f9 100644 --- a/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts @@ -39,6 +39,7 @@ vi.mock("@/lib/medications/route-guards", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -124,6 +125,7 @@ beforeEach(() => { vi.mocked(assertMedicationOwnership).mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -239,6 +241,7 @@ describe("POST /api/medications/[id]/intake/bulk-delete — F-1 H-6 rate limit", it("returns 429 when the per-user cap is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts index f4c5a0664..a66164c05 100644 --- a/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts @@ -97,6 +97,7 @@ beforeEach(() => { vi.mocked(auditLog).mockResolvedValue(undefined as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -407,6 +408,7 @@ describe("POST /api/medications/[id]/intake/import — future bound and limiter" it("refuses with 429 once the import limiter trips", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/intake/route.ts b/src/app/api/medications/[id]/intake/route.ts index 6d684b667..635ee69f1 100644 --- a/src/app/api/medications/[id]/intake/route.ts +++ b/src/app/api/medications/[id]/intake/route.ts @@ -729,10 +729,17 @@ export const GET = apiHandler( // the history view. Pinning them last keeps the descending order // reading today → yesterday → … with real timestamps first. Other // sort columns are non-null so they keep the simple shape. + // The trailing `{ id: sortDir }` is the unique tiebreaker. Scheduled + // slots share an instant by construction — a twice-daily medication puts + // every morning dose on the same `scheduledFor` — so offset paging over + // the primary key alone can repeat a row on one page and drop another. const orderBy = sortBy === "takenAt" - ? { takenAt: { sort: sortDir, nulls: "last" as const } } - : { [sortBy]: sortDir }; + ? [ + { takenAt: { sort: sortDir, nulls: "last" as const } }, + { id: sortDir }, + ] + : [{ [sortBy]: sortDir }, { id: sortDir }]; const [events, total] = await Promise.all([ prisma.medicationIntakeEvent.findMany({ diff --git a/src/app/api/medications/[id]/inventory/__tests__/route.test.ts b/src/app/api/medications/[id]/inventory/__tests__/route.test.ts index cd9d4251a..5231ae2af 100644 --- a/src/app/api/medications/[id]/inventory/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/inventory/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -87,6 +88,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -253,6 +255,7 @@ describe("POST /api/medications/[id]/inventory", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts b/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts index e30b8372a..ba27f803e 100644 --- a/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts @@ -31,6 +31,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -84,6 +85,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -298,6 +300,7 @@ describe("POST /api/medications/[id]/side-effects", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/compliance/__tests__/route.test.ts b/src/app/api/medications/compliance/__tests__/route.test.ts index b947205d6..8490762bb 100644 --- a/src/app/api/medications/compliance/__tests__/route.test.ts +++ b/src/app/api/medications/compliance/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/cache/server-cache", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -106,6 +107,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -178,6 +180,7 @@ describe("GET /api/medications/compliance", () => { it("returns 429 when the per-user rate limit is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/extract/__tests__/route.test.ts b/src/app/api/medications/extract/__tests__/route.test.ts index b80464eb3..c01d578ee 100644 --- a/src/app/api/medications/extract/__tests__/route.test.ts +++ b/src/app/api/medications/extract/__tests__/route.test.ts @@ -110,6 +110,7 @@ beforeEach(() => { vi.clearAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 60_000, }); @@ -152,6 +153,7 @@ describe("POST /api/medications/extract — rate limit", () => { vi.mocked(requireAuth).mockResolvedValue(AUTH_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/mental-health/assessments/__tests__/route.test.ts b/src/app/api/mental-health/assessments/__tests__/route.test.ts index 1d780f051..b6311acfc 100644 --- a/src/app/api/mental-health/assessments/__tests__/route.test.ts +++ b/src/app/api/mental-health/assessments/__tests__/route.test.ts @@ -15,7 +15,11 @@ import { NextRequest } from "next/server"; vi.mock("@/lib/db", () => ({ prisma: { appSettings: { findUnique: vi.fn().mockResolvedValue(null) }, - mentalHealthAssessment: { create: vi.fn(), findFirst: vi.fn() }, + mentalHealthAssessment: { + create: vi.fn(), + findFirst: vi.fn(), + findMany: vi.fn(), + }, measurement: { create: vi.fn().mockResolvedValue({}) }, }, })); @@ -60,7 +64,7 @@ vi.mock("next/headers", () => ({ })), })); -import { POST } from "../route"; +import { GET, POST } from "../route"; import { enqueueReminderSatisfy } from "@/lib/jobs/reminder-satisfy"; import { getSession } from "@/lib/auth/session"; import { requireModuleEnabled } from "@/lib/modules/gate"; @@ -96,6 +100,7 @@ beforeEach(() => { vi.mocked(requireModuleEnabled).mockResolvedValue({ enabled: true }); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now(), }); @@ -448,3 +453,25 @@ describe("POST /api/mental-health/assessments — external-id stability floor", expect(prisma.mentalHealthAssessment.create).toHaveBeenCalledTimes(1); }); }); + +describe("GET /api/mental-health/assessments", () => { + it("orders the history on a unique tiebreaker", async () => { + vi.mocked(prisma.mentalHealthAssessment.findMany).mockResolvedValue( + [] as never, + ); + + const res = await (GET as unknown as (r: NextRequest) => Promise)( + new NextRequest( + new URL("http://localhost/api/mental-health/assessments"), + ), + ); + expect(res.status).toBe(200); + + // `takenAt` is day-resolution, so two instruments answered in one sitting + // tie; without the unique secondary key offset paging can repeat a row on + // one page and drop another. + const findArgs = vi.mocked(prisma.mentalHealthAssessment.findMany).mock + .calls[0][0] as { orderBy: unknown }; + expect(findArgs.orderBy).toEqual([{ takenAt: "desc" }, { id: "desc" }]); + }); +}); diff --git a/src/app/api/mental-health/assessments/route.ts b/src/app/api/mental-health/assessments/route.ts index a37b223b3..4229ecfd3 100644 --- a/src/app/api/mental-health/assessments/route.ts +++ b/src/app/api/mental-health/assessments/route.ts @@ -123,7 +123,9 @@ export const GET = apiHandler(async (request: NextRequest) => { deletedAt: null, ...(instrument ? { instrument } : {}), }, - orderBy: { takenAt: "desc" }, + // Unique tiebreaker — `takenAt` is day-resolution and two instruments + // taken in one sitting tie. + orderBy: [{ takenAt: "desc" }, { id: "desc" }], take: limit ?? 100, skip: offset ?? 0, select: { diff --git a/src/app/api/nutrients/water/__tests__/route.test.ts b/src/app/api/nutrients/water/__tests__/route.test.ts index 051a16d14..354ce7cc2 100644 --- a/src/app/api/nutrients/water/__tests__/route.test.ts +++ b/src/app/api/nutrients/water/__tests__/route.test.ts @@ -101,6 +101,7 @@ beforeEach(() => { vi.mocked(requireModuleEnabled).mockResolvedValue({ enabled: true } as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -137,6 +138,7 @@ describe("POST /api/nutrients/water", () => { it("returns 429 when the rate limit is exceeded", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/onboarding/__tests__/step.test.ts b/src/app/api/onboarding/__tests__/step.test.ts index 1700626a3..5856891f4 100644 --- a/src/app/api/onboarding/__tests__/step.test.ts +++ b/src/app/api/onboarding/__tests__/step.test.ts @@ -36,6 +36,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -79,6 +80,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -243,6 +245,7 @@ describe("POST /api/onboarding/step — rate limit", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/user/avatar/__tests__/route.test.ts b/src/app/api/user/avatar/__tests__/route.test.ts index 0cfb500ee..72e005dce 100644 --- a/src/app/api/user/avatar/__tests__/route.test.ts +++ b/src/app/api/user/avatar/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }), @@ -89,6 +90,7 @@ beforeEach(() => { // Default rate-limit pass-through; per-test overrides drop in below. vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); @@ -210,6 +212,7 @@ describe("POST /api/user/avatar", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/workouts/__tests__/batch-create.test.ts b/src/app/api/workouts/__tests__/batch-create.test.ts index 05a5dba93..37e971e2a 100644 --- a/src/app/api/workouts/__tests__/batch-create.test.ts +++ b/src/app/api/workouts/__tests__/batch-create.test.ts @@ -145,6 +145,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -218,6 +219,7 @@ describe("POST /api/workouts/batch — auth + size + rate-limit", () => { it("returns 429 when the rate-limit gate blocks the call", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/mcp/__tests__/route.test.ts b/src/app/mcp/__tests__/route.test.ts index 465939dc1..d132bd115 100644 --- a/src/app/mcp/__tests__/route.test.ts +++ b/src/app/mcp/__tests__/route.test.ts @@ -156,6 +156,7 @@ beforeEach(() => { vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(checkMcpRateLimit).mockResolvedValue({ allowed: true, + limit: 120, remaining: 119, resetAt: Date.now() + 60_000, }); @@ -352,6 +353,7 @@ describe("/mcp — rate limit", () => { validToken(); vi.mocked(checkMcpRateLimit).mockResolvedValue({ allowed: false, + limit: 120, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/mcp/route.ts b/src/app/mcp/route.ts index ae4a7dceb..c71635dc8 100644 --- a/src/app/mcp/route.ts +++ b/src/app/mcp/route.ts @@ -55,6 +55,7 @@ import { annotate } from "@/lib/logging/context"; import { isModuleEnabled } from "@/lib/modules/gate"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkMcpRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import type { RateLimitSnapshot } from "@/lib/rate-limit-context"; import { isMcpOriginConfigured, resolveBaseOrigin, @@ -114,11 +115,9 @@ function moduleDisabled(): Response { } /** 429 with the standard rate-limit headers. */ -function tooManyRequests(resetAt: number, remaining: number): Response { +function tooManyRequests(rl: RateLimitSnapshot): Response { const response = Response.json({ error: "rate_limited" }, { status: 429 }); - for (const [k, v] of Object.entries( - rateLimitHeaders({ allowed: false, remaining, resetAt }), - )) { + for (const [k, v] of Object.entries(rateLimitHeaders(rl))) { response.headers.set(k, v); } return response; @@ -179,7 +178,7 @@ async function handleMcp(request: Request): Promise { const rl = await checkMcpRateLimit(ctx.binding); if (!rl.allowed) { annotate({ action: { name: "mcp.rate_limited" } }); - return tooManyRequests(rl.resetAt, rl.remaining); + return tooManyRequests(rl); } // 3. MODULE GATE — off by default; hide the surface entirely when off. diff --git a/src/lib/__tests__/api-handler-429-dressing.test.ts b/src/lib/__tests__/api-handler-429-dressing.test.ts new file mode 100644 index 000000000..dba3e70cd --- /dev/null +++ b/src/lib/__tests__/api-handler-429-dressing.test.ts @@ -0,0 +1,191 @@ +/** + * Which 429s `apiHandler` is allowed to dress with rate-limit headers. + * + * The wrapper attaches `Retry-After` and the `X-RateLimit` triple to any 429 + * that leaves a handler without them, and it takes the numbers from the + * request-scoped limiter capture. That is only honest when a limiter actually + * refused: an AI budget that is spent for the day, an hourly generation quota + * and a provider-side 429 relayed onward are all 429s no bucket produced, and + * describing them with a bucket that still has room tells the client to retry + * into a ceiling that has not moved. + * + * So both directions are pinned here: a refusal from the limiter gets the + * headers, a 429 from anything else gets none. + */ +import { NextRequest, NextResponse } from "next/server"; +import { describe, expect, it, vi } from "vitest"; + +// --- Mocks must be hoisted before importing the module under test. --- + +vi.mock("@/lib/db", () => ({ + prisma: {}, +})); + +vi.mock("@/lib/auth/session", () => ({ + getSession: vi.fn(), +})); + +vi.mock("@/lib/auth/hmac", () => ({ + hashToken: vi.fn(), +})); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/transports", () => ({ + emitIfSampled: vi.fn(), +})); + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { apiHandler } from "@/lib/api-handler"; +import { captureRateLimitResult } from "@/lib/rate-limit-context"; + +const RATE_LIMIT_HEADERS = [ + "Retry-After", + "X-RateLimit-Limit", + "X-RateLimit-Remaining", + "X-RateLimit-Reset", +] as const; + +function refusal(): NextResponse { + return NextResponse.json({ data: null, error: "Too many" }, { status: 429 }); +} + +function request(): NextRequest { + return new NextRequest("http://localhost/api/test", { method: "POST" }); +} + +describe("apiHandler — 429 header dressing", () => { + it("dresses a 429 the limiter refused", async () => { + const resetAt = Date.now() + 30_000; + const route = apiHandler(async (_request: NextRequest) => { + captureRateLimitResult({ + allowed: false, + limit: 60, + remaining: 0, + resetAt, + }); + return refusal(); + }); + + const res = await route(request()); + + expect(res.status).toBe(429); + expect(res.headers.get("X-RateLimit-Limit")).toBe("60"); + expect(res.headers.get("X-RateLimit-Remaining")).toBe("0"); + expect(res.headers.get("X-RateLimit-Reset")).toBe( + new Date(resetAt).toISOString(), + ); + expect(Number(res.headers.get("Retry-After"))).toBeGreaterThan(0); + }); + + it("leaves a 429 no limiter produced undressed", async () => { + const route = apiHandler(async (_request: NextRequest) => refusal()); + + const res = await route(request()); + + expect(res.status).toBe(429); + for (const name of RATE_LIMIT_HEADERS) { + expect(res.headers.get(name)).toBeNull(); + } + }); + + it("leaves a 429 undressed when every bucket the handler consulted passed", async () => { + // The shape of the AI surfaces: a per-minute limiter lets the request + // through, then a daily budget or an upstream provider refuses it. The + // window the limiter cleared says nothing about when the budget resets. + const route = apiHandler(async (_request: NextRequest) => { + captureRateLimitResult({ + allowed: true, + limit: 10, + remaining: 9, + resetAt: Date.now() + 60_000, + }); + return refusal(); + }); + + const res = await route(request()); + + expect(res.status).toBe(429); + for (const name of RATE_LIMIT_HEADERS) { + expect(res.headers.get(name)).toBeNull(); + } + }); + + it("reports the bucket that refused when the handler consulted two", async () => { + const resetAt = Date.now() + 45_000; + const route = apiHandler(async (_request: NextRequest) => { + captureRateLimitResult({ + allowed: true, + limit: 600, + remaining: 599, + resetAt: Date.now() + 3_600_000, + }); + captureRateLimitResult({ + allowed: false, + limit: 5, + remaining: 0, + resetAt, + }); + return refusal(); + }); + + const res = await route(request()); + + expect(res.headers.get("X-RateLimit-Limit")).toBe("5"); + expect(res.headers.get("X-RateLimit-Reset")).toBe( + new Date(resetAt).toISOString(), + ); + }); + + it("keeps the headers a handler attached itself", async () => { + const route = apiHandler(async (_request: NextRequest) => { + captureRateLimitResult({ + allowed: false, + limit: 60, + remaining: 0, + resetAt: Date.now() + 30_000, + }); + return NextResponse.json( + { data: null, error: "Too many" }, + { + status: 429, + headers: { "Retry-After": "900", "X-RateLimit-Limit": "3" }, + }, + ); + }); + + const res = await route(request()); + + expect(res.headers.get("Retry-After")).toBe("900"); + expect(res.headers.get("X-RateLimit-Limit")).toBe("3"); + }); + + it("leaves a non-429 response alone even after a refusal was captured", async () => { + const route = apiHandler(async (_request: NextRequest) => { + captureRateLimitResult({ + allowed: false, + limit: 60, + remaining: 0, + resetAt: Date.now() + 30_000, + }); + return NextResponse.json({ data: { ok: true }, error: null }); + }); + + const res = await route(request()); + + expect(res.status).toBe(200); + for (const name of RATE_LIMIT_HEADERS) { + expect(res.headers.get(name)).toBeNull(); + } + }); +}); diff --git a/src/lib/__tests__/auth-error-codes.test.ts b/src/lib/__tests__/auth-error-codes.test.ts new file mode 100644 index 000000000..eb608b7e9 --- /dev/null +++ b/src/lib/__tests__/auth-error-codes.test.ts @@ -0,0 +1,249 @@ +/** + * The generic auth gates refuse with a stable machine code. + * + * This decides the single most important branch a native client makes: + * refresh the token and retry, drop the session and show the login screen, or + * stop retrying because the credential will never work. Before these codes the + * only way to tell those apart was to string-match the English sentence "Token + * expired", so a rewording — or a localisation pass — would have turned a + * recoverable refresh into a forced logout for every already-shipped build. + * + * So the assertions here are on the CODE, and deliberately not on the prose. + * The one place the sentence is pinned is the pair of cases proving the + * envelope is otherwise unchanged: `error` still carries a sentence, and an + * HttpError thrown without a code still serialises without a `meta`. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + apiToken: { findUnique: vi.fn(), update: vi.fn() }, + user: { findUnique: vi.fn() }, + session: { findUnique: vi.fn() }, + webauthnMfaCredential: { count: vi.fn() }, + passkey: { count: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +const headersGet = vi.fn<(name: string) => string | null>(); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: headersGet })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { + apiHandler, + AUTH_ERROR_CODES, + HttpError, + MFA_STEP_UP_MAX_AGE_SECONDS, + requireAdmin, + requireAuth, + requireFreshMfa, +} from "../api-handler"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { hashToken } from "@/lib/auth/hmac"; +import { auditLog } from "@/lib/auth/audit"; + +const FAKE_HASH = "deadbeefcafef00d"; +const RAW_TOKEN = "hlk_" + "a".repeat(64); +const FAKE_USER = { + id: "user-1", + role: "USER" as const, + username: "testuser", + email: "user@example.com", +}; + +function setBearerHeader(value: string | null): void { + headersGet.mockReset(); + headersGet.mockImplementation((name: string) => + name.toLowerCase() === "authorization" ? value : null, + ); +} + +function mockSession(user: unknown) { + vi.mocked(getSession).mockResolvedValue({ + session: { id: "sess-1", expiresAt: new Date(Date.now() + 1e6) }, + user, + } as never); +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(hashToken).mockReturnValue(FAKE_HASH); + vi.mocked(prisma.apiToken.update).mockResolvedValue({} as never); + vi.mocked(auditLog).mockResolvedValue(undefined as never); +}); + +/** Run a gate through `apiHandler` and read the envelope it produced. */ +async function envelopeFor(gate: () => Promise): Promise<{ + status: number; + body: { data: null; error: string; meta?: { errorCode?: string } }; +}> { + const handler = apiHandler(async (_request: Request) => { + await gate(); + return new Response(null, { status: 204 }); + }); + const res = (await handler( + new Request("http://localhost/api/anything"), + )) as Response; + return { + status: res.status, + body: (await res.json()) as { + data: null; + error: string; + meta?: { errorCode?: string }; + }, + }; +} + +describe("auth failures carry a machine-readable errorCode", () => { + it("names auth.missing when no credential was presented at all", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(null); + + const { status, body } = await envelopeFor(() => requireAuth()); + + expect(status).toBe(401); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing); + // The envelope is otherwise what it always was. + expect(body.data).toBeNull(); + expect(body.error).toBe("Not authenticated"); + }); + + it("names auth.token.expired so the client refreshes instead of logging out", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(`Bearer ${RAW_TOKEN}`); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "token-expired", + userId: "user-1", + permissions: ["*"], + revoked: false, + expiresAt: new Date(Date.now() - 60_000), + } as never); + + const { status, body } = await envelopeFor(() => requireAuth()); + + expect(status).toBe(401); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.expired); + }); + + it("names auth.token.invalid for an unknown token", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(`Bearer ${RAW_TOKEN}`); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue(null); + + const { status, body } = await envelopeFor(() => requireAuth()); + + expect(status).toBe(401); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.invalid); + }); + + it("names auth.token.invalid for a revoked token — not the expired code", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(`Bearer ${RAW_TOKEN}`); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "token-revoked", + userId: "user-1", + permissions: ["*"], + revoked: true, + expiresAt: null, + } as never); + + const { body } = await envelopeFor(() => requireAuth()); + + // Refreshing a revoked token loops forever; the two must not share a code. + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.invalid); + expect(body.meta?.errorCode).not.toBe(AUTH_ERROR_CODES.expired); + }); + + it("names auth.scope.insufficient when the token's scope does not reach the route", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(`Bearer ${RAW_TOKEN}`); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "token-narrow", + userId: "user-1", + permissions: ["something:else"], + revoked: false, + expiresAt: null, + } as never); + + const { status, body } = await envelopeFor(() => + requireAuth("medication:ingest"), + ); + + expect(status).toBe(403); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.scope); + }); + + it("names auth.scope.insufficient when a narrow token hits a route that declared none", async () => { + vi.mocked(getSession).mockResolvedValue(null); + setBearerHeader(`Bearer ${RAW_TOKEN}`); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "token-undeclared", + userId: "user-1", + permissions: ["health:read"], + revoked: false, + expiresAt: null, + } as never); + + const { status, body } = await envelopeFor(() => requireAuth()); + + expect(status).toBe(403); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.scope); + }); + + it("names auth.missing and auth.admin.required on the admin gate", async () => { + vi.mocked(getSession).mockResolvedValue(null); + const anonymous = await envelopeFor(() => requireAdmin()); + expect(anonymous.status).toBe(401); + expect(anonymous.body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing); + + mockSession(FAKE_USER); + const nonAdmin = await envelopeFor(() => requireAdmin()); + expect(nonAdmin.status).toBe(403); + expect(nonAdmin.body.meta?.errorCode).toBe(AUTH_ERROR_CODES.admin); + }); + + it("names auth.missing on the step-up gate with no session", async () => { + vi.mocked(getSession).mockResolvedValue(null); + + const { status, body } = await envelopeFor(() => + requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS), + ); + + expect(status).toBe(401); + expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing); + }); + + it("leaves an HttpError thrown without a code exactly as it was", async () => { + const handler = apiHandler(async (_request: Request) => { + throw new HttpError(404, "Not found"); + }); + const res = (await handler( + new Request("http://localhost/api/anything"), + )) as Response; + + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ data: null, error: "Not found" }); + }); + + it("keeps every code distinct and dot.case", () => { + const codes = Object.values(AUTH_ERROR_CODES); + expect(new Set(codes).size).toBe(codes.length); + for (const code of codes) { + expect(code).toMatch(/^auth(\.[a-z_]+)+$/); + } + }); +}); diff --git a/src/lib/__tests__/rate-limit-headers.test.ts b/src/lib/__tests__/rate-limit-headers.test.ts new file mode 100644 index 000000000..248cd2aad --- /dev/null +++ b/src/lib/__tests__/rate-limit-headers.test.ts @@ -0,0 +1,144 @@ +/** + * The rate-limit response headers and the request-scoped verdict capture. + * + * The headers are what a refused client backs off against, so the shape is + * pinned here rather than left to whichever route happens to be exercised: + * `Retry-After` in whole seconds rounding UP, the cap and the remainder as + * plain integers, and the reset instant in the ISO form it has always carried. + * + * The capture half is what lets `apiHandler` dress a 429 that a handler built + * without headers. It keeps refusals and nothing else: a handler that clears + * one bucket and is refused by the next reports the bucket that refused it, + * and a request every bucket let through reports nothing at all, because a + * 429 it ends up answering came from some other ceiling. + */ +import { describe, expect, it } from "vitest"; + +import { + capturedRateLimit, + captureRateLimitResult, + rateLimitResponseHeaders, + runWithRateLimitCapture, +} from "@/lib/rate-limit-context"; + +describe("rateLimitResponseHeaders", () => { + it("emits Retry-After, the cap, the remainder and the reset instant", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 60, remaining: 0, resetAt: now + 30_000 }, + now, + ); + + expect(headers).toEqual({ + "Retry-After": "30", + "X-RateLimit-Limit": "60", + "X-RateLimit-Remaining": "0", + "X-RateLimit-Reset": new Date(now + 30_000).toISOString(), + }); + }); + + it("rounds Retry-After up so the named delay outlasts the window", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 10, remaining: 0, resetAt: now + 1 }, + now, + ); + // One millisecond left still means "wait a second", never "retry now" — + // a floored zero would send the client straight back into the same window. + expect(headers["Retry-After"]).toBe("1"); + }); + + it("names at least a second even when the window already reads as expired", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 10, remaining: 0, resetAt: now - 5_000 }, + now, + ); + // `resetAt` is the database clock and `now` is this process's, so a row + // that still refuses can read as expired here. Zero would say "retry now" + // beside a refusal and turn the client into a hot loop. + expect(headers["Retry-After"]).toBe("1"); + }); + + it("keeps X-RateLimit-Reset in the ISO form already on the wire", () => { + const resetAt = Date.UTC(2026, 4, 17, 8, 30, 0); + const headers = rateLimitResponseHeaders({ + limit: 120, + remaining: 7, + resetAt, + }); + expect(headers["X-RateLimit-Reset"]).toBe("2026-05-17T08:30:00.000Z"); + }); +}); + +describe("request-scoped rate-limit capture", () => { + it("reads null outside a request scope", () => { + captureRateLimitResult({ + allowed: false, + limit: 5, + remaining: 0, + resetAt: Date.now(), + }); + expect(capturedRateLimit()).toBeNull(); + }); + + it("reports nothing when every bucket passed, so a 429 from elsewhere stays undressed", () => { + runWithRateLimitCapture(() => { + captureRateLimitResult({ + allowed: true, + limit: 5, + remaining: 4, + resetAt: 1_000, + }); + captureRateLimitResult({ + allowed: true, + limit: 9, + remaining: 8, + resetAt: 2_000, + }); + // Passing every bucket is exactly the state in which a 429 did not come + // from the limiter. Rendering the passing verdict would put + // `X-RateLimit-Remaining: 8` beside a refusal and a delay measured + // against a window that has nothing to do with it. + expect(capturedRateLimit()).toBeNull(); + }); + }); + + it("reports the bucket that refused, not one that passed after it", () => { + runWithRateLimitCapture(() => { + captureRateLimitResult({ + allowed: false, + limit: 60, + remaining: 0, + resetAt: 5_000, + }); + captureRateLimitResult({ + allowed: true, + limit: 120, + remaining: 100, + resetAt: 9_000, + }); + expect(capturedRateLimit()).toMatchObject({ limit: 60, resetAt: 5_000 }); + }); + }); + + it("gives each scope its own cell", async () => { + const first = runWithRateLimitCapture(async () => { + captureRateLimitResult({ + allowed: false, + limit: 1, + remaining: 0, + resetAt: 111, + }); + await Promise.resolve(); + return capturedRateLimit(); + }); + const second = runWithRateLimitCapture(async () => { + await Promise.resolve(); + return capturedRateLimit(); + }); + + expect((await first)?.resetAt).toBe(111); + expect(await second).toBeNull(); + }); +}); diff --git a/src/lib/api-errors.ts b/src/lib/api-errors.ts index 58597c667..29aaf9a22 100644 --- a/src/lib/api-errors.ts +++ b/src/lib/api-errors.ts @@ -14,17 +14,56 @@ import { RECORD_FENCE_ERROR_CODE } from "./sharing/record-session-fence-contract /** * Custom error class for HTTP errors with status codes. * Throw inside apiHandler-wrapped routes to return a JSON error response. + * + * `errorCode` is optional and lands in `meta.errorCode` when set — the same + * field the step-up, consent, module and sharing gates already use. An + * HttpError without one serialises exactly as before. */ export class HttpError extends Error { constructor( public statusCode: number, message: string, + public errorCode?: string, ) { super(message); this.name = "HttpError"; } } +/** + * The stable codes the generic auth gates refuse with. + * + * These decide the single most important branch a native client makes: + * refresh the token and retry, drop the session and show the login screen, or + * stop retrying because this credential will never work. Until they existed + * the only way to tell those apart was to string-match the English sentence + * "Token expired", so a rewording — or a localisation pass — would have turned + * a recoverable refresh into a forced logout for every shipped build. + * + * Dot.case, matching the majority convention and the `auth.refresh.*` / + * `auth.stepup.*` families already on the wire. Add to this list rather than + * inventing a second field. + */ +export const AUTH_ERROR_CODES = { + /** No credential at all: no session cookie and no Bearer header. */ + missing: "auth.missing", + /** The Bearer token was valid once and its expiry has passed. Refresh. */ + expired: "auth.token.expired", + /** Unknown, revoked, or belonging to a user that no longer exists. */ + invalid: "auth.token.invalid", + /** Authenticated, but the credential's scope does not reach this route. */ + scope: "auth.scope.insufficient", + /** Authenticated, but not an admin. Cookie-only surface; never a token. */ + admin: "auth.admin.required", + /** + * The second factor presented on a step-up flow did not verify — a wrong + * TOTP digit, a replayed one, or a recovery code that is spent. The session + * itself is untouched: the client re-prompts for a code rather than signing + * the user out, which is the opposite of what the other 401 codes ask for. + */ + mfaCode: "auth.mfa.code_invalid", +} as const; + /** * v1.36.0 — the two refusals the acting-account resolver can raise. * diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 63f6cdf28..5478c3a88 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -41,6 +41,11 @@ import { assertRecordSessionFence, attachRecordContextEcho, } from "./sharing/record-session-fence"; +import { + capturedRateLimit, + rateLimitResponseHeaders, + runWithRateLimitCapture, +} from "./rate-limit-context"; /** * HTTP methods a read-only credential may use on the REST surface. A request @@ -76,6 +81,7 @@ export function isMcpAudienceToken(permissions: readonly string[]): boolean { // the api-handler ↔ record-session-fence import cycle) and are re-exported // here so the hundreds of existing importers read exactly as before. import { + AUTH_ERROR_CODES, HttpError, SharingAuthError, SharingAccessDeniedError, @@ -85,6 +91,7 @@ import { } from "./api-errors"; export { + AUTH_ERROR_CODES, HttpError, SharingAuthError, SharingAccessDeniedError, @@ -221,133 +228,176 @@ export function apiHandler Promise>( undefined, }); - return eventStorage.run(evt, async () => { - let response: Response | undefined; - try { - response = await handler(...args); - } catch (error) { - if (error instanceof AssistantDisabledError) { - // v1.4.31 — operator has disabled the assistant surface. - // The 403 + `errorCode: "assistant.disabled."` - // envelope is locked per - // `.planning/RESPONSE-TO-IOS-TEAM-2026-05-16.md` §3 R5. - // Older iOS clients that don't know the errorCode surface - // this as a generic 403; v1.4.31+ clients can branch on the - // errorCode to render an inline operator-disabled notice. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: 403 }, - ); - } else if (error instanceof ConsentRequiredError) { - // v1.12.1 — server-side consent gate before external-LLM PHI - // egress on the operator's server-managed key. Mirrors the - // AssistantDisabledError envelope (403 + meta.errorCode) so the - // iOS client renders an inline "grant consent" notice instead of - // a generic failure. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: 403 }, - ); - } else if (error instanceof StepUpRequiredError) { - // v1.23 — step-up gate not satisfied. Same 401 + meta.errorCode - // envelope shape as the assistant/consent gates so the client can - // branch on the stable code and launch a re-verification flow. - // Checked before the generic HttpError branch because - // StepUpRequiredError extends it. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: error.statusCode }, - ); - } else if (error instanceof SharingAuthError) { - // v1.36.0 — the acting-account resolver refused. Same - // 403 + meta.errorCode envelope as the gates above, so a client - // distinguishes "you may not act as that account" from "this - // endpoint does not support acting on one" without parsing prose. - // Checked before the generic HttpError branch because it extends it. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: error.statusCode }, - ); - } else if (error instanceof HttpError) { - evt.setError(error); - response = NextResponse.json( - { data: null, error: error.message }, - { status: error.statusCode }, - ); - } else if (error instanceof SyntaxError) { - evt.setError(error); - response = NextResponse.json( - { data: null, error: "Invalid JSON body" }, - { status: 400 }, - ); - } else { - evt.setError(error); - // Report to GlitchTip (fire-and-forget) - reportToGlitchtip(error, request, evt).catch(() => {}); - response = NextResponse.json( - { data: null, error: "Interner Serverfehler" }, - { status: 500 }, - ); - } - } finally { - const status = (response as Response | undefined)?.status ?? 500; - evt.finish(status); + return runWithRateLimitCapture(() => + eventStorage.run(evt, async () => { + let response: Response | undefined; try { - emitIfSampled(evt.toJSON()); - } catch { - /* logging must never crash the handler */ + response = await handler(...args); + } catch (error) { + if (error instanceof AssistantDisabledError) { + // v1.4.31 — operator has disabled the assistant surface. + // The 403 + `errorCode: "assistant.disabled."` + // envelope is locked per + // `.planning/RESPONSE-TO-IOS-TEAM-2026-05-16.md` §3 R5. + // Older iOS clients that don't know the errorCode surface + // this as a generic 403; v1.4.31+ clients can branch on the + // errorCode to render an inline operator-disabled notice. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: 403 }, + ); + } else if (error instanceof ConsentRequiredError) { + // v1.12.1 — server-side consent gate before external-LLM PHI + // egress on the operator's server-managed key. Mirrors the + // AssistantDisabledError envelope (403 + meta.errorCode) so the + // iOS client renders an inline "grant consent" notice instead of + // a generic failure. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: 403 }, + ); + } else if (error instanceof StepUpRequiredError) { + // v1.23 — step-up gate not satisfied. Same 401 + meta.errorCode + // envelope shape as the assistant/consent gates so the client can + // branch on the stable code and launch a re-verification flow. + // Checked before the generic HttpError branch because + // StepUpRequiredError extends it. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: error.statusCode }, + ); + } else if (error instanceof SharingAuthError) { + // v1.36.0 — the acting-account resolver refused. Same + // 403 + meta.errorCode envelope as the gates above, so a client + // distinguishes "you may not act as that account" from "this + // endpoint does not support acting on one" without parsing prose. + // Checked before the generic HttpError branch because it extends it. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: error.statusCode }, + ); + } else if (error instanceof HttpError) { + // `meta.errorCode` only when the throw site named one, so every + // HttpError without a code serialises byte-identically to before. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + ...(error.errorCode + ? { meta: { errorCode: error.errorCode } } + : {}), + }, + { status: error.statusCode }, + ); + } else if (error instanceof SyntaxError) { + evt.setError(error); + response = NextResponse.json( + { data: null, error: "Invalid JSON body" }, + { status: 400 }, + ); + } else { + evt.setError(error); + // Report to GlitchTip (fire-and-forget) + reportToGlitchtip(error, request, evt).catch(() => {}); + response = NextResponse.json( + { data: null, error: "Interner Serverfehler" }, + { status: 500 }, + ); + } + } finally { + const status = (response as Response | undefined)?.status ?? 500; + evt.finish(status); + try { + emitIfSampled(evt.toJSON()); + } catch { + /* logging must never crash the handler */ + } } - } - const nr = response as NextResponse; - nr.headers.set("x-request-id", evt.getRequestId()); - // v1.37.0 — echo the record context this response was actually served - // under, when one was decided. The value comes from the wide event and - // from nowhere else: the fence stamps it on every call it makes, so - // exactly the responses that resolved a record scope carry it and - // everything else — public routes, actor surfaces, admin, `/me`, static — - // carries nothing. - // - // Deliberately NOT derived here with a `getSession()`. That would put a - // session read on every public route, and it would make the echo a second - // derivation of the context rather than a report of the one that was - // used, which is the difference between "what this response served" and - // "what a later read thinks is true". - // - // Absence is therefore meaningful and safe: the client discards a - // response whose echo CONTRADICTS its adopted context, and serves one - // that carries no echo normally. A discard-on-absence rule would throw - // away the `/api/version` poll on every cycle. - // - // The two header names stay inside the fence module, which is what keeps - // them to four files overall (declared, read, attached client-side, - // published) — see `src/__tests__/record-session-fence-guard.test.ts`. - attachRecordContextEcho(nr.headers, evt.getRecordContext()); - return nr; - }); + const nr = response as NextResponse; + nr.headers.set("x-request-id", evt.getRequestId()); + // v1.37.0 — echo the record context this response was actually served + // under, when one was decided. The value comes from the wide event and + // from nowhere else: the fence stamps it on every call it makes, so + // exactly the responses that resolved a record scope carry it and + // everything else — public routes, actor surfaces, admin, `/me`, static — + // carries nothing. + // + // Deliberately NOT derived here with a `getSession()`. That would put a + // session read on every public route, and it would make the echo a second + // derivation of the context rather than a report of the one that was + // used, which is the difference between "what this response served" and + // "what a later read thinks is true". + // + // Absence is therefore meaningful and safe: the client discards a + // response whose echo CONTRADICTS its adopted context, and serves one + // that carries no echo normally. A discard-on-absence rule would throw + // away the `/api/version` poll on every cycle. + // + // The two header names stay inside the fence module, which is what keeps + // them to four files overall (declared, read, attached client-side, + // published) — see `src/__tests__/record-session-fence-guard.test.ts`. + attachRecordContextEcho(nr.headers, evt.getRecordContext()); + attachRateLimitHeaders(nr); + return nr; + }), + ); }; return wrapped as T; } +/** + * Put the rate-limit headers on a refusal that left the handler without them. + * + * Only ~70 of the 199 route files that consult a limiter attach + * `rateLimitHeaders(rl)` by hand; the rest answer 429 with a bare prose + * envelope, so an offline replay queue that trips the batch ceiling has + * nothing to back off against but a guess. The limiter records its verdict in + * the request-scoped cell, so the headers can be derived here for every one of + * them at once instead of being threaded through each call site — and a route + * added tomorrow gets them without knowing this exists. + * + * `Retry-After` is the presence test rather than a flag: a handler that built + * its own headers already carries it (they come from the same builder), so + * this leaves that response exactly as it was written. + * + * The cell holds refusals only, so a 429 no limiter produced — a spent daily + * AI budget, an hourly generation quota, a provider-side 429 relayed onward — + * reads null here and goes out undressed. There is no bucket to describe, and + * one that still has room would name a delay against the wrong window. + */ +function attachRateLimitHeaders(response: NextResponse): void { + if (response.status !== 429) return; + if (response.headers.has("Retry-After")) return; + const snapshot = capturedRateLimit(); + if (!snapshot) return; + for (const [name, value] of Object.entries( + rateLimitResponseHeaders(snapshot), + )) { + response.headers.set(name, value); + } +} + /** * Authenticated request context. Returned by both session-cookie and Bearer-token * authentication paths. The `session.id` is the session-record id for cookie auth @@ -505,7 +555,7 @@ async function authenticateCaller( } // 3. No credentials. - throw new HttpError(401, "Not authenticated"); + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); } /** @@ -549,13 +599,15 @@ async function authenticateBearer( : {}), }, }).catch(() => {}); - const message = + // Sentence and code are decided together, so a future rewording of the + // prose cannot silently change what a client does with the refusal. + const [message, errorCode] = err.statusCode === 403 - ? "Insufficient permissions" + ? (["Insufficient permissions", AUTH_ERROR_CODES.scope] as const) : err.reason === "expired" - ? "Token expired" - : "Invalid token"; - throw new HttpError(err.statusCode, message); + ? (["Token expired", AUTH_ERROR_CODES.expired] as const) + : (["Invalid token", AUTH_ERROR_CODES.invalid] as const); + throw new HttpError(err.statusCode, message, errorCode); } throw err; } @@ -591,7 +643,11 @@ async function authenticateBearer( method: method || "unknown", }, }).catch(() => {}); - throw new HttpError(403, "Insufficient permissions"); + throw new HttpError( + 403, + "Insufficient permissions", + AUTH_ERROR_CODES.scope, + ); } } @@ -1079,7 +1135,8 @@ async function resolveSwitchedRecord( */ export async function requireAdmin(): Promise { const sessionData = await getSession(); - if (!sessionData) throw new HttpError(401, "Not authenticated"); + if (!sessionData) + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); const evt = getEvent(); if (evt) { @@ -1091,7 +1148,7 @@ export async function requireAdmin(): Promise { } if (sessionData.user.role !== "ADMIN") { - throw new HttpError(403, "Admin access required"); + throw new HttpError(403, "Admin access required", AUTH_ERROR_CODES.admin); } return { ...sessionData, authMethod: "cookie" }; } @@ -1108,7 +1165,8 @@ export async function requireAdmin(): Promise { */ export async function requireCookieAuth(): Promise { const sessionData = await getSession(); - if (!sessionData) throw new HttpError(401, "Not authenticated"); + if (!sessionData) + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); const evt = getEvent(); if (evt) { @@ -1182,7 +1240,8 @@ export async function requireFreshMfa( proofSource: FreshFactorProofSource = "second-factor", ): Promise { const sessionData = await getSession(); - if (!sessionData) throw new HttpError(401, "Not authenticated"); + if (!sessionData) + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); const evt = getEvent(); if (evt) { @@ -1413,7 +1472,7 @@ export async function requireBearerAuth(): Promise { authHeader = null; } if (!authHeader?.startsWith("Bearer ")) { - throw new HttpError(401, "Not authenticated"); + throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing); } const raw = authHeader.slice(7); const auth = await authenticateBearer(raw, undefined); diff --git a/src/lib/documents/__tests__/ai-route-support-limit.test.ts b/src/lib/documents/__tests__/ai-route-support-limit.test.ts index d2a4b1110..f47184ce6 100644 --- a/src/lib/documents/__tests__/ai-route-support-limit.test.ts +++ b/src/lib/documents/__tests__/ai-route-support-limit.test.ts @@ -77,6 +77,7 @@ describe("documentAiRateLimited", () => { const resetAt = Date.now() + 42 * 60_000; const response = documentAiRateLimited({ allowed: false, + limit: 60, remaining: 0, resetAt, }); diff --git a/src/lib/openapi/routes/cycle.ts b/src/lib/openapi/routes/cycle.ts index 903a8cd93..e32173f30 100644 --- a/src/lib/openapi/routes/cycle.ts +++ b/src/lib/openapi/routes/cycle.ts @@ -112,7 +112,7 @@ const cycleSymptomDto = z.object({ severity: z.number().int().min(1).max(4).nullable(), }); -const cycleDayLogDto = z +export const cycleDayLogDto = z .object({ id: z.string(), date: z.string(), @@ -145,7 +145,7 @@ const cycleDayLogDto = z "The canonical day-log row iOS mirrors. `note` is decrypted on read. Soft-deleted rows ride `/api/sync/changes` as tombstones.", }); -const menstrualCycleDto = z +export const menstrualCycleDto = z .object({ id: z.string(), startDate: z.string(), diff --git a/src/lib/openapi/routes/documents.ts b/src/lib/openapi/routes/documents.ts index 81159f2b7..95d5d20d8 100644 --- a/src/lib/openapi/routes/documents.ts +++ b/src/lib/openapi/routes/documents.ts @@ -793,7 +793,7 @@ export const inboundDocumentPaths: NonNullable = { tags: ["Documents"], summary: "Suggest filing metadata (drafts only)", description: - 'Optional AI assist on an already-stored document. Runs ONE provider call over the stored original (VISION, empty body) or browser-OCR\'d text (TEXT, `application/json` `{ mode: "text", text }`, opt-in local OCR) and returns a `{ title, kind, documentDate }` DRAFT for the edit form. AI-consent / rate / budget gated (shares the per-user document-AI bucket with extract/summary/index — default 6/hour, operator-tunable via `DOCUMENT_AI_LIMIT_PER_HOUR`; a slot is only consumed when the request reaches the provider, and the 429 carries the reset instant in `X-RateLimit-Reset` and `meta.retryAt`). WRITES NOTHING — never stages facts, never flips status; the user reviews and saves. 422 (`documents.inbound.providerUnsupported`) with no provider configured. Never interprets or diagnoses; the title is a neutral filing label.', + 'Optional AI assist on an already-stored document. Runs ONE provider call over the stored original (VISION, empty body) or browser-OCR\'d text (TEXT, `application/json` `{ mode: "text", text }`, opt-in local OCR) and returns a `{ title, kind, documentDate }` DRAFT for the edit form. AI-consent / rate / budget gated (shares the per-user document-AI bucket with extract/summary/index — default 6/hour, operator-tunable via `DOCUMENT_AI_LIMIT_PER_HOUR`; a slot is only consumed when the request reaches the provider, and the 429 carries `Retry-After`, the `X-RateLimit-Limit` / `-Remaining` / `-Reset` triple, and `meta.retryAt`). WRITES NOTHING — never stages facts, never flips status; the user reviews and saves. 422 (`documents.inbound.providerUnsupported`) with no provider configured. Never interprets or diagnoses; the title is a neutral filing label.', parameters: [ { name: "id", in: "path", required: true, schema: { type: "string" } }, ], diff --git a/src/lib/openapi/routes/ingest.ts b/src/lib/openapi/routes/ingest.ts index fb2cfb643..216619427 100644 --- a/src/lib/openapi/routes/ingest.ts +++ b/src/lib/openapi/routes/ingest.ts @@ -92,7 +92,7 @@ export const ingestPaths: NonNullable = { }, "429": { description: - "More than 60 requests in a minute for this token — or, when no valid token was presented, from this client IP. `meta.errorCode` = `ingest.rate_limited`, plus the standard `X-RateLimit-Remaining` / `X-RateLimit-Reset` headers.", + "More than 60 requests in a minute for this token — or, when no valid token was presented, from this client IP. `meta.errorCode` = `ingest.rate_limited`, plus the standard rate-limit headers: `Retry-After` (whole seconds, at least 1), `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset`.", content: { "application/json": { schema: errorEnvelope } }, }, }, diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index 4ce94c266..43b625a9e 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -21,20 +21,76 @@ import { createBatchWorkoutSchema as createBatchWorkoutSchemaBase } from "@/lib/ * `{ data, error, meta? }`. The OpenAPI surface mirrors that contract * so iOS / external-ingest clients can decode uniformly. */ +/** + * One rejected field from a multi-issue 422. + * + * `returnAllZodIssues` has emitted this since v1.4.42 so a client can fix + * three bad fields in one round-trip instead of three, and 166 route files + * send it — but no component schema declared it, so the very consumer it was + * built for could not read it from the contract. `params` never ships: some + * Zod codes embed the offending value in it and that is user content. + */ +const validationIssue = z + .object({ + path: z + .string() + .describe( + "Dot-joined path to the rejected field, e.g. `entries.3.measuredAt`. Empty string when the whole body was rejected.", + ), + code: z + .string() + .describe( + "Zod issue code, e.g. `invalid_type`, `too_small`, `unrecognized_keys`. Treat an unfamiliar code as a generic rejection of `path`.", + ), + message: z + .string() + .describe( + "English sentence describing the rejection. Safe to log; show the user your own wording keyed on `path`.", + ), + keys: z + .array(z.string()) + .optional() + .describe( + "Only on `unrecognized_keys`: the refused key names, bounded to 10 and 64 characters each. Zod reports every unknown key of one object in a single issue whose `path` is the object, so the names live here and nowhere in `path`.", + ), + }) + .meta({ + id: "ValidationIssue", + description: + "One rejected field in a multi-issue 422. Sanitised: no rejected VALUE is echoed back.", + }); + export const errorEnvelope = z .object({ data: z.null(), error: z.string(), + details: z + .object({ issues: z.array(validationIssue) }) + .optional() + .describe( + "Present on a 422 raised by the multi-issue validator: every field the request got wrong, not only the first. Absent on every other status.", + ), + // Loose on purpose. Real refusals put more than the two named keys here — + // `removedIn` / `replacedBy` on a 410, `module` beside `module.disabled`, + // the per-integration context on a failed connection test — and a closed + // object made the body invalid against its own published schema, which any + // strict generated decoder is entitled to reject. meta: z - .object({ + .looseObject({ requestId: z.string().optional(), - errorCode: z.string().optional(), + errorCode: z + .string() + .optional() + .describe( + "Stable machine code for this refusal. Branch on it rather than on `error`, which is prose and may be reworded.", + ), }) .optional(), }) .meta({ id: "ErrorEnvelope", - description: "Standard error response: data is null, error is human prose.", + description: + "Standard error response: `data` is null, `error` is human prose. `meta.errorCode` carries the stable machine code where one exists, and `meta` may carry further per-refusal context. A 422 from the multi-issue validator additionally carries `details.issues` — every field the request got wrong, so a client can fix them in one round-trip.", }); export function dataEnvelope(payload: T, id: string) { @@ -173,7 +229,8 @@ export const invalidBaseTokenResponse = { export const stdResponses = { "401": { - description: "Authentication required or invalid credentials.", + description: + "Authentication required or invalid credentials. The generic auth gates — the ones every route passes through before its own body runs — name the reason in `meta.errorCode`, and that is the field to branch on there; the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is gone; discard the credential and sign in again — retrying will not help. `auth.mfa.code_invalid`: a second factor presented on a step-up flow did not verify; re-prompt for a code and keep the session. The related 403 refusals carry `auth.scope.insufficient` (the credential is valid but its scope does not reach this route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's). A route that checks a credential of its own — sign-in, the MFA challenge exchange, a password confirmation — may still answer 401 with prose and no code, so a missing `meta.errorCode` means the route refused the credential it was handed, not that the response is malformed.", content: { "application/json": { schema: errorEnvelope } }, }, "422": { @@ -181,8 +238,31 @@ export const stdResponses = { content: { "application/json": { schema: errorEnvelope } }, }, "429": { - description: "Rate limit exceeded.", + description: + "Rate limit exceeded. The response carries `Retry-After` (whole seconds, rounded up and never below 1 — wait at least that long before retrying), `X-RateLimit-Limit` (the bucket's cap), `X-RateLimit-Remaining` and `X-RateLimit-Reset` (the reset instant, ISO-8601, not epoch seconds). Back off on `Retry-After` rather than guessing. A 429 raised by a ceiling that is not one of these buckets — a daily AI budget, an upstream provider's own refusal relayed onward — carries none of them, because there is no bucket to describe.", content: { "application/json": { schema: errorEnvelope } }, + // Declared rather than only described: a generated client gets a typed + // accessor for the field the description tells it to back off on. + headers: { + "Retry-After": { + description: + "Whole seconds to wait before retrying. Rounded up, never below 1.", + schema: { type: "integer" as const, minimum: 1 }, + }, + "X-RateLimit-Limit": { + description: "Requests the bucket allows per window.", + schema: { type: "integer" as const, minimum: 1 }, + }, + "X-RateLimit-Remaining": { + description: "Requests left in the current window; 0 on a refusal.", + schema: { type: "integer" as const, minimum: 0 }, + }, + "X-RateLimit-Reset": { + description: + "Instant the current window rolls over, ISO-8601 — not epoch seconds.", + schema: { type: "string" as const, format: "date-time" }, + }, + }, }, }; diff --git a/src/lib/openapi/routes/sync.ts b/src/lib/openapi/routes/sync.ts index 2f767327b..007fd424c 100644 --- a/src/lib/openapi/routes/sync.ts +++ b/src/lib/openapi/routes/sync.ts @@ -9,6 +9,7 @@ import { z } from "zod/v4"; import type { ZodOpenApiObject } from "zod-openapi"; import { measurementResource } from "./measurements"; import { moodContextWire } from "./mood"; +import { cycleDayLogDto, menstrualCycleDto } from "./cycle"; import { dataEnvelope, stdResponses } from "./shared"; // ── Sync (v1.7.0 offline / server-optional) ───────────────────────── @@ -185,6 +186,36 @@ const syncIntakeTombstone = z "A soft-deleted medication intake, keyed on server `id`. Apply before upserts within the domain page.", }); +const syncCycleDayTombstone = z + .object({ + id: z.string(), + externalId: z + .string() + .nullable() + .describe("The client's own key for the day log, when it set one."), + syncVersion: z.number().int().nonnegative(), + deletedAt: z.iso.datetime({ offset: true }), + updatedAt: z.iso.datetime({ offset: true }), + }) + .meta({ + id: "SyncCycleDayTombstone", + description: + "A soft-deleted cycle day log, keyed on server `id`. Apply before upserts within the domain page.", + }); + +const syncCycleTombstone = z + .object({ + id: z.string(), + syncVersion: z.number().int().nonnegative(), + deletedAt: z.iso.datetime({ offset: true }), + updatedAt: z.iso.datetime({ offset: true }), + }) + .meta({ + id: "SyncCycleTombstone", + description: + "A soft-deleted menstrual cycle, keyed on server `id`. Apply before upserts within the domain page.", + }); + const syncChangesQuery = z .object({ cursor: z @@ -205,7 +236,15 @@ const syncChangesQuery = z }) .meta({ id: "SyncChangesQuery" }); -const syncChangesResponse = z +/** + * Exported so a test can `safeParse` a real `/api/sync/changes` body against + * it. Nothing else checks a response SHAPE against its published schema — + * `openapi:check` proves the YAML matches the registry, and the route-coverage + * guard proves the path is listed, so a schema that disagrees with what + * `apiSuccess()` sends passes both. That is how this endpoint went five + * domains wide against a three-domain contract. + */ +export const syncChangesResponse = z .object({ serverNow: z.iso.datetime({ offset: true }), cursor: z @@ -233,12 +272,28 @@ const syncChangesResponse = z upserts: z.array(syncIntakeUpsert), tombstones: z.array(syncIntakeTombstone), }), + cycleDays: z + .object({ + upserts: z.array(cycleDayLogDto), + tombstones: z.array(syncCycleDayTombstone), + }) + .describe( + "Cycle day logs. Present on every page, empty arrays when the account has the cycle module off.", + ), + cycles: z + .object({ + upserts: z.array(menstrualCycleDto), + tombstones: z.array(syncCycleTombstone), + }) + .describe( + "Menstrual cycles, observed and forward-predicted. Present on every page, empty arrays when the account has the cycle module off.", + ), }), }) .meta({ id: "SyncChangesResponse", description: - "Multi-domain delta page (v1.7.0): measurements + mood + intakes. One opaque multi-domain keyset cursor; tombstones apply before upserts within each domain. Tombstone identity: measurements key on externalId, mood + intakes on server id. The iOS consumer is measurements-only this cycle; mood + intakes are forward-prep.", + "Multi-domain delta page: measurements, mood, intakes, cycle day logs and cycles. One opaque multi-domain keyset cursor; tombstones apply before upserts within each domain. Tombstone identity: measurements key on externalId, every other domain on server id. All five keys are present on every response, including the `cursorExpired` short-circuit — a domain with nothing to report sends empty arrays rather than being omitted.", }); export const syncPaths: NonNullable = { @@ -264,9 +319,9 @@ export const syncPaths: NonNullable = { "/api/sync/changes": { get: { tags: ["Sync"], - summary: "Measurements delta feed", + summary: "Multi-domain delta feed", description: - "Incremental catch-up after the first-pair backfill (never a replacement for it). Pages over an opaque keyset cursor; each page carries `tombstones` (soft-deleted rows, keyed on `externalId`) and `upserts` (live rows). Apply tombstones before upserts within a page. `cursorExpired: true` forces a clean re-init.", + "Incremental catch-up after the first-pair backfill (never a replacement for it). Pages over one opaque keyset cursor spanning five domains — measurements, mood, medication intakes, cycle day logs and cycles. Each domain carries `tombstones` (soft-deleted rows) and `upserts` (live rows); apply tombstones before upserts within a domain. Tombstone identity is `externalId` for measurements and the server `id` everywhere else. `cursorExpired: true` forces a clean re-init and the page is empty in every domain.", requestParams: { query: syncChangesQuery, }, diff --git a/src/lib/rate-limit-context.ts b/src/lib/rate-limit-context.ts new file mode 100644 index 000000000..fd1181233 --- /dev/null +++ b/src/lib/rate-limit-context.ts @@ -0,0 +1,107 @@ +/** + * Request-scoped capture of the rate-limiter's refusals, plus the one + * builder that turns a verdict into response headers — a LEAF module. + * + * It lives apart from `rate-limit.ts` on purpose. Nearly two hundred route + * tests replace `@/lib/rate-limit` wholesale with a `vi.mock` factory, so + * anything `apiHandler` needs from the limiter has to come from a module those + * factories do not stand in for; otherwise the wrapper calls `undefined()` the + * moment a mocked route is exercised. + * + * Why the capture exists at all: a route that refuses a request answers 429 + * from its own handler body, and most of them build that response with a bare + * `apiError(...)`. Threading the headers through every one of those call sites + * is 129 edits that the next new route immediately forgets. Instead the + * limiter records its verdict here, `apiHandler` runs every handler inside the + * store, and any 429 that leaves without headers gets them on the way out. A + * route that already attaches them keeps exactly the response it built. + */ +import { AsyncLocalStorage } from "node:async_hooks"; + +/** What a rate-limit verdict has to carry to be renderable as headers. */ +export interface RateLimitSnapshot { + /** Requests allowed per window. */ + limit: number; + /** Requests left in the current window; zero once the cap is hit. */ + remaining: number; + /** Epoch milliseconds at which the current window rolls over. */ + resetAt: number; +} + +interface RateLimitCapture { + denied: RateLimitSnapshot | null; +} + +const storage = new AsyncLocalStorage(); + +/** + * Run `fn` with a fresh capture cell. Called once per request by + * `apiHandler`; outside one, every capture below is a no-op and + * {@link capturedRateLimit} reads null. + */ +export function runWithRateLimitCapture(fn: () => T): T { + return storage.run({ denied: null }, fn); +} + +/** + * Record a limiter verdict for the request in flight. + * + * Only refusals are kept. A verdict that let the request through describes a + * bucket that still has room, and there is no 429 it could honestly explain: + * if the response ends up being a 429 anyway, some other ceiling produced it. + * The last refusal wins, since a handler that consults two buckets refuses at + * the first denial it acts on. + */ +export function captureRateLimitResult( + result: RateLimitSnapshot & { allowed: boolean }, +): void { + const store = storage.getStore(); + if (!store) return; + if (!result.allowed) store.denied = result; +} + +/** + * The refusal to render for this request, or null when no limiter refused it. + * + * Null is the answer for a request every bucket let through, so a 429 raised + * by something else — a daily AI budget, an hourly generation quota, a + * provider-side 429 relayed onward — is left undressed rather than labelled + * with a bucket that has room left and a delay measured against an unrelated + * window. + */ +export function capturedRateLimit(): RateLimitSnapshot | null { + const store = storage.getStore(); + if (!store) return null; + return store.denied; +} + +/** + * The rate-limit response headers. + * + * `Retry-After` is whole seconds and rounds UP, per RFC 9110 §10.2.3: a client + * that waits the value it is given must find the window already rolled over, + * which flooring cannot promise. It never goes below one second: `resetAt` + * comes from the database clock and the subtraction happens on the app + * process's, so a window can read as already expired here while the row still + * refuses; and zero on a refusal means "retry now", which contradicts the + * status it rides on and invites a hot loop. + * + * `X-RateLimit-Reset` keeps the ISO-8601 instant it has always carried. It is + * the wrong convention for the header name and every other deployment uses + * epoch or delta seconds, but it is on the wire today and changing it is a + * contract decision of its own; `Retry-After` gives an integer-parsing client + * the number it needs without touching the existing field. + */ +export function rateLimitResponseHeaders( + snapshot: RateLimitSnapshot, + now: number = Date.now(), +): Record { + return { + "Retry-After": String( + Math.max(1, Math.ceil((snapshot.resetAt - now) / 1000)), + ), + "X-RateLimit-Limit": String(snapshot.limit), + "X-RateLimit-Remaining": String(snapshot.remaining), + "X-RateLimit-Reset": new Date(snapshot.resetAt).toISOString(), + }; +} diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index b8dccc0a5..d76113579 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -5,11 +5,14 @@ import { prisma } from "@/lib/db"; import { getClientIpOrTrustWarning } from "@/lib/api-response"; +import { + captureRateLimitResult, + rateLimitResponseHeaders, + type RateLimitSnapshot, +} from "@/lib/rate-limit-context"; -export interface RateLimitResult { +export interface RateLimitResult extends RateLimitSnapshot { allowed: boolean; - remaining: number; - resetAt: number; } /** @@ -45,11 +48,16 @@ export async function checkRateLimit( const resetAt = row.reset_at.getTime(); const count = Number(row.count); - return { + const result: RateLimitResult = { allowed: count <= limit, + limit, remaining: Math.max(0, limit - count), resetAt, }; + // Hand the verdict to the request-scoped cell so `apiHandler` can dress a + // bare 429 on the way out. A no-op outside a request. + captureRateLimitResult(result); + return result; } /** @@ -77,15 +85,13 @@ export async function refundRateLimit(key: string): Promise { } /** - * Rate limit response headers. + * Rate limit response headers: `Retry-After` plus the `X-RateLimit-*` triple. + * See `rateLimitResponseHeaders` for what each one promises. */ export function rateLimitHeaders( - result: RateLimitResult, + result: RateLimitSnapshot, ): Record { - return { - "X-RateLimit-Remaining": String(result.remaining), - "X-RateLimit-Reset": new Date(result.resetAt).toISOString(), - }; + return rateLimitResponseHeaders(result); } /** diff --git a/tests/integration/list-pagination-tiebreaker.test.ts b/tests/integration/list-pagination-tiebreaker.test.ts new file mode 100644 index 000000000..171aa9aad --- /dev/null +++ b/tests/integration/list-pagination-tiebreaker.test.ts @@ -0,0 +1,214 @@ +/** + * Offset pagination over a tied sort key must not repeat or drop a row. + * + * `/api/measurements` and `/api/labs` page with `skip`/`take` over a single + * non-unique column. Ties are the normal case on both: an Apple Health import + * stamps long runs of samples with one instant and the documented + * `stats::YYYY-MM-DD` rollup convention puts every daily total at + * the same one, while a lab panel drawn in one sitting shares `takenAt` across + * every analyte in it. Without a unique secondary key Postgres may order the + * tied block differently for the query that fetches page N and the one that + * fetches page N+1, so a client mirroring its own history silently receives + * one row twice and never receives another. For a health record that is a + * data-integrity bug, and it is invisible to both sides. + * + * The fixtures are seeded with explicit ids in a deliberately scrambled order, + * so heap order and id order disagree. That is what makes this a real gate: + * asserting only that two pages are disjoint would pass on an unordered scan + * that happened to be stable, whereas the id ordering inside a tied block can + * only hold if the secondary sort key is actually there. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const TEST_USER_ID = "user-pagination-tiebreaker"; +const TIED_AT = new Date("2026-03-04T09:00:00.000Z"); + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +/** + * Insertion order deliberately unrelated to id order: 04, 09, 01, 06, 00, … + * A scan that ignores the secondary key returns this sequence; one that + * honours it returns 09, 08, 07, … on a descending sort. + */ +const SCRAMBLED = [4, 9, 1, 6, 0, 8, 3, 7, 2, 5]; + +/** + * One type per row. The partial unique index on + * `(user_id, type, measured_at, source, sleep_stage)` forbids two live rows of + * the SAME type at one instant, so the tie that actually occurs in production + * is the cross-type one: a daily `stats:` rollup posts steps, distance, + * flights, active energy and the rest at the identical timestamp, and an + * unfiltered history list pages straight through the block. + */ +const TYPES = [ + "ACTIVITY_STEPS", + "FLIGHTS_CLIMBED", + "WALKING_RUNNING_DISTANCE", + "ACTIVE_ENERGY_BURNED", + "RESTING_HEART_RATE", + "HEART_RATE_VARIABILITY", + "OXYGEN_SATURATION", + "RESPIRATORY_RATE", + "BODY_TEMPERATURE", + "WEIGHT", +] as const; + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + await prisma.user.create({ + data: { + id: TEST_USER_ID, + username: "pagination-tiebreaker", + email: "pagination-tiebreaker@example.test", + }, + }); + const session = await prisma.session.create({ + data: { + userId: TEST_USER_ID, + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", session.id); + + // Every row shares one `measuredAt` / `takenAt` — the whole page is one + // tied block, which is the worst case and the realistic one. + for (const n of SCRAMBLED) { + const suffix = String(n).padStart(2, "0"); + await prisma.measurement.create({ + data: { + id: `tie-measurement-${suffix}`, + userId: TEST_USER_ID, + type: TYPES[n], + value: 80 + n, + unit: "count", + measuredAt: TIED_AT, + source: "APPLE_HEALTH", + }, + }); + await prisma.labResult.create({ + data: { + id: `tie-lab-${suffix}`, + userId: TEST_USER_ID, + analyte: `Analyte ${suffix}`, + value: 1 + n, + unit: "mg/dl", + takenAt: TIED_AT, + }, + }); + } +}); + +function get(path: string): NextRequest { + return new NextRequest(`http://localhost${path}`); +} + +async function idsOf( + res: Response, + key: "measurements" | "results", +): Promise { + const body = (await res.json()) as { + data: Record>; + }; + return body.data[key].map((row) => row.id); +} + +describe("offset pagination over a tied sort key (real Postgres)", () => { + it("pages /api/measurements with no duplicate and no gap", async () => { + const { GET } = await import("@/app/api/measurements/route"); + + const first = await idsOf( + await GET(get("/api/measurements?limit=4&offset=0") as never), + "measurements", + ); + const second = await idsOf( + await GET(get("/api/measurements?limit=4&offset=4") as never), + "measurements", + ); + const third = await idsOf( + await GET(get("/api/measurements?limit=4&offset=8") as never), + "measurements", + ); + + const seen = [...first, ...second, ...third]; + expect(seen).toHaveLength(10); + expect(new Set(seen).size).toBe(10); + expect([...seen].sort()).toEqual( + SCRAMBLED.map((n) => `tie-measurement-${String(n).padStart(2, "0")}`) + .slice() + .sort(), + ); + + // The default sort is `measuredAt desc`; with every row tied, the + // tiebreaker is the only thing deciding the order, so it must be + // strictly descending by id across the page boundaries. + expect(seen).toEqual([...seen].sort().reverse()); + }); + + it("pages /api/labs with no duplicate and no gap", async () => { + const { GET } = await import("@/app/api/labs/route"); + + const first = await idsOf( + await GET(get("/api/labs?limit=4&offset=0") as never), + "results", + ); + const second = await idsOf( + await GET(get("/api/labs?limit=4&offset=4") as never), + "results", + ); + const third = await idsOf( + await GET(get("/api/labs?limit=4&offset=8") as never), + "results", + ); + + const seen = [...first, ...second, ...third]; + expect(seen).toHaveLength(10); + expect(new Set(seen).size).toBe(10); + expect(seen).toEqual([...seen].sort().reverse()); + }); + + it("keeps the ascending direction stable too", async () => { + const { GET } = await import("@/app/api/measurements/route"); + + const first = await idsOf( + await GET(get("/api/measurements?limit=5&offset=0&sortDir=asc") as never), + "measurements", + ); + const second = await idsOf( + await GET(get("/api/measurements?limit=5&offset=5&sortDir=asc") as never), + "measurements", + ); + + const seen = [...first, ...second]; + expect(new Set(seen).size).toBe(10); + expect(seen).toEqual([...seen].sort()); + }); +}); diff --git a/tests/integration/openapi-response-shape.test.ts b/tests/integration/openapi-response-shape.test.ts new file mode 100644 index 000000000..59c640c99 --- /dev/null +++ b/tests/integration/openapi-response-shape.test.ts @@ -0,0 +1,305 @@ +/** + * A real response, parsed against the schema that publishes it. + * + * Nothing in the gate did this before. `pnpm openapi:check` proves the YAML + * matches the registry; `openapi-route-coverage-guard` proves the verb is + * listed. Both pass when the registered schema disagrees with what the handler + * actually sends, and that gap is how `/api/sync/changes` ended up shipping + * five domains against a three-domain contract while the body was invalid + * against its own published schema. + * + * Two responses are covered — the two that were broken: + * + * - `GET /api/sync/changes`, the multi-domain delta the native client mirrors + * its whole history from. + * - a multi-issue 422, whose `details.issues` array is the reason the + * envelope exists and was the field the envelope did not declare. + * + * The check is stricter than `safeParse` alone. Zod strips undeclared keys + * rather than refusing them, so a pass would say nothing about a field the + * server sends and the contract omits — exactly the direction both of those + * drifted in. Comparing the parsed value back to the raw body catches + * the strip: if anything was dropped, the schema is short of the wire. + * + * One limit worth knowing before leaning on that half: `meta` is declared as a + * loose object, deliberately, because real refusals put `removedIn`, + * `replacedBy` and per-integration context there. Loose keys survive the parse, + * so the strip comparison does not reach inside `meta`. A `meta` field that + * ever becomes load-bearing for a client needs its own declaration and its own + * assertion. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; +import { syncChangesResponse } from "@/lib/openapi/routes/sync"; +import { errorEnvelope } from "@/lib/openapi/routes/shared"; + +const TEST_USER_ID = "user-openapi-response-shape"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + + // FEMALE so the cycle module is on by default and the two domains the + // contract used to omit actually carry rows. + await prisma.user.create({ + data: { + id: TEST_USER_ID, + username: "openapi-response-shape", + email: "openapi-response-shape@example.test", + gender: "FEMALE", + timezone: "UTC", + }, + }); + const session = await prisma.session.create({ + data: { + userId: TEST_USER_ID, + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", session.id); +}); + +/** + * Seed one live row and one soft-deleted row per domain, so every `upserts` + * and every `tombstones` array in the page is non-empty. An all-empty page + * would parse against almost any schema. + */ +async function seedEveryDomain(): Promise { + const prisma = getPrismaClient(); + const at = new Date("2026-02-02T08:00:00.000Z"); + const gone = new Date("2026-02-02T09:00:00.000Z"); + + await prisma.measurement.createMany({ + data: [ + { + id: "shape-measurement-live", + userId: TEST_USER_ID, + type: "WEIGHT", + value: 79.2, + unit: "kg", + measuredAt: at, + source: "MANUAL", + externalId: "shape-live", + }, + { + id: "shape-measurement-gone", + userId: TEST_USER_ID, + type: "PULSE", + value: 61, + unit: "bpm", + measuredAt: at, + source: "MANUAL", + externalId: "shape-gone", + deletedAt: gone, + }, + ], + }); + + await prisma.moodEntry.createMany({ + data: [ + { + id: "shape-mood-live", + userId: TEST_USER_ID, + date: "2026-02-02", + mood: "GOOD", + score: 4, + moodLoggedAt: at, + source: "MANUAL", + }, + { + id: "shape-mood-gone", + userId: TEST_USER_ID, + date: "2026-02-01", + mood: "OKAY", + score: 3, + moodLoggedAt: at, + source: "MANUAL", + deletedAt: gone, + }, + ], + }); + + const medication = await prisma.medication.create({ + data: { + id: "shape-medication", + userId: TEST_USER_ID, + name: "Test compound", + dose: "1 tablet", + }, + }); + await prisma.medicationIntakeEvent.createMany({ + data: [ + { + id: "shape-intake-live", + userId: TEST_USER_ID, + medicationId: medication.id, + scheduledFor: at, + takenAt: at, + skipped: false, + source: "WEB", + }, + { + id: "shape-intake-gone", + userId: TEST_USER_ID, + medicationId: medication.id, + scheduledFor: gone, + takenAt: null, + skipped: true, + source: "WEB", + deletedAt: gone, + }, + ], + }); + + await prisma.menstrualCycle.createMany({ + data: [ + { + id: "shape-cycle-live", + userId: TEST_USER_ID, + startDate: "2026-01-05", + }, + { + id: "shape-cycle-gone", + userId: TEST_USER_ID, + startDate: "2025-12-05", + deletedAt: gone, + }, + ], + }); + await prisma.cycleDayLog.createMany({ + data: [ + { + id: "shape-day-live", + userId: TEST_USER_ID, + date: "2026-01-05", + cycleId: "shape-cycle-live", + flow: "MEDIUM", + source: "MANUAL", + }, + { + id: "shape-day-gone", + userId: TEST_USER_ID, + date: "2026-01-04", + source: "MANUAL", + deletedAt: gone, + }, + ], + }); +} + +describe("published response schemas against real bodies", () => { + it("parses a full /api/sync/changes page and drops nothing", async () => { + await seedEveryDomain(); + const { GET } = await import("@/app/api/sync/changes/route"); + + const res = await GET( + new NextRequest("http://localhost/api/sync/changes") as never, + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: unknown }; + + const parsed = syncChangesResponse.safeParse(body.data); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.success).toBe(true); + + // Every domain actually carried rows, so the parse above was asked a real + // question rather than being handed five empty pairs. + const changes = ( + body.data as { + changes: Record; + } + ).changes; + for (const domain of [ + "measurements", + "mood", + "intakes", + "cycleDays", + "cycles", + ]) { + expect(changes[domain].upserts.length).toBeGreaterThan(0); + expect(changes[domain].tombstones.length).toBeGreaterThan(0); + } + + // Zod strips what it does not declare, so an equal round-trip is the part + // that proves the contract is not short of the wire. + expect(parsed.data).toEqual(body.data); + }); + + it("parses the cursorExpired short-circuit page too", async () => { + const { GET } = await import("@/app/api/sync/changes/route"); + + // A cursor whose watermark predates tombstone retention forces the + // early return, which builds its own literal page rather than the one + // the domain walks assemble — a second shape under the same schema. + const { encodeCursor } = await import("@/lib/sync/cursor"); + const stale = encodeCursor({ + measurements: { + updatedAtMs: Date.now() - 400 * 24 * 60 * 60 * 1000, + id: "long-gone", + }, + }); + + const res = await GET( + new NextRequest( + `http://localhost/api/sync/changes?cursor=${stale}`, + ) as never, + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cursorExpired: boolean } }; + + const parsed = syncChangesResponse.safeParse(body.data); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.data).toEqual(body.data); + }); + + it("parses a multi-issue 422 against the published error envelope", async () => { + const { GET } = await import("@/app/api/measurements/route"); + + const res = await GET( + new NextRequest( + "http://localhost/api/measurements?limit=0&offset=-3&sortDir=sideways", + ) as never, + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details?: { issues: unknown[] }; + }; + + // The envelope exists to report every bad field at once; if only one + // arrives, the fixture stopped exercising the multi-issue path. + expect(body.details?.issues.length ?? 0).toBeGreaterThan(1); + + const parsed = errorEnvelope.safeParse(body); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.data).toEqual(body); + }); +}); diff --git a/tests/integration/rate-limit-429-headers.test.ts b/tests/integration/rate-limit-429-headers.test.ts new file mode 100644 index 000000000..87800caef --- /dev/null +++ b/tests/integration/rate-limit-429-headers.test.ts @@ -0,0 +1,213 @@ +/** + * Which 429s carry the rate-limit headers, against a real limiter. + * + * `POST /api/measurements/batch` is the case that matters most: a first-run + * Apple Health import reliably trips its 60-per-minute ceiling, and until now + * the client got a prose sentence and nothing to back off against. The route + * builds its refusal with a bare `apiError(...)`, so this proves the headers + * come from `apiHandler` and the limiter's request-scoped verdict rather than + * from the handler body — which is what makes the other 128 header-less routes + * covered too. + * + * Two more shapes ride along, because the dressing is only honest when the + * numbers describe the ceiling that actually refused: + * + * - `POST /api/withings/sync` consults two buckets — a 5/minute baseline and + * a 1/hour bucket on the expensive full walk. A request the baseline lets + * through and the full bucket refuses must report the full bucket. + * - `POST /api/medications/extract` clears its limiter and can then be + * refused by the daily AI token budget. That 429 belongs to no bucket, so + * it must go out with no rate-limit headers at all rather than one naming + * a bucket that still has room. + * + * Run against a real Postgres because the limiter's window and remainder come + * out of the atomic upsert, and the budget ledger is a real row too; a mocked + * verdict would prove the plumbing and not the numbers. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const TEST_USER_ID = "user-rate-limit-headers"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + await getPrismaClient().user.create({ + data: { + id: TEST_USER_ID, + username: "rate-limit-headers", + email: "rate-limit-headers@example.test", + }, + }); + const session = await getPrismaClient().session.create({ + data: { + userId: TEST_USER_ID, + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", session.id); +}); + +function batchRequest(): NextRequest { + return new NextRequest("http://localhost/api/measurements/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + entries: [ + { + hkIdentifier: "HKQuantityTypeIdentifierBodyMass", + value: 81.4, + unit: "kg", + startDate: "2026-07-20T06:00:00.000Z", + endDate: "2026-07-20T06:00:00.000Z", + externalId: `rl-${Math.random().toString(36).slice(2)}`, + }, + ], + }), + }); +} + +describe("429 rate-limit headers (real Postgres)", () => { + it("carries Retry-After and the X-RateLimit triple on a route that attaches none", async () => { + const { POST } = await import("@/app/api/measurements/batch/route"); + + // Burn the bucket directly rather than posting sixty batches: the route + // and the pre-charge share one key, so the next call through the handler + // is the one that is refused. + const { checkRateLimit } = await import("@/lib/rate-limit"); + const key = `measurements:batch:${TEST_USER_ID}`; + for (let i = 0; i < 60; i += 1) { + await checkRateLimit(key, 60, 60_000); + } + + const before = Date.now(); + const res = await POST(batchRequest() as never); + expect(res.status).toBe(429); + + const retryAfter = res.headers.get("Retry-After"); + expect(retryAfter).toMatch(/^\d+$/); + const seconds = Number(retryAfter); + expect(seconds).toBeGreaterThan(0); + expect(seconds).toBeLessThanOrEqual(60); + + expect(res.headers.get("X-RateLimit-Limit")).toBe("60"); + expect(res.headers.get("X-RateLimit-Remaining")).toBe("0"); + + const reset = res.headers.get("X-RateLimit-Reset"); + expect(reset).toBeTruthy(); + const resetAt = Date.parse(reset as string); + expect(Number.isNaN(resetAt)).toBe(false); + expect(resetAt).toBeGreaterThan(before); + + // The named delay must actually outlast the window it names. + expect(before + seconds * 1000).toBeGreaterThanOrEqual(resetAt - 1_000); + + // The body is unchanged — the headers are additive. + const body = (await res.json()) as { data: null; error: string }; + expect(body.data).toBeNull(); + expect(typeof body.error).toBe("string"); + }); + + it("leaves a successful response without them", async () => { + const { POST } = await import("@/app/api/measurements/batch/route"); + + const res = await POST(batchRequest() as never); + expect(res.status).toBe(200); + expect(res.headers.get("Retry-After")).toBeNull(); + }); + + it("reports the bucket that refused when a route consults two", async () => { + const { POST } = await import("@/app/api/withings/sync/route"); + + // Burn only the full-sync bucket. The route's baseline 5/60s bucket is + // untouched, so it passes on the way in and the refusal comes from the + // 1/hour one — the numbers on the wire have to be that one's. + const { checkRateLimit } = await import("@/lib/rate-limit"); + await checkRateLimit(`withings-sync-full:${TEST_USER_ID}`, 1, 60 * 60_000); + + const res = await POST( + new NextRequest("http://localhost/api/withings/sync", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ fullSync: true }), + }) as never, + ); + + expect(res.status).toBe(429); + expect(res.headers.get("X-RateLimit-Limit")).toBe("1"); + expect(res.headers.get("X-RateLimit-Remaining")).toBe("0"); + // The baseline bucket's window is 60 seconds; the hour-long one cannot be + // mistaken for it. + expect(Number(res.headers.get("Retry-After"))).toBeGreaterThan(60); + }); + + it("leaves a 429 the limiter did not produce undressed", async () => { + // The daily AI budget, not a bucket. The route's own 10-per-5-minutes + // limiter passes on the way in; dressing this refusal with that verdict + // would tell the client nine calls are still free and to retry in five + // minutes, when the budget does not move until UTC midnight. + await getPrismaClient().user.update({ + where: { id: TEST_USER_ID }, + // A self-hosted local endpoint resolves as the chain's primary, which + // keeps the request on the user-plan ceiling and off the consent gate. + data: { aiBaseUrl: "http://127.0.0.1:11434/v1" }, + }); + const { buildDateKey, USER_PLAN_CAP } = + await import("@/lib/ai/coach/budget"); + await getPrismaClient().coachUsage.create({ + data: { + userId: TEST_USER_ID, + dateKey: buildDateKey(), + totalTokens: USER_PLAN_CAP, + messageCount: 1, + }, + }); + + const { POST } = await import("@/app/api/medications/extract/route"); + const res = await POST( + new NextRequest("http://localhost/api/medications/extract", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ text: "Mounjaro 5mg weekly Wednesday" }), + }) as never, + ); + + expect(res.status).toBe(429); + for (const name of [ + "Retry-After", + "X-RateLimit-Limit", + "X-RateLimit-Remaining", + "X-RateLimit-Reset", + ]) { + expect(res.headers.get(name)).toBeNull(); + } + }); +});