From fa900358f1ba9215107bb3ce51f12bdc1db0639e Mon Sep 17 00:00:00 2001 From: Daniel Moerner Date: Tue, 21 Apr 2026 16:47:25 -0400 Subject: [PATCH 1/5] fix(config): Run server-side dry-run When we implemented config diffing we started doing the diffing client-side. I think this is still generally the correct behavior. However, we should also run and display the server-side dry-run on a dry-run, since this actually runs the validation before a change. --- .changeset/orgs-diffing-quirks.md | 5 ++ .../cli-core/src/commands/config/README.md | 16 +++--- .../cli-core/src/commands/config/push.test.ts | 53 ++++++++++++++++--- packages/cli-core/src/commands/config/push.ts | 30 ++++++----- packages/cli-core/src/lib/plapi.ts | 9 ++-- .../src/test/integration/config-put.test.ts | 17 +++--- .../src/test/integration/dry-run.test.ts | 7 +-- 7 files changed, 97 insertions(+), 40 deletions(-) create mode 100644 .changeset/orgs-diffing-quirks.md diff --git a/.changeset/orgs-diffing-quirks.md b/.changeset/orgs-diffing-quirks.md new file mode 100644 index 00000000..903e7bbe --- /dev/null +++ b/.changeset/orgs-diffing-quirks.md @@ -0,0 +1,5 @@ +--- +"clerk": patch +--- + +Run `config patch --dry-run` and `config put --dry-run` against the server so validation errors are caught and the projected configuration (including any server-applied defaults) is returned before changes are committed. diff --git a/packages/cli-core/src/commands/config/README.md b/packages/cli-core/src/commands/config/README.md index c64995a4..ed5c4b9f 100644 --- a/packages/cli-core/src/commands/config/README.md +++ b/packages/cli-core/src/commands/config/README.md @@ -100,7 +100,7 @@ clerk config patch --file partial-config.json --dry-run | `--instance ` | Instance to target (`dev`, `prod`, or a full instance ID). Defaults to development. | | `--file ` | Read config JSON from a file | | `--json ` | Pass config JSON inline (takes priority over `--file`) | -| `--dry-run` | Show what would be sent without making the API call | +| `--dry-run` | Validate server-side and preview the projected result without persisting changes | | `--yes` | Skip confirmation prompts | #### Requirements @@ -112,9 +112,9 @@ clerk config patch --file partial-config.json --dry-run #### API Endpoints -| Method | Endpoint | Description | -| ------- | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | -| `PATCH` | `/v1/platform/applications/{appID}/instances/{instanceID}/config` | Partially updates instance configuration. Authenticated via `Bearer` token from `CLERK_PLATFORM_API_KEY`. | +| Method | Endpoint | Description | +| ------- | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `PATCH` | `/v1/platform/applications/{appID}/instances/{instanceID}/config` | Partially updates instance configuration. Sends `?dry_run=true` under `--dry-run` to validate and preview without persisting. Authenticated via `Bearer` token from `CLERK_PLATFORM_API_KEY`. | --- @@ -140,7 +140,7 @@ clerk config put --file full-config.json --dry-run | `--instance ` | Instance to target (`dev`, `prod`, or a full instance ID). Defaults to development. | | `--file ` | Read config JSON from a file | | `--json ` | Pass config JSON inline (takes priority over `--file`) | -| `--dry-run` | Show what would be sent without making the API call | +| `--dry-run` | Validate server-side and preview the projected result without persisting changes | | `--yes` | Skip confirmation prompts | #### Requirements @@ -152,6 +152,6 @@ clerk config put --file full-config.json --dry-run #### API Endpoints -| Method | Endpoint | Description | -| ------ | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | -| `PUT` | `/v1/platform/applications/{appID}/instances/{instanceID}/config` | Replaces the full instance configuration. Authenticated via `Bearer` token from `CLERK_PLATFORM_API_KEY`. | +| Method | Endpoint | Description | +| ------ | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `PUT` | `/v1/platform/applications/{appID}/instances/{instanceID}/config` | Replaces the full instance configuration. Sends `?dry_run=true` under `--dry-run` to validate and preview without persisting. Authenticated via `Bearer` token from `CLERK_PLATFORM_API_KEY`. | diff --git a/packages/cli-core/src/commands/config/push.test.ts b/packages/cli-core/src/commands/config/push.test.ts index 9270e151..fc40ecc6 100644 --- a/packages/cli-core/src/commands/config/push.test.ts +++ b/packages/cli-core/src/commands/config/push.test.ts @@ -525,10 +525,10 @@ describe("config push", () => { // --- Dry run --- - test("dry-run fetches current config and shows diff without mutating", async () => { - let mutatingCallMade = false; - stubFetch(async (_input, init) => { - if (init?.method && init.method !== "GET") mutatingCallMade = true; + test("dry-run sends PATCH with ?dry_run=true and prints diff and projected result", async () => { + let patchUrl = ""; + stubFetch(async (input, init) => { + if (init?.method === "PATCH") patchUrl = input.toString(); const body = init?.method && init.method !== "GET" ? mockResponse : currentConfig; return new Response(JSON.stringify(body), { status: 200 }); }); @@ -543,13 +543,21 @@ describe("config push", () => { json: '{"session":{"lifetime":3600}}', dryRun: true, }); - expect(mutatingCallMade).toBe(false); + expect(patchUrl).toContain("dry_run=true"); expect(captured.err).toContain("[dry-run] Would PATCH"); expect(captured.err).toContain("- 604800"); expect(captured.err).toContain("+ 3600"); + expect(captured.out).toContain(JSON.stringify(mockResponse, null, 2)); + expect(captured.err).toContain("[dry-run] Validation passed"); }); - test("dry-run reports no changes when payload matches current", async () => { + test("dry-run reports no changes when payload matches current without hitting server", async () => { + let mutatingCallMade = false; + stubFetch(async (_input, init) => { + if (init?.method && init.method !== "GET") mutatingCallMade = true; + return new Response(JSON.stringify(currentConfig), { status: 200 }); + }); + await setProfile(process.cwd(), { workspaceId: "org_1", appId: "app_1", @@ -560,10 +568,18 @@ describe("config push", () => { json: '{"session":{"lifetime":604800}}', dryRun: true, }); + expect(mutatingCallMade).toBe(false); expect(captured.err).toContain("[dry-run] No changes detected"); }); - test("dry-run for put shows PUT method", async () => { + test("dry-run for put sends PUT with ?dry_run=true", async () => { + let putUrl = ""; + stubFetch(async (input, init) => { + if (init?.method === "PUT") putUrl = input.toString(); + const body = init?.method && init.method !== "GET" ? mockResponse : currentConfig; + return new Response(JSON.stringify(body), { status: 200 }); + }); + await setProfile(process.cwd(), { workspaceId: "org_1", appId: "app_1", @@ -571,9 +587,32 @@ describe("config push", () => { }); await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true }); + expect(putUrl).toContain("dry_run=true"); expect(captured.err).toContain("[dry-run] Would PUT"); }); + test("dry-run surfaces server validation errors", async () => { + stubFetch(async (_input, init) => { + if (!init?.method || init.method === "GET") { + return new Response(JSON.stringify(currentConfig), { status: 200 }); + } + return new Response( + JSON.stringify({ errors: [{ message: "invalid organization_settings" }] }), + { status: 422 }, + ); + }); + + await setProfile(process.cwd(), { + workspaceId: "org_1", + appId: "app_1", + instances: { development: "ins_dev" }, + }); + + await expect( + runConfigPatch({ json: '{"session":{"lifetime":3600}}', dryRun: true }), + ).rejects.toThrow("API error"); + }); + // --- API error handling --- test("handles API errors gracefully", async () => { diff --git a/packages/cli-core/src/commands/config/push.ts b/packages/cli-core/src/commands/config/push.ts index 4d4be71b..bc8eaa8a 100644 --- a/packages/cli-core/src/commands/config/push.ts +++ b/packages/cli-core/src/commands/config/push.ts @@ -25,7 +25,7 @@ type Operation = { appId: string, instId: string, config: Record, - options?: { destructive?: boolean }, + options?: { destructive?: boolean; dryRun?: boolean }, ) => Promise>; }; @@ -91,9 +91,7 @@ async function configPush(options: ConfigPushOptions, op: Operation): Promise - withApiContext( - op.apiFn(ctx.appId, ctx.instanceId, configPayload, { destructive: options.destructive }), - "Failed to push config", - ), + const spinnerMsg = options.dryRun + ? `[dry-run] Validating config on ${ctx.appLabel} (${ctx.instanceLabel})...` + : `${op.verb} config on ${ctx.appLabel} (${ctx.instanceLabel})...`; + const result = await withSpinner(spinnerMsg, () => + withApiContext( + op.apiFn(ctx.appId, ctx.instanceId, configPayload, { + destructive: options.destructive, + dryRun: options.dryRun, + }), + options.dryRun ? "Dry-run failed" : "Failed to push config", + ), ); log.data(JSON.stringify(result, null, 2)); - log.success("Config pushed successfully"); + log.success( + options.dryRun + ? "[dry-run] Validation passed — no changes applied" + : "Config pushed successfully", + ); } export async function readInput(options: { file?: string; json?: string }): Promise { diff --git a/packages/cli-core/src/lib/plapi.ts b/packages/cli-core/src/lib/plapi.ts index 4e2fa331..dd52b57d 100644 --- a/packages/cli-core/src/lib/plapi.ts +++ b/packages/cli-core/src/lib/plapi.ts @@ -138,7 +138,7 @@ async function sendInstanceConfig( applicationId: string, instanceId: string, config: Record, - options?: { destructive?: boolean }, + options?: { destructive?: boolean; dryRun?: boolean }, ): Promise> { const url = new URL( `/v1/platform/applications/${applicationId}/instances/${instanceId}/config`, @@ -147,6 +147,9 @@ async function sendInstanceConfig( if (options?.destructive) { url.searchParams.set("destructive", "true"); } + if (options?.dryRun) { + url.searchParams.set("dry_run", "true"); + } const response = await plapiFetch(method, url, { body: JSON.stringify(config) }); return response.json() as Promise>; } @@ -155,14 +158,14 @@ export const putInstanceConfig = ( applicationId: string, instanceId: string, config: Record, - options?: { destructive?: boolean }, + options?: { destructive?: boolean; dryRun?: boolean }, ) => sendInstanceConfig("PUT", applicationId, instanceId, config, options); export const patchInstanceConfig = ( applicationId: string, instanceId: string, config: Record, - options?: { destructive?: boolean }, + options?: { destructive?: boolean; dryRun?: boolean }, ) => sendInstanceConfig("PATCH", applicationId, instanceId, config, options); export async function createApplication(name: string): Promise { diff --git a/packages/cli-core/src/test/integration/config-put.test.ts b/packages/cli-core/src/test/integration/config-put.test.ts index b9bffcdd..e1f98c5f 100644 --- a/packages/cli-core/src/test/integration/config-put.test.ts +++ b/packages/cli-core/src/test/integration/config-put.test.ts @@ -103,13 +103,16 @@ test("config put aborted when user declines confirmation", async () => { expect(putReqs.length).toBe(0); }); -test("config put --dry-run shows payload without sending request", async () => { - // Mock the current config endpoint (needed for the diff preview) - http.mock({ - "/config": { session: { lifetime: 604800 } }, +test("config put --dry-run sends PUT with ?dry_run=true and shows projected result", async () => { + http.stub(async (_url, init) => { + const body = + init?.method && init.method !== "GET" + ? { session: { lifetime: 3600 } } + : { session: { lifetime: 604800 } }; + return new Response(JSON.stringify(body), { status: 200 }); }); - const { stdout, stderr } = await clerk( + const { stderr } = await clerk( "--mode", "human", "config", @@ -122,7 +125,7 @@ test("config put --dry-run shows payload without sending request", async () => { expect(stderr).toContain("[dry-run]"); expect(stderr).toContain("3600"); - // No PUT request sent const putReqs = http.requests.filter((r) => r.method === "PUT"); - expect(putReqs.length).toBe(0); + expect(putReqs.length).toBe(1); + expect(putReqs[0]!.url).toContain("dry_run=true"); }); diff --git a/packages/cli-core/src/test/integration/dry-run.test.ts b/packages/cli-core/src/test/integration/dry-run.test.ts index 9ca54f93..70e6e88a 100644 --- a/packages/cli-core/src/test/integration/dry-run.test.ts +++ b/packages/cli-core/src/test/integration/dry-run.test.ts @@ -45,11 +45,11 @@ test.each([{ mode: "human" }, { mode: "agent" }])( ); test.each([{ mode: "human" }, { mode: "agent" }])( - "config patch dry-run fetches config and shows diff ($mode mode)", + "config patch dry-run sends PATCH with ?dry_run=true and shows diff ($mode mode)", async ({ mode }) => { http.stub(async (_url, init) => { const isGet = !init?.method || init.method === "GET"; - const body = isGet ? { session: { lifetime: 604800 } } : {}; + const body = isGet ? { session: { lifetime: 604800 } } : { session: { lifetime: 3600 } }; return new Response(JSON.stringify(body), { status: 200 }); }); @@ -65,7 +65,8 @@ test.each([{ mode: "human" }, { mode: "agent" }])( const getReqs = http.requests.filter((r) => r.method === "GET"); const patchReqs = http.requests.filter((r) => r.method === "PATCH"); expect(getReqs.length).toBe(1); - expect(patchReqs.length).toBe(0); + expect(patchReqs.length).toBe(1); + expect(patchReqs[0]!.url).toContain("dry_run=true"); expect(stderr).toContain("[dry-run]"); expect(stderr).toContain("604800"); expect(stderr).toContain("3600"); From fc0c31aa5816eff346b3682674270b3ec7bf9410 Mon Sep 17 00:00:00 2001 From: Daniel Moerner Date: Wed, 22 Apr 2026 12:01:16 -0400 Subject: [PATCH 2/5] fix: Improve wording of dry run proposal --- packages/cli-core/src/commands/config/push.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/config/push.ts b/packages/cli-core/src/commands/config/push.ts index bc8eaa8a..940e878f 100644 --- a/packages/cli-core/src/commands/config/push.ts +++ b/packages/cli-core/src/commands/config/push.ts @@ -87,7 +87,7 @@ async function configPush(options: ConfigPushOptions, op: Operation): Promise Date: Wed, 22 Apr 2026 12:05:05 -0400 Subject: [PATCH 3/5] chore: Improve changeset wording --- .changeset/orgs-diffing-quirks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/orgs-diffing-quirks.md b/.changeset/orgs-diffing-quirks.md index 903e7bbe..5868a129 100644 --- a/.changeset/orgs-diffing-quirks.md +++ b/.changeset/orgs-diffing-quirks.md @@ -2,4 +2,4 @@ "clerk": patch --- -Run `config patch --dry-run` and `config put --dry-run` against the server so validation errors are caught and the projected configuration (including any server-applied defaults) is returned before changes are committed. +Run `config patch --dry-run` and `config put --dry-run` against the server when changes are detected, so validation errors are caught and the projected configuration (including any server-applied defaults) is returned before changes are committed. From 4beb430ee548d351fa1a2983148911da81161f45 Mon Sep 17 00:00:00 2001 From: Daniel Moerner Date: Wed, 22 Apr 2026 12:10:10 -0400 Subject: [PATCH 4/5] test: Fix stale assertions --- packages/cli-core/src/commands/config/push.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/config/push.test.ts b/packages/cli-core/src/commands/config/push.test.ts index fc40ecc6..6add060b 100644 --- a/packages/cli-core/src/commands/config/push.test.ts +++ b/packages/cli-core/src/commands/config/push.test.ts @@ -544,7 +544,7 @@ describe("config push", () => { dryRun: true, }); expect(patchUrl).toContain("dry_run=true"); - expect(captured.err).toContain("[dry-run] Would PATCH"); + expect(captured.err).toContain("[dry-run] Proposing PATCH"); expect(captured.err).toContain("- 604800"); expect(captured.err).toContain("+ 3600"); expect(captured.out).toContain(JSON.stringify(mockResponse, null, 2)); @@ -588,7 +588,7 @@ describe("config push", () => { await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true }); expect(putUrl).toContain("dry_run=true"); - expect(captured.err).toContain("[dry-run] Would PUT"); + expect(captured.err).toContain("[dry-run] Proposing PUT"); }); test("dry-run surfaces server validation errors", async () => { @@ -610,7 +610,7 @@ describe("config push", () => { await expect( runConfigPatch({ json: '{"session":{"lifetime":3600}}', dryRun: true }), - ).rejects.toThrow("API error"); + ).rejects.toThrow("invalid organization_settings"); }); // --- API error handling --- From cec9f3672461650845a4977a7aed5b5d18cb1366 Mon Sep 17 00:00:00 2001 From: Daniel Moerner Date: Wed, 22 Apr 2026 12:12:24 -0400 Subject: [PATCH 5/5] test: More precise PUT test assertions --- packages/cli-core/src/commands/config/push.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli-core/src/commands/config/push.test.ts b/packages/cli-core/src/commands/config/push.test.ts index 6add060b..5b720dad 100644 --- a/packages/cli-core/src/commands/config/push.test.ts +++ b/packages/cli-core/src/commands/config/push.test.ts @@ -589,6 +589,10 @@ describe("config push", () => { await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true }); expect(putUrl).toContain("dry_run=true"); expect(captured.err).toContain("[dry-run] Proposing PUT"); + expect(captured.err).toContain("- 604800"); + expect(captured.err).toContain("+ 3600"); + expect(captured.out).toContain(JSON.stringify(mockResponse, null, 2)); + expect(captured.err).toContain("[dry-run] Validation passed"); }); test("dry-run surfaces server validation errors", async () => {