From 209b8855a4be2165274c912a8d993243e78100eb Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 13 Nov 2026 10:23:41 -0800 Subject: [PATCH 1/5] Add host::ContainerTarget, the host-neutral way into a project container Phase 1 of the AWS host work (docs/superpowers/specs/2026-11-13-aws-ecs-host-design.md). - host::ContainerTarget (one Docker variant today) wraps every container-interior operation: one-shot/attached/detached execs, resize, exit status, uploads, archive download, is_running. It delegates to docker/exec.rs with no behaviour change. - AttachedExec now yields host-neutral ExecChunk instead of bollard's LogOutput; docker/exec.rs converts at the boundary. - ExecSessionManager moves to host::sessions and holds a ContainerTarget. - A boundary test lists the files that still reach into containers through bollard / docker::exec directly; follow-up commits empty it. Co-Authored-By: Claude Opus 5.5 --- app/src-tauri/Cargo.lock | 1 + app/src-tauri/Cargo.toml | 1 + app/src-tauri/src/auth_bridge/tunnel.rs | 6 +- .../src/commands/migration_commands.rs | 10 +- .../src/commands/terminal_commands.rs | 10 +- app/src-tauri/src/docker/exec.rs | 350 ++++--------- app/src-tauri/src/file_viewer/write.rs | 10 +- app/src-tauri/src/host/mod.rs | 490 ++++++++++++++++++ app/src-tauri/src/host/sessions.rs | 211 ++++++++ app/src-tauri/src/lib.rs | 3 +- app/src-tauri/src/marketplace/gh_login.rs | 14 +- app/src-tauri/src/web_terminal/server.rs | 2 +- app/src-tauri/src/web_terminal/ws_handler.rs | 8 +- 13 files changed, 843 insertions(+), 273 deletions(-) create mode 100644 app/src-tauri/src/host/mod.rs create mode 100644 app/src-tauri/src/host/sessions.rs diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index f14dc98..27b38b6 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -6618,6 +6618,7 @@ dependencies = [ "axum", "base64 0.22.1", "bollard", + "bytes", "chrono", "dirs", "fern", diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index bcd8928..d2f8c1d 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -21,6 +21,7 @@ bollard = "0.18" keyring = { version = "3", features = ["apple-native", "windows-native", "linux-native"] } tokio = { version = "1", features = ["full"] } futures-util = "0.3" +bytes = "1" uuid = { version = "1", features = ["v4"] } chrono = { version = "0.4", features = ["serde"] } dirs = "6" diff --git a/app/src-tauri/src/auth_bridge/tunnel.rs b/app/src-tauri/src/auth_bridge/tunnel.rs index 60c8ff3..9f353c5 100644 --- a/app/src-tauri/src/auth_bridge/tunnel.rs +++ b/app/src-tauri/src/auth_bridge/tunnel.rs @@ -56,7 +56,7 @@ use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::time::Duration; -use bollard::container::LogOutput; +use crate::host::ExecChunk; use futures_util::StreamExt; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; @@ -587,12 +587,12 @@ pub async fn tunnel_connection_with_prelude( // Only stdout is payload. The exec is created with tty = false // precisely so Docker demultiplexes these, keeping socat's stderr // diagnostics out of the proxied byte stream. - Ok(LogOutput::StdOut { message }) => { + Ok(ExecChunk::StdOut(message)) => { if host_tx.write_all(&message).await.is_err() { break; } } - Ok(LogOutput::StdErr { message }) => { + Ok(ExecChunk::StdErr(message)) => { log::debug!( "Auth bridge: socat stderr for port {}: {}", port, diff --git a/app/src-tauri/src/commands/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index 923f9b5..5117479 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -1860,7 +1860,7 @@ async fn stage_payload( project_id: &str, verbatim: &[String], ) -> Result { - use bollard::container::LogOutput; + use crate::host::ExecChunk; use futures_util::StreamExt; use tokio::io::AsyncWriteExt; @@ -1913,14 +1913,14 @@ async fn stage_payload( while let Some(msg) = output.next().await { match msg { - Ok(LogOutput::StdOut { message }) => { + Ok(ExecChunk::StdOut(message)) => { if let Err(e) = writer.write_all(&message).await { stream_err = Some(format!("Failed to write the staging file: {}", e)); break; } total += message.len() as u64; } - Ok(LogOutput::StdErr { message }) => { + Ok(ExecChunk::StdErr(message)) => { stderr_text.push_str(&String::from_utf8_lossy(&message)); } Ok(_) => {} @@ -1979,7 +1979,7 @@ async fn stage_payload( /// anything already present in the base, so collisions should be rare either /// way.) async fn restore_payload(container_id: &str, host_path: &str) -> Result<(), String> { - use bollard::container::LogOutput; + use crate::host::ExecChunk; use futures_util::StreamExt; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -2007,7 +2007,7 @@ async fn restore_payload(container_id: &str, host_path: &str) -> Result<(), Stri let mut text = String::new(); while let Some(msg) = output.next().await { match msg { - Ok(LogOutput::StdErr { message }) | Ok(LogOutput::StdOut { message }) => { + Ok(ExecChunk::StdErr(message)) | Ok(ExecChunk::StdOut(message)) => { text.push_str(&String::from_utf8_lossy(&message)) } Ok(_) => {} diff --git a/app/src-tauri/src/commands/terminal_commands.rs b/app/src-tauri/src/commands/terminal_commands.rs index d1f28e4..5b95164 100644 --- a/app/src-tauri/src/commands/terminal_commands.rs +++ b/app/src-tauri/src/commands/terminal_commands.rs @@ -189,7 +189,7 @@ pub async fn open_terminal_session( state .exec_manager .create_session( - container_id, + &crate::host::ContainerTarget::docker(container_id.as_str()), &session_id, cmd, move |data| { @@ -250,7 +250,11 @@ pub async fn paste_image_to_terminal( state .exec_manager - .write_file_to_container(&container_id, &file_name, &image_data) + .write_file_to_container( + &crate::host::ContainerTarget::docker(container_id.as_str()), + &file_name, + &image_data, + ) .await } @@ -362,7 +366,7 @@ pub async fn start_audio_bridge( state .exec_manager .create_session_with_tty( - &container_id, + &crate::host::ContainerTarget::docker(container_id.as_str()), &audio_session_id, cmd, false, diff --git a/app/src-tauri/src/docker/exec.rs b/app/src-tauri/src/docker/exec.rs index f9f1e91..0e4caac 100644 --- a/app/src-tauri/src/docker/exec.rs +++ b/app/src-tauri/src/docker/exec.rs @@ -1,13 +1,9 @@ use bollard::container::{LogOutput, UploadToContainerOptions}; use bollard::exec::{CreateExecOptions, ResizeExecOptions, StartExecResults}; -use futures_util::{Stream, StreamExt}; -use std::collections::HashMap; -use std::pin::Pin; -use std::sync::Arc; -use tokio::io::{AsyncWrite, AsyncWriteExt}; -use tokio::sync::{mpsc, Mutex}; +use futures_util::StreamExt; use super::client::get_docker; +use crate::host::{ExecChunk, ExecSpec}; /// A `docker exec` that has been created and started with stdin/stdout/stderr /// attached — the raw duplex halves, before any policy about what to do with @@ -24,11 +20,7 @@ use super::client::get_docker; /// consumer can tell [`LogOutput::StdOut`] from [`LogOutput::StdErr`]. That /// distinction matters for the auth bridge: `socat`'s diagnostics must not be /// spliced into the proxied byte stream. -pub struct AttachedExec { - pub exec_id: String, - pub output: Pin> + Send>>, - pub input: Pin>, -} +pub use crate::host::AttachedExec; /// Create and start an exec with stdin + stdout + stderr attached, returning the /// raw duplex halves. Runs as `claude` in `/workspace`, like every other exec @@ -53,6 +45,20 @@ pub async fn create_attached_exec_as( tty: bool, user: &str, working_dir: &str, +) -> Result { + let spec = ExecSpec::new(cmd) + .tty(tty) + .user(user) + .working_dir(Some(working_dir)); + create_attached_exec_spec(container_id, &spec).await +} + +/// The single place an attached exec is opened on Docker. Everything else — +/// [`create_attached_exec`], [`create_attached_exec_as`], and +/// `host::ContainerTarget::attach` — is a thin wrapper over this. +pub async fn create_attached_exec_spec( + container_id: &str, + spec: &ExecSpec, ) -> Result { let docker = get_docker()?; @@ -60,13 +66,14 @@ pub async fn create_attached_exec_as( .create_exec( container_id, CreateExecOptions { - attach_stdin: Some(true), + attach_stdin: Some(spec.attach_stdin), attach_stdout: Some(true), attach_stderr: Some(true), - tty: Some(tty), - cmd: Some(cmd), - user: Some(user.to_string()), - working_dir: Some(working_dir.to_string()), + tty: Some(spec.tty), + cmd: Some(spec.cmd.clone()), + env: if spec.env.is_empty() { None } else { Some(spec.env.clone()) }, + user: Some(spec.user.clone()), + working_dir: spec.working_dir.clone(), ..Default::default() }, ) @@ -82,244 +89,87 @@ pub async fn create_attached_exec_as( { StartExecResults::Attached { output, input } => Ok(AttachedExec { exec_id, - output, + output: Box::pin(output.map(|r| { + r.map(log_output_to_chunk) + .map_err(|e| e.to_string()) + })), input, }), StartExecResults::Detached => Err("Exec started in detached mode".to_string()), } } -pub struct ExecSession { - pub exec_id: String, - pub container_id: String, - pub input_tx: mpsc::UnboundedSender>, - shutdown_tx: mpsc::Sender<()>, -} - -impl ExecSession { - pub async fn send_input(&self, data: Vec) -> Result<(), String> { - self.input_tx - .send(data) - .map_err(|e| format!("Failed to send input: {}", e)) - } - - #[allow(dead_code)] - pub async fn resize(&self, cols: u16, rows: u16) -> Result<(), String> { - let docker = get_docker()?; - docker - .resize_exec( - &self.exec_id, - ResizeExecOptions { - width: cols, - height: rows, - }, - ) - .await - .map_err(|e| format!("Failed to resize exec: {}", e)) - } - - pub fn shutdown(&self) { - let _ = self.shutdown_tx.try_send(()); +/// bollard's `LogOutput` → the host-neutral [`ExecChunk`]. `Console` is what +/// Docker calls a TTY stream; it has no stderr, so it is stdout. +pub fn log_output_to_chunk(o: LogOutput) -> ExecChunk { + match o { + LogOutput::StdOut { message } | LogOutput::Console { message } => ExecChunk::StdOut(message), + LogOutput::StdErr { message } => ExecChunk::StdErr(message), + LogOutput::StdIn { message } => ExecChunk::Other(message), } } -pub struct ExecSessionManager { - sessions: Arc>>, +/// Start a command with nothing attached and return its exec id without +/// waiting for it. +pub async fn exec_detached(container_id: &str, spec: &ExecSpec) -> Result { + let docker = get_docker()?; + let exec = docker + .create_exec( + container_id, + CreateExecOptions { + attach_stdout: Some(false), + attach_stderr: Some(false), + tty: Some(spec.tty), + cmd: Some(spec.cmd.clone()), + env: if spec.env.is_empty() { None } else { Some(spec.env.clone()) }, + user: Some(spec.user.clone()), + working_dir: spec.working_dir.clone(), + ..Default::default() + }, + ) + .await + .map_err(|e| format!("Failed to create exec: {}", e))?; + docker + .start_exec( + &exec.id, + Some(bollard::exec::StartExecOptions { + detach: true, + ..Default::default() + }), + ) + .await + .map_err(|e| format!("Failed to start exec: {}", e))?; + Ok(exec.id) } -impl ExecSessionManager { - pub fn new() -> Self { - Self { - sessions: Arc::new(Mutex::new(HashMap::new())), - } - } - - pub async fn create_session( - &self, - container_id: &str, - session_id: &str, - cmd: Vec, - on_output: F, - on_exit: Box, - ) -> Result<(), String> - where - F: Fn(Vec) + Send + 'static, - { - self.create_session_with_tty(container_id, session_id, cmd, true, on_output, on_exit) - .await - } - - pub async fn create_session_with_tty( - &self, - container_id: &str, - session_id: &str, - cmd: Vec, - tty: bool, - on_output: F, - on_exit: Box, - ) -> Result<(), String> - where - F: Fn(Vec) + Send + 'static, - { - let AttachedExec { +pub async fn resize_exec(exec_id: &str, cols: u16, rows: u16) -> Result<(), String> { + let docker = get_docker()?; + docker + .resize_exec( exec_id, - mut output, - mut input, - } = create_attached_exec(container_id, cmd, tty).await?; + ResizeExecOptions { + width: cols, + height: rows, + }, + ) + .await + .map_err(|e| format!("Failed to resize exec: {}", e)) +} - let (input_tx, mut input_rx) = mpsc::unbounded_channel::>(); - let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); - - // Output reader task - let session_id_clone = session_id.to_string(); - let shutdown_tx_clone = shutdown_tx.clone(); - tokio::spawn(async move { - loop { - tokio::select! { - msg = output.next() => { - match msg { - Some(Ok(output)) => { - on_output(output.into_bytes().to_vec()); - } - Some(Err(e)) => { - log::error!("Exec output error for {}: {}", session_id_clone, e); - break; - } - None => { - log::info!("Exec output stream ended for {}", session_id_clone); - break; - } - } - } - _ = shutdown_rx.recv() => { - log::info!("Exec session {} shutting down", session_id_clone); - break; - } - } - } - on_exit(); - let _ = shutdown_tx_clone; - }); - - // Input writer task - tokio::spawn(async move { - while let Some(data) = input_rx.recv().await { - if let Err(e) = input.write_all(&data).await { - log::error!("Failed to write to exec stdin: {}", e); - break; - } - } - }); - - let session = ExecSession { - exec_id, - container_id: container_id.to_string(), - input_tx, - shutdown_tx, - }; - - self.sessions - .lock() - .await - .insert(session_id.to_string(), session); - - Ok(()) - } - - pub async fn send_input(&self, session_id: &str, data: Vec) -> Result<(), String> { - let sessions = self.sessions.lock().await; - let session = sessions - .get(session_id) - .ok_or_else(|| format!("Session {} not found", session_id))?; - session.send_input(data).await - } - - pub async fn resize(&self, session_id: &str, cols: u16, rows: u16) -> Result<(), String> { - // Clone the exec_id under the lock, then drop the lock before the - // async Docker API call to avoid holding the mutex across await. - let exec_id = { - let sessions = self.sessions.lock().await; - let session = sessions - .get(session_id) - .ok_or_else(|| format!("Session {} not found", session_id))?; - session.exec_id.clone() - }; - let docker = get_docker()?; - docker - .resize_exec( - &exec_id, - ResizeExecOptions { - width: cols, - height: rows, - }, - ) - .await - .map_err(|e| format!("Failed to resize exec: {}", e)) - } - - pub async fn close_session(&self, session_id: &str) { - let mut sessions = self.sessions.lock().await; - if let Some(session) = sessions.remove(session_id) { - session.shutdown(); - } - } - - pub async fn close_sessions_for_container(&self, container_id: &str) { - let mut sessions = self.sessions.lock().await; - let ids_to_close: Vec = sessions - .iter() - .filter(|(_, s)| s.container_id == container_id) - .map(|(id, _)| id.clone()) - .collect(); - for id in ids_to_close { - if let Some(session) = sessions.remove(&id) { - session.shutdown(); - } - } - } - - pub async fn close_all_sessions(&self) { - let mut sessions = self.sessions.lock().await; - for (_, session) in sessions.drain() { - session.shutdown(); - } - } - - pub async fn get_container_id(&self, session_id: &str) -> Result { - let sessions = self.sessions.lock().await; - let session = sessions - .get(session_id) - .ok_or_else(|| format!("Session {} not found", session_id))?; - Ok(session.container_id.clone()) - } - - pub async fn write_file_to_container( - &self, - container_id: &str, - file_name: &str, - data: &[u8], - ) -> Result { - let docker = get_docker()?; - - // Owned by the container user, stamped now: a default tar header would - // land it as root:root/1970 and Claude Code could not rewrite it. - let (uid, gid) = container_user_ids(container_id).await; - let tar_buf = build_single_file_tar(file_name, data, 0o644, uid, gid, now_epoch_secs())?; - - docker - .upload_to_container( - container_id, - Some(UploadToContainerOptions { - path: "/tmp".to_string(), - ..Default::default() - }), - tar_buf.into(), - ) - .await - .map_err(|e| format!("Failed to upload file to container: {}", e))?; - - Ok(format!("/tmp/{}", file_name)) - } +/// Docker's archive API: `path` as a tar stream. Errors are stringified so the +/// stream is host-neutral. +pub fn download_archive(container_id: &str, path: &str) -> crate::host::ArchiveStream { + let docker = match get_docker() { + Ok(d) => d, + Err(e) => return Box::pin(futures_util::stream::once(async move { Err(e) })), + }; + let stream = docker.download_from_container( + container_id, + Some(bollard::container::DownloadFromContainerOptions { + path: path.to_string(), + }), + ); + Box::pin(stream.map(|r| r.map_err(|e| e.to_string()))) } /// Ceiling on one host file packed into a container upload. @@ -444,14 +294,22 @@ pub async fn upload_bytes_to_container( file_name: &str, data: &[u8], mode: u32, +) -> Result { + upload_bytes_with_ids(container_id, dest_dir, file_name, data, mode, (0, 0)).await +} + +/// [`upload_bytes_to_container`] with the tar header's owner set, stamped now. +pub async fn upload_bytes_with_ids( + container_id: &str, + dest_dir: &str, + file_name: &str, + data: &[u8], + mode: u32, + (uid, gid): (u64, u64), ) -> Result { let docker = get_docker()?; - // Root-owned on purpose: migration's `tar -T` list is read back as root, - // and the marketplace sync uploads into a `claude`-owned directory it - // prepares first, so `claude` can still read and delete the files. The - // mtime still gets stamped so the file doesn't read as 1970. - let tar_buf = build_single_file_tar(file_name, data, mode, 0, 0, now_epoch_secs())?; + let tar_buf = build_single_file_tar(file_name, data, mode, uid, gid, now_epoch_secs())?; docker .upload_to_container( diff --git a/app/src-tauri/src/file_viewer/write.rs b/app/src-tauri/src/file_viewer/write.rs index e256adb..d11315c 100644 --- a/app/src-tauri/src/file_viewer/write.rs +++ b/app/src-tauri/src/file_viewer/write.rs @@ -11,7 +11,7 @@ use serde::Serialize; use sha2::{Digest, Sha256}; use crate::commands::file_commands::clip_container_text; -use crate::docker::exec::{exec_oneshot_streams_as, ExecSessionManager}; +use crate::host::{ContainerTarget, ExecSessionManager}; /// Spec §4/§5: only untruncated (≤ 1 MiB) text is editable, so nothing larger is saved. pub const MAX_WRITE_BYTES: usize = 1024 * 1024; @@ -180,12 +180,14 @@ pub async fn write_file( ) -> Result { check_write_input(bytes.len(), base_hash)?; let tmp_name = format!("triple-c-viewer-{}", uuid::Uuid::new_v4().simple()); + let container = ContainerTarget::docker(container_id); let tmp_path = exec_manager - .write_file_to_container(container_id, &tmp_name, bytes) + .write_file_to_container(&container, &tmp_name, bytes) .await?; let cmd = write_command(target, &tmp_path, base_hash); - let (stdout, stderr, code) = - exec_oneshot_streams_as(container_id, "claude", cmd, Vec::new()).await?; + let (stdout, stderr, code) = container + .exec_oneshot_streams_as("claude", cmd, Vec::new()) + .await?; saved_file(classify_write(code, &stdout, &stderr), bytes) } diff --git a/app/src-tauri/src/host/mod.rs b/app/src-tauri/src/host/mod.rs new file mode 100644 index 0000000..5a1f4ab --- /dev/null +++ b/app/src-tauri/src/host/mod.rs @@ -0,0 +1,490 @@ +//! Where a project's container runs, and the one interface the rest of the app +//! uses to reach inside it. +//! +//! Spec: `docs/superpowers/specs/2026-11-13-aws-ecs-host-design.md` ("Host +//! abstraction"). This is phase 1 of that plan: every operation that happens +//! *inside* a project container — one-shot and attached execs, file transfer, +//! "is it running" — goes through [`ContainerTarget`] instead of naming +//! `docker::exec` or bollard directly. Today the only variant is +//! [`ContainerTarget::Docker`], which delegates to the existing code in +//! `docker/exec.rs` and `docker/container.rs` with no change in behaviour. An +//! AWS variant (reached through the in-container agent) slots in beside it. +//! +//! ## Why an enum and not a trait object +//! +//! The set of hosts is closed and known at compile time, every method is async, +//! and a `dyn` async trait would need boxing on every call (or the +//! `async-trait` crate) for no benefit. Adding a host means adding a variant and +//! letting the compiler point at every `match` that needs an arm. +//! +//! ## What does *not* go through here +//! +//! Things that only make sense for the local Docker host: image build/pull, +//! base-image migration, snapshot commits, volumes, the LiteLLM gateway and STT +//! sidecars, and (until phase 1b) container create/start/stop and the `die` +//! event stream. Those keep calling `crate::docker` directly. The +//! `no_container_interior_bollard_outside_the_host_layer` test pins which files +//! may still do so. + +pub mod sessions; + +pub use crate::docker::exec::{ + MAX_DROP_BYTES, MAX_ONESHOT_OUTPUT, OUTPUT_LIMIT_MARKER, PROC_NET_OUTPUT_LIMIT, +}; +pub use sessions::ExecSessionManager; + +use bytes::Bytes; +use futures_util::Stream; +use std::pin::Pin; +use tokio::io::AsyncWrite; + +use crate::docker; +use crate::models::Project; + +/// One chunk of output from an exec, already demultiplexed. +/// +/// Host-neutral stand-in for bollard's `LogOutput`. With `tty = true` there is +/// no stdout/stderr split and everything arrives as [`ExecChunk::StdOut`] (Docker +/// reports a TTY stream as `Console`, which maps here too — consumers that cared +/// treated the two the same). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ExecChunk { + StdOut(Bytes), + StdErr(Bytes), + /// stdin echo or anything else a host might surface. Consumers ignore it. + Other(Bytes), +} + +impl ExecChunk { + pub fn into_bytes(self) -> Bytes { + match self { + ExecChunk::StdOut(b) | ExecChunk::StdErr(b) | ExecChunk::Other(b) => b, + } + } +} + +pub type ExecOutputStream = Pin> + Send>>; + +/// An exec that has been created and started with its streams attached. +/// +/// `input` is a sink that is never read when the exec was opened with +/// `attach_stdin: false`. +pub struct AttachedExec { + pub exec_id: String, + pub output: ExecOutputStream, + pub input: Pin>, +} + +/// Everything about an attached exec except where it runs. +/// +/// Defaults match what this app has always used: run as `claude` in +/// `/workspace`, no TTY, stdin attached, no extra env. +#[derive(Debug, Clone)] +pub struct ExecSpec { + pub cmd: Vec, + pub user: String, + pub working_dir: Option, + pub env: Vec, + pub tty: bool, + pub attach_stdin: bool, +} + +impl ExecSpec { + pub fn new(cmd: Vec) -> Self { + Self { + cmd, + user: "claude".to_string(), + working_dir: Some("/workspace".to_string()), + env: Vec::new(), + tty: false, + attach_stdin: true, + } + } + pub fn user(mut self, user: &str) -> Self { + self.user = user.to_string(); + self + } + pub fn working_dir(mut self, dir: Option<&str>) -> Self { + self.working_dir = dir.map(str::to_string); + self + } + pub fn env(mut self, env: Vec) -> Self { + self.env = env; + self + } + pub fn tty(mut self, tty: bool) -> Self { + self.tty = tty; + self + } + pub fn no_stdin(mut self) -> Self { + self.attach_stdin = false; + self + } +} + +/// A byte stream of a tar archive read out of a container. +pub type ArchiveStream = Pin> + Send>>; + +/// A specific project container on a specific host. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ContainerTarget { + /// A container on the local Docker daemon, by id or name. + Docker { container_id: String }, +} + +impl ContainerTarget { + pub fn docker(container_id: impl Into) -> Self { + ContainerTarget::Docker { + container_id: container_id.into(), + } + } + + /// The target for a project's container, if it has one yet. + pub fn for_project(project: &Project) -> Option { + project.container_id.as_deref().map(Self::docker) + } + + /// The host-specific identifier (a Docker container id today). For logs + /// and for keying per-container state; never parse it. + pub fn id(&self) -> &str { + match self { + ContainerTarget::Docker { container_id } => container_id, + } + } + + // ── state ──────────────────────────────────────────────────────────────── + + /// Whether the container is running. An inspect failure (gone, daemon + /// down) reads as `false`, as `docker::is_container_running` always has. + pub async fn is_running(&self) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::container::is_container_running(container_id).await + } + } + } + + // ── one-shot execs ─────────────────────────────────────────────────────── + + /// Run as `claude`, return combined output. Errors on a non-UTF-8-safe + /// transport failure only; a non-zero exit is *not* an error here. + pub async fn exec_oneshot(&self, cmd: Vec) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot(container_id, cmd).await + } + } + } + + pub async fn exec_oneshot_env( + &self, + cmd: Vec, + env: Vec, + ) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot_env(container_id, cmd, env).await + } + } + } + + pub async fn exec_oneshot_env_status( + &self, + cmd: Vec, + env: Vec, + ) -> Result<(String, i64), String> { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot_env_status(container_id, cmd, env).await + } + } + } + + pub async fn exec_oneshot_as( + &self, + user: &str, + cmd: Vec, + env: Vec, + ) -> Result<(String, i64), String> { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot_as(container_id, user, cmd, env).await + } + } + } + + pub async fn exec_oneshot_limited( + &self, + user: &str, + cmd: Vec, + limit: usize, + ) -> Result<(String, i64), String> { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot_limited(container_id, user, cmd, limit).await + } + } + } + + /// stdout and stderr kept apart: `(stdout, stderr, exit_code)`. + pub async fn exec_oneshot_streams_as( + &self, + user: &str, + cmd: Vec, + env: Vec, + ) -> Result<(String, String, i64), String> { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_oneshot_streams_as(container_id, user, cmd, env).await + } + } + } + + /// Start a command and return without waiting or reading its output. + /// Returns the exec id. + pub async fn exec_detached(&self, spec: ExecSpec) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::exec_detached(container_id, &spec).await + } + } + } + + // ── attached execs ─────────────────────────────────────────────────────── + + /// Run as `claude` in `/workspace` with stdin attached. + pub async fn attach_exec(&self, cmd: Vec, tty: bool) -> Result { + self.attach(ExecSpec::new(cmd).tty(tty)).await + } + + pub async fn attach(&self, spec: ExecSpec) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::create_attached_exec_spec(container_id, &spec).await + } + } + } + + /// The exit code of an exec this target started, once it has finished. + /// `None` if the host could not say. + pub async fn wait_exec_exit(&self, exec_id: &str) -> Option { + match self { + ContainerTarget::Docker { .. } => docker::exec::wait_for_exec_exit(exec_id).await, + } + } + + pub async fn resize_exec(&self, exec_id: &str, cols: u16, rows: u16) -> Result<(), String> { + match self { + ContainerTarget::Docker { .. } => docker::exec::resize_exec(exec_id, cols, rows).await, + } + } + + // ── files ──────────────────────────────────────────────────────────────── + + /// The container user's `(uid, gid)`, falling back to `(1000, 1000)`. + pub async fn user_ids(&self) -> (u64, u64) { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::container_user_ids(container_id).await + } + } + } + + /// Write `data` to `dest_dir/file_name` as root-owned with `mode`. + pub async fn upload_bytes( + &self, + dest_dir: &str, + file_name: &str, + data: &[u8], + mode: u32, + ) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::upload_bytes_to_container( + container_id, + dest_dir, + file_name, + data, + mode, + ) + .await + } + } + } + + /// [`Self::upload_bytes`] with the owner set to `ids`. + pub async fn upload_bytes_with_ids( + &self, + dest_dir: &str, + file_name: &str, + data: &[u8], + mode: u32, + ids: (u64, u64), + ) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::upload_bytes_with_ids( + container_id, + dest_dir, + file_name, + data, + mode, + ids, + ) + .await + } + } + } + + /// Copy a host file in, owned by the container user. + pub async fn upload_host_file( + &self, + host_path: &str, + dest_dir: &str, + dest_name: &str, + ) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::upload_host_file_to_container( + container_id, + host_path, + dest_dir, + dest_name, + ) + .await + } + } + } + + /// [`Self::upload_host_file`] with the owner ids already known. + pub async fn upload_host_file_with_ids( + &self, + host_path: &str, + dest_dir: &str, + dest_name: &str, + ids: (u64, u64), + ) -> Result { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::upload_host_file_with_ids( + container_id, + host_path, + dest_dir, + dest_name, + ids, + ) + .await + } + } + } + + /// Read `path` out of the container as a tar stream (Docker's archive + /// API). Dropping the stream cancels the transfer. + pub fn download_archive(&self, path: &str) -> ArchiveStream { + match self { + ContainerTarget::Docker { container_id } => { + docker::exec::download_archive(container_id, path) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn exec_spec_defaults_match_what_the_app_always_used() { + let s = ExecSpec::new(vec!["true".into()]); + assert_eq!(s.user, "claude"); + assert_eq!(s.working_dir.as_deref(), Some("/workspace")); + assert!(s.env.is_empty()); + assert!(!s.tty); + assert!(s.attach_stdin); + } + + #[test] + fn for_project_follows_the_container_id() { + let mut p: Project = serde_json::from_value(serde_json::json!({ + "id": "p1", "name": "p", "paths": [], "container_id": null, + "status": "stopped", "backend": "anthropic", + "allow_docker_access": false, "full_permissions": false, + "created_at": "now", "updated_at": "now" + })) + .expect("minimal project deserializes"); + assert_eq!(ContainerTarget::for_project(&p), None); + p.container_id = Some("abc".into()); + assert_eq!( + ContainerTarget::for_project(&p), + Some(ContainerTarget::docker("abc")) + ); + assert_eq!(ContainerTarget::docker("abc").id(), "abc"); + } + + /// Phase 1's boundary, pinned: outside `docker/` and `host/`, only the + /// files listed here may name bollard or `docker::exec`. Each entry is + /// Docker-only by nature (migration, snapshots, sidecars) or is scheduled + /// for phase 1b (lifecycle, the `die` event stream). Shrink this list; do + /// not grow it — new container-interior code goes through + /// [`ContainerTarget`]. + #[test] + fn no_container_interior_bollard_outside_the_host_layer() { + const ALLOWED: &[&str] = &[ + // Docker-only: base-image migration replays apt / restores a payload. + "commands/migration_commands.rs", + // Phase 1b: lifecycle and the `die` event stream. + "container_watch.rs", + // Phase 1a, not yet migrated. Each migration removes its own + // entries; the list must be empty before phase 1a merges. + "auth_bridge/mod.rs", + "auth_bridge/tunnel.rs", + "browser_view/detect.rs", + "browser_view/install.rs", + "browser_view/mod.rs", + "browser_view/page.rs", + "commands/auth_token_commands.rs", + "commands/file_commands.rs", + "commands/inspect_commands.rs", + "commands/terminal_commands.rs", + "file_viewer/poll.rs", + "file_viewer/resolve.rs", + "marketplace/gh_login.rs", + "marketplace/sync.rs", + ]; + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut offenders = Vec::new(); + let mut stack = vec![src.clone()]; + while let Some(dir) = stack.pop() { + for entry in std::fs::read_dir(&dir).unwrap() { + let path = entry.unwrap().path(); + let rel = path + .strip_prefix(&src) + .unwrap() + .to_string_lossy() + .replace('\\', "/"); + if path.is_dir() { + if rel != "docker" && rel != "host" { + stack.push(path); + } + continue; + } + if !rel.ends_with(".rs") || ALLOWED.contains(&rel.as_str()) { + continue; + } + let text = std::fs::read_to_string(&path).unwrap(); + if text.contains("bollard::") + || text.contains("docker::exec") + || text.contains("get_docker") + { + offenders.push(rel); + } + } + } + offenders.sort(); + assert!( + offenders.is_empty(), + "these files reach into the container without going through host::ContainerTarget: {:?}", + offenders + ); + } +} diff --git a/app/src-tauri/src/host/sessions.rs b/app/src-tauri/src/host/sessions.rs new file mode 100644 index 0000000..acdcbba --- /dev/null +++ b/app/src-tauri/src/host/sessions.rs @@ -0,0 +1,211 @@ +//! Interactive exec sessions — terminals and the audio bridge — keyed by the +//! frontend's session id. Host-neutral: every session holds the +//! [`ContainerTarget`] it was opened on and does all its I/O through it. +//! (Moved here from `docker/exec.rs` in phase 1 of the AWS host work.) + +use futures_util::StreamExt; +use std::collections::HashMap; +use std::sync::Arc; +use tokio::io::AsyncWriteExt; +use tokio::sync::{mpsc, Mutex}; + +use super::{AttachedExec, ContainerTarget}; + +pub struct ExecSession { + pub exec_id: String, + pub target: ContainerTarget, + pub input_tx: mpsc::UnboundedSender>, + shutdown_tx: mpsc::Sender<()>, +} + +impl ExecSession { + pub async fn send_input(&self, data: Vec) -> Result<(), String> { + self.input_tx + .send(data) + .map_err(|e| format!("Failed to send input: {}", e)) + } + + #[allow(dead_code)] + pub async fn resize(&self, cols: u16, rows: u16) -> Result<(), String> { + self.target.resize_exec(&self.exec_id, cols, rows).await + } + + pub fn shutdown(&self) { + let _ = self.shutdown_tx.try_send(()); + } +} + +pub struct ExecSessionManager { + sessions: Arc>>, +} + +impl ExecSessionManager { + pub fn new() -> Self { + Self { + sessions: Arc::new(Mutex::new(HashMap::new())), + } + } + + pub async fn create_session( + &self, + target: &ContainerTarget, + session_id: &str, + cmd: Vec, + on_output: F, + on_exit: Box, + ) -> Result<(), String> + where + F: Fn(Vec) + Send + 'static, + { + self.create_session_with_tty(target, session_id, cmd, true, on_output, on_exit) + .await + } + + pub async fn create_session_with_tty( + &self, + target: &ContainerTarget, + session_id: &str, + cmd: Vec, + tty: bool, + on_output: F, + on_exit: Box, + ) -> Result<(), String> + where + F: Fn(Vec) + Send + 'static, + { + let AttachedExec { + exec_id, + mut output, + mut input, + } = target.attach_exec(cmd, tty).await?; + + let (input_tx, mut input_rx) = mpsc::unbounded_channel::>(); + let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); + + // Output reader task + let session_id_clone = session_id.to_string(); + let shutdown_tx_clone = shutdown_tx.clone(); + tokio::spawn(async move { + loop { + tokio::select! { + msg = output.next() => { + match msg { + Some(Ok(output)) => { + on_output(output.into_bytes().to_vec()); + } + Some(Err(e)) => { + log::error!("Exec output error for {}: {}", session_id_clone, e); + break; + } + None => { + log::info!("Exec output stream ended for {}", session_id_clone); + break; + } + } + } + _ = shutdown_rx.recv() => { + log::info!("Exec session {} shutting down", session_id_clone); + break; + } + } + } + on_exit(); + let _ = shutdown_tx_clone; + }); + + // Input writer task + tokio::spawn(async move { + while let Some(data) = input_rx.recv().await { + if let Err(e) = input.write_all(&data).await { + log::error!("Failed to write to exec stdin: {}", e); + break; + } + } + }); + + let session = ExecSession { + exec_id, + target: target.clone(), + input_tx, + shutdown_tx, + }; + + self.sessions + .lock() + .await + .insert(session_id.to_string(), session); + + Ok(()) + } + + pub async fn send_input(&self, session_id: &str, data: Vec) -> Result<(), String> { + let sessions = self.sessions.lock().await; + let session = sessions + .get(session_id) + .ok_or_else(|| format!("Session {} not found", session_id))?; + session.send_input(data).await + } + + pub async fn resize(&self, session_id: &str, cols: u16, rows: u16) -> Result<(), String> { + // Clone the exec_id under the lock, then drop the lock before the + // async host call to avoid holding the mutex across await. + let (exec_id, target) = { + let sessions = self.sessions.lock().await; + let session = sessions + .get(session_id) + .ok_or_else(|| format!("Session {} not found", session_id))?; + (session.exec_id.clone(), session.target.clone()) + }; + target.resize_exec(&exec_id, cols, rows).await + } + + pub async fn close_session(&self, session_id: &str) { + let mut sessions = self.sessions.lock().await; + if let Some(session) = sessions.remove(session_id) { + session.shutdown(); + } + } + + pub async fn close_sessions_for_container(&self, container_id: &str) { + let mut sessions = self.sessions.lock().await; + let ids_to_close: Vec = sessions + .iter() + .filter(|(_, s)| s.target.id() == container_id) + .map(|(id, _)| id.clone()) + .collect(); + for id in ids_to_close { + if let Some(session) = sessions.remove(&id) { + session.shutdown(); + } + } + } + + pub async fn close_all_sessions(&self) { + let mut sessions = self.sessions.lock().await; + for (_, session) in sessions.drain() { + session.shutdown(); + } + } + + pub async fn get_container_id(&self, session_id: &str) -> Result { + let sessions = self.sessions.lock().await; + let session = sessions + .get(session_id) + .ok_or_else(|| format!("Session {} not found", session_id))?; + Ok(session.target.id().to_string()) + } + + pub async fn write_file_to_container( + &self, + target: &ContainerTarget, + file_name: &str, + data: &[u8], + ) -> Result { + // Owned by the container user, stamped now: a default tar header would + // land it as root:root/1970 and Claude Code could not rewrite it. + let ids = target.user_ids().await; + target + .upload_bytes_with_ids("/tmp", file_name, data, 0o644, ids) + .await + } +} diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 3155056..dd06e6a 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -7,6 +7,7 @@ mod container_watch; #[cfg(target_os = "linux")] mod linux_media; mod docker; +mod host; pub mod file_viewer; mod install_helper; mod logging; @@ -22,7 +23,7 @@ use std::sync::{Arc, Mutex}; use std::time::Duration; use auth_bridge::AuthBridgeManager; -use docker::exec::ExecSessionManager; +use host::ExecSessionManager; use storage::projects_store::ProjectsStore; use storage::settings_store::SettingsStore; use tauri::async_runtime::JoinHandle; diff --git a/app/src-tauri/src/marketplace/gh_login.rs b/app/src-tauri/src/marketplace/gh_login.rs index 8a4be8b..3382c29 100644 --- a/app/src-tauri/src/marketplace/gh_login.rs +++ b/app/src-tauri/src/marketplace/gh_login.rs @@ -5,7 +5,7 @@ use std::time::Duration; -use bollard::container::LogOutput; +use crate::host::ExecChunk; use futures_util::{Stream, StreamExt}; use tauri::{AppHandle, Emitter}; use tokio::io::{AsyncWrite, AsyncWriteExt}; @@ -187,7 +187,7 @@ async fn drive_login( mut emit: E, ) -> Result where - S: Stream> + Unpin, + S: Stream> + Unpin, W: AsyncWrite + Unpin, E: FnMut(&'static str, serde_json::Value), { @@ -496,23 +496,19 @@ mod tests { /// successful one does not. mod teardown { use super::super::*; - use bollard::container::LogOutput; use futures_util::stream; use std::pin::Pin; use std::sync::{Arc, Mutex}; use std::task::{Context, Poll}; - type Frame = Result; + type Frame = Result; fn out(s: &'static str) -> Frame { - Ok(LogOutput::StdOut { message: s.into() }) + Ok(ExecChunk::StdOut(s.into())) } fn lost() -> Frame { - Err(bollard::errors::Error::DockerResponseServerError { - status_code: 500, - message: "connection reset".to_string(), - }) + Err("connection reset".to_string()) } /// Records every write separately; or fails every write. diff --git a/app/src-tauri/src/web_terminal/server.rs b/app/src-tauri/src/web_terminal/server.rs index c3af982..07a4578 100644 --- a/app/src-tauri/src/web_terminal/server.rs +++ b/app/src-tauri/src/web_terminal/server.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use tokio::sync::watch; use tower_http::cors::CorsLayer; -use crate::docker::exec::ExecSessionManager; +use crate::host::ExecSessionManager; use crate::storage::projects_store::ProjectsStore; use crate::storage::settings_store::SettingsStore; diff --git a/app/src-tauri/src/web_terminal/ws_handler.rs b/app/src-tauri/src/web_terminal/ws_handler.rs index e38eb61..1782381 100644 --- a/app/src-tauri/src/web_terminal/ws_handler.rs +++ b/app/src-tauri/src/web_terminal/ws_handler.rs @@ -329,7 +329,13 @@ async fn handle_open( state .exec_manager - .create_session(container_id, &session_id, cmd, on_output, on_exit) + .create_session( + &crate::host::ContainerTarget::docker(container_id.as_str()), + &session_id, + cmd, + on_output, + on_exit, + ) .await?; // Track this session for cleanup on disconnect -- 2.52.0 From 737a608dd58f666f66d18b34c1db61ac442ecccd Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 13 Nov 2026 10:28:02 -0800 Subject: [PATCH 2/5] Route auth-token, file viewer and marketplace execs through ContainerTarget setup-token, the viewer's poll/probe execs, the marketplace sync and the container gh login now reach the container via host::ContainerTarget instead of docker::exec / is_container_running. No behaviour change: same user, working dir, env, tty and stdin attachment for every exec. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/auth_token_commands.rs | 34 +++++++------- .../src/commands/marketplace_commands.rs | 12 ++--- app/src-tauri/src/file_viewer/poll.rs | 7 +-- app/src-tauri/src/file_viewer/resolve.rs | 7 +-- app/src-tauri/src/marketplace/gh_login.rs | 47 +++++++++---------- app/src-tauri/src/marketplace/sync.rs | 42 ++++++++--------- 6 files changed, 71 insertions(+), 78 deletions(-) diff --git a/app/src-tauri/src/commands/auth_token_commands.rs b/app/src-tauri/src/commands/auth_token_commands.rs index c56545a..69b01d1 100644 --- a/app/src-tauri/src/commands/auth_token_commands.rs +++ b/app/src-tauri/src/commands/auth_token_commands.rs @@ -76,8 +76,7 @@ use tauri::{AppHandle, Emitter, State}; use tokio::io::AsyncWriteExt; use tokio::sync::{mpsc, oneshot, Mutex}; -use crate::docker::container::is_container_running; -use crate::docker::exec::{create_attached_exec, wait_for_exec_exit, AttachedExec}; +use crate::host::{AttachedExec, ContainerTarget}; use crate::storage::secure; use crate::AppState; @@ -867,7 +866,7 @@ async fn type_code_then_enter( async fn run_setup_token( app: &AppHandle, project_id: &str, - container_id: &str, + target: &ContainerTarget, mut input_rx: mpsc::UnboundedReceiver>, mut cancel_rx: oneshot::Receiver<()>, ) -> Result { @@ -877,16 +876,16 @@ async fn run_setup_token( exec_id, mut output, mut input, - } = create_attached_exec( - container_id, - vec![ - "sh".to_string(), - "-c".to_string(), - SETUP_TOKEN_SCRIPT.to_string(), - ], - true, - ) - .await?; + } = target + .attach_exec( + vec![ + "sh".to_string(), + "-c".to_string(), + SETUP_TOKEN_SCRIPT.to_string(), + ], + true, + ) + .await?; let mut stripper = AnsiStripper::default(); let mut redactor = SecretRedactor::default(); @@ -1041,7 +1040,7 @@ async fn run_setup_token( // Falling through to the token parse is right either way — a run that // printed a token succeeded whatever Docker says about it — but it is worth // saying so rather than silently calling it a clean exit. - match wait_for_exec_exit(&exec_id).await { + match target.wait_exec_exit(&exec_id).await { Some(0) => {} Some(code) => { return Err(format!( @@ -1091,13 +1090,13 @@ pub async fn acquire_claude_token( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project.container_id.clone().ok_or_else(|| { + let target = ContainerTarget::for_project(&project).ok_or_else(|| { format!( "Project '{}' has no container yet. Start it, then run authentication again.", project.name ) })?; - if !is_container_running(&container_id).await.unwrap_or(false) { + if !target.is_running().await.unwrap_or(false) { return Err(format!( "The container for '{}' is not running. Start it, then run authentication again.", project.name @@ -1133,8 +1132,7 @@ pub async fn acquire_claude_token( "Running `claude setup-token` — sign in at the URL below, then submit the code it gives you.", ); - let result = - run_setup_token(&app_handle, &project_id, &container_id, input_rx, cancel_rx).await; + let result = run_setup_token(&app_handle, &project_id, &target, input_rx, cancel_rx).await; // Release the flow. Nothing else needs unwinding: this command changes no // persisted state until the token itself is stored, which is the point. diff --git a/app/src-tauri/src/commands/marketplace_commands.rs b/app/src-tauri/src/commands/marketplace_commands.rs index 0f96436..c346224 100644 --- a/app/src-tauri/src/commands/marketplace_commands.rs +++ b/app/src-tauri/src/commands/marketplace_commands.rs @@ -7,7 +7,7 @@ use std::collections::{BTreeMap, HashSet}; use tauri::{AppHandle, Emitter, State}; use tokio::sync::oneshot; -use crate::docker::container::is_container_running; +use crate::host::ContainerTarget; use crate::marketplace::{self as mk, auth, diff, gh_login, git, MarketplaceManager}; use crate::models::marketplace::{ is_valid_commit, is_valid_item_key, AccountMethod, FileDiff, InstallScope, ItemUpdate, @@ -944,8 +944,8 @@ pub async fn apply_marketplace_now( }; let mut results = Vec::new(); for p in projects { - let running = match &p.container_id { - Some(cid) => is_container_running(cid).await.unwrap_or(false), + let running = match ContainerTarget::for_project(&p) { + Some(target) => target.is_running().await.unwrap_or(false), None => false, }; if !running { @@ -1051,13 +1051,13 @@ pub async fn start_marketplace_gh_container_login( return Err("Signing in through a container needs a host name without a port.".to_string()); } let project = find_project(&state, &project_id)?; - let container_id = project.container_id.clone().ok_or_else(|| { + let target = ContainerTarget::for_project(&project).ok_or_else(|| { format!( "\"{}\" has no container yet. Start it, then try again.", project.name ) })?; - if !is_container_running(&container_id).await.unwrap_or(false) { + if !target.is_running().await.unwrap_or(false) { return Err(format!( "\"{}\" is not running. Start it, then try again.", project.name @@ -1070,7 +1070,7 @@ pub async fn start_marketplace_gh_container_login( } let account_id = uuid::Uuid::new_v4().to_string(); let result = - gh_login::run_gh_container_login(&app_handle, &account_id, &container_id, &host, rx).await; + gh_login::run_gh_container_login(&app_handle, &account_id, &target, &host, rx).await; // `rx` is gone now, so this frees our slot and never a newer login's. state.marketplace.release_gh_login().await; let token = result?; diff --git a/app/src-tauri/src/file_viewer/poll.rs b/app/src-tauri/src/file_viewer/poll.rs index d7301cc..28dd359 100644 --- a/app/src-tauri/src/file_viewer/poll.rs +++ b/app/src-tauri/src/file_viewer/poll.rs @@ -7,7 +7,7 @@ use serde::Serialize; -use crate::docker::exec::exec_oneshot_streams_as; +use crate::host::ContainerTarget; #[derive(Clone, Debug, Serialize, PartialEq, Eq)] pub struct ViewerPoll { @@ -48,8 +48,9 @@ pub async fn poll_file(container_id: &str, container_path: &str) -> Result Result, String> { let mut cmd: Vec = vec!["sh".into(), "-c".into(), PROBE_SCRIPT.into(), "probe".into()]; cmd.extend(candidates.iter().cloned()); - let (stdout, _stderr, _code) = - exec_oneshot_streams_as(container_id, "claude", cmd, Vec::new()).await?; + let (stdout, _stderr, _code) = ContainerTarget::docker(container_id) + .exec_oneshot_streams_as("claude", cmd, Vec::new()) + .await?; Ok(parse_probe_output(&stdout)) } diff --git a/app/src-tauri/src/marketplace/gh_login.rs b/app/src-tauri/src/marketplace/gh_login.rs index 3382c29..760e85a 100644 --- a/app/src-tauri/src/marketplace/gh_login.rs +++ b/app/src-tauri/src/marketplace/gh_login.rs @@ -5,16 +5,13 @@ use std::time::Duration; -use crate::host::ExecChunk; use futures_util::{Stream, StreamExt}; use tauri::{AppHandle, Emitter}; use tokio::io::{AsyncWrite, AsyncWriteExt}; use tokio::sync::oneshot; use crate::commands::auth_token_commands::{push_capped_tail, AnsiStripper, SUBMIT_ENTER_DELAY}; -use crate::docker::exec::{ - create_attached_exec_as, exec_oneshot_as, wait_for_exec_exit, AttachedExec, -}; +use crate::host::{AttachedExec, ContainerTarget, ExecChunk, ExecSpec}; pub const CODE_EVENT: &str = "marketplace-gh-login-code"; pub const OUTPUT_EVENT: &str = "marketplace-gh-login-output"; @@ -32,8 +29,8 @@ const MAX_PENDING_LINE: usize = 4096; /// script around it (whose text contains the same words); errors are ignored. const CANCEL_PKILL: [&str; 3] = ["pkill", "-f", "gh auth login --hostname"]; -/// Constant script; the host is `$1` (argv, never interpolated), because -/// `create_attached_exec_as` takes no env. +/// Constant script; the host is `$1` (argv, never interpolated); the attached +/// exec is opened with no extra env. /// /// * `GH_CONFIG_DIR` / `GIT_CONFIG_GLOBAL` live in a temp dir removed on exit, /// so the container is never left logged in. The `HUP INT TERM` trap turns a @@ -150,9 +147,9 @@ fn failure_tail(transcript: &str) -> String { /// Pre-flight N9 / review fix 1: stop the in-container login after any /// failed attempt. -async fn kill_container_login(container_id: &str) { +async fn kill_container_login(target: &ContainerTarget) { let cmd = CANCEL_PKILL.iter().map(|s| s.to_string()).collect(); - let _ = exec_oneshot_as(container_id, "claude", cmd, vec![]).await; + let _ = target.exec_oneshot_as("claude", cmd, vec![]).await; } /// Hand `result` back, running `cleanup` first when it is a failure. @@ -262,7 +259,7 @@ where pub async fn run_gh_container_login( app: &AppHandle, account_id: &str, - container_id: &str, + target: &ContainerTarget, host: &str, mut cancel: oneshot::Receiver<()>, ) -> Result { @@ -273,20 +270,20 @@ pub async fn run_gh_container_login( exec_id, output, input, - } = create_attached_exec_as( - container_id, - vec![ - "sh".to_string(), - "-c".to_string(), - GH_LOGIN_SCRIPT.to_string(), - "triple-c-gh-login".to_string(), - host.to_string(), - ], - true, - "claude", - "/home/claude", - ) - .await?; + } = target + .attach( + ExecSpec::new(vec![ + "sh".to_string(), + "-c".to_string(), + GH_LOGIN_SCRIPT.to_string(), + "triple-c-gh-login".to_string(), + host.to_string(), + ]) + .tty(true) + .user("claude") + .working_dir(Some("/home/claude")), + ) + .await?; let deadline = tokio::time::Instant::now() + LOGIN_TIMEOUT; let result = async { @@ -305,7 +302,7 @@ pub async fn run_gh_container_login( if let Some(token) = extract_token(&transcript) { return Ok(token); } - let status = wait_for_exec_exit(&exec_id).await; + let status = target.wait_exec_exit(&exec_id).await; Err(format!( "gh did not complete the sign-in (exit status {}). Nothing was stored.\n{}", status @@ -315,7 +312,7 @@ pub async fn run_gh_container_login( )) } .await; - cleanup_on_error(result, || kill_container_login(container_id)).await + cleanup_on_error(result, || kill_container_login(target)).await } #[cfg(test)] diff --git a/app/src-tauri/src/marketplace/sync.rs b/app/src-tauri/src/marketplace/sync.rs index 7ce9607..7c7352b 100644 --- a/app/src-tauri/src/marketplace/sync.rs +++ b/app/src-tauri/src/marketplace/sync.rs @@ -4,7 +4,7 @@ use std::time::Duration; use super::payload::Payload; -use crate::docker::exec::{exec_oneshot_as, exec_oneshot_streams_as, upload_bytes_to_container}; +use crate::host::ContainerTarget; use crate::models::marketplace::{SkippedItem, SyncReport}; /// Where the payload and the script are uploaded. Owned by `claude`. @@ -57,10 +57,12 @@ fn run_script_env() -> Vec { vec!["HOME=/home/claude".to_string()] } -async fn wait_until_ready(container_id: &str) -> Result<(), String> { +async fn wait_until_ready(target: &ContainerTarget) -> Result<(), String> { let deadline = tokio::time::Instant::now() + READY_TIMEOUT; loop { - let (_, code) = exec_oneshot_as(container_id, "root", ready_probe_cmd(), vec![]).await?; + let (_, code) = target + .exec_oneshot_as("root", ready_probe_cmd(), vec![]) + .await?; if code == 0 { return Ok(()); } @@ -91,34 +93,28 @@ fn tail(text: &str, max: usize) -> &str { /// Wait for readiness, upload the payload and the script, run the script as /// `claude`, and return its report. pub async fn sync_container(container_id: &str, payload: &Payload) -> Result { - wait_until_ready(container_id).await?; + let target = ContainerTarget::docker(container_id); + wait_until_ready(&target).await?; - let (out, code) = exec_oneshot_as(container_id, "root", sh(PREPARE_SCRIPT), vec![]).await?; + let (out, code) = target + .exec_oneshot_as("root", sh(PREPARE_SCRIPT), vec![]) + .await?; if code != 0 { return Err(format!( "Could not prepare the container for the marketplace sync: {}", tail(&out, 500) )); } - upload_bytes_to_container( - container_id, - INCOMING_DIR, - "payload.tar", - &payload.tar, - 0o644, - ) - .await?; - upload_bytes_to_container( - container_id, - INCOMING_DIR, - "sync.sh", - SYNC_SCRIPT.as_bytes(), - 0o755, - ) - .await?; + target + .upload_bytes(INCOMING_DIR, "payload.tar", &payload.tar, 0o644) + .await?; + target + .upload_bytes(INCOMING_DIR, "sync.sh", SYNC_SCRIPT.as_bytes(), 0o755) + .await?; - let (stdout, stderr, code) = - exec_oneshot_streams_as(container_id, "claude", run_script_cmd(), run_script_env()).await?; + let (stdout, stderr, code) = target + .exec_oneshot_streams_as("claude", run_script_cmd(), run_script_env()) + .await?; parse_report(&stdout).map_err(|e| { format!( "The marketplace sync script failed (exit {code}): {e}. {}", -- 2.52.0 From 41010a9c50c84ffd48f702beadc24af461f94588 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 13 Nov 2026 10:28:27 -0800 Subject: [PATCH 3/5] Route file and inspect commands through host::ContainerTarget file_commands.rs and inspect_commands.rs no longer name bollard, docker::exec or get_docker. The archive read, running checks, attached download/backup execs, one-shot execs, uploads and the detached scheduler run all go through ContainerTarget, with each exec's options (user, env, no working dir, no TTY, no stdin) and user-facing error wording preserved. fetch_container_file and require_running keep their container-id signatures because file_viewer_commands.rs calls them with an id. Co-Authored-By: Claude Opus 5.5 --- app/src-tauri/src/commands/file_commands.rs | 256 ++++++++---------- .../src/commands/inspect_commands.rs | 151 +++++------ 2 files changed, 177 insertions(+), 230 deletions(-) diff --git a/app/src-tauri/src/commands/file_commands.rs b/app/src-tauri/src/commands/file_commands.rs index 53e4480..f28f7a3 100644 --- a/app/src-tauri/src/commands/file_commands.rs +++ b/app/src-tauri/src/commands/file_commands.rs @@ -4,14 +4,11 @@ use std::sync::Arc; use base64::engine::general_purpose::STANDARD as BASE64; use base64::Engine as _; -use bollard::container::{DownloadFromContainerOptions, LogOutput}; -use bollard::exec::{CreateExecOptions, StartExecResults}; use futures_util::StreamExt; use serde::Serialize; use tauri::State; -use crate::docker::client::get_docker; -use crate::docker::exec::{exec_oneshot_as, exec_oneshot_streams_as, OUTPUT_LIMIT_MARKER}; +use crate::host::{ContainerTarget, ExecChunk, ExecSpec, OUTPUT_LIMIT_MARKER}; use crate::AppState; #[derive(Debug, PartialEq, Serialize)] @@ -63,9 +60,7 @@ pub async fn list_container_files( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; // `exec_oneshot` discards the exit code, which is how a `find` that listed @@ -78,7 +73,8 @@ pub async fn list_container_files( // are the error message, its `-printf` records are the listing, and the // parser should never be handed the former. let (records, diagnostics, code) = - exec_oneshot_streams_as(container_id, "claude", list_argv(&path), Vec::new()) + target + .exec_oneshot_streams_as("claude", list_argv(&path), Vec::new()) .await .map_err(|e| describe_listing_failure(&path, e))?; @@ -156,7 +152,7 @@ fn list_argv(path: &str) -> Vec { /// folder can act on. /// /// One case is worth naming: a directory with more entries than -/// [`crate::docker::exec::MAX_ONESHOT_OUTPUT`] will hold. Roughly 100k names is +/// [`crate::host::MAX_ONESHOT_OUTPUT`] will hold. Roughly 100k names is /// the point where a `find` record set passes 8 MiB, and what the panel showed /// was "Command output exceeded 8388608 bytes and was abandoned" — a true /// statement about a buffer, and no help at all about a directory. @@ -430,7 +426,11 @@ pub(crate) fn validate_container_write_path(what: &str, path: &str) -> Result<() /// and the lexical answer stands: failing every write closed would break the /// panel outright for a risk the container user does not need this code path to /// take. -async fn resolve_container_dir(container_id: &str, what: &str, dir: &str) -> Result<(), String> { +async fn resolve_container_dir( + target: &ContainerTarget, + what: &str, + dir: &str, +) -> Result<(), String> { validate_container_write_path(what, dir)?; // Split streams, not the combined buffer: `realpath`'s answer is a *path* @@ -438,18 +438,18 @@ async fn resolve_container_dir(container_id: &str, what: &str, dir: &str) -> Res // hazard the listing above took apart for `find`. A warning on stderr — // and there is one whenever a component is unreadable — used to be spliced // into the string this then compared against the write roots. - let (stdout, diagnostics, code) = exec_oneshot_streams_as( - container_id, - "claude", - vec![ - "realpath".to_string(), - "-m".to_string(), - "--".to_string(), - dir.to_string(), - ], - Vec::new(), - ) - .await?; + let (stdout, diagnostics, code) = target + .exec_oneshot_streams_as( + "claude", + vec![ + "realpath".to_string(), + "-m".to_string(), + "--".to_string(), + dir.to_string(), + ], + Vec::new(), + ) + .await?; let resolved = stdout.trim(); if code != 0 || resolved.is_empty() { @@ -1188,7 +1188,7 @@ pub(crate) struct FetchedFile { /// Fetch a single regular file from a container as exact bytes. /// -/// The viewer's read. It deliberately goes through Docker's archive endpoint rather than +/// The viewer's read. It deliberately goes through the archive endpoint rather than /// `exec_oneshot`: that reader runs every chunk through `String::from_utf8_lossy` /// and merges stderr into stdout, so it would both corrupt any non-UTF-8 file /// and be able to splice diagnostics into what the caller believes is content. @@ -1202,19 +1202,15 @@ pub(crate) struct FetchedFile { /// file — or the whole *directory tree*, since the type check happens after the /// read — landed in host RAM twice. This function buffers, so every caller of /// it must name a ceiling. +/// +/// Takes a container id rather than a [`ContainerTarget`] because the file +/// viewer's commands call it with the id they already hold. pub(crate) async fn fetch_container_file( container_id: &str, container_path: &str, max_bytes: u64, ) -> Result { - let docker = get_docker()?; - - let mut stream = docker.download_from_container( - container_id, - Some(DownloadFromContainerOptions { - path: container_path.to_string(), - }), - ); + let mut stream = ContainerTarget::docker(container_id).download_archive(container_path); // A tar member is a 512-byte header plus payload padded to 512. 8 KiB of // slack past the payload cap guarantees the header and the whole capped @@ -1297,13 +1293,11 @@ pub async fn read_container_file( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; let cap = max_bytes.unwrap_or(MAX_READ_BYTES).min(MAX_READ_BYTES); - let fetched = fetch_container_file(container_id, &path, cap).await?; + let fetched = fetch_container_file(target.id(), &path, cap).await?; Ok(FileContents { contents_base64: BASE64.encode(&fetched.bytes), @@ -1333,9 +1327,7 @@ pub async fn rename_container_path( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; // The name is checked by `validate_entry_name`; the path it is applied to @@ -1349,7 +1341,7 @@ pub async fn rename_container_path( // directory component to resolve. let parent = parent_dir(&from_path); if parent != "/" { - resolve_container_dir(container_id, "Item", &parent).await?; + resolve_container_dir(&target, "Item", &parent).await?; } let new_name = to_path.trim(); @@ -1364,8 +1356,7 @@ pub async fn rename_container_path( // and exits 0 — so `-n` on its own would report a rename that never // happened. The existence check is what turns it into an error the user // sees; `-n` stays as the belt-and-braces against the race between them. - let (_, exists) = exec_oneshot_as( - container_id, + let (_, exists) = target.exec_oneshot_as( "claude", vec!["test".to_string(), "-e".to_string(), dest.clone()], Vec::new(), @@ -1375,8 +1366,7 @@ pub async fn rename_container_path( return Err(format!("\"{}\" already exists in this folder", new_name)); } - let (output, code) = exec_oneshot_as( - container_id, + let (output, code) = target.exec_oneshot_as( "claude", vec![ "mv".to_string(), @@ -1417,19 +1407,16 @@ pub async fn create_container_directory( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; - resolve_container_dir(container_id, "Folder", &parent_path).await?; + resolve_container_dir(&target, "Folder", &parent_path).await?; let name = name.trim(); validate_entry_name(name)?; let dest = join_path(&parent_path, name); - let (output, code) = exec_oneshot_as( - container_id, + let (output, code) = target.exec_oneshot_as( "claude", vec!["mkdir".to_string(), "--".to_string(), dest.clone()], Vec::new(), @@ -1448,6 +1435,17 @@ pub async fn create_container_directory( Ok(dest) } +/// The underlying cause of a [`ContainerTarget::attach`] failure, without the +/// host layer's own "Failed to create exec: " / "Failed to start exec: " +/// prefix, so the transfers below can keep framing it in their own words as +/// they did when they opened the exec themselves. Anything else (an +/// unreachable daemon, a detached start) passes through whole. +fn strip_exec_error_prefix(e: &str) -> &str { + e.strip_prefix("Failed to create exec: ") + .or_else(|| e.strip_prefix("Failed to start exec: ")) + .unwrap_or(e) +} + /// Every "container is not running" refusal starts with this, so a caller (the file /// viewer's poll, `app/src/viewer/ipcMessages.ts`) can tell it apart from any other failure. pub(crate) const NOT_RUNNING_PREFIX: &str = "Start the project before"; @@ -1459,20 +1457,19 @@ pub(crate) fn not_running_message(action: &str, why: &str) -> String { /// Refuse, in a sentence, before a Docker error has to speak for us. /// /// Both file transfers and the backup run through `docker exec`, which needs a -/// running container. Without this the failure surfaces as bollard's +/// running container. Without this the failure surfaces as Docker's /// `is not running` wrapped in whatever the caller was doing, and for the /// upload it surfaces even less usefully: `resolve_container_dir`'s `realpath` /// is the first thing to touch the container, so a stopped project fails inside /// path *validation* and reads like the path was the problem. +/// +/// Takes a container id rather than a [`ContainerTarget`] because the file +/// viewer's commands call it with the id they already hold. pub(crate) async fn require_running(container_id: &str, action: &str) -> Result<(), String> { - let docker = get_docker()?; - let running = docker - .inspect_container(container_id, None) - .await - .ok() - .and_then(|info| info.state) - .and_then(|s| s.running) - .unwrap_or(false); + // An unreachable daemon is an error; an inspect failure (container gone) + // reads as "not running" — `is_running`'s contract, and what this did + // when it inspected the container itself. + let running = ContainerTarget::docker(container_id).is_running().await?; if running { return Ok(()); } @@ -1543,12 +1540,10 @@ pub async fn download_container_file( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "No container exists for this project yet — start it first".to_string())?; - require_running(container_id, "saving a file to the host").await?; + require_running(target.id(), "saving a file to the host").await?; let Some(chosen) = pick_save_path(&window, &suggested_save_name(&container_path)).await else { // Dismissed. Not an error, and deliberately distinguishable from one: @@ -1564,8 +1559,6 @@ pub async fn download_container_file( // costs. See the note on `pick_save_path` about which ones those are. let dest = resolve_host_path(&chosen, HostPathUse::WriteChosenName).await?; - let docker = get_docker()?; - // `$TC_SRC`, never argv interpolation: the path reaches the shell as an // environment value, so a name containing a quote or a `$` is data. // @@ -1627,30 +1620,19 @@ esac echo "TC_SIZE=$SZ" >&2 dd iflag=nonblock bs=64k status=none if="$TC_SRC" || exit 5"#; - let exec = docker - .create_exec( - container_id, - CreateExecOptions { - attach_stdout: Some(true), - attach_stderr: Some(true), - cmd: Some(vec!["sh".to_string(), "-c".to_string(), script.to_string()]), - env: Some(vec![format!("TC_SRC={}", container_path)]), - user: Some("claude".to_string()), - ..Default::default() - }, + // As `claude`, no TTY, no stdin and no working directory (the image's + // default) — the options this exec has always been created with. + let exec = target + .attach( + ExecSpec::new(vec!["sh".to_string(), "-c".to_string(), script.to_string()]) + .env(vec![format!("TC_SRC={}", container_path)]) + .working_dir(None) + .no_stdin(), ) .await - .map_err(|e| format!("Failed to start download: {}", e))?; - - let result = docker - .start_exec(&exec.id, None) - .await - .map_err(|e| format!("Failed to start download: {}", e))?; - - let mut output = match result { - StartExecResults::Attached { output, .. } => output, - StartExecResults::Detached => return Err("Download exec started detached".to_string()), - }; + .map_err(|e| format!("Failed to start download: {}", strip_exec_error_prefix(&e)))?; + let exec_id = exec.exec_id; + let mut output = exec.output; use tokio::io::AsyncWriteExt; let partial = partial_download_path(&dest)?; @@ -1696,7 +1678,7 @@ dd iflag=nonblock bs=64k status=none if="$TC_SRC" || exit 5"#; while let Some(msg) = output.next().await { match msg { - Ok(LogOutput::StdOut { message }) => { + Ok(ExecChunk::StdOut(message)) => { if let Err(e) = writer.write_all(&message).await { stream_err = Some(format!("Failed to write {}: {}", dest.display(), e)); break; @@ -1727,7 +1709,7 @@ dd iflag=nonblock bs=64k status=none if="$TC_SRC" || exit 5"#; } } } - Ok(LogOutput::StdErr { message }) => { + Ok(ExecChunk::StdErr(message)) => { push_capped(&mut stderr_text, &message); if declared.is_none() { declared = parse_declared_size(&stderr_text); @@ -1760,7 +1742,7 @@ dd iflag=nonblock bs=64k status=none if="$TC_SRC" || exit 5"#; // success renames a truncated partial over the file the user already had // and reports the byte count as if it were the whole thing. That is the one // way this command could destroy data, so silence is failure. - let exit_code = crate::docker::exec::wait_for_exec_exit(&exec.id).await; + let exit_code = target.wait_exec_exit(&exec_id).await; if stream_err.is_none() && exit_code != Some(0) { // Framed and clipped, never verbatim. // @@ -1894,7 +1876,7 @@ pub struct UploadOutcome { /// * the destination goes through [`resolve_container_dir`], so it is inside /// [`CONTAINER_WRITE_ROOTS`] both as written and as it resolves; /// * the read, the size ceiling and the descriptor check are -/// `docker::exec::upload_host_file_with_ids`', unchanged — including +/// `ContainerTarget::upload_host_file_with_ids`', unchanged — including /// landing the file owned by the container user rather than root. /// /// The dialog itself is opened from Rust ([`pick_files_to_upload`]), which is @@ -1911,15 +1893,13 @@ pub async fn upload_files_to_container( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "No container exists for this project yet — start it first".to_string())?; // Both checks before the dialog, so a project that cannot receive files // says so instead of asking the user to choose some first. - require_running(container_id, "uploading files").await?; - resolve_container_dir(container_id, "Upload", &container_dir).await?; + require_running(target.id(), "uploading files").await?; + resolve_container_dir(&target, "Upload", &container_dir).await?; let Some(picked) = pick_files_to_upload(&window).await else { return Ok(None); @@ -1930,14 +1910,14 @@ pub async fn upload_files_to_container( // files; this one is the check that actually guards the extraction. A modal // has no time limit, and `resolve_container_dir` answers a question about // the container's filesystem — which the container is free to change while - // the dialog is open. Without this, `docker::exec`'s doc comment claim that + // the dialog is open. Without this, the uploader's doc comment claim that // the destination "has already been confirmed" would be true only of a // moment that had passed. - resolve_container_dir(container_id, "Upload", &container_dir).await?; + resolve_container_dir(&target, "Upload", &container_dir).await?; // Once for the whole selection, not once per file — see // `upload_host_file_with_ids`. - let ids = crate::docker::exec::container_user_ids(container_id).await; + let ids = target.user_ids().await; let mut outcome = UploadOutcome { uploaded: Vec::new(), @@ -1951,7 +1931,7 @@ pub async fn upload_files_to_container( continue; } }; - match upload_one(container_id, &path, &container_dir, ids).await { + match upload_one(&target, &path, &container_dir, ids).await { Ok(dest) => { log::info!("Uploaded {} into project {} at {}", path, project_id, dest); outcome.uploaded.push(dest); @@ -1966,7 +1946,7 @@ pub async fn upload_files_to_container( /// naming the file, because the caller may be reporting several at once and /// "is a folder" on its own does not say which one. async fn upload_one( - container_id: &str, + target: &ContainerTarget, host_path: &str, container_dir: &str, ids: (u64, u64), @@ -1990,7 +1970,7 @@ async fn upload_one( // The ceiling is enforced against the open descriptor inside the uploader; // this copy of it exists so the refusal arrives as a sentence instead of // after a 300 MB read. - use crate::docker::exec::MAX_DROP_BYTES; + use crate::host::MAX_DROP_BYTES; if meta.len() > MAX_DROP_BYTES { return Err(format!( "{} is too large to upload ({:.0} MB; limit {} MB). Mount it into the project instead.", @@ -2000,14 +1980,9 @@ async fn upload_one( )); } - crate::docker::exec::upload_host_file_with_ids( - container_id, - &resolved, - container_dir, - &name, - ids, - ) - .await + target + .upload_host_file_with_ids(&resolved, container_dir, &name, ids) + .await } /// Container-authored diagnostic text, cut down to something safe to quote. @@ -2274,14 +2249,10 @@ pub async fn download_container_backup( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "No container exists for this project yet — start it first".to_string())?; - let docker = get_docker()?; - - require_running(container_id, "backing up").await?; + require_running(target.id(), "backing up").await?; let path = container_path.unwrap_or_else(|| "/workspace".to_string()); // Read-only source: `tar -C` it, so absoluteness and `..` are what matter. @@ -2326,33 +2297,25 @@ tar czf - --ignore-failed-read \ let cmd = vec!["sh".to_string(), "-c".to_string(), script.to_string()]; - let exec = docker - .create_exec( - container_id, - CreateExecOptions { - attach_stdout: Some(true), - attach_stderr: Some(true), - cmd: Some(cmd), - env: Some(vec![ + // As `claude`, no TTY, no stdin and no working directory (the image's + // default) — the options this exec has always been created with. + let exec = target + .attach( + ExecSpec::new(cmd) + .env(vec![ "HOME=/home/claude".to_string(), format!("TC_BACKUP_SRC={}", path), - ]), - user: Some("claude".to_string()), - ..Default::default() - }, + ]) + .working_dir(None) + .no_stdin(), ) .await - .map_err(|e| format!("Failed to create backup exec: {}", e))?; - - let result = docker - .start_exec(&exec.id, None) - .await - .map_err(|e| format!("Failed to start backup exec: {}", e))?; - - let mut output = match result { - StartExecResults::Attached { output, .. } => output, - StartExecResults::Detached => return Err("Backup exec started detached".to_string()), - }; + .map_err(|e| match e.strip_prefix("Failed to create exec: ") { + Some(rest) => format!("Failed to create backup exec: {}", rest), + None => format!("Failed to start backup exec: {}", strip_exec_error_prefix(&e)), + })?; + let exec_id = exec.exec_id; + let mut output = exec.output; use tokio::io::AsyncWriteExt; let partial = partial_download_path(&dest)?; @@ -2401,14 +2364,14 @@ tar czf - --ignore-failed-read \ while let Some(msg) = output.next().await { match msg { - Ok(LogOutput::StdOut { message }) => { + Ok(ExecChunk::StdOut(message)) => { if let Err(e) = writer.write_all(&message).await { stream_err = Some(format!("Failed to write backup file: {}", e)); break; } total += message.len() as u64; } - Ok(LogOutput::StdErr { message }) => { + Ok(ExecChunk::StdErr(message)) => { push_capped(&mut stderr_text, &message); } Ok(_) => {} @@ -2430,7 +2393,7 @@ tar czf - --ignore-failed-read \ // when `total > 0`. Poll until the exec actually reports finished so the // exit code is reliably populated; if it can't be determined we fall back to // the `total == 0` check below. - let exit_code = crate::docker::exec::wait_for_exec_exit(&exec.id).await; + let exit_code = target.wait_exec_exit(&exec_id).await; if stream_err.is_none() && exit_code.is_some_and(|c| c != 0) { stream_err = Some(format!( @@ -3564,10 +3527,11 @@ mod tests { // `/workspace/escape` is under a write root as a string and is `/etc` as // a location. - let escaped = resolve_container_dir(&name, "Folder", "/workspace/escape").await; + let target = ContainerTarget::docker(name.clone()); + let escaped = resolve_container_dir(&target, "Folder", "/workspace/escape").await; println!("container escape: {:?}", escaped); assert!(escaped.is_err(), "a symlink out of /workspace was accepted"); - resolve_container_dir(&name, "Folder", "/workspace") + resolve_container_dir(&target, "Folder", "/workspace") .await .expect("/workspace itself"); @@ -3575,7 +3539,7 @@ mod tests { // reporting itself empty — `-H` follows the start point, and only the // start point. let (records, diagnostics, code) = - exec_oneshot_streams_as(&name, "claude", list_argv("/workspace/link"), Vec::new()) + target.exec_oneshot_streams_as("claude", list_argv("/workspace/link"), Vec::new()) .await .unwrap(); println!("link: code={} diagnostics={:?}", code, diagnostics); @@ -3588,7 +3552,7 @@ mod tests { // The link itself is still *marked* as one in its parent's listing, and // still navigable. let (records, _, _) = - exec_oneshot_streams_as(&name, "claude", list_argv("/workspace"), Vec::new()) + target.exec_oneshot_streams_as("claude", list_argv("/workspace"), Vec::new()) .await .unwrap(); let parent = parse_find_output("/workspace", &records); @@ -3607,7 +3571,7 @@ mod tests { for path in ["/workspace/broken", "/workspace/loop"] { let started = std::time::Instant::now(); let (records, diagnostics, code) = - exec_oneshot_streams_as(&name, "claude", list_argv(path), Vec::new()) + target.exec_oneshot_streams_as("claude", list_argv(path), Vec::new()) .await .unwrap(); let took = started.elapsed(); diff --git a/app/src-tauri/src/commands/inspect_commands.rs b/app/src-tauri/src/commands/inspect_commands.rs index 0bb1059..a637fc6 100644 --- a/app/src-tauri/src/commands/inspect_commands.rs +++ b/app/src-tauri/src/commands/inspect_commands.rs @@ -12,8 +12,9 @@ //! //! ## Container access //! -//! All work happens inside the container via the existing `docker exec` -//! plumbing in [`crate::docker::exec`] — no second mechanism is introduced. +//! All work happens inside the container via the existing exec plumbing, +//! reached through [`crate::host::ContainerTarget`] — no second mechanism is +//! introduced. //! The heavy lifting (walking dirs, grepping transcripts, parsing JSON with //! `jq`) runs *in* the container and only a small JSON summary crosses the //! wire, so multi-megabyte transcripts are never streamed back. @@ -56,14 +57,11 @@ //! commands fail loudly, since they cannot do anything useful without a //! running container. -use bollard::exec::{CreateExecOptions, StartExecOptions}; use serde::{Deserialize, Serialize}; use tauri::State; -use crate::docker::client::get_docker; -use crate::docker::container::is_container_running; -use crate::docker::exec::{exec_oneshot_env, exec_oneshot_env_status}; use crate::docker::CONTAINER_HOME; +use crate::host::{ContainerTarget, ExecSpec}; use crate::AppState; /// Newest N session transcripts to inspect. Caps the work done inside the @@ -192,7 +190,7 @@ pub struct SchedulerNotification { // Shared helpers // ───────────────────────────────────────────────────────────────────────────── -/// Resolve a project to a *running* container id. +/// Resolve a project to its *running* container. /// /// `Ok(None)` means "there is nothing to inspect" — no container recorded, or /// the container exists but is stopped. Callers that are read-only turn that @@ -201,19 +199,19 @@ pub struct SchedulerNotification { async fn running_container_for( project_id: &str, state: &State<'_, AppState>, -) -> Result, String> { +) -> Result, String> { let project = state .projects_store .get(project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = match project.container_id { - Some(id) => id, + let target = match ContainerTarget::for_project(&project) { + Some(t) => t, None => return Ok(None), }; - if is_container_running(&container_id).await.unwrap_or(false) { - Ok(Some(container_id)) + if target.is_running().await.unwrap_or(false) { + Ok(Some(target)) } else { Ok(None) } @@ -224,7 +222,7 @@ async fn running_container_for( async fn require_running_container( project_id: &str, state: &State<'_, AppState>, -) -> Result { +) -> Result { running_container_for(project_id, state).await?.ok_or_else(|| { "Container is not running — start the project first.".to_string() }) @@ -240,13 +238,13 @@ fn home_env() -> Vec { /// the first line) so the combined stream `exec_oneshot_env` returns is pure /// stdout and stays parseable as JSON. A non-zero exit therefore surfaces as /// empty output, which the callers treat as "nothing to report". -async fn run_script(container_id: &str, script: impl Into) -> Result { - exec_oneshot_env( - container_id, - vec!["sh".to_string(), "-c".to_string(), script.into()], - home_env(), - ) - .await +async fn run_script(target: &ContainerTarget, script: impl Into) -> Result { + target + .exec_oneshot_env( + vec!["sh".to_string(), "-c".to_string(), script.into()], + home_env(), + ) + .await } /// Parse script output as JSON, degrading to a default value (and a log line) @@ -315,11 +313,11 @@ pub(crate) fn validate_session_id(id: &str) -> Result<(), String> { /// Run `triple-c-scheduler ` as a bare argv vector — no shell is /// involved, so caller-supplied ids cannot be interpreted as shell syntax. /// Returns the combined output, erroring with it on a non-zero exit. -async fn run_scheduler(container_id: &str, args: Vec) -> Result { +async fn run_scheduler(target: &ContainerTarget, args: Vec) -> Result { let mut cmd = vec!["triple-c-scheduler".to_string()]; cmd.extend(args); - let (output, exit_code) = exec_oneshot_env_status(container_id, cmd, home_env()).await?; + let (output, exit_code) = target.exec_oneshot_env_status(cmd, home_env()).await?; if exit_code != 0 { let detail = output.trim(); return Err(if detail.is_empty() { @@ -395,8 +393,8 @@ pub async fn list_claude_sessions( project_id: String, state: State<'_, AppState>, ) -> Result, String> { - let container_id = match running_container_for(&project_id, &state).await? { - Some(id) => id, + let target = match running_container_for(&project_id, &state).await? { + Some(t) => t, None => return Ok(Vec::new()), }; @@ -404,7 +402,7 @@ pub async fn list_claude_sessions( // a const usize — never from the caller. let script = SESSIONS_SCRIPT.replace("__MAX__", &MAX_SESSIONS.to_string()); - let raw = run_script(&container_id, script).await?; + let raw = run_script(&target, script).await?; let sessions: Vec = parse_or_default(&raw, "session list"); Ok(sessions @@ -566,12 +564,12 @@ pub async fn list_container_capabilities( project_id: String, state: State<'_, AppState>, ) -> Result { - let container_id = match running_container_for(&project_id, &state).await? { - Some(id) => id, + let target = match running_container_for(&project_id, &state).await? { + Some(t) => t, None => return Ok(ContainerCapabilities::default()), }; - let raw = run_script(&container_id, CAPABILITIES_SCRIPT).await?; + let raw = run_script(&target, CAPABILITIES_SCRIPT).await?; Ok(parse_or_default(&raw, "container capabilities")) } @@ -681,12 +679,12 @@ pub async fn list_scheduled_tasks( project_id: String, state: State<'_, AppState>, ) -> Result, String> { - let container_id = match running_container_for(&project_id, &state).await? { - Some(id) => id, + let target = match running_container_for(&project_id, &state).await? { + Some(t) => t, None => return Ok(Vec::new()), }; - let raw = run_script(&container_id, SCHEDULER_LIST_SCRIPT).await?; + let raw = run_script(&target, SCHEDULER_LIST_SCRIPT).await?; let tasks: Vec = parse_or_default(&raw, "scheduled tasks"); Ok(tasks @@ -731,13 +729,13 @@ pub async fn get_scheduled_task_log( // Clamped, and an integer by type — cannot carry shell syntax. let tail = tail_lines.unwrap_or(200).clamp(1, 5000); - let container_id = match running_container_for(&project_id, &state).await? { - Some(id) => id, + let target = match running_container_for(&project_id, &state).await? { + Some(t) => t, None => return Ok(String::new()), }; run_scheduler( - &container_id, + &target, vec![ "logs".to_string(), "--id".to_string(), @@ -756,15 +754,15 @@ pub async fn get_scheduler_notifications( project_id: String, state: State<'_, AppState>, ) -> Result, String> { - let container_id = match running_container_for(&project_id, &state).await? { - Some(id) => id, + let target = match running_container_for(&project_id, &state).await? { + Some(t) => t, None => return Ok(Vec::new()), }; let script = SCHEDULER_NOTIFICATIONS_SCRIPT.replace("__MAX__", &MAX_NOTIFICATIONS.to_string()); - let raw = run_script(&container_id, script).await?; + let raw = run_script(&target, script).await?; let notifications: Vec = parse_or_default(&raw, "scheduler notifications"); Ok(notifications @@ -1140,9 +1138,9 @@ pub async fn add_scheduled_task( &schedule, working_dir.as_deref(), )?; - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; - let output = run_scheduler(&container_id, input.add_args()).await?; + let output = run_scheduler(&target, input.add_args()).await?; let task_id = parse_created_task_id(&output).ok_or_else(|| { format!( "The scheduler did not report a task id. Its output was: {}", @@ -1200,9 +1198,9 @@ pub async fn update_scheduled_task( &schedule, working_dir.as_deref(), )?; - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; - let output = run_scheduler(&container_id, input.add_args()).await?; + let output = run_scheduler(&target, input.add_args()).await?; let new_id = parse_created_task_id(&output).ok_or_else(|| { format!( "The scheduler did not report a task id, so the original task was left in place. Its output was: {}", @@ -1211,7 +1209,7 @@ pub async fn update_scheduled_task( })?; run_scheduler( - &container_id, + &target, vec!["remove".to_string(), "--id".to_string(), task_id.clone()], ) .await @@ -1224,7 +1222,7 @@ pub async fn update_scheduled_task( if enabled == Some(false) { if let Err(e) = run_scheduler( - &container_id, + &target, vec!["disable".to_string(), "--id".to_string(), new_id.clone()], ) .await @@ -1253,11 +1251,11 @@ pub async fn set_scheduled_task_enabled( state: State<'_, AppState>, ) -> Result { validate_task_id(&task_id)?; - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; let subcommand = if enabled { "enable" } else { "disable" }; let output = run_scheduler( - &container_id, + &target, vec![subcommand.to_string(), "--id".to_string(), task_id], ) .await?; @@ -1277,47 +1275,32 @@ pub async fn run_scheduled_task_now( state: State<'_, AppState>, ) -> Result { validate_task_id(&task_id)?; - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; - let docker = get_docker()?; - let exec = docker - .create_exec( - &container_id, - CreateExecOptions { - attach_stdout: Some(false), - attach_stderr: Some(false), - // Argv vector — no shell, so `task_id` is inert as data. - cmd: Some(vec![ - "triple-c-scheduler".to_string(), - "run".to_string(), - "--id".to_string(), - task_id.clone(), - ]), - env: Some(home_env()), - user: Some("claude".to_string()), - working_dir: Some("/workspace".to_string()), - ..Default::default() - }, - ) - .await - .map_err(|e| format!("Failed to create exec: {}", e))?; - - docker - .start_exec( - &exec.id, - Some(StartExecOptions { - detach: true, - ..Default::default() - }), - ) - .await - .map_err(|e| format!("Failed to start task: {}", e))?; + // Argv vector — no shell, so `task_id` is inert as data. Stdin, stdout and + // stderr are all left unattached; the exec is started detached. + let spec = ExecSpec::new(vec![ + "triple-c-scheduler".to_string(), + "run".to_string(), + "--id".to_string(), + task_id.clone(), + ]) + .env(home_env()) + .no_stdin(); + let exec_id = target.exec_detached(spec).await.map_err(|e| { + // Keep this command's own wording for a start failure ("Failed to start + // task: …"); a create failure reads "Failed to create exec: …" either way. + match e.strip_prefix("Failed to start exec: ") { + Some(rest) => format!("Failed to start task: {}", rest), + None => e, + } + })?; log::info!( "Triggered scheduler task {} in project {} (detached exec {})", task_id, project_id, - exec.id + exec_id ); Ok(format!("Task {} started.", task_id)) } @@ -1330,10 +1313,10 @@ pub async fn remove_scheduled_task( state: State<'_, AppState>, ) -> Result { validate_task_id(&task_id)?; - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; let output = run_scheduler( - &container_id, + &target, vec!["remove".to_string(), "--id".to_string(), task_id], ) .await?; @@ -1346,9 +1329,9 @@ pub async fn clear_scheduler_notifications( project_id: String, state: State<'_, AppState>, ) -> Result<(), String> { - let container_id = require_running_container(&project_id, &state).await?; + let target = require_running_container(&project_id, &state).await?; run_scheduler( - &container_id, + &target, vec!["notifications".to_string(), "--clear".to_string()], ) .await?; -- 2.52.0 From 229dc2cefc786ec0fdbf2daf3b26dfdb91245b97 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 13 Nov 2026 10:30:52 -0800 Subject: [PATCH 4/5] Route auth bridge, browser view and terminal commands through ContainerTarget Phase 1a of the AWS host work: these files no longer name bollard or docker::exec; every exec, upload and is-running check goes through host::ContainerTarget. No behaviour change: same user, working dir, env, tty and stdin attachment for every exec. - auth_bridge: the poller, reconcile and PortForward hold a ContainerTarget (AuthBridgeManager::start keeps its container_id signature). tunnel_connection_with_prelude keeps its String signature for the browser-view proxy and wraps the new tunnel_into_with_prelude. - browser_view: the supervisor and viewer helpers take a ContainerTarget; detect::detect_in is the target-taking form of detect; install.rs's run_step uses ContainerTarget::attach with an ExecSpec equal to the old create_attached_exec_as call. page.rs builds the target at each exec. - terminal_commands: open_terminal_session uses ContainerTarget::for_project; session-derived calls build ContainerTarget::docker from the session's id. Co-Authored-By: Claude Opus 5.5 --- app/src-tauri/src/auth_bridge/mod.rs | 27 +++-- app/src-tauri/src/auth_bridge/tunnel.rs | 49 ++++++-- app/src-tauri/src/browser_view/detect.rs | 19 ++- app/src-tauri/src/browser_view/install.rs | 77 +++++++------ app/src-tauri/src/browser_view/mod.rs | 109 +++++++++--------- app/src-tauri/src/browser_view/page.rs | 50 ++++---- .../src/commands/terminal_commands.rs | 47 ++++---- 7 files changed, 209 insertions(+), 169 deletions(-) diff --git a/app/src-tauri/src/auth_bridge/mod.rs b/app/src-tauri/src/auth_bridge/mod.rs index 88a0824..cd55876 100644 --- a/app/src-tauri/src/auth_bridge/mod.rs +++ b/app/src-tauri/src/auth_bridge/mod.rs @@ -32,7 +32,7 @@ //! |---|---| //! | Bridge disabled | `set_auth_bridge_enabled(false)` → [`AuthBridgeManager::stop`] | //! | Container stopped via UI | `stop_project_container` → [`AuthBridgeManager::stop`] | -//! | Container stopped/died another way | poller's own `is_container_running` check → loop exits | +//! | Container stopped/died another way | poller's own `is_running` check → loop exits | //! | Project deleted | `remove_project` → [`AuthBridgeManager::stop`]; also the poller's `store.get()` check | //! | Container rebuilt | `rebuild_project_container` → stop, then start re-arms it | //! | App exit | window `CloseRequested` → [`AuthBridgeManager::stop_all`] | @@ -55,8 +55,7 @@ use tauri::{AppHandle, Emitter}; use tokio::sync::{watch, Mutex}; use tokio::task::JoinHandle; -use crate::docker::container::is_container_running; -use crate::docker::exec::{exec_oneshot_limited, PROC_NET_OUTPUT_LIMIT}; +use crate::host::{ContainerTarget, PROC_NET_OUTPUT_LIMIT}; use crate::storage::projects_store::ProjectsStore; use proc_net::PortFamily; @@ -205,7 +204,7 @@ impl AuthBridgeManager { let poller = tokio::spawn(poll_loop( project_id.clone(), - container_id, + ContainerTarget::docker(container_id), epoch, app, store, @@ -276,7 +275,7 @@ impl AuthBridgeManager { #[allow(clippy::too_many_arguments)] async fn poll_loop( project_id: String, - container_id: String, + container: ContainerTarget, epoch: u64, app: AppHandle, store: Arc, @@ -301,7 +300,7 @@ async fn poll_loop( log::info!("Auth bridge: disabled for project {} — tearing down", project_id); break; } - if !is_container_running(&container_id).await.unwrap_or(false) { + if !container.is_running().await.unwrap_or(false) { log::info!( "Auth bridge: container for project {} is no longer running — tearing down", project_id @@ -327,7 +326,7 @@ async fn poll_loop( // bridge or stopping the container doesn't wait out an in-flight poll. let discovery = tokio::select! { _ = cancel.changed() => break, - res = exec_oneshot_limited(&container_id, "claude", cmd, PROC_NET_OUTPUT_LIMIT) => { + res = container.exec_oneshot_limited("claude", cmd, PROC_NET_OUTPUT_LIMIT) => { res.map(|(out, _)| out) } }; @@ -340,7 +339,7 @@ async fn poll_loop( // gateway/STT/web-terminal ports can be re-pointed while the // bridge is running, and a stale reservation set is a hole. let skip = skipped_ports(&project, &store.list(), &app_settings(&app)); - if reconcile(&container_id, &discovered, &skip, &state).await { + if reconcile(&container, &discovered, &skip, &state).await { emit_status(&app, &project_id, &state, true).await; } } @@ -530,7 +529,7 @@ const MAX_CONFLICTS: usize = 32; /// Bring the set of host listeners in line with what the container is currently /// listening on. Returns whether anything the UI cares about changed. async fn reconcile( - container_id: &str, + container: &ContainerTarget, discovered: &BTreeMap, skip: &HashSet, state: &Arc>, @@ -583,7 +582,7 @@ async fn reconcile( ); continue; } - match PortForward::bind(container_id.to_string(), port, family).await { + match PortForward::bind(container.clone(), port, family).await { Ok(forward) => { if st.conflicts.remove(&port).is_some() { log::info!("Auth bridge: host port {} became available", port); @@ -833,7 +832,13 @@ mod tests { (45000u16..45200).map(|p| (p, PortFamily::V4)).collect(); let state = Arc::new(Mutex::new(BridgeState::default())); - reconcile("no-such-container", &discovered, &HashSet::new(), &state).await; + reconcile( + &ContainerTarget::docker("no-such-container"), + &discovered, + &HashSet::new(), + &state, + ) + .await; let mut st = state.lock().await; assert!( diff --git a/app/src-tauri/src/auth_bridge/tunnel.rs b/app/src-tauri/src/auth_bridge/tunnel.rs index 9f353c5..d40588d 100644 --- a/app/src-tauri/src/auth_bridge/tunnel.rs +++ b/app/src-tauri/src/auth_bridge/tunnel.rs @@ -11,7 +11,7 @@ //! and stdout wired to the TCP socket. `socat` ships in the container image. //! //! The exec plumbing itself is *not* reimplemented here: it comes from -//! [`crate::docker::exec::create_attached_exec`], the same helper the +//! [`crate::host::ContainerTarget::attach_exec`], the same helper the //! interactive terminal sessions are built on. //! //! ## What the host listener is, and is not @@ -56,14 +56,12 @@ use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::time::Duration; -use crate::host::ExecChunk; +use crate::host::{AttachedExec, ContainerTarget, ExecChunk}; use futures_util::StreamExt; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; use tokio::task::{JoinHandle, JoinSet}; -use crate::docker::exec::{create_attached_exec, AttachedExec}; - use super::proc_net::PortFamily; /// Buffer size for the host→container direction. OAuth callbacks are tiny; this @@ -143,13 +141,13 @@ impl Drop for PortForward { } impl PortForward { - /// Bind `port` on the host loopback and start proxying into `container_id`. + /// Bind `port` on the host loopback and start proxying into `container`. /// /// The bind happens before the task is spawned, so an already-taken port is /// reported to the caller as an error rather than disappearing into a /// background task. pub async fn bind( - container_id: String, + container: ContainerTarget, port: u16, family: PortFamily, ) -> Result { @@ -196,7 +194,7 @@ impl PortForward { }; let target = family.socat_target(port); - let task = tokio::spawn(accept_loop(container_id, port, target, v4, v6)); + let task = tokio::spawn(accept_loop(container, port, target, v4, v6)); Ok(Self { port, @@ -219,7 +217,7 @@ impl PortForward { /// Accept on both loopback listeners until aborted. Dropping this future drops /// the listeners (freeing the port) and the `JoinSet` (aborting live tunnels). async fn accept_loop( - container_id: String, + container: ContainerTarget, port: u16, target: String, v4: TcpListener, @@ -258,7 +256,7 @@ async fn accept_loop( log::debug!("Auth bridge: connection from {} to bridged port {}", peer, port); let _ = stream.set_nodelay(true); conns.spawn(tunnel_connection( - container_id.clone(), + container.clone(), target.clone(), stream, port, @@ -288,7 +286,12 @@ async fn accept_optional( /// /// Nothing is forwarded until that decision is made, so a refused request never /// reaches the container at all — not even a `docker exec`. -async fn tunnel_connection(container_id: String, target: String, mut stream: TcpStream, port: u16) { +async fn tunnel_connection( + container: ContainerTarget, + target: String, + mut stream: TcpStream, + port: u16, +) { let head = match read_leading_bytes(&mut stream).await { Ok(head) => head, Err(e) => { @@ -319,7 +322,7 @@ async fn tunnel_connection(container_id: String, target: String, mut stream: Tcp } // The bytes already off the socket go back on the wire first, byte-exact. - tunnel_connection_with_prelude(container_id, target, stream, port, head.into_buffer()).await + tunnel_into_with_prelude(container, target, stream, port, head.into_buffer()).await } /// What the first bytes of an accepted connection turned out to be. @@ -511,12 +514,34 @@ pub(crate) fn web_request_verdict(head: &str) -> Verdict { /// head off the socket to check a token, and then has to put those same bytes /// back on the wire. Passing them here keeps the byte stream exact, rather than /// re-serialising a parsed request. +/// +/// Takes a Docker container id because that is what the browser-view proxy +/// holds; [`tunnel_into_with_prelude`] is the same thing for a +/// [`ContainerTarget`] already in hand. pub async fn tunnel_connection_with_prelude( container_id: String, target: String, stream: TcpStream, port: u16, prelude: Vec, +) { + tunnel_into_with_prelude( + ContainerTarget::docker(container_id), + target, + stream, + port, + prelude, + ) + .await +} + +/// [`tunnel_connection_with_prelude`] for a [`ContainerTarget`]. +pub async fn tunnel_into_with_prelude( + container: ContainerTarget, + target: String, + stream: TcpStream, + port: u16, + prelude: Vec, ) { let cmd = vec!["socat".to_string(), "-".to_string(), target.clone()]; @@ -524,7 +549,7 @@ pub async fn tunnel_connection_with_prelude( mut output, mut input, .. - } = match create_attached_exec(&container_id, cmd, false).await { + } = match container.attach_exec(cmd, false).await { Ok(e) => e, Err(e) => { log::warn!( diff --git a/app/src-tauri/src/browser_view/detect.rs b/app/src-tauri/src/browser_view/detect.rs index 241eac9..eb88a06 100644 --- a/app/src-tauri/src/browser_view/detect.rs +++ b/app/src-tauri/src/browser_view/detect.rs @@ -36,7 +36,7 @@ use serde::{Deserialize, Serialize}; -use crate::docker::exec::exec_oneshot; +use crate::host::ContainerTarget; /// Marks the JSON payload in the probe's stdout, so unrelated chatter on the /// same stream (npm notices, Node warnings) can't be mistaken for the result. @@ -278,11 +278,18 @@ impl PlaywrightDetection { /// `npx @playwright/mcp`, is found as readily as one in /// `/workspace/node_modules`. pub async fn detect(container_id: &str) -> Result { - let output = exec_oneshot( - container_id, - vec!["node".to_string(), "-e".to_string(), PROBE.to_string()], - ) - .await?; + detect_in(&ContainerTarget::docker(container_id)).await +} + +/// [`detect`] against a target already in hand. +pub async fn detect_in(container: &ContainerTarget) -> Result { + let output = container + .exec_oneshot(vec![ + "node".to_string(), + "-e".to_string(), + PROBE.to_string(), + ]) + .await?; parse_probe_output(&output) } diff --git a/app/src-tauri/src/browser_view/install.rs b/app/src-tauri/src/browser_view/install.rs index 87aff0e..a6c149d 100644 --- a/app/src-tauri/src/browser_view/install.rs +++ b/app/src-tauri/src/browser_view/install.rs @@ -67,9 +67,7 @@ use serde::Serialize; use tauri::AppHandle; use crate::commands::project_commands::emit_progress; -use crate::docker::exec::{ - create_attached_exec_as, exec_oneshot_as, wait_for_exec_exit, AttachedExec, -}; +use crate::host::{AttachedExec, ContainerTarget, ExecSpec}; use super::detect::{self, PlaywrightDetection}; @@ -242,13 +240,14 @@ pub async fn install_packages( project_id: &str, container_id: &str, ) -> Result { + let container = ContainerTarget::docker(container_id); emit_progress( app, project_id, &format!("Installing @playwright/cli into {}/node_modules…", INSTALL_DIR), ); - let mut step = npm_install(app, project_id, container_id, &[VIEWER_PACKAGE]).await?; + let mut step = npm_install(app, project_id, &container, &[VIEWER_PACKAGE]).await?; if step.exit_code != 0 { return Err(format!( "npm couldn't install the viewer package in this container (exit {}).\n\nnpm said:\n{}", @@ -265,9 +264,9 @@ pub async fn install_packages( // see the note on `npm_install`. The pin can only be read after the first // install has written the manifest, which is why this stays two commands // rather than one. - let spec = pinned_playwright_spec(container_id).await; + let spec = pinned_playwright_spec(&container).await; emit_progress(app, project_id, &format!("Installing {}…", spec)); - let second = npm_install(app, project_id, container_id, &[VIEWER_PACKAGE, &spec]).await?; + let second = npm_install(app, project_id, &container, &[VIEWER_PACKAGE, &spec]).await?; if second.exit_code != 0 { return Err(format!( "npm couldn't install {} in this container (exit {}).\n\nnpm said:\n{}", @@ -279,7 +278,7 @@ pub async fn install_packages( step.log = merge_logs(step.log, second.log); emit_progress(app, project_id, "Re-checking what the container has…"); - let detection = detect::detect(container_id).await?; + let detection = detect::detect_in(&container).await?; // A probe that still finds something missing after a successful npm run is // the interesting case, so it is surfaced rather than swallowed. And a @@ -331,7 +330,7 @@ pub async fn install_packages( async fn npm_install( app: &AppHandle, project_id: &str, - container_id: &str, + container: &ContainerTarget, specs: &[&str], ) -> Result { let mut cmd = vec![ @@ -348,7 +347,7 @@ async fn npm_install( run_step( app, project_id, - container_id, + container, "claude", INSTALL_DIR, cmd, @@ -363,20 +362,20 @@ async fn npm_install( /// Read from the manifest npm just wrote rather than guessed, and falling back /// to `@latest` when it can't be read — an unreadable manifest is a reason to /// install something, not nothing. -async fn pinned_playwright_spec(container_id: &str) -> String { +async fn pinned_playwright_spec(container: &ContainerTarget) -> String { let script = format!( "try{{const d=require('{}/node_modules/@playwright/cli/package.json').dependencies||{{}};\ process.stdout.write(d.playwright||'');}}catch(e){{}}", INSTALL_DIR ); - let (out, _code) = exec_oneshot_as( - container_id, - "claude", - vec!["node".to_string(), "-e".to_string(), script], - Vec::new(), - ) - .await - .unwrap_or_default(); + let (out, _code) = container + .exec_oneshot_as( + "claude", + vec!["node".to_string(), "-e".to_string(), script], + Vec::new(), + ) + .await + .unwrap_or_default(); let version: &str = out.trim(); // A version, not a range or a URL: anything else goes to the fallback @@ -414,7 +413,8 @@ pub async fn install_browser( container_id: &str, target: BrowserTarget, ) -> Result { - let before = detect::detect(container_id).await?; + let container = ContainerTarget::docker(container_id); + let before = detect::detect_in(&container).await?; let Some(cli) = before.playwright_cli.clone() else { return Err( "Playwright isn't installed in this container yet, so there is nothing to install a \ @@ -458,7 +458,7 @@ pub async fn install_browser( project_id, "Step 1/3 — checking whether this container already has the browser system libraries…", ); - let state = check_libraries(container_id, &cli, target).await; + let state = check_libraries(&container, &cli, target).await; match &state { LibraryState::Present => { emit_progress( @@ -482,7 +482,7 @@ pub async fn install_browser( let deps = run_step( app, project_id, - container_id, + &container, "root", "/tmp", vec![ @@ -530,7 +530,7 @@ pub async fn install_browser( let dl = run_step( app, project_id, - container_id, + &container, user, workdir, vec![ @@ -559,7 +559,7 @@ pub async fn install_browser( project_id, &format!("Step 3/3 — checking that {} actually launches…", target.cli_name()), ); - let verdict = verify_launch(container_id, &before, target).await; + let verdict = verify_launch(&container, &before, target).await; push_section(&mut log, &verdict.detail); if !verdict.ok { warning = merge( @@ -590,7 +590,7 @@ pub async fn install_browser( } emit_progress(app, project_id, "Re-checking what the container has…"); - let detection = detect::detect(container_id).await?; + let detection = detect::detect_in(&container).await?; Ok(BrowserSetupOutcome { detection, @@ -642,9 +642,12 @@ impl LibraryState { /// dpkg's status file. A *missing* package on such an image is simply not in /// any index, so apt fails, Playwright reports the failure, and this returns /// [`LibraryState::Unknown`] — which installs, which is the right answer. -async fn check_libraries(container_id: &str, cli: &str, target: BrowserTarget) -> LibraryState { - let run = exec_oneshot_as( - container_id, +async fn check_libraries( + container: &ContainerTarget, + cli: &str, + target: BrowserTarget, +) -> LibraryState { + let run = container.exec_oneshot_as( "root", vec![ "node".to_string(), @@ -708,7 +711,7 @@ struct LaunchVerdict { /// Launch the browser headless, load one page, and close it. Seconds, and it is /// the only thing that distinguishes "downloaded" from "usable". async fn verify_launch( - container_id: &str, + container: &ContainerTarget, detection: &PlaywrightDetection, target: BrowserTarget, ) -> LaunchVerdict { @@ -724,8 +727,7 @@ async fn verify_launch( }; let dir = manifest.trim_end_matches("package.json").trim_end_matches('/'); - let run = exec_oneshot_as( - container_id, + let run = container.exec_oneshot_as( "claude", vec![ "node".to_string(), @@ -884,13 +886,13 @@ impl StepResult { /// Run one command in the container, streaming every line it prints to the pane /// as `container-progress` and keeping the tail for the result. /// -/// Uses `create_attached_exec_as` — the single attached-exec path — rather than +/// Uses [`ContainerTarget::attach`] — the single attached-exec path — rather than /// `exec_oneshot`, because these commands run for minutes and the point is that /// the user can watch them. async fn run_step( app: &AppHandle, project_id: &str, - container_id: &str, + container: &ContainerTarget, user: &str, workdir: &str, cmd: Vec, @@ -900,7 +902,14 @@ async fn run_step( exec_id, mut output, input, - } = create_attached_exec_as(container_id, cmd, false, user, workdir).await?; + } = container + .attach( + ExecSpec::new(cmd) + .tty(false) + .user(user) + .working_dir(Some(workdir)), + ) + .await?; // Nothing is ever written to this exec. Closing stdin means anything that // would prompt sees EOF and gives up, instead of waiting for a person who @@ -944,7 +953,7 @@ async fn run_step( )), Ok(Err(e)) => Err(e), Ok(Ok(())) => Ok(StepResult { - exit_code: wait_for_exec_exit(&exec_id).await.unwrap_or(0), + exit_code: container.wait_exec_exit(&exec_id).await.unwrap_or(0), log, }), } diff --git a/app/src-tauri/src/browser_view/mod.rs b/app/src-tauri/src/browser_view/mod.rs index dd35dcb..374acb3 100644 --- a/app/src-tauri/src/browser_view/mod.rs +++ b/app/src-tauri/src/browser_view/mod.rs @@ -50,7 +50,7 @@ //! | Trigger | Path | //! |---|---| //! | Turned off in the UI | `set_browser_view_enabled(false)` → persist `false`, then [`BrowserViewManager::stop`] | -//! | Container stopped, by the UI or otherwise | supervisor's `is_container_running` check | +//! | Container stopped, by the UI or otherwise | supervisor's `is_running` check | //! | Project deleted | supervisor's `store.get()` check | //! | Container rebuilt | old container stops → supervisor exits; the new one is not auto-started | //! | Viewer died in the container | supervisor's periodic HTTP liveness probe | @@ -90,8 +90,7 @@ use tokio::sync::{watch, Mutex}; use tokio::task::JoinHandle; use crate::auth_bridge::proc_net::{self, PortFamily}; -use crate::docker::container::is_container_running; -use crate::docker::exec::exec_oneshot; +use crate::host::ContainerTarget; use crate::storage::projects_store::ProjectsStore; use detect::PlaywrightDetection; @@ -300,7 +299,8 @@ impl BrowserViewManager { return Ok(self.status(&project_id, true).await); } - let detection = detect::detect(&container_id).await?; + let container = ContainerTarget::docker(container_id.as_str()); + let detection = detect::detect_in(&container).await?; if !detection.is_usable() { let blocker = detection.blocker().unwrap_or_else(|| { "Playwright is present but incomplete in this container.".to_string() @@ -320,9 +320,9 @@ impl BrowserViewManager { // Doing this before choosing a port also frees the one a previous // session was using, so sessions don't walk up the range. Best-effort: // a container with no dashboard makes this a no-op. - let _ = kill_dashboard(&container_id, &cli_entry).await; + let _ = kill_dashboard(&container, &cli_entry).await; - let (container_port, entry_path) = start_viewer(&container_id, &cli_entry).await?; + let (container_port, entry_path) = start_viewer(&container, &cli_entry).await?; let token = generate_token(); // `--host 127.0.0.1` is ours to set, so the family is known and there is @@ -337,7 +337,7 @@ impl BrowserViewManager { { Ok(p) => p, Err(e) => { - let _ = kill_dashboard(&container_id, &cli_entry).await; + let _ = kill_dashboard(&container, &cli_entry).await; return Err(e); } }; @@ -354,7 +354,7 @@ impl BrowserViewManager { let (cancel_tx, cancel_rx) = watch::channel(false); let supervisor = tokio::spawn(supervise( project_id.clone(), - container_id.clone(), + container, cli_entry, container_port, epoch, @@ -427,7 +427,7 @@ impl BrowserViewManager { #[allow(clippy::too_many_arguments)] async fn supervise( project_id: String, - container_id: String, + container: ContainerTarget, cli_entry: String, container_port: u16, epoch: u64, @@ -443,7 +443,7 @@ async fn supervise( log::info!("Browser view: project {} is gone — tearing down", project_id); break; } - if !is_container_running(&container_id).await.unwrap_or(false) { + if !container.is_running().await.unwrap_or(false) { log::info!( "Browser view: container for project {} is no longer running — tearing down", project_id @@ -459,7 +459,7 @@ async fn supervise( // view never waits out an in-flight exec. let alive = tokio::select! { _ = cancel.changed() => break, - res = probe_entry_path(&container_id, container_port) => res.is_ok(), + res = probe_entry_path(&container, container_port) => res.is_ok(), }; if !alive { log::warn!( @@ -477,7 +477,7 @@ async fn supervise( } proxy.shutdown().await; - let _ = kill_dashboard(&container_id, &cli_entry).await; + let _ = kill_dashboard(&container, &cli_entry).await; // Deregister, unless a newer session has already taken this project's slot. let superseded = { @@ -538,7 +538,11 @@ const VIEWER_LOG: &str = "/tmp/triple-c-browser-view.log"; /// redirected to [`VIEWER_LOG`] (both so `exec_oneshot` can return immediately /// rather than waiting on an inherited stdout, and so a failure has a trail), /// and readiness is established by [`wait_until_ready`] instead. -async fn launch_viewer(container_id: &str, cli_entry: &str, port: u16) -> Result<(), String> { +async fn launch_viewer( + container: &ContainerTarget, + cli_entry: &str, + port: u16, +) -> Result<(), String> { // `NO_UPDATE_NOTIFIER` stops the CLI phoning registry.npmjs.org on every // launch; the container may have no egress, and we don't want to wait out a // DNS timeout before the dashboard binds. @@ -549,13 +553,11 @@ async fn launch_viewer(container_id: &str, cli_entry: &str, port: u16) -> Result port, VIEWER_LOG ); - exec_oneshot( - container_id, - vec!["sh".to_string(), "-c".to_string(), script], - ) - .await - .map(|_| ()) - .map_err(|e| format!("Could not start the Playwright viewer: {}", e)) + container + .exec_oneshot(vec!["sh".to_string(), "-c".to_string(), script]) + .await + .map(|_| ()) + .map_err(|e| format!("Could not start the Playwright viewer: {}", e)) } /// The dashboard singleton is keyed on a hash of the working directory, so @@ -567,17 +569,15 @@ async fn launch_viewer(container_id: &str, cli_entry: &str, port: u16) -> Result const WORKDIR_PREFIX: &str = "cd /workspace 2>/dev/null || true"; /// Stop the dashboard daemon. Verified to free the port and stop answering. -async fn kill_dashboard(container_id: &str, cli_entry: &str) -> Result { +async fn kill_dashboard(container: &ContainerTarget, cli_entry: &str) -> Result { let script = format!( "{}; NO_UPDATE_NOTIFIER=1 node {} show --kill", WORKDIR_PREFIX, shell_quote(cli_entry) ); - exec_oneshot( - container_id, - vec!["sh".to_string(), "-c".to_string(), script], - ) - .await + container + .exec_oneshot(vec!["sh".to_string(), "-c".to_string(), script]) + .await } /// Turn a failed start into something the user can act on. @@ -604,13 +604,16 @@ fn explain_start_failure(err: &str, log: &str) -> String { } /// Tail of the viewer's own output, for a start that didn't come up. -async fn read_viewer_log(container_id: &str) -> String { - exec_oneshot( - container_id, - vec!["tail".to_string(), "-n".to_string(), "40".to_string(), VIEWER_LOG.to_string()], - ) - .await - .unwrap_or_default() +async fn read_viewer_log(container: &ContainerTarget) -> String { + container + .exec_oneshot(vec![ + "tail".to_string(), + "-n".to_string(), + "40".to_string(), + VIEWER_LOG.to_string(), + ]) + .await + .unwrap_or_default() } // ───────────────────────────────────────────────────────────────────────────── @@ -645,14 +648,17 @@ const MAX_PORT_ATTEMPTS: usize = 3; /// this layer, and the pane would then front it. What contains that is /// downstream — the host proxy is loopback-only and token-gated, and the pane's /// iframe is sandboxed — not this function. -async fn start_viewer(container_id: &str, cli_entry: &str) -> Result<(u16, String), String> { +async fn start_viewer( + container: &ContainerTarget, + cli_entry: &str, +) -> Result<(u16, String), String> { let mut tried: Vec = Vec::new(); let mut last: Option = None; for _ in 0..MAX_PORT_ATTEMPTS { // Re-read the listener snapshot each attempt: the port that was free a // moment ago is exactly the one we may have just lost. - let port = match pick_viewer_port(container_id, &tried).await { + let port = match pick_viewer_port(container, &tried).await { Ok(p) => p, Err(e) => { // Report why the *attempts* failed, not just "nothing free": @@ -666,21 +672,21 @@ async fn start_viewer(container_id: &str, cli_entry: &str) -> Result<(u16, Strin }; tried.push(port); - launch_viewer(container_id, cli_entry, port).await?; + launch_viewer(container, cli_entry, port).await?; // Wait for it to actually answer, and learn the entry URL while we're // there — see `probe_entry_path` for why that matters. This, not the // launcher's stdout, is the readiness signal: verified that the // "Listening on …" line is printed only on the very first start. - match wait_until_ready(container_id, port).await { + match wait_until_ready(container, port).await { Ok(path) => return Ok((port, path)), Err(e) => { - let log = read_viewer_log(container_id).await; + let log = read_viewer_log(container).await; // Always kill before retrying: the dashboard is a singleton, so // a launcher that came up on some *other* port would otherwise // make every further attempt a no-op that silently ignores the // port we asked for. - let _ = kill_dashboard(container_id, cli_entry).await; + let _ = kill_dashboard(container, cli_entry).await; last = Some(explain_start_failure(&e, &log)); } } @@ -691,10 +697,9 @@ async fn start_viewer(container_id: &str, cli_entry: &str) -> Result<(u16, Strin /// First port in [`VIEWER_PORTS`] that nothing in the container is listening on /// and that this start has not already tried. -async fn pick_viewer_port(container_id: &str, tried: &[u16]) -> Result { - let text = exec_oneshot( - container_id, - vec![ +async fn pick_viewer_port(container: &ContainerTarget, tried: &[u16]) -> Result { + let text = container + .exec_oneshot(vec![ // Absolute path, deliberately, for the same reason the auth bridge // uses one: `container/Dockerfile` puts a container-writable // directory first on `PATH`, so a bare `cat` is a name the container @@ -704,10 +709,9 @@ async fn pick_viewer_port(container_id: &str, tried: &[u16]) -> Result Result Result { +async fn wait_until_ready(container: &ContainerTarget, port: u16) -> Result { let deadline = tokio::time::Instant::now() + READY_TIMEOUT; loop { - let last = match probe_entry_path(container_id, port).await { + let last = match probe_entry_path(container, port).await { Ok(path) => return Ok(path), Err(e) => e, }; @@ -751,7 +755,7 @@ const PROBE_MARKER: &str = "__TRIPLE_C_BV_PATH__"; /// a redirect — which matters, because a redirect drops the `?token=` the proxy /// gate wants and would leave a fresh connection to be authorised with nothing. /// A `200` (no redirect) is fine too; then the entry point is just `/`. -async fn probe_entry_path(container_id: &str, port: u16) -> Result { +async fn probe_entry_path(container: &ContainerTarget, port: u16) -> Result { // The request is bounded on both sides. Verified: the dashboard answers a // bad WebSocket path by holding the socket open forever rather than // erroring, so "no reply" is a state this probe has to be able to leave — @@ -766,10 +770,7 @@ async fn probe_entry_path(container_id: &str, port: u16) -> Result PageState { "try{{process.stdout.write(require('fs').readFileSync('{}','utf8'));}}catch(e){{}}", STATE_PATH ); - let Ok((out, _)) = exec_oneshot_as( - container_id, - "claude", - vec!["node".to_string(), "-e".to_string(), script], - Vec::new(), - ) - .await + let Ok((out, _)) = ContainerTarget::docker(container_id) + .exec_oneshot_as( + "claude", + vec!["node".to_string(), "-e".to_string(), script], + Vec::new(), + ) + .await else { return PageState::default(); }; @@ -209,15 +209,15 @@ async fn write_control(container_id: &str, json: String) -> Result<(), String> { "require('fs').writeFileSync('{}',process.argv[1]);", CONTROL_PATH ); - exec_oneshot_as( - container_id, - "claude", - vec!["node".to_string(), "-e".to_string(), script, json], - Vec::new(), - ) - .await - .map(|_| ()) - .map_err(|e| format!("Could not reach the browser helper: {}", e)) + ContainerTarget::docker(container_id) + .exec_oneshot_as( + "claude", + vec!["node".to_string(), "-e".to_string(), script, json], + Vec::new(), + ) + .await + .map(|_| ()) + .map_err(|e| format!("Could not reach the browser helper: {}", e)) } /// Wait for a *running* helper to report the URL we just asked it for. diff --git a/app/src-tauri/src/commands/terminal_commands.rs b/app/src-tauri/src/commands/terminal_commands.rs index 5b95164..3cab350 100644 --- a/app/src-tauri/src/commands/terminal_commands.rs +++ b/app/src-tauri/src/commands/terminal_commands.rs @@ -1,6 +1,7 @@ use tauri::{AppHandle, Emitter, State}; use crate::commands::aws_commands; +use crate::host::ContainerTarget; use crate::models::{Backend, BedrockAuthMethod, Project}; use crate::AppState; @@ -159,9 +160,7 @@ pub async fn open_terminal_session( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; - let container_id = project - .container_id - .as_ref() + let target = ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; if let Some(id) = resume_session_id.as_deref() { @@ -189,7 +188,7 @@ pub async fn open_terminal_session( state .exec_manager .create_session( - &crate::host::ContainerTarget::docker(container_id.as_str()), + &target, &session_id, cmd, move |data| { @@ -240,7 +239,7 @@ pub async fn paste_image_to_terminal( image_data: Vec, state: State<'_, AppState>, ) -> Result { - let container_id = state.exec_manager.get_container_id(&session_id).await?; + let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); let timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -250,11 +249,7 @@ pub async fn paste_image_to_terminal( state .exec_manager - .write_file_to_container( - &crate::host::ContainerTarget::docker(container_id.as_str()), - &file_name, - &image_data, - ) + .write_file_to_container(&target, &file_name, &image_data) .await } @@ -287,7 +282,7 @@ pub async fn upload_host_file_to_terminal( let base = crate::commands::file_commands::host_upload_name(&host_path)?; let host_path = crate::commands::file_commands::resolve_host_read_path(&host_path).await?; - let container_id = state.exec_manager.get_container_id(&session_id).await?; + let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); let meta = tokio::fs::metadata(&host_path) .await @@ -317,7 +312,7 @@ pub async fn upload_host_file_to_terminal( // with the code that does the reading, which re-applies it to the open // descriptor — this check is here only so the refusal reads like a sentence // instead of arriving after a 300 MB read. - use crate::docker::exec::MAX_DROP_BYTES; + use crate::host::MAX_DROP_BYTES; if meta.len() > MAX_DROP_BYTES { return Err(format!( "File too large to drop into the terminal ({:.0} MB; limit {} MB). Mount it into the project instead.", @@ -330,20 +325,18 @@ pub async fn upload_host_file_to_terminal( // Ensure the destination directory exists rather than relying on Docker's // archive extractor to create the parent for the uploaded tar entry. - crate::docker::exec::exec_oneshot( - &container_id, - vec!["mkdir".to_string(), "-p".to_string(), "/tmp/triple-c-drops".to_string()], - ) - .await?; + target + .exec_oneshot(vec![ + "mkdir".to_string(), + "-p".to_string(), + "/tmp/triple-c-drops".to_string(), + ]) + .await?; let file_name = format!("triple-c-drops/{}", base); - crate::docker::exec::upload_host_file_to_container( - &container_id, - &host_path, - "/tmp", - &file_name, - ) - .await + target + .upload_host_file(&host_path, "/tmp", &file_name) + .await } #[tauri::command] @@ -351,8 +344,8 @@ pub async fn start_audio_bridge( session_id: String, state: State<'_, AppState>, ) -> Result<(), String> { - // Get container_id from the terminal session - let container_id = state.exec_manager.get_container_id(&session_id).await?; + // Get the container from the terminal session + let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); // Create audio bridge exec session with ID "audio-{session_id}" // The loop handles reconnection when the FIFO reader (fake rec) is killed and restarted @@ -366,7 +359,7 @@ pub async fn start_audio_bridge( state .exec_manager .create_session_with_tty( - &crate::host::ContainerTarget::docker(container_id.as_str()), + &target, &audio_session_id, cmd, false, -- 2.52.0 From a19643e0e828aeb3e9455245a09f431be6e5e4c4 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 13 Nov 2026 10:32:46 -0800 Subject: [PATCH 5/5] Finish phase 1a: empty the boundary allowlist, sessions hand out their target - ExecSessionManager::get_target replaces get_container_id, so image paste, file drop and the audio bridge reuse the session's own target instead of rebuilding a Docker one; the web terminal builds its target per project. - Drop the error-prefix rewriting helper: download/backup/run-now errors now carry the host layer's own message after their prefix. - Remove the now-unused create_attached_exec and MAX_ONESHOT_OUTPUT re-export. - The boundary test's allowlist is down to migration (Docker-only) and container_watch (phase 1b). CLAUDE.md documents the host layer. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 13 +++++- app/src-tauri/src/commands/file_commands.rs | 18 +------- .../src/commands/inspect_commands.rs | 12 ++--- .../src/commands/terminal_commands.rs | 6 +-- app/src-tauri/src/docker/exec.rs | 46 ++++++------------- app/src-tauri/src/host/mod.rs | 18 +------- app/src-tauri/src/host/sessions.rs | 7 ++- app/src-tauri/src/web_terminal/ws_handler.rs | 6 +-- 8 files changed, 41 insertions(+), 85 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f929ec7..ca0ad90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -264,8 +264,8 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li - **`docker/`** — Docker API layer using bollard: - `client.rs` — Singleton Docker connection via `OnceLock` - `container.rs` — Container lifecycle (create, start, stop, remove, inspect) - - `exec.rs` — Attached exec streaming. `create_attached_exec()` is the **single** place an - attached exec is opened; terminal sessions and the auth bridge both go through it. + - `exec.rs` — Docker's implementation of everything `host::ContainerTarget` does inside a + container. `create_attached_exec_spec()` is the **single** place an attached exec is opened. - `image.rs` — Image build/pull with progress streaming - `gateway.rs` — Optional LiteLLM sibling container giving Claude Code an Anthropic-format front end for providers that only speak OpenAI (see `gateway-container/`). Mirrors `stt.rs`. @@ -282,6 +282,15 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li - `legacy_cleanup.rs` — One-release migration shim removing leftovers from the deleted MCP feature (containers labelled `triple-c.mcp-server`, `triple-c-net-*` networks). Deletable once users have migrated. +- **`host/`** — Where a project's container runs, and the only way into it. `ContainerTarget` + (one `Docker` variant today; AWS planned — `docs/superpowers/specs/2026-11-13-aws-ecs-host-design.md`) + wraps execs, file transfer and `is_running`; `sessions.rs` holds `ExecSessionManager`. + **Container-interior code must go through `ContainerTarget`, never `docker::exec`, bollard or + `get_docker()`** — `host::tests::no_container_interior_bollard_outside_the_host_layer` fails + the build on it. Its allowlist is Docker-only code (migration) plus lifecycle/`die` events, + which phase 1b moves; shrink it, never grow it. Prefer `ContainerTarget::for_project` or a + target a session already holds (`ExecSessionManager::get_target`) over + `ContainerTarget::docker(id)`, which hard-codes the host. - **`web_terminal/`** — Remote terminal access via axum HTTP+WebSocket server: - `server.rs` — Axum server lifecycle (start/stop), serves embedded HTML and handles WS upgrades - `ws_handler.rs` — Per-connection WebSocket handler with JSON protocol, session management, cleanup on disconnect diff --git a/app/src-tauri/src/commands/file_commands.rs b/app/src-tauri/src/commands/file_commands.rs index f28f7a3..1a472df 100644 --- a/app/src-tauri/src/commands/file_commands.rs +++ b/app/src-tauri/src/commands/file_commands.rs @@ -1435,17 +1435,6 @@ pub async fn create_container_directory( Ok(dest) } -/// The underlying cause of a [`ContainerTarget::attach`] failure, without the -/// host layer's own "Failed to create exec: " / "Failed to start exec: " -/// prefix, so the transfers below can keep framing it in their own words as -/// they did when they opened the exec themselves. Anything else (an -/// unreachable daemon, a detached start) passes through whole. -fn strip_exec_error_prefix(e: &str) -> &str { - e.strip_prefix("Failed to create exec: ") - .or_else(|| e.strip_prefix("Failed to start exec: ")) - .unwrap_or(e) -} - /// Every "container is not running" refusal starts with this, so a caller (the file /// viewer's poll, `app/src/viewer/ipcMessages.ts`) can tell it apart from any other failure. pub(crate) const NOT_RUNNING_PREFIX: &str = "Start the project before"; @@ -1630,7 +1619,7 @@ dd iflag=nonblock bs=64k status=none if="$TC_SRC" || exit 5"#; .no_stdin(), ) .await - .map_err(|e| format!("Failed to start download: {}", strip_exec_error_prefix(&e)))?; + .map_err(|e| format!("Failed to start download: {}", e))?; let exec_id = exec.exec_id; let mut output = exec.output; @@ -2310,10 +2299,7 @@ tar czf - --ignore-failed-read \ .no_stdin(), ) .await - .map_err(|e| match e.strip_prefix("Failed to create exec: ") { - Some(rest) => format!("Failed to create backup exec: {}", rest), - None => format!("Failed to start backup exec: {}", strip_exec_error_prefix(&e)), - })?; + .map_err(|e| format!("Failed to start backup: {}", e))?; let exec_id = exec.exec_id; let mut output = exec.output; diff --git a/app/src-tauri/src/commands/inspect_commands.rs b/app/src-tauri/src/commands/inspect_commands.rs index a637fc6..be6c81c 100644 --- a/app/src-tauri/src/commands/inspect_commands.rs +++ b/app/src-tauri/src/commands/inspect_commands.rs @@ -1287,14 +1287,10 @@ pub async fn run_scheduled_task_now( ]) .env(home_env()) .no_stdin(); - let exec_id = target.exec_detached(spec).await.map_err(|e| { - // Keep this command's own wording for a start failure ("Failed to start - // task: …"); a create failure reads "Failed to create exec: …" either way. - match e.strip_prefix("Failed to start exec: ") { - Some(rest) => format!("Failed to start task: {}", rest), - None => e, - } - })?; + let exec_id = target + .exec_detached(spec) + .await + .map_err(|e| format!("Failed to start task: {}", e))?; log::info!( "Triggered scheduler task {} in project {} (detached exec {})", diff --git a/app/src-tauri/src/commands/terminal_commands.rs b/app/src-tauri/src/commands/terminal_commands.rs index 3cab350..a1ddfd7 100644 --- a/app/src-tauri/src/commands/terminal_commands.rs +++ b/app/src-tauri/src/commands/terminal_commands.rs @@ -239,7 +239,7 @@ pub async fn paste_image_to_terminal( image_data: Vec, state: State<'_, AppState>, ) -> Result { - let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); + let target = state.exec_manager.get_target(&session_id).await?; let timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -282,7 +282,7 @@ pub async fn upload_host_file_to_terminal( let base = crate::commands::file_commands::host_upload_name(&host_path)?; let host_path = crate::commands::file_commands::resolve_host_read_path(&host_path).await?; - let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); + let target = state.exec_manager.get_target(&session_id).await?; let meta = tokio::fs::metadata(&host_path) .await @@ -345,7 +345,7 @@ pub async fn start_audio_bridge( state: State<'_, AppState>, ) -> Result<(), String> { // Get the container from the terminal session - let target = ContainerTarget::docker(state.exec_manager.get_container_id(&session_id).await?); + let target = state.exec_manager.get_target(&session_id).await?; // Create audio bridge exec session with ID "audio-{session_id}" // The loop handles reconnection when the FIFO reader (fake rec) is killed and restarted diff --git a/app/src-tauri/src/docker/exec.rs b/app/src-tauri/src/docker/exec.rs index 0e4caac..3a7ab60 100644 --- a/app/src-tauri/src/docker/exec.rs +++ b/app/src-tauri/src/docker/exec.rs @@ -5,40 +5,20 @@ use futures_util::StreamExt; use super::client::get_docker; use crate::host::{ExecChunk, ExecSpec}; -/// A `docker exec` that has been created and started with stdin/stdout/stderr -/// attached — the raw duplex halves, before any policy about what to do with -/// them. -/// -/// This is the single place in the codebase that knows how to open an attached -/// exec. Both consumers are built on it: -/// * [`ExecSessionManager`] — interactive terminals and the audio bridge, -/// which pump bytes through mpsc channels and a callback. -/// * `auth_bridge` — per-connection `socat` tunnels, which pump bytes -/// straight between a host TCP socket and these halves. -/// -/// With `tty = false` the output stream is demultiplexed by Docker, so the -/// consumer can tell [`LogOutput::StdOut`] from [`LogOutput::StdErr`]. That -/// distinction matters for the auth bridge: `socat`'s diagnostics must not be -/// spliced into the proxied byte stream. +// `AttachedExec` lives in `host` now (host-neutral output chunks); this +// module's job is to produce one from Docker. Its consumers are +// `host::sessions::ExecSessionManager` (terminals, audio bridge) and the auth +// bridge's per-connection `socat` tunnels, both through `host::ContainerTarget`. +// With `tty = false` Docker demultiplexes stdout from stderr, which the auth +// bridge relies on: `socat`'s diagnostics must not be spliced into the proxied +// byte stream. pub use crate::host::AttachedExec; -/// Create and start an exec with stdin + stdout + stderr attached, returning the -/// raw duplex halves. Runs as `claude` in `/workspace`, like every other exec -/// this app opens. -pub async fn create_attached_exec( - container_id: &str, - cmd: Vec, - tty: bool, -) -> Result { - create_attached_exec_as(container_id, cmd, tty, "claude", "/workspace").await -} - -/// [`create_attached_exec`] with the user and working directory spelled out. +/// An attached exec with the user and working directory spelled out. /// -/// Only base-image migration needs this: replaying `apt` and unpacking a -/// payload tar at `/` have to run as **root**, and every other caller wants the -/// `claude` / `/workspace` defaults that [`create_attached_exec`] supplies. It -/// stays the single place an attached exec is opened. +/// Only base-image migration calls this directly: replaying `apt` and unpacking +/// a payload tar at `/` have to run as **root**. Everything else goes through +/// `host::ContainerTarget::attach`. pub async fn create_attached_exec_as( container_id: &str, cmd: Vec, @@ -54,8 +34,8 @@ pub async fn create_attached_exec_as( } /// The single place an attached exec is opened on Docker. Everything else — -/// [`create_attached_exec`], [`create_attached_exec_as`], and -/// `host::ContainerTarget::attach` — is a thin wrapper over this. +/// [`create_attached_exec_as`] and `host::ContainerTarget::attach` — is a thin +/// wrapper over this. pub async fn create_attached_exec_spec( container_id: &str, spec: &ExecSpec, diff --git a/app/src-tauri/src/host/mod.rs b/app/src-tauri/src/host/mod.rs index 5a1f4ab..1350610 100644 --- a/app/src-tauri/src/host/mod.rs +++ b/app/src-tauri/src/host/mod.rs @@ -29,7 +29,7 @@ pub mod sessions; pub use crate::docker::exec::{ - MAX_DROP_BYTES, MAX_ONESHOT_OUTPUT, OUTPUT_LIMIT_MARKER, PROC_NET_OUTPUT_LIMIT, + MAX_DROP_BYTES, OUTPUT_LIMIT_MARKER, PROC_NET_OUTPUT_LIMIT, }; pub use sessions::ExecSessionManager; @@ -434,22 +434,6 @@ mod tests { "commands/migration_commands.rs", // Phase 1b: lifecycle and the `die` event stream. "container_watch.rs", - // Phase 1a, not yet migrated. Each migration removes its own - // entries; the list must be empty before phase 1a merges. - "auth_bridge/mod.rs", - "auth_bridge/tunnel.rs", - "browser_view/detect.rs", - "browser_view/install.rs", - "browser_view/mod.rs", - "browser_view/page.rs", - "commands/auth_token_commands.rs", - "commands/file_commands.rs", - "commands/inspect_commands.rs", - "commands/terminal_commands.rs", - "file_viewer/poll.rs", - "file_viewer/resolve.rs", - "marketplace/gh_login.rs", - "marketplace/sync.rs", ]; let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut offenders = Vec::new(); diff --git a/app/src-tauri/src/host/sessions.rs b/app/src-tauri/src/host/sessions.rs index acdcbba..38e977e 100644 --- a/app/src-tauri/src/host/sessions.rs +++ b/app/src-tauri/src/host/sessions.rs @@ -187,12 +187,15 @@ impl ExecSessionManager { } } - pub async fn get_container_id(&self, session_id: &str) -> Result { + /// The container a session was opened on — reuse it for side-channel work + /// on the same session (image paste, file drop, audio bridge) rather than + /// rebuilding a target from an id, which would assume the host. + pub async fn get_target(&self, session_id: &str) -> Result { let sessions = self.sessions.lock().await; let session = sessions .get(session_id) .ok_or_else(|| format!("Session {} not found", session_id))?; - Ok(session.target.id().to_string()) + Ok(session.target.clone()) } pub async fn write_file_to_container( diff --git a/app/src-tauri/src/web_terminal/ws_handler.rs b/app/src-tauri/src/web_terminal/ws_handler.rs index 1782381..8d60578 100644 --- a/app/src-tauri/src/web_terminal/ws_handler.rs +++ b/app/src-tauri/src/web_terminal/ws_handler.rs @@ -295,9 +295,7 @@ async fn handle_open( return Err(format!("Project '{}' is not running", project.name)); } - let container_id = project - .container_id - .as_ref() + let container = crate::host::ContainerTarget::for_project(&project) .ok_or_else(|| "Container not running".to_string())?; let cmd = match session_type { @@ -330,7 +328,7 @@ async fn handle_open( state .exec_manager .create_session( - &crate::host::ContainerTarget::docker(container_id.as_str()), + &container, &session_id, cmd, on_output, -- 2.52.0