diff --git a/home/dot_claude/hooks/executable_block-rot-comments.py b/home/dot_claude/hooks/executable_block-rot-comments.py index c59a9c4..3b7cdf8 100644 --- a/home/dot_claude/hooks/executable_block-rot-comments.py +++ b/home/dot_claude/hooks/executable_block-rot-comments.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# PreToolUse(Write|Edit|MultiEdit) hook: block code comments that contain +# PreToolUse(Write|Edit|MultiEdit|Bash) hook: block code comments that contain # patterns known to rot quickly. The existing rules in # ~/.claude/rules/development-principles.md already forbid these, but # instructions alone have not held — this turns the policy into a @@ -14,6 +14,18 @@ # "as of 2024"). Code evolves; this dates itself. # 4. Bare TODO / FIXME without an owner (@name) or date (YYYY-MM-DD). # Unactionable reminders accumulate forever. +# 5. Keep-in-sync instructions and provenance notes (Japanese patterns +# only). The first should be enforced by a test or check; the second +# belongs in the commit message. +# Japanese equivalents of 2 and 3 are matched as well. +# +# Where it runs: +# - Write / Edit / MultiEdit: the content being written. +# - Bash: when the command runs `git commit`, the diff the commit will +# record (staged changes; HEAD vs worktree with -a / --all). This also +# covers code written through Bash (heredocs, sed, one-off scripts). The +# repo dir is the payload cwd, a leading `cd &&`, or `git -C +# `. Any git failure or timeout allows the commit. # # Skipped: # - Prose extensions (.md, .mdx, .txt, .rst, .adoc) — issue refs and @@ -26,7 +38,10 @@ # when blocking, otherwise exits silently. import json +import os import re +import shlex +import subprocess import sys PROSE_EXTENSIONS = (".md", ".mdx", ".txt", ".rst", ".adoc") @@ -60,6 +75,26 @@ ), "temporal phrasing", ), + ( + re.compile(r"以前は|従来は|旧仕様|今回の(?:変更|修正|対応)"), + "temporal phrasing", + ), + ( + re.compile(r"[A-Za-z_][\w.]*(?:\(\))?\s*から呼ばれ"), + "caller / usage reference", + ), + ( + re.compile( + r"(?:と|に)(?:一致|同期)させ|にも(?:足す|追加する|反映する)|" + r"も合わせて(?:変更|更新)|(?:1|一)\s*つずつ足す|" + r"(?:[A-Z][A-Z0-9_]{2,}|行)\s*に足す" + ), + "keep-in-sync instruction — enforce with a test or check instead", + ), + ( + re.compile(r"で取得した(?:実)?値"), + "provenance — belongs in the commit message", + ), ( re.compile(r"\b(TEMP(?:ORARY)?|HACK|XXX)\s*:", re.I), "TEMP/HACK/XXX marker", @@ -129,6 +164,105 @@ def find_violations(lines: list[str]) -> list[tuple[str, str]]: return hits +GIT_COMMIT_COMMAND = re.compile( + r"(?:^|[\s;&|(])git(?:\s+-C\s+(?P\S+))?\s+commit(?![\w-])" +) +LEADING_CD = re.compile(r"^\s*cd\s+(?P\S+)\s*&&") +COMMIT_FLAGS_WITH_VALUE = "mFCcStu" +GIT_TIMEOUT_SECONDS = 5 + + +def commit_covers_all_tracked(args: str) -> bool: + try: + lexer = shlex.shlex(args, posix=True, punctuation_chars=True) + lexer.whitespace_split = True + tokens = list(lexer) + except ValueError: + return False + skip_next = False + for token in tokens: + if token in ("&&", "||", ";", "|", "&"): + break + if skip_next: + skip_next = False + elif token == "--all": + return True + elif token.startswith("-") and not token.startswith("--"): + for position, flag in enumerate(token[1:], 1): + if flag == "a": + return True + if flag in COMMIT_FLAGS_WITH_VALUE: + skip_next = position == len(token) - 1 + break + elif token.startswith("--") and "=" not in token: + skip_next = token in ("--message", "--file", "--author", "--date") + return False + + +def commit_target(command: str, cwd: str) -> tuple[str, bool] | None: + match = GIT_COMMIT_COMMAND.search(command) + if not match: + return None + repo_dir = cwd or os.getcwd() + cd = LEADING_CD.match(command) + if cd: + repo_dir = os.path.join(repo_dir, os.path.expanduser(cd.group("cd_dir").strip("\"'"))) + if match.group("git_dir"): + repo_dir = os.path.join(repo_dir, os.path.expanduser(match.group("git_dir").strip("\"'"))) + return repo_dir, commit_covers_all_tracked(command[match.end():]) + + +def commit_diff(repo_dir: str, covers_all_tracked: bool) -> str | None: + base = "HEAD" if covers_all_tracked else "--cached" + try: + result = subprocess.run( + [ + "git", "-C", repo_dir, "-c", "core.quotepath=off", + "diff", base, "-U0", "--no-color", "--no-ext-diff", + ], + capture_output=True, + timeout=GIT_TIMEOUT_SECONDS, + env={**os.environ, "GIT_OPTIONAL_LOCKS": "0"}, + ) + except (OSError, subprocess.SubprocessError): + return None + if result.returncode != 0: + return None + return result.stdout.decode("utf-8", errors="replace") + + +def added_lines_by_file(diff: str) -> dict[str, list[str]]: + by_file: dict[str, list[str]] = {} + current: str | None = None + in_hunk = False + for line in diff.splitlines(): + if line.startswith("diff --git "): + current, in_hunk = None, False + elif line.startswith("@@"): + in_hunk = True + elif not in_hunk: + if line.startswith("+++ b/"): + current = line[len("+++ b/"):] + elif line.startswith("+") and current is not None: + by_file.setdefault(current, []).append(line[1:]) + return by_file + + +def commit_violations(command: str, cwd: str) -> list[tuple[str, str]]: + target = commit_target(command, cwd) + if target is None: + return [] + diff = commit_diff(*target) + if diff is None: + return [] + hits: list[tuple[str, str]] = [] + for path, lines in added_lines_by_file(diff).items(): + if path.lower().endswith(PROSE_EXTENSIONS): + continue + hits.extend((f"{path}: {line}", label) for line, label in find_violations(lines)) + return hits + + def main() -> int: try: payload = json.load(sys.stdin) @@ -139,12 +273,21 @@ def main() -> int: tool_input = payload.get("tool_input") or {} file_path = tool_input.get("file_path") or "" - if file_path.lower().endswith(PROSE_EXTENSIONS): + if tool_name == "Bash": + violations = commit_violations( + tool_input.get("command") or "", payload.get("cwd") or "" + ) + commit_note = ( + "These lines are already in the file: remove the comment from the\n" + "file, then re-stage it (git add) before committing again.\n\n" + ) + elif tool_name in ("Write", "Edit", "MultiEdit"): + if file_path.lower().endswith(PROSE_EXTENSIONS): + return 0 + violations = find_violations(added_lines(tool_name, tool_input)) + commit_note = "" + else: return 0 - if tool_name not in ("Write", "Edit", "MultiEdit"): - return 0 - - violations = find_violations(added_lines(tool_name, tool_input)) if not violations: return 0 @@ -156,6 +299,7 @@ def main() -> int: reason = ( "Comment(s) that tend to rot blocked.\n\n" f"{sample}{overflow}\n\n" + f"{commit_note}" "Why these rot:\n" " - Issue / PR / ticket numbers belong in the commit message or PR body,\n" " not in code that will outlive them.\n" @@ -163,7 +307,11 @@ def main() -> int: " - Temporal phrasing (\"previously\", \"now does\", \"as of v2\") goes stale\n" " the next time the code is rewritten.\n" " - Bare TODO / FIXME without an owner or target date is an unactionable\n" - " reminder that accumulates forever.\n\n" + " reminder that accumulates forever.\n" + " - Keep-in-sync instructions silently drift; a test or check that fails\n" + " when the copies diverge does not.\n" + " - Provenance (where a value was fetched from) is history, which the\n" + " commit message records.\n\n" "How to fix:\n" " - Delete the comment if the code is self-explanatory.\n" " - Rewrite it to describe the WHY (a constraint, invariant, surprise).\n" diff --git a/home/dot_claude/rules/development-principles.md b/home/dot_claude/rules/development-principles.md index 051c0bc..091d039 100644 --- a/home/dot_claude/rules/development-principles.md +++ b/home/dot_claude/rules/development-principles.md @@ -43,10 +43,12 @@ Default: **no comments**. Only add a comment when ALL of the following are true: 2. Removing the comment would leave a future reader confused 3. The information cannot be conveyed by better naming alone -Explicitly forbidden (the rot-comment hook blocks task/PR refs, caller refs, and temporal phrasing mechanically): +A comment that passes is one line by default, in any language. Explicitly forbidden (the rot-comment hook catches a subset at write time and again at `git commit`; the rest is on you): - Repeating what the code does, restating the signature, or section banners - Commented-out code (delete it; git remembers) +- Keep-in-sync instructions (「X と Y の両方に足す」): enforce the sync with a test or check instead +- Provenance (「〜で取得した値」) and decision history: the commit message, PR body, or ADR — not a summary above the code - Facts whose source of truth lives elsewhere — counts (tests, cases, callers, fields), lists of callers, versions, dates, ticket numbers, "what changed". They go stale the moment someone edits a different file. Living docs (README, CLAUDE.md, rules, skills) follow the same rule; point-in-time records (PR bodies, commit messages, worklogs) may carry them ## Command Legibility diff --git a/home/dot_claude/settings.json.tmpl b/home/dot_claude/settings.json.tmpl index 5cb0553..c6cf9f1 100644 --- a/home/dot_claude/settings.json.tmpl +++ b/home/dot_claude/settings.json.tmpl @@ -183,6 +183,10 @@ { "type": "command", "command": "$HOME/.claude/hooks/block-unsigned-commits.sh" + }, + { + "type": "command", + "command": "python3 $HOME/.claude/hooks/block-rot-comments.py" } ] }, diff --git a/tests/hooks/block-rot-comments.test.sh b/tests/hooks/block-rot-comments.test.sh index e74fe71..636de71 100644 --- a/tests/hooks/block-rot-comments.test.sh +++ b/tests/hooks/block-rot-comments.test.sh @@ -1,8 +1,7 @@ #!/usr/bin/env bash -# Tests for executable_block-rot-comments.py: PreToolUse(Write|Edit) guard -# that blocks code comments containing patterns known to rot — references -# to current task/PR/issue numbers, caller names, temporal phrasing -# ("previously", "now does"), and TODO/FIXME without owner or date. +# Tests for executable_block-rot-comments.py: PreToolUse(Write|Edit|Bash) +# guard that blocks code comments containing patterns known to rot, in +# written files and in the diff a Bash `git commit` will record. set -uo pipefail hook="$HOOKS_DIR/executable_block-rot-comments.py" @@ -134,4 +133,124 @@ _temp_payload=$(printf '{"tool_name":"Write","tool_input":{"file_path":"a.ts","c out=$(run "$_temp_payload") is_block "$out" || fail "did not block block-comment rot marker: $out" +# === Japanese: legit why-comment → ALLOW === +out=$(run '{"tool_name":"Write","tool_input":{"file_path":"a.py","content":"# 上流 API が無言で接続を切るため、再試行が必要\ndef f(): pass\n"}}') +[[ -z $out ]] || fail "blocked legit Japanese why-comment: $out" + +# === Japanese: each rot label → BLOCK === +jp_block() { # jp_block + local payload + payload=$(jq -cn --arg c "$1" '{tool_name:"Write",tool_input:{file_path:"a.py",content:("# " + $c + "\ndef f(): pass\n")}}') + out=$(run "$payload") + is_block "$out" || fail "did not block Japanese comment '$1': $out" + reason_has "$out" "$2" || fail "Japanese comment '$1' missing label '$2': $out" +} +jp_block '以前は slug で判定していた' 'temporal phrasing' +jp_block '従来は null を返す' 'temporal phrasing' +jp_block '旧仕様との互換のため残す' 'temporal phrasing' +jp_block '今回の変更で追加した' 'temporal phrasing' +jp_block 'handleSubmit から呼ばれる' 'caller / usage reference' +jp_block 'validate_owners.py と一致させる' 'keep-in-sync instruction' +jp_block 'CODEOWNERS と同期させること' 'keep-in-sync instruction' +jp_block 'こちらにも追加する' 'keep-in-sync instruction' +jp_block 'ここも合わせて更新する' 'keep-in-sync instruction' +jp_block 'ここと CODEOWNERS の行に 1 つずつ足す' 'keep-in-sync instruction' +jp_block 'この行と validate_owners.py の MIGRATION_OPS_TEAMS に足す' 'keep-in-sync instruction' +jp_block 'team_id は gh api orgs/C-FO/teams/ で取得した実値' 'provenance' + +# === Japanese: why-comments that merely share vocabulary → ALLOW === +jp_allow() { # jp_allow + local payload + payload=$(jq -cn --arg c "$1" '{tool_name:"Write",tool_input:{file_path:"a.py",content:("# " + $c + "\ndef f(): pass\n")}}') + out=$(run "$payload") + [[ -z $out ]] || fail "blocked legit Japanese comment '$1': $out" +} +jp_allow 'このキャッシュは複数スレッドから呼び出されるため、ロックが必要' +jp_allow '空配列で使われると panic するため先に検査する' +jp_allow '呼び出し元の責任でロックを取得すること' +jp_allow '文字列を新たに追加した場合は encode が必要' +jp_allow '値を修正したあと再検証が必要' +jp_allow '上流がバグっているため、この値を追加した後に再取得する' +jp_allow '値を 0 に変更した場合はキャッシュを捨てる' +jp_allow 'API で取得したトークンは 1 時間で失効する' + +# === Commit gate: git repo fixtures === +tmp_root=$(mktemp -d) +trap 'rm -rf "$tmp_root"' EXIT + +new_repo() { # new_repo — create an initial-commit repo, print its path + local dir="$tmp_root/$1" + mkdir -p "$dir" + git -C "$dir" init -q + git -C "$dir" config user.email t@example.com + git -C "$dir" config user.name Test + git -C "$dir" config commit.gpgsign false + printf 'def base(): pass\n' >"$dir/base.py" + git -C "$dir" add base.py + git -C "$dir" commit -q -m init + printf '%s' "$dir" +} +bash_payload() { # bash_payload + jq -cn --arg cwd "$1" --arg cmd "$2" '{tool_name:"Bash",cwd:$cwd,tool_input:{command:$cmd}}' +} +rot_jp='# 以前は slug で判定していた' + +# Staged rot comment → BLOCK, reason names the file +repo=$(new_repo staged_rot) +printf '%s\ndef f(): pass\n' "$rot_jp" >"$repo/rotten.py" +git -C "$repo" add rotten.py +out=$(run "$(bash_payload "$repo" 'git commit -m x')") +is_block "$out" || fail "commit gate did not block staged rot comment: $out" +reason_has "$out" "rotten.py" || fail "commit gate reason missing file name: $out" +reason_has "$out" "re-stage" || fail "commit gate reason missing re-stage hint: $out" + +# Staged clean code → ALLOW +repo=$(new_repo staged_clean) +printf '# 上流 API が無言で接続を切るため、再試行が必要\ndef f(): pass\n' >"$repo/clean.py" +git -C "$repo" add clean.py +out=$(run "$(bash_payload "$repo" 'git commit -m x')") +[[ -z $out ]] || fail "commit gate blocked clean staged code: $out" + +# -am picks up an unstaged modification to a tracked file → BLOCK +repo=$(new_repo all_flag) +printf 'def base(): pass\n%s\n' "$rot_jp" >"$repo/base.py" +out=$(run "$(bash_payload "$repo" 'git commit -am x')") +is_block "$out" || fail "commit gate did not block -am unstaged rot comment: $out" +reason_has "$out" "base.py" || fail "-am reason missing file name: $out" +# Same change without -a is not part of the commit → ALLOW +out=$(run "$(bash_payload "$repo" 'git commit -m x')") +[[ -z $out ]] || fail "commit gate scanned unstaged change without -a: $out" + +# Staged prose file with temporal wording → ALLOW +repo=$(new_repo staged_md) +printf '以前は slug で判定していた\n' >"$repo/notes.md" +git -C "$repo" add notes.md +out=$(run "$(bash_payload "$repo" 'git commit -m x')") +[[ -z $out ]] || fail "commit gate blocked a .md file: $out" + +# Non-commit Bash commands → ALLOW +repo=$(new_repo non_commit) +printf '%s\ndef f(): pass\n' "$rot_jp" >"$repo/rotten.py" +git -C "$repo" add rotten.py +out=$(run "$(bash_payload "$repo" 'git status')") +[[ -z $out ]] || fail "gate fired on git status: $out" +out=$(run "$(bash_payload "$repo" 'git commit-tree HEAD^{tree}')") +[[ -z $out ]] || fail "gate fired on git commit-tree: $out" + +# cd && git commit with payload cwd elsewhere → BLOCK +repo=$(new_repo cd_prefix) +printf '%s\ndef f(): pass\n' "$rot_jp" >"$repo/rotten.py" +git -C "$repo" add rotten.py +out=$(run "$(bash_payload "$tmp_root" "cd $repo && git commit -m x")") +is_block "$out" || fail "commit gate ignored leading cd: $out" + +# git -C commit → BLOCK +out=$(run "$(bash_payload "$tmp_root" "git -C $repo commit -m x")") +is_block "$out" || fail "commit gate ignored git -C: $out" + +# Non-repo directory → fail open +mkdir -p "$tmp_root/not_a_repo" +out=$(run "$(bash_payload "$tmp_root/not_a_repo" 'git commit -m x')") +[[ -z $out ]] || fail "commit gate did not fail open outside a repo: $out" + echo "all assertions passed"