Compare commits

..
5 Commits
Author SHA1 Message Date
jknapp 06254db3d4 Merge pull request 'Report and retry Docker resources remove_project could not delete' (#36) from fix/remove-project-cleanup-reporting into main
Build App / compute-version (push) Successful in 5s
Secret Scan / scan (push) Successful in 4s
Build App / build-macos (push) Successful in 2m40s
Build App / build-windows (push) Successful in 4m54s
Build App / build-linux (push) Successful in 5m35s
Build App / create-tag (push) Successful in 13s
Build App / sync-to-github (push) Successful in 13s
2026-08-27 16:59:57 +00:00
shadow-testandClaude Sonnet 5 61bdbc4a5b Close the crash-window gap and exec-session leak a third review found
Secret Scan / scan (push) Successful in 16s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 3s
Build App (Preview) / build-macos (pull_request) Successful in 2m37s
Build App (Preview) / build-windows (pull_request) Successful in 4m52s
Build App (Preview) / build-linux (pull_request) Successful in 6m17s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
A third Opus review pass confirmed round 2's fixes hold up, then found:

- The pending-cleanup record `remove_project` writes is fully durable
  (fsync'd); the projects_store.remove() that follows it is a plain
  fs::write with no fsync. A crash or power loss in that window — or that
  store write failing outright, beyond what the previous round's in-process
  rollback catches — leaves a record on disk naming a project
  projects.json still lists as present. The very next startup retry would
  then delete that project's container, snapshot image, and both volumes
  (including the one holding the OAuth credential and every session
  transcript) out from under a project the user still sees in the sidebar.
  retry_pending_cleanup_logged now takes the ProjectsStore and refuses to
  touch — clearing instead — any record whose project id still exists.
  Also stopped swallowing the round-2 rollback's own failure.
- Resolving the container through find_existing_container instead of
  project.container_id (round 2's stale-id fix) changed what drove
  close_sessions_for_container in remove_project and rebuild_project_
  container: sessions are now leaked when Docker is unreachable (nothing
  resolves, so nothing closes, and the project record is gone a moment
  later) and in the stale-id race itself (sessions were opened against the
  container that actually exists, not the id find_existing_container
  bypasses). Both functions now close sessions for the stored id
  unconditionally, and again for the resolved id if it differs.
- A pronoun-agreement bug in the no-retry removal toast ("remove them
  manually" for a single leftover) that was fixed one line above for verb
  agreement but not for the pronoun.

Also closed the test gaps the review named: the pending-cleanup
corrupt-record aside-move had no test, the Reset toast's leftover copy
was inline and untested (extracted to lib/resetOutcome.ts, mirroring
components/projects/home/removalReport.ts, with unit tests), and nothing
asserted rebuild()'s success path maps outcome.project into the list
rather than the whole outcome.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:47:45 -07:00
shadow-testandClaude Sonnet 5 439ef16f07 Fix two new bugs a second review found: stale container id, orphaned record
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m40s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 6m28s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A second Opus review of commit 2 found it had introduced real problems of
its own rather than just polish gaps:

- remove_project's "None or stale" container-id fallback only handled
  None. A stale id (the documented start-failure race in
  start_project_container_locked, where the old container is removed and
  the new one's id isn't persisted until after start_container succeeds)
  still 404'd on removal — now treated as success by commit 1's own fix —
  while the real container survived to block every volume removal with a
  409 forever, with nothing in the pending-cleanup record ever naming it.
  Both remove_project and rebuild_project_container now resolve the
  container via find_existing_container() unconditionally, matching every
  other container-destroying path in the codebase, and remove_project
  fails closed (records a leftover rather than silently skipping) if
  Docker itself can't be reached to check.
- remove_project could leave a pending-cleanup record for a project still
  live in projects.json: if the store's own save failed after the record
  was written, startup housekeeping would delete that project's container
  and volumes out from under it on the next launch. The record is now
  rolled back when the store write fails.
- rebuild_project_container (Reset) only surfaced a leftover volume, not a
  leftover snapshot image — the more serious failure, since the next
  container is built from that image whenever it exists, silently
  reviving the exact system layer Reset was asked to discard.
  ProjectResetOutcome now carries leftover_image too, and the toast's
  "run docker volume rm" advice is corrected: the new container has
  already remounted the volume by the time the toast renders, so that
  command would just hit the same conflict Reset did.

Also from the same pass: reworded a couple of log/toast lines that still
asserted resources were "still present" when the daemon-unreachable case
covered by the same code path can't actually confirm that; fixed a
singular/verb mismatch in the leftover toast text; moved an unparseable
pending-cleanup record aside instead of re-warning about it forever; and
added a debug log when a record's recorded_at can't be parsed, so aging
never silently no-ops.

Pulled describeLeftovers/leftoverVerb out of ProjectHome.tsx into their
own module with unit tests, and added tests for the recorded_at staleness
check — the previous commit's equivalent logic had none.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:09:46 -07:00
shadow-testandClaude Sonnet 5 d8bb5ab262 Address review findings: durability, stale container ids, honest toasts
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 6m18s
Build App (Preview) / build-linux (pull_request) Successful in 7m48s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
An Opus review of the previous commit found several real gaps:

- pending_cleanup::save used plain write-temp-then-rename, unlike
  migration_store's fsync'd write it claimed to mirror — a crash in that
  window left a truncated record that list() would skip forever, silently
  reproducing the exact bug this module exists to fix. Now matches
  migration_store's File::create/write_all/sync_all/rename/sync_dir shape,
  and the tests exercise the real save/list/clear functions against a temp
  dir instead of re-implementing their bodies inline.
- remove_project and rebuild_project_container only ever looked at
  project.container_id, unlike every other container-destroying path in the
  codebase, which falls back to find_existing_container for exactly this
  race (a crash between creating a container and persisting its id). A miss
  here left a container that then blocked every subsequent volume removal
  with a 409, forever. Both now resolve the same way the rest of the
  codebase does, and record the container by its deterministic name rather
  than its id so a retry still has something that resolves.
- remove_project's toast promised an automatic retry unconditionally, even
  when writing the pending-cleanup record itself failed (the one case
  where nothing will actually retry). ProjectRemovalReport now carries
  retry_scheduled, and the UI is honest about which case it's in.
- remove_volumes_by_name now retries once after a short delay on a 409,
  since Docker releasing a volume's mount reference right after its
  container is removed is not always instantaneous, and this is exactly
  the sequence remove_project runs.
- rebuild_project_container (Reset) returns ProjectResetOutcome so the UI
  can warn when Reset could not fully clear a project's volumes, instead
  of only logging it — the new container silently reuses old data
  otherwise, which is what Reset promises not to do.
- retry_pending_cleanup_logged escalates a record's log level after it has
  failed for a week, since recorded_at was otherwise write-only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:36:27 -07:00
shadow-testandClaude Sonnet 5 4827170715 Report and retry Docker resources remove_project could not delete
Secret Scan / scan (push) Successful in 10s
Build App (Preview) / compute-version (pull_request) Successful in 7s
Secret Scan / scan (pull_request) Successful in 8s
Build App (Preview) / create-release (pull_request) Successful in 5s
Build App (Preview) / build-linux (pull_request) Successful in 6m5s
Build App (Preview) / build-macos (pull_request) Successful in 2m45s
Build App (Preview) / build-windows (pull_request) Successful in 5m42s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
remove_project_volumes always returned Ok(()) regardless of what actually
happened, making the `if let Err(e)` guarding it at every call site dead
code. remove_project then dropped the project record unconditionally, so a
volume, image or container that failed to delete became permanently
unreachable — confirmed against a real orphaned volume pair found in the
wild (fixes #31).

remove_project_volumes/remove_snapshot_image/remove_container now report
what they could not remove (treating "already gone" as success rather than
a leftover), remove_project surfaces this to the user via a toast, and
before dropping the project record it writes a pending-cleanup record that
startup housekeeping retries automatically on the next launch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:18:41 -07:00
16 changed files with 1181 additions and 48 deletions
+372 -19
View File
@@ -2,7 +2,7 @@ use tauri::{Emitter, State};
use crate::commands::aws_commands; use crate::commands::aws_commands;
use crate::docker; use crate::docker;
use crate::models::{container_config, AppSettings, Backend, BedrockAuthMethod, Project, ProjectPath, ProjectStatus}; use crate::models::{container_config, AppSettings, Backend, BedrockAuthMethod, Project, ProjectPath, ProjectRemovalReport, ProjectResetOutcome, ProjectStatus};
use crate::storage::secure; use crate::storage::secure;
use crate::AppState; use crate::AppState;
@@ -696,7 +696,7 @@ pub async fn add_project(
pub async fn remove_project( pub async fn remove_project(
project_id: String, project_id: String,
state: State<'_, AppState>, state: State<'_, AppState>,
) -> Result<(), String> { ) -> Result<ProjectRemovalReport, String> {
// **H-2: the only writer of these three categories that held nothing.** // **H-2: the only writer of these three categories that held nothing.**
// This purges migration artifacts, removes `triple-c-snapshot-{id}` and // This purges migration artifacts, removes `triple-c-snapshot-{id}` and
// both named volumes — and a compaction resolves that same tag when its // both named volumes — and a compaction resolves that same tag when its
@@ -722,12 +722,76 @@ pub async fn remove_project(
// holding an entire snapshot image that nothing will ever reference again. // holding an entire snapshot image that nothing will ever reference again.
crate::commands::migration_commands::purge_migration_artifacts(&project_id).await; crate::commands::migration_commands::purge_migration_artifacts(&project_id).await;
// Stop and remove container if it exists // Stop and remove container if it exists. Everything named in `report`
if let Some(ref project) = state.projects_store.get(&project_id) { // below is what will be unreachable the moment this function drops the
if let Some(ref container_id) = project.container_id { // project record — see [`ProjectRemovalReport`] and
state.exec_manager.close_sessions_for_container(container_id).await; // `storage::pending_cleanup`, which is what makes it reachable anyway.
let mut report = ProjectRemovalReport::default();
let existing_project = state.projects_store.get(&project_id);
if let Some(ref project) = existing_project {
// Resolved via `find_existing_container` unconditionally rather than
// trusting `project.container_id` — that field can be *stale*, not
// just absent: `start_project_container_locked`'s recreate path
// removes the old container, creates a new one, and does not persist
// the new id until after `start_container` succeeds, so a start
// failure in between (a missing `/dev/net/tun`, an image that exits
// immediately) leaves the stored id pointing at a container that no
// longer exists while a live one sits under the same deterministic
// name. Removing by a stale id then 404s — success as far as Docker
// is concerned — while the real container survives to block every
// subsequent volume removal with a 409, with nothing in the report
// ever naming it. `find_existing_container` is what every other
// destroyer of a project's container already resolves through
// (`start_project_container`, migration's recreate paths) for this
// exact reason.
//
// A `Docker unreachable` error here is treated as "assume a
// container is still there" rather than "assume none is", matching
// `remove_volumes_by_name`'s fail-closed handling of the same
// situation — the alternative silently drops the one resource most
// likely to block everything else if it does exist.
//
// Exec sessions are closed for `project.container_id` unconditionally,
// before the lookup above and regardless of whether it succeeds —
// that is host-side state with no Docker dependency, so it must not
// wait on a daemon that might not answer. Resolving through
// `find_existing_container` instead of using it directly would leave
// these open in exactly the two cases this whole change exists to
// handle: Docker unreachable (no id resolved, no way to ever close
// them again once the project record is gone) and the stale-id race
// (sessions were opened against the container that actually exists,
// which is what gets resolved below, not the stored id).
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 {
Ok(found) => found,
Err(e) => {
log::warn!(
"Could not check for an existing container for project {}: {}",
project_id, e
);
report.container = Some(project.container_name());
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;
}
let _ = docker::stop_container(container_id).await; let _ = docker::stop_container(container_id).await;
let _ = docker::remove_container(container_id).await; if let Err(e) = docker::remove_container(container_id).await {
log::warn!(
"Failed to remove container {} for project {}: {}",
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
// call accepts either), and it is what `container_ref` above
// falls back to finding in the first place.
report.container = Some(project.container_name());
}
} }
// Legacy MCP cleanup (pre-MCP-removal installs): drop any leftover MCP // Legacy MCP cleanup (pre-MCP-removal installs): drop any leftover MCP
@@ -738,10 +802,9 @@ pub async fn remove_project(
// Clean up the snapshot image + volumes // Clean up the snapshot image + volumes
if let Err(e) = docker::remove_snapshot_image(project).await { if let Err(e) = docker::remove_snapshot_image(project).await {
log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e); log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e);
report.image = Some(docker::get_snapshot_image_name(project));
} }
if let Err(e) = docker::remove_project_volumes(project).await { report.volumes = docker::remove_project_volumes(project).await;
log::warn!("Failed to remove project volumes for project {}: {}", project_id, e);
}
} }
// Clean up keychain secrets for this project // Clean up keychain secrets for this project
@@ -749,7 +812,216 @@ pub async fn remove_project(
log::warn!("Failed to delete keychain secrets for project {}: {}", project_id, e); log::warn!("Failed to delete keychain secrets for project {}: {}", project_id, e);
} }
state.projects_store.remove(&project_id) if !report.is_clean() {
let record = crate::storage::pending_cleanup::PendingCleanup {
project_id: project_id.clone(),
project_name: existing_project.map(|p| p.name).unwrap_or_default(),
container_id: report.container.clone(),
image: report.image.clone(),
volumes: report.volumes.clone(),
recorded_at: chrono::Utc::now().to_rfc3339(),
};
match crate::storage::pending_cleanup::save(&record) {
Ok(()) => {
report.retry_scheduled = true;
log::warn!(
"Project {} removed; could not confirm these Docker resources were removed: \
{:?} — recorded for automatic retry on next launch",
project_id, report
);
}
Err(e) => {
report.retry_scheduled = false;
log::error!(
"Project {} removed; could not confirm these Docker resources were removed \
({:?}), and the pending-cleanup record could not be written ({}) — nothing \
will retry removing them",
project_id, report, e
);
}
}
}
// The pending-cleanup record above must not outlive the project record it
// describes: if the store's own write fails (full disk, permissions) the
// project is still on disk and will reload on the next launch, but the
// record would tell startup housekeeping to delete its container and
// volumes out from under it. Roll the record back rather than leaving
// that mismatch for the retry to discover the hard way.
//
// This is a second, narrower line of defence, not the only one — a crash
// between the `save` above and the `remove` below leaves exactly the same
// mismatch with no error for either side to catch, which is why
// `retry_pending_cleanup_logged` also refuses to act on a record whose
// project is still listed in `projects.json`. Belt and suspenders: a
// caught failure here is handled immediately rather than waiting for the
// next launch to notice.
if let Err(e) = state.projects_store.remove(&project_id) {
if !report.is_clean() {
if let Err(clear_err) = crate::storage::pending_cleanup::clear(&project_id) {
log::error!(
"Project {} was not removed ({}), and its pending-cleanup record could not \
be rolled back either ({}) — it will name this still-live project until \
startup housekeeping's own guard clears it",
project_id, e, clear_err
);
}
}
return Err(e);
}
Ok(report)
}
/// Retry every pending-cleanup record left behind by a [`remove_project`]
/// that could not finish. Run once at startup alongside the other reapers
/// (see `lib.rs`'s "Startup disk housekeeping" block) — never on a timer and
/// never blocking anything, since a locked volume or an in-use image can sit
/// unresolved for an arbitrary amount of time and the daemon may not even be
/// up yet.
///
/// Not a `#[tauri::command]`: nothing in the UI surfaces this list yet
/// (deliberately — see `SnapshotSweepReport`'s doc comment for the same
/// reasoning), so there is no IPC contract to keep. A record that still has
/// leftovers after this is written back so the next run does not lose track
/// of what changed; one that is now empty is deleted.
///
/// Takes the `ProjectsStore` so it can refuse to touch a project that is
/// still live: `remove_project` writes a pending-cleanup record durably
/// (fsync'd) *before* it asks the store to drop the project, and that
/// store write is a plain `fs::write` with no fsync of its own. A crash or
/// power loss in the gap between the two — or the store write failing
/// outright, on top of the round-2 fix that only rolls the record back when
/// that failure is caught in-process — can leave a record on disk pointing
/// at a project `projects.json` still lists. Without this check, the very
/// first retry after such a crash deletes that project's container,
/// snapshot image and *both volumes, including the one holding the OAuth
/// credential and every session transcript*, out from under a project the
/// user still sees in the sidebar. A record whose project still exists is
/// therefore always stale — cleared without touching Docker, not retried.
pub async fn retry_pending_cleanup_logged(projects_store: &crate::storage::projects_store::ProjectsStore) {
let records = crate::storage::pending_cleanup::list();
if records.is_empty() {
return;
}
let mut cleaned = 0usize;
let mut still_pending = 0usize;
for mut record in records {
if projects_store.get(&record.project_id).is_some() {
log::warn!(
"Pending cleanup record for project {} ({}) names a project that still exists — \
clearing the record without touching Docker rather than risk deleting a live \
project's resources",
record.project_id, record.project_name
);
if let Err(e) = crate::storage::pending_cleanup::clear(&record.project_id) {
log::error!(
"Could not clear the stale pending-cleanup record for still-live project {} \
({}): {}",
record.project_id, record.project_name, e
);
}
continue;
}
if let Some(container_id) = record.container_id.take() {
match docker::remove_container(&container_id).await {
Ok(()) => {}
Err(e) => {
log::warn!(
"Pending cleanup: still could not remove container {} for project {} \
({}): {}",
container_id, record.project_id, record.project_name, e
);
record.container_id = Some(container_id);
}
}
}
if let Some(image) = record.image.take() {
match docker::remove_image_by_name(&image).await {
Ok(()) => {}
Err(e) => {
log::warn!(
"Pending cleanup: still could not remove image {} for project {} ({}): {}",
image, record.project_id, record.project_name, e
);
record.image = Some(image);
}
}
}
if !record.volumes.is_empty() {
record.volumes = docker::remove_volumes_by_name(&record.volumes).await;
}
if record.is_empty() {
if let Err(e) = crate::storage::pending_cleanup::clear(&record.project_id) {
log::warn!(
"Pending cleanup for project {} ({}) finished but the record could not be \
deleted: {}",
record.project_id, record.project_name, e
);
}
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.
match pending_cleanup_is_stale(&record.recorded_at, chrono::Utc::now()) {
Some(true) => {
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
);
}
Some(false) => {}
// Silent otherwise would mean a record with a corrupted
// timestamp never escalates and nothing says why.
None => log::debug!(
"Pending cleanup record for project {} ({}) has an unreadable recorded_at \
({:?}) — its age cannot be tracked",
record.project_id, record.project_name, record.recorded_at
),
}
if let Err(e) = crate::storage::pending_cleanup::save(&record) {
log::warn!(
"Could not update pending cleanup record for project {} ({}): {}",
record.project_id, record.project_name, e
);
}
}
}
log::info!(
"Pending cleanup retry: {} project(s) fully cleaned up, {} still have leftovers",
cleaned, still_pending
);
}
/// 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;
/// Whether a pending-cleanup record's `recorded_at` is older than
/// [`PENDING_CLEANUP_STALE_AFTER_DAYS`], measured against `now`. `None` means
/// the timestamp could not be parsed at all — a corrupted or (hypothetically)
/// hand-edited record — which callers must not silently treat as "not stale"
/// without saying why. `now` is a parameter rather than read internally so
/// this is testable without a live clock.
fn pending_cleanup_is_stale(recorded_at: &str, now: chrono::DateTime<chrono::Utc>) -> Option<bool> {
let recorded = chrono::DateTime::parse_from_rfc3339(recorded_at)
.ok()?
.with_timezone(&chrono::Utc);
Some(now.signed_duration_since(recorded) > chrono::Duration::days(PENDING_CLEANUP_STALE_AFTER_DAYS))
} }
#[tauri::command] #[tauri::command]
@@ -1226,7 +1498,7 @@ pub async fn rebuild_project_container(
project_id: String, project_id: String,
app_handle: tauri::AppHandle, app_handle: tauri::AppHandle,
state: State<'_, AppState>, state: State<'_, AppState>,
) -> Result<Project, String> { ) -> Result<ProjectResetOutcome, String> {
// Reset deletes both volumes and the snapshot image. Doing that while a // 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 // 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 // orphan migration record pointing at images that no longer exist — and
@@ -1253,25 +1525,58 @@ pub async fn rebuild_project_container(
// `start_project_container` below re-arms it against the new one. // `start_project_container` below re-arms it against the new one.
state.auth_bridge.stop(&project_id).await; state.auth_bridge.stop(&project_id).await;
// Remove existing container // Remove existing container. Resolved via `find_existing_container`
if let Some(ref container_id) = project.container_id { // unconditionally, not `project.container_id` — see the long comment in
state.exec_manager.close_sessions_for_container(container_id).await; // `remove_project` for why that field can be stale, not just absent. 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. Unlike `remove_project`'s best-effort
// handling of the same lookup failing, `?` here aborts Reset outright:
// every step after this one needs Docker too, so there is no useful
// partial progress to make without it.
// Closed for the stored id unconditionally, then again for the resolved
// one if it differs — see the matching comment in `remove_project` for
// why the stale-id race can leave sessions open under either identity.
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 _ = docker::stop_container(container_id).await; let _ = docker::stop_container(container_id).await;
docker::remove_container(container_id).await?; docker::remove_container(container_id).await?;
state.projects_store.set_container_id(&project_id, None)?; state.projects_store.set_container_id(&project_id, None)?;
} }
// Remove snapshot image + volumes so Reset creates from the clean base image // Remove snapshot image + volumes so Reset creates from the clean base
// image. Both leftovers are surfaced, not just logged — an image that
// survives is the more serious of the two, since
// `start_project_container_locked` below builds from
// `triple-c-snapshot-{id}:latest` whenever it exists, so a leftover image
// means Reset silently rebuilds the exact system layer it promised to
// 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 { if let Err(e) = docker::remove_snapshot_image(&project).await {
log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e); log::warn!("Failed to remove snapshot image for project {}: {}", project_id, e);
leftover_image = Some(docker::get_snapshot_image_name(&project));
} }
if let Err(e) = docker::remove_project_volumes(&project).await { let leftover_volumes = docker::remove_project_volumes(&project).await;
log::warn!("Failed to remove project volumes for project {}: {}", project_id, e); if leftover_image.is_some() || !leftover_volumes.is_empty() {
log::warn!(
"Reset for project {} could not fully clean up — image: {:?}, volumes: {:?} — the \
new container may be built from, or reuse, old contents instead of starting clean",
project_id, leftover_image, leftover_volumes
);
} }
// Start fresh. The locked variant, because `_guard` above is this project's // Start fresh. The locked variant, because `_guard` above is this project's
// claim and the public command would be refused by it. // 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_image, leftover_volumes })
} }
/// Reconcile project statuses against actual Docker container state. /// Reconcile project statuses against actual Docker container state.
@@ -1379,6 +1684,54 @@ fn default_docker_socket() -> String {
mod tests { mod tests {
use super::*; use super::*;
// ── Pending-cleanup aging ────────────────────────────────────────────
#[test]
fn a_record_younger_than_the_threshold_is_not_stale() {
let now = "2026-08-25T00:00:00Z".parse().unwrap();
let recorded_at = "2026-08-19T00:00:00Z"; // 6 days before `now`
assert_eq!(pending_cleanup_is_stale(recorded_at, now), Some(false));
}
#[test]
fn a_record_exactly_at_the_threshold_is_not_yet_stale() {
let now = "2026-08-25T00:00:00Z".parse().unwrap();
let recorded_at = "2026-08-18T00:00:00Z"; // exactly 7 days before `now`
assert_eq!(
pending_cleanup_is_stale(recorded_at, now),
Some(false),
"the boundary itself must not already read as stale"
);
}
#[test]
fn a_record_older_than_the_threshold_is_stale() {
let now = "2026-08-25T00:00:00Z".parse().unwrap();
let recorded_at = "2026-08-17T00:00:00Z"; // 8 days before `now`
assert_eq!(pending_cleanup_is_stale(recorded_at, now), Some(true));
}
/// A clock that ran fast when the record was written leaves a timestamp
/// in the future. This must read as "not stale" rather than underflow or
/// panic — `signed_duration_since` returns a negative `Duration` here,
/// which compares less than any positive threshold correctly.
#[test]
fn a_timestamp_in_the_future_is_not_stale() {
let now = "2026-08-25T00:00:00Z".parse().unwrap();
let recorded_at = "2026-08-26T00:00:00Z"; // one day after `now`
assert_eq!(pending_cleanup_is_stale(recorded_at, now), Some(false));
}
/// A corrupted or hand-edited `recorded_at` must not silently read as
/// "not stale" through some default — callers need to be able to tell
/// "definitely not stale" apart from "cannot tell".
#[test]
fn an_unparseable_recorded_at_reports_unknown_rather_than_not_stale() {
let now = "2026-08-25T00:00:00Z".parse().unwrap();
assert_eq!(pending_cleanup_is_stale("not a timestamp", now), None);
assert_eq!(pending_cleanup_is_stale("", now), None);
}
fn path(host: &str, mount: &str) -> ProjectPath { fn path(host: &str, mount: &str) -> ProjectPath {
ProjectPath { ProjectPath {
host_path: host.to_string(), host_path: host.to_string(),
+107 -17
View File
@@ -1935,7 +1935,7 @@ pub async fn remove_container(container_id: &str) -> Result<(), String> {
"Removing container {} (v=false: named volumes such as claude config are preserved)", "Removing container {} (v=false: named volumes such as claude config are preserved)",
container_id container_id
); );
docker match docker
.remove_container( .remove_container(
container_id, container_id,
Some(RemoveContainerOptions { Some(RemoveContainerOptions {
@@ -1945,7 +1945,17 @@ pub async fn remove_container(container_id: &str) -> Result<(), String> {
}), }),
) )
.await .await
.map_err(|e| format!("Failed to remove container: {}", e)) {
Ok(()) => Ok(()),
// Already gone is the outcome this call wants, not a failure — a
// caller retrying a leftover from a previous, partially-failed removal
// (see `remove_project`) must not be told it failed forever just
// because a *different* attempt already succeeded.
Err(bollard::errors::Error::DockerResponseServerError {
status_code: 404, ..
}) => Ok(()),
Err(e) => Err(format!("Failed to remove container: {}", e)),
}
} }
/// Return the snapshot image name for a project. /// Return the snapshot image name for a project.
@@ -3496,13 +3506,27 @@ async fn rewrite_image_without_secrets(
} }
/// Remove the snapshot image for a project (used on Reset / project removal). /// Remove the snapshot image for a project (used on Reset / project removal).
///
/// A project that never started never built a snapshot, so "no such image" is
/// the ordinary case, not a failure — it is treated the same as success and
/// logged at most at `info`. A real failure (the image is in use, a
/// permission error, the daemon dropped the connection) is the one thing this
/// returns `Err` for, and callers must not throw that away: see the
/// `remove_project` doc comment on `ProjectRemovalReport` for why an
/// unreported failure here used to make the resource unreachable forever.
pub async fn remove_snapshot_image(project: &Project) -> Result<(), String> { pub async fn remove_snapshot_image(project: &Project) -> Result<(), String> {
let docker = get_docker()?; remove_image_by_name(&get_snapshot_image_name(project)).await
let image_name = get_snapshot_image_name(project); }
docker /// Remove a Docker image by name/tag, treating "does not exist" as success.
/// Shared by [`remove_snapshot_image`] and the pending-cleanup retry, which
/// only has the image name (the project record is already gone by then).
pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
let docker = get_docker()?;
match docker
.remove_image( .remove_image(
&image_name, image_name,
Some(RemoveImageOptions { Some(RemoveImageOptions {
force: true, force: true,
noprune: false, noprune: false,
@@ -3510,25 +3534,91 @@ pub async fn remove_snapshot_image(project: &Project) -> Result<(), String> {
None, None,
) )
.await .await
.map_err(|e| format!("Failed to remove snapshot image {}: {}", image_name, e))?; {
Ok(_) => {
log::info!("Removed snapshot image {}", image_name); log::info!("Removed snapshot image {}", image_name);
Ok(()) Ok(())
}
Err(bollard::errors::Error::DockerResponseServerError {
status_code: 404, ..
}) => Ok(()),
Err(e) => Err(format!("Failed to remove snapshot image {}: {}", image_name, e)),
}
} }
/// Remove both named volumes for a project (used on Reset / project removal). /// Remove both named volumes for a project (used on Reset / project removal).
pub async fn remove_project_volumes(project: &Project) -> Result<(), String> { ///
let docker = get_docker()?; /// Returns the names of volumes that still exist afterwards — empty means
for vol in [ /// both are gone (removed here, or never created). This used to always
/// return `Ok(())` regardless of what actually happened, which made the
/// `if let Err(e)` at every call site unreachable by construction; see
/// triple-c#31. A volume Docker reports as simply not existing is not a
/// leftover and is not included.
pub async fn remove_project_volumes(project: &Project) -> Vec<String> {
remove_volumes_by_name(&[
home_volume_name(&project.id), home_volume_name(&project.id),
config_volume_name(&project.id), config_volume_name(&project.id),
] { ])
match docker.remove_volume(&vol, None).await { .await
}
/// Remove a set of named volumes, treating "does not exist" as success.
/// Returns the names that still exist afterwards — empty means every one is
/// gone (removed here, or never created).
///
/// Shared by [`remove_project_volumes`] and the pending-cleanup retry, the
/// latter calling this with whatever the former could not remove the first
/// time. Used to always report success regardless of what actually happened,
/// which made every `if let Err(e)` at its call sites unreachable by
/// construction; see triple-c#31.
pub async fn remove_volumes_by_name(names: &[String]) -> Vec<String> {
let docker = match get_docker() {
Ok(d) => d,
Err(e) => {
// Can't reach the daemon to even try, so nothing here can be
// confirmed removed. Reporting all as leftover is the safe
// direction: worst case a later retry finds them already gone.
log::warn!("Could not remove volumes {:?}: {}", names, e);
return names.to_vec();
}
};
let mut leftover = Vec::new();
for vol in names {
match remove_one_volume_with_retry(&docker, vol).await {
Ok(_) => log::info!("Removed volume {}", vol), Ok(_) => log::info!("Removed volume {}", vol),
Err(e) => log::warn!("Failed to remove volume {} (may not exist): {}", vol, e), Err(bollard::errors::Error::DockerResponseServerError {
status_code: 404, ..
}) => {}
Err(e) => {
log::warn!("Failed to remove volume {}: {}", vol, e);
leftover.push(vol.clone());
}
} }
} }
Ok(()) 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 /// Check whether the existing container's configuration still matches the
+10
View File
@@ -250,6 +250,7 @@ pub fn run() {
// an image open and the sweep will not force; pins are untagged // an image open and the sweep will not force; pins are untagged
// second so the images they were holding are dangling by the time // second so the images they were holding are dangling by the time
// the sweep lists them; the sweep runs last and collects both. // the sweep lists them; the sweep runs last and collects both.
let projects_store_for_cleanup = projects_store_setup.clone();
tauri::async_runtime::spawn(async move { tauri::async_runtime::spawn(async move {
crate::docker::reap_probe_containers().await; crate::docker::reap_probe_containers().await;
let reaped = crate::docker::reap_stale_migration_pins().await; let reaped = crate::docker::reap_stale_migration_pins().await;
@@ -257,6 +258,15 @@ pub fn run() {
log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped); log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped);
} }
crate::docker::sweep_orphaned_snapshots_logged("startup").await; crate::docker::sweep_orphaned_snapshots_logged("startup").await;
// A container/image/volume `remove_project` could not delete
// is recorded rather than lost — see triple-c#31 — and this is
// the only place anything ever retries it. Takes the store so
// it can refuse to touch a project that turns out to still be
// live — see the long comment on the function itself.
crate::commands::project_commands::retry_pending_cleanup_logged(
&projects_store_for_cleanup,
)
.await;
}); });
// Auto-start web terminal server if enabled in settings // Auto-start web terminal server if enabled in settings
+74
View File
@@ -422,6 +422,61 @@ pub enum ProjectStatus {
Error, Error,
} }
/// What `remove_project` could not delete, named so the UI can say so instead
/// of reporting a clean removal that was not one.
///
/// The project record is dropped from `projects.json` regardless — see the
/// long comment on `remove_project` for why refusing is not the answer — but
/// anything named here is also written to a pending-cleanup record that
/// startup housekeeping retries, so it stays reachable after the project it
/// belonged to no longer exists.
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
pub struct ProjectRemovalReport {
/// 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<String>,
/// The `triple-c-snapshot-{id}` image, if it could not be removed.
pub image: Option<String>,
/// Named volumes (home, claude config) that could not be removed.
pub volumes: Vec<String>,
/// 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 {
/// True when nothing was left behind.
pub fn is_clean(&self) -> bool {
self.container.is_none() && self.image.is_none() && self.volumes.is_empty()
}
}
/// What `rebuild_project_container` (Reset) produced: the project as it
/// stands after restarting, and anything Reset could not clear.
///
/// Reset's contract is "back to a clean base image", so a leftover volume or
/// image here is reused/rebuilt-from as-is by the container this creates —
/// the opposite of what was asked for — and unlike [`ProjectRemovalReport`]
/// there is no pending-cleanup record for either: the project id survives
/// Reset, so a later Reset attempt can retry them itself.
#[derive(Debug, Clone, Serialize)]
pub struct ProjectResetOutcome {
pub project: Project,
/// The `triple-c-snapshot-{id}` image, if Reset could not remove it. The
/// more serious of the two leftovers here: the new container is created
/// from this image whenever it exists, so a surviving image means Reset
/// silently rebuilt the exact system layer it was asked to discard.
pub leftover_image: Option<String>,
/// Volumes that survived Reset and were mounted into the new container
/// unchanged.
pub leftover_volumes: Vec<String>,
}
/// Which AI model backend/provider the project uses. /// Which AI model backend/provider the project uses.
/// - `Anthropic`: Direct Anthropic API (user runs `claude login` inside the container) /// - `Anthropic`: Direct Anthropic API (user runs `claude login` inside the container)
/// - `Bedrock`: AWS Bedrock with per-project AWS credentials /// - `Bedrock`: AWS Bedrock with per-project AWS credentials
@@ -650,6 +705,25 @@ impl Project {
mod tests { mod tests {
use super::*; use super::*;
// ── ProjectRemovalReport ────────────────────────────────────────────────
#[test]
fn a_report_is_clean_only_with_nothing_left_behind() {
assert!(ProjectRemovalReport::default().is_clean());
let mut r = ProjectRemovalReport::default();
r.container = Some("abc123".to_string());
assert!(!r.is_clean(), "a leftover container must not read as clean");
let mut r = ProjectRemovalReport::default();
r.image = Some("triple-c-snapshot-x:latest".to_string());
assert!(!r.is_clean(), "a leftover image must not read as clean");
let mut r = ProjectRemovalReport::default();
r.volumes.push("triple-c-home-x".to_string());
assert!(!r.is_clean(), "a leftover volume must not read as clean");
}
// ── Custom environment variable names ───────────────────────────────── // ── Custom environment variable names ─────────────────────────────────
#[test] #[test]
+1
View File
@@ -1,4 +1,5 @@
pub mod migration_store; pub mod migration_store;
pub mod pending_cleanup;
pub mod projects_store; pub mod projects_store;
pub mod secure; pub mod secure;
pub mod settings_store; pub mod settings_store;
@@ -0,0 +1,349 @@
//! Host-side record of Docker resources `remove_project` could not delete.
//!
//! `remove_project` drops a project's id from `projects.json` unconditionally
//! — see the comment on `ProjectRemovalReport` — so once that happens nothing
//! in the app can name the leftover container, image or volume again by any
//! path a user can reach. This is what keeps it reachable anyway: one JSON
//! file per affected project under `<data_dir>/triple-c/pending-cleanup/`,
//! written *before* the project record is dropped. Startup housekeeping
//! retries every record on the next launch (see
//! `commands::project_commands::retry_pending_cleanup_logged`) and deletes
//! the ones that fully succeed.
//!
//! **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::{Path, PathBuf};
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PendingCleanup {
pub project_id: String,
/// Kept only so a log line or a future UI can name the project without a
/// 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<String>,
pub image: Option<String>,
pub volumes: Vec<String>,
pub recorded_at: String,
}
impl PendingCleanup {
/// True once nothing named here still needs to be removed.
pub fn is_empty(&self) -> bool {
self.container_id.is_none() && self.image.is_none() && self.volumes.is_empty()
}
}
/// `<data_dir>/triple-c/pending-cleanup`, created on demand.
fn dir() -> Result<PathBuf, String> {
let dir = dirs::data_dir()
.ok_or_else(|| {
"Could not determine data directory. Set XDG_DATA_HOME on Linux.".to_string()
})?
.join("triple-c")
.join("pending-cleanup");
fs::create_dir_all(&dir)
.map_err(|e| format!("Failed to create pending-cleanup directory: {}", e))?;
Ok(dir)
}
/// Project ids are UUIDs, but they arrive over IPC, so refuse to let one steer
/// the write anywhere but the pending-cleanup directory. Mirrors
/// `storage::migration_store::sanitize`.
fn sanitize(project_id: &str) -> String {
project_id
.chars()
.map(|c| if c.is_ascii_alphanumeric() || c == '-' || c == '_' { c } else { '_' })
.collect()
}
/// Write (or overwrite) a project's pending-cleanup record.
pub fn save(record: &PendingCleanup) -> Result<(), String> {
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> {
clear_in(&dir()?, project_id)
}
/// Every pending-cleanup record on disk. An unparseable file is logged and
/// skipped rather than blocking every other project's retry — the same
/// "one bad record can't wedge the rest" reasoning as the migration store.
pub fn list() -> Vec<PendingCleanup> {
let Ok(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<PendingCleanup> {
let Ok(entries) = fs::read_dir(dir) else { return Vec::new() };
entries
.flatten()
.filter(|e| e.path().extension().is_some_and(|ext| ext == "json"))
.filter_map(|e| {
let path = e.path();
let data = fs::read_to_string(&path).ok()?;
match serde_json::from_str::<PendingCleanup>(&data) {
Ok(record) => Some(record),
Err(err) => {
// Moved aside rather than left in place: a record nothing
// ever repairs would otherwise warn on every single
// startup forever, same as an ordinary `.json` file it
// would keep looking like one to `list_in` on the next
// call too. One aside-copy is enough here — this only
// ever holds names to retry removing, not the class of
// once-in-a-lifetime crash evidence `migration_store`
// keeps multiple timestamped backups of.
let corrupt = path.with_extension("json.corrupt");
let moved = !corrupt.exists() && fs::rename(&path, &corrupt).is_ok();
log::warn!(
"Could not parse pending cleanup record {}: {}{}",
path.display(),
err,
if moved {
format!(" — moved aside to {}", corrupt.display())
} else {
" — leaving it in place".to_string()
}
);
None
}
}
})
.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_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("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(),
}
}
#[test]
fn project_ids_cannot_escape_the_pending_cleanup_directory() {
assert_eq!(sanitize("../../etc/passwd"), "______etc_passwd");
assert_eq!(sanitize("a/b"), "a_b");
assert_eq!(
sanitize("ab62cd24-51aa-4645-8f5c-17a124062050"),
"ab62cd24-51aa-4645-8f5c-17a124062050"
);
}
#[test]
fn is_empty_reflects_whatever_still_needs_removing() {
let mut r = record("p1");
assert!(!r.is_empty());
r.container_id = None;
r.image = None;
assert!(!r.is_empty(), "a leftover volume alone still counts");
r.volumes.clear();
assert!(r.is_empty());
}
/// 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_dir("roundtrip");
let rec = record("proj-1");
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()]);
clear_in(&dir, "proj-1").expect("clear");
assert!(list_in(&dir).is_empty());
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();
}
/// 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_dir("corrupt");
fs::write(dir.join("bad.json"), "{ not json").unwrap();
save_in(&dir, &record("proj-2")).expect("save");
let found = list_in(&dir);
assert_eq!(found.len(), 1);
assert_eq!(found[0].project_id, "proj-2");
fs::remove_dir_all(&dir).ok();
}
/// A record that fails to parse is moved aside once, rather than left in
/// place to be re-warned about — and re-warned about — on every future
/// launch forever.
#[test]
fn an_unparseable_record_is_moved_aside_exactly_once() {
let dir = temp_dir("corrupt-aside");
let bad = dir.join("bad.json");
fs::write(&bad, "{ not json").unwrap();
list_in(&dir);
assert!(!bad.exists(), "the bad file should have been moved aside");
let corrupt = dir.join("bad.json.corrupt");
assert!(corrupt.exists(), "and the moved copy should be at .json.corrupt");
// A second pass must not warn about `bad.json` again — it is gone —
// and must not choke on `.json.corrupt` already being there.
assert!(list_in(&dir).is_empty());
assert!(corrupt.exists(), "the aside copy is not itself deleted");
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();
}
}
@@ -1,5 +1,6 @@
import { useEffect, useMemo, useState } from "react"; import { useEffect, useMemo, useState } from "react";
import { useShallow } from "zustand/react/shallow"; import { useShallow } from "zustand/react/shallow";
import { projectRemovalIsClean } from "../../../lib/types";
import { useAppState } from "../../../store/appState"; import { useAppState } from "../../../store/appState";
import { useProjectActions } from "../../../hooks/useProjectActions"; import { useProjectActions } from "../../../hooks/useProjectActions";
import { useProjects } from "../../../hooks/useProjects"; import { useProjects } from "../../../hooks/useProjects";
@@ -18,6 +19,7 @@ import ConfigTab from "./ConfigTab";
import FilesTab from "./FilesTab"; import FilesTab from "./FilesTab";
import BrowserTab from "./BrowserTab"; import BrowserTab from "./BrowserTab";
import { formatUptime } from "./format"; import { formatUptime } from "./format";
import { describeLeftovers, leftoverPronoun, leftoverVerb } from "./removalReport";
const TABS = [ const TABS = [
{ id: "overview", label: "Overview" }, { id: "overview", label: "Overview" },
@@ -282,7 +284,25 @@ export default function ProjectHome({ projectId, active }: Props) {
onConfirm={async () => { onConfirm={async () => {
setConfirmRemove(false); setConfirmRemove(false);
try { try {
await remove(project.id); const report = await remove(project.id);
if (!projectRemovalIsClean(report)) {
const verb = leftoverVerb(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 removed`,
detail: `Triple-C could not confirm ${describeLeftovers(report)} ${verb} 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 Triple-C could not confirm its Docker resources were removed`,
detail: `Triple-C could not confirm ${describeLeftovers(report)} ${verb} removed, and could not record this for a retry. You may need to remove ${leftoverPronoun(report)} manually (\`docker rm\` / \`docker rmi\` / \`docker volume rm\`).`,
});
}
}
} catch (e) { } catch (e) {
useAppState.getState().pushToast({ useAppState.getState().pushToast({
kind: "error", kind: "error",
@@ -0,0 +1,51 @@
import { describe, it, expect } from "vitest";
import { describeLeftovers, leftoverVerb } from "./removalReport";
import { projectRemovalIsClean } from "../../../lib/types";
import type { ProjectRemovalReport } from "../../../lib/types";
function report(overrides: Partial<ProjectRemovalReport> = {}): ProjectRemovalReport {
return {
container: null,
image: null,
volumes: [],
retry_scheduled: false,
...overrides,
};
}
describe("projectRemovalIsClean", () => {
it("is true only when nothing survived", () => {
expect(projectRemovalIsClean(report())).toBe(true);
expect(projectRemovalIsClean(report({ container: "triple-c-abc" }))).toBe(false);
expect(projectRemovalIsClean(report({ image: "triple-c-snapshot-abc:latest" }))).toBe(false);
expect(projectRemovalIsClean(report({ volumes: ["triple-c-home-abc"] }))).toBe(false);
});
});
describe("describeLeftovers", () => {
it("names each kind of leftover", () => {
expect(describeLeftovers(report({ container: "triple-c-abc" }))).toBe("its container");
expect(describeLeftovers(report({ image: "x" }))).toBe("its saved image");
expect(describeLeftovers(report({ volumes: ["v1"] }))).toBe("a volume");
expect(describeLeftovers(report({ volumes: ["v1", "v2"] }))).toBe("2 volumes");
});
it("joins multiple kinds together", () => {
expect(
describeLeftovers(report({ container: "triple-c-abc", image: "x", volumes: ["v1", "v2"] })),
).toBe("its container, its saved image, 2 volumes");
});
});
describe("leftoverVerb", () => {
it("is singular for exactly one leftover of any kind", () => {
expect(leftoverVerb(report({ container: "triple-c-abc" }))).toBe("was");
expect(leftoverVerb(report({ image: "x" }))).toBe("was");
expect(leftoverVerb(report({ volumes: ["v1"] }))).toBe("was");
});
it("is plural once more than one thing survived, including multiple volumes alone", () => {
expect(leftoverVerb(report({ container: "triple-c-abc", image: "x" }))).toBe("were");
expect(leftoverVerb(report({ volumes: ["v1", "v2"] }))).toBe("were");
});
});
@@ -0,0 +1,39 @@
import type { ProjectRemovalReport } from "../../../lib/types";
/**
* 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.
*/
export function describeLeftovers(report: ProjectRemovalReport): string {
const parts: string[] = [];
if (report.container) parts.push("its container");
if (report.image) parts.push("its saved image");
if (report.volumes.length === 1) parts.push("a volume");
else if (report.volumes.length > 1) parts.push(`${report.volumes.length} volumes`);
return parts.join(", ");
}
/** How many distinct things `describeLeftovers` is describing a container
* and an image each count as one, however many volumes are named. Shared by
* `leftoverVerb` and `leftoverPronoun` so the two can never disagree about
* singular vs. plural. */
function leftoverCount(report: ProjectRemovalReport): number {
return (report.container ? 1 : 0) + (report.image ? 1 : 0) + report.volumes.length;
}
/** Verb agreement for `describeLeftovers`'s output "its container" needs
* "was", "its container, a volume" needs "were". */
export function leftoverVerb(report: ProjectRemovalReport): "was" | "were" {
return leftoverCount(report) === 1 ? "was" : "were";
}
/** Pronoun agreement for referring back to `describeLeftovers`'s output
* "remove it manually" for one thing, "remove them manually" for more. */
export function leftoverPronoun(report: ProjectRemovalReport): "it" | "them" {
return leftoverCount(report) === 1 ? "it" : "them";
}
+23 -4
View File
@@ -3,6 +3,7 @@ import { save } from "@tauri-apps/plugin-dialog";
import type { Project } from "../lib/types"; import type { Project } from "../lib/types";
import * as commands from "../lib/tauri-commands"; import * as commands from "../lib/tauri-commands";
import { formatBytes } from "../lib/formatBytes"; import { formatBytes } from "../lib/formatBytes";
import { describeResetLeftovers, resetLeftoverPronoun } from "../lib/resetOutcome";
import { useAppState } from "../store/appState"; import { useAppState } from "../store/appState";
import { useProjects } from "./useProjects"; import { useProjects } from "./useProjects";
import { useTerminal } from "./useTerminal"; import { useTerminal } from "./useTerminal";
@@ -28,13 +29,14 @@ export function useProjectActions(project: Project) {
); );
const run = useCallback( const run = useCallback(
async (label: string, fn: () => Promise<unknown>) => { async <T,>(label: string, fn: () => Promise<T>): Promise<T | undefined> => {
setBusy(true); setBusy(true);
setContainerProgress(project.id, null); setContainerProgress(project.id, null);
try { try {
await fn(); return await fn();
} catch (e) { } catch (e) {
fail(`${label} failed for “${project.name}`, e); fail(`${label} failed for “${project.name}`, e);
return undefined;
} finally { } finally {
setContainerProgress(project.id, null); setContainerProgress(project.id, null);
setBusy(false); setBusy(false);
@@ -54,8 +56,25 @@ export function useProjectActions(project: Project) {
); );
const handleReset = useCallback( const handleReset = useCallback(
() => run("Reset", () => rebuild(project.id)), () =>
[run, rebuild, project.id], run("Reset", async () => {
const outcome = await rebuild(project.id);
if (outcome.leftover_image || outcome.leftover_volumes.length > 0) {
// Not "run `docker volume rm`" — by the time this renders, the new
// container this same call just started already has the leftover
// volume mounted, so that command would just hit the same 409
// Reset did. Stopping the project first is what actually frees it.
pushToast({
kind: "error",
message: `Reset for “${project.name}” did not fully clean up`,
detail: `Triple-C could not remove ${describeResetLeftovers(outcome)} from before the reset, so \
the new container may still be built from, or contain, old data. Stop the project, then try \
Reset again, or remove ${resetLeftoverPronoun(outcome)} manually once stopped.`,
});
}
return outcome;
}),
[run, rebuild, project.id, project.name, pushToast],
); );
const openClaudeTerminal = useCallback(async () => { const openClaudeTerminal = useCallback(async () => {
+24
View File
@@ -140,3 +140,27 @@ describe("useProjects puts the status back when a refused command never ran", ()
expect(statusOf()).toBe("stopped"); expect(statusOf()).toBe("stopped");
}); });
}); });
describe("useProjects.rebuild on success", () => {
it("puts the outcome's project, not the whole outcome, into the list", async () => {
const rebuilt = project("running");
rebuildProjectContainer.mockResolvedValue({
project: rebuilt,
leftover_image: null,
leftover_volumes: [],
});
const { result } = renderHook(() => useProjects());
let outcome!: Awaited<ReturnType<typeof result.current.rebuild>>;
await act(async () => {
outcome = await result.current.rebuild("p1");
});
// A regression here would put the `{ project, leftover_image,
// leftover_volumes }` wrapper into the projects list instead of the
// `Project` it wraps — a shape mismatch `tsc` would not catch inside a
// callback typed to take `unknown` per Tauri's `invoke`.
expect(useAppState.getState().projects.find((p) => p.id === "p1")).toEqual(rebuilt);
expect(outcome.leftover_volumes).toEqual([]);
});
});
+5 -4
View File
@@ -44,8 +44,9 @@ export function useProjects() {
const remove = useCallback( const remove = useCallback(
async (id: string) => { async (id: string) => {
await commands.removeProject(id); const report = await commands.removeProject(id);
removeProjectFromList(id); removeProjectFromList(id);
return report;
}, },
[removeProjectFromList], [removeProjectFromList],
); );
@@ -135,9 +136,9 @@ export function useProjects() {
const rebuild = useCallback( const rebuild = useCallback(
(id: string) => (id: string) =>
withOptimisticStatus(id, "starting", async () => { withOptimisticStatus(id, "starting", async () => {
const updated = await commands.rebuildProjectContainer(id); const outcome = await commands.rebuildProjectContainer(id);
updateProjectInList(updated); updateProjectInList(outcome.project);
return updated; return outcome;
}), }),
[updateProjectInList, withOptimisticStatus], [updateProjectInList, withOptimisticStatus],
); );
+39
View File
@@ -0,0 +1,39 @@
import { describe, it, expect } from "vitest";
import { describeResetLeftovers, resetLeftoverPronoun } from "./resetOutcome";
import type { ProjectResetOutcome } from "./types";
function outcome(overrides: Partial<ProjectResetOutcome> = {}): ProjectResetOutcome {
return {
project: {} as ProjectResetOutcome["project"],
leftover_image: null,
leftover_volumes: [],
...overrides,
};
}
describe("describeResetLeftovers", () => {
it("names the image first, then the volumes", () => {
expect(describeResetLeftovers(outcome({ leftover_image: "x" }))).toBe(
"its previous container image",
);
expect(describeResetLeftovers(outcome({ leftover_volumes: ["v1"] }))).toBe("a volume");
expect(describeResetLeftovers(outcome({ leftover_volumes: ["v1", "v2"] }))).toBe("2 volumes");
expect(
describeResetLeftovers(outcome({ leftover_image: "x", leftover_volumes: ["v1", "v2"] })),
).toBe("its previous container image and 2 volumes");
});
});
describe("resetLeftoverPronoun", () => {
it("is singular for exactly one leftover", () => {
expect(resetLeftoverPronoun(outcome({ leftover_image: "x" }))).toBe("it");
expect(resetLeftoverPronoun(outcome({ leftover_volumes: ["v1"] }))).toBe("it");
});
it("is plural once more than one thing survived", () => {
expect(resetLeftoverPronoun(outcome({ leftover_image: "x", leftover_volumes: ["v1"] }))).toBe(
"them",
);
expect(resetLeftoverPronoun(outcome({ leftover_volumes: ["v1", "v2"] }))).toBe("them");
});
});
+32
View File
@@ -0,0 +1,32 @@
import type { ProjectResetOutcome } from "./types";
/**
* Names what a `ProjectResetOutcome` says Reset could not clear, for
* `useProjectActions`'s Reset toast.
*
* The image is named first and phrased as "its previous container image"
* rather than folded in with the volumes it is the more serious of the
* two: the new container is built from it whenever it exists, so a
* surviving image means Reset silently rebuilt the exact system layer it
* was asked to discard, while a surviving volume only means old data rides
* along.
*/
export function describeResetLeftovers(outcome: ProjectResetOutcome): string {
const parts: string[] = [];
if (outcome.leftover_image) parts.push("its previous container image");
if (outcome.leftover_volumes.length === 1) parts.push("a volume");
else if (outcome.leftover_volumes.length > 1) parts.push(`${outcome.leftover_volumes.length} volumes`);
return parts.join(" and ");
}
/** How many distinct things `describeResetLeftovers` is describing the
* image counts as one, however many volumes are named alongside it. */
function resetLeftoverCount(outcome: ProjectResetOutcome): number {
return (outcome.leftover_image ? 1 : 0) + outcome.leftover_volumes.length;
}
/** Pronoun agreement for referring back to `describeResetLeftovers`'s
* output "remove it manually" for one thing, "remove them" for more. */
export function resetLeftoverPronoun(outcome: ProjectResetOutcome): "it" | "them" {
return resetLeftoverCount(outcome) === 1 ? "it" : "them";
}
+3 -3
View File
@@ -1,5 +1,5 @@
import { invoke } from "@tauri-apps/api/core"; import { invoke } from "@tauri-apps/api/core";
import type { Project, ProjectPath, 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 // Docker
export const checkDocker = () => invoke<boolean>("check_docker"); export const checkDocker = () => invoke<boolean>("check_docker");
@@ -13,7 +13,7 @@ export const listProjects = () => invoke<Project[]>("list_projects");
export const addProject = (name: string, paths: ProjectPath[]) => export const addProject = (name: string, paths: ProjectPath[]) =>
invoke<Project>("add_project", { name, paths }); invoke<Project>("add_project", { name, paths });
export const removeProject = (projectId: string) => export const removeProject = (projectId: string) =>
invoke<void>("remove_project", { projectId }); invoke<ProjectRemovalReport>("remove_project", { projectId });
export const updateProject = (project: Project) => export const updateProject = (project: Project) =>
invoke<Project>("update_project", { project }); invoke<Project>("update_project", { project });
export const startProjectContainer = (projectId: string) => export const startProjectContainer = (projectId: string) =>
@@ -21,7 +21,7 @@ export const startProjectContainer = (projectId: string) =>
export const stopProjectContainer = (projectId: string) => export const stopProjectContainer = (projectId: string) =>
invoke<void>("stop_project_container", { projectId }); invoke<void>("stop_project_container", { projectId });
export const rebuildProjectContainer = (projectId: string) => export const rebuildProjectContainer = (projectId: string) =>
invoke<Project>("rebuild_project_container", { projectId }); invoke<ProjectResetOutcome>("rebuild_project_container", { projectId });
export const reconcileProjectStatuses = () => export const reconcileProjectStatuses = () =>
invoke<Project[]>("reconcile_project_statuses"); invoke<Project[]>("reconcile_project_statuses");
+31
View File
@@ -77,6 +77,37 @@ export type ProjectStatus =
| "stopping" | "stopping"
| "error"; | "error";
/** What `removeProject` could not delete. The project is removed from the
* 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;
/** The saved container image, if Reset could not remove it the new
* container is built from it whenever it exists, so this means Reset
* silently rebuilt the system layer it was asked to discard. */
leftover_image: string | null;
leftover_volumes: string[];
}
export type Backend = export type Backend =
| "anthropic" | "anthropic"
| "bedrock" | "bedrock"