Skip to content

wslc: add --digests to image list for docker parity - #41457

Merged
ggarzia-MSFT merged 26 commits into
masterfrom
user/ggarzia/wslc-images-digests
Sep 17, 2026
Merged

ggarzia-MSFT merged 26 commits into
masterfrom
user/ggarzia/wslc-images-digests

Conversation

@ggarzia-MSFT

@ggarzia-MSFT ggarzia-MSFT commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Adds --digests to wslc image list (and its image ls / images spellings), matching docker images --digests. The DIGEST column was previously hardcoded to <none> in json output and absent from the table.

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

WSLCListImagesFlagsDigests was already defined and already honored end to end — WSLCSession::ListImages forwards it to Docker().ListImages(all, digests, filters) and populates WSLCImageInformation::Digest. The CLI never set the flag and never read the field, so ToImageOutput hardcoded Digest to <none>.

  • New ArgType::Digests (--digests, no short alias, matching docker where only listing commands' --filter carries -f).
  • models::ImageInformation gains a Digest field, and ImageService::List takes a defaulted bool digests that sets the flag and populates it.
  • Table output adds a DIGEST column between TAG and IMAGE ID only when --digests is passed, matching docker's defaultImageTableFormatWithDigest.
  • The service reports a repo digest (repo@sha256:...); docker's DIGEST column shows only the digest. New DigestFromRepoDigest helper in ImageModel.h performs that reduction and is unit tested directly.
  • The digest is only populated when the flag is set. This matters for json: docker gates on needDigest(ctx), which is ctx.Digest || format.Contains("{{.Digest}}"), and --format json satisfies neither. So image list --format json still reports <none>, and only --digests --format json reports a real digest. An earlier revision of this change leaked the digest into plain json output; the tests below lock the gated behavior in.
  • Column configs were hoisted into named constexpr values so the 5- and 6-column tables cannot drift.

Validation Steps Performed

All new and existing tests in the touched areas were run against a locally deployed build. 25/25 pass.

Unit tests (WSLCCLIImageDigestUnitTests, new file, 3 tests):

  • DigestFromRepoDigest_StripsRepositoryPrefix — plain, fully qualified, and registry-with-port repositories all reduce to the bare sha256: digest. The port case guards the split from being confused by a colon.
  • DigestFromRepoDigest_PassesThroughBareValues — empty, already-bare, and <none> values are untouched.
  • DigestFromRepoDigest_SplitsOnFirstSeparator — splitting happens once, so a digest is never truncated further.

Unit test (WSLCCLICommandUnitTests):

  • ImageListCommand_HasDigestsArgument — both the image list and root-scoped images constructions register --digests as an optional flag with no short alias.

E2E tests (WSLCE2EImageListTests, 22 total, 5 new):

  • Digests_AddsColumnBetweenTagAndImageId — asserts DIGEST sits after TAG and before IMAGE ID, and that the default listing does not gain the column.
  • Digests_ReportsDigestOnlyWhenRequested — json reports <none> without the flag; anything reported with the flag must be a bare sha256: digest with no repo@ prefix.
  • Digests_TableMatchesJson — every digest in json output appears in the table's DIGEST column, so the two renderings cannot drift.
  • Digests_QuietStillOutputsIdsOnly — --quiet wins over --digests, emitting bare ids with no header, as docker does.
  • Digests_ListedInHelp — --digests and its localized description appear in image list --help.

Note on the test images: they are provisioned via image load from a tarball, so they carry no repo digest and correctly report <none> even with --digests — the same as docker for a tar-loaded image. The digest-reduction logic is therefore covered by the unit tests rather than by asserting a live registry digest in e2e, which would make the suite network dependent.

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

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 adds --digests support to wslc image list to match Docker parity, wiring the flag through CLI argument parsing into the image listing service and updating both table and JSON render paths to only surface digests when explicitly requested.

Changes:

  • Added a new --digests flag (no short alias) for image list / image ls / images, and threaded it through ImageService::List.
  • Implemented digest formatting by reducing service “repo@sha256:…” values to Docker’s “sha256:…” form via DigestFromRepoDigest, and conditionally added a DIGEST column in table output.
  • Added unit + E2E tests to lock the gating behavior (digests only appear when --digests is passed) and validate output parity between JSON and table formats.

Reviewed changes

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

Show a summary per file
File Description
test/windows/wslc/WSLCCLIImageDigestUnitTests.cpp New unit tests for DigestFromRepoDigest behavior.
test/windows/wslc/WSLCCLICommandUnitTests.cpp Verifies image list/images register --digests with no short alias.
test/windows/wslc/e2e/WSLCE2EImageListTests.cpp Adds E2E coverage for DIGEST column placement, gating, help output, and quiet behavior.
src/windows/wslc/tasks/ImageTasks.cpp Threads --digests into listing and conditionally adds DIGEST column + JSON field population.
src/windows/wslc/services/ImageService.h Extends ImageService::List signature with a digests parameter (defaulted).
src/windows/wslc/services/ImageService.cpp Sets the WSLC digest flag and populates ImageInformation::Digest only when requested.
src/windows/wslc/services/ImageModel.h Adds Digest field to the image model and introduces DigestFromRepoDigest.
src/windows/wslc/commands/ImageListCommand.cpp Registers the new ArgType::Digests argument for the list command.
src/windows/wslc/arguments/ArgumentDefinitions.h Defines the Digests argument metadata and localization key mapping.
localization/strings/en-US/Resources.resw Adds localized description string for --digests.

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

Comment thread localization/strings/en-US/Resources.resw
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 22:57

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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

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

  • Most CLI argument descriptions in Resources.resw do not end with a period; this new string does, which makes help output inconsistent.
    <value>Show image digests.</value>
Comment thread src/windows/wslc/tasks/ImageTasks.cpp

@dkbennett David Bennett (dkbennett) 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.

--quiet and --digests are valid options together and Docker CLI allows both. --quiet means only image ids are output, but --digests means that an image with multiple digests can appear multiple times in the output (same is true for tags)

Should add tests here which verify the --quiet and --digests combination (and also images with multiple tags should appear multiple times with --quiet).

This may be a pre-existing issue with quiet that could be addressed here.

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

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 20 out of 20 changed files in this pull request and generated no new comments.

ggarzia-MSFT and others added 2 commits August 27, 2026 13:38
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…olumn

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

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 12 out of 12 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

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

  • Most CLI argument descriptions in Resources.resw omit trailing punctuation (e.g., "Run container in detached mode"). For consistency, drop the trailing period from this new description.
  <data name="WSLCCLI_DigestsArgDescription" xml:space="preserve">
    <value>Show image digests.</value>
  </data>
Comment thread src/windows/wslc/tasks/ImageTasks.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EImageListTests.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 21:13

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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

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

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:495

  • This assertion is logically correct, but it reads as a double-negative ("IS_FALSE(find != npos)"). Using an explicit npos comparison makes failures easier to interpret and avoids precedence/clarity pitfalls in future edits.
        VERIFY_IS_FALSE(defaultResult.GetStdoutLines()[0].find(L"DIGEST") != std::wstring::npos);

src/windows/wslcsession/WSLCSession.cpp:1958

  • When WSLCListImagesFlagsDigests is not set, this branch still emits a digest value (it->second.front()) if RepoDigests are present. Since the flag is meant to gate whether digest info is included at all, it would be safer to always emit an empty digest when digests==false (even if the backend unexpectedly provides RepoDigests).
            else if (!digests)
            {
                rows.push_back({&e, tag, it->second.front()});
            }
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 21:38

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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

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

test/windows/wslc/WSLCCLITableOutputUnitTests.cpp:274

  • In this test, namePos/statusPos are used for arithmetic without first verifying the substrings were found. If either find() returns npos, the expected-position math can overflow and the assertion may become misleading.
        const auto namePos = dataLine.find(L"container-a");
        const auto statusPos = dataLine.find(L"running");
        const auto expected = namePos + wcslen(L"container-a") + TableOutput<3>::DefaultColumnPadding + wcslen(L"DIGEST") +
                              TableOutput<3>::DefaultColumnPadding;
        VERIFY_ARE_EQUAL(expected, statusPos);

test/windows/wslc/WSLCCLITableOutputUnitTests.cpp:306

  • This assertion uses namePos in arithmetic without checking that "container-a" (and "running") were actually found. If either find() returns npos, the computed offset can overflow and hide formatting regressions.
        const auto& dataLine = cap.lines()[1];
        const auto namePos = dataLine.find(L"container-a");
        VERIFY_ARE_EQUAL(namePos + wcslen(L"container-a") + TableOutput<3>::DefaultColumnPadding, dataLine.find(L"running"));

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

  • This new argument description includes a trailing period, which is inconsistent with nearby argument descriptions (e.g., "Run container in detached mode") and will show up in --help output.
  <data name="WSLCCLI_DigestsArgDescription" xml:space="preserve">
    <value>Show image digests.</value>
  </data>
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 01:07

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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:549

  • This assertion only checks whether the digest text appears anywhere in the table output. That can pass even if the value appears in a different column (or in multiple places), so it doesn't actually verify the DIGEST column as the comment claims. Consider extracting the DIGEST column slice using the header offsets and comparing against that field.
        // Every digest reported by json output must appear in the table's DIGEST column, so the two
        // renderings cannot drift.
        for (const auto& image : ParseNdjsonOutputAs<ImageOutputInformation>(jsonResult))
        {
            const auto digest = wsl::shared::string::MultiByteToWide(image.Digest);
            VERIFY_IS_TRUE(
                tableResult.StdoutContainsSubstring(digest),
                WEX::Common::String().Format(L"'%ls' was missing from the table DIGEST column", digest.c_str()));
        }
Comment thread localization/strings/en-US/Resources.resw

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

Four unresolved moderate issues remain around digest association, bare digest handling, and deterministic coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/windows/wslcsession/WSLCSession.cpp:1933

  • The repository Name preserves the daemon's spelling, so this key does not match equivalent references such as a tag ubuntu:latest and a digest docker.io/library/ubuntu@sha256:... (or index.docker.io/...). That leaves the tag row with <none> and emits a separate digest-only row instead of associating the digest with the tag; use the normalized repository identity consistently for this map and the tag lookup/set (for example, Repository.GetCanonical()).
            digestsByRepo[reference->Repository.Name].push_back(repoDigest);

src/windows/wslcsession/WSLCSession.cpp:1962

  • The added end-to-end cases use tar-loaded images, whose RepoDigests are empty, so this per-digest row path is never exercised; the unit tests only cover stripping a repository prefix. A regression in daemon digest parsing, tag/repository association, or multiple-digest expansion would therefore pass. Add deterministic coverage with a fixture or mock response containing at least one valid repo digest (and preferably two).
                for (const auto& repoDigest : it->second)
                {
                    rows.push_back({&e, tag, repoDigest});
                }

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:558

  • These E2E images are explicitly tar-loaded and have no repository digests, so this test never exercises the new RepoDigests loop that emits one row per digest. It only checks properties of the existing <none> rows; add deterministic service-level/fixture coverage with multiple repo digests before treating this as coverage for the new expansion behavior.
        auto result = RunWslc(L"image list --digests --format json");
        result.Verify({.Stderr = L"", .ExitCode = 0});

        const auto digestRows = ParseNdjsonOutputAs<ImageOutputInformation>(result);

  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite
Comment thread src/windows/wslcsession/WSLCSession.cpp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 18:51

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.

🔵 Needs a closer look

Repository-name canonicalization can misassociate digests, and deterministic coverage for real and multiple repository digests is still missing.

Review details

Suppressed comments (3)

src/windows/wslc/services/ImageService.cpp:334

  • The new end-to-end cases use tar-loaded images, so image.Digest remains empty and this branch is never exercised with a real repo digest; the helper unit tests only validate string stripping. That leaves the new service/session wiring and row grouping unverified. Add deterministic coverage using the existing local-registry setup or a fixture that supplies at least one non-empty RepoDigests value, including the tagged-row mapping.
        if (digests)
        {
            info.Digest = DigestFromRepoDigest(image.Digest);
        }

src/windows/wslcsession/WSLCSession.cpp:1950

  • This lookup uses Repository.Name, which preserves the daemon's original spelling, so equivalent references such as ubuntu:latest and docker.io/library/ubuntu@sha256:... are treated as different repositories. In that case the digest is not attached to the tagged row and an extra digest-only row is emitted; key the grouping and tagged-repository set by Repository.GetCanonical() while retaining the original spelling for display.
            auto repoName = reference->Repository.Name;
            const auto it = digestsByRepo.find(repoName);
            taggedRepos.emplace(std::move(repoName));

src/windows/wslcsession/WSLCSession.cpp:1962

  • The new per-digest expansion is not exercised by the added E2E tests: their tar-loaded images explicitly have no RepoDigests, so this loop is never non-empty; the existing native test only checks that a digest can exist, not that multiple rows are emitted. Add a deterministic fixture with at least two RepoDigests for one tagged repository to validate this loop and the tag/digest grouping.
                for (const auto& repoDigest : it->second)
                {
                    rows.push_back({&e, tag, repoDigest});
                }
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 17:38

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

Moderate issues remain around digest-row coverage and matching, default listing semantics, and dangling-image test-key uniqueness.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/windows/wslcsession/WSLCSession.cpp:1981

  • Repository.Name intentionally preserves the original spelling rather than the normalized identity. Keying this map by Name can fail to associate valid equivalent references such as ubuntu:latest and docker.io/library/ubuntu@sha256:..., producing an undigested tag row plus an extra digest-only row. Use a canonical key for matching while retaining the original repository name for display.
            digestsByRepo[reference->Repository.Name].push_back(repoDigest);

src/windows/wslcsession/WSLCSession.cpp:2025

  • When RepoTags is empty but the daemon still returns RepoDigests (for example, a pulled image after its last tag was removed), the normal listing now emits repoName with a <none> tag instead of the existing <none>:<none> dangling-image row. This changes default image-list semantics even though --digests was not requested; skip these digest-only rows when digests is false and let the fallback below create the unnamed row.
            if (!digests)
            {
                rows.push_back({&e, repoName, std::string{}});

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:570

  • <none> is not a unique repository/tag key: the service emits <none>:<none> for every dangling image (WSLCTests.cpp:1167-1171). If the test environment contains two dangling images, the first emplace ID is reused and this test fails even though the digest rows are valid; skip placeholder rows or include the image ID in the uniqueness key.
            const auto it = idByRepoTag.emplace(std::make_pair(image.Repository, image.Tag), image.ID).first;
            VERIFY_ARE_EQUAL(it->second, image.ID, L"Rows sharing a repository and tag must report the same image ID");

            VERIFY_IS_TRUE(
                seen.emplace(image.Repository, image.Tag, image.Digest).second,
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Lite
Comment thread src/windows/wslcsession/WSLCSession.cpp
…rtions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 16, 2026 18:07
@ggarzia-MSFT
ggarzia-MSFT requested a review from a team as a code owner September 16, 2026 18:07

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.

🔵 Needs a closer look

Three moderate issues remain regarding repository grouping, default digest-only rows, and non-empty digest test coverage.

Review details

Suppressed comments (3)

src/windows/wslcsession/WSLCSession.cpp:1981

  • This groups by Repository.Name, which deliberately preserves the input spelling rather than the normalized repository identity. If Docker returns a short RepoTag such as ubuntu:latest and a fully qualified RepoDigest such as docker.io/library/ubuntu@sha256:..., the lookup at line 1997 misses it: the tagged row gets <none> and a separate digest-only row is emitted. Key the tag/digest groups by Repository.GetCanonical() while retaining an original name separately for display.
            digestsByRepo[reference->Repository.Name].push_back(repoDigest);

src/windows/wslcsession/WSLCSession.cpp:2025

  • When digests is false, this branch still emits a row for every repository found only in RepoDigests, using a bare repository name. The previous behavior emitted <none>:<none> whenever RepoTags was empty, and the CLI relies on that sentinel, so a digest-only image can change from a dangling row to repo/<none> in the default listing. Skip digest-only repositories when !digests and let the fallback below create the unnamed row.
            if (!digests)
            {
                rows.push_back({&e, repoName, std::string{}});

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:567

  • All of these e2e cases use tar-loaded images with no RepoDigests, so none exercises the non-empty digest path: service field population, repo@sha256 reduction, or per-digest row expansion. The tests would still pass if those new branches were broken; add a deterministic local-registry fixture or a direct service/session fixture containing a real digest.
            // The fixtures carry no repo digest, so every row reports the placeholder.
            VERIFY_ARE_EQUAL(std::string{c_none}, image.Digest);
  • Files reviewed: 15/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 16, 2026 20:34

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.

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (3)

src/windows/wslcsession/WSLCSession.cpp:1981

  • These lookup keys use Repository.Name, which preserves the daemon's original token. If RepoTags contains ubuntu:latest while RepoDigests contains docker.io/library/ubuntu@sha256:..., the lookup at lines 1996-1997 misses, so --digests prints <none> for the tagged row and an extra digest-only row. Key both collections by a normalized repository name while retaining the appropriate familiar/original name for display, and cover mixed qualification with a test.
            digestsByRepo[reference->Repository.Name].push_back(repoDigest);

src/windows/wslcsession/WSLCSession.cpp:2009

  • This expands one tagged repository/tag into one output row per RepoDigests entry. ImageService::List and both formatters consume each WSLCImageInformation as a row, so an image carrying two digests for the same repository will duplicate the REPOSITORY/TAG/IMAGE ID rows. Docker's images --digests keeps one row per tag and puts the digest in that row; preserve the one-row-per-tag behavior and select the digest to display instead of expanding the row set.
                for (const auto& repoDigest : it->second)
                {
                    rows.push_back({&e, tag, repoDigest});

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:540

  • This assertion searches the entire table output, but every fixture digest is <none> and that placeholder can already occur in REPOSITORY or TAG. The test can therefore pass even if the DIGEST cells are missing or empty, leaving the real sha256 table/json consistency path untested; assert the value within the DIGEST column (or add a pulled-image table case).
            const auto digest = wsl::shared::string::MultiByteToWide(image.Digest);
            VERIFY_IS_TRUE(
                tableResult.StdoutContainsSubstring(digest),
  • Files reviewed: 15/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
Copilot AI review requested due to automatic review settings September 16, 2026 22:40

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

A critical row-cardinality regression remains, and multi-digest E2E coverage is incomplete.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:554

  • This test claims to cover a tag with several digests, but it uses the tar-loaded fixtures and then requires every Digest to be <none> at lines 566-567. It therefore never exercises the multi-digest path added in WSLCSession.cpp:2007-2010; an implementation that ignored or collapsed additional RepoDigests would still pass. Add a repository/tag state with at least two real repo digests and assert the expected rows.
        // A tag carrying several digests is emitted once per digest, so
        // rows stay unique per repository, tag and digest while the image ID repeats across them.
        std::map<std::pair<std::string, std::string>, std::string> idByRepoTag;
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Lite
Comment thread src/windows/wslcsession/WSLCSession.cpp
Copilot AI review requested due to automatic review settings September 16, 2026 23:02

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.

🔵 Needs a closer look

Three moderate digest-grouping issues remain unresolved, along with one test-coverage nit.

Review details

Suppressed comments (4)

src/windows/wslc/tasks/ImageTasks.cpp:260

  • The real-digest registry test validates only JSON; the table tests use tar-loaded images whose DIGEST is always <none>. A regression in the non-empty digest table cell would therefore pass. Extend the registry scenario to run image list --digests and assert each pulled digest appears in the DIGEST column.
            table.WriteRow({
                MultiByteToWide(entry.Repository),
                MultiByteToWide(entry.Tag),
                digests ? MultiByteToWide(entry.Digest) : std::wstring{},
                MultiByteToWide(entry.ID),

src/windows/wslcsession/WSLCSession.cpp:2020

  • This digest-only repository loop runs even when digests is false: the !digests branch appends a row for every RepoDigests entry. For a tagless image whose daemon response includes repo digests, the default listing therefore changes from the previous <none>:<none> row to repository-only rows, even though digest references were not requested. Only enumerate these repositories when digests is true so the fallback below preserves the no-flag behavior.
        for (const auto& [repoName, repoDigests] : digestsByRepo)
        {
            if (taggedRepos.contains(repoName))
            {
                continue;

src/windows/wslcsession/WSLCSession.cpp:1981

  • This correlation is keyed by Repository.Name, but that field intentionally preserves the original spelling while RepositoryReference::Parse normalizes equivalent names into Server/Path (for example, ubuntu and docker.io/library/ubuntu). If the daemon uses those valid spellings across RepoTags and RepoDigests, the lookup misses, leaving the tagged row at <none> and emitting an extra digest-only row. Use a canonical Server/Path key for matching while retaining the original name for display.
            digestsByRepo[reference->Repository.Name].push_back(repoDigest);

src/windows/wslcsession/WSLCSession.cpp:2010

  • The new multi-digest expansion path is not exercised by the added E2E coverage: Digests_RepeatsIdOncePerDigest uses tar-loaded images and asserts every digest is <none>, while the registry test creates only one digest per repository. Add a fixture with at least two repo digests (or a digest-only repository) and assert the resulting row/quiet multiplicity so this loop cannot regress silently.
                for (const auto& repoDigest : it->second)
                {
                    rows.push_back({&e, tag, repoDigest});
                }
  • Files reviewed: 15/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite
@ggarzia-MSFT
ggarzia-MSFT merged commit efa0584 into master Sep 17, 2026
12 checks passed
@ggarzia-MSFT
ggarzia-MSFT deleted the user/ggarzia/wslc-images-digests branch September 17, 2026 04:36
Blue (OneBlue) added a commit that referenced this pull request Sep 22, 2026
* Fix tracked routes being collapsed by incomplete comparison (#41393)

Mirrored route tracking used an incomplete comparator that considered only route class, destination address, and metric. Distinct routes with different prefix lengths or next hops could therefore be treated as equivalent and silently omitted.

The fix preserves the existing route-class ordering while using the complete EndpointRoute comparison for route identity. Regression tests cover prefix length, next hop, metric, exact duplicates, and dependency ordering.

* Add github issue suggestion in user visible error (#41432)

This PR adds a "search or file issue on github" suggestion in all user visible errors to:
Help users find solutions faster.
Collect more user reported issues to help reliability improvements.

This PR also updates all tests checking the error message to use a unified function for creating the expected error message.

* Match Docker output for prune operations and container inspection (#41430)

Cleanup various miscellaneous output divergences

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* CLI: Mount PR followup & fix two docker parser bugs (#41436)

* Invalid the TestImageRegistry cache after running prune --all (#41442)

* Fix unit test build failure on arm64 (#41437)

* Solve various issues found by verifier (#41445)

* Save state

* Save state

* Save state

* Cleanup diff

* Add a command line option to run the tests under verifier  (#41440)

* Save state

* Save state

* Save state

* Add a /verifier option to run-tests.ps1 to run the test under verifier

* Localization change from build: 155859879 (#41450)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Fix various arm64 test failures (#41444)

* Fix various arm64 test failures

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix LF

* Format

* Cleanup diff

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix unity build ODR collisions on duplicate file-local constants (#41446)

* CLI: Align alias listing with docker, Apple and other CLIs (#41439)

* Align container list format with Docker specifications (#41375)

wslc: match column order, status text, and json shape for container list


Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Localization change from build: 156161979 (#41479)

Co-authored-by: WSL localization <noreply@microsoft.com>

* wslc: alias -f to --format on inspect commands for docker parity (#41463)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* CLI: Add explicit per-command argument overrides (#41478)

* archlinux: Release 2026.09.01.176721 (#41493)

This is an automated release [1].

[1] https://gitlab.archlinux.org/archlinux/archlinux-wsl/-/blob/main/.gitlab-ci.yml

* Fix WSLC parser unit test argument overrides (#41496)

Update the remaining parser test call site to use ArgumentOverrides after the Argument::Create API change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: eb7946ff-cc0b-4ff1-a059-571334b1c988

* Fix systemd-tmpfiles failure on systemd v261+ with systemd boot disabled (#41491)

In systemd v261, the systemd's tmpfiles.d/x11.conf was changed from D! to D. systemd/systemd@5474cad.
This means the systemd-tmpfiles command will try to execute it when called out of the boot sequence. In the linked issue's case, it's called by the post install script by dpkg. This will fail as the wsl override is not in place when systemd is disabled.

This PR enables the wsl override file generation for all distros with wslg enabled, regardless of if the distro boots with systemd.

* Fix unvalidated TerminalProfileSize during distribution import (#41495)

* Fix unvalidated TerminalProfileSize when importing a distribution

_ProcessImportResultMessage constructed the terminal profile string_view
using the message-supplied TerminalProfileSize without validating it
against the received buffer length. Use the bounds-checked two-argument
span::subspan() overload (matching the existing ShortcutIconSize handling
a few lines above) so an inconsistent size value throws instead of
producing a string_view that runs past the end of the buffer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 35281c30-3d08-4f05-8c84-2ce4711023d5

* format source

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 35281c30-3d08-4f05-8c84-2ce4711023d5

* Bump WSL DeviceHost to 1.2.62 (#41499)

* Bump WSL DeviceHost to 1.2.62

Co-authored-by: damanm24 <9593793+damanm24@users.noreply.github.com>

* Correct WSL DeviceHost version to 1.2.62-0

Co-authored-by: damanm24 <9593793+damanm24@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: damanm24 <9593793+damanm24@users.noreply.github.com>

* Enable unity build repo-wide via WSL_UNITY_BATCH_SIZE (#41441)

Enable unity build repo-wide via WSL_UNITY_BATCH_SIZE

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add wslc system info command (#41408)

* updated source code paths in the wslservice tab (#41509)

Co-authored-by: Tega Ajise <tegaajise@Tegas-MacBook-Pro-2.local>

* Harden Windows macros against dangling-else ambiguity (#41513)

* Harden Windows macros against dangling-else ambiguity

* Harden Windows macros against dangling-else ambiguity

* Fix dangling-else bugs in FAIL_FAST_IF and EMIT_USER_WARNING macros (#41504)

* Fix dangling-else bugs in FAIL_FAST_IF and EMIT_USER_WARNING macros  

Both macros were bare if-statements without do/while(0) guards, causing
the dangling else problem when used as a single statement under an if.

* Fix dangling-else bugs in FAIL_FAST_IF and EMIT_USER_WARNING macros

* harden EMIT_USER_WARNING macro on Windows against dangling-else

* Fix dangling-else bugs in FAIL_FAST_IF and EMIT_USER_WARNING macros

* Fix intermittent ImportDistroInvalidTar test failures (#41490)

* Fix test to be more resilient

* Make WSL1 more lenient, make WSL2 exact

* Wslc events (#40971)

* Revert "Mount plugin folders on behalf of the user owning the wsl session" (#41331) (#41515)

Temporarily reverting the identity-based plugin folder mount change
(both the WslCoreVm.cpp behavior change and the accompanying
MountFolderAccess test coverage) introduced in #41331.

This reverts commit 78b9cf2.

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add container restart runtime support (#41454)

* Fix p9 drvfs read only mount regression (#41487)

#41129 introduces a regression where the ";ro" option is passed to the host for p9 shares. However, that option is not supported by the p9 server and causes the mount to fail.

This PR removes the special handling of "ro" in the common parser. And use MountParseFlags to add the required virtio option.

* Fix WSLC Plan9 mount and image-build failures (#41535)

* Fix WSLC Plan9 mounts using a per-user server

* remove debug code

* wslc: add --size to inspect for docker parity (#41489)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Bump actions/deploy-pages in the github-actions group (#41537)

Bumps the github-actions group with 1 update: [actions/deploy-pages](https://github.com/actions/deploy-pages).


Updates `actions/deploy-pages` from 5.0.0 to 5.0.1
- [Release notes](https://github.com/actions/deploy-pages/releases)
- [Commits](actions/deploy-pages@cd2ce8f...368f825)

---
updated-dependencies:
- dependency-name: actions/deploy-pages
  dependency-version: 5.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add wslc container restart command (#41435)

* init: fix dhcpcd option name in bridged-mode config (#41538)

* init: fix dhcpcd option name in bridged-mode config

dhcpcd has no option named "broadcast"; option 28 is "broadcast_address".
dhcpcd 10 rejects the whole "option" line, so DNS servers, domain, search
list, hostname and MTU were never requested from the DHCP server, leaving
/mnt/wsl/resolv.conf without nameservers on servers that honour the PRL.

* init: apply clang-format to dhcpcd config string

The longer broadcast_address option name pushed the literal past the
130-column limit in .clang-format. Split it as clang-format does; the
concatenated value is unchanged.

* Update SLE15SP7 [QU5] (#41506)

* Host plugin Plan9 shares as the session user (#41548)

* Host plugin Plan9 shares as the session user

Use a dedicated per-user Plan9 server for plugin folder mounts so host filesystem permissions are preserved without relying on HCS-managed share identity.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

* Simplify plugin Plan9 port plumbing

Use the fixed plugin port directly in mini_init and mirror the existing per-user Plan9 server lifecycle.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

* Recreate stopped plugin Plan9 servers

Recreate the per-user server before adding a share when its process is no longer running.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

* Fix virtiofs bind mounts exposing files as root-owned (#40719) (#40733)

* Fix virtiofs bind mounts exposing files as root-owned (#40719)

Add the 'metadata' option to virtiofs shares created via
HcsVirtualMachine::AddShare (the WSLC/Docker container path). Without
this option, the virtiofs device host cannot persist per-file uid/gid
in NTFS extended attributes, so all files default to uid=0/gid=0
regardless of the creating user.

This matches the behavior of the regular distro mount path
(WslCoreVm::AddVirtioFsShare) which receives metadata/uid/gid options
from the Linux init's ConvertDrvfsMountOptionsToPlan9.

Also adds a regression test (WindowsMountsVirtioFsFileOwnership) that
verifies file ownership is preserved on virtiofs mounts.

Fixes #40719

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address code review feedback

- Add inline comment explaining why 'metadata' is required
- Use unique mount point (/virtiofs-ownership-test) to avoid test interference
- Use numeric UID (su '#65534') instead of username to avoid environment dependency

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test: use 'nobody' user instead of numeric UID in virtiofs ownership test

The su command with numeric UID syntax ('#65534') requires the user to
exist in /etc/passwd. Use the 'nobody' account directly since it is
already available in the test VHD.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: wrap long test command strings to satisfy clang-format 130-col limit

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test: unmount inline with VERIFY_SUCCEEDED to match file convention

Replace the scope_exit unmount cleanup with an inline VERIFY_SUCCEEDED
call at the end of the test, matching every other mount test in this
file (e.g. WindowsMountsVirtioFsShareReuse). This also asserts the
unmount HRESULT rather than silently swallowing it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9213b7da-c5c8-4d9c-89ab-e80048d288a2

* Fix Windows mount test API calls

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Blue <OneBlue@users.noreply.github.com>
Copilot-Session: 9213b7da-c5c8-4d9c-89ab-e80048d288a2

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* wslc: add --all to image list for docker parity (#41456)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* wslc: add --details to container logs for docker parity (#41467)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: JohnMcPMS <johnmcp@microsoft.com>

* Localization change from build: 157218885 (#41559)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Don't fail the installation if DeprovisionMsix() fails (#41453)

* Don't fail the installation if DeprovisionMsix() fails

* Apply PR feedback

* Notice change from build: 157227119 (#41564)

Co-authored-by: WSL notice <noreply@microsoft.com>

* wslc: add --size to container list for docker parity (#41477)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* wslc: add --all-tags to push for docker parity (#41500)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix build issue for wslsettings, and add more logging to the pipelines (#40388)

* Validate variable-length message strings (#41567)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: 3086bdaf-bc43-4fed-88d1-3a95a21fd14e

* Fix WSLC fallback gateway collision (#41547)

Avoid selecting the guest IPv4 address as its synthesized default gateway when the host adapter does not expose one.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: ce168659-cb9d-4f0e-8fd1-2834d065ba9d

* Reduce distro termination log noise (#41541)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: adf24228-67c0-452e-9cc2-c698a8d7b3b7

* Localization change from build: 157277214 (#41571)

Co-authored-by: WSL localization <noreply@microsoft.com>

* CLI: Add global options to root help, adjust options usage (#41534)

* Add global options to root help, adjust options usage to match CLI conventions

* Trim some unnecessary test code

* Localization change from build: 157310027 (#41574)

Co-authored-by: WSL localization <noreply@microsoft.com>

* wslc: add docker --quiet to image load, image push and container cp (#41466)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Tear down the stale plugin Plan9 server before recreating it (#41565)

When the per-user plugin Plan9 server is no longer running, the previous instance was dropped without a Teardown call, so it could still hold the Plan9 port when the replacement tries to bind it.

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d028ad4-fe5e-4ef1-9832-18ee0e4825cc

* Use a locally imported version of docker/dockerfile instead of downloading it from the default registry (#41575)

* Use a locally imported version of docker/dockerfile instead of downloading it from the default registry

* Cleanup diff

* Use canonical path in VolumeMount_Parse_ReturnExpectedResult (#41577)

* Document WSL security model (#41556)

* Document WSL security model

Clarify WSL trust boundaries and explain that configuration settings do not establish a sandbox.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c77eaf09-799a-416e-b4b4-19f37a4201ef

* Update WSL security model explanation

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Clarify WSL isolation guidance

Distinguish functional distribution separation from a security boundary and recommend a separately managed VM for untrusted workloads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c77eaf09-799a-416e-b4b4-19f37a4201ef

* Document shared WSL VM trust model

Clarify that elevated and non-elevated sessions can share utility VM state and are not separate guest security boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c77eaf09-799a-416e-b4b4-19f37a4201ef

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot-Session: c77eaf09-799a-416e-b4b4-19f37a4201ef

* Bump GitPython to 3.1.59 (#41580)

Resolves open Dependabot alerts for GitPython <= 3.1.58 in
distributions/requirements.txt and tools/devops/requirements.txt.

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6fcfe0f1-6be5-4913-ab36-71823443cc36

* Remove two noisy warnings from the distro validation scripts (#41579)

* Remove two noisy warnings from the distro validation scripts

* Cleanup diff

* Fix formatting of USR_SHARE_WSL assignment

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix activating a stopping service treated as OOM (#41460)

The current factory function for LxssUserSession and WSLCSessionManager translates the error code CO_E_SERVER_STOPPING to S_FALSE. Which combined with *ppCreated == NULL causes COM to treat this as an OOM. Leading to error messages like this when activating a stopping service:

Not enough memory resources are available to complete this operation.
Error code: Wsl/E_OUTOFMEMORY
This PR removes this conversion. So, COM actually retries when the service is stopping. And returns CO_E_SERVER_EXEC_FAILURE if the retry times out.

* Set distributionStartTimeout to 2 minutes in the tests to solve distribution start timeouts errors (#41583)

* Set distributionStartTimeout to 2 minutes in the tests to solve distribution start timeouts errors

* Update tests

* Localization change from build: 157504221 (#41602)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Enable RedirectionGuard process mitigation (#41542)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Updates Ubuntu latest LTS images (#41465)

* Updates Ubuntu 26.04 to the .1 release

Just announced today.
Also published the up-to-date Ubuntu package to MS Store,
but I'm not referencing it here.

* Updates the 24.04.5 images just released today.

* Use the cdimages.u.c host for consistency

---------

Co-authored-by: Carlos Nihelton <carlos.nihelton@canonical.com>

* diagnostics: improve collect-wsl-logs for analysis (summary.json, README, profile info, WSL/guest state) (#40776)

* diagnostics: record capture profile in collected logs

collect-wsl-logs.ps1 did not record which WPR profile was used for a
capture. When analyzing an archive (e.g. a networking-only capture that
lacks the WSL core trace providers), there was no way to tell which
profile produced it without inferring it from the provider mix.

Write a collection-info.txt into the log folder capturing the selected
LogProfile, the mapped WPRP profile and file, the Dump and
RestartWslReproMode switches, and the collection timestamp.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* diagnostics: collect WSL version, guest state, and drop empty dumps

Add three further debugging improvements to collect-wsl-logs.ps1:

- Collect wsl --version / --status / --list --verbose into wsl-info.txt
  instead of forcing analyzers to infer the version and distro layout
  from the appx package and registry.
- Collect guest-side state (dmesg, free, uptime, ulimit, pid_max,
  threads-max, process/thread counts, top RSS) into linux_diagnostics.log
  after the repro. This is the data needed to diagnose in-distro failures
  such as 'Resource temporarily unavailable' (EAGAIN) from resource limits.
- Remove 0-byte dump files left behind when MiniDumpWriteDump fails, so
  the archive only contains real dumps.

Also set WSL_UTF8 and the console output encoding so wsl.exe output is
captured to the log files readably.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* diagnostics: add summary.json and README.md to log archive

Make collected log archives easier to analyze (by a human or an agent)
without having to run tools or infer state from individual artifacts:

- summary.json: machine-readable overview of the capture - profile,
  WSL/Windows versions, networking mode, installed distributions and
  their state, .wslconfig presence, and an inventory of non-empty dumps.
- README.md: an index of the archive contents describing each file, plus
  a note on how to decode logs.etl and how to tell when a non-default
  log profile was used.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* diagnostics: address PR feedback (utf8 wsl-info, wsl.exe timeouts, slimmer summary.json)

- Write wsl-info.txt as UTF-8 instead of the PS5.1 default UTF-16LE.
- Guard every newly-added wsl.exe call (wsl-info and guest diagnostics) with a
  timeout via a background job so a deadlocked service or bad VM state cannot
  hang log collection.
- Drop the duplicated distro-registry enumeration and .wslconfig networkingMode
  parsing from summary.json; that state is readily derived from HKCU.txt and the
  archived .wslconfig.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* diagnostics: drop wsl-info.txt and guest diagnostics collection

Remove the unconditional wsl.exe calls this PR introduced (wsl-info.txt and
linux_diagnostics.log) along with the now-unused timeout helper, per review
feedback that the log collection script should not call wsl.exe (which can hang
if the service is deadlocked or the VM is in a bad state). Pre-existing
networking-profile wsl.exe calls are left untouched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* collect-wsl-logs: drop bundled summary.json and README per review

OneBlue noted the agent-readable index and the summary values are redundant
in every archive (an analyzer can derive them from the archive contents).
Keep only collection-info.txt, which records non-derivable capture provenance
(which WPR profile/switches were used).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Move security model under technical documentation (#41605)

* Move security model under technical documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 43c7b692-ffc5-43bf-88b9-ecc927a1aa55

* Move security model into technical documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 43c7b692-ffc5-43bf-88b9-ecc927a1aa55

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Copilot-Session: 43c7b692-ffc5-43bf-88b9-ecc927a1aa55

* Fix typos (#41589)

* Don't stop parsing linux config files on invalid lines (#41606)

* wslc: add --all-tags to pull for docker parity (#41494)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Added WSL container to OOBE (#41402)

* Pre merge Localization strings prior to GA (#41585)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* wslc: keep image list json CreatedSince locale-invariant (#41609)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Localization change from build: 157621275 (#41610)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Warn on an escaped CR in wsl.conf instead of dropping it silently (#41591)

`case '\r': break;` made a backslash before a CR the only unrecognised
escape the parser accepts without a diagnostic. In a CRLF file that left
the value truncated at the backslash and the remainder parsed as its own
line, with nothing reported.

Letting it fall into the default case gives the same
MessageConfigInvalidEscape warning as any other bad escape. As with those,
the line is then discarded rather than kept truncated.

* Localization change from build: 157667513 (#41614)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Set a restricted ephemeral port range for test case ConsommeTests::PortZeroBindIsTracked (#41616)

* repo: modify CODEOWNERS to change wsl-maintainers to wsl-reviewers team (#41617)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Localization change from build: 157736413 (#41621)

Co-authored-by: WSL localization <noreply@microsoft.com>

* CLI: Global option scopes for nested commands (related to compose support) (#41546)

* Change default relay  buffer size to 64KiB (#41603)

The current logic uses a fixed 4KiB buffer for stdio relay on the Windows side. And uses an initial 4KiB buffer for stdio relay on the Linux side, which grows only if the message won't fit. This can severely limit the relay performance in some situations. For example, in #41572, when redirecting stdout to a SMB share.

This PR increases thedefault relay buffer size to 64KiB. Which shows significant performance improvements according to buffer size tests.

* Fix redirect stdout and stderror to the same file overlapping (#41611)

Currently the stdout and stderr relay tracks the output offset separately. And when redirected to the same file, the writes could overlap.

This PR uses the append mode for overlap io writes instead of the separate offsets in the relay. So, the file write offset is correctly tracked by the system.

* move installer log collection after repro (#41620)

The installer log files were collected before the log collection starts. Which will miss the user repro.

This PR moves it after the user repro.

* wslc: add --digests to image list for docker parity (#41457)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Localization change from build: 157843092 (#41630)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Explicitely return SOCKET from WSLCPluginAPI_ProcessGetFd() (#41626)

* In mirrored mode, handle route update as add instead of replace (#41391)

In mirrored mode, handle route update as add instead of replace
Remove usage of route update as it can overwrite routes on other interfaces. add tests that verify host routing changes are correctly reflected in Linux

---------

Co-authored-by: Catalin-Emil Fetoiu <cfetoiu@microsoft.com>

* Add wslc events command (#41608)

* Don't fail the UnitTests::Warnings test case if GlobalSecureAccess VPN is running (#41638)

* Don't fail the UnitTests::Warnings test case if GlobalSecureAccess VPN is running

* Cleanup diff

* Fix test

* Wait for wslservice to be started when running tests (#41639)

* Wait for wslservice to be started when running tests

* Fail on timeout

* Improve check

* Format

* Create new namespaces for distro cgroups (#41512)

In 2.9.8, the distro processes are separated into their own cgroups. But they remain in the same cgroup namespace. That caused compatibility issues with softwares that assume a fixed systemd cgroup layout. For example, rootless docker and nerdctl.
Fixes on Moby and nertctl are being worked on. However, to avoid issues with other software, WSL's cgroup handling should also be improved.

This PR creates new cgroup namespaces for the distros. So, to the non-critical distro processes, the systemd cgroup layout stays the same as before.
This PR also introduces a cgroup structure change to accomplish this. The systemd init is moved from wsl-user/distro-N/systemd to wsl-user/distro-N. And the initialization of the distro-N controllers is handled by systemd instead.
The processes are also moved into the non-systemd cgroup in systemdless distros. This makes sure that sub-group controllers can be enabled in the distro root.
Cgroup v2 is now enforced when distro isolation is enabled. Instead of constructing an unusable cgroup v1 layout when cgroup v1 and distro isolation are both enabled.

* Fix unnecessary delay in the port tracking loop (#41472)

Before this change, an additional 10ms delay was added in the port tracker loop presumably to make the main loop slower than the worker loop. So, the worker result does not get super dated. This is not reliable. And the 10ms delay also slows down every consecutive bind call.

This PR refactors the thread synchronization method. So, the timing between those two loops is more deterministic. And removes the performance penalty for all bind calls.

* Localization change from build: 157954210 (#41643)

Co-authored-by: WSL localization <noreply@microsoft.com>

* wslc: add --follow-link to cp (#41501)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Disable unstable bind cap test and re-enable Loopback test that was incorrectly disabled- #41648 (#41648)

Co-authored-by: Catalin-Emil Fetoiu <cfetoiu@microsoft.com>

* Move objects shared by wslc.exe and the tests to src/windows/common (#41619)

* Move objects shared by wslc.exe and the tests to src/windows/common

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Link yaml-cpp, advapi32 and Crypt32 into common

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Drop yaml-cpp, advapi32 and crypt32 from wslclib now that common provides them

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Move wslc code in common under common/wslc and namespace CLI types as wslc::cli

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix various protocol parsing issues (#41637)

* Save state

* Use proper arrays

* Format

* Add new tests

* Cleanup tests

* Cleanup diff

* Format

* Apply PR feedback

* Apply PR feedback

* Format

* Apply PR feedback

* Move logging call

* Apply PR feedback

* Apply PR feedback

* Localization change from build: 157980831 (#41647)

Co-authored-by: WSL localization <noreply@microsoft.com>

* Add WSLC network lifecycle events (#41576)

* Add WSLC network lifecycle events

* Localize pending network prune errors

* Fix network operation wait ownership and cleanup ordering

* Fix prune event correlation after timed-out

* Fix network event rollback and timeout recovery

* Format

* Forward Docker network events directly

* Restore lock_guard after removing event waits

* fix test

* align event stream test helpers after merge

* feedback

* Fix potential out of bound access when pretty-printing string field in init messages (#41664)

* Localization change from build: 158197536 (#41662)

Co-authored-by: WSL localization <noreply@microsoft.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Feng Wang <wang6922@outlook.com>
Co-authored-by: ggarzia-MSFT <gavingarzia@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: David Bennett <dbennett-msft@outlook.com>
Co-authored-by: WSL localization <noreply@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: beena352 <beenachauhan@microsoft.com>
Co-authored-by: Arch Linux Technical User <65091038+archlinux-github@users.noreply.github.com>
Co-authored-by: Ben Hillis <benhillis@gmail.com>
Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: damanm24 <9593793+damanm24@users.noreply.github.com>
Co-authored-by: tega-ajise <tegjise15@gmail.com>
Co-authored-by: Tega Ajise <tegaajise@Tegas-MacBook-Pro-2.local>
Co-authored-by: Eamon <eamon112009@gmail.com>
Co-authored-by: Kevin Vega <40717198+kvega005@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: DesertRatUa <desertratua@gmail.com>
Co-authored-by: Scott Bradnick <84082961+sbradnick@users.noreply.github.com>
Co-authored-by: Stephen Halter <shalter+msft@microsoft.com>
Co-authored-by: JohnMcPMS <johnmcp@microsoft.com>
Co-authored-by: Flor Chacón <14323496+florelis@users.noreply.github.com>
Co-authored-by: Carlos Nihelton <cnihelton@ubuntu.com>
Co-authored-by: Carlos Nihelton <carlos.nihelton@canonical.com>
Co-authored-by: Anton Kesy <antonkesy@gmail.com>
Co-authored-by: Craig Loewen <crloewen@microsoft.com>
Co-authored-by: Leo Camus <leo.camus23@gmail.com>
Co-authored-by: FetoiuCatalin <fetoiucatalin@gmail.com>
Co-authored-by: Catalin-Emil Fetoiu <cfetoiu@microsoft.com>
Copilot-Session: eb7946ff-cc0b-4ff1-a059-571334b1c988
Copilot-Session: 35281c30-3d08-4f05-8c84-2ce4711023d5
Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54
Copilot-Session: 9213b7da-c5c8-4d9c-89ab-e80048d288a2
Copilot-Session: 3086bdaf-bc43-4fed-88d1-3a95a21fd14e
Copilot-Session: ce168659-cb9d-4f0e-8fd1-2834d065ba9d
Copilot-Session: adf24228-67c0-452e-9cc2-c698a8d7b3b7
Copilot-Session: 0d028ad4-fe5e-4ef1-9832-18ee0e4825cc
Copilot-Session: c77eaf09-799a-416e-b4b4-19f37a4201ef
Copilot-Session: 6fcfe0f1-6be5-4913-ab36-71823443cc36
Copilot-Session: 43c7b692-ffc5-43bf-88b9-ecc927a1aa55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants