diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index c5058832..fa40a558 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -900,28 +900,15 @@ async fn controller_loop( Method::ListCompletions { .. } => {} Method::PodEvent(event) => { - // For agent-visible PodEvents, live echo travels through the - // SystemItem lane: once the interceptor drains the notify buffer, - // the typed `SystemItem::PodEvent` lands as a - // `LogEntry::SystemItem` entry and the sink forwards it - // to clients as `Event::SystemItem`. Control-plane-only - // PodEvents use this same receive path only for side effects. - // - // (1) system side effects — idempotent and tolerant of - // out-of-order delivery (e.g. `TurnEnded` arriving - // after `ShutDown`). - crate::ipc::event::apply_event_side_effects( - &event, + if handle_inbound_pod_event( + event, &spawned_registry, &spawner_name, - &self_parent_socket, + self_parent_socket.as_ref(), + ¬ify_buffer, ) - .await; - // (2) agent-visible events enter the notification/history lane. - // Control-plane-only events (currently ScopeSubDelegated) - // stop after side effects so they do not wake or notify the LLM. - if event.should_notify_agent() { - pod.push_pod_event_notify(event); + .await + { // Auto-kick a turn if the Pod is idle so the // notification is not stranded. Matches the // `Method::Notify` idle path. @@ -961,6 +948,35 @@ async fn controller_loop( let _ = shutdown_tx.send(()); } +/// Apply an inbound child `PodEvent` exactly once. +/// +/// Side effects are control-plane state updates and upward propagation; they +/// run for every event. Only agent-visible events are staged on the notify +/// buffer. The caller owns lifecycle-dependent follow-up such as idle +/// `RunForNotification` auto-kick. +async fn handle_inbound_pod_event( + event: protocol::PodEvent, + spawned_registry: &Arc, + self_name: &str, + parent_socket: Option<&PathBuf>, + notify_buffer: &NotifyBuffer, +) -> bool { + let self_parent_socket = parent_socket.cloned(); + crate::ipc::event::apply_event_side_effects( + &event, + spawned_registry, + self_name, + &self_parent_socket, + ) + .await; + + let notify_agent = event.should_notify_agent(); + if notify_agent { + notify_buffer.push_pod_event(event); + } + notify_agent +} + /// Drives a Pod future (one in-flight turn) while concurrently /// processing incoming methods through an inner select! arm. Returns /// `(final_status, shutdown_requested)`. @@ -1095,23 +1111,17 @@ where // mpsc is consume-once, so we cannot defer this // to the next main-loop iteration — drop here // would lose the event entirely (children fire - // and forget). Apply the side effects inline - // and, for agent-visible variants, stage the typed - // event on the notification buffer so the in-flight - // turn's next `pending_history_appends` surfaces it - // as a typed `SystemItem::PodEvent`. Control-plane-only - // variants stop after side effects. - let self_parent_socket = parent_socket.cloned(); - crate::ipc::event::apply_event_side_effects( - &event, + // and forget). Auto-kick remains unnecessary here: + // the in-flight turn will drain agent-visible events + // from the notify buffer on its next history append. + handle_inbound_pod_event( + event, spawned_registry, self_name, - &self_parent_socket, + parent_socket, + notify_buffer, ) .await; - if event.should_notify_agent() { - notify_buffer.push_pod_event(event); - } } None => { let _ = cancel_tx.try_send(()); diff --git a/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/artifacts/.gitkeep b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/artifacts/.gitkeep similarity index 100% rename from work-items/open/20260527-000007-pod-inbound-pod-event-dedup/artifacts/.gitkeep rename to work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/artifacts/.gitkeep diff --git a/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/item.md b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/item.md similarity index 98% rename from work-items/open/20260527-000007-pod-inbound-pod-event-dedup/item.md rename to work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/item.md index c37ccd62..afcf8ba6 100644 --- a/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/item.md +++ b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/item.md @@ -2,12 +2,12 @@ id: 20260527-000007-pod-inbound-pod-event-dedup slug: pod-inbound-pod-event-dedup title: Inbound PodEvent ハンドリングの重複を統合する -status: open +status: closed kind: task priority: P2 labels: [migrated] created_at: 2026-05-27T00:00:07Z -updated_at: 2026-05-27T00:00:07Z +updated_at: 2026-05-30T05:37:00Z assignee: null legacy_ticket: tickets/pod-inbound-pod-event-dedup.md --- diff --git a/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/resolution.md b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/resolution.md new file mode 100644 index 00000000..afcf8ba6 --- /dev/null +++ b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/resolution.md @@ -0,0 +1,76 @@ +--- +id: 20260527-000007-pod-inbound-pod-event-dedup +slug: pod-inbound-pod-event-dedup +title: Inbound PodEvent ハンドリングの重複を統合する +status: closed +kind: task +priority: P2 +labels: [migrated] +created_at: 2026-05-27T00:00:07Z +updated_at: 2026-05-30T05:37:00Z +assignee: null +legacy_ticket: tickets/pod-inbound-pod-event-dedup.md +--- + +## Migration reference + +- legacy_ticket: tickets/pod-inbound-pod-event-dedup.md +- migrated_from: TODO.md / tickets directory migration on 2026-05-27 + +# 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/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/thread.md b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/thread.md new file mode 100644 index 00000000..9eeeffe2 --- /dev/null +++ b/work-items/closed/20260527-000007-pod-inbound-pod-event-dedup/thread.md @@ -0,0 +1,91 @@ + + +## Migrated + +Migrated from tickets/pod-inbound-pod-event-dedup.md. No legacy review file was present at migration time. + +--- + + + +## Closed + +--- +id: 20260527-000007-pod-inbound-pod-event-dedup +slug: pod-inbound-pod-event-dedup +title: Inbound PodEvent ハンドリングの重複を統合する +status: closed +kind: task +priority: P2 +labels: [migrated] +created_at: 2026-05-27T00:00:07Z +updated_at: 2026-05-30T05:37:00Z +assignee: null +legacy_ticket: tickets/pod-inbound-pod-event-dedup.md +--- + +## Migration reference + +- legacy_ticket: tickets/pod-inbound-pod-event-dedup.md +- migrated_from: TODO.md / tickets directory migration on 2026-05-27 + +# 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/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/thread.md b/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/thread.md deleted file mode 100644 index e21e2266..00000000 --- a/work-items/open/20260527-000007-pod-inbound-pod-event-dedup/thread.md +++ /dev/null @@ -1,7 +0,0 @@ - - -## Migrated - -Migrated from tickets/pod-inbound-pod-event-dedup.md. No legacy review file was present at migration time. - ----