From 98e7bd000faaf5b6ba46c65dc46cce41008c3703 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:41:20 +0200 Subject: [PATCH 1/5] Harden Firefox selector contracts --- .kent/plans/FFCLI-1.md | 77 ++++++++++ docs/commands.md | 12 +- docs/firefox-cli-spec.md | 9 +- packages/cli/src/argv-contracts.ts | 73 ++++++++-- packages/cli/src/cli-tabs-targets.test.ts | 12 ++ packages/cli/src/cli-target-contract.test.ts | 135 ++++++++++++++++++ packages/cli/src/help.ts | 6 +- packages/cli/src/route-registry.test.ts | 10 ++ packages/cli/src/route-registry.ts | 12 +- packages/cli/src/runner.ts | 4 + .../src/browser-commands-targets.test.ts | 81 ++++++++++- packages/protocol/src/browser/output.ts | 1 - packages/protocol/src/metadata.ts | 3 + .../src/protocol-metadata-behavior.test.ts | 27 +++- .../protocol/src/protocol-request.test.ts | 93 ++++++++++++ .../protocol/src/protocol-test-support.ts | 28 +++- packages/protocol/src/registry/actions.ts | 30 ++-- packages/protocol/src/registry/browsing.ts | 28 ++-- packages/protocol/src/registry/content.ts | 10 +- packages/protocol/src/registry/core.ts | 2 +- packages/protocol/src/registry/define.ts | 37 ++++- packages/protocol/src/registry/index.ts | 2 +- packages/protocol/src/registry/pairing.ts | 2 +- packages/protocol/src/registry/phase8.ts | 46 +++--- .../protocol/src/registry/registry.test.ts | 33 +++++ packages/protocol/src/target.ts | 33 ++--- skills/firefox-cli/SKILL.md | 12 +- 27 files changed, 711 insertions(+), 107 deletions(-) create mode 100644 .kent/plans/FFCLI-1.md create mode 100644 packages/cli/src/cli-target-contract.test.ts diff --git a/.kent/plans/FFCLI-1.md b/.kent/plans/FFCLI-1.md new file mode 100644 index 0000000..1b16817 --- /dev/null +++ b/.kent/plans/FFCLI-1.md @@ -0,0 +1,77 @@ +## Definition + +The checked-in source already recognizes `--window` and `--tab` in every route contract, builders produce `TargetSelector` values, and extension resolution selects an explicit window before that window's active tab. An explicit tab ID without a window resolves globally; a tab selector combined with a window is constrained to that window. The reported cross-window behavior is therefore not reproduced by the existing unit coverage. The missing proof is an end-to-end-shaped two-window regression suite across CLI parsing, protocol request serialization, and extension execution. Confirmed defects remain at the CLI/protocol boundary: targetless routes accept and discard selectors, while asymmetric commands such as `tab.new`, `window.select`, and `window.close` accept target dimensions their handlers never consume. + +Relevant ownership: + +- `packages/cli/src/argv-contracts.ts` is the single option grammar for all routes. `parseCliRouteArgv` validates flags before sending; `packages/cli/src/parse.ts` turns `--window` / `--tab` values into protocol selectors. +- CLI builders in `packages/cli/src/commands/` attach parsed selectors to typed requests. `route-registry.ts` owns route help usage strings. +- `packages/protocol/src/target.ts` owns the selector and resolved-target schemas; registry target-policy metadata currently describes requirement/default semantics but is too coarse to describe which selector dimensions a route consumes. +- `packages/extension/src/browser-command/targets.ts` resolves selectors from one request-scoped window snapshot. Browser handlers route navigation, content reads/actions/logs, and screenshots through the resolved tab/window. +- `window.select` is a Firefox focus operation, not persistent CLI target state. The next command still resolves Firefox's active tab/window at its own resolution time; an explicit selector is the reliable isolation mechanism. Screenshot explicitly activates/focuses its requested target because Firefox visible-tab capture requires it. +- `docs/firefox-cli-spec.md` is the authoritative product specification and currently describes `--window` / `--tab` as global options without route-specific limits. `docs/commands.md` lists the value syntax, route-specific CLI help mostly omits it, and `skills/firefox-cli/SKILL.md` directs users to discover options without giving the safe separate-window invocation. + +Scope and constraints: + +- Treat `active`, ``, and `id:` as the value grammar for each selector dimension a route explicitly supports. Selector dimensions are route/command-specific: page-targeting commands generally support both window and tab; `tab.new` supports only window; `window.select` and `window.close` support only window; targetless routes support neither. Unsupported dimensions must be rejected before dispatch rather than serialized and ignored. +- Selector placement before or after command positionals must parse identically, including payload-bearing commands and batch steps. +- Invalid selector syntax must fail before a request is sent. A syntactically valid selector that does not resolve must return `INVALID_TARGET` before the command's navigation, content, capture, or interaction operation begins. +- Do not add a remembered CLI target/session state. It would contradict request-scoped target ownership and introduce a second source of truth. Document that `window select` changes Firefox focus only; use an explicit selector for follow-up isolation. +- Screenshot has an intentional focus/activation side effect for its explicitly selected target; this must be documented, not masked. Invalid selectors must not cause that activation. + +Completion criteria: + +- A two-window extension-level regression fixture proves `open`, a read/snapshot/log command, an interaction, and `screenshot` route to the requested window's active tab (or explicitly requested tab) and leave the other window's tab untouched; screenshot capture receives the requested window ID. +- CLI regression coverage proves each affected route family serializes every supported selector dimension regardless of option placement, while unsupported dimensions (including `tab new --tab`, `window select --tab`, `window close --tab`, and selectors on targetless commands) fail before transport dispatch. +- Two-window invalid-selector coverage proves no navigation, content request, tab/window selection, or focus occurs when target resolution fails. +- `window select` returns the selected window's refreshed focus metadata, and help/docs describe it as a focus command rather than a durable default-target selection. +- The authoritative Firefox CLI spec, route help, command reference, and Firefox CLI skill share the supported selector grammar and show the explicit-window QA flow; their claims match protocol route selector-dimension metadata. +- Focused CLI/extension tests, root typecheck, and the root quality gate pass. + +Decision: this can be addressed directly without user product/design input. The existing protocol ownership model fixes the selector semantics; the implementation must add explicit selector-dimension contracts, close coverage/documentation gaps, and reject dimensions that a route/command does not consume. + +## Design + +## Architecture + +### Selector-contract ownership and CLI validation + +- Extend protocol CLI route metadata with the authoritative set of supported selector dimensions: neither, window only, tab only, or both. Keep `CommandTargetPolicy` for target requirement/default semantics; do not infer selector dimensions from its coarse `none` / `optional` / `required` / `mixed` values. +- Populate selector dimensions beside each registry route declaration, including the asymmetric browsing routes: `tab.new` accepts window only; `window.select` and `window.close` accept window only; normal page/tab resolution routes accept both where their handlers consume a full target; targetless routes accept neither. Expose this typed route metadata through the existing `getCliRouteEntries` path so CLI parsing and help consume the same contract. +- Narrow protocol parameter schemas for asymmetric commands using reusable window-only and full target selector schemas. This makes `tab.new` with a tab selector and window operations with a tab selector invalid at every protocol boundary, including raw batch requests or version-skewed clients, rather than relying only on CLI filtering. Keep full `TargetSelector` for commands that resolve both dimensions. +- Keep `routeParserSpecs` responsible for tokenization, payload boundaries, and selector value syntax, but add only the selector options declared by the bound route metadata. Unsupported dimensions therefore produce a usage error during route validation, before request construction and `sendRequest`, while supported options retain placement on either side of positionals and payload disambiguation. +- Continue using `parseTargetOptions` for supported selectors and typed builders for request serialization. Add protocol registry invariants proving route dimensions are compatible with the command parameter schema and CLI invariants proving parser/help option exposure is derived from route metadata. This closes both silent-discard paths: selectors cannot enter unsupported request shapes, and accepted selectors cannot be omitted from help. + +### Request-scoped browser target resolution + +- Preserve `BrowserTargetContext` as the request-scoped owner of the ordered Firefox window snapshot and `targets.ts` as the only selector resolver. No persistent CLI selection or extension session target is introduced. +- Reuse `FakeBrowserAdapter`, `windowSnapshot`, and `tabSummary` to model two normal windows with distinct active tabs. Extend the adapter's existing operation journals only if a required side effect is not observable already; assertions should cover target IDs and absence of operations on the non-target window rather than handler internals. +- Exercise representative handler boundaries through `handleBrowserRequest`: navigation (`open`), content-routed reads/logs (`snapshot` and `console` or equivalent), content interaction (`click`), and visible-tab capture (`screenshot`). This proves the shared resolver feeds each major execution path instead of duplicating isolated resolver tests. +- Resolve a valid explicit window before any handler side effect. For a missing window or a tab that is absent from the selected window, propagate `BrowserCommandError("INVALID_TARGET")` through the existing response mapping; navigation, content dispatch, tab activation, window focus, and capture journals must remain empty. +- Preserve screenshot's required activation sequence. It may select the requested tab and focus the requested window, invalidates the request snapshot, rereads the target, and passes the refreshed window ID to `captureVisibleTab`. `window.select` similarly focuses, invalidates, and rereads so its result reports current focus metadata; neither operation establishes a durable default. + +### Help and documentation + +- Generate target syntax in route usage/help from the route's selector-dimension metadata. Each route advertises only the flags it consumes: both dimensions, window only, tab only, or neither. +- Update `docs/firefox-cli-spec.md` as the owning product contract: replace the claim that both selectors are global with route-specific dimension semantics, require unsupported dimensions to fail before dispatch, define `window select` as focus-only rather than durable selection, and specify screenshot activation/focus diagnostics. +- Align command reference wording and the Firefox CLI skill with the authoritative spec, show explicit `--window id:` follow-up commands for isolated work, and disclose screenshot's activation/focus side effect without duplicating longer normative explanations. +- Keep the documented selector grammar aligned with `parseTargetValue`: `active`, a non-negative index, or `id:`. Indexes remain those emitted by tab/window listings. + +## Planning + +- [x] Add failing protocol contract tests for selector dimensions. Prove the current broad schemas incorrectly accept `tab.new` with `target.tab` and `window.select` / `window.close` with `target.tab`, then define expected window-only rejection alongside full-target acceptance for commands whose handlers resolve both dimensions. Completion: the tests fail specifically on unsupported selector dimensions that can currently be silently discarded. (2026-07-13: added direct and raw-batch rejection coverage; confirmed red against the current broad schemas.) +- [x] Introduce reusable dimension-specific target schemas and authoritative selector-dimension metadata on protocol CLI routes; narrow asymmetric browsing command params and add registry invariants that metadata and parameter schemas agree. Completion: protocol tests reject unsupported dimensions for direct and raw batch requests, accept documented dimensions, and registry/type tests pass. (2026-07-13: added window-only target schema, route selector metadata, target-policy registry invariant, and direct/raw-batch protocol coverage; focused protocol tests and typecheck pass.) +- [x] Add failing CLI contract tests generated from route selector metadata. Prove each supported dimension serializes with `id:` before and after ordinary positionals, through payload-bearing routes and representative CLI batch steps; prove every unsupported dimension fails before `sendRequest`, explicitly including `tab new --tab`, `window select --tab`, `window close --tab`, and both flags on targetless routes. Completion: failures expose both broad global option registration and any builder that drops a metadata-supported selector. (2026-07-13: confirmed red against global selector registration, then green with parser metadata and pre-transport rejection coverage.) +- [x] Make route parsing and help consume selector-dimension metadata at the shared binding boundary, retaining existing payload disambiguation, value grammar, and builder serialization. Completion: the generated CLI contract suite, malformed/missing-value tests, batch tests, and existing parser/request tests pass; no unsupported selector reaches transport. (2026-07-13: parser contracts and route usage now derive selector options from protocol metadata; focused CLI suites and typecheck pass.) +- [x] Add a failing two-window extension regression fixture using the shared fake browser adapter. Drive explicit-window `open`, read/log, interaction, and screenshot requests through `handleBrowserRequest`; assert every request/result uses the selected window's active tab, the other window's tab receives no navigation/content/capture work, and screenshot capture receives the selected window ID. Completion: the regression fails if explicit-window resolution falls back to the focused window or if screenshot captures the wrong window. (2026-07-13: added the two-window cross-handler fixture; it confirms existing request-scoped resolution already routes correctly.) +- [x] Add failing invalid-target and focus-semantics regressions for the same two-window fixture. Cover an unknown window and a tab not contained in the selected window, asserting `INVALID_TARGET` and empty navigation/content/selection/focus/capture journals; verify `window.select` returns refreshed focused-window metadata without affecting how a later explicitly targeted request resolves. Completion: all failure paths are observable before side effects and the tests distinguish focus from durable target state. (2026-07-13: added missing-window/cross-window-tab side-effect guards and focus-only selection coverage.) +- [x] Make only the extension targeting/handler/test-support corrections exposed by the regressions, preserving request-scoped snapshots and screenshot activation. Completion: the two-window routing, invalid-target, screenshot, window-select, and existing browser-command tests pass with no persistent target state. (2026-07-13: no extension production correction was exposed; existing resolver and screenshot activation already satisfy the fixture.) +- [x] Update the authoritative `docs/firefox-cli-spec.md` global-options and relevant tab/window/screenshot contracts with route-specific supported dimensions, unsupported-dimension pre-dispatch rejection, focus-only `window select` semantics, and screenshot activation diagnostics. Align generated CLI help, `docs/commands.md`, and `skills/firefox-cli/SKILL.md` with that contract and the explicit-window isolation flow; add help assertions derived from selector metadata. Completion: CLI help tests pass, asymmetric and targetless routes advertise no unsupported flags, and a repository-wide documentation search finds no conflicting global-selector, durable-selection, or side-effect claims. (2026-07-13: updated the authoritative spec, command reference, skill, and metadata-derived help assertions.) +- [x] Run focused CLI and extension test files during implementation, then `bun run typecheck` and `bun run check`. Resolve every regression without weakening policy or side-effect assertions. Completion: both root commands exit successfully and `git diff --check` reports no formatting errors. (2026-07-13: focused suites, root typecheck, `bun run check`, and diff check pass.) + +## Review remediation + +- [x] Reject globally-known selector flags before targetless payload fallback and from targetless built-in commands such as `doctor`; cover direct and batch payload forms. (2026-07-13) +- [x] Enforce non-negative selector IDs at direct and raw-batch protocol boundaries. (2026-07-13) +- [x] Make the registry selector-dimension invariant schema-aware and cover a `both`/window-only mismatch. (2026-07-13) +- [x] Extend the two-window routing fixture with `console` and assert its content request is isolated to the selected window. (2026-07-13) diff --git a/docs/commands.md b/docs/commands.md index 17b226d..4f03bd5 100644 --- a/docs/commands.md +++ b/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] [--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 | --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. diff --git a/docs/firefox-cli-spec.md b/docs/firefox-cli-spec.md index 19d4663..4aa9b6d 100644 --- a/docs/firefox-cli-spec.md +++ b/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:`. 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 ` navigates the resolved active tab to match `agent-browser`. Use `tab new [url]` or `open --new-tab ` 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 `: override command timeout. - `--max-output `: cap text output. -- `--window ` and `--tab `: choose a target without hidden sessions. +- `--window ` and `--tab `: 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 `: closes a window. Result is closed window ID. -- `window select `: focuses a window. Result is selected window ID and active tab. +- `window select `: focuses a window. Result includes refreshed focus metadata and its active tab; it does not set a durable default target. Screenshots: diff --git a/packages/cli/src/argv-contracts.ts b/packages/cli/src/argv-contracts.ts index ecee97f..88472e7 100644 --- a/packages/cli/src/argv-contracts.ts +++ b/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(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 readonly optionalValueOptions: ReadonlySet; } { const flags = new Set(); - const valueOptions = new Set(); + const valueOptions = new Set(targetValueOptions); const optionalValueOptions = new Set(); for (const spec of Object.values(specs)) { for (const flag of spec.flags) { diff --git a/packages/cli/src/cli-tabs-targets.test.ts b/packages/cli/src/cli-tabs-targets.test.ts index 19432ae..c019ff3 100644 --- a/packages/cli/src/cli-tabs-targets.test.ts +++ b/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")) + ); +} diff --git a/packages/cli/src/cli-target-contract.test.ts b/packages/cli/src/cli-target-contract.test.ts new file mode 100644 index 0000000..94d4ec5 --- /dev/null +++ b/packages/cli/src/cli-target-contract.test.ts @@ -0,0 +1,135 @@ +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("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); + }); +}); diff --git a/packages/cli/src/help.ts b/packages/cli/src/help.ts index 166a03e..6b5b726 100644 --- a/packages/cli/src/help.ts +++ b/packages/cli/src/help.ts @@ -28,7 +28,7 @@ const routeHelpSpecs = { "Use listed indexes with `--window ` and ids with `--window id:`.", ]), "window.new": helpSpec("Open a new Firefox window, optionally at a URL."), - "window.select": helpSpec("Focus a Firefox window by index or id."), + "window.select": helpSpec("Focus a Firefox window by index or id; it does not establish a durable CLI target."), "window.close": helpSpec("Close a Firefox window by index or id."), open: helpSpec("Navigate the active tab or create a new tab for a URL.", ["Use `--new-tab` when navigation must not replace the active page."]), back: helpSpec("Go back in the target tab history."), @@ -277,7 +277,7 @@ export function renderHelp(): string { "Guidance:", " Use `--json` when another program or agent consumes results.", " Use `firefox-cli snapshot -i` before element actions to get stable `@ref` handles.", - " Use `--tab` and `--window` with active, index values, or id: to target a browser surface.", + " Use only the `--tab` and `--window` options advertised by each command; values are active, index, or id:.", " Use `firefox-cli -h` for contextual command help.", "", ].join("\n"); @@ -308,7 +308,7 @@ function renderGroupHelp(group: HelpGroup): string { "", "Guidance:", " Add `--json` for machine-readable output.", - " Use `--tab` and `--window` on browser-targeted commands when active target is not enough.", + " Use only the selector options advertised by each command when the active target is not enough.", "", ].join("\n"); } diff --git a/packages/cli/src/route-registry.test.ts b/packages/cli/src/route-registry.test.ts index fea1f4e..a255a8a 100644 --- a/packages/cli/src/route-registry.test.ts +++ b/packages/cli/src/route-registry.test.ts @@ -61,4 +61,14 @@ describe("CLI route registry", () => { expect(help).toContain(` ${binding.help}`); } }); + + it("advertises only protocol-declared selector dimensions in route usage", () => { + for (const binding of Object.values(cliRouteBindings)) { + const expectsWindow = binding.route.selectorDimensions === "window" || binding.route.selectorDimensions === "both"; + const expectsTab = binding.route.selectorDimensions === "tab" || binding.route.selectorDimensions === "both"; + + expect(binding.help.includes("--window "), binding.route.id).toBe(expectsWindow); + expect(binding.help.includes("--tab "), binding.route.id).toBe(expectsTab); + } + }); }); diff --git a/packages/cli/src/route-registry.ts b/packages/cli/src/route-registry.ts index 70abcd9..4536800 100644 --- a/packages/cli/src/route-registry.ts +++ b/packages/cli/src/route-registry.ts @@ -143,7 +143,7 @@ function bindCliRoute( return { route: routeEntry.route, command: formatter.command, - help, + help: withTargetSelectorUsage(help, routeEntry.route.selectorDimensions), parser, formatterKind: formatter.kind, formatter: formatter.formatter, @@ -151,6 +151,14 @@ function bindCliRoute( }; } +function withTargetSelectorUsage(help: string, selectorDimensions: CliRouteMetadata["selectorDimensions"]): string { + const selectorUsage = [ + ...(selectorDimensions === "window" || selectorDimensions === "both" ? ["[--window ]"] : []), + ...(selectorDimensions === "tab" || selectorDimensions === "both" ? ["[--tab ]"] : []), + ]; + return selectorUsage.length === 0 ? help : `${help} ${selectorUsage.join(" ")}`; +} + export const cliRouteBindings = { capabilities: bindCliRoute("capabilities", "firefox-cli capabilities [--json]", buildCapabilitiesRequest), "tab.list": bindCliRoute("tab.list", "firefox-cli tab [--json]", buildTabsRequest), @@ -184,7 +192,7 @@ export const cliRouteBindings = { clipboard: bindCliRoute("clipboard", "firefox-cli clipboard read|write|copy|paste [text-or-selector] [--json]", buildClipboardRequest), cookies: bindCliRoute("cookies", "firefox-cli cookies list|get|set|remove [name] [value] [--json]", buildCookiesRequest), storage: bindCliRoute("storage", "firefox-cli storage local|session get|set|remove|clear [key] [value] [--json]", buildStorageRequest), - network: bindCliRoute("network", "firefox-cli network list|clear [--window target] [--tab target] [--json]", buildNetworkRequest), + network: bindCliRoute("network", "firefox-cli network list|clear [--json]", buildNetworkRequest), console: bindCliRoute("console", "firefox-cli console list|clear [--json]", buildLogRequest), errors: bindCliRoute("errors", "firefox-cli errors list|clear [--json]", buildLogRequest), highlight: bindCliRoute("highlight", "firefox-cli highlight [--json]", buildHighlightRequest), diff --git a/packages/cli/src/runner.ts b/packages/cli/src/runner.ts index 02e2ad9..cebb3f9 100644 --- a/packages/cli/src/runner.ts +++ b/packages/cli/src/runner.ts @@ -14,6 +14,7 @@ import { InvalidBatchArgvCommandError, } from "./types.js"; import { createUploadBudget } from "./upload.js"; +import { rejectTargetSelectorOptions } from "./argv-contracts.js"; const gatedCapabilitiesByCommand = new Map(gatedCapabilities.map((capability) => [capability.command, capability] as const)); @@ -43,14 +44,17 @@ async function runCliOrThrow(args: readonly string[], dependencies: CliDependenc } if (args[0] === "setup") { + rejectTargetSelectorOptions(args.slice(1), "setup"); return setup(args.slice(1), dependencies, renderHelp); } if (args[0] === "doctor") { + rejectTargetSelectorOptions(args.slice(1), "doctor"); return doctor(args.slice(1), dependencies); } if (args[0] === "unpair") { + rejectTargetSelectorOptions(args.slice(1), "unpair"); await dependencies.clearPairState?.(); return ok("Pair state cleared. Run `firefox-cli connect` to request approval again.\n"); } diff --git a/packages/extension/src/browser-commands-targets.test.ts b/packages/extension/src/browser-commands-targets.test.ts index ee2b92a..0dab7bc 100644 --- a/packages/extension/src/browser-commands-targets.test.ts +++ b/packages/extension/src/browser-commands-targets.test.ts @@ -1,4 +1,6 @@ -import { describe, it } from "vitest"; +import { createRequest } from "@firefox-cli/protocol"; +import { describe, expect, it } from "vitest"; +import { handleBrowserRequest } from "./browser-commands.js"; import { runCase01, runCase02, @@ -11,6 +13,7 @@ import { runCase09, runCase10, } from "./browser-commands-targets-test-cases.js"; +import { FakeBrowserAdapter, tabSummary, windowSnapshot } from "./browser-commands-test-utils.js"; describe("browser command handling", () => { it("routes element state checks to the resolved tab content script and adds target metadata", runCase01); @@ -23,4 +26,80 @@ describe("browser command handling", () => { it("uses the resolved target tab for network-idle waits", runCase08); it("lists and clears network requests for the resolved target tab only", runCase09); it("runs eval in the resolved tab main world and adds target metadata", runCase10); + + it("routes explicit-window navigation, reads, interactions, and screenshots to that window's active tab", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + const target = { window: { kind: "id" as const, id: 20 } }; + + const responses = [ + await handleBrowserRequest(createRequest("open", { url: "https://qa.example/", newTab: false, target }, "qa-open"), adapter), + await handleBrowserRequest(createRequest("snapshot", { interactiveOnly: true, compact: true, target }, "qa-snapshot"), adapter), + await handleBrowserRequest(createRequest("console", { action: "list", target }, "qa-console"), adapter), + await handleBrowserRequest(createRequest("click", { selector: "#save", target }, "qa-click"), adapter), + await handleBrowserRequest(createRequest("screenshot", { path: "/tmp/qa.png", format: "png", target }, "qa-screenshot"), adapter), + ]; + + for (const response of [responses[0], responses[1], responses[3], responses[4]]) { + expect(response).toMatchObject({ ok: true, result: { target: { windowId: 20, tabId: 201 } } }); + } + expect(responses[2]).toMatchObject({ ok: true, result: { action: "list" } }); + expect(adapter.navigations).toEqual([{ tabId: 201, url: "https://qa.example/" }]); + expect(adapter.contentRequests).toEqual([ + { tabId: 201, command: "snapshot" }, + { tabId: 201, command: "console" }, + { tabId: 201, command: "click" }, + ]); + expect(adapter.captureRequests).toEqual([{ windowId: 20, options: { format: "png" } }]); + expect(adapter.selectedTabs).toEqual([]); + expect(adapter.focusedWindows).toEqual([20]); + expect(adapter.navigations.some((navigation) => navigation.tabId === 101)).toBe(false); + expect(adapter.contentRequests.some((request) => request.tabId === 101)).toBe(false); + }); + + it.each([ + ["missing window", { window: { kind: "id" as const, id: 999 } }], + ["tab outside selected window", { window: { kind: "id" as const, id: 20 }, tab: { kind: "id" as const, id: 101 } }], + ])("rejects %s before target-dependent side effects", async (_name, target) => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const responses = [ + await handleBrowserRequest(createRequest("open", { url: "https://qa.example/", newTab: false, target }, "invalid-open"), adapter), + await handleBrowserRequest(createRequest("snapshot", { interactiveOnly: true, compact: true, target }, "invalid-snapshot"), adapter), + await handleBrowserRequest(createRequest("click", { selector: "#save", target }, "invalid-click"), adapter), + await handleBrowserRequest(createRequest("screenshot", { path: "/tmp/invalid.png", format: "png", target }, "invalid-screenshot"), adapter), + ]; + + for (const response of responses) { + expect(response).toMatchObject({ ok: false, error: { code: "INVALID_TARGET" } }); + } + expect(adapter.navigations).toEqual([]); + expect(adapter.contentRequests).toEqual([]); + expect(adapter.selectedTabs).toEqual([]); + expect(adapter.focusedWindows).toEqual([]); + expect(adapter.captureRequests).toEqual([]); + }); + + it("reports refreshed focus metadata for window selection without changing later explicit target resolution", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const selected = await handleBrowserRequest(createRequest("window.select", { target: { window: { kind: "id", id: 20 } } }, "select-window-20"), adapter); + const snapshot = await handleBrowserRequest( + createRequest("snapshot", { interactiveOnly: true, compact: true, target: { window: { kind: "id", id: 10 } } }, "snapshot-window-10"), + adapter, + ); + + expect(selected).toMatchObject({ ok: true, result: { window: { id: 20, focused: true }, target: { tabId: 201 } } }); + expect(snapshot).toMatchObject({ ok: true, result: { target: { windowId: 10, tabId: 101 } } }); + expect(adapter.focusedWindows).toEqual([20]); + expect(adapter.contentRequests).toEqual([{ tabId: 101, command: "snapshot" }]); + }); }); diff --git a/packages/protocol/src/browser/output.ts b/packages/protocol/src/browser/output.ts index 53ad30e..acc4539 100644 --- a/packages/protocol/src/browser/output.ts +++ b/packages/protocol/src/browser/output.ts @@ -70,7 +70,6 @@ export type NotifyResult = z.infer; export const pdfParamsSchema = z .object({ - target: targetSelectorSchema.optional(), path: z.string().min(1), }) .strict(); diff --git a/packages/protocol/src/metadata.ts b/packages/protocol/src/metadata.ts index 2295b91..8369b0a 100644 --- a/packages/protocol/src/metadata.ts +++ b/packages/protocol/src/metadata.ts @@ -4,6 +4,7 @@ import type { CapabilityStatus } from "./core.js"; export type CommandOwner = "native-host" | "extension"; export type CommandTargetPolicy = "none" | "optional" | "required" | "mixed"; +export type CliRouteSelectorDimensions = "neither" | "window" | "tab" | "both"; export type CommandContentPolicy = "never" | "always" | "mixed" | "action"; export type CommandTimeoutPolicy = "none" | "command" | "batch"; export type CommandFrameScope = "not-applicable" | "main-frame-only" | "main-frame-with-iframe-diagnostics"; @@ -56,6 +57,7 @@ export interface CliRouteMetadata { readonly id: string; readonly path: readonly [string, ...string[]]; readonly batch: boolean; + readonly selectorDimensions: CliRouteSelectorDimensions; } export interface CliRouteEntry { @@ -71,6 +73,7 @@ export interface CommandBatchMetadata { } export interface CommandSchemaMetadata { + readonly targetSelectorSchema?: z.ZodType; readonly owner: CommandOwner; readonly target: CommandTargetPolicy; readonly content: CommandContentPolicy; diff --git a/packages/protocol/src/protocol-metadata-behavior.test.ts b/packages/protocol/src/protocol-metadata-behavior.test.ts index 6313fcb..1e818c9 100644 --- a/packages/protocol/src/protocol-metadata-behavior.test.ts +++ b/packages/protocol/src/protocol-metadata-behavior.test.ts @@ -23,7 +23,7 @@ import { kernelCapabilities, parseBoundaryRequest, } from "./index.js"; -import { commandIds, sorted, expectedCliRoutesByCommand } from "./protocol-test-support.js"; +import { commandIds, expectedCliRouteSelectorDimensions, expectedCliRoutesByCommand, sorted } from "./protocol-test-support.js"; describe("protocol command metadata", () => { it("uses unique CLI route ids and paths", () => { @@ -35,16 +35,22 @@ describe("protocol command metadata", () => { const routePaths = routes.map((route) => route.path.join("\0")); for (const command of commandIds()) { - expect(getCommandCliRoutes(command)).toEqual(expectedCliRoutesByCommand[command] ?? []); + expect(getCommandCliRoutes(command).map(withoutSelectorDimensions)).toEqual(expectedCliRoutesByCommand[command] ?? []); } - expect(routes).toEqual(expectedRoutes); - expect(routeEntries).toEqual(expectedRouteEntries); + expect(routes.map(withoutSelectorDimensions)).toEqual(expectedRoutes); + expect(routeEntries.map((entry) => ({ ...entry, route: withoutSelectorDimensions(entry.route) }))).toEqual(expectedRouteEntries); expect(new Set(routeIds).size).toBe(routeIds.length); expect(new Set(routePaths).size).toBe(routePaths.length); expect(routes.every((route) => route.path.length > 0)).toBe(true); expect(routes.every((route) => route.path.every((segment) => segment.length > 0))).toBe(true); }); + it("declares the selector dimensions consumed by every CLI route", () => { + for (const route of getCliRoutes()) { + expect(route.selectorDimensions, route.id).toBe(expectedCliRouteSelectorDimensions(route.id)); + } + }); + it("includes all command statuses and gated capabilities in kernel capabilities", () => { for (const command of commandIds()) { expect(kernelCapabilities).toContainEqual({ @@ -201,6 +207,19 @@ describe("protocol command metadata", () => { }); }); +function withoutSelectorDimensions(route: { + readonly id: string; + readonly path: readonly [string, ...string[]]; + readonly batch: boolean; + readonly selectorDimensions: string; +}): { readonly id: string; readonly path: readonly [string, ...string[]]; readonly batch: boolean } { + return { + id: route.id, + path: route.path, + batch: route.batch, + }; +} + describe("request protocol compatibility", () => { it("derives command protocol requirements from registry metadata", () => { expect(getCommandCompatibilityMetadata("network")).toEqual({ diff --git a/packages/protocol/src/protocol-request.test.ts b/packages/protocol/src/protocol-request.test.ts index ae6ab5a..53c08c5 100644 --- a/packages/protocol/src/protocol-request.test.ts +++ b/packages/protocol/src/protocol-request.test.ts @@ -200,4 +200,97 @@ describe("parseBoundaryRequest", () => { expect(requests.map((request) => parseBoundaryRequest("host-to-extension", request))).toEqual(requests.map((request) => ({ ok: true, value: request }))); }); + + it("rejects tab selectors for window-only browsing commands at direct and batch boundaries", () => { + const invalidRequests: readonly unknown[] = [ + { + protocolVersion: PROTOCOL_VERSION, + id: "tab-new-tab-target", + command: "tab.new", + params: { target: { tab: { kind: "id", id: 42 } } }, + }, + { + protocolVersion: PROTOCOL_VERSION, + id: "window-select-tab-target", + command: "window.select", + params: { target: { tab: { kind: "id", id: 42 } } }, + }, + { + protocolVersion: PROTOCOL_VERSION, + id: "window-close-tab-target", + command: "window.close", + params: { target: { tab: { kind: "id", id: 42 } } }, + }, + { + protocolVersion: PROTOCOL_VERSION, + id: "batch-window-only-tab-target", + command: "batch", + params: { + steps: [ + { command: "tab.new", params: { target: { tab: { kind: "id", id: 42 } } } }, + { command: "window.select", params: { target: { tab: { kind: "id", id: 42 } } } }, + { command: "window.close", params: { target: { tab: { kind: "id", id: 42 } } } }, + ], + }, + }, + ]; + + for (const request of invalidRequests) { + const parsed = parseBoundaryRequest("host-to-extension", request); + expect(parsed.ok).toBe(false); + if (!parsed.ok) { + expect(parsed.error.code).toBe("INVALID_ENVELOPE"); + } + } + }); + + it("rejects negative selector ids at direct and batch boundaries", () => { + const requests: readonly unknown[] = [ + { + protocolVersion: PROTOCOL_VERSION, + id: "negative-window", + command: "open", + params: { url: "https://example.com/", newTab: false, target: { window: { kind: "id", id: -1 } } }, + }, + { + protocolVersion: PROTOCOL_VERSION, + id: "negative-tab-batch", + command: "batch", + params: { + steps: [{ command: "snapshot", params: { target: { tab: { kind: "id", id: -1 } } } }], + }, + }, + ]; + + for (const request of requests) { + expect(parseBoundaryRequest("host-to-extension", request)).toMatchObject({ + ok: false, + error: { code: "INVALID_ENVELOPE" }, + }); + } + }); + + it("accepts window selectors for window-only browsing commands and full selectors for page commands", () => { + const validRequests = [ + createRequest("tab.new", { target: { window: { kind: "id", id: 7 } } }, "tab-new-window-target"), + createRequest("window.select", { target: { window: { kind: "id", id: 7 } } }, "window-select-window-target"), + createRequest("window.close", { target: { window: { kind: "id", id: 7 } } }, "window-close-window-target"), + createRequest( + "open", + { + url: "https://example.com/", + newTab: false, + target: { + window: { kind: "id", id: 7 }, + tab: { kind: "id", id: 42 }, + }, + }, + "open-full-target", + ), + ]; + + expect(validRequests.map((request) => parseBoundaryRequest("host-to-extension", request))).toEqual( + validRequests.map((request) => ({ ok: true, value: request })), + ); + }); }); diff --git a/packages/protocol/src/protocol-test-support.ts b/packages/protocol/src/protocol-test-support.ts index adf9559..86be51f 100644 --- a/packages/protocol/src/protocol-test-support.ts +++ b/packages/protocol/src/protocol-test-support.ts @@ -1,4 +1,4 @@ -import { commandSchemas, type Boundary, type CliRouteMetadata, type CommandId, type ComponentIdentity } from "./index.js"; +import { commandSchemas, type Boundary, type CliRouteSelectorDimensions, type CommandId, type ComponentIdentity } from "./index.js"; export const boundaries: readonly Boundary[] = ["cli-to-host", "host-to-extension", "extension-to-content-script"]; export const inheritedCommandNames = ["toString", "constructor", "__proto__"] as const; @@ -24,7 +24,13 @@ export function sorted(values: readonly string[]): string[] { return [...values].sort((left, right) => left.localeCompare(right)); } -export const expectedCliRoutesByCommand: Partial> = { +interface ExpectedCliRouteMetadata { + readonly id: string; + readonly path: readonly [string, ...string[]]; + readonly batch: boolean; +} + +export const expectedCliRoutesByCommand: Partial> = { capabilities: [{ id: "capabilities", path: ["capabilities"], batch: false }], "tabs.list": [{ id: "tab.list", path: ["tab"], batch: true }], "tab.new": [{ id: "tab.new", path: ["tab", "new"], batch: true }], @@ -84,3 +90,21 @@ export const expectedCliRoutesByCommand: Partial> = { + capabilities: "neither", + "tab.new": "window", + "window.list": "neither", + "window.new": "neither", + "window.select": "window", + "window.close": "window", + download: "neither", + cookies: "neither", + notify: "neither", + pdf: "neither", + connect: "neither", +}; + +export function expectedCliRouteSelectorDimensions(routeId: string): CliRouteSelectorDimensions { + return selectorDimensionsByRouteId[routeId] ?? "both"; +} diff --git a/packages/protocol/src/registry/actions.ts b/packages/protocol/src/registry/actions.ts index 2e7e1c2..0c04e34 100644 --- a/packages/protocol/src/registry/actions.ts +++ b/packages/protocol/src/registry/actions.ts @@ -23,7 +23,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "click", path: ["click"], batch: true }], + cliRoutes: [{ id: "click", path: ["click"], batch: true, selectorDimensions: "both" }], }, dblclick: { params: elementActionParamsSchema, @@ -35,7 +35,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "dblclick", path: ["dblclick"], batch: true }], + cliRoutes: [{ id: "dblclick", path: ["dblclick"], batch: true, selectorDimensions: "both" }], }, focus: { params: elementActionParamsSchema, @@ -47,7 +47,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "focus", path: ["focus"], batch: true }], + cliRoutes: [{ id: "focus", path: ["focus"], batch: true, selectorDimensions: "both" }], }, hover: { params: elementActionParamsSchema, @@ -59,7 +59,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "hover", path: ["hover"], batch: true }], + cliRoutes: [{ id: "hover", path: ["hover"], batch: true, selectorDimensions: "both" }], }, fill: { params: textActionParamsSchema, @@ -71,7 +71,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "fill", path: ["fill"], batch: true }], + cliRoutes: [{ id: "fill", path: ["fill"], batch: true, selectorDimensions: "both" }], }, type: { params: textActionParamsSchema, @@ -83,7 +83,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "type", path: ["type"], batch: true }], + cliRoutes: [{ id: "type", path: ["type"], batch: true, selectorDimensions: "both" }], }, press: { params: pressParamsSchema, @@ -95,7 +95,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "press", path: ["press"], batch: true }], + cliRoutes: [{ id: "press", path: ["press"], batch: true, selectorDimensions: "both" }], }, "keyboard.type": { params: keyboardTextActionParamsSchema, @@ -107,7 +107,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "keyboard.type", path: ["keyboard", "type"], batch: true }], + cliRoutes: [{ id: "keyboard.type", path: ["keyboard", "type"], batch: true, selectorDimensions: "both" }], }, "keyboard.inserttext": { params: keyboardTextActionParamsSchema, @@ -119,7 +119,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "keyboard.inserttext", path: ["keyboard", "inserttext"], batch: true }], + cliRoutes: [{ id: "keyboard.inserttext", path: ["keyboard", "inserttext"], batch: true, selectorDimensions: "both" }], }, check: { params: elementActionParamsSchema, @@ -131,7 +131,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "check", path: ["check"], batch: true }], + cliRoutes: [{ id: "check", path: ["check"], batch: true, selectorDimensions: "both" }], }, uncheck: { params: elementActionParamsSchema, @@ -143,7 +143,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "uncheck", path: ["uncheck"], batch: true }], + cliRoutes: [{ id: "uncheck", path: ["uncheck"], batch: true, selectorDimensions: "both" }], }, select: { params: selectParamsSchema, @@ -155,7 +155,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "select", path: ["select"], batch: true }], + cliRoutes: [{ id: "select", path: ["select"], batch: true, selectorDimensions: "both" }], }, scroll: { params: scrollParamsSchema, @@ -167,7 +167,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "scroll", path: ["scroll"], batch: true }], + cliRoutes: [{ id: "scroll", path: ["scroll"], batch: true, selectorDimensions: "both" }], }, scrollintoview: { params: elementActionParamsSchema, @@ -179,7 +179,7 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "scrollintoview", path: ["scrollintoview"], batch: true }], + cliRoutes: [{ id: "scrollintoview", path: ["scrollintoview"], batch: true, selectorDimensions: "both" }], }, swipe: { params: scrollParamsSchema, @@ -191,6 +191,6 @@ export const actionsCommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "swipe", path: ["swipe"], batch: true }], + cliRoutes: [{ id: "swipe", path: ["swipe"], batch: true, selectorDimensions: "both" }], }, }); diff --git a/packages/protocol/src/registry/browsing.ts b/packages/protocol/src/registry/browsing.ts index 3e7627d..3bbb105 100644 --- a/packages/protocol/src/registry/browsing.ts +++ b/packages/protocol/src/registry/browsing.ts @@ -13,6 +13,7 @@ import { windowNewResultSchema, windowSelectResultSchema, windowTargetParamsSchema, + windowTargetSelectorSchema, windowsListParamsSchema, windowsListResultSchema, } from "../target.js"; @@ -29,9 +30,10 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "tab.list", path: ["tab"], batch: true }], + cliRoutes: [{ id: "tab.list", path: ["tab"], batch: true, selectorDimensions: "both" }], }, "tab.new": { + targetSelectorSchema: windowTargetSelectorSchema, params: tabNewParamsSchema, result: tabNewResultSchema, status: "mvp", @@ -41,7 +43,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "tab.new", path: ["tab", "new"], batch: true }], + cliRoutes: [{ id: "tab.new", path: ["tab", "new"], batch: true, selectorDimensions: "window" }], }, "tab.select": { params: tabTargetParamsSchema, @@ -53,7 +55,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "tab.select", path: ["tab", "select"], batch: true }], + cliRoutes: [{ id: "tab.select", path: ["tab", "select"], batch: true, selectorDimensions: "both" }], }, "tab.close": { params: tabTargetParamsSchema, @@ -65,7 +67,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "tab.close", path: ["tab", "close"], batch: true }], + cliRoutes: [{ id: "tab.close", path: ["tab", "close"], batch: true, selectorDimensions: "both" }], }, "windows.list": { params: windowsListParamsSchema, @@ -77,7 +79,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true }, - cliRoutes: [{ id: "window.list", path: ["window"], batch: true }], + cliRoutes: [{ id: "window.list", path: ["window"], batch: true, selectorDimensions: "neither" }], }, "window.new": { params: windowNewParamsSchema, @@ -89,9 +91,10 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true }, - cliRoutes: [{ id: "window.new", path: ["window", "new"], batch: true }], + cliRoutes: [{ id: "window.new", path: ["window", "new"], batch: true, selectorDimensions: "neither" }], }, "window.select": { + targetSelectorSchema: windowTargetSelectorSchema, params: windowTargetParamsSchema, result: windowSelectResultSchema, status: "mvp", @@ -101,9 +104,10 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "window.select", path: ["window", "select"], batch: true }], + cliRoutes: [{ id: "window.select", path: ["window", "select"], batch: true, selectorDimensions: "window" }], }, "window.close": { + targetSelectorSchema: windowTargetSelectorSchema, params: windowTargetParamsSchema, result: windowCloseResultSchema, status: "mvp", @@ -113,7 +117,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "window.close", path: ["window", "close"], batch: true }], + cliRoutes: [{ id: "window.close", path: ["window", "close"], batch: true, selectorDimensions: "window" }], }, open: { params: openParamsSchema, @@ -125,7 +129,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "open", path: ["open"], batch: true }], + cliRoutes: [{ id: "open", path: ["open"], batch: true, selectorDimensions: "both" }], }, back: { params: navigationParamsSchema, @@ -137,7 +141,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "back", path: ["back"], batch: true }], + cliRoutes: [{ id: "back", path: ["back"], batch: true, selectorDimensions: "both" }], }, forward: { params: navigationParamsSchema, @@ -149,7 +153,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "forward", path: ["forward"], batch: true }], + cliRoutes: [{ id: "forward", path: ["forward"], batch: true, selectorDimensions: "both" }], }, reload: { params: navigationParamsSchema, @@ -161,6 +165,6 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "reload", path: ["reload"], batch: true }], + cliRoutes: [{ id: "reload", path: ["reload"], batch: true, selectorDimensions: "both" }], }, }); diff --git a/packages/protocol/src/registry/content.ts b/packages/protocol/src/registry/content.ts index dabdbe8..ebb0857 100644 --- a/packages/protocol/src/registry/content.ts +++ b/packages/protocol/src/registry/content.ts @@ -28,7 +28,7 @@ export const contentCommandEntries = defineCommandEntries({ future: "docs/iframe-targeting-future.md", }, batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "snapshot", path: ["snapshot"], batch: true }], + cliRoutes: [{ id: "snapshot", path: ["snapshot"], batch: true, selectorDimensions: "both" }], }, "ref.resolve": { params: refResolveParamsSchema, @@ -40,7 +40,7 @@ export const contentCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "ref", path: ["ref"], batch: true }], + cliRoutes: [{ id: "ref", path: ["ref"], batch: true, selectorDimensions: "both" }], }, get: { params: getParamsSchema, @@ -52,7 +52,7 @@ export const contentCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "get", path: ["get"], batch: true }], + cliRoutes: [{ id: "get", path: ["get"], batch: true, selectorDimensions: "both" }], }, is: { params: isParamsSchema, @@ -64,7 +64,7 @@ export const contentCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "is", path: ["is"], batch: true }], + cliRoutes: [{ id: "is", path: ["is"], batch: true, selectorDimensions: "both" }], }, wait: { params: waitParamsSchema, @@ -94,6 +94,6 @@ export const contentCommandEntries = defineCommandEntries({ ], }, batch: { allowed: true, extensionDefaultTarget: true, timeoutRebase: true }, - cliRoutes: [{ id: "wait", path: ["wait"], batch: true }], + cliRoutes: [{ id: "wait", path: ["wait"], batch: true, selectorDimensions: "both" }], }, }); diff --git a/packages/protocol/src/registry/core.ts b/packages/protocol/src/registry/core.ts index 417c1cf..25c8334 100644 --- a/packages/protocol/src/registry/core.ts +++ b/packages/protocol/src/registry/core.ts @@ -24,7 +24,7 @@ export const coreCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: false }, - cliRoutes: [{ id: "capabilities", path: ["capabilities"], batch: false }], + cliRoutes: [{ id: "capabilities", path: ["capabilities"], batch: false, selectorDimensions: "neither" }], }, noop: { params: noOpParamsSchema, diff --git a/packages/protocol/src/registry/define.ts b/packages/protocol/src/registry/define.ts index 914a151..1cdc6b6 100644 --- a/packages/protocol/src/registry/define.ts +++ b/packages/protocol/src/registry/define.ts @@ -1,4 +1,5 @@ -import type { CommandSchemaEntry } from "../metadata.js"; +import type { CliRouteSelectorDimensions, CommandSchemaEntry } from "../metadata.js"; +import { targetSelectorSchema } from "../target.js"; export type CommandRegistryFragment = Readonly>; @@ -23,6 +24,40 @@ export function assembleCommandRegistry(...fragments: readonly CommandRegistryFr registry[command] = schema; } } + assertCliRouteSelectorDimensions(registry); return registry; } + +function assertCliRouteSelectorDimensions(registry: Readonly>): void { + for (const [command, schema] of Object.entries(registry)) { + for (const route of schema.cliRoutes) { + const selectorDimensions: CliRouteSelectorDimensions = route.selectorDimensions; + const paramsSelectorDimensions = selectorDimensionsAcceptedByCommand(schema); + if (selectorDimensions !== paramsSelectorDimensions) { + throw new Error( + `CLI route selector dimensions disagree with command params: ${command} (${route.id}) declares ${selectorDimensions}, params accept ${paramsSelectorDimensions}.`, + ); + } + } + } +} + +function selectorDimensionsAcceptedByCommand(schema: CommandSchemaEntry): CliRouteSelectorDimensions { + const selectorSchema = schema.targetSelectorSchema ?? (schema.target === "none" ? undefined : targetSelectorSchema); + if (selectorSchema === undefined) { + return "neither"; + } + const acceptsWindow = selectorSchema.safeParse({ window: { kind: "active" } }).success; + const acceptsTab = selectorSchema.safeParse({ tab: { kind: "active" } }).success; + if (acceptsWindow && acceptsTab) { + return "both"; + } + if (acceptsWindow) { + return "window"; + } + if (acceptsTab) { + return "tab"; + } + return "neither"; +} diff --git a/packages/protocol/src/registry/index.ts b/packages/protocol/src/registry/index.ts index 6017a8c..95f82a1 100644 --- a/packages/protocol/src/registry/index.ts +++ b/packages/protocol/src/registry/index.ts @@ -63,7 +63,7 @@ const batchCommandEntries = defineCommandEntries({ action: false, timeout: "batch", batch: { allowed: false }, - cliRoutes: [{ id: "batch", path: ["batch"], batch: false }], + cliRoutes: [{ id: "batch", path: ["batch"], batch: false, selectorDimensions: "both" }], }, }); diff --git a/packages/protocol/src/registry/pairing.ts b/packages/protocol/src/registry/pairing.ts index 53e80c1..68372b8 100644 --- a/packages/protocol/src/registry/pairing.ts +++ b/packages/protocol/src/registry/pairing.ts @@ -53,7 +53,7 @@ export const pairingCommandEntries = defineCommandEntries({ ], }, batch: { allowed: false }, - cliRoutes: [{ id: "connect", path: ["connect"], batch: false }], + cliRoutes: [{ id: "connect", path: ["connect"], batch: false, selectorDimensions: "neither" }], }, "pair.openApproval": { params: pairOpenApprovalParamsSchema, diff --git a/packages/protocol/src/registry/phase8.ts b/packages/protocol/src/registry/phase8.ts index 1836a91..d9e8041 100644 --- a/packages/protocol/src/registry/phase8.ts +++ b/packages/protocol/src/registry/phase8.ts @@ -58,7 +58,7 @@ export const phase8CommandEntries = defineCommandEntries({ future: "docs/iframe-targeting-future.md", }, batch: { allowed: true, extensionDefaultTarget: true, timeoutRebase: true }, - cliRoutes: [{ id: "eval", path: ["eval"], batch: true }], + cliRoutes: [{ id: "eval", path: ["eval"], batch: true, selectorDimensions: "both" }], }, screenshot: { params: screenshotParamsSchema, @@ -70,7 +70,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "command", batch: { allowed: true, extensionDefaultTarget: true, timeoutRebase: true }, - cliRoutes: [{ id: "screenshot", path: ["screenshot"], batch: true }], + cliRoutes: [{ id: "screenshot", path: ["screenshot"], batch: true, selectorDimensions: "both" }], }, drag: { params: dragParamsSchema, @@ -82,7 +82,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "drag", path: ["drag"], batch: true }], + cliRoutes: [{ id: "drag", path: ["drag"], batch: true, selectorDimensions: "both" }], }, upload: { params: uploadParamsSchema, @@ -94,7 +94,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "upload", path: ["upload"], batch: true }], + cliRoutes: [{ id: "upload", path: ["upload"], batch: true, selectorDimensions: "both" }], }, mouse: { params: mouseParamsSchema, @@ -106,7 +106,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "mouse", path: ["mouse"], batch: true }], + cliRoutes: [{ id: "mouse", path: ["mouse"], batch: true, selectorDimensions: "both" }], }, keydown: { params: keyEventParamsSchema, @@ -118,7 +118,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "keydown", path: ["keydown"], batch: true }], + cliRoutes: [{ id: "keydown", path: ["keydown"], batch: true, selectorDimensions: "both" }], }, keyup: { params: keyEventParamsSchema, @@ -130,7 +130,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: true, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "keyup", path: ["keyup"], batch: true }], + cliRoutes: [{ id: "keyup", path: ["keyup"], batch: true, selectorDimensions: "both" }], }, find: { params: findParamsSchema, @@ -142,7 +142,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "find", path: ["find"], batch: true }], + cliRoutes: [{ id: "find", path: ["find"], batch: true, selectorDimensions: "both" }], }, frame: { params: frameParamsSchema, @@ -159,7 +159,7 @@ export const phase8CommandEntries = defineCommandEntries({ future: "docs/iframe-targeting-future.md", }, batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "frame", path: ["frame"], batch: true }], + cliRoutes: [{ id: "frame", path: ["frame"], batch: true, selectorDimensions: "both" }], }, download: { params: downloadParamsSchema, @@ -172,7 +172,7 @@ export const phase8CommandEntries = defineCommandEntries({ timeout: "none", security: { level: "sensitive", reasons: ["downloads"] }, batch: { allowed: true }, - cliRoutes: [{ id: "download", path: ["download"], batch: true }], + cliRoutes: [{ id: "download", path: ["download"], batch: true, selectorDimensions: "neither" }], }, dialog: { params: dialogParamsSchema, @@ -184,7 +184,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "dialog", path: ["dialog"], batch: true }], + cliRoutes: [{ id: "dialog", path: ["dialog"], batch: true, selectorDimensions: "both" }], }, clipboard: { params: clipboardParamsSchema, @@ -197,7 +197,7 @@ export const phase8CommandEntries = defineCommandEntries({ timeout: "none", security: { level: "sensitive", reasons: ["clipboard"] }, batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "clipboard", path: ["clipboard"], batch: true }], + cliRoutes: [{ id: "clipboard", path: ["clipboard"], batch: true, selectorDimensions: "both" }], }, cookies: { params: cookieParamsSchema, @@ -210,7 +210,7 @@ export const phase8CommandEntries = defineCommandEntries({ timeout: "none", security: { level: "sensitive", reasons: ["cookies"] }, batch: { allowed: true }, - cliRoutes: [{ id: "cookies", path: ["cookies"], batch: true }], + cliRoutes: [{ id: "cookies", path: ["cookies"], batch: true, selectorDimensions: "neither" }], }, storage: { params: storageParamsSchema, @@ -222,7 +222,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "storage", path: ["storage"], batch: true }], + cliRoutes: [{ id: "storage", path: ["storage"], batch: true, selectorDimensions: "both" }], }, network: { params: networkParamsSchema, @@ -243,7 +243,7 @@ export const phase8CommandEntries = defineCommandEntries({ ], }, batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "network", path: ["network"], batch: true }], + cliRoutes: [{ id: "network", path: ["network"], batch: true, selectorDimensions: "both" }], }, console: { params: consoleParamsSchema, @@ -255,7 +255,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "console", path: ["console"], batch: true }], + cliRoutes: [{ id: "console", path: ["console"], batch: true, selectorDimensions: "both" }], }, errors: { params: errorsParamsSchema, @@ -267,7 +267,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "errors", path: ["errors"], batch: true }], + cliRoutes: [{ id: "errors", path: ["errors"], batch: true, selectorDimensions: "both" }], }, highlight: { params: highlightParamsSchema, @@ -279,7 +279,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "highlight", path: ["highlight"], batch: true }], + cliRoutes: [{ id: "highlight", path: ["highlight"], batch: true, selectorDimensions: "both" }], }, notify: { params: notifyParamsSchema, @@ -300,19 +300,19 @@ export const phase8CommandEntries = defineCommandEntries({ ], }, batch: { allowed: true }, - cliRoutes: [{ id: "notify", path: ["notify"], batch: true }], + cliRoutes: [{ id: "notify", path: ["notify"], batch: true, selectorDimensions: "neither" }], }, pdf: { params: pdfParamsSchema, result: pdfResultSchema, status: "unsupported", owner: "extension", - target: "optional", + target: "none", content: "never", action: false, timeout: "none", batch: { allowed: true }, - cliRoutes: [{ id: "pdf", path: ["pdf"], batch: true }], + cliRoutes: [{ id: "pdf", path: ["pdf"], batch: true, selectorDimensions: "neither" }], }, "set.viewport": { params: setViewportParamsSchema, @@ -324,7 +324,7 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "set.viewport", path: ["set", "viewport"], batch: true }], + cliRoutes: [{ id: "set.viewport", path: ["set", "viewport"], batch: true, selectorDimensions: "both" }], }, diff: { params: diffParamsSchema, @@ -336,6 +336,6 @@ export const phase8CommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "diff", path: ["diff"], batch: true }], + cliRoutes: [{ id: "diff", path: ["diff"], batch: true, selectorDimensions: "both" }], }, }); diff --git a/packages/protocol/src/registry/registry.test.ts b/packages/protocol/src/registry/registry.test.ts index 6611c11..f02982d 100644 --- a/packages/protocol/src/registry/registry.test.ts +++ b/packages/protocol/src/registry/registry.test.ts @@ -118,6 +118,39 @@ describe("command registry assembly", () => { expect(() => assembleCommandRegistry(duplicate, duplicate)).toThrow("Duplicate command id: duplicate"); }); + it("rejects route dimensions that disagree with the target parameter schema", () => { + const windowOnly = defineCommandEntries({ + mismatch: { + params: z + .object({ + target: z + .object({ + window: z.object({ kind: z.literal("active") }).strict(), + }) + .strict() + .optional(), + }) + .strict(), + result: z.object({ ok: z.literal(true) }).strict(), + status: "mvp", + targetSelectorSchema: z + .object({ + window: z.object({ kind: z.literal("active") }).strict(), + }) + .strict(), + owner: "extension", + target: "optional", + content: "never", + action: false, + timeout: "none", + batch: { allowed: false }, + cliRoutes: [{ id: "mismatch", path: ["mismatch"], batch: false, selectorDimensions: "both" }], + }, + }); + + expect(() => assembleCommandRegistry(windowOnly)).toThrow("CLI route selector dimensions disagree with command params"); + }); + it("preserves command-specific public protocol types", () => { const assertions: ProtocolTypeAssertions = [true, true, true, true, true, true, true]; diff --git a/packages/protocol/src/target.ts b/packages/protocol/src/target.ts index 1953870..a2de226 100644 --- a/packages/protocol/src/target.ts +++ b/packages/protocol/src/target.ts @@ -14,26 +14,27 @@ export const tabSummarySchema = z .strict(); export type TabSummary = z.infer; +export const targetDimensionSelectorSchema = z.union([ + z.object({ kind: z.literal("active") }).strict(), + z.object({ kind: z.literal("id"), id: z.number().int().nonnegative() }).strict(), + z.object({ kind: z.literal("index"), index: z.number().int().nonnegative() }).strict(), +]); + export const targetSelectorSchema = z .object({ - window: z - .union([ - z.object({ kind: z.literal("active") }).strict(), - z.object({ kind: z.literal("id"), id: z.number().int() }).strict(), - z.object({ kind: z.literal("index"), index: z.number().int().nonnegative() }).strict(), - ]) - .optional(), - tab: z - .union([ - z.object({ kind: z.literal("active") }).strict(), - z.object({ kind: z.literal("id"), id: z.number().int() }).strict(), - z.object({ kind: z.literal("index"), index: z.number().int().nonnegative() }).strict(), - ]) - .optional(), + window: targetDimensionSelectorSchema.optional(), + tab: targetDimensionSelectorSchema.optional(), }) .strict(); export type TargetSelector = z.infer; +export const windowTargetSelectorSchema = z + .object({ + window: targetDimensionSelectorSchema, + }) + .strict(); +export type WindowTargetSelector = z.infer; + export const resolvedTargetSchema = z .object({ windowId: z.number().int(), @@ -77,7 +78,7 @@ export const tabsListResultSchema = z.object({ export const tabNewParamsSchema = z .object({ url: z.string().min(1).optional(), - target: targetSelectorSchema.optional(), + target: windowTargetSelectorSchema.optional(), }) .strict(); export const tabTargetParamsSchema = z @@ -104,7 +105,7 @@ export const windowNewParamsSchema = z .strict(); export const windowTargetParamsSchema = z .object({ - target: targetSelectorSchema, + target: windowTargetSelectorSchema, }) .strict(); export const windowNewResultSchema = z.object({ diff --git a/skills/firefox-cli/SKILL.md b/skills/firefox-cli/SKILL.md index 1a3c930..847ea7b 100644 --- a/skills/firefox-cli/SKILL.md +++ b/skills/firefox-cli/SKILL.md @@ -53,7 +53,17 @@ Then use the command-specific help for the next operation instead of relying on By default, commands target Firefox's active tab/window at command resolution time. -Use `firefox-cli tab -h` and `firefox-cli window -h` to discover targeting options. Use tab/window indexes and `id:` values from `firefox-cli tab` and `firefox-cli window` output when the active target is not enough. +Use only the `--window` and `--tab` flags advertised by each command. Values are `active`, an index from `firefox-cli tab` or `firefox-cli window`, or `id:`. + +For isolated work, create a window and target every follow-up command explicitly: + +```bash +firefox-cli window new +firefox-cli open --window id: https://example.com +firefox-cli snapshot --window id: -i +``` + +`window select` changes Firefox focus only; it does not establish a durable CLI target. Screenshots may activate their selected tab/window. ## Setup State ```bash From dbb33c20a990d7b0f69034bfa580bbb13323393d Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:45:32 +0200 Subject: [PATCH 2/5] Preserve batch selector errors --- packages/cli/src/runner.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/src/runner.ts b/packages/cli/src/runner.ts index cebb3f9..ba27fd3 100644 --- a/packages/cli/src/runner.ts +++ b/packages/cli/src/runner.ts @@ -107,6 +107,10 @@ async function buildRequestForBinding( async function buildRequestForArgv(argv: readonly string[], dependencies: CliDependencies, context: CliRequestBuildContext): Promise { const binding = findCliRouteBindingForArgv(argv); if (binding === undefined) { + const command = argv[0]; + if (command === "setup" || command === "doctor" || command === "unpair") { + rejectTargetSelectorOptions(argv.slice(1), command); + } throw new InvalidBatchArgvCommandError(); } From 4e27cbccd84876348f0feed2ea27bb83f890355f Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:47:16 +0200 Subject: [PATCH 3/5] Cover batch selector diagnostics --- packages/cli/src/cli-target-contract.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/cli/src/cli-target-contract.test.ts b/packages/cli/src/cli-target-contract.test.ts index 94d4ec5..b418996 100644 --- a/packages/cli/src/cli-target-contract.test.ts +++ b/packages/cli/src/cli-target-contract.test.ts @@ -53,6 +53,16 @@ describe("CLI route target contracts", () => { }); }); + 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; From 3c58062c68e86e9614038a6c44ddf59dc54db916 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:48:43 +0200 Subject: [PATCH 4/5] Derive selector contracts from params --- packages/protocol/src/metadata.ts | 1 - packages/protocol/src/registry/browsing.ts | 4 ---- packages/protocol/src/registry/define.ts | 10 +++++++--- packages/protocol/src/registry/registry.test.ts | 5 ----- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/protocol/src/metadata.ts b/packages/protocol/src/metadata.ts index 8369b0a..ec3ab0a 100644 --- a/packages/protocol/src/metadata.ts +++ b/packages/protocol/src/metadata.ts @@ -73,7 +73,6 @@ export interface CommandBatchMetadata { } export interface CommandSchemaMetadata { - readonly targetSelectorSchema?: z.ZodType; readonly owner: CommandOwner; readonly target: CommandTargetPolicy; readonly content: CommandContentPolicy; diff --git a/packages/protocol/src/registry/browsing.ts b/packages/protocol/src/registry/browsing.ts index 3bbb105..022a478 100644 --- a/packages/protocol/src/registry/browsing.ts +++ b/packages/protocol/src/registry/browsing.ts @@ -13,7 +13,6 @@ import { windowNewResultSchema, windowSelectResultSchema, windowTargetParamsSchema, - windowTargetSelectorSchema, windowsListParamsSchema, windowsListResultSchema, } from "../target.js"; @@ -33,7 +32,6 @@ export const browsingCommandEntries = defineCommandEntries({ cliRoutes: [{ id: "tab.list", path: ["tab"], batch: true, selectorDimensions: "both" }], }, "tab.new": { - targetSelectorSchema: windowTargetSelectorSchema, params: tabNewParamsSchema, result: tabNewResultSchema, status: "mvp", @@ -94,7 +92,6 @@ export const browsingCommandEntries = defineCommandEntries({ cliRoutes: [{ id: "window.new", path: ["window", "new"], batch: true, selectorDimensions: "neither" }], }, "window.select": { - targetSelectorSchema: windowTargetSelectorSchema, params: windowTargetParamsSchema, result: windowSelectResultSchema, status: "mvp", @@ -107,7 +104,6 @@ export const browsingCommandEntries = defineCommandEntries({ cliRoutes: [{ id: "window.select", path: ["window", "select"], batch: true, selectorDimensions: "window" }], }, "window.close": { - targetSelectorSchema: windowTargetSelectorSchema, params: windowTargetParamsSchema, result: windowCloseResultSchema, status: "mvp", diff --git a/packages/protocol/src/registry/define.ts b/packages/protocol/src/registry/define.ts index 1cdc6b6..54e2d29 100644 --- a/packages/protocol/src/registry/define.ts +++ b/packages/protocol/src/registry/define.ts @@ -1,5 +1,6 @@ +import { z } from "zod"; + import type { CliRouteSelectorDimensions, CommandSchemaEntry } from "../metadata.js"; -import { targetSelectorSchema } from "../target.js"; export type CommandRegistryFragment = Readonly>; @@ -44,10 +45,13 @@ function assertCliRouteSelectorDimensions(registry: Readonly { .strict(), result: z.object({ ok: z.literal(true) }).strict(), status: "mvp", - targetSelectorSchema: z - .object({ - window: z.object({ kind: z.literal("active") }).strict(), - }) - .strict(), owner: "extension", target: "optional", content: "never", From abfaa2ef798ba23bb91f30fbbd7bc4b93b4ad6fc Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:51:07 +0200 Subject: [PATCH 5/5] Type protocol selector invariant --- packages/protocol/src/metadata.ts | 2 +- packages/protocol/src/registry/define.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/protocol/src/metadata.ts b/packages/protocol/src/metadata.ts index ec3ab0a..d69ebe4 100644 --- a/packages/protocol/src/metadata.ts +++ b/packages/protocol/src/metadata.ts @@ -86,7 +86,7 @@ export interface CommandSchemaMetadata { } export type CommandSchemaEntry = CommandSchemaMetadata & { - readonly params: z.ZodType; + readonly params: z.ZodObject; readonly result: z.ZodType; readonly status: CapabilityStatus; }; diff --git a/packages/protocol/src/registry/define.ts b/packages/protocol/src/registry/define.ts index 54e2d29..c3c47e9 100644 --- a/packages/protocol/src/registry/define.ts +++ b/packages/protocol/src/registry/define.ts @@ -48,10 +48,8 @@ function selectorDimensionsAcceptedByCommand(schema: CommandSchemaEntry): CliRou if (!(schema.params instanceof z.ZodObject)) { return "neither"; } - // Zod's public object shape getter is untyped in v4 despite the runtime schema boundary. - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - const selectorSchema = schema.params.def.shape.target; - if (selectorSchema === undefined) return "neither"; + const selectorSchema = schema.params.shape.target; + if (!isZodType(selectorSchema)) return "neither"; const acceptsWindow = selectorSchema.safeParse({ window: { kind: "active" } }).success; const acceptsTab = selectorSchema.safeParse({ tab: { kind: "active" } }).success; if (acceptsWindow && acceptsTab) { @@ -65,3 +63,7 @@ function selectorDimensionsAcceptedByCommand(schema: CommandSchemaEntry): CliRou } return "neither"; } + +function isZodType(value: unknown): value is z.ZodType { + return value instanceof z.ZodType; +}