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
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