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
54 changes: 54 additions & 0 deletions docs/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,60 @@
- ADR-022 (責務分離): takt は commit 操作禁止、Rust 側が担当
- PR #44 の事故事例: 元 description の破壊で得た教訓

### 5. cli-pr-monitor の auto re-push に bookmark 自動前進を移植

- **やろうとしたこと**: takt 自動修正後の auto re-push で「修正コミットができても bookmark が動かず remote に届かない」問題を解消。cli-push-runner には PR #50 で `push_jj_bookmark.rs` の advance ロジックが入っているが、cli-pr-monitor の `run_push` は `jj new` → `jj git push` だけで bookmark を進めない
- **現在地**: 設計確定、未実装。PR #53 で症状を実測 (CodeRabbit 修正が local commit `466aff1d` に入ったが bookmark が動かず remote 未反映、手動で `jj bookmark set` + `jj git push --bookmark` が必要だった)
- **実装内容**:
- [ ] `src/cli-pr-monitor/src/stages/push.rs:run_push` の `jj git push` 直前に bookmark advance を挿入
- [ ] cli-push-runner の `push_jj_bookmark::advance_jj_bookmarks` を共有化するか、まず port (copy) するかを決定
- [ ] unit テスト: 既存の `decide_repush` テスト群と同じスタイルで bookmark advance の挙動を検証
- [ ] E2E 検証: CodeRabbit Major ありの PR で takt 修正が remote まで自動到達することを確認
- **詰まっている箇所**:
- **共通化方針**: ADR-024 (共通 jj helper crate、試験運用) の延長線で `jj-helpers` クレート化するのが理想だが、まず port から着手して機能等価を確認した方が安全
- **task 4 (コミット分離) との順序**: task 4 を先に実装すると takt fix が「明示的な子コミット」になるため bookmark advance のターゲットが安定する。task 5 → 4 の順か、4 と同時に対応するかを設計確定時に判断
- **参照 ADR / PR**:
- PR #50 (cli-push-runner の bookmark fallback)
- ADR-024 (共通 jj helper、試験運用)
- 関連: task 4 (コミット分離)

### 6. post-pr-review workflow の verdict に push 反映確認を追加

- **やろうとしたこと**: post-pr-review workflow が local file の状態だけを見て `approved` を判定する設計のため、auto re-push が失敗していても `approved` になる gap を埋める。PR #53 で「local 修正済み + bookmark 未前進 → workflow approved」の食い違いを実測
- **現在地**: 設計段階、未着手。task 5 (問題 A) の defense-in-depth 的位置付け
- **実装内容案**:
- [ ] workflow の analyze (or 終了前) ステップで `gh api` を叩いて remote の最新 commit SHA を取得し、local の fix commit SHA と比較
- [ ] 一致しなければ `verdict = action_required` にダウングレードし「remote 未反映」のメッセージを出す
- **詰まっている箇所**:
- **task 5 実装後の優先度再評価**: task 5 が入れば「auto re-push が失敗しない限り approved は妥当」となるため、task 6 の優先度は大きく下がる。実装可否は task 5 完了後に判断
- **takt workflow の YAML から外部コマンド呼び出しが可能か**: facets instruction 内でシェルが呼べるかは要調査 (ADR-019/020 の facets 設計内)
- **参照 ADR**:
- ADR-018 (post-pr-monitor takt 化)
- ADR-019 (CodeRabbit レビュー運用ハイブリッド)
- 関連: task 5 (bookmark auto-advance)

### 7. cli-pr-monitor の bookmark 検出を `@-..@--` まで拡張 (cli-merge-pipeline の水平展開)

- **やろうとしたこと**: [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 で実装パターンが確定済み)
- **実装内容**:
- [ ] [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-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」保証が強まるので挙動は改善方向だが、回帰テストで確認する
- **参照**:
- 先行実装: cli-merge-pipeline の同等対応 (task 7 旧、PR で完了済み)
- ADR-013 (cli-merge-pipeline), ADR-024 (共通 jj helpers、試験運用)

### 8. 雑務: 過去の 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 しないので緊急性なし
- **詰まっている箇所**: なし

---

## スコープ外だが将来検討
Expand Down
199 changes: 188 additions & 11 deletions src/cli-merge-pipeline/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,64 @@ fn run_gh_logged(args: &[&str]) -> Option<String> {
}
}

/// 現在の jj change に紐づく全ブックマーク名を取得する
/// Bookmark 検索に使用する revset のリスト (近い順 = 優先順)。
///
/// `select_from_revsets` は先頭から順に試し、最初に (trunk 除外後の) bookmark が
/// 見つかった時点で後続の revset を検索しない ("@" で見つかれば "@--" は触らない)。
///
/// - `@`: 標準 `git` ブランチ運用、または bookmark が現在のコミット上にある場合
/// - `@-`: `jj new` で空 `@` を作った直後 (PR #53 で実測)
/// - `@--`: 連続 `jj new` や中間空コミット運用向けのフォールバック
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。
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)。
fn query_bookmarks_at(revset: &str) -> Vec<String> {
let output = match Command::new("jj")
.args(["log", "-r", "@", "--no-graph", "-T", "local_bookmarks.map(|b| b.name()).join(\",\")"])
.args([
"log",
"-r",
revset,
"--no-graph",
"-T",
"local_bookmarks.map(|b| b.name()).join(\",\") ++ \"\\n\"",
])
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.output()
Expand All @@ -229,7 +283,10 @@ fn get_jj_bookmarks() -> Vec<String> {
Ok(o) => {
let stderr = String::from_utf8_lossy(&o.stderr).trim().to_string();
if !stderr.is_empty() {
log_info(&format!("jj bookmark 取得失敗: {}", stderr));
log_info(&format!(
"jj bookmark 取得失敗 (revset={}): {}",
revset, stderr
));
}
return Vec::new();
}
Expand All @@ -239,15 +296,25 @@ fn get_jj_bookmarks() -> Vec<String> {
}
};

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
}

/// 現在のブックマークから PR 番号を検出する
Expand Down Expand Up @@ -596,4 +663,114 @@ step_timeout = 60
fn combine_output_both_empty() {
assert_eq!(combine_output("", ""), "");
}

// ─── bookmark 検出ロジック ───

#[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() {
// @ と @- の両方にヒットした場合 (将来 revset を両方カバーする場合向け)
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() {
// fresh checkout で @- が master を指すケース (option B の注意点)
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() {
// @ と @- の両方に bookmark があるとき、近い @ を優先する
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() {
// PR #53 で実測した「@ 空, @- に 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() {
// 優先度の低い revset は検索されない (副作用が発生しないことを確認)
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()]);
}
}