Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions docs/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,43 @@
- SessionStart hook は master に実装済み (`src/hooks-session-start/`)。セッション引継ぎ設計は session ID → jsonl transcript 紐付けの ADR が必要
- takt-test-vc での試験運用を先に行い、本プロジェクトに反映

### 3. Cargo workspace 化 + rust-test template 反映 (PR-β、実装済み)
### 3. cli-pr-monitor の --body 複数行引数切り詰め修正

- **やろうとしたこと**: `pnpm create-pr -- --body "複数行テキスト"` で PR body が最初の行だけに切り詰められる問題を修正する
- **現在地**: 未着手。原因特定済み
- [ ] cli-pr-monitor.exe の引数転送ロジックで複数行 body のクォーティングが失われる原因を調査
- [ ] `--body-file` 対応の追加、または引数転送ロジックの修正
- [ ] 修正後に複数行 body での PR 作成テスト
- **詰まっている箇所**: なし
- **根拠**: PR #49 作成時に発生。`--body "$(cat <<'EOF' ... EOF)"` 形式で pnpm 経由で渡すと、シェル引数分割で body が `## Summary` だけに切り詰められた
- **Why**: cli-pr-monitor.exe が `--` 以降の引数を gh pr create に転送する際、pnpm のシェル経由で改行・スペースを含む body が複数の引数に分割される
- **How to apply / 再開手順**: `src/cli-pr-monitor/src/stages/create_pr.rs` の引数処理を確認し、`--body-file` を使う方式か、引数を結合する方式で修正

### 4. push-runner の takt fix 後 bookmark 乖離問題

- **やろうとしたこと**: takt fix ステップ後に @ が bookmark より先に進み、`jj git push` で "No bookmarks found" となる問題を修正する
- **現在地**: 未着手。原因特定済み
- [ ] `src/cli-push-runner/src/stages/push.rs` の push ステップで、push 前に bookmark を @ に追従させるロジックを追加
- [ ] または takt ステップ後に `jj squash` 相当の処理を自動実行
- [ ] 修正後に takt fix が発火するケースでの回帰テスト
- **詰まっている箇所**: なし
- **根拠**: PR #49 の push pipeline で発生。takt の fix ステップがコード修正 → @ が bookmark から乖離 → push で bookmark が見つからない
- **Why**: takt は @ 上で直接コード修正するため、fix が入ると @ が新しい commit に進むが、bookmark は旧 commit のまま残る
- **How to apply / 再開手順**: push ステップ内で `push_jj_bookmark::advance_jj_bookmarks()` が既にあるが、これは trunk 以降の bookmark を target に前進させるもの。takt fix 後の bookmark 乖離は別の問題 (bookmark 自体が @ より古い位置にある)。push 前に `jj bookmark set <name> -r @` で bookmark を @ に合わせる処理を追加する

### 5. push-runner の空 diff 時 pipeline 中断を正常終了に

- **やろうとしたこと**: push 対象の変更がない場合 (レビュー済みコードの再 push 等)、exit code 5 で中断するのではなく skip として正常終了するオプションを追加する
- **現在地**: 未着手。原因特定済み
- [ ] `src/cli-push-runner/src/stages/diff.rs` の空 diff 判定を "skip review + proceed to push" モードに変更
- [ ] push-runner-config.toml に `allow_empty_diff = true` 等のオプション追加を検討
- [ ] 修正後に空 diff ケースでの回帰テスト
- **詰まっている箇所**: なし
- **根拠**: PR #49 で squash 後の再 push 時に発生。jj squash で @ が空コミットになり、`jj diff -r @` が空 → push-runner が "diff 出力が空です" で exit 5
- **Why**: push-runner は diff が空 = レビュー対象なし = パイプライン中断と判断するが、takt fix 後の再 push や bookmark 移動後の push では「diff は空だが push は必要」なケースがある
- **How to apply / 再開手順**: diff が空の場合に takt レビューをスキップして push ステップに直接進むパスを追加。push-runner-config.toml で挙動を制御できるようにする
Comment on lines +136 to +158

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

タスク #4 / #5 のステータスが本 PR の実装内容と乖離しています

タスク #4(bookmark 乖離)と #5(空 diff スキップ)は本 PR の主目的そのものであり、src/cli-push-runner/src/stages/push_jj_bookmark.rssrc/cli-push-runner/src/stages/diff.rs / main.rs で既に実装済みです。にもかかわらず両項目が「現在地: 未着手。原因特定済み」のまま記載されており、マージ時点の事実と食い違います。

