fix(cli): make --help work on every subcommand

.changeset/help-flag-on-subcommands.md

@@ -0,0 +1,18 @@
+---
+"@taskless/cli": patch
+---
+
+Make `--help` work on every command, instead of running the command.
+
+`taskless check --help` printed no usage — it ran `check`. So did every other
+subcommand: `--help` was parsed as an unrecognized flag and the command body
+executed anyway, which meant asking `init` how it works installed skills, and
+asking `check` how it works migrated the `.taskless/` scaffold. The only place
+help worked was the bare `taskless --help`, whose own output tells you to run
+`taskless <command> --help`.
+
+`--help` and `-h` are now recognized at every depth, including nested commands
+(`taskless auth login --help` describes `login`, not `auth`), and a working
+directory passed before the command (`taskless -d ./repo check --help`) no
+longer confuses which command you asked about. The usage text itself is
+unchanged, and nothing else about how commands run has changed.

packages/cli/src/commands/check.ts

@@ -4,6 +4,7 @@ import { defineCommand } from "citty";
 
 import { hasValeRules, runEngines } from "../rules/dispatch";
 import { assembleEngineConfigs } from "../rules/assemble";
+import { splitRawArguments } from "../util/argv";
 import { formatText } from "../util/format";
 import { ensureTasklessDirectory } from "../filesystem/directory";
 import { listRuleIds, planEngineDispatch } from "../rules/engines";
@@ -66,40 +67,13 @@ async function filterExistingPaths(
 }
 
 /**
- * Extract positional arguments from rawArgs. citty's rawArgs contains the
- * original argv for this subcommand, so we drop anything starting with `-`
- * and drop known flag values (e.g. `-d <value>`). Once `--` is seen, all
- * remaining arguments are treated as positional paths (conventional POSIX
- * end-of-options marker), which lets users pass paths that begin with `-`.
+ * Extract positional path arguments from rawArgs. The shared scanner knows the
+ * global value-taking flags and the POSIX `--` end-of-options marker (which is
+ * what lets a path beginning with `-` be scanned); `--timeout` is check's own
+ * value-taking flag, so it is named here rather than in the shared set.
  */
 function extractPositionalPaths(rawArguments: string[]): string[] {
-  const paths: string[] = [];
-  let afterDoubleDash = false;
-  for (let index = 0; index < rawArguments.length; index++) {
-    const argument = rawArguments[index]!;
-    if (afterDoubleDash) {
-      paths.push(argument);
-      continue;
-    }
-    if (argument === "--") {
-      afterDoubleDash = true;
-      continue;
-    }
-    if (argument.startsWith("-")) {
-      // Skip value for short/long flags that take a value
-      if (
-        (argument === "-d" ||
-          argument === "--dir" ||
-          argument === "--timeout") &&
-        index + 1 < rawArguments.length
-      ) {
-        index += 1;
-      }
-      continue;
-    }
-    paths.push(argument);
-  }
-  return paths;
+  return splitRawArguments(rawArguments, ["--timeout"]).positionals;
 }
 
 /** A runtime rule that will not run, with why (advisory). */

packages/cli/src/index.ts

@@ -15,6 +15,8 @@ import {
   shutdownTelemetry,
 } from "./telemetry";
 import { emitRunEvents, resolveCommandName, resolveCwd } from "./telemetry-run";
