diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ca3d36810..fc31a604f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -17,6 +17,7 @@ - Fixed a submitted `/skill:` command staying invisible in the transcript until its awaited preflight (memory recall, `before_agent_start` hooks, auto-thinking classification, pre-prompt compaction) finished, so a slow step such as a Hindsight auto-recall timeout made the command look unaccepted. Idle skill submissions now paint an optimistic row immediately — like a normal prompt — and reconcile it in place when the canonical `message_start` lands ([#8895](https://github.com/can1357/oh-my-pi/issues/8895)). - Fixed broker-backed MCP OAuth credentials never refreshing, so remote OAuth MCP servers dropped out of `/mcp` once their access token expired under `omp auth-broker serve`. The client threw on the broker-redacted refresh sentinel instead of asking the broker to refresh, and the broker had no `mcp_oauth:*` refresh path (`POST /v1/credential/:id/refresh` answered `Unknown OAuth provider`). The client now routes redacted MCP refreshes through the broker, and the broker refreshes MCP credentials with a generic `refresh_token` grant from the credential's embedded token endpoint and client id — so the background refresher also keeps MCP tokens live ([#8933](https://github.com/can1357/oh-my-pi/issues/8933)). - Fixed `omp commit` split-commit failing with `corrupt binary patch` when a split commit contains a binary file. `parseFileDiffs` split the captured diff on `"\ndiff --git "`, consuming the `\n` that terminates each block, and `patch.join` stripped trailing newlines — both dropped the blank line that terminates a `GIT binary patch` block, so the rebuilt patch was rejected by `git apply --binary`. Both trailing and mid-diff binary blocks now survive the parse/rebuild round-trip byte-exact ([#8899](https://github.com/can1357/oh-my-pi/issues/8899)). +- Fixed `omp update` (and other non-launch subcommands) crashing with `error: Unknown option '--cwd'` when a leading global launch flag preceded the subcommand — e.g. a shell alias/wrapper that runs `omp --cwd update`. `resolveCliArgv` hoisted the subcommand to the front but forwarded the launch-only flag into `update`'s strict `node:util.parseArgs` parser, which rejected it. Launch-global flags before a launch-shaped command (`acp`/`launch`) are still forwarded; before any other subcommand they are now stripped as inapplicable ([#8891](https://github.com/can1357/oh-my-pi/issues/8891)). - Fixed Claude Code marketplace plugins ignoring the `enabledPlugins` switch in `~/.claude/settings.json` and `.claude/settings(.local).json`: a plugin turned off for a project no longer loads there, and a local-scope install enabled for a project loads even when its recorded `projectPath` is a different directory - Fixed revived subagents (warm lifecycle reviver and cold persisted reviver) rebuilding the session without initializing the extension runtime, leaving every runtime action throwing `ExtensionRuntimeNotInitializedError`. An extension with a `tool_call` handler that touched a runtime action (e.g. `appendEntry`) then tripped the fail-closed gate in `emitToolCall` and blocked every tool — including the hidden `yield` — so the revived agent could neither finish nor exit and looped until killed. Both revivers now call the shared `initializeExtensions` helper, restoring runtime actions, `onError`, and the `session_start` event ([#8824](https://github.com/can1357/oh-my-pi/issues/8824)). - Fixed `omp commit` split-commit crashing with a misleading `No diff found for ` when a staged binary (or any payload) pushed `git diff --cached --binary` past the 8 MiB subprocess output cap. The capture is truncated silently, so files sorting after the binary vanished from the parsed diff; the split flow now requests a complete diff and fails fast naming the real cause instead ([#8897](https://github.com/can1357/oh-my-pi/issues/8897)). diff --git a/packages/coding-agent/src/cli-commands.ts b/packages/coding-agent/src/cli-commands.ts index dbc797e7d..bb6946745 100644 --- a/packages/coding-agent/src/cli-commands.ts +++ b/packages/coding-agent/src/cli-commands.ts @@ -10,7 +10,13 @@ */ import type { CommandEntry } from "@oh-my-pi/pi-utils/cli"; import * as commandHelp from "./cli/command-help"; -import { flagConsumesValue } from "./cli/flag-tables"; +import { + EXTENSION_SHADOWABLE_STRING_FLAGS, + flagConsumesValue, + OPTIONAL_VALUE_FLAGS, + STRING_VALUE_FLAGS, + VALUELESS_FLAGS, +} from "./cli/flag-tables"; import { launchHelp } from "./commands/launch-help"; export const commands: CommandEntry[] = [ @@ -285,12 +291,54 @@ function leadingSubcommandIndex(argv: string[]): number { return -1; } +/** + * Subcommands that share the launch flag surface, so leading global flags + * (`--cwd`, `--model`, `--approval-mode`, …) placed before them are meaningful + * and must be forwarded ({@link resolveCliArgv}, #2970). Every other subcommand + * parses only its own flags. + */ +export const LAUNCH_FLAG_COMMANDS: Record = { launch: true, acp: true }; + +/** Whether `arg` names a flag from the launch surface (bare or `--flag=value`). */ +function isLaunchGlobalFlag(arg: string): boolean { + const eq = arg.indexOf("="); + const name = arg.startsWith("--") && eq !== -1 ? arg.slice(0, eq) : arg; + return ( + STRING_VALUE_FLAGS.has(name) || + OPTIONAL_VALUE_FLAGS.has(name) || + VALUELESS_FLAGS.has(name) || + EXTENSION_SHADOWABLE_STRING_FLAGS.has(name) + ); +} + +/** + * Drop recognized launch-global flags (and any value they consume) from the + * leading segment before a hoisted non-launch subcommand. `--cwd` and friends + * belong to the launch surface and mean nothing to a subcommand like `update`, + * whose strict parser would otherwise reject them with a cryptic + * `node:util.parseArgs` error (#8891). Tokens the launch tables don't recognize + * are kept, so a subcommand's own leading flags still reach it. + */ +function stripLaunchGlobalFlags(leading: readonly string[]): string[] { + const kept: string[] = []; + for (let index = 0; index < leading.length; index += 1) { + const arg = leading[index]; + if (isLaunchGlobalFlag(arg)) { + if (flagConsumesValue(arg, leading[index + 1])) index += 1; + continue; + } + kept.push(arg); + } + return kept; +} + /** * Decide what the CLI runner should do with raw argv: reject bare reserved * management words, pass help/version through untouched, route a recognized * subcommand (even behind leading global flags like `--approval-mode=yolo`) to - * that command with the flags preserved, and forward everything else to - * `launch` (#2970). + * that command, and forward everything else to `launch` (#2970). Leading + * launch-global flags are forwarded to launch-shaped commands but stripped for + * other subcommands that cannot parse them (#8891). */ export function resolveCliArgv(argv: string[]): ResolvedCliArgv { const first = argv[0]; @@ -302,12 +350,18 @@ export function resolveCliArgv(argv: string[]): ResolvedCliArgv { if (isSubcommand(first)) return { argv }; // A subcommand can hide behind leading global option flags // (`omp --approval-mode=yolo acp`). `run` dispatches strictly on argv[0], so - // hoist the subcommand to the front and keep the leading flags as its own - // argv; the command's parser then applies them. Genuine launch prompts (no - // trailing subcommand) are untouched. + // hoist the subcommand to the front. Launch-shaped commands share the launch + // flag surface, so their leading flags are forwarded and applied; every other + // subcommand parses only its own flags, so launch-global flags placed before + // it (`omp --cwd update`) are stripped rather than forwarded into a + // crash (#8891). Genuine launch prompts (no trailing subcommand) are untouched. const subIndex = leadingSubcommandIndex(argv); if (subIndex >= 0) { - return { argv: [argv[subIndex], ...argv.slice(0, subIndex), ...argv.slice(subIndex + 1)] }; + const sub = argv[subIndex]; + const leading = argv.slice(0, subIndex); + const trailing = argv.slice(subIndex + 1); + const forwardedLeading = LAUNCH_FLAG_COMMANDS[sub] === true ? leading : stripLaunchGlobalFlags(leading); + return { argv: [sub, ...forwardedLeading, ...trailing] }; } return { argv: ["launch", ...argv] }; } diff --git a/packages/coding-agent/src/cli/profile-bootstrap.ts b/packages/coding-agent/src/cli/profile-bootstrap.ts index ec6fd12e8..e1c4347e2 100644 --- a/packages/coding-agent/src/cli/profile-bootstrap.ts +++ b/packages/coding-agent/src/cli/profile-bootstrap.ts @@ -33,7 +33,7 @@ * them also activates (`omp --print --profile work`). */ -import { isSubcommand } from "../cli-commands"; +import { isSubcommand, LAUNCH_FLAG_COMMANDS } from "../cli-commands"; import { EXTENSION_SHADOWABLE_STRING_FLAGS, isUnknownLongValueCandidate, @@ -43,10 +43,6 @@ import { STRING_VALUE_FLAGS, } from "./flag-tables"; -function isProfileBootstrapSubcommand(arg: string): boolean { - return arg === "launch" || arg === "acp"; -} - function needsBoundaryAfterGlobalStrip(stripped: readonly string[]): boolean { const previous = stripped[stripped.length - 1]; return ( @@ -222,7 +218,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe // any other token has been forwarded, later subcommand names are launch text. // `launch` and `acp` are explicit spellings of launch-shaped commands, so // global launch flags that follow them must still be extracted. - if (canDispatchSubcommand && isSubcommand(arg) && !isProfileBootstrapSubcommand(arg)) { + if (canDispatchSubcommand && isSubcommand(arg) && LAUNCH_FLAG_COMMANDS[arg] !== true) { sawSubcommand = true; } canDispatchSubcommand = false; diff --git a/packages/coding-agent/test/cli-argv-routing.test.ts b/packages/coding-agent/test/cli-argv-routing.test.ts index c31892b44..c29ce7d96 100644 --- a/packages/coding-agent/test/cli-argv-routing.test.ts +++ b/packages/coding-agent/test/cli-argv-routing.test.ts @@ -62,3 +62,33 @@ describe("resolveCliArgv routes subcommands hidden behind leading global flags", }); }); }); + +describe("resolveCliArgv strips launch-global flags before non-launch subcommands (#8891)", () => { + test("`--cwd update` drops the inapplicable launch flag instead of forwarding it", () => { + // Forwarding `--cwd` into update's strict parser crashed with + // `Unknown option '--cwd'`; the launch-only flag is now dropped. + expect(resolveCliArgv(["--cwd", "/tmp", "update"])).toEqual({ argv: ["update"] }); + }); + + test("`--cwd=` inline form is stripped too", () => { + expect(resolveCliArgv(["--cwd=/tmp", "update"])).toEqual({ argv: ["update"] }); + }); + + test("a trailing subcommand flag survives while the leading launch flag is stripped", () => { + expect(resolveCliArgv(["--cwd", "/tmp", "update", "--force"])).toEqual({ + argv: ["update", "--force"], + }); + }); + + test("multiple leading launch flags are all stripped before a non-launch subcommand", () => { + expect(resolveCliArgv(["--model", "gpt", "--cwd", "/x", "update"])).toEqual({ argv: ["update"] }); + }); + + test("a subcommand's own flag placed before it is kept, not treated as launch-global", () => { + expect(resolveCliArgv(["-c", "update"])).toEqual({ argv: ["update", "-c"] }); + }); + + test("launch-shaped `acp` still receives forwarded launch-global flags", () => { + expect(resolveCliArgv(["--cwd", "/x", "acp"])).toEqual({ argv: ["acp", "--cwd", "/x"] }); + }); +});