From 26ead028058527ec21d0d6394aec955dd45a16c2 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 12 Aug 2026 05:54:43 +0200 Subject: [PATCH] fix(cli): report an unknown install target as an argument error `veryfront install not-a-tool` exited 1. The target was only checked inside `installCommand`, where `parseTargetFlag` throws a plain runtime error, so a typo was indistinguishable from an installation failure. AGENTS.md reserves exit 1 for runtime errors and exit 2 for usage and argument errors. Validate the target while parsing arguments instead, so the failure goes through `parseArgsOrThrow` and the router's "Invalid ..." usage path and the message names the valid targets. `--target not-a-tool` takes the same path and also exits 2 now. `parseTargetFlag` keeps its own check for programmatic callers. The positional-target fix this branch originally carried landed on main in #3609, so only the exit-code correction remains here. The end-to-end case #3609 added for an unknown positional asserted exit 1 and is updated to 2. --- cli/commands/install/handler.test.ts | 11 +++++++++++ cli/commands/install/handler.ts | 9 +++++++-- .../install/install.integration.test.ts | 19 +++++++++++++++++-- cli/commands/install/install.ts | 8 ++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/cli/commands/install/handler.test.ts b/cli/commands/install/handler.test.ts index e2079bd99c..36d4bb8da5 100644 --- a/cli/commands/install/handler.test.ts +++ b/cli/commands/install/handler.test.ts @@ -151,6 +151,17 @@ describe("commands/install/handler", () => { it("reads the positional for uninstall too", () => { assertEquals(parseCommandLine(["uninstall", "agents"]).target, "agents"); }); + + it("fails argument parsing for an unknown positional tool id", () => { + const result = parseInstallArgs(parseCliArgs(["install", "not-a-tool"])); + assertEquals(result.success, false); + assertEquals(result.error?.message.includes("Valid targets"), true); + }); + + it("fails argument parsing for an unknown --target too", () => { + const result = parseInstallArgs(parseCliArgs(["install", "--target", "not-a-tool"])); + assertEquals(result.success, false); + }); }); describe("uninstall argument extraction", () => { diff --git a/cli/commands/install/handler.ts b/cli/commands/install/handler.ts index e28c82051c..5a0c3fc0d0 100644 --- a/cli/commands/install/handler.ts +++ b/cli/commands/install/handler.ts @@ -3,14 +3,19 @@ */ import { defineSchema, lazySchema } from "veryfront/schemas"; -import { installCommand } from "./install.ts"; +import { installCommand, isValidTargetSpec, VALID_TARGET_VALUES } from "./install.ts"; import { uninstallCommand } from "./uninstall.ts"; import { CommonArgs, createArgParser, parseArgsOrThrow } from "#cli/shared/args"; import type { ParsedArgs } from "#cli/shared/types"; const getInstallArgsSchema = defineSchema((v) => v.object({ - target: v.string().optional(), + // Validated here rather than at install time so an unknown tool id is an + // argument error (exit 2), not a runtime failure (exit 1). + target: v.string().optional().refine( + (value) => value === undefined || isValidTargetSpec(value), + { message: `unknown tool. Valid targets: ${VALID_TARGET_VALUES}` }, + ), global: v.boolean().default(false), force: v.boolean().default(false), }) diff --git a/cli/commands/install/install.integration.test.ts b/cli/commands/install/install.integration.test.ts index e336694f55..a2eba541e7 100644 --- a/cli/commands/install/install.integration.test.ts +++ b/cli/commands/install/install.integration.test.ts @@ -194,8 +194,23 @@ describe("install command integration", () => { }); it("fails instead of installing something else for an unknown target", async () => { - const { code } = await runInstallArgs(["unknown-tool", "--force", "--no-input"]); - assertEquals(code, 1); + const { code, output } = await runInstallArgs(["unknown-tool", "--force", "--no-input"]); + // AGENTS.md reserves exit 2 for usage and argument errors. + assertEquals(code, 2); + assertEquals(output.includes("Valid targets"), true); + + await assertFileNotExists(join(tempDir, "SKILL.md")); + await assertFileNotExists(join(tempDir, "AGENTS.md")); + }); + + it("fails an unknown --target with the same usage exit code", async () => { + const { code } = await runInstallArgs([ + "--target", + "unknown-tool", + "--force", + "--no-input", + ]); + assertEquals(code, 2); await assertFileNotExists(join(tempDir, "SKILL.md")); await assertFileNotExists(join(tempDir, "AGENTS.md")); diff --git a/cli/commands/install/install.ts b/cli/commands/install/install.ts index f898cbd7ea..40251e5d30 100644 --- a/cli/commands/install/install.ts +++ b/cli/commands/install/install.ts @@ -37,6 +37,14 @@ export function parseTargetFlag(target: string): AIToolId[] { return TargetFlagSchema.parse(target); } +/** Valid target values, for messages and argument validation. */ +export const VALID_TARGET_VALUES = [...AI_TOOLS.map((t) => t.id), "all"].join(", "); + +/** True when the value names at least one known tool (or `all`). */ +export function isValidTargetSpec(target: string): boolean { + return TargetFlagSchema.safeParse(target).success; +} + const getAIToolIdArraySchema = defineSchema((v) => v.array(AIToolIdSchema).min(1)); const AIToolIdArraySchema = lazySchema(getAIToolIdArraySchema);