Skip to content

fix(windows): launch canonicalized batch shims - #293

Open
TreyThomasCodes wants to merge 1 commit into
cortexkit:mainfrom
TreyThomasCodes:fix/windows-canonical-batch-shims
Open

fix(windows): launch canonicalized batch shims#293
TreyThomasCodes wants to merge 1 commit into
cortexkit:mainfrom
TreyThomasCodes:fix/windows-canonical-batch-shims

Conversation

@TreyThomasCodes

@TreyThomasCodes TreyThomasCodes commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • convert canonical Windows DOS and UNC paths to cmd.exe-compatible spellings only at the shared .cmd/.bat launch boundary
  • retain canonical \\?\... paths internally and leave native .exe launches unchanged
  • add regression coverage for canonicalized npm-style shims, spaces, literal % characters, UNC paths, and unsupported verbatim namespaces

Problem

aft-pi 0.55.1 successfully installs yaml-language-server, and the LSP resolver correctly selects its Windows shim from lsp_paths_extra:

binary_path: \\?\C:\Users\...\AppData\Local\aft\lsp-packages\yaml-language-server\node_modules\.bin\yaml-language-server.cmd
binary_source: lsp_paths_extra

However, lsp_paths_extra directories are canonicalized with std::fs::canonicalize, which yields extended-length paths on Windows. AFT then passed that \\?\...\.cmd path unchanged to cmd.exe. cmd.exe and npm's %~dp0 shim logic do not reliably handle that namespace, so the server disconnected during initialization with:

The system cannot find the path specified.

A standalone spawn("yaml-language-server") experiment also returns ENOENT, but that is not AFT's execution path: AFT already probes .cmd, .exe, and .bat in 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 into cmd.exe:

  • \\?\C:\... becomes C:\...
  • \\?\UNC\server\share\... becomes \\server\share\...
  • unsupported namespaces such as \\?\Volume{GUID}\... are preserved rather than accidentally converted into relative paths
  • .exe execution 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 installed yaml-language-server@1.24.0 through AFT's NDJSON configure + lsp_inspect path on Windows.

With the released v0.55.1 binary:

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 resolved binary_path:

binary_path: \\?\C:\...\yaml-language-server.cmd
binary_source: lsp_paths_extra
spawn_status: ok

The unchanged resolved path plus changed launch result isolates the failure to the cmd.exe boundary 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 complete cargo test -p agent-file-tools --lib run 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.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with 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.exe and npm's %~dp0 logic do not handle \\?\ extended-length paths.

Bug Fixes

  • Converts canonical \\?\C:\... and \\?\UNC\... paths to cmd.exe-compatible spellings only at the batch launch boundary.
  • Keeps native .exe launches and internal canonical paths unchanged.
  • Adds regression coverage for npm-style shims, spaces, literal %, UNC paths, and unsupported verbatim namespaces.

Written for commit 4ac445c. Summary will update on new commits.

Review in cubic

Greptile Summary

This PR converts canonical Windows DOS and UNC paths into cmd.exe-compatible spellings at the shared batch-launch boundary while preserving internal canonical paths and unsupported namespaces.

  • Applies conversion only when launching .cmd and .bat files.
  • Leaves native executable launches unchanged.
  • Adds Windows regression coverage for canonical npm-style shims, spaces, percent characters, UNC paths, and unsupported verbatim namespaces.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/aft/src/windows_command.rs Adds narrowly scoped Windows path normalization for batch launches and regression tests covering the intended path forms.

Reviews (2): Last reviewed commit: "fix(windows): launch canonicalized batch..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@TreyThomasCodes
TreyThomasCodes force-pushed the fix/windows-canonical-batch-shims branch from 717f6d2 to 4ac445c Compare September 4, 2026 00:18
@TreyThomasCodes

Copy link
Copy Markdown
Contributor Author

Investigated the macOS and Linux Docker E2E failures from run 33819255950. They are not caused by this Windows-only patch:

  • The PR changes runtime code only under #[cfg(windows)]; macOS/Linux unit and integration suites all passed.
  • The PR's base commit, 577fc75d, failed the same two E2E jobs in its own earlier push run: https://github.com/cortexkit/aft/actions/runs/33810981315
  • In both base and PR failures, OpenCode 1.18.25 timed out at 90 seconds before making any request to aimock (Turns served by mock: 0). The first-scenario plugin log was therefore still empty when the assertion ran.
  • The immediately preceding main run used the same OpenCode 1.18.25 and passed both platforms, serving all eight turns: https://github.com/cortexkit/aft/actions/runs/33806638998
  • The uploaded logs from the failed jobs show the later scenario loading the plugin, spawning AFT, and successfully running read/grep/glob/search/edit/undo, further separating the timeout from this batch-command change.

I do not have permission to rerun upstream Actions jobs directly. I force-updated the one-commit branch without changing its tree (717f6d2f -> 4ac445c6) to trigger a clean rerun: https://github.com/cortexkit/aft/actions/runs/33821253100

@TreyThomasCodes

Copy link
Copy Markdown
Contributor Author

Follow-up from the fresh run (33821253100): this is now even more clearly an upstream CI/runtime flake rather than a regression in PR #293.

  • Linux and macOS again timed out before aimock received a single request (Turns served by mock: 0).
  • Windows E2E independently hung in its pre-test OpenCode warm-up for the full 240 seconds, with empty stdout, empty stderr, and no plugin log. That happens before AFT or this Rust patch is loaded.
  • The unrelated Windows libtest failure is the same timing-sensitive tier1_worker_panic_delivers_failed_to_waiter failure already present on the base commit's run (33810981315): base reported Pending after 359 ms; the rerun reported Pending after 297 ms. The isolated test passes locally in 0.12 s.
  • Every compile/check job and every Linux/macOS/Windows integration shard passed. The Windows bash-permission E2E also passed.

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.

@aft-alfonso

aft-alfonso Bot commented Sep 4, 2026

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant