feat(stop-tool-call-leak): tool call leak の hard-fail 経路対応 (ADR-061) - #326
Merged
Merged
Conversation
ハーネスが tool call parse 失敗の内部リトライも失敗した turn を isApiErrorMessage:true / model:"<synthetic>" の合成 assistant エントリで終端する。 この合成エントリは type:"assistant" だが leak を持たないため、scan_tail が 「非 leak の最終 assistant」としてチェーンを打ち切り、直前の実 leak を取り逃がす。 is_synthetic を追加し、isMeta user と同様チェーンを切らずスキップする。 828764ce incident の 2 パターン (leak→合成、leak→合成→実user→isMeta→leak→合成) を 再現する unit テストを追加。
…61 主因) hard-fail 経路 (ツール呼び出し parse 失敗 → 内部リトライも失敗 → 合成エントリで turn エラー終了) では Stop hook が発火しないため、Stop hook 側の修正では届かない。 単一 exe を hook_event_name で分岐させ、UserPromptSubmit で「最後の assistant 活動が hard-fail leak」を検知する回収層を追加。additionalContext (モデル向け、XML 再出力の 禁止を明示) + 任意 systemMessage (ユーザー可視 1 行、ADR-059) の非ブロッキング 2 チャネル で再実行を促す (decision:block は出さない)。config は prompt_recovery_enabled / recovery_system_message_enabled を追加 (ともに code default OFF、ADR-039)。telemetry は Decision::Warn / id suffix "/prompt-recovery" で Stop block と区別する。
…ig 有効化 (ADR-061) hooks-config.toml の [stop_tool_call_leak] に prompt_recovery_enabled / recovery_system_message_enabled を追加 (dogfood のため true)。 e2e に実 incident 由来 fixture を追加 (ADR-049): - synthetic_after_leak_blocks_in_stop_mode: 副因 (合成跨ぎ block) の end-to-end 検証 - recovery_mode_*: UserPromptSubmit で additionalContext / systemMessage 出力、 clean/leak-only 非発火、kill-switch skip config assertion に新キーの値 assert を追加 (dev-conventions § fixture は値まで assert)。
settings.local.json.template と settings.json (cloud dispatcher、ADR-060) の UserPromptSubmit 配列に hooks-stop-tool-call-leak を追加 (feedback-recovery の後、 timeout 5)。同一 exe が hook_event_name で Stop / UserPromptSubmit を分岐する。 settings.local.json は template から build:hooks-settings で再生成 (gitignore 対象)。
…ood 追記 ADR-061 を新規作成 (試験運用、ADR-039 3 点セット): 828764ce incident の背景・ corpus 調査結論・根本原因 2 層 (主因=Stop 不発火 / 副因=scan_tail 合成打ち切り)・ 設計決定・bounded lifetime (ADR-053 連動) を記録。 ADR-053 に dogfood 観測 (取り逃がし 2 件、2026-07-27/28) を追記し ADR-061 を相互参照。 CLAUDE.md の ADR index に ADR-061 を追加。実装指示書 (plan md) は完了に伴い削除。
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughtool call leak の hard-fail 経路を UserPromptSubmit で回収する処理を追加しました。合成 assistant を考慮した transcript 走査、non-blocking 出力、設定・フック登録、ADR、ユニットテストと E2E テストを更新しています。 ChangesTool call leak 回収
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UserPromptSubmit
participant hooks-stop-tool-call-leak
participant scan_recovery
participant recovery_render
UserPromptSubmit->>hooks-stop-tool-call-leak: hook_event_name と transcript_path
hooks-stop-tool-call-leak->>scan_recovery: transcript を走査
scan_recovery-->>hooks-stop-tool-call-leak: 回収要否とツール名
hooks-stop-tool-call-leak->>recovery_render: 回収メッセージを生成
recovery_render-->>UserPromptSubmit: additionalContext と任意 systemMessage
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし (レビュー未着) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし 差分概要10 ファイル変更 (ADR-061 新規、既存 ADR-053 更新、hooks-config.toml/settings.json/settings.local.json.template/CLAUDE.md 設定追加、Rust 実装 3 ファイル + 新規 recovery.rs + e2e テスト)。内容は既存の 次のアクション
|
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Owner
Author
|
@coderabbitai review full |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 12 minutes. |
Owner
Author
|
@coderabbitai review full |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks-stop-tool-call-leak/tests/e2e.rs`:
- Around line 287-297: Update the recovery_mode_includes_system_message test to
reject carriage returns as well as newline characters in systemMessage. Extend
the assertion around msg so both '\n' and '\r' are treated as line breaks while
preserving the existing one-line validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f7d4245-228b-46b9-a7f5-9c325b2c2056
📒 Files selected for processing (10)
.claude/hooks-config.toml.claude/settings.json.claude/settings.local.json.templateCLAUDE.mddocs/adr/adr-053-stop-tool-call-leak-detection.mddocs/adr/adr-061-tool-call-leak-hardfail-recovery.mdsrc/hooks-stop-tool-call-leak/src/main.rssrc/hooks-stop-tool-call-leak/src/recovery.rssrc/hooks-stop-tool-call-leak/src/transcript.rssrc/hooks-stop-tool-call-leak/tests/e2e.rs
Comment on lines
+287
to
+297
| #[test] | ||
| fn recovery_mode_includes_system_message() { | ||
| let dir = tempfile::tempdir().expect("temp dir"); | ||
| let path = write_transcript(&dir, &[assistant_text_entry(LEAK_TEXT), synthetic_entry()]); | ||
| let (stdout, _stderr) = run_hook(&stdin_for_event(&path, "UserPromptSubmit"), None); | ||
| let out: Value = serde_json::from_str(&stdout).expect("stdout は回収 JSON"); | ||
| let msg = out["systemMessage"] | ||
| .as_str() | ||
| .expect("recovery_system_message_enabled=true なので systemMessage が付く"); | ||
| assert!(!msg.contains('\n'), "systemMessage は 1 行 (ADR-059): {}", msg); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
systemMessage の CR も改行として拒否してください。
現在の検証は \n のみ拒否するため、\r を含むメッセージでも「1 行」テストを通過します。
修正案
- assert!(!msg.contains('\n'), "systemMessage は 1 行 (ADR-059): {}", msg);
+ assert!(
+ !msg.contains('\n') && !msg.contains('\r'),
+ "systemMessage は 1 行 (ADR-059): {}",
+ msg
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[test] | |
| fn recovery_mode_includes_system_message() { | |
| let dir = tempfile::tempdir().expect("temp dir"); | |
| let path = write_transcript(&dir, &[assistant_text_entry(LEAK_TEXT), synthetic_entry()]); | |
| let (stdout, _stderr) = run_hook(&stdin_for_event(&path, "UserPromptSubmit"), None); | |
| let out: Value = serde_json::from_str(&stdout).expect("stdout は回収 JSON"); | |
| let msg = out["systemMessage"] | |
| .as_str() | |
| .expect("recovery_system_message_enabled=true なので systemMessage が付く"); | |
| assert!(!msg.contains('\n'), "systemMessage は 1 行 (ADR-059): {}", msg); | |
| } | |
| #[test] | |
| fn recovery_mode_includes_system_message() { | |
| let dir = tempfile::tempdir().expect("temp dir"); | |
| let path = write_transcript(&dir, &[assistant_text_entry(LEAK_TEXT), synthetic_entry()]); | |
| let (stdout, _stderr) = run_hook(&stdin_for_event(&path, "UserPromptSubmit"), None); | |
| let out: Value = serde_json::from_str(&stdout).expect("stdout は回収 JSON"); | |
| let msg = out["systemMessage"] | |
| .as_str() | |
| .expect("recovery_system_message_enabled=true なので systemMessage が付く"); | |
| assert!( | |
| !msg.contains('\n') && !msg.contains('\r'), | |
| "systemMessage は 1 行 (ADR-059): {}", | |
| msg | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks-stop-tool-call-leak/tests/e2e.rs` around lines 287 - 297, Update
the recovery_mode_includes_system_message test to reject carriage returns as
well as newline characters in systemMessage. Extend the assertion around msg so
both '\n' and '\r' are treated as line breaks while preserving the existing
one-line validation.
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)該当なし 次のアクション
|
CodeRabbit (Minor): systemMessage の 1 行検証で復帰文字 (CR) も拒否するよう e2e / unit の assert を厳密化 (LF のみ拒否では CR 入りメッセージが「1 行」テストを 通過するため)。 simplicity-review (DRY): run_check / run_recovery で重複していた transcript 読み込み + fail-open ブロックを load_tail_entries ヘルパーに共通化。
aloekun
added a commit
that referenced
this pull request
Jul 28, 2026
…59) (#327) * feat(lib-hook-output): SingleLineMessage 型で systemMessage の単一行を構造保証 (ADR-059) systemMessage の「1 行」不変条件は従来 producer の format! + per-site の assert!(!msg.contains('\n')) に依存し、`\r` 見落とし (PR #326 CodeRabbit 指摘) の ように site ごとに再発し得た。 共有 crate lib-hook-output に newtype SingleLineMessage を追加。構築時に \r\n / \n / \r を 単一空白へサニタイズし、内部値が必ず 1 行になることを型で保証する (serde transparent で wire 形式は素の文字列のまま)。debug_assert で dev 時に producer の改行混入を surface。 * refactor(stop-tool-call-leak): recovery systemMessage を SingleLineMessage に移行 (ADR-059) RecoveryOutput.system_message を Option<String> → Option<SingleLineMessage> に、 build_system_message の戻り値を SingleLineMessage に変更。生の String を systemMessage に 載せることをコンパイル時に不可能にする。per-site の改行 assert は型保証に置換して削除。 * refactor(session-start): weekly-review systemMessage を SingleLineMessage に移行 (ADR-059) build_weekly_review_system_message / WeeklyReviewNudge.system_message / build_session_start_json を SingleLineMessage 型に移行。PR #326 で発見された weekly_review.rs の CR 未チェックギャップは、型による構造保証で恒久的に塞がれる (per-site の改行 assert は削除、単一行性は lib-hook-output のテストに集約)。 * docs(adr-059): 単一行不変条件の型による構造保証 (SingleLineMessage) を追補 per-site 検証 (format! + !contains) が site ごとに漏れた経緯 (PR #326 CodeRabbit 指摘 + diff 外 weekly_review.rs の同型ギャップ) と、lib-hook-output::SingleLineMessage による ルール→仕組み化 (ADR-042) の決定・スコープ (systemMessage 専用) を追補として記録。 * fix(lib-hook-output): new を全ビルド一律サニタイズに (debug_assert 除去) + ADR 追補日修正 PR #327 CodeRabbit 指摘への対応: - Major: SingleLineMessage::new の debug_assert がサニタイズより先に改行入力で panic し、 fail-open 安全網が debug/test で機能せず build 間で挙動が割れていた。assert を除去して 全ビルド一律サニタイズにする (new が改行を panic せずサニタイズする回帰テスト追加)。 - Minor: ADR-059 追補日 2026-07-29 (未来) を review 基準の 2026-07-28 に修正。 ADR-059 追補の debug_assert 記述も一律サニタイズの根拠に更新。
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
scan_tailが合成エントリ（isApiErrorMessage/model:"<synthetic>"）でチェーンを打ち切り、直前の実 leak を取り逃がす問題を修正（副因対処）hook_event_nameで Stop / UserPromptSubmit に分岐。回収層は非ブロッキング（additionalContext+ 任意systemMessage、decision:blockは出さない）prompt_recovery_enabled/recovery_system_message_enabledを追加（code default OFF、ADR-039）、kill-switch は既存 env を両経路共通化Context
scan_tailが合成エントリでチェーンを打ち切る。run_check/run_recoveryの transcript 読み込み重複）は follow-up 候補として保留。Validation
cargo test --workspace: 全 crate pass（hooks-stop-tool-call-leak: 57 unit + 13 e2e）cargo clippy --workspace -- -D warnings: clean /pnpm lint:md: 0 error / tsc: エラーなしpnpm pushpre-push review: verdict=APPROVE（security + simplicity、最新 master docs(adr): weekly-review 検出の ADR drift を修正 #325 への rebase 後の再検証を含む）References
Summary by CodeRabbit
改善
設定