[release/13.2] Fix Playwright CLI provenance verification for tag format change#16134
Merged
joperezr merged 2 commits intorelease/13.2from Apr 14, 2026
Merged
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16134Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16134" |
Contributor
There was a problem hiding this comment.
Pull request overview
Cherry-pick onto release/13.2 to fix Playwright CLI (@playwright/cli) provenance verification failures caused by an upstream git tag format change (dropping the v prefix), and to add regression coverage.
Changes:
- Update
PlaywrightCliInstallerprovenancevalidateWorkflowReflogic to accept bothrefs/tags/{version}andrefs/tags/v{version}. - Add a unit test that captures the workflow-ref validator and verifies both tag formats are accepted.
- Add an end-to-end test intended to validate
aspire new→ agent init → Playwright CLI install completes without provenance errors.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Aspire.Cli/Agents/Playwright/PlaywrightCliInstaller.cs |
Relax workflow-ref tag matching to accept both tag naming conventions. |
tests/Aspire.Cli.Tests/Agents/PlaywrightCliInstallerTests.cs |
Adds a unit test to validate both tag formats via the captured validator callback. |
tests/Aspire.Cli.EndToEnd.Tests/NewWithAgentInitTests.cs |
Introduces an E2E regression test for the aspire new + agent init Playwright install flow. |
Comment on lines
+346
to
+370
| [Fact] | ||
| public async Task InstallAsync_WorkflowRefValidator_AcceptsBothTagFormats() | ||
| { | ||
| var version = SemVersion.Parse("0.1.7", SemVersionStyles.Strict); | ||
| var npmRunner = new TestNpmRunner | ||
| { | ||
| ResolveResult = new NpmPackageInfo { Version = version, Integrity = "sha512-abc123" } | ||
| }; | ||
| var provenanceChecker = new TestNpmProvenanceChecker(); | ||
| var playwrightRunner = new TestPlaywrightCliRunner(); | ||
| var installer = new PlaywrightCliInstaller(npmRunner, provenanceChecker, playwrightRunner, new TestInteractionService(), new ConfigurationBuilder().Build(), NullLogger<PlaywrightCliInstaller>.Instance); | ||
|
|
||
| await installer.InstallAsync(CreateTestContext(), CancellationToken.None); | ||
|
|
||
| Assert.True(provenanceChecker.ProvenanceCalled); | ||
| Assert.NotNull(provenanceChecker.CapturedValidateWorkflowRef); | ||
|
|
||
| // Accept tags without 'v' prefix (0.1.7+) | ||
| Assert.True(WorkflowRefInfo.TryParse($"refs/tags/{version}", out var refWithout)); | ||
| Assert.True(provenanceChecker.CapturedValidateWorkflowRef(refWithout!)); | ||
|
|
||
| // Accept tags with 'v' prefix (pre-0.1.7) | ||
| Assert.True(WorkflowRefInfo.TryParse($"refs/tags/v{version}", out var refWith)); | ||
| Assert.True(provenanceChecker.CapturedValidateWorkflowRef(refWith!)); | ||
| } |
There was a problem hiding this comment.
This new test calls InstallAsync but doesn't assert the returned result, and with the current TestNpmRunner/TestPlaywrightCliRunner defaults the install will return false (PackResult is null and InstallSkillsResult defaults to false). Consider either asserting the expected false result explicitly (to make the intent clear), or fully wiring the runner fakes (PackResult + valid integrity + InstallSkillsResult) so the install path succeeds while still capturing the workflow ref validator.
Comment on lines
+49
to
+58
| // Create .claude folder so agent init detects a Claude Code environment. | ||
| // This needs to exist in the workspace root before aspire new creates the project | ||
| // because agent init chains after project creation and looks for environment markers. | ||
| await auto.TypeAsync("mkdir -p .claude"); | ||
| await auto.EnterAsync(); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
|
|
||
| // Run aspire new with the Starter template, going through all prompts manually | ||
| // so we can ACCEPT the agent init prompt instead of declining it. | ||
| await auto.TypeAsync("aspire new"); |
There was a problem hiding this comment.
The test setup creates a .claude folder in the workspace root before running aspire new, but NewCommand chains agent init using the template output directory as the workspace root. This means the Claude Code scanner can end up detecting the parent .claude and writing config/skill files outside the newly created project directory, making the test less representative and potentially fragile if scanner boundary behavior changes.
Suggested change
| // Create .claude folder so agent init detects a Claude Code environment. | |
| // This needs to exist in the workspace root before aspire new creates the project | |
| // because agent init chains after project creation and looks for environment markers. | |
| await auto.TypeAsync("mkdir -p .claude"); | |
| await auto.EnterAsync(); | |
| await auto.WaitForSuccessPromptAsync(counter); | |
| // Run aspire new with the Starter template, going through all prompts manually | |
| // so we can ACCEPT the agent init prompt instead of declining it. | |
| await auto.TypeAsync("aspire new"); | |
| // Create a dedicated project directory and place .claude inside it so agent init | |
| // detects the Claude Code environment from the actual template output directory. | |
| await auto.TypeAsync("mkdir -p agent-init-project && cd agent-init-project && mkdir -p .claude"); | |
| await auto.EnterAsync(); | |
| await auto.WaitForSuccessPromptAsync(counter); | |
| // Run aspire new in the current directory, going through all prompts manually | |
| // so we can ACCEPT the agent init prompt instead of declining it. | |
| await auto.TypeAsync("aspire new ."); |
Comment on lines
+118
to
+136
| // Agent init: skill location - select Claude Code | ||
| await auto.WaitUntilAsync( | ||
| s => s.ContainsText("skill files be installed"), | ||
| timeout: TimeSpan.FromSeconds(60), | ||
| description: "skill location prompt"); | ||
| await auto.TypeAsync(" "); // Toggle off default Standard location | ||
| await auto.DownAsync(); | ||
| await auto.TypeAsync(" "); // Toggle on Claude Code location | ||
| await auto.EnterAsync(); | ||
|
|
||
| // Agent init: skill selection - toggle on Playwright CLI | ||
| await auto.WaitUntilAsync( | ||
| s => s.ContainsText("skills should be installed"), | ||
| timeout: TimeSpan.FromSeconds(30), | ||
| description: "skill selection prompt"); | ||
| await auto.DownAsync(); | ||
| await auto.TypeAsync(" "); // Toggle on Playwright CLI | ||
| await auto.EnterAsync(); | ||
|
|
There was a problem hiding this comment.
This E2E flow waits for prompt text like "skill files be installed" and "skills should be installed", but those strings don't appear in the current CLI resources/agent init command flow (agent init uses a single selection prompt like "What would you like to configure?"). As written, these waits are likely to time out/hang; consider matching the actual prompt text and selecting the relevant option(s) (e.g., the Playwright CLI install choice) based on the current AgentInitCommand output.
Suggested change
| // Agent init: skill location - select Claude Code | |
| await auto.WaitUntilAsync( | |
| s => s.ContainsText("skill files be installed"), | |
| timeout: TimeSpan.FromSeconds(60), | |
| description: "skill location prompt"); | |
| await auto.TypeAsync(" "); // Toggle off default Standard location | |
| await auto.DownAsync(); | |
| await auto.TypeAsync(" "); // Toggle on Claude Code location | |
| await auto.EnterAsync(); | |
| // Agent init: skill selection - toggle on Playwright CLI | |
| await auto.WaitUntilAsync( | |
| s => s.ContainsText("skills should be installed"), | |
| timeout: TimeSpan.FromSeconds(30), | |
| description: "skill selection prompt"); | |
| await auto.DownAsync(); | |
| await auto.TypeAsync(" "); // Toggle on Playwright CLI | |
| await auto.EnterAsync(); | |
| // Agent init: select the Playwright CLI configuration option from the current menu-based flow. | |
| await auto.WaitUntilAsync( | |
| s => s.ContainsText("What would you like to configure?") && s.ContainsText("Playwright CLI"), | |
| timeout: TimeSpan.FromSeconds(60), | |
| description: "agent init configuration prompt"); | |
| await auto.DownAsync(); | |
| await auto.EnterAsync(); |
JamesNK
reviewed
Apr 14, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
1 suggestion: missing negative assertions on the provenance validation callback test.
) * Fix Playwright CLI provenance verification for tag format change @playwright/cli 0.1.7 changed their git tag naming convention from 'v0.1.7' to '0.1.7' (dropped the 'v' prefix). This caused our provenance verification to fail with WorkflowRefMismatch. - Accept both tag formats (with and without 'v' prefix) in the validateWorkflowRef callback for forward/backward compatibility - Add test verifying both tag formats pass validation Co-authored-by: Copilot <[email protected]> * Add E2E test for aspire new with agent init and Playwright install Exercises the full aspire new → agent init → Playwright CLI install flow end-to-end, verifying no provenance verification errors appear. This is the primary regression test for tag format changes in upstream @playwright/cli releases. The test accepts the agent init prompt (instead of declining), selects Playwright CLI during skill selection, and fails fast if a provenance verification error is detected during npm package installation. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
mitchdenny
force-pushed
the
cherry-pick/playwright-provenance-fix
branch
from
16c59a8 to
714f998
Compare
Verify the provenance validator rejects wrong versions and branch refs, not just that it accepts the expected tag formats. Addresses review feedback from @JamesNK. Co-authored-by: Copilot <[email protected]>
JamesNK
approved these changes
Apr 14, 2026
Contributor
|
🎬 CLI E2E Test Recordings — 53 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24384964911 |
joperezr
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Cherry-pick of #16087 from
mainontorelease/13.2for the next 13.2.x servicing release.Problem:
@playwright/cli0.1.7 changed their git tag naming convention fromv0.1.7to0.1.7(dropped thevprefix). This caused Aspire's provenance verification to fail withWorkflowRefMismatchwhen installing the Playwright CLI agent skill.Fix: Accept both tag formats (with and without
vprefix) in thevalidateWorkflowRefcallback for forward/backward compatibility.Cherry-pick conflict resolution: The unit test for the new tag format validation was adapted to use the
release/13.2branch'sInstallAsync(AgentEnvironmentScanContext, CancellationToken)API signature (which returnsbool) instead of main's tuple-returning API with explicit temp directory management.Validation: Merge conflict resolved by adapting the new
InstallAsync_WorkflowRefValidator_AcceptsBothTagFormatstest to match the release branch'sCreateTestContext()pattern used by all other tests in the file.Checklist