From 101a3660d2afb8cb32822d6e1a32287145325da3 Mon Sep 17 00:00:00 2001
From: Dongni Yang <dongniy@nvidia.com>
Date: Wed, 1 Jul 2026 11:02:37 +0800
Subject: [PATCH] fix(sandbox): surface actionable recovery hint when destroy
 wipe fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

wipeSandboxState() runs `openshell sandbox exec` to rm workspace files
before `sandbox delete`. When the pod is not live at destroy time (100%
repro in all 5 CI runners, #5970), exec returns non-zero and the wipe
silently warns-and-continues. The k3s PVC retains USER.md/SOUL.md and
re-onboarding with the same sandbox name rebinds the old PVC, resurrecting
the files despite destroy claiming a clean teardown.

The durable PVC retention is owned upstream by OpenShell `sandbox delete`
semantics (NemoClaw can only exec-and-rm while the pod is live). Within
NemoClaw's control, improve the exec-fail warning to name the two self-serve
recovery paths: re-onboard with a different name (fresh PVC), or — on the
last sandbox — re-run destroy with --cleanup-gateway to purge the shared
cluster volume that retains the PVC so the same name comes up clean.

Refs #5970

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
---
 src/lib/actions/sandbox/wipe-state.ts   |  5 ++++-
 test/destroy-wipe-sandbox-state.test.ts | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/lib/actions/sandbox/wipe-state.ts b/src/lib/actions/sandbox/wipe-state.ts
index 3d735706efa..cf5b39a5363 100644
--- a/src/lib/actions/sandbox/wipe-state.ts
+++ b/src/lib/actions/sandbox/wipe-state.ts
@@ -219,7 +219,10 @@ export function wipeSandboxState(sandboxName: string, deps: WipeSandboxStateDeps
     // pins the gateway-select-then-exec-then-delete order.
     warn(
       `  ${YW}⚠${R} Could not wipe workspace state for '${sandboxName}' (sandbox not live?); ` +
-        "re-onboarding with the same name may resurface old files.",
+        "re-onboarding with the same name may resurface old files. " +
+        "To start clean, re-onboard with a different sandbox name, or — when this " +
+        "is your last sandbox — re-run destroy with --cleanup-gateway to purge the " +
+        "retained cluster volume.",
     );
   }
 }
diff --git a/test/destroy-wipe-sandbox-state.test.ts b/test/destroy-wipe-sandbox-state.test.ts
index f8a8b29d027..48114b2e337 100644
--- a/test/destroy-wipe-sandbox-state.test.ts
+++ b/test/destroy-wipe-sandbox-state.test.ts
@@ -101,6 +101,28 @@ describe("wipeSandboxState (#5449)", () => {
     }
   });
 
+  // #5970: when sandbox exec fails (sandbox not live, 100% CI repro), the warning
+  // must name actionable recovery paths so the user knows how to avoid stale
+  // workspace files after re-onboard. Two self-serve paths exist: re-onboard with
+  // a different name (fresh PVC), or --cleanup-gateway on the last sandbox (purges
+  // the shared cluster volume that retains the PVC, so the same name comes up clean).
+  it("names both recovery paths in the exec-fail warning so users can avoid stale workspace after re-onboard (#5970)", () => {
+    const warnings: string[] = [];
+    const { deps } = buildDeps({
+      runOpenshell: vi.fn(() => ({ status: 1 })),
+      warn: (msg: string) => warnings.push(msg),
+    });
+
+    destroy.wipeSandboxState("test-sb", deps as never);
+
+    const wipeWarn = warnings.find((w) => w.includes("Could not wipe workspace state"));
+    expect(wipeWarn).toBeDefined();
+    // Simple path: different name → fresh PVC (always works).
+    expect(wipeWarn).toContain("re-onboard with a different sandbox name");
+    // Same-name path: --cleanup-gateway purges the retained cluster volume.
+    expect(wipeWarn).toContain("--cleanup-gateway");
+  });
+
   // PRA-6 #5455: a manifest declaring a relative escape (e.g. `../etc`) or an
   // absolute path (e.g. `/etc/passwd`) in state_dirs/state_files would be
   // shell-quoted but fed straight into `rm -rf -- ...` inside `cd ${dir}`,