Skip to content

chore: sync hardened commit-msg hook#35

Merged
Jercik merged 1 commit into
mainfrom
chore/hook-sync
Jul 2, 2026
Merged

chore: sync hardened commit-msg hook#35
Jercik merged 1 commit into
mainfrom
chore/hook-sync

Conversation

@Jercik

@Jercik Jercik commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Syncs the managed .githooks/commit-msg hook to j4k-align's hardened form (pinned npm exec --yes --package=conventional-commit-msg@latest), which is immune to npm_config_package env poisoning that breaks the old npx -y invocation.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)

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

Copy link
Copy Markdown

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

Comment thread .githooks/commit-msg
Comment on lines +2 to +6
# `@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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

@Jercik Jercik merged commit a8ca947 into main Jul 2, 2026
6 of 11 checks passed
@Jercik Jercik deleted the chore/hook-sync branch July 2, 2026 14:57
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.

1 participant