Skip to content

CLI: Initialize volume prune functionality#40307

Draft
AmelBawa-msft wants to merge 2 commits intofeature/wsl-for-appsfrom
user/amelbawa/volume-prune
Draft

CLI: Initialize volume prune functionality#40307
AmelBawa-msft wants to merge 2 commits intofeature/wsl-for-appsfrom
user/amelbawa/volume-prune

Conversation

@AmelBawa-msft
Copy link
Copy Markdown

@AmelBawa-msft AmelBawa-msft commented Apr 24, 2026

Summary of the Pull Request

  • Added wslc volume prune command
  • Volumes in use are preserved
  • E2E tests covering no-op prune, --all with single/multiple volumes, in-use volume protection, and idempotent re-prune.

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

Validation Steps Performed

Copilot AI review requested due to automatic review settings April 24, 2026 17:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds initial CLI support for wslc volume prune, including command wiring, service/task plumbing, localization strings, and end-to-end test coverage for core prune scenarios.

Changes:

  • Register new volume prune subcommand and wire it through command → task → service layers.
  • Add localization strings for prune help/output and include prune in volume help listing.
  • Add new E2E tests covering prune help, --all, multiple volumes, in-use volume behavior, and idempotency.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/windows/wslc/e2e/WSLCE2EVolumeTests.cpp Adds prune to the list of expected volume subcommands in help.
test/windows/wslc/e2e/WSLCE2EVolumePruneTests.cpp New E2E test suite for wslc volume prune.
src/windows/wslc/tasks/VolumeTasks.h Declares new PruneVolumes task entry point.
src/windows/wslc/tasks/VolumeTasks.cpp Implements prune task and prints deleted volumes + reclaimed space.
src/windows/wslc/services/VolumeService.h Adds VolumeService::Prune API.
src/windows/wslc/services/VolumeService.cpp Implements service call into session PruneVolumes.
src/windows/wslc/services/VolumeModel.h Adds PruneVolumesResult model for deleted volumes + reclaimed space.
src/windows/wslc/commands/VolumePruneCommand.cpp New command implementation for volume prune.
src/windows/wslc/commands/VolumeCommand.h Declares VolumePruneCommand.
src/windows/wslc/commands/VolumeCommand.cpp Registers VolumePruneCommand under volume.
localization/strings/en-US/Resources.resw Adds user-facing strings for prune descriptions, output, and args.

Comment on lines +66 to +80
WSLC_TEST_METHOD(WSLCE2E_Volume_Prune_AllFlag)
{
// Create a named volume, then prune with --all
RunWslc(std::format(L"volume create --name {}", TestVolumeName1)).Verify({.Stderr = L"", .ExitCode = 0});
VerifyVolumeIsListed(TestVolumeName1);

const auto result = RunWslc(L"volume prune --all");
result.Verify({.Stderr = L"", .ExitCode = 0});

VerifyStdoutContains(result, L"Deleted:");
VerifyStdoutContains(result, L"Total reclaimed space:");

// Verify the volume was actually removed
VerifyVolumeIsNotListed(TestVolumeName1);
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The E2E suite covers volume prune --all but does not cover the default behavior of volume prune without --all (i.e., that named volumes should not be removed when --all is omitted). Adding a test that creates a named volume, runs volume prune (no flags), and verifies the volume remains would exercise the non---all path and prevent regressions in default semantics.

Copilot uses AI. Check for mistakes.
PrintMessage(Localization::WSLCCLI_VolumePruneDeleted(volume));
}

PrintMessage(L"");
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

A blank line is always printed even when no volumes are deleted. This creates avoidable output churn and makes CLI output harder to parse consistently. Consider only printing the separator line when result.DeletedVolumes is non-empty (or otherwise aligning output formatting with the rest of the CLI’s conventions for empty results).

Suggested change
PrintMessage(L"");
if (!result.DeletedVolumes.empty())
{
PrintMessage(L"");
}

Copilot uses AI. Check for mistakes.
std::vector<Argument> VolumePruneCommand::GetArguments() const
{
return {
Argument::Create(ArgType::All, std::nullopt, std::nullopt, Localization::WSLCCLI_VolumePruneAllArgDescription()),
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The help text in the new E2E test expects -a,--all, but the argument registration passes std::nullopt for both names. If ArgType::All does not implicitly define -a/--all in this codebase, the command will not expose the intended flags and help output will diverge. To make the CLI contract explicit, define the short/long forms directly here (or adjust the help expectations to match the actual registered flags).

Suggested change
Argument::Create(ArgType::All, std::nullopt, std::nullopt, Localization::WSLCCLI_VolumePruneAllArgDescription()),
Argument::Create(ArgType::All, L"a", L"all", Localization::WSLCCLI_VolumePruneAllArgDescription()),

Copilot uses AI. Check for mistakes.
@benhillis
Copy link
Copy Markdown
Member

benhillis commented Apr 25, 2026

WSLCSession::PruneVolumes returns E_NOTIMPL. The CLI is wired up but the backend is a stub, so this will always throw. Hold this until the actual prune logic exists.

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