Validate playwrightCliVersion shape with strict SemVer#18225
Conversation
The playwrightCliVersion configuration value is forwarded to npm as the package version specifier. Previously any non-empty string would be passed through, so a typo or unsupported shape (a range, an npm dist-tag like 'latest', a v-prefixed version, etc.) would surface as a generic 'failed to resolve' error from npm. Validate the override with SemVersion.TryParse using SemVersionStyles.Strict and fail fast with a clear message that names the configuration key and the offending value when it is not a valid SemVer 2.0 version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18225Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18225" |
There was a problem hiding this comment.
Pull request overview
This PR ports the strict SemVer validation fix from the release/13.4 branch (#18205) to main. It prevents the playwrightCliVersion configuration override from accepting non-version strings (npm ranges, dist-tags, incomplete versions, etc.) that could be interpreted by npm in unexpected ways, instead returning a clear validation error early.
Changes:
- Adds
SemVersion.TryParsevalidation inPlaywrightCliInstaller.InstallCoreAsyncto reject non-strict SemVer values before they reach npm resolution - Adds a new localized error message (
PlaywrightCliInstaller_InvalidVersionOverride) to the .resx and all .xlf translation files - Adds a parameterized test covering five invalid version format variants
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Agents/Playwright/PlaywrightCliInstaller.cs | Refactors ternary into if/else and adds strict SemVer validation before using the version override |
| src/Aspire.Cli/Resources/AgentCommandStrings.resx | Adds PlaywrightCliInstaller_InvalidVersionOverride resource string |
| src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs | Auto-generated accessor for the new resource string |
| src/Aspire.Cli/Resources/xlf/AgentCommandStrings.*.xlf (12 files) | Adds the new string to all localization files |
| tests/Aspire.Cli.Tests/Agents/PlaywrightCliInstallerTests.cs | Adds [Theory] test for five invalid version formats verifying early failure |
Copilot's findings
Files not reviewed (1)
- src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs: Generated file
- Files reviewed: 16/17 changed files
- Comments generated: 0
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Dogfood install + commit verificationObjective: Verify dogfood artifact corresponds to the PR head. Steps:
Evidence:
Observations:
Scenario 2: Strict SemVer accepted (happy path)Objective: Verify strict SemVer override values are accepted. Steps:
Evidence:
Observations:
Scenario 3: Invalid override rejected (unhappy path)Objective: Verify non-strict SemVer values are rejected with clear failure behavior. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Validation failure status and non-empty message including the invalid value. Summary
Overall Result✅ PR VERIFIED Recommendations
|
PR Testing Complete\n\n✅ Verified\n\n- Installed dogfood CLI version matched PR head commit ().\n- Happy path passed: strict SemVer override accepted.\n- Unhappy path passed: invalid override values were rejected with validation failure behavior.\n\nOverall result: PR VERIFIED. |
PR Testing Complete✅ Verified
Overall result: PR VERIFIED. |
Port of #18205 to main.