From 3836df792efb950c5e6a8c8c4e5b76f333c897de Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Thu, 3 Sep 2026 23:40:22 -0400 Subject: [PATCH 1/9] feat: add scripted states so single-route apps get captured A route crawler cannot capture a single-route application. Pointed at an app whose dialogs, workspaces and populated views exist only behind interaction, ui-capture produced one screenshot of the boot view and reported the site fully covered. That is worse than useless: it looks like coverage. A scripted state is a named, declarative interaction script performed on a page before capture, and each state yields its own capture set under /states//. States are peers of routes rather than a phase bolted onto the end of a crawl: same bounded queue, same worker pool, same --concurrency, same results map, same report. Vocabulary: waitFor, wait, click, fill, select, press, request, reload, plus the shared optional / timeoutMs / settleMs modifiers. Admitting a kind requires that it produce committed page state, read as data in a diff, and not be expressible by composing the others -- which is why there is no hover (the viewport loop resizes underneath a cursor), no evaluate, and no variables anywhere in the format. waitFor's minCount polls locator.count() from the driver, so no user-supplied code ever crosses into the page. Determinism: every state starts from a fresh page load in a fresh browser context, since page.goto clears neither cookies nor localStorage. extends is script composition, not page-state carryover -- the child replays the parent's steps from its own clean load, so any state runs on any worker in any order. Failure model: authoring errors (duplicate names, unknown extends, a cycle, an off-host request, a viewport filter naming an unconfigured viewport) abort before Chromium launches. Runtime errors are recorded per state and the run continues, exactly as a failing route does, with stateStatus and failedStepIndex naming the step that broke -- including for a whole-state timeout, which reads the in-flight step from a Ref. A state whose precondition selector is absent is recorded as skipped rather than failed, so "this state does not exist here" stays distinguishable from "this state's script is broken". request is gated behind --allow-state-requests, checked at load time and again in the service so a programmatic caller cannot skip it. The states file carries a required version: 1 discriminant, because the step vocabulary becomes a public JSON format on other people's disks the day it ships. A state containing a request step skips video by default, since video replays the script in a second context and a non-idempotent seed would run twice. Flags: --states, --state-filter, --skip-routes, --state-timeout, --allow-state-requests, --fail-on-state-error. buildInvocation stays synchronous and I/O-free; reading and parsing the states file happens in runFromArgs, at the edge. Additive throughout: states defaults to [], captureRoutes to true, and an integration test asserts a no-states run still produces the pre-feature output tree and report numbers. A new docs test asserts README, USAGE and the schema agree, since documentation drift is the defect class that survives every other test. --- README.md | 224 +++++++++++++- cspell.json | 16 + src/docs.test.ts | 77 +++++ src/errors.ts | 39 +++ src/index.ts | 25 ++ src/integration.test.ts | 550 +++++++++++++++++++++++++++++++++ src/report.test.ts | 223 ++++++++++++++ src/report.ts | 65 +++- src/runner.test.ts | 168 ++++++++++ src/runner.ts | 122 +++++++- src/schemas.test.ts | 34 +++ src/schemas.ts | 252 +++++++++++++++ src/service.ts | 368 ++++++++++++++++++++-- src/shared.test.ts | 63 ++++ src/shared.ts | 41 ++- src/state-plan.test.ts | 256 ++++++++++++++++ src/state-plan.ts | 301 ++++++++++++++++++ src/state-script.test.ts | 641 +++++++++++++++++++++++++++++++++++++++ src/state-script.ts | 346 +++++++++++++++++++++ src/states.test.ts | 518 +++++++++++++++++++++++++++++++ src/states.ts | 339 +++++++++++++++++++++ src/video.ts | 13 + 22 files changed, 4647 insertions(+), 34 deletions(-) create mode 100644 src/docs.test.ts create mode 100644 src/report.test.ts create mode 100644 src/state-plan.test.ts create mode 100644 src/state-plan.ts create mode 100644 src/state-script.test.ts create mode 100644 src/state-script.ts create mode 100644 src/states.test.ts create mode 100644 src/states.ts diff --git a/README.md b/README.md index 73e9604..19b8a85 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,22 @@ Options: (default: desktop:1920x1080,tablet:768x1024,mobile:375x667) --hide CSS selectors to hide before screenshotting --menu-selectors Selectors to click before link discovery + (this opens menus so links become discoverable; + it is not a capture-state mechanism — see + --states for that) + --states JSON file of named interaction scripts. Each + named state is performed on a fresh page load + and yields its own capture set, so a single-route + app's dialogs and workspaces get captured too. + --state-filter Run only these named states (default: all) + --skip-routes Capture only scripted states, not crawled routes + --state-timeout Per-state budget covering navigation, script and + capture (default: 30000; a state may override it) + --allow-state-requests Permit request steps, which reach past the UI + into the app's own backend. Off by default: a + states file from a colleague should not be able + to POST to your app because you ran the tool. + --fail-on-state-error Exit non-zero when any scripted state failed --video Capture videos in addition to screenshots --video-duration Video duration when --video (default: 10000) --no-interactions Disable scripted scrolling during video @@ -167,6 +183,8 @@ Options: follows the OS theme renders light under headless Chromium, so pass dark to capture its dark face. --no-warmup Skip the pre-screenshot warm-up scroll + (warm-up triggers lazy-load + scroll-reveal + animations so screenshots capture real content) --ffmpeg ffmpeg binary path (default: ffmpeg) --launch-args Extra Chromium switches, whitespace separated (e.g. to enable an experimental web platform @@ -179,6 +197,9 @@ Examples: ui-capture https://example.com --viewports desktop:1920x1080,mobile:390x844 ui-capture https://example.com --hide ".cookie-banner,#chat-widget" ui-capture https://example.com --color-scheme dark + ui-capture https://example.com --launch-args "--enable-blink-features=CanvasDrawElement" + ui-capture http://localhost:5173 --max-depth 0 --states ./ui-capture.states.json + ui-capture http://localhost:5173 --states ./states.json --state-filter fleet-editor ``` ## Library @@ -330,9 +351,197 @@ All three errors are `S.TaggedError` subclasses, so they discriminate cleanly un | `colorScheme` | `--color-scheme` | `"light" \| "dark" \| "no-preference"` | `"light"` | The `prefers-color-scheme` reported to the page, applied to both the screenshot and video contexts; the default matches Playwright's, so existing captures are unchanged. | | `ffmpegPath` | `--ffmpeg` | `string` | `"ffmpeg"` | Absolute path or anything on `PATH`. | | `launchArgs` | `--launch-args` | `string[]` | `[]` | Appended after the baseline switches so they win on conflict; the CLI value splits on whitespace rather than commas, since one switch may itself contain commas. | +| `states` | `--states` | `CaptureState[]` | `[]` | Named interaction scripts, each yielding its own capture set; empty by default, so a run without a states file behaves exactly as it always has. | +| `stateTimeout` | `--state-timeout` | `int ≥ 1` (ms) | `30000` | Whole-state budget covering navigation, script and capture; a state may override it with its own `timeoutMs`. | +| `captureRoutes` | `--skip-routes` (¬) | `boolean` | `true` | Set `false` to capture only scripted states, for an app whose boot view is a loading spinner. | +| `allowStateRequests` | `--allow-state-requests`| `boolean` | `false` | Gate on `request` steps, checked at load time *and* in the service so a programmatic caller cannot skip it. | `(¬)` means the CLI flag *negates* the default — e.g. `--no-warmup` sets `warmupScroll: false`. +## Scripted states + +A route crawler cannot capture a single-route application. +Point this tool at a Three.js operator console — one URL, whose spawn dialog, environment dialog and populated fleet exist only after interaction — and it produces one screenshot of the boot view and reports the site fully covered. +That is worse than useless: it looks like coverage. + +Scripted states fix that. +A state is a named, declarative interaction script performed on the page before capture, and each state yields its own capture set. +States are first-class peers of routes: same queue, same worker pool, same `--concurrency`, same results map, same report. + +### The states file + +```json +{ + "version": 1, + "states": [ + { + "name": "spawn-dialog", + "description": "Drone spawn dialog, fixed-wing preset selected", + "steps": [ + { "kind": "waitFor", "selector": "canvas[data-scene-ready]", "timeoutMs": 20000 }, + { "kind": "click", "selector": "[data-testid='spawn-drone']", "settleMs": 400 }, + { "kind": "waitFor", "selector": "dialog#spawn", "state": "visible" }, + { "kind": "select", "selector": "#drone-type", "values": ["fixed-wing"] }, + { "kind": "fill", "selector": "#callsign", "value": "RESQ-01" } + ] + }, + { + "name": "fleet-multidomain", + "description": "Six drones across air/ground/marine, seeded via the app's own API", + "steps": [ + { "kind": "waitFor", "selector": "[data-app-ready]" }, + { "kind": "request", "method": "POST", "path": "/api/sim/seed", + "json": { "preset": "multidomain", "count": 6 }, "expectStatus": 201 }, + { "kind": "reload" }, + { "kind": "waitFor", "selector": ".fleet-row", "minCount": 6, "timeoutMs": 30000 } + ] + }, + { + "name": "fleet-editor", + "extends": "fleet-multidomain", + "viewports": ["desktop"], + "steps": [ + { "kind": "click", "selector": "[data-panel='editor']" }, + { "kind": "waitFor", "selector": ".editor-root .cm-content" } + ] + }, + { + "name": "safety-advanced", + "url": "/console?mode=advanced", + "precondition": "nav [data-tab='safety']", + "timeoutMs": 45000, + "steps": [ + { "kind": "click", "selector": "#consent-dismiss", "optional": true }, + { "kind": "click", "selector": "nav [data-tab='safety']" }, + { "kind": "waitFor", "selector": "[data-geofence-warning]" } + ] + } + ] +} +``` + +`version` is required rather than defaulted. +The step vocabulary becomes a public JSON format on files on other people's disks the day it ships, and a discriminant is the only cheap way to land a v2 that renames a kind without guessing at an unversioned file's intent. + +`name` is constrained to `^[a-z0-9][a-z0-9-]*$` because it becomes a directory component. +Rejecting loudly beats slugifying two states into one directory, and lowercase-only keeps `Spawn` and `spawn` from colliding on a case-insensitive filesystem. + +### State fields + +| Field | Type | Default | What it does | +| ----- | ---- | ------- | ------------ | +| `name` | `string` matching `^[a-z0-9][a-z0-9-]*$` | *required* | Identifies the state, and names its output directory. | +| `steps` | `CaptureStep[]` | *required* | The script, run in order on a fresh page load. | +| `description` | `string` | — | Free text; carried for the reader, not used by the tool. | +| `url` | `string` | the seed URL | Absolute, or relative to the seed URL, and subject to the same host filter as any crawled link. | +| `extends` | `string` | — | Another state's name, whose steps are prepended to this one's; chains deeper than five links are rejected. | +| `precondition` | `string` (CSS selector) | — | Probed on the fresh load before any step; absent means the state is recorded `skipped` rather than `failed`. | +| `viewports` | `string[]` | every configured viewport | Restrict the state to named viewports, for UI that does not exist at every breakpoint. | +| `timeoutMs` | `int ≥ 1` | `--state-timeout` (30000) | Whole-state budget covering navigation, script and capture. | +| `allowVideoReplay` | `boolean` | `false` | Record video for a state whose script contains a `request` step, when that seed is idempotent. | + +### Running it + +```bash +ui-capture http://localhost:5173 \ + --max-depth 0 \ + --states ./ui-capture.states.json \ + --state-timeout 45000 \ + --allow-state-requests \ + --viewports desktop:1920x1080,mobile:390x844 \ + --launch-args "--use-gl=angle --use-angle=swiftshader" \ + --fail-on-state-error +``` + +That run captures the boot view once as a route, then each state once as its own set: + +```text +ui-captures/ +├── REPORT.md +├── capture-report.json +├── root/ # the seed route, crawled as always +│ ├── screenshots/ # the boot view +│ └── states/ +│ ├── spawn-dialog/screenshots/ # desktop + mobile +│ ├── fleet-multidomain/screenshots/ +│ └── fleet-editor/screenshots/ # desktop only, per its viewports filter +└── console/ # safety-advanced set url: /console?mode=advanced + └── states/ + └── safety-advanced/screenshots/ +``` + +Each `screenshots/` directory holds the same `png/` `webp/` `jpg/` triple a route capture produces, so nothing downstream has to special-case a state. +`--skip-routes` drops `root/screenshots/` and captures only the four state sets, for an app whose boot view is a loading spinner. +`--state-filter fleet-editor` runs one state, which is how you iterate on a script you are still writing. + +If `fleet-editor`'s last `waitFor` never resolves, the run still finishes: the other three states capture, `root/states/fleet-editor/` is created and left empty, `REPORT.md` gains a row naming the failing step, and `--fail-on-state-error` makes the process exit non-zero so CI does not go green on a state that never rendered. + +### Step vocabulary + +| Kind | Fields | What it is for | +| ---- | ------ | -------------- | +| `waitFor` | `selector`, `state?` (`visible` \| `hidden` \| `attached` \| `detached`, default `visible`), `minCount?` | Readiness, and the assertion mechanism, because the load event is a lie in an SPA; `minCount` exists so waiting for *one* `.fleet-row` cannot shoot a half-populated fleet. | +| `wait` | `ms` | The crude one, and the only honest tool for a WebGL scene whose intro tween has no DOM correlate; prefer `settleMs`, or a `waitFor` on a readiness attribute. | +| `click` | `selector`, `nth?` (zero-based) | Opens the dialog, the tab, the workspace; Playwright auto-scrolls and auto-waits for actionability. | +| `fill` | `selector`, `value` | Callsigns, coordinates, waypoints; also handles `contenteditable`. | +| `select` | `selector`, `values` | Native ``. Not redundant with {@link ClickStep}: Chromium + * renders the option list in an OS-level popup that DOM clicks cannot reach. + */ +export class SelectStep extends S.Class("SelectStep")({ + kind: S.Literal("select"), + selector: S.String, + values: S.Array(S.String), + ...StepBaseFields, +}) {} + +export class PressStep extends S.Class("PressStep")({ + kind: S.Literal("press"), + key: S.String, + /** Target a specific element; omitted, the key goes to `page.keyboard`. */ + selector: S.optional(S.String), + ...StepBaseFields, +}) {} + +/** + * Seed application state through the app's own API, using the page's browser + * context so the request inherits its session cookie and origin. + * + * `path` is resolved against the page URL, making it same-origin by + * construction; the crawler's host filter is applied as a second gate. Runs + * only when `allowStateRequests` is enabled (`--allow-state-requests`). + */ +export class RequestStep extends S.Class("RequestStep")({ + kind: S.Literal("request"), + method: S.Literal("GET", "POST", "PUT", "PATCH", "DELETE"), + /** Resolved against the page URL. Absolute URLs must stay same-host. */ + path: S.String, + json: S.optional(S.Unknown), + headers: S.optional(S.Record({ key: S.String, value: S.String })), + /** Defaults to "any 2xx". A seed that silently 500s fails the state. */ + expectStatus: S.optional(S.Number.pipe(S.int(), S.positive())), + ...StepBaseFields, +}) {} + +/** + * Re-enter the app against new server state. Without it {@link RequestStep} is + * half-useless: an app that reads its fleet once at boot never shows seeded + * data on the already-loaded page. + */ +export class ReloadStep extends S.Class("ReloadStep")({ + kind: S.Literal("reload"), + waitUntil: S.optionalWith( + S.Literal("load", "domcontentloaded", "networkidle", "commit"), + { default: () => "networkidle" as const }, + ), + ...StepBaseFields, +}) {} + +/** + * The complete scripted-step vocabulary. + * + * The governing rule for admitting a kind: a step must produce **committed + * page state**, must be readable as data by a reviewer, and must not be + * expressible by composing the others. That rule is why there is no `hover` + * (cursor-transient, and the viewport loop resizes underneath it), no + * `evaluate` (unreviewable), and no variables — no response value is ever + * bound to a name, so there is no templating, interpolation, or expression + * language anywhere in the format. + */ +export const CaptureStep = S.Union( + WaitForStep, + WaitStep, + ClickStep, + FillStep, + SelectStep, + PressStep, + RequestStep, + ReloadStep, +); +export type CaptureStep = typeof CaptureStep.Type; + +/** + * A named interaction script performed on a page before capture, so the state + * it produces gets its own capture set. + * + * `name` is pattern-constrained because it becomes a directory component: + * rejecting loudly beats silently slugifying two states into one directory, + * and lowercase-only avoids `Spawn`/`spawn` colliding on a case-insensitive + * filesystem. + */ +export class CaptureState extends S.Class("CaptureState")({ + name: S.String.pipe(S.pattern(/^[a-z0-9][a-z0-9-]*$/)), + description: S.optional(S.String), + /** Absolute, or relative to the seed URL. Defaults to the seed URL. */ + url: S.optional(S.String), + /** + * Prepend another state's steps to this one's. The child still starts from a + * fresh page load in a fresh context and *replays* the parent — states never + * inherit live page state from each other. + */ + extends: S.optional(S.String), + /** + * A selector probed on the fresh load, before any step. When it is absent, + * the state is recorded as `skipped` rather than `failed`: "this state does + * not exist here" is a different event from "this state's script is broken". + */ + precondition: S.optional(S.String), + /** + * Restrict this state to named viewports. The script runs once and the + * viewport loop resizes afterwards, so a dialog that unmounts below a + * breakpoint would otherwise be screenshotted as the boot view. + */ + viewports: S.optional(S.Array(S.String)), + steps: S.Array(CaptureStep), + /** Whole-state budget: navigation + script + capture. */ + timeoutMs: S.optional(S.Number.pipe(S.int(), S.positive())), + /** + * Record video for this state even though its script contains a `request` + * step. Video replays the script in a second context, so a non-idempotent + * seed would run twice and the video would disagree with the stills; such + * states skip video unless this says otherwise. + */ + allowVideoReplay: S.optionalWith(S.Boolean, { default: () => false }), +}) {} + +/** + * The on-disk states file. `version` is required rather than defaulted: the + * step vocabulary becomes a public JSON format on files on other people's + * disks the day it ships, and a discriminant is what lets a v2 rename a kind + * without guessing at an unversioned file's intent. + */ +export class StatesFile extends S.Class("StatesFile")({ + version: S.Literal(1), + states: S.Array(CaptureState), +}) {} + +/** Outcome of one scripted state. */ +export const StateStatus = S.Literal("captured", "skipped", "failed"); +export type StateStatus = typeof StateStatus.Type; + export class CaptureResult extends S.Class("CaptureResult")({ url: S.String, route: S.String, + /** Set only for scripted-state captures; absent for crawled routes. */ + state: S.optional(S.String), + stateStatus: S.optional(StateStatus), + /** Index of the step that failed; `-1` for a whole-state failure. */ + failedStepIndex: S.optional(S.Number.pipe(S.int())), screenshots: S.Record({ key: S.String, value: ScreenshotPaths }), videos: S.optional(S.Record({ key: S.String, value: VideoQualityPaths })), error: S.optional(S.String), @@ -86,6 +289,20 @@ const CaptureConfigFields = { * capturing such a site's dark face requires saying so explicitly. */ colorScheme: S.Literal("light", "dark", "no-preference"), + /** + * Named interaction scripts run before capture. Empty by default, so a run + * without a states file behaves exactly as it always has. + */ + states: S.Array(CaptureState), + /** Default whole-state budget in ms; a state may override it. */ + stateTimeout: S.Number.pipe(S.int(), S.positive()), + /** Crawl and capture routes. `false` captures only scripted states. */ + captureRoutes: S.Boolean, + /** + * Permit `request` steps. Off by default: a states file handed to you by a + * colleague should not be able to POST to your app because you ran the tool. + */ + allowStateRequests: S.Boolean, }; export class CaptureConfig extends S.Class("CaptureConfig")( @@ -111,19 +328,30 @@ export class CaptureConfig extends S.Class("CaptureConfig")( warmupScroll: true, launchArgs: [], colorScheme: "light", + states: [], + stateTimeout: 30000, + captureRoutes: true, + allowStateRequests: false, }); } export class CaptureReport extends S.Class("CaptureReport")({ timestamp: S.String, + /** Crawled routes only; scripted states are counted separately. */ totalRoutes: S.Number.pipe(S.int(), S.nonNegative()), + totalStates: S.Number.pipe(S.int(), S.nonNegative()), successfulCaptures: S.Number.pipe(S.int(), S.nonNegative()), failedCaptures: S.Number.pipe(S.int(), S.nonNegative()), + /** States whose `precondition` was absent on the loaded page. */ + skippedStates: S.Number.pipe(S.int(), S.nonNegative()), viewports: S.Array(ViewportConfig), results: S.Array( S.Struct({ url: S.String, route: S.String, + state: S.optional(S.String), + stateStatus: S.optional(StateStatus), + failedStepIndex: S.optional(S.Number.pipe(S.int())), screenshots: S.Array(S.String), hasVideo: S.Boolean, error: S.optional(S.String), @@ -146,6 +374,13 @@ type VideoOptionsInput = readonly interactions?: boolean; }; +/** + * A {@link CaptureState}, or the plain object shape a states file decodes + * from. Plain objects are validated through the schema, so a programmatic + * caller gets the same errors a bad file does. + */ +export type CaptureStateInput = CaptureState | Record; + export type CaptureConfigOverrides = Partial<{ outputDir: string; captureVideo: boolean; @@ -162,6 +397,10 @@ export type CaptureConfigOverrides = Partial<{ warmupScroll: boolean; launchArgs: ReadonlyArray; colorScheme: "light" | "dark" | "no-preference"; + states: ReadonlyArray; + stateTimeout: number; + captureRoutes: boolean; + allowStateRequests: boolean; }>; const toViewportInstance = (viewport: ViewportConfigInput): ViewportConfig => @@ -179,6 +418,11 @@ const toVideoOptionsInstance = ( ...(input ?? {}), }); +const decodeCaptureState = S.decodeUnknownSync(CaptureState); + +const toCaptureStateInstance = (input: CaptureStateInput): CaptureState => + input instanceof CaptureState ? input : decodeCaptureState(input); + export const createCaptureConfig = ( overrides: CaptureConfigOverrides = {}, ): CaptureConfig => { @@ -193,11 +437,16 @@ export const createCaptureConfig = ( ? toVideoOptionsInstance(overrides.videoOptions, base.videoOptions) : base.videoOptions; + const states = overrides.states + ? overrides.states.map(toCaptureStateInstance) + : base.states; + return new CaptureConfig({ ...base, ...overrides, viewports, videoOptions, + states, allowedHosts: overrides.allowedHosts ? Array.from(overrides.allowedHosts) : base.allowedHosts, @@ -213,5 +462,8 @@ export const createCaptureConfig = ( ? Array.from(overrides.launchArgs) : base.launchArgs, colorScheme: overrides.colorScheme ?? base.colorScheme, + stateTimeout: overrides.stateTimeout ?? base.stateTimeout, + captureRoutes: overrides.captureRoutes ?? base.captureRoutes, + allowStateRequests: overrides.allowStateRequests ?? base.allowStateRequests, }); }; diff --git a/src/service.ts b/src/service.ts index 4c682e9..0b505f3 100644 --- a/src/service.ts +++ b/src/service.ts @@ -19,7 +19,13 @@ import fs from "node:fs/promises"; import path from "node:path"; import { Context, Effect, Layer, Option, Queue, Ref } from "effect"; import { type Browser, chromium, type Page } from "playwright"; -import { BrowserError, CaptureError, FileSystemError } from "./errors.js"; +import { + BrowserError, + CaptureError, + FileSystemError, + StateCaptureError, + type StateDefinitionError, +} from "./errors.js"; import { createLinkDiscoveryTools } from "./link-discovery.js"; import { generateReports } from "./report.js"; import { @@ -34,13 +40,21 @@ import { import { captureScreenshots } from "./screenshot.js"; import { createHostFilterState, + getCaptureDir, getRouteName, navigationRetryPolicy, normalizeUrl, type QueueTask, type RouteTask, ShutdownSignal, + type StateTask, + stateResultKey, } from "./shared.js"; +import { + createScriptedStateRunner, + INITIAL_STEP_PROGRESS, +} from "./state-script.js"; +import { type ResolvedState, validateStates } from "./states.js"; import { captureVideoForViewport } from "./video.js"; import { performWarmupScroll } from "./warmup.js"; @@ -59,36 +73,50 @@ export class CaptureConfigTag extends Context.Tag("CaptureConfig")< CaptureConfig >() {} +/** A one-line, report-ready rendering of any failure this service can raise. */ +const formatCaptureFailure = (error: unknown): string => { + if (error instanceof FileSystemError) { + return `${error.operation} failed for ${error.path}`; + } + if (error instanceof Error && error.message) return error.message; + return String(error); +}; + +/** + * Creates one capture unit's directory tree. `captureDir` is an opaque prefix: + * a crawled route's own directory, or `/states//` for a scripted + * state. + */ const createDirectories = ( - routeDir: string, + captureDir: string, captureVideo: boolean, ): Effect.Effect => Effect.tryPromise({ try: async () => { - await fs.mkdir(path.join(routeDir, "screenshots", "png", "history"), { + await fs.mkdir(path.join(captureDir, "screenshots", "png", "history"), { recursive: true, }); - await fs.mkdir(path.join(routeDir, "screenshots", "webp", "history"), { + await fs.mkdir(path.join(captureDir, "screenshots", "webp", "history"), { recursive: true, }); - await fs.mkdir(path.join(routeDir, "screenshots", "jpg", "history"), { + await fs.mkdir(path.join(captureDir, "screenshots", "jpg", "history"), { recursive: true, }); if (captureVideo) { - await fs.mkdir(path.join(routeDir, "videos", "high-quality"), { + await fs.mkdir(path.join(captureDir, "videos", "high-quality"), { recursive: true, }); - await fs.mkdir(path.join(routeDir, "videos", "medium-quality"), { + await fs.mkdir(path.join(captureDir, "videos", "medium-quality"), { recursive: true, }); - await fs.mkdir(path.join(routeDir, "videos", "low-quality"), { + await fs.mkdir(path.join(captureDir, "videos", "low-quality"), { recursive: true, }); } }, catch: (error) => new FileSystemError({ - path: routeDir, + path: captureDir, operation: "mkdir", cause: error, }), @@ -145,15 +173,43 @@ export class UICaptureService extends Effect.Service()( menuInteractionSelectors: cfg.menuInteractionSelectors, }); + // Link discovery opens menus so links become *discoverable*; the state + // runner performs a named script so a state becomes *capturable*. Two + // page-manipulation toolkits from one config, neither owning the other. + const { runStateScript, checkPrecondition } = createScriptedStateRunner( + {}, + ); + + /** + * What a scripted state changes about a capture: where it lands, which + * viewports it is valid at, whether video is recorded, and how the video + * context reaches the same state the stills show. + */ + interface StateCaptureContext { + readonly name: string; + readonly viewports: ReadonlyArray; + readonly captureVideo: boolean; + readonly prepare: (page: Page) => Effect.Effect; + } + const capturePage = ( page: Page, url: string, + stateContext?: StateCaptureContext, ): Effect.Effect => Effect.gen(function* () { const route = getRouteName(url); - const routeDir = path.join(cfg.outputDir, route); + const captureDir = getCaptureDir( + cfg.outputDir, + url, + stateContext?.name, + ); + const viewports = stateContext?.viewports ?? cfg.viewports; + const wantVideo = stateContext + ? stateContext.captureVideo + : cfg.captureVideo; - yield* createDirectories(routeDir, cfg.captureVideo); + yield* createDirectories(captureDir, wantVideo); yield* Effect.tryPromise({ try: () => page.waitForLoadState("networkidle"), catch: (error) => @@ -184,7 +240,7 @@ export class UICaptureService extends Effect.Service()( const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); const screenshotResults = yield* Effect.all( - cfg.viewports.map((viewport: ViewportConfig) => + viewports.map((viewport: ViewportConfig) => Effect.gen(function* () { console.log( ` Capturing ${viewport.name} (${viewport.width}x${viewport.height})`, @@ -192,7 +248,7 @@ export class UICaptureService extends Effect.Service()( const screenshots = yield* captureScreenshots( page, viewport, - routeDir, + captureDir, timestamp, { ffmpegPath: cfg.ffmpegPath, @@ -201,19 +257,22 @@ export class UICaptureService extends Effect.Service()( ); const videos = - cfg.captureVideo && browser + wantVideo && browser ? Option.some( yield* captureVideoForViewport( browser, page, viewport, - routeDir, + captureDir, timestamp, { waitTime: cfg.waitTime, ffmpegPath: cfg.ffmpegPath, videoOptions: cfg.videoOptions, colorScheme: cfg.colorScheme, + ...(stateContext + ? { prepare: stateContext.prepare } + : {}), }, ), ) @@ -238,6 +297,8 @@ export class UICaptureService extends Effect.Service()( return new CaptureResult({ url, route, + state: stateContext?.name, + stateStatus: stateContext ? "captured" : undefined, screenshots, videos: Object.keys(videos).length > 0 ? videos : undefined, timestamp: Date.now(), @@ -296,17 +357,238 @@ export class UICaptureService extends Effect.Service()( } }); + /** + * A scripted state is a capture leaf: it never feeds the crawl frontier, + * and it runs in its **own** browser context rather than on the worker's + * long-lived page. + * + * `page.goto` clears neither cookies nor `localStorage`, so reusing the + * worker page would let state N inherit state N-1's client storage and + * make the determinism guarantee aspirational rather than true. Roughly + * 100ms against a multi-second capture buys real isolation. + * + * Every runtime failure is recorded as a `CaptureResult` and swallowed + * here, so the worker pool cannot be disturbed by a bad script. + */ + const processStateTask = ( + task: StateTask, + resolved: ResolvedState, + results: Map, + workerLabel: string, + ): Effect.Effect => + Effect.gen(function* () { + const { state, steps } = resolved; + const budgetMs = state.timeoutMs ?? cfg.stateTimeout; + console.log( + `\n[Worker ${workerLabel}] [State ${state.name}] Capturing: ${task.url}`, + ); + + const progress = yield* Ref.make(INITIAL_STEP_PROGRESS); + + // The script runs once and the viewport loop resizes afterwards, so + // a state whose UI unmounts below a breakpoint must say which + // viewports it is valid at rather than silently shooting the boot + // view and recording it as success. + const stateViewports = state.viewports + ? cfg.viewports.filter((viewport) => + state.viewports?.includes(viewport.name), + ) + : cfg.viewports; + + const usesRequests = steps.some((step) => step.kind === "request"); + const stateCaptureVideo = + cfg.captureVideo && (!usesRequests || state.allowVideoReplay); + if (cfg.captureVideo && !stateCaptureVideo) { + console.log( + ` ! Skipping video for "${state.name}": recording replays the script in a second context, so a non-idempotent request step would seed twice and the video would disagree with the stills (set allowVideoReplay to override)`, + ); + } + + const stateFailure = (message: string, cause: unknown) => + new StateCaptureError({ + state: state.name, + stepIndex: -1, + stepKind: "state", + target: task.url, + message: `state "${state.name}" failed: ${message}`, + cause, + }); + + const prepare = (videoPage: Page) => + runStateScript(videoPage, state.name, steps, progress).pipe( + Effect.mapError( + (error) => + new CaptureError({ + url: task.url, + message: error.message, + cause: error, + }), + ), + ); + + const runInContext = Effect.acquireUseRelease( + Effect.gen(function* () { + if (!browser) { + return yield* Effect.fail( + stateFailure("browser not initialized", null), + ); + } + const browserRef = browser; + const context = yield* Effect.tryPromise({ + try: () => + browserRef.newContext({ colorScheme: cfg.colorScheme }), + catch: (error) => + stateFailure("failed to create a browser context", error), + }); + const page = yield* Effect.tryPromise({ + try: () => context.newPage(), + catch: (error) => + stateFailure("failed to create a page", error), + }); + return { context, page }; + }), + ({ page }) => + Effect.gen(function* () { + yield* Effect.tryPromise({ + try: () => + page + .goto(task.url, { + waitUntil: "networkidle", + timeout: 30000, + }) + .then(() => undefined), + catch: (error) => + stateFailure(`failed to navigate to ${task.url}`, error), + }).pipe(Effect.retry(navigationRetryPolicy)); + + if (state.precondition !== undefined) { + const present = yield* checkPrecondition( + page, + state.precondition, + ); + if (!present) { + console.log( + ` - State "${state.name}" skipped: precondition "${state.precondition}" is not present on ${task.url}`, + ); + results.set( + task.resultKey, + new CaptureResult({ + url: task.url, + route: getRouteName(task.url), + state: state.name, + stateStatus: "skipped", + screenshots: {}, + timestamp: Date.now(), + }), + ); + return; + } + } + + yield* runStateScript(page, state.name, steps, progress); + + const result = yield* capturePage(page, task.url, { + name: state.name, + viewports: stateViewports, + captureVideo: stateCaptureVideo, + prepare, + }).pipe( + Effect.mapError((error) => + stateFailure(formatCaptureFailure(error), error), + ), + ); + + results.set(task.resultKey, result); + }), + ({ context }) => + Effect.tryPromise({ + try: () => context.close(), + catch: () => undefined, + }).pipe(Effect.catchAll(() => Effect.void)), + ); + + yield* runInContext.pipe( + Effect.timeout(budgetMs), + Effect.catchTag("TimeoutException", () => + Effect.gen(function* () { + const at = yield* Ref.get(progress); + return yield* Effect.fail( + new StateCaptureError({ + state: state.name, + stepIndex: at.index, + stepKind: at.kind, + target: at.target, + message: + at.index < 0 + ? `state "${state.name}" timed out after ${budgetMs}ms before its first step completed` + : `state "${state.name}" timed out after ${budgetMs}ms on step ${at.index} (${at.kind} "${at.target}")`, + cause: null, + }), + ); + }), + ), + Effect.catchAll((error) => + Effect.gen(function* () { + const message = formatCaptureFailure(error); + console.error(`[Worker ${workerLabel}] ${message}`); + // An empty state directory is a visible artefact of a state + // that was attempted and did not reach its target — the + // opposite of a state that silently never existed. + yield* createDirectories( + getCaptureDir(cfg.outputDir, task.url, state.name), + false, + ).pipe(Effect.catchAll(() => Effect.void)); + results.set( + task.resultKey, + new CaptureResult({ + url: task.url, + route: getRouteName(task.url), + state: state.name, + stateStatus: "failed", + failedStepIndex: + error instanceof StateCaptureError ? error.stepIndex : -1, + screenshots: {}, + error: message, + timestamp: Date.now(), + }), + ); + }), + ), + ); + }); + const captureWebsite = ( url: string, ): Effect.Effect< Map, - BrowserError | CaptureError | FileSystemError + BrowserError | CaptureError | FileSystemError | StateDefinitionError > => Effect.gen(function* () { console.log("Starting UI capture for:", url); const urlObj = new URL(url); hostFilters.hydrate(urlObj.hostname, cfg.allowedHosts); + // Authoring errors abort before Chromium ever launches: no amount of + // retrying makes a typo'd `extends` resolve. + const resolvedStates = yield* validateStates({ + states: cfg.states, + seedUrl: url, + hostMatchesFilters: (hostname) => + hostFilters.hostMatchesFilters(hostname, cfg.includeSubdomains), + allowStateRequests: cfg.allowStateRequests, + viewportNames: cfg.viewports.map((viewport) => viewport.name), + captureRoutes: cfg.captureRoutes, + }); + + const seedingStates = Array.from(resolvedStates.values()).some( + (entry) => entry.steps.some((step) => step.kind === "request"), + ); + if (seedingStates && cfg.routeConcurrency > 1) { + console.warn( + " ! Some states seed through request steps and workers run in parallel: a fresh browser context cannot un-seed a server. Use idempotent or per-state-keyed seeds, or --concurrency 1.", + ); + } + const results = new Map(); yield* Effect.acquireUseRelease( @@ -323,7 +605,28 @@ export class UICaptureService extends Effect.Service()( ); } - const queueCapacity = Math.max(32, cfg.routeConcurrency * 8); + const stateTasks: StateTask[] = Array.from( + resolvedStates.values(), + (entry) => { + const stateUrl = new URL(entry.url ?? url, url).toString(); + return { + type: "state" as const, + url: stateUrl, + stateName: entry.state.name, + normalizedUrl: normalizeUrl(stateUrl), + resultKey: stateResultKey(stateUrl, entry.state.name), + }; + }, + ); + + // Every state is seeded before a worker starts taking, so the + // queue has to be able to hold them all or `Queue.offer` + // deadlocks against an empty pool. + const queueCapacity = Math.max( + 32, + cfg.routeConcurrency * 8, + stateTasks.length + 1, + ); const taskQueue = yield* Queue.bounded(queueCapacity); const pendingTasks = yield* Ref.make(0); @@ -405,6 +708,20 @@ export class UICaptureService extends Effect.Service()( if (task.type === "shutdown") { return yield* Effect.void; } + if (task.type === "state") { + const resolved = resolvedStates.get(task.stateName); + yield* ( + resolved + ? processStateTask( + task, + resolved, + results, + `#${workerId}`, + ) + : Effect.void + ).pipe(Effect.ensuring(markTaskComplete())); + continue; + } yield* processRouteTask( page, task, @@ -471,7 +788,22 @@ export class UICaptureService extends Effect.Service()( }).pipe(Effect.catchAll(() => Effect.void)), ); - yield* scheduleRoute(url, 0); + // States are seeded alongside the seed route, before any + // worker starts, so their ordering never depends on discovery + // order and a crawl that dies early cannot silently drop them. + if (cfg.captureRoutes) { + yield* scheduleRoute(url, 0); + } + yield* Effect.forEach( + stateTasks, + (stateTask) => + Effect.gen(function* () { + yield* Ref.update(pendingTasks, (count) => count + 1); + yield* Queue.offer(taskQueue, stateTask); + }), + { discard: true }, + ); + const initialPending = yield* Ref.get(pendingTasks); if (initialPending === 0) { yield* signalShutdown(); diff --git a/src/shared.test.ts b/src/shared.test.ts index 0366f01..c78e2d5 100644 --- a/src/shared.test.ts +++ b/src/shared.test.ts @@ -10,13 +10,16 @@ * */ +import path from "node:path"; import { describe, expect, it } from "vitest"; import { canonicalizeHost, computeHostSuffixes, createHostFilterState, + getCaptureDir, getRouteName, normalizeUrl, + stateResultKey, } from "./shared.js"; describe("canonicalizeHost", () => { @@ -136,3 +139,63 @@ describe("getRouteName", () => { expect(getRouteName("https://example.com/A%20B")).toBe("a-20b"); }); }); + +describe("getCaptureDir", () => { + it("is the route directory when no state is named", () => { + // Unchanged from before scripted states existed: a crawled route still + // writes straight into //. + expect(getCaptureDir("out", "https://example.com/blog/post-1")).toBe( + path.join("out", "blog-post-1"), + ); + expect(getCaptureDir("out", "https://example.com/")).toBe( + path.join("out", "root"), + ); + }); + + it("nests a scripted state under the route it belongs to", () => { + expect(getCaptureDir("out", "https://example.com/", "spawn-dialog")).toBe( + path.join("out", "root", "states", "spawn-dialog"), + ); + }); + + it("cannot collide with the route's own screenshot or video trees", () => { + // `states` is a fixed segment and a state name is pattern-constrained to + // [a-z0-9-]+, so no state can land on "screenshots" or "videos". + const routeDir = getCaptureDir("out", "https://example.com/"); + const stateDir = getCaptureDir( + "out", + "https://example.com/", + "screenshots", + ); + expect(stateDir).toBe(path.join(routeDir, "states", "screenshots")); + expect(stateDir).not.toBe(path.join(routeDir, "screenshots")); + }); + + it("keeps two states on the same route in separate directories", () => { + expect(getCaptureDir("out", "https://example.com/", "a")).not.toBe( + getCaptureDir("out", "https://example.com/", "b"), + ); + }); +}); + +describe("stateResultKey", () => { + it("never collides with the route key for the same URL", () => { + const url = "https://example.com/console"; + expect(stateResultKey(url, "safety")).not.toBe(normalizeUrl(url)); + }); + + it("separates two states on one URL, and one state across two URLs", () => { + expect(stateResultKey("https://example.com/", "a")).not.toBe( + stateResultKey("https://example.com/", "b"), + ); + expect(stateResultKey("https://example.com/x", "a")).not.toBe( + stateResultKey("https://example.com/y", "a"), + ); + }); + + it("ignores query and fragment, matching normalizeUrl", () => { + expect(stateResultKey("https://example.com/c?x=1#y", "a")).toBe( + stateResultKey("https://example.com/c", "a"), + ); + }); +}); diff --git a/src/shared.ts b/src/shared.ts index 380b2a2..4d42a44 100644 --- a/src/shared.ts +++ b/src/shared.ts @@ -16,6 +16,7 @@ * */ import { execFile, spawn } from "node:child_process"; +import path from "node:path"; import { promisify } from "node:util"; import { Effect, Schedule } from "effect"; import { FileSystemError } from "./errors.js"; @@ -27,11 +28,24 @@ export type RouteTask = { readonly normalizedUrl: string; }; +/** + * One scripted state, queued as a first-class peer of a route rather than a + * phase bolted onto the end of a crawl: same queue, same worker pool, same + * `--concurrency`, same results map, same report. + */ +export type StateTask = { + readonly type: "state"; + readonly url: string; + readonly stateName: string; + readonly normalizedUrl: string; + readonly resultKey: string; +}; + export type ShutdownTask = { readonly type: "shutdown"; }; -export type QueueTask = RouteTask | ShutdownTask; +export type QueueTask = RouteTask | StateTask | ShutdownTask; export const ShutdownSignal: ShutdownTask = { type: "shutdown" } as const; @@ -210,3 +224,28 @@ export const getRouteName = (url: string): string => { return "invalid-url"; } }; + +/** + * Keys a scripted-state result so it can never overwrite the route result for + * the same URL, nor another state's result on that URL. + */ +export const stateResultKey = (url: string, stateName: string): string => + `${normalizeUrl(url)}::state=${stateName}`; + +/** + * Where one capture unit's `screenshots/` and `videos/` live. + * + * A scripted state nests under the route it belongs to + * (`/states//`) rather than encoding both axes in one slug: the + * route/state relationship stays visible in the tree, and a future capture + * axis does not have to fight a separator convention. Everything downstream + * treats this as an opaque prefix. + */ +export const getCaptureDir = ( + outputDir: string, + url: string, + stateName?: string, +): string => { + const routeDir = path.join(outputDir, getRouteName(url)); + return stateName ? path.join(routeDir, "states", stateName) : routeDir; +}; diff --git a/src/state-plan.test.ts b/src/state-plan.test.ts new file mode 100644 index 0000000..154c456 --- /dev/null +++ b/src/state-plan.test.ts @@ -0,0 +1,256 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + */ + +import { Schema as S } from "@effect/schema"; +import { describe, expect, it } from "vitest"; +import { CaptureStep } from "./schemas.js"; +import { + COUNT_POLL_INTERVAL_MS, + DEFAULT_STEP_TIMEOUT_MS, + describeStep, + isExpectedStatus, + planStep, + resolveRequestUrl, + StepPlanError, +} from "./state-plan.js"; + +const decodeStep = S.decodeUnknownSync(CaptureStep); +const PAGE = "https://app.example.com/console?mode=advanced"; +const ctx = { pageUrl: PAGE }; + +describe("planStep — waitFor", () => { + it("defaults to waiting for the first match to become visible", () => { + const planned = planStep( + decodeStep({ kind: "waitFor", selector: "#d" }), + ctx, + ); + expect(planned.plan).toEqual({ + op: "waitForSelector", + selector: "#d", + state: "visible", + timeoutMs: DEFAULT_STEP_TIMEOUT_MS, + }); + expect(planned.optional).toBe(false); + expect(planned.settleMs).toBe(0); + expect(planned.label).toBe('waitFor "#d"'); + }); + + it("honours an explicit DOM state and per-step timeout", () => { + const planned = planStep( + decodeStep({ + kind: "waitFor", + selector: ".spinner", + state: "hidden", + timeoutMs: 20000, + }), + ctx, + ); + expect(planned.plan).toMatchObject({ state: "hidden", timeoutMs: 20000 }); + }); + + it("switches to a counting plan when minCount is set", () => { + const planned = planStep( + decodeStep({ kind: "waitFor", selector: ".fleet-row", minCount: 6 }), + ctx, + ); + expect(planned.plan).toEqual({ + op: "waitForCount", + selector: ".fleet-row", + minCount: 6, + state: "visible", + timeoutMs: DEFAULT_STEP_TIMEOUT_MS, + pollMs: COUNT_POLL_INTERVAL_MS, + }); + }); +}); + +describe("planStep — actions", () => { + it("defaults click to the first match", () => { + expect( + planStep(decodeStep({ kind: "click", selector: "button" }), ctx).plan, + ).toEqual({ + op: "click", + selector: "button", + nth: 0, + timeoutMs: DEFAULT_STEP_TIMEOUT_MS, + }); + }); + + it("carries nth through", () => { + expect( + planStep(decodeStep({ kind: "click", selector: ".row", nth: 2 }), ctx) + .plan, + ).toMatchObject({ nth: 2 }); + }); + + it("plans fill and select verbatim", () => { + expect( + planStep( + decodeStep({ kind: "fill", selector: "#callsign", value: "RESQ-01" }), + ctx, + ).plan, + ).toMatchObject({ op: "fill", value: "RESQ-01" }); + expect( + planStep( + decodeStep({ + kind: "select", + selector: "#drone-type", + values: ["fixed-wing"], + }), + ctx, + ).plan, + ).toMatchObject({ op: "select", values: ["fixed-wing"] }); + }); + + it("sends a keypress to the keyboard when no selector is given", () => { + const planned = planStep(decodeStep({ kind: "press", key: "Escape" }), ctx); + expect(planned.plan).toMatchObject({ op: "press", selector: undefined }); + expect(planned.target).toBe("Escape"); + }); + + it("names both key and selector when the press is targeted", () => { + expect( + planStep( + decodeStep({ kind: "press", key: "Enter", selector: "#form" }), + ctx, + ).target, + ).toBe("Enter @ #form"); + }); + + it("plans a bare wait as a sleep", () => { + expect(planStep(decodeStep({ kind: "wait", ms: 400 }), ctx).plan).toEqual({ + op: "sleep", + ms: 400, + }); + }); + + it("defaults reload to networkidle", () => { + expect(planStep(decodeStep({ kind: "reload" }), ctx).plan).toMatchObject({ + op: "reload", + waitUntil: "networkidle", + }); + }); +}); + +describe("planStep — modifiers", () => { + it("carries optional and settleMs onto every kind", () => { + const planned = planStep( + decodeStep({ + kind: "click", + selector: "#consent", + optional: true, + settleMs: 400, + }), + ctx, + ); + expect(planned.optional).toBe(true); + expect(planned.settleMs).toBe(400); + }); + + it("prefers a step timeout over the caller's action default", () => { + expect( + planStep(decodeStep({ kind: "click", selector: "b", timeoutMs: 900 }), { + ...ctx, + defaultTimeoutMs: 12000, + }).plan, + ).toMatchObject({ timeoutMs: 900 }); + expect( + planStep(decodeStep({ kind: "click", selector: "b" }), { + ...ctx, + defaultTimeoutMs: 12000, + }).plan, + ).toMatchObject({ timeoutMs: 12000 }); + }); +}); + +describe("planStep — request", () => { + it("resolves a path against the page URL, making it same-origin", () => { + const planned = planStep( + decodeStep({ kind: "request", method: "POST", path: "/api/sim/seed" }), + ctx, + ); + expect(planned.plan).toMatchObject({ + op: "request", + method: "POST", + url: "https://app.example.com/api/sim/seed", + hasJson: false, + }); + }); + + it("marks a body as present only when json is supplied", () => { + expect( + planStep( + decodeStep({ + kind: "request", + method: "POST", + path: "/api/seed", + json: { preset: "multidomain", count: 6 }, + expectStatus: 201, + }), + ctx, + ).plan, + ).toMatchObject({ hasJson: true, expectStatus: 201 }); + }); + + it("copies headers rather than aliasing the step", () => { + const step = decodeStep({ + kind: "request", + method: "GET", + path: "/api/x", + headers: { "x-test": "1" }, + }); + const plan = planStep(step, ctx).plan; + expect(plan).toMatchObject({ headers: { "x-test": "1" } }); + }); + + it("rejects a non-http scheme", () => { + expect(() => resolveRequestUrl("file:///etc/passwd", PAGE)).toThrow( + StepPlanError, + ); + }); + + it("keeps an absolute URL as written, for the host filter to judge", () => { + expect(resolveRequestUrl("https://other.test/api", PAGE).hostname).toBe( + "other.test", + ); + }); +}); + +describe("describeStep", () => { + it("quotes selectors and leaves verbs bare", () => { + expect(describeStep(decodeStep({ kind: "click", selector: "#a" }))).toBe( + 'click "#a"', + ); + expect( + describeStep( + decodeStep({ kind: "request", method: "POST", path: "/api/seed" }), + ), + ).toBe("request POST /api/seed"); + expect(describeStep(decodeStep({ kind: "wait", ms: 250 }))).toBe( + "wait 250ms", + ); + }); +}); + +describe("isExpectedStatus", () => { + it("accepts any 2xx by default and nothing else", () => { + expect(isExpectedStatus(200, undefined)).toBe(true); + expect(isExpectedStatus(204, undefined)).toBe(true); + expect(isExpectedStatus(302, undefined)).toBe(false); + expect(isExpectedStatus(500, undefined)).toBe(false); + }); + + it("demands an exact match when expectStatus is set", () => { + expect(isExpectedStatus(201, 201)).toBe(true); + expect(isExpectedStatus(200, 201)).toBe(false); + }); +}); diff --git a/src/state-plan.ts b/src/state-plan.ts new file mode 100644 index 0000000..0eb15be --- /dev/null +++ b/src/state-plan.ts @@ -0,0 +1,301 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/** + * What a step *means*, decided without a browser. + * + * This module is deliberately free of Playwright: it turns a declarative + * {@link CaptureStep} into a {@link StepPlan}, which is still plain data. + * `state-script.ts` is the only place that drives a `Page`, so the whole + * vocabulary — defaults, timeout precedence, URL resolution, the human-facing + * labels in failure messages — is testable without launching Chromium. + */ + +import type { CaptureStep } from "./schemas.js"; + +/** Action default when a step does not set `timeoutMs`. */ +export const DEFAULT_STEP_TIMEOUT_MS = 5000; + +/** How often `waitFor` with `minCount` re-counts matches. */ +export const COUNT_POLL_INTERVAL_MS = 100; + +export type ElementState = "visible" | "hidden" | "attached" | "detached"; + +export type StepPlan = + | { + readonly op: "waitForSelector"; + readonly selector: string; + readonly state: ElementState; + readonly timeoutMs: number; + } + | { + readonly op: "waitForCount"; + readonly selector: string; + readonly minCount: number; + readonly state: ElementState; + readonly timeoutMs: number; + readonly pollMs: number; + } + | { readonly op: "sleep"; readonly ms: number } + | { + readonly op: "click"; + readonly selector: string; + readonly nth: number; + readonly timeoutMs: number; + } + | { + readonly op: "fill"; + readonly selector: string; + readonly value: string; + readonly timeoutMs: number; + } + | { + readonly op: "select"; + readonly selector: string; + readonly values: readonly string[]; + readonly timeoutMs: number; + } + | { + readonly op: "press"; + readonly key: string; + readonly selector: string | undefined; + readonly timeoutMs: number; + } + | { + readonly op: "request"; + readonly method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; + readonly url: string; + readonly json: unknown; + readonly hasJson: boolean; + readonly headers: Record; + readonly expectStatus: number | undefined; + readonly timeoutMs: number; + } + | { + readonly op: "reload"; + readonly waitUntil: + | "load" + | "domcontentloaded" + | "networkidle" + | "commit"; + readonly timeoutMs: number; + }; + +export interface PlannedStep { + readonly plan: StepPlan; + /** Pause after the step succeeds. */ + readonly settleMs: number; + /** Log and continue instead of failing the state. */ + readonly optional: boolean; + /** Step kind, carried into {@link StateCaptureError}. */ + readonly kind: CaptureStep["kind"]; + /** The selector, key, path or duration the step acts on. */ + readonly target: string; + /** `click "#spawn"` — used verbatim in logs and failure messages. */ + readonly label: string; +} + +export interface PlanContext { + /** The page's current URL; `request` paths resolve against it. */ + readonly pageUrl: string; + /** Action default when a step omits `timeoutMs`. */ + readonly defaultTimeoutMs?: number; +} + +/** + * Thrown for a step that cannot be turned into a plan at all — today, only a + * `request` path that will not resolve into a URL. Callers map it onto a + * `StateDefinitionError` (pre-launch) or a `StateCaptureError` (at runtime). + */ +export class StepPlanError extends Error { + readonly kind: CaptureStep["kind"]; + readonly target: string; + + constructor(kind: CaptureStep["kind"], target: string, message: string) { + super(message); + this.name = "StepPlanError"; + this.kind = kind; + this.target = target; + } +} + +/** The value a failure message quotes for each step kind. */ +export const stepTarget = (step: CaptureStep): string => { + switch (step.kind) { + case "waitFor": + case "click": + case "fill": + case "select": + return step.selector; + case "wait": + return `${step.ms}ms`; + case "press": + return step.selector ? `${step.key} @ ${step.selector}` : step.key; + case "request": + return `${step.method} ${step.path}`; + case "reload": + return step.waitUntil; + } +}; + +/** `click "#spawn"` / `request POST /api/sim/seed`. */ +export const describeStep = (step: CaptureStep): string => + step.kind === "request" || step.kind === "wait" || step.kind === "reload" + ? `${step.kind} ${stepTarget(step)}` + : `${step.kind} "${stepTarget(step)}"`; + +/** + * Resolves a `request` path against the page URL. + * + * A path is same-origin by construction, which removes a whole class of + * misconfiguration before validation has to catch it; an absolute URL is + * still accepted so a states file can be explicit, and is then subjected to + * the crawler's host filter by the caller. + */ +export const resolveRequestUrl = ( + requestPath: string, + pageUrl: string, +): URL => { + let resolved: URL; + try { + resolved = new URL(requestPath, pageUrl); + } catch { + throw new StepPlanError( + "request", + requestPath, + `cannot resolve "${requestPath}" against "${pageUrl}"`, + ); + } + if (resolved.protocol !== "http:" && resolved.protocol !== "https:") { + throw new StepPlanError( + "request", + requestPath, + `unsupported scheme "${resolved.protocol}" (http and https only)`, + ); + } + return resolved; +}; + +/** + * Turns one declarative step into the operation a driver performs, applying + * the timeout precedence: step `timeoutMs`, then the action default. + */ +export const planStep = (step: CaptureStep, ctx: PlanContext): PlannedStep => { + const timeoutMs = + step.timeoutMs ?? ctx.defaultTimeoutMs ?? DEFAULT_STEP_TIMEOUT_MS; + const common = { + settleMs: step.settleMs ?? 0, + optional: step.optional, + kind: step.kind, + target: stepTarget(step), + label: describeStep(step), + } as const; + + switch (step.kind) { + case "waitFor": + return { + ...common, + plan: + step.minCount === undefined + ? { + op: "waitForSelector", + selector: step.selector, + state: step.state, + timeoutMs, + } + : { + op: "waitForCount", + selector: step.selector, + minCount: step.minCount, + state: step.state, + timeoutMs, + pollMs: COUNT_POLL_INTERVAL_MS, + }, + }; + case "wait": + return { ...common, plan: { op: "sleep", ms: step.ms } }; + case "click": + return { + ...common, + plan: { + op: "click", + selector: step.selector, + nth: step.nth ?? 0, + timeoutMs, + }, + }; + case "fill": + return { + ...common, + plan: { + op: "fill", + selector: step.selector, + value: step.value, + timeoutMs, + }, + }; + case "select": + return { + ...common, + plan: { + op: "select", + selector: step.selector, + values: [...step.values], + timeoutMs, + }, + }; + case "press": + return { + ...common, + plan: { + op: "press", + key: step.key, + selector: step.selector, + timeoutMs, + }, + }; + case "request": + return { + ...common, + plan: { + op: "request", + method: step.method, + url: resolveRequestUrl(step.path, ctx.pageUrl).toString(), + json: step.json, + hasJson: step.json !== undefined, + headers: { ...(step.headers ?? {}) }, + expectStatus: step.expectStatus, + timeoutMs, + }, + }; + case "reload": + return { + ...common, + plan: { op: "reload", waitUntil: step.waitUntil, timeoutMs }, + }; + } +}; + +/** True when a response satisfies the step's expectation. */ +export const isExpectedStatus = ( + status: number, + expectStatus: number | undefined, +): boolean => + expectStatus === undefined + ? status >= 200 && status < 300 + : status === expectStatus; diff --git a/src/state-script.test.ts b/src/state-script.test.ts new file mode 100644 index 0000000..d60052a --- /dev/null +++ b/src/state-script.test.ts @@ -0,0 +1,641 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + */ + +import { Schema as S } from "@effect/schema"; +import { Cause, Effect, Exit, Ref } from "effect"; +import type { Page } from "playwright"; +import { describe, expect, it, vi } from "vitest"; +import type { StateCaptureError } from "./errors.js"; +import { + type CaptureStep, + CaptureStep as CaptureStepSchema, +} from "./schemas.js"; +import { + createScriptedStateRunner, + INITIAL_STEP_PROGRESS, + type ScriptedStateRunnerOptions, + type StepProgress, +} from "./state-script.js"; + +const decodeStep = S.decodeUnknownSync(CaptureStepSchema); + +/** + * The step engine is exercised against a hand-rolled `Page` rather than a real + * browser. That is only possible because `state-plan.ts` decides what a step + * *means* without Playwright, so this file only has to prove the driver calls + * the right API with the right arguments and maps every rejection onto a + * `StateCaptureError` naming the step. + */ +interface ElementBehaviour { + /** How many elements the selector matches. */ + readonly count?: number; + /** Successive `count()` answers; the last repeats once exhausted. */ + readonly counts?: readonly number[]; + /** Per-index visibility; missing indices are visible. */ + readonly visible?: readonly boolean[]; + /** When set, the corresponding action rejects with this message. */ + readonly waitFor?: string; + readonly click?: string; + readonly fill?: string; + readonly select?: string; + readonly press?: string; +} + +interface FakePageOptions { + readonly url?: string; + readonly elements?: Record; + readonly reload?: string; + readonly request?: { + readonly status?: number; + readonly body?: string; + readonly reject?: string; + }; +} + +interface RecordedCall { + readonly op: string; + readonly [key: string]: unknown; +} + +const createFakePage = ( + options: FakePageOptions = {}, +): { page: Page; calls: RecordedCall[] } => { + const calls: RecordedCall[] = []; + const pollCursor = new Map(); + + const behaviourFor = (selector: string): ElementBehaviour => + options.elements?.[selector] ?? {}; + + const reject = (message: string) => Promise.reject(new Error(message)); + + const element = (selector: string, index: number) => ({ + waitFor: (opts: { state: string; timeout: number }) => { + calls.push({ op: "waitFor", selector, index, ...opts }); + const failure = behaviourFor(selector).waitFor; + return failure ? reject(failure) : Promise.resolve(); + }, + click: (opts: { timeout: number }) => { + calls.push({ op: "click", selector, index, ...opts }); + const failure = behaviourFor(selector).click; + return failure ? reject(failure) : Promise.resolve(); + }, + fill: (value: string, opts: { timeout: number }) => { + calls.push({ op: "fill", selector, index, value, ...opts }); + const failure = behaviourFor(selector).fill; + return failure ? reject(failure) : Promise.resolve(); + }, + selectOption: (values: string[], opts: { timeout: number }) => { + calls.push({ op: "select", selector, index, values, ...opts }); + const failure = behaviourFor(selector).select; + return failure ? reject(failure) : Promise.resolve(values); + }, + press: (key: string, opts: { timeout: number }) => { + calls.push({ op: "press", selector, index, key, ...opts }); + const failure = behaviourFor(selector).press; + return failure ? reject(failure) : Promise.resolve(); + }, + isVisible: () => { + const visible = behaviourFor(selector).visible?.[index] ?? true; + return Promise.resolve(visible); + }, + }); + + const locator = (selector: string) => ({ + first: () => element(selector, 0), + nth: (index: number) => element(selector, index), + count: () => { + const behaviour = behaviourFor(selector); + if (behaviour.counts) { + const cursor = pollCursor.get(selector) ?? 0; + pollCursor.set(selector, cursor + 1); + const value = + behaviour.counts[Math.min(cursor, behaviour.counts.length - 1)] ?? 0; + calls.push({ op: "count", selector, value }); + return Promise.resolve(value); + } + const value = behaviour.count ?? 1; + calls.push({ op: "count", selector, value }); + return Promise.resolve(value); + }, + }); + + const page = { + url: () => options.url ?? "https://app.example.com/console", + locator, + keyboard: { + press: (key: string) => { + calls.push({ op: "keyboard.press", key }); + return Promise.resolve(); + }, + }, + reload: (opts: { waitUntil: string; timeout: number }) => { + calls.push({ op: "reload", ...opts }); + return options.reload + ? reject(options.reload) + : Promise.resolve(undefined); + }, + waitForTimeout: (ms: number) => { + calls.push({ op: "waitForTimeout", ms }); + return new Promise((resolve) => setTimeout(resolve, ms)); + }, + request: { + fetch: (url: string, opts: Record) => { + calls.push({ op: "request", url, ...opts }); + if (options.request?.reject) return reject(options.request.reject); + return Promise.resolve({ + status: () => options.request?.status ?? 200, + text: () => Promise.resolve(options.request?.body ?? ""), + }); + }, + }, + // The vocabulary admits no `evaluate` step and `minCount` polls + // `locator.count()` from Node, so no user-supplied code — and in fact no + // code at all — ever crosses into the page. + evaluate: () => { + throw new Error("the state runner must never evaluate in-page code"); + }, + } as unknown as Page; + + return { page, calls }; +}; + +const run = async ( + page: Page, + steps: readonly unknown[], + options: ScriptedStateRunnerOptions = {}, +): Promise<{ + readonly exit: Exit.Exit; + readonly progress: StepProgress; +}> => { + const runner = createScriptedStateRunner(options); + const progress = Effect.runSync(Ref.make(INITIAL_STEP_PROGRESS)); + const decoded: CaptureStep[] = steps.map((step) => decodeStep(step)); + const exit = await Effect.runPromiseExit( + runner.runStateScript(page, "demo", decoded, progress), + ); + return { exit, progress: Effect.runSync(Ref.get(progress)) }; +}; + +const failureOf = ( + exit: Exit.Exit, +): StateCaptureError => { + if (Exit.isSuccess(exit)) throw new Error("expected the state to fail"); + const failure = Cause.failureOption(exit.cause); + if (failure._tag === "None") throw new Error("expected a typed failure"); + return failure.value; +}; + +const callsOfKind = (calls: readonly RecordedCall[], op: string) => + calls.filter((call) => call.op === op); + +describe("runStateScript — waitFor", () => { + it("waits for the first match in the requested DOM state", async () => { + const { page, calls } = createFakePage(); + const { exit } = await run(page, [ + { kind: "waitFor", selector: "dialog#spawn", state: "visible" }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(calls).toEqual([ + { + op: "waitFor", + selector: "dialog#spawn", + index: 0, + state: "visible", + timeout: 5000, + }, + ]); + }); + + it("fails the state, naming the step, when the selector never appears", async () => { + const { page } = createFakePage({ + elements: { "#never": { waitFor: "Timeout 5000ms exceeded." } }, + }); + const { exit } = await run(page, [ + { kind: "click", selector: "#open" }, + { kind: "waitFor", selector: "#never", timeoutMs: 250 }, + ]); + const error = failureOf(exit); + expect(error._tag).toBe("StateCaptureError"); + expect(error.stepIndex).toBe(1); + expect(error.stepKind).toBe("waitFor"); + expect(error.target).toBe("#never"); + expect(error.message).toContain('state "demo" failed at step 1'); + expect(error.message).toContain('waitFor "#never"'); + expect(error.message).toContain("never became visible within 250ms"); + }); + + it("passes a hidden-state wait straight through", async () => { + const { page, calls } = createFakePage(); + await run(page, [ + { kind: "waitFor", selector: ".spinner", state: "hidden" }, + ]); + expect(calls[0]).toMatchObject({ state: "hidden" }); + }); +}); + +describe("runStateScript — waitFor with minCount", () => { + it("polls locator.count() until enough elements match", async () => { + const { page, calls } = createFakePage({ + elements: { ".fleet-row": { counts: [0, 2, 6] } }, + }); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".fleet-row", minCount: 6, timeoutMs: 4000 }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(callsOfKind(calls, "count")).toHaveLength(3); + expect(callsOfKind(calls, "waitForTimeout")).toHaveLength(2); + }); + + it("reports how many it found, not merely that it timed out", async () => { + const { page } = createFakePage({ + elements: { ".fleet-row": { count: 2 } }, + }); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".fleet-row", minCount: 6, timeoutMs: 150 }, + ]); + const error = failureOf(exit); + expect(error.message).toContain('expected >=6 matching "visible", found 2'); + }); + + it("counts only visible matches when the requested state is visible", async () => { + const { page } = createFakePage({ + elements: { ".row": { count: 4, visible: [true, false, false, true] } }, + }); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", minCount: 3, timeoutMs: 150 }, + ]); + expect(failureOf(exit).message).toContain("found 2"); + }); + + it("counts every match when the requested state is attached", async () => { + const { page } = createFakePage({ + elements: { ".row": { count: 4, visible: [true, false, false, true] } }, + }); + const { exit } = await run(page, [ + { + kind: "waitFor", + selector: ".row", + minCount: 3, + state: "attached", + timeoutMs: 150, + }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + }); +}); + +describe("runStateScript — wait", () => { + it("sleeps without touching the page", async () => { + const { page, calls } = createFakePage(); + const started = Date.now(); + const { exit } = await run(page, [{ kind: "wait", ms: 40 }]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(Date.now() - started).toBeGreaterThanOrEqual(30); + expect(calls).toEqual([]); + }); +}); + +describe("runStateScript — click", () => { + it("clicks the first match by default", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "click", selector: "[data-testid='spawn']" }]); + expect(calls[0]).toMatchObject({ op: "click", index: 0, timeout: 5000 }); + }); + + it("clicks the nth match when asked", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "click", selector: ".fleet-row", nth: 2 }]); + expect(calls[0]).toMatchObject({ op: "click", index: 2 }); + }); + + it("fails the state when the element is not actionable", async () => { + const { page } = createFakePage({ + elements: { "#gone": { click: "element is not attached to the DOM" } }, + }); + const error = failureOf( + (await run(page, [{ kind: "click", selector: "#gone" }])).exit, + ); + expect(error.stepKind).toBe("click"); + expect(error.message).toContain("not attached to the DOM"); + }); +}); + +describe("runStateScript — fill", () => { + it("fills the first match", async () => { + const { page, calls } = createFakePage(); + await run(page, [ + { kind: "fill", selector: "#callsign", value: "RESQ-01" }, + ]); + expect(calls[0]).toMatchObject({ op: "fill", value: "RESQ-01" }); + }); + + it("fails the state on a readonly input", async () => { + const { page } = createFakePage({ + elements: { "#ro": { fill: "Element is not editable" } }, + }); + const error = failureOf( + (await run(page, [{ kind: "fill", selector: "#ro", value: "x" }])).exit, + ); + expect(error.stepKind).toBe("fill"); + expect(error.message).toContain("not editable"); + }); +}); + +describe("runStateScript — select", () => { + it("drives a native select with the requested values", async () => { + const { page, calls } = createFakePage(); + await run(page, [ + { kind: "select", selector: "#drone-type", values: ["fixed-wing"] }, + ]); + expect(calls[0]).toMatchObject({ op: "select", values: ["fixed-wing"] }); + }); + + it("fails the state when the option does not exist", async () => { + const { page } = createFakePage({ + elements: { "#t": { select: "did not find some options" } }, + }); + const error = failureOf( + (await run(page, [{ kind: "select", selector: "#t", values: ["nope"] }])) + .exit, + ); + expect(error.stepKind).toBe("select"); + expect(error.message).toContain("did not find some options"); + }); +}); + +describe("runStateScript — press", () => { + it("sends an untargeted key to the keyboard", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "press", key: "Escape" }]); + expect(calls).toEqual([{ op: "keyboard.press", key: "Escape" }]); + }); + + it("sends a targeted key to the element", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "press", key: "Enter", selector: "#form" }]); + expect(calls[0]).toMatchObject({ + op: "press", + selector: "#form", + key: "Enter", + }); + }); + + it("fails the state and names both key and selector", async () => { + const { page } = createFakePage({ + elements: { "#form": { press: "Timeout exceeded" } }, + }); + const error = failureOf( + (await run(page, [{ kind: "press", key: "Enter", selector: "#form" }])) + .exit, + ); + expect(error.target).toBe("Enter @ #form"); + }); +}); + +describe("runStateScript — reload", () => { + it("reloads with the configured wait condition", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "reload" }]); + expect(calls[0]).toMatchObject({ op: "reload", waitUntil: "networkidle" }); + }); + + it("fails the state when the reload does not settle", async () => { + const { page } = createFakePage({ reload: "Navigation timeout" }); + const error = failureOf((await run(page, [{ kind: "reload" }])).exit); + expect(error.stepKind).toBe("reload"); + expect(error.message).toContain("Navigation timeout"); + }); +}); + +describe("runStateScript — request", () => { + it("resolves the path against the page and sends the JSON body", async () => { + const { page, calls } = createFakePage({ + url: "https://app.example.com/console", + request: { status: 201 }, + }); + const { exit } = await run(page, [ + { + kind: "request", + method: "POST", + path: "/api/sim/seed", + json: { preset: "multidomain", count: 6 }, + expectStatus: 201, + }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(calls[0]).toMatchObject({ + op: "request", + url: "https://app.example.com/api/sim/seed", + method: "POST", + data: { preset: "multidomain", count: 6 }, + }); + }); + + it("omits the body entirely when no json is supplied", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "request", method: "GET", path: "/api/ping" }]); + expect(calls[0]).not.toHaveProperty("data"); + }); + + it("forwards custom headers", async () => { + const { page, calls } = createFakePage(); + await run(page, [ + { + kind: "request", + method: "GET", + path: "/api/ping", + headers: { "x-test": "1" }, + }, + ]); + expect(calls[0]).toMatchObject({ headers: { "x-test": "1" } }); + }); + + it("fails the state on a non-2xx, quoting a truncated body", async () => { + const { page } = createFakePage({ + request: { status: 500, body: "x".repeat(500) }, + }); + const error = failureOf( + ( + await run(page, [ + { kind: "request", method: "POST", path: "/api/seed" }, + ]) + ).exit, + ); + expect(error.stepKind).toBe("request"); + expect(error.target).toBe("POST /api/seed"); + expect(error.message).toContain("expected 2xx, got 500"); + // A seed that silently 500s and then screenshots the empty state is + // exactly the coverage lie this feature exists to eliminate. + expect(error.message).toContain("xxx"); + expect(error.message.length).toBeLessThan(400); + }); + + it("fails the state when the status is not the expected one", async () => { + const { page } = createFakePage({ request: { status: 200 } }); + const error = failureOf( + ( + await run(page, [ + { + kind: "request", + method: "POST", + path: "/api/seed", + expectStatus: 201, + }, + ]) + ).exit, + ); + expect(error.message).toContain("expected 201, got 200"); + }); + + it("fails the state when the transport itself fails", async () => { + const { page } = createFakePage({ request: { reject: "socket hang up" } }); + const error = failureOf( + (await run(page, [{ kind: "request", method: "GET", path: "/api/x" }])) + .exit, + ); + expect(error.message).toContain("socket hang up"); + }); + + it("fails the state when the path cannot be planned into a URL", async () => { + const { page } = createFakePage(); + const { exit } = await run(page, [ + { kind: "request", method: "GET", path: "file:///etc/passwd" }, + ]); + const error = failureOf(exit); + expect(error.stepIndex).toBe(0); + expect(error.stepKind).toBe("request"); + expect(error.message).toContain("unsupported scheme"); + }); +}); + +describe("runStateScript — modifiers", () => { + it("logs and skips an optional step instead of failing the state", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { page, calls } = createFakePage({ + elements: { "#banner": { click: "no such element" } }, + }); + const { exit } = await run(page, [ + { kind: "click", selector: "#banner", optional: true }, + { kind: "click", selector: "#next" }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(callsOfKind(calls, "click")).toHaveLength(2); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("(optional) — continuing"), + ); + warn.mockRestore(); + }); + + it("pauses for settleMs after the step succeeds", async () => { + const { page } = createFakePage(); + const started = Date.now(); + await run(page, [{ kind: "click", selector: "#a", settleMs: 60 }]); + expect(Date.now() - started).toBeGreaterThanOrEqual(45); + }); + + it("still settles after an optional step was skipped", () => { + // The pause describes the page, not the outcome, so it runs either way; + // what must not happen is the state failing. + return (async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { page } = createFakePage({ + elements: { "#a": { click: "nope" } }, + }); + const started = Date.now(); + const { exit } = await run(page, [ + { kind: "click", selector: "#a", optional: true, settleMs: 60 }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + expect(Date.now() - started).toBeGreaterThanOrEqual(45); + warn.mockRestore(); + })(); + }); + + it("applies the caller's action default when a step omits timeoutMs", async () => { + const { page, calls } = createFakePage(); + await run(page, [{ kind: "click", selector: "#a" }], { + defaultStepTimeoutMs: 11000, + }); + expect(calls[0]).toMatchObject({ timeout: 11000 }); + }); +}); + +describe("runStateScript — sequencing and progress", () => { + it("runs steps in order and stops at the first hard failure", async () => { + const { page, calls } = createFakePage({ + elements: { "#b": { click: "boom" } }, + }); + const { exit } = await run(page, [ + { kind: "click", selector: "#a" }, + { kind: "click", selector: "#b" }, + { kind: "click", selector: "#c" }, + ]); + expect(Exit.isFailure(exit)).toBe(true); + expect(callsOfKind(calls, "click").map((call) => call.selector)).toEqual([ + "#a", + "#b", + ]); + }); + + it("leaves the progress ref on the step that was actually running", async () => { + // This is what turns "state timed out" into "hung on step 2 + // (waitFor .fleet-row)" when the whole-state budget expires. + const { page } = createFakePage({ + elements: { ".fleet-row": { waitFor: "Timeout" } }, + }); + const { progress } = await run(page, [ + { kind: "click", selector: "#a" }, + { kind: "reload" }, + { kind: "waitFor", selector: ".fleet-row", timeoutMs: 100 }, + ]); + expect(progress).toEqual({ + index: 2, + kind: "waitFor", + target: ".fleet-row", + }); + }); + + it("starts from a whole-state progress marker before any step runs", async () => { + const { page } = createFakePage(); + const { progress } = await run(page, []); + expect(progress).toEqual(INITIAL_STEP_PROGRESS); + expect(INITIAL_STEP_PROGRESS.index).toBe(-1); + expect(INITIAL_STEP_PROGRESS.kind).toBe("state"); + }); +}); + +describe("checkPrecondition", () => { + it("is true when the selector is present", async () => { + const { page, calls } = createFakePage(); + const runner = createScriptedStateRunner({ preconditionTimeoutMs: 1234 }); + await expect( + Effect.runPromise(runner.checkPrecondition(page, "[data-advanced]")), + ).resolves.toBe(true); + expect(calls[0]).toMatchObject({ + op: "waitFor", + state: "visible", + timeout: 1234, + }); + }); + + it("is false — not an error — when the selector is absent", async () => { + // "This state does not exist here" is a different event from "this + // state's script is broken", and only the second is a failure. + const { page } = createFakePage({ + elements: { "[data-advanced]": { waitFor: "Timeout" } }, + }); + const runner = createScriptedStateRunner({ preconditionTimeoutMs: 50 }); + await expect( + Effect.runPromise(runner.checkPrecondition(page, "[data-advanced]")), + ).resolves.toBe(false); + }); +}); diff --git a/src/state-script.ts b/src/state-script.ts new file mode 100644 index 0000000..8300577 --- /dev/null +++ b/src/state-script.ts @@ -0,0 +1,346 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/** + * The scripted-state driver: the only place a `Page` is touched on behalf of a + * states file. What a step *means* is decided in `state-plan.ts`, which has no + * Playwright import, so the vocabulary is testable without a browser. + * + * No user-supplied code ever crosses into the page. There is no `evaluate` + * step, and even `waitFor` with `minCount` polls `locator.count()` from here + * rather than injecting a predicate, so the invariant holds literally. + */ + +import { Effect, Ref } from "effect"; +import type { Locator, Page } from "playwright"; +import { StateCaptureError } from "./errors.js"; +import type { CaptureStep } from "./schemas.js"; +import { + type ElementState, + isExpectedStatus, + type PlannedStep, + planStep, + type StepPlan, + StepPlanError, +} from "./state-plan.js"; + +/** How much of a failing response body a failure message quotes. */ +const RESPONSE_BODY_EXCERPT = 200; + +/** Default budget for the `precondition` probe on the freshly loaded page. */ +const DEFAULT_PRECONDITION_TIMEOUT_MS = 5000; + +/** + * The step a state is currently on. Held in a `Ref` so the whole-state timeout + * can name the step that was actually running — "hung on step 4 + * (waitFor .fleet-row)" rather than the useless "state timed out". + */ +export interface StepProgress { + readonly index: number; + readonly kind: string; + readonly target: string; +} + +/** Before any step runs: navigation, or the state as a whole. */ +export const INITIAL_STEP_PROGRESS: StepProgress = { + index: -1, + kind: "state", + target: "", +}; + +export interface ScriptedStateRunner { + readonly runStateScript: ( + page: Page, + stateName: string, + steps: ReadonlyArray, + progress: Ref.Ref, + ) => Effect.Effect; + readonly checkPrecondition: ( + page: Page, + selector: string, + ) => Effect.Effect; +} + +export interface ScriptedStateRunnerOptions { + /** Action default when a step omits `timeoutMs`. */ + readonly defaultStepTimeoutMs?: number; + readonly preconditionTimeoutMs?: number; +} + +const errorMessage = (error: unknown): string => { + if (error instanceof Error) { + const [first] = error.message.split("\n"); + return first?.trim() || error.message; + } + return String(error); +}; + +const countMatching = async ( + locator: Locator, + state: ElementState, +): Promise => { + const total = await locator.count(); + if (state === "attached") return total; + if (state === "detached") return total === 0 ? 1 : 0; + let matched = 0; + for (let i = 0; i < total; i++) { + const visible = await locator.nth(i).isVisible(); + if (state === "visible" ? visible : !visible) matched += 1; + } + return matched; +}; + +/** + * Two page-manipulation toolkits are built from the same config and sit side + * by side in the service: `createLinkDiscoveryTools` opens menus so links + * become *discoverable*, and this one performs a named script so a state + * becomes *capturable*. Neither owns the other, and `menuInteractionSelectors` + * is not a capture-state mechanism. + */ +export const createScriptedStateRunner = ( + options: ScriptedStateRunnerOptions = {}, +): ScriptedStateRunner => { + const { defaultStepTimeoutMs, preconditionTimeoutMs } = options; + + const stepError = ( + stateName: string, + index: number, + kind: string, + target: string, + label: string, + message: string, + cause: unknown, + ): StateCaptureError => + new StateCaptureError({ + state: stateName, + stepIndex: index, + stepKind: kind, + target, + message: `state "${stateName}" failed at step ${index} (${label}): ${message}`, + cause, + }); + + const runPlan = ( + page: Page, + plan: StepPlan, + fail: (message: string, cause: unknown) => StateCaptureError, + ): Effect.Effect => { + switch (plan.op) { + case "sleep": + return Effect.sleep(plan.ms); + + case "waitForSelector": + return Effect.tryPromise({ + try: () => + page + .locator(plan.selector) + .first() + .waitFor({ state: plan.state, timeout: plan.timeoutMs }), + catch: (error) => + fail( + `never became ${plan.state} within ${plan.timeoutMs}ms (${errorMessage(error)})`, + error, + ), + }); + + case "waitForCount": + return Effect.tryPromise({ + try: async () => { + const locator = page.locator(plan.selector); + const deadline = Date.now() + plan.timeoutMs; + let found = 0; + for (;;) { + found = await countMatching(locator, plan.state); + if (found >= plan.minCount) return; + if (Date.now() >= deadline) { + throw new Error( + `expected >=${plan.minCount} matching "${plan.state}", found ${found} after ${plan.timeoutMs}ms`, + ); + } + await page.waitForTimeout(plan.pollMs); + } + }, + catch: (error) => fail(errorMessage(error), error), + }); + + case "click": + return Effect.tryPromise({ + try: () => + page + .locator(plan.selector) + .nth(plan.nth) + .click({ timeout: plan.timeoutMs }), + catch: (error) => fail(errorMessage(error), error), + }); + + case "fill": + return Effect.tryPromise({ + try: () => + page + .locator(plan.selector) + .first() + .fill(plan.value, { timeout: plan.timeoutMs }), + catch: (error) => fail(errorMessage(error), error), + }); + + case "select": + return Effect.tryPromise({ + try: () => + page + .locator(plan.selector) + .first() + .selectOption([...plan.values], { timeout: plan.timeoutMs }) + .then(() => undefined), + catch: (error) => fail(errorMessage(error), error), + }); + + case "press": + return Effect.tryPromise({ + try: () => + plan.selector === undefined + ? page.keyboard.press(plan.key) + : page + .locator(plan.selector) + .first() + .press(plan.key, { timeout: plan.timeoutMs }), + catch: (error) => fail(errorMessage(error), error), + }); + + case "reload": + return Effect.tryPromise({ + try: () => + page + .reload({ waitUntil: plan.waitUntil, timeout: plan.timeoutMs }) + .then(() => undefined), + catch: (error) => fail(errorMessage(error), error), + }); + + case "request": + return Effect.tryPromise({ + try: async () => { + const response = await page.request.fetch(plan.url, { + method: plan.method, + timeout: plan.timeoutMs, + headers: plan.headers, + ...(plan.hasJson ? { data: plan.json } : {}), + }); + if (isExpectedStatus(response.status(), plan.expectStatus)) return; + let body = ""; + try { + body = (await response.text()).slice(0, RESPONSE_BODY_EXCERPT); + } catch { + body = ""; + } + throw new Error( + `expected ${plan.expectStatus ?? "2xx"}, got ${response.status()} — ${body}`, + ); + }, + catch: (error) => fail(errorMessage(error), error), + }); + } + }; + + const runStateScript = ( + page: Page, + stateName: string, + steps: ReadonlyArray, + progress: Ref.Ref, + ): Effect.Effect => + Effect.gen(function* () { + for (const [index, step] of steps.entries()) { + let planned: PlannedStep; + try { + planned = planStep(step, { + pageUrl: page.url(), + defaultTimeoutMs: defaultStepTimeoutMs, + }); + } catch (error) { + const kind = error instanceof StepPlanError ? error.kind : step.kind; + const target = + error instanceof StepPlanError ? error.target : String(step.kind); + return yield* Effect.fail( + stepError( + stateName, + index, + kind, + target, + `${step.kind} ${target}`, + errorMessage(error), + error, + ), + ); + } + + yield* Ref.set(progress, { + index, + kind: planned.kind, + target: planned.target, + }); + + const fail = (message: string, cause: unknown) => + stepError( + stateName, + index, + planned.kind, + planned.target, + planned.label, + message, + cause, + ); + + const attempt = runPlan(page, planned.plan, fail); + + if (planned.optional) { + yield* attempt.pipe( + Effect.catchAll((error) => { + console.warn( + ` ! step ${index} (${planned.label}) failed (optional) — continuing: ${error.message}`, + ); + return Effect.void; + }), + ); + } else { + yield* attempt; + } + + if (planned.settleMs > 0) { + yield* Effect.sleep(planned.settleMs); + } + } + }); + + const checkPrecondition = ( + page: Page, + selector: string, + ): Effect.Effect => + Effect.tryPromise({ + try: async () => { + await page + .locator(selector) + .first() + .waitFor({ + state: "visible", + timeout: preconditionTimeoutMs ?? DEFAULT_PRECONDITION_TIMEOUT_MS, + }); + return true; + }, + catch: (error) => error, + }).pipe(Effect.catchAll(() => Effect.succeed(false))); + + return { runStateScript, checkPrecondition } as const; +}; diff --git a/src/states.test.ts b/src/states.test.ts new file mode 100644 index 0000000..439c33b --- /dev/null +++ b/src/states.test.ts @@ -0,0 +1,518 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + */ + +import { Effect, Exit } from "effect"; +import { describe, expect, it } from "vitest"; +import { StateDefinitionError } from "./errors.js"; +import type { CaptureState } from "./schemas.js"; +import { + filterStates, + MAX_STATE_CHAIN_DEPTH, + parseStatesFile, + resolveStateSteps, + statesUseRequests, + validateStates, +} from "./states.js"; + +const file = (states: unknown): string => + JSON.stringify({ version: 1, states }); + +const parse = (states: unknown): ReadonlyArray => + parseStatesFile(file(states), "states.json"); + +const chain = (names: readonly string[]): ReadonlyArray => + parse( + names.map((name, index) => ({ + name, + steps: [{ kind: "click", selector: `#${name}` }], + ...(index === 0 ? {} : { extends: names[index - 1] }), + })), + ); + +const allowAllHosts = () => true; + +const runValidate = ( + overrides: Partial[0]>, +) => + Effect.runSyncExit( + validateStates({ + states: [], + seedUrl: "https://app.example.com/", + hostMatchesFilters: allowAllHosts, + allowStateRequests: false, + viewportNames: ["desktop", "mobile"], + captureRoutes: true, + ...overrides, + }), + ); + +const failureMessage = ( + exit: Exit.Exit, +): string => { + if (Exit.isSuccess(exit)) throw new Error("expected a definition failure"); + const error = Exit.causeOption(exit); + return JSON.stringify(error); +}; + +describe("parseStatesFile", () => { + it("decodes a well-formed file and applies step defaults", () => { + const states = parse([ + { + name: "spawn-dialog", + steps: [ + { kind: "waitFor", selector: "canvas[data-scene-ready]" }, + { kind: "click", selector: "[data-testid='spawn-drone']" }, + ], + }, + ]); + expect(states).toHaveLength(1); + expect(states[0]?.name).toBe("spawn-dialog"); + expect(states[0]?.steps[0]).toMatchObject({ + kind: "waitFor", + state: "visible", + optional: false, + }); + expect(states[0]?.allowVideoReplay).toBe(false); + }); + + it("requires the version discriminant rather than defaulting it", () => { + expect(() => + parseStatesFile(JSON.stringify({ states: [] }), "states.json"), + ).toThrow(/version/); + }); + + it("names the offending path when a step kind is unknown", () => { + expect(() => + parse([{ name: "x", steps: [{ kind: "evaluate", js: "alert(1)" }] }]), + ).toThrow(/states\.0\.steps\.0/); + }); + + it("rejects a state name that would not survive as a directory", () => { + expect(() => parse([{ name: "Spawn Dialog", steps: [] }])).toThrow( + /states\.0\.name/, + ); + }); + + it("reports unparseable JSON against the source path", () => { + expect(() => parseStatesFile("{not json", "states.json")).toThrow( + /states\.json is not valid JSON/, + ); + }); +}); + +describe("statesUseRequests", () => { + it("is true only when a request step is present", () => { + expect(statesUseRequests(parse([{ name: "a", steps: [] }]))).toBe(false); + expect( + statesUseRequests( + parse([ + { + name: "a", + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + ]), + ), + ).toBe(true); + }); +}); + +describe("resolveStateSteps", () => { + it("prepends the parent's steps and inherits its url", () => { + const states = parse([ + { + name: "fleet", + url: "/console", + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + { + name: "fleet-editor", + extends: "fleet", + steps: [{ kind: "click", selector: "[data-panel='editor']" }], + }, + ]); + const resolved = resolveStateSteps(states); + const child = resolved.get("fleet-editor"); + expect(child?.steps.map((step) => step.kind)).toEqual(["request", "click"]); + expect(child?.url).toBe("/console"); + }); + + it("lets a child override the inherited url", () => { + const resolved = resolveStateSteps( + parse([ + { name: "base", url: "/a", steps: [] }, + { name: "child", extends: "base", url: "/b", steps: [] }, + ]), + ); + expect(resolved.get("child")?.url).toBe("/b"); + }); + + it("rejects duplicate names, because names become directories", () => { + expect(() => + resolveStateSteps( + parse([ + { name: "dup", steps: [] }, + { name: "dup", steps: [] }, + ]), + ), + ).toThrow(StateDefinitionError); + }); + + it("rejects an unknown parent", () => { + expect(() => + resolveStateSteps(parse([{ name: "a", extends: "nope", steps: [] }])), + ).toThrow(/extends unknown state/); + }); + + it("rejects a cycle", () => { + expect(() => + resolveStateSteps( + parse([ + { name: "a", extends: "b", steps: [] }, + { name: "b", extends: "a", steps: [] }, + ]), + ), + ).toThrow(/extends cycle/); + }); + + it("allows a chain at the depth limit and rejects one past it", () => { + const names = Array.from( + { length: MAX_STATE_CHAIN_DEPTH + 1 }, + (_, index) => `s${index}`, + ); + expect(() => resolveStateSteps(chain(names))).not.toThrow(); + expect(() => resolveStateSteps(chain([...names, "over"]))).toThrow( + /links deep/, + ); + }); +}); + +describe("validateStates", () => { + it("succeeds trivially when there are no states", () => { + expect(Exit.isSuccess(runValidate({}))).toBe(true); + }); + + it("refuses a run that would capture nothing", () => { + expect(Exit.isSuccess(runValidate({ captureRoutes: false }))).toBe(false); + }); + + it("rejects a state URL outside the allowed hosts", () => { + const exit = runValidate({ + states: parse([{ name: "off", url: "https://evil.test/x", steps: [] }]), + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("outside the allowed hosts"); + }); + + it("rejects a viewport filter naming a viewport that is not configured", () => { + const exit = runValidate({ + states: parse([{ name: "v", viewports: ["ultrawide"], steps: [] }]), + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("ultrawide"); + }); + + it("accepts a viewport filter naming a configured viewport", () => { + expect( + Exit.isSuccess( + runValidate({ + states: parse([{ name: "v", viewports: ["desktop"], steps: [] }]), + }), + ), + ).toBe(true); + }); + + it("blocks request steps unless they are explicitly allowed", () => { + const states = parse([ + { + name: "seed", + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + ]); + expect(Exit.isSuccess(runValidate({ states }))).toBe(false); + expect( + Exit.isSuccess(runValidate({ states, allowStateRequests: true })), + ).toBe(true); + }); + + it("blocks an off-host request even when request steps are allowed", () => { + const exit = runValidate({ + states: parse([ + { + name: "seed", + steps: [ + { + kind: "request", + method: "POST", + path: "https://evil.test/collect", + }, + ], + }, + ]), + allowStateRequests: true, + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("outside the allowed hosts"); + }); + + it("checks a request inherited through extends, not just a state's own steps", () => { + const states = parse([ + { + name: "seed", + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + { name: "child", extends: "seed", steps: [] }, + ]); + expect(Exit.isSuccess(runValidate({ states }))).toBe(false); + }); +}); + +describe("filterStates", () => { + it("keeps ancestors so a filtered run still resolves", () => { + const states = parse([ + { name: "base", steps: [] }, + { name: "middle", extends: "base", steps: [] }, + { name: "leaf", extends: "middle", steps: [] }, + { name: "unrelated", steps: [] }, + ]); + expect(filterStates(states, ["leaf"]).map((state) => state.name)).toEqual([ + "base", + "middle", + "leaf", + ]); + }); + + it("throws on a name that matches nothing", () => { + expect(() => + filterStates(parse([{ name: "a", steps: [] }]), ["b"]), + ).toThrow(/Unknown state name/); + }); +}); + +describe("parseStatesFile — rejected shapes", () => { + /** + * The step vocabulary becomes a public JSON format on other people's disks + * the day it ships, so every rejection has to name the path that is wrong. + * Each case asserts the message points at the offending field, not just + * that a throw happened. + */ + const rejects = (states: unknown, pattern: RegExp) => { + expect(() => parse(states)).toThrow(pattern); + }; + + it("rejects a version it does not understand", () => { + expect(() => + parseStatesFile( + JSON.stringify({ version: 2, states: [] }), + "states.json", + ), + ).toThrow(/version/); + }); + + it("rejects a file with no states array", () => { + expect(() => + parseStatesFile(JSON.stringify({ version: 1 }), "states.json"), + ).toThrow(/states/); + }); + + it("rejects a state with no steps", () => { + rejects([{ name: "a" }], /states\.0\.steps/); + }); + + it("rejects a state name that is empty, uppercase, or oddly punctuated", () => { + for (const name of [ + "", + "Spawn", + "spawn dialog", + "-spawn", + "spawn_dialog", + ]) { + rejects([{ name, steps: [] }], /states\.0\.name/); + } + }); + + it("accepts the names that survive as directories", () => { + expect( + parse([ + { name: "spawn-dialog", steps: [] }, + { name: "fleet2", steps: [] }, + { name: "0", steps: [] }, + ]), + ).toHaveLength(3); + }); + + it("rejects a click with no selector", () => { + rejects([{ name: "a", steps: [{ kind: "click" }] }], /steps\.0/); + }); + + it("rejects a press with no key", () => { + rejects([{ name: "a", steps: [{ kind: "press" }] }], /steps\.0/); + }); + + it("rejects a select with no values", () => { + rejects( + [{ name: "a", steps: [{ kind: "select", selector: "#s" }] }], + /steps\.0/, + ); + }); + + it("rejects a negative wait", () => { + rejects([{ name: "a", steps: [{ kind: "wait", ms: -1 }] }], /steps\.0/); + }); + + it("rejects minCount of zero, which would assert nothing", () => { + rejects( + [ + { + name: "a", + steps: [{ kind: "waitFor", selector: "#x", minCount: 0 }], + }, + ], + /steps\.0/, + ); + }); + + it("rejects a non-positive timeout or a negative settle", () => { + rejects( + [{ name: "a", steps: [{ kind: "click", selector: "#x", timeoutMs: 0 }] }], + /steps\.0/, + ); + rejects( + [ + { + name: "a", + steps: [{ kind: "click", selector: "#x", settleMs: -5 }], + }, + ], + /steps\.0/, + ); + }); + + it("rejects an HTTP method outside the closed union", () => { + rejects( + [ + { + name: "a", + steps: [{ kind: "request", method: "TRACE", path: "/x" }], + }, + ], + /steps\.0/, + ); + }); + + it("rejects non-string header values", () => { + rejects( + [ + { + name: "a", + steps: [ + { + kind: "request", + method: "GET", + path: "/x", + headers: { "x-n": 1 }, + }, + ], + }, + ], + /steps\.0/, + ); + }); + + it("rejects a waitUntil Playwright would not accept", () => { + rejects( + [{ name: "a", steps: [{ kind: "reload", waitUntil: "idle" }] }], + /steps\.0/, + ); + }); + + it("rejects a waitFor DOM state outside the closed union", () => { + rejects( + [ + { + name: "a", + steps: [{ kind: "waitFor", selector: "#x", state: "painted" }], + }, + ], + /steps\.0/, + ); + }); + + it("rejects a whole-state timeout of zero", () => { + rejects([{ name: "a", steps: [], timeoutMs: 0 }], /states\.0\.timeoutMs/); + }); + + it("names the second state when the second state is the broken one", () => { + rejects( + [ + { name: "good", steps: [] }, + { name: "bad", steps: [{ kind: "nope" }] }, + ], + /states\.1\.steps\.0/, + ); + }); +}); + +describe("parseStatesFile — accepted shapes", () => { + it("accepts every step kind in the vocabulary", () => { + const states = parse([ + { + name: "everything", + description: "one of each", + url: "/console?mode=advanced", + precondition: "[data-advanced]", + viewports: ["desktop"], + timeoutMs: 45000, + allowVideoReplay: true, + steps: [ + { kind: "waitFor", selector: "#a", state: "hidden", minCount: 2 }, + { kind: "wait", ms: 0 }, + { kind: "click", selector: "#b", nth: 0, optional: true }, + { kind: "fill", selector: "#c", value: "v" }, + { kind: "select", selector: "#d", values: ["x", "y"] }, + { kind: "press", key: "w", selector: "#e" }, + { + kind: "request", + method: "DELETE", + path: "/api/x", + json: null, + headers: { a: "b" }, + expectStatus: 204, + }, + { kind: "reload", waitUntil: "commit", settleMs: 250 }, + ], + }, + ]); + expect(states[0]?.steps.map((step) => step.kind)).toEqual([ + "waitFor", + "wait", + "click", + "fill", + "select", + "press", + "request", + "reload", + ]); + expect(states[0]?.allowVideoReplay).toBe(true); + expect(states[0]?.precondition).toBe("[data-advanced]"); + expect(states[0]?.viewports).toEqual(["desktop"]); + }); + + it("has no step kind that carries arbitrary code", () => { + // The non-negotiable: no `evaluate`, `script`, `js` or `fn` step exists, + // and no value in the grammar is ever interpreted as code. + for (const kind of ["evaluate", "script", "js", "fn", "exec"]) { + expect(() => parse([{ name: "a", steps: [{ kind }] }])).toThrow(); + } + }); +}); diff --git a/src/states.ts b/src/states.ts new file mode 100644 index 0000000..b0e3c44 --- /dev/null +++ b/src/states.ts @@ -0,0 +1,339 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/** + * Parsing, chain resolution and pre-launch validation for scripted states. + * + * Everything here is pure: no filesystem, no Playwright. Authoring mistakes — + * a duplicate name, a typo'd `extends`, an off-host `request` — are static + * errors that abort the run before Chromium launches, because no amount of + * retrying makes them resolve. Runtime failures are the opposite: recorded per + * state, run continues. That split is the whole failure model. + */ + +import { ArrayFormatter, ParseResult, Schema as S } from "@effect/schema"; +import { Effect } from "effect"; +import { StateDefinitionError } from "./errors.js"; +import { type CaptureState, type CaptureStep, StatesFile } from "./schemas.js"; +import { resolveRequestUrl, StepPlanError } from "./state-plan.js"; + +/** How deep an `extends` chain may go before it stops being reviewable. */ +export const MAX_STATE_CHAIN_DEPTH = 5; + +/** A state with its `extends` chain flattened into one step list. */ +export interface ResolvedState { + readonly state: CaptureState; + /** The parent chain's steps, in order, followed by this state's own. */ + readonly steps: readonly CaptureStep[]; + /** The state's own `url`, or the nearest ancestor's. */ + readonly url: string | undefined; +} + +const decodeStatesFile = S.decodeUnknownEither(StatesFile); + +/** + * Decodes a states file's contents. Throws a plain `Error` carrying the Effect + * Schema issue paths, matching how `parseViewports` reports a bad spec. + */ +export const parseStatesFile = ( + contents: string, + sourcePath: string, +): ReadonlyArray => { + let raw: unknown; + try { + raw = JSON.parse(contents); + } catch (error) { + throw new Error( + `${sourcePath} is not valid JSON: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + + const decoded = decodeStatesFile(raw); + if (decoded._tag === "Right") return decoded.right.states; + + const issues = ParseResult.isParseError(decoded.left) + ? ArrayFormatter.formatErrorSync(decoded.left) + : []; + const detail = + issues.length > 0 + ? issues + .map( + (issue) => + ` ${issue.path.map(String).join(".") || "(root)"}: ${issue.message}`, + ) + .join("\n") + : String(decoded.left); + throw new Error(`Invalid states file ${sourcePath}:\n${detail}`); +}; + +/** True when any state's own steps include a `request`. */ +export const statesUseRequests = ( + states: ReadonlyArray, +): boolean => + states.some((state) => state.steps.some((step) => step.kind === "request")); + +const definitionError = ( + state: string, + message: string, +): StateDefinitionError => + new StateDefinitionError({ state, message, cause: null }); + +/** + * Flattens every state's `extends` chain. + * + * `extends` is script composition, not page-state carryover: the child still + * starts from a fresh load in a fresh context and replays the parent's steps. + * Replay costs wall clock and buys the thing that matters — any state runs on + * any worker, in any order, with no cross-task coupling. + * + * Throws {@link StateDefinitionError} on an unknown parent, a cycle, or a + * chain deeper than {@link MAX_STATE_CHAIN_DEPTH}. + */ +export const resolveStateSteps = ( + states: ReadonlyArray, +): ReadonlyMap => { + const byName = new Map(); + for (const state of states) { + if (byName.has(state.name)) { + throw definitionError( + state.name, + `duplicate state name "${state.name}"; names become directories and must be unique`, + ); + } + byName.set(state.name, state); + } + + const resolved = new Map(); + // Depth is tracked separately from the visit chain because resolution order + // is file order: a parent resolved on its own line would otherwise seed the + // cache and let an over-long chain through unnoticed. + const depths = new Map(); + + const resolve = ( + state: CaptureState, + chain: readonly string[], + ): ResolvedState => { + const cached = resolved.get(state.name); + if (cached) return cached; + + if (chain.includes(state.name)) { + throw definitionError( + state.name, + `extends cycle: ${[...chain, state.name].join(" -> ")}`, + ); + } + + let inherited: ResolvedState | undefined; + let depth = 0; + if (state.extends !== undefined) { + const parent = byName.get(state.extends); + if (!parent) { + throw definitionError( + state.name, + `extends unknown state "${state.extends}"`, + ); + } + inherited = resolve(parent, [...chain, state.name]); + depth = (depths.get(parent.name) ?? 0) + 1; + if (depth > MAX_STATE_CHAIN_DEPTH) { + throw definitionError( + state.name, + `extends chain is ${depth} links deep; the limit is ${MAX_STATE_CHAIN_DEPTH}`, + ); + } + } + depths.set(state.name, depth); + + const value: ResolvedState = { + state, + steps: [...(inherited?.steps ?? []), ...state.steps], + url: state.url ?? inherited?.url, + }; + resolved.set(state.name, value); + return value; + }; + + for (const state of states) resolve(state, []); + return resolved; +}; + +export interface ValidateStatesOptions { + readonly states: ReadonlyArray; + readonly seedUrl: string; + readonly hostMatchesFilters: (hostname: string) => boolean; + readonly allowStateRequests: boolean; + readonly viewportNames: ReadonlyArray; + readonly captureRoutes: boolean; +} + +/** + * Every check that can be made before the browser launches. + * + * A failure here aborts the run, so the user fixes their file once instead of + * watching a full crawl produce four identical timeouts. + */ +export const validateStates = ( + options: ValidateStatesOptions, +): Effect.Effect, StateDefinitionError> => + Effect.suspend(() => { + const { + states, + seedUrl, + hostMatchesFilters, + allowStateRequests, + viewportNames, + captureRoutes, + } = options; + + if (states.length === 0) { + return captureRoutes + ? Effect.succeed(new Map()) + : Effect.fail( + definitionError( + "(run)", + "nothing to capture: route capture is disabled and no scripted states were supplied", + ), + ); + } + + let resolved: ReadonlyMap; + try { + resolved = resolveStateSteps(states); + } catch (error) { + return error instanceof StateDefinitionError + ? Effect.fail(error) + : Effect.fail( + new StateDefinitionError({ + state: "(states)", + message: error instanceof Error ? error.message : String(error), + cause: error, + }), + ); + } + + const viewportSet = new Set(viewportNames); + + for (const entry of resolved.values()) { + const { state } = entry; + + let stateUrl: URL; + try { + stateUrl = new URL(entry.url ?? seedUrl, seedUrl); + } catch { + return Effect.fail( + definitionError( + state.name, + `url "${String(entry.url)}" does not resolve against "${seedUrl}"`, + ), + ); + } + if (!hostMatchesFilters(stateUrl.hostname)) { + return Effect.fail( + definitionError( + state.name, + `url "${stateUrl.toString()}" is outside the allowed hosts`, + ), + ); + } + + if (state.viewports) { + const unknownViewport = state.viewports.find( + (name) => !viewportSet.has(name), + ); + if (unknownViewport !== undefined) { + return Effect.fail( + definitionError( + state.name, + `viewports names "${unknownViewport}", which is not a configured viewport (${viewportNames.join(", ")})`, + ), + ); + } + } + + for (const [index, step] of entry.steps.entries()) { + if (step.kind !== "request") continue; + if (!allowStateRequests) { + return Effect.fail( + definitionError( + state.name, + `step ${index} (request ${step.method} ${step.path}) needs --allow-state-requests (allowStateRequests: true); request steps reach past the UI into the backend, so they are opt-in`, + ), + ); + } + let requestUrl: URL; + try { + requestUrl = resolveRequestUrl(step.path, stateUrl.toString()); + } catch (error) { + return Effect.fail( + definitionError( + state.name, + `step ${index} (request ${step.method} ${step.path}): ${ + error instanceof StepPlanError ? error.message : String(error) + }`, + ), + ); + } + if (!hostMatchesFilters(requestUrl.hostname)) { + return Effect.fail( + definitionError( + state.name, + `step ${index} (request ${step.method} ${step.path}) resolves to "${requestUrl.toString()}", which is outside the allowed hosts`, + ), + ); + } + } + } + + return Effect.succeed(resolved); + }); + +/** + * Narrows a states list to the named states, keeping every ancestor they + * `extends` so a filtered run still resolves. Throws when a name matches + * nothing, because silently running zero states is the coverage lie again. + */ +export const filterStates = ( + states: ReadonlyArray, + names: ReadonlyArray, +): ReadonlyArray => { + const byName = new Map(states.map((state) => [state.name, state])); + const unknown = names.filter((name) => !byName.has(name)); + if (unknown.length > 0) { + throw new Error( + `Unknown state name(s): ${unknown.join(", ")}. Available: ${ + states.map((state) => state.name).join(", ") || "(none)" + }`, + ); + } + + const keep = new Set(); + const visit = (name: string, seen: ReadonlySet): void => { + if (keep.has(name) || seen.has(name)) return; + const state = byName.get(name); + if (!state) return; + keep.add(name); + if (state.extends !== undefined) { + visit(state.extends, new Set([...seen, name])); + } + }; + for (const name of names) visit(name, new Set()); + + return states.filter((state) => keep.has(state.name)); +}; diff --git a/src/video.ts b/src/video.ts index 28ad413..4bd8fc5 100644 --- a/src/video.ts +++ b/src/video.ts @@ -37,6 +37,15 @@ export interface CaptureVideoConfig { }; /** Must match the screenshot context, or a run's stills and video disagree. */ readonly colorScheme: "light" | "dark" | "no-preference"; + /** + * Replays a scripted state inside the recording context. + * + * Without it the video context would navigate and record the *unscripted* + * boot view while the stills show the scripted state — the two silently + * disagreeing. Undefined for route captures, so existing behaviour is + * untouched. + */ + readonly prepare?: (page: Page) => Effect.Effect; } export const captureVideoForViewport = ( @@ -104,6 +113,10 @@ export const captureVideoForViewport = ( yield* Effect.sleep(cfg.waitTime); + if (cfg.prepare) { + yield* cfg.prepare(videoPage); + } + if (cfg.videoOptions.interactions) { const scrollSteps = 5; const scrollDelay = cfg.videoOptions.duration / (scrollSteps + 1); From 5ad07eb49c90b783585f384555cee8e88edb392f Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Thu, 3 Sep 2026 23:40:43 -0400 Subject: [PATCH 2/9] fix(report): record a failed route instead of only logging it processRouteTask's failure was caught in the worker loop, logged to stderr, and never entered into the results map. Because generateReports counts failures out of that map, failedCaptures was structurally 0: a crawl that lost half its routes to navigation errors still reported as fully covered, which is the same disease scripted states were added to treat. The catchAll now writes a CaptureResult carrying the route and the formatted error, the way a failed state does, unless a result for that URL already exists. This changes report content for runs that already had failing routes: failedCaptures becomes accurate, results.size grows by the number of failed routes, and those routes appear under "Failed Captures" in REPORT.md. Runs with no failures are unaffected. Kept as its own commit because it is a behaviour change to existing output rather than part of the additive feature. --- src/service.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/service.ts b/src/service.ts index 0b505f3..a6f27ae 100644 --- a/src/service.ts +++ b/src/service.ts @@ -734,6 +734,21 @@ export class UICaptureService extends Effect.Service()( `[Worker ${workerId}] Failed to capture ${task.url}:`, error, ); + // A failed route is recorded, not merely logged: + // otherwise `failedCaptures` is structurally 0 and + // a half-crawled site reports as fully covered. + if (!results.has(task.normalizedUrl)) { + results.set( + task.normalizedUrl, + new CaptureResult({ + url: task.url, + route: getRouteName(task.url), + screenshots: {}, + error: formatCaptureFailure(error), + timestamp: Date.now(), + }), + ); + } return Effect.void; }), Effect.ensuring(markTaskComplete()), From c459a78b059dc1f3c8df09b9c60275ccecb4491d Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 00:42:29 -0400 Subject: [PATCH 3/9] fix(states): correct the failure, origin, budget and resource models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of the scripted-states feature confirmed defects in four models the feature had gotten subtly wrong. Each one made a run report something other than what actually happened. Failure model. A video recording that failed discarded the screenshots already written for that viewport and reported the whole capture as a failure that produced nothing; it is now a success that names what it lost, through a `videoErrors` field carried into both reports. A `precondition` that could not be *evaluated* — a typo'd or malformed selector — answered "not present here", which is how a broken selector produced a green run with nothing captured; it now fails, and only a genuinely absent selector skips. A state that timed out named the last step it had started, often one that had already succeeded; it now names the phase it was actually in: navigating, probing, inside step N, or settling after it. Origin model. The pre-launch check and the runtime `request` gate had drifted into two different comparisons, one of them on hostname alone, so a states file could be validated against one rule and run against a wider one. Both now call a single `isAllowedOrigin` — scheme, host and port — applied where each resolution actually happens. Budget model. `--state-timeout` covered capture as well as reaching the state, which made it unsatisfiable: with `--video` on, no default could cover navigation plus a screenshot and a recording per viewport, so every state timed out on a configuration that looks entirely reasonable. It now bounds reaching the state only, its default clears the 30 s navigation ceiling, and the precondition probe gets its own budget and a `--precondition-timeout` flag. Resource model. A browser context was stranded whenever the page created inside it failed to open, because Effect registers a release only once its acquire has completed. The state path and the worker pool now acquire context and page separately, and the one best-effort close helper is shared rather than reimplemented at each call site. Chain composition. `filterStates` inlined an ancestor's steps but not its `allowVideoReplay`, so a chain whose seed step suppresses video recorded it on a full run and silently dropped it under `--state-filter` — one file, two results. Everything the chain contributes is now written back into the flattened state. Also: an empty `viewports` array is rejected rather than capturing nothing and reporting success, and two helpers with no consumer outside their own module stop being exported. README and USAGE track every behaviour change above. --- README.md | 82 ++++++-- src/docs.test.ts | 5 + src/errors.ts | 7 +- src/report.ts | 7 + src/runner.test.ts | 31 +++ src/runner.ts | 28 ++- src/schemas.test.ts | 5 +- src/schemas.ts | 77 +++++++- src/service.ts | 399 ++++++++++++++++++++++++++------------- src/shared.ts | 56 +++++- src/state-plan.test.ts | 190 +++++++++++++++++++ src/state-plan.ts | 142 ++++++++++++-- src/state-script.test.ts | 358 ++++++++++++++++++++++++++++++++++- src/state-script.ts | 224 ++++++++++++++++++---- src/states.test.ts | 316 ++++++++++++++++++++++++++++++- src/states.ts | 140 +++++++++++--- src/video.test.ts | 275 +++++++++++++++++++++++++++ src/video.ts | 193 ++++++++++--------- 18 files changed, 2208 insertions(+), 327 deletions(-) create mode 100644 src/video.test.ts diff --git a/README.md b/README.md index 19b8a85..263216e 100644 --- a/README.md +++ b/README.md @@ -166,10 +166,22 @@ Options: named state is performed on a fresh page load and yields its own capture set, so a single-route app's dialogs and workspaces get captured too. - --state-filter Run only these named states (default: all) + --state-filter Capture only these named states (default: all). + A state they extend is replayed to reach them, + and is not captured itself. --skip-routes Capture only scripted states, not crawled routes - --state-timeout Per-state budget covering navigation, script and - capture (default: 30000; a state may override it) + --state-timeout Budget for reaching a state: navigation, + precondition probe and script. Screenshot and + video capture are outside it (default: 60000; + a state may override it with timeoutMs) + --precondition-timeout Budget for a state's precondition probe, which + decides whether the state exists on this build + at all (default: 10000; a state may override it + with preconditionTimeoutMs). Raise it for an app + whose first meaningful frame lands well after + networkidle, such as a WebGL console — a probe + that gives up first records the state as skipped + rather than slow. --allow-state-requests Permit request steps, which reach past the UI into the app's own backend. Off by default: a states file from a colleague should not be able @@ -352,7 +364,8 @@ All three errors are `S.TaggedError` subclasses, so they discriminate cleanly un | `ffmpegPath` | `--ffmpeg` | `string` | `"ffmpeg"` | Absolute path or anything on `PATH`. | | `launchArgs` | `--launch-args` | `string[]` | `[]` | Appended after the baseline switches so they win on conflict; the CLI value splits on whitespace rather than commas, since one switch may itself contain commas. | | `states` | `--states` | `CaptureState[]` | `[]` | Named interaction scripts, each yielding its own capture set; empty by default, so a run without a states file behaves exactly as it always has. | -| `stateTimeout` | `--state-timeout` | `int ≥ 1` (ms) | `30000` | Whole-state budget covering navigation, script and capture; a state may override it with its own `timeoutMs`. | +| `stateTimeout` | `--state-timeout` | `int ≥ 1` (ms) | `60000` | Budget for *reaching* a state — navigation, the `precondition` probe and the script; screenshot and video capture sit outside it, and a state may override it with its own `timeoutMs`. | +| `preconditionTimeout` | `--precondition-timeout`| `int ≥ 1` (ms) | `10000` | Budget for a state's `precondition` probe: long enough for an app whose first meaningful frame lands after `networkidle` — a probe that gives up first records the state as `skipped` rather than slow — and short enough that a state which genuinely is not here skips cheaply; a state may override it with its own `preconditionTimeoutMs`. | | `captureRoutes` | `--skip-routes` (¬) | `boolean` | `true` | Set `false` to capture only scripted states, for an app whose boot view is a loading spinner. | | `allowStateRequests` | `--allow-state-requests`| `boolean` | `false` | Gate on `request` steps, checked at load time *and* in the service so a programmatic caller cannot skip it. | @@ -433,12 +446,13 @@ Rejecting loudly beats slugifying two states into one directory, and lowercase-o | `name` | `string` matching `^[a-z0-9][a-z0-9-]*$` | *required* | Identifies the state, and names its output directory. | | `steps` | `CaptureStep[]` | *required* | The script, run in order on a fresh page load. | | `description` | `string` | — | Free text; carried for the reader, not used by the tool. | -| `url` | `string` | the seed URL | Absolute, or relative to the seed URL, and subject to the same host filter as any crawled link. | -| `extends` | `string` | — | Another state's name, whose steps are prepended to this one's; chains deeper than five links are rejected. | -| `precondition` | `string` (CSS selector) | — | Probed on the fresh load before any step; absent means the state is recorded `skipped` rather than `failed`. | -| `viewports` | `string[]` | every configured viewport | Restrict the state to named viewports, for UI that does not exist at every breakpoint. | -| `timeoutMs` | `int ≥ 1` | `--state-timeout` (30000) | Whole-state budget covering navigation, script and capture. | -| `allowVideoReplay` | `boolean` | `false` | Record video for a state whose script contains a `request` step, when that seed is idempotent. | +| `url` | `string` | the seed URL | Absolute, or relative to the seed URL, and confined to an allowed **origin** — scheme, host and port all compared, with the host filter deciding the host and the seed deciding the scheme and port. | +| `extends` | `string` | — | Another state's name, whose steps are prepended to this one's; `url` and `allowVideoReplay` are inherited with them, and chains deeper than five links are rejected. | +| `precondition` | `string` (CSS selector) | — | Probed on the fresh load before any step; absent means the state is recorded `skipped` rather than `failed`, while a selector that cannot be *evaluated* is neither — the state fails. | +| `preconditionTimeoutMs` | `int ≥ 1` | `--precondition-timeout` (10000) | Budget for this state's probe, for UI that is ready at first paint or, at the other end, several seconds after it. | +| `viewports` | `string[]` (at least one) | every configured viewport | Restrict the state to named viewports, for UI that does not exist at every breakpoint; an empty array is rejected, because it would capture nothing and still be reported as captured, so omit the field to use every configured viewport. | +| `timeoutMs` | `int ≥ 1` | `--state-timeout` (60000) | Budget for reaching the state: navigation, the `precondition` probe and the script; capture sits outside it. | +| `allowVideoReplay` | `boolean` | `false` | Record video for a state whose script contains a `request` step, when that seed is idempotent; inherited through `extends`, because the `request` step that triggers the suppression is inherited too. | ### Running it @@ -473,6 +487,7 @@ ui-captures/ Each `screenshots/` directory holds the same `png/` `webp/` `jpg/` triple a route capture produces, so nothing downstream has to special-case a state. `--skip-routes` drops `root/screenshots/` and captures only the four state sets, for an app whose boot view is a loading spinner. `--state-filter fleet-editor` runs one state, which is how you iterate on a script you are still writing. +A filter selects *capture targets*, not a subgraph: if `fleet-editor` extends `fleet-multidomain`, the parent's steps are folded into `fleet-editor` and replayed, but `fleet-multidomain` is not itself captured and its own steps — a `request` seed among them — do not run a second time as a state of their own. If `fleet-editor`'s last `waitFor` never resolves, the run still finishes: the other three states capture, `root/states/fleet-editor/` is created and left empty, `REPORT.md` gains a row naming the failing step, and `--fail-on-state-error` makes the process exit non-zero so CI does not go green on a state that never rendered. @@ -480,7 +495,7 @@ If `fleet-editor`'s last `waitFor` never resolves, the run still finishes: the o | Kind | Fields | What it is for | | ---- | ------ | -------------- | -| `waitFor` | `selector`, `state?` (`visible` \| `hidden` \| `attached` \| `detached`, default `visible`), `minCount?` | Readiness, and the assertion mechanism, because the load event is a lie in an SPA; `minCount` exists so waiting for *one* `.fleet-row` cannot shoot a half-populated fleet. | +| `waitFor` | `selector`, `state?` (`visible` \| `hidden` \| `attached` \| `detached`, default `visible`), `minCount?` (with `visible` or `attached` only) | Readiness, and the assertion mechanism, because the load event is a lie in an SPA; `minCount` exists so waiting for *one* `.fleet-row` cannot shoot a half-populated fleet. | | `wait` | `ms` | The crude one, and the only honest tool for a WebGL scene whose intro tween has no DOM correlate; prefer `settleMs`, or a `waitFor` on a readiness attribute. | | `click` | `selector`, `nth?` (zero-based) | Opens the dialog, the tab, the workspace; Playwright auto-scrolls and auto-waits for actionability. | | `fill` | `selector`, `value` | Callsigns, coordinates, waypoints; also handles `contenteditable`. | @@ -491,6 +506,17 @@ If `fleet-editor`'s last `waitFor` never resolves, the run still finishes: the o Every step also accepts three modifiers: `optional` (log and skip on failure — this is how "dismiss the cookie banner if it's there" is expressed, as one shared modifier rather than a parallel `clickIfPresent` family), `timeoutMs` (per-step override, default 5000), and `settleMs` (pause after the step succeeds). +Two field combinations the schema admits are rejected rather than reinterpreted, because in both cases one field would silently redefine another: + +- **`minCount` with `state: "hidden"` or `"detached"`.** + Both of those states also pass when *nothing matches at all*, which is not something a minimum over matches can express — the counting path and the selector path would mean different things by the same word. + Use `visible` or `attached` with `minCount`, or drop `minCount` to wait for the first match to become `hidden`/`detached`. +- **`timeoutMs` on a `press` with no `selector`.** + The key goes to `page.keyboard`, which has no element to wait for and takes no timeout, so the value would be computed and then dropped. + Add a selector, or drop `timeoutMs`. + +Both are reported before Chromium launches, and rejected again when the step is planned, so a programmatic caller cannot route around the early check. + ### The rules that keep the vocabulary small **No variables.** @@ -510,25 +536,44 @@ The same reasoning excludes `drag` and `mouseMove`. Each state starts from a **fresh page load in a fresh browser context**. `page.goto` clears neither cookies nor `localStorage`, so reusing a worker's page would let state N inherit state N-1's client storage and make determinism aspirational rather than true. -`extends` is script composition, not page-state carryover: the child replays the parent's steps from its own clean load, and inherits the parent's `url` unless it sets one. +`extends` is script composition, not page-state carryover: the child replays the parent's steps from its own clean load. +Exactly three things flow down a chain: the steps, `url` (unless the child sets one), and `allowVideoReplay` — the last of those because the inherited `request` step is what suppresses video in the first place, and inheriting the suppression without its opt-out would leave a child unable to undo a decision it never made. +`precondition`, `viewports` and `timeoutMs` describe the child's own capture rather than the script it replays, and stay per-state. Replay costs wall clock and buys the thing that matters — any state runs on any worker, in any order, with no cross-task coupling. Cycles, unknown parents and chains deeper than five links are rejected before Chromium launches. +An ancestor is resolution input rather than a capture target: under `--state-filter` it is folded into the states that name it and is not captured itself. Failures split two ways. -**Authoring errors abort** — duplicate names, a typo'd `extends`, an off-host `request`, a viewport filter naming a viewport that is not configured. +**Authoring errors abort** — duplicate names, a typo'd `extends`, an off-origin `url` or `request`, a viewport filter that is empty or names a viewport that is not configured. **Runtime errors are recorded and the run continues**, exactly as a failing route does: the state's `CaptureResult` carries `stateStatus: "failed"`, the message names the step (`state "fleet-editor" failed at step 3 (waitFor ".fleet-row"): expected >=6 matching "visible", found 2 after 30000ms`), and `failedStepIndex` is what CI greps for. -A state that times out reports the step it was actually on, not just "timed out". +A state that times out reports what it was actually doing — `while loading `, `while probing precondition ""`, `on step 3 (waitFor ".fleet-row")`, `while settling after step 3` — rather than the step it last started, which by then may be one that already succeeded. + +`--state-timeout` (and a state's own `timeoutMs`) bounds **reaching** the state: the navigation, the `precondition` probe and the script. +It stops there. +Screenshot and video capture are bounded by their own timeouts and by `--video-duration` × the number of viewports, and folding those into one whole-state budget makes the budget unsatisfiable rather than protective: a state captured with `--video` cannot fit any default, so every state times out on a configuration that looks entirely reasonable. +The default is `60000` because a single navigation may take the full 30 s Playwright allows it, and a budget at or below that leaves the script none. Its directory is created and left empty, because an empty `states/fleet-editor/` is a visible artefact of something attempted and missed. `precondition` separates a third outcome from those two. A state whose precondition selector is absent on the loaded page is recorded as `skipped`, not `failed` — "this state does not exist here" (feature flag off, unauthenticated build, dev-only panel) is a different event from "this state's script is broken", and collapsing them is how a report becomes noise you learn to ignore. `--fail-on-state-error` counts failures and deliberately ignores skips. +A precondition that cannot be **evaluated** is a fourth thing again, and it is a failure. +The probe evaluates the selector once before it starts waiting on it: a malformed selector rejects there whether or not the element exists, while a valid selector that matches nothing yet counts zero and falls through to the wait. +Without that split, a typo'd selector answers "not present here" and the state skips — a green run that captured nothing, which is the worst outcome available and the hardest to notice. + +The probe's budget is `--precondition-timeout` (default 10000), or the state's own `preconditionTimeoutMs`. +Raise it for an app whose first meaningful frame lands well after `networkidle` — a WebGL console is the motivating case — because a probe that expires first reports the state as absent rather than slow, which is the same green-run-with-nothing-captured failure by another route. + ### Gotchas worth knowing before you write one - **`request` is opt-in, and it is the sharpest edge here.** - The `path` form makes it same-origin by construction and the crawler's host filter is applied as a second gate, but no validation stops a committed `DELETE /api/fleet` from running against a staging URL that happens to resolve to production. + The `path` form makes it same-origin by construction and an origin gate is applied as a second one — scheme, host **and** port must all match an allowed origin, so an absolute path to `http://app.test:9000` is rejected rather than quietly permitted for sharing a hostname — but no validation stops a committed `DELETE /api/fleet` from running against a staging URL that happens to resolve to production. The URL and payload are legible in a diff; that is the mitigation. + **The gate that decides is the runtime one.** + A path resolves against the page's *live* URL, so a script that clicks through to another origin first resolves its requests against an origin the pre-launch pass never saw. + The same origin comparison is therefore applied again in the driver, to the URL each step actually resolves to, before the request is planned — same rule, same seed, so a states file that validated cannot be widened at runtime and a run cannot abort on something the runtime would have allowed. + The pre-launch check in `validateStates` sees only the state's configured `url`: it exists to fail an obviously off-origin path before Chromium launches, and is not the boundary. - **Server state is outside the isolation boundary.** A fresh context cannot un-seed a server, so two seeding states can interfere — and with `--concurrency` above 1 they can interfere concurrently. Use idempotent or per-state-keyed seeds, or `--concurrency 1`; the run prints an advisory when a states file seeds and workers run in parallel. @@ -536,8 +581,12 @@ A state whose precondition selector is absent on the loaded page is recorded as A responsive app that unmounts the dialog at 375px will otherwise produce a mobile screenshot of the boot view recorded as success. Pin such a state with `"viewports": ["desktop"]`. - **Video replays the script in a second context.** + The recording context navigates to the state's entry URL — the same page the capture started from, not wherever the script happened to leave the captured page — and replays the steps from there, so the recording and the stills are the same run. A state containing a `request` step therefore skips video by default, because a non-idempotent seed would run twice and the video would show twelve drones beside stills showing six. - Set `"allowVideoReplay": true` on the state when the seed is idempotent. + Set `"allowVideoReplay": true` on the state when the seed is idempotent; a state that `extends` such a parent inherits both the step and the flag. +- **A failed recording does not discard the screenshots.** + The stills for a viewport are already on disk when its recording starts, so a video that fails afterwards is reported per viewport in `videoErrors` and the capture still counts as a success. + A run that lost only its videos should not read as a run that captured nothing. - **Selectors are a maintenance liability.** A crawl adapts to a site that changed; a script does not. That is the trade scripted states make — the crawler's zero-maintenance property for reach into states a crawler cannot see. @@ -591,6 +640,7 @@ type CaptureReport = { failedStepIndex?: number; // -1 for a whole-state failure screenshots: string[]; // viewport names that produced a triple hasVideo: boolean; + videoErrors?: string[]; // ": " — stills kept, video lost error?: string; }>; }; diff --git a/src/docs.test.ts b/src/docs.test.ts index 9839cd3..7608629 100644 --- a/src/docs.test.ts +++ b/src/docs.test.ts @@ -36,6 +36,7 @@ describe("README ↔ USAGE", () => { "--state-filter", "--skip-routes", "--state-timeout", + "--precondition-timeout", "--allow-state-requests", "--fail-on-state-error", ]; @@ -63,6 +64,10 @@ describe("README ↔ schema", () => { const rows: ReadonlyArray = [ ["states", "`[]`"], ["stateTimeout", `\`${CaptureConfig.Default.stateTimeout}\``], + [ + "preconditionTimeout", + `\`${CaptureConfig.Default.preconditionTimeout}\``, + ], ["captureRoutes", `\`${CaptureConfig.Default.captureRoutes}\``], ["allowStateRequests", `\`${CaptureConfig.Default.allowStateRequests}\``], ]; diff --git a/src/errors.ts b/src/errors.ts index 99c6285..9c6957d 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -67,8 +67,11 @@ export class StateDefinitionError extends S.TaggedError()( * an action that threw, a seeding request that returned the wrong status, or * the whole state exceeding its budget. * - * `stepIndex: -1` with `stepKind: "state"` denotes a whole-state failure - * (navigation or timeout) rather than one attributable step. + * `stepIndex: -1` denotes a failure that no step is answerable for, with + * `stepKind` naming which one: `"state"` for navigation or the whole-state + * budget, `"precondition"` for a probe that could not be evaluated at all — + * a malformed selector, a page that went away. A precondition that is simply + * *absent* is not a failure and produces no error; the state is `skipped`. */ export class StateCaptureError extends S.TaggedError()( "StateCaptureError", diff --git a/src/report.ts b/src/report.ts index 310c4e9..300f4db 100644 --- a/src/report.ts +++ b/src/report.ts @@ -101,6 +101,12 @@ const generateMarkdown = ( } } + if (result.videoErrors && result.videoErrors.length > 0) { + // A capture that lost its video still lists the screenshots above; + // this is the part that did not happen, named rather than dropped. + md += `**Video capture failed:** ${result.videoErrors.join("; ")}\n\n`; + } + md += "---\n\n"; } @@ -149,6 +155,7 @@ export const generateReports = ( failedStepIndex: result.failedStepIndex, screenshots: Object.keys(result.screenshots), hasVideo: !!result.videos, + videoErrors: result.videoErrors, error: result.error, })), }); diff --git a/src/runner.test.ts b/src/runner.test.ts index d9a1c32..35cd9cc 100644 --- a/src/runner.test.ts +++ b/src/runner.test.ts @@ -317,12 +317,43 @@ describe("USAGE scripted states", () => { expect(USAGE).toContain("--state-filter"); expect(USAGE).toContain("--skip-routes"); expect(USAGE).toContain("--state-timeout"); + expect(USAGE).toContain("--precondition-timeout"); expect(USAGE).toContain("--allow-state-requests"); expect(USAGE).toContain("--fail-on-state-error"); }); }); describe("buildInvocation scripted-state flags — parsing edge cases", () => { + it("carries --precondition-timeout into overrides", () => { + const inv = buildInvocation( + parseCliArgs([ + "https://example.com", + "--states", + "./states.json", + "--precondition-timeout", + "25000", + ]), + ); + expect(inv.overrides.preconditionTimeout).toBe(25000); + }); + + it("leaves preconditionTimeout unset when the flag is absent", () => { + const inv = buildInvocation(parseCliArgs(["https://example.com"])); + expect(inv.overrides.preconditionTimeout).toBeUndefined(); + }); + + it("rejects a non-numeric --precondition-timeout", () => { + expect(() => + buildInvocation( + parseCliArgs([ + "https://example.com", + "--precondition-timeout", + "later", + ]), + ), + ).toThrow(/--precondition-timeout must be an integer/); + }); + it("rejects a non-numeric --state-timeout", () => { expect(() => buildInvocation( diff --git a/src/runner.ts b/src/runner.ts index 4bdde25..1678e0f 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -66,10 +66,22 @@ Options: named state is performed on a fresh page load and yields its own capture set, so a single-route app's dialogs and workspaces get captured too. - --state-filter Run only these named states (default: all) + --state-filter Capture only these named states (default: all). + A state they extend is replayed to reach them, + and is not captured itself. --skip-routes Capture only scripted states, not crawled routes - --state-timeout Per-state budget covering navigation, script and - capture (default: 30000; a state may override it) + --state-timeout Budget for reaching a state: navigation, + precondition probe and script. Screenshot and + video capture are outside it (default: 60000; + a state may override it with timeoutMs) + --precondition-timeout Budget for a state's precondition probe, which + decides whether the state exists on this build + at all (default: 10000; a state may override it + with preconditionTimeoutMs). Raise it for an app + whose first meaningful frame lands well after + networkidle, such as a WebGL console — a probe + that gives up first records the state as skipped + rather than slow. --allow-state-requests Permit request steps, which reach past the UI into the app's own backend. Off by default: a states file from a colleague should not be able @@ -102,7 +114,7 @@ Examples: ui-capture http://localhost:5173 --states ./states.json --state-filter fleet-editor `; -export const printUsage = (): void => { +const printUsage = (): void => { console.log(USAGE); }; @@ -261,6 +273,14 @@ export const buildInvocation = (parsed: ParsedArgs): CliInvocation => { const stateTimeout = parseInteger(opts["state-timeout"], "--state-timeout"); if (stateTimeout !== undefined) overrides.stateTimeout = stateTimeout; + const preconditionTimeout = parseInteger( + opts["precondition-timeout"], + "--precondition-timeout", + ); + if (preconditionTimeout !== undefined) { + overrides.preconditionTimeout = preconditionTimeout; + } + const statesOpt = opts.states; const statesPath = typeof statesOpt === "string" diff --git a/src/schemas.test.ts b/src/schemas.test.ts index 7c37d77..238df66 100644 --- a/src/schemas.test.ts +++ b/src/schemas.test.ts @@ -122,7 +122,10 @@ describe("createCaptureConfig scripted states", () => { const cfg = createCaptureConfig(); expect(cfg.states).toEqual([]); expect(cfg.captureRoutes).toBe(true); - expect(cfg.stateTimeout).toBe(30000); + // Raised from 30000 when the budget was narrowed to the reach phase: it + // has to exceed the 30 s navigation timeout, or a slow first load can + // spend the whole budget and leave the script none of it. + expect(cfg.stateTimeout).toBe(60000); expect(cfg.allowStateRequests).toBe(false); }); diff --git a/src/schemas.ts b/src/schemas.ts index 9144370..28ec62f 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -202,14 +202,33 @@ export class CaptureState extends S.Class("CaptureState")({ * A selector probed on the fresh load, before any step. When it is absent, * the state is recorded as `skipped` rather than `failed`: "this state does * not exist here" is a different event from "this state's script is broken". + * + * A selector that cannot be *evaluated* — a typo, a malformed CSS — is + * neither: the state fails, because a probe that silently answers "not here" + * to a broken selector produces a green run with nothing captured. */ precondition: S.optional(S.String), + /** + * Budget for this state's `precondition` probe, overriding + * `preconditionTimeout` for the run. + * + * Per state because readiness is not uniform: a state gated on a nav link + * present at first paint should not wait as long as one gated on a WebGL + * console's first frame, and a probe that gives up early reports the state + * as *absent here* rather than slow. + */ + preconditionTimeoutMs: S.optional(S.Number.pipe(S.int(), S.positive())), /** * Restrict this state to named viewports. The script runs once and the * viewport loop resizes afterwards, so a dialog that unmounts below a * breakpoint would otherwise be screenshotted as the boot view. + * + * An empty array is rejected rather than treated as "none": it would + * capture zero screenshots and still be reported as captured, and a state + * that captured nothing must never report success. Omit the field to use + * every configured viewport. */ - viewports: S.optional(S.Array(S.String)), + viewports: S.optional(S.Array(S.String).pipe(S.minItems(1))), steps: S.Array(CaptureStep), /** Whole-state budget: navigation + script + capture. */ timeoutMs: S.optional(S.Number.pipe(S.int(), S.positive())), @@ -247,6 +266,15 @@ export class CaptureResult extends S.Class("CaptureResult")({ failedStepIndex: S.optional(S.Number.pipe(S.int())), screenshots: S.Record({ key: S.String, value: ScreenshotPaths }), videos: S.optional(S.Record({ key: S.String, value: VideoQualityPaths })), + /** + * Per-viewport video failures on a capture whose screenshots landed. + * + * A failed recording does not un-write the stills that are already on + * disk, so it is reported here rather than through `error`: the capture + * stays a success and says what it lost, instead of discarding good work + * and counting as a failure that produced nothing. + */ + videoErrors: S.optional(S.Array(S.String)), error: S.optional(S.String), timestamp: S.Number.pipe(S.int()), }) {} @@ -265,6 +293,22 @@ export class VideoOptions extends S.Class("VideoOptions")( }); } +/** + * Default budget for a state's `precondition` probe. + * + * Generous on purpose. The probe runs after `goto` has settled, but "settled" + * is a network fact rather than a rendering one: an app that boots a WebGL + * scene, or hydrates and then fetches, reaches its first meaningful frame + * seconds later. A probe that gives up first reports the state as *not present + * here* — the one outcome that yields a green run with nothing captured. Ten + * seconds still sits well inside the whole-state budget, so a state that + * genuinely does not exist here skips cheaply instead of consuming it. + * + * Lives here rather than in the driver so the config default and the driver's + * own fallback cannot drift apart. + */ +export const DEFAULT_PRECONDITION_TIMEOUT_MS = 10000; + const CaptureConfigFields = { outputDir: S.String, captureVideo: S.Boolean, @@ -294,8 +338,29 @@ const CaptureConfigFields = { * without a states file behaves exactly as it always has. */ states: S.Array(CaptureState), - /** Default whole-state budget in ms; a state may override it. */ + /** + * Default budget in ms for *reaching* a state: navigation, the + * `precondition` probe and the script. A state may override it. + * + * It deliberately stops there. Screenshot and video capture are bounded by + * their own timeouts and by `videoOptions.duration` × viewport count, and + * folding them in made the default unsatisfiable: a state captured with + * `--video` could not fit a 30 s budget on any configuration, so every + * state timed out. The default exceeds the 30 s navigation timeout so a + * slow first load still leaves the script a budget to run in. + */ stateTimeout: S.Number.pipe(S.int(), S.positive()), + /** + * Budget for a state's `precondition` probe; a state may override it with + * its own `preconditionTimeoutMs`. + * + * Separate from `stateTimeout`, and much smaller, because the two answer + * different questions. The probe decides whether the state *exists here* at + * all, and its cost is paid in full by every state that legitimately does + * not — so it has to be long enough for a slow-booting app to reach first + * paint, and short enough that a skip is not the run's dominant cost. + */ + preconditionTimeout: S.Number.pipe(S.int(), S.positive()), /** Crawl and capture routes. `false` captures only scripted states. */ captureRoutes: S.Boolean, /** @@ -329,7 +394,8 @@ export class CaptureConfig extends S.Class("CaptureConfig")( launchArgs: [], colorScheme: "light", states: [], - stateTimeout: 30000, + stateTimeout: 60000, + preconditionTimeout: DEFAULT_PRECONDITION_TIMEOUT_MS, captureRoutes: true, allowStateRequests: false, }); @@ -354,6 +420,8 @@ export class CaptureReport extends S.Class("CaptureReport")({ failedStepIndex: S.optional(S.Number.pipe(S.int())), screenshots: S.Array(S.String), hasVideo: S.Boolean, + /** Viewports whose video failed while their screenshots succeeded. */ + videoErrors: S.optional(S.Array(S.String)), error: S.optional(S.String), }), ), @@ -399,6 +467,7 @@ export type CaptureConfigOverrides = Partial<{ colorScheme: "light" | "dark" | "no-preference"; states: ReadonlyArray; stateTimeout: number; + preconditionTimeout: number; captureRoutes: boolean; allowStateRequests: boolean; }>; @@ -463,6 +532,8 @@ export const createCaptureConfig = ( : base.launchArgs, colorScheme: overrides.colorScheme ?? base.colorScheme, stateTimeout: overrides.stateTimeout ?? base.stateTimeout, + preconditionTimeout: + overrides.preconditionTimeout ?? base.preconditionTimeout, captureRoutes: overrides.captureRoutes ?? base.captureRoutes, allowStateRequests: overrides.allowStateRequests ?? base.allowStateRequests, }); diff --git a/src/service.ts b/src/service.ts index a6f27ae..fd80f53 100644 --- a/src/service.ts +++ b/src/service.ts @@ -39,9 +39,11 @@ import { } from "./schemas.js"; import { captureScreenshots } from "./screenshot.js"; import { + closeQuietly, createHostFilterState, getCaptureDir, getRouteName, + isAllowedOrigin, navigationRetryPolicy, normalizeUrl, type QueueTask, @@ -53,6 +55,7 @@ import { import { createScriptedStateRunner, INITIAL_STEP_PROGRESS, + type StepProgress, } from "./state-script.js"; import { type ResolvedState, validateStates } from "./states.js"; import { captureVideoForViewport } from "./video.js"; @@ -68,11 +71,29 @@ const DEFAULT_LAUNCH_ARGS = [ "--disable-dev-shm-usage", ] as const; +/** + * Playwright's own ceiling on one `goto`, named here because the state budget + * has to exceed it: a budget below this can be spent entirely on a slow first + * load, leaving the script none of it. + */ +const STATE_NAVIGATION_TIMEOUT_MS = 30000; + export class CaptureConfigTag extends Context.Tag("CaptureConfig")< CaptureConfigTag, CaptureConfig >() {} +/** One viewport's video outcome: what was recorded, or why nothing was. */ +interface ViewportVideoOutcome { + readonly paths: Option.Option; + readonly error: Option.Option; +} + +const NO_VIDEO_CAPTURED: ViewportVideoOutcome = { + paths: Option.none(), + error: Option.none(), +}; + /** A one-line, report-ready rendering of any failure this service can raise. */ const formatCaptureFailure = (error: unknown): string => { if (error instanceof FileSystemError) { @@ -131,6 +152,12 @@ export class UICaptureService extends Effect.Service()( let browser: Browser | null = null; const processedRoutes = new Set(); const hostFilters = createHostFilterState(); + // Hydrated beside the host filters, and for the same reason: the seed + // is not known until `captureWebsite` is called, but the runtime + // `request` gate needs it to compare scheme and port the way the + // pre-launch pass does. Null until then, which denies rather than + // allows. + let seedUrl: URL | null = null; const initialize = Effect.tryPromise({ try: async () => { @@ -176,9 +203,23 @@ export class UICaptureService extends Effect.Service()( // Link discovery opens menus so links become *discoverable*; the state // runner performs a named script so a state becomes *capturable*. Two // page-manipulation toolkits from one config, neither owning the other. - const { runStateScript, checkPrecondition } = createScriptedStateRunner( - {}, - ); + const { runStateScript, checkPrecondition } = createScriptedStateRunner({ + // The gate that actually decides whether a `request` step may be + // sent. `validateStates` checked request origins before launch, but + // against each state's configured URL; a script that navigates + // first resolves its paths against an origin that pass never saw, + // so the same `isAllowedOrigin` comparison is handed to the driver + // to apply to the URL each step resolves to at the moment it runs. + // Same comparison, same seed: a states file that validated cannot + // be widened at runtime, and a run cannot abort on something the + // runtime would have allowed. + isAllowedRequestUrl: (candidate) => + seedUrl !== null && + isAllowedOrigin(candidate, seedUrl, (hostname) => + hostFilters.hostMatchesFilters(hostname, cfg.includeSubdomains), + ), + preconditionTimeoutMs: cfg.preconditionTimeout, + }); /** * What a scripted state changes about a capture: where it lands, which @@ -256,29 +297,52 @@ export class UICaptureService extends Effect.Service()( }, ); - const videos = - wantVideo && browser - ? Option.some( - yield* captureVideoForViewport( - browser, - page, - viewport, - captureDir, - timestamp, - { - waitTime: cfg.waitTime, - ffmpegPath: cfg.ffmpegPath, - videoOptions: cfg.videoOptions, - colorScheme: cfg.colorScheme, - ...(stateContext - ? { prepare: stateContext.prepare } - : {}), - }, - ), - ) - : Option.none(); + // This viewport's screenshots are on disk by now. A + // recording that fails afterwards — the replay script, a + // dead context, ffmpeg — must not un-write them or turn a + // capture that produced files into one that reports nothing. + const video = yield* wantVideo && browser + ? captureVideoForViewport( + browser, + viewport, + captureDir, + timestamp, + { + waitTime: cfg.waitTime, + ffmpegPath: cfg.ffmpegPath, + videoOptions: cfg.videoOptions, + colorScheme: cfg.colorScheme, + // Where this capture began, not `page.url()`: a + // scripted state has already driven the page, so + // its current URL is where the script *ended* — + // replaying from there records a different run + // than the stills show. + startUrl: url, + ...(stateContext + ? { prepare: stateContext.prepare } + : {}), + }, + ).pipe( + Effect.map( + (paths): ViewportVideoOutcome => ({ + paths: Option.some(paths), + error: Option.none(), + }), + ), + Effect.catchAll((error) => { + const message = formatCaptureFailure(error); + console.warn( + ` ! Video failed for ${viewport.name}, screenshots kept: ${message}`, + ); + return Effect.succeed({ + paths: Option.none(), + error: Option.some(`${viewport.name}: ${message}`), + }); + }), + ) + : Effect.succeed(NO_VIDEO_CAPTURED); - return [viewport.name, { screenshots, videos }] as const; + return [viewport.name, { screenshots, video }] as const; }), ), { concurrency: 1 }, @@ -289,18 +353,25 @@ export class UICaptureService extends Effect.Service()( for (const [name, data] of screenshotResults) { screenshots[name] = data.screenshots; - if (Option.isSome(data.videos)) { - videos[name] = data.videos.value; + if (Option.isSome(data.video.paths)) { + videos[name] = data.video.paths.value; } } + const videoErrors = screenshotResults.flatMap(([, data]) => + Option.isSome(data.video.error) ? [data.video.error.value] : [], + ); + return new CaptureResult({ url, route, state: stateContext?.name, + // Screenshots landed, so this is a capture that succeeded and + // names what it lost — not a failure that kept nothing. stateStatus: stateContext ? "captured" : undefined, screenshots, videos: Object.keys(videos).length > 0 ? videos : undefined, + videoErrors: videoErrors.length > 0 ? videoErrors : undefined, timestamp: Date.now(), }); }); @@ -395,9 +466,12 @@ export class UICaptureService extends Effect.Service()( ) : cfg.viewports; + // Both halves read the *resolved* state: `extends` prepends the + // parent's steps, so a child inherits the `request` step that + // suppresses video, and must inherit the opt-out with it. const usesRequests = steps.some((step) => step.kind === "request"); const stateCaptureVideo = - cfg.captureVideo && (!usesRequests || state.allowVideoReplay); + cfg.captureVideo && (!usesRequests || resolved.allowVideoReplay); if (cfg.captureVideo && !stateCaptureVideo) { console.log( ` ! Skipping video for "${state.name}": recording replays the script in a second context, so a non-idempotent request step would seed twice and the video would disagree with the stills (set allowVideoReplay to override)`, @@ -426,7 +500,110 @@ export class UICaptureService extends Effect.Service()( ), ); + /** Names what the state was doing when its budget expired. */ + const timedOutDoing = (at: StepProgress): string => { + switch (at.phase) { + case "navigate": + return `while loading ${task.url}`; + case "precondition": + return `while probing precondition "${at.target}"`; + case "settle": + return `while settling after step ${at.index} (${at.kind} "${at.target}")`; + case "done": + return "after its last step, with the script already complete"; + default: + return `on step ${at.index} (${at.kind} "${at.target}")`; + } + }; + + /** + * Everything `budgetMs` covers, and nothing else: the navigation, + * the `precondition` probe and the script — the part a wrong + * selector can hang on forever. + * + * Capture is deliberately outside it. Screenshots and video are + * bounded by their own timeouts and by `--video-duration` × the + * viewport count, and folding them into one whole-state budget made + * the default unsatisfiable: with `--video` on, a single state + * could not fit any default budget, so every state timed out. + */ + const reachState = ( + page: Page, + ): Effect.Effect<"ready" | "skipped", StateCaptureError> => + Effect.gen(function* () { + yield* Ref.set(progress, { + index: -1, + kind: "navigate", + target: task.url, + phase: "navigate", + }); + + yield* Effect.tryPromise({ + try: () => + page + .goto(task.url, { + waitUntil: "networkidle", + timeout: STATE_NAVIGATION_TIMEOUT_MS, + }) + .then(() => undefined), + catch: (error) => + stateFailure(`failed to navigate to ${task.url}`, error), + }).pipe(Effect.retry(navigationRetryPolicy)); + + if (state.precondition !== undefined) { + yield* Ref.set(progress, { + index: -1, + kind: "precondition", + target: state.precondition, + phase: "precondition", + }); + const present = yield* checkPrecondition( + page, + state.name, + state.precondition, + state.preconditionTimeoutMs, + ); + if (!present) { + console.log( + ` - State "${state.name}" skipped: precondition "${state.precondition}" is not present on ${task.url}`, + ); + return "skipped" as const; + } + } + + yield* runStateScript(page, state.name, steps, progress); + return "ready" as const; + }).pipe( + Effect.timeout(budgetMs), + Effect.catchTag("TimeoutException", () => + Effect.gen(function* () { + const at = yield* Ref.get(progress); + return yield* Effect.fail( + new StateCaptureError({ + state: state.name, + // A step index is only meaningful while a step is what + // was running; naming the last completed step for a + // navigation or probe hang points the reader at code + // that already worked. + stepIndex: + at.phase === "step" || at.phase === "settle" + ? at.index + : -1, + stepKind: at.kind, + target: at.target, + message: `state "${state.name}" timed out after ${budgetMs}ms ${timedOutDoing(at)}`, + cause: null, + }), + ); + }), + ), + ); + const runInContext = Effect.acquireUseRelease( + // The context is acquired on its own. Effect registers a release + // only once its acquire has *completed*, so an acquire holding + // two resources strands the first when the second throws: a + // rejecting `newPage` used to leak the context for the whole run. Effect.gen(function* () { if (!browser) { return yield* Effect.fail( @@ -434,99 +611,58 @@ export class UICaptureService extends Effect.Service()( ); } const browserRef = browser; - const context = yield* Effect.tryPromise({ + return yield* Effect.tryPromise({ try: () => browserRef.newContext({ colorScheme: cfg.colorScheme }), catch: (error) => stateFailure("failed to create a browser context", error), }); - const page = yield* Effect.tryPromise({ - try: () => context.newPage(), - catch: (error) => - stateFailure("failed to create a page", error), - }); - return { context, page }; }), - ({ page }) => - Effect.gen(function* () { - yield* Effect.tryPromise({ - try: () => - page - .goto(task.url, { - waitUntil: "networkidle", - timeout: 30000, - }) - .then(() => undefined), + (context) => + Effect.acquireUseRelease( + Effect.tryPromise({ + try: () => context.newPage(), catch: (error) => - stateFailure(`failed to navigate to ${task.url}`, error), - }).pipe(Effect.retry(navigationRetryPolicy)); + stateFailure("failed to create a page", error), + }), + (page) => + Effect.gen(function* () { + const outcome = yield* reachState(page); + + if (outcome === "skipped") { + results.set( + task.resultKey, + new CaptureResult({ + url: task.url, + route: getRouteName(task.url), + state: state.name, + stateStatus: "skipped", + screenshots: {}, + timestamp: Date.now(), + }), + ); + return; + } - if (state.precondition !== undefined) { - const present = yield* checkPrecondition( - page, - state.precondition, - ); - if (!present) { - console.log( - ` - State "${state.name}" skipped: precondition "${state.precondition}" is not present on ${task.url}`, - ); - results.set( - task.resultKey, - new CaptureResult({ - url: task.url, - route: getRouteName(task.url), - state: state.name, - stateStatus: "skipped", - screenshots: {}, - timestamp: Date.now(), - }), + const result = yield* capturePage(page, task.url, { + name: state.name, + viewports: stateViewports, + captureVideo: stateCaptureVideo, + prepare, + }).pipe( + Effect.mapError((error) => + stateFailure(formatCaptureFailure(error), error), + ), ); - return; - } - } - yield* runStateScript(page, state.name, steps, progress); - - const result = yield* capturePage(page, task.url, { - name: state.name, - viewports: stateViewports, - captureVideo: stateCaptureVideo, - prepare, - }).pipe( - Effect.mapError((error) => - stateFailure(formatCaptureFailure(error), error), - ), - ); - - results.set(task.resultKey, result); - }), - ({ context }) => - Effect.tryPromise({ - try: () => context.close(), - catch: () => undefined, - }).pipe(Effect.catchAll(() => Effect.void)), + results.set(task.resultKey, result); + }), + (page) => closeQuietly(() => page.close()), + ), + (context) => closeQuietly(() => context.close()), ); yield* runInContext.pipe( - Effect.timeout(budgetMs), - Effect.catchTag("TimeoutException", () => - Effect.gen(function* () { - const at = yield* Ref.get(progress); - return yield* Effect.fail( - new StateCaptureError({ - state: state.name, - stepIndex: at.index, - stepKind: at.kind, - target: at.target, - message: - at.index < 0 - ? `state "${state.name}" timed out after ${budgetMs}ms before its first step completed` - : `state "${state.name}" timed out after ${budgetMs}ms on step ${at.index} (${at.kind} "${at.target}")`, - cause: null, - }), - ); - }), - ), Effect.catchAll((error) => Effect.gen(function* () { const message = formatCaptureFailure(error); @@ -567,6 +703,7 @@ export class UICaptureService extends Effect.Service()( console.log("Starting UI capture for:", url); const urlObj = new URL(url); hostFilters.hydrate(urlObj.hostname, cfg.allowedHosts); + seedUrl = urlObj; // Authoring errors abort before Chromium ever launches: no amount of // retrying makes a typo'd `extends` resolve. @@ -613,7 +750,6 @@ export class UICaptureService extends Effect.Service()( type: "state" as const, url: stateUrl, stateName: entry.state.name, - normalizedUrl: normalizeUrl(stateUrl), resultKey: stateResultKey(stateUrl, entry.state.name), }; }, @@ -756,6 +892,13 @@ export class UICaptureService extends Effect.Service()( } }); + // Nested rather than one acquire holding both, for the + // reason `processStateTask` spells out: Effect registers a + // release only once its acquire has *completed*, so an + // acquire that takes the context and then the page strands + // the context when `newPage` rejects — a leaked context for + // the rest of the run, on the one path where something is + // already going wrong. const createWorker = ( workerId: number, ): Effect.Effect => @@ -771,7 +914,7 @@ export class UICaptureService extends Effect.Service()( ); } const browserRef = browser; - const context = yield* Effect.tryPromise({ + return yield* Effect.tryPromise({ try: () => browserRef.newContext({ colorScheme: cfg.colorScheme, @@ -783,24 +926,26 @@ export class UICaptureService extends Effect.Service()( cause: error, }), }); - const page = yield* Effect.tryPromise({ - try: () => context.newPage(), - catch: (error) => - new CaptureError({ - url, - message: `Worker ${workerId}: Failed to create page`, - cause: error, - }), - }); - console.log(`✓ Worker ${workerId} ready`); - return { context, page }; }), - ({ page }) => workerLoop(page, workerId), - ({ context }) => - Effect.tryPromise({ - try: () => context.close(), - catch: () => undefined, - }).pipe(Effect.catchAll(() => Effect.void)), + (context) => + Effect.acquireUseRelease( + Effect.tryPromise({ + try: () => context.newPage(), + catch: (error) => + new CaptureError({ + url, + message: `Worker ${workerId}: Failed to create page`, + cause: error, + }), + }), + (page) => + Effect.gen(function* () { + console.log(`✓ Worker ${workerId} ready`); + return yield* workerLoop(page, workerId); + }), + (page) => closeQuietly(() => page.close()), + ), + (context) => closeQuietly(() => context.close()), ); // States are seeded alongside the seed route, before any diff --git a/src/shared.ts b/src/shared.ts index 4d42a44..256bb23 100644 --- a/src/shared.ts +++ b/src/shared.ts @@ -37,7 +37,13 @@ export type StateTask = { readonly type: "state"; readonly url: string; readonly stateName: string; - readonly normalizedUrl: string; + /** + * Where the state's result lands. A state is a capture leaf — it never + * feeds the crawl frontier and is never deduplicated by URL — so the + * normalized URL a `RouteTask` needs has no reader here, and carrying one + * would only invite a caller to key a state by it and collide with the + * route capture for the same page. + */ readonly resultKey: string; }; @@ -49,6 +55,26 @@ export type QueueTask = RouteTask | StateTask | ShutdownTask; export const ShutdownSignal: ShutdownTask = { type: "shutdown" } as const; +/** + * Best-effort teardown, for the release half of an `acquireUseRelease`. + * + * Closing a context or a page is what reaps its browser-side resources — and + * for a recording context, what flushes the video to disk — so it has to run + * on every exit path, including an interrupted one. It must never fail: a + * `close()` that rejects on an already-dead target would otherwise replace the + * real error with a teardown error and lose the reason the run stopped. + * + * Lives here because both the service (state and worker contexts, worker + * pages) and the video recorder need exactly this, and two copies are two + * chances for one of them to start reporting its failures. + */ +export const closeQuietly = ( + close: () => Promise, +): Effect.Effect => + Effect.tryPromise({ try: close, catch: () => undefined }).pipe( + Effect.catchAll(() => Effect.void), + ); + export const LINK_FILTER_CONCURRENCY = 32; export const navigationRetryPolicy = Schedule.recurs(3); export const captureRetryPolicy = Schedule.recurs(2); @@ -200,6 +226,34 @@ export const createHostFilterState = (): HostFilterState => { }; }; +/** + * The origin gate every URL check in this codebase must agree on. + * + * An origin is **scheme + host + port**, so that is what gets compared: + * `hostMatchesFilters` decides the host (it canonicalizes `www.` and honors + * `--allowed-hosts` / `--include-subdomains`), and the seed decides the scheme + * and the port. Matching on hostname alone let `http://app.test:4000` through + * a filter whose whole purpose was to confine a run to `https://app.test` — + * a different port and a downgraded scheme are different servers, and for a + * `request` step that means a POST at a machine the user never named. + * + * `URL.port` is already normalized (`""` for a scheme's default), so + * `https://a.test` and `https://a.test:443` compare equal without special + * casing. + * + * Both the pre-launch validation in `states.ts` and the runtime request gate + * call this, so a states file that validates cannot be widened at runtime and + * a run cannot abort on something the runtime would have allowed. + */ +export const isAllowedOrigin = ( + candidate: URL, + seed: URL, + hostMatchesFilters: (hostname: string) => boolean, +): boolean => + candidate.protocol === seed.protocol && + candidate.port === seed.port && + hostMatchesFilters(candidate.hostname); + export const normalizeUrl = (url: string): string => { try { const u = new URL(url); diff --git a/src/state-plan.test.ts b/src/state-plan.test.ts index 154c456..5fe5d4e 100644 --- a/src/state-plan.test.ts +++ b/src/state-plan.test.ts @@ -17,10 +17,12 @@ import { COUNT_POLL_INTERVAL_MS, DEFAULT_STEP_TIMEOUT_MS, describeStep, + isCountableState, isExpectedStatus, planStep, resolveRequestUrl, StepPlanError, + stepShapeError, } from "./state-plan.js"; const decodeStep = S.decodeUnknownSync(CaptureStep); @@ -254,3 +256,191 @@ describe("isExpectedStatus", () => { expect(isExpectedStatus(200, 201)).toBe(false); }); }); + +describe("stepShapeError", () => { + it("passes every shape the vocabulary can actually carry out", () => { + const fine = [ + { kind: "waitFor", selector: ".row", minCount: 3 }, + { kind: "waitFor", selector: ".row", state: "attached", minCount: 3 }, + { kind: "waitFor", selector: ".row", state: "hidden" }, + { kind: "waitFor", selector: ".row", state: "detached" }, + { kind: "press", key: "Escape" }, + { kind: "press", key: "Enter", selector: "#form", timeoutMs: 900 }, + { kind: "click", selector: "#a", timeoutMs: 900 }, + ]; + for (const step of fine) { + expect(stepShapeError(decodeStep(step)), JSON.stringify(step)).toBe( + undefined, + ); + } + }); + + it("rejects minCount on a state that also passes when nothing matches", () => { + // `hidden` and `detached` both succeed on an empty match set, so a + // minimum *over matches* cannot express them; letting minCount through + // would give the counting path and the selector path two different + // meanings for the same word. + for (const state of ["hidden", "detached"]) { + const message = stepShapeError( + decodeStep({ kind: "waitFor", selector: ".row", state, minCount: 2 }), + ); + expect(message).toContain("minCount counts matching elements"); + expect(message).toContain(`state "${state}"`); + expect(message).toContain('use state "visible" or "attached"'); + } + }); + + it("rejects a timeoutMs on a press with nothing to wait for", () => { + const message = stepShapeError( + decodeStep({ kind: "press", key: "Escape", timeoutMs: 9000 }), + ); + expect(message).toContain("timeoutMs has no effect on an untargeted press"); + expect(message).toContain("add a selector, or drop timeoutMs"); + }); +}); + +describe("planStep — inexpressible shapes", () => { + it("throws rather than planning one of the two meanings", () => { + expect(() => + planStep( + decodeStep({ + kind: "waitFor", + selector: ".row", + state: "hidden", + minCount: 2, + }), + ctx, + ), + ).toThrow(StepPlanError); + expect(() => + planStep(decodeStep({ kind: "press", key: "Escape", timeoutMs: 1 }), ctx), + ).toThrow(StepPlanError); + }); + + it("names the step it refused, so the failure reads like any other", () => { + try { + planStep( + decodeStep({ + kind: "waitFor", + selector: ".row", + state: "detached", + minCount: 2, + }), + ctx, + ); + throw new Error("expected planStep to throw"); + } catch (error) { + expect(error).toBeInstanceOf(StepPlanError); + expect((error as StepPlanError).kind).toBe("waitFor"); + expect((error as StepPlanError).target).toBe(".row"); + } + }); + + it("only ever counts a state a count can mean something in", () => { + expect(isCountableState("visible")).toBe(true); + expect(isCountableState("attached")).toBe(true); + expect(isCountableState("hidden")).toBe(false); + expect(isCountableState("detached")).toBe(false); + }); +}); + +describe("planStep — press timeouts", () => { + it("carries no timeout at all on an untargeted press", () => { + // Not "carries one the driver silently drops": `page.keyboard.press` has + // no element to wait for, and a plan that pretends otherwise is how a + // per-step timeoutMs came to be accepted and ignored. + expect( + planStep(decodeStep({ kind: "press", key: "Escape" }), { + ...ctx, + defaultTimeoutMs: 12000, + }).plan, + ).toEqual({ + op: "press", + key: "Escape", + selector: undefined, + timeoutMs: undefined, + }); + }); + + it("carries the resolved timeout on a targeted press", () => { + expect( + planStep(decodeStep({ kind: "press", key: "Enter", selector: "#form" }), { + ...ctx, + defaultTimeoutMs: 12000, + }).plan, + ).toMatchObject({ selector: "#form", timeoutMs: 12000 }); + }); +}); + +describe("planStep — the request host gate", () => { + // The same comparison both gates run: scheme, host and port. + const allow = (origin: string) => (url: URL) => url.origin === origin; + + it("judges the host the path resolves to, against the page it resolves from", () => { + expect(() => + planStep( + decodeStep({ kind: "request", method: "POST", path: "/api/seed" }), + { + pageUrl: "https://evil.test/landing", + isAllowedRequestUrl: allow("https://app.example.com"), + }, + ), + ).toThrow(/outside the allowed origins/); + }); + + it("quotes both the resolved URL and the page it came from", () => { + try { + planStep(decodeStep({ kind: "request", method: "GET", path: "/api/x" }), { + pageUrl: "https://evil.test/landing", + isAllowedRequestUrl: allow("https://app.example.com"), + }); + throw new Error("expected planStep to throw"); + } catch (error) { + expect(error).toBeInstanceOf(StepPlanError); + expect((error as StepPlanError).message).toContain( + "https://evil.test/api/x", + ); + expect((error as StepPlanError).message).toContain( + "https://evil.test/landing", + ); + expect((error as StepPlanError).target).toBe("GET /api/x"); + } + }); + + it("rejects an absolute off-host URL the same way", () => { + expect(() => + planStep( + decodeStep({ + kind: "request", + method: "GET", + path: "https://other.test/api", + }), + { ...ctx, isAllowedRequestUrl: allow("https://app.example.com") }, + ), + ).toThrow(StepPlanError); + }); + + it("plans a request the gate accepts", () => { + expect( + planStep(decodeStep({ kind: "request", method: "GET", path: "/api/x" }), { + ...ctx, + isAllowedRequestUrl: allow("https://app.example.com"), + }).plan, + ).toMatchObject({ url: "https://app.example.com/api/x" }); + }); + + it("applies no gate when the caller supplies none, because planning is pure", () => { + // A caller that plans without driving a page is not making a request; + // the driver never omits the gate. See `createScriptedStateRunner`. + expect( + planStep( + decodeStep({ + kind: "request", + method: "GET", + path: "https://other.test/api", + }), + ctx, + ).plan, + ).toMatchObject({ url: "https://other.test/api" }); + }); +}); diff --git a/src/state-plan.ts b/src/state-plan.ts index 0eb15be..f7fbc68 100644 --- a/src/state-plan.ts +++ b/src/state-plan.ts @@ -24,6 +24,16 @@ * `state-script.ts` is the only place that drives a `Page`, so the whole * vocabulary — defaults, timeout precedence, URL resolution, the human-facing * labels in failure messages — is testable without launching Chromium. + * + * It is also where the two gates that *decide* rather than describe live, and + * they live here for the same reason: {@link stepShapeError} rejects a step + * whose fields contradict each other, and the `request` host check in + * {@link planStep} runs against the URL the step actually resolves to. Both are + * mirrored by pre-launch checks in `states.ts` so an authoring mistake aborts + * before Chromium launches, but the copies here are the authoritative ones: + * a plan is never built without them, whatever the caller did or did not + * validate first. Both gates run the same comparison the pre-launch pass does + * — `isAllowedOrigin` for a request — so neither can be wider than the other. */ import type { CaptureStep } from "./schemas.js"; @@ -36,6 +46,22 @@ export const COUNT_POLL_INTERVAL_MS = 100; export type ElementState = "visible" | "hidden" | "attached" | "detached"; +/** + * The DOM states a `minCount` wait can count. + * + * `minCount` is a minimum over *matched elements*, so it only means something + * for a state an element can be matched in. `hidden` and `detached` both treat + * "there is no such element at all" as a pass on the selector path, which no + * minimum count can express — counting would silently redefine them. Those + * combinations are rejected by {@link stepShapeError} instead. + */ +export type CountableState = Extract; + +/** Narrows an {@link ElementState} to one `minCount` can count. */ +export const isCountableState = ( + state: ElementState, +): state is CountableState => state === "visible" || state === "attached"; + export type StepPlan = | { readonly op: "waitForSelector"; @@ -47,7 +73,7 @@ export type StepPlan = readonly op: "waitForCount"; readonly selector: string; readonly minCount: number; - readonly state: ElementState; + readonly state: CountableState; readonly timeoutMs: number; readonly pollMs: number; } @@ -74,7 +100,13 @@ export type StepPlan = readonly op: "press"; readonly key: string; readonly selector: string | undefined; - readonly timeoutMs: number; + /** + * Only the targeted path takes one. Without a `selector` the key goes + * to `page.keyboard`, which has no element to wait for and accepts no + * timeout, so the plan carries none rather than one the driver would + * quietly drop. + */ + readonly timeoutMs: number | undefined; } | { readonly op: "request"; @@ -115,6 +147,26 @@ export interface PlanContext { readonly pageUrl: string; /** Action default when a step omits `timeoutMs`. */ readonly defaultTimeoutMs?: number; + /** + * The authoritative origin gate for `request` steps. + * + * `validateStates` also checks request origins before Chromium launches, but + * against the state's *configured* URL, while a path resolves at runtime + * against the live {@link pageUrl} — a script that clicks through to another + * origin first resolves against one the pre-launch pass never saw. The gate + * therefore has to be applied where the resolution happens, which is here; + * the pre-launch check is a convenience that fails the run early. + * + * The predicate takes the resolved URL rather than its hostname so both + * gates can be the same `isAllowedOrigin` comparison — scheme, host and + * port — and a states file that validated cannot be widened at runtime. + * + * Omitted, no gate is applied: planning is a pure function, and a caller + * that plans without driving a page is not making a request. The driver + * never omits it — see `createScriptedStateRunner`, which falls back to a + * same-origin-as-the-page gate when its own option is unset. + */ + readonly isAllowedRequestUrl?: (url: URL) => boolean; } /** @@ -134,8 +186,14 @@ export class StepPlanError extends Error { } } -/** The value a failure message quotes for each step kind. */ -export const stepTarget = (step: CaptureStep): string => { +/** + * The value a failure message quotes for each step kind. + * + * Internal: callers outside this module want {@link describeStep}, which is + * the whole human-facing label, or `PlannedStep.target`, which is this value + * already attached to the step it belongs to. + */ +const stepTarget = (step: CaptureStep): string => { switch (step.kind) { case "waitFor": case "click": @@ -191,11 +249,51 @@ export const resolveRequestUrl = ( return resolved; }; +/** + * Step shapes the schema admits but that cannot be carried out as written. + * + * The schema validates each field on its own; these are the combinations whose + * fields contradict *each other*, and the alternative to rejecting them is + * worse than a hard error — one field silently redefining another is how a + * script comes to mean something its author never wrote. + * + * Returns the reason, or `undefined` when the step is expressible. One + * function, two callers: `validateStates` runs it before Chromium launches so + * an authoring mistake aborts the run, and {@link planStep} runs it on every + * step it plans so no caller reaches the driver around it. + */ +export const stepShapeError = (step: CaptureStep): string | undefined => { + if ( + step.kind === "waitFor" && + step.minCount !== undefined && + !isCountableState(step.state) + ) { + return `minCount counts matching elements, but state "${step.state}" also passes when nothing matches at all, so a minimum over matches cannot express it; use state "visible" or "attached" with minCount, or drop minCount to wait for the first match to become ${step.state}`; + } + if ( + step.kind === "press" && + step.selector === undefined && + step.timeoutMs !== undefined + ) { + return `timeoutMs has no effect on an untargeted press: with no selector the key goes to page.keyboard, which has no element to wait for; add a selector, or drop timeoutMs`; + } + return undefined; +}; + /** * Turns one declarative step into the operation a driver performs, applying * the timeout precedence: step `timeoutMs`, then the action default. + * + * Throws {@link StepPlanError} for a step that cannot be planned: a shape + * {@link stepShapeError} rejects, a `request` path that will not resolve, or a + * `request` that resolves outside `ctx.isAllowedRequestUrl`. */ export const planStep = (step: CaptureStep, ctx: PlanContext): PlannedStep => { + const shapeError = stepShapeError(step); + if (shapeError !== undefined) { + throw new StepPlanError(step.kind, stepTarget(step), shapeError); + } + const timeoutMs = step.timeoutMs ?? ctx.defaultTimeoutMs ?? DEFAULT_STEP_TIMEOUT_MS; const common = { @@ -211,20 +309,25 @@ export const planStep = (step: CaptureStep, ctx: PlanContext): PlannedStep => { return { ...common, plan: - step.minCount === undefined + // The state re-test is what proves to the type system that a + // counting plan only ever carries a countable state; the throw + // above has already rejected the alternative, and if it were ever + // removed this degrades to the plain selector wait rather than + // counting something `hidden` does not mean. + step.minCount !== undefined && isCountableState(step.state) ? { - op: "waitForSelector", + op: "waitForCount", selector: step.selector, + minCount: step.minCount, state: step.state, timeoutMs, + pollMs: COUNT_POLL_INTERVAL_MS, } : { - op: "waitForCount", + op: "waitForSelector", selector: step.selector, - minCount: step.minCount, state: step.state, timeoutMs, - pollMs: COUNT_POLL_INTERVAL_MS, }, }; case "wait": @@ -266,16 +369,30 @@ export const planStep = (step: CaptureStep, ctx: PlanContext): PlannedStep => { op: "press", key: step.key, selector: step.selector, - timeoutMs, + // An untargeted press carries no timeout at all, rather than one + // the driver would compute and then silently drop; an explicit + // one on that shape was rejected by stepShapeError above. + timeoutMs: step.selector === undefined ? undefined : timeoutMs, }, }; - case "request": + case "request": { + const url = resolveRequestUrl(step.path, ctx.pageUrl); + if ( + ctx.isAllowedRequestUrl !== undefined && + !ctx.isAllowedRequestUrl(url) + ) { + throw new StepPlanError( + "request", + stepTarget(step), + `resolves to "${url.toString()}" against the page URL "${ctx.pageUrl}", which is outside the allowed origins: an origin is scheme + host + port`, + ); + } return { ...common, plan: { op: "request", method: step.method, - url: resolveRequestUrl(step.path, ctx.pageUrl).toString(), + url: url.toString(), json: step.json, hasJson: step.json !== undefined, headers: { ...(step.headers ?? {}) }, @@ -283,6 +400,7 @@ export const planStep = (step: CaptureStep, ctx: PlanContext): PlannedStep => { timeoutMs, }, }; + } case "reload": return { ...common, diff --git a/src/state-script.test.ts b/src/state-script.test.ts index d60052a..8f842aa 100644 --- a/src/state-script.test.ts +++ b/src/state-script.test.ts @@ -16,8 +16,10 @@ import type { Page } from "playwright"; import { describe, expect, it, vi } from "vitest"; import type { StateCaptureError } from "./errors.js"; import { + CaptureConfig, type CaptureStep, CaptureStep as CaptureStepSchema, + DEFAULT_PRECONDITION_TIMEOUT_MS, } from "./schemas.js"; import { createScriptedStateRunner, @@ -42,6 +44,16 @@ interface ElementBehaviour { readonly counts?: readonly number[]; /** Per-index visibility; missing indices are visible. */ readonly visible?: readonly boolean[]; + /** Delay each `isVisible()` answer, to make a counting pass slow. */ + readonly visibleDelayMs?: number; + /** `count()` never settles at all. */ + readonly countHangs?: boolean; + /** + * `count()` rejects with this message — how a malformed selector behaves: + * real Playwright rejects with "Unexpected token ... while parsing css + * selector" whether or not any element would have matched. + */ + readonly countRejects?: string; /** When set, the corresponding action rejects with this message. */ readonly waitFor?: string; readonly click?: string; @@ -104,8 +116,14 @@ const createFakePage = ( return failure ? reject(failure) : Promise.resolve(); }, isVisible: () => { - const visible = behaviourFor(selector).visible?.[index] ?? true; - return Promise.resolve(visible); + const behaviour = behaviourFor(selector); + const visible = behaviour.visible?.[index] ?? true; + calls.push({ op: "isVisible", selector, index, visible }); + return behaviour.visibleDelayMs === undefined + ? Promise.resolve(visible) + : new Promise((resolve) => + setTimeout(() => resolve(visible), behaviour.visibleDelayMs), + ); }, }); @@ -114,6 +132,14 @@ const createFakePage = ( nth: (index: number) => element(selector, index), count: () => { const behaviour = behaviourFor(selector); + if (behaviour.countHangs) { + calls.push({ op: "count", selector, hung: true }); + return new Promise(() => {}); + } + if (behaviour.countRejects) { + calls.push({ op: "count", selector, rejected: behaviour.countRejects }); + return reject(behaviour.countRejects); + } if (behaviour.counts) { const cursor = pollCursor.get(selector) ?? 0; pollCursor.set(selector, cursor + 1); @@ -601,26 +627,69 @@ describe("runStateScript — sequencing and progress", () => { index: 2, kind: "waitFor", target: ".fleet-row", + phase: "step", }); }); it("starts from a whole-state progress marker before any step runs", async () => { - const { page } = createFakePage(); - const { progress } = await run(page, []); - expect(progress).toEqual(INITIAL_STEP_PROGRESS); expect(INITIAL_STEP_PROGRESS.index).toBe(-1); expect(INITIAL_STEP_PROGRESS.kind).toBe("state"); + expect(INITIAL_STEP_PROGRESS.phase).toBe("navigate"); + }); + + it("marks the script done rather than leaving the last step named", async () => { + // Updated with the phase field: an empty script *completes*, so leaving + // the ref on the pre-navigation marker would have the whole-state + // timeout report a state that finished its script as still navigating. + const { page } = createFakePage(); + const { progress } = await run(page, []); + expect(progress).toEqual({ ...INITIAL_STEP_PROGRESS, phase: "done" }); + }); + + it("names the last step as settling, not running, during its settleMs", async () => { + // The off-by-one this closes: the ref still named step 0 for the whole + // of its settle delay, so a budget that expired there blamed a step + // that had already succeeded. + const { page } = createFakePage(); + const runner = createScriptedStateRunner(); + const progress = Effect.runSync(Ref.make(INITIAL_STEP_PROGRESS)); + await Effect.runPromiseExit( + runner + .runStateScript( + page, + "demo", + [decodeStep({ kind: "click", selector: "#a", settleMs: 5000 })], + progress, + ) + .pipe(Effect.timeout(50)), + ); + expect(Effect.runSync(Ref.get(progress))).toEqual({ + index: 0, + kind: "click", + target: "#a", + phase: "settle", + }); }); }); describe("checkPrecondition", () => { - it("is true when the selector is present", async () => { + it("evaluates the selector before it starts waiting on it", async () => { + // The `count()` is not incidental: it is the whole discriminator. A + // malformed selector rejects there whether or not an element would have + // matched, which is what separates "not present here" from "this + // selector is broken" without any page-side evaluation. const { page, calls } = createFakePage(); const runner = createScriptedStateRunner({ preconditionTimeoutMs: 1234 }); await expect( - Effect.runPromise(runner.checkPrecondition(page, "[data-advanced]")), + Effect.runPromise( + runner.checkPrecondition(page, "demo", "[data-advanced]"), + ), ).resolves.toBe(true); expect(calls[0]).toMatchObject({ + op: "count", + selector: "[data-advanced]", + }); + expect(calls[1]).toMatchObject({ op: "waitFor", state: "visible", timeout: 1234, @@ -631,11 +700,282 @@ describe("checkPrecondition", () => { // "This state does not exist here" is a different event from "this // state's script is broken", and only the second is a failure. const { page } = createFakePage({ - elements: { "[data-advanced]": { waitFor: "Timeout" } }, + elements: { "[data-advanced]": { waitFor: "Timeout 50ms exceeded." } }, }); const runner = createScriptedStateRunner({ preconditionTimeoutMs: 50 }); await expect( - Effect.runPromise(runner.checkPrecondition(page, "[data-advanced]")), + Effect.runPromise( + runner.checkPrecondition(page, "demo", "[data-advanced]"), + ), ).resolves.toBe(false); }); + + it("fails the state when the selector cannot be evaluated at all", async () => { + // The failure this exists to prevent: a typo'd selector answering "not + // present here", the state skipping, and the run going green having + // captured nothing. + const { page } = createFakePage({ + elements: { + "##typo": { + countRejects: + 'locator.count: Unexpected token "#" while parsing css selector "##typo".', + }, + }, + }); + const runner = createScriptedStateRunner({ preconditionTimeoutMs: 50 }); + const exit = await Effect.runPromiseExit( + runner.checkPrecondition(page, "demo", "##typo"), + ); + const error = failureOf(exit as Exit.Exit); + expect(error._tag).toBe("StateCaptureError"); + expect(error.stepKind).toBe("precondition"); + expect(error.stepIndex).toBe(-1); + expect(error.target).toBe("##typo"); + expect(error.message).toContain("could not evaluate its precondition"); + expect(error.message).toContain("while parsing css selector"); + }); + + it("fails, rather than skipping, when the wait dies for a non-timeout reason", async () => { + const { page } = createFakePage({ + elements: { + "[data-advanced]": { + waitFor: "Target page, context or browser has been closed", + }, + }, + }); + const runner = createScriptedStateRunner({ preconditionTimeoutMs: 50 }); + const exit = await Effect.runPromiseExit( + runner.checkPrecondition(page, "demo", "[data-advanced]"), + ); + expect( + failureOf(exit as Exit.Exit).message, + ).toContain("has been closed"); + }); + + it("lets a state override the run's probe budget", async () => { + const { page, calls } = createFakePage(); + const runner = createScriptedStateRunner({ preconditionTimeoutMs: 1000 }); + await Effect.runPromise( + runner.checkPrecondition(page, "demo", "[data-advanced]", 25000), + ); + expect(callsOfKind(calls, "waitFor")[0]).toMatchObject({ timeout: 25000 }); + }); + + it("falls back to the shared default when nothing configures it", async () => { + // Ten seconds, not five: the probe runs after `networkidle`, which is a + // network fact rather than a rendering one, and an app that reaches its + // first meaningful frame later than the probe is reported as *absent*. + const { page, calls } = createFakePage(); + await Effect.runPromise( + createScriptedStateRunner({}).checkPrecondition( + page, + "demo", + "[data-advanced]", + ), + ); + expect(callsOfKind(calls, "waitFor")[0]).toMatchObject({ + timeout: DEFAULT_PRECONDITION_TIMEOUT_MS, + }); + // The config default and the driver fallback are the same constant; this + // is what stops them drifting apart in opposite directions. + expect(CaptureConfig.Default.preconditionTimeout).toBe( + DEFAULT_PRECONDITION_TIMEOUT_MS, + ); + }); +}); + +describe("runStateScript — the request host gate", () => { + it("judges the URL the step resolves to against the live page, not the configured one", async () => { + // The bypass this closes: a script navigates somewhere else first, so a + // relative path resolves against an origin the pre-launch check never + // saw. The gate has to run where the resolution happens. + const { page } = createFakePage({ url: "https://evil.test/landing" }); + const { exit } = await run( + page, + [{ kind: "request", method: "POST", path: "/api/seed" }], + { isAllowedRequestUrl: (url) => url.hostname === "app.example.com" }, + ); + const error = failureOf(exit); + expect(error.stepKind).toBe("request"); + expect(error.target).toBe("POST /api/seed"); + expect(error.message).toContain("https://evil.test/api/seed"); + expect(error.message).toContain("outside the allowed origins"); + }); + + it("sends nothing when the gate rejects", async () => { + const { page, calls } = createFakePage({ + url: "https://evil.test/landing", + }); + await run( + page, + [{ kind: "request", method: "DELETE", path: "/api/fleet" }], + { + isAllowedRequestUrl: (url) => url.hostname === "app.example.com", + }, + ); + expect(callsOfKind(calls, "request")).toHaveLength(0); + }); + + it("allows a request the gate accepts", async () => { + const { page, calls } = createFakePage({ + url: "https://app.example.com/console", + }); + const { exit } = await run( + page, + [{ kind: "request", method: "GET", path: "/api/ping" }], + { isAllowedRequestUrl: (url) => url.hostname === "app.example.com" }, + ); + expect(Exit.isSuccess(exit)).toBe(true); + expect(callsOfKind(calls, "request")).toHaveLength(1); + }); + + it("does not widen to another port on an allowed host", async () => { + // The reason the gate takes a URL rather than a hostname: the pre-launch + // pass compares scheme, host *and* port, so a script that navigates to + // another port on the same host must not be able to reach a backend the + // validated file could not. + const { page, calls } = createFakePage({ + url: "https://app.example.com:9000/console", + }); + const { exit } = await run( + page, + [{ kind: "request", method: "POST", path: "/api/seed" }], + { + isAllowedRequestUrl: (url) => url.origin === "https://app.example.com", + }, + ); + expect(failureOf(exit).message).toContain("outside the allowed origins"); + expect(callsOfKind(calls, "request")).toHaveLength(0); + }); + + it("falls back to the page's own origin when no gate is configured", async () => { + // Fail closed. A caller that forgets to pass a filter gets the property + // the `path` form is supposed to guarantee, not an open door. + const { page, calls } = createFakePage({ + url: "https://app.example.com/console", + }); + const { exit } = await run(page, [ + { kind: "request", method: "POST", path: "https://other.test/api/seed" }, + ]); + expect(failureOf(exit).message).toContain("outside the allowed origins"); + expect(callsOfKind(calls, "request")).toHaveLength(0); + }); + + it("denies everything when the page URL will not parse", async () => { + // about:blank and friends: nothing to be same-origin with, so nothing is. + const { page, calls } = createFakePage({ url: "about:blank" }); + const { exit } = await run(page, [ + { kind: "request", method: "GET", path: "https://app.example.com/api" }, + ]); + expect(failureOf(exit).message).toContain("outside the allowed origins"); + expect(callsOfKind(calls, "request")).toHaveLength(0); + }); +}); + +describe("runStateScript — inexpressible step shapes", () => { + it("refuses minCount with a state that also passes on no match", async () => { + for (const state of ["hidden", "detached"] as const) { + const { page, calls } = createFakePage(); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", state, minCount: 3 }, + ]); + const error = failureOf(exit); + expect(error.stepKind).toBe("waitFor"); + expect(error.message).toContain("minCount counts matching elements"); + expect(error.message).toContain(`state "${state}"`); + // Refused, not attempted with one of the two meanings guessed at. + expect(calls).toEqual([]); + } + }); + + it("still allows minCount with visible and attached", async () => { + for (const state of ["visible", "attached"] as const) { + const { page } = createFakePage({ elements: { ".row": { count: 4 } } }); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", state, minCount: 3 }, + ]); + expect(Exit.isSuccess(exit)).toBe(true); + } + }); + + it("refuses a timeoutMs on a press that has no element to wait for", async () => { + const { page, calls } = createFakePage(); + const { exit } = await run(page, [ + { kind: "press", key: "Escape", timeoutMs: 9000 }, + ]); + const error = failureOf(exit); + expect(error.stepKind).toBe("press"); + expect(error.message).toContain("timeoutMs has no effect on an untargeted"); + expect(calls).toEqual([]); + }); + + it("leaves an untargeted press without a timeout alone", async () => { + const { page, calls } = createFakePage(); + const { exit } = await run(page, [{ kind: "press", key: "Escape" }], { + defaultStepTimeoutMs: 9000, + }); + expect(Exit.isSuccess(exit)).toBe(true); + expect(calls).toEqual([{ op: "keyboard.press", key: "Escape" }]); + }); + + it("keeps honouring timeoutMs on a targeted press", async () => { + const { page, calls } = createFakePage(); + await run(page, [ + { kind: "press", key: "Enter", selector: "#form", timeoutMs: 900 }, + ]); + expect(calls[0]).toMatchObject({ op: "press", timeout: 900 }); + }); +}); + +describe("runStateScript — a counting pass respects its own deadline", () => { + it("stops inspecting matches once the step's timeout has passed", async () => { + // A pass that began inside the budget must not run on past it: one round + // trip per match means a selector matching enough elements would + // otherwise outlive the step timeout entirely. + const { page, calls } = createFakePage({ + elements: { ".row": { count: 40, visibleDelayMs: 20 } }, + }); + const started = Date.now(); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", minCount: 40, timeoutMs: 120 }, + ]); + const elapsed = Date.now() - started; + const error = failureOf(exit); + expect(error.message).toContain("the step deadline passed"); + expect(error.message).toContain("of 40 checked"); + // 40 × 20ms is 800ms of inspection; the deadline is 120ms. + expect(elapsed).toBeLessThan(500); + expect(callsOfKind(calls, "isVisible").length).toBeLessThan(40); + }); + + it("gives up on a page call that never settles at all", async () => { + // No in-loop deadline can reach a promise that never resolves, so the + // counting pass carries a hard backstop; without it the step outlives + // its own timeout and eats the whole-state budget instead. + const { page } = createFakePage({ + elements: { ".row": { countHangs: true } }, + }); + const started = Date.now(); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", minCount: 1, timeoutMs: 100 }, + ]); + const error = failureOf(exit); + expect(error.stepKind).toBe("waitFor"); + expect(error.message).toContain("never settled"); + expect(Date.now() - started).toBeLessThan(3000); + }); + + it("still reports the last count taken while the step could run", async () => { + // The deadline check must not degrade the message into "found 0": the + // count worth reading is the last one taken inside the budget. + const { page } = createFakePage({ + elements: { ".row": { count: 4, visible: [true, false, false, true] } }, + }); + const { exit } = await run(page, [ + { kind: "waitFor", selector: ".row", minCount: 3, timeoutMs: 150 }, + ]); + expect(failureOf(exit).message).toContain( + 'expected >=3 matching "visible", found 2', + ); + }); }); diff --git a/src/state-script.ts b/src/state-script.ts index 8300577..bc9cb59 100644 --- a/src/state-script.ts +++ b/src/state-script.ts @@ -29,9 +29,12 @@ import { Effect, Ref } from "effect"; import type { Locator, Page } from "playwright"; import { StateCaptureError } from "./errors.js"; -import type { CaptureStep } from "./schemas.js"; import { - type ElementState, + type CaptureStep, + DEFAULT_PRECONDITION_TIMEOUT_MS, +} from "./schemas.js"; +import { + type CountableState, isExpectedStatus, type PlannedStep, planStep, @@ -42,8 +45,37 @@ import { /** How much of a failing response body a failure message quotes. */ const RESPONSE_BODY_EXCERPT = 200; -/** Default budget for the `precondition` probe on the freshly loaded page. */ -const DEFAULT_PRECONDITION_TIMEOUT_MS = 5000; +/** + * Slack on the hard backstop around a `minCount` counting pass. + * + * The pass enforces its own deadline and produces the message worth reading + * ("found 2 after 4000ms"), so the backstop has to lose that race in every + * normal case. It exists for the pathological one: a single `count()` or + * `isVisible()` that never settles, where the loop never reaches its own + * check and the step would otherwise outlive its timeout entirely. + */ +const COUNT_BACKSTOP_GRACE_MS = 500; + +/** + * What a state is doing right now, not merely which step it last started. + * + * `index`/`kind`/`target` alone are ambiguous at a boundary: they still name + * the last step for the whole of its settle delay and for everything that + * happens after the script returns, so a budget that expired *there* would be + * reported against a step that had already succeeded. `phase` is what makes + * the reading honest. + */ +export type StepPhase = + /** Loading the page, before any step. */ + | "navigate" + /** Probing the state's `precondition` on the fresh load. */ + | "precondition" + /** Inside the step named by `index`. */ + | "step" + /** The step named by `index` succeeded; serving its `settleMs`. */ + | "settle" + /** Every step finished. */ + | "done"; /** * The step a state is currently on. Held in a `Ref` so the whole-state timeout @@ -54,6 +86,7 @@ export interface StepProgress { readonly index: number; readonly kind: string; readonly target: string; + readonly phase: StepPhase; } /** Before any step runs: navigation, or the state as a whole. */ @@ -61,6 +94,7 @@ export const INITIAL_STEP_PROGRESS: StepProgress = { index: -1, kind: "state", target: "", + phase: "navigate", }; export interface ScriptedStateRunner { @@ -70,18 +104,57 @@ export interface ScriptedStateRunner { steps: ReadonlyArray, progress: Ref.Ref, ) => Effect.Effect; + /** + * Probes a state's `precondition` on the freshly loaded page. + * + * Succeeds with `true` when the selector became visible and `false` when it + * legitimately did not — that second answer is what makes a state `skipped` + * rather than `failed`. It *fails* when the precondition could not be + * evaluated at all: a malformed selector, a page that went away. Collapsing + * those two into `false` is what let a typo'd selector produce a green run + * with no capture, so they are different outcomes here. + */ readonly checkPrecondition: ( page: Page, + stateName: string, selector: string, - ) => Effect.Effect; + timeoutMs?: number, + ) => Effect.Effect; } export interface ScriptedStateRunnerOptions { /** Action default when a step omits `timeoutMs`. */ readonly defaultStepTimeoutMs?: number; + /** Budget for the `precondition` probe; a state may override it. */ readonly preconditionTimeoutMs?: number; + /** + * The authoritative origin gate for `request` steps, applied to the URL each + * step resolves to from the *live* page URL. + * + * `validateStates` checks request origins before Chromium launches too, but + * against the state's configured URL; a script that navigates first resolves + * its paths against an origin that pass never saw. Left unset, the runner + * falls back to "same origin as the page", which is the property the `path` + * form is supposed to guarantee — fail closed, so a caller that forgets to + * pass a gate gets the narrow one rather than none. + */ + readonly isAllowedRequestUrl?: (url: URL) => boolean; } +/** + * A Playwright timeout, as distinct from a selector that could not be + * evaluated at all. + * + * Matched by name rather than by `instanceof errors.TimeoutError`: a duplicate + * playwright copy in the module graph makes class identity unreliable, and the + * message test catches a timeout re-wrapped on its way out. A malformed + * selector rejects with a plain `Error` ("Unexpected token ... while parsing + * css selector"), so the two never collide. + */ +const isTimeoutFailure = (error: unknown): boolean => + error instanceof Error && + (error.name === "TimeoutError" || /\btimeout\b/i.test(error.message)); + const errorMessage = (error: unknown): string => { if (error instanceof Error) { const [first] = error.message.split("\n"); @@ -90,17 +163,29 @@ const errorMessage = (error: unknown): string => { return String(error); }; +/** + * Counts matches in the requested DOM state, from Node — `locator.count()` and + * `locator.isVisible()`, never an injected predicate. + * + * `deadline` is the step's own, checked between elements: a selector matching + * thousands of nodes takes one round trip each, and a pass that began inside + * the budget must not run on past it merely because it started in time. + */ const countMatching = async ( locator: Locator, - state: ElementState, + state: CountableState, + deadline: number, ): Promise => { const total = await locator.count(); if (state === "attached") return total; - if (state === "detached") return total === 0 ? 1 : 0; let matched = 0; for (let i = 0; i < total; i++) { - const visible = await locator.nth(i).isVisible(); - if (state === "visible" ? visible : !visible) matched += 1; + if (Date.now() >= deadline) { + throw new Error( + `the step deadline passed while inspecting matches: ${i} of ${total} checked, ${matched} matched so far`, + ); + } + if (await locator.nth(i).isVisible()) matched += 1; } return matched; }; @@ -115,7 +200,25 @@ const countMatching = async ( export const createScriptedStateRunner = ( options: ScriptedStateRunnerOptions = {}, ): ScriptedStateRunner => { - const { defaultStepTimeoutMs, preconditionTimeoutMs } = options; + const { defaultStepTimeoutMs, preconditionTimeoutMs, isAllowedRequestUrl } = + options; + + /** + * The gate `planStep` applies to a `request` before it becomes a plan. + * + * Fails closed: with no configured gate the only origin allowed is the one + * the page is actually on, and a page URL that will not parse allows none. + */ + const requestUrlGate = (pageUrl: string): ((url: URL) => boolean) => { + if (isAllowedRequestUrl !== undefined) return isAllowedRequestUrl; + let pageOrigin: string; + try { + pageOrigin = new URL(pageUrl).origin; + } catch { + return () => false; + } + return (url) => url.origin === pageOrigin; + }; const stepError = ( stateName: string, @@ -164,19 +267,32 @@ export const createScriptedStateRunner = ( const locator = page.locator(plan.selector); const deadline = Date.now() + plan.timeoutMs; let found = 0; - for (;;) { - found = await countMatching(locator, plan.state); + // A pass is only ever *started* inside the budget, so the + // count this reports is the last one taken while the step was + // still entitled to run — not a degraded final pass. + do { + found = await countMatching(locator, plan.state, deadline); if (found >= plan.minCount) return; - if (Date.now() >= deadline) { - throw new Error( - `expected >=${plan.minCount} matching "${plan.state}", found ${found} after ${plan.timeoutMs}ms`, - ); - } + if (Date.now() >= deadline) break; await page.waitForTimeout(plan.pollMs); - } + } while (Date.now() < deadline); + throw new Error( + `expected >=${plan.minCount} matching "${plan.state}", found ${found} after ${plan.timeoutMs}ms`, + ); }, catch: (error) => fail(errorMessage(error), error), - }); + }).pipe( + // The loop's own deadline governs; this only catches a page call + // that never settles, which no in-loop check can reach. + Effect.timeoutFail({ + duration: plan.timeoutMs + COUNT_BACKSTOP_GRACE_MS, + onTimeout: () => + fail( + `a page call made while counting matches never settled; abandoned after ${plan.timeoutMs + COUNT_BACKSTOP_GRACE_MS}ms`, + null, + ), + }), + ); case "click": return Effect.tryPromise({ @@ -209,17 +325,22 @@ export const createScriptedStateRunner = ( catch: (error) => fail(errorMessage(error), error), }); - case "press": + case "press": { + // Destructured so the untargeted path is visibly the one that has no + // timeout to apply, rather than one that silently drops a computed + // one — `planStep` carries `undefined` there for the same reason. + const { selector, timeoutMs } = plan; return Effect.tryPromise({ try: () => - plan.selector === undefined + selector === undefined ? page.keyboard.press(plan.key) : page - .locator(plan.selector) + .locator(selector) .first() - .press(plan.key, { timeout: plan.timeoutMs }), + .press(plan.key, { timeout: timeoutMs }), catch: (error) => fail(errorMessage(error), error), }); + } case "reload": return Effect.tryPromise({ @@ -265,9 +386,11 @@ export const createScriptedStateRunner = ( for (const [index, step] of steps.entries()) { let planned: PlannedStep; try { + const pageUrl = page.url(); planned = planStep(step, { - pageUrl: page.url(), + pageUrl, defaultTimeoutMs: defaultStepTimeoutMs, + isAllowedRequestUrl: requestUrlGate(pageUrl), }); } catch (error) { const kind = error instanceof StepPlanError ? error.kind : step.kind; @@ -290,6 +413,7 @@ export const createScriptedStateRunner = ( index, kind: planned.kind, target: planned.target, + phase: "step", }); const fail = (message: string, cause: unknown) => @@ -319,28 +443,62 @@ export const createScriptedStateRunner = ( } if (planned.settleMs > 0) { + // The step succeeded; a budget that expires in here expired while + // settling, not inside a step that is still running. + yield* Ref.set(progress, { + index, + kind: planned.kind, + target: planned.target, + phase: "settle", + }); yield* Effect.sleep(planned.settleMs); } } + + // Past the last step the caller is no longer inside the script, so the + // last step's name must stop standing in for "what is running now". + yield* Ref.update(progress, (at) => ({ ...at, phase: "done" as const })); }); const checkPrecondition = ( page: Page, + stateName: string, selector: string, - ): Effect.Effect => + timeoutMs?: number, + ): Effect.Effect => Effect.tryPromise({ - try: async () => { - await page - .locator(selector) - .first() - .waitFor({ + try: async (): Promise => { + const locator = page.locator(selector); + // Evaluating the selector once before waiting on it is what keeps + // the two answers apart. A malformed selector rejects here whether + // or not the element exists; a valid selector that matches nothing + // yet counts zero and falls through to the wait below, which is the + // case that legitimately means "not present here". + await locator.count(); + try { + await locator.first().waitFor({ state: "visible", - timeout: preconditionTimeoutMs ?? DEFAULT_PRECONDITION_TIMEOUT_MS, + timeout: + timeoutMs ?? + preconditionTimeoutMs ?? + DEFAULT_PRECONDITION_TIMEOUT_MS, }); + } catch (error) { + if (isTimeoutFailure(error)) return false; + throw error; + } return true; }, - catch: (error) => error, - }).pipe(Effect.catchAll(() => Effect.succeed(false))); + catch: (error) => + new StateCaptureError({ + state: stateName, + stepIndex: -1, + stepKind: "precondition", + target: selector, + message: `state "${stateName}" could not evaluate its precondition "${selector}": ${errorMessage(error)}`, + cause: error, + }), + }); return { runStateScript, checkPrecondition } as const; }; diff --git a/src/states.test.ts b/src/states.test.ts index 439c33b..3704237 100644 --- a/src/states.test.ts +++ b/src/states.test.ts @@ -145,6 +145,38 @@ describe("resolveStateSteps", () => { expect(child?.url).toBe("/console"); }); + it("inherits allowVideoReplay alongside the steps that make it matter", () => { + // The child inherits the parent's `request` step, which is what + // suppresses video. Inheriting the step without its opt-out leaves the + // child unable to undo a decision it never made. + const resolved = resolveStateSteps( + parse([ + { + name: "seeded", + allowVideoReplay: true, + steps: [{ kind: "request", method: "PUT", path: "/api/seed" }], + }, + { + name: "seeded-editor", + extends: "seeded", + steps: [{ kind: "click", selector: "#edit" }], + }, + ]), + ); + expect(resolved.get("seeded")?.allowVideoReplay).toBe(true); + expect(resolved.get("seeded-editor")?.allowVideoReplay).toBe(true); + }); + + it("leaves allowVideoReplay false when no state in the chain sets it", () => { + const resolved = resolveStateSteps( + parse([ + { name: "base", steps: [] }, + { name: "child", extends: "base", steps: [] }, + ]), + ); + expect(resolved.get("child")?.allowVideoReplay).toBe(false); + }); + it("lets a child override the inherited url", () => { const resolved = resolveStateSteps( parse([ @@ -210,7 +242,54 @@ describe("validateStates", () => { hostMatchesFilters: (hostname) => hostname === "app.example.com", }); expect(Exit.isSuccess(exit)).toBe(false); - expect(failureMessage(exit)).toContain("outside the allowed hosts"); + expect(failureMessage(exit)).toContain("outside the allowed origins"); + }); + + it("rejects an allowed host on a different port, because that is a different origin", () => { + const exit = runValidate({ + states: parse([ + { + name: "other-port", + url: "https://app.example.com:8443/x", + steps: [], + }, + ]), + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("scheme + host + port"); + }); + + it("rejects an allowed host on a downgraded scheme", () => { + const exit = runValidate({ + states: parse([ + { name: "plain", url: "http://app.example.com/x", steps: [] }, + ]), + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("outside the allowed origins"); + }); + + it("accepts the seed's own port written explicitly", () => { + const exit = runValidate({ + seedUrl: "http://localhost:5173/", + states: parse([ + { name: "same", url: "http://localhost:5173/console", steps: [] }, + ]), + hostMatchesFilters: (hostname) => hostname === "localhost", + }); + expect(Exit.isSuccess(exit)).toBe(true); + }); + + it("accepts a default port written explicitly, since URL normalises it away", () => { + const exit = runValidate({ + states: parse([ + { name: "explicit", url: "https://app.example.com:443/x", steps: [] }, + ]), + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(true); }); it("rejects a viewport filter naming a viewport that is not configured", () => { @@ -262,7 +341,28 @@ describe("validateStates", () => { hostMatchesFilters: (hostname) => hostname === "app.example.com", }); expect(Exit.isSuccess(exit)).toBe(false); - expect(failureMessage(exit)).toContain("outside the allowed hosts"); + expect(failureMessage(exit)).toContain("outside the allowed origins"); + }); + + it("blocks a request that resolves to the allowed host on another port", () => { + const exit = runValidate({ + states: parse([ + { + name: "seed", + steps: [ + { + kind: "request", + method: "POST", + path: "https://app.example.com:9000/api/seed", + }, + ], + }, + ]), + allowStateRequests: true, + hostMatchesFilters: (hostname) => hostname === "app.example.com", + }); + expect(Exit.isSuccess(exit)).toBe(false); + expect(failureMessage(exit)).toContain("outside the allowed origins"); }); it("checks a request inherited through extends, not just a state's own steps", () => { @@ -278,18 +378,135 @@ describe("validateStates", () => { }); describe("filterStates", () => { - it("keeps ancestors so a filtered run still resolves", () => { + /** + * The previous assertion here was that the ancestors came back in the list + * ("keeps ancestors so a filtered run still resolves"). That was the bug: + * `--state-filter leaf` then captured `base` and `middle` as well, and re-ran + * their steps. Resolution needs the ancestors; capture must not see them. + */ + const chainStates = () => + parse([ + { + name: "base", + url: "/console", + steps: [{ kind: "click", selector: "#b" }], + }, + { + name: "middle", + extends: "base", + steps: [{ kind: "click", selector: "#m" }], + }, + { + name: "leaf", + extends: "middle", + steps: [{ kind: "click", selector: "#l" }], + }, + { name: "unrelated", steps: [] }, + ]); + + it("returns only the named states, never the ancestors they resolve through", () => { + expect( + filterStates(chainStates(), ["leaf"]).map((state) => state.name), + ).toEqual(["leaf"]); + }); + + it("folds the ancestors' steps and inherited url into the named state", () => { + const [leaf] = filterStates(chainStates(), ["leaf"]); + expect(leaf?.steps.map((step) => step.selector)).toEqual([ + "#b", + "#m", + "#l", + ]); + expect(leaf?.url).toBe("/console"); + expect(leaf?.extends).toBeUndefined(); + }); + + it("keeps the child's own url rather than the inherited one", () => { + const states = parse([ + { name: "base", url: "/a", steps: [] }, + { name: "child", extends: "base", url: "/b", steps: [] }, + ]); + expect(filterStates(states, ["child"])[0]?.url).toBe("/b"); + }); + + it("carries the child's other fields through the flattening", () => { const states = parse([ { name: "base", steps: [] }, - { name: "middle", extends: "base", steps: [] }, - { name: "leaf", extends: "middle", steps: [] }, - { name: "unrelated", steps: [] }, + { + name: "child", + extends: "base", + description: "d", + precondition: "[data-x]", + viewports: ["desktop"], + timeoutMs: 1234, + allowVideoReplay: true, + steps: [], + }, + ]); + expect(filterStates(states, ["child"])[0]).toMatchObject({ + description: "d", + precondition: "[data-x]", + viewports: ["desktop"], + timeoutMs: 1234, + allowVideoReplay: true, + }); + }); + + it("carries the inherited allowVideoReplay, not just the child's own", () => { + // The flag and the `request` step that makes it matter travel together: + // the child inherits the seed step, so it has to inherit the opt-out + // too, or the same states file records video unfiltered and silently + // drops it under --state-filter. + const states = parse([ + { + name: "seed", + allowVideoReplay: true, + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + { name: "child", extends: "seed", steps: [] }, + ]); + const [child] = filterStates(states, ["child"]); + expect(child?.steps.some((step) => step.kind === "request")).toBe(true); + expect(child?.allowVideoReplay).toBe(true); + }); + + it("does not re-run an ancestor's request seed as a state of its own", () => { + const states = parse([ + { + name: "seed", + steps: [{ kind: "request", method: "POST", path: "/api/seed" }], + }, + { name: "child", extends: "seed", steps: [] }, ]); - expect(filterStates(states, ["leaf"]).map((state) => state.name)).toEqual([ - "base", - "middle", - "leaf", + const selected = filterStates(states, ["child"]); + expect(selected.map((state) => state.name)).toEqual(["child"]); + expect( + selected.flatMap((state) => + state.steps.filter((step) => step.kind === "request"), + ), + ).toHaveLength(1); + }); + + it("still returns an ancestor when the ancestor is what was named", () => { + expect( + filterStates(chainStates(), ["base"]).map((state) => state.name), + ).toEqual(["base"]); + }); + + it("returns file order and deduplicates a repeated name", () => { + expect( + filterStates(chainStates(), ["unrelated", "base", "base"]).map( + (state) => state.name, + ), + ).toEqual(["base", "unrelated"]); + }); + + it("propagates a broken chain, since a filtered run still has to resolve", () => { + const states = parse([ + { name: "a", extends: "b", steps: [] }, + { name: "b", extends: "a", steps: [] }, ]); + expect(() => filterStates(states, ["a"])).toThrow(/extends cycle/); }); it("throws on a name that matches nothing", () => { @@ -448,6 +665,12 @@ describe("parseStatesFile — rejected shapes", () => { ); }); + it("rejects an empty viewports list, which would capture nothing", () => { + // A state with `viewports: []` used to decode, pass validation vacuously + // and be reported as captured with zero screenshots. + rejects([{ name: "a", steps: [], viewports: [] }], /states\.0\.viewports/); + }); + it("rejects a whole-state timeout of zero", () => { rejects([{ name: "a", steps: [], timeoutMs: 0 }], /states\.0\.timeoutMs/); }); @@ -516,3 +739,76 @@ describe("parseStatesFile — accepted shapes", () => { } }); }); + +describe("validateStates — inexpressible step shapes", () => { + // These abort the run rather than failing one state at a time. A shape that + // contradicts itself cannot start working on a retry, and finding out per + // state per viewport is four identical failures instead of one fixable + // message. `planStep` rejects them again at runtime, so a programmatic + // caller cannot route around this pass. + it("rejects minCount on a state that also passes when nothing matches", () => { + for (const state of ["hidden", "detached"]) { + const states = parse([ + { + name: "fleet", + steps: [ + { kind: "waitFor", selector: ".fleet-row", state, minCount: 6 }, + ], + }, + ]); + const message = failureMessage(runValidate({ states })); + expect(message).toContain("step 0"); + expect(message).toContain("minCount counts matching elements"); + expect(message).toContain(state); + } + }); + + it("accepts minCount with visible and attached", () => { + for (const state of ["visible", "attached"]) { + const states = parse([ + { + name: "fleet", + steps: [ + { kind: "waitFor", selector: ".fleet-row", state, minCount: 6 }, + ], + }, + ]); + expect(Exit.isSuccess(runValidate({ states }))).toBe(true); + } + }); + + it("rejects a timeoutMs on a press with no element to wait for", () => { + const states = parse([ + { + name: "esc", + steps: [{ kind: "press", key: "Escape", timeoutMs: 9000 }], + }, + ]); + const message = failureMessage(runValidate({ states })); + expect(message).toContain("timeoutMs has no effect on an untargeted press"); + }); + + it("leaves a targeted press with a timeoutMs alone", () => { + const states = parse([ + { + name: "submit", + steps: [ + { kind: "press", key: "Enter", selector: "#form", timeoutMs: 9000 }, + ], + }, + ]); + expect(Exit.isSuccess(runValidate({ states }))).toBe(true); + }); + + it("checks inherited steps too, and names the index in the flattened script", () => { + const states = parse([ + { name: "base", steps: [{ kind: "click", selector: "#open" }] }, + { + name: "child", + extends: "base", + steps: [{ kind: "press", key: "Escape", timeoutMs: 10 }], + }, + ]); + expect(failureMessage(runValidate({ states }))).toContain("step 1"); + }); +}); diff --git a/src/states.ts b/src/states.ts index b0e3c44..1c83510 100644 --- a/src/states.ts +++ b/src/states.ts @@ -24,13 +24,25 @@ * errors that abort the run before Chromium launches, because no amount of * retrying makes them resolve. Runtime failures are the opposite: recorded per * state, run continues. That split is the whole failure model. + * + * The `request` origin check here is an early abort, not the boundary. A + * request path resolves against the *live* page URL at the moment the step + * runs, and a script that navigates first moves that base out from under this + * pass, which only ever sees the state's configured URL. The gate that decides + * is the one `planStep` applies at runtime; see `state-plan.ts`. */ import { ArrayFormatter, ParseResult, Schema as S } from "@effect/schema"; import { Effect } from "effect"; import { StateDefinitionError } from "./errors.js"; -import { type CaptureState, type CaptureStep, StatesFile } from "./schemas.js"; -import { resolveRequestUrl, StepPlanError } from "./state-plan.js"; +import { CaptureState, type CaptureStep, StatesFile } from "./schemas.js"; +import { isAllowedOrigin } from "./shared.js"; +import { + describeStep, + resolveRequestUrl, + StepPlanError, + stepShapeError, +} from "./state-plan.js"; /** How deep an `extends` chain may go before it stops being reviewable. */ export const MAX_STATE_CHAIN_DEPTH = 5; @@ -42,6 +54,17 @@ export interface ResolvedState { readonly steps: readonly CaptureStep[]; /** The state's own `url`, or the nearest ancestor's. */ readonly url: string | undefined; + /** + * Set by this state or by any ancestor. + * + * The video suppression it opts out of is triggered by the *resolved* step + * list, so a child that inherits a parent's `request` step inherits the + * suppression; inheriting the opt-out alongside it is what keeps the pair + * from disagreeing. `extends` inherits exactly three things — steps, `url` + * and this flag; `precondition`, `viewports` and `timeoutMs` describe the + * child's own capture and stay per-state. + */ + readonly allowVideoReplay: boolean; } const decodeStatesFile = S.decodeUnknownEither(StatesFile); @@ -103,6 +126,10 @@ const definitionError = ( * Replay costs wall clock and buys the thing that matters — any state runs on * any worker, in any order, with no cross-task coupling. * + * Three things flow down a chain: the steps, `url`, and `allowVideoReplay`. + * Everything else (`precondition`, `viewports`, `timeoutMs`) describes the + * child's own capture rather than the script it replays. + * * Throws {@link StateDefinitionError} on an unknown parent, a cycle, or a * chain deeper than {@link MAX_STATE_CHAIN_DEPTH}. */ @@ -165,6 +192,8 @@ export const resolveStateSteps = ( state, steps: [...(inherited?.steps ?? []), ...state.steps], url: state.url ?? inherited?.url, + allowVideoReplay: + state.allowVideoReplay || (inherited?.allowVideoReplay ?? false), }; resolved.set(state.name, value); return value; @@ -230,6 +259,21 @@ export const validateStates = ( const viewportSet = new Set(viewportNames); + // Parsed once, and up front: it is the origin every state URL and every + // `request` path is measured against, so an unusable seed is a definition + // error rather than a defect thrown from inside the loop. + let seed: URL; + try { + seed = new URL(seedUrl); + } catch { + return Effect.fail( + definitionError( + "(run)", + `seed url "${seedUrl}" is not an absolute URL, so state urls have nothing to resolve against`, + ), + ); + } + for (const entry of resolved.values()) { const { state } = entry; @@ -244,11 +288,11 @@ export const validateStates = ( ), ); } - if (!hostMatchesFilters(stateUrl.hostname)) { + if (!isAllowedOrigin(stateUrl, seed, hostMatchesFilters)) { return Effect.fail( definitionError( state.name, - `url "${stateUrl.toString()}" is outside the allowed hosts`, + `url "${stateUrl.toString()}" is outside the allowed origins: an origin is scheme + host + port, and the seed's is "${seed.origin}"`, ), ); } @@ -268,6 +312,21 @@ export const validateStates = ( } for (const [index, step] of entry.steps.entries()) { + // Fields that contradict each other — a `minCount` on a state no + // count can express, a `timeoutMs` on a press with nothing to wait + // for. `planStep` rejects these again at runtime; catching them here + // is what turns them into a fixable authoring error rather than a + // failed state per viewport. + const shapeError = stepShapeError(step); + if (shapeError !== undefined) { + return Effect.fail( + definitionError( + state.name, + `step ${index} (${describeStep(step)}): ${shapeError}`, + ), + ); + } + if (step.kind !== "request") continue; if (!allowStateRequests) { return Effect.fail( @@ -290,11 +349,11 @@ export const validateStates = ( ), ); } - if (!hostMatchesFilters(requestUrl.hostname)) { + if (!isAllowedOrigin(requestUrl, seed, hostMatchesFilters)) { return Effect.fail( definitionError( state.name, - `step ${index} (request ${step.method} ${step.path}) resolves to "${requestUrl.toString()}", which is outside the allowed hosts`, + `step ${index} (request ${step.method} ${step.path}) resolves to "${requestUrl.toString()}", which is outside the allowed origins: an origin is scheme + host + port, and the seed's is "${seed.origin}"`, ), ); } @@ -305,9 +364,48 @@ export const validateStates = ( }); /** - * Narrows a states list to the named states, keeping every ancestor they - * `extends` so a filtered run still resolves. Throws when a name matches - * nothing, because silently running zero states is the coverage lie again. + * Rewrites a resolved state as a self-contained one: the chain's steps inlined + * in order, the inherited `url` and `allowVideoReplay` made explicit, and + * `extends` dropped so nothing downstream needs the ancestor to still be in + * the list. + * + * Everything {@link resolveStateSteps} inherits has to be written back here, + * not just the steps. `allowVideoReplay` is the one that bites: the `request` + * step which suppresses video is inherited, so a child that carried the step + * but not the parent's opt-out would record video on an unfiltered run and + * silently drop it under `--state-filter`: one file, two different results. + * + * Spread-then-override rather than a field-by-field copy, so a field added to + * {@link CaptureState} later is carried instead of silently lost here. + */ +const flattenChain = (entry: ResolvedState): CaptureState => { + const { state } = entry; + if (state.extends === undefined) return state; + const { extends: _inherited, ...own } = state; + return new CaptureState({ + ...own, + steps: [...entry.steps], + allowVideoReplay: entry.allowVideoReplay, + ...(entry.url !== undefined ? { url: entry.url } : {}), + }); +}; + +/** + * Narrows a states list to the named states, and to those only. + * + * An ancestor reached through `extends` is *resolution* input, not a capture + * target. Returning it alongside the named states — which is what a flat + * "keep the ancestors too" list does — captures states the user did not name + * and re-runs their side-effecting steps, a `request` seed among them, which + * is the opposite of what a filter means. So the chain is resolved here and + * folded into each named state instead: the returned states carry everything + * the chain contributes — the ancestors' steps, the inherited `url` and the + * inherited `allowVideoReplay` — and carry no `extends`, so a filtered run + * captures each named state exactly as an unfiltered one does. + * + * Throws when a name matches nothing, because silently running zero states is + * the coverage lie again, and propagates {@link StateDefinitionError} from + * chain resolution — a filtered run has to resolve before it can be flattened. */ export const filterStates = ( states: ReadonlyArray, @@ -323,17 +421,17 @@ export const filterStates = ( ); } - const keep = new Set(); - const visit = (name: string, seen: ReadonlySet): void => { - if (keep.has(name) || seen.has(name)) return; - const state = byName.get(name); - if (!state) return; - keep.add(name); - if (state.extends !== undefined) { - visit(state.extends, new Set([...seen, name])); - } - }; - for (const name of names) visit(name, new Set()); + // Resolution runs over the *whole* file: the ancestors have to be reachable + // to be folded in, even though none of them is being captured. + const resolved = resolveStateSteps(states); + const selected = new Set(names); - return states.filter((state) => keep.has(state.name)); + // File order, deduplicated: a name repeated in --state-filter must not + // capture the same directory twice. + return states + .filter((state) => selected.has(state.name)) + .map((state) => { + const entry = resolved.get(state.name); + return entry === undefined ? state : flattenChain(entry); + }); }; diff --git a/src/video.test.ts b/src/video.test.ts new file mode 100644 index 0000000..6671a61 --- /dev/null +++ b/src/video.test.ts @@ -0,0 +1,275 @@ +/** + * + * Copyright 2026 Mike Odnis + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + */ + +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { Deferred, Effect, Exit, Fiber } from "effect"; +import type { Browser, Page } from "playwright"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CaptureError } from "./errors.js"; +import { ViewportConfig } from "./schemas.js"; +import { type CaptureVideoConfig, captureVideoForViewport } from "./video.js"; + +const VIEWPORT = new ViewportConfig({ + name: "desktop", + width: 1280, + height: 720, +}); + +/** The URL a capture started at — the entry point of the route or state. */ +const ENTRY_URL = "https://app.example.com/console"; + +interface FakeBrowserOptions { + /** Path returned by `page.video().path()`; `null` models "no video". */ + readonly videoPath?: string | null; + readonly newContextRejects?: Error; + readonly gotoRejects?: Error; +} + +interface FakeBrowser { + readonly browser: Browser; + /** Every `context.close()` this run performed, in order. */ + readonly closed: string[]; + readonly created: string[]; + readonly visited: string[]; + readonly closedPages: number[]; +} + +const makeBrowser = (options: FakeBrowserOptions = {}): FakeBrowser => { + const closed: string[] = []; + const created: string[] = []; + const visited: string[] = []; + const closedPages: number[] = []; + let contextCount = 0; + + const browser = { + newContext: async () => { + if (options.newContextRejects) throw options.newContextRejects; + const id = `context-${contextCount++}`; + created.push(id); + const context = { + newPage: async () => { + const page = { + goto: async (url: string) => { + visited.push(url); + if (options.gotoRejects) throw options.gotoRejects; + return null; + }, + evaluate: async () => undefined, + close: async () => { + closedPages.push(created.length); + }, + video: () => + options.videoPath === null + ? undefined + : { + path: async () => + options.videoPath ?? "/tmp/unused-video.webm", + }, + }; + return page as unknown as Page; + }, + close: async () => { + closed.push(id); + }, + }; + return context; + }, + } as unknown as Browser; + + return { browser, closed, created, visited, closedPages }; +}; + +const config = ( + overrides: Partial = {}, +): CaptureVideoConfig => ({ + waitTime: 0, + // Absent on purpose: every transcode fails fast with ENOENT, which is the + // already-tolerated path (logged, master still returned). + ffmpegPath: path.join(os.tmpdir(), "ui-capture-no-such-ffmpeg"), + videoOptions: { duration: 0, interactions: false }, + colorScheme: "light", + startUrl: ENTRY_URL, + ...overrides, +}); + +describe("captureVideoForViewport", () => { + let dir: string; + let log: ReturnType; + let error: ReturnType; + + beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "ui-capture-video-")); + await fs.mkdir(path.join(dir, "videos", "high-quality"), { + recursive: true, + }); + log = vi.spyOn(console, "log").mockImplementation(() => {}); + error = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(async () => { + log.mockRestore(); + error.mockRestore(); + await fs.rm(dir, { recursive: true, force: true }); + }); + + const run = ( + fake: FakeBrowser, + cfg: Partial = {}, + ): Effect.Effect => + captureVideoForViewport( + fake.browser, + VIEWPORT, + dir, + "2026-01-01T00-00-00-000Z", + config(cfg), + ); + + describe("recording context lifetime", () => { + it("closes the recording context on the happy path", async () => { + const raw = path.join(dir, "raw.webm"); + await fs.writeFile(raw, "video"); + const fake = makeBrowser({ videoPath: raw }); + + const exit = await Effect.runPromiseExit(run(fake)); + + expect(Exit.isSuccess(exit)).toBe(true); + expect(fake.closed).toEqual(fake.created); + }); + + it("closes the recording context when the replay fails", async () => { + const raw = path.join(dir, "raw.webm"); + await fs.writeFile(raw, "video"); + const fake = makeBrowser({ videoPath: raw }); + + const exit = await Effect.runPromiseExit( + run(fake, { + prepare: () => + Effect.fail( + new CaptureError({ + url: ENTRY_URL, + message: "state script blew up during replay", + cause: null, + }), + ), + }), + ); + + expect(Exit.isFailure(exit)).toBe(true); + expect(fake.created).toHaveLength(1); + expect(fake.closed).toEqual(fake.created); + }); + + it("closes the recording context when navigation fails", async () => { + const fake = makeBrowser({ + videoPath: path.join(dir, "raw.webm"), + gotoRejects: new Error("net::ERR_CONNECTION_REFUSED"), + }); + + const exit = await Effect.runPromiseExit(run(fake)); + + expect(Exit.isFailure(exit)).toBe(true); + expect(fake.closed).toEqual(fake.created); + }); + + it("closes the recording context when the fiber is interrupted", async () => { + const fake = makeBrowser({ videoPath: path.join(dir, "raw.webm") }); + const replaying = await Effect.runPromise(Deferred.make()); + + const fiber = Effect.runFork( + run(fake, { + prepare: () => + Deferred.succeed(replaying, undefined).pipe( + Effect.zipRight(Effect.never), + ), + }), + ); + + // Interrupt only once the replay is actually in flight, so the + // interruption lands inside the acquired context rather than before it. + await Effect.runPromise(Deferred.await(replaying)); + const exit = await Effect.runPromise(Fiber.interrupt(fiber)); + + expect(Exit.isInterrupted(exit)).toBe(true); + expect(fake.created).toHaveLength(1); + expect(fake.closed).toEqual(fake.created); + }); + + it("closes the recording context when the state budget times out", async () => { + const fake = makeBrowser({ videoPath: path.join(dir, "raw.webm") }); + + // The real interruption path: `Effect.timeout` on the state budget in + // service.ts cancels the fiber mid-replay. + const exit = await Effect.runPromiseExit( + run(fake, { prepare: () => Effect.never }).pipe(Effect.timeout(25)), + ); + + expect(Exit.isFailure(exit)).toBe(true); + expect(fake.created).toHaveLength(1); + expect(fake.closed).toEqual(fake.created); + }); + }); + + describe("replay entry point", () => { + it("navigates to the URL the capture began at, not the post-script URL", async () => { + const raw = path.join(dir, "raw.webm"); + await fs.writeFile(raw, "video"); + const fake = makeBrowser({ videoPath: raw }); + + // A state script that navigates leaves the captured page on a detail + // view; the video must still replay from the state's entry point. + await Effect.runPromiseExit( + run(fake, { + startUrl: ENTRY_URL, + prepare: () => Effect.void, + }), + ); + + expect(fake.visited).toEqual([ENTRY_URL]); + expect(fake.visited).not.toContain( + "https://app.example.com/console/drone/42", + ); + }); + }); + + describe("master recording", () => { + it("renames the raw recording into the high-quality directory", async () => { + const raw = path.join(dir, "raw.webm"); + await fs.writeFile(raw, "video"); + const fake = makeBrowser({ videoPath: raw }); + + const exit = await Effect.runPromiseExit(run(fake)); + const master = path.join( + dir, + "videos", + "high-quality", + `desktop_1280x720_2026-01-01T00-00-00-000Z.webm`, + ); + + expect(Exit.isSuccess(exit)).toBe(true); + expect(await fs.readFile(master, "utf8")).toBe("video"); + // The rename can only succeed once the context has been closed and the + // recording flushed, so release must run before it. + expect(fake.closed).toHaveLength(1); + }); + + it("fails, and still closes the context, when no video was recorded", async () => { + const fake = makeBrowser({ videoPath: null }); + + const exit = await Effect.runPromiseExit(run(fake)); + + expect(Exit.isFailure(exit)).toBe(true); + expect(fake.closed).toEqual(fake.created); + }); + }); +}); diff --git a/src/video.ts b/src/video.ts index 4bd8fc5..57c51e7 100644 --- a/src/video.ts +++ b/src/video.ts @@ -18,11 +18,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import { Effect } from "effect"; -import type { Browser, Page } from "playwright"; +import type { Browser, BrowserContext, Page } from "playwright"; import { CaptureError, FileSystemError } from "./errors.js"; import { VideoQualityPaths, type ViewportConfig } from "./schemas.js"; import { captureRetryPolicy, + closeQuietly, navigationRetryPolicy, transcodeVideo, VIDEO_QUALITY_PROFILES, @@ -37,6 +38,18 @@ export interface CaptureVideoConfig { }; /** Must match the screenshot context, or a run's stills and video disagree. */ readonly colorScheme: "light" | "dark" | "no-preference"; + /** + * Where the capture began — the URL the recording context navigates to + * before {@link prepare} replays the script. + * + * Deliberately supplied by the caller rather than read off the page being + * captured: by the time video runs, a scripted state has already driven + * that page, so its current URL is where the script *ended*. Replaying the + * script from there records the wrong thing — a step that navigates would + * run from the wrong entry point, and a first step that assumes the entry + * view fails outright. + */ + readonly startUrl: string; /** * Replays a scripted state inside the recording context. * @@ -50,13 +63,13 @@ export interface CaptureVideoConfig { export const captureVideoForViewport = ( browser: Browser, - referencePage: Page, viewport: ViewportConfig, routeDir: string, timestamp: string, cfg: CaptureVideoConfig, ): Effect.Effect => Effect.gen(function* () { + const url = cfg.startUrl; const baseFilename = `${viewport.name}_${viewport.width}x${viewport.height}_${timestamp}`; const masterProfile = VIDEO_QUALITY_PROFILES[0]; const masterPath = path.join( @@ -66,7 +79,7 @@ export const captureVideoForViewport = ( `${baseFilename}.webm`, ); - const context = yield* Effect.tryPromise({ + const acquireContext = Effect.tryPromise({ try: () => browser.newContext({ recordVideo: { @@ -81,110 +94,114 @@ export const captureVideoForViewport = ( }), catch: (error) => new CaptureError({ - url: referencePage.url(), + url, message: "Failed to create master video context", cause: error, }), }).pipe(Effect.retry(captureRetryPolicy)); - const videoPage = yield* Effect.tryPromise({ - try: () => context.newPage(), - catch: (error) => - new CaptureError({ - url: referencePage.url(), - message: "Failed to create video page", - cause: error, - }), - }).pipe(Effect.retry(captureRetryPolicy)); + const record = (context: BrowserContext) => + Effect.gen(function* () { + const videoPage = yield* Effect.tryPromise({ + try: () => context.newPage(), + catch: (error) => + new CaptureError({ + url, + message: "Failed to create video page", + cause: error, + }), + }).pipe(Effect.retry(captureRetryPolicy)); - yield* Effect.tryPromise({ - try: () => - videoPage.goto(referencePage.url(), { - waitUntil: "networkidle", - timeout: 30000, - }), - catch: (error) => - new CaptureError({ - url: referencePage.url(), - message: "Failed to navigate video page", - cause: error, - }), - }).pipe(Effect.retry(navigationRetryPolicy)); + yield* Effect.tryPromise({ + try: () => + videoPage.goto(url, { + waitUntil: "networkidle", + timeout: 30000, + }), + catch: (error) => + new CaptureError({ + url, + message: "Failed to navigate video page", + cause: error, + }), + }).pipe(Effect.retry(navigationRetryPolicy)); - yield* Effect.sleep(cfg.waitTime); + yield* Effect.sleep(cfg.waitTime); - if (cfg.prepare) { - yield* cfg.prepare(videoPage); - } + if (cfg.prepare) { + yield* cfg.prepare(videoPage); + } - if (cfg.videoOptions.interactions) { - const scrollSteps = 5; - const scrollDelay = cfg.videoOptions.duration / (scrollSteps + 1); + if (cfg.videoOptions.interactions) { + const scrollSteps = 5; + const scrollDelay = cfg.videoOptions.duration / (scrollSteps + 1); - for (let i = 0; i < scrollSteps; i++) { - yield* Effect.tryPromise({ - try: () => - videoPage.evaluate((step: number) => { - window.scrollTo({ - top: (document.body.scrollHeight / 5) * step, - behavior: "smooth", - }); - }, i + 1), + for (let i = 0; i < scrollSteps; i++) { + yield* Effect.tryPromise({ + try: () => + videoPage.evaluate((step: number) => { + window.scrollTo({ + top: (document.body.scrollHeight / 5) * step, + behavior: "smooth", + }); + }, i + 1), + catch: (error) => + new CaptureError({ + url, + message: "Failed to run scroll interaction", + cause: error, + }), + }).pipe(Effect.catchAll(() => Effect.void)); + yield* Effect.sleep(scrollDelay); + } + + yield* Effect.tryPromise({ + try: () => + videoPage.evaluate(() => { + window.scrollTo({ top: 0, behavior: "smooth" }); + }), + catch: (error) => + new CaptureError({ + url, + message: "Failed to reset scroll position", + cause: error, + }), + }).pipe(Effect.catchAll(() => Effect.void)); + yield* Effect.sleep(1000); + } else { + yield* Effect.sleep(cfg.videoOptions.duration); + } + + yield* closeQuietly(() => videoPage.close()); + + return yield* Effect.tryPromise({ + try: async () => await videoPage.video()?.path(), catch: (error) => new CaptureError({ - url: referencePage.url(), - message: "Failed to run scroll interaction", + url, + message: "Failed to finalize video recording", cause: error, }), - }).pipe(Effect.catchAll(() => Effect.void)); - yield* Effect.sleep(scrollDelay); - } - - yield* Effect.tryPromise({ - try: () => - videoPage.evaluate(() => { - window.scrollTo({ top: 0, behavior: "smooth" }); - }), - catch: (error) => - new CaptureError({ - url: referencePage.url(), - message: "Failed to reset scroll position", - cause: error, - }), - }).pipe(Effect.catchAll(() => Effect.void)); - yield* Effect.sleep(1000); - } else { - yield* Effect.sleep(cfg.videoOptions.duration); - } + }); + }); - yield* Effect.tryPromise({ - try: () => videoPage.close(), - catch: (error) => - new CaptureError({ - url: referencePage.url(), - message: "Failed to close video page", - cause: error, - }), - }).pipe(Effect.catchAll(() => Effect.void)); - - const rawVideoPath = yield* Effect.tryPromise({ - try: async () => { - const vp = await videoPage.video()?.path(); - await context.close(); - return vp; - }, - catch: (error) => - new CaptureError({ - url: referencePage.url(), - message: "Failed to finalize video recording", - cause: error, - }), - }); + // The rename has to happen after the context is closed, because that is + // when Playwright flushes the recording to disk — so the whole recording + // lives inside acquire/use/release and only the path escapes it. + // Closing the context is what finalizes the recording and reaps its + // ffmpeg pipe, so it has to run on *every* exit — a failed navigation, a + // replay that throws, an interrupted run. Leaking it leaks a live browser + // context, and with it a recording that is never flushed, per capture. + const rawVideoPath = yield* Effect.acquireUseRelease( + acquireContext, + record, + (context: BrowserContext) => closeQuietly(() => context.close()), + ); if (!rawVideoPath) { return yield* Effect.fail( new CaptureError({ - url: referencePage.url(), + url, message: "Video path is null", cause: null, }), From f6d02569f13dc113fc287bcc95f123692bb45c6a Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 00:42:53 -0400 Subject: [PATCH 4/9] test(states): prove the scripted-state fixes against real Chromium MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claims that only a browser can settle, added to the opt-in RUN_INTEGRATION suite so each fix in the preceding commit is held up by something other than a unit test agreeing with itself. A `--state-filter` run captures only the named state and replays its ancestor to reach it rather than capturing it — asserted on the seed endpoint being reached once and not twice, and on the ancestor having no output directory. The ancestor's `allowVideoReplay` survives that flattening, which is the regression that made the same file record video unfiltered and drop it when filtered. A budget that expires inside a step names that step and sets `failedStepIndex` to it, the counterpart to the existing proof that a budget lost during navigation names no step at all. No browser context is alive when the run closes the browser, measured after a state whose script failed and a state interrupted mid-navigation by a 1 ms budget — asserted on the context count at teardown rather than on the absence of an error, which a leak would not have raised anyway. And a state captured under the shipped defaults with video — no overridden timeout, viewport list, wait time or video options — completes in about a minute instead of timing out. That configuration is the one the budget fix exists for, so it is the one worth running. --- src/integration.test.ts | 438 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 437 insertions(+), 1 deletion(-) diff --git a/src/integration.test.ts b/src/integration.test.ts index ea8327d..ca46f19 100644 --- a/src/integration.test.ts +++ b/src/integration.test.ts @@ -16,9 +16,10 @@ import type { AddressInfo } from "node:net"; import os from "node:os"; import path from "node:path"; import { Effect } from "effect"; +import { chromium } from "playwright"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { CaptureConfigLive, UICaptureService } from "./service.js"; -import { parseStatesFile } from "./states.js"; +import { filterStates, parseStatesFile } from "./states.js"; // Real browser+ffmpeg integration. Off by default; flip RUN_INTEGRATION=1 to opt in. const RUN = process.env.RUN_INTEGRATION === "1"; @@ -117,6 +118,21 @@ describe.skipIf(!RUN)("integration: scripted states", () => { let baseUrl: string; let tmpRoot: string; let seedCount = 0; + // How many times the seed endpoint was actually reached. `seedCount` is the + // number of rows it makes the page render; this is the number of POSTs, and + // it is the only way to tell "the ancestor was replayed once to reach the + // child" from "the ancestor was captured as a state of its own too". + let seedHits = 0; + // `/once` answers the first request and 500s afterwards. It is the only + // asymmetry available between a capture and its video replay, which by + // design run the same script against the same URL. + let onceCount = 0; + // A second origin on the same host, and the count of requests that reached + // it. The runtime `request` gate is the only thing standing between a + // script that navigates here and a POST the states file never declared. + let otherServer: http.Server; + let otherBaseUrl: string; + let otherHits = 0; const PAGE_HTML = ` fixture console @@ -203,12 +219,31 @@ describe.skipIf(!RUN)("integration: scripted states", () => { beforeAll(async () => { server = http.createServer((req, res) => { const url = new URL(req.url ?? "/", "http://127.0.0.1"); + if (url.pathname === "/once") { + onceCount += 1; + if (onceCount > 1) { + res.writeHead(500, { "content-type": "text/html; charset=utf-8" }); + res.end("

