Move the scenario runner out of the MCP server - #331862
Conversation
The validate-ui-scenario skill runs `runScenario`, which drives VS Code through `test/automation` and writes an evidence bundle. None of that is MCP: the runner loads no MCP module at runtime, and the SDK import it inherited was type-only, so TypeScript already elided it. It only lived under `test/mcp` because that is where the evidence pipeline was first written. That matters now: deleting the MCP server would take the skill with it. Move the six files that have nothing to do with MCP into a new `test/scenario` package, and leave `test/mcp` as one of its consumers alongside the skill. The MCP evidence tools move to `test/mcp/src/evidenceTools.ts`, where the server-specific schemas belong. Deleting `test/mcp` now removes only MCP code. Drop the step banner along with it. `showOverlay` appended a banner to the DOM of the product under test, which can shift layout and influence focus, so the runner always opted out via VSCODE_EVIDENCE_CLEAN_CAPTURE. With the runner as the only caller that opinion is unanimous, so the overlay and its opt-out both go and the capture is unconditionally faithful. Step titles are still rendered onto the finished recording by renderEvidenceChapters. The new package emits declarations, matching `test/automation`, so the MCP server keeps real types rather than silently degrading to `any`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: TylerLeonhardtMatched files:
|
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
Pull request overview
Moves UI scenario execution into an MCP-independent package while retaining MCP integration.
Changes:
- Adds the typed
test/scenariopackage for launching, evidence capture, and video captions. - Updates MCP tools to consume the shared scenario package.
- Removes in-product overlays and updates build rules and documentation.
Show a summary per file
| File | Description |
|---|---|
.github/skills/validate-ui-scenario/SKILL.md |
Updates scenario commands and paths. |
build/npm/dirs.ts |
Registers the scenario package for dependency installation. |
eslint.config.js |
Adds scenario package import boundaries. |
test/scenario/.gitignore |
Ignores generated package artifacts. |
test/scenario/package.json |
Defines scenario build and watch scripts. |
test/scenario/tsconfig.json |
Configures compilation and declaration output. |
test/scenario/src/index.ts |
Exposes the scenario package API. |
test/scenario/src/application.ts |
Hosts shared application lifecycle logic. |
test/scenario/src/evidence.ts |
Decouples evidence capture from MCP and removes overlays. |
test/scenario/src/options.ts |
Hosts shared CLI option parsing. |
test/scenario/src/renderEvidenceChapters.ts |
Generates captioned evidence recordings. |
test/scenario/src/runScenario.ts |
Updates the standalone scenario entry point. |
test/scenario/src/utils.ts |
Hosts shared runner utilities. |
test/mcp/package.json |
Builds and watches the scenario dependency. |
test/mcp/src/automation.ts |
Uses shared application and evidence services. |
test/mcp/src/evidenceTools.ts |
Contains MCP-specific evidence tool schemas. |
test/mcp/src/stdio.ts |
Imports shared runtime services. |
test/mcp/src/automationTools/activityBar.ts |
Uses the shared application service. |
test/mcp/src/automationTools/chat.ts |
Uses the shared application service. |
test/mcp/src/automationTools/core.ts |
Uses shared application and profile helpers. |
test/mcp/src/automationTools/debug.ts |
Uses the shared application service. |
test/mcp/src/automationTools/editor.ts |
Uses the shared application service. |
test/mcp/src/automationTools/explorer.ts |
Uses the shared application service. |
test/mcp/src/automationTools/extensions.ts |
Uses the shared application service. |
test/mcp/src/automationTools/index.ts |
Uses the shared application service. |
test/mcp/src/automationTools/keybindings.ts |
Uses the shared application service. |
test/mcp/src/automationTools/localization.ts |
Uses the shared application service. |
test/mcp/src/automationTools/notebook.ts |
Uses the shared application service. |
test/mcp/src/automationTools/problems.ts |
Uses the shared application service. |
test/mcp/src/automationTools/profiler.ts |
Uses the shared application service. |
test/mcp/src/automationTools/quickAccess.ts |
Uses the shared application service. |
test/mcp/src/automationTools/scm.ts |
Uses the shared application service. |
test/mcp/src/automationTools/search.ts |
Uses the shared application service. |
test/mcp/src/automationTools/settings.ts |
Uses the shared application service. |
test/mcp/src/automationTools/statusbar.ts |
Uses the shared application service. |
test/mcp/src/automationTools/task.ts |
Uses the shared application service. |
test/mcp/src/automationTools/terminal.ts |
Uses the shared application service. |
test/mcp/src/automationTools/windows.ts |
Uses the shared application service. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 35/38 changed files
- Comments generated: 1
- Review effort level: Balanced
Registering `test/scenario` in `build/npm/dirs.ts` makes the root install run npm in that directory, and CI uses `npm ci`, which requires a lockfile. Every other package registered there has one, so a clean CI install failed immediately with ENOENT on `test/scenario/package-lock.json` before anything compiled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485
`test/mcp/**` notifies @TylerLeonhardt, so moving the runner to `test/scenario` silently dropped notifications for it. Point the new path at the owner of the validate-ui-scenario skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485
The root package already declares `@types/node` as a devDependency, so the extracted package inherits it through normal ancestor resolution; declaring it again added a dependency that the OSS license check cannot cover, because `@types/node` ships no LICENSE file and is not in ClearlyDefined or cglicenses.json. Verified against the state CI produces: `npm ci` in `test/scenario` installs no `@types/node`, and both packages still compile, so the types resolve from the repository root as intended. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485
Follow-up to #331674, and a prerequisite for deleting the VS Code automation MCP server.
Why
The
validate-ui-scenarioskill runsrunScenario, which drives VS Code throughtest/automationand writes an evidence bundle (video, per-step screenshots, trace,manifest.json,report.html).None of that is MCP. Verified by hooking
Module.requireand loading the runner:The
McpServerimport it inherited is used only in type positions, so TypeScript already elides it — compiledevidence.jsrequires justfs,path,zodand./application. The runner lived undertest/mcponly because that is where the evidence pipeline was first written.That matters now that deleting the MCP server is on the table: today
rm -rf test/mcpwould silently take the skill with it.What
Six files with no MCP dependency move to a new
test/scenariopackage:runScenario.tsapplication.tsevidence.tsrenderEvidenceChapters.tsutils.ts,options.tstest/mcpbecomes a consumer of that package, alongside the skill. The MCP evidence tools move totest/mcp/src/evidenceTools.ts, where the server-specific zod schemas belong. Deletingtest/mcpnow removes only MCP code.The new package emits declarations, matching
test/automation— without that the MCP server would have compiled against an untyped module and silently degraded toany.The step banner goes too
showOverlayappended a banner to the DOM of the product under test, which can shift layout and influence focus or selectors, so the runner always opted out withVSCODE_EVIDENCE_CLEAN_CAPTURE=1. With the runner as the only remaining caller that opinion is unanimous, so the overlay and its opt-out both go and the capture is unconditionally faithful. Step titles are still rendered onto the finished recording byrenderEvidenceChapters.Verification
test/scenarioandtest/mcpboth compile from clean, with types flowing across the boundarytest/scenario/**import boundary was confirmed active by probing it with a deliberately forbidden import rather than trusting a clean runNot included
.build/vscode-playwright-mcp/keeps its name. It is generated output shared with the MCP server today, and renaming it would invalidate paths documented in a skill that shipped this week — better as a follow-up once the server is actually gone.