From 10286f784d7190b3c54c68c155cd0e910511a0c1 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 15 Jun 2026 14:20:59 +0900 Subject: [PATCH 1/2] =?UTF-8?q?docs(adr):=20ADR-044=20subprocess=20utility?= =?UTF-8?q?=20extraction=20=E3=81=AE=E5=A2=83=E7=95=8C=E5=88=A4=E5=AE=9A?= =?UTF-8?q?=20(=E9=A0=86=E4=BD=8D=20173e=20=E8=A9=95=E4=BE=A1=E7=B5=90?= =?UTF-8?q?=E6=9E=9C)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAUDE.md | 1 + ...-subprocess-utility-extraction-boundary.md | 148 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 docs/adr/adr-044-subprocess-utility-extraction-boundary.md diff --git a/CLAUDE.md b/CLAUDE.md index 8614e953..c39387b0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,6 +44,7 @@ - [ADR-041: Test Isolation Patterns for Multi-Condition Guards](docs/adr/adr-041-test-isolation-patterns.md) *(試験運用)* - [ADR-042: ルール vs 仕組み化の境界基準](docs/adr/adr-042-rule-vs-mechanism-boundary.md) *(試験運用)* - [ADR-043: Security/Quality Gate での Fail-Closed 原則](docs/adr/adr-043-security-gates-fail-closed.md) *(試験運用)* +- [ADR-044: subprocess utility extraction の境界判定 — 共通化と分離の線引き](docs/adr/adr-044-subprocess-utility-extraction-boundary.md) *(試験運用)* ## Build diff --git a/docs/adr/adr-044-subprocess-utility-extraction-boundary.md b/docs/adr/adr-044-subprocess-utility-extraction-boundary.md new file mode 100644 index 00000000..abd1a096 --- /dev/null +++ b/docs/adr/adr-044-subprocess-utility-extraction-boundary.md @@ -0,0 +1,148 @@ +# ADR-044: subprocess utility extraction の境界判定 — 共通化と分離の線引き + +## ステータス + +試験運用 (2026-06-15) + +> ADR-039 (Experimental feature 標準パターン) に準拠: config opt-in なし (本 ADR は decision criteria であり実装機構ではないため該当しない) / kill-switch = 本 ADR を supersede する後続 ADR で停止可能 / bounded lifetime = 採用判定 6 ヶ月 (2026-12-15) を目安に dogfood 結果から本採用 / 修正 / 却下を判定。 + +## コンテキスト + +### 問題 + +順位 173 (subprocess utils 5 crate 重複を `lib-subprocess` に extract) を 173a-e の 5 sub-PR に分割して実施した結果、`lib-subprocess` には複数 variant の subprocess utility が export された (PR #205 / #206 / #207 / #208 で land)。 + +最終 sub-PR 173e で「variant 共存維持 vs merge」を判断する局面で、**「ここまでは共通化、ここから先は共通化しない」という境界の判断根拠を ADR に残しておくべき** という指摘を受けた。理由は以下: + +1. 将来同様の utility 重複が観測されたとき、「lib-subprocess に取り込むべきか / 各 crate に残すべきか」を再判定する判断材料が必要 +2. 同様に「複数 variant を維持すべきか / 1 つに merge すべきか」を再判定する判断材料が必要 +3. ADR 化しないと git 履歴と doc comments の散在状態となり、再判定時に判断根拠の再構築が必要になる + +### 順位 173 の構造的成果 + +PR #205 (173a) - #208 (173d) で以下を `lib-subprocess` に集約: + +| Function | Variants | 用途別の使い分け | +|---|---|---| +| `combine_output` | 1 (単一) | stdout / stderr 結合 (`\n` suffix 吸収) | +| `wait_with_timeout` | `_safe` / `_basic` | Err 経路で child を kill するか / しないか | +| `drain_pipe` | `_unlimited` / `_capped` / `_capped_reporting` | 全量 / silent truncate / truncate + 末尾報告 | +| `run_cmd_shell` | `_capped` / `_capped_reporting` | drain variant 違い (上記に従う) | +| `kill_and_join_err` | 1 (内部 helper) | Err 経路で child kill + reader thread join (PR #208 CR Major 対応) | + +5 callsite (cli-push-runner / cli-push-pipeline / cli-merge-pipeline / hooks-stop-quality / hooks-post-tool-linter) が `lib_subprocess::*` 経由で utility を共有。 + +### 173e で判定した境界 + +**共通化したもの** (= lib-subprocess に extract): +- 上記 5 関数群。各 variant は明示的な policy 違いで使い分けられており、各 callsite が「どの variant を呼ぶか」で意図を表明している + +**共通化しなかったもの** (= 各 crate に残置): +- `cli-pr-monitor/src/runner.rs::run_cmd_direct`: direct args (shell なし) + `drain_pipe_unlimited` + 独自 polling。Windows shell escape を避ける設計意図が `lib-subprocess` の `cmd /c` 経由 shell pattern と signature レベルで非互換 +- `cli-pr-monitor/src/runner.rs::run_cmd_inherit` / `cli-push-runner/src/runner.rs::run_cmd_inherit`: stdio inherit + timeout 有無の policy 違い (cli-pr-monitor は timeout あり、cli-push-runner は timeout なし)。同じ名前の 2 実装が意図的に異なる semantics を持つ + +**variant merge しなかったもの** (= lib-subprocess 内に複数 variant 維持): +- `wait_with_timeout_safe` / `_basic`: spec が明示した merge trigger (`.takt/runs/` に `zombie` / `defunct` / `Failed to wait` 発生記録) が dogfood で未顕在化 (2026-06-15 時点で `grep -rn` 結果 0 件) +- `drain_pipe_capped` / `_capped_reporting`: bool flag で merge 可能だが、`run_cmd_shell_capped_reporting` 等の callsite 名で variant 意図が直接読めるため self-documenting 価値が bool flag の認知コストを上回る +- `run_cmd_shell_capped` / `_capped_reporting`: 同上 + +## 検討した選択肢 + +### 選択肢 A: 順位 173 完了として境界判断を記録しない + +- todo11.md / git log / lib-subprocess の doc comments に部分的に残るが、再判定時に判断根拠の再構築コストが高い +- 将来「同じような重複あるけど共通化する? しない?」の判断が出たとき、過去判例として参照可能な single source がない +- **却下** + +### 選択肢 B: 新規 ADR で「境界判定」を独立 codify (採用) + +- 順位 173 で形成された具体的な境界 = 「共通化した境界」「共通化しなかった境界」「variant merge しなかった境界」の 3 軸を ADR として記録 +- ADR-024 (`lib-jj-helpers`) と同じく「共通 utility library の boundary 判定」の事例として参照可能 +- 将来の re-evaluation 時に「以下の条件が変わったら見直す」という trigger 条件も併記 +- **採用** + +### 選択肢 C: ADR-024 (shared jj-helpers library) に拡張 + +- ADR-024 は jj 専用ヘルパーの boundary 判定で、subprocess utility は scope が異なる +- jj helpers と subprocess utility は責務が直交しており、同 ADR に詰め込むと両方の主旨が霞む +- **却下** + +## 決定 (試験運用) + +### 採用する 3 層の境界記録 + +#### 層 1: extract 対象の境界 (`lib-subprocess` に入れるか / crate 個別に残すか) + +| 条件 | 判定 | +|---|---| +| **3+ crate で重複** かつ **signature が同じか同型化可能** | ✅ extract | +| **signature が構造的に異なる** (例: shell vs direct args、stdio inherit vs piped) | ❌ 各 crate に残置、別 ADR で extract 検討 | +| **2 crate で重複** だが variant policy が違う | 🤔 variant として export を検討、要 dogfood | +| **1 crate でしか使われていない** | ❌ extract せず、将来 2 つ目の使用例待ち (ADR-024 § 早期の共通化はリスク に従う) | + +#### 層 2: variant 維持の境界 (1 関数 vs 複数 variant) + +| 条件 | 判定 | +|---|---| +| **callsite ごとに intent が異なる** (例: 全量読み / truncate / truncate + 報告) | ✅ variant 維持、callsite が variant 名で intent 表明 | +| **policy 差が runtime 値で表現可能** (例: timeout secs、max lines) | ✅ parameter として統一 | +| **policy 差が control flow に影響** (例: Err 経路で kill する / しない) | 🤔 variant 維持、dogfood で merge trigger を待つ | +| **bool flag で merge 可能** だが flag 意味が callsite で自明でない | ❌ variant 維持、self-documenting variant 名を優先 | + +#### 層 3: variant merge の境界 (再評価 trigger) + +以下のいずれかが発生したら variant merge を再評価: + +1. **dogfood で `_basic` 側が想定外の挙動** (例: zombie process / defunct child / "Failed to wait" ログ) — `.takt/runs/` に grep して証拠を確認 +2. **callsite で「どの variant を呼ぶか」の判断ミス** が CR Major / pre-push review で複数 PR 連続観測 (Frequency Medium 以上) +3. **新規 callsite が増えて variant 名の選択が機械的でなくなった** + +### 順位 173 の判定実績 + +| Item | 判定 | 根拠 | +|---|---|---| +| `combine_output` (5 crate 重複) | ✅ 共通化 | 3+ crate 重複 + signature 同一 (層 1) | +| `wait_with_timeout_safe` vs `_basic` | ✅ 2 variant 維持 | Err 経路で kill する / しないが control flow に影響 (層 2)、dogfood 未顕在 (層 3) | +| `drain_pipe_unlimited` / `_capped` / `_capped_reporting` | ✅ 3 variant 維持 | 読み戦略が構造的に異なる (`read_to_string` vs line-by-line)、callsite intent 異なる (層 2) | +| `run_cmd_shell_capped` vs `_capped_reporting` | ✅ 2 variant 維持 | drain variant 違い (層 2)、callsite 名で intent 表明 (層 2) | +| `run_cmd_direct` (cli-pr-monitor) | ❌ 各 crate 残置 | shell vs direct args で signature 非互換 (層 1) | +| `run_cmd_inherit` (2 crate) | ❌ 各 crate 残置 | timeout 有無の policy 差が意図的、signature 異なる (層 1) | + +## 影響 + +### 良い影響 + +- 将来の utility extract 候補が出た際、3 層の境界基準で機械的に判定可能 +- variant merge を回避した判断の根拠が文書化され、後続 PR で「merge した方が綺麗では?」と提案された際の reject 根拠を即座に提示できる +- ADR-024 (`lib-jj-helpers`) との対比で「subprocess utility は variant を許容、jj helpers は単一 API で統一」という domain 差を示せる + +### 注意点 + +- 境界基準は **dogfood 観測前提**。新規 callsite 追加で境界が変わる可能性があり、本 ADR は静的なルールではなく **再評価可能な judgment criteria** として運用する +- 「self-documenting variant 名 vs bool flag」の trade-off は callsite 数が増えると逆転する可能性 (例: variant が 5+ 個になったら bool/enum flag に移行する方が読みやすい) + +## 再評価 trigger + +以下のいずれかが発生したら本 ADR を見直す: + +1. 新規 subprocess utility extract 候補が出て、3 層の境界基準で判定不能な edge case が発生 +2. `_basic` variant の dogfood で zombie process / "Failed to wait" 等が `.takt/runs/` に観測される +3. variant 数が 5+ に増えて self-documenting naming が破綻 (lib-subprocess の export 関数一覧で迷う状態) +4. 採用判定期限 (2026-12-15) で本採用 / 修正 / 却下を判定 + +## 関連 ADR + +- ADR-012: src/ ディレクトリの命名規約 — `lib-*` naming +- ADR-024: 共通 jj ヘルパーライブラリ — 同型の boundary 判定事例 +- ADR-026: Cargo workspace — 本 ADR の前提構造 +- ADR-039: Experimental feature 標準パターン — 本 ADR のステータス管理形式 +- ADR-042: ルール vs 仕組み化の境界基準 — 本 ADR は subprocess utility 領域での具体適用 + +## 由来 + +- 順位 173 (PR #205-#208) の作業完了後、ユーザー指示 (2026-06-15) で「ここまでは共通化、ここから先は共通化しない判断を ADR に残す」要望から起案 +- 173a (PR #205): `combine_output` extract +- 173b (PR #206): `wait_with_timeout_safe` / `_basic` extract +- 173c (PR #207): `drain_pipe_unlimited` / `_capped` / `_capped_reporting` extract +- 173d (PR #208): `run_cmd_shell_capped` / `_capped_reporting` extract + `kill_and_join_err` helper (CR Major 対応) +- 173e: 評価のみ実施、本 ADR で結果を codify From 88fa24637601924dbb61dfa08bc6c0ca9054bc24 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 15 Jun 2026 14:20:59 +0900 Subject: [PATCH 2/2] =?UTF-8?q?docs(todo):=20=E9=A0=86=E4=BD=8D=20173=20(s?= =?UTF-8?q?ubprocess=20utils=20extraction)=20=E5=AE=8C=E4=BA=86=E3=81=AB?= =?UTF-8?q?=E4=BC=B4=E3=81=84=E5=89=8A=E9=99=A4=20(ADR-044=20=E3=81=A7?= =?UTF-8?q?=E8=A9=95=E4=BE=A1=E7=B5=90=E6=9E=9C=20codify=20=E6=B8=88)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/todo-summary.md | 1 - docs/todo11.md | 123 ------------------------------------------- 2 files changed, 124 deletions(-) diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 2c4f9e46..fe9844c8 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -67,7 +67,6 @@ | 170 | 💎 Tier 3 | **`git-workflow.md § Multi-PR chaining` を「1 PR 内 multi-commit + intent 明記」パターンに拡張 (PR #183 T3-#1 採用)** | todo11.md | S | なし (PR #119/#120/#121 + #183 の 4 観測で Frequency High、commit 分割判断 + intent 記述ガイドを既存 section に追記、`~/.claude/rules/common/git-workflow.md` 編集、派生プロジェクトへ自動波及、`feedback_global_config_backup` 適用必須) | | 171 | 💎 Tier 3 | **`docs-governance.md` に「Operational reference vs Pointer reference」区別 section を追加 (PR #183 T3-#2 採用) ★ Bundle DG-RULES** | todo11.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** | todo11.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 | **subprocess utils 5 crate 重複を `lib-subprocess` に extract — 173a-e に sub-PR 分割 (PR #182 dry-run S01 + Phase E dogfood WR-2026-06-01-S01 採用、2026-06-14 挙動保存型 sub-PR 分割)** | todo11.md | 173a: S / 173b-d: S-M each | 173a 単独着手可、173b/c/d は 173a 依存、173e (variant merge) は 173a-d 完了後 dogfood で判断。`src/cli-pr-monitor/src/runner.rs:80-89` の `combine_output` (5 crate 重複) + `drain_pipe` / `wait_with_timeout` / `run_cmd` の挙動 variant 統合、ADR-026 Cargo workspace + ADR-012 lib-* naming で解決。挙動保存型分割により各 sub-PR が pure refactor として独立 land 可能 | | 176 | 🔧 Tier 2 | **check-ci-coderabbit format extraction 関数への variant fixture 追加 (PR #185 T2-#4 採用)** | todo12.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) | | 178 | 🔧 Tier 2 | **`state.rs` の behavioral invariant test を ADR-041 pattern で追加 (週次レビュー 2026-05-30 S02 採用)** | todo12.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) | | 179 | 🔧 Tier 2 | **rate-limit retry decision boundary test を rstest parameterized で追加 (週次レビュー 2026-05-30 S03 採用)** | todo12.md | S | なし (Phase D dogfood で発見、`src/cli-pr-monitor/src/config.rs:94-122` + `stages/poll.rs` の `max_retries=3` 固定 test のみで boundary (0/1/3/off-by-one) 未検証、rstest parameterized で 3-4 case 追加 ~15 行、rstest 既存使用 + Bundle CR-RL = 順位 167-169 隣接領域 follow-up、off-by-one regression が test で検出可能化) | diff --git a/docs/todo11.md b/docs/todo11.md index 141110b9..aad8f903 100644 --- a/docs/todo11.md +++ b/docs/todo11.md @@ -399,129 +399,6 @@ --- -### subprocess utils 5 crate 重複を `lib-subprocess` に extract (PR #182 dry-run S01 + Phase E dogfood WR-2026-06-01-S01 採用、2026-06-14 挙動保存型 sub-PR 分割) - -> **動機**: PR #182 Phase B dry-run で検出された finding WR-2026-05-29-S01 に加え、**Phase E dogfood (2026-06-01) で WR-2026-06-01-S01 (High) として scope が拡大**: subprocess 管理 utility 群が 4-5 crate 横断で重複している。 -> -> **重複対象**: -> - `combine_output(stdout, stderr)` 8 行関数: 5 crate (`cli-pr-monitor` / `cli-push-runner` / `cli-push-pipeline` / `cli-merge-pipeline` / `hooks-post-tool-linter`) で重複 (cli-pr-monitor では `#[allow(dead_code)]` 付与で生産 path 未到達) -> - `drain_pipe` / `wait_with_timeout` / `run_cmd`: 4-5 crate (`cli-push-runner/src/runner.rs` / `cli-pr-monitor/src/runner.rs` / `cli-merge-pipeline/src/main.rs` / `cli-push-pipeline/src/main.rs` / `hooks-stop-quality/src/main.rs`) で重複かつ挙動 variant あり -> - `MAX_LINES` 定数: 4 crate 間で不整合 (20 / 40 / 200 / 無制限) — callsite 設定可能パラメータとして公開する -> -> ADR-026 Cargo workspace + ADR-012 lib-* naming の既存パターンで解決コスト低、保守リスクが各 PR で蓄積中。 -> -> **本タスクの位置づけ**: PR #182 dry-run S01 採用 (2026-05-29 ユーザー承認) + Phase E dogfood (2026-06-01) WR-2026-06-01-S01 (Severity High) で augment 採用。ADR-031 § Phase 4 「重複検出は MVP では実装しない」運用の partial overlap 検出 → augment 判断のフローが機能した実例 (skill 重複検出 → 3 択 → user augment 選択)。 -> -> **参照**: PR #182 dry-run report、Phase E dogfood report (`.claude/weekly-reviews/2026-06-01.md` および `.takt/runs/20260601-095710-weekly-review-2026-06-01/reports/`)、`src/cli-pr-monitor/src/runner.rs:80-89` (function) + `:282-298` (tests)、`src/cli-push-runner/src/runner.rs` / `src/cli-merge-pipeline/src/main.rs` / `src/cli-push-pipeline/src/main.rs` (drain_pipe 等の重複)、`src/hooks-post-tool-linter/` (combine_output の `\n` suffix 吸収版実装)、ADR-024 (shared jj-helpers library パターン)、ADR-026 (Cargo workspace) -> -> **実行優先度**: 🔧 **Tier 2** → 🚀 **Tier 1 検討余地** — Effort S-M (sub-PR 分割で各 ~150-250 行)。Phase E dogfood で High severity 再確認、4-5 crate 横断で更新コスト線形成長中。 - -#### 挙動保存型 sub-PR 分割方針 (2026-06-14 ユーザー合意) - -scope 確認の結果、`drain_pipe` / `wait_with_timeout` / `run_cmd` は **crate ごとに挙動 variant が存在** することが判明。1 PR で全部やると挙動統一の意思決定が混入して PR が肥大化 + リスク増大するため、以下原則で分割: - -- **原則**: 挙動保存。`lib-subprocess` に複数 variant 関数を export し、各 callsite はこれまでと同じ挙動の variant を呼ぶだけにする -- **variant merge 検討は最終 sub-PR (173e) に分離**: 「実装上ほぼ同じ variant を 1 つに統合できるか」は dogfood 後判断 -- ADR-024 (`lib-jj-helpers`) の異なる semantics の helper を別関数として export する pattern に倣う - -#### sub-PR 一覧 - -| sub | 対象 | 推定 diff | リスク | 依存 | -|---|---|---|---|---| -| **173a** | `combine_output` 抽出 + `lib-subprocess` crate 新設 | ~150 行 | 低 (pure 関数、`hooks-post-tool-linter` の `\n` suffix 吸収版に統一しても既存 test 全 13 件 pass) | なし | -| 173b | `wait_with_timeout` 2 variant 抽出 | ~150 行 | 中 (subprocess lifecycle、integration test 望ましい) | 173a | -| 173c | `drain_pipe` 3 variant 抽出 | ~200 行 | 中 (API 形状変更、MAX_LINES の callsite parameter 化) | 173a | -| 173d | `run_cmd` variant 抽出 | ~250 行 | 中-高 (173a/b/c に依存) | 173a/b/c | -| 173e (optional) | variant merge 検討 (dogfood 後判断) | 〜 | 〜 | 173a-d 完了後 | - -##### 173a: `combine_output` 抽出 (本 PR scope) ✅ 実装完了 (2026-06-14) - -- **採用 variant**: `hooks-post-tool-linter` の `\n` suffix 吸収版 (4 crate の basic 版が既存 test で全 case pass するため後方互換) -- **抽出先**: `src/lib-subprocess/` 新規 crate (`lib-subprocess` 名称、ADR-026 workspace + ADR-012 lib-* naming に整合、既存 lib-jj-helpers / lib-ollama-client / lib-pending-file / lib-report-formatter のいずれも subprocess utility の自然な home ではないため新規) -- **作業計画**: - - [x] `src/lib-subprocess/Cargo.toml` + `src/lib-subprocess/src/lib.rs` 作成 (`combine_output(stdout: &str, stderr: &str) -> String` pub 関数 + 5 variant test) - - [x] ルート `Cargo.toml` workspace members に `src/lib-subprocess` 追加 - - [x] 4 crate (cli-push-runner / cli-push-pipeline / cli-merge-pipeline / hooks-post-tool-linter) の `Cargo.toml` に `lib-subprocess = { path = "../lib-subprocess" }` dep 追加。cli-pr-monitor は dead-code (`#[allow(dead_code)]`) のため関数 + test 削除のみで dep 不要 - - [x] 各 crate の `combine_output` impl + duplicate test 削除、`use lib_subprocess::combine_output;` に置換 - - [x] `cargo test --workspace` で全 pass 確認 (lib-subprocess 5 test + 5 crate 含む workspace 全体 pass) - - [x] `cargo clippy --workspace -- -D warnings` production code clean (cli-pr-monitor の `#[allow(dead_code)]` 削除確認)。NOTE: `--all-targets` 付与時に `cli-merge-pipeline/src/feedback.rs` に PR #159 (Bundle l) 以来の事前 clippy 違反 2 件 (`items-after-test-module` / `assertions-on-constants`) が残存。本 PR の編集対象外のため別 entry として follow-up 候補 - - [x] 173a 完了マーク (本 sub section)、173b-e は本 entry 内に未着手として残置 - -##### 173b: `wait_with_timeout` 2 variant 抽出 ✅ 実装完了 (2026-06-14) - -- **variant A `wait_with_timeout_safe`**: Err 経路で `child.kill()` + `child.wait()` 実施 — cli-pr-monitor (`runner.rs` 内 def → `stages/push_jj_bookmark.rs` で 1 callsite) で使用 -- **variant B `wait_with_timeout_basic`**: Err 経路でそのまま `Err` 返却 — cli-push-runner (`runner.rs` 内部 1 callsite + `stages/` 5 callsite = 計 6 callsite) で使用 -- variant 統一は本 sub では行わない (173e 判断) -- **scope 調整 (2026-06-14)**: 当初 plan で「cli-pr-monitor `classifier_runner.rs` (2 callsites)」が `_basic` を共用すると想定していたが、実コード確認の結果 channel-based + Child move + `wait_with_output()` 同時 drain の **completely 異なる設計** (signature: `fn(Child, Duration) -> Option`、pipe buffer overflow 対策の意図的構造) であることが判明。signature 互換性なし + 設計意図が独立のため本 sub では touch せず、必要なら 173e で別 variant として再評価 -- **作業計画**: - - [x] `lib-subprocess/src/lib.rs` に `wait_with_timeout_safe` / `wait_with_timeout_basic` 2 関数 + 各 variant 2 test (success/timeout) 追加 - - [x] cli-pr-monitor `Cargo.toml` に `lib-subprocess` dep 復活 (173a で removed) - - [x] cli-pr-monitor `runner.rs` の `wait_with_timeout` impl + `use ExitStatus` 削除 - - [x] cli-pr-monitor `stages/push_jj_bookmark.rs:172` callsite を `lib_subprocess::wait_with_timeout_safe` に置換 - - [x] cli-push-runner `runner.rs` の `wait_with_timeout` impl + 1 test (`wait_with_timeout_returns_exit_status_directly`) + `use ExitStatus/Instant` 削除 - - [x] cli-push-runner `runner.rs` 内 1 callsite + `stages/{scratch_file_warning, push_jj_bookmark, pr_size_check, lint_screen, bookmark_check}.rs` 5 callsite を `lib_subprocess::wait_with_timeout_basic` に置換 - - [x] `cargo test --workspace` で全 pass 確認 (lib-subprocess 9 test + workspace 全体 pass) - - [x] `cargo clippy --workspace -- -D warnings` clean - - [x] 173b 完了マーク (本 sub section)、173c/d/e は本 entry 内に未着手として残置 - -##### 173c: `drain_pipe` 3 variant 抽出 ✅ 実装完了 (2026-06-14、CR Major fix 含む) - -- **variant A `drain_pipe_unlimited`**: `read_to_string` 全読み — cli-pr-monitor (`runner.rs` 内部 2 callsites + `stages/push_jj_bookmark.rs` 2 callsites = 計 4 callsites) -- **variant B `drain_pipe_capped(pipe, max_lines)`**: `read_until` + silent truncate — cli-push-runner (`runner.rs` 2 callsites + `stages/{scratch_file_warning, push_jj_bookmark, pr_size_check, lint_screen, bookmark_check}.rs` 10 callsites = 計 12 callsites)、cli-push-pipeline (2 callsites)、hooks-stop-quality (2 callsites) -- **variant C `drain_pipe_capped_reporting(pipe, max_lines)`**: variant B + `"... (N lines truncated)"` 末尾報告 — cli-merge-pipeline (2 callsites、MAX_LINES=200) -- 計 22 callsites を 5 crate 横断で migration、`MAX_LINES` 定数は各 crate に保持して callsite で parameter として渡す形に変更 (挙動保存、crate 別の設計意図 = ログ用なら 40、メモリ保護なら 200、stop hook なら 20 を維持) -- **作業計画**: - - [x] `lib-subprocess/src/lib.rs` に 3 variant + 6 test (各 variant の代表 case: 短/長/over-cap/under-cap/reporting on/off) 追加 - - [x] cli-pr-monitor `runner.rs` の `drain_pipe` impl + NOTE コメント削除、4 callsite を `lib_subprocess::drain_pipe_unlimited` に置換 - - [x] cli-push-runner `runner.rs` の `drain_pipe` impl 削除、`MAX_LINES` を `pub(crate) const` 化して stage から参照可能に、12 callsite を `drain_pipe_capped(pipe, MAX_LINES)` に置換 - - [x] cli-push-pipeline `main.rs` の `drain_pipe` impl 削除、2 callsite を `drain_pipe_capped(pipe, MAX_LINES)` に置換 (lib-subprocess は 173a で dep 追加済) - - [x] hooks-stop-quality `Cargo.toml` に `lib-subprocess` dep 追加、`drain_pipe` impl 削除、2 callsite を `drain_pipe_capped(pipe, MAX_LINES)` に置換 - - [x] cli-merge-pipeline `main.rs` の `drain_pipe` impl 削除、2 callsite を `drain_pipe_capped_reporting(pipe, MAX_LINES)` に置換 (lib-subprocess は 173a で dep 追加済) - - [x] `cargo test --workspace` で全 pass 確認 (lib-subprocess 15 test + 6 影響 crate 含む workspace 全体 pass) - - [x] `cargo clippy --workspace -- -D warnings` clean - - [x] 173c 完了マーク (本 sub section)、173d/e は本 entry 内に未着手として残置 - -##### 173d: `run_cmd_shell` 2 variant 抽出 ✅ 実装完了 (2026-06-14) - -- **variant A `run_cmd_shell_capped`**: `cmd /c` shell + `drain_pipe_capped` + `wait_with_timeout_basic` + `combine_output` を内部組み立て、silent truncate semantics — cli-push-runner (`runner.rs` 内部経由 + `stages/quality_gate.rs` 2 callsites)、cli-push-pipeline (2 callsites)、hooks-stop-quality (1 callsite) = 計 6 callsites -- **variant B `run_cmd_shell_capped_reporting`**: 上記 + `drain_pipe_capped_reporting` で truncation 行数を末尾報告 — cli-merge-pipeline (4 callsites) -- 計 10 callsites を 4 crate 横断で migration。`MAX_LINES` 定数は各 crate に保持 (push-runner/pipeline=40、merge-pipeline=200、stop-quality=20) -- **scope 調整 (2026-06-14)**: - - `cli-pr-monitor/runner.rs` の `run_cmd_direct` (direct args + drain_pipe_unlimited + wait_with_timeout_safe variant) は signature と設計意図 (Windows shell escape を避けるため direct args) が異なるため本 sub では touch せず、173e で評価する - - `run_cmd_inherit` 系 (cli-pr-monitor + cli-push-runner、stdio inherit) も touch せず、173e で評価 -- **挙動の minor 変更** (PR description で明示): - - hooks-stop-quality の timeout message format: 旧 `"{cmd} timed out after Ns"` → 新 `"timed out after Ns"` (cmd prefix 削除) - - hooks-stop-quality の inline combine: 旧 `"out\nerr"` 結合 → 新 `combine_output` (`\n` suffix 吸収版、production callsite で挙動差は顕在化しない) - - cli-push-pipeline / cli-merge-pipeline / hooks-stop-quality の wait: 旧 inline loop → 新 `wait_with_timeout_basic` (try_wait Err 経路で child を kill しない basic semantics、production で try_wait Err は unreachable のため挙動変化なし) -- **作業計画**: - - [x] `lib-subprocess/src/lib.rs` に `run_cmd_shell_capped` / `_reporting` 2 関数 + 6 test (success / timeout / nonzero exit / 各 variant) 追加 - - [x] cli-push-runner: `runner.rs` 内 `run_cmd` impl 削除 (`run_stage_cmd` のみ残し lib 経由に置換)、`stages/quality_gate.rs` 2 callsite を `run_cmd_shell_capped` に - - [x] cli-push-pipeline: `run_cmd` impl 削除、2 callsite を `run_cmd_shell_capped` に、不要 import (Command, Duration) 削除、test 4 件 (combine_output 系、lib に存在) 削除 - - [x] cli-merge-pipeline: `run_cmd` impl 削除、4 callsite を `run_cmd_shell_capped_reporting` に、`combine_output` は他箇所 (`gh` output 結合) で利用継続のため import 保持 - - [x] hooks-stop-quality: `run_step` impl 削除、1 callsite を `run_cmd_shell_capped` に、不要 import (Command, Duration) 削除 - - [x] `cargo test --workspace` で全 pass 確認 (lib-subprocess 21 test + workspace 全体 pass) - - [x] `cargo clippy --workspace -- -D warnings` clean - - [x] 173d 完了マーク (本 sub section)、173e は本 entry 内に未着手として残置 - -##### 173e (optional): variant merge 検討 - -- 173a-d 完了後の dogfood 観測で「variant 共存に意味があるか/merge 可能か」を判断 -- 例: `wait_with_timeout_safe` と `_basic` の 2 variant は「Err 経路で child を kill すべきか」の policy 違い。dogfood で `_basic` 側で zombie process 顕在化したら `_safe` に統合 -- 判断時点まで延期 (= 現時点で merge 計画を立てない) - -#### 完了基準 (本 entry 全体) - -- 173a-d 全 sub-PR が land し、subprocess utility の単一 source of truth が `lib-subprocess` に確立 -- 5 cli-*/hooks-* crate が `lib_subprocess::*` 経由で utility を共有 -- `#[allow(dead_code)]` 付与が不要になる -- cargo workspace 全体で cargo test + cargo clippy が pass - -#### 詰まっている箇所 - -なし。挙動保存型分割で各 sub-PR が独立着手可能。173a 着手中。 - ---- - ## 既知課題 (記録のみ、本セッションで未対応) (現時点で本ファイルへの既知課題は無し。docs/todo9.md 末尾を参照。)