Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
148 changes: 148 additions & 0 deletions docs/adr/adr-044-subprocess-utility-extraction-boundary.md
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 で検出可能化) |
Expand Down
Loading