Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 13, 2025

Summary

The parameterNamesToIgnore option in rush-project.json accepts any parameter names without validating they exist for the operation in command-line.json, leading to silent configuration errors.

This PR adds validation to ensure that all parameter names specified in parameterNamesToIgnore actually exist for the operation and are associated with it in command-line.json.

Details

  • Validation: Extended RushProjectConfiguration.validatePhaseConfiguration() to verify each parameter in parameterNamesToIgnore exists in the phase's associatedParameters.
  • Error Reporting: Collects all invalid parameter names and reports them in a single error message with a list of valid parameters for the operation.
  • Test coverage: Added comprehensive test cases covering multiple scenarios:
    • Invalid parameters when no valid parameters exist (including malformed parameter -malformed-parameter)
    • Nonexistent parameters when operation has valid parameters
    • Mix of existent and nonexistent parameters (only nonexistent ones flagged as errors)
  • Changelog: Added an empty changelog entry for @microsoft/rush (type: "none") since the original parameterNamesToIgnore feature hasn't been published yet. Per Rush version policy, all changelogs go to the @microsoft/rush project.

Error Examples

No valid parameters:

The project "test-project-e" has a "config/rush-project.json" configuration that specifies invalid parameter(s) in "parameterNamesToIgnore" for operation "_phase:build": --invalid-parameter, --another-invalid, -malformed-parameter. Valid parameters for this operation are: (none).

With valid parameters available:

The project "test-project-f" has a "config/rush-project.json" configuration that specifies invalid parameter(s) in "parameterNamesToIgnore" for operation "_phase:build": --nonexistent-param, --another-nonexistent. Valid parameters for this operation are: --production, --verbose.

Mix of valid and invalid (only invalid shown):

The project "test-project-g" has a "config/rush-project.json" configuration that specifies invalid parameter(s) in "parameterNamesToIgnore" for operation "_phase:build": --nonexistent-param. Valid parameters for this operation are: --production, --verbose.

This prevents configuration mistakes like typos in parameter names (--prodution vs --production) or referencing parameters that aren't associated with the operation.

How it was tested

  • ✅ All 506 tests pass (added 2 new comprehensive test cases)
  • ✅ Test validates invalid parameters when no valid parameters exist
  • ✅ Test validates nonexistent parameters when valid parameters exist for the operation
  • ✅ Test validates mix of existent and nonexistent parameters (correctly identifies only the invalid ones)
  • ✅ Existing parameterIgnoringRepo test validates that valid parameters continue to work correctly
  • ✅ CodeQL security scan: 0 alerts

Impacted documentation

None. This is an internal validation improvement that doesn't require documentation updates.

Original prompt

The parameterNamesToIgnore option in rush-project.json does not validate that the parameters exist for the operation in common/config/rush/command-line.json. Can you add validation to ensure that parameter exists?


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 13, 2025
Copilot AI and others added 2 commits November 13, 2025 22:50
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Copilot AI changed the title [WIP] Add validation for parameter existence in rush-project.json Add validation for parameterNamesToIgnore in rush-project.json Nov 13, 2025
Copilot finished work on behalf of iclanton November 13, 2025 23:00
Copilot AI requested a review from iclanton November 13, 2025 23:00
Copy link
Member

@iclanton iclanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot - left some feedback.

…er test

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Copilot AI changed the title Add validation for parameterNamesToIgnore in rush-project.json [rush-lib] Add validation for parameterNamesToIgnore in rush-project.json Nov 13, 2025
Copilot AI requested a review from iclanton November 13, 2025 23:16
Copilot finished work on behalf of iclanton November 13, 2025 23:16
@iclanton
Copy link
Member

@copilot you need a changelog entry. An empty entry is fine since the original feature hasn't been published yet.

@iclanton
Copy link
Member

@copilot can you add a test where there are some parameters for the operation and nonexistent parameters are specified? Can you also add a test case where there are some parameters for the operation and both existent and nonexistent parameters are specified?

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 13, 2025

@copilot you need a changelog entry. An empty entry is fine since the original feature hasn't been published yet.

Added empty changelog entry for @microsoft/rush-lib. See commit e71cd0e.

Copilot finished work on behalf of iclanton November 13, 2025 23:30
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 13, 2025

@copilot can you add a test where there are some parameters for the operation and nonexistent parameters are specified? Can you also add a test case where there are some parameters for the operation a...

