Merge pull request 'Sweep the snapshot commits recreation leaves behind' (#23) from sweep-orphaned-snapshots into main

Reviewed-on: #23

README.md

@@ -133,6 +133,14 @@ forces that).
 4. **Stop**: Container halted (its filesystem layer and both named volumes persist)
 5. **Restart**: Existing container restarted; if any `triple-c.*` label no longer matches the project's settings, the container is committed to a snapshot image, removed, and recreated from that snapshot — so installed packages survive
 6. **Migrate**: The project is moved onto a newer base image without losing its volumes — see below
+
+Each recreation moves the `triple-c-snapshot-{projectId}:latest` tag, leaving the image it pointed
+at before untagged but still on disk — multiple gigabytes per recreation. `sweep_orphaned_snapshots`
+clears those after a recreation and after a migration is accepted. It only ever removes images that
+are **both** untagged *and* labelled `triple-c.managed=true`, so a live snapshot tag and a
+migration's `pre-migration-*` rollback pin are structurally out of reach, and removal is unforced so
+Docker itself refuses while any container — including a stopped project's — is still built from the
+image.
 7. **Reset**: Container, snapshot image **and both named volumes** all removed, then recreated from the clean base image. `remove_project_volumes` deletes `triple-c-home-{projectId}` and `triple-c-claude-config-{projectId}`, so `~/.claude`, `~/.claude.json`, the OAuth login, installed skills, session transcripts and the scheduler's tasks are all lost.
 
 ### Base-Image Migration

app/src-tauri/src/commands/migration_commands.rs

@@ -833,6 +833,16 @@ pub async fn confirm_migration(
     migration_store::clear_staging(&project_id)?;
     migration_store::clear(&project_id)?;
     log::info!("Migration confirmed for project {}", project_id);
+
+    // Dropping the pin above is what turns the pre-migration image into an
+    // orphan: it was the only tag holding a multi-gigabyte pre-migration
+    // snapshot. Accepting the update is therefore the moment to sweep, and
+    // waiting for the project's next recreation would leave it lying around
+    // indefinitely.
+    tauri::async_runtime::spawn(async {
+        crate::docker::sweep_orphaned_snapshots().await;
+    });
+
     Ok(())
 }
 

app/src-tauri/src/commands/project_commands.rs

@@ -450,6 +450,18 @@ pub async fn start_project_container(
                 ).await?;
                 emit_progress(&app_handle, &project_id, "Starting container...");
                 docker::start_container(&new_id).await?;
