remove_project cannot report a failed cleanup, and drops the record anyway #31

Closed
opened 2026-08-25 18:12:20 +00:00 by jknapp · 0 comments
Owner

What happens

docker::remove_project_volumes (app/src-tauri/src/docker/container.rs:3520) logs a warning per volume and then returns Ok(()) unconditionally:

match docker.remove_volume(&vol, None).await {
    Ok(_) => log::info!("Removed volume {}", vol),
    Err(e) => log::warn!("Failed to remove volume {} (may not exist): {}", vol, e),
}
...
Ok(())

So the if let Err(e) guarding it in commands::project_commands::remove_project (:742) is unreachable by construction. remove_snapshot_image is best-effort in the same way at :739.

remove_project then runs state.projects_store.remove(&project_id) regardless. Once the id leaves the store the app has no record the project ever existed, so nothing that failed can ever be retried — not by the user, not by the startup housekeeping, not by sweep_orphaned_snapshots, which scans per project. The leaked resource becomes unreachable by any path in the UI.

The user is told the removal succeeded.

Evidence this has already happened

Found on 2026-08-25 while checking an unrelated project removal:

triple-c-claude-config-390aba0b-9a34-403e-94cc-1739dc1f1494
  created / last written: 2026-02-27
  no home volume, no container, no snapshot image, nothing referencing it
  160 KB, 5 files (cache/, debug/, backups/.claude.json.backup.*)

Volumes are created in pairs (triple-c-home-{id} + triple-c-claude-config-{id}). A split pair is the signature of exactly this bug: one remove_volume succeeded and the other failed, and the failure was swallowed. It has been sitting unreferenced for six months. (Swept manually; the code path is unchanged.)

Note the backups/.claude.json.backup.* files — .claude.json carries oauthAccount, so a leaked config volume is credential-adjacent, not merely untidy.

Why the current shape is defensible but wrong

Best-effort is the right instinct here: a removal that hard-fails on a locked volume and leaves the project in the sidebar is worse than one that gets most of the way. The bug is not that it continues — it is that it continues silently and then destroys the only handle that would let anyone finish the job.

Suggested fix

  1. Make remove_project_volumes and remove_snapshot_image return what they could not remove (e.g. Vec<String> of resource names) rather than Ok(()).
  2. Have remove_project collect those. Still remove the project record — but surface the leftovers to the user in the completion toast, naming them, so a manual docker volume rm is possible.
  3. Better: before dropping the record, write the leftovers somewhere the app still reads after the project is gone, so startup housekeeping can retry. A triple-c.orphaned label on the resource, or a small pending-cleanup.json beside projects.json, would both work. The label approach fits the existing sweep machinery.
  4. Consider whether remove_container at :735 deserves the same treatment — it is let _ = today, so a container that refuses to die is also silent.

Test notes

Whatever lands should be mutation-checked: the natural test ("removal reports the volume it could not delete") passes trivially against a function that cannot fail, which is how this shape survived in the first place.

Not urgent

No user-facing breakage; the cost is a slow leak of volumes and snapshot images that nothing can reclaim. Filed from a session on 2026-08-25 after PR #30 merged, to be picked up on a later run.

## What happens `docker::remove_project_volumes` (`app/src-tauri/src/docker/container.rs:3520`) logs a warning per volume and then returns `Ok(())` unconditionally: ```rust match docker.remove_volume(&vol, None).await { Ok(_) => log::info!("Removed volume {}", vol), Err(e) => log::warn!("Failed to remove volume {} (may not exist): {}", vol, e), } ... Ok(()) ``` So the `if let Err(e)` guarding it in `commands::project_commands::remove_project` (`:742`) is unreachable by construction. `remove_snapshot_image` is best-effort in the same way at `:739`. `remove_project` then runs `state.projects_store.remove(&project_id)` regardless. **Once the id leaves the store the app has no record the project ever existed**, so nothing that failed can ever be retried — not by the user, not by the startup housekeeping, not by `sweep_orphaned_snapshots`, which scans per project. The leaked resource becomes unreachable by any path in the UI. The user is told the removal succeeded. ## Evidence this has already happened Found on 2026-08-25 while checking an unrelated project removal: ``` triple-c-claude-config-390aba0b-9a34-403e-94cc-1739dc1f1494 created / last written: 2026-02-27 no home volume, no container, no snapshot image, nothing referencing it 160 KB, 5 files (cache/, debug/, backups/.claude.json.backup.*) ``` Volumes are created in pairs (`triple-c-home-{id}` + `triple-c-claude-config-{id}`). A **split** pair is the signature of exactly this bug: one `remove_volume` succeeded and the other failed, and the failure was swallowed. It has been sitting unreferenced for six months. (Swept manually; the code path is unchanged.) Note the `backups/.claude.json.backup.*` files — `.claude.json` carries `oauthAccount`, so a leaked config volume is credential-adjacent, not merely untidy. ## Why the current shape is defensible but wrong Best-effort is the right *instinct* here: a removal that hard-fails on a locked volume and leaves the project in the sidebar is worse than one that gets most of the way. The bug is not that it continues — it is that it continues **silently** and then destroys the only handle that would let anyone finish the job. ## Suggested fix 1. Make `remove_project_volumes` and `remove_snapshot_image` return what they could not remove (e.g. `Vec<String>` of resource names) rather than `Ok(())`. 2. Have `remove_project` collect those. Still remove the project record — but surface the leftovers to the user in the completion toast, naming them, so a manual `docker volume rm` is possible. 3. Better: before dropping the record, write the leftovers somewhere the app still reads after the project is gone, so startup housekeeping can retry. A `triple-c.orphaned` label on the resource, or a small `pending-cleanup.json` beside `projects.json`, would both work. The label approach fits the existing sweep machinery. 4. Consider whether `remove_container` at `:735` deserves the same treatment — it is `let _ =` today, so a container that refuses to die is also silent. ## Test notes Whatever lands should be mutation-checked: the natural test ("removal reports the volume it could not delete") passes trivially against a function that cannot fail, which is how this shape survived in the first place. ## Not urgent No user-facing breakage; the cost is a slow leak of volumes and snapshot images that nothing can reclaim. Filed from a session on 2026-08-25 after PR #30 merged, to be picked up on a later run.
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: CyberCoveLLC/Triple-C#31