Deep Review: 20260727-195856-pr-5341

Date2026-07-27 19:58
Repolima-vm/lima
Round3 (of target)
Author@jandubois
PR#5341 — docs: document the Windows host SSH toolchain
Branchdocs-plain-windows-wsl2
Commitsdf1997c1 docs: document the Windows host SSH toolchain
Review SHAdf1997c15eac7201fe06af79caf30586fda9349b
ReviewersClaude Opus 5 (effort: xhigh), Codex GPT 5.6 Sol (effort: xhigh), Gemini 3.1 Pro (effort: default), Gemini 3.5 Flash (effort: default)
VerdictMerge as-is — (corrected). I1 and I2 are Suggestions, not Important; merge after #5299 and #5300. See Resolution.
Wall-clock time26 min 48 s

Executive Summary

Round 3 audits the nine round-2 fixes. Seven hold under execution: the shell-tokenisation rules, the PowerShell example, the sftpDriver pin, the builtin exemption, the QEMU mount-type claim, the installation bullet, and all four cross-reference anchors. Two defects remain, both in rewritten wsl2.md paragraphs. The fstab note keeps a remedy its new mechanism cannot produce, and the SSH/PATH warning promises a fast failure where the code stalls ten minutes.

All four reviewers produced output. Claude carried the round with both Important findings and three suggestions; Codex independently reached I1 and re-raised a claim round 2 had already refuted; Gemini 3.1 Pro returned clean; Flash returned one phrasing note. I ran the parser probe, built the site, traced every claim through pkg/sshutil, pkg/hostagent, and sshocker v0.3.11, and rejected two agent findings that verification did not support.

Critical Issues

None.

Important Issues

I1. The fstab note keeps a remedy its rewritten mechanism cannot produce Claude Opus 5 Codex GPT 5.6 Sol
website/content/en/docs/config/vmtype/wsl2.md
message at start and falls back to `sshocker`'s own detection; a missing
OpenSSH.Server install on a native-only host typically shows up there.

Note: Lima converts the host path with the selected toolchain on every
start, and `sshocker` maps it back to a drive letter from the leading
`/x/` segment without consulting any `fstab`. A custom MSYS2 `fstab` that
remaps drive prefixes (rare) therefore survives the round trip only by
coincidence, so avoid changing toolchains between starts of an existing
QEMU instance with a reverse-sshfs mount on such a host. A mount pinned to
`sftpDriver: builtin` is exempt, because `sshocker` serves it from the
native path.

Separately, a mount without an explicit `mountPoint` takes its guest path
from whichever `cygpath` `PATH` resolves, re-derived on every load rather

The round-2 rewrite replaced the mechanism and kept the conclusion. Nothing about the conversion persists across starts: pkg/instance/create.go:78 writes the raw config bytes, and mountPathAndSftpServer re-runs PathForSSH on every start. Changing toolchains is therefore not the trigger.

pkg/instance/create.go
		if !createSucceeded {
			_ = os.RemoveAll(instDir)
		}
	}()

	if err := os.WriteFile(filePath, instConfig, 0o644); err != nil {
		return nil, err
	}
	if err := os.WriteFile(filepath.Join(instDir, filenames.LimaVersion), []byte(version.Version), 0o444); err != nil {
		return nil, err
	}

"Only by coincidence" overstates the hazard. convertMSYS2Path rewrites a path only when byte 2 is /, so /cygdrive/c/… and /mnt/c/… pass through untouched and the paired Cygwin sftp-server resolves them correctly. Only a remap onto a different single-letter prefix misroutes, and that one breaks on every start, not just after a toolchain change.

Fix: name the single-letter-prefix condition, say it fails on every start, and drop the cross-start advice.

I2. An SSH/PATH mismatch stalls for ten minutes, and mounts are still attempted Claude Opus 5 Gemini 3.5 Flash
website/content/en/docs/config/vmtype/wsl2.md
whatever `ssh` `PATH` yields, complete or not.

The `SSH` environment variable overrides that search, but it does not
replace it. The hostagent still runs the `ssh` that `PATH` resolves, while
the key and socket paths it is handed take the form of whichever toolchain
`SSH` names, so the two must name the same install. Naming a Cygwin
toolchain in `SSH` while native OpenSSH leads `PATH` fails `limactl start`
at its readiness check, before any mount. Other helpers convert
paths with the `cygpath` that `PATH` resolves, regardless of `SSH`. Fixing
`PATH` is the reliable route.

Lima detects which ssh toolchain is in use on each `limactl start` and
takes both the path form and the `sftp-server` binary from that

The ssh essential requirement sets noMaster but never fatal (pkg/hostagent/requirements.go:216-226), so waitForRequirements retries it 200 times at three seconds, about ten minutes, logging nothing per attempt. limactl start's own deadline is also ten minutes (DefaultWatchHostAgentEventsTimeout). The user sees one "Waiting for the essential requirement" line and then silence, not a readiness error.

