Skip to content

Commit c1f9087

Browse files
ml12580steipete
andauthored
fix(cli): accept parent options placed after lazy subcommands (#55563 regression 1) [AI-assisted] (#94431)
* fix(cli): restore lazy parent option ordering Co-authored-by: ml12580 <[email protected]> * chore: defer release-owned changelog entry --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent c9c4226 commit c1f9087

3 files changed

Lines changed: 283 additions & 8 deletions

File tree

extensions/browser/src/cli/browser-cli.lazy.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,28 @@ describe("registerBrowserCli lazy browser subcommands", () => {
180180
expect(tabsCommand.parent?.opts().json).toBe(true);
181181
});
182182

183+
it("accepts the shipped trailing browser profile order after lazy loading", async () => {
184+
const program = new Command().name("openclaw").enablePositionalOptions();
185+
registerBrowserCli(program, [
186+
"node",
187+
"openclaw",
188+
"browser",
189+
"tabs",
190+
"--browser-profile",
191+
"remote",
192+
]);
193+
194+
await program.parseAsync(["browser", "tabs", "--browser-profile", "remote"], {
195+
from: "user",
196+
});
197+
198+
const tabsCommand = requireTrailingCommand(
199+
requireFirstCall(manageMocks.tabsAction, "tabs action call"),
200+
"tabs action",
201+
);
202+
expect(tabsCommand.parent?.opts().browserProfile).toBe("remote");
203+
});
204+
183205
it("skips browser option values when selecting the lazy command group", async () => {
184206
const program = new Command();
185207
program.name("openclaw");

src/cli/program/action-reparse.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,25 @@ function deleteRawArgs(command: Command): void {
2424
delete (command as Command & { rawArgs?: string[] }).rawArgs;
2525
}
2626

27+
async function expectReparseArgv(params: {
28+
parent: Command;
29+
action: Command;
30+
argv: string[];
31+
expected: string[];
32+
}): Promise<void> {
33+
let root = params.parent;
34+
while (root.parent) {
35+
root = root.parent;
36+
}
37+
setRawArgs(root, params.argv);
38+
buildParseArgvMock.mockReturnValue(params.argv);
39+
const parseAsync = vi.spyOn(root, "parseAsync").mockResolvedValue(root);
40+
41+
await reparseProgramFromActionArgs(params.parent, [params.action]);
42+
43+
expect(parseAsync).toHaveBeenCalledWith(params.expected);
44+
}
45+
2746
describe("reparseProgramFromActionArgs", () => {
2847
beforeEach(() => {
2948
vi.clearAllMocks();
@@ -168,4 +187,91 @@ describe("reparseProgramFromActionArgs", () => {
168187
});
169188
expect(parseAsync).toHaveBeenCalled();
170189
});
190+
191+
it("hoists a trailing lazy-parent option before the loaded command", async () => {
192+
const root = new Command().name("openclaw");
193+
const browser = root.command("browser").option("--browser-profile <name>");
194+
const tabs = browser.command("tabs");
195+
await expectReparseArgv({
196+
parent: browser,
197+
action: tabs,
198+
argv: ["node", "openclaw", "browser", "tabs", "--browser-profile", "remote"],
199+
expected: ["node", "openclaw", "browser", "--browser-profile", "remote", "tabs"],
200+
});
201+
});
202+
203+
it("skips root option values that match the parent command name", async () => {
204+
const root = new Command().name("openclaw").option("--profile <name>");
205+
const browser = root.command("browser").option("--browser-profile <name>");
206+
const tabs = browser.command("tabs");
207+
await expectReparseArgv({
208+
parent: browser,
209+
action: tabs,
210+
argv: [
211+
"node",
212+
"openclaw",
213+
"--profile",
214+
"browser",
215+
"browser",
216+
"tabs",
217+
"--browser-profile",
218+
"remote",
219+
],
220+
expected: [
221+
"node",
222+
"openclaw",
223+
"--profile",
224+
"browser",
225+
"browser",
226+
"--browser-profile",
227+
"remote",
228+
"tabs",
229+
],
230+
});
231+
});
232+
233+
it("hoists parent options after nested lazy commands", async () => {
234+
const root = new Command().name("openclaw");
235+
const browser = root.command("browser").option("--browser-profile <name>");
236+
const tab = browser.command("tab");
237+
tab.command("new");
238+
await expectReparseArgv({
239+
parent: browser,
240+
action: tab,
241+
argv: ["node", "openclaw", "browser", "tab", "new", "--browser-profile", "work"],
242+
expected: ["node", "openclaw", "browser", "--browser-profile", "work", "tab", "new"],
243+
});
244+
});
245+
246+
it("leaves a child-owned option collision after the child command", async () => {
247+
const root = new Command().name("openclaw");
248+
const browser = root.command("browser").option("--json");
249+
const extension = browser.command("extension");
250+
extension.command("path");
251+
extension.command("pair").option("--json");
252+
const argv = ["node", "openclaw", "browser", "extension", "pair", "--json"];
253+
await expectReparseArgv({ parent: browser, action: extension, argv, expected: argv });
254+
});
255+
256+
it("hoists a parent option when only a sibling command owns the same flag", async () => {
257+
const root = new Command().name("openclaw");
258+
const browser = root.command("browser").option("--url <url>");
259+
const cookies = browser.command("cookies");
260+
cookies.command("list");
261+
cookies.command("set").option("--url <url>");
262+
await expectReparseArgv({
263+
parent: browser,
264+
action: cookies,
265+
argv: ["node", "openclaw", "browser", "cookies", "list", "--url", "ws://gateway"],
266+
expected: ["node", "openclaw", "browser", "--url", "ws://gateway", "cookies", "list"],
267+
});
268+
});
269+
270+
it("keeps a missing parent option value after the loaded command", async () => {
271+
const root = new Command().name("openclaw");
272+
const browser = root.command("browser").option("--browser-profile <name>");
273+
const tabs = browser.command("tabs");
274+
const argv = ["node", "openclaw", "browser", "tabs", "--browser-profile"];
275+
await expectReparseArgv({ parent: browser, action: tabs, argv, expected: argv });
276+
});
171277
});

src/cli/program/action-reparse.ts

Lines changed: 155 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
// Reparse support for lazy commands after their placeholder has been replaced.
2-
import type { Command } from "commander";
2+
import type { Command, Option } from "commander";
33
import { buildParseArgv } from "../argv.js";
44
import { resolveActionArgs, resolveCommandOptionArgs } from "./helpers.js";
55

6-
function getCommandPathFromRoot(command: Command | undefined): string[] {
7-
const path: string[] = [];
6+
function getCommandPathFromRoot(command: Command | undefined): Command[] {
7+
const path: Command[] = [];
88
let current = command;
99
while (current?.parent) {
10-
const name = current.name();
11-
if (name) {
12-
path.unshift(name);
10+
if (current.name()) {
11+
path.unshift(current);
1312
}
1413
current = current.parent;
1514
}
@@ -20,7 +19,7 @@ function buildFallbackArgv(program: Command, actionCommand: Command | undefined)
2019
const actionArgsList = resolveActionArgs(actionCommand);
2120
const parentOptionArgs =
2221
actionCommand?.parent === program ? resolveCommandOptionArgs(program) : [];
23-
const commandPath = getCommandPathFromRoot(actionCommand);
22+
const commandPath = getCommandPathFromRoot(actionCommand).map((command) => command.name());
2423
if (commandPath.length === 0) {
2524
return [...parentOptionArgs, ...actionArgsList];
2625
}
@@ -40,6 +39,151 @@ function findRootCommand(cmd: Command): Command {
4039
return current;
4140
}
4241

42+
function findOption(command: Command, token: string): Option | undefined {
43+
const equalsIndex = token.indexOf("=");
44+
const flag = equalsIndex === -1 ? token : token.slice(0, equalsIndex);
45+
return command.options.find(
46+
(candidate) =>
47+
(candidate.short === flag || candidate.long === flag) &&
48+
(equalsIndex === -1 || candidate.required || candidate.optional),
49+
);
50+
}
51+
52+
function findNearestOption(commands: readonly Command[], token: string): Option | undefined {
53+
for (let index = commands.length - 1; index >= 0; index -= 1) {
54+
const command = commands[index];
55+
const option = command ? findOption(command, token) : undefined;
56+
if (option) {
57+
return option;
58+
}
59+
}
60+
return undefined;
61+
}
62+
63+
function matchesCommandName(command: Command, token: string): boolean {
64+
return command.name() === token || command.aliases().includes(token);
65+
}
66+
67+
// Returns 0 for a missing required value, otherwise the number of consumed tokens.
68+
function optionTokenCount(option: Option, argv: readonly string[], index: number): number {
69+
const token = argv[index] ?? "";
70+
if (token.includes("=") || (!option.required && !option.optional)) {
71+
return 1;
72+
}
73+
const next = argv[index + 1];
74+
if (option.required) {
75+
return next === undefined ? 0 : 2;
76+
}
77+
const optionalValue = next && (!next.startsWith("-") || /^-\d/.test(next));
78+
return optionalValue ? 2 : 1;
79+
}
80+
81+
function findCommandPathEnd(argv: readonly string[], command: Command): number {
82+
const path = getCommandPathFromRoot(command);
83+
const root = path[0]?.parent;
84+
if (!root) {
85+
return -1;
86+
}
87+
const selectedCommands = [root];
88+
let pathIndex = 0;
89+
for (let index = 2; index < argv.length; index += 1) {
90+
const token = argv[index] ?? "";
91+
const option = findNearestOption(selectedCommands, token);
92+
if (option) {
93+
const count = optionTokenCount(option, argv, index);
94+
if (count === 0) {
95+
return -1;
96+
}
97+
index += count - 1;
98+
continue;
99+
}
100+
const nextCommand = path[pathIndex];
101+
if (!nextCommand || !matchesCommandName(nextCommand, token)) {
102+
return -1;
103+
}
104+
selectedCommands.push(nextCommand);
105+
pathIndex += 1;
106+
if (pathIndex === path.length) {
107+
return index + 1;
108+
}
109+
}
110+
return -1;
111+
}
112+
113+
/** Restore parent-option placement without stealing options owned by the loaded child command. */
114+
function hoistLazyParentOptions(
115+
argv: string[],
116+
parentCommand: Command,
117+
lazyCommandName: string,
118+
): string[] {
119+
let lazyCommandIndex = findCommandPathEnd(argv, parentCommand);
120+
if (lazyCommandIndex === -1) {
121+
return argv;
122+
}
123+
while (lazyCommandIndex < argv.length) {
124+
const option = findOption(parentCommand, argv[lazyCommandIndex] ?? "");
125+
if (!option) {
126+
break;
127+
}
128+
const count = optionTokenCount(option, argv, lazyCommandIndex);
129+
if (count === 0) {
130+
return argv;
131+
}
132+
lazyCommandIndex += count;
133+
}
134+
if (argv[lazyCommandIndex] !== lazyCommandName) {
135+
return argv;
136+
}
137+
138+
const lazyCommand = parentCommand.commands.find((command) =>
139+
matchesCommandName(command, lazyCommandName),
140+
);
141+
if (!lazyCommand) {
142+
return argv;
143+
}
144+
let selectedCommand = lazyCommand;
145+
const selectedCommands = [selectedCommand];
146+
147+
const hoisted: string[] = [];
148+
const remaining: string[] = [];
149+
let acceptsSubcommands = true;
150+
for (let index = lazyCommandIndex + 1; index < argv.length; index += 1) {
151+
const token = argv[index] ?? "";
152+
if (token === "--") {
153+
remaining.push(...argv.slice(index));
154+
break;
155+
}
156+
const childOption = findNearestOption(selectedCommands, token);
157+
const parentOption = findOption(parentCommand, token);
158+
const option = childOption ?? parentOption;
159+
if (option) {
160+
const count = optionTokenCount(option, argv, index);
161+
if (count === 0) {
162+
return argv;
163+
}
164+
const tokens = argv.slice(index, index + count);
165+
(childOption ? remaining : hoisted).push(...tokens);
166+
index += count - 1;
167+
continue;
168+
}
169+
if (acceptsSubcommands && !token.startsWith("-")) {
170+
const nextCommand: Command | undefined = selectedCommand.commands.find((command) =>
171+
matchesCommandName(command, token),
172+
);
173+
if (nextCommand) {
174+
selectedCommand = nextCommand;
175+
selectedCommands.push(nextCommand);
176+
} else {
177+
acceptsSubcommands = false;
178+
}
179+
}
180+
remaining.push(token);
181+
}
182+
return hoisted.length === 0
183+
? argv
184+
: [...argv.slice(0, lazyCommandIndex), ...hoisted, lazyCommandName, ...remaining];
185+
}
186+
43187
/** Rebuild argv from Commander action args and re-run parsing after lazy registration. */
44188
export async function reparseProgramFromActionArgs(
45189
program: Command,
@@ -57,5 +201,8 @@ export async function reparseProgramFromActionArgs(
57201
rawArgs,
58202
fallbackArgv,
59203
});
60-
await rootProgram.parseAsync(parseArgv);
204+
const normalizedArgv = actionCommand
205+
? hoistLazyParentOptions(parseArgv, program, actionCommand.name())
206+
: parseArgv;
207+
await rootProgram.parseAsync(normalizedArgv);
61208
}

0 commit comments

Comments
 (0)