Added two new test cases:

  1. test-project-f: Operation has valid parameters (--production, --verbose), but only nonexistent parameters are specified in parameterNamesToIgnore
  2. test-project-g: Operation has valid parameters, and both existent (--production, --verbose) and nonexistent (--nonexistent-param) parameters are specified - only the nonexistent one triggers an error

See commit 380e1ec.

Copilot finished work on behalf of iclanton November 13, 2025 23:44
Co-authored-by: David Michon <dmichon@microsoft.com>
@iclanton iclanton marked this pull request as ready for review November 17, 2025 22:15
Copilot finished work on behalf of iclanton November 17, 2025 22:18
Copilot AI requested a review from iclanton November 17, 2025 22:18
iclanton and others added 2 commits November 17, 2025 14:19
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Copilot finished work on behalf of iclanton November 17, 2025 22:22
@iclanton iclanton merged commit 1001da0 into main Nov 17, 2025
12 checks passed
@iclanton iclanton deleted the copilot/add-validation-for-parameter-existence branch November 17, 2025 23:08
@github-project-automation github-project-automation bot moved this from Needs triage to Closed in Bug Triage Nov 17, 2025
iclanton added a commit that referenced this pull request Nov 19, 2025
…ion-test (#5456)

* [rush-lib] fix bridge package command (#5411)

* rush change

* fix: enable prerelease version matching in bridge-package command

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Introduce an ESLint rule to coalesce imports. (#5414)

* Use the no-duplicate-imports rule.

* fixup! Use the no-duplicate-imports rule.

* fixup! Use the no-duplicate-imports rule.

* Fix duplicate import issues.

* Use the import/no-duplicates rule.

* Manual fixes.

* [eslint-plugin] Document the @rushstack/typedef-var rule and clarify scope (#5410)

* docs(eslint-plugin): document the @rushstack/typedef-var rule

* chore(changefile): add changefile for typedef-var documentation

* docs: remove for...of and for...in exception from typedef-var rule documentation

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Bump decpouled local dependencies. (#5415)

* [heft-rspack-plugin] Initial implementation of a Rspack Heft plugin. (#4015)

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [rush] Fork npm-check into rush-lib (#5416)

* Fork npm-check

* rush update

* Update test scripts in package json

* rush update

* Fix test file

* Add and update readmes

* Update readme, update config

* Update readme

---------

Co-authored-by: cmalonzo <cmalonzo@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Fix an issue with the return type of Executable.waitForExitAsync. (#5417)

* Clean up the `repo-scripts bump-decoupled-local-dependencies` action. (#5418)

* Move files around in repo-toolbox.

* Don't bump allowed alternative dependencies.

* Include @rushstack/set-webpack-public-path-plugin@^4.1.16 in common-versions.json.

* [eslint-plugin] Introduce `@rushstack/import-requires-chunk-name` and `@rushstack/pair-react-dom-render-unmount` rules. (#5419)

* Include the @rushstack/import-requires-chunk-name rule.

* Include the @rushstack/pair-react-dom-render-unmount rule.

* fixup! Include the @rushstack/pair-react-dom-render-unmount rule.

* Include missing documentation.

* Rush change.

* Sort README entries.

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Extract CredentialCache from Rush into @rushstack/credential-cache (#5421)

* Add new credential-cache package

* Add test command

* Factor out CredentialCache

* Fix routing

* Fix API

* Update readme

* Update readme

* rush update

* Forward rush-user folder name

* Remove old credentials schema

* Restore caching in getHomeFolder

* Remove reexport of areDeepEqual

* Update API md

* Centralize polyfill

---------

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Fix an issue where suppressed rule violations still show up in the report in ESLint >=9.37.0. (#5422)

* Fix an issue where suppressed rule violations still show up in the report in ESLint >=9.37.0.

* Fix an issue where the ESLint process will crash when running in the ESLint VSCode extension in ESLint >=9.37.0.

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Bump decoupled local dependencies. (#5420)

* [heft-lint] Fix TypeScript program passing (#5428)

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>

* [API Extractor] Add support for printing a diff of changed API reports in non-local builds. (#5427)

* Fix an issue where verbose API extractor messages may not be passed to the Heft plugin.

* Add diff printing to API Extractor's report issue logging.

* Expose an alwaysShowChangedApiReportDiffOnNonLocalBuild option in the api-extractor-heft-plugin configuration file.

* Use the alwaysShowChangedApiReportDiffOnNonLocalBuild in the local rig

* fixup! Add diff printing to API Extractor's report issue logging.

* Generate a JSON schemas artifact after publish. (#5429)

* Copy JSON schemas to projects' temp/json-schemas folders.

* Introduce a tool for collecting JSON schemas.

* Record JSON schemas during publish.

* Produce artifacts during CI.

* Fix publish. (#5430)

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [heft-node-rig] Add lib-dts and lib-esm to cleanFiles glob (#5438)

* Initial plan

* Add lib-dts and lib-esm to cleanFiles in heft-node-rig

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Add change file for heft-node-rig

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [eslint-plugin] Fix no-external-local-imports in editor (#5441)

* [eslint-plugin] Fix calculation of project folder in ESLint
* [heft-lint] Set parserOptions.tsconfigRootDir

---------

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Bump decoupled local dependencies. (#5440)

* Bump cyclic dependencies.

* Bump TSDoc dependencies.

* Rush update.

* [devx] Apply schemas to Rush/Heft config files in VS Code, add clean build launch config (#5445)

* Bind schemas in vscode

* Add clean build launch config

---------

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>

* [heft-typescript] Support `${configDir}` token in tsconfig.json (#5444)

* Support ${configDir} in typescript file copier

* Use ${configDir} in local rigs

* Use const

* Fix a TSDoc link.

---------

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [lint] Ensure correctness of SARIF output logs (#5443)

* [eslint-bulk] Report bulk suppressions as suppressions

* [heft-lint] Include suppressed results

* Enable sarif log for local rigs, add test

* Fix lint cache

* Fix typo in TSLint

---------

Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [rush] Add project-level parameter ignoring to prevent unnecessary cache invalidation (#5433)

* Initial plan

* Add parameter ignoring feature to rush-lib

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Add tests for parameter ignoring feature

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Refactor to eliminate code duplication in parameter collection

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Improve documentation and add clarifying comments

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Address PR feedback: use ignoredParameterValues and improve filtering

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Address final PR feedback: improve logging and test coverage

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Centralize parameter creation helper and share with BaseScriptAction

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Ensure all parameter types are tested for filtering

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Refactor parameter helpers into cli/parsing directory

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Use CommandLineParser to parse parameter values in test

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Use executeWithoutErrorHandlingAsync instead of executeAsync

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Add rush change file for project-level parameter ignoring feature

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Adjust formatting of change file

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Co-authored-by: David Michon <dmichon@microsoft.com>

* [rush-lib] Add validation for parameterNamesToIgnore in rush-project.json (#5448)

* Initial plan

* Add validation for parameterNamesToIgnore in rush-project.json

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Remove unused test repository

* Address PR feedback: combine error messages and add malformed parameter test

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Add changelog entry for rush-lib

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Add tests for parameter validation with valid parameters

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Update libraries/rush-lib/src/api/test/RushProjectConfiguration.test.ts

Co-authored-by: David Michon <dmichon@microsoft.com>

* Update common/changes/@microsoft/rush-lib/copilot-validate-parameter-names-to-ignore_2025-11-13-23-26.json

* Fix changelog entry to use @microsoft/rush package name

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
Co-authored-by: David Michon <dmichon@microsoft.com>

* [rush-lib] Add PathProjectSelectorParser with path: scheme and auto-routing (#5451)

* Initial plan

* Add PathProjectSelectorParser implementation with path: scheme

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Address code review feedback and add comprehensive unit tests for selector parsers

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Add tests for VersionPolicyProjectSelectorParser and SubspaceSelectorParser

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Refactor tests into separate files and improve PathProjectSelectorParser testability

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Add change file for PathProjectSelectorParser feature

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Update common/changes/@microsoft/rush/copilot-add-path-project-selector-parser_2025-11-17-22-30.json

* Address code review feedback: make workingDirectory required and add comprehensive tests

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Fix Windows path separator issue by normalizing to forward slashes for LookupByPath

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Use consistent cwd from RushCommandLineParser for path resolution

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

* Auto-route relative and absolute POSIX paths to path: selector

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Use Path.convertToSlashes helper instead of manual path separator conversion

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>

* Rush change.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Co-authored-by: David Michon <dmichon@microsoft.com>
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>

* Initial plan

* Generate changelogs for webpack version bump

Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>

---------

Co-authored-by: Lincoln <778157949@qq.com>
Co-authored-by: Rushbot <rushbot@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
Co-authored-by: yuza🍊 <yuji.min.dev@gmail.com>
Co-authored-by: Camille Malonzo <cmalonzo@microsoft.com>
Co-authored-by: cmalonzo <cmalonzo@users.noreply.github.com>
Co-authored-by: David Michon <dmichon@microsoft.com>
Co-authored-by: David Michon <dmichon-msft@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants