Fix Windows hook dispatch via explicit bash invocation#114
Open
ianliuy wants to merge 1 commit intoPolyArch:devfrom
Open
Fix Windows hook dispatch via explicit bash invocation#114ianliuy wants to merge 1 commit intoPolyArch:devfrom
ianliuy wants to merge 1 commit intoPolyArch:devfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8927759f2c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
|
Can you try to re-target to |
Normalize Windows path separators before validator path parsing so wrong-round checks still apply to backslash-separated RLCR paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Author
|
Follow-up pushed.
Local checks:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GitHub Copilot CLI on Windows installs plugin hooks under
~/.copilot/installed-plugins/<owner>/<plugin>/hooks/and dispatches eachhooks.jsonentry'scommandfield through PowerShell. With the existing bare-path form (${CLAUDE_PLUGIN_ROOT}/hooks/<name>.sh), Windows dispatches the.shfile through file association instead of executing it. The visible symptom is that hook source files open as editor tabs and the hooks never fire.This makes Humanize unusable under Copilot CLI on Windows even though the hook scripts themselves are host-agnostic.
Fix
Switch each hook entry's
commandfrom a bare script path to an explicit bash invocation:On Windows this resolves
bashfromPATH(for example Git for Windows'bash.exe) and passes the.shfile as an argument instead of letting Windows file association handle it. On macOS/Linux this preserves the same effective execution path as the existing shebang-based script dispatch.Windows path normalization
Windows hook payloads can carry native backslash paths such as
C:\repo\.humanize\rlcr\<loop>\round-99-summary.md. The validators previously matched.humanize/rlcr/and extracted loop-relative filenames with forward-slash-only patterns, so Windows paths could be misclassified or skip round-number enforcement.This PR normalizes path separators before read/write/edit validator path checks. That keeps all existing POSIX behavior while ensuring Windows paths still:
.humanize/rlcr/round-*-summary.md/round-*-contract.mdplan.mdbackups inside loop directoriesWhy not the
windows:platform-override field?An earlier iteration tried a
windowsplatform override based on VS Code Agent Hooks documentation. Empirical testing on Copilot CLI showed plugin-loaded hooks follow Claude Code's nested matcher/hooks structure and do not honor that field. The fix therefore needs to live directly in thecommandvalue.Validation
bash tests/test-allowlist-validators.sh.shfilesgit diff --check upstream/dev...HEADhooks/hooks.json,.claude-plugin/plugin.json, and.claude-plugin/marketplace.jsonI also attempted the requested retarget to
beta, but GitHub rejected it becausePolyArch/humanizehas nobetabase branch. The PR is retargeted todev, which matches the repository's PR target-branch workflow.Scope
1.17.0because this PR now targetsdev; the main-branch version bump workflow only applies to PRs targetingmain.