From 10eaf4a5fb808ef91b5ebf6e58ff6db7d856943e Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 11:21:14 +0900 Subject: [PATCH 1/7] refactor: name worker event channels by role --- crates/worker/src/controller.rs | 183 +++++++++++---------- crates/worker/src/in_flight.rs | 68 ++++---- crates/worker/src/internal_worker.rs | 2 +- crates/worker/src/ipc/alerter.rs | 10 +- crates/worker/src/segment_log_sink.rs | 18 +- crates/worker/src/spawn/registry.rs | 4 +- crates/worker/src/worker.rs | 64 +++---- crates/worker/tests/compact_events_test.rs | 8 +- crates/worker/tests/controller_test.rs | 4 +- 9 files changed, 184 insertions(+), 177 deletions(-) diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index fbbedf50..5af8355b 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -45,12 +45,12 @@ use workdir::{ #[derive(Clone)] pub struct WorkerHandle { method_tx: mpsc::Sender, - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, pub shared_state: Arc, pub runtime_dir: Arc, pub alerter: Alerter, pub in_flight: InFlightEvents, - /// Segment-log mirror + broadcast handle. The IPC server snapshots + /// Segment-log mirror + session-entry channel. The IPC server snapshots /// it on every new connection (Event::Snapshot) and forwards /// subsequent commits (Event::Entry) on the receiver. pub sink: SegmentLogSink, @@ -63,7 +63,7 @@ impl WorkerHandle { } pub fn subscribe(&self) -> broadcast::Receiver { - self.event_tx.subscribe() + self.working_event_tx.subscribe() } pub fn committed_entries(&self) -> Vec { @@ -117,7 +117,7 @@ impl WorkerHandle { /// Broadcast an event to all listeners (including socket clients). pub fn send_event(&self, event: Event) -> Result> { - self.event_tx.send(event) + self.working_event_tx.send(event) } /// Emit a user-facing alert. Thin wrapper over `Alerter::alert`. @@ -129,19 +129,19 @@ impl WorkerHandle { async fn set_controller_status( shared_state: &Arc, runtime_dir: &RuntimeDir, - event_tx: &broadcast::Sender, + working_event_tx: &broadcast::Sender, status: WorkerStatus, ) { shared_state.set_status(status); let _ = runtime_dir.write_status(shared_state).await; - let _ = event_tx.send(Event::Status { status }); + let _ = working_event_tx.send(Event::Status { status }); } async fn finish_controller_run( worker: &mut Worker, shared_state: &Arc, runtime_dir: &RuntimeDir, - event_tx: &broadcast::Sender, + working_event_tx: &broadcast::Sender, new_status: WorkerStatus, ) where C: LlmClient + Clone + 'static, @@ -157,7 +157,7 @@ async fn finish_controller_run( // the terminal run boundary so reconnect snapshots cannot append stale // partial text/tool arguments after newer entries. worker.clear_in_flight_events(); - set_controller_status(shared_state, runtime_dir, event_tx, new_status).await; + set_controller_status(shared_state, runtime_dir, working_event_tx, new_status).await; worker.spawn_post_run_memory_jobs(); } @@ -353,9 +353,9 @@ impl WorkerController { // bash-output scope) === let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); let (method_tx, method_rx) = mpsc::channel::(32); - let (event_tx, _) = broadcast::channel::(256); - let alerter = Alerter::new(event_tx.clone()); - let in_flight = InFlightEvents::new(event_tx.clone()); + let (working_event_tx, _) = broadcast::channel::(256); + let alerter = Alerter::new(working_event_tx.clone()); + let in_flight = InFlightEvents::new(working_event_tx.clone()); worker.attach_in_flight_events(in_flight.clone()); // Runtime directory is created before tool registration because it owns @@ -395,7 +395,7 @@ impl WorkerController { // Also hand the raw broadcast sender so Worker-internal operations // can emit typed lifecycle `Event`s (currently: compact progress). worker.attach_internal_worker_registry(spawned_registry.clone()); - worker.attach_event_tx(event_tx.clone()); + worker.attach_working_event_tx(working_event_tx.clone()); // Bash spills long outputs to a per-worker subdir under the runtime // dir. Push a recursive `allow(Read)` for that path into the @@ -430,7 +430,7 @@ impl WorkerController { worker.wire_history_persistence(); // === 2. Engine event bridge wiring === - wire_event_bridges_on_engine(&mut worker, &event_tx, &alerter, &in_flight); + wire_event_bridges_on_engine(&mut worker, &working_event_tx, &alerter, &in_flight); // === 3. Tool registration (builtin / memory / spawn-orchestration) === let fs_for_view = register_worker_tools( @@ -477,7 +477,7 @@ impl WorkerController { let handle = WorkerHandle { method_tx, - event_tx: event_tx.clone(), + working_event_tx: working_event_tx.clone(), shared_state: shared_state.clone(), runtime_dir: runtime_dir.clone(), alerter: alerter.clone(), @@ -502,7 +502,7 @@ impl WorkerController { tokio::spawn(controller_loop( worker, method_rx, - event_tx, + working_event_tx, shared_state, runtime_dir, cancel_tx, @@ -640,7 +640,7 @@ fn protocol_command_status(status: WorkdirCommandStatus) -> ProtocolCommandStatu } /// Wire the per-event broadcast bridges on the Worker's Engine. Each callback -/// re-publishes a worker-level signal as a `protocol::Event` on `event_tx` +/// re-publishes a worker-level signal as a `protocol::Event` on `working_event_tx` /// so subscribers (TUI, socket clients) get a single typed stream. /// /// `Worker::wire_history_persistence` is called separately to wire the @@ -649,7 +649,7 @@ fn protocol_command_status(status: WorkdirCommandStatus) -> ProtocolCommandStatu /// / `AnnotatedToolResult` commit through the sync writer. pub(crate) fn wire_event_bridges_on_engine( worker: &mut Worker, - event_tx: &broadcast::Sender, + working_event_tx: &broadcast::Sender, alerter: &Alerter, in_flight: &InFlightEvents, ) where @@ -659,12 +659,12 @@ pub(crate) fn wire_event_bridges_on_engine( let ai_activity = worker.ai_activity_counter(); let worker = worker.engine_mut(); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_turn_start(move |turn| { let _ = tx.send(Event::TurnStart { turn }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_turn_end(move |turn| { let _ = tx.send(Event::TurnEnd { turn, @@ -672,17 +672,17 @@ pub(crate) fn wire_event_bridges_on_engine( }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_llm_call_start(move |llm_call| { let _ = tx.send(Event::LlmCallStart { llm_call }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_llm_call_end(move |llm_call| { let _ = tx.send(Event::LlmCallEnd { llm_call }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_llm_retry(move |llm_call, notice| { let _ = tx.send(Event::LlmRetry { llm_call, @@ -695,7 +695,7 @@ pub(crate) fn wire_event_bridges_on_engine( }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_llm_continuation(move |llm_call, attempt, max_attempts, reason| { let _ = tx.send(Event::LlmContinuation { llm_call, @@ -768,7 +768,7 @@ pub(crate) fn wire_event_bridges_on_engine( }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); let activity = ai_activity.clone(); worker.on_tool_result(move |result| { activity.fetch_add(1, Ordering::SeqCst); @@ -793,7 +793,7 @@ pub(crate) fn wire_event_bridges_on_engine( }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_usage(move |event| { let _ = tx.send(Event::Usage { input_tokens: event.input_tokens, @@ -802,7 +802,7 @@ pub(crate) fn wire_event_bridges_on_engine( }); }); - let tx = event_tx.clone(); + let tx = working_event_tx.clone(); worker.on_error(move |event| { let _ = tx.send(Event::Error { code: ErrorCode::ProviderError, @@ -1156,7 +1156,7 @@ where async fn controller_loop( mut worker: Worker, mut method_rx: mpsc::Receiver, - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, shared_state: Arc, runtime_dir: Arc, cancel_tx: mpsc::Sender<()>, @@ -1213,7 +1213,7 @@ async fn controller_loop( set_controller_status( &shared_state, &runtime_dir, - &event_tx, + &working_event_tx, WorkerStatus::Running, ) .await; @@ -1230,7 +1230,7 @@ async fn controller_loop( }, ), &mut method_rx, - &event_tx, + &working_event_tx, &cancel_tx, &pause_tx, &shared_state, @@ -1255,7 +1255,7 @@ async fn controller_loop( }, ), &mut method_rx, - &event_tx, + &working_event_tx, &cancel_tx, &pause_tx, &shared_state, @@ -1273,7 +1273,7 @@ async fn controller_loop( drive_turn( worker.run_for_notification(kind), &mut method_rx, - &event_tx, + &working_event_tx, &cancel_tx, &pause_tx, &shared_state, @@ -1291,7 +1291,7 @@ async fn controller_loop( drive_turn( worker.resume(), &mut method_rx, - &event_tx, + &working_event_tx, &cancel_tx, &pause_tx, &shared_state, @@ -1315,16 +1315,16 @@ async fn controller_loop( &mut worker, &shared_state, &runtime_dir, - &event_tx, + &working_event_tx, new_status, ) .await; if shutdown { - let _ = event_tx.send(Event::Shutdown); + let _ = working_event_tx.send(Event::Shutdown); break; } if take_shutdown_request_after_status(&shutdown_after_idle, new_status) { - let _ = event_tx.send(Event::Shutdown); + let _ = working_event_tx.send(Event::Shutdown); break; } continue; @@ -1342,7 +1342,7 @@ async fn controller_loop( // already rejects `Run` while a turn is live, so // this branch is only reachable across a race window // around status flips. - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn".into(), }); @@ -1393,7 +1393,7 @@ async fn controller_loop( Method::Resume => { if shared_state.get_status() != WorkerStatus::Paused { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::NotPaused, message: "Worker is not paused".into(), }); @@ -1409,20 +1409,20 @@ async fn controller_loop( set_controller_status( &shared_state, &runtime_dir, - &event_tx, + &working_event_tx, WorkerStatus::Idle, ) .await; } Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: worker_error_code(&error), message: error.to_string(), }); } }, WorkerStatus::Idle | WorkerStatus::Stopped => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::NotRunning, message: "Worker is not running".into(), }); @@ -1439,7 +1439,7 @@ async fn controller_loop( // Worker is Idle (Running turns go through `drive_turn`, // not this outer match), so there is nothing to pause. if shared_state.get_status() != WorkerStatus::Paused { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::NotRunning, message: "Worker is not running".into(), }); @@ -1449,21 +1449,21 @@ async fn controller_loop( Method::Compact => match shared_state.get_status() { WorkerStatus::Idle => { if let Err(error) = worker.manual_compact().await { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: worker_error_code(&error), message: error.to_string(), }); } } WorkerStatus::Paused => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: "Cannot compact while the Worker is paused; resume or start a fresh turn first" .into(), }); } WorkerStatus::Running | WorkerStatus::Stopped => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn; compact can only run while idle" @@ -1474,10 +1474,10 @@ async fn controller_loop( Method::ListRewindTargets => match shared_state.get_status() { WorkerStatus::Idle | WorkerStatus::Paused => { - emit_rewind_targets(&worker, &event_tx) + emit_rewind_targets(&worker, &working_event_tx) } WorkerStatus::Running | WorkerStatus::Stopped => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn; rewind can only run while idle or paused" .into(), @@ -1490,23 +1490,28 @@ async fn controller_loop( expected_head_entries, } => match shared_state.get_status() { WorkerStatus::Idle => { - if apply_rewind(&mut worker, &event_tx, target, expected_head_entries) { + if apply_rewind( + &mut worker, + &working_event_tx, + target, + expected_head_entries, + ) { worker.clear_in_flight_events(); shared_state.set_status(WorkerStatus::Idle); - let _ = event_tx.send(Event::Status { + let _ = working_event_tx.send(Event::Status { status: WorkerStatus::Idle, }); } } WorkerStatus::Paused => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: "Cannot apply rewind while the Worker is paused; resume or wait for idle first" .into(), }); } WorkerStatus::Running | WorkerStatus::Stopped => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn; rewind can only run while idle or paused" .into(), @@ -1515,24 +1520,24 @@ async fn controller_loop( }, Method::Shutdown => { - let _ = event_tx.send(Event::Shutdown); + let _ = working_event_tx.send(Event::Shutdown); break; } Method::ListWorkers => match discovery.list_visible().await { Ok(workers) => match serde_json::to_value(workers) { Ok(workers) => { - let _ = event_tx.send(Event::WorkersListed { workers }); + let _ = working_event_tx.send(Event::WorkersListed { workers }); } Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::Internal, message: format!("serialize visible workers: {error}"), }); } }, Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: error.to_string(), }); @@ -1542,17 +1547,17 @@ async fn controller_loop( Method::RestoreWorker { name } => match discovery.restore(&name).await { Ok(result) => match serde_json::to_value(result) { Ok(result) => { - let _ = event_tx.send(Event::WorkerRestored { result }); + let _ = working_event_tx.send(Event::WorkerRestored { result }); } Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::Internal, message: format!("serialize worker restore result: {error}"), }); } }, Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: error.to_string(), }); @@ -1562,17 +1567,17 @@ async fn controller_loop( Method::RegisterPeer { name } => match discovery.register_peer(&name) { Ok(result) => match serde_json::to_value(result) { Ok(result) => { - let _ = event_tx.send(Event::PeerRegistered { result }); + let _ = working_event_tx.send(Event::PeerRegistered { result }); } Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::Internal, message: format!("serialize peer registration result: {error}"), }); } }, Err(error) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: error.to_string(), }); @@ -1691,7 +1696,7 @@ async fn handle_inbound_worker_event( async fn drive_turn( worker_future: F, method_rx: &mut mpsc::Receiver, - event_tx: &broadcast::Sender, + working_event_tx: &broadcast::Sender, cancel_tx: &mpsc::Sender<()>, pause_tx: &mpsc::Sender<()>, shared_state: &Arc, @@ -1727,7 +1732,7 @@ where set_controller_status( shared_state, runtime_dir, - event_tx, + working_event_tx, WorkerStatus::Running, ) .await; @@ -1745,11 +1750,11 @@ where WorkerRunResult::LimitReached => (WorkerStatus::Idle, RunResult::LimitReached), WorkerRunResult::RolledBack => (WorkerStatus::Idle, RunResult::RolledBack), WorkerRunResult::Interrupted { .. } if pause_requested => { - let _ = event_tx.send(Event::RunEnd { result: RunResult::Paused }); + let _ = working_event_tx.send(Event::RunEnd { result: RunResult::Paused }); return (WorkerStatus::Paused, shutdown_requested); } WorkerRunResult::Interrupted { code, message } => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code, message: message.clone(), }); @@ -1765,7 +1770,7 @@ where return (WorkerStatus::Idle, shutdown_requested); } }; - let _ = event_tx.send(Event::RunEnd { result: run_result }); + let _ = working_event_tx.send(Event::RunEnd { result: run_result }); if parent_originated && matches!(run_result, RunResult::Finished) { crate::ipc::event::fire_and_forget( parent_socket.cloned(), @@ -1782,13 +1787,13 @@ where // intentionally skip `WorkerEvent::Errored` upward: // that channel is reserved for worker runtime // failures, not deliberate interruptions. - let _ = event_tx.send(Event::RunEnd { result: RunResult::Paused }); + let _ = working_event_tx.send(Event::RunEnd { result: RunResult::Paused }); (WorkerStatus::Paused, shutdown_requested) } Err(e) => { let code = worker_error_code(&e); let message = e.to_string(); - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code, message: message.clone(), }); @@ -1819,13 +1824,13 @@ where let _ = cancel_tx.try_send(()); } Some(Method::Run { .. } | Method::RunTracked { .. } | Method::Resume) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn".into(), }); } Some(Method::Compact | Method::ListRewindTargets | Method::RewindTo { .. }) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn; rewind/compact can only run while idle or paused" .into(), @@ -1839,7 +1844,7 @@ where } Some(Method::ListCompletions { .. }) => {} Some(Method::ListWorkers | Method::RestoreWorker { .. } | Method::RegisterPeer { .. }) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker discovery/control requests are only handled while the Worker is idle or paused" .into(), @@ -1872,20 +1877,20 @@ where } } -fn emit_rewind_targets(worker: &Worker, event_tx: &broadcast::Sender) +fn emit_rewind_targets(worker: &Worker, working_event_tx: &broadcast::Sender) where C: LlmClient + 'static, St: Store, { match worker.list_rewind_targets() { Ok((head_entries, targets)) => { - let _ = event_tx.send(Event::RewindTargets { + let _ = working_event_tx.send(Event::RewindTargets { head_entries, targets, }); } Err(err) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::Internal, message: err.to_string(), }); @@ -1895,7 +1900,7 @@ where fn apply_rewind( worker: &mut Worker, - event_tx: &broadcast::Sender, + working_event_tx: &broadcast::Sender, target: RewindTargetId, expected_head_entries: usize, ) -> bool @@ -1907,7 +1912,7 @@ where Ok(applied) => { let session = session_store::public_snapshot::project_current_session_snapshot(&applied.entries); - let _ = event_tx.send(Event::RewindApplied { + let _ = working_event_tx.send(Event::RewindApplied { session, input: applied.input, summary: applied.summary, @@ -1915,7 +1920,7 @@ where true } Err(err) => { - let _ = event_tx.send(Event::Error { + let _ = working_event_tx.send(Event::Error { code: ErrorCode::InvalidRequest, message: err.to_string(), }); @@ -2049,7 +2054,7 @@ mod tests { // would observe channel-closed and confuse the select! arm. _method_tx: mpsc::Sender, method_rx: mpsc::Receiver, - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, cancel_tx: mpsc::Sender<()>, _cancel_rx: mpsc::Receiver<()>, pause_tx: mpsc::Sender<()>, @@ -2070,7 +2075,7 @@ mod tests { .expect("runtime dir create"), ); let (method_tx, method_rx) = mpsc::channel::(16); - let (event_tx, _) = broadcast::channel::(16); + let (working_event_tx, _) = broadcast::channel::(16); let (cancel_tx, cancel_rx) = mpsc::channel::<()>(1); let (pause_tx, pause_rx) = mpsc::channel::<()>(1); let shared_state = Arc::new(WorkerSharedState::new( @@ -2095,7 +2100,7 @@ mod tests { DriveTurnEnv { _method_tx: method_tx, method_rx, - event_tx, + working_event_tx, cancel_tx, _cancel_rx: cancel_rx, pause_tx, @@ -2157,7 +2162,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2200,7 +2205,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2230,7 +2235,7 @@ mod tests { let (status, _) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2268,7 +2273,7 @@ mod tests { let (status, _) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2312,7 +2317,7 @@ mod tests { let (status, _) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2354,7 +2359,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2393,7 +2398,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2430,7 +2435,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, @@ -2453,7 +2458,7 @@ mod tests { #[tokio::test] async fn compact_method_is_rejected_while_running() { let mut env = make_env().await; - let mut events = env.event_tx.subscribe(); + let mut events = env.working_event_tx.subscribe(); env._method_tx .send(Method::Compact) .await @@ -2466,7 +2471,7 @@ mod tests { let (status, shutdown) = drive_turn( worker_future, &mut env.method_rx, - &env.event_tx, + &env.working_event_tx, &env.cancel_tx, &env.pause_tx, &env.shared_state, diff --git a/crates/worker/src/in_flight.rs b/crates/worker/src/in_flight.rs index 095fffb8..39e1d844 100644 --- a/crates/worker/src/in_flight.rs +++ b/crates/worker/src/in_flight.rs @@ -15,7 +15,7 @@ pub struct InFlightBlockId(u64); #[derive(Debug, Clone)] pub struct InFlightEvents { inner: Arc>, - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, } #[derive(Debug)] @@ -47,14 +47,14 @@ enum TrackedBlock { } impl InFlightEvents { - pub(crate) fn new(event_tx: broadcast::Sender) -> Self { + pub(crate) fn new(working_event_tx: broadcast::Sender) -> Self { Self { inner: Arc::new(Mutex::new(InFlightInner { next_block_id: 1, blocks: Vec::new(), commands: Vec::new(), })), - event_tx, + working_event_tx, } } @@ -84,7 +84,7 @@ impl InFlightEvents { current.push_str(&text); *finished = false; } - let _ = self.event_tx.send(Event::TextDelta { text }); + let _ = self.working_event_tx.send(Event::TextDelta { text }); } pub(crate) fn text_done(&self, block_id: InFlightBlockId, text: String) { @@ -100,7 +100,7 @@ impl InFlightEvents { } *finished = true; } - let _ = self.event_tx.send(Event::TextDone { text }); + let _ = self.working_event_tx.send(Event::TextDone { text }); } pub(crate) fn thinking_start(&self) -> InFlightBlockId { @@ -111,7 +111,7 @@ impl InFlightEvents { text: String::new(), finished: false, }); - let _ = self.event_tx.send(Event::ThinkingStart); + let _ = self.working_event_tx.send(Event::ThinkingStart); block_id } @@ -126,7 +126,7 @@ impl InFlightEvents { current.push_str(&text); *finished = false; } - let _ = self.event_tx.send(Event::ThinkingDelta { text }); + let _ = self.working_event_tx.send(Event::ThinkingDelta { text }); } pub(crate) fn thinking_done(&self, block_id: InFlightBlockId, text: String) { @@ -142,7 +142,7 @@ impl InFlightEvents { } *finished = true; } - let _ = self.event_tx.send(Event::ThinkingDone { text }); + let _ = self.working_event_tx.send(Event::ThinkingDone { text }); } pub(crate) fn tool_call_start(&self, id: String, name: String) -> InFlightBlockId { @@ -155,7 +155,9 @@ impl InFlightEvents { args: String::new(), state: InFlightToolCallState::Pending, }); - let _ = self.event_tx.send(Event::ToolCallStart { id, name }); + let _ = self + .working_event_tx + .send(Event::ToolCallStart { id, name }); block_id } @@ -171,7 +173,7 @@ impl InFlightEvents { *state = InFlightToolCallState::StreamingArgs; } let _ = self - .event_tx + .working_event_tx .send(Event::ToolCallArgsDelta { id, json: delta }); } @@ -191,7 +193,7 @@ impl InFlightEvents { } *state = InFlightToolCallState::Done; } - let _ = self.event_tx.send(Event::ToolCallDone { + let _ = self.working_event_tx.send(Event::ToolCallDone { id, name, arguments: args, @@ -210,7 +212,7 @@ impl InFlightEvents { pub(crate) fn publish_command_event(&self, event: CommandEvent) { self.lock().apply_command_event(&event); - let _ = self.event_tx.send(Event::Command { event }); + let _ = self.working_event_tx.send(Event::Command { event }); } pub(crate) fn replace_command_snapshot(&self, commands: Vec) { @@ -492,13 +494,13 @@ mod tests { #[test] fn snapshot_boundary_does_not_duplicate_or_gap_delta_sent_after_subscribe() { - let (event_tx, _) = broadcast::channel(16); - let in_flight = InFlightEvents::new(event_tx.clone()); + let (working_event_tx, _) = broadcast::channel(16); + let in_flight = InFlightEvents::new(working_event_tx.clone()); let block_id = in_flight.start_text_block(); in_flight.text_delta(block_id, "hel".into()); let guard = in_flight.snapshot_guard(); - let mut rx = event_tx.subscribe(); + let mut rx = working_event_tx.subscribe(); let snapshot = snapshot_from_guard(&guard); drop(guard); @@ -526,9 +528,9 @@ mod tests { use crate::segment_log_sink::SegmentLogSink; use session_store::{LogEntry, LoggedRole}; - let (event_tx, _) = broadcast::channel(16); + let (working_event_tx, _) = broadcast::channel(16); let sink = SegmentLogSink::new(); - let in_flight = InFlightEvents::new(event_tx); + let in_flight = InFlightEvents::new(working_event_tx); let block_id = in_flight.start_text_block(); in_flight.text_delta(block_id, "done".into()); in_flight.text_done(block_id, "done".into()); @@ -580,9 +582,9 @@ mod tests { use crate::segment_log_sink::SegmentLogSink; use session_store::{LogEntry, LoggedRole}; - let (event_tx, _) = broadcast::channel(16); + let (working_event_tx, _) = broadcast::channel(16); let sink = SegmentLogSink::new(); - let in_flight = InFlightEvents::new(event_tx); + let in_flight = InFlightEvents::new(working_event_tx); let block_id = in_flight.start_text_block(); in_flight.text_delta(block_id, "done".into()); in_flight.text_done(block_id, "done".into()); @@ -615,8 +617,8 @@ mod tests { #[test] fn committed_item_clears_matching_in_flight_block() { - let (event_tx, _) = broadcast::channel(16); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let in_flight = InFlightEvents::new(working_event_tx); let block_id = in_flight.start_text_block(); in_flight.text_delta(block_id, "done".into()); in_flight.clear_for_committed_item_then( @@ -635,8 +637,8 @@ mod tests { #[test] fn committed_reasoning_summary_clears_matching_in_flight_thinking_blocks() { - let (event_tx, _) = broadcast::channel(16); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let in_flight = InFlightEvents::new(working_event_tx); let first = in_flight.thinking_start(); in_flight.thinking_delta(first, "summary A".into()); in_flight.thinking_done(first, "".into()); @@ -660,8 +662,8 @@ mod tests { #[test] fn committed_encrypted_only_reasoning_clears_empty_finished_thinking_block() { - let (event_tx, _) = broadcast::channel(16); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let in_flight = InFlightEvents::new(working_event_tx); let first = in_flight.thinking_start(); in_flight.thinking_done(first, "".into()); let second = in_flight.thinking_start(); @@ -689,9 +691,9 @@ mod tests { #[test] fn command_events_are_bounded_and_recoverable_from_snapshot() { - let (event_tx, _) = broadcast::channel(16); - let mut rx = event_tx.subscribe(); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let mut rx = working_event_tx.subscribe(); + let in_flight = InFlightEvents::new(working_event_tx); in_flight.publish_command_event(CommandEvent::Started { command_id: "command-1".into(), tool_call_id: Some("tool-1".into()), @@ -740,9 +742,9 @@ mod tests { #[test] fn clear_discards_uncommitted_blocks_without_protocol_event() { - let (event_tx, _) = broadcast::channel(16); - let mut rx = event_tx.subscribe(); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let mut rx = working_event_tx.subscribe(); + let in_flight = InFlightEvents::new(working_event_tx); let text = in_flight.start_text_block(); in_flight.text_delta(text, "stale".into()); let tool = in_flight.tool_call_start("call-1".into(), "Bash".into()); @@ -770,8 +772,8 @@ mod tests { #[test] fn snapshot_omits_empty_finished_thinking_blocks() { - let (event_tx, _) = broadcast::channel(16); - let in_flight = InFlightEvents::new(event_tx); + let (working_event_tx, _) = broadcast::channel(16); + let in_flight = InFlightEvents::new(working_event_tx); let empty_finished = in_flight.thinking_start(); in_flight.thinking_done(empty_finished, "".into()); let empty_running = in_flight.thinking_start(); diff --git a/crates/worker/src/internal_worker.rs b/crates/worker/src/internal_worker.rs index e033369e..6dc23c51 100644 --- a/crates/worker/src/internal_worker.rs +++ b/crates/worker/src/internal_worker.rs @@ -752,7 +752,7 @@ pub(crate) async fn prepare_internal_worker_session( } let actor_in_flight = in_flight.clone(); worker.attach_alerter(alerter.clone()); - worker.attach_event_tx(event_tx.clone()); + worker.attach_working_event_tx(event_tx.clone()); worker.attach_in_flight_events(in_flight.clone()); wire_event_bridges_on_engine(&mut worker, &event_tx, &alerter, &in_flight); diff --git a/crates/worker/src/ipc/alerter.rs b/crates/worker/src/ipc/alerter.rs index 4ae3d0e0..1ab5f55b 100644 --- a/crates/worker/src/ipc/alerter.rs +++ b/crates/worker/src/ipc/alerter.rs @@ -28,15 +28,15 @@ pub struct Alerter { } struct Inner { - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, buffer: Mutex>, } impl Alerter { - pub fn new(event_tx: broadcast::Sender) -> Self { + pub fn new(working_event_tx: broadcast::Sender) -> Self { Self { inner: Arc::new(Inner { - event_tx, + working_event_tx, buffer: Mutex::new(VecDeque::with_capacity(MAX_BUFFERED_ALERTS)), }), } @@ -66,7 +66,7 @@ impl Alerter { buf.pop_front(); } buf.push_back(alert.clone()); - let _ = self.inner.event_tx.send(Event::Alert(alert)); + let _ = self.inner.working_event_tx.send(Event::Alert(alert)); } } @@ -81,7 +81,7 @@ impl Alerter { .buffer .lock() .expect("alerter buffer mutex poisoned"); - let rx = self.inner.event_tx.subscribe(); + let rx = self.inner.working_event_tx.subscribe(); let snapshot: Vec = buf.iter().cloned().collect(); (snapshot, rx) } diff --git a/crates/worker/src/segment_log_sink.rs b/crates/worker/src/segment_log_sink.rs index 38192af7..91c2792b 100644 --- a/crates/worker/src/segment_log_sink.rs +++ b/crates/worker/src/segment_log_sink.rs @@ -51,7 +51,7 @@ struct SinkInner { /// survives session swaps so existing subscribers keep their /// receiver — they observe the swap as a freshly broadcast /// `LogEntry::AnnotatedSegmentStart` and reset their view accordingly. - broadcast_tx: broadcast::Sender, + session_entry_tx: broadcast::Sender, } impl SegmentLogSink { @@ -59,11 +59,11 @@ impl SegmentLogSink { /// has been written (deferred SegmentStart) or as a placeholder in /// tests. pub fn new() -> Self { - let (broadcast_tx, _) = broadcast::channel(BROADCAST_CAPACITY); + let (session_entry_tx, _) = broadcast::channel(BROADCAST_CAPACITY); Self { inner: Arc::new(SinkInner { mirror: StdMutex::new(Vec::new()), - broadcast_tx, + session_entry_tx, }), } } @@ -72,11 +72,11 @@ impl SegmentLogSink { /// Used by restore / fork-at-restore code paths that materialise /// the existing log before the sink starts taking new commits. pub fn with_initial(entries: Vec) -> Self { - let (broadcast_tx, _) = broadcast::channel(BROADCAST_CAPACITY); + let (session_entry_tx, _) = broadcast::channel(BROADCAST_CAPACITY); Self { inner: Arc::new(SinkInner { mirror: StdMutex::new(entries), - broadcast_tx, + session_entry_tx, }), } } @@ -111,7 +111,7 @@ impl SegmentLogSink { // SendError means there are zero subscribers; harmless. The // mirror lock is held across `send` so subscribers cannot // observe an inconsistent (snapshot, receiver) pair. - let _ = self.inner.broadcast_tx.send(entry); + let _ = self.inner.session_entry_tx.send(entry); } } @@ -144,7 +144,7 @@ impl SegmentLogSink { .expect("session log mirror mutex poisoned"); mirror.clear(); mirror.push(initial.clone()); - let _ = self.inner.broadcast_tx.send(initial); + let _ = self.inner.session_entry_tx.send(initial); } /// Atomically swap the mirror to the supplied replacement-session prefix @@ -161,7 +161,7 @@ impl SegmentLogSink { .expect("session log mirror mutex poisoned"); *mirror = entries; if let Some(initial) = first { - let _ = self.inner.broadcast_tx.send(initial); + let _ = self.inner.session_entry_tx.send(initial); } } @@ -199,7 +199,7 @@ impl SegmentLogSink { .lock() .expect("session log mirror mutex poisoned"); let snapshot = mirror.clone(); - let rx = self.inner.broadcast_tx.subscribe(); + let rx = self.inner.session_entry_tx.subscribe(); (snapshot, rx) } diff --git a/crates/worker/src/spawn/registry.rs b/crates/worker/src/spawn/registry.rs index 2c769f6e..62fec8d4 100644 --- a/crates/worker/src/spawn/registry.rs +++ b/crates/worker/src/spawn/registry.rs @@ -414,10 +414,10 @@ impl SpawnedWorkerRegistry { pub(crate) fn attach_parent_protocol( &self, - event_tx: broadcast::Sender, + working_event_tx: broadcast::Sender, parent_session_id: String, ) { - *self.parent_protocol.lock().unwrap() = Some((event_tx, parent_session_id)); + *self.parent_protocol.lock().unwrap() = Some((working_event_tx, parent_session_id)); for record in self.internal_records.lock().unwrap().clone() { self.start_protocol_forwarding(record); } diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 01788a13..cf2e579e 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -1149,7 +1149,7 @@ pub struct Worker { /// etc.). Attached by the Controller alongside `alerter`. Unlike /// notifications, events sent here are NOT replayed to clients that /// connect after the fact — they are fire-and-forget broadcasts. - event_tx: Option>, + working_event_tx: Option>, /// Parent-owned projection/control boundary for observable Internal service Workers. /// Service Workers are never exposed through the model-facing SubWorker control surface. internal_worker_registry: Option>, @@ -1304,7 +1304,7 @@ impl Worker system_prompt_template: None, feature_instructions: self.feature_instructions.clone(), alerter: self.alerter.clone(), - event_tx: self.event_tx.clone(), + working_event_tx: self.working_event_tx.clone(), internal_worker_registry: self.internal_worker_registry.clone(), in_flight: self.in_flight.clone(), ai_activity_counter: self.ai_activity_counter.clone(), @@ -1497,7 +1497,7 @@ impl Worker { system_prompt_template: None, feature_instructions: Vec::new(), alerter: None, - event_tx: None, + working_event_tx: None, internal_worker_registry: None, in_flight: None, ai_activity_counter: Arc::new(AtomicUsize::new(0)), @@ -2190,13 +2190,13 @@ impl Worker { /// The Controller wires this alongside [`attach_alerter`] so that /// Worker-internal operations (currently: compaction) can surface /// progress to connected clients. - pub fn attach_event_tx(&mut self, event_tx: broadcast::Sender) { + pub fn attach_working_event_tx(&mut self, working_event_tx: broadcast::Sender) { let session_id = self.session_id().to_string(); let registry = self.internal_worker_registry.get_or_insert_with( crate::spawn::registry::SpawnedWorkerRegistry::new_for_internal_services, ); - registry.attach_parent_protocol(event_tx.clone(), session_id); - self.event_tx = Some(event_tx); + registry.attach_parent_protocol(working_event_tx.clone(), session_id); + self.working_event_tx = Some(working_event_tx); } pub(crate) fn attach_internal_worker_registry( @@ -2240,10 +2240,10 @@ impl Worker { } /// Broadcast a typed `Event` to connected clients. No-op when no - /// `event_tx` is attached (tests / direct `Worker::new` usage) or when + /// `working_event_tx` is attached (tests / direct `Worker::new` usage) or when /// no clients are currently subscribed. fn send_event(&self, event: Event) { - if let Some(tx) = self.event_tx.as_ref() { + if let Some(tx) = self.working_event_tx.as_ref() { let _ = tx.send(event); } } @@ -4407,7 +4407,7 @@ impl Worker { .with_memory_settings(&memory_cfg) .emit( self.workspace_client(), - self.event_tx.as_ref(), + self.working_event_tx.as_ref(), memory::audit::WorkerLifecycleStatus::Skipped, "extract_threshold_disabled", None, @@ -4438,7 +4438,7 @@ impl Worker { .with_memory_settings(&memory_cfg) .emit( self.workspace_client(), - self.event_tx.as_ref(), + self.working_event_tx.as_ref(), memory::audit::WorkerLifecycleStatus::Skipped, "extract_already_in_flight", None, @@ -4503,7 +4503,7 @@ impl Worker { Some(model_audit_from_manifest(model)), ) .with_memory_settings(memory_cfg); - let event_tx = self.event_tx.as_ref(); + let working_event_tx = self.working_event_tx.as_ref(); let pointer_snapshot = self .extract_pointer @@ -4519,7 +4519,7 @@ impl Worker { if tokens_since < threshold { audit.emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Skipped, format!( "token_threshold_not_reached tokens_since={tokens_since} threshold={threshold}" @@ -4536,7 +4536,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Skipped, "no_new_history_items", None, @@ -4564,7 +4564,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Skipped, "empty_segment_log", None, @@ -4583,7 +4583,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Skipped, "no_new_segment_entries", None, @@ -4613,7 +4613,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Started, format!( "token_threshold_reached tokens_since={tokens_since} threshold={threshold}" @@ -4637,7 +4637,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Failed, format!("client_build_failed: {err}"), None, @@ -4659,7 +4659,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Failed, format!("prompt_render_failed: {err}"), None, @@ -4737,7 +4737,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Cancelled, "worker_cancelled: internal Worker run rolled back before AI output", usage, @@ -4760,7 +4760,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, lifecycle_status_for_worker_error(&err.source), format!("worker_failed: {}", err.source), usage, @@ -4812,7 +4812,7 @@ impl Worker { audit .emit( self.workspace_client(), - event_tx, + working_event_tx, memory::audit::WorkerLifecycleStatus::Completed, reason, usage, @@ -4847,7 +4847,7 @@ impl Worker { .with_memory_settings(&memory_cfg) .emit( self.workspace_client(), - self.event_tx.as_ref(), + self.working_event_tx.as_ref(), memory::audit::WorkerLifecycleStatus::Skipped, "consolidation_threshold_disabled", None, @@ -4889,7 +4889,7 @@ impl Worker { .with_memory_settings(&memory_cfg) .emit( self.workspace_client(), - self.event_tx.as_ref(), + self.working_event_tx.as_ref(), memory::audit::WorkerLifecycleStatus::Skipped, "consolidation_backend_operation_failed", None, @@ -4942,18 +4942,18 @@ fn model_audit_from_manifest(model: &manifest::ModelManifest) -> memory::audit:: } fn emit_memory_worker_event( - event_tx: Option<&broadcast::Sender>, + working_event_tx: Option<&broadcast::Sender>, run_id: uuid::Uuid, worker: memory::audit::AuditWorker, status: memory::audit::WorkerLifecycleStatus, trigger: memory::audit::AuditTrigger, reason: &str, ) { - let Some(event_tx) = event_tx else { + let Some(working_event_tx) = working_event_tx else { return; }; let message = format!("memory {} {}: {reason}", worker.label(), status.label()); - let _ = event_tx.send(Event::MemoryWorker(protocol::MemoryWorkerEvent { + let _ = working_event_tx.send(Event::MemoryWorker(protocol::MemoryWorkerEvent { worker: worker.label().to_string(), status: status.label().to_string(), run_id: run_id.to_string(), @@ -5003,7 +5003,7 @@ impl WorkerAuditBase { async fn emit( &self, workspace_client: &dyn WorkspaceClient, - event_tx: Option<&broadcast::Sender>, + working_event_tx: Option<&broadcast::Sender>, status: memory::audit::WorkerLifecycleStatus, reason: impl Into, usage: Option, @@ -5034,7 +5034,7 @@ impl WorkerAuditBase { .await; if should_emit_memory_worker_event(self.worker, status, &reason) { emit_memory_worker_event( - event_tx, + working_event_tx, self.run_id, self.worker, status, @@ -5218,7 +5218,7 @@ where system_prompt_template: common.system_prompt_template, feature_instructions: common.feature_instructions, alerter: None, - event_tx: None, + working_event_tx: None, internal_worker_registry: None, in_flight: None, ai_activity_counter: Arc::new(AtomicUsize::new(0)), @@ -5302,7 +5302,7 @@ where system_prompt_template: common.system_prompt_template, feature_instructions: common.feature_instructions, alerter: None, - event_tx: None, + working_event_tx: None, internal_worker_registry: None, in_flight: None, ai_activity_counter: Arc::new(AtomicUsize::new(0)), @@ -5421,7 +5421,7 @@ where system_prompt_template: common.system_prompt_template, feature_instructions: common.feature_instructions, alerter: None, - event_tx: None, + working_event_tx: None, internal_worker_registry: None, in_flight: None, ai_activity_counter: Arc::new(AtomicUsize::new(0)), @@ -5797,7 +5797,7 @@ where system_prompt_template: None, feature_instructions: common.feature_instructions, alerter: None, - event_tx: None, + working_event_tx: None, internal_worker_registry: None, in_flight: None, ai_activity_counter: Arc::new(AtomicUsize::new(0)), diff --git a/crates/worker/tests/compact_events_test.rs b/crates/worker/tests/compact_events_test.rs index e67519fe..708fd60a 100644 --- a/crates/worker/tests/compact_events_test.rs +++ b/crates/worker/tests/compact_events_test.rs @@ -382,7 +382,7 @@ async fn compact_emits_session_start_carrying_summary_and_task_snapshot() { let mut worker = make_worker(client).await; let (tx, _rx_keep) = broadcast::channel::(64); - worker.attach_event_tx(tx); + worker.attach_working_event_tx(tx); worker.run_text("first").await.unwrap(); let session_id = worker.session_id(); @@ -429,7 +429,7 @@ async fn pre_run_compact_success_broadcasts_start_and_done() { let mut worker = make_worker(client).await; let (tx, mut rx) = broadcast::channel::(64); - worker.attach_event_tx(tx); + worker.attach_working_event_tx(tx); worker.run_text("first").await.unwrap(); // Drain run events so only compact events remain in `rx`. @@ -539,7 +539,7 @@ async fn mid_turn_compact_success_broadcasts_start_and_done() { let mut worker = make_worker_with_manifest(MID_TURN_MANIFEST_TOML, client).await; let (tx, mut rx) = broadcast::channel::(64); - worker.attach_event_tx(tx); + worker.attach_working_event_tx(tx); // First run populates usage_history above the request threshold. worker.run_text("first").await.unwrap(); @@ -718,7 +718,7 @@ async fn pre_run_compact_failure_broadcasts_start_and_failed() { let mut worker = make_worker(client).await; let (tx, mut rx) = broadcast::channel::(64); - worker.attach_event_tx(tx); + worker.attach_working_event_tx(tx); worker.run_text("first").await.unwrap(); let _ = drain(&mut rx); diff --git a/crates/worker/tests/controller_test.rs b/crates/worker/tests/controller_test.rs index 364440b2..968f8292 100644 --- a/crates/worker/tests/controller_test.rs +++ b/crates/worker/tests/controller_test.rs @@ -1515,7 +1515,7 @@ async fn notify_while_idle_auto_starts_turn_and_injects_system_message() { // after that. wait_for_status(&handle, WorkerStatus::Idle).await; // The live echo arrives via the sink's `Event::SystemItem` lane, - // not on the `event_tx` broadcast that `handle.subscribe()` taps. + // not on the `working_event_tx` broadcast that `handle.subscribe()` taps. // Verify the notification landed on the sink mirror instead. let (entries, _) = handle.sink.subscribe_with_snapshot(); let saw_notify_in_mirror = entries.iter().any(|e| { @@ -1899,7 +1899,7 @@ async fn socket_worker_event_turn_ended_while_idle_auto_starts_turn() { let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); // The SystemItem and TurnEnd events arrive through independent - // broadcast lanes (sink fan-out vs `event_tx`), so their relative + // broadcast lanes (sink fan-out vs `working_event_tx`), so their relative // order on the wire is non-deterministic. Keep reading until both // are observed (or the deadline trips), rather than breaking on // the first TurnEnd. From a7f09fad9840397a684f250a4ebf5443fdceee2e Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 12:11:35 +0900 Subject: [PATCH 2/7] fix: route standalone protocol through client transports --- Cargo.lock | 2 + crates/client/Cargo.toml | 1 + crates/client/src/backend_runtime.rs | 187 +++------------------ crates/client/src/client.rs | 137 +++++++++++++++ crates/client/src/lib.rs | 11 +- crates/client/src/transport/in_process.rs | 115 +++++++++++++ crates/client/src/transport/mod.rs | 22 +++ crates/client/src/transport/unix_socket.rs | 172 +++++++++++++++++++ crates/client/src/transport/websocket.rs | 140 +++++++++++++++ crates/client/src/worker_client.rs | 186 -------------------- crates/standalone/Cargo.toml | 1 + crates/standalone/src/host.rs | 141 +++++++++++++--- crates/standalone/src/lib.rs | 4 +- crates/standalone/tests/host.rs | 65 +++++-- crates/tui/src/console/mod.rs | 112 +++++------- 15 files changed, 825 insertions(+), 471 deletions(-) create mode 100644 crates/client/src/client.rs create mode 100644 crates/client/src/transport/in_process.rs create mode 100644 crates/client/src/transport/mod.rs create mode 100644 crates/client/src/transport/unix_socket.rs create mode 100644 crates/client/src/transport/websocket.rs delete mode 100644 crates/client/src/worker_client.rs diff --git a/Cargo.lock b/Cargo.lock index 28870bb0..4b2f4b62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -637,6 +637,7 @@ checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" name = "client" version = "0.1.0" dependencies = [ + "async-trait", "chrono", "futures", "protocol", @@ -4622,6 +4623,7 @@ version = "0.1.0" dependencies = [ "agen", "async-trait", + "client", "fs4", "futures", "manifest", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index a17997e7..e3475d33 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +async-trait.workspace = true chrono = { version = "0.4", default-features = false, features = ["clock"] } protocol = { workspace = true } ticket = { workspace = true } diff --git a/crates/client/src/backend_runtime.rs b/crates/client/src/backend_runtime.rs index 8cb67625..78f61833 100644 --- a/crates/client/src/backend_runtime.rs +++ b/crates/client/src/backend_runtime.rs @@ -1,13 +1,7 @@ -use crate::{BackendApiClient, BackendApiClientError}; -use futures::{SinkExt, StreamExt}; -use protocol::stream::{decode_event, encode_method}; -use protocol::{ErrorCode, Event, Method}; +use crate::transport::websocket::{Socket as WebSocket, SocketError as WebSocketError}; +use crate::{BackendApiClient, BackendApiClientError, Client}; use reqwest::Method as HttpMethod; -use std::collections::VecDeque; use std::fmt; -use tokio::sync::mpsc; -use tokio_tungstenite::connect_async; -use tokio_tungstenite::tungstenite::Message as TungsteniteMessage; use tokio_tungstenite::tungstenite::client::IntoClientRequest; use tokio_tungstenite::tungstenite::http::HeaderValue; use tokio_tungstenite::tungstenite::http::header::AUTHORIZATION; @@ -106,20 +100,12 @@ impl BackendRuntimeListTarget { } } -#[derive(Debug)] -pub struct BackendRuntimeClient { - target: BackendRuntimeTarget, - command_tx: mpsc::UnboundedSender, - events: mpsc::UnboundedReceiver, - diagnostics: VecDeque, - _protocol_task: tokio::task::JoinHandle<()>, -} - #[derive(Debug)] pub enum BackendRuntimeClientError { InvalidTarget(String), Api(BackendApiClientError), Http(reqwest::Error), + Protocol(String), } impl fmt::Display for BackendRuntimeClientError { @@ -128,6 +114,7 @@ impl fmt::Display for BackendRuntimeClientError { Self::InvalidTarget(message) => f.write_str(message), Self::Api(error) => write!(f, "{error}"), Self::Http(error) => write!(f, "{error}"), + Self::Protocol(message) => f.write_str(message), } } } @@ -282,151 +269,22 @@ pub async fn restore_backend_worker( Ok(response.json::().await?) } -impl BackendRuntimeClient { - pub async fn connect(target: BackendRuntimeTarget) -> Result { - validate_target(&target)?; - let api = BackendApiClient::from_stored_token(&target.base_url)?; - let (event_tx, rx) = mpsc::unbounded_channel(); - let (command_tx, command_rx) = mpsc::unbounded_channel(); - - let protocol_target = target.clone(); - let protocol_event_tx = event_tx.clone(); - let protocol_task = tokio::spawn(async move { - run_worker_protocol_transport(protocol_target, api, command_rx, protocol_event_tx) - .await; - }); - - Ok(Self { - target, - command_tx, - events: rx, - diagnostics: VecDeque::new(), - _protocol_task: protocol_task, - }) - } - - pub fn try_next_event(&mut self) -> Option { - if let Some(event) = self.diagnostics.pop_front() { - return Some(event); - } - self.events.try_recv().ok() - } - - pub async fn next_event(&mut self) -> Option { - if let Some(event) = self.diagnostics.pop_front() { - return Some(event); - } - self.events.recv().await - } - - pub async fn send(&mut self, method: &Method) -> Result<(), BackendRuntimeClientError> { - self.command_tx.send(method.clone()).map_err(|_| { - BackendRuntimeClientError::InvalidTarget(format!( - "Backend protocol command stream is closed for {}", - self.target.display_label() - )) - })?; - Ok(()) - } -} - -impl Drop for BackendRuntimeClient { - fn drop(&mut self) { - self._protocol_task.abort(); - } -} - -async fn run_worker_protocol_transport( +pub async fn connect_backend_runtime( target: BackendRuntimeTarget, - api: BackendApiClient, - mut commands: mpsc::UnboundedReceiver, - tx: mpsc::UnboundedSender, -) { - let request = match protocol_ws_request(&target, &api) { - Ok(request) => request, - Err(error) => { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol request could not be constructed for {}: {error}", - target.display_label() - ))); - return; - } - }; - match connect_async(request).await { - Ok((ws, _)) => { - let (mut sink, mut stream) = ws.split(); - loop { - tokio::select! { - maybe_method = commands.recv() => { - let Some(method) = maybe_method else { - break; - }; - match encode_method(&method) { - Ok(text) => { - if let Err(error) = sink.send(TungsteniteMessage::Text(text.into())).await { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol command send failed for {}: {error}", - target.display_label() - ))); - break; - } - } - Err(error) => { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol command could not serialize method for {}: {error}", - target.display_label() - ))); - } - } - } - frame = stream.next() => { - match frame { - Some(Ok(TungsteniteMessage::Text(text))) => { - match decode_event(&text) { - Ok(event) => { - let _ = tx.send(event); - } - Err(error) => { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol response was not valid Event JSON for {}: {error}", - target.display_label() - ))); - } - } - } - Some(Ok(TungsteniteMessage::Close(_))) | None => { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol command stream closed for {}", - target.display_label() - ))); - break; - } - Some(Ok(TungsteniteMessage::Ping(_))) - | Some(Ok(TungsteniteMessage::Pong(_))) - | Some(Ok(TungsteniteMessage::Binary(_))) - | Some(Ok(TungsteniteMessage::Frame(_))) => {} - Some(Err(error)) => { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol WebSocket error for {}: {error}", - target.display_label() - ))); - break; - } - } - } - } - } - } - Err(error) => { - let message = protocol_connect_error_message(&target, &api, &error); - let _ = tx.send(diagnostic_event(message)); - while commands.recv().await.is_some() { - let _ = tx.send(diagnostic_event(format!( - "Backend protocol command was not sent because command stream is unavailable for {}", - target.display_label() - ))); - } - } +) -> Result, BackendRuntimeClientError> { + validate_target(&target)?; + let api = BackendApiClient::from_stored_token(&target.base_url)?; + let request = protocol_ws_request(&target, &api).map_err(|error| { + BackendRuntimeClientError::Protocol(format!( + "Backend protocol request could not be constructed for {}: {error}", + target.display_label() + )) + })?; + match WebSocket::connect(request).await { + Ok(socket) => Ok(Client::new(socket)), + Err(WebSocketError::WebSocket(error)) => Err(BackendRuntimeClientError::Protocol( + protocol_connect_error_message(&target, &api, &error), + )), } } @@ -453,13 +311,6 @@ fn protocol_connect_error_message( ) } -fn diagnostic_event(message: impl Into) -> Event { - Event::Error { - code: ErrorCode::Internal, - message: message.into(), - } -} - fn validate_target(target: &BackendRuntimeTarget) -> Result<(), BackendRuntimeClientError> { if target.base_url.trim().is_empty() { return Err(BackendRuntimeClientError::InvalidTarget( diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs new file mode 100644 index 00000000..517d6a7c --- /dev/null +++ b/crates/client/src/client.rs @@ -0,0 +1,137 @@ +use std::error::Error; +use std::fmt; + +use protocol::stream::{decode_event, encode_method}; +use protocol::{Event, Method}; + +use crate::transport::Socket; + +/// Typed Worker protocol client over an injected message transport. +pub struct Client { + socket: T, +} + +#[derive(Debug)] +pub enum ClientError { + Transport(E), + Protocol(serde_json::Error), +} + +impl Client { + pub fn new(socket: T) -> Self { + Self { socket } + } + + pub fn into_inner(self) -> T { + self.socket + } +} + +impl Client { + pub async fn send(&mut self, method: &Method) -> Result<(), ClientError> { + let message = encode_method(method).map_err(ClientError::Protocol)?; + self.socket + .send(message) + .await + .map_err(ClientError::Transport) + } + + pub async fn next_event(&mut self) -> Result, ClientError> { + self.socket + .next() + .await + .map_err(ClientError::Transport)? + .map(|message| decode_event(&message).map_err(ClientError::Protocol)) + .transpose() + } + + pub fn try_next_event(&mut self) -> Result, ClientError> { + self.socket + .try_next() + .map_err(ClientError::Transport)? + .map(|message| decode_event(&message).map_err(ClientError::Protocol)) + .transpose() + } +} + +impl fmt::Display for ClientError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Transport(error) => write!(formatter, "Worker transport error: {error}"), + Self::Protocol(error) => write!(formatter, "Worker protocol error: {error}"), + } + } +} + +impl Error for ClientError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Transport(error) => Some(error), + Self::Protocol(error) => Some(error), + } + } +} + +#[cfg(test)] +mod tests { + use std::collections::VecDeque; + use std::convert::Infallible; + + use async_trait::async_trait; + use protocol::stream::{decode_method, encode_event}; + use protocol::{Event, Method, WorkerStatus}; + + use super::Client; + use crate::transport::Socket; + + #[derive(Default)] + struct TestSocket { + sent: Vec, + incoming: VecDeque, + } + + #[async_trait] + impl Socket for TestSocket { + type Error = Infallible; + + async fn send(&mut self, message: String) -> Result<(), Self::Error> { + self.sent.push(message); + Ok(()) + } + + async fn next(&mut self) -> Result, Self::Error> { + Ok(self.incoming.pop_front()) + } + + fn try_next(&mut self) -> Result, Self::Error> { + Ok(self.incoming.pop_front()) + } + } + + #[tokio::test] + async fn encodes_methods_and_decodes_events_above_transport() { + let mut socket = TestSocket::default(); + socket.incoming.push_back( + encode_event(&Event::Status { + status: WorkerStatus::Idle, + }) + .expect("encode event"), + ); + let mut client = Client::new(socket); + + client + .send(&Method::run_text("hello")) + .await + .expect("send method"); + assert!(matches!( + decode_method(&client.socket.sent[0]), + Ok(Method::Run { .. }) + )); + assert!(matches!( + client.next_event().await, + Ok(Some(Event::Status { + status: WorkerStatus::Idle + })) + )); + } +} diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index b373747f..6aae3f6a 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -7,8 +7,9 @@ pub mod backend_api; mod backend_auth; pub mod backend_runtime; pub mod backend_workspace; +mod client; pub mod target; -mod worker_client; +pub mod transport; mod workspace_product; pub use backend_api::{ @@ -20,23 +21,23 @@ pub use backend_auth::{ poll_device_login, start_device_login, wait_for_device_login, }; pub use backend_runtime::{ - BackendDiagnostic, BackendDiagnosticSeverity, BackendRuntimeClient, BackendRuntimeClientError, + BackendDiagnostic, BackendDiagnosticSeverity, BackendRuntimeClientError, BackendRuntimeListResponse, BackendRuntimeListTarget, BackendRuntimeSummary, BackendRuntimeTarget, BackendWorkerCapabilitySummary, BackendWorkerImplementationSummary, BackendWorkerRestoreResponse, BackendWorkerRestoreResult, BackendWorkerSummary, - BackendWorkerWorkspaceSummary, BackendWorkingDirectorySummary, list_backend_stopped_workers, - list_backend_workers, restore_backend_worker, + BackendWorkerWorkspaceSummary, BackendWorkingDirectorySummary, connect_backend_runtime, + list_backend_stopped_workers, list_backend_workers, restore_backend_worker, }; pub use backend_workspace::{ BackendWorkspace, BackendWorkspaceCatalogTarget, BackendWorkspaceClientError, CreateBackendWorkspaceRepository, CreateBackendWorkspaceRequest, CreateBackendWorkspaceResponse, create_backend_workspace, list_backend_workspaces, }; +pub use client::{Client, ClientError}; pub use target::{ BackendTarget, Dashboard, ResolvedTarget, StandaloneSessionListIntent, StandaloneSessionResumeIntent, StandaloneTarget, Target, TargetError, TargetKind, WorkerConnection, WorkerConnectionSelector, WorkerList, WorkerListRequest, WorkerSpawn, }; -pub use worker_client::WorkerClient; pub use workspace_api::{ObjectiveDetail, ObjectiveSummary}; pub use workspace_product::BackendWorkspaceProductClient; diff --git a/crates/client/src/transport/in_process.rs b/crates/client/src/transport/in_process.rs new file mode 100644 index 00000000..b1db8e6b --- /dev/null +++ b/crates/client/src/transport/in_process.rs @@ -0,0 +1,115 @@ +use async_trait::async_trait; +use thiserror::Error; +use tokio::sync::mpsc; + +use super::Socket as SocketContract; + +const CHANNEL_CAPACITY: usize = 256; + +pub struct Socket { + outgoing: mpsc::Sender, + incoming: mpsc::Receiver, +} + +/// Host-side endpoint paired with an in-process client transport. +pub struct Peer { + incoming: mpsc::Receiver, + outgoing: mpsc::Sender, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] +pub enum SocketError { + #[error("in-process Worker protocol transport closed")] + Closed, +} + +impl Socket { + pub fn pair() -> (Self, Peer) { + let (client_tx, peer_rx) = mpsc::channel(CHANNEL_CAPACITY); + let (peer_tx, client_rx) = mpsc::channel(CHANNEL_CAPACITY); + ( + Self { + outgoing: client_tx, + incoming: client_rx, + }, + Peer { + incoming: peer_rx, + outgoing: peer_tx, + }, + ) + } +} + +#[async_trait] +impl SocketContract for Socket { + type Error = SocketError; + + async fn send(&mut self, message: String) -> Result<(), Self::Error> { + self.outgoing + .send(message) + .await + .map_err(|_| SocketError::Closed) + } + + async fn next(&mut self) -> Result, Self::Error> { + Ok(self.incoming.recv().await) + } + + fn try_next(&mut self) -> Result, Self::Error> { + match self.incoming.try_recv() { + Ok(message) => Ok(Some(message)), + Err(mpsc::error::TryRecvError::Empty | mpsc::error::TryRecvError::Disconnected) => { + Ok(None) + } + } + } +} + +impl Peer { + pub async fn next(&mut self) -> Option { + self.incoming.recv().await + } + + pub async fn send(&self, message: String) -> Result<(), String> { + self.outgoing.send(message).await.map_err(|error| error.0) + } +} + +#[cfg(test)] +mod tests { + use protocol::stream::{decode_method, encode_event}; + use protocol::{Event, Method, WorkerStatus}; + + use super::Socket; + use crate::Client; + + #[tokio::test] + async fn pair_carries_typed_protocol_through_generic_client() { + let (socket, mut peer) = Socket::pair(); + let mut client = Client::new(socket); + + client + .send(&Method::run_text("hello")) + .await + .expect("send method"); + assert!(matches!( + peer.next().await.as_deref().map(decode_method), + Some(Ok(Method::Run { .. })) + )); + + peer.send( + encode_event(&Event::Status { + status: WorkerStatus::Idle, + }) + .expect("encode event"), + ) + .await + .expect("send event"); + assert!(matches!( + client.next_event().await, + Ok(Some(Event::Status { + status: WorkerStatus::Idle + })) + )); + } +} diff --git a/crates/client/src/transport/mod.rs b/crates/client/src/transport/mod.rs new file mode 100644 index 00000000..12d5e6d2 --- /dev/null +++ b/crates/client/src/transport/mod.rs @@ -0,0 +1,22 @@ +use std::error::Error; + +use async_trait::async_trait; + +pub mod in_process; +pub mod unix_socket; +pub mod websocket; + +/// Message-oriented transport for one Worker protocol connection. +/// +/// Implementations own physical framing. `client::Client` owns the typed +/// Method/Event protocol encoding layered on top of these UTF-8 messages. +#[async_trait] +pub trait Socket { + type Error: Error + Send + Sync + 'static; + + async fn send(&mut self, message: String) -> Result<(), Self::Error>; + + async fn next(&mut self) -> Result, Self::Error>; + + fn try_next(&mut self) -> Result, Self::Error>; +} diff --git a/crates/client/src/transport/unix_socket.rs b/crates/client/src/transport/unix_socket.rs new file mode 100644 index 00000000..0262bff2 --- /dev/null +++ b/crates/client/src/transport/unix_socket.rs @@ -0,0 +1,172 @@ +use std::io; +use std::path::Path; + +use async_trait::async_trait; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::net::UnixStream; +use tokio::sync::mpsc; +use tokio::task::JoinHandle; + +use super::Socket as SocketContract; + +pub struct Socket { + writer: tokio::io::WriteHalf, + messages: mpsc::Receiver>, + reader_task: JoinHandle<()>, +} + +impl Socket { + pub async fn connect(path: &Path) -> io::Result { + let stream = UnixStream::connect(path).await?; + let (reader, writer) = tokio::io::split(stream); + let (message_tx, messages) = mpsc::channel(256); + let reader_task = tokio::spawn(async move { + let mut lines = BufReader::new(reader).lines(); + loop { + match lines.next_line().await { + Ok(Some(message)) if message.trim().is_empty() => {} + Ok(Some(message)) => { + if message_tx.send(Ok(message)).await.is_err() { + return; + } + } + Ok(None) => return, + Err(error) => { + let _ = message_tx.send(Err(error)).await; + return; + } + } + } + }); + Ok(Self { + writer, + messages, + reader_task, + }) + } +} + +#[async_trait] +impl SocketContract for Socket { + type Error = io::Error; + + async fn send(&mut self, message: String) -> Result<(), Self::Error> { + self.writer.write_all(message.as_bytes()).await?; + self.writer.write_all(b"\n").await?; + self.writer.flush().await + } + + async fn next(&mut self) -> Result, Self::Error> { + match self.messages.recv().await { + Some(message) => message.map(Some), + None => Ok(None), + } + } + + fn try_next(&mut self) -> Result, Self::Error> { + match self.messages.try_recv() { + Ok(message) => message.map(Some), + Err(mpsc::error::TryRecvError::Empty | mpsc::error::TryRecvError::Disconnected) => { + Ok(None) + } + } + } +} + +impl Drop for Socket { + fn drop(&mut self) { + self.reader_task.abort(); + } +} + +#[cfg(test)] +mod tests { + use std::io::ErrorKind; + use std::time::Duration; + + use protocol::stream::{decode_method, encode_event}; + use protocol::{Event, Method, WorkerStatus}; + use tempfile::tempdir; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::UnixListener; + + use super::*; + use crate::Client; + + async fn assert_peer_closed(stream: &mut UnixStream, reason: &str) { + let mut buf = [0_u8; 1]; + match tokio::time::timeout(Duration::from_secs(1), stream.read(&mut buf)) + .await + .expect(reason) + { + Ok(0) => {} + Err(error) if error.kind() == ErrorKind::ConnectionReset => {} + Ok(n) => panic!("server should observe peer close, read {n} byte(s)"), + Err(error) => panic!("server read failed unexpectedly: {error}"), + } + } + + #[tokio::test] + async fn client_receives_events_over_unix_socket() { + let socket_dir = tempdir().unwrap(); + let socket_path = socket_dir.path().join("events.sock"); + let listener = UnixListener::bind(&socket_path).unwrap(); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let event = encode_event(&Event::Status { + status: WorkerStatus::Idle, + }) + .unwrap(); + stream.write_all(event.as_bytes()).await.unwrap(); + stream.write_all(b"\n").await.unwrap(); + }); + + let mut client = Client::new(Socket::connect(&socket_path).await.unwrap()); + let event = tokio::time::timeout(Duration::from_secs(1), client.next_event()) + .await + .expect("client should receive event while alive") + .expect("transport should succeed"); + assert!(matches!( + event, + Some(Event::Status { + status: WorkerStatus::Idle + }) + )); + server.await.unwrap(); + } + + #[tokio::test] + async fn client_sends_methods_over_unix_socket() { + let socket_dir = tempdir().unwrap(); + let socket_path = socket_dir.path().join("send.sock"); + let listener = UnixListener::bind(&socket_path).unwrap(); + let server = tokio::spawn(async move { + let (reader, _) = listener.accept().await.unwrap(); + BufReader::new(reader).lines().next_line().await.unwrap() + }); + + let mut client = Client::new(Socket::connect(&socket_path).await.unwrap()); + client + .send(&Method::run_text("hello")) + .await + .expect("send method"); + + let received = server.await.unwrap().expect("method message"); + assert!(matches!(decode_method(&received), Ok(Method::Run { .. }))); + } + + #[tokio::test] + async fn dropping_socket_closes_server_connection() { + let socket_dir = tempdir().unwrap(); + let socket_path = socket_dir.path().join("drop.sock"); + let listener = UnixListener::bind(&socket_path).unwrap(); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + assert_peer_closed(&mut stream, "dropped socket should close promptly").await; + }); + + let socket = Socket::connect(&socket_path).await.unwrap(); + drop(socket); + server.await.unwrap(); + } +} diff --git a/crates/client/src/transport/websocket.rs b/crates/client/src/transport/websocket.rs new file mode 100644 index 00000000..e8640573 --- /dev/null +++ b/crates/client/src/transport/websocket.rs @@ -0,0 +1,140 @@ +use async_trait::async_trait; +use futures::{SinkExt, StreamExt}; +use thiserror::Error; +use tokio::net::TcpStream; +use tokio::sync::mpsc; +use tokio::task::JoinHandle; +use tokio_tungstenite::tungstenite::http::Request; +use tokio_tungstenite::tungstenite::{self, Message}; +use tokio_tungstenite::{MaybeTlsStream, WebSocketStream, connect_async}; + +use super::Socket as SocketContract; + +type Writer = futures::stream::SplitSink>, Message>; + +pub struct Socket { + writer: Writer, + messages: mpsc::Receiver>, + reader_task: JoinHandle<()>, +} + +#[derive(Debug, Error)] +pub enum SocketError { + #[error("WebSocket transport failed: {0}")] + WebSocket(#[from] tungstenite::Error), +} + +impl Socket { + pub async fn connect(request: Request<()>) -> Result { + let (stream, _) = connect_async(request).await?; + let (writer, mut reader) = stream.split(); + let (message_tx, messages) = mpsc::channel(256); + let reader_task = tokio::spawn(async move { + loop { + match reader.next().await { + Some(Ok(Message::Text(message))) => { + if message_tx.send(Ok(message.to_string())).await.is_err() { + return; + } + } + Some(Ok(Message::Close(_))) | None => return, + Some(Ok( + Message::Binary(_) + | Message::Ping(_) + | Message::Pong(_) + | Message::Frame(_), + )) => {} + Some(Err(error)) => { + let _ = message_tx.send(Err(SocketError::WebSocket(error))).await; + return; + } + } + } + }); + Ok(Self { + writer, + messages, + reader_task, + }) + } +} + +#[async_trait] +impl SocketContract for Socket { + type Error = SocketError; + + async fn send(&mut self, message: String) -> Result<(), Self::Error> { + self.writer.send(Message::Text(message.into())).await?; + Ok(()) + } + + async fn next(&mut self) -> Result, Self::Error> { + match self.messages.recv().await { + Some(message) => message.map(Some), + None => Ok(None), + } + } + + fn try_next(&mut self) -> Result, Self::Error> { + match self.messages.try_recv() { + Ok(message) => message.map(Some), + Err(mpsc::error::TryRecvError::Empty | mpsc::error::TryRecvError::Disconnected) => { + Ok(None) + } + } + } +} + +impl Drop for Socket { + fn drop(&mut self) { + self.reader_task.abort(); + } +} + +#[cfg(test)] +mod tests { + use futures::{SinkExt, StreamExt}; + use protocol::stream::{decode_method, encode_event}; + use protocol::{Event, Method, WorkerStatus}; + use tokio::net::TcpListener; + use tokio_tungstenite::accept_async; + use tokio_tungstenite::tungstenite::client::IntoClientRequest; + + use super::*; + use crate::Client; + + #[tokio::test] + async fn carries_typed_protocol_through_generic_client() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.unwrap(); + let mut socket = accept_async(stream).await.unwrap(); + let message = socket.next().await.unwrap().unwrap(); + assert!(matches!( + message, + Message::Text(ref text) + if matches!(decode_method(text), Ok(Method::Run { .. })) + )); + let event = encode_event(&Event::Status { + status: WorkerStatus::Idle, + }) + .unwrap(); + socket.send(Message::Text(event.into())).await.unwrap(); + }); + + let request = format!("ws://{address}").into_client_request().unwrap(); + let mut client = Client::new(Socket::connect(request).await.unwrap()); + client + .send(&Method::run_text("hello")) + .await + .expect("send method"); + assert!(matches!( + client.next_event().await, + Ok(Some(Event::Status { + status: WorkerStatus::Idle + })) + )); + server.await.unwrap(); + } +} diff --git a/crates/client/src/worker_client.rs b/crates/client/src/worker_client.rs deleted file mode 100644 index a0661f41..00000000 --- a/crates/client/src/worker_client.rs +++ /dev/null @@ -1,186 +0,0 @@ -use std::io; -use std::path::Path; - -use protocol::stream::{JsonLineReader, JsonLineWriter}; -use protocol::{Event, Method}; -use tokio::net::UnixStream; -use tokio::sync::mpsc; -use tokio::task::JoinHandle; - -pub struct WorkerClient { - writer: JsonLineWriter>, - event_rx: mpsc::Receiver, - reader_task: JoinHandle<()>, -} - -impl WorkerClient { - pub async fn connect(path: &Path) -> Result { - let stream = UnixStream::connect(path).await?; - let (reader, writer) = tokio::io::split(stream); - let writer = JsonLineWriter::new(writer); - - let (event_tx, event_rx) = mpsc::channel::(256); - - let reader_task = tokio::spawn(async move { - let mut reader = JsonLineReader::new(reader); - while let Ok(Some(event)) = reader.next::().await { - if event_tx.send(event).await.is_err() { - break; - } - } - }); - - Ok(Self { - writer, - event_rx, - reader_task, - }) - } - - pub async fn send(&mut self, method: &Method) -> Result<(), io::Error> { - self.writer.write(method).await - } - - pub fn try_next_event(&mut self) -> Option { - self.event_rx.try_recv().ok() - } - - pub async fn next_event(&mut self) -> Option { - self.event_rx.recv().await - } -} - -impl Drop for WorkerClient { - fn drop(&mut self) { - self.reader_task.abort(); - } -} - -#[cfg(test)] -mod tests { - use std::io::ErrorKind; - use std::time::Duration; - - use protocol::{Segment, WorkerStatus}; - use tempfile::tempdir; - use tokio::io::{AsyncReadExt, AsyncWriteExt}; - use tokio::net::UnixListener; - - use super::*; - - async fn assert_peer_closed(stream: &mut UnixStream, reason: &str) { - let mut buf = [0_u8; 1]; - match tokio::time::timeout(Duration::from_secs(1), stream.read(&mut buf)) - .await - .expect(reason) - { - Ok(0) => {} - Err(error) if error.kind() == ErrorKind::ConnectionReset => {} - Ok(n) => panic!("server should observe peer close, read {n} byte(s)"), - Err(error) => panic!("server read failed unexpectedly: {error}"), - } - } - - #[tokio::test] - async fn receives_events_while_client_is_alive() { - let socket_dir = tempdir().unwrap(); - let socket_path = socket_dir.path().join("events.sock"); - let listener = UnixListener::bind(&socket_path).unwrap(); - let server = tokio::spawn(async move { - let (stream, _) = listener.accept().await.unwrap(); - let mut writer = JsonLineWriter::new(stream); - writer - .write(&Event::Status { - status: WorkerStatus::Idle, - }) - .await - .unwrap(); - }); - - let mut client = WorkerClient::connect(&socket_path).await.unwrap(); - - let event = tokio::time::timeout(Duration::from_secs(1), client.next_event()) - .await - .expect("client should receive event while alive"); - assert!(matches!( - event, - Some(Event::Status { - status: WorkerStatus::Idle - }) - )); - server.await.unwrap(); - } - - #[tokio::test] - async fn send_writes_methods_while_client_is_alive() { - let socket_dir = tempdir().unwrap(); - let socket_path = socket_dir.path().join("send.sock"); - let listener = UnixListener::bind(&socket_path).unwrap(); - let server = tokio::spawn(async move { - let (stream, _) = listener.accept().await.unwrap(); - let mut reader = JsonLineReader::new(stream); - reader.next::().await.unwrap() - }); - - let mut client = WorkerClient::connect(&socket_path).await.unwrap(); - let method = Method::Run { - input: vec![Segment::text("hello")], - }; - client.send(&method).await.unwrap(); - - let received = tokio::time::timeout(Duration::from_secs(1), server) - .await - .expect("server should receive method while client is alive") - .unwrap(); - match received { - Some(Method::Run { input }) => assert_eq!(input, vec![Segment::text("hello")]), - other => panic!("expected Run method, got {other:?}"), - } - } - - #[tokio::test] - async fn dropping_repeated_clients_closes_server_connections() { - let socket_dir = tempdir().unwrap(); - let socket_path = socket_dir.path().join("drop.sock"); - let listener = UnixListener::bind(&socket_path).unwrap(); - let server = tokio::spawn(async move { - for _ in 0..16 { - let (mut stream, _) = listener.accept().await.unwrap(); - assert_peer_closed( - &mut stream, - "dropped client should close its socket promptly", - ) - .await; - } - }); - - for _ in 0..16 { - let client = WorkerClient::connect(&socket_path).await.unwrap(); - drop(client); - } - - server.await.unwrap(); - } - - #[tokio::test] - async fn dropping_client_aborts_blocked_reader_task() { - let socket_dir = tempdir().unwrap(); - let socket_path = socket_dir.path().join("blocked-reader.sock"); - let listener = UnixListener::bind(&socket_path).unwrap(); - let server = tokio::spawn(async move { - let (mut stream, _) = listener.accept().await.unwrap(); - stream.write_all(b"{\"event\"").await.unwrap(); - assert_peer_closed( - &mut stream, - "aborting the blocked client reader should close the socket", - ) - .await; - }); - - let client = WorkerClient::connect(&socket_path).await.unwrap(); - tokio::task::yield_now().await; - drop(client); - - server.await.unwrap(); - } -} diff --git a/crates/standalone/Cargo.toml b/crates/standalone/Cargo.toml index a3bb1813..8c5adc9d 100644 --- a/crates/standalone/Cargo.toml +++ b/crates/standalone/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true [dependencies] agen.workspace = true +client.workspace = true fs4.workspace = true manifest.workspace = true protocol.workspace = true diff --git a/crates/standalone/src/host.rs b/crates/standalone/src/host.rs index 031a554f..b6e2f7ba 100644 --- a/crates/standalone/src/host.rs +++ b/crates/standalone/src/host.rs @@ -2,14 +2,20 @@ use std::path::PathBuf; use std::time::Duration; use agen::llm_client::client::LlmClient; +use client::Client; +use client::transport::in_process::{Peer as InProcessPeer, Socket as InProcessSocket}; +use protocol::stream::{decode_method, encode_event}; use protocol::{Event, Method}; use session_store::{ CombinedStore, FsStore, FsWorkerStore, WorkerActiveSegmentRef, WorkerMetadataStore, }; use thiserror::Error; -use tokio::sync::broadcast; use worker::bootstrap::{WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout}; use worker::controller::WorkerControllerTransport; +use worker::ipc::protocol_session::{ + WorkerProtocolSessionStreams, dispatch_worker_protocol_method, live_log_entry_event, + subscribe_worker_protocol_session, +}; use worker::{BootstrappedWorker, WorkerError, WorkerFilesystemAuthority, WorkerWorkspaceContext}; use crate::launch::ResolvedStandaloneLaunch; @@ -55,12 +61,6 @@ pub enum StandaloneStartupError { Controller, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] -pub enum StandaloneRequestError { - #[error("the standalone Worker is no longer accepting requests")] - WorkerUnavailable, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] pub enum StandaloneShutdownError { #[error("the standalone Worker did not stop before the shutdown deadline")] @@ -277,19 +277,15 @@ impl StandaloneHost { &self.record } - pub async fn send(&self, method: Method) -> Result<(), StandaloneRequestError> { - self.handle - .send(method) - .await - .map_err(|_| StandaloneRequestError::WorkerUnavailable) - } - - pub fn subscribe(&self) -> broadcast::Receiver { - self.handle.subscribe() - } - - pub fn snapshot(&self) -> Event { - self.handle.snapshot_event() + /// Open one complete client-side Worker protocol session. + /// + /// Working events, committed session entries, alert snapshots, and the + /// initial history snapshot are merged behind the client boundary. + pub fn connect(&self) -> Client { + let streams = subscribe_worker_protocol_session(&self.handle); + let (socket, peer) = InProcessSocket::pair(); + tokio::spawn(run_protocol_session(self.handle.clone(), streams, peer)); + Client::new(socket) } pub fn with_shutdown_timeout(mut self, shutdown_timeout: Duration) -> Self { @@ -349,6 +345,111 @@ impl StandaloneHost { } } +async fn run_protocol_session( + handle: worker::WorkerHandle, + streams: WorkerProtocolSessionStreams, + mut peer: InProcessPeer, +) { + let WorkerProtocolSessionStreams { + snapshot_event, + mut log_entries, + alert_snapshot, + mut events, + } = streams; + + if !send_protocol_snapshot(&peer, alert_snapshot, snapshot_event).await { + return; + } + + loop { + tokio::select! { + message = peer.next() => { + let Some(message) = message else { + return; + }; + let Ok(method) = decode_method(&message) else { + return; + }; + if let Some(event) = dispatch_worker_protocol_method(&handle, method).await + && !send_protocol_event(&peer, event).await + { + return; + } + } + event = events.recv() => { + match event { + Ok(event) => { + if !send_protocol_event(&peer, event).await { + return; + } + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + let replacement = subscribe_worker_protocol_session(&handle); + let WorkerProtocolSessionStreams { + snapshot_event, + log_entries: replacement_log_entries, + alert_snapshot, + events: replacement_events, + } = replacement; + log_entries = replacement_log_entries; + events = replacement_events; + if !send_protocol_snapshot(&peer, alert_snapshot, snapshot_event).await { + return; + } + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => return, + } + } + entry = log_entries.recv() => { + match entry { + Ok(entry) => { + if let Some(event) = live_log_entry_event(entry) + && !send_protocol_event(&peer, event).await + { + return; + } + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + let replacement = subscribe_worker_protocol_session(&handle); + let WorkerProtocolSessionStreams { + snapshot_event, + log_entries: replacement_log_entries, + alert_snapshot, + events: replacement_events, + } = replacement; + log_entries = replacement_log_entries; + events = replacement_events; + if !send_protocol_snapshot(&peer, alert_snapshot, snapshot_event).await { + return; + } + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => return, + } + } + } + } +} + +async fn send_protocol_snapshot( + peer: &InProcessPeer, + alert_snapshot: Vec, + snapshot_event: Event, +) -> bool { + for alert in alert_snapshot { + if !send_protocol_event(peer, Event::Alert(alert)).await { + return false; + } + } + send_protocol_event(peer, snapshot_event).await +} + +async fn send_protocol_event(peer: &InProcessPeer, event: Event) -> bool { + let Ok(message) = encode_event(&event) else { + return false; + }; + peer.send(message).await.is_ok() +} + fn backing_store( store: &StandaloneSessionStore, id: StandaloneSessionId, diff --git a/crates/standalone/src/lib.rs b/crates/standalone/src/lib.rs index 19123cb4..8c04f863 100644 --- a/crates/standalone/src/lib.rs +++ b/crates/standalone/src/lib.rs @@ -8,9 +8,7 @@ pub mod host; pub mod launch; pub mod store; -pub use host::{ - StandaloneHost, StandaloneRequestError, StandaloneShutdownError, StandaloneStartupError, -}; +pub use host::{StandaloneHost, StandaloneShutdownError, StandaloneStartupError}; pub use launch::{ResolvedStandaloneLaunch, StandaloneLaunchConfig, StandaloneLaunchError}; pub use store::{ StaleLeasePolicy, StandaloneCwdIdentity, StandaloneListScope, StandaloneSessionId, diff --git a/crates/standalone/tests/host.rs b/crates/standalone/tests/host.rs index b01bcbfb..8198f3ac 100644 --- a/crates/standalone/tests/host.rs +++ b/crates/standalone/tests/host.rs @@ -8,6 +8,8 @@ use agen::llm_client::error::ClientError; use agen::llm_client::event::{Event as LlmEvent, StopReason}; use agen::llm_client::types::Request; use async_trait::async_trait; +use client::Client; +use client::transport::in_process::Socket as InProcessSocket; use futures::{Stream, stream}; use protocol::{Event, Method}; use standalone::{ @@ -88,17 +90,29 @@ async fn in_process_host_runs_text_and_read_tool_then_shuts_down() { let host = StandaloneHost::start_with_model_client(launch, client) .await .expect("start in-process host"); - let mut events = host.subscribe(); + let mut protocol_client = host.connect(); - host.send(Method::run_text("read the probe")) + protocol_client + .send(&Method::run_text("read the probe")) .await .expect("submit input"); tokio::time::timeout(Duration::from_secs(30), async { + let mut saw_user_message = false; let mut saw_text = false; let mut saw_tool_result = false; loop { - match events.recv().await.expect("worker event") { + match protocol_client + .next_event() + .await + .expect("protocol event") + .expect("worker event") + { + Event::UserMessage { segments } + if format!("{segments:?}").contains("read the probe") => + { + saw_user_message = true; + } Event::TextDelta { text } if text.contains("standalone response") => { saw_text = true; } @@ -106,6 +120,10 @@ async fn in_process_host_runs_text_and_read_tool_then_shuts_down() { saw_tool_result = true; } Event::RunEnd { .. } => { + assert!( + saw_user_message, + "stream must expose the committed user message" + ); assert!(saw_text, "stream must expose the model text delta"); assert!(saw_tool_result, "stream must expose the tool result"); break; @@ -251,15 +269,18 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( ]); let host = StandaloneHost::start_with_model_client(launch, first_client).await?; let session_id = host.session_id(); - let mut events = host.subscribe(); - host.send(Method::run_text("first request")).await?; - wait_for_run_end(&mut events).await?; - host.send(Method::Notify { - message: "persisted notification".to_string(), - auto_run: true, - }) - .await?; - wait_for_run_end(&mut events).await?; + let mut protocol_client = host.connect(); + protocol_client + .send(&Method::run_text("first request")) + .await?; + wait_for_run_end(&mut protocol_client).await?; + protocol_client + .send(&Method::Notify { + message: "persisted notification".to_string(), + auto_run: true, + }) + .await?; + wait_for_run_end(&mut protocol_client).await?; host.shutdown().await?; let store = StandaloneSessionStore::open(&state_dir)?; @@ -288,16 +309,24 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( let host = StandaloneHost::restore_with_model_client(state_dir.clone(), session_id, second_client) .await?; - let snapshot = format!("{:?}", host.snapshot()); + let mut protocol_client = host.connect(); + let snapshot = format!( + "{:?}", + protocol_client + .next_event() + .await + .expect("restored protocol stream") + .expect("restored snapshot") + ); assert!(snapshot.contains("first request"), "{snapshot}"); assert!(snapshot.contains("first answer"), "{snapshot}"); assert!(snapshot.contains("persisted task"), "{snapshot}"); assert!(snapshot.contains("persisted notification"), "{snapshot}"); - let mut events = host.subscribe(); - host.send(Method::run_text("continue after restore")) + protocol_client + .send(&Method::run_text("continue after restore")) .await?; - wait_for_run_end(&mut events).await?; + wait_for_run_end(&mut protocol_client).await?; let request = second_inspection .requests() .into_iter() @@ -503,10 +532,10 @@ async fn standalone_metadata_fails_closed_on_incomplete_or_newer_records() -> Te Ok(()) } -async fn wait_for_run_end(events: &mut tokio::sync::broadcast::Receiver) -> TestResult { +async fn wait_for_run_end(client: &mut Client) -> TestResult { tokio::time::timeout(Duration::from_secs(10), async { loop { - if matches!(events.recv().await, Ok(Event::RunEnd { .. })) { + if matches!(client.next_event().await, Ok(Some(Event::RunEnd { .. }))) { break; } } diff --git a/crates/tui/src/console/mod.rs b/crates/tui/src/console/mod.rs index 0d8fe7d2..541ff446 100644 --- a/crates/tui/src/console/mod.rs +++ b/crates/tui/src/console/mod.rs @@ -21,10 +21,13 @@ use protocol::{Greeting, RewindSummary, RewindTarget, RewindTargetId, Segment}; use ratatui::Terminal; use ratatui::backend::CrosstermBackend; use standalone::{StandaloneHost, StandaloneLaunchConfig}; -use tokio::sync::{broadcast, mpsc}; +use tokio::sync::mpsc; use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64_STANDARD}; -use client::{BackendRuntimeClient, BackendRuntimeTarget, StandaloneSessionResumeIntent}; +use client::transport::Socket; +use client::{ + BackendRuntimeTarget, Client, StandaloneSessionResumeIntent, connect_backend_runtime, +}; use crate::app::{ActionbarNoticeLevel, ActionbarNoticeSource, App}; use crate::composer_keys::{ComposerEditAction, composer_edit_action}; @@ -119,74 +122,40 @@ fn copy_selection_to_terminal(app: &mut App) -> bool { copy_selection_to_writer(app, &mut stdout) } -enum ConsoleConnection { - BackendRuntime(BackendRuntimeClient), - Standalone { - host: Option, - events: broadcast::Receiver, - initial_snapshot: Option, - }, +struct ConsoleConnection { + client: Client, + standalone_host: Option, } -impl ConsoleConnection { - fn standalone(host: StandaloneHost) -> Self { - let events = host.subscribe(); - let initial_snapshot = Some(host.snapshot()); - Self::Standalone { - host: Some(host), - events, - initial_snapshot, +impl ConsoleConnection { + fn new(client: Client) -> Self { + Self { + client, + standalone_host: None, } } - fn try_next_event(&mut self) -> Option { - match self { - Self::BackendRuntime(client) => client.try_next_event(), - Self::Standalone { - events, - initial_snapshot, - .. - } => initial_snapshot.take().or_else(|| events.try_recv().ok()), + fn with_standalone_host(client: Client, host: StandaloneHost) -> Self { + Self { + client, + standalone_host: Some(host), } } - async fn next_event(&mut self) -> Option { - match self { - Self::BackendRuntime(client) => client.next_event().await, - Self::Standalone { host, events, .. } => loop { - match events.recv().await { - Ok(event) => break Some(event), - Err(broadcast::error::RecvError::Lagged(_)) => { - let Some(host) = host.as_ref() else { - break None; - }; - break Some(host.snapshot()); - } - Err(broadcast::error::RecvError::Closed) => break None, - } - }, - } + fn try_next_event(&mut self) -> Result, Box> { + Ok(self.client.try_next_event()?) + } + + async fn next_event(&mut self) -> Result, Box> { + Ok(self.client.next_event().await?) } async fn send(&mut self, method: &Method) -> Result<(), Box> { - match self { - Self::BackendRuntime(client) => Ok(client.send(method).await?), - Self::Standalone { host, .. } => { - let host = host.as_ref().ok_or_else(|| { - io::Error::new( - io::ErrorKind::BrokenPipe, - "Standalone Worker has already shut down", - ) - })?; - Ok(host.send(method.clone()).await?) - } - } + Ok(self.client.send(method).await?) } async fn shutdown(&mut self) -> Result<(), Box> { - if let Self::Standalone { host, .. } = self - && let Some(host) = host.take() - { + if let Some(host) = self.standalone_host.take() { host.shutdown().await?; } Ok(()) @@ -251,7 +220,8 @@ async fn run_standalone_host( worker_label: String, history_root: PathBuf, ) -> Result<(), Box> { - let mut connection = ConsoleConnection::standalone(host); + let client = host.connect(); + let mut connection = ConsoleConnection::with_standalone_host(client, host); let mut terminal = match enter_fullscreen() { Ok(terminal) => terminal, @@ -280,12 +250,12 @@ pub(crate) async fn run_backend_runtime( target: BackendRuntimeTarget, ) -> Result<(), Box> { let worker_label = target.display_label(); - let client = BackendRuntimeClient::connect(target).await?; + let client = connect_backend_runtime(target).await?; let mut terminal = enter_fullscreen()?; let workspace_root = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")); let mut app = App::new_with_persistent_input_history(worker_label, &workspace_root); app.connected = true; - let mut connection = ConsoleConnection::BackendRuntime(client); + let mut connection = ConsoleConnection::new(client); let result = run_loop(&mut terminal, &mut app, &mut connection).await; let _ = leave_fullscreen(&mut terminal); result @@ -560,7 +530,7 @@ enum E2eRewindInput { enum LoopInput

{ Terminal(TerminalEventResult), - Worker(Option

), + Worker(P), } async fn next_loop_input( @@ -569,7 +539,7 @@ async fn next_loop_input( pod_next: F, ) -> LoopInput

where - F: Future>, + F: Future, { tokio::select! { biased; @@ -586,9 +556,9 @@ where } } -async fn drain_terminal_events( +async fn drain_terminal_events( app: &mut App, - client: &mut ConsoleConnection, + client: &mut ConsoleConnection, term_rx: &mut mpsc::UnboundedReceiver, ) -> Result> { let mut handled = false; @@ -613,13 +583,13 @@ async fn drain_terminal_events( Ok(handled) } -async fn drain_worker_events( +async fn drain_worker_events( app: &mut App, - client: &mut ConsoleConnection, + client: &mut ConsoleConnection, ) -> Result> { let mut handled = false; for _ in 0..POD_EVENT_DRAIN_LIMIT { - match client.try_next_event() { + match client.try_next_event()? { Some(ev) => { handled = true; if let Some(method) = app.handle_worker_event(ev) { @@ -632,10 +602,10 @@ async fn drain_worker_events( Ok(handled) } -async fn run_loop( +async fn run_loop( terminal: &mut Terminal>, app: &mut App, - client: &mut ConsoleConnection, + client: &mut ConsoleConnection, ) -> Result<(), Box> { let (_terminal_reader, mut term_rx) = TerminalEventReader::spawn()?; @@ -660,7 +630,7 @@ async fn run_loop( LoopInput::Terminal(term_event) => { handle_terminal_event(app, client, term_event?).await?; } - LoopInput::Worker(event) => match event { + LoopInput::Worker(event) => match event? { Some(ev) => { if let Some(method) = app.handle_worker_event(ev) { client.send(&method).await?; @@ -680,9 +650,9 @@ async fn run_loop( Ok(()) } -async fn handle_terminal_event( +async fn handle_terminal_event( app: &mut App, - client: &mut ConsoleConnection, + client: &mut ConsoleConnection, event: TermEvent, ) -> Result<(), Box> { match event { From 13a021c480fb9374ce7021f313c672f412bdacbb Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 12:46:47 +0900 Subject: [PATCH 3/7] feat: add TUI run status spinner --- crates/tui/src/app.rs | 33 +++++-- crates/tui/src/console/mod.rs | 67 ++++++++++++-- crates/tui/src/ui.rs | 160 ++++++++++++++++++++++++---------- 3 files changed, 205 insertions(+), 55 deletions(-) diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index 0c620570..d9a8e09d 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -249,6 +249,9 @@ pub struct App { pub running: bool, /// True while the Worker is in `WorkerStatus::Paused`. pub paused: bool, + /// Local observation time for the current run. Used only for live UI + /// elapsed time and spinner animation; it is not persisted in history. + pub run_started_at: Option, pub run_requests: usize, /// Sum of `input_tokens - cache_read_input_tokens` across the /// current turn's LLM requests — i.e. the net tokens this turn @@ -352,6 +355,7 @@ impl App { worker_status: WorkerStatus::Idle, running: false, paused: false, + run_started_at: None, run_requests: 0, run_upload_tokens: 0, run_output_tokens: 0, @@ -553,11 +557,17 @@ impl App { } pub fn set_worker_status(&mut self, status: WorkerStatus) { + let was_running = self.running; self.worker_status = status; self.running = status == WorkerStatus::Running; self.paused = status == WorkerStatus::Paused; if self.running { + if !was_running { + self.run_started_at = Some(Instant::now()); + } self.quit_confirm = None; + } else { + self.run_started_at = None; } } @@ -1121,11 +1131,13 @@ impl App { self.latest_llm_wait_event = None; self.assistant_streaming = false; } - // UI consumers of Invoke / LlmCall semantics are out of scope - // for `tickets/invoke-turn-llmcall-semantics.md`; events flow - // through to subscribers but the TUI currently derives its - // turn header from `UserMessage` / `SystemItem` arrivals. - Event::InvokeStart { .. } | Event::LlmCallStart { .. } | Event::LlmCallEnd { .. } => { + Event::InvokeStart { .. } => { + self.set_worker_status(WorkerStatus::Running); + } + // UI consumers of per-attempt LlmCall semantics remain out of scope; + // the run-level status starts at InvokeStart and TurnStart counts each + // LLM request within that run. + Event::LlmCallStart { .. } | Event::LlmCallEnd { .. } => { self.latest_llm_wait_event = None; } Event::LlmRetry { @@ -3377,6 +3389,17 @@ mod completion_flow_tests { } } + #[test] + fn running_status_starts_and_stops_live_run_clock() { + let mut app = App::new("test".into()); + + app.set_worker_status(WorkerStatus::Running); + assert!(app.run_started_at.is_some()); + + app.set_worker_status(WorkerStatus::Idle); + assert!(app.run_started_at.is_none()); + } + #[test] fn running_submit_is_queued_locally_and_clears_composer() { let mut app = App::new("test".into()); diff --git a/crates/tui/src/console/mod.rs b/crates/tui/src/console/mod.rs index 541ff446..cc8316ab 100644 --- a/crates/tui/src/console/mod.rs +++ b/crates/tui/src/console/mod.rs @@ -531,15 +531,19 @@ enum E2eRewindInput { enum LoopInput

{ Terminal(TerminalEventResult), Worker(P), + Tick, } -async fn next_loop_input( +async fn next_loop_input( term_rx: &mut mpsc::UnboundedReceiver, connected: bool, pod_next: F, + animate: bool, + animation_tick: T, ) -> LoopInput

where F: Future, + T: Future, { tokio::select! { biased; @@ -553,6 +557,7 @@ where })) } event = pod_next, if connected => LoopInput::Worker(event), + _ = animation_tick, if animate => LoopInput::Tick, } } @@ -608,6 +613,8 @@ async fn run_loop( client: &mut ConsoleConnection, ) -> Result<(), Box> { let (_terminal_reader, mut term_rx) = TerminalEventReader::spawn()?; + let mut animation_tick = tokio::time::interval(Duration::from_millis(80)); + animation_tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); terminal.draw(|f| ui::draw(f, app))?; @@ -626,7 +633,15 @@ async fn run_loop( continue; } - match next_loop_input(&mut term_rx, app.connected, client.next_event()).await { + match next_loop_input( + &mut term_rx, + app.connected, + client.next_event(), + app.running, + animation_tick.tick(), + ) + .await + { LoopInput::Terminal(term_event) => { handle_terminal_event(app, client, term_event?).await?; } @@ -642,6 +657,7 @@ async fn run_loop( app.push_error("Connection lost"); } }, + LoopInput::Tick => {} } terminal.draw(|f| ui::draw(f, app))?; @@ -1216,6 +1232,23 @@ mod tests { ); } + #[tokio::test] + async fn animation_tick_wakes_loop_while_running() { + let (_tx, mut rx) = mpsc::unbounded_channel::(); + + assert!(matches!( + next_loop_input( + &mut rx, + true, + std::future::pending::>(), + true, + std::future::ready(()), + ) + .await, + LoopInput::Tick + )); + } + #[tokio::test] async fn terminal_event_is_selected_before_ready_worker_event() { let (tx, mut rx) = mpsc::unbounded_channel(); @@ -1225,7 +1258,15 @@ mod tests { )))) .unwrap(); - match next_loop_input(&mut rx, true, std::future::ready(Some(()))).await { + match next_loop_input( + &mut rx, + true, + std::future::ready(Some(())), + false, + std::future::pending::<()>(), + ) + .await + { LoopInput::Terminal(Ok(TermEvent::Key(key))) => { assert_eq!(key.code, KeyCode::Char('x')); } @@ -1237,7 +1278,15 @@ mod tests { async fn terminal_event_is_preserved_after_worker_event_wins() { let (tx, mut rx) = mpsc::unbounded_channel(); - match next_loop_input(&mut rx, true, std::future::ready(Some(1_u8))).await { + match next_loop_input( + &mut rx, + true, + std::future::ready(Some(1_u8)), + false, + std::future::pending::<()>(), + ) + .await + { LoopInput::Worker(Some(1)) => {} _ => panic!("expected the first ready Worker event to win before any terminal input"), } @@ -1248,7 +1297,15 @@ mod tests { )))) .unwrap(); - match next_loop_input(&mut rx, true, std::future::ready(Some(2_u8))).await { + match next_loop_input( + &mut rx, + true, + std::future::ready(Some(2_u8)), + false, + std::future::pending::<()>(), + ) + .await + { LoopInput::Terminal(Ok(TermEvent::Key(key))) => { assert_eq!(key.code, KeyCode::Char('y')); } diff --git a/crates/tui/src/ui.rs b/crates/tui/src/ui.rs index 63706fe2..bd5ae01d 100644 --- a/crates/tui/src/ui.rs +++ b/crates/tui/src/ui.rs @@ -36,6 +36,9 @@ use crate::task::{TaskCounts, TaskEntry, TaskStatus, TaskStore}; use crate::text_selection::{HistoryViewport, SelectionRow}; use crate::view_mode::Mode; +const RUN_SPINNER_FRAMES: [&str; 8] = ["⣷", "⣯", "⣟", "⡿", "⢿", "⣻", "⣽", "⣾"]; +const RUN_SPINNER_FRAME_MS: u128 = 80; + pub fn draw(frame: &mut Frame, app: &mut App) { let area = frame.area(); // Input content starts after the prompt (`> ` or `: `), so the width @@ -57,19 +60,27 @@ pub fn draw(frame: &mut Frame, app: &mut App) { let tabs = app.worker_view_tabs(); let show_tabs = tabs.len() > 1; let mini_view_h = task_mini_view_height(&app.selected_worker_view().task_store, show_tabs); - // One blank row separates the history tail from the mini-view so - // the latest message doesn't visually crash into the task summary. - // Folds away with the mini-view when there are no tasks. - let mini_view_gap = if mini_view_h > 0 { 1 } else { 0 }; + let run_status_h = u16::from(app.running); + let run_status_gap = run_status_h; + // One blank row separates the history tail from the run/task mini-view so + // the latest message doesn't visually crash into operational status. + // Folds away when neither run status nor tasks are visible. + let mini_view_gap = if mini_view_h > 0 || run_status_h > 0 { + 1 + } else { + 0 + }; let chunks = Layout::vertical([ - Constraint::Min(0), // history view - Constraint::Length(mini_view_gap), // gap above mini-view - Constraint::Length(mini_view_h), // task mini-view (0 when empty) - Constraint::Length(1), // separator - Constraint::Length(1), // status - Constraint::Length(input_height), // input area - Constraint::Length(1), // actionbar + Constraint::Min(0), // history view + Constraint::Length(mini_view_gap), // gap above run/task mini-view + Constraint::Length(run_status_h), // active run status + Constraint::Length(run_status_gap), // gap below active run status + Constraint::Length(mini_view_h), // task mini-view (0 when empty) + Constraint::Length(1), // separator + Constraint::Length(1), // status + Constraint::Length(input_height), // input area + Constraint::Length(1), // actionbar ]) .split(area); @@ -82,24 +93,27 @@ pub fn draw(frame: &mut Frame, app: &mut App) { } else { draw_history(frame, app, chunks[0]); } + if run_status_h > 0 { + draw_run_status(frame, app, chunks[2]); + } if mini_view_h > 0 { draw_task_mini_view( frame, &app.selected_worker_view().task_store, &tabs, - chunks[2], + chunks[4], ); } - draw_separator(frame, chunks[3]); + draw_separator(frame, chunks[5]); // Status/composer/control surfaces remain parent-owned. View selection changes // only transcript/task presentation and never implies SubWorker control. - draw_status(frame, app, chunks[4]); - draw_input(frame, app, &input_render, chunks[5]); - draw_actionbar(frame, app, chunks[6]); + draw_status(frame, app, chunks[6]); + draw_input(frame, app, &input_render, chunks[7]); + draw_actionbar(frame, app, chunks[8]); if app.is_command_mode() { - draw_command_popup(frame, app, chunks[5]); + draw_command_popup(frame, app, chunks[7]); } else if let Some(state) = app.completion.as_ref().filter(|c| c.is_active()) { - draw_completion_popup(frame, state, chunks[5]); + draw_completion_popup(frame, state, chunks[7]); } } @@ -120,6 +134,65 @@ fn task_mini_view_height(store: &TaskStore, show_tabs: bool) -> u16 { (active_shown as u16).saturating_add(1) } +fn draw_run_status(frame: &mut Frame, app: &App, area: Rect) { + frame.render_widget(Paragraph::new(run_status_line(app, Instant::now())), area); +} + +fn run_status_line(app: &App, now: Instant) -> Line<'static> { + let elapsed = app + .run_started_at + .and_then(|started_at| now.checked_duration_since(started_at)) + .unwrap_or_default(); + let spinner_index = + ((elapsed.as_millis() / RUN_SPINNER_FRAME_MS) as usize) % RUN_SPINNER_FRAMES.len(); + let request_label = if app.run_requests == 1 { + "1 req".to_owned() + } else { + format!("{} reqs", app.run_requests) + }; + + Line::from(vec![ + Span::styled( + RUN_SPINNER_FRAMES[spinner_index], + Style::default() + .fg(Color::Cyan) + .add_modifier(Modifier::BOLD), + ), + Span::raw(" "), + Span::styled( + fmt_run_elapsed(elapsed.as_secs()), + Style::default().fg(Color::Gray), + ), + Span::styled(" ・ ", Style::default().fg(Color::DarkGray)), + Span::styled(request_label, Style::default().fg(Color::Gray)), + Span::styled(" | ", Style::default().fg(Color::DarkGray)), + Span::styled("↑", Style::default().fg(Color::Green)), + Span::styled( + fmt_tokens(app.run_upload_tokens), + Style::default().fg(Color::Green), + ), + Span::styled("/", Style::default().fg(Color::DarkGray)), + Span::styled("↓", Style::default().fg(Color::Yellow)), + Span::styled( + fmt_tokens(app.run_output_tokens), + Style::default().fg(Color::Yellow), + ), + ]) +} + +fn fmt_run_elapsed(secs: u64) -> String { + let hours = secs / 3600; + let minutes = (secs % 3600) / 60; + let seconds = secs % 60; + if hours > 0 { + format!("{hours}h {minutes}m {seconds:02}s") + } else if minutes > 0 { + format!("{minutes}m {seconds:02}s") + } else { + format!("{seconds}s") + } +} + fn draw_task_mini_view(frame: &mut Frame, store: &TaskStore, tabs: &[WorkerViewTab], area: Rect) { if area.height == 0 || area.width == 0 { return; @@ -1726,32 +1799,7 @@ fn draw_status(frame: &mut Frame, app: &App, area: Rect) { ), ]; - if app.running { - let status = if let Some(wait_event) = &app.latest_llm_wait_event { - format!( - "request: {} | ↑{}/↓{} | {wait_event}", - app.run_requests, - fmt_tokens(app.run_upload_tokens), - fmt_tokens(app.run_output_tokens), - ) - } else if let Some(tool) = &app.current_tool { - format!( - "request: {} | ↑{}/↓{} | tool: {tool}", - app.run_requests, - fmt_tokens(app.run_upload_tokens), - fmt_tokens(app.run_output_tokens), - ) - } else { - format!( - "request: {} | ↑{}/↓{}", - app.run_requests, - fmt_tokens(app.run_upload_tokens), - fmt_tokens(app.run_output_tokens), - ) - }; - spans.push(Span::raw(" | ")); - spans.push(Span::styled(status, Style::default().fg(Color::Yellow))); - } else if app.paused { + if app.paused { spans.push(Span::raw(" | ")); spans.push(Span::styled( "paused", @@ -1763,7 +1811,7 @@ fn draw_status(frame: &mut Frame, app: &App, area: Rect) { " — Enter to resume, Ctrl-X to cancel, type to start new turn", Style::default().fg(Color::DarkGray), )); - } else { + } else if !app.running { spans.push(Span::styled(" idle", Style::default().fg(Color::DarkGray))); } @@ -2053,6 +2101,28 @@ mod tests { use protocol::WorkerStatus; use std::time::{Duration, Instant}; + #[test] + fn run_status_line_matches_console_metrics_and_spinner_frame() { + let now = Instant::now(); + let mut app = App::new("worker".into()); + app.run_started_at = now.checked_sub(Duration::from_millis(160)); + app.run_requests = 1; + app.run_upload_tokens = 1_200; + app.run_output_tokens = 45; + + assert_eq!( + line_text(&run_status_line(&app, now)), + "⣟ 0s ・ 1 req | ↑1.2k/↓45" + ); + } + + #[test] + fn run_elapsed_uses_console_style_units() { + assert_eq!(fmt_run_elapsed(9), "9s"); + assert_eq!(fmt_run_elapsed(65), "1m 05s"); + assert_eq!(fmt_run_elapsed(3_726), "1h 2m 06s"); + } + #[test] fn task_summary_right_aligns_worker_tabs_and_highlights_selection() { let tabs = vec![ From bde1dea2a53352588eb490b0b9622fbbfaf62c56 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 13:07:39 +0900 Subject: [PATCH 4/7] fix: require confirmation before Ctrl-X shutdown --- crates/tui/src/app.rs | 5 ++ crates/tui/src/console/mod.rs | 92 +++++++++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index d9a8e09d..d3b2396d 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -284,6 +284,9 @@ pub struct App { /// records the instant; a second press within the timeout exits the /// TUI (the Worker itself stays alive). pub quit_confirm: Option, + /// Independent 2-tap guard for `Ctrl-X` when the Worker is idle or + /// stopped. A second press within the timeout shuts down the Worker. + pub shutdown_confirm: Option, /// Full display history in render order. pub blocks: Vec, /// Turn/protocol errors retained when a real `SegmentStart` replaces the @@ -373,6 +376,7 @@ impl App { command_completion_selected: None, quit: false, quit_confirm: None, + shutdown_confirm: None, blocks: Vec::new(), run_error_messages: Vec::new(), internal_workers: Vec::new(), @@ -566,6 +570,7 @@ impl App { self.run_started_at = Some(Instant::now()); } self.quit_confirm = None; + self.shutdown_confirm = None; } else { self.run_started_at = None; } diff --git a/crates/tui/src/console/mod.rs b/crates/tui/src/console/mod.rs index cc8316ab..ec0454ff 100644 --- a/crates/tui/src/console/mod.rs +++ b/crates/tui/src/console/mod.rs @@ -824,13 +824,7 @@ fn handle_key(app: &mut App, key: KeyEvent) -> Option { Some(None) } KeyCode::Char('c') if ctrl => Some(handle_pause_or_quit(app)), - KeyCode::Char('x') if ctrl => Some(match app.worker_status { - WorkerStatus::Running | WorkerStatus::Paused => { - app.clear_queued_inputs(); - Some(Method::Cancel) - } - WorkerStatus::Idle | WorkerStatus::Stopped => Some(Method::Shutdown), - }), + KeyCode::Char('x') if ctrl => Some(handle_cancel_or_shutdown(app)), KeyCode::Char('d') if ctrl => { app.quit = true; Some(None) @@ -1087,6 +1081,33 @@ fn handle_command_key(app: &mut App, key: KeyEvent) -> Option { const CONFIRM_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); +/// Running / Paused → send `Method::Cancel` immediately. +/// Idle / Stopped → 2-tap to shut down the Worker. +fn handle_cancel_or_shutdown(app: &mut App) -> Option { + if matches!( + app.worker_status, + WorkerStatus::Running | WorkerStatus::Paused + ) { + app.shutdown_confirm = None; + app.clear_queued_inputs(); + return Some(Method::Cancel); + } + if let Some(pressed_at) = app.shutdown_confirm + && pressed_at.elapsed() < CONFIRM_TIMEOUT + { + app.shutdown_confirm = None; + return Some(Method::Shutdown); + } + app.shutdown_confirm = Some(std::time::Instant::now()); + app.flash_actionbar_notice( + "Press Ctrl-X again within 3 s to shut down the Worker.", + ActionbarNoticeLevel::Warn, + ActionbarNoticeSource::Tui, + CONFIRM_TIMEOUT, + ); + None +} + /// Running → send `Method::Pause`. /// Idle / Paused → 2-tap to quit the TUI (the Worker keeps running). fn handle_pause_or_quit(app: &mut App) -> Option { @@ -1472,15 +1493,53 @@ mod tests { } #[test] - fn ctrl_x_shutdown_while_idle_is_unchanged() { + fn ctrl_x_requires_confirmation_before_shutdown_while_idle() { + let mut app = App::new("agent".to_string()); + app.set_worker_status(WorkerStatus::Idle); + let ctrl_x = || KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL); + + assert!(handle_key(&mut app, ctrl_x()).is_none()); + assert!(app.shutdown_confirm.is_some()); + let notice = app + .current_actionbar_notice(std::time::Instant::now()) + .expect("first Ctrl-X should arm shutdown confirmation"); + assert_eq!(notice.level, ActionbarNoticeLevel::Warn); + assert_eq!(notice.source, ActionbarNoticeSource::Tui); + assert!(notice.text.contains("Ctrl-X")); + assert!(notice.text.contains("shut down the Worker")); + assert!(!has_alert(&app, "shut down the Worker")); + + assert!(matches!( + handle_key(&mut app, ctrl_x()), + Some(Method::Shutdown) + )); + assert!(app.shutdown_confirm.is_none()); + } + + #[test] + fn ctrl_c_and_ctrl_x_confirmations_do_not_authorize_each_other() { let mut app = App::new("agent".to_string()); app.set_worker_status(WorkerStatus::Idle); - let shutdown = handle_key( - &mut app, - KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + assert!( + handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL), + ) + .is_none() ); - assert!(matches!(shutdown, Some(Method::Shutdown))); + assert!(app.quit_confirm.is_some()); + assert!(app.shutdown_confirm.is_none()); + + assert!( + handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + ) + .is_none() + ); + assert!(!app.quit); + assert!(app.shutdown_confirm.is_some()); } #[test] @@ -2196,12 +2255,17 @@ mod tests { handle_key(&mut app, key(KeyCode::Tab)); assert_eq!(app.selected_worker_view().worker_name, "subworker-hoge"); - let method = handle_key( + let first = handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + ); + let second = handle_key( &mut app, KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), ); - assert!(matches!(method, Some(Method::Shutdown))); + assert!(first.is_none()); + assert!(matches!(second, Some(Method::Shutdown))); assert_eq!(app.worker_status, WorkerStatus::Idle); } From e7f4c6864fc2f136b87bd2ae635573557c04fe7b Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 14:37:46 +0900 Subject: [PATCH 5/7] fix: make WorkerId the standalone primary identity --- Cargo.lock | 1 + crates/client/src/lib.rs | 6 +- crates/client/src/target.rs | 48 ++--- crates/protocol/Cargo.toml | 3 +- crates/protocol/src/identity.rs | 132 ++++++++++++++ crates/protocol/src/lib.rs | 3 + crates/standalone/src/host.rs | 140 ++++++++------- crates/standalone/src/lib.rs | 6 +- crates/standalone/src/store.rs | 249 +++++++++++--------------- crates/standalone/tests/host.rs | 94 +++++----- crates/tui/src/console/mod.rs | 14 +- crates/tui/src/lib.rs | 4 +- crates/tui/src/standalone_picker.rs | 39 ++-- crates/worker-runtime/src/identity.rs | 110 +----------- crates/yoi/src/cli_connection.rs | 2 +- crates/yoi/src/main.rs | 30 ++-- 16 files changed, 444 insertions(+), 437 deletions(-) create mode 100644 crates/protocol/src/identity.rs diff --git a/Cargo.lock b/Cargo.lock index 4b2f4b62..d87247cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3508,6 +3508,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "sha2 0.11.0", "tokio", "ts-rs", "uuid", diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 6aae3f6a..b3cea8de 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -35,9 +35,9 @@ pub use backend_workspace::{ }; pub use client::{Client, ClientError}; pub use target::{ - BackendTarget, Dashboard, ResolvedTarget, StandaloneSessionListIntent, - StandaloneSessionResumeIntent, StandaloneTarget, Target, TargetError, TargetKind, - WorkerConnection, WorkerConnectionSelector, WorkerList, WorkerListRequest, WorkerSpawn, + BackendTarget, Dashboard, ResolvedTarget, StandaloneTarget, StandaloneWorkerListIntent, + StandaloneWorkerResumeIntent, Target, TargetError, TargetKind, WorkerConnection, + WorkerConnectionSelector, WorkerList, WorkerListRequest, WorkerSpawn, }; pub use workspace_api::{ObjectiveDetail, ObjectiveSummary}; pub use workspace_product::BackendWorkspaceProductClient; diff --git a/crates/client/src/target.rs b/crates/client/src/target.rs index 42695859..6b5fc783 100644 --- a/crates/client/src/target.rs +++ b/crates/client/src/target.rs @@ -105,16 +105,16 @@ pub struct WorkerSpawn { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct StandaloneSessionListIntent { +pub struct StandaloneWorkerListIntent { pub state_dir: PathBuf, pub cwd: PathBuf, pub include_all: bool, } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct StandaloneSessionResumeIntent { +pub struct StandaloneWorkerResumeIntent { pub state_dir: PathBuf, - pub session_id: String, + pub worker_id: String, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -175,22 +175,22 @@ pub trait Target: fmt::Debug + Send + Sync { Err(TargetError::unsupported("Worker spawn", self.kind())) } - fn standalone_session_list( + fn standalone_worker_list( &self, _include_all: bool, - ) -> Result { + ) -> Result { Err(TargetError::unsupported( - "standalone session listing", + "standalone Worker listing", self.kind(), )) } - fn standalone_session_resume( + fn standalone_worker_resume( &self, - _session_id: String, - ) -> Result { + _worker_id: String, + ) -> Result { Err(TargetError::unsupported( - "standalone session restore", + "standalone Worker restore", self.kind(), )) } @@ -243,26 +243,26 @@ impl Target for StandaloneTarget { }) } - fn standalone_session_list( + fn standalone_worker_list( &self, include_all: bool, - ) -> Result { + ) -> Result { let cwd = std::env::current_dir() .map_err(|error| TargetError::invalid(self.kind(), error.to_string()))?; - Ok(StandaloneSessionListIntent { + Ok(StandaloneWorkerListIntent { state_dir: self.state_dir.clone(), cwd, include_all, }) } - fn standalone_session_resume( + fn standalone_worker_resume( &self, - session_id: String, - ) -> Result { - Ok(StandaloneSessionResumeIntent { + worker_id: String, + ) -> Result { + Ok(StandaloneWorkerResumeIntent { state_dir: self.state_dir.clone(), - session_id, + worker_id, }) } } @@ -437,17 +437,17 @@ mod tests { } #[test] - fn standalone_target_builds_explicit_session_intents() { - let target = StandaloneTarget::new("/tmp/yoi-client-sessions"); - let list = target.standalone_session_list(true).unwrap(); - assert_eq!(list.state_dir, PathBuf::from("/tmp/yoi-client-sessions")); + fn standalone_target_builds_explicit_worker_intents() { + let target = StandaloneTarget::new("/tmp/yoi-client-workers"); + let list = target.standalone_worker_list(true).unwrap(); + assert_eq!(list.state_dir, PathBuf::from("/tmp/yoi-client-workers")); assert!(list.include_all); assert!(list.cwd.is_absolute()); let resume = target - .standalone_session_resume("019d1234-0000-7000-8000-000000000000".to_string()) + .standalone_worker_resume("019d1234-0000-7000-8000-000000000000".to_string()) .unwrap(); assert_eq!(resume.state_dir, list.state_dir); - assert_eq!(resume.session_id, "019d1234-0000-7000-8000-000000000000"); + assert_eq!(resume.worker_id, "019d1234-0000-7000-8000-000000000000"); } } diff --git a/crates/protocol/Cargo.toml b/crates/protocol/Cargo.toml index 7d6032ab..8a48a041 100644 --- a/crates/protocol/Cargo.toml +++ b/crates/protocol/Cargo.toml @@ -14,6 +14,7 @@ json-schema = ["dep:schemars"] schemars = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +sha2.workspace = true tokio = { workspace = true, features = ["io-util"], optional = true } ts-rs = { version = "12.0.1", optional = true } -uuid = { workspace = true, features = ["serde"] } +uuid = { workspace = true, features = ["serde", "v7"] } diff --git a/crates/protocol/src/identity.rs b/crates/protocol/src/identity.rs new file mode 100644 index 00000000..48c420ab --- /dev/null +++ b/crates/protocol/src/identity.rs @@ -0,0 +1,132 @@ +use std::{fmt, str::FromStr}; + +use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; +use sha2::{Digest, Sha256}; +use uuid::{Uuid, Version}; + +/// Stable Worker identity independent of its current Runtime placement or +/// conversation Session. +/// +/// Workspace authority allocates this ID for managed Workers. A standalone +/// Worker store allocates it locally when no Workspace authority is present. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct WorkerId(Uuid); + +impl WorkerId { + pub fn now_v7() -> Self { + Self(Uuid::now_v7()) + } + + /// Converts a legacy Runtime-local numeric id into a syntactically valid + /// migration-only UUIDv7 value. New Worker allocation must use `now_v7`. + pub fn from_legacy_u64(value: u64) -> Self { + let mut bytes = [0_u8; 16]; + bytes[8..].copy_from_slice(&value.to_be_bytes()); + bytes[6] = 0x70; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + Self(Uuid::from_bytes(bytes)) + } + + pub fn from_legacy_binding(workspace_id: &str, runtime_id: &str, value: u64) -> Self { + let mut hasher = Sha256::new(); + hasher.update(b"yoi.workspace-worker-id.v1\0"); + hasher.update(workspace_id.as_bytes()); + hasher.update([0]); + hasher.update(runtime_id.as_bytes()); + hasher.update([0]); + hasher.update(value.to_be_bytes()); + let digest = hasher.finalize(); + let mut bytes = [0_u8; 16]; + bytes.copy_from_slice(&digest[..16]); + // Migrated ids sort before normally allocated UUIDv7 values while retaining + // deterministic collision-resistant payload bits. + bytes[..6].fill(0); + bytes[6] = (bytes[6] & 0x0f) | 0x70; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + Self(Uuid::from_bytes(bytes)) + } + + pub fn parse(value: &str) -> Option { + let value = Uuid::parse_str(value).ok()?; + (value.get_version() == Some(Version::SortRand)).then_some(Self(value)) + } + + pub const fn as_uuid(self) -> Uuid { + self.0 + } + + #[must_use] + pub fn short(self) -> String { + let simple = self.0.simple().to_string(); + simple[simple.len() - 12..].to_string() + } +} + +impl fmt::Display for WorkerId { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(formatter) + } +} + +impl FromStr for WorkerId { + type Err = WorkerIdParseError; + + fn from_str(value: &str) -> Result { + Self::parse(value).ok_or(WorkerIdParseError) + } +} + +impl Serialize for WorkerId { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} + +impl<'de> Deserialize<'de> for WorkerId { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = String::deserialize(deserializer)?; + Self::parse(&value).ok_or_else(|| de::Error::custom("Worker id must be a UUIDv7")) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct WorkerIdParseError; + +impl fmt::Display for WorkerIdParseError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("Worker id must be a UUIDv7") + } +} + +impl std::error::Error for WorkerIdParseError {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn worker_id_accepts_only_uuid_v7() { + let worker_id = WorkerId::now_v7(); + assert_eq!(WorkerId::parse(&worker_id.to_string()), Some(worker_id)); + assert!(WorkerId::parse("30").is_none()); + assert!(WorkerId::parse(&Uuid::nil().to_string()).is_none()); + } + + #[test] + fn legacy_worker_id_mapping_is_stable() { + assert_eq!( + WorkerId::from_legacy_binding("workspace", "runtime", 42), + WorkerId::from_legacy_binding("workspace", "runtime", 42) + ); + assert_ne!( + WorkerId::from_legacy_binding("workspace", "runtime", 42), + WorkerId::from_legacy_binding("workspace", "runtime", 43) + ); + } +} diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 8c498f09..9d494b5d 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -1,3 +1,4 @@ +pub mod identity; #[cfg(feature = "stream")] pub mod stream; pub mod subscription; @@ -8,6 +9,8 @@ use std::path::PathBuf; use serde::{Deserialize, Serialize}; +pub use identity::{WorkerId, WorkerIdParseError}; + fn default_true() -> bool { true } diff --git a/crates/standalone/src/host.rs b/crates/standalone/src/host.rs index b6e2f7ba..7bfc1e51 100644 --- a/crates/standalone/src/host.rs +++ b/crates/standalone/src/host.rs @@ -5,7 +5,7 @@ use agen::llm_client::client::LlmClient; use client::Client; use client::transport::in_process::{Peer as InProcessPeer, Socket as InProcessSocket}; use protocol::stream::{decode_method, encode_event}; -use protocol::{Event, Method}; +use protocol::{Event, Method, WorkerId}; use session_store::{ CombinedStore, FsStore, FsWorkerStore, WorkerActiveSegmentRef, WorkerMetadataStore, }; @@ -20,14 +20,14 @@ use worker::{BootstrappedWorker, WorkerError, WorkerFilesystemAuthority, WorkerW use crate::launch::ResolvedStandaloneLaunch; use crate::store::{ - StaleLeasePolicy, StandaloneSessionId, StandaloneSessionLease, StandaloneSessionRecord, - StandaloneSessionStore, StandaloneShutdownReason, StandaloneStoreError, + StaleLeasePolicy, StandaloneShutdownReason, StandaloneStoreError, StandaloneWorkerLease, + StandaloneWorkerRecord, StandaloneWorkerStore, }; const DEFAULT_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10); type StandaloneBackingStore = CombinedStore; -/// One client-owned top-level Worker and its standalone session authority. +/// One client-owned top-level Worker and its standalone Worker authority. /// /// The host deliberately exposes the existing typed Worker protocol rather than owning an /// HTTP/WebSocket server or creating Runtime/Workspace/Ticket/Workdir domain records. @@ -35,21 +35,21 @@ pub struct StandaloneHost { handle: worker::WorkerHandle, shutdown: Option, shutdown_timeout: Duration, - store: StandaloneSessionStore, + store: StandaloneWorkerStore, worker_store: FsWorkerStore, - record: StandaloneSessionRecord, - lease: Option, + record: StandaloneWorkerRecord, + lease: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] pub enum StandaloneStartupError { #[error("the standalone state store could not be opened or validated")] StateStore, - #[error("the standalone session is already active")] - SessionActive, - #[error("the standalone session lease cannot be observed safely; recovery is rejected")] + #[error("the standalone Worker is already active")] + WorkerActive, + #[error("the standalone Worker lease cannot be observed safely; recovery is rejected")] LeaseLivenessUnknown, - #[error("the standalone session working directory is unavailable or changed")] + #[error("the standalone Worker working directory is unavailable or changed")] WorkingDirectoryUnavailable, #[error("the resolved Worker configuration or persisted history is invalid")] WorkerConfiguration, @@ -67,7 +67,7 @@ pub enum StandaloneShutdownError { DeadlineExceeded, #[error("the standalone Worker shutdown confirmation was lost")] ConfirmationLost, - #[error("the standalone session final state could not be committed")] + #[error("the standalone Worker final state could not be committed")] StateStore, } @@ -87,22 +87,24 @@ impl StandaloneHost { } async fn start_with_optional_model_client( - mut launch: ResolvedStandaloneLaunch, + launch: ResolvedStandaloneLaunch, model_client: Option>, ) -> Result { - let store = StandaloneSessionStore::open(&launch.state_dir) - .map_err(classify_store_startup_error)?; + let store = + StandaloneWorkerStore::open(&launch.state_dir).map_err(classify_store_startup_error)?; let allocation = store .allocate(&launch.cwd, StaleLeasePolicy::Reject) .map_err(classify_store_startup_error)?; - let id = allocation.id(); + let worker_id = allocation.worker_id(); - // The standalone session ID is the local identity. A unique internal Worker name avoids - // process-global allocation collisions without creating a Runtime/Workspace Worker ID. - launch.profile.manifest.worker.name = format!("standalone-{id}"); + // WorkerId is the stable identity. The current Worker store remains + // name-keyed, so keep its derived storage key separate from the + // user-facing profile name. let manifest = launch.profile.manifest.clone(); - let worker_name = manifest.worker.name.clone(); - let (backing_store, worker_store) = match backing_store(&store, id) { + let storage_key = format!("standalone-{worker_id}"); + let mut bootstrap_manifest = manifest.clone(); + bootstrap_manifest.worker.name = storage_key.clone(); + let (backing_store, worker_store) = match backing_store(&store, worker_id) { Ok(stores) => stores, Err(error) => { let _ = store.abandon_allocation(allocation); @@ -112,10 +114,10 @@ impl StandaloneHost { let filesystem_authority = WorkerFilesystemAuthority::local(launch.cwd.clone(), launch.cwd.clone()); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); - let runtime_base = store.runtime_dir(id); + let runtime_base = store.runtime_dir(worker_id); let mut bootstrap = WorkerBootstrap::new( - manifest.clone(), + bootstrap_manifest, backing_store, launch.prompt_catalog, workspace_context, @@ -133,7 +135,7 @@ impl StandaloneHost { return Err(classify_startup_error(error)); } }; - let active = match active_pointer(&worker_store, &worker_name) { + let active = match active_pointer(&worker_store, &storage_key) { Ok(active) => active, Err(error) => { stop_started_worker(started).await; @@ -141,16 +143,20 @@ impl StandaloneHost { return Err(error); } }; - let record = - match store.commit_created(&allocation, manifest, active.session_id, active.segment_id) - { - Ok(record) => record, - Err(_) => { - stop_started_worker(started).await; - let _ = store.abandon_allocation(allocation); - return Err(StandaloneStartupError::StateStore); - } - }; + let record = match store.commit_created( + &allocation, + manifest, + storage_key, + active.session_id, + active.segment_id, + ) { + Ok(record) => record, + Err(_) => { + stop_started_worker(started).await; + let _ = store.abandon_allocation(allocation); + return Err(StandaloneStartupError::StateStore); + } + }; Ok(Self::from_started( started, store, @@ -162,50 +168,46 @@ impl StandaloneHost { pub async fn restore( state_dir: PathBuf, - session_id: StandaloneSessionId, + worker_id: WorkerId, ) -> Result { - Self::restore_with_optional_model_client(state_dir, session_id, None).await + Self::restore_with_optional_model_client(state_dir, worker_id, None).await } pub async fn restore_with_model_client( state_dir: PathBuf, - session_id: StandaloneSessionId, + worker_id: WorkerId, model_client: C, ) -> Result where C: LlmClient + 'static, { - Self::restore_with_optional_model_client( - state_dir, - session_id, - Some(Box::new(model_client)), - ) - .await + Self::restore_with_optional_model_client(state_dir, worker_id, Some(Box::new(model_client))) + .await } async fn restore_with_optional_model_client( state_dir: PathBuf, - session_id: StandaloneSessionId, + worker_id: WorkerId, model_client: Option>, ) -> Result { - let store = - StandaloneSessionStore::open(state_dir).map_err(classify_store_startup_error)?; + let store = StandaloneWorkerStore::open(state_dir).map_err(classify_store_startup_error)?; let record = store - .load(session_id) + .load(worker_id) .map_err(classify_store_startup_error)?; record.cwd.verify().map_err(classify_store_startup_error)?; let lease = store - .acquire_lease(session_id, StaleLeasePolicy::Recover) + .acquire_lease(worker_id, StaleLeasePolicy::Recover) .map_err(classify_store_startup_error)?; - let (backing_store, worker_store) = backing_store(&store, session_id)?; - let worker_name = record.worker_name.clone(); - let manifest = record.manifest.clone(); + let (backing_store, worker_store) = backing_store(&store, worker_id)?; + let storage_key = record.storage_key.clone(); + let mut manifest = record.manifest.clone(); + manifest.worker.name = storage_key.clone(); let filesystem_authority = WorkerFilesystemAuthority::local( record.cwd.canonical_path.clone(), record.cwd.canonical_path.clone(), ); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); - let runtime_base = store.runtime_dir(session_id); + let runtime_base = store.runtime_dir(worker_id); let mut bootstrap = WorkerBootstrap::new( manifest, @@ -220,11 +222,11 @@ impl StandaloneHost { bootstrap = bootstrap.with_model_client(model_client); } let prepared = bootstrap - .prepare_restored(&worker_name) + .prepare_restored(&storage_key) .await .map_err(classify_startup_error)?; let started = prepared.start().await.map_err(classify_startup_error)?; - let active = match active_pointer(&worker_store, &worker_name) { + let active = match active_pointer(&worker_store, &storage_key) { Ok(active) => active, Err(error) => { stop_started_worker(started).await; @@ -251,10 +253,10 @@ impl StandaloneHost { fn from_started( started: BootstrappedWorker, - store: StandaloneSessionStore, + store: StandaloneWorkerStore, worker_store: FsWorkerStore, - record: StandaloneSessionRecord, - lease: StandaloneSessionLease, + record: StandaloneWorkerRecord, + lease: StandaloneWorkerLease, ) -> Self { Self { handle: started.handle, @@ -268,12 +270,12 @@ impl StandaloneHost { } #[must_use] - pub fn session_id(&self) -> StandaloneSessionId { - self.record.session_id + pub fn worker_id(&self) -> WorkerId { + self.record.worker_id } #[must_use] - pub fn record(&self) -> &StandaloneSessionRecord { + pub fn record(&self) -> &StandaloneWorkerRecord { &self.record } @@ -310,7 +312,7 @@ impl StandaloneHost { return Err(StandaloneShutdownError::DeadlineExceeded); } } - let active = match active_pointer(&self.worker_store, &self.record.worker_name) { + let active = match active_pointer(&self.worker_store, &self.record.storage_key) { Ok(active) => active, Err(_) => { self.retain_lease(); @@ -451,12 +453,12 @@ async fn send_protocol_event(peer: &InProcessPeer, event: Event) -> bool { } fn backing_store( - store: &StandaloneSessionStore, - id: StandaloneSessionId, + store: &StandaloneWorkerStore, + worker_id: WorkerId, ) -> Result<(StandaloneBackingStore, FsWorkerStore), StandaloneStartupError> { - let session_store = - FsStore::new(store.session_log_dir(id)).map_err(|_| StandaloneStartupError::StateStore)?; - let worker_store = FsWorkerStore::new(store.worker_metadata_dir(id)) + let session_store = FsStore::new(store.sessions_dir(worker_id)) + .map_err(|_| StandaloneStartupError::StateStore)?; + let worker_store = FsWorkerStore::new(store.worker_metadata_dir(worker_id)) .map_err(|_| StandaloneStartupError::StateStore)?; Ok(( CombinedStore::new(session_store, worker_store.clone()), @@ -466,10 +468,10 @@ fn backing_store( fn active_pointer( worker_store: &FsWorkerStore, - worker_name: &str, + storage_key: &str, ) -> Result { worker_store - .read_by_name(worker_name) + .read_by_name(storage_key) .map_err(|_| StandaloneStartupError::StateStore)? .and_then(|metadata| metadata.active) .ok_or(StandaloneStartupError::StateStore) @@ -482,7 +484,7 @@ async fn stop_started_worker(started: BootstrappedWorker) { fn classify_store_startup_error(error: StandaloneStoreError) -> StandaloneStartupError { match error { - StandaloneStoreError::SessionLeased(_) => StandaloneStartupError::SessionActive, + StandaloneStoreError::WorkerLeased(_) => StandaloneStartupError::WorkerActive, StandaloneStoreError::LeaseLivenessUnknown(_) => { StandaloneStartupError::LeaseLivenessUnknown } diff --git a/crates/standalone/src/lib.rs b/crates/standalone/src/lib.rs index 8c04f863..100537a9 100644 --- a/crates/standalone/src/lib.rs +++ b/crates/standalone/src/lib.rs @@ -10,8 +10,8 @@ pub mod store; pub use host::{StandaloneHost, StandaloneShutdownError, StandaloneStartupError}; pub use launch::{ResolvedStandaloneLaunch, StandaloneLaunchConfig, StandaloneLaunchError}; +pub use protocol::WorkerId; pub use store::{ - StaleLeasePolicy, StandaloneCwdIdentity, StandaloneListScope, StandaloneSessionId, - StandaloneSessionRecord, StandaloneSessionStatus, StandaloneSessionStore, - StandaloneShutdownReason, StandaloneStoreError, + StaleLeasePolicy, StandaloneCwdIdentity, StandaloneListScope, StandaloneShutdownReason, + StandaloneStoreError, StandaloneWorkerRecord, StandaloneWorkerStatus, StandaloneWorkerStore, }; diff --git a/crates/standalone/src/store.rs b/crates/standalone/src/store.rs index e1dfea0c..78923f3a 100644 --- a/crates/standalone/src/store.rs +++ b/crates/standalone/src/store.rs @@ -1,12 +1,11 @@ -use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::str::FromStr; use std::time::{SystemTime, UNIX_EPOCH}; use fs4::fs_std::FileExt; use manifest::WorkerManifest; +use protocol::WorkerId; use serde::{Deserialize, Serialize}; use session_store::{SegmentId, SessionId}; use thiserror::Error; @@ -16,47 +15,10 @@ const RECORD_FILE: &str = "record.json"; const COMMIT_MARKER: &str = "commit.pending"; const LEASE_FILE: &str = "lease.json"; const LEASE_LOCK_FILE: &str = "lease.lock"; -const SESSION_DIR: &str = "session"; +const SESSIONS_DIR: &str = "sessions"; const WORKER_DIR: &str = "worker"; const SCHEMA_VERSION: u32 = 1; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -#[serde(transparent)] -pub struct StandaloneSessionId(Uuid); - -impl StandaloneSessionId { - #[must_use] - pub fn new() -> Self { - Self(Uuid::now_v7()) - } - - #[must_use] - pub fn short(self) -> String { - let simple = self.0.simple().to_string(); - simple[simple.len() - 12..].to_string() - } -} - -impl Default for StandaloneSessionId { - fn default() -> Self { - Self::new() - } -} - -impl fmt::Display for StandaloneSessionId { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(formatter) - } -} - -impl FromStr for StandaloneSessionId { - type Err = uuid::Error; - - fn from_str(value: &str) -> Result { - Uuid::parse_str(value).map(Self) - } -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct StandaloneCwdIdentity { pub canonical_path: PathBuf, @@ -100,7 +62,7 @@ impl StandaloneCwdIdentity { #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum StandaloneSessionStatus { +pub enum StandaloneWorkerStatus { Active, Stopped, } @@ -115,17 +77,20 @@ pub enum StandaloneShutdownReason { } #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct StandaloneSessionRecord { +pub struct StandaloneWorkerRecord { pub schema_version: u32, pub revision: u64, - pub session_id: StandaloneSessionId, + pub worker_id: WorkerId, + /// User-facing Worker name resolved from the profile. pub worker_name: String, + /// Internal key used by the current name-keyed Worker store. + pub storage_key: String, pub cwd: StandaloneCwdIdentity, pub manifest: WorkerManifest, pub active_session_id: SessionId, #[serde(default, skip_serializing_if = "Option::is_none")] pub active_segment_id: Option, - pub status: StandaloneSessionStatus, + pub status: StandaloneWorkerStatus, pub created_at_unix_ms: u64, pub updated_at_unix_ms: u64, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -145,11 +110,11 @@ pub enum StaleLeasePolicy { } #[derive(Debug, Clone)] -pub struct StandaloneSessionStore { +pub struct StandaloneWorkerStore { root: PathBuf, } -impl StandaloneSessionStore { +impl StandaloneWorkerStore { pub fn open(root: impl Into) -> Result { let root = root.into(); fs::create_dir_all(&root).map_err(StandaloneStoreError::Io)?; @@ -171,35 +136,41 @@ impl StandaloneSessionStore { &self, cwd: impl AsRef, policy: StaleLeasePolicy, - ) -> Result { - let id = StandaloneSessionId::new(); + ) -> Result { + let worker_id = WorkerId::now_v7(); let cwd = StandaloneCwdIdentity::capture(cwd)?; - let dir = self.session_dir(id); + let dir = self.worker_dir(worker_id); fs::create_dir(&dir).map_err(StandaloneStoreError::Io)?; - fs::create_dir(dir.join(SESSION_DIR)).map_err(StandaloneStoreError::Io)?; + fs::create_dir(dir.join(SESSIONS_DIR)).map_err(StandaloneStoreError::Io)?; fs::create_dir(dir.join(WORKER_DIR)).map_err(StandaloneStoreError::Io)?; - let lease = self.acquire_lease(id, policy)?; - Ok(StandaloneSessionAllocation { id, cwd, lease }) + let lease = self.acquire_lease(worker_id, policy)?; + Ok(StandaloneWorkerAllocation { + worker_id, + cwd, + lease, + }) } pub fn commit_created( &self, - allocation: &StandaloneSessionAllocation, + allocation: &StandaloneWorkerAllocation, manifest: WorkerManifest, + storage_key: String, active_session_id: SessionId, active_segment_id: Option, - ) -> Result { + ) -> Result { let now = now_unix_ms()?; - let record = StandaloneSessionRecord { + let record = StandaloneWorkerRecord { schema_version: SCHEMA_VERSION, revision: 1, - session_id: allocation.id, + worker_id: allocation.worker_id, worker_name: manifest.worker.name.clone(), + storage_key, cwd: allocation.cwd.clone(), manifest, active_session_id, active_segment_id, - status: StandaloneSessionStatus::Active, + status: StandaloneWorkerStatus::Active, created_at_unix_ms: now, updated_at_unix_ms: now, shutdown_reason: None, @@ -208,22 +179,19 @@ impl StandaloneSessionStore { Ok(record) } - pub fn load( - &self, - id: StandaloneSessionId, - ) -> Result { - let dir = self.session_dir(id); + pub fn load(&self, id: WorkerId) -> Result { + let dir = self.worker_dir(id); if dir.join(COMMIT_MARKER).exists() { return Err(StandaloneStoreError::IncompleteCommit(id)); } let bytes = fs::read(dir.join(RECORD_FILE)).map_err(|error| { if error.kind() == io::ErrorKind::NotFound { - StandaloneStoreError::SessionNotFound(id) + StandaloneStoreError::WorkerNotFound(id) } else { StandaloneStoreError::Io(error) } })?; - let record: StandaloneSessionRecord = serde_json::from_slice(&bytes) + let record: StandaloneWorkerRecord = serde_json::from_slice(&bytes) .map_err(|source| StandaloneStoreError::CorruptRecord { id, source })?; if record.schema_version > SCHEMA_VERSION { return Err(StandaloneStoreError::NewerSchema { @@ -232,7 +200,7 @@ impl StandaloneSessionStore { supported: SCHEMA_VERSION, }); } - if record.schema_version != SCHEMA_VERSION || record.session_id != id { + if record.schema_version != SCHEMA_VERSION || record.worker_id != id { return Err(StandaloneStoreError::InvalidRecord(id)); } Ok(record) @@ -243,7 +211,7 @@ impl StandaloneSessionStore { cwd: impl AsRef, scope: StandaloneListScope, limit: usize, - ) -> Result, StandaloneStoreError> { + ) -> Result, StandaloneStoreError> { let current_cwd = (scope == StandaloneListScope::CurrentCwd) .then(|| StandaloneCwdIdentity::capture(cwd)) .transpose()?; @@ -269,12 +237,7 @@ impl StandaloneSessionStore { right .updated_at_unix_ms .cmp(&left.updated_at_unix_ms) - .then_with(|| { - right - .session_id - .to_string() - .cmp(&left.session_id.to_string()) - }) + .then_with(|| right.worker_id.to_string().cmp(&left.worker_id.to_string())) }); records.truncate(limit); Ok(records) @@ -282,10 +245,10 @@ impl StandaloneSessionStore { pub fn acquire_lease( &self, - id: StandaloneSessionId, + id: WorkerId, policy: StaleLeasePolicy, - ) -> Result { - let dir = self.session_dir(id); + ) -> Result { + let dir = self.worker_dir(id); let path = dir.join(LEASE_FILE); let _guard = LeaseMutationGuard::acquire(&dir)?; let lease = LeaseRecord::current()?; @@ -296,7 +259,7 @@ impl StandaloneSessionStore { file.write_all(b"\n").map_err(StandaloneStoreError::Io)?; file.sync_all().map_err(StandaloneStoreError::Io)?; sync_directory(&dir)?; - return Ok(StandaloneSessionLease { + return Ok(StandaloneWorkerLease { path, lease_id: lease.lease_id, released: false, @@ -306,7 +269,7 @@ impl StandaloneSessionStore { let existing = read_lease(&path, id)?; match existing.liveness() { LeaseLiveness::Live => { - return Err(StandaloneStoreError::SessionLeased(id)); + return Err(StandaloneStoreError::WorkerLeased(id)); } LeaseLiveness::Unknown => { return Err(StandaloneStoreError::LeaseLivenessUnknown(id)); @@ -326,16 +289,16 @@ impl StandaloneSessionStore { pub fn update_active_pointer( &self, - record: &StandaloneSessionRecord, + record: &StandaloneWorkerRecord, active_session_id: SessionId, active_segment_id: Option, - ) -> Result { + ) -> Result { let mut next = record.clone(); next.revision = next.revision.saturating_add(1); next.updated_at_unix_ms = now_unix_ms()?; next.active_session_id = active_session_id; next.active_segment_id = active_segment_id; - next.status = StandaloneSessionStatus::Active; + next.status = StandaloneWorkerStatus::Active; next.shutdown_reason = None; self.commit_record(Some(record.revision), &next)?; Ok(next) @@ -343,73 +306,73 @@ impl StandaloneSessionStore { pub fn mark_stopped( &self, - record: &StandaloneSessionRecord, + record: &StandaloneWorkerRecord, active_session_id: SessionId, active_segment_id: Option, reason: StandaloneShutdownReason, - ) -> Result { + ) -> Result { let mut next = record.clone(); next.revision = next.revision.saturating_add(1); next.updated_at_unix_ms = now_unix_ms()?; next.active_session_id = active_session_id; next.active_segment_id = active_segment_id; - next.status = StandaloneSessionStatus::Stopped; + next.status = StandaloneWorkerStatus::Stopped; next.shutdown_reason = Some(reason); self.commit_record(Some(record.revision), &next)?; Ok(next) } - pub fn delete(&self, id: StandaloneSessionId) -> Result<(), StandaloneStoreError> { + pub fn delete(&self, id: WorkerId) -> Result<(), StandaloneStoreError> { let record = self.load(id)?; - if record.status != StandaloneSessionStatus::Stopped { + if record.status != StandaloneWorkerStatus::Stopped { return Err(StandaloneStoreError::DeleteActive(id)); } - let session_dir = self.session_dir(id); - let _guard = LeaseMutationGuard::acquire(&session_dir)?; - let lease_path = session_dir.join(LEASE_FILE); + let worker_dir = self.worker_dir(id); + let _guard = LeaseMutationGuard::acquire(&worker_dir)?; + let lease_path = worker_dir.join(LEASE_FILE); if lease_path.exists() { let lease = read_lease(&lease_path, id)?; return Err(match lease.liveness() { - LeaseLiveness::Live => StandaloneStoreError::SessionLeased(id), + LeaseLiveness::Live => StandaloneStoreError::WorkerLeased(id), LeaseLiveness::Stale => StandaloneStoreError::StaleLease(id), LeaseLiveness::Unknown => StandaloneStoreError::LeaseLivenessUnknown(id), }); } - fs::remove_dir_all(self.session_dir(id)).map_err(StandaloneStoreError::Io)?; + fs::remove_dir_all(self.worker_dir(id)).map_err(StandaloneStoreError::Io)?; sync_directory(&self.root) } #[must_use] - pub fn session_log_dir(&self, id: StandaloneSessionId) -> PathBuf { - self.session_dir(id).join(SESSION_DIR) + pub fn sessions_dir(&self, id: WorkerId) -> PathBuf { + self.worker_dir(id).join(SESSIONS_DIR) } #[must_use] - pub fn worker_metadata_dir(&self, id: StandaloneSessionId) -> PathBuf { - self.session_dir(id).join(WORKER_DIR) + pub fn worker_metadata_dir(&self, id: WorkerId) -> PathBuf { + self.worker_dir(id).join(WORKER_DIR) } #[must_use] - pub(crate) fn runtime_dir(&self, id: StandaloneSessionId) -> PathBuf { - self.session_dir(id).join("runtime") + pub(crate) fn runtime_dir(&self, id: WorkerId) -> PathBuf { + self.worker_dir(id).join("runtime") } pub(crate) fn abandon_allocation( &self, - allocation: StandaloneSessionAllocation, + allocation: StandaloneWorkerAllocation, ) -> Result<(), StandaloneStoreError> { - let id = allocation.id; + let worker_id = allocation.worker_id; allocation.lease.release()?; - fs::remove_dir_all(self.session_dir(id)).map_err(StandaloneStoreError::Io)?; + fs::remove_dir_all(self.worker_dir(worker_id)).map_err(StandaloneStoreError::Io)?; sync_directory(&self.root) } fn commit_record( &self, expected_revision: Option, - next: &StandaloneSessionRecord, + next: &StandaloneWorkerRecord, ) -> Result<(), StandaloneStoreError> { - let dir = self.session_dir(next.session_id); + let dir = self.worker_dir(next.worker_id); let marker = dir.join(COMMIT_MARKER); let mut marker_file = OpenOptions::new() .write(true) @@ -417,7 +380,7 @@ impl StandaloneSessionStore { .open(&marker) .map_err(|error| { if error.kind() == io::ErrorKind::AlreadyExists { - StandaloneStoreError::IncompleteCommit(next.session_id) + StandaloneStoreError::IncompleteCommit(next.worker_id) } else { StandaloneStoreError::Io(error) } @@ -427,11 +390,11 @@ impl StandaloneSessionStore { sync_directory(&dir)?; if let Some(expected) = expected_revision { - let current = self.load_record_while_committing(next.session_id)?; + let current = self.load_record_while_committing(next.worker_id)?; if current.revision != expected { let _ = fs::remove_file(&marker); return Err(StandaloneStoreError::RevisionConflict { - id: next.session_id, + id: next.worker_id, expected, found: current.revision, }); @@ -461,30 +424,30 @@ impl StandaloneSessionStore { fn load_record_while_committing( &self, - id: StandaloneSessionId, - ) -> Result { + id: WorkerId, + ) -> Result { let bytes = - fs::read(self.session_dir(id).join(RECORD_FILE)).map_err(StandaloneStoreError::Io)?; + fs::read(self.worker_dir(id).join(RECORD_FILE)).map_err(StandaloneStoreError::Io)?; serde_json::from_slice(&bytes) .map_err(|source| StandaloneStoreError::CorruptRecord { id, source }) } - fn session_dir(&self, id: StandaloneSessionId) -> PathBuf { + fn worker_dir(&self, id: WorkerId) -> PathBuf { self.root.join(id.to_string()) } } #[derive(Debug)] -pub struct StandaloneSessionAllocation { - id: StandaloneSessionId, +pub struct StandaloneWorkerAllocation { + worker_id: WorkerId, cwd: StandaloneCwdIdentity, - lease: StandaloneSessionLease, + lease: StandaloneWorkerLease, } -impl StandaloneSessionAllocation { +impl StandaloneWorkerAllocation { #[must_use] - pub fn id(&self) -> StandaloneSessionId { - self.id + pub fn worker_id(&self) -> WorkerId { + self.worker_id } #[must_use] @@ -492,19 +455,19 @@ impl StandaloneSessionAllocation { &self.cwd } - pub fn into_lease(self) -> StandaloneSessionLease { + pub fn into_lease(self) -> StandaloneWorkerLease { self.lease } } #[derive(Debug)] -pub struct StandaloneSessionLease { +pub struct StandaloneWorkerLease { path: PathBuf, lease_id: Uuid, released: bool, } -impl StandaloneSessionLease { +impl StandaloneWorkerLease { pub fn release(mut self) -> Result<(), StandaloneStoreError> { self.release_inner() } @@ -534,7 +497,7 @@ impl StandaloneSessionLease { } } -impl Drop for StandaloneSessionLease { +impl Drop for StandaloneWorkerLease { fn drop(&mut self) { let _ = self.release_inner(); } @@ -624,7 +587,7 @@ fn classify_lease_liveness( } } -fn read_lease(path: &Path, id: StandaloneSessionId) -> Result { +fn read_lease(path: &Path, id: WorkerId) -> Result { let bytes = fs::read(path).map_err(StandaloneStoreError::Io)?; serde_json::from_slice(&bytes) .map_err(|source| StandaloneStoreError::CorruptLease { id, source }) @@ -692,47 +655,47 @@ pub enum StandaloneStoreError { CwdUnavailable(#[source] io::Error), #[error("standalone cwd is not a directory")] CwdNotDirectory, - #[error("standalone cwd identity no longer matches the persisted session")] + #[error("standalone cwd identity no longer matches the persisted Worker")] CwdIdentityMismatch, - #[error("standalone session {0} was not found")] - SessionNotFound(StandaloneSessionId), - #[error("standalone session {0} has an incomplete metadata commit")] - IncompleteCommit(StandaloneSessionId), - #[error("standalone session {0} has invalid metadata")] - InvalidRecord(StandaloneSessionId), - #[error("standalone session {id} metadata is corrupt")] + #[error("standalone Worker {0} was not found")] + WorkerNotFound(WorkerId), + #[error("standalone Worker {0} has an incomplete metadata commit")] + IncompleteCommit(WorkerId), + #[error("standalone Worker {0} has invalid metadata")] + InvalidRecord(WorkerId), + #[error("standalone Worker {id} metadata is corrupt")] CorruptRecord { - id: StandaloneSessionId, + id: WorkerId, #[source] source: serde_json::Error, }, - #[error("standalone session {id} lease is corrupt")] + #[error("standalone Worker {id} lease is corrupt")] CorruptLease { - id: StandaloneSessionId, + id: WorkerId, #[source] source: serde_json::Error, }, - #[error("standalone session {id} uses schema {found}, newer than supported schema {supported}")] + #[error("standalone Worker {id} uses schema {found}, newer than supported schema {supported}")] NewerSchema { - id: StandaloneSessionId, + id: WorkerId, found: u32, supported: u32, }, - #[error("standalone session {0} is already active")] - SessionLeased(StandaloneSessionId), - #[error("standalone session {0} lease liveness cannot be proven; recovery is rejected")] - LeaseLivenessUnknown(StandaloneSessionId), - #[error("standalone session {0} has a stale lease; explicit recovery is required")] - StaleLease(StandaloneSessionId), - #[error("standalone session lease ownership changed")] + #[error("standalone Worker {0} is already active")] + WorkerLeased(WorkerId), + #[error("standalone Worker {0} lease liveness cannot be proven; recovery is rejected")] + LeaseLivenessUnknown(WorkerId), + #[error("standalone Worker {0} has a stale lease; explicit recovery is required")] + StaleLease(WorkerId), + #[error("standalone Worker lease ownership changed")] LeaseOwnershipLost, - #[error("standalone session {0} must be stopped before deletion")] - DeleteActive(StandaloneSessionId), + #[error("standalone Worker {0} must be stopped before deletion")] + DeleteActive(WorkerId), #[error( - "standalone session {id} metadata revision changed (expected {expected}, found {found})" + "standalone Worker {id} metadata revision changed (expected {expected}, found {found})" )] RevisionConflict { - id: StandaloneSessionId, + id: WorkerId, expected: u64, found: u64, }, diff --git a/crates/standalone/tests/host.rs b/crates/standalone/tests/host.rs index 8198f3ac..755f5718 100644 --- a/crates/standalone/tests/host.rs +++ b/crates/standalone/tests/host.rs @@ -14,7 +14,7 @@ use futures::{Stream, stream}; use protocol::{Event, Method}; use standalone::{ StaleLeasePolicy, StandaloneHost, StandaloneLaunchConfig, StandaloneListScope, - StandaloneSessionStatus, StandaloneSessionStore, StandaloneStartupError, StandaloneStoreError, + StandaloneStartupError, StandaloneStoreError, StandaloneWorkerStatus, StandaloneWorkerStore, }; use uuid::Uuid; @@ -90,6 +90,12 @@ async fn in_process_host_runs_text_and_read_tool_then_shuts_down() { let host = StandaloneHost::start_with_model_client(launch, client) .await .expect("start in-process host"); + assert_eq!(host.record().worker_name, worker_name); + assert_eq!(host.record().manifest.worker.name, worker_name); + assert_eq!( + host.record().storage_key, + format!("standalone-{}", host.worker_id()) + ); let mut protocol_client = host.connect(); protocol_client @@ -238,7 +244,7 @@ type TestResult = Result<(), Box>; async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope() -> TestResult { let temp = tempfile::tempdir()?; let cwd = temp.path().join("project"); - let state_dir = temp.path().join("client").join("standalone-sessions"); + let state_dir = temp.path().join("client").join("standalone-workers"); std::fs::create_dir_all(&cwd)?; let launch = StandaloneLaunchConfig::new( &cwd, @@ -268,7 +274,7 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( ], ]); let host = StandaloneHost::start_with_model_client(launch, first_client).await?; - let session_id = host.session_id(); + let worker_id = host.worker_id(); let mut protocol_client = host.connect(); protocol_client .send(&Method::run_text("first request")) @@ -283,11 +289,11 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( wait_for_run_end(&mut protocol_client).await?; host.shutdown().await?; - let store = StandaloneSessionStore::open(&state_dir)?; + let store = StandaloneWorkerStore::open(&state_dir)?; let current = store.list(&cwd, StandaloneListScope::CurrentCwd, 100)?; assert_eq!(current.len(), 1); - assert_eq!(current[0].session_id, session_id); - assert_eq!(current[0].status, StandaloneSessionStatus::Stopped); + assert_eq!(current[0].worker_id, worker_id); + assert_eq!(current[0].status, StandaloneWorkerStatus::Stopped); let other_cwd = temp.path().join("other"); std::fs::create_dir(&other_cwd)?; assert!( @@ -307,8 +313,13 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( ]]); let second_inspection = second_client.clone(); let host = - StandaloneHost::restore_with_model_client(state_dir.clone(), session_id, second_client) + StandaloneHost::restore_with_model_client(state_dir.clone(), worker_id, second_client) .await?; + assert_eq!( + host.record().worker_name, + "display-name-is-not-session-identity" + ); + assert_eq!(host.record().storage_key, format!("standalone-{worker_id}")); let mut protocol_client = host.connect(); let snapshot = format!( "{:?}", @@ -338,11 +349,11 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( assert!(projected.contains("persisted task"), "{projected}"); host.shutdown().await?; - store.delete(session_id)?; + store.delete(worker_id)?; assert!(cwd.exists(), "deleting session state must not mutate cwd"); assert!(matches!( - store.load(session_id), - Err(StandaloneStoreError::SessionNotFound(_)) + store.load(worker_id), + Err(StandaloneStoreError::WorkerNotFound(_)) )); Ok(()) } @@ -363,28 +374,25 @@ async fn standalone_restore_rejects_concurrent_lease_and_missing_cwd() -> TestRe .resolve()?; let host = StandaloneHost::start_with_model_client(launch, ScriptedClient::new(Vec::new())).await?; - let session_id = host.session_id(); - let store = StandaloneSessionStore::open(&state_dir)?; + let worker_id = host.worker_id(); + let store = StandaloneWorkerStore::open(&state_dir)?; assert!(matches!( - store.acquire_lease(session_id, StaleLeasePolicy::Recover), - Err(StandaloneStoreError::SessionLeased(id)) if id == session_id + store.acquire_lease(worker_id, StaleLeasePolicy::Recover), + Err(StandaloneStoreError::WorkerLeased(id)) if id == worker_id )); let restore = StandaloneHost::restore_with_model_client( state_dir.clone(), - session_id, + worker_id, ScriptedClient::new(Vec::new()), ) .await; - assert!(matches!( - restore, - Err(StandaloneStartupError::SessionActive) - )); + assert!(matches!(restore, Err(StandaloneStartupError::WorkerActive))); host.shutdown().await?; std::fs::rename(&cwd, &moved)?; let restore = StandaloneHost::restore_with_model_client( state_dir, - session_id, + worker_id, ScriptedClient::new(Vec::new()), ) .await; @@ -421,11 +429,11 @@ async fn standalone_restore_recovers_only_a_proven_stale_lease() -> TestResult { }); let host = StandaloneHost::start_with_model_client(launch, ScriptedClient::new(Vec::new())).await?; - let session_id = host.session_id(); + let worker_id = host.worker_id(); host.shutdown().await?; - let store = StandaloneSessionStore::open(&state_dir)?; + let store = StandaloneWorkerStore::open(&state_dir)?; assert!(matches!( - store.load(session_id)?.manifest.profile, + store.load(worker_id)?.manifest.profile, Some(manifest::ProfileManifestSnapshot { source: manifest::ProfileSource::Registry { source: manifest::ProfileRegistrySource::User, @@ -434,9 +442,9 @@ async fn standalone_restore_recovers_only_a_proven_stale_lease() -> TestResult { .. }) )); - let session_dir = state_dir.join(session_id.to_string()); + let worker_dir = state_dir.join(worker_id.to_string()); std::fs::write( - session_dir.join("lease.json"), + worker_dir.join("lease.json"), serde_json::to_vec(&serde_json::json!({ "lease_id": uuid::Uuid::now_v7(), "pid": u32::MAX, @@ -447,7 +455,7 @@ async fn standalone_restore_recovers_only_a_proven_stale_lease() -> TestResult { let host = StandaloneHost::restore_with_model_client( state_dir, - session_id, + worker_id, ScriptedClient::new(Vec::new()), ) .await?; @@ -468,11 +476,11 @@ async fn standalone_restore_rejects_lease_with_missing_start_marker() -> TestRes .resolve()?; let host = StandaloneHost::start_with_model_client(launch, ScriptedClient::new(Vec::new())).await?; - let session_id = host.session_id(); + let worker_id = host.worker_id(); host.shutdown().await?; - let session_dir = state_dir.join(session_id.to_string()); + let worker_dir = state_dir.join(worker_id.to_string()); std::fs::write( - session_dir.join("lease.json"), + worker_dir.join("lease.json"), serde_json::to_vec(&serde_json::json!({ "lease_id": uuid::Uuid::now_v7(), "pid": std::process::id(), @@ -480,14 +488,14 @@ async fn standalone_restore_rejects_lease_with_missing_start_marker() -> TestRes }))?, )?; - let store = StandaloneSessionStore::open(&state_dir)?; + let store = StandaloneWorkerStore::open(&state_dir)?; assert!(matches!( - store.acquire_lease(session_id, StaleLeasePolicy::Recover), - Err(StandaloneStoreError::LeaseLivenessUnknown(id)) if id == session_id + store.acquire_lease(worker_id, StaleLeasePolicy::Recover), + Err(StandaloneStoreError::LeaseLivenessUnknown(id)) if id == worker_id )); let restore = StandaloneHost::restore_with_model_client( state_dir, - session_id, + worker_id, ScriptedClient::new(Vec::new()), ) .await; @@ -511,23 +519,23 @@ async fn standalone_metadata_fails_closed_on_incomplete_or_newer_records() -> Te .resolve()?; let host = StandaloneHost::start_with_model_client(launch, ScriptedClient::new(Vec::new())).await?; - let session_id = host.session_id(); + let worker_id = host.worker_id(); host.shutdown().await?; - let store = StandaloneSessionStore::open(&state_dir)?; - let session_dir = state_dir.join(session_id.to_string()); - std::fs::write(session_dir.join("commit.pending"), b"interrupted\n")?; + let store = StandaloneWorkerStore::open(&state_dir)?; + let worker_dir = state_dir.join(worker_id.to_string()); + std::fs::write(worker_dir.join("commit.pending"), b"interrupted\n")?; assert!(matches!( - store.load(session_id), - Err(StandaloneStoreError::IncompleteCommit(id)) if id == session_id + store.load(worker_id), + Err(StandaloneStoreError::IncompleteCommit(id)) if id == worker_id )); - std::fs::remove_file(session_dir.join("commit.pending"))?; - let record_path = session_dir.join("record.json"); + std::fs::remove_file(worker_dir.join("commit.pending"))?; + let record_path = worker_dir.join("record.json"); let mut record: serde_json::Value = serde_json::from_slice(&std::fs::read(&record_path)?)?; record["schema_version"] = serde_json::json!(u32::MAX); std::fs::write(&record_path, serde_json::to_vec_pretty(&record)?)?; assert!(matches!( - store.load(session_id), - Err(StandaloneStoreError::NewerSchema { id, .. }) if id == session_id + store.load(worker_id), + Err(StandaloneStoreError::NewerSchema { id, .. }) if id == worker_id )); Ok(()) } diff --git a/crates/tui/src/console/mod.rs b/crates/tui/src/console/mod.rs index ec0454ff..6a63dff0 100644 --- a/crates/tui/src/console/mod.rs +++ b/crates/tui/src/console/mod.rs @@ -25,9 +25,7 @@ use tokio::sync::mpsc; use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64_STANDARD}; use client::transport::Socket; -use client::{ - BackendRuntimeTarget, Client, StandaloneSessionResumeIntent, connect_backend_runtime, -}; +use client::{BackendRuntimeTarget, Client, StandaloneWorkerResumeIntent, connect_backend_runtime}; use crate::app::{ActionbarNoticeLevel, ActionbarNoticeSource, App}; use crate::composer_keys::{ComposerEditAction, composer_edit_action}; @@ -193,18 +191,18 @@ pub(crate) async fn run_standalone( } pub(crate) async fn run_standalone_restore( - intent: StandaloneSessionResumeIntent, + intent: StandaloneWorkerResumeIntent, ) -> Result<(), Box> { - let session_id = intent.session_id.parse().map_err(|error| { + let worker_id = intent.worker_id.parse().map_err(|error| { io::Error::new( io::ErrorKind::InvalidInput, - format!("Invalid standalone session ID: {error}"), + format!("Invalid standalone Worker ID: {error}"), ) })?; - let host = StandaloneHost::restore(intent.state_dir, session_id) + let host = StandaloneHost::restore(intent.state_dir, worker_id) .await .map_err(|error| io::Error::other(format!("Standalone restore failed: {error}")))?; - let worker_label = format!("standalone-{}", session_id.short()); + let worker_label = host.record().worker_name.clone(); let history_root = host.record().cwd.canonical_path.clone(); run_standalone_host(host, worker_label, history_root).await } diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 724b9fc0..71eacd14 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -46,8 +46,8 @@ pub enum LaunchMode { worker_name: Option, profile: Option, }, - /// Restore one client-owned standalone session. The current cwd is the default scope; - /// `include_all` opts into all standalone sessions under the same client data root. + /// Restore one client-owned standalone Worker. The current cwd is the default scope; + /// `include_all` opts into all standalone Workers under the same client data root. StandaloneResume { include_all: bool }, /// List Backend Workers and attach to the selected Worker. Workers { diff --git a/crates/tui/src/standalone_picker.rs b/crates/tui/src/standalone_picker.rs index 44581429..35ccbb5f 100644 --- a/crates/tui/src/standalone_picker.rs +++ b/crates/tui/src/standalone_picker.rs @@ -1,7 +1,7 @@ use std::io; use std::time::Duration; -use client::{StandaloneSessionListIntent, StandaloneSessionResumeIntent, Target}; +use client::{StandaloneWorkerListIntent, StandaloneWorkerResumeIntent, Target}; use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers}; use ratatui::Terminal; use ratatui::backend::CrosstermBackend; @@ -9,7 +9,7 @@ use ratatui::layout::{Constraint, Layout}; use ratatui::prelude::{Color, Line, Modifier, Span, Style}; use ratatui::widgets::Paragraph; use ratatui::{TerminalOptions, Viewport}; -use standalone::{StandaloneListScope, StandaloneSessionRecord, StandaloneSessionStore}; +use standalone::{StandaloneListScope, StandaloneWorkerRecord, StandaloneWorkerStore}; use thiserror::Error; const LIMIT: usize = 100; @@ -17,28 +17,28 @@ const LIMIT: usize = 100; pub(crate) fn pick( target: &dyn Target, include_all: bool, -) -> Result, StandalonePickerError> { +) -> Result, StandalonePickerError> { let intent = target - .standalone_session_list(include_all) + .standalone_worker_list(include_all) .map_err(StandalonePickerError::Target)?; let records = load_records(&intent)?; if records.is_empty() { - return Err(StandalonePickerError::NoSessions { include_all }); + return Err(StandalonePickerError::NoWorkers { include_all }); } let selected = run_picker(records)?; selected .map(|record| { target - .standalone_session_resume(record.session_id.to_string()) + .standalone_worker_resume(record.worker_id.to_string()) .map_err(StandalonePickerError::Target) }) .transpose() } fn load_records( - intent: &StandaloneSessionListIntent, -) -> Result, StandalonePickerError> { - let store = StandaloneSessionStore::open(&intent.state_dir) + intent: &StandaloneWorkerListIntent, +) -> Result, StandalonePickerError> { + let store = StandaloneWorkerStore::open(&intent.state_dir) .map_err(StandalonePickerError::StateStore)?; store .list( @@ -54,8 +54,8 @@ fn load_records( } fn run_picker( - records: Vec, -) -> Result, StandalonePickerError> { + records: Vec, +) -> Result, StandalonePickerError> { let height = u16::try_from(records.len().saturating_add(3).min(20)).unwrap_or(20); let mut terminal = Terminal::with_options( CrosstermBackend::new(io::stdout()), @@ -94,14 +94,14 @@ fn run_picker( } } -fn draw(frame: &mut ratatui::Frame<'_>, records: &[StandaloneSessionRecord], selected: usize) { +fn draw(frame: &mut ratatui::Frame<'_>, records: &[StandaloneWorkerRecord], selected: usize) { let mut constraints = vec![Constraint::Length(1)]; constraints.extend(records.iter().map(|_| Constraint::Length(1))); constraints.push(Constraint::Length(1)); let rows = Layout::vertical(constraints).split(frame.area()); frame.render_widget( Paragraph::new(Line::from(Span::styled( - "resume standalone session", + "resume standalone Worker", Style::default().add_modifier(Modifier::BOLD), ))), rows[0], @@ -120,7 +120,10 @@ fn draw(frame: &mut ratatui::Frame<'_>, records: &[StandaloneSessionRecord], sel frame.render_widget( Paragraph::new(Line::from(vec![ Span::raw(marker), - Span::styled(record.session_id.short(), style), + Span::styled( + format!("{} ({})", record.worker_name, record.worker_id.short()), + style, + ), Span::raw(format!( " [{:?}] updated:{} {}", record.status, record.updated_at_unix_ms, cwd @@ -139,13 +142,13 @@ fn draw(frame: &mut ratatui::Frame<'_>, records: &[StandaloneSessionRecord], sel pub(crate) enum StandalonePickerError { #[error("standalone target error: {0}")] Target(#[source] client::TargetError), - #[error("standalone session state is unavailable: {0}")] + #[error("standalone Worker state is unavailable: {0}")] StateStore(#[source] standalone::StandaloneStoreError), #[error( - "no standalone sessions found for this cwd; use `yoi --local --resume --all` to include all cwd identities" + "no standalone Workers found for this cwd; use `yoi --local --resume --all` to include all cwd identities" )] - NoSessions { include_all: bool }, - #[error("standalone session picker I/O failed: {0}")] + NoWorkers { include_all: bool }, + #[error("standalone Worker picker I/O failed: {0}")] Io(#[source] io::Error), } diff --git a/crates/worker-runtime/src/identity.rs b/crates/worker-runtime/src/identity.rs index c16fa7c3..3b1b85dd 100644 --- a/crates/worker-runtime/src/identity.rs +++ b/crates/worker-runtime/src/identity.rs @@ -1,105 +1,9 @@ -use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; +use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use std::{fmt, str::FromStr}; -use uuid::{Uuid, Version}; +pub use protocol::{WorkerId, WorkerIdParseError}; pub use workdir::workspace::RuntimeWorkerRef; -/// Stable Workspace-owned Worker identity. -/// -/// Runtime placement is deliberately not part of this value. New identities are -/// allocated by Workspace authority before a Runtime create request is sent. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub struct WorkerId(Uuid); - -impl WorkerId { - pub fn now_v7() -> Self { - Self(Uuid::now_v7()) - } - - /// Converts a legacy Runtime-local numeric id into a syntactically valid - /// migration-only UUIDv7 value. New Worker allocation must use `now_v7`. - pub fn from_legacy_u64(value: u64) -> Self { - let mut bytes = [0_u8; 16]; - bytes[8..].copy_from_slice(&value.to_be_bytes()); - bytes[6] = 0x70; - bytes[8] = (bytes[8] & 0x3f) | 0x80; - Self(Uuid::from_bytes(bytes)) - } - - pub fn from_legacy_binding(workspace_id: &str, runtime_id: &str, value: u64) -> Self { - let mut hasher = Sha256::new(); - hasher.update(b"yoi.workspace-worker-id.v1\0"); - hasher.update(workspace_id.as_bytes()); - hasher.update([0]); - hasher.update(runtime_id.as_bytes()); - hasher.update([0]); - hasher.update(value.to_be_bytes()); - let digest = hasher.finalize(); - let mut bytes = [0_u8; 16]; - bytes.copy_from_slice(&digest[..16]); - // Migrated ids sort before normally allocated UUIDv7 values while retaining - // deterministic collision-resistant payload bits. - bytes[..6].fill(0); - bytes[6] = (bytes[6] & 0x0f) | 0x70; - bytes[8] = (bytes[8] & 0x3f) | 0x80; - Self(Uuid::from_bytes(bytes)) - } - - pub fn parse(value: &str) -> Option { - let value = Uuid::parse_str(value).ok()?; - (value.get_version() == Some(Version::SortRand)).then_some(Self(value)) - } - - pub const fn as_uuid(self) -> Uuid { - self.0 - } -} - -impl fmt::Display for WorkerId { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(formatter) - } -} - -impl FromStr for WorkerId { - type Err = WorkerIdParseError; - - fn from_str(value: &str) -> Result { - Self::parse(value).ok_or(WorkerIdParseError) - } -} - -impl Serialize for WorkerId { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&self.to_string()) - } -} - -impl<'de> Deserialize<'de> for WorkerId { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let value = String::deserialize(deserializer)?; - Self::parse(&value).ok_or_else(|| de::Error::custom("Worker id must be a UUIDv7")) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct WorkerIdParseError; - -impl fmt::Display for WorkerIdParseError { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("Worker id must be a UUIDv7") - } -} - -impl std::error::Error for WorkerIdParseError {} - #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct LegacyWorkerIdentityMapping { pub workspace_id: String, @@ -140,7 +44,7 @@ pub fn legacy_worker_identity_mapping_digest(mappings: &[LegacyWorkerIdentityMap } /// Runtime-local authority reference for Worker operations. The contained id is -/// nevertheless the Workspace-owned stable identity; the Runtime does not mint it. +/// nevertheless the stable Worker identity; the Runtime does not mint it. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub struct WorkerRef { pub worker_id: WorkerId, @@ -164,14 +68,6 @@ impl TryFrom<&RuntimeWorkerRef> for WorkerRef { mod tests { use super::*; - #[test] - fn worker_id_accepts_only_uuid_v7() { - let worker_id = WorkerId::now_v7(); - assert_eq!(WorkerId::parse(&worker_id.to_string()), Some(worker_id)); - assert!(WorkerId::parse("30").is_none()); - assert!(WorkerId::parse(&Uuid::nil().to_string()).is_none()); - } - #[test] fn runtime_worker_ref_preserves_stable_worker_identity() { let worker_id = WorkerId::now_v7(); diff --git a/crates/yoi/src/cli_connection.rs b/crates/yoi/src/cli_connection.rs index a9670187..7a669ab7 100644 --- a/crates/yoi/src/cli_connection.rs +++ b/crates/yoi/src/cli_connection.rs @@ -118,7 +118,7 @@ fn standalone_target() -> Result, ParseError> { })? .join("client") .join("standalone") - .join("sessions"); + .join("workers"); Ok(Box::new(StandaloneTarget::new(state_dir))) } diff --git a/crates/yoi/src/main.rs b/crates/yoi/src/main.rs index 1c97da5f..8700b670 100644 --- a/crates/yoi/src/main.rs +++ b/crates/yoi/src/main.rs @@ -798,7 +798,7 @@ fn parse_console_options( } if session.is_some() { return Err(ParseError( - "--local does not accept legacy --session; use --local --resume for Standalone session restore" + "--local does not accept legacy --session; use --local --resume for Standalone Worker restore" .to_string(), )); } @@ -834,7 +834,7 @@ fn parse_console_options( if target.kind() == TargetKind::Standalone && (session.is_some() || socket_override.is_some()) { return Err(ParseError( - "Standalone does not accept legacy Worker session or socket selectors; use --resume for the standalone session store" + "Standalone does not accept legacy Worker session or socket selectors; use --resume for the standalone Worker store" .to_string(), )); } @@ -951,7 +951,7 @@ fn parse_workers_args( )?; if target.kind() != TargetKind::Backend { return Err(ParseError( - "yoi workers requires a Backend connection target; use yoi --local --resume for Standalone sessions" + "yoi workers requires a Backend connection target; use yoi --local --resume for Standalone Workers" .to_string(), )); } @@ -1750,8 +1750,8 @@ Usage: Target selection: --local Use the client-owned one-process Standalone host - --resume With --local, restore from the Standalone session store - --all With Standalone restore, include sessions from every cwd identity + --resume With --local, restore from the Standalone Worker store + --all With Standalone restore, include Workers from every cwd identity --backend Use a Workspace Backend explicitly --workspace-id Scope Backend routes to a Workspace id @@ -1761,7 +1761,7 @@ Target selection: Connection-aware commands: yoi Standalone: new Console. Backend: Worker picker. - yoi resume Standalone session picker or stopped Backend Worker picker. + yoi resume Standalone Worker picker or stopped Backend Worker picker. yoi workers Backend Workspace Worker picker. yoi panel Backend Workspace dashboard. @@ -1800,7 +1800,7 @@ Usage: yoi --backend [--workspace-id ] workers [-r|--stopped] [--workspace ] [--runtime-id ] Authority: - Lists Workers from the selected Backend Workspace. Standalone sessions are restored with + Lists Workers from the selected Backend Workspace. Standalone Workers are restored with `yoi --local --resume` and are not part of the Workspace Worker catalog. Options: @@ -1822,13 +1822,13 @@ Usage: yoi [TARGET] resume [--workspace |--all] [--runtime-id ] Target options: - --local Restore from the client-owned Standalone session store + --local Restore from the client-owned Standalone Worker store --backend Restore a stopped Backend Workspace Worker --workspace-id Scope Backend routes to a Workspace id Options: - --workspace Scope Standalone sessions to this cwd identity (defaults to cwd) - --all Include Standalone sessions from every cwd identity + --workspace Scope Standalone Workers to this cwd identity (defaults to cwd) + --all Include Standalone Workers from every cwd identity --runtime-id Restrict the Backend stopped-Worker picker to a Runtime id -h, --help Print help "#; @@ -2222,8 +2222,8 @@ backend = "shared" mode, LaunchMode::StandaloneResume { include_all: false } )); - let intent = target.standalone_session_list(false).unwrap(); - assert!(intent.state_dir.ends_with("client/standalone/sessions")); + let intent = target.standalone_worker_list(false).unwrap(); + assert!(intent.state_dir.ends_with("client/standalone/workers")); assert!(!intent.include_all); let mode = parse_args_from(["--local", "--resume", "--all"]).unwrap(); @@ -2264,7 +2264,7 @@ backend = "shared" let err = parse_args_slice_with_connection_resolver(&session_args, &resolver).unwrap_err(); assert_eq!( err.0, - "--local does not accept legacy --session; use --local --resume for Standalone session restore" + "--local does not accept legacy --session; use --local --resume for Standalone Worker restore" ); let socket_args = [ @@ -2903,12 +2903,12 @@ backend = "shared" } #[test] - fn parse_resume_help_uses_standalone_session_store_terminology() { + fn parse_resume_help_uses_standalone_worker_store_terminology() { match parse_args_from(["resume", "--help"]).unwrap() { Mode::ResumeHelp => {} _ => panic!("expected ResumeHelp mode"), } - assert!(RESUME_HELP.contains("Standalone session store")); + assert!(RESUME_HELP.contains("Standalone Worker store")); assert!(RESUME_HELP.contains("Backend stopped-Worker picker")); assert!(!RESUME_HELP.contains("local Worker records")); assert!(!RESUME_HELP.contains("local workspace")); From 62eaefb1fa6b18b07e519acb6c5816b085b0128f Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 16:52:27 +0900 Subject: [PATCH 6/7] feat: spill long bash output to worker temp storage --- crates/fs-operation/src/lib.rs | 2 + crates/fs-operation/src/operation.rs | 24 +- crates/standalone/src/host.rs | 16 +- crates/tools/src/bash.rs | 140 +++++++++++- crates/tools/src/grep.rs | 4 +- crates/tools/src/read.rs | 6 +- crates/tools/tests/edge_cases.rs | 13 +- crates/tools/tests/integration.rs | 13 +- crates/workdir/src/delegation.rs | 5 + crates/workdir/src/local.rs | 234 +++++++++++++++++++- crates/workdir/src/operation.rs | 7 + crates/worker-runtime/src/http_server.rs | 1 + crates/worker-runtime/src/worker_backend.rs | 17 +- crates/worker/examples/worker_protocol.rs | 4 +- crates/worker/src/bootstrap.rs | 69 +++++- crates/worker/src/controller.rs | 75 +++++-- crates/worker/src/entrypoint.rs | 2 + crates/worker/src/lib.rs | 2 +- crates/worker/src/spawn/tool.rs | 58 ++++- crates/worker/tests/compact_events_test.rs | 3 +- crates/worker/tests/controller_test.rs | 46 +++- crates/workspace-server/src/server.rs | 1 + 22 files changed, 664 insertions(+), 78 deletions(-) diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index fb2cd54f..1488d505 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -177,6 +177,8 @@ mod tests { fn logical_paths_reject_absolute_parent_and_backslash_forms() { assert!(FsPath::new("src/lib.rs").is_ok()); assert!(FsPath::new("/tmp/file").is_err()); + assert!(FsPath::new_scoped("/tmp/file").is_ok()); + assert!(FsPath::new_scoped("/tmp/../secret").is_err()); assert!(FsPath::new("../file").is_err()); assert!(FsPath::new("src\\lib.rs").is_err()); } diff --git a/crates/fs-operation/src/operation.rs b/crates/fs-operation/src/operation.rs index b69ce30b..a883b8ee 100644 --- a/crates/fs-operation/src/operation.rs +++ b/crates/fs-operation/src/operation.rs @@ -5,7 +5,8 @@ use serde::{Deserialize, Serialize}; use crate::FsError; -/// Logical path relative to the bound Workdir root. +/// Scope-checked filesystem path. Relative paths resolve below the bound +/// Workdir root; absolute paths require an explicit matching scope rule. #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)] #[serde(transparent)] pub struct FsPath(String); @@ -16,11 +17,30 @@ impl<'de> Deserialize<'de> for FsPath { D: serde::Deserializer<'de>, { let value = String::deserialize(deserializer)?; - Self::new(&value).map_err(serde::de::Error::custom) + Self::new_scoped(&value).map_err(serde::de::Error::custom) } } impl FsPath { + /// Construct a path for a scope-checked operation that may target an + /// explicitly granted absolute path outside the provider root. + pub fn new_scoped(value: impl Into) -> Result { + let value = value.into(); + if !Path::new(&value).is_absolute() { + return Self::new(value); + } + if value.contains('\\') { + return Err(FsError::InvalidPath(value)); + } + if Path::new(&value) + .components() + .any(|component| component == Component::ParentDir) + { + return Err(FsError::InvalidPath(value)); + } + Ok(Self(value)) + } + pub fn root() -> Self { Self(String::new()) } diff --git a/crates/standalone/src/host.rs b/crates/standalone/src/host.rs index 7bfc1e51..405ad8af 100644 --- a/crates/standalone/src/host.rs +++ b/crates/standalone/src/host.rs @@ -10,7 +10,9 @@ use session_store::{ CombinedStore, FsStore, FsWorkerStore, WorkerActiveSegmentRef, WorkerMetadataStore, }; use thiserror::Error; -use worker::bootstrap::{WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout}; +use worker::bootstrap::{ + WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout, bash_output_dir_for_worker_id, +}; use worker::controller::WorkerControllerTransport; use worker::ipc::protocol_session::{ WorkerProtocolSessionStreams, dispatch_worker_protocol_method, live_log_entry_event, @@ -115,6 +117,7 @@ impl StandaloneHost { WorkerFilesystemAuthority::local(launch.cwd.clone(), launch.cwd.clone()); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); let runtime_base = store.runtime_dir(worker_id); + let bash_output_dir = bash_output_dir_for_worker_id(worker_id); let mut bootstrap = WorkerBootstrap::new( bootstrap_manifest, @@ -122,7 +125,10 @@ impl StandaloneHost { launch.prompt_catalog, workspace_context, filesystem_authority, - WorkerBootstrapLayout::Direct { runtime_base }, + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + }, WorkerControllerTransport::InProcess, ); if let Some(model_client) = model_client { @@ -208,6 +214,7 @@ impl StandaloneHost { ); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); let runtime_base = store.runtime_dir(worker_id); + let bash_output_dir = bash_output_dir_for_worker_id(worker_id); let mut bootstrap = WorkerBootstrap::new( manifest, @@ -215,7 +222,10 @@ impl StandaloneHost { worker::PromptCatalogSource::builtins_only(), workspace_context, filesystem_authority, - WorkerBootstrapLayout::Direct { runtime_base }, + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + }, WorkerControllerTransport::InProcess, ); if let Some(model_client) = model_client { diff --git a/crates/tools/src/bash.rs b/crates/tools/src/bash.rs index bc573849..fb68fcbc 100644 --- a/crates/tools/src/bash.rs +++ b/crates/tools/src/bash.rs @@ -21,6 +21,7 @@ struct BashParams { pub(crate) struct BashTool { session: WorkdirSessionHandle, + output_dir: PathBuf, state: Arc>, } @@ -117,6 +118,7 @@ impl Tool for BashTool { command: params.command, timeout_secs, output_limit: INLINE_BYTE_BUDGET, + spill_dir: Some(self.output_dir.clone()), tool_call_id: Some(call_id.clone()), }) .await @@ -183,10 +185,15 @@ impl Tool for BashTool { let content = if output.content.is_empty() { None } else if output.truncated { - Some(format!( - "[showing bounded WorkdirSession command output; additional output was truncated]\n{}", - output.content - )) + let notice = match output.output_path { + Some(path) => format!( + "[showing bounded WorkdirSession command output; full output saved to {}]", + path.display() + ), + None => "[showing bounded WorkdirSession command output; additional output was truncated]" + .to_owned(), + }; + Some(format!("{notice}\n{}", output.content)) } else { Some(output.content) }; @@ -259,16 +266,137 @@ fn truncate_for_summary(command: &str) -> String { summary } -pub fn bash_tool(session: WorkdirSessionHandle, _output_dir: PathBuf) -> ToolDefinition { +pub fn bash_tool(session: WorkdirSessionHandle, output_dir: PathBuf) -> ToolDefinition { Arc::new(move || { let schema = schemars::schema_for!(BashParams); let meta = ToolMeta::new("Bash") - .description("Execute a shell command in the bound Workdir. Process start, bounded output, timeout and cancellation are owned by the WorkdirSession provider. This is not a sandbox.") + .description("Execute a shell command in the bound Workdir. Process start, bounded inline output, full-output spill, timeout and cancellation are owned by the WorkdirSession provider. This is not a sandbox.") .input_schema(serde_json::to_value(schema).expect("Bash schema serialization")); let tool: Arc = Arc::new(BashTool { session: session.clone(), + output_dir: output_dir.clone(), state: Arc::new(Mutex::new(BashExecutionState::default())), }); (meta, tool) }) } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; + use tempfile::TempDir; + use workdir::{LocalWorkdirSession, WorkdirSessionHandle}; + + use super::bash_tool; + use crate::{grep::grep_tool, read::read_tool, tracker::Tracker}; + + fn session_with_output_scope(root: &TempDir, output: &TempDir) -> WorkdirSessionHandle { + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: root.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: output.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + Arc::new(LocalWorkdirSession::new(scope, root.path().to_path_buf())) + } + + #[tokio::test] + async fn long_output_is_spilled_and_available_to_read_and_grep() { + let root = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let session = session_with_output_scope(&root, &output); + let (_, bash) = bash_tool(session.clone(), output.path().to_path_buf())(); + let command = "i=0; while [ $i -lt 2000 ]; do printf 'line-%04d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'"; + let result = bash + .execute( + &serde_json::json!({ "command": command }).to_string(), + Default::default(), + ) + .await + .unwrap(); + let rendered = result.content.expect("bounded Bash output"); + let artifact = std::fs::read_dir(output.path()) + .unwrap() + .next() + .expect("artifact entry") + .unwrap() + .path(); + + assert!(rendered.contains("full output saved to")); + assert!(rendered.contains(&artifact.display().to_string())); + let retained = std::fs::read_to_string(&artifact).unwrap(); + assert!(retained.starts_with("line-0000\n")); + assert!(retained.ends_with("FINAL-NEEDLE\n")); + assert_eq!(retained.lines().count(), 2001); + + let (_, read) = read_tool(session.clone(), Tracker::new())(); + let read_result = read + .execute( + &serde_json::json!({ + "file_path": artifact, + "offset": 2000, + "limit": 1, + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + assert!( + read_result + .content + .expect("Read content") + .contains("FINAL-NEEDLE") + ); + + let (_, grep) = grep_tool(session)(); + let grep_result = grep + .execute( + &serde_json::json!({ + "pattern": "FINAL-NEEDLE", + "path": artifact, + "output_mode": "content", + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + let grep_content = grep_result.content.expect("Grep content"); + assert!( + grep_content.contains("FINAL-NEEDLE"), + "unexpected Grep content: {grep_content:?}" + ); + } + + #[tokio::test] + async fn short_output_does_not_leave_a_spill_artifact() { + let root = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let session = session_with_output_scope(&root, &output); + let (_, bash) = bash_tool(session, output.path().to_path_buf())(); + + let result = bash + .execute( + &serde_json::json!({ "command": "printf short" }).to_string(), + Default::default(), + ) + .await + .unwrap(); + + assert_eq!(result.content.as_deref(), Some("short")); + assert_eq!(std::fs::read_dir(output.path()).unwrap().count(), 0); + } +} diff --git a/crates/tools/src/grep.rs b/crates/tools/src/grep.rs index 785470d4..4774e6ff 100644 --- a/crates/tools/src/grep.rs +++ b/crates/tools/src/grep.rs @@ -22,7 +22,7 @@ enum OutputMode { #[derive(Debug, Deserialize, JsonSchema)] struct GrepParams { pattern: String, - /// Logical Workdir-relative file or directory to search. Defaults to the Workdir root. + /// Workdir-relative path, or an absolute path covered by readable scope. Defaults to the Workdir root. #[serde(default)] path: Option, #[serde(default)] @@ -61,7 +61,7 @@ impl Tool for GrepTool { let params: GrepParams = serde_json::from_str(input_json) .map_err(|error| ToolError::InvalidArgument(format!("invalid Grep input: {error}")))?; let path = match params.path { - Some(path) => WorkdirPath::new(&path).map_err(ToolsError::from)?, + Some(path) => WorkdirPath::new_scoped(&path).map_err(ToolsError::from)?, None => WorkdirPath::root(), }; let mode = match params.output_mode.unwrap_or_default() { diff --git a/crates/tools/src/read.rs b/crates/tools/src/read.rs index e743c56f..668f4a90 100644 --- a/crates/tools/src/read.rs +++ b/crates/tools/src/read.rs @@ -13,14 +13,14 @@ use workdir::{ReadRequest, WorkdirPath, WorkdirSessionHandle}; const DESCRIPTION: &str = "Read a text file from the local filesystem. \ Supports offset/limit for large files. Returns line-numbered output (1-based). \ Directories cannot be read. The file must be read before Write or Edit can \ -modify it. Paths are relative to the bound Workdir."; +modify it. Paths are Workdir-relative unless an absolute path is explicitly readable."; const DEFAULT_LIMIT: usize = 2000; const PROVIDER_BYTE_LIMIT: usize = 256 * 1024; #[derive(Debug, Deserialize, schemars::JsonSchema)] pub(crate) struct ReadParams { - /// Logical path relative to the bound Workdir root. + /// Workdir-relative path, or an absolute path covered by readable scope. pub file_path: String, /// 0-based line offset from the start. Defaults to 0. #[serde(default)] @@ -47,7 +47,7 @@ impl Tool for ReadTool { let offset = params.offset.unwrap_or(0); let limit = params.limit.unwrap_or(DEFAULT_LIMIT).max(1); - let path = WorkdirPath::new(¶ms.file_path).map_err(ToolsError::from)?; + let path = WorkdirPath::new_scoped(¶ms.file_path).map_err(ToolsError::from)?; tracing::debug!(path = %path, offset, limit, "Read"); let result = self diff --git a/crates/tools/tests/edge_cases.rs b/crates/tools/tests/edge_cases.rs index 6e139d07..e00549aa 100644 --- a/crates/tools/tests/edge_cases.rs +++ b/crates/tools/tests/edge_cases.rs @@ -224,20 +224,23 @@ async fn very_long_single_line() { } #[tokio::test] -async fn absolute_path_is_rejected() { - let (dir, _spill, reg) = setup(); +async fn absolute_path_requires_matching_read_scope() { + let (_dir, _spill, reg) = setup(); + let outside = tempfile::tempdir().unwrap(); + let outside_file = outside.path().join("outside.txt"); + std::fs::write(&outside_file, "secret").unwrap(); let read = reg.get("Read"); let err = read .execute( - &json!({ "file_path": dir.path().join("outside.txt") }).to_string(), + &json!({ "file_path": outside_file }).to_string(), Default::default(), ) .await .unwrap_err(); let msg = format!("{err}"); assert!( - msg.contains("invalid logical filesystem path"), - "absolute path was not rejected as invalid: {msg}" + msg.contains("outside allowed scope"), + "absolute path escaped readable scope: {msg}" ); } diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index 98e2dab6..3888c659 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -394,14 +394,21 @@ async fn bash_inherits_workdir_cwd() { } #[tokio::test] -async fn bash_provider_output_does_not_expose_internal_paths() { +async fn bash_provider_output_exposes_readable_retained_path() { let (_dir, spill, reg) = setup(); let bash = reg.get("Bash"); let out = call(&bash, json!({ "command": "printf 'x%.0s' {1..20480}" })).await; let body = out.content.unwrap(); assert!(body.contains("bounded WorkdirSession command output")); - assert!(!body.contains(spill.path().to_str().unwrap())); - assert_eq!(std::fs::read_dir(spill.path()).unwrap().count(), 0); + assert!(body.contains("full output saved to")); + assert!(body.contains(spill.path().to_str().unwrap())); + let artifact = std::fs::read_dir(spill.path()) + .unwrap() + .next() + .expect("retained output") + .unwrap() + .path(); + assert_eq!(std::fs::metadata(artifact).unwrap().len(), 20_480); } #[tokio::test] diff --git a/crates/workdir/src/delegation.rs b/crates/workdir/src/delegation.rs index 031ac9b5..17598dab 100644 --- a/crates/workdir/src/delegation.rs +++ b/crates/workdir/src/delegation.rs @@ -743,6 +743,7 @@ mod tests { command: command.into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some(tool_call_id.into()), }) .await @@ -770,6 +771,7 @@ mod tests { command: "printf ready; sleep 0.2; printf done".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-delegated".into()), }) .await @@ -858,6 +860,7 @@ mod tests { command: "printf denied".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("read-only-command".into()), }) .await, @@ -993,6 +996,7 @@ mod tests { command: "printf revoked".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("revoked-child-command".into()), }) .await, @@ -1171,6 +1175,7 @@ mod tests { command: "printf closed".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("closed-parent-command".into()), }) .await, diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index a897076f..8f9556dd 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -10,9 +10,7 @@ use std::collections::{BTreeMap, HashMap}; use std::fmt::Debug; -#[cfg(test)] -use std::io::Write as _; -use std::io::{Read as _, Seek as _, SeekFrom}; +use std::io::{Read as _, Seek as _, SeekFrom, Write as _}; use std::path::{Path, PathBuf}; use std::process::Stdio; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -691,6 +689,11 @@ impl WorkdirSession for LocalWorkdirSession { async fn start_command(&self, request: CommandRequest) -> Result { self.ensure_capability(WorkdirSessionCapability::Command)?; self.ensure_open()?; + if let Some(spill_dir) = request.spill_dir.as_deref() + && !self.inner.scope.snapshot().is_readable(spill_dir) + { + return Err(WorkdirError::OutOfScope(spill_dir.to_path_buf())); + } let id = self.inner.next_command_id.fetch_add(1, Ordering::Relaxed); let handle = CommandHandle(format!("command-{id}")); let cwd = self.inner.cwd.clone(); @@ -776,6 +779,7 @@ impl WorkdirSession for LocalWorkdirSession { content: String::new(), next_cursor: None, truncated: false, + output_path: None, }); } drop(commands); @@ -792,6 +796,7 @@ impl WorkdirSession for LocalWorkdirSession { content: String::new(), next_cursor: None, truncated: false, + output_path: None, }); } break commands @@ -901,6 +906,7 @@ fn command_output_page(output: &CommandOutput, cursor: usize, limit: usize) -> C content, next_cursor: (end < total_chars).then_some(end), truncated: output.truncated || end < total_chars, + output_path: output.output_path.clone(), } } @@ -1059,6 +1065,22 @@ async fn run_command( let (content, truncated) = read_command_output_files(&stdout_path, &stderr_path, request.output_limit.max(1))?; + let output_path = match (truncated, request.spill_dir) { + (true, Some(spill_dir)) => { + let stdout_path = stdout_path.to_path_buf(); + let stderr_path = stderr_path.to_path_buf(); + Some( + tokio::task::spawn_blocking(move || { + persist_command_output(&stdout_path, &stderr_path, &spill_dir) + }) + .await + .map_err(|error| { + WorkdirError::Unavailable(format!("Bash output spill task failed: {error}")) + })??, + ) + } + _ => None, + }; Ok(CommandOutput { status, exit_code, @@ -1066,6 +1088,7 @@ async fn run_command( content, next_cursor: None, truncated, + output_path, }) } @@ -1154,6 +1177,59 @@ fn stable_utf8_prefix_len(bytes: &[u8]) -> usize { inspected } +fn persist_command_output( + stdout_path: &Path, + stderr_path: &Path, + spill_dir: &Path, +) -> Result { + std::fs::create_dir_all(spill_dir).map_err(|error| WorkdirError::io(spill_dir, error))?; + let mut artifact = tempfile::Builder::new() + .prefix("bash-") + .suffix(".log") + .tempfile_in(spill_dir) + .map_err(|error| WorkdirError::io(spill_dir, error))?; + let artifact_path = artifact.path().to_path_buf(); + + let mut stdout = + std::fs::File::open(stdout_path).map_err(|error| WorkdirError::io(stdout_path, error))?; + let stdout_len = stdout + .metadata() + .map_err(|error| WorkdirError::io(stdout_path, error))? + .len(); + std::io::copy(&mut stdout, &mut artifact) + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + + let mut stderr = + std::fs::File::open(stderr_path).map_err(|error| WorkdirError::io(stderr_path, error))?; + let stderr_len = stderr + .metadata() + .map_err(|error| WorkdirError::io(stderr_path, error))? + .len(); + if stdout_len > 0 && stderr_len > 0 { + stdout + .seek(SeekFrom::End(-1)) + .map_err(|error| WorkdirError::io(stdout_path, error))?; + let mut last = [0_u8; 1]; + stdout + .read_exact(&mut last) + .map_err(|error| WorkdirError::io(stdout_path, error))?; + if last[0] != b'\n' { + artifact + .write_all(b"\n") + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + } + } + std::io::copy(&mut stderr, &mut artifact) + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + artifact + .flush() + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + artifact + .keep() + .map(|(_, path)| path) + .map_err(|error| WorkdirError::io(&artifact_path, error.error)) +} + fn read_command_output_files( stdout_path: &Path, stderr_path: &Path, @@ -1440,6 +1516,7 @@ mod tests { command: "sleep 30".to_owned(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -1966,6 +2043,7 @@ mod tests { command: "pwd && printf provider-command".into(), timeout_secs: 5, output_limit: 4096, + spill_dir: None, tool_call_id: None, }, ) @@ -1991,6 +2069,151 @@ mod tests { ); } + #[tokio::test] + async fn explicitly_scoped_absolute_artifact_can_be_read_and_grepped() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let artifact = spill.path().join("bash-output.log"); + std::fs::write(&artifact, "first\nFINAL-NEEDLE\nlast\n").unwrap(); + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: spill.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + let workdir = LocalWorkdirSession::new(scope, dir.path().to_path_buf()); + let artifact_path = WorkdirPath::new_scoped(artifact.to_string_lossy()).unwrap(); + + let read = WorkdirSession::read( + &workdir, + ReadRequest { + path: artifact_path.clone(), + offset: 1, + limit: 1, + max_bytes: 1024, + }, + ) + .await + .unwrap(); + assert_eq!(String::from_utf8(read.bytes).unwrap(), "FINAL-NEEDLE\n"); + + let grep = WorkdirSession::grep( + &workdir, + GrepRequest { + pattern: "FINAL-NEEDLE".into(), + path: artifact_path, + glob: None, + file_type: None, + case_insensitive: false, + before_context: 0, + after_context: 0, + multiline: false, + output_mode: crate::GrepOutputMode::Content, + limit: 10, + offset: 0, + }, + ) + .await + .unwrap(); + assert_eq!(grep.match_count, 1); + assert!(grep.output.contains("FINAL-NEEDLE")); + } + + #[tokio::test] + async fn command_rejects_spill_directory_without_read_scope() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let workdir = make_fs(&dir); + + let error = WorkdirSession::start_command( + &workdir, + CommandRequest { + command: "printf hidden".into(), + timeout_secs: 5, + output_limit: 1, + spill_dir: Some(spill.path().to_path_buf()), + tool_call_id: None, + }, + ) + .await + .unwrap_err(); + + assert!(matches!(error, WorkdirError::OutOfScope(path) if path == spill.path())); + } + + #[tokio::test] + async fn truncated_command_output_is_retained_in_the_requested_spill_directory() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: spill.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + let workdir = LocalWorkdirSession::new(scope, dir.path().to_path_buf()); + let handle = WorkdirSession::start_command( + &workdir, + CommandRequest { + command: "i=0; while [ $i -lt 200 ]; do printf 'line-%03d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'".into(), + timeout_secs: 5, + output_limit: 64, + spill_dir: Some(spill.path().to_path_buf()), + tool_call_id: None, + }, + ) + .await + .unwrap(); + let output = WorkdirSession::command_output( + &workdir, + CommandOutputRequest { + handle, + cursor: 0, + limit: 4096, + wait: true, + }, + ) + .await + .unwrap(); + + assert!(output.truncated); + let output_path = output.output_path.expect("retained output path"); + assert_eq!(output_path.parent(), Some(spill.path())); + let retained = std::fs::read_to_string(&output_path).unwrap(); + assert!(retained.starts_with("line-000\n")); + assert!(retained.ends_with("FINAL-NEEDLE\n")); + assert_eq!(retained.lines().count(), 201); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + assert_eq!( + std::fs::metadata(output_path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + } + #[tokio::test] async fn completed_command_output_can_be_read_in_bounded_unicode_pages() { let dir = TempDir::new().unwrap(); @@ -2001,6 +2224,7 @@ mod tests { command: "printf 'aéz'".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -2120,6 +2344,7 @@ mod tests { content: "done".into(), next_cursor: None, truncated: false, + output_path: None, }) }); workdir.inner.commands.lock().await.insert( @@ -2224,6 +2449,7 @@ mod tests { command: "printf ready; printf warning >&2; sleep 0.2; printf done".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-7".into()), }, ) @@ -2327,6 +2553,7 @@ mod tests { command: "sleep 30".into(), timeout_secs: 1, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -2396,6 +2623,7 @@ mod tests { command: "sleep 30".into(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) diff --git a/crates/workdir/src/operation.rs b/crates/workdir/src/operation.rs index 47527ad8..67858685 100644 --- a/crates/workdir/src/operation.rs +++ b/crates/workdir/src/operation.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -9,6 +11,9 @@ pub struct CommandRequest { pub command: String, pub timeout_secs: u64, pub output_limit: usize, + /// Provider-local directory where complete output is retained when the + /// inline result exceeds `output_limit`. + pub spill_dir: Option, /// Optional caller-owned correlation id. Bash supplies its tool-call id so /// user-facing command telemetry can update the corresponding Console row /// without exposing provider/session handles. @@ -96,4 +101,6 @@ pub struct CommandOutput { pub content: String, pub next_cursor: Option, pub truncated: bool, + /// Complete output retained by the provider when `truncated` is true. + pub output_path: Option, } diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index 610ea770..a8486523 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -703,6 +703,7 @@ async fn run_workdir_session_operation( content: String::new(), next_cursor: Some(cursor), truncated: false, + output_path: None, }, }; WorkdirSessionOperationResult::CommandOutput(output) diff --git a/crates/worker-runtime/src/worker_backend.rs b/crates/worker-runtime/src/worker_backend.rs index a35114e5..5f587c1b 100644 --- a/crates/worker-runtime/src/worker_backend.rs +++ b/crates/worker-runtime/src/worker_backend.rs @@ -60,6 +60,7 @@ use worker::{ Worker, WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout, WorkerControllerTransport, WorkerError, WorkerFilesystemAuthority, WorkerHandle, WorkerSharedState, WorkerWorkspaceContext, WorkspaceClient, WorkspaceId, + bash_output_dir_for_worker_id, }; const DEFAULT_BACKEND_ID: &str = "worker-crate"; @@ -886,6 +887,7 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { let run_dir = worker_aggregate_dir .join("runs") .join(request.run_generation.to_string()); + let bash_output_dir = bash_output_dir_for_worker_id(&request.worker_ref.worker_id); let mut prepared = WorkerBootstrap::new( manifest, store, @@ -894,6 +896,7 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { filesystem_authority, WorkerBootstrapLayout::RuntimeManagedRun { run_dir: run_dir.clone(), + bash_output_dir, }, self.controller_transport, ) @@ -1131,10 +1134,12 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { let run_dir = worker_aggregate_dir .join("runs") .join(request.run_generation.to_string()); + let bash_output_dir = bash_output_dir_for_worker_id(&request.worker_ref.worker_id); let started = PreparedWorker::new( worker, WorkerBootstrapLayout::RuntimeManagedRun { run_dir: run_dir.clone(), + bash_output_dir, }, self.controller_transport, ) @@ -2552,10 +2557,14 @@ mod tests { ) .await .map_err(|err| err.to_string())?; - let (handle, shutdown_rx) = - WorkerController::spawn_runtime_managed(worker, &self.runtime_base) - .await - .map_err(|err| err.to_string())?; + let bash_output_dir = self.runtime_base.join("bash-output"); + let (handle, shutdown_rx) = WorkerController::spawn_runtime_managed( + worker, + &self.runtime_base, + &bash_output_dir, + ) + .await + .map_err(|err| err.to_string())?; Ok(RuntimeWorkerController { handle, shutdown: Arc::new(tokio::sync::Mutex::new(Some(shutdown_rx))), diff --git a/crates/worker/examples/worker_protocol.rs b/crates/worker/examples/worker_protocol.rs index fc54a1d0..1e2c618f 100644 --- a/crates/worker/examples/worker_protocol.rs +++ b/crates/worker/examples/worker_protocol.rs @@ -47,7 +47,9 @@ async fn main() -> Result<(), Box> { let worker = worker::Worker::from_manifest_toml(&toml, store).await?; let runtime_tmp = tempfile::tempdir()?; - let (handle, _shutdown_rx) = WorkerController::spawn(worker, runtime_tmp.path()).await?; + let bash_output_dir = runtime_tmp.path().join("bash-output"); + let (handle, _shutdown_rx) = + WorkerController::spawn(worker, runtime_tmp.path(), &bash_output_dir).await?; // Check initial status via shared state println!("[shared_state] {}", handle.shared_state.status_json()); diff --git a/crates/worker/src/bootstrap.rs b/crates/worker/src/bootstrap.rs index c5ec07c5..ad292df1 100644 --- a/crates/worker/src/bootstrap.rs +++ b/crates/worker/src/bootstrap.rs @@ -17,9 +17,28 @@ use manifest::WorkerManifest; #[derive(Debug, Clone)] pub enum WorkerBootstrapLayout { /// A direct Worker rooted below the supplied runtime base directory. - Direct { runtime_base: PathBuf }, + Direct { + runtime_base: PathBuf, + bash_output_dir: PathBuf, + }, /// A runtime-managed Worker with an exact persisted run directory. - RuntimeManagedRun { run_dir: PathBuf }, + RuntimeManagedRun { + run_dir: PathBuf, + bash_output_dir: PathBuf, + }, +} + +/// Return the temporary Bash spill directory owned by a stable Worker identity. +/// +/// The directory deliberately lives outside session/run-generation storage so a +/// restarted controller for the same Worker keeps the same readable artifact +/// boundary. +pub fn bash_output_dir_for_worker_id(worker_id: impl std::fmt::Display) -> PathBuf { + std::env::temp_dir() + .join("yoi") + .join("workers") + .join(worker_id.to_string()) + .join("bash-output") } /// Construction and controller inputs that are stable for one Worker launch. @@ -204,12 +223,29 @@ where { let cleanup_session = worker.workdir_session().cloned(); let controller = match layout { - WorkerBootstrapLayout::Direct { runtime_base } => { - WorkerController::spawn_with_transport(worker, &runtime_base, transport).await + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + } => { + WorkerController::spawn_with_transport( + worker, + &runtime_base, + &bash_output_dir, + transport, + ) + .await } - WorkerBootstrapLayout::RuntimeManagedRun { run_dir } => { - WorkerController::spawn_runtime_managed_run_with_transport(worker, &run_dir, transport) - .await + WorkerBootstrapLayout::RuntimeManagedRun { + run_dir, + bash_output_dir, + } => { + WorkerController::spawn_runtime_managed_run_with_transport( + worker, + &run_dir, + &bash_output_dir, + transport, + ) + .await } }; @@ -227,3 +263,22 @@ where } } } + +#[cfg(test)] +mod tests { + use super::bash_output_dir_for_worker_id; + + #[test] + fn bash_output_directory_is_stable_per_worker_below_system_temp() { + let path = bash_output_dir_for_worker_id("019c1234-worker"); + + assert_eq!( + path, + std::env::temp_dir() + .join("yoi") + .join("workers") + .join("019c1234-worker") + .join("bash-output") + ); + } +} diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 5af8355b..7d35739e 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -222,6 +222,7 @@ impl WorkerController { pub async fn spawn( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, @@ -230,6 +231,7 @@ impl WorkerController { Self::spawn_inner( worker, runtime_base, + bash_output_dir, false, None, WorkerControllerTransport::UnixSocket, @@ -242,24 +244,9 @@ impl WorkerController { pub async fn spawn_with_transport( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, transport: WorkerControllerTransport, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> - where - C: LlmClient + Clone + 'static, - St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, - { - Self::spawn_inner(worker, runtime_base, false, None, transport).await - } - - /// Spawn a Worker owned by `worker-runtime`. - /// - /// The controller still uses an ephemeral directory for Unix sockets and - /// tool spill artifacts, but does not write legacy pid/status/manifest - /// liveness projections. - pub async fn spawn_runtime_managed( - worker: Worker, - runtime_base: &Path, - ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, @@ -267,6 +254,33 @@ impl WorkerController { Self::spawn_inner( worker, runtime_base, + bash_output_dir, + false, + None, + transport, + ) + .await + } + + /// Spawn a Worker owned by `worker-runtime`. + /// + /// The controller uses an ephemeral directory for Unix sockets while tool + /// spill artifacts use the separately supplied Worker-owned temporary path. + /// Runtime-managed Workers do not write legacy pid/status/manifest liveness + /// projections. + pub async fn spawn_runtime_managed( + worker: Worker, + runtime_base: &Path, + bash_output_dir: &Path, + ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> + where + C: LlmClient + Clone + 'static, + St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, + { + Self::spawn_inner( + worker, + runtime_base, + bash_output_dir, true, None, WorkerControllerTransport::UnixSocket, @@ -278,6 +292,7 @@ impl WorkerController { pub async fn spawn_runtime_managed_run( worker: Worker, run_dir: &Path, + bash_output_dir: &Path, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, @@ -286,6 +301,7 @@ impl WorkerController { Self::spawn_runtime_managed_run_with_transport( worker, run_dir, + bash_output_dir, WorkerControllerTransport::UnixSocket, ) .await @@ -296,6 +312,7 @@ impl WorkerController { pub async fn spawn_runtime_managed_run_with_transport( worker: Worker, run_dir: &Path, + bash_output_dir: &Path, transport: WorkerControllerTransport, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where @@ -305,12 +322,21 @@ impl WorkerController { let parent = run_dir .parent() .ok_or_else(|| std::io::Error::other("run path has no parent"))?; - Self::spawn_inner(worker, parent, true, Some(run_dir), transport).await + Self::spawn_inner( + worker, + parent, + bash_output_dir, + true, + Some(run_dir), + transport, + ) + .await } async fn spawn_inner( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, runtime_managed: bool, runtime_run: Option<&Path>, transport: WorkerControllerTransport, @@ -323,6 +349,7 @@ impl WorkerController { let result = Self::spawn_initialized( worker, runtime_base, + bash_output_dir, runtime_managed, runtime_run, transport, @@ -340,6 +367,7 @@ impl WorkerController { async fn spawn_initialized( mut worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, runtime_managed: bool, runtime_run: Option<&Path>, transport: WorkerControllerTransport, @@ -397,11 +425,11 @@ impl WorkerController { worker.attach_internal_worker_registry(spawned_registry.clone()); worker.attach_working_event_tx(working_event_tx.clone()); - // Bash spills long outputs to a per-worker subdir under the runtime - // dir. Push a recursive `allow(Read)` for that path into the - // Worker's runtime scope so the agent can `Read` saved files - // without polluting the workspace. - let bash_output_dir = runtime_dir.path().join("bash-output"); + // Bash spill artifacts are owned by the stable Worker identity rather + // than a controller session/run generation. Push a recursive + // `allow(Read)` for the exact tool output path into the Worker's shared + // runtime scope so the Workdir session and system prompt stay aligned. + let bash_output_dir = bash_output_dir.to_path_buf(); std::fs::create_dir_all(&bash_output_dir).map_err(|e| { std::io::Error::other(format!( "create bash output dir {}: {e}", @@ -880,7 +908,7 @@ where .register_tools(tools::core_builtin_tools( workdir.clone(), tracker.clone(), - bash_output_dir, + bash_output_dir.clone(), )); if feature_config.image.enabled && model_supports_image_attachments(&spawner_manifest.model) { @@ -1103,6 +1131,7 @@ where spawner_workspace_context, parent_notifications, runtime_base.clone(), + bash_output_dir.clone(), spawner_workspace_root, source_workdir_session, spawned_registry.clone(), diff --git a/crates/worker/src/entrypoint.rs b/crates/worker/src/entrypoint.rs index 2b258df6..562e7b67 100644 --- a/crates/worker/src/entrypoint.rs +++ b/crates/worker/src/entrypoint.rs @@ -634,10 +634,12 @@ async fn run_cli_inner(cli: Cli) -> ExitCode { return ExitCode::FAILURE; } }; + let bash_output_dir = crate::bash_output_dir_for_worker_id(&worker_name); let started = match start_worker_controller( worker, WorkerBootstrapLayout::Direct { runtime_base: runtime_base.clone(), + bash_output_dir, }, WorkerControllerTransport::UnixSocket, ) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 72212e87..166bdeb3 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -27,7 +27,7 @@ mod worker; pub use bootstrap::{ BootstrappedWorker, PreparedWorker, WorkerBootstrap, WorkerBootstrapError, - WorkerBootstrapLayout, start_worker_controller, + WorkerBootstrapLayout, bash_output_dir_for_worker_id, start_worker_controller, }; pub use compact::token_counter::{EstimateSource, SplitPoint, TokenEstimate}; pub use controller::{ShutdownReceiver, WorkerController, WorkerControllerTransport, WorkerHandle}; diff --git a/crates/worker/src/spawn/tool.rs b/crates/worker/src/spawn/tool.rs index 603ed190..8c3ee231 100644 --- a/crates/worker/src/spawn/tool.rs +++ b/crates/worker/src/spawn/tool.rs @@ -22,7 +22,7 @@ use manifest::{ use serde::Deserialize; use tokio::sync::mpsc; use workdir::{ - WorkdirDelegationPermission, WorkdirDelegationRequest, WorkdirDelegationRule, + WorkdirDelegationPermission, WorkdirDelegationRequest, WorkdirDelegationRule, WorkdirPath, WorkdirSessionHandle, }; @@ -258,9 +258,10 @@ pub struct SubWorkerSpawnTool { spawner_name: String, workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, - /// Runtime-owned root used only for bounded Internal Worker tool artifacts such as Bash spill - /// output. It is not an Internal Worker identity or catalog location. + /// Runtime-owned root used for Internal Worker controller state. runtime_base: PathBuf, + /// Parent Worker-owned temporary root used for bounded Bash spill output. + bash_output_dir: PathBuf, /// Inherited runtime workspace root for Profile/project/Ticket/workflow/ /// memory context. SubWorkerSpawn `cwd` must not affect this value. workspace_root: PathBuf, @@ -292,6 +293,7 @@ impl SubWorkerSpawnTool { workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -304,6 +306,7 @@ impl SubWorkerSpawnTool { workspace_context, parent_notifications, runtime_base, + bash_output_dir, workspace_root, source_workdir_session, registry, @@ -367,7 +370,22 @@ impl Tool for SubWorkerSpawnTool { .reserve_internal_name(input.name.clone()) .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; - let workdir_rules = parse_workdir_scope(&input.scope)?; + let mut workdir_rules = parse_workdir_scope(&input.scope)?; + let child_bash_output_dir = self.bash_output_dir.join("sub-workers").join(&input.name); + tokio::fs::create_dir_all(&child_bash_output_dir) + .await + .map_err(|error| { + ToolError::ExecutionFailed(format!( + "create Internal Worker Bash output directory {}: {error}", + child_bash_output_dir.display() + )) + })?; + workdir_rules.push(WorkdirDelegationRule { + target: WorkdirPath::new_scoped(child_bash_output_dir.to_string_lossy()) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?, + permission: WorkdirDelegationPermission::Read, + recursive: true, + }); let source_workdir_session = require_active_workdir_session(self.source_workdir_session.as_ref())?; let delegation_request = workdir_delegation_request(input.cwd.as_deref(), workdir_rules)?; @@ -464,14 +482,22 @@ impl Tool for SubWorkerSpawnTool { .await .map_err(|error| ToolError::ExecutionFailed(format!("build Internal Worker: {error}")))?; child.bind_workdir_session(Some(workdir_delegation.scoped_session.clone())); + child + .add_scope_rules([ScopeRule { + target: child_bash_output_dir.clone(), + permission: manifest::Permission::Read, + recursive: true, + }]) + .map_err(|error| { + ToolError::ExecutionFailed(format!( + "grant Internal Worker Bash output scope: {error}" + )) + })?; let child_scope = child.scope().clone(); let child_registry = SpawnedWorkerRegistry::new_internal(input.name.clone(), child_scope); register_worker_tools( &mut child, - self.runtime_base - .join("internal-workers") - .join(&input.name) - .join("bash-output"), + child_bash_output_dir, self.runtime_base.clone(), child_registry.clone(), None, @@ -883,6 +909,7 @@ pub(crate) fn sub_worker_spawn_tool( workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -894,6 +921,7 @@ pub(crate) fn sub_worker_spawn_tool( workspace_context, parent_notifications, runtime_base, + bash_output_dir, workspace_root, source_workdir_session, registry, @@ -907,6 +935,7 @@ fn sub_worker_spawn_tool_impl( workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -938,6 +967,7 @@ fn sub_worker_spawn_tool_impl( workspace_context.clone(), parent_notifications.clone(), runtime_base.clone(), + bash_output_dir.clone(), workspace_root.clone(), source_workdir_session.clone(), registry.clone(), @@ -1082,12 +1112,17 @@ extract_threshold = 4000 async fn reviewer_profile_write_scope_exposes_command_tools_and_notifies_parent_controller() { let runtime = TempDir::new().unwrap(); let workspace_root = runtime.path().join("project"); + let bash_output_dir = runtime.path().join("bash-output"); let available_profiles = write_project_profile_registry( &workspace_root, Some("reviewer"), &[("reviewer", "reviewer.toml", INTERNAL_REVIEWER_PROFILE)], ); let mut manifest = parent_manifest(&workspace_root, None); + manifest + .scope + .allow + .push(abs_rule(&bash_output_dir, Permission::Read)); manifest.delegation_scope = ScopeConfig { allow: vec![abs_rule(&workspace_root, Permission::Write)], deny: Vec::new(), @@ -1118,6 +1153,7 @@ extract_threshold = 4000 workspace_context, ParentNotificationTarget::Controller(parent_method_tx.downgrade()), runtime.path().to_path_buf(), + bash_output_dir.clone(), workspace_root.clone(), Some(source_workdir_session), registry.clone(), @@ -1167,6 +1203,12 @@ extract_threshold = 4000 .await .expect("spawn project reviewer as Internal Worker"); assert!(output.summary.contains("internal worker `reviewer-child`")); + assert!( + bash_output_dir + .join("sub-workers") + .join("reviewer-child") + .is_dir() + ); assert!(spawner_scope.snapshot().is_writable(&workspace_root)); let record = registry .get_internal("reviewer-child") diff --git a/crates/worker/tests/compact_events_test.rs b/crates/worker/tests/compact_events_test.rs index 708fd60a..afef80bb 100644 --- a/crates/worker/tests/compact_events_test.rs +++ b/crates/worker/tests/compact_events_test.rs @@ -861,7 +861,8 @@ async fn controller_compact_method_emits_start_and_done() { ]); let worker = make_worker_with_manifest(POST_RUN_MANIFEST_TOML, client).await; let runtime_tmp = tempfile::tempdir().unwrap(); - let (handle, _shutdown) = WorkerController::spawn(worker, runtime_tmp.path()) + let bash_output_dir = runtime_tmp.path().join("bash-output"); + let (handle, _shutdown) = WorkerController::spawn(worker, runtime_tmp.path(), &bash_output_dir) .await .unwrap(); let mut rx = handle.subscribe(); diff --git a/crates/worker/tests/controller_test.rs b/crates/worker/tests/controller_test.rs index 968f8292..7b13e752 100644 --- a/crates/worker/tests/controller_test.rs +++ b/crates/worker/tests/controller_test.rs @@ -276,12 +276,38 @@ async fn spawn_controller(worker: Worker) -> WorkerHandle let tmp = tempfile::tempdir().unwrap(); let runtime_base = tmp.path().to_owned(); std::mem::forget(tmp); - let (handle, _shutdown_rx) = WorkerController::spawn(worker, &runtime_base) + let bash_output_dir = runtime_base.join("bash-output"); + let (handle, _shutdown_rx) = WorkerController::spawn(worker, &runtime_base, &bash_output_dir) .await .unwrap(); handle } +#[tokio::test] +async fn controller_grants_read_scope_for_exact_bash_output_directory() { + let worker = make_worker(MockClient::new(simple_text_events())).await; + let shared_scope = worker.scope().clone(); + let runtime_base = tempfile::tempdir().unwrap(); + let worker_tmp = tempfile::tempdir().unwrap(); + let bash_output_dir = worker_tmp.path().join("worker-1").join("bash-output"); + + let (handle, shutdown_rx) = + WorkerController::spawn(worker, runtime_base.path(), &bash_output_dir) + .await + .unwrap(); + + assert!(bash_output_dir.is_dir()); + assert!(shared_scope.snapshot().allow_rules().iter().any(|rule| { + rule.target == bash_output_dir + && rule.permission == manifest::Permission::Read + && rule.recursive + })); + assert!(!handle.runtime_dir.path().join("bash-output").exists()); + + handle.send(Method::Shutdown).await.unwrap(); + shutdown_rx.await.unwrap(); +} + #[tokio::test] async fn shutdown_closes_bound_workdir_session() { let (mut worker, pwd) = make_worker_with_pwd(MockClient::new(simple_text_events())).await; @@ -297,6 +323,7 @@ async fn shutdown_closes_bound_workdir_session() { command: "sleep 30".to_owned(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }) .await @@ -304,9 +331,11 @@ async fn shutdown_closes_bound_workdir_session() { worker.bind_workdir_session(Some(Arc::clone(&session))); let runtime_base = tempfile::tempdir().unwrap(); - let (handle, shutdown_rx) = WorkerController::spawn(worker, runtime_base.path()) - .await - .unwrap(); + let bash_output_dir = runtime_base.path().join("bash-output"); + let (handle, shutdown_rx) = + WorkerController::spawn(worker, runtime_base.path(), &bash_output_dir) + .await + .unwrap(); handle.send(Method::Shutdown).await.unwrap(); tokio::time::timeout(std::time::Duration::from_secs(5), shutdown_rx) .await @@ -338,6 +367,7 @@ async fn controller_projects_workdir_command_events_and_snapshot_state() { command: "printf ready; sleep 0.3; printf done".to_owned(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-command-1".into()), }) .await @@ -445,6 +475,7 @@ async fn controller_refreshes_command_snapshot_after_high_output_provider_lag() .to_owned(), timeout_secs: 10, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-high-output".into()), }) .await @@ -508,8 +539,9 @@ async fn controller_startup_failure_closes_bound_workdir_session() { let invalid_runtime_base = runtime_base.path().join("not-a-directory"); std::fs::write(&invalid_runtime_base, "file").unwrap(); + let bash_output_dir = runtime_base.path().join("bash-output"); assert!( - WorkerController::spawn(worker, &invalid_runtime_base) + WorkerController::spawn(worker, &invalid_runtime_base, &bash_output_dir) .await .is_err() ); @@ -519,6 +551,7 @@ async fn controller_startup_failure_closes_bound_workdir_session() { command: "printf unreachable".to_owned(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: None, }) .await, @@ -863,7 +896,8 @@ permission = "write" let client = MockClient::new(simple_text_events()); let worker = make_worker_with_pwd_and_manifest(client, manifest).await.0; let tmp = tempfile::tempdir().unwrap(); - let result = WorkerController::spawn(worker, tmp.path()).await; + let bash_output_dir = tmp.path().join("bash-output"); + let result = WorkerController::spawn(worker, tmp.path(), &bash_output_dir).await; assert!( result.is_ok(), "feature exposure must not imply delegation authority" diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 615073d0..2ec12ea0 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -15430,6 +15430,7 @@ mod tests { command: "printf ready; sleep 30".to_string(), timeout_secs: 60, output_limit: 4096, + spill_dir: None, tool_call_id: Some("tool-call-command-session".to_string()), }) .await From bb8bb6d0995e2056df267483f277a909381e6e97 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 18:43:46 +0900 Subject: [PATCH 7/7] feat: restore interactive standalone profile selection --- crates/tui/src/lib.rs | 27 +- crates/tui/src/standalone_spawn.rs | 494 +++++++++++++++++++++++++++++ 2 files changed, 510 insertions(+), 11 deletions(-) create mode 100644 crates/tui/src/standalone_spawn.rs diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 71eacd14..699da9a7 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -16,6 +16,7 @@ mod markdown; mod scroll; pub mod setup_model; mod standalone_picker; +mod standalone_spawn; mod task; mod text_selection; mod tool; @@ -136,17 +137,21 @@ pub async fn launch(options: LaunchOptions) -> ExitCode { LaunchMode::Spawn { worker_name, profile, - } => match target.spawn_worker() { - Ok(spawn) => { - console::run_standalone( - workspace_root.clone(), - spawn.state_dir, - worker_name, - profile, - ) - .await - } - Err(e) => Err(Box::new(e) as Box), + } => match standalone_spawn::select(&workspace_root, worker_name, profile) { + Ok(Some(selection)) => match target.spawn_worker() { + Ok(spawn) => { + console::run_standalone( + workspace_root.clone(), + spawn.state_dir, + Some(selection.worker_name), + Some(selection.profile), + ) + .await + } + Err(error) => Err(Box::new(error) as Box), + }, + Ok(None) => Ok(()), + Err(error) => Err(Box::new(error) as Box), }, LaunchMode::StandaloneResume { include_all } => { match standalone_picker::pick(target.as_ref(), include_all) { diff --git a/crates/tui/src/standalone_spawn.rs b/crates/tui/src/standalone_spawn.rs new file mode 100644 index 00000000..1880803d --- /dev/null +++ b/crates/tui/src/standalone_spawn.rs @@ -0,0 +1,494 @@ +use std::io::{self, Stdout}; +use std::path::Path; +use std::time::Duration; + +use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; +use manifest::ProfileDiscovery; +use ratatui::Terminal; +use ratatui::backend::CrosstermBackend; +use ratatui::layout::{Constraint, Direction, Layout}; +use ratatui::style::{Color, Modifier, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; +use thiserror::Error; + +const VIEWPORT_HEIGHT: u16 = 6; +const FALLBACK_WORKER_NAME: &str = "worker"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct StandaloneSpawnSelection { + pub worker_name: String, + pub profile: String, +} + +#[derive(Debug, Error)] +pub(crate) enum StandaloneSpawnError { + #[error("profile discovery failed: {0}")] + ProfileDiscovery(#[from] manifest::ProfileError), + #[error("no profiles are available")] + NoProfiles, + #[error("standalone spawn picker terminal error: {0}")] + Terminal(#[from] io::Error), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ProfileChoice { + selector: String, + label: String, + is_default: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum StatusKind { + Info, + Progress, + Error, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SpawnAction { + None, + Submit, + Cancel, +} + +struct SpawnForm { + worker_name: String, + cursor: usize, + profile_choices: Vec, + selected_profile: usize, + status: Option<(String, StatusKind)>, +} + +impl SpawnForm { + fn new( + worker_name: Option, + default_worker_name: String, + profile_choices: Vec, + ) -> Self { + let worker_name = worker_name.unwrap_or(default_worker_name); + let cursor = worker_name.chars().count(); + let selected_profile = profile_choices + .iter() + .position(|choice| choice.is_default) + .unwrap_or(0); + Self { + worker_name, + cursor, + profile_choices, + selected_profile, + status: None, + } + } + + fn selected_profile(&self) -> &ProfileChoice { + &self.profile_choices[self.selected_profile] + } + + fn apply_key(&mut self, key: KeyEvent) -> SpawnAction { + if key.kind == KeyEventKind::Release { + return SpawnAction::None; + } + + if key.modifiers.contains(KeyModifiers::CONTROL) { + match key.code { + KeyCode::Char('c') | KeyCode::Char('u') => return SpawnAction::Cancel, + _ => return SpawnAction::None, + } + } + + self.status = None; + match key.code { + KeyCode::Esc => SpawnAction::Cancel, + KeyCode::Enter => { + if self.worker_name.trim().is_empty() { + self.status = + Some(("worker name cannot be empty".to_owned(), StatusKind::Error)); + SpawnAction::None + } else { + SpawnAction::Submit + } + } + KeyCode::Tab | KeyCode::Down => { + self.selected_profile = (self.selected_profile + 1) % self.profile_choices.len(); + SpawnAction::None + } + KeyCode::BackTab | KeyCode::Up => { + self.selected_profile = if self.selected_profile == 0 { + self.profile_choices.len() - 1 + } else { + self.selected_profile - 1 + }; + SpawnAction::None + } + KeyCode::Left => { + self.cursor = self.cursor.saturating_sub(1); + SpawnAction::None + } + KeyCode::Right => { + self.cursor = (self.cursor + 1).min(self.worker_name.chars().count()); + SpawnAction::None + } + KeyCode::Home => { + self.cursor = 0; + SpawnAction::None + } + KeyCode::End => { + self.cursor = self.worker_name.chars().count(); + SpawnAction::None + } + KeyCode::Backspace => { + if self.cursor > 0 { + let idx = byte_index(&self.worker_name, self.cursor - 1); + self.worker_name.remove(idx); + self.cursor -= 1; + } + SpawnAction::None + } + KeyCode::Delete => { + if self.cursor < self.worker_name.chars().count() { + let idx = byte_index(&self.worker_name, self.cursor); + self.worker_name.remove(idx); + } + SpawnAction::None + } + KeyCode::Char(ch) if is_safe_worker_char(ch) => { + let idx = byte_index(&self.worker_name, self.cursor); + self.worker_name.insert(idx, ch); + self.cursor += 1; + SpawnAction::None + } + _ => SpawnAction::None, + } + } +} + +pub(crate) fn select( + workspace_root: &Path, + worker_name: Option, + profile: Option, +) -> Result, StandaloneSpawnError> { + let default_worker_name = default_worker_name(workspace_root); + if let Some(profile) = profile { + return Ok(Some(StandaloneSpawnSelection { + worker_name: worker_name.unwrap_or(default_worker_name), + profile, + })); + } + + let registry = ProfileDiscovery::user_settings().discover()?; + let choices = profile_choices(®istry); + if choices.is_empty() { + return Err(StandaloneSpawnError::NoProfiles); + } + + let terminal = open_inline_terminal()?; + run_picker( + terminal, + SpawnForm::new(worker_name, default_worker_name, choices), + ) +} + +fn run_picker( + mut terminal: Terminal>, + mut form: SpawnForm, +) -> Result, StandaloneSpawnError> { + loop { + terminal.draw(|frame| draw_form(frame, &form))?; + if !event::poll(Duration::from_millis(100))? { + continue; + } + let Event::Key(key) = event::read()? else { + continue; + }; + + match form.apply_key(key) { + SpawnAction::None => {} + SpawnAction::Cancel => { + form.status = Some(("cancelled".to_owned(), StatusKind::Info)); + terminal.draw(|frame| draw_form(frame, &form))?; + return Ok(None); + } + SpawnAction::Submit => { + let selection = StandaloneSpawnSelection { + worker_name: form.worker_name.trim().to_owned(), + profile: form.selected_profile().selector.clone(), + }; + form.status = Some(("starting worker...".to_owned(), StatusKind::Progress)); + terminal.draw(|frame| draw_form(frame, &form))?; + return Ok(Some(selection)); + } + } + } +} + +fn open_inline_terminal() -> io::Result>> { + let options = ratatui::TerminalOptions { + viewport: ratatui::Viewport::Inline(VIEWPORT_HEIGHT), + }; + Terminal::with_options(CrosstermBackend::new(io::stdout()), options) +} + +fn profile_choices(registry: &manifest::ProfileRegistry) -> Vec { + registry + .entries() + .iter() + .map(|entry| { + let selector = entry.qualified_name(); + let default_marker = if entry.is_default { " (default)" } else { "" }; + let mut label = format!("{selector}{default_marker}"); + if let Some(description) = &entry.description { + label.push_str(" — "); + label.push_str(description); + } + ProfileChoice { + selector, + label, + is_default: entry.is_default, + } + }) + .collect() +} + +fn draw_form(frame: &mut ratatui::Frame<'_>, form: &SpawnForm) { + let chunks = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), + Constraint::Length(1), + Constraint::Length(1), + Constraint::Length(1), + Constraint::Length(1), + Constraint::Min(0), + ]) + .split(frame.area()); + + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(" "), + Span::styled( + "spawn worker", + Style::default().add_modifier(Modifier::BOLD), + ), + ])), + chunks[0], + ); + + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(" "), + Span::styled("name: ", Style::default().fg(Color::DarkGray)), + Span::styled( + &form.worker_name, + Style::default() + .fg(Color::Cyan) + .add_modifier(Modifier::BOLD), + ), + ])), + chunks[1], + ); + + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(" "), + Span::styled("profile: ", Style::default().fg(Color::DarkGray)), + Span::styled( + &form.selected_profile().label, + Style::default().fg(Color::Green), + ), + Span::styled( + " (tab/down to change)", + Style::default().fg(Color::DarkGray), + ), + ])), + chunks[2], + ); + + frame.render_widget( + Paragraph::new(Line::from(Span::styled( + " enter spawn · left/right edit · esc cancel", + Style::default().fg(Color::DarkGray), + ))), + chunks[3], + ); + + let (message, color) = form + .status + .as_ref() + .map(|(message, kind)| { + let color = match kind { + StatusKind::Info => Color::DarkGray, + StatusKind::Progress => Color::Yellow, + StatusKind::Error => Color::Red, + }; + (message.as_str(), color) + }) + .unwrap_or(("", Color::Reset)); + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(" "), + Span::styled(message, Style::default().fg(color)), + ])), + chunks[4], + ); + + let prefix_width = " name: ".chars().count() as u16; + let x = chunks[1] + .x + .saturating_add(prefix_width) + .saturating_add(form.cursor as u16) + .min(chunks[1].right().saturating_sub(1)); + frame.set_cursor_position((x, chunks[1].y)); +} + +fn default_worker_name(workspace_root: &Path) -> String { + workspace_root + .file_name() + .and_then(|name| name.to_str()) + .map(sanitise_default_name) + .filter(|name| !name.is_empty()) + .unwrap_or_else(|| FALLBACK_WORKER_NAME.to_owned()) +} + +fn sanitise_default_name(name: &str) -> String { + name.chars() + .map(|ch| if is_safe_worker_char(ch) { ch } else { '-' }) + .collect() +} + +fn is_safe_worker_char(ch: char) -> bool { + ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.') +} + +fn byte_index(input: &str, char_index: usize) -> usize { + input + .char_indices() + .nth(char_index) + .map_or(input.len(), |(idx, _)| idx) +} + +#[cfg(test)] +mod tests { + use crossterm::event::{KeyEvent, KeyModifiers}; + + use super::*; + + fn choices() -> Vec { + vec![ + ProfileChoice { + selector: "builtin:default".to_owned(), + label: "builtin:default (default) — Default".to_owned(), + is_default: true, + }, + ProfileChoice { + selector: "builtin:coder".to_owned(), + label: "builtin:coder — Coder".to_owned(), + is_default: false, + }, + ] + } + + #[test] + fn default_form_preserves_old_spawn_layout_defaults() { + let form = SpawnForm::new(None, "yoi".to_owned(), choices()); + assert_eq!(form.worker_name, "yoi"); + assert_eq!(form.selected_profile().selector, "builtin:default"); + } + + #[test] + fn tab_and_arrows_cycle_profiles() { + let mut form = SpawnForm::new(None, "yoi".to_owned(), choices()); + assert_eq!( + form.apply_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)), + SpawnAction::None + ); + assert_eq!(form.selected_profile().selector, "builtin:coder"); + form.apply_key(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + assert_eq!(form.selected_profile().selector, "builtin:default"); + form.apply_key(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(form.selected_profile().selector, "builtin:coder"); + } + + #[test] + fn name_input_uses_old_safe_character_policy() { + let mut form = SpawnForm::new(Some("worker".to_owned()), "yoi".to_owned(), choices()); + form.apply_key(KeyEvent::new(KeyCode::Char('-'), KeyModifiers::NONE)); + form.apply_key(KeyEvent::new(KeyCode::Char('1'), KeyModifiers::NONE)); + form.apply_key(KeyEvent::new(KeyCode::Char('/'), KeyModifiers::NONE)); + assert_eq!(form.worker_name, "worker-1"); + } + + #[test] + fn enter_rejects_empty_name_and_escape_cancels() { + let mut form = SpawnForm::new(Some(String::new()), "yoi".to_owned(), choices()); + assert_eq!( + form.apply_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + SpawnAction::None + ); + assert_eq!( + form.status.as_ref().map(|(message, _)| message.as_str()), + Some("worker name cannot be empty") + ); + assert_eq!( + form.apply_key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)), + SpawnAction::Cancel + ); + } + + #[test] + fn renderer_preserves_legacy_inline_spawn_form() { + let backend = ratatui::backend::TestBackend::new(100, VIEWPORT_HEIGHT); + let mut terminal = ratatui::Terminal::new(backend).unwrap(); + let form = SpawnForm::new(None, "yoi".to_owned(), choices()); + terminal.draw(|frame| draw_form(frame, &form)).unwrap(); + let buffer = terminal.backend().buffer(); + let rendered = buffer + .content + .chunks(buffer.area.width as usize) + .map(|row| row.iter().map(|cell| cell.symbol()).collect::()) + .collect::>() + .join("\n"); + + assert!(rendered.contains("spawn worker")); + assert!(rendered.contains("name: yoi")); + assert!(rendered.contains("profile: builtin:default (default) — Default")); + assert!(rendered.contains("enter spawn · left/right edit · esc cancel")); + } + + #[test] + fn builtin_discovery_produces_a_default_profile_choice() { + let registry = ProfileDiscovery::with_sources(None, None) + .discover() + .unwrap(); + let choices = profile_choices(®istry); + let default = choices.iter().find(|choice| choice.is_default).unwrap(); + assert_eq!(default.selector, "builtin:default"); + assert!(default.label.contains("(default)")); + } + + #[test] + fn default_worker_name_comes_from_sanitised_directory_basename() { + assert_eq!( + default_worker_name(Path::new("/home/hare/Project/yoi")), + "yoi" + ); + assert_eq!( + default_worker_name(Path::new("/home/hare/Project/my project")), + "my-project" + ); + assert_eq!(default_worker_name(Path::new("/")), "worker"); + } + + #[test] + fn explicit_profile_bypasses_discovery_and_uses_directory_name() { + let selection = select( + Path::new("/home/hare/Project/yoi"), + None, + Some("builtin:coder".to_owned()), + ) + .unwrap() + .unwrap(); + assert_eq!(selection.worker_name, "yoi"); + assert_eq!(selection.profile, "builtin:coder"); + } +}