Compare commits
2 Commits
6e5b1482e6
...
456af3167b
| Author | SHA1 | Date | |
|---|---|---|---|
| 456af3167b | |||
| 8019d0d77c |
|
|
@ -99,6 +99,21 @@ enum PendingRun {
|
||||||
Resume,
|
Resume,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl PendingRun {
|
||||||
|
/// Whether this turn was kicked off by the parent (via `Method::Run`
|
||||||
|
/// or `Method::Resume`). Used by [`drive_turn`] to gate upward
|
||||||
|
/// `PodEvent::TurnEnded` / `PodEvent::Errored` reports so the parent
|
||||||
|
/// only sees completion signals for work it actually delegated.
|
||||||
|
/// `RunForNotification` covers self-initiated turns kicked from the
|
||||||
|
/// notify buffer (Notify / inbound PodEvent) and stays silent.
|
||||||
|
fn is_parent_originated(&self) -> bool {
|
||||||
|
match self {
|
||||||
|
PendingRun::Run(_) | PendingRun::InterruptAndRun(_) | PendingRun::Resume => true,
|
||||||
|
PendingRun::RunForNotification => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// PodController — actor that owns a Pod
|
// PodController — actor that owns a Pod
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
@ -504,6 +519,7 @@ async fn controller_loop<C, St>(
|
||||||
// in one place, regardless of which Method caused it.
|
// in one place, regardless of which Method caused it.
|
||||||
if let Some(run) = pending.take() {
|
if let Some(run) = pending.take() {
|
||||||
set_controller_status(&shared_state, &runtime_dir, &event_tx, PodStatus::Running).await;
|
set_controller_status(&shared_state, &runtime_dir, &event_tx, PodStatus::Running).await;
|
||||||
|
let parent_originated = run.is_parent_originated();
|
||||||
let (new_status, shutdown) = match run {
|
let (new_status, shutdown) = match run {
|
||||||
PendingRun::Run(input) => {
|
PendingRun::Run(input) => {
|
||||||
drive_turn(
|
drive_turn(
|
||||||
|
|
@ -516,6 +532,7 @@ async fn controller_loop<C, St>(
|
||||||
self_parent_socket.as_ref(),
|
self_parent_socket.as_ref(),
|
||||||
&spawner_name,
|
&spawner_name,
|
||||||
&spawned_registry,
|
&spawned_registry,
|
||||||
|
parent_originated,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
@ -530,6 +547,7 @@ async fn controller_loop<C, St>(
|
||||||
self_parent_socket.as_ref(),
|
self_parent_socket.as_ref(),
|
||||||
&spawner_name,
|
&spawner_name,
|
||||||
&spawned_registry,
|
&spawned_registry,
|
||||||
|
parent_originated,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
@ -544,6 +562,7 @@ async fn controller_loop<C, St>(
|
||||||
self_parent_socket.as_ref(),
|
self_parent_socket.as_ref(),
|
||||||
&spawner_name,
|
&spawner_name,
|
||||||
&spawned_registry,
|
&spawned_registry,
|
||||||
|
parent_originated,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
@ -558,6 +577,7 @@ async fn controller_loop<C, St>(
|
||||||
self_parent_socket.as_ref(),
|
self_parent_socket.as_ref(),
|
||||||
&spawner_name,
|
&spawner_name,
|
||||||
&spawned_registry,
|
&spawned_registry,
|
||||||
|
parent_originated,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
@ -736,6 +756,12 @@ async fn controller_loop<C, St>(
|
||||||
/// `None` parent skips the send (top-level Pod). Transient method
|
/// `None` parent skips the send (top-level Pod). Transient method
|
||||||
/// rejections such as `AlreadyRunning` are intentionally NOT reported
|
/// rejections such as `AlreadyRunning` are intentionally NOT reported
|
||||||
/// as `Errored` — only the worker-execution `Err` branch below fires.
|
/// as `Errored` — only the worker-execution `Err` branch below fires.
|
||||||
|
///
|
||||||
|
/// `parent_originated` further restricts both upward reports to turns
|
||||||
|
/// the parent actually delegated (`Method::Run` / `Method::Resume`).
|
||||||
|
/// `Method::Notify` / inbound `PodEvent` auto-kicks complete silently
|
||||||
|
/// so the parent's history does not get flooded with child-internal
|
||||||
|
/// turn boundaries.
|
||||||
#[allow(clippy::too_many_arguments)]
|
#[allow(clippy::too_many_arguments)]
|
||||||
async fn drive_turn<F>(
|
async fn drive_turn<F>(
|
||||||
pod_future: F,
|
pod_future: F,
|
||||||
|
|
@ -747,6 +773,7 @@ async fn drive_turn<F>(
|
||||||
parent_socket: Option<&PathBuf>,
|
parent_socket: Option<&PathBuf>,
|
||||||
self_name: &str,
|
self_name: &str,
|
||||||
spawned_registry: &Arc<SpawnedPodRegistry>,
|
spawned_registry: &Arc<SpawnedPodRegistry>,
|
||||||
|
parent_originated: bool,
|
||||||
) -> (PodStatus, bool)
|
) -> (PodStatus, bool)
|
||||||
where
|
where
|
||||||
F: std::future::Future<Output = Result<PodRunResult, PodError>>,
|
F: std::future::Future<Output = Result<PodRunResult, PodError>>,
|
||||||
|
|
@ -766,7 +793,7 @@ where
|
||||||
PodRunResult::LimitReached => (PodStatus::Idle, RunResult::LimitReached),
|
PodRunResult::LimitReached => (PodStatus::Idle, RunResult::LimitReached),
|
||||||
};
|
};
|
||||||
let _ = event_tx.send(Event::RunEnd { result: run_result });
|
let _ = event_tx.send(Event::RunEnd { result: run_result });
|
||||||
if matches!(run_result, RunResult::Finished) {
|
if parent_originated && matches!(run_result, RunResult::Finished) {
|
||||||
crate::ipc::event::fire_and_forget(
|
crate::ipc::event::fire_and_forget(
|
||||||
parent_socket.cloned(),
|
parent_socket.cloned(),
|
||||||
protocol::PodEvent::TurnEnded {
|
protocol::PodEvent::TurnEnded {
|
||||||
|
|
@ -792,13 +819,15 @@ where
|
||||||
code,
|
code,
|
||||||
message: message.clone(),
|
message: message.clone(),
|
||||||
});
|
});
|
||||||
crate::ipc::event::fire_and_forget(
|
if parent_originated {
|
||||||
parent_socket.cloned(),
|
crate::ipc::event::fire_and_forget(
|
||||||
protocol::PodEvent::Errored {
|
parent_socket.cloned(),
|
||||||
pod_name: self_name.to_string(),
|
protocol::PodEvent::Errored {
|
||||||
message,
|
pod_name: self_name.to_string(),
|
||||||
},
|
message,
|
||||||
);
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
(PodStatus::Idle, shutdown_requested)
|
(PodStatus::Idle, shutdown_requested)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
@ -919,3 +948,222 @@ fn worker_error_code(e: &PodError) -> ErrorCode {
|
||||||
_ => ErrorCode::Internal,
|
_ => ErrorCode::Internal,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use protocol::PodEvent;
|
||||||
|
use protocol::stream::JsonLineReader;
|
||||||
|
use std::time::Duration;
|
||||||
|
use tempfile::TempDir;
|
||||||
|
use tokio::net::UnixListener;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn pending_run_parent_origin_table() {
|
||||||
|
assert!(PendingRun::Run(Vec::new()).is_parent_originated());
|
||||||
|
assert!(PendingRun::InterruptAndRun(Vec::new()).is_parent_originated());
|
||||||
|
assert!(PendingRun::Resume.is_parent_originated());
|
||||||
|
assert!(!PendingRun::RunForNotification.is_parent_originated());
|
||||||
|
}
|
||||||
|
|
||||||
|
struct DriveTurnEnv {
|
||||||
|
// Held to keep the channel alive; without this `method_rx.recv()`
|
||||||
|
// would observe channel-closed and confuse the select! arm.
|
||||||
|
_method_tx: mpsc::Sender<Method>,
|
||||||
|
method_rx: mpsc::Receiver<Method>,
|
||||||
|
event_tx: broadcast::Sender<Event>,
|
||||||
|
cancel_tx: mpsc::Sender<()>,
|
||||||
|
_cancel_rx: mpsc::Receiver<()>,
|
||||||
|
shared_state: Arc<PodSharedState>,
|
||||||
|
notify_buffer: NotifyBuffer,
|
||||||
|
spawned_registry: Arc<SpawnedPodRegistry>,
|
||||||
|
parent_socket_path: PathBuf,
|
||||||
|
_runtime_dir: Arc<RuntimeDir>,
|
||||||
|
_temp: TempDir,
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn make_env() -> DriveTurnEnv {
|
||||||
|
let temp = tempfile::tempdir().expect("tempdir");
|
||||||
|
let runtime_dir = Arc::new(
|
||||||
|
RuntimeDir::create(temp.path(), "child-pod")
|
||||||
|
.await
|
||||||
|
.expect("runtime dir create"),
|
||||||
|
);
|
||||||
|
let (method_tx, method_rx) = mpsc::channel::<Method>(16);
|
||||||
|
let (event_tx, _) = broadcast::channel::<Event>(16);
|
||||||
|
let (cancel_tx, cancel_rx) = mpsc::channel::<()>(1);
|
||||||
|
let shared_state = Arc::new(PodSharedState::new(
|
||||||
|
"child-pod".to_string(),
|
||||||
|
session_store::new_session_id(),
|
||||||
|
String::new(),
|
||||||
|
protocol::Greeting {
|
||||||
|
pod_name: "child-pod".to_string(),
|
||||||
|
cwd: String::new(),
|
||||||
|
provider: String::new(),
|
||||||
|
model: String::new(),
|
||||||
|
scope_summary: String::new(),
|
||||||
|
tools: Vec::new(),
|
||||||
|
},
|
||||||
|
));
|
||||||
|
let notify_buffer = NotifyBuffer::new();
|
||||||
|
let spawned_registry = SpawnedPodRegistry::new(runtime_dir.clone());
|
||||||
|
let parent_socket_path = temp.path().join("parent.sock");
|
||||||
|
|
||||||
|
DriveTurnEnv {
|
||||||
|
_method_tx: method_tx,
|
||||||
|
method_rx,
|
||||||
|
event_tx,
|
||||||
|
cancel_tx,
|
||||||
|
_cancel_rx: cancel_rx,
|
||||||
|
shared_state,
|
||||||
|
notify_buffer,
|
||||||
|
spawned_registry,
|
||||||
|
parent_socket_path,
|
||||||
|
_runtime_dir: runtime_dir,
|
||||||
|
_temp: temp,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Listen on a bound UnixListener for one inbound connection and
|
||||||
|
/// return the first `Method::PodEvent` read from it. Returns `None`
|
||||||
|
/// on timeout / EOF / non-PodEvent.
|
||||||
|
async fn recv_pod_event(listener: UnixListener, timeout: Duration) -> Option<PodEvent> {
|
||||||
|
let accept = async {
|
||||||
|
let (stream, _) = listener.accept().await.ok()?;
|
||||||
|
let mut reader = JsonLineReader::new(stream);
|
||||||
|
match reader.next::<Method>().await {
|
||||||
|
Ok(Some(Method::PodEvent(e))) => Some(e),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
tokio::time::timeout(timeout, accept).await.ok().flatten()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn parent_originated_finished_fires_turn_ended() {
|
||||||
|
let mut env = make_env().await;
|
||||||
|
let listener = UnixListener::bind(&env.parent_socket_path).expect("bind listener");
|
||||||
|
let recv = tokio::spawn(recv_pod_event(listener, Duration::from_secs(2)));
|
||||||
|
|
||||||
|
let pod_future = async { Ok::<_, PodError>(PodRunResult::Finished) };
|
||||||
|
let (status, shutdown) = drive_turn(
|
||||||
|
pod_future,
|
||||||
|
&mut env.method_rx,
|
||||||
|
&env.event_tx,
|
||||||
|
&env.cancel_tx,
|
||||||
|
&env.shared_state,
|
||||||
|
&env.notify_buffer,
|
||||||
|
Some(&env.parent_socket_path),
|
||||||
|
"child-pod",
|
||||||
|
&env.spawned_registry,
|
||||||
|
true,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_eq!(status, PodStatus::Idle);
|
||||||
|
assert!(!shutdown);
|
||||||
|
|
||||||
|
let event = recv.await.expect("recv task").expect("PodEvent received");
|
||||||
|
match event {
|
||||||
|
PodEvent::TurnEnded { pod_name } => assert_eq!(pod_name, "child-pod"),
|
||||||
|
other => panic!("expected TurnEnded, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn non_parent_originated_finished_stays_silent() {
|
||||||
|
let mut env = make_env().await;
|
||||||
|
let listener = UnixListener::bind(&env.parent_socket_path).expect("bind listener");
|
||||||
|
|
||||||
|
let pod_future = async { Ok::<_, PodError>(PodRunResult::Finished) };
|
||||||
|
let (status, _) = drive_turn(
|
||||||
|
pod_future,
|
||||||
|
&mut env.method_rx,
|
||||||
|
&env.event_tx,
|
||||||
|
&env.cancel_tx,
|
||||||
|
&env.shared_state,
|
||||||
|
&env.notify_buffer,
|
||||||
|
Some(&env.parent_socket_path),
|
||||||
|
"child-pod",
|
||||||
|
&env.spawned_registry,
|
||||||
|
false,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_eq!(status, PodStatus::Idle);
|
||||||
|
|
||||||
|
// Wait long enough for any (incorrect) fire-and-forget send to
|
||||||
|
// land; expect the accept to time out.
|
||||||
|
let accept = tokio::time::timeout(Duration::from_millis(200), listener.accept()).await;
|
||||||
|
assert!(
|
||||||
|
accept.is_err(),
|
||||||
|
"expected no PodEvent for non-parent-originated turn"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn parent_originated_worker_error_fires_errored() {
|
||||||
|
let mut env = make_env().await;
|
||||||
|
let listener = UnixListener::bind(&env.parent_socket_path).expect("bind listener");
|
||||||
|
let recv = tokio::spawn(recv_pod_event(listener, Duration::from_secs(2)));
|
||||||
|
|
||||||
|
let pod_future = async {
|
||||||
|
Err::<PodRunResult, _>(PodError::Worker(WorkerError::Aborted(
|
||||||
|
"boom from test".into(),
|
||||||
|
)))
|
||||||
|
};
|
||||||
|
let (status, _) = drive_turn(
|
||||||
|
pod_future,
|
||||||
|
&mut env.method_rx,
|
||||||
|
&env.event_tx,
|
||||||
|
&env.cancel_tx,
|
||||||
|
&env.shared_state,
|
||||||
|
&env.notify_buffer,
|
||||||
|
Some(&env.parent_socket_path),
|
||||||
|
"child-pod",
|
||||||
|
&env.spawned_registry,
|
||||||
|
true,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_eq!(status, PodStatus::Idle);
|
||||||
|
|
||||||
|
let event = recv.await.expect("recv task").expect("PodEvent received");
|
||||||
|
match event {
|
||||||
|
PodEvent::Errored { pod_name, message } => {
|
||||||
|
assert_eq!(pod_name, "child-pod");
|
||||||
|
assert!(message.contains("boom from test"), "got message: {message}");
|
||||||
|
}
|
||||||
|
other => panic!("expected Errored, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn non_parent_originated_worker_error_stays_silent() {
|
||||||
|
let mut env = make_env().await;
|
||||||
|
let listener = UnixListener::bind(&env.parent_socket_path).expect("bind listener");
|
||||||
|
|
||||||
|
let pod_future = async {
|
||||||
|
Err::<PodRunResult, _>(PodError::Worker(WorkerError::Aborted(
|
||||||
|
"boom from notify".into(),
|
||||||
|
)))
|
||||||
|
};
|
||||||
|
let (status, _) = drive_turn(
|
||||||
|
pod_future,
|
||||||
|
&mut env.method_rx,
|
||||||
|
&env.event_tx,
|
||||||
|
&env.cancel_tx,
|
||||||
|
&env.shared_state,
|
||||||
|
&env.notify_buffer,
|
||||||
|
Some(&env.parent_socket_path),
|
||||||
|
"child-pod",
|
||||||
|
&env.spawned_registry,
|
||||||
|
false,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_eq!(status, PodStatus::Idle);
|
||||||
|
|
||||||
|
let accept = tokio::time::timeout(Duration::from_millis(200), listener.accept()).await;
|
||||||
|
assert!(
|
||||||
|
accept.is_err(),
|
||||||
|
"expected no PodEvent for notification-originated worker error"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -2,12 +2,15 @@
|
||||||
|
|
||||||
## 背景
|
## 背景
|
||||||
|
|
||||||
子 Pod は `controller.rs` の `run_with_cancel_support` で turn 終了時に `PodEvent::TurnEnded` / `PodEvent::Errored` を `parent_socket` 宛に fire-and-forget している。現状この発火条件は「`pod_future` が `Finished` / 失敗で抜けたら」であり、その turn が何起点かを区別していない。
|
子 Pod は `controller.rs` の `drive_turn` で turn 終了時に `PodEvent::TurnEnded` / `PodEvent::Errored` を `parent_socket` 宛に fire-and-forget している。現状この発火条件は「`pod_future` が `Finished` / 失敗で抜けたら」であり、その turn が何起点かを区別していない。
|
||||||
|
|
||||||
|
`controller_loop` は次の turn を `PendingRun` enum (`Run` / `InterruptAndRun` / `RunForNotification` / `Resume`) として stage してから `drive_turn` を呼ぶ構造になっており、ここに turn の起源情報がすでに揃っている。
|
||||||
|
|
||||||
子 Pod のターンは複数の経路で開始しうる:
|
子 Pod のターンは複数の経路で開始しうる:
|
||||||
|
|
||||||
- 親からの `Method::Run`(`SpawnPod` の初回起動 / `SendToPod`)
|
- 親からの `Method::Run`(`SpawnPod` の初回起動 / `SendToPod`)。Pod が Paused 中なら `PendingRun::InterruptAndRun`、それ以外は `PendingRun::Run` に分岐するが、どちらも親由来。
|
||||||
- 子内部での `Method::Notify` 起点の auto-kick(`run_for_notification`)。Notify の出どころは孫 Pod からの `PodEvent`、system reminder、外部 Notify など多岐にわたる
|
- 子内部での `Method::Notify` または `Method::PodEvent` 起点の idle auto-kick(`PendingRun::RunForNotification` → `pod.run_for_notification()`)。Notify / PodEvent の出どころは孫 Pod の `PodEvent`、system reminder、外部 Notify など多岐にわたる。
|
||||||
|
- 親からの `Method::Resume`(`PendingRun::Resume`)
|
||||||
- 将来的に増える可能性のある自走経路
|
- 将来的に増える可能性のある自走経路
|
||||||
|
|
||||||
入れ子 Pod を本格的に使い始めると、子の Notify 起点 turn が頻発する。これらが完了するたびに親へ `TurnEnded` が飛ぶと、親の `NotifyBuffer` には「自分が投げていないターンの完了」が積まれ、`pending_history_appends` で history に system message として commit され、親まで auto-kick される。親の history は「自分が把握していない子の内部活動」のノイズで埋まり、auto-kick の連鎖も意味的に正当化しづらい。
|
入れ子 Pod を本格的に使い始めると、子の Notify 起点 turn が頻発する。これらが完了するたびに親へ `TurnEnded` が飛ぶと、親の `NotifyBuffer` には「自分が投げていないターンの完了」が積まれ、`pending_history_appends` で history に system message として commit され、親まで auto-kick される。親の history は「自分が把握していない子の内部活動」のノイズで埋まり、auto-kick の連鎖も意味的に正当化しづらい。
|
||||||
|
|
@ -16,11 +19,12 @@
|
||||||
|
|
||||||
## 要件
|
## 要件
|
||||||
|
|
||||||
- 子 Pod が parent へ送る `PodEvent::TurnEnded` は、親由来の `Method::Run` を起点とする turn が `Finished` で完了した場合に限る。
|
- 子 Pod が parent へ送る `PodEvent::TurnEnded` は、親由来 turn が `Finished` で完了した場合に限る。
|
||||||
- 「親由来」の判定は「`Method::Run` で開始された turn」とする。SpawnPod 初回起動 / SendToPod はどちらも `Method::Run` 経由なので両方対象になる。
|
- 「親由来」の判定は `PendingRun` のバリアントベースとする。`PendingRun::Run` / `PendingRun::InterruptAndRun` / `PendingRun::Resume` の3つを親由来として扱う。
|
||||||
- `run_for_notification` 起点の turn は完了しても `TurnEnded` を上げない。
|
- `Run` / `InterruptAndRun`: いずれも `Method::Run` 経由(SpawnPod 初回起動 / SendToPod。Paused 中なら `InterruptAndRun` に分岐するが、起点は親の `Method::Run`)。
|
||||||
- `Method::Resume` 起点の turn は親由来として扱う(親が再開を指示した turn のため)。
|
- `Resume`: 親の `Method::Resume` 指示で再開した turn のため親由来扱い。
|
||||||
- `PodEvent::Errored` も同じスコープに揃える。Notify 起点 turn の worker 失敗は `parent_socket` への報告対象外とする(親が知らない指示の失敗報告になるため)。`Event::Error` でローカルに通知される経路は維持。
|
- `PendingRun::RunForNotification` 起点の turn は完了しても `TurnEnded` を上げない。`Method::Notify` 経由でも `Method::PodEvent` 経由でも同様。
|
||||||
|
- `PodEvent::Errored` も同じスコープに揃える。`RunForNotification` 起点 turn の worker 失敗は `parent_socket` への報告対象外とする(親が知らない指示の失敗報告になるため)。`Event::Error` でローカルに通知される経路は維持。
|
||||||
- `PodEvent::ShutDown` は turn とは独立した Pod プロセス終了通知なので、本チケットの対象外(従来通り常に発火)。
|
- `PodEvent::ShutDown` は turn とは独立した Pod プロセス終了通知なので、本チケットの対象外(従来通り常に発火)。
|
||||||
- `ScopeSubDelegated` も turn とは独立しているので対象外。
|
- `ScopeSubDelegated` も turn とは独立しているので対象外。
|
||||||
- 親側の受信処理(`apply_event_side_effects` / `NotifyBuffer` への投入 / history への append)は変更しない。発火源を絞ることで自然にノイズが減る前提。
|
- 親側の受信処理(`apply_event_side_effects` / `NotifyBuffer` への投入 / history への append)は変更しない。発火源を絞ることで自然にノイズが減る前提。
|
||||||
|
|
@ -28,9 +32,9 @@
|
||||||
## 完了条件
|
## 完了条件
|
||||||
|
|
||||||
- 子 Pod を spawn して `SpawnPod` の初回 Run または `SendToPod` で投げた turn が `Finished` で完了すると、親の history に当該 `TurnEnded` 由来の system message が 1 件 append される。
|
- 子 Pod を spawn して `SpawnPod` の初回 Run または `SendToPod` で投げた turn が `Finished` で完了すると、親の history に当該 `TurnEnded` 由来の system message が 1 件 append される。
|
||||||
- 子 Pod が孫 Pod からの `PodEvent::TurnEnded`(または他の Notify)で auto-kick された turn が完了しても、親の history には何も append されない。
|
- 子 Pod が孫 Pod からの `PodEvent`(または他の Notify)で auto-kick された turn が完了しても、親の history には何も append されない。
|
||||||
- 親由来 turn が worker エラーで失敗すると親に `Errored` が届く。Notify 起点 turn の worker エラーは親に届かない。
|
- 親由来 turn が worker エラーで失敗すると親に `Errored` が届く。`RunForNotification` 起点 turn の worker エラーは親に届かない。
|
||||||
- ユニットテストで「`Method::Run` 完了 → 親に届く」「`run_for_notification` 完了 → 親に届かない」「`Method::Resume` 完了 → 親に届く」「Notify 起点 turn の Errored → 親に届かない」を最低限カバーする。
|
- ユニットテストで「`PendingRun::Run` 完了 → 親に届く」「`PendingRun::RunForNotification` 完了 → 親に届かない」「`PendingRun::Resume` 完了 → 親に届く」「`RunForNotification` 起点 turn の Errored → 親に届かない」を最低限カバーする。
|
||||||
|
|
||||||
## 範囲外
|
## 範囲外
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,63 +0,0 @@
|
||||||
# TUI `#` Knowledge 補完の未実装解消
|
|
||||||
|
|
||||||
## 背景
|
|
||||||
|
|
||||||
TUI 入力欄で `#` を打つと `CompletionKind::Knowledge` で Pod に `ListCompletions` を投げる導線は既にある(`crates/tui/src/input.rs`、`crates/tui/src/app.rs`)。一方 Pod 側 IPC は `crates/pod/src/ipc/server.rs:105` で
|
|
||||||
|
|
||||||
```rust
|
|
||||||
protocol::CompletionKind::Knowledge => Vec::new(),
|
|
||||||
```
|
|
||||||
|
|
||||||
と無条件に空ベクタを返しており、ワークスペースに knowledge エントリがあっても TUI 上では候補が一切出ない。`#` で何も出ないのはフロントマター(`model_invokation`)の問題ではなく、補完経路そのものが未配線。
|
|
||||||
|
|
||||||
Workflow 側は同じ構造で既に動いている:
|
|
||||||
|
|
||||||
- `Pod::workflow_completions()` → `Vec<String>`(`crates/pod/src/pod.rs:1236`)
|
|
||||||
- `PodController::start` で `PodSharedState::set_workflows()` に投入(`crates/pod/src/controller.rs:385`)
|
|
||||||
- IPC が `shared_state.list_workflow_completions(prefix)` を引いて返す
|
|
||||||
|
|
||||||
Knowledge も同じ形に揃えれば足りる。
|
|
||||||
|
|
||||||
## 前提
|
|
||||||
|
|
||||||
- knowledge の物理レイアウトは `<workspace>/.insomnia/knowledge/<slug>.md`(`crates/memory/src/workspace.rs`)。
|
|
||||||
- memory クレートには `collect_resident_knowledge` があるが、これは `model_invokation: true` のみを返す resident-injection 用 API で、補完用途には不適。
|
|
||||||
- `#` 補完では「ユーザーが参照可能な knowledge slug すべて」を返したい(`model_invokation` は resident 注入対象のフラグであって参照可否ではない)。
|
|
||||||
- workflow の `list_user_invocable` に相当する「列挙 API」が memory クレートに無いので追加が要る。
|
|
||||||
|
|
||||||
## 方針
|
|
||||||
|
|
||||||
- memory クレートに「knowledge slug 一覧を返す」公開 API を追加する。`collect_resident_knowledge` とは別関数とし、`model_invokation` でフィルタしない。frontmatter が壊れているファイルは silently skip する(`collect_resident_knowledge` と同じく Linter が write 時に shape を保証する前提)。
|
|
||||||
- `PodSharedState` に knowledge 候補スロット(`workflows` と同形の `OnceLock<Vec<...>>`)を追加し、`PodController::start` で memory layout から列挙して投入する。memory layout 未設定の Pod(`Pod::memory_layout` が `None`)では空のまま残す。
|
|
||||||
- IPC server の `CompletionKind::Knowledge` 分岐を、`shared_state.list_knowledge_completions(&prefix)` を引いて `CompletionEntry { value: slug, is_dir: false }` に詰める実装に置き換える。
|
|
||||||
- 補完結果の並びは slug 昇順、prefix マッチは `starts_with`(workflow と揃える)。
|
|
||||||
|
|
||||||
## 要件
|
|
||||||
|
|
||||||
- TUI で `#` を入力した状態で、ワークスペース `.insomnia/knowledge/` 配下に存在する slug(`model_invokation` の真偽に関わらず)が候補として列挙される。
|
|
||||||
- `#foo` のように prefix 入力中の場合、prefix にマッチする slug のみが返る。
|
|
||||||
- memory layout が無効な Pod(`memory_layout: None`)では空ベクタが返り、エラーにはならない。
|
|
||||||
- 確定時の挙動(`replace_with_knowledge_ref` → `Segment::KnowledgeRef` 化、`#slug` チップ表示)は既存の TUI 側の実装をそのまま使う。Pod 側補完を埋めるだけで TUI 改修は不要。
|
|
||||||
- 単体テストで以下をカバーする
|
|
||||||
- 全件列挙(複数 slug、`model_invokation: true/false` 混在で全部返る)
|
|
||||||
- prefix フィルタ
|
|
||||||
- knowledge ディレクトリ不在時の空ベクタ
|
|
||||||
- 壊れた frontmatter / `.md` 以外のファイルがスキップされる
|
|
||||||
|
|
||||||
## 範囲外
|
|
||||||
|
|
||||||
- knowledge のフロントマター仕様変更や、補完候補に description / model_invokation を載せて TUI で表示する強化(`CompletionEntry.value` 以外の表示は別チケットで検討)
|
|
||||||
- workflow / file の補完経路への手入れ
|
|
||||||
- resident 注入経路(`collect_resident_knowledge` の挙動)の変更
|
|
||||||
|
|
||||||
## 参照
|
|
||||||
|
|
||||||
- 未実装箇所: `crates/pod/src/ipc/server.rs:105`
|
|
||||||
- mirror 対象: `crates/pod/src/pod.rs:1236`(`workflow_completions`)、`crates/pod/src/shared_state.rs:74-89`、`crates/pod/src/controller.rs:385-390`
|
|
||||||
- TUI 側既存導線: `crates/tui/src/input.rs:260`、`crates/tui/src/app.rs:281,315`
|
|
||||||
- 列挙対象: `crates/memory/src/workspace.rs`(`knowledge_dir`)、`crates/memory/src/resident.rs`(参考)
|
|
||||||
|
|
||||||
## Review
|
|
||||||
- 状態: Approve
|
|
||||||
- レビュー詳細: [./tui-knowledge-completion.review.md](./tui-knowledge-completion.review.md)
|
|
||||||
- 日付: 2026-05-12
|
|
||||||
|
|
@ -1,35 +0,0 @@
|
||||||
# Review: TUI `#` Knowledge 補完の未実装解消
|
|
||||||
|
|
||||||
対象実装: `7b8eb3a feat(pod): wire knowledge slugs into # completion` (branch `tui-knowledge-completion`)
|
|
||||||
|
|
||||||
## 前提・要件の確認
|
|
||||||
|
|
||||||
要件4項目すべてを diff 上に対応コードと根拠付きで確認した。
|
|
||||||
|
|
||||||
- 「`.insomnia/knowledge/` 配下の slug が `model_invokation` の真偽に関わらず列挙される」: `memory::list_knowledge_slugs` が `walk_knowledge` を `model_invokation` フィルタなしで通す(`crates/memory/src/resident.rs:47-52`)。テスト `list_slugs_returns_all_regardless_of_model_invokation`(同 `:196-204`)が `true/false/true` の 3 件を全部返すことを確認している。
|
|
||||||
- 「`#foo` の prefix 入力中は prefix マッチのみが返る」: `PodSharedState::list_knowledge_completions` が `starts_with` で絞り込み(`crates/pod/src/shared_state.rs:102-113`)。テスト `knowledge_completions_filter_by_prefix`(同 `:262-287`)が `alpha` prefix で `alpha`/`alphabet` のみ返り、`zzz` で空、空 prefix で全件、を確認。
|
|
||||||
- 「memory_layout が None の Pod で空ベクタ、エラーにならない」: `Pod::knowledge_completions` が `memory_layout.as_ref().map(...).unwrap_or_default()` で短絡(`crates/pod/src/pod.rs:1240-1245`)。controller も `Vec<String>` を素通しで `set_knowledge` に渡すだけで panic 経路なし(`crates/pod/src/controller.rs:391-396`)。IPC 側も `OnceLock` 未 set で空を返す(テスト `knowledge_completions_empty_when_unset` `:256-260` で確認)。
|
|
||||||
- 「確定時挙動は既存 TUI のまま、Pod 側を埋めるだけ」: TUI クレートには変更なし。IPC server の `CompletionKind::Knowledge` 分岐のみが `Vec::new()` から実装に差し替えられている(`crates/pod/src/ipc/server.rs:105-113`)。`CompletionEntry` の `value` に slug、`is_dir: false` を詰める形は Workflow 分岐と完全に同形。
|
|
||||||
|
|
||||||
単体テスト 4 項目もすべて存在し、`cargo test -p memory --lib resident::` と `cargo test -p pod --lib shared_state::` でグリーン。
|
|
||||||
|
|
||||||
## アーキテクチャ・スコープ
|
|
||||||
|
|
||||||
- 列挙 API を `memory` クレート(低レベル workspace 操作の所在)に追加し、Pod 層は Memory layout から「引いて詰めるだけ」というレイヤ分割を保っている。`llm-worker` を巻き込まない、higher-level は上層という方針に合致。
|
|
||||||
- `WorkflowCandidate` / `set_workflows` / `list_workflow_completions` と `KnowledgeCandidate` / `set_knowledge` / `list_knowledge_completions` がフィールド順・docstring の有無・実装行数まで揃っており、mirror 対象(`shared_state.rs:74-89`、`controller.rs:385-390`、`pod.rs:1236`)のスタイルと一貫している。
|
|
||||||
- `walk_knowledge` の共通化は `FnMut(String, KnowledgeFrontmatter)` 1 つの最小抽象で、2 呼び出し元の重複(`read_dir` のエラー早抜け、非 `.md` スキップ、frontmatter parse スキップ)を素直にまとめている。やりすぎ(Iterator 化、ジェネリック化)はしておらず、CLAUDE.md の「変更量を最小に」「設計を歪めない」に合致する。逆に共通化を見送って書き写すと 30 行程度の同形コード重複になるので、この程度の抽出は妥当。
|
|
||||||
- 範囲外は守られている。frontmatter スキーマ、`collect_resident_knowledge` の挙動(`model_invokation: true` のみ返す resident 注入用途)、workflow/file 補完経路、TUI コードへの手入れは一切なし。
|
|
||||||
|
|
||||||
## 指摘事項
|
|
||||||
|
|
||||||
### Non-blocking / Follow-up
|
|
||||||
|
|
||||||
- なし。
|
|
||||||
|
|
||||||
### Nits
|
|
||||||
|
|
||||||
- `crates/memory/src/resident.rs:26-28` の `collect_resident_knowledge` の docstring が `<workspace>/knowledge/*.md` のままで、実 path `<workspace>/.insomnia/knowledge/*.md`(`list_knowledge_slugs` 側の docstring `:43-46` では正しく書かれている)と齟齬がある。本チケットの範囲外の既存記述だが、隣接行を編集したついでに同期しておくと整う。今回は追わなくてよい。
|
|
||||||
|
|
||||||
## 判断
|
|
||||||
|
|
||||||
Approve — ticket の前提・方針・要件・テスト 4 項目すべてに対応コードとパスする単体テストがあり、範囲外を踏まず、Workflow 経路と整合した最小実装になっている。
|
|
||||||
Loading…
Reference in New Issue
Block a user