diff --git a/src/cli/runtime/body-flags.test.ts b/src/cli/runtime/body-flags.test.ts index 9359682..59c96c5 100644 --- a/src/cli/runtime/body-flags.test.ts +++ b/src/cli/runtime/body-flags.test.ts @@ -146,6 +146,251 @@ describe("generateBodyFlags", () => { expect(flags[0]?.description).toBe("User email address"); }); + + test("merges top-level allOf into flat properties", () => { + const flags = generateBodyFlags( + { + allOf: [ + { + type: "object", + properties: { + name: { type: "string", description: "Name" }, + }, + required: ["name"], + }, + { + type: "object", + properties: { + email: { type: "string", description: "Email" }, + }, + }, + ], + }, + new Set(), + ); + + expect(flags).toHaveLength(2); + expect(flags.find((f) => f.flag === "--name")).toEqual({ + flag: "--name", + path: ["name"], + type: "string", + description: "Name", + required: true, + }); + expect(flags.find((f) => f.flag === "--email")).toEqual({ + flag: "--email", + path: ["email"], + type: "string", + description: "Email", + required: false, + }); + }); + + test("merges nested allOf in property schemas", () => { + const flags = generateBodyFlags( + { + type: "object", + properties: { + transaction: { + allOf: [ + { type: "object" }, + { + type: "object", + properties: { + payee_name: { + type: "string", + description: "The payee name", + }, + amount: { + type: "integer", + description: "Amount in milliunits", + }, + }, + }, + ], + }, + }, + }, + new Set(), + ); + + expect(flags).toHaveLength(2); + expect(flags.find((f) => f.flag === "--transaction.payee_name")).toEqual({ + flag: "--transaction.payee_name", + path: ["transaction", "payee_name"], + type: "string", + description: "The payee name", + required: false, + }); + expect(flags.find((f) => f.flag === "--transaction.amount")).toEqual({ + flag: "--transaction.amount", + path: ["transaction", "amount"], + type: "integer", + description: "Amount in milliunits", + required: false, + }); + }); + + test("handles OpenAPI 3.1 nullable types (type arrays)", () => { + const flags = generateBodyFlags( + { + type: "object", + properties: { + name: { type: "string", description: "Name" }, + payee_name: { + type: ["string", "null"], + description: "Payee name", + }, + memo: { type: ["string", "null"], description: "Memo" }, + amount: { type: "integer", description: "Amount" }, + category_id: { + type: ["string", "null"], + description: "Category", + }, + }, + }, + new Set(), + ); + + expect(flags).toHaveLength(5); + expect(flags.find((f) => f.flag === "--name")?.type).toBe("string"); + expect(flags.find((f) => f.flag === "--payee_name")?.type).toBe("string"); + expect(flags.find((f) => f.flag === "--memo")?.type).toBe("string"); + expect(flags.find((f) => f.flag === "--amount")?.type).toBe("integer"); + expect(flags.find((f) => f.flag === "--category_id")?.type).toBe("string"); + }); + + test("handles nullable types in nested allOf schemas", () => { + const flags = generateBodyFlags( + { + type: "object", + properties: { + transaction: { + allOf: [ + { type: "object" }, + { + type: "object", + properties: { + account_id: { type: "string" }, + payee_name: { + type: ["string", "null"], + description: "The payee name", + }, + memo: { type: ["string", "null"] }, + }, + }, + ], + }, + }, + }, + new Set(), + ); + + expect(flags).toHaveLength(3); + expect( + flags.find((f) => f.flag === "--transaction.account_id"), + ).toBeDefined(); + expect( + flags.find((f) => f.flag === "--transaction.payee_name"), + ).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.memo")).toBeDefined(); + }); + + test("reproduces YNAB PutTransactionWrapper schema (allOf + nullable types)", () => { + // This matches the dereferenced shape of YNAB's PUT /budgets/{id}/transactions/{id} + // request body: PutTransactionWrapper -> transaction: ExistingTransaction (allOf) + // -> SaveTransactionWithOptionalFields (mixed scalar and nullable types) + const schema = { + type: "object", + required: ["transaction"], + properties: { + transaction: { + allOf: [ + { type: "object" }, + { + type: "object", + properties: { + account_id: { type: "string" }, + date: { type: "string", description: "Transaction date" }, + amount: { + type: "integer", + description: "Amount in milliunits", + }, + payee_id: { type: ["string", "null"], description: "Payee ID" }, + payee_name: { + type: ["string", "null"], + description: "Payee name", + }, + category_id: { + type: ["string", "null"], + description: "Category ID", + }, + memo: { type: ["string", "null"], description: "Memo" }, + cleared: { type: "string", description: "Cleared status" }, + approved: { type: "boolean", description: "Approved" }, + flag_color: { + type: ["string", "null"], + description: "Flag color", + }, + subtransactions: { type: "array" }, + }, + }, + ], + }, + }, + }; + + const flags = generateBodyFlags(schema, new Set()); + + // Should generate flags for all scalar/nullable fields (not subtransactions array) + expect(flags).toHaveLength(10); + + // Scalar types + expect(flags.find((f) => f.flag === "--transaction.account_id")).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.date")).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.amount")?.type).toBe( + "integer", + ); + expect(flags.find((f) => f.flag === "--transaction.cleared")).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.approved")?.type).toBe( + "boolean", + ); + + // Nullable types — these were previously missing + expect(flags.find((f) => f.flag === "--transaction.payee_id")).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.payee_name")).toBeDefined(); + expect( + flags.find((f) => f.flag === "--transaction.category_id"), + ).toBeDefined(); + expect(flags.find((f) => f.flag === "--transaction.memo")).toBeDefined(); + expect( + flags.find((f) => f.flag === "--transaction.flag_color"), + ).toBeDefined(); + }); + + test("handles allOf with properties alongside", () => { + const flags = generateBodyFlags( + { + type: "object", + properties: { + id: { type: "string" }, + }, + allOf: [ + { + type: "object", + properties: { + name: { type: "string" }, + }, + }, + ], + }, + new Set(), + ); + + expect(flags).toHaveLength(2); + expect(flags.find((f) => f.flag === "--id")).toBeDefined(); + expect(flags.find((f) => f.flag === "--name")).toBeDefined(); + }); }); describe("parseDotNotationFlags", () => { diff --git a/src/cli/runtime/body-flags.ts b/src/cli/runtime/body-flags.ts index 0d3930f..c494814 100644 --- a/src/cli/runtime/body-flags.ts +++ b/src/cli/runtime/body-flags.ts @@ -6,13 +6,25 @@ */ type JsonSchema = { - type?: string; + type?: string | string[]; properties?: Record; items?: JsonSchema; required?: string[]; description?: string; + allOf?: JsonSchema[]; }; +/** + * Resolve OpenAPI 3.1 nullable type arrays (e.g. ["string", "null"]) + * to their non-null scalar type. Returns the raw type for simple strings. + */ +function resolveType(type: string | string[] | undefined): string | undefined { + if (Array.isArray(type)) { + return type.find((t) => t !== "null"); + } + return type; +} + export type BodyFlagDef = { flag: string; // e.g. "--name" or "--address.street" path: string[]; // e.g. ["name"] or ["address", "street"] @@ -21,22 +33,61 @@ export type BodyFlagDef = { required: boolean; }; +/** + * Merge allOf compositions into a single schema with combined properties. + * Recursively handles nested allOf. + */ +function flattenAllOf(schema: JsonSchema): JsonSchema { + if (!schema.allOf || !Array.isArray(schema.allOf)) return schema; + + const merged: JsonSchema = { + type: schema.type ?? "object", + properties: {}, + required: [], + }; + + for (const sub of schema.allOf) { + const flat = flattenAllOf(sub); + if (flat.properties) { + merged.properties = { ...merged.properties, ...flat.properties }; + } + if (flat.required) { + merged.required = [...(merged.required ?? []), ...flat.required]; + } + } + + // Preserve top-level fields from the original schema + if (schema.properties) { + merged.properties = { ...merged.properties, ...schema.properties }; + } + if (schema.required) { + merged.required = [...(merged.required ?? []), ...schema.required]; + } + if (schema.description) merged.description = schema.description; + + return merged; +} + /** * Generate flag definitions from a JSON schema. * Recursively handles nested objects using dot notation. + * Merges allOf compositions before generating flags. */ export function generateBodyFlags( schema: JsonSchema | undefined, reservedFlags: Set, ): BodyFlagDef[] { - if (!schema || schema.type !== "object" || !schema.properties) { + if (!schema) return []; + + const resolved = flattenAllOf(schema); + if (resolveType(resolved.type) !== "object" || !resolved.properties) { return []; } const flags: BodyFlagDef[] = []; - const requiredSet = new Set(schema.required ?? []); + const requiredSet = new Set(resolved.required ?? []); - collectFlags(schema.properties, [], requiredSet, flags, reservedFlags); + collectFlags(resolved.properties, [], requiredSet, flags, reservedFlags); return flags; } @@ -58,13 +109,14 @@ function collectFlags( // Skip if this flag would conflict with an operation parameter if (reservedFlags.has(flagName)) continue; - const t = propSchema.type; + const resolved = flattenAllOf(propSchema); + const t = resolveType(resolved.type); - if (t === "object" && propSchema.properties) { + if (t === "object" && resolved.properties) { // Recurse into nested object - const nestedRequired = new Set(propSchema.required ?? []); + const nestedRequired = new Set(resolved.required ?? []); collectFlags( - propSchema.properties, + resolved.properties, path, nestedRequired, out, @@ -84,7 +136,7 @@ function collectFlags( flag: flagName, path, type: t, - description: propSchema.description ?? `Body field '${path.join(".")}'`, + description: resolved.description ?? `Body field '${path.join(".")}'`, required: isRequired, }); } diff --git a/src/cli/runtime/generated.ts b/src/cli/runtime/generated.ts index dd8aff1..5469ddd 100644 --- a/src/cli/runtime/generated.ts +++ b/src/cli/runtime/generated.ts @@ -21,6 +21,7 @@ function formatCustomHelp( action: CommandAction, operationFlags: CommandFlag[], bodyFlagDefs: BodyFlagDef[], + hasRequestBody: boolean, ): string { const lines: string[] = []; const cmdName = cmd.name(); @@ -95,6 +96,16 @@ function formatCustomHelp( lines.push(""); } + // Body input options + if (hasRequestBody) { + lines.push("Body:"); + lines.push( + " --data \n Inline request body (JSON or YAML)", + ); + lines.push(" --file \n Read request body from file"); + lines.push(""); + } + // Global options (always available) lines.push("Global:"); lines.push(" --curl\n Print curl command instead of executing"); @@ -177,9 +188,14 @@ export function addGeneratedCommands( else cmd.option(key, desc, parser); } - // Collect reserved flags: operation params + --curl + // Collect reserved flags: operation params + --curl + --data/--file const operationFlagSet = new Set(action.flags.map((f) => f.flag)); - const reservedFlags = new Set([...operationFlagSet, "--curl"]); + const reservedFlags = new Set([ + ...operationFlagSet, + "--curl", + "--data", + "--file", + ]); // Only --curl is a built-in flag (for debugging) if (!operationFlagSet.has("--curl")) { @@ -190,6 +206,10 @@ export function addGeneratedCommands( let bodyFlagDefs: BodyFlagDef[] = []; if (action.requestBody) { + // Register --data and --file for raw body input + cmd.option("--data ", "Inline request body (JSON or YAML)"); + cmd.option("--file ", "Read request body from file"); + // Generate body flags from schema (recursive with dot notation) // Pass reserved flags to avoid conflicts with operation params and --curl bodyFlagDefs = generateBodyFlags( @@ -209,7 +229,13 @@ export function addGeneratedCommands( // Custom help output for better agent/human readability cmd.configureHelp({ formatHelp: () => - formatCustomHelp(cmd, action, action.flags, bodyFlagDefs), + formatCustomHelp( + cmd, + action, + action.flags, + bodyFlagDefs, + Boolean(action.requestBody), + ), }); // Commander passes positional args and then the Command instance as last arg. diff --git a/src/cli/runtime/request.test.ts b/src/cli/runtime/request.test.ts index bd474a4..c119cef 100644 --- a/src/cli/runtime/request.test.ts +++ b/src/cli/runtime/request.test.ts @@ -1,5 +1,7 @@ import { describe, expect, test } from "bun:test"; +import { writeFileSync, mkdirSync } from "node:fs"; +import { join } from "node:path"; import { tmpdir } from "node:os"; import type { CommandAction } from "../model/command-model.js"; @@ -225,6 +227,161 @@ describe("buildRequest (requestBody)", () => { }); }); +describe("buildRequest (--data / --file)", () => { + test("uses --data as raw body", async () => { + const prevHome = process.env.HOME; + const home = `${tmpdir()}/specli-test-${crypto.randomUUID()}`; + process.env.HOME = home; + + try { + const action = makeAction({ + requestBodySchema: undefined, // no schema — raw body + }); + + const { request } = await buildRequest({ + specId: "spec", + action, + positionalValues: [], + flagValues: { data: '{"transaction":{"payee_name":"Test"}}' }, + globals: {}, + servers: [ + { url: "https://api.example.com", variables: [], variableNames: [] }, + ], + authSchemes: [], + }); + + expect(await request.clone().text()).toBe( + '{"transaction":{"payee_name":"Test"}}', + ); + } finally { + process.env.HOME = prevHome; + } + }); + + test("--data takes priority over body field flags", async () => { + const prevHome = process.env.HOME; + const home = `${tmpdir()}/specli-test-${crypto.randomUUID()}`; + process.env.HOME = home; + + try { + const action = makeAction(); + const bodyFlagDefs = generateBodyFlags( + action.requestBodySchema, + new Set(), + ); + + const { request } = await buildRequest({ + specId: "spec", + action, + positionalValues: [], + flagValues: { data: '{"x":1}', name: "ignored" }, + globals: {}, + servers: [ + { url: "https://api.example.com", variables: [], variableNames: [] }, + ], + authSchemes: [], + bodyFlagDefs, + }); + + expect(await request.clone().text()).toBe('{"x":1}'); + } finally { + process.env.HOME = prevHome; + } + }); + + test("errors when both --data and --file are provided", async () => { + const prevHome = process.env.HOME; + const home = `${tmpdir()}/specli-test-${crypto.randomUUID()}`; + process.env.HOME = home; + + try { + const action = makeAction(); + + await expect(() => + buildRequest({ + specId: "spec", + action, + positionalValues: [], + flagValues: { data: '{"x":1}', file: "/tmp/body.json" }, + globals: {}, + servers: [ + { + url: "https://api.example.com", + variables: [], + variableNames: [], + }, + ], + authSchemes: [], + }), + ).toThrow("Cannot use both --data and --file"); + } finally { + process.env.HOME = prevHome; + } + }); + + test("--file reads body from file", async () => { + const prevHome = process.env.HOME; + const home = `${tmpdir()}/specli-test-${crypto.randomUUID()}`; + process.env.HOME = home; + + try { + const dir = `${tmpdir()}/specli-file-test-${crypto.randomUUID()}`; + mkdirSync(dir, { recursive: true }); + const filePath = join(dir, "body.json"); + writeFileSync(filePath, '{"name":"from-file"}'); + + const action = makeAction({ + requestBodySchema: undefined, + }); + + const { request } = await buildRequest({ + specId: "spec", + action, + positionalValues: [], + flagValues: { file: filePath }, + globals: {}, + servers: [ + { url: "https://api.example.com", variables: [], variableNames: [] }, + ], + authSchemes: [], + }); + + expect(await request.clone().text()).toBe('{"name":"from-file"}'); + } finally { + process.env.HOME = prevHome; + } + }); + + test("--data body appears in curl output", async () => { + const prevHome = process.env.HOME; + const home = `${tmpdir()}/specli-test-${crypto.randomUUID()}`; + process.env.HOME = home; + + try { + const action = makeAction({ + requestBodySchema: undefined, + }); + + const { curl } = await buildRequest({ + specId: "spec", + action, + positionalValues: [], + flagValues: { data: '{"name":"Ada"}' }, + globals: {}, + servers: [ + { url: "https://api.example.com", variables: [], variableNames: [] }, + ], + authSchemes: [], + }); + + expect(curl).toContain("--data"); + expect(curl).toContain('{"name":"Ada"}'); + } finally { + process.env.HOME = prevHome; + } + }); +}); + describe("buildRequest (query parameters)", () => { test("builds query string from flag values", async () => { const prevHome = process.env.HOME; diff --git a/src/cli/runtime/request.ts b/src/cli/runtime/request.ts index 614c23b..c122bdb 100644 --- a/src/cli/runtime/request.ts +++ b/src/cli/runtime/request.ts @@ -3,6 +3,7 @@ import type { AuthScheme } from "../parse/auth-schemes.js"; import type { ServerInfo } from "../parse/servers.js"; import { resolveAuthScheme } from "./auth/resolve.js"; +import { loadBody } from "./body.js"; import { getToken } from "./profile/secrets.js"; import { getProfile, readProfiles } from "./profile/store.js"; import { resolveServerUrl } from "./server-url.js"; @@ -259,60 +260,74 @@ export async function buildRequest( let body: string | undefined; if (input.action.requestBody) { - // Check if any body flags were provided using the flag definitions - const bodyFlagDefs = input.bodyFlagDefs ?? []; - const hasBodyFlags = bodyFlagDefs.some((def) => { - // Commander keeps dots in option names: --address.street -> "address.street" - const dotKey = def.path.join("."); - return input.flagValues[dotKey] !== undefined; - }); - const contentType = input.action.requestBody.preferredContentType; if (contentType) headers.set("Content-Type", contentType); - const schema = input.action.requestBodySchema; + const rawData = input.flagValues.data as string | undefined; + const filePath = input.flagValues.file as string | undefined; - // Check if there are any required fields in the body - const requiredFields = bodyFlagDefs.filter((d) => d.required); + if (rawData && filePath) { + throw new Error("Cannot use both --data and --file"); + } - if (!hasBodyFlags) { - if (requiredFields.length > 0) { - // Error: user must provide required fields - const flagList = requiredFields.map((d) => `--${d.path.join(".")}`); - throw new Error(`Required: ${flagList.join(", ")}`); - } - // No required fields - send empty body if body is required, otherwise skip - if (input.action.requestBody.required) { - body = "{}"; + if (rawData || filePath) { + // --data or --file: use raw body input directly + const loaded = await loadBody( + rawData + ? { kind: "data", data: rawData } + : { kind: "file", path: filePath as string }, + ); + if (loaded) { + body = loaded.raw; } } else { - if (!contentType?.includes("json")) { - throw new Error( - "Body field flags are only supported for JSON request bodies.", - ); - } + // Fall back to body field flags (--name, --address.city, etc.) + const bodyFlagDefs = input.bodyFlagDefs ?? []; + const hasBodyFlags = bodyFlagDefs.some((def) => { + const dotKey = def.path.join("."); + return input.flagValues[dotKey] !== undefined; + }); + + const schema = input.action.requestBodySchema; + const requiredFields = bodyFlagDefs.filter((d) => d.required); + + if (!hasBodyFlags) { + if (requiredFields.length > 0) { + const flagList = requiredFields.map( + (d) => `--${d.path.join(".")}`, + ); + throw new Error(`Required: ${flagList.join(", ")}`); + } + if (input.action.requestBody.required) { + body = "{}"; + } + } else { + if (!contentType?.includes("json")) { + throw new Error( + "Body field flags are only supported for JSON request bodies.", + ); + } - // Check for missing required fields - const { findMissingRequired, parseDotNotationFlags } = await import( - "./body-flags.js" - ); - const missing = findMissingRequired(input.flagValues, bodyFlagDefs); - if (missing.length > 0) { - const missingFlags = missing.map((m) => `--${m}`).join(", "); - throw new Error(`Missing required fields: ${missingFlags}`); - } + const { findMissingRequired, parseDotNotationFlags } = await import( + "./body-flags.js" + ); + const missing = findMissingRequired(input.flagValues, bodyFlagDefs); + if (missing.length > 0) { + const missingFlags = missing.map((m) => `--${m}`).join(", "); + throw new Error(`Missing required fields: ${missingFlags}`); + } - // Build nested object from dot-notation flags - const built = parseDotNotationFlags(input.flagValues, bodyFlagDefs); + const built = parseDotNotationFlags(input.flagValues, bodyFlagDefs); - if (schema) { - const validate = ajv.compile(schema); - if (!validate(built)) { - throw new Error(formatAjvErrors(validate.errors)); + if (schema) { + const validate = ajv.compile(schema); + if (!validate(built)) { + throw new Error(formatAjvErrors(validate.errors)); + } } - } - body = JSON.stringify(built); + body = JSON.stringify(built); + } } }