diff --git a/docs/adr/adr-030-deterministic-post-merge-feedback.md b/docs/adr/adr-030-deterministic-post-merge-feedback.md index 197e363..f1d7746 100644 --- a/docs/adr/adr-030-deterministic-post-merge-feedback.md +++ b/docs/adr/adr-030-deterministic-post-merge-feedback.md @@ -193,6 +193,68 @@ PR #78 dogfood で発覚した **Windows の `child.kill()` が takt の descend - takt が timeout で kill されたが、orphan が後から report を書き終えた - takt が exit=non-zero を返したが、aggregate-feedback は完了していた +#### Abrupt 終了の多層 recovery (Bundle c-1 で追加) + +PR #109 マージ直後の post-merge-feedback workflow が SIGPIPE で silent 中断され `.failed` marker 未生成という failure mode が実証された。原因は `feedback::run` が `Result::Err` を返した場合のみ `write_failed_marker` を呼ぶ設計で、Rust default の SIGPIPE 動作 (`SIG_DFL` = unwind せず process 終了) では `Result::Err` 経路にも Drop 経路にも到達しないため。本節は ADR-030 "失敗マーカーによる recovery" の決定論性を **abrupt 終了系 (SIGPIPE / SIGTERM / kill -9 / SIGKILL / power loss / OOM Killer / panic) でも担保するための多層構造** を spec として明記する。 + +##### L1: in-process recovery + +`cli-merge-pipeline::feedback::run` 内で **pre-emptive `.failed` marker** + **RAII Drop guard** の 2 機構で marker 存在を保証する: + +| 機構 | 動作 | カバー範囲 | +|---|---|---| +| pre-emptive marker | `feedback::run` の `check_concurrent_run_guard` 直後に `write_pending_marker` で `.failed` marker を先制書込み。正常完了時のみ `cleanup_failed_marker` で削除 | **SIGPIPE / SIGTERM / kill -9 / SIGKILL** など unwind せず即時 process 終了する経路 (Rust default では Drop は走らない) | +| RAII Drop guard (`FailedMarkerGuard`) | `armed = true` で生成。Drop 時に marker 存在を idempotent check し、欠落していれば backup marker を書込み。`disarm()` 呼出で no-op 化 | **panic / 早期 return** など Drop が走る経路。caller が detailed marker を書いた後でも idempotent (既存 marker は overwrite しない) | + +正常 path: +1. `write_pending_marker` で marker 書込み + `FailedMarkerGuard::new(armed=true)` +2. 全 step 成功 +3. `cleanup_failed_marker` で marker 削除 +4. `marker_guard.disarm()` → armed=false +5. scope 終了、Drop は no-op + +abnormal path (panic / 早期 return): +1. `write_pending_marker` で marker 書込み (armed=true) +2. 途中で panic or `?` で早期 return +3. scope 巻き戻し、Drop が `marker.exists() = true` を確認 → no-op (pre-emptive marker が既に在る) + +abrupt path (SIGPIPE / SIGKILL 等): +1. `write_pending_marker` で marker 書込み (armed=true) +2. process が **unwind せず即時終了** +3. Drop は走らない → しかし pre-emptive marker は既にディスクに残存 + +##### L2: out-of-process recovery (orphan run reaper) + +L1 の pre-emptive marker 書込み **直前** に process が死んだ場合 (例: `feedback::run` を呼び出す直前で OOM Killer 発火、power loss、`std::fs::write` 自体が完了する前の kill -9) は L1 の救済対象外。この極致 case 用に `hooks-session-start` が SessionStart hook で **out-of-process reaper** を走らせる: + +- **scan 対象**: `.takt/runs/*/meta.json` の `status: "running"` AND `task` が `"post-merge-feedback for #"` で始まる run +- **orphan 判定閾値**: `ORPHAN_THRESHOLD_SECS = TAKT_TIMEOUT_SECS + 300 (= 1500s)`。`TAKT_TIMEOUT_SECS` 経過後も `running` のまま放置されている run は abrupt termination で死んだとみなす +- **reap 動作**: `.claude/feedback-reports/.md.failed` marker を生成 + `meta.json` の `status` を `"failed"` に更新 (`reaped_by: "hooks-session-start"` field も追加) +- **冪等性 / false-positive 抑止**: 以下のいずれかに該当する run は reap を skip する: + 1. 既存の `.failed` marker がある (L1 もしくは前回 reaper pass で処理済み) + 2. **`.claude/feedback-reports/.md` 成功レポートが存在する** — 上記 Reconciliation 節で記述した「takt parent kill 後に descendants が report 完成」path では meta.json が `status: "running"` のまま残るが、実際には成功している。reap せず stale meta.json を放置する方が、false-positive の `.failed` marker で `hooks-user-prompt-feedback-recovery` が毎 prompt nag するより害が少ない +- **nudge**: 検出時は SessionStart の `additionalContext` に `[POST_MERGE_FEEDBACK_REAPER]` tag 付きで通知 + +##### 責務分離 + +| 層 | 場所 | 救済対象 | +|---|---|---| +| **L1 floor** (in-process pre-emptive marker) | `cli-merge-pipeline::feedback::run` | SIGPIPE / SIGTERM / kill -9 / SIGKILL / panic / `Result::Err` (= 大半の経路) | +| **L1 backstop** (in-process Drop guard) | 同上 (`FailedMarkerGuard`) | panic / 早期 return での marker 消失防止 (idempotent backup) | +| **L2 reaper** (out-of-process) | `hooks-session-start::compute_reaper_nudge` | pre-emptive write 完了前の OOM Killer / power loss / kill -9。Drop guard で救済不可な致命系の backstop | +| **L2 recovery** (UserPromptSubmit hook) | `hooks-user-prompt-feedback-recovery` | 上記いずれかで生成された `.failed` marker を Claude に通知 | + +L1 と L2 は **重複動作しない**: L1 が marker を書いていれば L2 reaper は `marker.exists()` で skip。L2 が走るのは L1 が完全に効かなかった致命系のみ。 + +##### SLA (post-merge-feedback の完了/失敗保証) + +「post-merge-feedback はマージ後、次のいずれかの状態に **必ず** 遷移する」をステートメントとして規定: + +- **完了 (`.claude/feedback-reports/.md` 生成)**: `pnpm merge-pr` 同期実行内、`TAKT_TIMEOUT_SECS` 以内 +- **失敗 marker 化 (`.failed` marker 残存)**: L1 経路は `feedback::run` の return 時点で確定。L2 経路は **次回 Claude Code SessionStart 時** で確定 (orphan が `ORPHAN_THRESHOLD_SECS` 経過後) + +つまり、L1 のみであれば「マージ後 `TAKT_TIMEOUT_SECS` 以内に完了 or marker 化」が保証される。L2 (致命系の backstop) を含めても「次回 SessionStart 時には必ず marker 化」が保証される。実数値は `cli-merge-pipeline::feedback::TAKT_TIMEOUT_SECS` / `ORPHAN_THRESHOLD_SECS` を参照のこと (本 ADR で数値固定するとコード変更時に drift する)。 + #### 並行起動 guard (Phase B post-fix で追加) cross-invocation context overwrite race の予防として、`feedback::run` の冒頭で `.takt/post-merge-feedback-context.json` の経過時間を確認: diff --git a/docs/local-llm-offload-analysis.md b/docs/local-llm-offload-analysis.md index 5ac07d4..e2fea40 100644 --- a/docs/local-llm-offload-analysis.md +++ b/docs/local-llm-offload-analysis.md @@ -213,7 +213,7 @@ cargo test -p cli-finding-classifier --test lint_screen_evals -- \ | 🛠️ D 前提整備 (順位 109) | ✅ 完了 | #144 (2026-05-11) | `.takt/lint-screen-report.md` に `## Diagnostic` section、real pipeline 経由で warn log が visible | | 🔄 D Round 1 (D-1〜D-3) | ✅ 完遂 | #145/#146/#147/#148 (2026-05-12) | env var override (順位 115) 解消、D-3 で初の real lint_screen 観測 = 1 data point | | 🔄 D Round 2 (D-4〜D-6) | ✅ 完遂 | #150/#151/#152 (2026-05-13) | size ramp-up + verdict variance 観測、累積 6 data points / 4 PR | -| 🔄 D Round 2 (D-7) | 未着手 | (Bundle c-1) | cli-merge-pipeline Drop guard / orphan reaper / ADR-030 spec | +| 🔄 D Round 2 (D-7) | 🚧 進行中 | (Bundle c-1) | cli-merge-pipeline pre-emptive marker + Drop guard / orphan reaper / ADR-030 spec | **Phase E 着手前提条件 = 3-5 PR 累積 dogfood は D-6 完遂時点で既に充足** (4 PR / 6 data points)。D-7 完遂後に Phase E (採否判定) 移行可能。Round 1/2 で観測した **false positive 5 件中 5 件が file-type / scope 混同型** (mistral:7b の context window 内 hook source 周辺 hallucinate) → Bundle k 順位 123 (MD 除外フィルター) の構造的解消対象。詳細 metrics・観測の意義・各 PR の dogfood outcome は [phase-d-outcomes.md](local-llm-offload-phase-d-outcomes.md) 参照。 diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 2fd02b5..c80cfdd 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -46,11 +46,8 @@ | 57 | 🔧 Tier 2 | **Aggregation cap integration test (PR #105 T2-1 採用)** | todo7.md | S | なし (`collect_all_violations` の MAX_VIOLATIONS contract を test 化、将来の lint 追加時に `truncate(MAX)` 削除 regression を防止する explicit 安全網) | | 60 | 💎 Tier 3 | **analyze-session の transcript filter 絞り込み (旧 #A-3)** | todo7.md | M | なし (旧 docs/pipeline-token-efficiency.md #A-3、ADR-036/037 化に伴い計画書削除、本 task のみ todo に移管。analyze-session の input range を PR 作成 commit〜merge に限定して input token 30-50% 削減見込み、dogfood で実測必要) | | 61 | 🔧 Tier 2 | **post-PR 検証フローに CR review.body 手動スキャン step 追加 (PR #108 T2-1 採用)** | todo7.md | XS | なし (PR #108 で analyze-coderabbit が review body の outside diff range comment を検出漏れし line 371/378 の修正が後追い、blind spot の暫定緩和策として手動 checklist を整備) | -| 63 | 🚀 Tier 1 | **cli-merge-pipeline に Drop guard / signal handler 追加 (PR #109 T1-1 採用) ★ Bundle c** | todo7.md | M | なし (PR #109 SIGPIPE 事故で ADR-030「失敗マーカーによる recovery」仕様の構造的違反が実証、Pre-emptive marker + signal trap で abrupt 経路を多層防御) | -| 64 | 🚀 Tier 1 | **orphan run reaper (`meta.json status=running` 5-15 分放置検出 + 自動再起動) (PR #109 T1-2 採用) ★ Bundle c** | todo7.md | M | なし (順位 63 で救済不可の致命系 = kill -9 / SIGKILL / power loss / OOM の backstop、SessionStart hook または cli-pr-monitor 経路で実装) | | 65 | 🚀 Tier 1 | **exe + `--help` を PreToolUse でブロックして src/ Read に誘導 (PR #109 T1-3 採用) ★ Bundle c** | todo7.md | S | なし (PR #109 SIGPIPE の直接トリガ = AI が `cli-merge-pipeline.exe --help` 実行 → exe は --help 未対応で merge 本体実行を構造的に防止、今後追加 exe にも自動適用) | | 66 | 💎 Tier 3 | **長時間 subprocess の pipe truncate 禁止ルールをグローバル明文化 (PR #109 T3-1 採用) ★ Bundle c** | todo7.md | XS | なし (順位 65 = 決定論層、本ルール = 判断ガイド層、`~/.claude/rules/common/development-workflow.md` 等に追加) | -| 67 | 💎 Tier 3 | **ADR-030 に abrupt 終了時の振る舞いを spec として明記 (PR #109 T3-2 採用) ★ Bundle c** | todo7.md | XS | 順位 63 / 64 と同 PR (実装と仕様の整合性確保、L1 in-process Drop guard + L2 out-of-process reaper の責務分離 + SLA 化) | | 69 | 💎 Tier 3 | **`no-ephemeral-todo-reference` の `yaml`/`yml` extensions 追加理由をコメントで明記 (PR #110 T3-1 採用) ★ Bundle d** | todo5.md | XS | なし (rule⑥ コメント欄に 1-2 行追記、設計 doc と実装の経緯保存、git blame 不要化) | | 78 | 💎 Tier 3 | **ADR-038 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (config が user-editable system boundary のとき `sanitize()` 値域検証を必須化し dependent arithmetic に `// SAFETY: により上限保証` コメントを要求するパターンを ADR + CLAUDE.md に codify、Rust 固有の checked_add + MAX_SAFE capping + time-dependent test の 3 層を明文化) | | 79 | 💎 Tier 3 | **`docs-governance.md` § Retirement Workflow に「残タスクの lifecycle 整合」要件明記 (PR #117 T3-1 採用)** | todo5.md | XS | なし (PR #117 で順位 15 を Bb-3 で吸収済として削除した際、現 Step 2「残タスクを priority table に登録」が priority table から除外するケース = 完了/deprioritize/defer を未定義だった実証。除外時の commit/PR で 3 値のいずれかを明示する要件を追加して将来の同型 ambiguity を構造的に防ぐ) | @@ -80,8 +77,9 @@ | 123 | 🚀 Tier 1 | **lint-screen の Markdown ファイル除外フィルター追加 (PR #151 T1-#2 採用、PR #152 で再観測) ★ Bundle k** | todo8.md | M | なし (D-3/D-4/D-5/**D-6** の 4 PR で一貫観測した `.md` への `unused-import` hallucinate FP を構造的に解消。Phase D dogfood 観測から導かれた最も価値ある決定論的防止策、拡張子ベース mechanical filter で実装可能、Frequency High + Adoption Risk None) | | 124 | 🚀 Tier 1 | **`no-ephemeral-todo-reference` rule の TOML positive test 追加 (PR #151 T1-#1 採用、PR #152 で再観測)** | todo8.md | S | なし (extensions 拡張が複数 PR にわたり反復する pattern (yaml/yml = PR #110、toml 等)、test gap 累積を構造的に防ぐ。Frequency Medium で採用基準を満たす) | | 125 | 🔧 Tier 2 | **UTF-8 マルチバイト boundary test を他の string-processing hooks に横展開 (PR #151 T2-#1 採用)** | todo8.md | M | なし (PR #151 で `byte_offset_to_line` char-boundary panic bug を test 拡充で発見、同型関数を持つ他 hooks に systemic 防御を確保。test 拡充が production fault detection に直結する事例の横展開) | -| 126 | 💎 Tier 3 | **ADR-038 に mistral:7b 「diff 外 context hallucinate」failure mode を追記 (PR #151 T3-#1 採用、順位 123 と同 PR 推奨、PR #152 で再観測)** | todo8.md | XS | なし (**4 PR 観測** = High freq の failure mode を ADR codify、Phase b' fixture では再現しない pattern のため永続記録の価値あり、順位 123 と同 PR で実装と仕様の整合性確保) | +| 126 | 💎 Tier 3 | **ADR-038 に mistral:7b 「diff 外 context hallucinate」failure mode を追記 (PR #151 T3-#1 採用、順位 123 と同 PR 推奨、PR #152 / PR #153 で再観測 = 5 PR 連続)** | todo8.md | XS | なし (**5 PR 連続観測** = High freq の failure mode を ADR codify、Phase b' fixture では再現しない pattern のため永続記録の価値あり、順位 123 と同 PR で実装と仕様の整合性確保。PR #153 T3-#1 では root cause + structural fix の両方を明記する要件追加) | | 127 | 💎 Tier 3 | **extensions 拡張時の test 追加 pattern をコード comment で明文化 (PR #151 T3-#2 採用、順位 124 と同 PR 推奨、PR #152 で再観測)** | todo8.md | XS | なし (`feedback_no_unenforced_rules.md` 例外 = 既存実践の明文化 + 機械強制ではなく guide 効果。順位 124 と同 PR で test location を正確に参照、順位 122 と同じロジック) | +| 128 | 💎 Tier 3 | **CLAUDE.md § Cross-File Reference Lifecycle に多ファイル同時削除 retirement condition checklist を追加 (PR #153 T3-#2 採用)** | todo8.md | XS | なし (PR #133 (todo.md 分割) + PR #153 (analysis.md 分割) の successful pattern を明文化、`feedback_no_unenforced_rules.md` 例外 = 既存実践の明文化 + guide 効果、順位 122 / 127 と同じロジック、`~/.claude/` global 配下で派生プロジェクトに自動波及) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 @@ -126,3 +124,5 @@ **Bundle k (PR #151 post-merge-feedback、Phase D dogfood 観測由来の lint-screen FP 対策、2026-05-13)**: PR #151 (Phase D D-5 = comment-lint test 拡充 + MAX cap test) merge 後の post-merge-feedback で 11 findings 中 **5 件採用** (Tier 1 #1, #2 / Tier 2 #1 / Tier 3 #1, #2) を 5 entries (順位 123-127) で登録。**コア発見**: D-3 (PR #148) / D-4 CR fix (PR #150) / D-5 ×2 (PR #151) の 3 PR・4 push events で「mistral:7b が docs-only diff や `.md` ファイルに対して Rust の `unused-import` を hallucinate する」FP pattern が一貫して観測 = Phase b' fixture では再現しない failure mode。**順位 123 (lint-screen MD 除外フィルター、Tier 1 / M / High freq)** が最重要 = 拡張子ベース mechanical filter で構造的に解消可能、Phase D dogfood 観測から導かれた最も価値ある決定論的防止策。**Sub-PR 推奨**: k-1 (順位 123 + 126、実装 + ADR-038 codify、Effort M+XS、コア層) / k-2 (順位 124 + 127、TOML test + extensions code comment、Effort S+XS、test gap 補強層) / k-3 (順位 125、UTF-8 boundary 横展開、Effort M、独立) で 3 PR 分割推奨。**却下** (4 件): UTF-8 lint rule (FP リスク、AST 必須) / `byte_offset_to_line` 強化 (PR #151 で既対応) / UTF-8 guideline + extensions checklist (`feedback_no_unenforced_rules.md` 適用)。**様子見** (3 件): T2 #2 (lint-screen dogfood CI step、L effort + takt test infra 調査依存) / T3 #3 (test 拡充→bug 発見 pattern を ADR-007 記録、1 PR 観測のみ) / T3 #4 (multi-rule scenario fixture pattern を test comment 明文化、Low × Low)。**本 PR 含意**: Phase D dogfood 観測 (analysis.md L334-340) が直接 actionable な決定論的防止層 (順位 123) に結実、Phase E 採否判定前に systemic FP root cause が解消される構造的進展。 **Bundle k 補強 (PR #152 post-merge-feedback、D-6 docs-only PR、2026-05-13)**: PR #152 (Phase D D-6 = fix.md instruction-level review-diff refresh + Bundle k 順位 123-127 entry 登録) merge 後の post-merge-feedback で 8 findings 中 4 件採用 (Tier 1 #1 / Tier 2 #1 / Tier 3 #1, #2)。**全 4 件が Bundle k 既存エントリ (順位 123/124/126/127) と完全重複** = post-merge-feedback analyzer 自身が「Bundle k 優先度 X で既に roadmap 済」と明記。新規順位を追加せず、**既存 4 entries (順位 123/124/126/127) に PR #152 を追加観測として追記** (frequency 観測: 3 PR → **4 PR** に更新、Bundle k の優先度 / Sub-PR 分割推奨は不変)。**含意**: PR #152 (docs-only) でも `.md` への `unused-import` FP が同根 root cause で再現したことが「lint_screen FP は diff 内容ではなく hook source 周辺 context を見て hallucinate している」仮説を 4th observation として裏付け = 順位 123 拡張子フィルター実装の confidence 向上。**様子見** (2 件): PostToolUse hook 自動化 (案 D、Frequency Low) / fix.md 自己参照 ambiguity (1 PR 観測のみ、次回 fix.md 編集機会に opportunistic 適用)。**却下** (2 件): 機械検知不可な `~/.claude/rules/*` 追加 (memory `feedback_no_unenforced_rules.md` 適用)。 + +**Bundle k 補強 (PR #153 post-merge-feedback、analysis.md 軽量化 PR、2026-05-13)**: PR #153 (D-6 post-merge follow-up + analysis.md 49KB→26KB split) merge 後の post-merge-feedback で 6 findings 中 **2 件採用** (Tier 3 #1, #2)。**T3 #1 = 順位 126 (ADR-038 hallucinate codify) と完全重複** → 順位 126 entry を「**5 PR 連続観測** (#148/#150/#151/#152/#153)」+ root cause / structural fix の明示記載要件追加で更新。**T3 #2 = 新規採用** → **順位 128 (CLAUDE.md § Cross-File Reference Lifecycle に多ファイル同時削除 retirement condition checklist 追加)** として登録、PR #133 (todo.md 分割) + PR #153 (analysis.md 分割) の successful pattern を明文化。**様子見** (2 件): CLAUDE.md → docs-governance.md cross-link / docs-only PR template の Retired sections list (どちらも Frequency Low)。**却下** (2 件): cross-reference lifecycle 自動 lint rule (NLP 必要) / file role scope exceptions guidance (1 観測のみ、過剰一般化リスク)。**含意**: docs-only PR でも mistral:7b の FP が 5 観測目として再現、Bundle k 順位 126 の優先度を High freq として確定。多ファイル分割の retirement workflow を順位 128 で global rule 化することで、今後の docs/* 50KB 分割 (history.md 等) で同 pattern を mechanical に reproducible 化。 diff --git a/docs/todo7.md b/docs/todo7.md index 12a0e41..fc12534 100644 --- a/docs/todo7.md +++ b/docs/todo7.md @@ -263,99 +263,6 @@ --- -### cli-merge-pipeline に Drop guard / signal handler を追加し abrupt 終了時に `.failed` marker を保証 (PR #109 T1-1 採用) ★ Bundle c - -> **動機**: PR #109 merge 直後の post-merge-feedback workflow が SIGPIPE で silent 中断され、`.takt/runs/.../reports/` が空 + `.claude/feedback-reports/109.md` 未生成 + `.failed` marker も無いという fail mode が実証された。原因は `feedback::run()` が `Result::Err` を返した場合のみ `write_failed_marker` を書く実装で、Rust default の SIGPIPE 動作 (parent process abrupt 終了) では Result::Err 経路に到達しない。ADR-030「失敗マーカーによる recovery」仕様を構造的に違反。 -> -> **本タスクの位置づけ**: Bundle c (PR #109 post-merge-feedback 堅牢化) の中核。Drop guard で `Result::Err` 経路に依存しない unconditional marker 書き出しを保証する。Pre-emptive marker (案 C) と signal trap (案 A) の組み合わせで abrupt 経路を多層防御。 -> -> **参照**: `.claude/feedback-reports/109.md` Tier 1 #1、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md)、`src/cli-merge-pipeline/src/feedback.rs:454-475` (`copy_feedback_report`) / `:1100-1180` (`run`) / `main.rs:555` (caller) -> -> **実行優先度**: 🚀 **Tier 1 Critical** — Effort M。仕様 (ADR-030) と実装の根本ギャップ閉鎖。 - -#### 設計決定 (案) - -- **修正方針**: Explore agent が提示した 3 案 (A: signal trap + Drop guard / B: thread + parent timeout / C: pre-emptive marker) のうち、**A + C の組み合わせ** を採用 (agent 推奨) - - **C (pre-emptive marker)**: `feedback::run` 呼び出し前に `.failed` marker を先制書き込み、正常完了時のみ削除。abrupt 終了の 99% を救済 (Effort XS-S) - - **A (signal trap + Drop guard)**: `tokio::signal` または `nix` crate で SIGPIPE/SIGTERM を trap、RAII Drop guard で marker 書き込みを保証。panic 経路もカバー (Effort M) -- **race 対策**: 同 PR で concurrent merge が走った場合の race は既存 `CONCURRENT_RUN_GUARD_SECS=1500s` で予防されるが、pre-emptive marker の lifecycle と整合性確認が必要 -- **OS 互換性**: signal handling は OS 依存。Windows では SIGPIPE 相当が無いため Ctrl+C / SIGTERM 経路を中心に対応。Unix と Windows のコードパス分岐は cfg gate で実装 - -#### 作業計画 - -- [ ] `src/cli-merge-pipeline/src/feedback.rs` に pre-emptive marker 書き出しを追加 (`run` 冒頭で `write_failed_marker(reason: "pending")`) -- [ ] 正常完了時に marker を削除する path を追加 (`copy_feedback_report` 成功後) -- [ ] `nix` または `tokio::signal` で SIGPIPE/SIGTERM trap を実装 (Unix) + Windows 用 cfg 分岐 -- [ ] RAII Drop guard 構造体を導入し、scope 終了時に marker 書き込みを保証 (正常時 `disarm()` で skip) -- [ ] 既存 `Result::Err` 経路の `write_failed_marker` 呼び出しは維持 (二重書きにならないよう pre-emptive marker と統合) -- [ ] dogfood: 本機能を有効にした状態で `cli-merge-pipeline.exe \| head -40` を実行し marker が残ることを確認 (今回事故の再現テスト) -- [ ] 派生プロジェクトに deploy (cli-merge-pipeline.exe を再ビルド + 配布) -- [ ] 本 todo7.md エントリを削除 - -#### 完了基準 - -- SIGPIPE / SIGTERM / panic / Result::Err いずれの経路でも `.claude/feedback-reports/.md.failed` が必ず残る -- 正常完了時には `.failed` marker が残らない (false positive ゼロ) -- 今回の事故 (PR #109 SIGPIPE) を再現するテストで pass -- 派生プロジェクト (`techbook-ledger` / `auto-review-fix-vc`) でも同等動作 - -#### 詰まっている箇所 - -- Windows での SIGPIPE 相当の挙動: Rust std はデフォルト SIGPIPE handler を install するが、Windows では pipe broken 時の挙動が異なる (CTRL_BREAK / I/O error)。整合性確保のため OS 別の signal mapping 設計が必要 -- 順位 64 (orphan reaper) との責務分離: Drop guard は process 内、reaper は process 外。両者の trigger 条件が重複しないよう設計 - ---- - -### orphan run reaper (post-merge-feedback の `meta.json status=running` 放置検出 + 自動再起動) (PR #109 T1-2 採用) ★ Bundle c - -> **動機**: 順位 63 (Drop guard) では救済できない致命系 (kill -9 / SIGKILL / power loss / OOM Killer) で post-merge-feedback workflow が中断された場合、`.failed` marker も書かれず orphan run のみが残る。仕様 (= フィードバックは必ず実行) を保証するには process 外からの監視層が必要。 -> -> **本タスクの位置づけ**: Bundle c (PR #109 post-merge-feedback 堅牢化) の第二防衛層。Drop guard (順位 63) を内側、reaper を外側とする多層防御で「フィードバックは必ず実行する」仕様を multi-layer で保証。 -> -> **参照**: `.claude/feedback-reports/109.md` Tier 1 #2、[ADR-029](adr/adr-029-post-merge-feedback-auto-trigger.md) (pending file 経由の再起動)、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md) -> -> **実行優先度**: 🚀 **Tier 1 Critical** — Effort M。順位 63 と組み合わせて致命系 hole を塞ぐ。 - -#### 設計決定 (案) - -- **配置先候補** (着手時に決定): - - **案 A**: `cli-pr-monitor` 起動時に `.takt/runs/*/meta.json` を scan (既存 monitor 機構との整合性高い) - - **案 B**: SessionStart hook (`src/hooks-session-start*/`) で scan (Claude Code session 起動毎に走る確定的 trigger) - - 推奨: **案 B** (SessionStart) — cli-pr-monitor は backend daemon 廃止 (ADR-018) で takt 経由になっており trigger 機構が複雑、SessionStart は単純で確実 -- **検出条件**: - - `.takt/runs/*/meta.json` の `status: "running"` かつ `startTime` が現時刻から **5 分以上経過** - - `currentStep` が `analyze` のまま (= 1 step も完了していない極短時間で死んだケース) も含める -- **recovery 動作**: - - 検出した orphan run の `meta.json` を `status: "failed"` に更新 (アトミックに) - - `.claude/feedback-reports/.md.failed` marker を書く (PR 番号は run slug `post-merge-feedback-for-` から抽出) - - ADR-029 pending file (`.claude/post-merge-feedback-pending.json`) を生成し、UserPromptSubmit hook で再起動 trigger -- **冪等性**: 同 orphan を 2 回検出しても重複 trigger しないよう既存 marker / pending file を check - -#### 作業計画 - -- [ ] 配置先 (案 A / B) を grep + `.claude/hooks-config.toml` 確認のうえ決定 -- [ ] `meta.json` parser + 5 分閾値判定ロジック実装 -- [ ] `.failed` marker 書き出し + pending file 生成ロジック実装 -- [ ] 冪等性 guard (既存 marker / pending file 検出時の skip) -- [ ] integration test: 人為的に orphan meta.json を作成して reaper が再起動 trigger することを assert -- [ ] dogfood: 既存の orphan (`.takt/runs/20260504-101353-post-merge-feedback-for-109/`) を fixture として retroactive detection 確認 -- [ ] 派生プロジェクトに deploy -- [ ] 本 todo7.md エントリを削除 - -#### 完了基準 - -- kill -9 / power loss シミュレート (forcibly kill) で `.failed` marker と pending file が遅延生成される -- Drop guard (順位 63) が機能している正常 case では reaper が誤検出しない (false positive ゼロ) -- 既存 orphan (PR #109 のもの) を retroactive に処理できる -- 仕様レベル: 「post-merge-feedback はマージ後 5 分以内に必ず完了 or 失敗 marker 化される」が AppCenter 級の SLA で保証される - -#### 詰まっている箇所 - -- SessionStart hook の発火頻度: 1 session 1 回しか走らないと、長時間 session 中に orphan が発生しても拾えない。`cli-pr-monitor` 経路と組み合わせるか、SessionStart + UserPromptSubmit の二段階検出が必要か検討 -- 5 分閾値の妥当性: takt の analyze step は最大 5-10 分かかる場合あり。閾値を 5 分にすると進行中の正常 run を誤検出するリスク。10-15 分が妥当か - ---- - ### exe + `--help` を PreToolUse でブロックして `src//` Read に誘導する hook (PR #109 T1-3 採用) ★ Bundle c > **動機**: PR #109 SIGPIPE 事故の **直接トリガ** が「AI が `cli-merge-pipeline.exe --help` を実行 → 当該 exe は `--help` 未対応のため merge 本体を実行 → 出力 truncate で SIGPIPE」だった。ユーザー提案: exe ごとに `--help` を実装する案は exe 数増加で漏れが出るが、`exe + --help` をセットで PreToolUse block すればソース閲覧フローに自動誘導でき、想定外実行を構造的に排除。今後追加される exe にも自動適用される一般解。 @@ -452,44 +359,3 @@ --- -### ADR-030 に abrupt 終了時の振る舞いを spec として明記 (PR #109 T3-2 採用) ★ Bundle c - -> **動機**: PR #109 で露呈した「ADR-030 の決定論性が SIGPIPE / kill -9 / power loss で破綻する」問題は、ADR 本文で abrupt 終了時の挙動が **spec として明記されていなかった** ことが根本原因。順位 63 / 64 の実装が ADR-030 の "決定論的" の真の意味を closure する形で land する以上、ADR 本文も同タイミングで spec を拡張する必要がある。 -> -> **本タスクの位置づけ**: Bundle c (PR #109 post-merge-feedback 堅牢化) の仕様層。順位 63 / 64 の実装と同 PR で land して仕様/実装の整合性を保つ (実装単独で spec ドリフトしない)。 -> -> **参照**: `.claude/feedback-reports/109.md` Tier 3 #6、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md) (試験運用) -> -> **実行優先度**: 💎 **Tier 3** — Effort XS。ADR 本文の "失敗マーカーによる recovery" 節を拡張。 - -#### 設計決定 (案) - -- **拡張する節**: ADR-030 の "失敗マーカーによる recovery" を「abrupt 終了 + reaper による多層保証」に拡張 -- **追記内容** (案): - - **L1 (in-process)**: Drop guard / signal trap で `Result::Err` 経路に依存せず `.failed` marker を保証 (順位 63 で実装) - - **L2 (out-of-process)**: orphan run reaper で `meta.json status=running` 5-15 分放置を検出し marker 補完 + 再起動 (順位 64 で実装) - - **致命系の挙動明記**: kill -9 / SIGKILL / power loss / OOM Killer → L1 で救済不可、L2 で救済 - - **仕様の SLA 化**: 「post-merge-feedback はマージ後 N 分以内に必ず完了 or .failed marker 化される」を保証ステートメントとして記述 -- **試験運用フラグの扱い**: 順位 63 / 64 land 後、本 ADR の "試験運用" フラグを外すか継続するかは dogfood 結果次第。本 task では仕様明記のみ、フラグ判断は別途 -- **関連 ADR との cross-link**: ADR-029 (pending file 自動起動) との関係明記、L2 reaper が ADR-029 経路を再利用する旨 - -#### 作業計画 - -- [ ] `docs/adr/adr-030-deterministic-post-merge-feedback.md` を読み、現行の "失敗マーカーによる recovery" 節を確認 -- [ ] 拡張内容を起草 (L1/L2 の責務分離 + SLA 化 + cross-link) -- [ ] 順位 63 / 64 と同 PR で land する前提で実装と整合 -- [ ] CLAUDE.md ADR index の ADR-030 description (試験運用フラグ等) も必要なら更新 -- [ ] 本 todo7.md エントリを削除 - -#### 完了基準 - -- ADR-030 本文に L1 (in-process Drop guard) + L2 (out-of-process reaper) の責務分離が記述される -- abrupt 終了 (SIGPIPE / kill -9 / power loss / OOM) 時の挙動が spec として明記される -- post-merge-feedback の SLA (= マージ後 N 分以内に完了 or marker 化) がステートメントとして残る - -#### 詰まっている箇所 - -- 試験運用フラグの去就: 順位 63 / 64 で実装が完成しても、dogfood 期間が必要なら試験運用フラグは残す。本 task では仕様明記のみだが、フラグ判断と整合性を取る必要あり -- SLA の妥当性: 順位 64 の閾値 (5-15 分) と同期する必要があり、閾値が決まらないと SLA も書けない (依存関係) - ---- diff --git a/docs/todo8.md b/docs/todo8.md index e9bef99..201db48 100644 --- a/docs/todo8.md +++ b/docs/todo8.md @@ -236,25 +236,25 @@ --- -### ADR-038 に mistral:7b 「diff 外 context hallucinate」failure mode を追記 (PR #151 T3-#1 採用、順位 123 と同 PR 推奨、**PR #152 で再観測**) +### ADR-038 に mistral:7b 「diff 外 context hallucinate」failure mode を追記 (PR #151 T3-#1 採用、順位 123 と同 PR 推奨、**PR #152 / PR #153 で再観測 = 5 PR 連続**) -> **動機**: PR #148 (D-3) / PR #150 (D-4 CR fix) / PR #151 (D-5 ×2) / **PR #152 (D-6 docs-only)** の 4 PR で観測された FP pattern = 「mistral:7b が diff 内容に関わらず hook source 周辺の context を見て `unused-import` を hallucinate する」を ADR-038 に codify。Phase b' fixture では再現しない failure mode のため、将来の prompt 改善や別モデル評価時の prior assumption として永続記録する価値あり。 +> **動機**: PR #148 (D-3) / PR #150 (D-4 CR fix) / PR #151 (D-5 ×2) / **PR #152 (D-6 docs-only)** / **PR #153 (analysis.md split)** の **5 PR 連続** で観測された FP pattern = 「mistral:7b が diff 内容に関わらず hook source 周辺の context を見て `unused-import` を hallucinate する」を ADR-038 に codify。Phase b' fixture では再現しない failure mode のため、将来の prompt 改善や別モデル評価時の prior assumption として永続記録する価値あり。 > -> **本タスクの位置づけ**: PR #151 post-merge-feedback Tier 3 #1 採用 → PR #152 post-merge-feedback で 4 PR 観測に拡大 (Severity Low / Frequency High / Effort XS / Adoption Risk None)。順位 123 (lint-screen MD フィルタ実装) と同 PR で land 効率的 (実装と仕様の整合性確保)。 +> **本タスクの位置づけ**: PR #151 post-merge-feedback Tier 3 #1 採用 → PR #152 post-merge-feedback で 4 PR 観測に拡大 → **PR #153 post-merge-feedback で Frequency High 閾値到達** (Severity Low / Frequency High / Effort XS / Adoption Risk None)。順位 123 (lint-screen MD フィルタ実装) と同 PR で land 効率的 (実装と仕様の整合性確保)。 > -> **参照**: `.claude/feedback-reports/151.md` Tier 3 #1、`docs/adr/adr-038-local-llm-finding-classification.md`、D-3/D-4/D-5 outcome (`docs/local-llm-offload-analysis.md`) +> **参照**: `.claude/feedback-reports/151.md` Tier 3 #1 / `.claude/feedback-reports/152.md` T3 #1 / `.claude/feedback-reports/153.md` T3 #1、`docs/adr/adr-038-local-llm-finding-classification.md`、D-3/D-4/D-5/D-6 outcome (`docs/local-llm-offload-phase-d-outcomes.md`) #### 作業計画 - [ ] ADR-038 に「Known failure mode: docs-only diff Rust context hallucinate」section 追加 -- [ ] 3 PR 観測の事実 (#148/#150/#151) を inline cite -- [ ] 根本原因の推定 (context window 内に hook source が含まれる → past commit の `use` 文を current diff として誤認) を記録 -- [ ] 対策として順位 123 (拡張子フィルタ) を citation +- [ ] 5 PR 観測の事実 (#148/#150/#151/#152/#153) を inline cite +- [ ] **Root cause を明記**: LLM context window に hook source コードが混入 → 過去 commit の `use` 文 (test fn 内 等) を current diff として hallucinate → `unused-import` FP を生成 (PR #153 post-merge-feedback T3 #1 で specifically 要求された記述) +- [ ] **Structural fix の cross-link を明記**: 対策 = Bundle k 順位 123 (拡張子フィルタで `.md` ハンクを diff 段階から除外) を ADR 本文から explicit 引用 (root cause と fix の両方を一箇所で逆引き可能にする、PR #153 post-merge-feedback T3 #1 で specifically 要求された記述) - [ ] 本エントリ削除 + todo-summary.md 行削除 #### 完了基準 -- ADR-038 から「なぜ Markdown 除外フィルタが必要か」が逆引きできる +- ADR-038 から「なぜ Markdown 除外フィルタが必要か」が逆引きできる (root cause + fix path が同一 section で記述) - 将来別モデル評価 (LLaMa / phi 等) で同 failure mode を検証する出発点になる --- @@ -280,3 +280,29 @@ - comment が機械強制ではなく guide として機能する (PR review 時の checklist としても再利用可) --- + +### CLAUDE.md § Cross-File Reference Lifecycle に多ファイル同時削除 retirement condition checklist を追加 (PR #153 T3-#2 採用) + +> **動機**: PR #153 で `docs/local-llm-offload-analysis.md` を `phase-d-outcomes.md` に分割した際、retirement clause を **3 ファイル (analysis.md / history.md / phase-d-outcomes.md) 同時削除** に統一する作業が developer/AI の手動 review でしか担保されていなかった。advisor 指摘で明示的に「3 ファイルすべてに同じ retirement clause を書く」ステップを踏んだが、これは structural pattern として再利用可能 (今後の docs/* 50KB 分割でも同じ checklist が必要)。同パターンが drift すると ephemeral artifact の lifecycle 整合が崩れ、stale pointer が増殖するリスクあり。 +> +> **本タスクの位置づけ**: PR #153 post-merge-feedback Tier 3 #2 採用 (Severity Low / Frequency Medium / Effort XS / Adoption Risk None)。**既存実践 (PR #133 todo.md 分割 + PR #153 analysis.md 分割) の明文化 + 機械強制ではなく guide 効果** のため、`feedback_no_unenforced_rules.md` の例外条件 (順位 122 / 127 と同じロジック) を満たす。 +> +> **参照**: `.claude/feedback-reports/153.md` Tier 3 #2、`~/.claude/rules/common/coding-style.md` § Cross-File Reference Lifecycle、`~/.claude/rules/common/docs-governance.md` § Retirement Workflow + +#### 作業計画 + +- [ ] `~/.claude/rules/common/coding-style.md` § Cross-File Reference Lifecycle に「多ファイル同時削除時の retirement condition consistency checklist」section を追加 (3-5 項目程度の bullet list) + - 「N ファイルを同時削除する設計の場合、全 N ファイルの header に同一の retirement clause が記載されているか」 + - 「retirement workflow の Step 3 (参照更新) で `grep -rn ''` を全ファイル分実施したか」 + - 「新ファイル追加時に既存ファイルの retirement clause にも追記したか」 + - 「参照先 (ADR / docs-governance.md) が permanent artifact であることを確認」 +- [ ] 派生プロジェクト (techbook-ledger / auto-review-fix-vc) は `~/.claude/` global 配下なので自動波及 +- [ ] グローバル設定変更前に `~/.claude/` snapshot 取得 (memory rule `feedback_global_config_backup.md` 適用) +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 次回多ファイル分割 (例: history.md 50KB 接近時) で同 checklist を踏むことで drift が構造的に防止される +- PR #133 (todo.md 分割) / PR #153 (analysis.md 分割) の successful pattern が明文化され、3 例目以降の reproducibility が確保される + +--- diff --git a/src/cli-merge-pipeline/src/feedback.rs b/src/cli-merge-pipeline/src/feedback.rs index 09442c5..83da844 100644 --- a/src/cli-merge-pipeline/src/feedback.rs +++ b/src/cli-merge-pipeline/src/feedback.rs @@ -31,8 +31,13 @@ use std::time::Duration; /// として含む `" []"` 形式とする。これにより takt の sanitization 後の /// dir 名 (`-`) が必ず workflow 名を含み、`find_latest_run_dir` /// が `name.contains("-")` でマッチできる。 -const TAKT_WORKFLOW: &str = "post-merge-feedback"; -const TAKT_TASK_PREFIX: &str = "post-merge-feedback for #"; +pub const TAKT_WORKFLOW: &str = "post-merge-feedback"; + +/// post-merge-feedback の task label prefix。`hooks-session-start` の orphan reaper +/// (ADR-030 §L2 out-of-process) も meta.json `task` field を本値で discriminate する。 +/// 値を変更する場合は両 crate を同 PR で更新する (Drift 検出用 test は +/// `hooks-session-start` 側で literal を assertion している)。 +pub const TAKT_TASK_PREFIX: &str = "post-merge-feedback for #"; /// takt 実行のデフォルトタイムアウト (20 分) /// @@ -41,6 +46,19 @@ const TAKT_TASK_PREFIX: &str = "post-merge-feedback for #"; /// スケールするため、長期 PR では再評価が必要 (ADR-030 §レイテンシ 参照)。 pub const TAKT_TIMEOUT_SECS: u64 = 1200; +/// orphan run reaper (ADR-030 §L2) の閾値秒数。`TAKT_TIMEOUT_SECS` + 余裕 5 分。 +/// +/// 正常 run は `TAKT_TIMEOUT_SECS` (1200s) 以内に completed / failed のいずれかに +/// 遷移するため、本値 (1500s) を超えても `status: "running"` のまま放置されている +/// run は abrupt 終了 (kill -9 / SIGKILL / power loss / OOM Killer) で in-process Drop +/// guard を経由せず死んだとみなす。`TAKT_TIMEOUT_SECS` 変更時に本値も自動追随する。 +/// +/// 本 const は canonical 参照値として保持し、out-of-process reaper 実装の +/// `hooks-session-start::ORPHAN_THRESHOLD_SECS` は同 literal `1500` を pin する +/// (両 crate の test で drift 検出)。 +#[allow(dead_code)] +pub const ORPHAN_THRESHOLD_SECS: u64 = TAKT_TIMEOUT_SECS + 300; + /// run_takt_workflow のポーリング間隔 (ms) const POLL_INTERVAL_MS: u64 = 500; @@ -518,12 +536,85 @@ pub fn write_failed_marker( /// 成功時に `.failed` marker が残っていたら削除する。 fn cleanup_failed_marker(repo_root: &Path, pr_number: u64) { - let path = repo_root - .join(FEEDBACK_DIR) - .join(format!("{}.md.failed", pr_number)); + let path = failed_marker_path(repo_root, pr_number); let _ = fs::remove_file(path); } +/// `.claude/feedback-reports/.md.failed` の絶対パスを返す (純粋関数)。 +pub fn failed_marker_path(repo_root: &Path, pr_number: u64) -> PathBuf { + repo_root + .join(FEEDBACK_DIR) + .join(format!("{}.md.failed", pr_number)) +} + +/// pre-emptive `.failed` marker (Drop guard 用)。 +/// +/// ADR-030 §L1 in-process recovery の中核。`feedback::run` の早期段階で marker を +/// 書き出し、正常完了時のみ `cleanup_failed_marker` で削除する。SIGPIPE / kill -9 等で +/// process が abrupt 終了しても marker がディスクに残るため L2 recovery (UserPromptSubmit +/// hook) が拾える。 +/// +/// 詳細 reason (例: takt timeout、report 不在) は caller (main.rs) が Err 経路で +/// `write_failed_marker` を再呼び出しして上書きするため、本関数は最小限の +/// "pending" 状態のみ書き出す。 +fn write_pending_marker(repo_root: &Path, pr_number: u64) -> Result { + write_failed_marker( + repo_root, + pr_number, + "pending: takt workflow が完了する前に process が終了した可能性あり \ + (pre-emptive marker, ADR-030 §L1)", + ) +} + +/// RAII guard: scope 終了時に `.failed` marker が残っていることを保証する。 +/// +/// ADR-030 §L1 in-process Drop guard。`feedback::run` 内で armed 状態の guard を +/// 作成し、正常完了時のみ `disarm()` で抑止する。abnormal 経路 (panic / 早期 return) +/// では Drop が marker 存在を check し、欠落していれば backup として書き直す +/// (idempotent: 既存 marker (例: caller の detailed marker) は overwrite しない)。 +/// +/// **Rust default SIGPIPE の制約**: `SIG_DFL` で process が abrupt 終了するため +/// Drop は呼ばれない。SIGPIPE 経路は `write_pending_marker` の **pre-emptive 書込み** +/// で marker をディスクに先置きすることで救済する。本 guard は panic / 早期 return +/// のような Drop が走る経路の backup として機能する。 +struct FailedMarkerGuard<'a> { + repo_root: &'a Path, + pr_number: u64, + armed: bool, +} + +impl<'a> FailedMarkerGuard<'a> { + fn new(repo_root: &'a Path, pr_number: u64) -> Self { + Self { + repo_root, + pr_number, + armed: true, + } + } + + /// 正常完了時に guard を解除する。Drop は no-op になる。 + fn disarm(&mut self) { + self.armed = false; + } +} + +impl Drop for FailedMarkerGuard<'_> { + fn drop(&mut self) { + if !self.armed { + return; + } + if failed_marker_path(self.repo_root, self.pr_number).exists() { + return; + } + let _ = write_failed_marker( + self.repo_root, + self.pr_number, + "pending: workflow が unexpected に終了した \ + (FailedMarkerGuard Drop, ADR-030 §L1)", + ); + } +} + /// 並行起動 guard の TTL (秒)。`TAKT_TIMEOUT_SECS` (1200s) より少し長い値。 /// /// 直前の cli-merge-pipeline 起動で `context.json` が書かれてから本値の経過時間内に @@ -901,6 +992,115 @@ mod tests { let _ = fs::remove_dir_all(&root); } + fn unique_temp_root(prefix: &str) -> PathBuf { + std::env::temp_dir().join(format!( + "{}-{}-{}", + prefix, + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )) + } + + #[test] + fn failed_marker_path_uses_feedback_dir_layout() { + let path = failed_marker_path(Path::new("/repo"), 42); + let suffix = format!("{}{}42.md.failed", FEEDBACK_DIR, std::path::MAIN_SEPARATOR); + assert!(path.to_string_lossy().ends_with(&suffix)); + } + + #[test] + fn orphan_threshold_exceeds_takt_timeout() { + assert!( + ORPHAN_THRESHOLD_SECS > TAKT_TIMEOUT_SECS, + "orphan threshold ({}s) must exceed TAKT_TIMEOUT_SECS ({}s) to avoid \ + false-positive reaping of legitimately-running takt workflows", + ORPHAN_THRESHOLD_SECS, + TAKT_TIMEOUT_SECS, + ); + assert_eq!( + ORPHAN_THRESHOLD_SECS, + TAKT_TIMEOUT_SECS + 300, + "ORPHAN_THRESHOLD_SECS must track TAKT_TIMEOUT_SECS + 300s margin \ + (ADR-030 §L2 reaper threshold)" + ); + } + + #[test] + fn write_pending_marker_creates_marker_with_pending_reason() { + let root = unique_temp_root("feedback-pending"); + fs::create_dir_all(&root).unwrap(); + let path = write_pending_marker(&root, 9).unwrap(); + assert!(path.exists()); + let body = fs::read_to_string(&path).unwrap(); + assert!(body.contains("PR #9")); + assert!(body.contains("pending")); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn marker_guard_disarmed_does_not_create_marker() { + let root = unique_temp_root("feedback-guard-disarmed"); + fs::create_dir_all(&root).unwrap(); + { + let mut guard = FailedMarkerGuard::new(&root, 11); + guard.disarm(); + } + assert!(!failed_marker_path(&root, 11).exists()); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn marker_guard_armed_writes_backup_when_missing() { + let root = unique_temp_root("feedback-guard-armed"); + fs::create_dir_all(&root).unwrap(); + { + let _guard = FailedMarkerGuard::new(&root, 12); + } + let path = failed_marker_path(&root, 12); + assert!(path.exists()); + let body = fs::read_to_string(&path).unwrap(); + assert!(body.contains("FailedMarkerGuard Drop")); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn marker_guard_armed_preserves_existing_detailed_marker() { + let root = unique_temp_root("feedback-guard-preserve"); + fs::create_dir_all(&root).unwrap(); + let existing = write_failed_marker(&root, 13, "takt timeout 1200s").unwrap(); + let original_body = fs::read_to_string(&existing).unwrap(); + { + let _guard = FailedMarkerGuard::new(&root, 13); + } + let after_body = fs::read_to_string(&existing).unwrap(); + assert_eq!( + original_body, after_body, + "Drop guard must not overwrite an existing detailed marker (idempotent backup)" + ); + assert!(after_body.contains("takt timeout 1200s")); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn marker_guard_disarmed_preserves_existing_marker() { + let root = unique_temp_root("feedback-guard-disarm-preserve"); + fs::create_dir_all(&root).unwrap(); + write_failed_marker(&root, 14, "leftover").unwrap(); + let path = failed_marker_path(&root, 14); + { + let mut guard = FailedMarkerGuard::new(&root, 14); + guard.disarm(); + } + assert!( + path.exists(), + "disarm must not delete an existing marker (only the Ok-path cleanup_failed_marker does)" + ); + let _ = fs::remove_dir_all(&root); + } + #[test] fn find_latest_prepush_picks_lexicographic_max() { let root = std::env::temp_dir().join(format!( @@ -1107,26 +1307,22 @@ mod tests { /// 観測された (PR #78 で kill 後 2 分 13 秒で feedback-report.md 完成)。 /// takt が exit=non-zero でも report が出ていれば成功扱いとする。 pub fn run(input: &FeedbackInput) -> Result { - let range = fetch_pr_time_range(input.pr_number, input.owner_repo) - .map_err(|e| format!("PR 時刻 range 取得失敗: {}", e))?; - let context_path = input.repo_root.join(CONTEXT_PATH); let transcript_path = input.repo_root.join(TRANSCRIPT_PATH); check_concurrent_run_guard(&context_path)?; - let written = match input.transcript_source_dir.as_ref() { - Some(dir) => filter_transcripts(dir, &range, &transcript_path) - .map_err(|e| format!("transcript filter 失敗: {}", e))?, - None => { - // ソース dir 不明: 空 jsonl を出力 (facet が「データなし」分岐に進む) - if let Some(parent) = transcript_path.parent() { - let _ = fs::create_dir_all(parent); - } - let _ = fs::write(&transcript_path, ""); - 0 - } - }; + let _ = write_pending_marker(&input.repo_root, input.pr_number); + let mut marker_guard = FailedMarkerGuard::new(&input.repo_root, input.pr_number); + + let range = fetch_pr_time_range(input.pr_number, input.owner_repo) + .map_err(|e| format!("PR 時刻 range 取得失敗: {}", e))?; + + let written = prepare_transcript( + input.transcript_source_dir.as_deref(), + &range, + &transcript_path, + )?; eprintln!( "[merge-pipeline] [feedback] transcript filter 完了 ({} entries → {})", written, @@ -1154,7 +1350,45 @@ pub fn run(input: &FeedbackInput) -> Result { ); } - match copy_feedback_report(&input.repo_root, input.pr_number) { + let result = reconcile_takt_output(&input.repo_root, input.pr_number, takt_ok); + if result.is_ok() { + marker_guard.disarm(); + } + result +} + +/// transcript jsonl を filter (source dir 既知時) または空ファイル書込 (source dir 不在時) する。 +/// +/// 戻り値は書き込んだ行数 (空ファイル時は 0)。source dir 不在のケースは facet 側の +/// 「データなし」分岐に流すため、エラーではなく 0 行で成功扱いとする。 +fn prepare_transcript( + transcript_source_dir: Option<&Path>, + range: &PrTimeRange, + transcript_path: &Path, +) -> Result { + match transcript_source_dir { + Some(dir) => filter_transcripts(dir, range, transcript_path) + .map_err(|e| format!("transcript filter 失敗: {}", e)), + None => { + if let Some(parent) = transcript_path.parent() { + let _ = fs::create_dir_all(parent); + } + let _ = fs::write(transcript_path, ""); + Ok(0) + } + } +} + +/// takt 完了後の report コピーと reconciliation。 +/// +/// `takt_ok = false` でも orphan takt が report を完成させた可能性があるため必ず copy を +/// 試す。成功時に `.failed` marker を cleanup、失敗時は cause prefix 付きで Err を返す。 +fn reconcile_takt_output( + repo_root: &Path, + pr_number: u64, + takt_ok: bool, +) -> Result { + match copy_feedback_report(repo_root, pr_number) { Ok(report) => { if !takt_ok { eprintln!( @@ -1162,7 +1396,7 @@ pub fn run(input: &FeedbackInput) -> Result { timeout/失敗扱いだったが orphan が report を完成させていた" ); } - cleanup_failed_marker(&input.repo_root, input.pr_number); + cleanup_failed_marker(repo_root, pr_number); Ok(report) } Err(copy_err) => { diff --git a/src/hooks-session-start/src/main.rs b/src/hooks-session-start/src/main.rs index 605660d..9439093 100644 --- a/src/hooks-session-start/src/main.rs +++ b/src/hooks-session-start/src/main.rs @@ -1,6 +1,6 @@ //! SessionStart hook — セッション ID を環境変数とファイルに伝播する + PR monitor catch-up //! -//! Claude Code の SessionStart イベントで発火し、以下の3つの経路で session 起動準備を行う: +//! Claude Code の SessionStart イベントで発火し、以下の経路で session 起動準備を行う: //! //! 1. $CLAUDE_ENV_FILE に export 文を追記 → Bash ツールから参照可能 //! 2. .claude/.session-id ファイルに書き出し → 子プロセス (exe) から参照可能 @@ -9,6 +9,13 @@ //! `additionalContext` で Claude に手動再起動を促すメッセージを差し込む。 //! 別プロセス spawn ではなく Claude に nudge する設計 (handle 継承や stdout //! 可視性の問題を回避し、PARK signal flow を session 内に保つ)。 +//! 4. Orphan run reaper (Bundle c-1 順位 64、ADR-030 §L2 out-of-process): +//! `.takt/runs//meta.json` を scan し、`status: "running"` のまま +//! `ORPHAN_THRESHOLD_SECS` を超えた post-merge-feedback run を「abrupt +//! termination で死んだ」とみなして `.failed` marker を生成 + meta.json +//! `status` を `failed` に更新する。kill -9 / SIGKILL / power loss / +//! OOM Killer など in-process Drop guard (§L1) で救済できない致命系で +//! `.failed` marker が書かれなかった orphan run を L2 で拾う。 //! //! .session-id ファイルは「同一 ID スキップ」方式: //! - 既に同じ session_id が書かれていれば何もしない (冪等) @@ -30,6 +37,255 @@ struct HookInput { /// cli-push-runner にあり、本 const は表示用 hint のみ。 const RESUME_MONITORING_COMMAND: &str = "pnpm push --monitor-only"; +/// post-merge-feedback task label prefix (ADR-030 §task labeling convention)。 +/// +/// 値は `cli-merge-pipeline::feedback::TAKT_TASK_PREFIX` と同一でなければならない。 +/// crate 間直接依存を避けるため inline duplicate しているが、両 crate の unit test +/// で literal `"post-merge-feedback for #"` を pin する drift 検出を行う +/// (`task_prefix_matches_canonical_literal` 系 test)。 +const TAKT_TASK_PREFIX_PMF: &str = "post-merge-feedback for #"; + +/// orphan reaper の閾値秒数 (ADR-030 §L2 out-of-process)。 +/// +/// `cli-merge-pipeline::feedback::TAKT_TIMEOUT_SECS` (1200s) + 余裕 5 分。正常 run は +/// 1200s 以内に completed / failed のいずれかに遷移するため、本値を超えても +/// `status: "running"` のまま放置される run は abrupt termination で in-process Drop +/// guard を経由せず死んだとみなす。両 crate の test で `1500` を pin する。 +const ORPHAN_THRESHOLD_SECS: u64 = 1500; + +/// `.claude/feedback-reports/` の相対パス (repo root から)。 +const FEEDBACK_DIR_REPO_RELATIVE: &str = ".claude/feedback-reports"; + +/// `.takt/runs/` の相対パス (repo root から)。 +const TAKT_RUNS_DIR: &str = ".takt/runs"; + +/// takt meta.json の必要 field のみ部分デシリアライズ。 +#[derive(Deserialize)] +struct TaktMeta { + task: Option, + status: Option, + #[serde(rename = "startTime")] + start_time: Option, +} + +/// 検出された orphan run の情報。`reap_orphans` が `.failed` marker を書く際に使う。 +struct OrphanRun { + meta_path: PathBuf, + pr_number: u64, + age_secs: u64, +} + +/// `2026-05-13T12:33:23.908Z` 形式の ISO 8601 文字列を Unix 秒に変換する。 +/// +/// 失敗 (invalid date / non-ASCII / 月日範囲外) 時は `None`。fractional 秒は +/// truncate (整数秒精度で十分)。実装は `check-ci-coderabbit::parse_iso8601_to_unix` +/// と同型 (no chrono dep policy)。 +fn parse_iso8601_to_unix(s: &str) -> Option { + let no_frac = s.split('.').next()?.trim_end_matches('Z'); + let mut parts = no_frac.split('T'); + let date = parts.next()?; + let time = parts.next()?; + let mut date_parts = date.split('-'); + let year: i64 = date_parts.next()?.parse().ok()?; + let month: i64 = date_parts.next()?.parse().ok()?; + let day: i64 = date_parts.next()?.parse().ok()?; + let mut time_parts = time.split(':'); + let hour: i64 = time_parts.next()?.parse().ok()?; + let minute: i64 = time_parts.next()?.parse().ok()?; + let second: i64 = time_parts.next()?.parse().ok()?; + if !(1970..=9999).contains(&year) + || !(1..=12).contains(&month) + || !(1..=days_in_month(year, month)).contains(&day) + || !(0..=23).contains(&hour) + || !(0..=59).contains(&minute) + || !(0..=59).contains(&second) + { + return None; + } + let mut days: i64 = 0; + for y in 1970..year { + days += if is_leap_year(y) { 366 } else { 365 }; + } + let month_days = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + for m in 1..month { + let idx = (m - 1) as usize; + days += month_days[idx]; + if m == 2 && is_leap_year(year) { + days += 1; + } + } + days += day - 1; + Some(days * 86400 + hour * 3600 + minute * 60 + second) +} + +fn is_leap_year(year: i64) -> bool { + (year % 4 == 0 && year % 100 != 0) || year % 400 == 0 +} + +fn days_in_month(year: i64, month: i64) -> i64 { + let month_days = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + let base = month_days[(month - 1) as usize]; + if month == 2 && is_leap_year(year) { + base + 1 + } else { + base + } +} + +fn read_takt_meta(path: &Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; + serde_json::from_str(&content).ok() +} + +/// task label `"post-merge-feedback for #N"` から PR 番号 N を抽出する。 +fn extract_pr_number_from_task(task: &str) -> Option { + task.strip_prefix(TAKT_TASK_PREFIX_PMF)?.trim().parse().ok() +} + +/// meta.json から orphan 判定に必要な要素 (pr_number, start_unix) を抽出する。 +/// +/// status / task / startTime のいずれかが orphan 条件を満たさなければ `None`。 +fn meta_to_orphan_inputs(meta: &TaktMeta) -> Option<(u64, i64)> { + if meta.status.as_deref() != Some("running") { + return None; + } + let pr = extract_pr_number_from_task(meta.task.as_deref()?)?; + let start = parse_iso8601_to_unix(meta.start_time.as_deref()?)?; + Some((pr, start)) +} + +/// `.takt/runs//meta.json` を scan して orphan な post-merge-feedback run を返す。 +/// +/// 条件: `status: "running"` AND task が `TAKT_TASK_PREFIX_PMF` で始まる AND +/// `now_unix - startTime >= ORPHAN_THRESHOLD_SECS`。malformed meta.json / non-PMF task / +/// PR 番号 parse 失敗 / startTime parse 失敗は defensive に skip。 +fn find_orphan_post_merge_feedback_runs(runs_dir: &Path, now_unix: i64) -> Vec { + let entries = match std::fs::read_dir(runs_dir) { + Ok(e) => e, + Err(_) => return Vec::new(), + }; + let mut orphans = Vec::new(); + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + let meta_path = path.join("meta.json"); + let Some(meta) = read_takt_meta(&meta_path) else { + continue; + }; + let Some((pr_number, start_unix)) = meta_to_orphan_inputs(&meta) else { + continue; + }; + let age = now_unix.saturating_sub(start_unix); + if age < ORPHAN_THRESHOLD_SECS as i64 { + continue; + } + orphans.push(OrphanRun { + meta_path, + pr_number, + age_secs: age as u64, + }); + } + orphans +} + +/// orphan の meta.json を `status: "failed"` に書き換える。reaper の責任明示のため +/// `reaped_by: "hooks-session-start"` も追加する。malformed JSON は skip (Err 返す)。 +fn mark_meta_failed(meta_path: &Path) -> std::io::Result<()> { + let content = std::fs::read_to_string(meta_path)?; + let mut value: serde_json::Value = serde_json::from_str(&content) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + if let Some(obj) = value.as_object_mut() { + obj.insert( + "status".to_string(), + serde_json::Value::String("failed".to_string()), + ); + obj.insert( + "reaped_by".to_string(), + serde_json::Value::String("hooks-session-start".to_string()), + ); + } + let serialized = serde_json::to_string_pretty(&value).map_err(std::io::Error::other)?; + std::fs::write(meta_path, serialized) +} + +/// `.failed` marker の本文を組み立てる。L2 recovery が拾う際の根拠 + 復旧手順を含む。 +fn build_reaper_failed_marker_body(orphan: &OrphanRun) -> String { + format!( + "# post-merge-feedback failed (PR #{pr})\n\n\ + takt workflow が abrupt 終了 (kill -9 / SIGKILL / power loss / OOM 等) で中断され、\n\ + in-process Drop guard 経路を経由せずに死んだとみなされました\n\ + (orphan reaper, ADR-030 §L2 out-of-process)。\n\n\ + ## 検出情報\n\n\ + - meta.json: `{meta}`\n\ + - 経過時間: {age} 秒 (閾値: {threshold} 秒 = TAKT_TIMEOUT_SECS + 余裕 5 分)\n\n\ + ## 復旧手順\n\n\ + 1. このマーカーを残したまま、Claude Code セッションで何か入力する\n\ + 2. UserPromptSubmit hook (`hooks-user-prompt-feedback-recovery`) が検出し、\n \ + Claude に再実行を促す\n\ + 3. 手動で再実行する場合: `pnpm exec takt -w post-merge-feedback -t \"post-merge-feedback for #{pr}\"`\n", + pr = orphan.pr_number, + meta = orphan.meta_path.display(), + age = orphan.age_secs, + threshold = ORPHAN_THRESHOLD_SECS, + ) +} + +/// 検出された orphan run に対し `.failed` marker と meta.json `status=failed` を書く。 +/// +/// 冪等性: +/// - 既存 `.failed` marker がある → skip (L1 / 前回 reaper pass による処理済み) +/// - 既存 `.md` 成功レポートがある → skip (ADR-030 §Reconciliation で documented されている +/// 「takt parent kill 後に descendants が report 完成」path。meta.json は `status: "running"` +/// のままだが実際は成功しているため、ここで `.failed` marker を書くと false-positive nag になる) +/// +/// marker 書込失敗時は当該 orphan を skip して次に進む (best-effort)。 +/// 戻り値: 新規 reap した (PR 番号, age_secs) リスト。 +fn reap_orphans(repo_root: &Path, orphans: &[OrphanRun]) -> Vec<(u64, u64)> { + let mut reaped = Vec::new(); + for orphan in orphans { + let feedback_dir = repo_root.join(FEEDBACK_DIR_REPO_RELATIVE); + let marker = feedback_dir.join(format!("{}.md.failed", orphan.pr_number)); + let success_report = feedback_dir.join(format!("{}.md", orphan.pr_number)); + if marker.exists() || success_report.exists() { + continue; + } + if let Some(parent) = marker.parent() { + let _ = std::fs::create_dir_all(parent); + } + let body = build_reaper_failed_marker_body(orphan); + if std::fs::write(&marker, body).is_err() { + continue; + } + let _ = mark_meta_failed(&orphan.meta_path); + reaped.push((orphan.pr_number, orphan.age_secs)); + } + reaped +} + +/// SessionStart 時の reaper エントリポイント。orphan を検出 + reap し、 +/// nudge メッセージを返す。何も検出しなければ `None`。 +fn compute_reaper_nudge(repo_root: &Path, now_unix: i64) -> Option { + let runs_dir = repo_root.join(TAKT_RUNS_DIR); + let orphans = find_orphan_post_merge_feedback_runs(&runs_dir, now_unix); + let reaped = reap_orphans(repo_root, &orphans); + if reaped.is_empty() { + return None; + } + let mut lines = Vec::with_capacity(reaped.len() + 2); + lines.push("[POST_MERGE_FEEDBACK_REAPER]".to_string()); + lines.push(format!( + "orphan post-merge-feedback runs を {} 件検出、`.failed` marker を生成しました \ + (abrupt termination 経路の L2 recovery、ADR-030 §L2)", + reaped.len() + )); + for (pr, age) in &reaped { + lines.push(format!(" - PR #{} (経過 {} 秒)", pr, age)); + } + Some(lines.join("\n")) +} + /// cli-pr-monitor の state file から catch-up に必要な field のみ部分デシリアライズ。 /// 完全な PrMonitorState を別 crate から共有しないことで coupling を最小化する。 #[derive(Deserialize)] @@ -154,17 +410,24 @@ fn main() { emit_session_start_output(&session_id); } -/// `additionalContext` (session_id + 任意の PR monitor catch-up nudge) を組み立て -/// Claude Code に返す JSON を stdout に書き出す。 +/// `additionalContext` (session_id + 任意の PR monitor catch-up nudge + 任意の reaper nudge) を +/// 組み立て、Claude Code に返す JSON を stdout に書き出す。 /// serde_json で組み立てることで session_id 内の特殊文字を安全にエスケープする。 fn emit_session_start_output(session_id: &str) { let mut context = format!("CLAUDE_CODE_SESSION_ID={}", session_id); + let now_unix = current_unix_secs(); if let Some(state) = read_parked_state(&pr_monitor_state_path()) { - if let Some(nudge) = compute_catchup_nudge(&state, current_unix_secs()) { + if let Some(nudge) = compute_catchup_nudge(&state, now_unix) { context.push_str("\n\n"); context.push_str(&nudge); } } + if let Ok(cwd) = std::env::current_dir() { + if let Some(reaper_nudge) = compute_reaper_nudge(&cwd, now_unix) { + context.push_str("\n\n"); + context.push_str(&reaper_nudge); + } + } let output = serde_json::json!({ "hookSpecificOutput": { "hookEventName": "SessionStart", @@ -487,16 +750,275 @@ mod tests { let tmp = std::env::temp_dir().join(format!("test-sid-empty-{}", std::process::id())); let _ = std::fs::write(&tmp, ""); - // 空ファイル → 書き込むべき ("" != "session-A") let existing = std::fs::read_to_string(&tmp).unwrap(); let should_write = existing.trim() != "session-A"; assert!(should_write); - // 実際に書き込んで結果を検証 let _ = std::fs::write(&tmp, "session-A"); let content = std::fs::read_to_string(&tmp).unwrap(); assert_eq!(content, "session-A"); let _ = std::fs::remove_file(&tmp); } + + fn unique_temp_root(prefix: &str) -> PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0); + std::env::temp_dir().join(format!( + "reaper-{}-{}-{}", + prefix, + std::process::id(), + nanos + )) + } + + fn write_meta(run_dir: &Path, task: &str, status: &str, start_time: &str) { + std::fs::create_dir_all(run_dir).unwrap(); + let json = serde_json::json!({ + "task": task, + "status": status, + "startTime": start_time, + }); + std::fs::write(run_dir.join("meta.json"), serde_json::to_string_pretty(&json).unwrap()) + .unwrap(); + } + + #[test] + fn task_prefix_matches_canonical_literal() { + assert_eq!( + TAKT_TASK_PREFIX_PMF, "post-merge-feedback for #", + "TAKT_TASK_PREFIX_PMF must match cli-merge-pipeline::feedback::TAKT_TASK_PREFIX. \ + If you changed this constant, update the corresponding test in feedback.rs as well." + ); + } + + #[test] + fn orphan_threshold_matches_canonical_value() { + assert_eq!( + ORPHAN_THRESHOLD_SECS, 1500, + "ORPHAN_THRESHOLD_SECS must match cli-merge-pipeline::feedback::ORPHAN_THRESHOLD_SECS \ + (= TAKT_TIMEOUT_SECS + 300). If TAKT_TIMEOUT_SECS changes, both crates must update." + ); + } + + #[test] + fn parse_iso8601_basic_epoch() { + assert_eq!(parse_iso8601_to_unix("1970-01-01T00:00:00Z"), Some(0)); + } + + #[test] + fn parse_iso8601_handles_fractional_seconds() { + let t = parse_iso8601_to_unix("2026-05-13T12:33:23.908Z").unwrap(); + let t_no_frac = parse_iso8601_to_unix("2026-05-13T12:33:23Z").unwrap(); + assert_eq!(t, t_no_frac, "fractional seconds must be truncated, not rejected"); + } + + #[test] + fn parse_iso8601_rejects_invalid_month() { + assert!(parse_iso8601_to_unix("2026-13-01T00:00:00Z").is_none()); + } + + #[test] + fn extract_pr_number_from_post_merge_feedback_task() { + assert_eq!(extract_pr_number_from_task("post-merge-feedback for #109"), Some(109)); + assert_eq!(extract_pr_number_from_task("post-merge-feedback for #42"), Some(42)); + } + + #[test] + fn extract_pr_number_rejects_non_pmf_task() { + assert_eq!(extract_pr_number_from_task("pre-push-review"), None); + assert_eq!(extract_pr_number_from_task("post-pr-review"), None); + assert_eq!(extract_pr_number_from_task("post-merge-feedback"), None); + assert_eq!(extract_pr_number_from_task("post-merge-feedback for #abc"), None); + } + + #[test] + fn find_orphans_returns_empty_when_runs_dir_missing() { + let root = unique_temp_root("missing-runs"); + assert!(find_orphan_post_merge_feedback_runs(&root.join(".takt/runs"), 9_999_999_999).is_empty()); + } + + #[test] + fn find_orphans_detects_running_post_merge_feedback_past_threshold() { + let root = unique_temp_root("detect"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-109"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #109", "running", start_iso); + let now = start_unix + ORPHAN_THRESHOLD_SECS as i64 + 1; + let orphans = find_orphan_post_merge_feedback_runs(&runs, now); + assert_eq!(orphans.len(), 1); + assert_eq!(orphans[0].pr_number, 109); + assert!(orphans[0].age_secs >= ORPHAN_THRESHOLD_SECS); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn find_orphans_skips_runs_within_threshold() { + let root = unique_temp_root("within-threshold"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-150"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #150", "running", start_iso); + let now = start_unix + (ORPHAN_THRESHOLD_SECS as i64 - 1); + let orphans = find_orphan_post_merge_feedback_runs(&runs, now); + assert!(orphans.is_empty(), "in-flight run within timeout window must not be reaped"); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn find_orphans_skips_completed_runs() { + let root = unique_temp_root("completed"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-151"); + write_meta(&run, "post-merge-feedback for #151", "completed", "2026-05-13T03:26:40Z"); + let orphans = find_orphan_post_merge_feedback_runs(&runs, 9_999_999_999); + assert!(orphans.is_empty(), "completed runs must not be reaped"); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn find_orphans_skips_non_post_merge_feedback_workflows() { + let root = unique_temp_root("non-pmf"); + let runs = root.join(".takt/runs"); + let pre_push = runs.join("20260513-100000-pre-push-review"); + write_meta(&pre_push, "pre-push-review", "running", "2026-05-13T03:26:40Z"); + let post_pr = runs.join("20260513-100001-post-pr-review"); + write_meta(&post_pr, "post-pr-review", "running", "2026-05-13T03:26:40Z"); + let orphans = find_orphan_post_merge_feedback_runs(&runs, 9_999_999_999); + assert!(orphans.is_empty(), "non-post-merge-feedback workflows have different recovery semantics"); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn find_orphans_skips_malformed_meta_json() { + let root = unique_temp_root("malformed"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-160"); + std::fs::create_dir_all(&run).unwrap(); + std::fs::write(run.join("meta.json"), "not-valid-json{").unwrap(); + let orphans = find_orphan_post_merge_feedback_runs(&runs, 9_999_999_999); + assert!(orphans.is_empty(), "malformed meta.json must be skipped defensively"); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn reap_orphans_writes_marker_and_updates_meta() { + let root = unique_temp_root("reap"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-200"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #200", "running", start_iso); + let now = start_unix + ORPHAN_THRESHOLD_SECS as i64 + 60; + let orphans = find_orphan_post_merge_feedback_runs(&runs, now); + assert_eq!(orphans.len(), 1); + + let reaped = reap_orphans(&root, &orphans); + assert_eq!(reaped.len(), 1); + assert_eq!(reaped[0].0, 200); + + let marker = root.join(FEEDBACK_DIR_REPO_RELATIVE).join("200.md.failed"); + assert!(marker.exists()); + let body = std::fs::read_to_string(&marker).unwrap(); + assert!(body.contains("PR #200")); + assert!(body.contains("abrupt")); + assert!(body.contains("orphan reaper")); + + let updated_meta: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(run.join("meta.json")).unwrap()).unwrap(); + assert_eq!(updated_meta.get("status").and_then(|v| v.as_str()), Some("failed")); + assert_eq!( + updated_meta.get("reaped_by").and_then(|v| v.as_str()), + Some("hooks-session-start") + ); + + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn reap_orphans_skips_when_success_report_exists_despite_stale_meta() { + let root = unique_temp_root("reconciled-success"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-202"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #202", "running", start_iso); + let now = start_unix + ORPHAN_THRESHOLD_SECS as i64 + 60; + + let feedback_dir = root.join(FEEDBACK_DIR_REPO_RELATIVE); + std::fs::create_dir_all(&feedback_dir).unwrap(); + let success_report = feedback_dir.join("202.md"); + std::fs::write( + &success_report, + "# post-merge-feedback for PR #202\n\n(takt parent killed at timeout, descendants finished after)", + ) + .unwrap(); + + let orphans = find_orphan_post_merge_feedback_runs(&runs, now); + let reaped = reap_orphans(&root, &orphans); + assert!( + reaped.is_empty(), + "ADR-030 §Reconciliation path: success report exists despite stale meta.json — must not write .failed marker" + ); + assert!( + !feedback_dir.join("202.md.failed").exists(), + "no .failed marker may be written when .md success report is present" + ); + + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn reap_orphans_is_idempotent_when_marker_exists() { + let root = unique_temp_root("idempotent"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-201"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #201", "running", start_iso); + let now = start_unix + ORPHAN_THRESHOLD_SECS as i64 + 60; + + let marker_dir = root.join(FEEDBACK_DIR_REPO_RELATIVE); + std::fs::create_dir_all(&marker_dir).unwrap(); + let marker = marker_dir.join("201.md.failed"); + std::fs::write(&marker, "pre-existing detailed marker from L1").unwrap(); + + let orphans = find_orphan_post_merge_feedback_runs(&runs, now); + let reaped = reap_orphans(&root, &orphans); + assert!(reaped.is_empty(), "must not re-reap when marker already exists"); + + let body = std::fs::read_to_string(&marker).unwrap(); + assert_eq!(body, "pre-existing detailed marker from L1"); + + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn compute_reaper_nudge_returns_none_when_no_orphans() { + let root = unique_temp_root("nudge-none"); + std::fs::create_dir_all(root.join(".takt/runs")).unwrap(); + assert!(compute_reaper_nudge(&root, 9_999_999_999).is_none()); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn compute_reaper_nudge_emits_message_when_reaped() { + let root = unique_temp_root("nudge-some"); + let runs = root.join(".takt/runs"); + let run = runs.join("20260513-100000-post-merge-feedback-for-300"); + let start_iso = "2026-05-13T03:26:40Z"; + let start_unix = parse_iso8601_to_unix(start_iso).unwrap(); + write_meta(&run, "post-merge-feedback for #300", "running", start_iso); + let now = start_unix + ORPHAN_THRESHOLD_SECS as i64 + 100; + let nudge = compute_reaper_nudge(&root, now).expect("nudge must be emitted"); + assert!(nudge.contains("[POST_MERGE_FEEDBACK_REAPER]")); + assert!(nudge.contains("1 件")); + assert!(nudge.contains("PR #300")); + let _ = std::fs::remove_dir_all(&root); + } }