Skip to content

fix: correct regex syntax in governance hook for linked-issues detection - #1490

Closed
ashleyshaw wants to merge 28 commits into
developfrom
fix/governance-hook-regex-bug
Closed

fix: correct regex syntax in governance hook for linked-issues detection#1490
ashleyshaw wants to merge 28 commits into
developfrom
fix/governance-hook-regex-bug

Conversation

@ashleyshaw

@ashleyshawashleyshaw commented Aug 4, 2026

Copy link
Copy Markdown
Member

Linked issues

Fixes#1489

Context

  • Severity/Impact: High (blocks valid PR merges)
  • Affected versions/environments: develop branch

Reproduction

Root Cause

Line 102 of hooks/pr-merge-governance-validator.sh uses \s+ (POSIX character class) which is NOT supported by grep -E (ERE regex). Correct syntax is [[:space:]]+ for space/tab/newline matching in ERE.

Fix Summary

  • Replace unsupported \s+ with ERE-compatible [[:space:]]+ in regex pattern
  • Enables proper detection of "Fixes #XXX", "Relates to #XXX", "Closes #XXX" patterns in PR bodies
  • Validates locally: echo "Fixes #1376" | grep -qiE "(fixes|resolves|closes|relates to)[[:space:]]+#[0-9]+" && echo "PASS" || echo "FAIL"

Verification

  • Regex fix verified locally with multiple test cases
  • Hook logic remains unchanged (only regex syntax fixed)
  • No impact on other validation rules

Risk & Rollback

  • Risk level: Low (isolated regex syntax fix in validation hook)
  • Rollback plan: Revert commit if needed

Changelog

Fixed

Added

Changed

Removed


Checklist (Global DoD / PR)

  • All AC met and demonstrated
  • Tests added/updated (unit/E2E as appropriate)
  • Accessibility checklist completed (where relevant):
    • N/A - shell script validation hook
  • Docs/readme/changelog updated (if user-facing)
  • Security checklist completed (where relevant):
    • No untrusted input; validation logic only
    • No secrets introduced
  • Code/design reviews approved
  • CI green; linked issues closed; release notes prepared (if shipping)

