Skip to content

Fix Windows hook dispatch via explicit bash invocation#114

Open
ianliuy wants to merge 1 commit intoPolyArch:devfrom
ianliuy:pr/windows-fix
Open

Fix Windows hook dispatch via explicit bash invocation#114
ianliuy wants to merge 1 commit intoPolyArch:devfrom
ianliuy:pr/windows-fix

Conversation

@ianliuy
Copy link
Copy Markdown

@ianliuy ianliuy commented Apr 26, 2026

Problem

GitHub Copilot CLI on Windows installs plugin hooks under ~/.copilot/installed-plugins/<owner>/<plugin>/hooks/ and dispatches each hooks.json entry's command field through PowerShell. With the existing bare-path form (${CLAUDE_PLUGIN_ROOT}/hooks/<name>.sh), Windows dispatches the .sh file 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 command from a bare script path to an explicit bash invocation:

- "command": "${CLAUDE_PLUGIN_ROOT}/hooks/loop-bash-validator.sh"
+ "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/loop-bash-validator.sh\""

On Windows this resolves bash from PATH (for example Git for Windows' bash.exe) and passes the .sh file 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:

  • match .humanize/rlcr/
  • extract round-*-summary.md / round-*-contract.md
  • enforce current-round checks
  • block protected plan.md backups inside loop directories

Why not the windows: platform-override field?

An earlier iteration tried a windows platform 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 the command value.

Validation

  • bash tests/test-allowlist-validators.sh
    • 55 passing tests
    • includes Windows-backslash regression coverage for Write, Edit, and Read validators
  • Bash syntax check for all .sh files
  • git diff --check upstream/dev...HEAD
  • JSON parse checks for hooks/hooks.json, .claude-plugin/plugin.json, and .claude-plugin/marketplace.json

I also attempted the requested retarget to beta, but GitHub rejected it because PolyArch/humanize has no beta base branch. The PR is retargeted to dev, which matches the repository's PR target-branch workflow.

Scope

  • 6 files touched
  • No new dependencies
  • Version files are intentionally unchanged at 1.17.0 because this PR now targets dev; the main-branch version bump workflow only applies to PRs targeting main.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment thread hooks/lib/loop-common.sh Outdated
@SihaoLiu
Copy link
Copy Markdown
Contributor

SihaoLiu commented Apr 26, 2026

Can you try to re-target to beta branch?

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>
@ianliuy ianliuy changed the base branch from main to dev May 5, 2026 06:53
@ianliuy
Copy link
Copy Markdown
Author

ianliuy commented May 5, 2026

Follow-up pushed.

  • Tried retargeting to beta, but GitHub rejected it because PolyArch/humanize does not currently have a beta base branch.
  • Retargeted the PR to dev, which satisfies the repo's target-branch rule.
  • Rebased on latest upstream/dev; PR is now cleanly mergeable.
  • Addressed the Codex review by normalizing Windows path separators before Write/Edit/Read validator path parsing, so wrong-round checks still apply to backslash-separated Windows paths.
  • Removed the stale 1.15.5 version bump from the diff; version remains 1.17.0 from dev.

Local checks:

  • bash tests/test-allowlist-validators.sh
  • Bash syntax check for all .sh files
  • git diff --check upstream/dev...HEAD
  • JSON parse checks for changed JSON files

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.

2 participants