Add environment path variable resource - #1726
Conversation
Split scalar and path environment variable contracts, retain mixed list operations, and add filterable list export with wildcard name matching. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The manifest must require name for scalar list items to match executable validation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a dedicated path environment-variable resource while extending mixed list operations, filtering, export support, and test coverage.
Changes:
- Introduces
EnvironmentPathVariable. - Supports mixed scalar/path list operations and filtered exports.
- Adds Rust and Pester coverage.
- Retains required-name validation for scalar list items.
File summaries
| File | Summary |
|---|---|
resources/environment_variable/tests/environment_variable_test.tests.ps1 |
Updates mixed-list tests. |
resources/environment_variable/tests/environment_variable_single_test.tests.ps1 |
Updates scalar tests. |
resources/environment_variable/tests/environment_variable_set.tests.ps1 |
Tests scalar/path setting. |
resources/environment_variable/tests/environment_variable_get.tests.ps1 |
Tests scalar/path retrieval. |
resources/environment_variable/tests/environment_variable_export.tests.ps1 |
Tests export and filtering. |
resources/environment_variable/tests/environment_path_variable.tests.ps1 |
Tests the new path resource. |
resources/environment_variable/src/types.rs |
Defines scalar/path types and validation. |
resources/environment_variable/src/main.rs |
Handles path and export inputs. |
resources/environment_variable/src/environment.rs |
Implements mixed operations and export filtering. |
resources/environment_variable/locales/en-us.toml |
Adds validation and export messages. |
resources/environment_variable/environment_variable.dsc.manifests.json |
Defines resource schemas and operations. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep scalar and path list schemas aligned with executable validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
😊 Code Coverage ReportChanged Code Coverage86% (80%+ coverage)
🔵 Full Codebase Coverage82% (good)
|
Add registry-backed tests for scalar, path, export, removal, and unsupported values. Refactor CLI parsing and result serialization into testable helpers and cover valid and invalid input shapes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The missing-operation message advertises unsupported export syntax and incorrectly presents filter input as mandatory.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
resources/environment_variable/locales/en-us.toml:4
- This usage advertises
export --pathand makes--inputappear mandatory, but export is accepted only with--listand its filter input is optional. Please show the export syntax separately so the error directs callers to a valid command.
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Microsoft.Windows/EnvironmentVariablecurrently mixes scalar and path semantics, so callers cannot reliably retrieve variables such asPATHas arrays. This implements the working-group decision to separate those contracts while preserving mixed list operations.Microsoft.Windows/EnvironmentVariablescalar-only.Microsoft.Windows/EnvironmentPathVariablewith array-valuedvalue, write-onlydelimiter, and write-onlysetAction.Microsoft.Windows/EnvironmentVariableListto get, set, and test mixed scalar/path items.*wildcard matching for names.Fixes: #1724