Skip to content

Commit 2f491b0

Browse files
stewartjarodclaude
andcommitted
fix(cli): keep negative numeric values attached to their flag
mri reads any "-<token>" as a cluster of short flags, so `--limit -1` lost its value (the command fell back to the API default) and a bare `-1` became a phantom flag `1`. `--limit=-1` already worked, so the two forms behaved differently. parseCliArgs now rewrites argv before mri sees it: - `--name -1` becomes `--name=-1` when --name is a value flag (not a declared boolean and not a `--no-*` form). This covers declared string flags such as --limit and undeclared ones such as --offset. - a bare `-1` is kept as a positional in `sub`. - a short string flag given a negative number (`-d -1`, `-r -1`) gets the number back as its value, so it matches the long form. Real flags are unaffected: only tokens matching /^-\d+(\.\d+)?$/ are touched, so `-h`, `-v`, `-y` and `-x` keep their current meaning, boolean flags never take a negative value, and everything after `--` is passed through untouched. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GxJAQNUP4FKZHL1yzPDsGd
1 parent 2d00a01 commit 2f491b0

3 files changed

Lines changed: 259 additions & 3 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* `wraps email logs list --limit <n>`: the space form and the `=` form must
3+
* reach the API identically. Before the fix, `--limit -1` lost its value to
4+
* the arg parser, so the request silently fell back to the API default.
5+
*/
6+
7+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
8+
import { parseCliArgs } from "../../utils/shared/arg-parser.js";
9+
10+
vi.mock("@clack/prompts");
11+
vi.mock("../../utils/shared/api-target.js", () => ({
12+
resolveApiTarget: vi.fn(),
13+
checkApiTarget: vi.fn(),
14+
}));
15+
16+
import {
17+
checkApiTarget,
18+
resolveApiTarget,
19+
} from "../../utils/shared/api-target.js";
20+
import { emailLogsList } from "../email/logs.js";
21+
22+
const argv = (...args: string[]) => [
23+
"node",
24+
"wraps",
25+
"email",
26+
"logs",
27+
"list",
28+
...args,
29+
];
30+
31+
/** Mirrors the router in cli.ts: parsed flags are passed straight through. */
32+
async function runLogsList(...args: string[]): Promise<URL> {
33+
const { flags } = parseCliArgs(argv(...args));
34+
await emailLogsList({ limit: flags.limit, status: flags.status });
35+
const [url] = vi.mocked(fetch).mock.calls[0] as [string, unknown];
36+
return new URL(url);
37+
}
38+
39+
describe("email logs list — --limit with a negative value", () => {
40+
beforeEach(() => {
41+
vi.mocked(resolveApiTarget).mockResolvedValue({} as never);
42+
vi.mocked(checkApiTarget).mockReturnValue({
43+
ok: true,
44+
target: { apiBase: "https://api.example.test", token: "t" },
45+
} as never);
46+
vi.stubGlobal(
47+
"fetch",
48+
vi.fn(async () => ({
49+
ok: true,
50+
status: 200,
51+
json: async () => ({ logs: [], total: 0, nextCursor: null }),
52+
}))
53+
);
54+
});
55+
56+
afterEach(() => {
57+
vi.unstubAllGlobals();
58+
vi.clearAllMocks();
59+
});
60+
61+
it("sends the same limit for --limit -1 as for --limit=-1", async () => {
62+
const spaced = await runLogsList("--limit", "-1");
63+
const attached = await runLogsList("--limit=-1");
64+
expect(spaced.searchParams.get("limit")).toBe("-1");
65+
expect(attached.searchParams.get("limit")).toBe("-1");
66+
});
67+
});

