ticket: PodとControllerの責務の抱え違いを修正するチケット

2026-05-15 04:52:39 +09:00 · 2026-05-15 04:52:39 +09:00 · bb6f7e2022
commit bb6f7e2022
parent 86b48a9fdf
5 changed files with 173 additions and 17 deletions
--- a/TODO.md
+++ b/TODO.md
@ -6,6 +6,9 @@
 - Permission: allow-all 既定 policy への整理 → [tickets/permission-default-policy.md](tickets/permission-default-policy.md)
 - Pod: 空応答ターン (Submit 後 AI 応答ゼロで Pause/Cancel) を自動巻き戻し → [tickets/pod-empty-turn-rollback.md](tickets/pod-empty-turn-rollback.md)
 - Pod: 任意ターンからの Fork（複数ターン巻き戻しを汎用化） → [tickets/pod-session-fork.md](tickets/pod-session-fork.md)
+- Pod: Paused→Run の interrupt 前処理を Pod 内部に閉じる → [tickets/pod-interrupt-prep-internalize.md](tickets/pod-interrupt-prep-internalize.md)
+- Pod: Run 入力の事前 validate を Pod 内部に閉じる → [tickets/pod-input-validate-internalize.md](tickets/pod-input-validate-internalize.md)
+- Pod: Inbound PodEvent ハンドリングの重複を統合 → [tickets/pod-inbound-pod-event-dedup.md](tickets/pod-inbound-pod-event-dedup.md)
 - Pod: セッションログをバックエンドにした Pod 単位の永続化 → [tickets/pod-persistent-state.md](tickets/pod-persistent-state.md)
 - 永続化層のセマンティック整理 → [tickets/persistence-semantics.md](tickets/persistence-semantics.md)
 - Exchange / Turn / Call セマンティクス整理 → [tickets/exchange-turn-call-semantics.md](tickets/exchange-turn-call-semantics.md)
--- a/tickets/exchange-turn-call-semantics.md
+++ b/tickets/exchange-turn-call-semantics.md
@ -23,19 +23,19 @@
 構造としては以下を基本にする。

 ```text
-Thread
-  └─ Segment
-      └─ Exchange
-          ├─ UserTurn
-          ├─ AssistantTurn
-          │   └─ LlmCall
-          ├─ ToolTurn
-          │   └─ ToolExecution
-          ├─ AssistantTurn
-          │   └─ LlmCall
-          └─ ...
+Exchange
+  ├─ UserTurn
+  ├─ AssistantTurn
+  │   └─ LlmCall
+  ├─ ToolTurn
+  │   └─ ToolExecution
+  ├─ AssistantTurn
+  │   └─ LlmCall
+  └─ ...
 ```

+Exchange 列をまとめる外側の階層（会話の家系や永続化単位）は永続化層の責務であり、本チケットの範囲外。
+
 ## 要件

 - `Exchange` / `Turn` / `Call` / `Request` / `Run` の定義を、永続化セマンティクスおよび worker/protocol/UI の用語に反映できる状態にする。
@ -56,14 +56,12 @@ Thread
  - LLM Call ごとの表示を残す場合は `CallHeader` 相当に名称を変える。
 - `WorkerResult` / `RunResult` / `TurnResult` の責務境界。
  - Exchange の結果、actor Turn の結果、LlmCall の結果を混同しない。
- compaction / resume により Exchange が複数 Segment または複数 runtime attempt にまたがる場合の扱い。
- `persistence-semantics.md` の `Thread` / `Segment` / `Entry` / `Checkpoint` モデルへの反映。
+- compaction / resume により Exchange が複数の永続化単位 / runtime attempt にまたがる場合の扱い。
 - `Notify` / `PodEvent` / `SystemReminder` など user input ではない起点を Exchange と呼ぶことが適切か、または `ExchangeKind` のような分類で表すか。

 ## 完了条件

 - `Exchange` / `Turn` / `Call` / `Request` / `Run` の定義が文書化されている。
