diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bde1313a3..a27217aef 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed advisor turns hammering the same usage-limited account: a failed advisor turn now marks the exhausted credential blocked (with the provider's retry hint and usage-report reset time), so the next retry rotates to a sibling instead of re-picking the blocked account every few seconds. Previously the in-stream auth retry rotated within a request but never blocked the last failing credential, and the advisor loop — unlike the primary retry pipeline — never called `markUsageLimitReached`. - Added the account key to the `codex-auto-reset: skipped` debug log so skip reasons (e.g. `weekly-not-exhausted`) can be attributed to the evaluated account. +- Fixed documented `omp marketplace`/`discover`/`upgrade`/`uninstall`/`enable`/`disable` CLI verbs silently leaking to the model as a launch prompt instead of managing plugins. `omp marketplace add xyz` (and similar multi-word invocations following the documented `omp plugin ` grammar) now surface a hint pointing at the real `omp plugin ` command, while genuine prose prompts beginning with these words still route to `launch` ([#4845](https://github.com/can1357/oh-my-pi/issues/4845)). ## [16.3.11] - 2026-07-06 diff --git a/packages/coding-agent/src/cli-commands.ts b/packages/coding-agent/src/cli-commands.ts index fb2b04072..d141c7d2c 100644 --- a/packages/coding-agent/src/cli-commands.ts +++ b/packages/coding-agent/src/cli-commands.ts @@ -46,31 +46,62 @@ export const commands: CommandEntry[] = [ { name: "search", load: () => import("./commands/web-search").then(m => m.default), aliases: ["q"] }, ]; -// Documented-looking plugin-management verbs that are NOT registered top-level -// commands. Without a guard `resolveCliArgv` rewrites e.g. `omp list` to -// `omp launch list`, silently forwarding the bare verb to the model as a prompt -// instead of managing plugins (#2935; same class as the `install` leak fixed in -// #1496/#1498). A bare (single-arg) use gets a hint pointing at the real -// `omp plugin ` command; multi-word invocations still fall through to -// `launch`, so genuine prompts that merely begin with one of these words work. -const RESERVED_TOP_LEVEL_WORDS = new Map([ - [ - "extensions", +// Documented-looking plugin/marketplace verbs that are NOT registered top-level +// commands. Without a guard `resolveCliArgv` rewrites e.g. `omp marketplace add +// xyz` to `omp launch marketplace add xyz`, silently forwarding the argv to the +// model as a prompt instead of managing plugins (#4845; same class as the +// `list`/`remove` leak fixed in #2935 and the `install` leak in #1496/#1498). +// The real commands live under `omp plugin `; each entry maps a verb to +// a hint pointing there. See {@link reservedTopLevelWordMessage} for when a hint +// fires vs. when the argv still falls through to `launch`. +const RESERVED_TOP_LEVEL_WORDS: Record = { + extensions: '`omp extensions` is not a management command. Use `omp plugin list` / `omp plugin install`, or run `omp launch extensions` if you meant to send "extensions" as a prompt.', - ], - [ - "list", - '`omp list` is not a top-level command. Use `omp plugin list` to list installed plugins, or run `omp launch list` if you meant to send "list" as a prompt.', - ], - [ - "remove", + list: '`omp list` is not a top-level command. Use `omp plugin list` to list installed plugins, or run `omp launch list` if you meant to send "list" as a prompt.', + remove: '`omp remove` is not a top-level command. Use `omp plugin uninstall ` to remove a plugin, or run `omp launch remove` if you meant to send "remove" as a prompt.', - ], -]); + uninstall: + '`omp uninstall` is not a top-level command. Use `omp plugin uninstall ` to remove a plugin, or run `omp launch uninstall` if you meant to send "uninstall" as a prompt.', + marketplace: + '`omp marketplace` is not a top-level command. Use `omp plugin marketplace ` to manage marketplaces, or run `omp launch marketplace` if you meant to send "marketplace" as a prompt.', + discover: + '`omp discover` is not a top-level command. Use `omp plugin discover [marketplace]` to browse available plugins, or run `omp launch discover` if you meant to send "discover" as a prompt.', + upgrade: + '`omp upgrade` is not a top-level command. Use `omp plugin upgrade [name@marketplace]` to upgrade plugins, or run `omp launch upgrade` if you meant to send "upgrade" as a prompt.', + enable: + '`omp enable` is not a top-level command. Use `omp plugin enable ` to enable a plugin, or run `omp launch enable` if you meant to send "enable" as a prompt.', + disable: + '`omp disable` is not a top-level command. Use `omp plugin disable ` to disable a plugin, or run `omp launch disable` if you meant to send "disable" as a prompt.', +}; -export function reservedTopLevelWordMessage(first: string | undefined, argc = 1): string | undefined { - if (argc !== 1 || !first || first.startsWith("-") || first.startsWith("@")) return undefined; - return RESERVED_TOP_LEVEL_WORDS.get(first); +// Sub-actions that make `omp marketplace ` unambiguously a management +// command even when multi-word (the reporter's `omp marketplace add xyz`, +// #4845). Mirrors the switch in `handleMarketplace` (cli/plugin-cli.ts). +const MARKETPLACE_SUBCOMMANDS: Record = { add: true, remove: true, rm: true, update: true, list: true }; + +/** + * Hint for a reserved plugin/marketplace verb used as a top-level command, or + * `undefined` when the argv should fall through to `launch`. + * + * A bare verb (`omp marketplace`) always hints. A multi-word invocation only + * hints when the arguments follow the documented plugin grammar — a marketplace + * sub-action (`omp marketplace add …`) or a `name@marketplace` plugin id + * (`omp uninstall foo@bar`) — so genuine prompts that merely begin with one of + * these words (`omp list all my files`, `omp upgrade the deps`) still launch. + * + * Flags (`-…`) and `@file` arguments in the verb slot are never management + * commands; those fall through to the default `launch` command. + */ +export function reservedTopLevelWordMessage(argv: readonly string[]): string | undefined { + const first = argv[0]; + if (!first || first.startsWith("-") || first.startsWith("@")) return undefined; + const hint = RESERVED_TOP_LEVEL_WORDS[first]; + if (!hint) return undefined; + const second = argv[1]; + if (second === undefined) return hint; + if (first === "marketplace" && MARKETPLACE_SUBCOMMANDS[second]) return hint; + if (second.includes("@")) return hint; + return undefined; } /** @@ -112,7 +143,7 @@ function leadingSubcommandIndex(argv: string[]): number { */ export function resolveCliArgv(argv: string[]): ResolvedCliArgv { const first = argv[0]; - const reservedMessage = reservedTopLevelWordMessage(first, argv.length); + const reservedMessage = reservedTopLevelWordMessage(argv); if (reservedMessage) return { error: reservedMessage }; if (first === "--help" || first === "-h" || first === "--version" || first === "-v" || first === "help") { return { argv }; diff --git a/packages/coding-agent/test/plugin-verb-launch-leak.test.ts b/packages/coding-agent/test/plugin-verb-launch-leak.test.ts index a13759fe7..1714ae26c 100644 --- a/packages/coding-agent/test/plugin-verb-launch-leak.test.ts +++ b/packages/coding-agent/test/plugin-verb-launch-leak.test.ts @@ -1,16 +1,20 @@ /** - * Regression test for #2935: the plugins docs advertise `omp list` / `omp remove` + * Regression test for #2935 and #4845: the plugins/marketplace docs advertise + * `omp list` / `omp remove` / `omp marketplace ` / `omp uninstall …` etc. * as top-level commands, but only `omp install` is registered. Before the fix, * `resolveCliArgv(["list"])` rewrote the bare verb to `["launch", "list"]`, so * `omp list` silently started an interactive agent session with "list" as the * initial LLM prompt instead of managing plugins (the real command is - * `omp plugin list`). Same footgun for `omp remove`. + * `omp plugin list`). #4845 extended the same footgun to the multi-word + * documented grammar: `omp marketplace add xyz` leaked the whole argv to the + * model as a prompt. * - * These tests pin the chosen bugfix: a bare, single-arg documented plugin verb - * yields a helpful hint pointing at the real `omp plugin ` command - * rather than leaking the word to the model — while multi-word invocations that - * merely happen to begin with one of these verbs still fall through to `launch` - * so genuine prompts are unaffected. + * These tests pin the chosen bugfix: a documented plugin/marketplace verb that + * is bare, or that follows the documented grammar (a marketplace sub-action or a + * `name@marketplace` plugin id), yields a helpful hint pointing at the real + * `omp plugin ` command rather than leaking to the model — while + * genuine prose prompts that merely begin with one of these words still fall + * through to `launch`. * * Imported via a relative path (not the `@oh-my-pi/pi-coding-agent` alias) so the * assertions exercise this checkout's `cli-commands.ts` directly. @@ -52,4 +56,45 @@ describe("documented-but-unregistered plugin verbs do not leak to launch (#2935) expect(isSubcommand("list")).toBe(false); expect(isSubcommand("remove")).toBe(false); }); + + test("multi-word `omp marketplace add xyz` hints at `omp plugin marketplace` instead of leaking to the prompt (#4845)", () => { + const result = resolveCliArgv(["marketplace", "add", "xyz"]); + expect(result).not.toEqual({ argv: ["launch", "marketplace", "add", "xyz"] }); + expect(result).not.toHaveProperty("argv"); + expect(result).toHaveProperty("error"); + expect("error" in result && result.error).toContain("omp plugin marketplace"); + }); + + test("bare marketplace-family verbs hint at their `omp plugin` command (#4845)", () => { + for (const [verb, hint] of [ + ["marketplace", "omp plugin marketplace"], + ["discover", "omp plugin discover"], + ["upgrade", "omp plugin upgrade"], + ["uninstall", "omp plugin uninstall"], + ["enable", "omp plugin enable"], + ["disable", "omp plugin disable"], + ] as const) { + const result = resolveCliArgv([verb]); + expect(result).not.toHaveProperty("argv"); + expect("error" in result && result.error).toContain(hint); + } + }); + + test("`name@marketplace` plugin ids hint instead of launching (#4845)", () => { + for (const verb of ["uninstall", "upgrade", "enable", "disable"] as const) { + const result = resolveCliArgv([verb, "code-review@claude-plugins-official"]); + expect(result).not.toHaveProperty("argv"); + expect(result).toHaveProperty("error"); + } + }); + + test("prose prompts beginning with the new verbs still route to launch (#4845)", () => { + expect(resolveCliArgv(["upgrade", "the", "deps"])).toEqual({ + argv: ["launch", "upgrade", "the", "deps"], + }); + // `marketplace` followed by a non-subcommand word is a genuine prompt. + expect(resolveCliArgv(["marketplace", "research", "for", "me"])).toEqual({ + argv: ["launch", "marketplace", "research", "for", "me"], + }); + }); });