ashleyshawand others added 26 commits August 4, 2026 12:32
Add portable governance guardrail to block PR merges that violate:
1. PR template requirements (based on branch prefix per AGENTS.md)
2. Linked issue requirement (Fixes #XXX or Relates to #XXX)
Hook runs before gh pr merge succeeds, providing clear error messages
with actionable fixes. Prevents repeated governance violations that
were costing tokens and undermining issue tracking.
References:
- AGENTS.md: PR template routing by branch prefix
- CLAUDE.md: Repository structure and governance rules
- docs/BRANCHING_STRATEGY.md: Branch naming discipline
Hook location: hooks/pr-merge-governance-validator.sh (portable asset)
Configuration: .claude/settings.json PreToolUse hook on Bash(gh pr merge *)
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Merges latest develop changes while preserving feature branch commits.
Resolves conflicts by taking feature branch version of workflow file
(feat branch has the final correct Node.js script solution).
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Replace semicolon-separated shell commands with pipe-delimited multiline
syntax to avoid YAML parser errors. Preserves set +e behavior for error
handling in milestone assignment step.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Replace unsupported \s+ (POSIX character class) with [[:space:]]+ (ERE-compatible)
in grep -qiE pattern. Previous syntax would fail to detect 'Fixes#123' patterns
and incorrectly block valid PR merges.
Fixes issue #1489 (code review finding from PR #1488).
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@ashleyshaw, you've reached your PR review limit, so we couldn't start this review.

Next review available in:2 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1ec5117b-5e6c-4458-a9a0-fe8ee1b50daf

📥 Commits

Reviewing files that changed from the base of the PR and between c27fa3b and f25dba4.

📒 Files selected for processing (3)
  • .github/workflows/gitleaks-update.yml
  • CHANGELOG.md
  • hooks/pr-merge-governance-validator.sh

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.

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

✅ Template check passed after update. Thanks for fixing the PR description.

ashleyshaw added a commit that referenced this pull request Aug 4, 2026
…ompt
Add CONTINUATION_PROMPT_2026-08-04.md documenting:
- Critical regex bug fix (PR #1490)
- Governance process violations in #1489
- Pre-commit validation hook implementation plan
- Root cause analysis and long-term improvements
Update README with Phase 2 AI governance process work.
Related to issue #1489, PR #1490.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Keep fix branch version with [[:space:]]+ (ERE-compatible).
Develop still has old \s+ syntax. Fix takes precedence.
Resolves merge conflict in PR #1490.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@ashleyshaw
ashleyshaw enabled auto-merge (squash) August 4, 2026 13:42
@github-actionsgithub-actionsBot added status:needs-review Awaiting code review priority:normal Default priority area:ci Build and CI pipelines area:documentation Docs & guides area:scripts Scripts & tooling lang:md Markdown content/docs meta:needs-changelog Requires a changelog entry before merge and removed area:governance labels Aug 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #1490

CI Status:success
Files changed: 3
Risk Distribution: 1 critical, 0 high, 0 medium, 2 low

Recommendations

  • ⚠️ 1 critical-risk file(s) modified (workflows, secrets)
  • ⚠️ Security-sensitive files modified (review carefully)

@github-actionsgithub-actionsBot removed the priority:normal Default priority label Aug 4, 2026
@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⏱️ Aging and SLA annotation

  • Age: 0 day(s)
  • SLA state: Within SLA
  • Thresholds: warn at 7 days, breach at 14 days
  • Last updated: 2026-08-04T13:56:21.154Z

Maintained by project-meta-sync workflow.

@mergify

mergifyBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@ashleyshaw

Copy link
Copy Markdown
MemberAuthor

@Mergifyio queue

@mergify

mergifyBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

CopilotAI 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.

🟡 Not ready to approve

The linked-issues regex can still yield false positives by matching keywords inside longer words, weakening the governance check.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR restores linked-issue detection in the PR merge governance hook by replacing a grep -E-incompatible regex escape with an ERE-compatible whitespace class, unblocking valid merges on develop.

Changes:

  • Fix linked-issues grep -E pattern by switching from \s+ to [[:space:]]+.
  • Minor Markdown whitespace tweak in CHANGELOG.md.
  • Normalise YAML quoting for the scheduled cron expression in gitleaks-update.yml.
File summaries
FileDescription
hooks/pr-merge-governance-validator.shUpdates the linked-issues detection regex to be compatible with grep -E.
CHANGELOG.mdAdjusts spacing around the Unreleased sections.
.github/workflows/gitleaks-update.ymlNormalises cron quoting style in the workflow schedule.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

has_linked_issues=false

if echo "$pr_body" | grep -qiE "(fixes|resolves|closes|relates to)\s+#[0-9]+"; then
if echo "$pr_body" | grep -qiE "(fixes|resolves|closes|relates to)[[:space:]]+#[0-9]+"; then
Add word boundary check by requiring keywords to be preceded by
either start-of-line or whitespace. This prevents matching keywords
inside longer words (e.g., 'prefixes #123' no longer matches 'fixes').
Maintains support for case-insensitive matching and flexible whitespace
in 'relates to' pattern.
Fixes Copilot review feedback on PR #1490.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

@ashleyshawashleyshaw left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Fixed: Improved regex to require keywords at start-of-line or after whitespace to prevent false positives like 'prefixes #123' matching 'fixes'. Pattern now: (^|[[:space:]])(fixes|resolves|closes|relates[[:space:]]+to)[[:space:]]+#[0-9]+. Verified against test cases (6 positive, 6 negative).

🤖 Addressed by Claude Code

@ashleyshaw

Copy link
Copy Markdown
MemberAuthor

@copilot-pull-request-reviewer review

@ashleyshaw

Copy link
Copy Markdown
MemberAuthor

Work continued in clean PRs #1494 (regex fix) and #1495 (ESLint config). Closing in favor of those streamlined PRs.

What was done in #1490:

Why moving to clean PRs:

Related: #1489 (governance validation improvements)

auto-merge was automatically disabled August 4, 2026 14:24

Pull request was closed

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ciBuild and CI pipelinesarea:documentationDocs & guidesarea:scriptsScripts & toolinglang:mdMarkdown content/docsmeta:needs-changelogRequires a changelog entry before mergepriority:criticalProduction/launch-blockingstatus:needs-reviewAwaiting code reviewtype:bugBug or defect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: PR #1488 governance validation hook has regex bug + process workflow gaps

2 participants

@ashleyshaw