Skip to content

fix(commit-validate): handle heredoc inside -m command substitution#66

Open
mouchar wants to merge 1 commit into
rohitg00:mainfrom
mouchar:fix-commit-hook
Open

fix(commit-validate): handle heredoc inside -m command substitution#66
mouchar wants to merge 1 commit into
rohitg00:mainfrom
mouchar:fix-commit-hook

Conversation

@mouchar

@mouchar mouchar commented Jun 6, 2026

Copy link
Copy Markdown

The -m flag regex matched first and captured the literal $(cat <<DELIM ... DELIM) string as the message, failing conventional commit validation. Reordered detection to check heredoc patterns before -m/--message flags so the heredoc body is used as the message when present.

When claude code tried to run idiomatic git commit:

● Bash(git commit -m "$(cat <<'EOF'
      fix(secret): stop pulp-server Secret churn from nondeterministic migration settings

      needsMigrationSetting iterated MigrationSettingsList as a map, whose
      range order Go randomizes per invocation. With RedirectToObjectStorage=true.
      <<truncated>>

      Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
      EOF
      )" && git status)

then it failed with error:

    Error: PreToolUse:Bash hook error: [node "${CLAUDE_PLUGIN_ROOT}/scripts/commit-validate.js"]: [pro-workflow] commit-validate: Commit message must follow conventional commits: 
     <type>(<scope>): <summary>. Valid types: feat, fix, refactor, test, docs, chore, perf, ci, style, build, revert.

This PR allows claude code to use multi-line $(cat <<DELIM ... DELIM) passed as heredoc commit message.

Summary by CodeRabbit

  • Bug Fixes
    • Commit message validation now correctly handles heredoc-style messages (including quoted delimiters). It captures the full multi-line body and trims a leading blank line, preventing valid messages that start with a blank line from being rejected. Validation behavior for determining the message’s first line remains unchanged.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The commit validator's extractMessage now recognizes bash-style heredocs (with optional quoted delimiters), captures the full multiline heredoc body, trims a leading blank line, and returns it as { msg, form: 'heredoc' } instead of truncating to the first line.

Changes

Heredoc commit message extraction

Layer / File(s) Summary
Full-body heredoc extraction
scripts/commit-validate.js
extractMessage now matches heredocs with optional quoted delimiters, captures the entire heredoc body, trims an initial blank line from the captured body, and returns the full body as { msg, form: 'heredoc' }. The previous behavior that reduced heredoc content to the first line was removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble bytes in moonlit code tonight,
A heredoc's whole tale caught in my sight.
Blank leaf trimmed, each line hops into place,
Commit messages smiling, no lost trace.
Rabbit cheers the validator's gentle might.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main fix: enabling heredoc handling within -m command substitution in the commit-validate script.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
scripts/commit-validate.js (1)

18-18: ⚡ Quick win

Add explanatory comment for the complex regex pattern.

The heredoc regex pattern uses capture groups, backreferences, and multiline matching, making it difficult to understand at a glance. Consider adding a brief comment explaining the pattern structure.

📝 Suggested addition
+  // Match heredoc: <<[-]'?DELIM'?\n...content...\nDELIM (capturing delimiter and body)
   const heredocAny = command.match(/<<-?\s*'?([A-Za-z_][A-Za-z0-9_]*)'?\s*\n([\s\S]*?)\n\s*\1\s*$/m);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/commit-validate.js` at line 18, Add a short inline comment above the
heredoc regex assignment that explains what the pattern in the variable
heredocAny matches (e.g., it detects bash/heredoc syntax: optional indented form
<<- or <<, optional quoting of the delimiter, captures the delimiter name in
group 1 and the body in group 2, uses a backreference (\1) to find the closing
delimiter, and relies on multiline/singleline flags), referencing the heredocAny
variable and the regex literal so future readers understand the capture groups,
backreference, and flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/commit-validate.js`:
- Around line 18-22: The heredoc regex in scripts/commit-validate.js (the const
heredocAny assignment) doesn't match double-quoted delimiters; update the
pattern to capture an optional quote char (single or double) and the delimiter
name, ensure the same quote is used to close (so opening quote is captured in
group 1, delimiter in group 2), and then extract the body from the correct
capture group (body will move to group 3 instead of group 2); keep the rest of
the logic (trimming leading blank line and returning { msg: ..., form: 'heredoc'
}) the same but reference the new capture groups when computing body.

---

Nitpick comments:
In `@scripts/commit-validate.js`:
- Line 18: Add a short inline comment above the heredoc regex assignment that
explains what the pattern in the variable heredocAny matches (e.g., it detects
bash/heredoc syntax: optional indented form <<- or <<, optional quoting of the
delimiter, captures the delimiter name in group 1 and the body in group 2, uses
a backreference (\1) to find the closing delimiter, and relies on
multiline/singleline flags), referencing the heredocAny variable and the regex
literal so future readers understand the capture groups, backreference, and
flags.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1c39a40c-fc04-4f21-a009-ee9990c13435

📥 Commits

Reviewing files that changed from the base of the PR and between 5c313e2 and ba6359d.

📒 Files selected for processing (1)
  • scripts/commit-validate.js

Comment thread scripts/commit-validate.js Outdated
The -m flag regex matched first and captured the literal
$(cat <<DELIM ... DELIM) string as the message, failing
conventional commit validation. Reordered detection to check
heredoc patterns before -m/--message flags so the heredoc body
is used as the message when present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Robert Moucha <robert.moucha@gooddata.com>
@mouchar mouchar force-pushed the fix-commit-hook branch from ba6359d to 7361dc6 Compare June 6, 2026 20:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
scripts/commit-validate.js (1)

23-23: 💤 Low value

Optionally trim all leading blank lines, not just one.

The current pattern /^\s*\n/ removes only the first leading blank line. If the heredoc body starts with multiple consecutive blank lines, only the first is removed. For example:

  • Input: "\n\nfeat: test" → Output: "\nfeat: test" (one blank line remains)

This is likely fine for typical commit messages, but for robustness you could trim all leading blank lines:

♻️ Proposed refinement
-    const body = heredocAny[3].replace(/^\s*\n/, '');
+    const body = heredocAny[3].replace(/^(\s*\n)+/, '');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/commit-validate.js` at line 23, The assignment to const body uses
heredocAny[3].replace(/^\s*\n/, '') which only strips a single leading blank
line; update that replace call in scripts/commit-validate.js (the const body =
heredocAny[3].replace(...) statement) to remove all leading blank lines by
matching one-or-more leading newline blocks (e.g., use a regex that repeats the
leading blank-line pattern such as /^(?:\s*\r?\n)+/ or similar) so any number of
initial blank lines are trimmed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@scripts/commit-validate.js`:
- Line 23: The assignment to const body uses heredocAny[3].replace(/^\s*\n/, '')
which only strips a single leading blank line; update that replace call in
scripts/commit-validate.js (the const body = heredocAny[3].replace(...)
statement) to remove all leading blank lines by matching one-or-more leading
newline blocks (e.g., use a regex that repeats the leading blank-line pattern
such as /^(?:\s*\r?\n)+/ or similar) so any number of initial blank lines are
trimmed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: efa1c4a0-3c86-413a-ab79-dfce5a3a911c

📥 Commits

Reviewing files that changed from the base of the PR and between ba6359d and 7361dc6.

📒 Files selected for processing (1)
  • scripts/commit-validate.js

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