diff --git a/docs/claude-code-web-tasks.md b/docs/claude-code-web-tasks.md index 4661a28d..277bc6da 100644 --- a/docs/claude-code-web-tasks.md +++ b/docs/claude-code-web-tasks.md @@ -179,6 +179,7 @@ cargo test で検証完結するが、新規 module / lint rule / 軽微リフ | 340 | T2 | — | `decide.rs` の rate_limit × positive-evidence 複合境界テスト + `main.rs` の rate_limit threading テスト | `src/check-ci-coderabbit/src/{decide,main}.rs` | S | (a) は純関数で容易。(b) は `main.rs` の呼び出し側を I/O 無しでテスト可能にする小さな合成関数抽出リファクタが要る | | | 272 | T1 | — | cli-docs-lint に ADR 重複採番検出 + CLAUDE.md 索引整合チェック(新規 validator module) | `src/cli-docs-lint/src/adr_consistency.rs`(新規)+ `src/cli-docs-lint/src/main.rs`(CheckMode dispatch 拡張) | S-M | 中核(validator + fixture test)は cargo test で完結。「pnpm lint:docs 経由の発火確認」は Web 外だが成功条件ではない。CLAUDE.md は docs_dir の親なので TempDir で fake 構造を組む | | | 179 | T2 | — | rate-limit retry 境界(max_retries=0/1/3)で retry 継続 vs `action_required` 遷移の off-by-one を pin する parameterized テスト | `src/cli-pr-monitor/src/stages/poll/rate_limit.rs`(判定 L52)+ `src/cli-pr-monitor/src/config.rs`(L143-155) | S-M | **todo の「rstest 使用済」は誤り**(Cargo.lock に不在)。新 dev-dep 追加 or plain 複数 `#[test]` で代替を着手時判断。gh subprocess を踏まない早期 return 経路で構成する | | +| 498 | T2 | — | `other_ext_tests` を拡張子ごとの map へ移し、非主要拡張子も 1 つずつ coverage を要求する | `src/hooks-post-tool-linter/src/custom_rules/types.rs` + `src/hooks-post-tool-linter/src/custom_rules/coverage.rs` + `.claude/custom-lint-rules.toml` | M | 現行契約は「非主要拡張子は rule あたり 1+ test」で、`jsonc` と `json` を宣言し `jsonc` 用テストだけでも通る (PR #461 CodeRabbit 指摘)。既存の平坦な `other_ext_tests` を拡張子へ割り当て直す作業は、各テストがどの拡張子を実際に通しているか読む判断が要るため無人可にしない。契約の現状は `non_main_extension_coverage_is_per_rule_not_per_extension` が固定している | test(post-tool-linter): 非主要拡張子の coverage を拡張子ごとに要求する | | ### 無人可としなかった理由 diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index 8f887357..3a8f4410 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -200,6 +200,7 @@ | 495 | 🔧 Tier 2 | **`lib-*` crate の責務分類基準が ADR-012 に無い** | todo25.md | S | なし (週次レビュー WR-2026-08-22-A04、severity=medium、facet=architecture、category=module-boundary) | | 496 | 🔧 Tier 2 | **docs の 50KB 超過 3 ファイルを物理分割する** | todo25.md | M | なし (2026-08-22 週次レビューの決定論 scan 由来。`todo-summary2.md` は優先度表 1 枚のため節ではなく順位で切る必要がある) | | 497 | 🔧 Tier 2 | **PostToolUse で docs ファイルの 50KB 超過を即時ブロックする** | todo25.md | S | なし (2026-08-22 週次レビューの決定論 scan 由来。現在 file-length の検査は週次レビューの報告のみで、超過しても何も止まらない。順位 496 と対) | +| 498 | 🔧 Tier 2 | **非主要拡張子の coverage を拡張子ごとに要求する (`other_ext_tests` の map 化)** | todo25.md | M | なし (PR #461 の CodeRabbit 指摘由来。現行契約は「rule あたり 1+ test」で、その契約自体は `non_main_extension_coverage_is_per_rule_not_per_extension` が固定済み) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で計測基盤 (gate telemetry / weekly-review 保存) + rate-limit + convergence cost 削減を進める → Tier 3 でドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。(2026-08-12 更新: 旧記述の ADR-032 は ADR-057 置換で欠番) diff --git a/docs/todo25.md b/docs/todo25.md index 9bc21d64..fe9d0aae 100644 --- a/docs/todo25.md +++ b/docs/todo25.md @@ -538,3 +538,27 @@ green になること。検査を外す変異で落ちること。 台帳を読む step と agent が触る作業ツリーが**同一 SHA を見ている**ことがログから確認できる こと。run の途中で master が進んでも選択と実装がずれないこと。 + +### 順位 498: 非主要拡張子の coverage を拡張子ごとに要求する (`other_ext_tests` の map 化) + +> **実行優先度**: 🔧 **Tier 2** — 検査の穴であり実害はまだ出ていないが、rule に非主要拡張子を足したときに coverage 不足を見逃す。 + +**動機**: `custom-lint-rules.toml` の `test_coverage` は主要拡張子 (`rs` / `toml` / `yaml` / `yml`) を `main_ext_tests: BTreeMap<拡張子, Vec<テスト名>>` で拡張子ごとに持つ一方、非主要拡張子は `other_ext_tests: Vec<テスト名>` で**拡張子との対応を持たない**。そのため `jsonc` と `json` を宣言し `jsonc` 用テストだけを登録した rule が検査を通る (PR [#461](https://github.com/aloekun/claude-code-hook-test/pull/461) の CodeRabbit 指摘)。 +**これは実装漏れではなく契約**である — 順位 137 が定めた非主要拡張子の要件は「rule あたり 1+ positive test」で、`.claude/custom-lint-rules.toml` のコメントにもそう書いてある。契約を強める作業なので別起票にした。現行契約は `non_main_extension_coverage_is_per_rule_not_per_extension` が固定しており、意図せず緩んだ場合はそこで落ちる。 + +#### 作業内容 + +1. `CustomRuleTestCoverage::other_ext_tests` を `Vec` から `BTreeMap>` (拡張子 → テスト名) へ変える +2. `.claude/custom-lint-rules.toml` の既存 rule をすべて新形式へ移す。**平坦なリストを拡張子へ割り当て直すには、各テストがどの拡張子の fixture を実際に通しているかを読む必要がある** — ここが本タスクで一番時間を使う部分で、機械的な変換ではない +3. `extension_coverage_gaps` / `check_other_ext_coverage` を拡張子ごとの判定へ更新する +4. 契約を固定していた `non_main_extension_coverage_is_per_rule_not_per_extension` を、新契約 (拡張子ごと) を固定するテストへ差し替える + +#### 完了基準 + +- `jsonc` と `json` を宣言し `jsonc` 用テストだけを持つ rule が **検査で落ちる**ことを、fixture ベースのテストで固定する +- 既存 rule のすべてが新形式で `rule_test_coverage_check` を通る (移行漏れがないこと) +- 実 `.claude/custom-lint-rules.toml` を読む検査が green のままであること + +#### 無人可にしない理由 + +対象ファイルは Guard 禁止パスに当たらないが、**手順 2 が判断を要する** (どのテストがどの拡張子を通しているかの読み取り)。機械的な置換ではないため人間の lane に置く ([ADR-074](adr/adr-074-auto-lane-screening-criteria.md) 決定 2)。 diff --git a/src/hooks-post-tool-linter/src/custom_rules/coverage.rs b/src/hooks-post-tool-linter/src/custom_rules/coverage.rs index 5d76c3f2..fc900cd3 100644 --- a/src/hooks-post-tool-linter/src/custom_rules/coverage.rs +++ b/src/hooks-post-tool-linter/src/custom_rules/coverage.rs @@ -463,3 +463,178 @@ fn matching_declarations_yield_no_orphans() { let existing = declared.clone(); assert!(orphans_for_kind("bad", &declared, &existing).is_empty()); } + +/// 1 つの rule について、`extensions` に挙げた拡張子のうち `test_coverage` で +/// 網羅されていないものを列挙する (逆向き coverage 検査: extension → test)。 +/// +/// [`check_main_ext_coverage`] / [`check_other_ext_coverage`] は「主要拡張子を +/// 1 つも targets しない rule」でだけ `other_ext_tests` 非空を要求する +/// (`targets_main_empty` ゲート)。そのため、主要拡張子と非主要拡張子を**両方** +/// targets する rule (例: rule⑬ の `toml`/`yaml`/`yml` + `jsonc`) は、非主要拡張子側の +/// test が 1 件も無くても `rule_test_coverage_check` を素通りする。#402 で rule⑬ に +/// `json` を追加した際、実際にこのすり抜けが起きて CodeRabbit に指摘された。 +/// 本関数は `targets_main` の状態に関係なく、`extensions` に出現する拡張子ごとに +/// 個別に coverage を要求することでこの非対称を閉じる。 +#[cfg(test)] +fn extension_coverage_gaps(rule: &CustomRule) -> Vec { + let coverage = rule.test_coverage.clone().unwrap_or_default(); + let mut gaps: Vec = Vec::new(); + for ext in &rule.extensions { + let is_main = MAIN_EXTENSIONS.iter().any(|m| ext.eq_ignore_ascii_case(m)); + if is_main { + let has_test = coverage + .main_ext_tests + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case(ext.as_str())) + .is_some_and(|(_, v)| !v.is_empty()); + if !has_test { + gaps.push(format!( + "rule `{}` declares main extension `{}` in `extensions` but \ + `test_coverage.main_ext_tests.{}` has no test", + rule.id, ext, ext + )); + } + } else if coverage.other_ext_tests.is_empty() { + gaps.push(format!( + "rule `{}` declares non-main extension `{}` in `extensions` but \ + `test_coverage.other_ext_tests` is empty", + rule.id, ext + )); + } + } + gaps +} + +/// 逆向き coverage ゲート ([`rule_test_coverage_check`] を extension 起点で補完)。 +/// +/// allowlist は持たない: 現行 12 rule はいずれも `extensions` の全拡張子が +/// coverage 済みであり ([`rule_test_coverage_check`] が個別に main ext を検証、 +/// 本検査が非主要拡張子側の見逃しを塞ぐ)、免除が要る incident-derived でない +/// rule ([`NON_INCIDENT_RULES`] 相当のケース) も extension coverage の要件までは +/// 免除されない — 拡張子を宣言した以上、その拡張子に対する test は必須という +/// 単純な原則のため。 +#[cfg(test)] +#[test] +fn extension_test_coverage_check() { + let rules = load_deployed_custom_rules(); + let mut gaps: Vec = Vec::new(); + for rule in &rules { + gaps.extend(extension_coverage_gaps(rule)); + } + assert!( + gaps.is_empty(), + "extensions declared without test coverage ({} issue(s)):\n - {}", + gaps.len(), + gaps.join("\n - ") + ); +} + +#[cfg(test)] +fn rule_with_extensions_and_coverage( + id: &str, + extensions: &[&str], + coverage: CustomRuleTestCoverage, +) -> CustomRule { + CustomRule { + id: id.into(), + pattern: "x".into(), + severity: "warning".into(), + message: "test message".into(), + why: String::new(), + extensions: extensions.iter().map(|e| e.to_string()).collect(), + paths: None, + fix: None, + example: None, + test_coverage: Some(coverage), + incident: None, + } +} + +/// #402 で実際に起きたギャップの再現: 主要拡張子 (`toml`) と非主要拡張子 (`jsonc`) を +/// 両方 targets する rule で、`jsonc` 側の `other_ext_tests` が空だと検出されること。 +#[cfg(test)] +#[test] +fn extension_coverage_gaps_detects_missing_non_main_test_when_main_ext_present() { + let mut main_ext_tests = std::collections::BTreeMap::new(); + main_ext_tests.insert("toml".to_string(), vec!["some_toml_test".to_string()]); + let rule = rule_with_extensions_and_coverage( + "mixed-ext-rule", + &["toml", "jsonc"], + CustomRuleTestCoverage { + main_ext_tests, + other_ext_tests: Vec::new(), + }, + ); + let gaps = extension_coverage_gaps(&rule); + assert_eq!(gaps.len(), 1, "{gaps:?}"); + assert!(gaps[0].contains("jsonc"), "{gaps:?}"); +} + +/// 主要拡張子側の見逃しも (既存 `rule_test_coverage_check` と重複する形だが) 検出する。 +#[cfg(test)] +#[test] +fn extension_coverage_gaps_detects_missing_main_ext_test() { + let rule = rule_with_extensions_and_coverage( + "main-only-rule", + &["rs"], + CustomRuleTestCoverage::default(), + ); + let gaps = extension_coverage_gaps(&rule); + assert_eq!(gaps.len(), 1, "{gaps:?}"); + assert!(gaps[0].contains("rs"), "{gaps:?}"); +} + +/// 主要・非主要拡張子とも coverage が揃っていればギャップ 0 件 (正常系の固定)。 +#[cfg(test)] +#[test] +fn extension_coverage_gaps_empty_when_fully_covered() { + let mut main_ext_tests = std::collections::BTreeMap::new(); + main_ext_tests.insert("toml".to_string(), vec!["some_toml_test".to_string()]); + let rule = rule_with_extensions_and_coverage( + "fully-covered-rule", + &["toml", "jsonc"], + CustomRuleTestCoverage { + main_ext_tests, + other_ext_tests: vec!["some_jsonc_test".to_string()], + }, + ); + assert!(extension_coverage_gaps(&rule).is_empty()); +} + +/// **非主要拡張子は「rule あたり 1 件」で足りる** — これが現行の契約であることを固定する +/// (CodeRabbit #461)。 +/// +/// `main_ext_tests` は `BTreeMap<拡張子, Vec<テスト名>>` で拡張子ごとに持てるが、 +/// `other_ext_tests` は `Vec<テスト名>` で**拡張子との対応を持たない**。したがって +/// 「`jsonc` と `json` を宣言し `jsonc` 用テストだけ登録した」状態は、現行スキーマでは +/// 検出できない。これは本検査の実装漏れではなく**契約そのもの**である +/// (`.claude/custom-lint-rules.toml` の順位 137 由来コメント: 非主要拡張子は +/// 「rule あたり 1+ positive test」)。 +/// +/// 拡張子ごとの検証へ強化するには `other_ext_tests` を map 化するスキーマ移行が要り、 +/// 既存 rule の設定をすべて書き換えることになるため**順位 498 として別起票した**。 +/// 本テストは、その移行が入るまでの契約を明示し、意図せず緩んだ / 強まった場合に落とす。 +#[cfg(test)] +#[test] +fn non_main_extension_coverage_is_per_rule_not_per_extension() { + let rule = rule_with_extensions_and_coverage( + "two-non-main-extensions", + &["jsonc", "json"], + CustomRuleTestCoverage { + main_ext_tests: std::collections::BTreeMap::new(), + other_ext_tests: vec!["one_test_for_both".to_string()], + }, + ); + assert!( + extension_coverage_gaps(&rule).is_empty(), + "非主要拡張子 2 つに対しテスト 1 件は現行契約では充足 (順位 498 で強化予定)" + ); + + let uncovered = rule_with_extensions_and_coverage( + "two-non-main-extensions-uncovered", + &["jsonc", "json"], + CustomRuleTestCoverage::default(), + ); + let gaps = extension_coverage_gaps(&uncovered); + assert_eq!(gaps.len(), 2, "0 件なら拡張子ごとに不足を報告する: {gaps:?}"); +}