From 98e7bd000faaf5b6ba46c65dc46cce41008c3703 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Mon, 13 Jul 2026 05:41:20 +0200 Subject: [PATCH 1/8] 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/8] 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/8] 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/8] 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/8] 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; +} From 1c50378a2479e2039ef832b3423d2383b9d38178 Mon Sep 17 00:00:00 2001 From: "Nek.12" Date: Thu, 16 Jul 2026 11:55:47 +0200 Subject: [PATCH 6/8] Delete .kent/plans/FFCLI-1.md --- .kent/plans/FFCLI-1.md | 77 ------------------------------------------ 1 file changed, 77 deletions(-) delete mode 100644 .kent/plans/FFCLI-1.md diff --git a/.kent/plans/FFCLI-1.md b/.kent/plans/FFCLI-1.md deleted file mode 100644 index 1b16817..0000000 --- a/.kent/plans/FFCLI-1.md +++ /dev/null @@ -1,77 +0,0 @@ -## 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) From d766b1534b623643ff3c1952fbdb87d5ee8fef27 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Thu, 16 Jul 2026 12:51:32 +0200 Subject: [PATCH 7/8] Harden ambiguous Firefox targeting --- docs/all-commands-qa.md | 8 +- docs/architecture.md | 2 +- docs/commands.md | 14 +- docs/firefox-cli-spec.md | 25 ++-- packages/cli/src/cli-batch-core.test.ts | 38 ++++- packages/cli/src/cli-help.test.ts | 14 +- packages/cli/src/cli-tabs-targets.test.ts | 7 +- packages/cli/src/cli-target-contract.test.ts | 4 +- packages/cli/src/cli-target-safety.test.ts | 70 ++++++++++ packages/cli/src/commands/batch.ts | 45 +----- packages/cli/src/commands/tabs-windows.ts | 12 +- packages/cli/src/format.ts | 28 +++- packages/cli/src/help.ts | 18 ++- packages/cli/src/parse.ts | 9 +- packages/cli/src/route-registry.ts | 6 +- .../src/background-controller-test-cases.ts | 4 - .../extension/src/browser-command/batch.ts | 105 ++++++++++++-- .../browser-command/target-context.test.ts | 6 +- .../src/browser-command/target-context.ts | 6 +- .../extension/src/browser-command/targets.ts | 36 +++++ .../src/browser-commands-batch.test.ts | 57 ++++++++ .../src/browser-commands-targets.test.ts | 131 ++++++++++++++++++ .../src/browser-commands-test-cases.ts | 12 +- .../src/browser-handlers/navigation.ts | 2 +- .../src/browser-handlers/phase8-browser.ts | 11 +- .../src/browser-handlers/tabs-windows.ts | 11 +- packages/protocol/src/batch.ts | 19 +-- packages/protocol/src/browser/output.ts | 4 +- packages/protocol/src/command-validation.ts | 33 +---- packages/protocol/src/metadata.ts | 1 - .../src/protocol-metadata-behavior.test.ts | 11 +- .../protocol/src/protocol-request.test.ts | 33 ++++- .../protocol/src/protocol-response-6.test.ts | 15 +- .../protocol/src/protocol-test-support.ts | 4 +- packages/protocol/src/registry/browsing.ts | 22 +-- packages/protocol/src/registry/define.ts | 4 +- packages/protocol/src/registry/index.ts | 43 +----- packages/protocol/src/registry/phase8.ts | 2 +- packages/protocol/src/target.ts | 6 +- skills/firefox-cli/SKILL.md | 4 +- 40 files changed, 619 insertions(+), 263 deletions(-) create mode 100644 packages/cli/src/cli-target-safety.test.ts diff --git a/docs/all-commands-qa.md b/docs/all-commands-qa.md index 915c430..276072a 100644 --- a/docs/all-commands-qa.md +++ b/docs/all-commands-qa.md @@ -40,10 +40,10 @@ Create `UPLOAD_FILE` with any small text payload. Capture IDs from JSON output w - [ ] Run `$CLI connect`; expect an already-approved rejection that identifies the extension instance. - [ ] Run `$CLI window new "$BASE" --json`; save `WINDOW` and `TAB`. - [ ] Run `$CLI window --json`; expect `WINDOW` in the window list. -- [ ] Run `$CLI window select "id:$WINDOW" --json`; expect `WINDOW` to be selected. +- [ ] Run `$CLI window select "id:$WINDOW" --json`; expect `WINDOW` to be brought forward without establishing a durable target. - [ ] Run `$CLI tab --window "id:$WINDOW" --json`; expect `TAB` in the tab list. - [ ] Run `$CLI tab new "${BASE}?tab2" --window "id:$WINDOW" --json`; save `TAB2`. -- [ ] Run `$CLI tab select "id:$TAB2" --json`; expect `TAB2` to be active. +- [ ] Run `$CLI tab select "id:$TAB2" --json`; expect `TAB2` to be brought forward without establishing a durable target. - [ ] Run `$CLI open "${BASE}?open" --tab "id:$TAB2" --json`; expect `TAB2` to navigate. - [ ] Run `$CLI open --new-tab "${BASE}?tab3" --window "id:$WINDOW" --json`; save `TAB3`. - [ ] Run `$CLI back --tab "id:$TAB2" --json`; expect navigation success. @@ -132,12 +132,12 @@ Create `UPLOAD_FILE` with any small text payload. Capture IDs from JSON output w - [ ] Run `$CLI errors clear --tab "id:$TAB2" --json`; expect success. - [ ] Run `$CLI errors list --tab "id:$TAB2" --json`; expect an error-list result. - [ ] Run `$CLI highlight "#highlight-target" --tab "id:$TAB2" --json`; expect success. -- [ ] Run `$CLI set viewport 1000 700 --tab "id:$TAB2" --json`; expect window dimensions in the response. +- [ ] Run `$CLI set viewport 1000 700 --window "id:$WINDOW" --json`; expect window dimensions in the response. - [ ] Run `$CLI diff title "firefox-cli disposable E2E" --tab "id:$TAB2" --json`; expect `"matches": true`. - [ ] Run `$CLI diff url "${BASE}?open" --tab "id:$TAB2" --json`; expect `"matches": true`. - [ ] Run `$CLI batch '[["fill","#email","batch@example.test"],["click","#submit"],["wait","--text","Submitted batch@example.test","--timeout","5000"],["get","text","#status"]]' --tab "id:$TAB2" --json`; expect all batch steps to pass. - [ ] Run `printf '[["get","title"]]' | $CLI batch --stdin --tab "id:$TAB2" --json`; expect the stdin batch step to pass. -- [ ] Run `$CLI pdf "$PDF" --tab "id:$TAB2" --json`; expect `UNSUPPORTED_CAPABILITY`. +- [ ] Run `$CLI pdf "$PDF" --json`; expect `UNSUPPORTED_CAPABILITY`. - [ ] Run `$CLI close`; expect `UNSUPPORTED_CAPABILITY`. - [ ] Run `$CLI quit`; expect `UNSUPPORTED_CAPABILITY`. - [ ] Run `$CLI exit`; expect `UNSUPPORTED_CAPABILITY`. diff --git a/docs/architecture.md b/docs/architecture.md index 068da58..c2863e5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -21,7 +21,7 @@ The first approval request creates a pair token. The extension stores the token ## Targeting -Commands resolve targets inside the extension. Default target selection uses the active tab/window. Explicit targets use the indexes printed by `tab`/`window` or Firefox IDs with `id:`. +Commands resolve targets inside the extension. Omitted selectors are accepted only when the required window or tab is unique. Explicit selectors use `active`, the indexes printed by `tab`/`window`, or Firefox IDs with `id:`; a tab ID also identifies its owning window. Private windows are guarded at the extension command boundary. Read-only listings can report private tabs/windows; commands that would mutate private browsing state are rejected. diff --git a/docs/commands.md b/docs/commands.md index 4f03bd5..9fbb7d7 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -17,7 +17,9 @@ Commands advertise their supported selectors in `-h`. Selector values are: 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. +Omitted selectors are accepted only when the required browser surface is unique. Multiple windows require `--window id:` or explicit `--window active`; multiple tabs in a selected window require `--tab id:` or explicit `--tab active`. A globally unique tab ID also identifies its owning window. + +Page-targeted commands accept both selectors. `tab`, `tab new`, `window select`, `window close`, and `set viewport` accept `--window` only. `open --new-tab` resolves only a window. 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`. @@ -36,13 +38,13 @@ Private windows are listed and readable. Mutating commands against private windo | Command | Behavior | | --- | --- | -| `firefox-cli tab [--json]` | List tabs. | +| `firefox-cli tab [--window target] [--json]` | List tabs in the selected or only window. | | `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 select [target] [--json]` | Bring a tab forward to the user; later commands still require explicit selectors when ambiguous. | | `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] [--window target] [--json]` | Focus a window; this does not set a durable default target. | +| `firefox-cli window select [target] [--window target] [--json]` | Bring a window forward to the user; later commands still require explicit selectors when ambiguous. | | `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. | @@ -135,7 +137,7 @@ firefox-cli batch | --stdin [--bail] [--timeout ms] [--max-output bytes] `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. +`batch` accepts an array of protocol command objects or CLI argv arrays. Steps run serially against a once-resolved default target; window-only batches do not require a tab. With `--bail`, execution stops after the first failed step; without it, later steps continue and the batch result reports failures. Example: @@ -162,7 +164,7 @@ Example: | `firefox-cli console|errors list|clear [--json]` | List or clear page console/error capture buffers. | | `firefox-cli highlight [--json]` | Outline an element. | | `firefox-cli notify [--id id] [message...] [--json]` | Show a native Firefox notification. | -| `firefox-cli set viewport <width> <height> [--json]` | Request a target browser window resize and report Firefox's observed window dimensions. Tiling/window-manager rules can prevent the requested size from taking effect. | +| `firefox-cli set viewport <width> <height> [--window target] [--json]` | Request a target browser window resize and report Firefox's observed window dimensions. Tiling/window-manager rules can prevent the requested size from taking effect. | | `firefox-cli diff url|title|snapshot <expected> [--json]` | Compare URL, title, or snapshot text with an expected value. | | `firefox-cli pdf <path> [--json]` | Returns `UNSUPPORTED_CAPABILITY`; Firefox saves PDFs through a browser dialog rather than a requested CLI path. | diff --git a/docs/firefox-cli-spec.md b/docs/firefox-cli-spec.md index 4aa9b6d..c6e270f 100644 --- a/docs/firefox-cli-spec.md +++ b/docs/firefox-cli-spec.md @@ -211,17 +211,20 @@ Startup handshake: Target resolution is owned by the extension. -- `active` means Firefox's active tab in the active/focused normal window at command resolution time. +- `active` is an explicit selector. `--window active` chooses Firefox's focused normal window; `--tab active` chooses the active tab within the selected window. - Window and tab IDs use Firefox IDs in JSON output. - Human-facing indexes are derived from deterministic `browser.windows.getAll({ populate: true })` ordering: focused window first, then remaining windows by Firefox window ID; tabs by index within each window. - Private windows are reported but commands return `UNSUPPORTED_CAPABILITY` unless the extension has private browsing permission and the command is explicitly allowed there. - Container tab metadata should be included when Firefox exposes it, but no persistent container/session model is added. - Each command snapshots its resolved target before execution to avoid active-tab races. - Selector values are `active`, a non-negative listing index, or `id:<non-negative Firefox ID>`. A route supports neither selector, `--window` only, `--tab` only, or both as advertised by its CLI help; unsupported selector flags fail before dispatch. -- Page-targeted routes support both dimensions. `tab new`, `window select`, and `window close` support `--window` only. Targetless routes support neither. -- `window select` changes Firefox focus only. It does not establish a durable CLI target; use an explicit selector on each isolated follow-up command. +- An omitted window selector is accepted only when exactly one normal window exists. A globally unique `--tab id:<id>` also identifies its owning window. Other tab selectors remain window-relative. +- A tab-dependent command accepts an omitted tab selector only when the selected window has exactly one tab. Pass `--tab active` explicitly to choose Firefox's active tab when several tabs exist. +- Ambiguous omission returns `INVALID_TARGET` with `Ambiguous window` or `Ambiguous tab` guidance before browser side effects. +- Page-targeted routes support both dimensions. `tab`, `tab new`, `window select`, `window close`, and `set viewport` are window-scoped. `open --new-tab` resolves only a window. Targetless routes support neither. +- `tab select` and `window select` bring a browser surface forward to the user. They do not establish durable CLI target state or remove the requirement to pass selectors explicitly to later target-dependent commands. -`open <url>` navigates the resolved active tab to match `agent-browser`. Use `tab new [url]` or `open --new-tab <url>` to create a new tab. +`open <url>` navigates the resolved target tab to match `agent-browser`. Use `tab new [url]` or `open --new-tab <url>` to create a new tab. ## Snapshot And Ref Model @@ -292,7 +295,7 @@ Start with visible-tab screenshots. `batch` executes a serialized command transaction. - Input is a JSON array of argv arrays or command objects. -- Default target is resolved once at batch start unless a step overrides it. +- The required default window and/or tab is resolved once at batch start unless a step overrides it. Window-only batches do not require or inject a tab selector. - Steps share ref registry entries created earlier in the batch. - `--bail` stops at the first failed step. - Output is an ordered result array with per-step success/error, diagnostics, and final exit status. @@ -414,12 +417,12 @@ Unsupported unless Firefox provides an equivalent: ## MVP Command Contracts -Every MVP command returns `{ target, diagnostics }` in JSON output. `target` includes resolved window ID, tab ID, URL when available, and title when available. Text output may omit fields that are not useful for agents. +Target-dependent MVP command results include resolved window/tab metadata where their result schema exposes it. Text output may omit fields that are not useful for agents. Navigation: -- `open <url> [--new-tab]`: params are normalized URL and new-tab flag. Result is final target, URL, navigation status, and load state when known. Errors include invalid URL, no active tab, permission denied, navigation timeout, and unsupported restricted page. -- `back`, `forward`, `reload`: params are empty except target and timeout. Result is final URL and load state when known. Errors include no history entry, no active tab, navigation timeout, and restricted page. +- `open <url> [--new-tab]`: params are normalized URL and new-tab flag. Result is final target, URL, navigation status, and load state when known. Errors include invalid URL, ambiguous or missing target, permission denied, navigation timeout, and unsupported restricted page. +- `back`, `forward`, `reload`: params are empty except target and timeout. Result is final URL and load state when known. Errors include no history entry, ambiguous or missing target, navigation timeout, and restricted page. Snapshot: @@ -456,14 +459,14 @@ Waits: Tabs and windows: -- `tab`: lists tabs for the resolved or active window. Result includes ID, index, active flag, title, URL when permitted, window ID, private flag, and container metadata when available. +- `tab`: lists tabs for the selected or only window. Result includes ID, index, active flag, title, URL when permitted, window ID, private flag, and container metadata when available. - `tab new [url]`: creates a new tab. Result is created tab target and URL when provided. - `tab close [id|index|active]`: closes a tab. Result is closed tab ID and next active tab when known. -- `tab select <id|index>`: activates a tab. Result is selected target. +- `tab select [id|index|active]`: brings a tab forward to the user. Result is the selected target; later target-dependent commands still require explicit selectors when their target is ambiguous. - `window`: lists normal windows and their active tabs. Result includes Firefox window IDs, focus/active flags, bounds when available, and tab count. - `window new [url]`: creates a new window. Result is created window ID and active tab ID. - `window close <id|active>`: closes a window. Result is closed window ID. -- `window select <id|index>`: focuses a window. Result includes refreshed focus metadata and its active tab; it does not set a durable default target. +- `window select [id|index|active]`: brings a window forward to the user. Result includes refreshed focus metadata and its active tab; later target-dependent commands still require explicit selectors when their target is ambiguous. Screenshots: diff --git a/packages/cli/src/cli-batch-core.test.ts b/packages/cli/src/cli-batch-core.test.ts index ffa80ce..b29d56e 100644 --- a/packages/cli/src/cli-batch-core.test.ts +++ b/packages/cli/src/cli-batch-core.test.ts @@ -1,7 +1,7 @@ -import { createOkResponse } from "@firefox-cli/protocol"; +import { createErrorResponse, createOkResponse, type RequestEnvelope } from "@firefox-cli/protocol"; import { describe, expect, it } from "vitest"; -import { runCli } from "./index.js"; import { actionElement, baseDependencies } from "./cli-test-support.js"; +import { runCli } from "./index.js"; describe("runCli batch core", () => { it("runs batch command objects with bail, target, timeout, and result limits", async () => { @@ -123,6 +123,40 @@ describe("runCli batch core", () => { }); }); + it("preserves omitted selectors in window-only argv steps", async () => { + let sentRequest: RequestEnvelope | undefined; + + await runCli( + [ + "batch", + JSON.stringify([ + ["tab", "new"], + ["window", "select"], + ["window", "close"], + ]), + ], + { + ...baseDependencies(), + sendRequest: async (request) => { + sentRequest = request; + return createErrorResponse(request.id, { + code: "NATIVE_HOST_UNAVAILABLE", + message: "Expected test transport failure.", + }); + }, + }, + ); + + expect(sentRequest?.command).toBe("batch"); + expect(sentRequest?.params).toMatchObject({ + steps: [ + { command: "tab.new", params: {} }, + { command: "window.select", params: {} }, + { command: "window.close", params: {} }, + ], + }); + }); + it("preserves option-like argv payloads in direct and batch commands", async () => { const dialogOutput = await runCli(["dialog", "accept", "--proceed"], { ...baseDependencies(), diff --git a/packages/cli/src/cli-help.test.ts b/packages/cli/src/cli-help.test.ts index cdb62d6..0ab6bdd 100644 --- a/packages/cli/src/cli-help.test.ts +++ b/packages/cli/src/cli-help.test.ts @@ -23,7 +23,7 @@ describe("CLI help", () => { expect(output.exitCode).toBe(0); expect(output.stdout).toContain("Tabs, windows, and navigation"); expect(output.stdout).toContain("firefox-cli tab [--json]"); - expect(output.stdout).toContain("List tabs with indexes"); + expect(output.stdout).toContain("List tabs for the selected or only Firefox window"); expect(output.stdout).toContain("firefox-cli open [--new-tab] <url> [--json]"); }); @@ -37,6 +37,18 @@ describe("CLI help", () => { expect(output.stdout).toContain("firefox-cli snapshot -i"); }); + it.each([ + ["tab", "tab", "firefox-cli tab select"], + ["window", "window", "firefox-cli window select"], + ] as const)("explains that %s selection only brings the surface forward to the user", async (command, surface, usage) => { + const output = await runCli([command, "select", "--help"], baseDependencies()); + + expect(output.exitCode).toBe(0); + expect(output.stdout).toContain(usage); + expect(output.stdout).toContain(`Bring a ${surface} forward to the user`); + expect(output.stdout).toContain("This does not absolve you from passing `--window`/`--tab` explicitly to every later target-dependent command."); + }); + it("renders wait examples accepted by the wait parser", async () => { const output = await runCli(["wait", "--help"], baseDependencies()); diff --git a/packages/cli/src/cli-tabs-targets.test.ts b/packages/cli/src/cli-tabs-targets.test.ts index c019ff3..9292157 100644 --- a/packages/cli/src/cli-tabs-targets.test.ts +++ b/packages/cli/src/cli-tabs-targets.test.ts @@ -1,10 +1,10 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { createTempDir } from "@firefox-cli/test-support"; import { createErrorResponse, createOkResponse } from "@firefox-cli/protocol"; +import { createTempDir } from "@firefox-cli/test-support"; import { describe, expect, it } from "vitest"; -import { runCli } from "./index.js"; import { actionElement, baseDependencies, targetSummary } from "./cli-test-support.js"; +import { runCli } from "./index.js"; import { findCliRouteBindingForArgv } from "./route-registry.js"; describe("runCli tabs and targets", () => { @@ -146,7 +146,8 @@ describe("runCli tabs and targets", () => { expect(output).toEqual({ exitCode: 0, - stdout: "w7 t42 [0] Example https://example.com/\n", + stdout: + "w7 t42 [0] Example https://example.com/\nBrought this tab forward to the user. This does not absolve you from passing `--window`/`--tab` explicitly to every later target-dependent command.\n", stderr: "", }); }); diff --git a/packages/cli/src/cli-target-contract.test.ts b/packages/cli/src/cli-target-contract.test.ts index b418996..a82eb32 100644 --- a/packages/cli/src/cli-target-contract.test.ts +++ b/packages/cli/src/cli-target-contract.test.ts @@ -1,8 +1,8 @@ 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"; +import { runCli } from "./index.js"; describe("CLI route target contracts", () => { it("accepts selector dimensions declared by each protocol route", () => { @@ -21,9 +21,11 @@ describe("CLI route target contracts", () => { }); it.each([ + ["tab list", ["tab", "--tab", "id:42"]], ["tab new", ["tab", "new", "--tab", "id:42"]], ["window select", ["window", "select", "--tab", "id:42"]], ["window close", ["window", "close", "--tab", "id:42"]], + ["set viewport", ["set", "viewport", "1200", "800", "--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) => { diff --git a/packages/cli/src/cli-target-safety.test.ts b/packages/cli/src/cli-target-safety.test.ts new file mode 100644 index 0000000..e09b783 --- /dev/null +++ b/packages/cli/src/cli-target-safety.test.ts @@ -0,0 +1,70 @@ +import { createErrorResponse, type RequestEnvelope } from "@firefox-cli/protocol"; +import { describe, expect, it } from "vitest"; +import { baseDependencies, targetSummary } from "./cli-test-support.js"; +import { runCli } from "./index.js"; + +describe("runCli target safety", () => { + it("preserves an omitted tab selector for extension ambiguity checks", async () => { + let sentRequest: RequestEnvelope | undefined; + + await runCli(["tab", "select"], { + ...baseDependencies(), + sendRequest: async (request) => { + sentRequest = request; + return createErrorResponse(request.id, { + code: "INVALID_TARGET", + message: "Expected ambiguity test response.", + }); + }, + }); + + expect(sentRequest?.command).toBe("tab.select"); + expect(sentRequest?.params).toEqual({}); + }); + + it("preserves an omitted window selector for extension ambiguity checks", async () => { + let sentRequest: RequestEnvelope | undefined; + + await runCli(["window", "select"], { + ...baseDependencies(), + sendRequest: async (request) => { + sentRequest = request; + return createErrorResponse(request.id, { + code: "INVALID_TARGET", + message: "Expected ambiguity test response.", + }); + }, + }); + + expect(sentRequest?.command).toBe("window.select"); + expect(sentRequest?.params).toEqual({}); + }); + + it("warns that selecting a window only brings it forward to the user", async () => { + const output = await runCli(["window", "select", "id:7"], { + ...baseDependencies(), + sendRequest: async (request) => ({ + protocolVersion: request.protocolVersion, + id: request.id, + ok: true, + result: { + window: { + id: 7, + index: 0, + focused: true, + activeTabId: 42, + tabCount: 1, + }, + target: targetSummary(), + }, + }), + }); + + expect(output).toEqual({ + exitCode: 0, + stdout: + "w7 [0]\nBrought this window forward to the user. This does not absolve you from passing `--window`/`--tab` explicitly to every later target-dependent command.\n", + stderr: "", + }); + }); +}); diff --git a/packages/cli/src/commands/batch.ts b/packages/cli/src/commands/batch.ts index 0213f06..98c2752 100644 --- a/packages/cli/src/commands/batch.ts +++ b/packages/cli/src/commands/batch.ts @@ -1,19 +1,10 @@ -import { - batchParamsSchema, - commandAcceptsProtocolBatchDefaultTarget, - isBatchableCommandId, - MAX_UPLOAD_TOTAL_BYTES, - type BatchParams, - type BatchStep, - type CommandId, - type RequestEnvelope, -} from "@firefox-cli/protocol"; +import { type BatchParams, type BatchStep, batchParamsSchema, isBatchableCommandId, MAX_UPLOAD_TOTAL_BYTES, type RequestEnvelope } from "@firefox-cli/protocol"; import { parseCliRouteArgsForRoute } from "../argv-contracts.js"; import { readProcessStdin } from "../default-dependencies.js"; import { getOptionValue, hasOption, isRecord, optionalTarget, parsePositiveIntegerValue, parseTargetOptions } from "../parse.js"; import { createValidatedRequest } from "../protocol-validation.js"; +import { type CliDependencies, type CliRequestBuildContext, CliUsageError, InvalidBatchArgvCommandError } from "../types.js"; import { createUploadBudget, parseUploadArguments, statUploadFiles, uploadTotalTooLarge } from "../upload.js"; -import { CliUsageError, InvalidBatchArgvCommandError, type CliDependencies, type CliRequestBuildContext } from "../types.js"; interface ParsedBatchArguments { readonly optionArgs: readonly string[]; @@ -183,40 +174,10 @@ async function batchStepFromArgv(argv: readonly string[], index: number, depende return { command: request.command, - params: stripImplicitBatchTarget(request.command, request.params, argv), + params: request.params, }; } function batchArgvReadsStdin(argv: readonly string[]): boolean { return argv[0] === "eval" && argv.includes("--stdin"); } - -function stripImplicitBatchTarget(command: CommandId, params: unknown, argv: readonly string[]): unknown { - if (!isImplicitBatchDefaultTargetCommand(command) || !isRecord(params)) { - return params; - } - - if (hasExplicitTargetInBatchArgv(command, argv)) { - return params; - } - - return Object.fromEntries(Object.entries(params).filter(([key]) => key !== "target")); -} - -function isImplicitBatchDefaultTargetCommand(command: CommandId): boolean { - return commandAcceptsProtocolBatchDefaultTarget(command); -} - -function hasExplicitTargetInBatchArgv(command: CommandId, argv: readonly string[]): boolean { - if (command === "tab.select" || command === "tab.close") { - const positionals = parseCliRouteArgsForRoute(command, argv.slice(1)).positionals; - return positionals[1] !== undefined || hasOption(argv, "--tab") || hasOption(argv, "--window"); - } - - if (command === "window.select" || command === "window.close") { - const positionals = parseCliRouteArgsForRoute(command, argv.slice(1)).positionals; - return positionals[1] !== undefined || hasOption(argv, "--window"); - } - - return false; -} diff --git a/packages/cli/src/commands/tabs-windows.ts b/packages/cli/src/commands/tabs-windows.ts index 96220fb..e6ceb09 100644 --- a/packages/cli/src/commands/tabs-windows.ts +++ b/packages/cli/src/commands/tabs-windows.ts @@ -23,13 +23,15 @@ export function buildTabsRequest(argv: readonly string[]): RequestEnvelope { }); } if (subcommand === "select") { + const selectedTarget = mergeTarget(target, parseOptionalTabTarget(positional[1])); return createValidatedRequest("tab.select", { - target: mergeTarget(target, parseOptionalTabTarget(positional[1], target)), + ...optionalTarget(selectedTarget), }); } if (subcommand === "close") { + const selectedTarget = mergeTarget(target, parseOptionalTabTarget(positional[1])); return createValidatedRequest("tab.close", { - target: mergeTarget(target, parseOptionalTabTarget(positional[1], target)), + ...optionalTarget(selectedTarget), }); } return createValidatedRequest("tabs.list", { @@ -48,13 +50,15 @@ export function buildWindowsRequest(argv: readonly string[]): RequestEnvelope { }); } if (subcommand === "select") { + const selectedTarget = mergeTarget(target, parseOptionalWindowTarget(positional[1])); return createValidatedRequest("window.select", { - target: mergeTarget(target, parseOptionalWindowTarget(positional[1], target)), + ...optionalTarget(selectedTarget), }); } if (subcommand === "close") { + const selectedTarget = mergeTarget(target, parseOptionalWindowTarget(positional[1])); return createValidatedRequest("window.close", { - target: mergeTarget(target, parseOptionalWindowTarget(positional[1], target)), + ...optionalTarget(selectedTarget), }); } return createValidatedRequest("windows.list", {}); diff --git a/packages/cli/src/format.ts b/packages/cli/src/format.ts index 2dcce70..3c3b474 100644 --- a/packages/cli/src/format.ts +++ b/packages/cli/src/format.ts @@ -36,7 +36,7 @@ const formatTabList: CliResponseFormatter<"tabs.list"> = (response, json) => { return json ? ok(`${JSON.stringify(response.result, null, 2)}\n`) : ok(response.result.tabs.map(renderTabSummary).join("")); }; -const formatTabTarget: CliResponseFormatter<"tab.new" | "tab.select" | "open" | "back" | "forward" | "reload"> = (response, json) => { +const formatTabTarget: CliResponseFormatter<"tab.new" | "open" | "back" | "forward" | "reload"> = (response, json) => { if (!response.ok) { return error(formatProtocolError(response.error)); } @@ -44,6 +44,18 @@ const formatTabTarget: CliResponseFormatter<"tab.new" | "tab.select" | "open" | return json ? ok(`${JSON.stringify(response.result, null, 2)}\n`) : ok(`${renderTargetSummary(response.result.target)}\n`); }; +export const formatTabSelect: CliResponseFormatter<"tab.select"> = (response, json) => { + if (!response.ok) { + return error(formatProtocolError(response.error)); + } + + return json + ? ok(`${JSON.stringify(response.result, null, 2)}\n`) + : ok( + `${renderTargetSummary(response.result.target)}\nBrought this tab forward to the user. This does not absolve you from passing \`--window\`/\`--tab\` explicitly to every later target-dependent command.\n`, + ); +}; + const formatTabClose: CliResponseFormatter<"tab.close"> = (response, json) => { if (!response.ok) { return error(formatProtocolError(response.error)); @@ -71,7 +83,7 @@ const formatWindowList: CliResponseFormatter<"windows.list"> = (response, json) ); }; -const formatWindowTarget: CliResponseFormatter<"window.new" | "window.select"> = (response, json) => { +const formatWindowTarget: CliResponseFormatter<"window.new"> = (response, json) => { if (!response.ok) { return error(formatProtocolError(response.error)); } @@ -79,6 +91,18 @@ const formatWindowTarget: CliResponseFormatter<"window.new" | "window.select"> = return json ? ok(`${JSON.stringify(response.result, null, 2)}\n`) : ok(`w${String(response.result.window.id)} [${String(response.result.window.index)}]\n`); }; +export const formatWindowSelect: CliResponseFormatter<"window.select"> = (response, json) => { + if (!response.ok) { + return error(formatProtocolError(response.error)); + } + + return json + ? ok(`${JSON.stringify(response.result, null, 2)}\n`) + : ok( + `w${String(response.result.window.id)} [${String(response.result.window.index)}]\nBrought this window forward to the user. This does not absolve you from passing \`--window\`/\`--tab\` explicitly to every later target-dependent command.\n`, + ); +}; + const formatWindowClose: CliResponseFormatter<"window.close"> = (response, json) => { if (!response.ok) { return error(formatProtocolError(response.error)); diff --git a/packages/cli/src/help.ts b/packages/cli/src/help.ts index 6b5b726..4fd187f 100644 --- a/packages/cli/src/help.ts +++ b/packages/cli/src/help.ts @@ -18,19 +18,21 @@ interface HelpGroup { const routeHelpSpecs = { capabilities: helpSpec("List supported command families and browser capability metadata."), connect: helpSpec("Request Firefox control approval through a dedicated approval page."), - "tab.list": helpSpec("List tabs with indexes, ids, active state, titles, and URLs.", [ - "Use listed indexes with `--tab <index>` and ids with `--tab id:<id>`.", - ]), + "tab.list": helpSpec("List tabs for the selected or only Firefox window.", ["Use window indexes with `--window <index>` and ids with `--window id:<id>`."]), "tab.new": helpSpec("Open a new tab, optionally at a URL."), - "tab.select": helpSpec("Activate a tab by index, id, or URL substring."), + "tab.select": helpSpec("Bring a tab forward to the user by index, id, or URL substring.", [ + "This does not absolve you from passing `--window`/`--tab` explicitly to every later target-dependent command.", + ]), "tab.close": helpSpec("Close a tab by index, id, or URL substring."), "window.list": helpSpec("List Firefox windows with indexes, ids, focus state, and tab counts.", [ "Use listed indexes with `--window <index>` and ids with `--window id:<id>`.", ]), "window.new": helpSpec("Open a new Firefox window, optionally at a URL."), - "window.select": helpSpec("Focus a Firefox window by index or id; it does not establish a durable CLI target."), + "window.select": helpSpec("Bring a window forward to the user by index or id.", [ + "This does not absolve you from passing `--window`/`--tab` explicitly to every later target-dependent command.", + ]), "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."]), + open: helpSpec("Navigate a selected tab or create a new tab for a URL.", ["Use `--new-tab` when navigation must not replace an existing page."]), back: helpSpec("Go back in the target tab history."), forward: helpSpec("Go forward in the target tab history."), reload: helpSpec("Reload the target tab."), @@ -278,6 +280,7 @@ export function renderHelp(): string { " Use `--json` when another program or agent consumes results.", " Use `firefox-cli snapshot -i` before element actions to get stable `@ref` handles.", " Use only the `--tab` and `--window` options advertised by each command; values are active, index, or id:<id>.", + " Omitted selectors are accepted only when the required window or tab is unique; pass `active` explicitly to choose Firefox's focused/active surface.", " Use `firefox-cli <command> -h` for contextual command help.", "", ].join("\n"); @@ -308,7 +311,8 @@ function renderGroupHelp(group: HelpGroup): string { "", "Guidance:", " Add `--json` for machine-readable output.", - " Use only the selector options advertised by each command when the active target is not enough.", + " Omitted selectors are accepted only when the required window or tab is unique.", + " Pass `active`, an index, or `id:<id>` through only the selector options advertised by each command.", "", ].join("\n"); } diff --git a/packages/cli/src/parse.ts b/packages/cli/src/parse.ts index d353929..b3776ed 100644 --- a/packages/cli/src/parse.ts +++ b/packages/cli/src/parse.ts @@ -2,6 +2,7 @@ import type { TargetSelector } from "@firefox-cli/protocol"; import { cliArgumentOptionInventory } from "./argv-contracts.js"; import { readFlagValue } from "./parse-options.js"; import { CliUsageError } from "./types.js"; + export { getOptionValue, isOneOf, @@ -225,20 +226,20 @@ export function parseTargetValue(value: string | undefined): NonNullable<TargetS return prefix === "id" ? { kind: "id", id: parsed } : { kind: "index", index: parsed }; } -export function parseOptionalTabTarget(value: string | undefined, base: TargetSelector): TargetSelector { +export function parseOptionalTabTarget(value: string | undefined): TargetSelector { if (value !== undefined) { return { tab: parseTargetValue(value) }; } - return base.tab === undefined ? { tab: { kind: "active" } } : {}; + return {}; } -export function parseOptionalWindowTarget(value: string | undefined, base: TargetSelector): TargetSelector { +export function parseOptionalWindowTarget(value: string | undefined): TargetSelector { if (value !== undefined) { return { window: parseTargetValue(value) }; } - return base.window === undefined ? { window: { kind: "active" } } : {}; + return {}; } export function mergeTarget(base: TargetSelector, override: TargetSelector): TargetSelector { diff --git a/packages/cli/src/route-registry.ts b/packages/cli/src/route-registry.ts index 4536800..7bbcde7 100644 --- a/packages/cli/src/route-registry.ts +++ b/packages/cli/src/route-registry.ts @@ -38,7 +38,7 @@ import { buildPdfRequest, buildSetViewportRequest } from "./commands/phase8.js"; import { buildScreenshotRequest } from "./commands/screenshot.js"; import { buildTabsRequest, buildWindowsRequest } from "./commands/tabs-windows.js"; import { buildWaitRequest } from "./commands/wait.js"; -import { cliResponseFormatters, formatApprovalRequest } from "./format.js"; +import { cliResponseFormatters, formatApprovalRequest, formatTabSelect, formatWindowSelect } from "./format.js"; import { getPositionals } from "./parse.js"; import type { CliRequestBuilder, CliResponseFormatter, CliResponseFormatterKind, CliRouteBinding, CliRouteParserSpec } from "./types.js"; @@ -60,11 +60,11 @@ const routeFormatterSpecs = { capabilities: routeFormatter("capabilities", "capabilities", cliResponseFormatters.capabilities), "tab.list": routeFormatter("tabs.list", "tab-list", cliResponseFormatters["tab-list"]), "tab.new": routeFormatter("tab.new", "tab-target", cliResponseFormatters["tab-target"]), - "tab.select": routeFormatter("tab.select", "tab-target", cliResponseFormatters["tab-target"]), + "tab.select": routeFormatter("tab.select", "tab-target", formatTabSelect), "tab.close": routeFormatter("tab.close", "tab-close", cliResponseFormatters["tab-close"]), "window.list": routeFormatter("windows.list", "window-list", cliResponseFormatters["window-list"]), "window.new": routeFormatter("window.new", "window-target", cliResponseFormatters["window-target"]), - "window.select": routeFormatter("window.select", "window-target", cliResponseFormatters["window-target"]), + "window.select": routeFormatter("window.select", "window-target", formatWindowSelect), "window.close": routeFormatter("window.close", "window-close", cliResponseFormatters["window-close"]), open: routeFormatter("open", "tab-target", cliResponseFormatters["tab-target"]), back: routeFormatter("back", "tab-target", cliResponseFormatters["tab-target"]), diff --git a/packages/extension/src/background-controller-test-cases.ts b/packages/extension/src/background-controller-test-cases.ts index 0a0f05f..7a76197 100644 --- a/packages/extension/src/background-controller-test-cases.ts +++ b/packages/extension/src/background-controller-test-cases.ts @@ -134,10 +134,6 @@ export async function runCase04() { id: "request-1", ok: true, result: { - target: { - windowId: 7, - tabId: 42, - }, tabs: [ { id: 42, diff --git a/packages/extension/src/browser-command/batch.ts b/packages/extension/src/browser-command/batch.ts index 6219421..fd3f511 100644 --- a/packages/extension/src/browser-command/batch.ts +++ b/packages/extension/src/browser-command/batch.ts @@ -1,14 +1,16 @@ import { - MAX_BATCH_RESULT_BYTES, - MAX_SCREENSHOT_BYTES, + type BatchResult, + type BatchStepResult, + type CommandId, commandAcceptsBatchTimeout, commandAcceptsExtensionBatchDefaultTarget, createRequest, + getCommandTargetSelectorDimensions, isCommandId, + MAX_BATCH_RESULT_BYTES, + MAX_SCREENSHOT_BYTES, parseBatchStepResultAs, parseCommandParamsAs, - type BatchResult, - type BatchStepResult, type RequestEnvelope, type ResponseEnvelope, type ScreenshotResult, @@ -29,11 +31,7 @@ export async function executeBatch( const startedAt = Date.now(); const timeoutMs = command.params.timeoutMs; const maxResultBytes = command.params.maxResultBytes ?? MAX_BATCH_RESULT_BYTES; - const defaultTarget = (await targetContext.resolveTarget(command.params.target)).target; - const defaultSelector: TargetSelector = { - window: { kind: "id", id: defaultTarget.windowId }, - tab: { kind: "id", id: defaultTarget.tabId }, - }; + const defaultSelector = await resolveBatchDefaultSelector(command, targetContext); const steps: BatchStepResult[] = []; let totalScreenshotBytes = 0; @@ -85,7 +83,7 @@ function assertBatchHasTimeRemaining(timeoutMs: number | undefined, remainingMs: function createBatchStepRequest( command: RequestEnvelope<"batch">, index: number, - defaultSelector: TargetSelector, + defaultSelector: TargetSelector | undefined, remainingMs: number | undefined, ): RequestEnvelope | undefined { const step = command.params.steps[index]; @@ -129,18 +127,101 @@ function checkedScreenshotByteTotal(currentBytes: number, stepResult: BatchStepR return nextBytes; } -export function applyBatchStepDefaults(command: string, rawParams: unknown, defaultTarget: TargetSelector, remainingMs: number | undefined): unknown { +export function applyBatchStepDefaults( + command: string, + rawParams: unknown, + defaultTarget: TargetSelector | undefined, + remainingMs: number | undefined, +): unknown { if (!isRecord(rawParams)) { return rawParams; } return { ...rawParams, - ...(commandAcceptsExtensionBatchDefaultTarget(command) && rawParams.target === undefined ? { target: defaultTarget } : {}), + ...defaultTargetOverride(command, rawParams, defaultTarget), ...timeoutOverride(command, rawParams.timeoutMs, remainingMs), }; } +async function resolveBatchDefaultSelector(command: RequestEnvelope<"batch">, targetContext: BrowserTargetContext): Promise<TargetSelector | undefined> { + const resolution = command.params.steps.reduce<TargetResolution>( + (highest, step) => (isCommandId(step.command) ? highestTargetResolution(highest, requestTargetResolution(step.command, step.params)) : highest), + "none", + ); + if (resolution === "none") { + return undefined; + } + if (resolution === "window") { + const window = await targetContext.resolveTargetWindow(command.params.target); + return { window: { kind: "id", id: window.id } }; + } + + const target = (await targetContext.resolveTarget(command.params.target)).target; + return { + window: { kind: "id", id: target.windowId }, + tab: { kind: "id", id: target.tabId }, + }; +} + +type TargetResolution = "none" | "window" | "tab"; + +function requestTargetResolution(command: CommandId, params: unknown): TargetResolution { + const selectorDimensions = getCommandTargetSelectorDimensions(command); + if (selectorDimensions === "neither") { + return "none"; + } + if (selectorDimensions === "window") { + return "window"; + } + if (isWindowOnlyRequest(command, params)) { + return "window"; + } + if (isTargetlessRequest(command, params)) { + return "none"; + } + return "tab"; +} + +function isWindowOnlyRequest(command: CommandId, params: unknown): boolean { + return command === "open" && isRecord(params) && params.newTab === true; +} + +function isTargetlessRequest(command: CommandId, params: unknown): boolean { + if (!isRecord(params)) { + return false; + } + const targetlessWait = command === "wait" && (params.kind === "ms" || params.kind === "download"); + const targetlessClipboard = command === "clipboard" && (params.action === "read" || params.action === "write"); + return targetlessWait || targetlessClipboard; +} + +function highestTargetResolution(left: TargetResolution, right: TargetResolution): TargetResolution { + if (left === "tab" || right === "tab") { + return "tab"; + } + return left === "window" || right === "window" ? "window" : "none"; +} + +function defaultTargetOverride( + command: string, + params: Record<string, unknown>, + defaultTarget: TargetSelector | undefined, +): { readonly target?: TargetSelector } { + if (!commandAcceptsExtensionBatchDefaultTarget(command) || params.target !== undefined || defaultTarget === undefined || !isCommandId(command)) { + return {}; + } + + const resolution = requestTargetResolution(command, params); + if (resolution === "none") { + return {}; + } + if (resolution === "window") { + return defaultTarget.window === undefined ? {} : { target: { window: defaultTarget.window } }; + } + return { target: defaultTarget }; +} + function timeoutOverride(command: string, existingTimeout: unknown, remainingMs: number | undefined): { readonly timeoutMs?: number } { if (remainingMs === undefined || !commandAcceptsBatchTimeout(command)) { return {}; diff --git a/packages/extension/src/browser-command/target-context.test.ts b/packages/extension/src/browser-command/target-context.test.ts index 9836213..51c026e 100644 --- a/packages/extension/src/browser-command/target-context.test.ts +++ b/packages/extension/src/browser-command/target-context.test.ts @@ -20,13 +20,13 @@ describe("browser target context", () => { const adapter = new FakeBrowserAdapter([windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)])]); const context = createBrowserTargetContext(adapter); - await expect(context.resolveTarget(undefined)).resolves.toMatchObject({ tab: { id: 101 } }); + await expect(context.resolveTarget({ tab: { kind: "active" } })).resolves.toMatchObject({ tab: { id: 101 } }); await adapter.selectTab(102); - await expect(context.resolveTarget(undefined)).resolves.toMatchObject({ tab: { id: 101 } }); + await expect(context.resolveTarget({ tab: { kind: "active" } })).resolves.toMatchObject({ tab: { id: 101 } }); context.invalidate(); - await expect(context.resolveTarget(undefined)).resolves.toMatchObject({ tab: { id: 102 } }); + await expect(context.resolveTarget({ tab: { kind: "active" } })).resolves.toMatchObject({ tab: { id: 102 } }); expect(adapter.listWindowCalls).toBe(2); }); }); diff --git a/packages/extension/src/browser-command/target-context.ts b/packages/extension/src/browser-command/target-context.ts index fe06bfc..7443455 100644 --- a/packages/extension/src/browser-command/target-context.ts +++ b/packages/extension/src/browser-command/target-context.ts @@ -1,7 +1,7 @@ +import type { ResolvedTarget, TargetSelector } from "@firefox-cli/protocol"; import { withBrowserCommandDeadline } from "./deadline.js"; -import { findWindowById, resolveTarget, resolveWindow, toOrderedWindows } from "./targets.js"; +import { findWindowById, resolveTarget, resolveTargetWindow, resolveWindow, toOrderedWindows } from "./targets.js"; import type { BackgroundBrowserAdapter, OrderedWindow, ResolvedBrowserTarget } from "./types.js"; -import type { ResolvedTarget, TargetSelector } from "@firefox-cli/protocol"; interface DeadlineOptions { readonly deadlineMs?: number; @@ -15,6 +15,7 @@ type ResolveOptions = DeadlineOptions & { export interface BrowserTargetContext { getWindows(options?: DeadlineOptions): Promise<readonly OrderedWindow[]>; resolveTarget(selector: TargetSelector | undefined, options?: ResolveOptions): Promise<ResolvedBrowserTarget>; + resolveTargetWindow(selector: TargetSelector | undefined, options?: DeadlineOptions): Promise<OrderedWindow>; resolveWindow(selector: TargetSelector["window"] | undefined, options?: DeadlineOptions): Promise<OrderedWindow>; resolveFreshTarget(selector: TargetSelector, options?: ResolveOptions): Promise<ResolvedTarget>; findWindowById(windowId: number, options?: DeadlineOptions): Promise<OrderedWindow | undefined>; @@ -40,6 +41,7 @@ export function createBrowserTargetContext(adapter: BackgroundBrowserAdapter): B return { getWindows, resolveTarget: async (selector, options = {}) => resolveTarget(await getWindows(options), selector, resolveTargetOptions(options)), + resolveTargetWindow: async (selector, options = {}) => resolveTargetWindow(await getWindows(options), selector), resolveWindow: async (selector, options = {}) => resolveWindow(await getWindows(options), selector), resolveFreshTarget: async (selector, options = {}) => { windows = undefined; diff --git a/packages/extension/src/browser-command/targets.ts b/packages/extension/src/browser-command/targets.ts index 3634ad4..11af74f 100644 --- a/packages/extension/src/browser-command/targets.ts +++ b/packages/extension/src/browser-command/targets.ts @@ -34,6 +34,19 @@ export function resolveTarget( }; } +export function resolveTargetWindow(windows: readonly OrderedWindow[], selector: TargetSelector | undefined): OrderedWindow { + const tabById = findTargetedTabById(windows, selector); + if (tabById !== undefined) { + return tabById.window; + } + + const window = resolveWindow(windows, selector?.window); + if (selector?.tab !== undefined) { + resolveTab(window, selector.tab); + } + return window; +} + function findTargetedTabById( windows: readonly OrderedWindow[], selector: TargetSelector | undefined, @@ -53,6 +66,17 @@ export function resolveWindow(windows: readonly OrderedWindow[], selector: Targe throw new BrowserCommandError("NO_ACTIVE_TAB", "Firefox has no normal browser windows."); } + if (selector === undefined && windows.length > 1) { + throw new BrowserCommandError( + "INVALID_TARGET", + `Ambiguous window: Firefox has ${String(windows.length)} windows. Pass \`--window id:<id>\` or explicitly choose \`--window active\`.`, + { + reason: "ambiguous-window", + windowIds: windows.map((window) => window.id), + }, + ); + } + if (selector === undefined || selector.kind === "active") { const focused = windows.find((window) => window.focused); if (focused !== undefined) { @@ -86,6 +110,18 @@ export function resolveTab(window: OrderedWindow, selector: TargetSelector["tab" throw new BrowserCommandError("NO_ACTIVE_TAB", "Firefox window has no tabs."); } + if (selector === undefined && window.tabs.length > 1) { + throw new BrowserCommandError( + "INVALID_TARGET", + `Ambiguous tab: Firefox window ${String(window.id)} has ${String(window.tabs.length)} tabs. Pass \`--tab id:<id>\` or explicitly choose \`--tab active\`.`, + { + reason: "ambiguous-tab", + windowId: window.id, + tabIds: window.tabs.map((tab) => tab.id), + }, + ); + } + if (selector === undefined || selector.kind === "active") { const active = window.tabs.find((tab) => tab.active); if (active === undefined) { diff --git a/packages/extension/src/browser-commands-batch.test.ts b/packages/extension/src/browser-commands-batch.test.ts index 3c340b7..c9b8799 100644 --- a/packages/extension/src/browser-commands-batch.test.ts +++ b/packages/extension/src/browser-commands-batch.test.ts @@ -41,6 +41,62 @@ describe("browser batch command handling", () => { ]); }); + it("runs window-only steps without requiring an unambiguous tab", async () => { + const adapter = new FakeBrowserAdapter([windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(103, 1, false, 10)])]); + + const response = await handleBrowserRequest( + createRequest( + "batch", + { + steps: [ + { command: "tab.new", params: { url: "https://qa.example/" } }, + { command: "window.select", params: {} }, + ], + }, + "batch-window-only", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: true, + result: { + ok: true, + steps: [ + { index: 0, command: "tab.new", ok: true, result: { target: { windowId: 10, tabId: 102 } } }, + { index: 1, command: "window.select", ok: true, result: { window: { id: 10 } } }, + ], + }, + }); + }); + + it("rejects an omitted batch window before running tab-dependent steps", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const response = await handleBrowserRequest( + createRequest( + "batch", + { + steps: [{ command: "snapshot", params: { interactiveOnly: true } }], + }, + "batch-ambiguous-window", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: false, + error: { + code: "INVALID_TARGET", + message: "Ambiguous window: Firefox has 2 windows. Pass `--window id:<id>` or explicitly choose `--window active`.", + }, + }); + expect(adapter.contentRequests).toEqual([]); + }); + it("preserves step target overrides", async () => { const adapter = new FakeBrowserAdapter([ windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), @@ -51,6 +107,7 @@ describe("browser batch command handling", () => { createRequest( "batch", { + target: { tab: { kind: "id", id: 101 } }, steps: [ { command: "snapshot", params: {} }, { diff --git a/packages/extension/src/browser-commands-targets.test.ts b/packages/extension/src/browser-commands-targets.test.ts index 0dab7bc..9bbd4af 100644 --- a/packages/extension/src/browser-commands-targets.test.ts +++ b/packages/extension/src/browser-commands-targets.test.ts @@ -59,6 +59,137 @@ describe("browser command handling", () => { expect(adapter.contentRequests.some((request) => request.tabId === 101)).toBe(false); }); + it("rejects an omitted window selector when multiple windows exist", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const response = await handleBrowserRequest(createRequest("open", { url: "https://qa.example/", newTab: false }, "ambiguous-window"), adapter); + + expect(response).toMatchObject({ + ok: false, + error: { + code: "INVALID_TARGET", + message: "Ambiguous window: Firefox has 2 windows. Pass `--window id:<id>` or explicitly choose `--window active`.", + }, + }); + expect(adapter.navigations).toEqual([]); + }); + + it("rejects an omitted tab selector when the selected window has multiple tabs", async () => { + const adapter = new FakeBrowserAdapter([windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)])]); + + const response = await handleBrowserRequest( + createRequest("open", { url: "https://qa.example/", newTab: false, target: { window: { kind: "id", id: 10 } } }, "ambiguous-tab"), + adapter, + ); + + expect(response).toMatchObject({ + ok: false, + error: { + code: "INVALID_TARGET", + message: "Ambiguous tab: Firefox window 10 has 2 tabs. Pass `--tab id:<id>` or explicitly choose `--tab active`.", + }, + }); + expect(adapter.navigations).toEqual([]); + }); + + it("accepts explicit active selectors when several windows and tabs exist", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const response = await handleBrowserRequest( + createRequest( + "open", + { + url: "https://qa.example/", + newTab: false, + target: { + window: { kind: "active" }, + tab: { kind: "active" }, + }, + }, + "explicit-active", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: true, + result: { + target: { windowId: 10, tabId: 101 }, + }, + }); + expect(adapter.navigations).toEqual([{ tabId: 101, url: "https://qa.example/" }]); + }); + + it("lists tabs in an explicitly selected window without requiring a tab selector", async () => { + const adapter = new FakeBrowserAdapter([windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)])]); + + const response = await handleBrowserRequest(createRequest("tabs.list", { target: { window: { kind: "id", id: 10 } } }, "list-tabs-window"), adapter); + + expect(response).toMatchObject({ + ok: true, + result: { + tabs: [{ id: 101 }, { id: 102 }], + }, + }); + }); + + it("opens a new tab in the window identified by an explicit tab id", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const response = await handleBrowserRequest( + createRequest( + "open", + { + url: "https://qa.example/", + newTab: true, + target: { tab: { kind: "id", id: 201 } }, + }, + "new-tab-by-tab-id", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: true, + result: { + target: { windowId: 20, tabId: 102 }, + }, + }); + }); + + it("resizes an explicitly selected window without requiring a tab selector", async () => { + const adapter = new FakeBrowserAdapter([windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)])]); + + const response = await handleBrowserRequest( + createRequest( + "set.viewport", + { + width: 1200, + height: 800, + target: { window: { kind: "id", id: 10 } }, + }, + "resize-window", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: true, + result: { + window: { id: 10, width: 1200, height: 800 }, + }, + }); + }); + 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 } }], diff --git a/packages/extension/src/browser-commands-test-cases.ts b/packages/extension/src/browser-commands-test-cases.ts index 1bdff06..6457371 100644 --- a/packages/extension/src/browser-commands-test-cases.ts +++ b/packages/extension/src/browser-commands-test-cases.ts @@ -1,9 +1,8 @@ -import { PROTOCOL_VERSION, commandSchemas, createRequest, isCommandId } from "@firefox-cli/protocol"; +import { commandSchemas, createRequest, isCommandId, PROTOCOL_VERSION } from "@firefox-cli/protocol"; import { expect } from "vitest"; import { handleBrowserRequest } from "./browser-commands.js"; -import { FakeBrowserAdapter, parseTestBrowserRequest, tabSummary, windowSnapshot } from "./browser-commands-test-utils.js"; - import { browserSmokeRequests } from "./browser-commands-test-smoke.js"; +import { FakeBrowserAdapter, parseTestBrowserRequest, tabSummary, windowSnapshot } from "./browser-commands-test-utils.js"; export async function runCase01() { const expectedCommands = Object.keys(commandSchemas) @@ -76,16 +75,11 @@ export async function runCase02() { export async function runCase03() { const adapter = new FakeBrowserAdapter([windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), windowSnapshot(10, true, [tabSummary(101, 0, true, 10)])]); - const response = await handleBrowserRequest(createRequest("tabs.list", {}, "request-1"), adapter); + const response = await handleBrowserRequest(createRequest("tabs.list", { target: { window: { kind: "active" } } }, "request-1"), adapter); expect(response).toMatchObject({ ok: true, result: { - target: { - windowId: 10, - windowIndex: 0, - tabId: 101, - }, tabs: [{ id: 101 }], }, }); diff --git a/packages/extension/src/browser-handlers/navigation.ts b/packages/extension/src/browser-handlers/navigation.ts index bc53057..708be22 100644 --- a/packages/extension/src/browser-handlers/navigation.ts +++ b/packages/extension/src/browser-handlers/navigation.ts @@ -8,7 +8,7 @@ type NavigationCommand = "open" | "back" | "forward" | "reload"; export const navigationHandlers: BrowserHandlerMap<NavigationCommand> = { open: async (request, adapter, context) => { if (request.params.newTab) { - const window = await context.targetContext.resolveWindow(request.params.target?.window); + const window = await context.targetContext.resolveTargetWindow(request.params.target); assertMutableWindow(window); const tab = await adapter.createTab({ url: request.params.url, windowId: window.id }); context.targetContext.invalidate(); diff --git a/packages/extension/src/browser-handlers/phase8-browser.ts b/packages/extension/src/browser-handlers/phase8-browser.ts index 83cc2e8..f080117 100644 --- a/packages/extension/src/browser-handlers/phase8-browser.ts +++ b/packages/extension/src/browser-handlers/phase8-browser.ts @@ -1,14 +1,14 @@ import { - createErrorResponseForRequest, - createOkResponse, type ClipboardResult, type CookieResult, + createErrorResponseForRequest, + createOkResponse, type RequestEnvelope, type SetViewportResult, } from "@firefox-cli/protocol"; import { sendContentCommand } from "../browser-command/content-bridge.js"; import { BrowserCommandError } from "../browser-command/errors.js"; -import { toOrderedWindows, toWindowSummary } from "../browser-command/targets.js"; +import { assertMutableWindow, toOrderedWindows, toWindowSummary } from "../browser-command/targets.js"; import type { BrowserHandlerMap } from "./types.js"; type Phase8BrowserCommand = "download" | "clipboard" | "cookies" | "network" | "notify" | "pdf" | "set.viewport"; @@ -117,8 +117,9 @@ export const phase8BrowserHandlers: BrowserHandlerMap<Phase8BrowserCommand> = { }); }, "set.viewport": async (request, adapter, context) => { - const resolved = await context.targetContext.resolveTarget(request.params.target); - const window = await adapter.resizeWindow(resolved.window.id, { + const targetWindow = await context.targetContext.resolveTargetWindow(request.params.target); + assertMutableWindow(targetWindow); + const window = await adapter.resizeWindow(targetWindow.id, { width: request.params.width, height: request.params.height, }); diff --git a/packages/extension/src/browser-handlers/tabs-windows.ts b/packages/extension/src/browser-handlers/tabs-windows.ts index a6e0420..7f0c270 100644 --- a/packages/extension/src/browser-handlers/tabs-windows.ts +++ b/packages/extension/src/browser-handlers/tabs-windows.ts @@ -7,12 +7,9 @@ type TabsWindowsCommand = "tabs.list" | "windows.list" | "tab.new" | "tab.select export const tabsWindowsHandlers: BrowserHandlerMap<TabsWindowsCommand> = { "tabs.list": async (request, _adapter, context) => { - const resolved = await context.targetContext.resolveTarget(request.params.target, { - allowPrivate: true, - }); + const window = await context.targetContext.resolveTargetWindow(request.params.target); return createOkResponse(request, { - target: resolved.target, - tabs: [...resolved.window.tabs], + tabs: [...window.tabs], }); }, "windows.list": async (request, _adapter, context) => { @@ -68,7 +65,7 @@ export const tabsWindowsHandlers: BrowserHandlerMap<TabsWindowsCommand> = { }); }, "window.select": async (request, adapter, context) => { - const window = await context.targetContext.resolveWindow(request.params.target.window); + const window = await context.targetContext.resolveWindow(request.params.target?.window); assertMutableWindow(window); await adapter.focusWindow(window.id); context.targetContext.invalidate(); @@ -83,7 +80,7 @@ export const tabsWindowsHandlers: BrowserHandlerMap<TabsWindowsCommand> = { }); }, "window.close": async (request, adapter, context) => { - const window = await context.targetContext.resolveWindow(request.params.target.window); + const window = await context.targetContext.resolveWindow(request.params.target?.window); assertMutableWindow(window); await adapter.closeWindow(window.id); context.targetContext.invalidate(); diff --git a/packages/protocol/src/batch.ts b/packages/protocol/src/batch.ts index a2f953b..6578997 100644 --- a/packages/protocol/src/batch.ts +++ b/packages/protocol/src/batch.ts @@ -1,8 +1,7 @@ import { z } from "zod"; - +import { addUploadTotalIssue, uploadParamsSchema } from "./browser.js"; import { MAX_BATCH_RESULT_BYTES } from "./constants.js"; import { protocolErrorSchema } from "./core.js"; -import { addUploadTotalIssue, uploadParamsSchema } from "./browser.js"; import { targetSelectorSchema } from "./target.js"; export interface BatchRegistryLookup { @@ -10,18 +9,8 @@ export interface BatchRegistryLookup { readonly isBatchable: (command: string) => boolean; readonly paramsFor: (command: string) => z.ZodType | undefined; readonly resultFor: (command: string) => z.ZodType | undefined; - readonly paramsWithDefaultTarget: (command: string, params: unknown) => BatchDefaultTargetParams; } -type BatchDefaultTargetParams = - | { - readonly found: true; - readonly params: unknown; - } - | { - readonly found: false; - }; - export function createBatchSchemas(registry: BatchRegistryLookup) { const batchStepSchema = z .object({ @@ -59,16 +48,14 @@ export function createBatchSchemas(registry: BatchRegistryLookup) { } const params = paramsSchema.safeParse(step.params); - const paramsWithDefaultTarget = registry.paramsWithDefaultTarget(step.command, step.params); - const fallbackParams = params.success || !paramsWithDefaultTarget.found ? params : paramsSchema.safeParse(paramsWithDefaultTarget.params); - if (!fallbackParams.success) { + if (!params.success) { context.addIssue({ code: "custom", message: "Batch step params are invalid.", path: ["params"], params: { command: step.command, - issues: fallbackParams.error.issues, + issues: params.error.issues, }, }); } diff --git a/packages/protocol/src/browser/output.ts b/packages/protocol/src/browser/output.ts index acc4539..37bdada 100644 --- a/packages/protocol/src/browser/output.ts +++ b/packages/protocol/src/browser/output.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import { waitElementSummarySchema } from "../content.js"; -import { resolvedTargetSchema, targetSelectorSchema, windowSummarySchema } from "../target.js"; +import { resolvedTargetSchema, targetSelectorSchema, windowSummarySchema, windowTargetSelectorSchema } from "../target.js"; import { phase8ElementTargetParamsSchema } from "./interactions.js"; export const logActions = ["list", "clear"] as const; @@ -77,7 +77,7 @@ export const pdfResultSchema = z.object({ path: z.string().min(1) }).strict(); export const setViewportParamsSchema = z .object({ - target: targetSelectorSchema.optional(), + target: windowTargetSelectorSchema.optional(), width: z.number().int().positive().max(10_000), height: z.number().int().positive().max(10_000), }) diff --git a/packages/protocol/src/command-validation.ts b/packages/protocol/src/command-validation.ts index ef96254..0c48c30 100644 --- a/packages/protocol/src/command-validation.ts +++ b/packages/protocol/src/command-validation.ts @@ -1,8 +1,7 @@ import type { z } from "zod"; import type { CommandParams, CommandResult } from "./envelopes.js"; -import { commandAcceptsProtocolBatchDefaultTarget, commandSchemas, type CommandId } from "./registry/index.js"; -import { targetSelectorSchema } from "./target.js"; +import { type CommandId, commandSchemas } from "./registry/index.js"; export type CommandSafeParse<T> = | { @@ -21,38 +20,10 @@ export function safeParseStrictCommandParams(command: CommandId, params: unknown export function safeParseBatchStepCommandParams<C extends CommandId>(command: C, params: unknown): CommandSafeParse<CommandParams<C>>; export function safeParseBatchStepCommandParams(command: CommandId, params: unknown): CommandSafeParse<unknown> { - const parsed = safeParseStrictCommandParams(command, params); - if (parsed.success || !commandAcceptsProtocolBatchDefaultTarget(command)) { - return parsed; - } - - const fallbackParams = paramsWithDefaultTarget(params); - return fallbackParams === undefined ? parsed : safeParseStrictCommandParams(command, fallbackParams); + return safeParseStrictCommandParams(command, params); } export function safeParseCommandResult<C extends CommandId>(command: C, result: unknown): CommandSafeParse<CommandResult<C>>; export function safeParseCommandResult(command: CommandId, result: unknown): CommandSafeParse<unknown> { return commandSchemas[command].result.safeParse(result); } - -function paramsWithDefaultTarget(params: unknown): (Record<string, unknown> & { readonly target: unknown }) | undefined { - if (!isRecord(params)) { - return undefined; - } - - if (params.target !== undefined) { - return undefined; - } - - return { - ...params, - target: targetSelectorSchema.parse({ - window: { kind: "active" }, - tab: { kind: "active" }, - }), - }; -} - -function isRecord(value: unknown): value is Record<string, unknown> { - return typeof value === "object" && value !== null && !Array.isArray(value); -} diff --git a/packages/protocol/src/metadata.ts b/packages/protocol/src/metadata.ts index d69ebe4..178f74f 100644 --- a/packages/protocol/src/metadata.ts +++ b/packages/protocol/src/metadata.ts @@ -67,7 +67,6 @@ export interface CliRouteEntry<C extends string = string> { export interface CommandBatchMetadata { readonly allowed: boolean; - readonly protocolDefaultTarget?: boolean; readonly extensionDefaultTarget?: boolean; readonly timeoutRebase?: boolean; } diff --git a/packages/protocol/src/protocol-metadata-behavior.test.ts b/packages/protocol/src/protocol-metadata-behavior.test.ts index 1e818c9..b245143 100644 --- a/packages/protocol/src/protocol-metadata-behavior.test.ts +++ b/packages/protocol/src/protocol-metadata-behavior.test.ts @@ -3,17 +3,16 @@ import { actionKinds, commandAcceptsBatchTimeout, commandAcceptsExtensionBatchDefaultTarget, - commandAcceptsProtocolBatchDefaultTarget, commandSchemas, - createRequestProtocolMismatchError, createRequest, + createRequestProtocolMismatchError, gatedCapabilities, getCliRouteEntries, - getRequestProtocolCompatibility, getCliRoutes, getCommandCliRoutes, getCommandCompatibilityMetadata, getCommandSecurityMetadata, + getRequestProtocolCompatibility, getRequestProtocolRequirement, isActionCommand, isBatchableCommandId, @@ -77,12 +76,6 @@ describe("protocol command metadata", () => { expect(nonBatchableCommands).toEqual(["hello", "capabilities", "noop", "batch", "pair.approve", "pair.reset", "pair.requestApproval", "pair.openApproval"]); }); - it("marks only required tab/window selectors for protocol batch default targets", () => { - const protocolDefaultCommands = commandIds().filter(commandAcceptsProtocolBatchDefaultTarget); - - expect(protocolDefaultCommands).toEqual(["tab.select", "tab.close", "window.select", "window.close"]); - }); - it("marks extension batch default target commands", () => { const extensionDefaultCommands = commandIds().filter(commandAcceptsExtensionBatchDefaultTarget); diff --git a/packages/protocol/src/protocol-request.test.ts b/packages/protocol/src/protocol-request.test.ts index 53c08c5..aeaca53 100644 --- a/packages/protocol/src/protocol-request.test.ts +++ b/packages/protocol/src/protocol-request.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { PROTOCOL_VERSION, createRequest, parseBoundaryRequest } from "./index.js"; -import { boundaries, inheritedCommandNames, cliIdentity } from "./protocol-test-support.js"; +import { createRequest, PROTOCOL_VERSION, parseBoundaryRequest } from "./index.js"; +import { boundaries, cliIdentity, inheritedCommandNames } from "./protocol-test-support.js"; describe("parseBoundaryRequest", () => { it.each(boundaries)("validates hello requests across %s", (boundary) => { @@ -104,7 +104,7 @@ describe("parseBoundaryRequest", () => { protocolVersion: PROTOCOL_VERSION, id: "tab-close-1", command: "tab.close", - params: {}, + params: { unexpected: true }, }); expect(parsed).toMatchObject({ @@ -115,6 +115,17 @@ describe("parseBoundaryRequest", () => { }); }); + it("preserves omitted select and close targets for extension ambiguity checks", () => { + const requests = [ + createRequest("tab.select", {}, "tab-select-omitted"), + createRequest("tab.close", {}, "tab-close-omitted"), + createRequest("window.select", {}, "window-select-omitted"), + createRequest("window.close", {}, "window-close-omitted"), + ]; + + expect(requests.map((request) => parseBoundaryRequest("host-to-extension", request))).toEqual(requests.map((request) => ({ ok: true, value: request }))); + }); + it("validates tab list requests", () => { const request = createRequest( "tabs.list", @@ -203,6 +214,12 @@ describe("parseBoundaryRequest", () => { it("rejects tab selectors for window-only browsing commands at direct and batch boundaries", () => { const invalidRequests: readonly unknown[] = [ + { + protocolVersion: PROTOCOL_VERSION, + id: "tabs-list-tab-target", + command: "tabs.list", + params: { target: { tab: { kind: "id", id: 42 } } }, + }, { protocolVersion: PROTOCOL_VERSION, id: "tab-new-tab-target", @@ -221,15 +238,23 @@ describe("parseBoundaryRequest", () => { command: "window.close", params: { target: { tab: { kind: "id", id: 42 } } }, }, + { + protocolVersion: PROTOCOL_VERSION, + id: "set-viewport-tab-target", + command: "set.viewport", + params: { width: 1200, height: 800, target: { tab: { kind: "id", id: 42 } } }, + }, { protocolVersion: PROTOCOL_VERSION, id: "batch-window-only-tab-target", command: "batch", params: { steps: [ + { command: "tabs.list", params: { target: { tab: { kind: "id", id: 42 } } } }, { 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 } } } }, + { command: "set.viewport", params: { width: 1200, height: 800, target: { tab: { kind: "id", id: 42 } } } }, ], }, }, @@ -272,9 +297,11 @@ describe("parseBoundaryRequest", () => { it("accepts window selectors for window-only browsing commands and full selectors for page commands", () => { const validRequests = [ + createRequest("tabs.list", { target: { window: { kind: "id", id: 7 } } }, "tabs-list-window-target"), 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("set.viewport", { width: 1200, height: 800, target: { window: { kind: "id", id: 7 } } }, "set-viewport-window-target"), createRequest( "open", { diff --git a/packages/protocol/src/protocol-response-6.test.ts b/packages/protocol/src/protocol-response-6.test.ts index 0d2b3ab..0c3ae2a 100644 --- a/packages/protocol/src/protocol-response-6.test.ts +++ b/packages/protocol/src/protocol-response-6.test.ts @@ -1,20 +1,20 @@ import { describe, expect, it } from "vitest"; import { - PROTOCOL_VERSION, createOkResponse, createRequest, mergeDisjointHandlerMaps, - parseBoundaryRequest, - parseBoundaryResponse, + PROTOCOL_VERSION, parseBatchStepAs, parseBatchStepResultAs, + parseBoundaryRequest, + parseBoundaryResponse, safeParseStrictCommandParams, } from "./index.js"; import { boundaries } from "./protocol-test-support.js"; describe("parseBoundaryResponse", () => { it("parses command-correlated batch steps and results", () => { - expect(safeParseStrictCommandParams("tab.close", {}).success).toBe(false); + expect(safeParseStrictCommandParams("tab.close", {}).success).toBe(true); expect( parseBatchStepAs("tab.close", { @@ -25,12 +25,7 @@ describe("parseBoundaryResponse", () => { ok: true, value: { command: "tab.close", - params: { - target: { - window: { kind: "active" }, - tab: { kind: "active" }, - }, - }, + params: {}, }, }); diff --git a/packages/protocol/src/protocol-test-support.ts b/packages/protocol/src/protocol-test-support.ts index 86be51f..9ca8d5d 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 CliRouteSelectorDimensions, type CommandId, type ComponentIdentity } from "./index.js"; +import { type Boundary, type CliRouteSelectorDimensions, type CommandId, type ComponentIdentity, commandSchemas } 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; @@ -93,6 +93,7 @@ export const expectedCliRoutesByCommand: Partial<Record<CommandId, readonly Expe const selectorDimensionsByRouteId: Readonly<Record<string, CliRouteSelectorDimensions>> = { capabilities: "neither", + "tab.list": "window", "tab.new": "window", "window.list": "neither", "window.new": "neither", @@ -102,6 +103,7 @@ const selectorDimensionsByRouteId: Readonly<Record<string, CliRouteSelectorDimen cookies: "neither", notify: "neither", pdf: "neither", + "set.viewport": "window", connect: "neither", }; diff --git a/packages/protocol/src/registry/browsing.ts b/packages/protocol/src/registry/browsing.ts index 022a478..3fa934a 100644 --- a/packages/protocol/src/registry/browsing.ts +++ b/packages/protocol/src/registry/browsing.ts @@ -5,16 +5,16 @@ import { tabCloseResultSchema, tabNewParamsSchema, tabNewResultSchema, - tabTargetParamsSchema, tabsListParamsSchema, tabsListResultSchema, + tabTargetParamsSchema, windowCloseResultSchema, windowNewParamsSchema, windowNewResultSchema, windowSelectResultSchema, - windowTargetParamsSchema, windowsListParamsSchema, windowsListResultSchema, + windowTargetParamsSchema, } from "../target.js"; import { defineCommandEntries } from "./define.js"; @@ -29,7 +29,7 @@ export const browsingCommandEntries = defineCommandEntries({ action: false, timeout: "none", batch: { allowed: true, extensionDefaultTarget: true }, - cliRoutes: [{ id: "tab.list", path: ["tab"], batch: true, selectorDimensions: "both" }], + cliRoutes: [{ id: "tab.list", path: ["tab"], batch: true, selectorDimensions: "window" }], }, "tab.new": { params: tabNewParamsSchema, @@ -48,11 +48,11 @@ export const browsingCommandEntries = defineCommandEntries({ result: tabNewResultSchema, status: "mvp", owner: "extension", - target: "required", + target: "optional", content: "never", action: false, timeout: "none", - batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, + batch: { allowed: true, extensionDefaultTarget: true }, cliRoutes: [{ id: "tab.select", path: ["tab", "select"], batch: true, selectorDimensions: "both" }], }, "tab.close": { @@ -60,11 +60,11 @@ export const browsingCommandEntries = defineCommandEntries({ result: tabCloseResultSchema, status: "mvp", owner: "extension", - target: "required", + target: "optional", content: "never", action: false, timeout: "none", - batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, + batch: { allowed: true, extensionDefaultTarget: true }, cliRoutes: [{ id: "tab.close", path: ["tab", "close"], batch: true, selectorDimensions: "both" }], }, "windows.list": { @@ -96,11 +96,11 @@ export const browsingCommandEntries = defineCommandEntries({ result: windowSelectResultSchema, status: "mvp", owner: "extension", - target: "required", + target: "optional", content: "never", action: false, timeout: "none", - batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, + batch: { allowed: true, extensionDefaultTarget: true }, cliRoutes: [{ id: "window.select", path: ["window", "select"], batch: true, selectorDimensions: "window" }], }, "window.close": { @@ -108,11 +108,11 @@ export const browsingCommandEntries = defineCommandEntries({ result: windowCloseResultSchema, status: "mvp", owner: "extension", - target: "required", + target: "optional", content: "never", action: false, timeout: "none", - batch: { allowed: true, protocolDefaultTarget: true, extensionDefaultTarget: true }, + batch: { allowed: true, extensionDefaultTarget: true }, cliRoutes: [{ id: "window.close", path: ["window", "close"], batch: true, selectorDimensions: "window" }], }, open: { diff --git a/packages/protocol/src/registry/define.ts b/packages/protocol/src/registry/define.ts index c3c47e9..cd5246c 100644 --- a/packages/protocol/src/registry/define.ts +++ b/packages/protocol/src/registry/define.ts @@ -34,7 +34,7 @@ function assertCliRouteSelectorDimensions(registry: Readonly<Record<string, Comm for (const [command, schema] of Object.entries(registry)) { for (const route of schema.cliRoutes) { const selectorDimensions: CliRouteSelectorDimensions = route.selectorDimensions; - const paramsSelectorDimensions = selectorDimensionsAcceptedByCommand(schema); + const paramsSelectorDimensions = targetSelectorDimensionsAcceptedByCommand(schema); if (selectorDimensions !== paramsSelectorDimensions) { throw new Error( `CLI route selector dimensions disagree with command params: ${command} (${route.id}) declares ${selectorDimensions}, params accept ${paramsSelectorDimensions}.`, @@ -44,7 +44,7 @@ function assertCliRouteSelectorDimensions(registry: Readonly<Record<string, Comm } } -function selectorDimensionsAcceptedByCommand(schema: CommandSchemaEntry): CliRouteSelectorDimensions { +export function targetSelectorDimensionsAcceptedByCommand(schema: CommandSchemaEntry): CliRouteSelectorDimensions { if (!(schema.params instanceof z.ZodObject)) { return "neither"; } diff --git a/packages/protocol/src/registry/index.ts b/packages/protocol/src/registry/index.ts index 95f82a1..731d08e 100644 --- a/packages/protocol/src/registry/index.ts +++ b/packages/protocol/src/registry/index.ts @@ -1,8 +1,6 @@ import type { z } from "zod"; - -import { createBatchSchemas } from "../batch.js"; -import { targetSelectorSchema } from "../target.js"; import type { ActionKind } from "../actions.js"; +import { createBatchSchemas } from "../batch.js"; import type { CliRouteEntry, CliRouteMetadata, @@ -17,7 +15,7 @@ import { actionsCommandEntries } from "./actions.js"; import { browsingCommandEntries } from "./browsing.js"; import { contentCommandEntries } from "./content.js"; import { coreCommandEntries } from "./core.js"; -import { assembleCommandRegistry, defineCommandEntries } from "./define.js"; +import { assembleCommandRegistry, defineCommandEntries, targetSelectorDimensionsAcceptedByCommand } from "./define.js"; import { pairingCommandEntries } from "./pairing.js"; import { phase8CommandEntries } from "./phase8.js"; @@ -39,7 +37,6 @@ const batchSchemas = createBatchSchemas({ isBatchable: (command) => isNonBatchCommandId(command) && nonBatchCommandSchemas[command].batch.allowed, paramsFor: (command) => (isNonBatchCommandId(command) ? nonBatchCommandSchemas[command].params : undefined), resultFor: (command) => (isNonBatchCommandId(command) ? nonBatchCommandSchemas[command].result : undefined), - paramsWithDefaultTarget: (command, params) => batchStepParamsWithDefaultTarget(command, params), }); export const batchStepSchema = batchSchemas.batchStepSchema; @@ -110,16 +107,12 @@ export function getCliRouteEntries(): readonly CliRouteEntry[] { ); } -export function isBatchableCommandId(command: string): command is CommandId { - return isCommandId(command) && commandSchemas[command].batch.allowed; +export function getCommandTargetSelectorDimensions(command: CommandId): CliRouteMetadata["selectorDimensions"] { + return targetSelectorDimensionsAcceptedByCommand(commandSchemas[command]); } -export function commandAcceptsProtocolBatchDefaultTarget(command: string): command is CommandId { - if (!isCommandId(command)) { - return false; - } - const batch: CommandBatchMetadata = commandSchemas[command].batch; - return batch.protocolDefaultTarget === true; +export function isBatchableCommandId(command: string): command is CommandId { + return isCommandId(command) && commandSchemas[command].batch.allowed; } export function commandAcceptsExtensionBatchDefaultTarget(command: string): command is CommandId { @@ -239,30 +232,6 @@ function valueAtPath(value: unknown, path: readonly string[]): unknown { }, value); } -function batchStepParamsWithDefaultTarget( - command: string, - params: unknown, -): { readonly found: true; readonly params: Record<string, unknown> & { readonly target: unknown } } | { readonly found: false } { - if (!commandAcceptsProtocolBatchDefaultTarget(command) || !isRecord(params)) { - return { found: false }; - } - - if (params.target !== undefined) { - return { found: false }; - } - - return { - found: true, - params: { - ...params, - target: targetSelectorSchema.parse({ - window: { kind: "active" }, - tab: { kind: "active" }, - }), - }, - }; -} - function isRecord(value: unknown): value is Record<string, unknown> { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/packages/protocol/src/registry/phase8.ts b/packages/protocol/src/registry/phase8.ts index d9e8041..39756ba 100644 --- a/packages/protocol/src/registry/phase8.ts +++ b/packages/protocol/src/registry/phase8.ts @@ -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, selectorDimensions: "both" }], + cliRoutes: [{ id: "set.viewport", path: ["set", "viewport"], batch: true, selectorDimensions: "window" }], }, diff: { params: diffParamsSchema, diff --git a/packages/protocol/src/target.ts b/packages/protocol/src/target.ts index a2de226..bea4895 100644 --- a/packages/protocol/src/target.ts +++ b/packages/protocol/src/target.ts @@ -67,7 +67,7 @@ export type WindowSummary = z.infer<typeof windowSummarySchema>; export const tabsListParamsSchema = z .object({ - target: targetSelectorSchema.optional(), + target: windowTargetSelectorSchema.optional(), }) .strict(); export const tabsListResultSchema = z.object({ @@ -83,7 +83,7 @@ export const tabNewParamsSchema = z .strict(); export const tabTargetParamsSchema = z .object({ - target: targetSelectorSchema, + target: targetSelectorSchema.optional(), }) .strict(); export const tabNewResultSchema = z.object({ @@ -105,7 +105,7 @@ export const windowNewParamsSchema = z .strict(); export const windowTargetParamsSchema = z .object({ - target: windowTargetSelectorSchema, + target: windowTargetSelectorSchema.optional(), }) .strict(); export const windowNewResultSchema = z.object({ diff --git a/skills/firefox-cli/SKILL.md b/skills/firefox-cli/SKILL.md index 847ea7b..442b821 100644 --- a/skills/firefox-cli/SKILL.md +++ b/skills/firefox-cli/SKILL.md @@ -51,7 +51,7 @@ Then use the command-specific help for the next operation instead of relying on ## Targeting -By default, commands target Firefox's active tab/window at command resolution time. +Omitted selectors are accepted only when the required window or tab is unique. If several windows or tabs exist, pass an ID or explicitly pass `active`. 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:<id>`. @@ -63,7 +63,7 @@ firefox-cli open --window id:<window-id> https://example.com firefox-cli snapshot --window id:<window-id> -i ``` -`window select` changes Firefox focus only; it does not establish a durable CLI target. Screenshots may activate their selected tab/window. +`tab select` and `window select` bring the chosen surface forward to the user. They do not establish durable CLI target state or absolve you from passing `--window`/`--tab` explicitly to later target-dependent commands. Screenshots may activate their selected tab/window. ## Setup State ```bash From 5a50b789b2b45244396216a64b0d6a10bb754f3a Mon Sep 17 00:00:00 2001 From: Nek-12 <vaizin.nikita@gmail.com> Date: Thu, 16 Jul 2026 13:42:23 +0200 Subject: [PATCH 8/8] Skip unused batch target resolution --- .../extension/src/browser-command/batch.ts | 13 ++++-- .../browser-commands-batch-targeting.test.ts | 45 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 packages/extension/src/browser-commands-batch-targeting.test.ts diff --git a/packages/extension/src/browser-command/batch.ts b/packages/extension/src/browser-command/batch.ts index fd3f511..6eccfc7 100644 --- a/packages/extension/src/browser-command/batch.ts +++ b/packages/extension/src/browser-command/batch.ts @@ -146,7 +146,7 @@ export function applyBatchStepDefaults( async function resolveBatchDefaultSelector(command: RequestEnvelope<"batch">, targetContext: BrowserTargetContext): Promise<TargetSelector | undefined> { const resolution = command.params.steps.reduce<TargetResolution>( - (highest, step) => (isCommandId(step.command) ? highestTargetResolution(highest, requestTargetResolution(step.command, step.params)) : highest), + (highest, step) => (isCommandId(step.command) ? highestTargetResolution(highest, requestDefaultTargetResolution(step.command, step.params)) : highest), "none", ); if (resolution === "none") { @@ -166,6 +166,13 @@ async function resolveBatchDefaultSelector(command: RequestEnvelope<"batch">, ta type TargetResolution = "none" | "window" | "tab"; +function requestDefaultTargetResolution(command: CommandId, params: unknown): TargetResolution { + if (!commandAcceptsExtensionBatchDefaultTarget(command) || !isRecord(params) || params.target !== undefined) { + return "none"; + } + return requestTargetResolution(command, params); +} + function requestTargetResolution(command: CommandId, params: unknown): TargetResolution { const selectorDimensions = getCommandTargetSelectorDimensions(command); if (selectorDimensions === "neither") { @@ -208,11 +215,11 @@ function defaultTargetOverride( params: Record<string, unknown>, defaultTarget: TargetSelector | undefined, ): { readonly target?: TargetSelector } { - if (!commandAcceptsExtensionBatchDefaultTarget(command) || params.target !== undefined || defaultTarget === undefined || !isCommandId(command)) { + if (defaultTarget === undefined || !isCommandId(command)) { return {}; } - const resolution = requestTargetResolution(command, params); + const resolution = requestDefaultTargetResolution(command, params); if (resolution === "none") { return {}; } diff --git a/packages/extension/src/browser-commands-batch-targeting.test.ts b/packages/extension/src/browser-commands-batch-targeting.test.ts new file mode 100644 index 0000000..bcacbf2 --- /dev/null +++ b/packages/extension/src/browser-commands-batch-targeting.test.ts @@ -0,0 +1,45 @@ +import { createRequest } from "@firefox-cli/protocol"; +import { describe, expect, it } from "vitest"; +import { handleBrowserRequest } from "./browser-commands.js"; +import { FakeBrowserAdapter, tabSummary, windowSnapshot } from "./browser-commands-test-utils.js"; + +describe("browser batch target defaults", () => { + it("does not resolve an omitted default when every target-dependent step has an explicit target", async () => { + const adapter = new FakeBrowserAdapter([ + windowSnapshot(10, true, [tabSummary(101, 0, true, 10), tabSummary(102, 1, false, 10)]), + windowSnapshot(20, false, [tabSummary(201, 0, true, 20)]), + ]); + + const response = await handleBrowserRequest( + createRequest( + "batch", + { + steps: [ + { + command: "snapshot", + params: { target: { tab: { kind: "id", id: 201 } }, interactiveOnly: true }, + }, + { + command: "window.select", + params: { target: { window: { kind: "id", id: 10 } } }, + }, + ], + }, + "batch-step-targets-only", + ), + adapter, + ); + + expect(response).toMatchObject({ + ok: true, + result: { + ok: true, + steps: [ + { index: 0, command: "snapshot", ok: true }, + { index: 1, command: "window.select", ok: true, result: { window: { id: 10 } } }, + ], + }, + }); + expect(adapter.contentRequests).toEqual([{ tabId: 201, command: "snapshot" }]); + }); +});