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
5 changes: 5 additions & 0 deletions .changeset/dir-flag-before-subcommand.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@taskless/cli": patch
---

`-d <path>` / `--dir <path>` now works before a subcommand. `taskless auth -d .` and `taskless -d . info` used to fail with "Unknown command `.`", because the path was read as the subcommand's name. Only the `--dir=<path>` spelling worked.
6 changes: 4 additions & 2 deletions packages/cli/src/commands/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { getToken, removeToken } from "../auth/token";
import { fetchWhoami } from "../auth/whoami";
import { getTelemetry } from "../telemetry";
import { type CLIErrorCode, writeJsonError } from "../types/errors";
import { splitRawArguments } from "../util/argv";

const loginCommand = defineCommand({
meta: {
Expand Down Expand Up @@ -162,8 +163,9 @@ export const authCommand = defineCommand({
},
async run({ args, rawArgs }) {
// citty always calls the parent's run handler, even after a subcommand.
// Only show status when no subcommand was provided.
if (rawArgs.some((argument) => !argument.startsWith("-"))) {
// Only show status when no subcommand was provided. The shared scanner
// skips flag values, so the path in `auth -d <path>` is not read as one.
if (splitRawArguments(rawArgs).positionals.length > 0) {
return;
}

Expand Down
11 changes: 9 additions & 2 deletions packages/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
DIR_FLAGS,
hasHelpFlag,
hasVersionFlag,
joinDirectoryValues,
splitRawArguments,
} from "./util/argv";
import { shouldLaunchWizard } from "./util/interactive";
Expand Down Expand Up @@ -104,7 +105,11 @@ const main = defineCommand({
// unknown flags should fall through to citty's default help instead of
// silently launching the wizard. (`--help`/`-h` and `--version`/`-v`
// never reach here — they are intercepted before dispatch below.)
const onlyInitFlags = flags.every((flag) => DIR_FLAGS.has(flag));
// Compared by name before any `=`: argv arrives with `-d <path>` already
// joined into `--dir=<path>` (see joinDirectoryValues).
const onlyInitFlags = flags.every((flag) =>
DIR_FLAGS.has(flag.split("=", 1)[0]!)
);
if (!onlyInitFlags) {
await showUsage(cmd);
return;
Expand Down Expand Up @@ -146,7 +151,9 @@ const main = defineCommand({
});

// main loop to run cli and make every attempt to shut down gracefully
const rawArguments = process.argv.slice(2);
// `-d <path>` is joined into `--dir=<path>` so citty's subcommand resolution
// cannot mistake the path for a command name (see joinDirectoryValues).
const rawArguments = joinDirectoryValues(process.argv.slice(2));
Comment thread
theCodeDrift marked this conversation as resolved.
const runCwd = resolveCwd(rawArguments);
const startedAt = Date.now();
// Resolve identity at invocation START so cli_run reports who *initiated* the
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/src/util/argv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*
* - `-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`).
* citty does not know this on its own; see `joinDirectoryValues`.
* - `--` 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.
Expand Down Expand Up @@ -97,6 +98,45 @@ export function splitRawArguments(
return { positionals, flags, values };
}

/**
* Rewrite `-d <path>` and `--dir <path>` into the one-token `--dir=<path>`.
*
* citty picks a subcommand from the first raw token that does not start with
* `-`, at every level that has subcommands, and it does that before parsing
* any flags. In `taskless auth -d .` that token is `.`, the flag's value, so
* citty reports "Unknown command `.`" before `auth` ever runs. The `=`
* spelling keeps the value inside the flag token, where citty's resolution
* skips it and its parser still reads it as `dir`.
*
* Applied once to argv before dispatch rather than per command, because every
* command with subcommands (the root included) has the same exposure. A value
* that itself starts with `-` is left alone: citty already skips it, and
* joining it would change what citty parses. Tokens after `--` are
* positionals and are never touched.
*/
export function joinDirectoryValues(rawArguments: string[]): string[] {
const joined: string[] = [];
for (let index = 0; index < rawArguments.length; index++) {
const argument = rawArguments[index]!;
if (argument === END_OF_OPTIONS) {
joined.push(...rawArguments.slice(index));
break;
}
const value = rawArguments[index + 1];
if (
DIR_FLAGS.has(argument) &&
value !== undefined &&
!value.startsWith("-")
) {
joined.push(`--dir=${value}`);
index++;
continue;
}
joined.push(argument);
}
return joined;
}

/**
* True when any of `names` appears as a flag token. Scanned through
* `splitRawArguments` so neither a flag value nor a path after `--` that
Expand Down
51 changes: 51 additions & 0 deletions packages/cli/test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,57 @@ describe("cli", () => {
});
});

// citty resolves a subcommand from the first token not starting with `-`,
// so before argv was joined, the path after `-d` was read as a command name
// and every parent with subcommands failed with "Unknown command".
describe("-d <path> before a subcommand", () => {
let temporaryDirectory: string;

beforeEach(async () => {
temporaryDirectory = await mkdtemp(join(tmpdir(), "taskless-test-"));
});

afterEach(async () => {
await rm(temporaryDirectory, { recursive: true, force: true });
});

it("runs auth status against the given directory", async () => {
const environment = { ...process.env };
delete environment.TASKLESS_TOKEN;
const { stdout } = await execFileAsync(
"node",
[binPath, "auth", "-d", temporaryDirectory],
{ env: environment }
);
expect(stdout).toContain("Not logged in.");
});

it("dispatches the root's subcommand after -d", async () => {
const { stdout } = await execFileAsync("node", [
binPath,
"-d",
temporaryDirectory,
"info",
"--json",
]);
expect(JSON.parse(stdout.trim())).toHaveProperty("version");
});

// With no subcommand, `-d` alone still reaches the wizard / agent-index
// fallback rather than usage, in both spellings, once argv is joined.
it.each([["-d"], ["--dir="]])(
"routes a bare %s<path> to the agent index when not a TTY",
async (flag) => {
const argv =
flag === "-d"
? ["-d", temporaryDirectory]
: [`--dir=${temporaryDirectory}`];
const { stderr } = await execFileAsync("node", [binPath, ...argv]);
expect(stderr).toContain("non-interactive context detected");
}
);
});

describe("init", () => {
let temporaryDirectory: string;

Expand Down
55 changes: 54 additions & 1 deletion packages/cli/test/help-flag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import { join, resolve } from "node:path";
import { promisify } from "node:util";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { hasHelpFlag, splitRawArguments } from "../src/util/argv";
import {
hasHelpFlag,
joinDirectoryValues,
splitRawArguments,
} from "../src/util/argv";

const execFileAsync = promisify(execFile);
const binPath = resolve(import.meta.dirname, "../dist/index.js");
Expand Down Expand Up @@ -152,6 +156,55 @@ describe("splitRawArguments", () => {
});
});

describe("joinDirectoryValues", () => {
it.each([
[
["auth", "-d", "."],
["auth", "--dir=."],
],
[
["-d", "/tmp", "info"],
["--dir=/tmp", "info"],
],
[
["auth", "--dir", "/tmp", "login"],
["auth", "--dir=/tmp", "login"],
],
[
["auth", "--dir=."],
["auth", "--dir=."],
],
])("joins %j into %j", (argv, expected) => {
expect(joinDirectoryValues(argv)).toEqual(expected);
});

it("leaves a value that starts with - alone", () => {
expect(joinDirectoryValues(["auth", "-d", "--json"])).toEqual([
"auth",
"-d",
"--json",
]);
});

it("does not touch anything after --", () => {
expect(joinDirectoryValues(["check", "--", "-d", "src"])).toEqual([
"check",
"--",
"-d",
"src",
]);
});

it("does not join -- as the value of -d", () => {
expect(joinDirectoryValues(["check", "-d", "--", "src"])).toEqual([
"check",
"-d",
"--",
"src",
]);
});
});

describe("hasHelpFlag", () => {
it.each([
[["check", "--help"], true],
Expand Down
Loading