Deep Review: 20260726-220537-pr-5337

		if err != nil {
			return nil, err
		}
		// Report a bad path as itself; IsAvailableOnGuest would reduce it to
		// "rsync not available on guest(s)".
		if _, err := parseCopyPaths(ctx, paths); err != nil {
			return nil, err
		}
		if !rsync.IsAvailableOnGuest(ctx, paths) {
			return nil, errors.New("rsync not available on guest(s)")
		}
		return rsync, nil
	case BackendAuto:

Discarding the result shows the call exists for validation alone.

store.Inspect costs more than a file read. For a running instance it loads and validates the YAML, then dials the hostagent socket for an Info RPC under a three-second timeout (pkg/store/instance.go:71-86).

	if err != nil {
		inst.Status = limatype.StatusBroken
		inst.Errors = append(inst.Errors, err)
	}

	if inst.HostAgentPID != 0 {
		haSock := filepath.Join(instDir, filenames.HostAgentSock)
		haClient, err := hostagentclient.NewHostAgentClient(haSock)
		if err != nil {
			inst.Status = limatype.StatusBroken
			inst.Errors = append(inst.Errors, fmt.Errorf("failed to connect to %#q: %w", haSock, err))
		} else {
			ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
			defer cancel()
			info, err := haClient.Info(ctx)
			if err != nil {
				inst.Status = limatype.StatusBroken
				inst.Errors = append(inst.Errors, fmt.Errorf("failed to get Info from %#q: %w", haSock, err))
			} else {
				inst.SSHLocalPort = info.SSHLocalPort
				inst.AutoStartedIdentifier = info.AutoStartedIdentifier
			}
		}
	}
	if inst.SSHLocalPort == 0 {
		sshConfigPath := filepath.Join(instDir, filenames.SSHConfig)

The same paths are parsed again inside IsAvailableOnGuest (pkg/copytool/rsync.go:37) and a third time in Command. Before this change the explicit-rsync flow parsed twice, so it gains one round trip per remote path, and limactl shell with a synced workdir pays it on every invocation. The auto branch is unchanged at three.

func (t *rsyncTool) Name() string {
	return t.toolPath
}

func (t *rsyncTool) IsAvailableOnGuest(ctx context.Context, paths []string) bool {
	copyPaths, err := parseCopyPaths(ctx, paths)
	if err != nil {
		// New() has already reported this to the user.
		logrus.Debugf("failed to parse copy paths for rsync availability check: %v", err)
		return false
	}

Fix: parse once in New and hand copyPaths to an unexported availability helper.

func New(ctx context.Context, backend string, paths []string, opts *Options) (CopyTool, error) {
	switch Backend(backend) {
	case BackendSCP:
		return newSCPTool(opts)
	case BackendRsync:
		rsync, err := newRsyncTool(opts)
		if err != nil {
			return nil, err
		}
		// Report a bad path as itself; IsAvailableOnGuest would reduce it to
		// "rsync not available on guest(s)".
		if _, err := parseCopyPaths(ctx, paths); err != nil {
			return nil, err
		}
		if !rsync.IsAvailableOnGuest(ctx, paths) {
			return nil, errors.New("rsync not available on guest(s)")
		}

newRsyncTool runs exec.LookPath("rsync") before the new validation, so on a host without rsync the exact failure the commit title names survives. BackendSCP never validates at all and still defers to Command.

Both messages are true, so the user fixes one problem at a time. That is why this sits below the auto-backend case rather than beside it.

Gemini proposed hoisting the parse above the whole switch. That trades one masked error for another: --backend is an unvalidated string (cmd/limactl/copy.go:56) and the default: arm is its only rejection point, so a bad path would then hide invalid backend. Moving the parse up within the rsync arm avoids that.

		GroupID: advancedCommand,
	}

	copyCommand.Flags().BoolP("recursive", "r", false, "Copy directories recursively")
	copyCommand.Flags().BoolP("verbose", "v", false, "Enable verbose output")
	copyCommand.Flags().String("backend", "auto", "Copy backend (scp|rsync|auto)")

	return copyCommand
}

func copyAction(cmd *cobra.Command, args []string) error {

Fix: move the parseCopyPaths block above newRsyncTool.

		return rsync, nil
	case BackendAuto:
		// For rsync, the source and destination cannot both be remote
		bothRemote, err := hasRemoteSourceAndDestination(ctx, paths)
		if err != nil {
			// A bad path is fatal for every backend, so report it here. Falling
			// through to scp would replace it with "scp not found on host".
			return nil, err
		}
		if !bothRemote {
			rsync, err := newRsyncTool(opts)
			if err == nil {

Falling through replaced the message only when scp was absent. With scp present, New returned an scp tool and Command re-parsed the same paths and produced the correct text.

I reproduced this at the merge-base: New() returned &{/usr/bin/scp …}, nil, and Command() returned instance `nonexistent-instance-for-test` does not exist, run `limactl create …` . So on a typical host the change moves the report earlier and leaves the wording alone. The commit body's "On a host with scp the outcome was worse" reads the same way, and a git log reader will take it as the message having been wrong everywhere.

Fix: name both outcomes, e.g. "Falling through to scp defers the report to Command, and replaces it entirely when scp is missing." Amend the commit body to match.

// path as itself, rather than as a missing copy tool.
func TestNewAutoSurfacesPathError(t *testing.T) {
	t.Setenv("LIMA_HOME", t.TempDir())
	paths := []string{"nonexistent-instance-for-test:/tmp/x", "/tmp/y"}

	_, err := New(t.Context(), string(BackendAuto), paths, &Options{})
	assert.ErrorContains(t, err, "instance `nonexistent-instance-for-test`")
}

The test covers BackendAuto and fails against the merge-base for the right reason. Two other behaviors this commit introduces stay unpinned. One is the BackendRsync validation at copytool.go:67-69, the other the reworded fallback at copytool.go:98. Reordering or deleting either leaves the suite green.

		if err != nil {
			return nil, err
		}
		// Report a bad path as itself; IsAvailableOnGuest would reduce it to
		// "rsync not available on guest(s)".
		if _, err := parseCopyPaths(ctx, paths); err != nil {
			return nil, err
		}
		if !rsync.IsAvailableOnGuest(ctx, paths) {
			return nil, errors.New("rsync not available on guest(s)")
		}
		return rsync, nil
	case BackendAuto:

		tool, err := newSCPTool(opts)
		if err != nil {
			// rsync may well have been found and rejected above, so name the
			// outcome rather than guessing which tool is missing.
			return nil, fmt.Errorf("no usable copy tool on host: %w", err)
		}
		return tool, nil
	default:
		return nil, fmt.Errorf("invalid backend %#q, must be one of: scp, rsync, auto", backend)
	}

Fix: add a BackendRsync sibling asserting the same instance substring, following the t.Skip("rsync not found:", err) idiom the file already uses at lines 19 and 35. A t.Setenv("PATH", t.TempDir()) case would pin the fallback wording.

		tool, err := newSCPTool(opts)
		if err != nil {
			// rsync may well have been found and rejected above, so name the
			// outcome rather than guessing which tool is missing.
			return nil, fmt.Errorf("no usable copy tool on host: %w", err)
		}
		return tool, nil
	default:
		return nil, fmt.Errorf("invalid backend %#q, must be one of: scp, rsync, auto", backend)
	}

Dropping the old guess is right. But the branch knows which case it hit. Lines 88 and 90 already distinguish "rsync rejected by the guest" from "rsync absent from the host", and log it at Debugf only.

On a host with neither tool the user now reads no usable copy tool on host: scp not found on host: …, which no longer hints that installing rsync would also work. That is the one configuration where the old wording was accurate.

Fix: carry the reason, e.g. fmt.Errorf("no usable copy tool on host (rsync: %s): %w", rsyncReason, err).