Skip to content

fix(windows): install Claude Code hook shims as .cmd wrappers#926

Closed
jseramn wants to merge 1 commit into
DeusData:mainfrom
jseramn:fix/windows-hook-cmd-wrappers
Closed

fix(windows): install Claude Code hook shims as .cmd wrappers#926
jseramn wants to merge 1 commit into
DeusData:mainfrom
jseramn:fix/windows-hook-cmd-wrappers

Conversation

@jseramn

@jseramn jseramn commented Jul 7, 2026

Copy link
Copy Markdown

Summary

  • On Windows, Cursor and Claude Code spawn hook commands without a shell. Extensionless bash shims in ~/.claude/hooks/ trigger the "Open with" dialog and block workflows (especially on every Grep/Glob PreToolUse hook).
  • Install .cmd wrappers on Windows and wire settings.json to the absolute .cmd path so CreateProcess can execute hooks directly.
  • Add tests/windows/test_hook_scripts.py regression test and enable smoke-test coverage for Windows (previously skipped).

Root cause

cbm_install_hook_gate_script() always wrote #!/usr/bin/env bash scripts without a file extension. Windows has no default handler for extensionless files, so the OS prompts the user to pick an app. The smoke test explicitly skipped Windows (MINGW64_NT), so CI never caught this.

Test plan

  • python tests/windows/test_hook_scripts.py <binary> fails on current release, passes with this patch
  • CI smoke on windows-latest
  • Manual: install on Windows, run Grep in Cursor — no "Open with" dialog

Notes

Existing Unix installs are unchanged. Re-running codebase-memory-mcp install -y on Windows migrates settings to the .cmd path.

@jseramn jseramn requested a review from DeusData as a code owner July 7, 2026 02:13
@DeusData DeusData added bug Something isn't working windows Windows-specific issues editor/integration Editor compatibility and CLI integration ux/behavior Display bugs, docs, adoption UX priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 7, 2026
@DeusData

DeusData commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the Windows hook-shim fix. Triage: high-priority Windows/editor integration bug.

Review will check that Windows gets executable .cmd wrappers wired by absolute path, Unix installs stay unchanged, and smoke coverage really exercises the Windows hook path that previously skipped. This is a sharp fix for #929.

@DeusData DeusData added this to the 0.9.1-rc milestone Jul 8, 2026
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

The substance here is right and needed — extensionless bash shims genuinely dead-end on Windows ("Open with" dialog on every PreToolUse), the .cmd wrapper with exit /b 0 preserves the never-blocks contract, and 2>nul correctly leaves stdout (the additionalContext JSON) flowing. Blockers before this can merge:

  1. DCO — the commit has no Signed-off-by trailer (the checklist says otherwise, but CI is right): git commit --amend -s or git rebase --signoff.
  2. Lint — the src/cli/cli.c hunks fail clang-format (see the lint job log; repo style is Homebrew LLVM clang-format with the checked-in .clang-format).
  3. Your own Windows smoke leg fails — check 8e reports the shim missing at the expected path, so the smoke-test.sh changes and the install path logic aren't consistent yet; please get the windows-latest leg green on the PR itself.
  4. Rebase required — the hook installer and smoke check 8d were rewritten on main this week (the PreToolUse matcher is now Grep|Glob|Read for the coverage note, and scripts/smoke-test.sh asserts the new matcher). Your branch predates that; expect conflicts in both files.
  5. Hardening nitbinary_path is interpolated into the .cmd body inside quotes; cmd.exe still expands %VAR% sequences and a stray " would break out. The installer controls the path so real risk is low, but please either reject %/" in the path when writing a .cmd or add a comment stating the assumption.

Happy to re-review after the rebase — this fixes a real priority/high Windows pain.

Extensionless bash hook scripts trigger Windows Open with dialogs when Cursor/Claude Code spawn PreToolUse/SessionStart commands without a shell. Write .cmd wrappers on Windows, point settings.json at the absolute .cmd path, and add regression plus smoke-test coverage for the Windows hook path.

- Rebase on main: adopt Grep|Glob|Read matcher (DeusData#963)

- Fix smoke 8e: match MINGW*/MSYS*/CYGWIN* uname suffixes

- Harden .cmd shim: reject % in binary_path (cmd.exe expansion)

- clang-format src/cli/cli.c

Signed-off-by: Jose Ramon <jrgarciadr29@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@jseramn jseramn force-pushed the fix/windows-hook-cmd-wrappers branch from befa2f8 to 8d1ee1c Compare July 9, 2026 17:35
@jseramn

jseramn commented Jul 9, 2026

Copy link
Copy Markdown
Author

Addressed all review blockers in 8d1ee1c:

  • DCO: commit amended with Signed-off-by: Jose Ramon <jrgarciadr29@gmail.com>
  • Lint: clang-format applied to src/cli/cli.c
  • Smoke 8e: fixed Windows path detection — uname -s on GHA is MINGW64_NT-10.0-..., not exact MINGW64_NT; now uses MINGW*|MSYS*|CYGWIN* case match and keeps Unix executable-bit check
  • Rebase: rebased on current main; adopted upstream Grep|Glob|Read matcher (Best-effort indexing-coverage signal: mark files/regions that were not fully indexed #963) and updated test_hook_scripts.py accordingly
  • Hardening: .cmd shim writer now rejects % in binary_path (in addition to ") since cmd.exe expands %VAR% inside quoted strings

CI should be green on the next run — happy to iterate if anything else surfaces.

@DeusData

Copy link
Copy Markdown
Owner

Thank you for this, @jseramn — the diagnosis is spot-on (extensionless bash shims trigger the Open-With dialog because CreateProcess has no shell to interpret the shebang), and your .cmd wrapper approach is exactly right.

The same fix has landed on main via #1057 (merged in da4c006, commit b1502bf), shipping in v0.9.1: all three Claude hook scripts install as .cmd on Windows with cmd syntax, the settings.json paths point at the .cmd file, and the installer removes the legacy extensionless twins on upgrade so existing installs are cleaned too. It grew out of the linked bug #929.

I'm closing this as landed rather than merging to avoid a duplicate, but the credit is genuinely shared — you and the #929 reporter independently surfaced and correctly diagnosed the same Windows integration failure. If you spot anything the merged version misses (e.g. a hook path your setup uses that we didn't cover), please do open a follow-up. Thanks again for taking the time to write it up with a test.

@DeusData DeusData closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working editor/integration Editor compatibility and CLI integration priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX windows Windows-specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants