diff --git a/bin/cli/api-commands/combos.mjs b/bin/cli/api-commands/combos.mjs index e4e4ff62f5..8f1976be23 100644 --- a/bin/cli/api-commands/combos.mjs +++ b/bin/cli/api-commands/combos.mjs @@ -30,20 +30,60 @@ export function register_combos(parent) { const data = res.ok ? await res.json() : await res.text(); emit(data, gOpts); }); - tag.command("patch-api-combos-id-") - .description("Update combo") + tag.command("get-api-combos-id-") + .description("Get combo by ID") + .requiredOption("--id ", "") .action(async (opts, cmd) => { const gOpts = cmd.optsWithGlobals(); let url = "/api/combos/{id}"; - const res = await apiFetch(url, { method: "PATCH", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); + url = url.replace("{id}", encodeURIComponent(opts.id ?? "")); + const res = await apiFetch(url, { method: "GET", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); + const data = res.ok ? await res.json() : await res.text(); + emit(data, gOpts); + }); + tag.command("put-api-combos-id-") + .description("Update combo") + .requiredOption("--id ", "") + .option("--body ", "JSON body or @path/to/file.json") + .action(async (opts, cmd) => { + const gOpts = cmd.optsWithGlobals(); + let url = "/api/combos/{id}"; + url = url.replace("{id}", encodeURIComponent(opts.id ?? "")); + let body; + if (opts.body) { + body = opts.body.startsWith("@") + ? JSON.parse(readFileSync(opts.body.slice(1), "utf8")) + : JSON.parse(opts.body); + } + const res = await apiFetch(url, { method: "PUT", body, baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); + const data = res.ok ? await res.json() : await res.text(); + emit(data, gOpts); + }); + tag.command("patch-api-combos-id-") + .description("Update combo") + .requiredOption("--id ", "") + .option("--body ", "JSON body or @path/to/file.json") + .action(async (opts, cmd) => { + const gOpts = cmd.optsWithGlobals(); + let url = "/api/combos/{id}"; + url = url.replace("{id}", encodeURIComponent(opts.id ?? "")); + let body; + if (opts.body) { + body = opts.body.startsWith("@") + ? JSON.parse(readFileSync(opts.body.slice(1), "utf8")) + : JSON.parse(opts.body); + } + const res = await apiFetch(url, { method: "PATCH", body, baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); const data = res.ok ? await res.json() : await res.text(); emit(data, gOpts); }); tag.command("delete-api-combos-id-") .description("Delete combo") + .requiredOption("--id ", "") .action(async (opts, cmd) => { const gOpts = cmd.optsWithGlobals(); let url = "/api/combos/{id}"; + url = url.replace("{id}", encodeURIComponent(opts.id ?? "")); const res = await apiFetch(url, { method: "DELETE", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); const data = res.ok ? await res.json() : await res.text(); emit(data, gOpts); diff --git a/changelog.d/fixes/10955-cli-ref-params.md b/changelog.d/fixes/10955-cli-ref-params.md new file mode 100644 index 0000000000..9497b4129e --- /dev/null +++ b/changelog.d/fixes/10955-cli-ref-params.md @@ -0,0 +1 @@ +- fix(cli): resolve $ref path params and add PATCH combos requestBody in generated API commands (#10955) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index fb255d3543..e51b6091ae 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2105,11 +2105,26 @@ paths: patch: tags: [Combos] summary: Update combo + description: >- + Partial update: the body is merged onto the stored combo, so a field left out keeps + its current value. An array that IS sent replaces the stored one outright. parameters: - $ref: "#/components/parameters/ResourceId" + requestBody: + required: true + content: + application/json: + schema: + type: object responses: "200": description: Updated combo + "400": + description: Invalid body, or the resulting combo fails validation + "404": + description: Combo not found + "409": + description: Name already taken, or the combo is quota-share managed delete: tags: [Combos] summary: Delete combo diff --git a/scripts/check/check-env-doc-sync.mjs b/scripts/check/check-env-doc-sync.mjs index f2959131a0..9520da7647 100644 --- a/scripts/check/check-env-doc-sync.mjs +++ b/scripts/check/check-env-doc-sync.mjs @@ -156,8 +156,11 @@ const IGNORE_FROM_CODE = new Set([ // X11/Wayland display server vars used by tray heuristic (isTraySupported). "DISPLAY", "WAYLAND_DISPLAY", - // Build-time override for OpenAPI spec path used by generate-api-commands.mjs. + // Build-time overrides for generate-api-commands.mjs (spec input / commands output dir). + // OPENAPI_OUT_DIR exists so tests/unit/cli-api-generator-ref-params.test.ts can regenerate + // into a scratch dir instead of the real bin/cli/api-commands/ tree. "OPENAPI_SPEC", + "OPENAPI_OUT_DIR", // Aliases for documented vars handled via fallback ordering. "API_KEY", "APP_URL", diff --git a/scripts/cli/generate-api-commands.mjs b/scripts/cli/generate-api-commands.mjs index b937ceb389..b0bbff85f7 100644 --- a/scripts/cli/generate-api-commands.mjs +++ b/scripts/cli/generate-api-commands.mjs @@ -11,7 +11,7 @@ import * as yaml from "js-yaml"; const __dirname = dirname(fileURLToPath(import.meta.url)); const ROOT = join(__dirname, "..", ".."); const SPEC_PATH = process.env.OPENAPI_SPEC || join(ROOT, "docs/openapi.yaml"); -const OUT_DIR = join(ROOT, "bin/cli/api-commands"); +const OUT_DIR = process.env.OPENAPI_OUT_DIR || join(ROOT, "bin/cli/api-commands"); // Operations already covered by hand-crafted commands — skip in generated output. const IGNORED_OP_IDS = new Set([ @@ -51,6 +51,29 @@ if (!existsSync(OUT_DIR)) mkdirSync(OUT_DIR, { recursive: true }); const spec = yaml.load(readFileSync(SPEC_PATH, "utf8")); +// Minimal, scoped $ref resolver — only follows refs into components/parameters. +// This is not a generic dereferencer (no cycle handling, no cross-file refs): +// OpenAPI `parameters` entries in this spec only ever $ref a component parameter +// (see docs/openapi.yaml → components/parameters/ResourceId), so a full +// dereferencer would be scope creep. Without this, `p.in === "path"` silently +// drops every $ref'd path parameter (a bare `{ $ref }` object has no `.in`), +// which is what let generated PATCH/DELETE combo commands lose --id (#10955). +const PARAM_REF_PREFIX = "#/components/parameters/"; +function resolveParam(p) { + if (p && typeof p === "object" && typeof p.$ref === "string") { + if (!p.$ref.startsWith(PARAM_REF_PREFIX)) { + throw new Error(`Unsupported parameter $ref (only ${PARAM_REF_PREFIX}* is resolved): ${p.$ref}`); + } + const name = p.$ref.slice(PARAM_REF_PREFIX.length); + const resolved = spec.components?.parameters?.[name]; + if (!resolved) { + throw new Error(`Unresolvable parameter $ref: ${p.$ref}`); + } + return resolved; + } + return p; +} + /** @type {Record>} */ const byTag = {}; @@ -89,7 +112,7 @@ for (const [tag, ops] of Object.entries(byTag)) { for (const { path, method, opId, op } of ops) { const cmdName = kebab(opId); - const params = op.parameters || []; + const params = (op.parameters || []).map(resolveParam); const pathParams = params.filter((p) => p.in === "path"); const queryParams = params.filter((p) => p.in === "query"); const hasBody = !!op.requestBody; diff --git a/tests/unit/cli-api-generator-ref-params.test.ts b/tests/unit/cli-api-generator-ref-params.test.ts new file mode 100644 index 0000000000..be6b559753 --- /dev/null +++ b/tests/unit/cli-api-generator-ref-params.test.ts @@ -0,0 +1,150 @@ +// Regression test for #10955: generated `combos patch-*` CLI command sent a +// literal PATCH /api/combos/{id} to the server (405) because the generator +// dropped $ref'd path parameters (a bare `{ $ref }` object has no `.in`, so +// the `p.in === "path"` filter silently excluded it) and never emitted +// --body for the requestBody-less PATCH spec entry. +import test from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; +import { tmpdir } from "node:os"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const ROOT = join(__dirname, "..", ".."); +const GENERATOR = join(ROOT, "scripts", "cli", "generate-api-commands.mjs"); +const REAL_COMBOS = join(ROOT, "bin", "cli", "api-commands", "combos.mjs"); + +// Minimal fixture spec reproducing the exact shape that broke: a path +// parameter declared via $ref to a components/parameters entry, on a PATCH +// operation that also carries a requestBody. +const FIXTURE_SPEC = ` +openapi: 3.0.3 +info: + title: fixture + version: "1" +paths: + /api/widgets/{id}: + patch: + tags: [Widgets] + summary: Update widget + parameters: + - $ref: "#/components/parameters/ResourceId" + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + "200": + description: Updated widget +components: + parameters: + ResourceId: + name: id + in: path + required: true + schema: + type: string +`; + +function runGenerator(specPath, outDir) { + execFileSync(process.execPath, ["--import", "tsx/esm", GENERATOR], { + cwd: ROOT, + env: { ...process.env, OPENAPI_SPEC: specPath, OPENAPI_OUT_DIR: outDir }, + stdio: "pipe", + }); +} + +test("generator resolves a $ref path parameter into --id and substitutes {id} in the URL", () => { + const workDir = mkdtempSync(join(tmpdir(), "cli-api-gen-ref-")); + const specPath = join(workDir, "fixture.yaml"); + const outDir = join(workDir, "out"); + mkdirSync(outDir, { recursive: true }); + writeFileSync(specPath, FIXTURE_SPEC); + + try { + runGenerator(specPath, outDir); + const generated = readFileSync(join(outDir, "widgets.mjs"), "utf8"); + + // The $ref'd path param must have produced a required --id flag. + assert.match( + generated, + /\.requiredOption\("--id "/, + "generated command must declare --id from the resolved $ref path parameter" + ); + // The URL must be built with {id} substitution, not sent literally. + assert.match( + generated, + /url = url\.replace\("\{id\}", encodeURIComponent\(opts\.id/, + "generated command must substitute {id} in the URL" + ); + assert.doesNotMatch(generated, /url = "\/api\/widgets\/\{id\}";\s*\n\s*const res/); + + // requestBody presence must still produce --body. + assert.match( + generated, + /\.option\("--body "/, + "generated command must declare --body for the requestBody" + ); + } finally { + rmSync(workDir, { recursive: true, force: true }); + } +}); + +test("generator rejects an unsupported $ref target instead of silently dropping the parameter", () => { + const workDir = mkdtempSync(join(tmpdir(), "cli-api-gen-ref-bad-")); + const specPath = join(workDir, "fixture.yaml"); + const outDir = join(workDir, "out"); + mkdirSync(outDir, { recursive: true }); + writeFileSync( + specPath, + ` +openapi: 3.0.3 +info: + title: fixture + version: "1" +paths: + /api/widgets/{id}: + get: + tags: [Widgets] + summary: Get widget + parameters: + - $ref: "#/components/schemas/NotAParameter" + responses: + "200": + description: ok +components: + schemas: + NotAParameter: + type: object +` + ); + + try { + assert.throws(() => runGenerator(specPath, outDir)); + } finally { + rmSync(workDir, { recursive: true, force: true }); + } +}); + +test("real generated bin/cli/api-commands/combos.mjs has --id and --body on the PATCH combo command (#10955)", () => { + const src = readFileSync(REAL_COMBOS, "utf8"); + const patchBlockMatch = src.match(/ {2}tag\.command\("patch-[^"]*"\)[\s\S]*?\n {2}(?=tag\.command\(|\})/); + assert.ok(patchBlockMatch, "combos.mjs must have a generated patch-* command block"); + const patchBlock = patchBlockMatch[0]; + + assert.match(patchBlock, /\.requiredOption\("--id "/, "PATCH combo command must require --id"); + assert.match( + patchBlock, + /\.option\("--body "/, + "PATCH combo command must accept --body" + ); + assert.match( + patchBlock, + /url = url\.replace\("\{id\}", encodeURIComponent\(opts\.id/, + "PATCH combo command must substitute {id} in the URL, not send it literally" + ); +});