fix(plugin): an early-exiting head no longer voids its own output under pipefail (#4042)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 56s
CI & Build / Python tests (push) Successful in 1m31s
CI & Build / Build & push image (push) Successful in 14s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 56s
CI & Build / Python tests (push) Successful in 1m31s
CI & Build / Build & push image (push) Successful in 14s
In a repo where 3,000 files define `slug`, scribe_local_dups printed nothing; with 3 files it printed the duplicate line. Every hook runs under `set -uo pipefail`, and `hits=$(git grep -l … | head -4) || hits=""` lost head's four lines whenever git grep was still writing when head exited. That is a SIGPIPE, the substitution fails, and the outer fallback wipes the result. So the by-name duplicate arm went silent for exactly the most-duplicated names. Found while fixing the same trap in the new Stop hook (#4041). - The fallback moves inside the substitution, `$(… | head -N || true)`, at all five sites: scribe_defs.sh (local dups), scribe_prior_art.sh (names, old_first, shapes) and scribe_after_write.sh (names). A real upstream failure still yields empty output. - check_plugin gains a known-bad pattern for the shape, so no hook can bring it back. - Tests: the real function against a 3,000-file repo (past the pipe buffer), and the pattern shown to flag the old shape and pass the fix. Plugin version minted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "scribe",
|
"name": "scribe",
|
||||||
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
||||||
"version": "2026.09.14.2237",
|
"version": "2026.09.14.2254",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Bryan Van Deusen"
|
"name": "Bryan Van Deusen"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -138,7 +138,8 @@ while IFS= read -r rel_path; do
|
|||||||
code=$(cat "$file_path" 2>/dev/null) || code=""
|
code=$(cat "$file_path" 2>/dev/null) || code=""
|
||||||
fi
|
fi
|
||||||
[ -n "$code" ] || continue
|
[ -n "$code" ] || continue
|
||||||
names=$(printf '%s' "$code" | scribe_defs | sort -u | head -12) || names=""
|
# `|| true` inside: an early-exiting `head` must not void its own output (#4042).
|
||||||
|
names=$(printf '%s' "$code" | scribe_defs | sort -u | head -12 || true)
|
||||||
# Nothing DEFINED in what was written (prose, data, a call-site edit) →
|
# Nothing DEFINED in what was written (prose, data, a call-site edit) →
|
||||||
# nothing to say; the arms are about shapes.
|
# nothing to say; the arms are about shapes.
|
||||||
[ -n "$names" ] || continue
|
[ -n "$names" ] || continue
|
||||||
|
|||||||
@@ -114,7 +114,12 @@ scribe_local_dups() {
|
|||||||
*) pat="(function|def|class|func|fun|fn|sub|struct|trait|interface|enum|object|protocol|type)[[:space:]]+${name}[^A-Za-z0-9_]|func[[:space:]]*\([^)]*\)[[:space:]]*${name}[[:space:]]*\(|(const|let)[[:space:]]+${name}[[:space:]]*=" ;;
|
*) pat="(function|def|class|func|fun|fn|sub|struct|trait|interface|enum|object|protocol|type)[[:space:]]+${name}[^A-Za-z0-9_]|func[[:space:]]*\([^)]*\)[[:space:]]*${name}[[:space:]]*\(|(const|let)[[:space:]]+${name}[[:space:]]*=" ;;
|
||||||
esac
|
esac
|
||||||
# -I skips binaries; :(exclude) drops the file being written.
|
# -I skips binaries; :(exclude) drops the file being written.
|
||||||
hits=$(git -C "$root" grep -I -l -E -e "$pat" -- . ":(exclude)${rel}" 2>/dev/null | head -4) || hits=""
|
# `|| true` INSIDE the substitution, not `|| hits=""` outside it (#4042):
|
||||||
|
# under the hooks' `pipefail`, `head` exiting after four lines kills a
|
||||||
|
# git grep that is still writing, the pipeline reports SIGPIPE, and an
|
||||||
|
# outer fallback then wipes the four hits head already printed. The name
|
||||||
|
# most duplicated — the one this arm exists for — was the one it dropped.
|
||||||
|
hits=$(git -C "$root" grep -I -l -E -e "$pat" -- . ":(exclude)${rel}" 2>/dev/null | head -4 || true)
|
||||||
[ -n "$hits" ] || continue
|
[ -n "$hits" ] || continue
|
||||||
count=$(printf '%s\n' "$hits" | grep -c . 2>/dev/null || echo 0)
|
count=$(printf '%s\n' "$hits" | grep -c . 2>/dev/null || echo 0)
|
||||||
label=$([ "$kind" = css ] && printf '.%s' "$name" || printf '%s' "$name")
|
label=$([ "$kind" = css ] && printf '.%s' "$name" || printf '%s' "$name")
|
||||||
|
|||||||
@@ -75,7 +75,8 @@ fi
|
|||||||
# in the repo? (scribe_local_dups in scribe_defs.sh carries the why.)
|
# in the repo? (scribe_local_dups in scribe_defs.sh carries the why.)
|
||||||
names=""
|
names=""
|
||||||
if [ -n "$code" ]; then
|
if [ -n "$code" ]; then
|
||||||
names=$(printf '%s' "$code" | scribe_defs | sort -u | head -12) || names=""
|
# `|| true` inside: an early-exiting `head` must not void its own output (#4042).
|
||||||
|
names=$(printf '%s' "$code" | scribe_defs | sort -u | head -12 || true)
|
||||||
fi
|
fi
|
||||||
|
|
||||||
local_lines=""
|
local_lines=""
|
||||||
@@ -105,11 +106,11 @@ shapes="$names"
|
|||||||
if [ -z "$shapes" ] && [ -f "$file_path" ] && command -v tac >/dev/null 2>&1; then
|
if [ -z "$shapes" ] && [ -f "$file_path" ] && command -v tac >/dev/null 2>&1; then
|
||||||
old_first=$(printf '%s' "$event" \
|
old_first=$(printf '%s' "$event" \
|
||||||
| jq -r '.tool_input.old_string // .tool_input.old_str // empty' 2>/dev/null \
|
| jq -r '.tool_input.old_string // .tool_input.old_str // empty' 2>/dev/null \
|
||||||
| grep -m1 -v '^[[:space:]]*$') || old_first=""
|
| grep -m1 -v '^[[:space:]]*$' || true)
|
||||||
if [ -n "$old_first" ]; then
|
if [ -n "$old_first" ]; then
|
||||||
ln=$(grep -nF -m1 -- "$old_first" "$file_path" 2>/dev/null | cut -d: -f1) || ln=""
|
ln=$(grep -nF -m1 -- "$old_first" "$file_path" 2>/dev/null | cut -d: -f1) || ln=""
|
||||||
if [ -n "$ln" ]; then
|
if [ -n "$ln" ]; then
|
||||||
shapes=$(head -n "$ln" "$file_path" | tac | scribe_defs | head -1) || shapes=""
|
shapes=$(head -n "$ln" "$file_path" | tac | scribe_defs | head -1 || true)
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -208,6 +208,19 @@ PATTERNS: list[tuple[re.Pattern, str, str]] = [
|
|||||||
"produces separate encoded lines joined by raw newlines — an invalid "
|
"produces separate encoded lines joined by raw newlines — an invalid "
|
||||||
"URL. Use -s (slurp) as well, e.g. `jq -sRr '@uri'`.",
|
"URL. Use -s (slurp) as well, e.g. `jq -sRr '@uri'`.",
|
||||||
),
|
),
|
||||||
|
(
|
||||||
|
# `$(producer | head -N) || var=""` — the fallback OUTSIDE the
|
||||||
|
# substitution (#4042). Every hook runs under `pipefail`; once `head`
|
||||||
|
# (or `grep -m`/`-q`) has what it needs it exits, a producer still
|
||||||
|
# writing dies of SIGPIPE, the substitution fails, and the fallback
|
||||||
|
# wipes the output that was already captured. Silent, and only on big
|
||||||
|
# inputs — which for the duplicate arm meant the most-duplicated names.
|
||||||
|
re.compile(r"\|\s*(?:head\b|grep\s+-[A-Za-z]*[mq])[^)]*\)\s*\|\|\s*\w+=\"\""),
|
||||||
|
"early-exit consumer with the fallback outside the substitution",
|
||||||
|
"Under pipefail, `$(x | head -N) || v=\"\"` discards head's output whenever x "
|
||||||
|
"is still writing when head exits (SIGPIPE). Put the fallback inside: "
|
||||||
|
"`$(x | head -N || true)`.",
|
||||||
|
),
|
||||||
(
|
(
|
||||||
re.compile(r"\|\s*cut\s+-c"),
|
re.compile(r"\|\s*cut\s+-c"),
|
||||||
"line-oriented cut for a payload cap",
|
"line-oriented cut for a payload cap",
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
"""Under `pipefail`, an early-exiting `head` must not void the output it kept (#4042).
|
||||||
|
|
||||||
|
The hooks run with `set -uo pipefail`. `hits=$(git grep -l … | head -4) ||
|
||||||
|
hits=""` lost its four hits whenever git grep was still writing when head
|
||||||
|
exited: SIGPIPE, a failed substitution, and the fallback wiped the result. It
|
||||||
|
only showed on big outputs, so the by-name duplicate arm went silent for
|
||||||
|
exactly the most-duplicated names.
|
||||||
|
|
||||||
|
Two halves: the real function on a repo big enough to overflow the pipe, and
|
||||||
|
the check_plugin pattern that keeps the shape out of every hook (shown able to
|
||||||
|
fail, rule #167).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
ROOT = Path(__file__).resolve().parents[1]
|
||||||
|
DEFS = ROOT / "plugin" / "hooks" / "scribe_defs.sh"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_duplicate_arm_still_names_a_name_defined_in_thousands_of_files(tmp_path):
|
||||||
|
for tool in ("git", "bash"):
|
||||||
|
if shutil.which(tool) is None:
|
||||||
|
pytest.skip(f"{tool!r} not installed")
|
||||||
|
env = {"PATH": os.environ["PATH"], "HOME": str(tmp_path),
|
||||||
|
"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@x",
|
||||||
|
"GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@x"}
|
||||||
|
repo = tmp_path / "repo"
|
||||||
|
repo.mkdir()
|
||||||
|
subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env)
|
||||||
|
# ~3000 × ~60 bytes of `git grep -l` output: well past a 64 KiB pipe buffer,
|
||||||
|
# so git grep is still writing when head has its four lines.
|
||||||
|
for i in range(3000):
|
||||||
|
(repo / f"module_with_a_fairly_long_descriptive_name_{i}.py").write_text("def slug(t):\n return t\n")
|
||||||
|
subprocess.run(["git", "add", "."], cwd=repo, check=True, env=env)
|
||||||
|
subprocess.run(["git", "commit", "-q", "-m", "base"], cwd=repo, check=True, env=env)
|
||||||
|
|
||||||
|
script = f'set -uo pipefail\n. "{DEFS}"\nprintf "sym\\tslug\\n" | scribe_local_dups "{repo}" new.py\n'
|
||||||
|
out = subprocess.run(["bash", "-c", script], capture_output=True, text=True, env=env, timeout=60)
|
||||||
|
assert out.returncode == 0, out.stderr
|
||||||
|
assert "`slug` is already defined in 4 other file(s)" in out.stdout
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_lint_pattern_catches_the_shape_and_passes_the_fix():
|
||||||
|
from scripts.check_plugin import PATTERNS
|
||||||
|
|
||||||
|
pattern = next(p for p, label, _why in PATTERNS if label.startswith("early-exit consumer"))
|
||||||
|
for bad in ('hits=$(git grep -l x | head -4) || hits=""',
|
||||||
|
" | grep -m1 -v '^[[:space:]]*$') || old_first=\"\"",
|
||||||
|
'x=$(a | grep -q b) || x=""'):
|
||||||
|
assert pattern.search(bad), bad
|
||||||
|
for good in ('hits=$(git grep -l x | head -4 || true)',
|
||||||
|
'ln=$(grep -nF -m1 -- "$a" "$f" | cut -d: -f1) || ln=""'):
|
||||||
|
assert not pattern.search(good), good
|
||||||
Reference in New Issue
Block a user