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
27 changes: 27 additions & 0 deletions docs/adr/adr-044-subprocess-utility-extraction-boundary.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,33 @@ T5 の由来は本 ADR にとって示唆的である: `run_cmd_shell_capped`
**doc の契約だけでは守られず、正しい variant が存在しないと callsite は間違った variant を選ぶ**。
層 2 の「callsite が variant 名で intent 表明」は、選択肢が揃っていて初めて機能する。

### 後続の判定 (2026-07-17: diff stage の timeout / T6) — variant を**追加しなかった**事例

push パイプライン改善 T6 (diff stage の timeout 欠落) で `cli-push-runner` の
`stages/diff.rs::run_diff_cmd` に timeout を導入した。T5 の直後に同 crate で
「shell + 全量 + timeout」が再び必要になったが、**lib への variant 追加は行わず callsite に
残置した**。T5 と逆の結論になったため、境界基準の適用例として記録する。

| 論点 | 判定 | 根拠 |
|---|---|---|
| T5 の `run_cmd_shell_unlimited` を使うか | ❌ 使わない | `run_cmd_shell_*` は **3 variant すべてが `combine_output` で stdout と stderr を結合する**。diff の stdout は reviewers が読むレビュー対象そのものとしてファイルに書かれるため、jj が stderr に出す警告 (並列 workspace 運用時の `Concurrent modification detected` 等) の混入は許容できない。variant 間の差は drain 戦略だけで、**結合するか否かは `run_cmd_shell_with` の骨格そのもの**であり variant では表現できない |
| 4 つ目の variant (`_unlimited_separated` 等) を lib に足すか | ❌ 足さない | 戻り値の型が `(bool, String)` から変わり `run_cmd_shell_with` の骨格に載らない = **既存 family への variant 追加ではなく別 family の新設**。層 1 の「1 crate でしか使われていない → extract せず、将来 2 つ目の使用例待ち」が素直に適用される。T5 が層 1 を適用しなかったのは「確立済み family への variant 追加で、代替は骨格の複製」だったためで、本件はその条件を満たさない |
| `bookmark_check::run_jj_bookmark_list` (同 crate に既存の「全量 + 分離 + timeout」) と共通化するか | ❌ しない | 層 1 の「signature が構造的に異なる (shell vs direct args)」に該当。`run_jj_bookmark_list` は direct args (`jj` を直接起動)、diff は config 由来の文字列を `cmd /c` で実行する。`run_cmd_direct` を各 crate に残置した既存判定と同型 |
| `wait_with_timeout_safe` / `_basic` のどちらを使うか | ✅ `_safe` | 層 2 の「Err 経路で kill するか / しないか」。diff は try_wait 失敗時に早期 return するため、child を残さない `_safe` を選ぶ |

T6 は層 3 の再評価 trigger 3 (「新規 callsite が増えて variant 名の選択が機械的でなくなった」)
に**触れかけた**事例でもある。`run_cmd_shell_*` の variant 名 (`_capped` / `_capped_reporting` /
`_unlimited`) はすべて **drain 戦略**を表しており、「stdout と stderr を結合するか」という
直交する軸は名前に現れない。T6 の callsite はこの軸で family 全体を選べなかった。
variant を増やす前に「その差異は既存の軸か、直交する新しい軸か」を確認すること —
直交する軸を variant 名に混ぜ始めると層 3 の naming 破綻に向かう。

なお T6 の実装過程で、`run_cmd_shell_*` 3 variant すべてに **timeout が wall-clock を縛れない**
欠陥があることが判明した (timeout 検知後に reader thread を join するが、`cmd /c` の孫プロセスが
pipe を保持するため join が孫の自然終了までブロックする。実測 9.23s / `timeout_secs = 1` 指定)。
本 ADR の境界判定とは別軸の実装欠陥であり、対処は
`docs/push-pipeline-fix-plan.md` §6 backlog 10 に登録済み。

## 影響

### 良い影響
Expand Down
Loading