- `persistence-semantics.md` に反映すべき前提が整理されている。
 - worker / protocol / TUI / usage accounting で rename または互換 alias が必要な箇所が洗い出されている。
 - 実装変更を行う場合、外側 Exchange、actor Turn、内側 LlmCall の境界がコード上で判別できる。
 - 既存 API / event 名をすぐ壊さない段階移行方針が決まっている。
@ -77,9 +75,7 @@ Thread

 ## 関連

- `tickets/persistence-semantics.md`
- `tickets/pod-persistent-state.md`
- `tickets/pod-session-fork.md`
+- `tickets/persistence-semantics.md` — 外側の永続化単位を定義する別チケット。本チケットは論理単位 (Exchange / Turn / LlmCall) の定義に閉じる。
 - `crates/llm-worker/`
 - `crates/protocol/`
 - `crates/tui/`
--- a/tickets/pod-inbound-pod-event-dedup.md
+++ b/tickets/pod-inbound-pod-event-dedup.md
@ -0,0 +1,57 @@
+# Inbound PodEvent ハンドリングの重複を統合する
+
+## 背景
+
+子 Pod から `Method::PodEvent(event)` を受けたときの処理が `controller_loop` と `drive_turn` の 2 箇所にコピーされている。
+
+`controller.rs:693-720`（idle / paused 中）:
+
+```rust
+Method::PodEvent(event) => {
+    crate::ipc::event::apply_event_side_effects(
+        &event, &spawned_registry, &spawner_name, &self_parent_socket,
+    ).await;
+    pod.push_pod_event_notify(event);
+    if shared_state.get_status() == PodStatus::Idle {
+        pending = Some(PendingRun::RunForNotification);
+    }
+}
+```
+
+`controller.rs:861-879`（in-flight turn 中）:
+
+```rust
+Some(Method::PodEvent(event)) => {
+    let self_parent_socket = parent_socket.cloned();
+    crate::ipc::event::apply_event_side_effects(
+        &event, spawned_registry, self_name, &self_parent_socket,
+    ).await;
+    notify_buffer.push_pod_event(event);
+}
+```
+
+差分は 2 点:
+
+1. **buffer への push 経路**: `pod.push_pod_event_notify(event)` vs `notify_buffer.push_pod_event(event)`。両者は同じ `NotifyBuffer` を叩く（`pod.rs:845-846` は `self.pending_notifies.push_pod_event(event)` を呼ぶだけで、`notify_buffer_handle()` はその `pending_notifies.clone()` を返す）。**完全に等価**。
+2. **auto-kick**: idle 経路だけ `PendingRun::RunForNotification` を stage する。in-flight 経路は in-flight 自体が消化するので不要。
+
+つまり「event の処理本体」（side-effects + notify buffer への push）は同一で、後段の auto-kick だけが state-dependent な分岐。にもかかわらず関数化されておらず、片方をいじってもう片方を忘れると挙動が割れる。
+
+## 要件
+
+- side-effects 適用 + NotifyBuffer への typed push の流れを単一関数 `handle_inbound_pod_event` に切り出す。
+- `controller_loop` / `drive_turn` の両方からこのヘルパーを呼ぶ形に置き換える。
+- auto-kick (`PendingRun::RunForNotification` の stage) は呼び出し側の責務として残す。これは Pod のライフサイクル状態に依存した判断で、ヘルパー内には押し込めない。
+- 関数シグネチャは引数を最小化する。`event`、`spawned_registry`、`self_name: &str`、`self_parent_socket: &Option<PathBuf>` または `Option<&PathBuf>`、`notify_buffer: &NotifyBuffer` の 5 つで足りる前提。`Pod` への可変参照は不要（`notify_buffer` で代用可能）。
+- 動作変化なし。既存の `Method::PodEvent` 挙動（in-flight / idle 両方）が完全に同一で続行すること。
+
+## 完了条件
+
+- `controller.rs` 内に `apply_event_side_effects` 呼び出しが 1 箇所だけ残り、`controller_loop` と `drive_turn` の `Method::PodEvent` アームはどちらも `handle_inbound_pod_event(...)` 呼び出し + idle 経路のみ auto-kick stage、という形になる。
+- 既存の inbound PodEvent 関連テスト（特に `apply_event_side_effects` の idempotency や `notify_buffer` への typed push）が通る。
+
+## 範囲外
+
+- `apply_event_side_effects` 自体の中身変更。
+- `NotifyBuffer` API のリネーム / 統合。
+- `pod.push_pod_event_notify` の削除（[[pod-interrupt-prep-internalize]] と同じく将来の整理対象だが、本チケットでは外向き API は触らない）。
--- a/tickets/pod-input-validate-internalize.md
+++ b/tickets/pod-input-validate-internalize.md
@ -0,0 +1,50 @@
+# Run 入力の事前 validate を Pod 内部に閉じる
+
+## 背景
+
+`controller_loop` は `Method::Run { input }` を受けた直後、`drive_turn` に流す前に `pod.validate_workflow_invocations(&input)` を呼んで invalid なら `ErrorCode::InvalidRequest` を返している (`crates/pod/src/controller.rs:613`):
+
+```rust
+if let Err(e) = pod.validate_workflow_invocations(&input) {
+    let _ = event_tx.send(Event::Error {
+        code: ErrorCode::InvalidRequest,
+        message: e.to_string(),
+    });
+    continue;
+}
+```
+
+これは Pod 内部の `workflow_registry` を覗いてバリデーション判定する責務漏れになっている。`pod.validate_workflow_invocations` は read-only 関数で、`Pod::run` の冒頭でも当然呼べる。
+
+なぜ controller が肩代わりしているかというと、`Pod::run` (`crates/pod/src/pod.rs:1138`) は冒頭で:
+
+```rust
+self.commit_entry(LogEntry::UserInput { ts: ..., segments: input.clone() })?;
+self.user_segments.push(input.clone());
+// ...
+attachments.extend(self.resolve_workflow_invocations(&input)?);
+```
+
+の順で動く。`resolve_workflow_invocations` が失敗を返すころには既に UserInput が session log に commit されてしまっており、「invalid な input が history に残る」状態になる。それを避けるために controller 側で先打ち validate している。
+
+[[pod-interrupt-prep-internalize]] と同じパターン: Pod 内部の整合性を保つ前処理が Controller 層に染み出している。
+
+## 要件
+
+- `Pod::run` の冒頭、`commit_entry(LogEntry::UserInput { .. })` よりも前に `validate_workflow_invocations` を呼び、エラー時は session log に何も commit せず `PodError` で early return する。
+- `controller_loop` の `Method::Run` ハンドリングから事前 validate 呼び出しを削除する。
+- `Pod::run` から返る `PodError`（新規に追加する workflow validation 失敗のバリアントを含む）を `worker_error_code` 等の controller 側エラーコードマッピングで `ErrorCode::InvalidRequest` にマップする。
+  - `PodError` に既存の InvalidRequest 相当バリアントがあればそれを使う。無ければ `WorkflowResolveError` をラップする最小限のバリアントを追加する。
+- `Pod::validate_workflow_invocations` メソッドの可視性は `pub` のままでよい（IPC `ListCompletions` 経路や他テストから参照される可能性があるので外向き API を狭めない）。
+
+## 完了条件
+
+- `controller.rs` の `Method::Run` ブランチに `pod.validate_workflow_invocations` の呼び出しが残っていない。
+- 不正な workflow slug を含む `Method::Run` を投げると、UserInput が session log に commit されないまま `Event::Error { code: InvalidRequest, .. }` が flow する（既存の挙動と同等）。
+- 既存の workflow invocation 関連テスト（成功 / NotFound / NotUserInvocable / InvalidSlug）が通る。
+
+## 範囲外
+
+- `resolve_workflow_invocations` 側のロジック変更。
+- `validate_workflow_invocations` の判定基準変更（user_invocable / slug parse 等）。
+- `Method::Run` 以外の経路（Resume / RunForNotification）からの入力検証。Resume は入力を取らず、RunForNotification の入力は notify buffer drain で system message として入るため workflow invocation の経路に乗らない。
--- a/tickets/pod-interrupt-prep-internalize.md
+++ b/tickets/pod-interrupt-prep-internalize.md
@ -0,0 +1,50 @@
+# Paused→Run の interrupt 前処理を Pod 内部に閉じる
+
+## 背景
+
+`controller_loop` は `Method::Run { input }` を受けたとき、Pod の status を見て次のように 2 分岐している (`crates/pod/src/controller.rs:629`):
+
+```rust
+pending = Some(if status_before == PodStatus::Paused {
+    PendingRun::InterruptAndRun(input)
+} else {
+    PendingRun::Run(input)
+});
+```
+
+そして `drive_turn` 側でも `pod.run(input)` と `pod.interrupt_and_run(input)` を呼び分けている。
+
+`pod.interrupt_and_run`（`crates/pod/src/interrupt_and_run.rs`）の中身は:
+
+1. worker history に残る orphan な `Item::ToolCall`（前 turn が中断されて対応する `ToolResult` が未発行）に synthetic `ToolResult` を被せる（Anthropic 等の wire-validity 要件）
+2. 「直前の作業は中断された」旨の system note を `worker` に push
+3. 最後に `self.run(input)` に flow
+
+つまり**入口の前処理が違うだけで、出口は通常の `pod.run` に合流する**。
+
+しかもこの前処理が必要かどうかは、worker が一次情報として持っている `last_run_interrupted` フラグで判定できる（`pod.rs:1808` で既に使用済み）。`PodStatus::Paused` は controller がそれを観測したミラーに過ぎない。
+
+結果として、Controller 層が Pod の内部状態を覗いて分岐するという責務漏れになっている。`PendingRun` enum は `Method::Run` 経由のものが 2 variant に分裂し、本来「parent originated か否か」など Run の意味論を扱うはずの enum に Pod 内部都合の分岐が混ざる。
+
+## 要件
+
+- `PendingRun::InterruptAndRun(input)` バリアントを削除し、`PendingRun::Run(input)` 一本に統合する。
+- `controller_loop` の `Method::Run` ハンドリングは status を見ずに `PendingRun::Run(input)` を stage するだけにする。
+- interrupt 前処理（orphan tool_result の closure + system note 挿入）は `Pod::run` の入口で `self.worker().last_run_interrupted()` を見て自発的に行う。
+- `Pod::interrupt_and_run` メソッドは削除する。`interrupt_and_run.rs` のロジック（`orphan_tool_result_closures` 等）は `Pod::run` の前段として `pod.rs` 側、または同モジュールから `Pod::run` が直接呼ぶ形に再配置する。
+- `Method::Resume`（`PendingRun::Resume`）の経路には影響させない。Resume は前 turn の context を生かして続行する別経路で、interrupt 前処理を入れてはいけない。
+- 動作上の変化はゼロ（リファクタ）。Paused 中に `Method::Run { input }` が来たときの挙動は今と完全に同じであること。
+
+## 完了条件
+
+- `PendingRun` は `Run(Vec<Segment>)` / `RunForNotification` / `Resume` の 3 variant のみ。
+- `controller_loop` の `Method::Run` ブランチに `status_before == PodStatus::Paused` 分岐が残っていない。
+- `Pod::run` を Paused 直後（`last_run_interrupted == true`）の状態で呼ぶと、orphan `ToolCall` の closure と interrupt system note が history に積まれてから新規 input が処理される。これを既存の `interrupt_and_run` のテスト相当でカバーする。
+- `Pod::run` を非 Paused 状態で呼ぶと従来通り interrupt 前処理は走らない。
+- 既存の Paused 関連 E2E / 結合テストが全て通る。
+
+## 範囲外
+
+- `[[pod-parent-turn-callback]]` の `is_parent_originated` 判定ロジック自体の変更（本チケット完了後は `Run` / `Resume` の 2 variant が true になる形に自然に整理されるが、本チケットの目的ではない）。
+- `worker.last_run_interrupted` のセマンティクス変更。
+- TUI 側で Paused 中の送信を `Run` / `Resume` どちらにマップするかの UX 議論。