wslc: add --follow-link to cp

[ggarzia-MSFT]

Summary of the Pull Request

Adds --follow-link / -L to wslc container cp

Without -L the behavior is unchanged: the symlink itself is copied.

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

CLI

• ArgumentDefinitions.h gains ArgType::FollowLink (--follow-link, alias L, Kind::Flag).
• ContainerCpCommand registers it alongside the existing --archive.

Container → local: statting a path over the Docker Engine API

StatArchivePath issues HEAD /containers/{id}/archive?path=... and reads the X-Docker-Container-Path-Stat response header. This is the same call ContainerStatPath makes in the engine's own client — cli.head(ctx, "/containers/"+containerID+"/archive", query, nil) in moby/moby:client/container_copy.go — which is what docker cp uses to resolve a symlink.

HEAD is a good fit for DockerHTTPClient: SendRequest returns as soon as the response header has been consumed and parsed, handing the still-open socket back to the caller, so it never waits on a body. StatArchivePath drops the socket immediately. Each request gets its own freshly-connected socket via SendRequestImpl, so releasing it has no effect on subsequent calls.

On the engine side, the HEAD route maps to headContainersArchive, which sets the header via setContainerPathStatHeader and writes no body (moby/moby:daemon/server/router/container/copy.go); GET streams the tar in addition to that header. A missing container or path yields 404, which maps to std::nullopt.

One consequence of HEAD: error responses carry no body, so DockerHTTPException is constructed with an empty body and message and callers have only the status code to work with.

The header is base64-encoded JSON, decoded with wslutil::Base64Decode into a new docker_schema::ContainerPathStat. The symlink test uses Go's os.ModeSymlink bit (1 << 27), since the mode field is a marshalled Go os.FileMode.

When the target is relative, it is joined against the parent directory of the source path — the same rebasing docker performs via archive.SplitPathDirEntry.

Keeping the requested name

A followed link produces an archive named after the link's target, while the copy has to keep the name that was asked for. Container → local extraction therefore goes to a staging directory and the entries are moved up under the requested name afterwards; local → container stages the target's tree under the link's name (with its own nested links intact) because tar.exe cannot rename entries.

The requested name comes from wsl::windows::common::filesystem::PosixBaseName. A POSIX name may hold characters that no Windows file name can (: and \ are legal in POSIX), so the copy is rejected with WSLCCLI_CpSourceNameNotRepresentableError rather than silently landing under the target's name instead.

Interface changes

IWSLCContainer::DownloadArchive (wslc.idl) gains a FollowLink parameter:

HRESULT DownloadArchive([in, string] LPCSTR SrcPath, [in] BOOL FollowLink, [in] WSLCHandle OutHandle);

IWSLCContainer is internal and non-stable — it is rebuilt and ships in lockstep with its only clients — so changing an existing method's signature is safe here. The SDK-facing IWSLCCompatContainer in WSLCCompat.idl is untouched.

Validation Steps Performed

• Full cmake --build . — clean.

• wslc container cp --help lists the new option:

  Options:
    -a  --archive      Archive mode (accepted for Docker CLI compatibility)
    -L  --follow-link  Always follow symlinks in SRC_PATH
    -?  --help         Shows help about the selected command
  

• 11 new parser cases in CommandLineTestCases.h covering -L, --follow-link, both copy directions, =true/=false on both spellings, combination with -a, and the negatives -L=invalid, --followlink and -l (aliases are case-sensitive).

• ContainerCpCommand_HasFollowLinkArgumentWithDockerAlias pins the name, alias, kind and optionality.

• E2E coverage in WSLCE2EContainerCpTests.cpp:

  • WSLCE2E_Container_Cp_HelpListsFollowLink asserts the flag is surfaced in help output.
  • WSLCE2E_Container_Cp_LocalToContainer_FollowLinkCopiesTargetContents — file link, dereferenced into the container under the link's name.
  • WSLCE2E_Container_Cp_LocalToContainer_FollowLinkCopiesDirectoryTree — directory link, staged under the link's name, with a nested link preserved as a link.
  • WSLCE2E_Container_Cp_ContainerToLocal_FollowLinkCopiesTargetContents — absolute link target.
  • WSLCE2E_Container_Cp_ContainerToLocal_FollowLinkResolvesRelativeTarget — relative link target, which only resolves against the directory holding the link.

[Copilot]

🟡 Changes recommended

It adds new behavior without an e2e test validating -L actually follows symlinks, and it introduces at least one naming/doc inconsistency that should be addressed before merging.

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

Pull request overview

