From 9141a41f6cd378555ff3148397b6d42c812ce14f Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Sat, 19 Sep 2026 07:13:20 -0700 Subject: [PATCH] fix(ops-log): attribution must run FIRST, not after another hook's exits The commit hook was APPENDED to .git/hooks/post-commit so as not to disturb graphify's block -- and that made attribution a subordinate clause of graphify's control flow. graphify owns EIGHT `exit 0` paths (rebase / merge / cherry-pick in progress, GRAPHIFY_SKIP_HOOK, no changed files, graphify-out-only, no python found), and every one of them returns before reaching an appended line. infra-hermes reported commits going unlogged and proposed a lock race: the ops-log fcntl path held by graphify's detached rebuild long enough for a best-effort `|| true` invocation to drop. Plausible, and wrong. The cause is ordering and it is deterministic. eaaacc1 touched only graphify-out/GRAPH_REPORT.md and hit the graphify-out-only exit; an empty probe commit hit the no-changed-files exit. Neither could ever have been recorded. Two data points that look flaky are worth reading as a control-flow question before a concurrency one. The block now goes immediately after the shebang. graphify's hook is preserved byte-for-byte below it, and core.hooksPath stays unused because it would disable graphify outright. Verified in both states with real commits, then reset out of history: a graphify-out-only commit -- the exact case that was silently dropped -- now records, and a code commit still records AND still triggers graphify's rebuild. --- scripts/git-hooks/install.sh | 51 +++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/scripts/git-hooks/install.sh b/scripts/git-hooks/install.sh index c007dac..3ec5776 100755 --- a/scripts/git-hooks/install.sh +++ b/scripts/git-hooks/install.sh @@ -1,16 +1,53 @@ #!/usr/bin/env bash -# Chain the ops-log attribution hook onto .git/hooks/post-commit. +# Chain the ops-log attribution hook into .git/hooks/post-commit. # -# APPENDS rather than replaces: graphify already owns that file, and -# core.hooksPath would disable it. Idempotent — safe to re-run. +# ⚠ PREPENDS, and that is the whole point. The first version APPENDED, so as +# not to disturb graphify's hook — and thereby made attribution a subordinate +# clause of graphify's control flow. graphify's block owns EIGHT `exit 0` +# paths (rebase / merge / cherry-pick in progress, GRAPHIFY_SKIP_HOOK, no +# changed files, graphify-out-only, no python found), and every one of them +# returned before ever reaching the appended line. +# +# Measured 2026-09-19, after infra-hermes reported commits going unlogged: +# eaaacc1 touched only graphify-out/GRAPH_REPORT.md and hit the +# graphify-out-only exit; an empty probe commit hit the no-changed-files exit. +# Neither was recorded. It looked like a flaky hook or a lock race. It was +# ordering, and it was deterministic. +# +# Attribution must not depend on another hook's opinion about whether a commit +# is interesting, so it runs FIRST and unconditionally. +# +# Still an append-style install in spirit: graphify's block is preserved +# byte-for-byte below ours, and core.hooksPath is deliberately NOT used because +# it would disable graphify outright. Idempotent — safe to re-run. set -euo pipefail REPO="$(git rev-parse --show-toplevel)" HOOK="$REPO/.git/hooks/post-commit" + +BLOCK='# ── ops-log attribution — MUST RUN FIRST ────────────────────────────────── +# Records which althing handle made this commit. PREPENDED on purpose: see +# scripts/git-hooks/install.sh for why appending silently dropped commits. +"$(git rev-parse --show-toplevel)/scripts/git-hooks/post-commit-ops-log" || true +# ──────────────────────────────────────────────────────────────────────────' + if [ ! -f "$HOOK" ]; then - printf '#!/bin/sh\n' > "$HOOK"; chmod +x "$HOOK" + printf '#!/bin/sh\n\n%s\n' "$BLOCK" > "$HOOK" + chmod +x "$HOOK" + echo "installed (new hook file)" + exit 0 fi + if grep -q "post-commit-ops-log" "$HOOK"; then - echo "already installed"; exit 0 + echo "already installed" + exit 0 fi -printf '\n# ops-log attribution hook (scripts/git-hooks/post-commit-ops-log)\n"$(git rev-parse --show-toplevel)/scripts/git-hooks/post-commit-ops-log" || true\n' >> "$HOOK" -echo "installed — commits now record their althing handle to the ops log" + +# Insert immediately after the shebang so no other block can exit before us. +tmp=$(mktemp) +head -1 "$HOOK" > "$tmp" +printf '\n%s\n\n' "$BLOCK" >> "$tmp" +tail -n +2 "$HOOK" >> "$tmp" +cat "$tmp" > "$HOOK" +rm -f "$tmp" +chmod +x "$HOOK" +echo "installed — attribution now runs FIRST in the post-commit chain"