Fail on ambiguous Firefox targeting

docs/commands.md

@@ -4,7 +4,7 @@ All commands use the paired Firefox session. Add `--json` for structured output
 
 ## Targets
 
-Commands that operate on a page accept:
+Commands advertise their supported selectors in `-h`. Selector values are:
 
 ```sh
 --tab active
@@ -17,6 +17,8 @@ Commands that operate on a page accept:
 
 Bare numeric targets are indexes printed by `firefox-cli tab` and `firefox-cli window`; `id:` targets use Firefox tab or window IDs.
 
+Page-targeted commands accept both `--window` and `--tab`. `tab new`, `window select`, and `window close` accept `--window` only. Targetless commands accept neither. Unsupported selector flags fail before a request is sent.
+
 Private windows are listed and readable. Mutating commands against private windows return `UNSUPPORTED_CAPABILITY`.
 
 ## Setup And Diagnostics
@@ -35,13 +37,13 @@ Private windows are listed and readable. Mutating commands against private windo
 | Command | Behavior |
 | --- | --- |
 | `firefox-cli tab [--json]` | List tabs. |
-| `firefox-cli tab new [url] [--json]` | Open a new tab. |
+| `firefox-cli tab new [url] [--window target] [--json]` | Open a new tab in the selected window. |
 | `firefox-cli tab select [target] [--json]` | Select a tab. |
 | `firefox-cli tab close [target] [--json]` | Close a tab. |
 | `firefox-cli window [--json]` | List windows. |
 | `firefox-cli window new [url] [--json]` | Open a new window. |
-| `firefox-cli window select [target] [--json]` | Select a window. |
-| `firefox-cli window close [target] [--json]` | Close a window. |
+| `firefox-cli window select [target] [--window target] [--json]` | Focus a window; this does not set a durable default target. |
+| `firefox-cli window close [target] [--window target] [--json]` | Close a selected window. |
 | `firefox-cli open [--new-tab] <url> [--json]` | Navigate the target tab or open a URL in a new tab. |
 | `firefox-cli back|forward|reload [--json]` | Run browser navigation in the target tab. |
 
@@ -131,7 +133,7 @@ firefox-cli screenshot [path] [--format png|jpeg] [--screenshot-quality 1-100] [
 firefox-cli batch <json> | --stdin [--bail] [--timeout ms] [--max-output bytes] [--json]
 ```
 
-`eval` runs in the page main world and returns JSON-serializable values or `undefined`. Screenshot output captures the visible tab as PNG or JPEG; `--full` returns `UNSUPPORTED_CAPABILITY`.
+`eval` runs in the page main world and returns JSON-serializable values or `undefined`. Screenshot output captures the visible tab as PNG or JPEG and may activate the selected tab/window; `--full` returns `UNSUPPORTED_CAPABILITY`.
 
 `batch` accepts an array of protocol command objects or CLI argv arrays. Steps run serially. With `--bail`, execution stops after the first failed step; without it, later steps continue and the batch result reports failures.
 

docs/firefox-cli-spec.md

@@ -217,6 +217,9 @@ Target resolution is owned by the extension.
 - Private windows are reported but commands return `UNSUPPORTED_CAPABILITY` unless the extension has private browsing permission and the command is explicitly allowed there.
 - Container tab metadata should be included when Firefox exposes it, but no persistent container/session model is added.
 - Each command snapshots its resolved target before execution to avoid active-tab races.
+- Selector values are `active`, a non-negative listing index, or `id:<non-negative Firefox ID>`. A route supports neither selector, `--window` only, `--tab` only, or both as advertised by its CLI help; unsupported selector flags fail before dispatch.
+- Page-targeted routes support both dimensions. `tab new`, `window select`, and `window close` support `--window` only. Targetless routes support neither.
+- `window select` changes Firefox focus only. It does not establish a durable CLI target; use an explicit selector on each isolated follow-up command.
 
 `open <url>` navigates the resolved active tab to match `agent-browser`. Use `tab new [url]` or `open --new-tab <url>` to create a new tab.
 
