From 44b06d42b9def17f1130238abb11b27ab177244c Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 1 Jul 2026 14:52:08 +0900 Subject: [PATCH 1/4] =?UTF-8?q?docs(plan):=20file-length-enforcement-plan?= =?UTF-8?q?=20PR-W3=20=E3=82=92=20land=20=E6=B8=88=20(#230)=20=E3=81=AB?= =?UTF-8?q?=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/file-length-enforcement-plan.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/file-length-enforcement-plan.md b/docs/file-length-enforcement-plan.md index 70f0f2c5..3ea30a4b 100644 --- a/docs/file-length-enforcement-plan.md +++ b/docs/file-length-enforcement-plan.md @@ -329,7 +329,7 @@ Agent 委譲。ADR-018 (cli-pr-monitor の takt 移行) を参照させる必要 ### PR-W3: cli-merge-pipeline 分割 (2 file、計 2322 行) -- **status**: [in progress] @c392417a (refactor + local 検証 pass、push 済) +- **status**: [x] land 済 (#230, merged 2026-07-01T05:27:34Z) - **owner**: - - **effort**: M - **依存**: PR-W1 / W2 と並列可 @@ -443,7 +443,7 @@ batch mode 実装 (~50 行) + tests (~30 行) + config schema 更新。Agent 委 PR-W0 [x] #219 (merged at 2026-06-24T16:07:42Z) PR-W1 [x] #220 (merged at 2026-06-24T18:04:56Z) PR-W2 [ ] not started -PR-W3 [~] in progress @c392417a (refactor 完了・push 済、PR land 待ち) +PR-W3 [x] #230 (merged at 2026-07-01T05:27:34Z) PR-W4 [ ] not started PR-W5 [ ] not started ``` From 08f3f4cf76ec47247a760e89b86fd323ec9143d4 Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 1 Jul 2026 14:52:30 +0900 Subject: [PATCH 2/4] =?UTF-8?q?docs(todo):=20PR=20#230=20post-merge-feedba?= =?UTF-8?q?ck=20=E6=8E=A1=E7=94=A8=204=20=E4=BB=B6=E3=82=92=E7=99=BB?= =?UTF-8?q?=E9=8C=B2=20(=E9=A0=86=E4=BD=8D=20238-241)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/todo-summary.md | 4 ++ docs/todo13.md | 99 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 27bf1ad6..d5a4be04 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -106,6 +106,10 @@ | 235 | 💎 Tier 3 | **ADR-022 に Serialization Primitive Single-Instance Rule の Appendix 追加 (PR #227 post-merge-feedback T3-2 採用)** | todo13.md | S | なし (PR #224 T2-2 共有 env_override_lock helper 抽出 + PR #227 で同根の serialization primitive 単一化問題 2 PR 観測 = Frequency Medium。`OnceLock>` 等を複製すると各々独立した Mutex になり競合排除が破壊される特殊ケースを ADR-022 Appendix で明文化、通常 helper 複製推奨 (DRY) との例外境界を codify。ADR-046 独立化 (feedback T3-3) との overlap は着手時判断、順位 234 と相補) | | 236 | 🚀 Tier 1 | **tempfile mandate + PID+ms 命名 block の custom lint (PR #229 post-merge-feedback T1-1 採用)** | todo13.md | S | なし (#227 で修正した temp file collision flaky の再導入防止。#229 で本 flaky が push pipeline の `cargo test` を 3 回ブロックした実害。custom lint で `tempfile::Builder` / `NamedTempFile` を mandate + `gh-pr-body-{PID}-{ms}` 形式の手動命名を block。順位 237 = 検出層と二層防御) | | 237 | 🔧 Tier 2 | **create_pr flaky の高並列 regression test (PR #229 post-merge-feedback T2-1 採用)** | todo13.md | M | なし (#227 flaky fix の再導入検出網。`body_with_literal_newline_converted` を per-test `tempfile::tempdir()` + 高並列 concurrent run で回し collision を恒常 trap。順位 236 = 予防層と二層防御) | +| 238 | 🚀 Tier 1 | **`Command::new("gh")` 直叩き禁止 + timeout wrapper 必須の custom lint (PR #230 post-merge-feedback T1-#1 採用)** | todo13.md | M | なし (fetch_pr_time_range / fetch_pr_diff_summary / run_gh_logged / delete_remote_branch の 4 箇所が `Command::new("gh").output()` 同期実行でネットワーク不調時に無期限ハング = ADR-016 違反、CodeRabbit Major #2/#3。custom lint で直叩きを検出し `run_cmd_shell_capped_reporting` 相当の timeout wrapper を促す。extensions=["rs"] 限定で false positive 軽減。Severity High + Frequency High + Effort M。順位 240 と同 crate、bundle 検討可) | +| 239 | 🔧 Tier 2 | **`filter_transcripts` の複数 jsonl 走査を timestamp ソートで deterministic 化 + regression test (PR #230 post-merge-feedback T2-#1 採用)** | todo13.md | M | なし (`fs::read_dir` の非決定順により複数 Claude セッション時にファイル間時系列順が保証されず downstream takt workflow の context 品質が低下 = ADR-030 determinism 目標と乖離。timestamp ソート + regression test。Severity Medium + Effort M + Adoption Risk None) | +| 240 | 🔧 Tier 2 | **`takt.rs` の spawn/try_wait `Err(_)` 分岐に eprintln 追加 — 原因握り潰し解消 (PR #230 post-merge-feedback T3-#1 採用)** | todo13.md | XS | なし (spawn/try_wait の `Err(_) =>` が詳細を握り潰し `.failed` marker に実原因 (pnpm 未検出 / 権限エラー等) が残らず L2 recovery の debug 困難。`write_pending_marker_logged` 等の確立 eprintln パターン踏襲で XS。Severity Medium + Effort XS + Adoption Risk None、順位 238 と同 crate bundle 検討可) | +| 241 | 💎 Tier 3 | **binary crate の module symbol を `pub(crate)` 限定 + CLAUDE.md 明文化 (PR #230 post-merge-feedback T3-#2 採用)** | todo13.md | S | なし (feedback module 分割で write_failed_marker / fetch_pr_diff_summary / FeedbackInput / run 等が external consumer 不在なのに `pub` export = pub(crate) 方針と乖離。refactor PR ごとに再発する systemic pattern (Frequency Medium)。CLAUDE.md 明文化 + pub→pub(crate) 揃えで 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 138e7989..2f71852f 100644 --- a/docs/todo13.md +++ b/docs/todo13.md @@ -348,6 +348,105 @@ --- +### `Command::new("gh")` 直叩き禁止 + timeout wrapper 必須の custom lint (PR #230 post-merge-feedback T1-#1 採用) + +> **動機**: PR-W3 (cli-merge-pipeline 分割) で移動した `fetch_pr_time_range` / `fetch_pr_diff_summary` (pr_metadata.rs) と `run_gh_logged` / `delete_remote_branch` (github.rs) の計 4 箇所が `Command::new("gh").output()` を timeout なしで同期実行しており、ネットワーク不調や gh 側停止時に merge pipeline を無期限にハングさせる (CodeRabbit Major #2/#3、ADR-016 long-running command strategy 違反)。同 crate の pipeline.rs は既に `run_cmd_shell_capped_reporting` (timeout ラッパー) を使用しているため、直叩きを custom lint で検出して timeout 経路へ寄せる。 +> +> **本タスクの位置づけ**: PR #230 post-merge-feedback Tier 1 #1 採用 (High / Frequency High / Effort M / Adoption Risk = false positive リスク、`.rs` 限定で軽減)。PR-W3 で deferred した CodeRabbit findings #2/#3 の恒久対策層。 +> +> **参照**: `.claude/feedback-reports/230.md` Tier 1 #1、PR #230 (`3e7fdf9e`)、`src/cli-merge-pipeline/src/feedback/pr_metadata.rs` / `src/cli-merge-pipeline/src/github.rs` (対象)、`src/lib-subprocess/` `run_cmd_shell_capped_reporting` (推奨 wrapper)、`.claude/custom-lint-rules.toml` (追加先、rule①〜⑫ と同型)、`src/hooks-post-tool-linter/src/main.rs` (`CustomRule` + test)、ADR-016。 +> +> **実行優先度**: 🚀 **Tier 1** — Effort M。custom-lint-rules.toml に 1 rule + main.rs に positive/negative test。順位 240 と同 crate、1 PR bundle 検討可。 + +#### 設計決定 (案) + +- **pattern**: `Command::new("gh")` の直叩き (特に `.output()` / `.spawn()` を timeout 制御なしで呼ぶ経路) を検出。`run_cmd_shell_capped_reporting` 相当の timeout wrapper 使用を促す。 +- **severity**: warning (reviewer 判断補助)。block 化は着手時判断。 +- **scope**: extensions=["rs"]。false positive 軽減のため直叩き pattern を絞る (test code の扱いは着手時判断)。 +- **必須**: `rule_test_coverage_check` 用の positive (`Command::new("gh")` 直叩き検出) / negative (wrapper 経由は skip) test を main.rs に追加。 + +#### 作業計画 + +- [ ] `Command::new("gh")` 直叩きを検出する rule を custom-lint-rules.toml に追加 +- [ ] main.rs に positive/negative test 追加 +- [ ] 既存 `.rs` の直叩き箇所を grep して false positive 計測 +- [ ] `cargo test -p hooks-post-tool-linter` pass +- [ ] 本 entry 削除 + todo-summary.md 行削除 + +#### 完了基準 + +- `gh` の timeout なし直叩きが Write 時 (PostToolUse) に検出され timeout wrapper 使用が促される。将来同型の無期限ハング混入を構造的に予防。 + +--- + +### `filter_transcripts` の複数 jsonl 走査を timestamp ソートで deterministic 化 + regression test (PR #230 post-merge-feedback T2-#1 採用) + +> **動機**: `filter_transcripts` (transcript.rs) が `fs::read_dir` の非決定的走査順で複数 `.jsonl` を処理しており、複数 Claude セッションが並存する場合にファイル間の時系列順が保証されない。downstream の takt workflow (analyze-session) が受け取る context の順序品質が低下し、ADR-030 の determinism 目標と乖離する (CodeRabbit findings)。走査結果を timestamp ソートして決定論化し、regression test で保護する。 +> +> **本タスクの位置づけ**: PR #230 post-merge-feedback Tier 2 #1 採用 (Medium / Frequency Low / Effort M / Adoption Risk None)。 +> +> **参照**: `.claude/feedback-reports/230.md` Tier 2 #1、PR #230 (`3e7fdf9e`)、`src/cli-merge-pipeline/src/feedback/transcript.rs` (対象)、ADR-030 (determinism 目標)。 +> +> **実行優先度**: 🔧 **Tier 2** — Effort M。 + +#### 作業計画 + +- [ ] `filter_transcripts` の `fs::read_dir` 結果を timestamp (または名前) で sort してから処理するよう変更 +- [ ] 複数 jsonl の順序が入力順に依らず決定論になることを assert する regression test 追加 +- [ ] `cargo test -p cli-merge-pipeline` pass +- [ ] 本 entry 削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 複数 `.jsonl` 入力時の filter 出力が決定論的順序になり regression test で保護される。 + +--- + +### `takt.rs` の spawn/try_wait `Err(_)` 分岐に eprintln 追加 — 原因握り潰し解消 (PR #230 post-merge-feedback T3-#1 採用) + +> **動機**: `takt.rs` の `spawn()` / `try_wait()` の `Err(_) =>` 分岐がエラー詳細を握り潰しており、失敗時に `.failed` marker へ実際の原因 (`pnpm` 未検出 / 権限エラー等) が残らず L2 recovery の debugging が困難 (CodeRabbit findings)。同 crate に確立済の `write_pending_marker_logged` 等の `eprintln!` パターンを踏襲して原因を記録する。 +> +> **本タスクの位置づけ**: PR #230 post-merge-feedback Tier 3 #1 採用 (Medium / Frequency Low / Effort XS / Adoption Risk None)。 +> +> **参照**: `.claude/feedback-reports/230.md` Tier 3 #1、PR #230 (`3e7fdf9e`)、`src/cli-merge-pipeline/src/feedback/takt.rs` (対象)、同 crate `write_pending_marker_logged` (踏襲する eprintln パターン)。 +> +> **実行優先度**: 🔧 **Tier 2** — Effort XS。順位 238 と同 crate、1 PR bundle 検討可。 + +#### 作業計画 + +- [ ] `takt.rs` の `spawn()` / `try_wait()` の `Err(e)` を `eprintln!` で記録するよう変更 (握り潰しを解消) +- [ ] `cargo test -p cli-merge-pipeline` pass + `cargo clippy` clean +- [ ] 本 entry 削除 + todo-summary.md 行削除 + +#### 完了基準 + +- takt spawn/try_wait 失敗時に原因が stderr に記録され `.failed` marker からの debug が可能になる。 + +--- + +### binary crate の module symbol を `pub(crate)` 限定 + CLAUDE.md 明文化 (PR #230 post-merge-feedback T3-#2 採用) + +> **動機**: PR-W3 の feedback module 分割で `write_failed_marker` / `fetch_pr_diff_summary` / `FeedbackInput` / `run` 等、external consumer が存在しない binary crate 内シンボルが `pub` export されており、`pub(crate)` 方針と乖離している (CodeRabbit findings)。file split refactor PR ごとに繰り返す systemic pattern (Frequency Medium) のため、CLAUDE.md に方針を明文化し、既存 `pub` を `pub(crate)` に揃える。 +> +> **本タスクの位置づけ**: PR #230 post-merge-feedback Tier 3 #2 採用 (Low / Frequency Medium / Effort S / Adoption Risk None)。file-length-enforcement-plan.md の分割制約「Cross-module visibility は pub(crate)」の恒久 codify に相当。 +> +> **参照**: `.claude/feedback-reports/230.md` Tier 3 #2、PR #230 (`3e7fdf9e`)、`src/cli-merge-pipeline/src/feedback/*.rs` (pub → pub(crate) 揃え対象)、`CLAUDE.md` (方針明文化先)、docs/file-length-enforcement-plan.md § 制約条件 (既存の pub(crate) ガイド)。 +> +> **実行優先度**: 💎 **Tier 3** — Effort S。 + +#### 作業計画 + +- [ ] binary crate (cli-merge-pipeline) 内で external consumer 不在の `pub` シンボルを `pub(crate)` に変更 +- [ ] `cargo build` / `cargo clippy --workspace -- -D warnings` clean を確認 (未使用 pub 警告含む) +- [ ] CLAUDE.md に「binary crate では cross-module 共有シンボルは pub(crate)、pub は使わない」方針を明文化 +- [ ] 本 entry 削除 + todo-summary.md 行削除 + +#### 完了基準 + +- cli-merge-pipeline の module 間シンボルが `pub(crate)` に統一され、CLAUDE.md に方針が明文化される。将来の file split refactor で同型指摘が再発しない。 + +--- + ## 既知課題 (記録のみ、本セッションで未対応) (現時点で本ファイルへの既知課題は無し。docs/todo10.md / todo9.md 末尾を参照。) From f1636eefc162beab253b4de29b955237853caa10 Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 1 Jul 2026 15:01:13 +0900 Subject: [PATCH 3/4] =?UTF-8?q?refactor(cli-push-runner):=20config.rs=20+?= =?UTF-8?q?=20stages/lint=5Fscreen.rs=20=E3=82=92=20800=20=E8=A1=8C?= =?UTF-8?q?=E4=BB=A5=E4=B8=8B=E3=81=AB=20module=20=E5=88=86=E5=89=B2=20(PR?= =?UTF-8?q?-W4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli-push-runner/src/config.rs | 946 ----------------- src/cli-push-runner/src/config/lint_screen.rs | 429 ++++++++ src/cli-push-runner/src/config/mod.rs | 341 ++++++ .../src/config/pr_size_check.rs | 123 +++ .../src/config/scratch_file_warning.rs | 99 ++ src/cli-push-runner/src/stages/lint_screen.rs | 982 ------------------ .../src/stages/lint_screen/classifier.rs | 74 ++ .../src/stages/lint_screen/diff_filter.rs | 415 ++++++++ .../src/stages/lint_screen/mod.rs | 221 ++++ .../src/stages/lint_screen/report.rs | 331 ++++++ 10 files changed, 2033 insertions(+), 1928 deletions(-) delete mode 100644 src/cli-push-runner/src/config.rs create mode 100644 src/cli-push-runner/src/config/lint_screen.rs create mode 100644 src/cli-push-runner/src/config/mod.rs create mode 100644 src/cli-push-runner/src/config/pr_size_check.rs create mode 100644 src/cli-push-runner/src/config/scratch_file_warning.rs delete mode 100644 src/cli-push-runner/src/stages/lint_screen.rs create mode 100644 src/cli-push-runner/src/stages/lint_screen/classifier.rs create mode 100644 src/cli-push-runner/src/stages/lint_screen/diff_filter.rs create mode 100644 src/cli-push-runner/src/stages/lint_screen/mod.rs create mode 100644 src/cli-push-runner/src/stages/lint_screen/report.rs diff --git a/src/cli-push-runner/src/config.rs b/src/cli-push-runner/src/config.rs deleted file mode 100644 index 5489146a..00000000 --- a/src/cli-push-runner/src/config.rs +++ /dev/null @@ -1,946 +0,0 @@ -use serde::Deserialize; -use std::path::{Path, PathBuf}; - -pub(crate) const DEFAULT_STEP_TIMEOUT_SECS: u64 = 120; -pub(crate) const DEFAULT_PUSH_TIMEOUT_SECS: u64 = 300; -pub(crate) const DEFAULT_LINT_SCREEN_TIMEOUT_SECS: u64 = 60; -pub(crate) const DEFAULT_LINT_SCREEN_MAX_DIFF_LINES: usize = 5000; -pub(crate) const DEFAULT_LINT_SCREEN_MODEL: &str = "mistral:7b"; -pub(crate) const DEFAULT_LINT_SCREEN_ENDPOINT: &str = "http://localhost:11434"; -pub(crate) const DEFAULT_LINT_SCREEN_EXE_PATH: &str = ".claude/cli-finding-classifier.exe"; -pub(crate) const DEFAULT_LINT_SCREEN_OUTPUT_PATH: &str = ".takt/lint-screen-report.md"; - -/// 順位 151 (Bundle "既存ルール仕組み化"): PR diff 行数 (insertions + deletions) の -/// warning 閾値。`~/.claude/rules/common/git-workflow.md` § Multi-PR chaining の -/// 「1 PR あたり 250-800 lines」目安に同期。 -pub(crate) const DEFAULT_PR_SIZE_WARNING_THRESHOLD: usize = 800; - -/// 順位 151: PR diff 行数の block 閾値。これを超えると push を停止する。 -/// 大型 refactoring 時は config / env override で意図的バイパス。 -pub(crate) const DEFAULT_PR_SIZE_BLOCK_THRESHOLD: usize = 1500; - -/// 順位 151: PR base の default branch 名。`format!("{}..@", default_branch)` で -/// revset 組立 (rule⑫ `no-hardcoded-jj-revset-range` 適用)。 -pub(crate) const DEFAULT_PR_SIZE_BASE_BRANCH: &str = "master"; - -/// `LINT_SCREEN_ENABLED` env var の名前 (順位 115、Phase D D-1 workflow gap 解消)。 -/// -/// 用途: session-only opt-in (jj auto-snapshot 環境で `push-runner-config.toml` を編集せずに -/// lint_screen を一時的に有効化する)。 -/// -/// **解釈** (todo entry 順位 115 設計決定に基づく): -/// - `"true"` / `"1"` / `"yes"` (case-insensitive、空白 trim) → **force enable** (TOML override) -/// - `"false"` / `"0"` / `"no"` / `""` / unset → **TOML 値を尊重** (override しない、no-op) -/// - その他の値 → warning emit + TOML 値を尊重 (= invalid 扱い、安全側に倒す) -/// -/// **片方向設計の意図**: env を temporary に set すれば session opt-in、unset すれば TOML default -/// (= `enabled = false`) に自然復帰する。誤って commit しても remote PR は config 上 OFF のまま -/// (= dogfood は走らない) なので、Phase D guide §1 の「local enable / remote disable」が成立する。 -pub(crate) const ENV_LINT_SCREEN_ENABLED: &str = "LINT_SCREEN_ENABLED"; - -#[derive(Deserialize)] -pub(crate) struct Config { - pub(crate) quality_gate: QualityGateConfig, - pub(crate) diff: Option, - pub(crate) lint_screen: Option, - pub(crate) takt: TaktConfig, - pub(crate) push: PushConfig, - pub(crate) scratch_file_warning: Option, - pub(crate) pr_size_check: Option, -} - -/// 順位 151 (Bundle "既存ルール仕組み化") — PR diff size を `jj diff --stat` で計測し -/// warning / block する pre-push stage の config。 -/// -/// `~/.claude/rules/common/git-workflow.md` § Multi-PR chaining の「1 PR あたり -/// 250-800 lines」目安を決定論的に維持する。 -/// -/// ADR-039 (Experimental feature 標準パターン) 3 点セット準拠: -/// - **Config opt-in**: 試験運用のため default `enabled = false`。`[pr_size_check]` section -/// 不在 / `enabled` 未設定 / `enabled = false` のいずれも検査を完全 skip。 -/// - **Kill-switch**: `enabled = false` (TOML) + env override `PR_SIZE_CHECK_OVERRIDE=1` で -/// 個別 push の意図的バイパス可能 (大型 refactoring 時)。 -/// - **Bounded lifetime**: 3-5 PR の dogfood で false positive / 検出効果を観測後、 -/// default-ON 昇格 or 却下を判定。判定結果は `src/cli-push-runner/src/stages/pr_size_check.rs` -/// module doc + `push-runner-config.toml` の `[pr_size_check]` section コメントに反映する。 -/// -/// revset は `format!("{}..@", default_branch)` 形式で組立 (rule⑫ -/// `no-hardcoded-jj-revset-range` 適用、alternative branch "main" 等への切替を保護)。 -#[derive(Deserialize)] -pub(crate) struct PrSizeCheckConfig { - pub(crate) enabled: Option, - pub(crate) default_branch: Option, - pub(crate) warning_threshold: Option, - pub(crate) block_threshold: Option, -} - -/// 順位 1 (PR #85 T1-4) — scratch ファイル (`__*` 等) が `@` commit に -/// 混入していないか push 前に検査する stage の config。 -/// -/// ADR-039 (Experimental feature 標準パターン) § 1 Config opt-in 準拠: -/// `[scratch_file_warning]` section 不在 / `enabled` 未設定 / `enabled = false` -/// のいずれも検査を **skip** (= default `enabled = false`)。明示的に `enabled = true` -/// にしたときのみ検査実行 (3-5 PR の dogfood 後に default-ON 昇格 or 却下を判定)。 -/// -/// `patterns` は順位 5 (AI 生成一時スクリプト pattern の pre-push 検出) で -/// `_tmp_*` 等の追加 pattern を config-driven で拡張可能 (= 補完アプローチ)。 -/// `patterns` 未設定時の default は `["__*"]` (= stage 側 `DEFAULT_PATTERN`)。 -#[derive(Deserialize)] -pub(crate) struct ScratchFileWarningConfig { - pub(crate) enabled: Option, - pub(crate) patterns: Option>, -} - -/// Phase c (§8.E lint screen facet) — pre-push 時に diff を mistral:7b に流して -/// lint 一次フィルタの所見を `.takt/lint-screen-report.md` として出力する。 -/// -/// `enabled = false` の場合は完全 no-op (default OFF, 試験運用)。 -/// Ollama down / timeout / diff 過大時は skip + warn (push を block しない)。 -#[derive(Deserialize)] -pub(crate) struct LintScreenConfig { - pub(crate) enabled: bool, - pub(crate) exe_path: Option, - pub(crate) model: Option, - pub(crate) endpoint: Option, - pub(crate) timeout_secs: Option, - pub(crate) max_diff_lines: Option, - pub(crate) output_path: Option, -} - -#[derive(Deserialize)] -pub(crate) struct QualityGateConfig { - pub(crate) parallel: Option, - pub(crate) step_timeout: Option, - pub(crate) groups: Vec, -} - -#[derive(Deserialize, Clone)] -pub(crate) struct GroupConfig { - pub(crate) name: String, - pub(crate) pre: Option, - pub(crate) commands: Vec, -} - -#[derive(Deserialize)] -pub(crate) struct TaktConfig { - pub(crate) workflow: String, - pub(crate) task: String, - pub(crate) extra_args: Option>, -} - -#[derive(Deserialize)] -pub(crate) struct DiffConfig { - pub(crate) command: String, - pub(crate) output_path: String, -} - -#[derive(Deserialize)] -pub(crate) struct PushConfig { - pub(crate) command: String, - pub(crate) timeout: Option, -} - -pub(crate) fn config_path() -> PathBuf { - let filename = "push-runner-config.toml"; - - // 1. カレントディレクトリを優先（pnpm scripts はリポジトリルートで実行される） - let cwd_path = Path::new(filename).to_path_buf(); - if cwd_path.exists() { - return cwd_path; - } - - // 2. exe と同じディレクトリ（.claude/ 配置パターン） - std::env::current_exe() - .unwrap_or_default() - .parent() - .unwrap_or(Path::new(".")) - .join(filename) -} - -pub(crate) fn load_config() -> Result { - let path = config_path(); - let content = std::fs::read_to_string(&path) - .map_err(|e| format!("設定ファイルの読み込みに失敗: {} ({})", path.display(), e))?; - let mut config: Config = - toml::from_str(&content).map_err(|e| format!("設定ファイルのパースに失敗: {}", e))?; - apply_lint_screen_env_override(&mut config, std::env::var(ENV_LINT_SCREEN_ENABLED).ok()); - validate_config(&config)?; - Ok(config) -} - -/// `LINT_SCREEN_ENABLED` env var を解釈した結果。 -/// -/// `parse_lint_screen_env` の戻り値で、`apply_lint_screen_env_override` が分岐する。 -#[derive(Debug, PartialEq, Eq)] -enum LintScreenEnvOverride { - /// env が `true`/`1`/`yes` 系 → TOML 値を上書きして `enabled = true` を強制。 - ForceEnable, - /// env が `false`/`0`/`no`/`""`/unset → TOML 値を尊重 (no-op)。 - RespectToml, - /// env が解釈不能な文字列 → warning emit 候補、安全側で TOML 値を尊重。 - InvalidValue, -} - -/// `LINT_SCREEN_ENABLED` env var の生文字列を解釈する純粋関数 (test 容易性のため env 読み取りと分離)。 -/// -/// `None` (unset) は `RespectToml` として扱う。空白 trim + 小文字化して比較する。 -fn parse_lint_screen_env(raw: Option<&str>) -> LintScreenEnvOverride { - let Some(value) = raw else { - return LintScreenEnvOverride::RespectToml; - }; - let normalized = value.trim().to_ascii_lowercase(); - match normalized.as_str() { - "true" | "1" | "yes" | "on" => LintScreenEnvOverride::ForceEnable, - "false" | "0" | "no" | "off" | "" => LintScreenEnvOverride::RespectToml, - _ => LintScreenEnvOverride::InvalidValue, - } -} - -/// `LINT_SCREEN_ENABLED` env var の値を `config.lint_screen.enabled` に反映する。 -/// -/// 設計仕様は `ENV_LINT_SCREEN_ENABLED` の doc comment 参照。`[lint_screen]` section が -/// TOML に存在しなくても、env が `ForceEnable` の場合は default 値で `LintScreenConfig` を -/// 生成する (Phase D D-1 で発見した workflow gap の解消、順位 115)。 -/// -/// `raw` 引数は test 容易性のため caller が `std::env::var(...)` を解決して渡す。 -fn apply_lint_screen_env_override(config: &mut Config, raw: Option) { - let raw_ref = raw.as_deref(); - match parse_lint_screen_env(raw_ref) { - LintScreenEnvOverride::ForceEnable => { - match config.lint_screen.as_mut() { - Some(lint) => { - lint.enabled = true; - } - None => { - config.lint_screen = Some(default_lint_screen_enabled()); - } - } - eprintln!( - "[push-runner] {}: TOML override で [lint_screen] enabled を true に強制 (順位 115 env override)", - ENV_LINT_SCREEN_ENABLED - ); - } - LintScreenEnvOverride::RespectToml => {} - LintScreenEnvOverride::InvalidValue => { - eprintln!( - "[push-runner] WARN: {}='{}' を bool として解釈できません、TOML 値を尊重します。\ - 受容値: true/1/yes/on (enable) / false/0/no/off/\"\" (TOML 尊重)", - ENV_LINT_SCREEN_ENABLED, - raw_ref.unwrap_or("") - ); - } - } -} - -/// env override で `[lint_screen]` section を新規生成する際の default 値。 -/// -/// 他の field は `None` のままで、`stages::lint_screen` の `resolve_invoke_params` / `run_lint_screen` -/// 側が `DEFAULT_LINT_SCREEN_*` 定数で fallback する。 -fn default_lint_screen_enabled() -> LintScreenConfig { - LintScreenConfig { - enabled: true, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: None, - output_path: None, - } -} - -fn validate_config(config: &Config) -> Result<(), String> { - if config.quality_gate.groups.is_empty() { - return Err("設定ファイルエラー: quality_gate.groups が空です".into()); - } - for group in &config.quality_gate.groups { - if group.commands.is_empty() { - return Err(format!( - "設定ファイルエラー: group '{}' の commands が空です", - group.name - )); - } - } - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn config_parses_full_without_diff() { - let toml_str = r#" -[quality_gate] -parallel = true -step_timeout = 60 - -[[quality_gate.groups]] -name = "lint" -commands = ["pnpm lint"] - -[[quality_gate.groups]] -name = "test" -pre = "pnpm install" -commands = ["pnpm test", "pnpm test:e2e"] - -[takt] -workflow = "pre-push-review" -task = "pre-push review" -extra_args = ["--pipeline", "--skip-git"] - -[push] -command = "jj git push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - - assert_eq!(config.quality_gate.parallel, Some(true)); - assert_eq!(config.quality_gate.step_timeout, Some(60)); - assert_eq!(config.quality_gate.groups.len(), 2); - assert!(config.diff.is_none()); - - assert_eq!(config.takt.workflow, "pre-push-review"); - assert_eq!(config.takt.task, "pre-push review"); - assert_eq!(config.takt.extra_args.as_ref().unwrap().len(), 2); - - assert_eq!(config.push.command, "jj git push"); - assert!(config.push.timeout.is_none()); - } - - #[test] - fn config_push_timeout_explicit() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "jj git push" -timeout = 600 -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert_eq!(config.push.timeout, Some(600)); - assert_eq!( - config.push.timeout.unwrap_or(DEFAULT_PUSH_TIMEOUT_SECS), - 600, - ); - } - - #[test] - fn config_push_timeout_defaults() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!(config.push.timeout.is_none()); - assert_eq!( - config.push.timeout.unwrap_or(DEFAULT_PUSH_TIMEOUT_SECS), - DEFAULT_PUSH_TIMEOUT_SECS, - ); - } - - #[test] - fn config_parses_with_diff() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[diff] -command = "jj diff -r @" -output_path = ".takt/review-diff.txt" - -[takt] -workflow = "pre-push-review" -task = "pre-push review" - -[push] -command = "jj git push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - - let diff = config.diff.unwrap(); - assert_eq!(diff.command, "jj diff -r @"); - assert_eq!(diff.output_path, ".takt/review-diff.txt"); - } - - #[test] - fn config_parses_with_lint_screen_section_full_fields() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[lint_screen] -enabled = true -exe_path = ".claude/cli-finding-classifier.exe" -model = "mistral:7b" -endpoint = "http://localhost:11434" -timeout_secs = 90 -max_diff_lines = 4000 -output_path = ".takt/lint-screen-report.md" - -[takt] -workflow = "pre-push-review" -task = "pre-push review" - -[push] -command = "jj git push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - - let lint = config - .lint_screen - .expect("[lint_screen] section should produce Some(LintScreenConfig)"); - assert!(lint.enabled); - assert_eq!( - lint.exe_path.as_deref(), - Some(".claude/cli-finding-classifier.exe") - ); - assert_eq!(lint.model.as_deref(), Some("mistral:7b")); - assert_eq!(lint.endpoint.as_deref(), Some("http://localhost:11434")); - assert_eq!(lint.timeout_secs, Some(90)); - assert_eq!(lint.max_diff_lines, Some(4000)); - assert_eq!( - lint.output_path.as_deref(), - Some(".takt/lint-screen-report.md") - ); - } - - #[test] - fn config_parses_with_lint_screen_section_minimal_only_enabled() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[lint_screen] -enabled = false - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - - let lint = config - .lint_screen - .expect("section present even with only `enabled` should produce Some"); - assert!(!lint.enabled); - assert!(lint.exe_path.is_none()); - assert!(lint.model.is_none()); - assert!(lint.endpoint.is_none()); - assert!(lint.timeout_secs.is_none()); - assert!(lint.max_diff_lines.is_none()); - assert!(lint.output_path.is_none()); - } - - #[test] - fn config_lint_screen_section_absent_yields_none() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!( - config.lint_screen.is_none(), - "absent [lint_screen] should yield None (default OFF lane)" - ); - } - - const LINT_SCREEN_ONLY_ENABLED_TOML: &str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[lint_screen] -enabled = true - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - - #[test] - fn config_lint_screen_numeric_defaults_resolve_via_constants() { - let config: Config = toml::from_str(LINT_SCREEN_ONLY_ENABLED_TOML).unwrap(); - let lint = config.lint_screen.unwrap(); - assert_eq!( - lint.timeout_secs.unwrap_or(DEFAULT_LINT_SCREEN_TIMEOUT_SECS), - DEFAULT_LINT_SCREEN_TIMEOUT_SECS, - ); - assert_eq!( - lint.max_diff_lines - .unwrap_or(DEFAULT_LINT_SCREEN_MAX_DIFF_LINES), - DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, - ); - } - - #[test] - fn config_lint_screen_string_defaults_resolve_via_constants() { - let config: Config = toml::from_str(LINT_SCREEN_ONLY_ENABLED_TOML).unwrap(); - let lint = config.lint_screen.unwrap(); - assert_eq!( - lint.model.as_deref().unwrap_or(DEFAULT_LINT_SCREEN_MODEL), - DEFAULT_LINT_SCREEN_MODEL, - ); - assert_eq!( - lint.endpoint - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_ENDPOINT), - DEFAULT_LINT_SCREEN_ENDPOINT, - ); - assert_eq!( - lint.exe_path - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_EXE_PATH), - DEFAULT_LINT_SCREEN_EXE_PATH, - ); - assert_eq!( - lint.output_path - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_OUTPUT_PATH), - DEFAULT_LINT_SCREEN_OUTPUT_PATH, - ); - } - - #[test] - fn config_quality_gate_defaults() { - let toml_str = r#" -[quality_gate] - -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!(config.quality_gate.parallel.unwrap_or(true)); - assert_eq!( - config - .quality_gate - .step_timeout - .unwrap_or(DEFAULT_STEP_TIMEOUT_SECS), - DEFAULT_STEP_TIMEOUT_SECS, - ); - assert!(config.takt.extra_args.is_none()); - } - - #[test] - fn config_pre_field_optional() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "no-pre" -commands = ["echo test"] - -[[quality_gate.groups]] -name = "with-pre" -pre = "echo install" -commands = ["echo test"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!(config.quality_gate.groups[0].pre.is_none()); - assert!(config.quality_gate.groups[1].pre.is_some()); - } - - #[test] - fn validate_rejects_empty_groups() { - let config = Config { - quality_gate: QualityGateConfig { - parallel: None, - step_timeout: None, - groups: vec![], - }, - diff: None, - lint_screen: None, - scratch_file_warning: None, - pr_size_check: None, - takt: TaktConfig { - workflow: "w".into(), - task: "t".into(), - extra_args: None, - }, - push: PushConfig { - command: "echo".into(), - timeout: None, - }, - }; - let result = validate_config(&config); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("groups が空")); - } - - #[test] - fn parse_lint_screen_env_unset_yields_respect_toml() { - assert_eq!( - parse_lint_screen_env(None), - LintScreenEnvOverride::RespectToml - ); - } - - #[test] - fn parse_lint_screen_env_force_enable_variants() { - for value in ["true", "TRUE", "1", "yes", "YES", "on", "On", " true ", "\tyes\n"] { - assert_eq!( - parse_lint_screen_env(Some(value)), - LintScreenEnvOverride::ForceEnable, - "value '{}' should map to ForceEnable", - value - ); - } - } - - #[test] - fn parse_lint_screen_env_respect_toml_variants() { - for value in ["false", "FALSE", "0", "no", "NO", "off", "", " "] { - assert_eq!( - parse_lint_screen_env(Some(value)), - LintScreenEnvOverride::RespectToml, - "value '{}' should map to RespectToml", - value - ); - } - } - - #[test] - fn parse_lint_screen_env_invalid_value() { - for value in ["maybe", "2", "enable", "disabled", "yes please"] { - assert_eq!( - parse_lint_screen_env(Some(value)), - LintScreenEnvOverride::InvalidValue, - "value '{}' should map to InvalidValue", - value - ); - } - } - - fn make_config_without_lint_screen() -> Config { - Config { - quality_gate: QualityGateConfig { - parallel: None, - step_timeout: None, - groups: vec![GroupConfig { - name: "t".into(), - pre: None, - commands: vec!["echo".into()], - }], - }, - diff: None, - lint_screen: None, - scratch_file_warning: None, - pr_size_check: None, - takt: TaktConfig { - workflow: "w".into(), - task: "t".into(), - extra_args: None, - }, - push: PushConfig { - command: "echo".into(), - timeout: None, - }, - } - } - - fn make_config_with_lint_screen(enabled: bool) -> Config { - let mut config = make_config_without_lint_screen(); - config.lint_screen = Some(LintScreenConfig { - enabled, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: None, - output_path: None, - }); - config - } - - #[test] - fn apply_env_override_force_enable_on_absent_section_creates_lint_screen_config() { - let mut config = make_config_without_lint_screen(); - apply_lint_screen_env_override(&mut config, Some("true".to_string())); - let lint = config.lint_screen.expect( - "env=true should construct default LintScreenConfig when [lint_screen] section absent", - ); - assert!(lint.enabled); - assert!(lint.exe_path.is_none()); - assert!(lint.model.is_none()); - } - - #[test] - fn apply_env_override_force_enable_overwrites_toml_false() { - let mut config = make_config_with_lint_screen(false); - apply_lint_screen_env_override(&mut config, Some("1".to_string())); - assert!(config.lint_screen.unwrap().enabled); - } - - #[test] - fn apply_env_override_respect_toml_keeps_toml_enabled_true() { - let mut config = make_config_with_lint_screen(true); - apply_lint_screen_env_override(&mut config, Some("false".to_string())); - assert!( - config.lint_screen.unwrap().enabled, - "env=false should respect TOML (TOML had enabled=true, must remain true)" - ); - } - - #[test] - fn apply_env_override_respect_toml_keeps_toml_enabled_false() { - let mut config = make_config_with_lint_screen(false); - apply_lint_screen_env_override(&mut config, Some("".to_string())); - assert!(!config.lint_screen.unwrap().enabled); - } - - #[test] - fn apply_env_override_unset_keeps_toml_section_absent() { - let mut config = make_config_without_lint_screen(); - apply_lint_screen_env_override(&mut config, None); - assert!( - config.lint_screen.is_none(), - "env unset + [lint_screen] absent should remain None" - ); - } - - #[test] - fn apply_env_override_invalid_value_respects_toml() { - let mut config = make_config_with_lint_screen(false); - apply_lint_screen_env_override(&mut config, Some("maybe".to_string())); - assert!( - !config.lint_screen.unwrap().enabled, - "invalid env value should treat as RespectToml (TOML enabled=false preserved)" - ); - } - - #[test] - fn config_parses_with_scratch_file_warning_full() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[scratch_file_warning] -enabled = true -patterns = ["__*", "_tmp_*"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let s = config - .scratch_file_warning - .expect("[scratch_file_warning] should parse to Some"); - assert_eq!(s.enabled, Some(true)); - assert_eq!( - s.patterns.unwrap(), - vec!["__*".to_string(), "_tmp_*".to_string()] - ); - } - - #[test] - fn config_parses_with_scratch_file_warning_only_enabled_false() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[scratch_file_warning] -enabled = false - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let s = config.scratch_file_warning.unwrap(); - assert_eq!(s.enabled, Some(false)); - assert!(s.patterns.is_none()); - } - - #[test] - fn config_parses_with_pr_size_check_full() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[pr_size_check] -enabled = true -default_branch = "main" -warning_threshold = 500 -block_threshold = 2000 - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let s = config - .pr_size_check - .expect("[pr_size_check] should parse to Some"); - assert_eq!(s.enabled, Some(true)); - assert_eq!(s.default_branch.as_deref(), Some("main")); - assert_eq!(s.warning_threshold, Some(500)); - assert_eq!(s.block_threshold, Some(2000)); - } - - #[test] - fn config_pr_size_check_absent_yields_none() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!( - config.pr_size_check.is_none(), - "absent [pr_size_check] should yield None (default OFF lane)" - ); - } - - #[test] - fn config_pr_size_check_only_enabled_false() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[pr_size_check] -enabled = false - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let s = config.pr_size_check.unwrap(); - assert_eq!(s.enabled, Some(false)); - assert!(s.default_branch.is_none()); - assert!(s.warning_threshold.is_none()); - assert!(s.block_threshold.is_none()); - } - - #[test] - fn config_scratch_file_warning_absent_yields_none() { - let toml_str = r#" -[quality_gate] -[[quality_gate.groups]] -name = "test" -commands = ["echo ok"] - -[takt] -workflow = "w" -task = "t" - -[push] -command = "echo push" -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!( - config.scratch_file_warning.is_none(), - "absent [scratch_file_warning] should yield None (default-ON 動作は stage 側で解決)" - ); - } - - #[test] - fn validate_rejects_empty_commands() { - let config = Config { - quality_gate: QualityGateConfig { - parallel: None, - step_timeout: None, - groups: vec![GroupConfig { - name: "empty".into(), - pre: None, - commands: vec![], - }], - }, - diff: None, - lint_screen: None, - scratch_file_warning: None, - pr_size_check: None, - takt: TaktConfig { - workflow: "w".into(), - task: "t".into(), - extra_args: None, - }, - push: PushConfig { - command: "echo".into(), - timeout: None, - }, - }; - let result = validate_config(&config); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("'empty'")); - } -} diff --git a/src/cli-push-runner/src/config/lint_screen.rs b/src/cli-push-runner/src/config/lint_screen.rs new file mode 100644 index 00000000..f40cee43 --- /dev/null +++ b/src/cli-push-runner/src/config/lint_screen.rs @@ -0,0 +1,429 @@ +use serde::Deserialize; + +use super::Config; + +pub(crate) const DEFAULT_LINT_SCREEN_TIMEOUT_SECS: u64 = 60; +pub(crate) const DEFAULT_LINT_SCREEN_MAX_DIFF_LINES: usize = 5000; +pub(crate) const DEFAULT_LINT_SCREEN_MODEL: &str = "mistral:7b"; +pub(crate) const DEFAULT_LINT_SCREEN_ENDPOINT: &str = "http://localhost:11434"; +pub(crate) const DEFAULT_LINT_SCREEN_EXE_PATH: &str = ".claude/cli-finding-classifier.exe"; +pub(crate) const DEFAULT_LINT_SCREEN_OUTPUT_PATH: &str = ".takt/lint-screen-report.md"; + +/// `LINT_SCREEN_ENABLED` env var の名前 (順位 115、Phase D D-1 workflow gap 解消)。 +/// +/// 用途: session-only opt-in (jj auto-snapshot 環境で `push-runner-config.toml` を編集せずに +/// lint_screen を一時的に有効化する)。 +/// +/// **解釈** (todo entry 順位 115 設計決定に基づく): +/// - `"true"` / `"1"` / `"yes"` (case-insensitive、空白 trim) → **force enable** (TOML override) +/// - `"false"` / `"0"` / `"no"` / `""` / unset → **TOML 値を尊重** (override しない、no-op) +/// - その他の値 → warning emit + TOML 値を尊重 (= invalid 扱い、安全側に倒す) +/// +/// **片方向設計の意図**: env を temporary に set すれば session opt-in、unset すれば TOML default +/// (= `enabled = false`) に自然復帰する。誤って commit しても remote PR は config 上 OFF のまま +/// (= dogfood は走らない) なので、Phase D guide §1 の「local enable / remote disable」が成立する。 +pub(crate) const ENV_LINT_SCREEN_ENABLED: &str = "LINT_SCREEN_ENABLED"; + +/// Phase c (§8.E lint screen facet) — pre-push 時に diff を mistral:7b に流して +/// lint 一次フィルタの所見を `.takt/lint-screen-report.md` として出力する。 +/// +/// `enabled = false` の場合は完全 no-op (default OFF, 試験運用)。 +/// Ollama down / timeout / diff 過大時は skip + warn (push を block しない)。 +#[derive(Deserialize)] +pub(crate) struct LintScreenConfig { + pub(crate) enabled: bool, + pub(crate) exe_path: Option, + pub(crate) model: Option, + pub(crate) endpoint: Option, + pub(crate) timeout_secs: Option, + pub(crate) max_diff_lines: Option, + pub(crate) output_path: Option, +} + +/// `LINT_SCREEN_ENABLED` env var を解釈した結果。 +/// +/// `parse_lint_screen_env` の戻り値で、`apply_lint_screen_env_override` が分岐する。 +#[derive(Debug, PartialEq, Eq)] +enum LintScreenEnvOverride { + /// env が `true`/`1`/`yes` 系 → TOML 値を上書きして `enabled = true` を強制。 + ForceEnable, + /// env が `false`/`0`/`no`/`""`/unset → TOML 値を尊重 (no-op)。 + RespectToml, + /// env が解釈不能な文字列 → warning emit 候補、安全側で TOML 値を尊重。 + InvalidValue, +} + +/// `LINT_SCREEN_ENABLED` env var の生文字列を解釈する純粋関数 (test 容易性のため env 読み取りと分離)。 +/// +/// `None` (unset) は `RespectToml` として扱う。空白 trim + 小文字化して比較する。 +fn parse_lint_screen_env(raw: Option<&str>) -> LintScreenEnvOverride { + let Some(value) = raw else { + return LintScreenEnvOverride::RespectToml; + }; + let normalized = value.trim().to_ascii_lowercase(); + match normalized.as_str() { + "true" | "1" | "yes" | "on" => LintScreenEnvOverride::ForceEnable, + "false" | "0" | "no" | "off" | "" => LintScreenEnvOverride::RespectToml, + _ => LintScreenEnvOverride::InvalidValue, + } +} + +/// `LINT_SCREEN_ENABLED` env var の値を `config.lint_screen.enabled` に反映する。 +/// +/// 設計仕様は `ENV_LINT_SCREEN_ENABLED` の doc comment 参照。`[lint_screen]` section が +/// TOML に存在しなくても、env が `ForceEnable` の場合は default 値で `LintScreenConfig` を +/// 生成する (Phase D D-1 で発見した workflow gap の解消、順位 115)。 +/// +/// `raw` 引数は test 容易性のため caller が `std::env::var(...)` を解決して渡す。 +pub(super) fn apply_lint_screen_env_override(config: &mut Config, raw: Option) { + let raw_ref = raw.as_deref(); + match parse_lint_screen_env(raw_ref) { + LintScreenEnvOverride::ForceEnable => { + match config.lint_screen.as_mut() { + Some(lint) => { + lint.enabled = true; + } + None => { + config.lint_screen = Some(default_lint_screen_enabled()); + } + } + eprintln!( + "[push-runner] {}: TOML override で [lint_screen] enabled を true に強制 (順位 115 env override)", + ENV_LINT_SCREEN_ENABLED + ); + } + LintScreenEnvOverride::RespectToml => {} + LintScreenEnvOverride::InvalidValue => { + eprintln!( + "[push-runner] WARN: {}='{}' を bool として解釈できません、TOML 値を尊重します。\ + 受容値: true/1/yes/on (enable) / false/0/no/off/\"\" (TOML 尊重)", + ENV_LINT_SCREEN_ENABLED, + raw_ref.unwrap_or("") + ); + } + } +} + +/// env override で `[lint_screen]` section を新規生成する際の default 値。 +/// +/// 他の field は `None` のままで、`stages::lint_screen` の `resolve_invoke_params` / `run_lint_screen` +/// 側が `DEFAULT_LINT_SCREEN_*` 定数で fallback する。 +fn default_lint_screen_enabled() -> LintScreenConfig { + LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{GroupConfig, PushConfig, QualityGateConfig, TaktConfig}; + + #[test] + fn config_parses_with_lint_screen_section_full_fields() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[lint_screen] +enabled = true +exe_path = ".claude/cli-finding-classifier.exe" +model = "mistral:7b" +endpoint = "http://localhost:11434" +timeout_secs = 90 +max_diff_lines = 4000 +output_path = ".takt/lint-screen-report.md" + +[takt] +workflow = "pre-push-review" +task = "pre-push review" + +[push] +command = "jj git push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + + let lint = config + .lint_screen + .expect("[lint_screen] section should produce Some(LintScreenConfig)"); + assert!(lint.enabled); + assert_eq!( + lint.exe_path.as_deref(), + Some(".claude/cli-finding-classifier.exe") + ); + assert_eq!(lint.model.as_deref(), Some("mistral:7b")); + assert_eq!(lint.endpoint.as_deref(), Some("http://localhost:11434")); + assert_eq!(lint.timeout_secs, Some(90)); + assert_eq!(lint.max_diff_lines, Some(4000)); + assert_eq!( + lint.output_path.as_deref(), + Some(".takt/lint-screen-report.md") + ); + } + + #[test] + fn config_parses_with_lint_screen_section_minimal_only_enabled() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[lint_screen] +enabled = false + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + + let lint = config + .lint_screen + .expect("section present even with only `enabled` should produce Some"); + assert!(!lint.enabled); + assert!(lint.exe_path.is_none()); + assert!(lint.model.is_none()); + assert!(lint.endpoint.is_none()); + assert!(lint.timeout_secs.is_none()); + assert!(lint.max_diff_lines.is_none()); + assert!(lint.output_path.is_none()); + } + + #[test] + fn config_lint_screen_section_absent_yields_none() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!( + config.lint_screen.is_none(), + "absent [lint_screen] should yield None (default OFF lane)" + ); + } + + const LINT_SCREEN_ONLY_ENABLED_TOML: &str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[lint_screen] +enabled = true + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + + #[test] + fn config_lint_screen_numeric_defaults_resolve_via_constants() { + let config: Config = toml::from_str(LINT_SCREEN_ONLY_ENABLED_TOML).unwrap(); + let lint = config.lint_screen.unwrap(); + assert_eq!( + lint.timeout_secs + .unwrap_or(DEFAULT_LINT_SCREEN_TIMEOUT_SECS), + DEFAULT_LINT_SCREEN_TIMEOUT_SECS, + ); + assert_eq!( + lint.max_diff_lines + .unwrap_or(DEFAULT_LINT_SCREEN_MAX_DIFF_LINES), + DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, + ); + } + + #[test] + fn config_lint_screen_string_defaults_resolve_via_constants() { + let config: Config = toml::from_str(LINT_SCREEN_ONLY_ENABLED_TOML).unwrap(); + let lint = config.lint_screen.unwrap(); + assert_eq!( + lint.model.as_deref().unwrap_or(DEFAULT_LINT_SCREEN_MODEL), + DEFAULT_LINT_SCREEN_MODEL, + ); + assert_eq!( + lint.endpoint + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_ENDPOINT), + DEFAULT_LINT_SCREEN_ENDPOINT, + ); + assert_eq!( + lint.exe_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_EXE_PATH), + DEFAULT_LINT_SCREEN_EXE_PATH, + ); + assert_eq!( + lint.output_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_OUTPUT_PATH), + DEFAULT_LINT_SCREEN_OUTPUT_PATH, + ); + } + + #[test] + fn parse_lint_screen_env_unset_yields_respect_toml() { + assert_eq!( + parse_lint_screen_env(None), + LintScreenEnvOverride::RespectToml + ); + } + + #[test] + fn parse_lint_screen_env_force_enable_variants() { + for value in [ + "true", "TRUE", "1", "yes", "YES", "on", "On", " true ", "\tyes\n", + ] { + assert_eq!( + parse_lint_screen_env(Some(value)), + LintScreenEnvOverride::ForceEnable, + "value '{}' should map to ForceEnable", + value + ); + } + } + + #[test] + fn parse_lint_screen_env_respect_toml_variants() { + for value in ["false", "FALSE", "0", "no", "NO", "off", "", " "] { + assert_eq!( + parse_lint_screen_env(Some(value)), + LintScreenEnvOverride::RespectToml, + "value '{}' should map to RespectToml", + value + ); + } + } + + #[test] + fn parse_lint_screen_env_invalid_value() { + for value in ["maybe", "2", "enable", "disabled", "yes please"] { + assert_eq!( + parse_lint_screen_env(Some(value)), + LintScreenEnvOverride::InvalidValue, + "value '{}' should map to InvalidValue", + value + ); + } + } + + fn make_config_without_lint_screen() -> Config { + Config { + quality_gate: QualityGateConfig { + parallel: None, + step_timeout: None, + groups: vec![GroupConfig { + name: "t".into(), + pre: None, + commands: vec!["echo".into()], + }], + }, + diff: None, + lint_screen: None, + scratch_file_warning: None, + pr_size_check: None, + takt: TaktConfig { + workflow: "w".into(), + task: "t".into(), + extra_args: None, + }, + push: PushConfig { + command: "echo".into(), + timeout: None, + }, + } + } + + fn make_config_with_lint_screen(enabled: bool) -> Config { + let mut config = make_config_without_lint_screen(); + config.lint_screen = Some(LintScreenConfig { + enabled, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }); + config + } + + #[test] + fn apply_env_override_force_enable_on_absent_section_creates_lint_screen_config() { + let mut config = make_config_without_lint_screen(); + apply_lint_screen_env_override(&mut config, Some("true".to_string())); + let lint = config.lint_screen.expect( + "env=true should construct default LintScreenConfig when [lint_screen] section absent", + ); + assert!(lint.enabled); + assert!(lint.exe_path.is_none()); + assert!(lint.model.is_none()); + } + + #[test] + fn apply_env_override_force_enable_overwrites_toml_false() { + let mut config = make_config_with_lint_screen(false); + apply_lint_screen_env_override(&mut config, Some("1".to_string())); + assert!(config.lint_screen.unwrap().enabled); + } + + #[test] + fn apply_env_override_respect_toml_keeps_toml_enabled_true() { + let mut config = make_config_with_lint_screen(true); + apply_lint_screen_env_override(&mut config, Some("false".to_string())); + assert!( + config.lint_screen.unwrap().enabled, + "env=false should respect TOML (TOML had enabled=true, must remain true)" + ); + } + + #[test] + fn apply_env_override_respect_toml_keeps_toml_enabled_false() { + let mut config = make_config_with_lint_screen(false); + apply_lint_screen_env_override(&mut config, Some("".to_string())); + assert!(!config.lint_screen.unwrap().enabled); + } + + #[test] + fn apply_env_override_unset_keeps_toml_section_absent() { + let mut config = make_config_without_lint_screen(); + apply_lint_screen_env_override(&mut config, None); + assert!( + config.lint_screen.is_none(), + "env unset + [lint_screen] absent should remain None" + ); + } + + #[test] + fn apply_env_override_invalid_value_respects_toml() { + let mut config = make_config_with_lint_screen(false); + apply_lint_screen_env_override(&mut config, Some("maybe".to_string())); + assert!( + !config.lint_screen.unwrap().enabled, + "invalid env value should treat as RespectToml (TOML enabled=false preserved)" + ); + } +} diff --git a/src/cli-push-runner/src/config/mod.rs b/src/cli-push-runner/src/config/mod.rs new file mode 100644 index 00000000..669c8112 --- /dev/null +++ b/src/cli-push-runner/src/config/mod.rs @@ -0,0 +1,341 @@ +use serde::Deserialize; +use std::path::{Path, PathBuf}; + +mod lint_screen; +mod pr_size_check; +mod scratch_file_warning; + +pub(crate) use lint_screen::{ + LintScreenConfig, DEFAULT_LINT_SCREEN_ENDPOINT, DEFAULT_LINT_SCREEN_EXE_PATH, + DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, DEFAULT_LINT_SCREEN_MODEL, DEFAULT_LINT_SCREEN_OUTPUT_PATH, + DEFAULT_LINT_SCREEN_TIMEOUT_SECS, +}; +pub(crate) use pr_size_check::{ + PrSizeCheckConfig, DEFAULT_PR_SIZE_BASE_BRANCH, DEFAULT_PR_SIZE_BLOCK_THRESHOLD, + DEFAULT_PR_SIZE_WARNING_THRESHOLD, +}; +pub(crate) use scratch_file_warning::ScratchFileWarningConfig; + +use lint_screen::{apply_lint_screen_env_override, ENV_LINT_SCREEN_ENABLED}; + +pub(crate) const DEFAULT_STEP_TIMEOUT_SECS: u64 = 120; +pub(crate) const DEFAULT_PUSH_TIMEOUT_SECS: u64 = 300; + +#[derive(Deserialize)] +pub(crate) struct Config { + pub(crate) quality_gate: QualityGateConfig, + pub(crate) diff: Option, + pub(crate) lint_screen: Option, + pub(crate) takt: TaktConfig, + pub(crate) push: PushConfig, + pub(crate) scratch_file_warning: Option, + pub(crate) pr_size_check: Option, +} + +#[derive(Deserialize)] +pub(crate) struct QualityGateConfig { + pub(crate) parallel: Option, + pub(crate) step_timeout: Option, + pub(crate) groups: Vec, +} + +#[derive(Deserialize, Clone)] +pub(crate) struct GroupConfig { + pub(crate) name: String, + pub(crate) pre: Option, + pub(crate) commands: Vec, +} + +#[derive(Deserialize)] +pub(crate) struct TaktConfig { + pub(crate) workflow: String, + pub(crate) task: String, + pub(crate) extra_args: Option>, +} + +#[derive(Deserialize)] +pub(crate) struct DiffConfig { + pub(crate) command: String, + pub(crate) output_path: String, +} + +#[derive(Deserialize)] +pub(crate) struct PushConfig { + pub(crate) command: String, + pub(crate) timeout: Option, +} + +/// `push-runner-config.toml` の探索順序: カレントディレクトリ (pnpm scripts は +/// リポジトリルートで実行される) を優先し、無ければ exe 隣接パスに fallback する。 +pub(crate) fn config_path() -> PathBuf { + let filename = "push-runner-config.toml"; + let cwd_path = Path::new(filename).to_path_buf(); + if cwd_path.exists() { + return cwd_path; + } + exe_adjacent_config_path(filename) +} + +/// exe と同じディレクトリ (`.claude/` 配置パターン) 上の config path を返す。 +/// `config_path` が cwd に見つからなかった場合の fallback。 +fn exe_adjacent_config_path(filename: &str) -> PathBuf { + std::env::current_exe() + .unwrap_or_default() + .parent() + .unwrap_or(Path::new(".")) + .join(filename) +} + +pub(crate) fn load_config() -> Result { + let path = config_path(); + let content = std::fs::read_to_string(&path) + .map_err(|e| format!("設定ファイルの読み込みに失敗: {} ({})", path.display(), e))?; + let mut config: Config = + toml::from_str(&content).map_err(|e| format!("設定ファイルのパースに失敗: {}", e))?; + apply_lint_screen_env_override(&mut config, std::env::var(ENV_LINT_SCREEN_ENABLED).ok()); + validate_config(&config)?; + Ok(config) +} + +fn validate_config(config: &Config) -> Result<(), String> { + if config.quality_gate.groups.is_empty() { + return Err("設定ファイルエラー: quality_gate.groups が空です".into()); + } + for group in &config.quality_gate.groups { + if group.commands.is_empty() { + return Err(format!( + "設定ファイルエラー: group '{}' の commands が空です", + group.name + )); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_parses_full_without_diff() { + let toml_str = r#" +[quality_gate] +parallel = true +step_timeout = 60 + +[[quality_gate.groups]] +name = "lint" +commands = ["pnpm lint"] + +[[quality_gate.groups]] +name = "test" +pre = "pnpm install" +commands = ["pnpm test", "pnpm test:e2e"] + +[takt] +workflow = "pre-push-review" +task = "pre-push review" +extra_args = ["--pipeline", "--skip-git"] + +[push] +command = "jj git push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + + assert_eq!(config.quality_gate.parallel, Some(true)); + assert_eq!(config.quality_gate.step_timeout, Some(60)); + assert_eq!(config.quality_gate.groups.len(), 2); + assert!(config.diff.is_none()); + + assert_eq!(config.takt.workflow, "pre-push-review"); + assert_eq!(config.takt.task, "pre-push review"); + assert_eq!(config.takt.extra_args.as_ref().unwrap().len(), 2); + + assert_eq!(config.push.command, "jj git push"); + assert!(config.push.timeout.is_none()); + } + + #[test] + fn config_push_timeout_explicit() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "jj git push" +timeout = 600 +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert_eq!(config.push.timeout, Some(600)); + assert_eq!( + config.push.timeout.unwrap_or(DEFAULT_PUSH_TIMEOUT_SECS), + 600, + ); + } + + #[test] + fn config_push_timeout_defaults() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(config.push.timeout.is_none()); + assert_eq!( + config.push.timeout.unwrap_or(DEFAULT_PUSH_TIMEOUT_SECS), + DEFAULT_PUSH_TIMEOUT_SECS, + ); + } + + #[test] + fn config_parses_with_diff() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[diff] +command = "jj diff -r @" +output_path = ".takt/review-diff.txt" + +[takt] +workflow = "pre-push-review" +task = "pre-push review" + +[push] +command = "jj git push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + + let diff = config.diff.unwrap(); + assert_eq!(diff.command, "jj diff -r @"); + assert_eq!(diff.output_path, ".takt/review-diff.txt"); + } + + #[test] + fn config_quality_gate_defaults() { + let toml_str = r#" +[quality_gate] + +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(config.quality_gate.parallel.unwrap_or(true)); + assert_eq!( + config + .quality_gate + .step_timeout + .unwrap_or(DEFAULT_STEP_TIMEOUT_SECS), + DEFAULT_STEP_TIMEOUT_SECS, + ); + assert!(config.takt.extra_args.is_none()); + } + + #[test] + fn config_pre_field_optional() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "no-pre" +commands = ["echo test"] + +[[quality_gate.groups]] +name = "with-pre" +pre = "echo install" +commands = ["echo test"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(config.quality_gate.groups[0].pre.is_none()); + assert!(config.quality_gate.groups[1].pre.is_some()); + } + + #[test] + fn validate_rejects_empty_groups() { + let config = Config { + quality_gate: QualityGateConfig { + parallel: None, + step_timeout: None, + groups: vec![], + }, + diff: None, + lint_screen: None, + scratch_file_warning: None, + pr_size_check: None, + takt: TaktConfig { + workflow: "w".into(), + task: "t".into(), + extra_args: None, + }, + push: PushConfig { + command: "echo".into(), + timeout: None, + }, + }; + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("groups が空")); + } + + #[test] + fn validate_rejects_empty_commands() { + let config = Config { + quality_gate: QualityGateConfig { + parallel: None, + step_timeout: None, + groups: vec![GroupConfig { + name: "empty".into(), + pre: None, + commands: vec![], + }], + }, + diff: None, + lint_screen: None, + scratch_file_warning: None, + pr_size_check: None, + takt: TaktConfig { + workflow: "w".into(), + task: "t".into(), + extra_args: None, + }, + push: PushConfig { + command: "echo".into(), + timeout: None, + }, + }; + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("'empty'")); + } +} diff --git a/src/cli-push-runner/src/config/pr_size_check.rs b/src/cli-push-runner/src/config/pr_size_check.rs new file mode 100644 index 00000000..4c73885e --- /dev/null +++ b/src/cli-push-runner/src/config/pr_size_check.rs @@ -0,0 +1,123 @@ +use serde::Deserialize; + +/// 順位 151 (Bundle "既存ルール仕組み化"): PR diff 行数 (insertions + deletions) の +/// warning 閾値。`~/.claude/rules/common/git-workflow.md` § Multi-PR chaining の +/// 「1 PR あたり 250-800 lines」目安に同期。 +pub(crate) const DEFAULT_PR_SIZE_WARNING_THRESHOLD: usize = 800; + +/// 順位 151: PR diff 行数の block 閾値。これを超えると push を停止する。 +/// 大型 refactoring 時は config / env override で意図的バイパス。 +pub(crate) const DEFAULT_PR_SIZE_BLOCK_THRESHOLD: usize = 1500; + +/// 順位 151: PR base の default branch 名。`format!("{}..@", default_branch)` で +/// revset 組立 (rule⑫ `no-hardcoded-jj-revset-range` 適用)。 +pub(crate) const DEFAULT_PR_SIZE_BASE_BRANCH: &str = "master"; + +/// 順位 151 (Bundle "既存ルール仕組み化") — PR diff size を `jj diff --stat` で計測し +/// warning / block する pre-push stage の config。 +/// +/// `~/.claude/rules/common/git-workflow.md` § Multi-PR chaining の「1 PR あたり +/// 250-800 lines」目安を決定論的に維持する。 +/// +/// ADR-039 (Experimental feature 標準パターン) 3 点セット準拠: +/// - **Config opt-in**: 試験運用のため default `enabled = false`。`[pr_size_check]` section +/// 不在 / `enabled` 未設定 / `enabled = false` のいずれも検査を完全 skip。 +/// - **Kill-switch**: `enabled = false` (TOML) + env override `PR_SIZE_CHECK_OVERRIDE=1` で +/// 個別 push の意図的バイパス可能 (大型 refactoring 時)。 +/// - **Bounded lifetime**: 3-5 PR の dogfood で false positive / 検出効果を観測後、 +/// default-ON 昇格 or 却下を判定。判定結果は `src/cli-push-runner/src/stages/pr_size_check.rs` +/// module doc + `push-runner-config.toml` の `[pr_size_check]` section コメントに反映する。 +/// +/// revset は `format!("{}..@", default_branch)` 形式で組立 (rule⑫ +/// `no-hardcoded-jj-revset-range` 適用、alternative branch "main" 等への切替を保護)。 +#[derive(Deserialize)] +pub(crate) struct PrSizeCheckConfig { + pub(crate) enabled: Option, + pub(crate) default_branch: Option, + pub(crate) warning_threshold: Option, + pub(crate) block_threshold: Option, +} + +#[cfg(test)] +mod tests { + use crate::config::Config; + + #[test] + fn config_parses_with_pr_size_check_full() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[pr_size_check] +enabled = true +default_branch = "main" +warning_threshold = 500 +block_threshold = 2000 + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let s = config + .pr_size_check + .expect("[pr_size_check] should parse to Some"); + assert_eq!(s.enabled, Some(true)); + assert_eq!(s.default_branch.as_deref(), Some("main")); + assert_eq!(s.warning_threshold, Some(500)); + assert_eq!(s.block_threshold, Some(2000)); + } + + #[test] + fn config_pr_size_check_absent_yields_none() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!( + config.pr_size_check.is_none(), + "absent [pr_size_check] should yield None (default OFF lane)" + ); + } + + #[test] + fn config_pr_size_check_only_enabled_false() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[pr_size_check] +enabled = false + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let s = config.pr_size_check.unwrap(); + assert_eq!(s.enabled, Some(false)); + assert!(s.default_branch.is_none()); + assert!(s.warning_threshold.is_none()); + assert!(s.block_threshold.is_none()); + } +} diff --git a/src/cli-push-runner/src/config/scratch_file_warning.rs b/src/cli-push-runner/src/config/scratch_file_warning.rs new file mode 100644 index 00000000..f3a4c5ff --- /dev/null +++ b/src/cli-push-runner/src/config/scratch_file_warning.rs @@ -0,0 +1,99 @@ +use serde::Deserialize; + +/// 順位 1 (PR #85 T1-4) — scratch ファイル (`__*` 等) が `@` commit に +/// 混入していないか push 前に検査する stage の config。 +/// +/// ADR-039 (Experimental feature 標準パターン) § 1 Config opt-in 準拠: +/// `[scratch_file_warning]` section 不在 / `enabled` 未設定 / `enabled = false` +/// のいずれも検査を **skip** (= default `enabled = false`)。明示的に `enabled = true` +/// にしたときのみ検査実行 (3-5 PR の dogfood 後に default-ON 昇格 or 却下を判定)。 +/// +/// `patterns` は順位 5 (AI 生成一時スクリプト pattern の pre-push 検出) で +/// `_tmp_*` 等の追加 pattern を config-driven で拡張可能 (= 補完アプローチ)。 +/// `patterns` 未設定時の default は `["__*"]` (= stage 側 `DEFAULT_PATTERN`)。 +#[derive(Deserialize)] +pub(crate) struct ScratchFileWarningConfig { + pub(crate) enabled: Option, + pub(crate) patterns: Option>, +} + +#[cfg(test)] +mod tests { + use crate::config::Config; + + #[test] + fn config_parses_with_scratch_file_warning_full() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[scratch_file_warning] +enabled = true +patterns = ["__*", "_tmp_*"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let s = config + .scratch_file_warning + .expect("[scratch_file_warning] should parse to Some"); + assert_eq!(s.enabled, Some(true)); + assert_eq!( + s.patterns.unwrap(), + vec!["__*".to_string(), "_tmp_*".to_string()] + ); + } + + #[test] + fn config_parses_with_scratch_file_warning_only_enabled_false() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[scratch_file_warning] +enabled = false + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let s = config.scratch_file_warning.unwrap(); + assert_eq!(s.enabled, Some(false)); + assert!(s.patterns.is_none()); + } + + #[test] + fn config_scratch_file_warning_absent_yields_none() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!( + config.scratch_file_warning.is_none(), + "absent [scratch_file_warning] should yield None (default-ON 動作は stage 側で解決)" + ); + } +} diff --git a/src/cli-push-runner/src/stages/lint_screen.rs b/src/cli-push-runner/src/stages/lint_screen.rs deleted file mode 100644 index 7353cc27..00000000 --- a/src/cli-push-runner/src/stages/lint_screen.rs +++ /dev/null @@ -1,982 +0,0 @@ -//! Phase c (§8.E lint screen facet) stage -//! -//! diff を mistral:7b に流して lint 一次フィルタの所見を markdown として出力する。 -//! 設計詳細: `docs/adr/adr-038-local-llm-finding-classification.md` -//! -//! 動作モード: -//! - `enabled = false` (default): 完全 no-op、push pipeline は影響を受けない -//! - `enabled = true`: cli-finding-classifier.exe を subprocess で起動、stdin に diff を流し -//! stdout の LintScreenResult JSON を markdown に整形して output_path に書き出す -//! -//! Phase b' で agreement 75% (conditional GO) のため、本 stage は **gating しない**。 -//! Ollama down / timeout / diff 過大 / JSON parse 失敗 等のエラーは全て skip + warn で処理し、 -//! push pipeline をブロックしない。 - -use std::io::Write; -use std::path::Path; -use std::process::{Command, Stdio}; -use std::time::Instant; - -use crate::config::{ - LintScreenConfig, DEFAULT_LINT_SCREEN_ENDPOINT, DEFAULT_LINT_SCREEN_EXE_PATH, - DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, DEFAULT_LINT_SCREEN_MODEL, DEFAULT_LINT_SCREEN_OUTPUT_PATH, - DEFAULT_LINT_SCREEN_TIMEOUT_SECS, -}; -use crate::log::log_stage; -use lib_subprocess::wait_with_timeout_basic; - -const STAGE: &str = "lint-screen"; - -struct InvokeParams<'a> { - exe: &'a str, - model: &'a str, - endpoint: &'a str, - timeout_secs: u64, -} - -fn resolve_invoke_params(config: &LintScreenConfig) -> InvokeParams<'_> { - InvokeParams { - exe: config - .exe_path - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_EXE_PATH), - model: config.model.as_deref().unwrap_or(DEFAULT_LINT_SCREEN_MODEL), - endpoint: config - .endpoint - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_ENDPOINT), - timeout_secs: config - .timeout_secs - .unwrap_or(DEFAULT_LINT_SCREEN_TIMEOUT_SECS), - } -} - -pub(crate) fn run_lint_screen(config: &LintScreenConfig, diff_path: &str) { - if !config.enabled { - return; - } - - let started = Instant::now(); - log_stage(STAGE, "実行中 (試験運用、エラーは skip + warn)"); - - let raw_diff = match read_diff(diff_path, config) { - Ok(d) => d, - Err(reason) => { - log_stage(STAGE, &format!("skip: {}", reason)); - return; - } - }; - - let output_path = config - .output_path - .as_deref() - .unwrap_or(DEFAULT_LINT_SCREEN_OUTPUT_PATH); - - let diff = match filter_excluded_hunks(&raw_diff) { - FilterResult::Kept(filtered) => filtered, - FilterResult::AllExcluded => { - log_stage( - STAGE, - "skip: docs-only diff (`.md`/`.markdown` のみ)、Bundle k 順位 123", - ); - write_skip_report_logged(output_path); - return; - } - }; - - let diff = strip_diff_metadata_lines(&diff); - - invoke_and_write_report(config, output_path, &diff, started); -} - -/// classifier 呼び出し + report 書き出しを 1 ステップにまとめた helper。 -/// -/// `run_lint_screen` を 50 行ガイドラインに収めるための機能分離。 -fn invoke_and_write_report( - config: &LintScreenConfig, - output_path: &str, - diff: &str, - started: Instant, -) { - let params = resolve_invoke_params(config); - let output = match invoke_classifier(¶ms, diff) { - Ok(o) => o, - Err(reason) => { - log_stage(STAGE, &format!("skip: classifier {}", reason)); - return; - } - }; - - match write_report(output_path, &output.stdout, &output.stderr) { - Ok(()) => log_stage( - STAGE, - &format!( - "出力: {} ({:.0}s)", - output_path, - started.elapsed().as_secs_f64() - ), - ), - Err(e) => log_stage(STAGE, &format!("skip: report 書き出し失敗: {}", e)), - } -} - -fn read_diff(diff_path: &str, config: &LintScreenConfig) -> Result { - let raw = std::fs::read_to_string(diff_path) - .map_err(|e| format!("diff 読み込み失敗 ({}): {}", diff_path, e))?; - let max_lines = config - .max_diff_lines - .unwrap_or(DEFAULT_LINT_SCREEN_MAX_DIFF_LINES); - let lines = raw.lines().count(); - if lines > max_lines { - return Err(format!("diff 過大 ({} 行 > 上限 {})", lines, max_lines)); - } - if raw.trim().is_empty() { - return Err("diff が空".to_string()); - } - Ok(raw) -} - -struct ClassifierOutput { - stdout: String, - stderr: String, -} - -fn invoke_classifier(params: &InvokeParams<'_>, diff: &str) -> Result { - if !Path::new(params.exe).exists() { - return Err(format!("exe 不在 ({})", params.exe)); - } - - let timeout_str = params.timeout_secs.to_string(); - let mut child = Command::new(params.exe) - .args([ - "--mode", - "lint-screen", - "--model", - params.model, - "--endpoint", - params.endpoint, - "--timeout-secs", - &timeout_str, - ]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|e| format!("spawn 失敗: {}", e))?; - - if let Some(mut stdin) = child.stdin.take() { - stdin - .write_all(diff.as_bytes()) - .map_err(|e| format!("stdin 書き込み失敗: {}", e))?; - } - - let stdout_handle = lib_subprocess::drain_pipe_capped( - child.stdout.take().expect("stdout piped"), - crate::runner::MAX_LINES, - ); - let stderr_handle = lib_subprocess::drain_pipe_capped( - child.stderr.take().expect("stderr piped"), - crate::runner::MAX_LINES, - ); - - let exit = wait_with_timeout_basic(STAGE, &mut child, params.timeout_secs + 5) - .map_err(|e| format!("wait 失敗: {}", e))?; - let stdout = stdout_handle.join().unwrap_or_default(); - let stderr = stderr_handle.join().unwrap_or_default(); - - match exit { - None => Err(format!("timeout ({}s)", params.timeout_secs + 5)), - Some(status) if !status.success() => Err(format!("非 0 終了: {}", stderr)), - Some(_) if stdout.trim().is_empty() => Err("stdout 空".to_string()), - Some(_) => Ok(ClassifierOutput { stdout, stderr }), - } -} - -fn write_report(output_path: &str, classifier_json: &str, stderr: &str) -> Result<(), String> { - let path = Path::new(output_path); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| format!("ディレクトリ作成失敗: {}", e))?; - } - let markdown = format_report(classifier_json, stderr); - std::fs::write(path, markdown).map_err(|e| format!("write: {}", e)) -} - -const REPORT_PREAMBLE: &str = "# Lint Screen Report (mistral:7b, Phase b' agreement 75%)\n\n\ -> **試験運用**: 本 report は ADR-038 Phase c lint screen facet による mistral:7b の AI 所見。\n\ -> agreement 75% (conditional GO) のため誤指摘あり。reviewer が独立判断する前提で参考情報として扱う。\n\n"; - -fn render_parse_error(err: &serde_json::Error, raw: &str) -> String { - let mut out = String::from(REPORT_PREAMBLE); - out.push_str(&format!("## JSON parse 失敗\n\nerror: {}\n\n", err)); - out.push_str("```\n"); - out.push_str(raw); - out.push_str("\n```\n"); - out -} - -fn render_summary(decision: &str, findings_count: usize, fallback_reason: &str) -> String { - let mut s = format!( - "## Summary\n\n- screen_decision: `{}`\n- findings: {}\n", - decision, findings_count - ); - if !fallback_reason.is_empty() { - s.push_str(&format!("- fallback_reason: `{}`\n", fallback_reason)); - } - s.push('\n'); - s -} - -fn render_findings_table(findings: &[serde_json::Value]) -> String { - if findings.is_empty() { - return "## Findings\n\n(なし)\n".to_string(); - } - let mut out = - String::from("## Findings\n\n| severity | rule | file | line | issue | suggestion |\n"); - out.push_str("|---|---|---|---|---|---|\n"); - for f in findings { - let s = f.get("severity").and_then(|v| v.as_str()).unwrap_or("?"); - let r = f.get("rule").and_then(|v| v.as_str()).unwrap_or("?"); - let file = f.get("file").and_then(|v| v.as_str()).unwrap_or("?"); - let line = f - .get("line") - .map(|v| v.to_string()) - .unwrap_or_else(|| "?".to_string()); - let issue = f.get("issue").and_then(|v| v.as_str()).unwrap_or(""); - let sug = f.get("suggestion").and_then(|v| v.as_str()).unwrap_or(""); - out.push_str(&format!( - "| {} | {} | {} | {} | {} | {} |\n", - sanitize_cell(s), - sanitize_cell(r), - sanitize_cell(file), - sanitize_cell(&line), - sanitize_cell(issue), - sanitize_cell(sug), - )); - } - out -} - -fn format_report(classifier_json: &str, stderr: &str) -> String { - let value: serde_json::Value = match serde_json::from_str(classifier_json) { - Ok(v) => v, - Err(e) => { - let mut out = render_parse_error(&e, classifier_json); - out.push_str(&render_diagnostic(stderr)); - return out; - } - }; - let decision = value - .get("screen_decision") - .and_then(|v| v.as_str()) - .unwrap_or("?"); - let fallback_reason = value - .get("fallback_reason") - .and_then(|v| v.as_str()) - .unwrap_or(""); - let findings = value - .get("lint_findings") - .and_then(|v| v.as_array()) - .cloned() - .unwrap_or_default(); - - let mut out = String::from(REPORT_PREAMBLE); - out.push_str(&render_summary(decision, findings.len(), fallback_reason)); - out.push_str(&render_findings_table(&findings)); - out.push_str(&render_diagnostic(stderr)); - out -} - -fn render_diagnostic(stderr: &str) -> String { - let trimmed = stderr.trim(); - if trimmed.is_empty() { - return String::new(); - } - let mut out = String::from("\n## Diagnostic\n\n"); - out.push_str("classifier exe からの stderr 出力 (Phase A 順位 98 の num_ctx overflow 診断 log 等):\n\n"); - out.push_str("```text\n"); - out.push_str(trimmed); - out.push_str("\n```\n"); - out -} - -/// markdown table cell 用に `|` と改行を escape する。 -fn sanitize_cell(s: &str) -> String { - s.replace('|', "\\|").replace('\n', " ") -} - -/// lint_screen の対象外とする拡張子 (lowercase で比較)。 -/// -/// 由来 (Bundle k 順位 123): mistral:7b が docs-only diff や `.md` ファイルに対して -/// Rust の `unused-import` を hallucinate する FP が PR #148/#150/#151/#152/#153 で -/// 5 PR 連続観測された。diff 段階で `.md` / `.markdown` ハンクを drop することで -/// この failure mode を構造的に解消する (ADR-038 §Known failure mode 参照)。 -const EXCLUDED_EXTENSIONS: &[&str] = &["md", "markdown"]; - -/// `filter_excluded_hunks` の戻り値。Markdown 100% の diff は invoke を完全に -/// skip して別 path (skip-report 書き出し + 短絡 return) に流す必要があるため、 -/// 通常 case (`Kept`) と区別する enum を返す。 -enum FilterResult { - Kept(String), - AllExcluded, -} - -/// 入力 diff から `EXCLUDED_EXTENSIONS` 拡張子のハンクを除外する。 -/// -/// 戻り値: -/// - `FilterResult::Kept(text)`: 1 件以上の対象外ハンクが残った場合、その diff text -/// - `FilterResult::AllExcluded`: 全ハンクが対象外拡張子だった (= docs-only diff) 場合 -/// -/// 実装方針: `diff --git ` 行を file-diff の境界として 1 ハンク = 1 chunk に分割、 -/// 各 chunk の `+++ b/` (なければ `--- a/`) から拡張子を取り出して判定。 -/// 拡張子は ASCII lowercase 比較 (= 大文字 `.MD` / `.Markdown` も除外対象に含む)。 -fn filter_excluded_hunks(raw_diff: &str) -> FilterResult { - let chunks = split_into_file_diffs(raw_diff); - if chunks.is_empty() { - return FilterResult::Kept(raw_diff.to_string()); - } - let kept: Vec<&str> = chunks - .iter() - .filter(|chunk| !chunk_has_excluded_extension(chunk)) - .copied() - .collect(); - if kept.is_empty() { - return FilterResult::AllExcluded; - } - FilterResult::Kept(kept.join("")) -} - -/// diff text を `diff --git ` 行を境界に file-diff chunks に分割する。 -/// -/// 行頭の `diff --git ` のみを境界とみなす。chunk 末尾は次の境界直前 (改行込み)。 -/// 入力が `diff --git ` で始まらない場合 (= unified diff fragment ではない可能性)、 -/// 空 vec を返して caller が原文 fallthrough する。 -fn split_into_file_diffs(raw_diff: &str) -> Vec<&str> { - if !raw_diff.starts_with("diff --git ") { - return Vec::new(); - } - let mut chunks = Vec::new(); - let mut chunk_start = 0; - for (idx, _) in raw_diff.match_indices("\ndiff --git ") { - let end = idx + 1; - chunks.push(&raw_diff[chunk_start..end]); - chunk_start = end; - } - chunks.push(&raw_diff[chunk_start..]); - chunks -} - -/// chunk 内の `+++ b/` (new path) を優先して拡張子を抽出する。 -/// new path が無い場合 (= delete 操作で `+++ /dev/null` のケース) のみ -/// `--- a/` (old path) にフォールバック。`EXCLUDED_EXTENSIONS` に -/// 該当すれば true を返す。 -/// -/// 新パス優先の根拠 (CR #155 Major 指摘): unified diff の慣例では `--- a/` -/// が `+++ b/` より先に出現するため、単純な `find_map` で両者を OR にすると -/// 旧パスが優先されてしまう。これだと `*.rs → *.md` の rename で **新パス側が `.md` -/// にも関わらず旧 `.rs` 拡張子で判定**され、Markdown 除外が機能しない bug が生じる。 -/// new path を chunk 全体から先に探し、無い場合のみ old path に落とす。 -fn chunk_has_excluded_extension(chunk: &str) -> bool { - let new_path = chunk.lines().find_map(|line| line.strip_prefix("+++ b/")); - let old_path = chunk.lines().find_map(|line| line.strip_prefix("--- a/")); - let path = new_path.or(old_path).unwrap_or(""); - if path.is_empty() { - return false; - } - let ext = path.rsplit('.').next().unwrap_or("").to_ascii_lowercase(); - EXCLUDED_EXTENSIONS.contains(&ext.as_str()) -} - -/// git diff の metadata 行を strip して LLM 入力の signal/noise 比を改善する。 -/// -/// 由来 (Bundle l 順位 132): mistral:7b が `similarity index 100%` の `100%` を -/// magic-number として false positive 検出する事象が PR #155 (Bundle k-1) / -/// PR #156 (Phase E) で観測された。git diff metadata 行は file rename / move を含む -/// PR で必ず出現するため、LLM 入力前に決定論的に除去することで構造的 FP を解消する。 -/// -/// 除去対象 (lossless: 各行を空行に置き換えずに完全削除): -/// - `similarity index NN%` — rename / copy 時の similarity ratio (magic-number FP の主因) -/// - `dissimilarity index NN%` — 同上 (git 1.6.5+ 形式) -/// - `index ..[ ]` — blob hash + file mode (hex の連続が magic 化されやすい) -/// - `new file mode NNNNNN` / `deleted file mode NNNNNN` / `old mode NNNNNN` / -/// `new mode NNNNNN` — Unix mode の 6 桁数値も magic 化されやすい -/// - `rename from ` / `rename to ` — rename target は filter_excluded_hunks -/// が `+++ b/` で既に判定済 (情報量ゼロ) -/// - `copy from ` / `copy to ` — 同上 -/// -/// 保持: `diff --git ` (ハンク境界、file 識別) / `--- a/` / `+++ b/` (path 識別) / -/// `@@ ... @@` (hunk header、line range 情報は LLM が file 位置を理解するのに必要) / -/// `+` / `-` / ` ` (content 行)。 -fn strip_diff_metadata_lines(diff: &str) -> String { - diff.lines() - .filter(|line| !is_diff_metadata_line(line)) - .map(|line| { - let mut s = String::with_capacity(line.len() + 1); - s.push_str(line); - s.push('\n'); - s - }) - .collect() -} - -/// `strip_diff_metadata_lines` の per-line 判定。除去対象なら true。 -fn is_diff_metadata_line(line: &str) -> bool { - line.starts_with("similarity index ") - || line.starts_with("dissimilarity index ") - || line.starts_with("index ") - || line.starts_with("new file mode ") - || line.starts_with("deleted file mode ") - || line.starts_with("old mode ") - || line.starts_with("new mode ") - || line.starts_with("rename from ") - || line.starts_with("rename to ") - || line.starts_with("copy from ") - || line.starts_with("copy to ") -} - -/// `write_skip_report` を呼び出し、失敗時はステージログに記録する。 -/// -/// `run_lint_screen` のネスト深度を抑えるための分離 (match arm 内に if let を -/// 重ねないよう、エラー処理を 1 関数に閉じ込める)。 -fn write_skip_report_logged(output_path: &str) { - if let Err(e) = write_skip_report(output_path) { - log_stage(STAGE, &format!("skip: skip-report 書き出し失敗: {}", e)); - } -} - -/// 全ハンクが対象外拡張子だった場合に書き出す skip-report。invoke は完全に skip する。 -fn write_skip_report(output_path: &str) -> Result<(), String> { - let path = Path::new(output_path); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| format!("ディレクトリ作成失敗: {}", e))?; - } - let body = format!( - "{}## Summary\n\n- screen_decision: `skipped`\n- 理由: docs-only diff のため lint_screen はスキップしました \ - (`.md` / `.markdown` 拡張子のみで Rust hallucinate FP を構造的に防止、Bundle k 順位 123 / ADR-038)\n\n\ - ## Findings\n\n(なし)\n", - REPORT_PREAMBLE - ); - std::fs::write(path, body).map_err(|e| format!("write: {}", e)) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn format_report_renders_findings_table() { - let json = r#"{ - "lint_findings": [ - {"severity":"minor","rule":"unused-import","file":"src/a.rs","line":1,"issue":"x","suggestion":"y"} - ], - "screen_decision":"auto_fix" - }"#; - let md = format_report(json, ""); - assert!(md.contains("auto_fix")); - assert!(md.contains("unused-import")); - assert!(md.contains("src/a.rs")); - assert!(md.contains("| severity | rule | file | line | issue | suggestion |")); - } - - #[test] - fn format_report_handles_empty_findings() { - let json = r#"{"lint_findings":[],"screen_decision":"informational"}"#; - let md = format_report(json, ""); - assert!(md.contains("informational")); - assert!(md.contains("(なし)")); - } - - #[test] - fn format_report_recovers_from_invalid_json() { - let md = format_report("not json", ""); - assert!(md.contains("JSON parse 失敗")); - assert!(md.contains("not json")); - } - - #[test] - fn format_report_includes_diagnostic_section_when_stderr_non_empty() { - let json = r#"{ - "lint_findings": [], - "screen_decision": "human_review", - "fallback_reason": "ollama error: JSON parse error" - }"#; - let stderr = "[lib-ollama-client] WARN: Ollama JSON output may be truncated.\n prompt_eval_count: 8192 (vs num_ctx: 8192)"; - let md = format_report(json, stderr); - assert!(md.contains("## Diagnostic")); - assert!(md.contains("prompt_eval_count: 8192")); - assert!(md.contains("num_ctx: 8192")); - } - - #[test] - fn format_report_skips_diagnostic_section_when_stderr_empty() { - let json = r#"{ - "lint_findings": [], - "screen_decision": "informational" - }"#; - let md = format_report(json, ""); - assert!(!md.contains("## Diagnostic")); - assert!(!md.contains("classifier exe からの stderr")); - } - - #[test] - fn format_report_skips_diagnostic_section_when_stderr_whitespace_only() { - let json = r#"{ - "lint_findings": [], - "screen_decision": "informational" - }"#; - let md = format_report(json, " \n\n "); - assert!(!md.contains("## Diagnostic")); - } - - #[test] - fn format_report_appends_diagnostic_to_parse_error_path() { - let stderr = "[lib-ollama-client] WARN: truncated"; - let md = format_report("not json", stderr); - assert!(md.contains("JSON parse 失敗")); - assert!(md.contains("## Diagnostic")); - assert!(md.contains("[lib-ollama-client] WARN")); - } - - #[test] - fn format_report_includes_fallback_reason_when_present() { - let json = r#"{ - "lint_findings":[], - "screen_decision":"human_review", - "fallback_reason":"ollama error: empty" - }"#; - let md = format_report(json, ""); - assert!(md.contains("fallback_reason")); - assert!(md.contains("ollama error")); - } - - #[test] - fn sanitize_cell_escapes_pipe_and_newline() { - assert_eq!(sanitize_cell("a|b"), "a\\|b"); - assert_eq!(sanitize_cell("a\nb"), "a b"); - } - - #[test] - fn render_findings_table_sanitizes_pipe_in_all_columns() { - let json = r#"{ - "lint_findings": [ - {"severity":"mi|nor","rule":"un|used","file":"src/a|b.rs","line":1,"issue":"x|y","suggestion":"y|z"} - ], - "screen_decision":"auto_fix" - }"#; - let md = format_report(json, ""); - assert!(!md.contains("mi|nor"), "severity must be sanitized"); - assert!(md.contains("mi\\|nor")); - assert!(!md.contains("un|used"), "rule must be sanitized"); - assert!(md.contains("un\\|used")); - assert!(!md.contains("a|b.rs"), "file must be sanitized"); - assert!(md.contains("a\\|b.rs")); - } - - #[test] - fn run_lint_screen_is_noop_when_disabled() { - let cfg = LintScreenConfig { - enabled: false, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: None, - output_path: None, - }; - run_lint_screen(&cfg, "/nonexistent/diff/path"); - } - - #[test] - fn read_diff_returns_error_on_missing_file() { - let cfg = LintScreenConfig { - enabled: true, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: None, - output_path: None, - }; - let result = read_diff("/nonexistent/path.txt", &cfg); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("diff 読み込み失敗")); - } - - #[test] - fn read_diff_returns_error_when_diff_exceeds_limit() { - let path = std::env::temp_dir().join("test-lint-screen-large-diff.txt"); - let large = "x\n".repeat(100); - std::fs::write(&path, large).unwrap(); - - let cfg = LintScreenConfig { - enabled: true, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: Some(50), - output_path: None, - }; - let result = read_diff(path.to_str().unwrap(), &cfg); - assert!(result.is_err()); - let err = result.unwrap_err(); - assert!(err.contains("diff 過大")); - assert!(err.contains("100")); - } - - #[test] - fn read_diff_returns_error_on_empty_diff() { - let path = std::env::temp_dir().join("test-lint-screen-empty-diff.txt"); - std::fs::write(&path, "").unwrap(); - let cfg = LintScreenConfig { - enabled: true, - exe_path: None, - model: None, - endpoint: None, - timeout_secs: None, - max_diff_lines: None, - output_path: None, - }; - let result = read_diff(path.to_str().unwrap(), &cfg); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("空")); - } - - fn rust_chunk(path: &str) -> String { - format!( - "diff --git a/{path} b/{path}\n\ - index abc..def 100644\n\ - --- a/{path}\n\ - +++ b/{path}\n\ - @@ -1,1 +1,1 @@\n\ - -old\n\ - +new\n", - path = path - ) - } - - fn md_chunk(path: &str) -> String { - format!( - "diff --git a/{path} b/{path}\n\ - index abc..def 100644\n\ - --- a/{path}\n\ - +++ b/{path}\n\ - @@ -1,1 +1,1 @@\n\ - -# heading\n\ - +# heading updated\n", - path = path - ) - } - - fn assert_kept(result: FilterResult) -> String { - match result { - FilterResult::Kept(text) => text, - FilterResult::AllExcluded => panic!("expected Kept, got AllExcluded"), - } - } - - #[test] - fn filter_excluded_hunks_keeps_rust_only_diff_unchanged() { - let diff = rust_chunk("src/lib.rs"); - let result = assert_kept(filter_excluded_hunks(&diff)); - assert_eq!(result, diff); - } - - #[test] - fn filter_excluded_hunks_drops_md_hunk_from_mixed_diff() { - let rust = rust_chunk("src/main.rs"); - let md = md_chunk("docs/README.md"); - let combined = format!("{}{}", rust, md); - let kept = assert_kept(filter_excluded_hunks(&combined)); - assert!(kept.contains("src/main.rs")); - assert!(!kept.contains("docs/README.md")); - } - - #[test] - fn filter_excluded_hunks_signals_all_excluded_for_pure_markdown_diff() { - let diff = format!( - "{}{}", - md_chunk("docs/a.md"), - md_chunk("docs/b.markdown") - ); - match filter_excluded_hunks(&diff) { - FilterResult::AllExcluded => {} - FilterResult::Kept(_) => panic!("expected AllExcluded for pure .md/.markdown diff"), - } - } - - #[test] - fn filter_excluded_hunks_treats_markdown_extension_case_insensitively() { - let diff = format!("{}{}", md_chunk("README.MD"), md_chunk("notes.Markdown")); - match filter_excluded_hunks(&diff) { - FilterResult::AllExcluded => {} - FilterResult::Kept(_) => panic!("uppercase .MD / mixed-case .Markdown must be excluded"), - } - } - - #[test] - fn filter_excluded_hunks_keeps_path_with_md_in_middle_not_extension() { - let diff = rust_chunk("src/something.mdxyz.rs"); - let kept = assert_kept(filter_excluded_hunks(&diff)); - assert_eq!(kept, diff); - } - - #[test] - fn filter_excluded_hunks_handles_non_diff_input_as_passthrough() { - let raw = "not a unified diff\njust raw text"; - let kept = assert_kept(filter_excluded_hunks(raw)); - assert_eq!(kept, raw); - } - - #[test] - fn filter_excluded_hunks_keeps_dev_null_create_path() { - let diff = "diff --git a/src/new.rs b/src/new.rs\n\ - new file mode 100644\n\ - index 0000000..1234567\n\ - --- /dev/null\n\ - +++ b/src/new.rs\n\ - @@ -0,0 +1,1 @@\n\ - +pub fn x() {}\n"; - let kept = assert_kept(filter_excluded_hunks(diff)); - assert!(kept.contains("src/new.rs")); - } - - #[test] - fn filter_excluded_hunks_prefers_b_path_on_rename_to_markdown() { - let diff = "diff --git a/src/a.rs b/docs/a.md\n\ - similarity index 100%\n\ - rename from src/a.rs\n\ - rename to docs/a.md\n\ - --- a/src/a.rs\n\ - +++ b/docs/a.md\n\ - @@ -1,1 +1,1 @@\n\ - -old\n\ - +new\n"; - match filter_excluded_hunks(diff) { - FilterResult::AllExcluded => {} - FilterResult::Kept(_) => panic!( - "rename .rs -> .md must be excluded based on new path (CR #155 Major)" - ), - } - } - - #[test] - fn filter_excluded_hunks_keeps_rename_from_md_to_rust() { - let diff = "diff --git a/docs/old.md b/src/new.rs\n\ - similarity index 100%\n\ - rename from docs/old.md\n\ - rename to src/new.rs\n\ - --- a/docs/old.md\n\ - +++ b/src/new.rs\n\ - @@ -1,1 +1,1 @@\n\ - -old\n\ - +new\n"; - let kept = assert_kept(filter_excluded_hunks(diff)); - assert!( - kept.contains("src/new.rs"), - "rename .md -> .rs must be kept based on new path (symmetric to rename-to-md test)" - ); - } - - #[test] - fn filter_excluded_hunks_excludes_dev_null_delete_of_md() { - let diff = "diff --git a/docs/old.md b/docs/old.md\n\ - deleted file mode 100644\n\ - index 1234567..0000000\n\ - --- a/docs/old.md\n\ - +++ /dev/null\n\ - @@ -1,1 +0,0 @@\n\ - -# removed\n"; - match filter_excluded_hunks(diff) { - FilterResult::AllExcluded => {} - FilterResult::Kept(_) => panic!( - "delete of .md file should be excluded (--- a/ path is .md, +++ is /dev/null)" - ), - } - } - - #[test] - fn filter_excluded_hunks_preserves_hunk_boundaries_for_three_file_mixed() { - let diff = format!( - "{}{}{}", - rust_chunk("src/a.rs"), - md_chunk("docs/b.md"), - rust_chunk("src/c.rs"), - ); - let kept = assert_kept(filter_excluded_hunks(&diff)); - assert!(kept.contains("src/a.rs")); - assert!(!kept.contains("docs/b.md")); - assert!(kept.contains("src/c.rs")); - let lines: Vec<&str> = kept.lines().filter(|l| l.starts_with("diff --git ")).collect(); - assert_eq!(lines.len(), 2, "exactly 2 diff --git boundaries must remain"); - } - - #[test] - fn strip_diff_metadata_drops_similarity_index_line() { - let diff = "diff --git a/src/a.rs b/src/b.rs\n\ - similarity index 100%\n\ - rename from src/a.rs\n\ - rename to src/b.rs\n\ - --- a/src/a.rs\n\ - +++ b/src/b.rs\n\ - @@ -1,1 +1,1 @@\n\ - -old\n\ - +new\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!( - !stripped.contains("similarity index"), - "similarity index line must be stripped, got: {}", - stripped - ); - assert!(!stripped.contains("100%")); - assert!(!stripped.contains("rename from")); - assert!(!stripped.contains("rename to")); - } - - #[test] - fn strip_diff_metadata_preserves_hunk_boundaries_and_content() { - let diff = "diff --git a/src/x.rs b/src/x.rs\nindex abc1234..def5678 100644\n--- a/src/x.rs\n+++ b/src/x.rs\n@@ -1,2 +1,2 @@\n-fn old() {}\n+fn new() {}\n // unchanged context line\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(stripped.contains("diff --git ")); - assert!(stripped.contains("--- a/src/x.rs")); - assert!(stripped.contains("+++ b/src/x.rs")); - assert!(stripped.contains("@@ -1,2 +1,2 @@")); - assert!(stripped.contains("-fn old() {}")); - assert!(stripped.contains("+fn new() {}")); - assert!(stripped.contains(" // unchanged context line")); - assert!(!stripped.contains("index abc1234")); - } - - #[test] - fn strip_diff_metadata_drops_file_mode_lines() { - let diff = "diff --git a/script.sh b/script.sh\n\ - old mode 100644\n\ - new mode 100755\n\ - --- a/script.sh\n\ - +++ b/script.sh\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(!stripped.contains("old mode")); - assert!(!stripped.contains("new mode")); - assert!(!stripped.contains("100755")); - assert!(stripped.contains("--- a/script.sh")); - } - - #[test] - fn strip_diff_metadata_drops_new_and_deleted_file_mode() { - let diff = "diff --git a/created.rs b/created.rs\n\ - new file mode 100644\n\ - index 0000000..1234567\n\ - --- /dev/null\n\ - +++ b/created.rs\n\ - diff --git a/removed.rs b/removed.rs\n\ - deleted file mode 100644\n\ - index 7654321..0000000\n\ - --- a/removed.rs\n\ - +++ /dev/null\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(!stripped.contains("new file mode")); - assert!(!stripped.contains("deleted file mode")); - assert!(!stripped.contains("100644")); - assert!(stripped.contains("--- /dev/null")); - assert!(stripped.contains("+++ /dev/null")); - } - - #[test] - fn strip_diff_metadata_drops_copy_lines() { - let diff = "diff --git a/orig.rs b/copy.rs\n\ - similarity index 95%\n\ - copy from orig.rs\n\ - copy to copy.rs\n\ - --- a/orig.rs\n\ - +++ b/copy.rs\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(!stripped.contains("copy from")); - assert!(!stripped.contains("copy to")); - assert!(!stripped.contains("similarity index")); - assert!(stripped.contains("+++ b/copy.rs")); - } - - #[test] - fn strip_diff_metadata_drops_dissimilarity_index() { - let diff = "dissimilarity index 30%\n+changed\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(!stripped.contains("dissimilarity index")); - assert!(stripped.contains("+changed")); - } - - #[test] - fn strip_diff_metadata_keeps_content_lines_with_metadata_keywords_as_substring() { - let diff = "+let index = 0;\n\ - -println!(\"similarity index ratio\");\n\ - + // index of array\n"; - let stripped = strip_diff_metadata_lines(diff); - assert!(stripped.contains("+let index = 0;")); - assert!(stripped.contains("-println!(\"similarity index ratio\");")); - assert!(stripped.contains("+ // index of array")); - } - - #[test] - fn write_skip_report_errors_when_parent_is_a_regular_file() { - let dir = tempfile::tempdir().unwrap(); - let blocking_file = dir.path().join("blocking-as-dir-name"); - std::fs::write(&blocking_file, "existing regular file").unwrap(); - - let bad_path = blocking_file.join("nested-report.md"); - let result = write_skip_report(bad_path.to_str().unwrap()); - assert!( - result.is_err(), - "regular file 配下への create_dir_all は err になるべき: {:?}", - result - ); - assert!( - !bad_path.exists(), - "err path で report が書き込まれないことを確認" - ); - } - - #[test] - fn write_skip_report_logged_does_not_panic_on_write_failure() { - let dir = tempfile::tempdir().unwrap(); - let blocking_file = dir.path().join("blocking-regular-file"); - std::fs::write(&blocking_file, "existing regular file").unwrap(); - - let bad_path = blocking_file.join("nested-report.md"); - write_skip_report_logged(bad_path.to_str().unwrap()); - - assert!( - blocking_file.is_file(), - "blocking file が regular file のまま保持されていること (副作用なし)" - ); - assert!( - !bad_path.exists(), - "err path で report が書き込まれないこと (silent fallback 再発防止 = Bundle l 順位 131)" - ); - } - - #[test] - fn write_skip_report_logged_succeeds_on_writable_path() { - let dir = tempfile::tempdir().unwrap(); - let nested_path = dir.path().join("sub").join("report.md"); - write_skip_report_logged(nested_path.to_str().unwrap()); - assert!( - nested_path.exists(), - "writable path では report が生成され、log path に流れていないこと" - ); - let body = std::fs::read_to_string(&nested_path).unwrap(); - assert!(body.contains("skipped")); - } - - #[test] - fn write_skip_report_writes_explanatory_body() { - let path = std::env::temp_dir().join(format!( - "test-lint-screen-skip-report-{}.md", - std::process::id() - )); - let path_str = path.to_str().unwrap(); - write_skip_report(path_str).unwrap(); - let body = std::fs::read_to_string(&path).unwrap(); - assert!(body.contains("skipped")); - assert!(body.contains("docs-only diff")); - assert!(body.contains("Bundle k 順位 123")); - let _ = std::fs::remove_file(&path); - } -} diff --git a/src/cli-push-runner/src/stages/lint_screen/classifier.rs b/src/cli-push-runner/src/stages/lint_screen/classifier.rs new file mode 100644 index 00000000..9b7c5fc2 --- /dev/null +++ b/src/cli-push-runner/src/stages/lint_screen/classifier.rs @@ -0,0 +1,74 @@ +//! cli-finding-classifier.exe を subprocess で起動し、diff を stdin に流して +//! lint-screen JSON を stdout から回収する層。 + +use std::io::Write; +use std::path::Path; +use std::process::{Command, Stdio}; + +use lib_subprocess::wait_with_timeout_basic; + +use super::{InvokeParams, STAGE}; + +pub(super) struct ClassifierOutput { + pub(super) stdout: String, + pub(super) stderr: String, +} + +/// classifier exe を lint-screen mode で spawn する (stdin/stdout/stderr は piped)。 +fn spawn_classifier(params: &InvokeParams<'_>) -> Result { + let timeout_str = params.timeout_secs.to_string(); + Command::new(params.exe) + .args([ + "--mode", + "lint-screen", + "--model", + params.model, + "--endpoint", + params.endpoint, + "--timeout-secs", + &timeout_str, + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| format!("spawn 失敗: {}", e)) +} + +pub(super) fn invoke_classifier( + params: &InvokeParams<'_>, + diff: &str, +) -> Result { + if !Path::new(params.exe).exists() { + return Err(format!("exe 不在 ({})", params.exe)); + } + + let mut child = spawn_classifier(params)?; + + if let Some(mut stdin) = child.stdin.take() { + stdin + .write_all(diff.as_bytes()) + .map_err(|e| format!("stdin 書き込み失敗: {}", e))?; + } + + let stdout_handle = lib_subprocess::drain_pipe_capped( + child.stdout.take().expect("stdout piped"), + crate::runner::MAX_LINES, + ); + let stderr_handle = lib_subprocess::drain_pipe_capped( + child.stderr.take().expect("stderr piped"), + crate::runner::MAX_LINES, + ); + + let exit = wait_with_timeout_basic(STAGE, &mut child, params.timeout_secs + 5) + .map_err(|e| format!("wait 失敗: {}", e))?; + let stdout = stdout_handle.join().unwrap_or_default(); + let stderr = stderr_handle.join().unwrap_or_default(); + + match exit { + None => Err(format!("timeout ({}s)", params.timeout_secs + 5)), + Some(status) if !status.success() => Err(format!("非 0 終了: {}", stderr)), + Some(_) if stdout.trim().is_empty() => Err("stdout 空".to_string()), + Some(_) => Ok(ClassifierOutput { stdout, stderr }), + } +} diff --git a/src/cli-push-runner/src/stages/lint_screen/diff_filter.rs b/src/cli-push-runner/src/stages/lint_screen/diff_filter.rs new file mode 100644 index 00000000..81dff8a4 --- /dev/null +++ b/src/cli-push-runner/src/stages/lint_screen/diff_filter.rs @@ -0,0 +1,415 @@ +//! lint_screen の diff 前処理 (対象外拡張子の除外 + metadata 行の strip)。 +//! +//! LLM 入力前に決定論的に diff を整形し、mistral:7b の構造的 false positive を防止する。 + +/// lint_screen の対象外とする拡張子 (lowercase で比較)。 +/// +/// 由来 (Bundle k 順位 123): mistral:7b が docs-only diff や `.md` ファイルに対して +/// Rust の `unused-import` を hallucinate する FP が PR #148/#150/#151/#152/#153 で +/// 5 PR 連続観測された。diff 段階で `.md` / `.markdown` ハンクを drop することで +/// この failure mode を構造的に解消する (ADR-038 §Known failure mode 参照)。 +const EXCLUDED_EXTENSIONS: &[&str] = &["md", "markdown"]; + +/// `filter_excluded_hunks` の戻り値。Markdown 100% の diff は invoke を完全に +/// skip して別 path (skip-report 書き出し + 短絡 return) に流す必要があるため、 +/// 通常 case (`Kept`) と区別する enum を返す。 +pub(super) enum FilterResult { + Kept(String), + AllExcluded, +} + +/// 入力 diff から `EXCLUDED_EXTENSIONS` 拡張子のハンクを除外する。 +/// +/// 戻り値: +/// - `FilterResult::Kept(text)`: 1 件以上の対象外ハンクが残った場合、その diff text +/// - `FilterResult::AllExcluded`: 全ハンクが対象外拡張子だった (= docs-only diff) 場合 +/// +/// 実装方針: `diff --git ` 行を file-diff の境界として 1 ハンク = 1 chunk に分割、 +/// 各 chunk の `+++ b/` (なければ `--- a/`) から拡張子を取り出して判定。 +/// 拡張子は ASCII lowercase 比較 (= 大文字 `.MD` / `.Markdown` も除外対象に含む)。 +pub(super) fn filter_excluded_hunks(raw_diff: &str) -> FilterResult { + let chunks = split_into_file_diffs(raw_diff); + if chunks.is_empty() { + return FilterResult::Kept(raw_diff.to_string()); + } + let kept: Vec<&str> = chunks + .iter() + .filter(|chunk| !chunk_has_excluded_extension(chunk)) + .copied() + .collect(); + if kept.is_empty() { + return FilterResult::AllExcluded; + } + FilterResult::Kept(kept.join("")) +} + +/// diff text を `diff --git ` 行を境界に file-diff chunks に分割する。 +/// +/// 行頭の `diff --git ` のみを境界とみなす。chunk 末尾は次の境界直前 (改行込み)。 +/// 入力が `diff --git ` で始まらない場合 (= unified diff fragment ではない可能性)、 +/// 空 vec を返して caller が原文 fallthrough する。 +fn split_into_file_diffs(raw_diff: &str) -> Vec<&str> { + if !raw_diff.starts_with("diff --git ") { + return Vec::new(); + } + let mut chunks = Vec::new(); + let mut chunk_start = 0; + for (idx, _) in raw_diff.match_indices("\ndiff --git ") { + let end = idx + 1; + chunks.push(&raw_diff[chunk_start..end]); + chunk_start = end; + } + chunks.push(&raw_diff[chunk_start..]); + chunks +} + +/// chunk 内の `+++ b/` (new path) を優先して拡張子を抽出する。 +/// new path が無い場合 (= delete 操作で `+++ /dev/null` のケース) のみ +/// `--- a/` (old path) にフォールバック。`EXCLUDED_EXTENSIONS` に +/// 該当すれば true を返す。 +/// +/// 新パス優先の根拠 (CR #155 Major 指摘): unified diff の慣例では `--- a/` +/// が `+++ b/` より先に出現するため、単純な `find_map` で両者を OR にすると +/// 旧パスが優先されてしまう。これだと `*.rs → *.md` の rename で **新パス側が `.md` +/// にも関わらず旧 `.rs` 拡張子で判定**され、Markdown 除外が機能しない bug が生じる。 +/// new path を chunk 全体から先に探し、無い場合のみ old path に落とす。 +fn chunk_has_excluded_extension(chunk: &str) -> bool { + let new_path = chunk.lines().find_map(|line| line.strip_prefix("+++ b/")); + let old_path = chunk.lines().find_map(|line| line.strip_prefix("--- a/")); + let path = new_path.or(old_path).unwrap_or(""); + if path.is_empty() { + return false; + } + let ext = path.rsplit('.').next().unwrap_or("").to_ascii_lowercase(); + EXCLUDED_EXTENSIONS.contains(&ext.as_str()) +} + +/// git diff の metadata 行を strip して LLM 入力の signal/noise 比を改善する。 +/// +/// 由来 (Bundle l 順位 132): mistral:7b が `similarity index 100%` の `100%` を +/// magic-number として false positive 検出する事象が PR #155 (Bundle k-1) / +/// PR #156 (Phase E) で観測された。git diff metadata 行は file rename / move を含む +/// PR で必ず出現するため、LLM 入力前に決定論的に除去することで構造的 FP を解消する。 +/// +/// 除去対象 (lossless: 各行を空行に置き換えずに完全削除): +/// - `similarity index NN%` — rename / copy 時の similarity ratio (magic-number FP の主因) +/// - `dissimilarity index NN%` — 同上 (git 1.6.5+ 形式) +/// - `index ..[ ]` — blob hash + file mode (hex の連続が magic 化されやすい) +/// - `new file mode NNNNNN` / `deleted file mode NNNNNN` / `old mode NNNNNN` / +/// `new mode NNNNNN` — Unix mode の 6 桁数値も magic 化されやすい +/// - `rename from ` / `rename to ` — rename target は filter_excluded_hunks +/// が `+++ b/` で既に判定済 (情報量ゼロ) +/// - `copy from ` / `copy to ` — 同上 +/// +/// 保持: `diff --git ` (ハンク境界、file 識別) / `--- a/` / `+++ b/` (path 識別) / +/// `@@ ... @@` (hunk header、line range 情報は LLM が file 位置を理解するのに必要) / +/// `+` / `-` / ` ` (content 行)。 +pub(super) fn strip_diff_metadata_lines(diff: &str) -> String { + diff.lines() + .filter(|line| !is_diff_metadata_line(line)) + .map(|line| { + let mut s = String::with_capacity(line.len() + 1); + s.push_str(line); + s.push('\n'); + s + }) + .collect() +} + +/// `strip_diff_metadata_lines` の per-line 判定。除去対象なら true。 +fn is_diff_metadata_line(line: &str) -> bool { + line.starts_with("similarity index ") + || line.starts_with("dissimilarity index ") + || line.starts_with("index ") + || line.starts_with("new file mode ") + || line.starts_with("deleted file mode ") + || line.starts_with("old mode ") + || line.starts_with("new mode ") + || line.starts_with("rename from ") + || line.starts_with("rename to ") + || line.starts_with("copy from ") + || line.starts_with("copy to ") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rust_chunk(path: &str) -> String { + format!( + "diff --git a/{path} b/{path}\n\ + index abc..def 100644\n\ + --- a/{path}\n\ + +++ b/{path}\n\ + @@ -1,1 +1,1 @@\n\ + -old\n\ + +new\n", + path = path + ) + } + + fn md_chunk(path: &str) -> String { + format!( + "diff --git a/{path} b/{path}\n\ + index abc..def 100644\n\ + --- a/{path}\n\ + +++ b/{path}\n\ + @@ -1,1 +1,1 @@\n\ + -# heading\n\ + +# heading updated\n", + path = path + ) + } + + fn assert_kept(result: FilterResult) -> String { + match result { + FilterResult::Kept(text) => text, + FilterResult::AllExcluded => panic!("expected Kept, got AllExcluded"), + } + } + + #[test] + fn filter_excluded_hunks_keeps_rust_only_diff_unchanged() { + let diff = rust_chunk("src/lib.rs"); + let result = assert_kept(filter_excluded_hunks(&diff)); + assert_eq!(result, diff); + } + + #[test] + fn filter_excluded_hunks_drops_md_hunk_from_mixed_diff() { + let rust = rust_chunk("src/main.rs"); + let md = md_chunk("docs/README.md"); + let combined = format!("{}{}", rust, md); + let kept = assert_kept(filter_excluded_hunks(&combined)); + assert!(kept.contains("src/main.rs")); + assert!(!kept.contains("docs/README.md")); + } + + #[test] + fn filter_excluded_hunks_signals_all_excluded_for_pure_markdown_diff() { + let diff = format!("{}{}", md_chunk("docs/a.md"), md_chunk("docs/b.markdown")); + match filter_excluded_hunks(&diff) { + FilterResult::AllExcluded => {} + FilterResult::Kept(_) => panic!("expected AllExcluded for pure .md/.markdown diff"), + } + } + + #[test] + fn filter_excluded_hunks_treats_markdown_extension_case_insensitively() { + let diff = format!("{}{}", md_chunk("README.MD"), md_chunk("notes.Markdown")); + match filter_excluded_hunks(&diff) { + FilterResult::AllExcluded => {} + FilterResult::Kept(_) => { + panic!("uppercase .MD / mixed-case .Markdown must be excluded") + } + } + } + + #[test] + fn filter_excluded_hunks_keeps_path_with_md_in_middle_not_extension() { + let diff = rust_chunk("src/something.mdxyz.rs"); + let kept = assert_kept(filter_excluded_hunks(&diff)); + assert_eq!(kept, diff); + } + + #[test] + fn filter_excluded_hunks_handles_non_diff_input_as_passthrough() { + let raw = "not a unified diff\njust raw text"; + let kept = assert_kept(filter_excluded_hunks(raw)); + assert_eq!(kept, raw); + } + + #[test] + fn filter_excluded_hunks_keeps_dev_null_create_path() { + let diff = "diff --git a/src/new.rs b/src/new.rs\n\ + new file mode 100644\n\ + index 0000000..1234567\n\ + --- /dev/null\n\ + +++ b/src/new.rs\n\ + @@ -0,0 +1,1 @@\n\ + +pub fn x() {}\n"; + let kept = assert_kept(filter_excluded_hunks(diff)); + assert!(kept.contains("src/new.rs")); + } + + #[test] + fn filter_excluded_hunks_prefers_b_path_on_rename_to_markdown() { + let diff = "diff --git a/src/a.rs b/docs/a.md\n\ + similarity index 100%\n\ + rename from src/a.rs\n\ + rename to docs/a.md\n\ + --- a/src/a.rs\n\ + +++ b/docs/a.md\n\ + @@ -1,1 +1,1 @@\n\ + -old\n\ + +new\n"; + match filter_excluded_hunks(diff) { + FilterResult::AllExcluded => {} + FilterResult::Kept(_) => { + panic!("rename .rs -> .md must be excluded based on new path (CR #155 Major)") + } + } + } + + #[test] + fn filter_excluded_hunks_keeps_rename_from_md_to_rust() { + let diff = "diff --git a/docs/old.md b/src/new.rs\n\ + similarity index 100%\n\ + rename from docs/old.md\n\ + rename to src/new.rs\n\ + --- a/docs/old.md\n\ + +++ b/src/new.rs\n\ + @@ -1,1 +1,1 @@\n\ + -old\n\ + +new\n"; + let kept = assert_kept(filter_excluded_hunks(diff)); + assert!( + kept.contains("src/new.rs"), + "rename .md -> .rs must be kept based on new path (symmetric to rename-to-md test)" + ); + } + + #[test] + fn filter_excluded_hunks_excludes_dev_null_delete_of_md() { + let diff = "diff --git a/docs/old.md b/docs/old.md\n\ + deleted file mode 100644\n\ + index 1234567..0000000\n\ + --- a/docs/old.md\n\ + +++ /dev/null\n\ + @@ -1,1 +0,0 @@\n\ + -# removed\n"; + match filter_excluded_hunks(diff) { + FilterResult::AllExcluded => {} + FilterResult::Kept(_) => panic!( + "delete of .md file should be excluded (--- a/ path is .md, +++ is /dev/null)" + ), + } + } + + #[test] + fn filter_excluded_hunks_preserves_hunk_boundaries_for_three_file_mixed() { + let diff = format!( + "{}{}{}", + rust_chunk("src/a.rs"), + md_chunk("docs/b.md"), + rust_chunk("src/c.rs"), + ); + let kept = assert_kept(filter_excluded_hunks(&diff)); + assert!(kept.contains("src/a.rs")); + assert!(!kept.contains("docs/b.md")); + assert!(kept.contains("src/c.rs")); + let lines: Vec<&str> = kept + .lines() + .filter(|l| l.starts_with("diff --git ")) + .collect(); + assert_eq!( + lines.len(), + 2, + "exactly 2 diff --git boundaries must remain" + ); + } + + #[test] + fn strip_diff_metadata_drops_similarity_index_line() { + let diff = "diff --git a/src/a.rs b/src/b.rs\n\ + similarity index 100%\n\ + rename from src/a.rs\n\ + rename to src/b.rs\n\ + --- a/src/a.rs\n\ + +++ b/src/b.rs\n\ + @@ -1,1 +1,1 @@\n\ + -old\n\ + +new\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!( + !stripped.contains("similarity index"), + "similarity index line must be stripped, got: {}", + stripped + ); + assert!(!stripped.contains("100%")); + assert!(!stripped.contains("rename from")); + assert!(!stripped.contains("rename to")); + } + + #[test] + fn strip_diff_metadata_preserves_hunk_boundaries_and_content() { + let diff = "diff --git a/src/x.rs b/src/x.rs\nindex abc1234..def5678 100644\n--- a/src/x.rs\n+++ b/src/x.rs\n@@ -1,2 +1,2 @@\n-fn old() {}\n+fn new() {}\n // unchanged context line\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(stripped.contains("diff --git ")); + assert!(stripped.contains("--- a/src/x.rs")); + assert!(stripped.contains("+++ b/src/x.rs")); + assert!(stripped.contains("@@ -1,2 +1,2 @@")); + assert!(stripped.contains("-fn old() {}")); + assert!(stripped.contains("+fn new() {}")); + assert!(stripped.contains(" // unchanged context line")); + assert!(!stripped.contains("index abc1234")); + } + + #[test] + fn strip_diff_metadata_drops_file_mode_lines() { + let diff = "diff --git a/script.sh b/script.sh\n\ + old mode 100644\n\ + new mode 100755\n\ + --- a/script.sh\n\ + +++ b/script.sh\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(!stripped.contains("old mode")); + assert!(!stripped.contains("new mode")); + assert!(!stripped.contains("100755")); + assert!(stripped.contains("--- a/script.sh")); + } + + #[test] + fn strip_diff_metadata_drops_new_and_deleted_file_mode() { + let diff = "diff --git a/created.rs b/created.rs\n\ + new file mode 100644\n\ + index 0000000..1234567\n\ + --- /dev/null\n\ + +++ b/created.rs\n\ + diff --git a/removed.rs b/removed.rs\n\ + deleted file mode 100644\n\ + index 7654321..0000000\n\ + --- a/removed.rs\n\ + +++ /dev/null\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(!stripped.contains("new file mode")); + assert!(!stripped.contains("deleted file mode")); + assert!(!stripped.contains("100644")); + assert!(stripped.contains("--- /dev/null")); + assert!(stripped.contains("+++ /dev/null")); + } + + #[test] + fn strip_diff_metadata_drops_copy_lines() { + let diff = "diff --git a/orig.rs b/copy.rs\n\ + similarity index 95%\n\ + copy from orig.rs\n\ + copy to copy.rs\n\ + --- a/orig.rs\n\ + +++ b/copy.rs\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(!stripped.contains("copy from")); + assert!(!stripped.contains("copy to")); + assert!(!stripped.contains("similarity index")); + assert!(stripped.contains("+++ b/copy.rs")); + } + + #[test] + fn strip_diff_metadata_drops_dissimilarity_index() { + let diff = "dissimilarity index 30%\n+changed\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(!stripped.contains("dissimilarity index")); + assert!(stripped.contains("+changed")); + } + + #[test] + fn strip_diff_metadata_keeps_content_lines_with_metadata_keywords_as_substring() { + let diff = "+let index = 0;\n\ + -println!(\"similarity index ratio\");\n\ + + // index of array\n"; + let stripped = strip_diff_metadata_lines(diff); + assert!(stripped.contains("+let index = 0;")); + assert!(stripped.contains("-println!(\"similarity index ratio\");")); + assert!(stripped.contains("+ // index of array")); + } +} diff --git a/src/cli-push-runner/src/stages/lint_screen/mod.rs b/src/cli-push-runner/src/stages/lint_screen/mod.rs new file mode 100644 index 00000000..fbe52c60 --- /dev/null +++ b/src/cli-push-runner/src/stages/lint_screen/mod.rs @@ -0,0 +1,221 @@ +//! Phase c (§8.E lint screen facet) stage +//! +//! diff を mistral:7b に流して lint 一次フィルタの所見を markdown として出力する。 +//! 設計詳細: `docs/adr/adr-038-local-llm-finding-classification.md` +//! +//! 動作モード: +//! - `enabled = false` (default): 完全 no-op、push pipeline は影響を受けない +//! - `enabled = true`: cli-finding-classifier.exe を subprocess で起動、stdin に diff を流し +//! stdout の LintScreenResult JSON を markdown に整形して output_path に書き出す +//! +//! Phase b' で agreement 75% (conditional GO) のため、本 stage は **gating しない**。 +//! Ollama down / timeout / diff 過大 / JSON parse 失敗 等のエラーは全て skip + warn で処理し、 +//! push pipeline をブロックしない。 +//! +//! module 構成: +//! - `diff_filter`: LLM 入力前の diff 前処理 (対象外拡張子除外 + metadata strip) +//! - `classifier`: cli-finding-classifier.exe の subprocess 起動 +//! - `report`: classifier 出力 (JSON) の markdown 整形 + 書き出し + +mod classifier; +mod diff_filter; +mod report; + +use std::time::Instant; + +use crate::config::{ + LintScreenConfig, DEFAULT_LINT_SCREEN_ENDPOINT, DEFAULT_LINT_SCREEN_EXE_PATH, + DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, DEFAULT_LINT_SCREEN_MODEL, DEFAULT_LINT_SCREEN_OUTPUT_PATH, + DEFAULT_LINT_SCREEN_TIMEOUT_SECS, +}; +use crate::log::log_stage; + +use classifier::invoke_classifier; +use diff_filter::{filter_excluded_hunks, strip_diff_metadata_lines, FilterResult}; +use report::{write_report, write_skip_report_logged}; + +const STAGE: &str = "lint-screen"; + +struct InvokeParams<'a> { + exe: &'a str, + model: &'a str, + endpoint: &'a str, + timeout_secs: u64, +} + +fn resolve_invoke_params(config: &LintScreenConfig) -> InvokeParams<'_> { + InvokeParams { + exe: config + .exe_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_EXE_PATH), + model: config.model.as_deref().unwrap_or(DEFAULT_LINT_SCREEN_MODEL), + endpoint: config + .endpoint + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_ENDPOINT), + timeout_secs: config + .timeout_secs + .unwrap_or(DEFAULT_LINT_SCREEN_TIMEOUT_SECS), + } +} + +pub(crate) fn run_lint_screen(config: &LintScreenConfig, diff_path: &str) { + if !config.enabled { + return; + } + + let started = Instant::now(); + log_stage(STAGE, "実行中 (試験運用、エラーは skip + warn)"); + + let raw_diff = match read_diff(diff_path, config) { + Ok(d) => d, + Err(reason) => { + log_stage(STAGE, &format!("skip: {}", reason)); + return; + } + }; + + let output_path = config + .output_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_OUTPUT_PATH); + + let diff = match filter_excluded_hunks(&raw_diff) { + FilterResult::Kept(filtered) => filtered, + FilterResult::AllExcluded => { + log_stage( + STAGE, + "skip: docs-only diff (`.md`/`.markdown` のみ)、Bundle k 順位 123", + ); + write_skip_report_logged(output_path); + return; + } + }; + + let diff = strip_diff_metadata_lines(&diff); + + invoke_and_write_report(config, output_path, &diff, started); +} + +/// classifier 呼び出し + report 書き出しを 1 ステップにまとめた helper。 +/// +/// `run_lint_screen` を 50 行ガイドラインに収めるための機能分離。 +fn invoke_and_write_report( + config: &LintScreenConfig, + output_path: &str, + diff: &str, + started: Instant, +) { + let params = resolve_invoke_params(config); + let output = match invoke_classifier(¶ms, diff) { + Ok(o) => o, + Err(reason) => { + log_stage(STAGE, &format!("skip: classifier {}", reason)); + return; + } + }; + + match write_report(output_path, &output.stdout, &output.stderr) { + Ok(()) => log_stage( + STAGE, + &format!( + "出力: {} ({:.0}s)", + output_path, + started.elapsed().as_secs_f64() + ), + ), + Err(e) => log_stage(STAGE, &format!("skip: report 書き出し失敗: {}", e)), + } +} + +fn read_diff(diff_path: &str, config: &LintScreenConfig) -> Result { + let raw = std::fs::read_to_string(diff_path) + .map_err(|e| format!("diff 読み込み失敗 ({}): {}", diff_path, e))?; + let max_lines = config + .max_diff_lines + .unwrap_or(DEFAULT_LINT_SCREEN_MAX_DIFF_LINES); + let lines = raw.lines().count(); + if lines > max_lines { + return Err(format!("diff 過大 ({} 行 > 上限 {})", lines, max_lines)); + } + if raw.trim().is_empty() { + return Err("diff が空".to_string()); + } + Ok(raw) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn run_lint_screen_is_noop_when_disabled() { + let cfg = LintScreenConfig { + enabled: false, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + run_lint_screen(&cfg, "/nonexistent/diff/path"); + } + + #[test] + fn read_diff_returns_error_on_missing_file() { + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + let result = read_diff("/nonexistent/path.txt", &cfg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("diff 読み込み失敗")); + } + + #[test] + fn read_diff_returns_error_when_diff_exceeds_limit() { + let path = std::env::temp_dir().join("test-lint-screen-large-diff.txt"); + let large = "x\n".repeat(100); + std::fs::write(&path, large).unwrap(); + + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: Some(50), + output_path: None, + }; + let result = read_diff(path.to_str().unwrap(), &cfg); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.contains("diff 過大")); + assert!(err.contains("100")); + } + + #[test] + fn read_diff_returns_error_on_empty_diff() { + let path = std::env::temp_dir().join("test-lint-screen-empty-diff.txt"); + std::fs::write(&path, "").unwrap(); + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + let result = read_diff(path.to_str().unwrap(), &cfg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("空")); + } +} diff --git a/src/cli-push-runner/src/stages/lint_screen/report.rs b/src/cli-push-runner/src/stages/lint_screen/report.rs new file mode 100644 index 00000000..b395f1a6 --- /dev/null +++ b/src/cli-push-runner/src/stages/lint_screen/report.rs @@ -0,0 +1,331 @@ +//! lint_screen の classifier 出力 (JSON) を markdown report に整形して書き出す層。 +//! +//! docs-only diff の skip-report も本 module が担当する。 + +use std::path::Path; + +use super::STAGE; +use crate::log::log_stage; + +const REPORT_PREAMBLE: &str = "# Lint Screen Report (mistral:7b, Phase b' agreement 75%)\n\n\ +> **試験運用**: 本 report は ADR-038 Phase c lint screen facet による mistral:7b の AI 所見。\n\ +> agreement 75% (conditional GO) のため誤指摘あり。reviewer が独立判断する前提で参考情報として扱う。\n\n"; + +pub(super) fn write_report( + output_path: &str, + classifier_json: &str, + stderr: &str, +) -> Result<(), String> { + let path = Path::new(output_path); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|e| format!("ディレクトリ作成失敗: {}", e))?; + } + let markdown = format_report(classifier_json, stderr); + std::fs::write(path, markdown).map_err(|e| format!("write: {}", e)) +} + +fn render_parse_error(err: &serde_json::Error, raw: &str) -> String { + let mut out = String::from(REPORT_PREAMBLE); + out.push_str(&format!("## JSON parse 失敗\n\nerror: {}\n\n", err)); + out.push_str("```\n"); + out.push_str(raw); + out.push_str("\n```\n"); + out +} + +fn render_summary(decision: &str, findings_count: usize, fallback_reason: &str) -> String { + let mut s = format!( + "## Summary\n\n- screen_decision: `{}`\n- findings: {}\n", + decision, findings_count + ); + if !fallback_reason.is_empty() { + s.push_str(&format!("- fallback_reason: `{}`\n", fallback_reason)); + } + s.push('\n'); + s +} + +fn render_findings_table(findings: &[serde_json::Value]) -> String { + if findings.is_empty() { + return "## Findings\n\n(なし)\n".to_string(); + } + let mut out = + String::from("## Findings\n\n| severity | rule | file | line | issue | suggestion |\n"); + out.push_str("|---|---|---|---|---|---|\n"); + for f in findings { + let s = f.get("severity").and_then(|v| v.as_str()).unwrap_or("?"); + let r = f.get("rule").and_then(|v| v.as_str()).unwrap_or("?"); + let file = f.get("file").and_then(|v| v.as_str()).unwrap_or("?"); + let line = f + .get("line") + .map(|v| v.to_string()) + .unwrap_or_else(|| "?".to_string()); + let issue = f.get("issue").and_then(|v| v.as_str()).unwrap_or(""); + let sug = f.get("suggestion").and_then(|v| v.as_str()).unwrap_or(""); + out.push_str(&format!( + "| {} | {} | {} | {} | {} | {} |\n", + sanitize_cell(s), + sanitize_cell(r), + sanitize_cell(file), + sanitize_cell(&line), + sanitize_cell(issue), + sanitize_cell(sug), + )); + } + out +} + +fn format_report(classifier_json: &str, stderr: &str) -> String { + let value: serde_json::Value = match serde_json::from_str(classifier_json) { + Ok(v) => v, + Err(e) => { + let mut out = render_parse_error(&e, classifier_json); + out.push_str(&render_diagnostic(stderr)); + return out; + } + }; + let decision = value + .get("screen_decision") + .and_then(|v| v.as_str()) + .unwrap_or("?"); + let fallback_reason = value + .get("fallback_reason") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let findings = value + .get("lint_findings") + .and_then(|v| v.as_array()) + .cloned() + .unwrap_or_default(); + + let mut out = String::from(REPORT_PREAMBLE); + out.push_str(&render_summary(decision, findings.len(), fallback_reason)); + out.push_str(&render_findings_table(&findings)); + out.push_str(&render_diagnostic(stderr)); + out +} + +fn render_diagnostic(stderr: &str) -> String { + let trimmed = stderr.trim(); + if trimmed.is_empty() { + return String::new(); + } + let mut out = String::from("\n## Diagnostic\n\n"); + out.push_str( + "classifier exe からの stderr 出力 (Phase A 順位 98 の num_ctx overflow 診断 log 等):\n\n", + ); + out.push_str("```text\n"); + out.push_str(trimmed); + out.push_str("\n```\n"); + out +} + +/// markdown table cell 用に `|` と改行を escape する。 +fn sanitize_cell(s: &str) -> String { + s.replace('|', "\\|").replace('\n', " ") +} + +/// `write_skip_report` を呼び出し、失敗時はステージログに記録する。 +/// +/// `run_lint_screen` のネスト深度を抑えるための分離 (match arm 内に if let を +/// 重ねないよう、エラー処理を 1 関数に閉じ込める)。 +pub(super) fn write_skip_report_logged(output_path: &str) { + if let Err(e) = write_skip_report(output_path) { + log_stage(STAGE, &format!("skip: skip-report 書き出し失敗: {}", e)); + } +} + +/// 全ハンクが対象外拡張子だった場合に書き出す skip-report。invoke は完全に skip する。 +fn write_skip_report(output_path: &str) -> Result<(), String> { + let path = Path::new(output_path); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|e| format!("ディレクトリ作成失敗: {}", e))?; + } + let body = format!( + "{}## Summary\n\n- screen_decision: `skipped`\n- 理由: docs-only diff のため lint_screen はスキップしました \ + (`.md` / `.markdown` 拡張子のみで Rust hallucinate FP を構造的に防止、Bundle k 順位 123 / ADR-038)\n\n\ + ## Findings\n\n(なし)\n", + REPORT_PREAMBLE + ); + std::fs::write(path, body).map_err(|e| format!("write: {}", e)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_report_renders_findings_table() { + let json = r#"{ + "lint_findings": [ + {"severity":"minor","rule":"unused-import","file":"src/a.rs","line":1,"issue":"x","suggestion":"y"} + ], + "screen_decision":"auto_fix" + }"#; + let md = format_report(json, ""); + assert!(md.contains("auto_fix")); + assert!(md.contains("unused-import")); + assert!(md.contains("src/a.rs")); + assert!(md.contains("| severity | rule | file | line | issue | suggestion |")); + } + + #[test] + fn format_report_handles_empty_findings() { + let json = r#"{"lint_findings":[],"screen_decision":"informational"}"#; + let md = format_report(json, ""); + assert!(md.contains("informational")); + assert!(md.contains("(なし)")); + } + + #[test] + fn format_report_recovers_from_invalid_json() { + let md = format_report("not json", ""); + assert!(md.contains("JSON parse 失敗")); + assert!(md.contains("not json")); + } + + #[test] + fn format_report_includes_diagnostic_section_when_stderr_non_empty() { + let json = r#"{ + "lint_findings": [], + "screen_decision": "human_review", + "fallback_reason": "ollama error: JSON parse error" + }"#; + let stderr = "[lib-ollama-client] WARN: Ollama JSON output may be truncated.\n prompt_eval_count: 8192 (vs num_ctx: 8192)"; + let md = format_report(json, stderr); + assert!(md.contains("## Diagnostic")); + assert!(md.contains("prompt_eval_count: 8192")); + assert!(md.contains("num_ctx: 8192")); + } + + #[test] + fn format_report_skips_diagnostic_section_when_stderr_empty() { + let json = r#"{ + "lint_findings": [], + "screen_decision": "informational" + }"#; + let md = format_report(json, ""); + assert!(!md.contains("## Diagnostic")); + assert!(!md.contains("classifier exe からの stderr")); + } + + #[test] + fn format_report_skips_diagnostic_section_when_stderr_whitespace_only() { + let json = r#"{ + "lint_findings": [], + "screen_decision": "informational" + }"#; + let md = format_report(json, " \n\n "); + assert!(!md.contains("## Diagnostic")); + } + + #[test] + fn format_report_appends_diagnostic_to_parse_error_path() { + let stderr = "[lib-ollama-client] WARN: truncated"; + let md = format_report("not json", stderr); + assert!(md.contains("JSON parse 失敗")); + assert!(md.contains("## Diagnostic")); + assert!(md.contains("[lib-ollama-client] WARN")); + } + + #[test] + fn format_report_includes_fallback_reason_when_present() { + let json = r#"{ + "lint_findings":[], + "screen_decision":"human_review", + "fallback_reason":"ollama error: empty" + }"#; + let md = format_report(json, ""); + assert!(md.contains("fallback_reason")); + assert!(md.contains("ollama error")); + } + + #[test] + fn sanitize_cell_escapes_pipe_and_newline() { + assert_eq!(sanitize_cell("a|b"), "a\\|b"); + assert_eq!(sanitize_cell("a\nb"), "a b"); + } + + #[test] + fn render_findings_table_sanitizes_pipe_in_all_columns() { + let json = r#"{ + "lint_findings": [ + {"severity":"mi|nor","rule":"un|used","file":"src/a|b.rs","line":1,"issue":"x|y","suggestion":"y|z"} + ], + "screen_decision":"auto_fix" + }"#; + let md = format_report(json, ""); + assert!(!md.contains("mi|nor"), "severity must be sanitized"); + assert!(md.contains("mi\\|nor")); + assert!(!md.contains("un|used"), "rule must be sanitized"); + assert!(md.contains("un\\|used")); + assert!(!md.contains("a|b.rs"), "file must be sanitized"); + assert!(md.contains("a\\|b.rs")); + } + + #[test] + fn write_skip_report_errors_when_parent_is_a_regular_file() { + let dir = tempfile::tempdir().unwrap(); + let blocking_file = dir.path().join("blocking-as-dir-name"); + std::fs::write(&blocking_file, "existing regular file").unwrap(); + + let bad_path = blocking_file.join("nested-report.md"); + let result = write_skip_report(bad_path.to_str().unwrap()); + assert!( + result.is_err(), + "regular file 配下への create_dir_all は err になるべき: {:?}", + result + ); + assert!( + !bad_path.exists(), + "err path で report が書き込まれないことを確認" + ); + } + + #[test] + fn write_skip_report_logged_does_not_panic_on_write_failure() { + let dir = tempfile::tempdir().unwrap(); + let blocking_file = dir.path().join("blocking-regular-file"); + std::fs::write(&blocking_file, "existing regular file").unwrap(); + + let bad_path = blocking_file.join("nested-report.md"); + write_skip_report_logged(bad_path.to_str().unwrap()); + + assert!( + blocking_file.is_file(), + "blocking file が regular file のまま保持されていること (副作用なし)" + ); + assert!( + !bad_path.exists(), + "err path で report が書き込まれないこと (silent fallback 再発防止 = Bundle l 順位 131)" + ); + } + + #[test] + fn write_skip_report_logged_succeeds_on_writable_path() { + let dir = tempfile::tempdir().unwrap(); + let nested_path = dir.path().join("sub").join("report.md"); + write_skip_report_logged(nested_path.to_str().unwrap()); + assert!( + nested_path.exists(), + "writable path では report が生成され、log path に流れていないこと" + ); + let body = std::fs::read_to_string(&nested_path).unwrap(); + assert!(body.contains("skipped")); + } + + #[test] + fn write_skip_report_writes_explanatory_body() { + let path = std::env::temp_dir().join(format!( + "test-lint-screen-skip-report-{}.md", + std::process::id() + )); + let path_str = path.to_str().unwrap(); + write_skip_report(path_str).unwrap(); + let body = std::fs::read_to_string(&path).unwrap(); + assert!(body.contains("skipped")); + assert!(body.contains("docs-only diff")); + assert!(body.contains("Bundle k 順位 123")); + let _ = std::fs::remove_file(&path); + } +} From c744bb1486ac8531491d8b2e9f08e49f3a8921a1 Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 1 Jul 2026 15:18:46 +0900 Subject: [PATCH 4/4] =?UTF-8?q?docs(plan):=20PR-W4=20=E5=88=86=E5=89=B2?= =?UTF-8?q?=E3=81=AB=E4=BC=B4=E3=81=84=20lint=5Fscreen.rs=20/=20config.rs?= =?UTF-8?q?=20=E3=81=AE=E3=83=AA=E3=83=B3=E3=82=AF=E3=82=92=20module=20?= =?UTF-8?q?=E3=83=87=E3=82=A3=E3=83=AC=E3=82=AF=E3=83=88=E3=83=AA=E3=81=AB?= =?UTF-8?q?=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/file-length-enforcement-plan.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/file-length-enforcement-plan.md b/docs/file-length-enforcement-plan.md index 3ea30a4b..b924b931 100644 --- a/docs/file-length-enforcement-plan.md +++ b/docs/file-length-enforcement-plan.md @@ -359,8 +359,8 @@ Agent 委譲。ADR-018 (cli-pr-monitor の takt 移行) を参照させる必要 #### スコープ -- [`src/cli-push-runner/src/stages/lint_screen.rs`](../src/cli-push-runner/src/stages/lint_screen.rs) (982 行) — ADR-038 試験運用 (local LLM lint screen) -- [`src/cli-push-runner/src/config.rs`](../src/cli-push-runner/src/config.rs) (946 行) — 各 stage の config struct 集約 +- [`src/cli-push-runner/src/stages/lint_screen/`](../src/cli-push-runner/src/stages/lint_screen/) (旧 lint_screen.rs 982 行 → 4 module に分割) — ADR-038 試験運用 (local LLM lint screen) +- [`src/cli-push-runner/src/config/`](../src/cli-push-runner/src/config/) (旧 config.rs 946 行 → 4 module に分割) — 各 stage の config struct 集約 `config.rs` は struct 集約のため、stage 別 module (`config/lint_screen.rs` / `config/pr_size_check.rs` 等) への分割が素直。