feat(post-merge-feedback): Bundle c-1 (D-7) — L1 Drop guard + L2 orphan reaper + ADR-030 spec#154
Merged
Merged
Conversation
…an reaper + ADR-030 spec PR #109 で実証された ADR-030 仕様違反 (SIGPIPE で feedback workflow が silent 中断 + .failed marker 未生成) への構造的解消。3 層 (pre-emptive marker / RAII Drop guard / out-of-process reaper) で abrupt termination 経路を多層防御する。 ## 順位 63 — cli-merge-pipeline pre-emptive marker + Drop guard - write_pending_marker を feedback::run の concurrent_run_guard 直後に追加 - RAII FailedMarkerGuard を armed 状態で生成、Ok path で disarm - Drop guard は idempotent (marker.exists() check で caller の detailed marker を保護) - Rust default SIGPIPE は SIG_DFL = unwind せず即時終了するため Drop は呼ばれない。 pre-emptive marker のディスク先置きで救済する設計 (signal trap A は skip) - run() を 50 行ガイドライン遵守のため prepare_transcript / reconcile_takt_output に分割 ## 順位 64 — hooks-session-start orphan run reaper - .takt/runs/*/meta.json scan: status=running + post-merge-feedback task prefix + ORPHAN_THRESHOLD_SECS (1500s = TAKT_TIMEOUT_SECS + 余裕 5 分) 経過の run を検出 - reap: .failed marker 生成 + meta.json status=failed 更新 (reaped_by field 追加) - 冪等性: 既存 .failed marker または .claude/feedback-reports/<pr>.md 成功レポート 存在時は skip (advisor 指摘の ADR-030 §Reconciliation false-positive 対策) - ISO 8601 parser は check-ci-coderabbit::parse_iso8601_to_unix と同型 (no chrono dep) - TAKT_TASK_PREFIX_PMF / ORPHAN_THRESHOLD_SECS は cli-merge-pipeline 側と inline duplicate + 両 crate の test で literal pin (drift 検出) ## 順位 67 — ADR-030 spec amendment - "Abrupt 終了の多層 recovery (Bundle c-1 で追加)" subsection を追加 - L1 in-process / L2 out-of-process の責務分離マトリクス - Reconciliation-aware reaper skip ルールを spec として明記 - SLA は ORPHAN_THRESHOLD_SECS / TAKT_TIMEOUT_SECS を name で参照 (数値 drift 防止) ## 検証 - workspace tests: 901+ pass / 0 failures (新規 test: feedback.rs 7 件 + session-start 16 件) - clippy clean (Error::other 移行 + dead_code on canonical const) - release exe ビルド + .claude/ にコピー済 ## Bundle c 残り (本 PR scope 外) - 順位 65 (exe + --help PreToolUse block) / 順位 66 (pipe truncate global rule) は Bundle c-2 として todo7.md に残置 Refs: ADR-030, PR #109 (root cause), feedback-reports/109.md (Tier 1 #1/#2 + Tier 3 #6)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughADR-030 にポストマージフィードバックの多層確定的回復設計を追加指定し、 Changes多層確定的回復メカニズム実装
Sequence Diagram(s)sequenceDiagram
participant Client as User/CI
participant FbRun as feedback::run()
participant Guard as FailedMarkerGuard
participant Takt as takt workflow
participant Report as feedback-report.md
participant SessionStart as hooks-session-start (次セッション)
participant Meta as meta.json
Note over FbRun,Guard: L1: インプロセス即時回復
FbRun->>FbRun: 同時実行ガード確認
FbRun->>FbRun: pending marker 即座に書き込み
FbRun->>Guard: guard arm
FbRun->>Takt: takt workflow 実行開始
alt takt 実行成功 && 報告生成
Takt->>Report: feedback-report.md 生成
FbRun->>FbRun: reconcile_takt_output() 試行
FbRun->>FbRun: 報告コピー成功
FbRun->>Guard: disarm (guard発火抑止)
FbRun->>FbRun: pending marker 削除
FbRun->>Client: success 返却
else takt timeout/失敗 || reconcile失敗
FbRun->>FbRun: 異常終了
Guard->>FbRun: drop 発火
Guard->>FbRun: backup pending marker 維持
end
Note over SessionStart,Meta: L2: アウトオブプロセス定期修復
SessionStart->>Meta: .takt/runs/*/meta.json 走査
loop orphan threshold 超過ラン検出
Meta-->>SessionStart: "post-merge-feedback for `#N`" + status="running" + startTime > 1500s
alt marker既存 || 成功報告既存
SessionStart->>SessionStart: reconciliation skip
else orphan確定
SessionStart->>Meta: status="failed", reaped_by="hooks-session-start" 更新
SessionStart->>FbRun: .failed marker 作成
end
end
SessionStart->>Client: additionalContext に reaper nudge 追記
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merged
7 tasks
aloekun
added a commit
that referenced
this pull request
May 15, 2026
…hase D Round 2 完遂反映 (#155) * docs(llm-offload): Phase D Round 2 完遂状況を analysis.md / outcomes.md に反映 (D-7 #154 結果込み) * feat(lint-screen): Bundle k-1 (順位 123) — Markdown ハンク除外フィルターを追加 Phase D dogfood で 5 PR 連続観測された false positive (mistral:7b が docs-only diff や `.md` ファイルに対して Rust の `unused-import` を hallucinate) を 拡張子ベースの mechanical filter で構造的に解消する。 ## 設計 - `src/cli-push-runner/src/stages/lint_screen.rs` に `filter_excluded_hunks` 関数を追加 - `EXCLUDED_EXTENSIONS = ["md", "markdown"]` を hardcode (大文字小文字無視) - `diff --git ` 行を file-diff 境界として 1 ハンク = 1 chunk に分割、各 chunk の `+++ b/<path>` (または `--- a/<path>`) から拡張子を抽出して判定 - 全ハンクが対象外拡張子 (= docs-only diff) の場合は `FilterResult::AllExcluded` を 返し、invoke を完全に skip + skip-report (`screen_decision: skipped` + 理由) を書き出す short-circuit path に分岐 ## なぜ filter 適用箇所を (b) lint_screen stage 内にしたか - (a) `.takt/review-diff.txt` 生成時に drop すると review 全体の汎用性を壊す - (c) prompt 内で「.md は無視せよ」と instruct は LLM 信頼で危険 - (b) lint_screen stage 限定で副作用最小、Bundle k 順位 123 計画通り ## なぜ `run_lint_screen` を `invoke_and_write_report` に分割したか filter 経路追加で 50 行ガイドラインを超過 (58 行)。invoke + write_report の 2 step を `invoke_and_write_report` ヘルパーに切り出して 30 行台に収めた。 ## テストカバレッジ (10 件新規) - Rust-only / mixed (Rust + .md) / pure .md / pure .markdown - 大文字 (.MD / .Markdown) も除外対象に含む - `something.mdxyz.rs` のような中途 `.md` を含む path は除外しない - `/dev/null` create / delete の片側 path 判定 - 3-file mixed diff で hunk 境界が正しく保たれる - skip-report 本文に "skipped" / "docs-only diff" / "Bundle k 順位 123" を含む ## 累積 false positive への効果見積 5 観測のうち: - D-4 CR fix (TOML) — scope 外 (TOML は除外対象に含まない) = 残存 - D-5 ×2 + D-6 (Markdown / docs-only) — **構造的解消** (3 件消滅) - D-5 初回 (Rust + docs mixed) — Rust 部分は依然 mistral:7b に渡る = scope 内 Rust FP が残る可能性 期待: Phase E 採否判定前の FP rate を 5/7 → ~2/7 に低減。残 2 件は別 root cause (Rust scope hallucinate / TOML hallucinate) のため別途分析が必要。 Refs: Bundle k 順位 123 (todo8.md 削除済)、ADR-038 Known failure mode、 docs/local-llm-offload-phase-d-outcomes.md L162 (false positive 累積観測) * fix(lint-screen): CR #155 Major — chunk_has_excluded_extension で +++ b/ (new path) を優先 CodeRabbit Major 指摘の rename 時除外漏れ bug を解消する。unified diff の慣例で `--- a/<path>` が `+++ b/<path>` より先に出現するため、find_map の旧実装では 両者を OR で取ると常に旧パスが優先されてしまう問題があった。 ## 修正前の bug ```rust let path = chunk.lines().find_map(|line| { line.strip_prefix("+++ b/") .or_else(|| line.strip_prefix("--- a/")) }).unwrap_or(""); ``` `*.rs → *.md` の rename で: - `--- a/src/a.rs` を先に処理 → `find_map` が "src/a.rs" を返す - `+++ b/docs/a.md` に到達せず - 拡張子 = "rs" → EXCLUDED_EXTENSIONS に含まれず → mistral:7b が `.md` 内容を見る - = Bundle k-1 の filter contract (新パス優先) に違反 ## 修正後 ```rust let new_path = chunk.lines().find_map(|line| line.strip_prefix("+++ b/")); let old_path = chunk.lines().find_map(|line| line.strip_prefix("--- a/")); let path = new_path.or(old_path).unwrap_or(""); ``` new_path を chunk 全体から先に探す → rename でも新拡張子で判定。new_path が無い 場合 (= `+++ /dev/null` の delete) のみ old_path にフォールバック。 ## テスト追加 (2 件) - `filter_excluded_hunks_prefers_b_path_on_rename_to_markdown`: `*.rs → *.md` rename が AllExcluded になることを assert - `filter_excluded_hunks_keeps_rename_from_md_to_rust`: 逆方向 (`*.md → *.rs`) rename が Kept になることを assert (symmetric 検証) ## 累積テスト cli-push-runner stages::lint_screen 24 → 26 件 pass、clippy clean、release exe deploy 済 Refs: PR #155 CR review コメント (Major 🟠 Quick win)、Bundle k-1 順位 123、ADR-038
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ADR-030 §「失敗マーカーによる recovery」を abrupt termination 経路 (SIGPIPE / SIGTERM / kill -9 / SIGKILL / power loss / OOM Killer / panic) でも保証する 3 層 recovery を実装。PR #109 で実証された silent loss を構造的に解消する。
feedback::run冒頭で.failedmarker を pre-emptive 書込み + RAIIFailedMarkerGuard(idempotent backup)。Rust default SIGPIPE = SIG_DFL は unwind せず Drop が走らないため、marker のディスク先置きで救済 (順位 63)。hooks-session-startに orphan run reaper を追加。.takt/runs/*/meta.jsonのstatus: "running"+ post-merge-feedback task prefix +ORPHAN_THRESHOLD_SECS (= 1500s = TAKT_TIMEOUT_SECS + 余裕 5 分)経過の run を検出 →.failedmarker 生成 + meta.json 更新。Advisor 指摘の ADR-030 §Reconciliation false-positive 対策 として、<pr>.md成功レポートが既存の場合は skip (takt parent kill 後に descendants が完成した path) (順位 64)。Phase D dogfood data point #7
human_review(fallback)os error 10060) — server unavailable## Diagnosticsection新規 failure mode 観測 (Phase E 採否判定の追加考慮事項):
unused-import」とは異なる、Ollama サーバ自体の network timeout を確認Design notes
SIG_DFLで即時終了し Drop guard すら走らない。nixcrate + Unix/Windows cfg gate を追加しても pre-emptive marker と比べた marginal な追加カバー範囲は panic 経路のみ。よって C (pre-emptive) を主軸に、Drop guard を backup として配置する 2 層構成にスコープを絞った。TAKT_TASK_PREFIX/ORPHAN_THRESHOLD_SECSを直接 dep 化 (cycle risk) ではなく inline duplicate + 両 crate の test で literal pin (advisor option c) を採用。drift 検出はcargo testで機械的に行う。run()50 行ガイドライン: pre-emptive marker 追加でrun()が 50 行超 (72 行) になったため、prepare_transcript/reconcile_takt_outputヘルパーに分割して責務を明示化。.failedmarker 既存だけでなく<pr>.md成功レポート既存も check (advisor フィードバック反映)。stale meta.json を放置する方が、false-positive.failedで UserPromptSubmit hook が毎 prompt nag するより害が少ない設計トレードオフ。Bundle c の残作業 (本 PR scope 外)
exe + --helpを PreToolUse でブロック (todo7.md L266-)両者は Bundle c-2 として別 PR 化推奨 (c-1 = 中断後 recovery / c-2 = 中断 trigger の事前防止)。
Test plan
cargo test --workspace— 901+ tests pass / 0 failures (新規: feedback.rs 7 件 + hooks-session-start 16 件)cargo clippy -p cli-merge-pipeline -p hooks-session-startclean.claude/deploy 完了Refs
.claude/feedback-reports/109.mdTier 1 feat(hooks): 設定駆動型アーキテクチャに移行し配布自動化を実装 #1/fix(hooks): stop-quality のパイプデッドロックを修正 #2 + Tier 3 feat(hooks): カスタムリンターエンジンの導入 #6Summary by CodeRabbit
リリースノート
ドキュメント
改善