fix(cli): resolve --url option collision in browser cookies set

When addGatewayClientOptions registers --url on the parent browser
command, Commander.js captures it before the cookies set subcommand
can receive it. Switch from requiredOption to option and resolve
via inheritOptionFromParent, matching the existing pattern used
for --target-id.

Fixes #24811

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
HCL
2026-02-24 12:04:06 +08:00
parent ebde897bb8
commit 96fcb963ec
2 changed files with 47 additions and 3 deletions

View File

@@ -4,6 +4,17 @@ import { defaultRuntime } from "../runtime.js";
import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js"; import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js";
import { inheritOptionFromParent } from "./command-options.js"; import { inheritOptionFromParent } from "./command-options.js";
function resolveUrl(opts: { url?: string }, command: Command): string | undefined {
if (typeof opts.url === "string" && opts.url.trim()) {
return opts.url.trim();
}
const inherited = inheritOptionFromParent<string>(command, "url");
if (typeof inherited === "string" && inherited.trim()) {
return inherited.trim();
}
return undefined;
}
function resolveTargetId(rawTargetId: unknown, command: Command): string | undefined { function resolveTargetId(rawTargetId: unknown, command: Command): string | undefined {
const local = typeof rawTargetId === "string" ? rawTargetId.trim() : ""; const local = typeof rawTargetId === "string" ? rawTargetId.trim() : "";
if (local) { if (local) {
@@ -58,12 +69,18 @@ export function registerBrowserCookiesAndStorageCommands(
.description("Set a cookie (requires --url or domain+path)") .description("Set a cookie (requires --url or domain+path)")
.argument("<name>", "Cookie name") .argument("<name>", "Cookie name")
.argument("<value>", "Cookie value") .argument("<value>", "Cookie value")
.requiredOption("--url <url>", "Cookie URL scope (recommended)") .option("--url <url>", "Cookie URL scope (recommended)")
.option("--target-id <id>", "CDP target id (or unique prefix)") .option("--target-id <id>", "CDP target id (or unique prefix)")
.action(async (name: string, value: string, opts, cmd) => { .action(async (name: string, value: string, opts, cmd) => {
const parent = parentOpts(cmd); const parent = parentOpts(cmd);
const profile = parent?.browserProfile; const profile = parent?.browserProfile;
const targetId = resolveTargetId(opts.targetId, cmd); const targetId = resolveTargetId(opts.targetId, cmd);
const url = resolveUrl(opts, cmd);
if (!url) {
defaultRuntime.error(danger("Missing required --url option for cookies set"));
defaultRuntime.exit(1);
return;
}
try { try {
const result = await callBrowserRequest( const result = await callBrowserRequest(
parent, parent,
@@ -73,7 +90,7 @@ export function registerBrowserCookiesAndStorageCommands(
query: profile ? { profile } : undefined, query: profile ? { profile } : undefined,
body: { body: {
targetId, targetId,
cookie: { name, value, url: opts.url }, cookie: { name, value, url },
}, },
}, },
{ timeoutMs: 20000 }, { timeoutMs: 20000 },

View File

@@ -26,12 +26,15 @@ vi.mock("../runtime.js", () => ({
})); }));
describe("browser state option collisions", () => { describe("browser state option collisions", () => {
const createBrowserProgram = () => { const createBrowserProgram = ({ withGatewayUrl = false } = {}) => {
const program = new Command(); const program = new Command();
const browser = program const browser = program
.command("browser") .command("browser")
.option("--browser-profile <name>", "Browser profile") .option("--browser-profile <name>", "Browser profile")
.option("--json", "Output JSON", false); .option("--json", "Output JSON", false);
if (withGatewayUrl) {
browser.option("--url <url>", "Gateway WebSocket URL");
}
const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts;
registerBrowserStateCommands(browser, parentOpts); registerBrowserStateCommands(browser, parentOpts);
return program; return program;
@@ -79,6 +82,30 @@ describe("browser state option collisions", () => {
expect((request as { body?: { targetId?: string } }).body?.targetId).toBe("tab-1"); expect((request as { body?: { targetId?: string } }).body?.targetId).toBe("tab-1");
}); });
it("resolves --url via parent when addGatewayClientOptions captures it", async () => {
const program = createBrowserProgram({ withGatewayUrl: true });
await program.parseAsync(
["browser", "--url", "ws://gw", "cookies", "set", "session", "abc", "--url", "https://example.com"],
{ from: "user" },
);
const call = mocks.callBrowserRequest.mock.calls.at(-1);
expect(call).toBeDefined();
const request = call![1] as { body?: { cookie?: { url?: string } } };
expect(request.body?.cookie?.url).toBe("https://example.com");
});
it("inherits --url from parent when subcommand does not provide it", async () => {
const program = createBrowserProgram({ withGatewayUrl: true });
await program.parseAsync(
["browser", "--url", "https://inherited.example.com", "cookies", "set", "session", "abc"],
{ from: "user" },
);
const call = mocks.callBrowserRequest.mock.calls.at(-1);
expect(call).toBeDefined();
const request = call![1] as { body?: { cookie?: { url?: string } } };
expect(request.body?.cookie?.url).toBe("https://inherited.example.com");
});
it("accepts legacy parent `--json` by parsing payload via positional headers fallback", async () => { it("accepts legacy parent `--json` by parsing payload via positional headers fallback", async () => {
const request = (await runBrowserCommandAndGetRequest([ const request = (await runBrowserCommandAndGetRequest([
"set", "set",