From 8149a6194b5c62ecdc7fa5b5481ab5bca96a2f46 Mon Sep 17 00:00:00 2001 From: aloekun Date: Thu, 2 Jul 2026 00:24:15 +0900 Subject: [PATCH 1/3] =?UTF-8?q?docs(todo):=20PR=20#232=20post-merge-feedba?= =?UTF-8?q?ck=20=E6=8E=A1=E7=94=A8=201=20=E4=BB=B6=E3=82=92=E7=99=BB?= =?UTF-8?q?=E9=8C=B2=20(=E9=A0=86=E4=BD=8D=20246)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/todo-summary.md | 1 + docs/todo13.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 95c15862..32cddf95 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -114,6 +114,7 @@ | 243 | 💎 Tier 3 | **`pub(crate)` vs `pub` 可視性チェックリストを module split 手順に追加 (PR #231 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (W-series module split で visibility scoping の判断が都度必要 = crate 内共有は `pub(crate)`、`pub` は同一 crate 内では有効だが library target 公開時のみ外部 API 化 (binary crate では pub(crate) と実質同等ゆえ pub(crate) 推奨) の違いを具体例付きで明示。Frequency Medium (file-length 強制継続で split 継続) + Effort XS + Adoption Risk None。順位 241 (pub(crate) 方針) と相補、追記先の file-length-enforcement-plan.md は W5 land 後削除予定のため coding-style.md / CLAUDE.md への恒久配置を着手時判断) | | 244 | 💎 Tier 3 | **per-module test helper 複製方針を coding-style.md に明文化 (PR #231 post-merge-feedback T3-2 採用)** | todo13.md | XS | なし (`unique_temp_root` / `write_meta` / `parked_state` 等の test helper を各 module に複製し shared util module を作らない方針が前提知識化しておらず split の度に混乱。coupling vs isolation トレードオフの根拠 + split レビュー確認項目を coding-style.md に追記。Frequency Medium + Effort XS + Adoption Risk None、memory `feedback_test_dry_antipattern` の恒久 codify) | | 245 | 💎 Tier 3 | **`PR_SIZE_CHECK_OVERRIDE=1` 適用ポリシーを push-runner-config.toml に明文化 (PR #231 post-merge-feedback T3-3 採用)** | todo13.md | XS | なし (override の使い方が「知っている人だけが知る」暗黙知化、機械的 refactor (削除≒追加) の定義と override 判断基準を push-runner-config.toml の `[pr_size_check]` コメントまたは docs に追記。file-length 強制継続で機械 refactor の override 判断は今後も発生 (Frequency Medium) + Effort XS + Adoption Risk None) | +| 246 | 🔧 Tier 2 | **monitor の CI 完了判定を短絡 — CodeRabbit review-complete + mergeability CLEAN で CI 待機を skip し merge-ready 判定 (PR #232 post-merge-feedback T2-1 採用)** | todo13.md | S | なし (CodeRabbit のみが check の構成 (GitHub Actions 等の実 CI 不在) で monitor が「CI: pending」を無限に誤報し、GitHub API 直接確認 (mergeStateStatus=CLEAN / mergeable=MERGEABLE) で merge 可能を確認する手動対応が PR #231/#232 で 2 回発生 = 幻の CI pending。docs-only PR の共通 pattern で再現見込み。poll ループに「review 完了 + mergeability CLEAN なら CI 待機を短絡」条件分岐を追加 (parse logic 改修不要)。Severity Medium + Frequency Medium + Effort S + Adoption Risk None) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 diff --git a/docs/todo13.md b/docs/todo13.md index 9e0967bd..053e058d 100644 --- a/docs/todo13.md +++ b/docs/todo13.md @@ -536,6 +536,34 @@ --- +### monitor の CI 完了判定を短絡 — CodeRabbit review-complete + mergeability CLEAN で CI 待機を skip (PR #232 post-merge-feedback T2-1 採用) + +> **動機**: 本リポジトリは check が CodeRabbit のみで GitHub Actions 等の実 CI が存在しない構成。この構成で cli-pr-monitor の poll が「CI: pending」を完了と判定できず recheck を上限まで繰り返す。PR #231 / #232 の両方で、GitHub API を直接確認 (`gh pr view --json mergeStateStatus,mergeable` → `CLEAN` / `MERGEABLE`) して merge 可能を人手で確認する必要が生じた (= 幻の CI pending)。 +> +> **本タスクの位置づけ**: PR #232 post-merge-feedback Tier 2 #1 採用 (Severity Medium / Frequency Medium / Effort S / Adoption Risk None)。docs-only PR で共通に再現する pattern。 +> +> **参照**: `.claude/feedback-reports/232.md` Tier 2 #1、`src/cli-pr-monitor/src/stages/poll/` (CI 完了判定 + poll ループ)、PR #231 / #232 (幻の CI pending を手動 GitHub API 確認で回避した実例)、ADR-018 (park モデル)。 +> +> **実行優先度**: 🔧 **Tier 2** — Effort S。既存 poll ループへの条件分岐追加のみ (parse logic 改修不要)。 + +#### 設計決定 (案) + +- CI 状態が「実 check 不在 or CodeRabbit のみ」かつ CodeRabbit review が完了 (unresolved 0 / actionable 0) かつ mergeability が `CLEAN` / `MERGEABLE` の場合、CI 待機 (pending) を短絡して merge-ready 判定に倒す。 +- 誤短絡防止: 実 CI check が 1 件でも存在し pending なら従来通り待機 (CodeRabbit-only 構成に限定)。 + +#### 作業計画 + +- [ ] poll の CI 完了判定に「review-complete + mergeability CLEAN」短絡条件を追加 +- [ ] CodeRabbit-only 構成の判定 (実 CI check の有無) を実装 +- [ ] `cargo test -p cli-pr-monitor` pass + regression test (短絡が誤発火しないこと) +- [ ] 本 entry 削除 + todo-summary.md 行削除 + +#### 完了基準 + +- CodeRabbit-only 構成の PR で review 完了 + CLEAN なら monitor が recheck を無駄に繰り返さず merge-ready と判定する。実 CI がある場合は従来の pending 待機を維持。 + +--- + ## 既知課題 (記録のみ、本セッションで未対応) (現時点で本ファイルへの既知課題は無し。docs/todo10.md / todo9.md 末尾を参照。) From c1d7f38b5e31a46bcc8dc3cb43ad6f9a9eaeb159 Mon Sep 17 00:00:00 2001 From: aloekun Date: Thu, 2 Jul 2026 19:20:00 +0900 Subject: [PATCH 2/3] =?UTF-8?q?feat(hooks):=20PR-W5=20file-length=20Stop?= =?UTF-8?q?=20gate=20=E2=80=94=20comment-lint=20--check-modified-files=20b?= =?UTF-8?q?atch=20mode=20+=20[file=5Flength=5Fgate]=20opt-in?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 (PR-W1〜W4、#220/#224/#230/#231) で 800 行以下に整えた clean state を恒久維持する 強制層。hooks-post-tool-comment-lint-rust に --check-modified-files batch mode を追加し、 Stop hook quality_gate の 1 step として PR 範囲 (base..@) の .rs file 行数を検査。 800 行超が 1 件でもあれば exit 1 で Stop を block する (Option C-2)。 実装: - src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs (新規、17 tests) - main.rs に --check-modified-files dispatch 追加、Cargo.toml に toml 依存追加 - .claude/hooks-config.toml に file-length step + [file_length_gate] section mechanical refactor、behavior 不変。既存 lint (comment/function/file_length/metrics) は不変。 ## ADR-039 3 点セット (experimental feature 標準パターン) - Config opt-in (default OFF): gate_enabled() が unwrap_or(false)。本 repo のみ dogfood で enabled=true - Kill-switch: 下表 - Bounded lifetime: file-length-enforcement-plan.md 削除条件 3 (override 未使用で 1-2 セッション通過) ## Kill-switch table | 起動経路 | 停止コマンド | 影響範囲 | |---|---|---| | .claude/hooks-config.toml の [file_length_gate] enabled=true + file-length step | enabled=false (恒久) | Stop hook の file-length 判定のみ (他 step 不変) | | Stop hook 発火時に file-length step 実行 | env FILE_LENGTH_CHECK_OVERRIDE=1 (緊急、truthy 値) | 当該 Stop の判定を skip | ## 設計判断 - jj 変更検出は base branch を config 引数化 (default master、ADR-021 § Revset Composability) - cmd path は cmd.exe の forward-slash 非対応のため backslash TOML literal string - jj 失敗時は fail-closed で block (ADR-043、stop_hook_active retry-skip が永続 lock を防止) - templates (TS/Python) は Rust 非対象のため未追加 ## 検証 - cargo test -p hooks-post-tool-comment-lint-rust: 116 pass / clippy clean / fmt clean - cargo test --workspace: regression なし / cargo clippy --workspace clean - dogfood (deploy 済 exe): clean=exit0 / 850行file=block / OVERRIDE(=1,=true)=bypass / enabled=false=no-op / self-host=exit0 Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/hooks-config.toml | 27 ++ Cargo.lock | 1 + docs/file-length-enforcement-plan.md | 44 ++- .../Cargo.toml | 1 + .../src/main.rs | 4 + .../src/modified_files_check.rs | 350 ++++++++++++++++++ 6 files changed, 424 insertions(+), 3 deletions(-) create mode 100644 src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs diff --git a/.claude/hooks-config.toml b/.claude/hooks-config.toml index 9e03f802..a788ef4f 100644 --- a/.claude/hooks-config.toml +++ b/.claude/hooks-config.toml @@ -182,6 +182,33 @@ cmd = "pnpm test:e2e" name = "build" cmd = "pnpm build" +# 順位 147 / PR-W5 (file-length-enforcement-plan.md): file-length Stop gate。 +# PR 範囲 (base branch..@) の .rs file が 800 行超なら Stop を block する強制層。 +# cmd は cmd.exe (`cmd /c`) 経由で実行されるため forward-slash (`./`) 始まりは不可。 +# backslash 相対パスを TOML literal string (single quote、escape 不要) で指定する。 +# gate の有効化は下記 [file_length_gate] enabled で制御 (この step があっても +# enabled = false なら exe は即 exit 0 で no-op)。 +[[stop_quality.steps]] +name = "file-length" +cmd = '.\.claude\hooks-post-tool-comment-lint-rust.exe --check-modified-files' + +# ─── PR-W5: file-length Stop gate 設定 (ADR-039 experimental feature 標準パターン) ─── +# +# hooks-post-tool-comment-lint-rust.exe --check-modified-files が参照する opt-in gate。 +# 上の [[stop_quality.steps]] "file-length" step から起動される。 +# +# ADR-039 3 点セット: +# - Config opt-in (default OFF): code default は enabled = false (unwrap_or(false))。 +# 本 repo は Phase 1 (PR-W1〜W4) の clean state を dogfood するため enabled = true。 +# - Kill-switch: 恒久停止は enabled = false。緊急バイパスは env +# FILE_LENGTH_CHECK_OVERRIDE=1 (truthy 値、順位 151 pr_size_check と同 pattern)。 +# - Bounded lifetime: 採否判定は file-length-enforcement-plan.md 削除条件 3 +# (override 未使用で 1-2 セッション通過) を trigger とする。 +# +# base: PR 範囲算出の base branch (省略時 "master"、ADR-021 § Revset Composability で引数化)。 +[file_length_gate] +enabled = true + # ─── Push Pipeline: push-runner-config.toml に移行済み (ADR-015) ─── # # pnpm push は cli-push-runner.exe (takt ベース) を使用。 diff --git a/Cargo.lock b/Cargo.lock index c408a0a2..627f2a1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -387,6 +387,7 @@ dependencies = [ "serde", "serde_json", "tempfile", + "toml", "tree-sitter", "tree-sitter-rust", ] diff --git a/docs/file-length-enforcement-plan.md b/docs/file-length-enforcement-plan.md index aedbafb3..003331d5 100644 --- a/docs/file-length-enforcement-plan.md +++ b/docs/file-length-enforcement-plan.md @@ -374,10 +374,10 @@ Agent 委譲。ADR-018 (cli-pr-monitor の takt 移行) を参照させる必要 ### PR-W5: Stop hook gate (C) 追加 -- **status**: not started +- **status**: 実装完了 (dogfood pass、PR 未作成) @5cfcfb5a - **owner**: - - **effort**: S -- **依存**: PR-W1 + W2 + W3 + W4 が **全て land 済** (clean state 必須、未 land 状態で C を入れると Stop が常に block) +- **依存**: PR-W1 + W2 + W3 + W4 が **全て land 済** (clean state 必須、未 land 状態で C を入れると Stop が常に block) — 4 件とも land 済 (#220/#224/#230/#231) #### スコープ @@ -406,6 +406,44 @@ Stop hook quality_gate に file_length check を追加。Phase 1 完了後の cl - ADR-039 § 4 self-review checklist (config schema / default OFF / docs / kill-switch test の 4 点) を満たす - dogfood: 意図的に 800 行超 file を作って block されることを確認 + override で通過することを確認 +#### 実績 (実装完了、PR 未作成) + +実装構成: + +| 対象 | 内容 | +|---|---| +| `src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs` (新規、~345 行) | `--check-modified-files` batch mode 本体 + 17 tests | +| `src/hooks-post-tool-comment-lint-rust/src/main.rs` | dispatch 追加 (`--metrics` の隣) + `mod modified_files_check` | +| `src/hooks-post-tool-comment-lint-rust/Cargo.toml` | `toml = "0.8"` 依存追加 (config parse 用) | +| `.claude/hooks-config.toml` | `[[stop_quality.steps]]` "file-length" step + `[file_length_gate] enabled = true` section | + +設計上の判断: + +- **jj 変更検出**: `jj diff -r '..@' --name-only` で PR 範囲 (working copy 含む) の `.rs` を取得。`base` は config 引数化 (default `master`、ADR-021 § Revset Composability — custom lint `NO_HARDCODED_JJ_REVSET_RANGE` の指摘に対応) +- **cmd path**: `[[stop_quality.steps]]` は `cmd /c` 経由で実行されるため forward-slash (`./`) 始まりは cmd.exe で `'.' is not recognized` エラー。backslash 相対パス (`.\.claude\...exe`) を TOML literal string で指定 (実測で判明) +- **fail-closed (ADR-043)**: jj 失敗時は判定不能として exit 1 (block 側)。`stop_hook_active` retry-skip (ADR-004) が永続 lock を防ぐ +- **templates**: `templates/hooks-config-{typescript,python}.toml` は Rust 非対象 (comment-lint-rust は Rust 限定) のため本 gate を追加せず + +ADR-039 § 4 self-review checklist (4 点) 充足: + +1. **config schema**: `FileLengthGateConfig { enabled: Option, base: Option }` +2. **default OFF**: `gate_enabled()` が `unwrap_or(false)` (code default)。本 repo のみ dogfood で `enabled = true` +3. **docs / config example**: `.claude/hooks-config.toml` の `[file_length_gate]` section に 3 点セット + kill-switch を comment 明記 (plan doc 削除後も永続、ADR-039 § "明示的 decision trigger の必須化") +4. **kill-switch test**: unit test `gate_disabled_when_enabled_false` + dogfood で `enabled = false` 実機 no-op を確認 + +dogfood 結果 (deploy 済 exe で実機確認): + +| シナリオ | 結果 | +|---|---| +| clean state (enabled=true、超過なし) | exit 0 ✅ | +| 850 行 file を作成 | exit 1 + 該当 file 列挙で block ✅ | +| `FILE_LENGTH_CHECK_OVERRIDE=1` | exit 0 + 実受理値表示で bypass ✅ | +| `FILE_LENGTH_CHECK_OVERRIDE=true` (truthy variant) | exit 0 + `=true` 表示で bypass ✅ (ADR-039 § 2) | +| `enabled = false` (超過 file 残存) | exit 0 + 無出力の no-op ✅ | +| 本 PR 自身の変更に対する self-host | exit 0 (self-consistent) ✅ | + +削除条件 3 (`FILE_LENGTH_CHECK_OVERRIDE=1` 未使用で 1-2 セッション通過) の観測は **land 後** に開始する。 + #### 進め方 batch mode 実装 (~50 行) + tests (~30 行) + config schema 更新。Agent 委譲不要、直接実装可能な規模。 @@ -445,7 +483,7 @@ PR-W1 [x] #220 (merged at 2026-06-24T18:04:56Z) PR-W2 [x] #224 (merged at 2026-06-28T13:15:36Z) PR-W3 [x] #230 (merged at 2026-07-01T05:27:34Z) PR-W4 [x] #231 (merged at 2026-07-01T08:42:27Z) -PR-W5 [ ] not started +PR-W5 [~] 実装完了 (dogfood pass、PR 未作成) @5cfcfb5a ``` land 後は `[x]` + PR 番号を記入し、最終的に 6 件全て `[x]` で本 file を削除。 diff --git a/src/hooks-post-tool-comment-lint-rust/Cargo.toml b/src/hooks-post-tool-comment-lint-rust/Cargo.toml index 2eb836e1..e689ba04 100644 --- a/src/hooks-post-tool-comment-lint-rust/Cargo.toml +++ b/src/hooks-post-tool-comment-lint-rust/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [dependencies] serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +toml = "0.8" tree-sitter = "0.22" tree-sitter-rust = "0.21" diff --git a/src/hooks-post-tool-comment-lint-rust/src/main.rs b/src/hooks-post-tool-comment-lint-rust/src/main.rs index 62387dde..a6d7cc2e 100644 --- a/src/hooks-post-tool-comment-lint-rust/src/main.rs +++ b/src/hooks-post-tool-comment-lint-rust/src/main.rs @@ -26,6 +26,7 @@ mod file_length; mod function_length; mod line_filter; mod metrics; +mod modified_files_check; mod violations; use line_filter::{ @@ -105,6 +106,9 @@ fn main() { if args.len() >= 3 && args[1] == "--metrics" { std::process::exit(run_metrics_mode(&args[2])); } + if args.len() >= 2 && args[1] == "--check-modified-files" { + std::process::exit(modified_files_check::run_check_modified_files()); + } let mut input = String::new(); if io::stdin().read_to_string(&mut input).is_err() { diff --git a/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs b/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs new file mode 100644 index 00000000..1f385074 --- /dev/null +++ b/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs @@ -0,0 +1,350 @@ +//! `--check-modified-files` batch mode (PR-W5 Stop hook gate、Option C-2)。 +//! +//! Phase 1 (PR-W1〜W4) で 800 行以下に整えた clean state を恒久維持するための +//! 強制層。Stop hook `[stop_quality.steps]` の 1 step として起動され、PR 範囲 +//! (base branch から working copy まで) で変更された `.rs` file の行数を検査し、 +//! 800 行超が 1 件でもあれば exit 1 で session 終了を block する。 +//! +//! PostToolUse の [`crate::file_length`] soft-nag (additionalContext のみ、block しない) +//! と異なり、本 batch mode は Stop を block する **強制 gate** である。 +//! +//! # ADR-039 3 点セット (experimental feature 標準パターン) +//! +//! - **Config opt-in (default OFF)**: `[file_length_gate]` section の `enabled = true` +//! のときのみ検査を実行。section 不在 / `enabled = false` は完全 skip +//! ([`gate_enabled`] が `unwrap_or(false)`)。本 gate は Stop を block するため +//! § 1.b mechanical lint 例外 (default ON) には該当せず、§ 1 opt-in を適用する。 +//! - **Kill-switch**: 緊急バイパスは env `FILE_LENGTH_CHECK_OVERRIDE` (truthy 値で skip、 +//! 順位 151 `pr_size_check` と同 pattern)。恒久停止は `enabled = false`。 +//! - **Bounded lifetime**: 採否判定基準は `docs/file-length-enforcement-plan.md` +//! 削除条件 3 (override 未使用で 1-2 セッション通過) を trigger とする。 +//! +//! # Fail-closed (ADR-043) +//! +//! jj による変更検出が失敗 (jj 起動失敗 / revset 解決失敗) した場合、判定不能として +//! block 側にデフォルトする (exit 1)。ADR-043 § 原則 1 は hooks-stop-quality を +//! fail-closed 適用対象として明示している。`stop_hook_active` retry-skip +//! (ADR-004) が永続 lock を防ぐため、fail-closed でも session が詰まることはない。 + +use crate::file_length::{count_source_lines, MAX_FILE_LINES}; +use crate::line_filter::is_rust_file; +use serde::Deserialize; +use std::path::PathBuf; +use std::process::Command; + +/// 緊急バイパス用 env var (kill-switch)。truthy 値で検査を skip する。 +const OVERRIDE_ENV_VAR: &str = "FILE_LENGTH_CHECK_OVERRIDE"; + +/// PR 範囲を求める diff の default base branch。`base` config 未指定時に使用する。 +/// ADR-021 § Revset Composability: revset range は引数化し、alternative branch +/// (`main` 等) でも silent breakage しないようにする。 +const DEFAULT_BASE_BRANCH: &str = "master"; + +/// `hooks-config.toml` のうち本 gate が参照する section のみ部分デシリアライズ。 +#[derive(Deserialize, Default)] +struct GateConfigFile { + file_length_gate: Option, +} + +/// `[file_length_gate]` section。ADR-039 § 1 opt-in: `enabled` default OFF。 +#[derive(Deserialize, Default)] +struct FileLengthGateConfig { + enabled: Option, + base: Option, +} + +/// `--check-modified-files` mode の entry point。process exit code を返す。 +/// +/// 0 = 通過 (disabled / override / 違反なし)、1 = block (違反あり / jj 失敗)。 +pub(crate) fn run_check_modified_files() -> i32 { + let config = load_gate_config(); + if !gate_enabled(&config) { + return 0; + } + if let Some(raw) = override_value() { + println!( + "[file-length-gate] {}={} を検出、検査を skip します (意図的バイパス)", + OVERRIDE_ENV_VAR, raw + ); + return 0; + } + let base = effective_base(&config); + let files = match list_changed_rust_files(&base) { + Ok(files) => files, + Err(e) => { + println!( + "[file-length-gate] jj による変更 file 検出に失敗しました (fail-closed / ADR-043): {}\n\ + jj repo 状態を確認するか、緊急時は {}=1 で bypass してください。", + e, OVERRIDE_ENV_VAR + ); + return 1; + } + }; + let violations = collect_oversize_files(&files); + if violations.is_empty() { + return 0; + } + print!("{}", format_violation_report(&base, &violations)); + 1 +} + +/// `enabled = Some(true)` のときのみ true。section 不在 / `None` / `Some(false)` は +/// すべて false (ADR-039 § 1 default OFF)。 +fn gate_enabled(config: &GateConfigFile) -> bool { + config + .file_length_gate + .as_ref() + .and_then(|g| g.enabled) + .unwrap_or(false) +} + +/// `base` config を解決する。未指定 / 空文字なら [`DEFAULT_BASE_BRANCH`]。 +fn effective_base(config: &GateConfigFile) -> String { + config + .file_length_gate + .as_ref() + .and_then(|g| g.base.as_deref()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .unwrap_or(DEFAULT_BASE_BRANCH) + .to_string() +} + +/// `FILE_LENGTH_CHECK_OVERRIDE` が truthy であればその生値を返す。 +/// +/// ADR-039 § 2: kill-switch 診断メッセージは実受理値を反映する。生値を返し呼び出し側で +/// `"{}={} を検出"` と表示することで、`1` / `true` / `on` 等どの値で bypass したかを +/// user が確認できる。 +fn override_value() -> Option { + let raw = std::env::var(OVERRIDE_ENV_VAR).ok()?; + is_truthy(&raw).then_some(raw) +} + +/// override env の受理値判定 (順位 151 `pr_size_check::parse_override_env` と同 pattern)。 +fn is_truthy(value: &str) -> bool { + matches!( + value.trim().to_ascii_lowercase().as_str(), + "1" | "true" | "yes" | "on" + ) +} + +/// exe と同じ directory の `hooks-config.toml` を読み込む (hooks-stop-quality と同方式)。 +/// 読み込み / parse 失敗時は default (= gate disabled) を返す。 +fn load_gate_config() -> GateConfigFile { + let Ok(content) = std::fs::read_to_string(config_path()) else { + return GateConfigFile::default(); + }; + toml::from_str(&content).unwrap_or_default() +} + +/// `hooks-config.toml` のパス解決。current_exe の親 directory を優先し、 +/// 取得不能時は cwd 相対にフォールバック。 +fn config_path() -> PathBuf { + std::env::current_exe() + .ok() + .and_then(|exe| exe.parent().map(|dir| dir.join("hooks-config.toml"))) + .unwrap_or_else(|| PathBuf::from("hooks-config.toml")) +} + +/// `jj diff -r '..@' --name-only` で PR 範囲の変更 file を取得し `.rs` のみ返す。 +/// +/// working copy (@) の変更も含む。jj 起動失敗 / 非 0 exit は `Err` (fail-closed 側で処理)。 +fn list_changed_rust_files(base: &str) -> Result, String> { + let revset = format!("{}..@", base); + let output = Command::new("jj") + .args(["diff", "-r", &revset, "--name-only"]) + .output() + .map_err(|e| format!("jj 起動失敗: {}", e))?; + if !output.status.success() { + return Err(String::from_utf8_lossy(&output.stderr).trim().to_string()); + } + Ok(parse_changed_rust_files(&String::from_utf8_lossy( + &output.stdout, + ))) +} + +/// `jj diff --name-only` の stdout から `.rs` file path のみ抽出する (pure)。 +fn parse_changed_rust_files(stdout: &str) -> Vec { + stdout + .lines() + .map(str::trim) + .filter(|line| !line.is_empty() && is_rust_file(line)) + .map(str::to_string) + .collect() +} + +/// 各 file の行数を数え、`MAX_FILE_LINES` 超を `(path, line_count)` で列挙する。 +/// 読み取り不能 (削除済 file 等) は skip する。 +fn collect_oversize_files(files: &[String]) -> Vec<(String, usize)> { + files + .iter() + .filter_map(|path| { + let source = std::fs::read_to_string(path).ok()?; + let lines = count_source_lines(&source); + (lines > MAX_FILE_LINES).then_some((path.clone(), lines)) + }) + .collect() +} + +/// block 時に stdout へ出力する診断メッセージを組み立てる (pure)。 +/// +/// Stop hook (`run_cmd_shell_capped`) は stdout+stderr を捕捉して block reason に埋め込む +/// ため、対処法と override hint を含める。 +fn format_violation_report(base: &str, violations: &[(String, usize)]) -> String { + let mut out = format!( + "[file-length-gate] PR 範囲 ({}..@) に {} 行超の Rust file が {} 件あります (順位 147 / PR-W5 Stop gate):\n", + base, + MAX_FILE_LINES, + violations.len() + ); + for (path, lines) in violations { + out.push_str(&format!( + " - {} ({} 行 > {} 行)\n", + path, lines, MAX_FILE_LINES + )); + } + out.push_str(&format!( + "対処: file を責務ごとに module 分割してください。\ + mechanical refactor 等で一時的に超過するのが意図的なら {}=1 で bypass 可能です。\n", + OVERRIDE_ENV_VAR + )); + out +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + #[test] + fn gate_disabled_when_section_absent() { + let config: GateConfigFile = toml::from_str("").unwrap(); + assert!(!gate_enabled(&config)); + } + + #[test] + fn gate_disabled_when_enabled_false() { + let config: GateConfigFile = + toml::from_str("[file_length_gate]\nenabled = false\n").unwrap(); + assert!(!gate_enabled(&config)); + } + + #[test] + fn gate_disabled_when_enabled_omitted() { + let config: GateConfigFile = toml::from_str("[file_length_gate]\n").unwrap(); + assert!(!gate_enabled(&config)); + } + + #[test] + fn gate_enabled_only_when_enabled_true() { + let config: GateConfigFile = + toml::from_str("[file_length_gate]\nenabled = true\n").unwrap(); + assert!(gate_enabled(&config)); + } + + #[test] + fn effective_base_defaults_to_master() { + let config: GateConfigFile = + toml::from_str("[file_length_gate]\nenabled = true\n").unwrap(); + assert_eq!(effective_base(&config), "master"); + } + + #[test] + fn effective_base_honors_configured_branch() { + let config: GateConfigFile = + toml::from_str("[file_length_gate]\nenabled = true\nbase = \"main\"\n").unwrap(); + assert_eq!(effective_base(&config), "main"); + } + + #[test] + fn effective_base_falls_back_when_blank() { + let config: GateConfigFile = + toml::from_str("[file_length_gate]\nenabled = true\nbase = \" \"\n").unwrap(); + assert_eq!(effective_base(&config), "master"); + } + + #[test] + fn is_truthy_accepts_documented_values() { + for v in ["1", "true", "TRUE", "True", "yes", "on", " on "] { + assert!(is_truthy(v), "{:?} should be truthy", v); + } + } + + #[test] + fn is_truthy_rejects_falsey_values() { + for v in ["0", "false", "no", "off", "", " ", "2", "enable"] { + assert!(!is_truthy(v), "{:?} should be falsey", v); + } + } + + #[test] + fn parse_changed_rust_files_filters_non_rust_and_blanks() { + let stdout = "src/a.rs\ndocs/readme.md\n\nsrc/nested/b.rs\nCargo.toml\n"; + assert_eq!( + parse_changed_rust_files(stdout), + vec!["src/a.rs".to_string(), "src/nested/b.rs".to_string()] + ); + } + + #[test] + fn parse_changed_rust_files_trims_surrounding_whitespace() { + assert_eq!( + parse_changed_rust_files(" src/a.rs \n"), + vec!["src/a.rs".to_string()] + ); + } + + #[test] + fn parse_changed_rust_files_empty_when_no_rust() { + assert!(parse_changed_rust_files("README.md\npackage.json\n").is_empty()); + } + + fn write_rs_file(dir: &std::path::Path, name: &str, line_count: usize) -> String { + let path = dir.join(name); + let mut f = std::fs::File::create(&path).unwrap(); + for i in 0..line_count { + writeln!(f, "let _x{} = {};", i, i).unwrap(); + } + path.to_string_lossy().to_string() + } + + #[test] + fn collect_oversize_files_flags_over_threshold_only() { + let dir = tempfile::tempdir().unwrap(); + let big = write_rs_file(dir.path(), "big.rs", MAX_FILE_LINES + 5); + let small = write_rs_file(dir.path(), "small.rs", 10); + let violations = collect_oversize_files(&[big.clone(), small]); + assert_eq!(violations.len(), 1); + assert_eq!(violations[0].0, big); + assert_eq!(violations[0].1, MAX_FILE_LINES + 5); + } + + #[test] + fn collect_oversize_files_at_threshold_is_ok() { + let dir = tempfile::tempdir().unwrap(); + let exact = write_rs_file(dir.path(), "exact.rs", MAX_FILE_LINES); + assert!(collect_oversize_files(&[exact]).is_empty()); + } + + #[test] + fn collect_oversize_files_skips_missing_file() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("gone.rs").to_string_lossy().to_string(); + assert!(collect_oversize_files(&[missing]).is_empty()); + } + + #[test] + fn format_violation_report_lists_files_and_override_hint() { + let violations = vec![ + ("src/big.rs".to_string(), 950), + ("src/huge.rs".to_string(), 1200), + ]; + let report = format_violation_report("master", &violations); + assert!(report.contains("src/big.rs")); + assert!(report.contains("950")); + assert!(report.contains("src/huge.rs")); + assert!(report.contains("1200")); + assert!(report.contains(OVERRIDE_ENV_VAR)); + assert!(report.contains("2 件")); + } +} From 5cc827dce04ff347f303eed059858461528dd9e2 Mon Sep 17 00:00:00 2001 From: aloekun Date: Thu, 2 Jul 2026 20:07:18 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix(review):=20CodeRabbit=20#234-1=20fail-c?= =?UTF-8?q?losed=20=E5=AF=BE=E5=BF=9C=20+=20=E5=89=8A=E9=99=A4=20file=20sk?= =?UTF-8?q?ip=20=E8=A3=9C=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit Major #234-1 (読み取り不能な既存 .rs は fail-closed に) を適用: collect_oversize_files を Result 化し、存在するのに読めない .rs は Err → exit 1 (block)。 さらに削除 file 誤検知を補正: jj diff --name-only は削除 file も列挙する (実測確認) ため、 Path::exists() で存在確認し削除 file (非存在) は skip する。これがないと file split refactor (元 file 削除を伴う、本 plan が促進する作業そのもの) を gate が誤 block する。 CodeRabbit の指摘文言も「*既存* .rs」であり削除 file は対象外。 - collect_oversize_files: filter(Path::exists) + Result - fail-closed 診断を run_check_modified_files で block 表示 (ADR-043 § 原則1) - tests: skips_deleted_file (skip) + errors_on_present_but_unreadable (Err) で両分岐を assert Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/modified_files_check.rs | 60 +++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs b/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs index 1f385074..09d957ee 100644 --- a/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs +++ b/src/hooks-post-tool-comment-lint-rust/src/modified_files_check.rs @@ -29,7 +29,7 @@ use crate::file_length::{count_source_lines, MAX_FILE_LINES}; use crate::line_filter::is_rust_file; use serde::Deserialize; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; /// 緊急バイパス用 env var (kill-switch)。truthy 値で検査を skip する。 @@ -69,18 +69,17 @@ pub(crate) fn run_check_modified_files() -> i32 { return 0; } let base = effective_base(&config); - let files = match list_changed_rust_files(&base) { - Ok(files) => files, + let violations = match find_violations(&base) { + Ok(v) => v, Err(e) => { println!( - "[file-length-gate] jj による変更 file 検出に失敗しました (fail-closed / ADR-043): {}\n\ - jj repo 状態を確認するか、緊急時は {}=1 で bypass してください。", + "[file-length-gate] {} (fail-closed / ADR-043)\n\ + 緊急時は {}=1 で bypass してください。", e, OVERRIDE_ENV_VAR ); return 1; } }; - let violations = collect_oversize_files(&files); if violations.is_empty() { return 0; } @@ -173,17 +172,31 @@ fn parse_changed_rust_files(stdout: &str) -> Vec { .collect() } +/// 変更 file を検出しサイズ違反を収集する。jj 失敗 / file 読み取り失敗は `Err` (fail-closed)。 +fn find_violations(base: &str) -> Result, String> { + let files = list_changed_rust_files(base)?; + collect_oversize_files(&files) +} + /// 各 file の行数を数え、`MAX_FILE_LINES` 超を `(path, line_count)` で列挙する。 -/// 読み取り不能 (削除済 file 等) は skip する。 -fn collect_oversize_files(files: &[String]) -> Vec<(String, usize)> { +/// +/// 削除された file は検査対象外 (skip): `jj diff --name-only` は削除 file も列挙するが、 +/// file split refactor で元 file が消えるのは正常であり、存在しない path を block すると +/// 本 plan が促進する分割作業自体を誤 block してしまう。存在するのに読み取り不能な file +/// のみ fail-closed で `Err` を返す (ADR-043 § 原則1 / CodeRabbit #234-1 は「読み取り不能な +/// *既存* `.rs`」を対象と明記)。 +fn collect_oversize_files(files: &[String]) -> Result, String> { files .iter() - .filter_map(|path| { - let source = std::fs::read_to_string(path).ok()?; + .filter(|path| Path::new(path).exists()) + .map(|path| { + let source = std::fs::read_to_string(path) + .map_err(|e| format!("既存 .rs の読み取り失敗 {}: {}", path, e))?; let lines = count_source_lines(&source); - (lines > MAX_FILE_LINES).then_some((path.clone(), lines)) + Ok((lines > MAX_FILE_LINES).then_some((path.clone(), lines))) }) - .collect() + .collect::, _>>() + .map(|v| v.into_iter().flatten().collect()) } /// block 時に stdout へ出力する診断メッセージを組み立てる (pure)。 @@ -313,7 +326,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let big = write_rs_file(dir.path(), "big.rs", MAX_FILE_LINES + 5); let small = write_rs_file(dir.path(), "small.rs", 10); - let violations = collect_oversize_files(&[big.clone(), small]); + let violations = collect_oversize_files(&[big.clone(), small]).unwrap(); assert_eq!(violations.len(), 1); assert_eq!(violations[0].0, big); assert_eq!(violations[0].1, MAX_FILE_LINES + 5); @@ -323,14 +336,27 @@ mod tests { fn collect_oversize_files_at_threshold_is_ok() { let dir = tempfile::tempdir().unwrap(); let exact = write_rs_file(dir.path(), "exact.rs", MAX_FILE_LINES); - assert!(collect_oversize_files(&[exact]).is_empty()); + assert!(collect_oversize_files(&[exact]).unwrap().is_empty()); + } + + #[test] + fn collect_oversize_files_skips_deleted_file() { + let dir = tempfile::tempdir().unwrap(); + let deleted = dir.path().join("gone.rs").to_string_lossy().to_string(); + assert!( + collect_oversize_files(&[deleted]).unwrap().is_empty(), + "削除 file (非存在) は検査対象外 = skip (file split refactor を誤 block しない)" + ); } #[test] - fn collect_oversize_files_skips_missing_file() { + fn collect_oversize_files_errors_on_present_but_unreadable() { let dir = tempfile::tempdir().unwrap(); - let missing = dir.path().join("gone.rs").to_string_lossy().to_string(); - assert!(collect_oversize_files(&[missing]).is_empty()); + let unreadable = dir.path().to_string_lossy().to_string(); + assert!( + collect_oversize_files(&[unreadable]).is_err(), + "存在するのに読み取り不能 (directory 等) は fail-closed で Err (ADR-043 / CodeRabbit #234-1)" + ); } #[test]