-
Notifications
You must be signed in to change notification settings - Fork 0
feat(completion-integrity): add git pre-commit hook for bypass mode #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||
| # ============================================================================= | ||||||||||||||||||||||||||||||
| # INSTALL GIT PRE-COMMIT HOOK | ||||||||||||||||||||||||||||||
| # ============================================================================= | ||||||||||||||||||||||||||||||
| # Installs the completion-integrity check as a real git pre-commit hook. | ||||||||||||||||||||||||||||||
| # This works independently of Claude's permission system. | ||||||||||||||||||||||||||||||
| # ============================================================================= | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Colors for install script output | ||||||||||||||||||||||||||||||
| GREEN='\033[0;32m' | ||||||||||||||||||||||||||||||
| NC='\033[0m' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Find git root | ||||||||||||||||||||||||||||||
| GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) | ||||||||||||||||||||||||||||||
| if [[ -z "${GIT_ROOT}" ]]; then | ||||||||||||||||||||||||||||||
| echo "ERROR: Not in a git repository" | ||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| HOOKS_DIR="${GIT_ROOT}/.git/hooks" | ||||||||||||||||||||||||||||||
| PRE_COMMIT_HOOK="${HOOKS_DIR}/pre-commit" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| mkdir -p "${HOOKS_DIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current backup mechanism for an existing pre-commit hook overwrites pre-commit.backup on each run. This means if you run the installer multiple times, you will lose older backups. To prevent data loss, consider using a timestamp in the backup filename to make it unique.
| echo "Backing up to ${PRE_COMMIT_HOOK}.backup" | |
| cp "${PRE_COMMIT_HOOK}" "${PRE_COMMIT_HOOK}.backup" | |
| BACKUP_FILE="${PRE_COMMIT_HOOK}.backup.$(date +%s)" | |
| echo "Backing up to ${BACKUP_FILE}" | |
| cp "${PRE_COMMIT_HOOK}" "${BACKUP_FILE}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git diff command is configured to exclude test files (e.g., **/*.test.*, **/*.spec.*) from the diff analysis. However, RULE 2 of this script is specifically designed to detect commented-out tests, which are typically found only in these files. By excluding them, the check for commented-out tests is rendered ineffective. To ensure the hook functions as intended, these exclusion patterns for test files should be removed.
| STAGED_DIFF=$(git diff --cached --unified=0 -- \ | |
| ':(exclude)*.md' \ | |
| ':(exclude)**/hooks/scripts/*.sh' \ | |
| ':(exclude)**/scripts/*.sh' \ | |
| ':(exclude)**/*.test.*' \ | |
| ':(exclude)**/*.spec.*' \ | |
| ':(exclude)**/test-fixtures/**' \ | |
| 2>/dev/null || echo "") | |
| STAGED_DIFF=$(git diff --cached --unified=0 -- \ | |
| ':(exclude)*.md' \ | |
| ':(exclude)**/hooks/scripts/*.sh' \ | |
| ':(exclude)**/scripts/*.sh' \ | |
| ':(exclude)**/test-fixtures/**' \ | |
| 2>/dev/null || echo "") |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The C# suppression pattern is incomplete compared to the existing integrity-check.sh script. The pattern should include 'DisableWarning' to match line 52 of integrity-check.sh which uses: '#pragma\s+warning\s+disable|SuppressMessage|DisableWarning'
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JS/TS suppression pattern is incomplete compared to integrity-check.sh. Missing patterns: '@ts-expect-error' and 'tslint:disable'. The complete pattern should be: 'eslint-disable|@ts-ignore|@ts-nocheck|@ts-expect-error|tslint:disable' (see integrity-check.sh line 62)
| JS_SUPPRESS=$(echo "${ADDED_LINES}" | grep -iE 'eslint-disable|@ts-ignore|@ts-nocheck' || true) | |
| JS_SUPPRESS=$(echo "${ADDED_LINES}" | grep -iE 'eslint-disable|@ts-ignore|@ts-nocheck|@ts-expect-error|tslint:disable' || true) |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python suppression pattern is missing 'pylint:disable'. The complete pattern should be: '#\snoqa|#\stype:\signore|#\spylint:\s*disable' (see integrity-check.sh line 72)
| PY_SUPPRESS=$(echo "${ADDED_LINES}" | grep -iE '#\s*noqa|#\s*type:\s*ignore' || true) | |
| PY_SUPPRESS=$(echo "${ADDED_LINES}" | grep -iE '#\s*noqa|#\s*type:\s*ignore|#\s*pylint:\s*disable' || true) |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented test pattern is missing 'TestMethod' which is used in some C# test frameworks. The complete pattern should be: '//\s*[(Test|Fact|Theory|TestMethod)]' (see integrity-check.sh line 86)
| COMMENTED_TESTS=$(echo "${ADDED_LINES}" | grep -iE '//\s*\[(Test|Fact|Theory)\]|//\s*(it|test|describe)\s*\(' || true) | |
| COMMENTED_TESTS=$(echo "${ADDED_LINES}" | grep -iE '//\s*\[(Test|Fact|Theory|TestMethod)\]|//\s*(it|test|describe)\s*\(' || true) |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion detection only uses C#-style patterns (Assert.|Should.|Expect(). This misses JavaScript/TypeScript assertions which use different patterns like 'expect(|assert.|should.' (lowercase). The integrity-check.sh script has separate detection for C# (line 139) and JS (line 151) assertions. Consider either adding JS-specific assertion detection or updating this pattern to catch both.
| DELETED_ASSERTS=$(echo "${DELETED_LINES}" | grep -iE 'Assert\.|Should\.|Expect\(' || true) | |
| DELETED_ASSERTS=$(echo "${DELETED_LINES}" | grep -iE 'Assert\.|Should\.|Expect\(|assert\.|should\.|expect\(' || true) |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test file deletion pattern is incomplete compared to integrity-check.sh. Missing pattern: 'Test.cs' (single test files). The complete pattern should be: '.test.|.spec.|_test.|Tests.cs|Test.cs' (see integrity-check.sh line 167)
| DELETED_TESTS=$(echo "${DELETED_FILES}" | grep -iE '\.test\.|\.spec\.|_test\.|Tests\.cs' || true) | |
| DELETED_TESTS=$(echo "${DELETED_FILES}" | grep -iE '\.test\.|\.spec\.|_test\.|Tests\.cs|Test\.cs' || true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new compatibility table's columns are not aligned in the raw markdown source. To improve readability and adhere to the repository's style guide, please add padding to the table cells.
References