Harden explicit Firefox window targeting

.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`, `<index>`, and `id:<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:<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:<non-negative Firefox 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:<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)

docs/commands.md

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