|
| 1 | +#!/usr/bin/env python3 |
| 2 | +"""Notice friction with the cmem-plugin-template at the end of a session. |
| 3 | +
|
| 4 | +This is a Claude Code ``Stop`` hook. ``.claude/settings.json`` runs it |
| 5 | +directly, and not through a task, because a task runner writes to stdout and |
| 6 | +picks its own exit codes - and stdout is this hook's JSON channel. It reads the |
| 7 | +hook payload from stdin, so a hand run needs one:: |
| 8 | +
|
| 9 | + echo '{}' | python3 .claude/hooks/template-feedback.py |
| 10 | +
|
| 11 | +It belongs to the template - see ``.claude/rules/copier-template.md``. |
| 12 | +
|
| 13 | +It says nothing at all unless the session left evidence that a template owned |
| 14 | +file got in the way, and then it asks the agent to consider whether that is |
| 15 | +worth reporting upstream. Deciding there is nothing to report is a valid |
| 16 | +answer; the hook fires at most once per session either way. |
| 17 | +
|
| 18 | +Three rules govern everything below. Print **nothing** unless there is |
| 19 | +something to say, because stdout is the hook's JSON channel. Always exit 0, |
| 20 | +whatever goes wrong - a broken hook must never keep a session from ending. And |
| 21 | +when a fact cannot be established, drop the evidence rather than guessing: a |
| 22 | +false positive blocks a session, a false negative only misses one report. |
| 23 | +
|
| 24 | +This file is not part of the package and is not covered by ``task check``, so |
| 25 | +keep it small, dependency free and readable. |
| 26 | +""" |
| 27 | + |
| 28 | +import json |
| 29 | +import re |
| 30 | +import subprocess |
| 31 | +import sys |
| 32 | +import tempfile |
| 33 | +import tomllib |
| 34 | +from pathlib import Path |
| 35 | + |
| 36 | +OPT_OUT = Path(".claude") / "no-template-feedback" |
| 37 | + |
| 38 | +# Paths the template owns. The shipped rules tell the agent not to edit these, |
| 39 | +# so a modification here is the strongest available signal that something in |
| 40 | +# the template did not fit. |
| 41 | +TEMPLATE_OWNED = ( |
| 42 | + ".claude/", |
| 43 | + ".github/workflows/", |
| 44 | + ".gitlab-ci.yml", |
| 45 | + ".pre-commit-config.yaml", |
| 46 | + "Taskfile.yaml", |
| 47 | +) |
| 48 | + |
| 49 | +# The one template owned file that *documents* what a conflict looks like, in |
| 50 | +# a fenced example. Every copier update that rewrites it adds those markers to |
| 51 | +# the diff, which would block the session during the very workflow this hook |
| 52 | +# exists to support. |
| 53 | +CONFLICT_PROSE = ".claude/skills/copier-update/SKILL.md" |
| 54 | + |
| 55 | +# Suffixes the suppression comments below can appear in at all. Without this, |
| 56 | +# a CHANGELOG entry or a rules file merely naming `# noqa` counts as one. |
| 57 | +PYTHON_SUFFIXES = (".py", ".pyi") |
| 58 | + |
| 59 | +# A comment silencing a check. The rules forbid these outright, so a new one |
| 60 | +# means a rule lost an argument with real code. Counted per file against the |
| 61 | +# removed side, so editing a line that already carried one is not a finding. |
| 62 | +SILENCER = re.compile(r"#\s*(noqa|type:\s*ignore)") |
| 63 | + |
| 64 | +# What copier leaves behind when an update could not be merged. Only an added |
| 65 | +# line counts; a marker already committed is this project's problem, not the |
| 66 | +# template's. |
| 67 | +CONFLICT = re.compile(r"^\+<{7} ") |
| 68 | + |
| 69 | +# A changed `_commit` in the copier answers file. Its presence means the |
| 70 | +# working tree *is* a copier update, so the template owned files it rewrote |
| 71 | +# were not edited by anyone and are not evidence of anything. |
| 72 | +COPIER_UPDATE = re.compile(r"^\+_commit:") |
| 73 | + |
| 74 | +# Colour codes git writes when the user configured `color.diff = always`, which |
| 75 | +# it does even when the output is a pipe. |
| 76 | +ANSI = re.compile(r"\x1b\[[0-9;]*m") |
| 77 | + |
| 78 | +# Enough of a file to find a suppression in, without reading a data fixture |
| 79 | +# somebody left untracked. |
| 80 | +READ_LIMIT = 512 * 1024 |
| 81 | + |
| 82 | +# The shortest useful `git status --porcelain` record: "XY " and one character |
| 83 | +# of path. |
| 84 | +MIN_STATUS_FIELD = 4 |
| 85 | + |
| 86 | +TIMEOUT = 10 |
| 87 | + |
| 88 | + |
| 89 | +def git(*args: str) -> str | None: |
| 90 | + """Run a git command and return its plain output, or None if it failed. |
| 91 | +
|
| 92 | + "Failed" and "said nothing" must stay apart. `git diff HEAD` exits non-zero |
| 93 | + on an unborn HEAD, and a caller reading that as an empty diff would go half |
| 94 | + blind - seeing every untracked file through `git status` while believing |
| 95 | + nothing had changed. The same applies when git is missing from PATH or |
| 96 | + another process holds `index.lock`. |
| 97 | +
|
| 98 | + The configuration passed here pins the output format the parsers assume: |
| 99 | + colour off (`color.diff` outranks `color.ui` and paints even into a pipe), |
| 100 | + the `a/` and `b/` diff prefixes on, and paths unquoted. |
| 101 | + """ |
| 102 | + command = [ |
| 103 | + "git", |
| 104 | + "--no-pager", |
| 105 | + "-c", |
| 106 | + "color.ui=false", |
| 107 | + "-c", |
| 108 | + "core.quotePath=false", |
| 109 | + "-c", |
| 110 | + "diff.noprefix=false", |
| 111 | + "-c", |
| 112 | + "diff.mnemonicPrefix=false", |
| 113 | + *args, |
| 114 | + ] |
| 115 | + try: |
| 116 | + result = subprocess.run( |
| 117 | + command, |
| 118 | + capture_output=True, |
| 119 | + text=True, |
| 120 | + timeout=TIMEOUT, |
| 121 | + check=False, |
| 122 | + ) |
| 123 | + except (OSError, subprocess.SubprocessError): |
| 124 | + return None |
| 125 | + if result.returncode != 0: |
| 126 | + return None |
| 127 | + return ANSI.sub("", result.stdout) |
| 128 | + |
| 129 | + |
| 130 | +def entries(status: str) -> list[tuple[str, str]]: |
| 131 | + """Return the (status, path) pairs of a `git status --porcelain -uall -z`. |
| 132 | +
|
| 133 | + `-z` is what makes this safe to parse: paths arrive verbatim, so neither |
| 134 | + `core.quotePath` nor a space in a name can defeat a prefix or suffix test. |
| 135 | + A rename is emitted as the new path followed by the original one, and only |
| 136 | + the new path is of any interest here. |
| 137 | + """ |
| 138 | + fields = [field for field in status.split("\0") if field] |
| 139 | + pairs = [] |
| 140 | + index = 0 |
| 141 | + while index < len(fields): |
| 142 | + field = fields[index] |
| 143 | + index += 1 |
| 144 | + if len(field) < MIN_STATUS_FIELD: |
| 145 | + continue |
| 146 | + code, path = field[:2], field[3:] |
| 147 | + pairs.append((code, path)) |
| 148 | + if code[0] in "RC" or code[1] in "RC": |
| 149 | + index += 1 # the original path of a rename or copy |
| 150 | + return pairs |
| 151 | + |
| 152 | + |
| 153 | +def attributed(diff: str) -> list[tuple[str | None, str]]: |
| 154 | + """Pair every content line of a diff with the file it belongs to. |
| 155 | +
|
| 156 | + A flat scan cannot tell a project's own code from a template owned file |
| 157 | + that merely *writes about* the thing being looked for - the rules file |
| 158 | + discusses `# noqa`, and matching that prose reports the template to itself. |
| 159 | +
|
| 160 | + A line whose file could not be established is paired with None, and every |
| 161 | + caller drops those. That happens for a deletion, whose `+++` side is |
| 162 | + `/dev/null`, and it is the safe direction: unattributed evidence is no |
| 163 | + evidence. |
| 164 | + """ |
| 165 | + pairs: list[tuple[str | None, str]] = [] |
| 166 | + path: str | None = None |
| 167 | + in_header = False |
| 168 | + for line in diff.splitlines(): |
| 169 | + if line.startswith("diff --git "): |
| 170 | + path, in_header = None, True |
| 171 | + elif not in_header: |
| 172 | + pairs.append((path, line)) |
| 173 | + elif line.startswith("@@"): |
| 174 | + in_header = False |
| 175 | + elif line.startswith("+++ b/"): |
| 176 | + # Git pads the path with a tab when the name needs it. |
| 177 | + path = line[len("+++ b/") :].split("\t")[0] |
| 178 | + return pairs |
| 179 | + |
| 180 | + |
| 181 | +def owned(path: str | None) -> bool: |
| 182 | + """Say whether a path is one the template writes.""" |
| 183 | + return path is not None and path.startswith(TEMPLATE_OWNED) |
| 184 | + |
| 185 | + |
| 186 | +def added_silencers(pairs: list[tuple[str | None, str]], untracked: list[str]) -> bool: |
| 187 | + """Say whether this project gained a suppression comment. |
| 188 | +
|
| 189 | + Three things have to line up. The file must be one this project authored - |
| 190 | + a `# noqa` inside a template owned file was written by the template. It |
| 191 | + must be Python, or prose naming the comment counts as using it. And the |
| 192 | + file must end up with more of them than it started with, so editing a line |
| 193 | + that has always carried one is not a finding. |
| 194 | +
|
| 195 | + Untracked files are read from disk, because `git diff HEAD` does not list |
| 196 | + them and writing a new module is the most common way a suppression arrives. |
| 197 | + """ |
| 198 | + counts: dict[str, int] = {} |
| 199 | + for path, line in pairs: |
| 200 | + if owned(path) or path is None or not path.endswith(PYTHON_SUFFIXES): |
| 201 | + continue |
| 202 | + if not SILENCER.search(line[1:]): |
| 203 | + continue |
| 204 | + if line.startswith("+"): |
| 205 | + counts[path] = counts.get(path, 0) + 1 |
| 206 | + elif line.startswith("-"): |
| 207 | + counts[path] = counts.get(path, 0) - 1 |
| 208 | + if any(count > 0 for count in counts.values()): |
| 209 | + return True |
| 210 | + for path in untracked: |
| 211 | + if owned(path) or not path.endswith(PYTHON_SUFFIXES): |
| 212 | + continue |
| 213 | + try: |
| 214 | + text = Path(path).read_text(encoding="utf-8", errors="replace")[:READ_LIMIT] |
| 215 | + except OSError: |
| 216 | + continue |
| 217 | + if SILENCER.search(text): |
| 218 | + return True |
| 219 | + return False |
| 220 | + |
| 221 | + |
| 222 | +def collect_evidence() -> list[str]: |
| 223 | + """Return human readable evidence of template friction, newest first.""" |
| 224 | + status = git("status", "--porcelain", "-uall", "-z") |
| 225 | + diff = git("diff", "HEAD") |
| 226 | + if status is None or diff is None: |
| 227 | + return [] |
| 228 | + |
| 229 | + pairs = attributed(diff) |
| 230 | + listed = entries(status) |
| 231 | + evidence = [] |
| 232 | + |
| 233 | + # A `copier update` rewrites every template owned path by definition. Do not |
| 234 | + # report the act of taking a new template version as friction with it - a |
| 235 | + # suppression written by hand while resolving an update conflict is still a |
| 236 | + # real finding, and is caught below because it lands in this project's own |
| 237 | + # source rather than in a template owned file. |
| 238 | + updating = any(COPIER_UPDATE.match(line) for _, line in pairs) |
| 239 | + |
| 240 | + touched = sorted({path for _, path in listed if path.startswith(TEMPLATE_OWNED)}) |
| 241 | + if touched and not updating: |
| 242 | + evidence.append(f"template owned files were changed here: {', '.join(touched)}") |
| 243 | + |
| 244 | + rejects = sorted({path for _, path in listed if path.endswith(".rej")}) |
| 245 | + if rejects: |
| 246 | + evidence.append(f"a copier update left rejected hunks behind: {', '.join(rejects)}") |
| 247 | + |
| 248 | + if added_silencers(pairs, [path for code, path in listed if code == "??"]): |
| 249 | + evidence.append("a '# noqa' or '# type: ignore' was added") |
| 250 | + |
| 251 | + # Conflict markers are the opposite case: one inside a template owned file |
| 252 | + # is precisely the "the update could not be merged" report worth having. |
| 253 | + if any( |
| 254 | + CONFLICT.match(line) |
| 255 | + for path, line in pairs |
| 256 | + if path is not None and path != CONFLICT_PROSE |
| 257 | + ): |
| 258 | + evidence.append("conflict markers are still in the working tree") |
| 259 | + |
| 260 | + # `pyproject.toml` is not template owned, but its lint configuration is |
| 261 | + # rendered - so during an update a new ignore entry arrived with the |
| 262 | + # template rather than being chosen here. |
| 263 | + try: |
| 264 | + current = Path("pyproject.toml").read_text(encoding="utf-8") |
| 265 | + except OSError: |
| 266 | + current = None |
| 267 | + silenced = set() if updating else added_ignores(git("show", "HEAD:pyproject.toml"), current) |
| 268 | + if silenced: |
| 269 | + evidence.append( |
| 270 | + "these lint rules joined the ruff ignore list in pyproject.toml: " |
| 271 | + + ", ".join(sorted(silenced)) |
| 272 | + ) |
| 273 | + |
| 274 | + return evidence |
| 275 | + |
| 276 | + |
| 277 | +def added_ignores(before: str | None, after: str | None) -> set[str]: |
| 278 | + """Return the rule codes `[tool.ruff.lint] ignore` gained, comparing two files. |
| 279 | +
|
| 280 | + Reading the table is the whole point. Matching a quoted rule code against |
| 281 | + the diff cannot tell which table it is in, so it fired on `extend-select`, |
| 282 | + which *tightens* linting, on a `[tool.deptry]` entry, and on the |
| 283 | + `per-file-ignores` relaxation for tests that the shipped rules prescribe - |
| 284 | + reporting the project for doing what it was told. |
| 285 | +
|
| 286 | + `per-file-ignores` is deliberately not looked at for that reason. Anything |
| 287 | + unreadable on either side yields no codes, because a suppression that |
| 288 | + cannot be established is not evidence. |
| 289 | + """ |
| 290 | + |
| 291 | + def ignores(source: str | None) -> set[str] | None: |
| 292 | + if source is None: |
| 293 | + return None |
| 294 | + try: |
| 295 | + lint = tomllib.loads(source)["tool"]["ruff"]["lint"] |
| 296 | + except (tomllib.TOMLDecodeError, ValueError, KeyError, TypeError): |
| 297 | + return None |
| 298 | + listed = lint.get("ignore") if isinstance(lint, dict) else None |
| 299 | + if not isinstance(listed, list): |
| 300 | + return set() |
| 301 | + return {code for code in listed if isinstance(code, str)} |
| 302 | + |
| 303 | + old, new = ignores(before), ignores(after) |
| 304 | + if old is None or new is None: |
| 305 | + return set() |
| 306 | + return new - old |
| 307 | + |
| 308 | + |
| 309 | +def marker_for(session_id: str) -> Path: |
| 310 | + """Return the once-per-session marker file for a session id.""" |
| 311 | + safe = re.sub(r"[^A-Za-z0-9_-]", "", session_id)[:64] or "unknown" |
| 312 | + return Path(tempfile.gettempdir()) / f"cmem-template-feedback-{safe}" |
| 313 | + |
| 314 | + |
| 315 | +def main() -> None: |
| 316 | + """Read the hook payload, and block once if there is something to report.""" |
| 317 | + try: |
| 318 | + payload = json.load(sys.stdin) |
| 319 | + except (json.JSONDecodeError, ValueError, OSError): |
| 320 | + return |
| 321 | + |
| 322 | + if not isinstance(payload, dict) or payload.get("stop_hook_active"): |
| 323 | + return |
| 324 | + if OPT_OUT.exists(): |
| 325 | + return |
| 326 | + |
| 327 | + marker = marker_for(str(payload.get("session_id", ""))) |
| 328 | + if marker.exists(): |
| 329 | + return |
| 330 | + |
| 331 | + evidence = collect_evidence() |
| 332 | + if not evidence: |
| 333 | + return |
| 334 | + |
| 335 | + try: |
| 336 | + marker.touch() |
| 337 | + except OSError: |
| 338 | + return |
| 339 | + |
| 340 | + reason = ( |
| 341 | + "Before finishing: this session shows signs that something in the " |
| 342 | + "cmem-plugin-template got in the way - " |
| 343 | + + "; ".join(evidence) |
| 344 | + + ". Consider whether that is a finding other generated projects would " |
| 345 | + "share. If it is, use the 'template-feedback' skill to check what has " |
| 346 | + "already been decided and to draft an issue for the template, which " |
| 347 | + "you must show the user before filing anything. If it is specific to " |
| 348 | + "this project, say so in one sentence and stop - that is a complete " |
| 349 | + "answer and this check will not ask again in this session." |
| 350 | + ) |
| 351 | + json.dump({"decision": "block", "reason": reason}, sys.stdout) |
| 352 | + |
| 353 | + |
| 354 | +if __name__ == "__main__": |
| 355 | + try: |
| 356 | + main() |
| 357 | + except Exception: |
| 358 | + pass |
0 commit comments