Skip to content

wslc: match docker prune semantics (confirmation prompt, -f aliases --force) - #41455

Open
ggarzia-MSFT wants to merge 11 commits into
masterfrom
user/ggarzia/wslc-prune-docker-parity
Open

wslc: match docker prune semantics (confirmation prompt, -f aliases --force)#41455
ggarzia-MSFT wants to merge 11 commits into
masterfrom
user/ggarzia/wslc-prune-docker-parity

Conversation

@ggarzia-MSFT

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Aligns wslc <object> prune with docker: -f now means --force, and prune without --force shows docker's warning plus an Are you sure you want to continue? [y/N] confirmation. Also plumbs container prune --filter through to the service, which was previously parsed but discarded.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

In docker, --filter only carries a -f short alias on listing commands (docker images, docker ps); on every prune command -f is bound to --force instead. wslc bound -f to --filter everywhere, so wslc container prune -f did the wrong thing.

  • Added a PruneFilter argument type: identical parsing to Filter, but with no short alias. The prune commands now register PruneFilter + Force. Listing commands keep Filter with -f, unchanged.
    • A separate type is required because Argument::Create can override description/required/limit but not the alias, and findArgumentByAlias is first-match-wins with no collision detection.
  • Terminal::Confirm mirrors docker's prompt.Confirm: renders <message> [y/N] , trims the answer, and accepts only a case-insensitive y. End of input declines rather than hanging.
  • CLIExecutionContext::ConfirmPrune is the single choke point: returns immediately when --force is set, otherwise prints the per-command warning and the shared confirmation question.
  • Warning text matches docker per command, including the --all variants for image prune and volume prune, and volume prune's filter help that documents label= rather than until=.
  • ContainerService::Prune now marshals the parsed filters into WSLCFilter[] (mirroring NetworkService::Prune) instead of passing nullptr, 0.
  • Prune filter parse failures report WSLCCLI_InvalidPruneFilterError so the message can name the correct flag spelling.

Validation Steps Performed

All new and affected tests were run against a locally deployed build. 46/46 pass.

Unit tests:

  • WSLCCLITerminalUnitTests — 6 new Terminal_Confirm* tests covering lowercase/uppercase y, surrounding whitespace, rejection of other answers, end-of-input declining, and verbatim rendering of messages containing format characters.
  • WSLCCLICommandUnitTestsPruneCommands_BindShortFToForce asserts all four prune commands expose --force/-f, expose --filter with no alias, and no longer register ArgType::Filter; ListCommands_KeepShortFOnFilter guards that the four listing commands still bind -f to --filter.

E2E tests:

  • WSLCE2EContainerPruneTests (10) — including 5 new tests: declining at end of input, accepting via piped y, -f skipping the prompt, --filter excluding a non-matching container, and a malformed filter being rejected before the prompt is shown.
  • WSLCE2EImagePruneTests (7), WSLCE2ENetworkPruneTests (10), WSLCE2EVolumePruneTests (11) — existing prune invocations updated to pass --force, since these run with stdin bound to NUL and would otherwise decline at the prompt. The three malformed-filter tests deliberately omit --force to prove validation runs before the prompt.

…--force

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 26, 2026 21:41

Copilot AI 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.

Pull request overview

This pull request aligns wslc <object> prune behavior with Docker semantics by introducing a confirmation prompt unless --force is provided, rebinding -f to --force for prune commands, and ensuring container prune filters are actually sent to the service.

Changes:

  • Added an interactive confirmation flow for prune operations via CLIExecutionContext::ConfirmPrune() and Terminal::Confirm(), with --force (and -f on prune commands) bypassing the prompt.
  • Introduced ArgType::PruneFilter (no short alias) and updated prune commands/tasks to use it, keeping list commands’ -f--filter behavior unchanged.
  • Plumbed container prune --filter through to the service by marshaling filters into WSLCFilter[] for the COM call, and updated/added unit + E2E tests accordingly.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/windows/wslc/WSLCCLITerminalUnitTests.cpp Adds unit tests for Terminal::Confirm() input handling and prompt rendering.
