From a759c23fb5271852360d5a41523a787ca0688347 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 18:21:28 +0000 Subject: [PATCH 1/3] fix(admin-digest): fail closed when the super-admin recipient list cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The digest's recipients are the code/env allowlist plus every super_admins row. The lookup read the table with a bare `const { data } = await ...`: a PostgREST error resolved to "no rows", a thrown query hit an empty catch, and the digest went out to the allowlist alone and answered 200 ok. An admin who exists only in the table silently stopped receiving it (characterized in #677; its contract case C6). - lib/feedback/notify.ts: new readSuperAdminRecipients returns the allowlist ∪ the table, or an error for a failed, thrown or truncated read (readAll with a 1,000-row ceiling, ordered by the primary key). An empty table is NOT an error: the configured allowlist is then the whole list. - The admin digest uses it and answers 502 {ok:false, adminDigest.recipientsUnavailable} before sending anything. - allSuperAdminEmails keeps its best-effort fallback for its only other caller, notifySuperAdmins (alert emails still reach the allowlist rather than nobody), but now logs the failed read instead of swallowing it. - New key adminDigest.recipientsUnavailable in en-US and the six full catalogues. - tests/a-paged-read-needs-a-total-order: super_admins' key registered (0008: `email text PRIMARY KEY`). Regressions: tests/admin-digest-recipients-fail-closed.test.ts (6) and tests/super-admin-recipients-read.test.ts (10). Against the previous source 10 of them fail (the three fail-closed route cases and seven reader/log cases); the three unchanged-behaviour cases pass on both. No live email, configuration or migration. Duplicate delivery (#677 C1-C5) is untouched and still unresolved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KP9rt5rVQ9jDMpZp2xBy3K --- app/api/cron/admin-digest/route.ts | 13 +- lib/feedback/notify.ts | 45 +++++- lib/i18n/messages/de-DE.json | 1 + lib/i18n/messages/en-US.json | 1 + lib/i18n/messages/es-ES.json | 1 + lib/i18n/messages/fr-FR.json | 1 + lib/i18n/messages/it-IT.json | 1 + lib/i18n/messages/nl-NL.json | 1 + lib/i18n/messages/pt-PT.json | 1 + .../a-paged-read-needs-a-total-order.test.ts | 1 + ...dmin-digest-recipients-fail-closed.test.ts | 152 ++++++++++++++++++ tests/super-admin-recipients-read.test.ts | 126 +++++++++++++++ 12 files changed, 336 insertions(+), 8 deletions(-) create mode 100644 tests/admin-digest-recipients-fail-closed.test.ts create mode 100644 tests/super-admin-recipients-read.test.ts diff --git a/app/api/cron/admin-digest/route.ts b/app/api/cron/admin-digest/route.ts index 7d139eeaa..950e192b4 100644 --- a/app/api/cron/admin-digest/route.ts +++ b/app/api/cron/admin-digest/route.ts @@ -3,7 +3,7 @@ import { getTranslations } from '@/lib/i18n/server'; import { createServiceClient } from '@/lib/supabase/server'; import { hasCronAuthorization } from '@/lib/server/cron-auth'; import { readAll } from '@/lib/supabase/read-all'; -import { allSuperAdminEmails } from '@/lib/feedback/notify'; +import { readSuperAdminRecipients } from '@/lib/feedback/notify'; import { sendEmail } from '@/lib/server/email'; import { buildAdminDigest, digestSubject, renderAdminDigestHtml, summarizeDigestDelivery, type DigestRow, @@ -72,7 +72,16 @@ export async function GET(req: NextRequest) { return NextResponse.json({ ok: true, sent: 0, total: 0, reason: 'no activity in the last 24h' }); } - const emails = await allSuperAdminEmails(admin); + // Fail CLOSED on the recipient list. Sending to the env allowlist alone after a + // failed super_admins read silently drops every admin who exists only in the + // table, and used to answer 200 ok while doing it. An empty table is fine: the + // allowlist is then the whole list. + const recipients = await readSuperAdminRecipients(admin); + if (recipients.error !== null) { + console.error('[admin-digest] super-admin recipient read failed', recipients.error); + return NextResponse.json({ ok: false, error: t('adminDigest.recipientsUnavailable') }, { status: 502 }); + } + const emails = recipients.emails; if (emails.length === 0) { return NextResponse.json({ ok: true, sent: 0, total: digest.total, reason: 'no super-admin recipients' }); } diff --git a/lib/feedback/notify.ts b/lib/feedback/notify.ts index 2fff352fa..66d4ae9db 100644 --- a/lib/feedback/notify.ts +++ b/lib/feedback/notify.ts @@ -8,6 +8,7 @@ import { } from '@/lib/integrations/github'; import { issueTitle, issueBody, githubLabels } from '@/lib/feedback/github-map'; import { wroteNoRows } from '@/lib/supabase/errors'; +import { readAll } from '@/lib/supabase/read-all'; type Admin = ReturnType; @@ -17,14 +18,46 @@ function appUrl(): string { return (process.env.NEXT_PUBLIC_APP_URL || 'https://www.bubaly.com').replace(/\/$/, ''); } -/** Every super-admin email: the code/env allowlist ∪ the super_admins table. */ -export async function allSuperAdminEmails(admin: Admin): Promise { +/** The most `super_admins` rows a recipient read accepts; one more is a truncated read. */ +const SUPER_ADMIN_READ_MAX = 1_000; + +export type SuperAdminRecipients = + | { emails: string[]; error: null } + | { emails: null; error: string }; + +/** + * Every super-admin email — the code/env allowlist ∪ the super_admins table — or + * an error when the table could not be read COMPLETELY. + * + * An empty table is not an error: the allowlist is then the whole, legitimate + * list. A failed read, a thrown query and a read past the ceiling are errors, + * because the allowlist alone would silently drop every admin who exists only in + * the table. `{ data }` alone could not tell those apart: a PostgREST call + * RESOLVES with `{ data: null, error }`, which read as "no rows". + */ +export async function readSuperAdminRecipients(admin: Admin): Promise { const set = new Set(envSuperAdminEmails()); try { - const { data } = await admin.from('super_admins').select('email'); - for (const r of data ?? []) if (r.email) set.add(r.email.toLowerCase()); - } catch { /* table may not exist yet — env allowlist still applies */ } - return [...set]; + const { rows, error } = await readAll<{ email: string | null }>((from, to) => admin + .from('super_admins').select('email').order('email').range(from, to), { max: SUPER_ADMIN_READ_MAX }); + if (error) return { emails: null, error: error.message ?? String(error) }; + for (const r of rows) if (r.email) set.add(r.email.toLowerCase()); + } catch (e) { + return { emails: null, error: e instanceof Error ? e.message : String(e) }; + } + return { emails: [...set], error: null }; +} + +/** + * Best-effort form for alerts (notifySuperAdmins): when the table cannot be read + * it still reaches the allowlist rather than nobody, and says so in the log. A + * caller that must not send to a partial list uses readSuperAdminRecipients. + */ +export async function allSuperAdminEmails(admin: Admin): Promise { + const read = await readSuperAdminRecipients(admin); + if (read.error === null) return read.emails; + console.error('[feedback-notify] super_admins read failed; using the env allowlist only', read.error); + return envSuperAdminEmails(); } /** diff --git a/lib/i18n/messages/de-DE.json b/lib/i18n/messages/de-DE.json index 8996dd058..7b47ee2eb 100644 --- a/lib/i18n/messages/de-DE.json +++ b/lib/i18n/messages/de-DE.json @@ -875,6 +875,7 @@ "adminCredential.secret": "Der konfigurierte Schlüssel hat das richtige Format (sb_secret_…). Prüfe, ob er zu einem anderen Projekt gehört oder widerrufen wurde.", "adminCredential.unknown": "Der konfigurierte Schlüssel entspricht keinem bekannten Supabase-Schlüsselformat.", "adminDigest.notificationFeedUnavailable": "Benachrichtigungs-Feed nicht verfügbar.", + "adminDigest.recipientsUnavailable": "Empfängerliste der Super-Admins nicht verfügbar.", "adminDigest.unauthorized": "Nicht autorisiert", "adminFeedback.couldNotDeleteIdea": "Die Idee konnte nicht gelöscht werden.", "adminFeedback.couldNotPinIdea": "Die Idee konnte nicht angeheftet werden.", diff --git a/lib/i18n/messages/en-US.json b/lib/i18n/messages/en-US.json index a676f27da..e37ae5198 100644 --- a/lib/i18n/messages/en-US.json +++ b/lib/i18n/messages/en-US.json @@ -868,6 +868,7 @@ "adminCredential.secret": "The configured key has the right shape (sb_secret_…). Check whether it belongs to a different project or has been revoked.", "adminCredential.unknown": "The configured key matches no known Supabase key format.", "adminDigest.notificationFeedUnavailable": "Notification feed unavailable.", + "adminDigest.recipientsUnavailable": "Super-admin recipient list unavailable.", "adminDigest.unauthorized": "Unauthorized", "adminFeedback.couldNotDeleteIdea": "Could not delete the idea.", "adminFeedback.couldNotPinIdea": "Could not pin the idea.", diff --git a/lib/i18n/messages/es-ES.json b/lib/i18n/messages/es-ES.json index 702c638be..9040b3516 100644 --- a/lib/i18n/messages/es-ES.json +++ b/lib/i18n/messages/es-ES.json @@ -875,6 +875,7 @@ "adminCredential.secret": "La clave configurada tiene el formato correcto (sb_secret_…). Comprueba si pertenece a otro proyecto o ha sido revocada.", "adminCredential.unknown": "La clave configurada no coincide con ningún formato de clave conocido de Supabase.", "adminDigest.notificationFeedUnavailable": "Feed de notificaciones no disponible.", + "adminDigest.recipientsUnavailable": "Lista de destinatarios superadministradores no disponible.", "adminDigest.unauthorized": "No autorizado", "adminFeedback.couldNotDeleteIdea": "No se ha podido eliminar la idea.", "adminFeedback.couldNotPinIdea": "No se ha podido fijar la idea.", diff --git a/lib/i18n/messages/fr-FR.json b/lib/i18n/messages/fr-FR.json index 9fb95e411..d95933db0 100644 --- a/lib/i18n/messages/fr-FR.json +++ b/lib/i18n/messages/fr-FR.json @@ -875,6 +875,7 @@ "adminCredential.secret": "La clé configurée a le bon format (sb_secret_…). Vérifiez si elle appartient à un autre projet ou si elle a été révoquée.", "adminCredential.unknown": "La clé configurée ne correspond à aucun format de clé Supabase connu.", "adminDigest.notificationFeedUnavailable": "Fil de notifications indisponible.", + "adminDigest.recipientsUnavailable": "Liste des destinataires super-administrateurs indisponible.", "adminDigest.unauthorized": "Non autorisé", "adminFeedback.couldNotDeleteIdea": "Impossible de supprimer l’idée.", "adminFeedback.couldNotPinIdea": "Impossible d’épingler l’idée.", diff --git a/lib/i18n/messages/it-IT.json b/lib/i18n/messages/it-IT.json index 0865c844b..eb5a2051c 100644 --- a/lib/i18n/messages/it-IT.json +++ b/lib/i18n/messages/it-IT.json @@ -875,6 +875,7 @@ "adminCredential.secret": "La chiave configurata ha il formato corretto (sb_secret_…). Verifica se appartiene a un altro progetto o è stata revocata.", "adminCredential.unknown": "La chiave configurata non corrisponde a nessun formato di chiave Supabase conosciuto.", "adminDigest.notificationFeedUnavailable": "Feed di notifiche non disponibile.", + "adminDigest.recipientsUnavailable": "Elenco dei destinatari super amministratori non disponibile.", "adminDigest.unauthorized": "Non autorizzato", "adminFeedback.couldNotDeleteIdea": "Impossibile eliminare l’idea.", "adminFeedback.couldNotPinIdea": "Impossibile fissare l’idea.", diff --git a/lib/i18n/messages/nl-NL.json b/lib/i18n/messages/nl-NL.json index 8fda9e2ed..263f4df9d 100644 --- a/lib/i18n/messages/nl-NL.json +++ b/lib/i18n/messages/nl-NL.json @@ -875,6 +875,7 @@ "adminCredential.secret": "De ingestelde sleutel heeft het juiste formaat (sb_secret_…). Controleer of deze bij een ander project hoort of is ingetrokken.", "adminCredential.unknown": "De ingestelde sleutel komt niet overeen met een bekend Supabase-sleutelformaat.", "adminDigest.notificationFeedUnavailable": "Meldingenfeed niet beschikbaar.", + "adminDigest.recipientsUnavailable": "Ontvangerslijst van superbeheerders niet beschikbaar.", "adminDigest.unauthorized": "Niet geautoriseerd", "adminFeedback.couldNotDeleteIdea": "Kan het idee niet verwijderen.", "adminFeedback.couldNotPinIdea": "Kan het idee niet vastzetten.", diff --git a/lib/i18n/messages/pt-PT.json b/lib/i18n/messages/pt-PT.json index e8cef8924..0d548ac57 100644 --- a/lib/i18n/messages/pt-PT.json +++ b/lib/i18n/messages/pt-PT.json @@ -876,6 +876,7 @@ "adminCredential.secret": "A chave configurada tem o formato correto (sb_secret_…). Verifica se pertence a outro projeto ou se foi revogada.", "adminCredential.unknown": "A chave configurada não corresponde a nenhum formato de chave Supabase conhecido.", "adminDigest.notificationFeedUnavailable": "Feed de notificações indisponível.", + "adminDigest.recipientsUnavailable": "Lista de destinatários superadministradores indisponível.", "adminDigest.unauthorized": "Não autorizado", "adminFeedback.couldNotDeleteIdea": "Não foi possível eliminar a ideia.", "adminFeedback.couldNotPinIdea": "Não foi possível fixar a ideia.", diff --git a/tests/a-paged-read-needs-a-total-order.test.ts b/tests/a-paged-read-needs-a-total-order.test.ts index 0aabbe63c..c46db91ca 100644 --- a/tests/a-paged-read-needs-a-total-order.test.ts +++ b/tests/a-paged-read-needs-a-total-order.test.ts @@ -54,6 +54,7 @@ const UNIQUE_KEYS: Record = { network_aggregates: [{ columns: ['scope', 'cohort_key', 'metric', 'value'], why: '0135: `unique (scope, cohort_key, metric, value)`' }], network_consent: [{ columns: ['family_id'], why: '0132: `family_id uuid primary key`' }], network_contributions: [{ columns: ['family_id'], why: '0135: `family_id uuid primary key`' }], + super_admins: [{ columns: ['email'], why: '0008: `email text PRIMARY KEY`' }], }; function walk(dir: string, out: string[] = []): string[] { diff --git a/tests/admin-digest-recipients-fail-closed.test.ts b/tests/admin-digest-recipients-fail-closed.test.ts new file mode 100644 index 000000000..4b8c522ea --- /dev/null +++ b/tests/admin-digest-recipients-fail-closed.test.ts @@ -0,0 +1,152 @@ +// The admin digest fails CLOSED when it cannot read its recipient list. +// +// The recipients are the code/env allowlist (lib/constants/super-admins.ts) plus +// every row of `super_admins`. Before this fix the lookup read the table with a +// bare `const { data } = await …`: a PostgREST error resolved to "no rows", a +// thrown query was swallowed by an empty catch, and the digest went out to the +// allowlist alone and answered 200 ok. A super admin who exists only in the table +// simply stopped receiving it, and nothing anywhere said so (#677 characterized +// this; its contract case C6 is the same requirement). +// +// The contract pinned here: +// - a failed, thrown or truncated `super_admins` read sends NOTHING and answers +// 502, so the scheduler sees a failed run rather than a partial one; +// - a SUCCESSFUL read with zero rows is not an error: the configured allowlist +// is the legitimate recipient list, as before; +// - a successful read with rows is the allowlist ∪ the table, lowercased and +// de-duplicated, as before. +// +// Real: the route handler, cron auth, readAll paging, the recipient lookup and +// sendEmail down to `fetch`. Fake: the database (tests/helpers/in-memory-supabase), +// the clock (Date only), translations (identity) and the email provider, which is +// a stub for global `fetch` that counts attempts and fails the test on any other +// URL. Nothing is sent. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createInMemorySupabase, type InMemorySupabase } from './helpers/in-memory-supabase'; + +const state = vi.hoisted(() => ({ + db: null as unknown, + attempts: [] as string[], + stray: [] as string[], + failSuperAdmins: null as null | 'error' | 'throw', + superAdminReads: 0, +})); + +vi.mock('@/lib/i18n/server', () => ({ getTranslations: async () => (key: string) => key })); +vi.mock('@/lib/supabase/server', () => ({ createServiceClient: () => state.db })); + +const { GET } = await import('@/app/api/cron/admin-digest/route'); +const { superAdminEmails } = await import('@/lib/constants/super-admins'); + +const CRON = 'test-cron-secret-not-real'; +const TABLE_ADMIN = 'table-admin@example.test'; +const ENV_ADMIN = 'env-admin@example.test'; + +let db: InMemorySupabase; +function client() { + return { + from(table: string) { + const qb = db.from(table) as unknown as { then: (f?: (v: unknown) => unknown, r?: (e: unknown) => unknown) => Promise }; + if (table !== 'super_admins') return qb; + const then = qb.then.bind(qb); + qb.then = (f, r) => { + state.superAdminReads += 1; + if (state.failSuperAdmins === 'throw') return Promise.reject(new Error('injected: super_admins read threw')).then(f, r); + if (state.failSuperAdmins === 'error') { + return Promise.resolve({ data: null, error: { code: 'XX000', message: 'injected: super_admins read failed', details: null, hint: null }, count: null, status: 500, statusText: 'Internal Server Error' }).then(f, r); + } + return then(f, r); + }; + return qb; + }, + }; +} +function happen(at: string, title: string) { + db.seed('admin_notifications', [{ id: `n-${title}`, kind: 'family_signup', title, created_at: new Date(at).toISOString() }]); +} +async function runAt(at: string) { + vi.setSystemTime(new Date(at)); + const res = await GET(new Request('https://bubaly.test/api/cron/admin-digest', { headers: { authorization: `Bearer ${CRON}` } }) as never); + return { status: res.status, body: await res.json() as Record }; +} + +const saved = { ...process.env }; +beforeEach(() => { + vi.useFakeTimers({ toFake: ['Date'] }); + process.env.CRON_SECRET = CRON; + process.env.RESEND_API_KEY = 'test-resend-key-not-real'; + process.env.SUPER_ADMIN_EMAILS = ENV_ADMIN; + db = createInMemorySupabase(); + state.db = client(); + state.attempts = []; state.stray = []; state.failSuperAdmins = null; state.superAdminReads = 0; + vi.stubGlobal('fetch', vi.fn(async (input: unknown, init?: RequestInit) => { + if (String(input) !== 'https://api.resend.com/emails') { state.stray.push(String(input)); throw new Error('network is disabled in this hermetic test'); } + state.attempts.push((JSON.parse(String(init?.body)) as { to: string }).to); + return new Response('{"id":"msg"}', { status: 200 }); + })); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); +}); +afterEach(() => { + expect(state.stray, 'no call may leave the process').toEqual([]); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + vi.useRealTimers(); + for (const k of ['CRON_SECRET', 'RESEND_API_KEY', 'SUPER_ADMIN_EMAILS']) { + if (saved[k] === undefined) delete process.env[k]; else process.env[k] = saved[k]; + } +}); + +const allowlist = () => superAdminEmails(); // built-in + SUPER_ADMIN_EMAILS, lowercased + +describe('admin digest: an unreadable recipient list sends nothing', () => { + for (const how of ['error', 'throw'] as const) { + it(`a super_admins read that ${how === 'error' ? 'answers an error' : 'throws'}: 502, not one email, not 200 ok`, async () => { + db.seed('super_admins', [{ email: TABLE_ADMIN }]); + happen('2026-09-30T08:00:00Z', 'Signup Alpha'); + state.failSuperAdmins = how; + const res = await runAt('2026-09-30T12:30:00Z'); + expect(state.superAdminReads).toBeGreaterThan(0); + expect(res.status).toBe(502); + expect(res.body).toMatchObject({ ok: false, error: 'adminDigest.recipientsUnavailable' }); + expect(state.attempts).toEqual([]); + }); + } + + it('a super_admins list longer than the read ceiling is a truncated read: 502, not one email', async () => { + db.seed('super_admins', Array.from({ length: 1_001 }, (_, i) => ({ email: `admin-${String(i).padStart(4, '0')}@example.test` }))); + happen('2026-09-30T08:00:00Z', 'Signup Alpha'); + const res = await runAt('2026-09-30T12:30:00Z'); + expect(res.status).toBe(502); + expect(res.body).toMatchObject({ ok: false, error: 'adminDigest.recipientsUnavailable' }); + expect(state.attempts).toEqual([]); + }); +}); + +describe('admin digest: a readable recipient list behaves as before', () => { + it('a successful read with ZERO rows is not an error: the configured allowlist receives it, 200', async () => { + happen('2026-09-30T08:00:00Z', 'Signup Alpha'); + const res = await runAt('2026-09-30T12:30:00Z'); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ ok: true, sent: allowlist().length, failed: 0, recipients: allowlist().length }); + expect([...state.attempts].sort()).toEqual([...allowlist()].sort()); + expect(state.attempts).toContain(ENV_ADMIN); + }); + + it('a successful read with rows: the allowlist plus the table, lowercased and de-duplicated, 200', async () => { + db.seed('super_admins', [{ email: TABLE_ADMIN.toUpperCase() }, { email: ENV_ADMIN.toUpperCase() }, { email: null }]); + happen('2026-09-30T08:00:00Z', 'Signup Alpha'); + const res = await runAt('2026-09-30T12:30:00Z'); + const expected = [...new Set([...allowlist(), TABLE_ADMIN])].sort(); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ ok: true, sent: expected.length, recipients: expected.length }); + expect([...state.attempts].sort()).toEqual(expected); + }); + + it('a quiet day still answers before looking up recipients at all', async () => { + state.failSuperAdmins = 'error'; + const res = await runAt('2026-09-30T12:30:00Z'); + expect(res).toEqual({ status: 200, body: { ok: true, sent: 0, total: 0, reason: 'no activity in the last 24h' } }); + expect(state.superAdminReads).toBe(0); + }); +}); diff --git a/tests/super-admin-recipients-read.test.ts b/tests/super-admin-recipients-read.test.ts new file mode 100644 index 000000000..2284ad235 --- /dev/null +++ b/tests/super-admin-recipients-read.test.ts @@ -0,0 +1,126 @@ +// The shared super-admin recipient lookup, and BOTH of its callers' contracts. +// +// lib/feedback/notify.ts has two readers of the same list: +// - readSuperAdminRecipients: strict. The allowlist ∪ the super_admins table, +// or an ERROR when the table could not be read completely. The admin digest +// uses it and fails closed (tests/admin-digest-recipients-fail-closed.test.ts). +// - allSuperAdminEmails: best-effort, for notifySuperAdmins' alert emails. A +// failed read still reaches the allowlist rather than nobody (unchanged), and +// is now logged instead of swallowed. +// The only other callers of either are the admin digest route and +// notifySuperAdmins (checked with a repository grep when this was written). +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createInMemorySupabase, type InMemorySupabase } from './helpers/in-memory-supabase'; + +const state = vi.hoisted(() => ({ attempts: [] as string[], stray: [] as string[], fail: null as null | 'error' | 'throw' })); + +const { readSuperAdminRecipients, allSuperAdminEmails, notifySuperAdmins } = await import('@/lib/feedback/notify'); +const { superAdminEmails } = await import('@/lib/constants/super-admins'); + +const TABLE_ADMIN = 'table-admin@example.test'; +const ENV_ADMIN = 'env-admin@example.test'; + +let db: InMemorySupabase; +function client() { + return { + from(table: string) { + const qb = db.from(table) as unknown as { then: (f?: (v: unknown) => unknown, r?: (e: unknown) => unknown) => Promise }; + if (table !== 'super_admins' || !state.fail) return qb; + qb.then = (f, r) => (state.fail === 'throw' + ? Promise.reject(new Error('injected: super_admins read threw')) + : Promise.resolve({ data: null, error: { code: 'XX000', message: 'injected: super_admins read failed', details: null, hint: null }, count: null, status: 500, statusText: 'Internal Server Error' }) + ).then(f, r); + return qb; + }, + } as never; +} + +const saved = { ...process.env }; +let errors: ReturnType; +beforeEach(() => { + process.env.SUPER_ADMIN_EMAILS = ENV_ADMIN; + process.env.RESEND_API_KEY = 'test-resend-key-not-real'; + db = createInMemorySupabase(); + state.attempts = []; state.stray = []; state.fail = null; + vi.stubGlobal('fetch', vi.fn(async (input: unknown, init?: RequestInit) => { + if (String(input) !== 'https://api.resend.com/emails') { state.stray.push(String(input)); throw new Error('network is disabled in this hermetic test'); } + state.attempts.push((JSON.parse(String(init?.body)) as { to: string }).to); + return new Response('{"id":"msg"}', { status: 200 }); + })); + errors = vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + expect(state.stray).toEqual([]); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + for (const k of ['SUPER_ADMIN_EMAILS', 'RESEND_API_KEY']) { + if (saved[k] === undefined) delete process.env[k]; else process.env[k] = saved[k]; + } +}); + +const sorted = (xs: string[]) => [...xs].sort(); + +describe('readSuperAdminRecipients: an empty table is not an error; a failed read is', () => { + it('a read that answers an error is an error, with no list', async () => { + state.fail = 'error'; + expect(await readSuperAdminRecipients(client())).toEqual({ emails: null, error: 'injected: super_admins read failed' }); + }); + + it('a read that throws is an error, with no list', async () => { + state.fail = 'throw'; + expect(await readSuperAdminRecipients(client())).toEqual({ emails: null, error: 'injected: super_admins read threw' }); + }); + + it('more rows than the ceiling is a truncated read: an error, not a short list', async () => { + db.seed('super_admins', Array.from({ length: 1_001 }, (_, i) => ({ email: `admin-${String(i).padStart(4, '0')}@example.test` }))); + const read = await readSuperAdminRecipients(client()); + expect(read.emails).toBeNull(); + expect(read.error).toMatch(/1000|ceiling|more rows|stopped/i); + }); + + it('exactly the ceiling is a complete read', async () => { + db.seed('super_admins', Array.from({ length: 1_000 }, (_, i) => ({ email: `admin-${String(i).padStart(4, '0')}@example.test` }))); + const read = await readSuperAdminRecipients(client()); + expect(read.error).toBeNull(); + expect(read.emails).toHaveLength(superAdminEmails().length + 1_000); + }); + + it('a successful read with zero rows is the configured allowlist, and no error', async () => { + expect(await readSuperAdminRecipients(client())).toEqual({ emails: superAdminEmails(), error: null }); + expect(superAdminEmails()).toContain(ENV_ADMIN); + }); + + it('a successful read with rows is the allowlist ∪ the table, lowercased, de-duplicated, blanks skipped', async () => { + db.seed('super_admins', [{ email: TABLE_ADMIN.toUpperCase() }, { email: ENV_ADMIN.toUpperCase() }, { email: null }, { email: '' }]); + const read = await readSuperAdminRecipients(client()); + expect(read.error).toBeNull(); + expect(sorted(read.emails!)).toEqual(sorted([...new Set([...superAdminEmails(), TABLE_ADMIN])])); + }); +}); + +describe('allSuperAdminEmails / notifySuperAdmins: alerts stay best-effort, and a failed read is no longer silent', () => { + it('a failed read still returns the allowlist, and logs why', async () => { + state.fail = 'error'; + expect(await allSuperAdminEmails(client())).toEqual(superAdminEmails()); + expect(errors).toHaveBeenCalledWith('[feedback-notify] super_admins read failed; using the env allowlist only', 'injected: super_admins read failed'); + }); + + it('a successful read is unchanged: the allowlist ∪ the table, nothing logged', async () => { + db.seed('super_admins', [{ email: TABLE_ADMIN }]); + expect(sorted(await allSuperAdminEmails(client()))).toEqual(sorted([...superAdminEmails(), TABLE_ADMIN])); + expect(errors).not.toHaveBeenCalled(); + }); + + it('notifySuperAdmins after a failed read: the alert is recorded and still emailed to the allowlist', async () => { + state.fail = 'throw'; + await notifySuperAdmins(client(), { kind: 'info', title: 'Test alert' }); + expect(db.table('admin_notifications')).toHaveLength(1); + expect(sorted(state.attempts)).toEqual(sorted(superAdminEmails())); + }); + + it('notifySuperAdmins after a successful read: the table admin is emailed too', async () => { + db.seed('super_admins', [{ email: TABLE_ADMIN }]); + await notifySuperAdmins(client(), { kind: 'info', title: 'Test alert' }); + expect(sorted(state.attempts)).toEqual(sorted([...superAdminEmails(), TABLE_ADMIN])); + }); +}); From 23cf5ac4f2796b54e983a99796c5b9e085fdef02 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 18:35:30 +0000 Subject: [PATCH 2/3] fix(super-admin recipients): classify the failure; keep the raw cause for the log only tests/the-database-does-not-talk-to-the-browser flagged `error: error.message` in readSuperAdminRecipients' result: a raw database message under an `error` key is one refactor away from a response body. The result now carries a classification (`read_failed` | `truncated`) and the raw cause under `cause`, which only the two server-side log lines read. Behaviour is unchanged: the digest still answers 502 with the translated message and sends nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KP9rt5rVQ9jDMpZp2xBy3K --- app/api/cron/admin-digest/route.ts | 4 ++-- lib/feedback/notify.ts | 22 +++++++++++++--------- tests/super-admin-recipients-read.test.ts | 15 +++++++-------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/app/api/cron/admin-digest/route.ts b/app/api/cron/admin-digest/route.ts index 950e192b4..0ec6f3454 100644 --- a/app/api/cron/admin-digest/route.ts +++ b/app/api/cron/admin-digest/route.ts @@ -77,8 +77,8 @@ export async function GET(req: NextRequest) { // table, and used to answer 200 ok while doing it. An empty table is fine: the // allowlist is then the whole list. const recipients = await readSuperAdminRecipients(admin); - if (recipients.error !== null) { - console.error('[admin-digest] super-admin recipient read failed', recipients.error); + if (recipients.failure !== null) { + console.error('[admin-digest] super-admin recipient read failed', recipients.failure, recipients.cause); return NextResponse.json({ ok: false, error: t('adminDigest.recipientsUnavailable') }, { status: 502 }); } const emails = recipients.emails; diff --git a/lib/feedback/notify.ts b/lib/feedback/notify.ts index 66d4ae9db..3012c20bb 100644 --- a/lib/feedback/notify.ts +++ b/lib/feedback/notify.ts @@ -21,9 +21,13 @@ function appUrl(): string { /** The most `super_admins` rows a recipient read accepts; one more is a truncated read. */ const SUPER_ADMIN_READ_MAX = 1_000; +/** + * `failure` is a classification, not a message: the raw cause is kept apart for + * the server log only, so nothing a database said can reach a response body. + */ export type SuperAdminRecipients = - | { emails: string[]; error: null } - | { emails: null; error: string }; + | { emails: string[]; failure: null } + | { emails: null; failure: 'read_failed' | 'truncated'; cause: unknown }; /** * Every super-admin email — the code/env allowlist ∪ the super_admins table — or @@ -38,14 +42,14 @@ export type SuperAdminRecipients = export async function readSuperAdminRecipients(admin: Admin): Promise { const set = new Set(envSuperAdminEmails()); try { - const { rows, error } = await readAll<{ email: string | null }>((from, to) => admin + const { rows, error, truncated } = await readAll<{ email: string | null }>((from, to) => admin .from('super_admins').select('email').order('email').range(from, to), { max: SUPER_ADMIN_READ_MAX }); - if (error) return { emails: null, error: error.message ?? String(error) }; + if (error) return { emails: null, failure: truncated ? 'truncated' : 'read_failed', cause: error }; for (const r of rows) if (r.email) set.add(r.email.toLowerCase()); - } catch (e) { - return { emails: null, error: e instanceof Error ? e.message : String(e) }; + } catch (cause) { + return { emails: null, failure: 'read_failed', cause }; } - return { emails: [...set], error: null }; + return { emails: [...set], failure: null }; } /** @@ -55,8 +59,8 @@ export async function readSuperAdminRecipients(admin: Admin): Promise { const read = await readSuperAdminRecipients(admin); - if (read.error === null) return read.emails; - console.error('[feedback-notify] super_admins read failed; using the env allowlist only', read.error); + if (read.failure === null) return read.emails; + console.error('[feedback-notify] super_admins read failed; using the env allowlist only', read.failure, read.cause); return envSuperAdminEmails(); } diff --git a/tests/super-admin-recipients-read.test.ts b/tests/super-admin-recipients-read.test.ts index 2284ad235..7343b57c7 100644 --- a/tests/super-admin-recipients-read.test.ts +++ b/tests/super-admin-recipients-read.test.ts @@ -63,37 +63,36 @@ const sorted = (xs: string[]) => [...xs].sort(); describe('readSuperAdminRecipients: an empty table is not an error; a failed read is', () => { it('a read that answers an error is an error, with no list', async () => { state.fail = 'error'; - expect(await readSuperAdminRecipients(client())).toEqual({ emails: null, error: 'injected: super_admins read failed' }); + expect(await readSuperAdminRecipients(client())).toMatchObject({ emails: null, failure: 'read_failed', cause: { message: 'injected: super_admins read failed' } }); }); it('a read that throws is an error, with no list', async () => { state.fail = 'throw'; - expect(await readSuperAdminRecipients(client())).toEqual({ emails: null, error: 'injected: super_admins read threw' }); + expect(await readSuperAdminRecipients(client())).toMatchObject({ emails: null, failure: 'read_failed', cause: { message: 'injected: super_admins read threw' } }); }); it('more rows than the ceiling is a truncated read: an error, not a short list', async () => { db.seed('super_admins', Array.from({ length: 1_001 }, (_, i) => ({ email: `admin-${String(i).padStart(4, '0')}@example.test` }))); const read = await readSuperAdminRecipients(client()); - expect(read.emails).toBeNull(); - expect(read.error).toMatch(/1000|ceiling|more rows|stopped/i); + expect(read).toMatchObject({ emails: null, failure: 'truncated' }); }); it('exactly the ceiling is a complete read', async () => { db.seed('super_admins', Array.from({ length: 1_000 }, (_, i) => ({ email: `admin-${String(i).padStart(4, '0')}@example.test` }))); const read = await readSuperAdminRecipients(client()); - expect(read.error).toBeNull(); + expect(read.failure).toBeNull(); expect(read.emails).toHaveLength(superAdminEmails().length + 1_000); }); it('a successful read with zero rows is the configured allowlist, and no error', async () => { - expect(await readSuperAdminRecipients(client())).toEqual({ emails: superAdminEmails(), error: null }); + expect(await readSuperAdminRecipients(client())).toEqual({ emails: superAdminEmails(), failure: null }); expect(superAdminEmails()).toContain(ENV_ADMIN); }); it('a successful read with rows is the allowlist ∪ the table, lowercased, de-duplicated, blanks skipped', async () => { db.seed('super_admins', [{ email: TABLE_ADMIN.toUpperCase() }, { email: ENV_ADMIN.toUpperCase() }, { email: null }, { email: '' }]); const read = await readSuperAdminRecipients(client()); - expect(read.error).toBeNull(); + expect(read.failure).toBeNull(); expect(sorted(read.emails!)).toEqual(sorted([...new Set([...superAdminEmails(), TABLE_ADMIN])])); }); }); @@ -102,7 +101,7 @@ describe('allSuperAdminEmails / notifySuperAdmins: alerts stay best-effort, and it('a failed read still returns the allowlist, and logs why', async () => { state.fail = 'error'; expect(await allSuperAdminEmails(client())).toEqual(superAdminEmails()); - expect(errors).toHaveBeenCalledWith('[feedback-notify] super_admins read failed; using the env allowlist only', 'injected: super_admins read failed'); + expect(errors).toHaveBeenCalledWith('[feedback-notify] super_admins read failed; using the env allowlist only', 'read_failed', expect.objectContaining({ message: 'injected: super_admins read failed' })); }); it('a successful read is unchanged: the allowlist ∪ the table, nothing logged', async () => { From 4edfce86c1a29ee107322beb221b1651b78537b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:00:35 +0000 Subject: [PATCH 3/3] =?UTF-8?q?test(admin-digest):=20combined=20#677/#685?= =?UTF-8?q?=20acceptance=20=E2=80=94=20re-pin=20the=20recipient=20cases,?= =?UTF-8?q?=20adapt=20the=20harness,=20promote=20C6=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner review on #685: prepare the combined test acceptance before integration, with #677 at exactly e90fa5a2 (merged in the previous commit). tests/admin-digest-replay-contract.test.ts on the combined source: - the two super_admins read cases (error, throw) move from "200 ok, built-in admin only" to 502 adminDigest.recipientsUnavailable with zero provider attempts; the pre-#685 observation stays in the dated evidence; - the empty-recipient override also answers readSuperAdminRecipients (the reader the route now uses) in its success shape {emails: [], failure: null}, so the empty-recipient branch is still the one exercised; - recipient reads are paged since #685 (one-row page + empty page per call), so the concurrency barrier holds only the FIRST page of each invocation instead of pinning the number of paged reads; replay/concurrency send counts (2 per admin) are unchanged; - C6 is promoted (it failed on 231e8140 and passes with #685). C1-C5 stay in the evidence folder as unmet contract cases. Evidence folder: the dated 231e8140 outputs are unchanged; combined-source outputs are added (characterization 20/20; contract 5 failed / 2 passed before C6 was moved), C6 is replaced by a pointer in desired-contract.test.ts, and the README gets a dated addendum. Duplicate delivery is not fixed; no live email. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KP9rt5rVQ9jDMpZp2xBy3K --- .../README.md | 11 ++ ...racterization-output-combined-with-685.txt | 29 ++++ ...ired-contract-output-combined-with-685.txt | 127 ++++++++++++++++++ .../tests/desired-contract.test.ts | 11 +- tests/admin-digest-replay-contract.test.ts | 42 ++++-- 5 files changed, 205 insertions(+), 15 deletions(-) create mode 100644 docs/final-audit/admin-digest-replay-contract-2026-09-30/characterization-output-combined-with-685.txt create mode 100644 docs/final-audit/admin-digest-replay-contract-2026-09-30/desired-contract-output-combined-with-685.txt diff --git a/docs/final-audit/admin-digest-replay-contract-2026-09-30/README.md b/docs/final-audit/admin-digest-replay-contract-2026-09-30/README.md index eaf7abaa0..80d4190cb 100644 --- a/docs/final-audit/admin-digest-replay-contract-2026-09-30/README.md +++ b/docs/final-audit/admin-digest-replay-contract-2026-09-30/README.md @@ -170,3 +170,14 @@ Each change was made to the route in a scratch checkout only, and then reverted. - **The fake database sorts stably,** so the tie test proves every row is counted across pages. It does not prove that `.order('id')` is needed: a nondeterministic server order is not modelled. - **Resend is modelled from its documentation, not called.** Rate limits, bounces and key retention were not measured. - **Nothing here touched production.** `finalaudit.md` records `CRON_SECRET` as unset in production, and the latest observed GitHub dispatch exits before dispatching. With the secret unset the route answers 401 to every call, as the unauthorised case shows, so today the digest does not run at all. Once the secret is set, both schedulers fire at 12:30 UTC and the sequential-repeat row above applies every day a GitHub tick lands in the 12:30–12:34 window. + +## Addendum (2026-09-30): combined with #685, the recipient fail-closed fix + +Everything above is the **dated record at `231e8140`**, before #685; `characterization-output.txt` and `desired-contract-output.txt` are kept unchanged as that record. On the combined source (#685 plus this folder, at #677's reviewed head `e90fa5a2`): + +- **Characterization** (`tests/admin-digest-replay-contract.test.ts`, 20/20, saved as `characterization-output-combined-with-685.txt`): + - The two `super_admins` read cases (error, throw) are re-pinned from "200 ok, built-in admin only" to **502 `adminDigest.recipientsUnavailable`, zero provider attempts**. + - The empty-recipient override now also answers the strict reader the route uses (`readSuperAdminRecipients` → `{ emails: [], failure: null }`), so that case still exercises the empty-recipient branch. + - Recipient reads are paged since #685 (a one-row page plus an empty page per call). The concurrency barrier now holds only the **first page of each invocation**, so it no longer pins how many pages a read takes. The replay and concurrency send counts are unchanged: 2 per admin. +- **Desired contract** (`tests/desired-contract.test.ts`): with #685, C6 passes and C1–C5 still fail (`desired-contract-output-combined-with-685.txt`: 5 failed, 2 passed). **Only C6 is promoted**, into the characterization file. C1–C5 stay here as unmet contract cases: one digest per occurrence; retry only the unaccepted recipient; same key and frozen payload on an ambiguous send; no row in two digests. Run now, this file shows 5 failed and 1 passed (the control). +- Duplicate delivery is **not** fixed. No live email was sent, and no provider delivery is claimed. diff --git a/docs/final-audit/admin-digest-replay-contract-2026-09-30/characterization-output-combined-with-685.txt b/docs/final-audit/admin-digest-replay-contract-2026-09-30/characterization-output-combined-with-685.txt new file mode 100644 index 000000000..d1d625c83 --- /dev/null +++ b/docs/final-audit/admin-digest-replay-contract-2026-09-30/characterization-output-combined-with-685.txt @@ -0,0 +1,29 @@ + + RUN v4.1.10 + + ✓ tests/admin-digest-replay-contract.test.ts > fixture > two recipients (the built-in admin and one super_admins row), and both schedulers name 12:30 UTC + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > refuses every unauthorised call with 401 before touching the database or the provider + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a quiet day (nothing in the last 24 h) sends nothing, does not look up recipients, and answers 200 + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > empty recipients answers 200 with the count and sends nothing — reachable only by replacing the lookup, because the built-in admin is always present + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins row that repeats the env allowlist in another case is one recipient, sent once + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > with no RESEND_API_KEY it answers 200 ok with sent 0 and skipped 2, having delivered nothing (the JOB-EF2453D9F633 body) + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > pages past a 1,000-row server cap: 2,345 rows in the window (all tied on created_at and title) are counted exactly, in 4 reads + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > exactly 20,000 rows is a complete read; 20,001 is a truncated one and answers 502 without looking up recipients or sending + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a failed first page of the notification feed answers 502 and sends nothing + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a failed THIRD page discards the two good pages: 502, nothing sent + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins read that answers an error fails closed (#685): 502, and not one email + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins read that throws fails closed (#685): 502, and not one email + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > SEQUENTIAL REPEAT: a second call for the same 12:30 occurrence sends every admin the identical digest again — 4 attempted, 4 accepted, nothing written + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > SEQUENTIAL REPEAT inside the dispatcher window: Vercel at 12:30 and GitHub at 12:33 both send; the second digest re-reports the first one’s rows + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > CONCURRENT: two workers that both finish reading before either sends each email every admin — 4 attempted, 4 accepted + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ONE RECIPIENT FAILS (refuse) AFTER ANOTHER SUCCEEDS: 502 {sent 1, failed 1}; the retry re-sends to the admin who already had it + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ONE RECIPIENT FAILS (network-error) AFTER ANOTHER SUCCEEDS: 502 {sent 1, failed 1}; the retry re-sends to the admin who already had it + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ACCEPTED BEFORE A TIMEOUT: the provider accepts both, the handler reports one failure and 502; the retry gives BOTH admins a second copy + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > PARTIAL DELIVERY STATUS: 502 whenever any recipient fails, 200 only when none does; the body always carries the per-run counts + ✓ tests/admin-digest-replay-contract.test.ts > admin-digest contract (promoted) > C6 a run that could not read its recipient list does not report a clean success + + Test Files 1 passed (1) + Tests 20 passed (20) + Start at + Duration + diff --git a/docs/final-audit/admin-digest-replay-contract-2026-09-30/desired-contract-output-combined-with-685.txt b/docs/final-audit/admin-digest-replay-contract-2026-09-30/desired-contract-output-combined-with-685.txt new file mode 100644 index 000000000..c7ba91208 --- /dev/null +++ b/docs/final-audit/admin-digest-replay-contract-2026-09-30/desired-contract-output-combined-with-685.txt @@ -0,0 +1,127 @@ + + RUN v4.1.10 + + ✓ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > CONTROL (passes): one on-time run delivers exactly one digest to each admin + × docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C1 one occurrence, two sequential calls: each admin receives exactly one digest + → expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 } + × docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C2 one occurrence, two concurrent calls: each admin receives exactly one digest + → expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 } + × docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C3 a retry after one recipient failed attempts ONLY that recipient + → expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 2 } + × docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C4 an ambiguous send (accepted, then timed out) is retried under the SAME idempotency key, and nobody else is re-sent + → expected 'object' to be 'string' // Object.is equality + × docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C5 Vercel at 12:30 and GitHub at 12:33: no activity row is reported in two digests + → expected [ { …(5) }, { …(5) } ] to have a length of 1 but got 2 + ✓ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C6 a run that could not read its recipient list does not report a clean success + +⎯⎯⎯⎯⎯⎯⎯ Failed Tests 5 ⎯⎯⎯⎯⎯⎯⎯ + + FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C1 one occurrence, two sequential calls: each admin receives exactly one digest +AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 } + +- Expected ++ Received + + { +- "admin-1": 1, +- "admin-2": 1, ++ "admin-1": 2, ++ "admin-2": 2, + } + + ❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:116:24 + 114| await runAt('2026-09-30T12:30:00Z'); + 115| await runAt('2026-09-30T12:30:00Z'); + 116| expect(accepted()).toEqual(once()); + | ^ + 117| }); + 118| + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/5]⎯ + + FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C2 one occurrence, two concurrent calls: each admin receives exactly one digest +AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 } + +- Expected ++ Received + + { +- "admin-1": 1, +- "admin-2": 1, ++ "admin-1": 2, ++ "admin-2": 2, + } + + ❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:134:24 + 132| await Promise.all([call(), call()].map((p) => p.finally(() => re… + 133| } finally { clearTimeout(bound); } + 134| expect(accepted()).toEqual(once()); + | ^ + 135| }); + 136| + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/5]⎯ + + FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C3 a retry after one recipient failed attempts ONLY that recipient +AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 2 } + +- Expected ++ Received + + { +- "admin-1": 1, ++ "admin-1": 2, + "admin-2": 2, + } + + ❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:143:25 + 141| state.mode = {}; + 142| await runAt('2026-09-30T12:30:00Z'); + 143| expect(attempted()).toEqual({ 'admin-1': 1, 'admin-2': 2 }); + | ^ + 144| expect(accepted()).toEqual(once()); + 145| }); + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/5]⎯ + + FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C4 an ambiguous send (accepted, then timed out) is retried under the SAME idempotency key, and nobody else is re-sent +AssertionError: expected 'object' to be 'string' // Object.is equality + +Expected: "string" +Received: "object" + + ❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:158:41 + 156| const [first, retry] = ambiguous; + 157| // A real key, the same key, and the same payload, so the provider… + 158| expect(typeof first.idempotencyKey).toBe('string'); + | ^ + 159| expect(first.idempotencyKey).not.toBe(''); + 160| expect(retry.idempotencyKey).toBe(first.idempotencyKey); + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/5]⎯ + + FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C5 Vercel at 12:30 and GitHub at 12:33: no activity row is reported in two digests +AssertionError: expected [ { …(5) }, { …(5) } ] to have a length of 1 but got 2 + +- Expected ++ Received + +- 1 ++ 2 + + ❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:172:69 + 170| await runAt('2026-09-30T12:33:00Z'); + 171| const toSecond = state.attempts.filter((a) => a.to === SECOND && a… + 172| expect(toSecond.filter((a) => a.html.includes('Signup Alpha'))).to… + | ^ + 173| }); + 174| + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/5]⎯ + + + Test Files 1 failed (1) + Tests 5 failed | 2 passed (7) + Start at + Duration + diff --git a/docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts b/docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts index 504e48470..57eda9d39 100644 --- a/docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts +++ b/docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts @@ -102,7 +102,7 @@ afterEach(() => { } }); -describe('admin-digest idempotency contract (UNRESOLVED on current main — expected to fail)', () => { +describe('admin-digest idempotency contract (UNRESOLVED — C1–C5 expected to fail)', () => { it('CONTROL (passes): one on-time run delivers exactly one digest to each admin', async () => { happen('2026-09-30T08:00:00Z', 'Signup Alpha'); expect((await runAt('2026-09-30T12:30:00Z')).status).toBe(200); @@ -172,10 +172,7 @@ describe('admin-digest idempotency contract (UNRESOLVED on current main — expe expect(toSecond.filter((a) => a.html.includes('Signup Alpha'))).toHaveLength(1); }); - it('C6 a run that could not read its recipient list does not report a clean success', async () => { - happen('2026-09-30T08:00:00Z', 'Signup Alpha'); - state.failSuperAdmins = true; - const res = await runAt('2026-09-30T12:30:00Z'); - expect(res.body.ok).toBe(false); - }); + // C6 (a run that could not read its recipient list does not report a clean success) failed on + // main at 231e8140, passes with #685 (desired-contract-output-combined-with-685.txt), and was + // promoted to tests/admin-digest-replay-contract.test.ts. C1–C5 remain unmet. }); diff --git a/tests/admin-digest-replay-contract.test.ts b/tests/admin-digest-replay-contract.test.ts index cd3302ca0..f4bfcb03e 100644 --- a/tests/admin-digest-replay-contract.test.ts +++ b/tests/admin-digest-replay-contract.test.ts @@ -41,7 +41,7 @@ const state = vi.hoisted(() => ({ writes: [] as { table: string; op: string }[], /** Fault injection: the Nth read of a table answers an error, or throws. */ failRead: null as null | { table: string; call: number; how: 'error' | 'throw' }, - /** A read barrier: every party must COMPLETE this table's read before any continues. */ + /** A read barrier: every party must COMPLETE this table's first-page read before any continues. */ barrier: null as null | { table: string; parties: number; arrived: number; attemptsAtRelease: number | null; release: () => void; gate: Promise }, /** Replaces the recipient lookup's result (the only way to reach the empty-recipient branch). */ recipientsOverride: null as string[] | null, @@ -55,6 +55,10 @@ vi.mock('@/lib/feedback/notify', async (importOriginal) => { ...actual, allSuperAdminEmails: (...args: Parameters) => (state.recipientsOverride ? Promise.resolve(state.recipientsOverride) : actual.allSuperAdminEmails(...args)), + // Since #685 the route reads recipients through the strict reader; the override + // answers in its success shape so the empty-recipient branch is still the one exercised. + readSuperAdminRecipients: (...args: Parameters) => + (state.recipientsOverride ? Promise.resolve({ emails: state.recipientsOverride, failure: null }) : actual.readSuperAdminRecipients(...args)), }; }); @@ -97,6 +101,11 @@ function clientOver(inner: InMemorySupabase) { const original = qb[op].bind(qb); qb[op] = (...args: unknown[]) => { state.writes.push({ table, op }); return original(...args); }; } + // Paged reads (readAll) ask for .range(from, to); remember `from` so the barrier can hold + // only the FIRST page of each invocation, without pinning how many pages a read takes. + let rangeFrom = 0; + const originalRange = qb.range.bind(qb); + qb.range = (...args: unknown[]) => { rangeFrom = Number(args[0]); return originalRange(...args); }; const originalThen = qb.then.bind(qb) as (f?: (v: unknown) => unknown, r?: (e: unknown) => unknown) => Promise; qb.then = ((onFulfilled?: (v: unknown) => unknown, onRejected?: (e: unknown) => unknown) => { state.reads[table] = (state.reads[table] ?? 0) + 1; @@ -108,7 +117,7 @@ function clientOver(inner: InMemorySupabase) { } const b = state.barrier; return originalThen(async (reply) => { - if (b && b.table === table) { + if (b && b.table === table && rangeFrom === 0) { b.arrived += 1; if (b.arrived >= b.parties) { b.attemptsAtRelease = state.attempts.length; b.release(); } await b.gate; @@ -309,14 +318,16 @@ describe('admin-digest replay: current main, observed (not endorsed)', () => { expect(state.attempts).toHaveLength(0); }); + // Until #685 these two answered 200 ok and emailed the built-in admin alone; that + // observation is kept, dated, in docs/final-audit/admin-digest-replay-contract-2026-09-30/. for (const how of ['error', 'throw'] as const) { - it(`a super_admins read that ${how === 'error' ? 'answers an error' : 'throws'} is swallowed: 200 ok, and only the built-in admin is emailed`, async () => { + it(`a super_admins read that ${how === 'error' ? 'answers an error' : 'throws'} fails closed (#685): 502, and not one email`, async () => { happen('2026-09-30T08:00:00Z', 'Signup Alpha'); state.failRead = { table: 'super_admins', call: 1, how }; const res = await runAt('2026-09-30T12:30:00Z'); - expect(res.status).toBe(200); - expect(res.body).toMatchObject({ ok: true, sent: 1, failed: 0, recipients: 1 }); - expect(attempted()).toEqual({ 'admin-1': 1, 'admin-2': 0 }); + expect(res.status).toBe(502); + expect(res.body).toEqual({ ok: false, error: 'adminDigest.recipientsUnavailable' }); + expect(attempted()).toEqual({ 'admin-1': 0, 'admin-2': 0 }); }); } @@ -355,9 +366,9 @@ describe('admin-digest replay: current main, observed (not endorsed)', () => { it('CONCURRENT: two workers that both finish reading before either sends each email every admin — 4 attempted, 4 accepted', async () => { happen('2026-09-30T08:00:00Z', 'Signup Alpha'); vi.setSystemTime(new Date('2026-09-30T12:30:00Z')); - barrierOn('super_admins', 2); // the last read before the send fan-out + barrierOn('super_admins', 2); // the first page of the last read before the send fan-out const [a, b] = await Promise.all([call(), call()]); - expect(state.barrier!.arrived).toBe(2); + expect(state.barrier!.arrived).toBe(2); // one first page per invocation; later pages are not counted expect(state.barrier!.attemptsAtRelease).toBe(0); // neither had sent when both had read expect([a.status, b.status]).toEqual([200, 200]); expect(await a.json()).toMatchObject({ ok: true, sent: 2 }); @@ -420,3 +431,18 @@ describe('admin-digest replay: current main, observed (not endorsed)', () => { expect(accepted()).toEqual({ 'admin-1': 2, 'admin-2': 1 }); }); }); + +// ── Promoted from the desired contract (docs/final-audit/admin-digest-replay-contract-2026-09-30) ── +// C6 failed on main at 231e8140 and passes since #685. Only C6 is promoted: C1–C5 (one digest per +// occurrence, retry only the unaccepted recipient, same key and frozen payload on an ambiguous send, +// no row in two digests) are still unmet and stay in the evidence folder as failing contract cases. +describe('admin-digest contract (promoted)', () => { + it('C6 a run that could not read its recipient list does not report a clean success', async () => { + happen('2026-09-30T08:00:00Z', 'Signup Alpha'); + state.failRead = { table: 'super_admins', call: 1, how: 'error' }; + const res = await runAt('2026-09-30T12:30:00Z'); + expect(res.body.ok).toBe(false); + expect(res.status).toBe(502); + expect(state.attempts).toHaveLength(0); + }); +});