+import { DIR_FLAGS, hasHelpFlag, splitRawArguments } from "./util/argv";
+import { showResolvedUsage } from "./util/help";
 import { CLIError } from "./util/cli-error";
 
 const subCommands = {
@@ -64,29 +66,18 @@ const main = defineCommand({
     // citty always calls the parent's run handler, even after a subcommand.
     // Only take action when no positional args (i.e. no subcommand) were
     // provided. A value following `-d`/`--dir` is a flag value, not a
-    // positional, so skip it when scanning.
-    const hasPositional = rawArgs.some((argument, index) => {
-      if (argument.startsWith("-")) return false;
-      const previous = rawArgs[index - 1];
-      if (previous === "-d" || previous === "--dir") return false;
-      return true;
-    });
-    if (hasPositional) {
+    // positional, so splitRawArguments skips it.
+    const { positionals, flags } = splitRawArguments(rawArgs);
+    if (positionals.length > 0) {
       return;
     }
 
     // Only delegate to `init` when the only flags present are ones init
-    // also understands (`-d` / `--dir`). Help/version/json flags and
-    // any unknown flags should fall through to citty's default help instead
-    // of silently launching the wizard.
-    const onlyInitFlags = rawArgs.every((argument, index) => {
-      if (!argument.startsWith("-")) {
-        const previous = rawArgs[index - 1];
-        return previous === "-d" || previous === "--dir";
-      }
-      if (argument === "-d" || argument === "--dir") return true;
-      return false;
-    });
+    // also understands (`-d` / `--dir`). Version/json flags, a bare `--`, and
+    // any unknown flags should fall through to citty's default help instead of
+    // silently launching the wizard. (`--help`/`-h` never reach here — they
+    // are intercepted before dispatch below.)
+    const onlyInitFlags = flags.every((flag) => DIR_FLAGS.has(flag));
     if (!onlyInitFlags) {
       await showUsage(cmd);
       return;
@@ -122,7 +113,13 @@ const startedAt = Date.now();
 const startIdentity = await resolveRunIdentity(runCwd);
 let thrown: unknown;
 try {
-  await runCommand(main, { rawArgs: rawArguments });
+  // Help is intercepted here, before dispatch, because citty implements
+  // `--help` only in runMain — and runMain exits the process on both its help
+  // and error paths, which would skip the `finally` below and drop the cli_run
+  // denominator. Rendering here returns normally instead.
+  await (hasHelpFlag(rawArguments)
+    ? showResolvedUsage(main, rawArguments)
+    : runCommand(main, { rawArgs: rawArguments }));
 } catch (error) {
   // CLIError = expected failure. Most throw sites (the `fail()` helpers) print
   // and set exitCode first and mark themselves `reported`; one that does not

packages/cli/src/telemetry-run.ts

@@ -1,6 +1,7 @@
 import { resolve } from "node:path";
 
 import type { TelemetryClient } from "./telemetry";
+import { splitRawArguments } from "./util/argv";
 import { CLIError } from "./util/cli-error";
 
 /**
@@ -11,16 +12,7 @@ import { CLIError } from "./util/cli-error";
  * command for an agent invocation is just `agent`.
  */
 export function resolveCommandName(rawArguments: string[]): string {
-  const valueFlags = new Set(["-d", "--dir"]);
-  const positionals: string[] = [];
-  for (let index = 0; index < rawArguments.length; index++) {
-    const argument = rawArguments[index]!;
-    if (argument.startsWith("-")) {
-      if (!argument.includes("=") && valueFlags.has(argument)) index++;
-      continue;
-    }
-    positionals.push(argument);
-  }
+  const { positionals } = splitRawArguments(rawArguments);
 
   if (positionals.length === 0) return "(default)";
   const top = positionals[0]!;

packages/cli/src/util/argv.ts

@@ -0,0 +1,79 @@
+/**
+ * Raw-argv scanning shared by everything that has to answer "which tokens are
+ * positionals?" — subcommand resolution for `--help`, the root command's
+ * no-subcommand check, `check`'s path arguments, and telemetry's command name.
+ * One copy, because the rules are subtle:
+ *
+ * - `-d`/`--dir` take a value, so the token after one of them is a flag value
+ *   and NOT a positional (`taskless -d /tmp check` runs `check`, not `/tmp`).
+ * - `--` is the POSIX end-of-options marker: every token after it is a
+ *   positional even if it starts with `-`, which is what lets `taskless check
+ *   -- -h` scan a path literally named `-h` instead of asking for help.
+ */
+
+/** Flags every command takes a value for. */
+export const DIR_FLAGS = new Set(["-d", "--dir"]);
+
+/** The POSIX end-of-options marker. */
+const END_OF_OPTIONS = "--";
+
+export interface SplitArguments {
+  /**
+   * Tokens that are not flags and not the value of a value-taking flag, plus
+   * everything after `--`.
+   */
+  positionals: string[];
+  /**
+   * Option tokens, in order, including the `--` marker itself. Values consumed
+   * by a value-taking flag are not included.
+   */
+  flags: string[];
+}
+
+/**
+ * Split raw argv into positionals and flags, skipping flag values and honoring
+ * `--`. `valueFlags` names flags beyond `-d`/`--dir` that consume the next
+ * token (e.g. `check`'s `--timeout`).
+ */
+export function splitRawArguments(
+  rawArguments: string[],
+  valueFlags: readonly string[] = []
+): SplitArguments {
+  const consumesValue =
+    valueFlags.length > 0 ? new Set([...DIR_FLAGS, ...valueFlags]) : DIR_FLAGS;
+  const positionals: string[] = [];
+  const flags: string[] = [];
+  for (let index = 0; index < rawArguments.length; index++) {
+    const argument = rawArguments[index]!;
+    if (argument === END_OF_OPTIONS) {
+      flags.push(argument);
+      positionals.push(...rawArguments.slice(index + 1));
+      break;
+    }
+    if (argument.startsWith("-")) {
+      flags.push(argument);
+      // `--dir=<path>` carries its own value; `-d <path>` eats the next token —
+      // unless that token is `--`, which ends the options rather than being one.
+      if (
+        consumesValue.has(argument) &&
+        rawArguments[index + 1] !== undefined &&
+        rawArguments[index + 1] !== END_OF_OPTIONS
+      ) {
+        index++;
+      }
+      continue;
+    }
+    positionals.push(argument);
+  }
+  return { positionals, flags };
+}
+
+/**
+ * True when argv asks for help. Scanned through `splitRawArguments` so neither
+ * a flag value nor a path after `--` that happens to read like `-h` is mistaken
+ * for a help request.
+ */
+export function hasHelpFlag(rawArguments: string[]): boolean {
+  const { flags } = splitRawArguments(rawArguments);
+  return flags.includes("--help") || flags.includes("-h");
+}

packages/cli/src/util/help.ts

@@ -0,0 +1,56 @@
+import { showUsage } from "citty";
+import type { ArgsDef, CommandDef, Resolvable } from "citty";
+
+import { splitRawArguments } from "./argv";
+
+/**
+ * citty only implements `--help` inside `runMain`, and `runMain`'s help path
+ * calls `process.exit(0)` — which skips `finally` blocks, including the one in
+ * `src/index.ts` that emits the `cli_run` telemetry denominator. So the CLI
+ * keeps `runCommand` and resolves help itself, here, using citty's own
+ * exported `showUsage` so no usage text is hand-written.
+ *
+ * citty's `resolveSubCommand` is not exported, and its own walk would resolve
+ * `taskless -d /tmp check --help` to `/tmp` (it takes the first non-`-` token),
+ * so the walk below uses the shared argv scanner instead.
+ */
+
+/** Resolve a citty `Resolvable<T>` (value, promise, or factory) to its value. */
+async function resolveValue<T>(
+  value: Resolvable<T> | undefined
+): Promise<T | undefined> {
+  return typeof value === "function"
+    ? await (value as () => T | Promise<T>)()
+    : await value;
+}
+
+/**
+ * Render usage for the deepest command the positionals resolve to, with its
+ * parent (so nested commands like `auth login` print their own usage under
+ * their own name). Returns normally — never exits — so the caller's telemetry
+ * `finally` still runs.
+ */
+export async function showResolvedUsage<T extends ArgsDef = ArgsDef>(
+  root: CommandDef<T>,
+  rawArguments: string[]
+): Promise<void> {
+  // Widen once: the walk descends into subcommands, each with its own args
+  // shape, and only the rendering (which reads meta and args generically) is
+  // done with the result.
+  let command = root as unknown as CommandDef;
+  let parent: CommandDef | undefined;
+
+  for (const name of splitRawArguments(rawArguments).positionals) {
+    const subCommands = await resolveValue(command.subCommands);
+    const child = subCommands
+      ? await resolveValue(subCommands[name])
+      : undefined;
+    // The first token that is not a child of the current command ends the
+    // walk (it is a positional argument, or a typo) — render what resolved.
+    if (!child) break;
+    parent = command;
+    command = child;
+  }
+
+  await showUsage(command, parent);
+}