Skip to content

feat(db): verify schema at startup and expose /ready - #553

Closed
titankrypt-gif wants to merge 1 commit into
Divinity-Labs:mainfrom
titankrypt-gif:feat/issue-352-schema-verification
Closed

titankrypt-gif wants to merge 1 commit into
Divinity-Labs:mainfrom
titankrypt-gif:feat/issue-352-schema-verification

Conversation

@titankrypt-gif

Copy link
Copy Markdown

Summary

Adds a cheap startup schema check, a public /ready probe that reports schema readiness, tests for the mismatch paths, and documentation of why the repository has no Prisma migrations yet.

Problem

prisma/ has only schema.prisma and no migrations directory. A deployed process could start cleanly against a database missing a table or column and only fail later, at request time, with an opaque query error. There was also no readiness signal that distinguishes "the process is up" from "this instance can serve the schema it was built for".

Solution

  • src/db/schemaVerify.ts:
    • EXPECTED_SCHEMA lists the physical tables/columns used by the code (Prisma @@map / @map names).
    • verifyDatabaseSchema(pool) runs two information_schema queries and reports missing tables and columns.
    • assertSchemaValid(pool) throws DatabaseSchemaMismatchError naming every missing entity.
    • Results are cached so /ready does not re-query on every probe.
  • src/index.ts calls assertSchemaValid(pgPool) after the database connection and before loading pairs or listening. A mismatch exits through the existing fatal-startup path.
  • GET /ready returns 200 with ready: true when the cached check passed, and 503 with missingTables / missingColumns otherwise. Database outages return 503 with database_unavailable rather than being misreported as schema drift.
  • /ready is public and included in the rate-limit allow-list, like /status.
  • docs/database-schema.md records the current db push strategy and rationale, the guard, /ready, and the concrete path to Prisma Migrate.
  • openapi.yaml / openapi.json document the new endpoint.
  • Tests cover the valid schema, a missing table, missing columns, the startup error message, connection failures, /ready 200/503, caching, and the OpenAPI coverage guard.

Validation

  • npx tsc --noEmit - pass
  • npx vitest run src/__tests__/schemaVerification.test.ts src/__tests__/ready.test.ts src/__tests__/status.test.ts tests/openapi.test.ts - 17 passed, 0 failed
  • npm test - 831 passed, 1 skipped, 1 failed: graphqlSchemaDrift.test.ts compares the LF-generated SDL against the CRLF checkout on this Windows workspace. npm run graphql:schema:gen produces no content diff and git checkout -- schema.graphql leaves the tree clean, so this is a local line-ending artifact rather than a schema change.

Risk

  • The guard runs after the existing boot-time prisma db push, so it verifies the result of that push rather than replacing it. The docs call out that adopting Prisma Migrate (baseline, resolve, deploy pipeline) is the follow-up that should remove the boot-time push.
  • EXPECTED_SCHEMA is intentionally explicit. A future model change must add its table/columns there; the startup check will not silently ignore a newly expected field.

Issue

Closes #352

@drips-wave

drips-wave Bot commented Oct 19, 2026

Copy link
Copy Markdown

@titankrypt-gif Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema verification itself is good work and #352 is yours — but two things need to change, and one of them now stops the app booting.

1. The duplicate /ready breaks boot

#544 (issue #334, /live and /ready probes) merged a few minutes ago, and it registers /ready in src/routes/health.ts. This PR registers it again in src/api/rest.ts. Fastify refuses duplicate routes, so the app no longer starts. I merged your branch onto current main and ran it rather than guessing:

FastifyError: Method 'GET' already declared for route '/ready'

tests/openapi.test.ts fails to load at all as a result, because it boots the real app.

/ready belongs to #334, which was assigned to CollinsKRO and landed first, so that part comes out of this PR. That is not a judgement about which implementation is better — it's that the issue was theirs.

2. EXPECTED_SCHEMA has nothing stopping it drifting

This is the more interesting problem, and it's the one I'd most like you to solve.

EXPECTED_SCHEMA is a hand-written list of ten tables and their columns, taken from prisma/schema.prisma's @@map/@map names. Your comment explains why it's explicit rather than a DMMF walk — it has to work in the deployed container and stay cheap — and I agree with that reasoning.

But nothing ties the list to the Prisma schema. So the next migration that adds a column leaves this check passing while the thing it exists to verify has drifted, and a migration that renames one makes it fail on a database that is actually correct. A drift guard that drifts is worse than none, because it is trusted.

This repo already solves exactly this shape of problem twice: src/__tests__/graphqlSchemaDrift.test.ts byte-compares schema.graphql against printSchema() output, and tests/openapi.test.ts enumerates registered routes and fails on any missing from openapi.yaml. Both keep a hand-maintained artifact honest with a test rather than with discipline.

What I'd like is the same pattern here: a test that parses prisma/schema.prisma, derives the mapped table and column names, and asserts EXPECTED_SCHEMA matches exactly — failing in both directions, with a message naming what to add or remove. Then the runtime check stays the cheap two information_schema queries you wrote, and the list cannot rot.

What I'd suggest instead of a second route

Keep verifyDatabaseSchema and its startup call, drop the route, and surface the result through #544's readiness path in src/health.ts. A "the database schema does not match the code" signal is genuinely a readiness concern — the process is up but must not take traffic — so it belongs as a check inside the existing /ready rather than as a competing endpoint. That also gets you the caching and the 503 shape for free.

What's already right

  • Letting query failures propagate so an outage is distinguishable from a mismatch. That distinction is the whole value of the check and it would have been easy to swallow.
  • Caching the successful result, since the schema cannot change under a running process.
  • Parameterising the table list into = ANY($1::text[]) rather than interpolating it.
  • Not querying columns for tables already known to be missing.
  • Five tests covering missing table, missing column, the startup throw, and the connection-failure case separately.

#352 stays yours and stays assigned to you. Drop the route, add the drift guard, and I'll re-review quickly — I'm working through the queue continuously today.

One unrelated note: tests/aggregator.property.test.ts also failed on my merge run, on a random-input case. I don't think it's yours — it's a property test and it passes on main — but if you see it, that's why.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

@titankrypt-gif

Copy link
Copy Markdown
Author

Closing as superseded. #352 was already resolved on main by #537 (merged 2026-10-22), which added src/db/schemaVerify.ts, the startup assertion, the /ready probe, tests and docs/database-schema.md. This branch now conflicts with that implementation and would duplicate it, so it should not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Startup does not verify the database schema matches the code

2 participants