From ecb437e48d958372dc6fd45f6a2eb818bfffde5a Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Tue, 13 Oct 2026 12:25:17 +0100 Subject: [PATCH 1/3] feat(cli)!: remove deprecated db pull and domains flags (CLI-2596) Three deprecated surfaces now fail with exit 1 and a message naming the replacement. Each is a hidden, still-parsed tombstone that raises RemovedSurfaceError (error_fingerprint ending in :removed_flag). --use-pg-delta and --include-raw-output fail on a credential-free runtime before any work; the db pull --experimental rejection fails after the profile and project .env load, before any config, database or network work. - db pull --use-pg-delta (any value): use --declarative. The diff engine is still chosen with --diff-engine or [experimental.pgdelta] enabled. - db pull in the --experimental structured-dump mode (--experimental or SUPABASE_EXPERIMENTAL resolving true without --declarative): use --declarative, or --experimental=false for a migration pull. --experimental --declarative is unchanged. The check lives in the db pull handler, so the supabase pull orchestrator keeps working when SUPABASE_EXPERIMENTAL is set. - domains create|get|reverify|activate|delete --include-raw-output (any value): use -o json. Rejected on a credential-free runtime, so it needs no login. Removes the --declarative/--use-pg-delta argv replay, the deprecation lines, DbPullInvoke.forceMigrationMode, the structured-dump branch and the emitHostnameResult includeRawOutput forcing. The supabase pull retry hint keeps its --experimental=false suffix. BREAKING CHANGE: supabase db pull --use-pg-delta is removed; use --declarative. supabase db pull with --experimental or SUPABASE_EXPERIMENTAL and no --declarative is removed; use --declarative, or pass --experimental=false for a migration pull. supabase domains create|get|reverify|activate|delete --include-raw-output is removed; use -o json. Each removed surface exits 1 and prints its replacement. --- .../db-password-rejection.integration.test.ts | 3 +- apps/cli/src/command-internal/db-pull-run.ts | 132 ++------ apps/cli/src/command-internal/diff-engine.ts | 37 --- .../command-internal/diff-engine.unit.test.ts | 53 +-- apps/cli/src/commands/db/pull/SIDE_EFFECTS.md | 96 +++--- apps/cli/src/commands/db/pull/pull.command.ts | 72 ++-- apps/cli/src/commands/db/pull/pull.errors.ts | 4 +- apps/cli/src/commands/db/pull/pull.handler.ts | 29 +- .../commands/db/pull/pull.integration.test.ts | 309 +++++------------- .../pull.removed-flag.integration.test.ts | 241 ++++++++++++++ .../seed-flag-telemetry.integration.test.ts | 2 +- apps/cli/src/commands/domains/SIDE_EFFECTS.md | 27 +- .../domains/activate/activate.command.ts | 21 +- .../domains/activate/activate.handler.ts | 2 +- .../activate/activate.integration.test.ts | 10 +- .../commands/domains/create/create.command.ts | 21 +- .../commands/domains/create/create.handler.ts | 2 +- .../domains/create/create.integration.test.ts | 12 +- .../commands/domains/delete/delete.command.ts | 21 +- .../commands/domains/delete/delete.handler.ts | 3 - .../domains/delete/delete.integration.test.ts | 11 +- apps/cli/src/commands/domains/domains.emit.ts | 18 +- .../domains.removed-flag.integration.test.ts | 138 ++++++++ .../commands/domains/domains.removed-flag.ts | 22 ++ .../src/commands/domains/get/get.command.ts | 21 +- .../src/commands/domains/get/get.handler.ts | 2 +- .../domains/get/get.integration.test.ts | 19 +- .../domains/reverify/reverify.command.ts | 21 +- .../domains/reverify/reverify.handler.ts | 2 +- .../reverify/reverify.integration.test.ts | 10 +- apps/cli/src/commands/pull/SIDE_EFFECTS.md | 13 +- apps/cli/src/commands/pull/pull.aggregate.ts | 5 +- .../commands/pull/pull.aggregate.unit.test.ts | 2 +- .../commands/pull/pull.integration.test.ts | 30 +- apps/cli/src/commands/pull/pull.steps.ts | 12 +- apps/cli/src/docs/docs-spec.tables.ts | 14 +- apps/cli/src/docs/docs-spec.unit.test.ts | 2 +- 37 files changed, 791 insertions(+), 648 deletions(-) create mode 100644 apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts create mode 100644 apps/cli/src/commands/domains/domains.removed-flag.integration.test.ts create mode 100644 apps/cli/src/commands/domains/domains.removed-flag.ts diff --git a/apps/cli/src/command-internal/db-password-rejection.integration.test.ts b/apps/cli/src/command-internal/db-password-rejection.integration.test.ts index 4aac1598e0..bda9747d08 100644 --- a/apps/cli/src/command-internal/db-password-rejection.integration.test.ts +++ b/apps/cli/src/command-internal/db-password-rejection.integration.test.ts @@ -66,7 +66,8 @@ const runCommand = (entry: WalkedCommand, selector: "db-url" | "local" | "none") ...selectorArgs(selector), "--workdir", home, - "--experimental", + // `db pull` rejects `--experimental` without `--declarative` before it validates the password. + ...(entry.path.join(" ") === "db pull" ? [] : ["--experimental"]), ]; const exit = yield* Command.runWith(root, { version: "0.0.0-test" })(args).pipe( Effect.provide( diff --git a/apps/cli/src/command-internal/db-pull-run.ts b/apps/cli/src/command-internal/db-pull-run.ts index 81aa859af7..8d6d59863e 100644 --- a/apps/cli/src/command-internal/db-pull-run.ts +++ b/apps/cli/src/command-internal/db-pull-run.ts @@ -5,10 +5,8 @@ import { DebugFlag, DnsResolverFlag, NetworkIdFlag, - resolveExperimentalWithProjectEnv, resolveYesWithProjectEnv, } from "./global-flags.ts"; -import { CliArgs } from "../shared/cli/cli-args.service.ts"; import { Output } from "../shared/output/output.service.ts"; import { RuntimeInfo } from "../shared/runtime/runtime-info.service.ts"; import { CommandSettings } from "../config/command-settings.service.ts"; @@ -25,10 +23,7 @@ import { loadProjectEnvValues, readDbToml, resolveDeclarativeDir } from "./db-co import type { DbConnType } from "./db-target-flags.ts"; import { makeDir } from "./make-dir.ts"; import { toPostgresURL } from "./postgres-url.ts"; -import { - buildLocalDbContainerInputs, - type LocalDbContainerInputs, -} from "./db-bootstrap/local-container-inputs.ts"; +import { buildLocalDbContainerInputs } from "./db-bootstrap/local-container-inputs.ts"; import { withShadowDatabase } from "./db-bootstrap/shadow-cache.ts"; import { shadowRunInputFromLocalContainerInputs } from "./db-bootstrap/shadow-database.ts"; import { LinkedProjectCache } from "../telemetry/linked-project-cache.service.ts"; @@ -38,11 +33,7 @@ import { warnPreservedUnmanagedDeclarativeFiles, writeDeclarativeSchemas, } from "../commands/db/shared/pgdelta.write.ts"; -import { - resolveDeclarativeFromArgs, - resolvePullDiffEngine, - schemaPathsTransitionWarning, -} from "./diff-engine.ts"; +import { resolvePullDiffEngine, schemaPathsTransitionWarning } from "./diff-engine.ts"; import { diffMigra } from "../commands/db/shared/migra.ts"; import { writePgDeltaMigrations } from "../commands/db/shared/pgdelta-migrations.write.ts"; import { type DumpOptions, buildSchemaDumpEnv } from "./pg-dump.env.ts"; @@ -68,7 +59,7 @@ import { prepareShadowSource } from "../commands/db/shared/shadow-source.ts"; import { currentStackBackend } from "./stack-backend.ts"; import { stackRejectNativeDockerDiffEngine } from "./stack-local-database.ts"; import { stackPrepareShadowSource, stackWithShadowDatabase } from "./stack-shadow.ts"; -import type { DbPullFlags } from "../commands/db/pull/pull.command.ts"; +import type { DbPullFlags as DbPullCommandFlags } from "../commands/db/pull/pull.command.ts"; import { DbPullDumpError, DbPullEngineConflictError, @@ -83,13 +74,12 @@ import { } from "./migration-history.ts"; import { updateMigrationHistory } from "../commands/db/pull/pull.sync.ts"; -// `DbPullFlags` is `runDbPull`'s own parameter type, defined alongside the standalone -// `db pull` command that also constructs it — re-exported here so an in-process caller -// (`pull`) can build one without reaching into `commands/db/**` directly. -export type { DbPullFlags }; - -// Established output contract; ends with a `.`. -const DEPRECATION_LINE = "Flag --use-pg-delta has been deprecated, use --declarative instead."; +/** + * `runDbPull`'s parameter type: the `db pull` command flags without the tombstoned + * `--use-pg-delta`, so an in-process caller (`pull`) can build one without reaching into + * `commands/db/**` directly. + */ +export type DbPullRunFlags = Omit; /** * Explains the in-sync non-zero exit instead of the generic "Try rerunning the command with @@ -101,12 +91,6 @@ const IN_SYNC_SUGGESTION = /** Migration-file mode for the initial pg_dump seed. */ const MIGRATION_FILE_MODE = 0o644; -// `--experimental` without `--declarative` is a deprecated path: the in-process declarative -// export covers the same per-object-files outcome. Printed only when the experimental gate -// selected this branch, not when `--declarative` already did. -const EXPERIMENTAL_STRUCTURED_DUMP_DEPRECATION_LINE = - "The --experimental structured-dump mode for `db pull` is deprecated and will be removed in a future release. Use --declarative instead to pull the remote schema as per-object files."; - export type DbPullInvoke = { /** * Overrides `--yes`/`SUPABASE_YES`/`supabase/.env` resolution for an @@ -114,15 +98,6 @@ export type DbPullInvoke = { * `undefined` keeps the existing resolution. */ readonly assumeYes?: boolean; - /** - * Forces migration mode regardless of the ambient `--experimental`/`SUPABASE_EXPERIMENTAL` - * gate, for an in-process caller (`pull`) whose dirty-guard, confirmation message, and - * SIDE_EFFECTS.md all assume this step only ever writes into `supabase/migrations`. Without - * this override, an ambient experimental flag would silently switch to the declarative export - * path, writing `supabase/schemas/**` and potentially `config.toml`'s `schema_paths` - * unguarded. `undefined` keeps the existing resolution. - */ - readonly forceMigrationMode?: boolean; }; export type DbPullOutcome = @@ -145,7 +120,7 @@ export type DbPullOutcome = * owns that emission based on the returned {@link DbPullOutcome}. */ export const runDbPull = Effect.fn("db.pull.run")(function* ( - flags: DbPullFlags, + flags: DbPullRunFlags, invoke?: DbPullInvoke, ) { const output = yield* Output; @@ -159,7 +134,6 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( const path = yield* Path.Path; const dnsResolver = yield* DnsResolverFlag; const debug = yield* DebugFlag; - const cliArgs = yield* CliArgs; const configValues = yield* CliConfigValues; // `--yes` or `SUPABASE_YES`. The project `.env` is loaded before the migration @@ -167,42 +141,15 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( // the native initial-migra history repair too. const projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); const yes = yield* resolveYesWithProjectEnv(projectEnv); - // `EXPERIMENTAL` resolves from either the global `--experimental` flag or - // `SUPABASE_EXPERIMENTAL`, reusing `resolveExperimentalWithProjectEnv`'s flag-over-env - // precedence instead of re-deriving it. Resolved once, same as `yes` above; declarative mode - // ignores it. `invoke?.forceMigrationMode` overrides this for an in-process caller that must - // never take the declarative export path — see {@link DbPullInvoke.forceMigrationMode}. - const experimental = invoke?.forceMigrationMode - ? false - : yield* resolveExperimentalWithProjectEnv(projectEnv); let linkedRefForCache: string | undefined; return yield* Effect.gen(function* () { const name = Option.getOrElse(flags.name, () => "remote_schema"); - // `--declarative` and the deprecated `--use-pg-delta` both bind to the same - // `useDeclarative` outcome. When both are passed, the last occurrence in argv wins - // (e.g. `--declarative --use-pg-delta=false` => migration mode); since parsed Options don't - // carry order, the both-present case replays that rule off the raw argv instead of ORing the - // two, which would diverge on conflicting values. - const useDeclarative = - Option.isSome(flags.declarative) && Option.isSome(flags.usePgDelta) - ? (resolveDeclarativeFromArgs(cliArgs.args) ?? false) - : Option.getOrElse(flags.declarative, () => false) || - Option.getOrElse(flags.usePgDelta, () => false); - if (Option.isSome(flags.usePgDelta)) { - yield* output.raw(`${DEPRECATION_LINE}\n`, "stderr"); - } - // Deprecated `--experimental` dump: same in-process export as `--declarative`. - const useExperimentalExport = experimental && !useDeclarative; - if (useExperimentalExport) { - yield* output.raw(`${EXPERIMENTAL_STRUCTURED_DUMP_DEPRECATION_LINE}\n`, "stderr"); - } - const useDeclarativeExport = useDeclarative || useExperimentalExport; + const useDeclarative = Option.getOrElse(flags.declarative, () => false); - // Mutually exclusive flag groups: `[db-url linked local]`, `[declarative - // diff-engine]`, `[use-pg-delta diff-engine]`. "set" means the flag was - // explicitly passed. + // Mutually exclusive flag groups: `[db-url linked local]`, `[declarative diff-engine]`. + // "set" means the flag was explicitly passed. const targetSet: Array = []; if (Option.isSome(flags.dbUrl)) targetSet.push("db-url"); if (Option.isSome(flags.linked)) targetSet.push("linked"); @@ -212,15 +159,11 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( message: `if any flags in the group [db-url linked local] are set none of the others can be; [${[...targetSet].sort().join(" ")}] were all set`, }); } - for (const [flagName, present] of [ - ["declarative", Option.isSome(flags.declarative)], - ["use-pg-delta", Option.isSome(flags.usePgDelta)], - ] as const) { - if (present && Option.isSome(flags.diffEngine)) { - return yield* new DbPullEngineConflictError({ - message: `if any flags in the group [${flagName} diff-engine] are set none of the others can be; [${[flagName, "diff-engine"].sort().join(" ")}] were all set`, - }); - } + if (Option.isSome(flags.declarative) && Option.isSome(flags.diffEngine)) { + return yield* new DbPullEngineConflictError({ + message: + "if any flags in the group [declarative diff-engine] are set none of the others can be; [declarative diff-engine] were all set", + }); } const connType: DbConnType = Option.isSome(flags.dbUrl) @@ -230,7 +173,7 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( : "linked"; yield* Effect.annotateCurrentSpan({ "db.conn_type": connType, - "db.pull.mode": useDeclarativeExport ? "declarative" : "migration", + "db.pull.mode": useDeclarative ? "declarative" : "migration", }); // `--project-ref` never implies `--linked` and must not be silently @@ -270,20 +213,16 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( const runtimeInfo = yield* RuntimeInfo; const networkIdFlag = yield* NetworkIdFlag; // Validate native shadow inputs before target resolution performs remote side effects. - const localInputs: Option.Option = useExperimentalExport - ? Option.none() - : Option.some( - yield* buildLocalDbContainerInputs( - spawner, - cliSettings.workdir, - networkIdFlag, - runtimeInfo.platform, - debug, - // So the shadow's own container spec reflects the matching `[remotes.]` - // override, same as `toml` above. - connType === "linked" ? linkedRef : undefined, - ), - ); + const localInputs = yield* buildLocalDbContainerInputs( + spawner, + cliSettings.workdir, + networkIdFlag, + runtimeInfo.platform, + debug, + // So the shadow's own container spec reflects the matching `[remotes.]` + // override, same as `toml` above. + connType === "linked" ? linkedRef : undefined, + ); const resolved = yield* resolver.resolve({ dbUrl: flags.dbUrl, @@ -379,8 +318,7 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( dnsResolver, }); - // Declarative export path (`--declarative` or deprecated `--experimental`). - if (useDeclarativeExport) { + if (useDeclarative) { yield* output.raw("Preparing declarative schema export using pg-delta...\n", "stderr"); const declarativeDirRel = resolveDeclarativeDir(path, toml.pgDelta); const declarativeDir = path.resolve(cliSettings.workdir, declarativeDirRel); @@ -460,10 +398,6 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( // is "in sync", a non-empty dump is a valid initial migration on its own. let seedWroteBytes = false; - // `Option.getOrThrow` is safe here: this point is only reached after the - // declarative-export branch already returned, so `localInputs` was always built. - const pullLocalInputs = Option.getOrThrow(localInputs); - if (seededFromDump) { yield* makeDir(fs, path.dirname(migrationPath)).pipe( Effect.mapError((cause) => new DbPullWriteError({ message: cause.message })), @@ -593,13 +527,13 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( const stackBackend = (yield* currentStackBackend).kind === "stack"; const resolvedPullShadowImage = stackBackend ? "stack-ephemeral" - : yield* pullLocalInputs.resolvePostgresImage; + : yield* localInputs.resolvePostgresImage; const migrationMode: "legacy" | "pgdelta-next" = usePgDeltaDiff ? "pgdelta-next" : "legacy"; const shadowInput = { ...shadowRunInputFromLocalContainerInputs( - pullLocalInputs, + localInputs, resolvedPullShadowImage, toml, fs, @@ -608,7 +542,7 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( targetLocal: resolved.isLocal, migrationMode, // `toml.schemaPathPatterns` is used here, not the raw `@supabase/config` field on - // `pullLocalInputs`, since only `toml` (via `readDbToml`) resolves the + // `localInputs`, since only `toml` (via `readDbToml`) resolves the // `SUPABASE_DB_MIGRATIONS_SCHEMA_PATHS` env override. schemaPaths: toml.schemaPathPatterns, pgDelta: toml.pgDelta, diff --git a/apps/cli/src/command-internal/diff-engine.ts b/apps/cli/src/command-internal/diff-engine.ts index 8246995d8e..b6ff5b8430 100644 --- a/apps/cli/src/command-internal/diff-engine.ts +++ b/apps/cli/src/command-internal/diff-engine.ts @@ -34,40 +34,3 @@ export function resolvePullDiffEngine(inputs: { } return inputs.pgDeltaDefault; } - -/** - * Parses a boolean env var: accepts exactly `1`, `t`, `T`, `TRUE`, `true`, `True` as true, and - * every other value (including unparseable strings and unset) as false. - */ -export function parseBoolEnv(raw: string | undefined): boolean { - switch (raw) { - case "1": - case "t": - case "T": - case "TRUE": - case "true": - case "True": - return true; - default: - return false; - } -} - -/** - * Resolves `db pull` declarative mode from raw argv: `--declarative` and the deprecated - * `--use-pg-delta` alias bind to the same outcome, so the last occurrence in argv wins (ORing - * the two would wrongly select declarative mode from either flag alone). A bare flag is `true`; - * `--flag=value` parses with {@link parseBoolEnv}'s true-set — a following space-separated token - * is never consumed as a value. Returns `undefined` when neither flag is present. - */ -export function resolveDeclarativeFromArgs(args: ReadonlyArray): boolean | undefined { - const FLAG_PATTERN = /^--(?:declarative|use-pg-delta)(?:=(.*))?$/u; - let result: boolean | undefined; - for (const arg of args) { - if (arg === "--") break; - const match = FLAG_PATTERN.exec(arg); - if (match === null) continue; - result = match[1] === undefined ? true : parseBoolEnv(match[1]); - } - return result; -} diff --git a/apps/cli/src/command-internal/diff-engine.unit.test.ts b/apps/cli/src/command-internal/diff-engine.unit.test.ts index de7c1e36a5..6286e19af8 100644 --- a/apps/cli/src/command-internal/diff-engine.unit.test.ts +++ b/apps/cli/src/command-internal/diff-engine.unit.test.ts @@ -1,11 +1,6 @@ import { describe, expect, it } from "vitest"; -import { - parseBoolEnv, - resolveDeclarativeFromArgs, - resolveDiffEngine, - resolvePullDiffEngine, -} from "./diff-engine.ts"; +import { resolveDiffEngine, resolvePullDiffEngine } from "./diff-engine.ts"; describe("resolveDiffEngine", () => { const base = { @@ -63,49 +58,3 @@ describe("resolvePullDiffEngine", () => { ).toBe(false); }); }); - -describe("parseBoolEnv", () => { - it("accepts only the boolean truthy strings", () => { - for (const v of ["1", "t", "T", "TRUE", "true", "True"]) { - expect(parseBoolEnv(v)).toBe(true); - } - }); - - it("treats every other value (including unset) as false", () => { - for (const v of ["0", "f", "FALSE", "false", "yes", "on", "2", "", "TrUe"]) { - expect(parseBoolEnv(v)).toBe(false); - } - expect(parseBoolEnv(undefined)).toBe(false); - }); -}); - -describe("resolveDeclarativeFromArgs", () => { - it("returns undefined when neither flag is present", () => { - expect(resolveDeclarativeFromArgs(["db", "pull"])).toBeUndefined(); - expect(resolveDeclarativeFromArgs([])).toBeUndefined(); - }); - - it("treats a bare flag as true", () => { - expect(resolveDeclarativeFromArgs(["db", "pull", "--declarative"])).toBe(true); - expect(resolveDeclarativeFromArgs(["db", "pull", "--use-pg-delta"])).toBe(true); - }); - - it("parses an =value with boolean semantics", () => { - expect(resolveDeclarativeFromArgs(["--declarative=false"])).toBe(false); - expect(resolveDeclarativeFromArgs(["--declarative=true"])).toBe(true); - expect(resolveDeclarativeFromArgs(["--use-pg-delta=0"])).toBe(false); - expect(resolveDeclarativeFromArgs(["--use-pg-delta=1"])).toBe(true); - }); - - it("lets the last occurrence win across both flag names", () => { - expect(resolveDeclarativeFromArgs(["--declarative", "--use-pg-delta=false"])).toBe(false); - expect(resolveDeclarativeFromArgs(["--use-pg-delta", "--declarative=false"])).toBe(false); - expect(resolveDeclarativeFromArgs(["--declarative=false", "--use-pg-delta"])).toBe(true); - expect(resolveDeclarativeFromArgs(["--use-pg-delta=false", "--declarative"])).toBe(true); - }); - - it("ignores tokens after the `--` argv terminator", () => { - expect(resolveDeclarativeFromArgs(["--declarative", "--", "--use-pg-delta=false"])).toBe(true); - expect(resolveDeclarativeFromArgs(["--", "--declarative"])).toBeUndefined(); - }); -}); diff --git a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md index ef0ab246d5..7e230d60bb 100644 --- a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md @@ -2,15 +2,13 @@ Pulls the remote schema into either a new timestamped migration (diffing a throwaway shadow against the remote, bundled pg-delta or -migra) or declarative files (`--declarative`, or the deprecated `--experimental` -gate without `--declarative`). Both export modes run the native pg-delta -export. The initial-migra pull (no local migrations) seeds the migration file +migra) or declarative files (`--declarative`). The declarative mode runs the native +pg-delta export. The initial-migra pull (no local migrations) seeds the migration file with a native `pg_dump` of the remote schema (a Docker `pg_dump` container, with IPv4 transaction-pooler fallback) and then appends the migra diff. -`--experimental` without `--declarative` runs the same in-process declarative export (`supabase/schemas` plus -`.pgdelta-export.json`) and prints a deprecation line pointing at -`--declarative`. `--experimental --declarative` does not print that line: -`--declarative` already selected the export. +`--use-pg-delta` and the `--experimental` structured-dump mode (`--experimental` or +`SUPABASE_EXPERIMENTAL` resolving true without `--declarative`) are removed — see +"Removed surfaces" below. When `[experimental].stack` is on, each shadow is a fresh database in an invocation-owned, unique temporary stack namespace. The command applies the catalog and project migrations as needed, @@ -37,7 +35,7 @@ disables formatting without disabling safe compaction. | Path | Format | When | | ----------------------------------------------------------------------------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `/supabase/config.json` or `config.toml` | JSON / TOML | always, `config.json` preferred (db port/password, `[experimental.pgdelta]`) | -| `/supabase/.env`, `.env.local`, project-root/`SUPABASE_ENV`-selected dotenv file | dotenv | migration-style pull's shadow provisioning, and declarative / deprecated-`--experimental` export config/env resolution | +| `/supabase/.env`, `.env.local`, project-root/`SUPABASE_ENV`-selected dotenv file | dotenv | migration-style pull's shadow provisioning, and declarative export config/env resolution | | `api.tls.cert_path` / `api.tls.key_path` (under `/supabase/`) | PEM | shadow provisioning, when `api.enabled && api.tls.enabled` | | `/supabase/migrations/*.sql` | SQL | history reconciliation + shadow provisioning | | `/supabase/roles.sql` | SQL | migration-style pull only (`--declarative` provisions no shadow); also hashed into the shadow-baseline cache key on every cache-eligible acquire, warm hits included (where no baseline is applied at all); missing file tolerated | @@ -49,18 +47,18 @@ disables formatting without disabling safe compaction. ## Files Written -| Path | Format | When | -| --------------------------------------------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `/supabase/migrations/_.sql` | SQL | migration-style pull (non-empty diff, or the initial-migra `pg_dump` seed) | -| `/supabase/schemas/**` | SQL | `--declarative` or deprecated `--experimental` export | -| `/supabase/schemas/.pgdelta-export.json` | JSON | bundled declarative / deprecated-`--experimental` export metadata | -| `/supabase/config.toml` | TOML | declarative export updates `[db.migrations].schema_paths` only when `[experimental.pgdelta] enabled = false`; skipped when `enabled` resolves true (section absent, `enabled` omitted, or `enabled = true` — the default) | -| `/supabase/.temp/pgdelta/v2/debug//*.json` | JSON | bundled engine with `PGDELTA_DEBUG` | -| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | cache-enabled COLD shadow provision creates the current key's snapshot, migration-style pull only (never a declarative / deprecated-`--experimental` export, which provisions no shadow); a warm hit `touch`es its mtime (LRU); every cache-eligible acquire may delete other keys under LRU keep-3 + 2-day mtime TTL — ~90MB (`SUPABASE_HOME` overrides the root) | -| managed snapshot key `` | backend | stack-mode cache-enabled cold shadow provision saves the database baseline; the last completed save replaces the key | -| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar..partial` | tar | during a cold export — the in-flight temp file, `rename`d into the tar above on success and removed on failure; only a crash/SIGKILL leaves it behind, and later cold exports / warm hits sweep leftovers older than 5 minutes | -| `~/.supabase//linked-project.json` | JSON | linked (post-run cache) | -| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | +| Path | Format | When | +| --------------------------------------------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `/supabase/migrations/_.sql` | SQL | migration-style pull (non-empty diff, or the initial-migra `pg_dump` seed) | +| `/supabase/schemas/**` | SQL | `--declarative` export | +| `/supabase/schemas/.pgdelta-export.json` | JSON | bundled declarative export metadata | +| `/supabase/config.toml` | TOML | declarative export updates `[db.migrations].schema_paths` only when `[experimental.pgdelta] enabled = false`; skipped when `enabled` resolves true (section absent, `enabled` omitted, or `enabled = true` — the default) | +| `/supabase/.temp/pgdelta/v2/debug//*.json` | JSON | bundled engine with `PGDELTA_DEBUG` | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | cache-enabled COLD shadow provision creates the current key's snapshot, migration-style pull only (never a declarative export, which provisions no shadow); a warm hit `touch`es its mtime (LRU); every cache-eligible acquire may delete other keys under LRU keep-3 + 2-day mtime TTL — ~90MB (`SUPABASE_HOME` overrides the root) | +| managed snapshot key `` | backend | stack-mode cache-enabled cold shadow provision saves the database baseline; the last completed save replaces the key | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar..partial` | tar | during a cold export — the in-flight temp file, `rename`d into the tar above on success and removed on failure; only a crash/SIGKILL leaves it behind, and later cold exports / warm hits sweep leftovers older than 5 minutes | +| `~/.supabase//linked-project.json` | JSON | linked (post-run cache) | +| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | ## Docker @@ -135,15 +133,16 @@ at all, so nothing is cached for it. | `SUPABASE_HOME` | overrides the `~/.supabase` root used for the shadow baseline cache (and other CLI state) | no | | `SUPABASE_SHADOW_CACHE` | shadow baseline cache; on by default, opt-out (`0`/`false`); the shadow's post-baseline state is saved under a managed snapshot key and restored into the next run's fresh stack database (see Notes) | no | | `SUPABASE_EXPERIMENTAL_PGDELTA_ENABLED` | overrides `[experimental.pgdelta].enabled` (default true; `false` selects migra for the migration-style diff unless `--diff-engine` is passed, and lets a declarative export write `schema_paths`); a value that is not a boolean fails the command | no | -| `SUPABASE_EXPERIMENTAL` | selects the deprecated in-process structured-dump export (same as `--declarative`) when `--declarative` is not set | no | +| `SUPABASE_EXPERIMENTAL` | resolving true (shell, then project `.env`) without `--declarative` fails the command with the removed structured-dump error; `--experimental=false` overrides it for one invocation | no | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `0` | success (migration written + optional history update; declarative export) | -| `1` | target mutex; `--declarative`/`--use-pg-delta` with `--diff-engine`; migration-history conflict; **no schema changes ("No schema changes found")**; connection/shadow/engine failure; file IO error | -| `1` | `--project-ref` set with a resolved target other than linked | +| Code | Condition | +| ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success (migration written + optional history update; declarative export) | +| `1` | target mutex; `--declarative` with `--diff-engine`; migration-history conflict; **no schema changes ("No schema changes found")**; connection/shadow/engine failure; file IO error | +| `1` | `--use-pg-delta` passed at all (any value, including `=false`); `--experimental`/`SUPABASE_EXPERIMENTAL` resolving true without `--declarative` — removed, see Removed surfaces | +| `1` | `--project-ref` set with a resolved target other than linked | > Note: unlike `db diff`, an empty diff (`No schema changes found`) is a **non-zero > exit** for `db pull`. Instead of the generic @@ -158,9 +157,7 @@ at all, so nothing is cached for it. Progress to stderr. Migration path: `Creating shadow database...`, `Diffing schemas[: ]`, `Schema written to `. Declarative path: `Preparing declarative schema export using pg-delta...`, `Declarative schema -written to `. Plus the `--use-pg-delta` deprecation line, the -`--experimental` structured-dump deprecation line, and the history-update -prompt. On success the PostRun line `Finished supabase db pull.` is printed to +written to `. Plus the history-update prompt. On success the PostRun line `Finished supabase db pull.` is printed to stdout. A configured `[db.migrations].schema_paths` also warns on the migration path @@ -180,7 +177,7 @@ Progress strings still go to stderr; stdout carries a single structured envelope block. The linked-database password env is withheld when the target differs from `.temp/project-ref`, with a `Not sending SUPABASE_DB_PASSWORD to : ...` warning and a temporary login role instead. `--password` is rejected with `--db-url` or `--local`. -- `--declarative` / deprecated `--use-pg-delta` are mutually exclusive with +- `--declarative` is mutually exclusive with `--diff-engine`; `--db-url` / `--linked` (default) / `--local` are a target group. - `--diff-engine` defaults to pg-delta. Without the flag, migra is selected only by `[experimental.pgdelta] enabled = false`; the historical @@ -191,14 +188,7 @@ Progress strings still go to stderr; stdout carries a single structured envelope id/labels. It never implies `--linked`: passing it with a resolved `--local`/`--db-url` target is a hard error rather than a silently discarded flag (deliberately stricter than `SUPABASE_PROJECT_ID`, which is simply - unused on a non-linked target). The deprecated - `--experimental` export honors `--project-ref` the same way `--declarative` - does. -- `--use-pg-delta` is hidden, selects the declarative export, and prints - `Flag --use-pg-delta has been deprecated, use --declarative instead.` - to stderr. It also sets `[experimental.pgdelta].enabled` at the flag tier, so the declarative - export leaves `[db.migrations].schema_paths` alone and, if the pull falls back to migration mode - (`--use-pg-delta --declarative=false`, last flag wins), the migration diff uses pg-delta. + unused on a non-linked target). - Migration-style pulls always compare migrations with the live target; declarative files and `schema_paths` do not replace that baseline. - Bundled nontransactional files begin with @@ -207,9 +197,27 @@ Progress strings still go to stderr; stdout carries a single structured envelope it streams a `pg_dump` of the remote schema into the migration file, then appends the migra diff. An empty diff after a non-empty dump is swallowed; an empty dump + empty diff is "No schema changes found". Stack-backed pulls never take this path. -- The `--experimental` structured-dump branch (or the `SUPABASE_EXPERIMENTAL` - project-`.env` equivalent) now runs the same in-process declarative export - as `--declarative`. It is deprecated: a warning pointing at `--declarative` - prints to stderr before the export. `--experimental --declarative` does not - print that line. Output is the pg-delta declarative tree under - `supabase/schemas` (plus `.pgdelta-export.json`). + +### Removed surfaces + +Both fail with `RemovedSurfaceError` (exit 1). Telemetry records `cli_command_executed` with +`error_fingerprint` ending in `:removed_flag`; the post-run `telemetry.json` read and write under +`SUPABASE_HOME` still happens, and `use-pg-delta` stays in the recorded `flags`. + +- `--use-pg-delta` (any value, including `=false`; checked first, even alongside `--declarative`) + fails before any work, on a runtime that needs no credentials, profile or config. Message: + `--use-pg-delta was removed.` Suggestion: "Use --declarative to pull the declarative schema. + The diff engine is chosen with --diff-engine or [experimental.pgdelta] enabled in config.toml." + The flag stays parsed (hidden, bound to `[experimental.pgdelta].enabled`) only so misuse reaches + this error. The migration diff engine is unchanged: pg-delta by default, with migra selected by + `--diff-engine migra`, `[experimental.pgdelta] enabled = false` or its env var. +- The `--experimental` structured-dump mode is the global `--experimental` flag, + `SUPABASE_EXPERIMENTAL` in the shell, or `SUPABASE_EXPERIMENTAL` in the project `.env` resolving + true (flag > shell env > project `.env`) without `--declarative`. It is rejected after the + profile and project `.env*` load, so a malformed `.env` fails first, but before any config, + database or network work. Message: "The --experimental structured-dump mode of db pull was + removed." Suggestion: "Use --declarative to pull per-object schema files, or pass + --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull." + `--experimental --declarative` runs the declarative export without an error, and an explicit + `--experimental=false` beats the env. The check lives in the `db pull` command handler, not in + the shared pull runner, so `supabase pull` is unaffected by `SUPABASE_EXPERIMENTAL`. diff --git a/apps/cli/src/commands/db/pull/pull.command.ts b/apps/cli/src/commands/db/pull/pull.command.ts index 114ed17059..91b53a3cc5 100644 --- a/apps/cli/src/commands/db/pull/pull.command.ts +++ b/apps/cli/src/commands/db/pull/pull.command.ts @@ -1,8 +1,12 @@ +import { Effect, Layer, Option } from "effect"; import { Argument, Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withCliConfigFlags } from "../../../config/cli-config-flags.ts"; import { CliConfigKeys } from "../../../config/cli-config-keys.ts"; +import { removedFlag } from "../../../command-internal/removed-command.ts"; +import { commandRuntimeLayer } from "../../../shared/runtime/command-runtime.layer.ts"; +import { telemetryStateLayer } from "../../../telemetry/telemetry-state.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { parseSchemaFlags } from "../../../command-internal/schema-flags.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; @@ -15,18 +19,19 @@ const config = { Argument.withDescription("Optional name for the migration file."), Argument.optional, ), - // `--declarative` and the deprecated `--use-pg-delta` both select declarative - // export and are mutually exclusive with `--diff-engine`. Optional so the - // mutex tracks whether the flag was passed. + // Mutually exclusive with `--diff-engine`. Optional so the mutex tracks whether the flag was + // passed. declarative: Flag.Boolean("declarative").pipe( Flag.withDescription( "Replace the declarative schema tree from the selected database instead of creating a migration; migration history is not updated.", ), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error; bound to its config key because `db diff` owns the same name. usePgDelta: CliConfigKeys.experimental.pgdelta.enabled.flag({ name: "use-pg-delta", - description: "Use pg-delta to pull declarative schema.", + description: "Removed: use --declarative instead.", hidden: true, }), diffEngine: Flag.Literals("diff-engine", ["migra", "pg-delta"] as const).pipe( @@ -77,35 +82,50 @@ const config = { export type DbPullFlags = CliCommand.Command.Config.Infer; +const DECLARATIVE_SUGGESTION = + "Use --declarative to pull the declarative schema. The diff engine is chosen with --diff-engine or [experimental.pgdelta] enabled in config.toml."; + +// Rejected on a credential-free runtime: `dbPullRuntimeLayer` loads the profile and command +// settings eagerly, so building it first could fail before the removal message. +const removedFlagLayer = Layer.mergeAll(commandRuntimeLayer(["db", "pull"]), telemetryStateLayer); + +const telemetryOptions = (flags: DbPullFlags) => ({ + flags: { + declarative: flags.declarative, + "use-pg-delta": flags.usePgDelta, + "diff-engine": flags.diffEngine, + "strict-coverage": flags.strictCoverage, + schema: flags.schema, + "db-url": flags.dbUrl, + linked: flags.linked, + local: flags.local, + "project-ref": flags.projectRef, + // `password` is a credential — always reaches telemetry as ``. + password: flags.password, + }, + // Not on the established `--project-ref` safeFlags allowlist, so it stays redacted. + aliases: { s: "schema", p: "password" }, + config, +}); + export const dbPullCommand = Command.make("pull", config).pipe( Command.withDescription( "Migration mode compares supabase/migrations with the selected live database (--linked by default), writes the complete difference as migration files, and may record them in that database's migration history. --declarative instead replaces the declarative schema tree and does not create migrations or update migration history.", ), Command.withShortDescription("Pull schema from the remote database"), Command.withHandler((flags) => - dbPull(flags).pipe( - withCommandTelemetry({ - flags: { - declarative: flags.declarative, - "use-pg-delta": flags.usePgDelta, - "diff-engine": flags.diffEngine, - "strict-coverage": flags.strictCoverage, - schema: flags.schema, - "db-url": flags.dbUrl, - linked: flags.linked, - local: flags.local, - "project-ref": flags.projectRef, - // `password` is a credential — always reaches telemetry as ``. - password: flags.password, - }, - // Not on the established `--project-ref` safeFlags allowlist, so it stays redacted. - aliases: { s: "schema", p: "password" }, - config, - }), - withJsonErrorHandling, - ), + Option.isSome(flags.usePgDelta) + ? removedFlag("--use-pg-delta", DECLARATIVE_SUGGESTION).pipe( + withCommandTelemetry(telemetryOptions(flags)), + withJsonErrorHandling, + Effect.provide(removedFlagLayer), + ) + : dbPull(flags).pipe( + withCommandTelemetry(telemetryOptions(flags)), + withJsonErrorHandling, + Effect.provide(dbPullRuntimeLayer), + ), ), - Command.provide(dbPullRuntimeLayer), Command.provide(cliConfigValuesLayer), withCliConfigFlags(config), ); diff --git a/apps/cli/src/commands/db/pull/pull.errors.ts b/apps/cli/src/commands/db/pull/pull.errors.ts index 93389bda8c..93b9bd9596 100644 --- a/apps/cli/src/commands/db/pull/pull.errors.ts +++ b/apps/cli/src/commands/db/pull/pull.errors.ts @@ -19,8 +19,8 @@ export class DbPullTargetFlagsError extends Data.TaggedError("DbPullTargetFlagsE } /** - * `--declarative` / `--use-pg-delta` combined with `--diff-engine`; message - * text is an established output contract. + * `--declarative` combined with `--diff-engine`; message text is an established output + * contract. */ export class DbPullEngineConflictError extends Data.TaggedError("DbPullEngineConflictError")<{ readonly message: string; diff --git a/apps/cli/src/commands/db/pull/pull.handler.ts b/apps/cli/src/commands/db/pull/pull.handler.ts index 32e0e52328..c075c36bb8 100644 --- a/apps/cli/src/commands/db/pull/pull.handler.ts +++ b/apps/cli/src/commands/db/pull/pull.handler.ts @@ -1,18 +1,39 @@ -import { Effect } from "effect"; +import { Effect, FileSystem, Option, Path } from "effect"; import { aqua } from "../../../command-internal/colors.ts"; +import { loadProjectEnvValues } from "../../../command-internal/db-config.toml-read.ts"; +import { resolveExperimentalWithProjectEnv } from "../../../command-internal/global-flags.ts"; +import { removedFlag } from "../../../command-internal/removed-command.ts"; +import { CommandSettings } from "../../../config/command-settings.service.ts"; import { Output } from "../../../shared/output/output.service.ts"; -import { runDbPull, type DbPullInvoke } from "../../../command-internal/db-pull-run.ts"; +import { runDbPull } from "../../../command-internal/db-pull-run.ts"; import type { DbPullFlags } from "./pull.command.ts"; +const EXPERIMENTAL_SUGGESTION = + "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; + /** * `dbPull`'s established external behavior (stdout/stderr, JSON payload, * "Finished" line), reimplemented on top of {@link runDbPull}: run the * pull, then emit based on the returned outcome. */ -export const dbPull = Effect.fn("db.pull")(function* (flags: DbPullFlags, invoke?: DbPullInvoke) { +export const dbPull = Effect.fn("db.pull")(function* (flags: DbPullFlags) { + // Checked here rather than in `runDbPull` so the `pull` orchestrator, which calls `runDbPull` + // directly, keeps working when `SUPABASE_EXPERIMENTAL` is set. + if (!Option.getOrElse(flags.declarative, () => false)) { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cliSettings = yield* CommandSettings; + const projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); + if (yield* resolveExperimentalWithProjectEnv(projectEnv)) { + return yield* removedFlag( + "The --experimental structured-dump mode of db pull", + EXPERIMENTAL_SUGGESTION, + ); + } + } const output = yield* Output; - const outcome = yield* runDbPull(flags, invoke); + const outcome = yield* runDbPull(flags); yield* Effect.annotateCurrentSpan({ "db.pull.outcome": outcome.kind, diff --git a/apps/cli/src/commands/db/pull/pull.integration.test.ts b/apps/cli/src/commands/db/pull/pull.integration.test.ts index 92a931a95e..77cd5ce1bf 100644 --- a/apps/cli/src/commands/db/pull/pull.integration.test.ts +++ b/apps/cli/src/commands/db/pull/pull.integration.test.ts @@ -36,7 +36,7 @@ import { mockStdin, mockTty, } from "../../../../tests/helpers/mocks.ts"; -import { configValuesLayer, flagInput } from "../../../../tests/helpers/config-values-layer.ts"; +import { configValuesLayer } from "../../../../tests/helpers/config-values-layer.ts"; import { DebugFlag, DnsResolverFlag, @@ -65,7 +65,10 @@ import { PgDeltaSslProbe } from "../../../command-internal/pgdelta-ssl-probe.ser import { PgDeltaEngine, PgDeltaEngineError } from "../shared/pgdelta-engine.service.ts"; import type { DbPullFlags } from "./pull.command.ts"; import { dbPull } from "./pull.handler.ts"; -import { runDbPull } from "../../../command-internal/db-pull-run.ts"; +import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; + +const EXPERIMENTAL_SUGGESTION = + "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; const alwaysReadyHttpClientLayer = Layer.succeed( HttpClient.HttpClient, @@ -95,7 +98,6 @@ const pgDeltaDiffEnvelope = ( }); interface SetupOpts { - readonly usePgDelta?: boolean; readonly env?: Readonly>; readonly nextDebugDirectory?: string; readonly format?: OutputFormat; @@ -122,8 +124,7 @@ interface SetupOpts { readonly dumpFailFirstWith?: string; // Bytes the first dump attempt streams before failing with `dumpFailFirstWith`. readonly dumpFailFirstPartialBytes?: string; - // Raw argv seen by the handler (CliArgs). Only consulted when both `--declarative` - // and `--use-pg-delta` are present, to replay last-occurrence-wins ordering. + // Raw argv seen by the handler (CliArgs); the explicit `--yes`/`--experimental` scanners read it. readonly args?: ReadonlyArray; readonly networkId?: string; readonly platform?: NodeJS.Platform; @@ -450,10 +451,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { out.layer, configValuesLayer({ output: out.layer, - flags: - opts.usePgDelta === undefined - ? [] - : [flagInput("experimental.pgdelta.enabled", "use-pg-delta", opts.usePgDelta)], env: opts.env, }), telemetry.layer, @@ -652,11 +649,11 @@ describe("db pull", () => { }).pipe(Effect.provide(s.layer)); }); - it.effect("honors --project-ref on the deprecated --experimental export", () => { + it.effect("honors --project-ref on the declarative export", () => { const FLAG_REF = "flagflagflagflagflag"; - const s = setup(tmp.current, { experimental: true, edgeStdout: EXPORT_JSON }); + const s = setup(tmp.current, { edgeStdout: EXPORT_JSON }); return Effect.gen(function* () { - yield* dbPull(flags({ projectRef: Option.some(FLAG_REF) })); + yield* dbPull(flags({ declarative: Option.some(true), projectRef: Option.some(FLAG_REF) })); expect(s.engineCalls[0]?.operation).toBe("export"); expect(s.engineCalls[0]?.projectRef).toBe(FLAG_REF); }).pipe(Effect.provide(s.layer)); @@ -1039,21 +1036,6 @@ describe("db pull", () => { }).pipe(Effect.provide(s.layer)); }); - it.effect( - "deprecated --use-pg-delta prints the deprecation line and behaves like --declarative", - () => { - const s = setup(tmp.current, { usePgDelta: true, edgeStdout: EXPORT_JSON }); - return Effect.gen(function* () { - yield* dbPull(flags({ usePgDelta: Option.some(true) })); - expect(streamText(s.out, "stderr")).toContain("Flag --use-pg-delta has been deprecated"); - const path = yield* Path.Path; - expect(streamText(s.out, "stderr")).toContain( - `Declarative schema written to ${path.join("supabase", "schemas")}\n`, - ); - }).pipe(Effect.provide(s.layer)); - }, - ); - it.effect("passes the config/workdir-resolved project id to the pg-delta engine", () => { // Absent env/config project_id, the workdir basename must reach the pg-delta // engine's project id, including on the declarative-export path (read before any @@ -1092,67 +1074,6 @@ describe("db pull", () => { }, ); - it.effect( - "--declarative --use-pg-delta=false stays in migration mode (last occurrence wins)", - () => { - // Both flags bind to one variable, so the last occurrence wins — ORing the two - // parsed flags would wrongly take the declarative path instead. - const s = setup(tmp.current, { - usePgDelta: false, - migrations: ["20240101000000"], - remoteVersions: ["20240101000000"], - files: { "supabase/config.toml": "[experimental.pgdelta]\nenabled = false\n" }, - edgeStdout: "create table remote ();\n", - yes: true, - args: ["db", "pull", "--declarative", "--use-pg-delta=false"], - }); - return Effect.gen(function* () { - yield* dbPull(flags({ declarative: Option.some(true), usePgDelta: Option.some(false) })); - expect(s.historyUpserts.length).toBe(1); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect( - "--use-pg-delta --declarative=false stays in migration mode and diffs with pg-delta", - () => { - const s = setup(tmp.current, { - usePgDelta: true, - migrations: ["20240101000000"], - remoteVersions: ["20240101000000"], - files: { "supabase/config.toml": "[experimental.pgdelta]\nenabled = false\n" }, - edgeStdout: pgDeltaDiffEnvelope([ - { name: "schema_changes", sql: "create table remote ();" }, - ]), - yes: true, - args: ["db", "pull", "--use-pg-delta", "--declarative=false"], - }); - return Effect.gen(function* () { - yield* dbPull(flags({ declarative: Option.some(false), usePgDelta: Option.some(true) })); - expect(s.historyUpserts.length).toBe(1); - expect(s.engineCalls[0]?.operation).toBe("diff"); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect("--declarative --use-pg-delta (both true) takes the declarative export path", () => { - const s = setup(tmp.current, { - usePgDelta: true, - edgeStdout: EXPORT_JSON, - args: ["db", "pull", "--declarative", "--use-pg-delta"], - }); - return Effect.gen(function* () { - yield* dbPull(flags({ declarative: Option.some(true), usePgDelta: Option.some(true) })); - expect(s.engineCalls[0]?.operation).toBe("export"); - const fs = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - expect( - yield* fs.exists(path.join(tmp.current, "supabase", "schemas", "public", "t.sql")), - ).toBe(true); - expect(s.historyUpserts.length).toBe(0); - }).pipe(Effect.provide(s.layer)); - }); - it.effect("a migration-history conflict fails with the repair suggestion", () => { const s = setup(tmp.current, { migrations: ["20240102000000"], @@ -1761,139 +1682,84 @@ describe("db pull", () => { }, ); - it.effect( - "SUPABASE_EXPERIMENTAL prints a deprecation warning and runs the in-process declarative export", - () => { - const s = setup(tmp.current, { edgeStdout: EXPORT_JSON }); - return Effect.gen(function* () { - yield* withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, dbPull(flags())); - expect(s.engineCalls[0]?.operation).toBe("export"); - expect(streamText(s.out, "stderr")).toContain("Connecting to remote database..."); - expect(streamText(s.out, "stderr")).toContain( - "The --experimental structured-dump mode for `db pull` is deprecated", - ); - expect(streamText(s.out, "stderr")).toContain("Preparing declarative schema export"); - }).pipe(Effect.provide(s.layer)); - }, - ); + const expectNothingRan = (s: ReturnType) => { + expect(s.resolveCalls).toEqual([]); + expect(s.connectedDatabases).toEqual([]); + expect(s.engineCalls).toEqual([]); + expect(s.edgeCalls).toEqual([]); + expect(s.shadowSpawned).toEqual([]); + expect(s.cache.cached).toBe(false); + }; - it.effect( - "forceMigrationMode overrides SUPABASE_EXPERIMENTAL to keep runDbPull in migration mode; unset preserves the existing declarative-export resolution", - () => { - const forced = setup(tmp.current, { - migrations: ["20240101000000"], - remoteVersions: ["20240101000000"], - edgeStdout: pgDeltaDiffEnvelope([ - { name: "schema_changes", sql: "create table remote ();" }, - ]), + it.effect("rejects the global --experimental flag without --declarative", () => { + const s = setup(tmp.current, { + experimental: true, + edgeStdout: EXPORT_JSON, + args: ["db", "pull", "--experimental"], + }); + return Effect.gen(function* () { + const error = yield* dbPull(flags()).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expect(error).toMatchObject({ + kind: "flag", + message: "The --experimental structured-dump mode of db pull was removed.", + suggestion: EXPERIMENTAL_SUGGESTION, }); - const unforced = setup(tmp.current, { edgeStdout: EXPORT_JSON }); - return withConfigEnv( - { SUPABASE_EXPERIMENTAL: "true" }, - Effect.gen(function* () { - // `forceMigrationMode: true` keeps an in-process caller in migration mode - // even though the ambient `SUPABASE_EXPERIMENTAL` gate would otherwise select - // the deprecated declarative export. - yield* runDbPull(flags({ diffEngine: Option.some("pg-delta") }), { - forceMigrationMode: true, - }).pipe(Effect.provide(forced.layer)); - expect(forced.engineCalls[0]?.operation).toBe("diff"); - expect(streamText(forced.out, "stderr")).not.toContain( - "Preparing declarative schema export", - ); + expectNothingRan(s); + }).pipe(Effect.provide(s.layer)); + }); - // Unset preserves the original behavior: the ambient gate still switches to - // the in-process declarative export. - yield* runDbPull(flags()).pipe(Effect.provide(unforced.layer)); - expect(unforced.engineCalls[0]?.operation).toBe("export"); - expect(streamText(unforced.out, "stderr")).toContain( - "Preparing declarative schema export", - ); - }), - ); - }, - ); + it.effect("rejects SUPABASE_EXPERIMENTAL without --declarative", () => { + const s = setup(tmp.current, { edgeStdout: EXPORT_JSON }); + return Effect.gen(function* () { + const error = yield* dbPull(flags({ diffEngine: Option.some("migra") })).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expect(error).toMatchObject({ suggestion: EXPERIMENTAL_SUGGESTION }); + expectNothingRan(s); + }).pipe(Effect.provide(s.layer), (body) => + withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), + ); + }); - it.effect("--experimental still exports when the last --declarative alias is false", () => { + it.effect("rejects SUPABASE_EXPERIMENTAL set only in supabase/.env", () => { const s = setup(tmp.current, { - usePgDelta: false, - experimental: true, edgeStdout: EXPORT_JSON, - args: ["db", "pull", "--experimental", "--declarative", "--use-pg-delta=false"], + files: { "supabase/.env": "SUPABASE_EXPERIMENTAL=true\n" }, }); return Effect.gen(function* () { - yield* dbPull(flags({ declarative: Option.some(true), usePgDelta: Option.some(false) })); - expect(s.engineCalls[0]?.operation).toBe("export"); - expect(streamText(s.out, "stderr")).toContain( - "The --experimental structured-dump mode for `db pull` is deprecated", - ); + const error = yield* dbPull(flags()).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expectNothingRan(s); + }).pipe(Effect.provide(s.layer), withEmptyConfigEnv); + }); + + it.effect("rejects --experimental with --declarative=false", () => { + const s = setup(tmp.current, { experimental: true, args: ["db", "pull", "--experimental"] }); + return Effect.gen(function* () { + const error = yield* dbPull(flags({ declarative: Option.some(false) })).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expectNothingRan(s); }).pipe(Effect.provide(s.layer)); }); - it.effect("--experimental with --diff-engine still runs the in-process export", () => { - const s = setup(tmp.current, { experimental: true, edgeStdout: EXPORT_JSON }); + it.effect("--experimental --declarative runs the declarative export without an error", () => { + const s = setup(tmp.current, { + experimental: true, + edgeStdout: EXPORT_JSON, + args: ["db", "pull", "--experimental", "--declarative"], + }); return Effect.gen(function* () { - yield* dbPull(flags({ diffEngine: Option.some("migra") })); + yield* dbPull(flags({ declarative: Option.some(true) })); expect(s.engineCalls[0]?.operation).toBe("export"); + expect(streamText(s.out, "stderr")).toContain( + "Preparing declarative schema export using pg-delta...", + ); }).pipe(Effect.provide(s.layer)); }); - it.effect( - "the global --experimental flag prints a deprecation warning and runs the in-process export", - () => { - const s = setup(tmp.current, { experimental: true, edgeStdout: EXPORT_JSON }); - return Effect.gen(function* () { - yield* dbPull(flags()); - expect(s.engineCalls[0]?.operation).toBe("export"); - expect(streamText(s.out, "stderr")).toContain("Connecting to remote database..."); - expect(streamText(s.out, "stderr")).toContain( - "The --experimental structured-dump mode for `db pull` is deprecated", - ); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect( - "an experimental pull in json mode reports a declarative export with no history repair", - () => { - const s = setup(tmp.current, { - experimental: true, - format: "json", - edgeStdout: EXPORT_JSON, - }); - return Effect.gen(function* () { - yield* dbPull(flags()); - const success = s.out.messages.find((m) => m.type === "success"); - expect(success?.data).toMatchObject({ - declarative: true, - remoteHistoryUpdated: false, - engine: "pg-delta", - }); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect( - "--declarative wins over --experimental and is unaffected by the deprecated experimental mode", - () => { - // Declarative mode is checked before the experimental gate, so it runs normally - // even when --experimental is also set. - const s = setup(tmp.current, { experimental: true, edgeStdout: EXPORT_JSON }); - return Effect.gen(function* () { - yield* dbPull(flags({ declarative: Option.some(true) })); - expect(streamText(s.out, "stderr")).toContain( - "Preparing declarative schema export using pg-delta...", - ); - expect(streamText(s.out, "stderr")).not.toContain("is deprecated"); - }).pipe(Effect.provide(s.layer)); - }, - ); - it.effect( "an explicit --experimental=false wins over SUPABASE_EXPERIMENTAL=true and pulls normally", () => { - // A set flag value wins over env regardless of true/false, so `--experimental=false` - // isn't overridden by a truthy `SUPABASE_EXPERIMENTAL`. const s = setup(tmp.current, { migrations: ["20240101000000"], remoteVersions: ["20240101000000"], @@ -1904,6 +1770,7 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags({ diffEngine: Option.some("migra") })); expect(streamText(s.out, "stderr")).toContain("Connecting to remote database...\n"); + expect(s.historyUpserts.length).toBe(1); }).pipe(Effect.provide(s.layer), (body) => withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), ); @@ -1913,17 +1780,13 @@ describe("db pull", () => { it.effect( "a migration name literally '--experimental=false' after -- does not suppress SUPABASE_EXPERIMENTAL", () => { - // Flag parsing stops at the first bare - // `--`, so `db pull -- --experimental=false` passes it as the positional - // migration-name argument, not a flag occurrence — unlike the unterminated case - // above, this must still take the experimental export. - const s = setup(tmp.current, { - args: ["db", "pull", "--", "--experimental=false"], - edgeStdout: EXPORT_JSON, - }); + const s = setup(tmp.current, { args: ["db", "pull", "--", "--experimental=false"] }); return Effect.gen(function* () { - yield* dbPull(flags({ name: Option.some("--experimental=false") })); - expect(s.engineCalls[0]?.operation).toBe("export"); + const error = yield* dbPull(flags({ name: Option.some("--experimental=false") })).pipe( + Effect.flip, + ); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expectNothingRan(s); }).pipe(Effect.provide(s.layer), (body) => withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), ); @@ -1931,35 +1794,29 @@ describe("db pull", () => { ); it.effect( - "a repeated --experimental=false --experimental=true still exports (last Set() wins)", + "a repeated --experimental=false --experimental=true is rejected (last one wins)", () => { - // Each flag binds one variable: repeated occurrences collapse to - // whichever Set() call happened last, so a resolver must not get this ordering - // backwards. const s = setup(tmp.current, { args: ["db", "pull", "--experimental=false", "--experimental=true"], - edgeStdout: EXPORT_JSON, }); return Effect.gen(function* () { - yield* dbPull(flags()); - expect(s.engineCalls[0]?.operation).toBe("export"); + const error = yield* dbPull(flags()).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expectNothingRan(s); }).pipe(Effect.provide(s.layer)); }, ); it.effect( - "a bare --password consumes the following token, so SUPABASE_EXPERIMENTAL still gates the experimental export", + "a bare --password consumes the following token, so SUPABASE_EXPERIMENTAL is still rejected", () => { - // `--password --experimental=false` parses as `--password`'s value being the - // literal string "--experimental=false" — `--experimental` itself was never - // Changed, so the pull must still fall back to `SUPABASE_EXPERIMENTAL=true`. const s = setup(tmp.current, { args: ["db", "pull", "--password", "--experimental=false"], - edgeStdout: EXPORT_JSON, }); return Effect.gen(function* () { - yield* dbPull(flags()); - expect(s.engineCalls[0]?.operation).toBe("export"); + const error = yield* dbPull(flags()).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expectNothingRan(s); }).pipe(Effect.provide(s.layer), (body) => withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), ); diff --git a/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts new file mode 100644 index 0000000000..3a0a26d367 --- /dev/null +++ b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts @@ -0,0 +1,241 @@ +import { BunServices } from "@effect/platform-bun"; +import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { Cause, Effect, Exit, FileSystem, Layer, Path, Stdio } from "effect"; +import { CliOutput, Command } from "effect/cli"; + +import { GLOBAL_FLAGS } from "../../../command-internal/global-flags.ts"; +import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; +import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; +import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; +import { + mockContextualAnalytics, + mockOutput, + mockProcessControl, + mockTelemetryRuntime, + mockTty, +} from "../../../../tests/helpers/mocks.ts"; +import { isolatedHomeLayer } from "../../../../tests/helpers/command-mocks.ts"; +import { dbCommand } from "../db.command.ts"; + +const testRoot = Command.make("supabase").pipe( + Command.withSubcommands([dbCommand]), + Command.withGlobalFlags(GLOBAL_FLAGS), +); + +const USE_PG_DELTA_SUGGESTION = + "Use --declarative to pull the declarative schema. The diff engine is chosen with --diff-engine or [experimental.pgdelta] enabled in config.toml."; +const EXPERIMENTAL_MESSAGE = "The --experimental structured-dump mode of db pull was removed."; +const EXPERIMENTAL_SUGGESTION = + "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; + +const runDbPull = ( + args: ReadonlyArray, + opts: { + readonly env?: Readonly>; + readonly files?: Readonly>; + readonly format?: "text" | "json"; + } = {}, +) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workdir = yield* fs.makeTempDirectoryScoped({ prefix: "supabase-db-pull-removed-" }); + for (const [file, content] of Object.entries(opts.files ?? {})) { + yield* fs.makeDirectory(path.dirname(path.join(workdir, file)), { recursive: true }); + yield* fs.writeFileString(path.join(workdir, file), content); + } + const argv = ["db", "pull", "--workdir", workdir, ...args]; + const out = mockOutput({ format: opts.format ?? "text" }); + const analytics = mockContextualAnalytics(); + const layer = Layer.mergeAll( + CliOutput.layer(textCliOutputFormatter()), + Layer.succeed(CliArgs, { args: argv }), + Stdio.layerTest({ args: Effect.succeed(argv) }), + out.layer, + analytics.layer, + mockTelemetryRuntime({ configDir: path.join(workdir, ".supabase") }), + mockTty(), + mockProcessControl().layer, + isolatedHomeLayer(workdir, { + SUPABASE_HOME: workdir, + SUPABASE_NO_KEYRING: "1", + SUPABASE_ACCESS_TOKEN: undefined, + SUPABASE_PROFILE: undefined, + SUPABASE_EXPERIMENTAL: undefined, + ...opts.env, + }), + ); + const exit = yield* Command.runWith(testRoot, { version: "0.0.0-test" })(argv).pipe( + Effect.provide(layer), + Effect.exit, + ); + return { exit, out, analytics, workdir }; + }).pipe(Effect.scoped, Effect.provide(BunServices.layer)); + +const removedError = (exit: Exit.Exit) => { + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) throw new Error("expected failure"); + const error = Cause.squash(exit.cause); + expect(error).toBeInstanceOf(RemovedSurfaceError); + return error as RemovedSurfaceError; +}; + +describe("db pull removed surfaces through command dispatch", () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new Error("unexpected network request")); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + }); + + for (const flag of ["--use-pg-delta", "--use-pg-delta=false"]) { + it.live(`rejects ${flag} with the removal error`, () => + Effect.gen(function* () { + const { exit, out, analytics } = yield* runDbPull([flag]); + + const error = removedError(exit); + expect(error.kind).toBe("flag"); + expect(error.message).toBe("--use-pg-delta was removed."); + expect(error.suggestion).toBe(USE_PG_DELTA_SUGGESTION); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(out.rawChunks).toEqual([]); + + const events = analytics.captured.filter((c) => c.event === "cli_command_executed"); + expect(events).toHaveLength(1); + expect(events[0]?.properties).toMatchObject({ + command: "db pull", + exit_code: 1, + error_fingerprint: "tag:RemovedSurfaceError:removed_flag", + }); + expect(events[0]?.properties.flags).toMatchObject({ + "use-pg-delta": flag === "--use-pg-delta", + }); + }), + ); + } + + it.live("rejects --use-pg-delta with a missing profile and no credentials", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--use-pg-delta"], { + env: { SUPABASE_PROFILE: "missing-profile" }, + }); + + expect(removedError(exit).message).toBe("--use-pg-delta was removed."); + }), + ); + + it.live("a missing profile still fails plain db pull with a different error", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull([], { env: { SUPABASE_PROFILE: "missing-profile" } }); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + }), + ); + + it.live("does not reject a plain db pull", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull([]); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + }), + ); + + it.live("does not reject --experimental=false over SUPABASE_EXPERIMENTAL=true", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--experimental=false"], { + env: { SUPABASE_EXPERIMENTAL: "true" }, + }); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + }), + ); + + it.live("does not reject --experimental --declarative", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--experimental", "--declarative"]); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + }), + ); + + it.live("rejects --use-pg-delta combined with --declarative", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--declarative", "--use-pg-delta"]); + + expect(removedError(exit).message).toBe("--use-pg-delta was removed."); + }), + ); + + const experimentalCases: ReadonlyArray<{ + readonly title: string; + readonly args: ReadonlyArray; + readonly env?: Record; + }> = [ + { title: "--experimental", args: ["--experimental"] }, + { title: "--experimental=true", args: ["--experimental=true"] }, + { title: "SUPABASE_EXPERIMENTAL=true", args: [], env: { SUPABASE_EXPERIMENTAL: "true" } }, + { + title: "SUPABASE_EXPERIMENTAL=true with --declarative=false", + args: ["--declarative=false"], + env: { SUPABASE_EXPERIMENTAL: "true" }, + }, + ]; + + for (const { title, args, env } of experimentalCases) { + it.live(`rejects the structured-dump mode via ${title}`, () => + Effect.gen(function* () { + const { exit, analytics } = yield* runDbPull(args, { env }); + + const error = removedError(exit); + expect(error.kind).toBe("flag"); + expect(error.message).toBe(EXPERIMENTAL_MESSAGE); + expect(error.suggestion).toBe(EXPERIMENTAL_SUGGESTION); + expect(fetchSpy).not.toHaveBeenCalled(); + + const events = analytics.captured.filter((c) => c.event === "cli_command_executed"); + expect(events).toHaveLength(1); + expect(events[0]?.properties).toMatchObject({ + command: "db pull", + exit_code: 1, + error_fingerprint: "tag:RemovedSurfaceError:removed_flag", + }); + }), + ); + } + + it.live("rejects SUPABASE_EXPERIMENTAL set only in supabase/.env", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull([], { + files: { "supabase/.env": "SUPABASE_EXPERIMENTAL=1\n" }, + }); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + + it.live("reports the removal through the JSON error envelope", () => + Effect.gen(function* () { + const { exit, out } = yield* runDbPull(["--use-pg-delta"], { format: "json" }); + + expect(Exit.isSuccess(exit)).toBe(true); + expect(out.failures).toHaveLength(1); + expect(out.failures[0]).toMatchObject({ + code: "RemovedSurfaceError", + message: "--use-pg-delta was removed.", + }); + }), + ); +}); diff --git a/apps/cli/src/commands/db/seed-flag-telemetry.integration.test.ts b/apps/cli/src/commands/db/seed-flag-telemetry.integration.test.ts index 5a07050557..9543ae33f3 100644 --- a/apps/cli/src/commands/db/seed-flag-telemetry.integration.test.ts +++ b/apps/cli/src/commands/db/seed-flag-telemetry.integration.test.ts @@ -53,7 +53,7 @@ describe("seed and pg-delta flag telemetry", () => { ); it.live("records --use-pg-delta=false as a boolean", () => - capturedFlags(["db", "pull"], ["--use-pg-delta=false"], { + capturedFlags(["db", "diff"], ["--use-pg-delta=false"], { "use-pg-delta": Option.some(false), }).pipe( Effect.tap((flags) => Effect.sync(() => expect(flags).toEqual({ "use-pg-delta": false }))), diff --git a/apps/cli/src/commands/domains/SIDE_EFFECTS.md b/apps/cli/src/commands/domains/SIDE_EFFECTS.md index 597288d829..4560aee7ce 100644 --- a/apps/cli/src/commands/domains/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/domains/SIDE_EFFECTS.md @@ -39,20 +39,21 @@ Cloudflare DNS-over-HTTPS CNAME pre-check. ## Exit Codes -| Code | Condition | -| ---- | ------------------------------------------------------------------------------- | -| `0` | success | -| `1` | project ref cannot be resolved / is malformed | -| `1` | `create`: hostname has no matching CNAME record (Cloudflare DNS pre-check) | -| `1` | API error — unexpected (non-201/200) response from the custom-hostname endpoint | -| `1` | network / connection failure | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | project ref cannot be resolved / is malformed | +| `1` | `create`: hostname has no matching CNAME record (Cloudflare DNS pre-check) | +| `1` | API error — unexpected (non-201/200) response from the custom-hostname endpoint | +| `1` | network / connection failure | +| `1` | `--include-raw-output` passed at all (any value, including `=false`) — removed, see Notes | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ----------------------- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | -| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` (all redacted — no `domains` flag is telemetry-safe) | -| `cli_upgrade_suggested` | `create`/`get`/`activate`/`reverify` 4xx carrying the `entitlement_required` envelope | `feature_key` (from the envelope), `org_slug` (parsed from `upgrade_url`); envelope-only, no entitlements fallback | +| Event | When | Notable properties / groups | +| ----------------------- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` (all redacted — no `domains` flag is telemetry-safe); a rejected `--include-raw-output` fails with `error_fingerprint` ending in `:removed_flag` (`RemovedSurfaceError`) | +| `cli_upgrade_suggested` | `create`/`get`/`activate`/`reverify` 4xx carrying the `entitlement_required` envelope | `feature_key` (from the envelope), `org_slug` (parsed from `upgrade_url`); envelope-only, no entitlements fallback | ## Output @@ -83,7 +84,7 @@ A `result` event carrying the custom-hostname response object. ### `-o {json,yaml,toml,env}` -When the `--output`/`-o` flag is set (or `--include-raw-output` forces `json`), +When the `--output`/`-o` flag is set, the full response is encoded to stdout in that format and the human status is suppressed on stderr. `delete` ignores `-o`. @@ -94,7 +95,7 @@ suppressed on stderr. `delete` ignores `-o`. - All subcommands resolve the ref via `--project-ref` → `SUPABASE_PROJECT_ID` env → linked-project file. - The project-ref fallback env var is `SUPABASE_PROJECT_ID`. - **Notes:** - - `--include-raw-output` is declared as a normal boolean **on each subcommand**. Two consequences: (a) it must appear after the subcommand name (`domains get --include-raw-output`) rather than before it (`domains --include-raw-output get`), matching how `--project-ref` is already handled shell-wide; (b) it is not hidden from help and emits no `Flag --include-raw-output has been deprecated` stderr warning, since Effect CLI has no hook for either. It forces `-o json` when `-o` is unset/pretty; on `delete` it is inert. + - `--include-raw-output` is removed on every subcommand, `delete` included. Passing it (any value, including `=false`) fails with `--include-raw-output was removed.` and the suggestion `Use -o json instead.`, before credentials are resolved or any API work runs, so it needs no login. `RemovedSurfaceError` records `cli_command_executed` with `error_fingerprint` ending in `:removed_flag`, and the post-run `telemetry.json` read and write under `SUPABASE_HOME` still happens. The flag stays parsed (hidden) only so misuse reaches this error instead of an unknown-flag parse error; its rejection lives in `domains.removed-flag.ts`. - The degenerate `validation_records != 1` status message is an approximate struct dump (the exact text is not stable). - Text-mode status output is newline-terminated. Without the final newline, interactive shell prompts can redraw over the last status line, hiding the ACME TXT record. - In a structured `-o` mode the human status is suppressed on stderr, keeping stdout clean. diff --git a/apps/cli/src/commands/domains/activate/activate.command.ts b/apps/cli/src/commands/domains/activate/activate.command.ts index e4cb5d7b8a..f517d6b288 100644 --- a/apps/cli/src/commands/domains/activate/activate.command.ts +++ b/apps/cli/src/commands/domains/activate/activate.command.ts @@ -1,8 +1,10 @@ +import { Effect, Option } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { managementApiRuntimeLayer } from "../../../command-internal/management-api-runtime.layer.ts"; +import { rejectIncludeRawOutput } from "../domains.removed-flag.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { domainsActivate } from "./activate.handler.ts"; @@ -11,12 +13,18 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error. includeRawOutput: Flag.Boolean("include-raw-output").pipe( - Flag.withDescription("(Deprecated) use -o json instead."), - Flag.withDefault(false), + Flag.withDescription("Removed: use -o json instead."), + Flag.optional, + Flag.withHidden, ), } as const; +const COMMAND_PATH = ["domains", "activate"]; +const managementApiLayer = managementApiRuntimeLayer(COMMAND_PATH); + export type DomainsActivateFlags = CliCommand.Command.Config.Infer; export const domainsActivateCommand = Command.make("activate", config).pipe( @@ -31,7 +39,12 @@ export const domainsActivateCommand = Command.make("activate", config).pipe( }, ]), Command.withHandler((flags) => - domainsActivate(flags).pipe(withCommandTelemetry({ flags }), withJsonErrorHandling), + Option.isSome(flags.includeRawOutput) + ? rejectIncludeRawOutput(COMMAND_PATH, flags) + : domainsActivate(flags).pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(managementApiLayer), + ), ), - Command.provide(managementApiRuntimeLayer(["domains", "activate"])), ); diff --git a/apps/cli/src/commands/domains/activate/activate.handler.ts b/apps/cli/src/commands/domains/activate/activate.handler.ts index cc267ad126..1728215196 100644 --- a/apps/cli/src/commands/domains/activate/activate.handler.ts +++ b/apps/cli/src/commands/domains/activate/activate.handler.ts @@ -32,6 +32,6 @@ export const domainsActivate = Effect.fn("domains.activate")(function* ( ); yield* activating?.clear ?? Effect.void; - yield* emitHostnameResult(response, flags.includeRawOutput); + yield* emitHostnameResult(response); }).pipe(Effect.ensuring(linkedProjectCache.cache(ref)), Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/commands/domains/activate/activate.integration.test.ts b/apps/cli/src/commands/domains/activate/activate.integration.test.ts index 0d124af06e..9c1ebdbfeb 100644 --- a/apps/cli/src/commands/domains/activate/activate.integration.test.ts +++ b/apps/cli/src/commands/domains/activate/activate.integration.test.ts @@ -65,7 +65,7 @@ function setup(opts: SetupOpts = {}) { return { layer, out, api, analytics, telemetry, linkedProjectCache }; } -const baseFlags = { projectRef: Option.none(), includeRawOutput: false }; +const baseFlags = { projectRef: Option.none(), includeRawOutput: Option.none() }; describe("domains activate integration", () => { it.live("suggests upgrade from entitlement_required envelope on 400", () => { @@ -127,14 +127,6 @@ describe("domains activate integration", () => { }).pipe(Effect.provide(layer)); }); - it.live("forces JSON output when --include-raw-output is set", () => { - const { layer, out } = setup(); - return Effect.gen(function* () { - yield* domainsActivate({ projectRef: Option.none(), includeRawOutput: true }); - expect(out.stdoutText.startsWith("{")).toBe(true); - }).pipe(Effect.provide(layer)); - }); - it.live("fails with DomainsUnexpectedStatusError on HTTP 503", () => { const { layer, telemetry } = setup({ status: 503 }); return Effect.gen(function* () { diff --git a/apps/cli/src/commands/domains/create/create.command.ts b/apps/cli/src/commands/domains/create/create.command.ts index 008fde822d..7b4923a276 100644 --- a/apps/cli/src/commands/domains/create/create.command.ts +++ b/apps/cli/src/commands/domains/create/create.command.ts @@ -1,8 +1,10 @@ +import { Effect, Option } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { managementApiRuntimeLayer } from "../../../command-internal/management-api-runtime.layer.ts"; +import { rejectIncludeRawOutput } from "../domains.removed-flag.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { domainsCreate } from "./create.handler.ts"; @@ -14,12 +16,18 @@ const config = { customHostname: Flag.String("custom-hostname").pipe( Flag.withDescription("The custom hostname to use for your Supabase project."), ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error. includeRawOutput: Flag.Boolean("include-raw-output").pipe( - Flag.withDescription("(Deprecated) use -o json instead."), - Flag.withDefault(false), + Flag.withDescription("Removed: use -o json instead."), + Flag.optional, + Flag.withHidden, ), } as const; +const COMMAND_PATH = ["domains", "create"]; +const managementApiLayer = managementApiRuntimeLayer(COMMAND_PATH); + export type DomainsCreateFlags = CliCommand.Command.Config.Infer; export const domainsCreateCommand = Command.make("create", config).pipe( @@ -35,7 +43,12 @@ export const domainsCreateCommand = Command.make("create", config).pipe( }, ]), Command.withHandler((flags) => - domainsCreate(flags).pipe(withCommandTelemetry({ flags }), withJsonErrorHandling), + Option.isSome(flags.includeRawOutput) + ? rejectIncludeRawOutput(COMMAND_PATH, flags) + : domainsCreate(flags).pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(managementApiLayer), + ), ), - Command.provide(managementApiRuntimeLayer(["domains", "create"])), ); diff --git a/apps/cli/src/commands/domains/create/create.handler.ts b/apps/cli/src/commands/domains/create/create.handler.ts index 96d91eca98..1d58a47ddf 100644 --- a/apps/cli/src/commands/domains/create/create.handler.ts +++ b/apps/cli/src/commands/domains/create/create.handler.ts @@ -48,6 +48,6 @@ export const domainsCreate = Effect.fn("domains.create")(function* (flags: Domai ); yield* creating?.clear ?? Effect.void; - yield* emitHostnameResult(response, flags.includeRawOutput); + yield* emitHostnameResult(response); }).pipe(Effect.ensuring(linkedProjectCache.cache(ref)), Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/commands/domains/create/create.integration.test.ts b/apps/cli/src/commands/domains/create/create.integration.test.ts index 05d3711b23..350501aae8 100644 --- a/apps/cli/src/commands/domains/create/create.integration.test.ts +++ b/apps/cli/src/commands/domains/create/create.integration.test.ts @@ -92,11 +92,11 @@ function setup(opts: SetupOpts = {}) { return { layer, out, api, analytics, telemetry, linkedProjectCache }; } -function flags(over: Partial<{ includeRawOutput: boolean }> = {}) { +function flags() { return { projectRef: Option.none(), customHostname: CUSTOM_HOSTNAME, - includeRawOutput: over.includeRawOutput ?? false, + includeRawOutput: Option.none(), }; } @@ -235,14 +235,6 @@ describe("domains create integration", () => { }).pipe(Effect.provide(layer)); }); - it.live("forces JSON output when --include-raw-output is set", () => { - const { layer, out } = setup(); - return Effect.gen(function* () { - yield* domainsCreate(flags({ includeRawOutput: true })); - expect(out.stdoutText.startsWith("{")).toBe(true); - }).pipe(Effect.provide(layer)); - }); - it.live("emits a structured success object for --output-format json", () => { const { layer, out } = setup({ format: "json" }); return Effect.gen(function* () { diff --git a/apps/cli/src/commands/domains/delete/delete.command.ts b/apps/cli/src/commands/domains/delete/delete.command.ts index 0206785052..f1d3a551a0 100644 --- a/apps/cli/src/commands/domains/delete/delete.command.ts +++ b/apps/cli/src/commands/domains/delete/delete.command.ts @@ -1,8 +1,10 @@ +import { Effect, Option } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { managementApiRuntimeLayer } from "../../../command-internal/management-api-runtime.layer.ts"; +import { rejectIncludeRawOutput } from "../domains.removed-flag.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { domainsDelete } from "./delete.handler.ts"; @@ -11,12 +13,18 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error. includeRawOutput: Flag.Boolean("include-raw-output").pipe( - Flag.withDescription("(Deprecated) use -o json instead."), - Flag.withDefault(false), + Flag.withDescription("Removed: use -o json instead."), + Flag.optional, + Flag.withHidden, ), } as const; +const COMMAND_PATH = ["domains", "delete"]; +const managementApiLayer = managementApiRuntimeLayer(COMMAND_PATH); + export type DomainsDeleteFlags = CliCommand.Command.Config.Infer; export const domainsDeleteCommand = Command.make("delete", config).pipe( @@ -29,7 +37,12 @@ export const domainsDeleteCommand = Command.make("delete", config).pipe( }, ]), Command.withHandler((flags) => - domainsDelete(flags).pipe(withCommandTelemetry({ flags }), withJsonErrorHandling), + Option.isSome(flags.includeRawOutput) + ? rejectIncludeRawOutput(COMMAND_PATH, flags) + : domainsDelete(flags).pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(managementApiLayer), + ), ), - Command.provide(managementApiRuntimeLayer(["domains", "delete"])), ); diff --git a/apps/cli/src/commands/domains/delete/delete.handler.ts b/apps/cli/src/commands/domains/delete/delete.handler.ts index 0cab96bb18..7a779f49bb 100644 --- a/apps/cli/src/commands/domains/delete/delete.handler.ts +++ b/apps/cli/src/commands/domains/delete/delete.handler.ts @@ -12,9 +12,6 @@ const mapDeleteError = mapDomainsHttpError("delete"); const DELETE_SUCCESS_MESSAGE = "Deleted custom hostname config successfully."; -// `flags.includeRawOutput` is unread: `--include-raw-output` is a persistent flag on the -// `domains` group, so it's accepted on `delete` too, but ignored — delete has no response -// body to encode. Asserted by the "ignores --include-raw-output" integration test. export const domainsDelete = Effect.fn("domains.delete")(function* (flags: DomainsDeleteFlags) { const output = yield* Output; const api = yield* CommandPlatformApi; diff --git a/apps/cli/src/commands/domains/delete/delete.integration.test.ts b/apps/cli/src/commands/domains/delete/delete.integration.test.ts index 12ba00856c..0fb5ce3566 100644 --- a/apps/cli/src/commands/domains/delete/delete.integration.test.ts +++ b/apps/cli/src/commands/domains/delete/delete.integration.test.ts @@ -43,7 +43,7 @@ function setup(opts: SetupOpts = {}) { return { layer, out, api, telemetry, linkedProjectCache }; } -const baseFlags = { projectRef: Option.none(), includeRawOutput: false }; +const baseFlags = { projectRef: Option.none(), includeRawOutput: Option.none() }; describe("domains delete integration", () => { it.live("prints the success line to stderr in text mode", () => { @@ -84,15 +84,6 @@ describe("domains delete integration", () => { }).pipe(Effect.provide(layer)); }); - it.live("ignores --include-raw-output (inert on delete)", () => { - const { layer, out } = setup(); - return Effect.gen(function* () { - yield* domainsDelete({ projectRef: Option.none(), includeRawOutput: true }); - expect(out.stdoutText).toBe(""); - expect(out.stderrText).toBe("Deleted custom hostname config successfully.\n"); - }).pipe(Effect.provide(layer)); - }); - it.live("fails with DomainsUnexpectedStatusError on HTTP 503", () => { const { layer, telemetry, linkedProjectCache } = setup({ status: 503 }); return Effect.gen(function* () { diff --git a/apps/cli/src/commands/domains/domains.emit.ts b/apps/cli/src/commands/domains/domains.emit.ts index 1704b9cbe4..79e295ed3d 100644 --- a/apps/cli/src/commands/domains/domains.emit.ts +++ b/apps/cli/src/commands/domains/domains.emit.ts @@ -114,36 +114,28 @@ function terminateHumanStatus(status: string): string { * nothing goes to stdout. * - In a structured `-o` mode (`json`/`yaml`/`toml`/`env`) the encoded * response goes to **stdout** and the human status is suppressed. - * - `--include-raw-output` (deprecated) forces `-o` to `json` when unset or `pretty`. * - For `--output-format json|stream-json` (no `-o` flag), emits a single * structured `success` event and suppresses the stderr status. */ -export const emitHostnameResult = Effect.fnUntraced(function* ( - response: HostnameResponse, - includeRawOutput: boolean, -) { +export const emitHostnameResult = Effect.fnUntraced(function* (response: HostnameResponse) { const output = yield* Output; const outputFlag = yield* OutputFlag; const outputFlagFormat = Option.getOrUndefined(outputFlag); - const effectiveOutputFlagFormat = - includeRawOutput && (outputFlagFormat === undefined || outputFlagFormat === "pretty") - ? "json" - : outputFlagFormat; - if (effectiveOutputFlagFormat === "json") { + if (outputFlagFormat === "json") { yield* output.raw(encodeSortedJson(normalizeHostnameResponse(response))); return; } - if (effectiveOutputFlagFormat === "yaml") { + if (outputFlagFormat === "yaml") { yield* output.raw(encodeStructYaml(response, HOSTNAME_RESPONSE_SHAPE)); return; } - if (effectiveOutputFlagFormat === "toml") { + if (outputFlagFormat === "toml") { yield* output.raw(encodeStructToml(response, HOSTNAME_RESPONSE_SHAPE)); return; } - if (effectiveOutputFlagFormat === "env") { + if (outputFlagFormat === "env") { yield* output.raw(encodeEnv(normalizeHostnameResponse(response)) + "\n"); return; } diff --git a/apps/cli/src/commands/domains/domains.removed-flag.integration.test.ts b/apps/cli/src/commands/domains/domains.removed-flag.integration.test.ts new file mode 100644 index 0000000000..6e7fd73924 --- /dev/null +++ b/apps/cli/src/commands/domains/domains.removed-flag.integration.test.ts @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { Cause, Effect, Exit, Layer, Stdio } from "effect"; +import { CliOutput, Command } from "effect/cli"; + +import { AccessTokenRequiredError } from "../../auth/errors.ts"; +import { GLOBAL_FLAGS } from "../../command-internal/global-flags.ts"; +import { RemovedSurfaceError } from "../../command-internal/removed-command.ts"; +import { textCliOutputFormatter } from "../../shared/output/text-formatter.ts"; +import { + buildTestRuntime, + isolatedHomeLayer, + mockCommandPlatformApi, + mockCommandSettings, + useTempWorkdir, +} from "../../../tests/helpers/command-mocks.ts"; +import { + mockContextualAnalytics, + mockOutput, + mockTelemetryRuntime, +} from "../../../tests/helpers/mocks.ts"; +import { domainsCommand } from "./domains.command.ts"; + +const tempRoot = useTempWorkdir("supabase-domains-removed-flag-"); + +const testRoot = Command.make("supabase").pipe( + Command.withSubcommands([domainsCommand]), + Command.withGlobalFlags(GLOBAL_FLAGS), +); + +const PROJECT_REF = "abcdefghijklmnopqrst"; + +const SUBCOMMANDS = [ + ["create", "--custom-hostname", "example.com"], + ["get"], + ["reverify"], + ["activate"], + ["delete"], +] as const; + +function runDomains(args: ReadonlyArray, format: "text" | "json" = "text") { + const out = mockOutput({ format }); + const analytics = mockContextualAnalytics(); + const layer = Layer.mergeAll( + buildTestRuntime({ + out, + api: mockCommandPlatformApi(), + cliSettings: mockCommandSettings({ workdir: tempRoot.current }), + analytics, + runtimeInfo: isolatedHomeLayer(tempRoot.current, { + SUPABASE_HOME: tempRoot.current, + SUPABASE_NO_KEYRING: "1", + }), + }), + CliOutput.layer(textCliOutputFormatter()), + mockTelemetryRuntime({ configDir: `${tempRoot.current}/.supabase` }), + Stdio.layerTest({ args: Effect.succeed(args) }), + ); + return Command.runWith(testRoot, { version: "0.0.0-test" })(args).pipe( + Effect.exit, + Effect.provide(layer), + Effect.map((exit) => ({ exit, out, analytics })), + ); +} + +describe("domains --include-raw-output through command dispatch", () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new Error("unexpected network request")); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + }); + + it.live("needs credentials without the removed flag", () => + Effect.gen(function* () { + const { exit } = yield* runDomains(["domains", "get", "--project-ref", PROJECT_REF]); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + expect(Cause.squash(exit.cause)).toBeInstanceOf(AccessTokenRequiredError); + }), + ); + + for (const [subcommand, ...subArgs] of SUBCOMMANDS) { + for (const flag of ["--include-raw-output", "--include-raw-output=false"]) { + it.live(`domains ${subcommand} rejects ${flag} with the removal error`, () => + Effect.gen(function* () { + const { exit, analytics } = yield* runDomains([ + "domains", + subcommand, + ...subArgs, + "--project-ref", + PROJECT_REF, + flag, + ]); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) return; + const error = Cause.squash(exit.cause); + expect(error).toBeInstanceOf(RemovedSurfaceError); + if (!(error instanceof RemovedSurfaceError)) return; + expect(error.kind).toBe("flag"); + expect(error.message).toBe("--include-raw-output was removed."); + expect(error.suggestion).toBe("Use -o json instead."); + expect(fetchSpy).not.toHaveBeenCalled(); + + const events = analytics.captured.filter((c) => c.event === "cli_command_executed"); + expect(events).toHaveLength(1); + expect(events[0]?.properties).toMatchObject({ + command: `domains ${subcommand}`, + exit_code: 1, + error_fingerprint: "tag:RemovedSurfaceError:removed_flag", + }); + }), + ); + } + } + + it.live("reports the removal through the JSON error envelope", () => + Effect.gen(function* () { + const { exit, out } = yield* runDomains( + ["domains", "get", "--project-ref", PROJECT_REF, "--include-raw-output"], + "json", + ); + + expect(Exit.isSuccess(exit)).toBe(true); + expect(out.failures).toHaveLength(1); + expect(out.failures[0]).toMatchObject({ + code: "RemovedSurfaceError", + message: "--include-raw-output was removed.", + }); + }), + ); +}); diff --git a/apps/cli/src/commands/domains/domains.removed-flag.ts b/apps/cli/src/commands/domains/domains.removed-flag.ts new file mode 100644 index 0000000000..ded0156086 --- /dev/null +++ b/apps/cli/src/commands/domains/domains.removed-flag.ts @@ -0,0 +1,22 @@ +import { Effect, Layer } from "effect"; + +import { removedFlag } from "../../command-internal/removed-command.ts"; +import { withJsonErrorHandling } from "../../shared/output/json-error-handling.ts"; +import { commandRuntimeLayer } from "../../shared/runtime/command-runtime.layer.ts"; +import { withCommandTelemetry } from "../../telemetry/command-telemetry.ts"; +import { telemetryStateLayer } from "../../telemetry/telemetry-state.layer.ts"; + +/** + * Rejects the removed `--include-raw-output` on a credential-free runtime: + * `managementApiRuntimeLayer` resolves the access token eagerly, so building it first would fail + * with a login error before the removal message. + */ +export const rejectIncludeRawOutput = >( + command: ReadonlyArray, + flags: Flags, +) => + removedFlag("--include-raw-output", "Use -o json instead.").pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(Layer.mergeAll(commandRuntimeLayer(command), telemetryStateLayer)), + ); diff --git a/apps/cli/src/commands/domains/get/get.command.ts b/apps/cli/src/commands/domains/get/get.command.ts index 03d0170ef3..bbbc1d7ba3 100644 --- a/apps/cli/src/commands/domains/get/get.command.ts +++ b/apps/cli/src/commands/domains/get/get.command.ts @@ -1,8 +1,10 @@ +import { Effect, Option } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { managementApiRuntimeLayer } from "../../../command-internal/management-api-runtime.layer.ts"; +import { rejectIncludeRawOutput } from "../domains.removed-flag.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { domainsGet } from "./get.handler.ts"; @@ -11,12 +13,18 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error. includeRawOutput: Flag.Boolean("include-raw-output").pipe( - Flag.withDescription("(Deprecated) use -o json instead."), - Flag.withDefault(false), + Flag.withDescription("Removed: use -o json instead."), + Flag.optional, + Flag.withHidden, ), } as const; +const COMMAND_PATH = ["domains", "get"]; +const managementApiLayer = managementApiRuntimeLayer(COMMAND_PATH); + export type DomainsGetFlags = CliCommand.Command.Config.Infer; export const domainsGetCommand = Command.make("get", config).pipe( @@ -31,7 +39,12 @@ export const domainsGetCommand = Command.make("get", config).pipe( }, ]), Command.withHandler((flags) => - domainsGet(flags).pipe(withCommandTelemetry({ flags }), withJsonErrorHandling), + Option.isSome(flags.includeRawOutput) + ? rejectIncludeRawOutput(COMMAND_PATH, flags) + : domainsGet(flags).pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(managementApiLayer), + ), ), - Command.provide(managementApiRuntimeLayer(["domains", "get"])), ); diff --git a/apps/cli/src/commands/domains/get/get.handler.ts b/apps/cli/src/commands/domains/get/get.handler.ts index e1c1faa711..1d3fae0082 100644 --- a/apps/cli/src/commands/domains/get/get.handler.ts +++ b/apps/cli/src/commands/domains/get/get.handler.ts @@ -34,6 +34,6 @@ export const domainsGet = Effect.fn("domains.get")(function* (flags: DomainsGetF ); yield* fetching?.clear ?? Effect.void; - yield* emitHostnameResult(response, flags.includeRawOutput); + yield* emitHostnameResult(response); }).pipe(Effect.ensuring(linkedProjectCache.cache(ref)), Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/commands/domains/get/get.integration.test.ts b/apps/cli/src/commands/domains/get/get.integration.test.ts index 63d04aec17..87f83660a6 100644 --- a/apps/cli/src/commands/domains/get/get.integration.test.ts +++ b/apps/cli/src/commands/domains/get/get.integration.test.ts @@ -78,7 +78,7 @@ function setup(opts: SetupOpts = {}) { return { layer, out, api, analytics, telemetry, linkedProjectCache }; } -const baseFlags = { projectRef: Option.none(), includeRawOutput: false }; +const baseFlags = { projectRef: Option.none(), includeRawOutput: Option.none() }; describe("domains get integration", () => { it.live("prints the hostname status to stderr in text mode", () => { @@ -334,23 +334,6 @@ describe("domains get integration", () => { }).pipe(Effect.provide(layer)); }); - it.live("forces JSON output when --include-raw-output is set", () => { - const { layer, out } = setup(); - return Effect.gen(function* () { - yield* domainsGet({ projectRef: Option.none(), includeRawOutput: true }); - expect(out.stdoutText.startsWith("{")).toBe(true); - expect(out.stdoutText).toContain('"custom_hostname": "shop.acme.dev"'); - }).pipe(Effect.provide(layer)); - }); - - it.live("forces JSON even when -o is explicitly pretty and --include-raw-output is set", () => { - const { layer, out } = setup({ outputFlag: "pretty" }); - return Effect.gen(function* () { - yield* domainsGet({ projectRef: Option.none(), includeRawOutput: true }); - expect(out.stdoutText.startsWith("{")).toBe(true); - }).pipe(Effect.provide(layer)); - }); - it.live("fails with DomainsUnexpectedStatusError on HTTP 503", () => { const { layer, telemetry, linkedProjectCache } = setup({ status: 503 }); return Effect.gen(function* () { diff --git a/apps/cli/src/commands/domains/reverify/reverify.command.ts b/apps/cli/src/commands/domains/reverify/reverify.command.ts index 0565b0126f..0d26352068 100644 --- a/apps/cli/src/commands/domains/reverify/reverify.command.ts +++ b/apps/cli/src/commands/domains/reverify/reverify.command.ts @@ -1,8 +1,10 @@ +import { Effect, Option } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { managementApiRuntimeLayer } from "../../../command-internal/management-api-runtime.layer.ts"; +import { rejectIncludeRawOutput } from "../domains.removed-flag.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { domainsReverify } from "./reverify.handler.ts"; @@ -11,12 +13,18 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces the removal error instead of an + // unknown-flag parse error. includeRawOutput: Flag.Boolean("include-raw-output").pipe( - Flag.withDescription("(Deprecated) use -o json instead."), - Flag.withDefault(false), + Flag.withDescription("Removed: use -o json instead."), + Flag.optional, + Flag.withHidden, ), } as const; +const COMMAND_PATH = ["domains", "reverify"]; +const managementApiLayer = managementApiRuntimeLayer(COMMAND_PATH); + export type DomainsReverifyFlags = CliCommand.Command.Config.Infer; export const domainsReverifyCommand = Command.make("reverify", config).pipe( @@ -29,7 +37,12 @@ export const domainsReverifyCommand = Command.make("reverify", config).pipe( }, ]), Command.withHandler((flags) => - domainsReverify(flags).pipe(withCommandTelemetry({ flags }), withJsonErrorHandling), + Option.isSome(flags.includeRawOutput) + ? rejectIncludeRawOutput(COMMAND_PATH, flags) + : domainsReverify(flags).pipe( + withCommandTelemetry({ flags }), + withJsonErrorHandling, + Effect.provide(managementApiLayer), + ), ), - Command.provide(managementApiRuntimeLayer(["domains", "reverify"])), ); diff --git a/apps/cli/src/commands/domains/reverify/reverify.handler.ts b/apps/cli/src/commands/domains/reverify/reverify.handler.ts index 6b2e47c58d..6b2afd4b8e 100644 --- a/apps/cli/src/commands/domains/reverify/reverify.handler.ts +++ b/apps/cli/src/commands/domains/reverify/reverify.handler.ts @@ -32,6 +32,6 @@ export const domainsReverify = Effect.fn("domains.reverify")(function* ( ); yield* reverifying?.clear ?? Effect.void; - yield* emitHostnameResult(response, flags.includeRawOutput); + yield* emitHostnameResult(response); }).pipe(Effect.ensuring(linkedProjectCache.cache(ref)), Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/commands/domains/reverify/reverify.integration.test.ts b/apps/cli/src/commands/domains/reverify/reverify.integration.test.ts index 7f3d033e13..e1efe27a93 100644 --- a/apps/cli/src/commands/domains/reverify/reverify.integration.test.ts +++ b/apps/cli/src/commands/domains/reverify/reverify.integration.test.ts @@ -65,7 +65,7 @@ function setup(opts: SetupOpts = {}) { return { layer, out, api, analytics, telemetry, linkedProjectCache }; } -const baseFlags = { projectRef: Option.none(), includeRawOutput: false }; +const baseFlags = { projectRef: Option.none(), includeRawOutput: Option.none() }; describe("domains reverify integration", () => { it.live("suggests upgrade from entitlement_required envelope on 400", () => { @@ -126,14 +126,6 @@ describe("domains reverify integration", () => { }).pipe(Effect.provide(layer)); }); - it.live("forces JSON output when --include-raw-output is set", () => { - const { layer, out } = setup(); - return Effect.gen(function* () { - yield* domainsReverify({ projectRef: Option.none(), includeRawOutput: true }); - expect(out.stdoutText.startsWith("{")).toBe(true); - }).pipe(Effect.provide(layer)); - }); - it.live("fails with DomainsUnexpectedStatusError on HTTP 503", () => { const { layer, telemetry } = setup({ status: 503 }); return Effect.gen(function* () { diff --git a/apps/cli/src/commands/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/pull/SIDE_EFFECTS.md index 1acc1a7613..1a37c36f2b 100644 --- a/apps/cli/src/commands/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/pull/SIDE_EFFECTS.md @@ -314,10 +314,9 @@ pull`/`migration fetch`'s own internal resolution) — this doesn't matter in pr reused step's own prompt is unconditionally bypassed (`assumeYes: true`) once `pull`'s own confirmation is accepted. - **The db step always runs in migration mode when orchestrated by `pull`, never the declarative - path.** `pull` never sets `--declarative`/`--use-pg-delta`/`--diff-engine` on its own db-step - invocation, AND `pullDbStep` (`pull.steps.ts`) passes `forceMigrationMode: true` into `runDbPull` - (`command-internal/db-pull-run.ts`), which forces `experimental` to `false` unconditionally, - regardless of any ambient `--experimental` flag or `SUPABASE_EXPERIMENTAL` (shell or project - dotenv). The deprecated `--experimental`-without-`--declarative` structured-dump export therefore - can never engage when the db step runs through `pull` — see `db/pull/SIDE_EFFECTS.md`'s own notes - on that path, which still applies to the standalone `db pull` command. + path.** `pull` never sets `--declarative`/`--diff-engine` on its own db-step invocation and calls + `runDbPull` (`command-internal/db-pull-run.ts`), which never reads `--experimental` or + `SUPABASE_EXPERIMENTAL`. Only the standalone `db pull` command rejects an ambient experimental + gate without `--declarative` (see `db/pull/SIDE_EFFECTS.md`), so `pull` keeps working when it is + set. The retry hint for the db step ends in `--experimental=false` so that command does not hit + that rejection either. diff --git a/apps/cli/src/commands/pull/pull.aggregate.ts b/apps/cli/src/commands/pull/pull.aggregate.ts index 4dfa8f11ba..7e91d148a5 100644 --- a/apps/cli/src/commands/pull/pull.aggregate.ts +++ b/apps/cli/src/commands/pull/pull.aggregate.ts @@ -267,9 +267,8 @@ export function pullRetryHint( const commandByStep: Record = { config: `supabase config pull --project-ref ${ref}${remoteLabelFlag}`, migration_history: `supabase migration fetch --project-ref ${ref}`, - // `pullDbStep` runs with `forceMigrationMode: true`, overriding any ambient `--experimental` - // gate; `--experimental=false` reproduces that override so this suggested command performs - // the same operation the failed step did. + // `db pull` rejects an ambient `--experimental`/`SUPABASE_EXPERIMENTAL` without `--declarative`; + // `--experimental=false` keeps this suggested command from failing on it. db: `supabase db pull --project-ref ${ref} --experimental=false`, functions: `supabase functions download --project-ref ${ref}`, }; diff --git a/apps/cli/src/commands/pull/pull.aggregate.unit.test.ts b/apps/cli/src/commands/pull/pull.aggregate.unit.test.ts index 8ea3f6e3b1..b321a79df9 100644 --- a/apps/cli/src/commands/pull/pull.aggregate.unit.test.ts +++ b/apps/cli/src/commands/pull/pull.aggregate.unit.test.ts @@ -470,7 +470,7 @@ describe("pullRetryHint", () => { ); }); - it("names 'supabase db pull --project-ref --experimental=false' for the db step, forcing migration mode", () => { + it("names 'supabase db pull --project-ref --experimental=false' so the db step avoids the structured-dump rejection", () => { expect(pullRetryHint("db", ref, undefined)).toBe( "To retry just this step, run: supabase db pull --project-ref abcdefghijklmnopqrst --experimental=false", ); diff --git a/apps/cli/src/commands/pull/pull.integration.test.ts b/apps/cli/src/commands/pull/pull.integration.test.ts index dbf13f0d57..b69595d6d2 100644 --- a/apps/cli/src/commands/pull/pull.integration.test.ts +++ b/apps/cli/src/commands/pull/pull.integration.test.ts @@ -37,6 +37,7 @@ import { mockShadowContainerCliSpawner, mockTelemetryStateTracked, transportFailure, + withConfigEnv, shadowCacheDisabledLayer, useTempWorkdir, } from "../../../tests/helpers/command-mocks.ts"; @@ -602,7 +603,7 @@ interface SetupOpts { readonly confirmSideEffect?: Effect.Effect; /** Overrides `cliSettings.workdir` — defaults to the temp project root. */ readonly workdir?: string; - /** Overrides the ambient `--experimental`/`SUPABASE_EXPERIMENTAL` gate — defaults to `false`. */ + /** Overrides the global `--experimental` flag — defaults to `false`. */ readonly experimental?: boolean; } @@ -1365,31 +1366,30 @@ describe("pull integration", () => { ); it.live( - "the db step stays in migration mode with the ambient --experimental gate on, never taking the declarative export path", + "the db step pulls migrations when --experimental or SUPABASE_EXPERIMENTAL is set", () => { const { layer, out } = setup({ yes: true, experimental: true, remoteMigrations: [{ version: "20260101000000", name: "init", statements: ["select 1;"] }], - // A real diff, so the db step actually writes. If `pullDbStep` didn't forward - // `forceMigrationMode: true`, the ambient `--experimental` gate would route this to - // the declarative path instead, which the mocked `PgDeltaEngine.exportDeclarativeSchema` - // dies on. diffOutcome: () => ({ changes: true, files: [{ name: "pull", sql: "alter table foo add column bar text;" }], }), }); - return Effect.gen(function* () { - yield* writeConfig(); - yield* seedLocalMigration("20260101000000"); - const exit = yield* Effect.exit(runPull(pullFlags())); - expect(Exit.isSuccess(exit)).toBe(true); + return withConfigEnv( + { SUPABASE_EXPERIMENTAL: "true" }, + Effect.gen(function* () { + yield* writeConfig(); + yield* seedLocalMigration("20260101000000"); + const exit = yield* Effect.exit(runPull(pullFlags())); + expect(Exit.isSuccess(exit)).toBe(true); - expect((yield* listMigrations).length).toBeGreaterThan(1); - expect(yield* projectFileExists("supabase", "schemas")).toBe(false); - expect(stepLine(out!.stdoutText, "db")).toContain("changed"); - }).pipe(Effect.provide(layer)); + expect((yield* listMigrations).length).toBeGreaterThan(1); + expect(yield* projectFileExists("supabase", "schemas")).toBe(false); + expect(stepLine(out!.stdoutText, "db")).toContain("changed"); + }).pipe(Effect.provide(layer)), + ); }, ); diff --git a/apps/cli/src/commands/pull/pull.steps.ts b/apps/cli/src/commands/pull/pull.steps.ts index c9ad374423..36583c8448 100644 --- a/apps/cli/src/commands/pull/pull.steps.ts +++ b/apps/cli/src/commands/pull/pull.steps.ts @@ -14,7 +14,7 @@ import { type ConfigPullRunPlan, type ConfigPullSource, } from "../../command-internal/config-pull-run.ts"; -import { runDbPull, type DbPullFlags } from "../../command-internal/db-pull-run.ts"; +import { runDbPull, type DbPullRunFlags } from "../../command-internal/db-pull-run.ts"; import { runMigrationFetch, type MigrationFetchFlags, @@ -83,17 +83,14 @@ export const pullMigrationHistoryStep = Effect.fnUntraced(function* (context: Pu /** * `db` step: pulls the linked project's schema in migration mode, targeting `context.ref` * directly and suppressing `db pull`'s remote-history-update prompt with `assumeYes`. - * `forceMigrationMode: true` prevents an ambient `--experimental` gate from silently switching to - * the declarative export path, which `pull`'s dirty-check and confirmation don't guard against. * `DbPullInSyncError` is caught and reported as `in_sync`, a finding rather than a failure at the * `pull` level (ADR 0024). */ export const pullDbStep = Effect.fnUntraced(function* (context: PullStepContext) { const cliSettings = yield* CommandSettings; - const flags: DbPullFlags = { + const flags: DbPullRunFlags = { name: Option.none(), declarative: Option.none(), - usePgDelta: Option.none(), diffEngine: Option.none(), strictCoverage: false, schema: [], @@ -103,10 +100,7 @@ export const pullDbStep = Effect.fnUntraced(function* (context: PullStepContext) projectRef: Option.some(context.ref), password: Option.none(), }; - return yield* runDbPull(flags, { - assumeYes: context.assumeYes, - forceMigrationMode: true, - }).pipe( + return yield* runDbPull(flags, { assumeYes: context.assumeYes }).pipe( Effect.map((outcome): PullDbStepOutcome => ({ kind: "applied", outcome, diff --git a/apps/cli/src/docs/docs-spec.tables.ts b/apps/cli/src/docs/docs-spec.tables.ts index 43495fb3e0..1c60dac12b 100644 --- a/apps/cli/src/docs/docs-spec.tables.ts +++ b/apps/cli/src/docs/docs-spec.tables.ts @@ -215,18 +215,10 @@ export const DOCS_CHOICE_OVERRIDES: Readonly "`. - * `--include-raw-output` stays parse-visible for compatibility (see - * `commands/domains/SIDE_EFFECTS.md`) but is hidden from docs here. Validated at build time - * like the other tables. + * Flags excluded from the published reference, keyed `" "`. Validated at build + * time like the other tables. */ -export const DOCS_EXCLUDED_FLAGS: ReadonlySet = new Set([ - "supabase-domains-activate include-raw-output", - "supabase-domains-create include-raw-output", - "supabase-domains-delete include-raw-output", - "supabase-domains-get include-raw-output", - "supabase-domains-reverify include-raw-output", -]); +export const DOCS_EXCLUDED_FLAGS: ReadonlySet = new Set([]); /** * Usage argument overrides, keyed by doc id, for commands whose published argument rendering diff --git a/apps/cli/src/docs/docs-spec.unit.test.ts b/apps/cli/src/docs/docs-spec.unit.test.ts index 4fe786976c..655aaa5cc2 100644 --- a/apps/cli/src/docs/docs-spec.unit.test.ts +++ b/apps/cli/src/docs/docs-spec.unit.test.ts @@ -290,7 +290,7 @@ describe("buildDocsSpec", () => { }); describe("review fixes stay fixed", () => { - it("excludes the deprecated --include-raw-output from every domains page", () => { + it("does not publish the removed --include-raw-output on any domains page", () => { const { spec, byId } = builtSpec(); for (const command of spec.commands) { expect( From 8a043225ca98eb006e7267e6d20f7f6b5f58c086 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Tue, 13 Oct 2026 16:42:02 +0100 Subject: [PATCH 2/3] fix(cli): tighten db pull removal-guard controls and reuse loaded project env (CLI-2596) --- apps/cli/src/command-internal/db-pull-run.ts | 5 ++- apps/cli/src/commands/db/pull/SIDE_EFFECTS.md | 2 +- apps/cli/src/commands/db/pull/pull.handler.ts | 5 +-- .../pull.removed-flag.integration.test.ts | 33 +++++++++++-------- apps/cli/src/commands/pull/SIDE_EFFECTS.md | 8 ++--- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/apps/cli/src/command-internal/db-pull-run.ts b/apps/cli/src/command-internal/db-pull-run.ts index 8d6d59863e..3dc830dc87 100644 --- a/apps/cli/src/command-internal/db-pull-run.ts +++ b/apps/cli/src/command-internal/db-pull-run.ts @@ -98,6 +98,8 @@ export type DbPullInvoke = { * `undefined` keeps the existing resolution. */ readonly assumeYes?: boolean; + /** Project `.env*` values the caller already loaded; `undefined` loads them here. */ + readonly projectEnv?: Record; }; export type DbPullOutcome = @@ -139,7 +141,8 @@ export const runDbPull = Effect.fn("db.pull.run")(function* ( // `--yes` or `SUPABASE_YES`. The project `.env` is loaded before the migration // history prompt, so a `SUPABASE_YES` set only in `supabase/.env` auto-confirms // the native initial-migra history repair too. - const projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); + const projectEnv = + invoke?.projectEnv ?? (yield* loadProjectEnvValues(fs, path, cliSettings.workdir)); const yes = yield* resolveYesWithProjectEnv(projectEnv); let linkedRefForCache: string | undefined; diff --git a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md index 7e230d60bb..3b9967c6b7 100644 --- a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md @@ -220,4 +220,4 @@ Both fail with `RemovedSurfaceError` (exit 1). Telemetry records `cli_command_ex --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull." `--experimental --declarative` runs the declarative export without an error, and an explicit `--experimental=false` beats the env. The check lives in the `db pull` command handler, not in - the shared pull runner, so `supabase pull` is unaffected by `SUPABASE_EXPERIMENTAL`. + the shared pull runner, so `supabase pull` does not reject `SUPABASE_EXPERIMENTAL`. diff --git a/apps/cli/src/commands/db/pull/pull.handler.ts b/apps/cli/src/commands/db/pull/pull.handler.ts index c075c36bb8..44c41abfeb 100644 --- a/apps/cli/src/commands/db/pull/pull.handler.ts +++ b/apps/cli/src/commands/db/pull/pull.handler.ts @@ -20,11 +20,12 @@ const EXPERIMENTAL_SUGGESTION = export const dbPull = Effect.fn("db.pull")(function* (flags: DbPullFlags) { // Checked here rather than in `runDbPull` so the `pull` orchestrator, which calls `runDbPull` // directly, keeps working when `SUPABASE_EXPERIMENTAL` is set. + let projectEnv: Record | undefined; if (!Option.getOrElse(flags.declarative, () => false)) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; const cliSettings = yield* CommandSettings; - const projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); + projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); if (yield* resolveExperimentalWithProjectEnv(projectEnv)) { return yield* removedFlag( "The --experimental structured-dump mode of db pull", @@ -33,7 +34,7 @@ export const dbPull = Effect.fn("db.pull")(function* (flags: DbPullFlags) { } } const output = yield* Output; - const outcome = yield* runDbPull(flags); + const outcome = yield* runDbPull(flags, { projectEnv }); yield* Effect.annotateCurrentSpan({ "db.pull.outcome": outcome.kind, diff --git a/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts index 3a0a26d367..b5351f2bc6 100644 --- a/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts +++ b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts @@ -5,6 +5,7 @@ import { CliOutput, Command } from "effect/cli"; import { GLOBAL_FLAGS } from "../../../command-internal/global-flags.ts"; import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; +import { PROJECT_NOT_LINKED_MESSAGE } from "../../../config/project-ref.service.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; import { @@ -80,6 +81,20 @@ const removedError = (exit: Exit.Exit) => { return error as RemovedSurfaceError; }; +const passedRemovalGuard = (exit: Exit.Exit) => { + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) throw new Error("expected failure"); + const error = Cause.squash(exit.cause); + expect(error).not.toBeInstanceOf(RemovedSurfaceError); + return error; +}; + +const expectNotLinked = (exit: Exit.Exit) => + expect(passedRemovalGuard(exit)).toMatchObject({ + _tag: "ProjectRefNotLinkedError", + message: PROJECT_NOT_LINKED_MESSAGE, + }); + describe("db pull removed surfaces through command dispatch", () => { let fetchSpy: ReturnType; @@ -129,13 +144,11 @@ describe("db pull removed surfaces through command dispatch", () => { }), ); - it.live("a missing profile still fails plain db pull with a different error", () => + it.live("a missing profile fails plain db pull with a profile load error", () => Effect.gen(function* () { const { exit } = yield* runDbPull([], { env: { SUPABASE_PROFILE: "missing-profile" } }); - expect(Exit.isFailure(exit)).toBe(true); - if (!Exit.isFailure(exit)) return; - expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + expect(passedRemovalGuard(exit)).toMatchObject({ _tag: "ProfileLoadError" }); }), ); @@ -143,9 +156,7 @@ describe("db pull removed surfaces through command dispatch", () => { Effect.gen(function* () { const { exit } = yield* runDbPull([]); - expect(Exit.isFailure(exit)).toBe(true); - if (!Exit.isFailure(exit)) return; - expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + expectNotLinked(exit); }), ); @@ -155,9 +166,7 @@ describe("db pull removed surfaces through command dispatch", () => { env: { SUPABASE_EXPERIMENTAL: "true" }, }); - expect(Exit.isFailure(exit)).toBe(true); - if (!Exit.isFailure(exit)) return; - expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + expectNotLinked(exit); }), ); @@ -165,9 +174,7 @@ describe("db pull removed surfaces through command dispatch", () => { Effect.gen(function* () { const { exit } = yield* runDbPull(["--experimental", "--declarative"]); - expect(Exit.isFailure(exit)).toBe(true); - if (!Exit.isFailure(exit)) return; - expect(Cause.squash(exit.cause)).not.toBeInstanceOf(RemovedSurfaceError); + expectNotLinked(exit); }), ); diff --git a/apps/cli/src/commands/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/pull/SIDE_EFFECTS.md index 1a37c36f2b..547e86dfd3 100644 --- a/apps/cli/src/commands/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/pull/SIDE_EFFECTS.md @@ -315,8 +315,8 @@ pull`/`migration fetch`'s own internal resolution) — this doesn't matter in pr confirmation is accepted. - **The db step always runs in migration mode when orchestrated by `pull`, never the declarative path.** `pull` never sets `--declarative`/`--diff-engine` on its own db-step invocation and calls - `runDbPull` (`command-internal/db-pull-run.ts`), which never reads `--experimental` or - `SUPABASE_EXPERIMENTAL`. Only the standalone `db pull` command rejects an ambient experimental - gate without `--declarative` (see `db/pull/SIDE_EFFECTS.md`), so `pull` keeps working when it is - set. The retry hint for the db step ends in `--experimental=false` so that command does not hit + `runDbPull` (`command-internal/db-pull-run.ts`), where the experimental gate no longer selects + the pull mode (its local-container prelude still resolves the gate for the local stack). Only the + standalone `db pull` command rejects an ambient experimental gate without `--declarative` (see + `db/pull/SIDE_EFFECTS.md`), so `pull` keeps working when it is set. The retry hint for the db step ends in `--experimental=false` so that command does not hit that rejection either. From eefb6cee194655629cb5945b4f4500a5d5094ea9 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Thu, 15 Oct 2026 10:31:33 +0100 Subject: [PATCH 3/3] fix(cli): reject db pull --experimental before loading the profile (CLI-2596) --- apps/cli/src/commands/db/pull/SIDE_EFFECTS.md | 6 +- apps/cli/src/commands/db/pull/pull.command.ts | 48 +++++--- apps/cli/src/commands/db/pull/pull.handler.ts | 58 +++++---- .../commands/db/pull/pull.integration.test.ts | 110 ------------------ .../pull.removed-flag.integration.test.ts | 49 ++++++++ 5 files changed, 122 insertions(+), 149 deletions(-) diff --git a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md index 60cf26db7b..06db3472da 100644 --- a/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/pull/SIDE_EFFECTS.md @@ -215,9 +215,9 @@ Both fail with `RemovedSurfaceError` (exit 1). Telemetry records `cli_command_ex `--diff-engine migra`, `[experimental.pgdelta] enabled = false` or its env var. - The `--experimental` structured-dump mode is the global `--experimental` flag, `SUPABASE_EXPERIMENTAL` in the shell, or `SUPABASE_EXPERIMENTAL` in the project `.env` resolving - true (flag > shell env > project `.env`) without `--declarative`. It is rejected after the - profile and project `.env*` load, so a malformed `.env` fails first, but before any config, - database or network work. Message: "The --experimental structured-dump mode of db pull was + true (flag > shell env > project `.env`) without `--declarative`. It is rejected + before the profile or credentials load, after only the project `.env*` read from the workdir, so a + malformed `.env` fails first, but before any config, database or network work. Message: "The --experimental structured-dump mode of db pull was removed." Suggestion: "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull." `--experimental --declarative` runs the declarative export without an error, and an explicit diff --git a/apps/cli/src/commands/db/pull/pull.command.ts b/apps/cli/src/commands/db/pull/pull.command.ts index f084fafddc..e1899f77d7 100644 --- a/apps/cli/src/commands/db/pull/pull.command.ts +++ b/apps/cli/src/commands/db/pull/pull.command.ts @@ -18,7 +18,7 @@ import { telemetryStateLayer } from "../../../telemetry/telemetry-state.layer.ts import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { parseSchemaFlags } from "../../../command-internal/schema-flags.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; -import { dbPull } from "./pull.handler.ts"; +import { dbPull, resolveRemovedExperimental } from "./pull.handler.ts"; import { dbPullRuntimeLayer } from "./pull.layers.ts"; import { cliConfigValuesLayer } from "../../../config/cli-config-values.layer.ts"; @@ -93,6 +93,9 @@ export type DbPullFlags = CliCommand.Command.Config.Infer; const DECLARATIVE_SUGGESTION = "Use --declarative to pull the declarative schema. The diff engine is chosen with --diff-engine or [experimental.pgdelta] enabled in config.toml."; +const EXPERIMENTAL_SUGGESTION = + "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; + // Rejected on a credential-free runtime: `dbPullRuntimeLayer` loads the profile and command // settings eagerly, so building it first could fail before the removal message. const removedFlagLayer = Layer.mergeAll(commandRuntimeLayer(["db", "pull"]), telemetryStateLayer); @@ -141,19 +144,36 @@ export const dbPullCommand = Command.make("pull", config).pipe( { default: "linked" }, ), ), - Command.withHandler((flags) => - Option.isSome(flags.usePgDelta) - ? removedFlag("--use-pg-delta", DECLARATIVE_SUGGESTION).pipe( - withCommandTelemetry(telemetryOptions(flags)), - withJsonErrorHandling, - Effect.provide(removedFlagLayer), - ) - : dbPull(flags).pipe( - withCommandTelemetry(telemetryOptions(flags)), - withJsonErrorHandling, - Effect.provide(dbPullRuntimeLayer), - ), - ), + Command.withHandler((flags) => { + const rejectRemoved = (rejection: Effect.Effect) => + rejection.pipe( + withCommandTelemetry(telemetryOptions(flags)), + withJsonErrorHandling, + Effect.provide(removedFlagLayer), + ); + + if (Option.isSome(flags.usePgDelta)) { + return rejectRemoved(removedFlag("--use-pg-delta", DECLARATIVE_SUGGESTION)); + } + return resolveRemovedExperimental(flags).pipe( + Effect.matchEffect({ + onFailure: (error) => rejectRemoved(Effect.fail(error)), + onSuccess: ({ experimental, projectEnv }) => + experimental + ? rejectRemoved( + removedFlag( + "The --experimental structured-dump mode of db pull", + EXPERIMENTAL_SUGGESTION, + ), + ) + : dbPull(flags, projectEnv).pipe( + withCommandTelemetry(telemetryOptions(flags)), + withJsonErrorHandling, + Effect.provide(dbPullRuntimeLayer), + ), + }), + ); + }), Command.provide(cliConfigValuesLayer), withCliConfigFlags(config), ); diff --git a/apps/cli/src/commands/db/pull/pull.handler.ts b/apps/cli/src/commands/db/pull/pull.handler.ts index 44c41abfeb..21ec74ba35 100644 --- a/apps/cli/src/commands/db/pull/pull.handler.ts +++ b/apps/cli/src/commands/db/pull/pull.handler.ts @@ -1,38 +1,52 @@ -import { Effect, FileSystem, Option, Path } from "effect"; +import { Config, ConfigProvider, Effect, FileSystem, Option, Path } from "effect"; import { aqua } from "../../../command-internal/colors.ts"; import { loadProjectEnvValues } from "../../../command-internal/db-config.toml-read.ts"; -import { resolveExperimentalWithProjectEnv } from "../../../command-internal/global-flags.ts"; -import { removedFlag } from "../../../command-internal/removed-command.ts"; -import { CommandSettings } from "../../../config/command-settings.service.ts"; +import { + resolveExperimentalWithProjectEnv, + WorkdirFlag, +} from "../../../command-internal/global-flags.ts"; +import { resolveWorkdir } from "../../../config/command-settings.layer.ts"; import { Output } from "../../../shared/output/output.service.ts"; +import { RuntimeInfo } from "../../../shared/runtime/runtime-info.service.ts"; import { runDbPull } from "../../../command-internal/db-pull-run.ts"; import type { DbPullFlags } from "./pull.command.ts"; -const EXPERIMENTAL_SUGGESTION = - "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; +/** + * Resolves the removed `--experimental` structured-dump mode and the project `.env` values from + * the same workdir `CommandSettings` computes, without loading the profile. Skipped under + * `--declarative`. Kept out of `runDbPull` so `supabase pull` ignores `SUPABASE_EXPERIMENTAL`. + */ +export const resolveRemovedExperimental = Effect.fn("db.pull.resolveRemovedExperimental")( + function* (flags: DbPullFlags) { + if (Option.getOrElse(flags.declarative, () => false)) { + return { experimental: false, projectEnv: undefined }; + } + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const runtimeInfo = yield* RuntimeInfo; + const provider = yield* ConfigProvider.ConfigProvider; + const workdirEnvValue = yield* Config.option(Config.String("SUPABASE_WORKDIR")).parse(provider); + const { workdir } = yield* resolveWorkdir( + yield* WorkdirFlag, + workdirEnvValue, + runtimeInfo.cwd, + path, + ); + const projectEnv = yield* loadProjectEnvValues(fs, path, workdir); + return { experimental: yield* resolveExperimentalWithProjectEnv(projectEnv), projectEnv }; + }, +); /** * `dbPull`'s established external behavior (stdout/stderr, JSON payload, * "Finished" line), reimplemented on top of {@link runDbPull}: run the * pull, then emit based on the returned outcome. */ -export const dbPull = Effect.fn("db.pull")(function* (flags: DbPullFlags) { - // Checked here rather than in `runDbPull` so the `pull` orchestrator, which calls `runDbPull` - // directly, keeps working when `SUPABASE_EXPERIMENTAL` is set. - let projectEnv: Record | undefined; - if (!Option.getOrElse(flags.declarative, () => false)) { - const fs = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - const cliSettings = yield* CommandSettings; - projectEnv = yield* loadProjectEnvValues(fs, path, cliSettings.workdir); - if (yield* resolveExperimentalWithProjectEnv(projectEnv)) { - return yield* removedFlag( - "The --experimental structured-dump mode of db pull", - EXPERIMENTAL_SUGGESTION, - ); - } - } +export const dbPull = Effect.fn("db.pull")(function* ( + flags: DbPullFlags, + projectEnv?: Record, +) { const output = yield* Output; const outcome = yield* runDbPull(flags, { projectEnv }); diff --git a/apps/cli/src/commands/db/pull/pull.integration.test.ts b/apps/cli/src/commands/db/pull/pull.integration.test.ts index 4008ea6fc5..6a3b7652e3 100644 --- a/apps/cli/src/commands/db/pull/pull.integration.test.ts +++ b/apps/cli/src/commands/db/pull/pull.integration.test.ts @@ -93,10 +93,6 @@ import { PgDeltaSslProbe } from "../../../command-internal/pgdelta-ssl-probe.ser import { PgDeltaEngine, PgDeltaEngineError } from "../shared/pgdelta-engine.service.ts"; import type { DbPullFlags } from "./pull.command.ts"; import { dbPull } from "./pull.handler.ts"; -import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; - -const EXPERIMENTAL_SUGGESTION = - "Use --declarative to pull per-object schema files, or pass --experimental=false (or unset SUPABASE_EXPERIMENTAL) for a migration pull."; const alwaysReadyHttpClientLayer = Layer.succeed( HttpClient.HttpClient, @@ -1725,66 +1721,6 @@ describe("db pull", () => { }, ); - const expectNothingRan = (s: ReturnType) => { - expect(s.resolveCalls).toEqual([]); - expect(s.connectedDatabases).toEqual([]); - expect(s.engineCalls).toEqual([]); - expect(s.edgeCalls).toEqual([]); - expect(s.shadowSpawned).toEqual([]); - expect(s.cache.cached).toBe(false); - }; - - it.effect("rejects the global --experimental flag without --declarative", () => { - const s = setup(tmp.current, { - experimental: true, - edgeStdout: EXPORT_JSON, - args: ["db", "pull", "--experimental"], - }); - return Effect.gen(function* () { - const error = yield* dbPull(flags()).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expect(error).toMatchObject({ - kind: "flag", - message: "The --experimental structured-dump mode of db pull was removed.", - suggestion: EXPERIMENTAL_SUGGESTION, - }); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer)); - }); - - it.effect("rejects SUPABASE_EXPERIMENTAL without --declarative", () => { - const s = setup(tmp.current, { edgeStdout: EXPORT_JSON }); - return Effect.gen(function* () { - const error = yield* dbPull(flags({ diffEngine: Option.some("migra") })).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expect(error).toMatchObject({ suggestion: EXPERIMENTAL_SUGGESTION }); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer), (body) => - withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), - ); - }); - - it.effect("rejects SUPABASE_EXPERIMENTAL set only in supabase/.env", () => { - const s = setup(tmp.current, { - edgeStdout: EXPORT_JSON, - files: { "supabase/.env": "SUPABASE_EXPERIMENTAL=true\n" }, - }); - return Effect.gen(function* () { - const error = yield* dbPull(flags()).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer), withEmptyConfigEnv); - }); - - it.effect("rejects --experimental with --declarative=false", () => { - const s = setup(tmp.current, { experimental: true, args: ["db", "pull", "--experimental"] }); - return Effect.gen(function* () { - const error = yield* dbPull(flags({ declarative: Option.some(false) })).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer)); - }); - it.effect("--experimental --declarative runs the declarative export without an error", () => { const s = setup(tmp.current, { experimental: true, @@ -1820,52 +1756,6 @@ describe("db pull", () => { }, ); - it.effect( - "a migration name literally '--experimental=false' after -- does not suppress SUPABASE_EXPERIMENTAL", - () => { - const s = setup(tmp.current, { args: ["db", "pull", "--", "--experimental=false"] }); - return Effect.gen(function* () { - const error = yield* dbPull(flags({ name: Option.some("--experimental=false") })).pipe( - Effect.flip, - ); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer), (body) => - withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), - ); - }, - ); - - it.effect( - "a repeated --experimental=false --experimental=true is rejected (last one wins)", - () => { - const s = setup(tmp.current, { - args: ["db", "pull", "--experimental=false", "--experimental=true"], - }); - return Effect.gen(function* () { - const error = yield* dbPull(flags()).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect( - "a bare --password consumes the following token, so SUPABASE_EXPERIMENTAL is still rejected", - () => { - const s = setup(tmp.current, { - args: ["db", "pull", "--password", "--experimental=false"], - }); - return Effect.gen(function* () { - const error = yield* dbPull(flags()).pipe(Effect.flip); - expect(error).toBeInstanceOf(RemovedSurfaceError); - expectNothingRan(s); - }).pipe(Effect.provide(s.layer), (body) => - withConfigEnv({ SUPABASE_EXPERIMENTAL: "true" }, body), - ); - }, - ); - it.effect( "a stale SUPABASE_EXPERIMENTAL_PG_DELTA in supabase/.env does not defeat enabled = false", () => { diff --git a/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts index b5351f2bc6..9f641222c2 100644 --- a/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts +++ b/apps/cli/src/commands/db/pull/pull.removed-flag.integration.test.ts @@ -144,6 +144,27 @@ describe("db pull removed surfaces through command dispatch", () => { }), ); + it.live("rejects --experimental with a missing profile and no credentials", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--experimental"], { + env: { SUPABASE_PROFILE: "missing-profile" }, + }); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + + it.live("rejects SUPABASE_EXPERIMENTAL set only in supabase/.env with a missing profile", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull([], { + env: { SUPABASE_PROFILE: "missing-profile" }, + files: { "supabase/.env": "SUPABASE_EXPERIMENTAL=1\n" }, + }); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + it.live("a missing profile fails plain db pull with a profile load error", () => Effect.gen(function* () { const { exit } = yield* runDbPull([], { env: { SUPABASE_PROFILE: "missing-profile" } }); @@ -223,6 +244,34 @@ describe("db pull removed surfaces through command dispatch", () => { ); } + it.live( + "a migration name literally '--experimental=false' after -- does not suppress the env", + () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--", "--experimental=false"], { + env: { SUPABASE_EXPERIMENTAL: "true" }, + }); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + + it.live("rejects a repeated --experimental=false --experimental=true (last one wins)", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--experimental=false", "--experimental=true"]); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + + it.live("rejects --experimental with --declarative=false", () => + Effect.gen(function* () { + const { exit } = yield* runDbPull(["--experimental", "--declarative=false"]); + + expect(removedError(exit).message).toBe(EXPERIMENTAL_MESSAGE); + }), + ); + it.live("rejects SUPABASE_EXPERIMENTAL set only in supabase/.env", () => Effect.gen(function* () { const { exit } = yield* runDbPull([], {