From aecadfec1535b24b595c0c1ef81569968886d1bd Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:30:41 +0000 Subject: [PATCH 1/4] fix(skill): answer `skill --help` with usage, exit 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wego skill install --help` (and list/path/uninstall variants) hit the flag parser and exited 1 with "Unknown option: --help" because skill() only ran isHelpArg on the whole argv, which matches args[0] alone. Check isHelpArg(args.slice(1)) too — the same group/leaf help convention the flights/hotels dispatchers already use, and the shape configFor in index.ts already guards with. A help token later in argv still reaches the parser as a normal value/error. Regression in integration/cli.test.ts' no-backend --help matrix: skill install --help and skill list help must answer usage, exit 0. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- integration/cli.test.ts | 2 ++ src/skill.ts | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/integration/cli.test.ts b/integration/cli.test.ts index d60b5a6..bf3b7c5 100644 --- a/integration/cli.test.ts +++ b/integration/cli.test.ts @@ -104,6 +104,8 @@ describe("the entrypoint", () => { ["update", "--help"], ["uninstall", "--help"], ["skill", "--help"], + ["skill", "install", "--help"], + ["skill", "list", "help"], ]) { const result = await s.run(args, { env: NO_BACKEND }); expect({ args, code: result.code }).toEqual({ args, code: 0 }); diff --git a/src/skill.ts b/src/skill.ts index 43c5bb4..166e4ca 100644 --- a/src/skill.ts +++ b/src/skill.ts @@ -1090,7 +1090,10 @@ async function uninstall(deps: SkillDeps, opts: SkillOptions): Promise { 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)) { + // The token right after a sub (`skill install --help`) is that leaf's help — + // same convention as the flights/hotels leaf dispatchers, which call + // `isHelpArg` on their own arg list after the group stripped the sub. + if (isHelpArg(args) || isHelpArg(args.slice(1))) { deps.log(SKILL_USAGE); return 0; } From 9562691dc81e2baef1c5a25db147599fb3b3f98b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:02:49 +0000 Subject: [PATCH 2/4] fix(skill): keep `skill --help` a failure Devin Review on #106: the second-position help check ran before the sub dispatch, so `wego skill instal --help` exited 0. Gate it on a recognized sub-command; released v1.4.1 already exits 1 there. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- integration/cli.test.ts | 11 +++++++++++ src/skill.ts | 10 ++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/integration/cli.test.ts b/integration/cli.test.ts index bf3b7c5..86cbbb2 100644 --- a/integration/cli.test.ts +++ b/integration/cli.test.ts @@ -113,6 +113,17 @@ describe("the entrypoint", () => { } }); + it("rejects --help on an unknown skill sub-command", async () => { + // `skill instal --help` is a typo, not a help request: it must fail so a + // script can detect the unknown sub-command. + const result = await s.run(["skill", "instal", "--help"], { + env: NO_BACKEND, + }); + expect(result.code).toBe(1); + // parseSkillArgs sees `--help` before the sub dispatch names the typo. + expect(result.err).toMatch(/Unknown option: --help/); + }); + 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 166e4ca..ee9123f 100644 --- a/src/skill.ts +++ b/src/skill.ts @@ -1086,14 +1086,20 @@ async function uninstall(deps: SkillDeps, opts: SkillOptions): Promise { return reportOutcomes(deps, outcomes); } +const SUB_COMMANDS = new Set(["install", "list", "path", "uninstall"]); + /** `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). // The token right after a sub (`skill install --help`) is that leaf's help — // same convention as the flights/hotels leaf dispatchers, which call - // `isHelpArg` on their own arg list after the group stripped the sub. - if (isHelpArg(args) || isHelpArg(args.slice(1))) { + // `isHelpArg` on their own arg list after the group stripped the sub. Gate it + // on a real sub so `skill instal --help` still fails as an unknown command. + if ( + isHelpArg(args) || + (sub !== undefined && SUB_COMMANDS.has(sub) && isHelpArg(args.slice(1))) + ) { deps.log(SKILL_USAGE); return 0; } From 5b15696e4db0896db13db1cbce3517e803e25a7e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:39:45 +0000 Subject: [PATCH 3/4] fix(skill): name the unknown sub-command before leaf parsing Review on #106: dispatch checked args before the sub, so a typo got a flag error. Check the sub first, keep one canonical list (SKILL_SUB_COMMANDS, shared with telemetry), exhaustive switch, and extend the help matrix to every sub. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- integration/cli.test.ts | 24 +++++++++------ src/skill.ts | 67 +++++++++++++++++++++++------------------ src/telemetry.ts | 3 +- 3 files changed, 55 insertions(+), 39 deletions(-) diff --git a/integration/cli.test.ts b/integration/cli.test.ts index 86cbbb2..75746d7 100644 --- a/integration/cli.test.ts +++ b/integration/cli.test.ts @@ -105,7 +105,10 @@ describe("the entrypoint", () => { ["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 }); @@ -113,15 +116,18 @@ describe("the entrypoint", () => { } }); - it("rejects --help on an unknown skill sub-command", async () => { - // `skill instal --help` is a typo, not a help request: it must fail so a - // script can detect the unknown sub-command. - const result = await s.run(["skill", "instal", "--help"], { - env: NO_BACKEND, - }); - expect(result.code).toBe(1); - // parseSkillArgs sees `--help` before the sub dispatch names the typo. - expect(result.err).toMatch(/Unknown option: --help/); + 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 () => { diff --git a/src/skill.ts b/src/skill.ts index ee9123f..8d948fd 100644 --- a/src/skill.ts +++ b/src/skill.ts @@ -1086,55 +1086,64 @@ async function uninstall(deps: SkillDeps, opts: SkillOptions): Promise { return reportOutcomes(deps, outcomes); } -const SUB_COMMANDS = new Set(["install", "list", "path", "uninstall"]); +/** 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). - // The token right after a sub (`skill install --help`) is that leaf's help — - // same convention as the flights/hotels leaf dispatchers, which call - // `isHelpArg` on their own arg list after the group stripped the sub. Gate it - // on a real sub so `skill instal --help` still fails as an unknown command. - if ( - isHelpArg(args) || - (sub !== undefined && SUB_COMMANDS.has(sub) && isHelpArg(args.slice(1))) - ) { + // `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"], From 2bcf6b0e2fbb3ae2cd1831a4daf36a15687e97e8 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:45:15 +0000 Subject: [PATCH 4/4] ci: re-run after transient bun audit connection failure Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>