feat(hub-projects): generate and apply datatable migrations on project publish/install

[diegoimbert]

Summary

Integrates datatable migrations into the Hub projects feature. When a project is published to the Hub, its scripts/flows/raw apps are scanned for datatable assets (datatable_name/table_name[/column_name]), and a best-effort CREATE TABLE migration is generated per referenced data table from the source workspace's live schema. On import, the shipped migrations can be run against the target workspace.

Stacked PR. Base is tl/workspace-to-hub. The diff also contains the datatable-migrations work (its own branch, change-68b704f7 / diego/git-890-datatable-migrations) because that branch isn't in the base. The integration-specific commits are 198cd33762, af3f87ee65, a4d64a9b08.

Changes

Detection & generation (publish side)

• projectMigrations.ts: detect datatable assets across a project's selected scripts/flows/raw apps and generate a CREATE TABLE migration per data table from the source workspace's live schema.
• Close the generated table set over foreign keys: transitively pull a referenced table's FK targets into the migration (ordered by FK dependency), and drop any FK whose target still isn't in the set so the SQL always runs.
• New "Data table migrations" section in DeployToHub — the publisher can edit/toggle the generated migrations in the bundle drawer.
• Extract the pure SQL-generation out of DatatableSchemaDiff.svelte into datatableSchemaSql.ts so plain .ts modules can import it.

Import side

• projects/install: run/skip modal for the shipped migrations. When the target data table opted into migrations they're applied as recorded migrations (datatable_migrations + _wm_migrations); otherwise as a one-off preview job.
• Missing target data tables are surfaced and skipped via a confirmation.

Backend

• POST /hub/migrations proxy in hub_publish.rs forwarding to the Hub (consistent with the existing thin-proxy pattern).

Fix

• ConfirmationModal gains an optional zIndexClass prop (default z-[9999], backward-compatible); the DB-manager out-of-order confirm uses z-[10000] so it stacks above the DB table editor's still-open preview modal.

Test plan

• projectMigrations.test.ts — asset detection, migration generation, FK closure ordering
• Publish a project whose script/flow/raw app references main/tableA and main/tableB → confirm one migration per table is generated with correct columns and FK-ordered CREATE TABLEs
• A table with an FK to a table not referenced by the project → confirm the FK target is pulled in, or the FK is dropped if unresolvable
• Import the project into a workspace where the target data table opted into migrations → migrations recorded in datatable_migrations + _wm_migrations
• Import where the data table did NOT opt in → migrations run as a one-off preview job
• Import where the target data table is missing → missing-datatable confirmation, migration skipped
• On a migrations-enabled data table, create a table via the DB editor → the out-of-order confirm stacks above the preview modal

Screenshots

Deferred. Exercising the publish→install datatable-migration flow requires the full backend + Hub running; screenshots to be added once verified against a running environment.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	1ba4488
Status:	✅  Deploy successful!
Preview URL:	https://4881132a.windmill.pages.dev
Branch Preview URL:	https://di-hub-projects-datatable.windmill.pages.dev

View logs

[diegoimbert]

/ai review this PR

[claude]

Claude finished @diegoimbert's task in 3m 49s —— View job

---

Review: datatable migrations on project publish/install

Read through all 8 files and traced the generation/install logic. Overall this is well-structured and thoroughly commented — the FK-closure + prune + topological-order pipeline is sound and the test coverage on the generation side is good. A few correctness/robustness points, none blocking:

1. Up migration is not idempotent, but FK closure makes collisions likely (medium)

generateMigrationSql emits CREATE TABLE (not CREATE TABLE IF NOT EXISTS) — datatableSchemaSql.ts:172. Meanwhile expandFkClosure (projectMigrations.ts:128) transitively pulls a referenced table's FK targets into the same transaction. So if a project references orders, customers gets pulled in; on import into a workspace that already has customers (common — it's a shared parent table), the whole BEGIN;…COMMIT; fails on the first CREATE TABLE customers and none of the tables are created. The down migration already uses DROP TABLE IF EXISTS, so the asymmetry is notable. Worth considering CREATE TABLE IF NOT EXISTS (with the caveat that a name collision on a differently-shaped table then silently no-ops). At minimum the "best-effort, editable" framing should call this out. Fix this →

2. orderByFkDependency keys by bare table name while the rest of the pipeline is schema-qualified (low)

expandFkClosure / pruneSchemaForTables use schema-qualified keys (tableKey = schema.table), but orderByFkDependency (projectMigrations.ts:168) builds inSet/byName/deps off t.tableName alone and derives FK targets via .split('.').pop(). Two same-named tables in different schemas collapse in byName, so one is silently dropped from ordered and never gets a CREATE TABLE. Data tables are usually single-schema so this is an edge case, but it's an inconsistency worth aligning (key by schema.table here too).

3. Rollback can DROP pre-existing tables the import didn't create (medium)

The generated sql_down (projectMigrations.ts:297) drops every table in the ordered set, including FK-pulled-in parents (customers) that may have pre-existed in the target. A rollback of a hub-import migration would then drop a table the project never created — potential data loss. It's editable and manually triggered, but the down SQL arguably shouldn't blindly include the transitively-added tables.

4. Import button stays enabled during the migration review modal (low)

In install/+page.svelte:359, planMigrations awaits the review drawer / missing-datatable modal before setting installing = true. The Import button is only disabled on installing || done, so a second click during review kicks off a concurrent install() (second review drawer, duplicated item creation). Consider gating the button while the plan modal is open. Fix this →

5. regenerateMigrations swallows failures silently (low)

