Skip to content

fix(hooks): remove unchecked perl dependency in simplify-ignore.sh#385

Open
HoneyTyagii wants to merge 1 commit into
addyosmani:mainfrom
HoneyTyagii:fix/simplify-ignore-perl-dependency
Open

fix(hooks): remove unchecked perl dependency in simplify-ignore.sh#385
HoneyTyagii wants to merge 1 commit into
addyosmani:mainfrom
HoneyTyagii:fix/simplify-ignore-perl-dependency

Conversation

@HoneyTyagii

Copy link
Copy Markdown

Summary

hooks/simplify-ignore.sh used perl -pe 'chomp if eof' to preserve a file's trailing-newline status, but perl was never dependency-checked (unlike jq and shasum/sha1sum, which are validated up front) and it isn't listed in the script's Dependencies header.

The bug

In the PostToolUse (Edit/Write) branch, the trailing-newline block runs at top level under set -euo pipefail:

perl -pe 'chomp if eof' "$EXPANDED" > "${EXPANDED}.nnl" && \
  cat "${EXPANDED}.nnl" > "$EXPANDED" && rm -f "${EXPANDED}.nnl"

On a host without perl (minimal CI images, or Alpine/busybox containers, which are common places for Claude Code hooks to run), this command fails. Because it's the last command in the then clause and set -e is active, the script aborts before the expanded content is written back to disk.

The result is not a cosmetic newline glitch. The file is left in its BLOCK_<hash> placeholder state and the backup is never refreshed, so the model's edits are effectively stranded, which is a data-integrity failure.

The same perl call is also used in filter_file, where it silently mishandles trailing-newline preservation on perl-less hosts.

The fix

Replace both perl invocations with a POSIX-safe equivalent that removes the hidden dependency entirely:

printf '%s' "$(cat "$EXPANDED")" > "${EXPANDED}.nnl"

Command substitution $(...) strips all trailing newlines, matching the behavior of perl -pe 'chomp if eof'. This aligns with the script's established pattern of validating (or not depending on) every external tool.

Testing

  • bash -n hooks/simplify-ignore.sh passes
  • hooks/simplify-ignore-test.sh: the trailing-newline preservation test (Test 5) passes, and results are unchanged from before this patch

The trailing-newline preservation logic relied on perl (perl -pe 'chomp
if eof'), but perl was never dependency-checked the way jq and
shasum/sha1sum are, nor listed in the script's Dependencies header.

On perl-less hosts (e.g. minimal CI images or Alpine/busybox
containers) the PostToolUse Edit|Write branch runs this block at top
level under `set -euo pipefail`, so a missing perl aborts the script
before the expanded content is written back to disk. The net effect is
that the file is left in its BLOCK_<hash> placeholder state and the
model's edits are stranded, a data-integrity failure rather than a
cosmetic newline issue.

Replace both perl invocations with a POSIX-safe printf '%s' "$(cat ...)"
trailing-newline trim, which removes the hidden dependency entirely
while preserving the existing behavior. Verified with bash -n and the
existing simplify-ignore-test.sh (trailing-newline test passes).
Copilot AI review requested due to automatic review settings July 10, 2026 16:20

Copilot AI 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.

Pull request overview

This PR removes an unchecked perl dependency from hooks/simplify-ignore.sh by replacing the trailing-newline-preservation logic with a POSIX-oriented alternative, improving reliability on minimal CI/container images where Perl isn’t installed.

Changes:

  • Replaced perl -pe 'chomp if eof' usage in filter_file() with a shell-based approach.
  • Replaced the same Perl usage in the PostToolUse (Edit/Write) expansion path.
  • Expanded inline comments explaining the trailing-newline handling and motivation (avoid hard Perl dependency).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread hooks/simplify-ignore.sh
# replacement for `perl -pe 'chomp if eof'` and avoids a hard perl dependency.
if [ -s "$dest" ] && [ -s "$src" ] && [ -n "$(tail -c 1 "$src")" ]; then
perl -pe 'chomp if eof' "$dest" > "${dest}.nnl" && \
printf '%s' "$(cat "$dest")" > "${dest}.nnl" && \
Comment thread hooks/simplify-ignore.sh
# hard perl dependency (perl is absent on minimal CI/container images).
if [ -s "$EXPANDED" ] && [ -s "$FILE_PATH" ] && [ -n "$(tail -c 1 "$FILE_PATH")" ]; then
perl -pe 'chomp if eof' "$EXPANDED" > "${EXPANDED}.nnl" && \
printf '%s' "$(cat "$EXPANDED")" > "${EXPANDED}.nnl" && \
@addyosmani

Copy link
Copy Markdown
Owner

Good catch, and exactly the portability bug that only shows up on the one machine without perl installed. Swapping to the tools the hook already dependency-checks (jq, shasum) keeps it honest about what it actually needs, instead of silently relying on perl being around. I verified it merges clean and the suite stays green. Straightforward yes from me, thanks for tightening it.

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.

3 participants