feat(metaschema-schema): port system-controlled database standing columns - #128
Conversation
…umns Mirrors constructive-db fac0698a8ce / 7218535ebd1: suspended_at + suspended_reason on metaschema_public.database, the paired check constraint, the suspended_at index, and the @behavior -insert -update column comments. Bundle repackaged.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟡 2 medium 💬 Inline comments (2)
🧹 Nitpicks (1) — 🟢 1 low
The change introduces a database-suspension feature by editing the write-once
Reviewed commit: ac02139 |
There was a problem hiding this comment.
This PR adds database suspension columns (suspended_at, suspended_reason), a CHECK constraint, and an index to the write-once metaschema_public.database table, plus the regenerated sql/metaschema-schema--0.44.0.sql artifact.
Key findings
- 🟡 Suspended columns lack promised guard trigger — table.sql:53
- 🟡 No upgrade path for existing databases — table.sql:28
| COMMENT ON COLUMN metaschema_public.database.suspended_at IS '@behavior -insert -update'; | ||
| COMMENT ON COLUMN metaschema_public.database.suspended_reason IS '@behavior -insert -update'; |
There was a problem hiding this comment.
🟡 security · medium
Suspended columns lack promised guard trigger
The comment at table.sql:24 promises a guard trigger restricting writes to the system role/platform admin, but no trigger or RLS policy on metaschema_public.database exists anywhere in metaschema-schema or metaschema-modules. Only the PostGraphile @behavior -insert -update comments (table.sql:53-54) restrict writes, and those are API-layer hints that do not constrain direct SQL, so any role with UPDATE on the table can set or clear suspension.
📋 Prompt for AI Agents
In packages/metaschema-schema/deploy/schemas/metaschema_public/tables/database/table.sql, the new suspended_at and suspended_reason columns (lines 28-29) are documented as system-controlled with a guard trigger, but no such trigger exists. Add a BEFORE INSERT OR UPDATE trigger function in metaschema_private that raises an exception unless the current role is the system role or a designated platform-admin role, and attach it to metaschema_public.database guarding the suspended_at/suspended_reason columns, so the invariant is enforced at the database level rather than only by the PostGraphile @behavior -insert -update comment.
| suspended_at timestamptz, | ||
| suspended_reason text, |
There was a problem hiding this comment.
🟡 bug · medium
No upgrade path for existing databases
The new suspended_at/suspended_reason columns, database_suspension_chk, and database_suspended_at_idx are added only inside the write-once CREATE TABLE (table.sql:28-50) and the regenerated base install script, with no ALTER TABLE migration, no version bump, and no extension upgrade script in sql/. Installations where metaschema_public.database already exists never receive the new columns, so the suspension feature silently does nothing there while fresh installs get it.
📋 Prompt for AI Agents
In packages/metaschema-schema, add a migration path for existing installs: bump the extension version (package.json, metaschema-schema.control, and the sql/ artifact name) and add an upgrade script (e.g. metaschema-schema--0.44.0--0.45.0.sql) that runs ALTER TABLE metaschema_public.database ADD COLUMN suspended_at timestamptz, ADD COLUMN suspended_reason text;, adds the database_suspension_chk CHECK constraint, and creates database_suspended_at_idx, so databases where the table already exists receive the same columns/constraint/index as fresh installs.
Summary
Ports the
metaschema_public.databasestanding columns that shipped in constructive-db (PR 3701, commitsfac0698a8ce/7218535ebd1) to the upstream mirror so@pgpm/metaschema-schemacan be published with them:Needed because
constructive-io/constructive's integration fixtures deploy the published@pgpm/metaschema-schema(pinned in itspgpm.json), and the new GraphQL standing loader in constructive#1815 reads these columns — CI there currently fails withcolumn "suspended_at" does not exist. After this lands: publish (0.44.2), then bump the pin in constructive.Bundle artifacts regenerated with
pgpm package. Only thedatabase/table.sqlchange is ported here; the mirror still trails constructive-db on unrelatedis_valid_step_up/index.nulls_not_distinctedits.Link to Devin session: https://app.devin.ai/sessions/86a73d903b3546c1afbf5361bfa384b6
Open in Devin Desktop: https://app.devin.ai/desktop/session/86a73d903b3546c1afbf5361bfa384b6?variant=devin
Requested by: @pyramation