Repository navigation
Own-schema tables with FKs to Interchange tenant/principal (CL-8210) - #799
Merged
Merged
Conversation
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
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
CL-8210 ruling: every custom (non-Interchange) Postgres table Workbench owns lives on its own schema (never
public, never inside Interchange's schemas) and carries real foreign keys to Interchange's tenant/principal id columns withON DELETE CASCADE.Inventory of every
pgTable(/pgSchema(outsidevendor/andpackages/chat/**:packages/notify(notify_dispatch)notifyschematenant_id/principal_idplain texttenant/principal,ON DELETE CASCADEpackages/onboarding(pending_seed)onboardingschematenant_idplain texttenant,ON DELETE CASCADE;user_idleft alone (see below)packages/webhook-triggers(webhook_trigger,repo_review_lease)webhook_triggersschematenant_id(both) andcreated_byplain texttenant(both) andprincipal(created_by),ON DELETE CASCADEpackages/evals(evals.run)evalsschemaInterchange's tenant/principal tables (
vendor/intx/db/src/schema/tenants.ts,principals.ts):tenant.id(text PK),principal.id(text PK, itself FK'd totenant.idcascade). Each fixed package now declares a local, never-migratedhostTenant/hostPrincipalstub (id column only) to carry the drizzle.references()— the same pattern@corbits/artifacts'ssrc/schema.tsalready uses.Forward-only migrations (literal SQL, matching each package's existing pattern) add the FK constraints:
packages/notify/src/migrations.ts(0002),packages/onboarding/src/migrations.ts(0002),packages/webhook-triggers/src/migrations.ts(0005).Column I could not FK:
packages/onboarding'spending_seed.user_idis the workbench web app's own better-auth user id (packages/connections/src/oauth-routes.ts'suser.id), not an Interchange principal id — there is no Interchange row for it to reference, so it stays plain text.Pinned Corbits library compliance (not modified, report only):
@corbits/artifacts— ownartifactsschema;tenant_id/principal_idare real.references()FKs. Fully compliant.@corbits/mailbox— ownmailboxschema;tenant_id/principal_idare real FKs enforced in its migration DDL (runMailboxMigrations), deliberately not restated as drizzle.references()thunks inschema.ts. Compliant in substance.@corbits/memory— ownmemoryschema;tenant_idcolumns are plain text with no FK anywhere (schema or migrations). Not compliant with CL-8210 — flagged for a follow-up ticket against that library, out of scope here (upstream repo, per house rule).New structural check:
scripts/checks/owned-tables-fk.tsfails anypgTable(...)/<schema>.table(...)declaration outsidevendor/that either (a) isn't declared on a namedpgSchema(...), or (b) has atenant_id/principal_idcolumn with no.references(...).packages/chat/src/schema.tsis explicitly exempted as tracked pre-existing debt (out of scope — this PR is barred from touchingpackages/chat/**).Also bumped
scripts/checks/no-product-tenancy.ts's per-filemaxOccurrencesfor the three touched files, since each now also declares a localtenant/principalFK-target stub.Test plan
bun run typecheckbun run lintbun run fmtbun run check:structural(all green, including newowned-tables-fkand updatedno-product-tenancy)bun test scripts/checks/test/owned-tables-fk.test.ts scripts/checks/test/no-product-tenancy.test.ts scripts/checks/test/run.test.tspackages/notify,packages/onboarding,packages/webhook-triggers) — not run: port 5432 was already occupied by another local Postgres/Docker instance and noDATABASE_URLwas set, so these suites skip locally with their usual banner. CI'sdb-suites/e2e-suitejobs provision Postgres and will exercise the new migrations.