From a302c60984ea0ad700e6535377b9230b02940676 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 14 Sep 2026 17:07:21 +0100 Subject: [PATCH 1/2] feat(cli)!: remove the residual Go delegation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the last delegation paths that spawned the bundled `supabase-go` sidecar binary and deletes the `GoProxy` service that wired them up. Removed: - `db branch create|delete|list|switch` — local database branches (a container-cloning feature distinct from hosted preview branches) are no longer supported. Use `supabase branches --help` for hosted preview branches. - `db remote changes` — use `supabase db diff --linked` instead. - `gen keys` — use `supabase projects api-keys --project-ref ` to read a project's API keys. - `db diff --use-pg-schema` — deprecated since CLI-1960; removed in favor of the default migra engine or `--use-pg-delta`. - `functions download --legacy-bundle` (hidden flag) — retry with `functions download --use-api `, or redeploy with a current CLI if the Function predates 1.120.0. Each removed path fails with exit code 1 and a suggestion naming its replacement, and is measurable via a new `RemovedSurfaceError` telemetry fingerprint, rather than a bare unknown-command/unknown-flag parse error. Linear: CLI-2432 Co-Authored-By: Claude Sonnet 5 --- .github/workflows/test.yml | 15 - apps/cli-e2e/.env.example | 6 - apps/cli-e2e/AGENTS.md | 29 - .../src/tests/go-binary-surface.e2e.test.ts | 210 ------- apps/cli/AGENTS.md | 9 - apps/cli/README.md | 22 +- apps/cli/docs/binary-distribution.md | 47 +- apps/cli/docs/go-cli-porting-status.md | 59 -- apps/cli/docs/go-cli-reference.md | 2 +- apps/cli/src/SIDE_EFFECTS_TEMPLATE.md | 6 - apps/cli/src/cli/root.ts | 58 +- apps/cli/src/command-internal/diff-engine.ts | 10 +- .../command-internal/diff-engine.unit.test.ts | 5 - .../command-internal/go-child-exit.error.ts | 27 - .../command-internal/go-proxy-invocation.ts | 21 - .../src/command-internal/go-proxy.layer.ts | 282 --------- .../go-proxy.layer.unit.test.ts | 571 ------------------ .../src/command-internal/go-proxy.service.ts | 44 -- .../src/command-internal/removed-command.ts | 55 ++ .../removed-command.unit.test.ts | 50 ++ apps/cli/src/command-internal/schema-flags.ts | 17 +- .../src/command-internal/upgrade-notice.ts | 35 +- .../upgrade-notice.unit.test.ts | 15 +- .../src/commands/db/branch/SIDE_EFFECTS.md | 69 +++ .../src/commands/db/branch/branch.command.ts | 49 +- .../src/commands/db/branch/branch.e2e.test.ts | 23 + .../commands/db/branch/create/SIDE_EFFECTS.md | 53 -- .../db/branch/create/create.command.ts | 17 - .../db/branch/create/create.handler.ts | 9 - .../commands/db/branch/delete/SIDE_EFFECTS.md | 52 -- .../db/branch/delete/delete.command.ts | 17 - .../db/branch/delete/delete.handler.ts | 9 - .../commands/db/branch/list/SIDE_EFFECTS.md | 51 -- .../commands/db/branch/list/list.command.ts | 13 - .../commands/db/branch/list/list.handler.ts | 8 - .../commands/db/branch/switch/SIDE_EFFECTS.md | 52 -- .../db/branch/switch/switch.command.ts | 17 - .../db/branch/switch/switch.handler.ts | 9 - apps/cli/src/commands/db/diff/SIDE_EFFECTS.md | 70 +-- apps/cli/src/commands/db/diff/diff.command.ts | 11 +- apps/cli/src/commands/db/diff/diff.errors.ts | 5 +- apps/cli/src/commands/db/diff/diff.handler.ts | 89 +-- .../commands/db/diff/diff.integration.test.ts | 177 ++---- .../commands/db/pull/pull.integration.test.ts | 34 -- .../db/remote/changes/SIDE_EFFECTS.md | 48 +- .../db/remote/changes/changes.command.ts | 20 +- .../db/remote/changes/changes.handler.ts | 17 - .../generate/generate.integration.test.ts | 8 - .../commands/db/test/test.integration.test.ts | 8 +- .../functions/download/SIDE_EFFECTS.md | 81 +-- .../functions/download/download.e2e.test.ts | 6 - .../functions/download/download.handler.ts | 32 +- .../download/download.integration.test.ts | 316 +++------- apps/cli/src/commands/gen/gen.command.ts | 2 +- .../cli/src/commands/gen/keys/SIDE_EFFECTS.md | 50 +- .../cli/src/commands/gen/keys/keys.command.ts | 19 +- .../cli/src/commands/gen/keys/keys.handler.ts | 13 - apps/cli/src/commands/pull/pull.steps.ts | 11 +- apps/cli/src/docs/docs-spec.tables.ts | 1 - .../src/shared/cli/hidden-flag.unit.test.ts | 81 +-- apps/cli/src/shared/cli/run.ts | 23 +- apps/cli/src/shared/cli/run.unit.test.ts | 40 +- apps/cli/src/shared/functions/deploy.ts | 7 +- apps/cli/src/shared/functions/download.ts | 114 +--- .../src/shared/functions/functions.shared.ts | 11 +- .../src/shared/output/json-error-handling.ts | 2 +- .../output/json-error-handling.unit.test.ts | 19 +- .../telemetry/__fixtures__/error-tags.txt | 2 +- .../shared/telemetry/error-actionability.ts | 9 + ...t-completion-and-go-cli-authority-scope.md | 8 +- 70 files changed, 739 insertions(+), 2638 deletions(-) delete mode 100644 apps/cli-e2e/src/tests/go-binary-surface.e2e.test.ts delete mode 100644 apps/cli/docs/go-cli-porting-status.md delete mode 100644 apps/cli/src/command-internal/go-child-exit.error.ts delete mode 100644 apps/cli/src/command-internal/go-proxy-invocation.ts delete mode 100644 apps/cli/src/command-internal/go-proxy.layer.ts delete mode 100644 apps/cli/src/command-internal/go-proxy.layer.unit.test.ts delete mode 100644 apps/cli/src/command-internal/go-proxy.service.ts create mode 100644 apps/cli/src/command-internal/removed-command.ts create mode 100644 apps/cli/src/command-internal/removed-command.unit.test.ts create mode 100644 apps/cli/src/commands/db/branch/SIDE_EFFECTS.md create mode 100644 apps/cli/src/commands/db/branch/branch.e2e.test.ts delete mode 100644 apps/cli/src/commands/db/branch/create/SIDE_EFFECTS.md delete mode 100644 apps/cli/src/commands/db/branch/create/create.command.ts delete mode 100644 apps/cli/src/commands/db/branch/create/create.handler.ts delete mode 100644 apps/cli/src/commands/db/branch/delete/SIDE_EFFECTS.md delete mode 100644 apps/cli/src/commands/db/branch/delete/delete.command.ts delete mode 100644 apps/cli/src/commands/db/branch/delete/delete.handler.ts delete mode 100644 apps/cli/src/commands/db/branch/list/SIDE_EFFECTS.md delete mode 100644 apps/cli/src/commands/db/branch/list/list.command.ts delete mode 100644 apps/cli/src/commands/db/branch/list/list.handler.ts delete mode 100644 apps/cli/src/commands/db/branch/switch/SIDE_EFFECTS.md delete mode 100644 apps/cli/src/commands/db/branch/switch/switch.command.ts delete mode 100644 apps/cli/src/commands/db/branch/switch/switch.handler.ts delete mode 100644 apps/cli/src/commands/db/remote/changes/changes.handler.ts delete mode 100644 apps/cli/src/commands/gen/keys/keys.handler.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5b482ac2fe..226797cdf9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -202,19 +202,6 @@ jobs: with: dependency-firewall-token: ${{ secrets.DF_FIREWALL_TOKEN }} - - name: Cache Go CLI binary - id: cache-go-binary - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: apps/cli-go/supabase-go - key: go-cli-${{ runner.os }}-${{ hashFiles('apps/cli-go/**/*.go', - 'apps/cli-go/go.mod', 'apps/cli-go/go.sum') }} - - - name: Build Go CLI - if: steps.cache-go-binary.outputs.cache-hit != 'true' - run: go build -o supabase-go . - working-directory: apps/cli-go - # The e2e harness invokes `node apps/cli/dist/supabase.js` with # `SUPABASE_CLI_BINARY_OVERRIDE` pointing at the compiled binary in # `apps/cli/dist/`. Build the CLI explicitly before invoking every @@ -224,8 +211,6 @@ jobs: - name: Run end-to-end tests run: pnpm exec turbo run test:e2e:run --only --concurrency=1 --filter=supabase --filter=@supabase/cli-e2e -- --shard=${{ matrix.shard }}/3 - env: - SUPABASE_GO_BINARY: ${{ github.workspace }}/apps/cli-go/supabase-go test-stack-e2e: if: | diff --git a/apps/cli-e2e/.env.example b/apps/cli-e2e/.env.example index d437835f33..fb502b3f75 100644 --- a/apps/cli-e2e/.env.example +++ b/apps/cli-e2e/.env.example @@ -7,12 +7,6 @@ CLI_E2E_MODE=record # Staging Management API token. Required in record mode. SUPABASE_ACCESS_TOKEN=sbp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -# The CLI shells out to the bundled Go binary for the proxied commands -# (db diff --use-pg-schema, db branch *, db remote changes, gen keys, -# functions download --legacy-bundle). Point at a freshly built binary: -# cd apps/cli-go && go build -o /tmp/supabase-test-binary . -SUPABASE_GO_BINARY=/tmp/supabase-test-binary - # --- Optional record overrides (sensible defaults in src/tests/env.ts) --- # Management API base (also accepted as SUPABASE_STAGING_URL). # CLI_E2E_API_URL=https://api.supabase.green diff --git a/apps/cli-e2e/AGENTS.md b/apps/cli-e2e/AGENTS.md index 0e6c6ad342..5182804955 100644 --- a/apps/cli-e2e/AGENTS.md +++ b/apps/cli-e2e/AGENTS.md @@ -180,32 +180,3 @@ per-process, so each shard still has deterministic intra-shard ordering. is a single-job operation; parallel shards would race on the shared `fixtures/recorded/` directory. The `record` script does not accept `--shard`. - -## Go binary version requirement - -The CLI proxies a fixed, small set of commands to a Go binary (`SUPABASE_GO_BINARY` → bundled package binary → system `supabase`) — as of CLI-1970, `apps/cli-go/` contains only that residual proxied subset, nothing else, and it is slated for cleanup and removal (the surface only shrinks; never add tests that grow it). If your system `supabase` binary predates a flag or subcommand change on one of these, `testBehaviour` tests for it will fail with "unknown command" or "unknown flag". - -Build the Go CLI from source and point `SUPABASE_GO_BINARY` at it: - -```sh -(cd apps/cli-go && go build -o /tmp/supabase-test-binary .) - -# Replay -SUPABASE_GO_BINARY=/tmp/supabase-test-binary \ - pnpm exec turbo run @supabase/cli-e2e#test:e2e:run - -# Record -SUPABASE_GO_BINARY=/tmp/supabase-test-binary \ - SUPABASE_ACCESS_TOKEN=sbp_... SUPABASE_STAGING_URL=https://api.supabase.green \ - pnpm run record -``` - -`SUPABASE_GO_BINARY` is inherited by the CLI subprocess via `exec()` in the harness, so you only need to set it once in the shell. - -Commands currently requiring this — the full proxied surface, nothing else needs a Go binary at all: - -- `db diff` (for `--use-pg-schema`) -- `db branch create`, `db branch delete`, `db branch list`, `db branch switch` -- `db remote changes` -- `gen keys` -- `functions download` (for the hidden `--legacy-bundle` flag) diff --git a/apps/cli-e2e/src/tests/go-binary-surface.e2e.test.ts b/apps/cli-e2e/src/tests/go-binary-surface.e2e.test.ts deleted file mode 100644 index fc0b9acfe0..0000000000 --- a/apps/cli-e2e/src/tests/go-binary-surface.e2e.test.ts +++ /dev/null @@ -1,210 +0,0 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { afterAll, beforeAll, describe, expect, test } from "vitest"; - -// The bundled `supabase-go` binary retains only the commands the TypeScript -// CLI's `GoProxy` can spawn; every other Go command was deleted. TS -// integration tests stub that subprocess boundary, so nothing else notices if -// a still-reachable Go command gets trimmed away by mistake. -// -// This suite enumerates every argv shape the TS side can hand to `GoProxy` -// and asserts the built `supabase-go` binary still resolves it. If this -// fails, either the TS spawn surface grew (add the command to the retained -// set in `apps/cli-go`) or the trim cut too deep (restore the command). -// -// `SUPABASE_GO_BINARY` is only set to a freshly built binary in CI (see -// `.github/workflows/test.yml`); locally this whole suite no-ops. -const GO_BINARY = process.env["SUPABASE_GO_BINARY"]; - -describe.skipIf(GO_BINARY === undefined)("go binary spawn surface (CLI-1970)", () => { - const binary = GO_BINARY as string; - - let workspaceDir: string; - let bogusProfilePath: string; - - beforeAll(() => { - workspaceDir = mkdtempSync(join(tmpdir(), "cli-e2e-go-binary-surface-")); - - // The upgrade check hits the real GitHub releases API on every invocation - // that returns a nil error (e.g. every `--help` call below), unless a - // `supabase/.temp/cli-latest` cache file already exists and is less than - // 10h old. Pre-seeding it here keeps this suite hermetic. - mkdirSync(join(workspaceDir, "supabase", ".temp"), { recursive: true }); - writeFileSync(join(workspaceDir, "supabase", ".temp", "cli-latest"), "v0.0.0"); - - // A bogus `--profile` for the two Management-API-gated delegates (`gen - // keys`, `functions download --legacy-bundle`): the unique profile name - // can never match a real stored credential, and the unreachable api_url - // means even a stray match still fails at connect. - bogusProfilePath = join(workspaceDir, "profile.yaml"); - writeFileSync( - bogusProfilePath, - [ - "name: cli-e2e-go-binary-surface-guard", - 'api_url: "http://127.0.0.1:1"', - 'dashboard_url: "http://127.0.0.1:1"', - "project_host: localhost", - ].join("\n"), - ); - }); - - afterAll(() => { - rmSync(workspaceDir, { recursive: true, force: true }); - }); - - function runGo(args: ReadonlyArray, envOverrides: Record = {}) { - const result = Bun.spawnSync([binary, ...args], { - cwd: workspaceDir, - env: { - PATH: process.env["PATH"] ?? "", - HOME: workspaceDir, - SUPABASE_HOME: workspaceDir, - // Belt-and-braces: no command exercised here should ever reach a - // real Docker daemon or send real telemetry. - DOCKER_HOST: "tcp://127.0.0.1:1", - SUPABASE_TELEMETRY_DISABLED: "1", - ...envOverrides, - }, - stdout: "pipe", - stderr: "pipe", - }); - return { - exitCode: result.exitCode, - stdout: result.stdout.toString(), - stderr: result.stderr.toString(), - }; - } - - // The complete spawn surface, mirrored from `GoProxy` call sites: - // - db diff (diff.handler.ts, `--use-pg-schema` delegate path) - // - db branch create|delete|list|switch (thin proxies) - // - db remote changes (thin proxy) - // - gen keys (keys.handler.ts) - // - functions download (shared/functions/download.ts, `--legacy-bundle`) - const RETAINED_COMMAND_PATHS: ReadonlyArray> = [ - ["db", "diff"], - ["db", "branch", "create"], - ["db", "branch", "delete"], - ["db", "branch", "list"], - ["db", "branch", "switch"], - ["db", "remote", "changes"], - ["gen", "keys"], - ["functions", "download"], - ]; - - describe("resolves every retained command path", () => { - for (const path of RETAINED_COMMAND_PATHS) { - test(`supabase-go ${path.join(" ")} --help exits 0`, () => { - const { exitCode, stdout, stderr } = runGo([...path, "--help"]); - const output = stdout + stderr; - expect(exitCode).toBe(0); - expect(output).not.toContain("unknown command"); - // A deleted nested subcommand doesn't error "unknown command": cobra - // only rejects unmatched args on the root command, so `db diff --help` - // with `diff` deleted would fall through to `db`'s own help (exit 0). - // Asserting the full command path appears in the usage output closes - // that gap — cobra only prints it once the whole path has resolved. - expect(output).toContain(`supabase ${path.join(" ")}`); - }, 5_000); - } - }); - - describe("accepts the exact argv shape the TS proxy builds", () => { - // `db diff` always provisions a Docker shadow database first, so the bogus - // DOCKER_HOST is what makes this fail fast, not the bogus --db-url. - test("db diff --use-pg-schema", () => { - const { exitCode, stderr } = runGo([ - "db", - "diff", - "--use-pg-schema", - "--db-url", - "postgresql://u:p@127.0.0.1:1/x", - "--schema", - "public", - ]); - expect(exitCode).toBe(1); - expect(stderr).not.toMatch(/unknown flag|invalid argument/i); - }, 5_000); - - // The only invocation in this suite exercising the full global-flag set at - // once. Also provisions a Docker shadow first, so DOCKER_HOST trips it. - test("db remote changes (full global flag set)", () => { - const { exitCode, stderr } = runGo([ - "--output", - "json", - "--profile", - "supabase-staging", - "--debug", - "--workdir", - workspaceDir, - "--experimental", - "--network-id", - "cli-e2e-test-net", - "--yes", - "--dns-resolver", - "https", - "--create-ticket", - "--agent", - "no", - "db", - "remote", - "changes", - "--db-url", - "postgresql://u:p@127.0.0.1:1/x", - "--schema", - "public", - ]); - expect(exitCode).toBe(1); - expect(stderr).not.toMatch(/unknown flag|invalid argument/i); - }, 5_000); - - // Gated behind a login check before any network call, so an isolated - // SUPABASE_HOME plus the bogus --profile fails fast without reaching a real API. - test("gen keys", () => { - const { exitCode, stderr } = runGo([ - "gen", - "keys", - "--project-ref", - "abcdefghijklmnopqrst", - "--override-name", - "db.host=CUSTOM_DB_HOST", - "--experimental", - "--profile", - bogusProfilePath, - ]); - expect(exitCode).toBe(1); - expect(stderr).not.toMatch(/unknown flag|invalid argument/i); - }, 5_000); - - // Same login-gate fail-fast as `gen keys`. - test("functions download --legacy-bundle", () => { - const { exitCode, stderr } = runGo([ - "functions", - "download", - "my-function", - "--project-ref", - "abcdefghijklmnopqrst", - "--legacy-bundle", - "--profile", - bogusProfilePath, - ]); - expect(exitCode).toBe(1); - expect(stderr).not.toMatch(/unknown flag|invalid argument/i); - }, 5_000); - }); - - describe("negative control: a deleted command still reports unknown", () => { - // `["db", "start"]` isn't used here even though it's a deleted command: - // like the nested-subcommand case above, `db` would fall through to its - // own help (exit 0) instead of erroring. Only a deleted top-level command - // reliably reproduces "unknown command". - for (const deletedCommand of ["inspect", "start"]) { - test(`supabase-go ${deletedCommand} reports unknown command`, () => { - const { exitCode, stderr } = runGo([deletedCommand]); - expect(exitCode).toBe(1); - expect(stderr).toContain("unknown command"); - }, 5_000); - } - }); -}); diff --git a/apps/cli/AGENTS.md b/apps/cli/AGENTS.md index 6d5f42ccbd..c3ebad2de9 100644 --- a/apps/cli/AGENTS.md +++ b/apps/cli/AGENTS.md @@ -99,15 +99,6 @@ compatibility decision in the PR. For the Compute transition, no local compatibi migrations are required. Compute error tags intentionally start new fingerprints; server-owned contracts remain unchanged. Update tests, generated schemas, and side-effect documentation. -## Go delegation - -- TypeScript owns CLI behavior. Consult `apps/cli-go` only for existing delegated - operations listed in [the delegation document](docs/go-cli-porting-status.md); do not expand delegation. -- Native replacements must preserve the public command, flags, output, side effects, - and exit behavior. Update the delegation document when removing a Go dependency. -- Emit command telemetry exactly once: bare proxies rely on Go telemetry; - instrumented TypeScript handlers suppress child telemetry. - ## Telemetry > The string passed to `Data.TaggedError("...")` is the PostHog `error_fingerprint` identity diff --git a/apps/cli/README.md b/apps/cli/README.md index 25f8a27552..05d003ad24 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -16,7 +16,6 @@ This workspace contains the stable, shipped `supabase` CLI. Earlier revisions ca For current migration/parity status, see: -- [`docs/go-cli-porting-status.md`](./docs/go-cli-porting-status.md) — the residual Go delegation surface - [`docs/go-cli-divergences.md`](./docs/go-cli-divergences.md) — TS-only flags and behavioral divergences from the old Go CLI For the generated command/reference docs, see: @@ -42,9 +41,10 @@ Examples: pnpm dev -- hello ``` -### CLI and the Go binary +### Running from source -Phase 0 commands in the CLI proxy to the Go CLI binary. To run these commands from source you need `supabase` (the Go CLI) available on your PATH. +No command in the CLI proxies to the Go CLI binary; `apps/cli-go/` builds and ships alongside +`supabase` but nothing in `apps/cli/src` spawns it (CLI-2432). For convenience, create a shell alias instead of using `pnpm dev` directly. For example in `.zshrc`: @@ -52,19 +52,6 @@ For convenience, create a shell alias instead of using `pnpm dev` directly. For alias supabase-dev="bun /absolute/path/to/dx-lab/apps/cli/src/main.ts" ``` -Then Phase 0 commands resolve the Go binary via PATH automatically: - -```sh -supabase-dev orgs list # proxied to supabase on PATH -supabase-dev login # native TypeScript -``` - -You can also point `SUPABASE_GO_BINARY` at a specific binary to skip the PATH lookup: - -```sh -export SUPABASE_GO_BINARY=/path/to/supabase -``` - ## Build There are two separate build paths depending on what you need. @@ -168,7 +155,8 @@ Platform-specific packages live under: Each platform package ships two binaries for the stable channel: - `bin/supabase` — the compiled TypeScript SFE (Bun single-file executable) -- `bin/supabase-go` — the compiled Go CLI binary, used by Phase 0 proxy commands +- `bin/supabase-go` — the compiled Go CLI binary, unused by `bin/supabase` (see + [`docs/binary-distribution.md`](./docs/binary-distribution.md)) The Go binary is compiled from `apps/cli-go/` at release time. Run `pnpm repos:install` after a fresh clone to make that source available. diff --git a/apps/cli/docs/binary-distribution.md b/apps/cli/docs/binary-distribution.md index dce49f57a9..db7a8baae3 100644 --- a/apps/cli/docs/binary-distribution.md +++ b/apps/cli/docs/binary-distribution.md @@ -10,7 +10,7 @@ The CLI is distributed as a set of platform-specific npm packages. Each platform @supabase/cli-darwin-arm64/ └── bin/ ├── supabase ← TypeScript CLI (Bun single-file executable) - └── supabase-go ← Go CLI binary (residual proxy commands only) + └── supabase-go ← Go CLI binary (unused by supabase; ships until apps/cli-go/ is deleted) ``` The base `supabase` package routes to the correct platform package via `src/shared/cli/bin.ts`, which resolves and `execFileSync`s the platform-specific `bin/supabase` binary. @@ -22,7 +22,13 @@ The CLI was built as a gradual TypeScript port of the Go CLI, moving each comman - **Phase 0** — The command is defined in the TS CLI tree but proxied to the Go binary at runtime via `GoProxy`. - **Phase 1+** — The command is implemented natively in TypeScript. -That port is complete (CLI-1970). `supabase-go` is the residual proxy target for a fixed, small command surface: `db diff` (for `--use-pg-schema`), the Go-deprecated `db branch`/`db remote changes` command families, `gen keys`, and `functions download` (for the hidden `--legacy-bundle` path). See "Go binary command surface" under Release Workflow below for the full list and why each command stays. Two lifecycles apply here and must not be conflated: the **public commands** in that surface are retained (dropping them was ruled a breaking change — CLI-1964), while their **Go implementations** are slated for eventual native TS replacement, after which `supabase-go` stops shipping. Until the proxied surface is empty, the TS binary (`supabase`) still needs `supabase-go` available on the same system for those invocations. Every other Go command from the original CLI has been deleted outright from `apps/cli-go/`, not merely excluded from the build — see the same section for how. +That port is complete (CLI-1970), and the last residual delegation surface — `db diff` +(`--use-pg-schema`), the `db branch`/`db remote changes` command families, `gen keys`, and +`functions download` (`--legacy-bundle`) — was removed rather than ported (CLI-2432): those +command paths are now tombstones (or, for `--legacy-bundle`, deleted outright) that never spawn +`supabase-go`. The TS binary (`supabase`) no longer needs `supabase-go` on the same system for +anything. `apps/cli-go/` and `supabase-go` still build and ship alongside `supabase` today, unused, +until the tree itself is deleted in a follow-up. ## Package Layout @@ -42,13 +48,8 @@ The musl packages only carry the Bun TS binary (compiled for musl). The Go binar ## Runtime Resolution -When a Phase 0 command runs, `go-proxy.layer.ts` resolves the Go binary in this order: - -1. **`SUPABASE_GO_BINARY` env var** — explicit override, takes priority. -2. **Co-located `supabase-go`** — looks next to `process.execPath`. Works in compiled SFE mode because the base shim uses `execFileSync`, making the TS SFE the main process with `process.execPath` pointing to itself. This is also how PATH-installed setups resolve: `supabase-go` ships next to the `supabase` shim. -3. **npm package resolution** — resolves `@supabase/cli-{platform}/bin/supabase-go`. Works when running from source with the platform packages installed. - -There is deliberately **no** `PATH` fallback (CLI-1488): the `supabase` shim itself is what's on `PATH`, so falling back to it would re-invoke the shim and fork-bomb. Resolution failure is a hard error with install guidance. +Nothing in the TypeScript CLI resolves or spawns `supabase-go` anymore (CLI-2432 removed the last +callers); `SUPABASE_GO_BINARY` is no longer read anywhere in `apps/cli/src`. ## Source of the Go Binary @@ -62,21 +63,18 @@ This must be run after a fresh clone before building a release. ## Development Workflow -No build step is required to run the CLI from source, but the Go binary must be resolvable — easiest via `SUPABASE_GO_BINARY` (below) or an installed `@supabase/cli-` package. +No build step is required to run the CLI from source, and no Go binary needs to be resolvable. -1. Build the Go binary once: `cd apps/cli-go && go build -o supabase-go .` -2. Create a shell alias to run the CLI from source. For example in `.zshrc`: +1. Create a shell alias to run the CLI from source. For example in `.zshrc`: ```sh alias supabase-dev="bun /path/to/dx-lab/apps/cli/src/main.ts" ``` -3. Point `SUPABASE_GO_BINARY` at the built binary and run commands: +2. Run commands directly: ```sh - export SUPABASE_GO_BINARY=/path/to/apps/cli-go/supabase-go - supabase-dev db branch list # proxied to the Go binary - supabase-dev login # native TypeScript implementation + supabase-dev login ``` ## Release Workflow @@ -97,15 +95,12 @@ This: ### Go binary command surface -`supabase-go` does not ship the full old Go CLI — only the fixed subset the TypeScript CLI still proxies to via `GoProxy`: - -- `db diff` — kept for `--use-pg-schema`, which wraps the in-process `stripe/pg-schema-diff` Go library with no TS/container equivalent (CLI-1960) -- `db branch create`, `db branch delete`, `db branch list`, `db branch switch` -- `db remote changes` -- `gen keys` — the public command is kept (its planned removal, CLI-1964, was cancelled as a breaking change); the Go implementation stays only until a native TS replacement lands -- `functions download` — kept for the hidden `--legacy-bundle` path (CLI-1963) - -That list is exhaustive: `GoProxy` is the only code path in the TypeScript CLI that spawns `supabase-go`. The one historical exception — the hidden pg-delta seam (`db schema declarative __catalog` + `db start`), spawned directly by the native `db schema declarative generate|sync` commands to provision shadow databases and export pg-delta catalogs — was ported to native TypeScript as part of CLI-1970 (`pgdelta.seam.layer.ts` now composes `setupShadowDatabase`/`legacyExportCatalogPgDelta`/`startLocalDatabase` in-process), and those two Go commands were deleted with it. +As of CLI-2432, nothing in the TypeScript CLI spawns `supabase-go` — the last delegation surface +(`db diff --use-pg-schema`, `db branch create|delete|list|switch`, `db remote changes`, `gen keys`, +`functions download --legacy-bundle`) was removed rather than ported: the first four command paths +are tombstones that fail with a removal error, and `--legacy-bundle` was deleted outright. `apps/cli-go/` +and `supabase-go` are unused dead weight in the shipped packages until the tree itself is deleted +in a follow-up. Everything else the original Go CLI implemented was deleted outright from `apps/cli-go/` (CLI-1970), not just excluded from the shipped binary. The reachable set was computed with a `go list -deps -test` fixpoint from the trimmed `main` package, and everything outside it was removed: the main module's first-party package count went from 138 to 38 (100 packages / ~29.5k LOC across 321 files deleted), including every other command's `cmd/*.go` file, `internal/{inspect,storage,sso,login,link,init,bootstrap}`, `internal/migration/{squash,up,fetch}`, the Go docs generator (`docs/`), `examples/`, and `tools/{jsonschema,shared}`. Counting stdlib and third-party dependencies too, the full dependency closure shrank from 1078 to 959 packages. `pkg/` (a separate, independently tagged/published Go module for external consumers) and `tools/listdep` (used by `cli-go-mirror.yml`) were left untouched. @@ -128,7 +123,7 @@ Measured on the CLI-1970 branch with the real release build (`build.ts`, `go bui Progression across both trims: the original two-binary baseline was 97–103 MB per platform; CLI-1966's `internal/start` deletion cut it to roughly 47–52 MB; CLI-1970 brings it to 39.1–42.8 MB. -The remaining size is dominated by dependencies the retained commands still need: `stripe/pg-schema-diff` (`db diff --use-pg-schema`), the Docker client (shadow-database provisioning for diff), pgx, the Management API client, cobra/viper, and sentry/posthog. +The remaining size is dominated by dependencies the now-unreachable command set still links against: `stripe/pg-schema-diff`, the Docker client (shadow-database provisioning), pgx, the Management API client, cobra/viper, and sentry/posthog. Release archive sizes (TS `supabase` binary + `supabase-go` together): `.tar.gz` 37.9–52.9 MB, `.zip` (Windows) 50.0–53.0 MB, `.deb` 52.7–53.4 MB, `.rpm` 52.3–53.2 MB, `.apk` 52.8–54.1 MB. diff --git a/apps/cli/docs/go-cli-porting-status.md b/apps/cli/docs/go-cli-porting-status.md deleted file mode 100644 index 60d7ab2215..0000000000 --- a/apps/cli/docs/go-cli-porting-status.md +++ /dev/null @@ -1,59 +0,0 @@ -# Go CLI Delegation - -> Still named `go-cli-porting-status.md` for historical reasons — the per-command porting tracker -> this file once held is gone now that the port is done; it documents only the residual Go -> delegation surface below. - -The Go→TypeScript port is complete (CLI-1970). The bundled `supabase-go` binary and the -`apps/cli-go/` tree contain **only** the delegation surface in the table below — Go source for -every other command was deleted outright once nothing in the TypeScript CLI could reach it, -directly or indirectly. For any other command's former Go source, the reference is the last commit -with it intact: `7b469f5b3` (CLI-1966's `internal/start` pin remains its own, separate commit, -`a253ccba2`). - -See [`binary-distribution.md`](./binary-distribution.md) for how these two binaries are packaged, -resolved at runtime, and sized. The TypeScript CLI is the source of truth for all CLI behavior; -`apps/cli-go/` is authoritative only for the proxied commands below, and the whole surface is -slated for cleanup and removal ([ADR 0016](../../../docs/adr/0016-port-completion-and-go-cli-authority-scope.md) -records the earlier transition policy). - -## The delegation surface - -| Command/path | TS proxy site | Go implementation (in-tree) | Why it stays | -| -------------------------------------------------- | ------------------------------------------------------------------------------------------------- | --------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | -| `db diff --use-pg-schema` | [`src/commands/db/diff/diff.handler.ts`](../src/commands/db/diff/diff.handler.ts) (delegate path) | `internal/db/diff/pgschema.go` | Wraps the Go-only `stripe/pg-schema-diff` library (CLI-1960); deprecated but sanctioned. | -| `db branch create\|delete\|list\|switch` | `src/commands/db/branch/*/` | `legacy/branch/*` | Go-deprecated wrapped commands kept indefinitely (CLI-1964 cancelled: dropping them was ruled a breaking change not worth shipping). | -| `db remote changes` | `src/commands/db/remote/changes/` | inline in `cmd/db.go` over `internal/db/diff` | Same CLI-1964 ruling. `db remote commit` is native `db pull`. | -| `gen keys` | `src/commands/gen/keys/` | `legacy/keys` | Same ruling; requires `--experimental`. | -| `functions download --legacy-bundle` (hidden flag) | `src/shared/functions/download.ts` `makeGoProxyLegacyBundleArgs` | `internal/functions/download` | Legacy Deno bundle extraction (CLI-1963). | - -## Mechanics - -All delegation goes through the shared `GoProxy` service -([`src/command-internal/go-proxy.service.ts`](../src/command-internal/go-proxy.service.ts), -[`go-proxy.layer.ts`](../src/command-internal/go-proxy.layer.ts)): - -- **Binary resolution order**: `SUPABASE_GO_BINARY` env var → binary co-located with the compiled - shim → the platform's `@supabase/cli-` npm package. There is deliberately **no** - `PATH` fallback (CLI-1488: the shim itself is what's on `PATH`, so falling back would re-invoke - it and fork-bomb) — resolution failure is a hard error with install guidance. PATH-installed - setups work via the co-location step: `supabase-go` sits next to the `supabase` shim. -- **Global-flag forwarding**: `cli/root.ts` translates the CLI's global flags - (`--output`, `--profile`, `--debug`, `--workdir`, `--experimental`, `--network-id`, `--yes`, - `--dns-resolver`, `--create-ticket`, `--agent`) into Go-style argv ahead of every proxied - invocation. -- **Child telemetry suppression**: a proxy handler that itself owns the parent - `cli_command_executed` event (wrapped in TS command instrumentation) suppresses the child's own - telemetry; a bare pass-through proxy leaves it enabled so the Go child stays the sole emitter. - -The spawn surface is guarded by -[`apps/cli-e2e/src/tests/go-binary-surface.e2e.test.ts`](../../cli-e2e/src/tests/go-binary-surface.e2e.test.ts) — -trimming the binary past this surface fails CI. - -The former pg-delta `__catalog`/`db start` seam (spawned directly by `db schema declarative -generate|sync`, outside `GoProxy`) was ported native in CLI-1970 and no longer exists. - -## See also - -TS-only flags and deliberate behavioral divergences from the old Go CLI live in -[`go-cli-divergences.md`](./go-cli-divergences.md). diff --git a/apps/cli/docs/go-cli-reference.md b/apps/cli/docs/go-cli-reference.md index e557eefadf..b38e5665b1 100644 --- a/apps/cli/docs/go-cli-reference.md +++ b/apps/cli/docs/go-cli-reference.md @@ -1,7 +1,7 @@ # Old Go CLI Reference > Complete help output for the old Go-based `supabase` CLI. -> Use this document as the raw parity reference for the residual Go delegation surface in [`go-cli-porting-status.md`](./go-cli-porting-status.md) and the TS divergences tracked in [`go-cli-divergences.md`](./go-cli-divergences.md). +> Use this document as the raw parity reference for the TS divergences tracked in [`go-cli-divergences.md`](./go-cli-divergences.md). ## Global Flags diff --git a/apps/cli/src/SIDE_EFFECTS_TEMPLATE.md b/apps/cli/src/SIDE_EFFECTS_TEMPLATE.md index 4bc1f85932..cfb21c9fa4 100644 --- a/apps/cli/src/SIDE_EFFECTS_TEMPLATE.md +++ b/apps/cli/src/SIDE_EFFECTS_TEMPLATE.md @@ -77,12 +77,6 @@ ## Telemetry Events Fired | Event | When | Notable properties / groups | diff --git a/apps/cli/src/cli/root.ts b/apps/cli/src/cli/root.ts index 3b201f12b6..eb5f0b2937 100644 --- a/apps/cli/src/cli/root.ts +++ b/apps/cli/src/cli/root.ts @@ -53,26 +53,13 @@ import { whoamiCommand } from "../commands/whoami/whoami.command.ts"; import { OutputFormatFlag } from "../shared/cli/global-flags.ts"; import { outputLayerFor } from "../shared/output/output.layer.ts"; import { quietProgressTextOutputLayer } from "../output/quiet-progress-text-output.layer.ts"; -import { makeGoProxyLayer } from "../command-internal/go-proxy.layer.ts"; import { AiTool } from "../shared/telemetry/ai-tool.service.ts"; import { aiToolLayer } from "../shared/telemetry/ai-tool.layer.ts"; import { CliArgs } from "../shared/cli/cli-args.service.ts"; import { commandRuntimeLayer } from "../shared/runtime/command-runtime.layer.ts"; import type { CliRootCommand } from "../shared/cli/run.ts"; import { isBuiltInTextRequest, resolveAgentOutputFormat } from "../shared/cli/agent-output.ts"; -import { - GLOBAL_FLAGS, - AgentFlag, - CreateTicketFlag, - DebugFlag, - DnsResolverFlag, - ExperimentalFlag, - NetworkIdFlag, - OutputFlag, - ProfileFlag, - WorkdirFlag, - YesFlag, -} from "../command-internal/global-flags.ts"; +import { GLOBAL_FLAGS, AgentFlag, OutputFlag } from "../command-internal/global-flags.ts"; const stackStartAliasCommand = stackStartCommand.pipe( Command.provide(commandRuntimeLayer(["start"])), @@ -143,15 +130,7 @@ export const rootCommandForFeatures = ( Layer.unwrap( Effect.gen(function* () { const explicitOutputFormat = yield* OutputFormatFlag; - const goOutput = yield* OutputFlag; - const profile = yield* ProfileFlag; - const debug = yield* DebugFlag; - const workdir = yield* WorkdirFlag; - const experimental = yield* ExperimentalFlag; - const networkId = yield* NetworkIdFlag; - const yes = yield* YesFlag; - const dnsResolver = yield* DnsResolverFlag; - const createTicket = yield* CreateTicketFlag; + const resourceOutput = yield* OutputFlag; const agent = yield* AgentFlag; const cliArgs = yield* CliArgs; @@ -160,42 +139,25 @@ export const rootCommandForFeatures = ( // human table), so the agent JSON default only applies when it's absent. const outputFormat = resolveAgentOutputFormat({ explicitOutputFormat, - goOutputFormat: goOutput, + goOutputFormat: resourceOutput, agentOverride: agent, detectedAgentName: aiTool.name, isBuiltInTextRequest: isBuiltInTextRequest(cliArgs.args), }); - const globalArgs: string[] = []; - if (Option.isSome(goOutput)) { - globalArgs.push("--output", goOutput.value); - } else if (outputFormat !== "text") { - globalArgs.push("--output", "json"); - } - if (profile !== "supabase") globalArgs.push("--profile", profile); - if (debug) globalArgs.push("--debug"); - if (Option.isSome(workdir)) globalArgs.push("--workdir", workdir.value); - if (experimental) globalArgs.push("--experimental"); - if (Option.isSome(networkId)) globalArgs.push("--network-id", networkId.value); - if (yes) globalArgs.push("--yes"); - if (dnsResolver !== "native") globalArgs.push("--dns-resolver", dnsResolver); - if (createTicket) globalArgs.push("--create-ticket"); - if (agent !== "auto") globalArgs.push("--agent", agent); - // Machine formats keep the text layer's error rendering but suppress the progress // spinner, which would otherwise corrupt the stdout payload. `-o pretty`/`-o table` // (db query's human default) and no `-o` keep the normal text/json layers. - const goFmt = Option.getOrUndefined(goOutput); - const isGoMachineFormat = goFmt !== undefined && goFmt !== "pretty" && goFmt !== "table"; - const outputLayer = isGoMachineFormat + const resourceFormat = Option.getOrUndefined(resourceOutput); + const isMachineResourceFormat = + resourceFormat !== undefined && + resourceFormat !== "pretty" && + resourceFormat !== "table"; + const outputLayer = isMachineResourceFormat ? quietProgressTextOutputLayer : outputLayerFor(outputFormat); - return Layer.mergeAll( - stackBackendLayer(options.stackBackend ?? "legacy"), - outputLayer, - makeGoProxyLayer({ globalArgs, parentOwnsCapturedSuccessTail: true }), - ); + return Layer.mergeAll(stackBackendLayer(options.stackBackend ?? "legacy"), outputLayer); }), ), ), diff --git a/apps/cli/src/command-internal/diff-engine.ts b/apps/cli/src/command-internal/diff-engine.ts index e0ebc1b3a5..b88a455853 100644 --- a/apps/cli/src/command-internal/diff-engine.ts +++ b/apps/cli/src/command-internal/diff-engine.ts @@ -18,18 +18,16 @@ export function shouldUsePgDelta(inputs: { } /** - * Reports whether `db diff` should run in pg-delta mode. An explicit `--use-migra`, - * `--use-pgadmin`, or `--use-pg-schema` is an authoritative rollback that clears pg-delta mode; - * `--use-migra` defaults to true, so only an explicit pass (`useMigraChanged`) counts as opting - * out. + * Reports whether `db diff` should run in pg-delta mode. An explicit `--use-migra` or + * `--use-pgadmin` is an authoritative rollback that clears pg-delta mode; `--use-migra` defaults + * to true, so only an explicit pass (`useMigraChanged`) counts as opting out. */ export function resolveDiffEngine(inputs: { readonly useMigraChanged: boolean; readonly usePgAdmin: boolean; - readonly usePgSchema: boolean; readonly pgDeltaDefault: boolean; }): boolean { - if (inputs.useMigraChanged || inputs.usePgAdmin || inputs.usePgSchema) { + if (inputs.useMigraChanged || inputs.usePgAdmin) { return false; } return inputs.pgDeltaDefault; 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 7f1a7998b9..0c24c29cc4 100644 --- a/apps/cli/src/command-internal/diff-engine.unit.test.ts +++ b/apps/cli/src/command-internal/diff-engine.unit.test.ts @@ -29,7 +29,6 @@ describe("resolveDiffEngine", () => { const base = { useMigraChanged: false, usePgAdmin: false, - usePgSchema: false, pgDeltaDefault: true, }; @@ -45,10 +44,6 @@ describe("resolveDiffEngine", () => { it("--use-pgadmin clears pg-delta mode", () => { expect(resolveDiffEngine({ ...base, usePgAdmin: true })).toBe(false); }); - - it("--use-pg-schema clears pg-delta mode", () => { - expect(resolveDiffEngine({ ...base, usePgSchema: true })).toBe(false); - }); }); describe("resolvePullDiffEngine", () => { diff --git a/apps/cli/src/command-internal/go-child-exit.error.ts b/apps/cli/src/command-internal/go-child-exit.error.ts deleted file mode 100644 index 511f1a8d9f..0000000000 --- a/apps/cli/src/command-internal/go-child-exit.error.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Data, Runtime } from "effect"; - -import { - actionability, - type CliErrorActionabilityDeclaration, - ErrorActionabilityId, -} from "../shared/telemetry/error-actionability.ts"; - -/** - * A spawned `supabase-go` child process exited non-zero, or couldn't be spawned at all. - * - * Carries the exit code through `Runtime.errorExitCode` so it survives every `Effect.ensuring` - * finalizer up to `runCli`'s own exit call, instead of exiting the process directly and skipping - * them. `runCli` also special-cases this class to skip its own generic stderr line, since the - * child already wrote its own failure detail there. `exitCode` must be a real non-zero status; - * every construction site guards this before creating the error. - */ -export class GoChildExitError extends Data.TaggedError("GoChildExitError")<{ - readonly exitCode: number; - readonly message: string; -}> { - override readonly [Runtime.errorExitCode] = this.exitCode; - - get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { - return actionability.unknown; - } -} diff --git a/apps/cli/src/command-internal/go-proxy-invocation.ts b/apps/cli/src/command-internal/go-proxy-invocation.ts deleted file mode 100644 index eb0abfe66f..0000000000 --- a/apps/cli/src/command-internal/go-proxy-invocation.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { Context, Effect, Layer, Ref } from "effect"; - -interface GoProxyInvocationShape { - readonly markDelegated: Effect.Effect; - readonly wasDelegated: Effect.Effect; -} - -export class GoProxyInvocation extends Context.Service()( - "supabase/cli/GoProxyInvocation", -) {} - -export const goProxyInvocationLayer = Layer.effect( - GoProxyInvocation, - Effect.gen(function* () { - const delegated = yield* Ref.make(false); - return GoProxyInvocation.of({ - markDelegated: Ref.set(delegated, true), - wasDelegated: Ref.get(delegated), - }); - }), -); diff --git a/apps/cli/src/command-internal/go-proxy.layer.ts b/apps/cli/src/command-internal/go-proxy.layer.ts deleted file mode 100644 index e5c10d39ea..0000000000 --- a/apps/cli/src/command-internal/go-proxy.layer.ts +++ /dev/null @@ -1,282 +0,0 @@ -import { existsSync } from "node:fs"; -import { createRequire } from "node:module"; -import os from "node:os"; -import path from "node:path"; -import process from "node:process"; -import { Effect, Layer, Option, Stream } from "effect"; -import * as ChildProcess from "effect/unstable/process/ChildProcess"; -import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner"; -import { CLI_VERSION } from "../shared/cli/version.ts"; -import { ProcessControl } from "../shared/runtime/process-control.service.ts"; -import { GoChildExitError } from "./go-child-exit.error.ts"; -import { GoProxyInvocation } from "./go-proxy-invocation.ts"; -import { GoProxy } from "./go-proxy.service.ts"; - -const markDelegated = Effect.serviceOption(GoProxyInvocation).pipe( - Effect.flatMap((invocation) => - Option.isSome(invocation) ? invocation.value.markDelegated : Effect.void, - ), -); - -const PLATFORM_CANDIDATES: Partial>>>> = - { - darwin: { arm64: ["darwin-arm64"], x64: ["darwin-x64"] }, - linux: { - arm64: ["linux-arm64", "linux-arm64-musl"], - x64: ["linux-x64", "linux-x64-musl"], - }, - win32: { arm64: ["windows-arm64"], x64: ["windows-x64"] }, - }; - -const require = createRequire(import.meta.url); - -/** - * Outcome of looking up `supabase-go`. `notFound` carries every location checked, so the error can - * be specific about what was tried, and callers never silently fall back to `supabase` on PATH - * (which would resolve to this shim itself and fork-bomb). - */ -export type BinaryResolution = - | { readonly found: string } - | { readonly notFound: ReadonlyArray }; - -function resolveBinary(): BinaryResolution { - const tried: string[] = []; - - const envBin = process.env["SUPABASE_GO_BINARY"]; - if (envBin) return { found: envBin }; - tried.push("$SUPABASE_GO_BINARY (unset)"); - - const ext = process.platform === "win32" ? ".exe" : ""; - - // When running as a compiled standalone binary (exec'd by the base shim), process.execPath is - // this binary's own path; look for supabase-go co-located next to it. - const colocated = path.join(path.dirname(process.execPath), `supabase-go${ext}`); - if (existsSync(colocated)) return { found: colocated }; - tried.push(`${colocated} (not found alongside the shim)`); - - // When running from source, resolve via installed npm packages. - // Guard with existsSync — in dev the workspace stub packages exist but their bin/ is empty. - const candidates = PLATFORM_CANDIDATES[process.platform]?.[os.arch()] ?? []; - for (const suffix of candidates) { - try { - const pkgPath = path.dirname(require.resolve(`@supabase/cli-${suffix}/package.json`)); - const bin = path.join(pkgPath, "bin", `supabase-go${ext}`); - if (existsSync(bin)) return { found: bin }; - tried.push(`${bin} (npm package present, binary missing)`); - } catch { - tried.push(`@supabase/cli-${suffix} (npm package not installed)`); - } - } - - return { notFound: tried }; -} - -/** - * Builds a `curl | tar` install snippet for the host platform, using the version baked into this - * shim at build time. Returns null when there's no concrete release URL (dev build) or the host - * arch isn't one the release pipeline targets. - */ -function reinstallTarballSnippet(): ReadonlyArray | null { - if (CLI_VERSION === "0.0.0-dev") return null; - const archSuffix = process.arch === "x64" ? "amd64" : process.arch === "arm64" ? "arm64" : null; - if (archSuffix === null) return null; - // Node's `process.platform` is `win32`; GitHub release assets use the modern `windows` slug. - const osSlug = process.platform === "win32" ? "windows" : process.platform; - const asset = `supabase_${CLI_VERSION}_${osSlug}_${archSuffix}.tar.gz`; - return [ - ` mkdir -p "$HOME/.local/share/supabase"`, - ` curl -sL https://github.com/supabase/cli/releases/download/v${CLI_VERSION}/${asset} \\`, - ` | tar -xzf - -C "$HOME/.local/share/supabase"`, - ` export PATH="$HOME/.local/share/supabase:$PATH"`, - ]; -} - -export function formatGoBinaryNotFoundError(tried: ReadonlyArray): string { - const snippet = reinstallTarballSnippet(); - return [ - "Could not find the `supabase-go` binary.", - "", - "The Supabase CLI ships as two co-located binaries: `supabase` (this shim)", - "and `supabase-go` (the Go CLI that the shim forwards to). The shim looked", - "for `supabase-go` in:", - "", - ...tried.map((line) => ` • ${line}`), - "", - "To fix, do one of:", - " • Extract the release tarball into a directory and add the directory to", - " PATH (do not move `supabase` somewhere `supabase-go` doesn't follow).", - ...(snippet === null ? [] : [" For example, on this host:", "", ...snippet, ""]), - " • Install via npm: `npm i -g supabase`.", - " • Set SUPABASE_GO_BINARY to the absolute path of `supabase-go`.", - ].join("\n"); -} - -/** - * Creates a GoProxy layer. - * - * In production use `goProxyLayer` (no options). - * - * In tests pass `{ cwd, env }` to run the binary in an isolated directory - * with a controlled SUPABASE_HOME, so tests don't pollute the real home dir: - * - * makeGoProxyLayer({ - * cwd: projectDir, - * env: { SUPABASE_HOME: homeDir, SUPABASE_NO_KEYRING: "1", SUPABASE_TELEMETRY_DISABLED: "1" }, - * }) - * - * @public - */ -export function makeGoProxyLayer(opts?: { - cwd?: string; - /** - * Extra env for every spawned child. - */ - env?: Record; - globalArgs?: ReadonlyArray; - /** - * Let the parent emit its success tail after re-emitting captured stdout. - */ - parentOwnsCapturedSuccessTail?: boolean; - /** - * Override binary resolution. Primarily a test seam so specs don't have to - * mutate `process.env.SUPABASE_GO_BINARY` or stub the filesystem: - * - `string` — treat as the resolved Go binary path. - * - `{ notFound: [...] }` — simulate the not-found path; `.exec` will print - * the diagnostic and fail with a non-zero exit code. - * - * In production, leave unset and let `resolveBinary()` pick the right - * artifact for the host platform. - */ - binary?: string | BinaryResolution; -}): Layer.Layer { - return Layer.effect( - GoProxy, - Effect.gen(function* () { - const processControl = yield* ProcessControl; - const spawner = yield* ChildProcessSpawner; - const resolved: BinaryResolution = - typeof opts?.binary === "string" - ? { found: opts.binary } - : (opts?.binary ?? resolveBinary()); - const globalArgs = opts?.globalArgs ?? []; - - return GoProxy.of({ - exec: (args, execOpts) => - Effect.scoped( - Effect.gen(function* () { - if (!("found" in resolved)) { - yield* Effect.sync(() => { - process.stderr.write(`${formatGoBinaryNotFoundError(resolved.notFound)}\n`); - }); - return yield* Effect.fail( - new GoChildExitError({ - exitCode: 1, - message: "supabase-go binary not found", - }), - ); - } - const binary = resolved.found; - - // Hold terminal signals on the parent for the child's lifetime: the child spawner - // defaults to `detached: true` on non-Windows, which would put the child in its own - // process group and miss tty signals, so `detached: false` below lets Ctrl+C reach - // the Go binary directly. Without a listener, Bun/Node would also default-terminate - // the parent on SIGINT before the child's real exit code is known. - yield* processControl.holdSignals(["SIGINT", "SIGTERM", "SIGHUP"]); - // Only an instrumented caller that delegates the whole command suppresses child - // telemetry: the parent already emits `cli_command_executed` there. Pure proxy - // commands have no parent event, so the child must stay free to report. - const env = { - ...opts?.env, - ...execOpts?.env, - ...(execOpts?.suppressChildTelemetry === true - ? { SUPABASE_TELEMETRY_DISABLED: "1" } - : {}), - }; - const command = ChildProcess.make(binary, [...globalArgs, ...args], { - cwd: execOpts?.cwd ?? opts?.cwd, - env, - extendEnv: true, - stdin: "inherit", - stdout: "inherit", - stderr: "inherit", - detached: false, - }); - const exitCode = yield* spawner.exitCode(command).pipe(Effect.orDie); - if (exitCode !== 0) { - return yield* Effect.fail( - new GoChildExitError({ - exitCode, - message: `supabase-go exited with code ${exitCode} (see stderr for details)`, - }), - ); - } - yield* markDelegated; - }), - ), - execCapture: (args, execOpts) => - Effect.scoped( - Effect.gen(function* () { - if (!("found" in resolved)) { - yield* Effect.sync(() => { - process.stderr.write(`${formatGoBinaryNotFoundError(resolved.notFound)}\n`); - }); - return yield* Effect.fail( - new GoChildExitError({ - exitCode: 1, - message: "supabase-go binary not found", - }), - ); - } - const binary = resolved.found; - yield* processControl.holdSignals(["SIGINT", "SIGTERM", "SIGHUP"]); - // Same rule as `exec`: only an instrumented caller that owns the - // parent `cli_command_executed` event suppresses child telemetry. - const env = { - ...opts?.env, - ...execOpts?.env, - ...(execOpts?.suppressChildTelemetry === true - ? { SUPABASE_TELEMETRY_DISABLED: "1" } - : {}), - ...(opts?.parentOwnsCapturedSuccessTail === true - ? { SUPABASE_NO_UPDATE_NOTIFIER: "1" } - : {}), - }; - // Capture stdout while keeping stderr inherited, so progress still reaches the user - // while stdout is collected for wrapping. Callers pass stdin: "ignore" to give the - // child a non-TTY stdin so it can't block on a prompt before the wrapper emits its - // machine-output envelope. - const command = ChildProcess.make(binary, [...globalArgs, ...args], { - cwd: execOpts?.cwd ?? opts?.cwd, - env, - extendEnv: true, - stdin: execOpts?.stdin ?? "inherit", - stdout: "pipe", - stderr: "inherit", - detached: false, - }); - const handle = yield* spawner.spawn(command).pipe(Effect.orDie); - // Drain stdout fully before awaiting exit so a full pipe buffer can't - // deadlock the child. - const captured = yield* Stream.mkString(Stream.decodeText(handle.stdout)).pipe( - Effect.orDie, - ); - const exitCode = yield* handle.exitCode.pipe(Effect.orDie); - if (exitCode !== 0) { - return yield* Effect.fail( - new GoChildExitError({ - exitCode, - message: `supabase-go exited with code ${exitCode} (see stderr for details)`, - }), - ); - } - if (opts?.parentOwnsCapturedSuccessTail !== true) { - yield* markDelegated; - } - return captured; - }), - ), - }); - }), - ); -} diff --git a/apps/cli/src/command-internal/go-proxy.layer.unit.test.ts b/apps/cli/src/command-internal/go-proxy.layer.unit.test.ts deleted file mode 100644 index 2ea151e9c1..0000000000 --- a/apps/cli/src/command-internal/go-proxy.layer.unit.test.ts +++ /dev/null @@ -1,571 +0,0 @@ -import { describe, expect, it, vi } from "@effect/vitest"; -import { Cause, Deferred, Effect, Exit, Fiber, Layer, Sink, Stream } from "effect"; -import { ChildProcessSpawner } from "effect/unstable/process"; -import { - type CliProcessSignal, - ProcessControl, -} from "../shared/runtime/process-control.service.ts"; -import { GoChildExitError } from "./go-child-exit.error.ts"; -import { GoProxyInvocation, goProxyInvocationLayer } from "./go-proxy-invocation.ts"; -import { GoProxy } from "./go-proxy.service.ts"; -import { formatGoBinaryNotFoundError, makeGoProxyLayer } from "./go-proxy.layer.ts"; - -/** - * Regression tests for SIGINT propagation: Ctrl+C on a proxied long-running command must reach - * the Go sidecar and not lose its exit code. `ChildProcess.make` must be called with - * `detached: false`, and `processControl.holdSignals` must be acquired before spawn (covering - * SIGINT/SIGTERM/SIGHUP) and released on every exit path. - */ - -type CapturedCommand = { - command: string; - args: readonly string[]; - options: { - detached?: boolean; - stdin?: unknown; - stdout?: unknown; - stderr?: unknown; - cwd?: string; - env?: Record; - extendEnv?: boolean; - }; -}; - -type ExitBehavior = - | { kind: "success"; code: number } - | { kind: "never" } - | { kind: "fail"; error: string }; - -type HoldEvent = - | { kind: "acquire"; id: number; signals: ReadonlyArray } - | { kind: "release"; id: number }; - -/** - * Records holdSignals(...) acquire/release transitions. Each acquire gets a monotonically - * increasing id so tests can pair an acquire with its release. - * - * `exit()` here only guards against a regression that reintroduces a direct - * `ProcessControl.exit()` call; it blocks on `Effect.never` since nothing in this file exercises - * it. - */ -function mockProcessControl() { - const holdEvents: HoldEvent[] = []; - const exitCalls: number[] = []; - let nextHoldId = 0; - - const exit = (code: number) => - Effect.sync(() => { - exitCalls.push(code); - }).pipe(Effect.flatMap(() => Effect.never)); - - return { - get holdEvents() { - return holdEvents; - }, - get exitCalls() { - return exitCalls; - }, - layer: Layer.succeed( - ProcessControl, - ProcessControl.of({ - awaitSignal: () => Effect.never, - awaitShutdown: Effect.never, - holdSignals: (signals) => - Effect.acquireRelease( - Effect.sync(() => { - const id = nextHoldId++; - holdEvents.push({ kind: "acquire", id, signals }); - return id; - }), - (id) => - Effect.sync(() => { - holdEvents.push({ kind: "release", id }); - }), - ).pipe(Effect.asVoid), - exit, - setExitCode: () => Effect.void, - getExitCode: Effect.succeed(undefined), - }), - ), - }; -} - -/** - * Builds a mock `ChildProcessSpawner` that records every spawned command and returns a - * controllable exit code. `spawnedBeforeExit` resolves as soon as the spawn is observed, to - * sequence a race-then-interrupt. - */ -function mockSpawner(exit: ExitBehavior, spawnedBeforeExit?: Deferred.Deferred) { - const spawned: CapturedCommand[] = []; - const layer = Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make((command: any) => - Effect.sync(() => { - const cmd = command as CapturedCommand & { _tag: string }; - spawned.push({ - command: cmd.command, - args: cmd.args, - options: cmd.options, - }); - if (spawnedBeforeExit !== undefined) { - Deferred.doneUnsafe(spawnedBeforeExit, Effect.void); - } - const exitCode = - exit.kind === "success" - ? Effect.succeed(ChildProcessSpawner.ExitCode(exit.code)) - : exit.kind === "never" - ? Effect.never - : Effect.fail(new Error(exit.error) as any); - return ChildProcessSpawner.makeHandle({ - pid: ChildProcessSpawner.ProcessId(42_424), - exitCode, - isRunning: Effect.succeed(false), - kill: () => Effect.void, - unref: Effect.succeed(Effect.void), - stdin: Sink.drain as any, - stdout: Stream.empty, - stderr: Stream.empty, - all: Stream.empty, - getInputFd: () => Sink.drain as any, - getOutputFd: () => Stream.empty, - }); - }), - ), - ); - return { layer, spawned }; -} - -/** Injected directly via `makeGoProxyLayer({ binary })` so tests don't depend on workspace package state or `process.env`. */ -const TEST_BINARY = "/test/fake-supabase-go"; - -describe("formatGoBinaryNotFoundError", () => { - const TRIED = [ - "$SUPABASE_GO_BINARY (unset)", - "/usr/local/bin/supabase-go (not found alongside the shim)", - "@supabase/cli-linux-x64 (npm package not installed)", - ]; - - it("renders each tried location as a bullet and includes remediation hints", () => { - const message = formatGoBinaryNotFoundError(TRIED); - expect(message).toContain("Could not find the `supabase-go` binary"); - expect(message).toContain(" • $SUPABASE_GO_BINARY (unset)"); - expect(message).toContain(" • /usr/local/bin/supabase-go (not found alongside the shim)"); - expect(message).toContain(" • @supabase/cli-linux-x64 (npm package not installed)"); - expect(message).toContain("npm i -g supabase"); - expect(message).toContain("SUPABASE_GO_BINARY"); - }); - - it("omits the curl|tar snippet on dev builds (no CLI_VERSION baked in)", () => { - const message = formatGoBinaryNotFoundError(TRIED); - expect(message).not.toContain("curl -sL"); - expect(message).toContain("Extract the release tarball"); - }); -}); - -// Instantiates a fresh module with a stubbed CLI_VERSION to assert against a known release -// version + asset filename; nested in its own describe so the module mock doesn't bleed into -// other suites. -describe("formatGoBinaryNotFoundError - pinned snippet", () => { - const TRIED = ["$SUPABASE_GO_BINARY (unset)"]; - const PINNED_VERSION = "2.100.0"; - - async function withMockedHost( - opts: { platform: NodeJS.Platform; arch: NodeJS.Architecture }, - fn: (mod: typeof import("./go-proxy.layer.ts")) => void | Promise, - ): Promise { - vi.resetModules(); - vi.doMock("../shared/cli/version.ts", () => ({ CLI_VERSION: PINNED_VERSION })); - const originalPlatform = process.platform; - const originalArch = process.arch; - Object.defineProperty(process, "platform", { value: opts.platform, configurable: true }); - Object.defineProperty(process, "arch", { value: opts.arch, configurable: true }); - try { - const mod = await import("./go-proxy.layer.ts"); - await fn(mod); - } finally { - Object.defineProperty(process, "platform", { - value: originalPlatform, - configurable: true, - }); - Object.defineProperty(process, "arch", { value: originalArch, configurable: true }); - vi.doUnmock("../cli/version.ts"); - vi.resetModules(); - } - } - - it("renders a copy-pasteable install snippet for linux x64", async () => { - await withMockedHost({ platform: "linux", arch: "x64" }, (mod) => { - const message = mod.formatGoBinaryNotFoundError(TRIED); - expect(message).toContain( - `https://github.com/supabase/cli/releases/download/v${PINNED_VERSION}/supabase_${PINNED_VERSION}_linux_amd64.tar.gz`, - ); - expect(message).toContain(`mkdir -p "$HOME/.local/share/supabase"`); - expect(message).toContain(`export PATH="$HOME/.local/share/supabase:$PATH"`); - }); - }); - - it("maps Node's win32 platform to the release asset's `windows` slug", async () => { - await withMockedHost({ platform: "win32", arch: "x64" }, (mod) => { - const message = mod.formatGoBinaryNotFoundError(TRIED); - expect(message).toContain( - `https://github.com/supabase/cli/releases/download/v${PINNED_VERSION}/supabase_${PINNED_VERSION}_windows_amd64.tar.gz`, - ); - expect(message).not.toContain("win32"); - }); - }); - - it("maps darwin arm64 to the matching release asset", async () => { - await withMockedHost({ platform: "darwin", arch: "arm64" }, (mod) => { - expect(mod.formatGoBinaryNotFoundError(TRIED)).toContain( - `supabase_${PINNED_VERSION}_darwin_arm64.tar.gz`, - ); - }); - }); - - it("omits the snippet on unsupported architectures (no release asset)", async () => { - await withMockedHost({ platform: "linux", arch: "ia32" }, (mod) => { - expect(mod.formatGoBinaryNotFoundError(TRIED)).not.toContain("curl -sL"); - }); - }); -}); - -describe("makeGoProxyLayer", () => { - it.effect("records delegated execution for the parent success trailer", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = Layer.mergeAll( - makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ), - goProxyInvocationLayer, - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const invocation = yield* GoProxyInvocation; - - expect(yield* invocation.wasDelegated).toBe(false); - yield* proxy.exec(["migration", "squash", "--local"]); - expect(yield* invocation.wasDelegated).toBe(true); - }).pipe(Effect.provide(layer)); - }); - - it.effect("leaves captured success tails to the parent when configured", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = Layer.mergeAll( - makeGoProxyLayer({ - binary: TEST_BINARY, - env: { SUPABASE_NO_UPDATE_NOTIFIER: "0" }, - parentOwnsCapturedSuccessTail: true, - }).pipe(Layer.provide(Layer.mergeAll(spawner.layer, pc.layer))), - goProxyInvocationLayer, - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const invocation = yield* GoProxyInvocation; - - yield* proxy.execCapture(["db", "diff"], { - env: { SUPABASE_NO_UPDATE_NOTIFIER: "0" }, - }); - - expect(spawner.spawned[0]?.options.env?.SUPABASE_NO_UPDATE_NOTIFIER).toBe("1"); - expect(yield* invocation.wasDelegated).toBe(false); - }).pipe(Effect.provide(layer)); - }); - - it.effect("passes detached:false and inherited stdio to the spawner", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY, globalArgs: ["--debug"] }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["projects", "list"]); - - expect(spawner.spawned).toHaveLength(1); - const captured = spawner.spawned[0]!; - expect(captured.command).toBe(TEST_BINARY); - expect(captured.args).toEqual(["--debug", "projects", "list"]); - expect(captured.options.detached).toBe(false); - expect(captured.options.stdin).toBe("inherit"); - expect(captured.options.stdout).toBe("inherit"); - expect(captured.options.stderr).toBe("inherit"); - expect(captured.options.extendEnv).toBe(true); - }).pipe(Effect.provide(layer)); - }); - - it.effect("leaves child telemetry enabled for pure proxy commands", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["migration", "squash"]); - yield* proxy.execCapture(["gen", "keys"]); - - for (const captured of spawner.spawned) { - expect(captured.options.env ?? {}).not.toHaveProperty("SUPABASE_TELEMETRY_DISABLED"); - } - }).pipe(Effect.provide(layer)); - }); - - it.effect("suppresses child telemetry when the caller owns the parent event", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["db", "pull"], { suppressChildTelemetry: true }); - yield* proxy.execCapture(["db", "diff"], { - env: { CUSTOM: "kept" }, - suppressChildTelemetry: true, - }); - - for (const captured of spawner.spawned) { - expect(captured.options.env).toMatchObject({ SUPABASE_TELEMETRY_DISABLED: "1" }); - } - expect(spawner.spawned[1]?.options.env).toMatchObject({ CUSTOM: "kept" }); - }).pipe(Effect.provide(layer)); - }); - - it.effect("passes the layer's env to children, letting per-call env win", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ - binary: TEST_BINARY, - env: { SUPABASE_NO_UPDATE_NOTIFIER: "1" }, - }).pipe(Layer.provide(Layer.mergeAll(spawner.layer, pc.layer))); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["projects", "list"]); - yield* proxy.execCapture(["gen", "keys"]); - yield* proxy.exec(["projects", "list"], { env: { SUPABASE_NO_UPDATE_NOTIFIER: "0" } }); - - expect(spawner.spawned[0]?.options.env?.SUPABASE_NO_UPDATE_NOTIFIER).toBe("1"); - expect(spawner.spawned[1]?.options.env?.SUPABASE_NO_UPDATE_NOTIFIER).toBe("1"); - expect(spawner.spawned[2]?.options.env?.SUPABASE_NO_UPDATE_NOTIFIER).toBe("0"); - }).pipe(Effect.provide(layer)); - }); - - it.effect("propagates non-zero exit codes via GoChildExitError", () => { - const spawner = mockSpawner({ kind: "success", code: 7 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const exit = yield* proxy.exec(["some", "command"]).pipe(Effect.exit); - expect(Exit.isFailure(exit)).toBe(true); - if (Exit.isFailure(exit)) { - const error = Cause.squash(exit.cause); - expect(error).toBeInstanceOf(GoChildExitError); - expect((error as GoChildExitError).exitCode).toBe(7); - } - expect(pc.exitCalls).toEqual([]); - }).pipe(Effect.provide(layer)); - }); - - it.effect("lets an Effect.ensuring finalizer run after a non-zero exit (CLI-1879)", () => { - const spawner = mockSpawner({ kind: "success", code: 5 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - let finalizerRan = false; - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["some", "command"]).pipe( - Effect.ensuring( - Effect.sync(() => { - finalizerRan = true; - }), - ), - Effect.exit, - ); - expect(finalizerRan).toBe(true); - }).pipe(Effect.provide(layer)); - }); - - it.effect("does not call ProcessControl.exit when the Go binary exits zero", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec(["some", "command"]); - expect(pc.exitCalls).toEqual([]); - }).pipe(Effect.provide(layer)); - }); - - it.effect("calls holdSignals with SIGINT+SIGTERM+SIGHUP before spawning", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec([]); - - const acquires = pc.holdEvents.filter((e) => e.kind === "acquire"); - expect(acquires).toHaveLength(1); - expect(acquires[0]!.signals).toEqual(["SIGINT", "SIGTERM", "SIGHUP"]); - - expect(spawner.spawned).toHaveLength(1); - expect(pc.holdEvents[0]).toEqual(expect.objectContaining({ kind: "acquire" })); - }).pipe(Effect.provide(layer)); - }); - - it.effect("releases the holdSignals scope on successful exec", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - yield* proxy.exec([]); - - expect(pc.holdEvents).toEqual([ - { kind: "acquire", id: 0, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - { kind: "release", id: 0 }, - ]); - }).pipe(Effect.provide(layer)); - }); - - it.effect("releases the holdSignals scope when the spawner fails", () => { - const spawner = mockSpawner({ kind: "fail", error: "spawn failed" }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - // spawner failures are Effect.orDie'd, so we swallow the defect here. - yield* proxy.exec([]).pipe(Effect.exit); - - expect(pc.holdEvents).toContainEqual({ kind: "release", id: 0 }); - }).pipe(Effect.provide(layer)); - }); - - it.effect("releases the holdSignals scope when the fiber is interrupted", () => { - const spawned = Deferred.makeUnsafe(); - const spawner = mockSpawner({ kind: "never" }, spawned); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const fiber = yield* proxy.exec([]).pipe(Effect.forkChild({ startImmediately: true })); - yield* Deferred.await(spawned); - - expect(pc.holdEvents).toEqual([ - { kind: "acquire", id: 0, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - ]); - - yield* Fiber.interrupt(fiber); - - expect(pc.holdEvents).toEqual([ - { kind: "acquire", id: 0, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - { kind: "release", id: 0 }, - ]); - }).pipe(Effect.provide(layer)); - }); - - it.effect( - "prints a diagnostic and fails with exit code 1 when supabase-go cannot be resolved", - () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); - const tried = [ - "$SUPABASE_GO_BINARY (unset)", - "/usr/local/bin/supabase-go (not found alongside the shim)", - ]; - const layer = makeGoProxyLayer({ binary: { notFound: tried } }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const exit = yield* proxy.exec(["db", "start"]).pipe(Effect.exit); - - expect(spawner.spawned).toHaveLength(0); - expect(Exit.isFailure(exit)).toBe(true); - if (Exit.isFailure(exit)) { - const error = Cause.squash(exit.cause); - expect(error).toBeInstanceOf(GoChildExitError); - expect((error as GoChildExitError).exitCode).toBe(1); - } - expect(pc.exitCalls).toEqual([]); - expect(stderr).toHaveBeenCalledTimes(1); - const written = String(stderr.mock.calls[0]![0]); - expect(written).toContain("Could not find the `supabase-go` binary"); - expect(written).toContain("$SUPABASE_GO_BINARY (unset)"); - expect(written).toContain("/usr/local/bin/supabase-go"); - expect(written).toContain("SUPABASE_GO_BINARY"); - stderr.mockRestore(); - }).pipe(Effect.provide(layer)); - }, - ); - - it.effect( - "execCapture also prints a diagnostic and fails with exit code 1 when supabase-go cannot be resolved", - () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); - const tried = ["$SUPABASE_GO_BINARY (unset)"]; - const layer = makeGoProxyLayer({ binary: { notFound: tried } }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - const exit = yield* proxy.execCapture(["db", "dump"]).pipe(Effect.exit); - - expect(spawner.spawned).toHaveLength(0); - expect(Exit.isFailure(exit)).toBe(true); - if (Exit.isFailure(exit)) { - const error = Cause.squash(exit.cause); - expect(error).toBeInstanceOf(GoChildExitError); - expect((error as GoChildExitError).exitCode).toBe(1); - } - expect(pc.exitCalls).toEqual([]); - expect(stderr).toHaveBeenCalledTimes(1); - stderr.mockRestore(); - }).pipe(Effect.provide(layer)); - }, - ); - - it.effect("opens and closes a fresh hold scope per sequential exec call", () => { - const spawner = mockSpawner({ kind: "success", code: 0 }); - const pc = mockProcessControl(); - const layer = makeGoProxyLayer({ binary: TEST_BINARY }).pipe( - Layer.provide(Layer.mergeAll(spawner.layer, pc.layer)), - ); - return Effect.gen(function* () { - const proxy = yield* GoProxy; - for (let i = 0; i < 3; i++) { - yield* proxy.exec([`call-${i}`]); - } - - expect(pc.holdEvents).toEqual([ - { kind: "acquire", id: 0, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - { kind: "release", id: 0 }, - { kind: "acquire", id: 1, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - { kind: "release", id: 1 }, - { kind: "acquire", id: 2, signals: ["SIGINT", "SIGTERM", "SIGHUP"] }, - { kind: "release", id: 2 }, - ]); - expect(spawner.spawned).toHaveLength(3); - }).pipe(Effect.provide(layer)); - }); -}); diff --git a/apps/cli/src/command-internal/go-proxy.service.ts b/apps/cli/src/command-internal/go-proxy.service.ts deleted file mode 100644 index 452fc2c51c..0000000000 --- a/apps/cli/src/command-internal/go-proxy.service.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { Effect } from "effect"; -import { Context } from "effect"; -import type { GoChildExitError } from "./go-child-exit.error.ts"; - -interface GoProxyShape { - /** - * Forwards args to the Go binary, inheriting stdio and propagating the exit code. Fails with - * `GoChildExitError` (carrying the exact exit code) on a non-zero exit or an unresolvable - * binary. - * - * `opts.suppressChildTelemetry` disables telemetry in the child; set it only when the caller's - * own command instrumentation already emits `cli_command_executed`, since a pure proxy - * handler's Go child is its only telemetry emitter. - */ - readonly exec: ( - args: ReadonlyArray, - opts?: { - readonly cwd?: string; - readonly env?: Record; - readonly suppressChildTelemetry?: boolean; - }, - ) => Effect.Effect; - - /** - * Like `exec`, but captures the child's stdout and returns it as a string instead of - * inheriting it; stderr stays inherited. Fails with `GoChildExitError` the same way `exec` - * does. - * - * `opts.stdin: "ignore"` gives the child a non-TTY stdin so a prompt (Go's `PromptYesNo`) - * takes its default instead of blocking — required when a machine-output caller delegates a - * command that would otherwise prompt before the JSON envelope is emitted. - */ - readonly execCapture: ( - args: ReadonlyArray, - opts?: { - readonly cwd?: string; - readonly env?: Record; - readonly stdin?: "inherit" | "ignore"; - readonly suppressChildTelemetry?: boolean; - }, - ) => Effect.Effect; -} - -export class GoProxy extends Context.Service()("supabase/cli/GoProxy") {} diff --git a/apps/cli/src/command-internal/removed-command.ts b/apps/cli/src/command-internal/removed-command.ts new file mode 100644 index 0000000000..b852802e65 --- /dev/null +++ b/apps/cli/src/command-internal/removed-command.ts @@ -0,0 +1,55 @@ +import { Data, Effect } from "effect"; +import { + actionability, + type CliErrorActionabilityDeclaration, + ErrorActionabilityId, +} from "../shared/telemetry/error-actionability.ts"; +import { + CommandRuntime, + getCommandRuntimeCommand, +} from "../shared/runtime/command-runtime.service.ts"; + +/** + * A tombstoned command path or a rejected removed flag. The caller supplies its own + * replacement suggestion; there is no closed vocabulary for it since removal + * suggestions vary per surface. + */ +export class RemovedSurfaceError extends Data.TaggedError("RemovedSurfaceError")<{ + readonly message: string; + readonly suggestion: string; + readonly kind: "command" | "flag"; +}> { + get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { + return { + ...actionability.removedSurface, + fingerprint_suffix: this.kind === "flag" ? "removed_flag" : "removed_command", + }; + } +} + +/** + * Fails with a `RemovedSurfaceError` naming the invoked command path. The caller wraps the + * returned effect in `withCommandTelemetry()` then `withJsonErrorHandling`, matching + * `commit.command.ts`'s composition order, and provides a `CommandRuntime` layer for its path. + */ +export const removedCommand = (suggestion: string) => + Effect.gen(function* () { + const commandRuntime = yield* CommandRuntime; + const command = getCommandRuntimeCommand(commandRuntime); + return yield* Effect.fail( + new RemovedSurfaceError({ + message: `supabase ${command} was removed.`, + suggestion, + kind: "command", + }), + ); + }); + +/** + * Fails with a `RemovedSurfaceError` for a removed flag on an otherwise-native command. The host + * command already wraps its whole handler in `withCommandTelemetry`, so this is a bare effect. + */ +export const removedFlag = (flag: string, suggestion: string) => + Effect.fail( + new RemovedSurfaceError({ message: `${flag} was removed.`, suggestion, kind: "flag" }), + ); diff --git a/apps/cli/src/command-internal/removed-command.unit.test.ts b/apps/cli/src/command-internal/removed-command.unit.test.ts new file mode 100644 index 0000000000..d9441ecd31 --- /dev/null +++ b/apps/cli/src/command-internal/removed-command.unit.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { classifyCliErrorActionability } from "../shared/telemetry/error-actionability.ts"; +import { RemovedSurfaceError } from "./removed-command.ts"; + +describe("RemovedSurfaceError actionability", () => { + it("classifies a removed command with the :removed_command fingerprint suffix", () => { + const classified = classifyCliErrorActionability( + new RemovedSurfaceError({ + message: "supabase db branch create was removed.", + suggestion: "Use `supabase branches --help` instead.", + kind: "command", + }), + ); + expect(classified).toMatchObject({ + error_kind: "user_actionable", + error_category: "invalid_input", + has_suggestion: true, + suggestion_type: "run_command", + }); + expect(classified.error_fingerprint).toBe("tag:RemovedSurfaceError:removed_command"); + }); + + it("classifies a removed flag with the :removed_flag fingerprint suffix", () => { + const classified = classifyCliErrorActionability( + new RemovedSurfaceError({ + message: "--use-pg-schema was removed.", + suggestion: "Use the default migra engine or --use-pg-delta.", + kind: "flag", + }), + ); + expect(classified).toMatchObject({ + error_kind: "user_actionable", + error_category: "invalid_input", + has_suggestion: true, + suggestion_type: "run_command", + }); + expect(classified.error_fingerprint).toBe("tag:RemovedSurfaceError:removed_flag"); + }); + + it("classifies a field-less probe instance without throwing", () => { + const probe = Object.create(RemovedSurfaceError.prototype) as RemovedSurfaceError; + expect(() => classifyCliErrorActionability(probe)).not.toThrow(); + const classified = classifyCliErrorActionability(probe); + expect(classified.error_fingerprint.endsWith(":removed_command")).toBe(true); + expect(classified).toMatchObject({ + error_kind: "user_actionable", + error_category: "invalid_input", + }); + }); +}); diff --git a/apps/cli/src/command-internal/schema-flags.ts b/apps/cli/src/command-internal/schema-flags.ts index 9aa44377ce..a8599718d3 100644 --- a/apps/cli/src/command-internal/schema-flags.ts +++ b/apps/cli/src/command-internal/schema-flags.ts @@ -1,8 +1,7 @@ /** - * Normalizes a repeated `--schema` flag into a flat list, matching the CSV-per-occurrence - * parsing every `--schema`-accepting delegated Go subprocess expects (shared logic lives in + * Normalizes a repeated `--schema` flag into a flat list (shared logic lives in * `string-slice-flag.ts`). Also provides `schemaToCsvField`, the CSV re-encoder used when - * forwarding `--schema` back to that subprocess. + * printing a `--schema` value back into a suggested shell command. * * Shared by `gen types`, `db lint`, `db dump`, `db pull`, `db diff`, and `db schema * {generate,sync}`. @@ -13,9 +12,10 @@ export { StringSliceFlagParseError as SchemaFlagParseError }; export const parseSchemaFlags = parseStringSliceFlag; -// Whether a CSV field must be quoted (matches the `encoding/csv` writer a delegated Go -// subprocess re-parses): never quote the empty string; always quote `\.`; quote when the -// field contains `,`, `"`, `\r`, or `\n`; otherwise quote when the first rune is whitespace. +// Whether a CSV field must be quoted (matches Go's `encoding/csv` writer, so a printed +// suggestion round-trips through the same CSV parsing rules `--schema` values use): never quote +// the empty string; always quote `\.`; quote when the field contains `,`, `"`, `\r`, or `\n`; +// otherwise quote when the first rune is whitespace. function fieldNeedsQuotes(field: string): boolean { if (field === "") return false; if (field === "\\.") return true; @@ -26,9 +26,8 @@ function fieldNeedsQuotes(field: string): boolean { /** * Serializes a single parsed schema value back into one CSV field — the inverse of * `readAsCSVStrict` for one element. A schema parsed from `--schema '"tenant,one"'` is the - * single value `tenant,one`; forwarding it raw to a delegated Go subprocess would let pflag's - * `StringSlice` re-parse it as CSV and split it into two schemas, so this re-encodes it to - * keep it one field when rebuilding argv for `db diff`/`db pull`. + * single value `tenant,one`; printing it raw into a suggested `--schema` command would let the + * next CSV parse re-split it into two schemas, so this re-encodes it to keep it one field. */ export function schemaToCsvField(value: string): string { if (!fieldNeedsQuotes(value)) return value; diff --git a/apps/cli/src/command-internal/upgrade-notice.ts b/apps/cli/src/command-internal/upgrade-notice.ts index b0e8825c4e..2820c3bd86 100644 --- a/apps/cli/src/command-internal/upgrade-notice.ts +++ b/apps/cli/src/command-internal/upgrade-notice.ts @@ -354,26 +354,23 @@ export const upgradeNoticeHook = ( args: ReadonlyArray, info: { readonly cleanShowHelp: boolean; - readonly delegatedToGo: boolean; readonly workingDirectory?: string; readonly isValueTakingFlagToken: (token: string) => boolean; }, ): Effect.Effect => - info.delegatedToGo - ? Effect.void - : Effect.promise(() => - runUpgradeNotice({ - env: process.env, - args, - cleanShowHelp: info.cleanShowHelp, - isValueTakingFlagToken: info.isValueTakingFlagToken, - cwd: process.cwd(), - resolvedCwd: info.workingDirectory, - currentVersion: CLI_VERSION, - now: Date.now, - fetchLatestTag: fetchLatestReleaseTag, - writeStderr: (text) => { - process.stderr.write(text); - }, - }), - ).pipe(Effect.ignoreCause); + Effect.promise(() => + runUpgradeNotice({ + env: process.env, + args, + cleanShowHelp: info.cleanShowHelp, + isValueTakingFlagToken: info.isValueTakingFlagToken, + cwd: process.cwd(), + resolvedCwd: info.workingDirectory, + currentVersion: CLI_VERSION, + now: Date.now, + fetchLatestTag: fetchLatestReleaseTag, + writeStderr: (text) => { + process.stderr.write(text); + }, + }), + ).pipe(Effect.ignoreCause); diff --git a/apps/cli/src/command-internal/upgrade-notice.unit.test.ts b/apps/cli/src/command-internal/upgrade-notice.unit.test.ts index 62c3243b89..ea69d782e2 100644 --- a/apps/cli/src/command-internal/upgrade-notice.unit.test.ts +++ b/apps/cli/src/command-internal/upgrade-notice.unit.test.ts @@ -624,14 +624,8 @@ describe("runUpgradeNotice", () => { }); }); -/** - * `delegatedToGo` is asserted directly rather than via a real proxied - * command, so this doesn't depend on which commands still delegate to Go — - * that set only shrinks (`docs/go-cli-porting-status.md`), and a test pinned - * to one command would break the moment it's ported. - */ describe("upgradeNoticeHook", () => { - async function stderrFromHook(delegatedToGo: boolean): Promise { + async function stderrFromHook(): Promise { const workdir = mkdtempSync(join(tmpdir(), "supabase-upgrade-notice-hook-")); mkdirSync(join(workdir, "supabase", ".temp"), { recursive: true }); writeFileSync(join(workdir, "supabase", "config.toml"), 'project_id = "demo"\n'); @@ -652,7 +646,6 @@ describe("upgradeNoticeHook", () => { await Effect.runPromise( upgradeNoticeHook(["db", "branch", "list"], { cleanShowHelp: false, - delegatedToGo, workingDirectory: workdir, isValueTakingFlagToken: () => false, }), @@ -667,12 +660,8 @@ describe("upgradeNoticeHook", () => { } it("prints the notice for a natively handled command", async () => { - expect(await stderrFromHook(false)).toContain( + expect(await stderrFromHook()).toContain( "A new version of Supabase CLI is available: v99.99.99", ); }); - - it("stays silent when the run delegated to Go, which printed its own notice", async () => { - expect(await stderrFromHook(true)).toBe(""); - }); }); diff --git a/apps/cli/src/commands/db/branch/SIDE_EFFECTS.md b/apps/cli/src/commands/db/branch/SIDE_EFFECTS.md new file mode 100644 index 0000000000..f896bb2616 --- /dev/null +++ b/apps/cli/src/commands/db/branch/SIDE_EFFECTS.md @@ -0,0 +1,69 @@ +# `supabase db branch ` + +Single shared side-effect document for all four `db branch` leaves. Local database +branches are no longer supported; each leaf is a tombstone that fails with a +removal error and a replacement suggestion instead of performing any work. + +## Files Read + +| Path | Format | When | +| ---- | ------ | ---- | +| — | — | — | + +## Files Written + +| Path | Format | When | +| ---- | ------ | ---- | +| — | — | — | + +## API Routes + +| Method | Path | Auth | Request body | Response (used fields) | +| ------ | ---- | ---- | ------------ | ---------------------- | +| — | — | — | — | — | + +## Environment Variables + +| Variable | Purpose | Required? | +| -------- | ------- | --------- | +| — | — | — | + +## Exit Codes + +| Code | Condition | +| ---- | ---------------------------------- | +| `1` | every invocation (removed command) | + +## Output + +### `--output-format text` + +Writes the removal message and the replacement suggestion to stderr, two lines, +regardless of `-o`/`--output`: + +``` +supabase db branch was removed. +Local database branches are no longer supported. For hosted preview branches, see `supabase branches --help`. +``` + +### `--output-format json` / `stream-json` + +Emits the JSON error envelope on stdout instead of the stderr lines above; the +envelope's `code` is `RemovedSurfaceError` and `suggestion` carries the same +replacement text. + +## Telemetry Events Fired + +One `cli_command_executed` event per invocation, with `exit_code: 1` and an +`error_fingerprint` ending in `:removed_command` (`RemovedSurfaceError`). + +## Notes + +- `create`/`delete`/`switch` accept an optional `` positional so a bare + invocation still reaches the tombstone (and emits telemetry) instead of failing + parse with a missing-argument error. +- No positional or flag value is read; every leaf fails identically regardless of + its arguments. +- `supabase branches --help` covers the hosted preview-branching product this + suggestion points to, which is a different product from local DB branches, not a + drop-in replacement. diff --git a/apps/cli/src/commands/db/branch/branch.command.ts b/apps/cli/src/commands/db/branch/branch.command.ts index d3363a0a22..b15dbb6d62 100644 --- a/apps/cli/src/commands/db/branch/branch.command.ts +++ b/apps/cli/src/commands/db/branch/branch.command.ts @@ -1,12 +1,47 @@ -import { Command } from "effect/unstable/cli"; -import { dbBranchCreateCommand } from "./create/create.command.ts"; -import { dbBranchDeleteCommand } from "./delete/delete.command.ts"; -import { dbBranchListCommand } from "./list/list.command.ts"; -import { dbBranchSwitchCommand } from "./switch/switch.command.ts"; +import { Argument, Command } from "effect/unstable/cli"; +import { removedCommand } 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"; + +const REMOVED_SUGGESTION = + "Local database branches are no longer supported. For hosted preview branches, see `supabase branches --help`."; + +/** A tombstoned `db branch` leaf taking an optional `` positional. */ +function removedBranchLeaf(name: string, argDescription: string) { + return Command.make(name, { + branchName: Argument.string("branch name").pipe( + Argument.withDescription(argDescription), + Argument.optional, + ), + } as const).pipe( + Command.withDescription("Removed: local database branches are no longer supported."), + Command.withShortDescription("Removed: local database branches are no longer supported"), + Command.withHandler(() => + removedCommand(REMOVED_SUGGESTION).pipe(withCommandTelemetry(), withJsonErrorHandling), + ), + Command.provide(commandRuntimeLayer(["db", "branch", name])), + ); +} + +const dbBranchCreateCommand = removedBranchLeaf("create", "Name for the new branch."); +const dbBranchDeleteCommand = removedBranchLeaf("delete", "Name of the branch to delete."); +const dbBranchSwitchCommand = removedBranchLeaf("switch", "Name of the branch to switch to."); + +const dbBranchListCommand = Command.make("list", {}).pipe( + Command.withDescription("Removed: local database branches are no longer supported."), + Command.withShortDescription("Removed: local database branches are no longer supported"), + Command.withHandler(() => + removedCommand(REMOVED_SUGGESTION).pipe(withCommandTelemetry(), withJsonErrorHandling), + ), + Command.provide(commandRuntimeLayer(["db", "branch", "list"])), +); export const dbBranchCommand = Command.make("branch").pipe( - Command.withDescription("Manage local database branches."), - Command.withShortDescription("Manage local database branches"), + Command.withDescription( + "Removed: local database branches are no longer supported. See each subcommand for details.", + ), + Command.withShortDescription("Removed: local database branches are no longer supported"), Command.withSubcommands([ dbBranchCreateCommand, dbBranchDeleteCommand, diff --git a/apps/cli/src/commands/db/branch/branch.e2e.test.ts b/apps/cli/src/commands/db/branch/branch.e2e.test.ts new file mode 100644 index 0000000000..1aa00cc4ed --- /dev/null +++ b/apps/cli/src/commands/db/branch/branch.e2e.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, test } from "vitest"; +import { makeTempHome, runSupabase } from "../../../../tests/helpers/cli.ts"; + +const E2E_TIMEOUT_MS = 30_000; + +describe("supabase db branch (removed)", () => { + test( + "list exits 1 with the removal message and its replacement suggestion", + { timeout: E2E_TIMEOUT_MS }, + async () => { + using home = makeTempHome(); + const { exitCode, stderr } = await runSupabase(["db", "branch", "list"], { + home: home.dir, + env: { HOME: home.dir }, + }); + expect(exitCode).toBe(1); + expect(stderr).toContain("supabase db branch list was removed."); + expect(stderr).toContain( + "Local database branches are no longer supported. For hosted preview branches, see `supabase branches --help`.", + ); + }, + ); +}); diff --git a/apps/cli/src/commands/db/branch/create/SIDE_EFFECTS.md b/apps/cli/src/commands/db/branch/create/SIDE_EFFECTS.md deleted file mode 100644 index 4bc31b63e5..0000000000 --- a/apps/cli/src/commands/db/branch/create/SIDE_EFFECTS.md +++ /dev/null @@ -1,53 +0,0 @@ -# `supabase db branch create` - -## Files Read - -| Path | Format | When | -| -------------------------------- | ------ | ---------------------------------- | -| `/supabase/config.toml` | TOML | always, to resolve local DB config | - -## Files Written - -| Path | Format | When | -| --------------------------------------------------------- | --------- | ------ | -| `/supabase/.branches//` (directory) | directory | always | - -## API Routes - -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ---- | ---- | ------------ | ---------------------- | -| — | — | — | — | — | - -## Environment Variables - -| Variable | Purpose | Required? | -| -------- | ------- | --------- | -| — | — | — | - -## Exit Codes - -| Code | Condition | -| ---- | -------------------------- | -| `0` | success | -| `1` | branch already exists | -| `1` | local database not running | - -## Output - -### `--output-format text` - -Prints a confirmation message to stdout on success. - -### `--output-format json` - -Not applicable. - -### `--output-format stream-json` - -Not applicable. - -## Notes - -- Deprecated: use `branches create ` instead. -- Requires exactly one positional argument: the branch name. -- This is a local-only operation (forking remote databases is not supported). diff --git a/apps/cli/src/commands/db/branch/create/create.command.ts b/apps/cli/src/commands/db/branch/create/create.command.ts deleted file mode 100644 index bdcb982f2b..0000000000 --- a/apps/cli/src/commands/db/branch/create/create.command.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Argument, Command } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { dbBranchCreate } from "./create.handler.ts"; - -const config = { - branchName: Argument.string("branch name").pipe( - Argument.withDescription("Name for the new branch."), - ), -} as const; - -export type DbBranchCreateFlags = CliCommand.Command.Config.Infer; - -export const dbBranchCreateCommand = Command.make("create", config).pipe( - Command.withDescription("Create a branch."), - Command.withShortDescription("Create a branch"), - Command.withHandler((flags) => dbBranchCreate(flags)), -); diff --git a/apps/cli/src/commands/db/branch/create/create.handler.ts b/apps/cli/src/commands/db/branch/create/create.handler.ts deleted file mode 100644 index f1308809b4..0000000000 --- a/apps/cli/src/commands/db/branch/create/create.handler.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { Effect } from "effect"; -import { GoProxy } from "../../../../command-internal/go-proxy.service.ts"; -import type { DbBranchCreateFlags } from "./create.command.ts"; - -export const dbBranchCreate = Effect.fn("db.branch.create")(function* (flags: DbBranchCreateFlags) { - const proxy = yield* GoProxy; - const args: string[] = ["db", "branch", "create", flags.branchName]; - yield* proxy.exec(args); -}); diff --git a/apps/cli/src/commands/db/branch/delete/SIDE_EFFECTS.md b/apps/cli/src/commands/db/branch/delete/SIDE_EFFECTS.md deleted file mode 100644 index af5a15e49d..0000000000 --- a/apps/cli/src/commands/db/branch/delete/SIDE_EFFECTS.md +++ /dev/null @@ -1,52 +0,0 @@ -# `supabase db branch delete` - -## Files Read - -| Path | Format | When | -| -------------------------------- | ------ | ---------------------------------- | -| `/supabase/config.toml` | TOML | always, to resolve local DB config | - -## Files Written - -| Path | Format | When | -| --------------------------------------------------------- | --------- | ------- | -| `/supabase/.branches//` (directory) | directory | removed | - -## API Routes - -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ---- | ---- | ------------ | ---------------------- | -| — | — | — | — | — | - -## Environment Variables - -| Variable | Purpose | Required? | -| -------- | ------- | --------- | -| — | — | — | - -## Exit Codes - -| Code | Condition | -| ---- | -------------------------- | -| `0` | success | -| `1` | branch not found | -| `1` | local database not running | - -## Output - -### `--output-format text` - -Prints a confirmation message to stdout on success. - -### `--output-format json` - -Not applicable. - -### `--output-format stream-json` - -Not applicable. - -## Notes - -- Deprecated: use `branches delete ` instead. -- Requires exactly one positional argument: the branch name. diff --git a/apps/cli/src/commands/db/branch/delete/delete.command.ts b/apps/cli/src/commands/db/branch/delete/delete.command.ts deleted file mode 100644 index cb35a6dbe7..0000000000 --- a/apps/cli/src/commands/db/branch/delete/delete.command.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Argument, Command } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { dbBranchDelete } from "./delete.handler.ts"; - -const config = { - branchName: Argument.string("branch name").pipe( - Argument.withDescription("Name of the branch to delete."), - ), -} as const; - -export type DbBranchDeleteFlags = CliCommand.Command.Config.Infer; - -export const dbBranchDeleteCommand = Command.make("delete", config).pipe( - Command.withDescription("Delete a branch."), - Command.withShortDescription("Delete a branch"), - Command.withHandler((flags) => dbBranchDelete(flags)), -); diff --git a/apps/cli/src/commands/db/branch/delete/delete.handler.ts b/apps/cli/src/commands/db/branch/delete/delete.handler.ts deleted file mode 100644 index dce5cc2c38..0000000000 --- a/apps/cli/src/commands/db/branch/delete/delete.handler.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { Effect } from "effect"; -import { GoProxy } from "../../../../command-internal/go-proxy.service.ts"; -import type { DbBranchDeleteFlags } from "./delete.command.ts"; - -export const dbBranchDelete = Effect.fn("db.branch.delete")(function* (flags: DbBranchDeleteFlags) { - const proxy = yield* GoProxy; - const args: string[] = ["db", "branch", "delete", flags.branchName]; - yield* proxy.exec(args); -}); diff --git a/apps/cli/src/commands/db/branch/list/SIDE_EFFECTS.md b/apps/cli/src/commands/db/branch/list/SIDE_EFFECTS.md deleted file mode 100644 index b5e7aaad88..0000000000 --- a/apps/cli/src/commands/db/branch/list/SIDE_EFFECTS.md +++ /dev/null @@ -1,51 +0,0 @@ -# `supabase db branch list` - -## Files Read - -| Path | Format | When | -| -------------------------------- | ------ | ---------------------------------- | -| `/supabase/config.toml` | TOML | always, to resolve local DB config | - -## Files Written - -| Path | Format | When | -| ---- | ------ | ---- | -| — | — | — | - -## API Routes - -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ---- | ---- | ------------ | ---------------------- | -| — | — | — | — | — | - -## Environment Variables - -| Variable | Purpose | Required? | -| -------- | ------- | --------- | -| — | — | — | - -## Exit Codes - -| Code | Condition | -| ---- | -------------------------- | -| `0` | success | -| `1` | local database not running | - -## Output - -### `--output-format text` - -Prints a list of local branches to stdout. - -### `--output-format json` - -Not applicable. - -### `--output-format stream-json` - -Not applicable. - -## Notes - -- Deprecated: use `branches list` instead. -- This is a local-only operation listing branches in `/supabase/.branches/`. diff --git a/apps/cli/src/commands/db/branch/list/list.command.ts b/apps/cli/src/commands/db/branch/list/list.command.ts deleted file mode 100644 index e53aa60eca..0000000000 --- a/apps/cli/src/commands/db/branch/list/list.command.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { Command } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { dbBranchList } from "./list.handler.ts"; - -const config = {} as const; - -export type DbBranchListFlags = CliCommand.Command.Config.Infer; - -export const dbBranchListCommand = Command.make("list", config).pipe( - Command.withDescription("List branches."), - Command.withShortDescription("List branches"), - Command.withHandler((flags) => dbBranchList(flags)), -); diff --git a/apps/cli/src/commands/db/branch/list/list.handler.ts b/apps/cli/src/commands/db/branch/list/list.handler.ts deleted file mode 100644 index c65ddace3c..0000000000 --- a/apps/cli/src/commands/db/branch/list/list.handler.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { Effect } from "effect"; -import { GoProxy } from "../../../../command-internal/go-proxy.service.ts"; -import type { DbBranchListFlags } from "./list.command.ts"; - -export const dbBranchList = Effect.fn("db.branch.list")(function* (_flags: DbBranchListFlags) { - const proxy = yield* GoProxy; - yield* proxy.exec(["db", "branch", "list"]); -}); diff --git a/apps/cli/src/commands/db/branch/switch/SIDE_EFFECTS.md b/apps/cli/src/commands/db/branch/switch/SIDE_EFFECTS.md deleted file mode 100644 index 7325e7e84f..0000000000 --- a/apps/cli/src/commands/db/branch/switch/SIDE_EFFECTS.md +++ /dev/null @@ -1,52 +0,0 @@ -# `supabase db branch switch` - -## Files Read - -| Path | Format | When | -| -------------------------------- | ------ | ---------------------------------- | -| `/supabase/config.toml` | TOML | always, to resolve local DB config | - -## Files Written - -| Path | Format | When | -| ---------------------------------------------- | ---------- | ------ | -| `/supabase/.branches/_current_branch` | plain text | always | - -## API Routes - -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ---- | ---- | ------------ | ---------------------- | -| — | — | — | — | — | - -## Environment Variables - -| Variable | Purpose | Required? | -| -------- | ------- | --------- | -| — | — | — | - -## Exit Codes - -| Code | Condition | -| ---- | -------------------------- | -| `0` | success | -| `1` | branch not found | -| `1` | local database not running | - -## Output - -### `--output-format text` - -Prints a confirmation message to stdout on success. - -### `--output-format json` - -Not applicable. - -### `--output-format stream-json` - -Not applicable. - -## Notes - -- Deprecated: use `branches create ` instead. -- Requires exactly one positional argument: the branch name to switch to. diff --git a/apps/cli/src/commands/db/branch/switch/switch.command.ts b/apps/cli/src/commands/db/branch/switch/switch.command.ts deleted file mode 100644 index d948f7dcf3..0000000000 --- a/apps/cli/src/commands/db/branch/switch/switch.command.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Argument, Command } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { dbBranchSwitch } from "./switch.handler.ts"; - -const config = { - branchName: Argument.string("branch name").pipe( - Argument.withDescription("Name of the branch to switch to."), - ), -} as const; - -export type DbBranchSwitchFlags = CliCommand.Command.Config.Infer; - -export const dbBranchSwitchCommand = Command.make("switch", config).pipe( - Command.withDescription("Switch the active branch."), - Command.withShortDescription("Switch the active branch"), - Command.withHandler((flags) => dbBranchSwitch(flags)), -); diff --git a/apps/cli/src/commands/db/branch/switch/switch.handler.ts b/apps/cli/src/commands/db/branch/switch/switch.handler.ts deleted file mode 100644 index c631c538f8..0000000000 --- a/apps/cli/src/commands/db/branch/switch/switch.handler.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { Effect } from "effect"; -import { GoProxy } from "../../../../command-internal/go-proxy.service.ts"; -import type { DbBranchSwitchFlags } from "./switch.command.ts"; - -export const dbBranchSwitch = Effect.fn("db.branch.switch")(function* (flags: DbBranchSwitchFlags) { - const proxy = yield* GoProxy; - const args: string[] = ["db", "branch", "switch", flags.branchName]; - yield* proxy.exec(args); -}); diff --git a/apps/cli/src/commands/db/diff/SIDE_EFFECTS.md b/apps/cli/src/commands/db/diff/SIDE_EFFECTS.md index 62b4391641..a21dc3d87c 100644 --- a/apps/cli/src/commands/db/diff/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/diff/SIDE_EFFECTS.md @@ -4,9 +4,9 @@ Native Effect port. Diffs the local project's expected schema (a throwaway shado database) against a target database (local / linked / `--db-url`), using one of three native engines: bundled in-process pg-delta, migra (edge-runtime), or pgAdmin (CLI-1968 — a native `docker run` of the differ container, no -edge-runtime involved). `--use-pg-schema` is the CLI's sole remaining Go -delegation on this command — a documented keep-in-Go exception (CLI-1960), not a -pending port. +edge-runtime involved). `--use-pg-schema` is removed: the flag stays parsed +(hidden) only so using it fails with an actionable removal error instead of an +unknown-flag parse error — see "`--use-pg-schema` is removed" below. When `[experimental].stack` is on, the shadow is `EphemeralPostgres` under `$SUPABASE_HOME/managed/ephemeral-postgres//` (`~/.supabase/managed/…` by default). Migra, pgAdmin, and `--use-pg-schema` are @@ -130,7 +130,8 @@ migra/pg-delta-engine-specific). | ---- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `0` | success; empty diff ("No schema changes found") | | `1` | `--from` without `--to`; engine-flag mutex; target mutex; unknown explicit target; connection/shadow/engine failure; file IO error; local db not running (`--use-pgadmin`); differ container non-zero exit; unparseable `--json-diff` output | -| `1` | `--project-ref` set with a resolved target other than linked; (in explicit mode) `--project-ref` with `--linked` unchanged and neither `--from` nor `--to` being `linked`; `--project-ref` combined with `--use-pg-schema` (see Notes) | +| `1` | `--project-ref` set with a resolved target other than linked; (in explicit mode) `--project-ref` with `--linked` unchanged and neither `--from` nor `--to` being `linked` | +| `1` | `--use-pg-schema` passed at all (any value, including `=false`) — removed, see Notes | ## Output @@ -191,9 +192,10 @@ transaction metadata. ## Notes / Delegation -- `--use-migra` (default), `--use-pgadmin`, `--use-pg-schema`, `--use-pg-delta` are a - mutually-exclusive engine group; `--db-url` / `--linked` / `--local` are a - mutually-exclusive target group (default `--local`). +- `--use-migra` (default), `--use-pgadmin`, `--use-pg-delta` are a mutually-exclusive engine + group; `--db-url` / `--linked` / `--local` are a mutually-exclusive target group (default + `--local`). `--use-pg-schema` is removed and rejects before this group is even checked (see + Notes below), so it is never a live member of the group. - **`--project-ref`** (TS-only, no Go equivalent on any user-facing `db` command) overrides ONLY the linked-ref resolution `ProjectRefResolver` performs (flag > `SUPABASE_PROJECT_ID` > `.temp/project-ref`) — unlike @@ -211,12 +213,8 @@ transaction metadata. (deliberately stricter than `SUPABASE_PROJECT_ID`, which Go's equivalent env var simply leaves unused on a non-linked target). `--use-pgadmin --linked` honors the flag like every other native engine (CLI-1968 — same target - resolve); `--use-pg-schema` rejects it up front, since the delegated Go child - never registered `--project-ref` and the flag would otherwise be silently - dropped. -- `--use-pg-schema` rebuilds the argv and exec's the bundled Go binary (its side - effects are Go's); the Go child's telemetry is disabled so the single - `cli_command_executed` event comes from this TS command. + resolve); `--use-pg-schema` is removed and rejects before any target + resolution happens (see Notes below), so this guard never runs for it. - Explicit `--from`/`--to` mode always uses pg-delta and writes the flattened review representation to `--output` (or stdout). It ignores `--file`; normal mode retains per-unit migration files for the CLI apply paths. @@ -289,39 +287,13 @@ reachable. Reaching both databases requires `--network-id host` **plus** a `[db] 5432` config override — a contrived setup no default user runs. Once both are reachable, this port reports the real diff. -### `--use-pg-schema` is deprecated (CLI-1960) — keep-in-Go exception - -`--use-pg-schema` wraps the in-process Go library `stripe/pg-schema-diff` -(`apps/cli-go/internal/db/diff/pgschema.go`). It is a keep-in-Go exception rather -than a pending port because: - -- it runs **in-process** inside the Go binary, with no container/binary boundary - to re-invoke from TS — unlike `--use-pgadmin` (now native, CLI-1968), which shelled - out to a container/binary path that could in principle be called from TS; -- no TS binding and no WASM build of the library exists, or is reasonably - buildable, within the M9 "Final Cleanup — Go Removal" milestone's scope; -- this specific exception (`db diff --use-pg-schema`) was pre-named when the M9 - milestone was scoped. - -The decision record is Linear issue CLI-1960 and the pull request that introduced -this deprecation notice; re-open only if a TS/WASM binding for -`stripe/pg-schema-diff` ships. It **is** the CLI's sole remaining Go delegation on -`db diff` now that `--use-pgadmin`'s delegation is gone (CLI-1968) — the sibling -`db __db-bootstrap` seam was already removed outright by CLI-1955, and the -`db __shadow` seam by CLI-1956. - -Given that, the flag is now deprecated rather than ported: - -- A TS-only stderr deprecation warning is printed immediately before delegating - (both text and machine `--output-format` modes — diagnostics stay stderr-only, - the CLI-1546 rule): `"--use-pg-schema" is deprecated. Use the pg-delta engine ([experimental.pgdelta] enabled = true / --use-pg-delta) or the default migra engine instead.` - The warning text intentionally does not promise a removal timeline. -- This is **additive** to (printed before) Go's own pre-existing "experimental" - warning (`cmd/db.go:121`, unchanged): `--use-pg-schema flag is experimental and may not include all entities, such as views and grants.` The delegated child - still prints its own warning; the TS wrapper does not suppress or replace it. -- `--help` for the flag now also carries a `Deprecated: …` suffix pointing at the - same migration path. -- Actual flag removal and any PostHog usage-telemetry gate for that removal are - explicitly out of scope for CLI-1960 — this is a documentation/deprecation-notice - change only, tracked as a follow-up decision outside this milestone, with no - owning issue yet. +### `--use-pg-schema` is removed + +The flag is removed: passing it (with any value, including `--use-pg-schema=false`) +fails with a removal error and a suggestion to use the default migra engine or +`--use-pg-delta` instead. It is checked before any other engine-conflict or +target resolution, so combining it with another engine flag (e.g. `--use-pgadmin`) +still hits the removal error first, not the mutex error. + +The flag definition itself stays in `diff.command.ts` (hidden from `--help`) only +so misuse produces this actionable error instead of an unknown-flag parse error. diff --git a/apps/cli/src/commands/db/diff/diff.command.ts b/apps/cli/src/commands/db/diff/diff.command.ts index 3fff3ac3d8..f8b0538ddd 100644 --- a/apps/cli/src/commands/db/diff/diff.command.ts +++ b/apps/cli/src/commands/db/diff/diff.command.ts @@ -19,15 +19,12 @@ const config = { Flag.withDescription("Use pgAdmin to generate schema diff."), Flag.optional, ), + // Kept parsed (and hidden) only so using it produces an actionable removal error instead of + // an unknown-flag parse error; see diff.handler.ts. usePgSchema: Flag.boolean("use-pg-schema").pipe( - // Deprecated in favor of the pg-delta engine (or the default migra engine): pg-schema-diff - // has no TS/container equivalent, so this stays proxied — see SIDE_EFFECTS.md. This - // description-only notice isn't enforced by the flag framework; see diff.handler.ts's - // runtime warning for the enforced half. - Flag.withDescription( - "Use pg-schema-diff to generate schema diff. Deprecated: use the pg-delta engine ([experimental.pgdelta] enabled = true / --use-pg-delta) or the default migra engine instead.", - ), + Flag.withDescription("Removed: use the default migra engine or --use-pg-delta instead."), Flag.optional, + Flag.withHidden, ), usePgDelta: Flag.boolean("use-pg-delta").pipe( Flag.withDescription("Use pg-delta to generate schema diff."), diff --git a/apps/cli/src/commands/db/diff/diff.errors.ts b/apps/cli/src/commands/db/diff/diff.errors.ts index 134c9e15ba..ba5e72e533 100644 --- a/apps/cli/src/commands/db/diff/diff.errors.ts +++ b/apps/cli/src/commands/db/diff/diff.errors.ts @@ -18,8 +18,9 @@ export class DbDiffTargetFlagsError extends Data.TaggedError("DbDiffTargetFlagsE } /** - * Conflicting diff-engine flags (`use-migra`/`use-pgadmin`/`use-pg-schema`/ - * `use-pg-delta`); message text is an established output contract. + * Conflicting diff-engine flags (`use-migra`/`use-pgadmin`/`use-pg-delta`); message text is + * an established output contract. `use-pg-schema` is rejected unconditionally before this + * check runs, so it can never appear in a real conflict. */ export class DbDiffEngineConflictError extends Data.TaggedError("DbDiffEngineConflictError")<{ readonly message: string; diff --git a/apps/cli/src/commands/db/diff/diff.handler.ts b/apps/cli/src/commands/db/diff/diff.handler.ts index f388414c23..7958a2b37d 100644 --- a/apps/cli/src/commands/db/diff/diff.handler.ts +++ b/apps/cli/src/commands/db/diff/diff.handler.ts @@ -6,7 +6,7 @@ import { DnsResolverFlag, NetworkIdFlag, } from "../../../command-internal/global-flags.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; +import { removedFlag } from "../../../command-internal/removed-command.ts"; import { detectGitBranch } from "../../../shared/git/git-branch.ts"; import { Output } from "../../../shared/output/output.service.ts"; import { RuntimeInfo } from "../../../shared/runtime/runtime-info.service.ts"; @@ -24,7 +24,6 @@ import { getHostname } from "../../../command-internal/hostname.ts"; import { makeDir } from "../../../command-internal/make-dir.ts"; import type { PgConnInput } from "../../../command-internal/db-connection.service.ts"; import { toPostgresURL } from "../../../command-internal/postgres-url.ts"; -import { schemaToCsvField } from "../../../command-internal/schema-flags.ts"; import { findDropStatements } from "../../../command-internal/sql-split.ts"; import { buildLocalDbContainerInputs } from "../../../command-internal/db-bootstrap/local-container-inputs.ts"; import { currentStackBackend } from "../../../command-internal/stack-backend.ts"; @@ -89,12 +88,6 @@ import { diffSchemaPgAdmin } from "./pgadmin-diff.ts"; const warnDiff = `WARNING: The diff tool is not foolproof, so you may need to manually rearrange and modify the generated migration. Run ${aqua("supabase db reset")} to verify that the new migration does not generate errors.`; -// `--use-pg-schema` delegates to the bundled Go binary's in-process `stripe/pg-schema-diff` -// library, which has no TS/container equivalent (see SIDE_EFFECTS.md); the flag is deprecated in -// favor of the pg-delta engine. This warning prints before the delegated child's own unchanged -// "experimental" warning. -const warnPgSchemaDeprecated = `${yellow("WARNING:")} "--use-pg-schema" is deprecated. Use the pg-delta engine ([experimental.pgdelta] enabled = true / --use-pg-delta) or the default migra engine instead.`; - const declarativeBaselineAdvisory = (declarativePath: string | null) => ({ code: "DeclarativeSchemaNotUsedAsDiffBaseline", severity: "info", @@ -109,35 +102,13 @@ const declarativeBaselineAdvisory = (declarativePath: string | null) => ({ const declarativeBaselineNote = (displayPath: string) => `Note: db diff -f uses supabase/migrations as its baseline. Declarative schema files in ${displayPath} are not part of that baseline. If migrations are empty or outdated, the generated migration may include existing declarative objects. -f names the migration; it does not filter objects.\n`; -/** - * Rebuilds the `db diff` argv for the `--use-pg-schema` delegate path — the CLI's sole remaining - * Go delegation on this command, since the in-process `stripe/pg-schema-diff` library has no - * TS/container equivalent. The explicit `--from`/`--to` and engine mutex are already handled - * before this runs, so it just forwards `--use-pg-schema` plus the target/schema/file flags. - */ -const rebuildPgSchemaDelegateArgs = (flags: DbDiffFlags): Array => { - const args = ["db", "diff", "--use-pg-schema"]; - const pushTarget = (name: string, value: Option.Option) => { - // The child binary treats an explicitly passed `--flag=false` as selecting that target, so - // forward every explicitly set flag, not just the true ones. - if (Option.isSome(value)) args.push(value.value ? `--${name}` : `--${name}=false`); - }; - if (Option.isSome(flags.dbUrl)) args.push("--db-url", flags.dbUrl.value); - pushTarget("linked", flags.linked); - pushTarget("local", flags.local); - if (Option.isSome(flags.file)) args.push("--file", flags.file.value); - if (Option.isSome(flags.output)) args.push("--output", flags.output.value); - // Re-encoded as a CSV field so the child's pflag CSV parser doesn't re-split a - // comma-containing schema (e.g. "tenant,one"). - for (const s of flags.schema) args.push("--schema", schemaToCsvField(s)); - return args; -}; - export const dbDiff = Effect.fn("db.diff")(function* (flags: DbDiffFlags) { + if (Option.isSome(flags.usePgSchema)) { + return yield* removedFlag("--use-pg-schema", "Use the default migra engine or --use-pg-delta."); + } const output = yield* Output; const resolver = yield* DbConfigResolver; const pgDelta = yield* PgDeltaEngine; - const proxy = yield* GoProxy; const cliSettings = yield* CommandSettings; const telemetryState = yield* TelemetryState; const linkedProjectCache = yield* LinkedProjectCache; @@ -153,18 +124,18 @@ export const dbDiff = Effect.fn("db.diff")(function* (flags: DbDiffFlags) { let linkedRefForCache: string | undefined; yield* Effect.gen(function* () { - // The engine flags (`use-migra use-pgadmin use-pg-schema use-pg-delta`) and the target flags + // The engine flags (`use-migra use-pgadmin use-pg-delta`) and the target flags // (`db-url linked local`) are each mutually exclusive groups; "set" means the flag was - // explicitly passed (`Option.isSome`). + // explicitly passed (`Option.isSome`). `use-pg-schema` is rejected unconditionally above, + // so it never reaches `engineSet`. const engineSet: Array = []; if (Option.isSome(flags.useMigra)) engineSet.push("use-migra"); if (Option.isSome(flags.usePgAdmin)) engineSet.push("use-pgadmin"); - if (Option.isSome(flags.usePgSchema)) engineSet.push("use-pg-schema"); if (Option.isSome(flags.usePgDelta)) engineSet.push("use-pg-delta"); if (engineSet.length > 1) { return yield* Effect.fail( new DbDiffEngineConflictError({ - message: `if any flags in the group [use-migra use-pgadmin use-pg-schema use-pg-delta] are set none of the others can be; [${[...engineSet].sort().join(" ")}] were all set`, + message: `if any flags in the group [use-migra use-pgadmin use-pg-delta] are set none of the others can be; [${[...engineSet].sort().join(" ")}] were all set`, }), ); } @@ -194,7 +165,7 @@ export const dbDiff = Effect.fn("db.diff")(function* (flags: DbDiffFlags) { // Config is read lazily per path, not unconditionally up front: reading the base config // before the ref is known would validate fields a `[remotes.]` block overrides, which - // would fail a linked diff that should succeed. The delegate paths load config themselves. + // would fail a linked diff that should succeed. Explicit mode loads its own config below. // Explicit `--from`/`--to` mode: both required, always pg-delta. An empty value // (a shell var expanding to `""`) counts as unset — `--from "" --to ""` falls @@ -416,48 +387,7 @@ export const dbDiff = Effect.fn("db.diff")(function* (flags: DbDiffFlags) { return; } - // `--use-pg-schema` is an explicit engine selection that doesn't depend on config, so it - // short-circuits before the target resolve (disabling the child's telemetry so only this - // command's instrumentation fires). `--use-pgadmin` doesn't short-circuit: it needs the same - // config validation and target resolve as the other native engines, further down. const usePgAdmin = Option.getOrElse(flags.usePgAdmin, () => false); - const usePgSchema = Option.getOrElse(flags.usePgSchema, () => false); - // The pg-schema engine delegates to the bundled Go binary, whose `db diff` never registered - // `--project-ref`, so forwarding it would silently drop the flag and diff the workdir's own - // linked ref instead. Fail up front rather than risk the wrong project. - if (usePgSchema && Option.isSome(flags.projectRef)) { - return yield* Effect.fail( - new DbDiffTargetFlagsError({ - message: "--project-ref is not supported with --use-pg-schema", - }), - ); - } - if (usePgSchema) { - // TS-only deprecation notice, printed before delegating (diagnostics stay stderr-only in - // every mode). The delegated Go `db diff --use-pg-schema` still prints its own - // experimental warning; this is additive, not a replacement, so don't drop it. - yield* output.raw(`${warnPgSchemaDeprecated}\n`, "stderr"); - const env = { SUPABASE_TELEMETRY_DISABLED: "1" }; - // In machine-output mode the child's stdout is captured and re-emitted as a structured - // envelope, so scripted callers get valid JSON instead of the raw SQL. The delegated - // child owns any `--file` write, so the written path isn't introspectable here (`file: - // null`). - if (output.format !== "text") { - const captured = yield* proxy.execCapture(rebuildPgSchemaDelegateArgs(flags), { - env, - suppressChildTelemetry: true, - }); - yield* output.success("Diff complete.", { - diff: captured, - file: null, - schemas: flags.schema, - engine: "pg-schema", - }); - return; - } - yield* proxy.exec(rebuildPgSchemaDelegateArgs(flags), { env, suppressChildTelemetry: true }); - return; - } // Native path: resolve the target, provision a live shadow source, then diff. const connType: DbConnType = Option.isSome(flags.dbUrl) @@ -551,7 +481,6 @@ export const dbDiff = Effect.fn("db.diff")(function* (flags: DbDiffFlags) { const useDelta = resolveDiffEngine({ useMigraChanged: Option.isSome(flags.useMigra), usePgAdmin, - usePgSchema, pgDeltaDefault, }); // pg-delta ignores schema_paths when building its migrations baseline. diff --git a/apps/cli/src/commands/db/diff/diff.integration.test.ts b/apps/cli/src/commands/db/diff/diff.integration.test.ts index 13ffebec97..272a97d16e 100644 --- a/apps/cli/src/commands/db/diff/diff.integration.test.ts +++ b/apps/cli/src/commands/db/diff/diff.integration.test.ts @@ -32,7 +32,6 @@ import { ExperimentalFlag, NetworkIdFlag, } from "../../../command-internal/global-flags.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; import { ProjectRefNotLinkedError } from "../../../config/project-ref.errors.ts"; import { @@ -61,6 +60,7 @@ import { type PgDeltaExplicitDiffInput, type PgDeltaHazardReport, } from "../shared/pgdelta-engine.service.ts"; +import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; import type { DbDiffFlags } from "./diff.command.ts"; import { dbDiff } from "./diff.handler.ts"; import { stackBackendLayer } from "../../../command-internal/stack-backend.ts"; @@ -78,7 +78,6 @@ interface SetupOpts { readonly diffSuffixes?: ReadonlyArray; readonly hazards?: PgDeltaHazardReport; readonly oom?: boolean; // edge-runtime OOMs; the bash fallback returns `diffSql` - readonly delegateStdout?: string; // stdout returned by a captured Go-delegate run // Message for a failing PGDELTA_DEBUG shadow-catalog export. readonly diffFailWith?: string; // Makes the shadow's PG15+ baseline job(s) exit non-zero; the shadow should @@ -369,18 +368,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(opts.linkedRef ?? VALID_REF), }); - const proxyCalls: Array<{ args: ReadonlyArray; env?: Record }> = []; - const proxyCaptureCalls: Array<{ args: ReadonlyArray; env?: Record }> = - []; - const proxy = Layer.succeed(GoProxy, { - exec: (args, execOpts) => Effect.sync(() => void proxyCalls.push({ args, env: execOpts?.env })), - execCapture: (args, execOpts) => - Effect.sync(() => { - proxyCaptureCalls.push({ args, env: execOpts?.env }); - return opts.delegateStdout ?? ""; - }), - }); - const baseLayer = Layer.mergeAll( // Listed first so the fake service layers below (`Layer.mergeAll` is last-wins) // override its real implementations, matching `start.integration.test.ts`. @@ -397,7 +384,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { alwaysReadyHttpClientLayer, resolver, projectRefResolver, - proxy, mockCommandSettings({ workdir, projectId: opts.projectId ?? Option.some("test") }), Layer.succeed(DnsResolverFlag, "native"), Layer.succeed( @@ -431,8 +417,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { databaseDiffCalls, edgeCalls, resolverCalls, - proxyCalls, - proxyCaptureCalls, dockerCalls, differCalls, differCaptureOpts, @@ -952,48 +936,61 @@ describe("db diff", () => { }, ); + it.effect("diffs with the native pgAdmin engine: shadow create/rm, one differ run", () => { + const s = setup(tmp.current, { pgadminStdout: [JSON.stringify([pgadminEntry()])] }); + return Effect.gen(function* () { + yield* dbDiff(flags({ usePgAdmin: Option.some(true) })); + expect(s.shadowSpawned.filter((c) => c.args[0] === "create")).toHaveLength(1); + expect(s.shadowSpawned.filter((c) => c.args[0] === "rm")).toHaveLength(1); + expect(s.differCalls).toHaveLength(1); + // Status lines go to stdout, not stderr. + expect(stdout(s.out)).toBe( + `Creating shadow database...\nDiffing local database with current migrations...\n${PGADMIN_DIFF_SQL}\n`, + ); + // Stderr carries the shared shadow-setup diagnostics but not pgAdmin's own status + // lines (those are on stdout) or the migra/pg-delta-only status lines. + const err = stderr(s.out); + expect(err).not.toContain("Creating shadow database..."); + expect(err).not.toContain("Diffing local database with current migrations..."); + expect(err).not.toContain("Diffing schemas"); + expect(err).not.toContain("Finished"); + }).pipe(Effect.provide(s.layer)); + }); + it.effect( - "diffs with the native pgAdmin engine: shadow create/rm, one differ run, no Go proxy call", + "rejects --use-pg-schema with a removal error before any engine-conflict or shadow work", () => { - const s = setup(tmp.current, { pgadminStdout: [JSON.stringify([pgadminEntry()])] }); + const s = setup(tmp.current); return Effect.gen(function* () { - yield* dbDiff(flags({ usePgAdmin: Option.some(true) })); - expect(s.proxyCalls).toEqual([]); - expect(s.proxyCaptureCalls).toEqual([]); - expect(s.shadowSpawned.filter((c) => c.args[0] === "create")).toHaveLength(1); - expect(s.shadowSpawned.filter((c) => c.args[0] === "rm")).toHaveLength(1); - expect(s.differCalls).toHaveLength(1); - // Status lines go to stdout, not stderr. - expect(stdout(s.out)).toBe( - `Creating shadow database...\nDiffing local database with current migrations...\n${PGADMIN_DIFF_SQL}\n`, - ); - // Stderr carries the shared shadow-setup diagnostics but not pgAdmin's own status - // lines (those are on stdout) or the migra/pg-delta-only status lines. - const err = stderr(s.out); - expect(err).not.toContain("Creating shadow database..."); - expect(err).not.toContain("Diffing local database with current migrations..."); - expect(err).not.toContain("Diffing schemas"); - expect(err).not.toContain("Finished"); + const error = yield* dbDiff(flags({ usePgSchema: Option.some(true) })).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + expect(s.shadowSpawned).toEqual([]); + expect(s.resolverCalls).toEqual([]); }).pipe(Effect.provide(s.layer)); }, ); - it.effect("rejects --project-ref combined with --use-pg-schema before delegating", () => { - // The delegated Go binary doesn't support --project-ref, so this must fail before - // forwarding rather than silently dropping the flag. - const FLAG_REF = "flagflagflagflagflag"; + it.effect("rejects --use-pg-schema=false the same as an explicit true value", () => { const s = setup(tmp.current); return Effect.gen(function* () { - const exit = yield* Effect.exit( - dbDiff(flags({ usePgSchema: Option.some(true), projectRef: Option.some(FLAG_REF) })), - ); - expect(Exit.isFailure(exit)).toBe(true); - expect(JSON.stringify(exit)).toContain("--project-ref is not supported with --use-pg-schema"); - expect(s.proxyCalls).toEqual([]); - expect(s.proxyCaptureCalls).toEqual([]); + const error = yield* dbDiff(flags({ usePgSchema: Option.some(false) })).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); }).pipe(Effect.provide(s.layer)); }); + it.effect( + "rejects --use-pg-schema combined with --use-pgadmin before the engine-conflict check", + () => { + const s = setup(tmp.current); + return Effect.gen(function* () { + const error = yield* dbDiff( + flags({ usePgSchema: Option.some(true), usePgAdmin: Option.some(true) }), + ).pipe(Effect.flip); + expect(error).toBeInstanceOf(RemovedSurfaceError); + }).pipe(Effect.provide(s.layer)); + }, + ); + it.effect("--use-pgadmin --linked honors --project-ref like the other native engines", () => { const FLAG_REF = "flagflagflagflagflag"; const s = setup(tmp.current, { @@ -1009,7 +1006,6 @@ describe("db diff", () => { projectRef: Option.some(FLAG_REF), }), ); - expect(s.proxyCalls).toEqual([]); expect(s.differCalls).toHaveLength(1); expect(s.cache.cached).toBe(true); expect(s.cache.cachedRef).toBe(FLAG_REF); @@ -1044,7 +1040,6 @@ describe("db diff", () => { return Effect.gen(function* () { yield* dbDiff(flags({ usePgAdmin: Option.some(true), linked: Option.some(true) })); expect(stderr(s.out)).toContain("Loading config override: [remotes.staging]"); - expect(s.proxyCalls).toEqual([]); expect(s.differCalls).toHaveLength(1); }).pipe(Effect.provide(s.layer)); }, @@ -1140,23 +1135,11 @@ describe("db diff", () => { }, ); - it.effect("re-quotes a comma-containing schema when delegating --use-pg-schema", () => { - // The parsed schema value `tenant,one` must be re-encoded as a quoted CSV field - // so the delegated Go child's pflag StringSlice doesn't split it into two schemas. - const s = setup(tmp.current); - return Effect.gen(function* () { - yield* dbDiff(flags({ usePgSchema: Option.some(true), schema: ["tenant,one"] })); - const args = s.proxyCalls[0]?.args ?? []; - const idx = args.indexOf("--schema"); - expect(args[idx + 1]).toBe('"tenant,one"'); - }).pipe(Effect.provide(s.layer)); - }); - it.effect( "forwards a comma-containing --schema value to the differ raw, with no CSV re-quoting (native path)", () => { - // Unlike the delegate above, the native differ argv isn't re-parsed by a pflag - // StringSlice, so the value reaches the container unchanged. + // The native differ argv is never re-parsed by a CSV-splitting flag parser, so the + // value reaches the container unchanged. const s = setup(tmp.current, { pgadminStdout: [JSON.stringify([pgadminEntry()])] }); return Effect.gen(function* () { yield* dbDiff(flags({ usePgAdmin: Option.some(true), schema: ["tenant,one"] })); @@ -1167,42 +1150,6 @@ describe("db diff", () => { }, ); - it.effect( - "delegates --use-pg-schema to the Go binary, printing a deprecation warning without duplicating Go's own warning", - () => { - const s = setup(tmp.current); - return Effect.gen(function* () { - yield* dbDiff(flags({ usePgSchema: Option.some(true) })); - // Asserts on a stable substring so wording tweaks don't require touching every test site. - expect(stderr(s.out)).toContain('"--use-pg-schema" is deprecated'); - expect(stderr(s.out)).not.toContain("--use-pg-schema flag is experimental"); - expect(s.proxyCalls[0]?.args).toEqual(["db", "diff", "--use-pg-schema"]); - // The child's own telemetry is disabled so the single `cli_command_executed` - // event comes from this TS command's instrumentation, not the delegated child. - expect(s.proxyCalls[0]?.env).toEqual({ SUPABASE_TELEMETRY_DISABLED: "1" }); - }).pipe(Effect.provide(s.layer)); - }, - ); - - it.effect("does not print the --use-pg-schema deprecation warning on other diff paths", () => { - const s = setup(tmp.current, { diffSql: "create table g ();\n" }); - return Effect.gen(function* () { - yield* dbDiff(flags()); - expect(stderr(s.out)).not.toContain('"--use-pg-schema" is deprecated'); - }).pipe(Effect.provide(s.layer)); - }); - - it.effect( - "does not print the --use-pg-schema deprecation warning on the native --use-pgadmin path", - () => { - const s = setup(tmp.current, { pgadminStdout: [JSON.stringify([pgadminEntry()])] }); - return Effect.gen(function* () { - yield* dbDiff(flags({ usePgAdmin: Option.some(true) })); - expect(stderr(s.out)).not.toContain('"--use-pg-schema" is deprecated'); - }).pipe(Effect.provide(s.layer)); - }, - ); - it.effect( "emits a json envelope for --use-pgadmin with status lines redirected to stderr (payload-only stdout)", () => { @@ -1216,8 +1163,6 @@ describe("db diff", () => { const err = stderr(s.out); expect(err).toContain("Creating shadow database..."); expect(err).toContain("Diffing local database with current migrations..."); - expect(s.proxyCalls).toEqual([]); - expect(s.proxyCaptureCalls).toEqual([]); const success = s.out.messages.find((m) => m.type === "success"); expect(success?.data).toMatchObject({ diff: PGADMIN_DIFF_SQL, @@ -1261,21 +1206,6 @@ describe("db diff", () => { }).pipe(Effect.provide(s.layer)); }); - it.effect("--use-pg-schema in json mode wraps the captured SQL in a structured envelope", () => { - const s = setup(tmp.current, { format: "json", delegateStdout: "create table e ();\n" }); - return Effect.gen(function* () { - yield* dbDiff(flags({ usePgSchema: Option.some(true) })); - expect(stdout(s.out)).toBe(""); - expect(s.proxyCaptureCalls).toHaveLength(1); - const success = s.out.messages.find((m) => m.type === "success"); - expect(success?.data).toMatchObject({ diff: "create table e ();\n", engine: "pg-schema" }); - // Diagnostics like the deprecation notice still reach stderr in machine mode. - expect(stderr(s.out)).toContain('"--use-pg-schema" is deprecated'); - // The child's own telemetry is disabled here too, same as the text-mode delegate. - expect(s.proxyCaptureCalls[0]?.env).toEqual({ SUPABASE_TELEMETRY_DISABLED: "1" }); - }).pipe(Effect.provide(s.layer)); - }); - it.effect("writes live-only SQL with --file even when declarative targets are configured", () => { mkdirSync(join(tmp.current, "supabase", "schemas"), { recursive: true }); writeFileSync( @@ -1489,19 +1419,6 @@ describe("db diff", () => { }).pipe(Effect.provide(s.layer)); }); - it.effect( - "forwards an explicit --linked=false target flag to the delegated pg-schema child", - () => { - // Target flags are selectors keyed on the delegated child's flag.Changed; dropping - // `Some(false)` would default it to local instead of the linked target selected. - const s = setup(tmp.current); - return Effect.gen(function* () { - yield* dbDiff(flags({ usePgSchema: Option.some(true), linked: Option.some(false) })); - expect(s.proxyCalls[0]?.args).toEqual(["db", "diff", "--use-pg-schema", "--linked=false"]); - }).pipe(Effect.provide(s.layer)); - }, - ); - it.effect( "an empty --file value prints to stdout instead of writing a nameless migration", () => { @@ -2344,7 +2261,7 @@ describe("db diff", () => { flags({ usePgAdmin: Option.some(true), usePgDelta: Option.some(true) }), ).pipe(Effect.flip); expect((error as { message: string }).message).toBe( - "if any flags in the group [use-migra use-pgadmin use-pg-schema use-pg-delta] are set none of the others can be; [use-pg-delta use-pgadmin] were all set", + "if any flags in the group [use-migra use-pgadmin use-pg-delta] are set none of the others can be; [use-pg-delta use-pgadmin] were all set", ); }).pipe(Effect.provide(s.layer)); }, 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 7138fe8c2d..36b07c5c5a 100644 --- a/apps/cli/src/commands/db/pull/pull.integration.test.ts +++ b/apps/cli/src/commands/db/pull/pull.integration.test.ts @@ -32,7 +32,6 @@ import { YesFlag, } from "../../../command-internal/global-flags.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; import { ProjectRefNotLinkedError } from "../../../config/project-ref.errors.ts"; import { @@ -101,7 +100,6 @@ interface SetupOpts { readonly edgeFailFirstWith?: string; // `resolvePoolerFallback` returns `Some(pooler conn)` when true, `None` otherwise. readonly poolerAvailable?: boolean; - readonly delegateStdout?: string; // stdout returned by a captured Go-delegate run // Initial-migra pull: the bytes the native pg_dump container streams to its sink, its // exit code/stderr, and (when set) an IPv6 stderr that fails the first dump attempt so // the pooler retry runs (the second attempt then streams `dumpStdout`). @@ -389,21 +387,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { }, }); - const proxyCalls: Array<{ args: ReadonlyArray; env?: Record }> = []; - const proxyCaptureCalls: Array<{ - args: ReadonlyArray; - env?: Record; - stdin?: "inherit" | "ignore"; - }> = []; - const proxy = Layer.succeed(GoProxy, { - exec: (args, execOpts) => Effect.sync(() => void proxyCalls.push({ args, env: execOpts?.env })), - execCapture: (args, execOpts) => - Effect.sync(() => { - proxyCaptureCalls.push({ args, env: execOpts?.env, stdin: execOpts?.stdin }); - return opts.delegateStdout ?? ""; - }), - }); - // Mirrors the same ref `resolver`'s own mock embeds above, and gives an explicit // `--project-ref` flag top precedence over `opts.resolvedRef` (mirrors // `reset.integration.test.ts`'s identical mock). @@ -438,7 +421,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { alwaysReadyHttpClientLayer, resolver, projectRefResolver, - proxy, mockCommandSettings({ workdir, projectId: opts.projectId ?? Option.some("test") }), mockTty({ stdinIsTty: opts.stdinIsTty ?? false, stdoutIsTty: false }), mockStdin( @@ -460,8 +442,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { return { layer: baseLayer, out, - proxyCalls, - proxyCaptureCalls, historyUpserts, execLog, connectedDatabases, @@ -620,7 +600,6 @@ describe("db pull", () => { yield* dbPull(flags({ projectRef: Option.some(FLAG_REF) })); expect(s.engineCalls[0]?.operation).toBe("export"); expect(s.engineCalls[0]?.projectRef).toBe(FLAG_REF); - expect(s.proxyCalls).toEqual([]); }).pipe(Effect.provide(s.layer)); }); @@ -1025,8 +1004,6 @@ describe("db pull", () => { }); return Effect.gen(function* () { yield* dbPull(flags()); - expect(s.proxyCalls).toHaveLength(0); - expect(s.proxyCaptureCalls).toHaveLength(0); // pg_dump ran with the schema-dump env (internal-schema exclude + comment strip). expect(s.dumpCalls).toHaveLength(1); expect(s.dumpCalls[0]?.env["EXTRA_SED"]).toBe("/^--/d"); @@ -1065,8 +1042,6 @@ describe("db pull", () => { }); return Effect.gen(function* () { yield* dbPull(flags()); - expect(s.proxyCalls).toHaveLength(0); - expect(s.proxyCaptureCalls).toHaveLength(0); const success = s.out.messages.find((m) => m.type === "success"); // Machine mode never prompts, so history updates by default (true); `schemaWritten` // is the real native path (not null as when delegated). @@ -1623,7 +1598,6 @@ describe("db pull", () => { if (prev === undefined) delete process.env["SUPABASE_EXPERIMENTAL"]; else process.env["SUPABASE_EXPERIMENTAL"] = prev; } - expect(s.proxyCalls).toHaveLength(0); expect(s.engineCalls[0]?.operation).toBe("export"); expect(streamText(s.out, "stderr")).toContain("Connecting to remote database..."); expect(streamText(s.out, "stderr")).toContain( @@ -1684,7 +1658,6 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags({ declarative: Option.some(true), usePgDelta: Option.some(false) })); expect(s.engineCalls[0]?.operation).toBe("export"); - expect(s.proxyCalls).toHaveLength(0); expect(streamText(s.out, "stderr")).toContain( "The --experimental structured-dump mode for `db pull` is deprecated", ); @@ -1696,7 +1669,6 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags({ diffEngine: Option.some("migra") })); expect(s.engineCalls[0]?.operation).toBe("export"); - expect(s.proxyCalls).toHaveLength(0); }).pipe(Effect.provide(s.layer)); }); @@ -1706,7 +1678,6 @@ describe("db pull", () => { const s = setup(tmp.current, { experimental: true, edgeStdout: EXPORT_JSON }); return Effect.gen(function* () { yield* dbPull(flags()); - expect(s.proxyCalls).toHaveLength(0); expect(s.engineCalls[0]?.operation).toBe("export"); expect(streamText(s.out, "stderr")).toContain("Connecting to remote database..."); expect(streamText(s.out, "stderr")).toContain( @@ -1726,7 +1697,6 @@ describe("db pull", () => { }); return Effect.gen(function* () { yield* dbPull(flags()); - expect(s.proxyCaptureCalls).toHaveLength(0); const success = s.out.messages.find((m) => m.type === "success"); expect(success?.data).toMatchObject({ declarative: true, @@ -1748,7 +1718,6 @@ describe("db pull", () => { expect(streamText(s.out, "stderr")).toContain( "Preparing declarative schema export using pg-delta...", ); - expect(s.proxyCalls).toHaveLength(0); expect(streamText(s.out, "stderr")).not.toContain("is deprecated"); }).pipe(Effect.provide(s.layer)); }, @@ -1799,7 +1768,6 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags({ name: Option.some("--experimental=false") })); expect(s.engineCalls[0]?.operation).toBe("export"); - expect(s.proxyCalls).toHaveLength(0); }).pipe( Effect.ensuring( Effect.sync(() => { @@ -1825,7 +1793,6 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags()); expect(s.engineCalls[0]?.operation).toBe("export"); - expect(s.proxyCalls).toHaveLength(0); }).pipe(Effect.provide(s.layer)); }, ); @@ -1845,7 +1812,6 @@ describe("db pull", () => { return Effect.gen(function* () { yield* dbPull(flags()); expect(s.engineCalls[0]?.operation).toBe("export"); - expect(s.proxyCalls).toHaveLength(0); }).pipe( Effect.ensuring( Effect.sync(() => { diff --git a/apps/cli/src/commands/db/remote/changes/SIDE_EFFECTS.md b/apps/cli/src/commands/db/remote/changes/SIDE_EFFECTS.md index 619062b43d..8c2f7df9e0 100644 --- a/apps/cli/src/commands/db/remote/changes/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/remote/changes/SIDE_EFFECTS.md @@ -1,10 +1,13 @@ # `supabase db remote changes` +Removed. The command is a tombstone: it fails with a removal error and a +replacement suggestion instead of connecting to any database. + ## Files Read -| Path | Format | When | -| -------------------------- | ---------- | ---------------------------------- | -| `~/.supabase/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset | +| Path | Format | When | +| ---- | ------ | ---- | +| — | — | — | ## Files Written @@ -20,34 +23,41 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | --------------------------------------- | ------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token | no (falls back to keyring → `~/.supabase/access-token`) | -| `DB_PASSWORD` | password for direct database connection | no | +| Variable | Purpose | Required? | +| -------- | ------- | --------- | +| — | — | — | ## Exit Codes -| Code | Condition | -| ---- | --------------------------- | -| `0` | success | -| `1` | database connection failure | +| Code | Condition | +| ---- | ---------------------------------- | +| `1` | every invocation (removed command) | ## Output ### `--output-format text` -Prints the schema diff (changes on the remote database since the last migration) to stdout. +Writes the removal message and the replacement suggestion to stderr, two lines, +regardless of `-o`/`--output`: + +``` +supabase db remote changes was removed. +Use `supabase db diff --linked` instead. +``` -### `--output-format json` +### `--output-format json` / `stream-json` -Not applicable. +Emits the JSON error envelope on stdout instead of the stderr lines above; the +envelope's `code` is `RemovedSurfaceError` and `suggestion` carries the same +replacement text. -### `--output-format stream-json` +## Telemetry Events Fired -Not applicable. +One `cli_command_executed` event per invocation, with `exit_code: 1` and an +`error_fingerprint` ending in `:removed_command` (`RemovedSurfaceError`). ## Notes -- Deprecated: use `db diff --use-migra --linked` instead. -- `--schema` / `-s` restricts the diff to specific schemas. -- `--db-url` and `--linked` (default true) are mutually exclusive. +- No flag value is read; the command fails identically regardless of + `--schema`/`--db-url`/`--linked`/`--password`. +- The native sibling `db remote commit` is unaffected. diff --git a/apps/cli/src/commands/db/remote/changes/changes.command.ts b/apps/cli/src/commands/db/remote/changes/changes.command.ts index 2f30f047ec..2dc52a8d3d 100644 --- a/apps/cli/src/commands/db/remote/changes/changes.command.ts +++ b/apps/cli/src/commands/db/remote/changes/changes.command.ts @@ -1,6 +1,8 @@ import { Command, Flag } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { dbRemoteChanges } from "./changes.handler.ts"; +import { removedCommand } 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"; const config = { schema: Flag.string("schema").pipe( @@ -23,10 +25,14 @@ const config = { ), } as const; -export type DbRemoteChangesFlags = CliCommand.Command.Config.Infer; - export const dbRemoteChangesCommand = Command.make("changes", config).pipe( - Command.withDescription("Show changes on the remote database since last migration."), - Command.withShortDescription("Show changes on the remote database"), - Command.withHandler((flags) => dbRemoteChanges(flags)), + Command.withDescription("Removed: use `supabase db diff --linked` instead."), + Command.withShortDescription("Removed: use `db diff --linked` instead"), + Command.withHandler(() => + removedCommand("Use `supabase db diff --linked` instead.").pipe( + withCommandTelemetry(), + withJsonErrorHandling, + ), + ), + Command.provide(commandRuntimeLayer(["db", "remote", "changes"])), ); diff --git a/apps/cli/src/commands/db/remote/changes/changes.handler.ts b/apps/cli/src/commands/db/remote/changes/changes.handler.ts deleted file mode 100644 index b7b6db5eb5..0000000000 --- a/apps/cli/src/commands/db/remote/changes/changes.handler.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Effect, Option } from "effect"; -import { GoProxy } from "../../../../command-internal/go-proxy.service.ts"; -import type { DbRemoteChangesFlags } from "./changes.command.ts"; - -export const dbRemoteChanges = Effect.fn("db.remote.changes")(function* ( - flags: DbRemoteChangesFlags, -) { - const proxy = yield* GoProxy; - const args: string[] = ["db", "remote", "changes"]; - for (const s of flags.schema) { - args.push("--schema", s); - } - if (Option.isSome(flags.dbUrl)) args.push("--db-url", flags.dbUrl.value); - if (flags.linked) args.push("--linked"); - if (Option.isSome(flags.password)) args.push("--password", flags.password.value); - yield* proxy.exec(args); -}); diff --git a/apps/cli/src/commands/db/schema/declarative/generate/generate.integration.test.ts b/apps/cli/src/commands/db/schema/declarative/generate/generate.integration.test.ts index bf68e2a5fb..2ca7cb6dba 100644 --- a/apps/cli/src/commands/db/schema/declarative/generate/generate.integration.test.ts +++ b/apps/cli/src/commands/db/schema/declarative/generate/generate.integration.test.ts @@ -37,7 +37,6 @@ import { NetworkIdFlag, YesFlag, } from "../../../../../command-internal/global-flags.ts"; -import { GoProxy } from "../../../../../command-internal/go-proxy.service.ts"; import { CommandPlatformApi } from "../../../../../auth/command-platform-api.service.ts"; import { CommandPlatformApiFactory } from "../../../../../auth/command-platform-api-factory.service.ts"; import { dockerRunLayer } from "../../../../../command-internal/docker-run.layer.ts"; @@ -198,11 +197,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { }, resolvePoolerFallback: () => Effect.succeed(Option.none()), }); - const proxyCalls: ReadonlyArray[] = []; - const proxy = Layer.succeed(GoProxy, { - exec: (args) => Effect.sync(() => void proxyCalls.push(args)), - execCapture: () => Effect.succeed(""), - }); const runtimeInfo = mockRuntimeInfo({ platform: "linux" }); const processControl = mockProcessControl(); const experimentalFlag = Layer.succeed(ExperimentalFlag, opts.experimental ?? true); @@ -223,7 +217,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { engine, mockLocalDockerEngineUnavailableLayer, resolver, - proxy, dbConn, mockCommandSettings({ workdir, projectId: opts.projectId ?? Option.some("test") }), mockTty({ stdinIsTty: opts.stdinIsTty ?? false, stdoutIsTty: false }), @@ -259,7 +252,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { dbExec, engineExportCalls, resolverCalls, - proxyCalls, localPostgresImageChecks, get ensureStartedCalls() { return ensureStartedCalls; diff --git a/apps/cli/src/commands/db/test/test.integration.test.ts b/apps/cli/src/commands/db/test/test.integration.test.ts index d388262b43..a355c932ee 100644 --- a/apps/cli/src/commands/db/test/test.integration.test.ts +++ b/apps/cli/src/commands/db/test/test.integration.test.ts @@ -52,7 +52,6 @@ import { DockerRun, type DockerRunOpts } from "../../../command-internal/docker- import { EdgeRuntimeScript } from "../../../command-internal/edge-runtime-script.service.ts"; import { PgDeltaSslProbe } from "../../../command-internal/pgdelta-ssl-probe.service.ts"; import { runTestDbCommand } from "../../../command-internal/test-db.command-handler.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; import { dbCommand } from "../db.command.ts"; const LOCAL_CONN: PgConnInput = { @@ -228,8 +227,7 @@ describe("db test (alias) integration", () => { Stdio.layerTest({ args: Effect.succeed(args) }), Layer.succeed(CliArgs, { args }), // `dbCommand` is the whole `db` subtree, so its R includes every sibling subcommand's - // global-flag/Go-delegation requirements too, even though this test only dispatches - // `db test`. + // global-flag requirements too, even though this test only dispatches `db test`. Layer.succeed(AgentFlag, "auto"), Layer.succeed(CreateTicketFlag, false), Layer.succeed(DebugFlag, false), @@ -240,10 +238,6 @@ describe("db test (alias) integration", () => { Layer.succeed(ProfileFlag, "supabase"), Layer.succeed(WorkdirFlag, Option.none()), Layer.succeed(YesFlag, false), - Layer.succeed(GoProxy, { - exec: () => Effect.die("GoProxy not needed for `db test` dispatch"), - execCapture: () => Effect.die("GoProxy not needed for `db test` dispatch"), - }), Layer.succeed(EdgeRuntimeScript, { run: () => Effect.die("EdgeRuntimeScript not needed for `db test` dispatch"), }), diff --git a/apps/cli/src/commands/functions/download/SIDE_EFFECTS.md b/apps/cli/src/commands/functions/download/SIDE_EFFECTS.md index 94c7037995..35bc4aa2fd 100644 --- a/apps/cli/src/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/functions/download/SIDE_EFFECTS.md @@ -42,17 +42,9 @@ | `docker pull ` | Docker-unbundle path, cache miss on a candidate | pull with 2 retries (4s/8s backoff) before falling through to the next registry candidate | | `docker network inspect` / `network create` / `volume create` | Docker-unbundle path, when Docker is running | ensure the shared per-project network/named volume exist (same primitives as `functions deploy`'s Docker bundler); the Deno-cache volume is `supabase_edge_runtime_` (mounted at `/root/.cache/deno`) | | `docker run --rm ... --label com.supabase.cli.project= --label com.docker.compose.project= unbundle --eszip ... --output ...` | Docker-unbundle path | extract the downloaded eszip into `supabase/functions//...`; labeled so orphaned containers can be associated with the project | -| `supabase-go functions download ... --legacy-bundle` | `--legacy-bundle` only | preserve the hidden, deprecated pre-1.120.0 bundling fallback (native TS port tracked separately, CLI-1963) | - -The `--legacy-bundle` delegated call runs with `SUPABASE_TELEMETRY_DISABLED=1` -so the Go child's own `cli_command_executed` doesn't double-count on top of -this command's own telemetry (mirrors `db pull`/`db diff`'s delegated-call -pattern). In `--output-format json|stream-json`, the child's stdout is -captured and discarded instead of inherited (`GoProxy.execCapture`) — -the raw text never reaches the terminal, and this command emits the `Output` -envelope itself once the child exits successfully. The Docker-unbundle path's -own container stdout is routed the same way: to the real stdout in text mode, -to stderr in machine-output modes (CLI-1546). + +The Docker-unbundle path's own container stdout is routed to the real stdout +in text mode, to stderr in machine-output modes (CLI-1546). ## Environment Variables @@ -73,20 +65,21 @@ to stderr in machine-output modes (CLI-1546). ## Exit Codes -| Code | Condition | -| ---- | ---------------------------------------------------------------------- | -| `0` | success | -| `1` | API error (non-2xx response) | -| `1` | authentication error (no token found) | -| `1` | network / connection failure | -| `1` | invalid function slug or flag conflict | -| `1` | Docker-unbundle container exited non-zero (suggests `--legacy-bundle`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | API error (non-2xx response) | +| `1` | authentication error (no token found) | +| `1` | network / connection failure | +| `1` | invalid function slug or flag conflict | +| `1` | Docker-unbundle container exited non-zero (suggests retrying with `--use-api`, or redeploying if that also fails) | +| `1` | `--legacy-bundle` 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` (`project-ref` recorded verbatim, matching `functions list`/`delete`; `use-api`/`use-docker`/`legacy-bundle` also recorded verbatim since they are boolean flags; no flag on this command is currently redacted) | +| Event | When | Notable properties / groups | +| ---------------------- | ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` (`project-ref` recorded verbatim, matching `functions list`/`delete`; `use-api`/`use-docker`/`legacy-bundle` also recorded verbatim since they are boolean flags; no flag on this command is currently redacted); a rejected `--legacy-bundle` fails with `error_fingerprint` ending in `:removed_flag` (`RemovedSurfaceError`) | ## Output @@ -95,19 +88,17 @@ to stderr in machine-output modes (CLI-1546). Prints progress and success messages as functions are downloaded. The Docker-unbundle path prints `Downloading function: ` (lowercase "function", unlike the `--use-api` path's "Downloading Function:") and does **not** print a final "Downloaded Function ... from project ..." line — that -line only appears on the `--use-api` and `--legacy-bundle` paths. +line only appears on the `--use-api` path. ### `--output-format json` Prints a structured success result with the downloaded function slugs and project ref. On the -`--legacy-bundle` proxy path, the Go child's stdout is captured/discarded (never inherited) so it -can't corrupt the envelope; the slug list is resolved independently for the payload. On the Docker-unbundle path, the `unbundle` container's own stdout is routed to stderr instead of stdout -for the same reason. +so it can't corrupt the envelope. ### `--output-format stream-json` -Same envelope as `json` above (including on the proxy and Docker-unbundle paths). +Same envelope as `json` above (including on the Docker-unbundle path). ## Notes @@ -120,11 +111,8 @@ Same envelope as `json` above (including on the proxy and Docker-unbundle paths) gap, not one introduced by this port. Slugs sourced from the Management API's function list (downloading-all) are validated against the same pattern as user-supplied slugs, on both paths, before any per-slug download runs (CLI-1891). -- `--legacy-bundle` is a hidden flag forwarded to the Go binary for backward compatibility — it - requires installing a real Deno binary on the host (`InstallOrUpgradeDeno`) and is a pre-1.120.0 - compatibility fallback; native TS port tracked separately (CLI-1963). `--use-docker` is a hidden - flag but now runs natively. -- `--use-docker`, `--use-api`, and `--legacy-bundle` are mutually exclusive. +- `--use-docker` is a hidden flag but runs natively. +- `--use-docker` and `--use-api` are mutually exclusive. - `--use-docker` defaults to `true`, so a bare `supabase functions download` runs the native Docker-unbundle downloader unless `--use-api` resolves to `true`, which forces the native server-side download path instead (the resolved flag value is what's checked, not presence — @@ -134,7 +122,32 @@ Same envelope as `json` above (including on the proxy and Docker-unbundle paths) installed or running. - The mutual-exclusivity check only counts flags the user explicitly passed on the command line, not `--use-docker`'s default value — so `--use-api` alone never trips the "mutually exclusive" - error. The `--legacy-bundle` Go proxy call itself only ever forwards `--legacy-bundle`, never - `--use-docker` alongside it, even though `--use-docker` defaults to `true`. + error. - Refreshes the linked-project telemetry cache and flushes telemetry state after resolving a project ref. + +### `--legacy-bundle` is removed + +The flag is removed: passing it (with any value, including `--legacy-bundle=false`) fails +with a removal error before any download, Docker, or API work runs. The suggestion reads: + +``` +Retry with `supabase functions download --use-api ` to unbundle server-side without Docker. +``` + +It is checked before `--use-api`/`--use-docker` mutual-exclusivity validation, so combining +it with either still hits the removal error first. + +The flag definition itself stays in `download.command.ts` (hidden from `--help`) only so +misuse produces this actionable error, with `error_fingerprint` ending in `:removed_flag` +(`RemovedSurfaceError`), instead of an unknown-flag parse error with no telemetry at all. + +### Docker-extraction failure suggestion + +Any Docker-unbundle failure (network/volume creation, container create/start, log +streaming, or a non-zero container exit) prints a suggestion leading with the +Docker-avoiding retry: + +``` +Retry with supabase functions download --use-api to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI. +``` diff --git a/apps/cli/src/commands/functions/download/download.e2e.test.ts b/apps/cli/src/commands/functions/download/download.e2e.test.ts index 83718fdc19..4759564803 100644 --- a/apps/cli/src/commands/functions/download/download.e2e.test.ts +++ b/apps/cli/src/commands/functions/download/download.e2e.test.ts @@ -13,8 +13,6 @@ const FAKE_REF = "a".repeat(20); describe("supabase functions download — argument validation", () => { const conflicts = [ { name: "--use-api + --use-docker", flags: ["--use-api", "--use-docker"] }, - { name: "--use-api + --legacy-bundle", flags: ["--use-api", "--legacy-bundle"] }, - { name: "--use-docker + --legacy-bundle", flags: ["--use-docker", "--legacy-bundle"] }, ] as const; for (const { name, flags } of conflicts) { @@ -35,8 +33,4 @@ describe("supabase functions download — argument validation", () => { // `--use-api` alone (without --use-docker) is covered in // download.integration.test.ts instead, since validating it here would // require a real network round-trip to the Management API. - - // `--legacy-bundle` alone is covered in download.integration.test.ts - // instead: it routes to the Go binary's downloader, which would trigger a - // real Deno download from GitHub on every e2e run. }); diff --git a/apps/cli/src/commands/functions/download/download.handler.ts b/apps/cli/src/commands/functions/download/download.handler.ts index 38f9b941fd..b25373c168 100644 --- a/apps/cli/src/commands/functions/download/download.handler.ts +++ b/apps/cli/src/commands/functions/download/download.handler.ts @@ -1,14 +1,11 @@ import { join } from "node:path"; import { Effect, Option, Stdio } from "effect"; -import { - downloadFunctions, - makeGoProxyLegacyBundleArgs, -} from "../../../shared/functions/download.ts"; +import { downloadFunctions } from "../../../shared/functions/download.ts"; import { resolveEdgeRuntimeVersionPin } from "../../../shared/functions/functions.shared.ts"; import { Output } from "../../../shared/output/output.service.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; import { aqua, bold, yellow } from "../../../command-internal/colors.ts"; import { functionsGoConfigCompat } from "../../../command-internal/functions-go-config.ts"; +import { removedFlag } from "../../../command-internal/removed-command.ts"; import { CommandPlatformApi } from "../../../auth/command-platform-api.service.ts"; import { CommandSettings } from "../../../config/command-settings.service.ts"; import { ProjectRefResolver } from "../../../config/project-ref.service.ts"; @@ -19,12 +16,17 @@ import type { FunctionsDownloadFlags } from "./download.command.ts"; export const functionsDownload = Effect.fn("functions.download")(function* ( flags: FunctionsDownloadFlags, ) { + if (flags.legacyBundle) { + return yield* removedFlag( + "--legacy-bundle", + "Retry with `supabase functions download --use-api ` to unbundle server-side without Docker.", + ); + } const api = yield* CommandPlatformApi; const cliSettings = yield* CommandSettings; const resolver = yield* ProjectRefResolver; const linkedProjectCache = yield* LinkedProjectCache; const telemetryState = yield* TelemetryState; - const proxy = yield* GoProxy; const output = yield* Output; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; @@ -54,26 +56,8 @@ export const functionsDownload = Effect.fn("functions.download")(function* ( }), ), ), - // Suppresses the delegated binary's own `cli_command_executed` so a - // proxied invocation fires exactly one event. In machine-output mode its - // stdout is captured and discarded instead of inherited, since - // `downloadFunctions` emits the `Output` envelope itself. - proxyDownload: (proxyFlags, projectRef, captureOutput) => { - const args = makeGoProxyLegacyBundleArgs(proxyFlags.functionName, projectRef); - const env = { SUPABASE_TELEMETRY_DISABLED: "1" }; - return captureOutput - ? Effect.asVoid( - proxy.execCapture(args, { env, stdin: "ignore", suppressChildTelemetry: true }), - ) - : proxy.exec(args, { env, suppressChildTelemetry: true }); - }, }); - // `--legacy-bundle` emits its own final summary inside `downloadFunctions`. - if (flags.legacyBundle) { - return; - } - if (result.empty) { if (output.format === "text") { yield* output.raw(`No functions found in project ${result.projectRef}\n`, "stderr"); diff --git a/apps/cli/src/commands/functions/download/download.integration.test.ts b/apps/cli/src/commands/functions/download/download.integration.test.ts index 5e7e56d9db..9878042798 100644 --- a/apps/cli/src/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/commands/functions/download/download.integration.test.ts @@ -11,6 +11,7 @@ import { Layer, Option, PlatformError, + Runtime, Sink, Stdio, Stream, @@ -32,8 +33,8 @@ import { } from "../../../../tests/helpers/command-mocks.ts"; import { mockOutput } from "../../../../tests/helpers/mocks.ts"; import { mockChildProcessSpawner } from "../../../../tests/helpers/child-process-spawner.ts"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; import { containerRuntimeNotFoundMessage } from "../../../command-internal/container-cli.ts"; +import { RemovedSurfaceError } from "../../../command-internal/removed-command.ts"; import { downloadFunctions } from "../../../shared/functions/download.ts"; import { functionsGoConfigCompat } from "../../../command-internal/functions-go-config.ts"; import { CommandPlatformApi } from "../../../auth/command-platform-api.service.ts"; @@ -179,32 +180,6 @@ function multipartResponse(request: Parameters> = []; - const envs: Array | undefined> = []; - const captureCalls: Array> = []; - const captureEnvs: Array | undefined> = []; - return { - calls, - envs, - captureCalls, - captureEnvs, - layer: Layer.succeed(GoProxy, { - exec: (args, opts) => - Effect.sync(() => { - calls.push([...args]); - envs.push(opts?.env); - }), - execCapture: (args, opts) => - Effect.sync(() => { - captureCalls.push([...args]); - captureEnvs.push(opts?.env); - return ""; - }), - }), - }; -} - describe("functions download", () => { it.live("downloads a function natively into the legacy workdir", () => { const out = mockOutput({ format: "text" }); @@ -214,7 +189,6 @@ describe("functions download", () => { ? Effect.succeed(multipartResponse(request)) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const linkedProjectCache = mockLinkedProjectCacheTracked(); const telemetry = mockTelemetryStateTracked(); const layer = Layer.mergeAll( @@ -225,7 +199,6 @@ describe("functions download", () => { linkedProjectCache: linkedProjectCache.layer, telemetry: telemetry.layer, }), - proxy.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", @@ -240,7 +213,6 @@ describe("functions download", () => { return Effect.gen(function* () { yield* functionsDownload(baseFlags); - expect(proxy.calls).toEqual([]); expect( yield* Effect.tryPromise(() => readFile( @@ -262,7 +234,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); // Non-empty stdout/stderr exercises both the text-mode stdout routing // and always-to-stderr branches in `downloadWithDockerUnbundle`. const child = mockDockerUnbundle({ @@ -275,7 +246,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -291,8 +261,6 @@ describe("functions download", () => { return Effect.gen(function* () { yield* functionsDownload({ ...baseFlags, useDocker: true }); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); expect(api.requests.some((request) => request.url.endsWith("/hello-world/body"))).toBe( true, ); @@ -304,8 +272,8 @@ describe("functions download", () => { expect(out.stderrText).toContain("Downloading function: hello-world\n"); expect(out.stdoutText).toContain("unbundle: wrote index.ts\n"); expect(out.stderrText).toContain("unbundle: warning about deno.json\n"); - // Unlike the server-side and --legacy-bundle paths, the native Docker - // path never prints a "Downloaded Function ..." success line. + // Unlike the server-side path, the native Docker path never prints a + // "Downloaded Function ..." success line. expect(out.stderrText).not.toContain("Downloaded Function"); // No `--debug` — the temp eszip file is removed after the run. expect( @@ -325,14 +293,12 @@ describe("functions download", () => { ? Effect.succeed(multipartResponse(request)) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const layer = Layer.mergeAll( buildTestRuntime({ out, api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", @@ -350,7 +316,6 @@ describe("functions download", () => { // the explicit `--use-api`. yield* functionsDownload({ ...baseFlags, useApi: true, useDocker: true }); - expect(proxy.calls).toEqual([]); expect( yield* Effect.tryPromise(() => readFile( @@ -368,7 +333,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -376,7 +340,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -395,8 +358,6 @@ describe("functions download", () => { // `--use-api=false` leaves `--use-docker`'s own default (true) in effect. yield* functionsDownload({ ...baseFlags, useApi: false, useDocker: true }); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); expect( child.spawned.some( (spawned) => spawned.command === "docker" && spawned.args[0] === "run", @@ -411,7 +372,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "json" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); // Non-empty container stdout exercises the machine-mode branch that // routes it to stderr, keeping stdout payload-only. const child = mockDockerUnbundle({ runStdout: ["unbundle: wrote index.ts"] }); @@ -421,7 +381,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -439,8 +398,6 @@ describe("functions download", () => { return Effect.gen(function* () { yield* functionsDownload({ ...baseFlags, useDocker: true }); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); expect( child.spawned.some( (spawned) => spawned.command === "docker" && spawned.args[0] === "run", @@ -468,7 +425,6 @@ describe("functions download", () => { ) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -476,7 +432,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -497,8 +452,6 @@ describe("functions download", () => { useDocker: true, }); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); expect( child.spawned.filter( (spawned) => spawned.command === "docker" && spawned.args[0] === "run", @@ -529,7 +482,6 @@ describe("functions download", () => { it.live("runs docker with the expected binds, network, and unbundle command", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -537,7 +489,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -610,7 +561,6 @@ describe("functions download", () => { // `deploy.ts`'s `buildDockerBinds` applies the same carve-out. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -618,7 +568,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -664,7 +613,6 @@ describe("functions download", () => { // risk a negotiated response instead of the raw eszip body. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -672,7 +620,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -697,7 +644,6 @@ describe("functions download", () => { it.live("uses an explicit --network-id override instead of the derived network name", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -705,7 +651,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -742,7 +687,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -750,7 +694,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -781,7 +724,6 @@ describe("functions download", () => { it.live("honors the final occurrence of a repeated --network-id flag", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -789,7 +731,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -821,7 +762,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -829,7 +769,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -864,7 +803,6 @@ describe("functions download", () => { // `--project-ref` rather than an ancestor's `project_id`. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const nestedWorkdir = join(tempRoot.current, "nested"); const layer = Layer.mergeAll( @@ -873,7 +811,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: nestedWorkdir }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -917,7 +854,6 @@ describe("functions download", () => { // both files resolves `project_id` from config.toml, not config.json. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -925,7 +861,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -970,7 +905,6 @@ describe("functions download", () => { // passed straight through to `docker run --network`. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -978,7 +912,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1009,7 +942,6 @@ describe("functions download", () => { // must not get a second `v`. const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1017,7 +949,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1049,7 +980,6 @@ describe("functions download", () => { it.live("keeps the temporary eszip file when --debug is passed", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1057,7 +987,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1086,7 +1015,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1094,7 +1022,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1124,7 +1051,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); // Every docker command (including the `docker info` probe) fails, // modeling Docker not running. const child = mockChildProcessSpawner({ exitCode: 1 }); @@ -1134,7 +1060,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1170,10 +1095,9 @@ describe("functions download", () => { ); describe("docker unbundle container failures", () => { - it.live("fails with the legacy-bundle suggestion when the container exits non-zero", () => { + it.live("fails with the redeploy suggestion when the container exits non-zero", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["boom"] }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1181,7 +1105,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1201,7 +1124,7 @@ describe("functions download", () => { expect(error).toBeInstanceOf(Error); expect((error as Error).message).toBe("error running container: exit 1"); expect((error as Error & { suggestion?: string }).suggestion).toBe( - "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + "\nRetry with supabase functions download --use-api hello-world to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI.", ); }).pipe(Effect.provide(layer)); }); @@ -1211,7 +1134,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockDockerUnbundle({ runExitCode: 1, // Full-line, case-insensitive match required; a substring like @@ -1224,7 +1146,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1257,7 +1178,7 @@ describe("functions download", () => { expect((error as Error).message).toBe("error running container: exit 1"); expect((error as Error & { suggestion?: string }).suggestion).toBe( "Please use deno v2 in supabase/config.toml to download this Function:\n\n[edge_runtime]\ndeno_version = 2\n" + - "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + "\nRetry with supabase functions download --use-api hello-world to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI.", ); }).pipe(Effect.provide(layer)); }, @@ -1268,7 +1189,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["permission denied"] }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1276,7 +1196,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1306,7 +1225,7 @@ describe("functions download", () => { ); expect((error as Error & { suggestion?: string }).suggestion).toBe( - "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + "\nRetry with supabase functions download --use-api hello-world to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI.", ); }).pipe(Effect.provide(layer)); }, @@ -1316,7 +1235,6 @@ describe("functions download", () => { it.live("fails when ensureDockerNetwork can't create a missing network", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const spawnerOpts: { exitCode?: number; stderr?: string[]; @@ -1333,7 +1251,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1370,7 +1287,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockDockerRunSpawnFailure(); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1378,7 +1294,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1403,7 +1318,7 @@ describe("functions download", () => { `failed to run the edge-runtime unbundle container: ${containerRuntimeNotFoundMessage}`, ); expect((error as Error & { suggestion?: string }).suggestion).toBe( - "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + "\nRetry with supabase functions download --use-api hello-world to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI.", ); expect(child.spawned.some((spawned) => spawned.args[0] === "run")).toBe(true); expect( @@ -1413,62 +1328,55 @@ describe("functions download", () => { }, ); - it.live( - "reports no functions found without delegating when the project is empty in machine mode", - () => { - const out = mockOutput({ format: "json" }); - const api = mockCommandPlatformApi({ - handler: (request) => - request.url.endsWith("/functions") - ? Effect.succeed(jsonResponse(request, 200, [])) - : Effect.succeed(jsonResponse(request, 200, {})), + it.live("reports no functions found when the project is empty in machine mode", () => { + const out = mockOutput({ format: "json" }); + const api = mockCommandPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed(jsonResponse(request, 200, [])) + : Effect.succeed(jsonResponse(request, 200, {})), + }); + // Stand-in for the real `ChildProcessSpawner`: `useDocker: true` still + // probes `docker info` even with no functions to download, so this must + // not spawn a real `docker` process. + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildTestRuntime({ + out, + api, + cliSettings: mockCommandSettings({ workdir: tempRoot.current }), + }), + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + yield* functionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, }); - const proxy = mockProxy(); - // Stand-in for the real `ChildProcessSpawner`: `useDocker: true` still - // probes `docker info` even with no functions to download, so this must - // not spawn a real `docker` process. - const child = mockChildProcessSpawner({ exitCode: 0 }); - const layer = Layer.mergeAll( - buildTestRuntime({ - out, - api, - cliSettings: mockCommandSettings({ workdir: tempRoot.current }), - }), - proxy.layer, - child.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "--project-ref", - "abcdefghijklmnopqrst", - "--output-format", - "json", - ]), + + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + message: "No functions found.", + data: { function_slugs: [], project_ref: "abcdefghijklmnopqrst" }, }), ); + }).pipe(Effect.provide(layer)); + }); - return Effect.gen(function* () { - yield* functionsDownload({ - ...baseFlags, - functionName: Option.none(), - useDocker: true, - }); - - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - message: "No functions found.", - data: { function_slugs: [], project_ref: "abcdefghijklmnopqrst" }, - }), - ); - }).pipe(Effect.provide(layer)); - }, - ); - - it.live("fails before delegating when the pre-flight function list fails in machine mode", () => { + it.live("fails when the pre-flight function list fails in machine mode", () => { const out = mockOutput({ format: "json" }); const api = mockCommandPlatformApi({ handler: (request) => @@ -1476,7 +1384,6 @@ describe("functions download", () => { ? Effect.succeed(jsonResponse(request, 500, { message: "unavailable" })) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1484,7 +1391,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1506,8 +1412,6 @@ describe("functions download", () => { }).pipe(Effect.exit); expect(Exit.isFailure(exit)).toBe(true); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([]); }).pipe(Effect.provide(layer)); }); @@ -1522,7 +1426,6 @@ describe("functions download", () => { ? Effect.succeed(jsonResponse(request, 200, [{}])) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1530,7 +1433,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1552,7 +1454,6 @@ describe("functions download", () => { }).pipe(Effect.exit); expect(Exit.isFailure(exit)).toBe(true); - expect(proxy.calls).toEqual([]); expect(out.messages).not.toContainEqual( expect.objectContaining({ type: "success", message: "No functions found." }), ); @@ -1578,14 +1479,12 @@ describe("functions download", () => { ? Effect.succeed(jsonResponse(request, 500, { message: "unavailable" })) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const layer = Layer.mergeAll( buildTestRuntime({ out, api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, Stdio.layerTest({ args: Effect.succeed(["functions", "download", "--project-ref", PROJECT_ID]), }), @@ -1616,60 +1515,15 @@ describe("functions download", () => { }, ); - it.live("forwards only --legacy-bundle to the Go proxy, not the --use-docker default too", () => { + it.live("rejects an invalid slug before any download work happens", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const layer = Layer.mergeAll( buildTestRuntime({ out, api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "hello-world", - "--legacy-bundle", - "--project-ref", - "abcdefghijklmnopqrst", - ]), - }), - ); - - return Effect.gen(function* () { - // `useDocker: true` mirrors the flag's own default even though only - // `--legacy-bundle` was passed; forwarding both would make the Go - // binary's own MarkFlagsMutuallyExclusive reject the combination. - yield* functionsDownload({ ...baseFlags, useDocker: true, legacyBundle: true }); - - expect(proxy.calls).toEqual([ - [ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--legacy-bundle", - ], - ]); - expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - }).pipe(Effect.provide(layer)); - }); - - it.live("rejects an invalid slug before ever reaching the Go proxy", () => { - const out = mockOutput({ format: "text" }); - const api = mockCommandPlatformApi(); - const proxy = mockProxy(); - const layer = Layer.mergeAll( - buildTestRuntime({ - out, - api, - cliSettings: mockCommandSettings({ workdir: tempRoot.current }), - }), - proxy.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", @@ -1682,8 +1536,6 @@ describe("functions download", () => { ); return Effect.gen(function* () { - // `useDocker: true` reflects the flag's own default; slug validation - // must run before the Go proxy sees this argv. const exit = yield* functionsDownload({ ...baseFlags, functionName: Option.some("../../etc"), @@ -1691,7 +1543,6 @@ describe("functions download", () => { }).pipe(Effect.exit); expect(Exit.isFailure(exit)).toBe(true); - expect(proxy.calls).toEqual([]); }).pipe(Effect.provide(layer)); }); @@ -1705,7 +1556,6 @@ describe("functions download", () => { ? Effect.succeed(multipartResponse(request)) : Effect.succeed(jsonResponse(request, 200, {})), }); - const proxy = mockProxy(); const analytics = mockContextualAnalytics(); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1714,7 +1564,6 @@ describe("functions download", () => { cliSettings: mockCommandSettings({ workdir: tempRoot.current }), analytics, }), - proxy.layer, commandRuntimeLayer(["functions", "download"]), Stdio.layerTest({ args: Effect.succeed([ @@ -1742,14 +1591,12 @@ describe("functions download", () => { it.live("rejects the bundler mutex with cobra's exact error text", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const layer = Layer.mergeAll( buildTestRuntime({ out, api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, Stdio.layerTest({ args: Effect.succeed(["functions", "download", "--use-api", "--use-docker"]), }), @@ -1767,19 +1614,50 @@ describe("functions download", () => { throw new Error(`unexpected error: ${String(error)}`); } expect(error.message).toBe( - "if any flags in the group [use-api use-docker legacy-bundle] are set none of the others can be; [use-api use-docker] were all set", + "if any flags in the group [use-api use-docker] are set none of the others can be; [use-api use-docker] were all set", ); - expect(proxy.calls).toEqual([]); }).pipe(Effect.provide(layer)); }); + it.live( + "rejects --legacy-bundle with a removal error before any download, Docker, or API work", + () => { + const out = mockOutput({ format: "text" }); + const api = mockCommandPlatformApi(); + const layer = Layer.mergeAll( + buildTestRuntime({ + out, + api, + cliSettings: mockCommandSettings({ workdir: tempRoot.current }), + }), + Stdio.layerTest({ + args: Effect.succeed(["functions", "download", "hello-world", "--legacy-bundle"]), + }), + ); + + return Effect.gen(function* () { + const error = yield* functionsDownload({ + ...baseFlags, + legacyBundle: true, + }).pipe(Effect.flip); + + expect(error).toBeInstanceOf(RemovedSurfaceError); + if (!(error instanceof RemovedSurfaceError)) { + throw new Error(`unexpected error: ${String(error)}`); + } + expect(error.kind).toBe("flag"); + expect(Runtime.getErrorExitCode(error)).toBe(1); + expect(api.requests).toEqual([]); + }).pipe(Effect.provide(layer)); + }, + ); + describe("Config.Validate / dotenv / env-override parity (CLI-1963)", () => { it.live( "fails before any Docker/API work when config.toml has an explicit empty project_id", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1787,7 +1665,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1825,7 +1702,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1833,7 +1709,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1876,7 +1751,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1884,7 +1758,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1929,7 +1802,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1937,7 +1809,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -1982,7 +1853,6 @@ describe("functions download", () => { it.live("prefers an explicit --network-id flag over SUPABASE_NETWORK_ID", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -1990,7 +1860,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -2039,7 +1908,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -2047,7 +1915,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -2116,14 +1983,12 @@ describe("functions download", () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const layer = Layer.mergeAll( buildTestRuntime({ out, api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -2157,7 +2022,6 @@ describe("functions download", () => { () => { const out = mockOutput({ format: "text" }); const api = mockCommandPlatformApi(); - const proxy = mockProxy(); const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildTestRuntime({ @@ -2165,7 +2029,6 @@ describe("functions download", () => { api, cliSettings: mockCommandSettings({ workdir: tempRoot.current }), }), - proxy.layer, child.layer, Stdio.layerTest({ args: Effect.succeed([ @@ -2238,7 +2101,6 @@ describe("functions download", () => { goConfigCompat: functionsGoConfigCompat, edgeRuntimeVersion: "1.69.12", resolveProjectRef: () => Effect.succeed(PROJECT_ID), - proxyDownload: () => Effect.die("unexpected proxy invocation"), styleWarning: (text) => `${text}`, }, ); diff --git a/apps/cli/src/commands/gen/gen.command.ts b/apps/cli/src/commands/gen/gen.command.ts index 245640f2b5..1f96cc3588 100644 --- a/apps/cli/src/commands/gen/gen.command.ts +++ b/apps/cli/src/commands/gen/gen.command.ts @@ -11,6 +11,6 @@ export const genCommand = Command.make("gen").pipe( genTypesCommand, genSigningKeyCommand, genBearerJwtCommand, - genKeysCommand, + genKeysCommand.pipe(Command.unlisted), ]), ); diff --git a/apps/cli/src/commands/gen/keys/SIDE_EFFECTS.md b/apps/cli/src/commands/gen/keys/SIDE_EFFECTS.md index 6b2f9d460b..1494730ae1 100644 --- a/apps/cli/src/commands/gen/keys/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/gen/keys/SIDE_EFFECTS.md @@ -1,5 +1,8 @@ # `supabase gen keys` +Removed and unlisted. The command is a tombstone: it fails with a removal error +and a replacement suggestion instead of contacting the Management API. + ## Files Read | Path | Format | When | @@ -14,42 +17,47 @@ ## API Routes -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ----------------------------- | ------------ | ------------ | ---------------------- | -| `GET` | `/v1/projects/{ref}/api-keys` | Bearer token | none | `[{name, api_key}]` | +| Method | Path | Auth | Request body | Response (used fields) | +| ------ | ---- | ---- | ------------ | ---------------------- | +| — | — | — | — | — | ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | --------------------------------------- | ------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token | no (falls back to keyring → `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | +| Variable | Purpose | Required? | +| -------- | ------- | --------- | +| — | — | — | ## Exit Codes -| Code | Condition | -| ---- | ------------------------------------- | -| `0` | success — keys printed to stdout | -| `1` | authentication error (no token found) | -| `1` | API error (non-2xx response) | +| Code | Condition | +| ---- | ---------------------------------- | +| `1` | every invocation (removed command) | ## Output ### `--output-format text` -Prints key-value pairs in env format (default) or JSON. +Writes the removal message and the replacement suggestion to stderr, two lines, +regardless of `-o`/`--output`: + +``` +supabase gen keys was removed. +Use `supabase projects api-keys --project-ref ` to read a project's API keys. +``` -### `--output-format json` +### `--output-format json` / `stream-json` -Not applicable. +Emits the JSON error envelope on stdout instead of the stderr lines above; the +envelope's `code` is `RemovedSurfaceError` and `suggestion` carries the same +replacement text. -### `--output-format stream-json` +## Telemetry Events Fired -Not applicable. +One `cli_command_executed` event per invocation, with `exit_code: 1` and an +`error_fingerprint` ending in `:removed_command` (`RemovedSurfaceError`). ## Notes -- **Deprecated**: use `gen signing-key` instead. -- `--project-ref` flag specifies the project ref. -- `--override-name` overrides specific variable names in the output. -- Experimental command for generating preview branch keys. +- Unlisted (not shown in `supabase gen --help`), still reachable by exact name. +- No flag value is read; the command fails identically regardless of + `--project-ref`/`--override-name`. diff --git a/apps/cli/src/commands/gen/keys/keys.command.ts b/apps/cli/src/commands/gen/keys/keys.command.ts index 232f8f10e0..46905f6464 100644 --- a/apps/cli/src/commands/gen/keys/keys.command.ts +++ b/apps/cli/src/commands/gen/keys/keys.command.ts @@ -1,6 +1,8 @@ import { Command, Flag } from "effect/unstable/cli"; -import type * as CliCommand from "effect/unstable/cli/Command"; -import { genKeys } from "./keys.handler.ts"; +import { removedCommand } 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"; const config = { projectRef: Flag.string("project-ref").pipe( @@ -13,12 +15,13 @@ const config = { ), } as const; -export type GenKeysFlags = CliCommand.Command.Config.Infer; - export const genKeysCommand = Command.make("keys", config).pipe( - Command.withDescription( - 'Generate keys for preview branch. Deprecated: use "gen signing-key" instead.', + Command.withDescription("Removed: use `supabase projects api-keys --project-ref ` instead."), + Command.withShortDescription("Removed: use `projects api-keys` instead"), + Command.withHandler(() => + removedCommand( + "Use `supabase projects api-keys --project-ref ` to read a project's API keys.", + ).pipe(withCommandTelemetry(), withJsonErrorHandling), ), - Command.withShortDescription("Generate keys for preview branch (experimental)"), - Command.withHandler((flags) => genKeys(flags)), + Command.provide(commandRuntimeLayer(["gen", "keys"])), ); diff --git a/apps/cli/src/commands/gen/keys/keys.handler.ts b/apps/cli/src/commands/gen/keys/keys.handler.ts deleted file mode 100644 index 620768e490..0000000000 --- a/apps/cli/src/commands/gen/keys/keys.handler.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { Effect, Option } from "effect"; -import { GoProxy } from "../../../command-internal/go-proxy.service.ts"; -import type { GenKeysFlags } from "./keys.command.ts"; - -export const genKeys = Effect.fn("gen.keys")(function* (flags: GenKeysFlags) { - const proxy = yield* GoProxy; - const args: string[] = ["gen", "keys"]; - if (Option.isSome(flags.projectRef)) args.push("--project-ref", flags.projectRef.value); - for (const name of flags.overrideName) { - args.push("--override-name", name); - } - yield* proxy.exec(args); -}); diff --git a/apps/cli/src/commands/pull/pull.steps.ts b/apps/cli/src/commands/pull/pull.steps.ts index 842bb7d62a..3f9564239a 100644 --- a/apps/cli/src/commands/pull/pull.steps.ts +++ b/apps/cli/src/commands/pull/pull.steps.ts @@ -120,8 +120,8 @@ export const pullDbStep = Effect.fnUntraced(function* (context: PullStepContext) /** * `functions` step: downloads every Edge Function's source, matching the standalone `functions - * download` command's `--use-api`/`--use-docker` defaults. `legacyBundle: false` keeps - * `proxyDownload` unreachable, so `resolveProjectRef` just returns the already-resolved ref. + * download` command's `--use-api`/`--use-docker` defaults. `resolveProjectRef` just returns the + * already-resolved ref. */ export const pullFunctionsStep = Effect.fnUntraced(function* (context: PullStepContext) { const api = yield* CommandPlatformApi; @@ -139,7 +139,6 @@ export const pullFunctionsStep = Effect.fnUntraced(function* (context: PullStepC projectRef: Option.some(context.ref), useApi: false, useDocker: true, - legacyBundle: false, }, { api, @@ -151,12 +150,6 @@ export const pullFunctionsStep = Effect.fnUntraced(function* (context: PullStepC styleAqua: (text) => aqua(text), styleWarning: (text) => yellow(text), resolveProjectRef: () => Effect.succeed(context.ref), - proxyDownload: () => - Effect.die( - new Error( - "supabase pull: functions download unexpectedly delegated to the Go binary (legacyBundle is always false for pull)", - ), - ), }, ); return { kind: "downloaded", result } satisfies PullFunctionsStepOutcome; diff --git a/apps/cli/src/docs/docs-spec.tables.ts b/apps/cli/src/docs/docs-spec.tables.ts index 3a6f32e421..936d6b5571 100644 --- a/apps/cli/src/docs/docs-spec.tables.ts +++ b/apps/cli/src/docs/docs-spec.tables.ts @@ -99,7 +99,6 @@ export const DOCS_EXPERIMENTAL_OPTIONAL: ReadonlySet = new Set([ * suffix) but excluded from the published reference. */ export const DOCS_EXCLUDED: ReadonlySet = new Set([ - "supabase-gen-keys", "supabase-inspect-db-cache-hit", "supabase-inspect-db-index-sizes", "supabase-inspect-db-index-usage", diff --git a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts index 0d317fec81..09932cf609 100644 --- a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts +++ b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts @@ -4,17 +4,20 @@ import { CliOutput, Command, type HelpDoc } from "effect/unstable/cli"; import { describe, expect, it } from "vitest"; import { branchesCommand } from "../../commands/branches/branches.command.ts"; import { dbCommand } from "../../commands/db/db.command.ts"; +import { dbDiffCommand } from "../../commands/db/diff/diff.command.ts"; import { functionsCommand } from "../../commands/functions/functions.command.ts"; import { functionsDeployCommand } from "../../commands/functions/deploy/deploy.command.ts"; import { functionsDownloadCommand } from "../../commands/functions/download/download.command.ts"; import { functionsServeCommand } from "../../commands/functions/serve/serve.command.ts"; +import { genCommand } from "../../commands/gen/gen.command.ts"; import { initCommand } from "../../commands/init/init.command.ts"; import { projectsCommand } from "../../commands/projects/projects.command.ts"; import { projectsCreateCommand } from "../../commands/projects/create/create.command.ts"; import { startCommand } from "../../commands/start/start.command.ts"; import { stopCommand } from "../../commands/stop/stop.command.ts"; import { GLOBAL_FLAGS } from "../../command-internal/global-flags.ts"; -import { GoProxy } from "../../command-internal/go-proxy.service.ts"; +import { RemovedSurfaceError } from "../../command-internal/removed-command.ts"; +import { mockAnalytics, mockOutput, mockProcessControl } from "../../../tests/helpers/mocks.ts"; import { textCliOutputFormatter } from "../output/text-formatter.ts"; interface CommandImpl { @@ -25,25 +28,13 @@ const buildHelpDoc = ( cmd: Command.Command, ): HelpDoc.HelpDoc => (cmd as unknown as CommandImpl).buildHelpDoc([]); -function mockGoProxy() { - const calls: Array> = []; - const layer = Layer.succeed(GoProxy, { - exec: (args) => - Effect.sync(() => { - calls.push([...args]); - }), - execCapture: () => Effect.succeed(""), - }); - - return { layer, calls }; -} - const testRoot = Command.make("supabase").pipe( Command.withSubcommands([ startCommand, stopCommand, initCommand, functionsCommand, + genCommand, projectsCommand, branchesCommand, dbCommand, @@ -51,6 +42,20 @@ const testRoot = Command.make("supabase").pipe( Command.withGlobalFlags(GLOBAL_FLAGS), ); +/** Satisfies a tombstoned command's `withCommandTelemetry`/`withJsonErrorHandling` requirements. */ +function tombstoneRuntimeLayer() { + const out = mockOutput({ format: "text" }); + const analytics = mockAnalytics(); + const processControl = mockProcessControl(); + return Layer.mergeAll( + out.layer, + analytics.layer, + processControl.layer, + BunServices.layer, + CliOutput.layer(textCliOutputFormatter()), + ); +} + function parserCommand( command: Command.Command, parsed: Array, @@ -121,6 +126,10 @@ describe("native hidden flags", () => { "size", "high-availability", ]); + + expect(buildHelpDoc(dbDiffCommand).flags.map((flag) => flag.name)).not.toContain( + "use-pg-schema", + ); }); it("passes hidden flag values to handlers by exact name", async () => { @@ -162,14 +171,7 @@ describe("native hidden flags", () => { "abcdefghijklmnopqrst", "--use-docker=false", ]); - yield* runParser([ - "functions", - "download", - "hello", - "--project-ref", - "abcdefghijklmnopqrst", - "--legacy-bundle", - ]); + yield* runParser(["functions", "download", "hello", "--legacy-bundle"]); yield* runParser(["functions", "deploy", "hello", "--use-docker=false"]); yield* runParser(["functions", "deploy", "hello", "--legacy-bundle"]); yield* runParser(["functions", "serve", "--all=false"]); @@ -188,8 +190,6 @@ describe("native hidden flags", () => { }); it("does not leak hidden flag names through unknown-flag suggestions", async () => { - const proxy = mockGoProxy(); - const exit = await Effect.runPromise( Command.runWith(testRoot, { version: "0.0.0-test" })([ "projects", @@ -197,7 +197,7 @@ describe("native hidden flags", () => { "demo", "--pla", ]).pipe( - Effect.provide(Layer.mergeAll(proxy.layer, CliOutput.layer(silentCliOutputFormatter))), + Effect.provide(CliOutput.layer(silentCliOutputFormatter)), Effect.exit, ) as Effect.Effect, ); @@ -236,29 +236,32 @@ describe("hidden subcommands", () => { ]); }); - it("still executes hidden subcommands by exact name", async () => { - const proxy = mockGoProxy(); + it("still executes hidden tombstoned subcommands by exact name", async () => { + const layer = tombstoneRuntimeLayer(); - await Effect.runPromise( - Effect.gen(function* () { - yield* Command.runWith(testRoot, { version: "0.0.0-test" })(["db", "branch", "list"]); - yield* Command.runWith(testRoot, { version: "0.0.0-test" })(["db", "remote", "changes"]); - }).pipe( - Effect.provide(Layer.mergeAll(proxy.layer, CliOutput.layer(textCliOutputFormatter()))), - ) as Effect.Effect, - ); + const causeOf = (exit: unknown) => + (exit as { cause: { reasons: Array<{ _tag: string; error?: unknown }> } }).cause; - expect(proxy.calls).toEqual([ + for (const args of [ ["db", "branch", "list"], ["db", "remote", "changes"], - ]); + ["gen", "keys"], + ]) { + const exit = await Effect.runPromise( + Command.runWith(testRoot, { version: "0.0.0-test" })(args).pipe( + Effect.provide(layer), + Effect.exit, + ) as Effect.Effect, + ); + expect((exit as { _tag: string })._tag).toBe("Failure"); + expect(causeOf(exit).reasons[0]?.error).toBeInstanceOf(RemovedSurfaceError); + } }); it("still executes the native `db test` hidden alias by exact name (CLI-1962)", async () => { // This test's minimal layer doesn't wire the services the native handler needs, so dispatch // reaching the handler (a Die on a missing service, not success) is what's being proven. - const proxy = mockGoProxy(); - const layer = Layer.mergeAll(proxy.layer, CliOutput.layer(textCliOutputFormatter())); + const layer = CliOutput.layer(textCliOutputFormatter()); const causeOf = (exit: unknown) => (exit as { cause: { reasons: Array<{ _tag: string; defect?: unknown; error?: unknown }> } }) diff --git a/apps/cli/src/shared/cli/run.ts b/apps/cli/src/shared/cli/run.ts index 1e54c8e767..d0c3626323 100644 --- a/apps/cli/src/shared/cli/run.ts +++ b/apps/cli/src/shared/cli/run.ts @@ -29,11 +29,6 @@ import { outputLayerFor } from "../output/output.layer.ts"; import { normalizeCause } from "../output/normalize-error.ts"; import type { OutputFormat } from "../output/types.ts"; import { Output } from "../output/output.service.ts"; -import { GoChildExitError } from "../../command-internal/go-child-exit.error.ts"; -import { - GoProxyInvocation, - goProxyInvocationLayer, -} from "../../command-internal/go-proxy-invocation.ts"; import { cliSettingsLayer } from "../config/cli-settings.layer.ts"; import { cliConfigProviderLayer } from "../config/cli-config-provider.layer.ts"; import { cliProjectHomeLayer } from "../config/cli-project-home.layer.ts"; @@ -350,15 +345,10 @@ export function exitCodeForFailure(cause: Cause.Cause): number { /** * Whether `handledProgram` should render its generic `output.fail` stderr line for a failed run. - * False for a clean exit (`0`), an interrupt (`130`), and a `GoChildExitError` — a delegated Go - * child already wrote its own failure to the inherited stderr, so a second line here would be - * redundant. Checked by concrete type rather than Effect's shared `[Runtime.errorReported]` - * marker, since `CliError.ShowHelp` also sets that marker `false` for an unrelated reason and - * would otherwise suppress real error rendering too. + * False only for a clean exit (`0`) or an interrupt (`130`); every other exit code reports. */ -export function shouldReportFailure(cause: Cause.Cause, exitCode: number): boolean { - if (exitCode === 0 || exitCode === 130) return false; - return !(Cause.squash(cause) instanceof GoChildExitError); +export function shouldReportFailure(exitCode: number): boolean { + return exitCode !== 0 && exitCode !== 130; } /** @@ -553,7 +543,6 @@ export interface RunCliOptions { args: ReadonlyArray, info: { readonly cleanShowHelp: boolean; - readonly delegatedToGo: boolean; readonly workingDirectory?: string; /** Value-taking-token predicate for this argv (global + resolved leaf flags) — see `valueTakingFlagTokenPredicateForArgv`. */ readonly isValueTakingFlagToken: (token: string) => boolean; @@ -693,7 +682,6 @@ export async function runCli< ): Effect.Effect => Effect.gen(function* () { const processControl = yield* ProcessControl; - const goProxyInvocation = yield* GoProxyInvocation; const output = yield* Output; const successTrailer = yield* SuccessTrailer; const exit = yield* program.pipe(Effect.exit); @@ -708,11 +696,9 @@ export async function runCli< Effect.andThen( Effect.gen(function* () { if (afterSuccessHook !== undefined) { - const delegatedToGo = yield* goProxyInvocation.wasDelegated; const workingDirectory = yield* successTrailer.workingDirectory; yield* afterSuccessHook(args, { cleanShowHelp, - delegatedToGo, workingDirectory, isValueTakingFlagToken: valueTakingFlagTokenPredicateForArgv( rootCommand, @@ -735,7 +721,7 @@ export async function runCli< const exitCode = exitCodeForFailure(exit.cause); // See `shouldReportFailure` and `exitCodeForFailure` for the exit-code/reporting rules; a // literal `--help` never reaches this branch — it exits 0 via the success path below. - if (shouldReportFailure(exit.cause, exitCode)) { + if (shouldReportFailure(exitCode)) { yield* output.fail(normalizeCause(exit.cause, suggestionContext)); } yield* afterSuccess(exitCode, true); @@ -754,7 +740,6 @@ export async function runCli< Effect.provide(runtimeInfoLayer), Effect.provide(ttyLayer), Effect.provide(BunServices.layer), - Effect.provide(goProxyInvocationLayer), Effect.provide(successTrailerLayer), Effect.provide(cliConfigProviderLayer), ); diff --git a/apps/cli/src/shared/cli/run.unit.test.ts b/apps/cli/src/shared/cli/run.unit.test.ts index 810fbc426c..5d5c748918 100644 --- a/apps/cli/src/shared/cli/run.unit.test.ts +++ b/apps/cli/src/shared/cli/run.unit.test.ts @@ -1,11 +1,10 @@ -import { Cause } from "effect"; +import { Cause, Data, Runtime } from "effect"; import { CliError, Command } from "effect/unstable/cli"; import { describe, expect, it } from "vitest"; import { branchesCommand } from "../../commands/branches/branches.command.ts"; import { migrationCommand } from "../../commands/migration/migration.command.ts"; import { ssoCommand } from "../../commands/sso/sso.command.ts"; -import { GoChildExitError } from "../../command-internal/go-child-exit.error.ts"; import { classifyParseErrorConsoleOutput, exitCodeForFailure, @@ -23,6 +22,14 @@ const testRoot = Command.make("supabase").pipe( Command.withSubcommands([branchesCommand, migrationCommand, ssoCommand]), ); +/** Local stand-in for a typed error opting into a custom process exit code. */ +class CustomExitCodeError extends Data.TaggedError("CustomExitCodeError")<{ + readonly exitCode: number; + readonly message: string; +}> { + override readonly [Runtime.errorExitCode] = this.exitCode; +} + describe("extractCommandPath", () => { it("returns positional command-path tokens", () => { expect(extractCommandPath(["functions", "serve"])).toEqual(["functions", "serve"]); @@ -133,9 +140,9 @@ describe("exitCodeForFailure", () => { expect(exitCodeForFailure(Cause.interrupt())).toBe(130); }); - it("exits with a GoChildExitError's exact exit code", () => { + it("exits with a typed error's exact custom exit code", () => { const cause = Cause.fail( - new GoChildExitError({ exitCode: 130, message: "supabase-go exited with code 130" }), + new CustomExitCodeError({ exitCode: 130, message: "exited with code 130" }), ); expect(exitCodeForFailure(cause)).toBe(130); }); @@ -143,32 +150,15 @@ describe("exitCodeForFailure", () => { describe("shouldReportFailure", () => { it("does not report a clean exit (0)", () => { - expect(shouldReportFailure(Cause.fail(new Error("unused")), 0)).toBe(false); + expect(shouldReportFailure(0)).toBe(false); }); it("does not report an interrupt (130)", () => { - expect(shouldReportFailure(Cause.interrupt(), 130)).toBe(false); - }); - - it("does not report a GoChildExitError", () => { - const cause = Cause.fail( - new GoChildExitError({ exitCode: 1, message: "supabase-go exited with code 1" }), - ); - expect(shouldReportFailure(cause, 1)).toBe(false); + expect(shouldReportFailure(130)).toBe(false); }); - it("reports a non-ShowHelp failure", () => { - expect(shouldReportFailure(Cause.fail(new Error("boom")), 1)).toBe(true); - }); - - it("still reports a ShowHelp failure carrying a genuine validation error (e.g. a missing required flag)", () => { - const cause = Cause.fail( - new CliError.ShowHelp({ - commandPath: ["sso", "add"], - errors: [new CliError.MissingOption({ option: "--type" })], - }), - ); - expect(shouldReportFailure(cause, 1)).toBe(true); + it("reports every other exit code", () => { + expect(shouldReportFailure(1)).toBe(true); }); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index bca11feef2..3b2a74d172 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -30,7 +30,7 @@ import { } from "../cli/cobra-flag-groups.ts"; import { edgeRuntimeImage, - FUNCTIONS_BUNDLER_MUTEX_GROUP, + FUNCTIONS_DEPLOY_BUNDLER_MUTEX_GROUP, invalidFunctionSlugDetail, validateFunctionSlugMessage, } from "./functions.shared.ts"; @@ -2313,7 +2313,10 @@ export function deployFunctions( if (changedModes.length > 1) { return yield* Effect.fail( new ConflictingFunctionDeployFlagsError({ - message: cobraMutuallyExclusiveErrorMessage(FUNCTIONS_BUNDLER_MUTEX_GROUP, changedModes), + message: cobraMutuallyExclusiveErrorMessage( + FUNCTIONS_DEPLOY_BUNDLER_MUTEX_GROUP, + changedModes, + ), }), ); } diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 82aaf569f5..bc039a9275 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -31,7 +31,7 @@ import { import { loadFunctionsCliConfig, type FunctionsGoConfigCompat } from "./functions-config.ts"; import { edgeRuntimeImage, - FUNCTIONS_BUNDLER_MUTEX_GROUP, + FUNCTIONS_DOWNLOAD_BUNDLER_MUTEX_GROUP, invalidFunctionSlugDetail, validateFunctionSlugMessage, } from "./functions.shared.ts"; @@ -55,7 +55,6 @@ export interface DownloadFunctionsOptions { readonly projectRef: Option.Option; readonly useApi: boolean; readonly useDocker: boolean; - readonly legacyBundle: boolean; } export interface DownloadFunctionsResult { @@ -81,9 +80,8 @@ interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies */ readonly styleEmphasis?: (text: string) => string; /** - * Optional shell-specific styling hook for the `--legacy-bundle` command - * suggested inside {@link suggestLegacyBundle} — same isolation rationale - * as {@link styleEmphasis}. + * Optional shell-specific styling hook for the command suggested inside + * {@link suggestLegacyBundle} — same isolation rationale as {@link styleEmphasis}. */ readonly styleAqua?: (text: string) => string; /** @@ -110,40 +108,11 @@ interface EdgeRuntimeImageDependencies { readonly edgeRuntimeVersion: string; } -export interface DownloadFunctionsDependencies< - ResolveError, - ResolveRequirements, - ProxyError, - ProxyRequirements, -> +export interface DownloadFunctionsDependencies extends DownloadDockerRuntimeDependencies, EdgeRuntimeImageDependencies { readonly resolveProjectRef: ( projectRef: Option.Option, ) => Effect.Effect; - /** - * `true` whenever `output.format !== "text"`: the child's raw stdout must - * not reach the terminal (it would corrupt the JSON/NDJSON envelope), so - * the dependency must capture/discard it instead of inheriting stdio. - */ - readonly proxyDownload: ( - flags: DownloadFunctionsOptions, - projectRef: string, - captureOutput: boolean, - ) => Effect.Effect; -} - -// `--legacy-bundle` is the only case `downloadFunctions()` still delegates -// to the Go binary; `functionName` is the one remaining input to forward. -export function makeGoProxyLegacyBundleArgs( - functionName: Option.Option, - projectRef: string, -): ReadonlyArray { - const args: string[] = ["functions", "download"]; - if (Option.isSome(functionName)) { - args.push(functionName.value); - } - args.push("--project-ref", projectRef, "--legacy-bundle"); - return args; } interface DownloadMetadata { @@ -217,16 +186,16 @@ function validateDownloadFlags( const changed = [ hasExplicitLongFlag(rawArgs, downloadCommandPath, "use-api") ? "use-api" : undefined, hasExplicitLongFlag(rawArgs, downloadCommandPath, "use-docker") ? "use-docker" : undefined, - hasExplicitLongFlag(rawArgs, downloadCommandPath, "legacy-bundle") - ? "legacy-bundle" - : undefined, ].filter((flag): flag is string => flag !== undefined); return changed.length <= 1 ? Effect.void : Effect.fail( new ConflictingFunctionDownloadFlagsError({ - message: cobraMutuallyExclusiveErrorMessage(FUNCTIONS_BUNDLER_MUTEX_GROUP, changed), + message: cobraMutuallyExclusiveErrorMessage( + FUNCTIONS_DOWNLOAD_BUNDLER_MUTEX_GROUP, + changed, + ), }), ); } @@ -838,10 +807,8 @@ function suggestLegacyBundle( slug: string, styleAqua: (text: string) => string = (text) => text, ): string { - // Preserves the established "trying running" wording (not a typo) and - // leading newline; `styleAqua` wraps only the suggested command, not the - // whole sentence. - return `\nIf your function is deployed using CLI < 1.120.0, trying running ${styleAqua(`supabase functions download --legacy-bundle ${slug}`)} instead.`; + // Preserves the established leading newline. + return `\nRetry with ${styleAqua(`supabase functions download --use-api ${slug}`)} to unbundle server-side. If that also fails and the Function was deployed with a CLI older than 1.120.0, redeploy it with the current CLI.`; } function suggestDenoV2(styleEmphasis: (text: string) => string = (text) => text): string { @@ -1150,14 +1117,9 @@ function attachDownloadWrittenSoFar( : Object.assign(error, { writtenSoFar: [...downloadedSoFar] }); } -export function downloadFunctions( +export function downloadFunctions( flags: DownloadFunctionsOptions, - dependencies: DownloadFunctionsDependencies< - ResolveError, - ResolveRequirements, - ProxyError, - ProxyRequirements - >, + dependencies: DownloadFunctionsDependencies, ) { return Effect.gen(function* () { const output = yield* Output; @@ -1168,52 +1130,6 @@ export function downloadFunctions text); const pulledEdgeRuntimeImage: PulledEdgeRuntimeImage | undefined = edgeRuntimeImage === undefined diff --git a/apps/cli/src/shared/functions/functions.shared.ts b/apps/cli/src/shared/functions/functions.shared.ts index 0c6ebabb3c..53a0e42d47 100644 --- a/apps/cli/src/shared/functions/functions.shared.ts +++ b/apps/cli/src/shared/functions/functions.shared.ts @@ -16,7 +16,16 @@ export function validateFunctionSlugMessage(slug: string): string | undefined { export const FUNCTIONS_PROJECT_REF_SAFE_FLAGS = ["project-ref"] as const; // Order is rendered verbatim in the mutually-exclusive-flags error message. -export const FUNCTIONS_BUNDLER_MUTEX_GROUP = ["use-api", "use-docker", "legacy-bundle"] as const; +export const FUNCTIONS_DEPLOY_BUNDLER_MUTEX_GROUP = [ + "use-api", + "use-docker", + "legacy-bundle", +] as const; + +// Order is rendered verbatim in the mutually-exclusive-flags error message. `legacy-bundle` is +// a removed, tombstoned flag on `functions download` (see `removedFlag`), rejected +// unconditionally before this group is ever checked, so it is not a member here. +export const FUNCTIONS_DOWNLOAD_BUNDLER_MUTEX_GROUP = ["use-api", "use-docker"] as const; /** * Full deno-1 edge-runtime image tag, resolved from the embedded Dockerfile diff --git a/apps/cli/src/shared/output/json-error-handling.ts b/apps/cli/src/shared/output/json-error-handling.ts index e9f83f60e8..b9a62de5d6 100644 --- a/apps/cli/src/shared/output/json-error-handling.ts +++ b/apps/cli/src/shared/output/json-error-handling.ts @@ -14,7 +14,7 @@ export const withJsonErrorHandling = ( if (output.format === "text") return yield* Effect.fail(error); yield* output.fail(normalizeCliError(error)); // `Runtime.getErrorExitCode` defaults to 1 unless the error opts in via - // `[Runtime.errorExitCode]`, so a delegated child's real exit code still + // `[Runtime.errorExitCode]`, so a typed error's custom exit code still // reaches the user under json/stream-json, matching the text-mode path. yield* processControl.setExitCode(Runtime.getErrorExitCode(error)); }), diff --git a/apps/cli/src/shared/output/json-error-handling.unit.test.ts b/apps/cli/src/shared/output/json-error-handling.unit.test.ts index bd050ca0e6..bc2fbbdb56 100644 --- a/apps/cli/src/shared/output/json-error-handling.unit.test.ts +++ b/apps/cli/src/shared/output/json-error-handling.unit.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; -import { Data, Effect, Exit, Layer, Option } from "effect"; +import { Data, Effect, Exit, Layer, Option, Runtime } from "effect"; import { mockProcessControl } from "../../../tests/helpers/mocks.ts"; -import { GoChildExitError } from "../../command-internal/go-child-exit.error.ts"; import { Output } from "./output.service.ts"; import { withJsonErrorHandling } from "./json-error-handling.ts"; @@ -15,6 +14,14 @@ class TaggedErrorMinimal extends Data.TaggedError("TaggedErrorMinimal")<{ readonly message: string; }> {} +/** Local stand-in for a typed error opting into a custom process exit code. */ +class CustomExitCodeError extends Data.TaggedError("CustomExitCodeError")<{ + readonly exitCode: number; + readonly message: string; +}> { + override readonly [Runtime.errorExitCode] = this.exitCode; +} + class PlainError { readonly message: string; constructor(message: string) { @@ -172,17 +179,17 @@ describe("withJsonErrorHandling", () => { }).pipe(Effect.provide(out.layer), Effect.provide(processControl.layer)); }); - it.live("sets the exact exit code for a GoChildExitError, not a generic 1", () => { + it.live("sets a typed error's exact custom exit code, not a generic 1", () => { const out = mockOutput("json"); const processControl = mockProcessControl(); return Effect.gen(function* () { - const error = new GoChildExitError({ + const error = new CustomExitCodeError({ exitCode: 130, - message: "supabase-go exited with code 130 (see stderr for details)", + message: "exited with code 130", }); yield* withJsonErrorHandling(Effect.fail(error)).pipe(Effect.provide(out.layer)); expect(out.failCalls).toHaveLength(1); - expect(out.failCalls[0]?.code).toBe("GoChildExitError"); + expect(out.failCalls[0]?.code).toBe("CustomExitCodeError"); expect(processControl.exitCode).toBe(130); }).pipe(Effect.provide(out.layer), Effect.provide(processControl.layer)); }); diff --git a/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt b/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt index ba6f6747c0..58c4f75b14 100644 --- a/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt +++ b/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt @@ -289,7 +289,6 @@ GenTypesNetworkIdUnsupportedError GenTypesParseConfigError GenTypesUnexpectedStatusError GenTypesWorkdirError -GoChildExitError HealthCheckProbeError HealthCheckTimeoutError HostPostgresClientError @@ -435,6 +434,7 @@ PullParentRefInvalidError PullUncommittedChangesError PullWorkdirError RemoteJwksError +RemovedSurfaceError ResetLocalDbFailedError ResetLocalDbNotRunningError ResetReplicationSlotsError diff --git a/apps/cli/src/shared/telemetry/error-actionability.ts b/apps/cli/src/shared/telemetry/error-actionability.ts index bd3a8e80dc..7d217167d2 100644 --- a/apps/cli/src/shared/telemetry/error-actionability.ts +++ b/apps/cli/src/shared/telemetry/error-actionability.ts @@ -113,6 +113,8 @@ const CLI_ERROR_FINGERPRINT_SUFFIXES = [ "port_conflict", "query", "registry_pull", + "removed_command", + "removed_flag", "replication_slots_active", "replication_slots_query", "request_encoding", @@ -367,6 +369,13 @@ export const actionability = { suggestion_type: CliSuggestionType.RunCommand, suggested_command: "supabase seed buckets", }, + /** A removed command path or flag; the suggestion is free-form replacement guidance. */ + removedSurface: { + error_kind: CliErrorKind.UserActionable, + error_category: CliErrorCategory.InvalidInput, + has_suggestion: true, + suggestion_type: CliSuggestionType.RunCommand, + }, externalNetwork: { error_kind: CliErrorKind.ExternalService, error_category: CliErrorCategory.Network, diff --git a/docs/adr/0016-legacy-port-completion-and-go-cli-authority-scope.md b/docs/adr/0016-legacy-port-completion-and-go-cli-authority-scope.md index 3067d974b6..b9328da098 100644 --- a/docs/adr/0016-legacy-port-completion-and-go-cli-authority-scope.md +++ b/docs/adr/0016-legacy-port-completion-and-go-cli-authority-scope.md @@ -11,7 +11,7 @@ command began as a Phase 0 proxy to the Go binary, then moved to a native TypeSc touching `src/legacy/` was correct — nearly every change was either wrapping a new command or replacing its proxy, and the Go source was the only available spec for what the command should do. -That phase is essentially over. Per [`apps/cli/docs/go-cli-porting-status.md`](../../apps/cli/docs/go-cli-porting-status.md), +That phase is essentially over. Per `apps/cli/docs/go-cli-porting-status.md` (removed, CLI-2432), 95 of 103 legacy leaf commands (~92%) are natively ported; only 8 remain Phase 0 proxies. Most changes landing in `src/legacy/` today are ordinary engineering on already-ported commands — bug fixes, internal refactors, hoisting shared helpers, adding documented TS-only flags, telemetry and @@ -72,8 +72,8 @@ surface from a parity check it never needed. - Contributors now have to briefly classify a change (does it touch the parity surface?) rather than defaulting to "always check Go." Misclassification risk is partly, not fully, mitigated: - [`apps/cli/docs/go-cli-porting-status.md`](../../apps/cli/docs/go-cli-porting-status.md) stays the - source of truth for which commands are still `wrapped`, and CI's `testParity` / + `apps/cli/docs/go-cli-porting-status.md` (since removed, CLI-2432) stayed the + source of truth for which commands were still `wrapped`, and CI's `testParity` / `*.e2e.test.ts` suites catch output/behavior drift on the already-ported commands and code paths they cover — but that coverage is deliberately partial (e.g. `db pull --local` and `db lint --local` skip `testParity` today, see `apps/cli-e2e/src/tests/database-core.e2e.test.ts`), so a @@ -96,7 +96,7 @@ surface from a parity check it never needed. ## See Also - [`apps/cli/AGENTS.md`](../../apps/cli/AGENTS.md) -- [`apps/cli/docs/go-cli-porting-status.md`](../../apps/cli/docs/go-cli-porting-status.md) +- `apps/cli/docs/go-cli-porting-status.md` (since removed, CLI-2432) ## Addendum (2026-08-12): CLI-1970 outcome From d7005643f6479847d0fd52383e330f9b987e16c3 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Fri, 18 Sep 2026 13:57:09 +0100 Subject: [PATCH 2/2] fix(cli): address AI review findings on PR #6609 Rejects --legacy-bundle=false on functions download the same as an explicit true (presence, not value, like --use-pg-schema); flushes TelemetryState from the tombstone/removed-flag paths, which previously skipped it entirely; and cleans up a stale flag description, a misleading suggestion-builder name, and a comment referencing the removed GoProxy. Co-Authored-By: Claude Sonnet 5 --- apps/cli/scripts/build.ts | 2 +- .../src/command-internal/removed-command.ts | 35 ++++++++++------ .../removed-command.unit.test.ts | 40 ++++++++++++++++++- .../src/commands/db/branch/branch.command.ts | 3 ++ .../db/remote/changes/changes.command.ts | 2 + .../functions/download/download.command.ts | 6 ++- .../functions/download/download.handler.ts | 2 +- .../download/download.integration.test.ts | 33 ++++++++++++++- .../cli/src/commands/gen/keys/keys.command.ts | 2 + .../src/shared/cli/hidden-flag.unit.test.ts | 40 ++++++++++++++----- apps/cli/src/shared/functions/download.ts | 10 ++--- 11 files changed, 140 insertions(+), 35 deletions(-) diff --git a/apps/cli/scripts/build.ts b/apps/cli/scripts/build.ts index 87a9e3e3b4..875b293f06 100644 --- a/apps/cli/scripts/build.ts +++ b/apps/cli/scripts/build.ts @@ -282,7 +282,7 @@ async function buildMuslBinaries() { }); // The Go binary is fully static (CGO_ENABLED=0), so the glibc build works on musl too; - // copy it into the musl package so GoProxy finds supabase-go there. + // copy it into the musl package since musl has no native Go build of its own. const glibcTarget = TARGETS.find( (candidate) => "nfpmArch" in candidate && candidate.nfpmArch === target.nfpmArch, ); diff --git a/apps/cli/src/command-internal/removed-command.ts b/apps/cli/src/command-internal/removed-command.ts index b852802e65..6209b9a2b9 100644 --- a/apps/cli/src/command-internal/removed-command.ts +++ b/apps/cli/src/command-internal/removed-command.ts @@ -8,6 +8,7 @@ import { CommandRuntime, getCommandRuntimeCommand, } from "../shared/runtime/command-runtime.service.ts"; +import { TelemetryState } from "../telemetry/telemetry-state.service.ts"; /** * A tombstoned command path or a rejected removed flag. The caller supplies its own @@ -31,25 +32,35 @@ export class RemovedSurfaceError extends Data.TaggedError("RemovedSurfaceError") * Fails with a `RemovedSurfaceError` naming the invoked command path. The caller wraps the * returned effect in `withCommandTelemetry()` then `withJsonErrorHandling`, matching * `commit.command.ts`'s composition order, and provides a `CommandRuntime` layer for its path. + * Flushes `TelemetryState` itself (`Effect.ensuring`), since a tombstoned command never reaches + * its own handler's finalizer wiring. */ export const removedCommand = (suggestion: string) => Effect.gen(function* () { - const commandRuntime = yield* CommandRuntime; - const command = getCommandRuntimeCommand(commandRuntime); - return yield* Effect.fail( - new RemovedSurfaceError({ - message: `supabase ${command} was removed.`, - suggestion, - kind: "command", - }), - ); + const telemetryState = yield* TelemetryState; + return yield* Effect.gen(function* () { + const commandRuntime = yield* CommandRuntime; + const command = getCommandRuntimeCommand(commandRuntime); + return yield* Effect.fail( + new RemovedSurfaceError({ + message: `supabase ${command} was removed.`, + suggestion, + kind: "command", + }), + ); + }).pipe(Effect.ensuring(telemetryState.flush)); }); /** * Fails with a `RemovedSurfaceError` for a removed flag on an otherwise-native command. The host * command already wraps its whole handler in `withCommandTelemetry`, so this is a bare effect. + * Flushes `TelemetryState` itself (`Effect.ensuring`): call sites check this before their own + * handler reaches its own finalizer wiring further down. */ export const removedFlag = (flag: string, suggestion: string) => - Effect.fail( - new RemovedSurfaceError({ message: `${flag} was removed.`, suggestion, kind: "flag" }), - ); + Effect.gen(function* () { + const telemetryState = yield* TelemetryState; + return yield* Effect.fail( + new RemovedSurfaceError({ message: `${flag} was removed.`, suggestion, kind: "flag" }), + ).pipe(Effect.ensuring(telemetryState.flush)); + }); diff --git a/apps/cli/src/command-internal/removed-command.unit.test.ts b/apps/cli/src/command-internal/removed-command.unit.test.ts index d9441ecd31..8a43a0729f 100644 --- a/apps/cli/src/command-internal/removed-command.unit.test.ts +++ b/apps/cli/src/command-internal/removed-command.unit.test.ts @@ -1,6 +1,9 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit } from "effect"; import { classifyCliErrorActionability } from "../shared/telemetry/error-actionability.ts"; -import { RemovedSurfaceError } from "./removed-command.ts"; +import { commandRuntimeLayer } from "../shared/runtime/command-runtime.layer.ts"; +import { mockTelemetryStateTracked } from "../../tests/helpers/command-mocks.ts"; +import { removedCommand, removedFlag, RemovedSurfaceError } from "./removed-command.ts"; describe("RemovedSurfaceError actionability", () => { it("classifies a removed command with the :removed_command fingerprint suffix", () => { @@ -48,3 +51,36 @@ describe("RemovedSurfaceError actionability", () => { }); }); }); + +describe("TelemetryState flush", () => { + it.effect("flushes on a removedCommand failure", () => { + const telemetry = mockTelemetryStateTracked(); + return removedCommand("Use `supabase branches --help` instead.").pipe( + Effect.exit, + Effect.tap((exit) => + Effect.sync(() => { + expect(Exit.isFailure(exit)).toBe(true); + expect(telemetry.flushed).toBe(true); + expect(telemetry.flushCount).toBe(1); + }), + ), + Effect.provide(commandRuntimeLayer(["db", "branch", "create"])), + Effect.provide(telemetry.layer), + ); + }); + + it.effect("flushes on a removedFlag failure", () => { + const telemetry = mockTelemetryStateTracked(); + return removedFlag("--use-pg-schema", "Use the default migra engine or --use-pg-delta.").pipe( + Effect.exit, + Effect.tap((exit) => + Effect.sync(() => { + expect(Exit.isFailure(exit)).toBe(true); + expect(telemetry.flushed).toBe(true); + expect(telemetry.flushCount).toBe(1); + }), + ), + Effect.provide(telemetry.layer), + ); + }); +}); diff --git a/apps/cli/src/commands/db/branch/branch.command.ts b/apps/cli/src/commands/db/branch/branch.command.ts index b15dbb6d62..a790aeffd2 100644 --- a/apps/cli/src/commands/db/branch/branch.command.ts +++ b/apps/cli/src/commands/db/branch/branch.command.ts @@ -3,6 +3,7 @@ import { removedCommand } 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"; const REMOVED_SUGGESTION = "Local database branches are no longer supported. For hosted preview branches, see `supabase branches --help`."; @@ -21,6 +22,7 @@ function removedBranchLeaf(name: string, argDescription: string) { removedCommand(REMOVED_SUGGESTION).pipe(withCommandTelemetry(), withJsonErrorHandling), ), Command.provide(commandRuntimeLayer(["db", "branch", name])), + Command.provide(telemetryStateLayer), ); } @@ -35,6 +37,7 @@ const dbBranchListCommand = Command.make("list", {}).pipe( removedCommand(REMOVED_SUGGESTION).pipe(withCommandTelemetry(), withJsonErrorHandling), ), Command.provide(commandRuntimeLayer(["db", "branch", "list"])), + Command.provide(telemetryStateLayer), ); export const dbBranchCommand = Command.make("branch").pipe( diff --git a/apps/cli/src/commands/db/remote/changes/changes.command.ts b/apps/cli/src/commands/db/remote/changes/changes.command.ts index 2dc52a8d3d..3e242b117b 100644 --- a/apps/cli/src/commands/db/remote/changes/changes.command.ts +++ b/apps/cli/src/commands/db/remote/changes/changes.command.ts @@ -3,6 +3,7 @@ import { removedCommand } 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"; const config = { schema: Flag.string("schema").pipe( @@ -35,4 +36,5 @@ export const dbRemoteChangesCommand = Command.make("changes", config).pipe( ), ), Command.provide(commandRuntimeLayer(["db", "remote", "changes"])), + Command.provide(telemetryStateLayer), ); diff --git a/apps/cli/src/commands/functions/download/download.command.ts b/apps/cli/src/commands/functions/download/download.command.ts index 188f2f61b1..ee53d997d8 100644 --- a/apps/cli/src/commands/functions/download/download.command.ts +++ b/apps/cli/src/commands/functions/download/download.command.ts @@ -24,9 +24,11 @@ const config = { Flag.withDefault(true), Flag.withHidden, ), + // Kept parsed (and hidden) only so using it produces an actionable removal error instead of + // an unknown-flag parse error; see download.handler.ts. legacyBundle: Flag.boolean("legacy-bundle").pipe( - Flag.withDescription("Use legacy bundling."), - Flag.withDefault(false), + Flag.withDescription("Removed: use --use-api instead."), + Flag.optional, Flag.withHidden, ), } as const; diff --git a/apps/cli/src/commands/functions/download/download.handler.ts b/apps/cli/src/commands/functions/download/download.handler.ts index b25373c168..01304fde4d 100644 --- a/apps/cli/src/commands/functions/download/download.handler.ts +++ b/apps/cli/src/commands/functions/download/download.handler.ts @@ -16,7 +16,7 @@ import type { FunctionsDownloadFlags } from "./download.command.ts"; export const functionsDownload = Effect.fn("functions.download")(function* ( flags: FunctionsDownloadFlags, ) { - if (flags.legacyBundle) { + if (Option.isSome(flags.legacyBundle)) { return yield* removedFlag( "--legacy-bundle", "Retry with `supabase functions download --use-api ` to unbundle server-side without Docker.", diff --git a/apps/cli/src/commands/functions/download/download.integration.test.ts b/apps/cli/src/commands/functions/download/download.integration.test.ts index 9878042798..4bbca4d9ea 100644 --- a/apps/cli/src/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/commands/functions/download/download.integration.test.ts @@ -153,7 +153,7 @@ const baseFlags: FunctionsDownloadFlags = { projectRef: Option.none(), useApi: false, useDocker: false, - legacyBundle: false, + legacyBundle: Option.none(), }; function multipartResponse(request: Parameters[0]) { @@ -1638,7 +1638,7 @@ describe("functions download", () => { return Effect.gen(function* () { const error = yield* functionsDownload({ ...baseFlags, - legacyBundle: true, + legacyBundle: Option.some(true), }).pipe(Effect.flip); expect(error).toBeInstanceOf(RemovedSurfaceError); @@ -1652,6 +1652,35 @@ describe("functions download", () => { }, ); + it.live("rejects --legacy-bundle=false the same as an explicit true value", () => { + const out = mockOutput({ format: "text" }); + const api = mockCommandPlatformApi(); + const layer = Layer.mergeAll( + buildTestRuntime({ + out, + api, + cliSettings: mockCommandSettings({ workdir: tempRoot.current }), + }), + Stdio.layerTest({ + args: Effect.succeed(["functions", "download", "hello-world", "--legacy-bundle=false"]), + }), + ); + + return Effect.gen(function* () { + const error = yield* functionsDownload({ + ...baseFlags, + legacyBundle: Option.some(false), + }).pipe(Effect.flip); + + expect(error).toBeInstanceOf(RemovedSurfaceError); + if (!(error instanceof RemovedSurfaceError)) { + throw new Error(`unexpected error: ${String(error)}`); + } + expect(error.kind).toBe("flag"); + expect(api.requests).toEqual([]); + }).pipe(Effect.provide(layer)); + }); + describe("Config.Validate / dotenv / env-override parity (CLI-1963)", () => { it.live( "fails before any Docker/API work when config.toml has an explicit empty project_id", diff --git a/apps/cli/src/commands/gen/keys/keys.command.ts b/apps/cli/src/commands/gen/keys/keys.command.ts index 46905f6464..dedb6163df 100644 --- a/apps/cli/src/commands/gen/keys/keys.command.ts +++ b/apps/cli/src/commands/gen/keys/keys.command.ts @@ -3,6 +3,7 @@ import { removedCommand } 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"; const config = { projectRef: Flag.string("project-ref").pipe( @@ -24,4 +25,5 @@ export const genKeysCommand = Command.make("keys", config).pipe( ).pipe(withCommandTelemetry(), withJsonErrorHandling), ), Command.provide(commandRuntimeLayer(["gen", "keys"])), + Command.provide(telemetryStateLayer), ); diff --git a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts index 09932cf609..6aee23a0c0 100644 --- a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts +++ b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts @@ -1,4 +1,4 @@ -import { Effect, Layer } from "effect"; +import { Effect, Layer, Option } from "effect"; import { BunServices } from "@effect/platform-bun"; import { CliOutput, Command, type HelpDoc } from "effect/unstable/cli"; import { describe, expect, it } from "vitest"; @@ -17,7 +17,13 @@ import { startCommand } from "../../commands/start/start.command.ts"; import { stopCommand } from "../../commands/stop/stop.command.ts"; import { GLOBAL_FLAGS } from "../../command-internal/global-flags.ts"; import { RemovedSurfaceError } from "../../command-internal/removed-command.ts"; -import { mockAnalytics, mockOutput, mockProcessControl } from "../../../tests/helpers/mocks.ts"; +import { + mockAnalytics, + mockOutput, + mockProcessControl, + mockTelemetryRuntime, +} from "../../../tests/helpers/mocks.ts"; +import { useTempWorkdir, withEnvVar } from "../../../tests/helpers/command-mocks.ts"; import { textCliOutputFormatter } from "../output/text-formatter.ts"; interface CommandImpl { @@ -42,7 +48,11 @@ const testRoot = Command.make("supabase").pipe( Command.withGlobalFlags(GLOBAL_FLAGS), ); -/** Satisfies a tombstoned command's `withCommandTelemetry`/`withJsonErrorHandling` requirements. */ +/** + * Satisfies a tombstoned command's `withCommandTelemetry`/`withJsonErrorHandling` requirements, + * plus the `TelemetryRuntime` its own `Command.provide(telemetryStateLayer)` needs to construct + * `TelemetryState`. + */ function tombstoneRuntimeLayer() { const out = mockOutput({ format: "text" }); const analytics = mockAnalytics(); @@ -51,6 +61,7 @@ function tombstoneRuntimeLayer() { out.layer, analytics.layer, processControl.layer, + mockTelemetryRuntime(), BunServices.layer, CliOutput.layer(textCliOutputFormatter()), ); @@ -182,7 +193,7 @@ describe("native hidden flags", () => { expect.objectContaining({ preview: true }), expect.objectContaining({ backup: false }), expect.objectContaining({ useDocker: false }), - expect.objectContaining({ legacyBundle: true }), + expect.objectContaining({ legacyBundle: Option.some(true) }), expect.objectContaining({ useDocker: false }), expect.objectContaining({ legacyBundle: true }), expect.objectContaining({ all: false }), @@ -209,6 +220,10 @@ describe("native hidden flags", () => { }); describe("hidden subcommands", () => { + // Pins the tombstoned commands' real `telemetryStateLayer` flush (`Command.provide`) to a temp + // dir, so it never touches the host machine's `~/.supabase/telemetry.json`. + const tombstoneHome = useTempWorkdir("supabase-hidden-flag-tombstone-"); + it("omits hidden branch and db subcommands from help docs", () => { const branchesHelp = buildHelpDoc(branchesCommand); expect(branchesHelp.subcommands?.[0]?.commands.map((command) => command.name)).toEqual([ @@ -242,17 +257,22 @@ describe("hidden subcommands", () => { const causeOf = (exit: unknown) => (exit as { cause: { reasons: Array<{ _tag: string; error?: unknown }> } }).cause; + const runTombstone = (args: ReadonlyArray) => + withEnvVar( + "SUPABASE_HOME", + tombstoneHome.current, + Command.runWith(testRoot, { version: "0.0.0-test" })(args).pipe( + Effect.provide(layer), + Effect.exit, + ), + ) as Effect.Effect; + for (const args of [ ["db", "branch", "list"], ["db", "remote", "changes"], ["gen", "keys"], ]) { - const exit = await Effect.runPromise( - Command.runWith(testRoot, { version: "0.0.0-test" })(args).pipe( - Effect.provide(layer), - Effect.exit, - ) as Effect.Effect, - ); + const exit = await Effect.runPromise(runTombstone(args)); expect((exit as { _tag: string })._tag).toBe("Failure"); expect(causeOf(exit).reasons[0]?.error).toBeInstanceOf(RemovedSurfaceError); } diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index bc039a9275..146897f0ab 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -81,7 +81,7 @@ interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies readonly styleEmphasis?: (text: string) => string; /** * Optional shell-specific styling hook for the command suggested inside - * {@link suggestLegacyBundle} — same isolation rationale as {@link styleEmphasis}. + * {@link suggestUseApiRetry} — same isolation rationale as {@link styleEmphasis}. */ readonly styleAqua?: (text: string) => string; /** @@ -803,7 +803,7 @@ const downloadEszipBody = Effect.fnUntraced(function* ( ); }); -function suggestLegacyBundle( +function suggestUseApiRetry( slug: string, styleAqua: (text: string) => string = (text) => text, ): string { @@ -827,7 +827,7 @@ function suggestDenoV2(styleEmphasis: (text: string) => string = (text) => text) function withLegacyBundleSuggestion(slug: string, styleAqua?: (text: string) => string) { return (cause: unknown): Error => Object.assign(new Error(describeContainerCliFailure(cause)), { - suggestion: suggestLegacyBundle(slug, styleAqua), + suggestion: suggestUseApiRetry(slug, styleAqua), }); } @@ -840,7 +840,7 @@ function withLegacyBundleSuggestion(slug: string, styleAqua?: (text: string) => function withDockerStepFailure(step: string, slug: string, styleAqua?: (text: string) => string) { return (cause: unknown): Error => Object.assign(new Error(`${step}: ${describeContainerCliFailure(cause)}`), { - suggestion: suggestLegacyBundle(slug, styleAqua), + suggestion: suggestUseApiRetry(slug, styleAqua), }); } @@ -1015,7 +1015,7 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( .split(/\r?\n/) .some((line) => line.trim().toLowerCase() === "invalid eszip v2"); const suggestion = - (invalidEszipV2 ? suggestDenoV2(styleEmphasis) : "") + suggestLegacyBundle(slug, styleAqua); + (invalidEszipV2 ? suggestDenoV2(styleEmphasis) : "") + suggestUseApiRetry(slug, styleAqua); return yield* Effect.fail( Object.assign(new Error(`error running container: exit ${result.exitCode}`), { suggestion,