From 4a6d91b3a466edb280dd2f92866f5f8c14b47594 Mon Sep 17 00:00:00 2001 From: Shinji Saito Date: Mon, 31 Aug 2026 14:40:43 +0900 Subject: [PATCH 1/2] =?UTF-8?q?test:=20pr-creation-log=20=E3=81=AE?= =?UTF-8?q?=E3=82=A2=E3=82=B5=E3=83=BC=E3=82=B7=E3=83=A7=E3=83=B3=E3=82=92?= =?UTF-8?q?=20matcher=20=E3=81=A7=E3=81=AF=E3=81=AA=E3=81=8F=E3=82=B3?= =?UTF-8?q?=E3=83=9E=E3=83=B3=E3=83=89=E3=81=A7=E9=81=B8=E3=81=B6=E3=82=88?= =?UTF-8?q?=E3=81=86=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #167 tests/unit/test-pr-creation-log.sh の 6 箇所（L53/54/73/74/93/94）が #161 と同じ「jq -e は最後の出力で終了コードを決める」仕様に依存していた。 jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0] | has("if") | not' select は一致したエントリの数だけ出力を生成するため、matcher: "Bash" の PostToolUse が 2 つ以上あると最後の 1 件だけで判定が決まる。 現状はキット内で matcher: "Bash" の PostToolUse が pr-creation-log の 1 件 だけなので誤判定しないが、2 つ目が追加された時点で発火する。CLAUDE.md の 「Adding a New Feature」は新しい hook を既存の後ろに追記するよう案内して おり、追記されたエントリは pr-creation-log より後ろに来る。これは否定形の アサーション（L93/94）が偽 PASS になる並びそのもの。 偽 PASS は、テストが守っているつもりの性質（legacy 環境で if/async を 持たないこと）が黙って守られなくなることを意味する。 修正: コマンドで対象を選び、all() で畳み込む 意図は「pr-creation-log のエントリがどうなっているか」なので、matcher では なくコマンド（pr-creation-log/log-pr.sh、modern・legacy 両方に含まれる）で 選ぶ。そのうえで length == 1 で対象を 1 件に固定し、all() で畳み込む。 否定形は any() では直らない。退行フィクスチャでの実測: 旧: matcher 基準 + 素の select rc=0 ← 偽 PASS matcher 基準 + any() rc=0 ← 直らない matcher 基準 + all() rc=1 採用: command 基準 + length==1 + all() rc=1 3 箇所で同じフィルタを使うため、_pr_modern_hook_ok / _pr_legacy_hook_ok の 2 つのヘルパーに集約した。 あわせて arity 自体の回帰テストを 1 件追加した。2 つ目の Bash matcher エントリ を足し、pr-creation-log 側を modern 形に退行させたフィクスチャに対して、 legacy 判定が失敗することを確認する。旧フィルタではこの検査が通ってしまう。 テストのみの変更のため CHANGELOG は更新していない（21a860d と同じ扱い）。 検証: - shellcheck -S warning tests/unit/test-pr-creation-log.sh: クリーン - tests/unit/test-pr-creation-log.sh: 7/7 通過 Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test-pr-creation-log.sh | 56 ++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/tests/unit/test-pr-creation-log.sh b/tests/unit/test-pr-creation-log.sh index 408a0db..249fa00 100644 --- a/tests/unit/test-pr-creation-log.sh +++ b/tests/unit/test-pr-creation-log.sh @@ -44,14 +44,38 @@ else fail "pr-creation-log: hook should use external script with tool_response schema" fi +# Select this feature's entry by its command, never by matcher. +# `.hooks.PostToolUse[] | select(.matcher == "Bash")` emits one output per +# matching entry and `jq -e` derives its exit code from the LAST one, so a +# second Bash-matcher PostToolUse hook would silently decide the verdict — and +# for the negated legacy checks it would decide it as a PASS, masking exactly +# the regression they exist to catch. Collect first, then assert on the whole +# set: `length == 1` pins the entry and `all(...)` is the correct collapse for +# a negated condition (`any(...)` would still pass on a regressed first entry). +_pr_entries='[.hooks.PostToolUse[]?.hooks[]? + | select((.command? // "") | contains("pr-creation-log/log-pr.sh"))]' + +_pr_modern_hook_ok() { # + jq -e "$_pr_entries + | length == 1 + and all(.if == \"Bash(gh pr create *)\" and .async == true)" \ + "$1" >/dev/null 2>&1 +} + +_pr_legacy_hook_ok() { # + jq -e "$_pr_entries + | length == 1 + and all((has(\"if\") | not) and (has(\"async\") | not))" \ + "$1" >/dev/null 2>&1 +} + build_settings_json \ "$PROJECT_DIR/config/settings-base.json" \ "$PROJECT_DIR/config/permissions.json" \ "$_pr_settings" \ "$_pr_hooks" >/dev/null -if jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0].if == "Bash(gh pr create *)"' "$_pr_settings" >/dev/null 2>&1 \ - && jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0].async == true' "$_pr_settings" >/dev/null 2>&1; then +if _pr_modern_hook_ok "$_pr_settings"; then pass "pr-creation-log: merged settings keep if condition and async execution" else fail "pr-creation-log: merged settings should keep if condition and async execution" @@ -70,8 +94,7 @@ _CLAUDE_SEMVER_CACHE="" _CLAUDE_SEMVER_CACHE_SET=false PATH="$_pr_tmp/claude-current-bin:$PATH" build_settings_file "$_pr_supported_settings" >/dev/null -if jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0].if == "Bash(gh pr create *)"' "$_pr_supported_settings" >/dev/null 2>&1 \ - && jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0].async == true' "$_pr_supported_settings" >/dev/null 2>&1; then +if _pr_modern_hook_ok "$_pr_supported_settings"; then pass "pr-creation-log: supported Claude Code gets if/async hook" else fail "pr-creation-log: supported Claude Code should get if/async hook" @@ -90,13 +113,34 @@ _CLAUDE_SEMVER_CACHE="" _CLAUDE_SEMVER_CACHE_SET=false PATH="$_pr_tmp/claude-legacy-bin:$PATH" build_settings_file "$_pr_legacy_settings" >/dev/null -if jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0] | has("if") | not' "$_pr_legacy_settings" >/dev/null 2>&1 \ - && jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0] | has("async") | not' "$_pr_legacy_settings" >/dev/null 2>&1; then +if _pr_legacy_hook_ok "$_pr_legacy_settings"; then pass "pr-creation-log: legacy Claude Code falls back to legacy hook fragment" else fail "pr-creation-log: legacy Claude Code should fall back to legacy hook fragment" fi +# The arity guard itself. Append a second Bash-matcher PostToolUse entry and +# regress the pr-creation-log one to the modern shape: the legacy assertion +# must now fail. The old `select(.matcher == "Bash") | .hooks[0] | has("if") | +# not` filter returns 0 here (the appended entry is the last output), and so +# does an `any(...)` rewrite — only selecting by command and collapsing with +# `all(...)` reports the regression. +_pr_arity_settings="$_pr_tmp/settings-two-bash.json" +jq '.hooks.PostToolUse = [ + {"matcher":"Bash","hooks":[{"type":"command", + "command":"__HOME__/.claude/hooks/pr-creation-log/log-pr.sh", + "if":"Bash(gh pr create *)","async":true}]}, + {"matcher":"Bash","hooks":[{"type":"command", + "command":"/somewhere/future-hook.sh"}]} + ]' "$_pr_legacy_settings" > "$_pr_arity_settings" + +if ! _pr_legacy_hook_ok "$_pr_arity_settings" \ + && _pr_modern_hook_ok "$_pr_arity_settings"; then + pass "pr-creation-log: a second Bash-matcher hook cannot decide the verdict" +else + fail "pr-creation-log: assertions must select this feature's entry by command" +fi + _pr_script="$_pr_tmp/pr-hook.sh" _pr_input="$_pr_tmp/input.json" _pr_out="$_pr_tmp/out.json" From 7f87b68021202e69e8892f5d34f55fb0d37b80d6 Mon Sep 17 00:00:00 2001 From: okash1n <48118431+okash1n@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:45:51 +0900 Subject: [PATCH 2/2] =?UTF-8?q?test:=20pr-creation-log=20=E3=82=A8?= =?UTF-8?q?=E3=83=B3=E3=83=88=E3=83=AA=E3=81=AE=20matcher=20"Bash"=20?= =?UTF-8?q?=E3=82=A2=E3=82=B5=E3=83=BC=E3=82=B7=E3=83=A7=E3=83=B3=E3=82=92?= =?UTF-8?q?=E5=BE=A9=E6=B4=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/test-pr-creation-log.sh | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/unit/test-pr-creation-log.sh b/tests/unit/test-pr-creation-log.sh index 249fa00..86ba5f6 100644 --- a/tests/unit/test-pr-creation-log.sh +++ b/tests/unit/test-pr-creation-log.sh @@ -52,20 +52,26 @@ fi # the regression they exist to catch. Collect first, then assert on the whole # set: `length == 1` pins the entry and `all(...)` is the correct collapse for # a negated condition (`any(...)` would still pass on a regressed first entry). -_pr_entries='[.hooks.PostToolUse[]?.hooks[]? - | select((.command? // "") | contains("pr-creation-log/log-pr.sh"))]' +# Selection walks the PARENT PostToolUse entries (not the inner hooks) so the +# matcher stays visible: this feature must remain bound to matcher "Bash". +_pr_entries='[.hooks.PostToolUse[]? + | select(any(.hooks[]?; (.command? // "") | contains("pr-creation-log/log-pr.sh")))]' _pr_modern_hook_ok() { # jq -e "$_pr_entries | length == 1 - and all(.if == \"Bash(gh pr create *)\" and .async == true)" \ + and all(.matcher == \"Bash\" + and (.hooks | length == 1) + and all(.hooks[]; .if == \"Bash(gh pr create *)\" and .async == true))" \ "$1" >/dev/null 2>&1 } _pr_legacy_hook_ok() { # jq -e "$_pr_entries | length == 1 - and all((has(\"if\") | not) and (has(\"async\") | not))" \ + and all(.matcher == \"Bash\" + and (.hooks | length == 1) + and all(.hooks[]; (has(\"if\") | not) and (has(\"async\") | not)))" \ "$1" >/dev/null 2>&1 }