test: improve CLI test determinism and remove redundant test logic (#1123)

## Summary
Improves test determinism, consistency, and reliability across CLI,
uninstall, and blueprint test suites by standardizing shell invocation,
tightening execution patterns, and removing redundant or outdated test
code.

---

## Related Issue
Fixes #977 (part 1)

---

## Changes
- Normalize shell invocation:
- Replace `bash -lc` with `bash -c` in uninstall tests to avoid shell
initialization side effects
- Improve CLI test stability:
  - Increase timeouts for long-running commands
  - Standardize usage of `runWithEnv(..., timeout)`
- Remove redundant / outdated test code:
  - Clean up unused or deprecated test logic in `runner.test.ts`
- Improve test consistency:
  - Align execution patterns across CLI and uninstall tests
- Preserve security coverage:
- Maintain regression protections (e.g., path validation and credential
handling)

---

## Verification
- `npm test` passes locally
- `npx prek run --all-files` passes in CI
- No changes to CLI behavior or runtime logic
- Existing security and regression tests continue to pass

---

## Rationale
Some tests relied on shell initialization behavior (`bash -lc`) and
inconsistent execution patterns, leading to flakiness and
non-deterministic outcomes.

These updates:
- eliminate shell-dependent variability  
- standardize execution across test suites  
- improve reliability without impacting functionality  

Additionally, minor cleanup removes redundant or outdated test code to
improve maintainability.

---

## Risk Assessment
**Low risk**

- Changes are limited to test code and execution behavior  
- No production code paths modified  
- Security and regression coverage preserved  

**Rollback**
- Fully reversible by reverting test changes

---

## Type of Change
- [x] Test / infrastructure improvement (no behavioral change)
- [x] Code cleanup / maintenance

---

## Testing
- [x] `npm test` passes  
- [x] `npx prek run --all-files` passes (CI)  

---

## Checklist

### General
- [x] Contributing guide followed  

### Code Changes
- [x] Formatters applied  
- [x] No user-facing behavior changes  
- [x] No secrets committed  

---

## Summary by CodeRabbit (updated)
* **Tests**
* Improved CLI and uninstall test determinism by standardizing shell
invocation
  * Increased timeouts to reduce flakiness in long-running test cases
* Removed redundant or outdated test logic for improved maintainability


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
  * Improved TypeScript typing inside test mocks for safer compilation.
* Added a shared snapshot constant and strengthened a null-check
assertion.
* Simplified test environment handling by explicitly setting HOME and
adjusting shell invocation semantics.
* Removed redundant inline comment assertions and tightened output
checks.
* Minor formatting and clarity tweaks across test suites for easier
maintenance.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Krish Sapru <ksapru@bu.edu>

---------

Co-authored-by: Carlos Villela <cvillela@nvidia.com>

Author: ksapru

nemoclaw/src/blueprint/runner.test.ts

@@ -32,7 +32,7 @@ vi.mock("node:crypto", () => ({
 }));
 
 vi.mock("node:fs", async (importOriginal) => {
-  const original = await importOriginal();
+  const original = await importOriginal() as typeof import("node:fs");
   return {
     ...original,
     existsSync: (p: string) => store.has(p),

nemoclaw/src/blueprint/snapshot.test.ts

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+const SNAP = "/snap/20260323";
 
 // ── In-memory filesystem ────────────────────────────────────────
 
@@ -22,12 +23,13 @@ function addDir(p: string): void {
 
 const FAKE_HOME = "/fakehome";
 
+
 vi.mock("node:os", () => ({
   homedir: () => FAKE_HOME,
 }));
 
 vi.mock("node:fs", async (importOriginal) => {
-  const original = await importOriginal();
+  const original = await importOriginal<typeof import("node:fs")>();
   return {
     ...original,
     existsSync: (p: string) => store.has(p),
@@ -123,7 +125,9 @@ describe("snapshot", () => {
 
       const result = createSnapshot();
 
+      expect(result).not.toBeNull();
       if (!result) throw new Error("createSnapshot returned null");
+      
       expect(result.startsWith(SNAPSHOTS_DIR)).toBe(true);
 
       // Manifest was written
@@ -140,34 +144,34 @@ describe("snapshot", () => {
 
   describe("restoreIntoSandbox", () => {
     it("returns false when snapshot has no openclaw dir", async () => {
-      addDir("/snap/20260323");
-      expect(await restoreIntoSandbox("/snap/20260323")).toBe(false);
+      addDir(SNAP);
+      expect(await restoreIntoSandbox(SNAP)).toBe(false);
     });
 
     it("calls openshell sandbox cp and returns true on success", async () => {
-      addDir("/snap/20260323/openclaw");
+      addDir(`${SNAP}/openclaw`);
       mockExeca.mockResolvedValue({ exitCode: 0 });
 
-      expect(await restoreIntoSandbox("/snap/20260323", "mybox")).toBe(true);
+      expect(await restoreIntoSandbox(SNAP, "mybox")).toBe(true);
       expect(mockExeca).toHaveBeenCalledWith(
         "openshell",
-        ["sandbox", "cp", "/snap/20260323/openclaw", "mybox:/sandbox/.openclaw"],
+        ["sandbox", "cp", `${SNAP}/openclaw`, "mybox:/sandbox/.openclaw"],
         { reject: false },
       );
     });
 
     it("returns false when openshell fails", async () => {
-      addDir("/snap/20260323/openclaw");
+      addDir(`${SNAP}/openclaw`);
       mockExeca.mockResolvedValue({ exitCode: 1 });
 
-      expect(await restoreIntoSandbox("/snap/20260323")).toBe(false);
+      expect(await restoreIntoSandbox(SNAP)).toBe(false);
     });
 
     it("uses default sandbox name 'openclaw'", async () => {
-      addDir("/snap/20260323/openclaw");
+      addDir(`${SNAP}/openclaw`);
       mockExeca.mockResolvedValue({ exitCode: 0 });
 
-      await restoreIntoSandbox("/snap/20260323");
+      await restoreIntoSandbox(SNAP);
       expect(mockExeca).toHaveBeenCalledWith(
         "openshell",
         expect.arrayContaining(["openclaw:/sandbox/.openclaw"]),
@@ -207,15 +211,15 @@ describe("snapshot", () => {
 
   describe("rollbackFromSnapshot", () => {
     it("returns false when snapshot openclaw dir is missing", () => {
-      addDir("/snap/20260323");
-      expect(rollbackFromSnapshot("/snap/20260323")).toBe(false);
+      addDir(SNAP);
+      expect(rollbackFromSnapshot(SNAP)).toBe(false);
     });
 
     it("restores snapshot to ~/.openclaw with content", () => {
-      addDir("/snap/20260323/openclaw");
-      addFile("/snap/20260323/openclaw/openclaw.json", '{"restored":true}');
+      addDir(`${SNAP}/openclaw`);
+      addFile(`${SNAP}/openclaw/openclaw.json`, '{"restored":true}');
 
-      expect(rollbackFromSnapshot("/snap/20260323")).toBe(true);
+      expect(rollbackFromSnapshot(SNAP)).toBe(true);
 
       const restored = store.get(`${OPENCLAW_DIR}/openclaw.json`);
       if (!restored) throw new Error("openclaw.json not restored");
@@ -225,10 +229,10 @@ describe("snapshot", () => {
     it("archives existing ~/.openclaw before restoring", () => {
       addDir(OPENCLAW_DIR);
       addFile(`${OPENCLAW_DIR}/openclaw.json`, '{"old":true}');
-      addDir("/snap/20260323/openclaw");
-      addFile("/snap/20260323/openclaw/openclaw.json", '{"restored":true}');
+      addDir(`${SNAP}/openclaw`);
+      addFile(`${SNAP}/openclaw/openclaw.json`, '{"restored":true}');
 
-      expect(rollbackFromSnapshot("/snap/20260323")).toBe(true);
+      expect(rollbackFromSnapshot(SNAP)).toBe(true);
 
       const archived = [...store.keys()].find((k) => k.includes(".openclaw.nemoclaw-archived."));
       expect(archived).toBeDefined();

nemoclaw/src/blueprint/state.test.ts

@@ -7,7 +7,7 @@ import { loadState, saveState, clearState, type NemoClawState } from "./state.js
 const store = new Map<string, string>();
 
 vi.mock("node:fs", async (importOriginal) => {
-  const original = await importOriginal();
+  const original = await importOriginal() as typeof import("node:fs");
   return {
     ...original,
     existsSync: (p: string) => store.has(p),

test/uninstall.test.js

@@ -16,6 +16,7 @@ function createFakeNpmEnv(tmp) {
   fs.writeFileSync(npmPath, "#!/usr/bin/env bash\nexit 0\n", { mode: 0o755 });
   return {
     ...process.env,
+    HOME: tmp,
     PATH: `${fakeBin}:${process.env.PATH || "/usr/bin:/bin"}`,
   };
 }
@@ -36,14 +37,21 @@ describe("uninstall CLI flags", () => {
   });
 
   it("--yes skips the confirmation prompt and completes successfully", () => {
-    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-yes-"));
+    const tmp = fs.mkdtempSync(
+      path.join(os.tmpdir(), "nemoclaw-uninstall-yes-"),
+    );
     const fakeBin = path.join(tmp, "bin");
     fs.mkdirSync(fakeBin);
 
     try {
-      // Provide stub executables so the uninstaller can run its steps as no-ops
       for (const cmd of ["npm", "openshell", "docker", "ollama", "pgrep"]) {
-        fs.writeFileSync(path.join(fakeBin, cmd), "#!/usr/bin/env bash\nexit 0\n", { mode: 0o755 });
+        fs.writeFileSync(
+          path.join(fakeBin, cmd),
+          "#!/usr/bin/env bash\nexit 0\n",
+          {
+            mode: 0o755,
+          },
+        );
       }
 
       const result = spawnSync("bash", [UNINSTALL_SCRIPT, "--yes"], {
@@ -58,7 +66,6 @@ describe("uninstall CLI flags", () => {
       });
 
       expect(result.status).toBe(0);
-      // Banner and bye statement should be present
       const output = `${result.stdout}${result.stderr}`;
       expect(output).toMatch(/NemoClaw/);
       expect(output).toMatch(/Claws retracted/);
@@ -72,7 +79,10 @@ describe("uninstall helpers", () => {
   it("returns the expected gateway volume candidate", () => {
     const result = spawnSync(
       "bash",
-      ["-lc", `source "${UNINSTALL_SCRIPT}"; gateway_volume_candidates nemoclaw`],
+      [
+        "-c",
+        `source "${UNINSTALL_SCRIPT}"; gateway_volume_candidates nemoclaw`,
+      ],
       {
         cwd: path.join(import.meta.dirname, ".."),
         encoding: "utf-8",
@@ -84,18 +94,21 @@ describe("uninstall helpers", () => {
   });
 
   it("removes the user-local nemoclaw shim", () => {
-    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-shim-"));
+    const tmp = fs.mkdtempSync(
+      path.join(os.tmpdir(), "nemoclaw-uninstall-shim-"),
+    );
     const shimDir = path.join(tmp, ".local", "bin");
     const shimPath = path.join(shimDir, "nemoclaw");
     const targetPath = path.join(tmp, "prefix", "bin", "nemoclaw");
+
     fs.mkdirSync(shimDir, { recursive: true });
     fs.mkdirSync(path.dirname(targetPath), { recursive: true });
     fs.writeFileSync(targetPath, "#!/usr/bin/env bash\n", { mode: 0o755 });
     fs.symlinkSync(targetPath, shimPath);
 
     const result = spawnSync(
       "bash",
-      ["-lc", `HOME="${tmp}" source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_cli`],
+      ["-c", `source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_cli`],
       {
         cwd: path.join(import.meta.dirname, ".."),
         encoding: "utf-8",
@@ -108,15 +121,18 @@ describe("uninstall helpers", () => {
   });
 
   it("preserves a user-managed nemoclaw file in the shim directory", () => {
-    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-preserve-"));
+    const tmp = fs.mkdtempSync(
+      path.join(os.tmpdir(), "nemoclaw-uninstall-preserve-"),
+    );
     const shimDir = path.join(tmp, ".local", "bin");
     const shimPath = path.join(shimDir, "nemoclaw");
+
     fs.mkdirSync(shimDir, { recursive: true });
     fs.writeFileSync(shimPath, "#!/usr/bin/env bash\n", { mode: 0o755 });
 
     const result = spawnSync(
       "bash",
-      ["-lc", `HOME="${tmp}" source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_cli`],
+      ["-c", `source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_cli`],
       {
         cwd: path.join(import.meta.dirname, ".."),
         encoding: "utf-8",
@@ -126,22 +142,28 @@ describe("uninstall helpers", () => {
 
     expect(result.status).toBe(0);
     expect(fs.existsSync(shimPath)).toBe(true);
-    expect(`${result.stdout}${result.stderr}`).toMatch(/not an installer-managed shim/);
+    expect(`${result.stdout}${result.stderr}`).toMatch(
+      /not an installer-managed shim/,
+    );
   });
 
   it("removes the onboard session file as part of NemoClaw state cleanup", () => {
-    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-session-"));
+    const tmp = fs.mkdtempSync(
+      path.join(os.tmpdir(), "nemoclaw-uninstall-session-"),
+    );
     const stateDir = path.join(tmp, ".nemoclaw");
     const sessionPath = path.join(stateDir, "onboard-session.json");
+
     fs.mkdirSync(stateDir, { recursive: true });
     fs.writeFileSync(sessionPath, JSON.stringify({ status: "complete" }));
 
     const result = spawnSync(
       "bash",
-      ["-lc", `HOME="${tmp}" source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_state`],
+      ["-c", `source "${UNINSTALL_SCRIPT}"; remove_nemoclaw_state`],
       {
         cwd: path.join(import.meta.dirname, ".."),
         encoding: "utf-8",
+        env: { ...process.env, HOME: tmp },
       },
     );