DeployToHub.svelte:645 — on error the drafts are cleared and the drawer shows "No data table usage detected", which is indistinguishable from a genuine schema-load failure. A sendUserToast on the catch would help the publisher tell the two apart.

Minor

• PR body describes the datatable asset path as datatable_name/table_name[/column_name], but the SQL parser emits datatable/[schema.]table with the column carried separately (from_column), never in the asset path. The code (parseDatatableAssetPath) matches the parser, not the PR text — so this is just a doc nit, code is correct.
• resolveTable (projectMigrations.ts:101): a schema-qualified ref whose schema doesn't match (e.g. wrongschema.orders) returns undefined rather than falling back to a bare-name lookup of orders. Edge case.
• Backend hub_publish.rs proxy is consistent with the existing thin-proxy pattern (admin gate + slug validation + forward_to_hub) — no issues.
• Nice: the reviewResolve capture-then-clear in closeMigrationReview correctly prevents the on:close-fired run=false from clobbering an explicit Run/Skip choice.

None of these are blockers; #1 and #3 are the ones most worth a decision before shipping (idempotency + rollback data-loss). I didn't run the test suite or exercise the publish→install flow (needs the full backend + Hub, as noted in the PR).
· di/hub-projects-datatable

[diegoimbert]

/ai review this PR

[claude]

Claude finished @diegoimbert's task in 3m 14s —— View job

---

Review: datatable migrations on project publish/install

Re-reviewed the full diff against origin/tl/workspace-to-hub. The five issues from the earlier pass have all been addressed in follow-up commits (IF NOT EXISTS, schema-qualified FK ordering, commented-out down DROPs, planningMigrations import guard, regenerate-failure toast) — nice. Below are the remaining points from a fresh read; none are blockers.

1. The FK ALTER TABLE … ADD CONSTRAINT is not idempotent, so the IF NOT EXISTS fix is only half-complete (medium)

generateMigrationSql (datatableSchemaSql.ts:176-183) now emits CREATE TABLE IF NOT EXISTS, but the foreign keys are still added unconditionally via ALTER TABLE … ADD CONSTRAINT fk_… FOREIGN KEY …, and renderForeignKey (dbQueriesUtils.ts:45-54) emits a named constraint. FK closure pulls a shared parent (customers) into the transaction; if that parent already exists in the target and itself carries an in-set FK, CREATE TABLE IF NOT EXISTS customers correctly no-ops, but the following ALTER TABLE customers ADD CONSTRAINT fk_… runs against the pre-existing table and fails on the duplicate constraint — aborting the whole BEGIN;…COMMIT;. That's the exact "one pre-existing parent kills the transaction" failure the IF NOT EXISTS commit set out to prevent, just narrowed to FK-bearing parents (and any re-run). Worth either guarding the FK add (e.g. DO $$ … IF NOT EXISTS (…pg_constraint…) $$) or at least noting in the SQL comment that FK re-adds aren't idempotent. Fix this →

2. Re-importing the same project can collide on the recorded migration name (low)

applyOneMigration (install/+page.svelte:340) records with a stable name hub_import_${slug}. On a migrations-enabled target, importing the same project twice (across page loads — the button self-disables only within one session) issues two createDatatableMigration calls with the same name. Depending on the backend's uniqueness handling in datatable_migrations this either errors or double-records. A per-import suffix (or a check for an existing entry) would make re-import safe.

3. The missing-datatable modal can't actually cancel the import (low)

planMigrations (install/+page.svelte:308) awaits missingDatatableModal.ask(...) but ignores the boolean. ask() resolves false on cancel / true on confirm and never rejects, so both the "Import without them" button and dismissing the modal proceed with the import. If the intent is to let the user back out when a data table is missing, wire the returned boolean to abort (if (!ok) throw / early-return); otherwise the modal is purely informational and the cancel affordance is misleading.

Minor / nits

• Migration sync ships disabled + comment-only drafts. DeployToHub.svelte:1291 posts all migrationDrafts (including enabled:false and comment-only stubs), relying on the Hub to prune empty-SQL entries. Not a bug given the import side re-filters on enabled && sql.trim(), but the publish payload carries dead weight and couples correctness to the Hub's pruning.
• orderByFkDependency cycle fallback. The topo-sort silently falls back to partial/input order on a cycle (data tables rarely have FK cycles, so fine) — worth a one-line log or comment if you ever expect cycles, since the emitted SQL would then fail at apply time rather than generation time.
• Backend publish_migrations proxy is consistent with the sibling handlers (admin gate + validate_project_slug + source_key + forward_to_hub) — looks correct.
• generateMigrationSql's new options?.ifNotExists param is backward-compatible; the existing DatatableSchemaDiff callers pass nothing and keep plain CREATE TABLE. Good.
• Test coverage on the generation pipeline (FK closure, ordering, pruning, dangling-FK drop, comment stubs) is solid. Gap: no test exercises the applyOneMigration recorded-vs-preview branch or planMigrations missing/runnable split — those are the install-side paths most likely to regress.

Overall this is well-structured and the commentary is unusually good at recording the why. #1 is the one I'd resolve before shipping (the idempotency story is incomplete for FK-bearing pulled-in parents); #2/#3 are cheap robustness wins. I didn't run the test suite or exercise the publish→install flow (needs full backend + Hub).

_{· di/hub-projects-datatable}

[claude]

Review in progress

• Re-read diff vs origin/tl/workspace-to-hub, focus on changes since last review
• Verify prior findings (FK idempotency, re-import name collision, missing-datatable cancel)
• Review new/changed logic (app data-table detection, policy recompute, run-lock serialization)
• Post review

View job run