refactor: deduplicate inbound pod events
This commit is contained in:
parent
45d2c67689
commit
205eb7bacb
|
|
@ -900,28 +900,15 @@ async fn controller_loop<C, St>(
|
|||
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<C, St>(
|
|||
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<SpawnedPodRegistry>,
|
||||
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(());
|
||||
|
|
|
|||
|
|
@ -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
|
||||
---
|
||||
|
|
@ -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<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 は触らない）。
|
||||
|
|
@ -0,0 +1,91 @@
|
|||
<!-- event: migration author: tickets.sh-migration at: 2026-05-27T00:00:07Z -->
|
||||
|
||||
## Migrated
|
||||
|
||||
Migrated from tickets/pod-inbound-pod-event-dedup.md. No legacy review file was present at migration time.
|
||||
|
||||
---
|
||||
|
||||
<!-- event: close author: hare at: 2026-05-30T05:37:00Z status: closed -->
|
||||
|
||||
## 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<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 は触らない）。
|
||||
|
||||
|
||||
---
|
||||
|
|
@ -1,7 +0,0 @@
|
|||
<!-- event: migration author: tickets.sh-migration at: 2026-05-27T00:00:07Z -->
|
||||
|
||||
## Migrated
|
||||
|
||||
Migrated from tickets/pod-inbound-pod-event-dedup.md. No legacy review file was present at migration time.
|
||||
|
||||
---
|
||||
Loading…
Reference in New Issue
Block a user