feat(cli-pr-monitor): bookmark 検出を @-/@-- まで拡張 (PR #54 の水平展開)

docs/todo.md

@@ -157,21 +157,29 @@
   - ADR-019 (CodeRabbit レビュー運用ハイブリッド)
   - 関連: task 5 (bookmark auto-advance)
 
-### 7. cli-pr-monitor の bookmark 検出を `@-..@--` まで拡張 (cli-merge-pipeline の水平展開)
+### 7. `jj-helpers` 共通クレート抽出 (ADR-024 延長)
 
-- **やろうとしたこと**: [src/cli-pr-monitor/src/util.rs:86](src/cli-pr-monitor/src/util.rs#L86) の `get_jj_bookmarks` は revset `@` のみで bookmark を探しており、cli-merge-pipeline で先に解消した task 7 (旧) と同じ「`@` 空 / bookmark が `@-` 上」問題を抱えている。`pnpm create-pr` や PR 検索フェーズで同じ空振りが起きる可能性があるため、横展開して整合させる
-- **現在地**: 設計確定、未着手 (cli-merge-pipeline 側の PR で実装パターンが確定済み)
+- **やろうとしたこと**: 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 完了後に判断" 条件を満たしたタイミング
+- **背景**:
+  - 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 系) が同パターンを必要としたら、機械的に広がる懸念
 - **実装内容**:
-  - [ ] [src/cli-pr-monitor/src/util.rs](src/cli-pr-monitor/src/util.rs) の `get_jj_bookmarks` を cli-merge-pipeline と同じ 3 層 (parse / query / select_from_revsets) に分割
-  - [ ] `BOOKMARK_SEARCH_REVSETS = &["@", "@-", "@--"]` + `TRUNK_BOOKMARKS` filter を移植
-  - [ ] unit テスト: parse と優先順位を cli-merge-pipeline のテストと同等に追加
-  - [ ] E2E: 空 `@` 状態で `pnpm create-pr` の Strategy B が成功することを確認
+  - [ ] 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)
 - **詰まっている箇所**:
