chore: sync hardened commit-msg hook#35
Conversation
There was a problem hiding this comment.
Approach Review: The approach looks good. The PR successfully addresses the npm_config_package environment poisoning and local shadowing issues by hardening the npm exec command. This is a targeted and standard fix for this specific class of npm execution edge cases without introducing any unnecessary complexity or structural changes.
Approach review by Approach Review 2 (gemini-3.1-pro-preview)
There was a problem hiding this comment.
Approach Review: Approach looks good.
The PR is a minimal, well-scoped swap of npx -y for npm exec --yes --package=conventional-commit-msg@latest --, which is the canonical npm fix for the two documented footguns (inherited npm_config_package poisoning and local-root shadowing). There's no simpler or more standard way to express "run a one-off package bin, immune to env/local shadowing" in the npm ecosystem — npm exec with an explicit pinned --package is the documented escape hatch and the right tool here. No existing codebase utility or alternative pattern would do this better.
Approach review by Approach Review 3 (OpenCode Wafer) (GLM-5.2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a59f9f4569
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # `@latest` + explicit `--package` dodge two npm footguns: an inherited | ||
| # npm_config_package (agent wrappers like axexec/axrun set it, hijacking | ||
| # `npx -y` into exit 127), and local-root shadowing — without the version, | ||
| # `npm exec` resolves the conventional-commit-msg repo's own package and the | ||
| # bin is "not found", breaking the hook in that repo after prepare wires it up. |
There was a problem hiding this comment.
Shorten the hook comment to a single line
The repo guideline in AGENTS.md says comments must be kept to one short line and never use multi-line comment blocks. This new five-line block violates that documented standard even though the rationale is useful; please collapse it to a concise single-line why-comment or move the fuller explanation to commit/PR context.
Useful? React with 👍 / 👎.
Syncs the managed
.githooks/commit-msghook to j4k-align's hardened form (pinnednpm exec --yes --package=conventional-commit-msg@latest), which is immune tonpm_config_packageenv poisoning that breaks the oldnpx -yinvocation.