pkg/hostagent/requirements.go
}

func (a *HostAgent) essentialRequirements() []requirement {
	req := make([]requirement, 0)
	req = append(req,
		requirement{
			description: "ssh",
			script: `#!/bin/sh
true
`,
			debugHint: `Failed to SSH into the guest.
Make sure that the YAML field "ssh.localPort" is not used by other processes on the host.
If any private key under ~/.ssh is protected with a passphrase, you need to have ssh-agent to be running.
`,
			noMaster: true,
		})
	startControlMasterReq := requirement{
		description: "Explicitly start ssh ControlMaster",
		script: `#!/bin/sh
true
`,

"Before any mount" is wrong too. pkg/hostagent/hostagent.go:591 appends the error and falls through, so setupMounts still runs at line 619 and adds its own failures.

pkg/hostagent/hostagent.go
	essentialRequirements := a.essentialRequirements()
	if *a.instConfig.OS == limatype.WINDOWS {
		essentialRequirements = a.essentialWinRequirements()
	}

	if err := a.waitForRequirements("essential", essentialRequirements); err != nil {
		errs = append(errs, err)
	}

	// Windows guest needs a marker file which shows all installation is done.
	// In the next boot, Lima will not mount OS installer ISO file to prevent unexpected re-installation.

Fix: say limactl start stalls at the readiness check for about ten minutes and then fails alongside mount errors. Recast "…leads PATH fails…" as "causes limactl start to fail", which reads without a garden path.

Suggestions

S1. The SSH entry's Default line contradicts the selection order its sibling page documents Claude Opus 5
website/content/en/docs/config/environment-variables.md
  on its scp backend, so a path with no arguments is the reliable form.
  Tokenization also reads `\` as an escape and splits on spaces. A Windows
  path therefore needs quotes, and its backslashes survive only inside single
  quotes; unquoted, `C:\Program Files\Git\usr\bin\ssh.exe` arrives as
  `C:Program`.
- **Default**: unset. Lima then searches `$PATH`, and on Windows falls back to
  `%SystemRoot%\System32\OpenSSH`.
- **Usage**:
  ```sh
  export SSH=/opt/homebrew/bin/ssh
  ```
  ```powershell

On Windows Lima does not simply search $PATH for ssh. pickCompleteSSHOnWindows walks PATH for a directory holding ssh.exe, scp.exe, and ssh-keygen.exe together and skips partial installs such as MinGit; only then does it try %SystemRoot%\System32\OpenSSH, and only when that also fails does NewSSHExe fall through to exec.LookPath("ssh"). A reader whose partial install is first on PATH would expect it to win.

wsl2.md:107-110 states the same order correctly, so the two pages disagree.

website/content/en/docs/config/vmtype/wsl2.md
`PATH` ahead of any other directory holding an `ssh.exe`. The Git
installer's default setting adds only `cmd\`, which holds none of those
binaries, and `usr\bin` also carries `find.exe` and `sort.exe`, which
shadow the Windows commands of those names.

Order matters because Lima takes the first directory on `PATH` that holds
`ssh.exe`, `scp.exe`, and `ssh-keygen.exe` together. When no directory
qualifies it tries `%SystemRoot%\System32\OpenSSH`, and failing that
whatever `ssh` `PATH` yields, complete or not.

The `SSH` environment variable overrides that search, but it does not
replace it. The hostagent still runs the `ssh` that `PATH` resolves, while
the key and socket paths it is handed take the form of whichever toolchain
`SSH` names, so the two must name the same install. Naming a Cygwin

Fix: give the three-step order here, or replace the sentence with a pointer to the toolchain section that already has it.

S2. qemu.md scopes a Lima-wide OpenSSH requirement to the QEMU driver Claude Opus 5
website/content/en/docs/config/vmtype/qemu.md

Recommended QEMU version:
- v8.2.1 or later (macOS)
- v6.2.0 or later (Linux)

On a Windows host, "qemu" needs an OpenSSH installation for its own `ssh`,
`scp`, and `ssh-keygen`. Mounts there must be reverse-sshfs, because QEMU
for Windows has no 9p support, and reverse-sshfs is the only thing in Lima
that uses `sftp-server`. See
[Windows toolchain]({{< ref "/docs/config/vmtype/wsl2#windows-toolchain" >}})
for which binaries to install and how Lima chooses between them.

"its own" has no referent, since none of the three binaries belong to QEMU. The requirement is not driver-specific either: ssh-keygen runs from sshutil.DefaultPubKeys on every limactl start, and scp.exe gates toolchain selection for every command through pickCompleteSSHOnWindows.

wsl2.md:84-85 says the same three binaries "cover the WSL2 driver", so a reader comparing the pages meets a contradiction. The 9p half of the sentence is correct.

website/content/en/docs/config/vmtype/wsl2.md
### Windows toolchain

Lima uses an OpenSSH installation on the host. On a default Windows
install that is the native binaries in `C:\Windows\System32\OpenSSH\`:

- **OpenSSH Client** (`ssh.exe`, `scp.exe`, `ssh-keygen.exe`) ships by default
  on Windows 10 and Windows 11, and covers the WSL2 driver.
- **`sftp-server.exe`** is part of OpenSSH Server, an [optional Feature on Demand](https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse).
  Only the QEMU driver's reverse-sshfs mounts use it, and they treat it as a
  preference: with no `sftp-server` on the host, `sshocker` serves the mount
  in-process instead. Pinning `sftpDriver: openssh-sftp-server` drops that
  in-process fallback, so the mount fails if the search below turns up

Fix: attribute ssh, scp, and ssh-keygen to Lima, and keep only sftp-server as the QEMU-specific addition.

S3. The SSH note calls a mismatch an inconsistency where wsl2.md calls it a start failure
website/content/en/docs/config/environment-variables.md
  $env:SSH = "'C:\Program Files\Git\usr\bin\ssh.exe'"
  ```
- **Note**: On Windows this overrides the toolchain detection described under
  [Windows toolchain]({{< ref "/docs/config/vmtype/wsl2#windows-toolchain" >}}).
  Lima pairs the path form and the `sftp-server` binary with whichever `ssh`
  this names, but some paths still follow `PATH`, among them the guest mount
  point and `limactl shell`'s working directory. Point `SSH` at the same
  install that comes first on `PATH` to keep them consistent.
  `limactl guest-install` ignores this variable altogether.

The advice is right, but "to keep them consistent" prices the failure as a cosmetic mismatch of mount point and working directory. Per I2 the same mismatch stalls limactl start for ten minutes and then fails, because the hostagent runs a PATH-resolved ssh against key paths formatted for the SSH-named toolchain. A reader who meets SSH on this page alone gets no warning that start breaks.

Keep "among them": limactl copy is a third PATH-following consumer on the #5300 branch (pkg/copytool/copytool.go:127), so the list is genuinely partial.

pkg/copytool/copytool.go
	for _, path := range paths {
		cp := &Path{}
		if runtime.GOOS == "windows" {
			if filepath.IsAbs(path) {
				var err error
				path, err = fsutil.WindowsSubsystemPath(ctx, path)
				if err != nil {
					return nil, err
				}
			} else {
				path = filepath.ToSlash(path)

Fix: state the consequence, and link #windows-toolchain for the mechanism.

S4. The replaced Known Issues bullet drops the remedy the old one carried Claude Opus 5
website/content/en/docs/config/vmtype/wsl2.md

### Known Issues
- "wsl2" currently doesn't support many of Lima's options. See [this file](https://github.com/lima-vm/lima/blob/master/pkg/wsl2/wsl_driver_windows.go#L19) for the latest supported options.
- When running lima using "wsl2", `${LIMA_HOME}/<INSTANCE>/serial.log` will not contain kernel boot logs
- WSL2 requires a `tar` formatted rootfs archive instead of a VM image. Standard VM disk images (like `.qcow2`, `.raw`, etc.) or `.squashfs` images cannot be natively imported by WSL2.
- Lima unpacks a `.tar.gz` rootfs on its own, but a `.tar.xz`, `.tar.bz2`, or `.tar.zst` one needs the matching `xz`, `bzip2`, or `zstd` binary on your `PATH`, and Windows ships none of them.

### Rootfs Image Requirements & Building Custom Images

WSL2 does not run a standard virtual machine disk image directly. Instead, `wsl.exe` imports a guest root filesystem from a `.tar` or `.tar.gz` archive.

The claim is correct: pkg/downloader falls back to an in-process decoder for gzip alone and errors for the rest. But the bullet this replaced told the reader what to install, and the replacement ends at the problem. Only custom rootfs images reach it, since the shipped templates/experimental/wsl2.yaml uses .tar.gz.

Fix: name where the three binaries come from. Do not point at #windows-toolchain without checking it first: that section covers the ssh toolchain, and whether Git for Windows usr\bin ships xz.exe and zstd.exe is unverified here.

Design Observations

Concerns

Strengths

Testing Assessment

Documentation-only, so no automated coverage is expected. What I executed on the review SHA:

Not executed: no Windows host was available, so the ten-minute stall in I2 is traced from retries = 200 and sleepDuration = 3 * time.Second rather than observed. Codex and Claude both reported building the site independently, and Gemini 3.1 Pro and Flash reported the same; I did not take those on trust and ran the build myself.

No CI job builds this site. test.yml carries paths-ignore: website/**, and the lychee step in lint-quick.yml checks README.md alone, so the two external links in the new text are never validated.

Documentation Assessment

The section reads well and is pitched at a user. The two Important findings are both in text the round-2 fixes introduced, which is the same shape round 2 reported against round 1: each round's fixes are the next round's highest-risk surface. The fstab note has now been rewritten twice and is wrong in a different way each time, which argues for cutting it to the one condition that actually breaks rather than rewriting it again.

One claim in the diff has neither in-tree support nor a way to check it here: wsl2.md:100 says MinGit omits cygpath.exe. Round 2 verified this against the shipped MinGit zip, so it is confirmed, but nothing in the repository records that.

Commit Structure

One commit, correctly scoped, DCO-signed. The message covers the toolchain documentation and the decompressor bullet. It does not mention the net-new installation/_index.md prerequisite bullet, which the generic "Document what to install" clause covers only loosely.

Acknowledged Limitations

Declined in prior rounds

Unresolved Feedback

None. The PR carries no review comments from other reviewers.

Agent Performance Retro

Claude Opus 5

Carried the round. Both Important findings are its own tracing, and I1 is the sharper one: it saw that the round-2 rewrite swapped the mechanism while keeping the conclusion, then went past the defect to bound it, showing that only a single-letter prefix misroutes while /cygdrive/ and /mnt/ pass through untouched. It was also the only agent to audit text outside the round-3 delta, which is where three of its four suggestions came from. Against that, its claim that two call sites exhaust the PATH-following consumers was wrong, and repeating its proposed edit would have made the doc incorrect. It also re-raised the _LIMA_WINDOWS_EXTRA_PATH suggestion round 2 declined, apparently without reading the decline it was given.

Codex GPT 5.6 Sol

The only other agent to find I1, and it arrived by a different route: it constructed the /x/c/Users/lima case and ran it against the pinned sshocker, where Claude reasoned from the guard condition. Two independent paths to one defect is the strongest signal the round produced. Its I1 repeated round 2's OpenSSH Client claim under better citations than last time, and the troubleshooting page it found genuinely does name both features and genuinely does exclude Windows 10 and 11. The manufacturing doc that separates Client from Server still outranks it. The same model producing the same false positive on the same sentence two rounds running is a pattern worth naming.

Gemini 3.1 Pro

Returned clean on all four files after doing real work: it traced the readiness-check failure through the identity-file path and derived the ten-minute retry loop that half of I2 now rests on. Then it filed that derivation under Documentation Assessment as evidence the page is correct, reasoning that the doc says the check fails and the check does fail. The gap it stepped over is between "the mechanism works as described" and "the description tells the user what they will see". This is the second round running where it built something real and reported nothing from it.

Gemini 3.5 Flash

Returned APPROVE with one phrasing note, its first finding across three rounds, and the note is fair: the sentence puts "leads PATH fails" adjacent with no punctuation, and its fix folded into I2. Its Documentation Assessment then claims the prose "perfectly aligns" with implementation files including pkg/fsutil — a real package, but one with no bearing on the sentences it was assessing. It also reported "no broken links" from a Hugo build that validates neither fragments nor external URLs.

Summary

Claude Opus 5Codex GPT 5.6 SolGemini 3.1 ProGemini 3.5 Flash
Duration17m 27s10m 10s6m 39s14m 58s
Findings2I 3S1Inone1I
Tool calls72 (Bash 65, Read 7)44 (shell 33, run 5, plan 3)60 (runshellcommand 58, write_file 2)68 (grepsearch 22, readfile 21, web_fetch 9)
Design observations1202
False positives1100
Unique insights4111
Files reviewed4444
Coverage misses0111
Totals2I 3S1Inone1I
Downgraded0000
Dropped2000

Both Gemini models again concluded the documentation is correct, one silently and one with a grammar note. They share a model family, so count that as one opinion rather than two, and note it is the third round running that Flash has returned APPROVE against text carrying real defects.

Claude gave the most value; Codex gave the round's best-evidenced single finding and its only repeat false positive. The two Important findings both came from auditing round-2 fixes rather than from the round-3 delta, which is what the prior-round context was injected to produce.

Reconciliation. Codex's S1 (fstab remedy) was promoted to I1 and merged with Claude's I1, which covered the same defect with a tighter bound on when it triggers. Flash's S1 (phrasing) was merged into I2 as part of its fix rather than kept separate, since one edit resolves both. Codex's I1 (OpenSSH Client not installed by default) was dropped as a false positive for the second consecutive round, after I read all three Microsoft pages in play. Claude's S3 (_LIMA_WINDOWS_EXTRA_PATH) was dropped under the prior-round decline, verified still valid. Claude's recommendation to make the PATH-following list exhaustive was dropped as factually wrong. S3 came from consolidation rather than any agent.

Review Process Notes

Skill improvements

Repo context updates