Deep Review: 20260514-141313-pr-350

// DockerSocket returns the path to the Docker socket for this instance
// (e.g., ~/.rd2/docker.sock). This is the host-side socket that Lima
// port-forwards from the guest's /var/run/docker.sock.
// On Windows, returns the named pipe path (\\.\pipe\docker_engine).
var DockerSocket = sync.OnceValue(func() string {
	if runtime.GOOS == "windows" {
		return `\\.\pipe\docker_engine`
	}
	return filepath.Join(ShortDir(), "docker.sock")
})

// DockerEndpoint returns the full Docker endpoint URL for this instance.
// On Windows this is a named-pipe URL (npipe:////./pipe/docker_engine);
// on Unix it is a unix-socket URL (unix:///path/to/docker.sock).
var DockerEndpoint = sync.OnceValue(func() string {
	if runtime.GOOS == "windows" {
		return `npipe:////./pipe/docker_engine`
	}
	return "unix://" + DockerSocket()
})

On Unix the socket path embeds ShortDir(), which factors in RDD_INSTANCE, so instances are isolated. On Windows both functions return the global docker_engine pipe — and pkg/socketbridge/host_windows.go:20-23 documents that exact name as the one "Docker CLI and Docker Desktop use to reach dockerd."

	"github.com/Microsoft/go-winio"
	"github.com/go-logr/logr"
	"github.com/linuxkit/virtsock/pkg/hvsock"
)

const (
	// DockerPipeName is the well-known Windows named pipe that Docker CLI
	// and Docker Desktop use to reach dockerd.
	DockerPipeName = `\\.\pipe\docker_engine`

	// VsockForwardPort is the AF_VSOCK port that rdd-guest listens on inside
	// the Lima VM.  Must match the constant in cmd/rdd-guest/main.go.
	VsockForwardPort uint32 = 6660
)

The failure chain is concrete. socketbridge.HostBridge.Run calls winio.ListenPipe(b.pipeName, nil) and returns an error if the pipe is already owned (pkg/socketbridge/host_windows.go:52-56). That error is swallowed: pkg/controllers/lima/limavm/controllers/hostswitch_windows.go:199-205 runs the bridge in an errgroup goroutine that logs the failure and return nil. Meanwhile the engine controller's Docker client connects to the same endpoint (pkg/controllers/app/engine/controllers/docker_watcher.go:55 via instance.DockerEndpoint()). So with Docker Desktop installed and listening, RDD's bridge either loses the listen race or coexists as a second pipe instance, and RDD's engine client reaches Docker Desktop's daemon. fullSync then mirrors Docker Desktop's containers into RDD's API, and a K8s-side delete forwards ContainerRemove to that daemon — RDD operating destructively on a container engine it does not own. Concurrent RDD instances (RDD_INSTANCE=1 and =2) collide the same way, though the repo's single-user calibration makes the Docker Desktop collision the more pressing leg.

	}
}