本 PR でマージする時点では「完了履歴」側へ移動するか、少なくともチェックリストを [x] に更新するのが妥当です(#3 の cli-pr-monitor --body 問題は本 PR では触っていないので #3 だけが in-flight として残る想定)。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/todo.md` around lines 136 - 158, The docs checklist incorrectly marks
tasks `#4` and `#5` as "未着手" even though the PR implements them; update docs/todo.md
to reflect completion by marking the checklist items for task `#4` (bookmark
divergence) and task `#5` (empty-diff skip) as completed (e.g., change to [x]) and
add a brief note referencing the implemented files/entries
src/cli-push-runner/src/stages/push_jj_bookmark.rs (advance/bookmark-set logic)
and src/cli-push-runner/src/stages/diff.rs / main.rs (empty-diff skip/config) so
the document matches the current PR state.


### 6. Cargo workspace 化 + rust-test template 反映 (PR-β、実装済み)

- **やろうとしたこと**: PR #44 のセッション知見を元に:
1. Rust test を push pipeline で一発実行できるよう Cargo workspace 化
Expand Down Expand Up @@ -167,11 +203,17 @@ ADR-019 および ADR-020 の「次ステップ」セクションで明記され

## 完了履歴

### conflicted bookmarks の棚卸し
### conflicted bookmarks の棚卸し + push 前 bookmark 自動前進 (PR #49)

- [x] **feat/merge-pipeline** (conflicted): `jj bookmark forget` で削除。ADR-013 は master にマージ済み、固有差分は takt 移行前の旧 push_pipeline 設定のみで価値なし
- [x] **feat/session-start-hook** (conflicted): `jj bookmark forget` で削除。bookmark 版は旧レイアウト (.claude/) + 「先勝ち」方式。master 版 (`src/hooks-session-start/`) は新レイアウト + 「同一IDスキップ」方式 + テスト 13 本で上位互換
- [x] **feat/push-runner-auto-bookmark** (未 push): `push_jj_bookmark.rs` (bookmark 自動前進機能) を master の `src/cli-push-runner/src/stages/` に cherry-pick 後、`jj bookmark forget` で削除。テスト 10 本 pass
- [x] **CodeRabbit Critical**: `jj bookmark set` の引数順序修正 (`--` 後に `-r` → `-r` を `--` の前に移動) — takt 自動修正
- [x] **CodeRabbit Major**: `main()` / `master()` を bare bookmark 名 `main` / `master` に修正 — takt 自動修正
- [x] **CodeRabbit Minor**: `@` が root commit の場合の `@-` 不在ガード追加 (`determine_target_revision` を `Option<String>` に変更)
- [x] **CodeRabbit Minor**: `trunk()` 成功かつ 0 件で `main`/`master` にフォールバックしない問題修正 (非空チェック追加)
- **知見**: cherry-pick 元の bookmark 放置コードに Critical バグが潜んでいた。cherry-pick = 「レビュー済みコードの移植」ではない。push pipeline + CodeRabbit の二重レビューが有効に機能
- **副産物**: 3 件の仕組み改善タスク (#3, #4, #5) を発見・記録

### cli-pr-monitor Known Issues (PR #13)

Expand Down
37 changes: 26 additions & 11 deletions src/cli-push-runner/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Push Runner — takt ベースの pre-push パイプライン
//! Push Runner — takt ベースの pre-push パイプライン
//!
//! pnpm push から呼び出され、以下のステージを実行する:
//! Stage 1: quality_gate — TOML で定義されたコマンド群をグループ間で並列実行
Expand All @@ -25,7 +25,14 @@ use std::time::Instant;

use config::load_config;
use log::log_info;
use stages::{run_diff, run_push, run_quality_gate, run_takt};
use stages::{run_diff, run_push, run_quality_gate, run_takt, DiffResult};

const EXIT_SUCCESS: i32 = 0;
const EXIT_QUALITY_GATE_FAILURE: i32 = 1;
const EXIT_TAKT_FAILURE: i32 = 2;
const EXIT_PUSH_FAILURE: i32 = 3;
const EXIT_CONFIG_ERROR: i32 = 4;
const EXIT_DIFF_FAILURE: i32 = 5;

fn run_pipeline() -> i32 {
let start = Instant::now();
Expand All @@ -34,7 +41,7 @@ fn run_pipeline() -> i32 {
Ok(c) => c,
Err(e) => {
log_info(&format!("設定エラー: {}", e));
return 4;
return EXIT_CONFIG_ERROR;
}
};

Expand All @@ -48,32 +55,40 @@ fn run_pipeline() -> i32 {
// Stage 1: quality_gate
if !run_quality_gate(&config.quality_gate) {
log_info("パイプライン中断: quality_gate 失敗。問題を修正して再実行してください。");
return 1;
return EXIT_QUALITY_GATE_FAILURE;
}

// Stage 1.5: diff
let mut skip_takt = false;
if let Some(diff_config) = &config.diff {
if !run_diff(diff_config) {
log_info("パイプライン中断: diff 取得失敗。");
return 5;
match run_diff(diff_config) {
DiffResult::HasContent => {}
DiffResult::Empty => {
log_info("diff が空のためレビューをスキップして push に進みます。");
skip_takt = true;
}
DiffResult::Error => {
log_info("パイプライン中断: diff 取得失敗。");
return EXIT_DIFF_FAILURE;
}
}
}

// Stage 2: takt
if !run_takt(&config.takt) {
if !skip_takt && !run_takt(&config.takt) {
log_info("パイプライン中断: takt ワークフロー失敗。");
return 2;
return EXIT_TAKT_FAILURE;
}

// Stage 3: push
if !run_push(&config.push) {
log_info("パイプライン中断: push 失敗。");
return 3;
return EXIT_PUSH_FAILURE;
}

let elapsed = start.elapsed();
log_info(&format!("パイプライン完了 ({:.0}s)", elapsed.as_secs_f64()));
0
EXIT_SUCCESS
}

fn main() {
Expand Down
34 changes: 22 additions & 12 deletions src/cli-push-runner/src/stages/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ use std::process::Command;
use crate::config::DiffConfig;
use crate::log::log_stage;

#[derive(Debug, PartialEq)]
pub(crate) enum DiffResult {
/// diff に内容があり、ファイルへの書き出しが完了した
HasContent,
/// diff 出力が空 (レビュー対象なし、push は続行可能)
Empty,
/// diff コマンドの実行またはファイル書き出しに失敗した
Error,
}

/// diff 取得専用: 出力を切り詰めずに全行を取得する。
/// runner::run_cmd は MAX_LINES=40 で打ち切るため diff には使えない。
fn run_diff_cmd(cmd: &str) -> Result<String, String> {
Expand All @@ -20,7 +30,7 @@ fn run_diff_cmd(cmd: &str) -> Result<String, String> {
}
}

pub(crate) fn run_diff(config: &DiffConfig) -> bool {
pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult {
log_stage("diff", &format!("実行: {}", config.command));

let output = match run_diff_cmd(&config.command) {
Expand All @@ -30,23 +40,23 @@ pub(crate) fn run_diff(config: &DiffConfig) -> bool {
if !err.is_empty() {
eprintln!("{}", err);
}
return false;
return DiffResult::Error;
}
};

if output.is_empty() {
log_stage(
"diff",
"diff 出力が空です。レビュー対象の変更がありません。diff コマンドの revision 指定を確認してください。",
"diff 出力が空です。レビューをスキップして push に進みます。",
);
return false;
return DiffResult::Empty;
}

let path = Path::new(&config.output_path);
if let Some(parent) = path.parent() {
if let Err(e) = std::fs::create_dir_all(parent) {
log_stage("diff", &format!("ディレクトリ作成失敗: {}", e));
return false;
return DiffResult::Error;
}
}

Expand All @@ -57,11 +67,11 @@ pub(crate) fn run_diff(config: &DiffConfig) -> bool {
"diff",
&format!("書き出し完了: {} ({} 行)", config.output_path, line_count),
);
true
DiffResult::HasContent
}
Err(e) => {
log_stage("diff", &format!("ファイル書き出し失敗: {}", e));
false
DiffResult::Error
}
}
}
Expand All @@ -84,9 +94,8 @@ mod tests {
}

#[test]
fn run_diff_returns_false_when_output_is_empty() {
fn run_diff_returns_empty_when_output_is_empty() {
let out_path = std::env::temp_dir().join("test-run-diff-empty.txt");
// Ensure a clean slate in case a previous run left the file.
let _ = std::fs::remove_file(&out_path);

let config = DiffConfig {
Expand All @@ -97,9 +106,10 @@ mod tests {

let result = run_diff(&config);

assert!(
!result,
"run_diff must return false when the diff command produces empty output"
assert_eq!(
result,
DiffResult::Empty,
"run_diff must return Empty when the diff command produces empty output"
);
assert!(
!out_path.exists(),
Expand Down
2 changes: 1 addition & 1 deletion src/cli-push-runner/src/stages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ mod push_jj_bookmark;
mod quality_gate;
mod takt;

pub(crate) use diff::run_diff;
pub(crate) use diff::{run_diff, DiffResult};
pub(crate) use push::run_push;
pub(crate) use quality_gate::run_quality_gate;
pub(crate) use takt::run_takt;
2 changes: 1 addition & 1 deletion src/cli-push-runner/src/stages/push.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::push_jj_bookmark::advance_jj_bookmarks;
use crate::config::{PushConfig, DEFAULT_PUSH_TIMEOUT_SECS};
use crate::log::log_stage;
use crate::runner::run_stage_cmd;
use super::push_jj_bookmark::advance_jj_bookmarks;

pub(crate) fn run_push(config: &PushConfig) -> bool {
// (takt fix や手動 jj describe で @ が進んでも bookmark が旧コミットのまま残る問題の対策)
Expand Down
Loading