fix(cli): always send domains in sso update PUT body to match Go (CLI-1981)

apps/cli/src/legacy/commands/sso/update/SIDE_EFFECTS.md

@@ -20,13 +20,13 @@
 
 ## API Routes
 
-| Method | Path                                                         | Auth         | Request body                                                                    | Response (used fields)                                             |
-| ------ | ------------------------------------------------------------ | ------------ | ------------------------------------------------------------------------------- | ------------------------------------------------------------------ |
-| `GET`  | `/v1/projects/{ref}/config/auth/sso/providers/{provider_id}` | Bearer token | none                                                                            | `{id, saml?, domains?, created_at?, updated_at?}`                  |
-| `PUT`  | `/v1/projects/{ref}/config/auth/sso/providers/{provider_id}` | Bearer token | `{metadata_xml?, metadata_url?, domains?, attribute_mapping?, name_id_format?}` | `{id, saml?, domains?, ...}` (parsed loosely)                      |
-| `GET`  | `<metadata-url>`                                             | none         | `Accept: application/xml`, 10s timeout                                          | XML body (UTF-8) — validation when `--skip-url-validation` not set |
-| `GET`  | `/v1/projects/{ref}`                                         | Bearer token | none                                                                            | `{organization_slug}` — upgrade-gate side-call on 4xx              |
-| `GET`  | `/v1/organizations/{slug}/entitlements`                      | Bearer token | none                                                                            | `{entitlements[].feature.key, .hasAccess}` — upgrade-gate          |
+| Method | Path                                                         | Auth         | Request body                                                                   | Response (used fields)                                             |
+| ------ | ------------------------------------------------------------ | ------------ | ------------------------------------------------------------------------------ | ------------------------------------------------------------------ |
+| `GET`  | `/v1/projects/{ref}/config/auth/sso/providers/{provider_id}` | Bearer token | none                                                                           | `{id, saml?, domains?, created_at?, updated_at?}`                  |
+| `PUT`  | `/v1/projects/{ref}/config/auth/sso/providers/{provider_id}` | Bearer token | `{metadata_xml?, metadata_url?, domains, attribute_mapping?, name_id_format?}` | `{id, saml?, domains?, ...}` (parsed loosely)                      |
+| `GET`  | `<metadata-url>`                                             | none         | `Accept: application/xml`, 10s timeout                                         | XML body (UTF-8) — validation when `--skip-url-validation` not set |
+| `GET`  | `/v1/projects/{ref}`                                         | Bearer token | none                                                                           | `{organization_slug}` — upgrade-gate side-call on 4xx              |
+| `GET`  | `/v1/organizations/{slug}/entitlements`                      | Bearer token | none                                                                           | `{entitlements[].feature.key, .hasAccess}` — upgrade-gate          |
 
 Bypasses the typed Management API client for the PUT so user-supplied keys inside
 `attribute_mapping.keys.<x>` (e.g. `default`) are preserved verbatim. The initial
@@ -83,6 +83,7 @@ Single `success` event with the parsed response as data.
 - `--metadata-file` and `--metadata-url` are mutually exclusive.
 - 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.
+- **`domains` is always present in the PUT body** (CLI-1981): Go's `--add-domains`/`--remove-domains` default to a non-nil `[]string{}` (`cmd/sso.go:171-172`), so `update.go:84`'s `!= nil` merge gate is always true from the CLI. With no domain flags (or an explicit empty `--domains=`) the body carries the recomputed existing set; when the provider has no domains it is the literal `"domains":[]` (non-nil `*[]string` under `omitempty` — verified by live capture against the Go binary).
 - Metadata URL validation error message: `only HTTPS Metadata URLs are supported Use --skip-url-validation to suppress this error.` (single trailing period — matches Go's `update.go:69`, which wraps with `%w Use --skip-url-validation to suppress this error.`; differs from `sso add`'s variant which omits the trailing period).
 - The `## Attribute Mapping` / `## SAML 2.0 Metadata XML` sections are emitted as plain markdown (heading + fence). Visual styling of the headings does not match Go's Glamour-rendered output; the table portion and the XML body inside the fence are byte-parity (see `formatSsoMetadataXml`).
 - **PUT failure message reuses the GET error string**: a non-2xx PUT response produces `unexpected error fetching identity provider: <body>` — note "fetching" not "updating". This matches Go's `update.go:133` verbatim.

apps/cli/src/legacy/commands/sso/update/update.handler.ts

