From 3bcdf14ef57077cdf83e997728f04818c250506a Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 3 Aug 2026 11:27:47 +0100 Subject: [PATCH 1/3] refactor(cli): hoist sso pflag/profile reconciliation into shared layer (CLI-1982) sso.pflag-reconcile.ts and sso.load-profile.ts implement a CLI-wide concern (pflag/viper-vs-Effect-parser reconciliation) but were coupled to sso-specific error types. Move both into legacy/shared/ as legacy-pflag-reconcile.ts and legacy-profile-load.ts, decoupled via new LegacyPflagWorkdirError/ LegacyProfileLoadError, so future command families needing the same reconciliation don't have to duplicate it. Pure move+rename; add/update wired to the new location with zero behavior change. --- .../legacy/commands/sso/add/SIDE_EFFECTS.md | 30 ++--- .../legacy/commands/sso/add/add.handler.ts | 36 +++--- .../commands/sso/add/add.integration.test.ts | 10 +- .../cli/src/legacy/commands/sso/sso.errors.ts | 25 ---- apps/cli/src/legacy/commands/sso/sso.saml.ts | 2 +- .../commands/sso/update/SIDE_EFFECTS.md | 32 ++--- .../commands/sso/update/update.handler.ts | 46 ++++--- .../sso/update/update.integration.test.ts | 10 +- .../legacy-pflag-reconcile.ts} | 80 +++++++----- .../legacy-pflag-reconcile.unit.test.ts} | 117 +++++++++--------- .../legacy-profile-load.ts} | 26 ++-- .../legacy-profile-load.unit.test.ts} | 21 ++-- 12 files changed, 221 insertions(+), 214 deletions(-) rename apps/cli/src/legacy/{commands/sso/sso.pflag-reconcile.ts => shared/legacy-pflag-reconcile.ts} (84%) rename apps/cli/src/legacy/{commands/sso/sso.pflag-reconcile.unit.test.ts => shared/legacy-pflag-reconcile.unit.test.ts} (72%) rename apps/cli/src/legacy/{commands/sso/sso.load-profile.ts => shared/legacy-profile-load.ts} (92%) rename apps/cli/src/legacy/{commands/sso/sso.load-profile.unit.test.ts => shared/legacy-profile-load.unit.test.ts} (94%) diff --git a/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md index 209a143670..36be091719 100644 --- a/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md @@ -40,20 +40,20 @@ same shape via an inline anonymous struct with `Default *any`. ## Exit Codes -| Code | Condition | -| ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | `LegacySsoInvalidFlagValueError` — a `--type`/`--skip-url-validation`/`--name-id-format` occurrence pflag's `Value.Set` would reject (enum membership / `strconv.ParseBool`; fails before every validation; no request) | -| `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before every validation; no request) | -| `1` | `LegacySsoProfileError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; beats the workdir, required-flag, and mutex checks; no request) | -| `1` | `LegacySsoWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; beats the required-flag and mutex checks; no request) | -| `1` | `LegacySsoAddRequiredFlagError` — pflag consumed the `--type`/`-t` token as another flag's value (cobra `ValidateRequiredFlags`) | -| `1` | `LegacySsoMutexFlagError` — `--metadata-file` and `--metadata-url` both set | -| `1` | `LegacySsoAddMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | -| `1` | `LegacySsoAddAttributeMappingFileError` — JSON file unreadable or malformed | -| `1` | `LegacySsoAddSamlDisabledError` — 404 from POST | -| `1` | `LegacySsoAddUnexpectedStatusError` — other non-2xx | -| `1` | `LegacySsoAddNetworkError` — transport-level failure | +| Code | Condition | +| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `LegacySsoInvalidFlagValueError` — a `--type`/`--skip-url-validation`/`--name-id-format` occurrence pflag's `Value.Set` would reject (enum membership / `strconv.ParseBool`; fails before every validation; no request) | +| `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before every validation; no request) | +| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; beats the workdir, required-flag, and mutex checks; no request) | +| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; beats the required-flag and mutex checks; no request) | +| `1` | `LegacySsoAddRequiredFlagError` — pflag consumed the `--type`/`-t` token as another flag's value (cobra `ValidateRequiredFlags`) | +| `1` | `LegacySsoMutexFlagError` — `--metadata-file` and `--metadata-url` both set | +| `1` | `LegacySsoAddMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | +| `1` | `LegacySsoAddAttributeMappingFileError` — JSON file unreadable or malformed | +| `1` | `LegacySsoAddSamlDisabledError` — 404 from POST | +| `1` | `LegacySsoAddUnexpectedStatusError` — other non-2xx | +| `1` | `LegacySsoAddNetworkError` — transport-level failure | ## Telemetry Events Fired @@ -87,7 +87,7 @@ Single `success` event with the parsed response as data. - Flag values follow pflag's consumption rules, not the TS parser's: every value the handler acts on (`--project-ref`, `--metadata-file`, `--metadata-url`, `--attribute-mapping-file`, `--domains`, `--name-id-format`, `--skip-url-validation`) is reconciled against a pflag-faithful raw-argv scan. E.g. `--project-ref --metadata-file x.xml --metadata-url u` hands `--metadata-file` to `--project-ref` as its value and fails ref validation — the metadata file is never read (CLI-1982). Repeated flags resolve last-wins (pflag Sets every occurrence; the TS parser is first-wins), and an occurrence pflag's `Value.Set` would reject — `--type` outside `[ saml ]`, a boolean outside Go's `strconv.ParseBool` set (`--skip-url-validation=yes`), or a `--name-id-format` outside the enum — fails with pflag's exact `invalid argument …` message before every validation and request. - Required-ness follows pflag too: when the `--type` token is itself consumed as another flag's value (`--domains --type saml`), the command fails with cobra's exact `required flag(s) "type" not set` before any request (cobra `ValidateRequiredFlags` runs before `ValidateFlagGroups`). `-t` shorthand occurrences are recognised by the scan and never trip this. - The workdir follows pflag/viper too: Go's `ChangeWorkDir` (root `PersistentPreRunE`) chdir's to the effective `--workdir` (last occurrence, even a flag-shaped consumed token like `--workdir --metadata-file`) or `SUPABASE_WORKDIR`, and a missing directory aborts with Go's exact `failed to change workdir: chdir …` before the required-flag check, the mutex check, and any request. A changed-but-empty `--workdir=` shadows the env var and falls back to the always-valid project-root walk-up, exactly like viper. -- The profile follows pflag/viper too (PR #5974 round 7): whenever the pflag-effective `--profile`/`SUPABASE_PROFILE` token differs from the one the Effect parser gave the config layer (a `--profile` token consumed by another flag — `--domains --profile alternate.yml` targets the env/default profile, not `alternate.yml`; a flag-shaped consumed value — `--profile --metadata-url`; repeats, which pflag resolves last-wins; an explicit `--profile supabase` shadowing the env; an untrimmed/empty persisted `~/.supabase/profile` file), the handler re-runs Go's `LoadProfile` on the effective token (`sso.load-profile.ts`) — the POST targets that profile's `api_url`, and a token Go cannot load aborts with Go's error (`failed to read profile: …` / `failed to parse profile: …` / `invalid profile: …`, byte-exact for the deterministic classes) before the workdir check and any request. Where the scan and the parser agree — every normal invocation — the config layer's resolution (including its pre-existing lenient missing/malformed-file fallback, which predates CLI-1982 and applies shell-wide) is used unchanged. The upgrade-gate fallback GETs and the linked-project cache fill also target the reconciled host (Go's `CurrentProfile` is process-wide). +- The profile follows pflag/viper too (PR #5974 round 7): whenever the pflag-effective `--profile`/`SUPABASE_PROFILE` token differs from the one the Effect parser gave the config layer (a `--profile` token consumed by another flag — `--domains --profile alternate.yml` targets the env/default profile, not `alternate.yml`; a flag-shaped consumed value — `--profile --metadata-url`; repeats, which pflag resolves last-wins; an explicit `--profile supabase` shadowing the env; an untrimmed/empty persisted `~/.supabase/profile` file), the handler re-runs Go's `LoadProfile` on the effective token (`legacy-profile-load.ts`) — the POST targets that profile's `api_url`, and a token Go cannot load aborts with Go's error (`failed to read profile: …` / `failed to parse profile: …` / `invalid profile: …`, byte-exact for the deterministic classes) before the workdir check and any request. Where the scan and the parser agree — every normal invocation — the config layer's resolution (including its pre-existing lenient missing/malformed-file fallback, which predates CLI-1982 and applies shell-wide) is used unchanged. The upgrade-gate fallback GETs and the linked-project cache fill also target the reconciled host (Go's `CurrentProfile` is process-wide). - Accepted micro-divergences of the profile emulation (each fail-closed: both CLIs exit 1 with zero requests; only stderr detail can differ): YAML parse-failure detail text (JS `yaml` vs go-yaml, shared `failed to read profile: While parsing config: ` prefix); non-YAML/JSON viper config types (`.toml`, `.env`, …) parsed as YAML; `http_url`/`hostname_rfc1123`/`uuid4` validator tags approximated; the final line of a padded multi-line error loses its trailing spaces to the shared error normalizer's trim. Also: when the effective and layer profiles differ AND the token is keyring-relevant, the keyring token lookup still uses the layer profile's name (env-token flows, e.g. the cli-e2e harness, are unaffected), and the upgrade-suggestion billing URL keeps the layer profile's dashboard host. - `--skip-url-validation` skips the HTTPS-only + 10s GET + UTF-8 body validation against the metadata URL. - Metadata URL validation error message: `only HTTPS Metadata URLs are supported Use --skip-url-validation to suppress this error` (no trailing period — matches Go's `create.go:47`; differs from `sso update`'s variant). diff --git a/apps/cli/src/legacy/commands/sso/add/add.handler.ts b/apps/cli/src/legacy/commands/sso/add/add.handler.ts index 36ba8a9d79..cc2b3420de 100644 --- a/apps/cli/src/legacy/commands/sso/add/add.handler.ts +++ b/apps/cli/src/legacy/commands/sso/add/add.handler.ts @@ -25,6 +25,14 @@ import { legacyMissingAccessTokenMessage } from "../../../auth/legacy-access-tok import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { legacySuggestUpgrade } from "../../../shared/legacy-upgrade-suggest.ts"; +import { + legacyPflagBoolValue, + legacyPflagEnumValue, + legacyPflagSliceValue, + legacyPflagStringValue, + legacyResolvePflagProfile, + legacyValidatePflagWorkdir, +} from "../../../shared/legacy-pflag-reconcile.ts"; import { LegacySsoAddAttributeMappingFileError, LegacySsoAddMetadataFileError, @@ -39,14 +47,6 @@ import { } from "../sso.errors.ts"; import { renderSingleProvider, toLegacySsoProviderView } from "../sso.format.ts"; import { validateMetadataUrl } from "../sso.metadata-url.ts"; -import { - legacySsoPflagBoolValue, - legacySsoPflagEnumValue, - legacySsoPflagSliceValue, - legacySsoPflagStringValue, - legacySsoResolvePflagProfile, - legacySsoValidatePflagWorkdir, -} from "../sso.pflag-reconcile.ts"; import { LEGACY_SSO_NAME_ID_FORMATS, readAttributeMappingFile, @@ -144,12 +144,12 @@ export const legacySsoAdd = Effect.fn("legacy.sso.add")(function* (flags: Legacy // values; `--type`'s stays unused because every valid occurrence is the // enum's single member, so the parsed `flags.type` is already // pflag-effective whenever this validation passes. - yield* Result.match(legacySsoPflagEnumValue(occurrences, "type", ["saml"], "-t, --type"), { + yield* Result.match(legacyPflagEnumValue(occurrences, "type", ["saml"], "-t, --type"), { onFailure: (message: string) => Effect.fail(new LegacySsoInvalidFlagValueError({ message })), onSuccess: Effect.succeed, }); const skipUrlValidation = yield* Result.match( - legacySsoPflagBoolValue(occurrences, "skip-url-validation"), + legacyPflagBoolValue(occurrences, "skip-url-validation"), { onFailure: (message: string) => Effect.fail(new LegacySsoInvalidFlagValueError({ message })), @@ -157,7 +157,7 @@ export const legacySsoAdd = Effect.fn("legacy.sso.add")(function* (flags: Legacy }, ); const nameIdFormat = yield* Result.match( - legacySsoPflagEnumValue(occurrences, "name-id-format", LEGACY_SSO_NAME_ID_FORMATS), + legacyPflagEnumValue(occurrences, "name-id-format", LEGACY_SSO_NAME_ID_FORMATS), { onFailure: (message: string) => Effect.fail(new LegacySsoInvalidFlagValueError({ message })), @@ -192,7 +192,7 @@ export const legacySsoAdd = Effect.fn("legacy.sso.add")(function* (flags: Legacy // contacts (binary-verified, PR #5974 review round 7). Where the scan // and the parser agree, this resolves to `none` and the config layer's // apiUrl below is already pflag-effective. - const reconciledProfile = yield* legacySsoResolvePflagProfile(scan); + const reconciledProfile = yield* legacyResolvePflagProfile(scan); const profileApiUrl = Option.map(reconciledProfile, (profile) => profile.apiUrl); // Reconciled-profile credentials, resolved ONCE for the main request and // every auxiliary call (linked-project cache fill, upgrade-gate fallback @@ -231,7 +231,7 @@ export const legacySsoAdd = Effect.fn("legacy.sso.add")(function* (flags: Legacy // metadata — without this check the reconciliation below would silently // drop the metadata source and POST a provider Go never creates // (binary-verified, PR #5974 review round 6). - yield* legacySsoValidatePflagWorkdir(scan); + yield* legacyValidatePflagWorkdir(scan); // `MarkFlagRequired("type")` (`cmd/sso.go:165`): when pflag consumed the // `--type` or `-t` token as another flag's value (e.g. `--domains --type @@ -270,11 +270,11 @@ export const legacySsoAdd = Effect.fn("legacy.sso.add")(function* (flags: Legacy // required-flag check above). `--name-id-format` and // `--skip-url-validation` were reconciled above, alongside their pflag // value validation. - const projectRef = legacySsoPflagStringValue(occurrences, "project-ref"); - const metadataFile = legacySsoPflagStringValue(occurrences, "metadata-file"); - const metadataUrl = legacySsoPflagStringValue(occurrences, "metadata-url"); - const attributeMappingFile = legacySsoPflagStringValue(occurrences, "attribute-mapping-file"); - const domains = legacySsoPflagSliceValue(occurrences, "domains", flags.domains); + const projectRef = legacyPflagStringValue(occurrences, "project-ref"); + const metadataFile = legacyPflagStringValue(occurrences, "metadata-file"); + const metadataUrl = legacyPflagStringValue(occurrences, "metadata-url"); + const attributeMappingFile = legacyPflagStringValue(occurrences, "attribute-mapping-file"); + const domains = legacyPflagSliceValue(occurrences, "domains", flags.domains); const ref = yield* resolver.resolve(projectRef); diff --git a/apps/cli/src/legacy/commands/sso/add/add.integration.test.ts b/apps/cli/src/legacy/commands/sso/add/add.integration.test.ts index 4cc0151703..1ad938e554 100644 --- a/apps/cli/src/legacy/commands/sso/add/add.integration.test.ts +++ b/apps/cli/src/legacy/commands/sso/add/add.integration.test.ts @@ -478,7 +478,7 @@ describe("legacy sso add integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoWorkdirError"); + expect(dump).toContain("LegacyPflagWorkdirError"); expect(dump).toContain( "failed to change workdir: chdir --metadata-file: no such file or directory", ); @@ -519,7 +519,7 @@ describe("legacy sso add integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoWorkdirError"); + expect(dump).toContain("LegacyPflagWorkdirError"); expect(dump).toContain( "failed to change workdir: chdir /nonexistent-sso-add-workdir: no such file or directory", ); @@ -1228,7 +1228,7 @@ describe("legacy sso add integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoProfileError"); + expect(dump).toContain("LegacyProfileLoadError"); expect(dump).toContain(`failed to read profile: Unsupported Config Type \\"\\"`); } expect(api.requests.length).toBe(0); @@ -1288,8 +1288,8 @@ describe("legacy sso add integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoProfileError"); - expect(dump).not.toContain("LegacySsoWorkdirError"); + expect(dump).toContain("LegacyProfileLoadError"); + expect(dump).not.toContain("LegacyPflagWorkdirError"); expect(dump).not.toContain("LegacySsoAddRequiredFlagError"); expect(dump).not.toContain("LegacySsoMutexFlagError"); } diff --git a/apps/cli/src/legacy/commands/sso/sso.errors.ts b/apps/cli/src/legacy/commands/sso/sso.errors.ts index 375bfa2eaa..5fe12e3251 100644 --- a/apps/cli/src/legacy/commands/sso/sso.errors.ts +++ b/apps/cli/src/legacy/commands/sso/sso.errors.ts @@ -100,31 +100,6 @@ export class LegacySsoAddRequiredFlagError extends Data.TaggedError( readonly message: string; }> {} -// Go's `ChangeWorkDir` (`internal/utils/misc.go:238-257`), run from the root -// `PersistentPreRunE` (`cmd/root.go:104`) — after `ParseFlags` and -// `ValidateArgs`, before `ValidateRequiredFlags`, `ValidateFlagGroups`, and -// `RunE` — so a missing workdir directory aborts with no API call ever made. -// Emulated for the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` the -// Effect layer never validates (and, when `--workdir` consumed a flag-shaped -// token, never even saw — PR #5974 review round 6). Shared across add + -// update; message byte-matches Go's template. -export class LegacySsoWorkdirError extends Data.TaggedError("LegacySsoWorkdirError")<{ - readonly message: string; -}> {} - -// Go's `LoadProfile` (`internal/utils/profile.go:94-118`), run from the root -// `PersistentPreRunE` (`cmd/root.go:98-102`) immediately BEFORE -// `ChangeWorkDir` — so a profile Go cannot load aborts before the workdir -// check, `ValidateRequiredFlags`, `ValidateFlagGroups`, and `RunE`, with no -// API call ever made. Emulated for the pflag/viper-effective `--profile`/ -// `SUPABASE_PROFILE` whenever it differs from the token the Effect config -// layer resolved (PR #5974 review round 7). Shared across add + update; -// message byte-matches Go for the deterministic failure classes (see -// `sso.load-profile.ts`). -export class LegacySsoProfileError extends Data.TaggedError("LegacySsoProfileError")<{ - readonly message: string; -}> {} - // Shared across add + update — metadata URL validation. export class LegacySsoMetadataUrlInvalidError extends Data.TaggedError( "LegacySsoMetadataUrlInvalidError", diff --git a/apps/cli/src/legacy/commands/sso/sso.saml.ts b/apps/cli/src/legacy/commands/sso/sso.saml.ts index 118f10a52b..8b3e93a6c0 100644 --- a/apps/cli/src/legacy/commands/sso/sso.saml.ts +++ b/apps/cli/src/legacy/commands/sso/sso.saml.ts @@ -5,7 +5,7 @@ import { Effect, FileSystem } from "effect"; * (both commands bind the same Go `ssoNameIDFormat` enum var, * `cmd/sso.go:158,176`). Order matters twice: it drives the CLI help text * and it is joined verbatim into pflag's `invalid argument … must be one of - * [ … ]` error (`legacySsoPflagEnumValue`), which must byte-match Go. + * [ … ]` error (`legacyPflagEnumValue`), which must byte-match Go. */ export const LEGACY_SSO_NAME_ID_FORMATS = [ "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", diff --git a/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md index cd10ca995e..b2e3937ec8 100644 --- a/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md @@ -41,21 +41,21 @@ GET still uses the typed client. ## Exit Codes -| Code | Condition | -| ---- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `0` | success | -| `1` | `LegacySsoInvalidFlagValueError` — a `--skip-url-validation`/`--name-id-format` occurrence pflag's `Value.Set` would reject (`strconv.ParseBool` / enum membership; fails before every validation; no request) | -| `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before `ValidateArgs`; no request) | -| `1` | `LegacySsoUpdateArityError` — pflag-effective positional count ≠ 1 (cobra `ValidateArgs`/`ExactArgs(1)`; a consumed flag token orphans its parser-value into the positionals) | -| `1` | `LegacySsoProfileError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; loses to the arity check, beats the workdir and mutex checks; no request) | -| `1` | `LegacySsoWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; loses to the arity check, beats the mutex checks; no request) | -| `1` | `LegacySsoInvalidUuidError` — provider ID is not a canonical UUID | -| `1` | `LegacySsoMutexFlagError` — flag combinations: `--domains` with `--add/--remove-domains`, or `--metadata-file` with `--metadata-url` | -| `1` | `LegacySsoUpdateMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | -| `1` | `LegacySsoUpdateAttributeMappingFileError` — JSON file unreadable or malformed | -| `1` | `LegacySsoUpdateNotFoundError` — 404 from GET | -| `1` | `LegacySsoUpdateUnexpectedStatusError` — non-2xx from GET or PUT | -| `1` | `LegacySsoUpdateNetworkError` — transport-level failure | +| Code | Condition | +| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | `LegacySsoInvalidFlagValueError` — a `--skip-url-validation`/`--name-id-format` occurrence pflag's `Value.Set` would reject (`strconv.ParseBool` / enum membership; fails before every validation; no request) | +| `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before `ValidateArgs`; no request) | +| `1` | `LegacySsoUpdateArityError` — pflag-effective positional count ≠ 1 (cobra `ValidateArgs`/`ExactArgs(1)`; a consumed flag token orphans its parser-value into the positionals) | +| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; loses to the arity check, beats the workdir and mutex checks; no request) | +| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; loses to the arity check, beats the mutex checks; no request) | +| `1` | `LegacySsoInvalidUuidError` — provider ID is not a canonical UUID | +| `1` | `LegacySsoMutexFlagError` — flag combinations: `--domains` with `--add/--remove-domains`, or `--metadata-file` with `--metadata-url` | +| `1` | `LegacySsoUpdateMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | +| `1` | `LegacySsoUpdateAttributeMappingFileError` — JSON file unreadable or malformed | +| `1` | `LegacySsoUpdateNotFoundError` — 404 from GET | +| `1` | `LegacySsoUpdateUnexpectedStatusError` — non-2xx from GET or PUT | +| `1` | `LegacySsoUpdateNetworkError` — transport-level failure | ## Telemetry Events Fired @@ -89,7 +89,7 @@ Single `success` event with the parsed response as data. - Flag values follow pflag's consumption rules, not the TS parser's: every value the handler acts on (`--project-ref`, `--metadata-file`, `--metadata-url`, `--attribute-mapping-file`, the three domain slices, `--name-id-format`, `--skip-url-validation`) is reconciled against a pflag-faithful raw-argv scan — same mechanism as `sso add` (CLI-1982). Repeated flags resolve last-wins (pflag Sets every occurrence; the TS parser is first-wins), and an occurrence pflag's `Value.Set` would reject — a boolean outside Go's `strconv.ParseBool` set (`--skip-url-validation=yes`), or a `--name-id-format` outside the enum — fails with pflag's exact `invalid argument …` message before any validation or request. - Positional arity follows pflag too: cobra's `ExactArgs(1)` is re-counted over pflag-effective positionals (`ValidateArgs` runs before every hook and flag validation), so a consumed flag token that orphans its parser-value (`--domains --metadata-url u `) fails with cobra's exact `accepts 1 arg(s), received 2` before the GET — and wins over both the mutex and invalid-UUID errors. - The workdir follows pflag/viper too: Go's `ChangeWorkDir` (root `PersistentPreRunE`) chdir's to the effective `--workdir` (last occurrence, even a flag-shaped consumed token like `--workdir --metadata-file`) or `SUPABASE_WORKDIR`, and a missing directory aborts with Go's exact `failed to change workdir: chdir …` after the arity check but before the mutex checks and any request. A changed-but-empty `--workdir=` shadows the env var and falls back to the always-valid project-root walk-up, exactly like viper. -- The profile follows pflag/viper too (PR #5974 round 7): whenever the pflag-effective `--profile`/`SUPABASE_PROFILE` token differs from the one the Effect parser gave the config layer (repeats — pflag is last-wins where the parser is first-wins, so `--profile a.yml --profile b.yml` GETs and PUTs `b.yml`'s `api_url`; a flag-shaped consumed value — `--profile --add-domains`; an explicit `--profile supabase` shadowing the env; an untrimmed/empty persisted `~/.supabase/profile` file), the handler re-runs Go's `LoadProfile` on the effective token (`sso.load-profile.ts`). Both the initial GET and the PUT then target that profile's `api_url` — the GET is issued through the raw HTTP client because the typed client bakes the layer's `api_url` in at construction — and a token Go cannot load aborts with Go's error (`failed to read profile: …` / `failed to parse profile: …` / `invalid profile: …`, byte-exact for the deterministic classes) after the arity check, before the workdir and mutex checks and any request. The raw GET mirrors Go's generated client on the 200 path too: an undecodable JSON body aborts with `failed to get sso provider: ` before any PUT (`update.go:42-45`; detail text is JS `JSON.parse`'s — micro-divergence), a 200 without a JSON content type falls into the gate + unexpected-status branch like Go's nil `JSON200`, and the response is stitched through the shared per-command identity guard exactly like the typed client (Go's `identityTransport` wraps every Management API response). The upgrade-gate fallback GETs and the linked-project cache fill also target the reconciled host (Go's `CurrentProfile` is process-wide). Where the scan and the parser agree — every normal invocation — the config layer's resolution (including its pre-existing lenient missing/malformed-file fallback, which predates CLI-1982 and applies shell-wide) is used unchanged, via the typed client. +- The profile follows pflag/viper too (PR #5974 round 7): whenever the pflag-effective `--profile`/`SUPABASE_PROFILE` token differs from the one the Effect parser gave the config layer (repeats — pflag is last-wins where the parser is first-wins, so `--profile a.yml --profile b.yml` GETs and PUTs `b.yml`'s `api_url`; a flag-shaped consumed value — `--profile --add-domains`; an explicit `--profile supabase` shadowing the env; an untrimmed/empty persisted `~/.supabase/profile` file), the handler re-runs Go's `LoadProfile` on the effective token (`legacy-profile-load.ts`). Both the initial GET and the PUT then target that profile's `api_url` — the GET is issued through the raw HTTP client because the typed client bakes the layer's `api_url` in at construction — and a token Go cannot load aborts with Go's error (`failed to read profile: …` / `failed to parse profile: …` / `invalid profile: …`, byte-exact for the deterministic classes) after the arity check, before the workdir and mutex checks and any request. The raw GET mirrors Go's generated client on the 200 path too: an undecodable JSON body aborts with `failed to get sso provider: ` before any PUT (`update.go:42-45`; detail text is JS `JSON.parse`'s — micro-divergence), a 200 without a JSON content type falls into the gate + unexpected-status branch like Go's nil `JSON200`, and the response is stitched through the shared per-command identity guard exactly like the typed client (Go's `identityTransport` wraps every Management API response). The upgrade-gate fallback GETs and the linked-project cache fill also target the reconciled host (Go's `CurrentProfile` is process-wide). Where the scan and the parser agree — every normal invocation — the config layer's resolution (including its pre-existing lenient missing/malformed-file fallback, which predates CLI-1982 and applies shell-wide) is used unchanged, via the typed client. - Accepted micro-divergences of the profile emulation (each fail-closed: both CLIs exit 1 with zero requests; only stderr detail can differ): YAML parse-failure detail text (JS `yaml` vs go-yaml, shared `failed to read profile: While parsing config: ` prefix); non-YAML/JSON viper config types (`.toml`, `.env`, …) parsed as YAML; `http_url`/`hostname_rfc1123`/`uuid4` validator tags approximated; the final line of a padded multi-line error loses its trailing spaces to the shared error normalizer's trim. Also: when the effective and layer profiles differ AND the token is keyring-relevant, the keyring token lookup still uses the layer profile's name (env-token flows, e.g. the cli-e2e harness, are unaffected), and the upgrade-suggestion billing URL keeps the layer profile's dashboard host. - Always performs the GET pre-check (matches Go's `update.go:42`), regardless of whether `--add-domains` / `--remove-domains` are used. - Domain merge: removals are applied first, then additions. Go uses a `map[string]bool` so the resulting order is **unordered**; consumers must sort if comparing. diff --git a/apps/cli/src/legacy/commands/sso/update/update.handler.ts b/apps/cli/src/legacy/commands/sso/update/update.handler.ts index 7fc315aa77..3fb85bbabd 100644 --- a/apps/cli/src/legacy/commands/sso/update/update.handler.ts +++ b/apps/cli/src/legacy/commands/sso/update/update.handler.ts @@ -31,6 +31,14 @@ import { legacyGateResponse, legacySuggestUpgrade, } from "../../../shared/legacy-upgrade-suggest.ts"; +import { + legacyPflagBoolValue, + legacyPflagEnumValue, + legacyPflagSliceValue, + legacyPflagStringValue, + legacyResolvePflagProfile, + legacyValidatePflagWorkdir, +} from "../../../shared/legacy-pflag-reconcile.ts"; import { LegacySsoFlagNeedsArgumentError, LegacySsoInvalidFlagValueError, @@ -45,14 +53,6 @@ import { } from "../sso.errors.ts"; import { renderSingleProvider, toLegacySsoProviderView, validateUuid } from "../sso.format.ts"; import { validateMetadataUrl } from "../sso.metadata-url.ts"; -import { - legacySsoPflagBoolValue, - legacySsoPflagEnumValue, - legacySsoPflagSliceValue, - legacySsoPflagStringValue, - legacySsoResolvePflagProfile, - legacySsoValidatePflagWorkdir, -} from "../sso.pflag-reconcile.ts"; import { LEGACY_SSO_NAME_ID_FORMATS, readAttributeMappingFile, @@ -241,7 +241,7 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* ( // through sane usage. The same helpers yield the pflag-effective // (last-occurrence) values the handler acts on below. const skipUrlValidation = yield* Result.match( - legacySsoPflagBoolValue(occurrences, "skip-url-validation"), + legacyPflagBoolValue(occurrences, "skip-url-validation"), { onFailure: (message: string) => Effect.fail(new LegacySsoInvalidFlagValueError({ message })), @@ -249,7 +249,7 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* ( }, ); const nameIdFormat = yield* Result.match( - legacySsoPflagEnumValue(occurrences, "name-id-format", LEGACY_SSO_NAME_ID_FORMATS), + legacyPflagEnumValue(occurrences, "name-id-format", LEGACY_SSO_NAME_ID_FORMATS), { onFailure: (message: string) => Effect.fail(new LegacySsoInvalidFlagValueError({ message })), @@ -292,10 +292,10 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* ( // an arity violation but beats the workdir check, the mutex checks, and // any GET/PUT — and a loadable one decides which API host receives them. // Reachable exactly where the scan and the parser disagree (see - // `add.handler.ts` and `legacySsoResolvePflagProfile` — PR #5974 + // `add.handler.ts` and `legacyResolvePflagProfile` — PR #5974 // review round 7); where they agree this is `none` and the config // layer's client/apiUrl below are already pflag-effective. - const reconciledProfile = yield* legacySsoResolvePflagProfile(scan); + const reconciledProfile = yield* legacyResolvePflagProfile(scan); const profileApiUrl = Option.map(reconciledProfile, (profile) => profile.apiUrl); // Reconciled-profile credentials, resolved ONCE for the main request and // every auxiliary call (linked-project cache fill, upgrade-gate fallback @@ -329,7 +329,7 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* ( // (binary-verified: `sso update a b --workdir /missing` reports the // arity error; `sso update --workdir /missing --domains a // --add-domains b` reports the chdir failure — PR #5974 review round 6). - yield* legacySsoValidatePflagWorkdir(scan); + yield* legacyValidatePflagWorkdir(scan); for (const group of SSO_UPDATE_MUTEX_GROUPS) { const changed = group.filter((flagName) => occurrences.has(flagName)); @@ -347,20 +347,16 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* ( // tokens as values while pflag consumes them unconditionally, and // resolves repeated flags first-wins while pflag is last-wins, so the // two can disagree on which flags are set and what they hold. See - // `add.handler.ts` and `sso.pflag-reconcile.ts` for the full rationale + // `add.handler.ts` and `legacy-pflag-reconcile.ts` for the full rationale // (CLI-1982). `--name-id-format` and `--skip-url-validation` were // reconciled above, alongside their pflag value validation. - const projectRefFlag = legacySsoPflagStringValue(occurrences, "project-ref"); - const metadataFile = legacySsoPflagStringValue(occurrences, "metadata-file"); - const metadataUrl = legacySsoPflagStringValue(occurrences, "metadata-url"); - const attributeMappingFile = legacySsoPflagStringValue(occurrences, "attribute-mapping-file"); - const domains = legacySsoPflagSliceValue(occurrences, "domains", flags.domains); - const addDomains = legacySsoPflagSliceValue(occurrences, "add-domains", flags.addDomains); - const removeDomains = legacySsoPflagSliceValue( - occurrences, - "remove-domains", - flags.removeDomains, - ); + const projectRefFlag = legacyPflagStringValue(occurrences, "project-ref"); + const metadataFile = legacyPflagStringValue(occurrences, "metadata-file"); + const metadataUrl = legacyPflagStringValue(occurrences, "metadata-url"); + const attributeMappingFile = legacyPflagStringValue(occurrences, "attribute-mapping-file"); + const domains = legacyPflagSliceValue(occurrences, "domains", flags.domains); + const addDomains = legacyPflagSliceValue(occurrences, "add-domains", flags.addDomains); + const removeDomains = legacyPflagSliceValue(occurrences, "remove-domains", flags.removeDomains); const providerId = yield* validateUuid(flags.providerId).pipe( Result.match({ onFailure: Effect.fail, onSuccess: Effect.succeed }), diff --git a/apps/cli/src/legacy/commands/sso/update/update.integration.test.ts b/apps/cli/src/legacy/commands/sso/update/update.integration.test.ts index 8c057f87e7..091645b63c 100644 --- a/apps/cli/src/legacy/commands/sso/update/update.integration.test.ts +++ b/apps/cli/src/legacy/commands/sso/update/update.integration.test.ts @@ -755,7 +755,7 @@ describe("legacy sso update integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoWorkdirError"); + expect(dump).toContain("LegacyPflagWorkdirError"); expect(dump).toContain( "failed to change workdir: chdir --metadata-file: no such file or directory", ); @@ -794,7 +794,7 @@ describe("legacy sso update integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoWorkdirError"); + expect(dump).toContain("LegacyPflagWorkdirError"); expect(dump).toContain( "failed to change workdir: chdir /nonexistent-sso-update-workdir: no such file or directory", ); @@ -816,7 +816,7 @@ describe("legacy sso update integration", () => { const dump = JSON.stringify(exit.cause); expect(dump).toContain("LegacySsoUpdateArityError"); expect(dump).toContain("accepts 1 arg(s), received 2"); - expect(dump).not.toContain("LegacySsoWorkdirError"); + expect(dump).not.toContain("LegacyPflagWorkdirError"); } expect(api.requests.length).toBe(0); }).pipe(Effect.provide(layer)); @@ -1792,7 +1792,7 @@ describe("legacy sso update integration", () => { expect(Exit.isFailure(exit)).toBe(true); if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); - expect(dump).toContain("LegacySsoProfileError"); + expect(dump).toContain("LegacyProfileLoadError"); expect(dump).toContain(`failed to read profile: Unsupported Config Type \\"\\"`); } expect(api.requests.length).toBe(0); @@ -1899,7 +1899,7 @@ describe("legacy sso update integration", () => { if (Exit.isFailure(exit)) { const dump = JSON.stringify(exit.cause); expect(dump).toContain("LegacySsoUpdateArityError"); - expect(dump).not.toContain("LegacySsoProfileError"); + expect(dump).not.toContain("LegacyProfileLoadError"); } expect(api.requests.length).toBe(0); }).pipe(Effect.ensuring(restoreEnv), Effect.provide(layer)); diff --git a/apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.ts b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts similarity index 84% rename from apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.ts rename to apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts index 6f8f252600..78c20028a4 100644 --- a/apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.ts +++ b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts @@ -1,13 +1,31 @@ -import { Effect, FileSystem, Option, Path, Result } from "effect"; +import { Data, Effect, FileSystem, Option, Path, Result } from "effect"; -import type { PflagArgvScan } from "../../../shared/cli/cobra-flag-groups.ts"; -import { LegacyProfileFlag, LegacyWorkdirFlag } from "../../../shared/legacy/global-flags.ts"; -import { RuntimeInfo } from "../../../shared/runtime/runtime-info.service.ts"; -import { legacyProfileFilePath } from "../../config/legacy-profile-file.ts"; -import { legacyParseStringSliceFlag } from "../../shared/legacy-string-slice-flag.ts"; -import { legacyValidateWorkdirIsDirectory } from "../../shared/legacy-workdir-validation.ts"; -import { LegacySsoWorkdirError } from "./sso.errors.ts"; -import { legacySsoLoadProfile, type LegacySsoLoadedProfile } from "./sso.load-profile.ts"; +import type { PflagArgvScan } from "../../shared/cli/cobra-flag-groups.ts"; +import { LegacyProfileFlag, LegacyWorkdirFlag } from "../../shared/legacy/global-flags.ts"; +import { RuntimeInfo } from "../../shared/runtime/runtime-info.service.ts"; +import { legacyProfileFilePath } from "../config/legacy-profile-file.ts"; +import { legacyLoadProfile, type LegacyLoadedProfile } from "./legacy-profile-load.ts"; +import { legacyParseStringSliceFlag } from "./legacy-string-slice-flag.ts"; +import { legacyValidateWorkdirIsDirectory } from "./legacy-workdir-validation.ts"; + +/** + * Go's `ChangeWorkDir` (`internal/utils/misc.go:238-257`), run from the root + * `PersistentPreRunE` (`cmd/root.go:104`) — after `ParseFlags` and + * `ValidateArgs`, before `ValidateRequiredFlags`, `ValidateFlagGroups`, and + * `RunE` — so a missing workdir directory aborts with no API call ever made. + * Emulated for the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` the + * Effect layer never validates (and, when `--workdir` consumed a flag-shaped + * token, never even saw — PR #5974 review round 6). Shared across add + + * update; message byte-matches Go's template. + * + * Flows through {@link legacyValidatePflagWorkdir}'s inferred Effect error + * channel; no call site imports the class by name. + * + * @public + */ +export class LegacyPflagWorkdirError extends Data.TaggedError("LegacyPflagWorkdirError")<{ + readonly message: string; +}> {} /** * Reconciles an Effect-parsed option flag with pflag semantics @@ -25,7 +43,7 @@ import { legacySsoLoadProfile, type LegacySsoLoadedProfile } from "./sso.load-pr * never makes. When the scan and the parser agree (every normal invocation), * the scan's value is byte-identical to the parsed one. */ -export function legacySsoPflagStringValue( +export function legacyPflagStringValue( occurrences: ReadonlyMap>, flagName: string, ): Option.Option { @@ -34,7 +52,7 @@ export function legacySsoPflagStringValue( } /** - * Like `legacySsoPflagStringValue`, but for pflag `StringSliceVar` flags: + * Like `legacyPflagStringValue`, but for pflag `StringSliceVar` flags: * every occurrence is CSV-split and accumulated, matching pflag's * `stringSliceValue.Set`. An absent flag reconciles to `[]` even when the * Effect parser produced values (its tokens were consumed by another flag). @@ -44,7 +62,7 @@ export function legacySsoPflagStringValue( * same raw values and rejects the command at parse time before the handler * runs; the fallback just keeps a handler-level disagreement from crashing. */ -export function legacySsoPflagSliceValue( +export function legacyPflagSliceValue( occurrences: ReadonlyMap>, flagName: string, parsedFallback: ReadonlyArray, @@ -82,12 +100,12 @@ export function legacySsoPflagSliceValue( * sso add …`), which cobra's `Find`/`stripFlags` routes to the same * persistent flag. */ -export function legacySsoPflagWorkdirValue( +export function legacyPflagWorkdirValue( scan: Pick, parsedWorkdir: Option.Option, envWorkdir: string | undefined, ): Option.Option { - const scanned = legacySsoPflagStringValue(scan.occurrences, "workdir"); + const scanned = legacyPflagStringValue(scan.occurrences, "workdir"); // Same last-wins order as the profile resolver: post-path occurrence → // pre-path occurrence (pflag parses persistent flags before the command // path and repeats resolve last-wins, while the Effect parser is @@ -115,7 +133,7 @@ export function legacySsoPflagWorkdirValue( /** * Emulates Go's `ChangeWorkDir` (`cmd/root.go:104`, `internal/utils/ - * misc.go:238-257`) for the workdir {@link legacySsoPflagWorkdirValue} + * misc.go:238-257`) for the workdir {@link legacyPflagWorkdirValue} * resolves: `os.Chdir` on a missing path or a non-directory aborts the * command from the root `PersistentPreRunE` — after `ParseFlags` and * `ValidateArgs`, before `ValidateRequiredFlags`, `ValidateFlagGroups`, and @@ -129,19 +147,19 @@ export function legacySsoPflagWorkdirValue( * config layer keeps the workdir it resolved from the parsed flag — both * sides then issue the identical request for these inputs. */ -export const legacySsoValidatePflagWorkdir = Effect.fnUntraced(function* ( +export const legacyValidatePflagWorkdir = Effect.fnUntraced(function* ( scan: Pick, ) { // `serviceOption`: absent outside the real CLI tree (handler-level tests // provide argv via `Stdio.layerTest`, not the global flag settings). const parsedWorkdir = Option.flatten(yield* Effect.serviceOption(LegacyWorkdirFlag)); - const workdir = legacySsoPflagWorkdirValue(scan, parsedWorkdir, process.env["SUPABASE_WORKDIR"]); + const workdir = legacyPflagWorkdirValue(scan, parsedWorkdir, process.env["SUPABASE_WORKDIR"]); if (Option.isNone(workdir)) { return; } const fs = yield* FileSystem.FileSystem; yield* legacyValidateWorkdirIsDirectory(workdir.value, fs).pipe( - Effect.mapError((cause) => new LegacySsoWorkdirError({ message: cause.message })), + Effect.mapError((cause) => new LegacyPflagWorkdirError({ message: cause.message })), ); }); @@ -151,7 +169,7 @@ export const legacySsoValidatePflagWorkdir = Effect.fnUntraced(function* ( * `Option.none` means Go would fall through to the persisted * `~/.supabase/profile` file and then the `supabase` default. * - * Resolution order mirrors {@link legacySsoPflagWorkdirValue} (same viper + * Resolution order mirrors {@link legacyPflagWorkdirValue} (same viper * semantics, binary-verified for `--profile` in PR #5974 review round 7): * - the scan's last `--profile` occurrence wins — pflag consumes flag-shaped * tokens the Effect parser refuses (`--profile --metadata-url` binds @@ -168,12 +186,12 @@ export const legacySsoValidatePflagWorkdir = Effect.fnUntraced(function* ( * flag's default, so that value is treated as unset — the same proxy the * config layer uses (`legacy-cli-config.layer.ts`). */ -export function legacySsoPflagProfileValue( +export function legacyPflagProfileValue( scan: Pick, parsedProfile: Option.Option, envProfile: string | undefined, ): Option.Option { - const scanned = legacySsoPflagStringValue(scan.occurrences, "profile"); + const scanned = legacyPflagStringValue(scan.occurrences, "profile"); // pflag's effective value is the LAST parsed occurrence anywhere in argv: // a post-path occurrence wins outright; otherwise a persistent pre-path // occurrence (`--profile A sso add …`) stays effective even when a later @@ -223,7 +241,7 @@ export function legacySsoPflagProfileValue( * provide argv via `Stdio.layerTest`) the flag settings and `RuntimeInfo` * may be absent; the emulation then only acts on what the scan itself shows. */ -export const legacySsoResolvePflagProfile = Effect.fnUntraced(function* ( +export const legacyResolvePflagProfile = Effect.fnUntraced(function* ( scan: Pick, ) { const parsedRaw = yield* Effect.serviceOption(LegacyProfileFlag); @@ -235,7 +253,7 @@ export const legacySsoResolvePflagProfile = Effect.fnUntraced(function* ( // (`resolveProfile`, `legacy-cli-config.layer.ts`: parsed flag ≠ default → // env). When both agree on a non-empty explicit token, the layer resolved // the exact same profile the Go binary would target. - const goExplicit = legacySsoPflagProfileValue(scan, parsedProfile, envProfile); + const goExplicit = legacyPflagProfileValue(scan, parsedProfile, envProfile); const layerExplicit = Option.isSome(parsedProfile) ? parsedProfile : envProfile !== undefined @@ -247,14 +265,14 @@ export const legacySsoResolvePflagProfile = Effect.fnUntraced(function* ( goExplicit.value === layerExplicit.value && goExplicit.value !== "" ) { - return Option.none(); + return Option.none(); } const fs = yield* Effect.serviceOption(FileSystem.FileSystem); const path = yield* Effect.serviceOption(Path.Path); const runtimeInfo = yield* Effect.serviceOption(RuntimeInfo); if (Option.isNone(fs) || Option.isNone(path) || Option.isNone(runtimeInfo)) { - return Option.none(); + return Option.none(); } // Lowest precedence: the persisted `~/.supabase/profile` file. Go uses the @@ -282,9 +300,9 @@ export const legacySsoResolvePflagProfile = Effect.fnUntraced(function* ( }); if (goToken === layerToken && goToken !== "") { - return Option.none(); + return Option.none(); } - return Option.some(yield* legacySsoLoadProfile(goToken, fs.value)); + return Option.some(yield* legacyLoadProfile(goToken, fs.value)); }); /** Go's `strconv.ParseBool` accepted literals (`strconv/atob.go:10-19`). */ @@ -304,7 +322,7 @@ const GO_PARSE_BOOL: ReadonlyMap = new Map([ ]); /** - * Like `legacySsoPflagStringValue`, but for pflag `BoolVar` flags. pflag + * Like `legacyPflagStringValue`, but for pflag `BoolVar` flags. pflag * calls `Value.Set` for every occurrence in argv order: a bare occurrence * sets `NoOptDefVal` (`"true"`), an inline `=value` goes through * `strconv.ParseBool`, an invalid literal aborts `ParseFlags` with @@ -329,7 +347,7 @@ const GO_PARSE_BOOL: ReadonlyMap = new Map([ * `--skip-url-validation=false --skip-url-validation=` aborts Go's * ParseFlags before any request; the parser accepts the argv). */ -export function legacySsoPflagBoolValue( +export function legacyPflagBoolValue( occurrences: ReadonlyMap>, flagName: string, ): Result.Result { @@ -351,7 +369,7 @@ export function legacySsoPflagBoolValue( } /** - * Like `legacySsoPflagStringValue`, but for Go enum-valued flags + * Like `legacyPflagStringValue`, but for Go enum-valued flags * (`ssoProviderType`, `ssoNameIDFormat` — `cmd/sso.go:157-158,176`), whose * `Value.Set` rejects anything outside the allowed set. pflag Sets every * occurrence in argv order and aborts `ParseFlags` on the first invalid one @@ -362,7 +380,7 @@ export function legacySsoPflagBoolValue( * `flagLabel` is how pflag names the flag in the error: `--name` without a * shorthand, `-s, --name` with one (pflag `errors.go:39-41`). */ -export function legacySsoPflagEnumValue( +export function legacyPflagEnumValue( occurrences: ReadonlyMap>, flagName: string, allowed: ReadonlyArray, diff --git a/apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.unit.test.ts b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.unit.test.ts similarity index 72% rename from apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.unit.test.ts rename to apps/cli/src/legacy/shared/legacy-pflag-reconcile.unit.test.ts index 5a68ffdb71..b05f5561a5 100644 --- a/apps/cli/src/legacy/commands/sso/sso.pflag-reconcile.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.unit.test.ts @@ -2,38 +2,46 @@ import { describe, expect, it } from "@effect/vitest"; import { Option, Result } from "effect"; import { - legacySsoPflagBoolValue, - legacySsoPflagEnumValue, - legacySsoPflagProfileValue, - legacySsoPflagWorkdirValue, -} from "./sso.pflag-reconcile.ts"; -import { LEGACY_SSO_NAME_ID_FORMATS } from "./sso.saml.ts"; + legacyPflagBoolValue, + legacyPflagEnumValue, + legacyPflagProfileValue, + legacyPflagWorkdirValue, +} from "./legacy-pflag-reconcile.ts"; + +// Go's SAML `nameid-format` enum (`cmd/sso.go:157-158,176`), reused here only +// as sample data for the generic enum-reconciliation helper under test. +const NAME_ID_FORMATS = [ + "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", + "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified", + "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", + "urn:oasis:names:tc:SAML:2.0:nameid-format:transient", +] as const; const occ = (entries: ReadonlyArray]>) => new Map(entries.map(([name, values]) => [name, [...values]])); -describe("legacySsoPflagBoolValue", () => { +describe("legacyPflagBoolValue", () => { it("is false when the flag never occurs (Go default)", () => { - expect(legacySsoPflagBoolValue(occ([]), "skip-url-validation")).toEqual(Result.succeed(false)); + expect(legacyPflagBoolValue(occ([]), "skip-url-validation")).toEqual(Result.succeed(false)); }); it('treats a bare occurrence (recorded as pflag\'s NoOptDefVal "true") as true', () => { expect( - legacySsoPflagBoolValue(occ([["skip-url-validation", ["true"]]]), "skip-url-validation"), + legacyPflagBoolValue(occ([["skip-url-validation", ["true"]]]), "skip-url-validation"), ).toEqual(Result.succeed(true)); }); it("resolves repeats last-wins, not first-wins (pflag Sets every occurrence)", () => { // `--skip-url-validation=false --skip-url-validation` — Go ends up true. expect( - legacySsoPflagBoolValue( + legacyPflagBoolValue( occ([["skip-url-validation", ["false", "true"]]]), "skip-url-validation", ), ).toEqual(Result.succeed(true)); // `--skip-url-validation --skip-url-validation=false` — Go ends up false. expect( - legacySsoPflagBoolValue( + legacyPflagBoolValue( occ([["skip-url-validation", ["true", "false"]]]), "skip-url-validation", ), @@ -46,7 +54,7 @@ describe("legacySsoPflagBoolValue", () => { // occurrence, but pflag hands `""` to strconv.ParseBool and aborts // ParseFlags before any request (binary-verified, PR #5974 round 5). expect( - legacySsoPflagBoolValue(occ([["skip-url-validation", ["false", ""]]]), "skip-url-validation"), + legacyPflagBoolValue(occ([["skip-url-validation", ["false", ""]]]), "skip-url-validation"), ).toEqual( Result.fail( `invalid argument "" for "--skip-url-validation" flag: strconv.ParseBool: parsing "": invalid syntax`, @@ -56,17 +64,17 @@ describe("legacySsoPflagBoolValue", () => { it("accepts exactly Go's strconv.ParseBool literal set", () => { for (const raw of ["1", "t", "T", "TRUE", "true", "True"]) { - expect(legacySsoPflagBoolValue(occ([["f", [raw]]]), "f")).toEqual(Result.succeed(true)); + expect(legacyPflagBoolValue(occ([["f", [raw]]]), "f")).toEqual(Result.succeed(true)); } for (const raw of ["0", "f", "F", "FALSE", "false", "False"]) { - expect(legacySsoPflagBoolValue(occ([["f", [raw]]]), "f")).toEqual(Result.succeed(false)); + expect(legacyPflagBoolValue(occ([["f", [raw]]]), "f")).toEqual(Result.succeed(false)); } }); it("fails with pflag's byte-exact invalid-argument message on the first bad occurrence", () => { // The Effect parser accepts `yes`/`no`; Go's strconv.ParseBool does not. expect( - legacySsoPflagBoolValue(occ([["skip-url-validation", ["yes"]]]), "skip-url-validation"), + legacyPflagBoolValue(occ([["skip-url-validation", ["yes"]]]), "skip-url-validation"), ).toEqual( Result.fail( `invalid argument "yes" for "--skip-url-validation" flag: strconv.ParseBool: parsing "yes": invalid syntax`, @@ -74,10 +82,7 @@ describe("legacySsoPflagBoolValue", () => { ); // A later invalid occurrence still fails — pflag Sets each one in order. expect( - legacySsoPflagBoolValue( - occ([["skip-url-validation", ["true", "no"]]]), - "skip-url-validation", - ), + legacyPflagBoolValue(occ([["skip-url-validation", ["true", "no"]]]), "skip-url-validation"), ).toEqual( Result.fail( `invalid argument "no" for "--skip-url-validation" flag: strconv.ParseBool: parsing "no": invalid syntax`, @@ -86,9 +91,9 @@ describe("legacySsoPflagBoolValue", () => { }); }); -describe("legacySsoPflagEnumValue", () => { +describe("legacyPflagEnumValue", () => { it("is none when the flag never occurs", () => { - expect(legacySsoPflagEnumValue(occ([]), "name-id-format", LEGACY_SSO_NAME_ID_FORMATS)).toEqual( + expect(legacyPflagEnumValue(occ([]), "name-id-format", NAME_ID_FORMATS)).toEqual( Result.succeed(Option.none()), ); }); @@ -97,10 +102,10 @@ describe("legacySsoPflagEnumValue", () => { const persistent = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"; const transient = "urn:oasis:names:tc:SAML:2.0:nameid-format:transient"; expect( - legacySsoPflagEnumValue( + legacyPflagEnumValue( occ([["name-id-format", [transient, persistent]]]), "name-id-format", - LEGACY_SSO_NAME_ID_FORMATS, + NAME_ID_FORMATS, ), ).toEqual(Result.succeed(Option.some(persistent))); }); @@ -108,28 +113,28 @@ describe("legacySsoPflagEnumValue", () => { it("fails with the Go enum Set message when any occurrence is invalid", () => { const persistent = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"; expect( - legacySsoPflagEnumValue( + legacyPflagEnumValue( occ([["name-id-format", [persistent, "bogus"]]]), "name-id-format", - LEGACY_SSO_NAME_ID_FORMATS, + NAME_ID_FORMATS, ), ).toEqual( Result.fail( - `invalid argument "bogus" for "--name-id-format" flag: must be one of [ ${LEGACY_SSO_NAME_ID_FORMATS.join(" | ")} ]`, + `invalid argument "bogus" for "--name-id-format" flag: must be one of [ ${NAME_ID_FORMATS.join(" | ")} ]`, ), ); }); it("names the flag with its shorthand when a label is given (pflag errors.go:39-41)", () => { expect( - legacySsoPflagEnumValue(occ([["type", ["bogus"]]]), "type", ["saml"], "-t, --type"), + legacyPflagEnumValue(occ([["type", ["bogus"]]]), "type", ["saml"], "-t, --type"), ).toEqual( Result.fail(`invalid argument "bogus" for "-t, --type" flag: must be one of [ saml ]`), ); }); }); -describe("legacySsoPflagWorkdirValue", () => { +describe("legacyPflagWorkdirValue", () => { const scan = ( entries: ReadonlyArray]>, consumed: ReadonlyArray = [], @@ -146,7 +151,7 @@ describe("legacySsoPflagWorkdirValue", () => { // Effect parser bound the first (pre-path workdir twin of the profile // fix, PR #5974 review round 11). expect( - legacySsoPflagWorkdirValue( + legacyPflagWorkdirValue( scan([], [], [["workdir", ["/existing", "/missing"]]]), Option.some("/existing"), undefined, @@ -156,7 +161,7 @@ describe("legacySsoPflagWorkdirValue", () => { it("keeps a pre-path occurrence when the only post-path workdir token was consumed", () => { expect( - legacySsoPflagWorkdirValue( + legacyPflagWorkdirValue( scan([], ["workdir"], [["workdir", ["/pre"]]]), Option.some("/pre"), "/env", @@ -166,7 +171,7 @@ describe("legacySsoPflagWorkdirValue", () => { it("post-path occurrences still win over pre-path ones (argv-order last-wins)", () => { expect( - legacySsoPflagWorkdirValue( + legacyPflagWorkdirValue( scan([["workdir", ["/post"]]], [], [["workdir", ["/pre"]]]), Option.some("/pre"), undefined, @@ -175,25 +180,25 @@ describe("legacySsoPflagWorkdirValue", () => { }); it("resolves nothing when no flag, parsed value, or env var is present (Go walks up)", () => { - expect(legacySsoPflagWorkdirValue(scan([]), Option.none(), undefined)).toEqual(Option.none()); + expect(legacyPflagWorkdirValue(scan([]), Option.none(), undefined)).toEqual(Option.none()); }); it("prefers the scan's occurrence over the parsed flag and the env var", () => { // `--workdir --metadata-file …`: pflag binds the flag-shaped token; the // Effect parser refused it and left the flag unset (PR #5974 round 6). expect( - legacySsoPflagWorkdirValue(scan([["workdir", ["--metadata-file"]]]), Option.none(), "/env"), + legacyPflagWorkdirValue(scan([["workdir", ["--metadata-file"]]]), Option.none(), "/env"), ).toEqual(Option.some("--metadata-file")); }); it("resolves repeats last-wins, matching pflag StringVar", () => { expect( - legacySsoPflagWorkdirValue(scan([["workdir", ["/a", "/b"]]]), Option.some("/a"), undefined), + legacyPflagWorkdirValue(scan([["workdir", ["/a", "/b"]]]), Option.some("/a"), undefined), ).toEqual(Option.some("/b")); }); it("falls back to the parsed flag when the anchored scan saw no occurrence (pre-path --workdir)", () => { - expect(legacySsoPflagWorkdirValue(scan([]), Option.some("/pre-path"), "/env")).toEqual( + expect(legacyPflagWorkdirValue(scan([]), Option.some("/pre-path"), "/env")).toEqual( Option.some("/pre-path"), ); }); @@ -202,36 +207,34 @@ describe("legacySsoPflagWorkdirValue", () => { // `--domains --workdir /x`: pflag hands `--workdir` to `--domains` and // never marks workdir changed, so viper falls to SUPABASE_WORKDIR // (binary-verified against apps/cli-go, PR #5974 round 6). - expect(legacySsoPflagWorkdirValue(scan([], ["workdir"]), Option.some("/x"), "/env")).toEqual( + expect(legacyPflagWorkdirValue(scan([], ["workdir"]), Option.some("/x"), "/env")).toEqual( Option.some("/env"), ); - expect(legacySsoPflagWorkdirValue(scan([], ["workdir"]), Option.some("/x"), undefined)).toEqual( + expect(legacyPflagWorkdirValue(scan([], ["workdir"]), Option.some("/x"), undefined)).toEqual( Option.none(), ); }); it("uses the env var when neither the scan nor the parser saw the flag", () => { - expect(legacySsoPflagWorkdirValue(scan([]), Option.none(), "/env")).toEqual( - Option.some("/env"), - ); + expect(legacyPflagWorkdirValue(scan([]), Option.none(), "/env")).toEqual(Option.some("/env")); }); it("treats a changed-but-empty flag as the walk-up default, shadowing the env var (viper precedence)", () => { // `--workdir=`: viper returns the changed flag's empty value and Go falls // through to the always-existing project root, never to SUPABASE_WORKDIR // (binary-verified: the command proceeds to POST). - expect(legacySsoPflagWorkdirValue(scan([["workdir", [""]]]), Option.none(), "/env")).toEqual( + expect(legacyPflagWorkdirValue(scan([["workdir", [""]]]), Option.none(), "/env")).toEqual( Option.none(), ); - expect(legacySsoPflagWorkdirValue(scan([]), Option.some(""), "/env")).toEqual(Option.none()); + expect(legacyPflagWorkdirValue(scan([]), Option.some(""), "/env")).toEqual(Option.none()); }); it("treats an empty env var as unset", () => { - expect(legacySsoPflagWorkdirValue(scan([]), Option.none(), "")).toEqual(Option.none()); + expect(legacyPflagWorkdirValue(scan([]), Option.none(), "")).toEqual(Option.none()); }); }); -describe("legacySsoPflagProfileValue", () => { +describe("legacyPflagProfileValue", () => { const scan = ( entries: ReadonlyArray]>, consumed: ReadonlyArray = [], @@ -248,7 +251,7 @@ describe("legacySsoPflagProfileValue", () => { // parsed the consumed token, so A stays effective — falling through to // env/default targeted a host Go never contacts (review r3686720491). expect( - legacySsoPflagProfileValue( + legacyPflagProfileValue( scan([], ["profile"], [["profile", ["a.yml"]]]), Option.some("a.yml"), "env.yml", @@ -258,7 +261,7 @@ describe("legacySsoPflagProfileValue", () => { it("resolves pre-path repeats last-wins, like pflag (the parser is first-wins)", () => { expect( - legacySsoPflagProfileValue( + legacyPflagProfileValue( scan([], [], [["profile", ["a.yml", "b.yml"]]]), Option.some("a.yml"), undefined, @@ -268,7 +271,7 @@ describe("legacySsoPflagProfileValue", () => { it("post-path occurrences still win over pre-path ones (argv-order last-wins)", () => { expect( - legacySsoPflagProfileValue( + legacyPflagProfileValue( scan([["profile", ["post.yml"]]], [], [["profile", ["pre.yml"]]]), Option.some("pre.yml"), undefined, @@ -277,7 +280,7 @@ describe("legacySsoPflagProfileValue", () => { }); it("resolves nothing when no flag, parsed value, or env var is present (Go falls to the file/default)", () => { - expect(legacySsoPflagProfileValue(scan([]), Option.none(), undefined)).toEqual(Option.none()); + expect(legacyPflagProfileValue(scan([]), Option.none(), undefined)).toEqual(Option.none()); }); it("prefers the scan's occurrence over the parsed flag and the env var", () => { @@ -285,13 +288,13 @@ describe("legacySsoPflagProfileValue", () => { // Effect parser refused it and left the flag at its default (PR #5974 // round 7). expect( - legacySsoPflagProfileValue(scan([["profile", ["--metadata-url"]]]), Option.none(), "env.yml"), + legacyPflagProfileValue(scan([["profile", ["--metadata-url"]]]), Option.none(), "env.yml"), ).toEqual(Option.some("--metadata-url")); }); it("resolves repeats last-wins, matching pflag StringVar (the parser is first-wins)", () => { expect( - legacySsoPflagProfileValue( + legacyPflagProfileValue( scan([["profile", ["a.yml", "b.yml"]]]), Option.some("a.yml"), undefined, @@ -304,18 +307,18 @@ describe("legacySsoPflagProfileValue", () => { // cannot see this (its parsed flag can't distinguish default from // explicit), so the scan is authoritative post-command-path. expect( - legacySsoPflagProfileValue(scan([["profile", ["supabase"]]]), Option.none(), "env.yml"), + legacyPflagProfileValue(scan([["profile", ["supabase"]]]), Option.none(), "env.yml"), ).toEqual(Option.some("supabase")); }); it("keeps a changed-but-empty occurrence — Go fails LoadProfile on it, never falling to the env", () => { - expect(legacySsoPflagProfileValue(scan([["profile", [""]]]), Option.none(), "env.yml")).toEqual( + expect(legacyPflagProfileValue(scan([["profile", [""]]]), Option.none(), "env.yml")).toEqual( Option.some(""), ); }); it("falls back to the parsed flag when the anchored scan saw no occurrence (pre-path --profile)", () => { - expect(legacySsoPflagProfileValue(scan([]), Option.some("pre.yml"), "env.yml")).toEqual( + expect(legacyPflagProfileValue(scan([]), Option.some("pre.yml"), "env.yml")).toEqual( Option.some("pre.yml"), ); }); @@ -326,20 +329,20 @@ describe("legacySsoPflagProfileValue", () => { // SUPABASE_PROFILE (binary-verified against apps/cli-go, PR #5974 // round 7 — the demonstrated divergent input). expect( - legacySsoPflagProfileValue(scan([], ["profile"]), Option.some("alternate.yml"), "env.yml"), + legacyPflagProfileValue(scan([], ["profile"]), Option.some("alternate.yml"), "env.yml"), ).toEqual(Option.some("env.yml")); expect( - legacySsoPflagProfileValue(scan([], ["profile"]), Option.some("alternate.yml"), undefined), + legacyPflagProfileValue(scan([], ["profile"]), Option.some("alternate.yml"), undefined), ).toEqual(Option.none()); }); it("uses the env var when neither the scan nor the parser saw the flag", () => { - expect(legacySsoPflagProfileValue(scan([]), Option.none(), "env.yml")).toEqual( + expect(legacyPflagProfileValue(scan([]), Option.none(), "env.yml")).toEqual( Option.some("env.yml"), ); }); it("treats an empty env var as unset", () => { - expect(legacySsoPflagProfileValue(scan([]), Option.none(), "")).toEqual(Option.none()); + expect(legacyPflagProfileValue(scan([]), Option.none(), "")).toEqual(Option.none()); }); }); diff --git a/apps/cli/src/legacy/commands/sso/sso.load-profile.ts b/apps/cli/src/legacy/shared/legacy-profile-load.ts similarity index 92% rename from apps/cli/src/legacy/commands/sso/sso.load-profile.ts rename to apps/cli/src/legacy/shared/legacy-profile-load.ts index 8bd8998ddc..f942414c5b 100644 --- a/apps/cli/src/legacy/commands/sso/sso.load-profile.ts +++ b/apps/cli/src/legacy/shared/legacy-profile-load.ts @@ -1,8 +1,20 @@ -import { Effect, FileSystem } from "effect"; +import { Data, Effect, FileSystem } from "effect"; import { parse as parseYaml } from "yaml"; -import { legacyApiUrl, legacyIsBuiltinProfileName } from "../../shared/legacy-profile.ts"; -import { LegacySsoProfileError } from "./sso.errors.ts"; +import { legacyApiUrl, legacyIsBuiltinProfileName } from "./legacy-profile.ts"; + +// Go's `LoadProfile` (`internal/utils/profile.go:94-118`), run from the root +// `PersistentPreRunE` (`cmd/root.go:98-102`) immediately BEFORE +// `ChangeWorkDir` — so a profile Go cannot load aborts before the workdir +// check, `ValidateRequiredFlags`, `ValidateFlagGroups`, and `RunE`, with no +// API call ever made. Emulated for the pflag/viper-effective `--profile`/ +// `SUPABASE_PROFILE` whenever it differs from the token the Effect config +// layer resolved (PR #5974 review round 7). Shared across add + update; +// message byte-matches Go for the deterministic failure classes (see +// `legacy-profile-load.ts`). +export class LegacyProfileLoadError extends Data.TaggedError("LegacyProfileLoadError")<{ + readonly message: string; +}> {} /** * Emulates Go's `LoadProfile` (`apps/cli-go/internal/utils/profile.go:94-118`) @@ -60,7 +72,7 @@ import { LegacySsoProfileError } from "./sso.errors.ts"; * go-playground/validator with WHATWG `URL` parsing and the validator's own * published regexes. */ -export interface LegacySsoLoadedProfile { +export interface LegacyLoadedProfile { readonly apiUrl: string; /** * Go's `CurrentProfile.Name` — the canonical built-in name (EqualFold @@ -72,10 +84,10 @@ export interface LegacySsoLoadedProfile { readonly name: string; } -export function legacySsoLoadProfile( +export function legacyLoadProfile( token: string, fs: FileSystem.FileSystem, -): Effect.Effect { +): Effect.Effect { return Effect.gen(function* () { // Go: `strings.EqualFold(p.Name, prof)` — the built-in names are all // ASCII lower-case, so folding is plain lower-casing here. @@ -184,7 +196,7 @@ export function legacySsoLoadProfile( }); } -const fail = (message: string) => Effect.fail(new LegacySsoProfileError({ message })); +const fail = (message: string) => Effect.fail(new LegacyProfileLoadError({ message })); const failRead = (detail: string) => fail(`failed to read profile: ${detail}`); diff --git a/apps/cli/src/legacy/commands/sso/sso.load-profile.unit.test.ts b/apps/cli/src/legacy/shared/legacy-profile-load.unit.test.ts similarity index 94% rename from apps/cli/src/legacy/commands/sso/sso.load-profile.unit.test.ts rename to apps/cli/src/legacy/shared/legacy-profile-load.unit.test.ts index 2380672320..56ce21acc5 100644 --- a/apps/cli/src/legacy/commands/sso/sso.load-profile.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-profile-load.unit.test.ts @@ -6,22 +6,25 @@ import { BunServices } from "@effect/platform-bun"; import { afterAll, describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem } from "effect"; -import type { LegacySsoProfileError } from "./sso.errors.ts"; -import { legacyPadGoErrorBlock, legacySsoLoadProfile } from "./sso.load-profile.ts"; +import { + legacyLoadProfile, + legacyPadGoErrorBlock, + type LegacyProfileLoadError, +} from "./legacy-profile-load.ts"; -const tempRoot = mkdtempSync(join(tmpdir(), "supabase-sso-load-profile-")); +const tempRoot = mkdtempSync(join(tmpdir(), "supabase-profile-load-")); afterAll(() => rmSync(tempRoot, { recursive: true, force: true })); const load = (token: string) => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; - return (yield* legacySsoLoadProfile(token, fs)).apiUrl; + return (yield* legacyLoadProfile(token, fs)).apiUrl; }).pipe(Effect.provide(BunServices.layer)); const loadError = (token: string) => load(token).pipe( Effect.flip, - Effect.map((error: LegacySsoProfileError) => error.message), + Effect.map((error: LegacyProfileLoadError) => error.message), ); const writeProfile = (name: string, content: string): string => { @@ -30,7 +33,7 @@ const writeProfile = (name: string, content: string): string => { return filePath; }; -describe("legacySsoLoadProfile", () => { +describe("legacyLoadProfile", () => { it.effect("resolves built-in profile names case-insensitively (Go strings.EqualFold)", () => Effect.gen(function* () { // Binary-verified: `--profile SUPABASE-LOCAL` targets localhost:8080. @@ -117,7 +120,7 @@ describe("legacySsoLoadProfile", () => { "Project_Host: supabase.co", ].join("\n"), ); - const profile = yield* legacySsoLoadProfile(file, fs); + const profile = yield* legacyLoadProfile(file, fs); expect(profile.apiUrl).toBe("http://127.0.0.1:44444"); expect(profile.name).toBe("harness"); }).pipe(Effect.provide(BunServices.layer)), @@ -146,7 +149,7 @@ describe("legacySsoLoadProfile", () => { const fs = yield* FileSystem.FileSystem; // Built-in: EqualFold match resolves to the canonical (lower-case) // table name — the keyring account Go reads (`access_token.go:43`). - expect((yield* legacySsoLoadProfile("SUPABASE-LOCAL", fs)).name).toBe("supabase-local"); + expect((yield* legacyLoadProfile("SUPABASE-LOCAL", fs)).name).toBe("supabase-local"); // File profile: `UnmarshalExact` populates Name from the required // `name:` key, NOT from the file path. const file = writeProfile( @@ -158,7 +161,7 @@ describe("legacySsoLoadProfile", () => { "project_host: supabase.co", ].join("\n"), ); - expect((yield* legacySsoLoadProfile(file, fs)).name).toBe("harness"); + expect((yield* legacyLoadProfile(file, fs)).name).toBe("harness"); }).pipe(Effect.provide(BunServices.layer)), ); From 762994941a306fe390ed34705eef01ea439b6bf3 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 3 Aug 2026 14:00:10 +0100 Subject: [PATCH 2/3] style(cli): apply oxfmt table padding after develop merge --- apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md | 4 ++-- apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md index 68fae87049..6836608123 100644 --- a/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/sso/add/SIDE_EFFECTS.md @@ -46,8 +46,8 @@ same shape via an inline anonymous struct with `Default *any`. | `1` | `LegacySsoInvalidFlagValueError` — a `--type`/`--skip-url-validation`/`--name-id-format` occurrence pflag's `Value.Set` would reject (enum membership / `strconv.ParseBool`; fails before every validation; no request) | | `1` | malformed CSV in a `--domains` value — fails during flag parsing, before the handler and telemetry, with pflag's exact diagnostic on stderr (e.g. `invalid argument "a\"b" for "--domains" flag: parse error on line 1, column 2: bare " in non-quoted-field`; a blank-only value fails with `EOF`) — CLI-2005 | | `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before every validation; no request) | -| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; beats the workdir, required-flag, and mutex checks; no request) | -| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; beats the required-flag and mutex checks; no request) | +| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; beats the workdir, required-flag, and mutex checks; no request) | +| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; beats the required-flag and mutex checks; no request) | | `1` | `LegacySsoAddRequiredFlagError` — pflag consumed the `--type`/`-t` token as another flag's value (cobra `ValidateRequiredFlags`) | | `1` | `LegacySsoMutexFlagError` — `--metadata-file` and `--metadata-url` both set | | `1` | `LegacySsoAddMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | diff --git a/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md index 11cb2ccbe9..a5405e8e59 100644 --- a/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md @@ -48,8 +48,8 @@ GET still uses the typed client. | `1` | malformed CSV in a `--domains`/`--add-domains`/`--remove-domains` value — fails during flag parsing, before the handler and telemetry, with pflag's exact diagnostic on stderr (e.g. `invalid argument "a\"b" for "--domains" flag: parse error on line 1, column 2: bare " in non-quoted-field`; a blank-only value fails with `EOF`) — CLI-2005 | | `1` | `LegacySsoFlagNeedsArgumentError` — a bare value-taking flag is the final argv token (pflag `ValueRequiredError`, fails before `ValidateArgs`; no request) | | `1` | `LegacySsoUpdateArityError` — pflag-effective positional count ≠ 1 (cobra `ValidateArgs`/`ExactArgs(1)`; a consumed flag token orphans its parser-value into the positionals) | -| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; loses to the arity check, beats the workdir and mutex checks; no request) | -| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; loses to the arity check, beats the mutex checks; no request) | +| `1` | `LegacyProfileLoadError` — the pflag/viper-effective `--profile`/`SUPABASE_PROFILE` cannot be loaded the way Go's `LoadProfile` loads it (root `PersistentPreRunE`, before `ChangeWorkDir`; loses to the arity check, beats the workdir and mutex checks; no request) | +| `1` | `LegacyPflagWorkdirError` — the pflag/viper-effective `--workdir`/`SUPABASE_WORKDIR` is not an existing directory (Go `ChangeWorkDir` in root `PersistentPreRunE`; loses to the arity check, beats the mutex checks; no request) | | `1` | `LegacySsoInvalidUuidError` — provider ID is not a canonical UUID | | `1` | `LegacySsoMutexFlagError` — flag combinations: `--domains` with `--add/--remove-domains`, or `--metadata-file` with `--metadata-url` | | `1` | `LegacySsoUpdateMetadataFileError` — metadata file unreadable, non-UTF-8, or metadata URL invalid/unreachable/non-UTF-8 | From d0fde74e7b07160bc4d616f4e01bbe7d892c8cb1 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 3 Aug 2026 14:06:36 +0100 Subject: [PATCH 3/3] docs(cli): note why pflag reconciliation was hoisted ahead of a 2nd consumer (review: sso reconciliation ownership) Codex flagged that legacy-pflag-reconcile.ts/legacy-profile-load.ts have only two current consumers (sso add + update), which by AGENTS.md's hoist rule reads as "family root" tier, not shared/. The hoist is intentional: a human reviewer asked for exactly this in #5974's review, on the grounds that the pflag-vs-Effect-parser divergence is CLI-wide, not sso-specific. Recording that link in the source so future readers don't re-litigate it. --- apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts index 78c20028a4..e601088a29 100644 --- a/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts +++ b/apps/cli/src/legacy/shared/legacy-pflag-reconcile.ts @@ -8,6 +8,14 @@ import { legacyLoadProfile, type LegacyLoadedProfile } from "./legacy-profile-lo import { legacyParseStringSliceFlag } from "./legacy-string-slice-flag.ts"; import { legacyValidateWorkdirIsDirectory } from "./legacy-workdir-validation.ts"; +/** + * Hoisted here ahead of a second command family landing on purpose: a human + * reviewer flagged in #5974 that the pflag-vs-Effect-parser divergence this + * module reconciles is CLI-wide, not sso-specific, and asked for it to live + * in a shared layer rather than be reimplemented per command family — + * https://github.com/supabase/cli/pull/5974#discussion_r3685149895 (CLI-1982). + */ + /** * Go's `ChangeWorkDir` (`internal/utils/misc.go:238-257`), run from the root * `PersistentPreRunE` (`cmd/root.go:104`) — after `ParseFlags` and