+
+                // The commit above moved `:latest` and orphaned the image it
+                // used to point at; the container holding that image open was
+                // removed a few lines up, so now is when Docker will actually
+                // let it go. Detached because this is housekeeping and the
+                // project is already running — and it sweeps every orphan, not
+                // just this one, so recreations that happened before the sweep
+                // existed are cleaned up too.
+                tauri::async_runtime::spawn(async {
+                    docker::sweep_orphaned_snapshots().await;
+                });
+
                 new_id
             } else {
                 emit_progress(&app_handle, &project_id, "Starting container...");

app/src-tauri/src/docker/container.rs

@@ -211,6 +211,12 @@ pub const SECRET_ENV_KEYS: &[&str] = &[
 ];
 
 /// Env var name prefixes Triple-C manages itself; users cannot set these by hand.
+/// The label every container Triple-C creates carries — and, because
+/// `docker commit` copies a container's labels onto the image, every snapshot it
+/// commits. [`sweep_orphaned_snapshots`] treats it as the mark of provenance,
+/// which is what keeps the sweep away from the user's own images.
+const LABEL_MANAGED: &str = "triple-c.managed";
+
 const RESERVED_ENV_PREFIXES: &[&str] = &["ANTHROPIC_", "AWS_", "GIT_", "HOST_", "TRIPLE_C_"];
 
 /// Exact env var names Triple-C manages itself. Not covered by
@@ -318,8 +324,19 @@ fn is_reserved_env_key(key: &str) -> bool {
         || RESERVED_ENV_EXACT.iter().any(|e| upper == *e)
 }
 
-/// Compute a fingerprint string for the custom environment variables.
+/// Compute a fingerprint for the custom environment variables.
-/// Sorted alphabetically so order changes do not cause spurious recreation.
+///
+/// Sorted alphabetically so order changes do not cause spurious recreation, and
+/// **hashed**, because this value is written as the
+/// `triple-c.custom-env-fingerprint` label. Labels are readable by anything on
+/// the host through `docker inspect`, `docker commit` copies them onto the
+/// project's snapshot image, and `container_needs_recreation` logs both sides on
+/// a mismatch — so a plaintext `KEY=VALUE` join published every custom
+/// variable's *value*, API tokens included, to all three places. Same treatment
+/// as `triple-c.git-token-hash`.
+///
+/// Empty stays empty rather than becoming the hash of the empty string: an empty
+/// label is how every other `triple-c.*` key says "nothing configured".
 fn compute_env_fingerprint(custom_env_vars: &[EnvVar]) -> String {
     let mut parts: Vec<String> = Vec::new();
     for env_var in custom_env_vars {
@@ -330,7 +347,10 @@ fn compute_env_fingerprint(custom_env_vars: &[EnvVar]) -> String {
         parts.push(format!("{}={}", key, env_var.value));
     }
     parts.sort();
-    parts.join(",")
+    if parts.is_empty() {
+        return String::new();
+    }
+    sha256_hex(&parts.join(","))
 }
 
 /// The shared Claude Code OAuth token to inject for this project, paired with
@@ -1341,7 +1361,7 @@ pub async fn create_container(
     }
 
     let mut labels = HashMap::new();
-    labels.insert("triple-c.managed".to_string(), "true".to_string());
+    labels.insert(LABEL_MANAGED.to_string(), "true".to_string());
     labels.insert("triple-c.project-id".to_string(), project.id.clone());
     labels.insert("triple-c.project-name".to_string(), project.name.clone());
     labels.insert("triple-c.backend".to_string(), format!("{:?}", project.backend));
@@ -1689,6 +1709,128 @@ fn env_holds_a_secret(env: &[String]) -> bool {
     })
 }
 
+/// Outcome of [`sweep_orphaned_snapshots`].
+#[derive(Debug, Default, Clone, serde::Serialize)]
+pub struct SnapshotSweepReport {
+    /// Image ids that were removed.
+    pub removed: Vec<String>,
+    /// Bytes the removed images accounted for, as Docker reported them. A
+    /// shared-layer estimate, not a disk-usage measurement.
+    pub reclaimed_bytes: i64,
+    /// Orphans Docker refused to delete because a container is still built
+    /// from them. Normal, not a failure — the next sweep gets them.
+    pub in_use: usize,
+    /// Orphans that could not be removed for any other reason, with the error.
+    pub failed: Vec<(String, String)>,
+    /// Set when the engine could not be reached or listed at all.
+    pub unavailable: Option<String>,
+}
+
+/// The filter every sweep runs under. Extracted so a test can hold the two
+/// conditions in place: **dangling** and **labelled as ours**. Losing either
+/// one turns a snapshot sweep into a prune of the user's whole image store.
+fn orphan_sweep_filters() -> HashMap<String, Vec<String>> {
+    HashMap::from([
+        ("dangling".to_string(), vec!["true".to_string()]),
+        (
+            "label".to_string(),
+            vec![format!("{}=true", LABEL_MANAGED)],
+        ),
+    ])
+}
+
+/// Remove the untagged snapshot commits left behind by recreation.
+///
+/// Every recreation commits the container to `triple-c-snapshot-{id}:latest`
+/// and moves that tag; the image the tag pointed at before keeps its layers and
+/// loses its name. Nothing else deletes those, so a project that has been
+/// recreated a dozen times leaves a dozen multi-gigabyte orphans behind.
+///
+/// Two conditions, and the safety of this whole function rests on them:
+///
+/// * **Dangling** — untagged. Every image the app relies on carries a tag:
+///   `triple-c-snapshot-{id}:latest` is what a project is rebuilt from, and a
+///   migration's `pre-migration-*` pin is the only copy of a rollback target.
+///   Neither can ever match this filter, so neither can be swept.
+/// * **`triple-c.managed=true`** — only images Triple-C itself committed.
+///   `docker commit` copies the container's labels onto the image, which is what
+///   makes the label a reliable mark of provenance. The user's own dangling
+///   images are none of our business.
+///
+/// Removal is not forced, so Docker refuses (409) while any container is still
+/// built from the image — including the stopped containers of projects that are
+/// not running. That refusal is the third safety net and it is the daemon's,
+/// not ours; those orphans are simply counted and left for a later sweep.
+///
+/// Never fails the caller: this is housekeeping, and a full disk is a better
+/// outcome than a project that will not start.
+pub async fn sweep_orphaned_snapshots() -> SnapshotSweepReport {
+    use bollard::image::ListImagesOptions;
+
+    let mut report = SnapshotSweepReport::default();
+
+    let docker = match get_docker() {
+        Ok(d) => d,
+        Err(e) => {
+            report.unavailable = Some(e);
+            return report;
+        }
+    };
+
+    let images = match docker
+        .list_images(Some(ListImagesOptions {
+            all: false,
+            filters: orphan_sweep_filters(),
+            ..Default::default()
+        }))
+        .await
+    {
+        Ok(images) => images,
+        Err(e) => {
+            report.unavailable = Some(format!("Could not list orphaned snapshots: {}", e));
+            return report;
+        }
+    };
+
+    for summary in images {
+        match docker
+            .remove_image(
+                &summary.id,
+                Some(RemoveImageOptions {
+                    force: false,
+                    noprune: false,
+                }),
+                None,
+            )
+            .await
+        {
+            Ok(_) => {
+                report.reclaimed_bytes += summary.size;
+                report.removed.push(summary.id);
+            }
+            Err(bollard::errors::Error::DockerResponseServerError {
+                status_code: 409, ..
+            }) => {
+                report.in_use += 1;
+            }
+            Err(e) => {
+                report.failed.push((summary.id, e.to_string()));
+            }
+        }
+    }
+
+    if !report.removed.is_empty() || report.in_use > 0 {
+        log::info!(
+            "Snapshot sweep: removed {} orphan(s) ({:.2} GB), {} still in use by a container",
+            report.removed.len(),
+            report.reclaimed_bytes as f64 / 1_073_741_824.0,
+            report.in_use
+        );
+    }
+
+    report
+}
+
 /// Outcome of [`scrub_secrets_from_snapshots`], so callers can tell the user
 /// what actually happened rather than guessing.
 #[derive(Debug, Default, Clone, serde::Serialize)]
