test(windows): guard clean MCP stdio startup - #2217

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Thank you for preserving the startup diagnostics. This overlaps #2213, but this version also carries sibling-runtime and stderr-join coverage. Could you identify one canonical PR retaining those cases and ensuring temporary runtimes are cleaned up on failures? We will keep both open until that handoff is clear; no duplicate closure or extra routine rebase is being requested.

Thank you for this — a native Windows MCP stdio guard for #654 is valuable, and the details here are the ones that matter: an isolated cache and runtime directory, deterministic stderr capture, and rejecting the diagnostic in both English and Spanish rather than assuming an English-locale runner.

First, a housekeeping question. This and #2213 are the same guard, opened thirteen seconds apart. This one looks like the fuller version — +59 vs +57 in tests/windows/test_mcp_stdio.py, plus tests/windows/mcp_stdio.py, plus the Spanish diagnostic handling. I have asked on #2213 which you would like to keep; my guess is this one, but it is your work and your call, so I will not close either until you say.

One real blocker: the dco check. Your commits carry no Signed-off-by trailer — the message is a headline with an empty body, so there is nothing for the check to parse. (Not the blank-line trap, where a sign-off sits outside the trailer block and git's parser ignores it while grep still finds it; here there is genuinely no trailer.)

git commit --amend -s          # single commit
git rebase --signoff origin/main   # several
git push --force-with-lease

Two of the red checks are ours, not yours:

• test / test-msan fails on its Docker image build whenever apt.llvm.org is flaky.
• test / test-windows-guards is known to redden PRs it has nothing to do with — confirmed on three unrelated ones.

Please ignore both. I track them separately and neither will block this.

On the Spanish diagnostic specifically: matching localized runner output is the sort of thing that usually gets discovered the hard way, months later, on someone else's machine. Thank you for getting there first.

Thanks again for the Windows startup coverage. The outstanding coordination question is still which of #2217 and #2213 should carry the final guard, retaining deterministic stderr capture and the localized diagnostic case. Please confirm your preferred PR; the recorded sign-off request applies to the chosen branch.

Thank you @Shinken009 for the focused cleanup suggestion and the clear native-Windows evidence and limits, and @lorenzozanee for the startup guard. Keeping cache and runtime under one owned temporary root addresses the reported test-cleanup gap without widening scope. We still need to settle which of #2213/#2217 carries the final guard and review the suggestion there. Sorry for the delay on our follow-up—I’ll follow up soon.

Thank you @lorenzozanee for this guard. You got the parts that matter right: an isolated cache and runtime directory so the check never touches a real install, joining the stderr reader on close so the captured output is complete, and checking for the path-not-found message in both English and Spanish instead of assuming an English-locale runner.

We added one small commit on top of your branch (76adece) that applies @Shinken009's inline suggestion from Oct 22. Thank you, @Shinken009, for the careful native-Windows check and the precise fix. The runtime directory used to be created next to the TemporaryDirectory (cache + "-runtime"), so it was never removed when the context exited and its lock files stayed behind in %TEMP%. Cache and runtime now live as two subdirectories under one owned temporary root, so both are cleaned up together, including when the test fails. Nothing else changes: same MCP exchange, same daemon stop, same diagnostic checks. We are treating this PR as the canonical one of the two, since it carries the fuller version.

One step only you can do: dco stays red because your commit 193c60d has no Signed-off-by line, and we cannot add a sign-off on your behalf. Could you pull our commit and sign off both commits?

git pull --ff-only
git rebase --signoff HEAD~2
git push --force-with-lease

Once dco is green we can move this forward. Thanks again to you both.

Thank you, @lorenzozanee, for the Windows startup guard, and thank you again, @Shinken009, for the cleanup fix and the native-Windows checks behind it.

The parts you got right are the parts that make this guard trustworthy: an isolated cache and runtime so it never touches a real install, joining the stderr reader before the output is inspected so a late message cannot slip past, and checking the Spanish message as well as the English one instead of assuming an English-locale machine.

It has now landed in #2952 (merge ff812bf), with both of you credited as co-authors on the commit. We took that route because the dco check needs a sign-off on the original commit that only its author can add. On top of your version and the one-temp-root cleanup, the guard now fails setup if the stderr reader never finishes, instead of passing on partial output, and it also rejects any raw, non-log line on stderr, so a message in the next language is caught too. It runs in the Windows guards job on every pull request.

Closing this one in favor of #2952. #654 itself stays open for a check against a released binary. Thanks to you both.