diff --git a/agentic-ai/Claude/README.md b/agentic-ai/Claude/README.md index 05cce3c..00410ea 100644 --- a/agentic-ai/Claude/README.md +++ b/agentic-ai/Claude/README.md @@ -78,13 +78,22 @@ After any source file edit, auto-detects and runs the project test suite. Detect Exits 2 on test failure (Claude sees the output) or timeout (60 s). Exits 0 silently on pass, printing timing to stderr. ### Stop: `driftcheck.sh` -At session end, validates project conventions for all git-tracked `.sh` files: +At session end, reports convention drift across all git-tracked `.sh` files: - Execute permission set - Shebang line present -Exits 2 if violations found, injecting the list back into Claude's context. +Drift is a **nudge, not a block**: findings go out as hook JSON +(`{"systemMessage": …}`) with exit 0, so Claude is free to stop. A style check +shouldn't be able to trap the agent into "fixing" a false positive — repos whose +convention legitimately differs exempt paths via glob patterns in +`~/.claude/hooks/driftcheck-ignore` (global) or `/.driftcheckignore`. -Hooks use **exit 2** to block — Claude receives the stderr message as the reason. +Exit 1 means the check itself couldn't run (`git ls-files` failed, `HOME` +unset). That path is deliberately loud: a guard that reports "all clear" without +having looked is worse than one that errors. + +The other hooks use **exit 2** to block — Claude receives the stderr message as +the reason. ## Rules (Tip 6 hierarchical structure) diff --git a/agentic-ai/Claude/TODO.md b/agentic-ai/Claude/TODO.md index 95fb6aa..4b8b2dd 100644 --- a/agentic-ai/Claude/TODO.md +++ b/agentic-ai/Claude/TODO.md @@ -11,7 +11,7 @@ After a fresh install, run each check to confirm hooks and railguard are wired c - [ ] `post-edit-shellcheck.sh` — fires after shell script edits and blocks on shellcheck errors - [ ] `post-test-runner.sh` — auto-detects test suite and runs after source edits; exits 2 on failure - **uv projects**: `.claude/test-cmd` must use `uv run --no-sync pytest`, not bare `uv run pytest`. Without `--no-sync`, every edit triggers `uv sync` which reverts out-of-band package installs (e.g. ROCm torch back to CUDA torch). Consider patching `post-test-runner.sh` to inject `--no-sync` when it detects `uv run` in the resolved test command. -- [ ] `driftcheck.sh` — flags `.sh` files missing execute permission or shebang at session end +- [ ] `driftcheck.sh` — flags `.sh` files missing execute permission or shebang at session end; nudges via `systemMessage` (exit 0), never blocks the stop ### Railguard diff --git a/agentic-ai/Claude/hooks/driftcheck.sh b/agentic-ai/Claude/hooks/driftcheck.sh index 25c99a6..868d874 100755 --- a/agentic-ai/Claude/hooks/driftcheck.sh +++ b/agentic-ai/Claude/hooks/driftcheck.sh @@ -1,6 +1,8 @@ #!/usr/bin/env bash -# Stop hook: validates project conventions before Claude finishes a session. -# Exit 2 = block stop (stderr injected back into Claude; must fix and try again). +# Stop hook: reports shell-script convention drift when Claude finishes a session. +# Findings are a nudge, never a block: they go out as hook JSON (systemMessage) +# with exit 0, so the agent is free to stop. Exit 1 = the check itself could not +# run, surfaced loudly rather than passing silently. # # Checks git-tracked .sh files for consistency: # - has shebang but not executable → flag (meant to run but can't) @@ -11,13 +13,17 @@ # whose documented convention conflicts, e.g. scripts that are intentionally # non-executable because a Dockerfile chmods its copies). The hook runs from # the repo root regardless of where the session started, so patterns always -# match against repo-root-relative paths. +# match against repo-root-relative paths. validate.sh regression-tests this +# parsing; a revert dropped it silently once already. set -euo pipefail -trap 'exit 2' ERR + +die() { printf 'driftcheck.sh: %s\n' "$1" >&2; exit 1; } git rev-parse --git-dir &>/dev/null || exit 0 cd "$(git rev-parse --show-toplevel)" +[[ -n "${HOME:-}" ]] || die 'HOME is unset, cannot read the global ignore list' + ignore_patterns=() for ignore_file in "$HOME/.claude/hooks/driftcheck-ignore" .driftcheckignore; do [[ -f "$ignore_file" ]] || continue @@ -26,6 +32,10 @@ for ignore_file in "$HOME/.claude/hooks/driftcheck-ignore" .driftcheckignore; do done < "$ignore_file" done +# A process substitution hides git's exit status from set -e, so a failed +# listing would read as "nothing to check". Capture the list first. +tracked=$(git ls-files '*.sh') || die 'git ls-files failed, nothing was checked' + issues=() while IFS= read -r f; do @@ -50,11 +60,13 @@ while IFS= read -r f; do elif $is_exec && ! $has_shebang; then issues+=("is executable but missing shebang: $f") fi -done < <(git ls-files '*.sh') - -if [[ ${#issues[@]} -gt 0 ]]; then - printf 'driftcheck.sh: convention violations found:\n' >&2 - printf ' - %s\n' "${issues[@]}" >&2 - printf 'Fix these before finishing.\n' >&2 - exit 2 -fi +done <<< "$tracked" + +[[ ${#issues[@]} -gt 0 ]] || exit 0 + +printf 'driftcheck.sh: convention drift (not blocking):\n' >&2 +printf ' - %s\n' "${issues[@]}" >&2 + +message=$(printf 'driftcheck: %s\n' "${issues[@]}") +message_json=$(jq -Rs . <<< "$message") +printf '{"systemMessage":%s}\n' "$message_json" diff --git a/agentic-ai/Claude/validate.sh b/agentic-ai/Claude/validate.sh index e136133..87eb9d3 100755 --- a/agentic-ai/Claude/validate.sh +++ b/agentic-ai/Claude/validate.sh @@ -202,11 +202,55 @@ else fi rm -f "$_TMPSH"; trap - EXIT -# driftcheck.sh: must pass on the current repo state -if (cd "$REPO_DIR" && bash "$REPO_DIR/hooks/driftcheck.sh" &>/dev/null); then - pass "driftcheck.sh: no convention violations in repo" +# driftcheck.sh: drift is a nudge (JSON on stdout, exit 0); non-zero means the +# check could not run at all, which is the real failure. +DRIFT_OUT=$(cd "$REPO_DIR" && bash "$REPO_DIR/hooks/driftcheck.sh" 2>/dev/null) +DRIFT_RC=$? +if [[ $DRIFT_RC -ne 0 ]]; then + fail "driftcheck.sh: could not run (exit $DRIFT_RC) — run hooks/driftcheck.sh directly for details" +elif [[ -n "$DRIFT_OUT" ]]; then + warn "driftcheck.sh: convention drift reported — run hooks/driftcheck.sh directly for details" else - fail "driftcheck.sh: convention violations found — run hooks/driftcheck.sh directly for details" + pass "driftcheck.sh: no convention drift in repo" +fi + +# The next three are regression tests for silent-failure modes: each one, when +# broken, makes driftcheck.sh report "all clear" without having checked anything. +_DRIFT_REPO=$(mktemp -d) +trap 'rm -rf "$_DRIFT_REPO"' EXIT +( + cd "$_DRIFT_REPO" || exit 1 + git init --quiet . + printf '#!/usr/bin/env bash\ntrue\n' > drift.sh + git add drift.sh + git -c user.email=validate@local -c user.name=validate commit --quiet -m init +) &>/dev/null + +if [[ "$(cd "$_DRIFT_REPO" && bash "$REPO_DIR/hooks/driftcheck.sh" 2>/dev/null)" == *drift.sh* ]]; then + pass "driftcheck.sh: flags a shebang script missing its execute bit" +else + fail "driftcheck.sh: failed to flag a shebang script missing its execute bit" +fi + +printf 'drift.sh\n' > "$_DRIFT_REPO/.driftcheckignore" +if [[ -z "$(cd "$_DRIFT_REPO" && bash "$REPO_DIR/hooks/driftcheck.sh" 2>/dev/null)" ]]; then + pass "driftcheck.sh: honors .driftcheckignore patterns" +else + fail "driftcheck.sh: .driftcheckignore patterns were not honored" +fi + +printf 'CORRUPT' > "$_DRIFT_REPO/.git/index" +if (cd "$_DRIFT_REPO" && bash "$REPO_DIR/hooks/driftcheck.sh") &>/dev/null; then + fail "driftcheck.sh: exits 0 when git ls-files fails (silent false negative)" +else + pass "driftcheck.sh: fails loudly when git ls-files fails" +fi +rm -rf "$_DRIFT_REPO"; trap - EXIT + +if [[ "$( (cd "$REPO_DIR" && env -u HOME bash "$REPO_DIR/hooks/driftcheck.sh") 2>&1 )" == *'HOME is unset'* ]]; then + pass "driftcheck.sh: unset HOME fails deliberately" +else + fail "driftcheck.sh: unset HOME does not fail deliberately (set -u crash?)" fi # ── Summary ─────────────────────────────────────────────────────────────────── diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ce5a33c..dab43bd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -6,6 +6,37 @@ Format loosely follows [Keep a Changelog](https://keepachangelog.com). Remaining work lives in [TODO.md](TODO.md); the design rationale for the unified layout is in [UNIFICATION.md](UNIFICATION.md). +## Unreleased — driftcheck as a nudge + +### Changed +- `driftcheck.sh` no longer blocks the Stop hook ([#69](https://github.com/ulises-c/Computer-Setup/issues/69)). + Convention drift is now reported as hook JSON (`{"systemMessage": …}`) with + exit 0 instead of exit 2. Blocking a stop over a style check pressured the + agent into the mechanical fix (`chmod +x`), which for a repo whose paths had + drifted outside its `.driftcheckignore` globs was exactly the wrong action. + Exit 1 is now reserved for "the check could not run". + +### Fixed +- Two silent-failure modes in `driftcheck.sh`, the follow-ups left out of scope + in [#65](https://github.com/ulises-c/Computer-Setup/pull/65) and tracked in + [#67](https://github.com/ulises-c/Computer-Setup/issues/67): + - A failing `git ls-files` was invisible. Feeding the loop from a process + substitution hid git's exit status from `set -e` and the `ERR` trap, so a + corrupt index produced zero lines and a clean "no violations" pass — a guard + reporting all-clear having checked nothing. The listing is now captured + before the loop and a failure exits 1 with a message. + - An unset `HOME` tripped `set -u` at expansion time, terminating before the + `ERR` trap with a bare `HOME: unbound variable`. It is now an explicit, + diagnosed exit 1. + +### Notes +`validate.sh` grew four `driftcheck.sh` regression tests, all verified to fail +against the pre-fix hook: it flags a shebang script missing its execute bit, +honors `.driftcheckignore` (parsing that a revert dropped silently once +already), fails loudly on a corrupt git index, and fails deliberately with +`HOME` unset. The two silent-failure bugs are exactly the class that hides from +a "run it and see" check, so each has a test that reproduces the original. + ## Unreleased — shared global AGENTS.md ### Added