From e647d1a7c9e202d1ddcb21c5bf6f708d841b3add Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 10 May 2026 00:57:50 +0900 Subject: [PATCH] =?UTF-8?q?feat:=20client-crate=E3=81=AE=E5=AE=9F=E8=A3=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 11 + Cargo.toml | 2 + crates/client/Cargo.toml | 11 + crates/client/src/lib.rs | 15 + .../client.rs => client/src/pod_client.rs} | 0 crates/client/src/spawn.rs | 294 ++++++++++++++++++ crates/tui/Cargo.toml | 1 + crates/tui/src/main.rs | 4 +- crates/tui/src/spawn.rs | 244 ++------------- tickets/client-crate.md | 5 + tickets/client-crate.review.md | 48 +++ 11 files changed, 412 insertions(+), 223 deletions(-) create mode 100644 crates/client/Cargo.toml create mode 100644 crates/client/src/lib.rs rename crates/{tui/src/client.rs => client/src/pod_client.rs} (100%) create mode 100644 crates/client/src/spawn.rs create mode 100644 tickets/client-crate.review.md diff --git a/Cargo.lock b/Cargo.lock index 4d8b0360..3f2fab3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,6 +328,16 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" +[[package]] +name = "client" +version = "0.1.0" +dependencies = [ + "manifest", + "protocol", + "tokio", + "uuid", +] + [[package]] name = "cmake" version = "0.1.57" @@ -3621,6 +3631,7 @@ dependencies = [ name = "tui" version = "0.1.0" dependencies = [ + "client", "crossterm 0.28.1", "manifest", "pod-registry", diff --git a/Cargo.toml b/Cargo.toml index 7751c6cd..fe47bfdc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [workspace] resolver = "2" members = [ + "crates/client", "crates/daemon", "crates/llm-worker", "crates/llm-worker-macros", @@ -22,6 +23,7 @@ license = "MIT" [workspace.dependencies] # Internal crates +client = { path = "crates/client" } llm-worker = { path = "crates/llm-worker", version = "0.2" } llm-worker-macros = { path = "crates/llm-worker-macros", version = "0.2" } manifest = { path = "crates/manifest" } diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml new file mode 100644 index 00000000..d9722ad0 --- /dev/null +++ b/crates/client/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "client" +version = "0.1.0" +edition.workspace = true +license.workspace = true + +[dependencies] +protocol = { workspace = true } +manifest = { workspace = true } +tokio = { workspace = true, features = ["rt", "macros", "net", "io-util", "sync", "time", "process", "fs"] } +uuid = { workspace = true } diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs new file mode 100644 index 00000000..af8820bc --- /dev/null +++ b/crates/client/src/lib.rs @@ -0,0 +1,15 @@ +//! Pod プロトコルを喋るクライアント。 +//! +//! - [`PodClient`]: 既存 pod の Unix ソケットへ接続して `Method` を送り、 +//! `Event` を受け取る低レベル接続。 +//! - [`spawn`]: pod バイナリをサブプロセスとして起動し、`INSOMNIA-READY` +//! ハンドシェイクが終わるまで待つフロー。subprocess を立ち上げる必要が +//! ない呼び出し側 (=既存 pod に attach する場合) は使わなくてよい。 +//! +//! TUI / GUI / E2E ハーネスはこの crate に依存して protocol を喋る。 + +mod pod_client; +pub mod spawn; + +pub use pod_client::PodClient; +pub use spawn::{SpawnConfig, SpawnError, SpawnReady, spawn_pod}; diff --git a/crates/tui/src/client.rs b/crates/client/src/pod_client.rs similarity index 100% rename from crates/tui/src/client.rs rename to crates/client/src/pod_client.rs diff --git a/crates/client/src/spawn.rs b/crates/client/src/spawn.rs new file mode 100644 index 00000000..3df9e3c1 --- /dev/null +++ b/crates/client/src/spawn.rs @@ -0,0 +1,294 @@ +//! pod バイナリをサブプロセスとして立ち上げ、`INSOMNIA-READY` を待つ +//! ハンドシェイク。 +//! +//! - 親プロセス (TUI / GUI / E2E) は overlay TOML を組み立ててこの関数に +//! 渡す。pod はそれを受けて socket を bind し、stderr に +//! `INSOMNIA-READY\t\t` を吐く。 +//! - 待機中の stderr 行は `progress` コールバック越しに呼び出し側へ流す。 +//! UI の進捗表示や E2E のログ収集はここで賄う。 +//! - `kill_on_drop = false` + `process_group(0)` により、親プロセス +//! ライフサイクルから切り離した detached pod を作る。ready 後の lifecycle +//! 管理は runtime ディレクトリ / socket を介して行う。 + +use std::io; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::time::Duration; + +use tokio::process::Command; +use uuid::Uuid; + +const READY_PREFIX: &str = "INSOMNIA-READY\t"; +const READY_TIMEOUT: Duration = Duration::from_secs(20); + +/// `spawn_pod` の入力。 +pub struct SpawnConfig { + /// `pod.name` として使う識別子。runtime ディレクトリ + /// (`manifest::paths::pod_runtime_dir`) の解決と、ready 行に乗る + /// 名前との突き合わせに使う。 + pub pod_name: String, + /// `--overlay` で pod に渡す TOML 文字列。 + pub overlay_toml: String, + /// pod の current_dir。 + pub cwd: PathBuf, + /// `Some(id)` のとき `--session ` を付与し、当該セッションから + /// resume させる。 + pub resume_from: Option, +} + +pub struct SpawnReady { + pub pod_name: String, + pub socket_path: PathBuf, +} + +#[derive(Debug)] +pub enum SpawnError { + Io(io::Error), + /// runtime ディレクトリが解決できなかった (環境変数未設定等)。 + RuntimeDirUnavailable, + PodLaunchFailed(io::Error), + PodExitedEarly { stderr_tail: String }, + Timeout, +} + +impl std::fmt::Display for SpawnError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io(e) => write!(f, "io error: {e}"), + Self::RuntimeDirUnavailable => write!( + f, + "could not resolve runtime directory (set INSOMNIA_HOME, INSOMNIA_RUNTIME_DIR, XDG_RUNTIME_DIR, or HOME)" + ), + Self::PodLaunchFailed(e) => write!(f, "failed to launch pod: {e}"), + Self::PodExitedEarly { stderr_tail } => { + if stderr_tail.is_empty() { + write!(f, "pod exited before becoming ready") + } else { + write!(f, "pod exited before becoming ready: {stderr_tail}") + } + } + Self::Timeout => write!( + f, + "pod did not become ready within {}s", + READY_TIMEOUT.as_secs() + ), + } + } +} + +impl std::error::Error for SpawnError {} + +impl From for SpawnError { + fn from(e: io::Error) -> Self { + Self::Io(e) + } +} + +/// pod を spawn し、`INSOMNIA-READY` ハンドシェイクが終わるまで待つ。 +/// +/// `progress` は ready 行を見つけるまでに観測した stderr の各行で呼ばれる +/// (ready 行自体は除外される)。UI の表示更新や E2E ログ取得に使う。 +pub async fn spawn_pod( + config: SpawnConfig, + mut progress: F, +) -> Result +where + F: FnMut(&str), +{ + let pod_bin = resolve_pod_command(); + + let pod_runtime_dir = manifest::paths::pod_runtime_dir(&config.pod_name) + .ok_or(SpawnError::RuntimeDirUnavailable)?; + std::fs::create_dir_all(&pod_runtime_dir).map_err(SpawnError::Io)?; + let stderr_path = pod_runtime_dir.join("stderr.log"); + let stderr_file = std::fs::File::create(&stderr_path).map_err(SpawnError::Io)?; + + let mut command = Command::new(&pod_bin); + command + .arg("--overlay") + .arg(&config.overlay_toml) + .current_dir(&config.cwd) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(stderr_file)) + .process_group(0); + if let Some(id) = config.resume_from { + command.arg("--session").arg(id.to_string()); + } + let mut child = command.spawn().map_err(SpawnError::PodLaunchFailed)?; + + // Default `kill_on_drop = false` plus `process_group(0)` makes this + // a detached Pod once startup succeeds: dropping the handle does not + // terminate it, and terminal-generated signals for the parent's + // process group do not hit the Pod. Runtime state/socket files are + // the source of truth after that point. + let ready = match wait_for_ready_file(&mut progress, &stderr_path, &mut child).await { + Ok(ready) => ready, + Err(e) => { + let _ = child.start_kill(); + let _ = child.wait().await; + return Err(e); + } + }; + tokio::spawn(async move { + let _ = child.wait().await; + }); + Ok(ready) +} + +async fn wait_for_ready_file( + progress: &mut F, + stderr_path: &Path, + child: &mut tokio::process::Child, +) -> Result +where + F: FnMut(&str), +{ + let mut tail = StderrTail::new(); + let deadline = tokio::time::Instant::now() + READY_TIMEOUT; + let mut offset = 0usize; + + loop { + let content = match tokio::fs::read_to_string(stderr_path).await { + Ok(content) => content, + Err(e) if e.kind() == io::ErrorKind::NotFound => String::new(), + Err(e) => return Err(SpawnError::Io(e)), + }; + if content.len() > offset { + for line in content[offset..].lines() { + if let Some(rest) = line.strip_prefix(READY_PREFIX) { + let mut parts = rest.splitn(2, '\t'); + let pod_name = parts.next().unwrap_or("").to_string(); + let socket_str = parts.next().unwrap_or("").to_string(); + if pod_name.is_empty() || socket_str.is_empty() { + return Err(SpawnError::PodExitedEarly { + stderr_tail: format!("malformed ready line: {line}"), + }); + } + let socket_path = PathBuf::from(socket_str); + wait_for_socket( + &socket_path, + deadline, + child, + stderr_path, + &mut tail, + &mut offset, + ) + .await?; + return Ok(SpawnReady { + pod_name, + socket_path, + }); + } + tail.push(line); + progress(line); + } + offset = content.len(); + } + + if tokio::time::Instant::now() >= deadline { + return Err(SpawnError::Timeout); + } + tokio::select! { + status = child.wait() => { + let _ = status; + // Pod は exit 直前に最終 stderr 行を flush することがある。 + // child.wait() が解決した後に再読みして、原因行を取りこ + // ぼさず PodExitedEarly に載せる。 + drain_stderr_into_tail(stderr_path, &mut tail, &mut offset).await; + return Err(SpawnError::PodExitedEarly { + stderr_tail: tail.into_string(), + }); + } + _ = tokio::time::sleep(Duration::from_millis(100)) => {} + } + } +} + +async fn wait_for_socket( + socket_path: &Path, + deadline: tokio::time::Instant, + child: &mut tokio::process::Child, + stderr_path: &Path, + tail: &mut StderrTail, + offset: &mut usize, +) -> Result<(), SpawnError> { + loop { + match tokio::net::UnixStream::connect(socket_path).await { + Ok(_) => return Ok(()), + Err(e) + if e.kind() == io::ErrorKind::NotFound + || e.kind() == io::ErrorKind::ConnectionRefused => {} + Err(e) => return Err(SpawnError::Io(e)), + } + if tokio::time::Instant::now() >= deadline { + return Err(SpawnError::Timeout); + } + tokio::select! { + status = child.wait() => { + let _ = status; + drain_stderr_into_tail(stderr_path, tail, offset).await; + return Err(SpawnError::PodExitedEarly { + stderr_tail: tail.as_string(), + }); + } + _ = tokio::time::sleep(Duration::from_millis(50)) => {} + } + } +} + +async fn drain_stderr_into_tail(stderr_path: &Path, tail: &mut StderrTail, offset: &mut usize) { + let Ok(content) = tokio::fs::read_to_string(stderr_path).await else { + return; + }; + if content.len() <= *offset { + return; + } + for line in content[*offset..].lines() { + if !line.starts_with(READY_PREFIX) { + tail.push(line); + } + } + *offset = content.len(); +} + +/// Resolves the binary used to launch a child Pod. Must point at a +/// `pod`-compatible executable — the parent reads the child's stderr +/// directly looking for `INSOMNIA-READY`, so any wrapper that emits +/// extra lines on stderr will pollute that handshake. +/// +/// `INSOMNIA_POD_COMMAND` overrides the lookup (used by tests to inject +/// a mock binary). Otherwise we defer to `PATH` — missing binary +/// surfaces as the spawn `io::Error`. +fn resolve_pod_command() -> PathBuf { + if let Ok(cmd) = std::env::var("INSOMNIA_POD_COMMAND") + && !cmd.is_empty() + { + return PathBuf::from(cmd); + } + PathBuf::from("pod") +} + +struct StderrTail { + lines: std::collections::VecDeque, +} + +impl StderrTail { + fn new() -> Self { + Self { + lines: std::collections::VecDeque::with_capacity(8), + } + } + fn push(&mut self, line: &str) { + if self.lines.len() == 8 { + self.lines.pop_front(); + } + self.lines.push_back(line.to_string()); + } + fn as_string(&self) -> String { + self.lines.iter().cloned().collect::>().join(" | ") + } + fn into_string(self) -> String { + self.lines.into_iter().collect::>().join(" | ") + } +} diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index 36181268..f240581a 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +client = { workspace = true } protocol = { workspace = true } ratatui = { version = "0.30.0", features = ["scrolling-regions"] } crossterm = "0.28" diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index bb9cd4a2..f24425f8 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -1,7 +1,6 @@ mod app; mod block; mod cache; -mod client; mod input; mod markdown; mod picker; @@ -28,8 +27,9 @@ use ratatui::Terminal; use ratatui::backend::CrosstermBackend; use session_store::SessionId; +use client::PodClient; + use crate::app::App; -use crate::client::PodClient; use crate::picker::PickerOutcome; use crate::spawn::{SpawnOutcome, SpawnReady}; diff --git a/crates/tui/src/spawn.rs b/crates/tui/src/spawn.rs index 0d5479f5..41aa67ad 100644 --- a/crates/tui/src/spawn.rs +++ b/crates/tui/src/spawn.rs @@ -14,9 +14,9 @@ use std::io; use std::path::PathBuf; -use std::process::Stdio; use std::time::Duration; +use client::{SpawnConfig, spawn_pod}; use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers}; use manifest::{ PodManifestConfig, ScopeConfig, find_project_manifest_from, load_layer, user_manifest_path, @@ -29,11 +29,8 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use ratatui::{Frame, TerminalOptions, Viewport}; use session_store::SessionId; -use tokio::process::Command; -const READY_PREFIX: &str = "INSOMNIA-READY\t"; const VIEWPORT_LINES: u16 = 6; -const READY_TIMEOUT: Duration = Duration::from_secs(20); pub struct SpawnReady { pub pod_name: String, @@ -50,9 +47,7 @@ pub enum SpawnError { Io(io::Error), Store(session_store::StoreError), MissingResumeScope { session_id: SessionId }, - PodLaunchFailed(io::Error), - PodExitedEarly { stderr_tail: String }, - Timeout, + Spawn(client::SpawnError), } impl std::fmt::Display for SpawnError { @@ -64,19 +59,7 @@ impl std::fmt::Display for SpawnError { f, "session {session_id} has no persisted scope snapshot; refusing resume without explicit scope" ), - Self::PodLaunchFailed(e) => write!(f, "failed to launch pod: {e}"), - Self::PodExitedEarly { stderr_tail } => { - if stderr_tail.is_empty() { - write!(f, "pod exited before becoming ready") - } else { - write!(f, "pod exited before becoming ready: {stderr_tail}") - } - } - Self::Timeout => write!( - f, - "pod did not become ready within {}s", - READY_TIMEOUT.as_secs() - ), + Self::Spawn(e) => write!(f, "{e}"), } } } @@ -95,6 +78,12 @@ impl From for SpawnError { } } +impl From for SpawnError { + fn from(e: client::SpawnError) -> Self { + Self::Spawn(e) + } +} + type InlineTerminal = Terminal>; /// Source session for a resume run. `None` = fresh spawn (current @@ -283,169 +272,23 @@ async fn wait_for_ready( form: &mut Form, overlay_toml: &str, ) -> Result { - let pod_bin = resolve_pod_command(); let cwd = std::env::current_dir().map_err(SpawnError::Io)?; - let pod_runtime_dir = manifest::paths::pod_runtime_dir(&form.name).ok_or_else(|| { - io::Error::new( - io::ErrorKind::NotFound, - "could not resolve runtime directory (set INSOMNIA_HOME, INSOMNIA_RUNTIME_DIR, XDG_RUNTIME_DIR, or HOME)", - ) - })?; - std::fs::create_dir_all(&pod_runtime_dir).map_err(SpawnError::Io)?; - let stderr_path = pod_runtime_dir.join("stderr.log"); - let stderr_file = std::fs::File::create(&stderr_path).map_err(SpawnError::Io)?; - - let mut command = Command::new(&pod_bin); - command - .arg("--overlay") - .arg(overlay_toml) - .current_dir(&cwd) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::from(stderr_file)) - .process_group(0); - if let Some(id) = form.resume_from { - command.arg("--session").arg(id.to_string()); - } - let mut child = command.spawn().map_err(SpawnError::PodLaunchFailed)?; - - // Default `kill_on_drop = false` plus `process_group(0)` makes this - // a detached Pod for TUI lifecycle purposes once startup succeeds: - // dropping the handle does not terminate it, and terminal-generated - // signals for the TUI's process group do not hit the Pod. Runtime - // state/socket files are the source of truth after that point. - let ready = match wait_for_ready_file(terminal, form, &stderr_path, &mut child).await { - Ok(ready) => ready, - Err(e) => { - let _ = child.start_kill(); - let _ = child.wait().await; - return Err(e); - } + let config = SpawnConfig { + pod_name: form.name.clone(), + overlay_toml: overlay_toml.to_string(), + cwd, + resume_from: form.resume_from, }; - tokio::spawn(async move { - let _ = child.wait().await; - }); - Ok(ready) -} - -async fn wait_for_ready_file( - terminal: &mut InlineTerminal, - form: &mut Form, - stderr_path: &std::path::Path, - child: &mut tokio::process::Child, -) -> Result { - let mut tail = StderrTail::new(); - let deadline = tokio::time::Instant::now() + READY_TIMEOUT; - let mut offset = 0usize; - - loop { - let content = match tokio::fs::read_to_string(stderr_path).await { - Ok(content) => content, - Err(e) if e.kind() == io::ErrorKind::NotFound => String::new(), - Err(e) => return Err(SpawnError::Io(e)), - }; - if content.len() > offset { - for line in content[offset..].lines() { - if let Some(rest) = line.strip_prefix(READY_PREFIX) { - let mut parts = rest.splitn(2, '\t'); - let pod_name = parts.next().unwrap_or("").to_string(); - let socket_str = parts.next().unwrap_or("").to_string(); - if pod_name.is_empty() || socket_str.is_empty() { - return Err(SpawnError::PodExitedEarly { - stderr_tail: format!("malformed ready line: {line}"), - }); - } - let socket_path = PathBuf::from(socket_str); - wait_for_socket( - &socket_path, - deadline, - child, - stderr_path, - &mut tail, - &mut offset, - ) - .await?; - return Ok(SpawnReady { - pod_name, - socket_path, - }); - } - tail.push(line); - form.message = Some((line.to_string(), MessageKind::Progress)); - let _ = terminal.draw(|f| draw_form(f, form)); - } - offset = content.len(); - } - - if tokio::time::Instant::now() >= deadline { - return Err(SpawnError::Timeout); - } - tokio::select! { - status = child.wait() => { - let _ = status; - // Pod は exit 直前に最終 stderr 行を flush することがある。 - // child.wait() が解決した後に再読みして、原因行を取りこ - // ぼさず PodExitedEarly に載せる。 - drain_stderr_into_tail(stderr_path, &mut tail, &mut offset).await; - return Err(SpawnError::PodExitedEarly { - stderr_tail: tail.into_string(), - }); - } - _ = tokio::time::sleep(Duration::from_millis(100)) => {} - } - } -} - -async fn wait_for_socket( - socket_path: &std::path::Path, - deadline: tokio::time::Instant, - child: &mut tokio::process::Child, - stderr_path: &std::path::Path, - tail: &mut StderrTail, - offset: &mut usize, -) -> Result<(), SpawnError> { - loop { - match tokio::net::UnixStream::connect(socket_path).await { - Ok(_) => return Ok(()), - Err(e) - if e.kind() == io::ErrorKind::NotFound - || e.kind() == io::ErrorKind::ConnectionRefused => {} - Err(e) => return Err(SpawnError::Io(e)), - } - if tokio::time::Instant::now() >= deadline { - return Err(SpawnError::Timeout); - } - tokio::select! { - status = child.wait() => { - let _ = status; - drain_stderr_into_tail(stderr_path, tail, offset).await; - return Err(SpawnError::PodExitedEarly { - stderr_tail: tail.as_string(), - }); - } - _ = tokio::time::sleep(Duration::from_millis(50)) => {} - } - } -} - -async fn drain_stderr_into_tail( - stderr_path: &std::path::Path, - tail: &mut StderrTail, - offset: &mut usize, -) { - let Ok(content) = tokio::fs::read_to_string(stderr_path).await else { - return; - }; - if content.len() <= *offset { - return; - } - for line in content[*offset..].lines() { - if !line.starts_with(READY_PREFIX) { - tail.push(line); - } - } - *offset = content.len(); + let ready = spawn_pod(config, |line| { + form.message = Some((line.to_string(), MessageKind::Progress)); + let _ = terminal.draw(|f| draw_form(f, form)); + }) + .await?; + Ok(SpawnReady { + pod_name: ready.pod_name, + socket_path: ready.socket_path, + }) } fn build_overlay_toml(form: &Form) -> String { @@ -496,47 +339,6 @@ async fn load_resume_scope(session_id: SessionId) -> Result PathBuf { - if let Ok(cmd) = std::env::var("INSOMNIA_POD_COMMAND") { - if !cmd.is_empty() { - return PathBuf::from(cmd); - } - } - PathBuf::from("pod") -} - -struct StderrTail { - lines: std::collections::VecDeque, -} - -impl StderrTail { - fn new() -> Self { - Self { - lines: std::collections::VecDeque::with_capacity(8), - } - } - fn push(&mut self, line: &str) { - if self.lines.len() == 8 { - self.lines.pop_front(); - } - self.lines.push_back(line.to_string()); - } - fn as_string(&self) -> String { - self.lines.iter().cloned().collect::>().join(" | ") - } - fn into_string(self) -> String { - self.lines.into_iter().collect::>().join(" | ") - } -} - enum MessageKind { Info, Ok, diff --git a/tickets/client-crate.md b/tickets/client-crate.md index 3c7486dd..56812df0 100644 --- a/tickets/client-crate.md +++ b/tickets/client-crate.md @@ -49,3 +49,8 @@ TUI 内に置いたまま GUI と E2E から再利用しようとすると、TUI - `tickets/e2e-harness.md` - `crates/tui/src/client.rs` - `crates/protocol/` + +## Review +- 状態: Approve +- レビュー詳細: [./client-crate.review.md](./client-crate.review.md) +- 日付: 2026-05-09 diff --git a/tickets/client-crate.review.md b/tickets/client-crate.review.md new file mode 100644 index 00000000..1e866ce5 --- /dev/null +++ b/tickets/client-crate.review.md @@ -0,0 +1,48 @@ +# Review: Client crate の切り出し + +## 前提・要件の確認 + +- `crates/client/` 新設、`crates/tui/src/client.rs` 相当の機能を提供 (`tickets/client-crate.md:28`) + - 満たされている。`crates/tui/src/client.rs` は `crates/client/src/pod_client.rs` へ純粋 rename されており (`PodClient::{connect, send, try_next_event, next_event}` の 4 API は完全に同一)、`Drop` で reader タスクが mpsc 切断を契機に終了する graceful shutdown 挙動も同じ。 +- TUI が新 crate に依存して動作・既存テストが通る (`tickets/client-crate.md:29`) + - 満たされている。`crates/tui/Cargo.toml:8` で `client = { workspace = true }` を追加、`crates/tui/src/main.rs:30` で `use client::PodClient`、`crates/tui/src/spawn.rs:19` で `use client::{SpawnConfig, spawn_pod}`。`cargo test -p tui` 91 件 pass、ローカルで `cargo build --workspace` も完走。 +- API は GUI / E2E から呼べる粒度で公開 (`tickets/client-crate.md:30`) + - 満たされている。`crates/client/src/lib.rs:14-15` で `PodClient` と `spawn::{SpawnConfig, SpawnError, SpawnReady, spawn_pod}` が re-export されており、接続 / Method 送信 / Event 購読 (`next_event` / `try_next_event`) / shutdown (Drop) と subprocess spawn が独立して呼び出せる粒度で揃う。 +- pod subprocess を spawn する経路の決定 (`tickets/client-crate.md:31`) + - 満たされている。client crate に同梱しつつ `spawn_pod` と `PodClient::connect` を分離。GUI が「自身で spawn せず attach する」「自身で spawn する」のいずれを選んでも一段の関数呼び出しで済む。 + +## 完了条件 (`tickets/client-crate.md:33-37`) + +- 上記要件を満たす点は OK。 +- TUI 既存挙動・テスト通過は OK (`spawn::tests::*` 6 件含めて 91 件 pass)。 +- E2E ハーネス (`tickets/e2e-harness.md`) は本 crate に依存して protocol を喋れる状態 — 公開 API として `PodClient::connect(&Path) -> Result` と `spawn_pod(SpawnConfig, FnMut(&str)) -> Result` が揃ったため、依存追加だけで E2E が protocol を直接駆動できる。OK。 + +## アーキテクチャ・スコープ + +- 範囲外 (`tickets/client-crate.md:39-44`) を侵していない: + - GUI バイナリ実装には踏み込んでいない。 + - protocol 互換破壊なし (`pod_client.rs` は `JsonLineReader/Writer` をそのまま使用)。 + - TUI のリファクタは責務移動のみ。`Form` / `draw_form` / `build_overlay_toml` / `load_resume_scope` 等の UI / manifest 解決ロジックは TUI に残置。`crates/tui/src/spawn.rs:541-653` の単体テストも手付かず。 + - daemon 層には触れていない。 +- crate 命名: `client` (プレフィックス無し)。`feedback_crate_naming.md` 方針に準拠。 +- 依存追加方法: `Cargo.toml` の `[workspace.dependencies]` への登録は workspace 規約上必須なため手動編集が正当 (`cargo add` の対象外)。`crates/client/Cargo.toml` も `protocol` / `manifest` / `tokio` / `uuid` を `workspace = true` 経由で取得しており方針通り。 +- レイヤ整合: `client` は `protocol` / `manifest` / `tokio` のみに依存し、`session-store` や上位レイヤを引きずり込んでいない。`session_store::StoreError` 関連の error は TUI 側の `SpawnError::{Store, MissingResumeScope}` に残置されており、責務分割が綺麗に出来ている (`crates/tui/src/spawn.rs:46-85`)。 +- `SpawnError` の Display 移送: 旧 TUI の `SpawnError` のうち `PodLaunchFailed` / `PodExitedEarly` / `Timeout` / runtime dir 解決失敗 / `Io` を `client::SpawnError` 側へ移動し、TUI 側は `Spawn(client::SpawnError)` でラップして `{e}` で透過表示。重複なくユーザー向けメッセージが保持されている。 +- 公開 API 境界: 生 socket / `JsonLineReader` を露出せず、薄いラッパーに留めた。チケットの検討事項 (`tickets/client-crate.md:22`) に対する妥当な選択で、既存挙動を壊さない最小範囲。 + +## 指摘事項 + +### Non-blocking / Follow-up + +- `crates/client/src/spawn.rs` に単体テストが無い (`INSOMNIA_POD_COMMAND` で mock pod を差し込めば ready / timeout / 早期 exit の各パスをテスト可能)。E2E ハーネス側で間接的にカバーされる予定なら不要だが、E2E チケット先行よりこの crate の自己完結テストが入っていると将来回帰検出が容易。 +- `SpawnConfig::cwd` が `PathBuf` フィールドで強制される一方、TUI 側 `wait_for_ready` (`crates/tui/src/spawn.rs:275`) は `run()` の冒頭で取った cwd と独立にもう一度 `current_dir()` を呼んでいる。動作上は等価だが、責務切り出しの完了を機に `run()` で取得した cwd を form に持たせて一度だけ参照する形にできる (今回スコープ外でも可)。 +- `crates/client/src/spawn.rs:21-22` の `READY_PREFIX` / `READY_TIMEOUT` は pod 側 (`crates/pod/`) の出力フォーマットと結合した contract だが、両者の同期は型/定数共有でなく目視確認のまま。protocol crate に const として置く方が長期的に安全だが、これも今回の責務切り出しスコープを超える。 + +### Nits + +- `crates/client/src/lib.rs:5` の docstring "pod バイナリをサブプロセスとして起動し" は OK だが、`spawn_pod` が `kill_on_drop = false` + `process_group(0)` で detach する点を 1 行追加するとライフサイクル不変条件が見えやすい (本体側 `crates/client/src/spawn.rs:9-11` には記載されているのでそちらへの参照でもよい)。 +- `SpawnReady` の同名構造体が `crates/tui/src/spawn.rs:35-38` にも残っており、`client::SpawnReady` を unwrap して TUI 側で再構築している (`crates/tui/src/spawn.rs:288-291`)。TUI 側の `SpawnReady` は外部公開されないため `pub use client::SpawnReady` に置換可能だが、TUI 内 API 表面の安定性に関わるので任意。 + +## 判断 + +Approve — 要件・完了条件・範囲外の規定に正しく収まっており、責務移動以上のリファクタは行われていない。新 crate は他 crate から再利用できる粒度で公開され、TUI 既存挙動は 91 件のテストで保たれている。Follow-up の指摘は本チケットの完了を妨げない。