diff --git a/packages/cli/src/commands/__tests__/email-logs-limit.test.ts b/packages/cli/src/commands/__tests__/email-logs-limit.test.ts new file mode 100644 index 000000000..426ca6b24 --- /dev/null +++ b/packages/cli/src/commands/__tests__/email-logs-limit.test.ts @@ -0,0 +1,67 @@ +/** + * `wraps email logs list --limit `: the space form and the `=` form must + * reach the API identically. Before the fix, `--limit -1` lost its value to + * the arg parser, so the request silently fell back to the API default. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { parseCliArgs } from "../../utils/shared/arg-parser.js"; + +vi.mock("@clack/prompts"); +vi.mock("../../utils/shared/api-target.js", () => ({ + resolveApiTarget: vi.fn(), + checkApiTarget: vi.fn(), +})); + +import { + checkApiTarget, + resolveApiTarget, +} from "../../utils/shared/api-target.js"; +import { emailLogsList } from "../email/logs.js"; + +const argv = (...args: string[]) => [ + "node", + "wraps", + "email", + "logs", + "list", + ...args, +]; + +/** Mirrors the router in cli.ts: parsed flags are passed straight through. */ +async function runLogsList(...args: string[]): Promise { + const { flags } = parseCliArgs(argv(...args)); + await emailLogsList({ limit: flags.limit, status: flags.status }); + const [url] = vi.mocked(fetch).mock.calls[0] as [string, unknown]; + return new URL(url); +} + +describe("email logs list — --limit with a negative value", () => { + beforeEach(() => { + vi.mocked(resolveApiTarget).mockResolvedValue({} as never); + vi.mocked(checkApiTarget).mockReturnValue({ + ok: true, + target: { apiBase: "https://api.example.test", token: "t" }, + } as never); + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ logs: [], total: 0, nextCursor: null }), + })) + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); + }); + + it("sends the same limit for --limit -1 as for --limit=-1", async () => { + const spaced = await runLogsList("--limit", "-1"); + const attached = await runLogsList("--limit=-1"); + expect(spaced.searchParams.get("limit")).toBe("-1"); + expect(attached.searchParams.get("limit")).toBe("-1"); + }); +}); diff --git a/packages/cli/src/utils/shared/__tests__/arg-parser.test.ts b/packages/cli/src/utils/shared/__tests__/arg-parser.test.ts index ca6e3093b..89f7ed06d 100644 --- a/packages/cli/src/utils/shared/__tests__/arg-parser.test.ts +++ b/packages/cli/src/utils/shared/__tests__/arg-parser.test.ts @@ -241,3 +241,103 @@ describe("parseCliArgs — workflows push slug", () => { expect(extraPositionals).toEqual([]); }); }); + +describe("parseCliArgs — negative numeric values", () => { + const argv = (...args: string[]) => ["node", "wraps", ...args]; + const base = ["email", "logs", "list"]; + const parse = (...rest: string[]) => parseCliArgs(argv(...base, ...rest)); + + it.each([ + ["--limit -1", ["--limit", "-1"], "-1"], + ["--limit -5", ["--limit", "-5"], "-5"], + ["--limit -0.5", ["--limit", "-0.5"], "-0.5"], + ])("consumes %s as the value of the string flag", (_label, tokens, value) => { + const { flags, sub, extraPositionals } = parse(...tokens); + expect(flags.limit).toBe(value); + expect(sub).toEqual(base); + expect(extraPositionals).toEqual([]); + }); + + it("parses --limit=-1 exactly as --limit -1 does", () => { + expect(parse("--limit=-1")).toEqual(parse("--limit", "-1")); + expect(parse("--limit=-1").flags.limit).toBe("-1"); + }); + + it("consumes a negative value for an undeclared flag without leaking it", () => { + // --offset is not a declared flag, so nothing reads it; the observable + // contract is that -2 is neither a positional nor a phantom flag. + const spaced = parse("--offset", "-2"); + expect(spaced.sub).toEqual(base); + expect(spaced.extraPositionals).toEqual([]); + expect(spaced).toEqual(parse("--offset=-2")); + }); + + it("keeps a non-numeric dash token as the existing (non-value) behaviour", () => { + // `-x` is not a negative number: --limit still receives no value. + const { flags, sub } = parse("--limit", "-x"); + expect(flags.limit).toBeUndefined(); + expect(sub).toEqual(base); + }); + + it.each([ + ["--limit -h", "help"], + ["--limit -y", "yes"], + ["--limit -v", "version"], + ])("keeps %s reading the short flag, not a value", (tokens, flag) => { + const { flags, sub } = parse(...tokens.split(" ")); + expect(flags[flag as keyof typeof flags]).toBe(true); + expect(flags.limit).toBeUndefined(); + expect(sub).toEqual(base); + }); + + it("keeps a bare -1 positional instead of a phantom flag", () => { + const { flags, sub } = parse("-1"); + expect(sub).toEqual([...base, "-1"]); + expect(flags).toEqual({}); + }); + + it("never gives a boolean flag a negative value", () => { + const { flags, sub } = parse("--json", "-1"); + expect(flags.json).toBe(true); + expect(sub).toEqual([...base, "-1"]); + }); + + it("does not rewrite tokens after the -- terminator", () => { + const { sub } = parse("--", "--limit", "-1"); + expect(sub).toEqual([...base, "--limit", "-1"]); + }); + + it.each([ + ["-r -1", "region", "-1"], + ["-d -1", "domain", "-1"], + ["-p -1", "provider", "-1"], + ])("gives the value to the short string alias: %s", (tokens, key, value) => { + // Short aliases of string flags must match the long form: the number is + // the value, never a placeholder leaking out of the parser. + const short = parse(...tokens.split(" ")); + expect(short.flags[key as keyof typeof short.flags]).toBe(value); + expect(short).toEqual(parse(`--${key}`, value)); + }); + + it("keeps -y -1 as the boolean -y with -1 as a positional", () => { + const { flags, sub } = parse("-y", "-1"); + expect(flags.yes).toBe(true); + expect(sub).toEqual([...base, "-1"]); + }); + + it.each([ + [["-r", "-1"]], + [["-d", "-1"]], + [["-z", "-1"]], + [["-yv", "-1"]], + [["-1", "-2"]], + [["--limit", "-1", "-2"]], + [["--json", "-1", "--limit", "-5"]], + ])( + "never leaks the placeholder into any flag or positional: %j", + (tokens) => { + const result = parse(...tokens); + expect(JSON.stringify(result)).not.toContain("\uE000"); + } + ); +}); diff --git a/packages/cli/src/utils/shared/arg-parser.ts b/packages/cli/src/utils/shared/arg-parser.ts index 91ef48773..9fded082e 100644 --- a/packages/cli/src/utils/shared/arg-parser.ts +++ b/packages/cli/src/utils/shared/arg-parser.ts @@ -327,6 +327,91 @@ export function resolveNegatableFlag( return argv.includes(negatedArg) ? false : undefined; } +/** + * A negative number (`-1`, `-5`, `-0.5`). mri reads any `-` as a cluster + * of short flags, so `--limit -1` lost its value and `-1` became a phantom + * flag `1`. + */ +const NEGATIVE_NUMBER = /^-\d+(\.\d+)?$/; + +/** A long flag name without its value, e.g. `--limit`. */ +const LONG_FLAG_NAME = /^--([a-z][a-z0-9-]*)$/; + +/** Private-use prefix marking a protected positional; never a real argument. */ +const PROTECTED_POSITIONAL = "\uE000"; + +/** + * Rewrites argv so mri keeps negative numbers intact, without touching any + * real flag. Two cases: + * + * - `--name -1`, where `--name` is a value flag (not declared boolean, not + * `--no-*`): becomes `--name=-1`, which mri already parses correctly. + * Applies to declared string flags and undeclared ones alike, the same + * tokens mri already treats as values; `-x` or `-h` are not numeric, so + * they keep today's behaviour. + * - a bare `-1` anywhere else: swapped for a placeholder so mri files it as a + * positional, and `restore` puts `-1` back in place. + * + * Everything after `--` is passed through untouched. + */ +function protectNegativeNumbers(userArgs: string[]): { + argv: string[]; + restore: (positional: string) => string; + restoreFlagValues: (parsed: Record) => void; +} { + const booleans = new Set([ + ...BOOLEAN_FLAGS, + ...NEGATED_BOOLEANS.map((n) => n.positive), + ]); + const originals: string[] = []; + const argv: string[] = []; + + for (let i = 0; i < userArgs.length; i++) { + const token = userArgs[i]; + if (token === "--") { + argv.push(...userArgs.slice(i)); + break; + } + const name = LONG_FLAG_NAME.exec(token)?.[1]; + const next = userArgs[i + 1]; + if ( + name !== undefined && + !booleans.has(name) && + !name.startsWith("no-") && + next !== undefined && + NEGATIVE_NUMBER.test(next) + ) { + argv.push(`${token}=${next}`); + i += 1; + } else if (NEGATIVE_NUMBER.test(token)) { + originals.push(token); + argv.push(`${PROTECTED_POSITIONAL}${originals.length - 1}`); + } else { + argv.push(token); + } + } + + // A short string flag (`-d -1`) takes the placeholder as its value, since + // mri cannot tell that it is not a flag. Put the number back. + const restore = (text: string): string => + text.startsWith(PROTECTED_POSITIONAL) + ? (originals[Number(text.slice(PROTECTED_POSITIONAL.length))] ?? text) + : text; + + return { + argv, + restore, + restoreFlagValues: (parsed) => { + for (const key of Object.keys(parsed)) { + const value = parsed[key]; + if (typeof value === "string") { + parsed[key] = restore(value); + } + } + }, + }; +} + /** * Parse `process.argv` (including the leading `node` + script entries) into a * legacy-compatible `{ flags, sub }` shape. @@ -344,11 +429,13 @@ export function parseCliArgs(argv: string[]): ParsedCli { // Skip the leading `node` + script entries, matching process.argv.slice(2). const userArgs = argv.slice(2); - const parsed = mri>(userArgs, { + const protectedArgs = protectNegativeNumbers(userArgs); + const parsed = mri>(protectedArgs.argv, { boolean: [...BOOLEAN_FLAGS, ...NEGATED_BOOLEANS.map((n) => n.positive)], string: [...STRING_FLAGS, ...LIST_FLAGS], alias: ALIAS, }); + protectedArgs.restoreFlagValues(parsed); const flags: CliFlags = {}; @@ -384,9 +471,11 @@ export function parseCliArgs(argv: string[]): ParsedCli { } } + const positionals = (parsed._ as string[]).map(protectedArgs.restore); + return { flags, - sub: parsed._ as string[], - extraPositionals: (parsed._ as string[]).slice(4), + sub: positionals, + extraPositionals: positionals.slice(4), }; }