Skip to content

Conversation

@kevalyq
Copy link
Contributor

@kevalyq kevalyq commented Oct 26, 2025

Summary

This PR fixes multiple bugs in the preflight script related to pattern validation, CRLF compatibility, and network reliability. It also disables actionlint locally to prevent network timeout issues.

Fixes

Pattern Validation Bugs (Bugs #7-9)

  1. Bug build(deps-dev): Bump @types/react from 18.3.26 to 19.2.2 #9: Pattern validation logic (60cbb9c, 1c34ff9)

    • Fixed comment filtering in .preflight-exclude parsing
    • Corrected regex pattern and comment filtering logic
  2. Bug build(deps): Bump react-dom from 18.3.1 to 19.2.0 #8: CRLF support (eb392c1)

    • Added tr -d '\r' to handle Windows/CRLF line endings
    • Prevents grep flag injection with -- before regex
  3. Documentation cleanup (d3a8f5c)

    • Removed duplicate "Bypassing the PR size check" section from README

Network Reliability Fixes

  1. Network hang prevention (c0bac8c)

    • Replaced git remote show origin with git symbolic-ref
    • Avoids SSH/network connection hang when detecting default branch
    • Faster execution and more reliable in offline/slow network scenarios
  2. actionlint hanging issue (03c8e50)

    • Disabled actionlint in local preflight (runs in CI only)
    • actionlint hangs indefinitely when run locally due to network timeouts
    • Prevents pre-push hook from blocking all git pushes
  3. Critical: Exit code bug (09d4005)

    • Added || true to grep command in regex validation
    • Without this, set -euo pipefail caused script to exit with code 1
    • This was blocking ALL git pushes even when checks passed

Tooling

  1. setup-pre-push.sh script (2d9d2a7)
    • Automated hook installation script
    • Configures git to use .githooks directory

Testing

All changes have been tested:

  • ✅ Preflight script runs successfully with exit code 0
  • ✅ Pattern validation works correctly
  • ✅ CRLF files are handled properly
  • ✅ Network-independent operation (no hanging)
  • ✅ Pre-push hook blocks invalid changes
  • ✅ Git push works when all checks pass
  • ✅ TypeScript/React checks work

Impact

Before: Pre-push hook hung indefinitely or failed with exit 1, blocking all pushes

After: Pre-push hook runs reliably and completes successfully

Related Issues

Checklist

  • All pattern validation fixes preserved (Bugs build(deps-dev): Bump vitest from 3.2.4 to 4.0.3 #7-9)
  • CRLF compatibility added
  • Network reliability improvements added
  • Critical exit code bug fixed
  • Pre-push hook tested and working
  • All commits follow conventional commits format

1. Fixed LICENSES pattern from ^LICENSES/.\*\.txt$ to ^LICENSES/.*\.txt$
   (the escaped asterisk prevented matching any .txt files)

2. Fixed comment filtering pattern from ^(#|[[:space:]]*$) to ^[[:space:]]*(#|$)
   (now correctly filters indented comments)
Found during comprehensive code review - section was duplicated.
Same fixes as .github repo - see detailed description there.

Changes in scripts/preflight.sh:
- Added tr -d '\r' to strip Windows CRLF line endings
- Added -- to grep -vE to prevent flag injection from dash-prefixed patterns
- grep exit codes: 0=match, 1=no match, 2=error
- Previous logic: if pattern matches empty string → valid
  Problem: '^file\.txt$' is valid but doesn't match ''
- New logic: if grep exit != 2 → valid
  Correctly handles both matching and non-matching patterns
- Tested with 6 test cases: all pass ✅
…twork hang

- Replace 'git remote show origin' with 'git symbolic-ref' to detect base branch
- Avoids SSH/network connection hang when detecting default branch
- Faster execution and more reliable in offline/slow network scenarios
- Maintains same fallback behavior (defaults to 'main')

Resolves the issue where preflight script would hang indefinitely
when trying to detect the base branch via network-dependent command.
…issue

- actionlint hangs indefinitely when run locally (network timeout)
- Keep actionlint enabled in CI where it works reliably
- This fixes the pre-push hook hanging issue
- Users can uncomment the line to re-enable if needed
- Automatically creates symlink from .git/hooks/pre-push to scripts/preflight.sh
- Backs up existing pre-push hook if not a symlink
- Makes hook installation consistent across all repos
- Complements existing setup-pre-commit.sh script
Copilot AI review requested due to automatic review settings October 26, 2025 13:27
Copy link

Copilot AI left a comment

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 addresses multiple critical bugs in the preflight script that were causing git pushes to fail or hang indefinitely. The fixes include pattern validation improvements, CRLF compatibility, network reliability enhancements, and documentation cleanup.

Key Changes:

  • Fixed pattern validation bugs in .preflight-exclude parsing (proper comment filtering and CRLF handling)
  • Eliminated network dependencies causing hangs (replaced git remote show with git symbolic-ref, disabled local actionlint)
  • Added automated pre-push hook installation script

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
scripts/setup-pre-push.sh New automated script for installing pre-push hook as symlink
scripts/preflight.sh Fixed network hangs, pattern validation, CRLF support, and actionlint timeout
CONTRIBUTING.md Removed duplicate "Bypassing the PR size check" section
.preflight-exclude Fixed regex escape sequence in LICENSES pattern

…nd validation improvements

- Fix CRITICAL bug: capture grep exit code before || true (set +e/set -e pattern)
- Remove ineffective chmod +x on symlink in setup-pre-push.sh
- Improve pattern validation with more diverse test filenames (README.md, package.json)

Addresses Copilot review comment in PR #25
…names

- Add more diverse test filenames for overly-broad pattern detection
  (hidden files, uppercase, numbers, special chars)
- Better catch patterns like ^[a-z] that match many but not all files

Addresses Copilot review comment
@kevalyq kevalyq merged commit f56d27a into main Oct 26, 2025
11 checks passed
@kevalyq kevalyq deleted the fix/pattern-validation-comprehensive branch October 26, 2025 14:29
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.

2 participants