chore(workspace): 旧 cli-push-pipeline crate を削除 (push パイプライン改善 T2) - #286
Merged
Conversation
📝 WalkthroughWalkthrough旧 ChangesPush pipeline crate removal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし — レビュー自体が未実施のためフィルタ対象の指摘なし) 軽量サマリー (レビュー指摘が無いため CI + diff 概要)
次のアクション
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし — レビュー内容自体が未着のためフィルタ対象の指摘なし) 軽量サマリー (レビュー指摘が無いため CI + diff 概要)
次のアクション
|
![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 `@docs/push-pipeline-fix-plan.md`:
- Around line 53-57: Update the T2 implementation plan around the T2 execution
guidance to remove the unconditional instruction to use PR_SIZE_CHECK_OVERRIDE.
State that the override should be used only when measurement shows it is
necessary, consistent with the corrected T2 results and gate-bypass policy.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ec6cdf5-0248-4340-bedd-0315a59e13f3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomldocs/adr/adr-015-push-runner-takt-migration.mddocs/adr/adr-026-cargo-workspace.mddocs/adr/adr-044-subprocess-utility-extraction-boundary.mddocs/push-pipeline-fix-plan.mddocs/todo10.mdsrc/cli-push-pipeline/Cargo.tomlsrc/cli-push-pipeline/src/main.rssrc/lib-subprocess/src/lib.rs
💤 Files with no reviewable changes (3)
- src/cli-push-pipeline/Cargo.toml
- src/cli-push-pipeline/src/main.rs
- Cargo.toml
Comment on lines
+53
to
+57
| > ~~T2 (crate 削除) のみ大量削除になるため `PR_SIZE_CHECK_OVERRIDE=1` の使用を | ||
| > PR 説明に明記して bypass する。~~ → **この前提は誤りだった (2026-07-17、§5 T2 実施結果)**: | ||
| > T2 の実測は crate 削除分 329 行 / docs 追記込みの PR 全体 394 行 (= `pr_size_check` の | ||
| > 計測単位) で、warning 800 にも届かず override は不要だった。 | ||
| > gate の bypass は「計画にそう書いてあるから」ではなく、実測で必要になった時だけ使う。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
T2 の実施方針にも override 不要の前提を反映してください。
この訂正では実測により PR_SIZE_CHECK_OVERRIDE が不要だったとしていますが、同じ文書の T2 方針（Line 772-773）には依然として「override を使う」と残っています。次回の実施者が古い方針を採用しないよう、T2 の計画欄も「実測で必要な場合のみ使用」に更新してください。
🤖 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 `@docs/push-pipeline-fix-plan.md` around lines 53 - 57, Update the T2
implementation plan around the T2 execution guidance to remove the unconditional
instruction to use PR_SIZE_CHECK_OVERRIDE. State that the override should be
used only when measurement shows it is necessary, consistent with the corrected
T2 results and gate-bypass policy.
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)
Filtered (not applicable)(該当なし) 次のアクション
|
|
Tip For best results, initiate chat on the files or code changes.
残っているのは Critical / High / Major のブロッキング指摘がない点も確認しました。 |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)(該当なし) Applicable Findings (Medium 以下)
Filtered (not applicable)(該当なし) 次のアクション
|
ADR-015 で cli-push-runner に置換済みの旧実装が workspace member に残存し、 毎 push の clippy / test が dead crate をビルド・実行し続けていた。 crate ディレクトリごと削除し members から除去する (22 → 21 crate)。 dead code の根拠: path 依存 0 / pnpm scripts・build:all 0 / 配布 exe なし。 加えて main.rs が読む hooks-config.toml の [push_pipeline] は ADR-015 の 設定分離で削除済みのため、仮に実行しても動作しない状態だった。 計画が想定した PR_SIZE_CHECK_OVERRIDE は不使用 — push 時の pr_size stage 実測 396 行 (PR 全体) で warning 800 にも届かず、bypass する理由がなかった。 削除で stale になる参照を更新: - lib-subprocess の drain_pipe_capped doc (生きたコードの callsite 例) - ADR-044 の「5 callsite」/ docs/todo10.md の stress test 対象一覧 (後続セッションが存在しない crate を探すのを防ぐ) - ADR-008/009/010/012 は当時の設計記録のため残置 ADR-015 §廃止 に削除節を追記。ADR-026 の「削除は別 PR」を完了に更新し、 先送りから 3 か月分の毎 push コストを教訓として記録。 post-PR 修正 (CodeRabbit Minor 1 件を採用): override 不要の訂正を 4 箇所に 入れながら T2 の方針欄自体を見落としており、「block 閾値を超える」という誤った 前提が残っていた。方針の書き換えではなく打ち消し線 + 訂正注記で対応 — 本計画は「方針 → 実施結果 (逸脱の記録)」構造で、方針を遡って正しかったことに すると「計画時の見積もりが実測で覆った」学びが消えるため。 あわせて「PR 全体 394 行」が push 前の手計算値で実測 396 行と食い違っていたのを 発見し、push 時実測値に統一した (結論は不変)。 検証: cargo clippy --workspace warning 0 / cargo test --workspace 全 pass / cargo test -- --ignored 全 pass / lint:md 0 error。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aloekun
force-pushed
the
chore/remove-cli-push-pipeline
branch
from
July 17, 2026 11:18
a7bb3ff to
880a367
Compare
1 task
aloekun
added a commit
that referenced
this pull request
Jul 18, 2026
- todo-summary.md: 順位 323 (lib-subprocess timeout が wall-clock を縛れない) / 順位 324 (cli-pr-monitor push_to_remote の拒否検知欠落) / 順位 325 (push パイプライン per-run メトリクスの JSONL 永続化) を Tier 1 で追加 - todo13.md: 3 項目の詳細エントリを起票。計画ファイル (T99 で削除予定) に依存しないよう §6 backlog の分析要点を転記し、順位 323 には post-merge-feedback #286 の stale marker (2026-07-17 実観測、orphan report 手動復旧済) が同根の実害である旨を記録。 順位 325 は T12 後検証セッションで実測した可観測性ギャップ (stage ログ stderr のみ) の解消で、 ADR-057/058 判定期限 (2026-08-15) と T99 after 計測の前提データ基盤 - push-pipeline-fix-plan.md: §6 backlog 9/10 に todo 移管記録を追記 (§8 完了条件 2 の処置)
aloekun
added a commit
that referenced
this pull request
Jul 18, 2026
…290) * docs(todo): push パイプライン改善 T13 backlog 9/10 + メトリクス永続化を順位 323-325 に起票 - todo-summary.md: 順位 323 (lib-subprocess timeout が wall-clock を縛れない) / 順位 324 (cli-pr-monitor push_to_remote の拒否検知欠落) / 順位 325 (push パイプライン per-run メトリクスの JSONL 永続化) を Tier 1 で追加 - todo13.md: 3 項目の詳細エントリを起票。計画ファイル (T99 で削除予定) に依存しないよう §6 backlog の分析要点を転記し、順位 323 には post-merge-feedback #286 の stale marker (2026-07-17 実観測、orphan report 手動復旧済) が同根の実害である旨を記録。 順位 325 は T12 後検証セッションで実測した可観測性ギャップ (stage ログ stderr のみ) の解消で、 ADR-057/058 判定期限 (2026-08-15) と T99 after 計測の前提データ基盤 - push-pipeline-fix-plan.md: §6 backlog 9/10 に todo 移管記録を追記 (§8 完了条件 2 の処置) * fix(review): apply CodeRabbit fixes for #290 Resolved findings: - [Minor] docs/todo-summary.md:172 サマリーの更新日も更新してください。 - [Major] docs/todo13.md:1843 メトリクス収集の変更範囲を `log.rs` と各呼び出し元まで明記してください。
aloekun
added a commit
that referenced
this pull request
Aug 21, 2026
着手前に台帳と実装を突き合わせ、台帳の記述どおりであることを確認した (今回はずれなし)。 再現 (経過時間 assert 付きテストを先に作成、T6 = PR #283 の教訓): - run_cmd_shell_* の 3 variant とも timeout 1s に対し制御が戻るまで 9.59s (台帳の 9.23s と一致)。shell_command の child はシェルで、実際のコマンドは孫。 kill されるのはシェルだけなので、孫はパイプの書き込み端を握ったまま生き残り、 reader thread の join() が孫の自然終了までブロックしていた 対処 (ユーザー判断: (b) tree-kill + join 上限): - kill_process_tree を追加し、timeout / wait 失敗の経路で子孫ごと終了させる (Windows: taskkill /T /F、Unix: shell_command に process_group(0) を付けて pgid 宛に kill -9)。外部コマンド経由で libc 依存を増やさない (check-ci-coderabbit の kill_process_by_id と同じ方針) - join_within_grace で reader thread の回収に上限 (500ms) を設ける。tree kill が 失敗し得る以上、上限が無いと timeout の保証が「kill が成功すること」に依存した 条件付きのものになる (ADR-043) - 採らなかった案 (a) 失敗経路で detach は、制御は戻るが孫が孤児として走り続ける。 本 crate の callsite は cargo / jj のような重いコマンドを起動するため、孤児は #286 の orphan takt と同クラスの実害になる。理由を run_cmd_shell_with の doc に記録 別件 (調査中に実測で判明、ユーザー判断で同 PR に同梱): - drain_pipe_unlimited が read_to_string を使っており、出力が valid UTF-8 でないと 全出力を無言で捨てていた (read_to_string は Err 時に buf を元の長さへ戻す)。 実測: exit 0 の成功コマンドで unlimited=0 バイト / capped=444 バイト - 本 variant は「出力を control flow 判定に使う」callsite 専用で、push_was_refused (拒否を見逃し成功と誤報告)、レビュー用 diff (空 diff → レビュー skip)、 docs_only_routing / pr_size_check / ledger_completion / bookmark_check に及ぶ - read_to_end + from_utf8_lossy に変更。capped 系は元から lossy なので挙動も揃う 回帰テスト: - rank323_grandchild_outliving_the_shell 4 件 (3 variant の経過時間 + 正常系の対照) - orphan_tests 2 件 (孫が timeout 後に書き続けないこと + プローブ自体が空振りして いないことの対照) - non_utf8_tests 3 件 (不正 UTF-8 の周囲が残ること + capped との一致 + 正常系) 変異テストで判別力を確認 (両 OS): - tree-kill を外す → orphan_tests が FAILED (孤児 ping が 5 回とも完走) - from_utf8_lossy を read_to_string に戻す → non_utf8_tests 2 件が FAILED - 経過時間テストは join 上限だけでも通るため tree-kill を判別しない。これが orphan_tests を足した理由 (最初の版は変異で素通りした) テスト自身の空振りを 2 度踏んだ: - 非 UTF-8 をシェル経由で吐かせる版はクォートが崩れて不正バイトを 1 つも出して いなかった → Cursor で直接バイト列を流す形に変更 - 孤児プローブのマーカー読み取りが read_to_string で、本 PR が直したのと同じ罠 (ping の Shift-JIS 出力) を踏んで常に空だった → lossy 読みに変更 Linux 検証 (WSL Ubuntu-24.04): 42 件 green。変異テストも Linux で判別することを確認し、 process_group(0) + kill -9 -pgid が実際に効いていることを実測した 後始末: todo17.md 323 節 + todo-summary2.md 323 行を削除 検証: cargo test --workspace green / cargo clippy --workspace --all-targets green / pnpm lint:docs green / pnpm lint:md green CodeRabbit 指摘 3 件に対応 (PR #436): - Major: wait_with_timeout_safe の try_wait 失敗経路が child.kill() のみだった。 本 variant は diff stage が shell_command の child に使うため、この経路でも孫が 残り得る。両経路とも tree-kill するよう修正。wait_with_timeout_basic 側は 「cleanup は呼び出し側」が契約そのものなので変えず、シェル child を渡す唯一の 呼び出し元 (run_cmd_shell_with) が kill_and_join_err で孫まで殺していること、 他の呼び出し元が全て direct argv であることを確認して doc に記録 - Minor: 非 UTF-8 テストが contains() だけで、置換文字や後続出力を落とす実装でも 通っていた。期待値を完全一致 (assert_eq!) に変更。変異テストで判別を確認 (置換文字を落とす実装を入れると 2 件 FAILED) - Minor: マーカーパスの引用。Unix は '...' の '\'' 方式で escape。Windows は 引用できないことを実測で再確認 (`> "<path>"` にするとコマンドごと起動に失敗、 0.11s で ping が 1 度も走らない = Rust の引数エスケープが cmd.exe と非互換)。 代わりに前提検査を追加し、パスに空白/メタ文字があれば loud に落とす (黙って空振りするテストが本 PR で 2 度踏んだ失敗そのものなので) 再検証: Windows / 実 Linux (WSL Ubuntu-24.04) とも cargo test 42 件 green + clippy clean。cargo test --workspace green
aloekun
added a commit
that referenced
this pull request
Aug 21, 2026
着手前に台帳と実装を突き合わせ、台帳の記述どおりであることを確認した (今回はずれなし)。 再現 (経過時間 assert 付きテストを先に作成、T6 = PR #283 の教訓): - run_cmd_shell_* の 3 variant とも timeout 1s に対し制御が戻るまで 9.59s (台帳の 9.23s と一致)。shell_command の child はシェルで、実際のコマンドは孫。 kill されるのはシェルだけなので、孫はパイプの書き込み端を握ったまま生き残り、 reader thread の join() が孫の自然終了までブロックしていた 対処 (ユーザー判断: (b) tree-kill + join 上限): - kill_process_tree を追加し、timeout / wait 失敗の経路で子孫ごと終了させる (Windows: taskkill /T /F、Unix: shell_command に process_group(0) を付けて pgid 宛に kill -9)。外部コマンド経由で libc 依存を増やさない (check-ci-coderabbit の kill_process_by_id と同じ方針) - join_within_grace で reader thread の回収に上限 (500ms) を設ける。tree kill が 失敗し得る以上、上限が無いと timeout の保証が「kill が成功すること」に依存した 条件付きのものになる (ADR-043) - 採らなかった案 (a) 失敗経路で detach は、制御は戻るが孫が孤児として走り続ける。 本 crate の callsite は cargo / jj のような重いコマンドを起動するため、孤児は #286 の orphan takt と同クラスの実害になる。理由を run_cmd_shell_with の doc に記録 別件 (調査中に実測で判明、ユーザー判断で同 PR に同梱): - drain_pipe_unlimited が read_to_string を使っており、出力が valid UTF-8 でないと 全出力を無言で捨てていた (read_to_string は Err 時に buf を元の長さへ戻す)。 実測: exit 0 の成功コマンドで unlimited=0 バイト / capped=444 バイト - 本 variant は「出力を control flow 判定に使う」callsite 専用で、push_was_refused (拒否を見逃し成功と誤報告)、レビュー用 diff (空 diff → レビュー skip)、 docs_only_routing / pr_size_check / ledger_completion / bookmark_check に及ぶ - read_to_end + from_utf8_lossy に変更。capped 系は元から lossy なので挙動も揃う 回帰テスト: - rank323_grandchild_outliving_the_shell 4 件 (3 variant の経過時間 + 正常系の対照) - orphan_tests 2 件 (孫が timeout 後に書き続けないこと + プローブ自体が空振りして いないことの対照) - non_utf8_tests 3 件 (不正 UTF-8 の周囲が残ること + capped との一致 + 正常系) 変異テストで判別力を確認 (両 OS): - tree-kill を外す → orphan_tests が FAILED (孤児 ping が 5 回とも完走) - from_utf8_lossy を read_to_string に戻す → non_utf8_tests 2 件が FAILED - 経過時間テストは join 上限だけでも通るため tree-kill を判別しない。これが orphan_tests を足した理由 (最初の版は変異で素通りした) テスト自身の空振りを 2 度踏んだ: - 非 UTF-8 をシェル経由で吐かせる版はクォートが崩れて不正バイトを 1 つも出して いなかった → Cursor で直接バイト列を流す形に変更 - 孤児プローブのマーカー読み取りが read_to_string で、本 PR が直したのと同じ罠 (ping の Shift-JIS 出力) を踏んで常に空だった → lossy 読みに変更 Linux 検証 (WSL Ubuntu-24.04): 42 件 green。変異テストも Linux で判別することを確認し、 process_group(0) + kill -9 -pgid が実際に効いていることを実測した 後始末: todo17.md 323 節 + todo-summary2.md 323 行を削除 検証: cargo test --workspace green / cargo clippy --workspace --all-targets green / pnpm lint:docs green / pnpm lint:md green CodeRabbit 指摘 3 件に対応 (PR #436): - Major: wait_with_timeout_safe の try_wait 失敗経路が child.kill() のみだった。 本 variant は diff stage が shell_command の child に使うため、この経路でも孫が 残り得る。両経路とも tree-kill するよう修正。wait_with_timeout_basic 側は 「cleanup は呼び出し側」が契約そのものなので変えず、シェル child を渡す唯一の 呼び出し元 (run_cmd_shell_with) が kill_and_join_err で孫まで殺していること、 他の呼び出し元が全て direct argv であることを確認して doc に記録 - Minor: 非 UTF-8 テストが contains() だけで、置換文字や後続出力を落とす実装でも 通っていた。期待値を完全一致 (assert_eq!) に変更。変異テストで判別を確認 (置換文字を落とす実装を入れると 2 件 FAILED) - Minor: マーカーパスの引用。Unix は '...' の '\'' 方式で escape。Windows は 引用できないことを実測で再確認 (`> "<path>"` にするとコマンドごと起動に失敗、 0.11s で ping が 1 度も走らない = Rust の引数エスケープが cmd.exe と非互換)。 代わりに前提検査を追加し、パスに空白/メタ文字があれば loud に落とす (黙って空振りするテストが本 PR で 2 度踏んだ失敗そのものなので) 再検証: Windows / 実 Linux (WSL Ubuntu-24.04) とも cargo test 42 件 green + clippy clean。cargo test --workspace green
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
src/cli-push-pipeline/を削除し、Cargo workspace members から除去 (22 → 21 crate)cargo clippy --workspace/cargo testが dead crate をビルド・実行し続けていた無駄を解消lib-subprocessのdrain_pipe_cappeddoc (生きたコードの callsite 例)、ADR-044 の「5 callsite」、docs/todo10.mdの stress test transfer 候補一覧Context
docs/push-pipeline-fix-plan.mdの T2。ADR-015 (2026-04-14) でcli-push-pipeline.exeはcli-push-runner.exe(takt ベース) に置換されたが、crate は Cargo workspace の member として残存していた。ADR-026 (2026-04-17) が「dead code だが本 ADR の scope 外、削除は別 PR」と先送りした結果、3 か月にわたり毎 push の clippy / test が dead crate を対象にし続けていた。dead code の根拠は想定より強かった: 計画は「他 crate から参照が無いこと」の確認を求めていたが、実際には path 依存 0 / pnpm scripts・
build:all0 / 配布 exe なしに加え、main.rsが読むhooks-config.tomlの[push_pipeline]セクションは ADR-015 の設定分離ですでに削除済み = 仮に実行しても動作しない状態だった。⚠ 計画の「大量削除」前提は誤りで
PR_SIZE_CHECK_OVERRIDEは不使用: 計画 §2 原則 1 は「T2 は block 閾値を超えるため override で bypass する」としていたが、実測は PR 全体 408 行 (初回 push 時点 396 行) で warning 800 にも届かなかった (pr_sizestage で確認)。gate の bypass は計画の記述ではなく実測で必要になった時だけ使う (常態化は ADR-043 fail-closed の空洞化)。計画側の該当記述も本 PR で修正した。Scope 判断: ADR-008 / 009 / 010 / 012 は当時の設計判断の記録であり、置換と削除の経緯は ADR-015 が持つため書き換えない。生きたコードの参照と、後続セッションが存在しない crate を探す原因になる未実施タスクの crate 一覧のみ更新した。
Validation
cargo clippy --workspace --all-targets --all-features -- -D warnings: PASS (warning 0)cargo test --workspace: 全 crate pass (削除前 1568 passed → 削除後 1563 passed。削除 crate の#[test]5 本分で総数 -5)main.rsL208 以降の#[cfg(test)]に 5 本存在)。push 時の pre-push review (simplicity) が検出し refute の verify でも SURVIVE (棄却されず確定)。実測 1563 passed + 削除分 5 = 1568 を再確認して訂正したcargo test -- --ignored --test-threads=1: 全 passpnpm lint:md: 98 file / 0 errorcargo metadata --no-depsで 22 → 21 を前後計測。grep cli-push-pipeline Cargo.lockも 0 件pnpm pushpre-push review (最新 = 本 PR head、workflow=pre-push-review-refute):simplicity=needs_fix → fix →
convergence_verdict: fully_resolved(3 iterations / 8m24s)、security=approved。simplicity が上記「テスト 0 本」の虚偽記述を検出し、refute の verify も SURVIVE (棄却されず確定) → fix が実測値へ訂正
= 同じ誤りを初回レビューは見逃していた
変更は docs のみだが、fix 後の状態で
cargo test --workspace= 1563 passed を別途実測して確認したReferences
Summary by CodeRabbit
変更
ドキュメント