diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs index 0f0ce5d..27a1e5f 100644 --- a/app/src-tauri/src/commands/project_commands.rs +++ b/app/src-tauri/src/commands/project_commands.rs @@ -2,7 +2,7 @@ use tauri::{Emitter, State}; use crate::commands::aws_commands; use crate::docker; -use crate::models::{container_config, AppSettings, Backend, BedrockAuthMethod, Project, ProjectPath, ProjectRemovalReport, ProjectStatus}; +use crate::models::{container_config, AppSettings, Backend, BedrockAuthMethod, Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ProjectStatus}; use crate::storage::secure; use crate::AppState; @@ -730,7 +730,21 @@ pub async fn remove_project( let existing_project = state.projects_store.get(&project_id); if let Some(ref project) = existing_project { - if let Some(ref container_id) = project.container_id { + // `project.container_id` can be `None` or stale — a crash between + // creating a container and persisting its id is the same race every + // other destroyer of a project's container already guards against + // with `find_existing_container` (`start_project_container`, + // migration's recreate paths). Removal is the one place that + // mattered least before this fix, because a container `remove_project` + // missed just sat there; now a miss here poisons the volume removal + // right after it (Docker refuses to delete a volume a container still + // references) and mints a pending-cleanup record for volumes with no + // way to name the container actually blocking them. + let container_ref = match &project.container_id { + Some(id) => Some(id.clone()), + None => docker::find_existing_container(project).await.ok().flatten(), + }; + if let Some(ref container_id) = container_ref { 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 { @@ -738,7 +752,11 @@ pub async fn remove_project( "Failed to remove container {} for project {}: {}", container_id, project_id, e ); - report.container = Some(container_id.clone()); + // Recorded by name, not id: the name is the stable handle a + // later retry can still resolve (Docker's remove-container + // call accepts either), and it is what `container_ref` above + // falls back to finding in the first place. + report.container = Some(project.container_name()); } } @@ -770,17 +788,23 @@ pub async fn remove_project( recorded_at: chrono::Utc::now().to_rfc3339(), }; match crate::storage::pending_cleanup::save(&record) { - Ok(()) => log::warn!( - "Project {} removed with Docker resources still present: {:?} — recorded for \ - automatic retry on next launch", - project_id, report - ), - Err(e) => log::error!( - "Project {} removed with Docker resources still present ({:?}), and the \ - pending-cleanup record could not be written ({}) — nothing will retry removing \ - them", - project_id, report, e - ), + Ok(()) => { + report.retry_scheduled = true; + log::warn!( + "Project {} removed with Docker resources still present: {:?} — recorded for \ + automatic retry on next launch", + project_id, report + ); + } + Err(e) => { + report.retry_scheduled = false; + log::error!( + "Project {} removed with Docker resources still present ({:?}), and the \ + pending-cleanup record could not be written ({}) — nothing will retry \ + removing them", + project_id, report, e + ); + } } } @@ -852,6 +876,26 @@ pub async fn retry_pending_cleanup_logged() { cleaned += 1; } else { still_pending += 1; + // `recorded_at` is otherwise write-only — nothing read it back, + // which is exactly the shape `storage::migration_store` calls out + // as a bug in its own history ("nothing ever removed them"). A + // record that has failed every retry for a week is no longer + // routine: escalate the log level so it is not indistinguishable + // from one seen for the first time. + let age = chrono::DateTime::parse_from_rfc3339(&record.recorded_at) + .ok() + .map(|t| chrono::Utc::now().signed_duration_since(t.with_timezone(&chrono::Utc))); + match age { + Some(age) if age > chrono::Duration::days(PENDING_CLEANUP_STALE_AFTER_DAYS) => { + log::error!( + "Pending cleanup for project {} ({}) has not succeeded in over {} days: \ + {:?} — this may need a manual `docker volume rm` / `docker rmi` / \ + `docker rm`", + record.project_id, record.project_name, PENDING_CLEANUP_STALE_AFTER_DAYS, record + ); + } + _ => {} + } if let Err(e) = crate::storage::pending_cleanup::save(&record) { log::warn!( "Could not update pending cleanup record for project {} ({}): {}", @@ -867,6 +911,11 @@ pub async fn retry_pending_cleanup_logged() { ); } +/// After this many days of a pending-cleanup record failing every retry, +/// `retry_pending_cleanup_logged` escalates its log line from `warn` to +/// `error` — see the comment at its call site. +const PENDING_CLEANUP_STALE_AFTER_DAYS: i64 = 7; + #[tauri::command] pub async fn update_project( project: serde_json::Value, @@ -1341,7 +1390,7 @@ pub async fn rebuild_project_container( project_id: String, app_handle: tauri::AppHandle, state: State<'_, AppState>, -) -> Result { +) -> Result { // Reset deletes both volumes and the snapshot image. Doing that while a // migration is mid-flight pulls the ground out from under it and leaves an // orphan migration record pointing at images that no longer exist — and @@ -1368,8 +1417,16 @@ pub async fn rebuild_project_container( // `start_project_container` below re-arms it against the new one. state.auth_bridge.stop(&project_id).await; - // Remove existing container - if let Some(ref container_id) = project.container_id { + // Remove existing container. Resolved the same way `remove_project` now + // is — `project.container_id` can be `None` or stale — because a + // container this misses blocks the volume removal immediately below with + // a 409, and Reset silently keeping the old volumes is exactly the bug + // this whole change is closing. + let container_ref = match &project.container_id { + Some(id) => Some(id.clone()), + None => docker::find_existing_container(&project).await.ok().flatten(), + }; + if let Some(ref container_id) = container_ref { state.exec_manager.close_sessions_for_container(container_id).await; let _ = docker::stop_container(container_id).await; docker::remove_container(container_id).await?; @@ -1397,7 +1454,8 @@ pub async fn rebuild_project_container( // Start fresh. The locked variant, because `_guard` above is this project's // claim and the public command would be refused by it. - start_project_container_locked(project_id, app_handle, state).await + let project = start_project_container_locked(project_id, app_handle, state).await?; + Ok(ProjectResetOutcome { project, leftover_volumes }) } /// Reconcile project statuses against actual Docker container state. diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index 39dd9ff..038e837 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -3585,7 +3585,7 @@ pub async fn remove_volumes_by_name(names: &[String]) -> Vec { let mut leftover = Vec::new(); for vol in names { - match docker.remove_volume(vol, None).await { + match remove_one_volume_with_retry(&docker, vol).await { Ok(_) => log::info!("Removed volume {}", vol), Err(bollard::errors::Error::DockerResponseServerError { status_code: 404, .. @@ -3599,6 +3599,28 @@ pub async fn remove_volumes_by_name(names: &[String]) -> Vec { leftover } +/// Remove one volume, retrying once after a short delay on a 409 ("volume is +/// in use"). Docker releasing a volume's mount reference after the container +/// using it is removed is not always instantaneous, so the very first call +/// site of this — `remove_project`, whose container removal lands +/// immediately before its volume removal — could otherwise turn an ordinary +/// race into a permanent pending-cleanup record and an alarming toast for +/// something that would have cleared itself half a second later. +async fn remove_one_volume_with_retry( + docker: &bollard::Docker, + name: &str, +) -> Result<(), bollard::errors::Error> { + match docker.remove_volume(name, None).await { + Err(bollard::errors::Error::DockerResponseServerError { + status_code: 409, .. + }) => { + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + docker.remove_volume(name, None).await + } + other => other, + } +} + /// Check whether the existing container's configuration still matches the /// current project settings. Returns `true` when the container must be /// recreated (mounts or env vars differ). diff --git a/app/src-tauri/src/models/project.rs b/app/src-tauri/src/models/project.rs index f17747b..c6b09c1 100644 --- a/app/src-tauri/src/models/project.rs +++ b/app/src-tauri/src/models/project.rs @@ -432,12 +432,21 @@ pub enum ProjectStatus { /// belonged to no longer exists. #[derive(Debug, Default, Clone, Serialize, Deserialize)] pub struct ProjectRemovalReport { - /// The project's container, if it could not be removed. + /// The project's container, if it could not be removed. Named by its + /// deterministic `triple-c-{id}` name (see `Project::container_name`), + /// not the container id, since the id can be stale or absent and the + /// name is what a later retry can still resolve. pub container: Option, /// The `triple-c-snapshot-{id}` image, if it could not be removed. pub image: Option, /// Named volumes (home, claude config) that could not be removed. pub volumes: Vec, + /// True once the leftovers above were durably recorded for automatic + /// retry on the next launch. False means the pending-cleanup record + /// itself could not be written — nothing will retry these, and the UI + /// must say so rather than promising a retry that will not happen. + /// Meaningless (and left at its default) when `is_clean()` is true. + pub retry_scheduled: bool, } impl ProjectRemovalReport { @@ -447,6 +456,22 @@ impl ProjectRemovalReport { } } +/// What `rebuild_project_container` (Reset) produced: the project as it +/// stands after restarting, and any volume Reset could not clear. +/// +/// Reset's contract is "back to a clean base image", so a leftover volume +/// here is reused as-is by the container this creates — the opposite of what +/// was asked for — and unlike [`ProjectRemovalReport`] there is no +/// pending-cleanup record for it: the project id survives Reset, so a later +/// Reset attempt can retry the same volume itself. +#[derive(Debug, Clone, Serialize)] +pub struct ProjectResetOutcome { + pub project: Project, + /// Volumes that survived Reset and were mounted into the new container + /// unchanged. + pub leftover_volumes: Vec, +} + /// Which AI model backend/provider the project uses. /// - `Anthropic`: Direct Anthropic API (user runs `claude login` inside the container) /// - `Bedrock`: AWS Bedrock with per-project AWS credentials diff --git a/app/src-tauri/src/storage/pending_cleanup.rs b/app/src-tauri/src/storage/pending_cleanup.rs index 0045ca6..f83bc75 100644 --- a/app/src-tauri/src/storage/pending_cleanup.rs +++ b/app/src-tauri/src/storage/pending_cleanup.rs @@ -10,13 +10,21 @@ //! `commands::project_commands::retry_pending_cleanup_logged`) and deletes //! the ones that fully succeed. //! -//! Same write-temp-then-rename shape as `projects.json` and the migration -//! store, and the same per-project-file layout as -//! `storage::migration_store` — a stuck cleanup record for one project must -//! never block the retry of another's. +//! **This record is written in the same instant its record in `projects.json` +//! is destroyed, and it is the only remaining handle on the leftover +//! resource** — which is a stronger claim on durability than an ordinary +//! write-temp-then-rename gives. `storage::migration_store::save` carries the +//! same reasoning for the migration state file: `fs::write` returns once the +//! bytes are in the page cache, and a rename over them is atomic with respect +//! to other readers, not to power loss. A crash in that window leaves the +//! rename applied and the data half-written, which [`list`] then treats as +//! unparseable and skips — reproducing the exact bug this module exists to +//! close, silently, with only a startup log line as evidence. So `save` here +//! takes the same `File::create` → `write_all` → `sync_all` → `rename` → +//! directory-sync shape `migration_store` does. use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -27,6 +35,11 @@ pub struct PendingCleanup { /// second lookup — the project record itself is already gone by the time /// this is read back. pub project_name: String, + /// The project's container, if it could not be removed. Named by its + /// deterministic `triple-c-{id}` name rather than the (possibly stale) + /// container id Docker handed out — Docker's remove-container API + /// accepts either, and the name is the one identifier guaranteed to still + /// resolve to the same container by the time a retry runs. pub container_id: Option, pub image: Option, pub volumes: Vec, @@ -63,29 +76,15 @@ fn sanitize(project_id: &str) -> String { .collect() } -fn path_for(project_id: &str) -> Result { - Ok(dir()?.join(format!("{}.json", sanitize(project_id)))) -} - /// Write (or overwrite) a project's pending-cleanup record. pub fn save(record: &PendingCleanup) -> Result<(), String> { - let path = path_for(&record.project_id)?; - let data = serde_json::to_string_pretty(record) - .map_err(|e| format!("Failed to serialize pending cleanup record: {}", e))?; - let tmp = path.with_extension("json.tmp"); - fs::write(&tmp, data).map_err(|e| format!("Failed to write pending cleanup record: {}", e))?; - fs::rename(&tmp, &path).map_err(|e| format!("Failed to commit pending cleanup record: {}", e)) + save_in(&dir()?, record) } /// Remove a project's pending-cleanup record. Missing is success — this is /// how a fully-succeeded retry (or a record that never existed) is expressed. pub fn clear(project_id: &str) -> Result<(), String> { - let path = path_for(project_id)?; - match fs::remove_file(&path) { - Ok(()) => Ok(()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), - Err(e) => Err(format!("Failed to remove pending cleanup record: {}", e)), - } + clear_in(&dir()?, project_id) } /// Every pending-cleanup record on disk. An unparseable file is logged and @@ -93,7 +92,50 @@ pub fn clear(project_id: &str) -> Result<(), String> { /// "one bad record can't wedge the rest" reasoning as the migration store. pub fn list() -> Vec { let Ok(dir) = dir() else { return Vec::new() }; - let Ok(entries) = fs::read_dir(&dir) else { return Vec::new() }; + list_in(&dir) +} + +fn path_in(dir: &Path, project_id: &str) -> PathBuf { + dir.join(format!("{}.json", sanitize(project_id))) +} + +/// Durable write: fsync the file before the rename, and fsync the directory +/// after it — see the module doc comment for why a plain +/// write-temp-then-rename is not enough here. Mirrors +/// `storage::migration_store::save`/`sync_dir`. +fn save_in(dir: &Path, record: &PendingCleanup) -> Result<(), String> { + let path = path_in(dir, &record.project_id); + let data = serde_json::to_string_pretty(record) + .map_err(|e| format!("Failed to serialize pending cleanup record: {}", e))?; + let tmp = path.with_extension("json.tmp"); + + { + use std::io::Write; + let mut file = fs::File::create(&tmp) + .map_err(|e| format!("Failed to write pending cleanup record: {}", e))?; + file.write_all(data.as_bytes()) + .map_err(|e| format!("Failed to write pending cleanup record: {}", e))?; + file.sync_all() + .map_err(|e| format!("Failed to flush pending cleanup record to disk: {}", e))?; + } + + fs::rename(&tmp, &path) + .map_err(|e| format!("Failed to commit pending cleanup record: {}", e))?; + sync_dir(&path); + Ok(()) +} + +fn clear_in(dir: &Path, project_id: &str) -> Result<(), String> { + let path = path_in(dir, project_id); + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("Failed to remove pending cleanup record: {}", e)), + } +} + +fn list_in(dir: &Path) -> Vec { + let Ok(entries) = fs::read_dir(dir) else { return Vec::new() }; entries .flatten() @@ -116,28 +158,50 @@ pub fn list() -> Vec { .collect() } +/// fsync the directory holding `path`, so a rename into it survives power +/// loss. Best effort only on the platforms where it is meaningless: Windows +/// has no directory handle to sync and errors on the attempt, so failure is +/// logged rather than propagated — the file's own `sync_all` above is what +/// carries the data. Mirrors `storage::migration_store::sync_dir`, which is +/// private to that module, so this is a small deliberate duplicate rather +/// than a shared dependency between two otherwise-independent stores. +fn sync_dir(path: &Path) { + let Some(dir) = path.parent() else { return }; + match fs::File::open(dir).and_then(|d| d.sync_all()) { + Ok(()) => {} + Err(e) => log::debug!( + "Could not fsync the pending-cleanup directory {}: {} — the record itself was flushed", + dir.display(), + e + ), + } +} + #[cfg(test)] mod tests { use super::*; - fn temp_data_dir(name: &str) -> PathBuf { - std::env::temp_dir().join(format!("triple-c-pending-cleanup-{}-{}", name, uuid::Uuid::new_v4().simple())) + fn temp_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "triple-c-pending-cleanup-{}-{}", + name, + uuid::Uuid::new_v4().simple() + )); + fs::create_dir_all(&dir).unwrap(); + dir } fn record(project_id: &str) -> PendingCleanup { PendingCleanup { project_id: project_id.to_string(), project_name: "Some Project".to_string(), - container_id: Some("abc123".to_string()), + container_id: Some("triple-c-abc".to_string()), image: Some("triple-c-snapshot-abc:latest".to_string()), volumes: vec!["triple-c-home-abc".to_string()], recorded_at: "2026-08-25T00:00:00Z".to_string(), } } - /// `list`/`save`/`clear` go through `dirs::data_dir()`, so these exercise - /// the pure parts directly against a temp directory rather than the real - /// one — same approach `migration_store`'s tests take for `sanitize`. #[test] fn project_ids_cannot_escape_the_pending_cleanup_directory() { assert_eq!(sanitize("../../etc/passwd"), "______etc_passwd"); @@ -161,26 +225,43 @@ mod tests { assert!(r.is_empty()); } - /// Save-then-list-then-clear against a real (temp) directory, bypassing - /// `dir()`'s hardcoded `dirs::data_dir()` join by writing/reading the - /// files directly the way `save`/`list` do internally. + /// Exercises the real `save_in`/`list_in`/`clear_in` — not a + /// re-implementation of their bodies — against a temp directory standing + /// in for `dir()`. #[test] fn a_saved_record_round_trips_and_clearing_removes_it() { - let dir = temp_data_dir("roundtrip"); - fs::create_dir_all(&dir).unwrap(); + let dir = temp_dir("roundtrip"); let rec = record("proj-1"); - let path = dir.join(format!("{}.json", sanitize(&rec.project_id))); - let data = serde_json::to_string_pretty(&rec).unwrap(); - fs::write(&path, data).unwrap(); + save_in(&dir, &rec).expect("save"); + let found = list_in(&dir); + assert_eq!(found.len(), 1); + assert_eq!(found[0].project_id, "proj-1"); + assert_eq!(found[0].volumes, vec!["triple-c-home-abc".to_string()]); - let loaded: PendingCleanup = - serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(loaded.project_id, "proj-1"); - assert_eq!(loaded.volumes, vec!["triple-c-home-abc".to_string()]); + clear_in(&dir, "proj-1").expect("clear"); + assert!(list_in(&dir).is_empty()); - fs::remove_file(&path).unwrap(); - assert!(!path.exists()); + fs::remove_dir_all(&dir).ok(); + } + + /// A second `save` for the same project overwrites rather than appending + /// — a retry that narrows the leftovers must not leave the old, wider + /// record behind it. + #[test] + fn saving_the_same_project_twice_overwrites_not_appends() { + let dir = temp_dir("overwrite"); + let mut rec = record("proj-1"); + save_in(&dir, &rec).expect("save"); + + rec.container_id = None; + rec.image = None; + save_in(&dir, &rec).expect("save again"); + + let found = list_in(&dir); + assert_eq!(found.len(), 1, "one file per project, not one per save"); + assert!(found[0].container_id.is_none()); + assert_eq!(found[0].volumes, vec!["triple-c-home-abc".to_string()]); fs::remove_dir_all(&dir).ok(); } @@ -188,30 +269,44 @@ mod tests { /// A record that fails to parse must not poison the rest of the listing. #[test] fn an_unparseable_record_is_skipped_not_fatal() { - let dir = temp_data_dir("corrupt"); - fs::create_dir_all(&dir).unwrap(); + let dir = temp_dir("corrupt"); fs::write(dir.join("bad.json"), "{ not json").unwrap(); - let good = record("proj-2"); - fs::write( - dir.join("proj-2.json"), - serde_json::to_string_pretty(&good).unwrap(), - ) - .unwrap(); + save_in(&dir, &record("proj-2")).expect("save"); - let mut found = Vec::new(); - for entry in fs::read_dir(&dir).unwrap().flatten() { - let path = entry.path(); - if path.extension().is_some_and(|ext| ext == "json") { - if let Ok(data) = fs::read_to_string(&path) { - if let Ok(r) = serde_json::from_str::(&data) { - found.push(r); - } - } - } - } + let found = list_in(&dir); assert_eq!(found.len(), 1); assert_eq!(found[0].project_id, "proj-2"); fs::remove_dir_all(&dir).ok(); } + + /// `list_in` must not pick up the `.json.tmp` staging file `save_in` + /// leaves behind if a crash lands between the write and the rename — the + /// whole point of the temp-then-rename dance is that only the renamed + /// file is ever a complete record. + #[test] + fn a_leftover_tmp_file_is_not_listed() { + let dir = temp_dir("tmp-leftover"); + fs::write(dir.join("proj-3.json.tmp"), "not a complete record").unwrap(); + assert!(list_in(&dir).is_empty()); + + fs::remove_dir_all(&dir).ok(); + } + + /// Clearing by project id must remove exactly the file that id maps to + /// under `sanitize`, and nothing else. + #[test] + fn clearing_one_project_does_not_touch_another() { + let dir = temp_dir("clear-scoped"); + save_in(&dir, &record("proj-a")).unwrap(); + save_in(&dir, &record("proj-b")).unwrap(); + + clear_in(&dir, "proj-a").unwrap(); + + let found = list_in(&dir); + assert_eq!(found.len(), 1); + assert_eq!(found[0].project_id, "proj-b"); + + fs::remove_dir_all(&dir).ok(); + } } diff --git a/app/src/components/projects/home/ProjectHome.tsx b/app/src/components/projects/home/ProjectHome.tsx index a83b04a..cde6832 100644 --- a/app/src/components/projects/home/ProjectHome.tsx +++ b/app/src/components/projects/home/ProjectHome.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useState } from "react"; import { useShallow } from "zustand/react/shallow"; -import type { ProjectRemovalReport } from "../../../lib/types"; +import { projectRemovalIsClean, type ProjectRemovalReport } from "../../../lib/types"; import { useAppState } from "../../../store/appState"; import { useProjectActions } from "../../../hooks/useProjectActions"; import { useProjects } from "../../../hooks/useProjects"; @@ -31,7 +31,15 @@ const TABS = [ export type ProjectHomeTabId = (typeof TABS)[number]["id"]; -/** Names what a `ProjectRemovalReport` says survived, for the leftover toast. */ +/** + * Names what a `ProjectRemovalReport` says survived, for the leftover toast. + * + * Worded as "could not confirm" rather than "is still on disk": the same + * report shape covers a genuine leftover (a locked volume) and a daemon that + * was simply unreachable at the time, in which case nothing was ever created + * and there is nothing to find — asserting certainty either way would be + * wrong in one of those cases. + */ function describeLeftovers(report: ProjectRemovalReport): string { const parts: string[] = []; if (report.container) parts.push("its container"); @@ -294,12 +302,22 @@ export default function ProjectHome({ projectId, active }: Props) { setConfirmRemove(false); try { const report = await remove(project.id); - if (report.container || report.image || report.volumes.length > 0) { - useAppState.getState().pushToast({ - kind: "info", - message: `“${project.name}” was removed, but some Docker resources are still on disk`, - detail: `Triple-C will retry removing ${describeLeftovers(report)} the next time it starts.`, - }); + if (!projectRemovalIsClean(report)) { + if (report.retry_scheduled) { + useAppState.getState().pushToast({ + kind: "info", + message: `“${project.name}” was removed, but Triple-C could not confirm all its Docker resources were cleaned up`, + detail: `Triple-C could not confirm ${describeLeftovers(report)} were removed. It will check again the next time it starts.`, + }); + } else { + // The pending-cleanup record itself failed to save — no + // retry will happen, so this must not promise one. + useAppState.getState().pushToast({ + kind: "error", + message: `“${project.name}” was removed, but its Docker resources could not be cleaned up`, + detail: `Triple-C could not confirm ${describeLeftovers(report)} were removed, and could not record this for a retry. You may need to remove them manually (\`docker rm\` / \`docker rmi\` / \`docker volume rm\`).`, + }); + } } } catch (e) { useAppState.getState().pushToast({ diff --git a/app/src/hooks/useProjectActions.ts b/app/src/hooks/useProjectActions.ts index 07ded95..ed66e2d 100644 --- a/app/src/hooks/useProjectActions.ts +++ b/app/src/hooks/useProjectActions.ts @@ -28,13 +28,14 @@ export function useProjectActions(project: Project) { ); const run = useCallback( - async (label: string, fn: () => Promise) => { + async (label: string, fn: () => Promise): Promise => { setBusy(true); setContainerProgress(project.id, null); try { - await fn(); + return await fn(); } catch (e) { fail(`${label} failed for “${project.name}”`, e); + return undefined; } finally { setContainerProgress(project.id, null); setBusy(false); @@ -54,8 +55,22 @@ export function useProjectActions(project: Project) { ); const handleReset = useCallback( - () => run("Reset", () => rebuild(project.id)), - [run, rebuild, project.id], + () => + run("Reset", async () => { + const outcome = await rebuild(project.id); + if (outcome.leftover_volumes.length > 0) { + const n = outcome.leftover_volumes.length; + pushToast({ + kind: "error", + message: `Reset for “${project.name}” could not fully clean up`, + detail: `${n === 1 ? "A volume" : `${n} volumes`} could not be removed, so the new \ +container may still contain data from before the reset. You may need to remove ${n === 1 ? "it" : "them"} \ +manually with \`docker volume rm\`.`, + }); + } + return outcome; + }), + [run, rebuild, project.id, project.name, pushToast], ); const openClaudeTerminal = useCallback(async () => { diff --git a/app/src/hooks/useProjects.ts b/app/src/hooks/useProjects.ts index 2ebcb5b..4b88c5e 100644 --- a/app/src/hooks/useProjects.ts +++ b/app/src/hooks/useProjects.ts @@ -136,9 +136,9 @@ export function useProjects() { const rebuild = useCallback( (id: string) => withOptimisticStatus(id, "starting", async () => { - const updated = await commands.rebuildProjectContainer(id); - updateProjectInList(updated); - return updated; + const outcome = await commands.rebuildProjectContainer(id); + updateProjectInList(outcome.project); + return outcome; }), [updateProjectInList, withOptimisticStatus], ); diff --git a/app/src/lib/tauri-commands.ts b/app/src/lib/tauri-commands.ts index 3264818..6389a4c 100644 --- a/app/src/lib/tauri-commands.ts +++ b/app/src/lib/tauri-commands.ts @@ -1,5 +1,5 @@ import { invoke } from "@tauri-apps/api/core"; -import type { Project, ProjectPath, ProjectRemovalReport, ContainerInfo, AppSettings, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; +import type { Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ContainerInfo, AppSettings, UpdateInfo, ImageUpdateInfo, FileEntry, FileContents, WebTerminalInfo, SttStatus, GatewayStatus, InstallOptions, ClaudeSession, ContainerCapabilities, ScheduledTask, ScheduledTaskInput, SchedulerNotification, AuthBridgeStatus, BrowserViewStatus, BrowserViewPopoutState, BrowserPageState, PlaywrightDetection, BrowserSetupOutcome, BrowserInstallTarget, ContainerStaleness, MigrationOptions, MigrationReport, MigrationState, ClearTokenOutcome, CaCertInfo, UploadOutcome } from "./types"; // Docker export const checkDocker = () => invoke("check_docker"); @@ -21,7 +21,7 @@ export const startProjectContainer = (projectId: string) => export const stopProjectContainer = (projectId: string) => invoke("stop_project_container", { projectId }); export const rebuildProjectContainer = (projectId: string) => - invoke("rebuild_project_container", { projectId }); + invoke("rebuild_project_container", { projectId }); export const reconcileProjectStatuses = () => invoke("reconcile_project_statuses"); diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index a84e85a..b6c2be8 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -78,12 +78,30 @@ export type ProjectStatus = | "error"; /** What `removeProject` could not delete. The project is removed from the - * sidebar either way; anything named here is recorded on the host and - * retried automatically the next time the app starts. */ + * sidebar either way. When `retry_scheduled` is true, anything named here + * was recorded on the host and will be retried automatically the next time + * the app starts; when false, the record itself could not be saved and + * nothing will retry it. `retry_scheduled` is meaningless when nothing was + * left behind. */ export interface ProjectRemovalReport { container: string | null; image: string | null; volumes: string[]; + retry_scheduled: boolean; +} + +/** True when a `ProjectRemovalReport` left nothing behind. Mirrors the + * Rust-side `ProjectRemovalReport::is_clean`. */ +export function projectRemovalIsClean(report: ProjectRemovalReport): boolean { + return !report.container && !report.image && report.volumes.length === 0; +} + +/** What Reset (`rebuildProjectContainer`) produced: the project as it stands + * after restarting, and any volume Reset could not clear — which is reused + * as-is by the new container instead of starting clean. */ +export interface ProjectResetOutcome { + project: Project; + leftover_volumes: string[]; } export type Backend =