Skip to content

Commit 900ae6f

Browse files
theCodeDriftclaude
andcommitted
fix(cli): honor -- when scanning argv for help
Review found a real regression in the pre-dispatch help interception: the shared scanner did not know about the POSIX end-of-options marker, so `taskless check -- -h` was read as a help request and printed usage instead of scanning a path literally named `-h` — the escape hatch check.ts was built to support. splitRawArguments now treats everything after `--` as a positional and will not swallow `--` as the value of `-d`. check.ts's extractPositionalPaths was the second copy of that rule (the complete one). It now calls the shared scanner, passing its own value-taking flag `--timeout`, and the root's onlyInitFlags scan — a third copy — goes through the same scanner's flags. Tests: unit coverage for the scanner and hasHelpFlag, plus a CLI case asserting `check --json -- -h` still runs check. Verified the case fails against the previous commit's build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
1 parent 939c122 commit 900ae6f

4 files changed

Lines changed: 127 additions & 60 deletions

File tree

‎packages/cli/src/commands/check.ts‎

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { defineCommand } from "citty";
44

55
import { hasValeRules, runEngines } from "../rules/dispatch";
66
import { assembleEngineConfigs } from "../rules/assemble";
7+
import { splitRawArguments } from "../util/argv";
78
import { formatText } from "../util/format";
89
import { ensureTasklessDirectory } from "../filesystem/directory";
910
import { listRuleIds, planEngineDispatch } from "../rules/engines";
@@ -66,40 +67,13 @@ async function filterExistingPaths(
6667
}
6768

6869
/**
69-
* Extract positional arguments from rawArgs. citty's rawArgs contains the
70-
* original argv for this subcommand, so we drop anything starting with `-`
71-
* and drop known flag values (e.g. `-d <value>`). Once `--` is seen, all
72-
* remaining arguments are treated as positional paths (conventional POSIX
73-
* end-of-options marker), which lets users pass paths that begin with `-`.
70+
* Extract positional path arguments from rawArgs. The shared scanner knows the
71+
* global value-taking flags and the POSIX `--` end-of-options marker (which is
72+
* what lets a path beginning with `-` be scanned); `--timeout` is check's own
73+
* value-taking flag, so it is named here rather than in the shared set.
7474
*/
7575
function extractPositionalPaths(rawArguments: string[]): string[] {
76-
const paths: string[] = [];
77-
let afterDoubleDash = false;
78-
for (let index = 0; index < rawArguments.length; index++) {
79-
const argument = rawArguments[index]!;
80-
if (afterDoubleDash) {
81-
paths.push(argument);
82-
continue;
83-
}
84-
if (argument === "--") {
85-
afterDoubleDash = true;
86-
continue;
87-
}
88-
if (argument.startsWith("-")) {
89-
// Skip value for short/long flags that take a value
90-
if (
91-
(argument === "-d" ||
92-
argument === "--dir" ||
93-
argument === "--timeout") &&
94-
index + 1 < rawArguments.length
95-
) {
96-
index += 1;
97-
}
98-
continue;
99-
}
100-
paths.push(argument);
101-
}
102-
return paths;
76+
return splitRawArguments(rawArguments, ["--timeout"]).positionals;
10377
}
10478

10579
/** A runtime rule that will not run, with why (advisory). */

‎packages/cli/src/index.ts‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
shutdownTelemetry,
1616
} from "./telemetry";
1717
import { emitRunEvents, resolveCommandName, resolveCwd } from "./telemetry-run";
18-
import { hasHelpFlag, splitRawArguments } from "./util/argv";
18+
import { DIR_FLAGS, hasHelpFlag, splitRawArguments } from "./util/argv";
1919
import { showResolvedUsage } from "./util/help";
2020
import { CLIError } from "./util/cli-error";
2121

