-
Notifications
You must be signed in to change notification settings - Fork 0
feat(completion-integrity): add plugin to prevent shortcuts #52
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
Conversation
New plugin that blocks commits and warns on completion claims when Claude takes shortcuts instead of fixing issues properly. Pre-Commit Gate (PreToolUse: Bash): - Blocks git commit when staged changes contain integrity violations - Detects: C#/JS/Python warning suppressions - Detects: Commented-out tests, test file deletion - Detects: Deleted assertions, empty catch blocks Phase-End Check (Stop): - Warns when Claude claims "done!" but response indicates shortcuts - Detects: Untested claims, dismissed warnings, deferred work Exclusions for false positives: - Markdown files (documentation) - Shell scripts in plugin directories - Test fixtures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughA new "completion-integrity" plugin (v1.0.0) is added to the marketplace. It provides hooks and scripts: a pre-commit gate that scans staged diffs for suppressions, commented tests, deleted assertions, etc., and a phase-end check that scores Claude responses for premature completion claims. Documentation and manifests included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Git
participant PreCommitHook as Pre-commit Hook
participant IntegrityCheck as Integrity Check
participant Claude
participant ResponseHook as Response Hook
participant PhaseCheck as Phase-end Check
rect rgb(220, 235, 255)
Note over User,Git: Pre-commit Gate Flow
User->>Git: git commit (with staged changes)
Git->>PreCommitHook: Trigger PreToolUse
PreCommitHook->>Git: Read staged diff
Git-->>PreCommitHook: Diff content
PreCommitHook->>IntegrityCheck: Run scan
IntegrityCheck->>IntegrityCheck: Detect suppressions, commented tests,<br/>deleted assertions/tests, TODOs, empty catches
alt Violations detected
IntegrityCheck-->>PreCommitHook: Exit non-zero
PreCommitHook-->>Git: Return block decision + JSON message
Git-->>User: Commit rejected
else Clean or only warnings
IntegrityCheck-->>PreCommitHook: Exit zero
PreCommitHook-->>Git: Allow commit
Git-->>User: Commit accepted
end
end
rect rgb(245, 225, 255)
Note over Claude,PhaseCheck: Phase-end Response Check
Claude->>ResponseHook: Stop event (response)
ResponseHook->>PhaseCheck: Execute with response stdin
PhaseCheck->>PhaseCheck: Scan for completion claims and red flags
alt Score > threshold
PhaseCheck-->>Claude: Emit completion-integrity-warning (JSON with score & signals)
else Score <= threshold
PhaseCheck-->>Claude: No warning (silent)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @ANcpLua, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a new 'completion-integrity' plugin designed to prevent shortcuts and ensure honest task completion. The plugin implements a pre-commit gate that blocks commits containing integrity violations such as warning suppressions, commented-out tests, deleted assertions, or deleted test files. It also includes a phase-end check that warns if Claude claims task completion but its response indicates unverified tests, dismissed warnings, or deferred work. The review comments primarily address a recurring issue in the newly added shell scripts: unquoted variables within heredocs and command strings. This practice violates the repository's style guide and could lead to unexpected behavior due to word splitting or glob expansion, particularly when constructing JSON output, and the reviewer suggests quoting these variables or using jq/printf for robust JSON generation.
| cat <<EOF | ||
| { | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "Stop", | ||
| "additionalContext": "<completion-integrity-warning score=\"$score\">\\n\\nCompletion claim detected but integrity issues found:\\n\\n${SIGNALS_LIST}\\nBefore claiming completion:\\n1. Run the actual tests (don't assume they pass)\\n2. Fix warnings instead of suppressing them\\n3. Fix code instead of deleting/commenting it\\n4. Complete TODOs or explicitly defer them to the user\\n\\nDo you want to verify the work is actually complete?\\n</completion-integrity-warning>" | ||
| } | ||
| } | ||
| EOF |
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 variables $score and ${SIGNALS_LIST} are unquoted within the heredoc on line 89. This violates the repository style guide (line 61) and can lead to unexpected behavior from word splitting or glob expansion. It's safer to construct the JSON payload using a tool like jq or printf to avoid these issues and ensure the generated JSON is always valid.
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
| cat << EOF | ||
| { | ||
| "decision": "block", | ||
| "reason": "COMPLETION INTEGRITY: Violations detected in staged changes", | ||
| "message": "BLOCKED: Cannot commit with integrity violations.\\n\\n${ESCAPED_OUTPUT}\\n\\nFix these issues before committing:\\n1. Remove warning suppressions - fix the actual warnings\\n2. Uncomment or properly delete tests - don't hide them\\n3. Keep assertions - they exist for a reason\\n\\nIf you believe these are false positives, explain why in the commit message." | ||
| } | ||
| EOF |
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 ${ESCAPED_OUTPUT} variable is unquoted within the heredoc on line 47. This violates the repository style guide (line 61) and can lead to unexpected behavior from word splitting or glob expansion. Since this variable contains escaped output for a JSON message, it's critical to handle it safely. Consider using jq or printf to construct the JSON payload robustly.
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-commit-gate.sh", |
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.
According to the repository style guide (line 61), all shell variables must be quoted to prevent word splitting and globbing issues. The ${CLAUDE_PLUGIN_ROOT} variable should be enclosed in double quotes within the command string.
| "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-commit-gate.sh", | |
| "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-commit-gate.sh\"", |
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/phase-end-check.sh", |
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.
In compliance with the repository style guide (line 61), all shell variables must be quoted. The ${CLAUDE_PLUGIN_ROOT} variable should be quoted here to avoid potential issues with paths containing spaces or special characters.
| "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/phase-end-check.sh", | |
| "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/scripts/phase-end-check.sh\"", |
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
| PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-}" | ||
| [[ -z "$PLUGIN_ROOT" ]] && exit 0 | ||
|
|
||
| BLACKBOARD="$PLUGIN_ROOT/.blackboard" |
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.
Per the repository style guide (line 61), all variables must be quoted. $PLUGIN_ROOT should be quoted to prevent issues if the path contains spaces or special characters. This pattern of unquoted variables appears elsewhere in the script (e.g., $COMPLETION_CLAIM on line 33 and $score on line 79). Please ensure all variable expansions are quoted.
| BLACKBOARD="$PLUGIN_ROOT/.blackboard" | |
| BLACKBOARD="${PLUGIN_ROOT}/.blackboard" |
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
| # Check if there are staged changes | ||
| if ! git diff --cached --quiet 2>/dev/null; then | ||
| # Run the integrity check | ||
| CHECK_OUTPUT=$("$PLUGIN_ROOT/scripts/integrity-check.sh" 2>&1 || 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 repository style guide (line 61) requires all variables to be quoted. The $PLUGIN_ROOT variable should be quoted to prevent word splitting and globbing if the path contains spaces or special characters.
| CHECK_OUTPUT=$("$PLUGIN_ROOT/scripts/integrity-check.sh" 2>&1 || true) | |
| CHECK_OUTPUT=$("${PLUGIN_ROOT}/scripts/integrity-check.sh" 2>&1 || true) |
References
- All shell script variables must be quoted (e.g.,
"$VAR") to prevent word splitting and globbing issues. (link)
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.
Pull request overview
This PR adds a new completion-integrity plugin that prevents Claude from taking shortcuts when completing tasks by detecting and blocking problematic patterns like warning suppressions, commented-out tests, and deleted assertions.
Key changes:
- Pre-commit hook that scans staged changes and blocks commits containing integrity violations
- Phase-end hook that warns when Claude claims completion while indicators suggest premature closure
- Manual integrity check script for on-demand validation
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
plugins/completion-integrity/.claude-plugin/plugin.json |
Plugin manifest defining metadata, keywords, and capability declarations |
plugins/completion-integrity/README.md |
User-facing documentation explaining problem, solution, and usage |
plugins/completion-integrity/skills/completion-integrity/SKILL.md |
Skill definition with YAML frontmatter describing the plugin's behavior |
plugins/completion-integrity/hooks/hooks.json |
Hook configuration for PreToolUse (Bash) and Stop events |
plugins/completion-integrity/hooks/scripts/pre-commit-gate.sh |
Pre-commit hook that intercepts git commit commands and runs integrity checks |
plugins/completion-integrity/hooks/scripts/phase-end-check.sh |
Response-end hook that detects premature completion claims |
plugins/completion-integrity/scripts/integrity-check.sh |
Core validation script checking for 6 categories of integrity violations |
CHANGELOG.md |
Documents the new plugin features under Unreleased section |
.claude-plugin/marketplace.json |
Registers the plugin in the marketplace catalog |
| --- | ||
| name: completion-integrity | ||
| description: Prevents shortcuts and cheating when completing tasks. Blocks commits with warning suppressions, commented tests, or deleted assertions. | ||
| --- | ||
|
|
||
| # Completion Integrity | ||
|
|
||
| This plugin enforces honest task completion by detecting and blocking shortcuts. | ||
|
|
||
| ## What It Catches | ||
|
|
||
| | Pattern | Why It's Bad | | ||
| |---------|--------------| | ||
| | Warning suppression (`#pragma warning disable`, `eslint-disable`) | Hides problems instead of fixing them | | ||
| | Commented-out tests | Tests exist for a reason | | ||
| | Deleted assertions | Removing checks doesn't fix bugs | | ||
| | Test file deletion | Don't delete tests to make them "pass" | | ||
| | Empty catch blocks | Swallowing errors hides failures | | ||
| | Fresh TODOs | Defer work explicitly, not via comments | | ||
|
|
||
| ## Hooks | ||
|
|
||
| ### Pre-Commit Gate (PreToolUse: Bash) | ||
|
|
||
| Runs before any `git commit` command. Scans staged changes for violations. | ||
|
|
||
| **Blocks commit if:** | ||
| - Warning suppressions added | ||
| - Tests commented out | ||
| - Assertions deleted | ||
| - Test files deleted | ||
|
|
||
| ### Phase-End Check (Stop) | ||
|
|
||
| Runs after each Claude response. Detects premature completion claims. | ||
|
|
||
| **Warns if Claude says "done" but:** | ||
| - Didn't actually run tests | ||
| - Dismissed warnings as unimportant | ||
| - Mentioned "for now" or "TODO" | ||
| - Deleted code instead of fixing | ||
|
|
||
| ## Manual Check | ||
|
|
||
| Run the integrity check manually: | ||
|
|
||
| ```bash | ||
| bash "${CLAUDE_PLUGIN_ROOT}/scripts/integrity-check.sh" | ||
| ``` | ||
|
|
||
| ## False Positives | ||
|
|
||
| Sometimes suppressions are legitimate. If blocked: | ||
|
|
||
| 1. Explain WHY the suppression is necessary in the commit message | ||
| 2. The explanation should convince a reviewer | ||
| 3. If you can't explain it, fix the underlying issue instead |
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 SKILL.md file is missing the "## FAILURE CONDITIONS" section. According to the coding guidelines, this section should specify that skipping this skill when it applies equals FAILURE. This helps ensure the skill is properly activated when needed.
| # RULE 6: CATCH-ALL EXCEPTION HANDLERS ADDED | ||
| # ============================================================================= | ||
|
|
||
| CATCH_ALL=$(echo "$ADDED_LINES" | grep -iE 'catch\s*\(\s*(Exception|Error|e|ex|err)?\s*\)\s*\{?\s*\}|except:\s*$|except\s+Exception:' || 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 regex pattern for catching empty catch blocks has a potential false positive issue. The pattern 'catch\s*(\s*(Exception|Error|e|ex|err)?\s*)\s*{?\s*}' will match catch blocks with just the opening brace but may not accurately detect truly empty blocks if there's whitespace or newlines between braces. Consider using a more robust detection that checks the git diff context for empty catch blocks more reliably, or document this limitation.
| CATCH_ALL=$(echo "$ADDED_LINES" | grep -iE 'catch\s*\(\s*(Exception|Error|e|ex|err)?\s*\)\s*\{?\s*\}|except:\s*$|except\s+Exception:' || true) | |
| CATCH_ALL=$(echo "$ADDED_LINES" | grep -iE 'catch\s*\(\s*(Exception|Error|e|ex|err)?\s*\)\s*\{\s*\}|except:\s*$|except\s+Exception:' || true) |
| BLACKBOARD="$PLUGIN_ROOT/.blackboard" | ||
| mkdir -p "$BLACKBOARD" 2>/dev/null || 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 phase-end-check.sh script uses the .blackboard directory (line 14) but never actually uses it. The directory is created but no files are written to or read from this location. Either implement the intended blackboard functionality or remove the unused code to avoid confusion.
| BLACKBOARD="$PLUGIN_ROOT/.blackboard" | |
| mkdir -p "$BLACKBOARD" 2>/dev/null || true |
| if [[ "$CS_ASSERT_DELETED_COUNT" -gt 2 ]]; then | ||
| VIOLATIONS+=("ASSERT_DELETED|$CS_ASSERT_DELETED_COUNT assertions removed") | ||
| echo -e "${RED}ASSERT_DELETED|$CS_ASSERT_DELETED_COUNT assertions deleted${NC}" | ||
| echo "$CS_ASSERT_DELETED" | head -3 | while read -r line; do | ||
| echo " - $line" | ||
| done | ||
| fi | ||
|
|
||
| # JS assertions deleted | ||
| JS_ASSERT_DELETED=$(echo "$DELETED_LINES" | grep -iE 'expect\(|assert\.|should\.' || true) | ||
| JS_ASSERT_DELETED_COUNT=0 | ||
| [[ -n "$JS_ASSERT_DELETED" ]] && JS_ASSERT_DELETED_COUNT=$(echo "$JS_ASSERT_DELETED" | wc -l | tr -d ' ') | ||
| if [[ "$JS_ASSERT_DELETED_COUNT" -gt 2 ]]; then |
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 threshold of "greater than 2" assertions deleted (lines 137, 149) is a magic number without explanation. Why is 2 the threshold? This should either be documented in a comment explaining the rationale, or extracted to a named constant at the top of the script for easier configuration.
| if echo "$RESPONSE" | grep -qiE \ | ||
| "done!|finished!|complete!|all set|that.s it|ready to|we.re done|task complete|implementation complete|fixed!|working now|should work|tests pass"; then |
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 completion claim detection regex on lines 27-28 uses several patterns that could produce false positives. For example, "should work" could appear in legitimate discussion of how something should work, not necessarily as a completion claim. Consider tightening these patterns to reduce false positives, particularly for phrases like "should work" which may appear in non-completion contexts.
| # Escape the output for JSON | ||
| ESCAPED_OUTPUT=$(echo "$CHECK_OUTPUT" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | sed ':a;N;$!ba;s/\n/\\n/g') | ||
|
|
||
| cat << EOF | ||
| { | ||
| "decision": "block", | ||
| "reason": "COMPLETION INTEGRITY: Violations detected in staged changes", | ||
| "message": "BLOCKED: Cannot commit with integrity violations.\\n\\n${ESCAPED_OUTPUT}\\n\\nFix these issues before committing:\\n1. Remove warning suppressions - fix the actual warnings\\n2. Uncomment or properly delete tests - don't hide them\\n3. Keep assertions - they exist for a reason\\n\\nIf you believe these are false positives, explain why in the commit message." | ||
| } | ||
| EOF |
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 JSON escaping logic using sed is fragile and may not handle all edge cases correctly. Special characters like newlines, tabs, and backslashes in the check output could break the JSON structure. Consider using a more robust JSON escaping approach, such as using jq to construct the JSON object or a proper JSON escaping function.
| # Escape the output for JSON | |
| ESCAPED_OUTPUT=$(echo "$CHECK_OUTPUT" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | sed ':a;N;$!ba;s/\n/\\n/g') | |
| cat << EOF | |
| { | |
| "decision": "block", | |
| "reason": "COMPLETION INTEGRITY: Violations detected in staged changes", | |
| "message": "BLOCKED: Cannot commit with integrity violations.\\n\\n${ESCAPED_OUTPUT}\\n\\nFix these issues before committing:\\n1. Remove warning suppressions - fix the actual warnings\\n2. Uncomment or properly delete tests - don't hide them\\n3. Keep assertions - they exist for a reason\\n\\nIf you believe these are false positives, explain why in the commit message." | |
| } | |
| EOF | |
| # Use jq to safely construct JSON and escape the check output | |
| jq -n --arg check_output "$CHECK_OUTPUT" '{ | |
| decision: "block", | |
| reason: "COMPLETION INTEGRITY: Violations detected in staged changes", | |
| message: ( | |
| "BLOCKED: Cannot commit with integrity violations.\n\n" | |
| + $check_output | |
| + "\n\nFix these issues before committing:\n1. Remove warning suppressions - fix the actual warnings\n2. Uncomment or properly delete tests - don't hide them\n3. Keep assertions - they exist for a reason\n\nIf you believe these are false positives, explain why in the commit message." | |
| ) | |
| }' |
| # ============================================================================= | ||
|
|
||
| DELETED_FILES=$(git diff --cached --name-only --diff-filter=D 2>/dev/null || true) | ||
| DELETED_TEST_FILES=$(echo "$DELETED_FILES" | grep -iE '\.test\.|\.spec\.|_test\.|Tests\.cs|Test\.cs' || 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 script filters out test files from the integrity check using patterns like '/.test.' and '/.spec.', but then Rule 5 detects deleted test files. This creates an inconsistency: modifications to test files are excluded from checks, but deletions are flagged. This could be confusing. Consider either checking test files for violations or not flagging their deletion, to maintain consistency.
| DELETED_TEST_FILES=$(echo "$DELETED_FILES" | grep -iE '\.test\.|\.spec\.|_test\.|Tests\.cs|Test\.cs' || true) | |
| DELETED_TEST_FILES=$(echo "$DELETED_FILES" | grep -iE '_test\.|Tests\.cs|Test\.cs' || true) |
| # OUTPUT WARNING IF ISSUES DETECTED | ||
| # ============================================================================= | ||
|
|
||
| if [[ "$score" -gt 10 ]]; then |
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 score threshold of 10 (line 79) is a magic number without documentation. It's unclear why 10 is the cutoff for triggering a warning versus staying silent. Add a comment explaining the scoring system and why this threshold was chosen, or extract it to a named constant with a descriptive name.
| - `#pragma warning disable` / `eslint-disable` / `# noqa` | ||
| - `// [Test]` (commented-out test attributes) | ||
| - Deleted `Assert.*` / `expect()` calls | ||
| - Deleted test files |
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.
In the README.md, the "What Gets Blocked" section doesn't mention all the violation types that are actually detected by the integrity-check.sh script. The script also detects empty catch blocks (RULE 6) which isn't listed here. Update the documentation to include all blocked patterns for completeness.
| - Deleted test files | |
| - Deleted test files | |
| - Empty `catch` blocks (swallowed exceptions) |
| # Escape the output for JSON | ||
| ESCAPED_OUTPUT=$(echo "$CHECK_OUTPUT" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | sed ':a;N;$!ba;s/\n/\\n/g') | ||
|
|
||
| cat << EOF | ||
| { | ||
| "decision": "block", | ||
| "reason": "COMPLETION INTEGRITY: Violations detected in staged changes", | ||
| "message": "BLOCKED: Cannot commit with integrity violations.\\n\\n${ESCAPED_OUTPUT}\\n\\nFix these issues before committing:\\n1. Remove warning suppressions - fix the actual warnings\\n2. Uncomment or properly delete tests - don't hide them\\n3. Keep assertions - they exist for a reason\\n\\nIf you believe these are false positives, explain why in the commit message." | ||
| } | ||
| EOF |
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 error message in line 47 uses escaped newlines in a heredoc-style JSON construction, but this could become difficult to read and maintain. The message is quite long and mixes instructions with output. Consider breaking this into smaller, more maintainable pieces or using jq to construct the JSON with proper multiline string handling.
| # Escape the output for JSON | |
| ESCAPED_OUTPUT=$(echo "$CHECK_OUTPUT" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | sed ':a;N;$!ba;s/\n/\\n/g') | |
| cat << EOF | |
| { | |
| "decision": "block", | |
| "reason": "COMPLETION INTEGRITY: Violations detected in staged changes", | |
| "message": "BLOCKED: Cannot commit with integrity violations.\\n\\n${ESCAPED_OUTPUT}\\n\\nFix these issues before committing:\\n1. Remove warning suppressions - fix the actual warnings\\n2. Uncomment or properly delete tests - don't hide them\\n3. Keep assertions - they exist for a reason\\n\\nIf you believe these are false positives, explain why in the commit message." | |
| } | |
| EOF | |
| jq -n --arg output "$CHECK_OUTPUT" ' | |
| { | |
| decision: "block", | |
| reason: "COMPLETION INTEGRITY: Violations detected in staged changes", | |
| message: | |
| "BLOCKED: Cannot commit with integrity violations.\n\n" | |
| + $output + "\n\n" | |
| + "Fix these issues before committing:\n" | |
| + "1. Remove warning suppressions - fix the actual warnings\n" | |
| + "2. Uncomment or properly delete tests - do not hide them\n" | |
| + "3. Keep assertions - they exist for a reason\n\n" | |
| + "If you believe these are false positives, explain why in the commit message." | |
| } | |
| ' |
Changes based on Gemini and Copilot reviews:
JSON Construction:
- Use jq for safe JSON construction instead of sed escaping
- Properly escape all special characters in output
Variable Quoting:
- Quote all shell variables with ${VAR} syntax
- Quote CLAUDE_PLUGIN_ROOT in hooks.json command strings
Documentation:
- Add FAILURE CONDITIONS section to SKILL.md
- Document scoring thresholds and rationale
- Document assertion deletion threshold (>2)
- Document TODO threshold (>2)
- Update README with all violation types including empty catch blocks
Bug Fixes:
- Fix git pathspec syntax (single -- separator with all exclusions)
- Remove unused BLACKBOARD directory code from phase-end-check.sh
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
git commitwith integrity violationsWhat It Detects
#pragma warning disable,eslint-disable,# noqa// [Fact],// it().skip(),[Skip])Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.