feat(cli-push-runner): Bundle 1 — 順位 1 scratch file warning hook + 順位 116/134 docs + 順位 8 planning#174
Merged
Merged
Conversation
ADR-031 週次レビュー Phase B (順位 8) の MVP scope を 3 facets に維持しつつ、ユーザー 希望 7 観点を facet prompt に重点配分で対応する方針を確定 (2026-05-26 AskUserQuestion ヒアリング経由)。 - todo.md 順位 8 entry: 7 観点責務 mapping 表追記 (① ハーネス遵守 + ⑥ テストロジック を architecture-whole / simplicity-whole の筆頭 criteria に配置、② ③ は architecture-whole sub criterion、④ は security-whole、⑤ は順位 136 hook へ委譲、 ⑦ は aggregate 前 Rust 機械 pre-step で対応) - todo8.md 順位 136 entry: 週次レビュー ⑤ Todo 妥当性との責務分離を明記 (hook = 編集時 immediate / 週次 = batch 棚卸し) - todo9.md 順位 153 新規: review-harness-whole facet 追加 (Phase B+1、観点 ① 独立 facet 化の選択肢、Phase B dogfood 結果次第) - todo9.md 順位 154 新規: review-todo-whole facet + aggregate 前 file size pre-step 追加 (Phase B+1、観点 ⑤ ⑦ 拡張) - todo-summary.md: 順位 8 row update + 順位 153/154 row 追加
… 116/134 docs 順位 1 (PR #85 T1-4) push 前 untracked scratch file warning hook を実装。jj auto-snapshot 環境で .gitignore 漏れがあると `__*` 等の scratch ファイルが PR に意図せず混入する事故 (PR #85 で `__parse_transcripts.ps1` 実例) を構造的に 予防する。 ## 実装 (順位 1) - src/cli-push-runner/src/stages/scratch_file_warning.rs (新規): 新 stage - src/cli-push-runner/src/stages/mod.rs: module register - src/cli-push-runner/src/config.rs: ScratchFileWarningConfig 追加 + tests - src/cli-push-runner/src/main.rs: quality_gate 前に stage invoke + EXIT_SCRATCH_FILE_WARNING=6 - push-runner-config.toml: [scratch_file_warning] section 追加 (default-ON) 設計: - 配置: run_pipeline の最早期 (quality_gate より前) — 検出時は無駄な quality_gate 実行を回避 - 検出: `jj file list -r @` で @ commit に含まれるファイルを列挙、basename が config patterns (default: ["__*"]) に match するか検査 - Override: env var SCRATCH_FILE_WARNING_OVERRIDE=1/true/yes/on で意図的バイパス可能 - fail-open: jj 失敗時 (timeout/起動失敗) は warning ログのみで push 続行 - Config-driven patterns: 順位 5 (AI 生成一時スクリプト pattern) は本 stage の patterns 拡張 + ADR-007 連携で補完実装する設計 (重複ではなく補完アプローチ確定) ## Bundled docs - 順位 116: ADR-040 § `step_timeout` 比例係数の根拠 に sublinear / KV cache locality clarification を追記。実測値 600s 採択 vs 線形 derivation 720s 保守上限の使い分けを明示、永続 ADR の数値整合性確保。 - 順位 134: ADR-035 § ❌ 適用しない criteria 表に Magic number / hardcoded value 行を追加。docs-only PR の reviewer 誤適用 (PR #156 観測) の構造的予防。 ## Tests cargo test --manifest-path src/cli-push-runner/Cargo.toml: 122 passed 新規 test 30 件 (scratch_file_warning module 内 + config.rs parse)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughpush パイプライン先頭に scratch ファイル検出ステージを追加（設定スキーマ・パイプライン統合・実装・ユニットテスト含む）。併せて ADR-035/ADR-040 と todo 系ドキュメントを複数ファイルで更新し、週次レビューファセットや運用手順を整理しました。 ChangesScratch file warning パイプラインステージ
Policy 明確化と Project task 管理の更新
Sequence Diagram(s)sequenceDiagram
participant Client as Push client
participant Pipeline as run_pipeline
participant ScratchStage as run_scratch_file_warning
participant JJ as jj file list
participant Config as ScratchFileWarningConfig
Client->>Pipeline: call with config
Pipeline->>ScratchStage: config enabled?
ScratchStage->>Config: check enabled flag
Config-->>ScratchStage: enabled=true
ScratchStage->>JJ: jj file list -r @
JJ-->>ScratchStage: file paths
ScratchStage->>ScratchStage: extract basenames, match glob patterns
alt violations found
ScratchStage->>ScratchStage: check SCRATCH_FILE_WARNING_OVERRIDE env
alt override=true
ScratchStage-->>Pipeline: return true (bypass)
else override=false
ScratchStage-->>Pipeline: return false (block push)
end
else no violations
ScratchStage-->>Pipeline: return true (proceed)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/cli-push-runner/src/stages/scratch_file_warning.rs`:
- Around line 29-36: The function run_scratch_file_warning currently treats a
missing config as enabled by default; change the opt-in behavior so the feature
is OFF unless explicitly enabled in ScratchFileWarningConfig (i.e., derive
enabled only when config.is_some() && config.enabled == Some(true) rather than
unwrap_or(true)), update the config handling to honor a kill-switch/expiry field
(e.g., a bounded_lifetime or expires_at in ScratchFileWarningConfig) and ensure
the function checks that lifetime before enabling, and keep the existing
fail-open logging behavior on errors; update references to
run_scratch_file_warning and ScratchFileWarningConfig accordingly.
🪄 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: 4827dd04-5ba4-4bce-8117-fe8c81f3dd68
📒 Files selected for processing (12)
docs/adr/adr-035-doc-evaluation-policy.mddocs/adr/adr-040-local-llm-context-size.mddocs/todo-summary.mddocs/todo.mddocs/todo2.mddocs/todo8.mddocs/todo9.mdpush-runner-config.tomlsrc/cli-push-runner/src/config.rssrc/cli-push-runner/src/main.rssrc/cli-push-runner/src/stages/mod.rssrc/cli-push-runner/src/stages/scratch_file_warning.rs
💤 Files with no reviewable changes (1)
- docs/todo2.md
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli-push-runner/src/stages/scratch_file_warning.rs (1)
94-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
patternsの空文字要素で検知が実質無効化されます
effective_patternsは配列の空判定のみなので、patterns = ["", " "]のような設定で default__*にフォールバックせず、検知漏れになります。要素ごとに trim・空文字除去した後、空なら default を適用してください。修正例
fn effective_patterns(config: Option<&ScratchFileWarningConfig>) -> Vec<String> { - config - .and_then(|c| c.patterns.as_ref()) - .filter(|p| !p.is_empty()) - .cloned() - .unwrap_or_else(|| vec![DEFAULT_PATTERN.to_string()]) + let normalized = config + .and_then(|c| c.patterns.as_ref()) + .map(|patterns| { + patterns + .iter() + .map(|p| p.trim()) + .filter(|p| !p.is_empty()) + .map(|p| p.to_string()) + .collect::<Vec<_>>() + }) + .filter(|patterns| !patterns.is_empty()); + + normalized.unwrap_or_else(|| vec![DEFAULT_PATTERN.to_string()]) }+ #[test] + fn effective_patterns_default_when_only_blank_entries() { + let config = ScratchFileWarningConfig { + enabled: Some(true), + patterns: Some(vec!["".to_string(), " ".to_string()]), + }; + assert_eq!(effective_patterns(Some(&config)), vec!["__*".to_string()]); + }🤖 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-push-runner/src/stages/scratch_file_warning.rs` around lines 94 - 100, The function effective_patterns currently only checks if the patterns Vec is empty and will treat entries like "" or " " as valid, which disables fallback to DEFAULT_PATTERN; update effective_patterns (and refer to ScratchFileWarningConfig::patterns and DEFAULT_PATTERN) to iterate over the provided patterns, trim each element, filter out empty strings, clone the remaining entries into a Vec, and if that resulting Vec is empty return vec![DEFAULT_PATTERN.to_string()] instead; keep handling Option<&ScratchFileWarningConfig> the same but replace the direct cloned unwrap logic with this trim/filter/collect then fallback flow.
🤖 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.
Outside diff comments:
In `@src/cli-push-runner/src/stages/scratch_file_warning.rs`:
- Around line 94-100: The function effective_patterns currently only checks if
the patterns Vec is empty and will treat entries like "" or " " as valid,
which disables fallback to DEFAULT_PATTERN; update effective_patterns (and refer
to ScratchFileWarningConfig::patterns and DEFAULT_PATTERN) to iterate over the
provided patterns, trim each element, filter out empty strings, clone the
remaining entries into a Vec, and if that resulting Vec is empty return
vec![DEFAULT_PATTERN.to_string()] instead; keep handling
Option<&ScratchFileWarningConfig> the same but replace the direct cloned unwrap
logic with this trim/filter/collect then fallback flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3188a7c6-6795-4a10-bca3-a239e2acb1f4
📒 Files selected for processing (3)
push-runner-config.tomlsrc/cli-push-runner/src/config.rssrc/cli-push-runner/src/stages/scratch_file_warning.rs
… default fallback 対象に (CR #174 Minor)
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
6 tasks
aloekun
added a commit
that referenced
this pull request
May 26, 2026
…158 docs (#175) * docs(todo): PR #174 post-merge-feedback 採用 4 件を todo9.md / summary table に登録 PR #174 (Bundle 1) post-merge-feedback で採用判定された 4 件を docs/todo9.md に 新規エントリとして追加し、docs/todo-summary.md table に行追加。 - 順位 155 (T1 #1): cli-pr-monitor fix chain 末尾に空 commit 検査 + jj abandon step 追加 (PR #174 で観測した kqvluqyv 事例の構造化予防、Effort S) - 順位 156 (T2 #1): effective_patterns() の空 variant test 4 件直交化 (CR Minor finding 起点の 4 filter path 独立検証化、Effort S) - 順位 157 (T2 #2): Bundle 1 dogfood checklist 実行 — __test.ps1 block + override env 確認 (ADR-039 bounded lifetime data point #1、Effort XS) - 順位 158 (T3 #1): ADR-039 に bounded lifetime decision trigger 必須化パターン 追記 (PR #174 の 3-5 PR dogfood 実例を reusable rule に昇格、Effort S) * feat(cli-push-runner): Bundle 2 — 順位 2 bookmark check stage + 順位 156/158 docs 順位 2 (PR #85 T1-3) jj bookmark 未設定 early-exit を実装。jj 環境では新規ブランチ で bookmark を作成し忘れる落とし穴があり、PR #85 で初回 `pnpm push` が `Nothing changed` で終了し 158s かけた quality_gate + takt review が無駄になった 事象の構造的予防。 ## 実装 (順位 2) - src/cli-push-runner/src/stages/bookmark_check.rs (新規): 新 stage - src/cli-push-runner/src/stages/mod.rs: module register - src/cli-push-runner/src/main.rs: - Stage -1 として run_bookmark_check を pipeline 最早期に追加 - EXIT_BOOKMARK_MISSING = 7 を新設 - run_pipeline が 50 行制限を超えたため run_pre_checks を抽出して責務分離 設計: - 配置: scratch_file_warning より前の最早期 stage (push 自体不可能な状態を最優先で fail-fast) - 検出: `jj bookmark list` で非 trunk bookmark の有無を確認 (lib_jj_helpers::is_trunk_bookmark で filter) - 失敗時挙動: error 終了 + 推奨コマンド (`jj bookmark create <name> -r @`) 提示 - fail-open: jj 実行失敗時は warning ログのみで push を続行 (jj 不調で push 自体を止めない) - 設計上の non-config: jj git push が bookmark 必須仕様のため、本 stage をバイパスする 正当な use case なし。`[bookmark_check]` config section は追加せず常に有効 ## Bundled docs - 順位 156: effective_patterns() の空 variant test 4 件直交化。既存 3 test を filter path 直交命名 (all_blank / mixed_blank_and_valid / whitespace_padded) にリネーム + 4 番目 variant (mixed_filter_to_empty) を新規追加。1 path regression で 1 test だけ failed する構造 (memory feedback_test_dry_antipattern.md 適用)。 - 順位 158: ADR-039 § 3 Bounded lifetime に「明示的 decision trigger 必須化」要件を 追記。PR #174 を実例 (Bundle 1 = 3-5 PR dogfood trigger) として cite、formless な 「未来の判定」記述を ADR レベルで禁止する reusable pattern として codify。 ## Tests cargo test --manifest-path src/cli-push-runner/Cargo.toml: 133 passed 新規 test: bookmark_check 6 件 + effective_patterns 1 件 (variant #4) * docs(todo): Bundle 2 完了に伴い 順位 2 / 156 / 158 を削除 * fix(cli-push-runner): Bundle 2 CR Minor 2 件対応 (main.rs message punctuation + todo9.md 順位 155 design clarification)
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle 1 (案 A 第 1 PR): 順位 1 (push 前 untracked
__*ファイル警告 hook) を実装し、orthogonal な Tier 3 docs 2 件 (順位 116 / 134) + 前セッションの週次レビュー planning を同 PR に bundle 化。scratch_file_warning追加。jj file list -r @で@commit 内の__*ファイル混入を検出、env override (SCRATCH_FILE_WARNING_OVERRIDE=1) で意図的バイパス可能。jj auto-snapshot 環境で.gitignore漏れがあると scratch file が PR に混入する事故 (PR docs(todo): 順位 1-3 グローバルルール完了に伴い todo エントリを削除 #85 で実証) の構造的予防。_tmp_*等の patterns 拡張 + ADR-007 連携のみで実装可能になり、別 PR (Bundle 3) に分離。step_timeout比例係数の根拠 に sublinear vs 線形 derivation 使い分け明示 (実測 600s 正規採択 / 線形 720s は保守上限)。Commit Chain
uvrtpuzpdocs(todo): 週次レビュー Phase B 7 観点責務 mapping 確定 + Phase B+1 follow-up 登録mplpnrtvfeat(cli-push-runner): Bundle 1 — 順位 1 scratch file warning hook + 順位 116/134 docswomtslrsdocs(todo): Bundle 1 完了に伴い 順位 1 / 116 / 134 を削除Implementation Details (順位 1)
新 stage 配置:
run_pipelineの最早期 (quality_gate より前)。検出時は無駄な quality_gate 実行を回避。src/cli-push-runner/src/stages/scratch_file_warning.rs: 新 stage 本体 (464 行、自己説明的なヘルパ関数consume_prefix/consume_middle/check_suffixで glob matching を分解、//非 doc コメント 0 件)src/cli-push-runner/src/config.rs:ScratchFileWarningConfig { enabled, patterns }追加、Configに Optional field 追加 + parse test 3 件src/cli-push-runner/src/main.rs: stage 呼び出し +EXIT_SCRATCH_FILE_WARNING = 6src/cli-push-runner/src/stages/mod.rs: module exportpush-runner-config.toml:[scratch_file_warning]section (defaultenabled = true,patterns = ["__*"])設計判断:
jj file listtimeout / 起動失敗時は warning ログのみで push 続行 (jj 不調で push 自体を止めない)subdir/__hidden.txtは match、__src/main.rsは match しない (dirname に__が含まれても basename が一致しなければ無視)_tmp_*等を追加するだけで pattern 拡張可能 (= 補完アプローチ確定)Test Plan
cargo test --manifest-path src/cli-push-runner/Cargo.toml: 122 passed (新規 30 件: scratch_file_warning module 19 件 + config parse 3 件 + 既存に統合)pnpm build:all: 全 hook/CLI exe release profile ビルド成功//非 doc コメント違反 → refactor で全解消、self-documenting helper 関数に分解)__test.ps1を意図的に作って push し、block 動作と override env の意図的バイパスを確認Out of Scope (案 A 後続 PR)
Summary by CodeRabbit
New Features
Documentation