Skip to content

CLI: Added container prune#40306

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

CLI: Added container prune#40306
AmelBawa-msft wants to merge 2 commits intofeature/wsl-for-appsfrom
user/amelbawa/container-prune

Conversation

@AmelBawa-msft
Copy link
Copy Markdown

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

Summary of the Pull Request

  • Added wslc container prune command
  • E2E tests covering no-op prune, pruning stopped containers, verifying running containers are preserved, 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:07
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 a new wslc container prune command to remove stopped containers and reports reclaimed space, with accompanying localization strings and E2E coverage.

Changes:

  • Registers a new container prune subcommand and wires it into task/service layers.
  • Implements prune logic in ContainerService and output formatting in ContainerTasks.
  • Adds E2E tests and new localized strings for prune output/help text.

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/WSLCE2EContainerTests.cpp Adds prune to the container command list verification.
test/windows/wslc/e2e/WSLCE2EContainerPruneTests.cpp New E2E test suite validating prune help and behavior.
src/windows/wslc/tasks/ContainerTasks.h Exposes PruneContainers task entrypoint.
src/windows/wslc/tasks/ContainerTasks.cpp Implements CLI output for deleted containers and reclaimed space.
src/windows/wslc/services/ContainerService.h Exposes ContainerService::Prune.
src/windows/wslc/services/ContainerService.cpp Calls session prune API and maps results into a model.
src/windows/wslc/services/ContainerModel.h Introduces PruneContainersResult model.
src/windows/wslc/commands/ContainerPruneCommand.cpp New command implementation wiring session creation + prune task.
src/windows/wslc/commands/ContainerCommand.h Declares ContainerPruneCommand.
src/windows/wslc/commands/ContainerCommand.cpp Registers ContainerPruneCommand under container.
localization/strings/en-US/Resources.resw Adds localized strings for prune descriptions and output.

Comment thread src/windows/wslc/tasks/ContainerTasks.cpp
Comment on lines +496 to +506
models::PruneContainersResult ContainerService::Prune(models::Session& session)
{
WSLCPruneContainersResults results{};
THROW_IF_FAILED(session.Get()->PruneContainers(nullptr, 0, 0, &results));

models::PruneContainersResult result;
result.SpaceReclaimed = results.SpaceReclaimed;
for (ULONG i = 0; i < results.ContainersCount; ++i)
{
result.DeletedContainers.push_back(results.Containers[i]);
}
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.

This assumes results.Containers is valid and populated after calling PruneContainers(nullptr, 0, 0, &results), but no buffer is allocated/provided for container IDs. If the underlying API expects a caller-provided buffer (common for COM-style “get required size then allocate then call again”), this will read from an uninitialized pointer and/or produce incomplete results. If the API allocates memory, there’s also no ownership/cleanup shown, which risks leaking. Please update this to follow the API’s documented allocation pattern (e.g., two-pass call to obtain required size then allocate, or wrap returned memory in an owning RAII type and free it appropriately) and only iterate when the container list pointer is valid for the duration needed.

Copilot uses AI. Check for mistakes.
const auto result = RunWslc(L"container prune");
result.Verify({.Stderr = L"", .ExitCode = 0});

VerifyStdoutContains(result, L"Total reclaimed space:");
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 test asserts English output directly. Since prune output strings are now localized (and this test already uses Localization::... for help text), this can become brittle if tests run under a non-en-US locale or if the localized string changes. Prefer asserting using the localized resource (or a helper that derives the expected prefix from Localization::WSLCCLI_ContainerPruneSpaceReclaimed(...)) rather than hardcoding L"Total reclaimed space:".

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

benhillis commented Apr 25, 2026

PruneContainers allocates results. Containers via COM task memory but the caller never frees it. ImageService::Prune handles this correctly with wil::unique_cotaskmem_array_ptr — same pattern should be used here.

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