// Run listens for named-pipe connections and proxies them to the guest until
// ctx is cancelled.
func (b *HostBridge) Run(ctx context.Context) error {
	ln, err := winio.ListenPipe(b.pipeName, nil)
	if err != nil {
		return fmt.Errorf("socket-bridge: listen on %s: %w", b.pipeName, err)
	}
	b.log.Info("Listening", "pipe", b.pipeName, "vsockPort", b.vsockPort)

	go func() {
		<-ctx.Done()
		ln.Close()

	// Start the host-side socket bridge now that we have the VM GUID.
	// It listens on the Docker named pipe and forwards each connection to
	// rdd-guest inside the VM via vsock port 6660.  rdd-guest is baked into
	// the VM image (via rancher-desktop-opensuse) and started by systemd.
	g.Go(func() error {
		bridge := socketbridge.NewDockerHostBridge(vmGUID, logger)
		if err := bridge.Run(ctx); err != nil {
			logger.Error(err, "Socket bridge exited with error")
		}
		return nil
	})

	// Accept vsock connections and feed them into the virtual network.
	g.Go(func() error {
		for {
			conn, err := ln.Accept()

// newDockerWatcher creates a Docker client, performs a full sync, and starts
// the event stream watcher goroutine.
func newDockerWatcher(ctx context.Context, k8s client.Client, apiNamespace string, reconcileChan chan<- event.GenericEvent) (*dockerWatcher, error) {
	cli, err := mobyclient.New(
		mobyclient.WithHost(instance.DockerEndpoint()),
	)
	if err != nil {
		return nil, fmt.Errorf("failed to create Docker client: %w", err)
	}

This is the same code Round 1 flagged as S4 (a docstring gap), but the Round 2 analysis is materially different: it is not the docstring, it is RDD silently mutating a foreign daemon. Round 1's S4 (still unaddressed — the docstring at instance.go:123 continues to say "for this instance") is subsumed here.

// length constraints.
var LimaHome = sync.OnceValue(func() string {
	return filepath.Join(ShortDir(), "lima")
})

// DockerSocket returns the path to the Docker socket for this instance
// (e.g., ~/.rd2/docker.sock). This is the host-side socket that Lima
// port-forwards from the guest's /var/run/docker.sock.
// On Windows, returns the named pipe path (\\.\pipe\docker_engine).
var DockerSocket = sync.OnceValue(func() string {
	if runtime.GOOS == "windows" {

Gemini additionally claimed the global pipe "breaks the VM boot sequence." That causal leg does not hold: the bridge starts after the VM GUID is known (hostswitch_windows.go:195-205), and @mook-as reported docker run / docker log working — so the pipe binds. His Running=False/Starting stall is a VM-side condition, not a pipe-bind failure.

	mux.Handle("/services/forwarder/expose", vn.Mux())
	mux.Handle("/services/forwarder/unexpose", vn.Mux())

	g, ctx := errgroup.WithContext(ctx)

	// Start the host-side socket bridge now that we have the VM GUID.
	// It listens on the Docker named pipe and forwards each connection to
	// rdd-guest inside the VM via vsock port 6660.  rdd-guest is baked into
	// the VM image (via rancher-desktop-opensuse) and started by systemd.
	g.Go(func() error {
		bridge := socketbridge.NewDockerHostBridge(vmGUID, logger)
		if err := bridge.Run(ctx); err != nil {
			logger.Error(err, "Socket bridge exited with error")
		}
		return nil
	})

	// Accept vsock connections and feed them into the virtual network.
	g.Go(func() error {
		for {
			conn, err := ln.Accept()

Fix: scope the pipe to the instance, mirroring the Unix isolation, and pass that name through socketbridge.NewDockerHostBridge instead of the hardcoded DockerPipeName constant. If a well-known alias is required for bare docker.exe compatibility, bind it only after proving RDD owns it, and surface the bind failure into readiness so the engine never treats an externally-owned pipe as RDD's endpoint.

 var DockerSocket = sync.OnceValue(func() string {
 	if runtime.GOOS == "windows" {
-		return `\\.\pipe\docker_engine`
+		return `\\.\pipe\` + Name() + `-docker_engine`
 	}
 	return filepath.Join(ShortDir(), "docker.sock")
 })

 var DockerEndpoint = sync.OnceValue(func() string {
 	if runtime.GOOS == "windows" {
-		return `npipe:////./pipe/docker_engine`
+		return `npipe:////./pipe/` + Name() + `-docker_engine`
 	}
 	return "unix://" + DockerSocket()
 })

    # Isolate all Docker config reads and writes from the developer's real
    # ~/.docker directory. Set DOCKER_CONFIG before starting the service so
    # the controller process inherits it (service.Start uses exec.Command
    # without an explicit Env, so it inherits the caller's environment).
    #
    # On Windows, rdd.exe is a native binary: it interprets /tmp/... as a
    # path from the drive root (C:\tmp\...) rather than MSYS2's /tmp which
    # maps to a different location. cygpath -m produces a mixed-format path
    # (C:/msys64/...) that both native Windows processes and MSYS2 agree on.
    if is_windows; then
        run -0 cygpath -m "${BATS_FILE_TMPDIR}/docker-config"
        DOCKER_CONFIG=${output}
    else
        DOCKER_CONFIG="${BATS_FILE_TMPDIR}/docker-config"
    fi
    export DOCKER_CONFIG
    mkdir -p "${DOCKER_CONFIG}"

    # Deliberately skip setup_rdd_control_plane here: `rdd set` internally
    # calls ensureServiceRunning, which is exactly the CLI path we want to
    # exercise — the engine controller is part of the default controller
    # set, so no explicit --controllers selection is needed.

is_windows() returns true for both MSYS2 and WSL2 — bats/helpers/os.bash:13-16 sets OS=windows when /proc/sys/kernel/osrelease contains WSL2, and os.bash:55 provides is_msys() precisely because the two "behave differently from WSL." cygpath is an MSYS2 tool absent under WSL2, so run -0 cygpath ... fails local_setup_file outright there. The rdd() wrapper in bats/helpers/commands.bash already shows the correct pattern — it branches is_msys (uses cygpath) versus WSL (uses wslpath plus a WSLENV adjustment). The same gap applies to DOCKER_HOST="npipe:////./pipe/docker_engine" at line 39 and the meta.json host assertion at lines 791-799: an npipe:// endpoint is meaningful only to a native docker.exe, not a Linux docker client. Before this PR the whole file was skip_on_windows, so removing that guard is what exposed the WSL2 path.

Darwin)
    # OS matches the directory name of the PATH_RESOURCES directory,
    # so uses "darwin" and not "macos".
    OS=darwin
    ;;
Linux)
    if grep --quiet WSL2 </proc/sys/kernel/osrelease; then # spellchecker:ignore
        OS=windows
    else
        OS=linux
    fi
    ;;
MSYS* | MINGW*)
    OS=windows

    fi
}

# Detect MSYS2 environment (MSYS or MINGW shells).
# Both report OS=windows, but behave differently from WSL.
is_msys() {
    [[ "${UNAME}" == MSYS* || "${UNAME}" == MINGW* ]]
}

is_unix() {
    ! is_windows "$@"

    # set, so no explicit --controllers selection is needed.
    rdd svc delete
    rdd set running=true
    if is_windows; then
        export DOCKER_HOST="npipe:////./pipe/docker_engine"
    else
        run -0 rdd svc paths docker_socket
        export DOCKER_HOST="unix://${output}"
    fi
    # Mirror resources live in App.spec.namespace. Override RDD_NAMESPACE
    # to whatever the App was created with so the test queries the same

    assert_file_exists "${meta_file}"

    run -0 jq -r '.Name' "${meta_file}"
    assert_output "${context_name}"

    if is_windows; then
        run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
        assert_output "npipe:////./pipe/docker_engine"
    else
        run -0 rdd service paths docker_socket
        socket_path=${output}
        run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
        assert_output "unix://${socket_path}"
    fi
}

@test "moby engine sets currentContext when no healthy context exists" {
    local context_name="rancher-desktop-${RDD_INSTANCE}"

This is exactly @mook-as's unresolved inline comment ("This wouldn't do the right thing if we're running BATS under WSL, right?"). It is latent rather than CI-breaking only if the project's Windows CI runs MSYS2 exclusively today.

Fix: gate the MSYS2-specific path on is_msys, and decide WSL2 behaviour explicitly — either handle wslpath + WSLENV for DOCKER_CONFIG and the DOCKER_HOST endpoint, or skip under WSL2 if that environment is genuinely unsupported for this suite.

-    if is_windows; then
+    if is_msys; then
         run -0 cygpath -m "${BATS_FILE_TMPDIR}/docker-config"
         DOCKER_CONFIG=${output}
     else
         DOCKER_CONFIG="${BATS_FILE_TMPDIR}/docker-config"
     fi

		errs = append(errs, fmt.Errorf("failed to delete ContainerNamespaces: %w", err))
	}
	if err := errors.Join(errs...); err != nil {
		return err
	}
	// Block until the API server's watch cache reflects the deletions.
	// The Kubernetes watch cache is updated asynchronously: DELETE
	// requests commit to etcd synchronously, but the cache event is
	// processed in a separate goroutine. On slower systems (e.g.
	// Windows CI with SQLite I/O) there is an observable window where
	// `kubectl get` returns a just-deleted resource. Polling here
	// ensures that ContainerEngineReady is only stamped — and `rdd set
	// running=false` only returns — after the cache reflects the clean
	// state, so the immediately-following test assertion passes.
	return r.waitMirrorResourcesGone(ctx)
}

// waitMirrorResourcesGone polls the informer cache until all four mirror
// resource types are empty or the 30-second timeout expires. r.List is
// used (not r.apiReader) so the poll drains the watch cache, not etcd.

	// running=false` only returns — after the cache reflects the clean
	// state, so the immediately-following test assertion passes.
	return r.waitMirrorResourcesGone(ctx)
}

// waitMirrorResourcesGone polls the informer cache until all four mirror
// resource types are empty or the 30-second timeout expires. r.List is
// used (not r.apiReader) so the poll drains the watch cache, not etcd.
func (r *EngineReconciler) waitMirrorResourcesGone(ctx context.Context) error {
	log := logf.FromContext(ctx)
	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
	defer cancel()
	for {

// DockerSocket returns the path to the Docker socket for this instance
// (e.g., ~/.rd2/docker.sock). This is the host-side socket that Lima
// port-forwards from the guest's /var/run/docker.sock.
// On Windows, returns the named pipe path (\\.\pipe\docker_engine).
var DockerSocket = sync.OnceValue(func() string {
	if runtime.GOOS == "windows" {
		return `\\.\pipe\docker_engine`
	}
	return filepath.Join(ShortDir(), "docker.sock")
})

// DockerEndpoint returns the full Docker endpoint URL for this instance.
// On Windows this is a named-pipe URL (npipe:////./pipe/docker_engine);
// on Unix it is a unix-socket URL (unix:///path/to/docker.sock).
var DockerEndpoint = sync.OnceValue(func() string {

	}
	return strings.Join([]string{
		baseTemplate,
		"param:",
		fmt.Sprintf("  CONTAINER_ENGINE: %s", spec.ContainerEngine.Name),
		fmt.Sprintf("  HOST_DOCKER_SOCKET: %q", instance.DockerSocket()),
		fmt.Sprintf("  HOST_HOME_GUEST: %q", toLinuxPath(hostHome)),
		fmt.Sprintf("  KUBERNETES_ENABLED: %v", spec.Kubernetes.Enabled),
		fmt.Sprintf("  KUBERNETES_VERSION: %s", spec.Kubernetes.Version),
		"",
	}, "\n"), nil

    # exercise — the engine controller is part of the default controller
    # set, so no explicit --controllers selection is needed.
    rdd svc delete
    rdd set running=true
    if is_windows; then
        export DOCKER_HOST="npipe:////./pipe/docker_engine"
    else
        run -0 rdd svc paths docker_socket
        export DOCKER_HOST="unix://${output}"
    fi
    # Mirror resources live in App.spec.namespace. Override RDD_NAMESPACE
    # to whatever the App was created with so the test queries the same
    # namespace the engine controller uses, regardless of CRD defaults.
    RDD_NAMESPACE=$(rdd ctl get app app -o jsonpath='{.spec.namespace}')