fix: make hooks work across VS Code Copilot, Copilot CLI, and Claude Code

viragtripathi · viragtripathi · commit ba604408eac6 · 2026-06-30T21:09:12.000-04:00
The safety hooks emitted only the VS Code / Claude output shape
(hookSpecificOutput and systemMessage), which the GitHub Copilot CLI ignores.
The CLI reads a top-level permissionDecision and additionalContext instead.
Both scripts now emit both shapes, so a blocked statement is denied and a lint
is surfaced regardless of which tool runs the hook.

check-sql-files.py also accepts the file path under file_path, filePath, or
path, and reads the toolArgs container the CLI uses in addition to tool_input.

Adds scripts/test-hooks.sh (run in CI) covering both output shapes, the
container and path-key variants, and the fail-open behavior for issues #20
and #23.
diff --git a/.github/workflows/test-hooks.yml b/.github/workflows/test-hooks.yml
@@ -0,0 +1,26 @@
+name: Test hooks
+
+on:
+  pull_request:
+    paths:
+      - '.github/hooks/**'
+      - 'scripts/**'
+      - '.github/workflows/test-hooks.yml'
+  push:
+    branches:
+      - main
+    paths:
+      - '.github/hooks/**'
+      - 'scripts/**'
+      - '.github/workflows/test-hooks.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  test-hooks:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Run hook regression tests
+        run: bash scripts/test-hooks.sh
diff --git a/scripts/check-sql-files.py b/scripts/check-sql-files.py
@@ -1,6 +1,10 @@
 #!/usr/bin/env python3
 """Post-edit check for CockroachDB anti-patterns in SQL and code files.
-Receives JSON on stdin from Claude Code PostToolUse hook.
+Receives JSON on stdin from a PostToolUse hook.
+
+The advisory message is emitted under both ``systemMessage`` (VS Code Copilot /
+Claude Code, user-visible) and ``additionalContext`` (GitHub Copilot CLI,
+model-visible), so the lint surfaces regardless of which tool runs the hook.
 """
 
 import json
@@ -18,12 +22,20 @@ def main():
     except (json.JSONDecodeError, EOFError):
         sys.exit(0)
 
-    # Claude Code uses snake_case (file_path); VS Code / Copilot tools use
-    # camelCase (filePath). Accept either so the hook fires on both.
-    tool_input = data.get("tool_input", {}) or data.get("toolInput", {})
+    # tool_input/toolInput (VS Code / Claude) or toolArgs (Copilot CLI). File
+    # path key varies by tool: file_path (Claude), filePath / path (others).
+    tool_input = (
+        data.get("tool_input")
+        or data.get("toolInput")
+        or data.get("toolArgs")
+        or {}
+    )
+    if not isinstance(tool_input, dict):
+        sys.exit(0)
     file_path = (
         tool_input.get("file_path")
         or tool_input.get("filePath")
+        or tool_input.get("path")
         or ""
     )
     if not file_path:
@@ -46,13 +58,13 @@ def main():
 
     if re.search(r"\b(SERIAL|BIGSERIAL)\b", content, re.IGNORECASE):
         warnings.append(
-            "SERIAL/BIGSERIAL detected — causes write hotspots in CockroachDB, "
+            "SERIAL/BIGSERIAL detected: causes write hotspots in CockroachDB, "
             "use UUID with gen_random_uuid() instead."
         )
 
     if re.search(r"SELECT\s+\*\s+FROM", content, re.IGNORECASE):
         warnings.append(
-            "SELECT * detected — enumerate columns explicitly for CockroachDB "
+            "SELECT * detected: enumerate columns explicitly for CockroachDB "
             "to enable covering index optimizations."
         )
 
@@ -61,7 +73,7 @@ def main():
         if re.search(r"\bBEGIN\b|sql\.Tx", content):
             if not re.search(r"crdb\.ExecuteTx|retry|40001", content, re.IGNORECASE):
                 warnings.append(
-                    "Transaction without retry logic detected — CockroachDB requires "
+                    "Transaction without retry logic detected: CockroachDB requires "
                     "retry on SQLSTATE 40001 (serialization_failure). "
                     "Use crdb.ExecuteTx from cockroach-go."
                 )
@@ -71,14 +83,16 @@ def main():
         if re.search(r"\bBEGIN\b|connection\.setAutoCommit", content):
             if not re.search(r"retry|40001|RetryableExecutor", content, re.IGNORECASE):
                 warnings.append(
-                    "Transaction without retry logic detected — CockroachDB requires "
+                    "Transaction without retry logic detected: CockroachDB requires "
                     "retry on SQLSTATE 40001. "
                     "Use cockroachdb-jdbc-wrapper RetryableExecutor."
                 )
 
     if warnings:
+        message = "CockroachDB lint: " + " ".join(warnings)
         json.dump({
-            "systemMessage": "CockroachDB lint: " + " ".join(warnings)
+            "systemMessage": message,
+            "additionalContext": message,
         }, sys.stdout)
 
     sys.exit(0)