test/windows/wslc/WSLCCLICommandUnitTests.cpp Adds command-level tests ensuring prune vs list -f alias binding matches Docker semantics.
test/windows/wslc/e2e/WSLCE2EVolumePruneTests.cpp Updates prune invocations to pass --force where needed; adjusts invalid-filter expectation string.
test/windows/wslc/e2e/WSLCE2ENetworkPruneTests.cpp Updates prune invocations to pass --force where needed; adjusts invalid-filter expectation string.
test/windows/wslc/e2e/WSLCE2EImagePruneTests.cpp Updates prune invocations to pass --force where needed; adjusts invalid-filter expectation string and formatting.
test/windows/wslc/e2e/WSLCE2EContainerPruneTests.cpp Adds E2E coverage for prune confirmation behavior, -f as force, and filter effects.
src/windows/wslc/tasks/VolumeTasks.cpp Adds ConfirmPrune() gate and switches to ArgType::PruneFilter.
src/windows/wslc/tasks/NetworkTasks.cpp Adds ConfirmPrune() gate and switches to ArgType::PruneFilter.
src/windows/wslc/tasks/ImageTasks.cpp Adds ConfirmPrune() gate and switches to ArgType::PruneFilter.
src/windows/wslc/tasks/ContainerTasks.cpp Adds ConfirmPrune() gate, switches to ArgType::PruneFilter, and passes filters into service prune.
src/windows/wslc/services/ContainerService.h Changes container prune API to accept filters.
src/windows/wslc/services/ContainerService.cpp Marshals filters into WSLCFilter[] for PruneContainers COM call.
src/windows/wslc/core/Terminal.h Declares Terminal::Confirm().
src/windows/wslc/core/Terminal.cpp Implements Terminal::Confirm() using PromptForLine + ASCII trim + case-insensitive y.
src/windows/wslc/core/CLIExecutionContext.h Declares CLIExecutionContext::ConfirmPrune().
src/windows/wslc/core/CLIExecutionContext.cpp Implements prune confirmation logic with --force bypass.
src/windows/wslc/commands/VolumePruneCommand.cpp Registers PruneFilter (no alias) + Force with prune-specific descriptions.
src/windows/wslc/commands/NetworkPruneCommand.cpp Registers PruneFilter (no alias) + Force.
src/windows/wslc/commands/ImagePruneCommand.cpp Registers PruneFilter (no alias) + Force.
src/windows/wslc/commands/ContainerPruneCommand.cpp Registers PruneFilter (no alias) + Force.
src/windows/wslc/arguments/SpecParsing.h Declares ParsePruneFilter().
src/windows/wslc/arguments/SpecParsing.cpp Implements ParsePruneFilter() with prune-specific invalid-format error string.
src/windows/wslc/arguments/ArgumentValidation.cpp Adds validation/caching path for ArgType::PruneFilter.
src/windows/wslc/arguments/ArgumentDefinitions.h Adds ArgType::PruneFilter definition (no alias) and documents prune alias semantics.
localization/strings/en-US/Resources.resw Adds new localized strings for prune warnings, prompt, prune force description, and prune-filter error.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/windows/wslc/core/Terminal.cpp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 21:52
@ggarzia-MSFT
ggarzia-MSFT marked this pull request as ready for review August 26, 2026 21:53
@ggarzia-MSFT
ggarzia-MSFT requested review from a team as code owners August 26, 2026 21:53

Copilot AI 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.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Comment thread src/windows/wslc/arguments/SpecParsing.h Outdated
Comment thread src/windows/wslc/core/CLIExecutionContext.cpp Outdated
Comment thread src/windows/wslc/core/CLIExecutionContext.h Outdated
Comment thread src/windows/wslc/core/Terminal.cpp
Comment thread src/windows/wslc/tasks/ContainerTasks.cpp Outdated
Comment thread test/windows/wslc/WSLCCLITerminalUnitTests.cpp Outdated
…t prompt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 22:54

Copilot AI 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.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 20:25

Copilot AI 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.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment thread src/windows/wslc/tasks/CommonTasks.h
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 20:38
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/windows/wslc/tasks/CommonTasks.cpp:34

  • ConfirmAction asserts Data::ConfirmMessage before checking --force. That makes --force still require ConfirmMessage to be present and can WI_ASSERT/terminate even though the prompt is skipped. Swapping the order keeps current behavior but makes the helper safe for future callers that skip prompting via --force.
    WI_ASSERT(context.Data.Contains(Data::ConfirmMessage));

    if (context.Args.GetValue<ArgType::Force>())

Copilot AI review requested due to automatic review settings August 27, 2026 20:43

Copilot AI 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.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 01:27

Copilot AI 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.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 18:11

Copilot AI 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.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/windows/wslc/tasks/CommonTasks.h:21

  • CommonTasks.h introduces a global using wsl::windows::wslc::execution::CLIExecutionContext; in a header. Other task headers in this directory fully-qualify CLIExecutionContext (e.g., VolumeTasks.h), and keeping using out of headers avoids namespace pollution and accidental symbol collisions for downstream includes.
using wsl::windows::wslc::execution::CLIExecutionContext;

namespace wsl::windows::wslc::task {
void ConfirmAction(CLIExecutionContext& context);
} // namespace wsl::windows::wslc::task

…rune-docker-parity

# Conflicts:
#	test/windows/wslc/WSLCCLICommandUnitTests.cpp

Copilot AI 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.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Comment thread localization/strings/en-US/Resources.resw
ggarzia-MSFT and others added 2 commits September 1, 2026 13:14
…ilter alias override

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 2, 2026 01:27

Copilot AI 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.

🟡 Changes recommended

The PR introduces a developer-specific script with hardcoded local paths (should be removed) and still has a user-facing filter error string mismatch with -f aliasing that needs follow-up for clarity.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/windows/wslc/tasks/CommonTasks.h:21

  • Avoid using declarations at global scope in headers (they leak into all includers and can cause name collisions). Prefer fully-qualifying the parameter type or placing any alias inside the wsl::windows::wslc::task namespace.
#include "CLIExecutionContext.h"

using wsl::windows::wslc::execution::CLIExecutionContext;

namespace wsl::windows::wslc::task {
void ConfirmAction(CLIExecutionContext& context);
} // namespace wsl::windows::wslc::task

localization/strings/en-US/Resources.resw:3043

  • WSLCCLI_InvalidFilterError now mentions only "--filter", but ArgType::Filter still has the "-f" alias on list commands (ArgumentDefinitions.h defines Filter alias as "f"). Because ParseFilter() throws WSLCCLI_InvalidFilterError without knowing which spelling the user used, wslc ... -f <bad> errors will now point users only at "--filter", while prune commands intentionally repurpose "-f" for "--force".

Consider splitting this into two localized messages (e.g., one that mentions "-f, --filter" for list commands and another that mentions only "--filter" for prune), and selecting the right one based on the specific argument instance/type at parse/validation time (this likely requires passing the arg name/alias into ParseFilter or adding a PruneFilter arg type as described in the PR description).

  <data name="WSLCCLI_InvalidFilterError" xml:space="preserve">
    <value>Invalid value "{}" for the '--filter' option: bad format of filter (expected name=value)</value>
    <comment>{FixedPlaceholder="{}"}{Locked="--filter'"}Command line arguments, file names and string inserts should not be translated</comment>
  </data>
  • Files reviewed: 28/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread fix-and-build.ps1
Comment on lines +1 to +6
$ErrorActionPreference = 'Continue'
$repo = 'C:\Users\gavingarzia\repos\WSL'
$logDir = 'C:\Users\gavingarzia\.copilot\session-state\5e1713ed-368a-4854-8fd8-38a8fa7216bf\files\buildlogs2'
New-Item -ItemType Directory -Force -Path $logDir | Out-Null
Set-Location $repo

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.

3 participants