From fd0b568384c5d0bdf412933c0aa8379a0ffe2924 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 27 Jul 2026 20:26:10 +0100 Subject: [PATCH 01/24] fix(cli): render SMS/MFA config templates instead of emitting the raw text/template escape (#5944) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What changed `supabase init` (and the `bootstrap` scratch-project path — both go through the shared `renderProjectConfigTemplate` in `apps/cli/src/shared/init/project-init.templates.ts`) wrote two broken lines into the generated `supabase/config.toml`: ```toml template = "Your code is {{ `{{ .Code }}` }}" ``` for both `[auth.sms]` and `[auth.mfa.phone]`. The Go CLI's template source contains that `text/template` self-escape, but Go renders the scaffold through `text/template` (`config.Eject`, `pkg/config/config.go:555,572`), which resolves the backtick raw-string action to the literal GoTrue template. The TS renderer copied the escape verbatim and runs no template engine, so the raw escape landed in users' configs — a functionally broken SMS/MFA OTP template (GoTrue would send the literal text instead of substituting the code). The fix pre-renders the two lines in the TS template constant: ```toml template = "Your code is {{ .Code }}" ``` Verified byte-identical against the Go CLI's actual `config.Eject` output (dumped via a throwaway Go test harness): these two lines were the only divergence, and after the fix the full generated file matches Go's eject byte-for-byte, including the trailing newline. ## Test changes The existing byte-parity test compared the TS render against the **raw Go template source**, so both sides carried the unresolved escape and the test passed while the shipped output was broken. It now emulates Go's eject-time rendering (`resolveGoTemplateEscapes`) before comparing, so it would have caught this bug. Also added: - a dedicated regression test asserting the two generated `template =` lines are the rendered GoTrue templates, not raw Go escapes; - a guard test that fails loudly if the Go scaffold ever gains a `{{ ... }}` construct the suite does not model, so this class of parity drift cannot silently return (raised by review). ## Deliberately out of scope (from review) - **Configs already generated with the broken lines are not auto-migrated.** Users who ran `init`/`bootstrap` with an affected build keep the raw escape in their `config.toml`; the manual fix is replacing the two `template =` lines under `[auth.sms]` and `[auth.mfa.phone]` with `"Your code is {{ .Code }}"`. A detect-and-warn on config load (particularly on the `config push` path, which can carry the broken template to hosted GoTrue) was suggested in review as a possible fast-follow, but auto-editing or warning about user-owned config is a scope/UX decision beyond this parity fix. Fixes CLI-1971 Linear: https://linear.app/supabase/issue/CLI-1971/initbootstrap-write-broken-smsmfa-template-lines-into-configtoml --- .../src/shared/init/project-init.templates.ts | 4 +- .../init/project-init.templates.unit.test.ts | 45 ++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/apps/cli/src/shared/init/project-init.templates.ts b/apps/cli/src/shared/init/project-init.templates.ts index f3a41b6db8..2c6a823579 100644 --- a/apps/cli/src/shared/init/project-init.templates.ts +++ b/apps/cli/src/shared/init/project-init.templates.ts @@ -260,7 +260,7 @@ enable_signup = false # If enabled, users need to confirm their phone number before signing in. enable_confirmations = false # Template for sending OTP to users -template = "Your code is {{ \`{{ .Code }}\` }}" +template = "Your code is {{ .Code }}" # Controls the minimum amount of time that must pass before sending another sms otp. max_frequency = "5s" @@ -308,7 +308,7 @@ verify_enabled = false enroll_enabled = false verify_enabled = false otp_length = 6 -template = "Your code is {{ \`{{ .Code }}\` }}" +template = "Your code is {{ .Code }}" max_frequency = "5s" # Configure MFA via WebAuthn diff --git a/apps/cli/src/shared/init/project-init.templates.unit.test.ts b/apps/cli/src/shared/init/project-init.templates.unit.test.ts index 7b58bf1aea..650ef7ec88 100644 --- a/apps/cli/src/shared/init/project-init.templates.unit.test.ts +++ b/apps/cli/src/shared/init/project-init.templates.unit.test.ts @@ -21,16 +21,51 @@ function readGoTemplate(...segments: ReadonlyArray): string { return normalizeNewlines(readFileSync(join(goCliRoot, ...segments), "utf8")); } -describe("project init templates", () => { - it("renders config.toml with the same content as the Go scaffold", () => { - const expected = readGoTemplate("pkg", "config", "templates", "config.toml") +// Go renders its config.toml scaffold through text/template (config.Eject), so an action +// containing a backtick raw string — {{ (backtick){{ .Code }}(backtick) }} in the template +// source — is rendered to the literal string it quotes: {{ .Code }} in the ejected file. +// This is the only text/template construct emulated here beyond the substituted fields; +// the "models every template action" test below fails loudly if the Go scaffold ever +// gains a construct this suite does not resolve. +function resolveGoTemplateEscapes(template: string): string { + return template.replace(/\{\{\s*`([^`]*)`\s*\}\}/g, "$1"); +} + +// Emulates what Go's config.Eject writes to disk for a fresh `supabase init` project. +function renderExpectedGoEject(): string { + return ( + resolveGoTemplateEscapes(readGoTemplate("pkg", "config", "templates", "config.toml")) .replace("{{ .ProjectId }}", "demo-project") .replace("{{ .Experimental.OrioleDBVersion }}", "15.1.0.150") // supabase init always opts new projects into pg-delta; the Go template renders // this from a flag set only on the init path (false when deriving defaults). - .replace("{{ .Experimental.PgDeltaInitEnabled }}", "true"); + .replace("{{ .Experimental.PgDeltaInitEnabled }}", "true") + ); +} - expect(normalizeNewlines(renderProjectConfigTemplate("demo-project", true))).toBe(expected); +describe("project init templates", () => { + it("renders config.toml with the same content as the Go CLI ejects", () => { + expect(normalizeNewlines(renderProjectConfigTemplate("demo-project", true))).toBe( + renderExpectedGoEject(), + ); + }); + + it("models every template action in the Go scaffold, so parity cannot silently drift", () => { + // After escape resolution and field substitution, the only {{ ... }} occurrences left + // must be the GoTrue OTP placeholders quoted by the backtick escapes. Anything else + // means the Go template gained a construct this suite does not emulate yet — update + // resolveGoTemplateEscapes/renderExpectedGoEject to match config.Eject before shipping. + const unresolvedActions = renderExpectedGoEject().match(/\{\{[^}]*\}\}/g) ?? []; + expect(new Set(unresolvedActions)).toEqual(new Set(["{{ .Code }}"])); + }); + + it("renders the SMS and MFA phone OTP templates as GoTrue templates, not raw Go escapes", () => { + const rendered = renderProjectConfigTemplate("demo-project", false); + const otpTemplateLines = rendered.split("\n").filter((line) => line.startsWith("template = ")); + expect(otpTemplateLines).toEqual([ + 'template = "Your code is {{ .Code }}"', + 'template = "Your code is {{ .Code }}"', + ]); }); it("enables pg-delta by default in the generated config", () => { From 78c8a7f5d4ad48310462c1607b172f3ec779ca5d Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 27 Jul 2026 20:54:39 +0100 Subject: [PATCH 02/24] fix(cli): gate vanity-subdomains and network-restrictions behind --experimental (CLI-1972) (#5945) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Current Behavior Go registers `vanityCmd` and `restrictionsCmd` (alongside `bansCmd`) as experimental (`apps/cli-go/cmd/root.go:56-65`) and refuses them in `PersistentPreRunE` with exit 1 and `must set the --experimental flag to run this command` — before login, ref resolution, telemetry, or any API call (`root.go:91-96`). The TS legacy shell only reproduced this gate for `network-bans`. `vanity-subdomains get|check-availability|activate|delete` and `network-restrictions get|update` had zero gate calls: they ran to completion without `--experimental` — API call made, exit 0, `cli_command_executed` fired, `linked-project.json` written — where Go does none of that. ## Expected Behavior All six leaves now fail with Go's exact gate message (exit 1) unless `--experimental` or `SUPABASE_EXPERIMENTAL` is set, with no API call, no telemetry event, and no `linked-project.json`/`telemetry.json` write behind a closed gate. The wiring mirrors the merged CLI-1854 precedent (#5766) and the in-repo `network-bans` reference exactly: - `legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS)` runs first, so an invalid `-o` value still wins over a missing `--experimental` (cobra enum-validates root's persistent `-o` at parse time, before `PersistentPreRunE`). - `legacyRequireExperimental` runs next, before any instrumentation or layer work. - `legacyManagementApiRuntimeLayer` moves from `Command.provide` to an inline `Effect.provide` after the gate — `Command.provide` builds the layer (and eagerly resolves an access token) before the handler's first yield, which would let a missing-token error mask the gate error, exactly the failure mode CLI-1854 fixed for the other three families. New gate integration suites for both families mirror `network-bans.experimental-gate.integration.test.ts`: a closed gate fails with `LegacyExperimentalRequiredError` and zero API requests; an open gate proceeds past the gate to the management-API auth step. The six `SIDE_EFFECTS.md` files document the gated file writes, the `SUPABASE_EXPERIMENTAL` env var, the new exit-code row, and the closed-gate telemetry behavior. ## Deliberately-open judgement calls - `vanity-subdomains activate|check-availability` mark `--desired-subdomain` required. When **both** it and `--experimental` are omitted, the TS parser reports the missing-required-flag error where Go's gate error wins (cobra runs `ValidateRequiredFlags` only after `PersistentPreRunE`). This is a pre-existing framework-level parse-ordering divergence, not introduced here — exit code is 1 either way and each error names the exact flag to add. Documented in both commands' `SIDE_EFFECTS.md`; these are the first gated leaves with a required flag, so none of the merged gate PRs hit this. - The gate-wiring comment/boilerplate is intentionally duplicated per leaf to stay byte-consistent with the merged network-bans/postgres-config/ssl-enforcement precedents rather than hoisting a shared helper in this fix. - Release-note callout: anyone who scripted these six commands against the TS legacy shell during the ungated window will now get the hard gate error, matching Go. The message names the fix (`--experimental`), and `SUPABASE_EXPERIMENTAL` remains the env escape hatch. ## Related Issue(s) Fixes CLI-1972 https://linear.app/supabase/issue/CLI-1972/missing-experimental-gate-on-vanity-subdomains-and-network --- .../network-restrictions/get/SIDE_EFFECTS.md | 42 +++--- .../network-restrictions/get/get.command.ts | 31 +++- ...ions.experimental-gate.integration.test.ts | 121 +++++++++++++++ .../update/SIDE_EFFECTS.md | 47 +++--- .../update/update.command.ts | 32 +++- .../activate/SIDE_EFFECTS.md | 53 ++++--- .../activate/activate.command.ts | 38 ++++- .../activate/activate.handler.ts | 18 ++- .../check-availability/SIDE_EFFECTS.md | 51 ++++--- .../check-availability.command.ts | 40 ++++- .../check-availability.handler.ts | 20 ++- .../vanity-subdomains/delete/SIDE_EFFECTS.md | 42 +++--- .../delete/delete.command.ts | 31 +++- .../vanity-subdomains/get/SIDE_EFFECTS.md | 42 +++--- .../vanity-subdomains/get/get.command.ts | 31 +++- .../vanity-subdomains.errors.ts | 16 ++ ...ains.experimental-gate.integration.test.ts | 135 +++++++++++++++++ .../vanity-subdomains.integration.test.ts | 139 ++++++++++++++++-- 18 files changed, 756 insertions(+), 173 deletions(-) create mode 100644 apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts create mode 100644 apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.experimental-gate.integration.test.ts diff --git a/apps/cli/src/legacy/commands/network-restrictions/get/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/network-restrictions/get/SIDE_EFFECTS.md index f39c04d7c9..0298b977ae 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/get/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/network-restrictions/get/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ----------------------------------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | always (after ref resolution), via `Effect.ensuring` — on success and failure | -| `~/.supabase/telemetry.json` | JSON | always, via `Effect.ensuring` — on success and failure | +| Path | Format | When | +| ------------------------------------------------ | ------ | -------------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if the gate is closed. | ## API Routes @@ -22,26 +22,28 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | -------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | 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`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref` → prompt) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | 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`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref` → prompt) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------- | -| `0` | success — network-restrictions status printed to stdout | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-200 (`LegacyNetworkRestrictionsGetUnexpectedStatusError`) | -| `1` | transport failure (`LegacyNetworkRestrictionsGetNetworkError`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success — network-restrictions status printed to stdout | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-200 (`LegacyNetworkRestrictionsGetUnexpectedStatusError`) | +| `1` | transport failure (`LegacyNetworkRestrictionsGetNetworkError`) | ## 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` → ``) | +| Event | When | Notable properties / groups | +| ---------------------- | ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` (`--project-ref` → ``) | Matches `apps/cli-go/internal/restrictions/get/`. Go does not fire any custom telemetry event for this command. @@ -90,6 +92,8 @@ One `result` event whose `data` is the full response object. - The Go `--output` flag wins over the TS `--output-format` flag when both are provided. - `linked-project.json` is written **after** the project ref is resolved, regardless of whether the subsequent API call succeeds (mirrors Go's `PersistentPostRun`). -- `telemetry.json` is written on every invocation, including failures. +- `telemetry.json` is written on every invocation past the `--experimental` gate, including + failures. A closed gate writes nothing (Go's `PersistentPreRunE` fails before + `PersistentPostRun` runs). - Go's `restrictions/get` itself does not honor `--output`. The legacy TS port honors both `--output` and `--output-format` per the legacy CLAUDE.md output-parity rules. diff --git a/apps/cli/src/legacy/commands/network-restrictions/get/get.command.ts b/apps/cli/src/legacy/commands/network-restrictions/get/get.command.ts index b9105e5d74..3ec90f170c 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/get/get.command.ts +++ b/apps/cli/src/legacy/commands/network-restrictions/get/get.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyNetworkRestrictionsGet } from "./get.handler.ts"; const config = { @@ -19,10 +25,23 @@ export const legacyNetworkRestrictionsGetCommand = Command.make("get", config).p Command.withDescription("Get the current network restrictions."), Command.withShortDescription("Get the current network restrictions"), Command.withHandler((flags) => - legacyNetworkRestrictionsGet(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `restrictionsCmd` (network-restrictions) behind `--experimental` in + // PersistentPreRunE (root.go:91-96) BEFORE the `IsManagementAPI` login check + // (root.go:105-109). `legacyManagementApiRuntimeLayer` eagerly resolves an + // access token as part of building its `LegacyPlatformApi` layer, so it must + // be provided AFTER the gate (inline here) rather than via `Command.provide` + // on the whole command — `Command.provide` would build the layer, and fail on + // a missing token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyNetworkRestrictionsGet(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide(legacyManagementApiRuntimeLayer(["network-restrictions", "get"])), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["network-restrictions", "get"])), ); diff --git a/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts b/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts new file mode 100644 index 0000000000..57acaad0b9 --- /dev/null +++ b/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit, Layer } from "effect"; +import { CliOutput, Command } from "effect/unstable/cli"; + +import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; +import { LEGACY_GLOBAL_FLAGS } from "../../../shared/legacy/global-flags.ts"; +import { TelemetryRuntime } from "../../../shared/telemetry/runtime.service.ts"; +import { makeTelemetryIdentity } from "../../../shared/telemetry/identity.ts"; +import { mockOutput, mockRuntimeInfo, processEnvLayer } from "../../../../tests/helpers/mocks.ts"; +import { + buildLegacyTestRuntime, + mockLegacyCliConfig, + mockLegacyPlatformApi, + useLegacyTempWorkdir, +} from "../../../../tests/helpers/legacy-mocks.ts"; +import { legacyNetworkRestrictionsCommand } from "./network-restrictions.command.ts"; + +// See postgres-config.experimental-gate.integration.test.ts for the full +// rationale: this proves `--experimental` is wired into the actual +// `.command.ts` handler pipeline AND runs before +// `legacyManagementApiRuntimeLayer`'s eager access-token resolution +// (Go's `IsExperimental` check precedes `IsManagementAPI` in +// `apps/cli-go/cmd/root.go:91-109`). + +const tempRoot = useLegacyTempWorkdir("supabase-network-restrictions-experimental-int-"); + +const testRoot = Command.make("supabase").pipe( + Command.withGlobalFlags(LEGACY_GLOBAL_FLAGS), + Command.withSubcommands([legacyNetworkRestrictionsCommand]), +); + +function setup() { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + response: { status: 200, body: { config: { dbAllowedCidrs: [] }, status: "applied" } }, + }); + const runtime = buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + // `RuntimeInfo` is ambient (not provided by `legacyManagementApiRuntimeLayer` + // itself), so the real `legacyCredentialsLayer` built inline inside the + // command for the "gate open" case resolves ITS `RuntimeInfo` from this + // layer. Point homeDir at this test's isolated tempRoot so the layer's + // file-based token fallback (`/.supabase/access-token`) can't pick + // up a stray token left at the shared default `/tmp/supabase-cli-test-home`. + runtimeInfo: mockRuntimeInfo({ homeDir: tempRoot.current }), + }); + const layer = Layer.mergeAll( + runtime, + CliOutput.layer(textCliOutputFormatter()), + // The "gate open" case reaches the real `legacyManagementApiRuntimeLayer` + // (provided inline inside the command, not by this test's mocked runtime), + // which reads credentials/env directly — an ambient SUPABASE_ACCESS_TOKEN, + // SUPABASE_EXPERIMENTAL, or OS keyring entry on the machine running the + // test would make these assertions non-deterministic. Wipe process.env + // down to just this and disable the keyring fallback. + processEnvLayer({ SUPABASE_NO_KEYRING: "1" }), + Layer.succeed( + TelemetryRuntime, + TelemetryRuntime.of({ + configDir: `${tempRoot.current}/.supabase`, + tracesDir: `${tempRoot.current}/.supabase/traces`, + consent: "granted", + showDebug: false, + deviceId: "test-device-id", + sessionId: "test-session-id", + identity: makeTelemetryIdentity(undefined), + isFirstRun: false, + isTty: false, + isCi: false, + os: "linux", + arch: "x64", + cliVersion: "0.1.0", + }), + ), + ); + return { layer, api }; +} + +describe("legacy network-restrictions experimental gate (Go PersistentPreRunE parity)", () => { + const leaves: ReadonlyArray<{ readonly name: string; readonly args: ReadonlyArray }> = [ + { name: "get", args: ["network-restrictions", "get"] }, + { name: "update", args: ["network-restrictions", "update"] }, + ]; + + for (const { name, args } of leaves) { + it.live( + `${name} fails with LegacyExperimentalRequiredError when --experimental is unset`, + () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })(args), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("LegacyExperimentalRequiredError"); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live(`${name} does not fail with the gate error once --experimental is set`, () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })([...args, "--experimental"]), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + const causeText = JSON.stringify(exit.cause); + expect(causeText).not.toContain("LegacyExperimentalRequiredError"); + expect(causeText).toContain("LegacyPlatformAuthRequiredError"); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + } +}); diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md index f30b5ed87e..3eb0cd2630 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ------------------------------------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | always (after ref resolution), via `Effect.ensuring` — success and HTTP failure | -| `~/.supabase/telemetry.json` | JSON | always, via outermost `Effect.ensuring` — including CIDR validation failures | +| Path | Format | When | +| ------------------------------------------------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------ | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — success and HTTP failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via outermost `Effect.ensuring` — including CIDR validation failures. Not written if closed. | ## API Routes @@ -28,28 +28,30 @@ when no `--db-allow-cidr` was supplied), matching Go's `&[]string{}` initializat ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | -------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | 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`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref` → prompt) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | 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`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref` → prompt) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | ------------------------------------------------------------------------------------------------- | -| `0` | success — network restrictions updated and status printed to stdout | -| `1` | CIDR parse failure — `LegacyNetworkRestrictionsInvalidCidrError` (`failed to parse IP: `) | -| `1` | private-IP rejection — `LegacyNetworkRestrictionsPrivateIpError` (`private IP provided: `) | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-201 (POST) / non-200 (PATCH) — `LegacyNetworkRestrictionsUpdateUnexpectedStatusError` | -| `1` | transport failure — `LegacyNetworkRestrictionsUpdateNetworkError` | +| Code | Condition | +| ---- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| `0` | success — network restrictions updated and status printed to stdout | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before CIDR validation/ref/API | +| `1` | CIDR parse failure — `LegacyNetworkRestrictionsInvalidCidrError` (`failed to parse IP: `) | +| `1` | private-IP rejection — `LegacyNetworkRestrictionsPrivateIpError` (`private IP provided: `) | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-201 (POST) / non-200 (PATCH) — `LegacyNetworkRestrictionsUpdateUnexpectedStatusError` | +| `1` | transport failure — `LegacyNetworkRestrictionsUpdateNetworkError` | ## 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` → ``) | +| Event | When | Notable properties / groups | +| ---------------------- | ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` (`--project-ref` → ``) | Matches `apps/cli-go/internal/restrictions/update/`. Go does not fire any custom telemetry event for this command. @@ -109,7 +111,8 @@ One `result` event whose `data` is the full response object. envelope (`{ dbAllowedCidrs, dbAllowedCidrsV6 }` → `{ add: { dbAllowedCidrs, dbAllowedCidrsV6 } }`). - `linked-project.json` writes after a successful project-ref resolution, regardless of whether the subsequent API call succeeds. -- `telemetry.json` writes on every invocation, including CIDR validation failures, ref - resolution failures, and API failures. +- `telemetry.json` writes on every invocation past the `--experimental` gate, including + CIDR validation failures, ref resolution failures, and API failures. A closed gate + writes nothing (Go's `PersistentPreRunE` fails before `PersistentPostRun` runs). - Go's `restrictions/update` itself does not honor `--output`. The legacy TS port honors both `--output` and `--output-format` per the legacy CLAUDE.md output-parity rules. diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts index 047d17eff5..a2e3cc0667 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts +++ b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyNetworkRestrictionsUpdate } from "./update.handler.ts"; const config = { @@ -29,10 +35,24 @@ export const legacyNetworkRestrictionsUpdateCommand = Command.make("update", con Command.withDescription("Update network restrictions."), Command.withShortDescription("Update network restrictions"), Command.withHandler((flags) => - legacyNetworkRestrictionsUpdate(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `restrictionsCmd` (network-restrictions) behind `--experimental` in + // PersistentPreRunE (root.go:91-96) BEFORE the `IsManagementAPI` login check + // (root.go:105-109) — and before RunE, so the gate also precedes this command's + // local CIDR validation. `legacyManagementApiRuntimeLayer` eagerly resolves an + // access token as part of building its `LegacyPlatformApi` layer, so it must + // be provided AFTER the gate (inline here) rather than via `Command.provide` + // on the whole command — `Command.provide` would build the layer, and fail on + // a missing token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyNetworkRestrictionsUpdate(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide(legacyManagementApiRuntimeLayer(["network-restrictions", "update"])), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["network-restrictions", "update"])), ); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/activate/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/vanity-subdomains/activate/SIDE_EFFECTS.md index aace0d1a0b..9ad8fa4b34 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/activate/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/vanity-subdomains/activate/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ----------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | after ref resolution, on success and failure | -| `~/.supabase/telemetry.json` | JSON | always, via `Effect.ensuring`, on success and failure | +| Path | Format | When | +| ------------------------------------------------ | ------ | ---------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if gate is closed. | ## API Routes @@ -22,27 +22,30 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | ---------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | +| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-2xx (`LegacyVanitySubdomainsActivateUnexpectedStatusError`) | -| `1` | transport failure (`LegacyVanitySubdomainsActivateNetworkError`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | `--desired-subdomain` omitted (`LegacyDesiredSubdomainRequiredError`) — checked after gate/login/ref resolution; telemetry still fires | +| `1` | API non-2xx (`LegacyVanitySubdomainsActivateUnexpectedStatusError`) | +| `1` | transport failure (`LegacyVanitySubdomainsActivateNetworkError`) | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ----------------------- | ------------------------------------------ | ------------------------------------------ | -| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` | -| `cli_upgrade_suggested` | gated 4xx responses only | `feature_key=vanity_subdomain`, `org_slug` | +| Event | When | Notable properties / groups | +| ----------------------- | ---------------------------------------------------------------------------------------------- | ------------------------------------------ | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` | +| `cli_upgrade_suggested` | gated 4xx responses only | `feature_key=vanity_subdomain`, `org_slug` | ## Output @@ -69,5 +72,15 @@ One `result` event with the full response object. ## Notes - The legacy `--output` flag wins over TS `--output-format` when both are provided. -- `linked-project.json` is written after ref resolution, even when the API call fails. +- `linked-project.json` is written after ref resolution (once the `--experimental` gate is open), + even when the API call fails. A closed gate writes nothing (Go's `PersistentPreRunE` fails + before `PersistentPostRun` runs). - On gated 4xx responses this command prints an upgrade suggestion and fires `cli_upgrade_suggested`. +- `--desired-subdomain` is required in Go (`cmd/vanitySubdomains.go:67`) but cobra validates + required flags only after `PersistentPreRunE` (gate → login → ref resolution), so the TS flag + is optional at parse time and enforced in the handler with cobra's exact wording + (`required flag(s) "desired-subdomain" not set`). The gate/login/ref errors win over the + missing flag, matching Go's ordering, and — as in Go, where `PersistentPostRun` still runs — + telemetry and `linked-project.json` are written on this failure. An explicit empty value + (`--desired-subdomain ""`) passes the check and reaches the API, as cobra only requires the + flag to be set. diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.command.ts b/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.command.ts index c17e24138c..2824618a25 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.command.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyVanitySubdomainsActivate } from "./activate.handler.ts"; const config = { @@ -11,8 +17,15 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Go marks this flag required (`cmd/vanitySubdomains.go:67`), but cobra + // validates required flags only AFTER `PersistentPreRunE` + // (`cobra@v1.10.2/command.go:985,1005`) — so the `--experimental` gate, + // login check, and project-ref resolution must all win over a missing + // `--desired-subdomain`. Optional at parse time; presence is enforced in + // the handler (after ref resolution) instead. desiredSubdomain: Flag.string("desired-subdomain").pipe( Flag.withDescription("The desired vanity subdomain to use for your Supabase project."), + Flag.optional, ), } as const; @@ -24,10 +37,23 @@ export const legacyVanitySubdomainsActivateCommand = Command.make("activate", co ), Command.withShortDescription("Activate a vanity subdomain"), Command.withHandler((flags) => - legacyVanitySubdomainsActivate(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `vanityCmd` (vanity-subdomains) behind `--experimental` in PersistentPreRunE + // (root.go:91-96) BEFORE the `IsManagementAPI` login check (root.go:105-109). + // `legacyManagementApiRuntimeLayer` eagerly resolves an access token as part + // of building its `LegacyPlatformApi` layer, so it must be provided AFTER + // the gate (inline here) rather than via `Command.provide` on the whole + // command — `Command.provide` would build the layer, and fail on a missing + // token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyVanitySubdomainsActivate(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "activate"])), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "activate"])), ); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.handler.ts b/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.handler.ts index 27877ce2c5..0b7af30cd8 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.handler.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/activate/activate.handler.ts @@ -15,6 +15,7 @@ import { } from "../../../shared/legacy-go-output.encoders.ts"; import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts"; import { + LegacyDesiredSubdomainRequiredError, LegacyVanitySubdomainsActivateNetworkError, LegacyVanitySubdomainsActivateUnexpectedStatusError, } from "../vanity-subdomains.errors.ts"; @@ -40,6 +41,21 @@ export const legacyVanitySubdomainsActivate = Effect.fn("legacy.vanity-subdomain const ref = yield* resolver.resolve(flags.projectRef); yield* Effect.gen(function* () { + // Go validates the required `--desired-subdomain` only after + // `PersistentPreRunE` completes (gate → login → ref resolution, + // `cmd/root.go:93-117`; `cobra@v1.10.2/command.go:985,1005`), and + // `PersistentPostRun` still fires telemetry + the linked-project cache + // on that failure — hence this check sits inside both `Effect.ensuring` + // wrappers, after ref resolution. Cobra checks the flag was *changed*, + // not non-empty, so `--desired-subdomain ""` passes and reaches the API. + if (Option.isNone(flags.desiredSubdomain)) { + return yield* Effect.fail( + new LegacyDesiredSubdomainRequiredError({ + message: `required flag(s) "desired-subdomain" not set`, + }), + ); + } + const desiredSubdomain = flags.desiredSubdomain.value; const activating = output.format === "text" ? yield* output.task("Activating vanity subdomain...") @@ -47,7 +63,7 @@ export const legacyVanitySubdomainsActivate = Effect.fn("legacy.vanity-subdomain const response = yield* api.v1 .activateVanitySubdomainConfig({ ref, - vanity_subdomain: flags.desiredSubdomain, + vanity_subdomain: desiredSubdomain, }) .pipe( Effect.tapError(() => activating?.fail() ?? Effect.void), diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/SIDE_EFFECTS.md index 9fc14225dc..180330a9c5 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ----------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | after ref resolution, on success and failure | -| `~/.supabase/telemetry.json` | JSON | always, via `Effect.ensuring`, on success and failure | +| Path | Format | When | +| ------------------------------------------------ | ------ | ---------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if gate is closed. | ## API Routes @@ -22,26 +22,29 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | ---------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | +| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-2xx (`LegacyVanitySubdomainsCheckUnexpectedStatusError`) | -| `1` | transport failure (`LegacyVanitySubdomainsCheckNetworkError`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | `--desired-subdomain` omitted (`LegacyDesiredSubdomainRequiredError`) — checked after gate/login/ref resolution; telemetry still fires | +| `1` | API non-2xx (`LegacyVanitySubdomainsCheckUnexpectedStatusError`) | +| `1` | transport failure (`LegacyVanitySubdomainsCheckNetworkError`) | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ---------------------- | ------------------------------------------ | ----------------------------------- | -| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` | +| Event | When | Notable properties / groups | +| ---------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` | This command may print an upgrade suggestion for gated 4xx responses, but it does not fire `cli_upgrade_suggested`. @@ -71,4 +74,14 @@ One `result` event with the full response object. ## Notes - The legacy `--output` flag wins over TS `--output-format` when both are provided. -- `linked-project.json` is written after ref resolution, even when the API call fails. +- `linked-project.json` is written after ref resolution (once the `--experimental` gate is open), + even when the API call fails. A closed gate writes nothing (Go's `PersistentPreRunE` fails + before `PersistentPostRun` runs). +- `--desired-subdomain` is required in Go (`cmd/vanitySubdomains.go:69`) but cobra validates + required flags only after `PersistentPreRunE` (gate → login → ref resolution), so the TS flag + is optional at parse time and enforced in the handler with cobra's exact wording + (`required flag(s) "desired-subdomain" not set`). The gate/login/ref errors win over the + missing flag, matching Go's ordering, and — as in Go, where `PersistentPostRun` still runs — + telemetry and `linked-project.json` are written on this failure. An explicit empty value + (`--desired-subdomain ""`) passes the check and reaches the API, as cobra only requires the + flag to be set. diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.command.ts b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.command.ts index 934d6f92e3..a015585fab 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.command.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyVanitySubdomainsCheckAvailability } from "./check-availability.handler.ts"; const config = { @@ -11,8 +17,15 @@ const config = { Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), + // Go marks this flag required (`cmd/vanitySubdomains.go:69`), but cobra + // validates required flags only AFTER `PersistentPreRunE` + // (`cobra@v1.10.2/command.go:985,1005`) — so the `--experimental` gate, + // login check, and project-ref resolution must all win over a missing + // `--desired-subdomain`. Optional at parse time; presence is enforced in + // the handler (after ref resolution) instead. desiredSubdomain: Flag.string("desired-subdomain").pipe( Flag.withDescription("The desired vanity subdomain to use for your Supabase project."), + Flag.optional, ), } as const; @@ -27,10 +40,25 @@ export const legacyVanitySubdomainsCheckAvailabilityCommand = Command.make( Command.withDescription("Checks if a desired subdomain is available for use."), Command.withShortDescription("Check subdomain availability"), Command.withHandler((flags) => - legacyVanitySubdomainsCheckAvailability(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `vanityCmd` (vanity-subdomains) behind `--experimental` in PersistentPreRunE + // (root.go:91-96) BEFORE the `IsManagementAPI` login check (root.go:105-109). + // `legacyManagementApiRuntimeLayer` eagerly resolves an access token as part + // of building its `LegacyPlatformApi` layer, so it must be provided AFTER + // the gate (inline here) rather than via `Command.provide` on the whole + // command — `Command.provide` would build the layer, and fail on a missing + // token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyVanitySubdomainsCheckAvailability(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide( + legacyManagementApiRuntimeLayer(["vanity-subdomains", "check-availability"]), + ), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "check-availability"])), ); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.handler.ts b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.handler.ts index d17836e693..f136f28298 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.handler.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/check-availability/check-availability.handler.ts @@ -15,6 +15,7 @@ import { } from "../../../shared/legacy-go-output.encoders.ts"; import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts"; import { + LegacyDesiredSubdomainRequiredError, LegacyVanitySubdomainsCheckNetworkError, LegacyVanitySubdomainsCheckUnexpectedStatusError, } from "../vanity-subdomains.errors.ts"; @@ -41,6 +42,21 @@ export const legacyVanitySubdomainsCheckAvailability = Effect.fn( const ref = yield* resolver.resolve(flags.projectRef); yield* Effect.gen(function* () { + // Go validates the required `--desired-subdomain` only after + // `PersistentPreRunE` completes (gate → login → ref resolution, + // `cmd/root.go:93-117`; `cobra@v1.10.2/command.go:985,1005`), and + // `PersistentPostRun` still fires telemetry + the linked-project cache + // on that failure — hence this check sits inside both `Effect.ensuring` + // wrappers, after ref resolution. Cobra checks the flag was *changed*, + // not non-empty, so `--desired-subdomain ""` passes and reaches the API. + if (Option.isNone(flags.desiredSubdomain)) { + return yield* Effect.fail( + new LegacyDesiredSubdomainRequiredError({ + message: `required flag(s) "desired-subdomain" not set`, + }), + ); + } + const desiredSubdomain = flags.desiredSubdomain.value; const checking = output.format === "text" ? yield* output.task("Checking vanity subdomain availability...") @@ -48,7 +64,7 @@ export const legacyVanitySubdomainsCheckAvailability = Effect.fn( const response = yield* api.v1 .checkVanitySubdomainAvailability({ ref, - vanity_subdomain: flags.desiredSubdomain, + vanity_subdomain: desiredSubdomain, }) .pipe( Effect.tapError(() => checking?.fail() ?? Effect.void), @@ -97,7 +113,7 @@ export const legacyVanitySubdomainsCheckAvailability = Effect.fn( return; } - yield* output.raw(`Subdomain ${flags.desiredSubdomain} available: ${response.available}\n`); + yield* output.raw(`Subdomain ${desiredSubdomain} available: ${response.available}\n`); }).pipe(Effect.ensuring(linkedProjectCache.cache(ref))); }).pipe(Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/delete/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/vanity-subdomains/delete/SIDE_EFFECTS.md index 5e67410233..0e9d2f1708 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/delete/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/vanity-subdomains/delete/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ----------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | after ref resolution, on success and failure | -| `~/.supabase/telemetry.json` | JSON | always, via `Effect.ensuring`, on success and failure | +| Path | Format | When | +| ------------------------------------------------ | ------ | ---------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if gate is closed. | ## API Routes @@ -22,26 +22,28 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | ---------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | +| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-2xx (`LegacyVanitySubdomainsDeleteUnexpectedStatusError`) | -| `1` | transport failure (`LegacyVanitySubdomainsDeleteNetworkError`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-2xx (`LegacyVanitySubdomainsDeleteUnexpectedStatusError`) | +| `1` | transport failure (`LegacyVanitySubdomainsDeleteNetworkError`) | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ---------------------- | ------------------------------------------ | ----------------------------------- | -| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` | +| Event | When | Notable properties / groups | +| ---------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` | ## Output @@ -68,4 +70,6 @@ One `result` event when the legacy `--output` flag is unset. ## Notes - The legacy `--output` flag wins over TS `--output-format` when both are provided. -- `linked-project.json` is written after ref resolution, even when the API call fails. +- `linked-project.json` is written after ref resolution (once the `--experimental` gate is open), + even when the API call fails. A closed gate writes nothing (Go's `PersistentPreRunE` fails + before `PersistentPostRun` runs). diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/delete/delete.command.ts b/apps/cli/src/legacy/commands/vanity-subdomains/delete/delete.command.ts index 2a6200c66e..10a798500f 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/delete/delete.command.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/delete/delete.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyVanitySubdomainsDelete } from "./delete.handler.ts"; const config = { @@ -21,10 +27,23 @@ export const legacyVanitySubdomainsDeleteCommand = Command.make("delete", config ), Command.withShortDescription("Delete the vanity subdomain"), Command.withHandler((flags) => - legacyVanitySubdomainsDelete(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `vanityCmd` (vanity-subdomains) behind `--experimental` in PersistentPreRunE + // (root.go:91-96) BEFORE the `IsManagementAPI` login check (root.go:105-109). + // `legacyManagementApiRuntimeLayer` eagerly resolves an access token as part + // of building its `LegacyPlatformApi` layer, so it must be provided AFTER + // the gate (inline here) rather than via `Command.provide` on the whole + // command — `Command.provide` would build the layer, and fail on a missing + // token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyVanitySubdomainsDelete(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "delete"])), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "delete"])), ); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/get/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/vanity-subdomains/get/SIDE_EFFECTS.md index 73b367ec63..80a46f4360 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/get/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/vanity-subdomains/get/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ----------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | after ref resolution, on success and failure | -| `~/.supabase/telemetry.json` | JSON | always, via `Effect.ensuring`, on success and failure | +| Path | Format | When | +| ------------------------------------------------ | ------ | ---------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if gate is closed. | ## API Routes @@ -22,26 +22,28 @@ ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ---------------------------------------------------- | ---------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| Variable | Purpose | Required? | +| ----------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring then `~/.supabase/access-token`) | +| `SUPABASE_PROFILE` | built-in profile name or YAML file path | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | project ref fallback when `--project-ref` is unset | no (falls back to `supabase/.temp/project-ref`) | +| `SUPABASE_EXPERIMENTAL` | enables `--experimental`-gated commands without the flag | no (pass `--experimental` instead; one of the two is required) | ## Exit Codes -| Code | Condition | -| ---- | --------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-2xx (`LegacyVanitySubdomainsGetUnexpectedStatusError`) | -| `1` | transport failure (`LegacyVanitySubdomainsGetNetworkError`) | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-2xx (`LegacyVanitySubdomainsGetUnexpectedStatusError`) | +| `1` | transport failure (`LegacyVanitySubdomainsGetNetworkError`) | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ---------------------- | ------------------------------------------ | ----------------------------------- | -| `cli_command_executed` | post-run, success or failure (via wrapper) | `exit_code`, `duration_ms`, `flags` | +| Event | When | Notable properties / groups | +| ---------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` | ## Output @@ -71,4 +73,6 @@ One `result` event with the full response object. ## Notes - The legacy `--output` flag wins over TS `--output-format` when both are provided. -- `linked-project.json` is written after ref resolution, even when the API call fails. +- `linked-project.json` is written after ref resolution (once the `--experimental` gate is open), + even when the API call fails. A closed gate writes nothing (Go's `PersistentPreRunE` fails + before `PersistentPostRun` runs). diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/get/get.command.ts b/apps/cli/src/legacy/commands/vanity-subdomains/get/get.command.ts index 76c8e87f50..9dd58b19f3 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/get/get.command.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/get/get.command.ts @@ -1,9 +1,15 @@ +import { Effect } from "effect"; import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; +import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; +import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; -import { withLegacyCommandInstrumentation } from "../../../telemetry/legacy-command-instrumentation.ts"; +import { + legacyValidateOutputFormat, + withLegacyCommandInstrumentation, +} from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyVanitySubdomainsGet } from "./get.handler.ts"; const config = { @@ -19,10 +25,23 @@ export const legacyVanitySubdomainsGetCommand = Command.make("get", config).pipe Command.withDescription("Get the current vanity subdomain."), Command.withShortDescription("Get the current vanity subdomain"), Command.withHandler((flags) => - legacyVanitySubdomainsGet(flags).pipe( - withLegacyCommandInstrumentation({ flags }), - withJsonErrorHandling, - ), + Effect.gen(function* () { + // Cobra parses flags — rejecting an out-of-enum `-o` (`internal/utils/enum.go:21-27`) + // — before `PersistentPreRunE` ever runs (`cobra@v1.10.2/command.go:919,985`), so an + // invalid `-o` value must win over a missing `--experimental` flag. + yield* legacyValidateOutputFormat(LEGACY_RESOURCE_OUTPUT_FORMATS); + // Go gates `vanityCmd` (vanity-subdomains) behind `--experimental` in PersistentPreRunE + // (root.go:91-96) BEFORE the `IsManagementAPI` login check (root.go:105-109). + // `legacyManagementApiRuntimeLayer` eagerly resolves an access token as part + // of building its `LegacyPlatformApi` layer, so it must be provided AFTER + // the gate (inline here) rather than via `Command.provide` on the whole + // command — `Command.provide` would build the layer, and fail on a missing + // token, before this generator's first `yield*` ever runs. + yield* legacyRequireExperimental; + return yield* legacyVanitySubdomainsGet(flags).pipe( + withLegacyCommandInstrumentation({ flags }), + Effect.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "get"])), + ); + }).pipe(withJsonErrorHandling), ), - Command.provide(legacyManagementApiRuntimeLayer(["vanity-subdomains", "get"])), ); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.errors.ts b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.errors.ts index aa849f89ef..7a06a00976 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.errors.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.errors.ts @@ -1,5 +1,21 @@ import { Data } from "effect"; +/** + * Raised by the `activate` and `check-availability` handlers when + * `--desired-subdomain` is omitted. Go marks the flag required + * (`cmd/vanitySubdomains.go:67,69`) but cobra validates required flags only + * AFTER `PersistentPreRunE` (`cobra@v1.10.2/command.go:985,1005`) — i.e. after + * the `--experimental` gate, login check, and project-ref resolution + * (`cmd/root.go:93-117`) — so the flag is optional at parse time and enforced + * in the handler instead. Byte-matches cobra's required-flag wording + * (`command.go:1198`), same pattern as `LegacyProjectRefRequiredError`. + */ +export class LegacyDesiredSubdomainRequiredError extends Data.TaggedError( + "LegacyDesiredSubdomainRequiredError", +)<{ + readonly message: string; +}> {} + export class LegacyVanitySubdomainsGetNetworkError extends Data.TaggedError( "LegacyVanitySubdomainsGetNetworkError", )<{ diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.experimental-gate.integration.test.ts b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.experimental-gate.integration.test.ts new file mode 100644 index 0000000000..f87a5c8ebc --- /dev/null +++ b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.experimental-gate.integration.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit, Layer } from "effect"; +import { CliOutput, Command } from "effect/unstable/cli"; + +import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; +import { LEGACY_GLOBAL_FLAGS } from "../../../shared/legacy/global-flags.ts"; +import { TelemetryRuntime } from "../../../shared/telemetry/runtime.service.ts"; +import { makeTelemetryIdentity } from "../../../shared/telemetry/identity.ts"; +import { mockOutput, mockRuntimeInfo, processEnvLayer } from "../../../../tests/helpers/mocks.ts"; +import { + buildLegacyTestRuntime, + mockLegacyCliConfig, + mockLegacyPlatformApi, + useLegacyTempWorkdir, +} from "../../../../tests/helpers/legacy-mocks.ts"; +import { legacyVanitySubdomainsCommand } from "./vanity-subdomains.command.ts"; + +// See postgres-config.experimental-gate.integration.test.ts for the full +// rationale: this proves `--experimental` is wired into the actual +// `.command.ts` handler pipeline AND runs before +// `legacyManagementApiRuntimeLayer`'s eager access-token resolution +// (Go's `IsExperimental` check precedes `IsManagementAPI` in +// `apps/cli-go/cmd/root.go:91-109`). + +const tempRoot = useLegacyTempWorkdir("supabase-vanity-subdomains-experimental-int-"); + +const testRoot = Command.make("supabase").pipe( + Command.withGlobalFlags(LEGACY_GLOBAL_FLAGS), + Command.withSubcommands([legacyVanitySubdomainsCommand]), +); + +function setup() { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + response: { status: 200, body: { status: "not-used" } }, + }); + const runtime = buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + // `RuntimeInfo` is ambient (not provided by `legacyManagementApiRuntimeLayer` + // itself), so the real `legacyCredentialsLayer` built inline inside the + // command for the "gate open" case resolves ITS `RuntimeInfo` from this + // layer. Point homeDir at this test's isolated tempRoot so the layer's + // file-based token fallback (`/.supabase/access-token`) can't pick + // up a stray token left at the shared default `/tmp/supabase-cli-test-home`. + runtimeInfo: mockRuntimeInfo({ homeDir: tempRoot.current }), + }); + const layer = Layer.mergeAll( + runtime, + CliOutput.layer(textCliOutputFormatter()), + // The "gate open" case reaches the real `legacyManagementApiRuntimeLayer` + // (provided inline inside the command, not by this test's mocked runtime), + // which reads credentials/env directly — an ambient SUPABASE_ACCESS_TOKEN, + // SUPABASE_EXPERIMENTAL, or OS keyring entry on the machine running the + // test would make these assertions non-deterministic. Wipe process.env + // down to just this and disable the keyring fallback. + processEnvLayer({ SUPABASE_NO_KEYRING: "1" }), + Layer.succeed( + TelemetryRuntime, + TelemetryRuntime.of({ + configDir: `${tempRoot.current}/.supabase`, + tracesDir: `${tempRoot.current}/.supabase/traces`, + consent: "granted", + showDebug: false, + deviceId: "test-device-id", + sessionId: "test-session-id", + identity: makeTelemetryIdentity(undefined), + isFirstRun: false, + isTty: false, + isCi: false, + os: "linux", + arch: "x64", + cliVersion: "0.1.0", + }), + ), + ); + return { layer, api }; +} + +describe("legacy vanity-subdomains experimental gate (Go PersistentPreRunE parity)", () => { + // `check-availability` and `activate` deliberately OMIT `--desired-subdomain`: + // Go marks it required (`cmd/vanitySubdomains.go:67,69`) but cobra validates + // required flags only after `PersistentPreRunE` (`cobra@v1.10.2 + // command.go:985,1005`), so the gate error must win when both flags are + // missing. The TS flag is optional at parse time (enforced in the handler) + // precisely so this ordering holds — these cases assert it end to end. + const leaves: ReadonlyArray<{ readonly name: string; readonly args: ReadonlyArray }> = [ + { name: "get", args: ["vanity-subdomains", "get"] }, + { + name: "check-availability", + args: ["vanity-subdomains", "check-availability"], + }, + { + name: "activate", + args: ["vanity-subdomains", "activate"], + }, + { name: "delete", args: ["vanity-subdomains", "delete"] }, + ]; + + for (const { name, args } of leaves) { + it.live( + `${name} fails with LegacyExperimentalRequiredError when --experimental is unset`, + () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })(args), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("LegacyExperimentalRequiredError"); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live(`${name} does not fail with the gate error once --experimental is set`, () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })([...args, "--experimental"]), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + const causeText = JSON.stringify(exit.cause); + expect(causeText).not.toContain("LegacyExperimentalRequiredError"); + expect(causeText).toContain("LegacyPlatformAuthRequiredError"); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + } +}); diff --git a/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.integration.test.ts b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.integration.test.ts index 40c484acfe..62019cc4b9 100644 --- a/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.integration.test.ts +++ b/apps/cli/src/legacy/commands/vanity-subdomains/vanity-subdomains.integration.test.ts @@ -249,7 +249,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toBe("Subdomain example.com available: true\n"); expect(api.requests[0]?.method).toBe("POST"); @@ -268,7 +268,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain('"available": true'); }).pipe(Effect.provide(layer)); @@ -282,7 +282,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain("available: true"); }).pipe(Effect.provide(layer)); @@ -296,7 +296,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toBe("Available = true\n\n"); }).pipe(Effect.provide(layer)); @@ -310,7 +310,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain('AVAILABLE="true"'); }).pipe(Effect.provide(layer)); @@ -324,7 +324,7 @@ describe("legacy vanity-subdomains check-availability", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); const success = out.messages.find((m) => m.type === "success"); expect(success?.data).toMatchObject({ available: true }); @@ -341,7 +341,7 @@ describe("legacy vanity-subdomains check-availability", () => { const exit = yield* Effect.exit( legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }), ); expect(Exit.isFailure(exit)).toBe(true); @@ -360,7 +360,7 @@ describe("legacy vanity-subdomains check-availability", () => { const exit = yield* Effect.exit( legacyVanitySubdomainsCheckAvailability({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }), ); expect(Exit.isFailure(exit)).toBe(true); @@ -371,6 +371,44 @@ describe("legacy vanity-subdomains check-availability", () => { } }).pipe(Effect.provide(layer)); }); + + // Go marks --desired-subdomain required but cobra validates it only after + // PersistentPreRunE (gate → login → ref resolution), so the handler enforces + // it with cobra's exact wording after the ref resolves. + it.live("fails with cobra's required-flag error when --desired-subdomain is omitted", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ response: { status: 201, body: SAMPLE_CHECK } }); + const layer = runtimeWith({ out, api }); + + return Effect.gen(function* () { + const error = yield* Effect.flip( + legacyVanitySubdomainsCheckAvailability({ + projectRef: Option.none(), + desiredSubdomain: Option.none(), + }), + ); + expect(error._tag).toBe("LegacyDesiredSubdomainRequiredError"); + expect(error.message).toBe('required flag(s) "desired-subdomain" not set'); + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + // Cobra's MarkFlagRequired checks the flag was *changed*, not non-empty — + // an explicit empty value must pass the check and reach the API. + it.live("passes an explicit empty --desired-subdomain through to the API", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ response: { status: 201, body: SAMPLE_CHECK } }); + const layer = runtimeWith({ out, api }); + + return Effect.gen(function* () { + yield* legacyVanitySubdomainsCheckAvailability({ + projectRef: Option.none(), + desiredSubdomain: Option.some(""), + }); + expect(api.requests[0]?.body).toEqual({ vanity_subdomain: "" }); + expect(out.stdoutText).toBe("Subdomain available: true\n"); + }).pipe(Effect.provide(layer)); + }); }); describe("legacy vanity-subdomains activate", () => { @@ -382,7 +420,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toBe("Activated vanity subdomain at example.com\n"); expect(api.requests[0]?.method).toBe("POST"); @@ -401,7 +439,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain('"custom_domain": "example.com"'); }).pipe(Effect.provide(layer)); @@ -415,7 +453,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain("custom_domain: example.com"); }).pipe(Effect.provide(layer)); @@ -429,7 +467,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toBe('CustomDomain = "example.com"\n\n'); }).pipe(Effect.provide(layer)); @@ -443,7 +481,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); expect(out.stdoutText).toContain('CUSTOM_DOMAIN="example.com"'); }).pipe(Effect.provide(layer)); @@ -457,7 +495,7 @@ describe("legacy vanity-subdomains activate", () => { return Effect.gen(function* () { yield* legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }); const success = out.messages.find((m) => m.type === "success"); expect(success?.data).toMatchObject({ custom_domain: "example.com" }); @@ -474,7 +512,7 @@ describe("legacy vanity-subdomains activate", () => { const exit = yield* Effect.exit( legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }), ); expect(Exit.isFailure(exit)).toBe(true); @@ -499,7 +537,7 @@ describe("legacy vanity-subdomains activate", () => { const exit = yield* Effect.exit( legacyVanitySubdomainsActivate({ projectRef: Option.none(), - desiredSubdomain: "example.com", + desiredSubdomain: Option.some("example.com"), }), ); expect(Exit.isFailure(exit)).toBe(true); @@ -512,6 +550,43 @@ describe("legacy vanity-subdomains activate", () => { expect(analytics.captured).toHaveLength(0); }).pipe(Effect.provide(layer)); }); + + // Go marks --desired-subdomain required but cobra validates it only after + // PersistentPreRunE (gate → login → ref resolution), so the handler enforces + // it with cobra's exact wording after the ref resolves. + it.live("fails with cobra's required-flag error when --desired-subdomain is omitted", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ response: { status: 201, body: SAMPLE_ACTIVATE } }); + const layer = runtimeWith({ out, api }); + + return Effect.gen(function* () { + const error = yield* Effect.flip( + legacyVanitySubdomainsActivate({ + projectRef: Option.none(), + desiredSubdomain: Option.none(), + }), + ); + expect(error._tag).toBe("LegacyDesiredSubdomainRequiredError"); + expect(error.message).toBe('required flag(s) "desired-subdomain" not set'); + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + // Cobra's MarkFlagRequired checks the flag was *changed*, not non-empty — + // an explicit empty value must pass the check and reach the API. + it.live("passes an explicit empty --desired-subdomain through to the API", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ response: { status: 201, body: SAMPLE_ACTIVATE } }); + const layer = runtimeWith({ out, api }); + + return Effect.gen(function* () { + yield* legacyVanitySubdomainsActivate({ + projectRef: Option.none(), + desiredSubdomain: Option.some(""), + }); + expect(api.requests[0]?.body).toEqual({ vanity_subdomain: "" }); + }).pipe(Effect.provide(layer)); + }); }); describe("legacy vanity-subdomains delete", () => { @@ -625,4 +700,36 @@ describe("legacy vanity-subdomains PersistentPostRun parity", () => { expect(cache.cached).toBe(true); }).pipe(Effect.provide(layer)); }); + + // In Go the missing-required-flag failure happens AFTER PersistentPreRunE + // completes, so PersistentPostRun still fires telemetry and writes the + // linked-project cache (`cmd/root.go:171-181,212-233`). The handler-level + // check sits inside both `Effect.ensuring` wrappers to match. + it.live( + "flushes telemetry and writes linked-project cache on a missing --desired-subdomain", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ response: { status: 201, body: SAMPLE_ACTIVATE } }); + const telemetry = mockLegacyTelemetryStateTracked(); + const cache = mockLegacyLinkedProjectCacheTracked(); + const layer = runtimeWith({ + out, + api, + telemetry: telemetry.layer, + linkedProjectCache: cache.layer, + }); + + return Effect.gen(function* () { + const error = yield* Effect.flip( + legacyVanitySubdomainsActivate({ + projectRef: Option.none(), + desiredSubdomain: Option.none(), + }), + ); + expect(error._tag).toBe("LegacyDesiredSubdomainRequiredError"); + expect(telemetry.flushed).toBe(true); + expect(cache.cached).toBe(true); + }).pipe(Effect.provide(layer)); + }, + ); }); From ceb81817e754ae3c3eb6c30aa08dc3ca6ce90006 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 27 Jul 2026 20:54:46 +0100 Subject: [PATCH 03/24] fix(cli): suppress the --debug hint when a confirmation prompt is declined (#5946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What changed When a confirmation prompt is declined, the Go CLI prints only a red `context canceled` on stderr and exits 1 — `recoverAndExit` (`apps/cli-go/cmd/root.go:287-303`) explicitly skips the `SuggestDebugFlag` fallback for `errors.Is(err, context.Canceled)`. The TS text `Output.fail` renderer appended the fallback hint for every suggestion-less error, including cancellations, so every prompting command (`logout`, `db push` (3 prompts), `db reset`, `migration fetch`/`repair`/`down`, `functions deploy --prune`, `gen signing-key`, `secrets unset`, `projects delete`, `branches create`, `bootstrap`) printed: ``` context canceled Try rerunning the command with --debug to troubleshoot the error. ← spurious ``` This PR mirrors Go's guard at the render boundary: - New shared sentinel `CONTEXT_CANCELED_MESSAGE` in `apps/cli/src/shared/output/errors.ts` — the byte-for-byte render of Go's `context.Canceled`. - The text-layer `fail` (`apps/cli/src/shared/output/output.layer.ts`) skips the debug-hint fallback when the normalized message equals the sentinel. The explicit-suggestion branch stays first, matching Go printing a pre-set `utils.CmdSuggestion` even for canceled errors. - All 14 cancellation construction sites (12 files: the legacy decline paths above plus `shared/functions/deploy.ts`) now reference the constant instead of an inline literal, so the sentinel is single-sourced like Go's. - `LEGACY_LOGOUT_CANCELLED_MESSAGE` is folded into the shared constant, and the documented intent in `logout.errors.ts` (which already described the Go behavior this PR implements) now matches reality. ## Why the fix is shared rather than legacy-scoped The failure-rendering seam (`shared/cli/run.ts` `handledProgram` → `Output.fail`) is shell-agnostic, and `next/` genuinely shares a cancellation producer: `shared/functions/deploy.ts` (used by `next functions deploy --prune`) fails with the same `context canceled` message on a declined prune prompt. Suppressing a troubleshooting hint for a user-declined prompt is the correct behavior in both shells, so the fix lives in the shared layer instead of forking the renderer per shell. Aside from that, `next/` handlers treat declines gracefully (no `context canceled` error), so next-shell text rendering is otherwise unchanged. This is deliberate and reviewed against the CLI-1546 invariant (no spinner/stream routing was touched). ## Tests - Unit (`output.layer.unit.test.ts`): byte assertions that the sentinel message renders as exactly the red `context canceled` line with no hint (fails without the fix), and that an explicit caller suggestion still prints for a cancellation (Go ordering). - Declined-prompt stderr-byte e2e per affected family, each asserting exit code 1, last stderr line exactly `context canceled`, and no debug-hint line: `logout` (new), `migration fetch` (extended), `db reset` (new file — Docker-free, the destructive prompt fires before any connection is dialed), `gen signing-key` (extended). - The existing non-canceled guards stay: a generic error still gets the hint (unit + `run.e2e.test.ts` asserts stderr ends with the hint for an unrecognized flag). - `stripAnsi` hoisted to `tests/helpers/cli.ts` for the files this PR touches. ## Deliberately-open judgement calls - **Exact-match vs `errors.Is`:** the TS check is message equality, narrower than Go's chain-walking `errors.Is`. A future producer surfacing a *wrapped* cancellation (`"...: context canceled"`) through `Output.fail` would keep the hint where Go suppresses it. No such producer exists today (mid-flight Ctrl-C takes the interrupt/exit-130 path and never reaches `Output.fail`); the invariant is documented on the constant. Reviewers (architect/engineer/parity passes) agreed a tag-registry or normalized-error flag isn't warranted for a single Go sentinel. - **Pre-existing divergences noted during the parity audit, out of scope here:** (a) Ctrl-C *at* a prompt → TS clack `Operation cancelled.`/exit 130 vs Go `context canceled`/exit 1; (b) the hint's `--debug` detection uses `process.argv` vs Go's `viper.GetBool("DEBUG")` (which also honors `SUPABASE_DEBUG`) — affects only the non-canceled branch. Fixes CLI-1973 https://linear.app/supabase/issue/CLI-1973/declined-confirmation-prompts-print-a-spurious-debug-hint-context --- .../src/legacy/cli/agent-output.e2e.test.ts | 23 +------- .../commands/bootstrap/bootstrap.handler.ts | 5 +- .../branches/create/create.handler.ts | 3 +- .../commands/db/diff/diff.integration.test.ts | 5 +- .../commands/db/pull/pull.debug.unit.test.ts | 5 +- .../commands/db/pull/pull.integration.test.ts | 3 +- .../legacy/commands/db/push/push.handler.ts | 7 ++- .../commands/db/reset/reset.e2e.test.ts | 44 ++++++++++++++ .../legacy/commands/db/reset/reset.handler.ts | 5 +- .../declarative/declarative.gate.unit.test.ts | 7 +-- .../gen/signing-key/signing-key.e2e.test.ts | 2 + .../gen/signing-key/signing-key.handler.ts | 3 +- .../legacy/commands/logout/logout.e2e.test.ts | 25 +++++++- .../legacy/commands/logout/logout.errors.ts | 11 ++-- .../legacy/commands/logout/logout.handler.ts | 5 +- .../commands/migration/down/down.handler.ts | 3 +- .../migration/down/down.integration.test.ts | 3 +- .../migration/fetch/fetch.e2e.test.ts | 15 +++-- .../commands/migration/fetch/fetch.handler.ts | 3 +- .../migration/list/list.integration.test.ts | 4 +- .../commands/migration/new/new.e2e.test.ts | 9 +-- .../migration/new/new.integration.test.ts | 5 +- .../migration/repair/repair.handler.ts | 3 +- .../repair/repair.integration.test.ts | 3 +- .../migration/up/up.integration.test.ts | 3 +- .../projects/delete/delete.handler.ts | 3 +- .../commands/secrets/unset/unset.handler.ts | 3 +- .../legacy/commands/start/start.e2e.test.ts | 8 +-- .../commands/start/start.exclude.unit.test.ts | 8 +-- .../commands/start/start.format.unit.test.ts | 8 +-- .../legacy-migration-history.unit.test.ts | 5 +- .../shared/legacy-status-pretty.unit.test.ts | 9 +-- apps/cli/src/shared/functions/deploy.ts | 5 +- apps/cli/src/shared/output/errors.ts | 26 +++++++++ apps/cli/src/shared/output/output.layer.ts | 13 ++++- .../shared/output/output.layer.unit.test.ts | 58 ++++++++++++++++++- apps/cli/tests/helpers/ansi.ts | 21 +++++++ apps/cli/tests/helpers/cli.ts | 2 + 38 files changed, 250 insertions(+), 123 deletions(-) create mode 100644 apps/cli/src/legacy/commands/db/reset/reset.e2e.test.ts create mode 100644 apps/cli/tests/helpers/ansi.ts diff --git a/apps/cli/src/legacy/cli/agent-output.e2e.test.ts b/apps/cli/src/legacy/cli/agent-output.e2e.test.ts index 15c124caa2..8cce87cbfe 100644 --- a/apps/cli/src/legacy/cli/agent-output.e2e.test.ts +++ b/apps/cli/src/legacy/cli/agent-output.e2e.test.ts @@ -1,26 +1,5 @@ import { describe, expect, test } from "vitest"; -import { runSupabase } from "../../../tests/helpers/cli.ts"; - -function stripAnsi(output: string): string { - let stripped = ""; - for (let i = 0; i < output.length; i++) { - const charCode = output.charCodeAt(i); - if (charCode !== 0x1b || output[i + 1] !== "[") { - stripped += output[i]; - continue; - } - - i += 2; - while (i < output.length) { - const code = output.charCodeAt(i); - if (code >= 0x40 && code <= 0x7e) { - break; - } - i++; - } - } - return stripped; -} +import { runSupabase, stripAnsi } from "../../../tests/helpers/cli.ts"; function parseJsonLines(output: string): Array { return stripAnsi(output) diff --git a/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts b/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts index 0ed9b20a8a..8541a8a147 100644 --- a/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts +++ b/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts @@ -6,6 +6,7 @@ import { LegacyCliConfig } from "../../config/legacy-cli-config.service.ts"; import { LegacyLinkedProjectCache } from "../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../telemetry/legacy-telemetry-state.service.ts"; import { LegacyWorkdirFlag, legacyResolveYes } from "../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../shared/output/errors.ts"; import { Output } from "../../../shared/output/output.service.ts"; import { LegacyGoProxy } from "../../../shared/legacy/go-proxy.service.ts"; import { RuntimeInfo } from "../../../shared/runtime/runtime-info.service.ts"; @@ -135,7 +136,9 @@ export const legacyBootstrap = Effect.fn("legacy.bootstrap")(function* ( { defaultValue: true }, ); if (!overwrite) { - return yield* new LegacyBootstrapOverwriteDeclinedError({ message: "context canceled" }); + return yield* new LegacyBootstrapOverwriteDeclinedError({ + message: CONTEXT_CANCELED_MESSAGE, + }); } } diff --git a/apps/cli/src/legacy/commands/branches/create/create.handler.ts b/apps/cli/src/legacy/commands/branches/create/create.handler.ts index 9cf9352b1f..dc013a2069 100644 --- a/apps/cli/src/legacy/commands/branches/create/create.handler.ts +++ b/apps/cli/src/legacy/commands/branches/create/create.handler.ts @@ -7,6 +7,7 @@ import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.ser import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { LegacyOutputFlag } from "../../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { detectGitBranch } from "../../../../shared/git/git-branch.ts"; import { @@ -68,7 +69,7 @@ export const legacyBranchesCreate = Effect.fn("legacy.branches.create")(function .promptConfirm(`Do you want to create a branch named ${gitBranch.value}?`) .pipe(Effect.orElseSucceed(() => true)); if (!confirmed) { - return yield* new LegacyBranchesCreateCancelledError({ message: "context canceled" }); + return yield* new LegacyBranchesCreateCancelledError({ message: CONTEXT_CANCELED_MESSAGE }); } branchName = gitBranch.value; if (gitBranchForBody === undefined) { diff --git a/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts b/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts index f4e6fb4ea6..f1a531c7a9 100644 --- a/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts +++ b/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts @@ -4,6 +4,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { legacyFailWriteStringOnNthCallFsLayer, mockLegacyCliConfig, @@ -230,10 +231,6 @@ const flags = (over: Partial = {}): LegacyDbDiffFlags => ({ schema: over.schema ?? [], }); -// Strip ANSI so assertions are colour-independent: `legacyAqua`/`legacyYellow` -// emit colour only when the test runner's stderr is a TTY. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); const stdout = (out: ReturnType) => stripAnsi( out.rawChunks diff --git a/apps/cli/src/legacy/commands/db/pull/pull.debug.unit.test.ts b/apps/cli/src/legacy/commands/db/pull/pull.debug.unit.test.ts index dc83ad49db..5c4c5a36f5 100644 --- a/apps/cli/src/legacy/commands/db/pull/pull.debug.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/pull/pull.debug.unit.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { legacyFormatByteSize, legacyFormatCatalogSummary, @@ -9,10 +10,6 @@ import { legacySummarizeCatalogJson, } from "./pull.debug.ts"; -// ANSI may wrap the bold debugDir; strip for assertions. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("legacyRedactPostgresURL", () => { it("replaces the password but keeps the username", () => { expect(legacyRedactPostgresURL("postgresql://postgres:secret@db.host:5432/postgres")).toBe( diff --git a/apps/cli/src/legacy/commands/db/pull/pull.integration.test.ts b/apps/cli/src/legacy/commands/db/pull/pull.integration.test.ts index 62b8188e8e..c028a42a79 100644 --- a/apps/cli/src/legacy/commands/db/pull/pull.integration.test.ts +++ b/apps/cli/src/legacy/commands/db/pull/pull.integration.test.ts @@ -4,6 +4,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { legacyFailWriteStringOnNthCallFsLayer, mockLegacyCliConfig, @@ -304,8 +305,6 @@ const flags = (over: Partial = {}): LegacyDbPullFlags => ({ password: over.password ?? Option.none(), }); -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); const streamText = (out: ReturnType, stream: "stdout" | "stderr") => stripAnsi( out.rawChunks diff --git a/apps/cli/src/legacy/commands/db/push/push.handler.ts b/apps/cli/src/legacy/commands/db/push/push.handler.ts index af75b4cfd3..e5f7d1a43c 100644 --- a/apps/cli/src/legacy/commands/db/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/db/push/push.handler.ts @@ -3,6 +3,7 @@ import { Clock, Effect, FileSystem, Option, Path } from "effect"; import { CliArgs } from "../../../../shared/cli/cli-args.service.ts"; import { LegacyDnsResolverFlag } from "../../../../shared/legacy/global-flags.ts"; import { legacyResolveYesWithProjectEnv } from "../../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -270,7 +271,7 @@ export const legacyDbPush = Effect.fn("legacy.db.push")(function* (flags: Legacy ); if (!ok) { return yield* Effect.fail( - new LegacyDbPushCancelledError({ message: "context canceled" }), + new LegacyDbPushCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } yield* legacySeedGlobals( @@ -292,7 +293,7 @@ export const legacyDbPush = Effect.fn("legacy.db.push")(function* (flags: Legacy ); if (!ok) { return yield* Effect.fail( - new LegacyDbPushCancelledError({ message: "context canceled" }), + new LegacyDbPushCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } yield* legacyUpsertVaultSecrets(session, vaultSecrets); @@ -335,7 +336,7 @@ export const legacyDbPush = Effect.fn("legacy.db.push")(function* (flags: Legacy ); if (!ok) { return yield* Effect.fail( - new LegacyDbPushCancelledError({ message: "context canceled" }), + new LegacyDbPushCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } yield* legacySeedData(session, fs, workdir, path, seeds, applyError); diff --git a/apps/cli/src/legacy/commands/db/reset/reset.e2e.test.ts b/apps/cli/src/legacy/commands/db/reset/reset.e2e.test.ts new file mode 100644 index 0000000000..7de2270883 --- /dev/null +++ b/apps/cli/src/legacy/commands/db/reset/reset.e2e.test.ts @@ -0,0 +1,44 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +import { runSupabase, stripAnsi } from "../../../../../tests/helpers/cli.ts"; + +const E2E_TIMEOUT_MS = 30_000; + +describe("supabase db reset (legacy)", () => { + let workdir: string; + beforeEach(() => { + workdir = mkdtempSync(join(tmpdir(), "sb-db-reset-e2e-")); + mkdirSync(join(workdir, "supabase"), { recursive: true }); + writeFileSync(join(workdir, "supabase", "config.toml"), "[db]\nport = 54322\n"); + }); + afterEach(() => { + rmSync(workdir, { recursive: true, force: true }); + }); + + // Docker-free: the destructive remote-reset confirmation fires after the config + // load and BEFORE any connection is dialed, so a piped decline exits without a + // database. Declining must byte-match Go: a single `context canceled` line on + // stderr and exit 1, with NO `--debug` troubleshooting hint — `recoverAndExit` + // skips `SuggestDebugFlag` for `context.Canceled` (apps/cli-go/cmd/root.go:287-303). + // CLI-1973. + test( + "declining the remote reset prompt prints only context canceled, no --debug hint", + { timeout: E2E_TIMEOUT_MS }, + async () => { + const { exitCode, stderr } = await runSupabase( + ["db", "reset", "--db-url", "postgresql://postgres:postgres@127.0.0.1:9999/postgres"], + { entrypoint: "legacy", cwd: workdir, stdin: "n\n" }, + ); + expect(exitCode).toBe(1); + // The destructive confirmation (default No → `[y/N]`) actually rendered and + // was answered — the cancellation didn't come from some other failure path. + expect(stripAnsi(stderr)).toContain("[y/N]"); + const lines = stripAnsi(stderr).trimEnd().split("\n"); + expect(lines.at(-1)).toBe("context canceled"); + expect(stderr).not.toContain("Try rerunning the command with --debug"); + }, + ); +}); diff --git a/apps/cli/src/legacy/commands/db/reset/reset.handler.ts b/apps/cli/src/legacy/commands/db/reset/reset.handler.ts index 4b5fc92782..c2ef78bab3 100644 --- a/apps/cli/src/legacy/commands/db/reset/reset.handler.ts +++ b/apps/cli/src/legacy/commands/db/reset/reset.handler.ts @@ -8,6 +8,7 @@ import { legacyResolveYesWithProjectEnv, } from "../../../../shared/legacy/global-flags.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { legacyAqua, legacyYellow } from "../../../shared/legacy-colors.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; @@ -415,7 +416,9 @@ export const legacyDbReset = Effect.fn("legacy.db.reset")(function* (flags: Lega false, ); if (!shouldReset) { - return yield* Effect.fail(new LegacyDbResetCancelledError({ message: "context canceled" })); + return yield* Effect.fail( + new LegacyDbResetCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), + ); } yield* output.raw(`Resetting remote database${toLogMessage(resolvedVersion)}\n`, "stderr"); diff --git a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.gate.unit.test.ts b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.gate.unit.test.ts index f605be37fe..030c0890ea 100644 --- a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.gate.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.gate.unit.test.ts @@ -1,6 +1,7 @@ import { Cause, Effect, Exit } from "effect"; import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../../../../tests/helpers/ansi.ts"; import { LegacyDeclarativeNotEnabledError } from "./declarative.errors.ts"; import { legacyIsPgDeltaEnabled, @@ -8,12 +9,6 @@ import { legacyRequirePgDelta, } from "./declarative.gate.ts"; -// `legacyAqua`/`legacyBold` colour their tokens when stderr is a TTY (matching -// Go's lipgloss). Strip ANSI so the assertions validate text content exactly, -// independent of the runner's colour profile. -const stripAnsi = (text: string) => - text.replace(new RegExp(`${String.fromCharCode(27)}\\[[0-9;]*m`, "g"), ""); - const EXPECTED_SUGGESTION = "Either pass --experimental or add [experimental.pgdelta] with enabled = true to supabase/config.toml"; diff --git a/apps/cli/src/legacy/commands/gen/signing-key/signing-key.e2e.test.ts b/apps/cli/src/legacy/commands/gen/signing-key/signing-key.e2e.test.ts index 3a2b377ee4..af5c6b3474 100644 --- a/apps/cli/src/legacy/commands/gen/signing-key/signing-key.e2e.test.ts +++ b/apps/cli/src/legacy/commands/gen/signing-key/signing-key.e2e.test.ts @@ -43,6 +43,8 @@ describe("supabase gen signing-key (legacy)", () => { }); expect(exitCode).toBe(1); expect(stderr).toContain("context canceled"); + // No SuggestDebugFlag fallback for context.Canceled (cmd/root.go:287-303, CLI-1973). + expect(stderr).not.toContain("Try rerunning the command with --debug"); expect(stderr).not.toContain("Service not found"); const saved = readFileSync(join(projectDir, "supabase", "signing_keys.json"), "utf8"); expect(JSON.parse(saved)).toEqual([]); diff --git a/apps/cli/src/legacy/commands/gen/signing-key/signing-key.handler.ts b/apps/cli/src/legacy/commands/gen/signing-key/signing-key.handler.ts index 87a734da73..9dcef82d60 100644 --- a/apps/cli/src/legacy/commands/gen/signing-key/signing-key.handler.ts +++ b/apps/cli/src/legacy/commands/gen/signing-key/signing-key.handler.ts @@ -11,6 +11,7 @@ import { LegacyDebugLogger } from "../../../shared/legacy-debug-logger.service.t import { legacyPromptYesNo } from "../../../shared/legacy-prompt-yes-no.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyResolveYesWithProjectEnv } from "../../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { Tty } from "../../../../shared/runtime/tty.service.ts"; import type { LegacyGenSigningKeyFlags } from "./signing-key.command.ts"; @@ -316,7 +317,7 @@ export const legacyGenSigningKey = Effect.fn("legacy.gen.signing-key")(function* ); if (!confirmed) { return yield* Effect.fail( - new LegacyGenSigningKeyCancelledError({ message: "context canceled" }), + new LegacyGenSigningKeyCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } return [key]; diff --git a/apps/cli/src/legacy/commands/logout/logout.e2e.test.ts b/apps/cli/src/legacy/commands/logout/logout.e2e.test.ts index 5fb3d31ab2..d039a3d25b 100644 --- a/apps/cli/src/legacy/commands/logout/logout.e2e.test.ts +++ b/apps/cli/src/legacy/commands/logout/logout.e2e.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { describe, expect, test } from "vitest"; -import { makeTempHome, runSupabase } from "../../../../tests/helpers/cli.ts"; +import { makeTempHome, runSupabase, stripAnsi } from "../../../../tests/helpers/cli.ts"; const E2E_TIMEOUT_MS = 30_000; const VALID_TOKEN = "sbp_" + "a".repeat(40); @@ -37,6 +37,29 @@ describe("supabase logout (legacy)", () => { }, ); + // Declining the confirmation must byte-match Go: a single `context canceled` + // line on stderr and exit 1, with NO `--debug` troubleshooting hint — + // `recoverAndExit` skips `SuggestDebugFlag` for `context.Canceled` + // (apps/cli-go/cmd/root.go:287-303). CLI-1973. + test( + "declining the logout prompt prints only context canceled, no --debug hint", + { timeout: E2E_TIMEOUT_MS }, + async () => { + using home = makeTempHome(); + seedTokenFile(home.dir); + const { exitCode, stderr } = await runSupabase(["logout"], { + entrypoint: "legacy", + home: home.dir, + env: { HOME: home.dir }, + stdin: "n\n", + }); + expect(exitCode).toBe(1); + const lines = stripAnsi(stderr).trimEnd().split("\n"); + expect(lines.at(-1)).toBe("context canceled"); + expect(stderr).not.toContain("Try rerunning the command with --debug"); + }, + ); + // No token at all: same not-logged-in message, exit 0. test( "logout --yes with no token reports not-logged-in and exits 0", diff --git a/apps/cli/src/legacy/commands/logout/logout.errors.ts b/apps/cli/src/legacy/commands/logout/logout.errors.ts index 1f3dd768f6..a7ced0612c 100644 --- a/apps/cli/src/legacy/commands/logout/logout.errors.ts +++ b/apps/cli/src/legacy/commands/logout/logout.errors.ts @@ -2,13 +2,14 @@ import { Data } from "effect"; /** * Raised when the user declines the logout confirmation prompt. Go returns - * `errors.New(context.Canceled)` (`apps/cli-go/internal/logout/logout.go:18`), + * `errors.New(context.Canceled)` (`apps/cli-go/internal/logout/logout.go:19`), * which the root error handler renders as `context canceled` on stderr with - * exit code 1 (`cmd/root.go:288-301` skips the debug suggestion for - * `context.Canceled`). + * exit code 1 and no `--debug` suggestion (`cmd/root.go:287-303` skips + * `SuggestDebugFlag` for `context.Canceled`). The TS renderer mirrors that: + * constructing this error with `CONTEXT_CANCELED_MESSAGE` + * (`shared/output/errors.ts`) is what makes the text `Output.fail` withhold + * the debug hint (CLI-1973). */ export class LegacyLogoutCancelledError extends Data.TaggedError("LegacyLogoutCancelledError")<{ readonly message: string; }> {} - -export const LEGACY_LOGOUT_CANCELLED_MESSAGE = "context canceled"; diff --git a/apps/cli/src/legacy/commands/logout/logout.handler.ts b/apps/cli/src/legacy/commands/logout/logout.handler.ts index 8b4a606249..a89bd0fad1 100644 --- a/apps/cli/src/legacy/commands/logout/logout.handler.ts +++ b/apps/cli/src/legacy/commands/logout/logout.handler.ts @@ -3,9 +3,10 @@ import { Effect } from "effect"; import { LegacyCredentials } from "../../auth/legacy-credentials.service.ts"; import { LegacyTelemetryState } from "../../telemetry/legacy-telemetry-state.service.ts"; import { legacyResolveYes } from "../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../shared/output/errors.ts"; import { Output } from "../../../shared/output/output.service.ts"; import { legacyPromptYesNo } from "../../shared/legacy-prompt-yes-no.ts"; -import { LegacyLogoutCancelledError, LEGACY_LOGOUT_CANCELLED_MESSAGE } from "./logout.errors.ts"; +import { LegacyLogoutCancelledError } from "./logout.errors.ts"; const LOGGED_OUT_MSG = "Access token deleted successfully. You are now logged out."; @@ -37,7 +38,7 @@ export const legacyLogout = Effect.fn("legacy.logout")(function* () { }); if (!confirmed) { return yield* Effect.fail( - new LegacyLogoutCancelledError({ message: LEGACY_LOGOUT_CANCELLED_MESSAGE }), + new LegacyLogoutCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } diff --git a/apps/cli/src/legacy/commands/migration/down/down.handler.ts b/apps/cli/src/legacy/commands/migration/down/down.handler.ts index d646db1782..69c7fc46a0 100644 --- a/apps/cli/src/legacy/commands/migration/down/down.handler.ts +++ b/apps/cli/src/legacy/commands/migration/down/down.handler.ts @@ -5,6 +5,7 @@ import { legacyResolveYesWithProjectEnv, } from "../../../../shared/legacy/global-flags.ts"; import { CliArgs } from "../../../../shared/cli/cli-args.service.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -138,7 +139,7 @@ const runDown = Effect.fnUntraced(function* ( ); if (!confirmed) { return yield* Effect.fail( - new LegacyOperationCanceledError({ message: "context canceled" }), + new LegacyOperationCanceledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } diff --git a/apps/cli/src/legacy/commands/migration/down/down.integration.test.ts b/apps/cli/src/legacy/commands/migration/down/down.integration.test.ts index 57c15c999e..b39174673c 100644 --- a/apps/cli/src/legacy/commands/migration/down/down.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/down/down.integration.test.ts @@ -5,6 +5,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { LEGACY_VALID_REF, mockLegacyCliConfig, @@ -151,8 +152,6 @@ const flags = (over: Partial = {}): LegacyMigrationDow local: over.local ?? true, }); -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); const seed = (workdir: string, name: string, body = "create table a;\n") => { const dir = join(workdir, "supabase", "migrations"); mkdirSync(dir, { recursive: true }); diff --git a/apps/cli/src/legacy/commands/migration/fetch/fetch.e2e.test.ts b/apps/cli/src/legacy/commands/migration/fetch/fetch.e2e.test.ts index ec8c28b274..48ac38e76b 100644 --- a/apps/cli/src/legacy/commands/migration/fetch/fetch.e2e.test.ts +++ b/apps/cli/src/legacy/commands/migration/fetch/fetch.e2e.test.ts @@ -3,13 +3,10 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { runSupabase } from "../../../../../tests/helpers/cli.ts"; +import { runSupabase, stripAnsi } from "../../../../../tests/helpers/cli.ts"; const E2E_TIMEOUT_MS = 30_000; -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("supabase migration fetch (legacy)", () => { let workdir: string; beforeEach(() => { @@ -40,9 +37,15 @@ describe("supabase migration fetch (legacy)", () => { stdin: "n\n", }); - // Declined → cancelled (non-zero), and the Go-style prompt label reached stderr. - expect(exitCode).not.toBe(0); + // Declined → cancelled (exit 1), and the Go-style prompt label reached stderr. + expect(exitCode).toBe(1); expect(stripAnsi(stderr)).toContain("[Y/n]"); + // Byte-parity with Go's `recoverAndExit` (apps/cli-go/cmd/root.go:287-303): + // a declined prompt renders a lone `context canceled` line, with NO + // `SuggestDebugFlag` troubleshooting hint appended. CLI-1973. + const lines = stripAnsi(stderr).trimEnd().split("\n"); + expect(lines.at(-1)).toBe("context canceled"); + expect(stderr).not.toContain("Try rerunning the command with --debug"); // The existing file was NOT overwritten — the piped answer was honored. expect(readdirSync(join(workdir, "supabase", "migrations"))).toEqual([ "20240101000000_existing.sql", diff --git a/apps/cli/src/legacy/commands/migration/fetch/fetch.handler.ts b/apps/cli/src/legacy/commands/migration/fetch/fetch.handler.ts index 6fadfea2b4..e5bc6f29c2 100644 --- a/apps/cli/src/legacy/commands/migration/fetch/fetch.handler.ts +++ b/apps/cli/src/legacy/commands/migration/fetch/fetch.handler.ts @@ -5,6 +5,7 @@ import { legacyResolveYesWithProjectEnv, } from "../../../../shared/legacy/global-flags.ts"; import { CliArgs } from "../../../../shared/cli/cli-args.service.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -114,7 +115,7 @@ const runFetch = Effect.fnUntraced(function* ( const overwrite = yield* legacyMigrationConfirm(title, { defaultValue: true, yes }); if (!overwrite) { return yield* Effect.fail( - new LegacyOperationCanceledError({ message: "context canceled" }), + new LegacyOperationCanceledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } } diff --git a/apps/cli/src/legacy/commands/migration/list/list.integration.test.ts b/apps/cli/src/legacy/commands/migration/list/list.integration.test.ts index 549dfad5e4..fb9a9c3907 100644 --- a/apps/cli/src/legacy/commands/migration/list/list.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/list/list.integration.test.ts @@ -4,6 +4,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { LEGACY_VALID_REF, mockLegacyCliConfig, @@ -108,9 +109,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { }; } -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - const flags = (over: Partial = {}): LegacyMigrationListFlags => ({ dbUrl: over.dbUrl ?? Option.none(), linked: over.linked ?? true, diff --git a/apps/cli/src/legacy/commands/migration/new/new.e2e.test.ts b/apps/cli/src/legacy/commands/migration/new/new.e2e.test.ts index 1c2f012fbd..08270e5b37 100644 --- a/apps/cli/src/legacy/commands/migration/new/new.e2e.test.ts +++ b/apps/cli/src/legacy/commands/migration/new/new.e2e.test.ts @@ -3,17 +3,10 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { runSupabase } from "../../../../../tests/helpers/cli.ts"; +import { runSupabase, stripAnsi } from "../../../../../tests/helpers/cli.ts"; const E2E_TIMEOUT_MS = 30_000; -// Strip ANSI so the assertion is colour-independent: the handler prints the path -// via `legacyBold`, which emits bold escapes under CI's `FORCE_COLOR` even on a -// piped stdout. The text content is the parity contract, not the colour. Mirrors -// `new.integration.test.ts`. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("supabase migration new (legacy)", () => { let workdir: string; beforeEach(() => { diff --git a/apps/cli/src/legacy/commands/migration/new/new.integration.test.ts b/apps/cli/src/legacy/commands/migration/new/new.integration.test.ts index caa3c051d4..2d8c4b02cb 100644 --- a/apps/cli/src/legacy/commands/migration/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/new/new.integration.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "@effect/vitest"; import { Cause, Effect, Exit, Layer, Option, Stream } from "effect"; import { badArgument } from "effect/PlatformError"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { mockLegacyCliConfig, mockLegacyTelemetryStateTracked, @@ -35,10 +36,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { return { layer, out, telemetry }; } -// Strip ANSI so assertions are colour-independent (`legacyBold` emits colour on a TTY). -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - const tmp = useLegacyTempWorkdir(); const migrationsDir = (workdir: string) => join(workdir, "supabase", "migrations"); diff --git a/apps/cli/src/legacy/commands/migration/repair/repair.handler.ts b/apps/cli/src/legacy/commands/migration/repair/repair.handler.ts index ea6727d68d..3f6d5578a5 100644 --- a/apps/cli/src/legacy/commands/migration/repair/repair.handler.ts +++ b/apps/cli/src/legacy/commands/migration/repair/repair.handler.ts @@ -5,6 +5,7 @@ import { legacyResolveYesWithProjectEnv, } from "../../../../shared/legacy/global-flags.ts"; import { CliArgs } from "../../../../shared/cli/cli-args.service.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -200,7 +201,7 @@ const runRepair = Effect.fnUntraced(function* ( ); if (!confirmed) { return yield* Effect.fail( - new LegacyOperationCanceledError({ message: "context canceled" }), + new LegacyOperationCanceledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } versions = yield* legacyLoadLocalVersions(fs, path, migrationsDir); diff --git a/apps/cli/src/legacy/commands/migration/repair/repair.integration.test.ts b/apps/cli/src/legacy/commands/migration/repair/repair.integration.test.ts index b6737c053b..a7d335cddd 100644 --- a/apps/cli/src/legacy/commands/migration/repair/repair.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/repair/repair.integration.test.ts @@ -4,6 +4,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { LEGACY_VALID_REF, mockLegacyCliConfig, @@ -133,8 +134,6 @@ const input = (over: Partial = {}): LegacyMigrationR password: over.password ?? Option.none(), }); -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); const seedMigration = (workdir: string, name: string, body: string) => { const dir = join(workdir, "supabase", "migrations"); mkdirSync(dir, { recursive: true }); diff --git a/apps/cli/src/legacy/commands/migration/up/up.integration.test.ts b/apps/cli/src/legacy/commands/migration/up/up.integration.test.ts index 8d7be64dcc..abab7670fd 100644 --- a/apps/cli/src/legacy/commands/migration/up/up.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/up/up.integration.test.ts @@ -4,6 +4,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { stripAnsi } from "../../../../../tests/helpers/ansi.ts"; import { LEGACY_VALID_REF, mockLegacyCliConfig, @@ -126,8 +127,6 @@ const flags = (over: Partial = {}): LegacyMigrationUpFla local: over.local ?? true, }); -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); const seed = (workdir: string, name: string, body = "create table a;\n") => { const dir = join(workdir, "supabase", "migrations"); mkdirSync(dir, { recursive: true }); diff --git a/apps/cli/src/legacy/commands/projects/delete/delete.handler.ts b/apps/cli/src/legacy/commands/projects/delete/delete.handler.ts index a3b7855b72..5dd1d9f1a9 100644 --- a/apps/cli/src/legacy/commands/projects/delete/delete.handler.ts +++ b/apps/cli/src/legacy/commands/projects/delete/delete.handler.ts @@ -13,6 +13,7 @@ import { import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { LegacyYesFlag } from "../../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { Tty } from "../../../../shared/runtime/tty.service.ts"; import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts"; @@ -81,7 +82,7 @@ export const legacyProjectsDelete = Effect.fn("legacy.projects.delete")(function confirmed = yield* output.promptConfirm(title).pipe(Effect.orElseSucceed(() => false)); } if (!confirmed) { - return yield* new LegacyProjectsDeleteCancelledError({ message: "context canceled" }); + return yield* new LegacyProjectsDeleteCancelledError({ message: CONTEXT_CANCELED_MESSAGE }); } const mapDeleteError = mapLegacyHttpError({ diff --git a/apps/cli/src/legacy/commands/secrets/unset/unset.handler.ts b/apps/cli/src/legacy/commands/secrets/unset/unset.handler.ts index 726c8e7b88..f12126477c 100644 --- a/apps/cli/src/legacy/commands/secrets/unset/unset.handler.ts +++ b/apps/cli/src/legacy/commands/secrets/unset/unset.handler.ts @@ -6,6 +6,7 @@ import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.ser import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyResolveYes } from "../../../../shared/legacy/global-flags.ts"; +import { CONTEXT_CANCELED_MESSAGE } from "../../../../shared/output/errors.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { Tty } from "../../../../shared/runtime/tty.service.ts"; import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts"; @@ -87,7 +88,7 @@ export const legacySecretsUnset = Effect.fn("legacy.secrets.unset")(function* ( if (!confirmed) { return yield* Effect.fail( - new LegacySecretsUnsetCancelledError({ message: "context canceled" }), + new LegacySecretsUnsetCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), ); } diff --git a/apps/cli/src/legacy/commands/start/start.e2e.test.ts b/apps/cli/src/legacy/commands/start/start.e2e.test.ts index 6b9932f1e7..9f6bb9eb44 100644 --- a/apps/cli/src/legacy/commands/start/start.e2e.test.ts +++ b/apps/cli/src/legacy/commands/start/start.e2e.test.ts @@ -3,16 +3,10 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { runSupabase } from "../../../../tests/helpers/cli.ts"; +import { runSupabase, stripAnsi } from "../../../../tests/helpers/cli.ts"; const E2E_TIMEOUT_MS = 30_000; -// Strip ANSI so the assertion is colour-independent — `legacyPartitionStartExcludeFlags` -// styles the warning via `legacy-colors.ts`, matching `start.exclude.unit.test.ts`'s -// own convention for the same text. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("supabase start (legacy)", () => { let projectDir: string; diff --git a/apps/cli/src/legacy/commands/start/start.exclude.unit.test.ts b/apps/cli/src/legacy/commands/start/start.exclude.unit.test.ts index 6964f40140..93f65230d8 100644 --- a/apps/cli/src/legacy/commands/start/start.exclude.unit.test.ts +++ b/apps/cli/src/legacy/commands/start/start.exclude.unit.test.ts @@ -1,14 +1,8 @@ import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { LEGACY_START_EXCLUDABLE_KEYS, legacyPartitionStartExcludeFlags } from "./start.exclude.ts"; -// The warning applies Go-parity ANSI styling via `legacy-colors.ts`, which -// no-ops on a real non-TTY stream but the vitest process presents its stderr -// as color-capable. Strip escapes so these assertions target the plain text -// content — matching `status.pretty.unit.test.ts`'s convention. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - // Go's `ExcludableContainers()` order (`apps/cli-go/internal/start/start.go: // 1297-1303` walking `config.Images.Services()`, // `apps/cli-go/pkg/config/constants.go:60-76`), expressed as the exact diff --git a/apps/cli/src/legacy/commands/start/start.format.unit.test.ts b/apps/cli/src/legacy/commands/start/start.format.unit.test.ts index 3f104d7a11..e2fd2310d6 100644 --- a/apps/cli/src/legacy/commands/start/start.format.unit.test.ts +++ b/apps/cli/src/legacy/commands/start/start.format.unit.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { LEGACY_START_STARTING_CONTAINERS_MESSAGE, LEGACY_START_STARTING_DATABASE_FROM_BACKUP_MESSAGE, @@ -10,13 +11,6 @@ import { legacyStartSecurityNotice, } from "./start.format.ts"; -// The formatters apply Go-parity ANSI styling via `legacy-colors.ts`, which -// no-ops on a real non-TTY stream but the vitest process presents its stderr -// as color-capable. Strip escapes so these assertions target the plain text -// content — matching `status.pretty.unit.test.ts`'s convention. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("legacyStartAlreadyRunningMessage", () => { it("matches Go's exact stderr line, with a single trailing newline", () => { expect(stripAnsi(legacyStartAlreadyRunningMessage())).toBe( diff --git a/apps/cli/src/legacy/shared/legacy-migration-history.unit.test.ts b/apps/cli/src/legacy/shared/legacy-migration-history.unit.test.ts index a7d2d9e132..446fe29b15 100644 --- a/apps/cli/src/legacy/shared/legacy-migration-history.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-migration-history.unit.test.ts @@ -1,6 +1,7 @@ import { Effect, Exit } from "effect"; import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../tests/helpers/ansi.ts"; import { LegacyDbExecError } from "./legacy-db-connection.errors.ts"; import type { LegacyDbSession } from "./legacy-db-connection.service.ts"; import { @@ -22,10 +23,6 @@ const failingSession = (error: LegacyDbExecError): LegacyDbSession => ({ queryRaw: () => Effect.die("unused"), }); -// Strip ANSI so the bold repair suggestions compare regardless of TTY colour. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - describe("legacyReconcileMigrations", () => { it("reports in-sync when remote and local match", () => { expect(legacyReconcileMigrations(["20240101000000"], ["20240101000000"])).toEqual({ diff --git a/apps/cli/src/legacy/shared/legacy-status-pretty.unit.test.ts b/apps/cli/src/legacy/shared/legacy-status-pretty.unit.test.ts index 51337df7e5..9c970819ed 100644 --- a/apps/cli/src/legacy/shared/legacy-status-pretty.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-status-pretty.unit.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import { stripAnsi } from "../../../tests/helpers/ansi.ts"; import { legacyRenderStatusPretty, legacyStatusColumnLayout, @@ -8,14 +9,6 @@ import { } from "./legacy-status-pretty.ts"; import type { LegacyStatusOutputNames } from "./legacy-status-values.ts"; -// The renderer applies Go-parity ANSI styling via `legacy-colors.ts`, which -// no-ops on a real non-TTY stream but the vitest process presents its stderr -// as color-capable. Strip escapes so these assertions target the plain -// structural output — the golden contract per the port plan — not whichever -// TTY heuristic the test runner happens to report. -// eslint-disable-next-line no-control-regex -const stripAnsi = (text: string) => text.replace(/\x1b\[[0-9;]*m/gu, ""); - // Default (un-overridden) output names, matching `legacy-status-values.ts`'s // `resolveOutputNames` with an empty override map — the KEYs the pretty // renderer looks values up by. diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index e707cb76f2..ef8be07080 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -11,6 +11,7 @@ import { import { Duration, Effect, Option, Schema, Stream } from "effect"; import { ChildProcessSpawner } from "effect/unstable/process"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; +import { CONTEXT_CANCELED_MESSAGE } from "../output/errors.ts"; import { Output } from "../output/output.service.ts"; import { spawnContainerCli } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; @@ -2084,7 +2085,9 @@ const pruneFunctions = Effect.fnUntraced(function* ( ].join("\n"); const confirmed = yes || (yield* output.promptConfirm(`${prompt}\n`, { defaultValue: false })); if (!confirmed) { - return yield* Effect.fail(new FunctionDeployCancelledError({ message: "context canceled" })); + return yield* Effect.fail( + new FunctionDeployCancelledError({ message: CONTEXT_CANCELED_MESSAGE }), + ); } for (const slug of toDelete) { diff --git a/apps/cli/src/shared/output/errors.ts b/apps/cli/src/shared/output/errors.ts index 02e93c8143..39bc34be27 100644 --- a/apps/cli/src/shared/output/errors.ts +++ b/apps/cli/src/shared/output/errors.ts @@ -1,5 +1,31 @@ import { Data } from "effect"; +/** + * Byte-for-byte render of Go's `context.Canceled` sentinel. + * + * Every declined confirmation prompt in the Go CLI surfaces as a bare + * `context.Canceled` (e.g. `errors.New(context.Canceled)` in + * `apps/cli-go/internal/logout/logout.go:19`), and `recoverAndExit` + * (`apps/cli-go/cmd/root.go:287-303`) deliberately skips the + * `SuggestDebugFlag` hint for it — declining a prompt is a user decision, + * not an error worth troubleshooting. Handlers that port those decline + * paths construct their cancellation errors with this exact message, and + * the text `Output.fail` renderer keys on it to suppress the `--debug` + * hint, mirroring Go's `!errors.Is(err, context.Canceled)` guard. + * + * Two invariants of that renderer check: + * - The value must stay trim-invariant: it round-trips through + * `normalizeCliError`'s trimming `readString` before reaching the + * renderer's equality check (`shared/output/normalize-error.ts`). + * - The check is exact-match, narrower than Go's chain-walking + * `errors.Is`: a future producer surfacing a WRAPPED cancellation + * (`"...: context canceled"`) through `Output.fail` would keep the hint + * where Go suppresses it — no such producer exists today (mid-flight + * Ctrl-C takes the interrupt/exit-130 path and never reaches + * `Output.fail`), but widen the check if one ever appears. + */ +export const CONTEXT_CANCELED_MESSAGE = "context canceled"; + export class NonInteractiveError extends Data.TaggedError("NonInteractiveError")<{ readonly detail: string; readonly suggestion: string; diff --git a/apps/cli/src/shared/output/output.layer.ts b/apps/cli/src/shared/output/output.layer.ts index 352e8190b5..fe8ff28710 100644 --- a/apps/cli/src/shared/output/output.layer.ts +++ b/apps/cli/src/shared/output/output.layer.ts @@ -17,7 +17,7 @@ import { styleText } from "node:util"; import { Effect, Layer, Stdio, Stream } from "effect"; import { Tty } from "../runtime/tty.service.ts"; -import { NonInteractiveError } from "./errors.ts"; +import { CONTEXT_CANCELED_MESSAGE, NonInteractiveError } from "./errors.ts"; import { Output } from "./output.service.ts"; import type { OutputFormat, StreamEvent } from "./types.ts"; @@ -346,8 +346,15 @@ export const textOutputLayer = Layer.effect( } if (err.suggestion !== undefined) { process.stderr.write(err.suggestion + "\n"); - } else if (!process.argv.includes("--debug")) { - // Go's `utils.SuggestDebugFlag` (apps/cli-go/internal/utils/misc.go:41). + } else if ( + err.message !== CONTEXT_CANCELED_MESSAGE && + !process.argv.includes("--debug") + ) { + // Go's `utils.SuggestDebugFlag` (apps/cli-go/internal/utils/misc.go:41), + // withheld for the canceled sentinel exactly like `recoverAndExit`'s + // `!errors.Is(err, context.Canceled)` guard (apps/cli-go/cmd/root.go:287-292): + // a declined confirmation prompt is a user decision, so Go prints only + // the red `context canceled` line with no troubleshooting hint (CLI-1973). process.stderr.write( "Try rerunning the command with --debug to troubleshoot the error.\n", ); diff --git a/apps/cli/src/shared/output/output.layer.unit.test.ts b/apps/cli/src/shared/output/output.layer.unit.test.ts index 0a6d7f0497..e3f8346f97 100644 --- a/apps/cli/src/shared/output/output.layer.unit.test.ts +++ b/apps/cli/src/shared/output/output.layer.unit.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import { afterEach, beforeEach, vi } from "vitest"; import { Cause, Effect, Exit, Layer, Sink, Stdio, Stream } from "effect"; -import { NonInteractiveError } from "./errors.ts"; +import { CONTEXT_CANCELED_MESSAGE, NonInteractiveError } from "./errors.ts"; import { mockTty } from "../../../tests/helpers/mocks.ts"; import { Output } from "./output.service.ts"; import { @@ -250,6 +250,62 @@ describe("Output", () => { ); }); + it.effect("fail withholds the --debug fallback for a declined-prompt cancellation", () => { + const writes: string[] = []; + const originalWrite = process.stderr.write.bind(process.stderr); + const originalArgv = process.argv; + process.stderr.write = ((chunk: string | Uint8Array) => { + writes.push(typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk)); + return true; + }) as typeof process.stderr.write; + // Strip --debug from argv so only the canceled-sentinel check can suppress the hint. + process.argv = originalArgv.filter((arg) => arg !== "--debug"); + return Effect.gen(function* () { + const out = yield* Output; + // Same shape `normalizeCause` produces for any declined confirmation prompt + // (logout, migration fetch/repair/down, db push/reset, functions deploy + // --prune, ...): Go's `recoverAndExit` prints only the red `context canceled` + // line for `context.Canceled` (apps/cli-go/cmd/root.go:287-303) — CLI-1973. + yield* out.fail({ code: "LegacyLogoutCancelledError", message: CONTEXT_CANCELED_MESSAGE }); + expect(writes).toEqual(["\x1B[31mcontext canceled\x1B[39m\n"]); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.sync(() => { + process.stderr.write = originalWrite; + process.argv = originalArgv; + }), + ), + ); + }); + + it.effect("fail still prints an explicit caller suggestion for a cancellation", () => { + const writes: string[] = []; + const originalWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = ((chunk: string | Uint8Array) => { + writes.push(typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk)); + return true; + }) as typeof process.stderr.write; + return Effect.gen(function* () { + const out = yield* Output; + // Go prints a pre-set `utils.CmdSuggestion` even for `context.Canceled` — + // only the `SuggestDebugFlag` fallback is withheld (cmd/root.go:287-292). + yield* out.fail({ + code: "E_TEST", + message: CONTEXT_CANCELED_MESSAGE, + suggestion: "custom hint", + }); + expect(writes).toEqual(["\x1B[31mcontext canceled\x1B[39m\n", "custom hint\n"]); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.sync(() => { + process.stderr.write = originalWrite; + }), + ), + ); + }); + it.effect("promptText passes validate callback to clack", () => { mockClack.text.mockImplementation( (opts: { validate?: (v: string | undefined) => string | undefined }) => { diff --git a/apps/cli/tests/helpers/ansi.ts b/apps/cli/tests/helpers/ansi.ts new file mode 100644 index 0000000000..689a67887f --- /dev/null +++ b/apps/cli/tests/helpers/ansi.ts @@ -0,0 +1,21 @@ +/** Strip ANSI CSI escape sequences (colors, styles, cursor codes) from captured CLI output. */ +export function stripAnsi(output: string): string { + let stripped = ""; + for (let i = 0; i < output.length; i++) { + const charCode = output.charCodeAt(i); + if (charCode !== 0x1b || output[i + 1] !== "[") { + stripped += output[i]; + continue; + } + + i += 2; + while (i < output.length) { + const code = output.charCodeAt(i); + if (code >= 0x40 && code <= 0x7e) { + break; + } + i++; + } + } + return stripped; +} diff --git a/apps/cli/tests/helpers/cli.ts b/apps/cli/tests/helpers/cli.ts index c232c776a9..9d0bec1214 100644 --- a/apps/cli/tests/helpers/cli.ts +++ b/apps/cli/tests/helpers/cli.ts @@ -13,6 +13,8 @@ import { registerTempStackProject, } from "./stack-e2e-cleanup.ts"; +export { stripAnsi } from "./ansi.ts"; + const BINARY_EXT = process.platform === "win32" ? ".exe" : ""; const SHIM_PATH = fileURLToPath(new URL("../../dist/supabase.js", import.meta.url)); const LEGACY_BINARY_PATH = fileURLToPath( From 3d62a687fff044b1258580d51a3afc0cade515e8 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 27 Jul 2026 20:54:56 +0100 Subject: [PATCH 04/24] fix(cli): surface db connection-failure detail and fix connect suggestion classifiers (#5948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What changed Every legacy db command's connection failure previously rendered as ``` failed to connect to postgres: effect/sql/SqlError: PgClient: Failed to connect ``` — host, user, database, and the underlying driver cause were all lost, on every `db push` / `db pull` / `db reset` / `db diff --linked` / `inspect` / `migration` connection failure. The Go CLI renders pgconn's full detail: ``` failed to connect to postgres: failed to connect to `host=… user=… database=…`: dial error (dial tcp …: connect: connection refused) ``` This PR ports pgconn's `connectError` rendering (`errors.go:66-72`, wrapped by `pkg/pgxv5/connect.go:33`) into the legacy connection layer, and fixes the `legacyConnectSuggestion` classifier branches that could never fire on real node-postgres error shapes. ### Message rendering (`legacyConnectFailureMessage`) - `toConnectError` in `legacy-db-connection.sql-pg.layer.ts` now renders `failed to connect to postgres: failed to connect to \`host=… user=… database=…\`: ` using the config-level identity (host/user/database — never the password), exactly like pgconn. - The cause is unwrapped through the real `SqlError → ConnectionError → driver error` chain (and through `AggregateError.errors[]`, taking the LAST attempt — pgconn's fallback loop also surfaces the last attempt's error). - Stage labels mirror pgconn where the node error identifies the stage unambiguously: `server error (SEVERITY: message (SQLSTATE code))` (byte-parity with pgconn's `PgError` rendering), `hostname resolving error (…)`, `dial error (…)`, `tls error (…)`. ### Classifier fixes (`legacyConnectSuggestion`) Branch-by-branch verification against real driver shapes (captured empirically under Bun, the CLI's runtime): | Go branch | node-postgres shape | Before | After | |---|---|---|---| | `connect: connection refused` / allow_list → network-restrictions hint | `ECONNREFUSED` code / server text | fired | fires (unchanged, now also covered by real-`SqlError` tests) | | `SSL connection is required` + `--debug` | server error text | fired | fires (unchanged) | | `SCRAM exchange: Wrong password` / `failed SASL auth` → `SUPABASE_DB_PASSWORD` hint | `DatabaseError` 28P01 `password authentication failed` | fired | fires (unchanged, proven against a real wire-protocol ErrorResponse) | | IPv6-only host → IPv6 pooler hint | `EHOSTUNREACH`/`EADDRNOTAVAIL`/`ENETUNREACH` with an IPv6 `address` field | **dead** for EHOSTUNREACH/EADDRNOTAVAIL (node puts the address in a structured field, not libpq's parenthesized literal) | fires via new structured errno+address check (`legacyHasIPv6DialCause`) | | `connect: no route to host` → wrong-profile hint | `connect EHOSTUNREACH :` | **dead** (node never emits "no route to host") | fires via `EHOSTUNREACH`, after the IPv6 branch, matching Go's branch order | | `Tenant or user not found` → wrong-profile hint | Supavisor server error text | fired | fires (unchanged) | `NODE_ENETUNREACH_PATTERN` now also tolerates a closing paren after the port, since the new message format parenthesizes the driver cause. ### CLI-1942 guard (no accidental divergence) Go has **no** suggestion branch for the session-pooler EOF drop (`unexpected EOF`); node-postgres' equivalent is `Connection terminated unexpectedly`. Tests pin that this shape stays unclassified (generic `--debug` fallback) and that the message still carries the full `host=… user=…` identity plus the cause verbatim. ## Residual driver-text differences (impossible to byte-match) The inner cause text comes from the driver, so exact byte parity with Go is impossible where the drivers word things differently — the **structure** (`failed to connect to postgres:` prefix + `host=… user=… database=…` + cause) and the stage labels match: - Dial errors: node `connect ECONNREFUSED 1.2.3.4:5432` vs Go `dial tcp 1.2.3.4:5432: connect: connection refused`. - Wrong password over SCRAM: pgconn labels by auth phase (`failed SASL auth (…)`), which node-postgres does not expose, so TS renders `server error (…)` — the inner `FATAL: password authentication failed for user "postgres" (SQLSTATE 28P01)` bytes and the fired hint are identical. - Unrecognized causes (e.g. the CLI-1942 EOF, connect timeout) render verbatim with no stage word, where pgconn would say `failed to receive message (unexpected EOF)` — no stage is guessed rather than fabricating a wrong one. - Server-side failures (`server error (SEVERITY: message (SQLSTATE code))`) are byte-identical to Go given the same server bytes. ## Review findings deliberately left open Five-perspective review (architect / engineer / security / DX / go-parity-auditor) all approved. Non-blocking items noted for follow-up rather than churned here: - Consolidating the module's four error-chain traversals (collector / deepest-path / two any-match predicates) into one walk primitive — they have genuinely different semantics and predate this PR; cross-referencing doc comments were added instead. - Pre-existing, negligible divergence: an IPv4 `ENETUNREACH` gets no suggestion in TS while Go's over-broad `network is unreachable` text match would show the IPv6 hint even for IPv4 — the TS IPv6-literal gate is deliberate and pre-existing. - Pre-existing: `password authentication failed` as a wrong-password disjunct would also classify a theoretical non-SCRAM 28P01 that Go leaves unclassified; Supabase auth is always SCRAM. Related: this PR does not touch `output.layer.ts` (PR #5946 / CLI-1973 territory) — the change is confined to the SqlError mapping and the classifier module. Fixes [CLI-1976](https://linear.app/supabase/issue/CLI-1976/db-connection-failure-errors-lose-all-detail-connect-suggestions-may) --- .../legacy/shared/legacy-connect-errors.ts | 282 +++++++++++++- .../shared/legacy-connect-errors.unit.test.ts | 358 ++++++++++++++++++ ...y-db-connection.sql-pg.integration.test.ts | 184 +++++++++ .../legacy-db-connection.sql-pg.layer.ts | 9 +- 4 files changed, 816 insertions(+), 17 deletions(-) create mode 100644 apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.integration.test.ts diff --git a/apps/cli/src/legacy/shared/legacy-connect-errors.ts b/apps/cli/src/legacy/shared/legacy-connect-errors.ts index 5a269abf87..fdc0a6069b 100644 --- a/apps/cli/src/legacy/shared/legacy-connect-errors.ts +++ b/apps/cli/src/legacy/shared/legacy-connect-errors.ts @@ -1,8 +1,10 @@ /** - * Connection-error classification ported from Go's `internal/utils/connect.go`. + * Connection-error classification and rendering ported from Go's + * `internal/utils/connect.go` and pgconn's `connectError` (`errors.go:60-77`). * Used by the container-level pooler fallback (`db dump --linked`) to decide * whether a failed pg_dump/pg container was an IPv6 connectivity failure that - * warrants retrying through the IPv4 transaction pooler. + * warrants retrying through the IPv4 transaction pooler, and by the connection + * layer to render connect failures with Go's `host=… user=… database=…` detail. */ import { isIPv6 } from "node:net"; @@ -29,7 +31,10 @@ export function legacyIpv6Suggestion(): string { // Go's `ipv6LiteralPattern` (`connect.go:181`): an IPv6 address in brackets // (Go dial form) or parens (libpq form). Run against the original-case message. const IPV6_LITERAL_PATTERN = /(?:\[[0-9a-fA-F:]+\]|\([0-9a-fA-F:]+\))/; -const NODE_ENETUNREACH_PATTERN = /\benetunreach\s+([0-9a-fA-F:]+):\d+(?:\s|$)/i; +// Node's dial-failure shape (`connect ENETUNREACH 2600:…:5432`). The port may be +// followed by whitespace, end-of-string, or a closing paren — the connect-failure +// message renders the driver cause parenthesized (pgconn `dial error (…)` form). +const NODE_ENETUNREACH_PATTERN = /\benetunreach\s+([0-9a-fA-F:]+):\d+(?:[\s)]|$)/i; /** * Port of Go's `isIPv6ConnectivityError` (`connect.go:189-208`). Lower-cases the @@ -70,13 +75,20 @@ export interface LegacyConnectSuggestionContext { } /** - * Flatten an error's `cause` chain and any `AggregateError.errors` into a single - * searchable string of every nested `message` and `code`. The `@effect/sql` - * `SqlError` wraps the node-postgres / node `net` driver error on its `cause`; a - * multi-address dial wraps an `AggregateError` whose `errors[]` carry the per-IP - * `ECONNREFUSED` / `ENETUNREACH` system errors. Including the `code` strings lets - * the matcher key off node's `ECONNREFUSED` the way Go keys off pgconn's - * `connect: connection refused`. + * Flatten an error's `cause` chain into a single searchable string of every + * nested `message` and `code`. The `@effect/sql` `SqlError` wraps the + * node-postgres / node `net` driver error on its `cause`; a multi-address dial + * wraps an `AggregateError` whose `errors[]` carry the per-IP `ECONNREFUSED` / + * `ENETUNREACH` system errors — an aggregate node contributes NOTHING itself + * and only its LAST child is visited, because that is the attempt pgconn + * surfaces: its fallback loop overwrites the error on every attempt so only the + * last one survives (`pgconn.go:171-203`), and Go's `SetConnectSuggestion` + * classifies exactly that `err.Error()` string (`connect.go:317`). The parent's + * own fields must be skipped: node's `aggregateErrors` copies `errors[0].code` + * onto the aggregate itself (`lib/internal/errors.js`, Bun matches), so reading + * them would blame an abandoned first attempt. Including the `code` strings of + * the visited nodes lets the matcher key off node's `ECONNREFUSED` the way Go + * keys off pgconn's `connect: connection refused`. */ function legacyCollectConnectErrorText(error: unknown): string { const parts: string[] = []; @@ -84,18 +96,236 @@ function legacyCollectConnectErrorText(error: unknown): string { const visit = (node: unknown, depth: number): void => { if (depth > 8 || typeof node !== "object" || node === null || seen.has(node)) return; seen.add(node); + const errors = Reflect.get(node, "errors"); + if (Array.isArray(errors) && errors.length > 0) { + visit(errors[errors.length - 1], depth + 1); + return; + } const message = Reflect.get(node, "message"); if (typeof message === "string") parts.push(message); const code = Reflect.get(node, "code"); if (typeof code === "string") parts.push(code); visit(Reflect.get(node, "cause"), depth + 1); - const errors = Reflect.get(node, "errors"); - if (Array.isArray(errors)) for (const child of errors) visit(child, depth + 1); }; visit(error, 0); return parts.join("\n"); } +/** + * The connection identity pgconn embeds in its `connectError` text + * (`errors.go:66-68`): the config-level (primary) host, user, and database — + * never the password. + */ +export interface LegacyConnectFailureTarget { + readonly host: string; + readonly user: string; + readonly database: string; +} + +/** + * Walk to the deepest underlying driver error: unwrap `cause` chains (the + * `@effect/sql` `SqlError` exposes its `ConnectionError` reason as `cause`, and + * the reason exposes the node-postgres error the same way) and descend into the + * LAST entry of an `AggregateError`'s `errors[]` — pgconn's multi-address + * fallback loop likewise surfaces the last attempt's error + * (`pgconn.go:159-192`). + */ +function legacyDeepestConnectCause(error: unknown): unknown { + let current: unknown = error; + const seen = new Set(); + for (let depth = 0; depth < 8; depth++) { + if (typeof current !== "object" || current === null || seen.has(current)) break; + seen.add(current); + const errors = Reflect.get(current, "errors"); + if (Array.isArray(errors) && errors.length > 0) { + current = errors[errors.length - 1]; + continue; + } + const cause = Reflect.get(current, "cause"); + if (typeof cause !== "object" || cause === null) break; + current = cause; + } + return current; +} + +// Node/Bun errno codes raised while dialing the server — pgconn wraps the +// equivalent net.Dial failures as `dial error (…)` (`pgconn.go:276`). +const LEGACY_DIAL_ERROR_CODES = new Set([ + "ECONNREFUSED", + "ETIMEDOUT", + "ENETUNREACH", + "EHOSTUNREACH", + "EADDRNOTAVAIL", +]); +// The complete documented Node/OpenSSL X509 certificate-verification code +// family (Node tls docs "X509 certificate error codes", OpenSSL's +// `X509_verify_cert_error` set), complemented by node's ERR_TLS_*/ERR_SSL_* +// prefixes at the use site. pgconn stages by connection PHASE — ANY `startTLS` +// failure becomes `tls error (…)` (`pgconn.go:283-289`) — but node exposes no +// phase marker, so the full code family is the proxy. These strings are unique +// to TLS-layer verification: server SQLSTATEs and dial/DNS `E…` errnos are +// classified by earlier branches. +const LEGACY_TLS_ERROR_CODES = new Set([ + "UNABLE_TO_GET_ISSUER_CERT", + "UNABLE_TO_GET_CRL", + "UNABLE_TO_DECRYPT_CERT_SIGNATURE", + "UNABLE_TO_DECRYPT_CRL_SIGNATURE", + "UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY", + "CERT_SIGNATURE_FAILURE", + "CRL_SIGNATURE_FAILURE", + "CERT_NOT_YET_VALID", + "CERT_HAS_EXPIRED", + "CRL_NOT_YET_VALID", + "CRL_HAS_EXPIRED", + "ERROR_IN_CERT_NOT_BEFORE_FIELD", + "ERROR_IN_CERT_NOT_AFTER_FIELD", + "ERROR_IN_CRL_LAST_UPDATE_FIELD", + "ERROR_IN_CRL_NEXT_UPDATE_FIELD", + "OUT_OF_MEM", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "UNABLE_TO_GET_ISSUER_CERT_LOCALLY", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "CERT_CHAIN_TOO_LONG", + "CERT_REVOKED", + "INVALID_CA", + "PATH_LENGTH_EXCEEDED", + "INVALID_PURPOSE", + "CERT_UNTRUSTED", + "CERT_REJECTED", + "HOSTNAME_MISMATCH", +]); +// node-postgres' own message when the server answers `N` to SSLRequest +// (`pg/lib/connection.js`); pgconn: `tls error (server refused TLS connection)`. +const LEGACY_SERVER_REFUSED_SSL = "The server does not support SSL connections"; +// Node/Bun's TLS-socket message when the server accepts SSLRequest but closes +// the socket before the handshake completes (node `lib/_tls_wrap.js` +// `onConnectEnd`; Bun emits the same text for both FIN and RST). Phase-specific +// by construction — only ever raised pre-secure-connection — so it maps to +// pgconn's startTLS stage (`tls error (…)`, `pgconn.go:283-289`). Its code is +// ECONNRESET, deliberately absent from LEGACY_DIAL_ERROR_CODES: a raw +// post-handshake `read ECONNRESET` is not phase-specific and stays verbatim. +const LEGACY_TLS_DISCONNECT_MESSAGE = + "Client network socket disconnected before secure TLS connection was established"; +// A Postgres SQLSTATE is exactly five uppercase alphanumerics. Combined with the +// `severity` field this identifies a node-postgres `DatabaseError` (a server +// ErrorResponse), never a node system error (whose codes are longer `E…` names). +const SQLSTATE_PATTERN = /^[0-9A-Z]{5}$/; + +/** + * Render the underlying driver failure the way pgconn stages its + * `connectError.msg` (`server error` / `hostname resolving error` / + * `dial error` / `tls error`, each with the cause parenthesized — + * `errors.go:66-72`). A server ErrorResponse reproduces pgconn's `PgError` + * rendering byte-for-byte (`Severity: Message (SQLSTATE Code)`, `errors.go:51`); + * for the other stages the parenthesized text is the node driver's own message, + * which cannot byte-match libpq/pgconn wording (e.g. node's + * `connect ECONNREFUSED 1.2.3.4:5432` vs Go's + * `dial tcp 1.2.3.4:5432: connect: connection refused`). An unrecognized cause + * (e.g. node-postgres' `Connection terminated unexpectedly`, where pgconn would + * say `failed to receive message (unexpected EOF)`) is rendered verbatim rather + * than guessing a stage. + * + * Known stage-label caveat: pgconn labels by auth PHASE, which node-postgres + * does not expose — a wrong password over SCRAM arrives mid-SASL, so Go renders + * `failed SASL auth (FATAL: password authentication failed … (SQLSTATE 28P01))` + * (`pgconn.go:359`) where this renders `server error (…)` with the identical + * inner `PgError` bytes. The suggestion classifier keys off the inner text, so + * the `SUPABASE_DB_PASSWORD` hint fires identically either way. + */ +function legacyConnectCauseDetail(cause: unknown): string { + if (typeof cause !== "object" || cause === null) return String(cause); + const message = Reflect.get(cause, "message"); + const code = Reflect.get(cause, "code"); + const severity = Reflect.get(cause, "severity"); + const syscall = Reflect.get(cause, "syscall"); + const text = + typeof message === "string" && message.length > 0 + ? message + : typeof code === "string" + ? code + : String(cause); + if (typeof severity === "string" && typeof code === "string" && SQLSTATE_PATTERN.test(code)) { + return `server error (${severity}: ${text} (SQLSTATE ${code}))`; + } + if (syscall === "getaddrinfo" || code === "ENOTFOUND" || code === "EAI_AGAIN") { + return `hostname resolving error (${text})`; + } + if (syscall === "connect" || (typeof code === "string" && LEGACY_DIAL_ERROR_CODES.has(code))) { + return `dial error (${text})`; + } + if ( + text === LEGACY_SERVER_REFUSED_SSL || + text === LEGACY_TLS_DISCONNECT_MESSAGE || + (typeof code === "string" && + (LEGACY_TLS_ERROR_CODES.has(code) || + code.startsWith("ERR_TLS") || + code.startsWith("ERR_SSL"))) + ) { + return `tls error (${text})`; + } + return text; +} + +/** + * Port of pgconn's `connectError.Error()` (`errors.go:66-72`), the inner text of + * Go's `failed to connect to postgres: %w` wrap (`pkg/pgxv5/connect.go:33`): + * `` failed to connect to `host=… user=… database=…`: ``. + * Callers pass the connection config (pgconn embeds the config-level identity, + * `errors.go:66-68`) and the raw failure — either the `@effect/sql` `SqlError` + * from the pooled connect probe or the bare node-postgres error from the raw + * client, both unwrapped by {@link legacyDeepestConnectCause}. + */ +export function legacyConnectFailureMessage( + target: LegacyConnectFailureTarget, + error: unknown, +): string { + const detail = legacyConnectCauseDetail(legacyDeepestConnectCause(error)); + return `failed to connect to \`host=${target.host} user=${target.user} database=${target.database}\`: ${detail}`; +} + +// Dial errno codes that mean the target address itself is unreachable. Combined +// with an IPv6 `address` they are node's equivalents of Go's IPv6-connectivity +// texts: ENETUNREACH → `network is unreachable`, EHOSTUNREACH → `no route to +// host`, EADDRNOTAVAIL → `cannot assign requested address` (`connect.go:204-223`). +const LEGACY_IPV6_DIAL_CODES = new Set(["ENETUNREACH", "EHOSTUNREACH", "EADDRNOTAVAIL"]); + +/** + * Whether the error chain carries a node dial failure whose errno + `address` + * fields identify an unreachable IPv6 target. This is the structured complement + * to {@link legacyIsIPv6ConnectivityError}: node system errors carry the dialed + * address as a field (`connect EHOSTUNREACH 2600:…:5432` has `code` and + * `address`), whereas Go's classifier reads the same facts out of pgconn's + * message text. Narrower than `legacyIsIPv6ConnectivityErrorCause` — it never + * treats `ENOTFOUND` (a plain DNS miss, e.g. a typo'd host) as IPv6, matching + * Go, where a failed lookup renders `hostname resolving error` and sets no + * suggestion. Use THIS one for the connect suggestion; the container-level + * pooler fallback keeps the broader `legacyIsIPv6ConnectivityErrorCause`. + * Like {@link legacyDeepestConnectCause}, an `AggregateError` descends into its + * LAST child only — the attempt pgconn surfaces (`pgconn.go:171-203`) and the + * one Go's classifier reads (`connect.go:317`). Depth-bounded recursion (no + * `seen` set): a pathological cause cycle re-walks at most 8 levels, which is + * cheap and cannot loop. + */ +function legacyHasIPv6DialCause(error: unknown, depth = 0): boolean { + if (depth > 8 || typeof error !== "object" || error === null) return false; + const code = Reflect.get(error, "code"); + const address = Reflect.get(error, "address"); + if ( + typeof code === "string" && + LEGACY_IPV6_DIAL_CODES.has(code) && + typeof address === "string" && + isIPv6(address) + ) { + return true; + } + const errors = Reflect.get(error, "errors"); + if (Array.isArray(errors) && errors.length > 0) { + return legacyHasIPv6DialCause(errors[errors.length - 1], depth + 1); + } + return legacyHasIPv6DialCause(Reflect.get(error, "cause"), depth + 1); +} + /** * Port of Go's `SetConnectSuggestion` (`internal/utils/connect.go:313-335`): map a * Postgres connect failure to an actionable hint that replaces the generic @@ -105,6 +335,15 @@ function legacyCollectConnectErrorText(error: unknown): string { * the `SqlError` cause/aggregate chain. The branch order mirrors Go's `if/else if`. * Returns `undefined` when no specific suggestion applies (the caller then falls * back to the generic suggestion, like Go leaving `CmdSuggestion` empty). + * + * Sourcing note: the rendered message ({@link legacyConnectFailureMessage}) and + * this classifier inspect the SAME surfaced attempt, as Go guarantees by + * construction — pgconn's fallback loop overwrites its error on every attempt so + * only the last one survives (`pgconn.go:171-203`), and `SetConnectSuggestion` + * classifies exactly that rendered `err.Error()` string (`connect.go:317`). The + * collectors above therefore descend into only the LAST aggregate child, the + * same attempt {@link legacyDeepestConnectCause} surfaces for rendering, so the + * displayed cause and the suggestion can never disagree. */ export function legacyConnectSuggestion( error: unknown, @@ -132,11 +371,21 @@ export function legacyConnectSuggestion( ) { return LEGACY_SUGGEST_ENV_VAR; } - if (legacyIsIPv6ConnectivityError(text)) { + // Go: `isIPv6ConnectivityError` on the pgconn text. node system errors carry + // the dialed address as a structured field instead of libpq's parenthesized + // literal, so also consult the errno + `address` classifier. + if (legacyIsIPv6ConnectivityError(text) || legacyHasIPv6DialCause(error)) { return legacyIpv6Suggestion(); } - // no route to host / Tenant or user not found → wrong profile. - if (text.includes("no route to host") || text.includes("Tenant or user not found")) { + // connect: no route to host / Tenant or user not found → wrong profile. + // node's "no route to host" is `connect EHOSTUNREACH :`; an IPv6 + // EHOSTUNREACH was already captured by the IPv6 branch above, mirroring Go's + // branch order ("Assumes IPv6 check has been performed before this"). + if ( + text.includes("no route to host") || + text.includes("EHOSTUNREACH") || + text.includes("Tenant or user not found") + ) { return `Make sure your project exists on profile: ${ctx.profileName}`; } return undefined; @@ -155,6 +404,9 @@ function hasStringCode(error: unknown): error is { * Classifies Node socket/getaddrinfo causes that carry errno-style `code` fields. * `ENOTFOUND` is intentionally broader than Go's text classifier (it can include * typo'd hosts); callers must combine this with a direct `db.` host gate. + * Used by the container-level pooler fallback (`gen types` / `db dump`); the + * connect-suggestion path uses the narrower `legacyHasIPv6DialCause` instead, + * which must not treat a DNS miss as IPv6. */ export function legacyIsIPv6ConnectivityErrorCause(error: unknown): boolean { if (error instanceof AggregateError) { diff --git a/apps/cli/src/legacy/shared/legacy-connect-errors.unit.test.ts b/apps/cli/src/legacy/shared/legacy-connect-errors.unit.test.ts index 844cda45e0..41867f4df8 100644 --- a/apps/cli/src/legacy/shared/legacy-connect-errors.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-connect-errors.unit.test.ts @@ -1,13 +1,47 @@ +import { ConnectionError, SqlError } from "effect/unstable/sql/SqlError"; +import * as Pg from "pg"; import { describe, expect, it } from "vitest"; import { LEGACY_SUGGEST_ENV_VAR, + legacyConnectFailureMessage, legacyConnectSuggestion, legacyIpv6Suggestion, legacyIsIPv6ConnectivityError, legacyIsIPv6ConnectivityErrorCause, } from "./legacy-connect-errors.ts"; +// The real `@effect/sql` wrapper produced by the connect probe +// (`legacyAcquireProbedPool`): a `SqlError` whose `ConnectionError` reason carries +// the node-postgres driver error as its `cause`. +const realSqlConnectError = (cause: unknown) => + new SqlError({ + reason: new ConnectionError({ + cause, + message: "PgClient: Failed to connect", + operation: "connect", + }), + }); + +// A node/Bun system error exactly as the `net` stack raises it while dialing: +// `connect ECONNREFUSED 127.0.0.1:5432` with errno-style fields attached. +const dialError = (code: string, address: string, port: number) => + Object.assign(new Error(`connect ${code} ${address}:${port}`), { + code, + errno: -61, + syscall: "connect", + address, + port, + }); + +// A real node-postgres server ErrorResponse (`DatabaseError` from pg-protocol), +// with the fields a live Postgres attaches for a failed password authentication. +const authFailedError = () => + Object.assign( + new Pg.DatabaseError('password authentication failed for user "postgres"', 104, "error"), + { severity: "FATAL", code: "28P01", file: "auth.c", line: "326", routine: "auth_failed" }, + ); + describe("legacyIsIPv6ConnectivityError", () => { it("classifies the getaddrinfo IPv6-only failures (case-insensitive)", () => { expect( @@ -41,12 +75,203 @@ describe("legacyIsIPv6ConnectivityError", () => { expect(legacyIsIPv6ConnectivityError("connect ENETUNREACH 10.0.0.1:5432")).toBe(false); }); + it("classifies Node ENETUNREACH inside the parenthesized connect-failure rendering", () => { + expect( + legacyIsIPv6ConnectivityError( + "failed to connect to `host=db.x.supabase.co user=postgres database=postgres`: dial error (connect ENETUNREACH 2600:1f18::1:5432)", + ), + ).toBe(true); + }); + it("does not classify unrelated errors", () => { expect(legacyIsIPv6ConnectivityError("permission denied for schema public")).toBe(false); expect(legacyIsIPv6ConnectivityError("")).toBe(false); }); }); +describe("legacyConnectFailureMessage", () => { + const target = { host: "db.abcdefghij.supabase.co", user: "postgres", database: "postgres" }; + const prefix = + "failed to connect to `host=db.abcdefghij.supabase.co user=postgres database=postgres`:"; + + it("renders host/user/database and the staged dial cause through the real SqlError chain", () => { + const error = realSqlConnectError(dialError("ECONNREFUSED", "127.0.0.1", 5432)); + expect(legacyConnectFailureMessage(target, error)).toBe( + `${prefix} dial error (connect ECONNREFUSED 127.0.0.1:5432)`, + ); + }); + + it("surfaces the last dial attempt of a dual-stack AggregateError (pgconn last-fallback parity)", () => { + // node dials ::1 then 127.0.0.1 for `localhost` and aggregates both failures + // into an AggregateError with an EMPTY message; pgconn's fallback loop + // likewise surfaces the last attempt's error. + const aggregate = Object.assign(new AggregateError([], ""), { + code: "ECONNREFUSED", + errors: [ + dialError("ECONNREFUSED", "::1", 5432), + dialError("ECONNREFUSED", "127.0.0.1", 5432), + ], + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(aggregate))).toBe( + `${prefix} dial error (connect ECONNREFUSED 127.0.0.1:5432)`, + ); + }); + + it("reproduces pgconn's server-error rendering byte-for-byte for a server ErrorResponse", () => { + expect(legacyConnectFailureMessage(target, realSqlConnectError(authFailedError()))).toBe( + `${prefix} server error (FATAL: password authentication failed for user "postgres" (SQLSTATE 28P01))`, + ); + }); + + it("stages a DNS failure as hostname resolving error (Bun getaddrinfo shape)", () => { + // Bun's getaddrinfo failure omits the hostname from the message; the host is + // already carried by the `host=…` identity. + const dns = Object.assign(new Error("getaddrinfo ENOTFOUND"), { + code: "ENOTFOUND", + syscall: "getaddrinfo", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(dns))).toBe( + `${prefix} hostname resolving error (getaddrinfo ENOTFOUND)`, + ); + // A transient resolver failure classifies by code alone (no syscall field). + const eaiAgain = Object.assign(new Error("getaddrinfo EAI_AGAIN db.x.supabase.co"), { + code: "EAI_AGAIN", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(eaiAgain))).toBe( + `${prefix} hostname resolving error (getaddrinfo EAI_AGAIN db.x.supabase.co)`, + ); + }); + + it("stages a dial errno by code alone when the syscall field is absent", () => { + const timedOut = Object.assign(new Error("connect ETIMEDOUT 10.0.0.9:5432"), { + code: "ETIMEDOUT", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(timedOut))).toBe( + `${prefix} dial error (connect ETIMEDOUT 10.0.0.9:5432)`, + ); + }); + + it("stages TLS failures as tls error", () => { + // node-postgres' own refusal message carries no code. + expect( + legacyConnectFailureMessage( + target, + realSqlConnectError(new Error("The server does not support SSL connections")), + ), + ).toBe(`${prefix} tls error (The server does not support SSL connections)`); + const selfSigned = Object.assign(new Error("self-signed certificate in certificate chain"), { + code: "SELF_SIGNED_CERT_IN_CHAIN", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(selfSigned))).toBe( + `${prefix} tls error (self-signed certificate in certificate chain)`, + ); + // node's ERR_TLS_* family (e.g. a hostname mismatch under verify-full). + const altname = Object.assign(new Error("Hostname/IP does not match certificate's altnames"), { + code: "ERR_TLS_CERT_ALTNAME_INVALID", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(altname))).toBe( + `${prefix} tls error (Hostname/IP does not match certificate's altnames)`, + ); + }); + + it("stages every documented X509 certificate-verification code as tls error", () => { + // pgconn stages by connection phase — ANY startTLS failure is `tls error (…)` + // (`pgconn.go:283-289`) — so the complete Node/OpenSSL verification family + // (Node tls docs "X509 certificate error codes") must keep the staged + // rendering under sslmode=verify-ca / verify-full. Pinned code-by-code so a + // future trim of the allowlist regresses loudly. + const x509Codes = [ + "UNABLE_TO_GET_ISSUER_CERT", + "UNABLE_TO_GET_CRL", + "UNABLE_TO_DECRYPT_CERT_SIGNATURE", + "UNABLE_TO_DECRYPT_CRL_SIGNATURE", + "UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY", + "CERT_SIGNATURE_FAILURE", + "CRL_SIGNATURE_FAILURE", + "CERT_NOT_YET_VALID", + "CERT_HAS_EXPIRED", + "CRL_NOT_YET_VALID", + "CRL_HAS_EXPIRED", + "ERROR_IN_CERT_NOT_BEFORE_FIELD", + "ERROR_IN_CERT_NOT_AFTER_FIELD", + "ERROR_IN_CRL_LAST_UPDATE_FIELD", + "ERROR_IN_CRL_NEXT_UPDATE_FIELD", + "OUT_OF_MEM", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "UNABLE_TO_GET_ISSUER_CERT_LOCALLY", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "CERT_CHAIN_TOO_LONG", + "CERT_REVOKED", + "INVALID_CA", + "PATH_LENGTH_EXCEEDED", + "INVALID_PURPOSE", + "CERT_UNTRUSTED", + "CERT_REJECTED", + "HOSTNAME_MISMATCH", + ] as const; + for (const code of x509Codes) { + const failure = Object.assign(new Error(`certificate verification failed: ${code}`), { + code, + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(failure))).toBe( + `${prefix} tls error (certificate verification failed: ${code})`, + ); + } + }); + + it("stages a mid-handshake TLS disconnect as tls error despite its ECONNRESET code", () => { + // Node/Bun's `_tls_wrap.js` onConnectEnd shape: the server accepted + // SSLRequest but closed the socket before the handshake completed. The + // message is phase-specific (only ever raised pre-secure-connection), so it + // stages like pgconn's startTLS wrap (`tls error (…)`, pgconn.go:283-289). + const midHandshake = Object.assign( + new Error("Client network socket disconnected before secure TLS connection was established"), + { code: "ECONNRESET" }, + ); + expect(legacyConnectFailureMessage(target, realSqlConnectError(midHandshake))).toBe( + `${prefix} tls error (Client network socket disconnected before secure TLS connection was established)`, + ); + }); + + it("renders a raw socket reset verbatim — not phase-specific, so no stage is guessed", () => { + // Node's hard-RST shape (`read ECONNRESET`, syscall "read") is identical + // before and after the handshake, so unlike the message above it must NOT + // be staged as tls error. + const rawReset = Object.assign(new Error("read ECONNRESET"), { + code: "ECONNRESET", + syscall: "read", + }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(rawReset))).toBe( + `${prefix} read ECONNRESET`, + ); + }); + + it("renders an unrecognized cause verbatim (CLI-1942 session-pooler EOF shape)", () => { + // node-postgres raises `Connection terminated unexpectedly` where pgconn + // says `failed to receive message (unexpected EOF)` — no stage is guessed. + const eof = new Error("Connection terminated unexpectedly"); + expect(legacyConnectFailureMessage(target, realSqlConnectError(eof))).toBe( + `${prefix} Connection terminated unexpectedly`, + ); + }); + + it("handles a bare driver error (raw-client path) and non-object failures", () => { + // `acquireRawClient` maps the node-postgres rejection without a SqlError wrapper. + expect(legacyConnectFailureMessage(target, dialError("ECONNREFUSED", "127.0.0.1", 6543))).toBe( + `${prefix} dial error (connect ECONNREFUSED 127.0.0.1:6543)`, + ); + expect(legacyConnectFailureMessage(target, "boom")).toBe(`${prefix} boom`); + }); + + it("falls back to the code when the cause carries an empty message", () => { + const bare = Object.assign(new Error(), { code: "ECONNREFUSED" }); + expect(legacyConnectFailureMessage(target, realSqlConnectError(bare))).toBe( + `${prefix} dial error (ECONNREFUSED)`, + ); + }); +}); + describe("legacyConnectSuggestion", () => { const ctx = { dashboardUrl: "https://supabase.com/dashboard", @@ -113,6 +338,139 @@ describe("legacyConnectSuggestion", () => { ); }); + it("maps node's no-route-to-host (EHOSTUNREACH over IPv4) to the wrong-profile hint", () => { + // Go matches pgconn's `connect: no route to host`; node renders the same + // failure as `connect EHOSTUNREACH :` with errno fields. + const err = realSqlConnectError(dialError("EHOSTUNREACH", "10.1.2.3", 5432)); + expect(legacyConnectSuggestion(err, ctx)).toBe( + "Make sure your project exists on profile: supabase", + ); + }); + + it("maps an IPv6 no-route-to-host (EHOSTUNREACH) to the IPv6 pooler suggestion", () => { + // Go's `no route to host` counts as IPv6 when the message carries an IPv6 + // literal; node carries the dialed address as a structured field instead. + const err = realSqlConnectError(dialError("EHOSTUNREACH", "2600:1f18::1", 5432)); + expect(legacyConnectSuggestion(err, ctx)).toBe(legacyIpv6Suggestion()); + }); + + it("maps an IPv6 cannot-assign-address (EADDRNOTAVAIL) to the IPv6 pooler suggestion", () => { + const err = realSqlConnectError(dialError("EADDRNOTAVAIL", "2a05:d014::1", 5432)); + expect(legacyConnectSuggestion(err, ctx)).toBe(legacyIpv6Suggestion()); + }); + + it("maps an aggregate of IPv6 dial failures to the IPv6 pooler suggestion", () => { + const aggregate = Object.assign(new AggregateError([], ""), { + errors: [dialError("EHOSTUNREACH", "2600:1f18::1", 5432)], + }); + expect(legacyConnectSuggestion(realSqlConnectError(aggregate), ctx)).toBe( + legacyIpv6Suggestion(), + ); + }); + + it("classifies only the LAST attempt of a mixed-family aggregate (pgconn last-fallback parity)", () => { + // Go can never blame an abandoned attempt: pgconn's fallback loop keeps only + // the last error (`pgconn.go:171-203`) and `SetConnectSuggestion` classifies + // that same rendered string (`connect.go:317`). An earlier IPv6 EHOSTUNREACH + // followed by a final unclassified IPv4 timeout must NOT fire the IPv6 hint. + // The parent carries `code` copied from errors[0] (node's `aggregateErrors`), + // which must not leak into the wrong-profile branch either. + const aggregate = Object.assign(new AggregateError([], ""), { + code: "EHOSTUNREACH", + errors: [ + dialError("EHOSTUNREACH", "2600:1f18::1", 5432), + dialError("ETIMEDOUT", "10.0.0.9", 5432), + ], + }); + expect(legacyConnectSuggestion(realSqlConnectError(aggregate), ctx)).toBeUndefined(); + }); + + it("fires the IPv6 pooler suggestion when the LAST aggregate attempt is the IPv6 dial failure", () => { + // The surfaced (last) attempt drives both the rendered cause and the + // suggestion — an earlier refused IPv4 attempt is ignored, like Go, even + // though node copies its `code` onto the aggregate parent. + const aggregate = Object.assign(new AggregateError([], ""), { + code: "ECONNREFUSED", + errors: [ + dialError("ECONNREFUSED", "10.0.0.9", 5432), + dialError("EHOSTUNREACH", "2600:1f18::1", 5432), + ], + }); + expect(legacyConnectSuggestion(realSqlConnectError(aggregate), ctx)).toBe( + legacyIpv6Suggestion(), + ); + }); + + it("ignores the parent aggregate's copied first-attempt code (node aggregateErrors shape)", () => { + // Node's `aggregateErrors` (`lib/internal/errors.js`, Bun matches) copies + // `errors[0].code` onto the AggregateError itself. A refused first attempt + // followed by a final unreachable-IPv6 attempt must classify the LAST + // attempt (IPv6 pooler hint), not the parent's copied ECONNREFUSED — + // otherwise the suggestion disagrees with the rendered cause, which Go + // makes impossible (`pgconn.go:171-203`, `connect.go:317`). + const aggregate = Object.assign(new AggregateError([], ""), { + code: "ECONNREFUSED", + errors: [ + dialError("ECONNREFUSED", "10.0.0.9", 5432), + dialError("ENETUNREACH", "2600:1f18::1", 5432), + ], + }); + expect(legacyConnectSuggestion(realSqlConnectError(aggregate), ctx)).toBe( + legacyIpv6Suggestion(), + ); + }); + + it("classifies a refused LAST attempt as network restrictions despite an IPv6 first attempt", () => { + // Reverse direction: the parent's copied ENETUNREACH (from the abandoned + // IPv6 first attempt) must not fabricate the IPv6 hint when the surfaced + // last attempt is a plain refusal. + const aggregate = Object.assign(new AggregateError([], ""), { + code: "ENETUNREACH", + errors: [ + dialError("ENETUNREACH", "2600:1f18::1", 5432), + dialError("ECONNREFUSED", "10.0.0.9", 5432), + ], + }); + expect(legacyConnectSuggestion(realSqlConnectError(aggregate), ctx)).toContain( + "Network Restrictions and Network Bans", + ); + }); + + it("sets no suggestion for a mid-handshake TLS disconnect, like Go", () => { + // Go's `SetConnectSuggestion` (`connect.go:313-335`) has no branch matching + // resets or TLS failures — the staged `tls error (…)` rendering must not + // change that. + const midHandshake = Object.assign( + new Error("Client network socket disconnected before secure TLS connection was established"), + { code: "ECONNRESET" }, + ); + expect(legacyConnectSuggestion(realSqlConnectError(midHandshake), ctx)).toBeUndefined(); + }); + + it("keeps an IPv4 EADDRNOTAVAIL unclassified, like Go without an IPv6 literal", () => { + const err = realSqlConnectError(dialError("EADDRNOTAVAIL", "10.1.2.3", 5432)); + expect(legacyConnectSuggestion(err, ctx)).toBeUndefined(); + }); + + it("fires the refused hint through the real SqlError chain, not just the test double", () => { + const err = realSqlConnectError(dialError("ECONNREFUSED", "127.0.0.1", 54322)); + expect(legacyConnectSuggestion(err, ctx)).toContain("Network Restrictions and Network Bans"); + }); + + it("fires the env-var hint for a real node-postgres 28P01 DatabaseError", () => { + expect(legacyConnectSuggestion(realSqlConnectError(authFailedError()), ctx)).toBe( + LEGACY_SUGGEST_ENV_VAR, + ); + }); + + it("keeps the CLI-1942 session-pooler EOF unclassified so the generic --debug suggestion applies", () => { + // Go's SetConnectSuggestion has no branch for pgconn's `unexpected EOF` + // (the session-pooler drop in CLI-1942); node-postgres' equivalent + // `Connection terminated unexpectedly` must stay unclassified too. + const err = realSqlConnectError(new Error("Connection terminated unexpectedly")); + expect(legacyConnectSuggestion(err, ctx)).toBeUndefined(); + }); + it("returns undefined for an unrecognized connect error", () => { expect(legacyConnectSuggestion(sqlError(new Error("some other failure")), ctx)).toBeUndefined(); }); diff --git a/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.integration.test.ts b/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.integration.test.ts new file mode 100644 index 0000000000..9918b6cbfd --- /dev/null +++ b/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.integration.test.ts @@ -0,0 +1,184 @@ +/** + * Connection-failure behavior of the real `@effect/sql-pg` driver layer against + * real sockets: the `LegacyDbConnectError` message must carry Go's + * `failed to connect to postgres: failed to connect to + * `host=… user=… database=…`: ` structure (pgconn `errors.go:66-72`, + * `pkg/pgxv5/connect.go:33`), and the connect suggestion must classify real + * node-postgres error shapes, not libpq wording. + */ +import * as net from "node:net"; +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { LEGACY_SUGGEST_ENV_VAR } from "./legacy-connect-errors.ts"; +import type { LegacyDbConnectError } from "./legacy-db-connection.errors.ts"; +import { type LegacyPgConnInput, LegacyDbConnection } from "./legacy-db-connection.service.ts"; +import { legacyDbConnectionSqlPgLayer } from "./legacy-db-connection.sql-pg.layer.ts"; + +const SUGGESTION_CONTEXT = { + dashboardUrl: "https://supabase.com/dashboard", + profileName: "supabase", + debug: false, +} as const; + +// A distinctive sentinel so a regression that leaks the password into the +// rendered message or suggestion fails the assertions below (pgconn embeds only +// host/user/database — never the password). +const SENTINEL_PASSWORD = "s3cr3t-pw-do-not-leak"; + +/** Connect through the real layer and flip the expected failure into the value. */ +const connectFailure = ( + cfg: Partial & { readonly port: number }, +): Effect.Effect => + Effect.gen(function* () { + const conn = yield* LegacyDbConnection; + return yield* conn + .connect( + { + host: "127.0.0.1", + user: "postgres", + password: SENTINEL_PASSWORD, + database: "postgres", + suggestionContext: SUGGESTION_CONTEXT, + ...cfg, + }, + { isLocal: true, dnsResolver: "native" }, + ) + .pipe( + Effect.scoped, + Effect.flip, + Effect.mapError(() => new Error("expected the connection to fail")), + Effect.orDie, + ); + }).pipe(Effect.provide(legacyDbConnectionSqlPgLayer)); + +/** A TCP port that is guaranteed closed: bind an ephemeral port, then release it. */ +const acquireClosedPort = (): Promise => + new Promise((resolve, reject) => { + const server = net.createServer(); + server.listen(0, "127.0.0.1", () => { + const address = server.address() as net.AddressInfo; + server.close((error) => (error === undefined ? resolve(address.port) : reject(error))); + }); + }); + +/** Encode a Postgres wire-protocol ErrorResponse ('E') message. */ +const errorResponse = (fields: Record): Buffer => { + const parts: Array = []; + for (const [key, value] of Object.entries(fields)) { + parts.push(Buffer.from(key, "ascii"), Buffer.from(value, "utf8"), Buffer.from([0])); + } + parts.push(Buffer.from([0])); + const body = Buffer.concat(parts); + const head = Buffer.alloc(5); + head.write("E", 0, "ascii"); + head.writeInt32BE(body.length + 4, 1); + return Buffer.concat([head, body]); +}; + +/** + * A minimal fake Postgres server: answers `N` to an SSLRequest and hands the + * first startup message to `onStartup`, so tests can drive the real driver + * through real server-side failure shapes. + */ +const fakePostgresServer = ( + onStartup: (socket: net.Socket) => void, +): Promise<{ readonly port: number; readonly close: () => void }> => + new Promise((resolve) => { + const server = net.createServer((socket) => { + let sawStartup = false; + socket.on("data", (data: Buffer) => { + // SSLRequest: 8-byte message with request code 80877103. + if (!sawStartup && data.length >= 8 && data.readInt32BE(4) === 80877103) { + socket.write("N"); + return; + } + if (!sawStartup) { + sawStartup = true; + onStartup(socket); + } + }); + socket.on("error", () => {}); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address() as net.AddressInfo; + resolve({ port: address.port, close: () => server.close() }); + }); + }); + +describe("legacyDbConnectionSqlPgLayer connect failures", () => { + it.live( + "surfaces host, user, database, and the driver cause when the connection is refused", + () => + Effect.gen(function* () { + const port = yield* Effect.promise(acquireClosedPort); + const error = yield* connectFailure({ port }); + expect(error._tag).toBe("LegacyDbConnectError"); + expect(error.message).toBe( + "failed to connect to postgres: failed to connect to `host=127.0.0.1 user=postgres database=postgres`: " + + `dial error (connect ECONNREFUSED 127.0.0.1:${port})`, + ); + expect(error.suggestion).toBe( + "Make sure your local IP is allowed in Network Restrictions and Network Bans.\n" + + "https://supabase.com/dashboard/project/_/database/settings", + ); + expect(error.message).not.toContain(SENTINEL_PASSWORD); + }), + ); + + it.live( + "reproduces pgconn's server-error rendering for an auth failure and suggests SUPABASE_DB_PASSWORD", + () => + Effect.gen(function* () { + const server = yield* Effect.promise(() => + fakePostgresServer((socket) => { + socket.write( + errorResponse({ + S: "FATAL", + V: "FATAL", + C: "28P01", + M: 'password authentication failed for user "postgres"', + F: "auth.c", + L: "326", + R: "auth_failed", + }), + ); + socket.end(); + }), + ); + const error = yield* connectFailure({ port: server.port }).pipe( + Effect.ensuring(Effect.sync(server.close)), + ); + expect(error.message).toBe( + "failed to connect to postgres: failed to connect to `host=127.0.0.1 user=postgres database=postgres`: " + + 'server error (FATAL: password authentication failed for user "postgres" (SQLSTATE 28P01))', + ); + expect(error.suggestion).toBe(LEGACY_SUGGEST_ENV_VAR); + expect(error.message).not.toContain(SENTINEL_PASSWORD); + }), + ); + + it.live( + "keeps the CLI-1942 session-pooler EOF shape unclassified while surfacing the cause", + () => + Effect.gen(function* () { + // The session pooler dropping the connection (CLI-1942) surfaces as + // node-postgres' `Connection terminated unexpectedly`. Go has no + // suggestion branch for the equivalent `unexpected EOF`, so no + // suggestion may fire — the generic --debug fallback applies. + const server = yield* Effect.promise(() => + fakePostgresServer((socket) => socket.destroy()), + ); + const error = yield* connectFailure({ + port: server.port, + user: "postgres.abcdefghijklmnopqrst", + }).pipe(Effect.ensuring(Effect.sync(server.close))); + expect(error.message).toBe( + "failed to connect to postgres: failed to connect to " + + "`host=127.0.0.1 user=postgres.abcdefghijklmnopqrst database=postgres`: " + + "Connection terminated unexpectedly", + ); + expect(error.suggestion).toBeUndefined(); + }), + ); +}); diff --git a/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts b/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts index 7553b1a3e0..d4f14f790a 100644 --- a/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts +++ b/apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts @@ -11,7 +11,7 @@ import { ConnectionError, SqlError } from "effect/unstable/sql/SqlError"; // resolves, so the COPY path and the pooled path use the same driver. import * as Pg from "pg"; import { to as pgCopyTo } from "pg-copy-streams"; -import { legacyConnectSuggestion } from "./legacy-connect-errors.ts"; +import { legacyConnectFailureMessage, legacyConnectSuggestion } from "./legacy-connect-errors.ts"; import { LegacyDbConnectError, LegacyDbCopyError, @@ -584,13 +584,18 @@ const connect = ( // (`connect.go:187`), mapping the driver error to an actionable hint that replaces // the generic "--debug" suggestion. The resolver attaches the profile context to // `cfg.suggestionContext`; map it here so the suggestion travels on the error. + // The message mirrors Go's `pgxv5.Connect` wrap of pgconn's `connectError` + // (`pkg/pgxv5/connect.go:33`, pgconn `errors.go:66-72`): the `failed to connect + // to postgres:` prefix plus the `host=… user=… database=…` identity and the + // underlying driver cause — not the bare `SqlError` toString, which drops all + // of that detail. const toConnectError = (error: unknown) => { const suggestion = cfg.suggestionContext === undefined ? undefined : legacyConnectSuggestion(error, cfg.suggestionContext); return new LegacyDbConnectError({ - message: `failed to connect to postgres: ${error}`, + message: `failed to connect to postgres: ${legacyConnectFailureMessage(cfg, error)}`, ...(suggestion === undefined ? {} : { suggestion }), }); }; From c4b458748a07a300ef43d736784c3d61b8bdbd1e Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 27 Jul 2026 21:28:23 +0100 Subject: [PATCH 05/24] fix(cli): honor --yes/SUPABASE_YES and Go prompt semantics consistently across confirmation prompts (#5947) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routes every legacy-parity confirmation through the single Go-faithful helper `legacyPromptYesNo` (Go's `PromptYesNo`, `apps/cli-go/internal/utils/console.go:38-107`) and deletes the per-command shortcuts, so all three Go prompt properties hold everywhere: 1. **`--yes` OR `SUPABASE_YES` auto-confirms** (viper `AutomaticEnv`; project `.env` consulted exactly where Go's `loadNestedEnv` runs before the prompt). 2. **Auto-confirm echoes `