Default to an installed build, and report missing video tooling

.github/skills/validate-ui-scenario/SKILL.md

@@ -18,21 +18,36 @@ step boundary, writes the report, and captions the recording with each step and
 ## Prepare
 
 ```bash
-npm install                        # once
+npm install                             # once
 npm --prefix test/scenario run compile  # after any change under test/scenario
 ```
 
-Add `ffmpeg` and `ffprobe` to `PATH` to get the caption band on the video. Without them the run still
-succeeds and the raw recording is kept.
-
-| Target | Extra flags | Also required | Use for |
-|--------|-------------|---------------|---------|
-| Installed Insiders | `--build <app-root>` | nothing | Reproducing a report against shipped behavior |
-| Dev build from this checkout | *(none)* | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change |
+**Check `ffmpeg` and `ffprobe` are available before running.** The runner looks on `PATH` and in the
+usual install locations, so an ffmpeg installed after the editor started is still found. Without them
+the scenario still runs and keeps the raw recording, but the video is not captioned with step titles.
+The runner warns at startup; if they are missing, tell the user how to install them rather than
+silently returning an unannotated video:
+
+| Platform | Install |
+|----------|---------|
+| Windows | `winget install Gyan.FFmpeg` |
+| macOS | `brew install ffmpeg` |
+| Linux | `sudo apt install ffmpeg` |
+
+A new terminal may be needed for `PATH` to pick them up, or set `FFMPEG_PATH` and `FFPROBE_PATH`. An
+existing run can be annotated afterwards with
+`node test/scenario/out/renderEvidenceChapters.js <run-dir>`.
+
+| Target | Flags | Also required | Use for |
+|--------|-------|---------------|---------|
+| Installed Insiders, else Stable | *(none — the default)* | nothing | Reproducing a report against shipped behavior |
+| Dev build from this checkout | `--dev` | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change |
+| A specific install | `--build <app-root>` | nothing | Pinning an exact build |
 | Web | `--web --headless` | `npm run transpile-client` | Browser-only behavior |
 
-`--build` takes the application root — the install directory on Windows and Linux, or the `.app`
-bundle on macOS:
+With no target flag the runner finds an installed VS Code Insiders (falling back to Stable) and logs
+which one it chose. `--build` takes the application root — the install directory on Windows and
+Linux, or the `.app` bundle on macOS:
 
 ```bash
 # Windows
@@ -41,9 +56,12 @@ bundle on macOS:
 --build "/Applications/Visual Studio Code - Insiders.app"
 ```
 
-An installed build runs with its own profile and extensions directory, so your extensions and
-settings never leak into the recording. Insiders only reproduces **shipped** behavior — to validate
-an unmerged change, run the dev build from a checkout that contains it.
+Every target runs with its own profile and extensions directory, so your extensions and settings
+never leak into the recording, and the window is sized to the recording canvas so the capture has no
+empty margins. The evidence records the quality of the build that actually ran (`Insiders`,
+`Stable`, `Dev`), so a report always names the product it validated. An installed build only
+reproduces **shipped** behavior — to validate an unmerged change, use `--dev` in a checkout that
+contains it.
 
 ## Write the scenario
 
