Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/orgs-diffing-quirks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"clerk": patch
---

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.
16 changes: 8 additions & 8 deletions packages/cli-core/src/commands/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ clerk config patch --file partial-config.json --dry-run
| `--instance <id>` | Instance to target (`dev`, `prod`, or a full instance ID). Defaults to development. |
| `--file <path>` | Read config JSON from a file |
| `--json <string>` | 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
Expand All @@ -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`. |

---

Expand All @@ -140,7 +140,7 @@ clerk config put --file full-config.json --dry-run
| `--instance <id>` | Instance to target (`dev`, `prod`, or a full instance ID). Defaults to development. |
| `--file <path>` | Read config JSON from a file |
| `--json <string>` | 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
Expand All @@ -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`. |
61 changes: 52 additions & 9 deletions packages/cli-core/src/commands/config/push.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
Expand All @@ -543,13 +543,21 @@ describe("config push", () => {
json: '{"session":{"lifetime":3600}}',
dryRun: true,
});
expect(mutatingCallMade).toBe(false);
expect(captured.err).toContain("[dry-run] Would PATCH");
expect(patchUrl).toContain("dry_run=true");
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));
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",
Expand All @@ -560,18 +568,53 @@ 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",
instances: { development: "ins_dev" },
});

await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true });
expect(captured.err).toContain("[dry-run] Would PUT");
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");
});
Comment on lines 589 to 596

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small one: the PATCH dry-run test above asserts the projected result on stdout and the Validation passed line. The PUT test stops earlier, so a regression in either would slip through for PUT only. Worth mirroring.

Suggested change
await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true });
expect(putUrl).toContain("dry_run=true");
expect(captured.err).toContain("[dry-run] Would PUT");
});
await runConfigPut({ json: '{"session":{"lifetime":3600}}', dryRun: true });
expect(putUrl).toContain("dry_run=true");
expect(captured.err).toContain("[dry-run] Would PUT");
expect(captured.out).toContain(JSON.stringify(mockResponse, null, 2));
expect(captured.err).toContain("[dry-run] Validation passed");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, adopted (and fixed the "Would" in the error assertion from Wyatt's comment)


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("invalid organization_settings");
});

// --- API error handling ---
Expand Down
32 changes: 19 additions & 13 deletions packages/cli-core/src/commands/config/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type Operation = {
appId: string,
instId: string,
config: Record<string, unknown>,
options?: { destructive?: boolean },
options?: { destructive?: boolean; dryRun?: boolean },
) => Promise<Record<string, unknown>>;
};

Expand Down Expand Up @@ -87,13 +87,11 @@ async function configPush(options: ConfigPushOptions, op: Operation): Promise<vo
return;
}

const prefix = options.dryRun ? `[dry-run] Would ${op.method}` : op.verb;
const prefix = options.dryRun ? `[dry-run] Proposing ${op.method}` : op.verb;
log.info(`\n${prefix} config on ${ctx.appLabel} (${ctx.instanceLabel}):\n`);
printDiff(currentConfig, configPayload, isPatch);

if (options.dryRun) return;

if (isHuman() && !options.yes) {
if (!options.dryRun && isHuman() && !options.yes) {
if (op.warning) {
log.warn(`${op.warning}`);
}
Expand All @@ -103,16 +101,24 @@ async function configPush(options: ConfigPushOptions, op: Operation): Promise<vo
}
}

const result = await withSpinner(
`${op.verb} config on ${ctx.appLabel} (${ctx.instanceLabel})...`,
() =>
withApiContext(
op.apiFn(ctx.appId, ctx.instanceId, configPayload, { destructive: options.destructive }),
"Failed to push config",
),
const spinnerMsg = options.dryRun
Comment thread
wyattjoh marked this conversation as resolved.
? `[dry-run] Validating config on ${ctx.appLabel} (${ctx.instanceLabel})...`
: `${op.verb} config on ${ctx.appLabel} (${ctx.instanceLabel})...`;
Comment thread
wyattjoh marked this conversation as resolved.
const result = await withSpinner(spinnerMsg, () =>
withApiContext(
op.apiFn(ctx.appId, ctx.instanceId, configPayload, {
destructive: options.destructive,
dryRun: options.dryRun,
Comment thread
wyattjoh marked this conversation as resolved.
}),
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
Comment thread
wyattjoh marked this conversation as resolved.
? "[dry-run] Validation passed — no changes applied"
: "Config pushed successfully",
);
}

export async function readInput(options: { file?: string; json?: string }): Promise<string> {
Expand Down
9 changes: 6 additions & 3 deletions packages/cli-core/src/lib/plapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ async function sendInstanceConfig(
applicationId: string,
instanceId: string,
config: Record<string, unknown>,
options?: { destructive?: boolean },
options?: { destructive?: boolean; dryRun?: boolean },
): Promise<Record<string, unknown>> {
const url = new URL(
`/v1/platform/applications/${applicationId}/instances/${instanceId}/config`,
Expand All @@ -147,6 +147,9 @@ async function sendInstanceConfig(
if (options?.destructive) {
url.searchParams.set("destructive", "true");
}
if (options?.dryRun) {
Comment thread
wyattjoh marked this conversation as resolved.
url.searchParams.set("dry_run", "true");
}
const response = await plapiFetch(method, url, { body: JSON.stringify(config) });
return response.json() as Promise<Record<string, unknown>>;
}
Expand All @@ -155,14 +158,14 @@ export const putInstanceConfig = (
applicationId: string,
instanceId: string,
config: Record<string, unknown>,
options?: { destructive?: boolean },
options?: { destructive?: boolean; dryRun?: boolean },
) => sendInstanceConfig("PUT", applicationId, instanceId, config, options);

export const patchInstanceConfig = (
applicationId: string,
instanceId: string,
config: Record<string, unknown>,
options?: { destructive?: boolean },
options?: { destructive?: boolean; dryRun?: boolean },
) => sendInstanceConfig("PATCH", applicationId, instanceId, config, options);

export async function createApplication(name: string): Promise<Application> {
Expand Down
17 changes: 10 additions & 7 deletions packages/cli-core/src/test/integration/config-put.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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");
});
7 changes: 4 additions & 3 deletions packages/cli-core/src/test/integration/dry-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});

Expand All @@ -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");
Expand Down
Loading