Skip to content

feat(cli): bundle pg-delta for database workflows - #6102

Open
avallete wants to merge 19 commits into
developfrom
feat/upgrade-pg-delta-next
Open

feat(cli): bundle pg-delta for database workflows#6102
avallete wants to merge 19 commits into
developfrom
feat/upgrade-pg-delta-next

Conversation

@avallete

@avallete avallete commented Aug 6, 2026

Copy link
Copy Markdown
Member

Runs pg-delta and pg-topo in-process for diff, pull, and declarative schema workflows, enabled by default behind a shared strategy boundary.

The existing edge-runtime implementation remains available through SUPABASE_USE_PG_DELTA_NEXT=false, with no automatic fallback. New-engine snapshots and diagnostics use an isolated v2 artifact format, and migration rendering preserves execution-required transaction boundaries.

Dependencies are temporarily pinned to pg-toolbelt PR #299 at commit 951daa9 and should move to released versions after publication. Generated SQL may differ from the legacy renderer; the compatibility contract is successful execution and state convergence.

The current shared live harness does not provision project Postgres, so linked-project, TLS-required, and pooler/SNI acceptance still requires a provisioned data-plane environment.

@avallete
avallete requested a review from a team as a code owner August 6, 2026 07:15
@avallete avallete added the run-live-e2e-ci Execute the supabox live e2e tests and report back label Aug 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8078b53b04

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

