diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md index 6cc01fa..613c0db 100644 --- a/.claude/rules/architecture.md +++ b/.claude/rules/architecture.md @@ -42,6 +42,7 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL - `openapi-loader.ts` - `OpenapiLoader`. Loads spec from URL or local file, caches it to `.ocli/specs/.json`, refreshes on demand. Resolves external `$ref` across multi-file specs. - `openapi-to-commands.ts` - `OpenapiToCommands`, `CliCommand`, `CliCommandOption`. Walks the spec, applies include/exclude filters, expands path-level params, resolves local `$ref`, builds command names with optional prefix, expands `enum`/`default`/`nullable`/`oneOf` schema hints for `--help`. - `command-search.ts` - `CommandSearch`. BM25 over `(name, method, path, description, options[].name)`, plus regex fallback. Same engine used by both `ocli commands` and any future agent skill. +- `command-args.ts` - `findUnknownFlags`, `acceptsFreeFormBody`, `formatUnknownFlagsError`. Validates parsed flags of a dynamic command against `CliCommand.options`, suggests the closest declared name, and keeps the free-form body passthrough for body-capable operations whose spec declares no body. - `bm25.ts` - tokenizer + BM25 scorer, no I/O. - `cli.ts` - `ocli` entry point. yargs command tree: `profiles add|remove|list`, `use`, `commands`, and dynamic per-spec commands. Builds the `axios` request from a `CliCommand` + parsed args; injects auth, custom headers, server URL overrides. - `version.ts` - generated by `scripts/generate-version.js` during `prebuild`. Do not edit by hand. @@ -51,7 +52,7 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL 1. **OpenAPI-driven**: commands and their options come from the spec. No hand-maintained registry. 2. **Profiles**: every API connection is named; `profiles.ini` is the source of truth. Global vs local `.ocli/` priority is decided by `ConfigLocator`. 3. **Spec cache**: never re-download a spec on every invocation. Refresh is explicit (`onboard`/profile add or refresh flag). -4. **Pure transform layer**: `bm25.ts`, `openapi-to-commands.ts`, and `command-search.ts` perform no I/O; they take inputs and return outputs. This keeps them trivially unit-testable. +4. **Pure transform layer**: `bm25.ts`, `openapi-to-commands.ts`, `command-search.ts`, and `command-args.ts` perform no I/O; they take inputs and return outputs. This keeps them trivially unit-testable. 5. **Side effects at the edges**: filesystem in `config.ts`/`profile-store.ts`/`openapi-loader.ts`, network in `cli.ts` via `HttpClient`. Inject these via constructors (`fs`, `httpClient`) so tests can swap them. 6. **TypeScript strict**: `strict: true` in `tsconfig.json`. Explicit types for exported functions and public interfaces. 7. **No surprise breaking changes**: every CLI-visible change must be reflected in `README.md`. @@ -61,7 +62,8 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL ``` Layer 0 (pure) bm25.ts, version.ts, types in openapi-to-commands.ts Layer 1 (I/O wrappers) config.ts, profile-store.ts, openapi-loader.ts -Layer 2 (transform) openapi-to-commands.ts (uses Profile), command-search.ts (uses CliCommand + bm25) +Layer 2 (transform) openapi-to-commands.ts (uses Profile), command-search.ts (uses CliCommand + bm25), + command-args.ts (uses CliCommand) Layer 3 (entry) cli.ts (uses everything above; only this layer talks to yargs/axios/process) ``` diff --git a/.cursor/rules/architecture.mdc b/.cursor/rules/architecture.mdc index 115d5a0..62844f6 100644 --- a/.cursor/rules/architecture.mdc +++ b/.cursor/rules/architecture.mdc @@ -43,6 +43,7 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL - `openapi-loader.ts` - `OpenapiLoader`. Loads spec from URL or local file, caches it to `.ocli/specs/.json`, refreshes on demand. Resolves external `$ref` across multi-file specs. - `openapi-to-commands.ts` - `OpenapiToCommands`, `CliCommand`, `CliCommandOption`. Walks the spec, applies include/exclude filters, expands path-level params, resolves local `$ref`, builds command names with optional prefix, expands `enum`/`default`/`nullable`/`oneOf` schema hints for `--help`. - `command-search.ts` - `CommandSearch`. BM25 over `(name, method, path, description, options[].name)`, plus regex fallback. Same engine used by both `ocli commands` and any future agent skill. +- `command-args.ts` - `findUnknownFlags`, `acceptsFreeFormBody`, `formatUnknownFlagsError`. Validates parsed flags of a dynamic command against `CliCommand.options`, suggests the closest declared name, and keeps the free-form body passthrough for body-capable operations whose spec declares no body. - `bm25.ts` - tokenizer + BM25 scorer, no I/O. - `cli.ts` - `ocli` entry point. yargs command tree: `profiles add|remove|list`, `use`, `commands`, and dynamic per-spec commands. Builds the `axios` request from a `CliCommand` + parsed args; injects auth, custom headers, server URL overrides. - `version.ts` - generated by `scripts/generate-version.js` during `prebuild`. Do not edit by hand. @@ -52,7 +53,7 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL 1. **OpenAPI-driven**: commands and their options come from the spec. No hand-maintained registry. 2. **Profiles**: every API connection is named; `profiles.ini` is the source of truth. Global vs local `.ocli/` priority is decided by `ConfigLocator`. 3. **Spec cache**: never re-download a spec on every invocation. Refresh is explicit (`onboard`/profile add or refresh flag). -4. **Pure transform layer**: `bm25.ts`, `openapi-to-commands.ts`, and `command-search.ts` perform no I/O; they take inputs and return outputs. This keeps them trivially unit-testable. +4. **Pure transform layer**: `bm25.ts`, `openapi-to-commands.ts`, `command-search.ts`, and `command-args.ts` perform no I/O; they take inputs and return outputs. This keeps them trivially unit-testable. 5. **Side effects at the edges**: filesystem in `config.ts`/`profile-store.ts`/`openapi-loader.ts`, network in `cli.ts` via `HttpClient`. Inject these via constructors (`fs`, `httpClient`) so tests can swap them. 6. **TypeScript strict**: `strict: true` in `tsconfig.json`. Explicit types for exported functions and public interfaces. 7. **No surprise breaking changes**: every CLI-visible change must be reflected in `README.md`. @@ -62,7 +63,8 @@ HttpClient (axios) -> performs the real HTTP request to API_BASE_URL ``` Layer 0 (pure) bm25.ts, version.ts, types in openapi-to-commands.ts Layer 1 (I/O wrappers) config.ts, profile-store.ts, openapi-loader.ts -Layer 2 (transform) openapi-to-commands.ts (uses Profile), command-search.ts (uses CliCommand + bm25) +Layer 2 (transform) openapi-to-commands.ts (uses Profile), command-search.ts (uses CliCommand + bm25), + command-args.ts (uses CliCommand) Layer 3 (entry) cli.ts (uses everything above; only this layer talks to yargs/axios/process) ``` diff --git a/README.md b/README.md index cb33c42..10ab95f 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,21 @@ ocli commands -p other --query "send message" `--profile` (short `-p`) overrides the profile selected by `ocli use` for this invocation only. It works for both dynamic API commands and `ocli commands`. Place it anywhere after the command name. When omitted, the profile set via `ocli use` is used (falling back to `default`). +### Strict flag validation + +`ocli` refuses to run a command with a flag the spec does not define, instead of dropping it from the request: + +```bash +$ ocli people_vanId_get --vanId 12345678 --expand addresses +Unknown option: --expand (did you mean --$expand?). Run 'ocli people_vanId_get --help' to see available options. +$ echo $? +1 +``` + +The same applies to built-in commands: `ocli commands --qeury pull` exits with `Unknown argument: qeury`. + +One exception is kept on purpose. When an operation accepts a body (`POST`, `PUT`, `PATCH`, `DELETE`) and the spec describes no request body, undeclared flags are still forwarded as JSON body fields — that is the only way to call endpoints whose payload is not documented. As soon as the spec declares body properties or `formData` parameters, those names become the full list of accepted flags. + Or use `npx` without global install: ```bash diff --git a/examples/skill-ocli-api.md b/examples/skill-ocli-api.md index 7d6fa9c..16bf840 100644 --- a/examples/skill-ocli-api.md +++ b/examples/skill-ocli-api.md @@ -77,6 +77,7 @@ ocli commands -p github --query "list pull requests" - All responses are JSON — pipe through `jq` for filtering - Path parameters (like `{id}`) are passed as `--id ` - Required parameters will error if missing +- Undeclared flags are rejected with `Unknown option: --x`, so copy names exactly from `--help` (including a leading `$`) - Use `ocli commands` to list all available commands - Use `--profile ` (or `-p `) to switch profile for a single call without running `ocli use` ```` diff --git a/skills/ocli-api/SKILL.md b/skills/ocli-api/SKILL.md index ebb062d..f9c3553 100644 --- a/skills/ocli-api/SKILL.md +++ b/skills/ocli-api/SKILL.md @@ -82,5 +82,6 @@ ocli repos_get --profile github --owner octocat --repo Hello-World - **Command not found**: re-search with different keywords or use `--regex`. - **Missing required parameter**: run `--help` and add the missing flag. +- **Unknown option**: the flag is not defined by the command; copy the exact name from `--help`, including a leading `$` when the spec uses one (`--$expand`). - **401/403**: check that the profile has a valid token (`ocli profiles show `). - **Spec not loaded**: run `ocli profiles add` again with `--openapi-spec` to refresh cache. diff --git a/src/cli.ts b/src/cli.ts index 148aa6d..614a02e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -9,6 +9,7 @@ import { ProfileStore, Profile } from "./profile-store"; import { OpenapiLoader } from "./openapi-loader"; import { OpenapiToCommands, CliCommand, CliCommandOption } from "./openapi-to-commands"; import { CommandSearch } from "./command-search"; +import { findUnknownFlags, formatUnknownFlagsError } from "./command-args"; import { VERSION } from "./version"; export interface HttpClient { @@ -212,6 +213,11 @@ async function runApiCommand( const { flags } = parseArgs(commandArgs); + const unknownFlags = findUnknownFlags(command, Object.keys(flags)); + if (unknownFlags.length > 0) { + throw new Error(formatUnknownFlagsError(command.name, unknownFlags)); + } + const missingRequired = command.options .filter((opt) => opt.required) .filter((opt) => flags[opt.name] === undefined) @@ -632,6 +638,9 @@ export async function run(argv: string[], options?: RunOptions): Promise { await yargs(argv) .scriptName("ocli") .version(VERSION) + // yargs localizes its built-in messages from the environment locale; the CLI surface is English-only + .locale("en") + .strict() .exitProcess(false) .fail((msg, err) => { if (err) { diff --git a/src/command-args.ts b/src/command-args.ts new file mode 100644 index 0000000..39c13a9 --- /dev/null +++ b/src/command-args.ts @@ -0,0 +1,83 @@ +import { CliCommand } from "./openapi-to-commands"; + +export interface UnknownFlag { + name: string; + suggestion?: string; +} + +const BODY_CAPABLE_METHODS = new Set(["post", "put", "patch", "delete"]); +const MAX_SUGGESTION_DISTANCE = 2; + +export function acceptsFreeFormBody(command: CliCommand): boolean { + const declaresBody = command.options.some( + (opt) => opt.location === "body" || opt.location === "formData" + ); + + return !declaresBody && BODY_CAPABLE_METHODS.has(command.method.toLowerCase()); +} + +export function findUnknownFlags(command: CliCommand, flagNames: string[]): UnknownFlag[] { + if (acceptsFreeFormBody(command)) { + return []; + } + + const knownNames = command.options.map((opt) => opt.name); + const known = new Set(knownNames); + + return flagNames + .filter((name) => !known.has(name)) + .map((name) => { + const suggestion = suggestOptionName(name, knownNames); + return suggestion ? { name, suggestion } : { name }; + }); +} + +export function formatUnknownFlagsError(commandName: string, unknown: UnknownFlag[]): string { + const listed = unknown + .map((flag) => (flag.suggestion ? `--${flag.name} (did you mean --${flag.suggestion}?)` : `--${flag.name}`)) + .join(", "); + const label = unknown.length === 1 ? "Unknown option" : "Unknown options"; + + return `${label}: ${listed}. Run 'ocli ${commandName} --help' to see available options.`; +} + +function suggestOptionName(unknownName: string, knownNames: string[]): string | undefined { + const normalizedUnknown = normalizeOptionName(unknownName); + + const normalizedMatch = knownNames.find((name) => normalizeOptionName(name) === normalizedUnknown); + if (normalizedMatch) { + return normalizedMatch; + } + + let best: { name: string; distance: number } | undefined; + knownNames.forEach((name) => { + const distance = editDistance(unknownName.toLowerCase(), name.toLowerCase()); + if (distance > MAX_SUGGESTION_DISTANCE || distance >= Math.min(unknownName.length, name.length)) { + return; + } + if (!best || distance < best.distance) { + best = { name, distance }; + } + }); + + return best?.name; +} + +function normalizeOptionName(name: string): string { + return name.toLowerCase().replace(/[^a-z0-9]/g, ""); +} + +function editDistance(a: string, b: string): number { + let previous = Array.from({ length: b.length + 1 }, (_, i) => i); + + for (let i = 1; i <= a.length; i += 1) { + const current = [i]; + for (let j = 1; j <= b.length; j += 1) { + const substitution = previous[j - 1] + (a[i - 1] === b[j - 1] ? 0 : 1); + current[j] = Math.min(previous[j] + 1, current[j - 1] + 1, substitution); + } + previous = current; + } + + return previous[b.length]; +} diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 5be9800..26af889 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -1683,4 +1683,170 @@ describe("cli", () => { expect(out).toContain("-p"); }); }); + + describe("unknown flags", () => { + function createExpandApiDeps() { + const localDir = `${cwd}/.ocli`; + const profilesPath = `${localDir}/profiles.ini`; + const cachePath = `${localDir}/specs/expand-api.json`; + + const spec = { + openapi: "3.0.0", + paths: { + "/widgets/{id}": { + get: { + summary: "Get a widget by id", + parameters: [ + { name: "id", in: "path", required: true, schema: { type: "string" } }, + { name: "$expand", in: "query", required: false, schema: { type: "string" } }, + ], + }, + }, + "/widgets": { + post: { + summary: "Create a widget", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["title"], + properties: { + title: { type: "string" }, + draft: { type: "boolean" }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + + const iniContent = [ + "[expand-api]", + "api_base_url = https://api.example.com", + "api_basic_auth = ", + "api_bearer_token = ", + "openapi_spec_source = /spec.json", + `openapi_spec_cache = ${cachePath}`, + "include_endpoints = ", + "exclude_endpoints = ", + "", + ].join("\n"); + + const capturedConfigs: unknown[] = []; + const fakeHttpClient: HttpClient = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + request: async (config: any) => { + capturedConfigs.push(config); + return { status: 200, statusText: "OK", headers: {}, config, data: { ok: true } }; + }, + }; + + const { profileStore, openapiLoader } = createCliDeps(cwd, homeDir, { + [profilesPath]: iniContent, + [cachePath]: JSON.stringify(spec), + [`${localDir}/current`]: "expand-api", + }); + + return { profileStore, openapiLoader, fakeHttpClient, capturedConfigs }; + } + + it("rejects an undeclared flag instead of silently dropping it", async () => { + const { profileStore, openapiLoader, fakeHttpClient, capturedConfigs } = createExpandApiDeps(); + + await expect( + run(["widgets_id", "--id", "widget-1", "--bogus", "value"], { + cwd, + profileStore, + openapiLoader, + httpClient: fakeHttpClient, + stdout: () => {}, + }) + ).rejects.toThrow("Unknown option: --bogus"); + + expect(capturedConfigs).toHaveLength(0); + }); + + it("suggests the declared option when the $ prefix is missing", async () => { + const { profileStore, openapiLoader, fakeHttpClient, capturedConfigs } = createExpandApiDeps(); + + await expect( + run(["widgets_id", "--id", "widget-1", "--expand", "parts"], { + cwd, + profileStore, + openapiLoader, + httpClient: fakeHttpClient, + stdout: () => {}, + }) + ).rejects.toThrow("did you mean --$expand?"); + + expect(capturedConfigs).toHaveLength(0); + }); + + it("still accepts the correctly spelled $-prefixed option", async () => { + const { profileStore, openapiLoader, fakeHttpClient, capturedConfigs } = createExpandApiDeps(); + + await run(["widgets_id", "--id", "widget-1", "--$expand", "parts"], { + cwd, + profileStore, + openapiLoader, + httpClient: fakeHttpClient, + stdout: () => {}, + }); + + const config = capturedConfigs[0] as { url: string }; + expect(config.url).toBe("https://api.example.com/widgets/widget-1?%24expand=parts"); + }); + + it("rejects an undeclared flag when the spec describes the request body", async () => { + const { profileStore, openapiLoader, fakeHttpClient, capturedConfigs } = createExpandApiDeps(); + + await expect( + run(["widgets", "--title", "Widget", "--drafts", "true"], { + cwd, + profileStore, + openapiLoader, + httpClient: fakeHttpClient, + stdout: () => {}, + }) + ).rejects.toThrow("Unknown option: --drafts (did you mean --draft?)"); + + expect(capturedConfigs).toHaveLength(0); + }); + + it("keeps forwarding undeclared flags as body fields when the spec describes no request body", async () => { + const { profileStore, openapiLoader, fakeHttpClient, capturedConfigs } = createPostApiDeps(); + + await run( + [ + "org_slug_repo_slug_ci_workflows_workflow_name_trigger", + "--org_slug", "myorg", + "--repo_slug", "myrepo", + "--workflow_name", "deploy", + "--revision", "main", + ], + { cwd, profileStore, openapiLoader, httpClient: fakeHttpClient, stdout: () => {} } + ); + + const config = capturedConfigs[0] as { data: Record }; + expect(config.data.revision).toBe("main"); + }); + + it("rejects an unknown flag on the commands subcommand", async () => { + const { profileStore, openapiLoader } = createExpandApiDeps(); + + await expect( + run(["commands", "--qeury", "widget"], { + cwd, + profileStore, + openapiLoader, + stdout: () => {}, + }) + ).rejects.toThrow("Unknown argument: qeury"); + }); + }); }); diff --git a/tests/command-args.test.ts b/tests/command-args.test.ts new file mode 100644 index 0000000..c5b910b --- /dev/null +++ b/tests/command-args.test.ts @@ -0,0 +1,143 @@ +import { acceptsFreeFormBody, findUnknownFlags, formatUnknownFlagsError } from "../src/command-args"; +import { CliCommand } from "../src/openapi-to-commands"; + +function makeCommand(overrides: Partial): CliCommand { + return { + name: "widgets_id", + method: "get", + path: "/widgets/{id}", + options: [], + ...overrides, + }; +} + +const getWithExpand = makeCommand({ + options: [ + { name: "id", location: "path", required: true, schemaType: "string" }, + { name: "$expand", location: "query", required: false, schemaType: "string" }, + ], +}); + +describe("command-args", () => { + describe("acceptsFreeFormBody", () => { + it("is true for a POST command that declares no body options", () => { + const command = makeCommand({ + method: "post", + options: [{ name: "org_slug", location: "path", required: true, schemaType: "string" }], + }); + + expect(acceptsFreeFormBody(command)).toBe(true); + }); + + it("is false for a GET command", () => { + expect(acceptsFreeFormBody(getWithExpand)).toBe(false); + }); + + it("is false for a POST command with a declared request body schema", () => { + const command = makeCommand({ + method: "post", + options: [{ name: "event_type", location: "body", required: true, schemaType: "string" }], + }); + + expect(acceptsFreeFormBody(command)).toBe(false); + }); + + it("is false for a POST command with declared formData parameters", () => { + const command = makeCommand({ + method: "post", + options: [{ name: "title", location: "formData", required: true, schemaType: "string" }], + }); + + expect(acceptsFreeFormBody(command)).toBe(false); + }); + }); + + describe("findUnknownFlags", () => { + it("reports a flag that the command does not declare", () => { + const unknown = findUnknownFlags(getWithExpand, ["id", "bogus"]); + + expect(unknown).toEqual([{ name: "bogus" }]); + }); + + it("suggests the declared option when only the $ prefix is missing", () => { + const unknown = findUnknownFlags(getWithExpand, ["id", "expand"]); + + expect(unknown).toEqual([{ name: "expand", suggestion: "$expand" }]); + }); + + it("suggests the declared option for a small typo", () => { + const command = makeCommand({ + options: [{ name: "workflow_name", location: "query", required: false, schemaType: "string" }], + }); + + const unknown = findUnknownFlags(command, ["workflow_nme"]); + + expect(unknown).toEqual([{ name: "workflow_nme", suggestion: "workflow_name" }]); + }); + + it("accepts declared options in every parameter location", () => { + const command = makeCommand({ + options: [ + { name: "id", location: "path", required: true, schemaType: "string" }, + { name: "limit", location: "query", required: false, schemaType: "integer" }, + { name: "X-Request-Id", location: "header", required: false, schemaType: "string" }, + { name: "session_id", location: "cookie", required: false, schemaType: "string" }, + ], + }); + + expect(findUnknownFlags(command, ["id", "limit", "X-Request-Id", "session_id"])).toEqual([]); + }); + + it("allows undeclared flags when the command carries a free-form request body", () => { + const command = makeCommand({ + method: "post", + options: [{ name: "org_slug", location: "path", required: true, schemaType: "string" }], + }); + + expect(findUnknownFlags(command, ["org_slug", "revision", "tags"])).toEqual([]); + }); + + it("reports undeclared flags when the command declares body properties", () => { + const command = makeCommand({ + method: "post", + options: [ + { name: "event_type", location: "body", required: true, schemaType: "string" }, + { name: "draft", location: "body", required: false, schemaType: "boolean" }, + ], + }); + + expect(findUnknownFlags(command, ["event_type", "drafts"])).toEqual([ + { name: "drafts", suggestion: "draft" }, + ]); + }); + + it("reports every unknown flag, in the order they were passed", () => { + const unknown = findUnknownFlags(getWithExpand, ["typo", "id", "bogus"]); + + expect(unknown.map((flag) => flag.name)).toEqual(["typo", "bogus"]); + }); + }); + + describe("formatUnknownFlagsError", () => { + it("mentions the flag and points at the command help", () => { + const message = formatUnknownFlagsError("widgets_id", [{ name: "bogus" }]); + + expect(message).toContain("--bogus"); + expect(message).toContain("ocli widgets_id --help"); + }); + + it("includes the suggestion when one is available", () => { + const message = formatUnknownFlagsError("widgets_id", [{ name: "expand", suggestion: "$expand" }]); + + expect(message).toContain("--expand"); + expect(message).toContain("--$expand"); + }); + + it("lists all unknown flags at once", () => { + const message = formatUnknownFlagsError("widgets_id", [{ name: "bogus" }, { name: "typo" }]); + + expect(message).toContain("--bogus"); + expect(message).toContain("--typo"); + }); + }); +});