diff --git a/.release/cli-boolean-negation.md b/.release/cli-boolean-negation.md new file mode 100644 index 0000000..6934e8f --- /dev/null +++ b/.release/cli-boolean-negation.md @@ -0,0 +1,4 @@ +--- +bump: minor +--- +`parseArgs` generates a `--no-` negation for every boolean input, so a `z.boolean().default(true)` can be turned off from the CLI without declaring a second override flag. The negation takes no value and consumes no token (`--no-loud=false` is rejected; `myverb --no-loud alice` still reads `alice` as a positional), and booleans stay scalar — `--loud --no-loud` is last-wins. The derived names live in a flag-only set: `no-` is not an input key, so `positionals: ["no-loud"]` remains a spec error, and an input field that would shadow a generated negation is rejected as one. `toHelp` renders booleans as `--loud / --no-loud` and now prints each field's default; a defaulted field is no longer also labelled `(required)`, which `z.toJSONSchema` had made it look like. diff --git a/README.md b/README.md index 5813b72..e46566f 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,33 @@ More generally: any argument that maps to nothing the verb declares — an unkno flag, an extra positional, or a positional naming no field — throws. Nothing falls through to a default. +### Boolean flags and `--no-` + +Every boolean input gets both spellings: `--loud` sets it true, `--no-loud` sets +it false. So a boolean that is *on by default* can be turned off from the command +line with the one field that declares it: + +```ts +input: z.object({ changedOnly: z.boolean().default(true) }), +// `check` → changedOnly: true (the default) +// `check --no-changedOnly` → changedOnly: false +``` + +- **The negation is a derived CLI name, not an input field.** `no-changedOnly` is + not a key of `input`, so it is not selectable as a positional — + `positionals: ["no-changedOnly"]` remains a spec error. An input field that + would shadow a generated negation (a boolean `loud` beside a field literally + named `no-loud`) is a spec error too, rather than a flag that quietly means + only one of the two things. +- **It takes no value and consumes no token.** `--no-loud=false` is rejected, and + `myverb --no-loud alice` reads `alice` as a positional, not as the flag's value. +- **Booleans stay scalar.** `--loud --no-loud` is last-wins, the same rule every + other scalar flag follows. +- **Both defaults get one.** `--no-` is generated for every boolean, not only the + `default(true)` ones — which spelling an author needs follows from the default, + and a default is a value the verb may change. Generating it conditionally would + mean flipping a default silently deletes a flag from every script using it. + ## Design - **One spec, four surfaces.** The CLI, MCP server, Anthropic tool schema, and diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 77d3fae..f922bdc 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -1,7 +1,14 @@ import { describe, expect, test } from "bun:test"; import { z } from "zod"; -import { defineVerb, dispatch, parseArgs, toMcpTool, toOpenApiPaths } from "@bounded-systems/verbspec"; +import { + defineVerb, + dispatch, + parseArgs, + toHelp, + toMcpTool, + toOpenApiPaths, +} from "@bounded-systems/verbspec"; // parseArgs CLI-isms for array-typed input fields: repeated flags accumulate, // comma-separated values split, and the two forms compose. Scalars take the @@ -313,3 +320,155 @@ describe("dispatch multi-word ids", () => { expect(out).toMatchObject({ kind: "ok", id: "plan", output: { kind: "plan" } }); }); }); + +// ── boolean negation (#12) ─────────────────────────────────────────────────── +// `z.boolean().default(true)` had no way to be turned OFF from the CLI: only +// presence-is-true worked, so "on by default, opt out on the CLI" forced authors +// to declare a SECOND override flag and compute the effective value by hand — +// two flags for one boolean dimension. `--no-` is that missing spelling. +const negVerb = defineVerb({ + id: "neg-probe", + summary: "test-only verb with a default-true boolean", + actor: "work", + positionals: [], + input: z.object({ + changedOnly: z.boolean().default(true), + dryRun: z.boolean().default(false), + slug: z.string().optional(), + }), + output: z.object({}), + run: () => ({}), +}); + +const negPos = defineVerb({ + id: "neg-pos-probe", + summary: "test-only verb with a positional beside a boolean", + actor: "work", + positionals: ["name"], + input: z.object({ name: z.string(), loud: z.boolean().default(true) }), + output: z.object({}), + run: () => ({}), +}); + +describe("parseArgs boolean negation", () => { + test("--no- turns a default(true) boolean off", () => { + expect(parseArgs(negVerb, ["--no-changedOnly"])).toMatchObject({ changedOnly: false }); + }); + + test("the default still applies when the negation is absent", () => { + expect(parseArgs(negVerb, [])).toMatchObject({ changedOnly: true }); + }); + + test("every boolean gets one, not only the default(true) ones", () => { + expect(parseArgs(negVerb, ["--dryRun", "--no-dryRun"])).toMatchObject({ dryRun: false }); + }); + + test("a boolean stays scalar — the last spelling wins in either order", () => { + expect(parseArgs(negVerb, ["--no-changedOnly", "--changedOnly"])).toMatchObject({ + changedOnly: true, + }); + expect(parseArgs(negVerb, ["--changedOnly", "--no-changedOnly"])).toMatchObject({ + changedOnly: false, + }); + }); + + test("the negation carries its own value — it never eats the next token", () => { + expect(parseArgs(negVerb, ["--no-changedOnly", "--slug", "abc"])).toMatchObject({ + changedOnly: false, + slug: "abc", + }); + // The sharper case: a bare token after the negation is still a positional, + // not the flag's value. + expect(parseArgs(negPos, ["--no-loud", "alice"])).toMatchObject({ + name: "alice", + loud: false, + }); + }); + + test("--no-=value is rejected rather than silently ignored", () => { + expect(() => parseArgs(negVerb, ["--no-changedOnly=false"])).toThrow(/takes no value/i); + }); + + test("only booleans get a negation — --no- is an unknown flag", () => { + expect(() => parseArgs(negVerb, ["--no-slug", "x"])).toThrow(/unknown flag|unrecognized/i); + }); + + test("the negation of an undeclared field is an unknown flag", () => { + expect(() => parseArgs(negVerb, ["--no-bogus"])).toThrow(/unknown flag|unrecognized/i); + }); + + test("the unknown-flag message lists the negations among the valid flags", () => { + expect(() => parseArgs(negVerb, ["--nope"])).toThrow(/--no-changedOnly/); + }); + + test("a negation reaches the same field through dispatch", async () => { + const out = await dispatch({ "neg-probe": negVerb }, ["neg-probe", "--no-changedOnly"]); + expect(out).toMatchObject({ kind: "ok", input: { changedOnly: false } }); + }); +}); + +// The negation names are DERIVED CLI spellings, not input fields. Widening the +// one set that gated both questions would have made `positionals: ["no-loud"]` +// legal — it binds a value `input.parse` then strips, the silent-drop this +// package exists to refuse. The two sets stay separate; these hold that line. +describe("parseArgs boolean negation — derived names are not input fields", () => { + test("a negation name in `positionals` is still a spec error", () => { + const negAsPositional = defineVerb({ + id: "neg-as-positional-probe", + summary: "test-only verb naming a negation as a positional", + actor: "work", + positionals: ["no-loud"], + input: z.object({ loud: z.boolean().default(true) }), + output: z.object({}), + run: () => ({}), + }); + expect(() => parseArgs(negAsPositional, [])).toThrow(/no input field/i); + expect(() => parseArgs(negAsPositional, [])).toThrow(/no-loud/); + }); + + test("an input field colliding with a generated negation is a spec error", () => { + const collide = defineVerb({ + id: "neg-collision-probe", + summary: "test-only verb whose field shadows a generated negation", + actor: "work", + input: z.object({ loud: z.boolean().default(true), "no-loud": z.string().optional() }), + output: z.object({}), + run: () => ({}), + }); + expect(() => parseArgs(collide, [])).toThrow(/collides/i); + expect(() => parseArgs(collide, [])).toThrow(/no-loud/); + }); +}); + +describe("toHelp boolean negation", () => { + test("a boolean renders both spellings instead of a value placeholder", () => { + const help = toHelp(negVerb); + expect(help).toContain("--changedOnly / --no-changedOnly"); + expect(help).not.toContain("--changedOnly "); + }); + + test("the default is shown, since it decides which spelling is useful", () => { + const help = toHelp(negVerb); + expect(help).toContain("--changedOnly / --no-changedOnly (default: true)"); + expect(help).toContain("--dryRun / --no-dryRun (default: false)"); + }); + + test("non-boolean flags are unchanged apart from their default", () => { + expect(toHelp(negVerb)).toContain("--slug "); + }); +}); + +// Fallout from showing a boolean's default: `z.toJSONSchema` lists a DEFAULTED +// field in `required` (the parsed output always carries it), so the old renderer +// labelled `--limit` required — and beside a printed default that reads as a +// flat contradiction. The default is the truthful half. +describe("toHelp defaults", () => { + test("a defaulted field is not also labelled required", () => { + expect(toHelp(negVerb)).not.toContain("(required) (default:"); + expect(toHelp(searchNotes)).toContain("--limit (default: 20)"); + }); + + test("a field with no default keeps its required marker", () => { + expect(toHelp(searchNotes)).toContain("--q (required)"); + }); +}); diff --git a/src/index.ts b/src/index.ts index 8fd8ee0..2523c3e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -323,10 +323,50 @@ export async function dispatchNdjson(reg: Registry, line: string): Promise; + properties?: Record; required?: string[]; }; +/** + * The `--no-` negations for a verb's boolean inputs, as a map from the + * derived flag name to the input field it turns off. `--no-changedOnly` is how + * a `z.boolean().default(true)` is opted out of on the command line, so one + * boolean input covers both directions and an author no longer has to declare a + * second override flag to express "on by default". + * + * These names are DERIVED, not declared: `no-changedOnly` is deliberately not an + * input key, so this set must never be folded into the set that gates positional + * names — `positionals: ["no-changedOnly"]` stays the spec error it is today. + * + * EVERY boolean gets one, not only the `default(true)` ones. Which spelling an + * author reaches for follows from the default, but the default is a value the + * verb may change: if `--no-x` existed only while `x` defaulted to true, + * flipping that default would silently delete a flag from every script using it. + */ +const negationsFor = ( + props: Record, + id: string, +): Map => { + const negations = new Map(); + for (const [field, meta] of Object.entries(props)) { + if (meta?.type !== "boolean") continue; + const flag = `no-${field}`; + // An input field literally named `no-x` alongside a boolean `x` shadows the + // negation: `--no-x` binds the declared field and `x` becomes impossible to + // turn off, with no error. Halt on the spec itself — same reason as the + // undeclared-positional check, so it fails on every invocation rather than + // only the ones that happen to pass the flag. + if (props[flag] !== undefined) { + throw new Error( + `invalid spec for '${id}': input field '${flag}' collides with the generated ` + + `negation of boolean field '${field}'. Rename one — '--${flag}' cannot mean both.`, + ); + } + negations.set(flag, field); + } + return negations; +}; + /** Render a {@link VerbSpec} as CLI `--help` text (usage, positionals, flags) for `bin`. */ export function toHelp(v: VerbSpec, bin = "prx"): string { const js = toInputJsonSchema(v) as JsonProps; @@ -338,11 +378,25 @@ export function toHelp(v: VerbSpec, bin = "prx"): string { const lines = [`${bin} ${v.id} ${usagePos}`.trimEnd(), "", ` ${v.summary}`, ""]; if (flags.length) { lines.push("Flags:"); + const negationOf = new Map( + [...negationsFor(props, v.id)].map(([flag, field]) => [field, flag] as const), + ); for (const f of flags) { const meta = props[f] ?? {}; - const req = required.has(f) ? " (required)" : ""; + // `z.toJSONSchema` lists a defaulted field in `required` (the parsed OUTPUT + // always carries it), but on a command line a field with a default is + // exactly the one you may omit. Printing "(required) (default: true)" says + // both at once; the default is the truthful half. + const req = required.has(f) && meta.default === undefined ? " (required)" : ""; const desc = meta.description ? ` — ${meta.description}` : ""; - lines.push(` --${f} <${meta.type ?? "value"}>${req}${desc}`); + const neg = negationOf.get(f); + // Shown for every defaulted field, because dropping the (wrong) "required" + // marker above would otherwise leave them annotated with nothing at all — + // and for a boolean the default is what decides which of the two spellings + // the reader actually needs. + const dflt = meta.default === undefined ? "" : ` (default: ${JSON.stringify(meta.default)})`; + const name = neg ? `--${f} / --${neg}` : `--${f} <${meta.type ?? "value"}>`; + lines.push(` ${name}${req}${desc}${dflt}`); } } return lines.join("\n"); @@ -350,9 +404,10 @@ export function toHelp(v: VerbSpec, bin = "prx"): string { /** * Parse argv into the verb's input, validated by its Zod schema. CLI-isms stay - * here, not in the schemas: `--k v` / `--k=v` / boolean `--flag` / positionals, - * and comma-split for array-typed fields (detected from the JSON Schema). The - * Zod `parse` does coercion (`z.coerce.number`) and is the single validation. + * here, not in the schemas: `--k v` / `--k=v` / boolean `--flag` and its + * `--no-flag` negation / positionals, and comma-split for array-typed fields + * (detected from the JSON Schema). The Zod `parse` does coercion + * (`z.coerce.number`) and is the single validation. */ export function parseArgs( v: VerbSpec, @@ -361,9 +416,14 @@ export function parseArgs( const js = toInputJsonSchema(v) as { properties?: Record }; const props = js.properties ?? {}; const isArray = (key: string) => props[key]?.type === "array"; - // Every declared input key is a valid flag; a `--flag` outside this set maps to - // nothing and must halt (see the strict-mapping block at the end). + // TWO sets, deliberately not one. `known` answers "is this a declared input + // field?" — it gates positional names and supplies the negation targets. + // `knownFlags` answers "is this an accepted flag name?" and is the only one + // carrying the derived `--no-` names, so widening the CLI vocabulary cannot + // quietly make `positionals: ["no-x"]` legal. const known = new Set(Object.keys(props)); + const negations = negationsFor(props, v.id); + const knownFlags = new Set([...known, ...negations.keys()]); const unknownFlags: string[] = []; // `positionals` SELECTS input fields to read positionally — it does not declare @@ -401,7 +461,21 @@ export function parseArgs( if (a.startsWith("--")) { const eq = a.indexOf("="); const key = eq >= 0 ? a.slice(2, eq) : a.slice(2); - if (!known.has(key) && !unknownFlags.includes(key)) unknownFlags.push(key); + if (!knownFlags.has(key) && !unknownFlags.includes(key)) unknownFlags.push(key); + const negated = negations.get(key); + if (negated !== undefined) { + // `--no-x` IS the value; it never consumes the next token, and it never + // accumulates (a boolean is a scalar — `--x --no-x` is last-wins, the + // same rule every other scalar flag follows). + if (eq >= 0) { + throw new Error( + `--${key} takes no value: it sets '${negated}' to false. ` + + `Write '--${key}' on its own, or '--${negated}' to set it true.`, + ); + } + raw[negated] = false; + continue; + } if (eq >= 0) { setRaw(key, a.slice(eq + 1)); } else { @@ -437,10 +511,15 @@ export function parseArgs( // (filter is a `--slug` flag; the verb declares no positionals) became a live // deploy of EVERY card instead of one. Report both leak paths together so a // caller fixes the whole line at once. + const validFlags = [...knownFlags].map((k) => `--${k}`).join(", ") || "(none)"; + // The positional message suggests somewhere to PUT a value, so it names input + // fields only — a `--no-x` accepts none. const valid = [...known].map((k) => `--${k}`).join(", ") || "(none)"; if (unknownFlags.length) { const flags = unknownFlags.map((k) => `--${k}`).join(", "); - throw new Error(`unknown flag(s) for '${v.id}': ${flags}. Valid flags: ${valid}. See --help.`); + throw new Error( + `unknown flag(s) for '${v.id}': ${flags}. Valid flags: ${validFlags}. See --help.`, + ); } // A trailing variadic (array-typed) positional absorbs every remaining value, // so it can never overflow; any other shape caps at the declared count.