Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@

## 開発 convention / チェックリスト

- [開発 convention / チェックリスト](docs/dev-conventions.md) — spike 見送り (negative result) 永続化 convention (順位261)、外部 SaaS 無料枠 / 制限の調査チェックリスト (順位262)
- [開発 convention / チェックリスト](docs/dev-conventions.md) — spike 見送り (negative result) 永続化 convention (順位261)、外部 SaaS 無料枠 / 制限の調査チェックリスト (順位262)、外部 fixture 参照テストは値まで assert (順位274)

## Build

Expand Down
10 changes: 10 additions & 0 deletions docs/dev-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,13 @@ integration test で外部バイナリを spawn する場合、**無期限 wait
2. **出力捕捉との両立** — 出力が必要なら stdout/stderr を `lib-subprocess::drain_pipe_unlimited` で別スレッド drain してから timeout wait する (pipe バッファ充填による deadlock 回避)。

**由来** (PR #254 / WP-08、[ADR-049](adr/adr-049-incident-eval-regression-suite.md)): codebase 初の exe-spawn E2E テスト (`incident_eval.rs`) パターンを確立したが timeout 境界が欠落し CodeRabbit nitpick。WP-16 CI smoke test 等で同パターン流用が見込まれるため convention 化する。

## 外部 fixture 参照テストは値まで assert (順位274)

テストが外部ファイル (実 config / 共有 fixture 等) を fixture として参照する場合、「section / キーの存在」だけでなく **テストの前提とする具体値まで assert** する:

1. **存在チェックだけでは silent break する** — 「section がある」だけを assert すると、外部ファイル側で値が変わってもテストは緑のまま、前提の乖離が別テストの原因の見えない失敗として遅れて表面化する (ADR-041 Test Isolation の該当パターン)。
2. **値ずれ時に更新箇所を指し示す** — assert メッセージに「この値を変えたらどのテストの期待値を更新すべきか」を明記し、外部ファイル側の変更が即座に「値まで assert したテスト」の失敗として表面化するようにする。
3. **lint ではなく convention** — fixture ごとにスキーマが異なり regex での自動検知は非現実的なため、機械 lint 化せず convention として運用する (ADR-042 の役割分担)。

**由来** (PR #261 T3-#2、[ADR-041](adr/adr-041-test-isolation-patterns.md)): `hooks-stop-tool-call-leak` の E2E (`tests/e2e.rs`) が実 config を隣にコピーする際、`[stop_tool_call_leak]` section の存在しか assert しておらず、`enabled = true` / `max_consecutive_blocks = 3` の値変更が cap 境界テスト (`consecutive_leaks_at_cap_fail_open` 等) を原因の見えない形で silent break させるリスクを CodeRabbit / session / pre-push simplicity の 3 ソースが独立指摘した。順位 273 で実例側 (値まで assert) を修正し、本 convention でパターンを一般化した。
3 changes: 0 additions & 3 deletions docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@
| 256 | ⏳ Tier 5 | **classifier FP 検出強化プロンプトで格上げ候補を再評価 (WP-04 見送りの follow-up、ADR-038 amendment 由来)** | todo13.md | M | なし (WP-04 実測で全候補が FP 検出未達 = 能力限界か `classify.txt` の mistral 向け tune 不適合かが未分離。FP 検出強化プロンプト版で qwen3-coder:30b 等を再測し、能力限界と確認できれば恒久見送り、プロンプト不適合なら該当モデル + 専用プロンプトで格上げ。eval 手法・gold セットは scratchpad WP-04 資産を再利用。materially better な新モデル出現時も再評価トリガー) |
| 257 | ⏳ Tier 5 | **push pipeline の `cargo test` を cargo-nextest 化 (WP-05 で Stop hook には無効と判明、push 側 follow-up)** | todo13.md | S-M | なし (WP-05 実測: Stop hook は cargo test 不在で nextest 非適用、真因は逐次実行→並列化で解決済。ただし push pipeline (cli-push-runner quality_gate) の `cargo test -- --ignored` は実測 ~80s で nextest 高速化の余地あり。ツール依存追加 = ADR-017 pinning + 派生プロジェクト配布のコスト、push が Stop より低頻度な点を踏まえた費用対効果を評価。doctest は nextest 非実行のため `cargo test --doc` 併走が必要) |
| 264 | 🔧 Tier 2 | **pre-push review-diff.txt の生成形式を `jj diff --git` に切替 — LLM レビュアーの add/delete 誤読解消 (PR #256 post-merge-feedback Tier1 #1 採用)** | todo13.md | S | なし (`push-runner-config.toml:113` の `[diff] command = "jj diff -r @"` は色+行番号2列形式で、色を落とした review-diff.txt では削除が `-` マーカー無しになり LLM レビュアーが「追加」と誤読。PR #256 で todo 25行削除を simplicity-review が false positive REJECT し ~19分浪費。`jj diff --git -r @` へ切替で解消、`templates/push-runner-config.toml:52` も同 PR で修正必須 (deploy:hooks 配布)、Adoption Risk None、memory `prepush-review-diff-plain-format-misread.md`) |
| 271 | 🚀 Tier 1 | **hooks-stop-tool-call-leak `emit_block` serialize 失敗時の stderr 診断追加 (PR #261 post-merge-feedback T1-#1 採用)** | todo13.md | XS | なし (fail-open 設計の observability 1 行修正。順位 273 と同 PR 可) |
| 272 | 🚀 Tier 1 | **cli-docs-lint に ADR 重複採番 + CLAUDE.md 索引整合チェック追加 (PR #261 post-merge-feedback T1-#2 採用)** | todo13.md | S | なし (PR #261 で ADR-052/053 採番衝突が実発生、既存 cli-docs-lint の check-mode 骨格流用。順位 135 placeholder policy は todo entry 側の「ルール」で本 entry は land 済ファイルの「仕組み」検知、相補で重複ではない) |
| 273 | 🔧 Tier 2 | **hooks-stop-tool-call-leak E2E の config 具体値 assert (PR #261 post-merge-feedback T2-#1 採用)** | todo13.md | S | なし (CodeRabbit / session / pre-push simplicity の 3 ソース独立指摘、ADR-041 パターン。順位 271 と同 PR 可、順位 274 と同時 land 推奨) |
| 274 | 💎 Tier 3 | **dev-conventions に「外部 fixture 参照テストは値まで assert」convention 追記 (PR #261 post-merge-feedback T3-#2 採用)** | todo13.md | S | なし (順位 273 の一般化、ADR-042 準拠で lint でなく convention 化。順位 273 と同 PR 推奨) |

**戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。

Expand Down
58 changes: 0 additions & 58 deletions docs/todo13.md
Original file line number Diff line number Diff line change
Expand Up @@ -805,25 +805,6 @@

---

### hooks-stop-tool-call-leak `emit_block` serialize 失敗時の stderr 診断追加 (PR #261 post-merge-feedback T1-#1 採用)

> **動機**: `emit_block` が `serde_json::to_string` 失敗時に何も出力せず終了し、block 判定の失敗が完全に不可視になる (fail-open の停止許可という挙動は変わらないが observability がゼロ)。CodeRabbit 指摘。
>
> **参照**: `.claude/feedback-reports/261.md` Tier 1 #1、`src/hooks-stop-tool-call-leak/src/main.rs` の `emit_block` (Err arm 不在)、ADR-053 (fail-open 設計 — stderr 明示が設計上の可視性担保手段)。
>
> **実行優先度**: 🚀 **Tier 1** — Effort XS。1 行修正 (Err arm に `eprintln!` 追加)。順位 273 と同 PR で処理可。

#### 作業計画

- [ ] `emit_block` の `serde_json::to_string` を match 化し、Err で `[stop-tool-call-leak]` prefix の stderr 診断を出力
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- block JSON serialize 失敗時に stderr へ診断が出て、fail-open の発生が transcript / hook ログから追跡可能になっていること。

---

### cli-docs-lint に ADR 重複採番 + CLAUDE.md 索引整合チェック追加 (PR #261 post-merge-feedback T1-#2 採用)

> **動機**: PR #261 で当方が ADR-052 として起草した ADR が、並行 land した PR #260 の ADR-052 (自律実行境界) と採番衝突し、rebase 時にファイル名 + 本文タイトル + ソース内参照 10+ 箇所の置換が発生した実例。ADR は既に 53 件、並行 PR 開発が常態化しており再発頻度 Medium。現状この衝突を機械検知する層が存在しない (発見は rebase 時の CLAUDE.md conflict 頼み)。
Expand All @@ -850,45 +831,6 @@

---

### hooks-stop-tool-call-leak E2E の config 具体値 assert (PR #261 post-merge-feedback T2-#1 採用)

> **動機**: `tests/e2e.rs` は実 config (`.claude/hooks-config.toml`) を exe の隣にコピーするが、`[stop_tool_call_leak]` section の存在しか assert しておらず、`enabled = true` / `max_consecutive_blocks = 3` の具体値を検証していない。config retuning や kill-switch flip (`enabled = false`) が入ると、上限 3 前提のテスト (`consecutive_leaks_at_cap_fail_open` 等) が原因の見えない形で silent break する (ADR-041 該当パターン)。CodeRabbit / session / pre-push simplicity の **3 ソース独立指摘**。
>
> **参照**: `.claude/feedback-reports/261.md` Tier 2 #1、`src/hooks-stop-tool-call-leak/tests/e2e.rs` (`ensure_config_beside_exe` と cap 境界テスト)、ADR-041 (Test Isolation Patterns)。
>
> **実行優先度**: 🔧 **Tier 2** — Effort S。既存テストへの assert 追加のみ。順位 271 と同 PR で処理可、順位 274 の convention 追記と同時 land 推奨。

#### 作業計画

- [ ] `ensure_config_beside_exe` で config を parse し `enabled == true` / `max_consecutive_blocks == 3` を assert (値が変わったらテスト側の期待値も同時更新を強制する明示メッセージ付き)
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- config の値変更が E2E テストの明示的な assert 失敗として即座に表面化し、cap 境界テストの前提と config の乖離が silent に進行しない構造になっていること。

---

### dev-conventions に「外部 fixture 参照テストは値まで assert」convention 追記 (PR #261 post-merge-feedback T3-#2 採用)

> **動機**: 順位 273 の一般化。テストが外部ファイル (実 config 等) を fixture として参照する場合、「section / キーの存在」だけでなく「テストの前提とする具体値」まで assert しないと、外部ファイル側の変更でテストが silent break する (ADR-041 パターンの convention 化)。fixture ごとにスキーマが異なり regex での自動検知は非現実的なため、lint ではなく convention として文書化する (ADR-042 準拠、feedback report の fact-check 済み判断)。
>
> **参照**: `.claude/feedback-reports/261.md` Tier 3 #2、`docs/dev-conventions.md` (追記先)、ADR-041、順位 273 (実例側の修正)。
>
> **実行優先度**: 💎 **Tier 3** — Effort S。順位 273 と同 PR での land を推奨 (実例修正 + convention 化をセットで)。

#### 作業計画

- [ ] `docs/dev-conventions.md` に本 convention を追記 (順位 273 の before/after を実例として引用)
- [ ] CLAUDE.md の dev-conventions 索引行に項目名を追加
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- 外部 fixture 参照テストを書く際の値 assert 要件が dev-conventions で参照可能になり、順位 273 型の指摘が review 前に自己防止できること。

---

## 既知課題 (記録のみ、本セッションで未対応)

(現時点で本ファイルへの既知課題は無し。docs/todo10.md / todo9.md 末尾を参照。)
4 changes: 3 additions & 1 deletion src/hooks-stop-tool-call-leak/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ toml = "0.8"

[dev-dependencies]
tempfile = "3"
# serde_json / lib-subprocess: exe-spawn E2E test (tests/e2e.rs) 用。
# serde_json / toml / lib-subprocess: exe-spawn E2E test (tests/e2e.rs) 用。
# integration tests は [dev-dependencies] のみ参照するため再宣言する
# (hooks-post-tool-linter の incident_eval と同 pattern)。
# toml は実 config の具体値 assert (ADR-041、順位273) 用。
serde_json = "1.0"
toml = "0.8"
lib-subprocess = { path = "../lib-subprocess" }

# [profile.release] は workspace root (Cargo.toml) に集約 (ADR-026)
8 changes: 6 additions & 2 deletions src/hooks-stop-tool-call-leak/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,12 @@ fn emit_block(reason: &str) {
decision: "block".to_string(),
reason: reason.to_string(),
};
if let Ok(json) = serde_json::to_string(&decision) {
println!("{}", json);
match serde_json::to_string(&decision) {
Ok(json) => println!("{}", json),
Err(e) => eprintln!(
"[stop-tool-call-leak] block 判定の JSON serialize 失敗 (fail-open): {}",
e
),
}
}

Expand Down
32 changes: 28 additions & 4 deletions src/hooks-stop-tool-call-leak/tests/e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ fn ensure_config_beside_exe() {
let src = repo_root().join(".claude").join("hooks-config.toml");
let content = std::fs::read_to_string(&src)
.unwrap_or_else(|e| panic!("repo hooks-config.toml read failed: {e}"));
assert!(
content.contains("[stop_tool_call_leak]"),
"repo config に [stop_tool_call_leak] section が必要 (false-green guard)"
);
assert_leak_config_matches_test_assumptions(&content);
let dst = exe_path()
.parent()
.expect("exe has a parent dir")
Expand All @@ -52,6 +49,33 @@ fn ensure_config_beside_exe() {
});
}

/// E2E fixture (実 config) が本テスト群の前提とする具体値と一致することを検証する。
/// section 存在だけでなく `enabled` / `max_consecutive_blocks` の値まで assert し、
/// config retuning や kill-switch flip (`enabled = false`) が cap 境界テストを原因の
/// 見えない形で silent break させるのを防ぐ (ADR-041、dev-conventions.md § 外部 fixture
/// 参照テストは値まで assert)。値を変えたら assert メッセージが更新箇所を指し示す。
fn assert_leak_config_matches_test_assumptions(content: &str) {
let config: toml::Value = toml::from_str(content)
.unwrap_or_else(|e| panic!("repo hooks-config.toml parse failed: {e}"));
let leak = config.get("stop_tool_call_leak").unwrap_or_else(|| {
panic!("repo config に [stop_tool_call_leak] section が必要 (false-green guard)")
});
assert_eq!(
leak.get("enabled").and_then(toml::Value::as_bool),
Some(true),
"E2E は [stop_tool_call_leak] enabled = true を前提とする。config で無効化するなら \
本テスト群 (leak_transcript_blocks_with_reason 等) の期待値も同時に更新すること"
);
assert_eq!(
leak.get("max_consecutive_blocks")
.and_then(toml::Value::as_integer),
Some(3),
"E2E は max_consecutive_blocks = 3 を前提とする。値を変えたら cap 境界テスト \
(consecutive_leaks_at_cap_fail_open / second_consecutive_leak_still_blocks) の \
leak 件数も同時に更新すること"
);
}

fn assistant_text_entry(text: &str) -> Value {
json!({"type": "assistant", "message": {"content": [{"type": "text", "text": text}]}})
}
Expand Down