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
30 changes: 30 additions & 0 deletions docs/adr/adr-004-stop-hook-quality-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@ Claude Code の Stop フック入力には `stop_hook_active` フラグが含ま
これにより最大1回のリトライで収束する。エージェントは1回目の停止で品質チェックの結果を受け取り、
修正を試みた後に再度停止を試みる。2回目は `stop_hook_active: true` なのでそのまま停止が許可される。

### takt subsession skip (2026-06-26 追加、PR-W1 follow-up)

takt workflow が起動する subsession (例: weekly-review の whole-tree reviewer / post-merge-feedback の analyze-pr / analyze-session / analyze-prepush-reports) は **`edit: false` で起動される read-only な分析セッション** が多い。これらの subsession で Stop フックが品質ゲート失敗を返すと、subsession は `edit: false` 制約と矛盾する「直せ」指示を受け取り、稀に **stray edit を試みる事故** が発生する (2026-06-26、PR #221 で観測。post-merge-feedback subsession が `src/lib-report-formatter/src/lib.rs` を意図せず編集)。

そもそも品質ゲートの趣旨は **本対話セッションの品質担保** であり、takt subsession に適用すべきではない (= 本 ADR の責務範囲外)。よって以下の条件で品質ゲートを skip する:

- `.takt/runs/*/meta.json` を scan し、いずれかが **`status: "running"` かつ mtime が `ACTIVE_RUN_FRESH_THRESHOLD_SECS` (= 1500s) 以内** であれば skip
- 1 件目が見つかった時点で短絡 return (= I/O 最小化)
- malformed JSON / read error / mtime 取得失敗 / 未来時刻 (clock skew) は defensive に skip (= active 扱いしない、fail-closed)

#### freshness check の必要性 (CR PR #222 Major 指摘対応)

`status: "running"` は **abrupt termination (kill -9 / SIGKILL / power loss / OOM)** で残った orphan run でも残り続ける。hooks-session-start の reaper module (ADR-030 §L2) は SessionStart 時のみ scan するため、reaper 発火前の Stop event では古い orphan run が `.takt/runs/` に残存している可能性がある。

orphan を fresh subsession と同一視すると、**1 つの orphan が残っているだけで以降の全ての通常セッションの品質ゲートが永続的に skip される** 致命的な regression が発生する (= ADR-004 の趣旨「本対話セッションの品質担保」が完全に崩れる)。

mtime ベースの freshness check (= takt の TAKT_TIMEOUT_SECS 1200s + 5 分余裕 = 1500s 以内) を AND 条件として追加することで、orphan の永続 skip 問題を構造的に防ぐ。1500s 閾値は reaper の `ORPHAN_THRESHOLD_SECS` と同値で、**両者が「これ以上の age は abrupt termination」と判定する共通契約** を形成する。

#### 同 marker の他用途

`.takt/runs/<slug>/meta.json` の `status` field は ADR-030 (= 決定論的 post-merge-feedback) の `.failed` marker 経路と、`hooks-session-start` の reaper module (ADR-030 §L2 out-of-process orphan run 検出) でも使われており、本 ADR の追加判定は既存 marker の **読み取り側責務拡張** のみで実装される (新規 marker 不要、既存設計を再利用)。

#### 実装の所在

[src/hooks-stop-quality/src/main.rs](../../src/hooks-stop-quality/src/main.rs) の `should_skip_quality_gate()` で `stop_hook_active` チェック直後に `takt_subsession_active()` を呼ぶ 2 段判定。test 9 件で各種ケース (no runs dir / no meta / status=completed のみ / status=running 混在 / malformed JSON 等) を網羅。

#### 由来事例

PR-3a 系統で複数 PR を local で iterative に merge していた最中、新 PC で `.jj/repo/config.toml` の `auto-track-bookmarks` 設定欠落により merge-pipeline の `sync_local()` が stale local master を base にしたことが root cause。働きとして stale tree 上で `cargo clippy` が `unnecessary_sort_by` warning を flag し、後続の post-merge-feedback subsession に Stop hook 経由で「修正せよ」指示が伝達された (= 連鎖の半分)。merge-pipeline 側の根本修正は [ADR-013](adr-013-merge-pipeline.md) § sync_local の前提条件 を参照。本 ADR の subsession skip は **同型事故の多層防御** として導入。

### 出力形式

品質ゲート失敗時:
Expand Down
23 changes: 20 additions & 3 deletions docs/adr/adr-013-merge-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Push Pipeline (ADR-008) と同様の「ガード + 専用 CLI」パターンで
### 現状の問題

1. **`gh pr merge` の直接実行**: マージ後にローカルの jj 環境を同期し忘れるリスクがある
2. **手動ステップの多さ**: マージ → fetch → new master を毎回手動で実行するのは煩雑
2. **手動ステップの多さ**: マージ → fetch → new master@origin を毎回手動で実行するのは煩雑
3. **将来の拡張**: マージ後に「直前の PR から学びを抽出し、次の開発に活かす」機能を追加する余地を確保したい

### 検討した選択肢
Expand Down Expand Up @@ -50,7 +50,7 @@ cli-merge-pipeline.exe (スタンドアロン)
├─ jj bookmark → gh pr list --head で PR を自動検出
├─ pre_steps を順次実行(マージ前チェック)
├─ gh pr merge --squash --delete-branch を実行
├─ jj git fetch && jj new master でローカル同期
├─ jj git fetch && jj new master@origin でローカル同期
└─ post_steps を順次実行(学び提案等の拡張ポイント)
```

Expand All @@ -61,7 +61,7 @@ cli-merge-pipeline.exe (スタンドアロン)
| マージ戦略 | squash 固定 | master の履歴を 1 PR = 1 コミットに保つ |
| PR 検出 | jj bookmark から自動検出 | `pnpm push` / `pnpm create-pr` と同じ方式で一貫性がある |
| ブランチ削除 | `--delete-branch` で自動削除 | マージ済みブランチの残留を防ぐ |
| ローカル同期 | `jj git fetch` + `jj new master` | マージ後すぐに master 最新から作業を開始できる |
| ローカル同期 | `jj git fetch` + `jj new master@origin` | マージ後すぐに master 最新から作業を開始できる。`master@origin` (= remote tracking ref) を直接参照することで local master bookmark の状態に依存しない (詳細: 後述「§ sync_local の前提条件」) |
Comment thread
coderabbitai[bot] marked this conversation as resolved.
| ステップ分離 | `pre_steps`(マージ前)/ `post_steps`(マージ後) | 学び提案等の post-merge 処理を正しいタイミングで実行 |
| 学び提案機能 | 将来実装(`post_steps` に `type = "ai"` ステップ) | config に追加するだけで拡張可能 |

Expand All @@ -84,6 +84,23 @@ step_timeout = 120
# prompt = "analyze_pr_learnings"
```

### sync_local の前提条件 (2026-06-26 追加、PR-W1 follow-up)

`sync_local()` は **squash マージで origin に新コミット (= マージ済 tip) が出来た直後** に、その新 tip を base にした空の作業コピーを置くことが責務。実装は以下の 2 ステップ:

1. `jj git fetch` で `master@origin` を最新化
2. `jj new master@origin` で remote tracking ref を base に新 commit を切る

`master@origin` (= remote tracking ref) を直接参照する設計上の理由:

- **local bookmark `master` の状態に依存しない**: jj は `jj git fetch` 時に local bookmark を自動 fast-forward させるかどうかが `.jj/repo/config.toml` の `[remotes.origin] auto-track-bookmarks` 設定に依存する。設定が無いと local master は古い tip に固定され、`jj new master` (= local bookmark 参照) は stale な base に着地してしまう
- **`master@origin` は jj clone 直後から自動生成される**: 設定なしで必ず存在する ref のため、新 PC / fresh clone でも前提条件を満たす
- **ADR-011 (push 戦略) との分離**: ADR-011 が確立した `auto-track-bookmarks = "*"` 設定は push の関心領域 (新規 bookmark の auto-track) のためのもの。merge-pipeline は同設定の副作用 (= local bookmark の fast-forward) に偶発的に依存していたが、本設計でその依存を解消した

#### 過去の不具合 (2026-06-26 観測)

新 PC で `.jj/repo/config.toml` に `auto-track-bookmarks` 設定が無い状態で merge-pipeline を実行したところ、stale local master に作業コピーが乗り、`post_steps` の post-merge-feedback subsession が古い lint warning (`unnecessary_sort_by`) を「fix」しようとして `src/lib-report-formatter/src/lib.rs` を stray 編集する事故が発生した。原因連鎖の半分が本 sync_local 設計のバグであり、本 ADR 改訂と [src/cli-merge-pipeline/src/main.rs](../../src/cli-merge-pipeline/src/main.rs) の修正で根本解消した。残り半分の連鎖 (Stop hook の subsession 無差別発火) は [ADR-004](adr-004-stop-hook-quality-gate.md) § takt subsession skip で多層防御を入れている。

## 影響

### Positive
Expand Down
37 changes: 34 additions & 3 deletions src/cli-merge-pipeline/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,14 @@ fn run_pipeline() -> i32 {
0
}

/// jj git fetch → jj new <branch> でローカルを最新に同期する
/// jj git fetch → jj new <branch>@origin でローカルを最新に同期する。
///
/// `<branch>@origin` は remote tracking ref への直接参照で、local bookmark の
/// 状態に依存しない。`<branch>` のみ (= local bookmark) を渡すと
/// `.jj/repo/config.toml` の `[remotes.origin] auto-track-bookmarks = "*"` 設定が
/// 無い環境で `jj git fetch` 後も local bookmark が古い tip に固定され、
/// stale code に working copy が乗る (= post-merge-feedback subsession が
/// 古い lint warning を「fix」しようとして stray edit する事故、ADR-013 参照)。
fn sync_local(branch: &str) -> i32 {
log_info("ローカル同期中: jj git fetch");
let (success, output) = run_cmd_shell_capped_reporting(
Expand All @@ -682,7 +689,7 @@ fn sync_local(branch: &str) -> i32 {
return 1;
}

let new_cmd = format!("jj new {}", branch);
let new_cmd = sync_local_new_command(branch);
log_info(&format!("ローカル同期中: {}", new_cmd));
let (success, output) =
run_cmd_shell_capped_reporting("new-branch", &new_cmd, DEFAULT_STEP_TIMEOUT_SECS, MAX_LINES);
Expand All @@ -695,12 +702,17 @@ fn sync_local(branch: &str) -> i32 {
}

log_info(&format!(
"ローカル同期完了。{} の最新状態で作業を開始できます。",
"ローカル同期完了。{}@origin の最新状態で作業を開始できます。",
branch
));
0
}

/// `jj new <branch>@origin` の command 文字列を組み立てる (test 用に切り出し)。
fn sync_local_new_command(branch: &str) -> String {
format!("jj new {}@origin", branch)
}

fn main() {
std::process::exit(run_pipeline());
}
Expand Down Expand Up @@ -764,6 +776,25 @@ prompt = "analyze_pr_learnings"
);
}

#[test]
fn sync_local_new_command_references_remote_tracking_ref_for_master() {
assert_eq!(sync_local_new_command("master"), "jj new master@origin");
}

#[test]
fn sync_local_new_command_references_remote_tracking_ref_for_main() {
assert_eq!(sync_local_new_command("main"), "jj new main@origin");
}

#[test]
fn sync_local_new_command_never_references_bare_local_bookmark() {
let cmd = sync_local_new_command("master");
assert!(
cmd.contains("@origin"),
"sync_local must use remote tracking ref (master@origin), never bare local bookmark — ADR-013 § sync_local 設計"
);
}

#[test]
fn should_skip_branch_delete_true_for_fork_pr() {
let info = PrHeadInfo {
Expand Down
Loading