test: pr-creation-log のアサーションを matcher ではなくコマンドで選ぶよう修正 - #169
Conversation
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) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a6d91b3a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _pr_entries='[.hooks.PostToolUse[]?.hooks[]? | ||
| | select((.command? // "") | contains("pr-creation-log/log-pr.sh"))]' |
There was a problem hiding this comment.
Keep asserting the hook's Bash matcher
Selecting only the nested command discards its parent matcher, so if the pr-creation-log entry regresses from "matcher": "Bash" to Write or another value, both helpers still pass even though gh pr create will no longer trigger the hook; the previous filter caught this case. Select the parent entry by its contained command and additionally assert that its matcher is Bash.
AGENTS.md reference: AGENTS.md:L247-L249
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Fixes #167
概要
tests/unit/test-pr-creation-log.shの 6 箇所(L53 / L54 / L73 / L74 / L93 / L94)が、#161 と同じ「jq -eは最後の出力で終了コードを決める」仕様に依存していた。現状は誤判定しないが、
matcher: "Bash"のPostToolUseが 2 つ目になった時点で発火する。否定形のアサーションは偽 PASS になるため、テストが守っているつもりの性質が黙って守られなくなる。原因
jq -e '.hooks.PostToolUse[] | select(.matcher == "Bash") | .hooks[0] | has("if") | not'selectは一致したエントリの数だけ出力を生成するため、matcher: "Bash"のエントリが 2 つ以上あると最後の 1 件だけで判定が決まる。キット内で
matcher: "Bash"のPostToolUseを持つのはpr-creation-logの 1 件だけ(他はprettier-hooks/biome-hooksがEdit|Write、doc-size-guardがWrite)。_versioned_hooks_fragmentはhooks.jsonかhooks.legacy.jsonのどちらか一方しか返すため、二重にもならない。よって現状は潜在。CLAUDE.md の「Adding a New Feature」は新しい hook を既存の後ろに追記するよう案内しており、追記されたエントリは
pr-creation-logより後ろに来る。これは否定形が偽 PASS になる並びそのもの。修正
意図は「pr-creation-log のエントリがどうなっているか」なので、
matcherではなくコマンドで選ぶ。pr-creation-log/log-pr.shは modern・legacy 双方のコマンド文字列に含まれるため、単一のセレクタで両方に使える。そのうえで
length == 1で対象を 1 件に固定し、all(...)で畳み込む。否定形は
any()では直らない#161 で採用した「配列集約 +
any()」をそのまま当てても、否定形は偽 PASS のまま残る。生成した退行フィクスチャ(pr-creation-logを modern 形に退行させ、2 つ目の Bash エントリを追加)での実測:select(L93)any()all()length == 1+all()3 箇所で同じフィルタを使うため、
_pr_modern_hook_ok/_pr_legacy_hook_okの 2 ヘルパーに集約した。追加した回帰テスト
arity 自体を守るテストを 1 件追加した。2 つ目の Bash matcher エントリを足し、
pr-creation-log側を modern 形に退行させたフィクスチャに対して、legacy 判定が失敗することと、modern 判定が成功することの両方を確認する。旧フィルタではこの検査が通ってしまう(=守れていない)。
実行した検証
shellcheck -S warning tests/unit/test-pr-creation-log.sh— クリーンtests/unit/test-pr-creation-log.sh— 7/7 通過(新規 1 件を含む)CHANGELOG
テストのみの変更で利用者に見える挙動は変わらないため、CHANGELOG は更新していない。直近の同種コミット
21a860d(test: test-mdm-detect.sh に watchdog タイムアウトを宣言)も CHANGELOG に触れていない。アップグレード経路 / ドキュメント
本番コード、
ENABLE_*フラグ、profile 既定値、生成ファイル、settings キー、manifest のいずれも変更していない。README.md/README.en.md/docs/も対象外。補足
本番コード側の同クラスの掃討は完了しており、該当したのは
lib/update.sh:2101/:2104の 2 箇所のみ(PR #164 で修正)。lib/・setup.sh・commands/・features/の他のjq -eはすべて単一出力であることを確認済み。🤖 Generated with Claude Code