Merge PR #8995: fix(cli): strip launch-global flags before non-launch subcommands (@roboomp)
This commit is contained in:
@@ -17,6 +17,7 @@
|
||||
- Fixed a submitted `/skill:<name>` 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 <dir> 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 <path>` 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)).
|
||||
|
||||
@@ -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<string, true> = { 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 <dir> 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] };
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 <dir> 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=<dir>` 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"] });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user