Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ src/*/Cargo.lock
.claude/.session-id
.claude/pr-monitor-state.json
.claude/pr-monitor-state.json.tmp
.claude/pipeline.lock
.claude/pr-monitor.lock
.claude/scheduled_tasks.lock
# ADR-029: post-merge-feedback の pending file (cli-merge-pipeline 生成、skill が consume する一時 artifact)
Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion docs/adr/adr-045-jj-workspace-parallel-sessions.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ src の大規模分割 (メイン) と lint/facet/docs (改善) は編集領域
6. マージ等の repo 境界操作の前に、background task (monitor / lint 等) の完了を確認する
7. 出力混線 (重複・欠落・身に覚えのないテキスト) を発見したら、直ちに両セッションを停止し、`jj op log` と会話ログを保存してから再開する

補足 — Stop hook との競合は機構で防止済み (2026-07-13、順位 280 実装): merge-pipeline / push-runner は実行区間で `.claude/pipeline.lock` を保持し、hooks-stop-quality は fresh な lock を検知すると品質ゲートを skip する (fail-open、`lib-jj-helpers::pipeline_lock`)。これにより「background の merge pipeline がローカル同期の checkout 実行中に、ターン終了で発火した Stop hook の cargo/jj が Concurrent checkout を誘発する」事故 (PR #267 マージで実観測) は構造的に再発しない。lock 実装前の暫定運用だった「merge をターン保持 (foreground 相当) で実行する」は不要になった。stale threshold は 30 分 (クラッシュした pipeline の lock は自動失効)。

### Operation Verification Checklist (2026-07-13 新設、暫定手順)

変更系 jj 操作 (`new` / `abandon` / `describe` / `rebase` / `squash` / `git fetch` / `git push`) の直後に、operation が記録されたことを確認する:
Expand All @@ -126,7 +128,8 @@ jj op log --limit 1 --no-graph
- 直前の操作に対応する op (description が操作内容と一致) が先頭にあること
- 無い場合は「operation not recorded」= 上記 output corruption リスクの兆候。作業を止めて状態を確認する
- `jj op log` は working copy を snapshot しない (副作用なし) ため、確認自体は安全
- 本手順は PostToolUse hook による自動化 (todo 順位 275-278 と同経緯の feedback 採用分) が実装されるまでの暫定。hook 実装後は自動検証に置き換わる
- 本手順は `hooks-post-tool-jj-op-verify` (Bash PostToolUse、PR #267 で実装) により自動化済み。手動確認は hook 無効時・hook 対象外の verb を使う場合の fallback
- hook 運用の既知の注意 (2026-07-13 実観測): Bash tool がコマンドを background 実行した場合、hook はコマンド完了前の PostToolUse 時点で op log を見るため「operation not recorded」警告が出ることがある。この場合は task 完了後に `jj op log` で実状態を確認する (警告は誤報だが、確認を促す方向の誤りなので安全側)

### マージ方法 (各 workspace で独立)

Expand Down
4 changes: 0 additions & 4 deletions docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@
| 275 | 🔧 Tier 2 | **層別テストテンプレート (StubOllama パターン・integration 独立性) の共有化 (PR #265 post-merge-feedback T2-1 採用)** | todo13.md | M | なし (WP-11/ADR-054 の多層防御実装で「空 StubOllama による LLM 未呼び出し証明」「tempdir+jj init+CwdRestore の integration 独立性」を都度設計。WP-17 の classifier/scope guard 拡張で同種判断が再発見込み。shared crate 化の境界は ADR-044 で判定、WP-17 着手前の実施が効果的) |
| 276 | 💎 Tier 3 | **ADR-007 に「コメント配置の意思決定フロー」を追加 (PR #265 post-merge-feedback T3-2 採用)** | todo13.md | S | なし (PR #265 で非 doc コメントの Bundle Z block が 2 回発生 = doc コメント/識別子名/マーカー付き Why の配置判断が未文書化。linter 自動化は NLP 必要で却下済み、既存 Q1-Q3 形式で人間/AI の判断補助を doc 化。バッチ PR で消化可) |
| 277 | 💎 Tier 3 | **PR body 配置タイミング規約を dev-conventions に明記 (PR #265 post-merge-feedback T3-3 採用)** | todo13.md | XS | なし (push パイプライン実行中の working copy に `__pr-body.md` を作成し snapshot 混入をかろうじて回避したヒヤリハット実発生。「push 完了後に scratchpad で準備し --body-file に絶対パス」を規約化。バッチ PR 消化可、並列安全化 PR docs への相乗りも可) |
| 280 | 🚀 Tier 1 | **pipeline lock + Stop hook 品質ゲート skip 機構 (PR #267 マージ事故の根本解決)** | todo13.md | S-M | なし (background merge pipeline の checkout と Stop hook 品質ゲートの Concurrent checkout 競合が実発生。--feedback-only (PR #268) は対症療法で競合自体は防げない。lock.rs パターン lib 化 + hooks-stop-quality の skip。**PR #268 の次の PR で対応予定 (2026-07-13 合意)**、実装後に ADR-045 の「merge は foreground」暫定ルールを撤去) |
| 281 | 🚀 Tier 1 | **config-reading hook の current_dir() 解決を検出する lint rule (PR #267 post-merge-feedback T1-1 採用)** | todo13.md | S | なし (新規 hook が cwd 基準 config 解決を実装し pre-push REJECT → fix 修正の実例。cwd drift による silent fail-open は新規 hook のたびに再発しうる。Severity High。順位 287 と同一 PR bundle 推奨) |
| 282 | 🚀 Tier 1 | **jj-op-verify の変更系 verb 網羅拡大 — undo/restore/split/bookmark move 等 (PR #267 post-merge-feedback T1-2 採用)** | todo13.md | M | なし (特に `jj undo` の検出漏れは lost-update 再発リスク高。拡張時は expected_op_keyword を jj 0.42 実機の op log 出力と要照合) |
| 283 | 🚀 Tier 1 | **jj-op-verify の verb 検出を command-boundary に anchor (PR #267 post-merge-feedback T1-3 採用)** | todo13.md | S | なし (commit message 引用符内の "jj new" 等での false positive 防止。実装時に accepted risk で一度見送った経緯あり = 着手時に実観測 0 件のままか再確認。順位 285 と表裏) |
Expand All @@ -137,9 +136,6 @@
| 286 | 🔧 Tier 2 | **config path 解決の cwd 跨ぎ integration test (PR #267 post-merge-feedback T2-3 採用)** | todo13.md | M | なし (FIXED 済 cwd-config bug の regression guard。既存テストは pure parser のみで file-lookup 経路未カバー。Severity High、Adoption Risk = OS 依存) |
| 287 | 💎 Tier 3 | **「config 読み hook は exe-relative 解決必須」convention の明文化 (PR #267 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (順位 281 の文書層補完。**281 と同一 PR bundle 推奨**、別作業に切り出す価値は低い) |
| 288 | 🔧 Tier 2 | **post-merge feedback の pre-push reports を対象 PR の全 run 集約に拡張 (PR #268 post-merge-feedback T2-1 採用)** | todo13.md | M | なし (「最新 1 run」参照は複数 push した PR で分析が最終 push 分に偏る。PR #267 feedback の evidence-scope 注記で実観測。context の prepush_reports_dir 配列化 + facet 複数 dir 対応。独立 PR 推奨) |
| 289 | 💎 Tier 3 | **run_feedback_only の docstring 修正 — 検出失敗パスは marker を書かない旨を明記 (PR #268 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (現行 docstring「marker は通常経路と同様に残る」と実装 (owner_repo 失敗パスは marker なし = 意図的) の drift 修正。**順位 280 PR に同乗推奨**) |
| 290 | 🚀 Tier 1 | **cli-push-runner の bookmark 検出を ::@ (自 workspace 祖先) に限定 (PR #269 post-merge-feedback T1-1 採用)** | todo13.md | S | なし (jj bookmark list はリポジトリ全体対象のため並行 workspace の bookmark も -b 付与対象に含む。実 push で複数 bookmark 付与を実観測、feedback と dogfood が独立に同一検出。--all 廃止の仕上げ。**順位 280 PR で消化予定**) |
| 291 | 🔧 Tier 2 | **run_ai_step_for の Result 伝播 regression test (PR #269 post-merge-feedback T2-1 採用)** | todo13.md | S | なし (SIM-NEW-pipeline-L224 = path.exists() 偽陽性 PASS の修正を両呼び出し元で固定。stale report 存在下の再実行失敗が exit 0 にならないことを assert。順位 280 PR 同乗可) |

**戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。

Expand Down
77 changes: 0 additions & 77 deletions docs/todo13.md
Original file line number Diff line number Diff line change
Expand Up @@ -890,29 +890,6 @@

---

### pipeline lock + Stop hook 品質ゲート skip 機構 (PR #267 マージ事故の根本解決)

> **動機**: PR #267 のマージで「background の merge pipeline がローカル同期の checkout を実行中に、ターン終了で発火した Stop hook 品質ゲート (cargo/jj) が同じ working copy 上で競合」し、jj が「Concurrent checkout」で中断・working copy が旧状態に取り残される事故が実発生 (jj 側の保護と自動解決で損失ゼロ、復旧 3 コマンド)。実施済みの `--feedback-only` (PR #268) は結果の一つ (feedback 未実行) の回復手段にすぎず、競合自体は防げていない。post-merge feedback の transcript window (first_commit〜merged_at) はマージ処理中の事故を構造的に見えないため、feedback からの提案も期待できない。
>
> **設計案**: (1) cli-merge-pipeline / cli-push-runner が実行中に lock ファイル (PID + timestamp、`cli-pr-monitor/src/lock.rs` の stale takeover パターンを lib 化して流用) を保持、(2) hooks-stop-quality が開始時に lock を確認し、生きている pipeline 保持中は品質ゲートを skip (ログ + fail-open。Stop 時点のゲートは助言層で、本物のゲートは push 側)、(3) ADR-045 運用ルールに「lock 機構実装までは merge-pr を foreground 実行」の暫定ルールを追記済み → 実装後に撤去。
>
> **参照**: ADR-045 § Known operational risks、ADR-030 (feedback recovery)、ADR-043 (skip は助言層ゲートの fail-open で整合)、`src/cli-pr-monitor/src/lock.rs` (流用元)、PR #268 (`--feedback-only` = 対症療法側)
>
> **実行優先度**: 🚀 Tier 1 — Effort S-M。**PR #268 の次の PR で対応予定 (ユーザー合意済み 2026-07-13)**。

#### 作業計画

- [ ] lock を lib (lib-jj-helpers or 新 lib) に一般化 (PID + timestamp + stale takeover)
- [ ] cli-merge-pipeline / cli-push-runner の実行区間で lock 保持
- [ ] hooks-stop-quality に lock 検知 → skip (fail-open) を追加 + off/stale ケースのテスト
- [ ] ADR-045 の「merge は foreground」暫定ルールを撤去し本機構に置換
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- background の merge/push pipeline 実行中にターンを終了しても Stop hook 品質ゲートが working copy に触れず、Concurrent checkout 事故が構造的に再発しないこと。

---

### config-reading hook の `current_dir()` 解決を検出する lint rule (PR #267 post-merge-feedback T1-1 採用)

Expand Down Expand Up @@ -1068,62 +1045,8 @@

---

### run_feedback_only の docstring 修正 — 検出失敗パスは marker を書かない旨を明記 (PR #268 post-merge-feedback T3-1 採用)

> **動機**: 現行 docstring は「失敗時 (marker は通常経路と同様に残る)」と記すが、owner_repo 検出/validation 失敗パスでは marker を書かない (同期 CLI で人間が直接ログを見るため意図的)。spec-impl drift の芽を摘む。
>
> **参照**: `.claude/feedback-reports/268.md` Tier 3 #1、`src/cli-merge-pipeline/src/pipeline.rs` (`run_feedback_only` docstring)
>
> **実行優先度**: 💎 Tier 3 — Effort XS。**順位 280 の実装 PR に同乗推奨** (cli-merge-pipeline を触る同一 PR で消化)。

#### 作業計画

- [ ] docstring の終了コード契約の記述を実装に合わせて修正
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- docstring と実装の marker 挙動が一致していること。

---

### cli-push-runner の bookmark 検出を `::@` (自 workspace 祖先) に限定 (PR #269 post-merge-feedback T1-1 採用)

> **動機**: bookmark_check の検出 (`jj bookmark list`) はリポジトリ全体を対象とするため、並行 workspace の bookmark も拾い、push の `-b` 付与対象に含めてしまう。本セッションの実 push で `-b <PR#268の bookmark> -b <PR#269の bookmark>` と複数 bookmark が付与された実観測あり (両方自分のもので無害だったが、並行 workspace では他者の作業中 bookmark を巻き込む余地)。`--all` 廃止 (PR #267) の仕上げとして、検出を `::@ ~ trunk()` 等の revset で自 workspace の祖先に限定する。feedback pipeline とセッション内 dogfood が独立に同一問題を検出 (相互裏付け)。
>
> **参照**: `.claude/feedback-reports/269.md` Tier 1 #1、`src/cli-push-runner/src/stages/bookmark_check.rs`、ADR-045 § Known operational risks (bookmark conflicts)
>
> **実行優先度**: 🚀 Tier 1 — Effort S。**順位 280 の実装 PR で消化予定** (並列安全化の仕上げとして同一テーマ)。

#### 作業計画

- [ ] bookmark 検出を revset ベース (`jj log -r 'bookmarks() & ::@ ~ trunk()'` 等) に変更。`::@` revset のみでは自 workspace 所有の保証にならない (祖先 commit が並行 workspace と共有され得る) ため、workspace root commit の照合等、追加の所有権検証を組み合わせる + テスト
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- push の `-b` 付与対象が自 workspace の祖先にある bookmark に限定され、`::@` revset のみに依存しない追加の所有権検証 (workspace root commit の照合等) を伴うこと。

---

### run_ai_step_for の Result 伝播 regression test (PR #269 post-merge-feedback T2-1 採用)

> **動機**: PR #268 の pre-push review で REJECT された `path.exists()` 偽陽性 PASS (SIM-NEW-pipeline-L224) の修正 (`Result<PathBuf, String>` の直接伝播) を、両呼び出し元 (`run_feedback_only` / `run_ai_step`) で regression test として固定する。stale report 存在下での再実行失敗が exit 0 にならないことの検証が核心。
>
> **参照**: `.claude/feedback-reports/269.md` Tier 2 #1、`src/cli-merge-pipeline/src/pipeline.rs` (`run_ai_step_for`)
>
> **実行優先度**: 🔧 Tier 2 — Effort S。順位 280 の実装 PR に同乗可 (cli-merge-pipeline を触る場合)。

#### 作業計画

- [ ] Result 伝播の unit/regression test を追加 (stale report + Err ケースで exit 1 を assert)
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- 偽陽性 PASS バグの再発がテストで検出されること。

---

## 既知課題 (記録のみ、本セッションで未対応)

Expand Down
3 changes: 3 additions & 0 deletions src/cli-merge-pipeline/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@ lib-jj-helpers = { path = "../lib-jj-helpers" }
lib-pending-file = { path = "../lib-pending-file" }
lib-subprocess = { path = "../lib-subprocess" }

[dev-dependencies]
tempfile = "3"

# [profile.release] は workspace root (Cargo.toml) に集約 (ADR-026)
45 changes: 43 additions & 2 deletions src/cli-merge-pipeline/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,13 @@ fn run_ai_step(label: &str, ctx: Option<&PipelineContext>) {
/// ADR-030 L2 recovery の対象にならない。本経路は marker の有無に依存せず feedback
/// workflow を単独で再実行する。マージ済み PR の番号を明示指定する前提。
///
/// 終了コード: 0 = report 生成成功、1 = 失敗 (marker は通常経路と同様に残る)。
/// 終了コード: 0 = report 生成成功、1 = 失敗。
///
/// marker の扱い (順位 289): feedback workflow 本体の失敗では通常経路と同様に
/// `.failed` marker が残る (`run_ai_step_for` 内の `FailedMarkerGuard`)。ただし
/// **owner_repo の検出/validation 失敗パスでは marker を書かない** — 本経路は同期 CLI
/// で人間が直接ログを見る前提のため、L2 recovery (UserPromptSubmit hook) への引き継ぎは
/// 不要という意図的な設計。
pub(crate) fn run_feedback_only(pr_number: u64) -> i32 {
let label = "feedback-only";
let Some(owner_repo) = detect_owner_repo() else {
Expand All @@ -222,7 +228,17 @@ pub(crate) fn run_feedback_only(pr_number: u64) -> i32 {
return 1;
}

match run_ai_step_for(label, pr_number, &owner_repo) {
feedback_only_outcome(label, run_ai_step_for(label, pr_number, &owner_repo))
}

/// `--feedback-only` の終了コードを**本呼び出しの Result のみ**から導出する。
///
/// 順位 291 の regression guard: 初版はディスク上の report 存在 (`path.exists()`) を
/// 成功根拠にしており、stale report が存在すると失敗した再実行でも exit 0 を返す
/// hidden-coupling があった (pre-push simplicity-review REJECT: SIM-NEW-pipeline-L224)。
/// 本関数はファイルシステム状態を一切参照しない。
fn feedback_only_outcome(label: &str, result: Result<PathBuf, String>) -> i32 {
match result {
Ok(report) => {
log_step(
label,
Expand Down Expand Up @@ -416,6 +432,8 @@ fn build_context() -> Result<PipelineContext, i32> {
}

pub(crate) fn run_pipeline() -> i32 {
let _pipeline_lock = lib_jj_helpers::pipeline_lock::hold_pipeline_lock("merge", log_info);

let settings = match resolve_settings() {
Ok(s) => s,
Err(code) => return code,
Expand Down Expand Up @@ -671,4 +689,27 @@ mod tests {
}
);
}

/// 順位 291 regression guard: 終了コードは本呼び出しの Result のみから導出され、
/// ディスク上の stale report の存在に影響されない (SIM-NEW-pipeline-L224 の再発防止)。
/// 「stale report が存在する状態で Err → exit 1」が incident の再現シナリオ。
#[test]
fn feedback_only_outcome_fails_on_err_even_when_stale_report_exists() {
let temp = tempfile::tempdir().expect("tempdir");
let stale_report = temp.path().join("267.md");
std::fs::write(&stale_report, "stale report from previous run").unwrap();

let code = feedback_only_outcome(
"test",
Err("concurrent run guard trip".to_string()),
);
assert_eq!(code, 1, "stale report が存在しても Err は exit 1");
assert!(stale_report.exists(), "前提: stale report は存在したまま");
}

#[test]
fn feedback_only_outcome_succeeds_only_from_ok_result() {
let code = feedback_only_outcome("test", Ok(PathBuf::from("reports/267.md")));
assert_eq!(code, 0);
}
}
2 changes: 2 additions & 0 deletions src/cli-push-runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ fn run_pipeline() -> i32 {
}
};

let _pipeline_lock = lib_jj_helpers::pipeline_lock::hold_pipeline_lock("push", log_info);

let has_diff = config.diff.is_some();
let workflow = resolve_takt_workflow(&config);
log_info(&format!(
Expand Down
Loading
Loading