Skip to content

docs(claude-code-hooks): three gaps found by actually building a guard with this skill - #295

Merged
daymade merged 2 commits into
mainfrom
feat/hooks-skill-shellcheck-selftest
Aug 15, 2026
Merged

docs(claude-code-hooks): three gaps found by actually building a guard with this skill#295
daymade merged 2 commits into
mainfrom
feat/hooks-skill-shellcheck-selftest

Conversation

@daymade

@daymade daymade commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Built a real PreToolUse guard using this skill, and hit three things it doesn't cover — each of which every hook author will hit. 63 lines added, 0 removed.

⚠️ Merge-order note — read before merging

perf/history-search-date-prefilter (pushed at e9c8235, no PR yet) carries two rounds of independent-review work on this same file, and one of its hunks edits the exact bullet this PR attaches to — "If the guard needs a release valve, make it a human gate, not an env var" — appending its own nested ⚠️ /dev/tty sub-bullet there.

This PR will conflict with that branch at that anchor. The resolution is trivial and non-semantic: both sub-bullets belong under the same parent and neither contradicts the other. Keep both, ⚠️ /dev/tty first, then Below Tier-0. Merging that branch first and rebasing this one is the cleaner order, since it is the more mature work on this file.

1. Check ShellCheck first — and record the answer (build order, step 3)

It is the de-facto standard for shell anti-patterns, so "why didn't you just use shellcheck" is the first thing a reviewer asks. Without the answer written down, every author re-derives it.

Measured on 0.11.0 against find . -name x | head -5 || echo "no" — a fallback that provably cannot fire, since || binds to the pipeline's last stage and head exits 0 on empty input:

Result
default config nothing reported, exit 0
--enable=all SC2312 (check-extra-masked-returns) fires
same rule on cmd | jq . || echo bad (a live fallback) fires identically

Three reasons that disqualifies it as the gate, each generalizing beyond this case: it is off by default (protecting nobody today), it cannot distinguish a dead fallback from a live one (blanket firing is the rule-1 false-block spiral), and its own suggested remedy is "use || true to ignore" — the opposite of the intent. It also lints files, not tool-call events.

General shape: the standard linter is the right thing to check and usually the wrong thing to delegate a blocking gate to — linters are tuned for advisory breadth, a gate needs precision.

2. Give the hook a --selftest, sized by mutants killed (build order, step 4)

The pattern existed informally in hook_pitfalls.md but was never in the build-order checklist. It is the only automatic check for the one failure bash -n and registration both structurally miss: a hook degraded into a permanent no-op — invisible by construction, because a guard that never fires produces output identical to a session with nothing to report.

The sizing rule is the part worth having, and it came from being wrong first. An earlier draft of this PR said "keep it to two probes". That is refuted by the shipped compounding-edit-review, whose own header records: its first version's two fixtures killed only 4 of 14 mutants — every behavior its comments declared load-bearing had zero coverage, including a mutation short-circuiting the anti-loop check while the selftest still printed OK. It now runs 58 cases.

So: two fixtures is the floor (one must-block + one must-pass; either kind alone is blind in one direction), and you size up by mutants killed. The real constraint is session-start wall-clock — measured ~5.3 s for those 58 cases vs ~140 ms for a two-probe liveness check. When killing the mutants exceeds that budget, split rather than shrink: cheap liveness probe on --selftest, full battery in test_hook.sh at build time.

3. Below Tier-0, make the escape hatch the correct usage (nested under rule 4)

Rule 4 is absolute for Tier-0 and this does not bend it — it addresses the correctness guards that fall short of Tier-0, which still need a way out for the legitimate case the detector cannot distinguish.

A SKIP=1 / --force escape trains exactly the reflex rule 1 warns about. Prefer an escape that is the thing you wanted them to do anyway, so taking it improves the command instead of disarming the guard. The test: if someone takes my escape hatch, is the resulting command better, or merely unblocked? Cross-referenced to the sibling Tier-0 principle already in hook_patterns.md ("the escape hatch may only make the gate stricter"), which points the other way mechanically and reads as tension until you work it out.

Placed as a nested bullet under rule 4's human-gate rule specifically so the Tier-0 scoping is structural rather than a qualifier someone can read past.

Verification

  • quick_validate → valid
  • Existing-skill migration gate (audit_skill_regression compare) → 0 candidates, 716 exact preservations. It earned its keep: it caught a real defect this PR introduced — a sub-bullet insertion had pushed the back half of step 4 to the end of the new text, where it read as part of the selftest advice. Fixed, re-run clean.
  • One fresh-context independent review, which re-ran shellcheck itself and reproduced every measured claim; all three of its substantive findings were verified and applied (they are what produced §2's refutation and §3's placement).
  • No step renumbering: step 4 is cross-referenced elsewhere in the file, so both additions extend existing steps in place.