/** Reads the rollout flag once when the command-scoped layer is constructed. */
export const legacyPgDeltaEngineLayer = Layer.unwrap(
Effect.gen(function* () {
const raw = process.env[FLAG];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Resolve the rollout flag from the project environment

When SUPABASE_USE_PG_DELTA_NEXT=false is defined only in supabase/.env, this layer is constructed before the handler loads project environment values and therefore always selects the default next engine. This also disagrees with legacyDbPushCore, which resolves the same flag through toml.envLookup, so one project can warm the legacy catalog during db push but still execute the next engine for db diff, db pull, generate, and sync. Resolve the selector from the project-aware environment so the documented opt-out behaves consistently.

Useful? React with 👍 / 👎.

Comment on lines +421 to +423
const rendered = libraries.renderPlanFiles(generatedPlan, {
allowDrops: input.allowDrops,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Forward format options to the plan renderer

When a project configures [experimental.pgdelta].format_options, the next-engine diff and declarative-plan paths silently discard it: the engine inputs carry formatOptions, but the adapter inputs do not, and both renderer calls receive only allowDrops. Consequently db diff, migration-style db pull, and declarative sync ignore overrides such as lowercase keywords, indentation, or maximum width despite the command documentation promising that behavior; thread the parsed options through these adapter operations and into renderPlanFiles.

AGENTS.md reference: apps/cli/AGENTS.md:L483-L485

Useful? React with 👍 / 👎.

* Management API. The synchronous `docker info` probe is read-only and runs once
* when this helper module is collected.
*/
export const describeDockerLive = describe.skipIf(!hasDockerDaemon());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep Docker live tests gated on the live environment

On any developer or CI host where Docker happens to be available, invoking the test:live target without live credentials now collects and runs this suite instead of leaving it inert, causing it to start a real local Supabase stack and run a scenario with a 15-minute timeout. The repository explicitly uses the configured live environment as the signal that these expensive suites may run, even for Docker-only commands, so this helper should retain that gate rather than probing Docker alone.

AGENTS.md reference: apps/cli/AGENTS.md:L438-L443

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@46a16147588b301b0a3e416bf66dc0ceffb7f9bf

Preview package for commit 46a1614.

@avallete

avallete commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Provisioned validation against exact head 4b697a2e5e0d5090ece6b319e6a1498db91afbcf: internal PG17 run.

  • Project provisioning reached the data plane, and the direct database endpoint connected over TLS with sslmode=require (SUPABASE_LIVE_DB_URL was set).
  • legacy-pgdelta-next.live.test.ts passed both tests: the full convergence scenario (260.2s) and explicit legacy opt-out smoke (25.8s).
  • The aggregate live project finished 19/22 tests. Its three failures were outside this PR's diff: an existing start-status timing assertion, an existing db-pull assertion, and an existing db-diff test colliding on host port 54320 while live files ran concurrently.

The labeled dispatch check itself cannot reach the internal repository because its GitHub App is not installed there, so this run was dispatched manually against the same SHA.

@avallete
avallete marked this pull request as draft August 6, 2026 08:12
@avallete
avallete marked this pull request as ready for review August 7, 2026 17:06
@avallete
avallete marked this pull request as draft August 7, 2026 17:07
Comment thread apps/cli/package.json Outdated
"@parcel/watcher": "^2.6.0",
"@supabase/api": "workspace:*",
"@supabase/config": "workspace:*",
"@supabase/pg-delta": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@ad62ae432865f67bb359a8183a2b3279fa9ebccb",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Severity: HIGH

Both @supabase/pg-delta (v1.0.0-alpha.33) and @supabase/pg-topo (v1.0.0-alpha.5) are sourced from pkg.pr.new, a third-party preview registry (StackBlitz), rather than the official npm registry. These pre-release packages are compiled into the CLI binary via bun build --compile and distributed to end users. They run in-process with direct access to user Postgres connection pools and can extract full database schemas. Shipping a production CLI binary with code from an ephemeral, unofficial PR-preview channel that has not been formally published to npm represents a supply-chain risk.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Replace the pkg.pr.new ephemeral preview-registry URLs for @supabase/pg-delta and @supabase/pg-topo with official npm registry version specifiers once the packages are formally published to npm. As acknowledged in the PR description, these are temporarily pinned to a PR-preview commit (pg-toolbelt PR #299 @ 951daa9). The resolution steps are: (1) Ensure the @supabase/pg-delta and @supabase/pg-topo packages are published to the official npm registry under the @supabase scope. (2) Replace lines 58–59 with proper semver specifiers, e.g. "@supabase/pg-delta": "^1.0.0-alpha.33" and "@supabase/pg-topo": "^1.0.0-alpha.5". (3) Do not ship a production CLI binary that bundles packages sourced from pkg.pr.new or any other ephemeral, unofficial preview channel, as those artifacts are not subject to the same security controls as the official npm registry.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

const exists = yield* fs.exists(w.path).pipe(

P2 Badge Detect collisions by migration version, not full path

When a multi-segment pg-delta plan is generated in the same second as an existing migration with a different name—or overlaps future-dated segments from a prior run—checking only w.path misses the collision because the filenames differ. This writes multiple files with the same 14-digit version; local migration loading accepts both, but schema_migrations.version is a primary key, so a pull can fail while repairing history and later push/reset operations can fail while applying the duplicate version. Check every candidate version against all existing migration filenames rather than only the generated pathname.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +236 to +239
const shadow = yield* shadowService.provision({
schema: input.schema,
...(input.projectRef !== undefined ? { projectRef: input.projectRef } : {}),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid provisioning the unused declarative shadow for database diffs

With the supported [db] major_version = 15 and the default next implementation, every pg-delta db diff and migration-style db pull fails before comparing databases: this call provisions both shadows, the second shadow's setup rejects every major other than 17 in SetupPgDeltaNextDeclarativeShadowDatabase, yet this operation only reads shadow.migrationsUrl and never uses declarativeUrl. Provision only the migrated shadow for diffDatabase (and explicit migrations endpoints), reserving the two-shadow path for declarative planning, so ordinary database diffs continue to work for supported Postgres 15 projects.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

Comment thread apps/cli-go/internal/db/diff/shadow.go Outdated
Comment on lines +78 to +82
migrationsPort, err := allocatePgDeltaNextPort(dependencies.freePort, 0)
if err != nil {
return PgDeltaNextShadow{}, err
}
migrationsContainer, err := dependencies.create(ctx, migrationsPort)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allocate shadow ports on the Docker daemon host

When the CLI runs in a dev container or uses a remote TCP DOCKER_HOST, dependencies.freePort calls GetFreeHostPort, which probes 127.0.0.1 in the CLI process's network namespace, while CreateShadowDatabase publishes the returned port on the Docker daemon's host. A port that is free locally can already be occupied on that host, causing either shadow container creation to fail nondeterministically even though utils.Config.Hostname otherwise supports connecting to that external host. Let Docker allocate each host port and inspect the resulting binding, or otherwise reserve ports in the daemon host's namespace.

Useful? React with 👍 / 👎.

@blacksmith-sh

This comment has been minimized.

@avallete
avallete marked this pull request as ready for review August 7, 2026 18:33
@avallete avallete removed the run-live-e2e-ci Execute the supabox live e2e tests and report back label Aug 7, 2026
Comment thread apps/cli/package.json Outdated
"@supabase/api": "workspace:*",
"@supabase/config": "workspace:*",
"@supabase/pg-delta": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@ad62ae432865f67bb359a8183a2b3279fa9ebccb",
"@supabase/pg-topo": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-topo@ad62ae432865f67bb359a8183a2b3279fa9ebccb",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Severity: HIGH

@supabase/pg-topo is sourced from pkg.pr.new (StackBlitz ephemeral PR-preview registry), not the official npm registry. It runs in-process with direct Postgres pool access and is compiled into the distributed CLI binary, presenting the same unresolved supply-chain risk as @supabase/pg-delta.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Replace the ephemeral pkg.pr.new preview URL for @supabase/pg-topo (and the companion @supabase/pg-delta on line 58) with official npm registry version specifiers once the packages are published. The PR body acknowledges these pins are temporary (pending pg-toolbelt PR #299 publication). Until then, this PR should not be merged into a release branch, as the ephemeral preview tarball is sourced from an untrusted, non-audited registry and is compiled directly into the distributed CLI binary with in-process Postgres pool access. When @supabase/pg-topo is published to npm, replace line 59 with: "@supabase/pg-topo": "^<published-version>". Apply the same change to @supabase/pg-delta on line 58.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4861957ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
}

paths.sort((left, right) => left.name.localeCompare(right.name));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve bytewise declarative file ordering

When declarative filenames differ by case or non-ASCII characters, localeCompare applies locale-sensitive collation—for example, a.sql can sort before Z.sql—rather than the Go implementation's bytewise sort.Strings ordering. Because this ordered array is passed to planSchemaFiles, projects whose DDL files rely on lexical ordering can be loaded differently under the default next engine and fail or produce a different desired schema; use a locale-independent code-point comparison to preserve legacy behavior.

AGENTS.md reference: apps/cli/AGENTS.md:L483-L485

Useful? React with 👍 / 👎.

Comment on lines +337 to +339
yield* rejectBlockingDiagnostic("declarativePlan", result.diagnostics);
return {
...normalizeNextDiff(result, debugDirectory),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject skipped declarative statements

When planSchemaFiles cannot load a declarative statement and reports it through result.skipped—for example, an out-of-scope CREATE ROLE—this return path discards the skipped list and continues with the partial plan. If all requested statements are skipped, db schema declarative sync can even print No schema changes found, falsely implying that the migrations state matches the files; fail the operation or surface each skipped statement before accepting the plan.

Useful? React with 👍 / 👎.

@avallete
avallete marked this pull request as draft August 8, 2026 06:46
@avallete
avallete marked this pull request as ready for review August 8, 2026 10:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

for (const w of set) {
const exists = yield* fs.exists(w.path).pipe(
Effect.mapError(
(cause) =>

P1 Badge Check migration version collisions across all filenames

When a multi-unit plan assigns a version already used by another migration with a different name, this exact-path check reports no collision and writes a second <same-version>_*.sql file. This can occur when a migration was created in the same second as the pull/diff, or when a later plan unit's future-dated timestamp overlaps an existing file; db pull then upserts that version in schema_migrations, replacing the existing history row while both local files remain. Check for any ${version}_*.sql entry before accepting the generated set, rather than only each proposed pathname.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (parsed !== undefined) return parsed;
throw new LegacyPgDeltaEngineError({
message: "failed to parse Postgres connection string for pg-delta",
cause: endpoint.ref.replace(/:[^:@/]+@/, ":***@"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Severity: MEDIUM

The password redaction regex :[^:@/]+@ stops at the first @ character in the password. For passwords containing @ (e.g., user:p@ssword@host), only p is redacted, leaving the remainder (ssword) exposed in the error cause that propagates through logs/error reporting.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Update the password redaction regex to handle passwords containing @ characters by adding an optional repeating group (?:@[^@/]+)* that captures additional @-delimited segments within the password before the final credential-separator @. Change the regex from /:[^:@/]+@/ to /:[^:@/]+(?:@[^@/]+)*@/. This causes the engine to backtrack correctly and match the full password (including embedded @ characters) up to the last @ before the host, rather than stopping at the first @ inside the password.

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change
cause: endpoint.ref.replace(/:[^:@/]+@/, ":***@"),
cause: endpoint.ref.replace(/:[^:@/]+(?:@[^@/]+)*@/, ":***@"),

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ebe58cd26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +343 to +344
const parsed: unknown = JSON.parse(raw);
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the explicit null formatting opt-out

When [experimental.pgdelta].format_options = "null", this branch converts the explicit opt-out to undefined, so legacyPgDeltaNextExportOptions omits format exactly as it does when the setting is absent. The documented contract requires those states to differ—unset uses default formatting while null emits raw SQL—so db pull --declarative and declarative generate cannot honor the opt-out under the default next engine. Preserve null as a distinct value through buildSchemaExport.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

pending.push(full);
continue;
}
if (path.extname(entry).toLowerCase() !== ".sql") continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep uppercase SQL files out of declarative input

When a declarative directory contains a file such as tables.SQL, lowercasing the extension causes the next engine to load and apply it, whereas the Go reference's hashDeclarativeSchemas and Glob.SQLFiles accept only an exact .sql extension. A project can therefore generate migrations from statements the stable Go workflow ignored; compare the extension without lowercasing to preserve the legacy command contract.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

| Old command | TS status | TS command path or `missing` | Missing flags/params | Extra TS flags/params | Notes |
| --------------------------------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -------------------- | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `db diff` | `ported` | `legacy/commands/db/diff/` | `n/a` | `n/a` | Native TS port. Native pg-delta / migra diff via edge-runtime against a Go-seam-provisioned live shadow (`db __shadow`); `--use-pgadmin` / `--use-pg-schema` delegate to the Go binary. `--use-pg-schema` is deprecated (CLI-1960: TS-only stderr warning + `--help` note) in favor of the pg-delta engine or the default migra engine — it wraps the in-process `stripe/pg-schema-diff` Go library, which has no TS/container equivalent, so it is a documented keep-in-Go exception, not a pending port. It will be the sole remaining Go delegation once `--use-pgadmin`, the `db __shadow`/`db __db-bootstrap` seams, and the other in-flight M9 issues are done. |
| `db diff` | `ported` | `legacy/commands/db/diff/` | `n/a` | `n/a` | Native TS port. Pg-delta runs in-process by default with bundled pg-topo against isolated Go-seam-provisioned shadows (`db __shadow`); `SUPABASE_USE_PG_DELTA_NEXT=false` retains the legacy edge-runtime implementation. Migra remains edge-runtime-backed; `--use-pgadmin` / `--use-pg-schema` delegate to the Go binary. `--use-pg-schema` is deprecated (CLI-1960: TS-only stderr warning + `--help` note) in favor of the pg-delta engine or the default migra engine — it wraps the in-process `stripe/pg-schema-diff` Go library, which has no TS/container equivalent, so it is a documented keep-in-Go exception, not a pending port. It will be the sole remaining Go delegation once `--use-pgadmin`, the `db __shadow` seam, and the other in-flight M9 issues are done. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Record the new db diff flag in the parity tracker

This change adds the TS-only --strict-coverage flag to db diff, but the updated tracker row still reports n/a under Extra TS flags/params. That leaves the command-surface parity record inaccurate and violates the workspace requirement to record added flags on already-ported commands; list --strict-coverage here (and audit the analogous changed leaves).

AGENTS.md reference: apps/cli/AGENTS.md:L453-L457

Useful? React with 👍 / 👎.

Comment on lines +195 to +197
for _, line := range m.Statements {
batch.ExecParams(line, nil, nil, nil, nil)
batchSize++

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep pipeline-incompatible SQL out of authored batches

When a migration contains an authored transaction followed by a standalone-only statement—for example BEGIN; …; COMMIT; CREATE INDEX CONCURRENTLY …—this branch queues every statement into pgconn.ExecBatch, bypassing the isPipelineIncompatible handling below. CREATE INDEX CONCURRENTLY, VACUUM, and the other recognized statements cannot execute through that pipeline, so otherwise valid migrations fail during Go application and while provisioning the default pg-delta migration shadow; preserve authored transaction boundaries while still flushing these statements through the standalone execution path.

Useful? React with 👍 / 👎.

Comment on lines +161 to +164
if transactional {
if _, err := conn.Exec(ctx, "BEGIN"); err != nil {
return errors.Errorf("failed to begin migration transaction: %w", err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep database recreation statements outside transactions

The new unconditional BEGIN around ordinary batches breaks internal/db/reset.recreateDatabase, which deliberately sends DROP DATABASE and CREATE DATABASE statements through MigrationFile.ExecBatch. PostgreSQL rejects those commands inside a transaction block, so the Go PG14 reset path now fails before recreating postgres; treat database-level commands as standalone or preserve the prior non-transactional execution contract for this internal batch.

Useful? React with 👍 / 👎.

),
);
yield* legacyStartInitSchema(spawner, input, tmpDir);
yield* legacyApplyDatabaseWebhooks(input, tmpDir);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reinstall configured webhooks during PG14 resets

For a supported Postgres 14 project with [experimental.webhooks] enabled = true, fresh setup installs pg_net here, but the native TS PG14 reset path still runs only legacyInitSchema14 followed by legacyApplyApiPrivileges. Because this commit also removes pg_net from the PG14 initial-schema template, db reset drops the database and never restores the configured extension, unlike the updated Go initDatabase, which now calls ApplyDatabaseWebhooks; expose and reuse this step from the reset composition.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f6fb326fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +85 to +88
yield* fs.writeFileString(
path.join(declarativeDir, EXPORT_MANIFEST_FILE),
`${JSON.stringify(serialized, null, 2)}\n`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject manifest-only declarative exports

When the next exporter returns zero SQL files—for example, after generating from an empty custom database—this still writes .pgdelta-export.json, so declarativeDirHasFiles and the post-generation check treat the export as nonempty. LegacyLoadPgDeltaSqlFiles then supplies an empty desired schema to the planner; against a nonempty migrations baseline, sync can generate drop migrations and apply them under --yes instead of reporting that generation produced no files. Only write the manifest after at least one SQL file exists, or make the presence checks count SQL files rather than arbitrary directory entries.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f9bd2890b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +107 to +109
if (stat.type === "Directory") {
pending.push(full);
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip symlinked declarative directories

When the declarative directory contains a symlink to another directory, FileSystem.stat follows the link and this branch traverses its target as though it were inside the configured schema root. A self-referential link can repeatedly enqueue deeper paths until the operation fails or hangs, while a link to an external directory causes SQL outside the declarative tree to be planned; the Go reference's afero.Walk does not follow directory symlinks. Use link-aware metadata to skip symlinks, or enforce resolved-path containment with a visited set.

AGENTS.md reference: apps/cli/AGENTS.md:L249-L257

Useful? React with 👍 / 👎.

Comment thread apps/cli/package.json
"@parcel/watcher": "^2.6.0",
"@supabase/api": "workspace:*",
"@supabase/config": "workspace:*",
"@supabase/pg-delta": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@2f1d6b677bb44485f0a6874caf288f2c77896f86",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Severity: HIGH

These two packages are resolved from pkg.pr.new, a third-party CDN for preview/CI releases that does not publish to npm and does not carry provenance attestations. There is no verifiable cryptographic link between the URL and the source commit, so a compromise of pkg.pr.new's infrastructure, storage, or CDN layer could silently substitute malicious tarballs. Because these packages are bundled directly into the CLI binary at build time, every CLI user would receive any injected code. The PR itself acknowledges this is temporary (should move to released versions after publication).
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Do not merge this PR until @supabase/pg-delta and @supabase/pg-topo are officially published to the npm registry. Once published, replace both pkg.pr.new tarball URLs with their proper versioned npm references, e.g.:

"@supabase/pg-delta": "^",
"@supabase/pg-topo": "^",

Publishing to npm with provenance attestations (via npm publish --provenance) will create a verifiable cryptographic link between the npm package and the originating GitHub Actions run and source commit, eliminating the implicit trust in the pkg.pr.new CDN infrastructure. In the meantime, if an urgent unblock is needed, using a direct github:supabase/pg-toolbelt#<sha> reference is marginally better (it pins to a specific commit in the known-good GitHub repo), but the preferred and safe path is to wait for the official npm release.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46a1614758

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const redactSecrets = readManifestValue(decoded, "redactSecrets");
const scope = readManifestValue(decoded, "scope");
if (
(formatVersion !== undefined && formatVersion !== 1) ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require a version on export manifests

When .pgdelta-export.json contains redactSecrets and scope but omits formatVersion—for example, a stale or partially hand-authored manifest—this condition accepts it as a valid next-engine export. legacyDiffDeclarativeToMigrations then sets manifestPresent: true, suppressing the legacy compatibility guard, so sync can write or apply extension-removal migrations without the repair prompt. Require formatVersion === 1 before treating the file as a manifest.

Useful? React with 👍 / 👎.

Comment on lines +132 to 135
const declarativeDirRel = Option.getOrElse(flags.output, () =>
legacyResolveDeclarativeDir(path, toml.pgDelta),
);
const declarativeDir = path.resolve(cliConfig.workdir, declarativeDirRel);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject output paths that resolve to the workdir

When generate is invoked with an empty or current-directory output such as --output "" --overwrite or --output . --overwrite, this resolves declarativeDir to the project workdir. The subsequent legacyWriteDeclarativeSchemas call recursively removes that directory before writing the export, deleting the entire project. Reject empty paths and paths that resolve to the workdir or another unsafe root before reaching the destructive writer.

Useful? React with 👍 / 👎.

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.

1 participant