‎packages/cli/src/utils/shared/__tests__/arg-parser.test.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,103 @@ describe("parseCliArgs — workflows push slug", () => {
241241
expect(extraPositionals).toEqual([]);
242242
});
243243
});
244+
245+
describe("parseCliArgs — negative numeric values", () => {
246+
const argv = (...args: string[]) => ["node", "wraps", ...args];
247+
const base = ["email", "logs", "list"];
248+
const parse = (...rest: string[]) => parseCliArgs(argv(...base, ...rest));
249+
250+
it.each([
251+
["--limit -1", ["--limit", "-1"], "-1"],
252+
["--limit -5", ["--limit", "-5"], "-5"],
253+
["--limit -0.5", ["--limit", "-0.5"], "-0.5"],
254+
])("consumes %s as the value of the string flag", (_label, tokens, value) => {
255+
const { flags, sub, extraPositionals } = parse(...tokens);
256+
expect(flags.limit).toBe(value);
257+
expect(sub).toEqual(base);
258+
expect(extraPositionals).toEqual([]);
259+
});
260+
261+
it("parses --limit=-1 exactly as --limit -1 does", () => {
262+
expect(parse("--limit=-1")).toEqual(parse("--limit", "-1"));
263+
expect(parse("--limit=-1").flags.limit).toBe("-1");
264+
});
265+
266+
it("consumes a negative value for an undeclared flag without leaking it", () => {
267+
// --offset is not a declared flag, so nothing reads it; the observable
268+
// contract is that -2 is neither a positional nor a phantom flag.
269+
const spaced = parse("--offset", "-2");
270+
expect(spaced.sub).toEqual(base);
271+
expect(spaced.extraPositionals).toEqual([]);
272+
expect(spaced).toEqual(parse("--offset=-2"));
273+
});
274+
275+
it("keeps a non-numeric dash token as the existing (non-value) behaviour", () => {
276+
// `-x` is not a negative number: --limit still receives no value.
277+
const { flags, sub } = parse("--limit", "-x");
278+
expect(flags.limit).toBeUndefined();
279+
expect(sub).toEqual(base);
280+
});
281+
282+
it.each([
283+
["--limit -h", "help"],
284+
["--limit -y", "yes"],
285+
["--limit -v", "version"],
286+
])("keeps %s reading the short flag, not a value", (tokens, flag) => {
287+
const { flags, sub } = parse(...tokens.split(" "));
288+
expect(flags[flag as keyof typeof flags]).toBe(true);
289+
expect(flags.limit).toBeUndefined();
290+
expect(sub).toEqual(base);
291+
});
292+
293+
it("keeps a bare -1 positional instead of a phantom flag", () => {
294+
const { flags, sub } = parse("-1");
295+
expect(sub).toEqual([...base, "-1"]);
296+
expect(flags).toEqual({});
297+
});
298+
299+
it("never gives a boolean flag a negative value", () => {
300+
const { flags, sub } = parse("--json", "-1");
301+
expect(flags.json).toBe(true);
302+
expect(sub).toEqual([...base, "-1"]);
303+
});
304+
305+
it("does not rewrite tokens after the -- terminator", () => {
306+
const { sub } = parse("--", "--limit", "-1");
307+
expect(sub).toEqual([...base, "--limit", "-1"]);
308+
});
309+
310+
it.each([
311+
["-r -1", "region", "-1"],
312+
["-d -1", "domain", "-1"],
313+
["-p -1", "provider", "-1"],
314+
])("gives the value to the short string alias: %s", (tokens, key, value) => {
315+
// Short aliases of string flags must match the long form: the number is
316+
// the value, never a placeholder leaking out of the parser.
317+
const short = parse(...tokens.split(" "));
318+
expect(short.flags[key as keyof typeof short.flags]).toBe(value);
319+
expect(short).toEqual(parse(`--${key}`, value));
320+
});
321+
322+
it("keeps -y -1 as the boolean -y with -1 as a positional", () => {
323+
const { flags, sub } = parse("-y", "-1");
324+
expect(flags.yes).toBe(true);
325+
expect(sub).toEqual([...base, "-1"]);
326+
});
327+
328+
it.each([
329+
[["-r", "-1"]],
330+
[["-d", "-1"]],
331+
[["-z", "-1"]],
332+
[["-yv", "-1"]],
333+
[["-1", "-2"]],
334+
[["--limit", "-1", "-2"]],
335+
[["--json", "-1", "--limit", "-5"]],
336+
])(
337+
"never leaks the placeholder into any flag or positional: %j",
338+
(tokens) => {
339+
const result = parse(...tokens);
340+
expect(JSON.stringify(result)).not.toContain("\uE000");
341+
}
342+
);
343+
});

‎packages/cli/src/utils/shared/arg-parser.ts‎

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,91 @@ export function resolveNegatableFlag(
423423
return argv.includes(negatedArg) ? false : undefined;
424424
}
425425

426+
/**
427+
* A negative number (`-1`, `-5`, `-0.5`). mri reads any `-<token>` as a cluster
428+
* of short flags, so `--limit -1` lost its value and `-1` became a phantom
429+
* flag `1`.
430+
*/
431+
const NEGATIVE_NUMBER = /^-\d+(\.\d+)?$/;
432+
433+
/** A long flag name without its value, e.g. `--limit`. */
434+
const LONG_FLAG_NAME = /^--([a-z][a-z0-9-]*)$/;
435+
436+
/** Private-use prefix marking a protected positional; never a real argument. */
437+
const PROTECTED_POSITIONAL = "\uE000";
438+
439+
/**
440+
* Rewrites argv so mri keeps negative numbers intact, without touching any
441+
* real flag. Two cases:
442+
*
443+
* - `--name -1`, where `--name` is a value flag (not declared boolean, not
444+
* `--no-*`): becomes `--name=-1`, which mri already parses correctly.
445+
* Applies to declared string flags and undeclared ones alike, the same
446+
* tokens mri already treats as values; `-x` or `-h` are not numeric, so
447+
* they keep today's behaviour.
448+
* - a bare `-1` anywhere else: swapped for a placeholder so mri files it as a
449+
* positional, and `restore` puts `-1` back in place.
450+
*
451+
* Everything after `--` is passed through untouched.
452+
*/
453+
function protectNegativeNumbers(userArgs: string[]): {
454+
argv: string[];
455+
restore: (positional: string) => string;
456+
restoreFlagValues: (parsed: Record<string, unknown>) => void;
457+
} {
458+
const booleans = new Set<string>([
459+
...BOOLEAN_FLAGS,
460+
...NEGATED_BOOLEANS.map((n) => n.positive),
461+
]);
462+
const originals: string[] = [];
463+
const argv: string[] = [];
464+
465+
for (let i = 0; i < userArgs.length; i++) {
466+
const token = userArgs[i];
467+
if (token === "--") {
468+
argv.push(...userArgs.slice(i));
469+
break;
470+
}
471+
const name = LONG_FLAG_NAME.exec(token)?.[1];
472+
const next = userArgs[i + 1];
473+
if (
474+
name !== undefined &&
475+
!booleans.has(name) &&
476+
!name.startsWith("no-") &&
477+
next !== undefined &&
478+
NEGATIVE_NUMBER.test(next)
479+
) {
480+
argv.push(`${token}=${next}`);
481+
i += 1;
482+
} else if (NEGATIVE_NUMBER.test(token)) {
483+
originals.push(token);
484+
argv.push(`${PROTECTED_POSITIONAL}${originals.length - 1}`);
485+
} else {
486+
argv.push(token);
487+
}
488+
}
489+
490+
// A short string flag (`-d -1`) takes the placeholder as its value, since
491+
// mri cannot tell that it is not a flag. Put the number back.
492+
const restore = (text: string): string =>
493+
text.startsWith(PROTECTED_POSITIONAL)
494+
? (originals[Number(text.slice(PROTECTED_POSITIONAL.length))] ?? text)
495+
: text;
496+
497+
return {
498+
argv,
499+
restore,
500+
restoreFlagValues: (parsed) => {
501+
for (const key of Object.keys(parsed)) {
502+
const value = parsed[key];
503+
if (typeof value === "string") {
504+
parsed[key] = restore(value);
505+
}
506+
}
507+
},
508+
};
509+
}
510+
426511
/**
427512
* Parse `process.argv` (including the leading `node` + script entries) into a
428513
* legacy-compatible `{ flags, sub }` shape.
@@ -440,11 +525,13 @@ export function parseCliArgs(argv: string[]): ParsedCli {
440525
// Skip the leading `node` + script entries, matching process.argv.slice(2).
441526
const userArgs = argv.slice(2);
442527

443-
const parsed = mri<Record<string, unknown>>(userArgs, {
528+
const protectedArgs = protectNegativeNumbers(userArgs);
529+
const parsed = mri<Record<string, unknown>>(protectedArgs.argv, {
444530
boolean: [...BOOLEAN_FLAGS, ...NEGATED_BOOLEANS.map((n) => n.positive)],
445531
string: [...STRING_FLAGS, ...LIST_FLAGS],
446532
alias: ALIAS,
447533
});
534+
protectedArgs.restoreFlagValues(parsed);
448535

449536
const flags: CliFlags = {};
450537

@@ -480,9 +567,11 @@ export function parseCliArgs(argv: string[]): ParsedCli {
480567
}
481568
}
482569

570+
const positionals = (parsed._ as string[]).map(protectedArgs.restore);
571+
483572
return {
484573
flags,
485-
sub: parsed._ as string[],
486-
extraPositionals: (parsed._ as string[]).slice(4),
574+
sub: positionals,
575+
extraPositionals: positionals.slice(4),
487576
};
488577
}

0 commit comments

Comments
 (0)