Skip to content

fix(action-fusion): then_run skipped for Git Bash/MSYS/Cygwin/WSL drive paths on Windows - #35

Open
kaluli123123 wants to merge 1 commit into
NVlabs:mainfrom
kaluli123123:fix/action-fusion-windows-shell-paths
Open

fix(action-fusion): then_run skipped for Git Bash/MSYS/Cygwin/WSL drive paths on Windows#35
kaluli123123 wants to merge 1 commit into
NVlabs:mainfrom
kaluli123123:fix/action-fusion-windows-shell-paths

Conversation

@kaluli123123

@kaluli123123 kaluli123123 commented Sep 13, 2026

Copy link
Copy Markdown

Fixes #45

Summary

On Windows, a fused edit/write whose path comes from a Git Bash, MSYS, Cygwin, or WSL shell mutates one file while Action Fusion's queue and hash guard address another, so then_run is skipped with ENOENT.

resolveToolPath() handles @, file://, ~, and ~/, then calls path.resolve(). Pi's resolveToCwd() does two more things on Windows, in utils/paths.js:

if (process.platform === "win32") {
    normalized = normalizeWindowsShellPath(normalized);   // /c/src -> C:\src
}
if (options.expandTilde ?? true) {
    if (normalized.startsWith("~/") || (process.platform === "win32" && normalized.startsWith("~\\"))) {
        return join(home, normalized.slice(2));
    }
}
export function normalizeWindowsShellPath(filePath) {
    if (!filePath.startsWith("/") || filePath.startsWith("//") || filePath.includes("\\")) return filePath;
    const match = filePath.match(/^\/(?:mnt\/|cygdrive\/)?([a-z])(?:\/(.*))?$/i);
    if (!match) return filePath;
    const suffix = match[2]?.replaceAll("/", "\\");
    return `${match[1].toUpperCase()}:\\${suffix ?? ""}`;
}

So for path: "/c/src/app.ts" the built-in mutation writes C:\src\app.ts, while resolveToolPath() returns a literal \c\src\app.ts for the queue slot and the assertUnchangedBeforeCommand() hash. The mutation succeeds, the guard reads a path that does not exist, and the model gets:

Error: [then_run:skipped] ENOENT: no such file or directory, open '...\c\src\app.ts'; the command was not run.

This is the same parity class as #2 and #7, on a different input class — the file-URL fix in #4 and the Unicode-space work in #7 do not touch shell drive paths. Given #16, Windows users are running this.

Scope of the claim

I do not have a Windows machine, so this is reasoned from Pi 0.84.2's own resolver source rather than executed on Windows, and the tests below drive the platform branch with a stubbed process.platform. Happy to hold this until someone can confirm on a real Windows runner — the upstream code path is quoted above so the divergence itself is checkable without one.

Change

normalizeWindowsShellPath() mirrors Pi's function exactly, including the //-prefixed UNC and already-native (\-containing) exclusions, and the tilde branch accepts ~\ on Windows. Both are no-ops off Windows, so a POSIX user whose repository genuinely contains /c/src/app.ts is unaffected.

docs/compatibility.md gains a sentence in the Action Fusion section next to the existing file-URL note.

Tests

Three added to tests/action-fusion-paths.test.ts, using a withPlatform() helper that swaps process.platform around one call and restores it:

  • under win32: /c/src/app.ts -> C:\src\app.ts, /mnt/d/work/notes.md -> D:\work\notes.md, /cygdrive/e/x/y -> E:\x\y, /c -> C:\, and /usr/local/bin/pi, //server/share/file.txt, C:\already\native.ts left alone;
  • under linux: /c/src/app.ts unchanged, through both the helper and resolveToolPath();
  • ~\notes.txt expands to the home directory under win32 and stays an ordinary relative filename under linux.

Red/green verified — with src/ reverted:

TypeError: normalizeWindowsShellPath is not a function
TypeError: normalizeWindowsShellPath is not a function
AssertionError: expected '/work/~\notes.txt' to be '/Users/…/notes.txt'
Tests  3 failed | 10 passed (13)

With the fix, on the pinned Pi 0.84.2 (macOS):

npx tsc --noEmit          TypeScript: No errors found
npx vitest run            Test Files 18 passed (18) | Tests 142 passed (142)

https://claude.ai/code/session_01Ax76YG2oRYYUvnYMdUdChk

…ed guard

`resolveToolPath()` handled `@`, `file://`, `~`, and `~/`, then went straight to
`path.resolve()`. Pi's own `resolveToCwd()` does more on Windows: it converts
Git Bash, MSYS, Cygwin, and WSL drive paths (`/c/...`, `/mnt/c/...`,
`/cygdrive/c/...`) to native form and expands `~\`. A `then_run` call against
such a path therefore mutated `C:\src\app.ts` while the queue and hash guard
addressed a literal `\c\src\app.ts`, so the fused command was skipped with
ENOENT.

Same parity class as NVlabs#2 and NVlabs#7, different input class: the file-URL fix in NVlabs#4
and the Unicode-space work in NVlabs#7 do not cover shell drive paths.

The conversion mirrors Pi 0.84.2's `normalizeWindowsShellPath()` and is a no-op
off Windows, so POSIX targets like `/c/src/app.ts` keep their current meaning.

Claude-Session: https://claude.ai/code/session_01Ax76YG2oRYYUvnYMdUdChk

@gaoanze888 gaoanze888 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Compared the converter and tilde branch with Pi 0.84.2 path normalization; the pure Windows path semantics match, and the POSIX behavior remains unchanged. npm run check passes locally: 18 files / 142 tests, typecheck, and package dry-run. No blocking correctness issue found. This overlaps #9, so merge resolution must preserve the order Unicode normalization → @ stripping → Windows drive normalization → file URL/tilde/cwd resolution; a real Windows run would still be valuable.

@gaoanze888

Copy link
Copy Markdown

Integration follow-up: I merged this exact head after #9 in a disposable tree that also included the currently approved #6, #8, #13, #14, #15, #17, #23, #26, #30, #31, #33, and #34.

The expected conflict is limited to file-queue.ts. Resolving it as

normalizeWindowsShellPath(normalizeToolPath(filePath))

preserves the intended sequence: Unicode-space normalization → optional @ stripping → Windows drive normalization → file:// / tilde / cwd resolution. With the other complementary conflicts resolved similarly, the combined tree passes npm run check: 21 files / 223 tests, typecheck, and package dry-run.

So this remains compatible with #9; it just needs that explicit single-file resolution if #9 lands first. This is still a macOS integration result, not a substitute for a native Windows run.

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.

Action Fusion skips then_run for Git Bash/MSYS/Cygwin/WSL drive paths on Windows

2 participants