Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
40 changes: 37 additions & 3 deletions docs/adr/adr-055-firing-telemetry-collection.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ warm-up 後に実データで棚卸し (step 2/3) を後続 PR で行う。本 A

`decision` は「hook がツールを実際に停止したか」ではなく「発火の重み」を表す軸である。
custom rule / jj-op-verify は additionalContext の助言層で実際には block しないが、severity
に応じて block/warn を記録する。逆に stop-quality は infra エラー (stdin/parse 失敗) の
fail-closed 経路でも block を emit するため、「hook が block を emit した総数」として記録
する。file-length gate の fail-closed 経路 (jj 失敗の判定不能 block) は ROI 信号を汚さない
に応じて block/warn を記録する。stop-quality は当初 infra エラー (stdin/parse 失敗) の
fail-closed 経路でも block を emit するため「hook が block を emit した総数」として記録して
いたが、後述の Amendment (2026-07-29) でこの定義を撤回し、実 quality 違反 (品質ステップ
失敗) の block のみ記録するよう限定した (WP-12 ROI 信号から infra ノイズを除外)。
file-length gate の fail-closed 経路 (jj 失敗の判定不能 block) は当初から ROI 信号を汚さない
よう記録しない。

### 副作用注入によるテスト可能性
Expand Down Expand Up @@ -239,6 +241,38 @@ stop-feedback-dispatch / user-prompt-feedback-recovery は本 PR では計装し
これらにも当てはまるが、計装は各 hook を触る PR で個別に行う (ADR-059 段階展開に連動)。
§計装スコープ の除外リストは本 amendment に合わせて更新した。

## Amendment (2026-07-29): block 記録を実 quality 違反に限定 (順位309、WP-12 ROI 信号の精度)

初版 § 計装スコープ は stop-quality の block 記録を「hook が block を emit した総数」と
定義し、infra エラー (stdin 読込 / JSON parse 失敗) の fail-closed 経路も計上していた。
WP-12 step 2/3 (発火数で hook の維持・削除を判断する ROI 棚卸し) の観点では、この infra
エラー混入が発火数を歪め「実際に品質違反を捕捉した回数」と乖離する。CodeRabbit Major 指摘
(順位309) を受け、**block 記録を実 quality 違反 (品質ステップ失敗) パス限定に絞り込む**。

### 3 hook の状態 (計装スコープ表のうち block を emit する hook)

| hook | 修正前 | 修正後 |
|---|---|---|
| hooks-stop-quality | stdin/parse 失敗の fail-closed block でも記録 | `record_block_firing` を `emit_block` から実 quality 違反経路 (`block_on_failures`) へ移動。infra エラー経路 (`read_stdin_or_block` / `parse_hook_input_or_block`) は block decision を emit するが telemetry には記録しない |
| hooks-stop-tool-call-leak | (変更なし) | `emit_block` は `run_check` の実 leak 検出時のみ呼ばれ、transcript 読取失敗・連続数 0・上限到達は fail-open で return するため既に実 leak 限定。ADR-061 の回収層 (`prompt-recovery`) は `should_recover` 成立時のみ warn 記録 |
| hooks-pre-tool-validate | (変更なし) | `record_preset_block` は `validate_command` が hit を返す (= preset にマッチした実 violation) 経路のみ。stdin/parse 失敗は `ExitCode::FAILURE` で return し記録しない。既に preset match 限定 |

### 設計: 経路を型で区別しテストで固定

stop-quality に `BlockCause { QualityViolation, InfraError }` を導入し、`emit_block(reason, cause)`
が `cause.records_firing()` (QualityViolation のみ真) のときだけ telemetry に記録する。
telemetry 記録を closure 注入した `emit_block_with` をテストの芯とし、「QualityViolation は
recorder を発火 / InfraError は発火しない」を副作用で観測して回帰ガードにする。leak / preset
は record 呼び出しが既に violation 経路にしか存在しないため、コード構造でこの不変条件が保たれる
(追加のテストは設けない)。
Comment on lines +254 to +267

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

ADR-055 の実装説明を最終コードに合わせてください。

表は record_block_firing を block_on_failures へ移動したと記載していますが、実装は emit_block_with 内で BlockCause::records_firing() を判定し、emit_block 経由で記録します。後段の設計記述とも不一致なので、「cause に応じて emit_block が記録する」と修正してください。

As per path instructions: ADR の適用内容は実装契約と一致させる必要があります。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/adr/adr-055-firing-telemetry-collection.md` around lines 254 - 267,
ADR-055の表と設計説明を、実装どおり`emit_block`経由で`BlockCause::records_firing()`を判定し、`QualityViolation`の場合のみ`emit_block_with`でtelemetryを記録する内容へ更新してください。`record_block_firing`を`block_on_failures`へ移動したという記述を削除し、`InfraError`では記録しない契約を明記してください。

Source: Path instructions


### 帰結

- 「発火 0 = 削除候補」の ROI 信号が infra エラーの fail-closed block で汚染されなくなり、
発火数は「hook が実際に品質違反を捕捉した回数」を表す。
- fail-open 原則は不変。telemetry 記録の有無に関わらず block decision 自体は emit するため、
infra エラー時も Claude への block 通知は従来どおり行われ、ゲート挙動は変わらない。

## 関連 ADR

- [ADR-039](adr-039-experimental-feature-standard-pattern.md) — 試験運用標準パターン (opt-in / kill-switch / bounded lifetime)
Expand Down
1 change: 0 additions & 1 deletion docs/todo-summary2.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
| 306 | 💎 Tier 3 | **quality gate isolation 機構を見送り、recovery による risk acceptance とした判断の記録 (negative result) (273.md T3-5 採用)** | todo16.md | S | なし (spike 見送り convention に従い、isolation 機構を却下し recovery コストの低さ (順位304) を理由に risk acceptance した根拠を記録。recovery は isolation の代替ではなく、予防機能の欠如という残存リスクと再検討条件を明記する) |
| 307 | 🔧 Tier 2 | **WP-12 step 2: 発火テレメトリ ROI 棚卸し pre-step (発火 0 の rule/preset/hook を削除候補提示)** | todo16.md | M | なし (**着手条件 = ADR-055 収集層マージから 28 日 warm-up 後**。それ以前は全項目が発火 0 = データ無しで判定無意味。集計は Rust exe、weekly-review に file-length-watchlist 同型 facet で接続、incident 由来ルールは発火 0 でも維持推奨の区別) |
| 308 | 💎 Tier 3 | **WP-12 step 3: ADR-039 bounded lifetime 判定の発火数機械化** | todo16.md | S | 順位 307 (step 2 の集計基盤に依存)。試験運用 ADR 機構の卒業/廃止検討を発火数で自動 promote。step 3 完了で WP-12 完了 |
| 309 | 🚀 Tier 1 | **telemetry の block 記録を実 quality 違反に限定(infra エラー混入除外)(275.md T1-1 採用)** | todo16.md | M | なし (CodeRabbit Major。ADR-055 で「emit 総数」と意図的定義したが WP-12 ROI 棚卸しが infra エラー〔stdin/parse 失敗〕混入で歪むため実 violation パス限定に絞る。3 hook 横断で分割 PR 推奨、ADR-055 amendment 併記) |
| 310 | 🚀 Tier 1 | **custom-regex preset の生 regex が telemetry id に流れる privacy footgun 是正(非ブロッキング follow-up 統合)(275.md T1-2 採用)** | todo16.md | S | なし (現行 config は named preset のみで非発火だが派生プロジェクトの latent footgun。fallback を合成 id〔"custom-block"〕に正規化 + ADR-055 に config privacy 注記) |
| 311 | 🚀 Tier 1 | **逐語的関数複製(3+ コピー)を pre-push 検出する DRY lint rule (275.md T1-3 採用)** | todo16.md | M | なし (is_truthy 三重複製事案。ADR-007 regex 層に threshold 検出追加。順位 313 の fixture と抱き合わせ) |
| 312 | 🔧 Tier 2 | **`.claude/telemetry/` の per-pid×日次 partition ファイル retention/cleanup (275.md T2-1 採用)** | todo16.md | M | 順位 307 (WP-12 step 2 と同時期=step1 マージから 28 日後 2026-08-12 頃に着手) |
Expand Down
23 changes: 0 additions & 23 deletions docs/todo16.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,29 +272,6 @@

---

### telemetry の block 記録を実 quality 違反に限定(infra エラー混入の除外)(275.md T1-1 採用)

> **動機**: CodeRabbit Major 指摘。`emit_block` / `record_*_firing` が品質違反だけでなく fail-closed の infra エラー(stdin 読込失敗 / JSON parse 失敗)でも発火を記録する。ADR-055 では「hook が block を emit した総数」として意図的にこの設計にしたが、WP-12 の ROI 棚卸し(発火数で hook 維持を判断)では infra エラー混入が発火数を歪めるため、実 quality 違反パス(`block_on_failures` 等)限定に絞り込む方が信号が正確になる。
>
> **重要**: これは ADR-055 で「意図的」と記録した判断の見直しであり、実装時は ADR-055 の該当記述(emit 総数の定義)も併せて amendment する。3 hook 横断(hooks-stop-quality / hooks-stop-tool-call-leak / hooks-pre-tool-validate)のため実装は分割 PR 推奨。stop-tool-call-leak は実 leak でのみ emit_block を呼ぶため既に実質限定されている点も確認する。
>
> **参照**: `.claude/feedback-reports/275.md` Tier 1 #1、`src/hooks-stop-quality/src/main.rs`(`emit_block` / `record_block_firing`)、[ADR-055](adr/adr-055-firing-telemetry-collection.md) § 計装スコープ、WP-12 step 2(順位 307、集計精度の前提)。
>
> **実行優先度**: 🚀 Tier 1 — Severity Medium / Effort M。

#### 作業計画

- [ ] 各 hook の記録呼び出しを実 quality 違反パス限定に移動(infra エラー経路では記録しない)。record 位置の見直し。
- [ ] [ADR-055](adr/adr-055-firing-telemetry-collection.md) の「emit 総数」定義を amendment(実 violation 限定に方針変更した根拠を記録)。
- [ ] 各 hook のユニットテストで「infra エラー経路では telemetry を記録しない」ことを検証。
- [ ] 本エントリ削除 + todo-summary2.md 行削除。

#### 完了基準

- telemetry の block 記録が実 quality 違反に限定され、infra エラー(stdin/parse 失敗)では記録されないことがテストで保証され、ADR-055 の定義も整合していること。

---

### custom-regex preset の生 regex が telemetry id に流れる privacy footgun の是正(非ブロッキング follow-up 統合)(275.md T1-2 採用)

> **動機**: PR #275 の pre-push simplicity review 非ブロッキング warning(= セッション中に検出された「非ブロッキング follow-up」)。`tag_source(name, ...)` の `name` が named preset 名でなく `blocked_patterns` の生正規表現文字列の場合、その regex テキストがそのまま telemetry の `id` フィールドに載り、ADR-055 の「コマンド本文・内容は非記録」プライバシー原則と緊張する。現行 `hooks-config.toml` は named preset のみのため**非発火**だが、派生プロジェクトが raw-regex エントリを足すと該当する latent footgun。
Expand Down
82 changes: 68 additions & 14 deletions src/hooks-stop-quality/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,37 @@ struct QualityStepConfig {
/// デフォルトのステップタイムアウト(秒)
const DEFAULT_STEP_TIMEOUT_SECS: u64 = 60;

/// block 判定を stdout に出力するヘルパー
fn emit_block(reason: &str) {
record_block_firing();
/// block 判定の発火源。telemetry 記録対象 (実 quality 違反) か、infra エラー
/// (stdin 読込 / JSON parse 失敗の fail-closed block) かを区別する。
///
/// 順位309 / ADR-055 § 計装スコープ amendment: WP-12 ROI 棚卸し (発火数で hook 維持を
/// 判断する) の信号を歪めないため、infra エラーの fail-closed block は telemetry に
/// 記録しない。実 quality 違反 (品質ステップ失敗) のみを「hook が発火した」として計上する。
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
enum BlockCause {
QualityViolation,
InfraError,
}

impl BlockCause {
/// この block を telemetry の firing として記録すべきか。実 quality 違反のみ true。
fn records_firing(self) -> bool {
matches!(self, BlockCause::QualityViolation)
}
}

/// block 判定を stdout に出力するヘルパー。実 quality 違反 (`QualityViolation`) のみ
/// telemetry に firing を記録する (`InfraError` は記録しない、順位309)。
fn emit_block(reason: &str, cause: BlockCause) {
emit_block_with(reason, cause, record_block_firing);
}

/// [`emit_block`] のテスト可能な芯。telemetry 記録を closure で注入し、`cause` による
/// 記録有無をテストが副作用で観測できるようにする (prod は [`record_block_firing`])。
fn emit_block_with(reason: &str, cause: BlockCause, record_firing: impl FnOnce()) {
if cause.records_firing() {
record_firing();
}
let decision = BlockDecision {
decision: "block".to_string(),
reason: reason.to_string(),
Expand All @@ -84,8 +112,9 @@ fn emit_block(reason: &str) {
}
}

/// Stop 品質ゲートが block を発火したこと (品質失敗・fail-closed infra エラーを含む
/// emit 総数) を telemetry に記録する (WP-12、fail-open)。
/// Stop 品質ゲートが実 quality 違反で block を発火したことを telemetry に記録する
/// (WP-12、fail-open)。infra エラー (stdin/parse 失敗) の fail-closed block はこの経路を
/// 通さず記録しない (順位309、ADR-055 § 計装スコープ amendment)。
fn record_block_firing() {
lib_telemetry::record(&lib_telemetry::Firing {
hook: "hooks-stop-quality",
Expand Down Expand Up @@ -254,10 +283,10 @@ fn pipeline_is_running() -> bool {
fn read_stdin_or_block() -> Option<String> {
let mut input = String::new();
if let Err(e) = io::stdin().read_to_string(&mut input) {
emit_block(&format!(
"品質ゲートエラー: stdin読み込みに失敗しました: {}",
e
));
emit_block(
&format!("品質ゲートエラー: stdin読み込みに失敗しました: {}", e),
BlockCause::InfraError,
);
return None;
}
Some(input)
Expand All @@ -268,10 +297,10 @@ fn parse_hook_input_or_block(input: &str) -> Option<HookInput> {
match serde_json::from_str(input) {
Ok(v) => Some(v),
Err(e) => {
emit_block(&format!(
"品質ゲートエラー: 入力JSONのパースに失敗しました: {}",
e
));
emit_block(
&format!("品質ゲートエラー: 入力JSONのパースに失敗しました: {}", e),
BlockCause::InfraError,
);
None
}
}
Expand Down Expand Up @@ -389,7 +418,7 @@ fn block_on_failures(failures: &[String]) {
"品質ゲートが失敗しました。以下の問題を修正してください:\n\n{}",
failures.join("\n\n")
);
emit_block(&reason);
emit_block(&reason, BlockCause::QualityViolation);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

worker thread の panic を QualityViolation として記録しないでください。

run_quality_steps は worker thread の panic も failures に追加するため、ここで常に QualityViolation を渡すと、品質違反がない内部障害まで telemetry に記録されます。品質失敗とインフラ失敗を分離し、品質失敗がある場合だけ QualityViolation、panic のみなら InfraError としてください。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks-stop-quality/src/main.rs` at line 421, run_quality_steps
の失敗分類を更新し、品質失敗が存在する場合のみ emit_block に BlockCause::QualityViolation
を渡してください。worker thread の panic だけで品質失敗がない場合は BlockCause::InfraError として記録し、既存の
failures 処理で品質失敗と内部障害を分離してください。

}

#[cfg(test)]
Expand Down Expand Up @@ -511,6 +540,31 @@ cmd = "pnpm test"
assert!(json.contains(r#""reason":"test failed""#));
}

/// 順位309: 実 quality 違反 (品質ステップ失敗) の block は telemetry recorder を発火する。
#[test]
fn quality_violation_block_invokes_the_telemetry_recorder() {
let mut recorded = false;
emit_block_with("品質ゲート失敗", BlockCause::QualityViolation, || {
recorded = true
});
assert!(recorded, "実 quality 違反の block は firing を記録する");
}

/// 順位309 / ADR-055 § 計装スコープ amendment: infra エラー (stdin/parse 失敗) の
/// fail-closed block は ROI 信号 (発火数で hook 維持を判断) を歪めるため telemetry
/// recorder を発火しない。record 位置を実 violation 経路限定に移した回帰ガード。
#[test]
fn infra_error_block_does_not_invoke_the_telemetry_recorder() {
let mut recorded = false;
emit_block_with("stdin 読み込み失敗", BlockCause::InfraError, || {
recorded = true
});
assert!(
!recorded,
"infra エラーの fail-closed block は firing を記録しない (順位309)"
);
}

#[test]
fn step_timeout_default_is_reasonable() {
const { assert!(DEFAULT_STEP_TIMEOUT_SECS >= 30) };
Expand Down
Loading