diff --git a/integration/cli.test.ts b/integration/cli.test.ts index d60b5a6..75746d7 100644 --- a/integration/cli.test.ts +++ b/integration/cli.test.ts @@ -104,6 +104,11 @@ describe("the entrypoint", () => { ["update", "--help"], ["uninstall", "--help"], ["skill", "--help"], + ["skill", "install", "--help"], + ["skill", "install", "-h"], + ["skill", "list", "help"], + ["skill", "path", "--help"], + ["skill", "uninstall", "--help"], ]) { const result = await s.run(args, { env: NO_BACKEND }); expect({ args, code: result.code }).toEqual({ args, code: 0 }); @@ -111,6 +116,20 @@ describe("the entrypoint", () => { } }); + it("names an unknown skill sub-command whatever args follow", async () => { + // `skill instal --help` is a typo, not a help request or a flag error: it + // must fail naming the bad sub so a script can detect it. + for (const args of [ + ["skill", "instal"], + ["skill", "instal", "--help"], + ["skill", "instal", "foo", "bar"], + ]) { + const result = await s.run(args, { env: NO_BACKEND }); + expect({ args, code: result.code }).toEqual({ args, code: 1 }); + expect(result.err).toMatch(/Unknown skill sub-command: instal/); + } + }); + it("lists the skill's installs with no backend configured", async () => { const result = await s.run(["skill", "list"], { env: NO_BACKEND }); expect(result.code).toBe(0); diff --git a/src/skill.ts b/src/skill.ts index 43c5bb4..8d948fd 100644 --- a/src/skill.ts +++ b/src/skill.ts @@ -1086,46 +1086,64 @@ async function uninstall(deps: SkillDeps, opts: SkillOptions): Promise { return reportOutcomes(deps, outcomes); } +/** The one list of `wego skill` sub-commands; telemetry reports the same set. */ +export const SKILL_SUB_COMMANDS = [ + "install", + "list", + "path", + "uninstall", +] as const; +type SkillSub = (typeof SKILL_SUB_COMMANDS)[number]; +const SKILL_SUB_SET: ReadonlySet = new Set(SKILL_SUB_COMMANDS); + +function isSkillSub(sub: string | undefined): sub is SkillSub { + return sub !== undefined && SKILL_SUB_SET.has(sub); +} + /** `wego skill …` — dispatch to the sub-command handlers. */ export async function skill(args: string[], deps: SkillDeps): Promise { const sub = args[0]; - // `wego skill --help`/`-h`/`help` prints usage on stdout with exit 0 (CLI-3). - if (isHelpArg(args)) { + // `wego skill --help`/`-h`/`help` prints the skill usage on stdout with exit + // 0 (CLI-3). The token right after a sub (`skill install --help`) asks for + // the same usage, the convention the flights/hotels leaf dispatchers use on + // their own arg list after the group stripped the sub. + if (isHelpArg(args) || (isSkillSub(sub) && isHelpArg(args.slice(1)))) { deps.log(SKILL_USAGE); return 0; } - let opts: SkillOptions; - try { - // Inside the try because `list` reads the overlay, so it can fail. - if (sub === "list") { - await list(deps, args.slice(1)); - return 0; - } - opts = parseSkillArgs(args.slice(1)); - } catch (err) { - deps.error(err instanceof Error ? err.message : String(err)); + // Name the typo before leaf parsing, so `skill instal --help` is an unknown + // sub-command, not a flag error. + if (!isSkillSub(sub)) { + deps.error( + sub ? `Unknown skill sub-command: ${sub}\n${SKILL_USAGE}` : SKILL_USAGE, + ); return 1; } try { switch (sub) { - case "install": + case "list": + // `list` reads the overlay, so it can fail. + await list(deps, args.slice(1)); + return 0; + case "install": { + const opts = parseSkillArgs(args.slice(1)); // `--owned-only` is the CLI surface of the `refreshOnly` dep: same // target selection, same never-create rule, one implementation. return await install( opts.ownedOnly ? { ...deps, refreshOnly: true } : deps, opts, ); + } case "path": - return printPath(deps, opts); + return printPath(deps, parseSkillArgs(args.slice(1))); case "uninstall": - return await uninstall(deps, opts); - default: - deps.error( - sub - ? `Unknown skill sub-command: ${sub}\n${SKILL_USAGE}` - : SKILL_USAGE, - ); + return await uninstall(deps, parseSkillArgs(args.slice(1))); + default: { + // A new SKILL_SUB_COMMANDS entry without a case fails this check. + const unhandled: never = sub; + deps.error(`Unknown skill sub-command: ${unhandled}\n${SKILL_USAGE}`); return 1; + } } } catch (err) { // `resolveSkill` throws on an unknown / ambiguous id — surface it as a usage diff --git a/src/telemetry.ts b/src/telemetry.ts index f7e1299..88889b3 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -1,4 +1,5 @@ import { EXIT } from "./error-report"; +import { SKILL_SUB_COMMANDS } from "./skill"; import { isProdTarget, type Target } from "./target"; /** One `cli_command_ran` event per invocation. See `docs/telemetry.md`. */ @@ -47,7 +48,7 @@ const SUBCOMMANDS: Readonly> = { "share", ], hotels: ["search", "results", "details", "reviews", "rooms", "booking-link"], - skill: ["list", "install", "path", "uninstall"], + skill: SKILL_SUB_COMMANDS, // `list`/`set`/`unset` only. The VALUE a `set` carries is the second // positional, which no allowlist reads, so a user's currency never leaves. config: ["list", "set", "unset"],