[Automated] Update winget CLI Options - #4033
Conversation
thomhurst
added
automated
dependencies
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (31)
📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (46)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe winget documentation now separates executable prerequisites from package installation, simplifies the module example, and documents additional command subcommands and options. ChangesWinget documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: 🟡 Moderate · up to The generated winget API surface has changed, but the API compatibility gate is currently failing. Merge readiness is blocked until the compatibility issue is corrected or explicitly accepted. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR regenerates the Winget integration from Winget v1.29.290, expanding command coverage and updating generated options, services, documentation, and coverage metadata.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported pin authentication-mode issue is fixed because the generated properties are valued string options and the command serializer emits non-null values as
|
| Filename | Overview |
|---|---|
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/WinGetCliScraper.cs | Prioritizes contextual valued-option parsing over boolean heuristics so authentication preference descriptions generate value-taking options. |
| tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/WinGetCliScraperTests.cs | Adds focused regression coverage confirming that explicit authentication choices are classified as a string-valued option. |
| src/ModularPipelines.WinGet/Options/WingetPinAddOptions.Generated.cs | Generates AuthenticationMode as a nullable string with CliOption, causing selected values to be emitted after --authentication-mode. |
| src/ModularPipelines.WinGet/Options/WingetPinListOptions.Generated.cs | Applies the corrected valued authentication-mode representation to pin listing. |
| src/ModularPipelines.WinGet/Options/WingetPinRemoveOptions.Generated.cs | Applies the corrected valued authentication-mode representation to pin removal. |
| src/ModularPipelines.WinGet/Services/IWinget.Generated.cs | Expands the public service contract for newly discovered Winget commands while retaining compatibility defaults. |
| src/ModularPipelines.WinGet/Services/Winget.Generated.cs | Implements the expanded generated command surface through the existing command execution service. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Winget v1.29.290 help output] --> B[WinGet CLI scraper]
B --> C[Option classification]
C --> D[Generated option records]
C --> E[Generated service interfaces]
C --> F[Generated service implementations]
D --> G[Command metadata]
G --> H[Command argument builder]
H --> I[Winget CLI invocation]
Reviews (6): Last reviewed commit: "chore(winget): regenerate corrected API" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Code reviewReviewed the regenerated winget options/services and the new pin/configure/dscv3/settings/source commands. Blocking: breaking public API change, self-reported as a failing gate
-public record WingetExportOptions : WingetOptions
+public record WingetExportOptions(
+ [property: CliArgument(0, Phase = CommandLinePhase.Passthrough, Required = true)] string Output
+) : WingetOptionsThe old This repo has
Right now the PR is asking to merge with a check it knows is failing, which seems like exactly the case automated PRs should not auto-merge through. Non-blocking:
|
ace8e4e to
eff1171
Compare
Code review (update)Re-checked against my previous review on this PR — the branch was updated ( Blocking: breaking public API change, self-reported as a failing gate — still present
- Task<CommandResult> ExportAsync(WingetExportOptions? options = null, ...);
+ Task<CommandResult> ExportAsync(WingetExportOptions options, ...)This is still a breaking change to existing consumers (both the constructor shape and losing the optional-parameter overload), and the PR description still self-reports "API compatibility gate: failure." This repo's Non-blocking:
|
eff1171 to
73fda5c
Compare
Code review (update)Re-checked against my previous review and its follow-up. The winget files themselves are byte-for-byte unchanged since the last-reviewed commit ( Blocking: breaking public API change, self-reported as a failing gate — still present
- Task<CommandResult> ExportAsync(WingetExportOptions? options = null, ...);
+ Task<CommandResult> ExportAsync(WingetExportOptions options, ...)The PR description still self-reports "API compatibility gate: failure." I dug into why this specific case isn't being caught, since two generator fix commits landed on the base branch since the last review ( So the fix isn't done yet — it needs Non-blocking:
|
|
Regeneration from current main with WinGet v1.29.290 now reaches the corrected authentication heuristic, but fails before writing output: winget hash declares positional operands in usage winget hash [-f] [], while no CliPositionalArgument is generated. PR #4003 owns this usage/operand fix and remains the prerequisite. Generated files were not hand-edited. |
|
Regenerated Winget v1.29.290 from a clean current-main baseline plus PR #4003: 42 commands, 49 generated artifacts, zero generator/API errors. The full Winget solution builds with 0 warnings/errors; focused tests pass 1/1. AuthenticationMode on pin add/list/remove is now a valued string CliOption. Do not merge before #4003. Generation used a disposable serial scraper override because Winget help invocations fail nondeterministically at parallelism 20; evidence recorded on #3996. |
6df21df to
32a7857
Compare
|
Reconstructed the long-lived automated branch as one clean Winget artifact commit on current main after #4003 merged; this avoids replaying 135 obsolete branch-history commits. The resulting PR diff is the same 47 Winget files from the validated 49-path manifest. Current-main Winget solution build: 0 warnings/errors. Focused tests: 1/1 passed. Head: 32a7857. |
GitHub Actions
Inactive
Code review (update)Re-checked against the exact current head Blocking: the previously-flagged breaking API change is still not fixed — it's just moved from compile-time to runtime
public record WingetExportOptions(
[property: CliArgument(0, Phase = CommandLinePhase.Passthrough, Required = true)] string Output
) : WingetOptions
{
public WingetExportOptions()
: this(default(string)!)
{
}
......and the This makes the PR's self-reported "API compatibility validation passed" checkbox misleading: call sites now compile, but Bug: several newly-generated boolean switches are typed as string
|
32a7857 to
5993fca
Compare
|
Addressed exact-head review findings at
Validation on rebased head:
@claude Please re-review exact head |
Code review (update)Re-checked against the current head Previously-blocking issue is resolved
Previously-flagged bug is fixed for the files it affects
Design suggestion (non-blocking): the flag-detection fix is a hardcoded, version-pinned allowlist rather than a structural fixThe root cause fix in private static readonly HashSet<string> BooleanOptions = new(StringComparer.OrdinalIgnoreCase)
{
"--allow-reboot", "--blocking", "--dependencies-only", "--details", "--exact",
"--force", "--history", "--include-versions", "--installed", "--msix",
"--preserve", "--suppress-initial-details", "--uninstall-previous",
"--upgrade-available", "--versions", "--wait",
};plus a one-off special case keyed on the generated C# class name: private static bool IsKnownBooleanOption(string className, string longForm) =>
BooleanOptions.Contains(longForm)
|| (className.Equals("WingetSourceAddOptions", StringComparison.Ordinal)
&& longForm.Equals("--explicit", StringComparison.OrdinalIgnoreCase));This fixes today's known instances, but it doesn't fix the underlying heuristic (
Not a blocker for this PR — the concrete instances it fixes are correct, and the pattern (heuristic + escape-hatch allowlist) already exists elsewhere in this generator — but worth tracking as a follow-up so this class of bug doesn't keep recurring one switch at a time. Still open (non-blocking, unchanged from prior review)
Everything elseNo new concerns. The rest of the regenerated surface (new |
GitHub Actions
Inactive
1 participant
Summary
This PR contains automatically generated updates to winget CLI options classes, reconstructed as one clean artifact commit on current
mainafter #4003.Changes
--authentication-modeas a valued string optionCommand coverage
be6725b4f2522a990030007897738ac016f66f7f5ce700a730ea0a50070fb1bbVerification
🤖 Generated with ModularPipelines.OptionsGenerator