feat(hooks): merge 前 mechanical gate 強化 (clippy + 空 commit sweep)

[aloekun]

Summary

• 順位 175 (PR feat: Bundle CR-RL — CR rate-limit detection を新フォーマット対応に拡張 (順位 167-169) + 174 #185 T1-feat(hooks): 設定駆動型アーキテクチャに移行し配布自動化を実装 #1): cargo clippy --workspace -- -D warnings を [stop_quality] + [quality_gate] に追加し Rust lint structural gap を補填
• 順位 155 (PR feat(cli-push-runner): Bundle 1 — 順位 1 scratch file warning hook + 順位 116/134 docs + 順位 8 planning #174 T1-feat(hooks): 設定駆動型アーキテクチャに移行し配布自動化を実装 #1): cli-pr-monitor execute_repush_flow 末尾で master..@ 範囲の空 commit を sweep して jj abandon (kqvluqyv 事例の構造的予防)
• 前提として既存 clippy errors 5 件を併せて修正 (両 Tier 1 を 1 PR でバンドル、feedback_minimize_pr_count_during_rate_limit 適用)

Why

• 順位 175: stop_quality は pnpm lint / pnpm lint:md / pnpm test / pnpm build のみで Rust の cargo clippy が完全欠落しており、PR feat: Bundle CR-RL — CR rate-limit detection を新フォーマット対応に拡張 (順位 167-169) + 174 #185 開発時に doc_lazy_continuation clippy error が手動実行まで未検出だった structural gap。本 PR 自身が dogfood として 5 件の pre-existing errors を land 前に検出。
• 順位 155: 既存 try_abandon_empty_fix_commit は tracked な単一 fix commit のみ対象。PR feat(cli-push-runner): Bundle 1 — 順位 1 scratch file warning hook + 順位 116/134 docs + 順位 8 planning #174 で観測した granduncle 位置の kqvluqyv のような untracked 空 commit を見逃し、次回 push で PR diff を汚染した構造的欠陥。master..@ 範囲を網羅 sweep する補完層を追加。

Changes

Phase 1 — 既存 clippy errors 修正 (順位 175 の前提)

• src/hooks-pre-tool-validate/src/main.rs: if behind.is_none() { return None; } → behind?;
• src/cli-push-runner/src/stages/scratch_file_warning.rs: doc list 段落区切り (doc_lazy_continuation ×3)
• src/cli-push-runner/src/stages/scratch_file_warning.rs: name.strip_prefix(prefix) (manual_strip)

Phase 2 — 順位 175: cargo clippy mechanical gate

• .claude/hooks-config.toml: [stop_quality] に lint:rust step 追加
• push-runner-config.toml: [quality_gate.groups] の rust-test → rust-lint-test rename し clippy を先頭追加 (cargo target dir のロック衝突回避のため同 group 統合)

Phase 3 — 順位 155: 範囲 sweep

• src/cli-pr-monitor/src/fix_commit.rs: sweep_empty_commits_in_pr_range() 新規 + parse_empty_change_ids() pure helper
• src/cli-pr-monitor/src/stages/repush.rs: execute_repush_flow 末尾で sweep 呼び出し
• 新規テスト: parse pure 4 件 + integration 2 件 + 3 test helper (setup / enter_repo / assert_descriptions_absent_in_pr_range)

docs cleanup (2 commit 構成)

• docs/todo10.md / docs/todo9.md / docs/todo-summary.md: 順位 175 + 155 完了 entry 削除 (memory feedback_todo_no_history 適用)

設計判断 (sweep 配置)

execute_repush_flow 内で sweep を呼ぶ位置として 3 案を検討:

1. match action の 前: 現 iteration の push 前に sweep → AutoPush の場合も remote が clean
2. match action の 中 (CleanupEmptyFixCommit と統合): tracked と untracked を統一処理
3. match action の 後 (採用): 既存 behavior を完全保護、追加的 sweep として動作

案 3 採用理由: 既存 try_abandon_empty_fix_commit は reparent_at_to_pr_tip を含む丁寧な cleanup を行うため、sweep を先に走らせて @ を動かすと既存ロジックが [warn] でスキップされ reparent が機能しなくなる。1 iteration 遅れ で次回 push 前に清掃されるトレードオフを受け入れ、現 iteration の既存 flow を保護する。

Test plan

• cargo clippy --workspace -- -D warnings: pass (本 PR が pre-existing 5 件を修正)
• cargo test --workspace: 1200+ unit tests pass, 0 failed
• cargo test -- --ignored integration_sweep: 2/2 pass (jj 実依存、PR feat(cli-push-runner): Bundle 1 — 順位 1 scratch file warning hook + 順位 116/134 docs + 順位 8 planning #174 kqvluqyv 事例の最小再現で動作実証)
• pnpm build:all: 14 exes 全 release build 成功
• pre-push review: APPROVE
• merge 後 next PR で dogfood: 新 lint:rust step が stop hook で発火するか / sweep が PR diff 汚染を防ぐか

Bundle 内 dogfood 観測

本 PR の Edit 中に hooks-post-tool-comment-lint-rust が 3 回発火 (コメント禁止 7 件 + 関数長 50 行超過 3 件) し、設計修正 (コメント削除 + helper 抽出) を Claude 自身が即座に対応。順位 175 の clippy gate と同じ Bundle Z #B-α 決定論層 family の mechanical enforcement が land 前のセッション内で機能した実例。

Summary by CodeRabbit

• 新機能

  • Rust 向けの静的チェック（clippy）を品質ゲート/パイプラインに追加
  • PR 内の空コミットを検出して後処理（sweep）する自動化を追加（設定で有効化）

• 改善

  • push パイプラインの Rust チェック範囲と実行順序を拡張
  • TODO一覧・進行中ドキュメントの整理

• 変更

  • 未判定のブランチ差分を「古い」と扱う挙動に変更（通知が生成されます）

• テスト

  • 空コミット処理のユニット/統合テストを追加

[coderabbitai]

📝 Walkthrough

Walkthrough

Rust ワークスペース向けに cargo clippy を stop_quality/push-runner 品質ゲートへ追加し、cli-pr-monitor に PR 範囲の空 fix commit を列挙して jj abandon で削除する sweep 機能を実装・repush フローへ統合、関連 todo を削除しました。

変更

Rust Clippy 品質ゲート追加

Layer / File(s)	Summary
stop_quality ゲートへの clippy ステップ追加 .claude/hooks-config.toml	[stop_quality] に lint:rust ステップを追加し、cargo clippy --workspace -- -D warnings と順位・所要時間・cold cache リスクに関するコメントを追記。
push-runner 品質ゲートグループの再編 push-runner-config.toml	quality_gate.groups を rust-lint-test に再編し、clippy を workspace 対象で fail-fast として追加。既存の cargo test/--ignored テストコマンドは保持。

空 commit 自動クリーンアップ実装

Layer / File(s)	Summary
sweep 関数とテスト実装 src/cli-pr-monitor/src/fix_commit.rs	sweep_empty_commits_in_pr_range と parse_empty_change_ids を実装し、jj log で PR 範囲（default_branch..@）の empty fix commit の change_id を抽出して jj abandon で順次削除する処理、ユニット/統合テストと補助セットアップ関数を追加。
repush フローへの統合 src/cli-pr-monitor/src/stages/repush.rs	execute_repush_flow の action 実行を整理し、fix_config.sweep.enabled のときに sweep_empty_commits_in_pr_range(&fix_config.sweep.default_branch) を呼び出すよう統合。
完了タスク記録の削除 docs/todo-summary.md, docs/todo10.md, docs/todo9.md	「cli-pr-monitor fix chain に空 commit 検査＋jj abandon 追加」および「cargo clippy を stop_quality に追加」の完了タスク記述を削除。

設定拡張とテスト

Layer / File(s)	Summary
FixConfig に sweep を追加 src/cli-pr-monitor/src/config.rs	SweepConfig 型（enabled: bool, default_branch: String）を追加し、FixConfig に #[serde(default)] pub(crate) sweep: SweepConfig を追加。デフォルト値と TOML 読み込みのテストを追加。

コード品質向上（小変更）

Layer / File(s)	Summary
文字列処理の簡素化 src/cli-push-runner/src/stages/scratch_file_warning.rs	consume_prefix を starts_with + 手動スライスから str::strip_prefix に置き換え。
staleness 判定の扱い変更 src/hooks-pre-tool-validate/src/main.rs	behind == None の場合も stale 扱いとし、メッセージに「lineage 判定不能のため fail-closed で block」を追加。早期 return を廃止して処理を継続。

Sequence Diagram(s)

sequenceDiagram
  participant RepushFlow as execute_repush_flow
  participant RepushAction as execute_repush_action
  participant Sweep as sweep_empty_commits_in_pr_range
  participant JJLog as "jj log"
  participant JJAbandon as "jj abandon"

  RepushFlow->>RepushAction: perform RepushAction
  RepushAction-->>RepushFlow: action result
  RepushFlow->>Sweep: if fix_config.sweep.enabled
  Sweep->>JJLog: run revset query (default_branch..@) for empty fix(review) commits
  JJLog-->>Sweep: change_id list
  Sweep->>JJAbandon: jj abandon <change_id> (for each)
  JJAbandon-->>Sweep: abandon results
  Sweep-->>RepushFlow: sweep complete

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

関連する可能性のあるプルリクエスト

• aloekun/claude-code-hook-test#177: hooks-pre-tool-validate の staleness 判定変更と同一領域に関係する PR。
• aloekun/claude-code-hook-test#63: 既存の空 fix commit abandon ロジック（fix_commit.rs 周辺）との機能的重なり。
• aloekun/claude-code-hook-test#44: repush フロー実装・拡張に関する既往 PR。

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	タイトルは、PR の主要な変更内容（mechanical gate 強化、clippy追加、空 commit sweep）を簡潔に正確に反映している。
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/cli-pr-monitor/src/stages/repush.rs (1)

145-169: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

fail-safe 分岐のあとで sweep を無条件実行しないでください。

上の分岐は IdCaptureFailed や try_abandon_empty_fix_commit() の安全側スキップでローカル履歴を触らない設計ですが、Line 169 で直後に再び jj abandon を走らせています。これだと「不確実なので何もしない」と判断したケースでも sweep が同じ空 commit を消し得るので、少なくとも安全に状態を信頼できる action に限定して呼ぶ必要があります。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli-pr-monitor/src/stages/repush.rs` around lines 145 - 169, The
unconditional call to
crate::fix_commit::sweep_empty_commits_in_pr_range("master") must be removed and
only invoked for actions that are allowed to mutate local history; update the
match over RepushAction to call sweep_empty_commits_in_pr_range("master") inside
the safe arms (e.g., RepushAction::AutoPush and
RepushAction::CleanupEmptyFixCommit { .. }) instead of after the match, leaving
the fail-safe arms (RepushAction::UserConfirmWithSeparatedFix,
RepushAction::UserConfirmNoSeparation, RepushAction::SkipNoChange,
RepushAction::FailSafeCaptureFailed and any IdCaptureFailed paths) without any
sweep; keep the existing try_abandon_empty_fix_commit("fix_state=Created:",
Some(&commit_id)) usage in CleanupEmptyFixCommit and add the sweep call there
(and in run_auto_push result path) so only trusted flows run
sweep_empty_commits_in_pr_range.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@push-runner-config.toml`:
- Line 64: The comment "# PostToolUse / Stop hook では実行せず、イテレーション速度を保護する。" in
push-runner-config.toml is outdated because a Stop hook (see
[[stop_quality.steps]] name = "lint:rust" in .claude/hooks-config.toml) now
runs; update that comment to accurately reflect current behavior (e.g., state
that the hook runs on Stop and mention impact on iteration speed or that
lint:rust is executed on Stop) so operators aren't misled.

In `@src/cli-pr-monitor/src/fix_commit.rs`:
- Around line 217-238: run_cmd_direct returns combined stdout/stderr so the
current code passes merged output into parse_empty_change_ids which may pick up
jj warnings from stderr; change this by making the jj log invocation use only
stdout (call/run the underlying function to return stdout separately) or by
filtering the returned out before calling parse_empty_change_ids: feed
parse_empty_change_ids only lines that match a valid change_id pattern (e.g.
regex for expected ID format) or explicitly split and discard stderr-like lines;
update the code around run_cmd_direct, the variable out, and the call site
parse_empty_change_ids to ensure only valid stdout-derived change IDs are
processed before using change_ids (and keep JJ_CMD_TIMEOUT_SECS/ok handling
unchanged).
- Around line 203-248: The revset used in sweep_empty_commits_in_pr_range is too
broad (empty() & (default_branch..@)) and abandons any empty commit; narrow it
to only target fix-related commits by adding a provenance/summary filter (e.g.
require commit summary to match the fix pattern). Change the revset construction
in sweep_empty_commits_in_pr_range (the revset variable) to include a summary
regex like summary ~ "^fix\\(" (or another project-specific marker for fix
commits) so parse_empty_change_ids and the subsequent run_cmd_direct("jj",
&["abandon", cid], ...) only operate on intended fix commits; update the log
message accordingly and keep existing fail-open behavior.

In `@src/cli-pr-monitor/src/stages/repush.rs`:
- Line 169: Replace the hardcoded "master" call in repush.rs to respect the
experimental feature pattern: read an opt-in flag and kill-switch from config
(e.g., config.experimental.sweep_empty_commits or
config.kill_switchs.sweep_empty_commits), fetch the repository default branch
name (do not assume "master") and pass that variable into
sweep_empty_commits_in_pr_range, and add a bounded lifetime parameter (e.g.,
max_age_days from config) so the sweep only runs for a limited window; ensure
the sweep is skipped unless the opt-in is set and not globally disabled by the
kill-switch.

In `@src/hooks-pre-tool-validate/src/main.rs`:
- Around line 845-847: The current use of behind? after calling
count_commits_branch_ahead causes check_todo_staleness to return None when
lineage cannot be determined, bypassing the fail-closed behavior; change the
handling so None is treated as "stale" instead of propagating None: in
check_todo_staleness replace the behind? call with explicit matching or map_or
(e.g., treat None as >0) so stale is true when count_commits_branch_ahead
returns None, and update build_todo_staleness_message to likewise interpret a
None behind result as stale (return the same stale message path as for non-zero
behind) so both functions consistently treat None => stale; refer to
count_commits_branch_ahead, check_todo_staleness, build_todo_staleness_message
and the behind variable.

---

Outside diff comments:
In `@src/cli-pr-monitor/src/stages/repush.rs`:
- Around line 145-169: The unconditional call to
crate::fix_commit::sweep_empty_commits_in_pr_range("master") must be removed and
only invoked for actions that are allowed to mutate local history; update the
match over RepushAction to call sweep_empty_commits_in_pr_range("master") inside
the safe arms (e.g., RepushAction::AutoPush and
RepushAction::CleanupEmptyFixCommit { .. }) instead of after the match, leaving
the fail-safe arms (RepushAction::UserConfirmWithSeparatedFix,
RepushAction::UserConfirmNoSeparation, RepushAction::SkipNoChange,
RepushAction::FailSafeCaptureFailed and any IdCaptureFailed paths) without any
sweep; keep the existing try_abandon_empty_fix_commit("fix_state=Created:",
Some(&commit_id)) usage in CleanupEmptyFixCommit and add the sweep call there
(and in run_auto_push result path) so only trusted flows run
sweep_empty_commits_in_pr_range.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16081d24-e75b-484d-ae11-7968a14a91ba

📥 Commits

Reviewing files that changed from the base of the PR and between f59a2f7 and ec62dff.

📒 Files selected for processing (9)

• .claude/hooks-config.toml
• docs/todo-summary.md
• docs/todo10.md
• docs/todo9.md
• push-runner-config.toml
• src/cli-pr-monitor/src/fix_commit.rs
• src/cli-pr-monitor/src/stages/repush.rs
• src/cli-push-runner/src/stages/scratch_file_warning.rs
• src/hooks-pre-tool-validate/src/main.rs

💤 Files with no reviewable changes (3)

• docs/todo10.md
• docs/todo9.md
• docs/todo-summary.md

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

jj log の結合出力をそのまま change_id 一覧として parse しないでください。

src/cli-pr-monitor/src/runner.rs:24-78 の run_cmd_direct() は成功時でも stdout/stderr を結合して返します。ここで jj log が warning を stderr に出すと、その行まで parse_empty_change_ids() に入り、存在しない ID を jj abandon に渡す形になります。ID 抽出は stdout のみに限定するか、少なくとも change_id として妥当な行だけを通してください。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli-pr-monitor/src/fix_commit.rs` around lines 217 - 238, run_cmd_direct
returns combined stdout/stderr so the current code passes merged output into
parse_empty_change_ids which may pick up jj warnings from stderr; change this by
making the jj log invocation use only stdout (call/run the underlying function
to return stdout separately) or by filtering the returned out before calling
parse_empty_change_ids: feed parse_empty_change_ids only lines that match a
valid change_id pattern (e.g. regex for expected ID format) or explicitly split
and discard stderr-like lines; update the code around run_cmd_direct, the
variable out, and the call site parse_empty_change_ids to ensure only valid
stdout-derived change IDs are processed before using change_ids (and keep
JJ_CMD_TIMEOUT_SECS/ok handling unchanged).

[aloekun]

resolved: run_cmd_direct 非成功時は早期 return で parse 対象外。jj log 成功時は stderr emit せず structured な change_id 出力のみを parse する経路。dogfood で stderr leakage 観測時に別途追加防御を検討。

[coderabbitai]

@aloekun、説明ありがとうございます。整理できました。

• 失敗パス: !ok で即 return → parse_empty_change_ids は呼ばれない ✓
• 成功パス: jj log --no-graph -T "change_id ++ \"\\n\"" は structured template 出力のみ。成功時に jj が stderr へ warning を出すケースは通常ない ✓
• 追加の安全層: 実装では revset に description(substring:"fix(review):") フィルタが入っており、diff 時点のドキュメントより scope がさらに絞られている ✓

dogfood で stderr leakage が観測された場合の段階的防御方針も妥当です。現状の実装で問題ないと判断します。

---

🧠 Learnings used

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 119
File: src/cli-finding-classifier/src/lib.rs:86-96
Timestamp: 2026-05-06T14:17:51.353Z
Learning: In `src/cli-finding-classifier/src/lib.rs`, the `normalized_issue` field from LLM output is intentionally validated only with trim + non-empty filter (no newline/length checks). Strict validation (single-line, max 80 chars enforcement + fallback) is deferred to Phase 5 (cli-pr-monitor integration) or when an actual problem occurs. The current prompt template (`classify.txt`) includes a `max 80 chars` instruction that is considered sufficient guard in the meantime. Do not flag this as missing validation in code reviews.

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 71
File: src/hooks-stop-feedback-dispatch/src/main.rs:127-135
Timestamp: 2026-04-23T17:49:44.755Z
Learning: In `aloekun/claude-code-hook-test`, `src/hooks-stop-feedback-dispatch/src/main.rs`, the stale-GC branch in `handle_pending()` unconditionally deletes `{pending}.lock` before acquiring the lock. This is an accepted tradeoff: the race window is ~0.001% (24h TTL vs 10–100ms dispatch), and acquiring the lock before stale cleanup was rejected because it would eliminate the only recovery path for leaked locks (from kill -9 etc.), causing permanent stuck state. The design relies on the 24h TTL as a backstop for leaked locks. Re-raise this finding only if: (1) double dispatch is observed in production, (2) a shorter stale TTL (e.g., 1h) is needed, or (3) the code is ported to a hostile multi-process environment.

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 53
File: docs/todo.md:106-106
Timestamp: 2026-04-18T08:42:52.662Z
Learning: In `aloekun/claude-code-hook-test`, within `src/cli-pr-monitor`, `run_push()` is defined in `src/cli-pr-monitor/src/stages/push.rs` (line 15) as `pub(crate) fn run_push(config: &FixConfig) -> bool`. It is imported and called from `src/cli-pr-monitor/src/stages/repush.rs` via `use crate::stages::push::run_push;`. Do NOT flag references to `push.rs` for `run_push` as incorrect — `push.rs` is the canonical definition site.

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 99
File: docs/pipeline-token-efficiency.md:222-242
Timestamp: 2026-05-01T17:18:31.023Z
Learning: In `aloekun/claude-code-hook-test`, `docs/pipeline-token-efficiency.md` is a phased improvement planning document. Specifications described within a "Phase N" section (e.g., `#B-β` Phase 2, `#B-γ` Phase 3) represent future planned implementations and should NOT be flagged as missing implementations in PRs that only cover an earlier phase. Always check the PR scope and the progress table at the bottom of the document to determine which phase is being implemented before raising findings about unimplemented specs.

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 44
File: src/cli-pr-monitor/src/stages/repush.rs:40-54
Timestamp: 2026-04-16T15:41:17.548Z
Learning: In `aloekun/claude-code-hook-test`, `auto_push_severity` in `src/cli-pr-monitor` is a configuration preset (not a severity threshold comparator). Valid values are "none" / "critical" / "major", where both "critical" and "major" mean "always auto push" per ADR-019 L44-46. The function `should_auto_push(setting: &str)` in `src/cli-pr-monitor/src/stages/repush.rs` correctly returns `true` for both "critical" and "major" — do NOT flag this as missing threshold comparison logic. ADR-019 L52 prohibits double-judging AI evaluations in Rust, and ADR-019 L78 prohibits raw-findings-based auto push judgment.

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 64
File: docs/adr/adr-022-automation-responsibility-separation.md:128-128
Timestamp: 2026-04-21T09:09:02.341Z
Learning: In aloekun/claude-code-hook-test, markdownlint-cli2 (0.22.0) can false-positive MD038 (no-space-in-code) on content inside fenced code blocks (``` ... ```). MD038 only applies to inline backtick code spans, not fenced code block content. Do not flag MD038 warnings for lines inside fenced code blocks in this repository.

---

Learnt from: CR
Repo: aloekun/claude-code-hook-test PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-25T14:21:38.565Z
Learning: Applies to hooks/**/*.rs : Implement Stop hooks as quality gates (ADR-004) to enforce code quality standards before proceeding

---

Learnt from: aloekun
Repo: aloekun/claude-code-hook-test PR: 38
File: src/cli-pr-monitor/src/runner.rs:109-115
Timestamp: 2026-04-15T05:44:09.231Z
Learning: In this Windows-only `cli-pr-monitor` tool, hardcoded Windows `.exe` expectations are intentional. Do not require cross-platform handling (e.g., avoid changing `.exe` suffix usage) when reviewing Rust code under `src/cli-pr-monitor/src/`—including logic such as `checker_exe_path()` and related `.exe` references in error messages.

[coderabbitai]

♻️ Duplicate comments (1)

src/cli-pr-monitor/src/config.rs (1)

97-118: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

ADR-039 の実験機能パターン要件が未充足です。

Line 97-118 の SweepConfig は opt-in（enabled）のみで、kill-switch と bounded lifetime がありません。設定スキーマに両方を追加し、実行側で必ず評価してください。

🔧 最小追加イメージ

 pub(crate) struct SweepConfig {
     #[serde(default)]
     pub(crate) enabled: bool,
+    #[serde(default)]
+    pub(crate) kill_switch: bool,
+    #[serde(default = "default_sweep_ttl_days")]
+    pub(crate) ttl_days: u16,
     #[serde(default = "default_sweep_branch")]
     pub(crate) default_branch: String,
 }
+
+fn default_sweep_ttl_days() -> u16 {
+    30
+}

// repush 側イメージ（別箇所）
if fix_config.sweep.enabled
    && !fix_config.sweep.kill_switch
    && !fix_config.sweep.is_expired()
{
    sweep_empty_commits_in_pr_range(&fix_config.sweep.default_branch);
}

As per coding guidelines "Implement Experimental feature standard pattern with config opt-in, kill-switch, and bounded lifetime (ADR-039)".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli-pr-monitor/src/config.rs` around lines 97 - 118, SweepConfig
currently only provides opt-in via enabled; add the ADR-039 required kill-switch
and bounded lifetime by adding a pub(crate) kill_switch: bool with serde default
false and a pub(crate) expires_at: Option<DateTime<Utc>> (or equivalent
timestamp string) plus an is_expired(&self) -> bool method that returns true if
expires_at is Some and <= now; update Default for SweepConfig to set kill_switch
= false and expires_at = None (or a sane default), and update the execution site
that calls sweep_empty_commits_in_pr_range to guard with the full check:
fix_config.sweep.enabled && !fix_config.sweep.kill_switch &&
!fix_config.sweep.is_expired() so the sweep runs only when enabled, not killed,
and not expired.

🧹 Nitpick comments (1)

src/hooks-pre-tool-validate/src/main.rs (1)

2187-2231: ⚡ Quick win

build_todo_staleness_message の behind = None ケースのテストがありません。

既存テストは behind が Some(0), Some(2), Some(3) のケースをカバーしていますが、今回追加された fail-closed 動作 (behind = None) のテストがありません。

🧪 テスト追加案

     #[test]
     fn build_todo_staleness_message_neither_returns_none() {
         let path = build_todo_path("");
         let msg = build_todo_staleness_message(&path, Some(0), &[], "master");
         assert!(msg.is_none());
     }
+
+    #[test]
+    fn build_todo_staleness_message_fail_closed_when_behind_none() {
+        let path = build_todo_path("");
+        let msg = build_todo_staleness_message(&path, None, &[], "master");
+        let msg = msg.expect("None behind should produce fail-closed message");
+        assert!(msg.contains("lineage 判定不能"));
+        assert!(msg.contains("fail-closed"));
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks-pre-tool-validate/src/main.rs` around lines 2187 - 2231, Add a unit
test covering the new fail-closed behavior when build_todo_staleness_message is
called with behind = None: create a test (e.g.,
build_todo_staleness_message_behind_none_returns_stale) that calls
build_todo_staleness_message(&path, None, &[], "main") and asserts the result is
Some(...) and that the message contains the path, the branch name ("main"), and
the stale indicator (e.g., "stale parent detected"); also add a second small
case with a non-empty matches vector to assert it still includes the
related-implementation text (e.g., "関連既実装の可能性") when matches are present. Ensure
the tests refer to build_todo_staleness_message and use the existing
build_todo_path helper for the path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/cli-pr-monitor/src/config.rs`:
- Around line 97-118: SweepConfig currently only provides opt-in via enabled;
add the ADR-039 required kill-switch and bounded lifetime by adding a pub(crate)
kill_switch: bool with serde default false and a pub(crate) expires_at:
Option<DateTime<Utc>> (or equivalent timestamp string) plus an is_expired(&self)
-> bool method that returns true if expires_at is Some and <= now; update
Default for SweepConfig to set kill_switch = false and expires_at = None (or a
sane default), and update the execution site that calls
sweep_empty_commits_in_pr_range to guard with the full check:
fix_config.sweep.enabled && !fix_config.sweep.kill_switch &&
!fix_config.sweep.is_expired() so the sweep runs only when enabled, not killed,
and not expired.

---

Nitpick comments:
In `@src/hooks-pre-tool-validate/src/main.rs`:
- Around line 2187-2231: Add a unit test covering the new fail-closed behavior
when build_todo_staleness_message is called with behind = None: create a test
(e.g., build_todo_staleness_message_behind_none_returns_stale) that calls
build_todo_staleness_message(&path, None, &[], "main") and asserts the result is
Some(...) and that the message contains the path, the branch name ("main"), and
the stale indicator (e.g., "stale parent detected"); also add a second small
case with a non-empty matches vector to assert it still includes the
related-implementation text (e.g., "関連既実装の可能性") when matches are present. Ensure
the tests refer to build_todo_staleness_message and use the existing
build_todo_path helper for the path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 092c1813-d53b-4d53-bc72-9aa75c297988

📥 Commits

Reviewing files that changed from the base of the PR and between ec62dff and dfad56f.

📒 Files selected for processing (4)

• src/cli-pr-monitor/src/config.rs
• src/cli-pr-monitor/src/fix_commit.rs
• src/cli-pr-monitor/src/stages/repush.rs
• src/hooks-pre-tool-validate/src/main.rs