From a118fd0c89affd1994a9a9e22837c684f5bc4888 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 09:19:12 -0700 Subject: [PATCH] fix(make-pdf): boolean flags no longer swallow the next positional argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2514. The parser treated any non-flag token after a flag as its value, so `$P generate --toc essay.md` ate essay.md as --toc's value and failed with "missing input" — the skill's own documented usage only worked when two boolean flags happened to be adjacent. BOOLEAN_FLAGS enumerates the no-value flags; value flags (--watermark, --to, --title, ...) are unchanged. main() now runs behind import.meta.main so tests import the parser directly. Co-Authored-By: Claude Fable 5 --- make-pdf/src/cli.ts | 22 +++++++++++++--- make-pdf/test/cli-args.test.ts | 47 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 make-pdf/test/cli-args.test.ts diff --git a/make-pdf/src/cli.ts b/make-pdf/src/cli.ts index 988e20444..b39ce6013 100644 --- a/make-pdf/src/cli.ts +++ b/make-pdf/src/cli.ts @@ -20,7 +20,20 @@ interface ParsedArgs { flags: Record; } -function parseArgs(argv: string[]): ParsedArgs { +/** + * Flags that never take a value (#2514). The parser used to treat ANY + * following non-flag token as the flag's value, so the skill's own + * documented usage — `$P generate --cover --toc essay.md essay.pdf` — worked + * only by luck of flag adjacency, while `$P generate --toc essay.md` ate + * `essay.md` as --toc's value and failed with "missing input". + */ +export const BOOLEAN_FLAGS = new Set([ + "cover", "toc", "no-chapter-breaks", "no-confidential", + "page-numbers", "no-page-numbers", "tagged", "no-tagged", + "outline", "no-outline", "quiet", "verbose", "allow-network", +]); + +export function parseArgs(argv: string[]): ParsedArgs { const args = argv.slice(2); if (args.length === 0) { printUsage(); @@ -37,7 +50,7 @@ function parseArgs(argv: string[]): ParsedArgs { if (a.startsWith("--")) { const key = a.slice(2); const next = args[i + 1]; - if (next !== undefined && !next.startsWith("--")) { + if (!BOOLEAN_FLAGS.has(key) && next !== undefined && !next.startsWith("--")) { flags[key] = next; i++; } else { @@ -272,4 +285,7 @@ async function main(): Promise { } } -main(); +// Guarded so tests can import parseArgs/BOOLEAN_FLAGS without running the CLI. +if (import.meta.main) { + main(); +} diff --git a/make-pdf/test/cli-args.test.ts b/make-pdf/test/cli-args.test.ts new file mode 100644 index 000000000..66fa69ed2 --- /dev/null +++ b/make-pdf/test/cli-args.test.ts @@ -0,0 +1,47 @@ +/** + * #2514: boolean flags (--toc, --cover, ...) must never swallow the next + * positional argument. The parser treated ANY following non-flag token as a + * flag value, so `$P generate --toc essay.md` ate essay.md as --toc's value + * and the skill's own documented invocations failed with "missing input". + */ + +import { describe, test, expect } from "bun:test"; +import { parseArgs, BOOLEAN_FLAGS } from "../src/cli"; + +// parseArgs slices argv from index 2 (node/bun + script path). +const parse = (...args: string[]) => parseArgs(["bun", "cli.ts", ...args]); + +describe("#2514 boolean flags do not swallow positionals", () => { + test("--toc before the input keeps the input positional", () => { + const r = parse("generate", "--toc", "essay.md"); + expect(r.command).toBe("generate"); + expect(r.flags.toc).toBe(true); + expect(r.positional).toEqual(["essay.md"]); + }); + + test("the skill's documented usage parses: --cover --toc essay.md essay.pdf", () => { + const r = parse("generate", "--cover", "--toc", "essay.md", "essay.pdf"); + expect(r.flags.cover).toBe(true); + expect(r.flags.toc).toBe(true); + expect(r.positional).toEqual(["essay.md", "essay.pdf"]); + }); + + test("value flags still consume their value", () => { + const r = parse("generate", "--watermark", "DRAFT", "memo.md"); + expect(r.flags.watermark).toBe("DRAFT"); + expect(r.positional).toEqual(["memo.md"]); + }); + + test("--to consumes its format value", () => { + const r = parse("generate", "--to", "html", "doc.md"); + expect(r.flags.to).toBe("html"); + expect(r.positional).toEqual(["doc.md"]); + }); + + test("every registered boolean flag is covered by the set", () => { + // The generate command's no-value flags per commands.ts + usage text. + for (const f of ["cover", "toc", "no-chapter-breaks", "quiet", "verbose", "allow-network"]) { + expect(BOOLEAN_FLAGS.has(f)).toBe(true); + } + }); +});