Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 90 additions & 19 deletions .claude/hooks/check-squash-subject.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@
# is worse than no guard, because it looks present. Note the program cannot simply read stdin
# instead: stdin is where the heredoc delivers the program.
#
# THE BODY IS GUARDED TOO, for squash merges. A squash body override (--body/-b/--body-file/-F)
# must carry a `Co-authored-by:` trailer - any merge performed through this harness is
# agent-assisted by definition, and the sign-off is the convention master's squash commits carry -
# and must NOT carry a `Claude-Session:` line, which belongs on ordinary commits only
# (docs/merge-checklist.md owns both rules). A --body-file is read when it is a readable file;
# when it is not (process substitution not yet materialised, typo), the hook fails open because
# `gh` itself will then fail loudly on the missing file - a guard needs no teeth where the tool
# already bites.
#
# FAILS OPEN otherwise, deliberately: unparseable JSON, unbalanced quotes, no python3. A hook that
# blocks on its own bug jams the tool call shut, and docs/merge-checklist.md still carries the rule
# for anyone merging by hand.
Expand Down Expand Up @@ -81,28 +90,90 @@ for start, end in zip(starts, starts[1:] + [len(cmd)]):
break
i += 1

if subject is None:
continue # no override - GitHub appends the number itself
if re.search(r"\(#\d+\)\s*$", subject):
continue # ends with the number, which is the whole rule
def deny(reason):
print(json.dumps({
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": reason + " See docs/merge-checklist.md.",
}
}))

pr = next((t for t in tokens[tokens.index("merge") + 1:] if t.isdigit()), None) \
if "merge" in tokens else None
hint = f"(#{pr})" if pr else "(#<pr>)"

print(json.dumps({
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": (
f"This --subject does not end with {hint}. Overriding the subject suppresses the "
"PR number GitHub would otherwise append, so the commit lands out of step with "
"every neighbour on master - and that is not fixable afterwards without rewriting "
f"a pushed commit (astubbs#206). End the subject with ' {hint}', or drop the flag "
"and let the PR title be used. --body/--body-file do not affect the subject. "
"See docs/merge-checklist.md."
),
}
}))
break
if subject is not None and not re.search(r"\(#\d+\)\s*$", subject):
deny(
f"This --subject does not end with {hint}. Overriding the subject suppresses the "
"PR number GitHub would otherwise append, so the commit lands out of step with "
"every neighbour on master - and that is not fixable afterwards without rewriting "
f"a pushed commit (astubbs#206). End the subject with ' {hint}', or drop the flag "
"and let the PR title be used. --body/--body-file do not affect the subject."
)
break

# Squash-body sign-off: same token walk as the subject, for the body flags.
if "--squash" not in tokens and "-s" not in tokens:
continue
body = None
i = 0
while i < len(tokens):
t = tokens[i]
if t in ("--body", "-b") and i + 1 < len(tokens):
body, i = tokens[i + 1], i + 2
continue
if t in ("--body-file", "-F") and i + 1 < len(tokens):
path = tokens[i + 1]
i += 2
try:
with open(path, encoding="utf-8") as fh:
body = fh.read()
except OSError:
body = None # gh will fail loudly on it; nothing to guard
continue
for flag in ("--body", "--body-file"):
if t.startswith(flag + "="):
val = t[len(flag) + 1:]
if flag == "--body":
body = val
else:
try:
with open(val, encoding="utf-8") as fh:
body = fh.read()
except OSError:
body = None
break
else:
for flag in ("-b", "-F"):
if t.startswith(flag) and len(t) > 2:
val = t[2:]
if flag == "-b":
body = val
else:
try:
with open(val, encoding="utf-8") as fh:
body = fh.read()
except OSError:
body = None
break
i += 1

if body is None:
continue # no override, or unreadable file gh will reject itself
if not re.search(r"^co-authored-by:", body, re.I | re.M):
deny(
"This squash --body carries no Co-authored-by trailer. A merge performed through "
"this harness is agent-assisted, and master's squash commits carry exactly one "
"sign-off line: 'Co-authored-by: Claude <model> (1M context) <noreply@anthropic.com>'. "
"Add it as the body's final line."
)
break
if re.search(r"^claude-session:", body, re.I | re.M):
deny(
"This squash --body carries a Claude-Session: line. Squash-merge commits on master "
"carry only the Co-authored-by trailer; the session link belongs on ordinary "
"branch commits, not the permanent squash record. Remove the Claude-Session line."
)
break
PY
21 changes: 20 additions & 1 deletion bin/test-check-agent-hooks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,28 @@ expect DENY "-t= equals form" 'gh pr merge 2999 --squash -t
expect DENY "-tVALUE attached short form" 'gh pr merge 2999 --squash -tfoo'
expect ALLOW "-tVALUE attached, with (#N)" 'gh pr merge 2999 --squash -t"foo (#2999)"'
expect DENY "(#N) present but not at the END" 'gh pr merge 2999 --squash --subject "port (#2999) to master"'

# Squash-body sign-off guard: a squash body override must carry the Co-authored-by trailer and
# must not carry a Claude-Session line (docs/merge-checklist.md owns both). Non-squash merges and
# merges without a body override are out of scope; an unreadable --body-file fails open because
# gh itself then fails loudly.
body_ok=$(mktemp); printf '%s\n%s\n' "Real explanation." "Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>" > "$body_ok"
body_bare=$(mktemp); printf '%s\n' "Real explanation, no trailer." > "$body_bare"
body_sess=$(mktemp); printf '%s\n%s\n%s\n' "Real explanation." "Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>" "Claude-Session: https://claude.ai/code/session_x" > "$body_sess"
expect DENY "squash --body without co-authored trailer" 'gh pr merge 2999 --squash --body "no trailer here"'
expect ALLOW "squash --body with co-authored trailer" 'gh pr merge 2999 --squash --body "why.
Co-authored-by: Claude Opus 5 (1M context) <x@y>"'
expect DENY "trailer mid-line is not a trailer" 'gh pr merge 2999 --squash --body "why. Co-authored-by: Claude Opus 5 (1M context) <x@y>"'
expect DENY "squash body with Claude-Session line" "gh pr merge 2999 --squash --body-file $body_sess"
expect ALLOW "squash --body-file with trailer" "gh pr merge 2999 --squash --body-file $body_ok"
expect DENY "squash --body-file without trailer" "gh pr merge 2999 --squash --body-file $body_bare"
expect ALLOW "squash --body-file unreadable fails open" 'gh pr merge 2999 --squash --body-file /nonexistent/nope.txt'
expect ALLOW "non-squash merge body needs no trailer" 'gh pr merge 2999 --merge --body "no trailer needed"'
rm -f "$body_ok" "$body_bare" "$body_sess"
expect ALLOW "--subject ending with (#N)" 'gh pr merge 2999 --squash --subject "foo (#2999)"'
expect ALLOW "-t ending with (#N)" 'gh pr merge 2999 --squash -t "foo (#2999)"'
expect ALLOW "--subject mentioned inside --body" 'gh pr merge 2999 --squash --body "why --subject matters"'
expect ALLOW "--subject mentioned inside --body" 'gh pr merge 2999 --squash --body "why --subject matters
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>"'
expect ALLOW "escaped apostrophe, number present" 'gh pr merge 2999 --squash --subject '"'"'don'"'"'\'"'"''"'"'t drop it (#2999)'"'"''
expect ALLOW "--body-file does not touch subject" 'gh pr merge 2999 --squash --body-file b.txt'
expect ALLOW "no subject override at all" 'gh pr merge 2999 --squash'
Expand Down
Loading
Loading