diff --git a/scripts/test-hooks.sh b/scripts/test-hooks.sh
@@ -0,0 +1,66 @@
+#!/usr/bin/env bash
+# Regression tests for the CockroachDB safety hooks (Copilot plugin).
+#
+# Runs the real commands from .github/hooks/cockroachdb.json (with
+# ${CLAUDE_PLUGIN_ROOT} substituted) and asserts the output satisfies BOTH
+# hook output contracts, so the hooks work across every Copilot surface:
+#   - VS Code Copilot / Claude Code: hookSpecificOutput + systemMessage
+#   - GitHub Copilot CLI:            top-level permissionDecision + additionalContext
+#
+# Also asserts fail-open (exit 0, no block) when the plugin root cannot be
+# resolved (issues #20 MAX_PATH and #23 unsubstituted token).
+set -uo pipefail
+
+ROOT="$(cd "$(dirname "$0")/.." && pwd)"
+HOOKS="$ROOT/.github/hooks/cockroachdb.json"
+TMP="$(mktemp -d)"
+trap 'rm -rf "$TMP"' EXIT
+fails=0
+
+hook_cmd() { # $1=event  $2=root
+  python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["hooks"][sys.argv[2]][0]["hooks"][0]["command"].replace("${CLAUDE_PLUGIN_ROOT}", sys.argv[3]))' "$HOOKS" "$1" "$2"
+}
+
+# check: desc, event, root, stdin, want_rc, mode(empty|all), space-separated tokens
+check() {
+  local desc="$1" event="$2" root="$3" stdin="$4" want_rc="$5" mode="$6" tokens="${7:-}"
+  local cmd out rc ok=1
+  cmd="$(hook_cmd "$event" "$root")"
+  out="$(printf '%s' "$stdin" | sh -c "$cmd" 2>/dev/null)"; rc=$?
+  [ "$rc" = "$want_rc" ] || ok=0
+  if [ "$mode" = "empty" ]; then
+    [ -z "$out" ] || ok=0
+  else
+    for t in $tokens; do printf '%s' "$out" | grep -q "$t" || ok=0; done
+  fi
+  if [ "$ok" = 1 ]; then echo "ok   - $desc"; else echo "FAIL - $desc (rc=$rc, out=${out:-<empty>})"; fails=$((fails + 1)); fi
+}
+
+printf 'CREATE TABLE t (id SERIAL PRIMARY KEY);\n' > "$TMP/a.sql"
+printf '# markdown, not sql\n' > "$TMP/a.md"
+
+# PreToolUse: deny must appear in BOTH shapes
+check "DROP DATABASE deny (both shapes)" PreToolUse "$ROOT" '{"tool_input":{"sql":"DROP DATABASE x"}}'        0 all "permissionDecision hookSpecificOutput"
+check "TRUNCATE deny (both shapes)"       PreToolUse "$ROOT" '{"tool_input":{"sql":"TRUNCATE TABLE t"}}'       0 all "permissionDecision hookSpecificOutput"
+check "toolArgs container deny (CLI)"     PreToolUse "$ROOT" '{"toolArgs":{"sql":"DROP DATABASE x"}}'          0 all "permissionDecision"
+check "SERIAL warn (both shapes)"         PreToolUse "$ROOT" '{"tool_input":{"sql":"CREATE TABLE t (id SERIAL)"}}' 0 all "systemMessage additionalContext"
+check "safe SQL: no output"               PreToolUse "$ROOT" '{"tool_input":{"sql":"SELECT 1"}}'               0 empty
+
+# PostToolUse: lint in BOTH shapes; all path-key aliases; CLI container
+check "lint .sql via file_path (both)"    PostToolUse "$ROOT" "{\"tool_input\":{\"file_path\":\"$TMP/a.sql\"}}" 0 all "systemMessage additionalContext"
+check "lint via filePath key"             PostToolUse "$ROOT" "{\"tool_input\":{\"filePath\":\"$TMP/a.sql\"}}"  0 all "systemMessage"
+check "lint via path key"                 PostToolUse "$ROOT" "{\"tool_input\":{\"path\":\"$TMP/a.sql\"}}"      0 all "systemMessage"
+check "lint via toolArgs container (CLI)" PostToolUse "$ROOT" "{\"toolArgs\":{\"filePath\":\"$TMP/a.sql\"}}"    0 all "additionalContext"
+check "non-SQL edit: no output"           PostToolUse "$ROOT" "{\"tool_input\":{\"file_path\":\"$TMP/a.md\"}}"  0 empty
+
+# fail-open when plugin root is missing / unsubstituted (issues #20, #23)
+check "PreToolUse fail-open on bad root"  PreToolUse  '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}'                0 empty
+check "PostToolUse fail-open on bad root" PostToolUse '${CLAUDE_PLUGIN_ROOT}' "{\"tool_input\":{\"file_path\":\"$TMP/a.sql\"}}"        0 empty
+
+echo
+if [ "$fails" -eq 0 ]; then
+  echo "All hook regression tests passed."
+else
+  echo "$fails test(s) failed."
+  exit 1
+fi
diff --git a/scripts/validate-sql.py b/scripts/validate-sql.py
@@ -1,77 +1,79 @@
 #!/usr/bin/env python3
 """Pre-execution SQL validation for CockroachDB.
 Blocks dangerous patterns before they reach the database.
-Receives JSON on stdin from Claude Code PreToolUse hook.
-Exit 0 = allow, exit 2 = block.
+Receives JSON on stdin from a PreToolUse hook.
+
+Output is written so both hook contracts understand it:
+- GitHub Copilot CLI reads a top-level ``permissionDecision`` (and
+  ``additionalContext`` for guidance).
+- VS Code Copilot and Claude Code read ``hookSpecificOutput`` (and
+  ``systemMessage``).
+Emitting both keeps this one script working across all three surfaces.
 """
 
 import json
 import re
 import sys
 
 
