test: guard the DB schema composer against unwired DDL modules (#5682) - #5810
Merged
Conversation
Adding a module under server/lib/db/schema/ and wiring only its import and re-export shipped a CREATE TABLE that never ran: ensureSchemaImpl() executes only what buildUpgradeDdl()/buildCatalogDdl() enumerate by hand, so a fresh install (and every peer install upgrading) silently missed the table and the feature failed at first query with a Postgres "relation does not exist". Nothing caught it — schema.test.js checks a hand-written module list, and the server/lib barrel guard's readdirSync is non-recursive. - New server/lib/db/schema/index.test.js derives its expectations from the directory listing: every non-test module must be imported by index.js, must export at least one *Ddl array, each such array's first statement must appear in the composed lists, and each module must have a README row. Membership is checked on the first statement rather than array identity so catalog.js (two separately-positioned arrays) and audit.js (auditDdl plus generated triggers) both pass without encoding any ordering — statement order stays the job of schema.test.js and db.catalogDdlParity.test.js. - server/lib/README.md gains a db/ row so the documented discovery grep can find the schema modules at all. - server/lib/index.test.js and client/src/lib/index.test.js now accept .jsx alongside .js (matching the hooks/services guards), so a future .jsx cannot bypass the barrel/README parity check. Claude-Session: https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE
Local review round 1: matching only an array's first statement let a composer spread part of a module's DDL and still pass, so the remaining CREATE TABLE / CREATE INDEX statements would silently never run. Check every statement (still membership-only, so no ordering is encoded) and report the first uncomposed one in the failure message. Also refresh the stale "non-test .js file" wording in server/lib/README.md now that the guard covers .jsx too. Claude-Session: https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE
Local review round 2: the guard only inspected `*Ddl` arrays, so a module that emits DDL from a generator — the shape audit.js already uses with buildAuditTriggers() — could be dropped from the composer and still pass, leaving its statements unrun. Also accept a zero-arg build*Ddl/build*Triggers() export, call it, and hold its statements to the same membership check. Claude-Session: https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
server/lib/db/schema/splits the boot DDL into per-domain modules thatindex.jscomposes by hand intobuildUpgradeDdl()/buildCatalogDdl(), the two listsensureSchemaImpl()runs on every boot. Nothing verified that a module in that directory actually reaches a composed list. Addingserver/lib/db/schema/foo.jsand wiring only theimport+export {}block — the obvious half of the change — shipped aCREATE TABLEthat never executes, so a fresh install (and every peer install upgrading) is missing the table and the feature fails at first query with a Postgresrelation does not exist. The existingschema.test.jschecks a hand-written module list, and the directory-level guard inserver/lib/index.test.jsuses a non-recursivereaddirSync, so nothing underserver/lib/db/was covered.Adds a directory-driven guard modeled on
server/lib/editorial/checkInfraBarrel.test.js.Changes
server/lib/db/schema/index.test.js(new) — derives its expectations fromreaddirSyncrather than a maintained list. For every non-test module it asserts:index.jsimports it; it contributes DDL; every statement of that contribution appears in the composed lists; and it has a backtick-wrappedREADME.mdrow.<domain>Ddlstatement array, and a zero-argbuild*Ddl/build*Triggers()generator (the shapeaudit.jsuses). Both are held to the same membership check, so generated statements can't be dropped either.catalog.js(two separately-positioned arrays) andaudit.js(auditDdlplus generated triggers) pass without encoding any ordering. Statement order stays out of scope —schema.test.jsanddb.catalogDdlParity.test.jsown it, and no DDL was touched.server/lib/README.md— newdb/row so the documented discovery grep can find the schema modules at all; refreshed the guard-contract line now that.jsxcounts.server/lib/index.test.js,client/src/lib/index.test.js— filters now accept.jsx(matching the hooks/services guards), so a future.jsxin either directory can't bypass the barrel/README parity check.Test plan
Verified the new guard is actually red for each failure mode by temporarily adding a throwaway
server/lib/db/schema/zzz.jsand deleting it afterwards:build*Triggers()that is never composed → fails, naming the uncomposed generated statement.Suites:
cd server && npm test→ 1835 files passed, 37302 tests passed, 1 skipped.cd client && npm test→ 855 files, 10719 tests passed.npm run test:db); the realportosdatabase was never touched.Closes #5682
https://claude.ai/code/session_01GMxEz43s3YCLaVZV9KmVwE