diff --git a/CLAUDE.md b/CLAUDE.md index 8451fae4..a0eb296e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,10 +25,11 @@ - [ADR-021: jj 変更検出ロジックの設計原則](docs/adr/adr-021-jj-change-detection-principles.md) - [ADR-022: 自動化コンポーネントの責務分離原則](docs/adr/adr-022-automation-responsibility-separation.md) - [ADR-023: CodeRabbit false positive 対応スキル](docs/adr/adr-023-coderabbit-reject-thread-skill.md) *(試験運用)* -- [ADR-024: 共通 jj ヘルパーライブラリ](docs/adr/adr-024-shared-jj-helpers-library.md) *(試験運用)* +- [ADR-024: 共通 jj ヘルパーライブラリ](docs/adr/adr-024-shared-jj-helpers-library.md) - [ADR-025: CwdRestore Drop guard パターン](docs/adr/adr-025-cwd-restore-drop-guard.md) *(試験運用)* - [ADR-026: Cargo workspace による Rust パッケージ統合](docs/adr/adr-026-cargo-workspace.md) - [ADR-027: Push-time review を simplicity に限定し architectural review は post-PR に委ねる](docs/adr/adr-027-push-review-simplicity-focus.md) +- [ADR-028: 外部可視成果物の生成コマンド (PR 作成/マージ) の実行ゲート](docs/adr/adr-028-pnpm-create-pr-gate.md) ## Automated actor boundary (ADR-022) diff --git a/docs/adr/adr-019-coderabbit-review-hybrid-policy.md b/docs/adr/adr-019-coderabbit-review-hybrid-policy.md index 52caa1f5..f6c5dfb4 100644 --- a/docs/adr/adr-019-coderabbit-review-hybrid-policy.md +++ b/docs/adr/adr-019-coderabbit-review-hybrid-policy.md @@ -64,6 +64,63 @@ CodeRabbit は自身の Learning システムで「この repo/path では cross これにより CodeRabbit のレビュー自体が徐々に適合していく。 +### CodeRabbit 無料枠の制約 (2026-04-19 追記) + +本プロジェクトは CodeRabbit の無料枠を前提に運用しており、以下の制約を受け入れる: + +| 制約 | 影響 | +|---|---| +| **1 時間 3 回のレビュー上限** | 連続 PR 作成時に 3 本目以降のレビューがスキップされる | +| **public リポジトリ限定** | 本リポジトリが public である前提が崩れると CodeRabbit が利用不能 | +| **アカウント単位の制約** | fork / 別アカウント運用すると制約が分離される (逆用可能性あり) | + +### レビュアー可換性の方針 (2026-04-19 追記) + +CodeRabbit は便利だが、無料枠制約と将来的な仕様変更リスクを踏まえて「CodeRabbit 依存を固定化しない」ことを設計方針とする。 + +#### 「ハイブリッド」の定義 (再定義) + +当初 ADR-019 起草時の「ハイブリッド」は「takt 分析 + CodeRabbit review」の意味合いが強かったが、本追記で以下に再定義する: + +> **ハイブリッド = takt 内製レビュー + 外部 AI レビュー (plugin 可換)** + +外部 AI レビューは CodeRabbit に限定せず、以下を交換可能な plugin として扱う: + +- CodeRabbit (現行) +- GitHub Copilot Reviews +- Greptile +- その他 future 候補 + +#### Layer 構成は外部 AI 可換を前提に保つ + +ADR-019 の 3 レイヤー構成は外部 AI の種類に依存しない形で設計されている: + +- **Layer 1 (fitness filter)**: `.takt/facets/instructions/analyze-.md` を tool 別に用意する +- **Layer 2 (severity classification)**: `needs_fix` / `user_decision` / `approved` の 3-way verdict は tool 共通 +- **Layer 3 (hybrid re-push)**: `auto_push_severity` の設定は tool 非依存 + +切り替え時の実装コストは「Layer 1 の analyze instruction を新 tool 用に書き起こす」程度に抑える設計を維持する。 + +#### 具体的な可換性確保策 + +- **CodeRabbit 固有の成果物に依存しない命名**: `pr-review` / `post-pr-monitor` 等、tool 名を含まない workflow 名を優先 +- **analyze instruction は tool 別ファイル**: `analyze-coderabbit.md` / (将来) `analyze-copilot.md` のように分離 +- **Rust 側 (cli-pr-monitor) は tool 固有 API に依存しない**: PR comments API を通じて取得できる汎用フォーマットに閉じる + +### M5 (rate limit 耐性作り込み) を不採用とする論拠 (2026-04-19 追記) + +無料枠制約に対する耐性機能 (例: "3 回超過後の自動 retry"、"レビュー失敗時の claude -p fallback") を作り込まない理由: + +1. **レビュアーロックインの温床**: rate limit 耐性は CodeRabbit 固有挙動への依存を深め、可換性の方針と矛盾する +2. **投資対効果が薄い**: 1h 3 回制限は日常運用でまず引っかからない。連続 PR 作成の局面は設計上避けるべきケース (CodeRabbit rate limit 対策として PR-B/PR-C の push 間に 1 時間インターバル = 運用でカバー) +3. **無料枠に高度機能を期待しない**: 有償プラン契約の判断は別 ADR で行う。現時点では「制約を受け入れて公式経路を使う」のが最小コスト + +代わりに以下で運用カバーする: + +- PR 作成間隔の調整 (運用ルール、自動化なし) +- レビュー空振りを検出したら「そもそも push しない」「手動で claude -p review に切り替える」 (interactive 判断) +- ロックインが問題化した時点で plugin 可換設計の具体実装に着手 (本 ADR の方針に従う) + ## 影響 ### 採用される構成要素 diff --git a/docs/adr/adr-021-jj-change-detection-principles.md b/docs/adr/adr-021-jj-change-detection-principles.md index 1cfd0bfc..ac320347 100644 --- a/docs/adr/adr-021-jj-change-detection-principles.md +++ b/docs/adr/adr-021-jj-change-detection-principles.md @@ -83,6 +83,55 @@ pub fn decide_repush( commit_id 取得失敗時 (`IdCaptureFailed`) は「変更なし」と同じ扱いにし、push を発動しない。「判定できないから念のため push」は致命的副作用 (元 description 上書き等) を招く。 +### 原則 5: bookmark 検出は優先度付き revset + trunk filter を標準とする (2026-04-19 追加) + +PR #54 / PR #55 で cli-merge-pipeline / cli-pr-monitor の bookmark 検出を標準化した。以下をプロジェクト共通の既定値とする: + +```rust +const BOOKMARK_SEARCH_REVSETS: &[&str] = &["@", "@-", "@--"]; +const TRUNK_BOOKMARKS: &[&str] = &["main", "master", "trunk", "develop"]; +``` + +#### 問題の背景 + +`jj new` 直後は「`@` = 空コミット / bookmark は `@-` 上」という構成になる (PR #53 実測)。`@` だけを見る実装では bookmark 検出が空振りする。 + +単純に `@-` まで広げると、fresh checkout 直後に `@-` が trunk bookmark (`master` 等) を指してしまい、PR の head として trunk を誤検出する。 + +#### 検討した選択肢 + +| option | revset | 評価 | 結果 | +|---|---|---|---| +| A | `@` のみ | 空コミット状況で空振り (PR #53 実測の症状) | 不採用 | +| **B** | **`@`, `@-`, `@--` の近い順** | 最大 2 階層までカバー。trunk filter 併用で false hit 回避可 | **採用** | +| C | `ancestors(@, N)` 等の広い revset | N の決定が恣意的、遠い祖先の bookmark を誤検出するリスク | 不採用 | + +option B + trunk filter で、以下の両方を成立させる: + +- `@` 空 + bookmark が `@-` / `@--` 上にある一般的ケースをカバー +- fresh checkout で `@-` = `master` の場合に false hit しない + +#### 検出ロジックの 3 層構造 + +```text +parse : jj bookmark list 出力のテキスト解析 (pure function) +query : revset を受けて bookmark 名リストを返す (副作用: jj プロセス) +select : BOOKMARK_SEARCH_REVSETS を近い順に走査し、 + 最初に非空かつ trunk filter 通過する revset の結果を返す +``` + +`select_from_revsets(revsets, query_fn)` はクロージャを受け取る pure function 設計にし、unit test で jj プロセスなしに network / revset priority / trunk filter を検証できる (ADR-021 原則 3 と整合)。 + +#### 実装箇所 (PR #54 / PR #55 時点) + +| クレート | ファイル | 用途 | +|---|---|---| +| cli-merge-pipeline | `src/cli-merge-pipeline/src/main.rs` | `pnpm merge-pr` の PR detection | +| cli-pr-monitor | `src/cli-pr-monitor/src/util.rs` | `pnpm create-pr` の bookmark 検出 + `--head` 自動補完 | +| cli-push-runner | `src/cli-push-runner/src/stages/push_jj_bookmark.rs` | `pnpm push` の bookmark fallback | + +3 クレートで定数・関数が重複している状態は ADR-024 の共通化対象 (PR-C / `docs/todo.md` #8 で `lib-jj-helpers` 抽出予定)。 + ## 影響 ### 採用される構成要素 diff --git a/docs/adr/adr-024-shared-jj-helpers-library.md b/docs/adr/adr-024-shared-jj-helpers-library.md index b31bfcd8..881e511a 100644 --- a/docs/adr/adr-024-shared-jj-helpers-library.md +++ b/docs/adr/adr-024-shared-jj-helpers-library.md @@ -1,10 +1,10 @@ -# ADR-024 (仮): 共通 jj ヘルパーライブラリ +# ADR-024: 共通 jj ヘルパーライブラリ ## ステータス -試験運用 (観察開始: 2026-04-17) +本採用 (2026-04-19、試験運用期間: 2026-04-17 ～ 2026-04-19) -> 本 ADR は試験運用ステータス。正式採用は他パッケージでの使用例出現による。 +> 観察期間中 (2026-04-17 起点、想定 3.5 ヶ月) の早い段階で正式採用条件 (3 箇所の port 完了) を達成したため繰上げ本採用。実抽出作業は PR-C (`docs/todo.md` #8) で `src/lib-jj-helpers/` を新設して実施する。 ## コンテキスト @@ -17,76 +17,94 @@ pub(crate) fn capture_commit_id() -> Option { ... } pub(crate) fn diff_is_empty(from: &str, to: &str) -> bool { ... } ``` -現状は `pub(crate)` (cli-pr-monitor 内からのみ呼び出し)。正式採用時に package を跨いで共有するとなった段階で `pub` に引き上げる。 +これらは ADR-021 の「jj 変更検出の二段構え判定」を実装する基盤ヘルパーで、cli-pr-monitor 固有のロジックではない。 -これらは ADR-021 の「jj 変更検出の二段構え判定」を実装する基盤ヘルパーで、cli-pr-monitor 固有のロジックではない。将来以下のような場面で同じ関数が必要になる可能性がある: - -- **cli-merge-pipeline の post_steps 実装** (ADR-013 + ADR-014): merge 後の AI ステップで「merge 後に変化があったか」を検出 -- **cli-push-runner の bookmark 自動化** (feat/push-runner-auto-bookmark): push 前に @ が変わったか検出 -- **その他の jj 連携 CLI**: 将来の拡張 +本 ADR 試験運用時点では 1 箇所のみの使用で、早期の共通化は YAGNI 違反と判断して観察ステータスに留めていた。 ### 早期の共通化はリスク -現時点で `src/lib-jj-helpers/` を新設して 2 関数を移すのは YAGNI に反する: - - 1 つの呼び出し元しか存在しない - API 設計 (関数シグネチャ、エラー型、timeout 値、log 出力方法) の安定性が読めない - 2 つ目の呼び出し例が出て初めて「共通部分」と「固有部分」を分離できる -## 決定 (試験運用方針) - -### 観察期間 - -2026-04-17 ～ 2026-07-31 (約 3.5 ヶ月、ADR-023 と同期)。 +## 決定 (本採用) -### 観察対象 +### 正式採用条件の達成実績 -- cli-pr-monitor 以外の Rust パッケージで `capture_commit_id` / `diff_is_empty` 相当の関数を使いたい場面が出現するか -- 出現時に「cli-pr-monitor の関数をそのまま呼ぶ」「コピペで再実装」「共通ライブラリ化」のどれが自然か +観察期間 2026-04-17 ～ 2026-07-31 を想定していたが、2026-04-19 時点で既に 3 箇所で同パターンが port 済みとなった: -### 正式採用条件 (2026-07-31 再評価) +| クレート | ファイル | 導入 PR | +|---|---|---| +| cli-pr-monitor | `src/cli-pr-monitor/src/util.rs` | PR #55 (2026-04-19) | +| cli-merge-pipeline | `src/cli-merge-pipeline/src/main.rs` | PR #54 (2026-04-19) | +| cli-push-runner | `src/cli-push-runner/src/stages/push_jj_bookmark.rs` | PR #50 (2026-04-18) | -| 他パッケージでの使用例 | アクション | -|----------------------|----------| -| 2 つ目の使用例が出現 | 正式採用 → `src/lib-jj-helpers/` 新設 | -| 1 つだけ (cli-pr-monitor のみ) | ADR 廃止 (cli-pr-monitor 内に留める) | -| 使用例なしだが明確な計画あり | 延長 (半年) | +試験運用方針で定めた「2 つ目の使用例出現で正式採用」を超えており、さらに ADR-021 原則 5 (bookmark 検出の優先度付き revset + trunk filter) で定数・関数が機械的に追加される見通しが立った。このままだと 4 箇所目以降も重複コピペが増える。 -### 正式採用時の候補構成 +### 採用する構成 ```text src/lib-jj-helpers/ ├── Cargo.toml └── src/ - └── lib.rs ← capture_commit_id / diff_is_empty / その他共通 jj ラッパー + └── lib.rs ``` -- 既存 `src/lib-report-formatter/` と同階層 (ADR-012 の命名規約 `lib-*`) -- 依存元パッケージは workspace の member として参照 (ADR-026 (予定) の Cargo workspace 化を前提) +公開する API の初期セット: + +- ADR-021 原則 1-4 系 (変更検出): + - `capture_commit_id() -> Option` + - `diff_is_empty(from: &str, to: &str) -> bool` +- ADR-021 原則 5 系 (bookmark 検出): + - 定数 `BOOKMARK_SEARCH_REVSETS = ["@", "@-", "@--"]` + - 定数 `TRUNK_BOOKMARKS = ["main", "master", "trunk", "develop"]` + - `is_trunk_bookmark(name: &str) -> bool` + - `parse_bookmark_list_output(stdout: &str) -> Vec` + - `select_from_revsets(...)` (クロージャ注入型 pure function) + - `query_bookmarks_at(revset: &str) -> Vec` + - `get_jj_bookmarks(stderr_mode: StderrMode) -> Vec` + +配置は ADR-012 の命名規約 `lib-*` に従い、ADR-026 の Cargo workspace の member として登録する。 + +### API 設計方針 (PR-C で確定) + +呼び出し側 3 クレートで `stderr` ハンドリングと log prefix が異なるため、以下で吸収する: + +- **`stderr` ハンドリングは引数化**: `enum StderrMode { Silent, Piped(LogFn) }` で `Stdio::null` 派 (cli-pr-monitor) と `Stdio::piped` + logging 派 (cli-merge-pipeline) を両立 +- **`log_info` 注入**: `fn(&str)` クロージャを引数で受ける設計。各クレート固有 prefix (`[post-pr-monitor]` / `[merge-pipeline]` 等) を崩さない +- **fallback 方針**: log 注入設計で詰まった場合は「各クレート固有の薄いラッパー関数を残す」方針で進める (PR-C 段階で判断) + +### 移行方針 + +PR-C (`docs/todo.md` #8) で以下を実施: + +1. `src/lib-jj-helpers/` 新設、workspace member 登録 +2. 共通定数・関数を移動し `pub` 公開 +3. 呼び出し側 3 クレートを差し替え、各 `Cargo.toml` に依存追加 +4. unit テストを `lib-jj-helpers` 側に集約、3 クレートの重複テスト削除 +5. `cargo test --workspace` / `pnpm build:all` でグリーン確認 ## 影響 -### 試験運用中の運用 +### 採用される構成要素 -- cli-pr-monitor の `runner.rs` に `capture_commit_id` / `diff_is_empty` を持つ (現状維持) -- 他パッケージで同機能が必要になったら: - 1. まず「cli-pr-monitor の関数を pub 化して参照」を試す - 2. Cargo workspace 化 (ADR-026 (予定)) 後なら cross-package dependency で呼び出せる - 3. 2 箇所以上で必要になったらライブラリ化を本 ADR の正式採用として検討 +- `src/lib-jj-helpers/` (PR-C で新設予定) +- 3 呼び出し側クレート (`cli-pr-monitor` / `cli-merge-pipeline` / `cli-push-runner`) の `Cargo.toml` への依存追加 -### 参照する他 ADR +### 避けるべきアンチパターン -- ADR-012 (src/ ディレクトリの命名規約): `lib-*` prefix に従う -- ADR-021 (jj 変更検出): 本 ADR のヘルパーが実装する原則 -- ADR-026 (予定): Cargo workspace 化が先行する前提 +- **4 箇所目のクレートが出現しても個別コピペで対応**: ADR-021 原則 5 の定数・関数が広がる機械的パターンでは保守性が崩壊する +- **`pub(crate)` のまま他クレートから呼び出そうとする**: workspace 依存で解決できるが、所在が不透明になり循環依存の温床になる +- **共通化で過度に汎用化する**: 既存 3 箇所の使い方を超える抽象化は YAGNI 違反。今必要な API だけを公開 -## 次ステップ (試験運用中に確認すること) +### 参照する他 ADR -- cli-merge-pipeline の post_steps 実装 (現 docs/todo.md #3) が始まったときに使用を検討 -- cli-push-runner の bookmark 自動化再開時も同様 -- 使用パターンを 2 件観察してから共通化すれば、過度に汎用化せずに済む +- ADR-012 (src/ ディレクトリの命名規約): `lib-*` prefix に従う +- ADR-021 (jj 変更検出): 本 ADR のヘルパーが実装する原則 (原則 1-5 すべて) +- ADR-026 (Cargo workspace): workspace member として参照する前提 -## 観察終了条件 +## 次ステップ (スコープ外、PR-C で実施) -- 2026-07-31 時点で使用例を再評価 -- 正式採用 / 延長 / 廃止 のいずれかを選択し、本 ADR の status を更新 +- **PR-C (`docs/todo.md` #8)**: `src/lib-jj-helpers/` 新設と 3 クレート差し替え +- **将来の新規クレート**: jj 連携が必要になったらまず `lib-jj-helpers` を依存に追加することから始める +- **API 拡張**: `jj new` / `jj describe` / `jj bookmark` 系のラッパーは都度検討 (早期汎用化を避ける) diff --git a/docs/adr/adr-028-pnpm-create-pr-gate.md b/docs/adr/adr-028-pnpm-create-pr-gate.md new file mode 100644 index 00000000..0adbdfd3 --- /dev/null +++ b/docs/adr/adr-028-pnpm-create-pr-gate.md @@ -0,0 +1,170 @@ +# ADR-028: 外部可視成果物の生成コマンド (PR 作成/マージ) の実行ゲート + +## ステータス + +承認済み (2026-04-19) + +## コンテキスト + +### 問題 + +auto mode の運用で「自律実行 OK」と「外部から観測可能な成果物の生成」を同じ尺度で扱ってしまい、セッション 247510ea (2026-04-18) では `run_in_background: true` で `pnpm create-pr` が実行されて PR #54 が意図しないタイミングで作成される事故が発生した。 + +auto mode 本来の趣旨は「低リスク作業 (lint / test / build / local refactor) を自律実行してスループットを上げる」ことであり、**GitHub 上に公開可能な成果物を生成するコマンドは射程外**だった。しかし明文化していなかったため、Claude 側の判断で auto 扱いされた。 + +### auto モードの「自律性」と「外部可視性」は独立軸 + +| 軸 | 低リスク | 高リスク | +|---|---|---| +| **取り消しコスト (=外部可視性)** | 消してやり直せる (`cargo build`, lint) | 取り消しコスト大 (`gh pr create`, `gh pr merge`) | +| **判断の複雑度 (=自律性)** | 機械的判断で完結 (format 適用) | 人間の意図が必要 (bookmark 命名は OK、commit description は人間) | + +auto mode が緩和するのは「判断の複雑度」軸であって、「取り消しコスト」軸ではない。外部可視成果物の生成は後者の軸に属するため、auto mode の緩和対象外。 + +### 取り消しコスト大のコマンド一覧 (本プロジェクト) + +以下は GitHub 上で観測可能な成果物を生成または破壊的に変更するため、実行後の巻き戻しが困難: + +- `pnpm create-pr` (内部で `gh pr create`) — PR 作成 +- `pnpm merge-pr` (内部で `gh pr merge`) — PR マージ (後戻り不可) +- `.claude/cli-pr-monitor.exe` — `pnpm create-pr` の実体 +- `.claude/cli-merge-pipeline.exe` — `pnpm merge-pr` の実体 +- `gh pr create` / `gh pr merge` 直接呼び出し + +加えて CodeRabbit の無料枠 (1h 3 回 / public リポジトリ) が PR 作成と同時に消費されるため (ADR-019)、巻き戻した後に「今日は CodeRabbit が動かない」状態を招く可能性もある。 + +### 既存防衛層の限界 + +**一次防衛層**: Claude 用 auto-memory `feedback_bookmark_auto_naming.md` (プロジェクト毎 memory ディレクトリ配下) + +> `pnpm create-pr`: auto mode であっても、実行前にユーザー許可を明示的に取る。`run_in_background: true` で走らせるのも NG + +これは「Claude が守る意志を持つ」ことに依存する soft な防衛で、セッション間のメモリ欠落や判断ブレで破られる。実際セッション 247510ea はこの層だけだった時期に突破された。 + +**既存 `preset_gh_pr_create_guard` (PreToolUse hook)**: 直接 `gh pr create` を呼ぶと `src/hooks-pre-tool-validate/src/main.rs` がブロックし `pnpm create-pr` に誘導する。ただし `pnpm create-pr` 自身はブロックしないので「経路統一」の役割までで、実行ゲートにはならない。 + +### なぜ hook `block` を採用しないか + +素直な案は「PreToolUse hook で `pnpm create-pr` も block する」だが、以下の理由で UX が崩壊する: + +- hook の `block` は permission prompt より前段で効く。ユーザーが「今は作っていい」と判断しても block される +- `allow` に登録すれば block は外れるが、そうすると許可/非許可のトグルが手動運用になる (`.claude/settings.json` を毎回編集) +- 目的は「**毎回確認する**」であって「**禁止する**」ではないため、block の意味論と一致しない + +### `permissions.ask` の性質 + +`.claude/settings.json` の `permissions.ask` は「該当パターンに一致するコマンドは毎回 permission prompt を出す」設定。性質: + +- Claude 側の判断に依存せず harness (Claude Code 本体) が強制する +- Claude Code の permission 優先順位は `Deny > Ask > Allow` ([公式 docs: Configure permissions](https://code.claude.com/docs/en/permissions))。`allow` にも同じパターンが登録されていても `ask` が優先され、auto mode でも毎回 prompt が出る +- permission prompt はユーザーが deny できるため「取り消しコスト」ゼロの事前ゲートとして機能する +- パターンは Bash glob ライクな syntax (`Bash(pnpm create-pr*)` 等) + +## 決定 + +### 原則 1: 「取り消しコスト大」のコマンドは auto mode 緩和の対象外 + +以下のコマンドは auto mode / interactive mode を問わず、**実行前にユーザー許可を取る**: + +- `pnpm create-pr` +- `pnpm merge-pr` +- `.claude/cli-pr-monitor.exe` (直接呼び出し) +- `.claude/cli-merge-pipeline.exe` (直接呼び出し) +- `gh pr create` / `gh pr merge` 直接呼び出し (既存 `preset_gh_pr_create_guard` で `pnpm` 経路に誘導済) + +### 原則 2: 二層防衛 + +| 層 | 仕組み | 対象 | 強度 | +|---|---|---|---| +| **一次防衛** | memory `feedback_bookmark_auto_naming.md` | Claude の判断 | soft (Claude が守る意志に依存) | +| **二次防衛** | `.claude/settings.json` の `permissions.ask` | harness 強制 | hard (Claude がスキップ不可) | + +一次だけだとメモリ欠落や判断ブレで突破される。二次だけだと「なぜ毎回確認するか」の意図が失われて運用が形骸化する。両方必要。 + +### 原則 3: hook `block` は採用しない + +`PreToolUse` hook の `block` は「許可後も等しく効く」ため、`permissions.ask` と二重になると UX が破壊される。`block` は「絶対に実行させない」場面 (例: `gh pr create` 直呼び → `pnpm` 経路に矯正) に限定し、「毎回確認したい」場面には `permissions.ask` を使う。 + +### 原則 4: `preset_gh_pr_create_guard` との直列二段フィルタ + +```text +直接呼び出し 経路統一済み 実行ゲート +gh pr create ──→ (block) → (到達しない) + preset_gh_pr_create_guard +pnpm create-pr ──→ (pass-through) → (ask) + permissions.ask +``` + +- **traffic cop** (`preset_gh_pr_create_guard`, PreToolUse hook): `gh pr create` 直接呼びを禁止し `pnpm create-pr` に経路統一 +- **実行ゲート** (`permissions.ask`, harness): `pnpm create-pr` 実行時に毎回 prompt を出す + +両者は責務が直交しているため干渉しない。PR-B (`docs/todo.md` #7) で `permissions.ask` のパターンを追加することで本 ADR の二次防衛層を実装する。 + +### 原則 5: ADR-022 との境界 + +| ADR | 対象 actor | 対象コマンド | +|---|---|---| +| **ADR-022** | takt / claude -p / cli-* の**自律ループ** | commit message / bookmark / tag / PR title/body の書き換え禁止 | +| **ADR-028 (本)** | interactive session の **Claude Code 自身** | 外部可視成果物の生成コマンド実行の事前許可 | + +ADR-022 は「automated actor は人間の意図表現に介入しない」、ADR-028 は「Claude 自身も取り消しコスト大の操作は事前確認」。両者は補完関係にあり、いずれかだけでは防衛が穴だらけになる。 + +**ADR-022 の射程内 (automated actor)**: +- takt fix による `@` edit → 自動 amend +- cli-pr-monitor の auto re-push + +**ADR-028 の射程内 (interactive session の Claude)**: +- `jj bookmark create ` → 自律実行 OK (ADR-022 の射程外、interactive 判断) +- `pnpm push` → 自律実行 OK (permission prompt がゲート) +- `pnpm create-pr` → 事前許可必須 (本 ADR) + +## 影響 + +### 採用される構成要素 + +- `.claude/settings.json` の `permissions.ask` (PR-B で追加予定): 4 パターン + - `Bash(pnpm create-pr*)` + - `Bash(pnpm merge-pr*)` + - `.claude/cli-pr-monitor.exe` 直接呼び出し捕捉パターン + - `.claude/cli-merge-pipeline.exe` 直接呼び出し捕捉パターン +- Claude auto-memory `feedback_bookmark_auto_naming.md` (既存、一次防衛) +- `src/hooks-pre-tool-validate/src/main.rs::preset_gh_pr_create_guard` (既存、traffic cop) + +### 避けるべきアンチパターン + +- **auto mode を「取り消しコスト大の操作も自律実行してよい」と拡大解釈する**: セッション 247510ea の事故の再発を招く +- **`permissions.ask` から `pnpm create-pr` パターンを外す**: 二次防衛層が無効化されて一次防衛 (memory) のみに戻る。`allow` への登録自体は `ask > allow` の precedence 上で無害だが、`ask` を外すと harness 強制ゲートが消える +- **hook `block` で `pnpm create-pr` を禁止する**: 許可後も block されて UX 崩壊 +- **memory の一次防衛層のみで済ませる**: セッション間でメモリが欠落すれば突破される +- **自動化コンポーネント (takt / cli-*) に PR 作成権限を与える**: ADR-022 違反 + +### 想定される運用 + +interactive session での PR 作成フロー: + +1. Claude が `jj bookmark create ` を自律実行 (確認不要) +2. `pnpm push` を foreground 実行 (permission prompt がゲート) +3. push 後、Claude が PR title / body のドラフトを提示 +4. ユーザーが明示承認 +5. `pnpm create-pr` 実行 (permissions.ask プロンプトで再確認) → PR 作成 + +### 非対象 + +- `pnpm push`: permission prompt が既に毎回発火するため、追加の ask ルールは不要 (memory `feedback_bookmark_auto_naming.md` の 2. と整合) +- `jj git push`: 本プロジェクトでは `pnpm push` 経路に統一しているため個別 ask 対象外 +- takt / cli-* 内部からの push: ADR-022 の射程で、そもそも PR 作成/マージに介入しない + +## 次ステップ (スコープ外、PR-B 以降で対応) + +- **PR-B (`docs/todo.md` #7)**: `.claude/settings.json` に `permissions.ask` 4 パターンを追加して二次防衛層を実装 +- **PR-D (`docs/todo.md` #9)**: `prepare-pr` skill で「ドラフト提示 → 明示承認 → 実行」フローを標準化 +- **運用レビュー**: 2026-07 に二次防衛層の発火頻度を計測。毎回 prompt 応答が形骸化していないか確認 + +## 参照 + +- ADR-022 (自動化コンポーネントの責務分離): 補完関係にある。automated actor 側の原則 +- ADR-019 (CodeRabbit ハイブリッド): 無料枠 1h 3 回制約が「取り消しコスト」を増幅する根拠 +- memory `feedback_bookmark_auto_naming.md`: 一次防衛層 +- `src/hooks-pre-tool-validate/src/main.rs::preset_gh_pr_create_guard`: traffic cop 層 +- セッション 247510ea-3f24-4b87-8f68-3c860e1b1b4e (2026-04-18): 事故発生源 +- PR #54 / PR #55: 事故後の水平展開作業 diff --git a/docs/todo.md b/docs/todo.md index 5e22343a..5d8d5bd9 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -157,31 +157,133 @@ - ADR-019 (CodeRabbit レビュー運用ハイブリッド) - 関連: task 5 (bookmark auto-advance) -### 7. `jj-helpers` 共通クレート抽出 (ADR-024 延長) +--- + +## セッション 247510ea 由来: 整備タスク群 (PR-B〜PR-D) + +> **最優先ブロック**。PR #54 / #55 のマージ後に確定した ADR / 仕組みの整備作業。3 タスクは **PR 粒度で分割実施**、依存関係あり。 +> +> **背景コンテキスト**: +> - **発端セッション**: `247510ea-3f24-4b87-8f68-3c860e1b1b4e` (2026-04-18) +> - **先行成果**: +> - PR #54 (cli-merge-pipeline の revset 拡張 `@-..@--` + trunk filter) +> - PR #55 (cli-pr-monitor への同パターン水平展開) +> - PR-A (ADR 集約 / 本 PR): ADR-028 新設、ADR-021 原則 5 追加、ADR-024 本採用、ADR-019 可換性追記 +> - **ユーザーフィードバック 3 点** (memory `feedback_bookmark_auto_naming.md` に記録済): +> 1. auto mode は試験導入。基本は自律実行だが、**最終出力の責任をユーザーが握るため `pnpm create-pr` / `pnpm merge-pr` は事前許可必須** +> 2. bookmark 名は Claude が自動採番して OK +> 3. `pnpm push` は foreground 実行 OK (permission prompt がゲート) +> - **設計的な確認事項** (ADR-028 / ADR-019 追記で明文化済): +> - `hooks` での `block` は許可後も効くので UX 崩壊 → 採用せず +> - 代わりに **settings.json の Ask ルール** で「毎回確認プロンプト」を出す +> - CodeRabbit 無料枠の制約 (1h 3 回、public リポジトリ限定) を許容し、rate limit 耐性の作り込みは **しない** (レビュアーロックイン回避のため) +> +> **PR 依存関係**: +> ``` +> PR-A (ADR 集約, merged) ──┬── PR-B (Ask ルール + body helper) ── PR-D (prepare-pr skill) +> └── PR-C (jj-helpers 抽出) +> ``` +> +> **推奨実行順序**: (PR-B, PR-C 並列) → PR-D +> +> **CodeRabbit rate limit 対策**: PR-B と PR-C の push 間に 1 時間のインターバルを入れる (無料枠は 1h 3 件制限) + +### 7. [PR-B] Ask ルール + PR body helper + +- **やろうとしたこと**: ADR-028 の二次防衛層を実装 + PR body 生成を標準化。`pnpm create-pr` 実行時の harness 強制プロンプト、および PR body を一時ファイル経由で渡す helper script を整備 +- **現在地**: 未着手。PR-A merge 後に着手可 (ADR-028 が前提) +- **実装内容**: + - [ ] **`.claude/settings.json` 更新**: `permissions.ask` に 4 パターン追加 + - `Bash(pnpm create-pr*)` + - `Bash(pnpm merge-pr*)` + - `.claude/cli-pr-monitor.exe` 直接呼び出しを捕捉するパターン + - `.claude/cli-merge-pipeline.exe` 直接呼び出しを捕捉するパターン + - パターン syntax は `https://code.claude.com/docs/en/permissions#manage-permissions` 参照 + - [ ] **`scripts/prepare-pr-body.ps1` 新規**: stdin から body を受け取り `.tmp-pr-body.md` に書き出し、パスを stdout に返す + - 終了時の cleanup 選択肢を用意 + - [ ] **`package.json` に `"prepare-pr-body"` スクリプト追加**: ps1 ラッパー + - [ ] **検証**: + - `pnpm create-pr` を Claude が実行しようとすると毎回プロンプトが出る + - `allow`-listed な他コマンド (`pnpm build:all` 等) は prompt なしで通る (回帰なし) + - 既存 `preset_gh_pr_create_guard` で `gh pr create` 直接呼び出しが block される (回帰なし) + - [ ] README or docs にワークフロー更新を反映 (任意) +- **詰まっている箇所**: + - **Ask ルールのパターン syntax**: Claude Code ドキュメントで `Bash(pattern)` 形式と確認が必要。ワイルドカード・前方後方一致の挙動を実装時に確認 + - **直接 exe 呼び出しパスのマッチング**: `.claude\` 区切りや絶対パス・相対パス両方を捕捉する regex か glob が必要 +- **想定サイズ**: 小 (~50-100 行、3-4 ファイル) +- **依存**: **PR-A** (ADR-028 確定が前提) +- **見積**: 45-90 分 +- **参照**: ADR-028, memory `feedback_bookmark_auto_naming.md` + +### 8. [PR-C] `jj-helpers` 共通クレート抽出 (ADR-024 本採用後) -- **やろうとしたこと**: PR #55 の CodeRabbit Nitpick で指摘された通り、bookmark 検出ロジック (`BOOKMARK_SEARCH_REVSETS` / `TRUNK_BOOKMARKS` / `is_trunk_bookmark` / `parse_bookmark_list_output` / `select_from_revsets` / `query_bookmarks_at`) が **cli-push-runner / cli-merge-pipeline / cli-pr-monitor の 3 クレートで重複定義** されている状態を解消 -- **現在地**: 未着手。3 回目の port で抽出するのが ADR-024 試験運用の "2 個目の port 完了後に判断" 条件を満たしたタイミング +- **やろうとしたこと**: PR #55 の CodeRabbit Nitpick で指摘された通り、bookmark 検出ロジックが **cli-push-runner / cli-merge-pipeline / cli-pr-monitor の 3 クレートで重複定義** されている状態を `jj-helpers` 共通クレートに集約 +- **現在地**: 未着手。PR-A merge 後に着手可 (ADR-024 本採用が前提) - **背景**: - cli-push-runner: `push_jj_bookmark.rs` に `TRUNK_BOOKMARKS`, `is_trunk_bookmark`, bookmark parsing - cli-merge-pipeline: PR #54 で 3 層構造 + trunk filter を実装 - - cli-pr-monitor: PR #55 で同パターンを移植 (本 PR) - - 次に 4 つ目のクレート (例: hooks 系) が同パターンを必要としたら、機械的に広がる懸念 + - cli-pr-monitor: PR #55 で同パターンを移植 + - ADR-024 の「試験運用」条件 = **3 箇所目の port 完了** が達成済 + - 次に 4 箇所目のクレートが同パターンを必要とすると機械的に広がる懸念 - **実装内容**: - - [ ] ADR-026 workspace 構成で新クレート `jj-helpers` を追加 - - [ ] 3 クレートの共通定数 (`BOOKMARK_SEARCH_REVSETS`, `TRUNK_BOOKMARKS`) と関数 (`is_trunk_bookmark`, `parse_bookmark_list_output`, `select_from_revsets`, `query_bookmarks_at`, `get_jj_bookmarks`) を `pub` で移動 - - [ ] 呼び出し側 3 クレートを `jj_helpers::get_jj_bookmarks` 等に差し替え - - [ ] unit テストは `jj-helpers` 側に集約 (3 クレートの重複テストを削除) - - [ ] `stderr` ハンドリングは cli 固有なので引数化 (cli-pr-monitor は `Stdio::null`, cli-merge-pipeline は `Stdio::piped` + logging) + - [ ] **新クレート `src/lib-jj-helpers/` 追加** (ADR-026 workspace 準拠) + - `Cargo.toml`, `src/lib.rs` + - Cargo workspace root の `members` に追加 + - [ ] **共通定数と関数を `pub` で移動**: + - `BOOKMARK_SEARCH_REVSETS`, `TRUNK_BOOKMARKS` + - `is_trunk_bookmark`, `parse_bookmark_list_output`, `select_from_revsets`, `query_bookmarks_at`, `get_jj_bookmarks` + - [ ] **`stderr` ハンドリングを引数化**: `fn get_jj_bookmarks(stderr_mode: StderrMode)` 等 + - cli-pr-monitor は `Stdio::null` 継続 + - cli-merge-pipeline は `Stdio::piped` + logging 継続 + - [ ] **`log_info` 注入**: `fn(&str) -> ()` を引数で受け取る設計 + - 各クレート固有 prefix (`[post-pr-monitor]` / `[merge-pipeline]` 等) を崩さない + - [ ] **呼び出し側 3 クレート差し替え**: + - `src/cli-push-runner/src/stages/push_jj_bookmark.rs` + - `src/cli-merge-pipeline/src/main.rs` + - `src/cli-pr-monitor/src/util.rs` + - 各 `Cargo.toml` に `lib-jj-helpers` 依存追加 + - [ ] **unit テスト集約**: `lib-jj-helpers` 側に集約、3 クレートの重複テスト削除 + - [ ] **検証**: + - `cargo test --workspace` でグリーン + - `pnpm build:all` で全 exe がビルドされる + - PR #54/#55 の動作パターン (`@` 空 / `@-` = bookmark) の smoke test - **詰まっている箇所**: - - **log_info の依存**: 各クレートの `log_info` が別実装 (cli-pr-monitor は prefix `[post-pr-monitor]`、cli-merge-pipeline は `[merge-pipeline]` 等)。`select_from_revsets` の「別 revset 検出」ログをどう出すか要設計 (logger インジェクションか、呼び出し側で wrap するか) - - **ADR-024 本格採用の判断**: 現在「試験運用」扱い。3 箇所目の port で明確な痛みが可視化された今、本採用に格上げする ADR 改訂が先か、先にコード抽出するかの順序判断 -- **参照**: - - PR #54 (cli-merge-pipeline 先行実装) - - PR #55 (cli-pr-monitor 移植 + CodeRabbit Nitpick 指摘) - - ADR-024 (共通 jj helper、試験運用) - - ADR-026 (Cargo workspace) + - **log_info 注入設計**: クロージャ vs. trait vs. 呼び出し側 wrap のどれが最もエルゴノミックか要設計 + - **stderr ハンドリング引数化**: `enum StderrMode { Silent, Piped(LogFn) }` のような型定義 +- **想定サイズ**: 中〜大 (refactor、~400-600 行差分、但し移動が主) +- **依存**: **PR-A** (ADR-024 格上げが前提) +- **見積**: 2-4 時間 +- **リスク**: log_info 注入の設計で躓いた場合、「各クレート固有の薄いラッパー関数を残す」フォールバック方針で進める +- **参照**: PR #54, PR #55 (CodeRabbit Nitpick 1), ADR-024, ADR-026 (Cargo workspace) + +### 9. [PR-D] `prepare-pr` skill (試験運用) + +- **やろうとしたこと**: auto mode で安全に PR を作成するためのインタビュー型 skill を試験運用として整備。commit log と diff から PR title / body の初稿を生成し、ユーザー承認後に `pnpm create-pr` を foreground 実行するフローを標準化 +- **現在地**: 未着手。PR-B merge 後に着手 (ADR-028 の運用フローと PR-B の body helper が前提) +- **実装内容**: + - [ ] **`.claude/skills/prepare-pr/SKILL.md` 新規** (試験運用ステータス) + - 起動条件: 「PR を作成して」等の明示依頼、または `/prepare-pr` 起動 + - ステップ: + 1. `jj status` + `jj log -r master..@` で差分サマリ取得 + 2. commit description から PR title 初稿生成 + 3. diff から PR body 初稿生成 (Summary / Changes / Test Plan / References セクション) + 4. Claude が提示 → **明示承認** (AskUserQuestion 強制) + 5. `pnpm prepare-pr-body` (PR-B 成果物) 経由で body 書き込み + 6. `pnpm create-pr --title ... --body-file ...` foreground 実行 (Ask プロンプトで再確認) + 7. `.tmp-pr-body.md` 削除 + - [ ] **検証**: skill 起動テスト、PR 作成完遂確認 +- **詰まっている箇所**: + - **skill の既存 frontend-design/pre-push-review との連携**: 既存スキルとの衝突可否を skill-sync-check で確認 +- **想定サイズ**: 小〜中 (skill 定義 1 本、~100-200 行) +- **依存**: **PR-A** (ADR-028), **PR-B** (M1 body helper, Ask ルール) +- **見積**: 1-2 時間 +- **参照**: ADR-028, PR-B 成果物 + +--- + +## その他の進行中タスク -### 8. 雑務: 過去の delete-pending bookmark cleanup +### 10. 雑務: 過去の delete-pending bookmark cleanup - **やろうとしたこと**: `jj git push --tracked` で `Refusing to push deleted bookmark fix/push-allow-new` の警告が出るため、`jj bookmark forget fix/push-allow-new` で消す - **現在地**: 未対応。push を block しないので緊急性なし