Skip to content

Commit 64f6f75

Browse files
committed
fix(api): address review feedback on --fapi passthrough
- Move dry-run check before resolveFapiHost so --fapi --dry-run avoids the Platform API round-trip; shows <fapi-host> placeholder instead - Import CLERK_JS_API_VERSION from lib/fapi.ts instead of redeclaring it - Warn when --secret-key is provided with --fapi (key is ignored) - Add tests: --fapi no-app NOT_LINKED error, /environment and /v1/environment path normalization, --secret-key warning, --dry-run no network call
1 parent 020a2f5 commit 64f6f75

4 files changed

Lines changed: 97 additions & 17 deletions

File tree

packages/cli-core/src/commands/api/fapi.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,11 @@ import {
1515
throwUsageError,
1616
withApiContext,
1717
} from "../../lib/errors.ts";
18-
import { decodePublishableKey } from "../../lib/fapi.ts";
18+
import { decodePublishableKey, CLERK_JS_API_VERSION } from "../../lib/fapi.ts";
1919
import { loggedFetch } from "../../lib/fetch.ts";
2020
import { fetchApplication, type ApplicationInstance } from "../../lib/plapi.ts";
2121
import type { BapiResponse } from "./bapi.ts";
2222

23-
/** clerk-js API version FAPI shapes its `/v1/environment` payload for. */
24-
const CLERK_JS_API_VERSION = "5";
25-
2623
interface ResolveOptions {
2724
app?: string;
2825
instance?: string;

packages/cli-core/src/commands/api/index.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,77 @@ describe("api command", () => {
542542
expect(captured.out).toContain(JSON.stringify(errorBody, null, 2));
543543
});
544544

545+
test("--fapi without --app and no linked project errors with NOT_LINKED guidance", async () => {
546+
delete process.env.CLERK_SECRET_KEY;
547+
548+
await expect(runApi("/environment", { fapi: true })).rejects.toThrow(/clerk link|--app/);
549+
});
550+
551+
test.each([
552+
["/environment", "/v1/environment"],
553+
["/v1/environment", "/v1/environment"],
554+
])("--fapi: %s resolves to the same FAPI path %s", async (input, expectedPath) => {
555+
process.env.CLERK_PLATFORM_API_KEY = "ak_test_platform";
556+
const pk = `pk_test_${btoa("clerk.example.com$")}`;
557+
let capturedPath = "";
558+
559+
stubFetch(async (input) => {
560+
const url = input.toString();
561+
if (url.includes("/v1/platform/applications/app_1")) {
562+
return new Response(
563+
JSON.stringify({
564+
application_id: "app_1",
565+
instances: [
566+
{ instance_id: "ins_dev", environment_type: "development", publishable_key: pk },
567+
],
568+
}),
569+
{ status: 200 },
570+
);
571+
}
572+
capturedPath = new URL(url).pathname;
573+
return new Response(JSON.stringify({}), { status: 200 });
574+
});
575+
576+
await runApi(input, { fapi: true, app: "app_1" });
577+
expect(capturedPath).toBe(expectedPath);
578+
});
579+
580+
test("--fapi + --secret-key emits a warning that --secret-key is ignored", async () => {
581+
process.env.CLERK_PLATFORM_API_KEY = "ak_test_platform";
582+
const pk = `pk_test_${btoa("clerk.example.com$")}`;
583+
584+
stubFetch(async (input) => {
585+
const url = input.toString();
586+
if (url.includes("/v1/platform/applications/app_1")) {
587+
return new Response(
588+
JSON.stringify({
589+
application_id: "app_1",
590+
instances: [
591+
{ instance_id: "ins_dev", environment_type: "development", publishable_key: pk },
592+
],
593+
}),
594+
{ status: 200 },
595+
);
596+
}
597+
return new Response(JSON.stringify({}), { status: 200 });
598+
});
599+
600+
await runApi("/environment", { fapi: true, app: "app_1", secretKey: "sk_test_ignored" });
601+
expect(captured.err).toMatch(/--secret-key is ignored/);
602+
});
603+
604+
test("--fapi + --dry-run does not make any network request", async () => {
605+
let fetchCalled = false;
606+
stubFetch(async () => {
607+
fetchCalled = true;
608+
return new Response(JSON.stringify({}), { status: 200 });
609+
});
610+
611+
await runApi("/environment", { fapi: true, app: "app_1", dryRun: true });
612+
expect(fetchCalled).toBe(false);
613+
expect(captured.err).toContain("[dry-run] GET <fapi-host>");
614+
});
615+
545616
// --- Error handling ---
546617

547618
test("errors when no secret key available", async () => {

packages/cli-core/src/commands/api/index.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,21 @@ const MUTATING_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
3535

3636
type RunRequest = (req: { method: string; path: string; body?: string }) => Promise<BapiResponse>;
3737

38+
/** Validate fapi flag combinations and emit warnings for ignored flags. */
39+
function validateFapiOptions(options: ApiOptions): void {
40+
if (options.platform) {
41+
throwUsageError("--fapi and --platform cannot be combined.", undefined, ERROR_CODE.USAGE_ERROR);
42+
}
43+
if (options.secretKey) {
44+
log.warn("--secret-key is ignored when --fapi is set.");
45+
}
46+
}
47+
3848
/** Resolve the API surface (base URL + request executor) from the flags. */
3949
async function resolveApiTarget(
4050
options: ApiOptions,
4151
): Promise<{ baseUrl: string; runRequest: RunRequest }> {
4252
if (options.fapi) {
43-
if (options.platform) {
44-
throwUsageError(
45-
"--fapi and --platform cannot be combined.",
46-
undefined,
47-
ERROR_CODE.USAGE_ERROR,
48-
);
49-
}
5053
const fapiHost = await resolveFapiHost(options);
5154
const baseUrl = `https://${fapiHost}`;
5255
return { baseUrl, runRequest: (req) => fapiRequest({ ...req, fapiHost }) };
@@ -93,18 +96,27 @@ export async function api(
9396
// 2. Determine HTTP method
9497
const method = (options.method ?? (body ? "POST" : "GET")).toUpperCase();
9598

96-
// 3. Resolve the request target (base URL + executor)
97-
const { baseUrl, runRequest } = await resolveApiTarget(options);
98-
99-
// 4. Dry run
99+
// 3. Dry run — for --fapi, skip host resolution to avoid a real Platform API round-trip
100100
if (options.dryRun) {
101-
log.info(`[dry-run] ${method} ${baseUrl}${normalizeBapiPath(endpoint)}`);
101+
if (options.fapi) {
102+
validateFapiOptions(options);
103+
log.info(`[dry-run] ${method} <fapi-host>${normalizeBapiPath(endpoint)}`);
104+
} else {
105+
const { baseUrl } = await resolveApiTarget(options);
106+
log.info(`[dry-run] ${method} ${baseUrl}${normalizeBapiPath(endpoint)}`);
107+
}
102108
if (body) {
103109
prettyPrint(body);
104110
}
105111
return;
106112
}
107113

114+
// 4. Resolve the request target (base URL + executor)
115+
if (options.fapi) {
116+
validateFapiOptions(options);
117+
}
118+
const { runRequest } = await resolveApiTarget(options);
119+
108120
// 5. Confirmation for mutating methods
109121
if (MUTATING_METHODS.has(method) && isHuman() && !options.yes) {
110122
log.info(`\nAbout to ${method} ${endpoint}`);

packages/cli-core/src/lib/fapi.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const PK_LIVE_PREFIX = "pk_live_";
1515
* The clerk-js client version FAPI's `/v1/environment` payload is shaped for.
1616
* Bump when consuming response fields introduced in a later major version.
1717
*/
18-
const CLERK_JS_API_VERSION = "5";
18+
export const CLERK_JS_API_VERSION = "5";
1919

2020
export type InstanceType = "development" | "production";
2121

0 commit comments

Comments
 (0)