From 06da51e3167c9a965c3b4f06a6023753d4d16014 Mon Sep 17 00:00:00 2001 From: 0xpolarzero <0xpolarzero@gmail.com> Date: Tue, 26 May 2026 13:32:34 +0200 Subject: [PATCH] fix: correct OpenAPI generated commands --- src/Cli.test-d.ts | 12 + src/Completions.test.ts | 60 +++ src/Completions.ts | 30 +- src/Help.test.ts | 62 +++ src/Help.ts | 30 ++ src/Openapi.test.ts | 862 +++++++++++++++++++++++++++++++++++++++- src/Openapi.ts | 195 +++++++-- src/Parser.test.ts | 60 +++ src/Parser.ts | 32 +- 9 files changed, 1297 insertions(+), 46 deletions(-) diff --git a/src/Cli.test-d.ts b/src/Cli.test-d.ts index dc44c93..791f248 100644 --- a/src/Cli.test-d.ts +++ b/src/Cli.test-d.ts @@ -170,6 +170,14 @@ test('OpenAPI-mounted operations are included in CLI command map type', () => { responses: { '200': { description: 'ok' } }, }, }, + '/widgets/{id}/actions': { + additionalOperations: { + Search: { + operationId: 'searchWidgetActions', + responses: { '200': { description: 'ok' } }, + }, + }, + }, }, }, }) @@ -177,6 +185,10 @@ test('OpenAPI-mounted operations are included in CLI command map type', () => { expectTypeOf().toMatchTypeOf< Cli.Cli<{ 'api listUsers': { args: Record; options: Record } + 'api searchWidgetActions': { + args: Record + options: Record + } }> >() }) diff --git a/src/Completions.test.ts b/src/Completions.test.ts index 6ba2d17..412a200 100644 --- a/src/Completions.test.ts +++ b/src/Completions.test.ts @@ -207,6 +207,66 @@ describe('complete', () => { expect(candidates).toEqual([]) }) + test('does not run transformed boolean schemas when completing flags', () => { + let calls = 0 + const flag = z + .union([ + z.boolean(), + z.enum(['true', 'false']).transform((value) => { + calls++ + return value === 'true' + }), + ]) + .pipe(z.boolean()) + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + { options: z.object({ flag: flag.optional() }) }, + ['mycli', '--flag', ''], + 2, + ) + + expect(candidates.map((c) => c.value)).toEqual(['status']) + expect(calls).toBe(0) + }) + + test('does not treat arbitrary boolean preprocessors as boolean flags', () => { + const debug = z.preprocess((value) => { + if (value === 'yes') return true + if (value === 'no') return false + return 'invalid' + }, z.boolean()) + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + { options: z.object({ debug }) }, + ['mycli', '--debug', ''], + 2, + ) + + expect(candidates).toEqual([]) + }) + + test('does not treat boolean literal options as boolean flags', () => { + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + { options: z.object({ exact: z.literal(false).optional() }) }, + ['mycli', '--exact', ''], + 2, + ) + + expect(candidates).toEqual([]) + }) + + test('does not treat string-parsed boolean options as boolean flags', () => { + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + { options: z.object({ debug: z.stringbool().optional() }) }, + ['mycli', '--debug', ''], + 2, + ) + + expect(candidates).toEqual([]) + }) + test('includes descriptions', () => { const cli = makeCli() const commands = Cli.toCommands.get(cli)! diff --git a/src/Completions.ts b/src/Completions.ts index ed4623d..b2b3d66 100644 --- a/src/Completions.ts +++ b/src/Completions.ts @@ -1,4 +1,4 @@ -import type { z } from 'zod' +import { z } from 'zod' import type { Shell } from './internal/command.js' @@ -196,7 +196,33 @@ function isBooleanOption(name: string, schema: z.ZodObject): boolean { const field = schema.shape[name] if (!field) return false if (typeof field.meta === 'function' && field.meta()?.count === true) return true - return unwrap(field).constructor.name === 'ZodBoolean' + const inner = unwrap(field) + if (inner.constructor.name === 'ZodBoolean') return true + // Generated OpenAPI booleans accept real booleans plus CLI "true"/"false" strings. + // Check that public schema shape instead of running user validation during introspection. + const input = z.toJSONSchema(inner, { unrepresentable: 'any', io: 'input' }) as { + anyOf?: { enum?: unknown[] | undefined; type?: unknown | undefined }[] | undefined + const?: unknown | undefined + type?: unknown | undefined + } + const output = z.toJSONSchema(inner, { unrepresentable: 'any', io: 'output' }) as { + const?: unknown | undefined + type?: unknown | undefined + } + if ( + input.anyOf?.some((schema) => schema.type === 'boolean') && + input.anyOf?.some( + (schema) => + schema.type === 'string' && + schema.enum?.includes('true') && + schema.enum.includes('false') && + schema.enum.length === 2, + ) && + output.type === 'boolean' && + !('const' in output) + ) + return true + return false } /** @internal Extracts possible values from enum schemas. */ diff --git a/src/Help.test.ts b/src/Help.test.ts index 071bdfa..b48d474 100644 --- a/src/Help.test.ts +++ b/src/Help.test.ts @@ -160,6 +160,68 @@ describe('formatCommand', () => { expect(line).toBe(' --dry-run Preview without submitting.') }) + test('does not run transformed boolean schemas when formatting help', () => { + let calls = 0 + const dryRun = z + .union([ + z.boolean(), + z.enum(['true', 'false']).transform((value) => { + calls++ + return value === 'true' + }), + ]) + .pipe(z.boolean()) + + const result = Help.formatCommand('tool deploy', { + options: z.object({ dryRun: dryRun.optional().describe('Preview without submitting.') }), + }) + const line = result.split('\n').find((line) => line.includes('--dry-run')) + + expect(line).toBe(' --dry-run Preview without submitting.') + expect(calls).toBe(0) + }) + + test('keeps value placeholders for arbitrary boolean preprocessors', () => { + const debug = z + .preprocess((value) => { + if (value === 'yes') return true + if (value === 'no') return false + return 'invalid' + }, z.boolean()) + .describe('Enable debug output.') + + const result = Help.formatCommand('tool deploy', { + options: z.object({ debug }), + }) + const line = result.split('\n').find((line) => line.includes('--debug')) + + expect(line).toBe(' --debug Enable debug output.') + }) + + test('keeps value placeholders for boolean literal options', () => { + const result = Help.formatCommand('tool deploy', { + options: z.object({ + exact: z.literal(false).optional().describe('Must be false.'), + }), + }) + + const line = result.split('\n').find((line) => line.includes('--exact')) + + expect(line).toBe(' --exact Must be false.') + }) + + test('keeps value placeholders for string-parsed boolean options', () => { + const result = Help.formatCommand('tool deploy', { + options: z.object({ + debug: z.stringbool().optional().describe('Enable debug output.'), + }), + }) + + const line = result.split('\n').find((line) => line.includes('--debug')) + + expect(line).toBe(' --debug Enable debug output.') + }) + test('omits value placeholders for aliased boolean flag options', () => { const result = Help.formatCommand('tool deploy', { options: z.object({ diff --git a/src/Help.ts b/src/Help.ts index d7537d5..d463903 100644 --- a/src/Help.ts +++ b/src/Help.ts @@ -292,6 +292,36 @@ function resolveTypeName(schema: unknown): string { if (unwrapped instanceof z.ZodString) return 'string' if (unwrapped instanceof z.ZodNumber) return 'number' if (unwrapped instanceof z.ZodBoolean) return 'boolean' + // Generated OpenAPI booleans accept real booleans plus CLI "true"/"false" strings. + // Check that public schema shape instead of running user validation during introspection. + const input = z.toJSONSchema(unwrapped as z.ZodType, { + unrepresentable: 'any', + io: 'input', + }) as { + anyOf?: { enum?: unknown[] | undefined; type?: unknown | undefined }[] | undefined + const?: unknown | undefined + type?: unknown | undefined + } + const output = z.toJSONSchema(unwrapped as z.ZodType, { + unrepresentable: 'any', + io: 'output', + }) as { + const?: unknown | undefined + type?: unknown | undefined + } + if ( + input.anyOf?.some((schema) => schema.type === 'boolean') && + input.anyOf?.some( + (schema) => + schema.type === 'string' && + schema.enum?.includes('true') && + schema.enum.includes('false') && + schema.enum.length === 2, + ) && + output.type === 'boolean' && + !('const' in output) + ) + return 'boolean' if (unwrapped instanceof z.ZodArray) return 'array' if (unwrapped instanceof z.ZodEnum) { const values = Object.values((unwrapped as any)._zod.def.entries) as string[] diff --git a/src/Openapi.test.ts b/src/Openapi.test.ts index 6b9a732..073c23c 100644 --- a/src/Openapi.test.ts +++ b/src/Openapi.test.ts @@ -12,8 +12,12 @@ import { app } from '../test/fixtures/hono-api.js' import { app as openapiApp, spec as openapiSpec } from '../test/fixtures/hono-openapi-app.js' import { spec } from '../test/fixtures/openapi-spec.js' import * as Cli from './Cli.js' +import * as Completions from './Completions.js' import * as Fetch from './Fetch.js' +import * as Help from './Help.js' import * as Openapi from './Openapi.js' +import * as Parser from './Parser.js' +import * as Schema from './Schema.js' function serve(cli: { serve: Cli.Cli['serve'] }, argv: string[]) { let output = '' @@ -35,6 +39,12 @@ function json(output: string) { return JSON.parse(output.replace(/"duration": "[^"]+"/g, '"duration": ""')) } +function command(commands: Map, name: string) { + const entry = commands.get(name) + if (!entry || '_group' in entry) throw new Error(`expected ${name} command`) + return entry +} + function openapiUrl() { return `data:application/json,${encodeURIComponent(JSON.stringify(spec))}` } @@ -149,28 +159,25 @@ describe('generateCommands', () => { test('command has description from summary', async () => { const commands = await Openapi.generateCommands(spec, app.fetch) - const cmd = commands.get('listUsers')! - if ('_group' in cmd) throw new Error('expected listUsers command') + const cmd = command(commands, 'listUsers') expect(cmd.description).toBe('List users') }) test('coerced number params preserve description', async () => { const commands = await Openapi.generateCommands(spec, app.fetch) - const cmd = commands.get('listUsers')! - if ('_group' in cmd) throw new Error('expected listUsers command') + const cmd = command(commands, 'listUsers') const limitSchema = cmd.options!.shape.limit expect(limitSchema.description).toBe('Max results') }) - test('infers output from JSON response schemas', async () => { + test('infers output and supports fallback command names', async () => { const commands = await Openapi.generateCommands( { paths: { '/users/posts': { get: { - operationId: 'listPosts', responses: { - '200': { + '204': { content: { 'application/json': { schema: { @@ -185,11 +192,20 @@ describe('generateCommands', () => { }, }, }, - () => new Response(JSON.stringify({ ok: true })), + (req) => { + expect(new URL(req.url).pathname).toBe('/users/posts') + return new Response(JSON.stringify({ ok: true }), { + headers: { 'content-type': 'application/json' }, + }) + }, ) - const command = commands.get('listPosts')! - if ('_group' in command) throw new Error('expected listPosts command') - expect(command.output).toBeDefined() + const cmd = command(commands, 'get users posts') + expect(cmd.output).toBeDefined() + await cmd.run({ + args: {}, + options: {}, + error: (value: unknown) => value, + }) }) test('generates namespace command groups from paths', async () => { @@ -223,6 +239,705 @@ describe('generateCommands', () => { ] `) }) + + test('path-level parameters are applied and non-operation fields are ignored', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/orgs/{orgId}/users': { + parameters: [ + { + name: 'orgId', + in: 'path', + required: true, + schema: { type: 'string' }, + }, + ], + get: { + operationId: 'listOrgUsers', + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + expect(commands.has('parameters__orgs__orgId__users')).toBe(false) + expect(command(commands, 'listOrgUsers').args!.safeParse({ orgId: 'acme' }).success).toBe(true) + }) + + test('path-level query parameter is inherited', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean' }, + }, + ], + get: { + operationId: 'listUsers', + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'listUsers').options! + expect(options.parse({ active: 'true' }).active).toBe(true) + }) + + test('operation-level parameter overrides path-level same in:name', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { enum: ['path'] }, + }, + ], + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'filter', + in: 'query', + schema: { enum: ['operation'] }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'listUsers').options! + expect(options.safeParse({ filter: 'operation' }).success).toBe(true) + expect(options.safeParse({ filter: 'path' }).success).toBe(false) + }) + + test('OpenAPI 3.2 query and additionalOperations generate commands', async () => { + const calls: { method: string; path: string; query: Record }[] = [] + const commands = await Openapi.generateCommands( + { + paths: { + '/widgets/{id}/actions': { + summary: 'Widget actions', + 'x-note': 'metadata', + parameters: [{ name: 'id', in: 'path', required: true, schema: { type: 'string' } }], + query: { + operationId: 'queryWidgetActions', + parameters: [{ name: 'filter', in: 'query', schema: { type: 'string' } }], + responses: { '200': { description: 'ok' } }, + }, + additionalOperations: { + Search: { + operationId: 'searchWidgetActions', + parameters: [{ name: 'cursor', in: 'query', schema: { type: 'string' } }], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + }, + (req) => { + const url = new URL(req.url) + calls.push({ + method: req.method, + path: url.pathname, + query: Object.fromEntries(url.searchParams), + }) + return Response.json({}) + }, + ) + + expect(commands.has('queryWidgetActions')).toBe(true) + expect(commands.has('searchWidgetActions')).toBe(true) + expect(commands.has('summary__widgets__id__actions')).toBe(false) + expect(commands.has('additionalOperations__widgets__id__actions')).toBe(false) + + await command(commands, 'queryWidgetActions').run({ + args: { id: 'a/b' }, + options: { filter: 'open' }, + error: (value: unknown) => value, + }) + await command(commands, 'searchWidgetActions').run({ + args: { id: 'a b' }, + options: { cursor: 'next' }, + error: (value: unknown) => value, + }) + + expect(calls).toEqual([ + { method: 'QUERY', path: '/widgets/a%2Fb/actions', query: { filter: 'open' } }, + { method: 'Search', path: '/widgets/a%20b/actions', query: { cursor: 'next' } }, + ]) + }) + + test('encodes interpolated path parameters', async () => { + const calls: string[] = [] + const commands = await Openapi.generateCommands( + { + paths: { + '/users/{id}': { + get: { + operationId: 'getUser', + parameters: [ + { + name: 'id', + in: 'path', + required: true, + schema: { type: 'string' }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + (req) => { + calls.push(new URL(req.url).pathname) + return new Response('{}', { headers: { 'content-type': 'application/json' } }) + }, + ) + + await command(commands, 'getUser').run({ + args: { id: 'a/b' }, + options: {}, + error: (value: unknown) => value, + }) + + expect(calls).toEqual(['/users/a%2Fb']) + }) + + test('optional request body enforces required properties only when body is provided', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + post: { + operationId: 'createUser', + requestBody: { + required: false, + content: { + 'application/json': { + schema: { + type: 'object', + required: ['name'], + properties: { name: { type: 'string' }, age: { type: 'number' } }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'createUser').options! + expect(options.safeParse({}).success).toBe(true) + expect(options.safeParse({ name: 'Alice' }).success).toBe(true) + expect(options.safeParse({ age: 42 }).success).toBe(false) + }) + + test('query options do not make optional request bodies count as provided', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + post: { + operationId: 'createUser', + parameters: [{ name: 'dryRun', in: 'query', schema: { type: 'boolean' } }], + requestBody: { + required: false, + content: { + 'application/json': { + schema: { + type: 'object', + required: ['name'], + properties: { name: { type: 'string' }, age: { type: 'number' } }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'createUser').options! + expect(options.safeParse({ dryRun: true }).success).toBe(true) + expect(options.safeParse({ dryRun: true, age: 42 }).success).toBe(false) + }) + + test('optional request body ignores defaults when deciding whether body is provided', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + post: { + operationId: 'createUser', + requestBody: { + required: false, + content: { + 'application/json': { + schema: { + type: 'object', + required: ['name'], + properties: { + name: { type: 'string' }, + dryRun: { type: 'boolean', default: false }, + }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'createUser').options! + expect(options.safeParse({}).success).toBe(true) + expect(options.safeParse({ dryRun: false }).success).toBe(false) + expect(options.safeParse({ name: 'Alice' }).success).toBe(true) + }) + + test('body boolean string coercion accepts true and false only', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + post: { + operationId: 'createUser', + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + active: { type: 'boolean', default: false }, + }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'createUser').options! + expect(options.parse({ active: 'true' }).active).toBe(true) + expect(options.parse({ active: 'false' }).active).toBe(false) + expect(options.parse({}).active).toBe(false) + expect(options.safeParse({ active: 'yes' }).success).toBe(false) + }) + + test('required request body requires required properties', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + post: { + operationId: 'createUser', + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + required: ['name'], + properties: { name: { type: 'string' }, age: { type: 'number' } }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const options = command(commands, 'createUser').options! + expect(options.safeParse({}).success).toBe(false) + expect(options.safeParse({ name: 'Alice' }).success).toBe(true) + }) + + test('required request body sends empty object when no body fields are provided', async () => { + const bodies: string[] = [] + const commands = await Openapi.generateCommands( + { + paths: { + '/profile': { + post: { + operationId: 'updateProfile', + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { nickname: { type: 'string' } }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + '/empty': { + post: { + operationId: 'createEmpty', + requestBody: { + required: true, + content: { 'application/json': { schema: { type: 'object', properties: {} } } }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + async (req) => { + bodies.push(await req.text()) + return new Response('{}', { headers: { 'content-type': 'application/json' } }) + }, + ) + + await command(commands, 'updateProfile').run({ options: {}, error: (value: unknown) => value }) + await command(commands, 'createEmpty').run({ options: {}, error: (value: unknown) => value }) + + expect(bodies).toEqual(['{}', '{}']) + }) + + test('required non-object request bodies do not synthesize JSON objects', async () => { + const calls: { body: string; contentType: string | null; path: string }[] = [] + const commands = await Openapi.generateCommands( + { + paths: { + '/scalar': { + post: { + operationId: 'postScalar', + requestBody: { + required: true, + content: { 'application/json': { schema: { type: 'string' } } }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + '/array': { + post: { + operationId: 'postArray', + requestBody: { + required: true, + content: { + 'application/json': { + schema: { type: 'array', items: { type: 'string' } }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + '/plain': { + post: { + operationId: 'postPlain', + requestBody: { + required: true, + content: { 'text/plain': { schema: { type: 'string' } } }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + async (req) => { + calls.push({ + path: new URL(req.url).pathname, + body: await req.text(), + contentType: req.headers.get('content-type'), + }) + return Response.json({}) + }, + ) + + for (const name of ['postScalar', 'postArray', 'postPlain']) + await command(commands, name).run({ options: {}, error: (value: unknown) => value }) + + expect(calls).toEqual([ + { path: '/scalar', body: '', contentType: null }, + { path: '/array', body: '', contentType: null }, + { path: '/plain', body: '', contentType: null }, + ]) + }) + + test('boolean string coercion accepts true and false only', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean' }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + '/users/{active}': { + get: { + operationId: 'getUsersByActive', + parameters: [ + { + name: 'active', + in: 'path', + required: true, + schema: { type: 'boolean' }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + '/defaulted': { + get: { + operationId: 'listDefaulted', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean', default: false }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const query = command(commands, 'listUsers').options! + const args = command(commands, 'getUsersByActive').args! + const defaulted = command(commands, 'listDefaulted').options! + + expect(query.parse({ active: 'true' }).active).toBe(true) + expect(query.parse({ active: 'false' }).active).toBe(false) + expect(query.safeParse({ active: 'yes' }).success).toBe(false) + expect(args.parse({ active: 'true' }).active).toBe(true) + expect(args.parse({ active: 'false' }).active).toBe(false) + expect(args.safeParse({ active: 'yes' }).success).toBe(false) + expect(defaulted.parse({}).active).toBe(false) + expect(defaulted.parse({ active: 'true' }).active).toBe(true) + expect(defaulted.safeParse({ active: 'yes' }).success).toBe(false) + }) + + test('boolean enum query options parse as flags', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean', enum: [true, false] }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const options = command(commands, 'listUsers').options! + const help = Help.formatCommand('api listUsers', { options }) + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + { options }, + ['api', '--active', ''], + 2, + ) + + expect(help).toContain('--active') + expect(help).not.toContain('--active <') + expect(candidates.map((c) => c.value)).toContain('status') + expect(Parser.parse(['--active'], { options }).options).toEqual({ active: true }) + expect(Parser.parse(['--no-active'], { options }).options).toEqual({ active: false }) + expect(Parser.parse(['--active=false'], { options }).options).toEqual({ active: false }) + expect(options.safeParse({ active: 'yes' }).success).toBe(false) + }) + + test('single-value boolean enum query options do not parse as general flags', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean', enum: [true] }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const options = command(commands, 'listUsers').options! + + expect(() => Parser.parse(['--active'], { options })).toThrow( + 'Missing value for flag: --active', + ) + expect(() => Parser.parse(['--no-active'], { options })).toThrow( + 'Flag does not support negation: --no-active', + ) + }) + + test('mixed literal enum query options do not collapse to booleans', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'active', + in: 'query', + schema: { enum: [true, false, 'auto'] }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const options = command(commands, 'listUsers').options! + + expect(options.parse({ active: 'auto' }).active).toBe('auto') + expect(Parser.parse(['--active', 'auto'], { options }).options).toEqual({ active: 'auto' }) + expect(() => Parser.parse(['--active'], { options })).toThrow( + 'Missing value for flag: --active', + ) + }) + + test('boolean query options are rendered as boolean flags in help', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [{ name: 'active', in: 'query', schema: { type: 'boolean' } }], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + + const output = Help.formatCommand('api listUsers', { + options: command(commands, 'listUsers').options, + }) + + expect(output).toContain('--active') + expect(output).not.toContain('--active <') + }) + + test('boolean query options do not consume the next completion word', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [{ name: 'active', in: 'query', schema: { type: 'boolean' } }], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const root = { options: command(commands, 'listUsers').options } + const candidates = Completions.complete( + new Map([['status', { description: 'Status' }]]), + root, + ['api', '--active', ''], + 2, + ) + + expect(candidates.map((c) => c.value)).toContain('status') + }) + + test('generated boolean query options parse as flags without marker metadata', async () => { + const commands = await Openapi.generateCommands( + { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [{ name: 'active', in: 'query', schema: { type: 'boolean' } }], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + app.fetch, + ) + const options = command(commands, 'listUsers').options! + + expect(Parser.parse(['--active'], { options }).options).toEqual({ active: true }) + expect(Parser.parse(['--no-active'], { options }).options).toEqual({ active: false }) + expect(Parser.parse(['--active=false'], { options }).options).toEqual({ active: false }) + + const jsonSchema = Schema.toJsonSchema(options) + expect(jsonSchema).toEqual({ + type: 'object', + properties: { active: { type: 'boolean' } }, + additionalProperties: false, + }) + expect(JSON.stringify(jsonSchema)).not.toContain('openapiStrictBoolean') + }) }) describe('cli integration', () => { @@ -373,6 +1088,131 @@ describe('cli integration', () => { const { exitCode } = await serve(createCli(), ['api', 'getUser']) expect(exitCode).toBe(1) }) + + test('generated OpenAPI boolean query option behaves like a CLI flag', async () => { + const createCli = () => + Cli.create('test', { description: 'test' }).command('api', { + fetch(req) { + const active = new URL(req.url).searchParams.get('active') + return Response.json({ active: active === null ? null : active === 'true' }) + }, + openapi: { + paths: { + '/users': { + get: { + operationId: 'listUsers', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean' }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + '/defaulted': { + get: { + operationId: 'listDefaulted', + parameters: [ + { + name: 'active', + in: 'query', + schema: { type: 'boolean', default: false }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + }) + + expect( + json((await serve(createCli(), ['api', 'listUsers', '--active', '--format', 'json'])).output), + ).toEqual({ active: true }) + expect( + json( + (await serve(createCli(), ['api', 'listUsers', '--no-active', '--format', 'json'])).output, + ), + ).toEqual({ active: false }) + expect( + json( + (await serve(createCli(), ['api', 'listUsers', '--active=false', '--format', 'json'])) + .output, + ), + ).toEqual({ active: false }) + expect((await serve(createCli(), ['api', 'listUsers', '--active=yes'])).exitCode).toBe(1) + expect((await serve(createCli(), ['api', 'listDefaulted', '--active=yes'])).exitCode).toBe(1) + }) + + test('generated OpenAPI boolean body option rejects non-boolean strings', async () => { + const createCli = () => + Cli.create('test', { description: 'test' }).command('api', { + async fetch(req) { + return Response.json(await req.json()) + }, + openapi: { + paths: { + '/users': { + post: { + operationId: 'createUser', + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + active: { type: 'boolean', default: false }, + }, + }, + }, + }, + }, + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + }) + + expect( + json( + (await serve(createCli(), ['api', 'createUser', '--active', '--format', 'json'])).output, + ), + ).toEqual({ active: true }) + expect((await serve(createCli(), ['api', 'createUser', '--active=yes'])).exitCode).toBe(1) + }) + + test('generated path params follow path-template order in help and CLI parsing', async () => { + const cli = Cli.create('test', { description: 'test' }).command('api', { + fetch(req) { + return Response.json({ path: new URL(req.url).pathname }) + }, + openapi: { + paths: { + '/users/{userId}/repos/{repoId}': { + get: { + operationId: 'getRepo', + parameters: [ + { name: 'repoId', in: 'path', required: true, schema: { type: 'string' } }, + { name: 'userId', in: 'path', required: true, schema: { type: 'string' } }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }, + }) + + expect((await serve(cli, ['api', 'getRepo', '--help'])).output).toContain( + 'Usage: test api getRepo ', + ) + expect( + json((await serve(cli, ['api', 'getRepo', 'alice', 'toolkit', '--format', 'json'])).output), + ).toEqual({ path: '/users/alice/repos/toolkit' }) + }) }) describe('@hono/zod-openapi integration', () => { diff --git a/src/Openapi.ts b/src/Openapi.ts index 44c593f..25b6c1a 100644 --- a/src/Openapi.ts +++ b/src/Openapi.ts @@ -44,13 +44,29 @@ export type Commands< : {} type OperationCommandName = item extends object - ? { - [method in keyof item & string]: method extends OperationMethod - ? item[method] extends { operationId: infer id extends string } + ? MethodCommandName | AdditionalOperationCommandName + : never + +type MethodCommandName = { + [method in keyof item & string]: method extends OperationMethod + ? item[method] extends { operationId: infer id extends string } + ? `${name} ${id}` + : `${name} ${method} ${string}` + : never +}[keyof item & string] + +type AdditionalOperationCommandName = item extends { + additionalOperations: infer operations +} + ? operations extends object + ? { + [method in keyof operations & string]: operations[method] extends { + operationId: infer id extends string + } ? `${name} ${id}` : `${name} ${method} ${string}` - : never - }[keyof item & string] + }[keyof operations & string] + : never : never type OperationMethod = @@ -152,9 +168,11 @@ type CommandSegment = { } type OperationEntry = { + httpMethod: string method: string operation: Operation path: string + pathParameters: readonly Parameter[] } function addEntry(paths: NonNullable, segments: string[], entry: any) { @@ -392,7 +410,7 @@ export function generateCommandsSync( const namespaceInfo = getNamespaceInfo(operations) const { config } = options - for (const { method, operation: op, path } of operations) { + for (const { httpMethod, method, operation: op, path, pathParameters } of operations) { const segments = commandSegments({ method, mode: config?.mode ?? 'operation', @@ -400,15 +418,21 @@ export function generateCommandsSync( operation: op, path, }) - const httpMethod = method.toUpperCase() - const pathParams = (op.parameters ?? []).filter((p) => p.in === 'path') - const queryParams = (op.parameters ?? []).filter((p) => p.in === 'query') + const parameters = mergeParameters(pathParameters, op.parameters ?? []) + const pathParams = orderPathParameters( + path, + parameters.filter((p) => p.in === 'path'), + ) + const queryParams = parameters.filter((p) => p.in === 'query') const bodySchema = op.requestBody?.content?.['application/json']?.schema const bodyProps = (bodySchema?.properties ?? {}) as Record> const bodyRequired = new Set((bodySchema?.required as string[]) ?? []) const outputSchema = responseSchema(op.responses) + const bodyKeys = Object.keys(bodyProps) + const bodyIsObject = bodySchema?.type === 'object' || bodySchema?.properties !== undefined + const requestBodyRequired = op.requestBody?.required === true // Build args Zod schema from path params let argsSchema: z.ZodObject | undefined @@ -433,10 +457,19 @@ export function generateCommandsSync( } for (const [key, schema] of Object.entries(bodyProps)) { let zodType = toZod(schema) - if (!bodyRequired.has(key)) zodType = zodType.optional() + if (!requestBodyRequired) zodType = withoutDefault(zodType) + zodType = coerceIfNeeded(zodType) + if (!requestBodyRequired || !bodyRequired.has(key)) zodType = zodType.optional() optShape[key] = zodType } - const optionsSchema = Object.keys(optShape).length > 0 ? z.object(optShape) : undefined + const optionsSchema = + Object.keys(optShape).length > 0 + ? refineOptionalBody(z.object(optShape), { + bodyKeys, + bodyRequired, + requestBodyRequired, + }) + : undefined setCommand(commands, segments, { description: op.summary ?? op.description, @@ -451,6 +484,8 @@ export function generateCommandsSync( pathParams, queryParams, bodyProps, + bodyIsObject, + requestBodyRequired, }), }) } @@ -476,15 +511,17 @@ const openapiMethods = new Set([ 'patch', 'post', 'put', + 'query', 'trace', ]) function openapiOperations(paths: Record>) { const operations: OperationEntry[] = [] - for (const [path, methods] of Object.entries(paths)) - for (const [method, operation] of Object.entries(methods)) - if (openapiMethods.has(method)) - operations.push({ method, operation: operation as Operation, path }) + for (const [path, item] of Object.entries(paths)) { + const pathParameters = (item.parameters ?? []) as Parameter[] + for (const [method, httpMethod, operation] of operationEntries(item)) + operations.push({ httpMethod, method, operation, path, pathParameters }) + } return operations } @@ -504,8 +541,7 @@ function getNamespaceInfo(operations: OperationEntry[]) { function commandSegments(options: commandSegments.Options): CommandSegment[] { const { method, mode, namespaceInfo, operation, path } = options - if (mode === 'operation') - return [{ name: operation.operationId ?? `${method}_${path.replace(/[/{}]/g, '_')}` }] + if (mode === 'operation') return [{ name: operation.operationId ?? fallbackName(method, path) }] const segments = namespaceSegments(path, operation) const needsMethod = @@ -569,6 +605,40 @@ function isCommandSegment(segment: CommandSegment | undefined): segment is Comma return segment !== undefined } +type PathItem = { + additionalOperations?: Record | undefined + parameters?: Parameter[] | undefined + [key: string]: unknown +} + +function operationEntries(item: Record): [string, string, Operation][] { + const entries: [string, string, Operation][] = [] + for (const [method, operation] of Object.entries(item)) + if (openapiMethods.has(method)) + entries.push([method, method.toUpperCase(), operation as Operation]) + for (const [method, operation] of Object.entries((item as PathItem).additionalOperations ?? {})) + entries.push([method, method, operation]) + return entries +} + +function mergeParameters( + pathParameters: readonly Parameter[], + operationParameters: readonly Parameter[], +) { + const parameters = new Map() + for (const p of pathParameters) parameters.set(`${p.in}:${p.name}`, p) + for (const p of operationParameters) parameters.set(`${p.in}:${p.name}`, p) + return [...parameters.values()] +} + +function orderPathParameters(path: string, parameters: Parameter[]) { + const order = new Map() + for (const match of path.matchAll(/\{([^}]+)\}/g)) order.set(match[1]!, order.size) + return parameters.sort( + (a, b) => (order.get(a.name) ?? Infinity) - (order.get(b.name) ?? Infinity), + ) +} + function describeNamespaceLeaf( segments: CommandSegment[], description: string | undefined, @@ -613,12 +683,14 @@ function getGroup(commands: Map, segment: CommandSegment function createHandler(config: { basePath?: string | undefined + bodyIsObject: boolean bodyProps: Record> fetch: FetchHandler httpMethod: string path: string pathParams: Parameter[] queryParams: Parameter[] + requestBodyRequired: boolean }) { return async (context: any) => { const { args = {}, options = {} } = context @@ -627,7 +699,8 @@ function createHandler(config: { let urlPath = (config.basePath ?? '') + config.path for (const p of config.pathParams) { const value = args[p.name] - if (value !== undefined) urlPath = urlPath.replace(`{${p.name}}`, String(value)) + if (value !== undefined) + urlPath = urlPath.replace(`{${p.name}}`, encodeURIComponent(String(value))) } // Build query string from query params @@ -640,10 +713,11 @@ function createHandler(config: { // Build body from body properties let body: string | undefined const bodyKeys = Object.keys(config.bodyProps) - if (bodyKeys.length > 0) { + if (bodyKeys.length > 0 || (config.requestBodyRequired && config.bodyIsObject)) { const bodyObj: Record = {} for (const key of bodyKeys) if (options[key] !== undefined) bodyObj[key] = options[key] - if (Object.keys(bodyObj).length > 0) body = JSON.stringify(bodyObj) + if (Object.keys(bodyObj).length > 0 || (config.requestBodyRequired && config.bodyIsObject)) + body = JSON.stringify(bodyObj) } const input: Fetch.FetchInput = { @@ -683,30 +757,81 @@ function toZod(schema: Record): z.ZodType { /** Wraps a Zod schema with coercion if the base type is number or boolean (argv is always strings). */ function coerceIfNeeded(schema: z.ZodType): z.ZodType { const isOptional = schema instanceof z.ZodOptional - const inner = isOptional ? schema.unwrap() : schema + const withoutOptional = isOptional ? schema.unwrap() : schema + const hasDefault = withoutOptional instanceof z.ZodDefault + const inner = hasDefault ? withoutOptional.unwrap() : withoutOptional - const coerced = (() => { + const coerced: z.ZodType | undefined = (() => { // Direct number - if (inner instanceof z.ZodNumber) - return isOptional ? z.coerce.number().optional() : z.coerce.number() + if (inner instanceof z.ZodNumber) return z.coerce.number() // Direct boolean - if (inner instanceof z.ZodBoolean) - return isOptional ? z.coerce.boolean().optional() : z.coerce.boolean() + if (inner instanceof z.ZodBoolean) return strictBoolean() // Union containing number or boolean (e.g. type: ["number", "null"] from OpenAPI 3.1) if (inner instanceof z.ZodUnion) { const options = (inner as any)._zod?.def?.options as z.ZodType[] | undefined - if (options?.some((o: z.ZodType) => o instanceof z.ZodNumber)) - return isOptional ? z.coerce.number().optional() : z.coerce.number() - if (options?.some((o: z.ZodType) => o instanceof z.ZodBoolean)) - return isOptional ? z.coerce.boolean().optional() : z.coerce.boolean() + if (options?.some((o: z.ZodType) => o instanceof z.ZodNumber)) return z.coerce.number() + if (options?.some((o: z.ZodType) => o instanceof z.ZodBoolean)) return strictBoolean() + const booleanValues = new Set( + options?.flatMap((o) => + o instanceof z.ZodLiteral + ? (((o as any)._zod?.def?.values ?? []) as unknown[]).filter( + (value) => typeof value === 'boolean', + ) + : [], + ), + ) + if ( + options?.every((o) => o instanceof z.ZodLiteral) && + options.length === booleanValues.size && + booleanValues.has(true) && + booleanValues.has(false) + ) + return strictBoolean() } // No coercion needed return undefined })() if (!coerced) return schema + let wrapped: z.ZodType = coerced + if (hasDefault) wrapped = (wrapped as any).default((withoutOptional as any)._zod.def.defaultValue) + if (isOptional) wrapped = wrapped.optional() const desc = (schema as any).description ?? (inner as any).description - return desc ? coerced.describe(desc) : coerced + return desc ? wrapped.describe(desc) : wrapped +} + +function withoutDefault(schema: z.ZodType): z.ZodType { + if (schema instanceof z.ZodDefault) return schema.unwrap() as z.ZodType + return schema +} + +function refineOptionalBody( + schema: z.ZodObject, + options: { + bodyKeys: string[] + bodyRequired: Set + requestBodyRequired: boolean + }, +) { + const { bodyKeys, bodyRequired, requestBodyRequired } = options + if (requestBodyRequired || bodyKeys.length === 0 || bodyRequired.size === 0) return schema + + return schema.superRefine((value, ctx) => { + if (!bodyKeys.some((key) => value[key] !== undefined)) return + for (const key of bodyRequired) + if (value[key] === undefined) + ctx.addIssue({ + code: 'custom', + path: [key], + message: 'Required when request body is provided', + }) + }) +} + +function strictBoolean() { + return z + .union([z.boolean(), z.enum(['true', 'false']).transform((value) => value === 'true')]) + .pipe(z.boolean()) } function responseSchema(responses: Record | undefined) { @@ -720,3 +845,11 @@ function responseSchema(responses: Record | undefined) { | undefined return response?.content?.['application/json']?.schema } + +function fallbackName(method: string, path: string) { + const words = path + .split('/') + .filter(Boolean) + .map((part) => part.replace(/[{}]/g, '')) + return [method, ...words].join(' ') +} diff --git a/src/Parser.test.ts b/src/Parser.test.ts index db4aa52..5403f1f 100644 --- a/src/Parser.test.ts +++ b/src/Parser.test.ts @@ -214,6 +214,66 @@ describe('parse', () => { expect(result.options).toEqual({ verbose: true }) }) + test('does not run transformed boolean schemas when detecting flags', () => { + let calls = 0 + const flag = z + .union([ + z.boolean(), + z.enum(['true', 'false']).transform((value) => { + calls++ + return value === 'true' + }), + ]) + .pipe(z.boolean()) + + const result = Parser.parse(['--flag'], { + options: z.object({ flag }), + }) + + expect(result.options).toEqual({ flag: true }) + expect(calls).toBe(0) + }) + + test('does not treat arbitrary boolean preprocessors as boolean flags', () => { + const debug = z.preprocess((value) => { + if (value === 'yes') return true + if (value === 'no') return false + return 'invalid' + }, z.boolean()) + + expect(() => + Parser.parse(['--debug'], { + options: z.object({ debug }), + }), + ).toThrow('Missing value for flag: --debug') + }) + + test('does not treat boolean literals as boolean flags', () => { + expect(() => + Parser.parse(['--exact'], { + options: z.object({ exact: z.literal(false).optional() }), + }), + ).toThrow('Missing value for flag: --exact') + expect(() => + Parser.parse(['--no-exact'], { + options: z.object({ exact: z.literal(false).optional() }), + }), + ).toThrow('Flag does not support negation: --no-exact') + }) + + test('does not treat string-parsed booleans as boolean flags', () => { + expect(() => + Parser.parse(['--debug'], { + options: z.object({ debug: z.stringbool().optional() }), + }), + ).toThrow('Missing value for flag: --debug') + expect(() => + Parser.parse(['--no-debug'], { + options: z.object({ debug: z.stringbool().optional() }), + }), + ).toThrow('Flag does not support negation: --no-debug') + }) + test('detects array through z.optional()', () => { const result = Parser.parse(['--label', 'bug', '--label', 'fix'], { options: z.object({ label: z.array(z.string()).optional() }), diff --git a/src/Parser.ts b/src/Parser.ts index 3a601ad..308e93f 100644 --- a/src/Parser.ts +++ b/src/Parser.ts @@ -1,4 +1,4 @@ -import type { z } from 'zod' +import { z } from 'zod' import type { FieldError } from './Errors.js' import { ParseError, ValidationError } from './Errors.js' @@ -25,6 +25,8 @@ export function parse< // --no-flag negation const name = normalizeOptionName(token.slice(5), optionNames) if (!name) throw new ParseError({ message: `Unknown flag: ${token}` }) + if (!isBooleanOption(name, optionsSchema)) + throw new ParseError({ message: `Flag does not support negation: ${token}` }) rawArgvOptions[name] = false i++ } else if (token.startsWith('--')) { @@ -218,7 +220,33 @@ function isBooleanOption(name: string, schema: z.ZodObject | undefined): bo if (!schema) return false const field = schema.shape[name] if (!field) return false - return unwrap(field).constructor.name === 'ZodBoolean' + const inner = unwrap(field) + if (inner.constructor.name === 'ZodBoolean') return true + // Generated OpenAPI booleans accept real booleans plus CLI "true"/"false" strings. + // Check that public schema shape instead of running user validation during introspection. + const input = z.toJSONSchema(inner, { unrepresentable: 'any', io: 'input' }) as { + anyOf?: { enum?: unknown[] | undefined; type?: unknown | undefined }[] | undefined + const?: unknown | undefined + type?: unknown | undefined + } + const output = z.toJSONSchema(inner, { unrepresentable: 'any', io: 'output' }) as { + const?: unknown | undefined + type?: unknown | undefined + } + if ( + input.anyOf?.some((schema) => schema.type === 'boolean') && + input.anyOf?.some( + (schema) => + schema.type === 'string' && + schema.enum?.includes('true') && + schema.enum.includes('false') && + schema.enum.length === 2, + ) && + output.type === 'boolean' && + !('const' in output) + ) + return true + return false } /** Checks if an option is a count type (z.count()). */