gone

"); + return; + } + res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + res.end(PAGE_HTML.replace("{ROWS}", "")); + return; + } + if (url.pathname === "/hop") { + res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + res.end( + `go`, + ); + return; + } if (url.pathname === "/api/seed") { if (req.method !== "POST") { res.writeHead(405).end(); return; } seedCount = 3; + seedHits += 1; res.writeHead(201, { "content-type": "application/json" }); res.end('{"ok":true}'); return; @@ -220,6 +255,24 @@ describe.skipIf(!RUN)("integration: scripted states", () => { res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); res.end(PAGE_HTML.replace("{ROWS}", rows)); }); + otherServer = http.createServer((req, res) => { + const url = new URL(req.url ?? "/", "http://127.0.0.1"); + if (url.pathname === "/api/pwn") { + otherHits += 1; + res.writeHead(200, { "content-type": "application/json" }); + res.end('{"ok":true}'); + return; + } + res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + res.end( + "

elsewhere

", + ); + }); + await new Promise((resolve) => + otherServer.listen(0, "127.0.0.1", () => resolve()), + ); + otherBaseUrl = `http://127.0.0.1:${(otherServer.address() as AddressInfo).port}/`; + await new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve()), ); @@ -229,6 +282,7 @@ describe.skipIf(!RUN)("integration: scripted states", () => { afterAll(async () => { await new Promise((resolve) => server.close(() => resolve())); + await new Promise((resolve) => otherServer.close(() => resolve())); await fs.rm(tmpRoot, { recursive: true, force: true }).catch(() => {}); }); @@ -461,6 +515,70 @@ describe.skipIf(!RUN)("integration: scripted states", () => { await expect(fs.readdir(dir)).resolves.toEqual(["ui-capture.states.json"]); }, 60_000); + it("re-judges a request against the origin the script actually navigated to", async () => { + // The bypass: the pre-launch pass validates `/api/pwn` against the + // state's configured URL, which is on the seed origin and passes. The + // script then navigates to a different origin, where the same relative + // path resolves somewhere the file never declared. The gate that + // decides therefore has to run at the moment the step resolves. + const dir = await outDir("gate-runtime"); + otherHits = 0; + const states = await writeStatesFile(dir, [ + { + name: "hop", + url: "/hop", + steps: [ + { kind: "click", selector: "#hop", settleMs: 300 }, + { kind: "waitFor", selector: "h1" }, + { kind: "request", method: "POST", path: "/api/pwn" }, + ], + }, + ]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + allowStateRequests: true, + states, + }); + + const report = await readReport(dir); + expect(report.results[0].stateStatus).toBe("failed"); + expect(report.results[0].error).toContain("outside the allowed origins"); + // The point of the whole exercise: the other origin was never touched. + expect(otherHits).toBe(0); + }, 120_000); + + it("fails a state whose precondition cannot be evaluated, rather than skipping it", async () => { + // A typo'd selector answering "not present here" is the worst outcome + // available: a green run that captured nothing, and nothing in the + // report to notice. + const dir = await outDir("precondition-broken"); + const states = await writeStatesFile(dir, [ + { + name: "typo", + precondition: "##panel", + preconditionTimeoutMs: 1500, + steps: [{ kind: "click", selector: "#reveal" }], + }, + ]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states, + }); + + const report = await readReport(dir); + expect(report.skippedStates).toBe(0); + expect(report.failedCaptures).toBe(1); + const typo = report.results.find( + (r: { state?: string }) => r.state === "typo", + ); + expect(typo.stateStatus).toBe("failed"); + expect(typo.error).toContain("precondition"); + }, 120_000); + it("records an unmet precondition as skipped, not failed", async () => { const dir = await outDir("precondition"); const states = await writeStatesFile(dir, [ @@ -482,6 +600,9 @@ describe.skipIf(!RUN)("integration: scripted states", () => { await capture(baseUrl, { ...baseConfig(dir), captureRoutes: false, + // The default probe budget is 10s; an absent precondition pays it in + // full, and this test has two states to get through. + preconditionTimeout: 1500, states, }); @@ -498,6 +619,145 @@ describe.skipIf(!RUN)("integration: scripted states", () => { expect(md).toContain("– skipped"); }, 120_000); + it("spends the state budget on reaching the state, not on capturing it", async () => { + // The budget used to wrap the capture too, which made it unsatisfiable: + // with --video on, no default could cover navigation + script + a + // screenshot per viewport + a recording per viewport, so every state + // timed out. Here the whole run deliberately outlives the budget. + const dir = await outDir("budget-scope"); + const states = await writeStatesFile(dir, [ + { + name: "revealed", + steps: [ + { kind: "waitFor", selector: "[data-app-ready]" }, + { kind: "click", selector: "#reveal", settleMs: 100 }, + { kind: "waitFor", selector: "#panel-inner", state: "visible" }, + ], + }, + ]); + + const budgetMs = 2500; + const startedAt = Date.now(); + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + captureVideo: true, + videoOptions: { duration: 4000, interactions: false }, + stateTimeout: budgetMs, + states, + }); + const elapsed = Date.now() - startedAt; + + const report = await readReport(dir); + expect(report.results[0].stateStatus).toBe("captured"); + expect(report.failedCaptures).toBe(0); + expect(report.results[0].hasVideo).toBe(true); + // The proof that the budget no longer covers capture: the state was + // captured even though the work took longer than the budget allows. + expect(elapsed).toBeGreaterThan(budgetMs); + expect( + (await fs.stat(shot(dir, "root", "states", "revealed"))).size, + ).toBeGreaterThan(0); + }, 180_000); + + it("keeps the screenshots when the video replay fails", async () => { + // The stills are on disk before recording starts. Losing the video must + // not discard them or turn a capture that produced files into a failure + // that reports none. + const dir = await outDir("video-partial"); + onceCount = 0; + const states = await writeStatesFile(dir, [ + { + name: "replay-loses", + url: "/once", + steps: [ + { kind: "waitFor", selector: "[data-app-ready]", timeoutMs: 2000 }, + { kind: "click", selector: "#reveal", settleMs: 100 }, + ], + }, + ]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + captureVideo: true, + videoOptions: { duration: 1000, interactions: false }, + states, + }); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.stateStatus).toBe("captured"); + expect(result.screenshots).toEqual(["desktop"]); + expect(result.hasVideo).toBe(false); + expect(result.videoErrors).toHaveLength(1); + expect(result.videoErrors[0]).toContain("desktop"); + expect(report.failedCaptures).toBe(0); + expect(report.successfulCaptures).toBe(1); + expect( + (await fs.stat(shot(dir, "once", "states", "replay-loses"))).size, + ).toBeGreaterThan(0); + const md = await fs.readFile(path.join(dir, "REPORT.md"), "utf8"); + expect(md).toContain("**Video capture failed:**"); + }, 180_000); + + it("names what it was doing when the budget expired, not the last step it started", async () => { + const dir = await outDir("budget-phase"); + const states = await writeStatesFile(dir, [ + { + name: "no-time", + timeoutMs: 1, + steps: [{ kind: "waitFor", selector: "[data-app-ready]" }], + }, + ]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states, + }); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.stateStatus).toBe("failed"); + expect(result.error).toContain("timed out after 1ms"); + expect(result.error).toContain("while loading"); + // A step index would point the reader at a step that never ran. + expect(result.failedStepIndex).toBe(-1); + }, 60_000); + + it("names the step that was actually running when the budget expired", async () => { + // The other half of the same claim: when a step *is* what is running, + // the message and `failedStepIndex` name that step — not the state, and + // not a step that already succeeded. + const dir = await outDir("budget-in-step"); + const states = await writeStatesFile(dir, [ + { + name: "hangs-on-step-1", + // Comfortably past a local navigation, and well short of the step's + // own 60 s timeout, so the *state* budget is what expires. + timeoutMs: 2500, + steps: [ + { kind: "waitFor", selector: "[data-app-ready]" }, + { kind: "waitFor", selector: "#never-appears", timeoutMs: 60_000 }, + ], + }, + ]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states, + }); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.stateStatus).toBe("failed"); + expect(result.error).toContain("timed out after 2500ms"); + expect(result.error).toContain('on step 1 (waitFor "#never-appears")'); + expect(result.failedStepIndex).toBe(1); + }, 120_000); + it("restricts a state to the viewports it is valid at", async () => { const dir = await outDir("viewports"); const states = await writeStatesFile(dir, [ @@ -530,6 +790,182 @@ describe.skipIf(!RUN)("integration: scripted states", () => { expect(files).toEqual(["desktop_1280x720_latest.png", "history"]); }, 120_000); + /** The two states a `--state-filter` run has to tell apart. */ + const seedChain = [ + { + name: "seeded", + // The opt-out lives on the ancestor, and the `request` step it opts + // out of is inherited with it — the pair a filtered run must not split. + allowVideoReplay: true, + steps: [ + { kind: "waitFor", selector: "[data-app-ready]" }, + { + kind: "request", + method: "POST", + path: "/api/seed", + expectStatus: 201, + }, + { kind: "reload", waitUntil: "networkidle" }, + ], + }, + { + name: "seeded-rows", + extends: "seeded", + steps: [{ kind: "waitFor", selector: ".seeded-row", minCount: 3 }], + }, + ] as const; + + it("captures only the named state, replaying its ancestor rather than capturing it", async () => { + const dir = await outDir("filter"); + seedCount = 0; + seedHits = 0; + const parsed = await writeStatesFile(dir, [...seedChain]); + // Exactly what the CLI does for `--state-filter seeded-rows`. + const selected = filterStates(parsed, ["seeded-rows"]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + allowStateRequests: true, + states: selected, + }); + + const report = await readReport(dir); + expect(report.results.map((r: { state: string }) => r.state)).toEqual([ + "seeded-rows", + ]); + expect(report.results[0].stateStatus).toBe("captured"); + // The ancestor is resolution input, not a capture target: its seed ran + // once, to reach the named state. Capturing it too would POST twice. + expect(seedHits).toBe(1); + await expect( + fs.stat(path.join(dir, "root", "states", "seeded")), + ).rejects.toThrow(); + expect( + (await fs.stat(shot(dir, "root", "states", "seeded-rows"))).size, + ).toBeGreaterThan(0); + }, 120_000); + + it("keeps the ancestor's allowVideoReplay when the chain is flattened by the filter", async () => { + // Flattening used to carry the ancestor's `request` step but not its + // opt-out, so the same file recorded video unfiltered and silently + // dropped it under --state-filter. + const dir = await outDir("filter-video"); + seedCount = 0; + seedHits = 0; + const parsed = await writeStatesFile(dir, [...seedChain]); + const selected = filterStates(parsed, ["seeded-rows"]); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + allowStateRequests: true, + captureVideo: true, + videoOptions: { duration: 1000, interactions: false }, + states: selected, + }); + + const report = await readReport(dir); + expect(report.results[0].stateStatus).toBe("captured"); + expect(report.results[0].hasVideo).toBe(true); + // Once for the stills, once for the replay the opt-out permits. + expect(seedHits).toBe(2); + }, 180_000); + + it("leaves no browser context alive when a state fails or is interrupted", async () => { + const dir = await outDir("contexts"); + const states = await writeStatesFile(dir, [ + { + name: "ok", + steps: [{ kind: "waitFor", selector: "[data-app-ready]" }], + }, + { + name: "script-fails", + steps: [ + { kind: "waitFor", selector: "#never-appears", timeoutMs: 500 }, + ], + }, + { + // A budget this small interrupts the fiber mid-navigation, which is + // the interruption path — not merely an error return. + name: "budget-interrupted", + timeoutMs: 1, + steps: [{ kind: "waitFor", selector: "[data-app-ready]" }], + }, + ]); + + // Counted at the last moment it can still be non-zero: after the run has + // finished with the browser, before the browser is torn down. + const liveAtClose: number[] = []; + const realLaunch = chromium.launch; + chromium.launch = async (options) => { + const browser = await realLaunch.call(chromium, options); + const realClose = browser.close.bind(browser); + browser.close = async (closeOptions?: { reason?: string }) => { + liveAtClose.push(browser.contexts().length); + await realClose(closeOptions); + }; + return browser; + }; + + try { + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states, + }); + } finally { + chromium.launch = realLaunch; + } + + const report = await readReport(dir); + expect( + report.results.map((r: { stateStatus: string }) => r.stateStatus).sort(), + ).toEqual(["captured", "failed", "failed"]); + // The assertion that matters is this one, not the absence of an error: + // every context the run opened — worker, state and video — was closed + // before the browser was. + expect(liveAtClose).toEqual([0]); + }, 120_000); + + it("captures a state under the shipped defaults with video, without timing out", async () => { + // No stateTimeout, no viewport list, no waitTime, no videoOptions: the + // configuration a user gets from `--video` alone, which is exactly the + // one that used to make every state time out. + const dir = await outDir("defaults-video"); + const states = await writeStatesFile(dir, [ + { + name: "revealed", + steps: [ + { kind: "waitFor", selector: "[data-app-ready]" }, + { kind: "click", selector: "#reveal", settleMs: 100 }, + { kind: "waitFor", selector: "#panel-inner", state: "visible" }, + ], + }, + ]); + + await capture(baseUrl, { + outputDir: dir, + captureVideo: true, + captureRoutes: false, + states, + }); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.stateStatus).toBe("captured"); + expect(result.error).toBeUndefined(); + expect(result.videoErrors).toBeUndefined(); + // All three default viewports, each with its own recording. + expect([...result.screenshots].sort()).toEqual([ + "desktop", + "mobile", + "tablet", + ]); + expect(result.hasVideo).toBe(true); + expect(report.failedCaptures).toBe(0); + }, 300_000); + it("aborts on an authoring error before the browser launches", async () => { const dir = await outDir("authoring"); const states = await writeStatesFile(dir, [ From 8d6921b8c413035c8dfddc1d5696849ca276115f Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 00:43:02 -0400 Subject: [PATCH 5/9] fix(lint): make knip able to report an export with no consumer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm run lint:knip` could not fail. tsdown's config declares `src/**/*.ts` as its entry list, knip's tsdown plugin adopts that verbatim, and knip does not report the exports of an entry file — so every module in the package counted as an entry and no unused export was reportable anywhere in src. A canary export appended to shared.ts went unreported, which is what confirmed it rather than assumed it. The plugin is disabled and the real entry points named instead: the package main, the bin, and the test files. Entry exports stay exempt, so the public API re-exported through index.ts is not flagged, while an export from an internal module that nothing imports is. Switched on, it reported four. Two belonged to the scripted-states work and are un-exported in the commit that owns that code. The other two are in src/utils/args.ts and predate this branch: `displayHeader` is called from nowhere and `SEPARATOR` exists only to serve it. They are deleted rather than ignored — a gate is worth having only if what it finds gets acted on. --- knip.json | 7 +++++-- src/utils/args.ts | 23 ----------------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/knip.json b/knip.json index ce81771..4e776a8 100644 --- a/knip.json +++ b/knip.json @@ -1,8 +1,11 @@ { "$schema": "https://unpkg.com/knip@5/schema.json", + "tsdown": false, + "entry": ["src/index.ts", "src/cli.ts", "src/**/*.test.ts"], + "project": ["src/**/*.ts"], + "includeEntryExports": false, "ignoreExportsUsedInFile": { "interface": true, "type": true - }, - "project": ["src/**/*.ts"] + } } diff --git a/src/utils/args.ts b/src/utils/args.ts index 0490cb2..27574d8 100644 --- a/src/utils/args.ts +++ b/src/utils/args.ts @@ -94,26 +94,3 @@ export function parseArgs( return { positional, options }; } - -/** - * Standardized separator for CLI output - */ -export const SEPARATOR = "=".repeat(80); - -/** - * Display a stylized header - */ -export function displayHeader( - title: string, - details: Record, -): void { - console.log(); - console.log(SEPARATOR); - console.log(` 🚀 ${title}`); - console.log(SEPARATOR); - for (const [key, value] of Object.entries(details)) { - console.log(` • ${key}: ${value}`); - } - console.log(SEPARATOR); - console.log(); -} From 52a67f332b854d1887b1ab964b8996ea8429c858 Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 01:31:54 -0400 Subject: [PATCH 6/9] docs(scope): say that a crawl is bounded by hostname, not by origin `isAllowedOrigin` was documented as "the origin gate every URL check in this codebase must agree on". It is not: route crawling has never used it. `scheduleRoute` and the link filter both match on `url.hostname` alone, so a crawl seeded at `http://app.test:3000` follows a link to `https://app.test` or `http://app.test:8080`. The claim was the defect, not the behaviour. Crawling navigates and screenshots, and inside one deployment an http->https or cross-port link is an ordinary internal link; the origin gate authorizes *driving* a URL -- a scripted state's entry `url` and a `request` step's POST or DELETE -- which is a stronger act deserving a stronger check. `--allowed-hosts` and `--include-subdomains` are documented as hostname filters, and hostname is what bounds a crawl. So the claim is narrowed to the two decisions it really covers, and the asymmetry is written down where each half is read: on `isAllowedOrigin`, on the crawl filter in `link-discovery.ts`, at `scheduleRoute`, and in the README beside the two flags it qualifies. No behaviour changes. Adds the unit coverage `isAllowedOrigin` never had: that it rejects a bare scheme or port difference, treats 443/80 as equal to writing them out, and widens the host half only for `--allowed-hosts` and `--include-subdomains`. --- README.md | 4 ++ src/link-discovery.ts | 9 ++++ src/service.ts | 7 +++ src/shared.test.ts | 101 ++++++++++++++++++++++++++++++++++++++++++ src/shared.ts | 19 ++++++-- 5 files changed, 137 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 263216e..86196ed 100644 --- a/README.md +++ b/README.md @@ -371,6 +371,10 @@ All three errors are `S.TaggedError` subclasses, so they discriminate cleanly un `(¬)` means the CLI flag *negates* the default — e.g. `--no-warmup` sets `warmupScroll: false`. +`includeSubdomains` and `allowedHosts` bound the crawl by **hostname**, and hostname alone. +A crawl seeded at `http://app.test:3000` will therefore follow and capture a link to `https://app.test` or to `http://app.test:8080`, because within one deployment an http→https or cross-port link is an ordinary internal link rather than an escape. +This is deliberately wider than the gate on a scripted state's `url` and on `request` steps, which compares the full origin — scheme, host and port — because those two *drive* a URL rather than merely screenshot it. + ## Scripted states A route crawler cannot capture a single-route application. diff --git a/src/link-discovery.ts b/src/link-discovery.ts index 26c58a6..81b9f71 100644 --- a/src/link-discovery.ts +++ b/src/link-discovery.ts @@ -89,6 +89,15 @@ export interface LinkDiscoveryTools { } export const createLinkDiscoveryTools = (options: { + /** + * Whether a discovered link's **hostname** is in crawl scope. + * + * Hostname, not origin: this is the crawl filter, not the `isAllowedOrigin` + * gate that guards scripted-state URLs and `request` steps. Following a link + * only navigates and screenshots, and inside one deployment an http→https or + * cross-port link is ordinary, so the scheme and port are deliberately not + * compared here. See the note on `isAllowedOrigin` in `shared.ts`. + */ readonly hostMatchesFilters: (hostname: string) => boolean; readonly menuInteractionSelectors: ReadonlyArray; }): LinkDiscoveryTools => { diff --git a/src/service.ts b/src/service.ts index fd80f53..37ad4b0 100644 --- a/src/service.ts +++ b/src/service.ts @@ -793,6 +793,13 @@ export class UICaptureService extends Effect.Service()( return; } + // Crawl scope is the **hostname** filter, not the + // `isAllowedOrigin` gate that guards a scripted state's + // URL and a `request` step: scheduling a route only + // navigates and screenshots it, and inside one + // deployment an http→https or cross-port link is + // ordinary. `--allowed-hosts` / `--include-subdomains` + // are what bound a crawl, and they are hostname filters. if ( !hostFilters.hostMatchesFilters( hostname, diff --git a/src/shared.test.ts b/src/shared.test.ts index c78e2d5..a168530 100644 --- a/src/shared.test.ts +++ b/src/shared.test.ts @@ -18,6 +18,7 @@ import { createHostFilterState, getCaptureDir, getRouteName, + isAllowedOrigin, normalizeUrl, stateResultKey, } from "./shared.js"; @@ -95,6 +96,106 @@ describe("createHostFilterState", () => { }); }); +describe("isAllowedOrigin", () => { + /** The gate as the service wires it: a real host filter, a real seed. */ + const gate = ( + candidate: string, + seed: string, + options?: { + readonly primaryHost?: string; + readonly allowedHosts?: readonly string[]; + readonly includeSubdomains?: boolean; + }, + ) => { + const seedUrl = new URL(seed); + const filters = createHostFilterState(); + filters.hydrate( + options?.primaryHost ?? seedUrl.hostname, + options?.allowedHosts ?? [], + ); + return isAllowedOrigin(new URL(candidate), seedUrl, (hostname) => + filters.hostMatchesFilters(hostname, options?.includeSubdomains ?? false), + ); + }; + + it("allows the seed origin itself", () => { + expect(gate("https://app.test/dash", "https://app.test/")).toBe(true); + }); + + it("rejects a different scheme on the same host and port", () => { + // A downgrade is a different server, and for a `request` step it is a + // POST sent in the clear at one. + expect(gate("http://app.test/", "https://app.test/")).toBe(false); + expect(gate("https://app.test/", "http://app.test/")).toBe(false); + }); + + it("rejects a different port on the same host and scheme", () => { + expect(gate("http://app.test:4000/", "http://app.test:3000/")).toBe(false); + expect( + gate("http://127.0.0.1:42269/secret", "http://127.0.0.1:45179/"), + ).toBe(false); + }); + + it("treats a scheme's default port as equal to writing it out", () => { + // `URL.port` normalizes to "" for 443/80, so neither direction needs + // special casing. + expect(gate("https://app.test:443/", "https://app.test/")).toBe(true); + expect(gate("https://app.test/", "https://app.test:443/")).toBe(true); + expect(gate("http://app.test:80/", "http://app.test/")).toBe(true); + }); + + it("rejects a host the filter does not allow, however matching the rest", () => { + expect(gate("https://evil.test/", "https://app.test/")).toBe(false); + }); + + it("defers the host decision to the filter, www and all", () => { + // `canonicalizeHost` strips `www.`, so the filter answers for both. + expect(gate("https://www.app.test/", "https://app.test/")).toBe(true); + }); + + it("honours --allowed-hosts, still at the seed's scheme and port", () => { + const allowedHosts = ["cdn.partner.io"]; + expect( + gate("https://cdn.partner.io/x", "https://app.test/", { allowedHosts }), + ).toBe(true); + // The extra host is allowed; a different port on it is still not. + expect( + gate("https://cdn.partner.io:8443/x", "https://app.test/", { + allowedHosts, + }), + ).toBe(false); + expect( + gate("http://cdn.partner.io/x", "https://app.test/", { allowedHosts }), + ).toBe(false); + }); + + it("follows --include-subdomains for the host half only", () => { + expect(gate("https://api.app.test/", "https://app.test/")).toBe(false); + expect( + gate("https://api.app.test/", "https://app.test/", { + includeSubdomains: true, + }), + ).toBe(true); + // Subdomains widen the host, never the scheme or the port. + expect( + gate("https://api.app.test:8443/", "https://app.test/", { + includeSubdomains: true, + }), + ).toBe(false); + expect( + gate("http://api.app.test/", "https://app.test/", { + includeSubdomains: true, + }), + ).toBe(false); + }); + + it("rejects a non-http scheme that shares the seed's host", () => { + // `new URL("file:///etc/passwd").hostname` is "", but a data: or ws: URL + // can carry the seed's host and would otherwise differ only by scheme. + expect(gate("ws://app.test/", "http://app.test/")).toBe(false); + }); +}); + describe("normalizeUrl", () => { it("returns origin+pathname with the trailing slash stripped", () => { // Even the bare-origin slash is stripped — the result is the origin alone. diff --git a/src/shared.ts b/src/shared.ts index 256bb23..bde1796 100644 --- a/src/shared.ts +++ b/src/shared.ts @@ -227,7 +227,8 @@ export const createHostFilterState = (): HostFilterState => { }; /** - * The origin gate every URL check in this codebase must agree on. + * The origin gate for the two decisions that let a run *act* on a URL: a + * scripted state's entry `url`, and the URL a `request` step resolves to. * * An origin is **scheme + host + port**, so that is what gets compared: * `hostMatchesFilters` decides the host (it canonicalizes `www.` and honors @@ -242,8 +243,20 @@ export const createHostFilterState = (): HostFilterState => { * casing. * * Both the pre-launch validation in `states.ts` and the runtime request gate - * call this, so a states file that validates cannot be widened at runtime and - * a run cannot abort on something the runtime would have allowed. + * in the state driver call this, so a states file that validates cannot be + * widened at runtime and a run cannot abort on something the runtime would + * have allowed. + * + * **Route crawling deliberately does not use this gate.** `scheduleRoute` in + * `service.ts` and the link filter in `link-discovery.ts` match on the + * hostname alone, so a crawl seeded at `http://app.test:3000` will follow and + * capture a link to `https://app.test` or `http://app.test:8080`. The two + * answer different questions: crawling navigates and screenshots, and within + * one deployment an http→https or cross-port link is ordinary rather than + * suspicious, while this gate authorizes driving a scripted state at a URL and + * sending a `request` step's POST or DELETE at it. `--allowed-hosts` and + * `--include-subdomains` are documented as hostname filters, and hostname is + * what bounds a crawl. */ export const isAllowedOrigin = ( candidate: URL, From 693380e691638d0871511deb5a2911346d01aeba Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 01:33:06 -0400 Subject: [PATCH 7/9] test(states): pin the budget's phase model with the pair that discriminates The test for "which step was running when the budget expired" asserted only the case where naming a step is right: a `waitFor` that hangs. The model it replaced -- an index set when a step *started*, carrying no phase -- satisfies that assertion exactly as well as the fix does, so the test could not fail if the fix were reverted. The case that separates them is a step that already succeeded and whose settle pause then overruns: the old model names it "on step 1 (click "#reveal")", sending the reader to a click that worked. So the state `settles-past-budget` is added alongside the hanging one and asserted as a pair -- the message must say `while settling after step 1`, and must not say `on step 1` -- while `failedStepIndex` stays 1, because the pause belongs to that step's definition. Also narrows `timeoutMs`'s doc. It called itself a "whole-state budget: navigation + script + capture", which capture is not inside: screenshot and video run after it, under their own timeouts. A budget that really covered capture could not be satisfied by any value once `--video` was on. The behaviour is right and already covered; the comment was wrong. --- src/integration.test.ts | 51 +++++++++++++++++++++++++++++++++-------- src/schemas.ts | 10 +++++++- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/integration.test.ts b/src/integration.test.ts index ca46f19..a4abb8f 100644 --- a/src/integration.test.ts +++ b/src/integration.test.ts @@ -726,10 +726,17 @@ describe.skipIf(!RUN)("integration: scripted states", () => { expect(result.failedStepIndex).toBe(-1); }, 60_000); - it("names the step that was actually running when the budget expired", async () => { - // The other half of the same claim: when a step *is* what is running, - // the message and `failedStepIndex` name that step — not the state, and - // not a step that already succeeded. + it("tells a step that is still running from one that already finished", async () => { + // The half of the phase claim that discriminates. The old model carried + // an index and nothing else, set when a step *started*, so it named that + // step for anything that hung afterwards — including the settle pause a + // step asks for once it has succeeded. That is the reported-against-work- + // that-already-worked case, and it is worded identically to a step that + // really is stuck unless the phase is recorded. + // + // So both are run here: `hangs-on-step-1`, where naming the step is + // right, and `settles-past-budget`, where it is wrong. Only the pair + // pins the fix; either alone is satisfied by the model that was replaced. const dir = await outDir("budget-in-step"); const states = await writeStatesFile(dir, [ { @@ -742,6 +749,16 @@ describe.skipIf(!RUN)("integration: scripted states", () => { { kind: "waitFor", selector: "#never-appears", timeoutMs: 60_000 }, ], }, + { + // Every step succeeds. The budget is spent in the settle pause the + // click asked for, long after the click itself was done. + name: "settles-past-budget", + timeoutMs: 2500, + steps: [ + { kind: "waitFor", selector: "[data-app-ready]" }, + { kind: "click", selector: "#reveal", settleMs: 30_000 }, + ], + }, ]); await capture(baseUrl, { @@ -751,11 +768,27 @@ describe.skipIf(!RUN)("integration: scripted states", () => { }); const report = await readReport(dir); - const result = report.results[0]; - expect(result.stateStatus).toBe("failed"); - expect(result.error).toContain("timed out after 2500ms"); - expect(result.error).toContain('on step 1 (waitFor "#never-appears")'); - expect(result.failedStepIndex).toBe(1); + const resultFor = (name: string) => + report.results.find((r: { state?: string }) => r.state === name); + + const stuck = resultFor("hangs-on-step-1"); + expect(stuck.stateStatus).toBe("failed"); + expect(stuck.error).toContain("timed out after 2500ms"); + expect(stuck.error).toContain('on step 1 (waitFor "#never-appears")'); + expect(stuck.failedStepIndex).toBe(1); + + const settling = resultFor("settles-past-budget"); + expect(settling.stateStatus).toBe("failed"); + expect(settling.error).toContain("timed out after 2500ms"); + // Named as the step it was settling *after*. "on step 1" would send the + // reader to a click that had already worked. + expect(settling.error).toContain( + 'while settling after step 1 (click "#reveal")', + ); + expect(settling.error).not.toContain('on step 1 (click "#reveal")'); + // The index still points at the step the pause belongs to: the pause is + // part of that step's definition, so it is the right thing to name. + expect(settling.failedStepIndex).toBe(1); }, 120_000); it("restricts a state to the viewports it is valid at", async () => { diff --git a/src/schemas.ts b/src/schemas.ts index 28ec62f..24f0504 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -230,7 +230,15 @@ export class CaptureState extends S.Class("CaptureState")({ */ viewports: S.optional(S.Array(S.String).pipe(S.minItems(1))), steps: S.Array(CaptureStep), - /** Whole-state budget: navigation + script + capture. */ + /** + * Budget for *reaching* this state — navigation, the `precondition` probe + * and the script — overriding `stateTimeout` for the run. + * + * It stops there, exactly as the run-wide default does: screenshot and + * video capture run outside it, under their own timeouts. A budget that + * covered capture too could not be satisfied by any value once `--video` + * was on. + */ timeoutMs: S.optional(S.Number.pipe(S.int(), S.positive())), /** * Record video for this state even though its script contains a `request` From ea338d4e77171a60f2a7444febcf5a7ef28b403f Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 01:33:40 -0400 Subject: [PATCH 8/9] fix(states): fail a state that captured nothing instead of reporting it captured Status was decided from reaching the end of the viewport loop, not from anything coming out of it. A state whose loop ran zero times therefore finished as `stateStatus: "captured"` with an empty `screenshots` map: a green result, an empty directory on disk, and a run whose success count included a state that produced no image. The schema's `minItems(1)` on a state's `viewports` is what keeps that unreachable today, and it stays the first line of defense. But the status is decided in the service, so that is where "the loop ended" must be distinguished from "something was captured" -- whatever else could put a state in front of the loop with nothing to iterate: a relaxed schema, a filter that intersects to nothing, a future capture axis. The guard fails through `CaptureError`, the channel the caller already maps to `stateStatus: "failed"`. Tested by constructing the state the schema refuses, via `disableValidation`, after first asserting that the schema does refuse it -- so the test covers the service guard without pretending the schema guard is gone. The expected message is imported rather than copied, so the assertion cannot drift from what the service reports. --- src/integration.test.ts | 50 ++++++++++++++++++++++++++++++++++++++++- src/service.ts | 30 +++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/integration.test.ts b/src/integration.test.ts index a4abb8f..f2fd78a 100644 --- a/src/integration.test.ts +++ b/src/integration.test.ts @@ -18,7 +18,12 @@ import path from "node:path"; import { Effect } from "effect"; import { chromium } from "playwright"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { CaptureConfigLive, UICaptureService } from "./service.js"; +import { CaptureState } from "./schemas.js"; +import { + CaptureConfigLive, + EMPTY_STATE_CAPTURE_MESSAGE, + UICaptureService, +} from "./service.js"; import { filterStates, parseStatesFile } from "./states.js"; // Real browser+ffmpeg integration. Off by default; flip RUN_INTEGRATION=1 to opt in. @@ -823,6 +828,49 @@ describe.skipIf(!RUN)("integration: scripted states", () => { expect(files).toEqual(["desktop_1280x720_latest.png", "history"]); }, 120_000); + it("fails a state that captured nothing, with the schema guard bypassed", async () => { + const dir = await outDir("empty-viewports"); + const [valid] = await writeStatesFile(dir, [ + { + name: "empty-vp", + viewports: ["desktop"], + steps: [{ kind: "waitFor", selector: "[data-app-ready]" }], + }, + ]); + if (!valid) throw new Error("fixture state did not parse"); + + // The schema's `minItems(1)` still rejects an empty viewport list, and + // stays the first line of defense. + expect(() => new CaptureState({ ...valid, viewports: [] })).toThrow(); + + // `disableValidation` is the only way to build the state the schema + // refuses, and that is the point: it stands in for whatever else could + // one day put a state in front of the viewport loop with nothing to + // iterate — a relaxed schema, a filter that intersects to nothing, a new + // capture axis. Before the service-level guard this run reported + // `stateStatus: "captured"` with an empty `screenshots` map and counted + // as a successful capture. + const state = new CaptureState( + { ...valid, viewports: [] }, + { disableValidation: true }, + ); + + await capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states: [state], + }); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.state).toBe("empty-vp"); + expect(result.stateStatus).toBe("failed"); + expect(result.error).toContain(EMPTY_STATE_CAPTURE_MESSAGE); + expect(result.screenshots).toEqual([]); + expect(report.successfulCaptures).toBe(0); + expect(report.failedCaptures).toBe(1); + }, 60_000); + /** The two states a `--state-filter` run has to tell apart. */ const seedChain = [ { diff --git a/src/service.ts b/src/service.ts index 37ad4b0..0ae45ce 100644 --- a/src/service.ts +++ b/src/service.ts @@ -94,6 +94,16 @@ const NO_VIDEO_CAPTURED: ViewportVideoOutcome = { error: Option.none(), }; +/** + * Why a state that reached the end of its viewport loop with nothing to show + * for it is a failure rather than a capture. + * + * Exported so the test that bypasses the schema's `minItems(1)` guard asserts + * on the same string the service reports, rather than on a copy of it. + */ +export const EMPTY_STATE_CAPTURE_MESSAGE = + "no screenshots were captured: the state resolved to zero viewports, and a state that captured nothing must not be reported as captured"; + /** A one-line, report-ready rendering of any failure this service can raise. */ const formatCaptureFailure = (error: unknown): string => { if (error instanceof FileSystemError) { @@ -362,6 +372,26 @@ export class UICaptureService extends Effect.Service()( Option.isSome(data.video.error) ? [data.video.error.value] : [], ); + // Defense in depth for the schema's `minItems(1)` on a state's + // `viewports`. The status is decided here, so it is here that + // "the viewport loop ended" must not be mistaken for "something + // was captured": an empty loop leaves `screenshots` empty and + // used to be reported `captured` with nothing in it — a green + // state, on disk as an empty directory. Whatever lets a state + // reach this point with no viewports — a relaxed schema, a + // filter that intersects to nothing, a future capture axis — + // this fails it instead. `CaptureError` is the channel the + // caller already maps to `stateStatus: "failed"`. + if (stateContext && Object.keys(screenshots).length === 0) { + return yield* Effect.fail( + new CaptureError({ + url, + message: EMPTY_STATE_CAPTURE_MESSAGE, + cause: null, + }), + ); + } + return new CaptureResult({ url, route, From a9f73d90e573bd5ff79787d172eb9adcfc0c3064 Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Fri, 4 Sep 2026 01:33:50 -0400 Subject: [PATCH 9/9] test(states): prove a context is released when the page inside it cannot open The existing leak test asserts that every context is closed, but every context in it gets its page, so it holds just as well for an acquire that takes context and page together as for one that takes them in sequence. It cannot reach the case that separates them. Effect registers a release only once its acquire has completed, so an acquire taking both leaks the context whenever `newPage` rejects -- a live Chromium context per bad state, on the one path where something was already going wrong. A rejecting `newPage` is the only stimulus that tells the two shapes apart, since nothing else fails between the two acquisitions. So it is injected at the Playwright boundary rather than in the service: `context.newPage()` rejects once, at a chosen call, and the count comes from `browser.contexts()` -- Playwright's own list -- sampled after the run is finished with the browser and before teardown, the only window in which a stranded context is still distinguishable from a closed one. A context counted there genuinely was not closed. Both paths are covered, since the state path strands one context per bad state and the worker path stranded one for a whole run. Each asserts that its injection actually fired, so a call ordering the test got wrong fails it rather than passing it vacuously. --- src/integration.test.ts | 129 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/src/integration.test.ts b/src/integration.test.ts index f2fd78a..c4f264b 100644 --- a/src/integration.test.ts +++ b/src/integration.test.ts @@ -1005,10 +1005,137 @@ describe.skipIf(!RUN)("integration: scripted states", () => { ).toEqual(["captured", "failed", "failed"]); // The assertion that matters is this one, not the absence of an error: // every context the run opened — worker, state and video — was closed - // before the browser was. + // before the browser was. Note what this cannot reach on its own: every + // context here got its page, so it holds equally for an acquire that + // takes context and page together. The two tests below inject the one + // stimulus that tells those apart. expect(liveAtClose).toEqual([0]); }, 120_000); + /** + * Runs `body` with one fault injected at the Playwright boundary: the + * `failAt`-th `context.newPage()` of the run rejects, once. + * + * The fault goes into Playwright, never into the service: every context is + * a real Chromium context, and `browser.contexts()` is Playwright's own + * list of the ones still open, so a context counted here is a context that + * genuinely was not closed. `contextsAtClose` is sampled at the last + * moment it can still be non-zero — after the run has finished with the + * browser, before the browser is torn down, which is the only window in + * which a stranded context is distinguishable from a closed one. + * + * A rejecting `newPage` is the one stimulus that separates "acquire the + * context, then acquire the page" from "acquire both in one step": only + * the first registers a release for the context before the page can throw. + * Nothing else fails between the two acquisitions. + */ + const withNewPageFailingAt = async ( + failAt: number, + body: () => Promise, + ) => { + const realLaunch = chromium.launch; + let pagesRequested = 0; + let injected = 0; + const contextsAtClose: number[] = []; + let failure: unknown; + + chromium.launch = async (options) => { + const browser = await realLaunch.call(chromium, options); + const realNewContext = browser.newContext.bind(browser); + browser.newContext = async (contextOptions) => { + const context = await realNewContext(contextOptions); + const realNewPage = context.newPage.bind(context); + context.newPage = async () => { + pagesRequested += 1; + if (pagesRequested === failAt) { + injected += 1; + throw new Error("injected: context.newPage() rejected"); + } + return await realNewPage(); + }; + return context; + }; + const realClose = browser.close.bind(browser); + browser.close = async (closeOptions?: { reason?: string }) => { + contextsAtClose.push(browser.contexts().length); + await realClose(closeOptions); + }; + return browser; + }; + + try { + await body(); + } catch (error) { + failure = error; + } finally { + chromium.launch = realLaunch; + } + + return { injected, contextsAtClose, failure }; + }; + + it("closes a state's context when the page inside it cannot be opened", async () => { + // The state path opens a context and then a page inside it. Effect + // registers a release only once its acquire has *completed*, so an + // acquire that took both stranded the context for the rest of the run + // whenever `newPage` rejected — a live browser context per bad state, + // on the one path where something was already going wrong. + const dir = await outDir("state-page-fails"); + const states = await writeStatesFile(dir, [ + { + name: "page-never-opens", + steps: [{ kind: "waitFor", selector: "[data-app-ready]" }], + }, + ]); + + // With one worker and no routes, page 1 is the worker's own and page 2 + // is the state's. `injected` is asserted below so that an ordering this + // test got wrong fails it rather than quietly passing it. + const { injected, contextsAtClose, failure } = await withNewPageFailingAt( + 2, + () => + capture(baseUrl, { + ...baseConfig(dir), + captureRoutes: false, + states, + }), + ); + + expect(injected).toBe(1); + expect(failure).toBeUndefined(); + + const report = await readReport(dir); + const result = report.results[0]; + expect(result.state).toBe("page-never-opens"); + expect(result.stateStatus).toBe("failed"); + expect(result.error).toContain("failed to create a page"); + // The claim: the context that could not get a page was closed anyway. + expect(contextsAtClose).toEqual([0]); + }, 120_000); + + it("closes a worker's context when the page inside it cannot be opened", async () => { + // The same split acquire in the worker pool, where the stranded context + // outlived a whole run rather than a single state. + const dir = await outDir("worker-page-fails"); + + // Page 1 is the first worker's, and there is only one worker. + const { injected, contextsAtClose, failure } = await withNewPageFailingAt( + 1, + () => + capture(baseUrl, { + ...baseConfig(dir), + maxDepth: 0, + }), + ); + + expect(injected).toBe(1); + // A worker that cannot open a page has nothing to crawl with, so the run + // fails — the point is that it does not also leak. + expect(failure).toBeDefined(); + expect(String(failure)).toContain("Failed to create page"); + expect(contextsAtClose).toEqual([0]); + }, 120_000); + it("captures a state under the shipped defaults with video, without timing out", async () => { // No stateTimeout, no viewport list, no waitTime, no videoOptions: the // configuration a user gets from `--video` alone, which is exactly the