diff --git a/TODO.md b/TODO.md index 3a8d4bef..2dbe8768 100644 --- a/TODO.md +++ b/TODO.md @@ -1,7 +1,6 @@ - [ ] テスト設計 → [tickets/test-design.md](tickets/test-design.md) - [ ] ツール設計 - [ ] Bash ツール (Permission 層と統合) → [tickets/bash-tool.md](tickets/bash-tool.md) - - [ ] ツール実行結果のサイズ上限 → [tickets/tool-output-limit.md](tickets/tool-output-limit.md) - [ ] 複数 Pod 間の Scope 排他制御 → [tickets/scope-exclusion.md](tickets/scope-exclusion.md) - [ ] Compact の改善（要約品質 + 挙動詳細） → [tickets/compact-improvements.md](tickets/compact-improvements.md) - [ ] Protocol の設計 → [tickets/protocol-design.md](tickets/protocol-design.md) diff --git a/crates/llm-worker/src/worker.rs b/crates/llm-worker/src/worker.rs index 41e06e8a..4ab4b8e1 100644 --- a/crates/llm-worker/src/worker.rs +++ b/crates/llm-worker/src/worker.rs @@ -650,10 +650,33 @@ impl Worker { } }; - // Cap `content` byte-size before it enters history. This is the - // single chokepoint that protects the next LLM request from - // blowing past the provider's per-minute input-token limit; no - // individual tool is trusted to self-limit. + // Phase 3: Apply post_tool_call interceptor + for tool_result in &mut results { + if let Some((tool_call, meta, tool)) = call_info_map.get(&tool_result.tool_use_id) { + let mut info = ToolResultInfo { + call: tool_call.clone(), + result: tool_result.clone(), + meta: meta.clone(), + tool: tool.clone(), + }; + + match self.interceptor.post_tool_call(&mut info).await { + PostToolAction::Continue => {} + PostToolAction::Abort(reason) => { + self.last_run_interrupted = true; + return Err(WorkerError::Aborted(reason)); + } + } + // Reflect interceptor-modified results + *tool_result = info.result; + } + } + + // Phase 4: Cap `content` byte-size before it enters history. + // Runs *after* post_tool_call so interceptors (audit, logging, + // classification) still observe the full content, and any + // content they inject is also truncated — closing the last gap + // before the data reaches the next LLM request. if let Some(limits) = self.tool_output_limits.as_ref() { for tool_result in &mut results { let Some(content) = tool_result.content.as_mut() else { @@ -677,28 +700,6 @@ impl Worker { } } - // Phase 3: Apply post_tool_call interceptor - for tool_result in &mut results { - if let Some((tool_call, meta, tool)) = call_info_map.get(&tool_result.tool_use_id) { - let mut info = ToolResultInfo { - call: tool_call.clone(), - result: tool_result.clone(), - meta: meta.clone(), - tool: tool.clone(), - }; - - match self.interceptor.post_tool_call(&mut info).await { - PostToolAction::Continue => {} - PostToolAction::Abort(reason) => { - self.last_run_interrupted = true; - return Err(WorkerError::Aborted(reason)); - } - } - // Reflect interceptor-modified results - *tool_result = info.result; - } - } - Ok(ToolExecutionResult::Completed(results)) } diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 60e61513..49e37480 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -80,8 +80,12 @@ pub struct WorkerManifest { pub max_turns: Option, #[serde(default)] pub temperature: Option, + /// Byte-size caps applied to tool `content` before it reaches the + /// conversation history. The section is optional in TOML — when + /// omitted, `ToolOutputLimits::default()` (16KB default cap, no + /// per-tool overrides) is applied so truncation is on by default. #[serde(default)] - pub tool_output: Option, + pub tool_output: ToolOutputLimits, } /// Byte-size caps applied to tool execution `content` before it enters @@ -411,9 +415,11 @@ permission = "write" } #[test] - fn omitted_tool_output_is_none() { + fn omitted_tool_output_falls_back_to_default_16k() { let manifest = PodManifest::from_toml(MINIMAL_REQUIRED).unwrap(); - assert!(manifest.worker.tool_output.is_none()); + let limits = &manifest.worker.tool_output; + assert_eq!(limits.default_max_bytes, 16 * 1024); + assert!(limits.per_tool.is_empty()); } #[test] @@ -428,7 +434,7 @@ permission = "write" Grep = 4096\n", ); let manifest = PodManifest::from_toml(&toml).unwrap(); - let limits = manifest.worker.tool_output.unwrap(); + let limits = &manifest.worker.tool_output; assert_eq!(limits.default_max_bytes, 8192); assert_eq!(limits.limit_for("Read"), 32768); assert_eq!(limits.limit_for("Grep"), 4096); @@ -436,14 +442,14 @@ permission = "write" } #[test] - fn tool_output_default_max_bytes_is_16k() { + fn empty_tool_output_section_uses_default_max_bytes() { let toml = MINIMAL_REQUIRED.replace( "[worker]\n", "[worker]\n\ [worker.tool_output]\n", ); let manifest = PodManifest::from_toml(&toml).unwrap(); - let limits = manifest.worker.tool_output.unwrap(); + let limits = &manifest.worker.tool_output; assert_eq!(limits.default_max_bytes, 16 * 1024); assert!(limits.per_tool.is_empty()); } diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 944ad1cb..14c7870a 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -846,9 +846,9 @@ pub fn apply_worker_manifest(worker: &mut Worker, wm: &WorkerMa } worker.set_request_config(config); worker.set_max_turns(wm.max_turns.map(|n| n.get())); - worker.set_tool_output_limits(wm.tool_output.as_ref().map(|limits| ToolOutputLimits { - default_max_bytes: limits.default_max_bytes, - per_tool: limits.per_tool.clone(), + worker.set_tool_output_limits(Some(ToolOutputLimits { + default_max_bytes: wm.tool_output.default_max_bytes, + per_tool: wm.tool_output.per_tool.clone(), })); } diff --git a/tickets/tool-output-limit.md b/tickets/tool-output-limit.md deleted file mode 100644 index 517738c7..00000000 --- a/tickets/tool-output-limit.md +++ /dev/null @@ -1,87 +0,0 @@ -# ツール実行結果のサイズ上限 - -## レビュー状態 - -初回レビュー実施済み。指摘事項と判断は [tool-output-limit.review.md](tool-output-limit.review.md) を参照。 -指摘1（デフォルト不適用）・指摘2（truncate と interceptor の順序）の修正待ち。 - -## 背景 - -Pod のセッションで、Glob が `pattern:"*"` でプロジェクト全体を走査し、 -約 125KB / 推定 70k トークン超の tool_result を返した結果、次ターンのリクエストが -組織レートリミット（30k input tokens/分）を単発で超過して永久に 429 で詰む事故が発生した。 - -一度肥大化した履歴は prune/compact が走る前に再送され続け、待っても抜けられない。 -根本原因はツールが「呼ばれた通りの結果を素直に全部返す」こと。ツール自身の件数上限 -（例: `Glob` の 1000 件）はバイト/トークン単位の上限ではないため機能しない。 - -ツール実行結果のサイズを LLM に投げる前に強制的にキャップし、LLM に -「検索範囲を絞れ」と促す必要がある。 - -## 単位について - -理想はトークン単位での上限だが、既存の `pod::token_counter` は provider の -`UsageRecord` 実測値を基にした按分・外挿専用で、**未送信の単発ツール出力を -事前にトークン計測する手段は現時点で存在しない**。 -ローカルトークナイザは精度・信頼性の理由で意図的に持たない方針。 - -そこで本チケットではバイト数ベースで上限をかける。UTF-8 のため -`str::len()` で O(1) に判定でき、`floor_char_boundary` を使えば文字境界で -安全に切断できる。将来 provider 実測値ベースのトークン上限に -置き換える余地は残す（マニフェストのキー名をそれに合わせる）。 - -## 要件 - -- **単一チョークポイントで全ツールに効く**: 個別ツールの実装を信用しない。 - Tool 実行境界（`llm-worker::worker::execute_tools` 内、`ToolResult::from_output` - 直後）で `ToolOutput.content` のバイト数を計測し、 - 上限を超えていたら切り詰めてから履歴に積む。 -- **マニフェストで設定可能**: デフォルトは 16KB（30k/分レートリミットに対して - 余裕を持った値）。プロジェクトごと・ツールごとに上書き可能。 -- **切り詰め後は LLM が検知できる**: `content` 末尾に - `[truncated: N bytes dropped, refine your query]` 形式の追記を入れ、 - LLM が自発的に絞り込みを試みるヒントにする。`ToolError` にはしない - （エラーにすると LLM がリトライループに入りやすい）。 -- **`summary` には手を入れない**: summary は常に短い 1-2 行で、上限に達しない前提。 -- **`content` が `None` の場合はスキップ**: 計測・切り詰めの対象外。 - -## マニフェスト - -```toml -[worker.tool_output] -# 全ツール共通の既定上限（バイト）。省略時 16384。 -default_max_bytes = 16384 - -# ツールごとの上書き。ツール名は登録名（"Glob", "Read", ...）。 -[worker.tool_output.per_tool] -Read = 32768 # Read は大きいファイルを意図的に返すので少し緩める -Grep = 8192 -``` - -- `[worker.tool_output]` セクション自体は省略可能。省略時はデフォルト 16KB が全ツールに適用。 -- `per_tool` も省略可能。 -- 未知のツール名がマップに含まれていても manifest エラーにはしない（ログ警告のみ）。 - -## 実装方針（実装順序） - -1. `manifest::WorkerManifest` に `tool_output: Option` を追加。 - `ToolOutputLimits { default_max_bytes: usize, per_tool: HashMap }`。 -2. 切り詰め関数を `llm-worker` 側に薄く追加。 - 入力: `content: String`, `limit: usize`, `tool_name: &str`。 - `content.len() <= limit` ならそのまま返す。超えていれば - `str::floor_char_boundary(limit - suffix.len())` で切って末尾に注記を追記。 -3. `Worker` 生成時に `tool_output: Option` を渡し、 - `execute_tools` の結果ループで `ToolResult::content` を in-place に書き換える。 -4. 各ツール単体には本チケットでは手を入れない。上限を踏んだツールに対して - 後続の改善（Glob が `git_ignore` を尊重する等）は別チケットで扱う。 - -## 非ゴール - -- **ツール固有の賢い縮退**（Glob が件数で、Read が行範囲で、など）は扱わない。 - まず一律上限で事故を止め、各ツールの自主制限は必要に応じて別チケットで追加する。 -- **prompt caching の導入**や compaction 側の改善は扱わない。 - 本チケットは「1 回のツール結果が履歴に載る前にキャップする」ことだけに集中する。 -- **入力側（ツール引数）のサイズ制限**は扱わない。 -- **トークン単位での上限**は扱わない。将来 provider 実測値ベースの - オンライン・トークン推定が利用可能になった段階で、本チケットで入れた - バイト上限をトークン上限に置き換えることを検討する。 diff --git a/tickets/tool-output-limit.review.md b/tickets/tool-output-limit.review.md deleted file mode 100644 index cbb3c188..00000000 --- a/tickets/tool-output-limit.review.md +++ /dev/null @@ -1,87 +0,0 @@ -# レビュー: ツール実行結果のサイズ上限 - -対象コミット: 未コミット（作業ツリー） -対象差分: `crates/manifest/src/lib.rs`, `crates/llm-worker/src/{lib,tool,worker}.rs`, `crates/pod/src/pod.rs` - -## 要件達成状況 - -| 要件 | 状態 | -|---|---| -| 単一チョークポイントで全ツールに効く | ✅ `worker::execute_tools` 内に配置 | -| マニフェストで設定可能（デフォルト 16KB） | 🚨 省略時にデフォルトが効かない（下記参照） | -| 切り詰めマーカーで LLM が検知できる | ✅ `[truncated: N bytes dropped, refine your query]` | -| `summary` 非改変 | ✅ `ToolResult::content` のみを書き換え | -| `content: None` スキップ | ✅ `as_mut()` の else で continue | -| 未知ツール名を許容 | ✅ `HashMap::get` なので自然にフォールバック | -| UTF-8 安全な切断 | ✅ `is_char_boundary` ループで実装 | -| 切り詰め関数の単体テスト | ✅ ASCII / UTF-8 / 境界 / per_tool | - -## 指摘 - -### 1. 🚨 デフォルト 16KB が `[worker.tool_output]` 省略時に効かない - -チケット本文: -> `[worker.tool_output]` セクション自体は省略可能。省略時はデフォルト 16KB が全ツールに適用。 - -現状の実装: - -- `manifest::WorkerManifest.tool_output: Option` + `#[serde(default)]` - → TOML で省略すると `None` -- `pod::apply_worker_manifest`: - ```rust - worker.set_tool_output_limits(wm.tool_output.as_ref().map(|l| ...)); - ``` - → `None` がそのまま worker に伝播 -- `Worker::execute_tools`: - ```rust - if let Some(limits) = self.tool_output_limits.as_ref() { ... } - ``` - → `None` なら truncate ブロック丸ごとスキップ - -結果、マニフェストに `[worker.tool_output]` を書き忘れると truncate 自体が無効化される。事故再発防止が目的のチケットで、**ユーザーの明示的なオプトイン**に縮退しており要件違反。 - -**判断**: 要修正（必須）。 - -**修正方針**: `manifest::WorkerManifest.tool_output` を非 Option にして `#[serde(default)]` + `impl Default for ToolOutputLimits` を利用する。Pod 側の `as_ref().map(...)` も `Some(...)` を常に渡す形に直す。 - ---- - -### 2. 🟡 truncate が post_tool_call interceptor の **前** に挿入されている - -現状の順序: - -``` -Phase 2: ツール実行 - → truncate -Phase 3: post_tool_call interceptor - → 履歴に積む -``` - -この順だと post_tool_call interceptor が切り詰め済みの content しか見られず、ログ・監査・分類などで full content を使いたいケースで情報ロスが起きる。 -また interceptor が content を差し替えた場合、その差し替え後の巨大データは truncate を経由せず履歴に入る（防御が迂回される）。 - -**判断**: 修正する。truncate を Phase 3 の直後、履歴コミット直前に移動する。 - -**修正方針**: `worker::execute_tools` 内の truncate ループを Phase 3 の for ループ後に移動。それ以外の構造は変えない。 - ---- - -### 3. 🟢 `ToolOutputLimits` 型が manifest と llm-worker で二重定義 - -`manifest::ToolOutputLimits`（serde 付き）と `llm_worker::ToolOutputLimits`（serde なし）が同構造で、`pod::apply_worker_manifest` で手変換している。 - -manifest クレートが llm-worker に依存しない方針は正しく、他の manifest 型（`CompactionConfig` など）も Pod 境界で類似の変換を行っているため、**既存パターンと整合**している。 - -**判断**: 不問。現行のまま。 - ---- - -### 4. 🟢 極小 limit でのオーバーラン - -`truncate_content` は limit が suffix のリザーブ分（約 70 bytes）を下回ると、body_budget が 0 になっても suffix 自体が 50〜70 bytes あるため結果サイズが limit を超える。実運用で limit < 128 を設定するケースは無く、実害は無い。 - -**判断**: 不問。必要なら将来 `debug_assert!(limit >= 128)` を足す程度で十分。 - -## 結論 - -**指摘1・2の修正を条件に受け入れ可**。3・4 はそのまま。