diff --git a/docs/adr/adr-029-post-merge-feedback-auto-trigger.md b/docs/adr/adr-029-post-merge-feedback-auto-trigger.md index 087018f..bb8a236 100644 --- a/docs/adr/adr-029-post-merge-feedback-auto-trigger.md +++ b/docs/adr/adr-029-post-merge-feedback-auto-trigger.md @@ -87,7 +87,8 @@ Claude がメイン会話内で /post-merge-feedback を起動 "status": "pending", "created_at": "2026-04-23T10:00:00Z", "dispatched_at": null, - "consumed_at": null + "consumed_at": null, + "producer": "cli-merge-pipeline@pid-1234@2026-04-23T10:00:00Z" } ``` @@ -103,6 +104,7 @@ Claude がメイン会話内で /post-merge-feedback を起動 | `created_at` | ISO 8601 UTC string | yes | cli-merge-pipeline が書き込んだ時刻 | | `dispatched_at` | ISO 8601 UTC string | nullable | hooks-stop-feedback-dispatch が additionalContext を出した時刻 | | `consumed_at` | ISO 8601 UTC string | nullable | skill が完了処理を行った時刻 | +| `producer` | string | optional | 書き込み元の識別子 (`cli-merge-pipeline@pid-{pid}@{iso8601}` 形式)。取りこぼし発生時の追跡用。schema v1 互換 (既存 reader は欠損 / 未知値を無視) | ### 状態遷移 @@ -130,12 +132,24 @@ cli-merge-pipeline が pending file を書き込もうとしたときの既存 | 既存 status | 挙動 | |---|---| -| 不在 | 新規書き込み (通常経路) | -| `consumed` (削除忘れ) | 上書き | +| 不在 | 新規書き込み (通常経路; `create_new` で atomic 排他作成) | +| `consumed` (削除忘れ) | 削除後に上書き | | `pending` / `dispatched` | **書き込み skip + WARN** (ステップ自体は PASS 扱いで merge-pr を中断しない) | -| 破損 (size 0 / JSON parse 失敗 / schema_version 不一致) | 削除してから書き込み | +| 破損 (size 0 / JSON parse 失敗 / schema_version 不一致 / 未知 status) | 削除後に上書き | -同一セッション内で短時間に複数 PR をマージした場合 (現実には稀)、最初の pending が consume されるまで後続は取りこぼす。取りこぼしは WARN ログで可観測性を残すことで後追い対応可能とする。 +**排他性保証 (TOCTOU 対策)**: + +新規書き込み経路では `OpenOptions::new().write(true).create_new(true).open(path)` (O_EXCL 相当) で最終ファイルを直接 atomic 排他作成する。`read_existing` と書き込みが分離していても、2 プロセスが同時に `None` を観測した場合は OS 層で一方のみ成功、他方は `AlreadyExists` で弾かれて **WARN ログを残す**。これにより「取りこぼしは可視」という本 ADR の保証が実装レベルで満たされる。 + +**tmp → rename は新規作成経路では使わない** — `rename` は既存を無条件上書きするため、placeholder 方式 (`create_new` で空ファイル予約 → `rename` で置換) だと排他予約が自壊する (他プロセスが後から rename で上書き可能)。PR #70 のレビューで指摘・修正済。 + +上書き経路 (既存 `Consumed` / 破損 を削除後の書き込み) の read→write 間 race は許容する。`Consumed` は skill バグ時、破損は schema drift 時のみの稀経路で、発生しても破損ポリシー (size=0 / parse 失敗 → 削除 → silent exit) で自己回復する。 + +**中間状態の可観測性**: 新規作成経路は `create_new` で直接書くため、`write_all` 完了前の reader が size=0 or 部分書き込みを観測し得る。hooks-stop-feedback-dispatch の破損ポリシーが吸収する設計判断 (**完全性より排他性を優先**)。 + +**producer 追跡**: pending file には `producer: Option` (`cli-merge-pipeline@pid-{pid}@{iso8601}` 形式) を含む。取りこぼし発生時に「誰が書いた pending が消えたか」を後追いできる観測性補助。schema_version は v1 のまま (未知フィールドは既存 reader が無視するため bump 不要)。 + +同一セッション内で短時間に複数 PR をマージした場合 (現実には稀)、最初の pending が consume されるまで後続は取りこぼす。取りこぼしは `create_new AlreadyExists` か `Active → skip` の WARN ログで観測可能。 **将来拡張**: 取りこぼしが問題化したらディレクトリベースのキュー (`.claude/post-merge-feedback/.json`) への移行を検討。現段階では YAGNI で単一ファイルを採用する。 @@ -155,14 +169,21 @@ hooks-stop-feedback-dispatch が pending file を読み取る際の分岐表: | `status == "dispatched"` | silent exit (二重通知しない) | | `status == "consumed"` | ファイル削除 + silent exit | -**書き込み方式**: 「一時ファイルに write → `fs::rename` で atomic rename」の 2 段階を常に使う。ロックファイルは不要。 +**書き込み方式** (2026-04-23 改訂): + +| 経路 | 方式 | 排他保証 | +|---|---|---| +| 新規作成 (既存不在) | `OpenOptions::new().write(true).create_new(true).open(path)` で直接書き込み + `sync_all` | OS の O_EXCL / CREATE_NEW による atomic 排他作成 | +| 上書き (Consumed / 破損 削除後) | 一時ファイルに write → `fs::rename` で atomic overwrite | fs::rename の atomic overwrite | + +**ロックファイルは不要** (create_new が OS 層で atomic 排他を保証するため)。placeholder + rename 方式は採用しない (rename の無条件上書きで排他予約が自壊するため)。 ただし atomic 保証はプラットフォームとファイルシステムに依存する: | 環境 | `std::fs::rename` の atomicity | |---|---| -| POSIX (Linux / macOS 等) | `rename(2)` により atomic overwrite (同一ファイルシステム内) | -| Windows 10 1607+ / NTFS or ReFS | `FileRenameInfoEx` + `FILE_RENAME_FLAG_POSIX_SEMANTICS` 経路が成功すれば atomic overwrite。本プロジェクトのターゲット (Windows 11 + NTFS) はこの範囲 | +| POSIX (Linux / macOS 等) | `open(O_EXCL)` / `rename(2)` により atomic (同一ファイルシステム内) | +| Windows 10 1607+ / NTFS or ReFS | `CREATE_NEW` (O_EXCL 相当) / `FileRenameInfoEx` + `FILE_RENAME_FLAG_POSIX_SEMANTICS` 経路が成功すれば atomic。本プロジェクトのターゲット (Windows 11 + NTFS) はこの範囲 | | 旧 Windows / 非対応 FS | `FileRenameInfo` に fallback し **atomic 保証なし**。他プロセスが中間状態を観測する可能性がある | Rust 側の実装順序は rust-lang/rust の [#131072](https://github.com/rust-lang/rust/pull/131072) / [#138133](https://github.com/rust-lang/rust/pull/138133) で 2024-2025 に変更されており、信頼性のため non-atomic を先に試行、失敗時のみ POSIX semantics 版へ fallback する挙動になっている点にも留意。 diff --git a/src/cli-merge-pipeline/src/main.rs b/src/cli-merge-pipeline/src/main.rs index bd8a515..e946f55 100644 --- a/src/cli-merge-pipeline/src/main.rs +++ b/src/cli-merge-pipeline/src/main.rs @@ -15,7 +15,10 @@ //! 1 - マージ失敗 / PR 検出失敗 //! 2 - 設定エラー -use lib_jj_helpers::{StderrMode, get_jj_bookmarks as lib_get_jj_bookmarks}; +mod pending_file; + +use lib_jj_helpers::{get_jj_bookmarks as lib_get_jj_bookmarks, StderrMode}; +use pending_file::{ExistingPending, PendingFile}; use serde::Deserialize; use std::path::{Path, PathBuf}; use std::process::Command; @@ -48,6 +51,56 @@ struct PipelineStepConfig { prompt: Option, } +/// `gh pr view --json headRefName,isCrossRepository` のレスポンス +/// +/// fork PR では `is_cross_repository == true` となり、upstream repo の +/// 同名ブランチを誤削除しないようにリモートブランチ削除をスキップする。 +#[derive(Deserialize)] +struct PrHeadInfo { + #[serde(rename = "headRefName")] + head_ref_name: String, + #[serde(rename = "isCrossRepository")] + is_cross_repository: bool, +} + +/// fork PR かどうかを判定し、リモートブランチ削除をスキップすべきか返す。 +/// +/// fork PR では `isCrossRepository == true` になるため、upstream repo の +/// 同名 ref への DELETE を防ぐ。 +fn should_skip_branch_delete(info: &PrHeadInfo) -> bool { + info.is_cross_repository +} + +/// RFC 3986 の unreserved characters (`A-Z a-z 0-9 - _ . ~`) 以外を percent-encode する。 +/// +/// `gh api` の URL path segment に branch 名等を埋め込む際の安全弁。 +/// `replace('/', "%2F")` だけでは `?` `#` `+` 等の特殊文字が素通りするため、 +/// CodeRabbit PR #70 指摘 (Major) を受けて全特殊文字を encode する実装に置換した。 +/// 実運用では git branch 命名規則によりほとんどの特殊文字は出現しないが、 +/// defense-in-depth として汎用 helper を提供する。 +fn percent_encode_path_segment(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + if matches!(b, b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~') { + out.push(b as char); + } else { + out.push_str(&format!("%{:02X}", b)); + } + } + out +} + +/// パイプライン実行時に post_steps へ渡すコンテキスト (ADR-029) +/// +/// pre_steps は PR 検出前 or 検出直後に走るため `None` を渡す (後方互換)。 +/// post_steps では PR 検出済みなので `Some(&PipelineContext)` を渡す。 +#[derive(Debug, Clone)] +struct PipelineContext { + pr_number: u64, + /// `{owner}/{repo}` 形式。`gh repo view` で取得できなかった場合は `None`。 + owner_repo: Option, +} + /// デフォルトのブランチ名 const DEFAULT_BRANCH: &str = "master"; @@ -174,19 +227,28 @@ fn combine_output(stdout: &str, stderr: &str) -> String { // ─── 設定ファイル読み込み ─── fn config_path() -> PathBuf { + config_dir().join("hooks-config.toml") +} + +/// exe と設定・pending file を配置するディレクトリ (`.claude/`)。 +fn config_dir() -> PathBuf { std::env::current_exe() .unwrap_or_default() .parent() .unwrap_or(Path::new(".")) - .join("hooks-config.toml") + .to_path_buf() } fn load_config() -> Result { let path = config_path(); - let content = std::fs::read_to_string(&path) - .map_err(|e| format!("hooks-config.toml の読み込みに失敗: {} ({})", path.display(), e))?; - toml::from_str(&content) - .map_err(|e| format!("hooks-config.toml のパースに失敗: {}", e)) + let content = std::fs::read_to_string(&path).map_err(|e| { + format!( + "hooks-config.toml の読み込みに失敗: {} ({})", + path.display(), + e + ) + })?; + toml::from_str(&content).map_err(|e| format!("hooks-config.toml のパースに失敗: {}", e)) } // ─── PR 検出 (cli-pr-monitor から移植) ─── @@ -208,7 +270,11 @@ fn run_gh_logged(args: &[&str]) -> Option { if output.status.success() { let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); - if s.is_empty() { None } else { Some(s) } + if s.is_empty() { + None + } else { + Some(s) + } } else { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); if !stderr.is_empty() { @@ -229,6 +295,18 @@ fn get_jj_bookmarks() -> Vec { lib_get_jj_bookmarks(StderrMode::Piped(log_info), Some(log_info)) } +/// 現在のリポジトリの `{owner}/{repo}` を検出する (ADR-029) +fn detect_owner_repo() -> Option { + run_gh_logged(&[ + "repo", + "view", + "--json", + "nameWithOwner", + "-q", + ".nameWithOwner", + ]) +} + /// 現在のブックマークから PR 番号を検出する fn detect_pr_number() -> Option { // Strategy A: gh pr view (git ブランチが使える場合) @@ -246,7 +324,14 @@ fn detect_pr_number() -> Option { // まず OPEN な PR を検索 let pr_number = run_gh_logged(&[ - "pr", "list", "--head", bookmark, "--json", "number", "-q", ".[0].number", + "pr", + "list", + "--head", + bookmark, + "--json", + "number", + "-q", + ".[0].number", ]) .and_then(|s| s.parse::().ok()); @@ -256,8 +341,16 @@ fn detect_pr_number() -> Option { // OPEN がなければ全状態で検索（マージ済み PR のローカル同期用） let pr_number = run_gh_logged(&[ - "pr", "list", "--head", bookmark, "--state", "all", - "--json", "number", "-q", ".[0].number", + "pr", + "list", + "--head", + bookmark, + "--state", + "all", + "--json", + "number", + "-q", + ".[0].number", ]) .and_then(|s| s.parse::().ok()); @@ -271,8 +364,16 @@ fn detect_pr_number() -> Option { // ─── ステップ実行 ─── -/// ステップリストを順次実行する。失敗時は Err(exit_code) を返す -fn run_steps(phase: &str, steps: &[PipelineStepConfig], timeout: u64) -> Result<(), i32> { +/// ステップリストを順次実行する。失敗時は Err(exit_code) を返す。 +/// +/// `ctx` は post_steps の AI ステップで必要になるコンテキスト (ADR-029)。 +/// pre_steps は `None` を渡す (後方互換)。 +fn run_steps( + phase: &str, + steps: &[PipelineStepConfig], + timeout: u64, + ctx: Option<&PipelineContext>, +) -> Result<(), i32> { if steps.is_empty() { return Ok(()); } @@ -311,15 +412,10 @@ fn run_steps(phase: &str, steps: &[PipelineStepConfig], timeout: u64) -> Result< } } "ai" => { - let prompt = step.prompt.as_deref().unwrap_or("(未定義)"); - log_step( - &label, - "SKIP", - &format!( - "AI ステップ (prompt: {}) — 将来実装予定。現在はスキップします。", - prompt - ), - ); + // ADR-029: pending file を書き込んで Stop hook 経由で skill を起動する。 + // 失敗しても WARN + PASS 扱い (merge 本体は完了済みなので pipeline を止めない)。 + let pending_path = pending_file::default_path(&config_dir()); + run_ai_step(&label, step, ctx, &pending_path); } unknown => { log_step( @@ -334,6 +430,227 @@ fn run_steps(phase: &str, steps: &[PipelineStepConfig], timeout: u64) -> Result< Ok(()) } +/// 書き込み経路の種別 (ADR-029 §競合ポリシー)。 +/// +/// - `NewExclusive`: 既存 pending 不在 → `create_new` で atomic 排他作成 (race 検出可能) +/// - `Overwrite`: 既存 Consumed/Corrupt を削除後の上書き (tmp → rename、race は許容) +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum WriteMode { + NewExclusive, + Overwrite, +} + +impl WriteMode { + fn label(self) -> &'static str { + match self { + WriteMode::NewExclusive => "NewExclusive", + WriteMode::Overwrite => "Overwrite", + } + } +} + +/// 既存の pending file の状態から書き込み経路を決定する (ADR-029 §競合ポリシー)。 +/// +/// Ok(mode) → 書き込みを続行する (mode に応じて write_new_exclusive / write_overwrite)。 +/// Err(()) → スキップ (ログ済み)。 +fn determine_write_mode(label: &str, pending_path: &Path) -> Result { + match pending_file::read_existing(pending_path) { + ExistingPending::None => Ok(WriteMode::NewExclusive), + ExistingPending::Consumed => { + log_info(&format!( + "[{}] 既存 pending は status='consumed' — 削除して上書き", + label + )); + remove_existing_pending(label, pending_path).map(|()| WriteMode::Overwrite) + } + ExistingPending::Corrupt(reason) => { + // reason は pending file 由来で制御文字を含み得るため {:?} でエスケープ (CodeRabbit Minor) + log_info(&format!( + "[{}] 既存 pending が破損 ({:?}) — 削除して上書き", + label, reason + )); + remove_existing_pending(label, pending_path).map(|()| WriteMode::Overwrite) + } + ExistingPending::Active(status) => { + log_step( + label, + "WARN", + &format!( + "既存 pending が status={:?} — 新規書き込みをスキップ (取りこぼしは ADR-029 将来拡張で対応)", + status + ), + ); + Err(()) + } + } +} + +fn remove_existing_pending(label: &str, path: &Path) -> Result<(), ()> { + std::fs::remove_file(path).map_err(|e| { + log_step( + label, + "WARN", + &format!("既存 pending の削除失敗: {} — 続行不可のためスキップ", e), + ); + }) +} + +fn write_pending_and_log(label: &str, pending: &PendingFile, pending_path: &Path, mode: WriteMode) { + let result = match mode { + WriteMode::NewExclusive => pending_file::write_new_exclusive(pending_path, pending), + WriteMode::Overwrite => pending_file::write_overwrite(pending_path, pending), + }; + match result { + Ok(()) => log_step( + label, + "PASS", + &format!( + "pending file 書き込み完了: {} (PR #{}, prompt={}, mode={})", + pending_path.display(), + pending.pr_number, + pending.prompt, + mode.label() + ), + ), + Err(pending_file::WriteError::AlreadyExists) => log_step( + label, + "WARN", + "別プロセスが同時に pending を書き込みました (create_new AlreadyExists) — 本プロセスの書き込みをスキップ (取りこぼしを WARN で観測)", + ), + // merge 本体は完了済みなので WARN にとどめ、次のマージで復帰可能とする (ADR-029 §破損耐性) + Err(e) => log_step( + label, + "WARN", + &format!( + "pending file 書き込み失敗: {} — merge 完了済みのため続行", + e + ), + ), + } +} + +/// `run_ai_step` の入力ガード: PipelineContext の存在・owner_repo の存在・形式を確認する。 +/// +/// Ok((pr_number, owner_repo)) → 書き込みを続行できる。 +/// Err(()) → スキップ (ログ済み)。 +fn validate_ai_step_context<'a>( + label: &str, + ctx: Option<&'a PipelineContext>, +) -> Result<(u64, &'a str), ()> { + let Some(ctx) = ctx else { + log_step( + label, + "SKIP", + "PipelineContext 未指定 (pre_steps 経路) — AI ステップは post_steps 専用です", + ); + return Err(()); + }; + + let Some(owner_repo) = ctx.owner_repo.as_deref() else { + log_step( + label, + "WARN", + "owner_repo を取得できませんでした (gh repo view 失敗?) — pending file を書き込まずスキップ", + ); + return Err(()); + }; + + if !pending_file::is_valid_owner_repo(owner_repo) { + log_step( + label, + "WARN", + &format!( + "owner_repo {:?} の形式が不正 — pending file を書き込まずスキップ", + owner_repo + ), + ); + return Err(()); + } + + Ok((ctx.pr_number, owner_repo)) +} + +/// post-merge の `type = "ai"` ステップを実行する (ADR-029)。 +/// +/// 戻り値はなし: どの分岐も PASS 扱いでステップを継続させる (pipeline を止めない)。 +/// 具体的な挙動は ADR-029 §競合ポリシー / §破損耐性 に従う。 +fn run_ai_step( + label: &str, + step: &PipelineStepConfig, + ctx: Option<&PipelineContext>, + pending_path: &Path, +) { + let prompt = step + .prompt + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + .unwrap_or("post-merge-feedback"); + + let Ok((pr_number, owner_repo)) = validate_ai_step_context(label, ctx) else { + return; + }; + + let Ok(mode) = determine_write_mode(label, pending_path) else { + return; + }; + + let pending = PendingFile { + schema_version: pending_file::SCHEMA_VERSION, + pr_number, + owner_repo: owner_repo.to_string(), + prompt: prompt.to_string(), + status: pending_file::STATUS_PENDING.to_string(), + created_at: pending_file::utc_now_iso8601(), + dispatched_at: None, + consumed_at: None, + producer: Some(pending_file::producer_string()), + }; + + write_pending_and_log(label, &pending, pending_path, mode); +} + +fn delete_remote_branch(branch_name: &str) { + let encoded_branch = percent_encode_path_segment(branch_name); + let ref_path = format!("repos/{{owner}}/{{repo}}/git/refs/heads/{}", encoded_branch); + let gh_output = Command::new("gh") + .args(["api", &ref_path, "-X", "DELETE"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output(); + let (del_ok, del_out) = match gh_output { + Ok(o) => { + let combined = combine_output( + String::from_utf8_lossy(&o.stdout).trim(), + String::from_utf8_lossy(&o.stderr).trim(), + ); + (o.status.success(), combined) + } + Err(e) => (false, format!("gh コマンド実行失敗: {}", e)), + }; + if del_ok { + log_info(&format!( + "リモートブランチ '{}' を削除しました", + branch_name + )); + } else if del_out.contains("Reference does not exist") { + log_info(&format!( + "リモートブランチ '{}' は既に削除済みです（GitHub による自動削除）", + branch_name + )); + } else { + let msg = if del_out.is_empty() { + "不明なエラー".to_string() + } else { + del_out + }; + log_info(&format!( + "リモートブランチ '{}' の削除失敗: {}", + branch_name, msg + )); + } +} + // ─── パイプライン実行 ─── fn run_pipeline() -> i32 { @@ -372,16 +689,39 @@ fn run_pipeline() -> i32 { } }; + // PipelineContext を構築 (post_steps の AI ステップ用; ADR-029) + let owner_repo = detect_owner_repo(); + if owner_repo.is_none() { + log_info( + "警告: owner_repo を検出できませんでした (gh repo view 失敗)。post_steps の AI ステップは pending file を書き込めずスキップします。", + ); + } + let ctx = PipelineContext { + pr_number, + owner_repo, + }; + // PR の状態を確認（マージ可能か） log_info("PR の状態を確認中..."); - let pr_state = run_gh_logged(&["pr", "view", &pr_number.to_string(), "--json", "state", "-q", ".state"]); + let pr_state = run_gh_logged(&[ + "pr", + "view", + &pr_number.to_string(), + "--json", + "state", + "-q", + ".state", + ]); match pr_state.as_deref() { Some("MERGED") => { log_info("この PR は既にマージ済みです。ローカル同期のみ実行します。"); let rc = sync_local(&branch); - if rc != 0 { return rc; } + if rc != 0 { + return rc; + } // マージ済みでも post_steps は実行する（学び提案等） - if let Err(code) = run_steps("post-merge ステップ", &post_steps, timeout) { + if let Err(code) = run_steps("post-merge ステップ", &post_steps, timeout, Some(&ctx)) + { return code; } return 0; @@ -396,8 +736,8 @@ fn run_pipeline() -> i32 { } } - // pre-merge ステップ実行 - if let Err(code) = run_steps("pre-merge ステップ", &pre_steps, timeout) { + // pre-merge ステップ実行 (AI ステップは post_steps 専用のため ctx=None) + if let Err(code) = run_steps("pre-merge ステップ", &pre_steps, timeout, None) { return code; } @@ -421,37 +761,36 @@ fn run_pipeline() -> i32 { log_info("マージ完了"); // リモートブランチを削除 (gh api) - let head_branch = run_gh_logged(&[ - "pr", "view", &pr_number.to_string(), - "--json", "headRefName", "-q", ".headRefName", + // fork PR の場合は isCrossRepository == true になるため、upstream repo の + // 同名ブランチを誤削除しないようにスキップする。 + let head_info_json = run_gh_logged(&[ + "pr", + "view", + &pr_number.to_string(), + "--json", + "headRefName,isCrossRepository", ]); - if let Some(ref branch_name) = head_branch { - let encoded_branch = branch_name.replace('/', "%2F"); - let delete_cmd = format!( - "gh api repos/{{owner}}/{{repo}}/git/refs/heads/{} -X DELETE", - encoded_branch - ); - let (del_ok, del_out) = run_cmd("delete-branch", &delete_cmd, 30); - if del_ok { - log_info(&format!("リモートブランチ '{}' を削除しました", branch_name)); - } else if del_out.contains("Reference does not exist") { - log_info(&format!("リモートブランチ '{}' は既に削除済みです（GitHub による自動削除）", branch_name)); - } else { - let msg = if del_out.is_empty() { - "不明なエラー".to_string() - } else { - del_out - }; - log_info(&format!("リモートブランチ '{}' の削除失敗: {}", branch_name, msg)); + if let Some(ref json) = head_info_json { + match serde_json::from_str::(json) { + Err(e) => log_info(&format!("PR head 情報のパース失敗: {}", e)), + Ok(info) if should_skip_branch_delete(&info) => { + log_info(&format!( + "fork PR のためリモートブランチ '{}' の削除をスキップします", + info.head_ref_name + )); + } + Ok(info) => delete_remote_branch(&info.head_ref_name), } } // ローカル同期 let rc = sync_local(&branch); - if rc != 0 { return rc; } + if rc != 0 { + return rc; + } - // post-merge ステップ実行（学び提案等） - if let Err(code) = run_steps("post-merge ステップ", &post_steps, timeout) { + // post-merge ステップ実行（学び提案等; AI ステップは PipelineContext 必須） + if let Err(code) = run_steps("post-merge ステップ", &post_steps, timeout, Some(&ctx)) { return code; } @@ -481,7 +820,10 @@ fn sync_local(branch: &str) -> i32 { return 1; } - log_info(&format!("ローカル同期完了。{} の最新状態で作業を開始できます。", branch)); + log_info(&format!( + "ローカル同期完了。{} の最新状態で作業を開始できます。", + branch + )); 0 } @@ -541,11 +883,31 @@ prompt = "analyze_pr_learnings" assert!(pipeline.pre_steps.unwrap_or_default().is_empty()); assert!(pipeline.post_steps.unwrap_or_default().is_empty()); assert_eq!( - pipeline.default_branch.unwrap_or_else(|| DEFAULT_BRANCH.to_string()), + pipeline + .default_branch + .unwrap_or_else(|| DEFAULT_BRANCH.to_string()), DEFAULT_BRANCH ); } + #[test] + fn should_skip_branch_delete_true_for_fork_pr() { + let info = PrHeadInfo { + head_ref_name: "feature-branch".to_string(), + is_cross_repository: true, + }; + assert!(should_skip_branch_delete(&info)); + } + + #[test] + fn should_skip_branch_delete_false_for_same_repo_pr() { + let info = PrHeadInfo { + head_ref_name: "feature-branch".to_string(), + is_cross_repository: false, + }; + assert!(!should_skip_branch_delete(&info)); + } + #[test] fn config_missing_merge_pipeline_section() { let toml_str = r#" @@ -576,10 +938,364 @@ step_timeout = 60 assert_eq!(combine_output("", ""), ""); } + // ─── percent_encode_path_segment (CodeRabbit PR #70 Major 対応) ─── + + #[test] + fn percent_encode_passes_unreserved_chars() { + // RFC 3986 unreserved: A-Z a-z 0-9 - _ . ~ + assert_eq!( + percent_encode_path_segment("abcXYZ-_0123.~"), + "abcXYZ-_0123.~" + ); + } + + #[test] + fn percent_encode_slash_and_special_chars() { + assert_eq!(percent_encode_path_segment("feat/foo"), "feat%2Ffoo"); + assert_eq!(percent_encode_path_segment("a?b#c"), "a%3Fb%23c"); + assert_eq!(percent_encode_path_segment("x+y&z=w"), "x%2By%26z%3Dw"); + assert_eq!(percent_encode_path_segment("has space"), "has%20space"); + } + + #[test] + fn percent_encode_multibyte_utf8() { + // "日" = 0xE6 0x97 0xA5 in UTF-8 + assert_eq!(percent_encode_path_segment("日"), "%E6%97%A5"); + } + + #[test] + fn percent_encode_empty_string() { + assert_eq!(percent_encode_path_segment(""), ""); + } + + // ─── should_skip_branch_delete ─── + + #[test] + fn skip_delete_when_cross_repository() { + let info = PrHeadInfo { + head_ref_name: "feat-x".into(), + is_cross_repository: true, + }; + assert!(should_skip_branch_delete(&info)); + } + + #[test] + fn delete_allowed_when_same_repository() { + let info = PrHeadInfo { + head_ref_name: "feat-x".into(), + is_cross_repository: false, + }; + assert!(!should_skip_branch_delete(&info)); + } + // ─── bookmark 検出ロジック (lib-jj-helpers に集約済) ─── // // TRUNK_BOOKMARKS / BOOKMARK_SEARCH_REVSETS / parse_bookmark_list_output / // select_from_revsets / query_bookmarks_at / get_jj_bookmarks の unit test は // lib-jj-helpers/src/lib.rs#tests に集約 (ADR-024 本採用、PR-C で移設)。 // cli-merge-pipeline 側からは lib_jj_helpers の公開 API 経由でのみ使用する。 + + // ─── AI step (ADR-029) ─── + + fn unique_tmp_pending(label: &str) -> PathBuf { + std::env::temp_dir().join(format!( + "cli-merge-ai-{}-{}-{}.json", + label, + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )) + } + + fn ai_step(prompt: Option<&str>) -> PipelineStepConfig { + PipelineStepConfig { + name: "post_merge_feedback".to_string(), + step_type: "ai".to_string(), + cmd: None, + prompt: prompt.map(str::to_string), + } + } + + fn read_pending(path: &Path) -> PendingFile { + let content = std::fs::read_to_string(path).expect("pending file should exist"); + serde_json::from_str(&content).expect("pending file should parse") + } + + #[test] + fn ai_step_writes_pending_when_ctx_present() { + let path = unique_tmp_pending("writes-pending"); + let ctx = PipelineContext { + pr_number: 123, + owner_repo: Some("aloekun/claude-code-hook-test".to_string()), + }; + let step = ai_step(Some("post-merge-feedback")); + + run_ai_step("test", &step, Some(&ctx), &path); + + let loaded = read_pending(&path); + assert_eq!(loaded.schema_version, pending_file::SCHEMA_VERSION); + assert_eq!(loaded.pr_number, 123); + assert_eq!(loaded.owner_repo, "aloekun/claude-code-hook-test"); + assert_eq!(loaded.prompt, "post-merge-feedback"); + assert_eq!(loaded.status, pending_file::STATUS_PENDING); + assert!(loaded.dispatched_at.is_none()); + assert!(loaded.consumed_at.is_none()); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_uses_default_prompt_when_not_set() { + let path = unique_tmp_pending("default-prompt"); + let ctx = PipelineContext { + pr_number: 1, + owner_repo: Some("o/r".to_string()), + }; + let step = ai_step(None); + + run_ai_step("test", &step, Some(&ctx), &path); + + assert_eq!(read_pending(&path).prompt, "post-merge-feedback"); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_skips_when_ctx_none() { + let path = unique_tmp_pending("ctx-none"); + let step = ai_step(Some("post-merge-feedback")); + + run_ai_step("test", &step, None, &path); + + assert!(!path.exists(), "pending file should not be created"); + } + + #[test] + fn ai_step_skips_when_owner_repo_none() { + let path = unique_tmp_pending("owner-repo-none"); + let ctx = PipelineContext { + pr_number: 42, + owner_repo: None, + }; + let step = ai_step(Some("post-merge-feedback")); + + run_ai_step("test", &step, Some(&ctx), &path); + + assert!(!path.exists()); + } + + #[test] + fn ai_step_skips_when_owner_repo_invalid() { + let path = unique_tmp_pending("owner-repo-invalid"); + let ctx = PipelineContext { + pr_number: 42, + owner_repo: Some("has space/repo".to_string()), + }; + let step = ai_step(Some("post-merge-feedback")); + + run_ai_step("test", &step, Some(&ctx), &path); + + assert!(!path.exists()); + } + + #[test] + fn ai_step_overwrites_consumed_pending() { + let path = unique_tmp_pending("overwrite-consumed"); + let consumed = PendingFile { + schema_version: pending_file::SCHEMA_VERSION, + pr_number: 999, + owner_repo: "old/repo".to_string(), + prompt: "post-merge-feedback".to_string(), + status: pending_file::STATUS_CONSUMED.to_string(), + created_at: "2026-04-01T00:00:00Z".to_string(), + dispatched_at: Some("2026-04-01T00:01:00Z".to_string()), + consumed_at: Some("2026-04-01T00:02:00Z".to_string()), + producer: None, + }; + pending_file::write_new_exclusive(&path, &consumed).unwrap(); + + let ctx = PipelineContext { + pr_number: 555, + owner_repo: Some("new/repo".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + let loaded = read_pending(&path); + assert_eq!(loaded.pr_number, 555); + assert_eq!(loaded.owner_repo, "new/repo"); + assert_eq!(loaded.status, pending_file::STATUS_PENDING); + assert!(loaded.dispatched_at.is_none()); + assert!(loaded.consumed_at.is_none()); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_skips_when_existing_pending_is_active() { + let path = unique_tmp_pending("existing-active"); + let existing = PendingFile { + schema_version: pending_file::SCHEMA_VERSION, + pr_number: 100, + owner_repo: "a/b".to_string(), + prompt: "post-merge-feedback".to_string(), + status: pending_file::STATUS_PENDING.to_string(), + created_at: "2026-04-22T00:00:00Z".to_string(), + dispatched_at: None, + consumed_at: None, + producer: None, + }; + pending_file::write_new_exclusive(&path, &existing).unwrap(); + + let ctx = PipelineContext { + pr_number: 200, + owner_repo: Some("c/d".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + // 既存が保持される + let loaded = read_pending(&path); + assert_eq!(loaded.pr_number, 100); + assert_eq!(loaded.owner_repo, "a/b"); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_skips_when_existing_pending_is_dispatched() { + let path = unique_tmp_pending("existing-dispatched"); + let existing = PendingFile { + schema_version: pending_file::SCHEMA_VERSION, + pr_number: 100, + owner_repo: "a/b".to_string(), + prompt: "post-merge-feedback".to_string(), + status: pending_file::STATUS_DISPATCHED.to_string(), + created_at: "2026-04-22T00:00:00Z".to_string(), + dispatched_at: Some("2026-04-22T00:01:00Z".to_string()), + consumed_at: None, + producer: None, + }; + pending_file::write_new_exclusive(&path, &existing).unwrap(); + + let ctx = PipelineContext { + pr_number: 200, + owner_repo: Some("c/d".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + let loaded = read_pending(&path); + assert_eq!(loaded.pr_number, 100); + assert_eq!(loaded.status, pending_file::STATUS_DISPATCHED); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_overwrites_corrupt_pending() { + let path = unique_tmp_pending("corrupt-parse"); + std::fs::write(&path, "this is not valid json").unwrap(); + + let ctx = PipelineContext { + pr_number: 777, + owner_repo: Some("x/y".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + let loaded = read_pending(&path); + assert_eq!(loaded.pr_number, 777); + assert_eq!(loaded.status, pending_file::STATUS_PENDING); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_overwrites_empty_pending() { + let path = unique_tmp_pending("corrupt-empty"); + std::fs::write(&path, "").unwrap(); + + let ctx = PipelineContext { + pr_number: 888, + owner_repo: Some("x/y".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + let loaded = read_pending(&path); + assert_eq!(loaded.pr_number, 888); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_leaves_no_tmp_residue_on_success() { + let path = unique_tmp_pending("no-tmp-residue"); + let ctx = PipelineContext { + pr_number: 1, + owner_repo: Some("o/r".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + // 新形式: tmp file 名は "{basename}.tmp.{pid}.{counter}" + let dir = path.parent().unwrap_or(std::path::Path::new(".")); + let basename = path.file_name().unwrap().to_string_lossy().into_owned(); + let tmp_prefix = format!("{}.tmp.", basename); + let residues: Vec<_> = std::fs::read_dir(dir) + .unwrap() + .flatten() + .filter_map(|e| { + let name = e.file_name().to_string_lossy().into_owned(); + if name.starts_with(&tmp_prefix) { + Some(e.path()) + } else { + None + } + }) + .collect(); + assert!(residues.is_empty(), "tmp residue: {:?}", residues); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_sets_producer_field() { + let path = unique_tmp_pending("producer-set"); + let ctx = PipelineContext { + pr_number: 1, + owner_repo: Some("o/r".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + + let loaded = read_pending(&path); + let producer = loaded.producer.expect("producer should be set"); + assert!(producer.starts_with("cli-merge-pipeline@pid-")); + assert!(producer.ends_with('Z')); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn ai_step_warns_when_concurrent_writer_wins_new_exclusive() { + // 同じ path に 2 度 run_ai_step を呼ぶ: 1 度目は成功、2 度目は既存 Active で skip + // (read_existing が Active を返す経路なので直接の AlreadyExists ではないが、 + // 不可視ロストが起きないことを担保する代表シナリオ) + let path = unique_tmp_pending("concurrent-win"); + let ctx = PipelineContext { + pr_number: 1, + owner_repo: Some("o/r".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx), &path); + let first = read_pending(&path); + assert_eq!(first.pr_number, 1); + + // 2 度目: 既存 pending (Active) があるため skip (上書きされない) + let ctx2 = PipelineContext { + pr_number: 2, + owner_repo: Some("o/r".to_string()), + }; + run_ai_step("test", &ai_step(None), Some(&ctx2), &path); + let second = read_pending(&path); + assert_eq!(second.pr_number, 1, "既存 pending が上書きされてはならない"); + + let _ = std::fs::remove_file(&path); + } } diff --git a/src/cli-merge-pipeline/src/pending_file.rs b/src/cli-merge-pipeline/src/pending_file.rs new file mode 100644 index 0000000..bccf586 --- /dev/null +++ b/src/cli-merge-pipeline/src/pending_file.rs @@ -0,0 +1,644 @@ +//! post-merge-feedback pending file の読み書きと入力検証 (ADR-029) +//! +//! pending file は `.claude/post-merge-feedback-pending.json` に配置され、 +//! cli-merge-pipeline が post-merge ステップ (`type = "ai"`) で書き込み、 +//! hooks-stop-feedback-dispatch が Stop 時に検出して Claude に skill 起動を指示する。 +//! +//! 書き込み経路は 2 種類 (ADR-029 §破損耐性): +//! - 新規作成: `OpenOptions::new().write(true).create_new(true).open(path)` で +//! 最終ファイルを直接 atomic 排他作成 (O_EXCL 相当)。rename は使わない。 +//! - 上書き (既存 Consumed/Corrupt 削除後): tmp file → `fs::rename` の 2 段階。 +//! +//! 排他性保証: +//! - 新規作成経路: `create_new` の `AlreadyExists` で TOCTOU race を atomic に検出 +//! (ADR-029 §競合ポリシーの「skip + WARN で取りこぼしを観測可能」を実装レベルで保証) +//! - 上書き経路: read→write 間の race は許容 (Consumed/Corrupt は稀経路、 +//! 破損ポリシーで自己回復) +//! +//! 中間状態の可観測性: +//! - 新規作成経路は `create_new` で直接書くため、write 完了前の reader は +//! size=0 or 途中の JSON を観測し得る。hooks-stop-feedback-dispatch の +//! 破損ポリシー (size=0 / parse 失敗 → 削除 → silent exit) が吸収する。 +//! **完全性より排他性を優先する設計判断** (ADR-029 §破損耐性)。 +//! +//! atomic 保証の前提 (ADR-029): +//! - POSIX: `open(O_EXCL)` / `rename(2)` により atomic (同一ファイルシステム内) +//! - Windows 10 1607+ / NTFS or ReFS: `CREATE_NEW` / `FileRenameInfoEx` 経路で atomic +//! (本プロジェクトのターゲット環境) +//! - 旧 Windows / 非対応 FS: atomic 保証なし (他プロセスが中間状態を観測可能) +//! +//! 非 atomic 環境では POST_MERGE_FEEDBACK_TRIGGER が 1 回発火失敗する可能性があるが、 +//! 次のマージで復帰可能なので本 module は fallback を許容する。失敗時の Err は +//! 戻り値として呼び出し側へ伝播させ、呼び出し側が log を出す (silent fail させない)。 + +use serde::{Deserialize, Serialize}; +use std::fs::OpenOptions; +use std::io::{ErrorKind, Write}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +/// プロセス内で一意な tmp ファイル名を生成するためのカウンタ。 +/// 複数の writer が同時に `write_overwrite` を呼んでも tmp パスが衝突しない。 +/// `write_new_exclusive` は tmp path を使わないため本カウンタを参照しない。 +static TMP_COUNTER: AtomicU64 = AtomicU64::new(0); + +/// pending file のスキーマバージョン。 +/// +/// 非互換変更時に bump する。hooks-stop-feedback-dispatch (task 1-C) は +/// これと一致しない pending を「破損」として削除する。 +pub(crate) const SCHEMA_VERSION: u32 = 1; + +/// ファイル名 (`.claude/` 配下に配置) +pub(crate) const FILE_NAME: &str = "post-merge-feedback-pending.json"; + +/// ADR-029 で定義された status 値 +pub(crate) const STATUS_PENDING: &str = "pending"; +pub(crate) const STATUS_DISPATCHED: &str = "dispatched"; +pub(crate) const STATUS_CONSUMED: &str = "consumed"; + +/// pending file の JSON スキーマ (ADR-029 §Pending file JSON スキーマ v1) +/// +/// `producer` は schema v1 互換の **optional** フィールド。取りこぼし発生時に +/// 「誰が書いた pending が消えたか」を破損残骸からも追跡可能にするための観測性補助。 +/// 既存 reader (hooks-stop-feedback-dispatch / skill Phase 0) は未知 / 欠損フィールドを +/// 無視するため schema_version bump は不要。 +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub(crate) struct PendingFile { + pub(crate) schema_version: u32, + pub(crate) pr_number: u64, + pub(crate) owner_repo: String, + pub(crate) prompt: String, + pub(crate) status: String, + pub(crate) created_at: String, + pub(crate) dispatched_at: Option, + pub(crate) consumed_at: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) producer: Option, +} + +/// producer 文字列を生成する (`cli-merge-pipeline@pid-{pid}@{iso8601}`)。 +/// +/// PID は再利用されるため timestamp を併記して時系列追跡可能にする。hostname は YAGNI で省略。 +pub(crate) fn producer_string() -> String { + format!( + "cli-merge-pipeline@pid-{}@{}", + std::process::id(), + utc_now_iso8601() + ) +} + +/// 既存 pending file の読み取り結果。 +/// +/// status 未知値は Corrupt 扱いとする (ADR-029 は `pending`/`dispatched`/`consumed` +/// のみを schema_version=1 で enumerate しているため、それ以外は schema drift とみなす)。 +#[derive(Debug)] +pub(crate) enum ExistingPending { + /// ファイル不在。通常経路。 + None, + /// size 0 / parse 失敗 / schema_version 不一致 / 未知 status。削除して再書き込みする。 + Corrupt(String), + /// status = "consumed"。上書き OK。 + Consumed, + /// status = "pending" または "dispatched"。書き込みをスキップする (WARN)。 + Active(String), +} + +/// 既存 pending file の状態を判定する。 +pub(crate) fn read_existing(path: &Path) -> ExistingPending { + let meta = match std::fs::metadata(path) { + Ok(m) => m, + Err(_) => return ExistingPending::None, + }; + if meta.len() == 0 { + return ExistingPending::Corrupt("size=0".to_string()); + } + let content = match std::fs::read_to_string(path) { + Ok(c) => c, + Err(e) => return ExistingPending::Corrupt(format!("read error: {}", e)), + }; + let pending: PendingFile = match serde_json::from_str(&content) { + Ok(p) => p, + Err(e) => return ExistingPending::Corrupt(format!("parse error: {}", e)), + }; + if pending.schema_version != SCHEMA_VERSION { + return ExistingPending::Corrupt(format!( + "schema_version mismatch (got {}, want {})", + pending.schema_version, SCHEMA_VERSION + )); + } + match pending.status.as_str() { + STATUS_CONSUMED => ExistingPending::Consumed, + STATUS_PENDING | STATUS_DISPATCHED => ExistingPending::Active(pending.status), + other => ExistingPending::Corrupt(format!("unknown status '{}'", other)), + } +} + +/// pending file 書き込みの失敗種別。 +/// +/// `AlreadyExists` は新規作成経路で TOCTOU race を atomic に検出した場合に返る +/// (ADR-029 §競合ポリシー)。呼び出し側は `WARN + skip` でログに残すことで +/// 「取りこぼしの可視化」を保証する。 +#[derive(Debug)] +pub(crate) enum WriteError { + /// 新規作成時に既に pending file が存在した (他プロセスが先に書き込んだ) + AlreadyExists, + /// I/O エラー (書き込み / sync / rename 失敗) + Io(String), + /// serde_json シリアライズ失敗 + Serialize(String), +} + +impl std::fmt::Display for WriteError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + WriteError::AlreadyExists => write!(f, "pending file already exists"), + WriteError::Io(msg) => write!(f, "I/O error: {}", msg), + WriteError::Serialize(msg) => write!(f, "serialize error: {}", msg), + } + } +} + +/// pending file を **新規排他作成** する (ADR-029 §競合ポリシー)。 +/// +/// `OpenOptions::create_new(true)` (O_EXCL 相当) で最終ファイルを直接 atomic 排他作成する。 +/// 他プロセスが先に作成済みなら `WriteError::AlreadyExists` を返し、呼び出し側は +/// WARN + skip で取りこぼしを観測可能にする。 +/// +/// **rename は使わない** — `rename` は既存を無条件上書きするため、placeholder 方式だと +/// 排他予約が自壊する (CodeRabbit PR #70 Major 指摘の本質)。 +/// +/// `write_all` の途中でエラー終了すると size=0 or 部分書き込みのファイルが残るが、 +/// ADR-029 §破損耐性 の Corrupt ポリシー (size=0 / parse 失敗 → 削除 → silent exit) が +/// 吸収する。完全性より排他性を優先する設計判断。 +pub(crate) fn write_new_exclusive(path: &Path, pending: &PendingFile) -> Result<(), WriteError> { + let json = + serde_json::to_string_pretty(pending).map_err(|e| WriteError::Serialize(e.to_string()))?; + + match OpenOptions::new().write(true).create_new(true).open(path) { + Ok(mut file) => { + if let Err(e) = file.write_all(json.as_bytes()) { + return Err(WriteError::Io(format!("write_all 失敗: {}", e))); + } + if let Err(e) = file.sync_all() { + return Err(WriteError::Io(format!("sync_all 失敗: {}", e))); + } + Ok(()) + } + Err(e) if e.kind() == ErrorKind::AlreadyExists => Err(WriteError::AlreadyExists), + Err(e) => Err(WriteError::Io(format!( + "create_new 失敗 ({}): {}", + path.display(), + e + ))), + } +} + +/// pending file を **上書き書き込み** する (tmp → rename 2 段階)。 +/// +/// 呼び出し元は事前に `read_existing` で `Consumed` / `Corrupt` を判定し、 +/// ファイルを削除してから本関数を呼ぶことを想定。稀経路なので read→write 間の +/// race は許容 (ADR-029 §競合ポリシー)。 +/// +/// tmp 名は `{file_name}.tmp.{pid}.{counter}` 形式で一意化、並行 writer の +/// staging file 共有を防ぐ。失敗時は tmp 残骸を best-effort で削除。 +pub(crate) fn write_overwrite(path: &Path, pending: &PendingFile) -> Result<(), WriteError> { + let json = + serde_json::to_string_pretty(pending).map_err(|e| WriteError::Serialize(e.to_string()))?; + let counter = TMP_COUNTER.fetch_add(1, Ordering::Relaxed); + let file_name = path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "pending".to_string()); + let tmp_name = format!("{}.tmp.{}.{}", file_name, std::process::id(), counter); + let tmp_path = path.with_file_name(tmp_name); + if let Err(e) = std::fs::write(&tmp_path, &json) { + let _ = std::fs::remove_file(&tmp_path); + return Err(WriteError::Io(format!( + "tmp 書き込み失敗 ({}): {}", + tmp_path.display(), + e + ))); + } + if let Err(e) = std::fs::rename(&tmp_path, path) { + let _ = std::fs::remove_file(&tmp_path); + return Err(WriteError::Io(format!( + "rename 失敗 ({} → {}): {}", + tmp_path.display(), + path.display(), + e + ))); + } + Ok(()) +} + +/// `{owner}/{repo}` 形式の文字列を検証する (ADR-029 todo 1-B の security-review 反映)。 +/// +/// 許容文字: ASCII 英数字 + `_` `.` `-`。スラッシュはちょうど 1 つ、owner/repo とも非空。 +/// newline / 制御文字は弾く (pending file / additionalContext への注入防御)。 +pub(crate) fn is_valid_owner_repo(s: &str) -> bool { + let Some((owner, repo)) = s.split_once('/') else { + return false; + }; + !owner.is_empty() + && !repo.is_empty() + && !repo.contains('/') + && owner.chars().all(is_repo_ident_char) + && repo.chars().all(is_repo_ident_char) +} + +fn is_repo_ident_char(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '_' || c == '.' || c == '-' +} + +/// pending file のデフォルト配置先 (exe と同じディレクトリ = `.claude/`)。 +/// +/// 本プロジェクトは `pnpm deploy:hooks` で exe を `.claude/` に配置するため、 +/// exe の親ディレクトリが pending file の正しい置き場になる。派生プロジェクトも同様。 +pub(crate) fn default_path(config_dir: &Path) -> PathBuf { + config_dir.join(FILE_NAME) +} + +// ─── UTC ISO 8601 helper ─── +// +// cli-pr-monitor/src/util.rs にも同等の pub(crate) 関数が存在する。 +// 1-C (hooks-stop-feedback-dispatch) も同じ helper を必要とするため、 +// 3 callers になった時点で lib へ切り出すか判断する (現段階では duplicate でよい)。 + +/// 現在時刻を ISO 8601 UTC 文字列に変換する (std のみ, chrono 不要)。 +pub(crate) fn utc_now_iso8601() -> String { + use std::time::SystemTime; + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default(); + epoch_secs_to_iso8601(now.as_secs()) +} + +// Constants for Hatcher's proleptic Gregorian civil-date algorithm. +// Reference: https://howardhinnant.github.io/date_algorithms.html +/// Days from the proleptic Gregorian epoch (0000-03-01) to the Unix epoch (1970-01-01). +const CIVIL_EPOCH_OFFSET: i64 = 719_468; +/// Days in a 400-year Gregorian era. +const DAYS_PER_ERA: i64 = 146_097; +/// DAYS_PER_ERA - 1; used for the era-floor sign correction. +const DAYS_PER_ERA_M1: i64 = 146_096; +/// Days in a 4-year cycle (excluding century boundaries). +const DAYS_PER_4Y: u64 = 1_460; +/// Days in a 100-year cycle. +const DAYS_PER_100Y: u64 = 36_524; +/// Days in an ordinary year. +const DAYS_PER_YEAR: u64 = 365; +/// Years per 400-year Gregorian era. +const YEARS_PER_ERA: i64 = 400; +/// Multiplier for the month-to-day-of-year encoding: (5*mp + 2) / 153. +const MONTH_ENCODE_MUL: u64 = 5; +/// Divisor for the month-to-day-of-year encoding. +const MONTH_ENCODE_DIV: u64 = 153; +/// Seconds per hour. +const SECS_PER_HOUR: u64 = 3_600; +/// Seconds per minute. +const SECS_PER_MIN: u64 = 60; +/// Seconds per day. +const SECS_PER_DAY: u64 = 86_400; + +fn epoch_secs_to_iso8601(epoch: u64) -> String { + let day_count = (epoch / SECS_PER_DAY) as i64; + let time_of_day = epoch % SECS_PER_DAY; + + let z = day_count + CIVIL_EPOCH_OFFSET; + let era = (if z >= 0 { z } else { z - DAYS_PER_ERA_M1 }) / DAYS_PER_ERA; + let doe = (z - era * DAYS_PER_ERA) as u64; + let yoe = (doe - doe / DAYS_PER_4Y + doe / DAYS_PER_100Y - doe / (DAYS_PER_ERA_M1 as u64)) + / DAYS_PER_YEAR; + let y = yoe as i64 + era * YEARS_PER_ERA; + let doy = doe - (DAYS_PER_YEAR * yoe + yoe / 4 - yoe / 100); + let mp = (MONTH_ENCODE_MUL * doy + 2) / MONTH_ENCODE_DIV; + let d = doy - (MONTH_ENCODE_DIV * mp + 2) / MONTH_ENCODE_MUL + 1; + let m = if mp < 10 { mp + 3 } else { mp - 9 }; + let y = if m <= 2 { y + 1 } else { y }; + + let hour = time_of_day / SECS_PER_HOUR; + let min = (time_of_day % SECS_PER_HOUR) / SECS_PER_MIN; + let sec = time_of_day % SECS_PER_MIN; + + format!( + "{:04}-{:02}-{:02}T{:02}:{:02}:{:02}Z", + y, m, d, hour, min, sec + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_pending(status: &str) -> PendingFile { + PendingFile { + schema_version: SCHEMA_VERSION, + pr_number: 123, + owner_repo: "aloekun/claude-code-hook-test".to_string(), + prompt: "post-merge-feedback".to_string(), + status: status.to_string(), + created_at: "2026-04-23T10:00:00Z".to_string(), + dispatched_at: None, + consumed_at: None, + producer: None, + } + } + + fn unique_tmp(label: &str) -> PathBuf { + std::env::temp_dir().join(format!( + "pending-{}-{}-{}.json", + label, + std::process::id(), + // nanosecond で同一 pid 内の衝突も回避 + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )) + } + + #[test] + fn valid_owner_repo_accepts_typical_slugs() { + assert!(is_valid_owner_repo("aloekun/claude-code-hook-test")); + assert!(is_valid_owner_repo("octo-org/my.repo")); + assert!(is_valid_owner_repo("a/b")); + assert!(is_valid_owner_repo("Ab_12/X.y-z")); + } + + #[test] + fn valid_owner_repo_rejects_malformed() { + assert!(!is_valid_owner_repo("")); + assert!(!is_valid_owner_repo("noslash")); + assert!(!is_valid_owner_repo("/missing-owner")); + assert!(!is_valid_owner_repo("missing-repo/")); + assert!(!is_valid_owner_repo("a/b/c")); // 複数スラッシュ + assert!(!is_valid_owner_repo("has space/repo")); + assert!(!is_valid_owner_repo("owner/repo\nfoo")); // newline injection + assert!(!is_valid_owner_repo("owner/repo\r")); + assert!(!is_valid_owner_repo("owner/repo\t")); + assert!(!is_valid_owner_repo("owner!/repo")); + } + + #[test] + fn write_new_exclusive_creates_file() { + let path = unique_tmp("write-new-exclusive"); + let pending = sample_pending(STATUS_PENDING); + + write_new_exclusive(&path, &pending).unwrap(); + + // 正しく書き込まれている + let loaded: PendingFile = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert_eq!(loaded, pending); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn write_new_exclusive_returns_already_exists_when_target_present() { + let path = unique_tmp("already-exists"); + let pending = sample_pending(STATUS_PENDING); + + // 1 回目は成功 + write_new_exclusive(&path, &pending).unwrap(); + // 2 回目は AlreadyExists + match write_new_exclusive(&path, &pending) { + Err(WriteError::AlreadyExists) => {} + other => panic!("expected AlreadyExists, got {:?}", other), + } + // 既存内容は上書きされていない + let loaded: PendingFile = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert_eq!(loaded, pending); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn write_new_exclusive_atomic_under_concurrent_writers() { + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering as AtomicOrdering; + use std::sync::Arc; + + let path = Arc::new(unique_tmp("concurrent-new-exclusive")); + let success_count = Arc::new(AtomicUsize::new(0)); + let already_exists_count = Arc::new(AtomicUsize::new(0)); + + let handles: Vec<_> = (0..8) + .map(|i| { + let p = Arc::clone(&path); + let ok = Arc::clone(&success_count); + let ae = Arc::clone(&already_exists_count); + std::thread::spawn(move || { + let mut pf = sample_pending(STATUS_PENDING); + pf.pr_number = i + 1; + match write_new_exclusive(&p, &pf) { + Ok(()) => { + ok.fetch_add(1, AtomicOrdering::Relaxed); + } + Err(WriteError::AlreadyExists) => { + ae.fetch_add(1, AtomicOrdering::Relaxed); + } + Err(other) => panic!("unexpected write error: {:?}", other), + } + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + + // atomic 排他予約が効いているなら、成功は 1 スレッドのみ・残りは AlreadyExists + assert_eq!(success_count.load(AtomicOrdering::Relaxed), 1); + assert_eq!(already_exists_count.load(AtomicOrdering::Relaxed), 7); + + // 最終ファイルは有効な JSON + let content = std::fs::read_to_string(path.as_ref()).unwrap(); + let _: PendingFile = serde_json::from_str(&content).unwrap(); + + let _ = std::fs::remove_file(path.as_ref()); + } + + #[test] + fn write_overwrite_replaces_existing_file() { + let path = unique_tmp("overwrite"); + let first = sample_pending(STATUS_CONSUMED); + let mut second = sample_pending(STATUS_PENDING); + second.pr_number = 999; + + write_new_exclusive(&path, &first).unwrap(); + write_overwrite(&path, &second).unwrap(); + + let loaded: PendingFile = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert_eq!(loaded, second); + + // tmp 残骸 (`{basename}.tmp.{pid}.{counter}`) が残っていないこと + let dir = path.parent().unwrap_or(std::path::Path::new(".")); + let basename = path.file_name().unwrap().to_string_lossy().into_owned(); + let tmp_prefix = format!("{}.tmp.", basename); + let residues: Vec<_> = std::fs::read_dir(dir) + .unwrap() + .flatten() + .filter(|e| e.file_name().to_string_lossy().starts_with(&tmp_prefix)) + .collect(); + assert!( + residues.is_empty(), + "tmp residue: {} entries", + residues.len() + ); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn producer_string_contains_pid_and_timestamp() { + let s = producer_string(); + assert!(s.starts_with("cli-merge-pipeline@pid-")); + // @{iso8601} が含まれることを "Z" で軽く確認 + assert!(s.ends_with('Z'), "expected iso8601 suffix: {}", s); + // pid 部分が数値 + let pid_part = s + .strip_prefix("cli-merge-pipeline@pid-") + .and_then(|rest| rest.split('@').next()) + .unwrap(); + assert!(pid_part.chars().all(|c| c.is_ascii_digit())); + } + + #[test] + fn pending_file_roundtrip_with_producer() { + let mut pending = sample_pending(STATUS_PENDING); + pending.producer = Some("cli-merge-pipeline@pid-1234@2026-04-23T12:34:56Z".to_string()); + + let json = serde_json::to_string(&pending).unwrap(); + let loaded: PendingFile = serde_json::from_str(&json).unwrap(); + assert_eq!(loaded, pending); + } + + #[test] + fn pending_file_without_producer_field_deserializes() { + // producer フィールド不在の JSON (schema v1 既存ファイル) が正しく読めること + let json = r#"{ + "schema_version": 1, + "pr_number": 42, + "owner_repo": "o/r", + "prompt": "post-merge-feedback", + "status": "pending", + "created_at": "2026-04-23T10:00:00Z", + "dispatched_at": null, + "consumed_at": null + }"#; + let loaded: PendingFile = serde_json::from_str(json).unwrap(); + assert_eq!(loaded.producer, None); + assert_eq!(loaded.pr_number, 42); + } + + #[test] + fn read_existing_returns_none_when_absent() { + let path = unique_tmp("absent"); + assert!(matches!(read_existing(&path), ExistingPending::None)); + } + + #[test] + fn read_existing_returns_corrupt_for_empty_file() { + let path = unique_tmp("empty"); + std::fs::write(&path, "").unwrap(); + + match read_existing(&path) { + ExistingPending::Corrupt(reason) => assert!(reason.contains("size=0")), + other => panic!("expected Corrupt, got {:?}", other), + } + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn read_existing_returns_corrupt_for_invalid_json() { + let path = unique_tmp("bad-json"); + std::fs::write(&path, "not a json").unwrap(); + + match read_existing(&path) { + ExistingPending::Corrupt(reason) => assert!(reason.contains("parse")), + other => panic!("expected Corrupt, got {:?}", other), + } + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn read_existing_returns_corrupt_for_schema_mismatch() { + let path = unique_tmp("bad-schema"); + let mut pending = sample_pending(STATUS_PENDING); + pending.schema_version = 99; + write_new_exclusive(&path, &pending).unwrap(); + + match read_existing(&path) { + ExistingPending::Corrupt(reason) => { + assert!(reason.contains("schema_version")); + } + other => panic!("expected Corrupt, got {:?}", other), + } + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn read_existing_returns_corrupt_for_unknown_status() { + let path = unique_tmp("bad-status"); + let pending = sample_pending("garbage"); + write_new_exclusive(&path, &pending).unwrap(); + + match read_existing(&path) { + ExistingPending::Corrupt(reason) => assert!(reason.contains("unknown status")), + other => panic!("expected Corrupt, got {:?}", other), + } + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn read_existing_returns_active_for_pending_and_dispatched() { + let path_p = unique_tmp("active-pending"); + write_new_exclusive(&path_p, &sample_pending(STATUS_PENDING)).unwrap(); + match read_existing(&path_p) { + ExistingPending::Active(s) => assert_eq!(s, STATUS_PENDING), + other => panic!("expected Active(pending), got {:?}", other), + } + let _ = std::fs::remove_file(&path_p); + + let path_d = unique_tmp("active-dispatched"); + write_new_exclusive(&path_d, &sample_pending(STATUS_DISPATCHED)).unwrap(); + match read_existing(&path_d) { + ExistingPending::Active(s) => assert_eq!(s, STATUS_DISPATCHED), + other => panic!("expected Active(dispatched), got {:?}", other), + } + let _ = std::fs::remove_file(&path_d); + } + + #[test] + fn read_existing_returns_consumed_for_consumed_status() { + let path = unique_tmp("consumed"); + write_new_exclusive(&path, &sample_pending(STATUS_CONSUMED)).unwrap(); + assert!(matches!(read_existing(&path), ExistingPending::Consumed)); + let _ = std::fs::remove_file(&path); + } + + #[test] + fn utc_now_iso8601_matches_expected_format() { + let s = utc_now_iso8601(); + // YYYY-MM-DDTHH:MM:SSZ = 20 chars + assert_eq!(s.len(), 20, "unexpected length: {}", s); + assert!(s.ends_with('Z')); + assert_eq!(s.chars().nth(4), Some('-')); + assert_eq!(s.chars().nth(7), Some('-')); + assert_eq!(s.chars().nth(10), Some('T')); + assert_eq!(s.chars().nth(13), Some(':')); + assert_eq!(s.chars().nth(16), Some(':')); + } + + #[test] + fn epoch_secs_to_iso8601_epoch_zero_is_unix_epoch() { + assert_eq!(epoch_secs_to_iso8601(0), "1970-01-01T00:00:00Z"); + } +}