pod-upstream-event完了
This commit is contained in:
parent
ed412cb6a8
commit
879858dc94
1
TODO.md
1
TODO.md
|
|
@ -6,7 +6,6 @@
|
|||
- [ ] Protocol の設計 → [tickets/protocol-design.md](tickets/protocol-design.md)
|
||||
- [ ] パーミッション: パターンベースのツール実行制御 → [tickets/permission-extension-point.md](tickets/permission-extension-point.md)
|
||||
- [ ] Pod オーケストレーション
|
||||
- [ ] Pod の上流イベント報告 (子 → 親) → [tickets/pod-upstream-events.md](tickets/pod-upstream-events.md)
|
||||
- [ ] 動的 Scope 変更 → [tickets/dynamic-scope.md](tickets/dynamic-scope.md)
|
||||
- [ ] ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md)
|
||||
- [ ] TUI 拡充
|
||||
|
|
|
|||
|
|
@ -1,165 +0,0 @@
|
|||
# Pod の上流イベント報告 (子 → 親)
|
||||
|
||||
レビュー中: [pod-upstream-events.review.md](pod-upstream-events.review.md)
|
||||
|
||||
## 背景
|
||||
|
||||
spawned Pod（子）のライフサイクルに親 Pod が反応する仕組みが必要。反応には 2 系統ある：
|
||||
|
||||
1. **system 処理**: `spawn_pods.json` 更新、`scope.lock` 整合、孫 Pod の把握
|
||||
2. **LLM 通知**: 親 LLM に「子でこれが起きた、次どうする？」を考えさせる
|
||||
|
||||
これを実現する通信 primitive を分離して定義する。既存の `Method::Notify`（LLM 向け自由テキスト注入）と混ぜない。
|
||||
|
||||
## 依存
|
||||
|
||||
- `tickets/spawn-pod-tool.md`: 済。callback address の受け渡しと spawn 記録
|
||||
- `tickets/scope-lock.md` 済に含まれる: allocation の delegate/reparent は既存
|
||||
- `Method::PodEvent` の追加は `protocol` crate の拡張
|
||||
|
||||
## 設計
|
||||
|
||||
### 新しい primitive
|
||||
|
||||
```rust
|
||||
pub enum Method {
|
||||
Run { input: String },
|
||||
Notify { message: String }, // 人間・tool → LLM 文脈、副作用なし（本チケットで source を削除）
|
||||
PodEvent(PodEvent), // 新: 子 → 親、typed なライフサイクル報告
|
||||
Resume,
|
||||
Cancel,
|
||||
Shutdown,
|
||||
GetHistory,
|
||||
}
|
||||
|
||||
pub enum PodEvent {
|
||||
/// 子が1ターン終えて IDLE になった
|
||||
TurnEnded { pod_name: String },
|
||||
|
||||
/// 子で Worker 実行エラーが発生した
|
||||
Errored { pod_name: String, message: String },
|
||||
|
||||
/// 子が停止した
|
||||
ShutDown { pod_name: String },
|
||||
|
||||
/// 子が孫 Pod に scope を又貸しした
|
||||
ScopeSubDelegated {
|
||||
parent_pod: String, // 又貸し元（= 子自身）
|
||||
sub_pod: String, // 孫 Pod の名前
|
||||
sub_socket: PathBuf, // 孫 Pod の socket path
|
||||
scope: Vec<ScopeRule>, // 又貸しされた scope（`protocol` crate に移動）
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
### `Method::Notify` の `source` 削除
|
||||
|
||||
現状 `Method::Notify { source: String, message: String }` の `source` フィールドを削除して `Method::Notify { message: String }` にする。`PodEvent` が typed な子 → 親報告を担うようになったことで、`Notify` は本来の「人間・tool が LLM 文脈に自由テキストを注入する」役割に純化する（発信者の識別は不要になる）。
|
||||
|
||||
影響範囲：
|
||||
- `protocol::Method::Notify` の定義変更、serde round-trip テストの更新
|
||||
- Controller main loop の `Method::Notify { source, message } => pod.push_notification(source, message)` を `pod.push_notification(message)` 形へ変更
|
||||
- `Pod::push_notification(source, message)` のシグネチャから `source` を落とす（呼び出し元を追って整理）
|
||||
- 既存テスト（`controller_test.rs` の Notify ケース、`pod_comm_tools_test.rs` など）の入力を新シグネチャに揃える
|
||||
|
||||
### `ScopeRule` / `Permission` の移動
|
||||
|
||||
`ScopeRule` と `Permission` は wire 型として `PodEvent::ScopeSubDelegated` で protocol を経由する必要があるため、現状の `manifest` crate から `protocol` crate へ移動する。`manifest` は `protocol::ScopeRule` を re-export するか、単に `protocol::ScopeRule` を直接参照する形に切り替える。移動により `protocol` → `manifest` の逆依存が発生しないようにする。
|
||||
|
||||
### 子（発信側）
|
||||
|
||||
子の Controller が以下のタイミングで親 socket に一発接続して `Method::PodEvent` を送信し、即切断する（fire-and-forget）。
|
||||
|
||||
| タイミング | variant |
|
||||
|---|---|
|
||||
| `RunEnd { result: Finished }` 発行時 | `TurnEnded` |
|
||||
| Worker 実行エラー発生時（後述） | `Errored` |
|
||||
| shutdown シーケンス（controller loop 終了直前） | `ShutDown` |
|
||||
| `SpawnPod` tool 成功直後 | `ScopeSubDelegated` |
|
||||
|
||||
`Errored` の対象は **`run_with_cancel_support` 内で Worker 実行が失敗した `Event::Error`** に限定する。`AlreadyRunning` / `NotPaused` / `NotRunning` のような method 拒否応答はクライアント向けの一時的なフィードバックであり、親に通知すべき子のライフサイクルイベントではない。実装時は発火点を worker エラー経路に絞ること。
|
||||
|
||||
送信は非同期 spawn で発射し、await しない。接続失敗はログのみで続行（親が落ちていても子は生きる）。
|
||||
|
||||
親 socket のアドレスは spawn 時に `--callback <PATH>` で受け取って保持している（既存）。
|
||||
|
||||
### 又貸しの親連鎖
|
||||
|
||||
`ScopeSubDelegated` は**直接の親にのみ送る**。曾孫が現れた場合：
|
||||
|
||||
1. 孫 (C) が曾孫 (D) を `SpawnPod` する
|
||||
2. C は自分の親 (B) に `ScopeSubDelegated { parent_pod: C, sub_pod: D, ... }` を送る
|
||||
3. B は D を自分の `spawned_pods.json` に追加し、さらに B の親 (A) へ `ScopeSubDelegated { parent_pod: B, sub_pod: D, ... }` を再発射する
|
||||
|
||||
これを再帰的に繰り返すことで、`scope.lock` の `delegated_from` チェーンと一致した形で root まで D の存在が登録される。各 Pod は「自分の直接の子＋紹介された孫（= 更に下の Pod も含む）」を把握し、孫より下の階層は子経由で管理される。
|
||||
|
||||
再発射は `ScopeSubDelegated` 受信時の system 処理の一部として行う（後述の表に反映）。
|
||||
|
||||
### 親（受信側）
|
||||
|
||||
親 Controller の main loop に `Method::PodEvent` ハンドラを追加：
|
||||
|
||||
```rust
|
||||
Method::PodEvent(event) => {
|
||||
// (1) system 処理 — variant ごとに固有
|
||||
apply_event_side_effects(&event, &spawned_registry, &lock_path).await;
|
||||
|
||||
// (2) LLM 通知 — variant を文字列にレンダリングしてバッファへ
|
||||
let text = render_event(&event);
|
||||
pod.push_notification(text);
|
||||
}
|
||||
```
|
||||
|
||||
variant 別の (1) の中身：
|
||||
|
||||
| variant | system 処理 |
|
||||
|---|---|
|
||||
| `TurnEnded` | なし |
|
||||
| `Errored` | なし（LLM に判断させる） |
|
||||
| `ShutDown` | `spawned_registry.remove(pod_name)`、scope lock を flock して該当 allocation を `release_pod` で解放 |
|
||||
| `ScopeSubDelegated` | `spawned_registry.add(SpawnedPodRecord { sub_pod, sub_socket, scope, ... })` で孫を追加。続けて自分の親（いれば）へ `ScopeSubDelegated { parent_pod: self, sub_pod, ... }` を再発射（親連鎖参照） |
|
||||
|
||||
(2) の `render_event` は一箇所に集約し、`format!("Pod `{pod_name}` finished a turn.")` のような短い human-readable 文字列を返す。
|
||||
|
||||
### Controller の配線変更
|
||||
|
||||
現状 `PodController::spawn` 内で `SpawnedPodRegistry` は tool 登録のためだけに生成され、main loop の `method_rx` 処理ループには渡っていない。`Method::PodEvent` ハンドラを追加するには：
|
||||
|
||||
- `spawned_registry: Arc<SpawnedPodRegistry>` を main loop に `clone` で持ち込む
|
||||
- `scope_lock::default_lock_path()` を event 処理時に呼ぶ（毎回 open するので保持は不要）
|
||||
- 親 callback socket（自分の親、トップ Pod では `None`）を `Pod::from_manifest_spawned` 経由で既に保持している値から再利用（再発射で使う）
|
||||
|
||||
`apply_event_side_effects` は Controller の main loop から呼ぶ非同期関数として定義する。
|
||||
|
||||
### 失敗時のフォールバック
|
||||
|
||||
- 子 → 親の PodEvent 送信が失敗しても諦める（再試行しない）
|
||||
- 親が再起動した場合や送信漏れた場合は、親の `ListPods` ツール（既存）による health check + `scope_lock::reclaim_stale` の stale 回収で不整合を解消する
|
||||
- これは「コールバックは最適化、ポーリングが真のフォールバック」という方針の継続
|
||||
|
||||
## 決定事項
|
||||
|
||||
- **順序保証は求めず、ハンドラを冪等・遅延到着に強くする**: fire-and-forget の unix socket 接続は順序を保証しない。`TurnEnded` 直後に `ShutDown` が届いても、逆順で到着しても親側で成立するように `apply_event_side_effects` を設計する。具体的には：
|
||||
- `ShutDown` 受信時、すでに registry から削除済みでもエラーにしない（`release_pod` の `UnknownPod` を swallow する既存挙動を踏襲）
|
||||
- `TurnEnded` が `ShutDown` より後に届いても、該当 Pod が既に registry にいなければ render だけして終える（LLM 向け通知は出る、system 処理は no-op）
|
||||
- `ScopeSubDelegated` で孫が既に registry にいたら上書きせず no-op（`DuplicatePodName` を swallow）
|
||||
- **`ScopeSubDelegated` の親連鎖は直接の親のみ + 再発射**: 上記「又貸しの親連鎖」セクション参照。曾孫以上は再帰的に再発射で root まで届く
|
||||
- **送信の接続タイムアウト**: `SpawnPod` / pod-comm-tools と揃えて 5 秒固定
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `Method::PodEvent(PodEvent)` が `protocol` crate に追加され、serde round-trip テストが通る
|
||||
- 子の Controller が 4 種の variant を適切なタイミングで親 socket に送信する。`Errored` は Worker 実行エラーにのみ限定されることを確認
|
||||
- 親の Controller が variant ごとの system 処理を実行し、レンダリングした文字列を LLM 通知バッファに流す
|
||||
- `ScopeSubDelegated` 受信後、孫 Pod が親の `spawned_pods.json` に現れ、親が更に上位親を持つ場合は上位へ再発射される
|
||||
- `ShutDown` 受信後、該当 Pod が親の registry から消え、scope lock からも解放される
|
||||
- イベント到着順が入れ替わっても副作用が安全（冪等）であることを単体テストで確認
|
||||
- 送信失敗しても子プロセスが続行する
|
||||
- 各 variant の送受信を検証する単体テスト
|
||||
|
||||
## 範囲外
|
||||
|
||||
- リモート親への送信（SSH 越し）。ローカル Unix socket のみ
|
||||
- 配信保証（at-least-once / exactly-once）
|
||||
- 親再起動時の「見逃したイベント」の再送。ポーリングで補う前提
|
||||
- 中間子が死亡した後の孫の upstream 通知路（孫 `callback_address` の再 attach / redirection）。本チケットは scope 管理の保全が主目的で、LLM 通知の連鎖保全は別課題
|
||||
|
|
@ -1,112 +0,0 @@
|
|||
# Review: pod-upstream-events
|
||||
|
||||
実装コミット `78a15bf`（pod-upstream-event実装）に対するレビュー。`cargo build` clean、`cargo test --workspace` 全 pass（366 + 8 の新規テスト含む）。
|
||||
|
||||
## 総評
|
||||
|
||||
チケット要件の中核（`Method::PodEvent` 新設、`Method::Notify` の `source` 削除、`ScopeRule`/`Permission` を protocol crate へ、子の 4 種 variant 発火、親の system 処理 + LLM 通知、`ScopeSubDelegated` 親連鎖の再発射、冪等ハンドラ）は達成。ただし**在 flight ターン中の `PodEvent` がロスする実装バグ**が 1 件あり、これは修正必須。
|
||||
|
||||
## 完了条件の対応
|
||||
|
||||
| 要件 | 状態 | 備考 |
|
||||
|---|---|---|
|
||||
| `Method::PodEvent(PodEvent)` + serde roundtrip | ✅ | 4 variant 全てのテストあり |
|
||||
| 子の 4 variant 発火 | ✅ | `TurnEnded` / `Errored` は `run_with_cancel_support` の Ok/Err、`ShutDown` は controller task 終了直前、`ScopeSubDelegated` は `spawn_pod.rs` の registry.add 後 |
|
||||
| `Errored` は worker 実行エラー限定 | ✅ | `Err(e)` アームのみ、method 拒否応答は発火せず |
|
||||
| 親の variant 別 system 処理 + LLM 通知 | ⚠️ | IDLE 時は動作、在 flight 中は **ロス**（後述） |
|
||||
| `ScopeSubDelegated` の再発射 | ✅ | `apply_event_side_effects` 内で `self_parent_socket` 経由で再帰 |
|
||||
| `ShutDown` 受信で registry + scope lock 解放 | ✅ | `release_scope_silently` が `UnknownPod` を swallow |
|
||||
| 冪等性（重複受信無害） | ✅ | `DuplicatePodName` swallow、missing ShutDown no-op |
|
||||
| 送信失敗で子は続行 | ✅ | fire-and-forget、tracing::warn のみ |
|
||||
| 単体テスト | ✅ | `pod_events_test.rs` 8 件、protocol の roundtrip 4 件 |
|
||||
|
||||
## 指摘と判断
|
||||
|
||||
### 修正必須
|
||||
|
||||
#### 1. 在 flight ターン中の `PodEvent` がロスする
|
||||
|
||||
**場所**: `crates/pod/src/controller.rs:579-588`
|
||||
|
||||
```rust
|
||||
Some(Method::PodEvent(_)) => {
|
||||
// PodEvent is handled in the main loop (next
|
||||
// iteration). Dropping it here is fine because
|
||||
// the sender is external and we will see it
|
||||
// again via `method_rx` after the current turn
|
||||
// ends — ...
|
||||
}
|
||||
```
|
||||
|
||||
**問題**: 親 Pod が RUNNING / Paused の間に子から `PodEvent` が到着すると、`run_with_cancel_support` 内の `select!` のこのアームが発火し、イベントを**単純に drop** する。コメントは「next iteration で再配送される」と言っているが、`method_rx.recv()` は mpsc の消費型で、一度 recv したメッセージは channel から消える。再配送機構はコード上存在しない。
|
||||
|
||||
- 結果: 親が長いターンを回している最中に子が `TurnEnded` / `Errored` / `ShutDown` / `ScopeSubDelegated` を送ると**ロス**する
|
||||
- チケットの方針は「配信保証なし、`ListPods` ポーリングが真のフォールバック」なので**動作としては成立**するが、ポーリング前に孫の `ScopeSubDelegated` が失われると、親が孫を把握しないまま子が死亡した場合に孫の scope 管理が宙に浮く懸念が残る（ただし `scope_lock::reclaim_stale` で delegated_from が辿られるので最終的には救済される）
|
||||
|
||||
**修正**: `run_with_cancel_support` 内で drop せず、`Method::Notify` と同様に notification_buffer に render 結果を push、加えて `apply_event_side_effects` を inline で呼ぶ。これには現状の `run_with_cancel_support` シグネチャに `spawned_registry` を追加する配線変更が必要：
|
||||
|
||||
```rust
|
||||
Some(Method::PodEvent(event)) => {
|
||||
apply_event_side_effects(&event, &spawned_registry, self_name, &parent_socket.cloned()).await;
|
||||
notification_buffer.push(render_event(&event));
|
||||
}
|
||||
```
|
||||
|
||||
コメントの「ordering guarantees that matter for PodEvent don't exist anyway」は部分的に正しい（順序は保証しない）が、「消失は許容」とは書いていないのでコメントの根拠にならない。少なくとも**コメントを「ロスを許容する。次ターンで `ListPods` で補う」と正直に書き換える**か、上記の inline 処理に変える。inline 処理を推奨。
|
||||
|
||||
### 要議論
|
||||
|
||||
#### 2. 孫の `callback_address` が祖父の registry で「死ぬ可能性のある子の socket」を指す
|
||||
|
||||
**場所**: `crates/pod/src/pod_events.rs:119-123`
|
||||
|
||||
```rust
|
||||
let callback_address = registry
|
||||
.get(parent_pod) // parent_pod = 送信者の子 (C)
|
||||
.await
|
||||
.map(|r| r.socket_path)
|
||||
.unwrap_or_else(PathBuf::new);
|
||||
```
|
||||
|
||||
B が C から `ScopeSubDelegated { parent_pod: C, sub_pod: D, ... }` を受信すると、B の registry に D を追加する際、D の `callback_address` を「C の socket」に設定する。これは意味的には正しい（D は C に報告する）。しかし C が死ぬと D の PodEvent 送り先が dead socket になり、D の上流報告が機能しなくなる。
|
||||
|
||||
- チケットの主張「親が子を経由せず孫を管理することで、子が死んでも孫の scope 管理が維持される」は *scope 管理* の話なので、B の registry に D が居ることだけで満たされる（OK）
|
||||
- ただし「D の LLM 通知は止まる」という副作用が残る
|
||||
- 将来 D を B に「再 attach」する仕組み（callback redirection）を作るときのフック点として、`callback_address` の更新 API を先回りで用意するか、今は割り切るか
|
||||
|
||||
**判断**: 本チケットは scope 管理の保全が主目的で LLM 通知の連鎖保全は範囲外なので、現状の割り切りで OK。ただしチケット or 範囲外セクションに「子が死んだ後の孫の upstream 通知路は別途」と一行書いておきたい。
|
||||
|
||||
#### 3. Controller task 終了時の `ShutDown` 送信レース
|
||||
|
||||
**場所**: `crates/pod/src/controller.rs:480-487`
|
||||
|
||||
```rust
|
||||
crate::pod_events::fire_and_forget(
|
||||
self_parent_socket.clone(),
|
||||
protocol::PodEvent::ShutDown { pod_name: spawner_name.clone() },
|
||||
);
|
||||
let _ = shutdown_tx.send(());
|
||||
```
|
||||
|
||||
`fire_and_forget` は `tokio::spawn` で送信を後回しにし、すぐに `shutdown_tx.send(())` で shutdown を通知。`main.rs` が `shutdown_rx` 受信後すぐ `ExitCode::SUCCESS` を返すと、spawn されたタスクは接続・送信を終える前にプロセス終了で殺される可能性がある。
|
||||
|
||||
**判断**: これは最後の一発なので `fire_and_forget` ではなく `send_pod_event(...).await` で inline 送信すべき（5 秒 timeout は `connect_and_send` 内で保証されている）。プロセス shutdown 時にのみ発生する特殊ケースなので、修正コストは小さい。
|
||||
|
||||
### 軽微
|
||||
|
||||
#### 4. `render_event` の `Errored` が message をそのまま埋め込む
|
||||
|
||||
Provider エラーメッセージは数 KB 級のこともあるので、LLM 通知に長大な文字列が入る可能性。実運用で問題になれば truncation を後付けで入れれば良い。今は放置で OK。
|
||||
|
||||
#### 5. コメントと実装の整合性（指摘 1 関連）
|
||||
|
||||
指摘 1 の修正に合わせて、`run_with_cancel_support` の doc comment にある「Transient method rejections such as `AlreadyRunning` are intentionally NOT reported as `Errored`」は正しく機能している（Err ブランチでのみ `fire_and_forget` が呼ばれる）。これは良い点で、維持したい。
|
||||
|
||||
## 完了に向けた作業
|
||||
|
||||
1. **指摘 1（必須）**: 在 flight 中の `Method::PodEvent` を inline 処理に変更
|
||||
2. **指摘 3（推奨）**: controller task 終了時の `ShutDown` を `await` 送信に
|
||||
3. **指摘 2（任意）**: 孫 callback の将来課題を範囲外セクションに一行追記
|
||||
4. 残りの軽微はそのままで OK
|
||||
|
||||
指摘 1 を直したら完了可。
|
||||
Loading…
Reference in New Issue
Block a user