@@ -117,20 +135,41 @@ module.exports = {
 | `workspacePath` | Disposable folder to open |
 | `userSettings` | Settings seeded into the profile before launch |
 | `extraArgs` | Extra VS Code command-line arguments |
+| `stepPauseMs` | How long to hold each finished step so its caption is readable. Defaults to `1000`; set `0` when the scenario is timing-sensitive |
 
-Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `skip(reason)`.
+Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `skip(reason, options)`.
 `workbench` exposes the feature helpers (`settingsEditor`, `quickaccess`, `editors`, `terminal`,
 `chat`, …); `page` is the Playwright page for anything they do not cover.
 
 - **Return a string** describing how the step was validated. It appears in the report.
 - **Throw** to fail the step. The message is recorded, and the run stops.
-- **Call `skip(reason)`** when hardware, an account, or a service is unavailable. The run stops and
-  is reported as `aborted`, never as passed.
+- **Call `skip(reason, { needs })`** when the step cannot be validated automatically. The run stops
+  and is reported as `aborted`, never as passed.
+
+## Steps that cannot be automated
+
+Decide this while planning, before writing the scenario, and classify each one — the two kinds have
+different consequences:
+
+| `needs` | Meaning | What to do |
+|---------|---------|------------|
+| `human` | A person is required: physical hardware, a subjective judgement, a sign-in that cannot be scripted | Report the step so someone can check it by hand |
+| `infrastructure` | Automatable in principle, but the harness cannot do it yet | Report it as an **enhancement to this skill**, naming the missing capability |
+
+```js
+ctx.skip('Comparing physical print output requires a person with a printer.', { needs: 'human' });
+ctx.skip('The harness cannot drive native OS file dialogs.', { needs: 'infrastructure' });
+```
+
+Blocked steps are recorded in `manifest.json`, highlighted in a **Needs attention** section of
+`report.html`, marked on the video caption (`SKIPPED - NEEDS HUMAN`), and printed at the end of the
+run. Surface them in your summary — never quietly drop a step you could not perform, and never
+weaken an assertion so that it passes.
 
 ## Run it
 
 ```bash
-node test/scenario/out/runScenario.js <scenario.cjs> --build "<app-root>"
+node test/scenario/out/runScenario.js <scenario.cjs>
 ```
 
 Exit code `0` means every step passed, `1` means the run failed or was aborted, `2` a usage error.
@@ -170,6 +209,12 @@ Summarize the outcome, list failed or skipped steps, link `report.html`, and sta
 VS Code version and quality (both are in `manifest.json`), and the source issue. Attach the video to
 the issue or pull request by dragging it into the comment box.
 
+Always call out, separately from the pass/fail result:
+
+- **steps that need a person**, so someone knows what is still unverified;
+- **steps blocked on a missing harness capability**, named as a concrete enhancement to this skill;
+- **anything that degraded the evidence**, such as a missing ffmpeg leaving the video uncaptioned.
+
 ## Related
 
 - **Interactive exploration.** `test/mcp` also serves these tools over MCP (`vscode_automation_*`),
@@ -181,16 +226,17 @@ the issue or pull request by dragging it into the comment box.
   skill when a scenario is not yet covered there, or to iterate locally before proposing one.
 
 <example>
-User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159 against my
-installed VS Code Insiders, and give me the report and the annotated video."
+User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159"
 
-1. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor
+1. Confirm `ffmpeg`/`ffprobe` are available; if not, say so and give the install command before
+   running, so the user is not surprised by a video without step titles.
+2. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor
    should match **Max Requests**, whose description mentions confirmation.
-2. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the
+3. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the
    setting being missing from the build.
-3. Write `.build/vscode-playwright-mcp/issue-250159.cjs`, run it with `--build`, and read the
-   printed report path.
-4. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`.
+4. Write `.build/vscode-playwright-mcp/issue-250159.cjs` and run it with no target flag, which uses
+   the installed Insiders; read the printed report path.
+5. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`.
 
 The run fails at the search step, and that is the answer: the issue reproduces. Report it as a
 successful reproduction, not as a broken scenario.

test/automation/src/playwrightBrowser.ts

@@ -133,7 +133,10 @@ async function launchBrowser(options: LaunchOptions, endpoint: string) {
 	// long enough to visibly skew offsets measured against it.
 	const videoStartedAt = options.videosPath ? Date.now() : undefined;
 	const page = await measureAndLog(() => context.newPage(), 'context.newPage()', logger);
-	await measureAndLog(() => page.setViewportSize({ width: 1440, height: 900 }), 'page.setViewportSize', logger);
+	// Match the recording canvas while recording, so the capture has no empty
+	// margins; keep the established size otherwise so smoke runs are unchanged.
+	const viewport = options.videosPath ? { width: 1920, height: 1080 } : { width: 1440, height: 900 };
+	await measureAndLog(() => page.setViewportSize(viewport), 'page.setViewportSize', logger);
 
 	// Always log failed requests and console errors/warnings (even without
 	// `--verbose`) so that hard-to-reproduce startup stalls can be root caused

test/automation/src/playwrightElectron.ts

@@ -37,6 +37,12 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess:
 async function launchElectron(configuration: IElectronConfiguration, options: LaunchOptions) {
 	const { logger, tracing, snapshots } = options;
 
+	// The recording canvas is fixed, but VS Code sizes its own window (1440x900
+	// with a workspace, 1200x800 empty), so the capture would otherwise show the
+	// window in the top-left corner of a larger frame. The window is resized to
+	// match below, which also renders reliably when it is larger than the screen.
+	const videoSize = { width: 1920, height: 1080 };
+
 	const playwrightImpl = options.playwright ?? playwright;
 	let electron;
 	try {
@@ -46,7 +52,7 @@ async function launchElectron(configuration: IElectronConfiguration, options: La
 			recordVideo: options.videosPath
 				? {
 					dir: options.videosPath,
-					size: { width: 1920, height: 1080 }
+					size: videoSize
 				} : undefined,
 			env: configuration.env as { [key: string]: string },
 			timeout: LAUNCH_TIMEOUT
@@ -63,6 +69,18 @@ async function launchElectron(configuration: IElectronConfiguration, options: La
 			throw enrichLaunchError(error, options);
 		}
 	}
+	if (options.videosPath) {
+		try {
+			await electron.evaluate(({ BrowserWindow }, size) => {
+				const target = BrowserWindow.getAllWindows()[0];
+				target?.setBounds({ x: 0, y: 0, width: size.width, height: size.height });
+			}, videoSize);
+		} catch (error) {
+			// A mismatched window only wastes pixels in the recording, so never fail
+			// a run because the window could not be resized.
+			logger.log(`Playwright (Electron): Failed to size the window to the recording (${error})`);
+		}
+	}
 	// Recording is per page, so sample the origin once the first window exists
 	// rather than when the application finished launching.
 	const videoStartedAt = options.videosPath ? Date.now() : undefined;

test/scenario/src/application.ts

@@ -71,12 +71,47 @@ function fail(errorMessage): void {
 let quality: Quality;
 let version: string | undefined;
 
-function parseQuality(): Quality {
-	if (process.env.VSCODE_DEV === '1') {
+/**
+ * Read the `quality` a build was stamped with.
+ *
+ * `parseQuality` reads the environment, which only describes a build made from
+ * this checkout. An installed build carries its own quality in `product.json`,
+ * and without it every installed run is labelled `Dev` in the evidence, which
+ * misreports which product was actually validated.
+ */
+function readBuildQuality(root: string): string | undefined {
+	// Windows installs nest the app under a commit-stamped directory, so the
+	// manifest is not always directly under the application root.
+	const candidates = [path.join(root, 'resources', 'app', 'product.json')];
+	try {
+		for (const entry of fs.readdirSync(root, { withFileTypes: true })) {
+			if (entry.isDirectory()) {
+				candidates.push(path.join(root, entry.name, 'resources', 'app', 'product.json'));
+			}
+		}
+	} catch {
+		// an unreadable root is reported by the electron path check below
+	}
+	candidates.push(path.join(root, 'Contents', 'Resources', 'app', 'product.json')); // macOS bundle
+	for (const candidate of candidates) {
+		try {
+			const product = JSON.parse(fs.readFileSync(candidate, 'utf8')) as { quality?: string };
+			if (product.quality) {
+				return product.quality;
+			}
+		} catch {
+			// try the next location
+		}
+	}
+	return undefined;
+}
+
+function parseQuality(stamped?: string): Quality {
+	if (!stamped && process.env.VSCODE_DEV === '1') {
 		return Quality.Dev;
 	}
 
-	const quality = process.env.VSCODE_QUALITY ?? '';
+	const quality = stamped ?? process.env.VSCODE_QUALITY ?? '';
 
 	switch (quality) {
 		case 'stable':
@@ -95,10 +130,69 @@ function parseQuality(): Quality {
 //
 // #### Electron ####
 //
+/**
+ * Locate an installed VS Code Insiders, then Stable.
+ *
+ * Reproducing a reported issue is the common case, and that means running the
+ * shipped product rather than a build from this checkout, so an installed build
+ * is used when the caller did not choose a target.
+ */
+function findInstalledBuild(): string | undefined {
+	const candidates: string[] = [];
+	switch (process.platform) {
+		case 'win32': {
+			const roots = [process.env.LOCALAPPDATA, process.env.ProgramFiles, process.env['ProgramFiles(x86)']].filter((root): root is string => !!root);
+			for (const root of roots) {
+				candidates.push(path.join(root, 'Programs', 'Microsoft VS Code Insiders'), path.join(root, 'Microsoft VS Code Insiders'));
+			}
+			for (const root of roots) {
+				candidates.push(path.join(root, 'Programs', 'Microsoft VS Code'), path.join(root, 'Microsoft VS Code'));
+			}
+			break;
+		}
+		case 'darwin':
+			candidates.push(
+				'/Applications/Visual Studio Code - Insiders.app',
+				path.join(os.homedir(), 'Applications', 'Visual Studio Code - Insiders.app'),
+				'/Applications/Visual Studio Code.app',
+				path.join(os.homedir(), 'Applications', 'Visual Studio Code.app')
+			);
+			break;
+		default:
+			candidates.push(
+				'/usr/share/code-insiders',
+				'/opt/visual-studio-code-insiders',
+				// Snap keeps the app under a read-only revision root.
+				'/snap/code-insiders/current/usr/share/code-insiders',
+				'/usr/share/code',
+				'/opt/visual-studio-code',
+				'/snap/code/current/usr/share/code'
+			);
+			break;
+	}
+	return candidates.find(candidate => {
+		try {
+			return fs.existsSync(candidate) && fs.existsSync(getBuildElectronPath(candidate));
+		} catch {
+			return false; // an incomplete install is not a usable target
+		}
+	});
+}
+
 if (!opts.web) {
 	let testCodePath = opts.build;
 	let electronPath: string | undefined;
 
+	if (!testCodePath && !opts.dev) {
+		testCodePath = findInstalledBuild();
+		if (testCodePath) {
+			// `getApplication` launches whatever `opts.build` names, so record the
+			// choice there rather than only in this block.
+			opts.build = testCodePath;
+			logger.log(`No target given, using the installed build at ${testCodePath}. Pass --dev to run this checkout instead.`);
+		}
+	}
+
 	if (testCodePath) {
 		electronPath = getBuildElectronPath(testCodePath);
 		version = getBuildVersion(testCodePath);
@@ -111,10 +205,18 @@ if (!opts.web) {
 	}
 
 	if (!fs.existsSync(electronPath || '')) {
-		fail(`Cannot find VSCode at ${electronPath}. Please run VSCode once first (scripts/code.sh, scripts\\code.bat) and try again.`);
+		fail(`Cannot find VS Code at ${electronPath}. Install VS Code Insiders, pass --build <app-root>, or build this checkout and pass --dev.`);
 	}
 
-	quality = parseQuality();
+	// Windows applies a downloaded update by swapping the executable during
+	// startup, so the launched process exits before it ever shows a window and
+	// the failure reads as a crash. Insiders updates daily, so say what is
+	// actually wrong instead of leaving a 60s timeout to be misread.
+	if (electronPath && fs.existsSync(path.join(path.dirname(electronPath), `new_${path.basename(electronPath)}`))) {
+		fail(`${electronPath} has a downloaded update waiting to be applied, and it exits during startup to install it instead of opening a window. Start and quit VS Code once to apply the update, then run this again.`);
+	}
+
+	quality = parseQuality(testCodePath ? readBuildQuality(testCodePath) : undefined);
 
 	if (opts.remote) {
 		logger.log(`Running desktop remote smoke tests against ${electronPath}`);