diff --git a/.claude/hooks-config.toml b/.claude/hooks-config.toml index 9d0ea892..e9be5589 100644 --- a/.claude/hooks-config.toml +++ b/.claude/hooks-config.toml @@ -119,6 +119,9 @@ steps = [ # ─── Stop: 品質ゲート ─── [stop_quality] +# 順位 175 (PR #185 T1-#1): cargo clippy 追加で Rust lint structural gap を補填。 +# incremental cache が効く限り 5-15s で完了するため step_timeout=60 内に収まる。 +# cold cache の初回のみ超過リスクあり (workspace 全 crate compile)。 step_timeout = 60 [[stop_quality.steps]] @@ -129,6 +132,10 @@ cmd = "pnpm lint" name = "lint:md" cmd = "pnpm lint:md" +[[stop_quality.steps]] +name = "lint:rust" +cmd = "cargo clippy --workspace -- -D warnings" + [[stop_quality.steps]] name = "test" cmd = "pnpm test" diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 3d642e31..b4fac504 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -70,7 +70,6 @@ | 152 | 🔧 Tier 2 | **todo entry 削除時の事前 land 確認手順 — 順位 136 hook 拡張 or 独立 follow-up (PR #173 T2-1 採用、2026-05-26)** | todo9.md | XS-S | 順位 136 (working copy staleness + 既実装 grep) と同型機械強制、lifecycle 補完 = 順位 136 (add/edit 時) + 本タスク (delete 時)。PreToolUse hook で `docs/todo*.md` 削除時に対応 land commit を `jj log` で grep 検証、land 確認なら allow + 証跡出力、未確認なら warning (block しない)。順位 136 hook 統合 (~+15 行) or 独立 (~40 行) のいずれか、ADR-042 § Decision matrix 適用 (mechanizable + FP 低 + Adoption Risk None) | | 153 | 🔧 Tier 2 | **`review-harness-whole` facet 追加 — 観点 ① 独立 facet 化 (ADR-031 weekly-review 拡張、2026-05-26 ユーザー合意) ★ 週次拡張** | todo9.md | S | ADR-031 本採用後 (2026-06-01) の Phase B+1 拡張、extract 不要と判明したら close、順位 146-151 Bundle 既存ルール仕組み化の継続的発見源、architecture-whole から ① 観点を extract して context 圧迫回避 | | 154 | 🔧 Tier 2 | **`review-todo-whole` facet + aggregate 前 file size pre-step — 観点 ⑤ ⑦ 拡張 (ADR-031 weekly-review 拡張、2026-05-26 ユーザー合意) ★ 週次拡張** | todo9.md | M | 順位 136 land + ADR-031 本採用 (2026-06-01) 後着手、cli-docs-lint (preamble) / 順位 147 (file length) と scope 整理必要 (CI 即時 vs 週次 batch)、ADR-031 3 層分離原則で file size は LLM 不要の Rust pre-step に分離 | -| 155 | 🚀 Tier 1 | **cli-pr-monitor fix chain 末尾に空 commit 検査 + `jj abandon` step 追加 (PR #174 T1-#1 採用)** | todo9.md | S | なし (PR #174 で `kqvluqyv` 空 commit が PR diff 汚染した実証ベース、`master..@` 範囲を `jj log` で sweep して機械強制、既存 `CleanupEmptyFixCommit` action の補完層) | | 157 | 🔧 Tier 2 | **Bundle 1 dogfood checklist 実行 — `__test.ps1` block + override env 確認 (PR #174 T2-#2 採用、ADR-039 bounded lifetime data point #1)** | todo9.md | XS | なし (PR #174 PR body の未消化 dogfood、Bundle 2 PR merge 前の前提条件として消化、結果は Bundle 2 PR body に記録) | | 160 | 💎 Tier 3 | **`docs-governance.md` に「ADR multi-variant pattern section 追加時の checklist」codify (PR #176 T3-#1 採用)** | todo9.md | XS | なし (PR #175 Minor + PR #176 Nitpick の 2 連続観測 = Frequency Medium で採用条件成立、ADR 拡張時の variant 網羅性 + 擬似コード vs 実コード齟齬を reviewer / Claude 視点で防止する checklist、global file `~/.claude/rules/common/docs-governance.md` 編集のため本リポジトリ外で実施、`feedback_global_config_backup` 適用) | | 161 | 🔧 Tier 2 | **Subprocess timeout+kill lifecycle 検証テスト追加 (PR #177 T2-#1 採用)** | todo9.md | M | なし (PR #177 Major #2 「jj kill on timeout 漏れ」fix の回帰テスト、`Child::is_finished` で 2 hook の `run_jj_with_timeout` lifecycle 検証、Severity High + Frequency Medium、ADR-024 shared lib 統合候補との関係明示) | @@ -81,7 +80,6 @@ | 171 | 💎 Tier 3 | **`docs-governance.md` に「Operational reference vs Pointer reference」区別 section を追加 (PR #183 T3-#2 採用) ★ Bundle DG-RULES** | todo9.md | S | 順位 172 と同 PR 推奨、PR #183 A01 修正で実適用した判定ロジック (operational = workflow 動作記述 = 保持可 / pointer = section 名・順位番号参照 = 置換必要) を `~/.claude/rules/common/docs-governance.md` § Cross-File Reference Lifecycle に新 sub-section として codify、ADR-031 lines 79-302 中 line 270 のみが真の pointer だった実例を inline cite、派生プロジェクトへ自動波及、`feedback_global_config_backup` 適用必須 | | 172 | 💎 Tier 3 | **CR ephemeral artifact Nitpick の統一 skip 基準を memory に codify (PR #183 T3-#3 採用) ★ Bundle DG-RULES** | todo9.md | XS | 順位 171 と同 PR 推奨、CR が `docs/todo*.md` 系 ephemeral artifact 内の行番号参照を Nitpick 指摘した場合は skip 推奨という判断基準を新 memory `feedback_coderabbit_ephemeral_nitpick.md` に codify、既存 memory `feedback_coderabbit_no_actionable_merge_signal` の補完、本リポジトリ専用 (派生プロジェクトには波及しない)、`feedback_global_config_backup` 適用推奨 | | 173 | 🔧 Tier 2 | **`combine_output` 5 crate 重複を `lib-runner-utils` (or 既存 lib-*) に extract (PR #182 dry-run S01 採用)** | todo9.md | S-M | なし (`src/cli-pr-monitor/src/runner.rs:80-89` の `combine_output` 8 行関数が `#[allow(dead_code)]` 付与で生産未使用、同関数が 4 他 crate (cli-push-runner, cli-push-pipeline, cli-merge-pipeline, hooks-post-tool-linter) にも複製 = 5 crate 横断 systemic duplication、ADR-026 Cargo workspace + ADR-012 lib-* naming で解決、Phase B dogfood の最初の実体ベース finding (A01 と並ぶ)、A01 は PR #183 で fix 済) | -| 175 | 🚀 Tier 1 | **`cargo clippy` を stop_quality ステップに追加 — Rust lint structural gap 補填 (PR #185 T1-#1 採用)** | todo10.md | S | なし (`.claude/hooks-config.toml` の stop_quality / quality_gate に `cargo clippy --workspace -- -D warnings` step を追加、現状 stop_quality は `pnpm lint` / `pnpm lint:md` / `pnpm test` / `pnpm build` のみで Rust の cargo clippy が完全欠落、PR #185 開発時の `doc_lazy_continuation` clippy error も手動 cargo clippy まで未検出だった structural gap、option A = 直接 cargo clippy / option B = `pnpm lint:rust` 新設の 2 アプローチ、incremental 後は高速で実行コスト許容範囲) | | 176 | 🔧 Tier 2 | **check-ci-coderabbit format extraction 関数への variant fixture 追加 (PR #185 T2-#4 採用)** | todo10.md | M | なし (順位 167-169 Bundle CR-RL の follow-up、bold-wrapper variant (`**More reviews will be available**`) / 短形態 (secs のみ) / 複数 separator / wait time なし graceful failure の 4 fixture 追加、PR #182 + #185 の 2 PR 連続観測で CR format 多様性 systemic、`extract_old_format_wait_time` / `extract_new_format_wait_time` の coverage gap 補填、regex 拡張 vs fixture 先行の 2 アプローチを着手時判断、analyzer rationale の「Edit 集中 = test gap signal」は incidental で採用根拠から除外、true 採用根拠は format 多様性 + 防御的 variant) | | 177 | 🚀 Tier 1 | **PostToolUse hook — Edit / Write したファイルのサイズ閾値超過を検出してファイル分割を促す (2026-05-29 ユーザー追加要望)** | todo10.md | S-M | なし (本セッション PR #181-#185 chain で `docs/todo9.md` が 50KB 超 + 1168 行に到達し `docs/todo10.md` split した実体観測ベース、PR #133 (todo.md → todo2.md) / PR #172 (todo8.md → todo9.md) / 本 PR (todo9.md → todo10.md) の 3 PR 観測で systemic、PostToolUse Edit/Write 直後にサイズチェックを mechanical 強制、`.claude/hooks-config.toml` の `[post_tool_use.file_size_check]` で `enabled = false` default OFF (ADR-039 opt-in) + `threshold_bytes` (default 51200 = 50KB) + `paths` glob + `touch_trigger` ratchet を設定可能、配置先は option A = 新 binary `hooks-post-tool-file-size-check` / option B = 既存 `hooks-post-tool-linter` 統合の 2 案を着手時判断、ADR-007 custom-linter layer boundary に位置付け追記) | | 178 | 🔧 Tier 2 | **`state.rs` の behavioral invariant test を ADR-041 pattern で追加 (週次レビュー 2026-05-30 S02 採用)** | todo10.md | S | なし (Phase D dogfood で発見、`src/cli-pr-monitor/src/state.rs:226-510` の test が JSON round-trip のみ、`rate_limit=Some` 時 CI 更新 skip 等の behavioral invariant 未検証、ADR-041 sentinel 事前投入 + mutation 不在 assert pattern で 3-5 test 追加、memory `feedback_test_dry_antipattern` 適用、Effort S で high value catches state regression) | diff --git a/docs/todo10.md b/docs/todo10.md index 3fee6789..f5601b98 100644 --- a/docs/todo10.md +++ b/docs/todo10.md @@ -10,47 +10,6 @@ ## 現在進行中 -### `cargo clippy` を stop_quality ステップに追加 — Rust lint structural gap 補填 (PR #185 T1-#1 採用) - -> **動機**: 本リポジトリの quality_gate / stop_quality は `pnpm lint` (oxlint / JS) / `pnpm lint:md` (markdownlint) / `pnpm test` (vitest) / `pnpm build` (tsc) の 4 step 構成だが **Rust の cargo clippy が完全に欠落**している。本セッション PR #185 (Bundle CR-RL) 開発時に `doc_lazy_continuation` clippy error が手動 `cargo clippy --release -- -D warnings` 実行まで検出されず、もし pre-push-review に押し込んでいたら reviewer が detect → fix iteration が増えていた structural gap として顕在化した。stop_quality に cargo clippy が存在しない以上、同種 Rust lint エラーは同じ window を通り続ける systemic 問題。 -> -> **本タスクの位置づけ**: PR #185 post-merge-feedback Tier 1 #1 採用 (Severity Medium / Frequency Low / Effort S / Adoption Risk = `実行速度増加` だが incremental 後は高速、2026-05-29 ユーザー承認)。stop_quality の Rust lint 空白を埋める structural fix。 -> -> **参照**: `.claude/feedback-reports/185.md` Tier 1 #1、`.claude/hooks-config.toml` の stop_quality steps 配列 (拡張対象)、PR #185 開発時の `doc_lazy_continuation` 手動 clippy 発見 (commit chain: 順位 167 実装 → clippy → docstring 修正の循環) -> -> **実行優先度**: 🚀 **Tier 1** — Effort S。stop_quality config の 1 step 追加で structural gap が塞がれる。 - -#### 設計決定 (案) - -- **追加 step**: `cargo clippy --workspace -- -D warnings` - - `--workspace`: 全 crate を対象 (cli-* + hooks-* + lib-* 横断、本リポジトリの Cargo workspace 構成に整合、ADR-026 準拠) - - `-- -D warnings`: warning を error 化 (`-D` = deny。`feedback_pnpm_create_pr_body` 同様の strict mode) -- **配置場所**: `.claude/hooks-config.toml` `[stop_quality]` または `[quality_gate]` の steps 配列に追加 (実装時に既存 step 並びと整合する位置に挿入) -- **代替案**: 直接 cargo clippy を steps に書くか、`pnpm lint:rust` スクリプトを新設して参照するか。後者は package.json の script naming 一貫性 (`lint` / `lint:md` / 新 `lint:rust`) で他 pnpm script と並びが綺麗 -- **incremental build cost**: cargo は incremental 後は高速。初回 clean state では Rust 全 build を伴うため数分かかる可能性あり、その場合は `cargo clippy --no-deps -- -D warnings` で dependency rebuild を回避する option を検討 - -#### 作業計画 - -- [ ] `.claude/hooks-config.toml` の `[stop_quality]` (または `[quality_gate]`) steps を Read で確認、既存 step 並びを把握 -- [ ] (option A) 直接 `cargo clippy --workspace -- -D warnings` を steps 配列に追加 -- [ ] (option B) `package.json` に `lint:rust` script を新設 (`"lint:rust": "cargo clippy --workspace -- -D warnings"`) し、steps では `pnpm lint:rust` を参照 -- [ ] dogfood: 本タスク実装 commit 自身を `pnpm push` で通して新 clippy step が pre-push pipeline で発火することを確認 -- [ ] 既存 cargo clippy エラーが存在しないか全 crate で確認 (`cargo clippy --workspace -- -D warnings` を手動実行、warning ゼロを baseline 化) -- [ ] 本エントリ削除 + todo-summary.md 行削除 - -#### 完了基準 - -- stop_quality / quality_gate に Rust lint step が追加され、`doc_lazy_continuation` 等の clippy error が pre-push pipeline で自動検出される -- 全 crate の cargo clippy が clean state を baseline として確立 -- 本タスク実装 commit 自身が新 step を通過する dogfood で機能を確認 -- `pnpm test` / `pnpm build` 等の既存 step と並列実行可能性を維持 (実行速度劣化なし or 許容範囲内) - -#### 詰まっている箇所 - -なし。Effort S、config 1 行追加で structural gap が塞がれる。実装時の判断点は option A (直接 cargo clippy) vs option B (`pnpm lint:rust` 新設) のみ。 - ---- - ### check-ci-coderabbit format extraction 関数への variant fixture 追加 (PR #185 T2-#4 採用) > **動機**: PR #185 (Bundle CR-RL) で `extract_old_format_wait_time` / `extract_new_format_wait_time` の 2 helper 関数に分離し 3 新規 fixture (full / minutes-only / 旧新混在) を追加したが、analyzer (post-merge-feedback) は **bold-wrapper variant** (例: `**More reviews will be available in N minutes and S seconds**`) や **その他の組合せ variant** の coverage gap を指摘。PR #182 (30+ 分 polling 浪費の実観測) + PR #185 (format 多様性対応) の 2 PR 連続観測で、CR の format は引き続き variants を生む可能性が高く、防御的 fixture coverage 追加が systemic 価値あり。 diff --git a/docs/todo9.md b/docs/todo9.md index a42db715..49e6f245 100644 --- a/docs/todo9.md +++ b/docs/todo9.md @@ -447,49 +447,6 @@ --- -### cli-pr-monitor fix chain 末尾に空 commit 検査 + `jj abandon` step を追加 (PR #174 T1-#1 採用) - -> **動機**: PR #174 で post-pr-monitor の `CleanupEmptyFixCommit` action 後に、別の空 commit (`kqvluqyv`) が祖父コミット位置に残存し、後続の Bundle 1 Minor fix push 時に PR diff を汚染する事象を観測。cleanup ロジックが「fix chain で直近 create された空 commit」のみ対象にしており、過去の空 commit を見逃す構造的欠陥が明らかになった。手動 `jj abandon` で 1 件解消したが、機械強制すべき。 -> -> **本タスクの位置づけ**: PR #174 post-merge-feedback Tier 1 #1 採用 (Severity Medium / Frequency Low / Effort S / Adoption Risk None)。cli-pr-monitor の cleanup phase に「`jj log --no-graph` で空 description の commit を検出 → 全て abandon」step を追加し、空 commit による PR diff 汚染を構造的に予防する。 -> -> **参照**: `.claude/feedback-reports/174.md` Tier 1 #1、PR #174 で観測した `kqvluqyv` 事例 (Bundle 1 fix loop 中に手動 abandon)、`src/cli-pr-monitor/src/` -> -> **実行優先度**: 🚀 **Tier 1** — Effort S。cli-pr-monitor fix chain への追加 step 1 件、機械強制で重複事故を防止。 - -#### 設計決定 (案) - -- 配置: `src/cli-pr-monitor/src/` の fix chain cleanup phase 末尾 (既存 `CleanupEmptyFixCommit` の後) -- 動作: - 1. `jj log -r 'master..@' --no-graph -T 'change_id ++ "\u{1f}" ++ if(empty, "EMPTY", "CONTENT") ++ "\n"'` で PR 範囲 commit を列挙 (`empty` は jj template の commit 自体が空か判定する keyword) - 2. 各行を `\u{1f}` (Unit Separator) で分割し、2 列目が `EMPTY` の commit を filter - 3. 該当 commit を `jj abandon ` で順次 abandon - - 注意: `description.first_line()` は description の 1 行目を返すため「全 description 空」と「複数行 description で 1 行目だけ空」を区別できない。実装では jj template の `empty` keyword (= commit が file change を含まないか) を直接使うか、`if(description, "DESCRIBED", "UNDESCRIBED")` で description 有無を判定する設計に固定する -- scope 限定: `master..@` 範囲のみ (= PR に含まれる範囲)。master 以下は対象外 -- 既存 `CleanupEmptyFixCommit` との関係: 既存は直近 fix commit のみ対象、本 step は全範囲 sweep の補完層 -- fail-open: jj log / abandon の失敗時は warning ログのみで cleanup を継続 (push を block しない) - -#### 作業計画 - -- [ ] cli-pr-monitor の cleanup phase 実装箇所を特定 (`CleanupEmptyFixCommit` action の呼び出し元) -- [ ] 空 commit 列挙ロジック (jj log + description filter) を追加 -- [ ] abandon ループ + error handling 実装 -- [ ] test 拡充: 空 commit 0 件 / 1 件 / 複数件 / 非空 commit のみ / mixed -- [ ] `pnpm build:cli-pr-monitor` で release 生成 + dogfood (次の PR で同様の状況を作って動作確認) -- [ ] 本エントリ削除 + todo-summary.md 行削除 - -#### 完了基準 - -- post-pr-monitor の cleanup phase 完了時に PR 範囲内の空 commit が全て abandon される -- 既存 `CleanupEmptyFixCommit` action と non-regression -- dogfood で空 commit 自動 cleanup が動作確認される - -#### 詰まっている箇所 - -なし。Effort S、既存 cleanup phase への追加 step で副作用最小。 - ---- - ### Bundle 1 dogfood checklist 実行 — `__test.ps1` block + override env 確認 (PR #174 T2-#2 採用、ADR-039 bounded lifetime data point #1) > **動機**: PR #174 で実装した `scratch_file_warning` stage は ADR-039 § 3 Bounded lifetime 準拠で「3-5 PR の dogfood 後に default-ON 昇格 or 却下を判定」する設計。PR #174 の PR body に未消化の dogfood checklist が残っており (`__test.ps1` を意図的に作って push し block 動作確認 / override env でバイパス確認)、これが ADR-039 bounded lifetime の初回データポイント。次の PR (Bundle 2 等) merge 前の前提条件として消化が必要。 diff --git a/push-runner-config.toml b/push-runner-config.toml index fecc34d7..1ac0027f 100644 --- a/push-runner-config.toml +++ b/push-runner-config.toml @@ -60,14 +60,18 @@ commands = ["pnpm test", "pnpm test:e2e"] name = "build" commands = ["pnpm build"] -# Rust test group: push pipeline でのみ実行。 +# Rust lint + test group: push pipeline でのみ実行。 # PostToolUse / Stop hook では実行せず、イテレーション速度を保護する。 # ADR-026 により Cargo workspace 化したため、workspace 全体を 1 コマンドで回せる。 -# 1 つ目: unit tests (並列実行可、高速) -# 2 つ目: 統合テスト (#[ignore] 付き、cwd 依存のため --test-threads=1 必須) +# 順位 175 (PR #185 T1-#1): cargo clippy を先頭に追加。Rust lint structural gap を補填。 +# cargo test と同 group に置くことで target dir のロック衝突 (並列 cargo invocation) を回避。 +# 1 つ目: clippy (lint、cargo test より高速で fail fast) +# 2 つ目: unit tests (並列実行可、高速) +# 3 つ目: 統合テスト (#[ignore] 付き、cwd 依存のため --test-threads=1 必須) [[quality_gate.groups]] -name = "rust-test" +name = "rust-lint-test" commands = [ + "cargo clippy --workspace -- -D warnings", "cargo test", "cargo test -- --ignored --test-threads=1", ] diff --git a/src/cli-pr-monitor/src/config.rs b/src/cli-pr-monitor/src/config.rs index 15820c06..288c98b5 100644 --- a/src/cli-pr-monitor/src/config.rs +++ b/src/cli-pr-monitor/src/config.rs @@ -73,6 +73,8 @@ pub(crate) struct FixConfig { /// push コマンド (jj git push / git push) #[serde(default = "default_push_command")] pub(crate) push_command: String, + #[serde(default)] + pub(crate) sweep: SweepConfig, } fn default_auto_push_severity() -> String { @@ -87,6 +89,32 @@ impl Default for FixConfig { Self { auto_push_severity: default_auto_push_severity(), push_command: default_push_command(), + sweep: SweepConfig::default(), + } + } +} + +/// PR 範囲内の `fix(review):` 空 commit を自動 abandon する sweep 設定。 +/// +/// デフォルトは `enabled = false` (opt-in)。有効化後は `default_branch..@` 範囲の +/// `fix(review):` 空 commit を `execute_repush_flow` 末尾で自動 abandon する。 +#[derive(Deserialize, Clone)] +pub(crate) struct SweepConfig { + #[serde(default)] + pub(crate) enabled: bool, + #[serde(default = "default_sweep_branch")] + pub(crate) default_branch: String, +} + +fn default_sweep_branch() -> String { + "master".into() +} + +impl Default for SweepConfig { + fn default() -> Self { + Self { + enabled: false, + default_branch: default_sweep_branch(), } } } @@ -398,6 +426,8 @@ task = "t" let config: Config = toml::from_str(toml_str).unwrap(); assert_eq!(config.fix.auto_push_severity, "critical"); assert_eq!(config.fix.push_command, "jj git push"); + assert!(!config.fix.sweep.enabled, "sweep はデフォルト無効 (opt-in)"); + assert_eq!(config.fix.sweep.default_branch, "master"); } #[test] @@ -414,6 +444,28 @@ push_command = "git push" assert_eq!(config.fix.push_command, "git push"); } + #[test] + fn config_fix_sweep_defaults() { + let toml_str = "[monitor]\n"; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(!config.fix.sweep.enabled, "sweep はデフォルト無効 (opt-in)"); + assert_eq!(config.fix.sweep.default_branch, "master"); + } + + #[test] + fn config_fix_sweep_custom() { + let toml_str = r#" +[monitor] + +[fix.sweep] +enabled = true +default_branch = "main" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(config.fix.sweep.enabled); + assert_eq!(config.fix.sweep.default_branch, "main"); + } + #[test] fn config_rate_limit_defaults() { let toml_str = "[monitor]\n"; diff --git a/src/cli-pr-monitor/src/fix_commit.rs b/src/cli-pr-monitor/src/fix_commit.rs index 91909085..c277aa9c 100644 --- a/src/cli-pr-monitor/src/fix_commit.rs +++ b/src/cli-pr-monitor/src/fix_commit.rs @@ -198,6 +198,79 @@ fn reparent_at_to_pr_tip(context: &str) { } } +/// `default_branch..@` 範囲の `fix(review):` 空 commit を sweep して全て abandon する (順位 155、PR #174 T1-#1)。 +/// +/// 既存 `try_abandon_empty_fix_commit` が tracked な単一 fix commit (= 直近 `create_fix_commit` +/// の戻り値) のみを対象とするのに対し、本関数は PR 範囲全体を sweep して +/// **untracked な空 commit** を網羅的に拾う。PR #174 で観測した `kqvluqyv` 事例 +/// (過去 fix loop で取りこぼされた granduncle 位置の空 commit が後続 push で PR diff 汚染) の +/// 構造的予防層。 +/// +/// 実装: jj revset `empty() & description("fix(review):") & (default_branch..@)` で範囲内の +/// fix(review): 空 commit を 1 step で列挙し、change_id ベースで順次 `jj abandon` する。 +/// change_id は jj の永続識別子のため、複数 abandon で graph が rebase されても残りの id 参照は invariant。 +/// description フィルタにより `create_fix_commit` 由来のコミットのみを対象とし、他の空コミットは除外する。 +/// +/// fail-open: jj log / abandon の失敗時は warn ログのみで cleanup を継続する +/// (push を block すると fix loop 全体が止まるため、ローカル副作用は次回再走で吸収する方針)。 +pub(crate) fn sweep_empty_commits_in_pr_range(default_branch: &str) { + let revset = format!( + "empty() & description(substring:\"fix(review):\") & ({}..@)", + default_branch + ); + let (ok, out) = run_cmd_direct( + "jj", + &[ + "log", + "-r", + &revset, + "--no-graph", + "-T", + "change_id ++ \"\\n\"", + ], + &[], + JJ_CMD_TIMEOUT_SECS, + ); + if !ok { + log_info(&format!( + "[warn] sweep_empty_commits: jj log 失敗 (sweep skip): {}", + out.trim() + )); + return; + } + + let change_ids = parse_empty_change_ids(&out); + if change_ids.is_empty() { + return; + } + log_info(&format!( + "[action] sweep_empty_commits: {}..@ 範囲に fix(review): 空 commit {} 件を検出 → abandon", + default_branch, + change_ids.len() + )); + for cid in &change_ids { + let (ok, out) = run_cmd_direct("jj", &["abandon", cid], &[], JJ_CMD_TIMEOUT_SECS); + if !ok { + log_info(&format!( + "[warn] sweep_empty_commits: jj abandon {} 失敗 (継続): {}", + cid, + out.trim() + )); + continue; + } + log_info(&format!("[action] sweep_empty_commits: abandoned {}", cid)); + } +} + +/// `jj log` 出力 (1 行 1 change_id) を parse する純関数。空行と前後空白を除去する。 +fn parse_empty_change_ids(log_output: &str) -> Vec { + log_output + .lines() + .map(|l| l.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() +} + /// `@-` (親 commit) の id が `expected` と一致するか判定する。 /// 取得失敗時は `false` (= 不一致扱いで reparent を試行) を返す。 fn parent_commit_id_is(expected: &str) -> bool { @@ -364,6 +437,188 @@ mod tests { ); } + #[test] + fn parse_empty_change_ids_handles_empty_input() { + assert!(parse_empty_change_ids("").is_empty()); + } + + #[test] + fn parse_empty_change_ids_extracts_single_id() { + let out = "abc123def\n"; + assert_eq!(parse_empty_change_ids(out), vec!["abc123def".to_string()]); + } + + #[test] + fn parse_empty_change_ids_extracts_multiple_ids() { + let out = "abc\ndef\nghi\n"; + assert_eq!( + parse_empty_change_ids(out), + vec!["abc".to_string(), "def".to_string(), "ghi".to_string()] + ); + } + + #[test] + fn parse_empty_change_ids_skips_blank_lines_and_whitespace() { + let out = " abc \n\n \ndef\n\n"; + assert_eq!( + parse_empty_change_ids(out), + vec!["abc".to_string(), "def".to_string()] + ); + } + + fn setup_jj_repo_with_master_at_base(base_msg: &str) -> tempfile::TempDir { + use std::process::Command as StdCommand; + let temp = tempfile::tempdir().expect("tempdir 作成失敗"); + let repo_dir = temp.path(); + assert!(StdCommand::new("jj") + .args(["git", "init"]) + .current_dir(repo_dir) + .status() + .expect("jj git init") + .success()); + std::fs::write(repo_dir.join("base.txt"), "content\n").expect("write base"); + assert!(StdCommand::new("jj") + .args(["describe", "-m", base_msg]) + .current_dir(repo_dir) + .status() + .expect("describe base") + .success()); + assert!(StdCommand::new("jj") + .args(["bookmark", "create", "master", "-r", "@"]) + .current_dir(repo_dir) + .status() + .expect("bookmark master") + .success()); + temp + } + + struct CwdGuard { + original: std::path::PathBuf, + } + impl Drop for CwdGuard { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original); + } + } + + fn enter_repo(repo_dir: &std::path::Path) -> CwdGuard { + let original = std::env::current_dir().expect("cwd"); + std::env::set_current_dir(repo_dir).expect("cd"); + CwdGuard { original } + } + + fn assert_descriptions_absent_in_pr_range( + repo_dir: &std::path::Path, + descriptions: &[&str], + ) { + let out = std::process::Command::new("jj") + .args([ + "log", + "-r", + "master..@", + "--no-graph", + "-T", + "description ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log master..@"); + let log_str = String::from_utf8_lossy(&out.stdout); + for d in descriptions { + assert!( + !log_str.contains(d), + "{:?} が abandon されている前提だが残存: {:?}", + d, + log_str + ); + } + } + + fn count_empty_in_pr_range(repo_dir: &std::path::Path) -> usize { + let out = std::process::Command::new("jj") + .args([ + "log", + "-r", + "empty() & (master..@)", + "--no-graph", + "-T", + "change_id ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + String::from_utf8_lossy(&out.stdout) + .lines() + .filter(|l| !l.trim().is_empty()) + .count() + } + + /// 統合: `master..@` 範囲に空 commit が無いとき sweep は no-op (非空 commit を保持)。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_empty_commits_no_op_when_no_empty_in_range() { + let temp = setup_jj_repo_with_master_at_base("feat: real change"); + let repo_dir = temp.path(); + let _guard = enter_repo(repo_dir); + + sweep_empty_commits_in_pr_range("master"); + + let log_out = std::process::Command::new("jj") + .args(["log", "-r", "::@", "--no-graph", "-T", "description"]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + let log_str = String::from_utf8_lossy(&log_out.stdout); + assert!( + log_str.contains("feat: real change"), + "non-empty commit が保持されていること: {:?}", + log_str + ); + } + + /// 統合: `master..@` 範囲の複数空 commit を sweep が全て abandon する。 + /// PR #174 `kqvluqyv` 事例の最小再現。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_empty_commits_abandons_multiple_in_range() { + use std::process::Command as StdCommand; + let temp = setup_jj_repo_with_master_at_base("feat: base"); + let repo_dir = temp.path(); + + for label in &["fix(review): empty 1", "fix(review): empty 2"] { + assert!(StdCommand::new("jj") + .args(["new", "-m", label]) + .current_dir(repo_dir) + .status() + .expect("jj new") + .success()); + } + assert!( + count_empty_in_pr_range(repo_dir) >= 2, + "前提: sweep 前に空 commit が 2 件以上" + ); + + let _guard = enter_repo(repo_dir); + sweep_empty_commits_in_pr_range("master"); + + assert_descriptions_absent_in_pr_range( + repo_dir, + &["fix(review): empty 1", "fix(review): empty 2"], + ); + + let master_out = StdCommand::new("jj") + .args(["log", "-r", "master", "--no-graph", "-T", "description"]) + .current_dir(repo_dir) + .output() + .expect("jj log master"); + let master_desc = String::from_utf8_lossy(&master_out.stdout); + assert!( + master_desc.contains("feat: base"), + "master commit (非空) は abandon されない: {:?}", + master_desc + ); + } + #[test] fn fix_commit_state_is_created_truth_table() { assert!(!FixCommitState::None.is_created()); diff --git a/src/cli-pr-monitor/src/stages/repush.rs b/src/cli-pr-monitor/src/stages/repush.rs index d917b144..924d3e6c 100644 --- a/src/cli-pr-monitor/src/stages/repush.rs +++ b/src/cli-pr-monitor/src/stages/repush.rs @@ -111,6 +111,36 @@ pub(crate) fn should_auto_push(setting: &str) -> bool { } } +fn execute_repush_action( + fix_config: &crate::config::FixConfig, + pr_label: &str, + action: RepushAction, +) { + match action { + RepushAction::AutoPush => run_auto_push(fix_config, pr_label), + RepushAction::UserConfirmWithSeparatedFix { commit_id } => { + log_info(&format!( + "[action] auto_push スキップ: ユーザー確認待ち (fix commit 分離済み: {})", + commit_id + )); + log_info("[action] 確認後に pnpm push するか、jj describe で再構成してください"); + } + RepushAction::UserConfirmNoSeparation => { + log_info("[action] auto_push スキップ: ユーザー確認待ち"); + log_info("[action] 確認後に pnpm push を実行してください"); + } + RepushAction::CleanupEmptyFixCommit { commit_id } => { + crate::fix_commit::try_abandon_empty_fix_commit("fix_state=Created:", Some(&commit_id)); + } + RepushAction::SkipNoChange => { + log_info("[action] re-push スキップ: takt は実質変更を加えていない"); + } + RepushAction::FailSafeCaptureFailed => { + log_info("[action] re-push スキップ: commit id 取得失敗 (fail-safe)"); + } + } +} + /// takt 実行後の re-push フロー。 /// /// 1. post_takt_cid を捕捉し、pre / post を比較して `decide_repush` で判定 @@ -142,28 +172,10 @@ pub(crate) fn execute_repush_flow( let action = decide_repush_action(&decision, fix_state, allow_auto); log_info(&format!("[decision] action: {:?}", action)); - match action { - RepushAction::AutoPush => run_auto_push(fix_config, pr_label), - RepushAction::UserConfirmWithSeparatedFix { commit_id } => { - log_info(&format!( - "[action] auto_push スキップ: ユーザー確認待ち (fix commit 分離済み: {})", - commit_id - )); - log_info("[action] 確認後に pnpm push するか、jj describe で再構成してください"); - } - RepushAction::UserConfirmNoSeparation => { - log_info("[action] auto_push スキップ: ユーザー確認待ち"); - log_info("[action] 確認後に pnpm push を実行してください"); - } - RepushAction::CleanupEmptyFixCommit { commit_id } => { - crate::fix_commit::try_abandon_empty_fix_commit("fix_state=Created:", Some(&commit_id)); - } - RepushAction::SkipNoChange => { - log_info("[action] re-push スキップ: takt は実質変更を加えていない"); - } - RepushAction::FailSafeCaptureFailed => { - log_info("[action] re-push スキップ: commit id 取得失敗 (fail-safe)"); - } + execute_repush_action(fix_config, pr_label, action); + + if fix_config.sweep.enabled { + crate::fix_commit::sweep_empty_commits_in_pr_range(&fix_config.sweep.default_branch); } } diff --git a/src/cli-push-runner/src/stages/scratch_file_warning.rs b/src/cli-push-runner/src/stages/scratch_file_warning.rs index eb721c96..b7fbc5a8 100644 --- a/src/cli-push-runner/src/stages/scratch_file_warning.rs +++ b/src/cli-push-runner/src/stages/scratch_file_warning.rs @@ -27,6 +27,7 @@ //! 列挙して basename match で検査 (= push 直前の最終防衛層) //! - ADR-007 § custom_lint_rule = PostToolUse hook で AI が edit/write した瞬間に //! text 内容を regex で検査 (= 編集時の即時検出層) +//! //! 両者は異なる timing / 検査対象で動作し、scratch file 検出は本 stage に集約。 //! scratch file は通常 .gitignore 対象で text content 検査の対象外のため、 //! file existence 検査である本 stage に責務を分離している。 @@ -170,10 +171,8 @@ fn pattern_middle_slice<'a>(parts: &'a [&'a str]) -> &'a [&'a str] { fn consume_prefix<'a>(name: &'a str, prefix: &str) -> Option<&'a str> { if prefix.is_empty() { Some(name) - } else if name.starts_with(prefix) { - Some(&name[prefix.len()..]) } else { - None + name.strip_prefix(prefix) } } diff --git a/src/hooks-pre-tool-validate/src/main.rs b/src/hooks-pre-tool-validate/src/main.rs index 7d5769f8..b8f95ca8 100644 --- a/src/hooks-pre-tool-validate/src/main.rs +++ b/src/hooks-pre-tool-validate/src/main.rs @@ -793,7 +793,7 @@ fn build_todo_staleness_message( keyword_matches: &[(String, Vec<(String, String)>)], branch: &str, ) -> Option { - let stale = behind.unwrap_or(0) > 0; + let stale = behind.is_none_or(|n| n > 0); let any_matches = keyword_matches.iter().any(|(_, m)| !m.is_empty()); if !stale && !any_matches { return None; @@ -810,6 +810,10 @@ fn build_todo_staleness_message( branch )); } + } else { + lines.push( + "stale parent detected: lineage 判定不能のため fail-closed で block".to_string(), + ); } for (keyword, matches) in keyword_matches { if matches.is_empty() { @@ -843,10 +847,7 @@ fn check_todo_staleness( .unwrap_or(TODO_STALENESS_DEFAULT_GREP_LIMIT); let behind = count_commits_branch_ahead(branch); - if behind.is_none() { - return None; - } - let stale = behind.unwrap_or(0) > 0; + let stale = behind.is_none_or(|n| n > 0); let keywords = extract_heading_keywords(text_for_keywords); let keyword_matches: Vec<(String, Vec<(String, String)>)> = if keywords.is_empty() {