@@ -2352,7 +2494,7 @@ pub async fn list_sibling_containers() -> Result<Vec<ContainerSummary>, String>
         .into_iter()
         .filter(|c| {
             if let Some(labels) = &c.labels {
-                !labels.contains_key("triple-c.managed")
+                !labels.contains_key(LABEL_MANAGED)
             } else {
                 true
             }
@@ -2473,6 +2615,45 @@ mod tests {
         assert_eq!(fp, "");
     }
 
+    #[test]
+    fn the_orphan_sweep_only_ever_looks_at_our_own_untagged_images() {
+        // Both conditions are load-bearing. Without `dangling` the sweep would
+        // match `triple-c-snapshot-{id}:latest` — what every project is rebuilt
+        // from — and a migration's `pre-migration-*` pin, which is the only copy
+        // of a rollback target. Without the label it would match every dangling
+        // image on the user's machine.
+        let filters = orphan_sweep_filters();
+        assert_eq!(filters.get("dangling"), Some(&vec!["true".to_string()]));
+        assert_eq!(
+            filters.get("label"),
+            Some(&vec!["triple-c.managed=true".to_string()])
+        );
+        assert_eq!(filters.len(), 2, "an extra filter widens or narrows the sweep");
+    }
+
+    #[test]
+    fn the_custom_env_fingerprint_never_carries_the_value() {
+        // It goes into `triple-c.custom-env-fingerprint`, which `docker inspect`
+        // hands to anything on the host, `docker commit` copies onto the
+        // project's snapshot image, and the recreation check logs on a mismatch.
+        let secret = "33da01c1b320644920c20d6b5e0a1c6b3c3451c2";
+        let fp = compute_env_fingerprint(&[EnvVar {
+            key: "TEA_TOKEN".to_string(),
+            value: secret.to_string(),
+        }]);
+        assert!(!fp.contains(secret), "fingerprint leaked the value: {}", fp);
+        assert!(!fp.contains("TEA_TOKEN"), "fingerprint leaked the key: {}", fp);
+        assert_eq!(fp.len(), 64, "expected a sha256 hex digest, got {:?}", fp);
+
+        // It still has to move when the value does, or a rotated token would
+        // never reach the container.
+        let rotated = compute_env_fingerprint(&[EnvVar {
+            key: "TEA_TOKEN".to_string(),
+            value: "rotated".to_string(),
+        }]);
+        assert_ne!(fp, rotated);
+    }
+
     #[test]
     fn the_deprecated_small_fast_model_var_is_never_emitted() {
         let rendered: Vec<String> = aliases(Some("m"), Some("h"))

app/src/components/projects/EnvVarsEditor.tsx

@@ -43,26 +43,36 @@ export default function EnvVarsEditor({
         </p>
       )}
 
+      {/* The row's widths live on wrapper divs, not on the inputs. `inputClass`
+          carries `w-full`, and a width utility on the input itself does not beat
+          it — class-attribute order is not what resolves the conflict, stylesheet
+          order is. Sizing the key input directly left it asking for the whole row
+          and collapsed the value input, whose `flex-1` basis of 0 gave it only the
+          leftover space, to an unusable sliver. */}
       {vars.map((ev, i) => (
         <div key={i} className="flex gap-2 items-center">
-          <input
+          <div className="w-2/5 shrink-0">
-            value={ev.key}
+            <input
-            onChange={(e) => updateVar(i, "key", e.target.value)}
+              value={ev.key}
-            onBlur={() => onSave(vars)}
+              onChange={(e) => updateVar(i, "key", e.target.value)}
-            placeholder="KEY"
+              onBlur={() => onSave(vars)}
-            aria-label={`Environment variable ${i + 1} name`}
+              placeholder="KEY"
-            disabled={disabled}
+              aria-label={`Environment variable ${i + 1} name`}
-            className={`w-2/5 ${monoInputClass}`}
+              disabled={disabled}
-          />
+              className={monoInputClass}
-          <input
+            />
-            value={ev.value}
+          </div>
-            onChange={(e) => updateVar(i, "value", e.target.value)}
+          <div className="flex-1 min-w-0">
-            onBlur={() => onSave(vars)}
+            <input
-            placeholder="value"
+              value={ev.value}
-            aria-label={`Environment variable ${i + 1} value`}
+              onChange={(e) => updateVar(i, "value", e.target.value)}
-            disabled={disabled}
+              onBlur={() => onSave(vars)}
-            className={`flex-1 ${monoInputClass}`}
+              placeholder="value"
-          />
+              aria-label={`Environment variable ${i + 1} value`}
+              disabled={disabled}
+              className={monoInputClass}
+            />
+          </div>
           <Button
             variant="danger"
             disabled={disabled}