This PR adds --follow-link / -L support to wslc container cp to match docker cp -L semantics by resolving symlinks in the source path (both local→container and container→local) before copying.

Changes:

• Adds a new CLI flag (ArgType::FollowLink, --follow-link, alias -L) and wires it into container cp.
• Implements container-side symlink resolution by statting /containers/{id}/archive and decoding X-Docker-Container-Path-Stat, exposed via a new IWSLCContainer::ResolveArchiveSymlink method.
• Adds parser/unit/e2e coverage for argument presence and help output surfacing.

File summaries

File	Description
test/windows/wslc/WSLCCLICommandUnitTests.cpp	Adds a unit test to pin --follow-link name/alias/kind.
test/windows/wslc/e2e/WSLCE2EContainerCpTests.cpp	Adds an e2e test ensuring container cp --help lists --follow-link/-L.
test/windows/wslc/CommandLineTestCases.h	Adds command-line parsing cases for -L/--follow-link (incl. boolean forms and negatives).
src/windows/wslcsession/WSLCContainer.h	Adds ResolveArchiveSymlink to the container implementation and COM interface class.
src/windows/wslcsession/WSLCContainer.cpp	Implements ResolveArchiveSymlink and exposes it via COM.
src/windows/wslcsession/DockerHTTPClient.h	Declares StatArchivePath helper for reading X-Docker-Container-Path-Stat.
src/windows/wslcsession/DockerHTTPClient.cpp	Implements StatArchivePath (GET archive, read stat header, close socket).
src/windows/wslc/tasks/ContainerTasks.cpp	Adds followLink behavior for container cp in both copy directions.
src/windows/wslc/services/ContainerService.h	Adds ResolveContainerSymlink service helper declaration.
src/windows/wslc/services/ContainerService.cpp	Implements ResolveContainerSymlink by calling IWSLCContainer::ResolveArchiveSymlink.
src/windows/wslc/commands/ContainerCpCommand.cpp	Registers ArgType::FollowLink for the container cp command.
src/windows/wslc/arguments/ArgumentDefinitions.h	Defines the new argument type and localization hook.
src/windows/service/inc/wslc.idl	Extends IWSLCContainer with ResolveArchiveSymlink.
src/windows/inc/docker_schema.h	Adds ContainerPathStat schema for the archive stat header payload.
localization/strings/en-US/Resources.resw	Adds localized help text for --follow-link.

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 3
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

DockerHTTPClient::StatArchivePath currently treats all non-200 responses (and even missing stat headers on 200) as “path not found,” which can silently mask real engine errors and cause --follow-link to behave incorrectly.

Review details

Suppressed comments (2)

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

src/windows/wslcsession/DockerHTTPClient.cpp:449

• StatArchivePath() currently returns nullopt for any non-200 response, but its contract says nullopt means the path does not exist. This can silently mask real engine errors (e.g., 500/401) and cause --follow-link to fall back to copying the symlink instead of failing fast.

This issue also appears on line 451 of the same file.

src/windows/wslcsession/DockerHTTPClient.cpp:455

• On a 200 response, a missing X-Docker-Container-Path-Stat header indicates an unexpected daemon/proxy behavior and should not be treated the same as "path does not exist"; otherwise --follow-link can silently do the wrong thing.

    const auto header = response["X-Docker-Container-Path-Stat"];
    if (header.empty())
    {
        return std::nullopt;
    }

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

DockerHTTPClient::StatArchivePath currently treats all non-200 responses (and missing stat headers) as “not found,” which can mask real engine/protocol failures and should be tightened to surface errors correctly.

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

Review details

Suppressed comments (2)

src/windows/wslcsession/DockerHTTPClient.cpp:455

• If the response is 200 but missing the X-Docker-Container-Path-Stat header, returning nullopt will make the caller treat it as "not a symlink" and proceed, which can mask a protocol/engine mismatch. It should be treated as an unexpected error so callers can fail fast with a clear diagnostic.

    const auto header = response["X-Docker-Container-Path-Stat"];
    if (header.empty())
    {
        return std::nullopt;
    }

src/windows/wslcsession/DockerHTTPClient.h:150

• The comment for StatArchivePath says it returns nullopt when the path does not exist, but the implementation also treats other non-200 responses (and a missing stat header) as nullopt. Update the comment to reflect the actual contract (e.g., 404 => nullopt, other errors => exception) to avoid misleading future callers.

    // Reads the X-Docker-Container-Path-Stat header for a container path. Returns nullopt if the path does not exist.
    std::optional<common::docker_schema::ContainerPathStat> StatArchivePath(const std::string& ContainerID, const std::string& Path);

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

