-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add pre-commit hooks for cycles, dead exports, signature warnings #381
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
079748b
77a2b30
02bb105
b3f6f41
9880b4d
9c1cb2a
35d2eb8
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,177 @@ | ||
| #!/usr/bin/env bash | ||
| # check-commit.sh — PreToolUse hook for Bash (git commit) | ||
| # Combined cycle-detection (blocking) + signature-change warning (informational). | ||
| # Runs checkData() ONCE with both predicates, single Node.js process. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| INPUT=$(cat) | ||
|
|
||
| # Extract the command from tool_input JSON | ||
| COMMAND=$(echo "$INPUT" | node -e " | ||
| let d=''; | ||
| process.stdin.on('data',c=>d+=c); | ||
| process.stdin.on('end',()=>{ | ||
| const p=JSON.parse(d).tool_input?.command||''; | ||
| if(p)process.stdout.write(p); | ||
| }); | ||
| " 2>/dev/null) || true | ||
|
|
||
| if [ -z "$COMMAND" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Only trigger on git commit commands | ||
| if ! echo "$COMMAND" | grep -qE '(^|\s|&&\s*)git\s+commit\b'; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Guard: codegraph DB must exist | ||
| WORK_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || WORK_ROOT="${CLAUDE_PROJECT_DIR:-.}" | ||
| if [ ! -f "$WORK_ROOT/.codegraph/graph.db" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Guard: must have staged changes | ||
| STAGED=$(git diff --cached --name-only 2>/dev/null) || true | ||
| if [ -z "$STAGED" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Load session edit log for cycle scoping | ||
| LOG_FILE="$WORK_ROOT/.claude/session-edits.log" | ||
| EDITED_FILES="" | ||
| if [ -f "$LOG_FILE" ] && [ -s "$LOG_FILE" ]; then | ||
| EDITED_FILES=$(awk '{print $2}' "$LOG_FILE" | sort -u) | ||
| fi | ||
|
|
||
| # Single Node.js invocation: run checkData once, process both predicates | ||
| RESULT=$(node -e " | ||
| const path = require('path'); | ||
| const root = process.argv[1]; | ||
| const editedRaw = process.argv[2] || ''; | ||
|
|
||
| const { checkData } = require(path.join(root, 'src/check.js')); | ||
| const { openReadonlyOrFail } = require(path.join(root, 'src/db.js')); | ||
|
|
||
| // Run check with cycles + signatures only (skip boundaries for speed) | ||
| const data = checkData(undefined, { | ||
| staged: true, | ||
| noTests: true, | ||
| boundaries: false, | ||
| }); | ||
|
|
||
| if (!data || data.error || !data.predicates) process.exit(0); | ||
|
|
||
| const output = { action: 'allow' }; | ||
|
|
||
| // ── Cycle check (blocking) ── | ||
| const cyclesPred = data.predicates.find(p => p.name === 'cycles'); | ||
| if (cyclesPred && !cyclesPred.passed && cyclesPred.cycles?.length) { | ||
| const edited = new Set(editedRaw.split('\n').filter(Boolean)); | ||
| // Only block if cycles involve files edited in this session | ||
| if (edited.size > 0) { | ||
| const relevant = cyclesPred.cycles.filter( | ||
| cycle => cycle.some(f => edited.has(f)) | ||
| ); | ||
| if (relevant.length > 0) { | ||
| const summary = relevant.slice(0, 5).map(c => c.join(' -> ')).join('\n '); | ||
| const extra = relevant.length > 5 ? '\n ... and ' + (relevant.length - 5) + ' more' : ''; | ||
| output.action = 'deny'; | ||
| output.reason = 'BLOCKED: Circular dependencies detected involving files you edited:\n ' + summary + extra + '\nFix the cycles before committing.'; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ── Signature warning (informational, never blocks) ── | ||
| const sigPred = data.predicates.find(p => p.name === 'signatures'); | ||
| if (sigPred && !sigPred.passed && sigPred.violations?.length) { | ||
| // Enrich with role + transitive caller count using a single DB connection | ||
| const db = openReadonlyOrFail(); | ||
| const lines = []; | ||
| try { | ||
| const stmtNode = db.prepare( | ||
| 'SELECT id, role FROM nodes WHERE name = ? AND file = ? AND line = ?' | ||
| ); | ||
| const stmtCallers = db.prepare( | ||
| 'SELECT DISTINCT n.id FROM edges e JOIN nodes n ON e.source_id = n.id WHERE e.target_id = ? AND e.kind = \\'calls\\'' | ||
| ); | ||
|
|
||
| for (const v of sigPred.violations) { | ||
| const node = stmtNode.get(v.name, v.file, v.line); | ||
| const role = node?.role || 'unknown'; | ||
|
|
||
| let callerCount = 0; | ||
| if (node) { | ||
| const visited = new Set([node.id]); | ||
| let frontier = [node.id]; | ||
| for (let d = 0; d < 3; d++) { | ||
| const next = []; | ||
| for (const fid of frontier) { | ||
| for (const c of stmtCallers.all(fid)) { | ||
| if (!visited.has(c.id)) { | ||
| visited.add(c.id); | ||
| next.push(c.id); | ||
| callerCount++; | ||
| } | ||
| } | ||
| } | ||
| frontier = next; | ||
| if (!frontier.length) break; | ||
| } | ||
| } | ||
|
|
||
| const risk = role === 'core' ? 'HIGH' : role === 'utility' ? 'MEDIUM' : 'LOW'; | ||
| lines.push(risk + ': ' + v.name + ' (' + v.kind + ') [' + role + '] at ' + v.file + ':' + v.line + ' — ' + callerCount + ' transitive callers'); | ||
| } | ||
| } finally { | ||
| db.close(); | ||
| } | ||
|
|
||
| if (lines.length > 0) { | ||
| output.sigWarning = lines.join('\n'); | ||
| } | ||
| } | ||
|
|
||
| process.stdout.write(JSON.stringify(output)); | ||
| " "$WORK_ROOT" "$EDITED_FILES" 2>/dev/null) || true | ||
|
|
||
| if [ -z "$RESULT" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| ACTION=$(echo "$RESULT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{process.stdout.write(JSON.parse(d).action||'allow')}catch{process.stdout.write('allow')}})" 2>/dev/null) || ACTION="allow" | ||
|
|
||
| if [ "$ACTION" = "deny" ]; then | ||
| REASON=$(echo "$RESULT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{process.stdout.write(JSON.parse(d).reason||'')}catch{}})" 2>/dev/null) || true | ||
| node -e " | ||
| console.log(JSON.stringify({ | ||
| hookSpecificOutput: { | ||
| hookEventName: 'PreToolUse', | ||
| permissionDecision: 'deny', | ||
| permissionDecisionReason: process.argv[1] | ||
| } | ||
| })); | ||
| " "$REASON" | ||
| exit 0 | ||
| fi | ||
|
Comment on lines
+145
to
+157
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Signature warnings silently dropped when a cycle block also fires When Consider including any output.action = 'deny';
output.reason = 'BLOCKED: Circular dependencies detected ...\n'
+ (output.sigWarning ? '\n--- Signature warnings ---\n' + output.sigWarning : ''); |
||
|
|
||
| # Signature warning (non-blocking) | ||
| SIG_WARNING=$(echo "$RESULT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{const w=JSON.parse(d).sigWarning;if(w)process.stdout.write(w)}catch{}})" 2>/dev/null) || true | ||
|
|
||
| if [ -n "$SIG_WARNING" ]; then | ||
| ESCAPED=$(printf '%s' "$SIG_WARNING" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>process.stdout.write(JSON.stringify(d)))" 2>/dev/null) || true | ||
| if [ -n "$ESCAPED" ]; then | ||
| node -e " | ||
| console.log(JSON.stringify({ | ||
| hookSpecificOutput: { | ||
| hookEventName: 'PreToolUse', | ||
| permissionDecision: 'allow', | ||
| additionalContext: '[codegraph] Signature changes detected in staged files:\\n' + JSON.parse(process.argv[1]) | ||
| } | ||
| })); | ||
| " "$ESCAPED" 2>/dev/null || true | ||
| fi | ||
| fi | ||
|
|
||
| exit 0 | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,104 @@ | ||||||
| #!/usr/bin/env bash | ||||||
| # check-dead-exports.sh — PreToolUse hook for Bash (git commit) | ||||||
| # Blocks commits if any src/ file edited in THIS SESSION has exports with zero consumers. | ||||||
| # Batches all files in a single Node.js invocation (one DB open) for speed. | ||||||
|
|
||||||
| set -euo pipefail | ||||||
|
|
||||||
| INPUT=$(cat) | ||||||
|
|
||||||
| # Extract the command from tool_input JSON | ||||||
| COMMAND=$(echo "$INPUT" | node -e " | ||||||
| let d=''; | ||||||
| process.stdin.on('data',c=>d+=c); | ||||||
| process.stdin.on('end',()=>{ | ||||||
| const p=JSON.parse(d).tool_input?.command||''; | ||||||
| if(p)process.stdout.write(p); | ||||||
| }); | ||||||
| " 2>/dev/null) || true | ||||||
|
|
||||||
| if [ -z "$COMMAND" ]; then | ||||||
| exit 0 | ||||||
| fi | ||||||
|
|
||||||
| # Only trigger on git commit commands | ||||||
| if ! echo "$COMMAND" | grep -qE '(^|\s|&&\s*)git\s+commit\b'; then | ||||||
| exit 0 | ||||||
| fi | ||||||
|
|
||||||
| # Guard: codegraph DB must exist | ||||||
| WORK_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || WORK_ROOT="${CLAUDE_PROJECT_DIR:-.}" | ||||||
| if [ ! -f "$WORK_ROOT/.codegraph/graph.db" ]; then | ||||||
| exit 0 | ||||||
| fi | ||||||
|
|
||||||
| # Guard: must have staged changes | ||||||
| STAGED=$(git diff --cached --name-only 2>/dev/null) || true | ||||||
| if [ -z "$STAGED" ]; then | ||||||
| exit 0 | ||||||
| fi | ||||||
|
|
||||||
| # Load session edit log to scope checks to files we actually edited | ||||||
| LOG_FILE="$WORK_ROOT/.claude/session-edits.log" | ||||||
| if [ ! -f "$LOG_FILE" ] || [ ! -s "$LOG_FILE" ]; then | ||||||
| exit 0 | ||||||
| fi | ||||||
| EDITED_FILES=$(awk '{print $2}' "$LOG_FILE" | sort -u) | ||||||
|
|
||||||
| # Filter staged files to src/*.js that were edited in this session | ||||||
| FILES_TO_CHECK="" | ||||||
| while IFS= read -r file; do | ||||||
| if ! echo "$file" | grep -qE '^src/.*\.(js|ts|tsx)$'; then | ||||||
| continue | ||||||
| fi | ||||||
| if echo "$EDITED_FILES" | grep -qxF "$file"; then | ||||||
| FILES_TO_CHECK="${FILES_TO_CHECK:+$FILES_TO_CHECK | ||||||
| }$file" | ||||||
| fi | ||||||
| done <<< "$STAGED" | ||||||
|
|
||||||
| if [ -z "$FILES_TO_CHECK" ]; then | ||||||
| exit 0 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "One DB connection" claim is inaccurate. The comment states "check all files with one DB connection", but export function exportsData(file, customDbPath, opts = {}) {
const db = openReadonlyOrFail(customDbPath); // opens
// ...
db.close(); // closes
}Because the hook loops over Consider updating the comment to say "single Node.js process" (accurate) instead of "one DB connection":
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in b3f6f41 — changed comment to "in one process" (accurate) instead of "one DB connection" (misleading). The real benefit is batching into a single Node.js process, not sharing a DB connection. |
||||||
| fi | ||||||
|
|
||||||
| # Single Node.js invocation: check all files in one process | ||||||
| DEAD_EXPORTS=$(node -e " | ||||||
| const path = require('path'); | ||||||
| const root = process.argv[1]; | ||||||
| const files = process.argv[2].split('\n').filter(Boolean); | ||||||
|
|
||||||
| const { exportsData } = require(path.join(root, 'src/queries.js')); | ||||||
|
|
||||||
| const dead = []; | ||||||
| for (const file of files) { | ||||||
| try { | ||||||
| const data = exportsData(file, undefined, { noTests: true, unused: true }); | ||||||
| if (data && data.results) { | ||||||
| for (const r of data.results) { | ||||||
| dead.push(r.name + ' (' + data.file + ':' + r.line + ')'); | ||||||
| } | ||||||
| } | ||||||
| } catch {} | ||||||
| } | ||||||
|
|
||||||
| if (dead.length > 0) { | ||||||
| process.stdout.write(dead.join(', ')); | ||||||
| } | ||||||
| " "$WORK_ROOT" "$FILES_TO_CHECK" 2>/dev/null) || true | ||||||
|
|
||||||
| if [ -n "$DEAD_EXPORTS" ]; then | ||||||
| REASON="BLOCKED: Dead exports (zero consumers) detected in files you edited: $DEAD_EXPORTS. Either add consumers, remove the exports, or verify these are intentionally public API." | ||||||
|
|
||||||
| node -e " | ||||||
| console.log(JSON.stringify({ | ||||||
| hookSpecificOutput: { | ||||||
| hookEventName: 'PreToolUse', | ||||||
| permissionDecision: 'deny', | ||||||
| permissionDecisionReason: process.argv[1] | ||||||
| } | ||||||
| })); | ||||||
| " "$REASON" | ||||||
| exit 0 | ||||||
| fi | ||||||
|
|
||||||
| exit 0 | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| #!/usr/bin/env bash | ||
| # Block PR creation if the body contains "generated with" (case-insensitive) | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| INPUT=$(cat) | ||
|
|
||
| # Extract just the command field to avoid false positives on the description field | ||
| cmd=$(echo "$INPUT" | node -e " | ||
| let d=''; | ||
| process.stdin.on('data',c=>d+=c); | ||
| process.stdin.on('end',()=>{ | ||
| const p=JSON.parse(d).tool_input?.command||''; | ||
| if(p)process.stdout.write(p); | ||
| }); | ||
| " 2>/dev/null) || true | ||
|
|
||
| echo "$cmd" | grep -qi 'gh pr create' || exit 0 | ||
|
|
||
| # Block if body contains "generated with" | ||
| if echo "$cmd" | grep -qi 'generated with'; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The check inspects Claude Code regularly uses To close the gap, extract the # Block if body contains "generated with" — inline body
if echo "$cmd" | grep -qi 'generated with'; then
# ... deny ...
fi
# Also check --body-file path
BODY_FILE=$(echo "$cmd" | grep -oP '(?<=--body-file\s)\S+' || true)
if [ -n "$BODY_FILE" ] && [ -f "$BODY_FILE" ]; then
if grep -qi 'generated with' "$BODY_FILE"; then
# ... deny ...
fi
fi
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 9880b4d — the hook now extracts the \ path from the command and inspects that file's content for "generated with", closing the bypass. |
||
| node -e " | ||
| console.log(JSON.stringify({ | ||
| hookSpecificOutput: { | ||
| hookEventName: 'PreToolUse', | ||
| permissionDecision: 'deny', | ||
| permissionDecisionReason: 'BLOCKED: Remove any \'Generated with ...\' line from the PR body.' | ||
| } | ||
| })); | ||
| " | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Also check --body-file path | ||
| BODY_FILE=$(echo "$cmd" | grep -oP '(?<=--body-file\s)\S+' || true) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The lookbehind regex A more robust extraction handles optional quotes: BODY_FILE=$(echo "$cmd" | grep -oP '(?<=--body-file\s)["\x27]?\S+["\x27]?' | tr -d "'\"" || true)Or, using a POSIX-safe BODY_FILE=$(echo "$cmd" | sed -nE "s/.*--body-file[[:space:]]+'?\"?([^'\" ]+).*/\1/p" || true) |
||
| if [ -n "$BODY_FILE" ] && [ -f "$BODY_FILE" ]; then | ||
| if grep -qi 'generated with' "$BODY_FILE"; then | ||
| node -e " | ||
| console.log(JSON.stringify({ | ||
| hookSpecificOutput: { | ||
| hookEventName: 'PreToolUse', | ||
| permissionDecision: 'deny', | ||
| permissionDecisionReason: 'BLOCKED: Remove any \'Generated with ...\' line from the PR body file.' | ||
| } | ||
| })); | ||
| " | ||
| exit 0 | ||
| fi | ||
| fi | ||
|
|
||
| exit 0 | ||
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.
Signature warning not scoped to session-edited files
The cycle check (lines 68–84) explicitly filters to cycles that involve files from the session edit log. The signature warning block here has no equivalent filter — it processes
sigPred.violationsunconditionally for all staged files, regardless of whether you authored those changes.In practice this means that after a
git mergeorgit cherry-pick, any signature changes introduced by a teammate will also fire warnings, even though the current session author has no context about them. The cycle check avoids this noise by scoping toedited; the sig check should too: