diff --git a/TODO.md b/TODO.md index c587017e..47487142 100644 --- 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) diff --git a/tickets/exchange-turn-call-semantics.md b/tickets/exchange-turn-call-semantics.md index 4b79bd9c..c25871d0 100644 --- 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/` diff --git a/tickets/pod-inbound-pod-event-dedup.md b/tickets/pod-inbound-pod-event-dedup.md new file mode 100644 index 00000000..fe066c26 --- /dev/null +++ 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` または `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 は触らない）。 diff --git a/tickets/pod-input-validate-internalize.md b/tickets/pod-input-validate-internalize.md new file mode 100644 index 00000000..2e806607 --- /dev/null +++ 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 の経路に乗らない。 diff --git a/tickets/pod-interrupt-prep-internalize.md b/tickets/pod-interrupt-prep-internalize.md new file mode 100644 index 00000000..6c07ba65 --- /dev/null +++ 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)` / `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 議論。