-  - **共通化するかコピーするか**: ADR-024 (共通 jj helpers crate、試験運用) の延長で `jj-helpers` クレート化するのが理想だが、まず cli-pr-monitor への port (コピー) で機能等価を確認するのが安全。共通化は 2 個目の port 完了後に判断
-  - **create_pr.rs での利用箇所**: [src/cli-pr-monitor/src/stages/create_pr.rs:183](src/cli-pr-monitor/src/stages/create_pr.rs#L183) でも `get_jj_bookmarks` を使っており、`--head` 引数に先頭の bookmark を渡している。trunk filter を入れた結果「先頭が feature bookmark」保証が強まるので挙動は改善方向だが、回帰テストで確認する
+  - **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 改訂が先か、先にコード抽出するかの順序判断
 - **参照**:
-  - 先行実装: cli-merge-pipeline の同等対応 (task 7 旧、PR で完了済み)
-  - ADR-013 (cli-merge-pipeline), ADR-024 (共通 jj helpers、試験運用)
+  - PR #54 (cli-merge-pipeline 先行実装)
+  - PR #55 (cli-pr-monitor 移植 + CodeRabbit Nitpick 指摘)
+  - ADR-024 (共通 jj helper、試験運用)
+  - ADR-026 (Cargo workspace)
 
 ### 8. 雑務: 過去の delete-pending bookmark cleanup
 

src/cli-pr-monitor/src/util.rs

@@ -82,16 +82,73 @@ pub(crate) fn parse_pr_number_from_url(output: &str) -> Option<u64> {
     None
 }
 
-/// 現在の jj change に紐づく全ブックマーク名を取得する
+/// Bookmark 検索に使用する revset のリスト (近い順 = 優先順)。
+///
+/// `select_from_revsets` は先頭から順に試し、最初に (trunk 除外後の) bookmark が
+/// 見つかった時点で後続の revset を検索しない ("@" で見つかれば "@--" は触らない)。
+///
+/// - `@`: 標準 `git` ブランチ運用、または bookmark が現在のコミット上にある場合
+/// - `@-`: `jj new` で空 `@` を作った直後 (PR #53 / #54 で実測)
+/// - `@--`: 連続 `jj new` や中間空コミット運用向けのフォールバック
+///
+/// cli-merge-pipeline/src/main.rs と同じ設計 (PR #54 で確定)。
+const BOOKMARK_SEARCH_REVSETS: &[&str] = &["@", "@-", "@--"];
+
+/// PR 検出から除外する trunk 系 bookmark。
+/// cli-push-runner/push_jj_bookmark.rs と同じリストを採用。
+const TRUNK_BOOKMARKS: &[&str] = &["main", "master", "trunk", "develop"];
+
+fn is_trunk_bookmark(name: &str) -> bool {
+    TRUNK_BOOKMARKS.contains(&name)
+}
+
+/// 現在の jj change に紐づく全ブックマーク名を取得する。
+///
+/// `BOOKMARK_SEARCH_REVSETS` の順で検索し、最初に非空の結果が得られた revset の
+/// bookmark を返す。すべての revset で空なら空 Vec。
 pub(crate) fn get_jj_bookmarks() -> Vec<String> {
+    select_from_revsets(BOOKMARK_SEARCH_REVSETS, query_bookmarks_at)
+}
+
+/// 指定 revset を優先順に試し、最初に非空の bookmark リストを得た revset の結果を返す。
+/// テスト用に `query` をクロージャで注入できる。
+fn select_from_revsets<F>(revsets: &[&str], query: F) -> Vec<String>
+where
+    F: Fn(&str) -> Vec<String>,
+{
+    for (i, revset) in revsets.iter().enumerate() {
+        let bookmarks = query(revset);
+        if !bookmarks.is_empty() {
+            if i > 0 {
+                log_info(&format!(
+                    "revset '{}' で bookmark を検出: {:?}",
+                    revset, bookmarks
+                ));
+            }
+            return bookmarks;
+        }
+    }
+    Vec::new()
+}
+
+/// 指定 revset の bookmark 名を `jj log` で取得する (I/O)。
+///
+/// stderr は意図的に抑止している (`Stdio::null`)。revset 不正や jj テンプレート
+/// 非互換等の失敗時は空 Vec を返し、呼び出し側 (`select_from_revsets`) が次候補
+/// revset へ fall through する動作。CI ログに jj の警告を大量に残さないためのトレードオフ。
+///
+/// 将来 jj テンプレート DSL の変更等で原因特定が必要になった場合は、一時的に
+/// `.stderr(Stdio::inherit())` に差し替えるか、`!o.status.success()` 分岐で
+/// `log_info` を出力するよう切り替えて調査する。
+fn query_bookmarks_at(revset: &str) -> Vec<String> {
     let output = match Command::new("jj")
         .args([
             "log",
             "-r",
-            "@",
+            revset,
             "--no-graph",
             "-T",
-            "local_bookmarks.map(|b| b.name()).join(\",\")",
+            "local_bookmarks.map(|b| b.name()).join(\",\") ++ \"\\n\"",
         ])
         .stdout(std::process::Stdio::piped())
         .stderr(std::process::Stdio::null())
@@ -101,15 +158,25 @@ pub(crate) fn get_jj_bookmarks() -> Vec<String> {
         _ => return Vec::new(),
     };
 
-    let s = String::from_utf8_lossy(&output.stdout).trim().to_string();
-    if s.is_empty() {
-        return Vec::new();
-    }
+    parse_bookmark_list_output(&String::from_utf8_lossy(&output.stdout))
+}
 
-    s.split(',')
-        .map(|b| b.trim().to_string())
-        .filter(|b| !b.is_empty())
-        .collect()
+/// `jj log` テンプレート出力 (カンマ区切り × 行) からユニークな bookmark 名を抽出する。
+/// trunk 系 bookmark (master/main/trunk/develop) は PR 検索対象から除外する。
+fn parse_bookmark_list_output(raw: &str) -> Vec<String> {
+    let mut seen = Vec::new();
+    for line in raw.lines() {
+        for name in line.split(',').map(str::trim).filter(|s| !s.is_empty()) {
+            if is_trunk_bookmark(name) {
+                continue;
+            }
+            let name = name.to_string();
+            if !seen.contains(&name) {
+                seen.push(name);
+            }
+        }
+    }
+    seen
 }
 
 /// epoch seconds を ISO 8601 UTC 文字列に変換する (std のみ, chrono 不要)
@@ -193,4 +260,110 @@ mod tests {
     fn parse_pr_url_empty() {
         assert_eq!(parse_pr_number_from_url(""), None);
     }
+
+    // ─── bookmark 検出ロジック (cli-merge-pipeline と同仕様) ───
+
+    #[test]
+    fn parse_bookmark_list_output_empty() {
+        assert!(parse_bookmark_list_output("").is_empty());
+        assert!(parse_bookmark_list_output("\n\n").is_empty());
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_single() {
+        assert_eq!(parse_bookmark_list_output("feat/x\n"), vec!["feat/x"]);
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_csv_on_one_line() {
+        assert_eq!(
+            parse_bookmark_list_output("feat/a,feat/b\n"),
+            vec!["feat/a", "feat/b"]
+        );
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_multiple_lines() {
+        let raw = "feat/current\nfeat/parent\n";
+        assert_eq!(
+            parse_bookmark_list_output(raw),
+            vec!["feat/current", "feat/parent"]
+        );
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_deduplicates() {
+        let raw = "feat/x,feat/x\nfeat/x\n";
+        assert_eq!(parse_bookmark_list_output(raw), vec!["feat/x"]);
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_trims_whitespace() {
+        assert_eq!(
+            parse_bookmark_list_output("  feat/a ,  feat/b  \n"),
+            vec!["feat/a", "feat/b"]
+        );
+    }
+
+    #[test]
+    fn parse_bookmark_list_output_excludes_trunk_bookmarks() {
+        assert!(parse_bookmark_list_output("master\n").is_empty());
+        assert_eq!(
+            parse_bookmark_list_output("master,feat/x\n"),
+            vec!["feat/x"]
+        );
+    }
+
+    #[test]
+    fn is_trunk_bookmark_known_names_rejected() {
+        assert!(is_trunk_bookmark("main"));
+        assert!(is_trunk_bookmark("master"));
+        assert!(is_trunk_bookmark("trunk"));
+        assert!(is_trunk_bookmark("develop"));
+        assert!(!is_trunk_bookmark("feat/x"));
+        assert!(!is_trunk_bookmark("main-feature"));
+    }
+
+    #[test]
+    fn select_from_revsets_returns_empty_when_all_revsets_empty() {
+        let result = select_from_revsets(&["@", "@-"], |_| Vec::new());
+        assert!(result.is_empty());
+    }
+
+    #[test]
+    fn select_from_revsets_prefers_current_over_parent() {
+        let result = select_from_revsets(&["@", "@-"], |r| match r {
+            "@" => vec!["feat/current".to_string()],
+            "@-" => vec!["feat/parent".to_string()],
+            _ => Vec::new(),
+        });
+        assert_eq!(result, vec!["feat/current"]);
+    }
+
+    #[test]
+    fn select_from_revsets_falls_back_to_parent_when_current_empty() {
+        // create_pr.rs の --head 自動補完ケース: @ 空 / @- に feature bookmark
+        let result = select_from_revsets(&["@", "@-"], |r| match r {
+            "@" => Vec::new(),
+            "@-" => vec!["feat/parent".to_string()],
+            _ => Vec::new(),
+        });
+        assert_eq!(result, vec!["feat/parent"]);
+    }
+
+    #[test]
+    fn select_from_revsets_stops_at_first_hit() {
+        use std::cell::RefCell;
+        let calls = RefCell::new(Vec::<String>::new());
+        let result = select_from_revsets(&["@", "@-", "@--"], |r| {
+            calls.borrow_mut().push(r.to_string());
+            if r == "@-" {
+                vec!["feat/hit".to_string()]
+            } else {
+                Vec::new()
+            }
+        });
+        assert_eq!(result, vec!["feat/hit"]);
+        assert_eq!(*calls.borrow(), vec!["@".to_string(), "@-".to_string()]);
+    }
 }