There is a confirmed path-handling bug in the container cp local→container flow where stripping a trailing separator can turn a drive-root parent directory (e.g. C:\) into C:, altering tar.exe -C behavior and potentially breaking copies from drive roots.

Review details

Suppressed comments (2)

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

src/windows/wslc/tasks/ContainerTasks.cpp:387

• When SRC_PATH is at a drive root (e.g. "C:\file"), absPath.parent_path() becomes "C:\". The subsequent trailing-separator trimming in this block turns that into "C:", which changes tar.exe -C semantics ("C:" means the current directory on that drive) and can break copies from the drive root. Preserve root paths when stripping trailing separators.
  src/windows/wslc/services/ContainerService.h:68
• This comment says ResolveContainerSymlink returns nullopt only when the path is not a symlink, but the underlying service method also returns nullopt when the path does not exist. Update the comment to match the actual behavior.

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new container-side symlink resolution currently holds a shared lock across network I/O, and the new local→container follow-link behavior lacks corresponding automated coverage.

Review details

Suppressed comments (2)

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

src/windows/wslc/tasks/ContainerTasks.cpp:384

• The new --follow-link behavior for local → container relies on std::filesystem::canonical() to resolve the source path before archiving. There is E2E coverage for container → local symlink following, but no test coverage for this local → container path-resolution behavior, so regressions here would be easy to miss.
  src/windows/wslcsession/WSLCContainer.cpp:2688
• ResolveArchiveSymlink holds the container shared lock while issuing a Docker API request (StatArchivePath). This can block other container operations longer than necessary; other archive operations (e.g., DownloadArchive/UploadArchive) release the lock before doing potentially long I/O.

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

ResolveArchiveSymlink currently resolves only a single symlink hop, which can make --follow-link copy an intermediate symlink instead of the final target for chained links.

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

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There is a correctness issue in a new E2E test (missing container setup) and the new stat request should use HEAD to avoid unnecessary archive streaming work.

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

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Three moderate issues remain in ContainerTasks.cpp, and the interface-change description needs correction.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/windows/service/inc/wslc.idl:587

• The PR description says a new IWSLCContainer::ResolveArchiveSymlink interface method was added, but this IDL changes only DownloadArchive and no ResolveArchiveSymlink declaration or implementation exists in the repository. Please update the description so the documented interface change matches the patch.

    // When FollowLink is set and SrcPath names a symbolic link, the archive is taken from the link's target.
    HRESULT DownloadArchive([in, string] LPCSTR SrcPath, [in] BOOL FollowLink, [in] WSLCHandle OutHandle);

src/windows/wslc/tasks/ContainerTasks.cpp:518

• The staging root is created under the system temp directory without checking whether it is inside resolved. A valid source symlink can point at %TEMP% (or a directory containing that staging root); the recursive copy then targets a newly created child of its own source, so it can recurse into the staging tree or fail after excessive path/space use. Choose a staging location outside the resolved source tree, or avoid recursive filesystem staging for this case.

                    stagingDir = MakeStagingDirectory(std::filesystem::temp_directory_path());

                    std::error_code copyError;
                    std::filesystem::copy(
                        resolved,
                        stagingDir / absPath.filename(),
                        std::filesystem::copy_options::recursive | std::filesystem::copy_options::copy_symlinks,

• Files reviewed: 13/13 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical destination path traversal and additional symlink-handling correctness issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

src/windows/service/inc/wslc.idl:587

• The detailed description says this change adds IWSLCContainer::ResolveArchiveSymlink, but the interface change here instead modifies DownloadArchive and no ResolveArchiveSymlink method is present. Please reconcile the PR description with the actual API change so consumers and reviewers are not misled.

    // When FollowLink is set and SrcPath names a symbolic link, the archive is taken from the link's target.
    HRESULT DownloadArchive([in, string] LPCSTR SrcPath, [in] BOOL FollowLink, [in] WSLCHandle OutHandle);

src/windows/wslc/tasks/ContainerTasks.cpp:520

• When a local symlink to a directory is passed with a trailing separator, std::filesystem::absolute preserves that separator and absPath.filename() is empty. The staged copy then targets stagingDir itself, so the archive is named after the generated staging directory (or the copy fails) instead of the requested link name. Derive the source leaf after removing trailing separators before using it here.

                        stagingDir / absPath.filename(),
                        std::filesystem::copy_options::recursive | std::filesystem::copy_options::copy_symlinks,
                        copyError);
                    THROW_HR_IF_MSG(HRESULT_FROM_WIN32(copyError.value()), !!copyError, "Failed to copy from: %ls", resolved.c_str());

