Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions packages/cli/src/commands/__tests__/email-logs-limit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* `wraps email logs list --limit <n>`: 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<URL> {
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");
});
});
100 changes: 100 additions & 0 deletions packages/cli/src/utils/shared/__tests__/arg-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
);
});
95 changes: 92 additions & 3 deletions packages/cli/src/utils/shared/arg-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,91 @@ export function resolveNegatableFlag(
return argv.includes(negatedArg) ? false : undefined;
}

/**
* A negative number (`-1`, `-5`, `-0.5`). mri reads any `-<token>` 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<string, unknown>) => void;
} {
const booleans = new Set<string>([
...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.
Expand All @@ -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<Record<string, unknown>>(userArgs, {
const protectedArgs = protectNegativeNumbers(userArgs);
const parsed = mri<Record<string, unknown>>(protectedArgs.argv, {
boolean: [...BOOLEAN_FLAGS, ...NEGATED_BOOLEANS.map((n) => n.positive)],
string: [...STRING_FLAGS, ...LIST_FLAGS],
alias: ALIAS,
});
protectedArgs.restoreFlagValues(parsed);

const flags: CliFlags = {};

Expand Down Expand Up @@ -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),
};
}
Loading