@@ -67,23 +67,17 @@ const main = defineCommand({
6767
// Only take action when no positional args (i.e. no subcommand) were
6868
// provided. A value following `-d`/`--dir` is a flag value, not a
6969
// positional, so splitRawArguments skips it.
70-
if (splitRawArguments(rawArgs).positionals.length > 0) {
70+
const { positionals, flags } = splitRawArguments(rawArgs);
71+
if (positionals.length > 0) {
7172
return;
7273
}
7374

7475
// Only delegate to `init` when the only flags present are ones init
75-
// also understands (`-d` / `--dir`). Version/json flags and any unknown
76-
// flags should fall through to citty's default help instead of silently
77-
// launching the wizard. (`--help`/`-h` never reach here — they are
78-
// intercepted before dispatch below.)
79-
const onlyInitFlags = rawArgs.every((argument, index) => {
80-
if (!argument.startsWith("-")) {
81-
const previous = rawArgs[index - 1];
82-
return previous === "-d" || previous === "--dir";
83-
}
84-
if (argument === "-d" || argument === "--dir") return true;
85-
return false;
86-
});
76+
// also understands (`-d` / `--dir`). Version/json flags, a bare `--`, and
77+
// any unknown flags should fall through to citty's default help instead of
78+
// silently launching the wizard. (`--help`/`-h` never reach here — they
79+
// are intercepted before dispatch below.)
80+
const onlyInitFlags = flags.every((flag) => DIR_FLAGS.has(flag));
8781
if (!onlyInitFlags) {
8882
await showUsage(cmd);
8983
return;

‎packages/cli/src/util/argv.ts‎

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,66 @@
11
/**
22
* Raw-argv scanning shared by everything that has to answer "which tokens are
33
* positionals?" — subcommand resolution for `--help`, the root command's
4-
* no-subcommand check, and telemetry's command name. One copy, because the
5-
* rule is subtle: `-d`/`--dir` take a value, so the token after one of them is
6-
* a flag value and NOT a positional (`taskless -d /tmp check` runs `check`,
7-
* not `/tmp`).
4+
* no-subcommand check, `check`'s path arguments, and telemetry's command name.
5+
* One copy, because the rules are subtle:
6+
*
7+
* - `-d`/`--dir` take a value, so the token after one of them is a flag value
8+
* and NOT a positional (`taskless -d /tmp check` runs `check`, not `/tmp`).
9+
* - `--` is the POSIX end-of-options marker: every token after it is a
10+
* positional even if it starts with `-`, which is what lets `taskless check
11+
* -- -h` scan a path literally named `-h` instead of asking for help.
812
*/
913

10-
/** Flags that consume the following token as their value. */
11-
const VALUE_FLAGS = new Set(["-d", "--dir"]);
14+
/** Flags every command takes a value for. */
15+
export const DIR_FLAGS = new Set(["-d", "--dir"]);
16+
17+
/** The POSIX end-of-options marker. */
18+
const END_OF_OPTIONS = "--";
1219

1320
export interface SplitArguments {
14-
/** Tokens that are not flags and not the value of a value-taking flag. */
21+
/**
22+
* Tokens that are not flags and not the value of a value-taking flag, plus
23+
* everything after `--`.
24+
*/
1525
positionals: string[];
16-
/** Tokens that start with `-`. Values consumed by them are not included. */
26+
/**
27+
* Option tokens, in order, including the `--` marker itself. Values consumed
28+
* by a value-taking flag are not included.
29+
*/
1730
flags: string[];
1831
}
1932

20-
/** Split raw argv into positionals and flags, skipping flag values. */
21-
export function splitRawArguments(rawArguments: string[]): SplitArguments {
33+
/**
34+
* Split raw argv into positionals and flags, skipping flag values and honoring
35+
* `--`. `valueFlags` names flags beyond `-d`/`--dir` that consume the next
36+
* token (e.g. `check`'s `--timeout`).
37+
*/
38+
export function splitRawArguments(
39+
rawArguments: string[],
40+
valueFlags: readonly string[] = []
41+
): SplitArguments {
42+
const consumesValue =
43+
valueFlags.length > 0 ? new Set([...DIR_FLAGS, ...valueFlags]) : DIR_FLAGS;
2244
const positionals: string[] = [];
2345
const flags: string[] = [];
2446
for (let index = 0; index < rawArguments.length; index++) {
2547
const argument = rawArguments[index]!;
48+
if (argument === END_OF_OPTIONS) {
49+
flags.push(argument);
50+
positionals.push(...rawArguments.slice(index + 1));
51+
break;
52+
}
2653
if (argument.startsWith("-")) {
2754
flags.push(argument);
28-
// `--dir=<path>` carries its own value; `-d <path>` eats the next token.
29-
if (VALUE_FLAGS.has(argument)) index++;
55+
// `--dir=<path>` carries its own value; `-d <path>` eats the next token —
56+
// unless that token is `--`, which ends the options rather than being one.
57+
if (
58+
consumesValue.has(argument) &&
59+
rawArguments[index + 1] !== undefined &&
60+
rawArguments[index + 1] !== END_OF_OPTIONS
61+
) {
62+
index++;
63+
}
3064
continue;
3165
}
3266
positionals.push(argument);
@@ -35,8 +69,9 @@ export function splitRawArguments(rawArguments: string[]): SplitArguments {
3569
}
3670

3771
/**
38-
* True when argv asks for help. Scanned through `splitRawArguments` so a flag value
39-
* that happens to read like `-h` is not mistaken for a help request.
72+
* True when argv asks for help. Scanned through `splitRawArguments` so neither
73+
* a flag value nor a path after `--` that happens to read like `-h` is mistaken
74+
* for a help request.
4075
*/
4176
export function hasHelpFlag(rawArguments: string[]): boolean {
4277
const { flags } = splitRawArguments(rawArguments);

‎packages/cli/test/help-flag.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { join, resolve } from "node:path";
55
import { promisify } from "node:util";
66
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
77

8+
import { hasHelpFlag, splitRawArguments } from "../src/util/argv";
9+
810
const execFileAsync = promisify(execFile);
911
const binPath = resolve(import.meta.dirname, "../dist/index.js");
1012

@@ -70,6 +72,24 @@ describe("--help below the root", () => {
7072
expect(await readdir(temporaryDirectory)).toEqual([]);
7173
});
7274

75+
// `--` is the escape hatch `check` supports so a path beginning with `-` can
76+
// still be scanned. Intercepting help before dispatch must not eat it.
77+
it("does not read a path after -- as a help request", async () => {
78+
const { stdout } = await execFileAsync("node", [
79+
binPath,
80+
"check",
81+
"-d",
82+
temporaryDirectory,
83+
"--json",
84+
"--",
85+
"-h",
86+
]);
87+
88+
expect(stdout).not.toContain("USAGE");
89+
const parsed = JSON.parse(stdout.trim()) as { success: boolean };
90+
expect(parsed.success).toBe(true);
91+
});
92+
7393
it("leaves the root's --help unchanged", async () => {
7494
const { stdout } = await execFileAsync("node", [binPath, "--help"]);
7595

@@ -80,6 +100,50 @@ describe("--help below the root", () => {
80100
});
81101
});
82102

103+
describe("splitRawArguments", () => {
104+
it("treats the token after -d/--dir as a value, not a positional", () => {
105+
expect(splitRawArguments(["-d", "/tmp", "check"])).toEqual({
106+
positionals: ["check"],
107+
flags: ["-d"],
108+
});
109+
});
110+
111+
it("treats everything after -- as a positional", () => {
112+
expect(splitRawArguments(["check", "--", "-h", "--json"])).toEqual({
113+
positionals: ["check", "-h", "--json"],
114+
flags: ["--"],
115+
});
116+
});
117+
118+
it("does not swallow -- as the value of -d", () => {
119+
expect(splitRawArguments(["-d", "--", "src"])).toEqual({
120+
positionals: ["src"],
121+
flags: ["-d", "--"],
122+
});
123+
});
124+
125+
it("accepts command-specific value flags", () => {
126+
expect(
127+
splitRawArguments(["check", "--timeout", "5", "src"], ["--timeout"])
128+
.positionals
129+
).toEqual(["check", "src"]);
130+
});
131+
});
132+
133+
describe("hasHelpFlag", () => {
134+
it.each([
135+
[["check", "--help"], true],
136+
[["auth", "login", "-h"], true],
137+
[["-d", "/tmp", "check", "--help"], true],
138+
[["check"], false],
139+
// A path after `--`, and a directory that reads like a flag, are values.
140+
[["check", "--", "-h"], false],
141+
[["-d", "-h", "check"], false],
142+
])("resolves %j to %s", (argv, expected) => {
143+
expect(hasHelpFlag(argv)).toBe(expected);
144+
});
145+
});
146+
83147
// REGRESSION GUARD — DO NOT "SIMPLIFY" THE HELP PATH TO citty's runMain.
84148
//
85149
// runMain's help branch calls process.exit(0), which does not run `finally`

0 commit comments

Comments
 (0)