From 371ba94609cb76a3ccb3b249ca30df9ffb9abc77 Mon Sep 17 00:00:00 2001
From: Val Alexander <bunsthedev@gmail.com>
Date: Tue, 7 Jul 2026 02:29:48 -0500
Subject: [PATCH] feat(worker): surface workspace cleanup failures; close out
 #12 container artifacts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With the container backend (#5) merged, the repo checkout is now removed on
every path — host and container (--rm) — so the last #12 gap is that a cleanup
FAILURE was silently swallowed (.ok()), which #12 flags as leaving private code
on disk. Now a failure that leaves the checkout behind is logged loudly and
audited (workspace_cleanup_failed). Docs move repo_checkout and container-log
handling from deferred to covered.

Closes #12.

Signed-off-by: Val Alexander <bunsthedev@gmail.com>
---
 crates/worker/src/lib.rs | 24 ++++++++++++++++++++++--
 docs/data-retention.md   | 18 ++++++++++++++----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs
index b32b138..5aad079 100644
--- a/crates/worker/src/lib.rs
+++ b/crates/worker/src/lib.rs
@@ -344,8 +344,28 @@ async fn execute_task_with_minter(
     )
     .await;
 
-    // Workspace cleanup ALWAYS runs — success or failure.
-    tokio::fs::remove_dir_all(&workspace).await.ok();
+    // Workspace cleanup ALWAYS runs — success or failure. The workspace holds
+    // the private repository checkout, so a cleanup FAILURE that leaves it on
+    // disk must be surfaced and audited, never silently swallowed (issue #12).
+    if let Err(e) = tokio::fs::remove_dir_all(&workspace).await {
+        // remove_dir_all also errors when the dir never existed; only alarm
+        // when the checkout is genuinely still on disk.
+        if tokio::fs::try_exists(&workspace).await.unwrap_or(true) {
+            error!(
+                task_id = %task.id,
+                workspace = %workspace.display(),
+                "workspace cleanup FAILED — a private checkout may remain on disk: {e:#}"
+            );
+            let _ = store
+                .record_api_read(
+                    &format!("worker:{}", task.id),
+                    &format!("{}/{}", task.repo_owner, task.repo_name),
+                    "workspace_cleanup_failed",
+                    "error",
+                )
+                .await;
+        }
+    }
 
     // The Check Run ALWAYS reaches a terminal conclusion; both arms complete it.
     match outcome {
diff --git a/docs/data-retention.md b/docs/data-retention.md
index bc28261..2d2f39b 100644
--- a/docs/data-retention.md
+++ b/docs/data-retention.md
@@ -25,8 +25,9 @@ survives.
 | `audit_events` | Yes | `api_audit`, table states below | See [Audit events](#audit-events). |
 | `memory_activity` | Opt-in, retention-limited | `memory_activity` | Per-installation; see [issue #6](memory-contract.md). |
 | `logs` / `transcripts` | Not persisted by the adapter | — | Streamed/redacted only; durable transcripts are out of scope until opt-in retention is designed. |
-| `repo_checkout` | **Never** after task cleanup | ephemeral workspace | Workspace is deleted after every task. Container-isolated cleanup is issue #5. |
-| `tokens` / `secrets` | **Never** | — | The brief is tokenless (#4); results, comments, Check Runs, and stored fields are redacted. |
+| `repo_checkout` | **Never** after task cleanup | ephemeral workspace | The per-task workspace is deleted after every task (success or failure); the container backend also removes the container (`--rm`). A cleanup failure that would leave a checkout on disk is surfaced and audited (`workspace_cleanup_failed`), never swallowed. |
+| `container_logs` | **Not persisted** | — | The container backend (#5) captures no stdout/stderr into any store; only the redacted failure detail reaches `task_attempts.detail`. |
+| `tokens` / `secrets` | **Never** | — | The brief is tokenless (#4); the git token travels to the runtime by environment name only (never in argv or container-inspect output); results, comments, Check Runs, and stored fields are redacted. |
 
 ## Redaction
 
@@ -78,10 +79,19 @@ never logged or stored.
 | Memory | Operator-managed, off by default | Opt-in, per-installation, revocable |
 | Worker isolation | May run on host | Container-isolated per task (#5) |
 
+## Worker execution artifacts
+
+The worker runs each task in a per-task workspace, then deletes it — on every
+path, success or failure. In the container backend (#5) the task runs in a
+fresh container removed by `--rm` (and killed by name on timeout), with a
+read-only root, dropped capabilities, resource limits, and only the workspace
+mounted; the git token is forwarded by environment *name*, so it never enters
+argv or `docker inspect`. If workspace removal ever fails while the checkout is
+still on disk, the worker logs it loudly and records a `workspace_cleanup_failed`
+audit entry rather than silently leaving private code behind.
+
 ## Not yet covered
 
-- Container-scoped artifact handling — `repo_checkout` cleanup guarantees and
-  container log retention — lands with hosted worker isolation (#5).
 - A unified append-only audit-event log (the tables above already provide the
   equivalent records; a single stream is a possible future consolidation).
 - Durable, opt-in agent transcripts with their own redaction/retention policy.