fix(windows): launch canonicalized batch shims - #293
Conversation
717f6d2 to
4ac445c
Compare
|
Investigated the macOS and Linux Docker E2E failures from run 33819255950. They are not caused by this Windows-only patch:
I do not have permission to rerun upstream Actions jobs directly. I force-updated the one-commit branch without changing its tree ( |
|
Follow-up from the fresh run (
Because the failures occur before plugin loading and reproduce on the exact base commit, I am not adding unrelated E2E/test-timing changes to this Windows shim-launch PR. An upstream maintainer will need to rerun the failed jobs once the OpenCode startup issue clears. |
|
Agreed on the diagnosis, from our side of the wall: main's own push run (33810981315) failed the same two E2E legs on the same first-scenario shape - OpenCode 1.18.25 never reached aimock (0 turns), empty plugin log, later scenarios healthy - 40 minutes after an identical suite went green on 33806638998 with the same OpenCode build. That is a first-launch startup stall on the runner, not this patch (which is cfg(windows) and only crosses into cmd.exe at the batch launch boundary). The Windows libtest failure is ours too: tier1_worker_panic_delivers_failed_to_waiter used a 250 ms soft deadline for a test about panic delivery, not timing; fixed on main in 5258861. Review of the change itself: the boundary is the right one (canonical paths stay internal, conversion only where cmd.exe sees the path), the UNC host+share guard and the refusal to strip \?\Volume{GUID}\ are the parts I would have asked for, and the canonicalized npm-shim test exercises real cmd.exe. One note for us, not for this PR: AFT already carries two private windows_non_verbatim_path copies (inspect/job.rs, oxc_engine/resolver.rs) that strip the prefix unconditionally; your stricter form is the one they should converge on, and I will unify them in a follow-up rather than ask you to widen this patch. Merging once the E2E rerun on main confirms the stall has cleared, so the merge lands on a green base. |
Summary - convert canonical Windows DOS and UNC paths to![View with [code]smith](https://camo.githubusercontent.com/6e22944985996f6a1201f1d35788e9dc855f2e6b7b781e7ed1c22d7fb14217e8/68747470733a2f2f70722d636f6d6d656e74732d6173736574732e626c61636b736d6974682e73682f636f6465736d6974682f766965772d776974682d636f6465736d6974682d6461726b2d76322e737667)
![Autofix with [code]smith](https://camo.githubusercontent.com/8e7d836069a9e09b18eb0f68d9afc9beb6c2b13211745830b36a5d88a500d653/68747470733a2f2f70722d636f6d6d656e74732d6173736574732e626c61636b736d6974682e73682f636f6465736d6974682f6175746f6669782d776974682d636f6465736d6974682d6461726b2e737667)
Need help on this PR? Tag
cmd.exe-compatible spellings only at the shared.cmd/.batlaunch boundary - retain canonical\\?\...paths internally and leave native.exelaunches unchanged - add regression coverage for canonicalized npm-style shims, spaces, literal%characters, UNC paths, and unsupported verbatim namespaces ## Problemaft-pi0.55.1 successfully installsyaml-language-server, and the LSP resolver correctly selects its Windows shim fromlsp_paths_extra:text binary_path: \\?\C:\Users\...\AppData\Local\aft\lsp-packages\yaml-language-server\node_modules\.bin\yaml-language-server.cmd binary_source: lsp_paths_extraHowever,lsp_paths_extradirectories are canonicalized withstd::fs::canonicalize, which yields extended-length paths on Windows. AFT then passed that\\?\...\.cmdpath unchanged tocmd.exe.cmd.exeand npm's%~dp0shim logic do not reliably handle that namespace, so the server disconnected during initialization with:text The system cannot find the path specified.A standalonespawn("yaml-language-server")experiment also returnsENOENT, but that is not AFT's execution path: AFT already probes.cmd,.exe, and.batin configured directories before falling back to the extensionless file. Changing PATH fallback resolution would therefore not fix this failure. ## Why this boundary is correct The canonical path remains useful for internal identity and filesystem operations. The compatibility conversion belongs specifically where AFT crosses intocmd.exe: -\\?\C:\...becomesC:\...-\\?\UNC\server\share\...becomes\\server\share\...- unsupported namespaces such as\\?\Volume{GUID}\...are preserved rather than accidentally converted into relative paths -.exeexecution does not use this batch launcher and is unaffected This also fixes formatter and other npm batch shims that use the same shared launcher. ## Reproduction and proof I exercised the actual installedyaml-language-server@1.24.0through AFT's NDJSONconfigure+lsp_inspectpath on Windows. With the released v0.55.1 binary:text binary_path: \\?\C:\...\yaml-language-server.cmd binary_source: lsp_paths_extra spawn_status: spawn_failed stderr: The system cannot find the path specified.With this branch, using the same project, cache directory, server package, and resolvedbinary_path:text binary_path: \\?\C:\...\yaml-language-server.cmd binary_source: lsp_paths_extra spawn_status: okThe unchanged resolved path plus changed launch result isolates the failure to thecmd.exeboundary and verifies the fix without changing resolver precedence. ## Tests -cargo test -p agent-file-tools --lib windows_command— 6 passed -cargo test -p agent-file-tools --lib lsp::registry::tests— 42 passed -cargo test -p agent-file-tools --lib lsp::client::tests— 9 passed -cargo test -p agent-file-tools --lib run_external_tool_invokes_npm_style_batch_shim— passed -cargo check -p agent-file-tools --lib— passed -cargo fmt --check— passed A completecargo test -p agent-file-tools --librun reached 2,829 passing tests. Five unrelated background-registry failures caused by parallel shared-artifact collisions all passed when rerun individually; the remaining three failures require Windows symlink privileges unavailable in this environment. ---@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes Windows batch shim launches failing with "The system cannot find the path specified." when the shim path comes from a canonicalized directory, because
cmd.exeand npm's%~dp0logic do not handle\\?\extended-length paths.Bug Fixes
\\?\C:\...and\\?\UNC\...paths tocmd.exe-compatible spellings only at the batch launch boundary..exelaunches and internal canonical paths unchanged.%, UNC paths, and unsupported verbatim namespaces.sleep 2in the pending bash push-frame test with a gated child so the deferred response can't raceconfigure_warningsunder CI load.Written for commit c83985d. Summary will update on new commits.
Closes #301
Greptile Summary
The PR converts canonicalized Windows DOS and UNC paths into
cmd.exe-compatible spellings at the shared batch-launch boundary while preserving internal canonical paths and native executable behavior.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (5): Last reviewed commit: "test: wait for persisted GC before resta..." | Re-trigger Greptile