mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 07:20:19 +00:00
feat: add pre-push git hook with delta lint mode (#833)
* feat: add pre-push git hook with delta lint mode Add pre-push hook and CI quality gate scripts: - .githooks/pre-push: runs quality gate before push - scripts/ci/quality_gate.sh: baseline fmt + clippy correctness + tests - scripts/ci/delta_lint.sh: clippy warnings filtered to changed lines only - Updated dev-setup.sh to install pre-push hook Supports environment-gated modes: - IRONCLAW_STRICT_LINT=1: deny all clippy warnings - IRONCLAW_STRICT_DELTA_LINT=1: deny warnings only on changed lines Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use git rev-parse for SCRIPT_DIR, add python3 check - Fix SCRIPT_DIR resolution in pre-push hook to work correctly with symlinks by using git rev-parse --show-toplevel - Add python3 availability check in delta_lint.sh Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: delta lint stderr handling, --locked flag, path normalization - Stop suppressing clippy stderr; capture it and show compilation errors if clippy produces no JSON output - Add --locked flag to clippy for lockfile consistency - Use repo root (via git rev-parse) for path normalization instead of os.getcwd() which may differ from repo root Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: dynamically detect upstream base branch in delta_lint.sh Instead of hard-coding `origin/main`, derive the base ref by checking `refs/remotes/origin/HEAD`, then falling back to `origin/main` and `origin/master`. If none can be resolved, skip delta lint gracefully with a warning and exit 0. Addresses PR #833 review feedback. Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: re-trigger CI after adding skip-regression-check label Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR #833 review feedback for delta lint - Pass remote name ($1) from pre-push hook to delta_lint.sh - Accept optional remote name arg, fall back to dynamic detection - Treat error-level diagnostics as always blocking - Check span overlap [line_start, line_end] vs changed ranges - Handle +++ /dev/null (file deletions) in parse_diff - Catch git merge-base failure with graceful skip - Add CLIPPY_STDERR to EXIT trap cleanup Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: drop -D warnings from delta lint, scope pre-push tests to --lib 1. Remove `-D warnings` from the clippy invocation in delta_lint.sh. With -D warnings, all warnings are promoted to error level in JSON output, which bypasses the delta filter entirely (errors are always blocking). The Python filter already handles the blocking decision for warnings based on changed-line overlap. 2. Scope pre-push tests to `cargo test --lib` (unit tests only) instead of the full test suite. Full integration tests can take minutes and will train developers to use --no-verify. The full suite runs in CI. Skip tests entirely with IRONCLAW_PREPUSH_TEST=0. Addresses zmanian's review feedback on PR #833. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
67b2c08a7c
commit
27e21fdabe
+13
-18
@@ -1,23 +1,18 @@
|
|||||||
#!/usr/bin/env bash
|
#!/usr/bin/env bash
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
# Pre-push hook: runs quality gate before pushing
|
||||||
|
# Skip with: git push --no-verify
|
||||||
|
|
||||||
# Pre-push hook: run clippy and tests before pushing.
|
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||||
# Install: git config core.hooksPath .githooks
|
SCRIPT_DIR="$REPO_ROOT/scripts/ci"
|
||||||
|
|
||||||
echo "pre-push: running clippy..."
|
# Default: baseline quality gate
|
||||||
if ! cargo clippy --all --benches --tests --examples --all-features -- -D warnings; then
|
"$SCRIPT_DIR/quality_gate.sh"
|
||||||
echo ""
|
|
||||||
echo "Push blocked: clippy warnings found."
|
# Optional strict delta lint (env-gated)
|
||||||
echo "To bypass: git push --no-verify"
|
if [ "${IRONCLAW_STRICT_DELTA_LINT:-0}" = "1" ]; then
|
||||||
exit 1
|
"$SCRIPT_DIR/delta_lint.sh" "$1"
|
||||||
|
elif [ "${IRONCLAW_STRICT_LINT:-0}" = "1" ]; then
|
||||||
|
echo "==> clippy (strict: all warnings)"
|
||||||
|
cargo clippy --locked --all-targets -- -D warnings
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "pre-push: running tests..."
|
|
||||||
if ! cargo test; then
|
|
||||||
echo ""
|
|
||||||
echo "Push blocked: tests failed."
|
|
||||||
echo "To bypass: git push --no-verify"
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo "pre-push: all checks passed."
|
|
||||||
|
|||||||
Executable
+216
@@ -0,0 +1,216 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
# Delta lint: only fail on clippy warnings/errors that touch changed lines.
|
||||||
|
# Compares the current branch against the merge base with the upstream default branch.
|
||||||
|
|
||||||
|
CLIPPY_OUT=""
|
||||||
|
DIFF_OUT=""
|
||||||
|
CLIPPY_STDERR=""
|
||||||
|
|
||||||
|
cleanup() {
|
||||||
|
[ -n "$CLIPPY_OUT" ] && rm -f "$CLIPPY_OUT"
|
||||||
|
[ -n "$DIFF_OUT" ] && rm -f "$DIFF_OUT"
|
||||||
|
[ -n "$CLIPPY_STDERR" ] && rm -f "$CLIPPY_STDERR"
|
||||||
|
}
|
||||||
|
trap cleanup EXIT
|
||||||
|
|
||||||
|
# Verify python3 is available (needed for diagnostic filtering)
|
||||||
|
if ! command -v python3 &>/dev/null; then
|
||||||
|
echo "ERROR: python3 is required for delta lint but not found"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Accept optional remote name argument; default to dynamic detection
|
||||||
|
REMOTE="${1:-}"
|
||||||
|
|
||||||
|
# Determine the upstream base ref dynamically
|
||||||
|
BASE_REF=""
|
||||||
|
if [ -n "$REMOTE" ]; then
|
||||||
|
# Use the provided remote name
|
||||||
|
if [ -z "$BASE_REF" ]; then
|
||||||
|
BASE_REF=$(git symbolic-ref "refs/remotes/$REMOTE/HEAD" 2>/dev/null | sed 's|refs/remotes/||' || true)
|
||||||
|
fi
|
||||||
|
if [ -z "$BASE_REF" ] && git rev-parse --verify "$REMOTE/main" &>/dev/null; then
|
||||||
|
BASE_REF="$REMOTE/main"
|
||||||
|
fi
|
||||||
|
if [ -z "$BASE_REF" ] && git rev-parse --verify "$REMOTE/master" &>/dev/null; then
|
||||||
|
BASE_REF="$REMOTE/master"
|
||||||
|
fi
|
||||||
|
else
|
||||||
|
# Try the remote HEAD symbolic ref (works for any default branch name)
|
||||||
|
if [ -z "$BASE_REF" ]; then
|
||||||
|
BASE_REF=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/||' || true)
|
||||||
|
fi
|
||||||
|
# Fall back to common default branch names
|
||||||
|
if [ -z "$BASE_REF" ] && git rev-parse --verify origin/main &>/dev/null; then
|
||||||
|
BASE_REF="origin/main"
|
||||||
|
fi
|
||||||
|
if [ -z "$BASE_REF" ] && git rev-parse --verify origin/master &>/dev/null; then
|
||||||
|
BASE_REF="origin/master"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
if [ -z "$BASE_REF" ]; then
|
||||||
|
echo "WARNING: could not determine upstream base branch, skipping delta lint"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Compute merge base
|
||||||
|
BASE=$(git merge-base "$BASE_REF" HEAD 2>/dev/null) || {
|
||||||
|
echo "WARNING: git merge-base failed for $BASE_REF, skipping delta lint"
|
||||||
|
exit 0
|
||||||
|
}
|
||||||
|
|
||||||
|
# Find changed .rs files
|
||||||
|
CHANGED_RS=$(git diff --name-only "$BASE" -- '*.rs' || true)
|
||||||
|
if [ -z "$CHANGED_RS" ]; then
|
||||||
|
echo "==> delta lint: no .rs files changed, skipping"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "==> delta lint: checking changed lines since $(echo "$BASE" | head -c 10)..."
|
||||||
|
|
||||||
|
# Extract unified-0 diff for changed line ranges
|
||||||
|
DIFF_OUT=$(mktemp "${TMPDIR:-/tmp}/ironclaw-diff.XXXXXX")
|
||||||
|
git diff --unified=0 "$BASE" -- '*.rs' > "$DIFF_OUT"
|
||||||
|
|
||||||
|
# Run clippy with JSON output (stderr shows compilation progress/errors)
|
||||||
|
CLIPPY_OUT=$(mktemp "${TMPDIR:-/tmp}/ironclaw-clippy.XXXXXX")
|
||||||
|
CLIPPY_STDERR=$(mktemp "${TMPDIR:-/tmp}/ironclaw-clippy-err.XXXXXX")
|
||||||
|
cargo clippy --locked --all-targets --message-format=json > "$CLIPPY_OUT" 2>"$CLIPPY_STDERR" || true
|
||||||
|
|
||||||
|
# Show compilation errors if clippy produced no JSON output
|
||||||
|
if [ ! -s "$CLIPPY_OUT" ] && [ -s "$CLIPPY_STDERR" ]; then
|
||||||
|
echo "ERROR: clippy failed to produce output. Compilation errors:"
|
||||||
|
cat "$CLIPPY_STDERR"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Get repo root for path normalization in Python
|
||||||
|
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||||
|
|
||||||
|
# Filter clippy diagnostics against changed line ranges
|
||||||
|
python3 - "$DIFF_OUT" "$CLIPPY_OUT" "$REPO_ROOT" <<'PYEOF'
|
||||||
|
import json
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
import os
|
||||||
|
|
||||||
|
def parse_diff(diff_path):
|
||||||
|
"""Parse unified-0 diff to extract {file: [[start, end], ...]} changed ranges."""
|
||||||
|
changed = {}
|
||||||
|
current_file = None
|
||||||
|
with open(diff_path) as f:
|
||||||
|
for line in f:
|
||||||
|
# Match +++ b/path/to/file.rs or +++ /dev/null (deletion)
|
||||||
|
if line.startswith('+++ /dev/null'):
|
||||||
|
current_file = None
|
||||||
|
continue
|
||||||
|
m = re.match(r'^\+\+\+ b/(.+)$', line)
|
||||||
|
if m:
|
||||||
|
current_file = m.group(1)
|
||||||
|
if current_file not in changed:
|
||||||
|
changed[current_file] = []
|
||||||
|
continue
|
||||||
|
# Match @@ hunk headers: @@ -old,count +new,count @@
|
||||||
|
m = re.match(r'^@@ .+ \+(\d+)(?:,(\d+))? @@', line)
|
||||||
|
if m and current_file:
|
||||||
|
start = int(m.group(1))
|
||||||
|
count = int(m.group(2)) if m.group(2) is not None else 1
|
||||||
|
if count == 0:
|
||||||
|
continue
|
||||||
|
end = start + count - 1
|
||||||
|
changed[current_file].append([start, end])
|
||||||
|
return changed
|
||||||
|
|
||||||
|
def normalize_path(path, repo_root):
|
||||||
|
"""Normalize absolute path to relative (from repo root)."""
|
||||||
|
if os.path.isabs(path):
|
||||||
|
if path.startswith(repo_root):
|
||||||
|
return os.path.relpath(path, repo_root)
|
||||||
|
return path
|
||||||
|
|
||||||
|
def in_changed_range(file_path, line_start, line_end, changed_ranges, repo_root):
|
||||||
|
"""Check if file:[line_start, line_end] overlaps any changed range."""
|
||||||
|
rel = normalize_path(file_path, repo_root)
|
||||||
|
ranges = changed_ranges.get(rel)
|
||||||
|
if not ranges:
|
||||||
|
return False
|
||||||
|
return any(start <= line_end and line_start <= end for start, end in ranges)
|
||||||
|
|
||||||
|
def main():
|
||||||
|
diff_path = sys.argv[1]
|
||||||
|
clippy_path = sys.argv[2]
|
||||||
|
repo_root = sys.argv[3]
|
||||||
|
|
||||||
|
changed_ranges = parse_diff(diff_path)
|
||||||
|
|
||||||
|
blocking = []
|
||||||
|
baseline = []
|
||||||
|
|
||||||
|
with open(clippy_path) as f:
|
||||||
|
for line in f:
|
||||||
|
line = line.strip()
|
||||||
|
if not line:
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
msg = json.loads(line)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
continue
|
||||||
|
|
||||||
|
if msg.get("reason") != "compiler-message":
|
||||||
|
continue
|
||||||
|
|
||||||
|
cm = msg.get("message", {})
|
||||||
|
level = cm.get("level", "")
|
||||||
|
if level not in ("warning", "error"):
|
||||||
|
continue
|
||||||
|
|
||||||
|
rendered = cm.get("rendered", "").strip()
|
||||||
|
|
||||||
|
# Errors are always blocking regardless of location
|
||||||
|
if level == "error":
|
||||||
|
blocking.append(rendered)
|
||||||
|
continue
|
||||||
|
|
||||||
|
# For warnings, only block if they overlap changed lines
|
||||||
|
spans = cm.get("spans", [])
|
||||||
|
primary = None
|
||||||
|
for s in spans:
|
||||||
|
if s.get("is_primary"):
|
||||||
|
primary = s
|
||||||
|
break
|
||||||
|
if not primary:
|
||||||
|
if spans:
|
||||||
|
primary = spans[0]
|
||||||
|
else:
|
||||||
|
baseline.append(rendered)
|
||||||
|
continue
|
||||||
|
|
||||||
|
file_name = primary.get("file_name", "")
|
||||||
|
line_start = primary.get("line_start", 0)
|
||||||
|
line_end = primary.get("line_end", line_start)
|
||||||
|
|
||||||
|
if in_changed_range(file_name, line_start, line_end, changed_ranges, repo_root):
|
||||||
|
blocking.append(rendered)
|
||||||
|
else:
|
||||||
|
baseline.append(rendered)
|
||||||
|
|
||||||
|
if baseline:
|
||||||
|
print(f"\n--- Baseline warnings (not in changed lines, informational) [{len(baseline)}] ---")
|
||||||
|
for w in baseline[:10]:
|
||||||
|
print(w)
|
||||||
|
if len(baseline) > 10:
|
||||||
|
print(f" ... and {len(baseline) - 10} more")
|
||||||
|
|
||||||
|
if blocking:
|
||||||
|
print(f"\n*** BLOCKING: {len(blocking)} issue(s) in changed lines ***")
|
||||||
|
for w in blocking:
|
||||||
|
print(w)
|
||||||
|
sys.exit(1)
|
||||||
|
else:
|
||||||
|
print("\n==> delta lint: passed (no issues in changed lines)")
|
||||||
|
sys.exit(0)
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
|
PYEOF
|
||||||
Executable
+13
@@ -0,0 +1,13 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
echo "==> fmt check"
|
||||||
|
cargo fmt --all -- --check
|
||||||
|
|
||||||
|
echo "==> clippy (correctness)"
|
||||||
|
cargo clippy --locked --all-targets -- -D clippy::correctness
|
||||||
|
|
||||||
|
if [ "${IRONCLAW_PREPUSH_TEST:-1}" = "1" ]; then
|
||||||
|
echo "==> tests (skip with IRONCLAW_PREPUSH_TEST=0)"
|
||||||
|
cargo test --locked --lib
|
||||||
|
fi
|
||||||
@@ -56,6 +56,9 @@ if [ -n "$HOOKS_DIR" ]; then
|
|||||||
echo " commit-msg hook installed (regression test enforcement)"
|
echo " commit-msg hook installed (regression test enforcement)"
|
||||||
ln -sf "$SCRIPTS_ABS/pre-commit-safety.sh" "$HOOKS_DIR/pre-commit"
|
ln -sf "$SCRIPTS_ABS/pre-commit-safety.sh" "$HOOKS_DIR/pre-commit"
|
||||||
echo " pre-commit hook installed (UTF-8, case-sensitivity, /tmp, redaction checks)"
|
echo " pre-commit hook installed (UTF-8, case-sensitivity, /tmp, redaction checks)"
|
||||||
|
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||||
|
ln -sf "$REPO_ROOT/.githooks/pre-push" "$HOOKS_DIR/pre-push"
|
||||||
|
echo " pre-push hook installed (quality gate + optional delta lint)"
|
||||||
else
|
else
|
||||||
echo " Skipped: not a git repository"
|
echo " Skipped: not a git repository"
|
||||||
fi
|
fi
|
||||||
|
|||||||
Reference in New Issue
Block a user