+def deny(reason):
+    json.dump({
+        "permissionDecision": "deny",
+        "permissionDecisionReason": reason,
+        "hookSpecificOutput": {
+            "hookEventName": "PreToolUse",
+            "permissionDecision": "deny",
+            "permissionDecisionReason": reason,
+        },
+    }, sys.stdout)
+    sys.exit(0)
+
+
+def warn(message):
+    json.dump({
+        "systemMessage": message,
+        "additionalContext": message,
+    }, sys.stdout)
+    sys.exit(0)
+
+
 def main():
     try:
         data = json.load(sys.stdin)
     except (json.JSONDecodeError, EOFError):
         sys.exit(0)
 
-    tool_input = data.get("tool_input", {})
+    # tool_input (VS Code / Claude) or toolArgs (Copilot CLI camelCase)
+    tool_input = data.get("tool_input") or data.get("toolArgs") or {}
+    if not isinstance(tool_input, dict):
+        sys.exit(0)
     sql = tool_input.get("sql", "") or tool_input.get("statement", "")
     if not sql:
         sys.exit(0)
 
     sql_upper = sql.upper()
 
-    # Block DROP DATABASE
     if re.search(r"DROP\s+DATABASE", sql_upper):
-        json.dump({
-            "hookSpecificOutput": {
-                "hookEventName": "PreToolUse",
-                "permissionDecision": "deny",
-                "permissionDecisionReason":
-                    "DROP DATABASE is blocked by CockroachDB plugin safety hook. "
-                    "Use DROP TABLE for individual tables instead."
-            }
-        }, sys.stdout)
-        sys.exit(0)
+        deny("DROP DATABASE is blocked by CockroachDB plugin safety hook. "
+             "Use DROP TABLE for individual tables instead.")
 
-    # Block TRUNCATE (data loss risk)
     if re.search(r"^\s*TRUNCATE\s", sql_upper, re.MULTILINE):
-        json.dump({
-            "hookSpecificOutput": {
-                "hookEventName": "PreToolUse",
-                "permissionDecision": "deny",
-                "permissionDecisionReason":
-                    "TRUNCATE is blocked by CockroachDB plugin safety hook. "
-                    "Use DELETE with a WHERE clause for targeted row removal."
-            }
-        }, sys.stdout)
-        sys.exit(0)
+        deny("TRUNCATE is blocked by CockroachDB plugin safety hook. "
+             "Use DELETE with a WHERE clause for targeted row removal.")
 
-    # Warn about SERIAL (anti-pattern — causes hotspots)
     if re.search(r"\b(SERIAL|BIGSERIAL)\b", sql_upper):
-        json.dump({
-            "systemMessage":
-                "WARNING: SERIAL/BIGSERIAL creates sequential IDs that cause write "
-                "hotspots in CockroachDB. Use UUID with gen_random_uuid() instead: "
-                "id UUID PRIMARY KEY DEFAULT gen_random_uuid()"
-        }, sys.stdout)
-        sys.exit(0)
+        warn("WARNING: SERIAL/BIGSERIAL creates sequential IDs that cause write "
+             "hotspots in CockroachDB. Use UUID with gen_random_uuid() instead: "
+             "id UUID PRIMARY KEY DEFAULT gen_random_uuid()")
 
-    # Warn about multiple DDL in one transaction
     ddl_count = len(re.findall(
         r"(CREATE|ALTER|DROP)\s+(TABLE|INDEX|VIEW|SEQUENCE|TYPE|SCHEMA)",
         sql_upper
     ))
     if ddl_count > 1:
-        json.dump({
-            "systemMessage":
-                "WARNING: Multiple DDL statements detected. CockroachDB supports "
-                "only one DDL per transaction. Split into separate statements or "
-                "use SET autocommit_before_ddl = true."
-        }, sys.stdout)
-        sys.exit(0)
+        warn("WARNING: Multiple DDL statements detected. CockroachDB supports "
+             "only one DDL per transaction. Split into separate statements or "
+             "use SET autocommit_before_ddl = true.")
 
     sys.exit(0)
 
