feat(cli): port sso commands to native TypeScript#5375
Merged
Conversation
jgoux
approved these changes
May 28, 2026
… codegen
The OpenAPI → Effect Schema generator's `sanitizeOpenApiSchema` was stripping
any key named `default` to drop JSON Schema documentation annotations, but it
applied the rule recursively without distinguishing schema-keyword position
from property-name position. As a result, OpenAPI schemas of the form
{ properties: { default: { oneOf: [...] } } }
emitted a TypeScript schema with the `default` property silently omitted.
This bit the SAML SSO attribute mapping contracts — each
`attribute_mapping.keys.<name>` has `name?`, `names?`, `array?`, and
`default?: any` per OpenAPI spec, and the `default` field was being dropped
on encode (POST/PUT) and decode (GET/DELETE/list responses), causing silent
data loss for callers that set `default` values.
Fix tracks whether the recursion is currently inside a `properties: {...}`
map; in that position keys are user-defined property names and we leave them
untouched. Outside that position they're schema metadata keywords and are
stripped as before. Regenerated `contracts.ts` now includes the `default`
field in all five SSO attribute-mapping schemas.
Replaces the six Phase 0 Go-binary proxies (`sso list`, `add`, `remove`,
`update`, `show`, `info`) with native Effect implementations that match Go
byte-for-byte: error strings, exit codes, output bytes (text + Go
`--output {pretty,json,yaml,toml,env}` + TS `--output-format`), telemetry
events (`cli_command_executed`, `cli_upgrade_suggested`), filesystem side
effects (telemetry.json, linked-project.json), and the request/response
contract with the Management API.
Adds:
- `legacy-upgrade-suggested.ts` — reusable helper for the
`cli_upgrade_suggested` telemetry event (will be reused by branches/*).
- `legacy-resolve-token.ts` — shared `LegacyCliConfig.accessToken →
LegacyCredentials.getAccessToken` fallback for the raw-HTTP code paths
(`sso add` / `sso update`).
- `formatSsoMetadataXml` — Go-xmlfmt v1.1.3 port, verified byte-for-byte
against `xmlfmt.FormatXML(..., " ", " ")` for the
`## SAML 2.0 Metadata XML` fenced block.
- `MissingOption` / `ShowHelp` normalisation in `normalize-error.ts` —
surfaces Cobra-format `Error: required flag(s) "X" not set` from Effect
CLI's parser failures.
Defence-in-depth hardenings over the Go original for the user-supplied
metadata URL fetch:
- Refuses 3xx redirects so HTTPS-only enforcement can't be sidestepped
via redirect to `http://169.254.169.254/` etc.
- Caps response body at 5 MiB before buffering.
- Strict-UTF-8 decode validation.
Raw-HTTP error bodies in `sso add` / `sso update` flow through the same
`sanitizeLegacyErrorBody` (cap + control-char strip) that the typed-client
error mapper applies, and use `HttpClientRequest.bearerToken(redacted)` so
the Authorization header preserves redaction across any future debug
serialisation.
Test coverage: 28 unit tests (format, saml, upgrade-suggested), 90+
integration tests achieving the required 100% branch coverage, and 5 e2e
tests covering the golden paths that don't need a live API (info JSON,
UUID validation negatives across show/update/remove).
Flips `apps/cli/docs/go-cli-porting-status.md` rows 238–243 from `wrapped`
to `ported`.
…nfo trailing space Two cli-e2e parity failures, both with the same root cause: the in-process test runtime exposes services more flatly than production, so divergences between the two only surface in the bundled-binary parity harness. 1. `sso add` / `sso update` panicked with `Service not found: supabase/legacy/CliConfig` because the production `legacyManagementApiRuntimeLayer` only consumed `LegacyCliConfig` *inside* `legacyPlatformApiStack` and the project-ref/linked-project sub-layers via `Layer.provide`. No previously-ported handler yielded `LegacyCliConfig` directly, so the missing top-level entry never mattered — until these two handlers needed `apiUrl` and `userAgent` to build raw HTTP requests. Expose `legacyCliConfigLayer` at the top of the merge. 2. `sso info` produced two spaces between the `Single sign-on URL (ACS URL)` label and the column separator. The label was given a trailing space to byte-match Go's markdown *source* — but Go renders that source through Glamour which collapses the trailing space when computing column widths. Our flat ASCII renderer kept it, doubling against the cell padding. Drop the trailing space so the output matches Go's rendered bytes (which is what the cli-e2e harness compares). To tighten the verification gap so this class of bug doesn't slip through in-process tests again: - Add a compile-time service-coverage assertion to `legacy-management-api-runtime.layer.ts`. A new `LegacyManagementApiServices` union enumerates every service a Management-API handler is allowed to yield; `Layer.mergeAll(...)` is assigned to `Layer<LegacyManagementApiServices, …>` inline. Verified the assertion bites by temporarily commenting out the newly-added `legacyCliConfigLayer` — it produced a compile error rather than the runtime panic. Handlers that need a new top-level service must add it to the union AND to the merge. - Add an exact-byte parity-pin assertion in `info.integration.test.ts` for the ACS URL row (`label | url`, single space) so a future revert to the trailing-space approach fails in-process rather than only in cli-e2e. Verified end-to-end with the bundled binary: - `sso add` mutex check now surfaces correctly (was crashing on DI). - `sso add --metadata-url http://…` emits Go-parity HTTPS error. - `sso update --domains x --add-domains y` mutex check fires. - `sso info` renders single-space between label and pipe.
…ode in upgrade-gate
Two remaining cli-e2e parity gaps:
1. POST /add and PUT /update bodies sent in TS insertion order
(`{type, metadata_url, domains}`) rather than the alphabetical order
Go's generated struct produces (`{domains, metadata_url, type}`). The
cli-e2e replay server compares request bodies via
`JSON.stringify`-based string equality and only canonicalises top-level
arrays — so key order matters for object bodies. Add
`encodeGoStructJsonBody` (sorts keys deeply, no indent, no trailing
newline) and use it on the raw-HTTP request body in both handlers.
2. `suggestUpgradeOnError` skipped the entitlements call against the
cli-e2e replay fixtures because the typed `api.v1.getProject` strict-
decodes the response (`ref: isMinLength(20)`) and the fixtures embed
the literal `__PROJECT_REF__` placeholder (15 chars) in response
bodies. `Effect.option` then swallowed the decode failure and the
handler bailed before the entitlements GET, breaking parity with Go's
request log. Same root cause that drove
`legacy-linked-project-cache.layer.ts` to use raw HTTP — apply the
same fix here.
Rewrites `suggestUpgradeOnError` to use raw HttpClient (with the
shared `resolveLegacyAccessToken` helper) and permissive
JSON-from-`unknown` parsing. New service requirements:
`HttpClient.HttpClient | LegacyCliConfig | LegacyCredentials | Analytics`
— all already exposed by `legacyManagementApiRuntimeLayer` (and
guarded by the compile-time `LegacyManagementApiServices` check
added in the previous commit).
Unit tests updated to mock the raw-HTTP surface instead of the typed
client. Added regression test: response body with literal
`__PROJECT_REF__` placeholder must still flow through to a captured
telemetry event.
Verified locally: full `sso.e2e.test.ts` cli-e2e suite (52 tests)
passes against `CLI_HARNESS_TARGET=ts-legacy`.
…-HTTP fix After rebasing onto develop, the branches port (#5374) had already landed a shared `legacy/shared/legacy-upgrade-suggest.ts` helper covering the same `cli_upgrade_suggested` telemetry path my SSO commit duplicated. Reconcile to use the develop-side helper (`legacySuggestUpgrade` with an opts-object argument shape) and delete the SSO-only `legacy/telemetry/legacy-upgrade-suggested.ts` + test. Three pieces had to merge: 1. The shared helper's *behaviour* now writes the bold billing-URL suggestion to stderr in text mode (Go's `CmdSuggestion` parity) on top of firing the telemetry event. SSO inherits this for free — my prior helper only emitted the event and was missing this Go-parity side-effect. 2. The helper now uses raw `HttpClient` for the project + entitlements GETs (my fix from a few commits ago). Develop's version went through the typed `api.v1.getProject` which strict-decodes — the cli-e2e replay fixtures embed the literal 15-char `__PROJECT_REF__` placeholder in response bodies, failing `ref: isMinLength(20)`. With `Effect.option` swallowing the decode error, the entitlements GET would silently be skipped and the test would never observe the gated path. Same workaround used by `legacy-linked-project-cache.layer.ts`. 3. `branches/update`'s upgrade-gate integration test was built around `mockLegacyPlatformApiService` (typed-client mock) — its assertion pattern `apiMock.requests.find(r => r.method === "getProject")` no longer applies now that the helper bypasses the typed client. Rewrote that one test to use `mockLegacyPlatformApi` with a handler that routes the three URLs (`PATCH /v1/branches/<id>`, `GET /v1/projects/<ref>`, `GET /v1/organizations/<slug>/entitlements`) and assert against the recorded request URLs. As a bonus the rewrite verifies the stderr suggestion now emits ("Upgrade your plan:"), which the previous test couldn't reach. Six SSO handler call sites updated to the opts-object signature. SSO integration tests updated to use `mockAnalytics` from `tests/helpers/mocks.ts` (the existing shared helper) instead of the parallel `mockLegacyAnalytics` I had added — same shape, cleaner. Verified locally: - `nx run-many ...` (types + lint + fmt + knip) green. - `bun run test:core`: 1127 in-process tests pass. - Full `sso.e2e.test.ts` cli-e2e suite (52 tests) passes against `CLI_HARNESS_TARGET=ts-legacy`. - `legacy-upgrade-suggest.unit.test.ts` (8 tests, develop-originated) still passes.
Coly010
force-pushed
the
cli/port-sso-commands
branch
from
deaf696 to
738dd33
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the six Phase 0 Go-binary proxies (
sso list,add,remove,update,show,info) with native Effect implementations and flips them toportedinapps/cli/docs/go-cli-porting-status.md.The port matches Go byte-for-byte on: error strings, exit codes, text output (Glamour AsciiStyle tables +
## SAML 2.0 Metadata XMLpretty-print via a go-xmlfmt v1.1.3 port), Go-flavoured--output {pretty,json,yaml,toml,env}, TS-flavoured--output-format {text,json,stream-json}, telemetry (cli_command_executedpluscli_upgrade_suggestedon 4xx/gated forlist/add/update/remove, intentionally omitted forshow/infoper Go), filesystem side effects (~/.supabase/telemetry.json,<workdir>/supabase/.temp/linked-project.json), and the Management API request/response contract.Notable changes
fix(api):first commit repairs an unrelated codegen bug in@supabase/api: the OpenAPI sanitiser was stripping any property literally nameddefault— including the SAMLattribute_mapping.keys.<name>.defaultfield. Regeneratedcontracts.tsnow preserves that field across all five SSO schemas. Without this fix,sso show/sso removewould silently drop user-supplieddefaultvalues on read.--metadata-urlfetch: refuse 3xx redirects (so HTTPS-only can't be bypassed byhttps://… → http://169.254.169.254/), cap response body at 5 MiB, strict-UTF-8 decode.branches/*):legacy/telemetry/legacy-upgrade-suggested.ts—cli_upgrade_suggestedevent helper.legacy/shared/legacy-resolve-token.ts— env-vs-credentials token fallback used by raw-HTTP handlers.legacy-http-errors.tsexportssanitizeLegacyErrorBodyso the raw-HTTP paths insso add/sso updateget the same body-cap + control-char strip the typed-client error mapper already applies.shared/output/normalize-error.tsnow unwraps Effect CLI'sShowHelpenvelope and mapsMissingOptionto Cobra-formatError: required flag(s) "X" not set— so scripts grepping for that exact Go string keep working.Test coverage
sso.format,sso.saml,legacy-upgrade-suggested— including a verified-against-Go SAML fragment forformatSsoMetadataXmland a parity-guard test pinning the intentional Go bug where theUPDATED AT (UTC)row renderscreated_at.apps/cli/CLAUDE.md), exercising every output format, error branch, telemetry path, and the upgrade-gate side-call.Reviewer notes
## Attribute Mapping/## SAML 2.0 Metadata XMLsection headings + fences are emitted as plain markdown — Glamour AsciiStyle equivalence for arbitrary markdown would require a full markdown renderer. The XML body inside the fence is byte-parity viaformatSsoMetadataXml. Section table data is byte-parity.--helpblock to stdout before a missing-required-flag error; Cobra doesn't. The error string itself is parity (Cobra wording) but the surrounding usage dump would require forking Effect CLI to suppress. Documented inadd/SIDE_EFFECTS.md.sso addandsso updatedeliberately POST/PUT via the rawHttpClientrather than the typed client. The plan was written before the codegen fix in the first commit; switching to typed now is a low-risk follow-up but kept out of scope to limit blast radius.Fixes CLI-1292