src/windows/wslcsession/WSLCContainer.cpp:1859

• The added E2E coverage only exercises an absolute link target. The resolved.front() != '/' branch is the implementation for common relative links such as ln -s ../target link; without a test for that case, a regression in rebasing against the source's parent would leave -L broken while the current tests still pass. Add an E2E case with a relative container symlink and verify the copied content and requested name.

            if (resolved.front() != '/')
            {
                const auto separator = effectivePath.find_last_of('/');
                if (separator != std::string::npos)
                {

src/windows/wslcsession/WSLCContainer.cpp:1860

• The relative-target join preserves ./.. segments instead of normalizing them like Docker's filepath.Join/SplitPathDirEntry. For a link whose target is dir/. (or .), the GET is made with a trailing /.; Docker would archive the directory under its basename, but this can produce an archive rooted at .. The later staged.size() heuristic can then rename a single child to the link name (or move nothing for an empty directory), yielding a file or missing directory instead of the followed directory. Normalize the POSIX path before GetArchive and add coverage for dir/. and empty/single-entry targets.

                const auto separator = effectivePath.find_last_of('/');
                if (separator != std::string::npos)
                {
                    resolved = effectivePath.substr(0, separator + 1) + resolved;

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate correctness, performance, and coverage issues remain.

Review details

Suppressed comments (7)

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

src/windows/wslcsession/WSLCContainer.cpp:1860

• The new relative-link rebasing branch is not covered by the E2E tests: the added container-to-local test creates an absolute target (ln -s /tmp/linktarget.txt ...). Add a case with a relative target such as ln -s linktarget.txt thelink.txt and verify that the target contents are copied under the link name, since parent-path joining is the behavior introduced here.

src/windows/service/inc/wslc.idl:587

• The PR description says a new IWSLCContainer::ResolveArchiveSymlink method is being added, but this interface instead changes DownloadArchive and no ResolveArchiveSymlink method exists. Please update the description to reflect the actual interface/API change so the documented contract matches the shipped surface.

    // When FollowLink is set and SrcPath names a symbolic link, the archive is taken from the link's target.
    HRESULT DownloadArchive([in, string] LPCSTR SrcPath, [in] BOOL FollowLink, [in] WSLCHandle OutHandle);

src/windows/wslc/commands/ContainerCpCommand.cpp:19

• The PR description claims a ContainerCpCommand_HasFollowLinkArgumentWithDockerAlias unit test and 13 parser cases, but the repository contains no such unit test and this diff adds only 11 parser cases (all under container cp). Please add the claimed coverage or update the validation section to match the submitted changes.

        Argument::Create(ArgType::FollowLink),

src/windows/wslc/tasks/ContainerTasks.cpp:613

• With --follow-link, this stages every container-to-local copy, including ordinary regular files and directories that are not symlinks. A large cp -L container:/regular-dir existing-dir therefore materializes a second full tree before moving it, adding unnecessary I/O and disk usage and potentially failing with disk-full where direct extraction would succeed. Use the stat result to stage only when the source is actually a symlink, or rebase the archive stream without staging.

            auto extractRoot = absTarget;
            if (followLink)
            {
                stagingDir = MakeStagingDirectory(absTarget);
                extractRoot = stagingDir;

src/windows/wslc/tasks/ContainerTasks.cpp:76

• This fallback silently loses the requested basename for valid POSIX container names containing : or \\ (for example, a link /tmp/link:name): requestedName becomes empty, so the target's basename is used at line 664 instead of the link's name. Since those characters cannot be represented as ordinary Windows filenames, reject the operation with a clear error (or otherwise handle it explicitly) rather than copying to a different name.

    return name.find_first_of("\\/:") == std::string::npos ? name : std::string{};

src/windows/wslc/tasks/ContainerTasks.cpp:523

• The local-to-container directory-symlink path is also untested: the new E2E case only exercises the dereference file branch, while this staging logic is what preserves the link name and nested symlinks when the target is a directory. Add a directory-link case (including at least one nested link) before relying on this path.

                if (std::filesystem::is_directory(resolved))
                {
                    // The archive has to carry the link's name while holding the target's tree, and tar.exe
                    // cannot rename entries, so the tree is staged under that name with its own links intact.
                    stagingDir = MakeStagingDirectory(std::filesystem::temp_directory_path());

src/windows/wslcsession/WSLCContainer.cpp:1860

• When SrcPath ends in /, find_last_of('/') returns that trailing separator, so a relative target for /dir/link/ is rebased to /dir/link/target instead of /dir/target. Since cp accepts trailing separators, trim trailing separators (while preserving /) before finding the link's parent.

                const auto separator = effectivePath.find_last_of('/');
                if (separator != std::string::npos)
                {
                    resolved = effectivePath.substr(0, separator + 1) + resolved;

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[David Bennett (dkbennett)]

This is fine for now, but the entire container cp methods are way too low level to be at the task layer and should be refactored. Several of the helpers should probably go into common as well along with unit tests for them.

[ggarzia-MSFT]

This is fine for now, but the entire container cp methods are way too low level to be at the task layer and should be refactored. Several of the helpers should probably go into common as well along with unit tests for them.

Happy to make a follow up PR to address this if you want me to

Looking at some of copilots latest comments, I've changed my mind and would like to see those addressed first.

[David Bennett (dkbennett)]

Please address the latest comments from Copilot about correctness.

Recommend moving some of these filesystem methods into common and adding unit tests for them to verify the behavior.

As a followup, I think the cp method in general needs to be refactored into the service, way too much stuff going on in the tasks. If you want to do that feel free, otherwise I'll probably do it in a few days.

[Copilot]

🟡 Changes recommended

Follow-link staging can fail for valid POSIX target names before requested-name rebasing.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/windows/wslc/tasks/ContainerTasks.cpp:624

• This predicate does not cover all names that the Win32 filesystem cannot represent: a component ending in . or a space is normalized, and DOS device names such as CON/NUL are reserved even without invalid characters. A container symlink named foo. or CON can therefore be copied under a different object (or a device) instead of retaining the requested name. Validate the complete Windows filename-component rules before calling MoveOver (and add cases for these names).

                    constexpr std::wstring_view reserved = L"<>:\"/\\|?*";
                    for (const auto character : requestedName)
                    {
                        if (character < L' ' || reserved.find(character) != std::wstring_view::npos)
                        {

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved moderate correctness issues remain in name handling, directory merging, and archive staging.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/windows/wslc/tasks/ContainerTasks.cpp:608

• The new container-to-local --follow-link staging/move path is not exercised by the added E2E tests: the container-to-local cases only follow file links, while the directory-tree case is local-to-container. Please add a container directory-link case that verifies the requested destination name and nested links, since that is the behavior implemented by this block and is where archive shape and MoveOver handling can regress.

            if (followLink)
            {
                // Moving entries invalidates an open directory iterator, so the listing is taken first.
                std::vector<std::filesystem::path> staged;
                for (const auto& entry : std::filesystem::directory_iterator(stagingDir))
                {
                    staged.push_back(entry.path());

src/windows/wslc/tasks/ContainerTasks.cpp:85

• When rename fails because the destination directory already exists, this fallback does not merge From into To: std::filesystem::copy treats an existing directory destination as a container and copies the source under To/From.filename(). Copying a followed directory link into an existing destination can therefore create an extra nested directory instead of matching tar's merge semantics. Enumerate and merge the source children into To while retaining the type checks.

    std::filesystem::copy(
        From,
        To,
        std::filesystem::copy_options::recursive | std::filesystem::copy_options::overwrite_existing | std::filesystem::copy_options::copy_symlinks,
        copyError);

src/windows/wslc/tasks/ContainerTasks.cpp:578

• If the symlink points to a POSIX-only target basename such as a:b while the requested link name is valid, this staging path still asks tar.exe to extract the archive under the target's basename before the later rebase. Windows tar cannot create that entry, so --follow-link fails even though the copy could be named after the valid link. The archive needs to be extracted under a sanitized staging name or have its entry names rewritten before extraction.

            auto extractRoot = absTarget;
            if (followLink)
            {
                stagingDir = wsl::windows::common::filesystem::MakeStagingDirectory(absTarget);
                extractRoot = stagingDir;
            }

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Lite

[David Bennett (dkbennett)]

This looks good enough to me to checkin. The correctness issues from copilot stem from possible failure cases and robustness in the validation, which can be addressed in a followup. The test coverage looks good as does the refactor.

[ggarzia-MSFT]

This looks good enough to me to checkin. The correctness issues from copilot stem from possible failure cases and robustness in the validation, which can be addressed in a followup. The test coverage looks good as does the refactor.

I will double check the latest copilot comment, but a few of those are incorrect and ours behaves the same as docker

[ggarzia-MSFT]

This looks good enough to me to checkin. The correctness issues from copilot stem from possible failure cases and robustness in the validation, which can be addressed in a followup. The test coverage looks good as does the refactor.

I will double check the latest copilot comment, but a few of those are incorrect and ours behaves the same as docker

We behave the same as docker for all of the cases presented by copilot