@@ -115,13 +115,15 @@ function mergeDomains(
   add: ReadonlyArray<string>,
   remove: ReadonlyArray<string>,
 ): ReadonlyArray<string> {
-  // Mirrors Go's `update.go:93-117` — seed from current domains, apply
+  // Mirrors Go's `update.go:84-109` — seed from current domains, apply
   // removals, then add new entries. Go uses a `map[string]bool`, so iteration
-  // order is unspecified; integration tests sort before asserting.
+  // order is unspecified; integration tests sort before asserting. Go's seed
+  // check is nil-ness only (`domain.Domain != nil`), so an empty-string domain
+  // from the GET response is kept, not filtered.
   const set = new Set<string>();
   if (existing !== undefined) {
     for (const item of existing) {
-      if (typeof item.domain === "string" && item.domain.length > 0) {
+      if (typeof item.domain === "string") {
         set.add(item.domain);
       }
     }
@@ -228,7 +230,16 @@ export const legacySsoUpdate = Effect.fn("legacy.sso.update")(function* (
 
       if (flags.domains.length > 0) {
         body["domains"] = [...flags.domains];
-      } else if (flags.addDomains.length > 0 || flags.removeDomains.length > 0) {
+      } else {
+        // Go's `update.go:84` reads as gating the merge on
+        // `params.AddDomains != nil || params.RemoveDomains != nil`, but
+        // `cmd/sso.go:171-172` declares both flags with a non-nil `[]string{}`
+        // default and passes them unconditionally — so from the CLI that
+        // condition is always true and every `sso update` PUT recomputes and
+        // sends `domains`, even when no domain flag was passed. `body.Domains`
+        // is a non-nil `*[]string` under `json:"domains,omitempty"`, so an
+        // empty merged set serializes as `"domains":[]`, never omitted
+        // (CLI-1981; live-captured against the Go binary).
         body["domains"] = mergeDomains(existing.domains, flags.addDomains, flags.removeDomains);
       }
 

apps/cli/src/legacy/commands/sso/update/update.integration.test.ts

@@ -476,12 +476,84 @@ describe("legacy sso update integration", () => {
     }).pipe(Effect.provide(layer));
   });
 
-  it.live("no domain flag set → body.domains omitted", () => {
+  it.live("no domain flag set → PUT still sends the recomputed existing domain set", () => {
+    // Go's `--add-domains`/`--remove-domains` default to a non-nil `[]string{}`
+    // (`cmd/sso.go:171-172`), so `update.go:84`'s `!= nil` gate is always true
+    // from the CLI — every `sso update` enters the merge and sends `domains`,
+    // even when no domain flag was passed (CLI-1981). Live-captured Go PUT:
+    // `{"domains":["old1.com","old2.com"]}`.
     const { layer, api } = setup();
     return Effect.gen(function* () {
       yield* legacySsoUpdate(defaultFlags);
       const putReq = api.requests.find((r) => r.method === "PUT");
-      expect((putReq?.body as { domains?: string[] })?.domains).toBeUndefined();
+      const domains = (putReq?.body as { domains: string[] })?.domains;
+      // Go map iteration is unordered — sort before asserting.
+      expect([...domains].sort()).toEqual(["old1.com", "old2.com"]);
+    }).pipe(Effect.provide(layer));
+  });
+
+  it.live(
+    "no domain flags + provider with no domains → PUT sends domains: [] (not omitted)",
+    () => {
+      // Go sets `body.Domains` to a non-nil pointer to `make([]string, 0)`, and
+      // `json:"domains,omitempty"` never omits a non-nil pointer — live-captured
+      // Go PUT body is exactly `{"domains":[]}`.
+      const { layer, api } = setup({ getBody: { ...EXISTING_PROVIDER, domains: [] } });
+      return Effect.gen(function* () {
+        yield* legacySsoUpdate(defaultFlags);
+        const putReq = api.requests.find((r) => r.method === "PUT");
+        const body = putReq?.body as Record<string, unknown>;
+        expect(Object.keys(body)).toContain("domains");
+        expect(body["domains"]).toEqual([]);
+      }).pipe(Effect.provide(layer));
+    },
+  );
+
+  it.live("no domain flags + GET response missing domains entirely → PUT sends domains: []", () => {
+    // Go's seed loop is skipped when `getResp.JSON200.Domains == nil`, leaving
+    // the merged set empty — same `{"domains":[]}` bytes as the empty-list case.
+    const { domains: _omitted, ...providerWithoutDomains } = EXISTING_PROVIDER;
+    const { layer, api } = setup({ getBody: providerWithoutDomains });
+    return Effect.gen(function* () {
+      yield* legacySsoUpdate(defaultFlags);
+      const putReq = api.requests.find((r) => r.method === "PUT");
+      const body = putReq?.body as Record<string, unknown>;
+      expect(Object.keys(body)).toContain("domains");
+      expect(body["domains"]).toEqual([]);
+    }).pipe(Effect.provide(layer));
+  });
+
+  it.live("explicit empty --domains= falls into the merge and resends the existing set", () => {
+    // `--domains=` parses to an empty slice, so Go's `len(params.Domains) != 0`
+    // replace gate is false and the merge branch runs with no add/remove —
+    // live-captured Go PUT resends the existing domains, it does NOT replace
+    // them with an empty list.
+    const { layer, api } = setup({
+      cliArgs: ["sso", "update", VALID_PROVIDER_ID, "--domains="],
+    });
+    return Effect.gen(function* () {
+      yield* legacySsoUpdate({ ...defaultFlags, domains: [] });
+      const putReq = api.requests.find((r) => r.method === "PUT");
+      const domains = (putReq?.body as { domains: string[] })?.domains;
+      expect([...domains].sort()).toEqual(["old1.com", "old2.com"]);
+    }).pipe(Effect.provide(layer));
+  });
+
+  it.live("merge keeps empty-string domains and skips entries without a domain field", () => {
+    // Go's seed check is nil-ness only (`domain.Domain != nil`,
+    // `update.go:89`): an empty-string domain from the GET response stays in
+    // the merged set, while an entry missing the field entirely is skipped.
+    const { layer, api } = setup({
+      getBody: {
+        ...EXISTING_PROVIDER,
+        domains: [{ id: "d1", domain: "" }, { id: "d2", domain: "old1.com" }, { id: "d3" }],
+      },
+    });
+    return Effect.gen(function* () {
+      yield* legacySsoUpdate(defaultFlags);
+      const putReq = api.requests.find((r) => r.method === "PUT");
+      const domains = (putReq?.body as { domains: string[] })?.domains;
+      expect([...domains].sort()).toEqual(["", "old1.com"]);
     }).pipe(Effect.provide(layer));
   });