🤖 Generated with Claude Code

daymade and others added 2 commits August 15, 2026 17:25
这次真做了一个 PreToolUse guard(拦「|| 兜底永远不触发」的存在性探测),
过程里撞到三处 skill 没覆盖、而每个 hook 作者都会撞到的东西。

1) build order step 3 补「先查 ShellCheck,并把答案记下来」
   它是 shell 反模式的事实标准,所以"你怎么不用 shellcheck"是 reviewer 的
   第一个问题,不写下来每个作者都要重新论证一遍。实测 0.11.0:默认配置对
   `find . | head -5 || echo no` 完全不报(exit 0);--enable=all 后 SC2312
   会报,但对合法的 `cmd | jq . || echo bad` 同样报。三条不合格理由都可泛化:
   默认关闭(今天没在保护任何人)、分不清死兜底和活兜底(无差别命中=rule 1 的
   误杀螺旋)、它给的修法是"用 || true 忽略"(方向相反);而且它 lint 文件不是
   tool-call 事件。一般形状:标准 linter 是该【查】的东西,通常不是该把闸门
   【托付】给它的东西——linter 调的是广度,闸门要的是精度。

2) build order step 4 补 `--selftest` 约定
   SessionStart 的守卫体检可以调它,这是唯一能自动抓到「hook 退化成永久
   no-op」的检查——那种失效 bash -n 和注册检查结构上都看不见,因为一个
   永不触发的守卫,输出和"这次没什么可报告"完全一样,能这样躺几周。
   要求两个 fixture 不是一个(必拦样本 + 必放样本),并且要反向标定:
   故意弄坏检测器,确认 --selftest 退非零——没见它红过的 selftest 和
   `exit 0` 不可区分。

3) rule 1 补「逃生口该怎么设计」
   原文讲透了反射性绕过的危害,但没给正面解法。补:逃生口应该是【你本来
   就希望他做的那件事】,不是 SKIP=1。判据一句话——如果有人走了我的逃生口,
   得到的命令是变好了,还是仅仅被放行了?答"仅仅被放行"就说明这是个换了
   好名字的 bypass flag。

不新增编号步骤(step 4 在第 771 行被交叉引用,重编号会打断它),只就地扩写。

迁移闸门:0 候选 / 716 处原样保留;quick_validate 通过;48 行纯新增 0 删除。
过程中审计抓到一处我自己造的缺陷——插子弹时把 step 4 后半句挤到了末尾、
读起来像 selftest 建议的一部分,已修复后复跑归零。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
独立审阅(fresh context,自己跑了 shellcheck 复核)带回三条,全部复验成立:

1) 「keep it to two probes」是错的,已推翻。
   审阅只能提出疑问(它手上没有那个 hook),我这边查到了硬证据:
   compounding-edit-review 的 --selftest 是同一个 CLI 机制、实跑 58 个用例,
   而且它自己的头注直接反驳这条——「自检的价值取决于它能杀死多少变异体,
   不取决于它是否通过。首版两个 fixture 只杀掉 14 个变异里的 4 个」,漏掉的
   包括一个把 stop_hook_active 短路成永不生效、自检照样印 OK 的变异。
   改成:两个是下限不是目标,按杀死的变异体定大小。同时实测了成本张力
   (58 用例 5.3s vs 2 探针 140ms),给出正确解法是【拆】不是【缩】:
   会话启动放廉价存活探针,完整回归留在 test_hook.sh。

2) 逃生口那条缺 Tier-0 作用域,会教出 rule 4 专门要杀的反模式。
   我推荐的是「模型自己就能满足的逃生口」(在命令里写 PIPESTATUS),而 rule 4
   对 Tier-0 是绝对的:模型能自己满足的就不是闸门。原来那条写成无限定的
   「every guard」,读者拿去做 Tier-0 守卫就会造出一个假闸门。
   修法不是加一句限定语,是把它整条移到 rule 4 那条规则底下做嵌套子弹——
   作用域变成结构性的。顺带补上 hook_patterns 里的姊妹原则交叉引用
   (「逃生口只能让闸门更严」,管的是 Tier-0 那一侧)。

3) 三个小疵:主谓不一致、引用了一个文件里没定义的术语「the registration
   check」、以及那条推论的格式没跟同节其它 Corollary 对齐(移位后自动解决)。

迁移闸门复跑:0 候选 / 716 处原样保留;quick_validate 通过;相对 main
63 行纯新增 0 删除。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@daymade
daymade merged commit 94ebba6 into main Aug 15, 2026
4 checks passed
@daymade
daymade deleted the feat/hooks-skill-shellcheck-selftest branch August 15, 2026 13:45
daymade added a commit that referenced this pull request Aug 15, 2026
…cleaner

docs(claude-code-hooks): land two rounds of independent-review fixes (conflict with #295 resolved)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant