feat(hooks-post-tool-linter): WP-08 incident→eval 回帰スイート — 由来 incident 再現 fixture + 実 exe E2E + fixture 必須 fail-closed gate (ADR-049) - #254
Merged
Conversation
…t 再現 fixture + 実 exe E2E + fixture 必須 fail-closed gate (ADR-049)
📝 WalkthroughWalkthroughADR-049に基づき、 ChangesIncident→Eval 回帰スイート
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as incident_eval.rs
participant Linter as hooks-post-tool-linter
participant Fixture as incident fixture
Test->>Test: ensure_rules_toml_beside_exe
Test->>Fixture: stage(workflow_rel)
Test->>Linter: run_linter(PostToolUse JSON stdin)
Linter-->>Test: stdout JSON
Test->>Test: parse_custom_lint_violations
Test->>Test: assert_bad_fixture_fires / assert_good_fixture_clean
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks-post-tool-linter/tests/incident_eval.rs (1)
111-131: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win子プロセス待機にタイムアウトがない
child.wait_with_output()にタイムアウトが設定されておらず、被テストバイナリが何らかの理由でハングした場合、このテストひいてはCIジョブが無期限にブロックされる可能性があります。♻️ ウォッチドッグスレッドによるタイムアウト案(概念例)
+use std::sync::mpsc; +use std::time::Duration; + fn run_linter(cwd: &Path, invoke_path: &str) -> Vec<serde_json::Value> { let payload = serde_json::json!({ "tool_input": { "file_path": invoke_path } }).to_string(); let mut child = Command::new(exe_path()) .current_dir(cwd) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() .expect("spawn hooks-post-tool-linter"); child .stdin .take() .expect("child stdin") .write_all(payload.as_bytes()) .expect("write stdin payload"); - let out = child.wait_with_output().expect("wait for linter exe"); + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(child.wait_with_output()); + }); + let out = rx + .recv_timeout(Duration::from_secs(30)) + .expect("linter exe timed out") + .expect("wait for linter exe"); let stdout = String::from_utf8_lossy(&out.stdout); parse_custom_lint_violations(&stdout) }🤖 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-post-tool-linter/tests/incident_eval.rs` around lines 111 - 131, The test helper run_linter currently waits on the spawned hooks-post-tool-linter process with child.wait_with_output() without any timeout, so add bounded waiting around the child execution to prevent hangs; use the existing run_linter and child setup in incident_eval.rs to locate it, and implement a timeout-based wait strategy (for example via a watchdog thread or equivalent) that kills or aborts the child if it exceeds a reasonable limit before parsing stdout.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@src/hooks-post-tool-linter/tests/incident_eval.rs`:
- Around line 111-131: The test helper run_linter currently waits on the spawned
hooks-post-tool-linter process with child.wait_with_output() without any
timeout, so add bounded waiting around the child execution to prevent hangs; use
the existing run_linter and child setup in incident_eval.rs to locate it, and
implement a timeout-based wait strategy (for example via a watchdog thread or
equivalent) that kills or aborts the child if it exceeds a reasonable limit
before parsing stdout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abb2867f-8700-411a-b50c-7adaadc8bbf6
📒 Files selected for processing (35)
.claude/custom-lint-rules.toml.markdownlint-cli2.jsoncCLAUDE.mddocs/adr/adr-049-incident-eval-regression-suite.mddocs/harness-improvement-plan.mdsrc/hooks-post-tool-linter/Cargo.tomlsrc/hooks-post-tool-linter/src/custom_rules/coverage.rssrc/hooks-post-tool-linter/src/custom_rules/deployed_tests.rssrc/hooks-post-tool-linter/src/custom_rules/engine_tests.rssrc/hooks-post-tool-linter/src/custom_rules/rule_tests.rssrc/hooks-post-tool-linter/src/custom_rules/rule_tests_extras.rssrc/hooks-post-tool-linter/src/custom_rules/types.rssrc/hooks-post-tool-linter/tests/incident_eval.rstests/fixtures/incidents/bad/no-docs-relative-back-to-docs.mdtests/fixtures/incidents/bad/no-empty-powershell-catch.ps1tests/fixtures/incidents/bad/no-ephemeral-todo-reference.rstests/fixtures/incidents/bad/no-hardcoded-jj-revset-range.rstests/fixtures/incidents/bad/no-jj-template-first-line.tomltests/fixtures/incidents/bad/no-mutable-anchor.mdtests/fixtures/incidents/bad/no-personal-paths.mdtests/fixtures/incidents/bad/no-silent-error-action.ps1tests/fixtures/incidents/bad/no-time-field-strict-greater.rstests/fixtures/incidents/bad/no-write-result-discard.rstests/fixtures/incidents/bad/takt-workflow-persona-without-model.yamltests/fixtures/incidents/good/no-docs-relative-back-to-docs.mdtests/fixtures/incidents/good/no-empty-powershell-catch.ps1tests/fixtures/incidents/good/no-ephemeral-todo-reference.rstests/fixtures/incidents/good/no-hardcoded-jj-revset-range.rstests/fixtures/incidents/good/no-jj-template-first-line.tomltests/fixtures/incidents/good/no-mutable-anchor.mdtests/fixtures/incidents/good/no-personal-paths.mdtests/fixtures/incidents/good/no-silent-error-action.ps1tests/fixtures/incidents/good/no-time-field-strict-greater.rstests/fixtures/incidents/good/no-write-result-discard.rstests/fixtures/incidents/good/takt-workflow-persona-without-model.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
ハーネス改善実行計画書 (
docs/harness-improvement-plan.md) の WP-08: incident→eval 回帰スイート を実装する。目的は「ハーネス自体の退行」の機械検出。カスタムリントルールの多くは実 incident (過去 PR の事故) 由来だが、ルールがその incident を今も検出できるかを機械検証する仕組みが無かった。設計は ADR-049 に集約。標準手法 (回帰テスト + linter fixture corpus) の応用に provenance ポリシー (各ルールが由来 incident と再現 fixture を機械可読に持つ) を重ねた構成。
調査で判明した前提の修正
no-console-logは汎用サンプルで incident 由来でない。ADR-042 の「11 本」と整合）。計画の「12 本全て由来」は要調整だった。severity付き JSON、exit 0）。block/warn の assert は「stdout JSON を parse して severity を検査」。変更内容
provenance の構造化
.claude/custom-lint-rules.tomlの incident 由来 11 ルールに[rules.incident](pr / bad_fixture / good_fixture / adr) を追加。CustomRuleIncidentstruct として parse。fixtures (
tests/fixtures/incidents/{bad,good}/, 22 個)Hook E2E test (
tests/incident_eval.rs)CARGO_BIN_EXEで spawn しPostToolUseJSON を stdin→stdout parse（内部関数呼びでなく arg/stdin パース〜feedback〜exit の全経路を通す）。.takt/workflows/に stage し path filter も検証。coverage gate (
incident_fixture_coverage_check)[rules.incident]+ bad/good fixture 実在を fail-closed で強制（ADR-043）。rule① はNON_INCIDENT_RULESallowlist で免除。fixture の隔離（ハーネス運用を壊さない）
src/**の外（deployed_tests.rsclean-baseline に触れない）・markdownlint ignore・.rsは//!doc ヘッダ（comment-lint 回避）。カスタムリンター自体が非致命のため運用は不変。検証
cargo test -p hooks-post-tool-linter: 146 unit + E2E(1) + coverage gate、全 okincident_eval_all_incident_rules ... ok（bad 11 全発火・good 11 全 clean を実 exe で確認）incident_fixture_coverage_check ... ok（fail-closed ゲート）cargo clippy --all-targets -- -D warnings: 0 warningslint:docsOK /lint:md0 errors関連
Summary by CodeRabbit
New Features
Bug Fixes