fix: make required team hooks cross-platform#2217
Open
neallee2012 wants to merge 5 commits into
Open
Conversation
Generate one Node CommonJS enforcement hook, fail closed when project context or the global install cannot be verified, and cover cmd and PowerShell execution paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Catch project hook resolution and load failures in the cross-shell Node launcher, returning a sanitized structured denial instead of a module-loader error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
@16francej — recommend closing this PR as superseded by #2229. It offers useful portability and migration groundwork, but omits Copilot response details and includes unrelated files; #2229 provides the narrower cross-host implementation. Credit #2217 for the CommonJS hook, shell-neutral launcher, fail-closed errors, and legacy migration, while leaving its unrelated files out. |
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.
Summary
Required team mode currently generates a Bash hook and registers it with Bash environment-variable syntax. Windows and Copilot hook runners can select a different shell, turning an intentional policy denial into
Denied by preToolUse hook ... (hook errored).This PR makes required-mode enforcement shell-neutral:
.claude/hooks/check-gstack.cjswith an explicit CommonJS extension and a Node 18+ implementation.node -elauncher that readsCLAUDE_PROJECT_DIRthroughprocess.env; noshellfield or Bash, PowerShell, or cmd environment expansion is used.{}when the global install is present. The hook never emitspermissionDecision: "allow", so normal permission prompts remain intact.PreToolUsedeny JSON with exit code 0 when gstack is absent, its install cannot be verified, the project directory is blank or stale, or the hook cannot be loaded. These are policy denials, not hook execution errors.HOME, thenUSERPROFILE, thenos.homedir().Legacy required-mode projects migrate cleanly:
.claude/hooks/check-gstack.shfile is removed only after the CommonJS hook and settings registration succeed.git addcommand includes its deletion so the suggested migration commit leaves no unstaged change.Test plan
bun test --timeout 30000 test\team-mode.test.tstwice: 32 passed, 0 failed, 114 assertions per run.USERPROFILEhome fallback.git addcommand, commits it, and verifies a clean worktree while preserving an unrelated hook.bash -n bin/gstack-team-init.git diff --check.