diff --git a/CLAUDE.md b/CLAUDE.md index ca0ad90..439ac46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -287,10 +287,14 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li 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. + the build on it. `lifecycle.rs`'s `ProjectHost` owns create/start/stop/find/remove and + discarding a project's persistent state; the project commands keep only host-neutral steps + (validation, status, sessions, auth bridge, syncs). `watch.rs` gives `container_watch` its + death reports and positive-exit check. `no_container_lifecycle_or_bare_docker_targets_outside_the_host_layer` + fails on lifecycle calls or `ContainerTarget::docker(id)` in non-test code outside `host/` and + `docker/`; get a target from `ContainerTarget::for_project`, `ProjectHost`, or a session + (`ExecSessionManager::get_target`). Base-image migration is the one Docker-only exception + (`DOCKER_ONLY`); shrink it, never grow it. - **`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/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index 5117479..6771e37 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -55,7 +55,8 @@ use tauri::State; -use crate::commands::project_commands::{create_container_for_project, emit_progress}; +use crate::commands::project_commands::emit_progress; +use crate::docker::{create_container_for_project, default_docker_socket}; use crate::docker; use crate::docker::migration::{self as mig, Recovery}; use crate::models::{ @@ -1810,14 +1811,6 @@ pub(crate) async fn purge_migration_artifacts(project_id: &str) { migration_store::clear_ownerless_for_project(project_id); } -fn default_docker_socket() -> String { - if cfg!(target_os = "windows") { - "//./pipe/docker_engine".to_string() - } else { - "/var/run/docker.sock".to_string() - } -} - fn human_bytes(n: u64) -> String { const UNITS: [&str; 5] = ["B", "KiB", "MiB", "GiB", "TiB"]; let mut v = n as f64; diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs index becd18c..412b17c 100644 --- a/app/src-tauri/src/commands/project_commands.rs +++ b/app/src-tauri/src/commands/project_commands.rs @@ -2,7 +2,9 @@ use tauri::{Emitter, State}; use crate::commands::aws_commands; use crate::docker; -use crate::models::{container_config, AppSettings, Backend, BedrockAuthMethod, Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ProjectStatus}; +use crate::host::lifecycle::{DiscardLeftovers, ProjectHost}; +use crate::host::ContainerTarget; +use crate::models::{Backend, BedrockAuthMethod, Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ProjectStatus}; use crate::storage::secure; use crate::AppState; @@ -114,48 +116,6 @@ fn store_secrets_for_project(project: &Project, explicitly_cleared: &[&str]) -> Ok(()) } -/// Create the project's container, threading every global setting through. -/// -/// Exists so that the two ordinary create paths below and base-image migration -/// cannot drift apart — a container created by a migration must be -/// indistinguishable from one created by a normal start, or the next -/// `container_needs_recreation` would immediately throw it away. -/// -/// `create_image` is what to create *from* (the snapshot or the base); -/// `base_image_name` is the configured base, which `create_container` needs in -/// order to tell those two apart when it stamps the lineage labels. -pub(crate) async fn create_container_for_project( - project: &Project, - settings: &AppSettings, - docker_socket: &str, - aws_config_path: Option<&str>, - create_image: &str, - base_image_name: &str, - extras: docker::CreateExtras<'_>, -) -> Result { - docker::create_container( - project, - docker_socket, - create_image, - base_image_name, - extras, - aws_config_path, - &settings.global_aws, - &settings.global_ollama, - &settings.global_llamacpp, - &settings.global_openai_compatible, - settings.global_claude_instructions.as_deref(), - &settings.global_custom_env_vars, - settings.timezone.as_deref(), - settings.global_claude_code_settings.as_ref(), - settings.default_ssh_key_path.as_deref(), - settings.ca_cert_path.as_deref(), - settings.default_git_user_name.as_deref(), - settings.default_git_user_email.as_deref(), - ) - .await -} - /// Populate secret fields on a project struct from the OS keychain. pub(crate) fn load_secrets_for_project(project: &mut Project) { project.git_token = secure::get_project_secret(&project.id, "git-token") @@ -774,7 +734,8 @@ pub async fn remove_project( if let Some(ref stored_id) = project.container_id { state.exec_manager.close_sessions_for_container(stored_id).await; } - let container_ref = match docker::find_existing_container(project).await { + let host = ProjectHost::for_project(project); + let container_ref = match host.find_current(project).await { Ok(found) => found, Err(e) => { log::warn!( @@ -785,15 +746,14 @@ pub async fn remove_project( None } }; - if let Some(ref container_id) = container_ref { - if project.container_id.as_deref() != Some(container_id.as_str()) { - state.exec_manager.close_sessions_for_container(container_id).await; + if let Some(ref container) = container_ref { + if project.container_id.as_deref() != Some(container.id()) { + state.exec_manager.close_sessions_for_container(container.id()).await; } - let _ = docker::stop_container(container_id).await; - if let Err(e) = docker::remove_container(container_id).await { + if let Err(e) = host.remove(container).await { log::warn!( "Failed to remove container {} for project {}: {}", - container_id, project_id, e + container.id(), project_id, e ); // Recorded by name, not id: the name is the stable handle a // later retry can still resolve (Docker's remove-container @@ -805,15 +765,12 @@ pub async fn remove_project( // Legacy MCP cleanup (pre-MCP-removal installs): drop any leftover MCP // containers first, then the per-project network they were attached to. - docker::remove_legacy_mcp_containers(&project.id).await; - docker::remove_legacy_project_network(&project.id).await; + host.remove_legacy_leftovers(project).await; // Clean up the snapshot image + volumes - if let Err(e) = docker::remove_snapshot_image(project).await { - log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e); - report.image = Some(docker::get_snapshot_image_name(project)); - } - report.volumes = docker::remove_project_volumes(project).await; + let left = host.discard_project_state(project).await; + report.image = left.image; + report.volumes = left.volumes; } // Clean up keychain secrets for this project @@ -1204,9 +1161,9 @@ async fn start_project_container_locked( // in memory when building environment variables for the container. load_secrets_for_project(&mut project); - // Load settings for image resolution and global AWS + // Load settings: backend validation below, and the host's start (image + // resolution, global AWS, …). let settings = state.settings_store.get(); - let image_name = container_config::resolve_image_name(&settings.image_source, &settings.custom_image_name); // Validate backend requirements if project.backend == Backend::Bedrock { @@ -1322,140 +1279,18 @@ async fn start_project_container_locked( } // Wrap container operations so that any failure resets status to Stopped. - let result: Result = async { - // Ensure image exists - emit_progress(&app_handle, &project_id, "Checking image..."); - if !docker::image_exists(&image_name).await? { - return Err(format!("Docker image '{}' not found. Please pull or build the image first.", image_name)); - } - - // Determine docker socket path - let docker_socket = settings.docker_socket_path - .as_deref() - .map(|s| s.to_string()) - .unwrap_or_else(|| default_docker_socket()); - - // AWS config path from global settings - let aws_config_path = settings.global_aws.aws_config_path.clone(); - - // What we would create this container from *right now*: the project's - // snapshot when one exists, else the configured base. This is the value - // `container_needs_recreation` compares against the container's - // `triple-c.create-image` label — the check that replaced the old - // tautological one. It is resolved *before* the commit below, so it - // describes the pre-commit world the existing container was born into. - let snapshot_image = docker::get_snapshot_image_name(&project); - let expected_create_image = - if docker::image_exists(&snapshot_image).await.unwrap_or(false) { - snapshot_image.clone() - } else { - image_name.clone() - }; - - let container_id = if let Some(existing_id) = docker::find_existing_container(&project).await? { - // Check if config changed — if so, snapshot + recreate - let needs_recreate = docker::container_needs_recreation( - &existing_id, - &project, - &expected_create_image, - &settings.global_aws, - &settings.global_ollama, - &settings.global_llamacpp, - &settings.global_openai_compatible, - settings.global_claude_instructions.as_deref(), - &settings.global_custom_env_vars, - settings.timezone.as_deref(), - settings.global_claude_code_settings.as_ref(), - settings.default_ssh_key_path.as_deref(), - settings.ca_cert_path.as_deref(), - settings.default_git_user_name.as_deref(), - settings.default_git_user_email.as_deref(), - ).await.unwrap_or(false); - - if needs_recreate { - log::info!("Container config changed for project {} — committing snapshot and recreating", project.id); - // Snapshot the filesystem before destroying - emit_progress(&app_handle, &project_id, "Saving container state..."); - if let Err(e) = docker::commit_container_snapshot(&existing_id, &project).await { - log::warn!("Failed to snapshot container before recreation: {}", e); - } - emit_progress(&app_handle, &project_id, "Recreating container..."); - let _ = docker::stop_container(&existing_id).await; - docker::remove_container(&existing_id).await?; - - // Legacy MCP cleanup: the old container may have been attached to - // `triple-c-net-`. Tear down leftover MCP containers and - // that network now, before the replacement is created without it. - docker::remove_legacy_mcp_containers(&project.id).await; - docker::remove_legacy_project_network(&project.id).await; - - // Create from snapshot image (preserves system-level changes). - // Re-resolved after the commit above: when no snapshot existed - // before, one does now, and creating from the base instead - // would throw away the state that was just saved. - let create_image = if docker::image_exists(&snapshot_image).await.unwrap_or(false) { - snapshot_image.clone() - } else { - image_name.clone() - }; - - let new_id = create_container_for_project( - &project, - &settings, - &docker_socket, - aws_config_path.as_deref(), - &create_image, - &image_name, - docker::CreateExtras::default(), - ).await?; - emit_progress(&app_handle, &project_id, "Starting container..."); - docker::start_container(&new_id).await?; - - // The commit above moved `:latest` and orphaned the image it - // used to point at; the container holding that image open was - // removed a few lines up, so now is when Docker will actually - // let it go. Detached because this is housekeeping and the - // project is already running — and it sweeps every orphan, not - // just this one, so recreations that happened before the sweep - // existed are cleaned up too. - tauri::async_runtime::spawn(async { - docker::sweep_orphaned_snapshots_logged("after recreation").await; - }); - - new_id - } else { - emit_progress(&app_handle, &project_id, "Starting container..."); - docker::start_container(&existing_id).await?; - existing_id - } - } else { - // Container doesn't exist (first start, or Docker pruned it). - // Check for a snapshot image first — it preserves system-level - // changes (apt/pip/npm installs) from the previous session. - if expected_create_image == snapshot_image { - log::info!("Creating container from snapshot image for project {}", project.id); - } - let create_image = expected_create_image.clone(); - - emit_progress(&app_handle, &project_id, "Creating container..."); - let new_id = create_container_for_project( - &project, - &settings, - &docker_socket, - aws_config_path.as_deref(), - &create_image, - &image_name, - docker::CreateExtras::default(), - ).await?; - emit_progress(&app_handle, &project_id, "Starting container..."); - docker::start_container(&new_id).await?; - new_id - }; + let host = ProjectHost::for_project(&project); + let result: Result = async { + let target = host + .ensure_started(&project, &settings, &|msg: &str| { + emit_progress(&app_handle, &project_id, msg) + }) + .await?; // Sync Bedrock credentials on every start: refresh static/session creds // so rotated keys are picked up without a full container recreation, and // clear stale creds when the project no longer uses static-cred Bedrock. - if let Err(e) = docker::sync_bedrock_credentials(&container_id, &project).await { + if let Err(e) = docker::sync_bedrock_credentials(&target, &project).await { log::warn!("Failed to sync AWS credentials for project {}: {}", project.id, e); } @@ -1466,10 +1301,10 @@ async fn start_project_container_locked( state.marketplace.clone(), state.settings_store.get(), project.clone(), - container_id.clone(), + target.clone(), ); - Ok(container_id) + Ok(target) }.await; // On failure, reset status to Stopped so the project doesn't get stuck. @@ -1478,7 +1313,8 @@ async fn start_project_container_locked( let _ = state.projects_store.update_status(&project_id, ProjectStatus::Stopped); } - let container_id = result?; + let target = result?; + let container_id = target.id().to_string(); // Update project with container info using granular methods (Issue 14: TOCTOU) state.projects_store.set_container_id(&project_id, Some(container_id.clone()))?; @@ -1491,8 +1327,7 @@ async fn start_project_container_locked( .auth_bridge .start( project_id.clone(), - // phase 1b: lifecycle will hand out the target - crate::host::ContainerTarget::docker(container_id.clone()), + target.clone(), app_handle.clone(), state.projects_store.clone(), ) @@ -1528,13 +1363,13 @@ pub async fn stop_project_container( // Drop host listeners first: they only make sense while the container runs. state.auth_bridge.stop(&project_id).await; - if let Some(ref container_id) = project.container_id { + if let Some(container) = ContainerTarget::for_project(&project) { // Close exec sessions for this project emit_progress(&app_handle, &project_id, "Stopping container..."); - state.exec_manager.close_sessions_for_container(container_id).await; + state.exec_manager.close_sessions_for_container(container.id()).await; - if let Err(e) = docker::stop_container(container_id).await { - log::warn!("Docker stop failed for container {} (project {}): {} — resetting to Stopped anyway", container_id, project_id, e); + if let Err(e) = ProjectHost::for_project(&project).stop(&container).await { + log::warn!("Stop failed for container {} (project {}): {} — resetting to Stopped anyway", container.id(), project_id, e); } } @@ -1589,13 +1424,13 @@ pub async fn rebuild_project_container( if let Some(ref stored_id) = project.container_id { state.exec_manager.close_sessions_for_container(stored_id).await; } - let container_ref = docker::find_existing_container(&project).await?; - if let Some(ref container_id) = container_ref { - if project.container_id.as_deref() != Some(container_id.as_str()) { - state.exec_manager.close_sessions_for_container(container_id).await; + let host = ProjectHost::for_project(&project); + let container_ref = host.find_current(&project).await?; + if let Some(ref container) = container_ref { + if project.container_id.as_deref() != Some(container.id()) { + state.exec_manager.close_sessions_for_container(container.id()).await; } - let _ = docker::stop_container(container_id).await; - docker::remove_container(container_id).await?; + host.remove(container).await?; state.projects_store.set_container_id(&project_id, None)?; } @@ -1608,12 +1443,10 @@ pub async fn rebuild_project_container( // discard. No pending-cleanup record for either: unlike `remove_project`, // Reset keeps the project record, so a later Reset attempt can retry // these itself rather than needing startup housekeeping to do it. - let mut leftover_image = None; - if let Err(e) = docker::remove_snapshot_image(&project).await { - log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e); - leftover_image = Some(docker::get_snapshot_image_name(&project)); - } - let leftover_volumes = docker::remove_project_volumes(&project).await; + let DiscardLeftovers { + image: leftover_image, + volumes: leftover_volumes, + } = host.discard_project_state(&project).await; if leftover_image.is_some() || !leftover_volumes.is_empty() { log::warn!( "Reset for project {} could not fully clean up — image: {:?}, volumes: {:?} — the \ @@ -1680,8 +1513,8 @@ pub async fn reconcile_project_statuses( continue; } - let is_running = if let Some(ref container_id) = project.container_id { - docker::is_container_running(container_id).await.unwrap_or(false) + let is_running = if let Some(container) = ContainerTarget::for_project(project) { + container.is_running().await.unwrap_or(false) } else { false }; @@ -1721,13 +1554,6 @@ pub async fn reconcile_project_statuses( Ok(state.projects_store.list()) } -fn default_docker_socket() -> String { - if cfg!(target_os = "windows") { - "//./pipe/docker_engine".to_string() - } else { - "/var/run/docker.sock".to_string() - } -} #[cfg(test)] mod tests { diff --git a/app/src-tauri/src/docker/client.rs b/app/src-tauri/src/docker/client.rs index 49c4e5b..e64eaf3 100644 --- a/app/src-tauri/src/docker/client.rs +++ b/app/src-tauri/src/docker/client.rs @@ -26,3 +26,13 @@ pub async fn check_docker_available() -> Result { } } } + +/// The daemon socket a project container gets bind-mounted when Docker access +/// is enabled and no path is configured. +pub fn default_docker_socket() -> String { + if cfg!(target_os = "windows") { + "//./pipe/docker_engine".to_string() + } else { + "/var/run/docker.sock".to_string() + } +} diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index 3d0ced1..ef39fbb 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -9,6 +9,7 @@ use sha2::{Sha256, Digest}; use super::ca_certs; use super::client::get_docker; +use crate::models::AppSettings; use crate::models::{Backend, BedrockAuthMethod, ClaudeCodeSettings, ContainerInfo, EnvVar, GlobalAwsSettings, GlobalLlamaCppSettings, GlobalOllamaSettings, GlobalOpenAiCompatibleSettings, PortMapping, Project, ProjectPath}; const SCHEDULER_INSTRUCTIONS: &str = r#"## Scheduled Tasks @@ -2015,7 +2016,7 @@ pub const CONFIG_VOLUME_PREFIX: &str = "triple-c-claude-config-"; /// `aws_config_path` is configured), since the entrypoint already refreshes /// `~/.aws` from the host on every start in that case. pub async fn sync_bedrock_credentials( - container_id: &str, + target: &crate::host::ContainerTarget, project: &Project, ) -> Result<(), String> { let static_bedrock = if project.backend == Backend::Bedrock { @@ -2038,10 +2039,10 @@ pub async fn sync_bedrock_credentials( let script = r#"if [ ! -d /tmp/.host-aws ]; then rm -f "$HOME/.aws/credentials"; fi"#; let cmd = vec!["sh".to_string(), "-c".to_string(), script.to_string()]; let env = vec!["HOME=/home/claude".to_string()]; - if let Err(e) = crate::docker::exec::exec_oneshot_env(container_id, cmd, env).await { + if let Err(e) = target.exec_oneshot_env(cmd, env).await { log::warn!( "Failed to clear stale AWS credentials in container {}: {}", - container_id, + target.id(), e ); } @@ -2093,7 +2094,7 @@ chmod 600 "$HOME/.aws/credentials""#; let cmd = vec!["sh".to_string(), "-c".to_string(), script.to_string()]; let (output, exit_code) = - crate::docker::exec::exec_oneshot_env_status(container_id, cmd, env) + target.exec_oneshot_env_status(cmd, env) .await .map_err(|e| format!("Failed to write AWS credentials into container: {}", e))?; if exit_code != 0 { @@ -2104,7 +2105,7 @@ chmod 600 "$HOME/.aws/credentials""#; )); } - log::info!("Wrote Bedrock static credentials into container {}", container_id); + log::info!("Wrote Bedrock static credentials into container {}", target.id()); Ok(()) } @@ -4143,6 +4144,49 @@ pub async fn is_container_running(container_id: &str) -> Result { } + +/// Create the project's container, threading every global setting through. +/// +/// Exists so that the ordinary create paths (`host::lifecycle`) and base-image migration +/// cannot drift apart — a container created by a migration must be +/// indistinguishable from one created by a normal start, or the next +/// `container_needs_recreation` would immediately throw it away. +/// +/// `create_image` is what to create *from* (the snapshot or the base); +/// `base_image_name` is the configured base, which `create_container` needs in +/// order to tell those two apart when it stamps the lineage labels. +pub async fn create_container_for_project( + project: &Project, + settings: &AppSettings, + docker_socket: &str, + aws_config_path: Option<&str>, + create_image: &str, + base_image_name: &str, + extras: CreateExtras<'_>, +) -> Result { + create_container( + project, + docker_socket, + create_image, + base_image_name, + extras, + aws_config_path, + &settings.global_aws, + &settings.global_ollama, + &settings.global_llamacpp, + &settings.global_openai_compatible, + settings.global_claude_instructions.as_deref(), + &settings.global_custom_env_vars, + settings.timezone.as_deref(), + settings.global_claude_code_settings.as_ref(), + settings.default_ssh_key_path.as_deref(), + settings.ca_cert_path.as_deref(), + settings.default_git_user_name.as_deref(), + settings.default_git_user_email.as_deref(), + ) + .await +} + #[cfg(test)] mod tests { use super::*; diff --git a/app/src-tauri/src/host/lifecycle.rs b/app/src-tauri/src/host/lifecycle.rs new file mode 100644 index 0000000..1196d57 --- /dev/null +++ b/app/src-tauri/src/host/lifecycle.rs @@ -0,0 +1,324 @@ +//! Bringing a project's container into existence, starting, stopping and +//! removing it — the host-specific half of the project lifecycle commands. +//! +//! The commands in `commands/project_commands.rs` keep everything that is the +//! same on every host: validating the LLM backend config, the Bedrock SSO +//! check, project status transitions, the project lock, closing terminal +//! sessions, the auth bridge, marketplace and credential syncs. They ask a +//! [`ProjectHost`] for the rest. For Docker that is the +//! find → maybe-snapshot-and-recreate → create → start sequence, which used to +//! sit inline in `start_project_container_locked` and is moved here unchanged. +//! +//! Docker-only operations with no host-neutral meaning — base-image migration, +//! snapshot sweeps, legacy cleanup at startup — keep calling `crate::docker` +//! directly; see the boundary test in `host/mod.rs`. + +use super::ContainerTarget; +use crate::docker; +use crate::models::{AppSettings, Project}; +use crate::models::container_config; + +/// The host a project runs on. One variant today; an AWS variant will carry +/// the connection, region and environment (see the spec's data model). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProjectHost { + Docker, +} + +/// What a host could not clean up when asked to discard a project's state. +/// Docker: the snapshot image and the named volumes. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct DiscardLeftovers { + pub image: Option, + pub volumes: Vec, +} + +impl ProjectHost { + pub fn for_project(_project: &Project) -> Self { + ProjectHost::Docker + } + + /// Make sure the project's container exists, matches the project's + /// current configuration, and is running. Returns where it runs. + /// + /// `progress` receives the same human-readable steps the start view has + /// always shown. Failure leaves the caller to reset the project status. + pub async fn ensure_started( + &self, + project: &Project, + settings: &AppSettings, + progress: &(dyn Fn(&str) + Send + Sync), + ) -> Result { + match self { + ProjectHost::Docker => docker_ensure_started(project, settings, progress) + .await + .map(ContainerTarget::docker), + } + } + + /// The container the host currently has for this project, found by asking + /// the host rather than trusting `project.container_id` — that field can + /// be stale (a crash between remove and the store update, a hand-edited + /// `projects.json`). + pub async fn find_current(&self, project: &Project) -> Result, String> { + match self { + ProjectHost::Docker => Ok(docker::find_existing_container(project) + .await? + .map(ContainerTarget::docker)), + } + } + + pub async fn stop(&self, target: &ContainerTarget) -> Result<(), String> { + match target { + ContainerTarget::Docker { container_id } => docker::stop_container(container_id).await, + } + } + + /// Stop (errors ignored — a stopped container is the goal) and remove. + pub async fn remove(&self, target: &ContainerTarget) -> Result<(), String> { + match target { + ContainerTarget::Docker { container_id } => { + let _ = docker::stop_container(container_id).await; + docker::remove_container(container_id).await + } + } + } + + /// Delete what outlives the container — for Docker the snapshot image and + /// both named volumes (`~/.claude`, the OAuth login and transcripts go with + /// them). Best-effort; reports what it could not remove. + pub async fn discard_project_state(&self, project: &Project) -> DiscardLeftovers { + match self { + ProjectHost::Docker => { + let mut left = DiscardLeftovers::default(); + if let Err(e) = docker::remove_snapshot_image(project).await { + log::warn!("Failed to remove snapshot image for project {}: {}", project.id, e); + left.image = Some(docker::get_snapshot_image_name(project)); + } + left.volumes = docker::remove_project_volumes(project).await; + left + } + } + } + + /// Remove leftovers of features this host no longer has (Docker: the + /// deleted MCP feature's containers and network). + pub async fn remove_legacy_leftovers(&self, project: &Project) { + match self { + ProjectHost::Docker => { + docker::remove_legacy_mcp_containers(&project.id).await; + docker::remove_legacy_project_network(&project.id).await; + } + } + } +} + +/// The Docker start sequence, moved here verbatim from +/// `start_project_container_locked`. Returns the running container's id. +async fn docker_ensure_started( + project: &Project, + settings: &AppSettings, + progress: &(dyn Fn(&str) + Send + Sync), +) -> Result { + let image_name = + container_config::resolve_image_name(&settings.image_source, &settings.custom_image_name); + + // Ensure image exists + progress("Checking image..."); + if !docker::image_exists(&image_name).await? { + return Err(format!("Docker image '{}' not found. Please pull or build the image first.", image_name)); + } + + // Determine docker socket path + let docker_socket = settings.docker_socket_path + .as_deref() + .map(|s| s.to_string()) + .unwrap_or_else(|| docker::default_docker_socket()); + + // AWS config path from global settings + let aws_config_path = settings.global_aws.aws_config_path.clone(); + + // What we would create this container from *right now*: the project's + // snapshot when one exists, else the configured base. This is the value + // `container_needs_recreation` compares against the container's + // `triple-c.create-image` label — the check that replaced the old + // tautological one. It is resolved *before* the commit below, so it + // describes the pre-commit world the existing container was born into. + let snapshot_image = docker::get_snapshot_image_name(project); + let expected_create_image = + if docker::image_exists(&snapshot_image).await.unwrap_or(false) { + snapshot_image.clone() + } else { + image_name.clone() + }; + + let container_id = if let Some(existing_id) = docker::find_existing_container(project).await? { + // Check if config changed — if so, snapshot + recreate + let needs_recreate = docker::container_needs_recreation( + &existing_id, + project, + &expected_create_image, + &settings.global_aws, + &settings.global_ollama, + &settings.global_llamacpp, + &settings.global_openai_compatible, + settings.global_claude_instructions.as_deref(), + &settings.global_custom_env_vars, + settings.timezone.as_deref(), + settings.global_claude_code_settings.as_ref(), + settings.default_ssh_key_path.as_deref(), + settings.ca_cert_path.as_deref(), + settings.default_git_user_name.as_deref(), + settings.default_git_user_email.as_deref(), + ).await.unwrap_or(false); + + if needs_recreate { + log::info!("Container config changed for project {} — committing snapshot and recreating", project.id); + // Snapshot the filesystem before destroying + progress("Saving container state..."); + if let Err(e) = docker::commit_container_snapshot(&existing_id, project).await { + log::warn!("Failed to snapshot container before recreation: {}", e); + } + progress("Recreating container..."); + let _ = docker::stop_container(&existing_id).await; + docker::remove_container(&existing_id).await?; + + // Legacy MCP cleanup: the old container may have been attached to + // `triple-c-net-`. Tear down leftover MCP containers and + // that network now, before the replacement is created without it. + docker::remove_legacy_mcp_containers(&project.id).await; + docker::remove_legacy_project_network(&project.id).await; + + // Create from snapshot image (preserves system-level changes). + // Re-resolved after the commit above: when no snapshot existed + // before, one does now, and creating from the base instead + // would throw away the state that was just saved. + let create_image = if docker::image_exists(&snapshot_image).await.unwrap_or(false) { + snapshot_image.clone() + } else { + image_name.clone() + }; + + let new_id = docker::create_container_for_project( + project, + settings, + &docker_socket, + aws_config_path.as_deref(), + &create_image, + &image_name, + docker::CreateExtras::default(), + ).await?; + progress("Starting container..."); + docker::start_container(&new_id).await?; + + // The commit above moved `:latest` and orphaned the image it + // used to point at; the container holding that image open was + // removed a few lines up, so now is when Docker will actually + // let it go. Detached because this is housekeeping and the + // project is already running — and it sweeps every orphan, not + // just this one, so recreations that happened before the sweep + // existed are cleaned up too. + tauri::async_runtime::spawn(async { + docker::sweep_orphaned_snapshots_logged("after recreation").await; + }); + + new_id + } else { + progress("Starting container..."); + docker::start_container(&existing_id).await?; + existing_id + } + } else { + // Container doesn't exist (first start, or Docker pruned it). + // Check for a snapshot image first — it preserves system-level + // changes (apt/pip/npm installs) from the previous session. + if expected_create_image == snapshot_image { + log::info!("Creating container from snapshot image for project {}", project.id); + } + let create_image = expected_create_image.clone(); + + progress("Creating container..."); + let new_id = docker::create_container_for_project( + project, + settings, + &docker_socket, + aws_config_path.as_deref(), + &create_image, + &image_name, + docker::CreateExtras::default(), + ).await?; + progress("Starting container..."); + docker::start_container(&new_id).await?; + new_id + }; + + Ok(container_id) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// End to end against a real daemon and the registry sandbox image: a + /// throwaway project (fresh UUID, so its container, volumes and snapshot + /// name are its own) is started, reached, stopped, started again on the + /// *same* container, then removed and its state discarded. + #[tokio::test] + #[ignore = "needs a Docker daemon and the triple-c-sandbox image; creates and removes a throwaway project container"] + async fn a_project_container_round_trips_through_the_host() { + let project = Project::new("lifecycle-live-test".to_string(), Vec::new()); + let settings = AppSettings::default(); + let host = ProjectHost::for_project(&project); + let steps = std::sync::Mutex::new(Vec::::new()); + let progress = |m: &str| steps.lock().unwrap().push(m.to_string()); + // Failures inside the round trip return instead of panicking, so the + // cleanup below always runs. + fn check(ok: bool, what: &str) -> Result<(), String> { + if ok { Ok(()) } else { Err(format!("check failed: {what}")) } + } + + let outcome = async { + check(host.find_current(&project).await?.is_none(), "no container before the first start")?; + + let first = host.ensure_started(&project, &settings, &progress).await?; + check(first.is_running().await?, "running after start")?; + let (out, code) = first + .exec_oneshot_env_status(vec!["id".into(), "-un".into()], vec![]) + .await?; + check(out.trim() == "claude" && code == 0, "exec runs as claude")?; + check(host.find_current(&project).await? == Some(first.clone()), "find_current names it")?; + + host.stop(&first).await?; + check(first.has_exited().await, "exited after stop")?; + + // Same config, so the existing container is started, not recreated. + let second = host.ensure_started(&project, &settings, &progress).await?; + check(second == first, "restart reuses the container")?; + check(second.is_running().await?, "running after restart")?; + + host.remove(&second).await?; + check(host.find_current(&project).await?.is_none(), "gone after remove")?; + Ok::<_, String>(()) + } + .await; + + // Always clean up, even after a failed assertion above. + if let Ok(Some(t)) = host.find_current(&project).await { + let _ = host.remove(&t).await; + } + let left = host.discard_project_state(&project).await; + outcome.expect("round trip"); + assert_eq!(left, DiscardLeftovers::default(), "nothing left behind"); + let steps = steps.into_inner().unwrap(); + assert_eq!( + steps, + [ + "Checking image...", + "Creating container...", + "Starting container...", + "Checking image...", + "Starting container...", + ] + ); + } +} diff --git a/app/src-tauri/src/host/mod.rs b/app/src-tauri/src/host/mod.rs index a259dc6..86359d4 100644 --- a/app/src-tauri/src/host/mod.rs +++ b/app/src-tauri/src/host/mod.rs @@ -26,6 +26,7 @@ //! `no_container_interior_bollard_outside_the_host_layer` test pins which files //! may still do so. +pub mod lifecycle; pub mod sessions; pub mod watch; @@ -422,20 +423,11 @@ mod tests { 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", - ]; + /// Every non-`docker/`, non-`host/` source file, with its path relative to + /// `src/`, minus the files in `allowed`. + fn sources_outside_the_host_layer(allowed: &[&str]) -> Vec<(String, String)> { let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); - let mut offenders = Vec::new(); + let mut out = Vec::new(); let mut stack = vec![src.clone()]; while let Some(dir) = stack.pop() { for entry in std::fs::read_dir(&dir).unwrap() { @@ -451,23 +443,83 @@ mod tests { } 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); + if rel.ends_with(".rs") && !allowed.contains(&rel.as_str()) { + out.push((rel, std::fs::read_to_string(&path).unwrap())); } } } - offenders.sort(); + out.sort(); + out + } + + /// Docker-only by nature: base-image migration replays apt, restores a + /// payload, commits and swaps containers. It has no AWS meaning. + const DOCKER_ONLY: &[&str] = &["commands/migration_commands.rs"]; + + /// The host boundary, pinned: outside `docker/` and `host/`, nothing names + /// bollard or `docker::exec`. Container-interior code goes through + /// [`ContainerTarget`]. Shrink [`DOCKER_ONLY`]; do not grow it. + #[test] + fn no_container_interior_bollard_outside_the_host_layer() { + let offenders: Vec = sources_outside_the_host_layer(DOCKER_ONLY) + .into_iter() + .filter(|(_, text)| { + text.contains("bollard::") + || text.contains("docker::exec") + || text.contains("get_docker") + }) + .map(|(rel, _)| rel) + .collect(); assert!( offenders.is_empty(), "these files reach into the container without going through host::ContainerTarget: {:?}", offenders ); } + + /// The lifecycle half of the boundary: outside `docker/` and `host/`, no + /// non-test code creates, starts, stops, finds or recreates a container + /// itself (that is [`lifecycle::ProjectHost`]'s job), and none builds a + /// [`ContainerTarget::docker`] from a bare id, which would hard-code the + /// host. Comments and `#[cfg(test)]` code are not checked. + /// + /// `docker::remove_container` is deliberately absent: startup's + /// pending-cleanup retry removes Docker leftovers it recorded *by name*, + /// which is Docker housekeeping, not lifecycle. + #[test] + fn no_container_lifecycle_or_bare_docker_targets_outside_the_host_layer() { + const BANNED: &[&str] = &[ + "docker::create_container(", + "docker::start_container(", + "docker::stop_container(", + "docker::find_existing_container(", + "docker::container_needs_recreation(", + "docker::commit_container_snapshot(", + "docker::remove_snapshot_image(", + "docker::remove_project_volumes(", + "docker::is_container_running(", + "container::is_container_running(", + "ContainerTarget::docker(", + ]; + let mut offenders = Vec::new(); + for (rel, text) in sources_outside_the_host_layer(DOCKER_ONLY) { + let code = text.split("#[cfg(test)]").next().unwrap_or(""); + for (n, line) in code.lines().enumerate() { + let line = line.trim_start(); + if line.starts_with("//") { + continue; + } + for pat in BANNED { + if line.contains(pat) { + offenders.push(format!("{}:{}: {}", rel, n + 1, pat)); + } + } + } + } + assert!( + offenders.is_empty(), + "container lifecycle / bare Docker targets outside host::lifecycle:\n{}", + offenders.join("\n") + ); + } } diff --git a/app/src-tauri/src/marketplace/mod.rs b/app/src-tauri/src/marketplace/mod.rs index bec3b98..a7d4a5c 100644 --- a/app/src-tauri/src/marketplace/mod.rs +++ b/app/src-tauri/src/marketplace/mod.rs @@ -557,16 +557,14 @@ pub fn spawn_project_sync( mgr: Arc, settings: AppSettings, project: Project, - container_id: String, + // The target the start just produced — not `ContainerTarget::for_project`: + // `project.container_id` is written after this is called, so it could name + // a container that was just replaced, or none at all. + container: ContainerTarget, ) { if !should_sync(&mgr, &settings, &project) { return; } - // phase 1b: lifecycle will hand out the target. Until then the start path - // only has the fresh container's id: `project.container_id` is written - // after this is called, so `ContainerTarget::for_project` could name a - // container that was just replaced, or none at all. - let container = ContainerTarget::docker(container_id); tauri::async_runtime::spawn(async move { let report = sync_project(&mgr, &settings, &project, &container).await; if !report.errors.is_empty() {