Skip to content

Add interactive selection for ambiguous package matches - #6575

Open
AmirMS (AmelBawa-msft) wants to merge 33 commits into
feature/multi-source-deduplicationfrom
user/amelbawa/interactive-package-selection
Open

AmirMS (AmelBawa-msft) wants to merge 33 commits into
feature/multi-source-deduplicationfrom
user/amelbawa/interactive-package-selection

Conversation

@AmelBawa-msft

@AmelBawa-msft AmirMS (AmelBawa-msft) commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

Let users resolve ambiguous matches without restarting single-package install, show, and download commands. Opt in with experimentalFeatures.interactivePackageSelection.

  • Reuse the existing ambiguity tables with automatic numbering and one row per candidate.
  • Apply source priority before prompting and continue with the selected package and original command options.
  • Support invalid-input recovery and cancellation with 0 or Ctrl+C.
  • Preserve ambiguity errors for noninteractive, redirected, and truncated-result scenarios.

Includes localized strings, regression coverage, release notes, and a UX specification. Narrow terminals use existing table truncation.

🎞️ Demo

winget-vs-wingetdev-selection

🔗 References

🔍 Validation

  • Built and deployed x64 Debug wingetdev from d729b8ba on the host. Focused native tests passed: 3,471 assertions across 92 test cases.
  • All 34 terminal scenarios passed, covering default-off and opt-in behavior, selection, version lists, invalid/empty/long input, integer boundaries, EOF, cancellation, noninteractive fallback, --silent prompting, and 24-column tables.
  • Three native-console GUI scenarios passed at 100 and 45 columns, including --no-vt. UI Automation exposed the table, prompt, retry feedback, and full selected ID.
  • All 150 repeated Ctrl+C cases across show, install, and download returned E_ABORT. The slowest cancellation was about 0.118 seconds.
  • Install/download scenarios cancelled before acting on a package. Production WinGet was unchanged, development settings were restored, and all test processes and windows were closed.

Screen-reader speech was not tested.

✅ Checklist

🤖 AI Assistance

  • AI assistance was used and has been disclosed in this PR
  • No AI assistance was used

GitHub Copilot assisted with implementation, tests, documentation, code review, and local validation.

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

@github-actions

This comment has been minimized.

@AmelBawa-msft
AmirMS (AmelBawa-msft) marked this pull request as ready for review September 28, 2026 23:13
@AmelBawa-msft
AmirMS (AmelBawa-msft) requested a review from a team as a code owner September 28, 2026 23:13

@JohnMcPMS JohnMcPMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It feels like the code should be structured as such:

  • WorkflowBase :: Does the top-level decision on flow support for selection, generates table and decides on strings.
  • PromptFlow :: Takes in table+strings and determines if prompting is allowed/possible. This is a generic selection flow task. Handles output of the values, input validation and repitition.
  • Reporter :: Handles improved input with support for cancellation. Performs no output.

Yes, this also means refactoring the table output into non-templated functions. That is probably for the best anyway.

Comment thread doc/specs/#5345 - Interactive package selection.md Outdated
Comment thread src/AppInstallerCLICore/Commands/DownloadCommand.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/UpdateFlow.cpp Outdated
Comment thread src/AppInstallerCLICore/ExecutionReporter.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread doc/specs/#5345 - Interactive package selection.md Outdated
Comment thread doc/specs/#5345 - Interactive package selection.md Outdated
Comment thread doc/specs/#5345 - Interactive package selection.md Outdated
Comment thread doc/ReleaseNotes.md Outdated
Comment thread src/AppInstallerCLICore/Commands/ShowCommand.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ranm-msft ranm-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked this against the invariant that matters most for a new prompt: nothing that is not a real interactive console session should ever start waiting on input. That holds here. The disable-interactivity switch, the settings-level disable, hidden and non-output channels, and a redirected stdin or stdout all fall back to the existing multiple-applications-found error rather than prompting, and the unambiguous single-match path is untouched. Strings are in the resource file with the placeholders locked, and there are tests for both the non-interactive fallback and user cancellation.

Two things I would want settled before this goes in.

The first is evidence rather than design: the build and test job is skipped at the current head, so nothing has actually run the new tests. I would like a green run before merge, since the whole value of this change is that the prompt appears exactly when it should and never otherwise.

The second is the product question already on the thread. Deliberately letting the silent switch through to this prompt is defensible - silent describes installer behavior, not CLI interactivity - but it is the kind of decision that surprises someone scripting against the CLI. Routing a new interactive surface through the experimental feature process first would let us change our minds cheaply. I do not feel strongly about which way that lands, only that it should be an explicit call.

Base automatically changed from user/amelbawa/source-filter to feature/multi-source-deduplication September 30, 2026 18:12
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.h Outdated
Comment thread src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Comment thread src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw Outdated
Comment thread src/AppInstallerCLICore/ExecutionReporter.h
@AmelBawa-msft
AmirMS (AmelBawa-msft) force-pushed the user/amelbawa/interactive-package-selection branch from 1de7935 to b85ba3b Compare September 30, 2026 18:36
Comment thread doc/specs/#5345 - Interactive package selection.md Outdated
Comment thread doc/specs/#5345 - Interactive package selection.md
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.h
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp
Comment thread src/AppInstallerCLICore/ExecutionReporter.cpp
Comment thread src/AppInstallerCLICore/ExecutionReporter.h
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp
Comment thread src/AppInstallerCLICore/ExecutionContextData.h Outdated
Comment thread doc/ReleaseNotes.md Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
@github-actions

This comment was marked as resolved.

Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.h Outdated
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
Comment thread src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated
Comment thread src/AppInstallerCLICore/Workflows/PromptFlow.cpp Outdated
// Outputs: None
void ReportMultiplePackageFoundResult(Execution::Context& context);
// Builds the ambiguity table for installed-package matches.
Execution::TableOutputBase GetMultiplePackageFoundResultTable(Execution::Context& context);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do these need to be public? I didn't see any product use of them outside of the implementation .cpp.

auto table = operationTargetsInstalled ? GetMultiplePackageFoundResultTable(context) :
GetMultiplePackageFoundResultTableWithSource(context);
bool selectionSupported = m_selectionBehavior == PackageSelectionBehavior::Prompt &&
(m_operationType == OperationType::Install || m_operationType == OperationType::Show || m_operationType == OperationType::Download) &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With the change to use those existing tables, the operation type should no longer be a blocker. Unless the blocker was more about deciding which paths should enable selection.

}
if (selectionSupported)
{
context.Reporter.Info() << Resource::String::PackageSelectionRefine << std::endl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this just be output regardless of selection support?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants