diff --git a/docs/adr/adr-027-push-review-simplicity-focus.md b/docs/adr/adr-027-push-review-simplicity-focus.md
index fe157773..dae4e2a0 100644
--- a/docs/adr/adr-027-push-review-simplicity-focus.md
+++ b/docs/adr/adr-027-push-review-simplicity-focus.md
@@ -42,6 +42,18 @@ push 時点で見たかったのは「コードのシンプルさ」であり、
「architectural 妥当性 (cross-file, ADR 準拠, 命名規約)」→「コードのシンプルさ (diff 局所)」に責務を狭める。後者は diff だけで完結するため、reviewer が Grep/Read で探索する必要がなくなる。
+### Amendment (2026-07-21): 「diff 局所」は *観点* の限定であって *コミット範囲* の限定ではない
+
+本 ADR が狭めたのは **reviewer が使う criteria** (cross-file 探索を要求しない) であり、**レビュー対象に含めるコミットの範囲**ではない。この 2 つが混同され、`push-runner-config.toml` の `[diff] command` が `jj diff -r @` (tip コミットのみ) のまま運用された結果、祖先コミットが pre-push レビューを一度も経ずに merge される状態が生まれた (todo 順位 288、Severity High で PR #268/#300/#301/#311 と 4 回再発)。
+
+レビュー対象は **PR 範囲 (`..@`) 全体**とする。根拠:
+
+- **速度は理由にならない (実測)**: 同一 PR に対しレビュー対象を 37 行 → 1011 行 (27 倍) に広げても所要時間は 4m32s → 4m43s の **+11 秒**。本 ADR の速度改善は arch-review facet の除去 (219-270s/iter) によるもので、diff 範囲の縮小は寄与していない。
+- **検知不能性**: レビュアーは渡された diff が PR 全体かを検証できないため、範囲が狭いことによる見落としは誰にも検知されない (実際 security-review が実 diff と矛盾して "docs-only / No dependency changes" と報告した)。
+- **CodeRabbit backstop では代替できない**: 第三者レビューは有用だが、セルフレビューを省いてよい理由にはならない。両者は独立した層として併用する。
+
+実装は `Config::resolve_base_branch` に範囲解決を一本化し、`[diff] command` は `{{PR_RANGE}}` プレースホルダ経由で範囲を受け取る (config に revset を直書きできない)。加えて生成 diff が PR 範囲を網羅しているかを fail-closed で機械検査する。
+
### simplicity-review の criteria (diff 局所で完結)
- ネスト深さ (>4 レベルで要改善)
diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md
index 5347e8fa..e3ff2398 100644
--- a/docs/todo-summary2.md
+++ b/docs/todo-summary2.md
@@ -40,7 +40,6 @@
| 255 | 💎 Tier 3 | **ADR-040 の実測値を新 GPU (RTX PRO 5000 48GB) で再 calibration (ADR-046 WP-01 スパイクで陳腐化を観測)** | todo15.md | S | なし (ADR-038/040 が前提とする RTX 3070 8GB は RTX PRO 5000 Blackwell 48GB に更新済み。27-31B Q4 モデルが 100% GPU で動き VRAM が制約でなくなったため、ADR-040 の VRAM/latency trade-off 表と「VRAM scarcity → model swap 制約」framing が陳腐化。ADR-046 で mistral:7b / gemma4 / qwen3-coder の VRAM・latency を実測済 → ADR-040 amendment に反映、num_ctx 選定 flow の memory 軸を latency 軸へ再重み付け) |
| 256 | ⏳ Tier 5 | **classifier FP 検出強化プロンプトで格上げ候補を再評価 (WP-04 見送りの follow-up、ADR-038 amendment 由来)** | todo15.md | M | なし (WP-04 実測で全候補が FP 検出未達 = 能力限界か `classify.txt` の mistral 向け tune 不適合かが未分離。FP 検出強化プロンプト版で qwen3-coder:30b 等を再測し、能力限界と確認できれば恒久見送り、プロンプト不適合なら該当モデル + 専用プロンプトで格上げ。eval 手法・gold セットは scratchpad WP-04 資産を再利用。materially better な新モデル出現時も再評価トリガー) |
| 257 | ⏳ Tier 5 | **push pipeline の `cargo test` を cargo-nextest 化 (WP-05 で Stop hook には無効と判明、push 側 follow-up)** | todo15.md | S-M | なし (WP-05 実測: Stop hook は cargo test 不在で nextest 非適用、真因は逐次実行→並列化で解決済。ただし push pipeline (cli-push-runner quality_gate) の `cargo test -- --ignored` は実測 ~80s で nextest 高速化の余地あり。ツール依存追加 = ADR-017 pinning + 派生プロジェクト配布のコスト、push が Stop より低頻度な点を踏まえた費用対効果を評価。doctest は nextest 非実行のため `cargo test --doc` 併走が必要) |
-| 264 | 🔧 Tier 2 | **pre-push review-diff.txt の生成形式を `jj diff --git` に切替 — LLM レビュアーの add/delete 誤読解消 (PR #256 post-merge-feedback Tier1 #1 採用)** | todo15.md | S | なし (`push-runner-config.toml:113` の `[diff] command = "jj diff -r @"` は色+行番号2列形式で、色を落とした review-diff.txt では削除が `-` マーカー無しになり LLM レビュアーが「追加」と誤読。PR #256 で todo 25行削除を simplicity-review が false positive REJECT し ~19分浪費。`jj diff --git -r @` へ切替で解消、`templates/push-runner-config.toml:52` も同 PR で修正必須 (deploy:hooks 配布)、Adoption Risk None、memory `prepush-review-diff-plain-format-misread.md`) |
| 272 | 🚀 Tier 1 | **cli-docs-lint に ADR 重複採番 + CLAUDE.md 索引整合チェック追加 (PR #261 post-merge-feedback T1-#2 採用)** | todo15.md | S | なし (PR #261 で ADR-052/053 採番衝突が実発生、既存 cli-docs-lint の check-mode 骨格流用。順位 135 placeholder policy は todo entry 側の「ルール」で本 entry は land 済ファイルの「仕組み」検知、相補で重複ではない) |
| 275 | 🔧 Tier 2 | **層別テストテンプレート (StubOllama パターン・integration 独立性) の共有化 (PR #265 post-merge-feedback T2-1 採用)** | todo15.md | M | なし (WP-11/ADR-054 の多層防御実装で「空 StubOllama による LLM 未呼び出し証明」「tempdir+jj init+CwdRestore の integration 独立性」を都度設計。WP-17 の classifier/scope guard 拡張で同種判断が再発見込み。shared crate 化の境界は ADR-044 で判定、WP-17 着手前の実施が効果的) |
| 276 | 💎 Tier 3 | **ADR-007 に「コメント配置の意思決定フロー」を追加 (PR #265 post-merge-feedback T3-2 採用)** | todo15.md | S | なし (PR #265 で非 doc コメントの Bundle Z block が 2 回発生 = doc コメント/識別子名/マーカー付き Why の配置判断が未文書化。linter 自動化は NLP 必要で却下済み、既存 Q1-Q3 形式で人間/AI の判断補助を doc 化。バッチ PR で消化可) |
@@ -52,7 +51,7 @@
| 285 | 🔧 Tier 2 | **jj keyword を含む commit message の tokenization edge-case テスト (PR #267 post-merge-feedback T2-2 採用)** | todo15.md | S | なし (順位 283 と表裏。283 の着手有無に関わらず現行挙動を regression test で固定する価値が独立して残る。283 と同一 PR 消化が効率的) |
| 286 | 🔧 Tier 2 | **config path 解決の cwd 跨ぎ integration test (PR #267 post-merge-feedback T2-3 採用)** | todo15.md | M | なし (FIXED 済 cwd-config bug の regression guard。既存テストは pure parser のみで file-lookup 経路未カバー。Severity High、Adoption Risk = OS 依存) |
| 287 | 💎 Tier 3 | **「config 読み hook は exe-relative 解決必須」convention の明文化 (PR #267 post-merge-feedback T3-1 採用)** | todo15.md | XS | なし (順位 281 の文書層補完。**281 と同一 PR bundle 推奨**、別作業に切り出す価値は低い) |
-| 288 | 🚀 Tier 1 | **pre-push review が PR 全体をカバーしない — post-merge の全 run 集約 + push-runner `[diff]` stage の tip-only 範囲修正 (PR #268/#300/#301 feedback 採用、Severity High 3連続再発)** | todo15.md | M | なし (根因は `[diff]` stage `jj diff -r @` が tip のみ = 祖先 code が AI レビュー未経由で merge。`docs_only_routing` は PR 範囲へ修正済だが `[diff]` は非対称。単一 push では全 run 集約でも救えず [diff] 範囲拡張 + bookmark_check 祖先検証が根治。ADR-027 射程はユーザー判断。独立 PR 推奨) |
+| 288 | 🚀 Tier 1 | **pre-push review が PR 全体をカバーしない — post-merge の全 run 集約 + push-runner `[diff]` stage の tip-only 範囲修正 (PR #268/#300/#301 feedback 採用、Severity High 3連続再発)** | todo15.md | M | なし (【一部実装済 2026-07-21】`[diff]` stage の PR 範囲化 + 範囲カバレッジ検査 (fail-closed) + top-level default_branch 一本化 + `--git` 切替 (順位 264 同時完了) を実施。ADR-027 射程は実測 (37行 4m32s → 1011行 4m43s = +11秒) を根拠に範囲拡張を採用。**残**: post-merge feedback の全 run 集約と bookmark_check.rs の祖先未レビュー穴の検証) |
| 292 | 🔧 Tier 2 | **cli-pr-monitor の lock.rs を token 方式の所有権検証へ統一** | todo15.md | S-M | なし (PR #271 で pipeline_lock.rs に導入した token ベース所有権検証と同型の Drop 無条件削除バグが cli-pr-monitor/src/lock.rs にも残存。参照実装が既にあるため低リスク) |
| 293 | ⏳ Tier 5 | **push-runner の stack push モード (opt-in、YAGNI につき見送り継続)** | todo15.md | M | なし (stacked bookmark 運用の実績が現状なく、必要になった時点で着手する opt-in 拡張として記録のみ) |
| 294 | 💎 Tier 3 | **jj-op-verify hook の位置づけ再整理 — 並列 workspace 安全化ではなく混線緩和層として再分類** | todo15.md | S | なし (検知対象は出力混線の症状であり並列 workspace とは独立に価値を持つ。ADR-045→ADR-053 の枠組みへ紐付け直すドキュメント再整理のみ) |
diff --git a/docs/todo15.md b/docs/todo15.md
index 1a492393..11926481 100644
--- a/docs/todo15.md
+++ b/docs/todo15.md
@@ -218,39 +218,6 @@
---
-### pre-push review-diff.txt の生成形式を jj diff --git に切替 — LLM レビュアーの add/delete 誤読解消 (PR #256 post-merge-feedback Tier1 #1 採用)
-
-> **動機**: `push-runner-config.toml:113` の `[diff] command = "jj diff -r @"`（jj デフォルト形式）で生成される `.takt/review-diff.txt` は、追加/削除を色 + 行番号2列（`NNN :` = 削除 / ` NNN:` = 追加）で表現する。ファイル化で色が落ちると `-`/`+` マーカーが無くなり、削除が「左列のみ行番号」でしか区別できず、pre-push の LLM レビュアー（simplicity-review 等）が削除ブロックを「追加」と誤読しうる。`--git`（標準 unified diff）は色非依存で `+`/`-` を明示するため誤読しない。PR #256（ADR-051 起票 PR）で todo エントリ25行の**削除**を simplicity-review が「追加」と誤読し stale-tracking-entry として false positive REJECT を出し、レビュー約19分を浪費した実害が発生した。
->
-> **本タスクの位置づけ**: PR #256 post-merge-feedback Tier1 #1 で採用（他6提案は over-engineering として却下）。fix ステップの「hunk-polarity bug」という診断は不正確で、真因は色を落とした平文 diff の LLM 可読性問題。
->
-> **参照**: `push-runner-config.toml:113`（`command = "jj diff -r @"` → `"jj diff --git -r @"`、修正対象）、`templates/push-runner-config.toml:52`（同様の変更、`pnpm deploy:hooks` で派生プロジェクトに配布されるため**両方修正必須**）、memory `prepush-review-diff-plain-format-misread.md`、PR #256 feedback report (`.claude/feedback-reports/256.md`) Tier1 #1
->
-> **実行優先度**: 🔧 Tier 2 — Effort S。false positive で約19分浪費した実害が既に発生しており、config + template 各1箇所の軽微な修正で再発を防止できる。
-
-#### 設計決定 (案)
-
-- `[diff] command` を `jj diff --git -r @` に変更。本番 config と template の2箇所を同一 PR で修正（template 未修正だと派生プロジェクトに同じ false positive が横展開）。
-- review-diff.txt を format-sensitive に parse する `.rs` 箇所は存在せず（LLM facet が読むのみ）、Adoption Risk None。
-
-#### 作業計画
-
-- [ ] `push-runner-config.toml:113` を `command = "jj diff --git -r @"` に変更
-- [ ] `templates/push-runner-config.toml:52` も同様に変更
-- [ ] review-diff.txt を参照する箇所（facet instruction / `.rs`）が `--git` 形式で問題ないか確認
-- [ ] dogfood: 削除を含む diff で pre-push review が正しく削除を認識することを確認
-- [ ] 本エントリ削除 + todo-summary2.md 行削除
-
-#### 完了基準
-
-- pre-push review が削除ブロックを「追加」と誤読しなくなり、config + template 両方が `--git` 形式、派生プロジェクトへの横展開も解消。
-
-#### 詰まっている箇所
-
-- なし（変更箇所・影響範囲とも確定済み、PR #256 feedback report で cross-validation 済み）。
-
----
-
### cli-docs-lint に ADR 重複採番 + CLAUDE.md 索引整合チェック追加 (PR #261 post-merge-feedback T1-#2 採用)
> **動機**: PR #261 で当方が ADR-052 として起草した ADR が、並行 land した PR #260 の ADR-052 (自律実行境界) と採番衝突し、rebase 時にファイル名 + 本文タイトル + ソース内参照 10+ 箇所の置換が発生した実例。ADR は既に 53 件、並行 PR 開発が常態化しており再発頻度 Medium。現状この衝突を機械検知する層が存在しない (発見は rebase 時の CLAUDE.md conflict 頼み)。
@@ -477,13 +444,17 @@
>
> **再発・優先度見直し (2026-07-19、#300/#301 feedback)**: PR-N2 (#300) / PR-N3 (#301) の feedback で同型 gap が Severity High で再発。根因は本エントリの集約範囲より上流の **push-runner `[diff]` stage** (`push-runner-config.toml` の `[diff] command = "jj diff -r @"`) が **tip コミットのみ**を AI レビュー用 diff に書き出す点。複数コミットを 1 回の `pnpm push` でまとめて送ると、tip 以外の祖先コミット (例 #300 の `resolve_main_workspace_root()` 実装、#301 の `Cargo.toml`/`main.rs` 変更) が local security/simplicity レビューを一度も経ずに merge される (security-review.md が実 diff と矛盾して "docs-only / No dependency changes" と記載)。**単一 push では pre-push run 自体が 1 回のみ**のため、本エントリの「全 run 集約」だけでは救えない。よって本エントリの実装時に、(a) `[diff]` stage の diff 範囲を `docs_only_routing` と同様に `..@` (PR 範囲) へ拡張し、(b) `bookmark_check.rs` の `@` 非 trunk 祖先が未レビューのまま push される穴 (T8 / PR #280 と同クラス) の検証を併せて行う。`docs_only_routing.rs` の skip 判定は既に `..@` に修正済みだが `[diff]` stage 自体は未修正で非対称。ADR-027 (push-time review を diff-local に限定し範囲外は CodeRabbit backstop) の trade-off 射程が security-review にも及ぶかはユーザー判断待ち。
>
+> **(a) 実装済 (2026-07-21)**: PR #311 で 4 回目の再発 (695 行の PR に対しレビュー対象 37 行、security-review が実 diff と矛盾して "docs-only / No dependency changes" と記載) を観測し、`[diff]` stage を修正した。top-level `default_branch` を新設して `diff` / `docs_only_routing` / `pr_size_check` の 3 stage が同一解決を共有し、`[diff] command` は `{{PR_RANGE}}` プレースホルダ経由で範囲を受け取る (config に revset を直書きできない)。加えて生成 diff が PR 範囲の全変更ファイルを含むかを `jj diff --summary` と突き合わせる**範囲カバレッジ検査**を fail-closed で追加し、config の書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるようにした (未更新の派生プロジェクト config も捕まる)。順位 264 (`--git` 形式切替) も同 PR で同時実施 (範囲検査が `diff --git` ヘッダを読む要件と重なるため)。**ADR-027 の射程についてのユーザー判断**: 範囲拡張のレビュー時間コストを実測したところ 37 行 4m32s → 1011 行 4m43s (**+11 秒**) で、ADR-027 の速度改善は arch-review facet 除去によるものであり diff 範囲縮小は寄与していないことが判明したため、範囲拡張を採用した。
+>
+> **残タスク**: 本エントリ本体の「post-merge feedback の全 run 集約」と、上記 (b) `bookmark_check.rs` の祖先未レビュー穴の検証は未着手。
+>
> **参照**: `.claude/feedback-reports/268.md` Tier 2 #1 / `.claude/feedback-reports/300.md` Tier1 #1 / `.claude/feedback-reports/301.md` Tier1 #1、`src/cli-merge-pipeline/src/feedback/context.rs` (`find_latest_prepush_reports_dir`)、`push-runner-config.toml` (`[diff]` section)、`src/cli-push-runner/src/stages/diff.rs`・`src/cli-push-runner/src/stages/bookmark_check.rs`、`src/cli-push-runner/src/config/docs_only_routing.rs` (既に PR 範囲へ修正済の対照)、`.takt/facets/instructions/analyze-prepush-reports.md`、[ADR-027](adr/adr-027-push-review-simplicity-focus.md)
>
> **実行優先度**: 🚀 Tier 1 — Severity High (review gate の silent 覆域縮小が 3 PR 連続で再発) / Frequency Medium (複数コミットを 1 push する運用で恒常発生) / Effort M。context スキーマ変更 + facet 更新 + `[diff]` stage 修正 + テストを伴うため独立 PR 推奨 (旧 Tier 2 から昇格)。
#### 作業計画
-- [ ] **`[diff]` stage の diff 範囲を `..@` (PR 範囲) に拡張** — 祖先コミットの code 変更も AI レビュー用 diff に含める (`docs_only_routing` の skip 判定と同基準に揃える)。
+- [x] **`[diff]` stage の diff 範囲を `..@` (PR 範囲) に拡張** — 祖先コミットの code 変更も AI レビュー用 diff に含める (`docs_only_routing` の skip 判定と同基準に揃える)。**PR #311/#313 で実装済** (上記 (a) 参照。範囲カバレッジ検査 + config-load 時の `{{PR_RANGE}}` 必須検証込み)。
- [ ] `bookmark_check.rs` で `@` 非 trunk 祖先が未レビューのまま push される穴を検証・塞ぐ (T8 / PR #280 と同クラス)。
- [ ] 対象 PR の pre-push run dir を列挙する関数に拡張。時刻範囲のみでの絞り込みは対象外 run の混入・対象 run の欠落を招くため、対象 PR のコミット範囲や関連 bookmark 名など複数の識別根拠を突き合わせて対象 run を判定すること (`.takt/runs/*-pre-push-review`)
- [ ] context json の `prepush_reports_dir` を配列化 + facet instruction を複数 dir 対応に (スキーマ契約変更のため: 全 reader の列挙 + 旧 string 形式との後方互換 or schema versioning + 空配列時の挙動を明記)
diff --git a/push-runner-config.toml b/push-runner-config.toml
index cf489de6..0f729469 100644
--- a/push-runner-config.toml
+++ b/push-runner-config.toml
@@ -3,6 +3,18 @@
# pnpm push で起動される push-runner.exe がこのファイルを読み込む。
# カレントディレクトリ (リポジトリルート) を優先的に検索する。
+# PR base branch。`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が
+# 「PR 範囲」= `..@` をここから解決する (唯一の真実源)。
+# 各 section にも `default_branch` を書けるが後方互換のための override であり、
+# 新規に section 側で持たせないこと (値が食い違うと stage 間で見る範囲がずれる)。
+#
+# TOML の table 構文上、本キーは**最初の [section] より前**に置く必要がある。
+# section の後ろに書くとその section のキーとして解釈され、top-level には効かない。
+#
+# 未設定なら "master" (DEFAULT_BASE_BRANCH)。main 系リポジトリでは
+# `default_branch = "main"` を有効化すること。
+# default_branch = "master"
+
# ---------------------------------------------------------------------------
# [scratch_file_warning] — 順位 1 (PR #85 T1-4): scratch ファイル混入の防御層。
# `@` commit に `__*` 等の scratch ファイルが含まれていないか push 前に検査し、
@@ -152,7 +164,24 @@ commands = [
]
[diff]
-command = "jj diff -r @"
+# `{{PR_RANGE}}` は push-runner が `..@` に展開する。
+#
+# **exe と config は同時に更新すること** (ADR-051 cross-system config coupling):
+# プレースホルダ対応前の cli-push-runner.exe が本 config を読むと `{{PR_RANGE}}` が
+# そのまま jj に渡り `Failed to parse revset: Syntax error` で exit 5 になる
+# (2026-07-21 実観測)。fail-closed なので silent 誤動作にはならないが、
+# `pnpm build:cli-push-runner` (or `build:all`) で exe を更新すれば解消する。
+# **revset を直書きしないこと**: `-r @` のように PR より狭い範囲を書くと、祖先
+# コミットが AI レビューを一度も経ずに merge される (todo 順位 288、Severity High で
+# PR #268/#300/#301/#311 と 4 回再発)。レビュアー側からは渡された diff が PR 全体か
+# 検証できないため、誤りは誰にも検知されない。
+#
+# `--git` (unified diff) 必須。(a) 範囲カバレッジ検査が `diff --git` ヘッダから
+# ファイル一覧を抽出する、(b) jj 既定形式は色を落とすと `+`/`-` が消え LLM
+# レビュアーが削除を追加と誤読する (todo 順位 264、PR #256 で約 19 分浪費)。
+#
+# 生成された diff が PR 範囲を網羅していない場合は exit 5 で中断する (fail-closed)。
+command = "jj diff --git -r {{PR_RANGE}}"
output_path = ".takt/review-diff.txt"
# timeout (T6): 未指定時は 60s (DEFAULT_DIFF_TIMEOUT_SECS)。旧実装は無限待ちで、
# ADR-045 の並列 workspace 運用で jj lock 競合が起きるとパイプラインが無言ハングした。
diff --git a/src/cli-push-runner/src/config/docs_only_routing.rs b/src/cli-push-runner/src/config/docs_only_routing.rs
index a7466771..9dde0c15 100644
--- a/src/cli-push-runner/src/config/docs_only_routing.rs
+++ b/src/cli-push-runner/src/config/docs_only_routing.rs
@@ -1,13 +1,5 @@
use serde::Deserialize;
-/// docs-only routing の base branch 既定値。PR 範囲 `format!("{}..@", branch)` の base。
-///
-/// `[pr_size_check]` の `default_branch` と**論理的に同一の値でなければならない**
-/// (両者とも「PR の base branch」を指す)。config で別々に持つのは ADR-039 の
-/// section 独立性のためだが、値が食い違うと一方が誤った範囲を見る (ADR-051 の
-/// cross-config coupling)。`push-runner-config.toml` のコメントに同期義務を明記する。
-pub(crate) const DEFAULT_DOCS_ONLY_BASE_BRANCH: &str = "master";
-
/// docs-only と判定されたとき skip する quality_gate group の既定値。
///
/// `rust-lint-test` は diff が docs-only (ADR-035 path 基準) のとき結果が変わり得ない
@@ -49,14 +41,6 @@ pub(crate) struct DocsOnlyRoutingConfig {
}
impl DocsOnlyRoutingConfig {
- pub(crate) fn effective_default_branch(&self) -> String {
- self.default_branch
- .as_ref()
- .map(|s| s.trim().to_string())
- .filter(|s| !s.is_empty())
- .unwrap_or_else(|| DEFAULT_DOCS_ONLY_BASE_BRANCH.to_string())
- }
-
pub(crate) fn effective_skip_groups(&self) -> Vec {
match &self.skip_groups {
Some(groups) if !groups.is_empty() => groups.clone(),
@@ -94,11 +78,15 @@ command = "echo push"
BASE
);
let config = parse(&toml_str);
+ assert_eq!(
+ config.docs_only_pr_range(),
+ "main..@",
+ "section の default_branch は top-level より優先される (後方互換)"
+ );
let s = config
.docs_only_routing
.expect("[docs_only_routing] should parse to Some");
assert_eq!(s.enabled, Some(true));
- assert_eq!(s.effective_default_branch(), "main");
assert_eq!(s.effective_skip_groups(), vec!["rust-lint-test", "heavy"]);
}
@@ -115,9 +103,13 @@ command = "echo push"
fn effective_defaults_when_fields_omitted() {
let toml_str = format!("{}\n[docs_only_routing]\nenabled = true\n", BASE);
let config = parse(&toml_str);
+ assert_eq!(
+ config.docs_only_pr_range(),
+ format!("{}..@", crate::config::DEFAULT_BASE_BRANCH),
+ "section / top-level とも未設定なら共通の既定 base branch に倒れる"
+ );
let s = config.docs_only_routing.unwrap();
assert_eq!(s.enabled, Some(true));
- assert_eq!(s.effective_default_branch(), "master");
assert_eq!(s.effective_skip_groups(), vec!["rust-lint-test"]);
}
diff --git a/src/cli-push-runner/src/config/lint_screen.rs b/src/cli-push-runner/src/config/lint_screen.rs
index 6fce7037..c0c2296d 100644
--- a/src/cli-push-runner/src/config/lint_screen.rs
+++ b/src/cli-push-runner/src/config/lint_screen.rs
@@ -335,6 +335,7 @@ command = "echo push"
fn make_config_without_lint_screen() -> Config {
Config {
+ default_branch: None,
quality_gate: QualityGateConfig {
parallel: None,
step_timeout: None,
diff --git a/src/cli-push-runner/src/config/mod.rs b/src/cli-push-runner/src/config/mod.rs
index d8d0a556..20de7073 100644
--- a/src/cli-push-runner/src/config/mod.rs
+++ b/src/cli-push-runner/src/config/mod.rs
@@ -15,7 +15,7 @@ pub(crate) use lint_screen::{
DEFAULT_LINT_SCREEN_TIMEOUT_SECS,
};
pub(crate) use pr_size_check::{
- PrSizeCheckConfig, DEFAULT_PR_SIZE_BASE_BRANCH, DEFAULT_PR_SIZE_BLOCK_THRESHOLD,
+ PrSizeCheckConfig, DEFAULT_PR_SIZE_BLOCK_THRESHOLD,
DEFAULT_PR_SIZE_WARNING_THRESHOLD,
};
pub(crate) use scratch_file_warning::ScratchFileWarningConfig;
@@ -34,8 +34,29 @@ pub(crate) const DEFAULT_PUSH_TIMEOUT_SECS: u64 = 300;
/// 余裕側に倒す。詰まる環境では `[diff] timeout` で上書きする。
pub(crate) const DEFAULT_DIFF_TIMEOUT_SECS: u64 = 60;
+/// PR base branch の既定値。
+///
+/// 「PR 範囲」= `format!("{}..@", base_branch)` を組み立てる全 stage
+/// (`diff` / `docs_only_routing` / `pr_size_check`) がこの 1 箇所を参照する。
+/// 以前は section ごとに `default_branch` を持ち「値を同期する義務」を config の
+/// コメントで課していたが、義務はコード上の不変条件ではないため非対称を許した
+/// (実際に `[diff]` だけ PR 範囲を見ておらず、祖先コミットが AI レビュー未経由で
+/// merge される欠陥が 4 回再発した。todo 順位 288 / ADR-051 cross-config coupling)。
+pub(crate) const DEFAULT_BASE_BRANCH: &str = "master";
+
+/// branch 名を trim し、空文字を `None` に落とす (空白のみの設定値を未設定扱いにする)。
+fn normalize_branch(value: Option<&str>) -> Option {
+ value
+ .map(str::trim)
+ .filter(|s| !s.is_empty())
+ .map(str::to_string)
+}
+
#[derive(Deserialize)]
pub(crate) struct Config {
+ /// 全 stage 共通の PR base branch。section 側の `default_branch` が
+ /// 設定されていればそちらが優先される (後方互換のための override)。
+ pub(crate) default_branch: Option,
pub(crate) quality_gate: QualityGateConfig,
pub(crate) diff: Option,
pub(crate) lint_screen: Option,
@@ -48,6 +69,83 @@ pub(crate) struct Config {
pub(crate) post_takt_regate: Option,
}
+impl Config {
+ /// PR base branch を解決する。優先順:
+ /// section override → [`Config::effective_default_branch`] (top-level か、
+ /// 全一致する section override 群) → 既定値。
+ ///
+ /// section override は後方互換のために残してある (既存の派生プロジェクト config が
+ /// `[pr_size_check] default_branch` 等を持つため)。新規に section 側で持たせないこと。
+ /// override 値が stage 間で食い違わないことは [`validate_base_branch_ranges_agree`]
+ /// が `load_config` 時に fail-closed で保証する。
+ pub(crate) fn resolve_base_branch(&self, section_override: Option<&str>) -> String {
+ normalize_branch(section_override)
+ .or_else(|| self.effective_default_branch())
+ .unwrap_or_else(|| DEFAULT_BASE_BRANCH.to_string())
+ }
+
+ /// config 全体で共有する effective base branch を返す (CodeRabbit #313)。
+ ///
+ /// top-level `default_branch` が明示されていればそれ。無ければ section override 群が
+ /// **全て一致**していればその値を全 stage 共通の base とする。top-level を書かず
+ /// section override だけで base を揃えた legacy config で、override 未設定の stage
+ /// (例 `[diff]`) が `DEFAULT_BASE_BRANCH` に落ちる理由だけで
+ /// [`validate_base_branch_ranges_agree`] に reject されるのを防ぐ。override 群が
+ /// 食い違う場合は `None` を返し、各 stage が自分の override or 既定に解決した結果を
+ /// validate が fail-closed で検知する (genuine な不一致は従来どおり拒否)。
+ fn effective_default_branch(&self) -> Option {
+ if let Some(top) = normalize_branch(self.default_branch.as_deref()) {
+ return Some(top);
+ }
+ let overrides: Vec = [
+ self.diff.as_ref().and_then(|c| c.default_branch.as_deref()),
+ self.docs_only_routing
+ .as_ref()
+ .and_then(|c| c.default_branch.as_deref()),
+ self.pr_size_check
+ .as_ref()
+ .and_then(|c| c.default_branch.as_deref()),
+ ]
+ .into_iter()
+ .filter_map(normalize_branch)
+ .collect();
+ let first = overrides.first()?;
+ overrides.iter().all(|v| v == first).then(|| first.clone())
+ }
+
+ /// PR 範囲の revset (`..@`) を組み立てる。
+ ///
+ /// revset literal を各所に散らさないための唯一の組立点
+ /// (rule⑫ `no-hardcoded-jj-revset-range` の趣旨を config 側にも適用する)。
+ pub(crate) fn pr_range_revset(&self, section_override: Option<&str>) -> String {
+ format!("{}..@", self.resolve_base_branch(section_override))
+ }
+
+ /// AI レビュー対象 diff の PR 範囲。
+ pub(crate) fn diff_pr_range(&self) -> String {
+ self.pr_range_revset(self.diff.as_ref().and_then(|c| c.default_branch.as_deref()))
+ }
+
+ /// docs-only routing 判定の PR 範囲。[`Config::diff_pr_range`] と一致していなければ
+ /// 「レビューした範囲」と「routing を決めた範囲」がずれる。
+ pub(crate) fn docs_only_pr_range(&self) -> String {
+ self.pr_range_revset(
+ self.docs_only_routing
+ .as_ref()
+ .and_then(|c| c.default_branch.as_deref()),
+ )
+ }
+
+ /// PR size 計測の PR 範囲。
+ pub(crate) fn pr_size_pr_range(&self) -> String {
+ self.pr_range_revset(
+ self.pr_size_check
+ .as_ref()
+ .and_then(|c| c.default_branch.as_deref()),
+ )
+ }
+}
+
#[derive(Deserialize)]
pub(crate) struct QualityGateConfig {
pub(crate) parallel: Option,
@@ -82,12 +180,27 @@ pub(crate) struct PrePushReviewConfig {
pub(crate) refute_workflow: Option,
}
+/// `[diff] command` 内で PR 範囲 revset に展開されるプレースホルダ。
+///
+/// config に revset を直書きさせないための間接層。直書きを許すと
+/// 「`-r @` (tip のみ)」のような **PR より狭い範囲**を書けてしまい、
+/// 祖先コミットが AI レビューを一度も経ずに merge される (todo 順位 288、4 回再発)。
+pub(crate) const DIFF_PR_RANGE_PLACEHOLDER: &str = "{{PR_RANGE}}";
+
#[derive(Deserialize)]
pub(crate) struct DiffConfig {
+ /// diff 生成コマンド。[`DIFF_PR_RANGE_PLACEHOLDER`] が PR 範囲 revset に展開される。
+ ///
+ /// 出力は **unified diff (`--git` 形式)** である必要がある。範囲カバレッジ検査が
+ /// `diff --git a/… b/…` ヘッダからファイル一覧を抽出するため。jj の既定形式は
+ /// `+`/`-` マーカーを持たず LLM レビュアーが削除を追加と誤読する問題もある
+ /// (todo 順位 264、PR #256 で実害)。
pub(crate) command: String,
pub(crate) output_path: String,
/// 未指定時は `DEFAULT_DIFF_TIMEOUT_SECS` (T6)。`[push] timeout` と同形。
pub(crate) timeout: Option,
+ /// PR base branch の section override (後方互換)。未設定なら top-level を使う。
+ pub(crate) default_branch: Option,
}
#[derive(Deserialize)]
@@ -157,344 +270,59 @@ fn validate_config(config: &Config) -> Result<(), String> {
));
}
}
+ validate_diff_command_uses_pr_range(config)?;
+ validate_base_branch_ranges_agree(config)?;
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");
- assert!(diff.timeout.is_none());
- }
-
- /// T6: `[diff] timeout` 未指定時は既定値に落ちる (本リポジトリの config は未指定)。
- #[test]
- fn config_diff_timeout_defaults() {
- 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 = "w"
-task = "t"
-
-[push]
-command = "echo push"
-"#;
- let config: Config = toml::from_str(toml_str).unwrap();
- let diff = config.diff.unwrap();
- assert!(diff.timeout.is_none());
- assert_eq!(
- diff.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS),
- DEFAULT_DIFF_TIMEOUT_SECS,
- );
- }
-
- /// T6: 大 diff / 低速環境向けの escape hatch (既定 60s では足りない場合)。
- #[test]
- fn config_diff_timeout_explicit() {
- 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"
-timeout = 180
-
-[takt]
-workflow = "w"
-task = "t"
-
-[push]
-command = "echo push"
-"#;
- let config: Config = toml::from_str(toml_str).unwrap();
- assert_eq!(config.diff.unwrap().timeout, Some(180));
- }
-
- #[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,
- pre_push_review: None,
- docs_only_routing: None,
- post_takt_regate: 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,
- pre_push_review: None,
- docs_only_routing: None,
- post_takt_regate: 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'"));
- }
-
- /// resolve_takt_workflow テスト用に base config + 任意の [pre_push_review]
- /// section を組み立てる。base workflow は "pre-push-review"。
- fn config_with_optional_pre_push(pre_push_section: &str) -> Config {
- let toml_str = format!(
- r#"
-[quality_gate]
-[[quality_gate.groups]]
-name = "test"
-commands = ["echo ok"]
-
-[takt]
-workflow = "pre-push-review"
-task = "pre-push review"
-
-[push]
-command = "echo push"
-{pre_push_section}
-"#
- );
- toml::from_str(&toml_str).unwrap()
- }
-
- #[test]
- fn resolve_workflow_base_when_section_absent() {
- let config = config_with_optional_pre_push("");
- assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
- }
-
- #[test]
- fn resolve_workflow_base_when_refute_disabled() {
- let config = config_with_optional_pre_push(
- "[pre_push_review]\nrefute_enabled = false\nrefute_workflow = \"pre-push-review-refute\"",
- );
- assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
- }
-
- #[test]
- fn resolve_workflow_refute_when_enabled() {
- let config = config_with_optional_pre_push(
- "[pre_push_review]\nrefute_enabled = true\nrefute_workflow = \"pre-push-review-refute\"",
- );
- assert_eq!(resolve_takt_workflow(&config), "pre-push-review-refute");
+/// `[diff] command` が [`DIFF_PR_RANGE_PLACEHOLDER`] を含むことを config-load 時に検証する
+/// (CodeRabbit #313)。
+///
+/// プレースホルダを欠いた command (例: legacy の `jj diff -r @`) は PR 範囲を無視して
+/// tip のみをレビュー対象にし得る。範囲カバレッジ検査 (`stages::diff::verify_diff_covers_pr_range`)
+/// が runtime の backstop として残るが、config-load 時に明示エラーで弾くことで
+/// 「なぜ diff が狭い/空なのか」を実行時ではなく設定時点で判る診断に前倒しする
+/// (fail-closed / ADR-043、defense-in-depth)。`[diff]` section 不在時は無検査。
+fn validate_diff_command_uses_pr_range(config: &Config) -> Result<(), String> {
+ let Some(diff) = &config.diff else {
+ return Ok(());
+ };
+ if diff.command.contains(DIFF_PR_RANGE_PLACEHOLDER) {
+ return Ok(());
}
+ Err(format!(
+ "設定ファイルエラー: [diff] command が {placeholder} を含みません (現在: {command:?})。\
+ PR 範囲全体をレビュー対象にするため command には {placeholder} を使うこと \
+ (例: \"jj diff --git -r {placeholder}\")。直書きの範囲 (`-r @` 等) は tip のみを\
+ レビューし、祖先コミットが未レビューで merge される (todo 順位 288、4 回再発)",
+ placeholder = DIFF_PR_RANGE_PLACEHOLDER,
+ command = diff.command,
+ ))
+}
- #[test]
- fn resolve_workflow_base_when_enabled_but_no_refute_workflow() {
- let config = config_with_optional_pre_push("[pre_push_review]\nrefute_enabled = true");
- assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
+/// 3 stage (`diff` / `docs_only_routing` / `pr_size_check`) が解決する PR 範囲が
+/// 一致することを検査する (SIM-NEW-config-mod-rs-L69)。
+///
+/// section 側の `default_branch` override は後方互換のために残しているが、
+/// 各 stage が独立解決するため、override が top-level や他 section の値と
+/// 食い違うと `[diff]` だけが狭い範囲を見る非対称が復活する (todo 順位 288、
+/// 4 回再発)。「値を同期する義務」を config のコメントだけに頼らず、
+/// コード上の不変条件として fail-closed で強制する。
+fn validate_base_branch_ranges_agree(config: &Config) -> Result<(), String> {
+ let ranges = [
+ ("diff", config.diff_pr_range()),
+ ("docs_only_routing", config.docs_only_pr_range()),
+ ("pr_size_check", config.pr_size_pr_range()),
+ ];
+ let (_, diff_range) = &ranges[0];
+ if let Some((name, range)) = ranges.iter().find(|(_, range)| range != diff_range) {
+ return Err(format!(
+ "設定ファイルエラー: PR 範囲が stage 間で一致しません ([{name}] は \"{range}\" に解決、\
+ [diff] は \"{diff_range}\" に解決)。default_branch は top-level か、全 section で同じ値にすること"
+ ));
}
+ Ok(())
}
+
+#[cfg(test)]
+mod tests;
diff --git a/src/cli-push-runner/src/config/pr_size_check.rs b/src/cli-push-runner/src/config/pr_size_check.rs
index 4c73885e..d6463af9 100644
--- a/src/cli-push-runner/src/config/pr_size_check.rs
+++ b/src/cli-push-runner/src/config/pr_size_check.rs
@@ -9,10 +9,6 @@ pub(crate) const DEFAULT_PR_SIZE_WARNING_THRESHOLD: usize = 800;
/// 大型 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。
///
diff --git a/src/cli-push-runner/src/config/tests.rs b/src/cli-push-runner/src/config/tests.rs
new file mode 100644
index 00000000..3683b8b5
--- /dev/null
+++ b/src/cli-push-runner/src/config/tests.rs
@@ -0,0 +1,549 @@
+//! config のテスト (production は ./mod.rs)。ファイル長 800 行ガイドライン
+//! (順位 147) 遵守のため test mod を切り出した。
+
+use super::*;
+
+/// base branch 解決の 3 段 (section override → top-level → 既定値) と、
+/// **3 stage が同じ範囲に解決される**ことを固定する。
+///
+/// 後者が本 PR の要点: 以前は stage ごとに独立解決で、`[diff]` だけが PR 範囲を
+/// 見ていない非対称を許していた (todo 順位 288、4 回再発)。
+mod base_branch_resolution {
+ use super::*;
+
+ const MINIMAL: &str = r#"
+[quality_gate]
+[[quality_gate.groups]]
+name = "t"
+commands = ["echo ok"]
+
+[takt]
+workflow = "w"
+task = "t"
+
+[push]
+command = "echo push"
+
+[diff]
+command = "jj diff --git -r {{PR_RANGE}}"
+output_path = ".takt/d.txt"
+"#;
+
+ /// TOML の table 構文上、top-level key は**全 section より前**に置く必要が
+ /// あるため、prefix / suffix を分けて組み立てる。
+ fn parse(top_level: &str, sections: &str) -> Config {
+ toml::from_str(&format!("{}{}{}", top_level, MINIMAL, sections))
+ .expect("config should parse")
+ }
+
+ #[test]
+ fn all_stages_share_the_same_range_by_default() {
+ let config = parse("", "");
+ let expected = format!("{}..@", DEFAULT_BASE_BRANCH);
+ assert_eq!(config.diff_pr_range(), expected);
+ assert_eq!(config.docs_only_pr_range(), expected);
+ assert_eq!(config.pr_size_pr_range(), expected);
+ }
+
+ #[test]
+ fn top_level_default_branch_applies_to_all_stages() {
+ let config = parse("default_branch = \"main\"\n", "");
+ assert_eq!(config.diff_pr_range(), "main..@");
+ assert_eq!(config.docs_only_pr_range(), "main..@");
+ assert_eq!(config.pr_size_pr_range(), "main..@");
+ }
+
+ /// 後方互換: 既存の派生プロジェクト config が持つ section 側の
+ /// `default_branch` は top-level より優先される。この divergence を
+ /// 個々の resolve 関数レベルでは許すが、`load_config` の
+ /// `validate_config` は stage 間の不一致を fail-closed で拒否する
+ /// (`validate_config_rejects_disagreeing_section_override` 参照。
+ /// SIM-NEW-config-mod-rs-L69)。
+ #[test]
+ fn section_override_wins_over_top_level() {
+ let config = parse(
+ "default_branch = \"main\"\n",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \"develop\"\n",
+ );
+ assert_eq!(config.pr_size_pr_range(), "develop..@");
+ assert_eq!(
+ config.diff_pr_range(),
+ "main..@",
+ "override は指定した section にのみ効く"
+ );
+ }
+
+ #[test]
+ fn blank_override_falls_back_instead_of_producing_empty_range() {
+ let config = parse(
+ "default_branch = \"main\"\n",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \" \"\n",
+ );
+ assert_eq!(
+ config.pr_size_pr_range(),
+ "main..@",
+ "空白のみの override は未設定として扱う (`..@` を作らない)"
+ );
+ }
+
+ #[test]
+ fn blank_top_level_falls_back_to_default() {
+ let config = parse("default_branch = \"\"\n", "");
+ assert_eq!(config.diff_pr_range(), format!("{}..@", DEFAULT_BASE_BRANCH));
+ }
+
+ /// SIM-NEW-config-mod-rs-L69: section override が top-level / 他 section と
+ /// 食い違う config は、`section_override_wins_over_top_level` が示す通り
+ /// 個々の resolve 関数では「解決できてしまう」が、`load_config` が呼ぶ
+ /// `validate_config` はこれを fail-closed で拒否しなければならない。
+ #[test]
+ fn validate_config_rejects_disagreeing_section_override() {
+ let config = parse(
+ "default_branch = \"main\"\n",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \"develop\"\n",
+ );
+ let err = validate_config(&config).expect_err("disagreeing ranges must fail-closed");
+ assert!(err.contains("PR 範囲が stage 間で一致しません"), "{err}");
+ }
+
+ /// 全 stage が同じ値を明示していれば (本リポジトリの
+ /// `push-runner-config.toml` の実運用形) 検証を通す。
+ #[test]
+ fn validate_config_accepts_matching_section_overrides() {
+ let config = parse(
+ "default_branch = \"main\"\n",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \"main\"\n\
+ \n[docs_only_routing]\nenabled = true\ndefault_branch = \"main\"\n",
+ );
+ assert!(validate_config(&config).is_ok());
+ }
+
+ /// CodeRabbit #313: top-level 不在でも section override が全一致していれば、
+ /// override 未設定の stage (例 [diff]) もその値を共有する。旧実装は [diff] が
+ /// `DEFAULT_BASE_BRANCH` に落ち、他 section が "main" のとき「一致しない」と誤って
+ /// reject していた (valid legacy config の誤 reject)。
+ #[test]
+ fn absent_top_level_inherits_agreed_section_overrides() {
+ let config = parse(
+ "",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \"main\"\n\
+ \n[docs_only_routing]\nenabled = true\ndefault_branch = \"main\"\n",
+ );
+ assert_eq!(
+ config.diff_pr_range(),
+ "main..@",
+ "override 未設定の [diff] も全一致値を共有する"
+ );
+ assert_eq!(config.docs_only_pr_range(), "main..@");
+ assert_eq!(config.pr_size_pr_range(), "main..@");
+ assert!(
+ validate_config(&config).is_ok(),
+ "一致する section override のみ (top-level 不在) の config は通す"
+ );
+ }
+
+ /// top-level 不在で section override が食い違う場合は従来どおり fail-closed
+ /// (genuine な不一致は緩めない)。
+ #[test]
+ fn absent_top_level_with_disagreeing_section_overrides_still_fails_closed() {
+ let config = parse(
+ "",
+ "\n[pr_size_check]\nenabled = true\ndefault_branch = \"main\"\n\
+ \n[docs_only_routing]\nenabled = true\ndefault_branch = \"develop\"\n",
+ );
+ assert!(
+ validate_config(&config).is_err(),
+ "食い違う section override は genuine な不一致として reject"
+ );
+ }
+}
+
+/// CodeRabbit #313: `[diff] command` が `{{PR_RANGE}}` を欠く config は config-load 時に
+/// fail-closed で拒否する (legacy の `-r @` 直書きを設定時点で弾く)。
+#[test]
+fn validate_config_rejects_diff_command_without_pr_range_placeholder() {
+ let toml_str = r#"
+[quality_gate]
+[[quality_gate.groups]]
+name = "t"
+commands = ["echo ok"]
+
+[diff]
+command = "jj diff -r @"
+output_path = ".takt/d.txt"
+
+[takt]
+workflow = "w"
+task = "t"
+
+[push]
+command = "echo push"
+"#;
+ let config: Config = toml::from_str(toml_str).unwrap();
+ let err = validate_config(&config)
+ .expect_err("{{PR_RANGE}} を欠く [diff] command は fail-closed で拒否");
+ assert!(
+ err.contains(DIFF_PR_RANGE_PLACEHOLDER),
+ "診断に placeholder 名を含めること: {err}"
+ );
+}
+
+/// `{{PR_RANGE}}` を含む modern な command は通す (本リポジトリの実運用形)。
+#[test]
+fn validate_config_accepts_diff_command_with_pr_range_placeholder() {
+ let toml_str = r#"
+[quality_gate]
+[[quality_gate.groups]]
+name = "t"
+commands = ["echo ok"]
+
+[diff]
+command = "jj diff --git -r {{PR_RANGE}}"
+output_path = ".takt/d.txt"
+
+[takt]
+workflow = "w"
+task = "t"
+
+[push]
+command = "echo push"
+"#;
+ let config: Config = toml::from_str(toml_str).unwrap();
+ assert!(validate_config(&config).is_ok());
+}
+
+#[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");
+ assert!(diff.timeout.is_none());
+}
+
+/// T6: `[diff] timeout` 未指定時は既定値に落ちる (本リポジトリの config は未指定)。
+#[test]
+fn config_diff_timeout_defaults() {
+ 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 = "w"
+task = "t"
+
+[push]
+command = "echo push"
+"#;
+ let config: Config = toml::from_str(toml_str).unwrap();
+ let diff = config.diff.unwrap();
+ assert!(diff.timeout.is_none());
+ assert_eq!(
+ diff.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS),
+ DEFAULT_DIFF_TIMEOUT_SECS,
+ );
+}
+
+/// T6: 大 diff / 低速環境向けの escape hatch (既定 60s では足りない場合)。
+#[test]
+fn config_diff_timeout_explicit() {
+ 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"
+timeout = 180
+
+[takt]
+workflow = "w"
+task = "t"
+
+[push]
+command = "echo push"
+"#;
+ let config: Config = toml::from_str(toml_str).unwrap();
+ assert_eq!(config.diff.unwrap().timeout, Some(180));
+}
+
+#[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 {
+ default_branch: None,
+ quality_gate: QualityGateConfig {
+ parallel: None,
+ step_timeout: None,
+ groups: vec![],
+ },
+ diff: None,
+ lint_screen: None,
+ scratch_file_warning: None,
+ pr_size_check: None,
+ pre_push_review: None,
+ docs_only_routing: None,
+ post_takt_regate: 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 {
+ default_branch: None,
+ 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,
+ pre_push_review: None,
+ docs_only_routing: None,
+ post_takt_regate: 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'"));
+}
+
+/// resolve_takt_workflow テスト用に base config + 任意の [pre_push_review]
+/// section を組み立てる。base workflow は "pre-push-review"。
+fn config_with_optional_pre_push(pre_push_section: &str) -> Config {
+ let toml_str = format!(
+ r#"
+[quality_gate]
+[[quality_gate.groups]]
+name = "test"
+commands = ["echo ok"]
+
+[takt]
+workflow = "pre-push-review"
+task = "pre-push review"
+
+[push]
+command = "echo push"
+{pre_push_section}
+"#
+ );
+ toml::from_str(&toml_str).unwrap()
+}
+
+#[test]
+fn resolve_workflow_base_when_section_absent() {
+ let config = config_with_optional_pre_push("");
+ assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
+}
+
+#[test]
+fn resolve_workflow_base_when_refute_disabled() {
+ let config = config_with_optional_pre_push(
+ "[pre_push_review]\nrefute_enabled = false\nrefute_workflow = \"pre-push-review-refute\"",
+ );
+ assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
+}
+
+#[test]
+fn resolve_workflow_refute_when_enabled() {
+ let config = config_with_optional_pre_push(
+ "[pre_push_review]\nrefute_enabled = true\nrefute_workflow = \"pre-push-review-refute\"",
+ );
+ assert_eq!(resolve_takt_workflow(&config), "pre-push-review-refute");
+}
+
+#[test]
+fn resolve_workflow_base_when_enabled_but_no_refute_workflow() {
+ let config = config_with_optional_pre_push("[pre_push_review]\nrefute_enabled = true");
+ assert_eq!(resolve_takt_workflow(&config), "pre-push-review");
+}
diff --git a/src/cli-push-runner/src/main.rs b/src/cli-push-runner/src/main.rs
index fc640519..d88ca14f 100644
--- a/src/cli-push-runner/src/main.rs
+++ b/src/cli-push-runner/src/main.rs
@@ -67,7 +67,8 @@ fn run_diff_and_lint_screen(config: &config::Config) -> Result {
let Some(diff_config) = &config.diff else {
return Ok(DiffGate::RunTakt { pre_diff: None });
};
- let diff_path = match run_diff(diff_config) {
+ let pr_range = config.diff_pr_range();
+ let diff_path = match run_diff(diff_config, &pr_range) {
DiffResult::HasContent => diff_config.output_path.as_str(),
DiffResult::Empty => {
log_info("diff が空のためレビューをスキップして push に進みます。");
@@ -100,7 +101,7 @@ fn run_pre_checks(config: &config::Config) -> Result, i32> {
);
return Err(EXIT_SCRATCH_FILE_WARNING);
}
- if !run_pr_size_check(config.pr_size_check.as_ref()) {
+ if !run_pr_size_check(config.pr_size_check.as_ref(), &config.pr_size_pr_range()) {
log_info(
"パイプライン中断: PR diff サイズが block_threshold を超過。\
PR 分割 / 閾値調整 / `PR_SIZE_CHECK_OVERRIDE=1` のいずれかで再実行してください。",
@@ -195,7 +196,7 @@ fn run_stages(metrics: &mut RunMetrics) -> i32 {
metrics.set_bookmarks(&detected_bookmarks);
let skip_groups = metrics.timed("docs_only_routing", || {
- run_docs_only_routing(config.docs_only_routing.as_ref())
+ run_docs_only_routing(config.docs_only_routing.as_ref(), &config.docs_only_pr_range())
});
metrics.set_skipped_groups(&skip_groups);
diff --git a/src/cli-push-runner/src/stages/diff.rs b/src/cli-push-runner/src/stages/diff.rs
index 6ba3b726..a8272fe7 100644
--- a/src/cli-push-runner/src/stages/diff.rs
+++ b/src/cli-push-runner/src/stages/diff.rs
@@ -2,13 +2,26 @@
//!
//! 出力は takt の reviewers が Read で参照するレビュー対象そのもののため、
//! **切り詰めない** (`run_diff_cmd` の doc)。実行は timeout 付き (T6)。
+//!
+//! ## レビュー範囲は PR 全体
+//!
+//! `[diff] command` の [`DIFF_PR_RANGE_PLACEHOLDER`] は `Config::diff_pr_range()`
+//! (= `..@`) に展開される。以前は config に `jj diff -r @` と直書きされており、
+//! **tip コミットしかレビュアーに渡らなかった**。祖先コミットは AI レビューを一度も
+//! 経ずに merge され、しかもレビュアー側からは「渡された diff が PR 全体か」を
+//! 検証できないため誤りが誰にも検知されなかった (todo 順位 288、Severity High で
+//! PR #268/#300/#301/#311 と 4 回再発)。
+//!
+//! 範囲の直書きを禁じるだけでは派生プロジェクトの古い config を救えないため、
+//! [`verify_diff_covers_pr_range`] が生成された diff と PR 範囲の変更ファイル集合を
+//! 突き合わせ、不足があれば fail-closed で中断する。
use std::path::Path;
use std::process::Stdio;
use lib_subprocess::{drain_pipe_unlimited, shell_command, wait_with_timeout_safe};
-use crate::config::{DiffConfig, DEFAULT_DIFF_TIMEOUT_SECS};
+use crate::config::{DiffConfig, DEFAULT_DIFF_TIMEOUT_SECS, DIFF_PR_RANGE_PLACEHOLDER};
use crate::log::log_stage;
#[derive(Debug, PartialEq)]
@@ -83,6 +96,128 @@ fn run_diff_cmd(cmd: &str, timeout_secs: u64) -> Result {
}
}
+/// レビュー対象 diff が PR 範囲の全変更ファイルを含むか検査する (fail-closed / ADR-043)。
+///
+/// **なぜ必要か**: `[diff] command` は config 由来の自由文字列で、`-r @` のように
+/// PR より狭い範囲を書けてしまう。狭い範囲を書いても reviewers 側からは「渡された
+/// diff が全体か」を検証できず、正しく「docs-only」等と判定してしまうため、
+/// 誤りが誰にも検知されないまま merge に至る (todo 順位 288、Severity High で 4 回再発)。
+/// 検査の真実源は `jj diff --summary` = docs_only_routing / pr_size_check と同じ経路。
+///
+/// 判定不能 (summary 取得失敗 / summary の行が 1 つでもパースできない / diff がヘッダを
+/// 持たない) は「網羅している」に倒さずエラーにする。「検証できない」を「検証した」と
+/// 扱わないための線引き。
+///
+/// パースの厳格性は [`parse_summary_paths`] が担う: **1 行でも解釈できない行があれば
+/// `Err`**。妥当な行だけ拾って未知行を silent drop すると、jj の出力書式が一部だけ
+/// 変わったとき (例: 一部行だけ新 status) に、変わっていない行で `expected` が非空になり
+/// coverage を通過してしまう — 本 PR が塞いでいる「誤りが誰にも検知されない」構造その
+/// ものを gate 自身が再生産する (CodeRabbit #313: per-line fail-open)。
+fn verify_diff_covers_pr_range(
+ diff_output: &str,
+ fetch_summary: impl FnOnce() -> Result,
+) -> Result<(), String> {
+ let summary = fetch_summary().map_err(|e| format!("PR 範囲の summary 取得に失敗: {}", e))?;
+
+ let expected = parse_summary_paths(&summary).map_err(|e| {
+ format!(
+ "summary をパースできませんでした: {}。jj の出力書式が変わった可能性があります",
+ e
+ )
+ })?;
+ if expected.is_empty() {
+ return Ok(());
+ }
+
+ let covered = parse_git_diff_paths(diff_output);
+ if covered.is_empty() {
+ return Err(
+ "diff 出力に `diff --git` ヘッダが無く、対象ファイルを特定できません".to_string(),
+ );
+ }
+
+ let missing: Vec<&String> = expected.iter().filter(|p| !covered.contains(*p)).collect();
+ if missing.is_empty() {
+ return Ok(());
+ }
+ Err(format!(
+ "{} ファイルが未収録 (例: {})",
+ missing.len(),
+ missing
+ .iter()
+ .take(3)
+ .map(|s| s.as_str())
+ .collect::>()
+ .join(", ")
+ ))
+}
+
+/// `jj diff --summary` の `M path` / `R old new` (rename) / `C old new` (copy)
+/// 形式からパス集合を作る。
+///
+/// **空行を除く 1 行でも解釈できなければ `Err`** を返す (CodeRabbit #313)。妥当な行
+/// だけを拾って未知行を silent drop すると、jj の出力書式が一部だけ変わったとき
+/// (一部の行だけ新 status / status とパスに分割不能) に、変わっていない行で `expected`
+/// が非空になり `verify_diff_covers_pr_range` の coverage 検査を通過してしまう。
+/// 全行が未知のときだけ fail-closed する旧実装 (SIM-NEW-diff-rs-L178) の per-line 版。
+/// 全行が空 (末尾改行のみ 等) なら `Ok` の空集合を返す (= 変更なし)。
+///
+/// `R`/`C` は ` ` の 3 トークン形式 (`lib_docs_policy` の
+/// `is_docs_only_summary` テストが同じ `"R docs/a.md docs/b.md"` 形状を実証)。
+/// `parse_git_diff_paths` (`diff --git a/old b/new` の new = `b/` 側のみ拾う) と
+/// 揃えるため new path 側だけを採用する (SIM-NEW-diff-rs-L146)。
+///
+/// Windows の jj は `\` 区切りで出すため `/` に正規化して `--git` 側と突き合わせる。
+fn parse_summary_paths(
+ summary: &str,
+) -> Result, String> {
+ let mut paths = std::collections::BTreeSet::new();
+ for line in summary.lines() {
+ if line.trim().is_empty() {
+ continue;
+ }
+ let (status, rest) = line
+ .split_once(' ')
+ .ok_or_else(|| format!("status とパスに分割できない行: {:?}", line))?;
+ let path = summary_line_new_path(status, rest)
+ .ok_or_else(|| format!("未知 status / new path 欠落の行: {:?}", line))?;
+ paths.insert(path);
+ }
+ Ok(paths)
+}
+
+/// 1 行分の ` ` から採用すべきパスを返す (rename/copy は new path のみ)。
+/// 受理できない行は `None` を返し、呼び出し側 [`parse_summary_paths`] が `Err` に昇格して
+/// fail-closed に倒す。
+///
+/// `None` になるのは: 未知 status (`jj diff --summary` の M/A/D/R/C 以外) / R・C で
+/// new path (末尾トークン) が無い崩れた行 / パスが空。catch-all で任意 status や崩れた
+/// R/C を通すと、jj の出力書式が変わった行を「妥当なパス」として取り込み、書式変化を
+/// 検知できず gate が沈黙する (SIM-NEW-diff-rs-L178)。旧実装は崩れた R/C の生トークンを
+/// 残して coverage 不一致に頼っていたが、CodeRabbit #313 指摘に従い「未知 status も崩れた
+/// R/C も明示的に reject」へ統一する (fail-closed の判定を coverage 副作用でなく
+/// パース時点に前倒しする)。
+fn summary_line_new_path(status: &str, rest: &str) -> Option {
+ let trimmed = rest.trim();
+ let path = match status {
+ "M" | "A" | "D" => trimmed,
+ "R" | "C" => trimmed.rsplit_once(' ').map(|(_, new)| new)?,
+ _ => return None,
+ };
+ (!path.is_empty()).then(|| path.replace('\\', "/"))
+}
+
+/// unified diff の `diff --git a/X b/X` ヘッダからパス集合を作る。
+fn parse_git_diff_paths(diff_output: &str) -> std::collections::BTreeSet {
+ diff_output
+ .lines()
+ .filter_map(|line| line.strip_prefix("diff --git "))
+ .filter_map(|rest| rest.split_once(" b/"))
+ .map(|(_a_side, b_path)| b_path.trim().replace('\\', "/"))
+ .filter(|p| !p.is_empty())
+ .collect()
+}
+
/// takt 実行後の diff snapshot を取得する (T12 post-takt re-gate の変化検出用)。
///
/// Stage 1.5 と同じ `[diff] command` を再実行し stdout を返す。呼び出し側 (re-gate) は
@@ -91,17 +226,36 @@ fn run_diff_cmd(cmd: &str, timeout_secs: u64) -> Result {
/// 呼び出し側が fail-closed (= 変化ありとみなし re-gate 実行) に扱う (ADR-043)。
///
/// `run_diff` と違いファイルには書かない (比較のためメモリ上で保持するだけ)。timeout /
-/// stderr 分離の要件は `run_diff_cmd` と同一 (同 doc 参照)。
-pub(crate) fn capture_diff_snapshot(config: &DiffConfig) -> Option {
+/// stderr 分離の要件は `run_diff_cmd` と同一 (同 doc 参照)。範囲カバレッジ検査も
+/// 行わない (前後比較が目的で、レビュー入力にはならないため)。
+pub(crate) fn capture_diff_snapshot(config: &DiffConfig, pr_range: &str) -> Option {
let timeout = config.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS);
- run_diff_cmd(&config.command, timeout).ok()
+ run_diff_cmd(&resolve_diff_command(&config.command, pr_range), timeout).ok()
}
-pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
- log_stage("diff", &format!("実行: {}", config.command));
+/// `[diff] command` の [`DIFF_PR_RANGE_PLACEHOLDER`] を PR 範囲 revset に展開する。
+pub(crate) fn resolve_diff_command(command: &str, pr_range: &str) -> String {
+ command.replace(DIFF_PR_RANGE_PLACEHOLDER, pr_range)
+}
+
+pub(crate) fn run_diff(config: &DiffConfig, pr_range: &str) -> DiffResult {
+ run_diff_with(config, pr_range, || {
+ super::docs_only_routing::run_jj_diff_summary(pr_range)
+ })
+}
+
+/// `run_diff` の本体。PR 範囲の summary 取得を注入可能にして、範囲カバレッジ検査を
+/// jj 実行なしでテストできるようにする (`post_takt_regate::decide_regate` と同じ流儀)。
+fn run_diff_with(
+ config: &DiffConfig,
+ pr_range: &str,
+ fetch_summary: impl FnOnce() -> Result,
+) -> DiffResult {
+ let command = resolve_diff_command(&config.command, pr_range);
+ log_stage("diff", &format!("実行: {}", command));
let timeout = config.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS);
- let output = match run_diff_cmd(&config.command, timeout) {
+ let output = match run_diff_cmd(&command, timeout) {
Ok(output) => output,
Err(err) => {
log_stage("diff", "diff コマンド失敗");
@@ -112,6 +266,11 @@ pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
}
};
+ if let Err(reason) = verify_diff_covers_pr_range(&output, fetch_summary) {
+ report_coverage_failure(pr_range, &reason);
+ return DiffResult::Error;
+ }
+
if output.is_empty() {
log_stage(
"diff",
@@ -120,7 +279,25 @@ pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
return DiffResult::Empty;
}
- let path = Path::new(&config.output_path);
+ write_diff_output(&config.output_path, &output)
+}
+
+/// 範囲検査に落ちたときの fail-closed 通知 (ADR-043)。
+///
+/// 「レビュー範囲が PR より狭い」ことは検知できても自動修復はできない
+/// (どこまで広げるべきかは config の意図次第) ため、push を止めて人間に返す。
+fn report_coverage_failure(pr_range: &str, reason: &str) {
+ log_stage("diff", &format!("レビュー範囲の検査に失敗: {}", reason));
+ eprintln!(
+ "[push-runner] [diff] レビュー対象 diff が PR 範囲 ({}) を網羅していません: {}\n\
+ このまま進めると祖先コミットが AI レビューを経ずに merge されます (todo 順位 288)。\n\
+ `[diff] command` が `{}` を使っているか、出力が unified diff (--git) 形式かを確認してください。",
+ pr_range, reason, DIFF_PR_RANGE_PLACEHOLDER
+ );
+}
+
+fn write_diff_output(output_path: &str, output: &str) -> DiffResult {
+ let path = Path::new(output_path);
if let Some(parent) = path.parent() {
if let Err(e) = std::fs::create_dir_all(parent) {
log_stage("diff", &format!("ディレクトリ作成失敗: {}", e));
@@ -128,12 +305,12 @@ pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
}
}
- match std::fs::write(path, &output) {
+ match std::fs::write(path, output) {
Ok(()) => {
let line_count = output.lines().count();
log_stage(
"diff",
- &format!("書き出し完了: {} ({} 行)", config.output_path, line_count),
+ &format!("書き出し完了: {} ({} 行)", output_path, line_count),
);
DiffResult::HasContent
}
@@ -145,239 +322,4 @@ pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
}
#[cfg(test)]
-mod tests {
- use super::*;
-
- /// 100 行を吐くコマンド。cmd.exe と POSIX sh で構文が非互換なため OS 別に
- /// 出し分ける (WP-15)。POSIX 側は `seq` 不在の最小環境でも動く while ループ。
- #[cfg(windows)]
- const EMIT_100_LINES_CMD: &str = "for /L %i in (1,1,100) do @echo line %i";
- #[cfg(not(windows))]
- const EMIT_100_LINES_CMD: &str = "i=1; while [ $i -le 100 ]; do echo line $i; i=$((i+1)); done";
-
- /// 何も出力せず正常終了するコマンド (0 バイト出力の検証用)。
- #[cfg(windows)]
- const ZERO_BYTE_OUTPUT_CMD: &str = "type nul";
- #[cfg(not(windows))]
- const ZERO_BYTE_OUTPUT_CMD: &str = "true";
-
- /// stderr へ出力してから非 0 で終わるコマンド (失敗診断の検証用)。
- #[cfg(windows)]
- const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2& exit /b 1";
- #[cfg(not(windows))]
- const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2; exit 1";
-
- /// stdout と stderr の両方へ出しつつ正常終了するコマンド。
- /// stderr (jj の警告相当) が diff 本体に混ざらない契約の検証用。
- #[cfg(windows)]
- const STDOUT_AND_STDERR_CMD: &str = "echo real diff& echo Concurrent modification 1>&2";
- #[cfg(not(windows))]
- const STDOUT_AND_STDERR_CMD: &str = "echo real diff; echo Concurrent modification 1>&2";
-
- #[test]
- fn run_diff_cmd_captures_more_than_40_lines() {
- let result = run_diff_cmd(EMIT_100_LINES_CMD, 30);
- assert!(result.is_ok(), "command should succeed");
- let output = result.unwrap();
- let line_count = output.lines().count();
- assert!(
- line_count > 40,
- "expected >40 lines captured, got {}; run_diff_cmd must not apply the 40-line cap",
- line_count
- );
- }
-
- #[test]
- fn run_diff_returns_empty_when_output_is_empty() {
- let out_path = std::env::temp_dir().join("test-run-diff-empty.txt");
- let _ = std::fs::remove_file(&out_path);
-
- let config = DiffConfig {
- command: ZERO_BYTE_OUTPUT_CMD.to_string(),
- output_path: out_path.to_string_lossy().into_owned(),
- timeout: None,
- };
-
- let result = run_diff(&config);
-
- assert_eq!(
- result,
- DiffResult::Empty,
- "run_diff must return Empty when the diff command produces empty output"
- );
- assert!(
- !out_path.exists(),
- "output file must not be created for an empty diff"
- );
- }
-
- /// T12: capture_diff_snapshot は成功時に stdout を Some で返す
- /// (post-takt re-gate の pre/post 比較の材料)。
- #[test]
- fn capture_diff_snapshot_returns_output_on_success() {
- let config = DiffConfig {
- command: "echo snapshot-content".to_string(),
- output_path: String::new(),
- timeout: None,
- };
- let snap = capture_diff_snapshot(&config).expect("成功時は Some");
- assert!(
- snap.contains("snapshot-content"),
- "stdout をそのまま返すこと: {:?}",
- snap
- );
- }
-
- /// T12: 取得失敗 (コマンド exit 非 0) は None を返す
- /// (呼び出し側は None を fail-closed = 変化ありに倒す)。
- #[test]
- fn capture_diff_snapshot_returns_none_on_failure() {
- let config = DiffConfig {
- command: STDERR_THEN_FAIL_CMD.to_string(),
- output_path: String::new(),
- timeout: Some(30),
- };
- assert!(
- capture_diff_snapshot(&config).is_none(),
- "失敗時は None (呼び出し側で fail-closed に扱う)"
- );
- }
-
- /// T6 回帰テスト群: diff stage に timeout が無く無限ハングし得た不具合
- /// (ADR-049 の流儀: 1 test = 1 failure mode + good/bad)。
- ///
- /// 由来: 2026-07-16 の push パイプライン調査 (コード監査で発見。T5 と同じく
- /// in the wild の発火記録は無く、「他 stage は全て timeout 付き = diff だけが穴」
- /// という非対称として特定された)。
- ///
- /// 事故の形: `run_diff_cmd` は `Command::output()` で子プロセスの終了を**無限に**
- /// 待っていた。ADR-045 の並列 workspace 運用で jj の lock 競合が起きると
- /// `pnpm push` は診断も timeout も無いまま停止し、ユーザーは手動 kill するしかない。
- ///
- /// 修正の核心は「timeout 付きで待ち、超過時は Err → `DiffResult::Error` = exit 5 で
- /// 中断する (fail-closed / ADR-043)」。あわせて、判定に使う stdout を stderr と
- /// 混ぜない契約 (レビュー対象を汚さない) も本 mod で seal する。
- mod t6_diff_timeout {
- use super::*;
- use std::time::{Duration, Instant};
-
- /// 実行し続けるコマンド (ハングした jj の代役)。timeout が無ければ約 9s 待たされる。
- /// cmd.exe と POSIX sh で構文が非互換なため OS 別に出し分ける (WP-15)。
- /// 所要時間を両 OS で揃えないと片側だけ timeout 経路を検証しない穴になる。
- #[cfg(windows)]
- const HANGING_COMMAND: &str = "ping 127.0.0.1 -n 10";
- #[cfg(not(windows))]
- const HANGING_COMMAND: &str = "sleep 10";
-
- const SHORT_TIMEOUT_SECS: u64 = 1;
-
- /// incident 再現 (bad): 応答しないコマンドを **timeout で打ち切る**こと。
- /// 修正前は `Command::output()` が返るまで待ち続け、本 assert には到達しなかった。
- #[test]
- fn hanging_command_times_out_instead_of_waiting_forever() {
- let started = Instant::now();
- let result = run_diff_cmd(HANGING_COMMAND, SHORT_TIMEOUT_SECS);
- let elapsed = started.elapsed();
-
- let err = result.expect_err("timeout は Err で返ること (無限待ちしない)");
- assert!(
- err.contains("タイムアウト"),
- "timeout と判る診断を返すこと: {:?}",
- err,
- );
- assert!(
- elapsed < Duration::from_secs(5),
- "timeout ({}s) 後すぐ制御を返すこと。{:?} 掛かった = コマンドの自然終了を\
- 待っている (T6 の不具合)",
- SHORT_TIMEOUT_SECS,
- elapsed,
- );
- }
-
- /// timeout の診断は原因調査に足りること: 超過秒数と実行コマンドを含む。
- #[test]
- fn timeout_error_reports_the_limit_and_the_command() {
- let err = run_diff_cmd(HANGING_COMMAND, SHORT_TIMEOUT_SECS)
- .expect_err("timeout は Err で返ること");
- assert!(
- err.contains(&format!("{}s", SHORT_TIMEOUT_SECS)) && err.contains(HANGING_COMMAND),
- "超過秒数と実行コマンドを診断に含めること: {:?}",
- err,
- );
- }
-
- /// timeout は fail-closed で pipeline を止めること (ADR-043)。
- /// `DiffResult::Error` は main.rs で exit 5 = 中断になる。空 diff 扱いで
- /// **レビューを skip したまま push に進んではならない**。
- #[test]
- fn timeout_aborts_the_pipeline_and_writes_no_diff_file() {
- let out_path = std::env::temp_dir().join("test-run-diff-timeout.txt");
- let _ = std::fs::remove_file(&out_path);
-
- let config = DiffConfig {
- command: HANGING_COMMAND.to_string(),
- output_path: out_path.to_string_lossy().into_owned(),
- timeout: Some(SHORT_TIMEOUT_SECS),
- };
-
- assert_eq!(
- run_diff(&config),
- DiffResult::Error,
- "timeout は Error (= exit 5 で中断) になること。Empty だとレビューを\
- skip して push に進んでしまう",
- );
- assert!(
- !out_path.exists(),
- "timeout 時に diff ファイルを書かないこと (古い/欠けた diff でレビューさせない)",
- );
- }
-
- /// good: timeout 内に終わるコマンドを誤って打ち切らないこと。
- #[test]
- fn command_within_the_timeout_succeeds() {
- let output = run_diff_cmd("echo diff line", 30).expect("即終了するコマンドは Ok");
- assert!(output.contains("diff line"), "stdout を返すこと: {:?}", output);
- }
-
- /// `[diff] timeout` 未指定なら既定値が使われること (既定値の適用漏れ防止)。
- #[test]
- fn absent_config_timeout_falls_back_to_the_default() {
- let config = DiffConfig {
- command: "echo ok".to_string(),
- output_path: std::env::temp_dir()
- .join("test-run-diff-default-timeout.txt")
- .to_string_lossy()
- .into_owned(),
- timeout: None,
- };
- assert_eq!(
- config.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS),
- DEFAULT_DIFF_TIMEOUT_SECS,
- );
- assert_eq!(run_diff(&config), DiffResult::HasContent);
- let _ = std::fs::remove_file(&config.output_path);
- }
-
- /// stderr を stdout に混ぜないこと: stdout は reviewers が読む diff そのものとして
- /// ファイルに書かれるため、jj の警告 (並列 workspace 時の `Concurrent modification
- /// detected` 等) が混入するとレビュー対象を汚す。`run_cmd_shell_*` (全 variant が
- /// stdout/stderr を結合する) に載せ替えるとこのテストが落ちる。
- #[test]
- fn stderr_is_not_merged_into_the_diff_output() {
- let output = run_diff_cmd(STDOUT_AND_STDERR_CMD, 30).expect("exit 0 なら Ok");
- assert!(output.contains("real diff"), "stdout は残ること: {:?}", output);
- assert!(
- !output.contains("Concurrent modification"),
- "stderr の警告が diff 内容に混入しないこと: {:?}",
- output,
- );
- }
-
- /// 失敗時は stderr を診断として返すこと (従来契約の維持)。
- #[test]
- fn failure_returns_stderr_as_the_diagnostic() {
- let err = run_diff_cmd(STDERR_THEN_FAIL_CMD, 30).expect_err("exit 1 は Err");
- assert!(err.contains("boom"), "stderr を診断に返すこと: {:?}", err);
- }
- }
-}
+mod tests;
diff --git a/src/cli-push-runner/src/stages/diff/tests.rs b/src/cli-push-runner/src/stages/diff/tests.rs
new file mode 100644
index 00000000..fc152ceb
--- /dev/null
+++ b/src/cli-push-runner/src/stages/diff/tests.rs
@@ -0,0 +1,506 @@
+//! diff stage のテスト (production は ../diff.rs)。ファイル長 800 行ガイドライン
+//! (順位 147) 遵守のため #312 pipeline_lock と同じく test mod を切り出した。
+
+use super::*;
+
+/// 範囲検査を jj 非依存でテストするための PR 範囲 revset。
+fn test_pr_range() -> String {
+ format!("{}..@", crate::config::DEFAULT_BASE_BRANCH)
+}
+
+/// PR #268/#300/#301/#311 の incident 形状: PR に 2 ファイル変更があるのに、
+/// レビュー対象 diff には tip コミットの 1 ファイルしか入っていない。
+const INCIDENT_PR_SUMMARY: &str = "M docs/plan.md\nM src/checker/decide.rs\n";
+const INCIDENT_TIP_ONLY_DIFF: &str =
+ "diff --git a/docs/plan.md b/docs/plan.md\n@@ -1 +1 @@\n-a\n+b\n";
+
+#[test]
+fn resolve_diff_command_expands_pr_range_placeholder() {
+ let range = test_pr_range();
+ assert_eq!(
+ resolve_diff_command("jj diff --git -r {{PR_RANGE}}", &range),
+ format!("jj diff --git -r {}", range)
+ );
+}
+
+/// base branch が `main` の派生プロジェクトでも展開が成立すること
+/// (rule⑫ が防ごうとしている alternative branch の silent breakage)。
+#[test]
+fn resolve_diff_command_expands_alternative_base_branch() {
+ assert_eq!(
+ resolve_diff_command("jj diff --git -r {{PR_RANGE}}", "main..@"),
+ "jj diff --git -r main..@"
+ );
+}
+
+/// incident 再現: レビュー対象 diff が PR 範囲より狭いことを検知する。
+/// 旧実装にはこの検査が無く、狭い diff が「全体」として reviewers に渡っていた。
+#[test]
+fn verify_rejects_diff_narrower_than_pr_range() {
+ let result = verify_diff_covers_pr_range(INCIDENT_TIP_ONLY_DIFF, || {
+ Ok(INCIDENT_PR_SUMMARY.to_string())
+ });
+ let err = result.expect_err("PR 範囲より狭い diff は検知されなければならない");
+ assert!(
+ err.contains("src/checker/decide.rs"),
+ "未収録ファイル名を示すこと: {}",
+ err
+ );
+}
+
+#[test]
+fn verify_accepts_diff_covering_whole_pr_range() {
+ let full_diff = format!(
+ "{}diff --git a/src/checker/decide.rs b/src/checker/decide.rs\n@@ -1 +1 @@\n-x\n+y\n",
+ INCIDENT_TIP_ONLY_DIFF
+ );
+ assert!(
+ verify_diff_covers_pr_range(&full_diff, || Ok(INCIDENT_PR_SUMMARY.to_string())).is_ok()
+ );
+}
+
+/// Windows の jj は `--summary` を `\` 区切りで出すため、正規化しないと
+/// 全ファイルが「未収録」に見えて常時 fail する。
+#[test]
+fn verify_normalizes_windows_path_separators() {
+ let summary = "M docs\\plan.md\n";
+ assert!(verify_diff_covers_pr_range(INCIDENT_TIP_ONLY_DIFF, || Ok(
+ summary.to_string()
+ ))
+ .is_ok());
+}
+
+/// SIM-NEW-diff-rs-L146: rename 行 (`R `) は new path だけを
+/// パス集合に採用すること。旧実装は old+new を 1 個の壊れたパスにしてしまい、
+/// coverage 検査が rename ファイルを絶対に "covered" と一致させられなかった。
+#[test]
+fn parse_summary_paths_extracts_new_path_for_rename() {
+ let paths = parse_summary_paths("R docs/a.md docs/b.md\n").expect("valid rename は Ok");
+ assert_eq!(
+ paths,
+ std::collections::BTreeSet::from(["docs/b.md".to_string()]),
+ "rename は new path のみを採用し、old path や結合パスを含めないこと: {:?}",
+ paths
+ );
+}
+
+/// copy 行 (`C `) も rename と同じ扱い (new path のみ採用)。
+#[test]
+fn parse_summary_paths_extracts_new_path_for_copy() {
+ let paths = parse_summary_paths("C docs/a.md docs/c.md\n").expect("valid copy は Ok");
+ assert_eq!(
+ paths,
+ std::collections::BTreeSet::from(["docs/c.md".to_string()]),
+ "copy は new path のみを採用すること: {:?}",
+ paths
+ );
+}
+
+/// incident 再現 (rename 版): rename されたファイルを含む PR 範囲でも、
+/// new path を収録した diff は coverage 検査を通ること。
+#[test]
+fn verify_accepts_diff_with_renamed_file() {
+ let summary = "R docs/a.md docs/b.md\n";
+ let diff = "diff --git a/docs/a.md b/docs/b.md\n@@ -1 +1 @@\n-old\n+new\n";
+ assert!(
+ verify_diff_covers_pr_range(diff, || Ok(summary.to_string())).is_ok(),
+ "rename ファイルの new path が --git ヘッダの b/ 側と一致し、coverage 検査を\
+ 通ること (旧実装は old+new の壊れたパスと一致せず常に未収録扱いだった)"
+ );
+}
+
+/// summary 取得に失敗したら「網羅している」に倒さない (fail-closed / ADR-043)。
+#[test]
+fn verify_fails_closed_when_summary_unavailable() {
+ let result =
+ verify_diff_covers_pr_range(INCIDENT_TIP_ONLY_DIFF, || Err("jj 失敗".to_string()));
+ assert!(
+ result.is_err(),
+ "検証できないことを検証したものとして扱ってはならない"
+ );
+}
+
+/// jj 既定形式 (`diff --git` ヘッダを持たない) では収録ファイルを特定できないため
+/// fail-closed にする。--git 形式への切替 (todo 順位 264) を機械的に要求する層。
+#[test]
+fn verify_fails_closed_when_diff_has_no_git_headers() {
+ let plain_format_diff = "Modified regular file docs/plan.md:\n 1 1: -a\n";
+ let result = verify_diff_covers_pr_range(plain_format_diff, || {
+ Ok(INCIDENT_PR_SUMMARY.to_string())
+ });
+ assert!(result.is_err(), "形式不明なら検証済みと扱わない");
+}
+
+/// PR 範囲が空 (変更なし) なら検査は素通しする (空 diff は上流で Empty 扱い)。
+#[test]
+fn verify_passes_when_pr_range_is_empty() {
+ assert!(verify_diff_covers_pr_range("", || Ok(String::new())).is_ok());
+}
+
+/// security-review 指摘: summary に行はあるのに 1 件もパースできない (= jj の
+/// 出力書式変更) 場合、空集合に丸めて素通しすると gate が無言で機能停止する。
+/// 「行はあるがパース不能」と「そもそも変更なし」を区別して前者は fail-closed。
+///
+/// 入力は**未知 status のみ**の行にして、意図した「パース不能」分岐を確実に突く
+/// (旧テストは status を含む行が catch-all で妥当パス化され、後段の「missing files」
+/// 分岐でたまたま Err になっていた = 主張と別分岐を検証していた。SIM-NEW-diff-rs-L178)。
+#[test]
+fn verify_fails_closed_when_summary_is_unparseable() {
+ let unparseable = "X docs/plan.md\nZ src/checker/decide.rs\n";
+ let result =
+ verify_diff_covers_pr_range(INCIDENT_TIP_ONLY_DIFF, || Ok(unparseable.to_string()));
+ let err = result.expect_err("パース不能を素通しすると gate が無言で死ぬ");
+ assert!(
+ err.contains("パースできませんでした"),
+ "『missing files』ではなく『summary パース不能』分岐で fail-closed すべき: {}",
+ err
+ );
+}
+
+/// CodeRabbit #313 (per-line fail-open): valid な docs/plan.md は diff に含まれるが、
+/// 未知 status の src/checker/decide.rs 行が混ざる。旧実装は未知行を silent drop し、
+/// docs/plan.md だけで coverage を通していた。1 行でも解釈不能なら fail-closed。
+#[test]
+fn verify_fails_closed_when_summary_has_mixed_unparseable_line() {
+ let mixed = "M docs/plan.md\nX src/checker/decide.rs\n";
+ let result = verify_diff_covers_pr_range(INCIDENT_TIP_ONLY_DIFF, || Ok(mixed.to_string()));
+ let err = result.expect_err("valid 行と未知行の混在は fail-closed");
+ assert!(
+ err.contains("パースできませんでした"),
+ "混在 summary は『summary パース不能』分岐で fail-closed すべき: {}",
+ err
+ );
+}
+
+/// 未知 status 行が「妥当なパス」として取り込まれないこと (catch-all で status を
+/// 素通しすると書式変化を検知できなくなる。SIM-NEW-diff-rs-L178)。
+#[test]
+fn parse_summary_paths_rejects_unknown_status() {
+ assert!(
+ parse_summary_paths("X docs/plan.md\n").is_err(),
+ "未知 status 行は Err (silent drop しない)"
+ );
+ assert_eq!(
+ parse_summary_paths("M docs/plan.md\n").expect("既知 status は Ok"),
+ std::collections::BTreeSet::from(["docs/plan.md".to_string()]),
+ "既知 status (M/A/D/R/C) は従来どおり受理する"
+ );
+}
+
+/// 書式が崩れた R/C 行 (2 トークン目が無い) を **明示的に reject** すること
+/// (CodeRabbit #313: 旧実装は生トークンを残して coverage 不一致に頼っていたが、
+/// fail-closed の判定をパース時点に前倒しする)。
+#[test]
+fn parse_summary_paths_rejects_malformed_rename_line() {
+ assert!(
+ parse_summary_paths("R docs/only-one-token.md\n").is_err(),
+ "new path (末尾トークン) が無い崩れた rename 行は Err"
+ );
+ assert_eq!(
+ parse_summary_paths("R docs/a.md docs/b.md\n").expect("正常 rename は Ok"),
+ std::collections::BTreeSet::from(["docs/b.md".to_string()]),
+ "正常な rename は new path のみ"
+ );
+}
+
+/// CodeRabbit #313: valid 行と未知 status 行が混在した summary は、valid 行だけで
+/// `expected` が非空になり coverage を通過してしまう per-line fail-open があった。
+/// 1 行でも解釈不能なら全体を Err にして fail-closed に倒す。
+#[test]
+fn parse_summary_paths_rejects_mixed_valid_and_unparseable_lines() {
+ assert!(
+ parse_summary_paths("M docs/plan.md\nX src/foo.rs\n").is_err(),
+ "valid 行があっても未知 status 行が 1 つでもあれば Err (silent drop しない)"
+ );
+}
+
+/// run_diff の実経路で範囲不足が Error になること (report 経路まで含めた固定)。
+#[test]
+fn run_diff_errors_when_coverage_check_fails() {
+ let out_path = std::env::temp_dir().join("test-run-diff-coverage.txt");
+ let _ = std::fs::remove_file(&out_path);
+ let config = DiffConfig {
+ command: format!("echo {}", INCIDENT_TIP_ONLY_DIFF.lines().next().unwrap()),
+ output_path: out_path.to_string_lossy().into_owned(),
+ timeout: Some(30),
+ default_branch: None,
+ };
+
+ let result = run_diff_with(&config, &test_pr_range(), || {
+ Ok(INCIDENT_PR_SUMMARY.to_string())
+ });
+
+ assert_eq!(result, DiffResult::Error);
+ assert!(
+ !out_path.exists(),
+ "範囲不足の diff はレビュー入力として書き出さない"
+ );
+}
+
+/// CodeRabbit #313: diff コマンドが空出力でも、PR 範囲に変更があれば (summary 非空)
+/// coverage 検査で fail-closed にする。旧実装は `output.is_empty()` を coverage 検査
+/// より先に評価し、範囲がずれて空になった diff を「レビュー対象なし」と誤認して
+/// gate を素通りさせていた。
+#[test]
+fn run_diff_errors_when_output_empty_but_pr_range_has_changes() {
+ let out_path = std::env::temp_dir().join("test-run-diff-empty-but-changes.txt");
+ let _ = std::fs::remove_file(&out_path);
+ let config = DiffConfig {
+ command: ZERO_BYTE_OUTPUT_CMD.to_string(),
+ output_path: out_path.to_string_lossy().into_owned(),
+ timeout: Some(30),
+ default_branch: None,
+ };
+
+ let result = run_diff_with(&config, &test_pr_range(), || {
+ Ok(INCIDENT_PR_SUMMARY.to_string())
+ });
+
+ assert_eq!(
+ result,
+ DiffResult::Error,
+ "空出力でも PR 範囲に変更があれば coverage 検査で fail-closed になること"
+ );
+ assert!(
+ !out_path.exists(),
+ "coverage 不足では diff ファイルを書き出さない"
+ );
+}
+
+/// 100 行を吐くコマンド。cmd.exe と POSIX sh で構文が非互換なため OS 別に
+/// 出し分ける (WP-15)。POSIX 側は `seq` 不在の最小環境でも動く while ループ。
+#[cfg(windows)]
+const EMIT_100_LINES_CMD: &str = "for /L %i in (1,1,100) do @echo line %i";
+#[cfg(not(windows))]
+const EMIT_100_LINES_CMD: &str = "i=1; while [ $i -le 100 ]; do echo line $i; i=$((i+1)); done";
+
+/// 何も出力せず正常終了するコマンド (0 バイト出力の検証用)。
+#[cfg(windows)]
+const ZERO_BYTE_OUTPUT_CMD: &str = "type nul";
+#[cfg(not(windows))]
+const ZERO_BYTE_OUTPUT_CMD: &str = "true";
+
+/// stderr へ出力してから非 0 で終わるコマンド (失敗診断の検証用)。
+#[cfg(windows)]
+const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2& exit /b 1";
+#[cfg(not(windows))]
+const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2; exit 1";
+
+/// stdout と stderr の両方へ出しつつ正常終了するコマンド。
+/// stderr (jj の警告相当) が diff 本体に混ざらない契約の検証用。
+#[cfg(windows)]
+const STDOUT_AND_STDERR_CMD: &str = "echo real diff& echo Concurrent modification 1>&2";
+#[cfg(not(windows))]
+const STDOUT_AND_STDERR_CMD: &str = "echo real diff; echo Concurrent modification 1>&2";
+
+#[test]
+fn run_diff_cmd_captures_more_than_40_lines() {
+ let result = run_diff_cmd(EMIT_100_LINES_CMD, 30);
+ assert!(result.is_ok(), "command should succeed");
+ let output = result.unwrap();
+ let line_count = output.lines().count();
+ assert!(
+ line_count > 40,
+ "expected >40 lines captured, got {}; run_diff_cmd must not apply the 40-line cap",
+ line_count
+ );
+}
+
+#[test]
+fn run_diff_returns_empty_when_output_is_empty() {
+ let out_path = std::env::temp_dir().join("test-run-diff-empty.txt");
+ let _ = std::fs::remove_file(&out_path);
+
+ let config = DiffConfig {
+ command: ZERO_BYTE_OUTPUT_CMD.to_string(),
+ output_path: out_path.to_string_lossy().into_owned(),
+ timeout: None,
+ default_branch: None,
+ };
+
+ let result = run_diff_with(&config, &test_pr_range(), || Ok(String::new()));
+
+ assert_eq!(
+ result,
+ DiffResult::Empty,
+ "run_diff must return Empty when the diff command produces empty output"
+ );
+ assert!(
+ !out_path.exists(),
+ "output file must not be created for an empty diff"
+ );
+}
+
+/// T12: capture_diff_snapshot は成功時に stdout を Some で返す
+/// (post-takt re-gate の pre/post 比較の材料)。
+#[test]
+fn capture_diff_snapshot_returns_output_on_success() {
+ let config = DiffConfig {
+ command: "echo snapshot-content".to_string(),
+ output_path: String::new(),
+ timeout: None,
+ default_branch: None,
+ };
+ let snap = capture_diff_snapshot(&config, &test_pr_range()).expect("成功時は Some");
+ assert!(
+ snap.contains("snapshot-content"),
+ "stdout をそのまま返すこと: {:?}",
+ snap
+ );
+}
+
+/// T12: 取得失敗 (コマンド exit 非 0) は None を返す
+/// (呼び出し側は None を fail-closed = 変化ありに倒す)。
+#[test]
+fn capture_diff_snapshot_returns_none_on_failure() {
+ let config = DiffConfig {
+ command: STDERR_THEN_FAIL_CMD.to_string(),
+ output_path: String::new(),
+ timeout: Some(30),
+ default_branch: None,
+ };
+ assert!(
+ capture_diff_snapshot(&config, &test_pr_range()).is_none(),
+ "失敗時は None (呼び出し側で fail-closed に扱う)"
+ );
+}
+
+/// T6 回帰テスト群: diff stage に timeout が無く無限ハングし得た不具合
+/// (ADR-049 の流儀: 1 test = 1 failure mode + good/bad)。
+///
+/// 由来: 2026-07-16 の push パイプライン調査 (コード監査で発見。T5 と同じく
+/// in the wild の発火記録は無く、「他 stage は全て timeout 付き = diff だけが穴」
+/// という非対称として特定された)。
+///
+/// 事故の形: `run_diff_cmd` は `Command::output()` で子プロセスの終了を**無限に**
+/// 待っていた。ADR-045 の並列 workspace 運用で jj の lock 競合が起きると
+/// `pnpm push` は診断も timeout も無いまま停止し、ユーザーは手動 kill するしかない。
+///
+/// 修正の核心は「timeout 付きで待ち、超過時は Err → `DiffResult::Error` = exit 5 で
+/// 中断する (fail-closed / ADR-043)」。あわせて、判定に使う stdout を stderr と
+/// 混ぜない契約 (レビュー対象を汚さない) も本 mod で seal する。
+mod t6_diff_timeout {
+ use super::*;
+ use std::time::{Duration, Instant};
+
+ /// 実行し続けるコマンド (ハングした jj の代役)。timeout が無ければ約 9s 待たされる。
+ /// cmd.exe と POSIX sh で構文が非互換なため OS 別に出し分ける (WP-15)。
+ /// 所要時間を両 OS で揃えないと片側だけ timeout 経路を検証しない穴になる。
+ #[cfg(windows)]
+ const HANGING_COMMAND: &str = "ping 127.0.0.1 -n 10";
+ #[cfg(not(windows))]
+ const HANGING_COMMAND: &str = "sleep 10";
+
+ const SHORT_TIMEOUT_SECS: u64 = 1;
+
+ /// incident 再現 (bad): 応答しないコマンドを **timeout で打ち切る**こと。
+ /// 修正前は `Command::output()` が返るまで待ち続け、本 assert には到達しなかった。
+ #[test]
+ fn hanging_command_times_out_instead_of_waiting_forever() {
+ let started = Instant::now();
+ let result = run_diff_cmd(HANGING_COMMAND, SHORT_TIMEOUT_SECS);
+ let elapsed = started.elapsed();
+
+ let err = result.expect_err("timeout は Err で返ること (無限待ちしない)");
+ assert!(
+ err.contains("タイムアウト"),
+ "timeout と判る診断を返すこと: {:?}",
+ err,
+ );
+ assert!(
+ elapsed < Duration::from_secs(5),
+ "timeout ({}s) 後すぐ制御を返すこと。{:?} 掛かった = コマンドの自然終了を\
+ 待っている (T6 の不具合)",
+ SHORT_TIMEOUT_SECS,
+ elapsed,
+ );
+ }
+
+ /// timeout の診断は原因調査に足りること: 超過秒数と実行コマンドを含む。
+ #[test]
+ fn timeout_error_reports_the_limit_and_the_command() {
+ let err = run_diff_cmd(HANGING_COMMAND, SHORT_TIMEOUT_SECS)
+ .expect_err("timeout は Err で返ること");
+ assert!(
+ err.contains(&format!("{}s", SHORT_TIMEOUT_SECS)) && err.contains(HANGING_COMMAND),
+ "超過秒数と実行コマンドを診断に含めること: {:?}",
+ err,
+ );
+ }
+
+ /// timeout は fail-closed で pipeline を止めること (ADR-043)。
+ /// `DiffResult::Error` は main.rs で exit 5 = 中断になる。空 diff 扱いで
+ /// **レビューを skip したまま push に進んではならない**。
+ #[test]
+ fn timeout_aborts_the_pipeline_and_writes_no_diff_file() {
+ let out_path = std::env::temp_dir().join("test-run-diff-timeout.txt");
+ let _ = std::fs::remove_file(&out_path);
+
+ let config = DiffConfig {
+ command: HANGING_COMMAND.to_string(),
+ output_path: out_path.to_string_lossy().into_owned(),
+ timeout: Some(SHORT_TIMEOUT_SECS),
+ default_branch: None,
+ };
+
+ assert_eq!(
+ run_diff_with(&config, &test_pr_range(), || Ok(String::new())),
+ DiffResult::Error,
+ "timeout は Error (= exit 5 で中断) になること。Empty だとレビューを\
+ skip して push に進んでしまう",
+ );
+ assert!(
+ !out_path.exists(),
+ "timeout 時に diff ファイルを書かないこと (古い/欠けた diff でレビューさせない)",
+ );
+ }
+
+ /// good: timeout 内に終わるコマンドを誤って打ち切らないこと。
+ #[test]
+ fn command_within_the_timeout_succeeds() {
+ let output = run_diff_cmd("echo diff line", 30).expect("即終了するコマンドは Ok");
+ assert!(output.contains("diff line"), "stdout を返すこと: {:?}", output);
+ }
+
+ /// `[diff] timeout` 未指定なら既定値が使われること (既定値の適用漏れ防止)。
+ #[test]
+ fn absent_config_timeout_falls_back_to_the_default() {
+ let config = DiffConfig {
+ command: "echo ok".to_string(),
+ output_path: std::env::temp_dir()
+ .join("test-run-diff-default-timeout.txt")
+ .to_string_lossy()
+ .into_owned(),
+ timeout: None,
+ default_branch: None,
+ };
+ assert_eq!(
+ config.timeout.unwrap_or(DEFAULT_DIFF_TIMEOUT_SECS),
+ DEFAULT_DIFF_TIMEOUT_SECS,
+ );
+ assert_eq!(run_diff_with(&config, &test_pr_range(), || Ok(String::new())), DiffResult::HasContent);
+ let _ = std::fs::remove_file(&config.output_path);
+ }
+
+ /// stderr を stdout に混ぜないこと: stdout は reviewers が読む diff そのものとして
+ /// ファイルに書かれるため、jj の警告 (並列 workspace 時の `Concurrent modification
+ /// detected` 等) が混入するとレビュー対象を汚す。`run_cmd_shell_*` (全 variant が
+ /// stdout/stderr を結合する) に載せ替えるとこのテストが落ちる。
+ #[test]
+ fn stderr_is_not_merged_into_the_diff_output() {
+ let output = run_diff_cmd(STDOUT_AND_STDERR_CMD, 30).expect("exit 0 なら Ok");
+ assert!(output.contains("real diff"), "stdout は残ること: {:?}", output);
+ assert!(
+ !output.contains("Concurrent modification"),
+ "stderr の警告が diff 内容に混入しないこと: {:?}",
+ output,
+ );
+ }
+
+ /// 失敗時は stderr を診断として返すこと (従来契約の維持)。
+ #[test]
+ fn failure_returns_stderr_as_the_diagnostic() {
+ let err = run_diff_cmd(STDERR_THEN_FAIL_CMD, 30).expect_err("exit 1 は Err");
+ assert!(err.contains("boom"), "stderr を診断に返すこと: {:?}", err);
+ }
+}
diff --git a/src/cli-push-runner/src/stages/docs_only_routing.rs b/src/cli-push-runner/src/stages/docs_only_routing.rs
index 4e188c61..e03cd349 100644
--- a/src/cli-push-runner/src/stages/docs_only_routing.rs
+++ b/src/cli-push-runner/src/stages/docs_only_routing.rs
@@ -16,11 +16,16 @@
//!
//! ## 判定範囲は PR 範囲 (`..@`)、単一コミット (`@`) ではない
//!
-//! `[diff]` stage は `jj diff -r @` (単一コミット) を使うが、本 stage は
-//! `pr_size_check` と同じ `..@` (PR 範囲) を使う。quality_gate は
-//! working copy 全体をビルド・テストするので、判定すべきは「push される差分全体が
-//! docs-only か」であり、`@` 単独が docs-only でも祖先コミットが Rust に触れていれば
-//! gate は必要。単一コミット判定は祖先の code 変更を見逃す穴になる。
+//! quality_gate は working copy 全体をビルド・テストするので、判定すべきは
+//! 「push される差分全体が docs-only か」であり、`@` 単独が docs-only でも祖先
+//! コミットが Rust に触れていれば gate は必要。単一コミット判定は祖先の code 変更を
+//! 見逃す穴になる。
+//!
+//! 範囲は `Config::docs_only_pr_range()` から受け取る。かつては本 stage だけが
+//! PR 範囲で `[diff]` stage は `jj diff -r @` (単一コミット) という**非対称**があり、
+//! 祖先コミットが AI レビューを一度も経ずに merge される欠陥が 4 回再発した
+//! (todo 順位 288)。現在は `diff` / `docs_only_routing` / `pr_size_check` の 3 stage が
+//! 同一の解決 (`Config::resolve_base_branch`) を共有し、非対称を構造的に排除している。
//!
//! ## 由来
//!
@@ -57,7 +62,10 @@ pub(crate) enum RoutingDecision {
///
/// ADR-039 § Config opt-in: section 不在 / `enabled != Some(true)` は完全 skip
/// (= 空 Vec を返し従来挙動)。明示的に `enabled = true` のときのみ判定する。
-pub(crate) fn run_docs_only_routing(config: Option<&DocsOnlyRoutingConfig>) -> Vec {
+pub(crate) fn run_docs_only_routing(
+ config: Option<&DocsOnlyRoutingConfig>,
+ pr_range: &str,
+) -> Vec {
let Some(config) = config else {
return Vec::new();
};
@@ -66,9 +74,8 @@ pub(crate) fn run_docs_only_routing(config: Option<&DocsOnlyRoutingConfig>) -> V
}
let override_active =
std::env::var(OVERRIDE_ENV_VAR).ok().as_deref() == Some("1");
- let revset = format!("{}..@", config.effective_default_branch());
- let decision = decide_routing(config, override_active, || run_jj_diff_summary(&revset));
- log_and_map(decision, &revset)
+ let decision = decide_routing(config, override_active, || run_jj_diff_summary(pr_range));
+ log_and_map(decision, pr_range)
}
/// 純関数の判定コア。jj 実行は `fetch_summary` closure で注入し (ADR-021 原則 3)、
@@ -132,9 +139,16 @@ fn log_and_map(decision: RoutingDecision, revset: &str) -> Vec {
/// `jj diff --summary -r ''` を実行し stdout を返す。
///
/// pr_size_check の `run_jj_diff_stat` と同型 (drain_pipe_unlimited + timeout)。
-/// direct args のため `run_cmd_shell_*` (shell 経由) とは signature 非互換で
-/// 共通化しない (ADR-044 層 1)。
-fn run_jj_diff_summary(revset: &str) -> Result {
+///
+/// **direct args 必須** (shell 経由にしないこと): `[diff] command` のような shell 実行に
+/// すると revset のクォートがシェル方言に依存する。cmd.exe は `-r "..@"` の
+/// `"` を除去せず jj に渡すため `Revision '"..@"' doesn't exist` で必ず失敗する
+/// (sh は除去するので Linux だけ通る = 片 OS でのみ壊れる形。2026-07-21 実測)。
+///
+/// `diff` stage の範囲カバレッジ検査もこの関数を使う。両者は「PR 範囲の変更ファイル
+/// 一覧」という**同一の問い**を扱うため、別実装にすると本 PR が排除した非対称
+/// (stage ごとに違う範囲を見る) を再導入することになる。
+pub(super) fn run_jj_diff_summary(revset: &str) -> Result {
let mut child = Command::new("jj")
.args(["diff", "--summary", "-r", revset])
.stdout(Stdio::piped())
@@ -272,11 +286,11 @@ mod tests {
#[test]
fn disabled_config_returns_empty_skip_set() {
let cfg = config(false, None);
- assert!(run_docs_only_routing(Some(&cfg)).is_empty());
+ assert!(run_docs_only_routing(Some(&cfg), "trunk()..@").is_empty());
}
#[test]
fn absent_config_returns_empty_skip_set() {
- assert!(run_docs_only_routing(None).is_empty());
+ assert!(run_docs_only_routing(None, "trunk()..@").is_empty());
}
}
diff --git a/src/cli-push-runner/src/stages/post_takt_regate.rs b/src/cli-push-runner/src/stages/post_takt_regate.rs
index 44375dea..6538300e 100644
--- a/src/cli-push-runner/src/stages/post_takt_regate.rs
+++ b/src/cli-push-runner/src/stages/post_takt_regate.rs
@@ -86,8 +86,8 @@ fn decide_regate(
/// takt 実行後の post-takt diff snapshot を取得する。`[diff]` 未設定時は None
/// (呼び出し側で Indeterminate = fail-closed に倒れる)。
-fn fetch_post_diff(diff_config: Option<&DiffConfig>) -> Option {
- diff_config.and_then(capture_diff_snapshot)
+fn fetch_post_diff(diff_config: Option<&DiffConfig>, pr_range: &str) -> Option {
+ diff_config.and_then(|c| capture_diff_snapshot(c, pr_range))
}
/// 判定結果を実行に写像する。skip 系は `true` (push 続行)、実行系は quality_gate の
@@ -164,8 +164,9 @@ pub(crate) fn run_post_takt_regate(config: &Config, pre_diff: Option<&str>) -> R
.is_some_and(|c| c.is_enabled());
let override_active = std::env::var(OVERRIDE_ENV_VAR).ok().as_deref() == Some("1");
+ let pr_range = config.diff_pr_range();
let decision = decide_regate(enabled, override_active, pre_diff, || {
- fetch_post_diff(config.diff.as_ref())
+ fetch_post_diff(config.diff.as_ref(), &pr_range)
});
let proceed = apply_regate_decision(decision, &config.quality_gate);
@@ -295,8 +296,9 @@ command = "echo push"
command: "echo unchanged".to_string(),
output_path: String::new(),
timeout: Some(30),
+ default_branch: None,
};
- let pre = capture_diff_snapshot(&diff_cfg).expect("pre snapshot 取得");
+ let pre = capture_diff_snapshot(&diff_cfg, "trunk()..@").expect("pre snapshot 取得");
let config = config_with(true, "exit 1", "echo unchanged");
let outcome = run_post_takt_regate(&config, Some(&pre));
diff --git a/src/cli-push-runner/src/stages/pr_size_check.rs b/src/cli-push-runner/src/stages/pr_size_check.rs
index e9a4e0ca..df0926f9 100644
--- a/src/cli-push-runner/src/stages/pr_size_check.rs
+++ b/src/cli-push-runner/src/stages/pr_size_check.rs
@@ -24,7 +24,7 @@
use std::process::{Command, Stdio};
use crate::config::{
- PrSizeCheckConfig, DEFAULT_PR_SIZE_BASE_BRANCH, DEFAULT_PR_SIZE_BLOCK_THRESHOLD,
+ PrSizeCheckConfig, DEFAULT_PR_SIZE_BLOCK_THRESHOLD,
DEFAULT_PR_SIZE_WARNING_THRESHOLD,
};
use crate::log::{log_info, log_stage};
@@ -41,15 +41,14 @@ const OVERRIDE_ENV_VAR: &str = "PR_SIZE_CHECK_OVERRIDE";
///
/// fail-open: jj 不調 (timeout / 起動失敗 / 出力 parse 失敗) 時は warning ログのみで
/// true を返し、push 自体は止めない。
-pub(crate) fn run_pr_size_check(config: Option<&PrSizeCheckConfig>) -> bool {
+pub(crate) fn run_pr_size_check(config: Option<&PrSizeCheckConfig>, pr_range: &str) -> bool {
let enabled = config.and_then(|c| c.enabled).unwrap_or(false);
if !enabled {
return true;
}
- let default_branch = effective_default_branch(config);
let warning = effective_warning_threshold(config);
let block = effective_block_threshold(config);
- let revset = format!("{}..@", default_branch);
+ let revset = pr_range.to_string();
let stat_line = match run_jj_diff_stat(&revset) {
Ok(line) => line,
Err(e) => {
@@ -122,14 +121,6 @@ fn classify_and_log(
true
}
-fn effective_default_branch(config: Option<&PrSizeCheckConfig>) -> String {
- config
- .and_then(|c| c.default_branch.as_ref())
- .map(|s| s.trim().to_string())
- .filter(|s| !s.is_empty())
- .unwrap_or_else(|| DEFAULT_PR_SIZE_BASE_BRANCH.to_string())
-}
-
fn effective_warning_threshold(config: Option<&PrSizeCheckConfig>) -> usize {
config
.and_then(|c| c.warning_threshold)
@@ -344,33 +335,6 @@ mod tests {
assert!(classify_and_log(100, 800, 1500, &r, false));
}
- #[test]
- fn effective_default_branch_uses_default_when_none() {
- assert_eq!(effective_default_branch(None), "master");
- }
-
- #[test]
- fn effective_default_branch_uses_config_when_present() {
- let c = PrSizeCheckConfig {
- enabled: Some(true),
- default_branch: Some("main".to_string()),
- warning_threshold: None,
- block_threshold: None,
- };
- assert_eq!(effective_default_branch(Some(&c)), "main");
- }
-
- #[test]
- fn effective_default_branch_falls_back_when_blank_string() {
- let c = PrSizeCheckConfig {
- enabled: Some(true),
- default_branch: Some(" ".to_string()),
- warning_threshold: None,
- block_threshold: None,
- };
- assert_eq!(effective_default_branch(Some(&c)), "master");
- }
-
#[test]
fn effective_warning_threshold_default() {
assert_eq!(effective_warning_threshold(None), 800);
@@ -405,7 +369,7 @@ mod tests {
#[test]
fn run_pr_size_check_skips_when_section_absent() {
- assert!(run_pr_size_check(None));
+ assert!(run_pr_size_check(None, &fixture_revset("master")));
}
#[test]
@@ -416,7 +380,7 @@ mod tests {
warning_threshold: None,
block_threshold: None,
};
- assert!(run_pr_size_check(Some(&c)));
+ assert!(run_pr_size_check(Some(&c), &fixture_revset("master")));
}
#[test]
@@ -427,7 +391,7 @@ mod tests {
warning_threshold: None,
block_threshold: None,
};
- assert!(run_pr_size_check(Some(&c)));
+ assert!(run_pr_size_check(Some(&c), &fixture_revset("master")));
}
#[test]
diff --git a/templates/push-runner-config.toml b/templates/push-runner-config.toml
index bf439c56..ce4aaea8 100644
--- a/templates/push-runner-config.toml
+++ b/templates/push-runner-config.toml
@@ -47,9 +47,24 @@ commands = ["pnpm build"]
# ]
[diff]
-# jj 環境: "jj diff -r @"
+# `{{PR_RANGE}}` は push-runner が `..@` に展開する。
+# top-level の `default_branch` で base branch を指定する (main 系なら "main")。
+#
+# **exe と config は同時に配布すること** (ADR-051): プレースホルダ対応前の
+# cli-push-runner.exe が本 config を読むと `{{PR_RANGE}}` がそのまま jj に渡り
+# `Failed to parse revset: Syntax error` で exit 5 になる。`pnpm deploy:hooks` は
+# exe と本 template を同時に配るため通常は整合するが、片方だけ手で置き換えないこと。
+#
+# **revset を直書きしないこと**: `-r @` のように PR より狭い範囲を書くと祖先
+# コミットが AI レビュー未経由で merge される (upstream で 4 回再発した欠陥)。
+# 生成 diff が PR 範囲を網羅していない場合は exit 5 で中断する (fail-closed)。
+#
+# `--git` (unified diff) 必須: 範囲カバレッジ検査がヘッダを読むほか、jj 既定形式は
+# 色を落とすと `+`/`-` が消え LLM レビュアーが削除を追加と誤読する。
+#
+# jj 環境: "jj diff --git -r {{PR_RANGE}}"
# git 環境: "git diff origin/HEAD...HEAD"
-command = "jj diff -r @"
+command = "jj diff --git -r {{PR_RANGE}}"
output_path = ".takt/review-diff.txt"
# timeout: 未指定時は 60s。超過時は diff 取得失敗 = exit 5 で中断 (fail-closed)。
# 大 diff / 低速環境で誤 timeout する場合のみ延長する (例: timeout = 180)。