CLI: Add Source to ArgMap for sourcing the origin of argument data - #41636
Draft
David Bennett (dkbennett) wants to merge 2 commits into
Draft
David Bennett (dkbennett) wants to merge 2 commits into
David Bennett (dkbennett) wants to merge 2 commits into
Conversation
Copilot started reviewing on behalf of
David Bennett (dkbennett)
September 17, 2026 20:02
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the critical header self-containment issue and add the missing parser source-reset regression test.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds provenance tracking to WSLC CLI arguments for improved diagnostics and source attribution.
Changes:
- Adds source masks and aggregation to
ArgMap. - Tags command-line, environment, default, and input values.
- Expands unit-test coverage for provenance and precedence.
File summaries
| File | Summary |
|---|---|
test/windows/wslc/WSLCCLIParserUnitTests.cpp |
Tests parser source attribution. |
test/windows/wslc/WSLCCLIExecutionUnitTests.cpp |
Updates source-aware argument tests. |
test/windows/wslc/WSLCCLIEnvironmentOptionsUnitTests.cpp |
Tests environment provenance and precedence. |
test/windows/wslc/WSLCCLIArgumentUnitTests.cpp |
Tests source aggregation, defaults, and rollback. |
src/windows/wslc/core/EnvironmentOptions.cpp |
Tags environment-derived values. |
src/windows/wslc/commands/RegistryCommand.cpp |
Tags prompted and stdin values. |
src/windows/wslc/arguments/ArgumentParser.cpp |
Tags parsed values as command-line sourced. |
src/windows/wslc/arguments/ArgMap.h |
Stores and exposes argument provenance. |
Review details
Suppressed comments (1)
src/windows/wslc/arguments/ArgumentParser.cpp:123
- The last-wins source reset is not covered: parser tests only repeat command-line flags, so they cannot detect a stale
Source::Environmentbit. Please add a regression case that starts with an environment-backed single-value argument, parses the same option from the command line, and assertsGetSource()is onlySource::CommandLineafterClearArgument/Add.
m_executionArgs.Add(type, value, Source::CommandLine);
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| namespace wsl::windows::wslc::argument { | ||
|
|
||
| enum class Source : uint32_t |
This branch has not been deployed
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.
Summary of the Pull Request
Adds Source data to arguments so their origin can be tracked for diagnostic and user error attribution. The immediate use case is any argument set by environment variable can be known to be set by environment variable so we could give the user more information than just "invalid arg" if that argument fails validation or during execution, since it would be hidden inside an environment var the user may not realize is being used.
This is future-looking plumbing for supporing something like #41468
Sourcebitmask toArgMapwithNone,Default,Environment,Settings,CommandLine, andInputvalues.ArgMap::Add()call to identify its source explicitly.ArgType; repeated values accumulate contributing sources, while single-value replacement clears the previous source.GetValue()defaults asSource::Default.CommandLine, environment bindings asEnvironment, and prompted or stdin values asInput.GetSource()for diagnostics and future user-facing error attribution.The idea here is that if an arg value is being set by settings or environment variables not obvious to the user that we can tailor a more appropriate and effective error message instead of just a somewhat bewildering "session not found" when they didn't provide a session in the command line. This may have other value for future diagnostics and logging to better replicate a failure case by ensuring arguments are sourced the same way as the failure repro which could be part of the bug.
This is not an urgent PR.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Build and added / changed tests pass.