@@ -278,7 +281,7 @@ Generated DOM events are not trusted user input. Sites that check `event.isTrust
 Start with visible-tab screenshots.
 
 - `screenshot [path]` captures the active visible tab of the target window.
-- If a non-active target is requested, the command may activate the target tab/window before capture and must report that side effect in diagnostics.
+- If the requested target is not active or focused, the command activates its tab and/or focuses its window before capture and reports those side effects in diagnostics. Invalid target resolution happens before activation.
 - If activation is impossible or would require unsupported user activation, return `UNSUPPORTED_CAPABILITY`.
 - Write image bytes through the native host to the requested path; JSON output should include path, format, dimensions when known, and diagnostics.
 - Support visible-tab PNG and JPEG captures with JPEG quality.
@@ -329,7 +332,7 @@ Global options:
 - `--json`: emit machine-readable JSON.
 - `--timeout <ms>`: override command timeout.
 - `--max-output <chars>`: cap text output.
-- `--window <id|index|active>` and `--tab <id|index|active>`: choose a target without hidden sessions.
+- `--window <target>` and `--tab <target>`: route-specific target selectors; use only flags advertised by that route's help.
 - `--debug`: include transport/protocol diagnostics.
 
 Setup and diagnostics:
@@ -460,7 +463,7 @@ Tabs and windows:
 - `window`: lists normal windows and their active tabs. Result includes Firefox window IDs, focus/active flags, bounds when available, and tab count.
 - `window new [url]`: creates a new window. Result is created window ID and active tab ID.
 - `window close <id|active>`: closes a window. Result is closed window ID.
-- `window select <id|index>`: focuses a window. Result is selected window ID and active tab.
+- `window select <id|index>`: focuses a window. Result includes refreshed focus metadata and its active tab; it does not set a durable default target.
 
 Screenshots:
 

packages/cli/src/argv-contracts.ts

@@ -1,7 +1,9 @@
+import { getCliRouteEntries, type CliRouteSelectorDimensions } from "@firefox-cli/protocol";
 import { CliUsageError, type CliRouteParserSpec } from "./types.js";
 
 const targetValueOptions = ["--window", "--tab"] as const;
 const jsonFlags = ["--json"] as const;
+const selectorDimensionsByRouteId = new Map(getCliRouteEntries().map(({ route }) => [route.id, route.selectorDimensions]));
 
 export const routeParserSpecs = {
   capabilities: parser("capabilities"),
@@ -118,6 +120,7 @@ export const routeParserSpecs = {
 
 export type CliRouteParserSpecById = typeof routeParserSpecs;
 export type CliRouteParserRouteId = keyof CliRouteParserSpecById;
+const routeParserSpecsById = new Map<string, CliRouteParserSpec>(Object.entries(routeParserSpecs));
 
 export interface ParsedCliRouteArgs {
   readonly positionals: readonly string[];
@@ -140,12 +143,16 @@ interface CliRouteArgContext {
   readonly state: CliRouteArgParserState;
 }
 
-export function parseCliRouteArgsForRoute(routeId: CliRouteParserRouteId, args: readonly string[]): ParsedCliRouteArgs {
-  return parseCliRouteArgs(routeParserSpecs[routeId], routeId, args);
+export function parseCliRouteArgsForRoute(routeId: string, args: readonly string[]): ParsedCliRouteArgs {
+  const parserSpec = routeParserSpecsById.get(routeId);
+  if (parserSpec === undefined) {
+    throw new Error(`CLI parser references unknown route: ${routeId}`);
+  }
+  return parseCliRouteArgs(withRouteSelectorOptions(parserSpec, routeId), routeId, args);
 }
 
 export function parseCliRouteArgv(parserSpec: CliRouteParserSpec, routeId: string, argv: readonly string[]): ParsedCliRouteArgs {
-  return parseCliRouteArgs(parserSpec, routeId, argv.slice(1));
+  return parseCliRouteArgs(withRouteSelectorOptions(parserSpec, routeId), routeId, argv.slice(1));
 }
 
 function parseCliRouteArgs(parserSpec: CliRouteParserSpec, routeId: string, args: readonly string[]): ParsedCliRouteArgs {
@@ -187,20 +194,37 @@ function parseCliRouteArgs(parserSpec: CliRouteParserSpec, routeId: string, args
       continue;
     }
 
-    if (arg.startsWith("-")) {
-      if (canTreatUnknownOptionAsPayload(parserSpec, state.positionals.length)) {
-        state.positionals.push(arg);
-        continue;
-      }
+    if (isTargetValueOption(arg)) {
       throw new CliUsageError(`Unsupported ${parserSpec.label} option: ${arg}`);
     }
 
+    if (arg.startsWith("-")) {
+      handleUnknownOption(context, arg);
+      continue;
+    }
+
     state.positionals.push(arg);
   }
 
   return { positionals: state.positionals, optionArgs: state.optionArgs, json: state.json };
 }
 
+function handleUnknownOption(context: CliRouteArgContext, arg: string): void {
+  const { parserSpec, state } = context;
+  if (isTargetValueOption(arg) || !canTreatUnknownOptionAsPayload(parserSpec, state.positionals.length)) {
+    throw new CliUsageError(`Unsupported ${parserSpec.label} option: ${arg}`);
+  }
+  state.positionals.push(arg);
+}
+
+export function rejectTargetSelectorOptions(args: readonly string[], label: string): void {
+  for (const arg of args) {
+    if (isTargetValueOption(arg)) {
+      throw new CliUsageError(`Unsupported ${label} option: ${arg}`);
+    }
+  }
+}
+
 function handleKnownFlag(context: CliRouteArgContext, arg: string, index: number): void {
   const { parserSpec, args, state } = context;
   if (shouldTreatKnownOptionAsPayload({ parserSpec, args, index, width: 1, currentPositionals: state.positionals.length })) {
@@ -261,13 +285,42 @@ function parser(
   return {
     label,
     flags: [...jsonFlags, ...(options.flags ?? [])],
-    valueOptions: [...targetValueOptions, ...(options.valueOptions ?? [])],
+    valueOptions: [...(options.valueOptions ?? [])],
     ...(options.optionalValueOptions === undefined ? {} : { optionalValueOptions: options.optionalValueOptions }),
     ...(options.payload === undefined ? {} : { payload: options.payload }),
     ...(options.allowDashDashPayload === undefined ? {} : { allowDashDashPayload: options.allowDashDashPayload }),
   };
 }
 
+function withRouteSelectorOptions(parserSpec: CliRouteParserSpec, routeId: string): CliRouteParserSpec {
+  const selectorDimensions = selectorDimensionsByRouteId.get(routeId);
+  if (selectorDimensions === undefined) {
+    throw new Error(`CLI parser references unknown protocol route: ${routeId}`);
+  }
+
+  return {
+    ...parserSpec,
+    valueOptions: [...selectorValueOptions(selectorDimensions), ...parserSpec.valueOptions],
+  };
+}
+
+function selectorValueOptions(selectorDimensions: CliRouteSelectorDimensions): readonly string[] {
+  if (selectorDimensions === "neither") {
+    return [];
+  }
+  if (selectorDimensions === "window") {
+    return [targetValueOptions[0]];
+  }
+  if (selectorDimensions === "tab") {
+    return [targetValueOptions[1]];
+  }
+  return targetValueOptions;
+}
+
+function isTargetValueOption(arg: string): arg is (typeof targetValueOptions)[number] {
+  return arg === targetValueOptions[0] || arg === targetValueOptions[1];
+}
+
 function shouldTreatKnownOptionAsPayload(context: {
   readonly parserSpec: CliRouteParserSpec;
   readonly args: readonly string[];
@@ -299,7 +352,7 @@ function buildOptionInventory(specs: Readonly<Record<string, CliRouteParserSpec>
   readonly optionalValueOptions: ReadonlySet<string>;
 } {
   const flags = new Set<string>();
-  const valueOptions = new Set<string>();
+  const valueOptions = new Set<string>(targetValueOptions);
   const optionalValueOptions = new Set<string>();
   for (const spec of Object.values(specs)) {
     for (const flag of spec.flags) {

packages/cli/src/cli-tabs-targets.test.ts

@@ -5,6 +5,7 @@ import { createErrorResponse, createOkResponse } from "@firefox-cli/protocol";
 import { describe, expect, it } from "vitest";
 import { runCli } from "./index.js";
 import { actionElement, baseDependencies, targetSummary } from "./cli-test-support.js";
+import { findCliRouteBindingForArgv } from "./route-registry.js";
 
 describe("runCli tabs and targets", () => {
   it("lists Firefox tabs as JSON", async () => {
@@ -210,6 +211,10 @@ describe("runCli tabs and targets", () => {
 
     for (const flag of flags) {
       for (const testCase of cases) {
+        const binding = findCliRouteBindingForArgv(testCase.argv);
+        if (binding === undefined || !supportsSelectorFlag(binding.route.selectorDimensions, flag)) {
+          continue;
+        }
         await expect(
           runCli([...testCase.argv, flag], {
             ...baseDependencies(),
@@ -338,3 +343,10 @@ describe("runCli tabs and targets", () => {
     });
   });
 });
+
+function supportsSelectorFlag(selectorDimensions: "neither" | "window" | "tab" | "both", flag: "--tab" | "--window"): boolean {
+  return (
+    (flag === "--tab" && (selectorDimensions === "tab" || selectorDimensions === "both")) ||
+    (flag === "--window" && (selectorDimensions === "window" || selectorDimensions === "both"))
+  );
+}

packages/cli/src/cli-target-contract.test.ts

@@ -0,0 +1,145 @@
+import { createErrorResponse, getCliRouteEntries, type RequestEnvelope } from "@firefox-cli/protocol";
+import { describe, expect, it } from "vitest";
+import { parseCliRouteArgsForRoute } from "./argv-contracts.js";
+import { runCli } from "./index.js";
+import { baseDependencies } from "./cli-test-support.js";
+
+describe("CLI route target contracts", () => {
+  it("accepts selector dimensions declared by each protocol route", () => {
+    for (const { route } of getCliRouteEntries()) {
+      for (const [flag, supported] of [
+        ["--window", route.selectorDimensions === "window" || route.selectorDimensions === "both"],
+        ["--tab", route.selectorDimensions === "tab" || route.selectorDimensions === "both"],
+      ] as const) {
+        if (supported) {
+          expect(parseCliRouteArgsForRoute(route.id, [flag, "id:7"]).optionArgs, `${route.id} ${flag}`).toEqual([flag, "id:7"]);
+        } else {
+          expect(() => parseCliRouteArgsForRoute(route.id, [flag, "id:7"]), `${route.id} ${flag}`).toThrow(`Unsupported`);
+        }
+      }
+    }
+  });
+
+  it.each([
+    ["tab new", ["tab", "new", "--tab", "id:42"]],
+    ["window select", ["window", "select", "--tab", "id:42"]],
+    ["window close", ["window", "close", "--tab", "id:42"]],
+    ["targetless capabilities window", ["capabilities", "--window", "id:7"]],
+    ["targetless capabilities tab", ["capabilities", "--tab", "id:42"]],
+  ] as const)("rejects unsupported selectors before transport for %s", async (_name, argv) => {
+    let requestCalls = 0;
+
+    const output = await runCli(argv, {
+      ...baseDependencies(),
+      sendRequest: async () => {
+        requestCalls += 1;
+        throw new Error(`Unexpected request for ${argv.join(" ")}`);
+      },
+    });
+
+    expect(output.exitCode).toBe(1);
+    expect(output.stdout).toBe("");
+    expect(output.stderr).toContain("Unsupported");
+    expect(requestCalls).toBe(0);
+  });
+
+  it.each(["--window", "--tab"] as const)("rejects unsupported doctor selector %s before diagnostics", async (flag) => {
+    const output = await runCli(["doctor", flag, "id:9"], baseDependencies());
+
+    expect(output).toEqual({
+      exitCode: 1,
+      stdout: "",
+      stderr: `Unsupported doctor option: ${flag}\n`,
+    });
+  });
+
+  it("preserves unsupported doctor selector diagnostics in batch argv", async () => {
+    const output = await runCli(["batch", JSON.stringify([["doctor", "--window", "id:9"]])], baseDependencies());
+
+    expect(output).toEqual({
+      exitCode: 1,
+      stdout: "",
+      stderr: "Invalid batch argv step 0: Unsupported doctor option: --window\n",
+    });
+  });
+
+  it("serializes supported selectors before and after ordinary and payload positionals", async () => {
+    const cases: readonly {
+      readonly name: string;
+      readonly argv: readonly string[];
+      readonly routeLength: number;
+      readonly dimensions: readonly ("window" | "tab")[];
+    }[] = [
+      { name: "page navigation", argv: ["open", "https://example.com/"], routeLength: 1, dimensions: ["window", "tab"] },
+      { name: "window-only tab creation", argv: ["tab", "new", "https://example.com/"], routeLength: 2, dimensions: ["window"] },
+      { name: "payload-bearing interaction", argv: ["fill", "#name", "Nikita"], routeLength: 1, dimensions: ["window", "tab"] },
+      { name: "batch request", argv: ["batch", JSON.stringify([["snapshot", "--tab", "id:42"]])], routeLength: 1, dimensions: ["window", "tab"] },
+    ];
+
+    for (const testCase of cases) {
+      for (const dimension of testCase.dimensions) {
+        const flag = `--${dimension}`;
+        for (const placement of ["before", "after"] as const) {
+          const argv =
+            placement === "before"
+              ? [...testCase.argv.slice(0, testCase.routeLength), flag, "id:7", ...testCase.argv.slice(testCase.routeLength)]
+              : [...testCase.argv, flag, "id:7"];
+          let request: RequestEnvelope | undefined;
+
+          await runCli(argv, {
+            ...baseDependencies(),
+            sendRequest: async (sent) => {
+              request = sent;
+              return createErrorResponse(sent.id, {
+                code: "NATIVE_HOST_UNAVAILABLE",
+                message: "Expected test transport failure.",
+              });
+            },
+          });
+
+          expect(request, `${testCase.name} ${dimension} ${placement}`).toMatchObject({
+            params: {
+              target: {
+                [dimension]: { kind: "id", id: 7 },
+              },
+            },
+          });
+        }
+      }
+    }
+  });
+
+  it("rejects unsupported batch step selectors before transport", async () => {
+    let requestCalls = 0;
+    const output = await runCli(["batch", JSON.stringify([["window", "close", "--tab", "id:42"]])], {
+      ...baseDependencies(),
+      sendRequest: async () => {
+        requestCalls += 1;
+        throw new Error("Unexpected batch request.");
+      },
+    });
+
+    expect(output.exitCode).toBe(1);
+    expect(output.stdout).toBe("");
+    expect(output.stderr).toContain("Unsupported");
+    expect(requestCalls).toBe(0);
+  });
+
+  it.each([
+    ["notify", ["notify", "title", "--window", "id:7"]],
+    ["notify batch step", ["batch", JSON.stringify([["notify", "title", "--tab", "id:7"]])]],
+  ] as const)("rejects target selectors that would otherwise look like payload for %s", async (_name, argv) => {
+    let requestCalls = 0;
+    const output = await runCli(argv, {
+      ...baseDependencies(),
+      sendRequest: async () => {
+        requestCalls += 1;
+        throw new Error("Unexpected request.");
+      },
+    });
+
+    expect(output.exitCode).toBe(1);
+    expect(output.stderr).toContain("Unsupported");
+    expect(requestCalls).toBe(0);
+  });
+});