diff --git a/src/composition/app.test.ts b/src/composition/app.test.ts index 2dc18d2..023bd68 100644 --- a/src/composition/app.test.ts +++ b/src/composition/app.test.ts @@ -28,6 +28,7 @@ const HEALTHY_REPORT: HealthReport = { mailboxes: [], webhooks: { autoDisabled: [], deliveryFailuresLast24h: 0 }, webauthn: { counterRegressionsLast24h: 0 }, + schema: { expectedMigrationId: 1, appliedMigrationId: 1, missing: [] }, } /** Build a handler over spy deps; the inbox API spy returns a recognizable 299 so delegation is observable. */ diff --git a/src/composition/health.test.ts b/src/composition/health.test.ts index 2743aa0..bdc37e5 100644 --- a/src/composition/health.test.ts +++ b/src/composition/health.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it } from 'vitest' import { createPgliteDb, type Db } from '../db/client.js' -import { migrate } from '../db/migrate.js' +import { LATEST_MIGRATION_ID, migrate } from '../db/migrate.js' import { createPostgresQueue } from '../providers/adapters/postgres-queue/index.js' import { WEBHOOK_DELIVERY_TOPIC } from '../webhooks/delivery.js' import { FORGED_TOKEN_ALERT_THRESHOLD, type HealthReport, runHealthCheck } from './health.js' @@ -434,4 +434,108 @@ describe('runHealthCheck', () => { expect(report.alerts.some((a) => a.startsWith('webauthn-counter-regression'))).toBe(false) }) }) + + // The deploy-without-migrate window. `root.ts` never migrates on cold start, + // so a build can serve traffic against an older schema; before this check the + // only symptom was whichever query happened to fail first. + describe('schema version skew', () => { + it('is silent when the database matches the build', async () => { + const { check } = await fresh() + + const report = await check() + + expect(report.schema).toEqual({ + expectedMigrationId: LATEST_MIGRATION_ID, + appliedMigrationId: LATEST_MIGRATION_ID, + missing: [], + }) + expect(report.alerts.some((a) => a.startsWith('schema-'))).toBe(false) + }) + + it('alerts, names both versions, and names the fix when the database is BEHIND', async () => { + const { database, check } = await fresh() + // Simulate a deploy that landed before its migration ran. + await database.query('DELETE FROM _migrations WHERE id = $1', [LATEST_MIGRATION_ID]) + + const report = await check() + + expect(report.ok).toBe(false) + expect(report.schema.appliedMigrationId).toBe(LATEST_MIGRATION_ID - 1) + const alert = report.alerts.find((a) => a.startsWith('schema-migration-pending: ')) + expect(alert).toBeDefined() + // The message has to be actionable on its own — someone reading a 503 + // body at 3am should not need to go find this file. + expect(alert).toContain(`missing migration(s) ${LATEST_MIGRATION_ID}`) + expect(alert).toContain(`expects through ${LATEST_MIGRATION_ID}`) + expect(alert).toContain('npm run migrate') + }) + + it('alerts when the database is AHEAD of the build — a rollback, not a no-op', async () => { + const { database, check } = await fresh() + await database.query('INSERT INTO _migrations (id, name) VALUES ($1, $2)', [ + LATEST_MIGRATION_ID + 1, + 'from-a-newer-build', + ]) + + const report = await check() + + expect(report.ok).toBe(false) + expect(report.schema.appliedMigrationId).toBe(LATEST_MIGRATION_ID + 1) + expect(report.alerts.some((a) => a.startsWith('schema-newer-than-build: '))).toBe(true) + }) + + // A GENUINELY empty database — nothing migrated, no application tables at + // all. This is the fresh-install case, and the one the diagnostic exists + // for. An earlier version of this test migrated first and then dropped only + // `_migrations`, which left every other table in place and so proved + // nothing: the real code threw `relation "queue_jobs" does not exist` + // before ever reaching the schema check (adversarial review, 2026-08-02). + it('DIAGNOSES a completely empty database rather than throwing on the first missing table', async () => { + const database = await createPgliteDb() + db = database + const queue = createPostgresQueue(database) + + const report = await runHealthCheck({ db: database, queue, pushConfigured: true }) + + expect(report.ok).toBe(false) + expect(report.schema.appliedMigrationId).toBeNull() + const alert = report.alerts.find((a) => a.startsWith('schema-migration-pending: ')) + expect(alert).toContain('never been migrated') + expect(alert).toContain('npm run migrate') + // And it says why the rest of the report is empty, rather than pretending + // those sections were checked and found healthy. + expect(report.alerts.some((a) => a.startsWith('health-checks-unavailable: '))).toBe(true) + }) + + it('reports a GAP in the applied history — max(id) alone would call this healthy', async () => { + const { database, check } = await fresh() + // 1..26 + 29 applied, 27 and 28 missing. `max(id)` equals the build's + // latest, so a highest-id-only check sees nothing wrong. + await database.query('DELETE FROM _migrations WHERE id IN ($1, $2)', [ + LATEST_MIGRATION_ID - 2, + LATEST_MIGRATION_ID - 1, + ]) + + const report = await check() + + expect(report.ok).toBe(false) + expect(report.schema.appliedMigrationId).toBe(LATEST_MIGRATION_ID) + expect(report.schema.missing).toEqual([LATEST_MIGRATION_ID - 2, LATEST_MIGRATION_ID - 1]) + const alert = report.alerts.find((a) => a.startsWith('schema-migration-pending: ')) + expect(alert).toContain( + `missing migration(s) ${LATEST_MIGRATION_ID - 2}, ${LATEST_MIGRATION_ID - 1}`, + ) + }) + + it('reports an existing-but-empty _migrations table as missing everything', async () => { + const { database, check } = await fresh() + await database.query('DELETE FROM _migrations') + + const report = await check() + + expect(report.ok).toBe(false) + expect(report.schema.appliedMigrationId).toBeNull() + expect(report.schema.missing).toContain(LATEST_MIGRATION_ID) + }) + }) }) diff --git a/src/composition/health.ts b/src/composition/health.ts index 0d69527..be1907e 100644 --- a/src/composition/health.ts +++ b/src/composition/health.ts @@ -10,6 +10,24 @@ * the platform aggregates logs (CHARTER.md §4), and this endpoint is the * one pull-based surface those logs can't provide. * + * ## Schema version: the one check about the deploy itself + * + * `src/composition/root.ts` deliberately does not migrate on cold start — + * schema changes are an operator step (`scripts/migrate.ts`). Every deploy + * therefore opens a window where the new build runs against the previous + * schema until someone runs `npm run migrate`, and with auto-deploy-on-merge + * that window opens without anyone deciding to open it. + * + * Observed 2026-08-02: HT-101 shipped three migrations, the deploy landed + * first, and an `imap-fetch` cron spent the gap erroring every two minutes + * against tables that did not exist yet. Nothing named the cause; the only + * symptom was a failing cron. This check turns that into a 503 on a URL, with + * the fix in the message. + * + * It deliberately does NOT migrate anything. Reporting the skew keeps the + * "schema changes are an operator step" rule intact while removing the part + * that made it dangerous — that the violation was silent. + * ## What it reports, and the alert each section can trip * * - **Queue** (`PostgresQueue.getStats` + a 24h dead-letter window): @@ -40,6 +58,11 @@ * 72h means renewal has been failing for days — caught while there is * still runway). `disconnected` mailboxes are deliberately silent: that * state is an operator's own explicit action (HT-47). + * - **Schema version** (`_migrations` vs the build's own + * `LATEST_MIGRATION_ID`): `schema-migration-pending` when the database is + * BEHIND the running build, `schema-newer-than-build` when it is ahead. + * Unlike every other section here, this one reports on the DEPLOYMENT, not + * on traffic — see the dedicated section below for why it earns a place. * - **Webhooks** (HT-69; specs/modules/substrate-v1.md §5: "surfaced by * `/api/v1/internal/health` (runbook Part G gains a section)"): * `webhook-endpoint-auto-disabled` for every `webhook_endpoints` row @@ -85,6 +108,7 @@ */ import type { Db } from '../db/client.js' +import { LATEST_MIGRATION_ID, MIGRATION_IDS } from '../db/migrate.js' import type { QueueStats } from '../providers/adapters/postgres-queue/index.js' import type { InboundDeliveryStatus } from '../store/inbound-deliveries.js' import { WEBHOOK_DELIVERY_TOPIC } from '../webhooks/delivery.js' @@ -169,8 +193,27 @@ export interface HealthReport { /** `webauthn_credentials` rows whose `sign_count_regression_at` falls in the last 24h. Any value `> 0` trips the `webauthn-counter-regression` alert. */ counterRegressionsLast24h: number } + /** + * Whether the database's schema matches what this build expects. See the + * module doc's Schema version section — this is the one check that reports + * on the DEPLOYMENT rather than on traffic. + */ + schema: { + /** Highest migration id in this build (`LATEST_MIGRATION_ID`). */ + expectedMigrationId: number + /** Highest id actually recorded in `_migrations`; `null` when the table does not exist, or exists with no rows. */ + appliedMigrationId: number | null + /** Every migration id this build has that the database does not — empty when in step. A GAP is reported here, not hidden behind `max(id)`. */ + missing: number[] + } } +/** Alert code for a database behind the running build — the deploy-without-migrate window. */ +const SCHEMA_BEHIND_ALERT = 'schema-migration-pending' + +/** Alert code for a database AHEAD of the running build — a rollback, or a deploy that never shipped. */ +const SCHEMA_AHEAD_ALERT = 'schema-newer-than-build' + /** Every ledger status, for zero-filling {@link HealthReport.ingest}'s per-status map (a status with no 24h rows must still appear, as `0`). */ const ALL_DELIVERY_STATUSES: InboundDeliveryStatus[] = [ 'received', @@ -187,9 +230,142 @@ const ALL_DELIVERY_STATUSES: InboundDeliveryStatus[] = [ * fails (a down database IS a health-check failure; the endpoint's generic * 500 — and the monitor alerting on any non-200 — reports it honestly). */ +/** + * Read the database's applied-migration state. Split out and run FIRST because + * every other check in this module queries an application table, and on a + * database that has not been migrated those queries throw — burying the one + * diagnostic that would have explained why (found by adversarial review, + * 2026-08-02; the original version failed exactly the fresh-install case it + * was written for, and its test passed only because it migrated first and + * dropped `_migrations` afterwards, leaving every other table in place). + * + * Existence is checked in its OWN statement. A single + * `CASE WHEN to_regclass(...) ... ELSE (SELECT max(id) FROM _migrations)` + * does not work: Postgres resolves the relation at parse time, so the subquery + * errors before the CASE can short-circuit. + * + * Returns every applied id, not just the highest. `max(id)` alone would call a + * database healthy when it holds 1..26 plus 29 and is missing 27 and 28 — + * `migrate()`'s single transaction makes that unreachable through the normal + * path, but manual repair and hand-edited bookkeeping are exactly when a + * health check earns its place. + */ +async function readSchemaState(db: Db): Promise<{ applied: number[] | null }> { + const table = await db.query<{ exists: string | null }>( + `SELECT to_regclass('_migrations')::text AS exists`, + ) + if (table[0]?.exists == null) { + return { applied: null } + } + const rows = await db.query<{ id: number }>('SELECT id FROM _migrations ORDER BY id') + return { applied: rows.map((r) => r.id) } +} + +/** Build the schema section + any alert it trips. Pure, so the ordering above stays obvious. */ +function assessSchema(applied: number[] | null): { + section: HealthReport['schema'] + alerts: string[] +} { + const expectedIds = [...MIGRATION_IDS] + if (applied === null) { + return { + section: { + expectedMigrationId: LATEST_MIGRATION_ID, + appliedMigrationId: null, + missing: expectedIds, + }, + alerts: [ + `${SCHEMA_BEHIND_ALERT}: the database has no _migrations table — it has never been migrated. This build expects migration ${LATEST_MIGRATION_ID}. Run \`npm run migrate\`.`, + ], + } + } + const appliedSet = new Set(applied) + const missing = expectedIds.filter((id) => !appliedSet.has(id)) + const highestApplied = applied.length === 0 ? null : applied[applied.length - 1] + const section = { + expectedMigrationId: LATEST_MIGRATION_ID, + appliedMigrationId: highestApplied, + missing, + } + if (missing.length > 0) { + // Named individually rather than as "behind by N": a GAP is a different + // problem from simply being behind, and the list says which it is. + return { + section, + alerts: [ + `${SCHEMA_BEHIND_ALERT}: database is missing migration(s) ${missing.join(', ')}; this build expects through ${LATEST_MIGRATION_ID}. Run \`npm run migrate\`. Until then, code paths using the newer schema will fail.`, + ], + } + } + if (highestApplied !== null && highestApplied > LATEST_MIGRATION_ID) { + return { + section, + alerts: [ + `${SCHEMA_AHEAD_ALERT}: database is at migration ${highestApplied} but this build only knows ${LATEST_MIGRATION_ID}. The running deployment is older than the schema — likely a rollback, or a deploy that never shipped.`, + ], + } + } + return { section, alerts: [] } +} + export async function runHealthCheck(deps: HealthCheckDeps): Promise { const alerts: string[] = [] + // --- Schema version, FIRST. ----------------------------------------------- + // Everything below queries an application table; on an un-migrated or older + // schema those throw. Knowing the schema state up front is what lets a + // failure below be reported as "you have not migrated" instead of a generic + // 500 naming whichever table happened to be queried first. + const { applied } = await readSchemaState(deps.db) + const schemaAssessment = assessSchema(applied) + alerts.push(...schemaAssessment.alerts) + + // Everything from here queries an application table. If the schema is behind, + // a failure here is EXPLAINED by that, so report the explanation rather than + // letting a raw "relation ... does not exist" reach the operator as a 500. + // A failure with an in-step schema is a real fault and still propagates. + try { + return await runTrafficChecks(deps, alerts, schemaAssessment.section) + } catch (err) { + if (schemaAssessment.alerts.length === 0) { + throw err + } + return { + ok: false, + alerts: [ + ...schemaAssessment.alerts, + `health-checks-unavailable: the remaining checks could not run against this schema (${err instanceof Error ? err.message : String(err)}).`, + ], + generatedAt: new Date().toISOString(), + queue: { ready: 0, oldestReadyAgeSeconds: null, deadLettered: 0, deadLetteredLast24h: 0 }, + ingest: { last24hByStatus: emptyStatusCounts(), deadLetterTotal: 0 }, + forgedTokens: { + deliveriesLast24h: 0, + tokensLast24h: 0, + alertThreshold: FORGED_TOKEN_ALERT_THRESHOLD, + }, + mailboxes: [], + webhooks: { autoDisabled: [], deliveryFailuresLast24h: 0 }, + webauthn: { counterRegressionsLast24h: 0 }, + schema: schemaAssessment.section, + } + } +} + +/** Zero-filled per-status map, for the degraded report above. */ +function emptyStatusCounts(): Record { + return Object.fromEntries(ALL_DELIVERY_STATUSES.map((k) => [k, 0])) as Record< + InboundDeliveryStatus, + number + > +} + +/** Every check that reads an application table. Split out so the schema guard above can wrap it. */ +async function runTrafficChecks( + deps: HealthCheckDeps, + alerts: string[], + schema: HealthReport['schema'], +): Promise { // --- Queue. --------------------------------------------------------------- const stats = await deps.queue.getStats() const deadLetteredLast24hRows = await deps.db.query<{ count: number }>( @@ -363,6 +539,7 @@ export async function runHealthCheck(deps: HealthCheckDeps): Promise (m.id > max ? m.id : max), 0) + +/** + * Every migration id this build carries, ascending. `src/composition/health.ts` + * compares the whole set against `_migrations` rather than only the highest: + * `max(id)` alone calls a database healthy when it holds 1..26 plus 29 and is + * missing 27 and 28. `migrate()`'s single transaction makes that unreachable + * through the normal path — manual repair and hand-edited bookkeeping are + * precisely the cases a health check exists for. + */ +export const MIGRATION_IDS: readonly number[] = MIGRATIONS.map((m) => m.id).sort((a, b) => a - b) + /** * Split a migration's SQL body into individual statements on `;`. *